opendev/gerrit
Code Issues Proposed changes

Split ChangeHookRunner along new interface lines

Browse Source 
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
This commit is contained in:
Martin Fick
2014-12-12 13:04:02 -07:00
committed by Hugo Arès
parent be61959313
commit f58c3c94cb
4 changed files with 202 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
View File

@@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
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 @@ public class Daemon extends SiteProgram {
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());

169
gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
View File

@@ -32,7 +32,6 @@ 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.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.ChangeAttribute;
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.HashtagsChangedEvent;
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 @@ import java.util.concurrent.TimeoutException;
/** 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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
}
}
/** 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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
/** 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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
event.projectName = project.get();
event.headName = headName;
fireEvent(project, event);
dispatcher.postEvent(project, event);
if (!projectCreatedHook.isPresent()) {
return;
@@ -378,7 +359,7 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
}
});
fireEvent(change, event, db);
dispatcher.postEvent(change, event, db);
if (!commentAddedHook.isPresent()) {
return;
@@ -511,7 +492,7 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
}
});
fireEvent(refName, event);
dispatcher.postEvent(refName, event);
if (!refUpdatedHook.isPresent()) {
return;
@@ -691,7 +672,7 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
event.changer = accountAttributeSupplier(account);
event.oldTopic = oldTopic;
fireEvent(change, event, db);
dispatcher.postEvent(change, event, db);
if (!topicChangedHook.isPresent()) {
return;
@@ -762,7 +743,7 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
}
}
@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 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher,
});
}
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

181
gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java Normal file
View File

@@ -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;
}
}

2
gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
View File

@@ -19,6 +19,7 @@ import static com.google.inject.Stage.PRODUCTION;
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 @@ public class WebAppInitializer extends GuiceServletContextListener
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());