Use a global version number instead of the commit timestamp
The commit timestamp is not a reliable source of repository
version numbering because of unreliability of the local
time of the committer and forced push use-case, where the
repository is brought back in time.
Define a new global version numbering
(current system time at the moment) and
use it as monotonic versioning for the repository.
Bug: Issue 12413
Change-Id: If1b642a56791bd3a4c1d6ef903ec2f7c61c19928
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java
index d13ed79..3b82582 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java
@@ -43,8 +43,6 @@
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;
@Singleton
public class ProjectVersionRefUpdate implements EventListener {
@@ -101,18 +99,16 @@
}
try {
Project.NameKey projectNameKey = refUpdatedEvent.getProjectNameKey();
+ long newVersion = getCurrentGlobalVersionNumber();
- Optional<Long> lastRefUpdatedTimestamp = getLastRefUpdatedTimestamp(projectNameKey, refName);
Optional<RefUpdate> newProjectVersionRefUpdate =
- updateLocalProjectVersion(projectNameKey, lastRefUpdatedTimestamp);
+ updateLocalProjectVersion(projectNameKey, newVersion);
- if (newProjectVersionRefUpdate.isPresent() && lastRefUpdatedTimestamp.isPresent()) {
- verLogger.log(projectNameKey, lastRefUpdatedTimestamp.get(), 0L);
+ if (newProjectVersionRefUpdate.isPresent()) {
+ verLogger.log(projectNameKey, newVersion, 0L);
if (updateSharedProjectVersion(
- projectNameKey,
- newProjectVersionRefUpdate.get().getNewObjectId(),
- lastRefUpdatedTimestamp.get())) {
+ projectNameKey, newProjectVersionRefUpdate.get().getNewObjectId(), newVersion)) {
gitReferenceUpdated.fire(projectNameKey, newProjectVersionRefUpdate.get(), null);
}
} else {
@@ -142,31 +138,6 @@
return newId;
}
- private Optional<Long> getLastRefUpdatedTimestamp(Project.NameKey projectNameKey, String refName)
- throws LocalProjectVersionUpdateException {
- logger.atFine().log(
- String.format(
- "Getting last ref updated time for project %s, ref %s", projectNameKey.get(), refName));
- try (Repository repository = gitRepositoryManager.openRepository(projectNameKey)) {
- Ref ref = repository.findRef(refName);
- if (ref == null) {
- logger.atWarning().log("Unable to find ref " + refName + " in project " + projectNameKey);
- return Optional.empty();
- }
- try (RevWalk walk = new RevWalk(repository)) {
- RevCommit commit = walk.parseCommit(ref.getObjectId());
- return Optional.of(Integer.toUnsignedLong(commit.getCommitTime()));
- }
- } catch (IOException ioe) {
- String message =
- String.format(
- "Error while getting last ref updated time for project %s, ref %s",
- projectNameKey.get(), refName);
- logger.atSevere().withCause(ioe).log(message);
- throw new LocalProjectVersionUpdateException(message);
- }
- }
-
private boolean updateSharedProjectVersion(
Project.NameKey projectNameKey, ObjectId newObjectId, Long newVersion)
throws SharedProjectVersionUpdateException {
@@ -282,21 +253,19 @@
}
private Optional<RefUpdate> updateLocalProjectVersion(
- Project.NameKey projectNameKey, Optional<Long> lastRefUpdatedTimestamp)
+ Project.NameKey projectNameKey, long newVersionNumber)
throws LocalProjectVersionUpdateException {
- if (!lastRefUpdatedTimestamp.isPresent()) {
- return Optional.empty();
- }
-
- logger.atFine().log("Updating local version for project " + projectNameKey.get());
+ logger.atFine().log(
+ "Updating local version for project %s with version %d",
+ projectNameKey.get(), newVersionNumber);
try (Repository repository = gitRepositoryManager.openRepository(projectNameKey)) {
- RefUpdate refUpdate = getProjectVersionRefUpdate(repository, lastRefUpdatedTimestamp.get());
+ RefUpdate refUpdate = getProjectVersionRefUpdate(repository, newVersionNumber);
RefUpdate.Result result = refUpdate.update();
if (!isSuccessful(result)) {
String message =
String.format(
"RefUpdate failed with result %s for: project=%s, version=%d",
- result.name(), projectNameKey.get(), lastRefUpdatedTimestamp.get());
+ result.name(), projectNameKey.get(), newVersionNumber);
logger.atSevere().log(message);
throw new LocalProjectVersionUpdateException(message);
}
@@ -309,6 +278,10 @@
}
}
+ private long getCurrentGlobalVersionNumber() {
+ return System.currentTimeMillis() / 1000;
+ }
+
private Boolean isSuccessful(RefUpdate.Result result) {
return SUCCESSFUL_RESULTS.contains(result);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
index 1839fe1..6360227 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
@@ -116,10 +116,56 @@
assertThat(ref).isNotNull();
ObjectLoader loader = repo.getRepository().open(ref.getObjectId());
- String storedVersion = IOUtils.toString(loader.openStream(), StandardCharsets.UTF_8.name());
- assertThat(Long.parseLong(storedVersion)).isEqualTo(masterCommit.getCommitTime());
+ long storedVersion =
+ Long.parseLong(IOUtils.toString(loader.openStream(), StandardCharsets.UTF_8.name()));
+ assertThat(storedVersion).isGreaterThan((long) masterCommit.getCommitTime());
- verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, masterCommit.getCommitTime(), 0);
+ verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, storedVersion, 0);
+ }
+
+ @Test
+ public void producerShouldUpdateProjectVersionUponForcedPushRefUpdatedEvent() throws Exception {
+ Context.setForwardedEvent(false);
+
+ Thread.sleep(1000L);
+ RevCommit masterPlusOneCommit = repo.branch("master").commit().create();
+
+ Thread.sleep(1000L);
+ repo.branch("master").update(masterCommit);
+
+ when(sharedRefDb.get(
+ A_TEST_PROJECT_NAME_KEY,
+ ProjectVersionRefUpdate.MULTI_SITE_VERSIONING_REF,
+ String.class))
+ .thenReturn(Optional.of("26f7ee61bf0e470e8393c884526eec8a9b943a63"));
+ when(sharedRefDb.get(
+ A_TEST_PROJECT_NAME_KEY,
+ ProjectVersionRefUpdate.MULTI_SITE_VERSIONING_VALUE_REF,
+ String.class))
+ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1)));
+ when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)))
+ .thenReturn(true);
+ when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any()))
+ .thenReturn(true);
+ when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY);
+ when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME);
+
+ new ProjectVersionRefUpdate(repoManager, sharedRefDb, gitReferenceUpdated, verLogger)
+ .onEvent(refUpdatedEvent);
+
+ Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF);
+
+ verify(sharedRefDb, atMost(1))
+ .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
+
+ assertThat(ref).isNotNull();
+
+ ObjectLoader loader = repo.getRepository().open(ref.getObjectId());
+ long storedVersion =
+ Long.parseLong(IOUtils.toString(loader.openStream(), StandardCharsets.UTF_8.name()));
+ assertThat(storedVersion).isGreaterThan((long) masterPlusOneCommit.getCommitTime());
+
+ verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, storedVersion, 0);
}
@Test
@@ -155,10 +201,11 @@
assertThat(ref).isNotNull();
ObjectLoader loader = repo.getRepository().open(ref.getObjectId());
- String storedVersion = IOUtils.toString(loader.openStream(), StandardCharsets.UTF_8.name());
- assertThat(Long.parseLong(storedVersion)).isEqualTo(masterCommit.getCommitTime());
+ long storedVersion =
+ Long.parseLong(IOUtils.toString(loader.openStream(), StandardCharsets.UTF_8.name()));
+ assertThat(storedVersion).isGreaterThan((long) masterCommit.getCommitTime());
- verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, masterCommit.getCommitTime(), 0);
+ verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, storedVersion, 0);
}
@Test
