Merge branch 'stable-3.5' into stable-3.6 * stable-3.5: Increase FetchOne coverage with unit tests Display the in-flight taskId when a runway conflict is detected Restore the fetching of all refs with empty delta Allow to filter out refs from the fetch-replication deltas Do not accumulate replication events with retrying tasks Add replication taskId during the execution of the fetch Allow FetchOne to retry the failed ref only Change-Id: Idf86be68b3678019a85e300e7174786bbd176b15
diff --git a/example-setup/broker/Dockerfile b/example-setup/broker/Dockerfile index 1b3038c..566e839 100644 --- a/example-setup/broker/Dockerfile +++ b/example-setup/broker/Dockerfile
@@ -1,4 +1,4 @@ -FROM gerritcodereview/gerrit:3.5.5-almalinux8 +FROM gerritcodereview/gerrit:3.6.3-almalinux8 USER root
diff --git a/example-setup/http/Dockerfile b/example-setup/http/Dockerfile index 552a114..b966b5f 100644 --- a/example-setup/http/Dockerfile +++ b/example-setup/http/Dockerfile
@@ -1,4 +1,4 @@ -FROM gerritcodereview/gerrit:3.5.5-almalinux8 +FROM gerritcodereview/gerrit:3.6.3-almalinux8 USER root
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl index 5d2ba13..3bf728d 100644 --- a/external_plugin_deps.bzl +++ b/external_plugin_deps.bzl
@@ -3,6 +3,6 @@ def external_plugin_deps(): maven_jar( name = "events-broker", - artifact = "com.gerritforge:events-broker:3.5.0.1", - sha1 = "af192a8bceaf7ff54d19356f9bfe1f1e83634b40", + artifact = "com.gerritforge:events-broker:3.6.0-rc3", + sha1 = "cb398afa4f76367be5c62b99a7ffce74ae1d3d8b", )
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 cd19be4..08a402e 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
@@ -19,7 +19,7 @@ import com.google.common.eventbus.EventBus; import com.google.gerrit.extensions.annotations.Exports; import com.google.gerrit.extensions.config.CapabilityDefinition; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.extensions.events.HeadUpdatedListener; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.events.ProjectDeletedListener; @@ -123,7 +123,7 @@ .annotatedWith(UniqueAnnotations.create()) .to(ReplicationQueue.class); - DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ReplicationQueue.class); + DynamicSet.bind(binder(), GitBatchRefUpdateListener.class).to(ReplicationQueue.class); bind(ConfigParser.class).to(SourceConfigParser.class).in(Scopes.SINGLETON);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java index 380ac8e..990c82c 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java
@@ -20,7 +20,7 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; import com.google.gerrit.entities.RefNames; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.extensions.events.HeadUpdatedListener; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.events.ProjectDeletedListener; @@ -65,7 +65,7 @@ public class ReplicationQueue implements ObservableQueue, LifecycleListener, - GitReferenceUpdatedListener, + GitBatchRefUpdateListener, ProjectDeletedListener, HeadUpdatedListener { @@ -147,21 +147,24 @@ } @Override - public void onGitReferenceUpdated(GitReferenceUpdatedListener.Event event) { - if (isRefToBeReplicated(event.getRefName())) { - repLog.info( - "Ref event received: {} on project {}:{} - {} => {}", - refUpdateType(event), - event.getProjectName(), - event.getRefName(), - event.getOldObjectId(), - event.getNewObjectId()); - fire( - event.getProjectName(), - ObjectId.fromString(event.getNewObjectId()), - event.getRefName(), - event.isDelete()); - } + public void onGitBatchRefUpdate(GitBatchRefUpdateListener.Event event) { + event.getUpdatedRefs().stream() + .sorted(ReplicationQueue::sortByMetaRefAsLast) + .forEachOrdered( + updateRef -> { + String refName = updateRef.getRefName(); + + if (isRefToBeReplicated(refName)) { + repLog.info( + "Ref event received: {} on project {}:{} - {} => {}", + refUpdateType(updateRef), + event.getProjectName(), + refName, + updateRef.getOldObjectId(), + updateRef.getNewObjectId()); + fire(ReferenceUpdatedEvent.from(event.getProjectName(), updateRef)); + } + }); } @Override @@ -174,11 +177,17 @@ source.getApis().forEach(apiUrl -> source.scheduleDeleteProject(apiUrl, project))); } - private static String refUpdateType(GitReferenceUpdatedListener.Event event) { - String forcedPrefix = event.isNonFastForward() ? "FORCED " : " "; - if (event.isCreate()) { + private static int sortByMetaRefAsLast(UpdatedRef a, @SuppressWarnings("unused") UpdatedRef b) { + repLog.info("sortByMetaRefAsLast(" + a.getRefName() + " <=> " + b.getRefName()); + return Boolean.compare( + RefNames.isNoteDbMetaRef(a.getRefName()), RefNames.isNoteDbMetaRef(b.getRefName())); + } + + private static String refUpdateType(UpdatedRef updateRef) { + String forcedPrefix = updateRef.isNonFastForward() ? "FORCED " : " "; + if (updateRef.isCreate()) { return forcedPrefix + "CREATE"; - } else if (event.isDelete()) { + } else if (updateRef.isDelete()) { return forcedPrefix + "DELETE"; } else { return forcedPrefix + "UPDATE"; @@ -189,34 +198,39 @@ return !refsFilter.match(refName); } - private void fire(String projectName, ObjectId objectId, String refName, boolean isDelete) { + private void fire(ReferenceUpdatedEvent event) { ReplicationState state = new ReplicationState(new GitUpdateProcessing(dispatcher.get())); - fire(Project.nameKey(projectName), objectId, refName, isDelete, state); + fire(event, state); state.markAllFetchTasksScheduled(); } - private void fire( - Project.NameKey project, - ObjectId objectId, - String refName, - boolean isDelete, - ReplicationState state) { + private void fire(ReferenceUpdatedEvent event, ReplicationState state) { if (!running) { stateLog.warn( "Replication plugin did not finish startup before event, event replication is postponed", state); - beforeStartupEventsQueue.add( - ReferenceUpdatedEvent.create(project.get(), refName, objectId, isDelete)); + beforeStartupEventsQueue.add(event); return; } ForkJoinPool fetchCallsPool = null; try { - fetchCallsPool = new ForkJoinPool(sources.get().getAll().size()); + List<Source> allSources = sources.get().getAll(); + int numSources = allSources.size(); + if (numSources == 0) { + repLog.debug("No replication sources configured -> skipping fetch"); + return; + } + fetchCallsPool = new ForkJoinPool(numSources); final Consumer<Source> callFunction = - callFunction(project, objectId, refName, isDelete, state); + callFunction( + Project.nameKey(event.projectName()), + event.objectId(), + event.refName(), + event.isDelete(), + state); fetchCallsPool - .submit(() -> sources.get().getAll().parallelStream().forEach(callFunction)) + .submit(() -> allSources.parallelStream().forEach(callFunction)) .get(fetchCallsTimeout, TimeUnit.MILLISECONDS); } catch (InterruptedException | ExecutionException | TimeoutException e) { stateLog.error( @@ -491,7 +505,7 @@ String eventKey = String.format("%s:%s", event.projectName(), event.refName()); if (!eventsReplayed.contains(eventKey)) { repLog.info("Firing pending task {}", event); - fire(event.projectName(), event.objectId(), event.refName(), event.isDelete()); + fire(event); eventsReplayed.add(eventKey); } } @@ -517,6 +531,14 @@ projectName, refName, objectId, isDelete); } + static ReferenceUpdatedEvent from(String projectName, UpdatedRef updateRef) { + return ReferenceUpdatedEvent.create( + projectName, + updateRef.getRefName(), + ObjectId.fromString(updateRef.getNewObjectId()), + updateRef.isDelete()); + } + public abstract String projectName(); public abstract String refName();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java index f897012..e49c8b6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java
@@ -35,7 +35,6 @@ import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; import com.googlesource.gerrit.plugins.replication.pull.Source; import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection; -import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject; import com.googlesource.gerrit.plugins.replication.pull.fetch.RefUpdateState; import java.io.IOException; import java.util.Optional; @@ -49,7 +48,6 @@ private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final PullReplicationStateLogger fetchStateLog; - private final ApplyObject applyObject; private final DynamicItem<EventDispatcher> eventDispatcher; private final ProjectCache projectCache; private final SourcesCollection sourcesCollection; @@ -61,13 +59,11 @@ PullReplicationStateLogger fetchStateLog, ProjectCache projectCache, SourcesCollection sourcesCollection, - ApplyObject applyObject, PermissionBackend permissionBackend, DynamicItem<EventDispatcher> eventDispatcher, LocalGitRepositoryManagerProvider gitManagerProvider) { this.fetchStateLog = fetchStateLog; this.projectCache = projectCache; - this.applyObject = applyObject; this.eventDispatcher = eventDispatcher; this.sourcesCollection = sourcesCollection; this.permissionBackend = permissionBackend;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetchValidator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetchValidator.java index 9a10898..434c0f0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetchValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/CGitFetchValidator.java
@@ -35,7 +35,7 @@ @Override public Void visit(AssistedInjectBinding<? extends FetchFactory> binding) { - TypeLiteral<CGitFetch> nativeGitFetchType = new TypeLiteral<CGitFetch>() {}; + TypeLiteral<CGitFetch> nativeGitFetchType = new TypeLiteral<>() {}; for (AssistedMethod method : binding.getAssistedMethods()) { if (method.getImplementationType().equals(nativeGitFetchType)) { String[] command = new String[] {"git", "--version"};
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java index acb68cf..0fa89b5 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/PermanentTransportException.java
@@ -14,7 +14,7 @@ package com.googlesource.gerrit.plugins.replication.pull.fetch; -import com.jcraft.jsch.JSchException; +import org.apache.sshd.common.SshException; import org.eclipse.jgit.errors.TransportException; public class PermanentTransportException extends TransportException { @@ -26,7 +26,8 @@ public static TransportException wrapIfPermanentTransportException(TransportException e) { Throwable cause = e.getCause(); - if (cause instanceof JSchException && cause.getMessage().startsWith("UnknownHostKey:")) { + if (cause instanceof SshException + && cause.getMessage().startsWith("Failed (UnsupportedCredentialItem) to execute:")) { return new PermanentTransportException("Terminal fetch failure", e); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FakeGitReferenceUpdatedEvent.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FakeGitReferenceUpdatedEvent.java index 43331dc..69549aa 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FakeGitReferenceUpdatedEvent.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FakeGitReferenceUpdatedEvent.java
@@ -17,10 +17,13 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.common.AccountInfo; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener.UpdatedRef; +import java.util.Set; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.transport.ReceiveCommand; -public class FakeGitReferenceUpdatedEvent implements GitReferenceUpdatedListener.Event { +public class FakeGitReferenceUpdatedEvent implements GitBatchRefUpdateListener.Event { private final String projectName; private final String ref; private final String oldObjectId; @@ -46,36 +49,6 @@ } @Override - public String getRefName() { - return ref; - } - - @Override - public String getOldObjectId() { - return oldObjectId; - } - - @Override - public String getNewObjectId() { - return newObjectId; - } - - @Override - public boolean isCreate() { - return type == ReceiveCommand.Type.CREATE; - } - - @Override - public boolean isDelete() { - return type == ReceiveCommand.Type.DELETE; - } - - @Override - public boolean isNonFastForward() { - return type == ReceiveCommand.Type.UPDATE_NONFASTFORWARD; - } - - @Override public AccountInfo getUpdater() { return null; } @@ -91,4 +64,46 @@ public NotifyHandling getNotify() { return NotifyHandling.ALL; } + + @Override + public Set<UpdatedRef> getUpdatedRefs() { + return Set.of( + new GitBatchRefUpdateListener.UpdatedRef() { + + @Override + public String getRefName() { + return ref; + } + + @Override + public String getOldObjectId() { + return ObjectId.zeroId().getName(); + } + + @Override + public String getNewObjectId() { + return newObjectId; + } + + @Override + public boolean isCreate() { + return type == ReceiveCommand.Type.CREATE; + } + + @Override + public boolean isDelete() { + return type == ReceiveCommand.Type.DELETE; + } + + @Override + public boolean isNonFastForward() { + return type == ReceiveCommand.Type.UPDATE_NONFASTFORWARD; + } + }); + } + + @Override + public Set<String> getRefNames() { + return Set.of(ref); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchGitUpdateProcessingTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchGitUpdateProcessingTest.java index 68044b4..5a26054 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchGitUpdateProcessingTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchGitUpdateProcessingTest.java
@@ -47,8 +47,7 @@ } @Test - public void headRefReplicatedInGitUpdateProcessing() - throws URISyntaxException, PermissionBackendException { + public void headRefReplicatedInGitUpdateProcessing() throws PermissionBackendException { FetchRefReplicatedEvent expectedEvent = new FetchRefReplicatedEvent( "someProject",
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java index 09a465c..fcb0702 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PermanentFailureExceptionTest.java
@@ -18,7 +18,7 @@ import com.googlesource.gerrit.plugins.replication.pull.fetch.InexistentRefTransportException; import com.googlesource.gerrit.plugins.replication.pull.fetch.PermanentTransportException; -import com.jcraft.jsch.JSchException; +import org.apache.sshd.common.SshException; import org.eclipse.jgit.errors.TransportException; import org.junit.Test; @@ -29,7 +29,9 @@ assertThat( PermanentTransportException.wrapIfPermanentTransportException( new TransportException( - "SSH error", new JSchException("UnknownHostKey: some.place")))) + "SSH error", + new SshException( + "Failed (UnsupportedCredentialItem) to execute: some.commands")))) .isInstanceOf(PermanentTransportException.class); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java index ee5876f..ff8265f 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
@@ -27,7 +27,7 @@ import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.projects.BranchInput; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; import com.googlesource.gerrit.plugins.replication.AutoReloadConfigDecorator; @@ -105,14 +105,14 @@ String sourceRef = pushResult.getPatchSet().refName(); ReplicationQueue pullReplicationQueue = getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, sourceRef, ObjectId.zeroId().getName(), sourceCommit.getId().getName(), ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null); @@ -141,14 +141,14 @@ RevCommit sourceCommit = pushResult.getCommit(); final String sourceRef = pushResult.getPatchSet().refName(); ReplicationQueue pullReplicationQueue = getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, sourceRef, ObjectId.zeroId().getName(), sourceCommit.getId().getName(), ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null); @@ -174,14 +174,14 @@ ReplicationQueue pullReplicationQueue = plugin.getSysInjector().getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, newBranch, ObjectId.zeroId().getName(), branchRevision, ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project); Repository sourceRepo = repoManager.openRepository(project)) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java index f3c62c5..6fd5cb3 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java
@@ -30,7 +30,7 @@ import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.projects.BranchInput; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.extensions.events.HeadUpdatedListener; import com.google.gerrit.extensions.events.ProjectDeletedListener; import com.google.gerrit.extensions.restapi.RestApiException; @@ -91,14 +91,14 @@ String sourceRef = pushResult.getPatchSet().refName(); ReplicationQueue pullReplicationQueue = getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, sourceRef, ObjectId.zeroId().getName(), sourceCommit.getId().getName(), ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null); @@ -124,14 +124,14 @@ ReplicationQueue pullReplicationQueue = plugin.getSysInjector().getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, newBranch, ObjectId.zeroId().getName(), branchRevision, ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project); Repository sourceRepo = repoManager.openRepository(project)) { @@ -167,14 +167,14 @@ ReplicationQueue pullReplicationQueue = plugin.getSysInjector().getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, newBranch, ObjectId.zeroId().getName(), branchRevision, ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, newBranch) != null); @@ -193,14 +193,14 @@ assertThat(pushedRefs).hasSize(1); assertThat(pushedRefs.iterator().next().getStatus()).isEqualTo(Status.OK); - GitReferenceUpdatedListener.Event forcedPushEvent = + GitBatchRefUpdateListener.Event forcedPushEvent = new FakeGitReferenceUpdatedEvent( project, newBranch, branchRevision, amendedCommit.getId().getName(), ReceiveCommand.Type.UPDATE_NONFASTFORWARD); - pullReplicationQueue.onGitReferenceUpdated(forcedPushEvent); + pullReplicationQueue.onGitBatchRefUpdate(forcedPushEvent); try (Repository repo = repoManager.openRepository(project); Repository sourceRepo = repoManager.openRepository(project)) { @@ -232,14 +232,14 @@ String sourceRef = pushResult.getPatchSet().refName(); ReplicationQueue pullReplicationQueue = getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, sourceRef, ObjectId.zeroId().getName(), sourceCommit.getId().getName(), ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null); @@ -273,14 +273,14 @@ ReplicationQueue pullReplicationQueue = plugin.getSysInjector().getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, newBranch, ObjectId.zeroId().getName(), branchRevision, ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project); Repository sourceRepo = repoManager.openRepository(project)) { @@ -336,7 +336,6 @@ BranchInput input = new BranchInput(); input.revision = master; gApi.projects().name(testProjectName).branch(newBranch).create(input); - String branchRevision = gApi.projects().name(testProjectName).branch(newBranch).get().revision; ReplicationQueue pullReplicationQueue = plugin.getSysInjector().getInstance(ReplicationQueue.class); @@ -365,14 +364,14 @@ String sourceRef = pushResult.getPatchSet().refName(); ReplicationQueue pullReplicationQueue = getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, sourceRef, ObjectId.zeroId().getName(), sourceCommit.getId().getName(), ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationWithGitHttpTransportProtocolIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationWithGitHttpTransportProtocolIT.java index d8e5947..2d1f51f 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationWithGitHttpTransportProtocolIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationWithGitHttpTransportProtocolIT.java
@@ -21,7 +21,7 @@ import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseLocalDisk; import com.google.gerrit.acceptance.config.GerritConfig; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import java.io.IOException; import java.util.List; import java.util.Optional; @@ -82,14 +82,14 @@ String sourceRef = pushResult.getPatchSet().refName(); ReplicationQueue pullReplicationQueue = getInstance(ReplicationQueue.class); - GitReferenceUpdatedListener.Event event = + GitBatchRefUpdateListener.Event event = new FakeGitReferenceUpdatedEvent( project, sourceRef, ObjectId.zeroId().getName(), sourceCommit.getId().getName(), ReceiveCommand.Type.CREATE); - pullReplicationQueue.onGitReferenceUpdated(event); + pullReplicationQueue.onGitBatchRefUpdate(event); try (Repository repo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java index 0743cb9..e7de264 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java
@@ -21,6 +21,7 @@ import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -33,8 +34,8 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.common.AccountInfo; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; -import com.google.gerrit.extensions.events.GitReferenceUpdatedListener.Event; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener.UpdatedRef; import com.google.gerrit.extensions.events.ProjectDeletedListener; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.metrics.DisabledMetricMaker; @@ -45,7 +46,6 @@ import com.googlesource.gerrit.plugins.replication.ReplicationConfig; import com.googlesource.gerrit.plugins.replication.ReplicationFileBasedConfig; import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData; -import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException; import com.googlesource.gerrit.plugins.replication.pull.client.FetchApiClient; import com.googlesource.gerrit.plugins.replication.pull.client.FetchRestApiClient; import com.googlesource.gerrit.plugins.replication.pull.client.HttpResult; @@ -55,6 +55,8 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import org.apache.http.client.ClientProtocolException; import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.lib.ObjectId; @@ -65,6 +67,7 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -162,70 +165,97 @@ @Test public void shouldCallSendObjectWhenMetaRef() throws ClientProtocolException, IOException { - Event event = new TestEvent("refs/changes/01/1/meta"); + TestEvent event = new TestEvent("refs/changes/01/1/meta"); objectUnderTest.start(); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient).callSendObjects(any(), anyString(), any(), any()); } @Test public void shouldCallInitProjectWhenProjectIsMissing() throws IOException { - Event event = new TestEvent("refs/changes/01/1/meta"); + TestEvent event = new TestEvent("refs/changes/01/1/meta"); when(httpResult.isSuccessful()).thenReturn(false); when(httpResult.isProjectMissing(any())).thenReturn(true); when(source.isCreateMissingRepositories()).thenReturn(true); objectUnderTest.start(); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient).initProject(any(), any()); } @Test public void shouldNotCallInitProjectWhenReplicateNewRepositoriesNotSet() throws IOException { - Event event = new TestEvent("refs/changes/01/1/meta"); + TestEvent event = new TestEvent("refs/changes/01/1/meta"); when(httpResult.isSuccessful()).thenReturn(false); when(httpResult.isProjectMissing(any())).thenReturn(true); when(source.isCreateMissingRepositories()).thenReturn(false); objectUnderTest.start(); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient, never()).initProject(any(), any()); } @Test public void shouldCallSendObjectWhenPatchSetRef() throws ClientProtocolException, IOException { - Event event = new TestEvent("refs/changes/01/1/1"); + TestEvent event = new TestEvent("refs/changes/01/1/1"); objectUnderTest.start(); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient).callSendObjects(any(), anyString(), any(), any()); } @Test + public void shouldCallSendObjectReorderingRefsHavingMetaAtTheEnd() + throws ClientProtocolException, IOException { + sendRefUpdatedEvents("refs/changes/01/1/meta", "refs/changes/01/1/1"); + verifySendObjectOrdering("refs/changes/01/1/1", "refs/changes/01/1/meta"); + } + + @Test + public void shouldCallSendObjectKeepingMetaAtTheEnd() + throws ClientProtocolException, IOException { + sendRefUpdatedEvents("refs/changes/01/1/1", "refs/changes/01/1/meta"); + verifySendObjectOrdering("refs/changes/01/1/1", "refs/changes/01/1/meta"); + } + + private void sendRefUpdatedEvents(String firstRef, String secondRef) { + objectUnderTest.start(); + objectUnderTest.onGitBatchRefUpdate(new TestEvent(firstRef, secondRef)); + } + + private void verifySendObjectOrdering(String firstRef, String secondRef) + throws ClientProtocolException, IOException { + InOrder inOrder = inOrder(fetchRestApiClient); + + inOrder.verify(fetchRestApiClient).callSendObjects(any(), eq(firstRef), any(), any()); + inOrder.verify(fetchRestApiClient).callSendObjects(any(), eq(secondRef), any(), any()); + } + + @Test public void shouldFallbackToCallFetchWhenIOException() - throws ClientProtocolException, IOException, LargeObjectException, RefUpdateException { - Event event = new TestEvent("refs/changes/01/1/meta"); + throws ClientProtocolException, IOException, LargeObjectException { + TestEvent event = new TestEvent("refs/changes/01/1/meta"); objectUnderTest.start(); when(revReader.read(any(), any(), anyString(), anyInt())).thenThrow(IOException.class); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient).callFetch(any(), anyString(), any()); } @Test public void shouldFallbackToCallFetchWhenLargeRef() - throws ClientProtocolException, IOException, LargeObjectException, RefUpdateException { - Event event = new TestEvent("refs/changes/01/1/1"); + throws ClientProtocolException, IOException, LargeObjectException { + TestEvent event = new TestEvent("refs/changes/01/1/1"); objectUnderTest.start(); when(revReader.read(any(), any(), anyString(), anyInt())).thenReturn(Optional.empty()); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient).callFetch(any(), anyString(), any()); } @@ -233,7 +263,7 @@ @Test public void shouldFallbackToCallFetchWhenParentObjectIsMissing() throws ClientProtocolException, IOException { - Event event = new TestEvent("refs/changes/01/1/1"); + TestEvent event = new TestEvent("refs/changes/01/1/1"); objectUnderTest.start(); when(httpResult.isSuccessful()).thenReturn(false); @@ -241,7 +271,7 @@ when(fetchRestApiClient.callSendObjects(any(), anyString(), any(), any())) .thenReturn(httpResult); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient).callFetch(any(), anyString(), any()); } @@ -249,7 +279,7 @@ @Test public void shouldFallbackToApplyAllParentObjectsWhenParentObjectIsMissingOnMetaRef() throws ClientProtocolException, IOException { - Event event = new TestEvent("refs/changes/01/1/meta"); + TestEvent event = new TestEvent("refs/changes/01/1/meta"); objectUnderTest.start(); when(httpResult.isSuccessful()).thenReturn(false, true); @@ -257,7 +287,7 @@ when(fetchRestApiClient.callSendObjects(any(), anyString(), any(), any())) .thenReturn(httpResult); - objectUnderTest.onGitReferenceUpdated(event); + objectUnderTest.onGitBatchRefUpdate(event); verify(fetchRestApiClient, times(2)) .callSendObjects(any(), anyString(), revisionsDataCaptor.capture(), any()); @@ -292,22 +322,22 @@ () -> revReader, applyObjectMetrics, fetchMetrics); - Event event = new TestEvent("refs/multi-site/version"); - objectUnderTest.onGitReferenceUpdated(event); + TestEvent event = new TestEvent("refs/multi-site/version"); + objectUnderTest.onGitBatchRefUpdate(event); verifyZeroInteractions(wq, rd, dis, sl, fetchClientFactory, accountInfo); } @Test public void shouldSkipEventWhenStarredChangesRef() { - Event event = new TestEvent("refs/starred-changes/41/2941/1000000"); - objectUnderTest.onGitReferenceUpdated(event); + TestEvent event = new TestEvent("refs/starred-changes/41/2941/1000000"); + objectUnderTest.onGitBatchRefUpdate(event); verifyZeroInteractions(wq, rd, dis, sl, fetchClientFactory, accountInfo); } @Test - public void shouldCallDeleteWhenReplicateProjectDeletionsTrue() throws IOException { + public void shouldCallDeleteWhenReplicateProjectDeletionsTrue() { when(source.wouldDeleteProject(any())).thenReturn(true); String projectName = "testProject"; @@ -323,7 +353,7 @@ } @Test - public void shouldNotCallDeleteWhenProjectNotToDelete() throws IOException { + public void shouldNotCallDeleteWhenProjectNotToDelete() { when(source.wouldDeleteProject(any())).thenReturn(false); FakeProjectDeletedEvent event = new FakeProjectDeletedEvent("testProject"); @@ -335,7 +365,7 @@ } @Test - public void shouldScheduleUpdateHeadWhenWouldFetchProject() throws IOException { + public void shouldScheduleUpdateHeadWhenWouldFetchProject() { when(source.wouldFetchProject(any())).thenReturn(true); String projectName = "aProject"; @@ -351,7 +381,7 @@ } @Test - public void shouldNotScheduleUpdateHeadWhenNotWouldFetchProject() throws IOException { + public void shouldNotScheduleUpdateHeadWhenNotWouldFetchProject() { when(source.wouldFetchProject(any())).thenReturn(false); String projectName = "aProject"; @@ -366,24 +396,22 @@ return createTempDirectory(prefix); } - private class TestEvent implements GitReferenceUpdatedListener.Event { + private static class TestEvent implements GitBatchRefUpdateListener.Event { private String refName; private String projectName; - private ObjectId newObjectId; + private List<UpdatedRef> refs; - public TestEvent(String refName) { - this(refName, "defaultProject", ObjectId.zeroId()); + public TestEvent(String... refNames) { + this( + "defaultProject", + Arrays.stream(refNames) + .map(refName -> updateRef(refName, ObjectId.zeroId())) + .collect(Collectors.toUnmodifiableList())); } - public TestEvent(String refName, String projectName, ObjectId newObjectId) { - this.refName = refName; + public TestEvent(String projectName, List<UpdatedRef> refs) { this.projectName = projectName; - this.newObjectId = newObjectId; - } - - @Override - public String getRefName() { - return refName; + this.refs = refs; } @Override @@ -397,34 +425,55 @@ } @Override - public String getOldObjectId() { - return ObjectId.zeroId().getName(); - } - - @Override - public String getNewObjectId() { - return newObjectId.getName(); - } - - @Override - public boolean isCreate() { - return false; - } - - @Override - public boolean isDelete() { - return false; - } - - @Override - public boolean isNonFastForward() { - return false; - } - - @Override public AccountInfo getUpdater() { return null; } + + @Override + public Set<UpdatedRef> getUpdatedRefs() { + return refs.stream().collect(Collectors.toSet()); + } + + private static final GitBatchRefUpdateListener.UpdatedRef updateRef( + String refName, ObjectId refObjectId) { + return new GitBatchRefUpdateListener.UpdatedRef() { + + @Override + public String getRefName() { + return refName; + } + + @Override + public String getOldObjectId() { + return ObjectId.zeroId().getName(); + } + + @Override + public String getNewObjectId() { + return refObjectId.getName(); + } + + @Override + public boolean isCreate() { + return false; + } + + @Override + public boolean isDelete() { + return false; + } + + @Override + public boolean isNonFastForward() { + return false; + } + }; + } + + @Override + public Set<String> getRefNames() { + return Set.of(refName); + } } private class FakeProjectDeletedEvent implements ProjectDeletedListener.Event {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java index 20c1fea..e638653 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java
@@ -163,7 +163,7 @@ } public ResponseHandler<Object> assertHttpResponseCode(int responseCode) { - return new ResponseHandler<Object>() { + return new ResponseHandler<>() { @Override public Object handleResponse(HttpResponse response)
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java index 9b5ef46..4c188df 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java
@@ -64,7 +64,6 @@ private String sampleCommitObjectId = "9f8d52853089a3cf00c02ff7bd0817bd4353a95a"; private String sampleTreeObjectId = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; - private String sampleBlobObjectId = "b5d7bcf1d1c5b0f0726d10a16c8315f06f900bfb"; @Mock private PullReplicationStateLogger fetchStateLog; @Mock private ApplyObject applyObject;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java index f1f9e44..fc1b02c 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java
@@ -102,7 +102,6 @@ fetchStateLog, projectCache, sourceCollection, - applyObject, permissionBackend, eventDispatcherDataItem, new LocalGitRepositoryManagerProvider(gitManager));