From 85fec4f256405febe2389795e9202559114ed9b3 Mon Sep 17 00:00:00 2001 From: Enrico Molino <57355804+symphony-enrico@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:55:38 +0200 Subject: [PATCH 1/3] Bugfix unregister activity (#840) * Bugfix unregister activity The ActivityRegistry is responsible for binding activities to a data feed. When an activity is registered, the system ensures that duplicates are not allowed. If an activity with the same identity is already registered, the old one is unregistered before the new one is added. A mechanism to explicitly unregister an activity was added in a previous PR, reusing the existing logic for handling duplicate registrations. However, both the original code and the previous PR did not always behave as expected. While this worked for Command activities, it failed for FormReplyActivity and UserJoinedRoomActivity. These activities create their listeners using lambdas, and each call produces a new lambda instance. In Java, even if two lambdas are created from the same code and the same activity instance, they are treated as different objects. Consequently, registering the same activity twice would generate a new listener object, preventing the system from detecting duplicates. The workaround is to modify listener generation: the activity that creates the listener is now encapsulated inside the listener itself, and equals() and hashCode() are implemented so that two listeners are considered equal if they originate from the same activity. This ensures that duplicate registrations are properly detected and handled. * Coverage improved --- .../core/activity/form/FormReplyActivity.java | 2 +- .../activity/room/UserJoinedRoomActivity.java | 2 +- .../datafeed/util/RealTimeEventsBinder.java | 127 ++++++++++++++---- .../util/RealTimeEventsBinderTest.java | 126 ++++++++++++++++- 4 files changed, 225 insertions(+), 32 deletions(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/form/FormReplyActivity.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/form/FormReplyActivity.java index f8cd0f26f..772366bb8 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/form/FormReplyActivity.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/form/FormReplyActivity.java @@ -26,7 +26,7 @@ public abstract class FormReplyActivity /** {@inheritDoc} */ @Override protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) { - bindOnSymphonyElementsAction(realTimeEventsSource, this::processEvent); + bindOnSymphonyElementsAction(realTimeEventsSource, this::processEvent, this); } /** {@inheritDoc} */ diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/room/UserJoinedRoomActivity.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/room/UserJoinedRoomActivity.java index 3c95a7f83..bcfed2696 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/room/UserJoinedRoomActivity.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/room/UserJoinedRoomActivity.java @@ -22,7 +22,7 @@ public abstract class UserJoinedRoomActivity */ @Override protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) { - bindOnUserJoinedRoom(realTimeEventsSource, this::processEvent); + bindOnUserJoinedRoom(realTimeEventsSource, this::processEvent, this); } /** diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinder.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinder.java index 859b8b441..b0560c93e 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinder.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinder.java @@ -1,17 +1,20 @@ package com.symphony.bdk.core.service.datafeed.util; +import com.symphony.bdk.core.activity.AbstractActivity; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; import com.symphony.bdk.gen.api.model.V4Initiator; import com.symphony.bdk.gen.api.model.V4MessageSent; import com.symphony.bdk.gen.api.model.V4SymphonyElementsAction; - import com.symphony.bdk.gen.api.model.V4UserJoinedRoom; import org.apiguardian.api.API; +import java.util.Objects; import java.util.function.BiConsumer; import java.util.function.Consumer; +import javax.annotation.Nullable; + /** * Utility class used to attach a method call (defined by a {@link BiConsumer}) to a specific real-time event. */ @@ -26,47 +29,40 @@ public RealTimeEventsBinder() { * Bind "onMessageSent" real-time event to a target method. * * @param subscriber The Datafeed real-time events subscriber. - * @param target Target method. + * @param target Target method. + * @param activity The activity that creates the listener. It can be null, but in that case it will not be possible + * to unsubscribe the activity later. */ - public static void bindOnMessageSent(Consumer subscriber, BiConsumer target) { - subscriber.accept(new RealTimeEventListener() { - - @Override - public void onMessageSent(V4Initiator initiator, V4MessageSent event) { - target.accept(initiator, event); - } - }); + public static void bindOnMessageSent(Consumer subscriber, + BiConsumer target, @Nullable AbstractActivity activity) { + subscriber.accept(new OnMessageSent(activity, target)); } /** * Bind "onSymphonyElementsAction" real-time event to a target method. * * @param subscriber The Datafeed real-time events subscriber. - * @param target Target method. + * @param target Target method. + * @param activity The activity that creates the listener. It can be null, but in that case it will not be possible + * to unsubscribe the activity later. */ - public static void bindOnSymphonyElementsAction(Consumer subscriber, BiConsumer target) { - subscriber.accept(new RealTimeEventListener() { - - @Override - public void onSymphonyElementsAction(V4Initiator initiator, V4SymphonyElementsAction event) { - target.accept(initiator, event); - } - }); + public static void bindOnSymphonyElementsAction(Consumer subscriber, + BiConsumer target, + @Nullable AbstractActivity activity) { + subscriber.accept(new OnSymphonyElementsAction(activity, target)); } /** * Bind "onUserJoinedRoom" real-time event to a target method. * * @param subscriber The Datafeed real-time events subscriber. - * @param target Target method. + * @param target Target method. + * @param activity The activity that creates the listener. It can be null, but in that case it will not be possible + * to unsubscribe the activity later. */ - public static void bindOnUserJoinedRoom(Consumer subscriber, BiConsumer target) { - subscriber.accept(new RealTimeEventListener() { - @Override - public void onUserJoinedRoom(V4Initiator initiator, V4UserJoinedRoom event) { - target.accept(initiator, event); - } - }); + public static void bindOnUserJoinedRoom(Consumer subscriber, + BiConsumer target, @Nullable AbstractActivity activity) { + subscriber.accept(new OnUserJoinedRoom(activity, target)); } /** @@ -78,4 +74,81 @@ public void onUserJoinedRoom(V4Initiator initiator, V4UserJoinedRoom event) { public static void bindRealTimeListener(Consumer consumer, RealTimeEventListener listener) { consumer.accept(listener); } + + /** + * Internal private records used to create listeners from an {@link AbstractActivity}. + * They keep a reference to the source {@link AbstractActivity}, which is used only for equality validation. + * This ensures that multiple identical listeners from the same {@link AbstractActivity} are not registered, + * and also allows the {@link AbstractActivity} to be unsubscribed later. + */ + + private record OnMessageSent(AbstractActivity activity, + BiConsumer target) + implements RealTimeEventListener { + + @Override + public void onMessageSent(V4Initiator initiator, V4MessageSent event) { + target.accept(initiator, event); + } + + @Override + public boolean equals(Object o) { + // If the listener is created with a null activity, there is no way to identify it later. + // For this reason, equals will always return false. + if (!(o instanceof OnMessageSent that) || this.activity == null) {return false;} + return activity.equals(that.activity); + } + + @Override + public int hashCode() { + return Objects.hashCode(activity); + } + } + + + private record OnSymphonyElementsAction(AbstractActivity activity, + BiConsumer target) + implements RealTimeEventListener { + + @Override + public void onSymphonyElementsAction(V4Initiator initiator, V4SymphonyElementsAction event) { + target.accept(initiator, event); + } + + @Override + public boolean equals(Object o) { + // If the listener is created with a null activity, there is no way to identify it later. + // For this reason, equals will always return false. + if (!(o instanceof OnSymphonyElementsAction that) || this.activity == null) {return false;} + return activity.equals(that.activity); + } + + @Override + public int hashCode() { + return Objects.hashCode(activity); + } + } + + + private record OnUserJoinedRoom(AbstractActivity activity, + BiConsumer target) implements RealTimeEventListener { + + @Override + public void onUserJoinedRoom(V4Initiator initiator, V4UserJoinedRoom event) { + target.accept(initiator, event); + } + + @Override + public boolean equals(Object o) { + // If the listener is created with a null activity, there is no way to identify it later. + // For this reason, equals will always return false. + if (!(o instanceof OnUserJoinedRoom that) || this.activity == null) {return false;} + return activity.equals(that.activity); + } + + @Override + public int hashCode() { + return Objects.hashCode(activity); + } + } } diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinderTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinderTest.java index 988b35a5e..68c1ba38f 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinderTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinderTest.java @@ -2,6 +2,11 @@ import static org.junit.jupiter.api.Assertions.*; +import com.symphony.bdk.core.activity.AbstractActivity; +import com.symphony.bdk.core.activity.ActivityContext; +import com.symphony.bdk.core.activity.ActivityMatcher; +import com.symphony.bdk.core.activity.model.ActivityInfo; +import com.symphony.bdk.core.service.datafeed.EventException; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; import com.symphony.bdk.gen.api.model.V4Initiator; import com.symphony.bdk.gen.api.model.V4MessageSent; @@ -38,29 +43,144 @@ void testConstructorJustToMakeJacocoHappy() { void testBindOnMessageSent() { final AtomicBoolean methodCalled = new AtomicBoolean(false); final BiConsumer methodToBind = (initiator, v4MessageSent) -> methodCalled.set(true); - RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind); + RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind, null); this.realTimeEventsProvider.trigger(l -> l.onMessageSent(new V4Initiator(), new V4MessageSent())); assertTrue(methodCalled.get()); } + @Test + void testBindOnMessageSentEqualsOnActivity() { + final BiConsumer methodToBind1 = (initiator, v4MessageSent) -> {}; + final BiConsumer methodToBind2 = (initiator, v4MessageSent) -> {}; + AbstractActivity activity = new AbstractActivity<>() { + + @Override + protected ActivityMatcher matcher() throws EventException { + return null; + } + + @Override + protected ActivityInfo info() { + return null; + } + + @Override + protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) { + + } + + @Override + protected void onActivity(ActivityContext context) throws EventException { + + } + }; + + + RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind1, activity); + RealTimeEventListener listener1 = this.realTimeEventsProvider.listener; + + RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind2, activity); + RealTimeEventListener listener2 = this.realTimeEventsProvider.listener; + + assertTrue(listener1 != listener2); + assertEquals(listener1, listener2); + assertEquals(listener1.hashCode(), listener2.hashCode()); + } + @Test void testBindOnSymphonyElementsAction() { final AtomicBoolean methodCalled = new AtomicBoolean(false); final BiConsumer methodToBind = (initiator, v4SymphonyElementsAction) -> methodCalled.set(true); - RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind); + RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind, null); this.realTimeEventsProvider.trigger(l -> l.onSymphonyElementsAction(new V4Initiator(), new V4SymphonyElementsAction())); assertTrue(methodCalled.get()); } + @Test + void testBindOnSymphonyElementsActionEqualsOnActivity() { + final BiConsumer methodToBind1 = (initiator, v4SymphonyElementsAction) -> {}; + final BiConsumer methodToBind2 = (initiator, v4SymphonyElementsAction) -> {}; + AbstractActivity activity = new AbstractActivity<>() { + + @Override + protected ActivityMatcher matcher() throws EventException { + return null; + } + + @Override + protected ActivityInfo info() { + return null; + } + + @Override + protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) { + + } + + @Override + protected void onActivity(ActivityContext context) throws EventException { + + } + }; + + RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind1, activity); + RealTimeEventListener listener1 = this.realTimeEventsProvider.listener; + + RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind2, activity); + RealTimeEventListener listener2 = this.realTimeEventsProvider.listener; + + assertTrue(listener1 != listener2); + assertEquals(listener1, listener2); + assertEquals(listener1.hashCode(), listener2.hashCode()); + } + @Test void testBindOnUserJoinedRoom() { final AtomicBoolean methodCalled = new AtomicBoolean(false); final BiConsumer methodToBind = ((initiator, v4UserJoinedRoom) -> methodCalled.set(true)); - RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind); + RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind, null); this.realTimeEventsProvider.trigger(l -> l.onUserJoinedRoom(new V4Initiator(), new V4UserJoinedRoom())); assertTrue(methodCalled.get()); } + @Test + void testBindOnUserJoinedRoomEqualsOnActivity() { + final BiConsumer methodToBind1 = (initiator, v4UserJoinedRoom) -> {}; + final BiConsumer methodToBind2 = (initiator, v4UserJoinedRoom) -> {}; + AbstractActivity activity = new AbstractActivity<>() { + + @Override + protected ActivityMatcher matcher() throws EventException { + return null; + } + + @Override + protected ActivityInfo info() { + return null; + } + + @Override + protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) { + + } + + @Override + protected void onActivity(ActivityContext context) throws EventException { + + } + }; + + RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind1, activity); + RealTimeEventListener listener1 = this.realTimeEventsProvider.listener; + + RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind2, activity); + RealTimeEventListener listener2 = this.realTimeEventsProvider.listener; + + assertTrue(listener1 != listener2); + assertEquals(listener1, listener2); + assertEquals(listener1.hashCode(), listener2.hashCode()); + } + private static class RealTimeEventsProvider { private RealTimeEventListener listener; From 46be60042ceb808c2791d422f62e69038cd4469a Mon Sep 17 00:00:00 2001 From: Enrico Molino <57355804+symphony-enrico@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:55:38 +0200 Subject: [PATCH 2/3] Bugfix unregister activity (#840) * Bugfix unregister activity The ActivityRegistry is responsible for binding activities to a data feed. When an activity is registered, the system ensures that duplicates are not allowed. If an activity with the same identity is already registered, the old one is unregistered before the new one is added. A mechanism to explicitly unregister an activity was added in a previous PR, reusing the existing logic for handling duplicate registrations. However, both the original code and the previous PR did not always behave as expected. While this worked for Command activities, it failed for FormReplyActivity and UserJoinedRoomActivity. These activities create their listeners using lambdas, and each call produces a new lambda instance. In Java, even if two lambdas are created from the same code and the same activity instance, they are treated as different objects. Consequently, registering the same activity twice would generate a new listener object, preventing the system from detecting duplicates. The workaround is to modify listener generation: the activity that creates the listener is now encapsulated inside the listener itself, and equals() and hashCode() are implemented so that two listeners are considered equal if they originate from the same activity. This ensures that duplicate registrations are properly detected and handled. * Coverage improved From aac6667fbc5ac7cf1cf09d7046558d564561a4aa Mon Sep 17 00:00:00 2001 From: Enrico Molino Date: Wed, 22 Oct 2025 10:53:49 +0200 Subject: [PATCH 3/3] Release 3.2.6 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index b4955d02f..5d712c5bd 100644 --- a/build.gradle +++ b/build.gradle @@ -3,7 +3,7 @@ plugins { id "org.owasp.dependencycheck" version "12.1.3" } -ext.projectVersion = '3.2.0-SNAPSHOT' +ext.projectVersion = '3.2.6' ext.isReleaseVersion = !ext.projectVersion.endsWith('SNAPSHOT') ext.mavenRepoUrl = project.properties['mavenRepoUrl'] ?: 'https://ossrh-staging-api.central.sonatype.com/service/local/staging/deploy/maven2/'