Trigger owners-autoassign when setting WIP to false
Fix a problem where the owners-autoassign action was not
triggered when the WIP flag was set to false as part of
a general review feedback, hence without the specific
change notes tag setReadyForReview.
Bug: Issue 14751
Change-Id: I44137a7cf6fa7677065b7d2c1cfc65164dbda657
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
index b6a1474..2a072d1 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
@@ -43,6 +43,8 @@
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeNotesCommit;
+import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
@@ -53,9 +55,13 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.util.List;
import java.util.Set;
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.FooterKey;
import org.eclipse.jgit.revwalk.RevCommit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -64,6 +70,8 @@
public class GitRefListener implements GitReferenceUpdatedListener {
private static final Logger logger = LoggerFactory.getLogger(GitRefListener.class);
+ private static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
+
private final GerritApi api;
private final PatchListCache patchListCache;
@@ -154,7 +162,7 @@
ChangeNotes changeNotes = notesFactory.createChecked(projectNameKey, changeId);
if ((!RefNames.isNoteDbMetaRef(refName)
&& isChangeToBeProcessed(changeNotes.getChange(), cfg))
- || isChangeSetReadyForReview(changeNotes, event.getNewObjectId())) {
+ || isChangeSetReadyForReview(repository, changeNotes, event.getNewObjectId())) {
processEvent(repository, event, changeId);
}
}
@@ -171,13 +179,32 @@
|| cfg.getEnum(PROJECT_CONFIG_AUTOASSIGN_WIP_CHANGES, TRUE).equals(TRUE);
}
- private boolean isChangeSetReadyForReview(ChangeNotes changeNotes, String metaObjectId) {
- return !changeNotes.getChange().isWorkInProgress()
- && changeNotes.getChangeMessages().stream()
- .filter(message -> message.getKey().uuid().equals(metaObjectId))
- .map(message -> message.getTag())
- .filter(Predicates.notNull())
- .anyMatch(tag -> tag.contains(ChangeMessagesUtil.TAG_SET_READY));
+ private boolean isChangeSetReadyForReview(
+ Repository repository, ChangeNotes changeNotes, String metaObjectId)
+ throws MissingObjectException, IncorrectObjectTypeException, IOException {
+ if (changeNotes.getChange().isWorkInProgress()) {
+ return false;
+ }
+
+ if (changeNotes.getChangeMessages().stream()
+ .filter(message -> message.getKey().uuid().equals(metaObjectId))
+ .map(message -> message.getTag())
+ .filter(Predicates.notNull())
+ .anyMatch(tag -> tag.contains(ChangeMessagesUtil.TAG_SET_READY))) {
+ return true;
+ }
+
+ try (ChangeNotesRevWalk revWalk = ChangeNotesCommit.newRevWalk(repository)) {
+ ChangeNotesCommit metaCommit = revWalk.parseCommit(ObjectId.fromString(metaObjectId));
+ if (metaCommit.getParentCount() == 0) {
+ // The first commit cannot be a 'Set ready' operation
+ return false;
+ }
+ List<String> wipFooterLines = metaCommit.getFooterLines(FOOTER_WORK_IN_PROGRESS);
+ return wipFooterLines != null
+ && !wipFooterLines.isEmpty()
+ && Boolean.FALSE.toString().equalsIgnoreCase(wipFooterLines.get(0));
+ }
}
public void processEvent(Repository repository, Event event, Change.Id cId) {
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
index db7e305..2cbceda 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
@@ -19,9 +19,10 @@
import static org.junit.Assert.assertEquals;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.api.projects.ConfigValue;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -63,19 +64,10 @@
@Test
public void shouldNotProcessNoteDbOnlyRefs() throws Exception {
- String changeRefPrefix = createChange().getChange().getId().toRefPrefix();
+ int changeNum = createChange().getChange().getId().get();
int baselineProcessedEvents = gitRefListener().getProcessedEvents();
- ReferenceUpdatedEventTest refUpdatedEvent =
- new ReferenceUpdatedEventTest(
- project,
- changeRefPrefix + RefNames.META_SUFFIX.substring(1),
- anOldObjectId,
- aNewObjectId,
- Type.CREATE,
- admin.id());
-
- gitRefListener().onGitReferenceUpdated(refUpdatedEvent);
+ gApi.changes().id(changeNum).current().review(new ReviewInput().message("Foo comment"));
assertEquals(baselineProcessedEvents, gitRefListener().getProcessedEvents());
}
@@ -91,6 +83,23 @@
}
@Test
+ public void shouldProcessSendAndStartReviewOnNoteDb() throws Exception {
+ int wipChangeNum = createChange().getChange().getId().get();
+ gApi.changes().id(wipChangeNum).setWorkInProgress();
+
+ int baselineProcessedEvents = gitRefListener().getProcessedEvents();
+
+ ReviewInput input = new ReviewInput().message("Let's start the review");
+ input.setWorkInProgress(false);
+
+ RestResponse resp =
+ adminRestSession.post("/changes/" + wipChangeNum + "/revisions/1/review", input);
+ resp.assertOK();
+
+ assertEquals(1, gitRefListener().getProcessedEvents() - baselineProcessedEvents);
+ }
+
+ @Test
public void shouldProcessWipChangesByDefault() throws Exception {
int baselineProcessedEvents = gitRefListener().getProcessedEvents();
