Merge branch 'stable-3.11'
* stable-3.11:
Prevent the synchronous replication of create-refs via apply-object
Flag ref-updates events creating a new ref
Fix the create/delete of ref-specs in FetchOne delta
Report deletions as succeeded in the fetch-ref-replicated events
Change-Id: I65c2b8e161dd5b165fd2929c6861aa34e8d71d35
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
index 5e7fda2..939a05b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
@@ -228,14 +228,16 @@
// set may risk to have both update and delete for the same
// ref, which would be an issue: the delta is an unordered set
// and executing updates and deletes for the same ref may have
- // unexpected results, depending on which one you execute firt.
- //
- // Delete any previous ref-spec as delete or update for the same
- // refname so that the subsequent addition will make sure that
- // there is only one operation per refName.
- delta.remove(ref);
+ // unexpected results, depending on which one you execute first.
+
+ // Delete any previous ref-spec as delete
+ delta.remove(FetchRefSpec.fromRef(":" + ref.refName()));
+
+ // Delete any previous ref-spec as update
delta.remove(FetchRefSpec.fromRef(ref.refName()));
+ // Add the new ref as-is for making sure that
+ // there is only one operation per refName.
delta.add(ref);
repLog.trace("[{}] Added ref {} for replication from {}", taskIdHex, ref, uri);
}
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 495915d..b94189e 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
@@ -48,6 +48,7 @@
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.ApplyObjectBannedCreateRefsFilter;
import com.googlesource.gerrit.plugins.replication.pull.filter.ApplyObjectsRefsFilter;
import com.googlesource.gerrit.plugins.replication.pull.filter.ExcludedRefsFilter;
import java.io.IOException;
@@ -107,6 +108,7 @@
private final String instanceId;
private final boolean useBatchUpdateEvents;
private ApplyObjectsRefsFilter applyObjectsRefsFilter;
+ private final ApplyObjectBannedCreateRefsFilter applyObjectsBannedCreateRefsFilter;
@Inject
ReplicationQueue(
@@ -122,6 +124,7 @@
@GerritInstanceId String instanceId,
@GerritServerConfig Config gerritConfig,
ApplyObjectsRefsFilter applyObjectsRefsFilter,
+ ApplyObjectBannedCreateRefsFilter applyObjectsBannedCreateRefsFilter,
ShutdownState shutdownState) {
workQueue = wq;
dispatcher = dis;
@@ -138,6 +141,7 @@
this.useBatchUpdateEvents =
gerritConfig.getBoolean("event", "stream-events", "enableBatchRefUpdatedEvents", false);
this.applyObjectsRefsFilter = applyObjectsRefsFilter;
+ this.applyObjectsBannedCreateRefsFilter = applyObjectsBannedCreateRefsFilter;
}
@Override
@@ -367,8 +371,9 @@
.map(ref -> toBatchApplyObject(project, ref, state))
.collect(Collectors.toList());
- if (!containsLargeOrDeletedRefs(refsBatch)) {
- return ((source) -> callBatchSendObject(source, project, refsBatch, eventCreatedOn, state));
+ if (!containsLargeOrDeletedRefs(refsBatch)
+ && !hasCreateRefsBannedFromApplyObject(refsBatch)) {
+ return (source -> callBatchSendObject(source, project, refsBatch, eventCreatedOn, state));
}
} catch (UncheckedIOException e) {
stateLog.error("Falling back to calling fetch", e, state);
@@ -381,7 +386,8 @@
try {
Optional<RevisionData> maybeRevisionData =
revReaderProvider.get().read(project, event.objectId(), event.refName(), 0);
- return BatchApplyObjectData.create(event.refName(), maybeRevisionData);
+ return BatchApplyObjectData.create(
+ event.refName(), maybeRevisionData, event.isDelete(), event.isCreate());
} catch (IOException e) {
stateLog.error(
String.format(
@@ -397,6 +403,13 @@
return batchApplyObjectData.stream().anyMatch(e -> e.revisionData().isEmpty());
}
+ private boolean hasCreateRefsBannedFromApplyObject(
+ List<BatchApplyObjectData> batchApplyObjectData) {
+ return batchApplyObjectData.stream()
+ .filter(BatchApplyObjectData::isCreate)
+ .anyMatch(e -> applyObjectsBannedCreateRefsFilter.match(e.refName()));
+ }
+
private Optional<HttpResult> callSendObject(
FetchApiClient fetchClient,
String remoteName,
@@ -820,9 +833,10 @@
String refName,
ObjectId objectId,
long eventCreatedOn,
- boolean isDelete) {
+ boolean isDelete,
+ boolean isCreate) {
return new AutoValue_ReplicationQueue_ReferenceUpdatedEvent(
- projectName, refName, objectId, eventCreatedOn, isDelete);
+ projectName, refName, objectId, eventCreatedOn, isDelete, isCreate);
}
static ReferenceUpdatedEvent from(RefUpdateAttribute refUpdateAttribute, long eventCreatedOn) {
@@ -831,7 +845,8 @@
refUpdateAttribute.refName,
ObjectId.fromString(refUpdateAttribute.newRev),
eventCreatedOn,
- ZEROS_OBJECTID.equals(refUpdateAttribute.newRev));
+ ZEROS_OBJECTID.equals(refUpdateAttribute.newRev),
+ ZEROS_OBJECTID.equals(refUpdateAttribute.oldRev));
}
public abstract String projectName();
@@ -843,6 +858,8 @@
public abstract long eventCreatedOn();
public abstract boolean isDelete();
+
+ public abstract boolean isCreate();
}
@FunctionalInterface
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 fe558e5..c3f91d6 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
@@ -73,7 +73,7 @@
name.get(),
r,
sourceLabel);
- return new RefUpdateState(r, RefUpdate.Result.IO_FAILURE);
+ return new RefUpdateState(":" + r, RefUpdate.Result.IO_FAILURE);
}
})
.toList();
@@ -95,7 +95,7 @@
taskIdRef,
refName,
name);
- return new RefUpdateState(refName, RefUpdate.Result.NO_CHANGE);
+ return new RefUpdateState(":" + refName, RefUpdate.Result.NO_CHANGE);
}
repLog.info(
@@ -112,7 +112,7 @@
Optional<Ref> ref = getRef(name, refName);
if (!ref.isPresent()) {
logger.atFine().log("[%s] Ref %s was not found in project %s", taskIdRef, refName, name);
- return new RefUpdateState(refName, RefUpdate.Result.NO_CHANGE);
+ return new RefUpdateState(":" + refName, RefUpdate.Result.NO_CHANGE);
}
RefUpdateState deleteResult = deleteRef(name, ref.get());
@@ -149,7 +149,7 @@
u.setForceUpdate(true);
result = u.delete();
- return new RefUpdateState(ref.getName(), result);
+ return new RefUpdateState(":" + ref.getName(), result);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/BatchApplyObjectData.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/BatchApplyObjectData.java
index 5b17002..95ba9a9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/BatchApplyObjectData.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/BatchApplyObjectData.java
@@ -20,9 +20,24 @@
@AutoValue
public abstract class BatchApplyObjectData {
- public static BatchApplyObjectData create(String refName, Optional<RevisionData> revisionData)
+ public static BatchApplyObjectData create(
+ String refName, Optional<RevisionData> revisionData, boolean isDelete, boolean isCreate)
throws IllegalArgumentException {
- return new AutoValue_BatchApplyObjectData(refName, revisionData, false);
+ return new AutoValue_BatchApplyObjectData(refName, revisionData, isDelete, isCreate);
+ }
+
+ public static BatchApplyObjectData newDeleteRef(String refName) throws IllegalArgumentException {
+ return create(refName, Optional.empty(), true, false);
+ }
+
+ public static BatchApplyObjectData newUpdateRef(
+ String refName, Optional<RevisionData> revisionData) throws IllegalArgumentException {
+ return create(refName, revisionData, false, false);
+ }
+
+ public static BatchApplyObjectData newCreateRef(
+ String refName, Optional<RevisionData> revisionData) throws IllegalArgumentException {
+ return create(refName, revisionData, false, true);
}
public abstract String refName();
@@ -31,6 +46,8 @@
public abstract boolean isDelete();
+ public abstract boolean isCreate();
+
@Override
public String toString() {
return String.format(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/filter/ApplyObjectBannedCreateRefsFilter.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/filter/ApplyObjectBannedCreateRefsFilter.java
new file mode 100644
index 0000000..d7c685a
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/filter/ApplyObjectBannedCreateRefsFilter.java
@@ -0,0 +1,37 @@
+// Copyright (C) 2025 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.filter;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+
+@Singleton
+public class ApplyObjectBannedCreateRefsFilter extends RefsFilter {
+ @Inject
+ public ApplyObjectBannedCreateRefsFilter(ReplicationConfig replicationConfig) {
+ super(replicationConfig);
+ }
+
+ @Override
+ protected List<String> getRefNamePatterns(Config cfg) {
+ String[] applyObjectBannedCreateRefs =
+ cfg.getStringList("replication", null, "applyObjectBannedCreateRefs");
+ return ImmutableList.copyOf(applyObjectBannedCreateRefs);
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 546df2e..da350ad 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -132,6 +132,33 @@
: If true, the default fetch refspec will be set to use forced update to
the local repository when no refspec is given. By default, false.
+
+replication.applyObjectBannedCreateRefs
+: Specify which refs created should be banned from being
+ replicated synchronously via apply-object.
+
+ It can be provided more than once, and supports three formats: regular expressions,
+ wildcard matching, and single ref matching. All three formats match are case-sensitive.
+
+ Values starting with a caret `^` are treated as regular
+ expressions. For the regular expressions details please follow
+ official [java documentation](https://docs.oracle.com/javase/tutorial/essential/regex/).
+
+ Please note that regular expressions could also be used
+ with inverse match.
+
+ Values that are not regular expressions and end in `*` are
+ treated as wildcard matches. Wildcards match refs whose
+ name agrees from the beginning until the trailing `*`. So
+ `foo/b*` would match the refs `foo/b`, `foo/bar`, and
+ `foo/baz`, but neither `foobar`, nor `bar/foo/baz`.
+
+ Values that are neither regular expressions nor wildcards are
+ treated as single ref matches. So `foo/bar` matches only
+ the ref `foo/bar`, but no other refs.
+
+ By default, set to empty (all create refs are replicated synchronously).
+
replication.lockErrorMaxRetries
: Number of times to retry a replication operation if a lock
error is detected.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java
index 73715d6..6ce3dbe 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java
@@ -70,6 +70,7 @@
private final String TEST_PROJECT_NAME = "FetchOneTest";
private final Project.NameKey PROJECT_NAME = Project.NameKey.parse(TEST_PROJECT_NAME);
private final String TEST_REF = "refs/heads/refForReplicationTask";
+ private final String TEST_DELETE_REF = ":" + TEST_REF;
private final FetchRefSpec TEST_REF_SPEC = FetchRefSpec.fromRef(TEST_REF);
private final String URI_PATTERN = "http://test.com/" + TEST_PROJECT_NAME + ".git";
private final TestMetricMaker testMetricMaker = new TestMetricMaker();
@@ -178,6 +179,26 @@
}
@Test
+ public void shouldDeleteRefWhenAddingDeleteRefSpec() throws IOException {
+ setupRemoteConfigMock(List.of(ALL_REFS_SPEC));
+ objectUnderTest.addRefs(refSpecsSetOf(TEST_REF));
+ objectUnderTest.addRefs(refSpecsSetOf(TEST_DELETE_REF));
+
+ assertThat(objectUnderTest.getFetchRefSpecs())
+ .isEqualTo(List.of(FetchRefSpec.fromRef(TEST_DELETE_REF)));
+ }
+
+ @Test
+ public void shouldAddRefWhenAddingRefSpecToDeleteRefSpec() throws IOException {
+ setupRemoteConfigMock(List.of(ALL_REFS_SPEC));
+ objectUnderTest.addRefs(refSpecsSetOf(TEST_DELETE_REF));
+ objectUnderTest.addRefs(refSpecsSetOf(TEST_REF));
+
+ assertThat(objectUnderTest.getFetchRefSpecs())
+ .isEqualTo(List.of(FetchRefSpec.fromRef(TEST_REF + ":" + TEST_REF)));
+ }
+
+ @Test
public void shouldReturnExistingStates() {
assertThat(createTestStates(TEST_REF_SPEC, 1))
.isEqualTo(objectUnderTest.getStates().get(TEST_REF_SPEC));
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 1c8a8ea..7f4ca4e 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
@@ -57,6 +57,7 @@
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.filter.ApplyObjectBannedCreateRefsFilter;
import com.googlesource.gerrit.plugins.replication.pull.filter.ApplyObjectsRefsFilter;
import com.googlesource.gerrit.plugins.replication.pull.filter.ExcludedRefsFilter;
import java.io.IOException;
@@ -88,6 +89,8 @@
private static final String TEST_REF_NAME = "refs/meta/heads/anyref";
private static final Project.NameKey PROJECT = Project.nameKey("defaultProject");
+ private static final String OLD_OBJECT_ID =
+ ObjectId.fromString("00f11fd1e3206333235603f889837bad2692da4b").getName();
private static final String NEW_OBJECT_ID =
ObjectId.fromString("3c1ddc050d7906adb0e29bc3bc46af8749b2f63b").getName();
@@ -110,6 +113,7 @@
@Mock HttpResult httpResult;
@Mock HttpResult batchHttpResult;
@Mock ApplyObjectsRefsFilter applyObjectsRefsFilter;
+ @Mock ApplyObjectBannedCreateRefsFilter applyObjectsBannedCreateRefsFilter;
@Mock Config config;
ApplyObjectMetrics applyObjectMetrics;
@@ -207,6 +211,7 @@
LOCAL_INSTANCE_ID,
config,
applyObjectsRefsFilter,
+ applyObjectsBannedCreateRefsFilter,
shutdownState);
}
@@ -238,6 +243,7 @@
LOCAL_INSTANCE_ID,
config,
applyObjectsRefsFilter,
+ applyObjectsBannedCreateRefsFilter,
shutdownState);
Event event = new TestEvent("refs/changes/01/1/meta");
@@ -332,6 +338,7 @@
LOCAL_INSTANCE_ID,
config,
applyObjectsRefsFilter,
+ applyObjectsBannedCreateRefsFilter,
shutdownState);
}
@@ -420,6 +427,34 @@
}
@Test
+ public void shouldCallApplyObjectWhenCreatedRefNotMatchesApplyObjectBannedCreateRefsFilter()
+ throws Exception {
+ String bannedRef = "refs/heads/banned";
+ Event event = generateBatchCreateRefEvent("refs/heads/not-banned");
+ objectUnderTest.start();
+ lenient().when(applyObjectsBannedCreateRefsFilter.match(eq(bannedRef))).thenReturn(true);
+
+ objectUnderTest.onEvent(event);
+
+ verify(fetchRestApiClient).callBatchSendObject(any(), any(), anyLong(), any());
+ verify(fetchRestApiClient, never()).callBatchFetch(any(), any(), any());
+ }
+
+ @Test
+ public void shouldFallbackToCallBatchFetchWhenCreatedRefMatchesApplyObjectBannedCreateRefsFilter()
+ throws Exception {
+ String bannedRef = "refs/heads/banned";
+ Event event = generateBatchCreateRefEvent("refs/heads/banned");
+ objectUnderTest.start();
+ when(applyObjectsBannedCreateRefsFilter.match(eq(bannedRef))).thenReturn(true);
+
+ objectUnderTest.onEvent(event);
+
+ verify(fetchRestApiClient).callBatchFetch(any(), any(), any());
+ verify(fetchRestApiClient, never()).callBatchSendObject(any(), any(), anyLong(), any());
+ }
+
+ @Test
public void shouldFallbackToApplyAllParentObjectsWhenParentObjectIsMissingOnMetaRef()
throws Exception {
Event event = generateBatchRefUpdateEvent("refs/changes/01/1/meta");
@@ -536,6 +571,7 @@
LOCAL_INSTANCE_ID,
config,
applyObjectsRefsFilter,
+ applyObjectsBannedCreateRefsFilter,
shutdownState);
Event event = generateBatchRefUpdateEvent("refs/multi-site/version");
objectUnderTest.onEvent(event);
@@ -618,13 +654,21 @@
}
private BatchRefUpdateEvent generateBatchRefUpdateEvent(String... refs) {
+ return generateBatchRefEvent(OLD_OBJECT_ID, NEW_OBJECT_ID, refs);
+ }
+
+ private BatchRefUpdateEvent generateBatchCreateRefEvent(String... refs) {
+ return generateBatchRefEvent(ObjectId.zeroId().name(), NEW_OBJECT_ID, refs);
+ }
+
+ private BatchRefUpdateEvent generateBatchRefEvent(String oldRev, String newRev, String[] refs) {
List<RefUpdateAttribute> refUpdates =
Arrays.stream(refs)
.map(
ref -> {
RefUpdateAttribute upd = new RefUpdateAttribute();
- upd.newRev = NEW_OBJECT_ID;
- upd.oldRev = ObjectId.zeroId().getName();
+ upd.newRev = newRev;
+ upd.oldRev = oldRev;
upd.project = PROJECT.get();
upd.refName = ref;
return upd;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientBase.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientBase.java
index 7f0df41..cfff177 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientBase.java
@@ -624,7 +624,7 @@
List<BatchApplyObjectData> batchApplyObjects = new ArrayList<>();
batchApplyObjects.add(
- BatchApplyObjectData.create(refName, Optional.of(createSampleRevisionData("a"))));
+ BatchApplyObjectData.newUpdateRef(refName, Optional.of(createSampleRevisionData("a"))));
objectUnderTest.callBatchSendObject(
Project.nameKey("test_repo"), batchApplyObjects, eventCreatedOn, new URIish(api));
@@ -647,8 +647,8 @@
RevisionData revisionA = createSampleRevisionData("a");
RevisionData revisionB = createSampleRevisionData("b");
String refNameB = "refs/heads/b";
- batchApplyObjects.add(BatchApplyObjectData.create(refName, Optional.of(revisionA)));
- batchApplyObjects.add(BatchApplyObjectData.create(refNameB, Optional.of(revisionB)));
+ batchApplyObjects.add(BatchApplyObjectData.newUpdateRef(refName, Optional.of(revisionA)));
+ batchApplyObjects.add(BatchApplyObjectData.newUpdateRef(refNameB, Optional.of(revisionB)));
objectUnderTest.callBatchSendObject(
Project.nameKey("test_repo"), batchApplyObjects, eventCreatedOn, new URIish(api));
