Merge pull request #1043 from ably/fix/RTP6e-implicit-channel-attach

[ECO-4972] Fix implicit channel ATTACH
ably · Oct 17, 2024 · dc30b41 · dc30b41
2 parents 6a66eec + 43f2eaa
commit dc30b41
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 19 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
@@ -695,7 +695,7 @@ public synchronized void unsubscribe() {
      * 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>
+     * <p>Spec: TB4, RTL7g, RTL7h, RTP6d, RTP6e</p>
      */
     protected boolean attachOnSubscribeEnabled() {
         return options == null || options.attachOnSubscribe;

diff --git a/lib/src/main/java/io/ably/lib/realtime/Presence.java b/lib/src/main/java/io/ably/lib/realtime/Presence.java
@@ -296,27 +296,26 @@ public void unsubscribe() {
         eventListeners.clear();
     }
 
-
-    /***
-     * internal
-     *
-     */
-
     /**
-     * Implicitly attach channel on subscribe. Throw exception if channel is in failed state
-     * @param completionListener
-     * @throws AblyException
+     * Implicitly attach channel on subscribe. Throw exception if channel is in failed state.
+     * @param completionListener Registers listener, gets called when ATTACH operation is a success.
+     * @throws AblyException Throws exception when channel is in failed state.
      */
     private void implicitAttachOnSubscribe(CompletionListener completionListener) throws AblyException {
+        // RTP6e
         if (!channel.attachOnSubscribeEnabled()) {
             if (completionListener != null) {
-                completionListener.onSuccess();
+                String errorString = String.format(
+                    "Channel %s: attachOnSubscribe=false doesn't expect attach completion callback", channel.name);
+                Log.e(TAG, errorString);
+                ErrorInfo errorInfo = new ErrorInfo(errorString, 400,40000);
+                throw AblyException.fromErrorInfo(errorInfo);
             }
             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);
+            Log.e(TAG, errorString);
             ErrorInfo errorInfo = new ErrorInfo(errorString, 90001);
             throw AblyException.fromErrorInfo(errorInfo);
         }

diff --git a/lib/src/main/java/io/ably/lib/types/ChannelOptions.java b/lib/src/main/java/io/ably/lib/types/ChannelOptions.java
@@ -47,7 +47,7 @@ public class ChannelOptions {
      * should trigger an implicit attach.
      * </p>
      * <p>Defaults to {@code true}.</p>
-     * <p>Spec: TB4, RTL7g, RTL7gh, RTP6d, RTP6e</p>
+     * <p>Spec: TB4, RTL7g, RTL7h, 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
@@ -396,7 +396,7 @@ public void onMessage(Message message) {
     /**
      * <p>
      * Validates a client can subscribe to messages without implicit channel attach
-     * Refer Spec TB4, RTL7g, RTL7gh
+     * Refer Spec TB4, RTL7g, RTL7h
      * </p>
      * @throws AblyException
      */

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
@@ -1,5 +1,6 @@
 package io.ably.lib.test.realtime;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.emptyCollectionOf;
 import static org.hamcrest.Matchers.equalTo;
@@ -1647,15 +1648,12 @@ public void presence_subscribe_without_implicit_attach() {
             channel.setOptions(chOpts);
 
             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(1, completionWaiter.successCount);
+            channel.presence.subscribe(m -> receivedPresenceMsg.add(true));
             assertEquals(ChannelState.initialized, channel.state);
 
-            channel.presence.subscribe(Action.enter, m -> receivedPresenceMsg.add(true), completionWaiter);
-            assertEquals(2, completionWaiter.successCount);
+            channel.presence.subscribe(Action.enter, m -> receivedPresenceMsg.add(true));
             assertEquals(ChannelState.initialized, channel.state);
 
             channel.presence.subscribe(EnumSet.of(Action.enter, Action.leave),m -> receivedPresenceMsg.add(true));
@@ -1686,6 +1684,42 @@ public void presence_subscribe_without_implicit_attach() {
         }
     }
 
+    /**
+     * <p>
+     * Validates a client can subscribe to presence without implicit channel attach
+     * Refer Spec TB4, RTP6d, RTP6e
+     * </p>
+     * @throws AblyException
+     */
+    @Test
+    public void presence_subscribe_without_implicit_attach_and_completion_listener_throws_exception() throws AblyException {
+        String ablyChannel = "subscribe_" + testParams.name;
+        ClientOptions option1 = createOptions(testVars.keys[0].keyStr);
+        option1.clientId = "client1";
+        try (AblyRealtime 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);
+
+            // When completionWaiter passed with attachOnSubscribe=false, throws exception.
+            CompletionWaiter completionWaiter = new CompletionWaiter();
+            try {
+                channel.presence.subscribe(m -> {}, completionWaiter);
+            } catch (AblyException e) {
+                assertEquals(400, e.errorInfo.statusCode);
+                assertEquals(40000, e.errorInfo.code);
+                assertThat(e.errorInfo.message, containsString("attachOnSubscribe=false doesn't expect attach completion callback"));
+            }
+            assertEquals(ChannelState.initialized, channel.state);
+
+        } catch (AblyException e) {
+            e.printStackTrace();
+            fail("presence_subscribe_without_implicit_attach: Unexpected exception");
+        }
+    }
+
     /**
      * <p>
      * Validates a client sending multiple presence updates when the channel is in the attaching