Skip to content

Commit 85fec4f

Browse files
symphony-enricosymphony-enrico
authored
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
1 parent 4cab9ae commit 85fec4f

4 files changed

Lines changed: 225 additions & 32 deletions

File tree

‎symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/form/FormReplyActivity.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public abstract class FormReplyActivity<C extends FormReplyContext>
2626
/** {@inheritDoc} */
2727
@Override
2828
protected void bindToRealTimeEventsSource(Consumer<RealTimeEventListener> realTimeEventsSource) {
29-
bindOnSymphonyElementsAction(realTimeEventsSource, this::processEvent);
29+
bindOnSymphonyElementsAction(realTimeEventsSource, this::processEvent, this);
3030
}
3131

3232
/** {@inheritDoc} */

‎symphony-bdk-core/src/main/java/com/symphony/bdk/core/activity/room/UserJoinedRoomActivity.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public abstract class UserJoinedRoomActivity<C extends UserJoinedRoomContext>
2222
*/
2323
@Override
2424
protected void bindToRealTimeEventsSource(Consumer<RealTimeEventListener> realTimeEventsSource) {
25-
bindOnUserJoinedRoom(realTimeEventsSource, this::processEvent);
25+
bindOnUserJoinedRoom(realTimeEventsSource, this::processEvent, this);
2626
}
2727

2828
/**

‎symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinder.java‎

Lines changed: 100 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
package com.symphony.bdk.core.service.datafeed.util;
22

3+
import com.symphony.bdk.core.activity.AbstractActivity;
34
import com.symphony.bdk.core.service.datafeed.RealTimeEventListener;
45
import com.symphony.bdk.gen.api.model.V4Initiator;
56
import com.symphony.bdk.gen.api.model.V4MessageSent;
67
import com.symphony.bdk.gen.api.model.V4SymphonyElementsAction;
7-
88
import com.symphony.bdk.gen.api.model.V4UserJoinedRoom;
99

1010
import org.apiguardian.api.API;
1111

12+
import java.util.Objects;
1213
import java.util.function.BiConsumer;
1314
import java.util.function.Consumer;
1415

16+
import javax.annotation.Nullable;
17+
1518
/**
1619
* Utility class used to attach a method call (defined by a {@link BiConsumer}) to a specific real-time event.
1720
*/
@@ -26,47 +29,40 @@ public RealTimeEventsBinder() {
2629
* Bind "onMessageSent" real-time event to a target method.
2730
*
2831
* @param subscriber The Datafeed real-time events subscriber.
29-
* @param target Target method.
32+
* @param target Target method.
33+
* @param activity The activity that creates the listener. It can be null, but in that case it will not be possible
34+
* to unsubscribe the activity later.
3035
*/
31-
public static void bindOnMessageSent(Consumer<RealTimeEventListener> subscriber, BiConsumer<V4Initiator, V4MessageSent> target) {
32-
subscriber.accept(new RealTimeEventListener() {
33-
34-
@Override
35-
public void onMessageSent(V4Initiator initiator, V4MessageSent event) {
36-
target.accept(initiator, event);
37-
}
38-
});
36+
public static void bindOnMessageSent(Consumer<RealTimeEventListener> subscriber,
37+
BiConsumer<V4Initiator, V4MessageSent> target, @Nullable AbstractActivity<V4MessageSent, ?> activity) {
38+
subscriber.accept(new OnMessageSent(activity, target));
3939
}
4040

4141
/**
4242
* Bind "onSymphonyElementsAction" real-time event to a target method.
4343
*
4444
* @param subscriber The Datafeed real-time events subscriber.
45-
* @param target Target method.
45+
* @param target Target method.
46+
* @param activity The activity that creates the listener. It can be null, but in that case it will not be possible
47+
* to unsubscribe the activity later.
4648
*/
47-
public static void bindOnSymphonyElementsAction(Consumer<RealTimeEventListener> subscriber, BiConsumer<V4Initiator, V4SymphonyElementsAction> target) {
48-
subscriber.accept(new RealTimeEventListener() {
49-
50-
@Override
51-
public void onSymphonyElementsAction(V4Initiator initiator, V4SymphonyElementsAction event) {
52-
target.accept(initiator, event);
53-
}
54-
});
49+
public static void bindOnSymphonyElementsAction(Consumer<RealTimeEventListener> subscriber,
50+
BiConsumer<V4Initiator, V4SymphonyElementsAction> target,
51+
@Nullable AbstractActivity<V4SymphonyElementsAction, ?> activity) {
52+
subscriber.accept(new OnSymphonyElementsAction(activity, target));
5553
}
5654

5755
/**
5856
* Bind "onUserJoinedRoom" real-time event to a target method.
5957
*
6058
* @param subscriber The Datafeed real-time events subscriber.
61-
* @param target Target method.
59+
* @param target Target method.
60+
* @param activity The activity that creates the listener. It can be null, but in that case it will not be possible
61+
* to unsubscribe the activity later.
6262
*/
63-
public static void bindOnUserJoinedRoom(Consumer<RealTimeEventListener> subscriber, BiConsumer<V4Initiator, V4UserJoinedRoom> target) {
64-
subscriber.accept(new RealTimeEventListener() {
65-
@Override
66-
public void onUserJoinedRoom(V4Initiator initiator, V4UserJoinedRoom event) {
67-
target.accept(initiator, event);
68-
}
69-
});
63+
public static void bindOnUserJoinedRoom(Consumer<RealTimeEventListener> subscriber,
64+
BiConsumer<V4Initiator, V4UserJoinedRoom> target, @Nullable AbstractActivity<V4UserJoinedRoom, ?> activity) {
65+
subscriber.accept(new OnUserJoinedRoom(activity, target));
7066
}
7167

7268
/**
@@ -78,4 +74,81 @@ public void onUserJoinedRoom(V4Initiator initiator, V4UserJoinedRoom event) {
7874
public static void bindRealTimeListener(Consumer<RealTimeEventListener> consumer, RealTimeEventListener listener) {
7975
consumer.accept(listener);
8076
}
77+
78+
/**
79+
* Internal private records used to create listeners from an {@link AbstractActivity}.
80+
* They keep a reference to the source {@link AbstractActivity}, which is used only for equality validation.
81+
* This ensures that multiple identical listeners from the same {@link AbstractActivity} are not registered,
82+
* and also allows the {@link AbstractActivity} to be unsubscribed later.
83+
*/
84+
85+
private record OnMessageSent(AbstractActivity<V4MessageSent, ?> activity,
86+
BiConsumer<V4Initiator, V4MessageSent> target)
87+
implements RealTimeEventListener {
88+
89+
@Override
90+
public void onMessageSent(V4Initiator initiator, V4MessageSent event) {
91+
target.accept(initiator, event);
92+
}
93+
94+
@Override
95+
public boolean equals(Object o) {
96+
// If the listener is created with a null activity, there is no way to identify it later.
97+
// For this reason, equals will always return false.
98+
if (!(o instanceof OnMessageSent that) || this.activity == null) {return false;}
99+
return activity.equals(that.activity);
100+
}
101+
102+
@Override
103+
public int hashCode() {
104+
return Objects.hashCode(activity);
105+
}
106+
}
107+
108+
109+
private record OnSymphonyElementsAction(AbstractActivity<V4SymphonyElementsAction, ?> activity,
110+
BiConsumer<V4Initiator, V4SymphonyElementsAction> target)
111+
implements RealTimeEventListener {
112+
113+
@Override
114+
public void onSymphonyElementsAction(V4Initiator initiator, V4SymphonyElementsAction event) {
115+
target.accept(initiator, event);
116+
}
117+
118+
@Override
119+
public boolean equals(Object o) {
120+
// If the listener is created with a null activity, there is no way to identify it later.
121+
// For this reason, equals will always return false.
122+
if (!(o instanceof OnSymphonyElementsAction that) || this.activity == null) {return false;}
123+
return activity.equals(that.activity);
124+
}
125+
126+
@Override
127+
public int hashCode() {
128+
return Objects.hashCode(activity);
129+
}
130+
}
131+
132+
133+
private record OnUserJoinedRoom(AbstractActivity<V4UserJoinedRoom, ?> activity,
134+
BiConsumer<V4Initiator, V4UserJoinedRoom> target) implements RealTimeEventListener {
135+
136+
@Override
137+
public void onUserJoinedRoom(V4Initiator initiator, V4UserJoinedRoom event) {
138+
target.accept(initiator, event);
139+
}
140+
141+
@Override
142+
public boolean equals(Object o) {
143+
// If the listener is created with a null activity, there is no way to identify it later.
144+
// For this reason, equals will always return false.
145+
if (!(o instanceof OnUserJoinedRoom that) || this.activity == null) {return false;}
146+
return activity.equals(that.activity);
147+
}
148+
149+
@Override
150+
public int hashCode() {
151+
return Objects.hashCode(activity);
152+
}
153+
}
81154
}

‎symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/util/RealTimeEventsBinderTest.java‎

Lines changed: 123 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
import static org.junit.jupiter.api.Assertions.*;
44

5+
import com.symphony.bdk.core.activity.AbstractActivity;
6+
import com.symphony.bdk.core.activity.ActivityContext;
7+
import com.symphony.bdk.core.activity.ActivityMatcher;
8+
import com.symphony.bdk.core.activity.model.ActivityInfo;
9+
import com.symphony.bdk.core.service.datafeed.EventException;
510
import com.symphony.bdk.core.service.datafeed.RealTimeEventListener;
611
import com.symphony.bdk.gen.api.model.V4Initiator;
712
import com.symphony.bdk.gen.api.model.V4MessageSent;
@@ -38,29 +43,144 @@ void testConstructorJustToMakeJacocoHappy() {
3843
void testBindOnMessageSent() {
3944
final AtomicBoolean methodCalled = new AtomicBoolean(false);
4045
final BiConsumer<V4Initiator, V4MessageSent> methodToBind = (initiator, v4MessageSent) -> methodCalled.set(true);
41-
RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind);
46+
RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind, null);
4247
this.realTimeEventsProvider.trigger(l -> l.onMessageSent(new V4Initiator(), new V4MessageSent()));
4348
assertTrue(methodCalled.get());
4449
}
4550

51+
@Test
52+
void testBindOnMessageSentEqualsOnActivity() {
53+
final BiConsumer<V4Initiator, V4MessageSent> methodToBind1 = (initiator, v4MessageSent) -> {};
54+
final BiConsumer<V4Initiator, V4MessageSent> methodToBind2 = (initiator, v4MessageSent) -> {};
55+
AbstractActivity<V4MessageSent, ?> activity = new AbstractActivity<>() {
56+
57+
@Override
58+
protected ActivityMatcher matcher() throws EventException {
59+
return null;
60+
}
61+
62+
@Override
63+
protected ActivityInfo info() {
64+
return null;
65+
}
66+
67+
@Override
68+
protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) {
69+
70+
}
71+
72+
@Override
73+
protected void onActivity(ActivityContext context) throws EventException {
74+
75+
}
76+
};
77+
78+
79+
RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind1, activity);
80+
RealTimeEventListener listener1 = this.realTimeEventsProvider.listener;
81+
82+
RealTimeEventsBinder.bindOnMessageSent(this.realTimeEventsProvider::setListener, methodToBind2, activity);
83+
RealTimeEventListener listener2 = this.realTimeEventsProvider.listener;
84+
85+
assertTrue(listener1 != listener2);
86+
assertEquals(listener1, listener2);
87+
assertEquals(listener1.hashCode(), listener2.hashCode());
88+
}
89+
4690
@Test
4791
void testBindOnSymphonyElementsAction() {
4892
final AtomicBoolean methodCalled = new AtomicBoolean(false);
4993
final BiConsumer<V4Initiator, V4SymphonyElementsAction> methodToBind = (initiator, v4SymphonyElementsAction) -> methodCalled.set(true);
50-
RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind);
94+
RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind, null);
5195
this.realTimeEventsProvider.trigger(l -> l.onSymphonyElementsAction(new V4Initiator(), new V4SymphonyElementsAction()));
5296
assertTrue(methodCalled.get());
5397
}
5498

99+
@Test
100+
void testBindOnSymphonyElementsActionEqualsOnActivity() {
101+
final BiConsumer<V4Initiator, V4SymphonyElementsAction> methodToBind1 = (initiator, v4SymphonyElementsAction) -> {};
102+
final BiConsumer<V4Initiator, V4SymphonyElementsAction> methodToBind2 = (initiator, v4SymphonyElementsAction) -> {};
103+
AbstractActivity<V4SymphonyElementsAction, ?> activity = new AbstractActivity<>() {
104+
105+
@Override
106+
protected ActivityMatcher matcher() throws EventException {
107+
return null;
108+
}
109+
110+
@Override
111+
protected ActivityInfo info() {
112+
return null;
113+
}
114+
115+
@Override
116+
protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) {
117+
118+
}
119+
120+
@Override
121+
protected void onActivity(ActivityContext context) throws EventException {
122+
123+
}
124+
};
125+
126+
RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind1, activity);
127+
RealTimeEventListener listener1 = this.realTimeEventsProvider.listener;
128+
129+
RealTimeEventsBinder.bindOnSymphonyElementsAction(this.realTimeEventsProvider::setListener, methodToBind2, activity);
130+
RealTimeEventListener listener2 = this.realTimeEventsProvider.listener;
131+
132+
assertTrue(listener1 != listener2);
133+
assertEquals(listener1, listener2);
134+
assertEquals(listener1.hashCode(), listener2.hashCode());
135+
}
136+
55137
@Test
56138
void testBindOnUserJoinedRoom() {
57139
final AtomicBoolean methodCalled = new AtomicBoolean(false);
58140
final BiConsumer<V4Initiator, V4UserJoinedRoom> methodToBind = ((initiator, v4UserJoinedRoom) -> methodCalled.set(true));
59-
RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind);
141+
RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind, null);
60142
this.realTimeEventsProvider.trigger(l -> l.onUserJoinedRoom(new V4Initiator(), new V4UserJoinedRoom()));
61143
assertTrue(methodCalled.get());
62144
}
63145

146+
@Test
147+
void testBindOnUserJoinedRoomEqualsOnActivity() {
148+
final BiConsumer<V4Initiator, V4UserJoinedRoom> methodToBind1 = (initiator, v4UserJoinedRoom) -> {};
149+
final BiConsumer<V4Initiator, V4UserJoinedRoom> methodToBind2 = (initiator, v4UserJoinedRoom) -> {};
150+
AbstractActivity<V4UserJoinedRoom, ?> activity = new AbstractActivity<>() {
151+
152+
@Override
153+
protected ActivityMatcher matcher() throws EventException {
154+
return null;
155+
}
156+
157+
@Override
158+
protected ActivityInfo info() {
159+
return null;
160+
}
161+
162+
@Override
163+
protected void bindToRealTimeEventsSource(Consumer realTimeEventsSource) {
164+
165+
}
166+
167+
@Override
168+
protected void onActivity(ActivityContext context) throws EventException {
169+
170+
}
171+
};
172+
173+
RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind1, activity);
174+
RealTimeEventListener listener1 = this.realTimeEventsProvider.listener;
175+
176+
RealTimeEventsBinder.bindOnUserJoinedRoom(this.realTimeEventsProvider::setListener, methodToBind2, activity);
177+
RealTimeEventListener listener2 = this.realTimeEventsProvider.listener;
178+
179+
assertTrue(listener1 != listener2);
180+
assertEquals(listener1, listener2);
181+
assertEquals(listener1.hashCode(), listener2.hashCode());
182+
}
183+
64184
private static class RealTimeEventsProvider {
65185

66186
private RealTimeEventListener listener;

0 commit comments

Comments
 (0)