Refactor FetchJob to an assisted injection factory FetchJob class is used in multiple places. Add guice assisted injection factory to allow code reuse and unit testing Bug: Issue 15636 Change-Id: I1544575c17b977188765e20f70ab7a9053dcd5b3
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 ebc36b3..3e51e03 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
@@ -42,6 +42,7 @@ import com.googlesource.gerrit.plugins.replication.ReplicationConfig; import com.googlesource.gerrit.plugins.replication.ReplicationFileBasedConfig; import com.googlesource.gerrit.plugins.replication.StartReplicationCapability; +import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob; import com.googlesource.gerrit.plugins.replication.pull.api.PullReplicationApiModule; import com.googlesource.gerrit.plugins.replication.pull.client.FetchApiClient; import com.googlesource.gerrit.plugins.replication.pull.client.FetchRestApiClient; @@ -74,7 +75,7 @@ bind(RevisionReader.class).in(Scopes.SINGLETON); bind(ApplyObject.class); - + install(new FactoryModuleBuilder().build(FetchJob.Factory.class)); install(new PullReplicationApiModule()); install(new FetchRefReplicatedEventModule());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java index 23b7018..2c9583a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
@@ -17,7 +17,6 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Strings; -import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.extensions.restapi.AuthException; @@ -32,6 +31,7 @@ import com.google.gerrit.server.project.ProjectResource; import com.google.inject.Inject; import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.Input; +import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob.Factory; import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException; import java.util.Optional; import java.util.concurrent.ExecutionException; @@ -42,17 +42,20 @@ private final WorkQueue workQueue; private final DynamicItem<UrlFormatter> urlFormatter; private final FetchPreconditions preConditions; + private final Factory fetchJobFactory; @Inject public FetchAction( FetchCommand command, WorkQueue workQueue, DynamicItem<UrlFormatter> urlFormatter, - FetchPreconditions preConditions) { + FetchPreconditions preConditions, + FetchJob.Factory fetchJobFactory) { this.command = command; this.workQueue = workQueue; this.urlFormatter = urlFormatter; this.preConditions = preConditions; + this.fetchJobFactory = fetchJobFactory; } public static class Input { @@ -101,7 +104,7 @@ @SuppressWarnings("unchecked") WorkQueue.Task<Void> task = (WorkQueue.Task<Void>) - workQueue.getDefaultQueue().submit(new FetchJob(command, project, input)); + workQueue.getDefaultQueue().submit(fetchJobFactory.create(project, input)); Optional<String> url = urlFormatter .get() @@ -110,32 +113,4 @@ checkState(url.isPresent()); return Response.accepted(url.get()); } - - public static class FetchJob implements Runnable { - private static final FluentLogger log = FluentLogger.forEnclosingClass(); - - private FetchCommand command; - private Project.NameKey project; - private FetchAction.Input input; - - public FetchJob(FetchCommand command, Project.NameKey project, FetchAction.Input input) { - this.command = command; - this.project = project; - this.input = input; - } - - @Override - public void run() { - try { - command.fetchAsync(project, input.label, input.refName); - } catch (InterruptedException - | ExecutionException - | RemoteConfigurationMissingException - | TimeoutException e) { - log.atSevere().withCause(e).log( - "Exception during the async fetch call for project %s, label %s and ref name %s", - project.get(), input.label, input.refName); - } - } - } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java new file mode 100644 index 0000000..f533734 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java
@@ -0,0 +1,57 @@ +// Copyright (C) 2022 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.api; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.entities.Project; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; +import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +public class FetchJob implements Runnable { + private static final FluentLogger log = FluentLogger.forEnclosingClass(); + + public interface Factory { + FetchJob create(Project.NameKey project, FetchAction.Input input); + } + + private FetchCommand command; + private Project.NameKey project; + private FetchAction.Input input; + + @Inject + public FetchJob( + FetchCommand command, @Assisted Project.NameKey project, @Assisted FetchAction.Input input) { + this.command = command; + this.project = project; + this.input = input; + } + + @Override + public void run() { + try { + command.fetchAsync(project, input.label, input.refName); + } catch (InterruptedException + | ExecutionException + | RemoteConfigurationMissingException + | TimeoutException e) { + log.atSevere().withCause(e).log( + "Exception during the async fetch call for project %s, label %s and ref name %s", + project.get(), input.label, input.refName); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java index 4360fcf..76e3cae 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java
@@ -32,8 +32,8 @@ import com.google.inject.Inject; import com.googlesource.gerrit.plugins.replication.pull.FetchOne; import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction; -import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.FetchJob; -import com.googlesource.gerrit.plugins.replication.pull.api.FetchCommand; +import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob; +import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob.Factory; import com.googlesource.gerrit.plugins.replication.pull.api.ProjectInitializationAction; import org.eclipse.jgit.lib.ObjectId; @@ -41,20 +41,21 @@ private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private String instanceId; - private FetchCommand fetchCommand; private WorkQueue workQueue; private ProjectInitializationAction projectInitializationAction; + private Factory fetchJobFactory; + @Inject public StreamEventListener( @Nullable @GerritInstanceId String instanceId, - FetchCommand command, ProjectInitializationAction projectInitializationAction, - WorkQueue workQueue) { + WorkQueue workQueue, + FetchJob.Factory fetchJobFactory) { this.instanceId = instanceId; - this.fetchCommand = command; this.projectInitializationAction = projectInitializationAction; this.workQueue = workQueue; + this.fetchJobFactory = fetchJobFactory; requireNonNull( Strings.emptyToNull(this.instanceId), "gerrit.instanceId cannot be null or empty"); @@ -97,7 +98,7 @@ FetchAction.Input input = new FetchAction.Input(); input.refName = refName; input.label = sourceInstanceId; - workQueue.getDefaultQueue().submit(new FetchJob(fetchCommand, projectNameKey, input)); + workQueue.getDefaultQueue().submit(fetchJobFactory.create(projectNameKey, input)); } private String getProjectRepositoryName(ProjectCreatedEvent projectCreatedEvent) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java index fb7f3d1..8fd4b78 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
@@ -55,6 +55,8 @@ int taskId = 1234; @Mock FetchCommand fetchCommand; + @Mock FetchJob fetchJob; + @Mock FetchJob.Factory fetchJobFactory; @Mock ProjectResource projectResource; @Mock WorkQueue workQueue; @Mock ScheduledExecutorService exceutorService; @@ -65,6 +67,7 @@ @Before public void setup() { + when(fetchJobFactory.create(any(), any())).thenReturn(fetchJob); when(workQueue.getDefaultQueue()).thenReturn(exceutorService); when(urlFormatter.getRestUrl(anyString())).thenReturn(Optional.of(location)); when(exceutorService.submit(any(Runnable.class))) @@ -79,7 +82,9 @@ when(task.getTaskId()).thenReturn(taskId); when(preConditions.canCallFetchApi()).thenReturn(true); - fetchAction = new FetchAction(fetchCommand, workQueue, urlFormatterDynamicItem, preConditions); + fetchAction = + new FetchAction( + fetchCommand, workQueue, urlFormatterDynamicItem, preConditions, fetchJobFactory); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java index 8f80883..655575f 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java
@@ -14,12 +14,14 @@ package com.googlesource.gerrit.plugins.replication.pull.event; +import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; 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.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.server.data.RefUpdateAttribute; @@ -28,36 +30,44 @@ import com.google.gerrit.server.events.RefUpdatedEvent; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.permissions.PermissionBackendException; -import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.FetchJob; -import com.googlesource.gerrit.plugins.replication.pull.api.FetchCommand; +import com.googlesource.gerrit.plugins.replication.pull.FetchOne; +import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.Input; +import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob; import com.googlesource.gerrit.plugins.replication.pull.api.ProjectInitializationAction; import java.util.concurrent.ScheduledExecutorService; import org.eclipse.jgit.lib.ObjectId; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) public class StreamEventListenerTest { + private static final String TEST_REF_NAME = "refs/changes/01/1/1"; private static final String TEST_PROJECT = "test-project"; private static final String INSTANCE_ID = "node_instance_id"; private static final String REMOTE_INSTANCE_ID = "remote_node_instance_id"; - @Mock private FetchCommand fetchCommand; @Mock private ProjectInitializationAction projectInitializationAction; @Mock private WorkQueue workQueue; @Mock private ScheduledExecutorService executor; + @Mock private FetchJob fetchJob; + @Mock private FetchJob.Factory fetchJobFactory; + @Captor ArgumentCaptor<Input> inputCaptor; private StreamEventListener objectUnderTest; @Before public void setup() { when(workQueue.getDefaultQueue()).thenReturn(executor); + when(fetchJobFactory.create(eq(Project.nameKey(TEST_PROJECT)), any())).thenReturn(fetchJob); objectUnderTest = - new StreamEventListener(INSTANCE_ID, fetchCommand, projectInitializationAction, workQueue); + new StreamEventListener( + INSTANCE_ID, projectInitializationAction, workQueue, fetchJobFactory); } @Test @@ -89,7 +99,7 @@ public void shouldScheduleFetchJobForRefUpdateEvent() { RefUpdatedEvent event = new RefUpdatedEvent(); RefUpdateAttribute refUpdate = new RefUpdateAttribute(); - refUpdate.refName = "refs/changes/01/1/1"; + refUpdate.refName = TEST_REF_NAME; refUpdate.project = TEST_PROJECT; event.instanceId = REMOTE_INSTANCE_ID; @@ -97,6 +107,12 @@ objectUnderTest.onEvent(event); + verify(fetchJobFactory).create(eq(Project.nameKey(TEST_PROJECT)), inputCaptor.capture()); + + Input input = inputCaptor.getValue(); + assertThat(input.label).isEqualTo(REMOTE_INSTANCE_ID); + assertThat(input.refName).isEqualTo(TEST_REF_NAME); + verify(executor).submit(any(FetchJob.class)); } @@ -109,8 +125,7 @@ objectUnderTest.onEvent(event); - verify(projectInitializationAction, times(1)) - .initProject(String.format("%s.git", TEST_PROJECT)); + verify(projectInitializationAction).initProject(String.format("%s.git", TEST_PROJECT)); } @Test @@ -121,6 +136,12 @@ objectUnderTest.onEvent(event); + verify(fetchJobFactory).create(eq(Project.nameKey(TEST_PROJECT)), inputCaptor.capture()); + + Input input = inputCaptor.getValue(); + assertThat(input.label).isEqualTo(REMOTE_INSTANCE_ID); + assertThat(input.refName).isEqualTo(FetchOne.ALL_REFS); + verify(executor).submit(any(FetchJob.class)); } }