Replace EventHandler with StreamEventPublisher
Currently both EventHandler and StreamEventPublisher provide the same
functionality. Remove EventHandler and switch EventTopic.STREAM_EVENT_TOPIC
key to `gerritEvents` and default value to `gerrit`.
Bug: Issue 14907
Change-Id: Ia47de40fabc571de271bf61a79f4d1892991391a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutor.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutor.java
deleted file mode 100644
index 07bbbf7..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutor.java
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright (C) 2015 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.googlesource.gerrit.plugins.multisite.event;
-
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import com.google.inject.BindingAnnotation;
-import java.lang.annotation.Retention;
-
-@Retention(RUNTIME)
-@BindingAnnotation
-@interface EventExecutor {}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutorProvider.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutorProvider.java
deleted file mode 100644
index e4979f9..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutorProvider.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (C) 2015 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.googlesource.gerrit.plugins.multisite.event;
-
-import com.google.gerrit.server.git.WorkQueue;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import com.googlesource.gerrit.plugins.multisite.ExecutorProvider;
-
-@Singleton
-class EventExecutorProvider extends ExecutorProvider {
-
- @Inject
- EventExecutorProvider(WorkQueue workQueue) {
- super(workQueue, 1, "Forward-Stream-Event");
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventHandler.java
deleted file mode 100644
index e001e15..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventHandler.java
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright (C) 2015 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.googlesource.gerrit.plugins.multisite.event;
-
-import com.google.gerrit.extensions.registration.DynamicSet;
-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.ProjectEvent;
-import com.google.inject.Inject;
-import com.googlesource.gerrit.plugins.multisite.forwarder.StreamEventForwarder;
-import java.util.concurrent.Executor;
-
-class EventHandler implements EventListener {
- private final Executor executor;
- private final DynamicSet<StreamEventForwarder> forwarders;
- private final String nodeInstanceId;
-
- @Inject
- EventHandler(
- DynamicSet<StreamEventForwarder> forwarders,
- @EventExecutor Executor executor,
- @GerritInstanceId String nodeInstanceId) {
- this.forwarders = forwarders;
- this.executor = executor;
- this.nodeInstanceId = nodeInstanceId;
- }
-
- @Override
- public void onEvent(Event event) {
-
- if (nodeInstanceId.equals(event.instanceId) && event instanceof ProjectEvent) {
- executor.execute(new EventTask(event));
- }
- }
-
- class EventTask implements Runnable {
- private final Event event;
-
- EventTask(Event event) {
- this.event = event;
- }
-
- @Override
- public void run() {
- forwarders.forEach(f -> f.send(event));
- }
-
- @Override
- public String toString() {
- return String.format("Send event '%s' to target instance", event.type);
- }
- }
-}
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 663d8e5..94d7867 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
@@ -24,7 +24,6 @@
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.EventTopic;
import com.googlesource.gerrit.plugins.multisite.validation.ProjectVersionRefUpdate;
-import java.util.concurrent.Executor;
public class EventModule extends LifecycleModule {
private final Configuration configuration;
@@ -36,13 +35,10 @@
@Override
protected void configure() {
- bind(Executor.class).annotatedWith(EventExecutor.class).toProvider(EventExecutorProvider.class);
- listener().to(EventExecutorProvider.class);
- DynamicSet.bind(binder(), EventListener.class).to(EventHandler.class);
DynamicSet.bind(binder(), EventListener.class).to(ProjectVersionRefUpdate.class);
bind(new TypeLiteral<String>() {})
.annotatedWith(Names.named(StreamEventPublisher.STREAM_EVENTS_TOPIC))
- .toInstance(EventTopic.GERRIT_TOPIC.topic(configuration));
+ .toInstance(EventTopic.STREAM_EVENT_TOPIC.topic(configuration));
DynamicSet.bind(binder(), EventListener.class).to(StreamEventPublisher.class);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwarderModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwarderModule.java
index ca17004..1d36e23 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwarderModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwarderModule.java
@@ -24,6 +24,5 @@
DynamicSet.setOf(binder(), CacheEvictionForwarder.class);
DynamicSet.setOf(binder(), IndexEventForwarder.class);
DynamicSet.setOf(binder(), ProjectListUpdateForwarder.class);
- DynamicSet.setOf(binder(), StreamEventForwarder.class);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/StreamEventForwarder.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/StreamEventForwarder.java
deleted file mode 100644
index 79a9af2..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/StreamEventForwarder.java
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright (C) 2019 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.googlesource.gerrit.plugins.multisite.forwarder;
-
-import com.google.gerrit.server.events.Event;
-
-public interface StreamEventForwarder {
- /**
- * Forward a stream event to the other master.
- *
- * @param event the event to forward.
- * @return true if successful, otherwise false.
- */
- boolean send(Event event);
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerForwarderModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerForwarderModule.java
index 6bd6437..d6c5658 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerForwarderModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerForwarderModule.java
@@ -19,7 +19,6 @@
import com.googlesource.gerrit.plugins.multisite.forwarder.CacheEvictionForwarder;
import com.googlesource.gerrit.plugins.multisite.forwarder.IndexEventForwarder;
import com.googlesource.gerrit.plugins.multisite.forwarder.ProjectListUpdateForwarder;
-import com.googlesource.gerrit.plugins.multisite.forwarder.StreamEventForwarder;
public class BrokerForwarderModule extends LifecycleModule {
@Override
@@ -28,6 +27,5 @@
DynamicSet.bind(binder(), CacheEvictionForwarder.class).to(BrokerCacheEvictionForwarder.class);
DynamicSet.bind(binder(), ProjectListUpdateForwarder.class)
.to(BrokerProjectListUpdateForwarder.class);
- DynamicSet.bind(binder(), StreamEventForwarder.class).to(BrokerStreamEventForwarder.class);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerStreamEventForwarder.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerStreamEventForwarder.java
deleted file mode 100644
index 8a020fc..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/broker/BrokerStreamEventForwarder.java
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (C) 2019 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.googlesource.gerrit.plugins.multisite.forwarder.broker;
-
-import com.google.gerrit.server.events.Event;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import com.googlesource.gerrit.plugins.multisite.Configuration;
-import com.googlesource.gerrit.plugins.multisite.broker.BrokerApiWrapper;
-import com.googlesource.gerrit.plugins.multisite.forwarder.StreamEventForwarder;
-import com.googlesource.gerrit.plugins.multisite.forwarder.events.EventTopic;
-
-@Singleton
-public class BrokerStreamEventForwarder implements StreamEventForwarder {
- private final BrokerApiWrapper broker;
- private final Configuration cfg;
-
- @Inject
- BrokerStreamEventForwarder(BrokerApiWrapper broker, Configuration cfg) {
- this.broker = broker;
- this.cfg = cfg;
- }
-
- @Override
- public boolean send(Event event) {
- return broker.sendSync(EventTopic.STREAM_EVENT_TOPIC.topic(cfg), event);
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/EventTopic.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/EventTopic.java
index d8e7607..3255bcf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/EventTopic.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/EventTopic.java
@@ -21,39 +21,7 @@
BATCH_INDEX_TOPIC("GERRIT.EVENT.BATCH.INDEX", "batchIndexEvent"),
CACHE_TOPIC("GERRIT.EVENT.CACHE", "cacheEvent"),
PROJECT_LIST_TOPIC("GERRIT.EVENT.PROJECT.LIST", "projectListEvent"),
- /**
- * the STREAM_EVENT_TOPICS and the GERRIT_TOPIC are both used to publish stream events. It might
- * not be immediately intuitive why there are _two_ streams for this, rather than one. These are
- * some gotchas regarding this:
- *
- * <ul>
- * <li>A subset of stream events is published to the STREAM_EVENT_TOPIC (i.e. only global
- * project events). This is done by the EventHandler.
- * <li>All, unfiltered gerrit events are published to a topic called `gerrit` (the
- * GERRIT_TOPIC). This is done by the StreamEventPublisher provided by the events-broker
- * library.
- * </ul>
- *
- * Historically, the GERRIT_TOPIC existed to allow the streaming of all stream events. With the
- * introduction of multi-site however, events had to be wrapped into EventMessage and streamed
- * separately, in order to provide a header containing the instance-id that produced that message.
- * For this purpose a *new* STREAM_EVENT_TOPIC topic was introduced.
- *
- * <p><a href="https://bugs.chromium.org/p/gerrit/issues/detail?id=14823">Issue 14823</a> now
- * removed the need of EventMessage altogether,so that the `STREAM_EVENT_TOPIC` stream,
- * conceptually would not be needed anymore.
- *
- * <p>However, before the `STREAM_EVENT_TOPIC` stream can be effectively removed, the filtering of
- * the events needs to happen on the receiving side, as explained in <a
- * href="https://bugs.chromium.org/p/gerrit/issues/detail?id=14835">Issue 14835</a>.
- *
- * <p>This would allow to *all* events to be streamed to the STREAM_EVENT_TOPICS.
- *
- * @deprecated Use {@link #GERRIT_TOPIC} instead.
- */
- @Deprecated
- STREAM_EVENT_TOPIC("GERRIT.EVENT.STREAM", "streamEvent"),
- GERRIT_TOPIC("gerrit", "gerritEvents");
+ STREAM_EVENT_TOPIC("gerrit", "streamEvent");
private final String topic;
private final String aliasKey;
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index d7d2239..7eedc6a 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -58,13 +58,7 @@
: Name of the topic to use for publishing indexing events
Defaults to GERRIT.EVENT.INDEX
-```broker.streamEventTopic```
-: Name of the topic to use for publishing stream events. Note that only
- project events are streamed and only for global projects (see
- `projects.pattern`).
- Defaults to GERRIT.EVENT.STREAM
-
-`broker.gerritEventsTopic`
+`broker.streamEventTopic`
: Name of the topic to use for publishing all stream events.
Default: gerrit
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutorProviderTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutorProviderTest.java
deleted file mode 100644
index a025dd0..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/EventExecutorProviderTest.java
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright (C) 2016 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.googlesource.gerrit.plugins.multisite.event;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import com.google.gerrit.server.git.WorkQueue;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class EventExecutorProviderTest {
- @Mock private ScheduledThreadPoolExecutor executorMock;
- private EventExecutorProvider eventsExecutorProvider;
-
- @Before
- public void setUp() throws Exception {
- WorkQueue workQueueMock = mock(WorkQueue.class);
- when(workQueueMock.createQueue(1, "Forward-Stream-Event")).thenReturn(executorMock);
- eventsExecutorProvider = new EventExecutorProvider(workQueueMock);
- }
-
- @Test
- public void shouldReturnExecutor() throws Exception {
- assertThat(eventsExecutorProvider.get()).isEqualTo(executorMock);
- }
-
- @Test
- public void testStop() throws Exception {
- eventsExecutorProvider.start();
- assertThat(eventsExecutorProvider.get()).isEqualTo(executorMock);
- eventsExecutorProvider.stop();
- verify(executorMock).shutdown();
- assertThat(eventsExecutorProvider.get()).isNull();
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/EventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/event/EventHandlerTest.java
deleted file mode 100644
index 1d75f9d..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/EventHandlerTest.java
+++ /dev/null
@@ -1,85 +0,0 @@
-// Copyright (C) 2015 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.googlesource.gerrit.plugins.multisite.event;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyZeroInteractions;
-
-import com.google.common.util.concurrent.MoreExecutors;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.server.events.Event;
-import com.google.gerrit.server.events.ProjectEvent;
-import com.google.gerrit.server.events.RefUpdatedEvent;
-import com.googlesource.gerrit.plugins.multisite.event.EventHandler.EventTask;
-import com.googlesource.gerrit.plugins.multisite.forwarder.StreamEventForwarder;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class EventHandlerTest {
- private static final String DEFAULT_INSTANCE_ID = "instance-id";
-
- private EventHandler eventHandler;
-
- @Mock private StreamEventForwarder forwarder;
-
- @Before
- public void setUp() {
- eventHandler =
- new EventHandler(
- asDynamicSet(forwarder), MoreExecutors.directExecutor(), DEFAULT_INSTANCE_ID);
- }
-
- private DynamicSet<StreamEventForwarder> asDynamicSet(StreamEventForwarder forwarder) {
- DynamicSet<StreamEventForwarder> result = new DynamicSet<>();
- result.add("multi-site", forwarder);
- return result;
- }
-
- @Test
- public void shouldForwardAnyProjectEvent() throws Exception {
- ProjectEvent event = mock(ProjectEvent.class);
- event.instanceId = DEFAULT_INSTANCE_ID;
- eventHandler.onEvent(event);
- verify(forwarder).send(event);
- }
-
- @Test
- public void shouldNotForwardNonProjectEvent() throws Exception {
- eventHandler.onEvent(mock(Event.class));
- verifyZeroInteractions(forwarder);
- }
-
- @Test
- public void shouldNotForwardIfAlreadyForwardedEvent() throws Exception {
- Event event = mock(ProjectEvent.class);
- event.instanceId = "instance-id-2";
- eventHandler.onEvent(event);
- verifyZeroInteractions(forwarder);
- }
-
- @Test
- public void tesEventTaskToString() throws Exception {
- Event event = new RefUpdatedEvent();
- EventTask task = eventHandler.new EventTask(event);
- assertThat(task.toString())
- .isEqualTo(String.format("Send event '%s' to target instance", event.type));
- }
-}
