Rework ref-update hook to use RefOperationValidationListener
Instead of being invoked on every commit received, the ref-update hook
is now invoked before the ref update operation is finalized. Note that
the hook is no longer invoked on commits pushed for review or on changes
that are merged. It is invoked for creation/deletion of refs, and for
ref updates caused by direct pushes (i.e. bypassing review).
The previous behavior of the ref-update hook is moved into a new hook
named commit-received. A new parameter '--cmdref' is added, and the
special handling of 'refs/for' and 'refs/changes' is removed.
Also-By: David Pursehouse <dpursehouse@collab.net>
Bug: Issue 5739
Change-Id: I24d8580aef463f8bfb078d90a4e9e2ce6e8b9b93
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/CommitReceived.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/CommitReceived.java
index 5d8d852..02800fc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/CommitReceived.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/CommitReceived.java
@@ -14,11 +14,7 @@
package com.googlesource.gerrit.plugins.hooks;
-import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
-import static org.eclipse.jgit.lib.Constants.R_HEADS;
-
import com.google.common.collect.ImmutableList;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidationListener;
@@ -34,40 +30,28 @@
@Inject
CommitReceived(HookFactory hookFactory) {
- this.hook = hookFactory.createSync("refUpdateHook", "ref-update");
+ this.hook = hookFactory.createSync("commitReceivedHook", "commit-received");
this.hookFactory = hookFactory;
}
@Override
public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
throws CommitValidationException {
- IdentifiedUser user = receiveEvent.user;
String refname = receiveEvent.refName;
+ String commandRef = receiveEvent.command.getRefName();
ObjectId old = ObjectId.zeroId();
if (receiveEvent.commit.getParentCount() > 0) {
old = receiveEvent.commit.getParent(0);
}
- if (receiveEvent.command.getRefName().startsWith(REFS_CHANGES)) {
- /*
- * 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.
- * Also, if this is targetting refs/for/, make sure we behave the same as
- * what a push to refs/for/ would behave; in particular, setting oldrev
- * to 0000000000000000000000000000000000000000.
- */
- refname = refname.replace(R_HEADS, "refs/for/refs/heads/");
- old = ObjectId.zeroId();
- }
-
HookArgs args = hookFactory.createArgs();
String projectName = receiveEvent.project.getName();
args.add("--project", projectName);
args.add("--refname", refname);
- args.add("--uploader", user.getNameEmail());
+ args.add("--uploader", receiveEvent.user.getNameEmail());
args.add("--oldrev", old.name());
args.add("--newrev", receiveEvent.commit.name());
+ args.add("--cmdref", commandRef);
HookResult result = hook.run(projectName, args);
if (result != null) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/Module.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/Module.java
index 2dd3352..2dcce56 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/Module.java
@@ -30,6 +30,7 @@
import com.google.gerrit.extensions.events.TopicEditedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.RefOperationValidationListener;
import com.google.inject.AbstractModule;
import com.google.inject.Scopes;
import com.google.inject.internal.UniqueAnnotations;
@@ -51,6 +52,7 @@
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(GitReferenceUpdated.class);
DynamicSet.bind(binder(), HashtagsEditedListener.class).to(HashtagsEdited.class);
DynamicSet.bind(binder(), NewProjectCreatedListener.class).to(NewProjectCreated.class);
+ DynamicSet.bind(binder(), RefOperationValidationListener.class).to(RefUpdate.class);
DynamicSet.bind(binder(), ReviewerAddedListener.class).to(ReviewerAdded.class);
DynamicSet.bind(binder(), ReviewerDeletedListener.class).to(ReviewerDeleted.class);
DynamicSet.bind(binder(), RevisionCreatedListener.class).to(RevisionCreated.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/RefUpdate.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/RefUpdate.java
new file mode 100644
index 0000000..6189277
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/RefUpdate.java
@@ -0,0 +1,61 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.hooks;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.server.events.RefReceivedEvent;
+import com.google.gerrit.server.git.validators.RefOperationValidationListener;
+import com.google.gerrit.server.git.validators.ValidationMessage;
+import com.google.gerrit.server.validators.ValidationException;
+import com.google.inject.Inject;
+import java.util.Collections;
+import java.util.List;
+
+public class RefUpdate implements RefOperationValidationListener {
+ private final SynchronousHook hook;
+ private final HookFactory hookFactory;
+
+ @Inject
+ RefUpdate(HookFactory hookFactory) {
+ this.hook = hookFactory.createSync("refUpdateHook", "ref-update");
+ this.hookFactory = hookFactory;
+ }
+
+ @Override
+ public List<ValidationMessage> onRefOperation(RefReceivedEvent refEvent)
+ throws ValidationException {
+ String projectName = refEvent.project.getName();
+
+ HookArgs args = hookFactory.createArgs();
+ args.add("--project", projectName);
+ args.add("--uploader", refEvent.user.getNameEmail());
+ args.add("--oldrev", refEvent.command.getOldId().getName());
+ args.add("--newrev", refEvent.command.getNewId().getName());
+ args.add("--refname", refEvent.command.getRefName());
+
+ HookResult result = hook.run(projectName, args);
+ if (result != null) {
+ String output = result.toString();
+ if (result.getExitValue() != 0) {
+ throw new ValidationException(output);
+ }
+ if (!output.isEmpty()) {
+ return ImmutableList.of(new ValidationMessage(output, false));
+ }
+ }
+
+ return Collections.emptyList();
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index ccd1aa6..afb26cf 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -10,11 +10,11 @@
Make sure your hook scripts are executable if running on *nix.
-With the exception of the `ref-update` hook, hooks are run in the background
-after the relevant change has taken place so are unable to affect the outcome
-of any given change. Because of the fact the hooks are run in the background
-after the activity, a hook might not be notified about an event if
-the server is shutdown before the hook can be invoked.
+With the exception of the `commit-received` and `ref-update` hooks, hooks are
+run in the background after the relevant change has taken place so are unable
+to affect the outcome of any given change. Because of the fact the hooks are
+run in the background after the activity, a hook might not be notified about
+an event if the server is shutdown before the hook can be invoked.
Configuration
-------------
@@ -44,6 +44,9 @@
hooks.commentAddedHook
: Filename for the comment added hook. If not set, defaults to `comment-added`.
+hooks.commitReceived
+: Filename for the commit received hook. If not set, defaults to `commit-received`.
+
hooks.draftPublishedHook
: Filename for the draft published hook. If not set, defaults to `draft-published`.
diff --git a/src/main/resources/Documentation/hooks.md b/src/main/resources/Documentation/hooks.md
index 2bd19d3..1fcb0c9 100644
--- a/src/main/resources/Documentation/hooks.md
+++ b/src/main/resources/Documentation/hooks.md
@@ -4,6 +4,23 @@
ref-update
----------
+This is called when a ref update request is received by Gerrit. It allows a
+request to be rejected before it is committed to the Gerrit repository. If
+the script exits with non-zero return code the update will be rejected. Any
+output from the script will be returned to the user, regardless of the return
+code.
+
+This hook is called synchronously so it is recommended that it not block. A
+default timeout on the hook is set to 30 seconds to avoid "runaway" hooks using
+up server threads. See [`hooks.syncHookTimeout`][1] for configuration details.
+
+```
+ ref-update --project <project name> --refname <refname> --uploader <uploader> --oldrev <sha1> --newrev <sha1>
+```
+
+commit-received
+---------------
+
This is called when a push request is received by Gerrit. It allows a push to be
rejected before it is committed to the Gerrit repository. If the script exits
with non-zero return code the push will be rejected. Any output from the script
@@ -14,7 +31,7 @@
up server threads. See [`hooks.syncHookTimeout`][1] for configuration details.
```
- ref-update --project <project name> --refname <refname> --uploader <uploader> --oldrev <sha1> --newrev <sha1>
+ commit-received --project <project name> --refname <refname> --uploader <uploader> --oldrev <sha1> --newrev <sha1> --cmdref <refname>
```
patchset-created
