Merge branch 'stable-3.4'
* stable-3.4:
Inject ProjectVersionRefUpdate as Provider<>
Fix exception when consuming index events of deleted changes
Change-Id: I1982d9e21485e2d47cddfbc880b0ad5453d20ed1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriber.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriber.java
index 274c34f..480fc50 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriber.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriber.java
@@ -15,7 +15,9 @@
package com.googlesource.gerrit.plugins.multisite.consumer;
import com.gerritforge.gerrit.globalrefdb.validation.ProjectsFilter;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.config.GerritInstanceId;
import com.google.gerrit.server.events.Event;
import com.google.inject.Inject;
@@ -26,10 +28,12 @@
import com.googlesource.gerrit.plugins.multisite.forwarder.events.EventTopic;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent;
import com.googlesource.gerrit.plugins.multisite.forwarder.router.IndexEventRouter;
+import java.util.Optional;
@Singleton
public class IndexEventSubscriber extends AbstractSubcriber {
private final ProjectsFilter projectsFilter;
+ private final ChangeFinder changeFinder;
@Inject
public IndexEventSubscriber(
@@ -39,9 +43,11 @@
MessageLogger msgLog,
SubscriberMetrics subscriberMetrics,
Configuration cfg,
- ProjectsFilter projectsFilter) {
+ ProjectsFilter projectsFilter,
+ ChangeFinder changeFinder) {
super(eventRouter, droppedEventListeners, instanceId, msgLog, subscriberMetrics, cfg);
this.projectsFilter = projectsFilter;
+ this.changeFinder = changeFinder;
}
@Override
@@ -52,11 +58,24 @@
@Override
protected Boolean shouldConsumeEvent(Event event) {
if (event instanceof ChangeIndexEvent) {
- return projectsFilter.matches(((ChangeIndexEvent) event).projectName);
+ ChangeIndexEvent changeIndexEvent = (ChangeIndexEvent) event;
+ String projectName = changeIndexEvent.projectName;
+ if (isDeletedChangeWithEmptyProject(changeIndexEvent)) {
+ projectName = findProjectFromChangeId(changeIndexEvent.changeId).orElse(projectName);
+ }
+ return projectsFilter.matches(projectName);
}
if (event instanceof ProjectIndexEvent) {
return projectsFilter.matches(((ProjectIndexEvent) event).projectName);
}
return true;
}
+
+ private boolean isDeletedChangeWithEmptyProject(ChangeIndexEvent changeIndexEvent) {
+ return changeIndexEvent.deleted && changeIndexEvent.projectName.isEmpty();
+ }
+
+ private Optional<String> findProjectFromChangeId(int changeId) {
+ return changeFinder.findOne(Change.id(changeId)).map(c -> c.getChange().getProject().get());
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java
index 7ce4606..40168c5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatus.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger;
@@ -60,14 +61,14 @@
private final Map<String, Long> localVersionPerProject = new HashMap<>();
private final Cache<String, Long> cache;
- private final ProjectVersionRefUpdate projectVersionRefUpdate;
+ private final Provider<ProjectVersionRefUpdate> projectVersionRefUpdate;
private final ProjectVersionLogger verLogger;
private final ProjectCache projectCache;
@Inject
public ReplicationStatus(
@Named(REPLICATION_STATUS_CACHE) Cache<String, Long> cache,
- ProjectVersionRefUpdate projectVersionRefUpdate,
+ Provider<ProjectVersionRefUpdate> projectVersionRefUpdate,
ProjectVersionLogger verLogger,
ProjectCache projectCache) {
this.cache = cache;
@@ -98,8 +99,9 @@
public void updateReplicationLag(Project.NameKey projectName) {
Optional<Long> remoteVersion =
- projectVersionRefUpdate.getProjectRemoteVersion(projectName.get());
- Optional<Long> localVersion = projectVersionRefUpdate.getProjectLocalVersion(projectName.get());
+ projectVersionRefUpdate.get().getProjectRemoteVersion(projectName.get());
+ Optional<Long> localVersion =
+ projectVersionRefUpdate.get().getProjectLocalVersion(projectName.get());
if (remoteVersion.isPresent() && localVersion.isPresent()) {
long lag = remoteVersion.get() - localVersion.get();
@@ -120,7 +122,8 @@
}
void removeProjectFromReplicationLagMetrics(Project.NameKey projectName) {
- Optional<Long> localVersion = projectVersionRefUpdate.getProjectLocalVersion(projectName.get());
+ Optional<Long> localVersion =
+ projectVersionRefUpdate.get().getProjectLocalVersion(projectName.get());
if (!localVersion.isPresent() && replicationStatusPerProject.containsKey(projectName.get())) {
cache.invalidate(projectName.get());
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java
index 70f811b..0e63528 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java
@@ -14,14 +14,23 @@
package com.googlesource.gerrit.plugins.multisite.consumer;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.events.Event;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.googlesource.gerrit.plugins.multisite.forwarder.CacheNotFoundException;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.AccountIndexEvent;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
@@ -31,11 +40,16 @@
import com.googlesource.gerrit.plugins.multisite.forwarder.router.IndexEventRouter;
import java.io.IOException;
import java.util.List;
+import java.util.Optional;
import org.junit.Test;
+import org.mockito.Mock;
public class IndexEventSubscriberTest extends AbstractSubscriberTestBase {
private static final boolean DELETED = false;
private static final int CHANGE_ID = 1;
+ private static final String EMPTY_PROJECT_NAME = "";
+
+ @Mock protected ChangeFinder changeFinderMock;
@SuppressWarnings("unchecked")
@Test
@@ -50,6 +64,38 @@
verify(droppedEventListeners, never()).onEventDropped(event);
}
+ @SuppressWarnings("unchecked")
+ @Test
+ public void shouldConsumeDeleteChangeIndexEventWithEmptyProjectNameWhenFound()
+ throws IOException, PermissionBackendException, CacheNotFoundException {
+ ChangeIndexEvent event = new ChangeIndexEvent(EMPTY_PROJECT_NAME, CHANGE_ID, true, INSTANCE_ID);
+
+ ChangeNotes changeNotesMock = mock(ChangeNotes.class);
+ when(changeNotesMock.getChange()).thenReturn(newChange());
+ when(changeFinderMock.findOne(any(Change.Id.class))).thenReturn(Optional.of(changeNotesMock));
+ when(projectsFilter.matches(PROJECT_NAME)).thenReturn(true);
+
+ objectUnderTest.getConsumer().accept(event);
+
+ verify(projectsFilter, times(1)).matches(PROJECT_NAME);
+ verify(eventRouter, times(1)).route(event);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void shouldNOTConsumeDeleteChangeIndexEventWithEmptyProjectNameWhenNotFound()
+ throws IOException, PermissionBackendException, CacheNotFoundException {
+ ChangeIndexEvent event = new ChangeIndexEvent("", CHANGE_ID, true, INSTANCE_ID);
+
+ when(changeFinderMock.findOne(any(Change.Id.class))).thenReturn(Optional.empty());
+ when(projectsFilter.matches(EMPTY_PROJECT_NAME)).thenReturn(false);
+
+ objectUnderTest.getConsumer().accept(event);
+
+ verify(projectsFilter, times(1)).matches(EMPTY_PROJECT_NAME);
+ verify(eventRouter, never()).route(event);
+ }
+
@SuppressWarnings("rawtypes")
@Override
protected ForwardedEventRouter eventRouter() {
@@ -72,6 +118,16 @@
msgLog,
subscriberMetrics,
cfg,
- projectsFilter);
+ projectsFilter,
+ changeFinderMock);
+ }
+
+ private Change newChange() {
+ return new Change(
+ Change.key(Integer.toString(CHANGE_ID)),
+ Change.id(CHANGE_ID),
+ Account.id(9999),
+ BranchNameKey.create(Project.nameKey(PROJECT_NAME), "refs/heads/master"),
+ TimeUtil.nowTs());
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java
index aec9404..a2a5e73 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/ReplicationStatusTest.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.multisite.consumer;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -24,6 +25,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.events.ProjectDeletedListener;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger;
import com.googlesource.gerrit.plugins.multisite.validation.ProjectVersionRefUpdate;
import java.util.Optional;
@@ -38,6 +40,7 @@
@Mock private ProjectVersionLogger verLogger;
@Mock private ProjectCache projectCache;
+ @Mock private Provider<ProjectVersionRefUpdate> projectVersionRefUpdateProvider;
@Mock private ProjectVersionRefUpdate projectVersionRefUpdate;
private ReplicationStatus objectUnderTest;
private Cache<String, Long> replicationStatusCache;
@@ -50,7 +53,8 @@
replicationStatusCache = CacheBuilder.newBuilder().build();
objectUnderTest =
new ReplicationStatus(
- replicationStatusCache, projectVersionRefUpdate, verLogger, projectCache);
+ replicationStatusCache, projectVersionRefUpdateProvider, verLogger, projectCache);
+ lenient().when(projectVersionRefUpdateProvider.get()).thenReturn(projectVersionRefUpdate);
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java
index ab1ef80..8ed4a14 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.RefUpdatedEvent;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger;
import com.googlesource.gerrit.plugins.multisite.validation.ProjectVersionRefUpdate;
import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationSucceededEvent;
@@ -48,6 +49,7 @@
@Mock private MetricMaker metricMaker;
@Mock private ProjectVersionLogger verLogger;
@Mock private ProjectCache projectCache;
+ @Mock private Provider<ProjectVersionRefUpdate> projectVersionRefUpdateProvider;
@Mock private ProjectVersionRefUpdate projectVersionRefUpdate;
private SubscriberMetrics metrics;
private ReplicationStatus replicationStatus;
@@ -56,8 +58,12 @@
public void setup() throws Exception {
replicationStatus =
new ReplicationStatus(
- CacheBuilder.newBuilder().build(), projectVersionRefUpdate, verLogger, projectCache);
+ CacheBuilder.newBuilder().build(),
+ projectVersionRefUpdateProvider,
+ verLogger,
+ projectCache);
metrics = new SubscriberMetrics(metricMaker, replicationStatus);
+ when(projectVersionRefUpdateProvider.get()).thenReturn(projectVersionRefUpdate);
}
@Test
