From 91fefc45e930e2e7b8ba89d906d81c2da333e84d Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 18 Nov 2013 16:37:42 +0100 Subject: [PATCH] Allow users to save update of parent project for review Only Gerrit administrators are allowed to change the parent of a project. Now normal users can change the parent project in the WebUI and save the modification for review. This creates a change for the refs/meta/config branch that can be approved and submitted by administrators. Since the administrators are the only ones who can submit this change they are automatically added as reviewer. This way we have an integrated workflow for requesting and approving changes of parent projects, instead of having requests by email outside of Gerrit. Change-Id: I080649990f80edf78fd6eb465cef25fb8012352d Signed-off-by: Edwin Kempin --- .../client/admin/ProjectAccessEditor.java | 2 +- .../rpc/project/ChangeProjectAccess.java | 3 +- .../rpc/project/ProjectAccessHandler.java | 16 ++++++--- .../rpc/project/ReviewProjectAccess.java | 34 +++++++++++++++++-- .../gerrit/server/project/SetParent.java | 8 ++--- 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java index 567f14caaa..02a19dc87d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java @@ -112,7 +112,7 @@ public class ProjectAccessEditor extends Composite implements parentProject.setTargetHistoryToken( // Dispatcher.toProjectAdmin(parent, ProjectScreen.ACCESS)); - parentProjectBox.setVisible(editing && value.canChangeParent()); + parentProjectBox.setVisible(editing); parentProjectBox.setProject(value.getProjectName()); parentProjectBox.setParentProject(value.getInheritsFrom()); parentProject.setVisible(!parentProjectBox.isVisible()); diff --git 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 index 57be31e41c..2b6c128d88 100644 --- 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 @@ -71,7 +71,8 @@ class ChangeProjectAccess extends ProjectAccessHandler { @Override protected ProjectAccess updateProjectConfig(ProjectConfig config, - MetaDataUpdate md) throws IOException, NoSuchProjectException, ConfigInvalidException { + MetaDataUpdate md, boolean parentProjectUpdate) throws IOException, + NoSuchProjectException, ConfigInvalidException { config.commit(md); projectCache.evict(config.getProject()); return projectAccessFactory.create(projectName).call(); diff --git 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 index 971c4b3caf..783915a7d5 100644 --- 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 @@ -132,12 +132,18 @@ public abstract class ProjectAccessHandler extends Handler { } } + boolean parentProjectUpdate = false; if (!config.getProject().getNameKey().equals(allProjects.get()) && !config.getProject().getParent(allProjects.get()).equals(parentProjectName)) { + parentProjectUpdate = true; try { - setParent.get().validateParentUpdate(projectControl, parentProjectName.get()); + setParent.get().validateParentUpdate(projectControl, + parentProjectName.get(), checkIfOwner); } catch (AuthException e) { - throw new UpdateParentFailedException(e.getMessage(), e); + throw new UpdateParentFailedException( + "You are not allowed to change the parent project since you are " + + "not an administrator. You may save the modifications for review " + + "so that an administrator can approve them.", e); } catch (ResourceConflictException e) { throw new UpdateParentFailedException(e.getMessage(), e); } catch (UnprocessableEntityException e) { @@ -155,15 +161,15 @@ public abstract class ProjectAccessHandler extends Handler { md.setMessage("Modify access rules\n"); } - return updateProjectConfig(config, md); + return updateProjectConfig(config, md, parentProjectUpdate); } finally { md.close(); } } protected abstract T updateProjectConfig(ProjectConfig config, - MetaDataUpdate md) throws IOException, NoSuchProjectException, - ConfigInvalidException, OrmException; + MetaDataUpdate md, boolean parentProjectUpdate) throws IOException, + NoSuchProjectException, ConfigInvalidException, OrmException; private void replace(ProjectConfig config, Set toDelete, AccessSection section) throws NoSuchGroupException { diff --git 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 index 3f4030dec2..946a703816 100644 --- 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 @@ -17,6 +17,8 @@ package com.google.gerrit.httpd.rpc.project; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GlobalCapability; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; @@ -39,6 +41,7 @@ import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.ChangeControl; +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.util.TimeUtil; @@ -78,6 +81,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { private final ChangeIndexer indexer; private final ChangeHooks hooks; private final CreateChangeSender.Factory createChangeSenderFactory; + private final ProjectCache projectCache; @Inject ReviewProjectAccess(final ProjectControl.Factory projectControlFactory, @@ -88,6 +92,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { ChangeControl.GenericFactory changeFactory, ChangeIndexer indexer, ChangeHooks hooks, CreateChangeSender.Factory createChangeSenderFactory, + ProjectCache projectCache, AllProjectsNameProvider allProjects, Provider setParent, @@ -107,11 +112,13 @@ public class ReviewProjectAccess extends ProjectAccessHandler { this.indexer = indexer; this.hooks = hooks; this.createChangeSenderFactory = createChangeSenderFactory; + this.projectCache = projectCache; } @Override - protected Change.Id updateProjectConfig(ProjectConfig config, MetaDataUpdate md) - throws IOException, OrmException { + protected Change.Id updateProjectConfig(ProjectConfig config, + MetaDataUpdate md, boolean parentProjectUpdate) throws IOException, + OrmException { Change.Id changeId = new Change.Id(db.nextChangeId()); PatchSet ps = new PatchSet(new PatchSet.Id(changeId, Change.INITIAL_PATCH_SET_ID)); @@ -158,6 +165,9 @@ public class ReviewProjectAccess extends ProjectAccessHandler { log.error("Cannot send email for new change " + change.getId(), err); } addProjectOwnersAsReviewers(change); + if (parentProjectUpdate) { + addAdministratorsAsReviewers(change); + } return changeId; } @@ -175,7 +185,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { db.patchSetAncestors().insert(toInsert); } - private void addProjectOwnersAsReviewers(final Change change) { + private void addProjectOwnersAsReviewers(Change change) { final String projectOwners = groupBackend.get(AccountGroup.PROJECT_OWNERS).getName(); try { @@ -189,4 +199,22 @@ public class ReviewProjectAccess extends ProjectAccessHandler { // can't be added as reviewer } } + + private void addAdministratorsAsReviewers(Change change) { + List adminRules = + projectCache.getAllProjects().getConfig() + .getAccessSection(AccessSection.GLOBAL_CAPABILITIES) + .getPermission(GlobalCapability.ADMINISTRATE_SERVER).getRules(); + for (PermissionRule r : adminRules) { + try { + ChangeResource rsrc = + new ChangeResource(changeFactory.controlFor(change, user)); + PostReviewers.Input input = new PostReviewers.Input(); + input.reviewer = r.getGroup().getUUID().get(); + reviewersProvider.get().apply(rsrc, input); + } catch (Exception e) { + // ignore + } + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java index 337c6f4415..dd989a5466 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java @@ -62,7 +62,7 @@ public class SetParent implements RestModifyView { throws AuthException, ResourceConflictException, ResourceNotFoundException, UnprocessableEntityException, IOException { ProjectControl ctl = rsrc.getControl(); - validateParentUpdate(ctl, input.parent); + validateParentUpdate(ctl, input.parent, true); IdentifiedUser user = (IdentifiedUser) ctl.getCurrentUser(); try { MetaDataUpdate md = updateFactory.create(rsrc.getNameKey()); @@ -97,11 +97,11 @@ public class SetParent implements RestModifyView { } } - public void validateParentUpdate(final ProjectControl ctl, String newParent) - throws AuthException, ResourceConflictException, + public void validateParentUpdate(final ProjectControl ctl, String newParent, + boolean checkIfAdmin) throws AuthException, ResourceConflictException, UnprocessableEntityException { IdentifiedUser user = (IdentifiedUser) ctl.getCurrentUser(); - if (!user.getCapabilities().canAdministrateServer()) { + if (checkIfAdmin && !user.getCapabilities().canAdministrateServer()) { throw new AuthException("not administrator"); }