Remove change indexing on ref-replicated event Move the FetchRefReplicatedEventHandler and the associated module regitration and test to the multi-site plugin. The multi-site plugin has always been responsible for propagating the Gerrit indexes events across the sites. However, since I5373bb8149, the pull-replication took the direction of also managing part of the reindexing. The propagation of indexing events is a lot more complex that just triggering a single change index action and need visibility of all the other machinery needed by a multi-site setup. This is the proper solution to the Issue 16934 which has been resolved with a workaround in I8ebec4e and I0a28e302ee. Bug: Issue 16934 Change-Id: Icd41a6f074088acf0d4c36982eb7524706e0fd9b
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java index 08a402e..c622830 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
@@ -50,7 +50,6 @@ import com.googlesource.gerrit.plugins.replication.pull.client.HttpClient; import com.googlesource.gerrit.plugins.replication.pull.client.SourceHttpClient; import com.googlesource.gerrit.plugins.replication.pull.event.EventsBrokerConsumerModule; -import com.googlesource.gerrit.plugins.replication.pull.event.FetchRefReplicatedEventModule; import com.googlesource.gerrit.plugins.replication.pull.event.StreamEventModule; import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject; import java.io.File; @@ -82,8 +81,6 @@ install(new FactoryModuleBuilder().build(FetchJob.Factory.class)); install(new PullReplicationApiModule()); - install(new FetchRefReplicatedEventModule()); - install( new FactoryModuleBuilder() .implement(HttpClient.class, SourceHttpClient.class)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventHandler.java deleted file mode 100644 index a618e16..0000000 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventHandler.java +++ /dev/null
@@ -1,67 +0,0 @@ -// Copyright (C) 2021 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.replication.pull.event; - -import com.google.common.flogger.FluentLogger; -import com.google.gerrit.entities.Change; -import com.google.gerrit.entities.Project; -import com.google.gerrit.entities.RefNames; -import com.google.gerrit.server.events.Event; -import com.google.gerrit.server.events.EventListener; -import com.google.gerrit.server.index.change.ChangeIndexer; -import com.google.inject.Inject; -import com.googlesource.gerrit.plugins.replication.pull.Context; -import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; -import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; - -public class FetchRefReplicatedEventHandler implements EventListener { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private ChangeIndexer changeIndexer; - - @Inject - FetchRefReplicatedEventHandler(ChangeIndexer changeIndexer) { - this.changeIndexer = changeIndexer; - } - - @Override - public void onEvent(Event event) { - if (event instanceof FetchRefReplicatedEvent && isLocalEvent()) { - FetchRefReplicatedEvent fetchRefReplicatedEvent = (FetchRefReplicatedEvent) event; - if (!RefNames.isNoteDbMetaRef(fetchRefReplicatedEvent.getRefName()) - || !fetchRefReplicatedEvent - .getStatus() - .equals(ReplicationState.RefFetchResult.SUCCEEDED.toString())) { - return; - } - - 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()); - } - } - } - - private boolean isLocalEvent() { - return Context.isLocalEvent(); - } -}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventModule.java deleted file mode 100644 index 675563a..0000000 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventModule.java +++ /dev/null
@@ -1,27 +0,0 @@ -// Copyright (C) 2021 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.replication.pull.event; - -import com.google.gerrit.extensions.registration.DynamicSet; -import com.google.gerrit.lifecycle.LifecycleModule; -import com.google.gerrit.server.events.EventListener; - -public class FetchRefReplicatedEventModule extends LifecycleModule { - - @Override - protected void configure() { - DynamicSet.bind(binder(), EventListener.class).to(FetchRefReplicatedEventHandler.class); - } -}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventHandlerTest.java deleted file mode 100644 index 81a4fc0..0000000 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/FetchRefReplicatedEventHandlerTest.java +++ /dev/null
@@ -1,136 +0,0 @@ -// Copyright (C) 2021 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.replication.pull.event; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -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 com.google.gerrit.entities.Change; -import com.google.gerrit.entities.Project; -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 org.eclipse.jgit.lib.RefUpdate; -import org.eclipse.jgit.transport.URIish; -import org.junit.Before; -import org.junit.Test; - -public class FetchRefReplicatedEventHandlerTest { - private ChangeIndexer changeIndexerMock; - private FetchRefReplicatedEventHandler fetchRefReplicatedEventHandler; - private static URIish sourceUri; - - @Before - public void setUp() throws Exception { - changeIndexerMock = mock(ChangeIndexer.class); - fetchRefReplicatedEventHandler = new FetchRefReplicatedEventHandler(changeIndexerMock); - sourceUri = new URIish("git://aSourceNode/testProject.git"); - } - - @Test - public void onEventShouldIndexExistingChange() { - Project.NameKey projectNameKey = Project.nameKey("testProject"); - String ref = "refs/changes/41/41/meta"; - Change.Id changeId = Change.Id.fromRef(ref); - try { - Context.setLocalEvent(true); - fetchRefReplicatedEventHandler.onEvent( - new FetchRefReplicatedEvent( - projectNameKey.get(), - ref, - sourceUri, - ReplicationState.RefFetchResult.SUCCEEDED, - RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, times(1)).index(eq(projectNameKey), eq(changeId)); - } finally { - Context.unsetLocalEvent(); - } - } - - @Test - public void onEventShouldNotIndexIfNotLocalEvent() { - Project.NameKey projectNameKey = Project.nameKey("testProject"); - String ref = "refs/changes/41/41/meta"; - Change.Id changeId = Change.Id.fromRef(ref); - fetchRefReplicatedEventHandler.onEvent( - new FetchRefReplicatedEvent( - projectNameKey.get(), - ref, - sourceUri, - ReplicationState.RefFetchResult.SUCCEEDED, - RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId)); - } - - @Test - public void onEventShouldIndexOnlyMetaRef() { - Project.NameKey projectNameKey = Project.nameKey("testProject"); - String ref = "refs/changes/41/41/1"; - Change.Id changeId = Change.Id.fromRef(ref); - fetchRefReplicatedEventHandler.onEvent( - new FetchRefReplicatedEvent( - projectNameKey.get(), - ref, - sourceUri, - ReplicationState.RefFetchResult.SUCCEEDED, - RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId)); - } - - @Test - public void onEventShouldNotIndexMissingChange() { - fetchRefReplicatedEventHandler.onEvent( - new FetchRefReplicatedEvent( - Project.nameKey("testProject").get(), - "invalidRef", - sourceUri, - ReplicationState.RefFetchResult.SUCCEEDED, - RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, never()).index(any(), any()); - } - - @Test - public void onEventShouldNotIndexFailingChange() { - Project.NameKey projectNameKey = Project.nameKey("testProject"); - String ref = "refs/changes/41/41/meta"; - fetchRefReplicatedEventHandler.onEvent( - new FetchRefReplicatedEvent( - projectNameKey.get(), - ref, - sourceUri, - ReplicationState.RefFetchResult.FAILED, - RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, never()).index(any(), any()); - } - - @Test - public void onEventShouldNotIndexNotAttemptedChange() { - Project.NameKey projectNameKey = Project.nameKey("testProject"); - String ref = "refs/changes/41/41/meta"; - fetchRefReplicatedEventHandler.onEvent( - new FetchRefReplicatedEvent( - projectNameKey.get(), - ref, - sourceUri, - ReplicationState.RefFetchResult.NOT_ATTEMPTED, - RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, never()).index(any(), any()); - } -}