Remove ProjectControl from httpd/ProjectAccess stack

Change-Id: Ie3fc72937ce6c4fee14ae9d87b47164cff2afd38
2017-10-11 13:40:04 +02:00
parent bc43ac3326
commit eddbd526a5
4 changed files with 26 additions and 53 deletions
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
@@ -30,7 +30,6 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ContributorAgreementsChecker;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.SetParent;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -58,7 +57,6 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
  @Inject
  ChangeProjectAccess(
      ProjectAccessFactory.Factory projectAccessFactory,
-      ProjectControl.Factory projectControlFactory,
      ProjectCache projectCache,
      GroupBackend groupBackend,
      MetaDataUpdate.User metaDataUpdateFactory,
@@ -74,7 +72,6 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
      @Nullable @Assisted("parentProjectName") Project.NameKey parentProjectName,
      @Nullable @Assisted String message) {
    super(
-        projectControlFactory,
        groupBackend,
        metaDataUpdateFactory,
        allProjects,
@@ -95,10 +92,7 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {

  @Override
  protected ProjectAccess updateProjectConfig(
-      ProjectControl projectControl,
-      ProjectConfig config,
-      MetaDataUpdate md,
-      boolean parentProjectUpdate)
+      ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
      throws IOException, NoSuchProjectException, ConfigInvalidException,
          PermissionBackendException {
    RevCommit commit = config.commit(md);
@@ -108,7 +102,7 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
        RefNames.REFS_CONFIG,
        base,
        commit.getId(),
-        projectControl.getUser().asIdentifiedUser().getAccount());
+        user.asIdentifiedUser().getAccount());

    projectCache.evict(config.getProject());
    return projectAccessFactory.create(projectName).call();
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -48,7 +48,7 @@ import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
@@ -70,7 +70,6 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
  private final ProjectCache projectCache;
  private final PermissionBackend permissionBackend;
  private final Provider<CurrentUser> user;
-  private final ProjectControl.GenericFactory projectControlFactory;
  private final GroupControl.Factory groupControlFactory;
  private final MetaDataUpdate.Server metaDataUpdateFactory;
  private final AllProjectsName allProjectsName;
@@ -84,7 +83,6 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
      ProjectCache projectCache,
      PermissionBackend permissionBackend,
      Provider<CurrentUser> user,
-      ProjectControl.GenericFactory projectControlFactory,
      GroupControl.Factory groupControlFactory,
      MetaDataUpdate.Server metaDataUpdateFactory,
      AllProjectsName allProjectsName,
@@ -94,7 +92,6 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
    this.projectCache = projectCache;
    this.permissionBackend = permissionBackend;
    this.user = user;
-    this.projectControlFactory = projectControlFactory;
    this.groupControlFactory = groupControlFactory;
    this.metaDataUpdateFactory = metaDataUpdateFactory;
    this.allProjectsName = allProjectsName;
@@ -107,7 +104,7 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
  public ProjectAccess call()
      throws NoSuchProjectException, IOException, ConfigInvalidException,
          PermissionBackendException {
-    ProjectControl pc = checkProjectControl();
+    ProjectState projectState = checkProjectState();

    // Load the current configuration from the repository, ensuring its the most
    // recent version available. If it differs from what was in the project
@@ -120,11 +117,11 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
        md.setMessage("Update group names\n");
        config.commit(md);
        projectCache.evict(config.getProject());
-        pc = checkProjectControl();
+        projectState = checkProjectState();
      } else if (config.getRevision() != null
-          && !config.getRevision().equals(pc.getProjectState().getConfig().getRevision())) {
+          && !config.getRevision().equals(projectState.getConfig().getRevision())) {
        projectCache.evict(config.getProject());
-        pc = checkProjectControl();
+        projectState = checkProjectState();
      }
    }

@@ -228,7 +225,7 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
            || (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE)));
    detail.setConfigVisible(canWriteProjectConfig || checkReadConfig);
    detail.setGroupInfo(buildGroupInfo(local));
-    detail.setLabelTypes(pc.getProjectState().getLabelTypes());
+    detail.setLabelTypes(projectState.getLabelTypes());
    detail.setFileHistoryLinks(getConfigFileLogLinks(projectName.get()));
    return detail;
  }
@@ -258,15 +255,15 @@ class ProjectAccessFactory extends Handler<ProjectAccess> {
    return Maps.filterEntries(infos, in -> in.getValue() != null);
  }

-  private ProjectControl checkProjectControl()
+  private ProjectState checkProjectState()
      throws NoSuchProjectException, IOException, PermissionBackendException {
-    ProjectControl pc = projectControlFactory.controlFor(projectName, user.get());
+    ProjectState state = projectCache.checkedGet(projectName);
    try {
      permissionBackend.user(user).project(projectName).check(ProjectPermission.ACCESS);
    } catch (AuthException e) {
      throw new NoSuchProjectException(projectName);
    }
-    return pc;
+    return state;
  }

  private static boolean check(PermissionBackend.ForProject ctx, String ref, RefPermission perm)
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -43,7 +43,6 @@ import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.ContributorAgreementsChecker;
 import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.RefPattern;
 import com.google.gerrit.server.project.SetParent;
 import com.google.gwtorm.server.OrmException;
@@ -58,25 +57,25 @@ import org.eclipse.jgit.lib.ObjectId;

 public abstract class ProjectAccessHandler<T> extends Handler<T> {

-  private final ProjectControl.Factory projectControlFactory;
  protected final GroupBackend groupBackend;
+  protected final Project.NameKey projectName;
+  protected final ObjectId base;
+  protected final CurrentUser user;
+
  private final MetaDataUpdate.User metaDataUpdateFactory;
  private final AllProjectsName allProjects;
  private final Provider<SetParent> setParent;
  private final ContributorAgreementsChecker contributorAgreements;
  private final PermissionBackend permissionBackend;
-
-  protected final Project.NameKey projectName;
-  protected final ObjectId base;
-  private List<AccessSection> sectionList;
  private final Project.NameKey parentProjectName;
+
  protected String message;
+
+  private List<AccessSection> sectionList;
  private boolean checkIfOwner;
-  private CurrentUser user;
  private Boolean canWriteConfig;

  protected ProjectAccessHandler(
-      ProjectControl.Factory projectControlFactory,
      GroupBackend groupBackend,
      MetaDataUpdate.User metaDataUpdateFactory,
      AllProjectsName allProjects,
@@ -90,7 +89,6 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
      ContributorAgreementsChecker contributorAgreements,
      PermissionBackend permissionBackend,
      boolean checkIfOwner) {
-    this.projectControlFactory = projectControlFactory;
    this.groupBackend = groupBackend;
    this.metaDataUpdateFactory = metaDataUpdateFactory;
    this.allProjects = allProjects;
@@ -112,11 +110,8 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
      throws NoSuchProjectException, IOException, ConfigInvalidException, InvalidNameException,
          NoSuchGroupException, OrmException, UpdateParentFailedException,
          PermissionDeniedException, PermissionBackendException {
-    final ProjectControl projectControl = projectControlFactory.controlFor(projectName);
-    this.user = projectControl.getUser();
-
    try {
-      contributorAgreements.check(projectName, projectControl.getUser());
+      contributorAgreements.check(projectName, user);
    } catch (AuthException e) {
      throw new PermissionDeniedException(e.getMessage());
    }
@@ -165,8 +160,8 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
          setParent
              .get()
              .validateParentUpdate(
-                  projectControl.getProject().getNameKey(),
-                  projectControl.getUser().asIdentifiedUser(),
+                  projectName,
+                  user.asIdentifiedUser(),
                  MoreObjects.firstNonNull(parentProjectName, allProjects).get(),
                  checkIfOwner);
        } catch (AuthException e) {
@@ -190,17 +185,14 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
        md.setMessage("Modify access rules\n");
      }

-      return updateProjectConfig(projectControl, config, md, parentProjectUpdate);
+      return updateProjectConfig(config, md, parentProjectUpdate);
    } catch (RepositoryNotFoundException notFound) {
      throw new NoSuchProjectException(projectName);
    }
  }

  protected abstract T updateProjectConfig(
-      ProjectControl projectControl,
-      ProjectConfig config,
-      MetaDataUpdate md,
-      boolean parentProjectUpdate)
+      ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
      throws IOException, NoSuchProjectException, ConfigInvalidException, OrmException,
          PermissionDeniedException, PermissionBackendException;

--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
@@ -47,7 +47,6 @@ import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.ContributorAgreementsChecker;
 import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.SetParent;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.UpdateException;
@@ -84,7 +83,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {

  @Inject
  ReviewProjectAccess(
-      final ProjectControl.Factory projectControlFactory,
      PermissionBackend permissionBackend,
      GroupBackend groupBackend,
      MetaDataUpdate.User metaDataUpdateFactory,
@@ -105,7 +103,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
      @Nullable @Assisted("parentProjectName") Project.NameKey parentProjectName,
      @Nullable @Assisted String message) {
    super(
-        projectControlFactory,
        groupBackend,
        metaDataUpdateFactory,
        allProjects,
@@ -134,15 +131,9 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
  @SuppressWarnings("deprecation")
  @Override
  protected Change.Id updateProjectConfig(
-      ProjectControl projectControl,
-      ProjectConfig config,
-      MetaDataUpdate md,
-      boolean parentProjectUpdate)
+      ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
      throws IOException, OrmException, PermissionDeniedException, PermissionBackendException {
-    PermissionBackend.ForProject perm =
-        permissionBackend
-            .user(projectControl.getUser())
-            .project(projectControl.getProject().getNameKey());
+    PermissionBackend.ForProject perm = permissionBackend.user(user).project(config.getName());
    if (!check(perm, ProjectPermission.READ_CONFIG)) {
      throw new PermissionDeniedException(RefNames.REFS_CONFIG + " not visible");
    }
@@ -165,8 +156,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
        ObjectReader objReader = objInserter.newReader();
        RevWalk rw = new RevWalk(objReader);
        BatchUpdate bu =
-            updateFactory.create(
-                db, config.getProject().getNameKey(), projectControl.getUser(), TimeUtil.nowTs())) {
+            updateFactory.create(db, config.getProject().getNameKey(), user, TimeUtil.nowTs())) {
      bu.setRepository(md.getRepository(), rw, objInserter);
      bu.insertChange(
          changeInserterFactory