Merge branch 'stable-3.1' into stable-3.2
* stable-3.1:
e2e-tests: Rename 'default' after GerritSimulation core change Id33ada17
e2e-tests: Rename 'unique' after GerritSimulation core change I229976f5
e2e-tests: Rename 'name' after GerritSimulation core change I3527fb39
e2e-tests: Remove unnecessary type specifications
Change-Id: I6db5b7568fdfc348a0f40e64f2c1119f97a7a77c
diff --git a/BUILD b/BUILD
index c3f5a11..3b896ff 100644
--- a/BUILD
+++ b/BUILD
@@ -25,6 +25,7 @@
junit_tests(
name = "high-availability_tests",
srcs = glob(["src/test/java/**/*.java"]),
+ javacopts = ["-Xep:DoNotMock:OFF"],
resources = glob(["src/test/resources/**/*"]),
tags = [
"high-availability",
diff --git a/WORKSPACE b/WORKSPACE
index 039af8d..7e3bf7c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -3,7 +3,7 @@
load("//:bazlets.bzl", "load_bazlets")
load_bazlets(
- commit = "3f9dadc615dc4053369a42d9ada37dafd8d4763c",
+ commit = "0f81174e3d1b892a1342ebc75bb4bbb158ae0efe",
#local_path = "/home/<user>/projects/bazlets",
)
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java
index 6255986..ff42a4e 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java
@@ -113,12 +113,12 @@
currCtx.onlyWithContext(
(ctx) -> {
try {
- ChangeNotes changeNotes = changeFinder.findOne(projectName + "~" + id);
+ Optional<ChangeNotes> changeNotes = changeFinder.findOne(projectName + "~" + id);
update(
IndexName.CHANGE,
- changeNotes == null
+ !changeNotes.isPresent()
? LocalDateTime.now()
- : changeNotes.getChange().getLastUpdatedOn().toLocalDateTime());
+ : changeNotes.get().getChange().getLastUpdatedOn().toLocalDateTime());
} catch (Exception e) {
log.atWarning().withCause(e).log("Unable to update the latest TS for change %d", id);
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java
index f156da4..fb651b7 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java
@@ -14,6 +14,7 @@
package com.ericsson.gerrit.plugins.highavailability.forwarder;
+import com.google.gerrit.server.config.GerritInstanceId;
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.EventBroker;
import com.google.gerrit.server.events.EventListener;
@@ -23,6 +24,8 @@
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
+import java.util.Objects;
+import javax.annotation.Nullable;
class ForwardedAwareEventBroker extends EventBroker {
@@ -32,12 +35,30 @@
PluginSetContext<EventListener> unrestrictedListeners,
PermissionBackend permissionBackend,
ProjectCache projectCache,
- Factory notesFactory) {
- super(listeners, unrestrictedListeners, permissionBackend, projectCache, notesFactory);
+ Factory notesFactory,
+ @Nullable @GerritInstanceId String gerritInstanceId) {
+ super(
+ listeners,
+ unrestrictedListeners,
+ permissionBackend,
+ projectCache,
+ notesFactory,
+ gerritInstanceId);
+ }
+
+ private boolean isProducedByLocalInstance(Event event) {
+ return Objects.equals(event.instanceId, gerritInstanceId);
}
@Override
protected void fireEventForUnrestrictedListeners(Event event) {
+ // An event should not be dispatched when it is "forwarded".
+ // Meaning, it was either produced somewhere else
+ if (!isProducedByLocalInstance(event)) {
+ Context.setForwardedEvent(true);
+ }
+ // or it was consumed by the high-availability rest endpoint and
+ // thus the context of its consumption has already been set to "forwarded".
if (!Context.isForwardedEvent()) {
super.fireEventForUnrestrictedListeners(event);
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java
index 4a0cad7..fe04ee9 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/ChangeCheckerImpl.java
@@ -76,7 +76,7 @@
@Override
public Optional<ChangeNotes> getChangeNotes() {
try (ManualRequestContext ctx = oneOffReqCtx.open()) {
- changeNotes = Optional.ofNullable(changeFinder.findOne(changeId));
+ changeNotes = changeFinder.findOne(changeId);
return changeNotes;
}
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index b2715bb..1440e0b 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -3,8 +3,8 @@
=========================
The @PLUGIN@ plugin must be installed on all the instances. Each instance should
-be configured with the same [gerrit.serverId](https://gerrit-documentation.storage.googleapis.com/Documentation/3.1.0/config-gerrit.html#gerrit.serverId).
-If there are existing changes in [NoteDb](https://gerrit-documentation.storage.googleapis.com/Documentation/3.1.0/note-db.html)
+be configured with the same [gerrit.serverId](https://gerrit-documentation.storage.googleapis.com/Documentation/3.2.0/config-gerrit.html#gerrit.serverId).
+If there are existing changes in [NoteDb](https://gerrit-documentation.storage.googleapis.com/Documentation/3.2.0/note-db.html)
made with another `serverId`, then this plugin might not be able to access them.
Likewise, if the HA gerrit.serverIds differ, then changes conveyed by one
instance will not be accessible by the other.
@@ -236,3 +236,23 @@
```healthcheck.enable```
: Whether to enable the health check endpoint. Defaults to 'true'.
+
+File 'gerrit.config'
+--------------------
+
+```gerrit.instanceId```
+: Optional identifier for this Gerrit instance.
+[[docs](https://gerrit-documentation.storage.googleapis.com/Documentation/3.2.0/config-gerrit.html#gerrit.instanceId)].
+
+Whilst this is not, specifically, a high-availability plugin configuration, it plays
+an important role on which events are forwarded to peers.
+
+If `instanceId` is set, events produced by that Gerrit instance will contain its
+origin in the `instanceId` field, allowing to track where they are coming from.
+
+The high-availability plugin will check the `instanceId` value to decide whether
+an event should be forwarded: events that originated from different Gerrit instances
+will not be forwarded.
+
+When neither `gerrit.instanceId` nor `event.instanceId` are set, it is not possible
+to identify the origin of the event and thus the event is always forwarded.
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java
index e91e2ae..8918c65 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java
@@ -26,8 +26,8 @@
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import com.google.common.cache.LoadingCache;
-import com.google.gerrit.acceptance.GerritConfig;
-import com.google.gerrit.acceptance.GlobalPluginConfig;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.TestPlugin;
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/ProjectListIT.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/ProjectListIT.java
index 22ef89e..dd51ba4 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/ProjectListIT.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/ProjectListIT.java
@@ -25,7 +25,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import com.google.gerrit.acceptance.GlobalPluginConfig;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.TestPlugin;
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java
index 36cc953..cfe0475 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java
@@ -30,7 +30,9 @@
private EventListener listenerMock;
private ForwardedAwareEventBroker broker;
- private Event event = new TestEvent();
+ private ForwardedAwareEventBroker brokerWithGerritInstanceId;
+ private Event event;
+ private String gerritInstanceId = "gerrit-instance-id";
@Before
public void setUp() {
@@ -38,8 +40,11 @@
listenerMock = mock(EventListener.class);
DynamicSet<EventListener> set = DynamicSet.emptySet();
set.add("high-availability", listenerMock);
+ event = new TestEvent();
PluginSetContext<EventListener> listeners = new PluginSetContext<>(set, mockMetrics);
- broker = new ForwardedAwareEventBroker(null, listeners, null, null, null);
+ broker = new ForwardedAwareEventBroker(null, listeners, null, null, null, null);
+ brokerWithGerritInstanceId =
+ new ForwardedAwareEventBroker(null, listeners, null, null, null, gerritInstanceId);
}
@Test
@@ -58,4 +63,43 @@
}
verifyZeroInteractions(listenerMock);
}
+
+ @Test
+ public void shouldNotDispatchEventWhenEventInstanceIdIsDefinedButGerritInstanceIdIsNot() {
+ event.instanceId = gerritInstanceId;
+ try {
+ broker.fireEventForUnrestrictedListeners(event);
+ } finally {
+ Context.unsetForwardedEvent();
+ }
+ verifyZeroInteractions(listenerMock);
+ }
+
+ @Test
+ public void shouldNotDispatchEventWhenGerritInstanceIdIsDefinedButEventInstanceIdIsNot() {
+ try {
+ brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event);
+ } finally {
+ Context.unsetForwardedEvent();
+ }
+ verifyZeroInteractions(listenerMock);
+ }
+
+ @Test
+ public void shouldNotDispatchEventWhenInstanceIdsAreDifferent() {
+ event.instanceId = "some-other-gerrit-instance-id";
+ try {
+ brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event);
+ } finally {
+ Context.unsetForwardedEvent();
+ }
+ verifyZeroInteractions(listenerMock);
+ }
+
+ @Test
+ public void shouldDispatchEventWhenInstanceIdsAreEqual() {
+ event.instanceId = gerritInstanceId;
+ brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event);
+ verify(listenerMock).onEvent(event);
+ }
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AbstractIndexForwardingIT.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AbstractIndexForwardingIT.java
index af4637e..bd4dfd1 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AbstractIndexForwardingIT.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/AbstractIndexForwardingIT.java
@@ -26,7 +26,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import com.google.gerrit.acceptance.GlobalPluginConfig;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.TestPlugin;
