Listen to change events instead of listening to ref updates At the moment the plugin is listening to all ref-updated events. It then filters for 'refs/changes/*' refs and parses the change and patch set information out of the ref name in order to add the reviewers for this change/patch set. This works fine except when Gerrit sends the ref-updated event before the new patch set is created in the database (see issue 2181). Because of this the plugin cannot add reviewers when a change is rebased in the WebUI or if modifications to the access rights are saved for review. Change the plugin to listen to change events instead of listening to ref-updated events since for the change events is it ensured that they are only fired after the patch set and change have been created in the database. Saving modifications to the access rights for review is currently not triggering a change event, but this is fixed by [1]. [1] https://gerrit-review.googlesource.com/51282 Bug: issue 2181 Change-Id: Ib971f5aa2ae5fc19141f1680b1292efa2757bbf8 Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/RefUpdateListener.java b/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ChangeUpdatedListener.java similarity index 86% rename from src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/RefUpdateListener.java rename to src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ChangeUpdatedListener.java index f40cdfe..36aec15 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/RefUpdateListener.java +++ b/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ChangeUpdatedListener.java
@@ -26,8 +26,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.gerrit.common.ChangeListener; import com.google.gerrit.extensions.annotations.PluginName; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; @@ -35,6 +35,8 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.PluginConfigFactory; +import com.google.gerrit.server.events.ChangeEvent; +import com.google.gerrit.server.events.PatchSetCreatedEvent; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.project.NoSuchProjectException; @@ -46,10 +48,10 @@ import com.google.inject.Provider; import com.google.inject.ProvisionException; -class RefUpdateListener implements GitReferenceUpdatedListener { +class ChangeUpdatedListener implements ChangeListener { private static final Logger log = LoggerFactory - .getLogger(RefUpdateListener.class); + .getLogger(ChangeUpdatedListener.class); private final ReviewersByBlame.Factory reviewersByBlameFactory; private final GitRepositoryManager repoManager; @@ -62,7 +64,7 @@ private ReviewDb db; @Inject - RefUpdateListener(final ReviewersByBlame.Factory reviewersByBlameFactory, + ChangeUpdatedListener(final ReviewersByBlame.Factory reviewersByBlameFactory, final GitRepositoryManager repoManager, final WorkQueue workQueue, final IdentifiedUser.GenericFactory identifiedUserFactory, final ThreadLocalRequestContext tl, @@ -80,11 +82,12 @@ } @Override - public void onGitReferenceUpdated(final Event e) { - if (!e.getRefName().startsWith("refs/changes/")) { + public void onChangeEvent(ChangeEvent event) { + if (!(event instanceof PatchSetCreatedEvent)) { return; } - Project.NameKey projectName = new Project.NameKey(e.getProjectName()); + PatchSetCreatedEvent e = (PatchSetCreatedEvent) event; + Project.NameKey projectName = new Project.NameKey(e.change.project); int maxReviewers; try { @@ -116,21 +119,22 @@ try { reviewDb = schemaFactory.open(); try { - PatchSet.Id psId = PatchSet.Id.fromRef(e.getRefName()); + Change.Id changeId = new Change.Id(Integer.parseInt(e.change.number)); + PatchSet.Id psId = new PatchSet.Id(changeId, Integer.parseInt(e.patchSet.number)); PatchSet ps = reviewDb.patchSets().get(psId); if (ps == null) { - log.warn("No patch set found for " + e.getRefName()); + log.warn("Patch set " + psId.get() + " not found."); return; } final Change change = reviewDb.changes().get(psId.getParentKey()); if (change == null) { - log.warn("No change found for " + e.getRefName()); + log.warn("Change " + changeId.get() + " not found."); return; } final RevCommit commit = - rw.parseCommit(ObjectId.fromString(e.getNewObjectId())); + rw.parseCommit(ObjectId.fromString(e.patchSet.revision)); final Runnable task = reviewersByBlameFactory.create(commit, change, ps, maxReviewers, git);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ReviewersByBlameModule.java b/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ReviewersByBlameModule.java index f61296b..508c443 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ReviewersByBlameModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/reviewersbyblame/ReviewersByBlameModule.java
@@ -14,15 +14,15 @@ package com.googlesource.gerrit.plugins.reviewersbyblame; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.common.ChangeListener; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.server.config.FactoryModule; public class ReviewersByBlameModule extends FactoryModule { @Override protected void configure() { - DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to( - RefUpdateListener.class); + DynamicSet.bind(binder(), ChangeListener.class).to( + ChangeUpdatedListener.class); factory(ReviewersByBlame.Factory.class); } }