computeAndPut should return false instead throwing exception
Return false instead of throwing `GlobalRefDbSystemError` when
global-refdb expected value doesn't match expected one. This
will match existing contract defined in global-refdb
`GlobalRefDatabase` interface[1].
1.https://gerrit.googlesource.com/modules/global-refdb/+/refs/heads/stable-3.4/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabase.java#76
Bug: Issue 297440085
Change-Id: I7b4beb1c107a2e54c7aa9bd4038042912a96bc3d
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
index 44822e9..f6bdddc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
@@ -132,11 +132,10 @@
project.get(), currValueForPath, newValueForPath);
return true;
} catch (ConditionalCheckFailedException e) {
- throw new GlobalRefDbSystemError(
- String.format(
- "Conditional Check Failure when updating refPath %s. expected: %s New: %s",
- refPath, currValueForPath, newValueForPath),
- e);
+ logger.atWarning().withCause(e).log(
+ "Conditional Check Failure when updating refPath %s. expected: %s New: %s",
+ refPath, currValueForPath, newValueForPath);
+ return false;
} catch (Exception e) {
throw new GlobalRefDbSystemError(
String.format(
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
index ba9b81d..b750c31 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
-import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.Configuration.DEFAULT_LOCKS_TABLE_NAME;
import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.Configuration.DEFAULT_REFS_DB_TABLE_NAME;
import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.DynamoDBRefDatabase.REF_DB_PRIMARY_KEY;
@@ -27,7 +26,6 @@
import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
import com.amazonaws.services.dynamodbv2.model.AttributeValue;
import com.amazonaws.services.dynamodbv2.model.PutItemRequest;
-import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestPlugin;
@@ -182,7 +180,7 @@
}
@Test
- public void compareAndPutShouldThrowWhenStoredRefIsNotExpected() {
+ public void compareAndPutShouldReturnFalseWhenStoredRefIsNotExpected() {
String refName = "refs/changes/01/01/meta";
String currentRefValue = "533d3ccf8a650fb26380faa732921a2c74924d5c";
String newRefValue = "9f6f2963cf44505428c61b935ff1ca65372cf28c";
@@ -190,20 +188,10 @@
Ref expectedRef = refOf(refName, expectedRefValue);
createRefInDynamoDB(project, refName, currentRefValue);
-
- GlobalRefDbSystemError thrown =
- assertThrows(
- GlobalRefDbSystemError.class,
- () ->
- dynamoDBRefDatabase()
- .compareAndPut(project, expectedRef, ObjectId.fromString(newRefValue)));
- assertThat(thrown)
- .hasMessageThat()
- .containsMatch(
- "Conditional Check Failure when updating refPath.*expected:.*"
- + expectedRefValue
- + " New:.*"
- + newRefValue);
+ assertThat(
+ dynamoDBRefDatabase()
+ .compareAndPut(project, expectedRef, ObjectId.fromString(newRefValue)))
+ .isFalse();
}
@Test
