[ECO-4996] chore: added spec adjustments

Reviewed sending and receiving message implementation and make sure all spec points are covered

Author: ttypic

chat-android/src/main/java/com/ably/chat/ChatApi.kt

@@ -12,6 +12,7 @@ import kotlin.coroutines.suspendCoroutine
 
 private const val API_PROTOCOL_VERSION = 3
 private const val PROTOCOL_VERSION_PARAM_NAME = "v"
+private const val RESERVED_ABLY_CHAT_KEY = "ably-chat"
 private val apiProtocolParam = Param(PROTOCOL_VERSION_PARAM_NAME, API_PROTOCOL_VERSION.toString())
 
 internal class ChatApi(private val realtimeClient: RealtimeClient, private val clientId: String) {
@@ -47,11 +48,15 @@ internal class ChatApi(private val realtimeClient: RealtimeClient, private val c
      * @return sent message instance
      */
     suspend fun sendMessage(roomId: String, params: SendMessageParams): Message {
+        validateSendMessageParams(params)
+
         val body = JsonObject().apply {
             addProperty("text", params.text)
+            // (CHA-M3b)
             params.headers?.let {
                 add("headers", it.toJson())
             }
+            // (CHA-M3b)
             params.metadata?.let {
                 add("metadata", it.toJson())
             }
@@ -62,6 +67,7 @@ internal class ChatApi(private val realtimeClient: RealtimeClient, private val c
             "POST",
             body,
         )?.let {
+            // (CHA-M3a)
             Message(
                 timeserial = it.requireString("timeserial"),
                 clientId = clientId,
@@ -74,6 +80,30 @@ internal class ChatApi(private val realtimeClient: RealtimeClient, private val c
         } ?: throw AblyException.fromErrorInfo(ErrorInfo("Send message endpoint returned empty value", HttpStatusCodes.InternalServerError))
     }
 
+    private fun validateSendMessageParams(params: SendMessageParams) {
+        // (CHA-M3c)
+        if (params.metadata?.containsKey(RESERVED_ABLY_CHAT_KEY) == true) {
+            throw AblyException.fromErrorInfo(
+                ErrorInfo(
+                    "Metadata contains reserved 'ably-chat' key",
+                    HttpStatusCodes.BadRequest,
+                    ErrorCodes.InvalidRequestBody,
+                ),
+            )
+        }
+
+        // (CHA-M3d)
+        if (params.headers?.keys?.any { it.startsWith(RESERVED_ABLY_CHAT_KEY) } == true) {
+            throw AblyException.fromErrorInfo(
+                ErrorInfo(
+                    "Headers contains reserved key with reserved 'ably-chat' prefix",
+                    HttpStatusCodes.BadRequest,
+                    ErrorCodes.InvalidRequestBody,
+                ),
+            )
+        }
+    }
+
     /**
      * return occupancy for specified room
      */
@@ -104,6 +134,7 @@ internal class ChatApi(private val realtimeClient: RealtimeClient, private val c
                 }
 
                 override fun onError(reason: ErrorInfo?) {
+                    // (CHA-M3e)
                     continuation.resumeWithException(AblyException.fromErrorInfo(reason))
                 }
             },

chat-android/src/main/java/com/ably/chat/ErrorCodes.kt

@@ -92,6 +92,16 @@ object ErrorCodes {
      * The request cannot be understood
      */
     const val BadRequest = 40_000
+
+    /**
+     * Invalid request body
+     */
+    const val InvalidRequestBody = 40_001
+
+    /**
+     * Internal error
+     */
+    const val InternalError = 50_000
 }
 
 /**

chat-android/src/main/java/com/ably/chat/Message.kt

@@ -68,3 +68,21 @@ data class Message(
      */
     val headers: MessageHeaders,
 )
+
+/**
+ * (CHA-M2a)
+ * @return true if the timeserial of the corresponding realtime channel message comes first.
+ */
+fun Message.isBefore(other: Message): Boolean = Timeserial.parse(timeserial) < Timeserial.parse(other.timeserial)
+
+/**
+ * (CHA-M2b)
+ * @return true if the timeserial of the corresponding realtime channel message comes second.
+ */
+fun Message.isAfter(other: Message): Boolean = Timeserial.parse(timeserial) > Timeserial.parse(other.timeserial)
+
+/**
+ * (CHA-M2c)
+ * @return true if they have the same timeserial.
+ */
+fun Message.isAtTheSameTime(other: Message): Boolean = Timeserial.parse(timeserial) == Timeserial.parse(other.timeserial)

chat-android/src/main/java/com/ably/chat/Messages.kt

@@ -180,6 +180,11 @@ data class SendMessageParams(
 )
 
 interface MessagesSubscription : Subscription {
+    /**
+     * (CHA-M5j)
+     * Get the previous messages that were sent to the room before the listener was subscribed.
+     * @return paginated result of messages, in newest-to-oldest order.
+     */
     suspend fun getPreviousMessages(start: Long? = null, end: Long? = null, limit: Int = 100): PaginatedResult<Message>
 }
 
@@ -195,6 +200,18 @@ internal class DefaultMessagesSubscription(
 
     override suspend fun getPreviousMessages(start: Long?, end: Long?, limit: Int): PaginatedResult<Message> {
         val fromSerial = fromSerialProvider().await()
+
+        // (CHA-M5j)
+        if (end != null && end > Timeserial.parse(fromSerial).timestamp) {
+            throw AblyException.fromErrorInfo(
+                ErrorInfo(
+                    "The `end` parameter is specified and is more recent than the subscription point timeserial",
+                    HttpStatusCodes.BadRequest,
+                    ErrorCodes.BadRequest,
+                ),
+            )
+        }
+
         val queryOptions = QueryOptions(start = start, end = end, limit = limit, orderBy = NewestFirst)
         return chatApi.getMessages(
             roomId = roomId,
@@ -217,6 +234,7 @@ internal class DefaultMessages(
     private var lock = Any()
 
     /**
+     * (CHA-M1)
      * the channel name for the chat messages channel.
      */
     private val messagesChannelName = "$roomId::\$chat::\$chatMessages"
@@ -249,8 +267,9 @@ internal class DefaultMessages(
             )
             listener.onEvent(MessageEvent(type = MessageEventType.Created, message = chatMessage))
         }
-
+        // (CHA-M4d)
         channel.subscribe(MessageEventType.Created.eventName, messageListener)
+        // (CHA-M5) setting subscription point
         associateWithCurrentChannelSerial(deferredChannelSerial)
 
         return DefaultMessagesSubscription(
@@ -293,10 +312,11 @@ internal class DefaultMessages(
     private fun associateWithCurrentChannelSerial(channelSerialProvider: DeferredValue<String>) {
         if (channel.state === ChannelState.attached) {
             channelSerialProvider.completeWith(requireChannelSerial())
+            return
         }
 
         channel.once(ChannelState.attached) {
-            channelSerialProvider.completeWith(requireChannelSerial())
+            channelSerialProvider.completeWith(requireAttachSerial())
         }
     }
 
@@ -307,6 +327,13 @@ internal class DefaultMessages(
             )
     }
 
+    private fun requireAttachSerial(): String {
+        return channel.properties.attachSerial
+            ?: throw AblyException.fromErrorInfo(
+                ErrorInfo("Channel has been attached, but attachSerial is not defined", HttpStatusCodes.BadRequest, ErrorCodes.BadRequest),
+            )
+    }
+
     private fun addListener(listener: Messages.Listener, deferredChannelSerial: DeferredValue<String>) {
         synchronized(lock) {
             listeners += listener to deferredChannelSerial
@@ -319,9 +346,12 @@ internal class DefaultMessages(
         }
     }
 
+    /**
+     * (CHA-M5c), (CHA-M5d)
+     */
     private fun updateChannelSerialsAfterDiscontinuity() {
         val deferredChannelSerial = DeferredValue<String>()
-        associateWithCurrentChannelSerial(deferredChannelSerial)
+        deferredChannelSerial.completeWith(requireAttachSerial())
 
         synchronized(lock) {
             listeners = listeners.mapValues {

chat-android/src/main/java/com/ably/chat/Room.kt

@@ -52,6 +52,7 @@ interface Room {
     val occupancy: Occupancy
 
     /**
+     * (CHA-RS2)
      * Returns an object that can be used to observe the status of the room.
      *
      * @returns The status observable.

chat-android/src/main/java/com/ably/chat/RoomOptions.kt

@@ -9,25 +9,25 @@ data class RoomOptions(
      * use {@link RoomOptionsDefaults.presence} to enable presence with default options.
      * @defaultValue undefined
      */
-    val presence: PresenceOptions = PresenceOptions(),
+    val presence: PresenceOptions? = null,
 
     /**
      * The typing options for the room. To enable typing in the room, set this property. You may use
      * {@link RoomOptionsDefaults.typing} to enable typing with default options.
      */
-    val typing: TypingOptions = TypingOptions(),
+    val typing: TypingOptions? = null,
 
     /**
      * The reactions options for the room. To enable reactions in the room, set this property. You may use
      * {@link RoomOptionsDefaults.reactions} to enable reactions with default options.
      */
-    val reactions: RoomReactionsOptions = RoomReactionsOptions,
+    val reactions: RoomReactionsOptions? = null,
 
     /**
      * The occupancy options for the room. To enable occupancy in the room, set this property. You may use
      * {@link RoomOptionsDefaults.occupancy} to enable occupancy with default options.
      */
-    val occupancy: OccupancyOptions = OccupancyOptions,
+    val occupancy: OccupancyOptions? = null,
 )
 
 /**

chat-android/src/main/java/com/ably/chat/RoomStatus.kt

@@ -7,11 +7,13 @@ import io.ably.lib.types.ErrorInfo
  */
 interface RoomStatus {
     /**
+     * (CHA-RS2a)
      * The current status of the room.
      */
     val current: RoomLifecycle
 
     /**
+     * (CHA-RS2b)
      * The current error, if any, that caused the room to enter the current status.
      */
     val error: ErrorInfo?
@@ -36,57 +38,68 @@ interface RoomStatus {
 }
 
 /**
+ * (CHA-RS1)
  * The different states that a room can be in throughout its lifecycle.
  */
 enum class RoomLifecycle(val stateName: String) {
     /**
+     * (CHA-RS1a)
      * A temporary state for when the library is first initialized.
      */
     Initialized("initialized"),
 
     /**
+     * (CHA-RS1b)
      * The library is currently attempting to attach the room.
      */
     Attaching("attaching"),
 
     /**
+     * (CHA-RS1c)
      * The room is currently attached and receiving events.
      */
     Attached("attached"),
 
     /**
+     * (CHA-RS1d)
      * The room is currently detaching and will not receive events.
      */
     Detaching("detaching"),
 
     /**
+     * (CHA-RS1e)
      * The room is currently detached and will not receive events.
      */
     Detached("detached"),
 
     /**
+     * (CHA-RS1f)
      * The room is in an extended state of detachment, but will attempt to re-attach when able.
      */
     Suspended("suspended"),
 
     /**
+     * (CHA-RS1g)
      * The room is currently detached and will not attempt to re-attach. User intervention is required.
      */
     Failed("failed"),
 
     /**
+     * (CHA-RS1h)
      * The room is in the process of releasing. Attempting to use a room in this state may result in undefined behavior.
      */
     Releasing("releasing"),
 
     /**
+     * (CHA-RS1i)
      * The room has been released and is no longer usable.
      */
     Released("released"),
 }
 
 /**
  * Represents a change in the status of the room.
+ * (CHA-RS4)
  */
 data class RoomStatusChange(
     /**

chat-android/src/main/java/com/ably/chat/Timeserial.kt

@@ -0,0 +1,65 @@
+package com.ably.chat
+
+import io.ably.lib.types.AblyException
+import io.ably.lib.types.ErrorInfo
+
+/**
+ * Represents a parsed timeserial.
+ */
+data class Timeserial(
+    /**
+     * The series ID of the timeserial.
+     */
+    val seriesId: String,
+
+    /**
+     * The timestamp of the timeserial.
+     */
+    val timestamp: Long,
+
+    /**
+     * The counter of the timeserial.
+     */
+    val counter: Int,
+
+    /**
+     * The index of the timeserial.
+     */
+    val index: Int?,
+) : Comparable<Timeserial> {
+    @Suppress("ReturnCount")
+    override fun compareTo(other: Timeserial): Int {
+        val timestampDiff = timestamp.compareTo(other.timestamp)
+        if (timestampDiff != 0) return timestampDiff
+
+        // Compare the counter
+        val counterDiff = counter.compareTo(other.counter)
+        if (counterDiff != 0) return counterDiff
+
+        // Compare the seriesId lexicographically
+        val seriesIdDiff = seriesId.compareTo(other.seriesId)
+        if (seriesIdDiff != 0) return seriesIdDiff
+
+        // Compare the index, if present
+        return if (index != null && other.index != null) index.compareTo(other.index) else 0
+    }
+
+    companion object {
+        @Suppress("DestructuringDeclarationWithTooManyEntries")
+        fun parse(timeserial: String): Timeserial {
+            val matched = """(\w+)@(\d+)-(\d+)(?::(\d+))?""".toRegex().matchEntire(timeserial)
+                ?: throw AblyException.fromErrorInfo(
+                    ErrorInfo("invalid timeserial", HttpStatusCodes.InternalServerError, ErrorCodes.InternalError),
+                )
+
+            val (seriesId, timestamp, counter, index) = matched.destructured
+
+            return Timeserial(
+                seriesId = seriesId,
+                timestamp = timestamp.toLong(),
+                counter = counter.toInt(),
+                index = if (index.isNotBlank()) index.toInt() else null,
+            )
+        }
+    }
+}

chat-android/src/main/java/com/ably/chat/Utils.kt

@@ -42,6 +42,8 @@ fun ChatChannelOptions(init: (ChannelOptions.() -> Unit)? = null): ChannelOption
     options.params = (options.params ?: mapOf()) + mapOf(
         AGENT_PARAMETER_NAME to "chat-kotlin/${BuildConfig.APP_VERSION}",
     )
+    // (CHA-M4a)
+    options.attachOnSubscribe = false
     return options
 }