Merge branch 'stable-3.9'
* stable-3.9:
Fix ApplyObjectActionIT flakiness due to the wrong ref used
Force async fetch as a fallback for sync replication
Index change asynchronously upon refs replicated event
Address follow-up comments to Change 396868
Introduce wait for replication events to reduce flaky tests
Fail apply-object on change /meta when missing patch-set
Change-Id: Ib0783cb2b0863f7327b415ee9917956e1cc5f1a1
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 ac17fac..baeb330 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
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.replication.pull;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Queues;
@@ -42,6 +44,7 @@
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
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;
import com.googlesource.gerrit.plugins.replication.pull.client.HttpResultUtils;
import com.googlesource.gerrit.plugins.replication.pull.filter.ApplyObjectsRefsFilter;
@@ -57,7 +60,6 @@
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ForkJoinPool;
-import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.Consumer;
import java.util.stream.Collectors;
@@ -304,7 +306,7 @@
Project.nameKey(event.projectName()), event.refs(), event.eventCreatedOn(), state);
fetchCallsPool
.submit(() -> allSources.parallelStream().forEach(callFunction))
- .get(fetchCallsTimeout, TimeUnit.MILLISECONDS);
+ .get(fetchCallsTimeout, MILLISECONDS);
} catch (InterruptedException | ExecutionException | TimeoutException e) {
stateLog.error(
String.format(
@@ -346,7 +348,7 @@
if (source.enableBatchedRefs()) {
callBatchFetch(source, project, refs, state);
} else {
- callFetch(source, project, refs, state);
+ callFetch(source, project, refs, state, FetchRestApiClient.FORCE_ASYNC);
}
}
};
@@ -673,7 +675,8 @@
Source source,
Project.NameKey project,
List<ReferenceUpdatedEvent> refs,
- ReplicationState state) {
+ ReplicationState state,
+ boolean forceAsyncCall) {
boolean resultIsSuccessful = true;
for (ReferenceUpdatedEvent refEvent : refs) {
String refName = refEvent.refName();
@@ -685,7 +688,14 @@
repLog.info(
"Pull replication REST API fetch to {} for {}:{}", apiUrl, project, refName);
long startTime = System.currentTimeMillis();
- Optional<HttpResult> result = Optional.of(fetchClient.callFetch(project, refName, uri));
+ Optional<HttpResult> result =
+ Optional.of(
+ fetchClient.callFetch(
+ project,
+ refName,
+ uri,
+ MILLISECONDS.toNanos(System.currentTimeMillis()),
+ forceAsyncCall));
long endTime = System.currentTimeMillis();
boolean resultSuccessful = HttpResultUtils.isSuccessful(result);
repLog.info(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
index 9911070..01d32fb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
@@ -19,6 +19,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -28,6 +29,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionInput;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingLatestPatchSetException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
import java.io.IOException;
@@ -132,6 +134,15 @@
input.getRevisionData(),
e);
throw new UnprocessableEntityException(e.getMessage());
+ } catch (MissingLatestPatchSetException e) {
+ repLog.error(
+ "Apply object API *FAILED* from {} for {}:{} - {}",
+ input.getLabel(),
+ resource.getNameKey(),
+ input.getRefName(),
+ input.getRevisionData(),
+ e);
+ throw new PreconditionFailedException(e.getMessage());
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java
index 75de6c8..157fac3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java
@@ -40,6 +40,7 @@
import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingLatestPatchSetException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject;
@@ -91,7 +92,7 @@
String sourceLabel,
long eventCreatedOn)
throws IOException, RefUpdateException, MissingParentObjectException,
- ResourceNotFoundException {
+ ResourceNotFoundException, MissingLatestPatchSetException {
applyObjects(name, refName, new RevisionData[] {revisionsData}, sourceLabel, eventCreatedOn);
}
@@ -102,7 +103,7 @@
String sourceLabel,
long eventCreatedOn)
throws IOException, RefUpdateException, MissingParentObjectException,
- ResourceNotFoundException {
+ ResourceNotFoundException, MissingLatestPatchSetException {
repLog.info(
"Apply object from {} for {}:{} - {}",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectsAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectsAction.java
index ffd5bfc..1088e40 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectsAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectsAction.java
@@ -19,6 +19,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -27,6 +28,7 @@
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionsInput;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingLatestPatchSetException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
import java.io.IOException;
@@ -130,6 +132,15 @@
Arrays.toString(input.getRevisionsData()),
e);
throw new UnprocessableEntityException(e.getMessage());
+ } catch (MissingLatestPatchSetException e) {
+ repLog.error(
+ "Apply object API *FAILED* from {} for {}:{} - {}",
+ input.getLabel(),
+ resource.getNameKey(),
+ input.getRefName(),
+ Arrays.toString(input.getRevisionsData()),
+ e);
+ throw new PreconditionFailedException(e.getMessage());
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java
index 155b6a1..a69afa4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java
@@ -46,6 +46,7 @@
import com.googlesource.gerrit.plugins.replication.LocalFS;
import com.googlesource.gerrit.plugins.replication.pull.GerritConfigOps;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionsInput;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingLatestPatchSetException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
import com.googlesource.gerrit.plugins.replication.pull.api.util.PayloadSerDes;
@@ -166,12 +167,23 @@
}
String projectName = gitRepositoryName.replace(".git", "");
- applyObjectCommand.applyObjects(
- Project.nameKey(projectName),
- input.getRefName(),
- input.getRevisionsData(),
- input.getLabel(),
- input.getEventCreatedOn());
+ try {
+ applyObjectCommand.applyObjects(
+ Project.nameKey(projectName),
+ input.getRefName(),
+ input.getRevisionsData(),
+ input.getLabel(),
+ input.getEventCreatedOn());
+ } catch (MissingLatestPatchSetException e) {
+ repLog.error(
+ "Init project API FAILED from {} for {} - configuration data cannot contain change meta refs: {}:{}",
+ input.getLabel(),
+ projectName,
+ input.getRefName(),
+ Arrays.toString(input.getRevisionsData()),
+ e);
+ throw new BadRequestException("Configuration data cannot contain change meta refs", e);
+ }
projectCache.onCreateProject(Project.nameKey(projectName));
// In case pull-replication is used in conjunction with multi-site, by convention the remote
// label is populated with the instanceId. That's why we are passing input.getLabel()
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/MissingLatestPatchSetException.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/MissingLatestPatchSetException.java
new file mode 100644
index 0000000..458c74a
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/MissingLatestPatchSetException.java
@@ -0,0 +1,26 @@
+// Copyright (C) 2023 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.exception;
+
+import com.google.gerrit.entities.Project;
+
+public class MissingLatestPatchSetException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ public MissingLatestPatchSetException(
+ Project.NameKey project, String refName, String errorMessage) {
+ super(String.format("%s for project %s ref name: %s", errorMessage, project.get(), refName));
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchApiClient.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchApiClient.java
index 27e13a7..28c74c9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchApiClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchApiClient.java
@@ -32,12 +32,17 @@
}
HttpResult callFetch(
- Project.NameKey project, String refName, URIish targetUri, long startTimeNanos)
+ Project.NameKey project,
+ String refName,
+ URIish targetUri,
+ long startTimeNanos,
+ boolean forceAsyncFetch)
throws IOException;
default HttpResult callFetch(Project.NameKey project, String refName, URIish targetUri)
throws IOException {
- return callFetch(project, refName, targetUri, MILLISECONDS.toNanos(System.currentTimeMillis()));
+ return callFetch(
+ project, refName, targetUri, MILLISECONDS.toNanos(System.currentTimeMillis()), false);
}
HttpResult callBatchFetch(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java
index 081661f..8531b0a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java
@@ -54,6 +54,7 @@
import org.apache.http.ParseException;
import org.apache.http.auth.AuthenticationException;
import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.ResponseHandler;
import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpPost;
@@ -67,6 +68,8 @@
import org.eclipse.jgit.transport.URIish;
public class FetchRestApiClient implements FetchApiClient, ResponseHandler<HttpResult> {
+ public static final boolean FORCE_ASYNC = true;
+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
static String GERRIT_ADMIN_PROTOCOL_PREFIX = "gerrit+";
@@ -111,15 +114,31 @@
this.urlAuthenticationPrefix = bearerTokenProvider.get().map(br -> "").orElse("a/");
}
- /* (non-Javadoc)
- * @see com.googlesource.gerrit.plugins.replication.pull.client.FetchApiClient#callFetch(com.google.gerrit.entities.Project.NameKey, java.lang.String, org.eclipse.jgit.transport.URIish)
- */
@Override
public HttpResult callFetch(
- Project.NameKey project, String refName, URIish targetUri, long startTimeNanos)
+ NameKey project,
+ String refName,
+ URIish targetUri,
+ long startTimeNanos,
+ boolean forceAsyncFetch)
+ throws ClientProtocolException, IOException {
+ return doCallFetch(
+ project,
+ refName,
+ targetUri,
+ startTimeNanos,
+ forceAsyncFetch || !syncRefsFilter.match(refName));
+ }
+
+ private HttpResult doCallFetch(
+ Project.NameKey project,
+ String refName,
+ URIish targetUri,
+ long startTimeNanos,
+ boolean callAsync)
throws IOException {
String url = formatUrl(targetUri.toString(), project, "fetch");
- Boolean callAsync = !syncRefsFilter.match(refName);
+
HttpPost post = new HttpPost(url);
post.setEntity(
new StringEntity(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java
index dc90c7f..70b4795 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java
@@ -22,6 +22,7 @@
import com.googlesource.gerrit.plugins.replication.pull.LocalGitRepositoryManagerProvider;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingLatestPatchSetException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import java.io.IOException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -46,7 +47,8 @@
}
public RefUpdateState apply(Project.NameKey name, RefSpec refSpec, RevisionData[] revisionsData)
- throws MissingParentObjectException, IOException, ResourceNotFoundException {
+ throws MissingParentObjectException, IOException, ResourceNotFoundException,
+ MissingLatestPatchSetException {
try (Repository git = gitManager.openRepository(name)) {
ObjectId refHead = null;
@@ -64,6 +66,12 @@
throw new MissingParentObjectException(name, refSpec.getSource(), parent.getId());
}
}
+
+ StringBuffer error = new StringBuffer();
+ if (!ChangeMetaCommitValidator.isValid(
+ git, refSpec.getSource(), commit, error::append)) {
+ throw new MissingLatestPatchSetException(name, refSpec.getSource(), error.toString());
+ }
}
for (RevisionObjectData rev : revisionData.getBlobs()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ChangeMetaCommitValidator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ChangeMetaCommitValidator.java
new file mode 100644
index 0000000..4e080af
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ChangeMetaCommitValidator.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2023 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.fetch;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.RefNames;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+import java.util.OptionalInt;
+import java.util.function.Consumer;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.FooterKey;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+class ChangeMetaCommitValidator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final FooterKey FOOTER_CHANGE_META_PATCH_SET = new FooterKey("Patch-set");
+
+ public static boolean isValid(
+ Repository repo, String refName, RevCommit commit, Consumer<String> errorCallback)
+ throws IOException {
+ if (!refName.startsWith(RefNames.REFS_CHANGES) || !refName.endsWith(RefNames.META_SUFFIX)) {
+ return true;
+ }
+
+ List<String> patchSetFooter = commit.getFooterLines(FOOTER_CHANGE_META_PATCH_SET);
+ OptionalInt latestPatchSet = patchSetFooter.stream().mapToInt(Integer::parseInt).max();
+
+ if (latestPatchSet.isEmpty()) {
+ return true;
+ }
+
+ String patchSetRef = refName.replace(RefNames.META_SUFFIX, "/" + latestPatchSet.getAsInt());
+ Optional<ObjectId> patchSetObjectId =
+ Optional.ofNullable(repo.exactRef(patchSetRef)).map(Ref::getObjectId);
+
+ if (patchSetObjectId.isEmpty()) {
+ errorCallback.accept("Unable to find latest patch-set ref " + patchSetRef);
+ return false;
+ }
+
+ RevCommit patchSetCommit = repo.parseCommit(patchSetObjectId.get());
+ logger.atFine().log(
+ "Change on repository %s ref %s has latest patch-set %d and is successfully resolved to %s with commit %s",
+ repo, refName, latestPatchSet.getAsInt(), patchSetObjectId.get().getName(), patchSetCommit);
+
+ return true;
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationITAbstract.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationITAbstract.java
index 43f6ef2..130bb41 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationITAbstract.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationITAbstract.java
@@ -548,7 +548,8 @@
assertThatRefReplicatedEventsContainsStatus(ReplicationState.RefFetchResult.SUCCEEDED);
}
- private void assertThatEventListenerHasReceivedNumEvents(int numExpectedEvents) {
+ private void assertThatEventListenerHasReceivedNumEvents(int numExpectedEvents) throws Exception {
+ waitUntil(() -> eventListener.numEventsReceived() >= numExpectedEvents);
assertThat(eventListener.numEventsReceived()).isEqualTo(numExpectedEvents);
}
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 90c9b9c..e3eb65b 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
@@ -167,14 +167,15 @@
lenient()
.when(fetchRestApiClient.callBatchSendObject(any(), any(), anyLong(), any()))
.thenReturn(batchHttpResult);
- when(fetchRestApiClient.callFetch(any(), anyString(), any())).thenReturn(fetchHttpResult);
+ when(fetchRestApiClient.callFetch(any(), anyString(), any(), anyLong(), anyBoolean()))
+ .thenReturn(fetchHttpResult);
when(fetchRestApiClient.callBatchFetch(any(), any(), any())).thenReturn(batchFetchHttpResult);
when(fetchRestApiClient.initProject(any(), any(), anyLong(), any()))
.thenReturn(successfulHttpResult);
when(successfulHttpResult.isSuccessful()).thenReturn(true);
when(httpResult.isSuccessful()).thenReturn(true);
- when(batchHttpResult.isSuccessful()).thenReturn(true);
- when(fetchHttpResult.isSuccessful()).thenReturn(true);
+ lenient().when(batchHttpResult.isSuccessful()).thenReturn(true);
+ lenient().when(fetchHttpResult.isSuccessful()).thenReturn(true);
when(batchFetchHttpResult.isSuccessful()).thenReturn(true);
when(httpResult.isProjectMissing(any())).thenReturn(false);
when(batchHttpResult.isProjectMissing(any())).thenReturn(false);
@@ -337,7 +338,7 @@
@Test
public void shouldFallbackToCallFetchWhenIOException() throws Exception {
- Event event = generateBatchRefUpdateEvent("refs/changes/01/1/meta");
+ BatchRefUpdateEvent event = generateBatchRefUpdateEvent("refs/changes/01/1/meta");
objectUnderTest.start();
@@ -397,7 +398,7 @@
@Test
public void shouldFallbackToCallBatchFetchWhenParentObjectNotMissingButApplyObjectFails()
throws Exception {
- Event event = generateBatchRefUpdateEvent("refs/changes/01/1/1");
+ BatchRefUpdateEvent event = generateBatchRefUpdateEvent("refs/changes/01/1/1");
objectUnderTest.start();
when(batchHttpResult.isSuccessful()).thenReturn(false);
@@ -467,7 +468,7 @@
@Test
public void shouldCallFetchIfBatchedRefsNotEnabledAtSource() throws Exception {
- Event event = generateBatchRefUpdateEvent("refs/changes/01/1/1");
+ BatchRefUpdateEvent event = generateBatchRefUpdateEvent("refs/changes/01/1/1");
when(source.enableBatchedRefs()).thenReturn(false);
when(httpResult.isSuccessful()).thenReturn(false);
when(httpResult.isParentObjectMissing()).thenReturn(false);
@@ -476,7 +477,7 @@
objectUnderTest.onEvent(event);
verify(fetchRestApiClient, never()).callBatchFetch(any(), any(), any());
- verify(fetchRestApiClient).callFetch(any(), anyString(), any());
+ verifyFallbackToRestApiClientFetchAsync(event);
}
@Test
@@ -675,4 +676,15 @@
return projectName;
}
}
+
+ private void verifyFallbackToRestApiClientFetchAsync(BatchRefUpdateEvent event)
+ throws IOException {
+ verify(fetchRestApiClient)
+ .callFetch(
+ eq(event.getProjectNameKey()),
+ eq(event.getRefNames().get(0)),
+ any(URIish.class),
+ any(Long.class),
+ eq(FetchRestApiClient.FORCE_ASYNC));
+ }
}
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 4a3fa55..f8110d1 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
@@ -197,7 +197,7 @@
return httpRequest;
}
- private Project.NameKey createTestProject(String name) throws Exception {
+ protected Project.NameKey createTestProject(String name) throws Exception {
return projectOperations.newProject().name(name).parent(project).create();
}
@@ -229,4 +229,9 @@
secureConfig.setString("remote", remoteName, "password", password);
secureConfig.save();
}
+
+ protected String firstPatchSetForChangeMetaRef(String metaRefName) {
+ String patchSetRefName = metaRefName.replace(RefNames.META_SUFFIX, "/1");
+ return patchSetRefName;
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionIT.java
index 652daed..e771e7a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionIT.java
@@ -19,6 +19,8 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.Url;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import java.util.Optional;
@@ -27,21 +29,25 @@
public class ApplyObjectActionIT extends ActionITBase {
+ private static final String REFS_HEADS_MASTER = RefNames.REFS_HEADS + "master";
+
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
public void shouldAcceptPayloadWithAsyncField() throws Exception {
+ createTestProjectWithReplicationSuffix();
String payloadWithAsyncFieldTemplate =
"{\"label\":\""
+ TEST_REPLICATION_REMOTE
+ "\",\"ref_name\":\"%s\",\"revision_data\":{\"commit_object\":{\"sha1\":\"%s\",\"type\":1,\"content\":\"%s\"},\"tree_object\":{\"type\":2,\"content\":\"%s\"},\"blobs\":[]}, \"async\":true}";
- String refName = createRef();
- Optional<RevisionData> revisionDataOption = createRevisionData(refName);
+ Optional<RevisionData> revisionDataOption = createRevisionData(REFS_HEADS_MASTER);
assertThat(revisionDataOption.isPresent()).isTrue();
RevisionData revisionData = revisionDataOption.get();
- String sendObjectPayload = createPayload(payloadWithAsyncFieldTemplate, refName, revisionData);
+ String sendObjectPayload =
+ createPayload(payloadWithAsyncFieldTemplate, REFS_HEADS_MASTER, revisionData);
+ deleteTestProjectBranch(REFS_HEADS_MASTER);
httpClientFactory
.create(source)
.execute(
@@ -52,19 +58,20 @@
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
public void shouldAcceptPayloadWithoutAsyncField() throws Exception {
+ createTestProjectWithReplicationSuffix();
String payloadWithoutAsyncFieldTemplate =
"{\"label\":\""
+ TEST_REPLICATION_REMOTE
+ "\",\"ref_name\":\"%s\",\"revision_data\":{\"commit_object\":{\"sha1\":\"%s\",\"type\":1,\"content\":\"%s\"},\"tree_object\":{\"type\":2,\"content\":\"%s\"},\"blobs\":[]}}";
- String refName = createRef();
- Optional<RevisionData> revisionDataOption = createRevisionData(refName);
+ Optional<RevisionData> revisionDataOption = createRevisionData(REFS_HEADS_MASTER);
assertThat(revisionDataOption.isPresent()).isTrue();
RevisionData revisionData = revisionDataOption.get();
String sendObjectPayload =
- createPayload(payloadWithoutAsyncFieldTemplate, refName, revisionData);
+ createPayload(payloadWithoutAsyncFieldTemplate, REFS_HEADS_MASTER, revisionData);
+ deleteTestProjectBranch(REFS_HEADS_MASTER);
httpClientFactory
.create(source)
.execute(
@@ -76,19 +83,20 @@
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
@GerritConfig(name = "container.replica", value = "true")
public void shouldAcceptPayloadWhenNodeIsAReplica() throws Exception {
+ createTestProjectWithReplicationSuffix();
String payloadWithoutAsyncFieldTemplate =
"{\"label\":\""
+ TEST_REPLICATION_REMOTE
+ "\",\"ref_name\":\"%s\",\"revision_data\":{\"commit_object\":{\"sha1\":\"%s\",\"type\":1,\"content\":\"%s\"},\"tree_object\":{\"type\":2,\"content\":\"%s\"},\"blobs\":[]}}";
- String refName = createRef();
- Optional<RevisionData> revisionDataOption = createRevisionData(refName);
+ Optional<RevisionData> revisionDataOption = createRevisionData(REFS_HEADS_MASTER);
assertThat(revisionDataOption.isPresent()).isTrue();
RevisionData revisionData = revisionDataOption.get();
String sendObjectPayload =
- createPayload(payloadWithoutAsyncFieldTemplate, refName, revisionData);
+ createPayload(payloadWithoutAsyncFieldTemplate, REFS_HEADS_MASTER, revisionData);
+ deleteTestProjectBranch(REFS_HEADS_MASTER);
httpClientFactory
.create(source)
.execute(
@@ -219,20 +227,21 @@
@GerritConfig(name = "container.replica", value = "false")
@GerritConfig(name = "auth.bearerToken", value = "some-bearer-token")
public void shouldAcceptPayloadWhenNodeIsAPrimaryWithBearerToken() throws Exception {
+ createTestProjectWithReplicationSuffix();
url = getURLWithoutAuthenticationPrefix(project.get());
String payloadWithoutAsyncFieldTemplate =
"{\"label\":\""
+ TEST_REPLICATION_REMOTE
+ "\",\"ref_name\":\"%s\",\"revision_data\":{\"commit_object\":{\"sha1\":\"%s\",\"type\":1,\"content\":\"%s\"},\"tree_object\":{\"type\":2,\"content\":\"%s\"},\"blobs\":[]}}";
- String refName = createRef();
- Optional<RevisionData> revisionDataOption = createRevisionData(refName);
+ Optional<RevisionData> revisionDataOption = createRevisionData(REFS_HEADS_MASTER);
assertThat(revisionDataOption.isPresent()).isTrue();
RevisionData revisionData = revisionDataOption.get();
String sendObjectPayload =
- createPayload(payloadWithoutAsyncFieldTemplate, refName, revisionData);
+ createPayload(payloadWithoutAsyncFieldTemplate, REFS_HEADS_MASTER, revisionData);
+ deleteTestProjectBranch(REFS_HEADS_MASTER);
httpClientFactory
.create(source)
.execute(
@@ -258,4 +267,12 @@
"%s/a/projects/%s/pull-replication~apply-object",
adminRestSession.url(), Url.encode(projectName));
}
+
+ private void createTestProjectWithReplicationSuffix() throws Exception {
+ createTestProject(project.get() + TEST_REPLICATION_SUFFIX);
+ }
+
+ private void deleteTestProjectBranch(String branchRefName) throws RestApiException {
+ gApi.projects().name(project.get()).branch(branchRefName).delete();
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java
index c75d32a..b562933 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java
@@ -28,6 +28,7 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.entities.RefNames;
@@ -42,6 +43,7 @@
import com.googlesource.gerrit.plugins.replication.pull.RevisionReader;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingLatestPatchSetException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import java.util.List;
import java.util.Optional;
@@ -75,7 +77,9 @@
testRepo = cloneProject(createTestProject(testRepoProjectName));
Result pushResult = createChange();
- String refName = RefNames.changeMetaRef(pushResult.getChange().getId());
+ Change.Id changeId = pushResult.getChange().getId();
+ String refName = RefNames.changeMetaRef(changeId);
+ String patchSetRefName = RefNames.patchSetRef(PatchSet.id(changeId, 1));
RefSpec refSpec = new RefSpec(refName);
Optional<RevisionData> revisionData;
@@ -83,6 +87,7 @@
try (Repository repo = repoManager.openRepository(testRepoKey)) {
revisionData = reader.read(testRepoKey, repo.exactRef(refName).getObjectId(), refName, 0);
+ objectUnderTest.apply(project, new RefSpec(patchSetRefName), toArray(revisionData));
objectUnderTest.apply(project, refSpec, toArray(revisionData));
}
@@ -124,6 +129,7 @@
Result pushResult = createChange();
Change.Id changeId = pushResult.getChange().getId();
+ String patchSetRefname = RefNames.patchSetRef(PatchSet.id(changeId, 1));
String refName = RefNames.changeMetaRef(changeId);
RefSpec refSpec = new RefSpec(refName);
@@ -131,6 +137,7 @@
try (Repository repo = repoManager.openRepository(testRepoKey)) {
Optional<RevisionData> revisionData =
reader.read(testRepoKey, repo.exactRef(refName).getObjectId(), refName, 0);
+ objectUnderTest.apply(project, new RefSpec(patchSetRefname), toArray(revisionData));
objectUnderTest.apply(project, refSpec, toArray(revisionData));
}
@@ -181,6 +188,27 @@
}
}
+ @Test
+ public void shouldThrowExceptionWhenPatchSetIsMissing() throws Exception {
+ String testRepoProjectName = project + TEST_REPLICATION_SUFFIX;
+ NameKey createTestProject = createTestProject(testRepoProjectName);
+ try (Repository repo = repoManager.openRepository(createTestProject)) {
+ testRepo = cloneProject(createTestProject);
+
+ Result pushResult = createChange();
+ Change.Id changeId = pushResult.getChange().getId();
+ String refName = RefNames.changeMetaRef(changeId);
+
+ Optional<RevisionData> revisionData =
+ reader.read(createTestProject, repo.exactRef(refName).getObjectId(), refName, 0);
+
+ RefSpec refSpec = new RefSpec(refName);
+ assertThrows(
+ MissingLatestPatchSetException.class,
+ () -> objectUnderTest.apply(project, refSpec, toArray(revisionData)));
+ }
+ }
+
private void compareObjects(RevisionData expected, Optional<RevisionData> actualOption) {
assertThat(actualOption.isPresent()).isTrue();
RevisionData actual = actualOption.get();
