Merge branch 'stable-3.2' into stable-3.3
* stable-3.2:
Trigger owners-autoassign processing for change readyForReview
Change-Id: I33b00227bae5e923bedb6cc7c03d64286ff8ebd1
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 a38a237..7792583 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
@@ -18,10 +18,12 @@
import static com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace.IGNORE_NONE;
+import com.google.common.base.Predicates;
import com.google.common.collect.Sets;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.Listen;
@@ -32,8 +34,10 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
@@ -46,6 +50,7 @@
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -64,6 +69,8 @@
private Provider<CurrentUser> currentUserProvider;
+ private ChangeNotes.Factory notesFactory;
+
@Inject
public GitRefListener(
GerritApi api,
@@ -72,7 +79,8 @@
Accounts accounts,
ReviewerManager reviewerManager,
OneOffRequestContext oneOffReqCtx,
- Provider<CurrentUser> currentUserProvider) {
+ Provider<CurrentUser> currentUserProvider,
+ ChangeNotes.Factory notesFactory) {
this.api = api;
this.patchListCache = patchListCache;
this.repositoryManager = repositoryManager;
@@ -80,6 +88,7 @@
this.reviewerManager = reviewerManager;
this.oneOffReqCtx = oneOffReqCtx;
this.currentUserProvider = currentUserProvider;
+ this.notesFactory = notesFactory;
}
@Override
@@ -120,11 +129,15 @@
String projectName = event.getProjectName();
Repository repository;
try {
- repository = repositoryManager.openRepository(Project.NameKey.parse(projectName));
+ NameKey projectNameKey = Project.NameKey.parse(projectName);
+ repository = repositoryManager.openRepository(projectNameKey);
try {
String refName = event.getRefName();
- if (refName.startsWith(RefNames.REFS_CHANGES) && !RefNames.isNoteDbMetaRef(refName)) {
- processEvent(repository, event);
+ Change.Id changeId = Change.Id.fromRef(refName);
+ if (changeId != null
+ && (!RefNames.isNoteDbMetaRef(refName)
+ || isChangeSetReadyForReview(projectNameKey, changeId, event.getNewObjectId()))) {
+ processEvent(repository, event, changeId);
}
} finally {
repository.close();
@@ -134,15 +147,25 @@
}
}
- public void processEvent(Repository repository, Event event) {
- Change.Id cId = Change.Id.fromRef(event.getRefName());
+ private boolean isChangeSetReadyForReview(
+ NameKey project, Change.Id changeId, String metaObjectId) {
+ ChangeNotes changeNotes = notesFactory.createChecked(project, changeId);
+ 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));
+ }
+
+ public void processEvent(Repository repository, Event event, Change.Id cId) {
Changes changes = api.changes();
// The provider injected by Gerrit is shared with other workers on the
// same local thread and thus cannot be closed in this event listener.
try {
ChangeApi cApi = changes.id(cId.get());
ChangeInfo change = cApi.get();
- PatchList patchList = getPatchList(event, change);
+ PatchList patchList = getPatchList(repository, event, change);
if (patchList != null) {
PathOwners owners = new PathOwners(accounts, repository, change.branch, patchList);
Set<Account.Id> allReviewers = Sets.newHashSet();
@@ -162,17 +185,23 @@
}
}
- private PatchList getPatchList(Event event, ChangeInfo change) {
+ private PatchList getPatchList(Repository repository, Event event, ChangeInfo change) {
ObjectId newId = null;
- if (event.getNewObjectId() != null) {
- newId = ObjectId.fromString(event.getNewObjectId());
- }
-
- PatchListKey plKey = PatchListKey.againstCommit(null, newId, IGNORE_NONE);
+ PatchListKey plKey;
try {
+ if (RefNames.isNoteDbMetaRef(event.getRefName())) {
+ newId = ObjectId.fromString(change.currentRevision);
+ RevCommit revCommit = repository.parseCommit(newId);
+ plKey = PatchListKey.againstBase(newId, revCommit.getParentCount());
+ } else {
+ if (event.getNewObjectId() != null) {
+ newId = ObjectId.fromString(event.getNewObjectId());
+ }
+ plKey = PatchListKey.againstCommit(null, newId, IGNORE_NONE);
+ }
return patchListCache.get(plKey, Project.nameKey(change.project));
- } catch (PatchListNotAvailableException e) {
- logger.warn("Could not load patch list: {}", plKey, e);
+ } catch (PatchListNotAvailableException | IOException e) {
+ logger.warn("Could not load patch list for change {}", change.id, e);
}
return null;
}
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 5c9f1a7..1e0f8ec 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
@@ -21,11 +21,14 @@
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
+import java.util.stream.StreamSupport;
import org.eclipse.jgit.transport.ReceiveCommand.Type;
import org.junit.Test;
@@ -34,62 +37,97 @@
sysModule = "com.vmware.gerrit.owners.common.GitRefListenerIT$TestModule")
public class GitRefListenerIT extends LightweightPluginDaemonTest {
- @Inject GitRefListenerTest gitRefListener;
+ @Inject DynamicSet<GitReferenceUpdatedListener> allRefUpdateListeners;
@Inject ThreadLocalRequestContext requestContext;
- String aRefChange = RefNames.REFS_CHANGES + "01/01/01";
String anOldObjectId = "anOldRef";
String aNewObjectId = "aNewRef";
public static class TestModule extends AbstractModule {
@Override
protected void configure() {
- bind(GitReferenceUpdatedListener.class).to(GitRefListenerTest.class);
+ DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(GitRefListenerTest.class);
}
}
@Test
- public void shouldNotProcessNoteDbOnlyRefs() {
+ public void shouldNotProcessNoteDbOnlyRefs() throws Exception {
+ String changeRefPrefix = createChange().getChange().getId().toRefPrefix();
+ int baselineProcessedEvents = gitRefListener().getProcessedEvents();
+
ReferenceUpdatedEventTest refUpdatedEvent =
new ReferenceUpdatedEventTest(
project,
- RefNames.REFS_CHANGES + "01/01" + RefNames.META_SUFFIX,
+ changeRefPrefix + RefNames.META_SUFFIX.substring(1),
anOldObjectId,
aNewObjectId,
Type.CREATE,
admin.id());
- gitRefListener.onGitReferenceUpdated(refUpdatedEvent);
- assertEquals(0, gitRefListener.getProcessedEvents());
+ gitRefListener().onGitReferenceUpdated(refUpdatedEvent);
+ assertEquals(baselineProcessedEvents, gitRefListener().getProcessedEvents());
}
@Test
- public void shouldProcessRefChanges() {
- gitRefListener.onGitReferenceUpdated(newRefUpdateEvent());
- assertEquals(1, gitRefListener.getProcessedEvents());
+ public void shoulProcessSetReadyForReviewOnNoteDb() throws Exception {
+ int wipChangeNum = createChange().getChange().getId().get();
+ gApi.changes().id(wipChangeNum).setWorkInProgress();
+
+ int baselineProcessedEvents = gitRefListener().getProcessedEvents();
+
+ gApi.changes().id(wipChangeNum).setReadyForReview();
+ assertEquals(1, gitRefListener().getProcessedEvents() - baselineProcessedEvents);
+ }
+
+ @Test
+ public void shoulNotProcessEmptyMetaRefUpdatesAfterSetReadyForReviewOnNoteDb() throws Exception {
+ int wipChangeNum = createChange().getChange().getId().get();
+ gApi.changes().id(wipChangeNum).setWorkInProgress();
+ gApi.changes().id(wipChangeNum).setReadyForReview();
+ int baselineProcessedEvents = gitRefListener().getProcessedEvents();
+
+ gApi.changes().id(wipChangeNum).addReviewer("user");
+ assertEquals(baselineProcessedEvents, gitRefListener().getProcessedEvents());
+ }
+
+ @Test
+ public void shouldProcessRefChanges() throws Exception {
+ gitRefListener().onGitReferenceUpdated(newRefUpdateEvent());
+ assertEquals(1, gitRefListener().getProcessedEvents());
}
@Test
public void shouldRetrieveChangeFromAnonymousContext() throws Exception {
try (ManualRequestContext ctx = new ManualRequestContext(new AnonymousUser(), requestContext)) {
- gitRefListener.onGitReferenceUpdated(newRefUpdateEvent());
- assertEquals(1, gitRefListener.getProcessedEvents());
+ gitRefListener().onGitReferenceUpdated(newRefUpdateEvent());
+ assertEquals(1, gitRefListener().getProcessedEvents());
}
}
@Test
public void shouldRetrieveChangeFromAnonymousContextWithoutAccountId() throws Exception {
+ String refPrefix = createChange().getChange().getId().toRefPrefix();
ReferenceUpdatedEventTest refUpdateWithoutAccountId =
new ReferenceUpdatedEventTest(
- project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE, null);
+ project, refPrefix, anOldObjectId, aNewObjectId, Type.CREATE, null);
try (ManualRequestContext ctx = new ManualRequestContext(new AnonymousUser(), requestContext)) {
- gitRefListener.onGitReferenceUpdated(refUpdateWithoutAccountId);
- assertEquals(1, gitRefListener.getProcessedEvents());
+ gitRefListener().onGitReferenceUpdated(refUpdateWithoutAccountId);
+ assertEquals(1, gitRefListener().getProcessedEvents());
}
}
- private ReferenceUpdatedEventTest newRefUpdateEvent() {
+ private GitRefListenerTest gitRefListener() {
+ return (GitRefListenerTest)
+ StreamSupport.stream(allRefUpdateListeners.entries().spliterator(), false)
+ .map(Extension::get)
+ .filter(listener -> GitRefListenerTest.class.isAssignableFrom(listener.getClass()))
+ .findFirst()
+ .get();
+ }
+
+ private ReferenceUpdatedEventTest newRefUpdateEvent() throws Exception {
+ String refPrefix = createChange().getChange().getId().toRefPrefix();
return new ReferenceUpdatedEventTest(
- project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE, admin.id());
+ project, refPrefix, anOldObjectId, aNewObjectId, Type.CREATE, admin.id());
}
}
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
index e3d4be4..75a9189 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
@@ -15,13 +15,17 @@
package com.vmware.gerrit.owners.common;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener.Event;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.owners.common.Accounts;
import com.googlesource.gerrit.owners.common.GitRefListener;
import com.googlesource.gerrit.owners.common.ReviewerManager;
@@ -29,6 +33,7 @@
import org.junit.Ignore;
@Ignore
+@Singleton
public class GitRefListenerTest extends GitRefListener {
int processedEvents = 0;
@@ -40,7 +45,8 @@
Accounts accounts,
ReviewerManager reviewerManager,
OneOffRequestContext oneOffReqCtx,
- Provider<CurrentUser> currentUserProvider) {
+ Provider<CurrentUser> currentUserProvider,
+ ChangeNotes.Factory notesFactory) {
super(
api,
patchListCache,
@@ -48,11 +54,12 @@
accounts,
reviewerManager,
oneOffReqCtx,
- currentUserProvider);
+ currentUserProvider,
+ notesFactory);
}
@Override
- public void processEvent(Repository repository, Event event) {
+ public void processEvent(Repository repository, Event event, Change.Id cId) {
processedEvents++;
}
