Merge branch 'stable-2.16'
* stable-2.16:
Add metric to count number of detected split-brain ref-updates
Allow to soft-fail for All-Users:refs/meta/external-ids
Refactor the ignore refs logic into a separate interface
Change-Id: I4f24029cae037dad00a6c44be80080f0cfa027eb
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdate.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdate.java
index 783f6a2..ef89f4d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdate.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdate.java
@@ -52,6 +52,7 @@
private final RefDatabase refDb;
private final SharedRefDatabase sharedRefDb;
private final String projectName;
+ private final ValidationMetrics validationMetrics;
public static class RefPair {
final Ref oldRef;
@@ -81,13 +82,17 @@
@Inject
public MultiSiteBatchRefUpdate(
- SharedRefDatabase sharedRefDb, @Assisted String projectName, @Assisted RefDatabase refDb) {
+ SharedRefDatabase sharedRefDb,
+ ValidationMetrics validationMetrics,
+ @Assisted String projectName,
+ @Assisted RefDatabase refDb) {
super(refDb);
this.sharedRefDb = sharedRefDb;
this.projectName = projectName;
this.refDb = refDb;
this.batchRefUpdate = refDb.newBatchUpdate();
+ this.validationMetrics = validationMetrics;
}
@Override
@@ -235,6 +240,8 @@
boolean compareAndPutResult =
sharedRefDb.compareAndPut(projectName, refPair.oldRef, refPair.newRef);
if (!compareAndPutResult) {
+ validationMetrics.incrementSplitBrainRefUpdates();
+
throw new IOException(
String.format(
"This repos is out of sync for project %s. old_ref=%s, new_ref=%s",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdate.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdate.java
index d60082e..b543a37 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdate.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdate.java
@@ -47,7 +47,7 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
protected final RefUpdate refUpdateBase;
-
+ private final ValidationMetrics validationMetrics;
private final SharedRefDatabase sharedDb;
private final String projectName;
@@ -57,9 +57,13 @@
@Inject
public MultiSiteRefUpdate(
- SharedRefDatabase db, @Assisted String projectName, @Assisted RefUpdate refUpdate) {
+ SharedRefDatabase db,
+ ValidationMetrics validationMetrics,
+ @Assisted String projectName,
+ @Assisted RefUpdate refUpdate) {
super(refUpdate.getRef());
refUpdateBase = refUpdate;
+ this.validationMetrics = validationMetrics;
this.sharedDb = db;
this.projectName = projectName;
}
@@ -81,6 +85,8 @@
+ "Trying to update it cannot extract the existing one on DB",
refUpdateBase.getName());
+ validationMetrics.incrementSplitBrainRefUpdates();
+
throw new IOException(
String.format(
"Unable to update ref '%s', cannot open the local ref on the local DB",
@@ -105,6 +111,8 @@
+ "Trying to delete it but it is not in the DB",
oldRef.getName());
+ validationMetrics.incrementSplitBrainRefUpdates();
+
throw new IOException(
String.format(
"Unable to delete ref '%s', cannot find it in the shared ref database",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java
new file mode 100644
index 0000000..fb6e08c
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationMetrics.java
@@ -0,0 +1,56 @@
+// Copyright (C) 2019 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.
+// Copyright (C) 2018 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.multisite.validation;
+
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+@Singleton
+public class ValidationMetrics {
+ private static final String REF_UPDATES = "ref_updates";
+
+ private final Counter1<String> splitBrain;
+
+ @Inject
+ public ValidationMetrics(MetricMaker metricMaker) {
+ this.splitBrain =
+ metricMaker.newCounter(
+ "multi_site/validation/split_brain",
+ new Description("Rate of REST API error responses").setRate().setUnit("errors"),
+ Field.ofString(
+ REF_UPDATES, "Ref-update operations detected as leading to split-brain"));
+ }
+
+ public void incrementSplitBrainRefUpdates() {
+ splitBrain.increment(REF_UPDATES);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
index ea12e19..ba8816b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
@@ -16,7 +16,10 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.inject.Scopes;
import com.googlesource.gerrit.plugins.multisite.Configuration;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.DefaultSharedRefEnforcement;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.zookeeper.ZkValidationModule;
public class ValidationModule extends FactoryModule {
@@ -39,6 +42,8 @@
bind(GitRepositoryManager.class).to(MultiSiteGitRepositoryManager.class);
}
+ bind(SharedRefEnforcement.class).to(DefaultSharedRefEnforcement.class).in(Scopes.SINGLETON);
+
install(new ZkValidationModule(cfg));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java
new file mode 100644
index 0000000..82aad80
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java
@@ -0,0 +1,41 @@
+// Copyright (C) 2019 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.multisite.validation.dfsrefdb;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+
+public class DefaultSharedRefEnforcement implements SharedRefEnforcement {
+
+ private static final Map<String, EnforcePolicy> PREDEF_ENFORCEMENTS =
+ ImmutableMap.of("All-Users:refs/meta/external-ids", EnforcePolicy.DESIRED);
+
+ @Override
+ public EnforcePolicy getPolicy(String projectName, String refName) {
+ if (ignoreRefInSharedDb(refName)) {
+ return EnforcePolicy.IGNORED;
+ }
+
+ return MoreObjects.firstNonNull(
+ PREDEF_ENFORCEMENTS.get(projectName + ":" + refName), EnforcePolicy.REQUIRED);
+ }
+
+ private boolean ignoreRefInSharedDb(String refName) {
+ return refName == null
+ || refName.startsWith("refs/draft-comments")
+ || (refName.startsWith("refs/changes") && !refName.endsWith("/meta"));
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefDatabase.java
index 87ebc96..c886751 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefDatabase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefDatabase.java
@@ -116,16 +116,4 @@
* @throws java.io.IOException the reference could not be removed due to a system error.
*/
boolean compareAndRemove(String project, Ref oldRef) throws IOException;
-
- /**
- * Some references should not be stored in the SharedRefDatabase.
- *
- * @param refName
- * @return true if it's to be ignore; false otherwise
- */
- default boolean ignoreRefInSharedDb(String refName) {
- return refName == null
- || refName.startsWith("refs/draft-comments")
- || (refName.startsWith("refs/changes") && !refName.endsWith("/meta"));
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java
new file mode 100644
index 0000000..1267b9e
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2019 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.multisite.validation.dfsrefdb;
+
+/** Type of enforcement to implement between the local and shared RefDb. */
+public interface SharedRefEnforcement {
+ public enum EnforcePolicy {
+ IGNORED,
+ DESIRED,
+ REQUIRED;
+ }
+
+ /**
+ * Get the enforcement policy for a project/refName.
+ *
+ * @param projectName project to be enforced
+ * @param refName ref name to be enforced
+ * @return the {@link EnforcePolicy} value
+ */
+ public EnforcePolicy getPolicy(String projectName, String refName);
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
index 503d93f..51eabc8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
@@ -18,6 +18,8 @@
import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import javax.inject.Named;
@@ -33,12 +35,16 @@
private final CuratorFramework client;
private final RetryPolicy retryPolicy;
+ private final SharedRefEnforcement refEnforcement;
@Inject
public ZkSharedRefDatabase(
- CuratorFramework client, @Named("ZkLockRetryPolicy") RetryPolicy retryPolicy) {
+ CuratorFramework client,
+ @Named("ZkLockRetryPolicy") RetryPolicy retryPolicy,
+ SharedRefEnforcement refEnforcement) {
this.client = client;
this.retryPolicy = retryPolicy;
+ this.refEnforcement = refEnforcement;
}
@Override
@@ -48,7 +54,11 @@
@Override
public boolean compareAndPut(String projectName, Ref oldRef, Ref newRef) throws IOException {
- if (ignoreRefInSharedDb(MoreObjects.firstNonNull(oldRef.getName(), newRef.getName()))) {
+ EnforcePolicy enforcementPolicy =
+ refEnforcement.getPolicy(
+ projectName, MoreObjects.firstNonNull(oldRef.getName(), newRef.getName()));
+
+ if (enforcementPolicy == EnforcePolicy.IGNORED) {
return true;
}
@@ -68,7 +78,16 @@
if (!newDistributedValue.succeeded() && refNotInZk(projectName, oldRef, newRef)) {
return distributedRefValue.initialize(writeObjectId(newRef.getObjectId()));
}
- return newDistributedValue.succeeded();
+
+ boolean succeeded = newDistributedValue.succeeded();
+
+ if (!succeeded && enforcementPolicy == EnforcePolicy.DESIRED) {
+ logger.atWarning().log(
+ "Unable to compareAndPut %s %s=>%s, local ref-db is out of synch with the shared-db");
+ return true;
+ }
+
+ return succeeded;
} catch (Exception e) {
logger.atWarning().withCause(e).log(
"Error trying to perform CAS at path %s", pathFor(projectName, oldRef, newRef));
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
index bc558d5..8d16687 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
@@ -27,7 +27,10 @@
package com.googlesource.gerrit.plugins.multisite.validation;
+import static org.junit.Assert.fail;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.zookeeper.RefFixture;
@@ -56,6 +59,7 @@
@Mock RefDatabase refDatabase;
@Mock RevWalk revWalk;
@Mock ProgressMonitor progressMonitor;
+ @Mock ValidationMetrics validationMetrics;
private final Ref oldRef =
new ObjectIdRef.Unpeeled(Ref.Storage.NETWORK, A_TEST_REF_NAME, AN_OBJECT_ID_1);
@@ -79,7 +83,11 @@
doReturn(oldRef).when(refDatabase).getRef(A_TEST_REF_NAME);
doReturn(newRef).when(sharedRefDb).newRef(A_TEST_REF_NAME, AN_OBJECT_ID_2);
- multiSiteRefUpdate = new MultiSiteBatchRefUpdate(sharedRefDb, A_TEST_PROJECT_NAME, refDatabase);
+ multiSiteRefUpdate =
+ new MultiSiteBatchRefUpdate(
+ sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refDatabase);
+
+ verifyZeroInteractions(validationMetrics);
}
@Test
@@ -91,13 +99,18 @@
multiSiteRefUpdate.execute(revWalk, progressMonitor, Collections.emptyList());
}
- @Test(expected = IOException.class)
+ @Test
public void executeAndFailsWithExceptions() throws IOException {
setMockRequiredReturnValues();
// When compareAndPut against sharedDb fails
doReturn(false).when(sharedRefDb).compareAndPut(A_TEST_PROJECT_NAME, oldRef, newRef);
- multiSiteRefUpdate.execute(revWalk, progressMonitor, Collections.emptyList());
+ try {
+ multiSiteRefUpdate.execute(revWalk, progressMonitor, Collections.emptyList());
+ fail("Expecting an IOException to be thrown");
+ } catch (IOException e) {
+ verify(validationMetrics).incrementSplitBrainRefUpdates();
+ }
}
@Test
@@ -105,7 +118,9 @@
doReturn(batchRefUpdate).when(refDatabase).newBatchUpdate();
doReturn(Collections.emptyList()).when(batchRefUpdate).getCommands();
- multiSiteRefUpdate = new MultiSiteBatchRefUpdate(sharedRefDb, A_TEST_PROJECT_NAME, refDatabase);
+ multiSiteRefUpdate =
+ new MultiSiteBatchRefUpdate(
+ sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refDatabase);
multiSiteRefUpdate.execute(revWalk, progressMonitor, Collections.emptyList());
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java
index 9b8acde..e88dffb 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java
@@ -28,7 +28,10 @@
package com.googlesource.gerrit.plugins.multisite.validation;
import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.zookeeper.RefFixture;
@@ -49,6 +52,7 @@
@Mock SharedRefDatabase sharedRefDb;
@Mock RefUpdate refUpdate;
+ @Mock ValidationMetrics validationMetrics;
private final Ref oldRef =
new ObjectIdRef.Unpeeled(Ref.Storage.NETWORK, A_TEST_REF_NAME, AN_OBJECT_ID_1);
private final Ref newRef =
@@ -77,9 +81,11 @@
doReturn(Result.NEW).when(refUpdate).update();
MultiSiteRefUpdate multiSiteRefUpdate =
- new MultiSiteRefUpdate(sharedRefDb, A_TEST_PROJECT_NAME, refUpdate);
+ new MultiSiteRefUpdate(sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refUpdate);
assertThat(multiSiteRefUpdate.update()).isEqualTo(Result.NEW);
+
+ verifyZeroInteractions(validationMetrics);
}
@Test(expected = IOException.class)
@@ -90,11 +96,29 @@
doReturn(false).when(sharedRefDb).compareAndPut(A_TEST_PROJECT_NAME, oldRef, newRef);
MultiSiteRefUpdate multiSiteRefUpdate =
- new MultiSiteRefUpdate(sharedRefDb, A_TEST_PROJECT_NAME, refUpdate);
+ new MultiSiteRefUpdate(sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refUpdate);
multiSiteRefUpdate.update();
}
@Test
+ public void newUpdateShouldIncreaseRefUpdateFailureCountWhenFailing() throws IOException {
+ setMockRequiredReturnValues();
+
+ // When compareAndPut fails
+ doReturn(false).when(sharedRefDb).compareAndPut(A_TEST_PROJECT_NAME, oldRef, newRef);
+
+ MultiSiteRefUpdate multiSiteRefUpdate =
+ new MultiSiteRefUpdate(sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refUpdate);
+
+ try {
+ multiSiteRefUpdate.update();
+ fail("Expecting an IOException to be thrown");
+ } catch (IOException e) {
+ verify(validationMetrics).incrementSplitBrainRefUpdates();
+ }
+ }
+
+ @Test
public void deleteShouldValidateAndSucceed() throws IOException {
setMockRequiredReturnValues();
@@ -103,9 +127,10 @@
doReturn(Result.FORCED).when(refUpdate).delete();
MultiSiteRefUpdate multiSiteRefUpdate =
- new MultiSiteRefUpdate(sharedRefDb, A_TEST_PROJECT_NAME, refUpdate);
+ new MultiSiteRefUpdate(sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refUpdate);
assertThat(multiSiteRefUpdate.delete()).isEqualTo(Result.FORCED);
+ verifyZeroInteractions(validationMetrics);
}
@Test(expected = IOException.class)
@@ -116,7 +141,24 @@
doReturn(false).when(sharedRefDb).compareAndRemove(A_TEST_PROJECT_NAME, oldRef);
MultiSiteRefUpdate multiSiteRefUpdate =
- new MultiSiteRefUpdate(sharedRefDb, A_TEST_PROJECT_NAME, refUpdate);
+ new MultiSiteRefUpdate(sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refUpdate);
multiSiteRefUpdate.delete();
}
+
+ @Test
+ public void deleteShouldIncreaseRefUpdateFailureCountWhenFailing() throws IOException {
+ setMockRequiredReturnValues();
+
+ // When compareAndPut fails
+ doReturn(false).when(sharedRefDb).compareAndRemove(A_TEST_PROJECT_NAME, oldRef);
+
+ MultiSiteRefUpdate multiSiteRefUpdate =
+ new MultiSiteRefUpdate(sharedRefDb, validationMetrics, A_TEST_PROJECT_NAME, refUpdate);
+ try {
+ multiSiteRefUpdate.delete();
+ fail("Expecting an IOException to be thrown");
+ } catch (IOException e) {
+ verify(validationMetrics).incrementSplitBrainRefUpdates();
+ }
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/DefaultSharedRefEnforcementTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/DefaultSharedRefEnforcementTest.java
new file mode 100644
index 0000000..e9597b8
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/DefaultSharedRefEnforcementTest.java
@@ -0,0 +1,97 @@
+// Copyright (C) 2019 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.
+// Copyright (C) 2018 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.multisite.validation.dfsrefdb.zookeeper;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.DefaultSharedRefEnforcement;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy;
+import java.io.IOException;
+import org.eclipse.jgit.lib.Ref;
+import org.junit.Test;
+
+public class DefaultSharedRefEnforcementTest implements RefFixture {
+
+ SharedRefDatabase refDb =
+ new SharedRefDatabase() {
+
+ @Override
+ public boolean compareAndRemove(String project, Ref oldRef) throws IOException {
+ return true;
+ }
+
+ @Override
+ public boolean compareAndPut(String project, Ref oldRef, Ref newRef) throws IOException {
+ return true;
+ }
+ };
+
+ SharedRefEnforcement refEnforcement = new DefaultSharedRefEnforcement();
+
+ @Test
+ public void anImmutableChangeShouldBeIgnored() {
+ Ref immutableChangeRef = refDb.newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1);
+ assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName()))
+ .isEqualTo(EnforcePolicy.IGNORED);
+ }
+
+ @Test
+ public void aChangeMetaShouldNotBeIgnored() {
+ Ref immutableChangeRef = refDb.newRef("refs/changes/01/1/meta", AN_OBJECT_ID_1);
+ assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName()))
+ .isEqualTo(EnforcePolicy.REQUIRED);
+ }
+
+ @Test
+ public void aDraftCommentsShouldBeIgnored() {
+ Ref immutableChangeRef = refDb.newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1);
+ assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName()))
+ .isEqualTo(EnforcePolicy.IGNORED);
+ }
+
+ @Test
+ public void regularRefHeadsMasterShouldNotBeIgnored() {
+ Ref immutableChangeRef = refDb.newRef("refs/heads/master", AN_OBJECT_ID_1);
+ assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName()))
+ .isEqualTo(EnforcePolicy.REQUIRED);
+ }
+
+ @Test
+ public void regularCommitShouldNotBeIgnored() {
+ Ref immutableChangeRef = refDb.newRef("refs/heads/stable-2.16", AN_OBJECT_ID_1);
+ assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName()))
+ .isEqualTo(EnforcePolicy.REQUIRED);
+ }
+
+ @Override
+ public String testBranch() {
+ return "fooBranch";
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java
index c3f3ace..2a26ae7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java
@@ -29,7 +29,9 @@
import static com.google.common.truth.Truth.assertThat;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.DefaultSharedRefEnforcement;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase;
+import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement;
import org.apache.curator.retry.RetryNTimes;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -44,12 +46,15 @@
ZookeeperTestContainerSupport zookeeperContainer;
ZkSharedRefDatabase zkSharedRefDatabase;
+ SharedRefEnforcement refEnforcement;
@Before
public void setup() {
+ refEnforcement = new DefaultSharedRefEnforcement();
zookeeperContainer = new ZookeeperTestContainerSupport(false);
zkSharedRefDatabase =
- new ZkSharedRefDatabase(zookeeperContainer.getCurator(), new RetryNTimes(5, 30));
+ new ZkSharedRefDatabase(
+ zookeeperContainer.getCurator(), new RetryNTimes(5, 30), refEnforcement);
}
@After
@@ -103,6 +108,21 @@
}
@Test
+ public void compareAndPutShouldSucceedIfTheObjectionHasNotTheExpectedValueWithDesiredEnforcement()
+ throws Exception {
+ String projectName = "All-Users";
+ String externalIds = "refs/meta/external-ids";
+
+ Ref oldRef = zkSharedRefDatabase.newRef(externalIds, AN_OBJECT_ID_1);
+ Ref expectedRef = zkSharedRefDatabase.newRef(externalIds, AN_OBJECT_ID_2);
+
+ zookeeperContainer.createRefInZk(projectName, oldRef);
+
+ assertThat(zkSharedRefDatabase.compareAndPut(projectName, expectedRef, refOf(AN_OBJECT_ID_3)))
+ .isTrue();
+ }
+
+ @Test
public void shouldCompareAndRemoveSuccessfully() throws Exception {
Ref oldRef = refOf(AN_OBJECT_ID_1);
String projectName = A_TEST_PROJECT_NAME;
@@ -159,37 +179,6 @@
}
@Test
- public void anImmutableChangeShouldBeIgnored() {
- Ref immutableChangeRef = zkSharedRefDatabase.newRef(A_REF_NAME_OF_A_PATCHSET, AN_OBJECT_ID_1);
- assertThat(zkSharedRefDatabase.ignoreRefInSharedDb(immutableChangeRef.getName())).isTrue();
- }
-
- @Test
- public void aChangeMetaShouldNotBeIgnored() {
- Ref immutableChangeRef = zkSharedRefDatabase.newRef("refs/changes/01/1/meta", AN_OBJECT_ID_1);
- assertThat(zkSharedRefDatabase.ignoreRefInSharedDb(immutableChangeRef.getName())).isFalse();
- }
-
- @Test
- public void aDraftCommentsShouldBeIgnored() {
- Ref immutableChangeRef =
- zkSharedRefDatabase.newRef("refs/draft-comments/01/1/1000000", AN_OBJECT_ID_1);
- assertThat(zkSharedRefDatabase.ignoreRefInSharedDb(immutableChangeRef.getName())).isTrue();
- }
-
- @Test
- public void regularRefHeadsMasterShouldNotBeIgnored() {
- Ref immutableChangeRef = zkSharedRefDatabase.newRef("refs/heads/master", AN_OBJECT_ID_1);
- assertThat(zkSharedRefDatabase.ignoreRefInSharedDb(immutableChangeRef.getName())).isFalse();
- }
-
- @Test
- public void regularCommitShouldNotBeIgnored() {
- Ref immutableChangeRef = zkSharedRefDatabase.newRef("refs/heads/stable-2.16", AN_OBJECT_ID_1);
- assertThat(zkSharedRefDatabase.ignoreRefInSharedDb(immutableChangeRef.getName())).isFalse();
- }
-
- @Test
public void compareAndPutShouldAlwaysIngoreAlwaysDraftCommentsEvenOutOfOrder() throws Exception {
// Test to reproduce a production bug where ignored refs were persisted in ZK because
// newRef == NULL
