From 935ff900fabc2261f909b63a5a5cafe6bad12791 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 16 Sep 2024 11:18:23 +0530 Subject: [PATCH 1/6] Added attachOnSubscribe option to ChannelOptions --- lib/src/main/java/io/ably/lib/types/ChannelOptions.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java index 29186eee9..725425203 100644 --- a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java +++ b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java @@ -40,6 +40,13 @@ public class ChannelOptions { */ public boolean encrypted; + /** + * Determines whether calling @subscribe@ on a channel or presence object should trigger an implicit attach. + * Defaults to @true@. + * Spec: RTP6d, RTP6e, TB4 + */ + public boolean attachOnSubscribe = true; + public boolean hasModes() { return null != modes && 0 != modes.length; } From 630c70dc238f399a2c78844fef84cccb6d5bed47 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 16 Sep 2024 11:22:26 +0530 Subject: [PATCH 2/6] Added attachOnSubscribe check before implicit attach on channel subscribe --- .../main/java/io/ably/lib/realtime/ChannelBase.java | 12 +++++++++--- lib/src/main/java/io/ably/lib/realtime/Presence.java | 4 ++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java index 8dc90c946..856bb3a4e 100644 --- a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java +++ b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java @@ -704,7 +704,9 @@ public synchronized void unsubscribe() { public synchronized void subscribe(MessageListener listener) throws AblyException { Log.v(TAG, "subscribe(); channel = " + this.name); listeners.add(listener); - attach(); + if (options.attachOnSubscribe) { + attach(); + } } /** @@ -739,7 +741,9 @@ public synchronized void unsubscribe(MessageListener listener) { public synchronized void subscribe(String name, MessageListener listener) throws AblyException { Log.v(TAG, "subscribe(); channel = " + this.name + "; event = " + name); subscribeImpl(name, listener); - attach(); + if (options.attachOnSubscribe) { + attach(); + } } /** @@ -773,7 +777,9 @@ public synchronized void subscribe(String[] names, MessageListener listener) thr Log.v(TAG, "subscribe(); channel = " + this.name + "; (multiple events)"); for(String name : names) subscribeImpl(name, listener); - attach(); + if (options.attachOnSubscribe) { + attach(); + } } /** diff --git a/lib/src/main/java/io/ably/lib/realtime/Presence.java b/lib/src/main/java/io/ably/lib/realtime/Presence.java index 1ada6d092..0a29ab9ab 100644 --- a/lib/src/main/java/io/ably/lib/realtime/Presence.java +++ b/lib/src/main/java/io/ably/lib/realtime/Presence.java @@ -308,6 +308,10 @@ public void unsubscribe() { * @throws AblyException */ private void implicitAttachOnSubscribe(CompletionListener completionListener) throws AblyException { + if (!channel.options.attachOnSubscribe) { + completionListener.onSuccess(); + return; + } if (channel.state == ChannelState.failed) { String errorString = String.format(Locale.ROOT, "Channel %s: subscribe in FAILED channel state", channel.name); Log.v(TAG, errorString); From e7fe76ed6657d4adb571a938d43328f7f5967369 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 16 Sep 2024 13:34:00 +0530 Subject: [PATCH 3/6] Added extra null channelOptions check while checking for attachOnSubscribe --- .../java/io/ably/lib/realtime/ChannelBase.java | 15 ++++++++++++--- .../main/java/io/ably/lib/realtime/Presence.java | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java index 856bb3a4e..bfffef44b 100644 --- a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java +++ b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java @@ -690,6 +690,15 @@ public synchronized void unsubscribe() { eventListeners.clear(); } + /** + * Checks for null channelOptions and checks if options.attachOnSubscribe is true + * Defaults to @true@ when channelOptions is null. + * Spec: RTP6d, RTP6e, TB4 + */ + protected boolean attachOnSubscribeEnabled() { + return options == null || options.attachOnSubscribe; + } + /** * Registers a listener for messages on this channel. * The caller supplies a listener function, which is called each time one or more messages arrives on the channel. @@ -704,7 +713,7 @@ public synchronized void unsubscribe() { public synchronized void subscribe(MessageListener listener) throws AblyException { Log.v(TAG, "subscribe(); channel = " + this.name); listeners.add(listener); - if (options.attachOnSubscribe) { + if (attachOnSubscribeEnabled()) { attach(); } } @@ -741,7 +750,7 @@ public synchronized void unsubscribe(MessageListener listener) { public synchronized void subscribe(String name, MessageListener listener) throws AblyException { Log.v(TAG, "subscribe(); channel = " + this.name + "; event = " + name); subscribeImpl(name, listener); - if (options.attachOnSubscribe) { + if (attachOnSubscribeEnabled()) { attach(); } } @@ -777,7 +786,7 @@ public synchronized void subscribe(String[] names, MessageListener listener) thr Log.v(TAG, "subscribe(); channel = " + this.name + "; (multiple events)"); for(String name : names) subscribeImpl(name, listener); - if (options.attachOnSubscribe) { + if (attachOnSubscribeEnabled()) { attach(); } } diff --git a/lib/src/main/java/io/ably/lib/realtime/Presence.java b/lib/src/main/java/io/ably/lib/realtime/Presence.java index 0a29ab9ab..cae2ec845 100644 --- a/lib/src/main/java/io/ably/lib/realtime/Presence.java +++ b/lib/src/main/java/io/ably/lib/realtime/Presence.java @@ -308,7 +308,7 @@ public void unsubscribe() { * @throws AblyException */ private void implicitAttachOnSubscribe(CompletionListener completionListener) throws AblyException { - if (!channel.options.attachOnSubscribe) { + if (!channel.attachOnSubscribeEnabled()) { completionListener.onSuccess(); return; } From d83d3aef9d545044cd1dff6d77a26cc956d12156 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 17 Sep 2024 15:02:43 +0530 Subject: [PATCH 4/6] Fixed spec annotations for the attachOnSubscribe usecases --- lib/src/main/java/io/ably/lib/realtime/ChannelBase.java | 2 +- lib/src/main/java/io/ably/lib/types/ChannelOptions.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java index bfffef44b..5dbb8d7c0 100644 --- a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java +++ b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java @@ -693,7 +693,7 @@ public synchronized void unsubscribe() { /** * Checks for null channelOptions and checks if options.attachOnSubscribe is true * Defaults to @true@ when channelOptions is null. - * Spec: RTP6d, RTP6e, TB4 + * Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e */ protected boolean attachOnSubscribeEnabled() { return options == null || options.attachOnSubscribe; diff --git a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java index 725425203..43a29a92f 100644 --- a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java +++ b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java @@ -43,7 +43,7 @@ public class ChannelOptions { /** * Determines whether calling @subscribe@ on a channel or presence object should trigger an implicit attach. * Defaults to @true@. - * Spec: RTP6d, RTP6e, TB4 + * Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e */ public boolean attachOnSubscribe = true; From d8181e874b980a20be2210019c557c92105938eb Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 17 Sep 2024 15:03:37 +0530 Subject: [PATCH 5/6] Added tests for channel/presence subscribe without implicit attach --- .../java/io/ably/lib/realtime/Presence.java | 4 +- .../test/realtime/RealtimeChannelTest.java | 62 +++++++++++++++++++ .../test/realtime/RealtimePresenceTest.java | 60 ++++++++++++++++++ 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/io/ably/lib/realtime/Presence.java b/lib/src/main/java/io/ably/lib/realtime/Presence.java index cae2ec845..504985e98 100644 --- a/lib/src/main/java/io/ably/lib/realtime/Presence.java +++ b/lib/src/main/java/io/ably/lib/realtime/Presence.java @@ -309,7 +309,9 @@ public void unsubscribe() { */ private void implicitAttachOnSubscribe(CompletionListener completionListener) throws AblyException { if (!channel.attachOnSubscribeEnabled()) { - completionListener.onSuccess(); + if (completionListener != null) { + completionListener.onSuccess(); + } return; } if (channel.state == ChannelState.failed) { diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java index f58fe832f..82763a62e 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java @@ -393,6 +393,68 @@ public void onMessage(Message message) { } } + /** + *

+ * Validates a client can subscribe to messages without implicit channel attach + * Refer Spec TB4, RTL7g, RTL7gh + *

+ * @throws AblyException + */ + @Test + public void subscribe_without_implicit_attach() { + String channelName = "subscribe_" + testParams.name; + AblyRealtime ably = null; + try { + ClientOptions opts = createOptions(testVars.keys[0].keyStr); + ably = new AblyRealtime(opts); + + /* create a channel and set attachOnSubscribe to false */ + final Channel channel = ably.channels.get(channelName); + ChannelOptions chOpts = new ChannelOptions(); + chOpts.attachOnSubscribe = false; + channel.setOptions(chOpts); + + List receivedMsg = new ArrayList<>(); + + /* Check for all subscriptions without ATTACHING state */ + channel.subscribe(message -> receivedMsg.add(true)); + assertEquals(channel.state, ChannelState.initialized); + + channel.subscribe("test_event", message -> receivedMsg.add(true)); + assertEquals(channel.state, ChannelState.initialized); + + channel.subscribe(new String[]{"test_event1", "test_event2"}, message -> receivedMsg.add(true)); + assertEquals(channel.state, ChannelState.initialized); + + channel.attach(); + (new ChannelWaiter(channel)).waitFor(ChannelState.attached); + + channel.publish("test_event", "hi there"); + Exception conditionError = new Helpers.ConditionalWaiter(). + wait(() -> receivedMsg.size() == 2, 5000); + assertNull(conditionError); + + receivedMsg.clear(); + channel.publish("test_event1", "hi there"); + conditionError = new Helpers.ConditionalWaiter(). + wait(() -> receivedMsg.size() == 2, 5000); + assertNull(conditionError); + + receivedMsg.clear(); + channel.publish("test_event2", "hi there"); + conditionError = new Helpers.ConditionalWaiter(). + wait(() -> receivedMsg.size() == 2, 5000); + assertNull(conditionError); + + } catch (AblyException e) { + e.printStackTrace(); + fail("init0: Unexpected exception instantiating library"); + } finally { + if(ably != null) + ably.close(); + } + } + /** *

* Verifies that unsubscribe call with no argument removes all listeners, diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java index 9fe903675..c5699be08 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java @@ -1624,6 +1624,66 @@ public void onPresenceMessage(PresenceMessage message) { } } + /** + *

+ * Validates a client can subscribe to presence without implicit channel attach + * Refer Spec TB4, RTP6d, RTP6e + *

+ * @throws AblyException + */ + @Test + public void presence_subscribe_without_implicit_attach() { + String ablyChannel = "subscribe_" + testParams.name; + AblyRealtime ably = null; + try { + ClientOptions option1 = createOptions(testVars.keys[0].keyStr); + option1.clientId = "client1"; + ably = new AblyRealtime(option1); + + /* create a channel and set attachOnSubscribe to false */ + final Channel channel = ably.channels.get(ablyChannel); + ChannelOptions chOpts = new ChannelOptions(); + chOpts.attachOnSubscribe = false; + channel.setOptions(chOpts); + + List receivedPresenceMsg = new ArrayList<>(); + CompletionWaiter completionWaiter = new CompletionWaiter(); + + /* Check for all subscriptions without ATTACHING state */ + channel.presence.subscribe(m -> receivedPresenceMsg.add(true), completionWaiter); + assertEquals(completionWaiter.successCount, 1); + assertEquals(channel.state, ChannelState.initialized); + + channel.presence.subscribe(Action.enter, m -> receivedPresenceMsg.add(true), completionWaiter); + assertEquals(completionWaiter.successCount, 2); + assertEquals(channel.state, ChannelState.initialized); + + channel.presence.subscribe(EnumSet.of(Action.enter, Action.leave),m -> receivedPresenceMsg.add(true)); + assertEquals(channel.state, ChannelState.initialized); + + channel.attach(); + (new ChannelWaiter(channel)).waitFor(ChannelState.attached); + + channel.presence.enter("enter client1", null); + Exception conditionError = new Helpers.ConditionalWaiter(). + wait(() -> receivedPresenceMsg.size() == 3, 5000); + assertNull(conditionError); + + receivedPresenceMsg.clear(); + channel.presence.leave(null); + conditionError = new Helpers.ConditionalWaiter(). + wait(() -> receivedPresenceMsg.size() == 2, 5000); + assertNull(conditionError); + + } catch (AblyException e) { + e.printStackTrace(); + fail("init0: Unexpected exception instantiating library"); + } finally { + if(ably != null) + ably.close(); + } + } + /** *

* Validates a client sending multiple presence updates when the channel is in the attaching From 3ecab484c44ff8fd5b14a12c70e87f66ff7a4bd6 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 17 Sep 2024 16:10:26 +0530 Subject: [PATCH 6/6] Refactored attachOnSubscribe code, spec and related tests as per rabbitcodeai comments --- .../java/io/ably/lib/realtime/ChannelBase.java | 8 +++++--- .../java/io/ably/lib/types/ChannelOptions.java | 10 +++++++--- .../lib/test/realtime/RealtimeChannelTest.java | 13 ++++++++----- .../lib/test/realtime/RealtimePresenceTest.java | 16 +++++++++------- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java index 5dbb8d7c0..9a786a602 100644 --- a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java +++ b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java @@ -691,9 +691,11 @@ public synchronized void unsubscribe() { } /** - * Checks for null channelOptions and checks if options.attachOnSubscribe is true - * Defaults to @true@ when channelOptions is null. - * Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e + *

+ * Checks if {@link io.ably.lib.types.ChannelOptions#attachOnSubscribe} is true. + *

+ * Defaults to {@code true} when {@link io.ably.lib.realtime.ChannelBase#options} is null. + *

Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e

*/ protected boolean attachOnSubscribeEnabled() { return options == null || options.attachOnSubscribe; diff --git a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java index 43a29a92f..8ee10faf3 100644 --- a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java +++ b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java @@ -41,9 +41,13 @@ public class ChannelOptions { public boolean encrypted; /** - * Determines whether calling @subscribe@ on a channel or presence object should trigger an implicit attach. - * Defaults to @true@. - * Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e + *

+ * Determines whether calling {@link io.ably.lib.realtime.Channel#subscribe Channel.subscribe} or + * {@link io.ably.lib.realtime.Presence#subscribe Presence.subscribe} method + * should trigger an implicit attach. + *

+ *

Defaults to {@code true}.

+ *

Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e

*/ public boolean attachOnSubscribe = true; diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java index 82763a62e..bdeb11921 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java @@ -414,41 +414,44 @@ public void subscribe_without_implicit_attach() { chOpts.attachOnSubscribe = false; channel.setOptions(chOpts); - List receivedMsg = new ArrayList<>(); + List receivedMsg = Collections.synchronizedList(new ArrayList<>()); /* Check for all subscriptions without ATTACHING state */ channel.subscribe(message -> receivedMsg.add(true)); - assertEquals(channel.state, ChannelState.initialized); + assertEquals(ChannelState.initialized, channel.state); channel.subscribe("test_event", message -> receivedMsg.add(true)); - assertEquals(channel.state, ChannelState.initialized); + assertEquals(ChannelState.initialized, channel.state); channel.subscribe(new String[]{"test_event1", "test_event2"}, message -> receivedMsg.add(true)); - assertEquals(channel.state, ChannelState.initialized); + assertEquals(ChannelState.initialized, channel.state); channel.attach(); (new ChannelWaiter(channel)).waitFor(ChannelState.attached); channel.publish("test_event", "hi there"); + // Expecting two msg: one from the wildcard subscription and one from test_event subscription Exception conditionError = new Helpers.ConditionalWaiter(). wait(() -> receivedMsg.size() == 2, 5000); assertNull(conditionError); receivedMsg.clear(); channel.publish("test_event1", "hi there"); + // Expecting two msg: one from the wildcard subscription and one from test_event1 subscription conditionError = new Helpers.ConditionalWaiter(). wait(() -> receivedMsg.size() == 2, 5000); assertNull(conditionError); receivedMsg.clear(); channel.publish("test_event2", "hi there"); + // Expecting two msg: one from the wildcard subscription and one from test_event2 subscription conditionError = new Helpers.ConditionalWaiter(). wait(() -> receivedMsg.size() == 2, 5000); assertNull(conditionError); } catch (AblyException e) { e.printStackTrace(); - fail("init0: Unexpected exception instantiating library"); + fail("subscribe_without_implicit_attach: Unexpected exception"); } finally { if(ably != null) ably.close(); diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java index c5699be08..a13fe235f 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java @@ -1646,38 +1646,40 @@ public void presence_subscribe_without_implicit_attach() { chOpts.attachOnSubscribe = false; channel.setOptions(chOpts); - List receivedPresenceMsg = new ArrayList<>(); + List receivedPresenceMsg = Collections.synchronizedList(new ArrayList<>()); CompletionWaiter completionWaiter = new CompletionWaiter(); /* Check for all subscriptions without ATTACHING state */ channel.presence.subscribe(m -> receivedPresenceMsg.add(true), completionWaiter); - assertEquals(completionWaiter.successCount, 1); - assertEquals(channel.state, ChannelState.initialized); + assertEquals(1, completionWaiter.successCount); + assertEquals(ChannelState.initialized, channel.state); channel.presence.subscribe(Action.enter, m -> receivedPresenceMsg.add(true), completionWaiter); - assertEquals(completionWaiter.successCount, 2); - assertEquals(channel.state, ChannelState.initialized); + assertEquals(2, completionWaiter.successCount); + assertEquals(ChannelState.initialized, channel.state); channel.presence.subscribe(EnumSet.of(Action.enter, Action.leave),m -> receivedPresenceMsg.add(true)); - assertEquals(channel.state, ChannelState.initialized); + assertEquals(ChannelState.initialized, channel.state); channel.attach(); (new ChannelWaiter(channel)).waitFor(ChannelState.attached); channel.presence.enter("enter client1", null); + // Expecting 3 msg: one from the wildcard subscription and two from specific event subscription Exception conditionError = new Helpers.ConditionalWaiter(). wait(() -> receivedPresenceMsg.size() == 3, 5000); assertNull(conditionError); receivedPresenceMsg.clear(); channel.presence.leave(null); + // Expecting 2 msg: one from the wildcard subscription and one from specific event subscription conditionError = new Helpers.ConditionalWaiter(). wait(() -> receivedPresenceMsg.size() == 2, 5000); assertNull(conditionError); } catch (AblyException e) { e.printStackTrace(); - fail("init0: Unexpected exception instantiating library"); + fail("presence_subscribe_without_implicit_attach: Unexpected exception"); } finally { if(ably != null) ably.close();