This is an automated email from the ASF dual-hosted git repository. timothyjward pushed a commit to branch fix/bugs in repository https://gitbox.apache.org/repos/asf/aries-typedevent.git
commit 834d0d9fa9fbc8a991d2af063f6be7b24a281491 Author: Tim Ward <[email protected]> AuthorDate: Fri Sep 15 16:05:46 2023 +0100 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 <[email protected]> --- .../typedevent/bus/impl/TypedEventBusImpl.java | 18 ++-- .../bus/osgi/EventDeliveryIntegrationTest.java | 101 ++++++++++++++++++++- 2 files changed, 112 insertions(+), 7 deletions(-) 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 @@ public class TypedEventBusImpl implements TypedEventBus { Long serviceId = getServiceId(properties); - doRemoveEventHandler(topicsToTypedHandlers, knownTypedHandlers, handler, serviceId); + doRemoveEventHandler(topicsToTypedHandlers, wildcardTopicsToTypedHandlers, knownTypedHandlers, handler, serviceId); synchronized (lock) { typedHandlersToTargetClasses.remove(handler); @@ -282,7 +282,7 @@ public class TypedEventBusImpl implements TypedEventBus { 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 @@ public class TypedEventBusImpl implements TypedEventBus { } } - 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 @@ public class TypedEventBusImpl implements TypedEventBus { 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 @@ package org.apache.aries.typedevent.bus.osgi; 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 @@ public class EventDeliveryIntegrationTest extends AbstractIntegrationTest { 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
