Fix bug in whiteboard service update/remove
The removal code did not correctly clear instances from the type specific id maps, or from the wildcard maps. This commit adds a test for the failing case and also fixes the map management logic.
Signed-off-by: Tim Ward <timothyjward@apache.org>
diff --git a/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/TypedEventBusImpl.java b/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/TypedEventBusImpl.java
index e055443..9c4057e 100644
--- a/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/TypedEventBusImpl.java
+++ b/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/TypedEventBusImpl.java
@@ -271,7 +271,7 @@
Long serviceId = getServiceId(properties);
- doRemoveEventHandler(topicsToTypedHandlers, knownTypedHandlers, handler, serviceId);
+ doRemoveEventHandler(topicsToTypedHandlers, wildcardTopicsToTypedHandlers, knownTypedHandlers, handler, serviceId);
synchronized (lock) {
typedHandlersToTargetClasses.remove(handler);
@@ -282,7 +282,7 @@
Long serviceId = getServiceId(properties);
- doRemoveEventHandler(topicsToUntypedHandlers, knownUntypedHandlers, handler, serviceId);
+ doRemoveEventHandler(topicsToUntypedHandlers, wildcardTopicsToUntypedHandlers, knownUntypedHandlers, handler, serviceId);
}
private Long getServiceId(Map<String, Object> properties) {
@@ -307,14 +307,20 @@
}
}
- private <T, U> void doRemoveEventHandler(Map<String, Map<T, U>> map, Map<Long, T> idMap,
+ private <T, U> void doRemoveEventHandler(Map<String, Map<T, U>> map, Map<String, Map<T, U>> wildcardMap, Map<Long, T> idMap,
T handler, Long serviceId) {
synchronized (lock) {
List<String> consumed = knownHandlers.remove(serviceId);
- knownHandlers.remove(serviceId);
+ idMap.remove(serviceId);
if (consumed != null) {
consumed.forEach(s -> {
- Map<T, ?> handlers = map.get(s);
+ Map<T, ?> handlers;
+ if(isWildcard(s)) {
+ handlers = wildcardMap.get(s.length() == 1 ? "" : s.substring(0, s.length() - 2));
+ } else {
+ handlers = map.get(s);
+ }
+
if (handlers != null) {
handlers.remove(handler);
if (handlers.isEmpty()) {
@@ -350,7 +356,7 @@
synchronized (lock) {
T handler = idToHandler.get(serviceId);
- doRemoveEventHandler(map, idToHandler, handler, serviceId);
+ doRemoveEventHandler(map, wildcardMap, idToHandler, handler, serviceId);
doAddEventHandler(map, wildcardMap, idToHandler, handler, defaultTopic, properties);
}
}
diff --git a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/EventDeliveryIntegrationTest.java b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/EventDeliveryIntegrationTest.java
index 40543fa..96d0b65 100644
--- a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/EventDeliveryIntegrationTest.java
+++ b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/EventDeliveryIntegrationTest.java
@@ -18,6 +18,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.eq;
+import static org.osgi.service.typedevent.TypedEventConstants.TYPED_EVENT_TOPICS;
import java.util.Dictionary;
import java.util.HashMap;
@@ -224,6 +225,104 @@
assertEquals("foo", received.subEvent.message);
}
-
+ /**
+ * Tests that events are delivered to untyped Event Handlers
+ * based on topic
+ *
+ * @throws InterruptedException
+ */
+ @Test
+ public void testEventReceivingUpdateTopic() throws InterruptedException {
+
+ TestEvent event = new TestEvent();
+ event.message = "boo";
+
+ Dictionary<String, Object> props = new Hashtable<>();
+ props.put(TYPED_EVENT_TOPICS, TEST_EVENT_TOPIC);
+
+ regs.add(context.registerService(TypedEventHandler.class, typedEventHandler, props));
+
+ regs.add(context.registerService(UntypedEventHandler.class, untypedEventHandler, props));
+
+ eventBus.deliver(event);
+
+ Mockito.verify(typedEventHandler, Mockito.timeout(1000)).notify(
+ Mockito.eq(TEST_EVENT_TOPIC), Mockito.argThat(isTestEventWithMessage("boo")));
+
+ Mockito.verify(untypedEventHandler, Mockito.timeout(1000)).notifyUntyped(
+ Mockito.eq(TEST_EVENT_TOPIC), Mockito.argThat(isUntypedTestEventWithMessage("boo")));
+
+ Mockito.clearInvocations(typedEventHandler, untypedEventHandler);
+
+ props.put(TYPED_EVENT_TOPICS, TEST_EVENT_2_TOPIC);
+
+ regs.forEach(s -> s.setProperties(props));
+
+ eventBus.deliver(event);
+
+ Mockito.verify(typedEventHandler, Mockito.after(1000).never()).notify(
+ Mockito.eq(TEST_EVENT_TOPIC), Mockito.any());
+ Mockito.verify(untypedEventHandler, Mockito.after(1000).never()).notifyUntyped(
+ Mockito.eq(TEST_EVENT_TOPIC), Mockito.any());
+
+ eventBus.deliver(TEST_EVENT_2_TOPIC, event);
+
+ Mockito.verify(typedEventHandler, Mockito.timeout(1000)).notify(
+ Mockito.eq(TEST_EVENT_2_TOPIC), Mockito.argThat(isTestEventWithMessage("boo")));
+
+ Mockito.verify(untypedEventHandler, Mockito.timeout(1000)).notifyUntyped(
+ Mockito.eq(TEST_EVENT_2_TOPIC), Mockito.argThat(isUntypedTestEventWithMessage("boo")));
+
+ }
+
+ /**
+ * Tests that events are delivered to untyped Event Handlers
+ * based on topic
+ *
+ * @throws InterruptedException
+ */
+ @Test
+ public void testEventReceivingUpdateWildcardTopic() throws InterruptedException {
+
+ TestEvent event = new TestEvent();
+ event.message = "boo";
+
+ Dictionary<String, Object> props = new Hashtable<>();
+ props.put(TYPED_EVENT_TOPICS, "foo/bar/*");
+
+ regs.add(context.registerService(TypedEventHandler.class, typedEventHandler, props));
+
+ regs.add(context.registerService(UntypedEventHandler.class, untypedEventHandler, props));
+
+ eventBus.deliver("foo/bar/foobar", event);
+
+ Mockito.verify(typedEventHandler, Mockito.timeout(1000)).notify(
+ Mockito.eq("foo/bar/foobar"), Mockito.argThat(isTestEventWithMessage("boo")));
+
+ Mockito.verify(untypedEventHandler, Mockito.timeout(1000)).notifyUntyped(
+ Mockito.eq("foo/bar/foobar"), Mockito.argThat(isUntypedTestEventWithMessage("boo")));
+
+ Mockito.clearInvocations(typedEventHandler, untypedEventHandler);
+
+ props.put(TYPED_EVENT_TOPICS, "foo/bar/foobar/*");
+
+ regs.forEach(s -> s.setProperties(props));
+
+ eventBus.deliver("foo/bar/foobar", event);
+
+ Mockito.verify(typedEventHandler, Mockito.after(1000).never()).notify(
+ Mockito.eq("foo/bar/foobar"), Mockito.any());
+ Mockito.verify(untypedEventHandler, Mockito.after(1000).never()).notifyUntyped(
+ Mockito.eq("foo/bar/foobar"), Mockito.any());
+
+ eventBus.deliver("foo/bar/foobar/fizzbuzz", event);
+
+ Mockito.verify(typedEventHandler, Mockito.timeout(1000)).notify(
+ Mockito.eq("foo/bar/foobar/fizzbuzz"), Mockito.argThat(isTestEventWithMessage("boo")));
+
+ Mockito.verify(untypedEventHandler, Mockito.timeout(1000)).notifyUntyped(
+ Mockito.eq("foo/bar/foobar/fizzbuzz"), Mockito.argThat(isUntypedTestEventWithMessage("boo")));
+
+ }
}
\ No newline at end of file
