Merge branch 'stable-3.10' into stable-3.11 * stable-3.10: Fix rollback implementation Change-Id: I8125447a374f170066b4c32c13352dbee430b602
diff --git a/README.md b/README.md index 81da677..e101a1a 100644 --- a/README.md +++ b/README.md
@@ -31,11 +31,14 @@ Example: +``` git clone https://gerrit.googlesource.com/gerrit git clone https://gerrit.googlesource.com/modules/global-refdb cd gerrit/plugins ln -s ../../global-refdb . -From the Gerrit source tree issue the command bazelsk build plugins/global-refdb +``` + +From the Gerrit source tree issue the command `bazelisk build plugins/global-refdb` Example: @@ -43,9 +46,9 @@ bazelisk build plugins/global-refdb ``` -The libModule jar file is created under basel-bin/plugins/global-refdb/global-refdb.jar +The libModule jar file is created under `bazel-bin/plugins/global-refdb/global-refdb.jar` -To execute the tests run bazelisk test plugins/global-refdb/... from the Gerrit source tree. +To execute the tests run `bazelisk test plugins/global-refdb/...` from the Gerrit source tree. Example:
diff --git a/bindings.md b/bindings.md index 920b911..c936c5d 100644 --- a/bindings.md +++ b/bindings.md
@@ -41,6 +41,7 @@ .to(CustomSharedRefEnforcementByProject.class) .in(Scopes.SINGLETON); } + DynamicSet.bind(binder(), ExceptionHook.class).to(SharedRefDbExceptionHook.class); } } ```
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java index 27ac348..d4157f7 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java
@@ -14,8 +14,10 @@ package com.gerritforge.gerrit.globalrefdb; +import com.google.gerrit.git.LockFailureException; + /** {@code RefDbLockException} is an exception that can be thrown when trying to lock a ref. */ -public class RefDbLockException extends RuntimeException { +public class RefDbLockException extends LockFailureException { private static final long serialVersionUID = 1L; /** @@ -30,6 +32,8 @@ } public RefDbLockException(String project, String refName, String message) { - super(String.format("Unable to lock ref %s on project %s: %s", refName, project, message)); + super( + String.format("Unable to lock ref %s on project %s: %s", refName, project, message), + (Throwable) null); } }
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java index 75a07d0..4a28687 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
@@ -137,22 +137,24 @@ return; } - List<RefPair> refsToUpdate = getRefPairs(commands).collect(Collectors.toList()); - List<RefPair> refsFailures = - refsToUpdate.stream().filter(RefPair::hasFailed).collect(Collectors.toList()); + List<RefUpdateSnapshot> refsToUpdate = + getRefUpdateSnapshots(commands).collect(Collectors.toList()); + List<RefUpdateSnapshot> refsFailures = + refsToUpdate.stream().filter(RefUpdateSnapshot::hasFailed).collect(Collectors.toList()); if (!refsFailures.isEmpty()) { String allFailuresMessage = refsFailures.stream() - .map(refPair -> String.format("Failed to fetch ref %s", refPair.compareRef.getName())) + .map(refSnapshot -> String.format("Failed to fetch ref %s", refSnapshot.getName())) .collect(Collectors.joining(", ")); - Exception firstFailureException = refsFailures.get(0).exception; + Exception firstFailureException = refsFailures.get(0).getException(); logger.atSevere().withCause(firstFailureException).log("%s", allFailuresMessage); throw new IOException(allFailuresMessage, firstFailureException); } try (CloseableSet<AutoCloseable> locks = new CloseableSet<>()) { - final List<RefPair> finalRefsToUpdate = compareAndGetLatestLocalRefs(refsToUpdate, locks); + final List<RefUpdateSnapshot> finalRefsToUpdate = + compareAndGetLatestLocalRefs(refsToUpdate, locks); delegateUpdate.invoke(); boolean sharedDbUpdateSucceeded = false; try { @@ -182,7 +184,7 @@ private void rollback( OneParameterVoidFunction<List<ReceiveCommand>> delegateUpdateRollback, - List<RefPair> refsBeforeUpdate, + List<RefUpdateSnapshot> refsBeforeUpdate, List<ReceiveCommand> receiveCommands) throws IOException { List<ReceiveCommand> rollbackCommands = @@ -190,61 +192,62 @@ .map( refBeforeUpdate -> new ReceiveCommand( - refBeforeUpdate.putValue, - refBeforeUpdate.compareRef.getObjectId(), + refBeforeUpdate.getNewValue(), + refBeforeUpdate.getOldValue(), refBeforeUpdate.getName())) .collect(Collectors.toList()); delegateUpdateRollback.invoke(rollbackCommands); receiveCommands.forEach(command -> command.setResult(ReceiveCommand.Result.LOCK_FAILURE)); } - private void updateSharedRefDb(Stream<ReceiveCommand> commandStream, List<RefPair> refsToUpdate) + private void updateSharedRefDb( + Stream<ReceiveCommand> commandStream, List<RefUpdateSnapshot> refsToUpdate) throws IOException { if (commandStream.anyMatch(cmd -> cmd.getResult() != ReceiveCommand.Result.OK)) { return; } - for (RefPair refPair : refsToUpdate) { - updateSharedDbOrThrowExceptionFor(refPair); + for (RefUpdateSnapshot refUpdateSnapshot : refsToUpdate) { + updateSharedDbOrThrowExceptionFor(refUpdateSnapshot); } } - private Stream<RefPair> getRefPairs(List<ReceiveCommand> receivedCommands) { - return receivedCommands.stream().map(this::getRefPairForCommand); + private Stream<RefUpdateSnapshot> getRefUpdateSnapshots(List<ReceiveCommand> receivedCommands) { + return receivedCommands.stream().map(this::getRefUpdateSnapshotForCommand); } - private RefPair getRefPairForCommand(ReceiveCommand command) { + private RefUpdateSnapshot getRefUpdateSnapshotForCommand(ReceiveCommand command) { try { switch (command.getType()) { case CREATE: - return new RefPair(nullRef(command.getRefName()), getNewRef(command)); + return new RefUpdateSnapshot(nullRef(command.getRefName()), getNewValue(command)); case UPDATE: case UPDATE_NONFASTFORWARD: - return new RefPair(getCurrentRef(command.getRefName()), getNewRef(command)); + return new RefUpdateSnapshot(getCurrentRef(command.getRefName()), getNewValue(command)); case DELETE: - return new RefPair(getCurrentRef(command.getRefName()), ObjectId.zeroId()); + return new RefUpdateSnapshot(getCurrentRef(command.getRefName()), ObjectId.zeroId()); default: - return new RefPair( + return new RefUpdateSnapshot( command.getRef(), new IllegalArgumentException("Unsupported command type " + command.getType())); } } catch (IOException e) { - return new RefPair(command.getRef(), e); + return new RefUpdateSnapshot(command.getRef(), e); } } - private ObjectId getNewRef(ReceiveCommand command) { + private ObjectId getNewValue(ReceiveCommand command) { return command.getNewId(); } - private List<RefPair> compareAndGetLatestLocalRefs( - List<RefPair> refsToUpdate, CloseableSet<AutoCloseable> locks) throws IOException { - List<RefPair> latestRefsToUpdate = new ArrayList<>(); - for (RefPair refPair : refsToUpdate) { - latestRefsToUpdate.add(compareAndGetLatestLocalRef(refPair, locks)); + private List<RefUpdateSnapshot> compareAndGetLatestLocalRefs( + List<RefUpdateSnapshot> refsToUpdate, CloseableSet<AutoCloseable> locks) throws IOException { + List<RefUpdateSnapshot> latestRefsToUpdate = new ArrayList<>(); + for (RefUpdateSnapshot refUpdateSnapshot : refsToUpdate) { + latestRefsToUpdate.add(compareAndGetLatestLocalRef(refUpdateSnapshot, locks)); } return latestRefsToUpdate; }
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefPair.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefPair.java deleted file mode 100644 index 4211c0c..0000000 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefPair.java +++ /dev/null
@@ -1,76 +0,0 @@ -// 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.gerritforge.gerrit.globalrefdb.validation; - -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Ref; - -/** - * A convenience object encompassing the old (current) and the new (candidate) value of a {@link - * Ref}. This is used to snapshot the current status of a ref update so that validations against the - * global refdb are unaffected by changes on the {@link org.eclipse.jgit.lib.RefDatabase}. - */ -public class RefPair { - public final Ref compareRef; - public final ObjectId putValue; - public final Exception exception; - - /** - * Constructs a {@code RefPair} with the provided old and new ref values. The oldRef value is - * required not to be null, in which case an {@link IllegalArgumentException} is thrown. - * - * @param oldRef the old ref - * @param newRefValue the new (candidate) value for this ref. - */ - RefPair(Ref oldRef, ObjectId newRefValue) { - if (oldRef == null) { - throw new IllegalArgumentException("Required not-null ref in RefPair"); - } - this.compareRef = oldRef; - this.putValue = newRefValue; - this.exception = null; - } - - /** - * Constructs a {@code RefPair} with the current ref and an Exception indicating why the new ref - * value failed being retrieved. - * - * @param newRef - * @param e - */ - RefPair(Ref newRef, Exception e) { - this.compareRef = newRef; - this.exception = e; - this.putValue = ObjectId.zeroId(); - } - - /** - * Getter for the current ref - * - * @return the current ref value - */ - public String getName() { - return compareRef.getName(); - } - - /** - * Whether the new value failed being retrieved - * - * @return true when this refPair has failed, false otherwise. - */ - public boolean hasFailed() { - return exception != null; - } -}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateSnapshot.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateSnapshot.java new file mode 100644 index 0000000..2e83be3 --- /dev/null +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateSnapshot.java
@@ -0,0 +1,114 @@ +// 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.gerritforge.gerrit.globalrefdb.validation; + +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; + +/** + * A convenience object encompassing the old (current) and the new (candidate) value of a {@link + * Ref}. This is used to snapshot the current status of a ref update so that validations against the + * global refdb are unaffected by changes on the underlying {@link + * org.eclipse.jgit.lib.RefDatabase}. + */ +class RefUpdateSnapshot { + private final Ref ref; + private final ObjectId newValue; + private final Exception exception; + + /** + * Constructs a {@code RefUpdateSnapshot} with the provided old and new values. The old value of + * this Ref must not be null, otherwise an {@link IllegalArgumentException} is thrown. + * + * @param ref the ref with its old value + * @param newRefValue the new (candidate) value for this ref. + */ + RefUpdateSnapshot(Ref ref, ObjectId newRefValue) { + if (ref == null) { + throw new IllegalArgumentException("RefUpdateSnapshot cannot be created for null Ref"); + } + this.ref = ref; + this.newValue = newRefValue; + this.exception = null; + } + + /** + * Constructs a {@code RefUpdateSnapshot} with the current ref and an Exception indicating why the + * ref's new value could not be retrieved. + * + * @param ref the ref with its old value + * @param e exception caught when trying to retrieve the ref's new value + */ + RefUpdateSnapshot(Ref ref, Exception e) { + this.ref = ref; + this.newValue = ObjectId.zeroId(); + this.exception = e; + } + + /** + * Get the ref name + * + * @return the ref name + */ + public String getName() { + return ref.getName(); + } + + /** + * Get the ref's old value + * + * @return the ref's old value + */ + public ObjectId getOldValue() { + return ref.getObjectId(); + } + + /** + * Get the ref's new (candidate) value + * + * @return the ref's new (candidate) value + */ + public ObjectId getNewValue() { + return newValue; + } + + /** + * Get the snapshotted ref with its old value + * + * @return the snapshotted ref with its old value + */ + public Ref getRef() { + return ref; + } + + /** + * Get the exception which occurred when retrieving the ref's new value + * + * @return the exception which occurred when retrieving the ref's new value + */ + public Exception getException() { + return exception; + } + + /** + * Whether retrieving the new (candidate) value failed + * + * @return {@code true} when retrieving the ref's new (candidate) value failed, {@code false} + * otherwise. + */ + public boolean hasFailed() { + return exception != null; + } +}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java index 50986da..895a7f4 100644 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
@@ -14,13 +14,13 @@ package com.gerritforge.gerrit.globalrefdb.validation; +import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException; import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError; import com.gerritforge.gerrit.globalrefdb.RefDbLockException; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.CustomSharedRefEnforcementByProject; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.OutOfSyncException; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedDbSplitBrainException; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedLockException; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy; import com.google.common.base.MoreObjects; @@ -184,16 +184,16 @@ OneParameterFunction<ObjectId, Result> rollbackFunction) throws IOException { try (CloseableSet<AutoCloseable> locks = new CloseableSet<>()) { - RefPair refPairForUpdate = newRefPairFrom(refUpdate); - compareAndGetLatestLocalRef(refPairForUpdate, locks); + RefUpdateSnapshot refUpdateSnapshot = newSnapshot(refUpdate); + compareAndGetLatestLocalRef(refUpdateSnapshot, locks); RefUpdate.Result result = refUpdateFunction.invoke(); if (!isSuccessful(result)) { return result; } try { - updateSharedDbOrThrowExceptionFor(refPairForUpdate); + updateSharedDbOrThrowExceptionFor(refUpdateSnapshot); } catch (Exception e) { - result = rollbackFunction.invoke(refPairForUpdate.compareRef.getObjectId()); + result = rollbackFunction.invoke(refUpdateSnapshot.getOldValue()); if (isSuccessful(result)) { result = RefUpdate.Result.LOCK_FAILURE; } @@ -208,30 +208,33 @@ } catch (OutOfSyncException e) { logger.atWarning().withCause(e).log( "Local node is out of sync with ref-db: %s", e.getMessage()); - return RefUpdate.Result.LOCK_FAILURE; } } - protected void updateSharedDbOrThrowExceptionFor(RefPair refPair) throws IOException { + protected void updateSharedDbOrThrowExceptionFor(RefUpdateSnapshot refSnapshot) + throws IOException { // We are not checking refs that should be ignored final EnforcePolicy refEnforcementPolicy = - refEnforcement.getPolicy(projectName, refPair.getName()); + refEnforcement.getPolicy(projectName, refSnapshot.getName()); if (refEnforcementPolicy == EnforcePolicy.IGNORED) return; boolean succeeded; try { if (!sharedRefDb.isNoop()) { ObjectId localObjectId = - Optional.ofNullable(refDb.findRef(refPair.compareRef.getName())) + Optional.ofNullable(refDb.findRef(refSnapshot.getName())) .map(Ref::getObjectId) .orElse(ObjectId.zeroId()); - if (!localObjectId.equals(refPair.putValue)) { + if (!localObjectId.equals(refSnapshot.getNewValue())) { String error = String.format( "Aborting the global-refdb update of %s = %s: local ref value is %s instead of" + " the expected value %s", - refPair.getName(), refPair.putValue, localObjectId.name(), refPair.putValue); + refSnapshot.getName(), + refSnapshot.getNewValue(), + localObjectId.name(), + refSnapshot.getNewValue()); logger.atSevere().log("%s", error); throw new IOException(error); } @@ -239,12 +242,12 @@ succeeded = sharedRefDb.compareAndPut( - Project.nameKey(projectName), refPair.compareRef, refPair.putValue); + Project.nameKey(projectName), refSnapshot.getRef(), refSnapshot.getNewValue()); } catch (GlobalRefDbSystemError e) { logger.atWarning().withCause(e).log( "Not able to persist the data in global-refdb for project '%s', ref '%s' and value %s," + " message: %s", - projectName, refPair.getName(), refPair.putValue, e.getMessage()); + projectName, refSnapshot.getName(), refSnapshot.getNewValue(), e.getMessage()); throw e; } @@ -254,17 +257,18 @@ "Not able to persist the data in SharedRef for project '%s' and ref '%s'," + "the cluster is now in Split Brain since the commit has been " + "persisted locally but not in global-refdb the value %s", - projectName, refPair.getName(), refPair.putValue); + projectName, refSnapshot.getName(), refSnapshot.getNewValue()); throw new SharedDbSplitBrainException(errorMessage); } } - protected RefPair compareAndGetLatestLocalRef(RefPair refPair, CloseableSet<AutoCloseable> locks) - throws SharedLockException, OutOfSyncException, IOException { - String refName = refPair.getName(); + protected RefUpdateSnapshot compareAndGetLatestLocalRef( + RefUpdateSnapshot refUpdateSnapshot, CloseableSet<AutoCloseable> locks) + throws GlobalRefDbLockException, OutOfSyncException, IOException { + String refName = refUpdateSnapshot.getName(); EnforcePolicy refEnforcementPolicy = refEnforcement.getPolicy(projectName, refName); if (refEnforcementPolicy == EnforcePolicy.IGNORED) { - return refPair; + return refUpdateSnapshot; } String sharedLockKey = String.format("%s:%s", projectName, refName); @@ -273,29 +277,32 @@ locks.addResourceIfNotExist(localLockKey, () -> sharedRefDb.lockLocalRef(projectKey, refName)); locks.addResourceIfNotExist(sharedLockKey, () -> sharedRefDb.lockRef(projectKey, refName)); - RefPair latestRefPair = getLatestLocalRef(refPair); - if (sharedRefDb.isUpToDate(projectKey, latestRefPair.compareRef)) { - return latestRefPair; + RefUpdateSnapshot latestRefUpdateSnapshot = getLatestLocalRef(refUpdateSnapshot); + if (sharedRefDb.isUpToDate(projectKey, latestRefUpdateSnapshot.getRef())) { + return latestRefUpdateSnapshot; } - if (isNullRef(latestRefPair.compareRef) || sharedRefDb.exists(projectKey, refName)) { + if (isNullRef(latestRefUpdateSnapshot.getRef()) || sharedRefDb.exists(projectKey, refName)) { validationMetrics.incrementSplitBrainPrevention(); softFailBasedOnEnforcement( - new OutOfSyncException(projectName, latestRefPair.compareRef), refEnforcementPolicy); + new OutOfSyncException(projectName, latestRefUpdateSnapshot.getRef()), + refEnforcementPolicy); } - return latestRefPair; + return latestRefUpdateSnapshot; } private boolean isNullRef(Ref ref) { return ref.getObjectId().equals(ObjectId.zeroId()); } - private RefPair getLatestLocalRef(RefPair refPair) throws IOException { - Ref latestRef = refDb.exactRef(refPair.getName()); - return new RefPair( - latestRef == null ? nullRef(refPair.getName()) : latestRef, refPair.putValue); + private RefUpdateSnapshot getLatestLocalRef(RefUpdateSnapshot refUpdateSnapshot) + throws IOException { + Ref latestRef = refDb.exactRef(refUpdateSnapshot.getName()); + return new RefUpdateSnapshot( + latestRef == null ? nullRef(refUpdateSnapshot.getName()) : latestRef, + refUpdateSnapshot.getNewValue()); } private Ref nullRef(String name) { @@ -323,8 +330,8 @@ } } - protected RefPair newRefPairFrom(RefUpdate refUpdate) throws IOException { - return new RefPair(getCurrentRef(refUpdate.getName()), refUpdate.getNewObjectId()); + protected RefUpdateSnapshot newSnapshot(RefUpdate refUpdate) throws IOException { + return new RefUpdateSnapshot(getCurrentRef(refUpdate.getName()), refUpdate.getNewObjectId()); } protected Ref getCurrentRef(String refName) throws IOException { @@ -343,8 +350,8 @@ } public void addResourceIfNotExist( - String key, ExceptionThrowingSupplier<T, SharedLockException> resourceFactory) - throws SharedLockException { + String key, ExceptionThrowingSupplier<T, RefDbLockException> resourceFactory) + throws RefDbLockException { if (!elements.containsKey(key)) { elements.put(key, resourceFactory.create()); }
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbExceptionHook.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbExceptionHook.java new file mode 100644 index 0000000..a7f2740 --- /dev/null +++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbExceptionHook.java
@@ -0,0 +1,50 @@ +// Copyright (C) 2024 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.gerritforge.gerrit.globalrefdb.validation; + +import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException; +import com.google.common.collect.ImmutableList; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.server.ExceptionHook; +import java.util.Optional; + +public class SharedRefDbExceptionHook implements ExceptionHook { + + @Override + public boolean shouldRetry(String actionType, String actionName, Throwable throwable) { + return throwable instanceof GlobalRefDbLockException; + } + + @Override + public Optional<Status> getStatus(Throwable throwable) { + if (throwable instanceof GlobalRefDbLockException) { + return Optional.of(Status.create(503, "Lock failure")); + } + return Optional.empty(); + } + + @Override + public ImmutableList<String> getUserMessages(Throwable throwable, @Nullable String traceId) { + if (throwable instanceof GlobalRefDbLockException) { + ImmutableList.Builder<String> builder = new ImmutableList.Builder<>(); + builder.add(throwable.getMessage()); + if (traceId != null && !traceId.isBlank()) { + builder.add(String.format("Trace ID: %s", traceId)); + } + return builder.build(); + } + return ImmutableList.of(); + } +}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedLockException.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedLockException.java deleted file mode 100644 index 5963da2..0000000 --- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedLockException.java +++ /dev/null
@@ -1,34 +0,0 @@ -// 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.gerritforge.gerrit.globalrefdb.validation.dfsrefdb; - -import java.io.IOException; - -/** Unable to lock a project/ref resource. */ -public class SharedLockException extends IOException { - private static final long serialVersionUID = 1L; - - /** - * Constructs a {@code SharedLockException} exception with the cause of failing to lock a - * project/ref resource - * - * @param project the project the lock is being acquired for - * @param refName the ref the project is being acquired for - * @param cause the cause of the failure - */ - public SharedLockException(String project, String refName, Exception cause) { - super(String.format("Unable to lock project %s on ref %s", project, refName), cause); - } -}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabaseTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabaseTest.java index 8ed1f4c..6284f42 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabaseTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabaseTest.java
@@ -95,26 +95,28 @@ } @Test - public void shouldReturnIsUpToDateWhenProjectDoesNotExistsInTheGlobalRefDB() { + public void shouldReturnIsUpToDateWhenProjectDoesNotExistsInTheGlobalRefDB() + throws GlobalRefDbLockException { assertThat(objectUnderTest.isUpToDate(project, initialRef)).isTrue(); } @Test - public void shouldReturnIsUpToDate() { + public void shouldReturnIsUpToDate() throws GlobalRefDbLockException { objectUnderTest.compareAndPut(project, nullRef, objectId1); assertThat(objectUnderTest.isUpToDate(project, ref1)).isTrue(); } @Test - public void shouldReturnIsNotUpToDateWhenLocalRepoIsOutdated() { + public void shouldReturnIsNotUpToDateWhenLocalRepoIsOutdated() throws GlobalRefDbLockException { objectUnderTest.compareAndPut(project, nullRef, objectId1); assertThat(objectUnderTest.isUpToDate(project, nullRef)).isFalse(); } @Test - public void shouldReturnIsNotUpToDateWhenLocalRepoIsAheadOfTheGlobalRefDB() { + public void shouldReturnIsNotUpToDateWhenLocalRepoIsAheadOfTheGlobalRefDB() + throws GlobalRefDbLockException { objectUnderTest.compareAndPut(project, nullRef, objectId1); assertThat(objectUnderTest.isUpToDate(project, ref2)).isFalse();
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapperTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapperTest.java index d2e52fd..ce1fcea 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapperTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapperTest.java
@@ -17,6 +17,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException; import com.google.gerrit.entities.Project; import com.google.gerrit.metrics.Timer0.Context; import org.eclipse.jgit.lib.ObjectId; @@ -59,14 +60,16 @@ } @Test - public void shouldUpdateLockRefExecutionTimeMetricWhenLockRefIsCalled() { + public void shouldUpdateLockRefExecutionTimeMetricWhenLockRefIsCalled() + throws GlobalRefDbLockException { objectUnderTest.lockRef(projectName, refName); verify(metrics).startLockRefExecutionTime(); verify(context).close(); } @Test - public void shouldUpdateIsUpToDateExecutionTimeMetricWhenIsUpToDate() { + public void shouldUpdateIsUpToDateExecutionTimeMetricWhenIsUpToDate() + throws GlobalRefDbLockException { objectUnderTest.isUpToDate(projectName, ref); verify(metrics).startIsUpToDateExecutionTime(); verify(context).close();
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/NoopSharedRefDatabaseTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/NoopSharedRefDatabaseTest.java index 8c749ea..eaeb6d4 100644 --- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/NoopSharedRefDatabaseTest.java +++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/NoopSharedRefDatabaseTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException; import org.eclipse.jgit.lib.Ref; import org.junit.Test; @@ -25,7 +26,7 @@ private NoopSharedRefDatabase objectUnderTest = new NoopSharedRefDatabase(); @Test - public void isUpToDateShouldAlwaysReturnTrue() { + public void isUpToDateShouldAlwaysReturnTrue() throws GlobalRefDbLockException { assertThat(objectUnderTest.isUpToDate(A_TEST_PROJECT_NAME_KEY, sampleRef)).isTrue(); }