Refactored attachOnSubscribe code, spec and related tests as per rabb…

…itcodeai comments
ably · Sep 17, 2024 · 3ecab48 · 3ecab48
1 parent d8181e8
commit 3ecab48
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 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
@@ -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
+     * <p>
+     * Checks if {@link io.ably.lib.types.ChannelOptions#attachOnSubscribe} is true.
+     * </p>
+     * Defaults to {@code true} when {@link io.ably.lib.realtime.ChannelBase#options} is null.
+     * <p>Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e</p>
      */
     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
@@ -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
+     * <p>
+     * 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.
+     * </p>
+     * <p>Defaults to {@code true}.</p>
+     * <p>Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e</p>
      */
     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
@@ -414,41 +414,44 @@ public void subscribe_without_implicit_attach() {
             chOpts.attachOnSubscribe = false;
             channel.setOptions(chOpts);
 
-            List<Object> receivedMsg = new ArrayList<>();
+            List<Boolean> 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
@@ -1646,38 +1646,40 @@ public void presence_subscribe_without_implicit_attach() {
             chOpts.attachOnSubscribe = false;
             channel.setOptions(chOpts);
 
-            List<Object> receivedPresenceMsg = new ArrayList<>();
+            List<Boolean> 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();