Deprecate releasing a realtime channel that isn't detached (RTS4b)

lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java

@@ -178,7 +178,8 @@ public interface Channels extends ReadOnlyMap<String, Channel> {
         /**
          * Releases a {@link Channel} object, deleting it, and enabling it to be garbage collected.
          * It also removes any listeners associated with the channel.
-         * To release a channel, the {@link ChannelState} must be INITIALIZED, DETACHED, or FAILED.
+         * A realtime channel should only be released when it is in the INITIALIZED, DETACHED, or FAILED state;
+         * releasing a realtime channel in any other state is deprecated and will throw an error in the next major version.
          * <p>
          * Spec: RSN4, RTS4
          * @param channelName The channel name.
@@ -223,9 +224,22 @@ public Channel get(final String channelName, final ChannelOptions channelOptions
 
         @Override
         public void release(String channelName) {
-            Channel channel = map.remove(channelName);
+            Channel channel = map.get(channelName);
             if(channel != null) {
-                channel.markAsReleased();
+                ChannelState state;
+                // The channel's state transitions hold its monitor, so holding it here stops the channel
+                // from starting to attach between the state check and it being marked as released.
+                synchronized (channel) {
+                    if (!map.remove(channelName, channel)) {
+                        return;
+                    }
+                    state = channel.state;
+                    channel.markAsReleased();
+                }
+                if (state != ChannelState.initialized && state != ChannelState.detached && state != ChannelState.failed) {
+                    // RTS4b
+                    Log.w(TAG, "Calling `channels.release()` on a channel in the " + state + " state is deprecated, and will throw an error in the next major version. Call `channel.detach()` and wait for it to complete before calling `channels.release(channelName)`.");
+                }
                 try {
                     channel.detach();
                 } catch (AblyException e) {

lib/src/test/kotlin/io/ably/lib/uts/unit/realtime/ChannelsCollectionTest.kt

@@ -0,0 +1,121 @@
+package io.ably.lib.uts.unit.realtime
+
+import io.ably.lib.uts.infra.unit.*
+import io.ably.lib.realtime.ChannelState
+import io.ably.lib.realtime.ConnectionState
+import io.ably.lib.types.ProtocolMessage
+import io.ably.lib.util.Log
+import io.ably.lib.uts.infra.awaitChannelState
+import io.ably.lib.uts.infra.awaitState
+import kotlinx.coroutines.test.runTest
+import java.util.UUID
+import java.util.concurrent.CopyOnWriteArrayList
+import kotlin.test.*
+
+class ChannelsCollectionTest {
+
+  /**
+   * @UTS realtime/unit/RTS4c/release-nonexistent-noop-0
+   */
+  @Test
+  fun `RTS4c - release on non-existent channel is no-op`() {
+    val channelName = "test-RTS4c-nonexistent-${UUID.randomUUID()}"
+    val client = TestRealtimeClient { autoConnect = false }
+
+    client.channels.release(channelName)
+
+    assertFalse(client.channels.containsKey(channelName))
+    client.close()
+  }
+
+  /**
+   * @UTS realtime/unit/RTS4d/release-removes-channel-0
+   */
+  @Test
+  fun `RTS4d - release removes an initialized channel`() {
+    val channelName = "test-RTS4d-${UUID.randomUUID()}"
+    val client = TestRealtimeClient { autoConnect = false }
+
+    val channel = client.channels.get(channelName)
+    assertEquals(ChannelState.initialized, channel.state)
+    assertTrue(client.channels.containsKey(channelName))
+
+    client.channels.release(channelName)
+
+    assertFalse(client.channels.containsKey(channelName))
+    client.close()
+  }
+
+  /**
+   * @UTS realtime/unit/RTS4d/release-after-detach-1
+   */
+  @Test
+  fun `RTS4d - release removes a channel once detached`() = runTest {
+    val channelName = "test-RTS4d-detached-${UUID.randomUUID()}"
+    val warnings = CopyOnWriteArrayList<String>()
+    val client = TestRealtimeClient {
+      autoConnect = false
+      install(attachDetachMock())
+      logHandler = Log.LogHandler { severity, _, msg, _ -> if (severity == Log.WARN) warnings.add(msg) }
+    }
+    val channel = client.channels.get(channelName)
+
+    client.connect()
+    awaitState(client, ConnectionState.connected)
+
+    channel.attach()
+    awaitChannelState(channel, ChannelState.attached)
+    channel.detach()
+    awaitChannelState(channel, ChannelState.detached)
+
+    client.channels.release(channelName)
+
+    assertFalse(client.channels.containsKey(channelName))
+    assertTrue(warnings.none { it.contains("channels.release()") })
+    client.close()
+  }
+
+  @Test
+  fun `RTS4b - release of an attached channel logs a deprecation warning and still releases it`() = runTest {
+    val channelName = "test-RTS4b-attached-${UUID.randomUUID()}"
+    val warnings = CopyOnWriteArrayList<String>()
+    val client = TestRealtimeClient {
+      autoConnect = false
+      install(attachDetachMock())
+      logHandler = Log.LogHandler { severity, _, msg, _ -> if (severity == Log.WARN) warnings.add(msg) }
+    }
+    val channel = client.channels.get(channelName)
+
+    client.connect()
+    awaitState(client, ConnectionState.connected)
+
+    channel.attach()
+    awaitChannelState(channel, ChannelState.attached)
+
+    client.channels.release(channelName)
+
+    assertFalse(client.channels.containsKey(channelName))
+    assertEquals(ChannelState.detached, channel.state)
+    val deprecationWarnings = warnings.filter { it.contains("channels.release()") }
+    assertEquals(1, deprecationWarnings.size)
+    assertContains(deprecationWarnings[0], "attached state is deprecated")
+    client.close()
+  }
+
+  private fun attachDetachMock(): MockWebSocket {
+    lateinit var mockWs: MockWebSocket
+    mockWs = MockWebSocket {
+      onConnectionAttempt = { conn -> conn.respondWithSuccess(CONNECTED_MESSAGE) }
+      onMessageFromClient = { msg ->
+        when (msg.action) {
+          ProtocolMessage.Action.attach ->
+            mockWs.sendToClient(ProtocolMessage(ProtocolMessage.Action.attached, msg.channel))
+          ProtocolMessage.Action.detach ->
+            mockWs.sendToClient(ProtocolMessage(ProtocolMessage.Action.detached, msg.channel))
+          else -> Unit
+        }
+      }
+    }
+    return mockWs
+  }
+}

pubsub-adapter/src/main/kotlin/com/ably/pubsub/Channels.kt

@@ -39,7 +39,8 @@ public interface Channels<ChannelType> : Iterable<ChannelType> {
   /**
    * Releases a [Channel] object, deleting it, and enabling it to be garbage collected.
    * It also removes any listeners associated with the channel.
-   * To release a channel, the [ChannelState] must be `INITIALIZED`, `DETACHED`, or `FAILED`.
+   * A realtime channel should only be released when it is in the `INITIALIZED`, `DETACHED`, or `FAILED` state;
+   * releasing a realtime channel in any other state is deprecated and will throw an error in the next major version.
    * <p>
    * Spec: RSN4, RTS4
    * @param name The channel name.