ReviewerAddedListener: Notify multiple reviewers added in same event

When multiple reviewers were added to a change, notify them all with
a single event and let listeners act on each individual reviewer as
necessary.

This change is needed to allow splitting the mail notifications to a
listener.

Change-Id: I8b1f3918877b9635a6592beae6ad5aeea0a5adce
This commit is contained in:
David Pursehouse
2016-07-12 16:44:23 +09:00
committed by Edwin Kempin
parent 770af87746
commit 6f0fb43127
5 changed files with 43 additions and 27 deletions

View File

@@ -17,12 +17,14 @@ package com.google.gerrit.extensions.events;
import com.google.gerrit.extensions.annotations.ExtensionPoint; import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.common.AccountInfo;
/** Notified whenever a Reviewer is added to a change. */ import java.util.List;
/** Notified whenever one or more Reviewers are added to a change. */
@ExtensionPoint @ExtensionPoint
public interface ReviewerAddedListener { public interface ReviewerAddedListener {
interface Event extends ChangeEvent { interface Event extends ChangeEvent {
AccountInfo getReviewer(); List<AccountInfo> getReviewers();
} }
void onReviewerAdded(Event event); void onReviewersAdded(Event event);
} }

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.change;
import static com.google.gerrit.extensions.client.ReviewerState.CC; import static com.google.gerrit.extensions.client.ReviewerState.CC;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER; import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import com.google.common.base.Function;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
@@ -40,7 +41,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.AccountsCollection; import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.account.GroupMembers; import com.google.gerrit.server.account.GroupMembers;
@@ -96,7 +96,6 @@ public class PostReviewers
private final Provider<IdentifiedUser> user; private final Provider<IdentifiedUser> user;
private final IdentifiedUser.GenericFactory identifiedUserFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final Config cfg; private final Config cfg;
private final AccountCache accountCache;
private final ReviewerJson json; private final ReviewerJson json;
private final ReviewerAdded reviewerAdded; private final ReviewerAdded reviewerAdded;
private final NotesMigration migration; private final NotesMigration migration;
@@ -115,7 +114,6 @@ public class PostReviewers
Provider<IdentifiedUser> user, Provider<IdentifiedUser> user,
IdentifiedUser.GenericFactory identifiedUserFactory, IdentifiedUser.GenericFactory identifiedUserFactory,
@GerritServerConfig Config cfg, @GerritServerConfig Config cfg,
AccountCache accountCache,
ReviewerJson json, ReviewerJson json,
ReviewerAdded reviewerAdded, ReviewerAdded reviewerAdded,
NotesMigration migration) { NotesMigration migration) {
@@ -132,7 +130,6 @@ public class PostReviewers
this.user = user; this.user = user;
this.identifiedUserFactory = identifiedUserFactory; this.identifiedUserFactory = identifiedUserFactory;
this.cfg = cfg; this.cfg = cfg;
this.accountCache = accountCache;
this.json = json; this.json = json;
this.reviewerAdded = reviewerAdded; this.reviewerAdded = reviewerAdded;
this.migration = migration; this.migration = migration;
@@ -355,11 +352,15 @@ public class PostReviewers
} }
emailReviewers(rsrc.getChange(), addedReviewers, addedCCs); emailReviewers(rsrc.getChange(), addedReviewers, addedCCs);
if (!addedReviewers.isEmpty()) { if (!addedReviewers.isEmpty()) {
for (PatchSetApproval psa : addedReviewers) { List<Account.Id> reviewers = Lists.transform(addedReviewers,
Account account = accountCache.get(psa.getAccountId()).getAccount(); new Function<PatchSetApproval, Account.Id>() {
reviewerAdded.fire(rsrc.getChange(), patchSet, account, @Override
public Account.Id apply(PatchSetApproval psa) {
return psa.getAccountId();
}
});
reviewerAdded.fire(rsrc.getChange(), patchSet, reviewers,
ctx.getAccount(), ctx.getWhen()); ctx.getAccount(), ctx.getWhen());
}
} }
} }
} }

View File

@@ -321,7 +321,7 @@ public class StreamEventsApiListener implements
} }
@Override @Override
public void onReviewerAdded(ReviewerAddedListener.Event ev) { public void onReviewersAdded(ReviewerAddedListener.Event ev) {
try { try {
ChangeNotes notes = getNotes(ev.getChange()); ChangeNotes notes = getNotes(ev.getChange());
Change change = notes.getChange(); Change change = notes.getChange();
@@ -330,9 +330,10 @@ public class StreamEventsApiListener implements
event.change = changeAttributeSupplier(change); event.change = changeAttributeSupplier(change);
event.patchSet = patchSetAttributeSupplier(change, event.patchSet = patchSetAttributeSupplier(change,
psUtil.current(db.get(), notes)); psUtil.current(db.get(), notes));
event.reviewer = accountAttributeSupplier(ev.getReviewer()); for (AccountInfo reviewer : ev.getReviewers()) {
event.reviewer = accountAttributeSupplier(reviewer);
dispatcher.get().postEvent(change, event); dispatcher.get().postEvent(change, event);
}
} catch (OrmException e) { } catch (OrmException e) {
log.error("Failed to dispatch event", e); log.error("Failed to dispatch event", e);
} }

View File

@@ -15,6 +15,8 @@
package com.google.gerrit.server.extensions.events; package com.google.gerrit.server.extensions.events;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.common.base.Function;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.common.RevisionInfo;
@@ -33,6 +35,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException; import java.io.IOException;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.List;
public class ReviewerAdded { public class ReviewerAdded {
private static final Logger log = private static final Logger log =
@@ -49,29 +52,38 @@ public class ReviewerAdded {
} }
public void fire(ChangeInfo change, RevisionInfo revision, public void fire(ChangeInfo change, RevisionInfo revision,
AccountInfo reviewer, AccountInfo adder, Timestamp when) { List<AccountInfo> reviewers, AccountInfo adder, Timestamp when) {
if (!listeners.iterator().hasNext()) { if (!listeners.iterator().hasNext()) {
return; return;
} }
Event event = new Event(change, revision, reviewer, adder, when); Event event = new Event(change, revision, reviewers, adder, when);
for (ReviewerAddedListener l : listeners) { for (ReviewerAddedListener l : listeners) {
try { try {
l.onReviewerAdded(event); l.onReviewersAdded(event);
} catch (Exception e) { } catch (Exception e) {
log.warn("Error in event listener, e"); log.warn("Error in event listener, e");
} }
} }
} }
public void fire(Change change, PatchSet patchSet, Account account, public void fire(Change change, PatchSet patchSet, List<Account.Id> reviewers,
Account adder, Timestamp when) { Account adder, Timestamp when) {
if (!listeners.iterator().hasNext()) { if (!listeners.iterator().hasNext() || reviewers.isEmpty()) {
return; return;
} }
List<AccountInfo> transformed = Lists.transform(reviewers,
new Function<Account.Id, AccountInfo>() {
@Override
public AccountInfo apply(Account.Id account) {
return util.accountInfo(account);
}
});
try { try {
fire(util.changeInfo(change), fire(util.changeInfo(change),
util.revisionInfo(change.getProject(), patchSet), util.revisionInfo(change.getProject(), patchSet),
util.accountInfo(account), transformed,
util.accountInfo(adder), util.accountInfo(adder),
when); when);
} catch (PatchListNotAvailableException | GpgException | IOException } catch (PatchListNotAvailableException | GpgException | IOException
@@ -82,17 +94,17 @@ public class ReviewerAdded {
private static class Event extends AbstractRevisionEvent private static class Event extends AbstractRevisionEvent
implements ReviewerAddedListener.Event { implements ReviewerAddedListener.Event {
private final AccountInfo reviewer; private final List<AccountInfo> reviewers;
Event(ChangeInfo change, RevisionInfo revision, AccountInfo reviewer, Event(ChangeInfo change, RevisionInfo revision, List<AccountInfo> reviewers,
AccountInfo adder, Timestamp when) { AccountInfo adder, Timestamp when) {
super(change, revision, adder, when, NotifyHandling.ALL); super(change, revision, adder, when, NotifyHandling.ALL);
this.reviewer = reviewer; this.reviewers = reviewers;
} }
@Override @Override
public AccountInfo getReviewer() { public List<AccountInfo> getReviewers() {
return reviewer; return reviewers;
} }
} }
} }