Merge branch 'stable-3.9' into stable-3.10
* stable-3.9:
Add sentinel to prevent out of sync with global-refdb
Verify global-refdb formatting using GJF 1.7
Change-Id: I942d7fe24453a9fb1308a9952ecbd13da71517eb
diff --git a/Jenkinsfile b/Jenkinsfile
index 80e2bf3..3217274 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -1,3 +1,2 @@
pluginPipeline(formatCheckId: 'gerritforge:global-refdb-format-59a34d6c50367a468c810e28d733855abc059f13',
- buildCheckId: 'gerritforge:global-refdb-59a34d6c50367a468c810e28d733855abc059f13',
- gjfVersion: '1.7')
+ buildCheckId: 'gerritforge:global-refdb-59a34d6c50367a468c810e28d733855abc059f13')
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDbLockException.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDbLockException.java
index fa757db..29ac90e 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDbLockException.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDbLockException.java
@@ -18,7 +18,7 @@
* {@code GlobalRefDbLockException} is an exception that can be thrown when interacting with the
* global-refdb to represent the inability to lock or acquire a resource.
*/
-public class GlobalRefDbLockException extends RuntimeException {
+public class GlobalRefDbLockException extends RefDbLockException {
private static final long serialVersionUID = 1L;
/**
@@ -30,6 +30,6 @@
* @param cause the cause of the locking failure
*/
public GlobalRefDbLockException(String project, String refName, Exception cause) {
- super(String.format("Unable to lock ref %s on project %s", refName, project), cause);
+ super(project, refName, cause);
}
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java
new file mode 100644
index 0000000..27ac348
--- /dev/null
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/RefDbLockException.java
@@ -0,0 +1,35 @@
+// 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.gerritforge.gerrit.globalrefdb;
+
+/** {@code RefDbLockException} is an exception that can be thrown when trying to lock a ref. */
+public class RefDbLockException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * Constructs a new {@code RefDbLockException} with the specified project, refName and cause.
+ *
+ * @param project the project containing refName
+ * @param refName the specific ref for which the locking failed
+ * @param cause the cause of the locking failure
+ */
+ public RefDbLockException(String project, String refName, Exception cause) {
+ super(String.format("Unable to lock ref %s on project %s", refName, project), cause);
+ }
+
+ public RefDbLockException(String project, String refName, String message) {
+ super(String.format("Unable to lock ref %s on project %s: %s", refName, project, message));
+ }
+}
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 1e45951..75a07d0 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidator.java
@@ -58,7 +58,6 @@
* @param refEnforcement Specific ref enforcements for this project. Either a {@link
* CustomSharedRefEnforcementByProject} when custom policies are provided via configuration *
* file or a {@link DefaultSharedRefEnforcement} for defaults.
- * @param lockWrapperFactory factory providing a {@link LockWrapper}
* @param projectsFilter filter to match whether the project being updated should be validated
* against global refdb
* @param projectName the name of the project being updated.
@@ -71,7 +70,6 @@
SharedRefDatabaseWrapper sharedRefDb,
ValidationMetrics validationMetrics,
SharedRefEnforcement refEnforcement,
- LockWrapper.Factory lockWrapperFactory,
ProjectsFilter projectsFilter,
@Assisted String projectName,
@Assisted RefDatabase refDb,
@@ -80,7 +78,6 @@
sharedRefDb,
validationMetrics,
refEnforcement,
- lockWrapperFactory,
projectsFilter,
projectName,
refDb,
@@ -176,7 +173,8 @@
} catch (OutOfSyncException e) {
List<ReceiveCommand> receiveCommands = batchRefUpdate.getCommands();
logger.atWarning().withCause(e).log(
- "Batch ref-update failing because node is out of sync with the shared ref-db. Set all commands Result to LOCK_FAILURE [%d]",
+ "Batch ref-update failing because node is out of sync with the shared ref-db. Set all"
+ + " commands Result to LOCK_FAILURE [%d]",
receiveCommands.size());
receiveCommands.forEach((command) -> command.setResult(ReceiveCommand.Result.LOCK_FAILURE));
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/DisabledSharedRefLogger.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/DisabledSharedRefLogger.java
similarity index 87%
rename from src/test/java/com/gerritforge/gerrit/globalrefdb/validation/DisabledSharedRefLogger.java
rename to src/main/java/com/gerritforge/gerrit/globalrefdb/validation/DisabledSharedRefLogger.java
index d6dd338..7ac7f01 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/DisabledSharedRefLogger.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/DisabledSharedRefLogger.java
@@ -16,9 +16,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
-import org.junit.Ignore;
-@Ignore
public class DisabledSharedRefLogger implements SharedRefLogger {
@Override
@@ -28,10 +26,10 @@
public void logProjectDelete(String project) {}
@Override
- public void logLockAcquisition(String project, String refName) {}
+ public void logLockAcquisition(String project, String refName, Scope scope) {}
@Override
- public void logLockRelease(String project, String refName) {}
+ public void logLockRelease(String project, String refName, Scope scope) {}
@Override
public <T> void logRefUpdate(String project, String refName, T currRef, T newRefValue) {}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LockWrapper.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LockWrapper.java
index a1312b0..02b2ec0 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LockWrapper.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LockWrapper.java
@@ -14,23 +14,13 @@
package com.gerritforge.gerrit.globalrefdb.validation;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
/** Wrapper around an {@link AutoCloseable} lock to allow logging of resource releasing. */
public class LockWrapper implements AutoCloseable {
- /** {@code LockWrapper} Factory for Guice assisted injection. */
- public interface Factory {
- LockWrapper create(
- @Assisted("project") String project,
- @Assisted("refName") String refName,
- @Assisted AutoCloseable lock);
- }
-
private final String project;
private final String refName;
private final AutoCloseable lock;
private final SharedRefLogger sharedRefLogger;
+ private final SharedRefLogger.Scope scope;
/**
* Constructs a {@code LockWrapper} object for a specific refName of a project, which wraps a held
@@ -41,16 +31,18 @@
* @param refName the refName the lock has been acquired for
* @param lock the acquired lock
*/
- @Inject
public LockWrapper(
SharedRefLogger sharedRefLogger,
- @Assisted("project") String project,
- @Assisted("refName") String refName,
- @Assisted AutoCloseable lock) {
+ String project,
+ String refName,
+ AutoCloseable lock,
+ SharedRefLogger.Scope scope) {
this.lock = lock;
this.sharedRefLogger = sharedRefLogger;
this.project = project;
this.refName = refName;
+ this.scope = scope;
+ sharedRefLogger.logLockAcquisition(project, refName, scope);
}
/**
@@ -61,6 +53,6 @@
@Override
public void close() throws Exception {
lock.close();
- sharedRefLogger.logLockRelease(project, refName);
+ sharedRefLogger.logLockRelease(project, refName, scope);
}
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLogger.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLogger.java
index 04ffeed..691d8b1 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLogger.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLogger.java
@@ -168,8 +168,8 @@
* <p>Logs Json serialization of {@link SharedRefLogEntry.LockAcquire}
*/
@Override
- public void logLockAcquisition(String project, String refName) {
- sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.LockAcquire(project, refName)));
+ public void logLockAcquisition(String project, String refName, Scope scope) {
+ sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.LockAcquire(project, refName, scope)));
}
/**
@@ -178,8 +178,8 @@
* <p>Logs Json serialization of {@link SharedRefLogEntry.LockRelease}
*/
@Override
- public void logLockRelease(String project, String refName) {
- sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.LockRelease(project, refName)));
+ public void logLockRelease(String project, String refName, Scope scope) {
+ sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.LockRelease(project, refName, scope)));
}
@VisibleForTesting
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/NoOpRefLocker.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/NoOpRefLocker.java
new file mode 100644
index 0000000..b8682b4
--- /dev/null
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/NoOpRefLocker.java
@@ -0,0 +1,30 @@
+// 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.gerritforge.gerrit.globalrefdb.validation;
+
+import com.gerritforge.gerrit.globalrefdb.RefDbLockException;
+import com.google.gerrit.entities.Project;
+import com.google.inject.Singleton;
+
+@Singleton
+class NoOpRefLocker implements RefLocker {
+
+ public static final NoOpRefLocker INSTANCE = new NoOpRefLocker();
+
+ @Override
+ public AutoCloseable lockRef(Project.NameKey project, String refName) throws RefDbLockException {
+ return () -> {};
+ }
+}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/DummyLockWrapper.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefLocker.java
similarity index 63%
rename from src/test/java/com/gerritforge/gerrit/globalrefdb/validation/DummyLockWrapper.java
rename to src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefLocker.java
index 84d6912..c0f1578 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/DummyLockWrapper.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefLocker.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2020 The Android Open Source Project
+// 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.
@@ -14,13 +14,11 @@
package com.gerritforge.gerrit.globalrefdb.validation;
-import org.junit.Ignore;
+import com.gerritforge.gerrit.globalrefdb.RefDbLockException;
+import com.google.gerrit.entities.Project;
+import com.google.inject.ImplementedBy;
-@Ignore
-public class DummyLockWrapper implements LockWrapper.Factory {
-
- @Override
- public LockWrapper create(String project, String refName, AutoCloseable lock) {
- return new LockWrapper(new DisabledSharedRefLogger(), project, refName, lock);
- }
+@ImplementedBy(NoOpRefLocker.class)
+public interface RefLocker {
+ AutoCloseable lockRef(Project.NameKey project, String refName) throws RefDbLockException;
}
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 a331a83..50986da 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
@@ -15,6 +15,7 @@
package com.gerritforge.gerrit.globalrefdb.validation;
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;
@@ -46,7 +47,6 @@
protected final ValidationMetrics validationMetrics;
protected final String projectName;
- private final LockWrapper.Factory lockWrapperFactory;
protected final RefDatabase refDb;
protected final SharedRefEnforcement refEnforcement;
protected final ProjectsFilter projectsFilter;
@@ -92,7 +92,6 @@
* @param refEnforcement Specific ref enforcements for this project. Either a {@link
* CustomSharedRefEnforcementByProject} when custom policies are provided via configuration
* file or a {@link DefaultSharedRefEnforcement} for defaults.
- * @param lockWrapperFactory factory providing a {@link LockWrapper}
* @param projectsFilter filter to match whether the project being updated should be validated
* against global refdb
* @param projectName the name of the project being updated.
@@ -105,14 +104,12 @@
SharedRefDatabaseWrapper sharedRefDb,
ValidationMetrics validationMetrics,
SharedRefEnforcement refEnforcement,
- LockWrapper.Factory lockWrapperFactory,
ProjectsFilter projectsFilter,
@Assisted String projectName,
@Assisted RefDatabase refDb,
@Assisted ImmutableSet<String> ignoredRefs) {
this.sharedRefDb = sharedRefDb;
this.validationMetrics = validationMetrics;
- this.lockWrapperFactory = lockWrapperFactory;
this.refDb = refDb;
this.ignoredRefs = ignoredRefs;
this.projectName = projectName;
@@ -205,6 +202,9 @@
e.getMessage());
}
return result;
+ } catch (RefDbLockException e) {
+ logger.atWarning().withCause(e).log("Unable to lock %s:%s", projectName, refUpdate.getName());
+ return Result.LOCK_FAILURE;
} catch (OutOfSyncException e) {
logger.atWarning().withCause(e).log(
"Local node is out of sync with ref-db: %s", e.getMessage());
@@ -242,8 +242,9 @@
Project.nameKey(projectName), refPair.compareRef, refPair.putValue);
} catch (GlobalRefDbSystemError e) {
logger.atWarning().withCause(e).log(
- "Not able to persist the data in global-refdb for project '%s' and ref '%s', message: %s",
- projectName, refPair.getName(), e.getMessage());
+ "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());
throw e;
}
@@ -266,19 +267,18 @@
return refPair;
}
- locks.addResourceIfNotExist(
- String.format("%s-%s", projectName, refName),
- () ->
- lockWrapperFactory.create(
- projectName, refName, sharedRefDb.lockRef(Project.nameKey(projectName), refName)));
+ String sharedLockKey = String.format("%s:%s", projectName, refName);
+ String localLockKey = String.format("%s:local", sharedLockKey);
+ Project.NameKey projectKey = Project.nameKey(projectName);
+ locks.addResourceIfNotExist(localLockKey, () -> sharedRefDb.lockLocalRef(projectKey, refName));
+ locks.addResourceIfNotExist(sharedLockKey, () -> sharedRefDb.lockRef(projectKey, refName));
RefPair latestRefPair = getLatestLocalRef(refPair);
- if (sharedRefDb.isUpToDate(Project.nameKey(projectName), latestRefPair.compareRef)) {
+ if (sharedRefDb.isUpToDate(projectKey, latestRefPair.compareRef)) {
return latestRefPair;
}
- if (isNullRef(latestRefPair.compareRef)
- || sharedRefDb.exists(Project.nameKey(projectName), refName)) {
+ if (isNullRef(latestRefPair.compareRef) || sharedRefDb.exists(projectKey, refName)) {
validationMetrics.incrementSplitBrainPrevention();
softFailBasedOnEnforcement(
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
index b973602..fd5d546 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
@@ -18,6 +18,7 @@
import com.gerritforge.gerrit.globalrefdb.GlobalRefDatabase;
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.NoopSharedRefDatabase;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.flogger.FluentLogger;
@@ -45,6 +46,7 @@
private final SharedRefLogger sharedRefLogger;
private final SharedRefDBMetrics metrics;
+ private final RefLocker localRefDbLocker;
/**
* Constructs a {@code SharedRefDatabaseWrapper} wrapping an optional {@link GlobalRefDatabase},
@@ -53,17 +55,20 @@
* @param sharedRefLogger logger of shared ref-db operations.
*/
@Inject
- public SharedRefDatabaseWrapper(SharedRefLogger sharedRefLogger, SharedRefDBMetrics metrics) {
+ public SharedRefDatabaseWrapper(
+ SharedRefLogger sharedRefLogger, SharedRefDBMetrics metrics, RefLocker localRefDbLocker) {
this.sharedRefLogger = sharedRefLogger;
this.metrics = metrics;
+ this.localRefDbLocker = localRefDbLocker;
}
@VisibleForTesting
public SharedRefDatabaseWrapper(
DynamicItem<GlobalRefDatabase> sharedRefDbDynamicItem,
SharedRefLogger sharedRefLogger,
- SharedRefDBMetrics metrics) {
- this(sharedRefLogger, metrics);
+ SharedRefDBMetrics metrics,
+ RefLocker localRefDbLocker) {
+ this(sharedRefLogger, metrics, localRefDbLocker);
this.sharedRefDbDynamicItem = sharedRefDbDynamicItem;
}
@@ -122,12 +127,25 @@
public AutoCloseable lockRef(Project.NameKey project, String refName)
throws GlobalRefDbLockException {
try (Context context = metrics.startLockRefExecutionTime()) {
- AutoCloseable locker = sharedRefDb().lockRef(project, refName);
- sharedRefLogger.logLockAcquisition(project.get(), refName);
- return locker;
+ return new LockWrapper(
+ sharedRefLogger,
+ project.get(),
+ refName,
+ sharedRefDb().lockRef(project, refName),
+ SharedRefLogger.Scope.GLOBAL);
}
}
+ public AutoCloseable lockLocalRef(Project.NameKey project, String refName)
+ throws RefDbLockException {
+ return new LockWrapper(
+ sharedRefLogger,
+ project.get(),
+ refName,
+ localRefDbLocker.lockRef(project, refName),
+ SharedRefLogger.Scope.LOCAL);
+ }
+
@Override
public boolean exists(Project.NameKey project, String refName) {
try (Context context = metrics.startExistsExecutionTime()) {
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
index 986175a..4b7f987 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
@@ -65,12 +65,16 @@
return sharedRefDb.get();
}
- /** @return Getter of projects checked against the global refdb */
+ /**
+ * @return Getter of projects checked against the global refdb
+ */
public Projects projects() {
return projects.get();
}
- /** @return name of the libModule consuming this library */
+ /**
+ * @return name of the libModule consuming this library
+ */
public String pluginName() {
return pluginName;
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogEntry.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogEntry.java
index 4b8132f..817b274 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogEntry.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogEntry.java
@@ -79,22 +79,26 @@
public static class LockAcquire extends SharedRefLogEntry {
public String refName;
+ public SharedRefLogger.Scope scope;
- LockAcquire(String projectName, String refName) {
+ LockAcquire(String projectName, String refName, SharedRefLogger.Scope scope) {
this.type = Type.LOCK_ACQUIRE;
this.projectName = projectName;
this.refName = refName;
+ this.scope = scope;
}
}
public static class LockRelease extends SharedRefLogEntry {
public String refName;
+ public SharedRefLogger.Scope scope;
- LockRelease(String projectName, String refName) {
+ LockRelease(String projectName, String refName, SharedRefLogger.Scope scope) {
this.type = Type.LOCK_RELEASE;
this.projectName = projectName;
this.refName = refName;
+ this.scope = scope;
}
}
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java
index f61f253..96db4e4 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java
@@ -21,6 +21,11 @@
@ImplementedBy(Log4jSharedRefLogger.class)
public interface SharedRefLogger {
+ enum Scope {
+ GLOBAL,
+ LOCAL
+ }
+
/**
* Log the update of currRef in project project to ref newRefValue
*
@@ -63,14 +68,16 @@
*
* @param project the project containing the ref
* @param refName the name of the ref the lock is acquired for
+ * @param scope scope of the lock
*/
- void logLockAcquisition(String project, String refName);
+ void logLockAcquisition(String project, String refName, Scope scope);
/**
* Log the releasing of a previously acquired lock for the 'refName' of 'project'
*
* @param project the project containing the ref
* @param refName the name of the ref the lock is being releaed for
+ * @param scope scope of the lock
*/
- void logLockRelease(String project, String refName);
+ void logLockRelease(String project, String refName, Scope scope);
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
index 893a16a..bb50f74 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
@@ -17,14 +17,15 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.Collections.singletonList;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.CREATE;
import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -37,7 +38,6 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.metrics.DisabledMetricMaker;
import java.io.IOException;
-import java.util.Arrays;
import java.util.List;
import org.eclipse.jgit.internal.storage.file.RefDirectory;
import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
@@ -47,13 +47,13 @@
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestName;
@@ -63,6 +63,9 @@
@RunWith(MockitoJUnitRunner.class)
public class BatchRefUpdateValidatorTest extends LocalDiskRepositoryTestCase implements RefFixture {
+ private static final String A_REF_NAME_1 = "refs/heads/a-ref-name-1";
+ private static final String A_REF_NAME_2 = "refs/heads/a-ref-name-2";
+
@Rule public TestName nameRule = new TestName();
private Repository diskRepo;
@@ -80,7 +83,7 @@
@Before
public void setup() throws Exception {
super.setUp();
- doReturn(false).when(sharedRefDatabase).isUpToDate(any(), any());
+ lenient().doReturn(false).when(sharedRefDatabase).isUpToDate(any(), any());
when(projectsFilter.matches(anyString())).thenReturn(true);
gitRepoSetup();
}
@@ -90,28 +93,26 @@
refdir = (RefDirectory) diskRepo.getRefDatabase();
repo = new TestRepository<>(diskRepo);
A = repo.commit().create();
+ updateRef(A_REF_NAME_1, A.getId());
+ updateRef(A_REF_NAME_2, A.getId());
B = repo.commit(repo.getRevWalk().parseCommit(A));
}
- @Ignore("This test will be enabled in stable-3.10")
@Test
public void shouldUpdateSharedRefDbForAllRefUpdates() throws IOException {
- String REF_NAME_A = "refs/changes/01/1/1";
- String REF_NAME_B = "refs/changes/02/1/1";
BatchRefUpdate batchRefUpdate =
newBatchUpdate(
List.of(
- new ReceiveCommand(A, B, REF_NAME_A, UPDATE),
- new ReceiveCommand(A, B, REF_NAME_B, UPDATE)));
+ new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE),
+ new ReceiveCommand(A, B, A_REF_NAME_2, UPDATE)));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, REF_NAME_A);
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any());
-
doReturn(true).when(sharedRefDatabase).compareAndPut(any(), any(), any());
batchRefUpdateValidator.executeBatchUpdateWithValidation(
@@ -129,73 +130,67 @@
public void immutableChangeShouldNotBeWrittenIntoSharedRefDb() throws Exception {
String AN_IMMUTABLE_REF = "refs/changes/01/1/1";
- List<ReceiveCommand> cmds = Arrays.asList(new ReceiveCommand(A, B, AN_IMMUTABLE_REF, UPDATE));
+ ReceiveCommand receiveCommand =
+ new ReceiveCommand(ObjectId.zeroId(), B, AN_IMMUTABLE_REF, CREATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(List.of(receiveCommand));
- BatchRefUpdate batchRefUpdate = newBatchUpdate(cmds);
- BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator(A_TEST_PROJECT_NAME);
-
+ BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator();
BatchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), this::defaultRollback);
verify(sharedRefDatabase, never())
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
+ assertThatReceiveCommandIsSuccessful(receiveCommand);
}
@Test
public void compareAndPutShouldAlwaysIngoreAlwaysDraftCommentsEvenOutOfOrder() throws Exception {
String DRAFT_COMMENT = "refs/draft-comments/56/450756/1013728";
- List<ReceiveCommand> cmds = Arrays.asList(new ReceiveCommand(A, B, DRAFT_COMMENT, UPDATE));
+ ReceiveCommand receiveCommand = new ReceiveCommand(ObjectId.zeroId(), B, DRAFT_COMMENT, CREATE);
- BatchRefUpdate batchRefUpdate = newBatchUpdate(cmds);
- BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator(A_TEST_PROJECT_NAME);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(List.of(receiveCommand));
+ BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator();
BatchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), this::defaultRollback);
verify(sharedRefDatabase, never())
.compareAndPut(A_TEST_PROJECT_NAME_KEY, newRef(DRAFT_COMMENT, A.getId()), B.getId());
+ assertThatReceiveCommandIsSuccessful(receiveCommand);
}
@Test
public void validationShouldFailWhenLocalRefDbIsOutOfSync() throws Exception {
- String AN_OUT_OF_SYNC_REF = "refs/changes/01/1/1";
- BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, AN_OUT_OF_SYNC_REF, UPDATE)));
+ ReceiveCommand receiveCommand = new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(singletonList(receiveCommand));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, AN_OUT_OF_SYNC_REF);
- lenient()
- .doReturn(false)
- .when(sharedRefDatabase)
- .isUpToDate(A_TEST_PROJECT_NAME_KEY, newRef(AN_OUT_OF_SYNC_REF, AN_OBJECT_ID_1));
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
+ doReturn(false).when(sharedRefDatabase).isUpToDate(eq(A_TEST_PROJECT_NAME_KEY), any());
+ doReturn(true).when(sharedRefDatabase).exists(eq(A_TEST_PROJECT_NAME_KEY), any());
batchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), rollbackFunction);
verify(rollbackFunction, never()).invoke(any());
- final List<ReceiveCommand> commands = batchRefUpdate.getCommands();
- assertThat(commands.size()).isEqualTo(1);
- commands.forEach(
- (command) -> assertThat(command.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE));
+ assertThat(receiveCommand.getResult()).isEqualTo(Result.LOCK_FAILURE);
}
- @Ignore("This test will be enabled in stable-3.10")
@Test
public void shouldRollbackWhenSharedRefUpdateCompareAndPutThrowsUncaughtThrowable()
throws Exception {
- String REF_NAME = "refs/changes/01/1/meta";
- BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, REF_NAME, UPDATE)));
+ ReceiveCommand receiveCommand = new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(singletonList(receiveCommand));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, REF_NAME);
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any());
doThrow(TestError.class).when(sharedRefDatabase).compareAndPut(any(), any(), any());
@@ -207,23 +202,19 @@
batchRefUpdate, () -> execute(batchRefUpdate), rollbackFunction));
verify(rollbackFunction).invoke(any());
- List<ReceiveCommand> commands = batchRefUpdate.getCommands();
- assertThat(commands.size()).isEqualTo(1);
- commands.forEach(
- (command) -> assertThat(command.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE));
+ assertThat(receiveCommand.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE);
}
@Test
public void shouldRollbackRefUpdateWhenRefDbIsNotUpdated() throws Exception {
- String REF_NAME = "refs/changes/01/1/meta";
- BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, REF_NAME, UPDATE)));
+ ReceiveCommand receiveCommand = new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(singletonList(receiveCommand));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, REF_NAME);
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any());
@@ -235,23 +226,18 @@
batchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), rollbackFunction);
- verify(rollbackFunction, times(1)).invoke(any());
-
- final List<ReceiveCommand> commands = batchRefUpdate.getCommands();
- assertThat(commands.size()).isEqualTo(1);
- commands.forEach(
- (command) -> assertThat(command.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE));
+ verify(rollbackFunction).invoke(any());
+ assertThat(receiveCommand.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE);
}
@Test
public void shouldNotUpdateSharedRefDbWhenProjectIsLocal() throws Exception {
when(projectsFilter.matches(anyString())).thenReturn(false);
- String AN_OUT_OF_SYNC_REF = "refs/changes/01/1/1";
BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, AN_OUT_OF_SYNC_REF, UPDATE)));
+ newBatchUpdate(singletonList(new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE)));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
batchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), this::defaultRollback);
@@ -260,35 +246,43 @@
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
}
- private BatchRefUpdateValidator newDefaultValidator(String projectName) {
- return getRefValidatorForEnforcement(projectName, new DefaultSharedRefEnforcement());
+ private void updateRef(String refName, ObjectId sha1) throws IOException {
+ RefUpdate refUpdate = refdir.newUpdate(refName, false);
+ refUpdate.setNewObjectId(sha1);
+ refUpdate.update();
+ }
+
+ private static void assertThatReceiveCommandIsSuccessful(ReceiveCommand receiveCommand) {
+ assertThat(receiveCommand.getResult()).isEqualTo(Result.OK);
+ }
+
+ private BatchRefUpdateValidator newDefaultValidator() {
+ return getRefValidatorForEnforcement(new DefaultSharedRefEnforcement());
}
private BatchRefUpdateValidator getRefValidatorForEnforcement(
- String projectName, SharedRefEnforcement sharedRefEnforcement) {
+ SharedRefEnforcement sharedRefEnforcement) {
return new BatchRefUpdateValidator(
sharedRefDatabase,
new ValidationMetrics(
new DisabledMetricMaker(), new SharedRefDbConfiguration(new Config(), "testplugin")),
sharedRefEnforcement,
- new DummyLockWrapper(),
projectsFilter,
- projectName,
+ RefFixture.A_TEST_PROJECT_NAME,
diskRepo.getRefDatabase(),
ImmutableSet.of());
}
- private Void execute(BatchRefUpdate u) throws IOException {
+ private void execute(BatchRefUpdate u) throws IOException {
try (RevWalk rw = new RevWalk(diskRepo)) {
u.execute(rw, NullProgressMonitor.INSTANCE);
}
- return null;
}
private BatchRefUpdate newBatchUpdate(List<ReceiveCommand> cmds) {
BatchRefUpdate u = refdir.newBatchUpdate();
u.addCommand(cmds);
- cmds.forEach(c -> c.setResult(Result.OK));
+ cmds.forEach(c -> c.setResult(Result.NOT_ATTEMPTED));
return u;
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java
index 47964cf..5b9a9dd 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/Log4jSharedRefLoggerTest.java
@@ -120,7 +120,7 @@
@Test
public void shouldLogLockAcquisition() {
String refName = "refs/foo/bar";
- log4jSharedRefLogger.logLockAcquisition(project.get(), refName);
+ log4jSharedRefLogger.logLockAcquisition(project.get(), refName, SharedRefLogger.Scope.GLOBAL);
SharedRefLogEntry.LockAcquire gotLogEntry =
gson.fromJson(logWriter.toString(), SharedRefLogEntry.LockAcquire.class);
@@ -133,7 +133,7 @@
@Test
public void shouldLogLockRelease() {
String refName = "refs/foo/bar";
- log4jSharedRefLogger.logLockRelease(project.get(), refName);
+ log4jSharedRefLogger.logLockRelease(project.get(), refName, SharedRefLogger.Scope.GLOBAL);
SharedRefLogEntry.LockAcquire gotLogEntry =
gson.fromJson(logWriter.toString(), SharedRefLogEntry.LockAcquire.class);
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java
index cf383bf..ba54cb9 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java
@@ -14,6 +14,8 @@
package com.gerritforge.gerrit.globalrefdb.validation;
+import static org.mockito.Mockito.verify;
+
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.events.ProjectDeletedListener;
@@ -22,7 +24,6 @@
import org.junit.rules.TestName;
import org.junit.runner.RunWith;
import org.mockito.Mock;
-import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
@RunWith(MockitoJUnitRunner.class)
@@ -53,6 +54,6 @@
projectDeletedSharedDbCleanup.onProjectDeleted(event);
- Mockito.verify(sharedRefDatabase, Mockito.times(1)).remove(A_TEST_PROJECT_NAME_KEY);
+ verify(sharedRefDatabase).remove(A_TEST_PROJECT_NAME_KEY);
}
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
index d686bf8..435db8f 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
@@ -18,25 +18,27 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError;
+import com.gerritforge.gerrit.globalrefdb.RefDbLockException;
import com.gerritforge.gerrit.globalrefdb.validation.RefUpdateValidator.OneParameterFunction;
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement;
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
+import java.io.IOException;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
@@ -94,7 +96,7 @@
@Test
public void validationShouldSucceedWhenSharedRefDbIsNoop() throws Exception {
SharedRefDatabaseWrapper noopSharedRefDbWrapper =
- new SharedRefDatabaseWrapper(sharedRefLogger, sharedRefDBMetrics);
+ new SharedRefDatabaseWrapper(sharedRefLogger, sharedRefDBMetrics, NoOpRefLocker.INSTANCE);
Result result =
newRefUpdateValidator(noopSharedRefDbWrapper)
@@ -102,7 +104,6 @@
assertThat(result).isEqualTo(Result.NEW);
}
- @Ignore("This test will be enabled in stable-3.10")
@Test
public void validationShouldSucceedWhenLocalRefDbIsUpToDate() throws Exception {
lenient()
@@ -119,7 +120,8 @@
.compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, newUpdateRef.getObjectId());
Result result =
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, this::defaultRollback);
+ refUpdateValidator.executeRefUpdate(
+ refUpdate, () -> doLocalRefUpdate(refName), this::defaultRollback);
assertThat(result).isEqualTo(Result.NEW);
}
@@ -179,6 +181,18 @@
}
@Test
+ public void validationShouldFailWhenLocalRefDbIsLocked() throws Exception {
+ doThrow(RefDbLockException.class)
+ .when(sharedRefDb)
+ .lockLocalRef(A_TEST_PROJECT_NAME_KEY, refName);
+
+ Result result =
+ refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, this::defaultRollback);
+
+ assertThat(result).isEqualTo(Result.LOCK_FAILURE);
+ }
+
+ @Test
public void shouldRollbackWhenLocalRefDbIsUpToDateButFinalCompareAndPutIsFailing()
throws Exception {
lenient()
@@ -190,16 +204,17 @@
.doReturn(true)
.when(sharedRefDb)
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
- lenient()
- .doReturn(false)
+ doReturn(false)
.when(sharedRefDb)
- .compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, newUpdateRef.getObjectId());
+ .compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, AN_OBJECT_ID_2);
doReturn(lock).when(sharedRefDb).lockRef(any(), anyString());
Result result =
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, rollbackFunction);
+ refUpdateValidator.executeRefUpdate(
+ refUpdate, () -> doLocalRefUpdate(refName), rollbackFunction);
- verify(rollbackFunction, times(1)).invoke(any());
+ verify(sharedRefDb).compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, AN_OBJECT_ID_2);
+ verify(rollbackFunction).invoke(any());
assertThat(result).isEqualTo(Result.LOCK_FAILURE);
}
@@ -227,17 +242,15 @@
.when(sharedRefDb)
.isUpToDate(any(Project.NameKey.class), any(Ref.class));
doReturn(true).when(sharedRefDb).isUpToDate(A_TEST_PROJECT_NAME_KEY, localRef);
- lenient()
- .when(
- sharedRefDb.compareAndPut(
- any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)))
+ when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)))
.thenThrow(GlobalRefDbSystemError.class);
when(rollbackFunction.invoke(any())).thenReturn(Result.LOCK_FAILURE);
Result result =
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, rollbackFunction);
+ refUpdateValidator.executeRefUpdate(
+ refUpdate, () -> doLocalRefUpdate(localRef.getName()), rollbackFunction);
- verify(rollbackFunction, times(1)).invoke(any());
+ verify(rollbackFunction).invoke(any());
}
@Test
@@ -267,12 +280,19 @@
return Result.NO_CHANGE;
}
+ private Result doLocalRefUpdate(String refName) throws IOException {
+ lenient()
+ .doReturn(new ObjectIdRef.Unpeeled(Ref.Storage.LOOSE, refName, AN_OBJECT_ID_2))
+ .when(localRefDb)
+ .findRef(refName);
+ return Result.NEW;
+ }
+
private RefUpdateValidator newRefUpdateValidator(SharedRefDatabaseWrapper refDbWrapper) {
return new RefUpdateValidator(
refDbWrapper,
validationMetrics,
defaultRefEnforcement,
- new DummyLockWrapper(),
projectsFilter,
A_TEST_PROJECT_NAME,
localRefDb,
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 884090e..d2e52fd 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapperTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapperTest.java
@@ -47,7 +47,8 @@
when(metrics.startExistsExecutionTime()).thenReturn(context);
when(metrics.startIsUpToDateExecutionTime()).thenReturn(context);
when(metrics.startRemoveExecutionTime()).thenReturn(context);
- objectUnderTest = new SharedRefDatabaseWrapper(sharedRefLogger, metrics);
+ objectUnderTest =
+ new SharedRefDatabaseWrapper(sharedRefLogger, metrics, NoOpRefLocker.INSTANCE);
}
@Test
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
index 6596e8d..e463cb5 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
@@ -22,6 +22,7 @@
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
@@ -30,6 +31,7 @@
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
+import java.util.List;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdRef;
@@ -40,7 +42,6 @@
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestName;
@@ -109,15 +110,19 @@
.thenReturn(asList(receiveCommandBeforeExecution))
.thenReturn(asList(successReceiveCommandAfterExecution));
- doReturn(oldRef).when(refDatabase).getRef(A_TEST_REF_NAME);
- doReturn(oldRef).when(refDatabase).exactRef(A_TEST_REF_NAME);
+ // Only needed to prevent NPEs should a rollback happen on the global-refdb updates
+ // and not supposed to be used if all tests are successful.
+ lenient().when(batchRefUpdate.addCommand(any(List.class))).thenReturn(batchRefUpdate);
+
+ lenient().when(refDatabase.getRef(A_TEST_REF_NAME)).thenReturn(oldRef).thenReturn(newRef);
+ lenient().when(refDatabase.exactRef(A_TEST_REF_NAME)).thenReturn(oldRef).thenReturn(newRef);
+ lenient().when(refDatabase.findRef(A_TEST_REF_NAME)).thenReturn(oldRef).thenReturn(newRef);
sharedRefDbRefUpdate = getSharedRefDbBatchRefUpdateWithDefaultPolicyEnforcement();
verifyNoInteractions(validationMetrics);
}
- @Ignore("This test will be enabled in stable-3.10")
@Test
public void executeAndDelegateSuccessfullyWithNoExceptions() throws Exception {
setMockRequiredReturnValues();
@@ -177,7 +182,6 @@
sharedRefDb,
validationMetrics,
new DefaultSharedRefEnforcement(),
- new DummyLockWrapper(),
projectsFilter,
projectName,
refDb,
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java
index b1af798..3283994 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ValidationModuleTest.java
@@ -91,7 +91,6 @@
bind(SharedRefDatabaseWrapper.class).in(Scopes.SINGLETON);
bind(SharedRefLogger.class).to(Log4jSharedRefLogger.class);
- factory(LockWrapper.Factory.class);
factory(SharedRefDbRepository.Factory.class);
factory(SharedRefDbRefDatabase.Factory.class);
