Merge changes Idf5a3bed,Ia27043f7

* changes:
  CommitValidators: uniformize argument ordering for creation methods
  ReceiveCommits: provide banned commits to forReceiveCommits
This commit is contained in:
David Pursehouse
2018-09-06 23:25:29 +00:00
committed by Gerrit Code Review
4 changed files with 40 additions and 33 deletions

View File

@@ -552,8 +552,6 @@ public class ChangeInserter implements InsertChangeOp {
return; return;
} }
PermissionBackend.ForRef perm =
permissionBackend.user(ctx.getUser()).project(ctx.getProject()).ref(refName);
try { try {
try (CommitReceivedEvent event = try (CommitReceivedEvent event =
new CommitReceivedEvent( new CommitReceivedEvent(
@@ -565,7 +563,7 @@ public class ChangeInserter implements InsertChangeOp {
ctx.getIdentifiedUser())) { ctx.getIdentifiedUser())) {
commitValidatorsFactory commitValidatorsFactory
.forGerritCommits( .forGerritCommits(
perm, permissionBackend.user(ctx.getUser()).project(ctx.getProject()),
new Branch.NameKey(ctx.getProject(), refName), new Branch.NameKey(ctx.getProject(), refName),
ctx.getIdentifiedUser(), ctx.getIdentifiedUser(),
new NoSshInfo(), new NoSshInfo(),

View File

@@ -323,9 +323,6 @@ public class PatchSetInserter implements BatchUpdateOp {
return; return;
} }
PermissionBackend.ForRef perm =
permissionBackend.user(ctx.getUser()).ref(origNotes.getChange().getDest());
String refName = getPatchSetId().toRefName(); String refName = getPatchSetId().toRefName();
try (CommitReceivedEvent event = try (CommitReceivedEvent event =
new CommitReceivedEvent( new CommitReceivedEvent(
@@ -340,7 +337,7 @@ public class PatchSetInserter implements BatchUpdateOp {
ctx.getIdentifiedUser())) { ctx.getIdentifiedUser())) {
commitValidatorsFactory commitValidatorsFactory
.forGerritCommits( .forGerritCommits(
perm, permissionBackend.user(ctx.getUser()).project(ctx.getProject()),
origNotes.getChange().getDest(), origNotes.getChange().getDest(),
ctx.getIdentifiedUser(), ctx.getIdentifiedUser(),
new NoSshInfo(), new NoSshInfo(),

View File

@@ -1911,13 +1911,15 @@ class ReceiveCommits {
} }
try { try {
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, receivePack.getRevWalk());
if (validCommit( if (validCommit(
receivePack.getRevWalk().getObjectReader(), receivePack.getRevWalk().getObjectReader(),
changeEnt.getDest(), changeEnt.getDest(),
cmd, cmd,
newCommit, newCommit,
false, false,
changeEnt)) { changeEnt,
rejectCommits)) {
logger.atFine().log("Replacing change %s", changeEnt.getId()); logger.atFine().log("Replacing change %s", changeEnt.getId());
requestReplace(cmd, true, changeEnt, newCommit); requestReplace(cmd, true, changeEnt, newCommit);
} }
@@ -1966,6 +1968,10 @@ class ReceiveCommits {
} }
} }
private NoteMap loadRejectCommits() throws IOException {
return BanCommit.loadRejectCommitsMap(repo, receivePack.getRevWalk());
}
private List<CreateRequest> selectNewAndReplacedChangesFromMagicBranch(Task newProgress) { private List<CreateRequest> selectNewAndReplacedChangesFromMagicBranch(Task newProgress) {
logger.atFine().log("Finding new and replaced changes"); logger.atFine().log("Finding new and replaced changes");
List<CreateRequest> newChanges = new ArrayList<>(); List<CreateRequest> newChanges = new ArrayList<>();
@@ -1975,6 +1981,7 @@ class ReceiveCommits {
GroupCollector.create(changeRefsById(), db, psUtil, notesFactory, project.getNameKey()); GroupCollector.create(changeRefsById(), db, psUtil, notesFactory, project.getNameKey());
try { try {
NoteMap rejectCommits = loadRejectCommits();
RevCommit start = setUpWalkForSelectingChanges(); RevCommit start = setUpWalkForSelectingChanges();
if (start == null) { if (start == null) {
return Collections.emptyList(); return Collections.emptyList();
@@ -2079,7 +2086,8 @@ class ReceiveCommits {
magicBranch.cmd, magicBranch.cmd,
c, c,
magicBranch.merged, magicBranch.merged,
null)) { null,
loadRejectCommits())) {
// Not a change the user can propose? Abort as early as possible. // Not a change the user can propose? Abort as early as possible.
logger.atFine().log("Aborting early due to invalid commit"); logger.atFine().log("Aborting early due to invalid commit");
return Collections.emptyList(); return Collections.emptyList();
@@ -3043,6 +3051,8 @@ class ReceiveCommits {
walk.reset(); walk.reset();
walk.sort(RevSort.NONE); walk.sort(RevSort.NONE);
try { try {
NoteMap rejectCommits = loadRejectCommits();
RevObject parsedObject = walk.parseAny(cmd.getNewId()); RevObject parsedObject = walk.parseAny(cmd.getNewId());
if (!(parsedObject instanceof RevCommit)) { if (!(parsedObject instanceof RevCommit)) {
return; return;
@@ -3065,7 +3075,7 @@ class ReceiveCommits {
continue; continue;
} }
if (!validCommit(walk.getObjectReader(), branch, cmd, c, false, null)) { if (!validCommit(walk.getObjectReader(), branch, cmd, c, false, null, rejectCommits)) {
break; break;
} }
@@ -3102,9 +3112,9 @@ class ReceiveCommits {
ReceiveCommand cmd, ReceiveCommand cmd,
RevCommit commit, RevCommit commit,
boolean isMerged, boolean isMerged,
@Nullable Change change) @Nullable Change change,
NoteMap rejectCommits)
throws IOException { throws IOException {
PermissionBackend.ForRef perm = permissions.ref(branch.get());
ValidCommitKey key = new AutoValue_ReceiveCommits_ValidCommitKey(commit.copy(), branch); ValidCommitKey key = new AutoValue_ReceiveCommits_ValidCommitKey(commit.copy(), branch);
if (validCommits.contains(key)) { if (validCommits.contains(key)) {
@@ -3113,18 +3123,21 @@ class ReceiveCommits {
try (CommitReceivedEvent receiveEvent = try (CommitReceivedEvent receiveEvent =
new CommitReceivedEvent(cmd, project, branch.get(), objectReader, commit, user)) { new CommitReceivedEvent(cmd, project, branch.get(), objectReader, commit, user)) {
CommitValidators validators = CommitValidators validators;
isMerged if (isMerged) {
? commitValidatorsFactory.forMergedCommits( validators =
project.getNameKey(), perm, user.asIdentifiedUser()) commitValidatorsFactory.forMergedCommits(permissions, branch, user.asIdentifiedUser());
: commitValidatorsFactory.forReceiveCommits( } else {
perm, validators =
commitValidatorsFactory.forReceiveCommits(
permissions,
branch, branch,
user.asIdentifiedUser(), user.asIdentifiedUser(),
sshInfo, sshInfo,
repo, rejectCommits,
receiveEvent.revWalk, receiveEvent.revWalk,
change); change);
}
for (CommitValidationMessage m : validators.validate(receiveEvent)) { for (CommitValidationMessage m : validators.validate(receiveEvent)) {
messages.add( messages.add(

View File

@@ -35,7 +35,6 @@ import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Branch.NameKey; import com.google.gerrit.reviewdb.client.Branch.NameKey;
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.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;
@@ -45,11 +44,9 @@ import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ValidationError; 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.PermissionBackend.ForRef;
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.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
@@ -123,15 +120,15 @@ public class CommitValidators {
} }
public CommitValidators forReceiveCommits( public CommitValidators forReceiveCommits(
PermissionBackend.ForRef perm, PermissionBackend.ForProject forProject,
Branch.NameKey branch, Branch.NameKey branch,
IdentifiedUser user, IdentifiedUser user,
SshInfo sshInfo, SshInfo sshInfo,
Repository repo, NoteMap rejectCommits,
RevWalk rw, RevWalk rw,
@Nullable Change change) @Nullable Change change)
throws IOException { throws IOException {
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw); PermissionBackend.ForRef perm = forProject.ref(branch.get());
ProjectState projectState = projectCache.checkedGet(branch.getParentKey()); ProjectState projectState = projectCache.checkedGet(branch.getParentKey());
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
@@ -157,13 +154,14 @@ public class CommitValidators {
} }
public CommitValidators forGerritCommits( public CommitValidators forGerritCommits(
ForRef perm, PermissionBackend.ForProject forProject,
NameKey branch, NameKey branch,
IdentifiedUser user, IdentifiedUser user,
SshInfo sshInfo, SshInfo sshInfo,
RevWalk rw, RevWalk rw,
@Nullable Change change) @Nullable Change change)
throws IOException { throws IOException {
PermissionBackend.ForRef perm = forProject.ref(branch.get());
ProjectState projectState = projectCache.checkedGet(branch.getParentKey()); ProjectState projectState = projectCache.checkedGet(branch.getParentKey());
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
@@ -187,7 +185,7 @@ public class CommitValidators {
} }
public CommitValidators forMergedCommits( public CommitValidators forMergedCommits(
Project.NameKey project, PermissionBackend.ForRef perm, IdentifiedUser user) PermissionBackend.ForProject forProject, Branch.NameKey branch, IdentifiedUser user)
throws IOException { throws IOException {
// 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
@@ -202,10 +200,11 @@ public class CommitValidators {
// discuss what to do about it. // discuss what to do about it.
// - Plugin validators may do things like require certain commit message // - Plugin validators may do things like require certain commit message
// formats, so we play it safe and exclude them. // formats, so we play it safe and exclude them.
PermissionBackend.ForRef perm = forProject.ref(branch.get());
return new CommitValidators( return new CommitValidators(
ImmutableList.of( ImmutableList.of(
new UploadMergesPermissionValidator(perm), new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectCache.checkedGet(project)), new ProjectStateValidationListener(projectCache.checkedGet(branch.getParentKey())),
new AuthorUploaderValidator(user, perm, canonicalWebUrl), new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl))); new CommitterUploaderValidator(user, perm, canonicalWebUrl)));
} }