opendev/gerrit
Code Issues Proposed changes

No longer require RefControl to construct ChangeHookValidator

Browse Source 
The RefControl was being used just to get the IdentifiedUser, however
the user is already in the CommitReceiveEvent.  Removing the need to
pass in RefControl to the constructor makes it more easy to eventually
make the ChangeHookValidator injectable via a plugin.

Change-Id: Ic721adaf46613d0e1f94604d9c5a47ba03758686
This commit is contained in:
Martin Fick
2015-11-11 17:46:20 -07:00
parent 27b252a160
commit 24ad0d9a18
1 changed files with 24 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@@ -131,7 +131,7 @@ public class CommitValidators {
validators.add(new ConfigValidator(refControl, repo)); validators.add(new ConfigValidator(refControl, repo));
validators.add(new BannedCommitsValidator(rejectCommits)); validators.add(new BannedCommitsValidator(rejectCommits));
validators.add(new PluginCommitValidationListener(commitValidationListeners)); validators.add(new PluginCommitValidationListener(commitValidationListeners));
validators.add(new ChangeHookValidator(refControl, hooks)); validators.add(new ChangeHookValidator(hooks));
List<CommitValidationMessage> messages = new LinkedList<>(); List<CommitValidationMessage> messages = new LinkedList<>();
@@ -165,7 +165,7 @@ public class CommitValidators {
} }
validators.add(new ConfigValidator(refControl, repo)); validators.add(new ConfigValidator(refControl, repo));
validators.add(new PluginCommitValidationListener(commitValidationListeners)); validators.add(new PluginCommitValidationListener(commitValidationListeners));
validators.add(new ChangeHookValidator(refControl, hooks)); validators.add(new ChangeHookValidator(hooks));
List<CommitValidationMessage> messages = new LinkedList<>(); List<CommitValidationMessage> messages = new LinkedList<>();
@@ -549,42 +549,37 @@ public class CommitValidators {
/** Reject commits that don't pass user-supplied ref-update hook. */ /** Reject commits that don't pass user-supplied ref-update hook. */
public static class ChangeHookValidator implements public static class ChangeHookValidator implements
CommitValidationListener { CommitValidationListener {
private final RefControl refControl;
private final ChangeHooks hooks; private final ChangeHooks hooks;
public ChangeHookValidator(RefControl refControl, ChangeHooks hooks) { public ChangeHookValidator(ChangeHooks hooks) {
this.refControl = refControl;
this.hooks = hooks; this.hooks = hooks;
} }
@Override @Override
public List<CommitValidationMessage> onCommitReceived( public List<CommitValidationMessage> onCommitReceived(
CommitReceivedEvent receiveEvent) throws CommitValidationException { CommitReceivedEvent receiveEvent) throws CommitValidationException {
IdentifiedUser user = receiveEvent.user;
String refname = receiveEvent.refName;
ObjectId old = ObjectId.zeroId();
if (receiveEvent.commit.getParentCount() > 0) {
old = receiveEvent.commit.getParent(0);
}
if (refControl.getUser().isIdentifiedUser()) { if (receiveEvent.command.getRefName().startsWith(REFS_CHANGES)) {
IdentifiedUser user = refControl.getUser().asIdentifiedUser(); /*
* If the ref-update hook tries to distinguish behavior between pushes to
String refname = receiveEvent.refName; * refs/heads/... and refs/for/..., make sure we send it the correct refname.
ObjectId old = ObjectId.zeroId(); * Also, if this is targetting refs/for/, make sure we behave the same as
if (receiveEvent.commit.getParentCount() > 0) { * what a push to refs/for/ would behave; in particular, setting oldrev to
old = receiveEvent.commit.getParent(0); * 0000000000000000000000000000000000000000.
} */
if (receiveEvent.command.getRefName().startsWith(REFS_CHANGES)) { refname = refname.replace(R_HEADS, "refs/for/refs/heads/");
/* old = ObjectId.zeroId();
* If the ref-update hook tries to distinguish behavior between pushes to }
* refs/heads/... and refs/for/..., make sure we send it the correct refname. HookResult result = hooks.doRefUpdateHook(receiveEvent.project, refname,
* Also, if this is targetting refs/for/, make sure we behave the same as user.getAccount(), old, receiveEvent.commit);
* what a push to refs/for/ would behave; in particular, setting oldrev to if (result != null && result.getExitValue() != 0) {
* 0000000000000000000000000000000000000000. throw new CommitValidationException(result.toString().trim());
*/
refname = refname.replace(R_HEADS, "refs/for/refs/heads/");
old = ObjectId.zeroId();
}
HookResult result = hooks.doRefUpdateHook(receiveEvent.project, refname,
user.getAccount(), old, receiveEvent.commit);
if (result != null && result.getExitValue() != 0) {
throw new CommitValidationException(result.toString().trim());
}
} }
return Collections.emptyList(); return Collections.emptyList();
} }