Manage change deletion performed by pull-replication
When a delete change is replicated through the pull-replication plugin,
the fetch-ref-replicated event contains the ref-name of the change /meta
with a FORCED|NO_CHANGE result and an inexistent ref on the repository.
Leverage the event in the FetchRefReplicatedEventHandler and delete also
the change from the index.
Depends-On: https://gerrit-review.googlesource.com/c/plugins/replication/+/480184?
Change-Id: I8567a583fc149eb630c72eb45084a5e7b423decb
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java
index d45f526..83076bf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java
@@ -21,21 +21,30 @@
import com.google.gerrit.server.config.GerritInstanceId;
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.EventListener;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.multisite.forwarder.Context;
import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent;
import com.googlesource.gerrit.plugins.replication.pull.ReplicationState;
+import java.io.IOException;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
public class FetchRefReplicatedEventHandler implements EventListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final GitRepositoryManager gitRepositoryManager;
private ChangeIndexer changeIndexer;
private final String instanceId;
@Inject
- FetchRefReplicatedEventHandler(ChangeIndexer changeIndexer, @GerritInstanceId String instanceId) {
+ FetchRefReplicatedEventHandler(
+ ChangeIndexer changeIndexer,
+ @GerritInstanceId String instanceId,
+ GitRepositoryManager gitRepositoryManager) {
this.changeIndexer = changeIndexer;
this.instanceId = instanceId;
+ this.gitRepositoryManager = gitRepositoryManager;
}
@Override
@@ -46,7 +55,15 @@
if (event instanceof FetchRefReplicatedEvent && isLocalEvent(event)) {
FetchRefReplicatedEvent fetchRefReplicatedEvent = (FetchRefReplicatedEvent) event;
- if (!RefNames.isNoteDbMetaRef(fetchRefReplicatedEvent.getRefName())
+ // This removal of the ':' prefix is needed because of Issue 426470297 in the
+ // pull-replication plugin
+ // that may leave the deleted refs notification as a ref-spec instead of the deleted
+ // ref-name.
+ String refName =
+ fetchRefReplicatedEvent.ref.startsWith(":")
+ ? fetchRefReplicatedEvent.ref.substring(1)
+ : fetchRefReplicatedEvent.ref;
+ if (!RefNames.isNoteDbMetaRef(refName)
|| !fetchRefReplicatedEvent
.getStatus()
.equals(ReplicationState.RefFetchResult.SUCCEEDED.toString())) {
@@ -54,18 +71,44 @@
}
Project.NameKey projectNameKey = fetchRefReplicatedEvent.getProjectNameKey();
- logger.atFine().log(
- "Indexing ref '%s' for project %s",
- fetchRefReplicatedEvent.getRefName(), projectNameKey.get());
- Change.Id changeId = Change.Id.fromRef(fetchRefReplicatedEvent.getRefName());
- if (changeId != null) {
- changeIndexer.index(projectNameKey, changeId);
- } else {
- logger.atWarning().log(
- "Couldn't get changeId from refName. Skipping indexing of change %s for project %s",
- fetchRefReplicatedEvent.getRefName(), projectNameKey.get());
+ Change.Id changeId = Change.Id.fromRef(refName);
+
+ try (Repository repo = gitRepositoryManager.openRepository(projectNameKey)) {
+ if (changeId != null) {
+ RefUpdate.Result fetchRefReplicatedEventResult =
+ fetchRefReplicatedEvent.getRefUpdateResult();
+ if ((RefUpdate.Result.FORCED.equals(fetchRefReplicatedEventResult)
+ || RefUpdate.Result.NO_CHANGE.equals(fetchRefReplicatedEventResult))
+ && repo.exactRef(refName) == null) {
+ logger.atInfo().log(
+ "Deleting change '%s' from index following the forced delete of '%s' on project"
+ + " %s",
+ changeId, refName, projectNameKey.get());
+ // NOTE: The change has been already deleted and we just don't know if it was a
+ // locally
+ // created change or imported, because the meta-data is not there anymore.
+ // In the future, it would be best to have that information passed to the event by the
+ // pull-replication plugin *before* the delete, and added to the
+ // FetchRefReplicatedEvent,
+ // so that multi-site can use it for computing the virtual change-id and assuring the
+ // delete
+ // of the imported changes as well.
+ changeIndexer.delete(changeId);
+ } else {
+ logger.atInfo().log(
+ "Indexing change '%s' following update of '%s' on project %s",
+ changeId, refName, projectNameKey.get());
+ changeIndexer.index(projectNameKey, changeId);
+ }
+ } else {
+ logger.atWarning().log(
+ "Couldn't get changeId from refName. Skipping indexing of change %s for project %s",
+ refName, projectNameKey.get());
+ }
}
}
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log("Unable to process event %s", event);
} finally {
Context.setForwardedEvent(isForwarded);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java
index 47b6072..ed2f958 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java
@@ -16,6 +16,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
@@ -23,12 +24,14 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.googlesource.gerrit.plugins.replication.pull.Context;
import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent;
import com.googlesource.gerrit.plugins.replication.pull.ReplicationState;
import com.googlesource.gerrit.plugins.replication.pull.ReplicationState.RefFetchResult;
import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.URIish;
import org.junit.Before;
import org.junit.Test;
@@ -36,20 +39,46 @@
public class FetchRefReplicatedEventHandlerTest {
private static final String LOCAL_INSTANCE_ID = "local-instance-id";
private static final String REMOTE_INSTANCE_ID = "remote-instance-id";
+ private static final Project.NameKey PROJECT_NAME_KEY = Project.nameKey("testProject");
private ChangeIndexer changeIndexerMock;
+ private GitRepositoryManager gitRepositoryManager;
+ private Repository repository;
private FetchRefReplicatedEventHandler fetchRefReplicatedEventHandler;
private static URIish sourceUri;
@Before
public void setUp() throws Exception {
changeIndexerMock = mock(ChangeIndexer.class);
+ gitRepositoryManager = mock(GitRepositoryManager.class);
+ repository = mock(Repository.class);
+ doReturn(repository).when(gitRepositoryManager).openRepository(any());
fetchRefReplicatedEventHandler =
- new FetchRefReplicatedEventHandler(changeIndexerMock, LOCAL_INSTANCE_ID);
+ new FetchRefReplicatedEventHandler(
+ changeIndexerMock, LOCAL_INSTANCE_ID, gitRepositoryManager);
sourceUri = new URIish("git://aSourceNode/testProject.git");
}
@Test
public void onEventShouldIndexExistingChange() {
+ String ref = "refs/changes/41/41/meta";
+ Change.Id changeId = Change.Id.fromRef(ref);
+ try {
+ Context.setLocalEvent(true);
+ fetchRefReplicatedEventHandler.onEvent(
+ newFetchRefReplicatedEvent(
+ PROJECT_NAME_KEY,
+ ref,
+ ReplicationState.RefFetchResult.SUCCEEDED,
+ LOCAL_INSTANCE_ID,
+ RefUpdate.Result.FAST_FORWARD));
+ verify(changeIndexerMock, times(1)).index(eq(PROJECT_NAME_KEY), eq(changeId));
+ } finally {
+ Context.unsetLocalEvent();
+ }
+ }
+
+ @Test
+ public void onEventShouldDeleteChangeFromIndex() {
Project.NameKey projectNameKey = Project.nameKey("testProject");
String ref = "refs/changes/41/41/meta";
Change.Id changeId = Change.Id.fromRef(ref);
@@ -57,8 +86,12 @@
Context.setLocalEvent(true);
fetchRefReplicatedEventHandler.onEvent(
newFetchRefReplicatedEvent(
- projectNameKey, ref, ReplicationState.RefFetchResult.SUCCEEDED, LOCAL_INSTANCE_ID));
- verify(changeIndexerMock, times(1)).index(eq(projectNameKey), eq(changeId));
+ projectNameKey,
+ ref,
+ ReplicationState.RefFetchResult.SUCCEEDED,
+ LOCAL_INSTANCE_ID,
+ RefUpdate.Result.FORCED));
+ verify(changeIndexerMock, times(1)).delete(eq(changeId));
} finally {
Context.unsetLocalEvent();
}
@@ -71,7 +104,11 @@
Change.Id changeId = Change.Id.fromRef(ref);
fetchRefReplicatedEventHandler.onEvent(
newFetchRefReplicatedEvent(
- projectNameKey, ref, ReplicationState.RefFetchResult.SUCCEEDED, REMOTE_INSTANCE_ID));
+ projectNameKey,
+ ref,
+ ReplicationState.RefFetchResult.SUCCEEDED,
+ REMOTE_INSTANCE_ID,
+ RefUpdate.Result.FAST_FORWARD));
verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId));
}
@@ -82,7 +119,11 @@
Change.Id changeId = Change.Id.fromRef(ref);
fetchRefReplicatedEventHandler.onEvent(
newFetchRefReplicatedEvent(
- projectNameKey, ref, ReplicationState.RefFetchResult.SUCCEEDED, LOCAL_INSTANCE_ID));
+ projectNameKey,
+ ref,
+ ReplicationState.RefFetchResult.SUCCEEDED,
+ LOCAL_INSTANCE_ID,
+ RefUpdate.Result.FAST_FORWARD));
verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId));
}
@@ -93,7 +134,8 @@
Project.nameKey("testProject"),
"invalidRef",
ReplicationState.RefFetchResult.SUCCEEDED,
- LOCAL_INSTANCE_ID));
+ LOCAL_INSTANCE_ID,
+ RefUpdate.Result.FAST_FORWARD));
verify(changeIndexerMock, never()).index(any(), any());
}
@@ -103,7 +145,11 @@
String ref = "refs/changes/41/41/meta";
fetchRefReplicatedEventHandler.onEvent(
newFetchRefReplicatedEvent(
- projectNameKey, ref, ReplicationState.RefFetchResult.FAILED, LOCAL_INSTANCE_ID));
+ projectNameKey,
+ ref,
+ ReplicationState.RefFetchResult.FAILED,
+ LOCAL_INSTANCE_ID,
+ RefUpdate.Result.FAST_FORWARD));
verify(changeIndexerMock, never()).index(any(), any());
}
@@ -113,15 +159,23 @@
String ref = "refs/changes/41/41/meta";
fetchRefReplicatedEventHandler.onEvent(
newFetchRefReplicatedEvent(
- projectNameKey, ref, ReplicationState.RefFetchResult.NOT_ATTEMPTED, LOCAL_INSTANCE_ID));
+ projectNameKey,
+ ref,
+ ReplicationState.RefFetchResult.NOT_ATTEMPTED,
+ LOCAL_INSTANCE_ID,
+ RefUpdate.Result.FAST_FORWARD));
verify(changeIndexerMock, never()).index(any(), any());
}
private FetchRefReplicatedEvent newFetchRefReplicatedEvent(
- Project.NameKey projectNameKey, String ref, RefFetchResult fetchResult, String instanceId) {
+ Project.NameKey projectNameKey,
+ String ref,
+ RefFetchResult fetchResult,
+ String instanceId,
+ RefUpdate.Result refUpdateResult) {
FetchRefReplicatedEvent event =
new FetchRefReplicatedEvent(
- projectNameKey.get(), ref, sourceUri, fetchResult, RefUpdate.Result.FAST_FORWARD);
+ projectNameKey.get(), ref, sourceUri, fetchResult, refUpdateResult);
event.instanceId = instanceId;
return event;
}
