From 0ffd50cbd48c20b7236f1183eb69d71a7d42cc86 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Thu, 12 Dec 2024 18:27:29 +0530 Subject: [PATCH 1/3] [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 --- .../io/ably/lib/realtime/ChannelBase.java | 18 ++--- .../test/realtime/RealtimeChannelTest.java | 71 ++++++++++++++++++- 2 files changed, 74 insertions(+), 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 index 9e0c9974c..c59b399e4 100644 --- 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 index d60f7179b..b6c979e5a 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 @@ -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 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 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(); From a6bbcb23c84b3c2a577a6aaea59944a51beee01e Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 17 Dec 2024 19:46:19 +0530 Subject: [PATCH 2/3] [ECO-5117][RTL5] Fix missing spec implementation for channel detach (RTL5g) 1. Added missing callCompletionListenerError when detachImpl throws exception on invalid connection state 2. Added separate test case for the spec RTL5g 3. Annotated channel detach tests with appropriate spec --- .../io/ably/lib/realtime/ChannelBase.java | 4 +- .../test/realtime/RealtimeChannelTest.java | 84 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) 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 c59b399e4..cf0345108 100644 --- a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java +++ b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java @@ -350,8 +350,9 @@ private void detachImpl(CompletionListener listener) throws AblyException { default: } ConnectionManager connectionManager = ably.connection.connectionManager; - if(!connectionManager.isActive()) + if(!connectionManager.isActive()) { // RTL5g throw AblyException.fromErrorInfo(connectionManager.getStateErrorInfo()); + } sendDetachMessage(listener); } @@ -609,6 +610,7 @@ public void onError(ErrorInfo reason) { detachImpl(completionListener); } catch (AblyException e) { attachTimer = null; + callCompletionListenerError(listener, e.errorInfo); // RTL5g } if(attachTimer == null) { 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 b6c979e5a..c5822cb13 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 @@ -945,6 +945,9 @@ public void attach_success_callback() { } } + /** + * Spec: RTL4g + */ @Test public void attach_success_callback_for_channel_in_failed_state() { AblyRealtime ably = null; @@ -1037,6 +1040,7 @@ public void attach_fail_callback() { /** * When client detaches from a channel successfully after initialized state, * verify attach {@code CompletionListener#onSuccess()} gets called. + * Spec: RTL5a */ @Test public void detach_success_callback_initialized() { @@ -1069,6 +1073,9 @@ public void detach_success_callback_initialized() { } } + /** + * Spec: RTL5j + */ @Test public void detach_success_callback_on_suspended_state() { AblyRealtime ably = null; @@ -1106,6 +1113,9 @@ public void detach_success_callback_on_suspended_state() { } } + /** + * Spec: RTL5b + */ @Test public void detach_failure_callback_on_failed_state() { AblyRealtime ably = null; @@ -1147,6 +1157,79 @@ public void detach_failure_callback_on_failed_state() { } } + /** + * When connection is in failed or suspended, set error in callback + * Spec: RTL5g + */ + @Test + public void detach_fail_callback_for_connection_invalid_state() { + AblyRealtime ably = null; + try { + ClientOptions opts = createOptions(testVars.keys[0].keyStr); + ably = new AblyRealtime(opts); + ConnectionWaiter connWaiter = new ConnectionWaiter(ably.connection); + + /* wait until connected */ + connWaiter.waitFor(ConnectionState.connected); + + /* create a channel and attach */ + final Channel channel = ably.channels.get("detach_failure"); + ChannelWaiter channelWaiter = new ChannelWaiter(channel); + channel.attach(); + channelWaiter.waitFor(ChannelState.attached); + + // Simulate connection closing from outside + ably.connection.connectionManager.requestState(new ConnectionManager.StateIndication( + ConnectionState.closing, + new ErrorInfo("Connection is closing", 80001) + )); + /* wait until connection closing */ + connWaiter.waitFor(ConnectionState.closing); + + // channel state is ATTACHED despite closing connection state + assertEquals(ChannelState.attached, channel.state); + + /* detach */ + Helpers.CompletionWaiter detachWaiter1 = new Helpers.CompletionWaiter(); + channel.detach(detachWaiter1); + + /* Verify onSuccess callback gets called */ + detachWaiter1.waitFor(); + assertFalse(detachWaiter1.success); + assertNotNull(detachWaiter1.error); + assertEquals("Connection is closing", detachWaiter1.error.message); + assertEquals(80001, detachWaiter1.error.code); + + // Simulate connection failure + ably.connection.connectionManager.requestState(ConnectionState.failed); + /* wait until connection failed */ + connWaiter.waitFor(ConnectionState.failed); + + // Mock channel state to ATTACHED despite failed connection state + channelWaiter.waitFor(ChannelState.failed); + channel.state = ChannelState.attached; + assertEquals(ChannelState.attached, channel.state); + + /* detach */ + Helpers.CompletionWaiter detachWaiter2 = new Helpers.CompletionWaiter(); + channel.detach(detachWaiter2); + + /* Verify onSuccess callback gets called */ + detachWaiter2.waitFor(); + assertFalse(detachWaiter2.success); + assertNotNull(detachWaiter2.error); + assertEquals("Connection failed", detachWaiter2.error.message); + assertEquals(80000, detachWaiter2.error.code); + + } catch (AblyException e) { + e.printStackTrace(); + fail("init0: Unexpected exception instantiating library"); + } finally { + if(ably != null) + ably.close(); + } + } + /** * When client detaches from a channel successfully after attached state, * verify attach {@code CompletionListener#onSuccess()} gets called. @@ -1184,6 +1267,7 @@ public void detach_success_callback_attached() throws AblyException { /** * When client detaches from a channel successfully after detaching state, * verify attach {@code CompletionListener#onSuccess()} gets called. + * Spec: RTL5i */ @Test public void detach_success_callback_detaching() throws AblyException { From 8eec5b573016e6fa6587d45f2bbecb6e0f5606c3 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 10 Jan 2025 17:45:41 +0530 Subject: [PATCH 3/3] [ECO-5117][RTL13a] Fixed channel ATTACHING event detach err 1. Updated `attachWithTimeout` and `attachImpl` method to accept attachReason param 2. Updated test assertions for spec RTL13a accordingly --- .../java/io/ably/lib/realtime/ChannelBase.java | 16 ++++++++-------- .../lib/test/realtime/RealtimeChannelTest.java | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 10 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 cf0345108..6e3773eb0 100644 --- a/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java +++ b/lib/src/main/java/io/ably/lib/realtime/ChannelBase.java @@ -194,7 +194,7 @@ public void attach(CompletionListener listener) throws AblyException { void attach(boolean forceReattach, CompletionListener listener) { clearAttachTimers(); - attachWithTimeout(forceReattach, listener); + attachWithTimeout(forceReattach, listener, null); } /** @@ -217,7 +217,7 @@ synchronized void transferQueuedPresenceMessages(List messagesToT private boolean attachResume; - private void attachImpl(final boolean forceReattach, final CompletionListener listener) throws AblyException { + private void attachImpl(final boolean forceReattach, final CompletionListener listener, ErrorInfo reattachmentReason) throws AblyException { Log.v(TAG, "attach(); channel = " + name); if(!forceReattach) { /* check preconditions */ @@ -249,7 +249,7 @@ private void attachImpl(final boolean forceReattach, final CompletionListener li if (listener != null) { on(new ChannelStateCompletionListener(listener, ChannelState.attached, ChannelState.failed)); } - setState(ChannelState.attaching, null); + setState(ChannelState.attaching, reattachmentReason); return; } @@ -277,7 +277,7 @@ private void attachImpl(final boolean forceReattach, final CompletionListener li attachMessage.setFlag(Flag.attach_resume); } - setState(ChannelState.attaching, null); + setState(ChannelState.attaching, reattachmentReason); connectionManager.send(attachMessage, true, null); } catch(AblyException e) { throw e; @@ -470,14 +470,14 @@ synchronized private void clearAttachTimers() { } private void attachWithTimeout(final CompletionListener listener) throws AblyException { - this.attachWithTimeout(false, listener); + this.attachWithTimeout(false, listener, null); } /** * Attach channel, if not attached within timeout set state to suspended and * set up timer to reattach it later */ - synchronized private void attachWithTimeout(final boolean forceReattach, final CompletionListener listener) { + synchronized private void attachWithTimeout(final boolean forceReattach, final CompletionListener listener, ErrorInfo reattachmentReason) { checkChannelIsNotReleased(); Timer currentAttachTimer; try { @@ -502,7 +502,7 @@ public void onError(ErrorInfo reason) { clearAttachTimers(); callCompletionListenerError(listener, reason); } - }); + }, reattachmentReason); } catch(AblyException e) { attachTimer = null; callCompletionListenerError(listener, e.errorInfo); @@ -1303,7 +1303,7 @@ void onChannelMessage(ProtocolMessage msg) { 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)); - attachWithTimeout(true, null); + attachWithTimeout(true, null, msg.error); 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 index c5822cb13..e6c8b8a6a 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 @@ -1813,11 +1813,16 @@ public void server_initiated_detach_for_attached_channel() throws AblyException ProtocolMessage detachedMessage = new ProtocolMessage() {{ action = Action.detached; channel = channelName; + error = new ErrorInfo("Simulated detach", 40000); }}; ably.connection.connectionManager.onMessage(null, detachedMessage); /* Channel should transition to attaching, then to attached */ - channelWaiter.waitFor(ChannelState.attaching); + ErrorInfo detachErr = channelWaiter.waitFor(ChannelState.attaching); + Assert.assertNotNull(detachErr); + Assert.assertEquals(40000, detachErr.code); + Assert.assertEquals("Simulated detach", detachErr.message); + channelWaiter.waitFor(ChannelState.attached); List channelStates = channelWaiter.getRecordedStates(); @@ -1869,11 +1874,16 @@ public void server_initiated_detach_for_suspended_channel() throws AblyException ProtocolMessage detachedMessage = new ProtocolMessage() {{ action = Action.detached; channel = channelName; + error = new ErrorInfo("Simulated detach", 40000); }}; ably.connection.connectionManager.onMessage(null, detachedMessage); /* Channel should transition to attaching, then to attached */ - channelWaiter.waitFor(ChannelState.attaching); + ErrorInfo detachError = channelWaiter.waitFor(ChannelState.attaching); + Assert.assertNotNull(detachError); + Assert.assertEquals(40000, detachError.code); + Assert.assertEquals("Simulated detach", detachError.message); + channelWaiter.waitFor(ChannelState.attached); List channelStates = channelWaiter.getRecordedStates();