Split ChangeHookRunner along new interface lines
Create a new EventBroker class that can post and fire Events to
listeners. Make ChangeHookRunner use this new class to post events.
Change-Id: I308a61cc53f102a76265d1f318c7f42983d5599f
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index 5108315..1edf4a1 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -20,6 +20,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.gerrit.common.ChangeHookRunner;
+import com.google.gerrit.common.EventBroker;
import com.google.gerrit.gpg.GpgModule;
import com.google.gerrit.httpd.AllRequestFilter;
import com.google.gerrit.httpd.GerritOptions;
@@ -340,6 +341,7 @@
modules.add(new WorkQueue.Module());
modules.add(new ChangeHookRunner.Module());
+ modules.add(new EventBroker.Module());
modules.add(new ReceiveCommitsExecutorModule());
modules.add(new DiffExecutorModule());
modules.add(new MimeUtil2Module());
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 400c1c0..af92b73 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
@@ -32,7 +32,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.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.AnonymousCowardName;
@@ -44,7 +43,6 @@
import com.google.gerrit.server.data.PatchSetAttribute;
import com.google.gerrit.server.data.RefUpdateAttribute;
import com.google.gerrit.server.events.ChangeAbandonedEvent;
-import com.google.gerrit.server.events.ChangeEvent;
import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.events.ChangeRestoredEvent;
import com.google.gerrit.server.events.CommentAddedEvent;
@@ -54,17 +52,12 @@
import com.google.gerrit.server.events.MergeFailedEvent;
import com.google.gerrit.server.events.PatchSetCreatedEvent;
import com.google.gerrit.server.events.ProjectCreatedEvent;
-import com.google.gerrit.server.events.ProjectEvent;
-import com.google.gerrit.server.events.RefEvent;
import com.google.gerrit.server.events.RefUpdatedEvent;
import com.google.gerrit.server.events.ReviewerAddedEvent;
import com.google.gerrit.server.events.TopicChangedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.WorkQueue;
-import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectControl;
-import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -100,8 +93,8 @@
/** Spawns local executables when a hook action occurs. */
@Singleton
-public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
- LifecycleListener, NewProjectCreatedListener {
+public class ChangeHookRunner implements ChangeHooks, LifecycleListener,
+ NewProjectCreatedListener {
/** A logger for this class. */
private static final Logger log = LoggerFactory.getLogger(ChangeHookRunner.class);
@@ -110,7 +103,6 @@
protected void configure() {
bind(ChangeHookRunner.class);
bind(ChangeHooks.class).to(ChangeHookRunner.class);
- bind(EventDispatcher.class).to(ChangeHookRunner.class);
DynamicSet.bind(binder(), NewProjectCreatedListener.class).to(ChangeHookRunner.class);
listener().to(ChangeHookRunner.class);
}
@@ -164,13 +156,6 @@
}
}
- /** Listeners to receive changes as they happen (limited by visibility
- * of user). */
- private final DynamicSet<UserScopedEventListener> listeners;
-
- /** Listeners to receive all changes as they happen. */
- private final DynamicSet<EventListener> unrestrictedListeners;
-
/** Path of the new patchset hook. */
private final Optional<Path> patchsetCreatedHook;
@@ -235,7 +220,7 @@
/** Timeout value for synchronous hooks */
private final int syncHookTimeout;
- private final ChangeNotes.Factory notesFactory;
+ private final EventDispatcher dispatcher;
/**
* Create a new ChangeHookRunner.
@@ -255,9 +240,7 @@
ProjectCache projectCache,
AccountCache accountCache,
EventFactory eventFactory,
- DynamicSet<UserScopedEventListener> listeners,
- DynamicSet<EventListener> unrestrictedListeners,
- ChangeNotes.Factory notesFactory) {
+ EventDispatcher dispatcher) {
this.anonymousCowardName = anonymousCowardName;
this.repoManager = repoManager;
this.hookQueue = queue.createQueue(1, "hook");
@@ -265,9 +248,7 @@
this.accountCache = accountCache;
this.eventFactory = eventFactory;
this.sitePaths = sitePath;
- this.listeners = listeners;
- this.unrestrictedListeners = unrestrictedListeners;
- this.notesFactory = notesFactory;
+ this.dispatcher = dispatcher;
Path hooksPath;
String hooksPathConfig = config.getString("hooks", null, "path");
@@ -353,7 +334,7 @@
event.projectName = project.get();
event.headName = headName;
- fireEvent(project, event);
+ dispatcher.postEvent(project, event);
if (!projectCreatedHook.isPresent()) {
return;
@@ -378,7 +359,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.uploader = accountAttributeSupplier(uploader);
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!patchsetCreatedHook.isPresent()) {
return;
@@ -415,7 +396,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.uploader = accountAttributeSupplier(uploader);
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!draftPublishedHook.isPresent()) {
return;
@@ -467,7 +448,7 @@
}
});
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!commentAddedHook.isPresent()) {
return;
@@ -511,7 +492,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.newRev = mergeResultRev;
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!changeMergedHook.isPresent()) {
return;
@@ -546,7 +527,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.reason = reason;
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!mergeFailedHook.isPresent()) {
return;
@@ -581,7 +562,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.reason = reason;
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!changeAbandonedHook.isPresent()) {
return;
@@ -616,7 +597,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.reason = reason;
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!changeRestoredHook.isPresent()) {
return;
@@ -662,7 +643,7 @@
}
});
- fireEvent(refName, event);
+ dispatcher.postEvent(refName, event);
if (!refUpdatedHook.isPresent()) {
return;
@@ -691,7 +672,7 @@
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.reviewer = accountAttributeSupplier(account);
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!reviewerAddedHook.isPresent()) {
return;
@@ -721,7 +702,7 @@
event.changer = accountAttributeSupplier(account);
event.oldTopic = oldTopic;
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!topicChangedHook.isPresent()) {
return;
@@ -762,7 +743,7 @@
event.added = hashtagArray(added);
event.removed = hashtagArray(removed);
- fireEvent(change, event, db);
+ dispatcher.postEvent(change, event, db);
if (!hashtagsChangedHook.isPresent()) {
return;
@@ -810,28 +791,6 @@
}
}
- @Override
- public void postEvent(Change change, ChangeEvent event, ReviewDb db)
- throws OrmException {
- fireEvent(change, event, db);
- }
-
- @Override
- public void postEvent(Branch.NameKey branchName, RefEvent event) {
- fireEvent(branchName, event);
- }
-
- @Override
- public void postEvent(Project.NameKey projectName, ProjectEvent event) {
- fireEvent(projectName, event);
- }
-
- @Override
- public void postEvent(com.google.gerrit.server.events.Event event,
- ReviewDb db) throws OrmException {
- fireEvent(event, db);
- }
-
private Supplier<AccountState> getAccountSupplier(
final Account.Id account) {
return Suppliers.memoize(
@@ -894,100 +853,6 @@
});
}
- private void fireEventForUnrestrictedListeners(com.google.gerrit.server.events.Event event) {
- for (EventListener listener : unrestrictedListeners) {
- listener.onEvent(event);
- }
- }
-
- private void fireEvent(Change change, ChangeEvent event, ReviewDb db)
- throws OrmException {
- for (UserScopedEventListener listener : listeners) {
- if (isVisibleTo(change, listener.getUser(), db)) {
- listener.onEvent(event);
- }
- }
-
- fireEventForUnrestrictedListeners( event );
- }
-
- private void fireEvent(Project.NameKey project, ProjectEvent event) {
- for (UserScopedEventListener listener : listeners) {
- if (isVisibleTo(project, listener.getUser())) {
- listener.onEvent(event);
- }
- }
-
- fireEventForUnrestrictedListeners(event);
- }
-
- private void fireEvent(Branch.NameKey branchName, RefEvent event) {
- for (UserScopedEventListener listener : listeners) {
- if (isVisibleTo(branchName, listener.getUser())) {
- listener.onEvent(event);
- }
- }
-
- fireEventForUnrestrictedListeners(event);
- }
-
- private void fireEvent(com.google.gerrit.server.events.Event event,
- ReviewDb db) throws OrmException {
- for (UserScopedEventListener listener : listeners) {
- if (isVisibleTo(event, listener.getUser(), db)) {
- listener.onEvent(event);
- }
- }
-
- fireEventForUnrestrictedListeners(event);
- }
-
- private boolean isVisibleTo(Project.NameKey project, CurrentUser user) {
- ProjectState pe = projectCache.get(project);
- if (pe == null) {
- return false;
- }
- return pe.controlFor(user).isVisible();
- }
-
- private boolean isVisibleTo(Change change, CurrentUser user, ReviewDb db)
- throws OrmException {
- if (change == null) {
- return false;
- }
- ProjectState pe = projectCache.get(change.getProject());
- if (pe == null) {
- return false;
- }
- ProjectControl pc = pe.controlFor(user);
- return pc.controlFor(db, change).isVisible(db);
- }
-
- private boolean isVisibleTo(Branch.NameKey branchName, CurrentUser user) {
- ProjectState pe = projectCache.get(branchName.getParentKey());
- if (pe == null) {
- return false;
- }
- ProjectControl pc = pe.controlFor(user);
- return pc.controlForRef(branchName).isVisible();
- }
-
- private boolean isVisibleTo(com.google.gerrit.server.events.Event event,
- CurrentUser user, ReviewDb db) throws OrmException {
- if (event instanceof RefEvent) {
- RefEvent refEvent = (RefEvent) event;
- String ref = refEvent.getRefName();
- if (PatchSet.isChangeRef(ref)) {
- Change.Id cid = PatchSet.Id.fromRef(ref).getParentKey();
- Change change = notesFactory
- .create(db, refEvent.getProjectNameKey(), cid).getChange();
- return isVisibleTo(change, user, db);
- }
- return isVisibleTo(refEvent.getBranchNameKey(), user);
- }
- return true;
- }
-
/**
* Create an ApprovalAttribute for the given approval suitable for serialization to JSON.
* @param approval
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java b/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
new file mode 100644
index 0000000..01317e2
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
@@ -0,0 +1,181 @@
+// Copyright (C) 2016 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.extensions.registration.DynamicSet;
+import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Change;
+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.ChangeEvent;
+import com.google.gerrit.server.events.Event;
+import com.google.gerrit.server.events.ProjectEvent;
+import com.google.gerrit.server.events.RefEvent;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+/** Distributes Events to listeners if they are allowed to see them */
+@Singleton
+public class EventBroker implements EventDispatcher {
+
+ public static class Module extends LifecycleModule {
+ @Override
+ protected void configure() {
+ bind(EventDispatcher.class).to(EventBroker.class);
+ }
+ }
+
+ /**
+ * Listeners to receive changes as they happen (limited by visibility of
+ * user).
+ */
+ private final DynamicSet<UserScopedEventListener> listeners;
+
+ /** Listeners to receive all changes as they happen. */
+ private final DynamicSet<EventListener> unrestrictedListeners;
+
+ private final ProjectCache projectCache;
+
+ private final ChangeNotes.Factory notesFactory;
+
+ @Inject
+ public EventBroker(DynamicSet<UserScopedEventListener> listeners,
+ DynamicSet<EventListener> unrestrictedListeners,
+ ProjectCache projectCache,
+ ChangeNotes.Factory notesFactory) {
+ this.listeners = listeners;
+ this.unrestrictedListeners = unrestrictedListeners;
+ this.projectCache = projectCache;
+ this.notesFactory = notesFactory;
+ }
+
+ @Override
+ public void postEvent(Change change, ChangeEvent event, ReviewDb db)
+ throws OrmException {
+ fireEvent(change, event, db);
+ }
+
+ @Override
+ public void postEvent(Branch.NameKey branchName, RefEvent event) {
+ fireEvent(branchName, event);
+ }
+
+ @Override
+ public void postEvent(Project.NameKey projectName, ProjectEvent event) {
+ fireEvent(projectName, event);
+ }
+
+ @Override
+ public void postEvent(Event event, ReviewDb db) throws OrmException {
+ fireEvent(event, db);
+ }
+
+ private void fireEventForUnrestrictedListeners(Event event) {
+ for (EventListener listener : unrestrictedListeners) {
+ listener.onEvent(event);
+ }
+ }
+
+ protected void fireEvent(Change change, ChangeEvent event, ReviewDb db)
+ throws OrmException {
+ for (UserScopedEventListener listener : listeners) {
+ if (isVisibleTo(change, listener.getUser(), db)) {
+ listener.onEvent(event);
+ }
+ }
+ fireEventForUnrestrictedListeners(event);
+ }
+
+ protected void fireEvent(Project.NameKey project, ProjectEvent event) {
+ for (UserScopedEventListener listener : listeners) {
+ if (isVisibleTo(project, listener.getUser())) {
+ listener.onEvent(event);
+ }
+ }
+ fireEventForUnrestrictedListeners(event);
+ }
+
+ protected void fireEvent(Branch.NameKey branchName, RefEvent event) {
+ for (UserScopedEventListener listener : listeners) {
+ if (isVisibleTo(branchName, listener.getUser())) {
+ listener.onEvent(event);
+ }
+ }
+ fireEventForUnrestrictedListeners(event);
+ }
+
+ protected void fireEvent(Event event, ReviewDb db) throws OrmException {
+ for (UserScopedEventListener listener : listeners) {
+ if (isVisibleTo(event, listener.getUser(), db)) {
+ listener.onEvent(event);
+ }
+ }
+ fireEventForUnrestrictedListeners(event);
+ }
+
+ protected boolean isVisibleTo(Project.NameKey project, CurrentUser user) {
+ ProjectState pe = projectCache.get(project);
+ if (pe == null) {
+ return false;
+ }
+ return pe.controlFor(user).isVisible();
+ }
+
+ protected boolean isVisibleTo(Change change, CurrentUser user, ReviewDb db)
+ throws OrmException {
+ if (change == null) {
+ return false;
+ }
+ ProjectState pe = projectCache.get(change.getProject());
+ if (pe == null) {
+ return false;
+ }
+ ProjectControl pc = pe.controlFor(user);
+ return pc.controlFor(db, change).isVisible(db);
+ }
+
+ protected boolean isVisibleTo(Branch.NameKey branchName, CurrentUser user) {
+ ProjectState pe = projectCache.get(branchName.getParentKey());
+ if (pe == null) {
+ return false;
+ }
+ ProjectControl pc = pe.controlFor(user);
+ return pc.controlForRef(branchName).isVisible();
+ }
+
+ protected boolean isVisibleTo(Event event, CurrentUser user, ReviewDb db)
+ throws OrmException {
+ if (event instanceof RefEvent) {
+ RefEvent refEvent = (RefEvent) event;
+ String ref = refEvent.getRefName();
+ if (PatchSet.isChangeRef(ref)) {
+ Change.Id cid = PatchSet.Id.fromRef(ref).getParentKey();
+ Change change = notesFactory
+ .create(db, refEvent.getProjectNameKey(), cid).getChange();
+ return isVisibleTo(change, user, db);
+ }
+ return isVisibleTo(refEvent.getBranchNameKey(), user);
+ }
+ return true;
+ }
+}
diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
index cf3e76c..20c3c2a 100644
--- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
+++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
@@ -19,6 +19,7 @@
import com.google.common.base.Splitter;
import com.google.gerrit.common.ChangeHookRunner;
+import com.google.gerrit.common.EventBroker;
import com.google.gerrit.gpg.GpgModule;
import com.google.gerrit.httpd.auth.oauth.OAuthModule;
import com.google.gerrit.httpd.auth.openid.OpenIdModule;
@@ -294,6 +295,7 @@
private Injector createSysInjector() {
final List<Module> modules = new ArrayList<>();
modules.add(new DropWizardMetricMaker.RestModule());
+ modules.add(new EventBroker.Module());
modules.add(new ChangeHookRunner.Module());
modules.add(new ReceiveCommitsExecutorModule());
modules.add(new DiffExecutorModule());
