Expose RevWalk in CommitReceivedEvent

CommitValidationListeners are called from the body of
BatchUpdateOp#updateRepo implementations prior to flushing the inserter
that created them. Thus, an implementation relying on just the commit
SHA-1 and the project name will be unable to actually read the commit
or, say, its tree.

Include a RevWalk in the event, and ensure that the RevCommit is parsed
using this RevWalk. This also avoids another class of bugs where the
caller constructs their own RevWalk which is different from the one that
created the RevCommit.

We could have passed the RevWalk that parsed the original copy of the
commit, but this would allow plugin implementations to reset it or set
additional flags/options. Since this listener is called from
ReceiveCommits, we can't let them do that lest they mess up the walk
over new changes.

Bug: Issue 5914
Change-Id: Iaf8317cb344fab0a2f87a1e11bff2642445ed510
This commit is contained in:
Dave Borowitz 2017-03-31 15:48:55 -04:00
parent 7261ceb876
commit bd778f8c01
4 changed files with 30 additions and 16 deletions

View File

@ -516,16 +516,18 @@ public class ChangeInserter implements InsertChangeOp {
RefControl refControl =
projectControlFactory.controlFor(ctx.getProject(), ctx.getUser()).controlForRef(refName);
String refName = psId.toRefName();
CommitReceivedEvent event =
try (CommitReceivedEvent event =
new CommitReceivedEvent(
new ReceiveCommand(ObjectId.zeroId(), commit.getId(), refName),
refControl.getProjectControl().getProject(),
change.getDest().get(),
ctx.getRevWalk().getObjectReader(),
commit,
ctx.getIdentifiedUser());
commitValidatorsFactory
.create(validatePolicy, refControl, new NoSshInfo(), ctx.getRepository())
.validate(event);
ctx.getIdentifiedUser())) {
commitValidatorsFactory
.create(validatePolicy, refControl, new NoSshInfo(), ctx.getRepository())
.validate(event);
}
} catch (CommitValidationException e) {
throw new ResourceConflictException(e.getFullMessage());
} catch (NoSuchProjectException e) {

View File

@ -311,7 +311,7 @@ public class PatchSetInserter implements BatchUpdateOp {
}
String refName = getPatchSetId().toRefName();
CommitReceivedEvent event =
try (CommitReceivedEvent event =
new CommitReceivedEvent(
new ReceiveCommand(
ObjectId.zeroId(),
@ -319,10 +319,9 @@ public class PatchSetInserter implements BatchUpdateOp {
refName.substring(0, refName.lastIndexOf('/') + 1) + "new"),
origCtl.getProjectControl().getProject(),
origCtl.getRefControl().getRefName(),
ctx.getRevWalk().getObjectReader(),
commit,
ctx.getIdentifiedUser());
try {
ctx.getIdentifiedUser())) {
commitValidatorsFactory
.create(validatePolicy, origCtl.getRefControl(), new NoSshInfo(), ctx.getRepository())
.validate(event);

View File

@ -16,14 +16,19 @@ package com.google.gerrit.server.events;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
import java.io.IOException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
public class CommitReceivedEvent extends RefEvent {
public class CommitReceivedEvent extends RefEvent implements AutoCloseable {
static final String TYPE = "commit-received";
public ReceiveCommand command;
public Project project;
public String refName;
public RevWalk revWalk;
public RevCommit commit;
public IdentifiedUser user;
@ -35,14 +40,18 @@ public class CommitReceivedEvent extends RefEvent {
ReceiveCommand command,
Project project,
String refName,
RevCommit commit,
IdentifiedUser user) {
ObjectReader reader,
ObjectId commitId,
IdentifiedUser user)
throws IOException {
this();
this.command = command;
this.project = project;
this.refName = refName;
this.commit = commit;
this.revWalk = new RevWalk(reader);
this.commit = revWalk.parseCommit(commitId);
this.user = user;
revWalk.parseBody(commit);
}
@Override
@ -54,4 +63,9 @@ public class CommitReceivedEvent extends RefEvent {
public String getRefName() {
return refName;
}
@Override
public void close() {
revWalk.close();
}
}

View File

@ -2763,8 +2763,6 @@ public class ReceiveCommits {
RevCommit c = rw.parseCommit(id);
rw.parseBody(c);
CommitReceivedEvent receiveEvent =
new CommitReceivedEvent(cmd, project, ctl.getRefName(), c, user);
CommitValidators.Policy policy;
if (magicBranch != null
@ -2775,7 +2773,8 @@ public class ReceiveCommits {
policy = CommitValidators.Policy.RECEIVE_COMMITS;
}
try {
try (CommitReceivedEvent receiveEvent =
new CommitReceivedEvent(cmd, project, ctl.getRefName(), rw.getObjectReader(), c, user)) {
messages.addAll(
commitValidatorsFactory.create(policy, ctl, sshInfo, repo).validate(receiveEvent));
} catch (CommitValidationException e) {