This is an automated email from the ASF dual-hosted git repository. timothyjward pushed a commit to branch fix/record_null in repository https://gitbox.apache.org/repos/asf/aries-typedevent.git
commit 40f5ebbc7c62e51f9ae2456062957c90e39fc0c4 Author: Tim Ward <[email protected]> AuthorDate: Mon Apr 7 16:40:56 2025 +0100 Correctly handle mapping records with null components to Maps Simply streaming the record components into a Map would throw an NPE when the record component's value was null. This is something that is permitted in a record and so we must use a null-safe way of populating the Map. Signed-off-by: Tim Ward <[email protected]> --- .../aries/typedevent/bus/impl/RecordConverter.java | 7 +- .../bus/osgi/AbstractIntegrationTest.java | 7 +- .../typedevent/bus/osgi/RecordIntegrationTest.java | 77 ++++++++++++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/org.apache.aries.typedevent.bus/src/main/java16/org/apache/aries/typedevent/bus/impl/RecordConverter.java b/org.apache.aries.typedevent.bus/src/main/java16/org/apache/aries/typedevent/bus/impl/RecordConverter.java index 67fe266..af779d3 100644 --- a/org.apache.aries.typedevent.bus/src/main/java16/org/apache/aries/typedevent/bus/impl/RecordConverter.java +++ b/org.apache.aries.typedevent.bus/src/main/java16/org/apache/aries/typedevent/bus/impl/RecordConverter.java @@ -22,6 +22,7 @@ import static java.util.stream.Collectors.toMap; import java.lang.reflect.RecordComponent; import java.lang.reflect.Type; import java.util.Arrays; +import java.util.HashMap; import java.util.Map; import org.osgi.util.converter.ConversionException; @@ -60,8 +61,10 @@ public class RecordConverter { } return createRecord(clz, args, argTypes); } else { - Map<String, Object> converted = Arrays.stream(sourceComponents) - .collect(toMap(RecordComponent::getName, rc -> getComponentValue(rc, o))); + Map<String, Object> converted = new HashMap<>(sourceComponents.length); + for(RecordComponent rc : sourceComponents) { + converted.put(rc.getName(), getComponentValue(rc, o)); + } return converter.convert(converted).to(target); } diff --git a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/AbstractIntegrationTest.java b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/AbstractIntegrationTest.java index 81cd6d2..f624d46 100644 --- a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/AbstractIntegrationTest.java +++ b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/AbstractIntegrationTest.java @@ -19,6 +19,7 @@ package org.apache.aries.typedevent.bus.osgi; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import org.apache.aries.typedevent.bus.common.TestEvent; import org.apache.aries.typedevent.bus.common.TestEvent2; @@ -55,7 +56,7 @@ public abstract class AbstractIntegrationTest { @Override public boolean matches(TestEvent argument) { - return message.equals(argument.message); + return argument != null && Objects.equals(message, argument.message); } }; } @@ -65,7 +66,7 @@ public abstract class AbstractIntegrationTest { @Override public boolean matches(TestEvent2 argument) { - return message.equals(argument.subEvent.message); + return argument != null && Objects.equals(message, argument.subEvent.message); } }; } @@ -75,7 +76,7 @@ public abstract class AbstractIntegrationTest { @Override public boolean matches(Map<String, Object> argument) { - return argument != null && message.equals(argument.get("message")); + return argument != null && Objects.equals(message, argument.get("message")); } }; } diff --git a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/RecordIntegrationTest.java b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/RecordIntegrationTest.java index 8b5d69d..32d493c 100644 --- a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/RecordIntegrationTest.java +++ b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/osgi/RecordIntegrationTest.java @@ -33,6 +33,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.osgi.framework.BundleContext; import org.osgi.service.typedevent.TypedEventBus; import org.osgi.service.typedevent.TypedEventHandler; +import org.osgi.service.typedevent.UntypedEventHandler; import org.osgi.test.common.annotation.InjectBundleContext; import org.osgi.test.common.annotation.InjectService; import org.osgi.test.junit5.context.BundleContextExtension; @@ -64,6 +65,9 @@ public class RecordIntegrationTest extends AbstractIntegrationTest { @Mock TestRecordListener recordEventHandler; + @Mock + UntypedEventHandler untypedEventHandler; + @Test public void testUnFilteredListenerEventToRecord() throws Exception { Dictionary<String, Object> props = new Hashtable<>(); @@ -140,6 +144,42 @@ public class RecordIntegrationTest extends AbstractIntegrationTest { } + @Test + public void testUnFilteredListenerRecordToMap() throws Exception { + Dictionary<String, Object> props = new Hashtable<>(); + props.put(TYPED_EVENT_TOPICS, TOPIC); + + regs.add(context.registerService(TypedEventHandler.class, typedEventHandler, props)); + + props = new Hashtable<>(); + props.put(TYPED_EVENT_TOPICS, TOPIC); + + regs.add(context.registerService(UntypedEventHandler.class, untypedEventHandler, props)); + + // Record to Map + TestRecord2 testRecord2 = new TestRecord2("foobar", 5); + + eventBus.deliver(TOPIC, testRecord2); + + Mockito.verify(typedEventHandler, Mockito.timeout(1000)) + .notify(Mockito.eq(TOPIC), Mockito.argThat(isTestEventWithMessage("foobar"))); + + Mockito.verify(untypedEventHandler, Mockito.timeout(1000)) + .notifyUntyped(Mockito.eq(TOPIC), Mockito.argThat(isUntypedTestEventWithMessage("foobar"))); + + // Record to Map with null + testRecord2 = new TestRecord2(null, 5); + + eventBus.deliver(TOPIC, testRecord2); + + Mockito.verify(typedEventHandler, Mockito.timeout(1000)) + .notify(Mockito.eq(TOPIC), Mockito.argThat(isTestEventWithMessage(null))); + + Mockito.verify(untypedEventHandler, Mockito.timeout(1000)) + .notifyUntyped(Mockito.eq(TOPIC), Mockito.argThat(isUntypedTestEventWithMessage(null))); + + } + @Test public void testFilteredListenerEventToRecord() throws Exception { Dictionary<String, Object> props = new Hashtable<>(); @@ -253,6 +293,43 @@ public class RecordIntegrationTest extends AbstractIntegrationTest { Mockito.verify(typedEventHandler, Mockito.after(1000).never()) .notify(Mockito.eq(TOPIC), Mockito.argThat(isTestEventWithMessage("bar"))); } + + @Test + public void testFilteredListenerRecordToMap() throws Exception { + Dictionary<String, Object> props = new Hashtable<>(); + props.put(TYPED_EVENT_FILTER, "(message=foo)"); + props.put(TYPED_EVENT_TOPICS, TOPIC); + + regs.add(context.registerService(TypedEventHandler.class, typedEventHandler, props)); + + props = new Hashtable<>(); + props.put(TYPED_EVENT_FILTER, "(message=bar)"); + props.put(TYPED_EVENT_TOPICS, TOPIC); + + regs.add(context.registerService(UntypedEventHandler.class, untypedEventHandler, props)); + + // Record to Record + TestRecord2 testRecord2 = new TestRecord2("foo", 5); + + eventBus.deliver(TOPIC, testRecord2); + + Mockito.verify(typedEventHandler, Mockito.timeout(1000)) + .notify(Mockito.eq(TOPIC), Mockito.argThat(isTestEventWithMessage("foo"))); + + Mockito.verify(untypedEventHandler, Mockito.after(1000).never()) + .notifyUntyped(Mockito.eq(TOPIC), Mockito.argThat(isUntypedTestEventWithMessage("foo"))); + + + testRecord2 = new TestRecord2("bar", 5); + + eventBus.deliver(TOPIC, testRecord2); + + Mockito.verify(untypedEventHandler, Mockito.timeout(1000)) + .notifyUntyped(Mockito.eq(TOPIC), Mockito.argThat(isUntypedTestEventWithMessage("bar"))); + + Mockito.verify(typedEventHandler, Mockito.after(1000).never()) + .notify(Mockito.eq(TOPIC), Mockito.argThat(isTestEventWithMessage("bar"))); + } public interface TestRecordListener extends TypedEventHandler<TestRecord> {}
