Split the ChangeHooks interface in 2
The current ChangeHook interface supports methods for event creators and
listeners. Since creators and listeners are unlikely to be the same
classes, it does not make sense to expose both sets of methods to each
type of user. While this change will clean things up API wise, it also
paves the way to split out some event firing logic from the ChangeHooks
class. By splitting this logic it provides a better path to make it
possible for other classes such as a plugins to fire events.
Change-Id: I0906c86fa6ea32aa519b452134d2caddf489df09
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 41918e3..66911b8 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -29,8 +29,8 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.SshSession;
-import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.EventListener;
+import com.google.gerrit.common.EventSource;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.ChangeStatus;
@@ -98,13 +98,13 @@
private IdentifiedUser.GenericFactory factory;
@Inject
- ChangeHooks hooks;
+ EventSource source;
@Before
public void setUp() throws Exception {
mergeResults = Maps.newHashMap();
CurrentUser listenerUser = factory.create(user.id);
- hooks.addEventListener(new EventListener() {
+ source.addEventListener(new EventListener() {
@Override
public void onEvent(Event event) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
index 90efc48..8c103e2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
@@ -86,7 +86,8 @@
/** Spawns local executables when a hook action occurs. */
@Singleton
-public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
+public class ChangeHookRunner implements ChangeHooks, EventSource,
+ LifecycleListener {
/** A logger for this class. */
private static final Logger log = LoggerFactory.getLogger(ChangeHookRunner.class);
@@ -95,6 +96,7 @@
protected void configure() {
bind(ChangeHookRunner.class);
bind(ChangeHooks.class).to(ChangeHookRunner.class);
+ bind(EventSource.class).to(ChangeHookRunner.class);
listener().to(ChangeHookRunner.class);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java
index ec8e39d..4e8a21e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java
@@ -22,7 +22,6 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.events.Event;
import com.google.gwtorm.server.OrmException;
@@ -34,10 +33,6 @@
/** Invokes hooks on server actions. */
public interface ChangeHooks {
- public void addEventListener(EventListener listener, CurrentUser user);
-
- public void removeEventListener(EventListener listener);
-
/**
* Fire the Patchset Created Hook.
*
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java b/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java
index bd3ed13..8bbb600 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java
@@ -32,7 +32,7 @@
import java.util.Set;
/** Does not invoke hooks. */
-public final class DisabledChangeHooks implements ChangeHooks {
+public final class DisabledChangeHooks implements ChangeHooks, EventSource {
@Override
public void addEventListener(EventListener listener, CurrentUser user) {
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/EventSource.java b/gerrit-server/src/main/java/com/google/gerrit/common/EventSource.java
new file mode 100644
index 0000000..e2c4b34
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/EventSource.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2014 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.google.gerrit.common;
+
+import com.google.gerrit.server.CurrentUser;
+
+/** Distributes Events to ChangeListeners. Register listeners here. */
+public interface EventSource {
+ public void addEventListener(EventListener listener, CurrentUser user);
+
+ public void removeEventListener(EventListener listener);
+}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java
index b02c591..889d3fb 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java
@@ -16,8 +16,8 @@
import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE;
-import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.EventListener;
+import com.google.gerrit.common.EventSource;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.server.IdentifiedUser;
@@ -52,7 +52,7 @@
private IdentifiedUser currentUser;
@Inject
- private ChangeHooks hooks;
+ private EventSource source;
@Inject
@StreamCommandExecutor
@@ -132,12 +132,12 @@
}
stdout = toPrintWriter(out);
- hooks.addEventListener(listener, currentUser);
+ source.addEventListener(listener, currentUser);
}
@Override
protected void onExit(final int rc) {
- hooks.removeEventListener(listener);
+ source.removeEventListener(listener);
synchronized (taskLock) {
done = true;
@@ -148,7 +148,7 @@
@Override
public void destroy() {
- hooks.removeEventListener(listener);
+ source.removeEventListener(listener);
final boolean exit;
synchronized (taskLock) {
@@ -196,7 +196,7 @@
// destroy() above, or it closed the stream and is no longer
// accepting output. Either way terminate this instance.
//
- hooks.removeEventListener(listener);
+ source.removeEventListener(listener);
flush();
onExit(0);
return;
