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
This commit is contained in:
Patrick Hiesel
2017-08-25 14:53:35 +02:00
parent ef213fd3e0
commit 3ee57453d4
7 changed files with 112 additions and 89 deletions

View File

@@ -57,9 +57,7 @@ import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
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.RefControl;
import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context; 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 static final Logger log = LoggerFactory.getLogger(ChangeInserter.class);
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final ProjectControl.GenericFactory projectControlFactory; private final ProjectCache projectCache;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final ChangeControl.GenericFactory changeControlFactory; private final ChangeControl.GenericFactory changeControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
@@ -144,7 +142,7 @@ public class ChangeInserter implements InsertChangeOp {
@Inject @Inject
ChangeInserter( ChangeInserter(
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
ProjectControl.GenericFactory projectControlFactory, ProjectCache projectCache,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
ChangeControl.GenericFactory changeControlFactory, ChangeControl.GenericFactory changeControlFactory,
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory,
@@ -161,7 +159,7 @@ public class ChangeInserter implements InsertChangeOp {
@Assisted ObjectId commitId, @Assisted ObjectId commitId,
@Assisted String refName) { @Assisted String refName) {
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.projectControlFactory = projectControlFactory; this.projectCache = projectCache;
this.userFactory = userFactory; this.userFactory = userFactory;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
@@ -567,24 +565,25 @@ public class ChangeInserter implements InsertChangeOp {
PermissionBackend.ForRef perm = PermissionBackend.ForRef perm =
permissionBackend.user(ctx.getUser()).project(ctx.getProject()).ref(refName); permissionBackend.user(ctx.getUser()).project(ctx.getProject()).ref(refName);
try { try {
RefControl refControl =
projectControlFactory.controlFor(ctx.getProject(), ctx.getUser()).controlForRef(refName);
try (CommitReceivedEvent event = try (CommitReceivedEvent event =
new CommitReceivedEvent( new CommitReceivedEvent(
cmd, cmd,
refControl.getProjectControl().getProject(), projectCache.checkedGet(ctx.getProject()).getProject(),
change.getDest().get(), change.getDest().get(),
ctx.getRevWalk().getObjectReader(), ctx.getRevWalk().getObjectReader(),
commitId, commitId,
ctx.getIdentifiedUser())) { ctx.getIdentifiedUser())) {
commitValidatorsFactory commitValidatorsFactory
.forGerritCommits(perm, refControl, new NoSshInfo(), ctx.getRevWalk()) .forGerritCommits(
perm,
new Branch.NameKey(ctx.getProject(), refName),
ctx.getIdentifiedUser(),
new NoSshInfo(),
ctx.getRevWalk())
.validate(event); .validate(event);
} }
} catch (CommitValidationException e) { } catch (CommitValidationException e) {
throw new ResourceConflictException(e.getFullMessage()); throw new ResourceConflictException(e.getFullMessage());
} catch (NoSuchProjectException e) {
throw new ResourceConflictException(e.getMessage());
} }
} }
} }

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction; 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.Change;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.CurrentUser; 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.permissions.RefPermission;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; 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.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView; import com.google.gerrit.server.update.RetryingRestModifyView;
@@ -70,7 +72,7 @@ public class CherryPick
public ChangeInfo applyImpl( public ChangeInfo applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, CherryPickInput input) BatchUpdate.Factory updateFactory, RevisionResource rsrc, CherryPickInput input)
throws OrmException, IOException, UpdateException, RestApiException, throws OrmException, IOException, UpdateException, RestApiException,
PermissionBackendException, ConfigInvalidException { PermissionBackendException, ConfigInvalidException, NoSuchProjectException {
input.parent = input.parent == null ? 1 : input.parent; input.parent = input.parent == null ? 1 : input.parent;
if (input.message == null || input.message.trim().isEmpty()) { if (input.message == null || input.message.trim().isEmpty()) {
throw new BadRequestException("message must be non-empty"); throw new BadRequestException("message must be non-empty");
@@ -93,7 +95,7 @@ public class CherryPick
rsrc.getChange(), rsrc.getChange(),
rsrc.getPatchSet(), rsrc.getPatchSet(),
input, input,
rsrc.getControl().getProjectControl().controlForRef(refName)); new Branch.NameKey(rsrc.getProject(), refName));
return json.noOptions().format(rsrc.getProject(), cherryPickedChangeId); return json.noOptions().format(rsrc.getProject(), cherryPickedChangeId);
} catch (InvalidChangeOperationException e) { } catch (InvalidChangeOperationException e) {
throw new BadRequestException(e.getMessage()); throw new BadRequestException(e.getMessage());

View File

@@ -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.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project; 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.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil; 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.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; 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.ProjectState;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
@@ -94,6 +94,7 @@ public class CherryPickChange {
private final PatchSetInserter.Factory patchSetInserterFactory; private final PatchSetInserter.Factory patchSetInserterFactory;
private final MergeUtil.Factory mergeUtilFactory; private final MergeUtil.Factory mergeUtilFactory;
private final ChangeNotes.Factory changeNotesFactory; private final ChangeNotes.Factory changeNotesFactory;
private final ProjectControl.GenericFactory projectControlFactory;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil changeMessagesUtil; private final ChangeMessagesUtil changeMessagesUtil;
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
@@ -111,6 +112,7 @@ public class CherryPickChange {
PatchSetInserter.Factory patchSetInserterFactory, PatchSetInserter.Factory patchSetInserterFactory,
MergeUtil.Factory mergeUtilFactory, MergeUtil.Factory mergeUtilFactory,
ChangeNotes.Factory changeNotesFactory, ChangeNotes.Factory changeNotesFactory,
ProjectControl.GenericFactory projectControlFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
ChangeMessagesUtil changeMessagesUtil, ChangeMessagesUtil changeMessagesUtil,
PatchSetUtil psUtil, PatchSetUtil psUtil,
@@ -125,6 +127,7 @@ public class CherryPickChange {
this.patchSetInserterFactory = patchSetInserterFactory; this.patchSetInserterFactory = patchSetInserterFactory;
this.mergeUtilFactory = mergeUtilFactory; this.mergeUtilFactory = mergeUtilFactory;
this.changeNotesFactory = changeNotesFactory; this.changeNotesFactory = changeNotesFactory;
this.projectControlFactory = projectControlFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.changeMessagesUtil = changeMessagesUtil; this.changeMessagesUtil = changeMessagesUtil;
this.psUtil = psUtil; this.psUtil = psUtil;
@@ -136,9 +139,9 @@ public class CherryPickChange {
Change change, Change change,
PatchSet patch, PatchSet patch,
CherryPickInput input, CherryPickInput input,
RefControl refControl) Branch.NameKey dest)
throws OrmException, IOException, InvalidChangeOperationException, IntegrationException, throws OrmException, IOException, InvalidChangeOperationException, IntegrationException,
UpdateException, RestApiException, ConfigInvalidException { UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException {
return cherryPick( return cherryPick(
batchUpdateFactory, batchUpdateFactory,
change, change,
@@ -146,7 +149,7 @@ public class CherryPickChange {
change.getProject(), change.getProject(),
ObjectId.fromString(patch.getRevision().get()), ObjectId.fromString(patch.getRevision().get()),
input, input,
refControl); dest);
} }
public Change.Id cherryPick( public Change.Id cherryPick(
@@ -156,9 +159,9 @@ public class CherryPickChange {
Project.NameKey project, Project.NameKey project,
ObjectId sourceCommit, ObjectId sourceCommit,
CherryPickInput input, CherryPickInput input,
RefControl destRefControl) Branch.NameKey dest)
throws OrmException, IOException, InvalidChangeOperationException, IntegrationException, throws OrmException, IOException, InvalidChangeOperationException, IntegrationException,
UpdateException, RestApiException, ConfigInvalidException { UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException {
IdentifiedUser identifiedUser = user.get(); IdentifiedUser identifiedUser = user.get();
try (Repository git = gitManager.openRepository(project); try (Repository git = gitManager.openRepository(project);
@@ -168,11 +171,10 @@ public class CherryPickChange {
ObjectInserter oi = git.newObjectInserter(); ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader(); ObjectReader reader = oi.newReader();
CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) { CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) {
String destRefName = destRefControl.getRefName(); Ref destRef = git.getRefDatabase().exactRef(dest.get());
Ref destRef = git.getRefDatabase().exactRef(destRefName);
if (destRef == null) { if (destRef == null) {
throw new InvalidChangeOperationException( 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); 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'; String commitMessage = ChangeIdUtil.insertId(input.message, computedChangeId).trim() + '\n';
CodeReviewCommit cherryPickCommit; CodeReviewCommit cherryPickCommit;
ProjectControl projectControl =
projectControlFactory.controlFor(dest.getParentKey(), identifiedUser);
try { try {
ProjectState projectState = destRefControl.getProjectControl().getProjectState(); ProjectState projectState = projectControl.getProjectState();
cherryPickCommit = cherryPickCommit =
mergeUtilFactory mergeUtilFactory
.create(projectState) .create(projectState)
@@ -242,8 +246,7 @@ public class CherryPickChange {
if (destChanges.size() == 1) { if (destChanges.size() == 1) {
// The change key exists on the destination branch. The cherry pick // The change key exists on the destination branch. The cherry pick
// will be added as a new patch set. // will be added as a new patch set.
ChangeControl destCtl = ChangeControl destCtl = projectControl.controlFor(destChanges.get(0).notes());
destRefControl.getProjectControl().controlFor(destChanges.get(0).notes());
result = insertPatchSet(bu, git, destCtl, cherryPickCommit, input); result = insertPatchSet(bu, git, destCtl, cherryPickCommit, input);
} else { } else {
// Change key not found on destination branch. We can create a new // Change key not found on destination branch. We can create a new
@@ -254,16 +257,13 @@ public class CherryPickChange {
} }
result = result =
createNewChange( createNewChange(
bu, cherryPickCommit, destRefName, newTopic, sourceChange, sourceCommit, input); bu, cherryPickCommit, dest.get(), newTopic, sourceChange, sourceCommit, input);
if (sourceChange != null && sourcePatchId != null) { if (sourceChange != null && sourcePatchId != null) {
bu.addOp( bu.addOp(
sourceChange.getId(), sourceChange.getId(),
new AddMessageToSourceChangeOp( new AddMessageToSourceChangeOp(
changeMessagesUtil, changeMessagesUtil, sourcePatchId, dest.getShortName(), cherryPickCommit));
sourcePatchId,
RefNames.shortName(destRefName),
cherryPickCommit));
} }
} }
bu.execute(); bu.execute();

View File

@@ -20,6 +20,7 @@ import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException; 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.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; 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.permissions.RefPermission;
import com.google.gerrit.server.project.CommitResource; import com.google.gerrit.server.project.CommitResource;
import com.google.gerrit.server.project.InvalidChangeOperationException; 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.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView; import com.google.gerrit.server.update.RetryingRestModifyView;
@@ -68,7 +70,7 @@ public class CherryPickCommit
public ChangeInfo applyImpl( public ChangeInfo applyImpl(
BatchUpdate.Factory updateFactory, CommitResource rsrc, CherryPickInput input) BatchUpdate.Factory updateFactory, CommitResource rsrc, CherryPickInput input)
throws OrmException, IOException, UpdateException, RestApiException, throws OrmException, IOException, UpdateException, RestApiException,
PermissionBackendException, ConfigInvalidException { PermissionBackendException, ConfigInvalidException, NoSuchProjectException {
RevCommit commit = rsrc.getCommit(); RevCommit commit = rsrc.getCommit();
String message = Strings.nullToEmpty(input.message).trim(); String message = Strings.nullToEmpty(input.message).trim();
input.message = message.isEmpty() ? commit.getFullMessage() : message; input.message = message.isEmpty() ? commit.getFullMessage() : message;
@@ -97,7 +99,7 @@ public class CherryPickCommit
projectName, projectName,
commit, commit,
input, input,
rsrc.getProject().controlForRef(refName)); new Branch.NameKey(rsrc.getProject().getProject().getNameKey(), refName));
return json.noOptions().format(projectName, cherryPickedChangeId); return json.noOptions().format(projectName, cherryPickedChangeId);
} catch (InvalidChangeOperationException e) { } catch (InvalidChangeOperationException e) {
throw new BadRequestException(e.getMessage()); throw new BadRequestException(e.getMessage());

View File

@@ -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.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account; 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.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@@ -338,7 +339,13 @@ public class PatchSetInserter implements BatchUpdateOp {
commitId, commitId,
ctx.getIdentifiedUser())) { ctx.getIdentifiedUser())) {
commitValidatorsFactory 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); .validate(event);
} catch (CommitValidationException e) { } catch (CommitValidationException e) {
throw new ResourceConflictException(e.getFullMessage()); throw new ResourceConflictException(e.getFullMessage());

View File

@@ -134,7 +134,6 @@ import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState; 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.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.ssh.SshInfo;
@@ -1012,8 +1011,7 @@ class ReceiveCommits {
return; return;
} }
RefControl ctl = projectControl.controlForRef(cmd.getRefName()); validateNewCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd);
validateNewCommits(ctl, cmd);
actualCommands.add(cmd); actualCommands.add(cmd);
} }
@@ -1033,7 +1031,7 @@ class ReceiveCommits {
if (!validRefOperation(cmd)) { if (!validRefOperation(cmd)) {
return; return;
} }
validateNewCommits(projectControl.controlForRef(cmd.getRefName()), cmd); validateNewCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd);
actualCommands.add(cmd); actualCommands.add(cmd);
} else { } else {
if (RefNames.REFS_CONFIG.equals(cmd.getRefName())) { if (RefNames.REFS_CONFIG.equals(cmd.getRefName())) {
@@ -1104,9 +1102,8 @@ class ReceiveCommits {
} }
logDebug("Rewinding {}", cmd); logDebug("Rewinding {}", cmd);
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (newObject != null) { if (newObject != null) {
validateNewCommits(ctl, cmd); validateNewCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd);
if (cmd.getResult() != NOT_ATTEMPTED) { if (cmd.getResult() != NOT_ATTEMPTED) {
return; return;
} }
@@ -1138,7 +1135,6 @@ class ReceiveCommits {
final NotesMigration notesMigration; final NotesMigration notesMigration;
private final boolean defaultPublishComments; private final boolean defaultPublishComments;
Branch.NameKey dest; Branch.NameKey dest;
RefControl ctl;
PermissionBackend.ForRef perm; PermissionBackend.ForRef perm;
Set<Account.Id> reviewer = Sets.newLinkedHashSet(); Set<Account.Id> reviewer = Sets.newLinkedHashSet();
Set<Account.Id> cc = Sets.newLinkedHashSet(); Set<Account.Id> cc = Sets.newLinkedHashSet();
@@ -1451,7 +1447,6 @@ class ReceiveCommits {
} }
magicBranch.dest = new Branch.NameKey(project.getNameKey(), ref); magicBranch.dest = new Branch.NameKey(project.getNameKey(), ref);
magicBranch.ctl = projectControl.controlForRef(ref);
magicBranch.perm = permissions.ref(ref); magicBranch.perm = permissions.ref(ref);
if (!projectControl.getProject().getState().permitsWrite()) { if (!projectControl.getProject().getState().permitsWrite()) {
reject(cmd, "project state does not permit write"); reject(cmd, "project state does not permit write");
@@ -1590,7 +1585,7 @@ class ReceiveCommits {
// commits and the target branch head. // commits and the target branch head.
// //
try { try {
Ref targetRef = rp.getAdvertisedRefs().get(magicBranch.ctl.getRefName()); Ref targetRef = rp.getAdvertisedRefs().get(magicBranch.dest.get());
if (targetRef == null || targetRef.getObjectId() == null) { if (targetRef == null || targetRef.getObjectId() == null) {
// The destination branch does not yet exist. Assume the // The destination branch does not yet exist. Assume the
// history being sent for review will start it and thus // 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); 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. // Not a change the user can propose? Abort as early as possible.
newChanges = Collections.emptyList(); newChanges = Collections.emptyList();
logDebug("Aborting early due to invalid commit"); logDebug("Aborting early due to invalid commit");
@@ -1992,7 +1987,7 @@ class ReceiveCommits {
rw.markUninteresting(c); rw.markUninteresting(c);
} }
} else { } else {
markHeadsAsUninteresting(rw, magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null); markHeadsAsUninteresting(rw, magicBranch.dest != null ? magicBranch.dest.get() : null);
} }
return start; return start;
} }
@@ -2002,11 +1997,11 @@ class ReceiveCommits {
for (RevCommit c : magicBranch.baseCommit) { for (RevCommit c : magicBranch.baseCommit) {
rp.getRevWalk().markUninteresting(c); rp.getRevWalk().markUninteresting(c);
} }
Ref targetRef = allRefs().get(magicBranch.ctl.getRefName()); Ref targetRef = allRefs().get(magicBranch.dest.get());
if (targetRef != null) { if (targetRef != null) {
logDebug( logDebug(
"Marking target ref {} ({}) uninteresting", "Marking target ref {} ({}) uninteresting",
magicBranch.ctl.getRefName(), magicBranch.dest.get(),
targetRef.getObjectId().name()); targetRef.getObjectId().name());
rp.getRevWalk().markUninteresting(rp.getRevWalk().parseCommit(targetRef.getObjectId())); rp.getRevWalk().markUninteresting(rp.getRevWalk().parseCommit(targetRef.getObjectId()));
} }
@@ -2014,7 +2009,7 @@ class ReceiveCommits {
private void rejectImplicitMerges(Set<RevCommit> mergedParents) throws IOException { private void rejectImplicitMerges(Set<RevCommit> mergedParents) throws IOException {
if (!mergedParents.isEmpty()) { if (!mergedParents.isEmpty()) {
Ref targetRef = allRefs().get(magicBranch.ctl.getRefName()); Ref targetRef = allRefs().get(magicBranch.dest.get());
if (targetRef != null) { if (targetRef != null) {
RevWalk rw = rp.getRevWalk(); RevWalk rw = rp.getRevWalk();
RevCommit tip = rw.parseCommit(targetRef.getObjectId()); RevCommit tip = rw.parseCommit(targetRef.getObjectId());
@@ -2380,8 +2375,7 @@ class ReceiveCommits {
} }
PermissionBackend.ForRef perm = permissions.ref(change.getDest().get()); PermissionBackend.ForRef perm = permissions.ref(change.getDest().get());
RefControl refctl = projectControl.controlForRef(change.getDest()); if (!validCommit(rp.getRevWalk(), perm, change.getDest(), inputCommand, newCommit)) {
if (!validCommit(rp.getRevWalk(), perm, refctl, inputCommand, newCommit)) {
return false; return false;
} }
rp.getRevWalk().parseBody(priorCommit); rp.getRevWalk().parseBody(priorCommit);
@@ -2685,9 +2679,9 @@ class ReceiveCommits {
return true; return true;
} }
private void validateNewCommits(RefControl ctl, ReceiveCommand cmd) private void validateNewCommits(Branch.NameKey branch, ReceiveCommand cmd)
throws PermissionBackendException { throws PermissionBackendException {
PermissionBackend.ForRef perm = permissions.ref(ctl.getRefName()); PermissionBackend.ForRef perm = permissions.ref(branch.get());
if (!RefNames.REFS_CONFIG.equals(cmd.getRefName()) if (!RefNames.REFS_CONFIG.equals(cmd.getRefName())
&& !(MagicBranch.isMagicBranch(cmd.getRefName()) && !(MagicBranch.isMagicBranch(cmd.getRefName())
|| NEW_PATCHSET_PATTERN.matcher(cmd.getRefName()).matches()) || NEW_PATCHSET_PATTERN.matcher(cmd.getRefName()).matches())
@@ -2721,7 +2715,7 @@ class ReceiveCommits {
i++; i++;
if (existing.keySet().contains(c)) { if (existing.keySet().contains(c)) {
continue; continue;
} else if (!validCommit(walk, perm, ctl, cmd, c)) { } else if (!validCommit(walk, perm, branch, cmd, c)) {
break; break;
} }
@@ -2757,7 +2751,11 @@ class ReceiveCommits {
} }
private boolean validCommit( 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 { throws IOException {
if (validCommits.contains(id)) { if (validCommits.contains(id)) {
@@ -2768,15 +2766,16 @@ class ReceiveCommits {
rw.parseBody(c); rw.parseBody(c);
try (CommitReceivedEvent receiveEvent = 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 = boolean isMerged =
magicBranch != null magicBranch != null
&& cmd.getRefName().equals(magicBranch.cmd.getRefName()) && cmd.getRefName().equals(magicBranch.cmd.getRefName())
&& magicBranch.merged; && magicBranch.merged;
CommitValidators validators = CommitValidators validators =
isMerged isMerged
? commitValidatorsFactory.forMergedCommits(perm, ctl) ? commitValidatorsFactory.forMergedCommits(perm, user.asIdentifiedUser())
: commitValidatorsFactory.forReceiveCommits(perm, ctl, sshInfo, repo, rw); : commitValidatorsFactory.forReceiveCommits(
perm, branch, user.asIdentifiedUser(), sshInfo, repo, rw);
messages.addAll(validators.validate(receiveEvent)); messages.addAll(validators.validate(receiveEvent));
} catch (CommitValidationException e) { } catch (CommitValidationException e) {
logDebug("Commit validation failed on {}", c.name()); logDebug("Commit validation failed on {}", c.name());

View File

@@ -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.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account; 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.reviewdb.client.RefNames;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; 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.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission; 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.ProjectState;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.server.util.MagicBranch; import com.google.gerrit.server.util.MagicBranch;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -89,6 +89,7 @@ public class CommitValidators {
private final AllUsersName allUsers; private final AllUsersName allUsers;
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker; private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
private final String installCommitMsgHookCommand; private final String installCommitMsgHookCommand;
private final ProjectCache projectCache;
@Inject @Inject
Factory( Factory(
@@ -97,7 +98,8 @@ public class CommitValidators {
@GerritServerConfig Config cfg, @GerritServerConfig Config cfg,
DynamicSet<CommitValidationListener> pluginValidators, DynamicSet<CommitValidationListener> pluginValidators,
AllUsersName allUsers, AllUsersName allUsers,
ExternalIdsConsistencyChecker externalIdsConsistencyChecker) { ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
ProjectCache projectCache) {
this.gerritIdent = gerritIdent; this.gerritIdent = gerritIdent;
this.canonicalWebUrl = canonicalWebUrl; this.canonicalWebUrl = canonicalWebUrl;
this.pluginValidators = pluginValidators; this.pluginValidators = pluginValidators;
@@ -105,26 +107,29 @@ public class CommitValidators {
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker; this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
this.installCommitMsgHookCommand = this.installCommitMsgHookCommand =
cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null; cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null;
this.projectCache = projectCache;
} }
public CommitValidators forReceiveCommits( public CommitValidators forReceiveCommits(
PermissionBackend.ForRef perm, PermissionBackend.ForRef perm,
RefControl refctl, Branch.NameKey branch,
IdentifiedUser user,
SshInfo sshInfo, SshInfo sshInfo,
Repository repo, Repository repo,
RevWalk rw) RevWalk rw)
throws IOException { throws IOException {
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw); NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw);
IdentifiedUser user = refctl.getUser().asIdentifiedUser(); ProjectState projectState = projectCache.checkedGet(branch.getParentKey());
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
new UploadMergesPermissionValidator(perm), new UploadMergesPermissionValidator(perm),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl), new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl), new CommitterUploaderValidator(user, perm, canonicalWebUrl),
new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), new ChangeIdValidator(
new ConfigValidator(refctl, rw, allUsers), projectState, user, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
new ConfigValidator(branch, user, rw, allUsers),
new BannedCommitsValidator(rejectCommits), new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators), new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -132,23 +137,31 @@ public class CommitValidators {
} }
public CommitValidators forGerritCommits( public CommitValidators forGerritCommits(
PermissionBackend.ForRef perm, RefControl refctl, SshInfo sshInfo, RevWalk rw) { PermissionBackend.ForRef perm,
IdentifiedUser user = refctl.getUser().asIdentifiedUser(); Branch.NameKey branch,
IdentifiedUser user,
SshInfo sshInfo,
RevWalk rw)
throws IOException {
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
new UploadMergesPermissionValidator(perm), new UploadMergesPermissionValidator(perm),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl), new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()), new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.getParentKey())),
new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo), new ChangeIdValidator(
new ConfigValidator(refctl, rw, allUsers), projectCache.checkedGet(branch.getParentKey()),
user,
canonicalWebUrl,
installCommitMsgHookCommand,
sshInfo),
new ConfigValidator(branch, user, rw, allUsers),
new PluginCommitValidationListener(pluginValidators), new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
new AccountValidator(allUsers))); new AccountValidator(allUsers)));
} }
public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, RefControl refControl) { public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, IdentifiedUser user) {
IdentifiedUser user = refControl.getUser().asIdentifiedUser();
// Generally only include validators that are based on permissions of the // Generally only include validators that are based on permissions of the
// user creating a change for a merged commit; generally exclude // user creating a change for a merged commit; generally exclude
// validators that would require amending the change in order to correct. // validators that would require amending the change in order to correct.
@@ -208,22 +221,23 @@ public class CommitValidators {
+ " line format in commit message footer"; + " line format in commit message footer";
private static final Pattern CHANGE_ID = Pattern.compile(CHANGE_ID_PATTERN); 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 canonicalWebUrl;
private final String installCommitMsgHookCommand; private final String installCommitMsgHookCommand;
private final SshInfo sshInfo; private final SshInfo sshInfo;
private final IdentifiedUser user; private final IdentifiedUser user;
public ChangeIdValidator( public ChangeIdValidator(
RefControl refControl, ProjectState projectState,
IdentifiedUser user,
String canonicalWebUrl, String canonicalWebUrl,
String installCommitMsgHookCommand, String installCommitMsgHookCommand,
SshInfo sshInfo) { SshInfo sshInfo) {
this.projectControl = refControl.getProjectControl(); this.projectState = projectState;
this.canonicalWebUrl = canonicalWebUrl; this.canonicalWebUrl = canonicalWebUrl;
this.installCommitMsgHookCommand = installCommitMsgHookCommand; this.installCommitMsgHookCommand = installCommitMsgHookCommand;
this.sshInfo = sshInfo; this.sshInfo = sshInfo;
this.user = projectControl.getUser().asIdentifiedUser(); this.user = user;
} }
@Override @Override
@@ -238,7 +252,7 @@ public class CommitValidators {
String sha1 = commit.abbreviate(SHA1_LENGTH).name(); String sha1 = commit.abbreviate(SHA1_LENGTH).name();
if (idList.isEmpty()) { if (idList.isEmpty()) {
if (projectControl.getProjectState().isRequireChangeID()) { if (projectState.isRequireChangeID()) {
String shortMsg = commit.getShortMessage(); String shortMsg = commit.getShortMessage();
if (shortMsg.startsWith(CHANGE_ID_PREFIX) if (shortMsg.startsWith(CHANGE_ID_PREFIX)
&& CHANGE_ID && CHANGE_ID
@@ -342,12 +356,15 @@ public class CommitValidators {
/** If this is the special project configuration branch, validate the config. */ /** If this is the special project configuration branch, validate the config. */
public static class ConfigValidator implements CommitValidationListener { 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 RevWalk rw;
private final AllUsersName allUsers; private final AllUsersName allUsers;
public ConfigValidator(RefControl refControl, RevWalk rw, AllUsersName allUsers) { public ConfigValidator(
this.refControl = refControl; Branch.NameKey branch, IdentifiedUser user, RevWalk rw, AllUsersName allUsers) {
this.branch = branch;
this.user = user;
this.rw = rw; this.rw = rw;
this.allUsers = allUsers; this.allUsers = allUsers;
} }
@@ -355,9 +372,7 @@ public class CommitValidators {
@Override @Override
public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent) public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
throws CommitValidationException { throws CommitValidationException {
IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser(); if (REFS_CONFIG.equals(branch.get())) {
if (REFS_CONFIG.equals(refControl.getRefName())) {
List<CommitValidationMessage> messages = new ArrayList<>(); List<CommitValidationMessage> messages = new ArrayList<>();
try { try {
@@ -373,7 +388,7 @@ public class CommitValidators {
} catch (ConfigInvalidException | IOException e) { } catch (ConfigInvalidException | IOException e) {
log.error( log.error(
"User " "User "
+ currentUser.getUserName() + user.getUserName()
+ " tried to push an invalid project configuration " + " tried to push an invalid project configuration "
+ receiveEvent.command.getNewId().name() + receiveEvent.command.getNewId().name()
+ " for project " + " for project "
@@ -383,10 +398,9 @@ public class CommitValidators {
} }
} }
if (allUsers.equals(refControl.getProjectControl().getProject().getNameKey()) if (allUsers.equals(branch.getParentKey()) && RefNames.isRefsUsers(branch.get())) {
&& RefNames.isRefsUsers(refControl.getRefName())) {
List<CommitValidationMessage> messages = new ArrayList<>(); List<CommitValidationMessage> messages = new ArrayList<>();
Account.Id accountId = Account.Id.fromRef(refControl.getRefName()); Account.Id accountId = Account.Id.fromRef(branch.get());
if (accountId != null) { if (accountId != null) {
try { try {
WatchConfig wc = new WatchConfig(accountId); WatchConfig wc = new WatchConfig(accountId);
@@ -401,7 +415,7 @@ public class CommitValidators {
} catch (IOException | ConfigInvalidException e) { } catch (IOException | ConfigInvalidException e) {
log.error( log.error(
"User " "User "
+ currentUser.getUserName() + user.getUserName()
+ " tried to push an invalid watch configuration " + " tried to push an invalid watch configuration "
+ receiveEvent.command.getNewId().name() + receiveEvent.command.getNewId().name()
+ " for account " + " for account "