From 3ee57453d4ae01ad6a6b131d6a19dbc0c560f706 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Fri, 25 Aug 2017 14:53:35 +0200 Subject: [PATCH] Reduce usage of RefControl Reduce the usage of RefControl by replacing it with Branch.NameKey and IdentifiedUser where it was just used as a container for these. Eventually, {Ref,Change,Project}Control should be package-private. This commit is an incremental step towards that goal. Change-Id: I649fd89ff3c2522e043384fb234bea9454019847 --- .../gerrit/server/change/ChangeInserter.java | 23 +++--- .../gerrit/server/change/CherryPick.java | 6 +- .../server/change/CherryPickChange.java | 36 ++++----- .../server/change/CherryPickCommit.java | 6 +- .../server/change/PatchSetInserter.java | 9 ++- .../server/git/receive/ReceiveCommits.java | 45 ++++++----- .../git/validators/CommitValidators.java | 76 +++++++++++-------- 7 files changed, 112 insertions(+), 89 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index d4009999b6..ac7248b0f8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -57,9 +57,7 @@ import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.NoSuchProjectException; -import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.RefControl; +import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.Context; @@ -94,7 +92,7 @@ public class ChangeInserter implements InsertChangeOp { private static final Logger log = LoggerFactory.getLogger(ChangeInserter.class); private final PermissionBackend permissionBackend; - private final ProjectControl.GenericFactory projectControlFactory; + private final ProjectCache projectCache; private final IdentifiedUser.GenericFactory userFactory; private final ChangeControl.GenericFactory changeControlFactory; private final PatchSetInfoFactory patchSetInfoFactory; @@ -144,7 +142,7 @@ public class ChangeInserter implements InsertChangeOp { @Inject ChangeInserter( PermissionBackend permissionBackend, - ProjectControl.GenericFactory projectControlFactory, + ProjectCache projectCache, IdentifiedUser.GenericFactory userFactory, ChangeControl.GenericFactory changeControlFactory, PatchSetInfoFactory patchSetInfoFactory, @@ -161,7 +159,7 @@ public class ChangeInserter implements InsertChangeOp { @Assisted ObjectId commitId, @Assisted String refName) { this.permissionBackend = permissionBackend; - this.projectControlFactory = projectControlFactory; + this.projectCache = projectCache; this.userFactory = userFactory; this.changeControlFactory = changeControlFactory; this.patchSetInfoFactory = patchSetInfoFactory; @@ -567,24 +565,25 @@ public class ChangeInserter implements InsertChangeOp { PermissionBackend.ForRef perm = permissionBackend.user(ctx.getUser()).project(ctx.getProject()).ref(refName); try { - RefControl refControl = - projectControlFactory.controlFor(ctx.getProject(), ctx.getUser()).controlForRef(refName); try (CommitReceivedEvent event = new CommitReceivedEvent( cmd, - refControl.getProjectControl().getProject(), + projectCache.checkedGet(ctx.getProject()).getProject(), change.getDest().get(), ctx.getRevWalk().getObjectReader(), commitId, ctx.getIdentifiedUser())) { commitValidatorsFactory - .forGerritCommits(perm, refControl, new NoSshInfo(), ctx.getRevWalk()) + .forGerritCommits( + perm, + new Branch.NameKey(ctx.getProject(), refName), + ctx.getIdentifiedUser(), + new NoSshInfo(), + ctx.getRevWalk()) .validate(event); } } catch (CommitValidationException e) { throw new ResourceConflictException(e.getFullMessage()); - } catch (NoSuchProjectException e) { - throw new ResourceConflictException(e.getMessage()); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java index 930cb8b9df..f3c5f0aaf4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.webui.UiAction; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.CurrentUser; @@ -32,6 +33,7 @@ import com.google.gerrit.server.permissions.ProjectPermission; import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryingRestModifyView; @@ -70,7 +72,7 @@ public class CherryPick public ChangeInfo applyImpl( BatchUpdate.Factory updateFactory, RevisionResource rsrc, CherryPickInput input) throws OrmException, IOException, UpdateException, RestApiException, - PermissionBackendException, ConfigInvalidException { + PermissionBackendException, ConfigInvalidException, NoSuchProjectException { input.parent = input.parent == null ? 1 : input.parent; if (input.message == null || input.message.trim().isEmpty()) { throw new BadRequestException("message must be non-empty"); @@ -93,7 +95,7 @@ public class CherryPick rsrc.getChange(), rsrc.getPatchSet(), input, - rsrc.getControl().getProjectControl().controlForRef(refName)); + new Branch.NameKey(rsrc.getProject(), refName)); return json.noOptions().format(rsrc.getProject(), cherryPickedChangeId); } catch (InvalidChangeOperationException e) { throw new BadRequestException(e.getMessage()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index deb379da6e..6e555e54a3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java @@ -31,7 +31,6 @@ import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; @@ -51,8 +50,9 @@ import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.NoSuchProjectException; +import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectState; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.update.BatchUpdate; @@ -94,6 +94,7 @@ public class CherryPickChange { private final PatchSetInserter.Factory patchSetInserterFactory; private final MergeUtil.Factory mergeUtilFactory; private final ChangeNotes.Factory changeNotesFactory; + private final ProjectControl.GenericFactory projectControlFactory; private final ApprovalsUtil approvalsUtil; private final ChangeMessagesUtil changeMessagesUtil; private final PatchSetUtil psUtil; @@ -111,6 +112,7 @@ public class CherryPickChange { PatchSetInserter.Factory patchSetInserterFactory, MergeUtil.Factory mergeUtilFactory, ChangeNotes.Factory changeNotesFactory, + ProjectControl.GenericFactory projectControlFactory, ApprovalsUtil approvalsUtil, ChangeMessagesUtil changeMessagesUtil, PatchSetUtil psUtil, @@ -125,6 +127,7 @@ public class CherryPickChange { this.patchSetInserterFactory = patchSetInserterFactory; this.mergeUtilFactory = mergeUtilFactory; this.changeNotesFactory = changeNotesFactory; + this.projectControlFactory = projectControlFactory; this.approvalsUtil = approvalsUtil; this.changeMessagesUtil = changeMessagesUtil; this.psUtil = psUtil; @@ -136,9 +139,9 @@ public class CherryPickChange { Change change, PatchSet patch, CherryPickInput input, - RefControl refControl) + Branch.NameKey dest) throws OrmException, IOException, InvalidChangeOperationException, IntegrationException, - UpdateException, RestApiException, ConfigInvalidException { + UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException { return cherryPick( batchUpdateFactory, change, @@ -146,7 +149,7 @@ public class CherryPickChange { change.getProject(), ObjectId.fromString(patch.getRevision().get()), input, - refControl); + dest); } public Change.Id cherryPick( @@ -156,9 +159,9 @@ public class CherryPickChange { Project.NameKey project, ObjectId sourceCommit, CherryPickInput input, - RefControl destRefControl) + Branch.NameKey dest) throws OrmException, IOException, InvalidChangeOperationException, IntegrationException, - UpdateException, RestApiException, ConfigInvalidException { + UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException { IdentifiedUser identifiedUser = user.get(); try (Repository git = gitManager.openRepository(project); @@ -168,11 +171,10 @@ public class CherryPickChange { ObjectInserter oi = git.newObjectInserter(); ObjectReader reader = oi.newReader(); CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) { - String destRefName = destRefControl.getRefName(); - Ref destRef = git.getRefDatabase().exactRef(destRefName); + Ref destRef = git.getRefDatabase().exactRef(dest.get()); if (destRef == null) { throw new InvalidChangeOperationException( - String.format("Branch %s does not exist.", destRefName)); + String.format("Branch %s does not exist.", dest.get())); } RevCommit baseCommit = getBaseCommit(destRef, project.get(), revWalk, input.base); @@ -200,8 +202,10 @@ public class CherryPickChange { String commitMessage = ChangeIdUtil.insertId(input.message, computedChangeId).trim() + '\n'; CodeReviewCommit cherryPickCommit; + ProjectControl projectControl = + projectControlFactory.controlFor(dest.getParentKey(), identifiedUser); try { - ProjectState projectState = destRefControl.getProjectControl().getProjectState(); + ProjectState projectState = projectControl.getProjectState(); cherryPickCommit = mergeUtilFactory .create(projectState) @@ -242,8 +246,7 @@ public class CherryPickChange { if (destChanges.size() == 1) { // The change key exists on the destination branch. The cherry pick // will be added as a new patch set. - ChangeControl destCtl = - destRefControl.getProjectControl().controlFor(destChanges.get(0).notes()); + ChangeControl destCtl = projectControl.controlFor(destChanges.get(0).notes()); result = insertPatchSet(bu, git, destCtl, cherryPickCommit, input); } else { // Change key not found on destination branch. We can create a new @@ -254,16 +257,13 @@ public class CherryPickChange { } result = createNewChange( - bu, cherryPickCommit, destRefName, newTopic, sourceChange, sourceCommit, input); + bu, cherryPickCommit, dest.get(), newTopic, sourceChange, sourceCommit, input); if (sourceChange != null && sourcePatchId != null) { bu.addOp( sourceChange.getId(), new AddMessageToSourceChangeOp( - changeMessagesUtil, - sourcePatchId, - RefNames.shortName(destRefName), - cherryPickCommit)); + changeMessagesUtil, sourcePatchId, dest.getShortName(), cherryPickCommit)); } } bu.execute(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java index c7ad77cca1..5d5a6ae9b1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java @@ -20,6 +20,7 @@ import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; @@ -30,6 +31,7 @@ import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.project.CommitResource; import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryingRestModifyView; @@ -68,7 +70,7 @@ public class CherryPickCommit public ChangeInfo applyImpl( BatchUpdate.Factory updateFactory, CommitResource rsrc, CherryPickInput input) throws OrmException, IOException, UpdateException, RestApiException, - PermissionBackendException, ConfigInvalidException { + PermissionBackendException, ConfigInvalidException, NoSuchProjectException { RevCommit commit = rsrc.getCommit(); String message = Strings.nullToEmpty(input.message).trim(); input.message = message.isEmpty() ? commit.getFullMessage() : message; @@ -97,7 +99,7 @@ public class CherryPickCommit projectName, commit, input, - rsrc.getProject().controlForRef(refName)); + new Branch.NameKey(rsrc.getProject().getProject().getNameKey(), refName)); return json.noOptions().format(projectName, cherryPickedChangeId); } catch (InvalidChangeOperationException e) { throw new BadRequestException(e.getMessage()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index 8f2c3a8bcd..5e26305720 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -27,6 +27,7 @@ import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; @@ -338,7 +339,13 @@ public class PatchSetInserter implements BatchUpdateOp { commitId, ctx.getIdentifiedUser())) { commitValidatorsFactory - .forGerritCommits(perm, origCtl.getRefControl(), new NoSshInfo(), ctx.getRevWalk()) + .forGerritCommits( + perm, + new Branch.NameKey( + origCtl.getProject().getNameKey(), origCtl.getRefControl().getRefName()), + ctx.getIdentifiedUser(), + new NoSshInfo(), + ctx.getRevWalk()) .validate(event); } catch (CommitValidationException e) { throw new ResourceConflictException(e.getFullMessage()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index fcd6e0dbeb..d4e085c8e8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -134,7 +134,6 @@ 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.gerrit.server.project.RefControl; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.ssh.SshInfo; @@ -1012,8 +1011,7 @@ class ReceiveCommits { return; } - RefControl ctl = projectControl.controlForRef(cmd.getRefName()); - validateNewCommits(ctl, cmd); + validateNewCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd); actualCommands.add(cmd); } @@ -1033,7 +1031,7 @@ class ReceiveCommits { if (!validRefOperation(cmd)) { return; } - validateNewCommits(projectControl.controlForRef(cmd.getRefName()), cmd); + validateNewCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd); actualCommands.add(cmd); } else { if (RefNames.REFS_CONFIG.equals(cmd.getRefName())) { @@ -1104,9 +1102,8 @@ class ReceiveCommits { } logDebug("Rewinding {}", cmd); - RefControl ctl = projectControl.controlForRef(cmd.getRefName()); if (newObject != null) { - validateNewCommits(ctl, cmd); + validateNewCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd); if (cmd.getResult() != NOT_ATTEMPTED) { return; } @@ -1138,7 +1135,6 @@ class ReceiveCommits { final NotesMigration notesMigration; private final boolean defaultPublishComments; Branch.NameKey dest; - RefControl ctl; PermissionBackend.ForRef perm; Set reviewer = Sets.newLinkedHashSet(); Set cc = Sets.newLinkedHashSet(); @@ -1451,7 +1447,6 @@ class ReceiveCommits { } magicBranch.dest = new Branch.NameKey(project.getNameKey(), ref); - magicBranch.ctl = projectControl.controlForRef(ref); magicBranch.perm = permissions.ref(ref); if (!projectControl.getProject().getState().permitsWrite()) { reject(cmd, "project state does not permit write"); @@ -1590,7 +1585,7 @@ class ReceiveCommits { // commits and the target branch head. // try { - Ref targetRef = rp.getAdvertisedRefs().get(magicBranch.ctl.getRefName()); + Ref targetRef = rp.getAdvertisedRefs().get(magicBranch.dest.get()); if (targetRef == null || targetRef.getObjectId() == null) { // The destination branch does not yet exist. Assume the // history being sent for review will start it and thus @@ -1798,7 +1793,7 @@ class ReceiveCommits { logDebug("Creating new change for {} even though it is already tracked", name); } - if (!validCommit(rp.getRevWalk(), magicBranch.perm, magicBranch.ctl, magicBranch.cmd, c)) { + if (!validCommit(rp.getRevWalk(), magicBranch.perm, magicBranch.dest, magicBranch.cmd, c)) { // Not a change the user can propose? Abort as early as possible. newChanges = Collections.emptyList(); logDebug("Aborting early due to invalid commit"); @@ -1992,7 +1987,7 @@ class ReceiveCommits { rw.markUninteresting(c); } } else { - markHeadsAsUninteresting(rw, magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null); + markHeadsAsUninteresting(rw, magicBranch.dest != null ? magicBranch.dest.get() : null); } return start; } @@ -2002,11 +1997,11 @@ class ReceiveCommits { for (RevCommit c : magicBranch.baseCommit) { rp.getRevWalk().markUninteresting(c); } - Ref targetRef = allRefs().get(magicBranch.ctl.getRefName()); + Ref targetRef = allRefs().get(magicBranch.dest.get()); if (targetRef != null) { logDebug( "Marking target ref {} ({}) uninteresting", - magicBranch.ctl.getRefName(), + magicBranch.dest.get(), targetRef.getObjectId().name()); rp.getRevWalk().markUninteresting(rp.getRevWalk().parseCommit(targetRef.getObjectId())); } @@ -2014,7 +2009,7 @@ class ReceiveCommits { private void rejectImplicitMerges(Set mergedParents) throws IOException { if (!mergedParents.isEmpty()) { - Ref targetRef = allRefs().get(magicBranch.ctl.getRefName()); + Ref targetRef = allRefs().get(magicBranch.dest.get()); if (targetRef != null) { RevWalk rw = rp.getRevWalk(); RevCommit tip = rw.parseCommit(targetRef.getObjectId()); @@ -2380,8 +2375,7 @@ class ReceiveCommits { } PermissionBackend.ForRef perm = permissions.ref(change.getDest().get()); - RefControl refctl = projectControl.controlForRef(change.getDest()); - if (!validCommit(rp.getRevWalk(), perm, refctl, inputCommand, newCommit)) { + if (!validCommit(rp.getRevWalk(), perm, change.getDest(), inputCommand, newCommit)) { return false; } rp.getRevWalk().parseBody(priorCommit); @@ -2685,9 +2679,9 @@ class ReceiveCommits { return true; } - private void validateNewCommits(RefControl ctl, ReceiveCommand cmd) + private void validateNewCommits(Branch.NameKey branch, ReceiveCommand cmd) throws PermissionBackendException { - PermissionBackend.ForRef perm = permissions.ref(ctl.getRefName()); + PermissionBackend.ForRef perm = permissions.ref(branch.get()); if (!RefNames.REFS_CONFIG.equals(cmd.getRefName()) && !(MagicBranch.isMagicBranch(cmd.getRefName()) || NEW_PATCHSET_PATTERN.matcher(cmd.getRefName()).matches()) @@ -2721,7 +2715,7 @@ class ReceiveCommits { i++; if (existing.keySet().contains(c)) { continue; - } else if (!validCommit(walk, perm, ctl, cmd, c)) { + } else if (!validCommit(walk, perm, branch, cmd, c)) { break; } @@ -2757,7 +2751,11 @@ class ReceiveCommits { } private boolean validCommit( - RevWalk rw, PermissionBackend.ForRef perm, RefControl ctl, ReceiveCommand cmd, ObjectId id) + RevWalk rw, + PermissionBackend.ForRef perm, + Branch.NameKey branch, + ReceiveCommand cmd, + ObjectId id) throws IOException { if (validCommits.contains(id)) { @@ -2768,15 +2766,16 @@ class ReceiveCommits { rw.parseBody(c); try (CommitReceivedEvent receiveEvent = - new CommitReceivedEvent(cmd, project, ctl.getRefName(), rw.getObjectReader(), c, user)) { + new CommitReceivedEvent(cmd, project, branch.get(), rw.getObjectReader(), c, user)) { boolean isMerged = magicBranch != null && cmd.getRefName().equals(magicBranch.cmd.getRefName()) && magicBranch.merged; CommitValidators validators = isMerged - ? commitValidatorsFactory.forMergedCommits(perm, ctl) - : commitValidatorsFactory.forReceiveCommits(perm, ctl, sshInfo, repo, rw); + ? commitValidatorsFactory.forMergedCommits(perm, user.asIdentifiedUser()) + : commitValidatorsFactory.forReceiveCommits( + perm, branch, user.asIdentifiedUser(), sshInfo, repo, rw); messages.addAll(validators.validate(receiveEvent)); } catch (CommitValidationException e) { logDebug("Commit validation failed on {}", c.name()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java index 17303f8dbf..41381e8024 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java @@ -28,6 +28,7 @@ import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyP import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; @@ -44,9 +45,8 @@ import com.google.gerrit.server.git.ValidationError; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.RefPermission; -import com.google.gerrit.server.project.ProjectControl; +import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; -import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.util.MagicBranch; import com.google.inject.Inject; @@ -89,6 +89,7 @@ public class CommitValidators { private final AllUsersName allUsers; private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker; private final String installCommitMsgHookCommand; + private final ProjectCache projectCache; @Inject Factory( @@ -97,7 +98,8 @@ public class CommitValidators { @GerritServerConfig Config cfg, DynamicSet pluginValidators, AllUsersName allUsers, - ExternalIdsConsistencyChecker externalIdsConsistencyChecker) { + ExternalIdsConsistencyChecker externalIdsConsistencyChecker, + ProjectCache projectCache) { this.gerritIdent = gerritIdent; this.canonicalWebUrl = canonicalWebUrl; this.pluginValidators = pluginValidators; @@ -105,26 +107,29 @@ public class CommitValidators { this.externalIdsConsistencyChecker = externalIdsConsistencyChecker; this.installCommitMsgHookCommand = cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null; + this.projectCache = projectCache; } public CommitValidators forReceiveCommits( PermissionBackend.ForRef perm, - RefControl refctl, + Branch.NameKey branch, + IdentifiedUser user, SshInfo sshInfo, Repository repo, RevWalk rw) throws IOException { NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw); - IdentifiedUser user = refctl.getUser().asIdentifiedUser(); + ProjectState projectState = projectCache.checkedGet(branch.getParentKey()); return new CommitValidators( ImmutableList.of( new UploadMergesPermissionValidator(perm), new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), new AuthorUploaderValidator(user, perm, canonicalWebUrl), new CommitterUploaderValidator(user, perm, canonicalWebUrl), - new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), - new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), - new ConfigValidator(refctl, rw, allUsers), + new SignedOffByValidator(user, perm, projectState), + new ChangeIdValidator( + projectState, user, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), + new ConfigValidator(branch, user, rw, allUsers), new BannedCommitsValidator(rejectCommits), new PluginCommitValidationListener(pluginValidators), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), @@ -132,23 +137,31 @@ public class CommitValidators { } public CommitValidators forGerritCommits( - PermissionBackend.ForRef perm, RefControl refctl, SshInfo sshInfo, RevWalk rw) { - IdentifiedUser user = refctl.getUser().asIdentifiedUser(); + PermissionBackend.ForRef perm, + Branch.NameKey branch, + IdentifiedUser user, + SshInfo sshInfo, + RevWalk rw) + throws IOException { return new CommitValidators( ImmutableList.of( new UploadMergesPermissionValidator(perm), new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), new AuthorUploaderValidator(user, perm, canonicalWebUrl), - new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), - new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), - new ConfigValidator(refctl, rw, allUsers), + new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.getParentKey())), + new ChangeIdValidator( + projectCache.checkedGet(branch.getParentKey()), + user, + canonicalWebUrl, + installCommitMsgHookCommand, + sshInfo), + new ConfigValidator(branch, user, rw, allUsers), new PluginCommitValidationListener(pluginValidators), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), new AccountValidator(allUsers))); } - public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, RefControl refControl) { - IdentifiedUser user = refControl.getUser().asIdentifiedUser(); + public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, IdentifiedUser user) { // Generally only include validators that are based on permissions of the // user creating a change for a merged commit; generally exclude // validators that would require amending the change in order to correct. @@ -208,22 +221,23 @@ public class CommitValidators { + " line format in commit message footer"; private static final Pattern CHANGE_ID = Pattern.compile(CHANGE_ID_PATTERN); - private final ProjectControl projectControl; + private final ProjectState projectState; private final String canonicalWebUrl; private final String installCommitMsgHookCommand; private final SshInfo sshInfo; private final IdentifiedUser user; public ChangeIdValidator( - RefControl refControl, + ProjectState projectState, + IdentifiedUser user, String canonicalWebUrl, String installCommitMsgHookCommand, SshInfo sshInfo) { - this.projectControl = refControl.getProjectControl(); + this.projectState = projectState; this.canonicalWebUrl = canonicalWebUrl; this.installCommitMsgHookCommand = installCommitMsgHookCommand; this.sshInfo = sshInfo; - this.user = projectControl.getUser().asIdentifiedUser(); + this.user = user; } @Override @@ -238,7 +252,7 @@ public class CommitValidators { String sha1 = commit.abbreviate(SHA1_LENGTH).name(); if (idList.isEmpty()) { - if (projectControl.getProjectState().isRequireChangeID()) { + if (projectState.isRequireChangeID()) { String shortMsg = commit.getShortMessage(); if (shortMsg.startsWith(CHANGE_ID_PREFIX) && CHANGE_ID @@ -342,12 +356,15 @@ public class CommitValidators { /** If this is the special project configuration branch, validate the config. */ public static class ConfigValidator implements CommitValidationListener { - private final RefControl refControl; + private final Branch.NameKey branch; + private final IdentifiedUser user; private final RevWalk rw; private final AllUsersName allUsers; - public ConfigValidator(RefControl refControl, RevWalk rw, AllUsersName allUsers) { - this.refControl = refControl; + public ConfigValidator( + Branch.NameKey branch, IdentifiedUser user, RevWalk rw, AllUsersName allUsers) { + this.branch = branch; + this.user = user; this.rw = rw; this.allUsers = allUsers; } @@ -355,9 +372,7 @@ public class CommitValidators { @Override public List onCommitReceived(CommitReceivedEvent receiveEvent) throws CommitValidationException { - IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser(); - - if (REFS_CONFIG.equals(refControl.getRefName())) { + if (REFS_CONFIG.equals(branch.get())) { List messages = new ArrayList<>(); try { @@ -373,7 +388,7 @@ public class CommitValidators { } catch (ConfigInvalidException | IOException e) { log.error( "User " - + currentUser.getUserName() + + user.getUserName() + " tried to push an invalid project configuration " + receiveEvent.command.getNewId().name() + " for project " @@ -383,10 +398,9 @@ public class CommitValidators { } } - if (allUsers.equals(refControl.getProjectControl().getProject().getNameKey()) - && RefNames.isRefsUsers(refControl.getRefName())) { + if (allUsers.equals(branch.getParentKey()) && RefNames.isRefsUsers(branch.get())) { List messages = new ArrayList<>(); - Account.Id accountId = Account.Id.fromRef(refControl.getRefName()); + Account.Id accountId = Account.Id.fromRef(branch.get()); if (accountId != null) { try { WatchConfig wc = new WatchConfig(accountId); @@ -401,7 +415,7 @@ public class CommitValidators { } catch (IOException | ConfigInvalidException e) { log.error( "User " - + currentUser.getUserName() + + user.getUserName() + " tried to push an invalid watch configuration " + receiveEvent.command.getNewId().name() + " for account "