diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java index 4df1a4709a..6cc8d3c628 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java @@ -31,6 +31,7 @@ import com.google.gerrit.server.events.ChangeMergedEvent; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.events.RefEvent; import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.server.events.ReviewerDeletedEvent; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -66,9 +67,13 @@ public class EventRecorder { new UserScopedEventListener() { @Override public void onEvent(Event e) { - if (e instanceof RefEvent) { + if (e instanceof ReviewerDeletedEvent) { + recordedEvents.put( + ReviewerDeletedEvent.TYPE, (ReviewerDeletedEvent) e); + } else if (e instanceof RefEvent) { RefEvent event = (RefEvent) e; - String key = key(event.getType(), event.getProjectNameKey().get(), + String key = refEventKey(event.getType(), + event.getProjectNameKey().get(), event.getRefName()); recordedEvents.put(key, event); } @@ -81,7 +86,7 @@ public class EventRecorder { }); } - private static String key(String type, String project, String ref) { + private static String refEventKey(String type, String project, String ref) { return String.format("%s-%s-%s", type, project, ref); } @@ -97,7 +102,7 @@ public class EventRecorder { private ImmutableList getRefUpdatedEvents(String project, String refName, int expectedSize) { - String key = key(RefUpdatedEvent.TYPE, project, refName); + String key = refEventKey(RefUpdatedEvent.TYPE, project, refName); if (expectedSize == 0) { assertThat(recordedEvents).doesNotContainKey(key); return ImmutableList.of(); @@ -114,7 +119,7 @@ public class EventRecorder { private ImmutableList getChangeMergedEvents(String project, String branch, int expectedSize) { - String key = key(ChangeMergedEvent.TYPE, project, branch); + String key = refEventKey(ChangeMergedEvent.TYPE, project, branch); if (expectedSize == 0) { assertThat(recordedEvents).doesNotContainKey(key); return ImmutableList.of(); @@ -129,6 +134,22 @@ public class EventRecorder { return events; } + private ImmutableList getReviewerDeletedEvents( + int expectedSize) { + String key = ReviewerDeletedEvent.TYPE; + if (expectedSize == 0) { + assertThat(recordedEvents).doesNotContainKey(key); + return ImmutableList.of(); + } + assertThat(recordedEvents).containsKey(key); + ImmutableList events = FluentIterable + .from(recordedEvents.get(key)) + .transform(new RefEventTransformer()) + .toList(); + assertThat(events).hasSize(expectedSize); + return events; + } + public void assertRefUpdatedEvents(String project, String branch, String... expected) throws Exception { ImmutableList events = getRefUpdatedEvents(project, @@ -180,6 +201,19 @@ public class EventRecorder { } } + public void assertReviewerDeletedEvents(String... expected) { + ImmutableList events = + getReviewerDeletedEvents(expected.length / 2); + int i = 0; + for (ReviewerDeletedEvent event : events) { + String id = event.change.get().id; + assertThat(id).isEqualTo(expected[i]); + String reviewer = event.reviewer.get().email; + assertThat(reviewer).isEqualTo(expected[i+1]); + i += 2; + } + } + public void close() { eventListenerRegistration.remove(); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index 4cbb4aeab9..728cbd46b3 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -690,6 +690,8 @@ public class ChangeIT extends AbstractDaemonTest { reviewerIt = reviewers.iterator(); assertThat(reviewerIt.next()._accountId) .isEqualTo(admin.getId().get()); + + eventRecorder.assertReviewerDeletedEvents(changeId, user.email); } @Test diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ReviewerDeletedListener.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ReviewerDeletedListener.java new file mode 100644 index 0000000000..3c2f723f48 --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ReviewerDeletedListener.java @@ -0,0 +1,34 @@ +// 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.extensions.events; + +import com.google.gerrit.extensions.annotations.ExtensionPoint; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.common.ApprovalInfo; + +import java.util.Map; + +/** Notified whenever a Reviewer is removed from a change. */ +@ExtensionPoint +public interface ReviewerDeletedListener { + interface Event extends ChangeEvent { + AccountInfo getReviewer(); + String getComment(); + Map getNewApprovals(); + Map getOldApprovals(); + } + + void onReviewerDeleted(Event event); +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookApiListener.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookApiListener.java index 412818497a..86ba0a23ce 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookApiListener.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookApiListener.java @@ -32,6 +32,7 @@ import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; import com.google.gerrit.extensions.events.HashtagsEditedListener; import com.google.gerrit.extensions.events.NewProjectCreatedListener; import com.google.gerrit.extensions.events.ReviewerAddedListener; +import com.google.gerrit.extensions.events.ReviewerDeletedListener; import com.google.gerrit.extensions.events.RevisionCreatedListener; import com.google.gerrit.extensions.events.TopicEditedListener; import com.google.gerrit.extensions.registration.DynamicSet; @@ -80,6 +81,7 @@ public class ChangeHookApiListener implements HashtagsEditedListener, NewProjectCreatedListener, ReviewerAddedListener, + ReviewerDeletedListener, RevisionCreatedListener, TopicEditedListener { /** A logger for this class. */ @@ -109,6 +111,8 @@ public class ChangeHookApiListener implements .to(ChangeHookApiListener.class); DynamicSet.bind(binder(), ReviewerAddedListener.class) .to(ChangeHookApiListener.class); + DynamicSet.bind(binder(), ReviewerDeletedListener.class) + .to(ChangeHookApiListener.class); DynamicSet.bind(binder(), RevisionCreatedListener.class) .to(ChangeHookApiListener.class); DynamicSet.bind(binder(), TopicEditedListener.class) @@ -288,6 +292,23 @@ public class ChangeHookApiListener implements } } + @Override + public void onReviewerDeleted(ReviewerDeletedListener.Event ev) { + try { + ChangeNotes notes = getNotes(ev.getChange()); + hooks.doReviewerDeletedHook(notes.getChange(), + getAccount(ev.getReviewer()), + psUtil.current(db.get(), notes), + ev.getComment(), + convertApprovalsMap(ev.getNewApprovals()), + convertApprovalsMap(ev.getOldApprovals()), + db.get()); + } catch (OrmException e) { + log.error("ReviewerDeleted hook failed to run " + + ev.getChange()._number, e); + } + } + @Override public void onTopicEdited(TopicEditedListener.Event ev) { try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java index 0fd10c5499..28d7dcd844 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.change; import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; @@ -39,6 +38,7 @@ import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.DeleteReviewer.Input; +import com.google.gerrit.server.extensions.events.ReviewerDeleted; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate.ChangeContext; import com.google.gerrit.server.git.BatchUpdate.Context; @@ -72,7 +72,7 @@ public class DeleteReviewer implements RestModifyView { private final ChangeMessagesUtil cmUtil; private final BatchUpdate.Factory batchUpdateFactory; private final IdentifiedUser.GenericFactory userFactory; - private final ChangeHooks hooks; + private final ReviewerDeleted reviewerDeleted; private final Provider user; private final DeleteReviewerSender.Factory deleteReviewerSenderFactory; @@ -83,7 +83,7 @@ public class DeleteReviewer implements RestModifyView { ChangeMessagesUtil cmUtil, BatchUpdate.Factory batchUpdateFactory, IdentifiedUser.GenericFactory userFactory, - ChangeHooks hooks, + ReviewerDeleted reviewerDeleted, Provider user, DeleteReviewerSender.Factory deleteReviewerSenderFactory) { this.dbProvider = dbProvider; @@ -92,7 +92,7 @@ public class DeleteReviewer implements RestModifyView { this.cmUtil = cmUtil; this.batchUpdateFactory = batchUpdateFactory; this.userFactory = userFactory; - this.hooks = hooks; + this.reviewerDeleted = reviewerDeleted; this.user = user; this.deleteReviewerSenderFactory = deleteReviewerSenderFactory; } @@ -182,13 +182,10 @@ public class DeleteReviewer implements RestModifyView { } emailReviewers(ctx.getProject(), currChange, del, changeMessage); - try { - hooks.doReviewerDeletedHook(currChange, reviewer, currPs, - changeMessage.getMessage(), newApprovals, oldApprovals, - dbProvider.get()); - } catch (OrmException e) { - log.warn("ChangeHook.doReviewerDeletedHook invocation failed", e); - } + reviewerDeleted.fire(currChange, currPs, reviewer, + changeMessage.getMessage(), + newApprovals, oldApprovals, + ctx.getWhen()); } private Iterable approvals(ChangeContext ctx, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index a9104021af..6b19d1a151 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -44,6 +44,7 @@ import com.google.gerrit.extensions.events.NewProjectCreatedListener; import com.google.gerrit.extensions.events.PluginEventListener; import com.google.gerrit.extensions.events.ProjectDeletedListener; import com.google.gerrit.extensions.events.ReviewerAddedListener; +import com.google.gerrit.extensions.events.ReviewerDeletedListener; import com.google.gerrit.extensions.events.RevisionCreatedListener; import com.google.gerrit.extensions.events.TopicEditedListener; import com.google.gerrit.extensions.events.UsageDataPublishedListener; @@ -301,6 +302,7 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.setOf(binder(), ChangeMergedListener.class); DynamicSet.setOf(binder(), ChangeRestoredListener.class); DynamicSet.setOf(binder(), ReviewerAddedListener.class); + DynamicSet.setOf(binder(), ReviewerDeletedListener.class); DynamicSet.setOf(binder(), RevisionCreatedListener.class); DynamicSet.setOf(binder(), TopicEditedListener.class); DynamicSet.setOf(binder(), AgreementSignupListener.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java index 9ade5ec9e7..447e8b214f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java @@ -32,6 +32,7 @@ public class EventTypes { register(RefUpdatedEvent.TYPE, RefUpdatedEvent.class); register(RefReceivedEvent.TYPE, RefReceivedEvent.class); register(ReviewerAddedEvent.TYPE, ReviewerAddedEvent.class); + register(ReviewerDeletedEvent.TYPE, ReviewerDeletedEvent.class); register(PatchSetCreatedEvent.TYPE, PatchSetCreatedEvent.class); register(TopicChangedEvent.TYPE, TopicChangedEvent.class); register(ProjectCreatedEvent.TYPE, ProjectCreatedEvent.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerDeletedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerDeletedEvent.java index 1b57906d01..f206cac6a4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerDeletedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerDeletedEvent.java @@ -20,7 +20,7 @@ import com.google.gerrit.server.data.AccountAttribute; import com.google.gerrit.server.data.ApprovalAttribute; public class ReviewerDeletedEvent extends PatchSetEvent { - static final String TYPE = "reviewer-deleted"; + public static final String TYPE = "reviewer-deleted"; public Supplier reviewer; public Supplier approvals; public String comment; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java new file mode 100644 index 0000000000..0320fc21f8 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java @@ -0,0 +1,128 @@ +// 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.server.extensions.events; + +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.common.ApprovalInfo; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.RevisionInfo; +import com.google.gerrit.extensions.events.ReviewerDeletedListener; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.server.GpgException; +import com.google.gerrit.server.patch.PatchListNotAvailableException; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.sql.Timestamp; +import java.util.Map; + +public class ReviewerDeleted { + private static final Logger log = + LoggerFactory.getLogger(ReviewerDeleted.class); + + private final DynamicSet listeners; + private final EventUtil util; + + @Inject + ReviewerDeleted(DynamicSet listeners, + EventUtil util) { + this.listeners = listeners; + this.util = util; + } + + public void fire(ChangeInfo change, RevisionInfo revision, + AccountInfo reviewer, String message, + Map newApprovals, + Map oldApprovals) { + if (!listeners.iterator().hasNext()) { + return; + } + Event e = new Event(change, + revision, + reviewer, + message, + newApprovals, + oldApprovals); + for (ReviewerDeletedListener listener : listeners) { + listener.onReviewerDeleted(e); + } + } + + public void fire(Change change, PatchSet patchSet, Account reviewer, + String message, + Map newApprovals, + Map oldApprovals, Timestamp ts) { + if (!listeners.iterator().hasNext()) { + return; + } + try { + fire(util.changeInfo(change), + util.revisionInfo(change.getProject(), patchSet), + util.accountInfo(reviewer), + message, + util.approvals(reviewer, newApprovals, ts), + util.approvals(reviewer, oldApprovals, ts)); + } catch (PatchListNotAvailableException | GpgException | IOException + | OrmException e) { + log.error("Couldn't fire event", e); + } + + } + + private static class Event extends AbstractRevisionEvent + implements ReviewerDeletedListener.Event { + private final AccountInfo reviewer; + private final String comment; + private final Map newApprovals; + private final Map oldApprovals; + + Event(ChangeInfo change, RevisionInfo revision, AccountInfo reviewer, + String comment, Map newApprovals, + Map oldApprovals) { + super(change, revision); + this.reviewer = reviewer; + this.comment = comment; + this.newApprovals = newApprovals; + this.oldApprovals = oldApprovals; + } + + @Override + public AccountInfo getReviewer() { + return reviewer; + } + + @Override + public String getComment() { + return comment; + } + + @Override + public Map getNewApprovals() { + return newApprovals; + } + + @Override + public Map getOldApprovals() { + return oldApprovals; + } + } +}