Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: Add 'compareAndPut' and 'get' for generic data type Change-Id: Ia767ed3bcb1cb1c673ad6ee983444a154611af98
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl index 058fbe7..5280f73 100644 --- a/external_plugin_deps.bzl +++ b/external_plugin_deps.bzl
@@ -35,6 +35,6 @@ maven_jar( name = "global-refdb", - artifact = "com.gerritforge:global-refdb:3.1.0-rc1", - sha1 = "61fc8defaed9c364e6bfa101563e434fcc70038f", + artifact = "com.gerritforge:global-refdb:3.1.2", + sha1 = "6ddee3de0f3fe9254453118ae1eca481ec03e957", )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/DeserializerException.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/DeserializerException.java new file mode 100644 index 0000000..21f7a94 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/DeserializerException.java
@@ -0,0 +1,22 @@ +// Copyright (C) 2020 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.validation.dfsrefdb.zookeeper; + +public class DeserializerException extends Exception { + + public DeserializerException(String message) { + super(message); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringDeserializer.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringDeserializer.java new file mode 100644 index 0000000..7b73e36 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringDeserializer.java
@@ -0,0 +1,22 @@ +// Copyright (C) 2020 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.validation.dfsrefdb.zookeeper; + +public interface StringDeserializer<T> { + + T fromString(String str); + + Class<T> getTypeClass(); +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringDeserializerFactory.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringDeserializerFactory.java new file mode 100644 index 0000000..e7f01df --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringDeserializerFactory.java
@@ -0,0 +1,37 @@ +// Copyright (C) 2020 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.validation.dfsrefdb.zookeeper; + +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.inject.Inject; + +public class StringDeserializerFactory { + + private final DynamicSet<StringDeserializer> stringToGenericDeserializers; + + @Inject + public StringDeserializerFactory(DynamicSet<StringDeserializer> stringToGenericDeserializers) { + this.stringToGenericDeserializers = stringToGenericDeserializers; + } + + public StringDeserializer create(final Class clazz) throws DeserializerException { + for (StringDeserializer stringDeserializer : stringToGenericDeserializers) { + if (stringDeserializer.getTypeClass().getName().equals(clazz.getTypeName())) { + return stringDeserializer; + } + } + throw new DeserializerException("No serializer registered for class " + clazz.getName()); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToIntDeserializer.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToIntDeserializer.java new file mode 100644 index 0000000..e455493 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToIntDeserializer.java
@@ -0,0 +1,27 @@ +// Copyright (C) 2020 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.validation.dfsrefdb.zookeeper; + +public class StringToIntDeserializer implements StringDeserializer<Integer> { + + public Class<Integer> getTypeClass() { + return Integer.class; + } + + @SuppressWarnings("unckecked") + public Integer fromString(String str) { + return Integer.parseInt(str); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToLongDeserializer.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToLongDeserializer.java new file mode 100644 index 0000000..0d21a5b --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToLongDeserializer.java
@@ -0,0 +1,27 @@ +// Copyright (C) 2020 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.validation.dfsrefdb.zookeeper; + +public class StringToLongDeserializer implements StringDeserializer<Long> { + + public Class<Long> getTypeClass() { + return Long.class; + } + + @SuppressWarnings("unckecked") + public Long fromString(String str) { + return Long.parseLong(str); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToObjectIdDeserializer.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToObjectIdDeserializer.java new file mode 100644 index 0000000..9a61844 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/StringToObjectIdDeserializer.java
@@ -0,0 +1,29 @@ +// Copyright (C) 2020 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.validation.dfsrefdb.zookeeper; + +import org.eclipse.jgit.lib.ObjectId; + +public class StringToObjectIdDeserializer implements StringDeserializer<ObjectId> { + + public Class<ObjectId> getTypeClass() { + return ObjectId.class; + } + + @SuppressWarnings("unckecked") + public ObjectId fromString(String str) { + return ObjectId.fromString(str); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java index 04dc067..6908487 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
@@ -23,6 +23,7 @@ import com.google.gerrit.entities.Project; import com.google.inject.Inject; import java.nio.charset.StandardCharsets; +import java.util.Optional; import org.apache.curator.RetryPolicy; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.atomic.AtomicValue; @@ -39,12 +40,17 @@ private final RetryPolicy retryPolicy; private final Long transactionLockTimeOut; + private StringDeserializerFactory stringDeserializerFactory; @Inject - public ZkSharedRefDatabase(CuratorFramework client, ZkConnectionConfig connConfig) { + public ZkSharedRefDatabase( + CuratorFramework client, + ZkConnectionConfig connConfig, + StringDeserializerFactory stringDeserializerFactory) { this.client = client; this.retryPolicy = connConfig.curatorRetryPolicy; this.transactionLockTimeOut = connConfig.transactionLockTimeout; + this.stringDeserializerFactory = stringDeserializerFactory; } @Override @@ -114,7 +120,7 @@ throws GlobalRefDbSystemError { final DistributedAtomicValue distributedRefValue = - new DistributedAtomicValue(client, pathFor(projectName, oldRef), retryPolicy); + new DistributedAtomicValue(client, pathFor(projectName, oldRef.getName()), retryPolicy); try { if ((oldRef.getObjectId() == null || oldRef.getObjectId().equals(ObjectId.zeroId())) @@ -133,18 +139,68 @@ return newDistributedValue.succeeded(); } catch (Exception e) { logger.atWarning().withCause(e).log( - "Error trying to perform CAS at path %s", pathFor(projectName, oldRef)); + "Error trying to perform CAS at path %s", pathFor(projectName, oldRef.getName())); throw new GlobalRefDbSystemError( - String.format("Error trying to perform CAS at path %s", pathFor(projectName, oldRef)), e); + String.format( + "Error trying to perform CAS at path %s", pathFor(projectName, oldRef.getName())), + e); + } + } + + public <T> boolean compareAndPut( + Project.NameKey project, String refName, T expectedValue, T newValue) + throws GlobalRefDbSystemError { + + final DistributedAtomicValue distributedRefValue = + new DistributedAtomicValue(client, pathFor(project, refName), retryPolicy); + + try { + if (expectedValue == null && refNotInZk(project, refName)) { + return distributedRefValue.initialize(writeGeneric(newValue)); + } + + final AtomicValue<byte[]> newDistributedValue = + distributedRefValue.compareAndSet(writeGeneric(expectedValue), writeGeneric(newValue)); + + return newDistributedValue.succeeded(); + } catch (Exception e) { + String message = + String.format( + "Error trying to perform CAS of generic value at path %s", pathFor(project, refName)); + logger.atWarning().withCause(e).log(message); + throw new GlobalRefDbSystemError(message, e); + } + } + + @Override + public <T> Optional<T> get(Project.NameKey project, String refName, Class<T> clazz) + throws GlobalRefDbSystemError { + if (!exists(project, refName)) { + return Optional.empty(); + } + + try { + final byte[] valueInZk = client.getData().forPath(pathFor(project, refName)); + + if (valueInZk == null) { + logger.atInfo().log("%s:%s not found in Zookeeper", project, refName); + return Optional.empty(); + } + + return Optional.of(readGenericType(valueInZk, clazz)); + + } catch (Exception e) { + logger.atSevere().withCause(e).log("Cannot get value for %s:%s", project, refName); + return Optional.empty(); } } private boolean refNotInZk(Project.NameKey projectName, Ref oldRef) throws Exception { - return client.checkExists().forPath(pathFor(projectName, oldRef)) == null; + return client.checkExists().forPath(pathFor(projectName, oldRef.getName())) == null; } - static String pathFor(Project.NameKey projectName, Ref oldRef) { - return pathFor(projectName, oldRef.getName()); + private boolean refNotInZk(Project.NameKey projectName, String oldRef) throws Exception { + return client.checkExists().forPath(pathFor(projectName, oldRef)) == null; } static String pathFor(Project.NameKey projectName, String refName) { @@ -155,7 +211,18 @@ return ObjectId.fromString(value, 0); } + @SuppressWarnings("unchecked") + <T> T readGenericType(byte[] value, Class<T> clazz) throws DeserializerException { + StringDeserializer stringDeserializer = stringDeserializerFactory.create(clazz); + String str = new String(value, StandardCharsets.US_ASCII); + return (T) stringDeserializer.fromString(str); + } + static byte[] writeObjectId(ObjectId value) { return ObjectId.toString(value).getBytes(StandardCharsets.US_ASCII); } + + static <T> byte[] writeGeneric(T value) { + return value.toString().getBytes(StandardCharsets.US_ASCII); + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java index bbbddc9..ddd5398 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java
@@ -17,6 +17,7 @@ import com.gerritforge.gerrit.globalrefdb.GlobalRefDatabase; import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.extensions.registration.DynamicSet; import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.Scopes; @@ -42,5 +43,16 @@ bind(ZkConnectionConfig.class) .toInstance( new ZkConnectionConfig(cfg.buildCasRetryPolicy(), cfg.getZkInterProcessLockTimeOut())); + + DynamicSet.setOf(binder(), StringDeserializer.class); + DynamicSet.bind(binder(), StringDeserializer.class) + .to(StringToIntDeserializer.class) + .in(Scopes.SINGLETON); + DynamicSet.bind(binder(), StringDeserializer.class) + .to(StringToLongDeserializer.class) + .in(Scopes.SINGLETON); + DynamicSet.bind(binder(), StringDeserializer.class) + .to(StringToObjectIdDeserializer.class) + .in(Scopes.SINGLETON); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java index 3ee4612..9ce3080 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseTest.java
@@ -17,6 +17,8 @@ import static com.google.common.truth.Truth.assertThat; import com.google.gerrit.entities.Project; +import com.google.gerrit.extensions.registration.DynamicSet; +import java.util.Optional; import org.apache.curator.retry.RetryNTimes; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdRef; @@ -34,6 +36,17 @@ private ZkSharedRefDatabase zkSharedRefDatabase; + private StringDeserializerFactory stringDeserializerFactory = + new StringDeserializerFactory(asDynamicSet()); + + private DynamicSet<StringDeserializer> asDynamicSet() { + DynamicSet<StringDeserializer> result = new DynamicSet<>(); + result.add("zookeeper", new StringToLongDeserializer()); + result.add("zookeeper", new StringToIntDeserializer()); + result.add("zookeeper", new StringToObjectIdDeserializer()); + return result; + } + @Before public void setup() { zookeeperContainer = new ZookeeperTestContainerSupport(); @@ -46,7 +59,8 @@ zookeeperContainer.getCurator(), new ZkConnectionConfig( new RetryNTimes(NUMBER_OF_RETRIES, SLEEP_BETWEEN_RETRIES_MS), - TRANSACTION_LOCK_TIMEOUT)); + TRANSACTION_LOCK_TIMEOUT), + stringDeserializerFactory); } @After @@ -67,6 +81,44 @@ } @Test + public void shouldCompareAndPutGenericSuccessfullyNewEntry() { + assertThat( + zkSharedRefDatabase.compareAndPut( + A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, null, new Object())) + .isTrue(); + } + + @Test + public void shouldFailCompareAndPutGenericIfOutOfSync() { + Object object1 = new Object(); + assertThat( + zkSharedRefDatabase.compareAndPut( + A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, null, object1)) + .isTrue(); + + Object object2 = new Object(); + Object object3 = new Object(); + assertThat( + zkSharedRefDatabase.compareAndPut( + A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, object2, object3)) + .isFalse(); + } + + @Test + public void shouldCompareAndPutGenericSuccessfullyUpdateEntry() { + Object object1 = new Object(); + assertThat( + zkSharedRefDatabase.compareAndPut( + A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, null, object1)) + .isTrue(); + Object object2 = new Object(); + assertThat( + zkSharedRefDatabase.compareAndPut( + A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, object1, object2)) + .isTrue(); + } + + @Test public void shouldFetchLatestObjectIdInZk() throws Exception { Ref oldRef = refOf(AN_OBJECT_ID_1); Ref newRef = refOf(AN_OBJECT_ID_2); @@ -138,6 +190,62 @@ assertThat(getNumChildrenForPath("/")).isEqualTo(0); } + @Test + public void shouldReturnIntValueIfExists() throws Exception { + zkSharedRefDatabase.compareAndPut(A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, null, 1); + assertThat( + zkSharedRefDatabase + .get(A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, Integer.class) + .isPresent()) + .isTrue(); + assertThat( + zkSharedRefDatabase.<Integer>get( + A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, Integer.class)) + .isEqualTo(Optional.of(1)); + } + + @Test + public void shouldReturnLongValueIfExists() throws Exception { + zkSharedRefDatabase.compareAndPut(A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, null, 1L); + assertThat( + zkSharedRefDatabase + .get(A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, Long.class) + .isPresent()) + .isTrue(); + assertThat(zkSharedRefDatabase.<Long>get(A_TEST_PROJECT_NAME_KEY, A_TEST_REF_NAME, Long.class)) + .isEqualTo(Optional.of(1L)); + } + + @Test + public void shouldReturnObjectIdValueIfExists() throws Exception { + Ref ref = refOf(AN_OBJECT_ID_1); + + zookeeperContainer.createRefInZk(A_TEST_PROJECT_NAME_KEY, ref); + + assertThat( + zkSharedRefDatabase + .get(A_TEST_PROJECT_NAME_KEY, ref.getName(), ObjectId.class) + .isPresent()) + .isTrue(); + assertThat( + zkSharedRefDatabase.<ObjectId>get( + A_TEST_PROJECT_NAME_KEY, ref.getName(), ObjectId.class)) + .isEqualTo(Optional.of(AN_OBJECT_ID_1)); + } + + @Test + public void shouldReturnEmptyIfDoesntExists() throws Exception { + Ref ref = refOf(AN_OBJECT_ID_1); + Project.NameKey projectName = A_TEST_PROJECT_NAME_KEY; + + zookeeperContainer.createRefInZk(projectName, ref); + + assertThat(zkSharedRefDatabase.compareAndPut(projectName, ref, ObjectId.zeroId())).isTrue(); + + Ref zerosRef = refOf(ObjectId.zeroId()); + assertThat(zkSharedRefDatabase.compareAndPut(projectName, zerosRef, AN_OBJECT_ID_1)).isTrue(); + } + @Override public String testBranch() { return "branch_" + nameRule.getMethodName();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java index 066aeb7..59b7afd 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java +++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java
@@ -80,7 +80,7 @@ } public ObjectId readRefValueFromZk(Project.NameKey projectName, Ref ref) throws Exception { - final byte[] bytes = curator.getData().forPath(pathFor(projectName, ref)); + final byte[] bytes = curator.getData().forPath(pathFor(projectName, ref.getName())); return ZkSharedRefDatabase.readObjectId(bytes); } @@ -88,6 +88,6 @@ curator .create() .creatingParentContainersIfNeeded() - .forPath(pathFor(projectName, ref), writeObjectId(ref.getObjectId())); + .forPath(pathFor(projectName, ref.getName()), writeObjectId(ref.getObjectId())); } }