Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-5147][CHA-RC3] Fix shared channel options for messages feature #75

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions chat-android/src/main/java/com/ably/chat/Messages.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package com.ably.chat

import com.ably.chat.QueryOptions.MessageOrder.NewestFirst
import com.google.gson.JsonObject
import io.ably.lib.realtime.AblyRealtime
import io.ably.lib.realtime.Channel
import io.ably.lib.realtime.ChannelState
import io.ably.lib.realtime.ChannelStateListener
import io.ably.lib.types.AblyException
Expand Down Expand Up @@ -211,27 +211,32 @@ internal class DefaultMessagesSubscription(
}

internal class DefaultMessages(
private val roomId: String,
private val realtimeChannels: AblyRealtime.Channels,
private val chatApi: ChatApi,
private val logger: Logger,
) : Messages, ContributesToRoomLifecycleImpl(logger) {
val room: DefaultRoom,
) : Messages, ContributesToRoomLifecycleImpl(room.roomLogger) {

override val featureName: String = "messages"

private var listeners: Map<Messages.Listener, DeferredValue<String>> = emptyMap()

private var channelStateListener: ChannelStateListener

private val logger = room.roomLogger.withContext(tag = "Messages")

private val roomId = room.roomId

private val chatApi = room.chatApi

private val realtimeChannels = room.realtimeClient.channels

private var lock = Any()

/**
* (CHA-M1)
* the channel name for the chat messages channel.
*/
private val messagesChannelName = "$roomId::\$chat::\$chatMessages"
private val messagesChannelName = "${room.roomId}::\$chat::\$chatMessages"

override val channel = realtimeChannels.get(messagesChannelName, ChatChannelOptions())
override val channel: Channel = realtimeChannels.get(messagesChannelName, room.options.messagesChannelOptions())

override val attachmentErrorCode: ErrorCode = ErrorCode.MessagesAttachmentFailed

Expand Down
14 changes: 1 addition & 13 deletions chat-android/src/main/java/com/ably/chat/Occupancy.kt
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,9 @@ internal class DefaultOccupancy(

override val detachmentErrorCode: ErrorCode = ErrorCode.OccupancyDetachmentFailed

private val realtimeChannels = room.realtimeClient.channels

private val logger = room.roomLogger.withContext(tag = "Occupancy")

// (CHA-O1)
private val messagesChannelName = "${room.roomId}::\$chat::\$chatMessages"

override val channel: Channel = realtimeChannels.get(
messagesChannelName,
ChatChannelOptions {
params = mapOf(
"occupancy" to "metrics",
)
},
)
override val channel: Channel = room.messages.channel

private val listeners: MutableList<Occupancy.Listener> = CopyOnWriteArrayList()

Expand Down
7 changes: 1 addition & 6 deletions chat-android/src/main/java/com/ably/chat/Room.kt
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,7 @@ internal class DefaultRoom(
private val roomScope =
CoroutineScope(Dispatchers.Default.limitedParallelism(1) + CoroutineName(roomId) + SupervisorJob())

override val messages = DefaultMessages(
roomId = roomId,
realtimeChannels = realtimeClient.channels,
chatApi = chatApi,
logger = roomLogger.withContext(tag = "Messages"),
)
override val messages = DefaultMessages(room = this)

private var _presence: Presence? = null
override val presence: Presence
Expand Down
31 changes: 30 additions & 1 deletion chat-android/src/main/java/com/ably/chat/RoomOptions.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.ably.chat

import io.ably.lib.types.ChannelMode
import io.ably.lib.types.ChannelOptions

/**
* Represents the options for a given chat room.
*/
Expand Down Expand Up @@ -89,10 +92,36 @@ object OccupancyOptions
* Throws AblyException for invalid room configuration.
* Spec: CHA-RC2a
*/
fun RoomOptions.validateRoomOptions() {
internal fun RoomOptions.validateRoomOptions() {
typing?.let {
if (typing.timeoutMs <= 0) {
throw ablyException("Typing timeout must be greater than 0", ErrorCode.InvalidRequestBody)
}
}
}

/**
* Merges channel options/modes from presence and occupancy to be used for shared channel.
* This channel is shared by Room messages, presence and occupancy feature.
* @return channelOptions for shared channel with options/modes from presence and occupancy.
* Spec: CHA-RC3
*/
internal fun RoomOptions.messagesChannelOptions(): ChannelOptions {
return ChatChannelOptions {
presence?.let {
val presenceModes = mutableListOf<ChannelMode>()
if (presence.enter) {
presenceModes.add(ChannelMode.presence)
}
if (presence.subscribe) {
presenceModes.add(ChannelMode.presence_subscribe)
}
modes = presenceModes.toTypedArray()
}
occupancy?.let {
params = mapOf(
"occupancy" to "metrics",
)
}
}
}
30 changes: 12 additions & 18 deletions chat-android/src/test/java/com/ably/chat/MessagesTest.kt
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
package com.ably.chat

import com.ably.chat.room.createMockLogger
import com.ably.chat.room.createMockChannel
import com.ably.chat.room.createMockChatApi
import com.ably.chat.room.createMockRealtimeClient
import com.ably.chat.room.createMockRoom
import com.google.gson.JsonObject
import io.ably.lib.realtime.AblyRealtime.Channels
import io.ably.lib.realtime.Channel
import io.ably.lib.realtime.ChannelBase
import io.ably.lib.realtime.ChannelState
import io.ably.lib.realtime.ChannelStateListener
import io.ably.lib.realtime.buildChannelStateChange
import io.ably.lib.realtime.buildRealtimeChannel
import io.ably.lib.types.AblyException
import io.ably.lib.types.MessageAction
import io.ably.lib.types.MessageExtras
import io.mockk.every
import io.mockk.mockk
import io.mockk.slot
import io.mockk.spyk
import io.mockk.verify
import java.lang.reflect.Field
import kotlinx.coroutines.runBlocking
Expand All @@ -27,29 +27,23 @@ import org.junit.Test

class MessagesTest {

private val realtimeClient = mockk<RealtimeClient>(relaxed = true)
private val realtimeChannels = mockk<Channels>(relaxed = true)
private val realtimeChannel = spyk<Channel>(buildRealtimeChannel())
private val chatApi = spyk(ChatApi(realtimeClient, "clientId", EmptyLogger(LogContext(tag = "TEST"))))
private lateinit var messages: DefaultMessages
private val logger = createMockLogger()
private val realtimeClient = createMockRealtimeClient()
private val realtimeChannel = realtimeClient.createMockChannel()

private lateinit var messages: DefaultMessages
private val channelStateListenerSlot = slot<ChannelStateListener>()

@Before
fun setUp() {
every { realtimeChannels.get(any(), any()) } returns realtimeChannel

every { realtimeClient.channels.get(any(), any()) } returns realtimeChannel
every { realtimeChannel.on(capture(channelStateListenerSlot)) } answers {
println("Channel state listener registered")
}

messages = DefaultMessages(
roomId = "room1",
realtimeChannels = realtimeChannels,
chatApi = chatApi,
logger,
)
val chatApi = createMockChatApi(realtimeClient)
val room = createMockRoom("room1", realtimeClient = realtimeClient, chatApi = chatApi)

messages = DefaultMessages(room)
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.ably.chat.room

import io.ably.lib.realtime.buildRealtimeChannel
import io.ably.lib.types.ChannelMode
import io.ably.lib.types.ChannelOptions
import io.mockk.every
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import org.junit.Assert
import org.junit.Test

/**
* Test to check shared channel for room features.
* Spec: CHA-RC3
*/
class RoomFeatureSharedChannelTest {

@Test
fun `(CHA-RC3a) Features with shared channel should call channels#get only once with combined modes+options`() = runTest {
val mockRealtimeClient = createMockRealtimeClient()
val capturedChannelOptions = mutableListOf<ChannelOptions>()

every {
mockRealtimeClient.channels.get("1234::\$chat::\$chatMessages", any<ChannelOptions>())
} answers {
capturedChannelOptions.add(secondArg())
buildRealtimeChannel()
}

// Create room with all feature enabled,
val room = createMockRoom(realtimeClient = mockRealtimeClient)

// Messages, occupancy and presence features uses the same channel
Assert.assertEquals(room.messages.channel, room.presence.channel)
Assert.assertEquals(room.messages.channel, room.occupancy.channel)

// Reactions and typing uses independent channel
Assert.assertNotEquals(room.messages.channel, room.typing.channel)
Assert.assertNotEquals(room.messages.channel, room.reactions.channel)
Assert.assertNotEquals(room.reactions.channel, room.typing.channel)

Assert.assertEquals(1, capturedChannelOptions.size)
// Check for set presence modes
Assert.assertEquals(2, capturedChannelOptions[0].modes.size)
Assert.assertEquals(ChannelMode.presence, capturedChannelOptions[0].modes[0])
Assert.assertEquals(ChannelMode.presence_subscribe, capturedChannelOptions[0].modes[1])
sacOO7 marked this conversation as resolved.
Show resolved Hide resolved
// Check if occupancy matrix is set
Assert.assertEquals("metrics", capturedChannelOptions[0].params["occupancy"])

// channels.get is called only once for Messages, occupancy and presence since they share the same channel
verify(exactly = 1) {
mockRealtimeClient.channels.get("1234::\$chat::\$chatMessages", any<ChannelOptions>())
}
// channels.get called separately for typing since it uses it's own channel
verify(exactly = 1) {
mockRealtimeClient.channels.get("1234::\$chat::\$typingIndicators", any<ChannelOptions>())
}
// channels.get called separately for reactions since it uses it's own channel
verify(exactly = 1) {
mockRealtimeClient.channels.get("1234::\$chat::\$reactions", any<ChannelOptions>())
}
// channels.get is called thrice for all features
verify(exactly = 3) {
mockRealtimeClient.channels.get(any<String>(), any<ChannelOptions>())
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ fun createRoomFeatureMocks(roomId: String = DEFAULT_ROOM_ID, clientId: String =
val logger = createMockLogger()
val room = createMockRoom(roomId, clientId, realtimeClient, chatApi, logger)

val messagesContributor = spyk(DefaultMessages(roomId, realtimeClient.channels, chatApi, logger), recordPrivateCalls = true)
val messagesContributor = spyk(DefaultMessages(room), recordPrivateCalls = true)
val presenceContributor = spyk(DefaultPresence(room), recordPrivateCalls = true)
val occupancyContributor = spyk(DefaultOccupancy(room), recordPrivateCalls = true)
val typingContributor = spyk(DefaultTyping(room), recordPrivateCalls = true)
Expand Down