[ECO-5172][RTL13] Fix existing impl. for server sent DETACH

1. Removed use of explicitly setting detached state 2. Fixed attachWithTimeout method call, set forcedAttach flag to true 3. Updated tests to track channel state changes on server sent DETACH
ably · Dec 17, 2024 · 0ffd50c · 0ffd50c
1 parent 2c0f111
commit 0ffd50c
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 15 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
@@ -244,8 +244,8 @@ private void attachImpl(final boolean forceReattach, final CompletionListener li
         }
 
         // (RTL4i)
-        if (connectionManager.getConnectionState().state == ConnectionState.connecting
-            || connectionManager.getConnectionState().state == ConnectionState.disconnected) {
+        ConnectionState connState = connectionManager.getConnectionState().state;
+        if (connState == ConnectionState.connecting || connState == ConnectionState.disconnected) {
             if (listener != null) {
                 on(new ChannelStateCompletionListener(listener, ChannelState.attached, ChannelState.failed));
             }
@@ -1296,18 +1296,12 @@ void onChannelMessage(ProtocolMessage msg) {
         case detached:
             ChannelState oldState = state;
             switch(oldState) {
+                // RTL13a
                 case attached:
-                case suspended: //RTL13a
-                    /* Unexpected detach, reattach when possible */
-                    setDetached((msg.error != null) ? msg.error : REASON_NOT_ATTACHED);
+                case suspended:
+                    /* Unexpected detach, reattach immediately as per RTL13a */
                     Log.v(TAG, String.format(Locale.ROOT, "Server initiated detach for channel %s; attempting reattach", name));
-                    try {
-                        attachWithTimeout(null);
-                    } catch (AblyException e) {
-                    /* Send message error */
-                        Log.e(TAG, "Attempting reattach threw exception", e);
-                        setDetached(e.errorInfo);
-                    }
+                    attachWithTimeout(true, null);
                     break;
                 case attaching:
                     /* RTL13b says we need to be suspended, but continue to retry */

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
@@ -26,6 +26,7 @@
 import io.ably.lib.types.ProtocolMessage;
 import io.ably.lib.util.Log;
 import org.hamcrest.Matchers;
+import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -1698,15 +1699,15 @@ public void channel_server_initiated_attached() throws AblyException {
 
     /*
      * Establish connection, attach channel, simulate sending detached messages
-     * from the server, test correct behaviour
+     * from the server for channel in attached state.
      *
      * Tests RTL13a
      */
     @Test
-    public void channel_server_initiated_detached() throws AblyException {
+    public void server_initiated_detach_for_attached_channel() throws AblyException {
         AblyRealtime ably = null;
         long oldRealtimeTimeout = Defaults.realtimeRequestTimeout;
-        final String channelName = "channel_server_initiated_attach_detach";
+        final String channelName = "channel_server_initiated_detach_for_attached_channel";
 
         try {
             ClientOptions opts = createOptions(testVars.keys[0].keyStr);
@@ -1735,6 +1736,70 @@ public void channel_server_initiated_detached() throws AblyException {
             channelWaiter.waitFor(ChannelState.attaching);
             channelWaiter.waitFor(ChannelState.attached);
 
+            List<ChannelState> channelStates = channelWaiter.getRecordedStates();
+            Assert.assertEquals(4, channelStates.size());
+            Assert.assertEquals(ChannelState.attaching, channelStates.get(0));
+            Assert.assertEquals(ChannelState.attached, channelStates.get(1));
+            Assert.assertEquals(ChannelState.attaching, channelStates.get(2));
+            Assert.assertEquals(ChannelState.attached, channelStates.get(3));
+
+        } finally {
+            if (ably != null)
+                ably.close();
+            Defaults.realtimeRequestTimeout = oldRealtimeTimeout;
+        }
+    }
+
+    /*
+     * Establish connection, attach channel, simulate sending detached messages
+     * from the server for channel in suspended state.
+     *
+     * Tests RTL13a
+     */
+    @Test
+    public void server_initiated_detach_for_suspended_channel() throws AblyException {
+        AblyRealtime ably = null;
+        long oldRealtimeTimeout = Defaults.realtimeRequestTimeout;
+        final String channelName = "channel_server_initiated_detach_for_suspended_channel";
+
+        try {
+            ClientOptions opts = createOptions(testVars.keys[0].keyStr);
+
+            /* Make test faster */
+            Defaults.realtimeRequestTimeout = 1000;
+            opts.channelRetryTimeout = 1000;
+
+            ably = new AblyRealtime(opts);
+            new ConnectionWaiter(ably.connection).waitFor(ConnectionState.connected);
+
+            Channel channel = ably.channels.get(channelName);
+            ChannelWaiter channelWaiter = new ChannelWaiter(channel);
+
+            channel.attach();
+            channelWaiter.waitFor(ChannelState.attached);
+
+            channel.setSuspended(new ErrorInfo("Set state to suspended", 400), true);
+            channelWaiter.waitFor(ChannelState.suspended);
+
+            /* Inject detached message as if from the server */
+            ProtocolMessage detachedMessage = new ProtocolMessage() {{
+                action = Action.detached;
+                channel = channelName;
+            }};
+            ably.connection.connectionManager.onMessage(null, detachedMessage);
+
+            /* Channel should transition to attaching, then to attached */
+            channelWaiter.waitFor(ChannelState.attaching);
+            channelWaiter.waitFor(ChannelState.attached);
+
+            List<ChannelState> channelStates = channelWaiter.getRecordedStates();
+            Assert.assertEquals(5, channelStates.size());
+            Assert.assertEquals(ChannelState.attaching, channelStates.get(0));
+            Assert.assertEquals(ChannelState.attached, channelStates.get(1));
+            Assert.assertEquals(ChannelState.suspended, channelStates.get(2));
+            Assert.assertEquals(ChannelState.attaching, channelStates.get(3));
+            Assert.assertEquals(ChannelState.attached, channelStates.get(4));
+
         } finally {
             if (ably != null)
                 ably.close();