Ignore events coming from another Gerrit server Fix any multi-Gerrit server scenario, where the its-* plugins are installed on all Gerrit servers and they are all receiving the same events. Do not process events when are received from a non-local Gerrit server. That is fixing a known issue where its-* actions were executed multiple times, once per Gerrit server. That was due to the inability to detect whether the event was generated locally or not. All its-* plugins on all Gerrit servers on the cluster were processing the same event creating duplicates on the associated issue tracker system. Introduce a new GerritInstanceIdTest annotation with the purpose of injecting different instance-ids and testing the different scenarios: - No instance-id defined (single-server) - Foreign instance-id - Local instance-id Bug: Issue 13089 Change-Id: Ie25ba427a8cb9a48d442a14ccfbb2854d0ae5ef9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java index af5e9a9..3429ba4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
@@ -17,11 +17,13 @@ import static java.util.stream.Collectors.toList; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.api.projects.CommentLinkInfo; +import com.google.gerrit.server.config.GerritInstanceId; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.PluginConfigFactory; @@ -55,6 +57,7 @@ private final ProjectCache projectCache; private final PluginConfigFactory pluginCfgFactory; private final Config gerritConfig; + private String instanceId; private static final ThreadLocal<Project.NameKey> currentProjectName = ThreadLocal.withInitial(() -> null); @@ -68,16 +71,25 @@ @PluginName String pluginName, ProjectCache projectCache, PluginConfigFactory pluginCfgFactory, - @GerritServerConfig Config gerritConfig) { + @GerritServerConfig Config gerritConfig, + @Nullable @GerritInstanceId String instanceId) { this.pluginName = pluginName; this.projectCache = projectCache; this.pluginCfgFactory = pluginCfgFactory; this.gerritConfig = gerritConfig; + this.instanceId = instanceId; } // Plugin enablement -------------------------------------------------------- public boolean isEnabled(RefEvent event) { + if ((instanceId == null && event.instanceId != null) + || (instanceId != null && !instanceId.equals(event.instanceId))) { + logger.atFine().log( + "Event %s is coming from a remote Gerrit instance-id (%s)", event, event.instanceId); + return false; + } + if (event instanceof PatchSetCreatedEvent || event instanceof CommentAddedEvent || event instanceof ChangeMergedEvent
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java index 34068bb..56f8ba4 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java
@@ -14,6 +14,7 @@ package com.googlesource.gerrit.plugins.its.base.its; +import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -24,6 +25,7 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.config.FactoryModule; +import com.google.gerrit.server.config.GerritInstanceId; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.PluginConfigFactory; @@ -39,8 +41,13 @@ import com.google.gerrit.server.project.ProjectState; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.util.Providers; import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase; import com.googlesource.gerrit.plugins.its.base.validation.ItsAssociationPolicy; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.lang.reflect.Method; import java.util.Arrays; import java.util.Optional; import org.eclipse.jgit.lib.Config; @@ -52,6 +59,17 @@ private PluginConfigFactory pluginConfigFactory; private Config serverConfig; + @Retention(RUNTIME) + @Target(ElementType.METHOD) + static @interface GerritInstanceIdTest {} + + static final String LOCAL_INSTANCE_ID = "local-gerrit-instance-id"; + static final String FOREIGN_INSTANCE_ID = "foreign-gerrit-instance-id"; + + public void enableItsConfig() { + setupIsEnabled("true", null, null, new String[] {}); + } + public void setupIsEnabled( String enabled, String itsProject, String parentEnabled, String[] branches) { ProjectState projectState = mock(ProjectState.class); @@ -253,6 +271,28 @@ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch")); } + @GerritInstanceIdTest + public void testIsNotEnabledEventFromForeignInstanceId() { + enableItsConfig(); + + assertFalse(createItsConfig().isEnabled(newEventFromGerritInstanceId(FOREIGN_INSTANCE_ID))); + assertLogMessageContains("is coming from a remote Gerrit instance-id"); + } + + @GerritInstanceIdTest + public void testIsEnabledEventFromLocalInstanceId() { + enableItsConfig(); + + assertTrue(createItsConfig().isEnabled(newEventFromGerritInstanceId(LOCAL_INSTANCE_ID))); + } + + public void testIsNotEnabledEventWithRemoteInstanceIdButNotDefinedLocally() { + enableItsConfig(); + + assertFalse(createItsConfig().isEnabled(newEventFromGerritInstanceId(FOREIGN_INSTANCE_ID))); + assertLogMessageContains("is coming from a remote Gerrit instance-id"); + } + public void testIsEnabledEventNoBranches() { String[] branches = {}; setupIsEnabled("true", null, null, branches); @@ -709,6 +749,12 @@ .getEnum("plugin", "ItsTestName", "association", ItsAssociationPolicy.MANDATORY); } + private PatchSetCreatedEvent newEventFromGerritInstanceId(String instanceId) { + PatchSetCreatedEvent event = new PatchSetCreatedEvent(testChange("testProject", "testBranch")); + event.instanceId = instanceId; + return event; + } + private ItsConfig createItsConfig() { return injector.getInstance(ItsConfig.class); } @@ -720,10 +766,20 @@ @Override public void setUp() throws Exception { super.setUp(); - injector = Guice.createInjector(new TestModule()); + + Method testMethod = ItsConfigTest.class.getMethod(getName()); + String gerritInstanceIdTest = + testMethod.getAnnotation(GerritInstanceIdTest.class) == null ? null : LOCAL_INSTANCE_ID; + injector = Guice.createInjector(new TestModule(gerritInstanceIdTest)); } private class TestModule extends FactoryModule { + private final String gerritInstanceId; + + TestModule(String gerritInstanceId) { + this.gerritInstanceId = gerritInstanceId; + } + @Override protected void configure() { projectCache = mock(ProjectCache.class); @@ -736,6 +792,10 @@ serverConfig = mock(Config.class); bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(serverConfig); + + bind(String.class) + .annotatedWith(GerritInstanceId.class) + .toProvider(Providers.of(gerritInstanceId)); } } }