Merge branch 'stable-3.5' into stable-3.6
* stable-3.5:
Bump global-refdb to v3.5.4
Do not include global-refdb library in high-availability
Bump global-refdb to v3.4.8
Fix issue with incorrect import for Nullable annotation
Cache the resolution of allowed listeners
Bump Gerrit to v3.4.5
Allow unrestricted listeners to be called for forwarded events
Change-Id: I5f55c12d22194a191a2c087c88210c502c831401
diff --git a/BUILD b/BUILD
index 9607415..7f32bc1 100644
--- a/BUILD
+++ b/BUILD
@@ -21,7 +21,7 @@
resources = glob(["src/main/resources/**/*"]),
deps = [
"@jgroups//jar",
- "@global-refdb//jar",
+ "@global-refdb//jar:neverlink",
],
)
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index 9af89f0..da477e6 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -15,6 +15,6 @@
maven_jar(
name = "global-refdb",
- artifact = "com.gerritforge:global-refdb:3.3.2.1:jdk8",
- sha1 = "7a293d577665dfc6f4d36371af21c4a3f7177b23",
+ artifact = "com.gerritforge:global-refdb:3.5.4",
+ sha1 = "6f96965d4cedd8b01b1fd9047d8c443c752bd675",
)
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/ConfigurableAllowedEventListeners.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/ConfigurableAllowedEventListeners.java
new file mode 100644
index 0000000..ea5f69c
--- /dev/null
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/ConfigurableAllowedEventListeners.java
@@ -0,0 +1,50 @@
+// Copyright (C) 2022 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.ericsson.gerrit.plugins.highavailability;
+
+import com.ericsson.gerrit.plugins.highavailability.forwarder.AllowedForwardedEventListener;
+import com.google.gerrit.server.events.EventListener;
+import com.google.inject.Inject;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+/** Configure the allowed listeners in high-availability.config */
+public class ConfigurableAllowedEventListeners implements AllowedForwardedEventListener {
+ private final Set<String> allowedListenerClasses;
+ private final ConcurrentHashMap<EventListener, Boolean> cachedAllowedListeners;
+
+ @Inject
+ ConfigurableAllowedEventListeners(Configuration config) {
+ allowedListenerClasses = config.event().allowedListeners();
+ cachedAllowedListeners = new ConcurrentHashMap<>();
+ }
+
+ @Override
+ public boolean isAllowed(EventListener listener) {
+ return cachedAllowedListeners.computeIfAbsent(listener, this::computeIsAllowed);
+ }
+
+ protected Boolean computeIsAllowed(EventListener listener) {
+ String listenerClassName = listener.getClass().getName();
+ boolean allowed = false;
+ while (!allowed && !listenerClassName.isEmpty()) {
+ allowed = allowedListenerClasses.contains(listenerClassName);
+ int lastDotPos = Math.max(listenerClassName.lastIndexOf('.'), 0);
+ listenerClassName = listenerClassName.substring(0, lastDotPos);
+ }
+
+ return allowed;
+ }
+}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
index f28b7c9..cdca21b 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
@@ -23,6 +23,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.server.config.ConfigUtil;
@@ -467,9 +468,18 @@
public static class Event extends Forwarding {
static final String EVENT_SECTION = "event";
+ static final String ALLOWED_LISTENERS = "allowedListeners";
+
+ private final Set<String> allowedListeners;
private Event(Config cfg) {
super(cfg, EVENT_SECTION);
+
+ allowedListeners = Sets.newHashSet(cfg.getStringList(EVENT_SECTION, null, ALLOWED_LISTENERS));
+ }
+
+ public Set<String> allowedListeners() {
+ return allowedListeners;
}
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/AllowedForwardedEventListener.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/AllowedForwardedEventListener.java
new file mode 100644
index 0000000..d79625a
--- /dev/null
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/AllowedForwardedEventListener.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2022 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.ericsson.gerrit.plugins.highavailability.forwarder;
+
+import com.google.gerrit.server.events.EventListener;
+
+/** Allow to trigger an event listener unconditionally. */
+public interface AllowedForwardedEventListener {
+
+ /**
+ * Control whether an event listener should be allowed unconditionally.
+ *
+ * @param listener the event listener
+ * @return true if the listener should be allowed, false otherwise
+ */
+ boolean isAllowed(EventListener listener);
+}
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 fb651b7..ce2caaf 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.common.Nullable;
import com.google.gerrit.server.config.GerritInstanceId;
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.EventBroker;
@@ -25,10 +26,11 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import java.util.Objects;
-import javax.annotation.Nullable;
class ForwardedAwareEventBroker extends EventBroker {
+ private final AllowedForwardedEventListener allowedListeners;
+
@Inject
ForwardedAwareEventBroker(
PluginSetContext<UserScopedEventListener> listeners,
@@ -36,7 +38,8 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
Factory notesFactory,
- @Nullable @GerritInstanceId String gerritInstanceId) {
+ @Nullable @GerritInstanceId String gerritInstanceId,
+ @Nullable AllowedForwardedEventListener allowedListeners) {
super(
listeners,
unrestrictedListeners,
@@ -44,6 +47,8 @@
projectCache,
notesFactory,
gerritInstanceId);
+
+ this.allowedListeners = allowedListeners;
}
private boolean isProducedByLocalInstance(Event event) {
@@ -59,8 +64,13 @@
}
// 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);
+ unrestrictedListeners.runEach(l -> fireEventForListener(l, event));
+ }
+
+ private void fireEventForListener(EventListener l, Event event) {
+ if (!Context.isForwardedEvent()
+ || (allowedListeners != null && allowedListeners.isAllowed(l))) {
+ l.onEvent(event);
}
}
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwarderModule.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwarderModule.java
index 99a820e..7cb107c 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwarderModule.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwarderModule.java
@@ -14,14 +14,19 @@
package com.ericsson.gerrit.plugins.highavailability.forwarder;
+import com.ericsson.gerrit.plugins.highavailability.ConfigurableAllowedEventListeners;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.server.events.EventDispatcher;
import com.google.inject.AbstractModule;
+import com.google.inject.Scopes;
public class ForwarderModule extends AbstractModule {
@Override
protected void configure() {
+ bind(AllowedForwardedEventListener.class)
+ .to(ConfigurableAllowedEventListeners.class)
+ .in(Scopes.SINGLETON);
DynamicItem.bind(binder(), EventDispatcher.class).to(ForwardedAwareEventBroker.class);
}
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 2f7d41a..e7ce6b2 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -175,6 +175,12 @@
Defaults to an empty list, meaning only evictions of the core caches are
forwarded.
+```event.allowedListeners```
+: Class name or package name of the event listener that is always allowed to receive
+ all events generated locally or from a remote end.
+ Can be specified multiple times for allowing multiple listeners classes or packages.
+ Defaults to an empty list.
+
```event.synchronize```
: Whether to synchronize stream events.
Defaults to true.
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java
index 19cc48d..fb1b8d0 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/ConfigurationTest.java
@@ -20,6 +20,7 @@
import static com.ericsson.gerrit.plugins.highavailability.Configuration.DEFAULT_NUM_STRIPED_LOCKS;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.DEFAULT_THREAD_POOL_SIZE;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.DEFAULT_TIMEOUT_MS;
+import static com.ericsson.gerrit.plugins.highavailability.Configuration.Event.ALLOWED_LISTENERS;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.Event.EVENT_SECTION;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.Forwarding.DEFAULT_SYNCHRONIZE;
import static com.ericsson.gerrit.plugins.highavailability.Configuration.Forwarding.SYNCHRONIZE_KEY;
@@ -68,10 +69,14 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.events.Event;
+import com.google.gerrit.server.events.EventListener;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.Arrays;
import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
@@ -331,6 +336,88 @@
}
@Test
+ public void testGetEventAllowedListener() throws Exception {
+ assertThat(getConfiguration().event().allowedListeners()).isEmpty();
+
+ List<String> allowedListeners = Arrays.asList("listener1", "listener2");
+ globalPluginConfig.setStringList(EVENT_SECTION, null, ALLOWED_LISTENERS, allowedListeners);
+ assertThat(getConfiguration().event().allowedListeners())
+ .containsExactlyElementsIn(allowedListeners);
+ }
+
+ @Test
+ public void testConfiguredListenerShouldBeAllowed() throws Exception {
+ EventListener listener =
+ new EventListener() {
+
+ @Override
+ public void onEvent(Event event) {}
+ };
+ assertThat(new ConfigurableAllowedEventListeners(getConfiguration()).isAllowed(listener))
+ .isFalse();
+
+ globalPluginConfig.setString(
+ EVENT_SECTION, null, ALLOWED_LISTENERS, listener.getClass().getName());
+
+ assertThat(new ConfigurableAllowedEventListeners(getConfiguration()).isAllowed(listener))
+ .isTrue();
+
+ globalPluginConfig.setString(
+ EVENT_SECTION, null, ALLOWED_LISTENERS, listener.getClass().getPackageName());
+
+ assertThat(new ConfigurableAllowedEventListeners(getConfiguration()).isAllowed(listener))
+ .isTrue();
+ }
+
+ @Test
+ public void testConfiguredListenerAllowedShouldBeCached() throws Exception {
+ AtomicInteger allowedListenerResolutionCount = new AtomicInteger();
+ EventListener listener =
+ new EventListener() {
+
+ @Override
+ public void onEvent(Event event) {}
+ };
+ assertThat(new ConfigurableAllowedEventListeners(getConfiguration()).isAllowed(listener))
+ .isFalse();
+
+ globalPluginConfig.setString(
+ EVENT_SECTION, null, ALLOWED_LISTENERS, listener.getClass().getName());
+
+ ConfigurableAllowedEventListeners allowedEventListener =
+ new ConfigurableAllowedEventListeners(getConfiguration()) {
+ @Override
+ protected Boolean computeIsAllowed(EventListener listener) {
+ allowedListenerResolutionCount.incrementAndGet();
+ return super.computeIsAllowed(listener);
+ }
+ };
+
+ for (int i = 0; i < 2; i++) {
+ assertThat(allowedEventListener.isAllowed(listener)).isTrue();
+ assertThat(allowedListenerResolutionCount.get()).isEqualTo(1);
+ }
+ }
+
+ @Test
+ public void testConfiguredPackageOfListenerShouldBeAllowed() throws Exception {
+ EventListener listener =
+ new EventListener() {
+
+ @Override
+ public void onEvent(Event event) {}
+ };
+ assertThat(new ConfigurableAllowedEventListeners(getConfiguration()).isAllowed(listener))
+ .isFalse();
+
+ globalPluginConfig.setString(
+ EVENT_SECTION, null, ALLOWED_LISTENERS, listener.getClass().getPackageName());
+
+ assertThat(new ConfigurableAllowedEventListeners(getConfiguration()).isAllowed(listener))
+ .isTrue();
+ }
+
+ @Test
public void testGetDefaultSharedDirectory() throws Exception {
assertEquals(
getConfiguration().main().sharedDirectory(), sitePaths.resolve(DEFAULT_SHARED_DIRECTORY));
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 cfe0475..7142841 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
@@ -14,9 +14,11 @@
package com.ericsson.gerrit.plugins.highavailability.forwarder;
+import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.events.Event;
@@ -29,6 +31,7 @@
public class ForwardedAwareEventBrokerTest {
private EventListener listenerMock;
+ private AllowedForwardedEventListener allowListenerMock;
private ForwardedAwareEventBroker broker;
private ForwardedAwareEventBroker brokerWithGerritInstanceId;
private Event event;
@@ -38,13 +41,16 @@
public void setUp() {
PluginMetrics mockMetrics = mock(PluginMetrics.class);
listenerMock = mock(EventListener.class);
+ allowListenerMock = mock(AllowedForwardedEventListener.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, null);
+ broker =
+ new ForwardedAwareEventBroker(null, listeners, null, null, null, null, allowListenerMock);
brokerWithGerritInstanceId =
- new ForwardedAwareEventBroker(null, listeners, null, null, null, gerritInstanceId);
+ new ForwardedAwareEventBroker(
+ null, listeners, null, null, null, gerritInstanceId, allowListenerMock);
}
@Test
@@ -97,6 +103,18 @@
}
@Test
+ public void shouldDispatchEventWhenInstanceIdsAreDifferentToAllowedListener() {
+ event.instanceId = "some-other-gerrit-instance-id";
+ when(allowListenerMock.isAllowed(any())).thenReturn(true);
+ try {
+ brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event);
+ } finally {
+ Context.unsetForwardedEvent();
+ }
+ verify(listenerMock).onEvent(event);
+ }
+
+ @Test
public void shouldDispatchEventWhenInstanceIdsAreEqual() {
event.instanceId = gerritInstanceId;
brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event);
