Merge branch 'stable-3.6' into stable-3.7 * stable-3.6: Update the refs/multi-site/version/value once per batch-refupdate Update refs/multi-site/version for starred changes and sequences Remove dead code when updating project version meta-ref Fix merge Ib6aa8adbd causing 2x updates of refs/multi-site/version/* Change-Id: Ibc60832e51bf836f02d46d0da07785848604663e
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java index d2de068..8c67823 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java
@@ -16,11 +16,10 @@ import com.gerritforge.gerrit.eventbroker.publisher.StreamEventPublisherConfig; import com.gerritforge.gerrit.eventbroker.publisher.StreamEventPublisherModule; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.lifecycle.LifecycleModule; -import com.google.gerrit.server.events.EventListener; import com.google.inject.Inject; -import com.google.inject.Scopes; import com.google.inject.multibindings.OptionalBinder; import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.events.EventTopic; @@ -48,11 +47,9 @@ OptionalBinder<ProjectVersionRefUpdate> projectVersionRefUpdateBinder = OptionalBinder.newOptionalBinder(binder(), ProjectVersionRefUpdate.class); if (configuration.getSharedRefDbConfiguration().getSharedRefDb().isEnabled()) { - DynamicSet.bind(binder(), EventListener.class).to(ProjectVersionRefUpdateImpl.class); - projectVersionRefUpdateBinder - .setBinding() - .to(ProjectVersionRefUpdateImpl.class) - .in(Scopes.SINGLETON); + DynamicSet.bind(binder(), GitBatchRefUpdateListener.class) + .to(ProjectVersionRefUpdateImpl.class); + projectVersionRefUpdateBinder.setBinding().to(ProjectVersionRefUpdateImpl.class); } } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java index d412913..150ea52 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java
@@ -24,14 +24,11 @@ import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; -import com.google.gerrit.entities.RefNames; -import com.google.gerrit.server.config.GerritInstanceId; -import com.google.gerrit.server.events.Event; -import com.google.gerrit.server.events.EventListener; -import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.inject.Inject; +import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -43,7 +40,9 @@ import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; -public class ProjectVersionRefUpdateImpl implements EventListener, ProjectVersionRefUpdate { +@Singleton +public class ProjectVersionRefUpdateImpl + implements GitBatchRefUpdateListener, ProjectVersionRefUpdate { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Set<RefUpdate.Result> SUCCESSFUL_RESULTS = ImmutableSet.of(RefUpdate.Result.NEW, RefUpdate.Result.FORCED, RefUpdate.Result.NO_CHANGE); @@ -51,7 +50,6 @@ private final GitRepositoryManager gitRepositoryManager; private final GitReferenceUpdated gitReferenceUpdated; private final ProjectVersionLogger verLogger; - private final String nodeInstanceId; protected final SharedRefDatabaseWrapper sharedRefDb; @@ -60,61 +58,43 @@ GitRepositoryManager gitRepositoryManager, SharedRefDatabaseWrapper sharedRefDb, GitReferenceUpdated gitReferenceUpdated, - ProjectVersionLogger verLogger, - @GerritInstanceId String nodeInstanceId) { + ProjectVersionLogger verLogger) { this.gitRepositoryManager = gitRepositoryManager; this.sharedRefDb = sharedRefDb; this.gitReferenceUpdated = gitReferenceUpdated; this.verLogger = verLogger; - this.nodeInstanceId = nodeInstanceId; } @Override - public void onEvent(Event event) { - logger.atFine().log("Processing event type: %s", event.type); + public void onGitBatchRefUpdate(Event event) { // Producer of the Event use RefUpdatedEvent to trigger the version update - if (nodeInstanceId.equals(event.instanceId) && event instanceof RefUpdatedEvent) { - updateProducerProjectVersionUpdate((RefUpdatedEvent) event); - } + updateProducerProjectVersionUpdate(event); } - private boolean isSpecialRefName(String refName) { - return refName.startsWith(RefNames.REFS_SEQUENCES) - || refName.startsWith(RefNames.REFS_STARRED_CHANGES) - || refName.equals(MULTI_SITE_VERSIONING_REF); - } - - private void updateProducerProjectVersionUpdate(RefUpdatedEvent refUpdatedEvent) { - String refName = refUpdatedEvent.getRefName(); - - if (isSpecialRefName(refName)) { + private void updateProducerProjectVersionUpdate(Event refUpdatedEvent) { + if (refUpdatedEvent.getRefNames().stream() + .allMatch(refName -> refName.equals(MULTI_SITE_VERSIONING_REF))) { logger.atFine().log( "Found a special ref name %s, skipping update for %s", - refName, refUpdatedEvent.getProjectNameKey().get()); + MULTI_SITE_VERSIONING_REF, refUpdatedEvent.getProjectName()); return; } + try { - Project.NameKey projectNameKey = refUpdatedEvent.getProjectNameKey(); + Project.NameKey projectNameKey = Project.nameKey(refUpdatedEvent.getProjectName()); long newVersion = getCurrentGlobalVersionNumber(); - Optional<RefUpdate> newProjectVersionRefUpdate = - updateLocalProjectVersion(projectNameKey, newVersion); + RefUpdate newProjectVersionRefUpdate = updateLocalProjectVersion(projectNameKey, newVersion); - if (newProjectVersionRefUpdate.isPresent()) { - verLogger.log(projectNameKey, newVersion, 0L); + verLogger.log(projectNameKey, newVersion, 0L); - if (updateSharedProjectVersion(projectNameKey, newVersion)) { - gitReferenceUpdated.fire(projectNameKey, newProjectVersionRefUpdate.get(), null); - } - } else { - logger.atWarning().log( - "Ref %s not found on projet %s: skipping project version update", - refUpdatedEvent.getRefName(), projectNameKey); + if (updateSharedProjectVersion(projectNameKey, newVersion)) { + gitReferenceUpdated.fire(projectNameKey, newProjectVersionRefUpdate, null); } } catch (LocalProjectVersionUpdateException | SharedProjectVersionUpdateException e) { logger.atSevere().withCause(e).log( "Issue encountered when updating version for project %s", - refUpdatedEvent.getProjectNameKey()); + refUpdatedEvent.getProjectName()); } } @@ -247,8 +227,7 @@ } @SuppressWarnings("FloggerLogString") - private Optional<RefUpdate> updateLocalProjectVersion( - Project.NameKey projectNameKey, long newVersionNumber) + private RefUpdate updateLocalProjectVersion(Project.NameKey projectNameKey, long newVersionNumber) throws LocalProjectVersionUpdateException { logger.atFine().log( "Updating local version for project %s with version %d", @@ -265,7 +244,7 @@ throw new LocalProjectVersionUpdateException(message); } - return Optional.of(refUpdate); + return refUpdate; } catch (IOException e) { String message = "Cannot create versioning command for " + projectNameKey.get(); logger.atSevere().withCause(e).log(message);
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 1cfb2cd..e2e3113 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
@@ -28,11 +28,9 @@ import static org.mockito.Mockito.when; import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDatabaseWrapper; -import com.google.common.base.Suppliers; import com.google.gerrit.entities.Project; -import com.google.gerrit.entities.RefNames; -import com.google.gerrit.server.data.RefUpdateAttribute; -import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.testing.InMemoryRepositoryManager; @@ -44,6 +42,7 @@ import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.util.Optional; +import java.util.Set; import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; @@ -51,6 +50,7 @@ import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.transport.ReceiveCommand; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -61,11 +61,14 @@ @RunWith(MockitoJUnitRunner.class) public class ProjectVersionRefUpdateTest implements RefFixture { - private static final String DEFAULT_INSTANCE_ID = "instance-id"; + private static final int A_TEST_ACCOUNT_ID = 1; + private static final GitReferenceUpdated.UpdatedRef A_TEST_UPDATED_REF = + new GitReferenceUpdated.UpdatedRef( + A_TEST_REF_NAME, ObjectId.zeroId(), ObjectId.zeroId(), ReceiveCommand.Type.UPDATE); @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment(); - @Mock RefUpdatedEvent refUpdatedEvent; + @Mock GitBatchRefUpdateListener.Event refUpdatedEvent; @Mock SharedRefDatabaseWrapper sharedRefDb; @Mock GitReferenceUpdated gitReferenceUpdated; @Mock ProjectVersionLogger verLogger; @@ -78,7 +81,6 @@ @Before public void setUp() throws Exception { - refUpdatedEvent.instanceId = DEFAULT_INSTANCE_ID; InMemoryRepository inMemoryRepo = repoManager.createRepository(A_TEST_PROJECT_NAME_KEY); project = projectConfigFactory.create(A_TEST_PROJECT_NAME_KEY); project.load(inMemoryRepo); @@ -95,12 +97,11 @@ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1))); 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); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -117,19 +118,47 @@ } @Test + public void producerShouldUpdateProjectVersionOnceUponMultipleRefUpdatedEvent() { + when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) + .thenReturn(true); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()) + .thenReturn(Set.of(A_TEST_REF_NAME, A_REF_NAME_OF_A_PATCHSET)); + + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); + + verify(sharedRefDb, atMost(1)) + .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)); + } + + @Test + public void producerShouldUpdateProjectVersionOnceUponMultipleRefWithOneMetaUpdatedEvent() { + when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) + .thenReturn(true); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()) + .thenReturn(Set.of(A_TEST_REF_NAME, MULTI_SITE_VERSIONING_REF)); + + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); + + verify(sharedRefDb, atMost(1)) + .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)); + } + + @Test public void producerShouldUsePutInsteadOfCompareAndPutWhenExtendedGlobalRefDb() throws IOException { when(sharedRefDb.isSetOperationSupported()).thenReturn(true); - RefUpdatedEvent refUpdatedEvent = new RefUpdatedEvent(); - refUpdatedEvent.instanceId = DEFAULT_INSTANCE_ID; - RefUpdateAttribute refUpdatedAttribute = new RefUpdateAttribute(); - refUpdatedAttribute.project = A_TEST_PROJECT_NAME_KEY.get(); - refUpdatedAttribute.refName = A_TEST_REF_NAME; - refUpdatedEvent.refUpdate = Suppliers.memoize(() -> refUpdatedAttribute); + GitBatchRefUpdateListener.Event refUpdatedEvent = + new GitReferenceUpdated.GitBatchRefUpdateEvent( + A_TEST_PROJECT_NAME_KEY, + Set.of(A_TEST_UPDATED_REF), + new AccountInfo(A_TEST_ACCOUNT_ID)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -160,13 +189,12 @@ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1))); 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); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); ProjectVersionRefUpdateImpl projectVersion = - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID); - projectVersion.onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger); + projectVersion.onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -197,12 +225,11 @@ 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); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -218,63 +245,19 @@ verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, storedVersion, 0); } - @Test - public void producerShouldNotUpdateProjectVersionUponSequenceRefUpdatedEvent() throws Exception { - producerShouldNotUpdateProjectVersionUponMagicRefUpdatedEvent(RefNames.REFS_SEQUENCES); - } - - @Test - public void producerShouldNotUpdateProjectVersionUponStarredChangesRefUpdatedEvent() - throws Exception { - producerShouldNotUpdateProjectVersionUponMagicRefUpdatedEvent(RefNames.REFS_STARRED_CHANGES); - } - private long readLongObject(ObjectLoader loader) throws LargeObjectException, UnsupportedEncodingException { String boutString = new String(loader.getBytes(), StandardCharsets.UTF_8.name()); return Long.parseLong(boutString); } - private void producerShouldNotUpdateProjectVersionUponMagicRefUpdatedEvent(String magicRefPrefix) - throws Exception { - String magicRefName = magicRefPrefix + "/foo"; - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(magicRefName); - repo.branch(magicRefName).commit().create(); - - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); - - Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); - assertThat(ref).isNull(); - - verifyNoInteractions(verLogger); - } - - @Test - public void producerShouldNotUpdateProjectVersionUponForwardedRefUpdatedEvent() - throws IOException { - refUpdatedEvent.instanceId = "instance-id-2"; - - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); - - Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); - assertThat(ref).isNull(); - - verifyNoInteractions(verLogger); - } - @Test public void shouldNotUpdateProjectVersionWhenProjectDoesntExist() throws IOException { - when(refUpdatedEvent.getProjectNameKey()).thenReturn(Project.nameKey("aNonExistentProject")); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn("aNonExistentProject"); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); assertThat(ref).isNull(); @@ -288,8 +271,7 @@ .thenReturn(Optional.of("123")); Optional<Long> version = - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) .getProjectRemoteVersion(A_TEST_PROJECT_NAME); assertThat(version.isPresent()).isTrue();