Bugfix: Fix manual refresh for post updates (#146)

ls1intum · Nov 29, 2024 · 4a8671b · 4a8671b
1 parent 4f112a4
commit 4a8671b
Show file tree

Hide file tree

Showing 15 changed files with 119 additions and 59 deletions.
diff --git a/...main/kotlin/de/tum/informatics/www1/artemis/native_app/core/test/TestWebsocketProvider.kt b/...main/kotlin/de/tum/informatics/www1/artemis/native_app/core/test/TestWebsocketProvider.kt
@@ -21,12 +21,12 @@ class TestWebsocketProvider : WebsocketProvider {
     override val isConnected: Flow<Boolean> = flowOf(true)
 
     override fun <T : Any> subscribe(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<WebsocketProvider.WebsocketData<T>> = flowOf(WebsocketProvider.WebsocketData.Subscribe())
 
     override fun <T : Any> subscribeMessage(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<T> = emptyFlow()
 

diff --git a/...kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/WebsocketProviderStub.kt b/...kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/WebsocketProviderStub.kt
@@ -15,12 +15,12 @@ class WebsocketProviderStub : WebsocketProvider {
     override val isConnected: Flow<Boolean> = flowOf(true)
 
     override fun <T : Any> subscribe(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<WebsocketProvider.WebsocketData<T>> = emptyFlow()
 
     override fun <T : Any> subscribeMessage(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<T> = emptyFlow()
 

diff --git a/...ain/kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/WebsocketProvider.kt b/...ain/kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/WebsocketProvider.kt
@@ -19,12 +19,12 @@ interface WebsocketProvider {
      * Performs automatic reconnects.
      */
     fun <T : Any> subscribe(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<WebsocketData<T>>
 
     fun <T : Any> subscribeMessage(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<T>
 

diff --git a/...n/de/tum/informatics/www1/artemis/native_app/core/websocket/impl/WebsocketProviderImpl.kt b/...n/de/tum/informatics/www1/artemis/native_app/core/websocket/impl/WebsocketProviderImpl.kt
@@ -199,28 +199,28 @@ class WebsocketProviderImpl(
      * The given flow can only be subscribed to once.
      */
     override fun <T : Any> subscribe(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<WebsocketProvider.WebsocketData<T>> {
         return session
             .transformLatest { currentSession ->
                 val flow: Flow<WebsocketProvider.WebsocketData<T>> = flow {
                     emitAll(
                         currentSession.subscribe(
-                            StompSubscribeHeaders(destination = channel),
+                            StompSubscribeHeaders(destination = topic),
                             deserializer
                         )
                     )
                 }
                     .onStart {
-                        Log.d(TAG, "subscribe! $channel")
+                        Log.d(TAG, "subscribe! $topic")
                         emit(WebsocketProvider.WebsocketData.Subscribe())
                     }
                     .onCompletion {
-                        Log.d(TAG, "unsubscribe! $channel")
+                        Log.d(TAG, "unsubscribe! $topic")
                     }
                     .catch { e ->
-                        Log.d(TAG, "Subscription $channel reported error: ${e.localizedMessage}")
+                        Log.d(TAG, "Subscription $topic reported error: ${e.localizedMessage}")
                     }
                     .map {
                         WebsocketProvider.WebsocketData.Message(it)
@@ -247,10 +247,10 @@ class WebsocketProviderImpl(
     }
 
     override fun <T : Any> subscribeMessage(
-        channel: String,
+        topic: String,
         deserializer: DeserializationStrategy<T>
     ): Flow<T> {
-        return subscribe(channel, deserializer).mapNotNull {
+        return subscribe(topic, deserializer).mapNotNull {
             when (it) {
                 is WebsocketProvider.WebsocketData.Message -> it.message
                 else -> null

diff --git a/...n/kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/impl/WebsocketTopic.kt b/...n/kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/impl/WebsocketTopic.kt
@@ -0,0 +1,26 @@
+package de.tum.informatics.www1.artemis.native_app.core.websocket.impl
+
+object WebsocketTopic {
+
+    /**
+     * Returns the topic for conversation updates for a course-wide conversation.
+     */
+    fun getCourseWideConversationUpdateTopic(courseId: Long): String {
+        return "/topic/metis/courses/$courseId"
+    }
+
+    /**
+     * Returns the topic for conversation updates for a non-course-wide conversation.
+     */
+    fun getNormalConversationUpdateTopic(userId: Long): String {
+        return "/topic/user/$userId/notifications/conversations"
+    }
+
+    /**
+     * Returns the topic for conversation meta updates. This includes channel creation, deletion,
+     * and updates (like changing the channel name).
+     */
+    fun getConversationMetaUpdateTopic(courseId: Long, userId: Long): String {
+        return "/user/topic/metis/courses/$courseId/conversations/user/$userId"
+    }
+}
diff --git a/...n/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metistest/MetisServiceStub.kt b/...n/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metistest/MetisServiceStub.kt
@@ -32,7 +32,10 @@ class MetisServiceStub(
         return NetworkResponse.Response(posts.first())
     }
 
-    override fun subscribeToPostUpdates(metisContext: MetisContext): Flow<WebsocketProvider.WebsocketData<MetisPostDTO>> {
+    override fun subscribeToPostUpdates(
+        courseId: Long,
+        clientId: Long
+    ): Flow<WebsocketProvider.WebsocketData<MetisPostDTO>> {
         return flowOf()
     }
 }
diff --git a/...matics/www1/artemis/native_app/feature/metis/conversation/service/network/MetisService.kt b/...matics/www1/artemis/native_app/feature/metis/conversation/service/network/MetisService.kt
@@ -34,7 +34,8 @@ interface MetisService {
     ): NetworkResponse<StandalonePost>
 
     fun subscribeToPostUpdates(
-        metisContext: MetisContext
+        courseId: Long,
+        clientId: Long,
     ): Flow<WebsocketProvider.WebsocketData<MetisPostDTO>>
 
     /**

diff --git a/...w1/artemis/native_app/feature/metis/conversation/service/network/impl/MetisServiceImpl.kt b/...w1/artemis/native_app/feature/metis/conversation/service/network/impl/MetisServiceImpl.kt
@@ -5,19 +5,21 @@ import de.tum.informatics.www1.artemis.native_app.core.data.cookieAuth
 import de.tum.informatics.www1.artemis.native_app.core.data.performNetworkCall
 import de.tum.informatics.www1.artemis.native_app.core.data.service.KtorProvider
 import de.tum.informatics.www1.artemis.native_app.core.websocket.WebsocketProvider
+import de.tum.informatics.www1.artemis.native_app.core.websocket.impl.WebsocketTopic
+import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.network.MetisService
+import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.network.RESOURCE_PATH_SEGMENTS
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisFilter
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisPostDTO
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisSortingStrategy
-import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.network.MetisService
-import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.network.RESOURCE_PATH_SEGMENTS
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.CourseWideContext
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.StandalonePost
 import io.ktor.client.call.body
 import io.ktor.client.request.get
 import io.ktor.client.request.parameter
 import io.ktor.http.appendPathSegments
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.merge
 
 internal class MetisServiceImpl(
     private val ktorProvider: KtorProvider,
@@ -148,9 +150,16 @@ internal class MetisServiceImpl(
         }
     }
 
-    override fun subscribeToPostUpdates(metisContext: MetisContext): Flow<WebsocketProvider.WebsocketData<MetisPostDTO>> {
-        val channel = "/topic/metis/courses/${metisContext.courseId}"
+    override fun subscribeToPostUpdates(
+        courseId: Long,
+        clientId: Long
+    ): Flow<WebsocketProvider.WebsocketData<MetisPostDTO>> {
+        val courseWideTopic = WebsocketTopic.getCourseWideConversationUpdateTopic(courseId)
+        val normalTopic = WebsocketTopic.getNormalConversationUpdateTopic(clientId)
+
+        val courseWideUpdates = websocketProvider.subscribe(courseWideTopic, MetisPostDTO.serializer())
+        val normalUpdates = websocketProvider.subscribe(normalTopic, MetisPostDTO.serializer())
 
-        return websocketProvider.subscribe(channel, MetisPostDTO.serializer())
+        return merge(courseWideUpdates, normalUpdates)
     }
 }
diff --git a/...nformatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationViewModel.kt b/...nformatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationViewModel.kt
@@ -42,7 +42,7 @@ import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.
 import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.reply.ReplyAutoCompleteHintProvider
 import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui.thread.ConversationThreadUseCase
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
-import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisPostAction
+import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisCrudAction
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.StandalonePostId
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.AnswerPost
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.ConversationWebsocketDto
@@ -233,7 +233,7 @@ internal open class ConversationViewModel(
         clientId.filterSuccess()
     ) { conversationDataState, clientId ->
         websocketProvider.subscribeToConversationUpdates(clientId, metisContext.courseId)
-            .filter { it.crudAction == MetisPostAction.UPDATE }
+            .filter { it.crudAction == MetisCrudAction.UPDATE }
             .map<ConversationWebsocketDto, DataState<Conversation>> { DataState.Success(it.conversation) }
             .onStart { emit(conversationDataState) }
     }
@@ -270,12 +270,14 @@ internal open class ConversationViewModel(
 
         // Receive websocket updates and store them in the db.
         viewModelScope.launch(coroutineContext) {
-            serverConfigurationService.host.collect { host ->
-                webSocketUpdateUseCase.updatePosts(
-                    host = host,
-                    context = MetisContext.Conversation(courseId, conversationId)
-                )
-            }
+            combine(serverConfigurationService.host, clientId.filterSuccess()) { host, clientId -> host to clientId }
+                .collect { (host, clientId) ->
+                    webSocketUpdateUseCase.updatePosts(
+                        host = host,
+                        context = metisContext,
+                        clientId = clientId
+                    )
+                }
         }
     }
 

diff --git a/...w1/artemis/native_app/feature/metis/conversation/ui/ConversationWebSocketUpdateUseCase.kt b/...w1/artemis/native_app/feature/metis/conversation/ui/ConversationWebSocketUpdateUseCase.kt
@@ -2,9 +2,10 @@ package de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.ui
 
 import de.tum.informatics.www1.artemis.native_app.core.websocket.WebsocketProvider
 import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.network.MetisService
-import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
-import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisPostAction
 import de.tum.informatics.www1.artemis.native_app.feature.metis.conversation.service.storage.MetisStorageService
+import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
+import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisCrudAction
+import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisPostDTO
 
 /**
  * Manages updates to the conversation over the web socket.
@@ -19,31 +20,46 @@ class ConversationWebSocketUpdateUseCase(
      */
     suspend fun updatePosts(
         host: String,
-        context: MetisContext
+        context: MetisContext,
+        clientId: Long
     ) {
-        metisService.subscribeToPostUpdates(context).collect { websocketData ->
+        metisService.subscribeToPostUpdates(
+            courseId = context.courseId,
+            clientId = clientId
+        ).collect { websocketData ->
             if (websocketData is WebsocketProvider.WebsocketData.Message) {
-                val dto = websocketData.message
-                when (dto.action) {
-                    MetisPostAction.CREATE -> {
-                        metisStorageService.insertLiveCreatedPost(host, context, dto.post)
-                    }
+                updateDatabaseWithDto(
+                    dto = websocketData.message,
+                    context = context,
+                    host = host
+                )
+            }
+        }
+    }
 
-                    MetisPostAction.UPDATE -> {
-                        metisStorageService.updatePost(host, context, dto.post)
-                    }
+    private suspend fun updateDatabaseWithDto(
+        dto: MetisPostDTO,
+        context: MetisContext,
+        host: String
+    ) {
+        when (dto.action) {
+            MetisCrudAction.CREATE -> {
+                metisStorageService.insertLiveCreatedPost(host, context, dto.post)
+            }
 
-                    MetisPostAction.DELETE -> {
-                        metisStorageService.deletePosts(
-                            host,
-                            listOf(dto.post.id ?: return@collect)
-                        )
-                    }
+            MetisCrudAction.UPDATE -> {
+                metisStorageService.updatePost(host, context, dto.post)
+            }
 
-                    MetisPostAction.NEW_MESSAGE -> {
+            MetisCrudAction.DELETE -> {
+                metisStorageService.deletePosts(
+                    host,
+                    listOf(dto.post.id ?: return)
+                )
+            }
 
-                    }
-                }
+            MetisCrudAction.NEW_MESSAGE -> {
+                // Nothing to do here. Only relevant for the conversation overview.
             }
         }
     }

diff --git a/...ature/metis/manageconversations/ui/conversation/overview/ConversationOverviewViewModel.kt b/...ature/metis/manageconversations/ui/conversation/overview/ConversationOverviewViewModel.kt
@@ -21,7 +21,7 @@ import de.tum.informatics.www1.artemis.native_app.core.websocket.WebsocketProvid
 import de.tum.informatics.www1.artemis.native_app.feature.metis.manageconversations.ConversationCollections.ConversationCollection
 import de.tum.informatics.www1.artemis.native_app.feature.metis.manageconversations.service.storage.ConversationPreferenceService
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisContext
-import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisPostAction
+import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisCrudAction
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.ConversationWebsocketDto
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.conversation.ChannelChat
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.conversation.Conversation
@@ -281,13 +281,15 @@ class ConversationOverviewViewModel(
                         is ServerSentConversationUpdate -> {
                             val serverSentUpdate = update.update
 
+                            // TODO: It seems like there are no updates received from the websocket -> investigate
+
                             when (serverSentUpdate.crudAction) {
-                                MetisPostAction.CREATE, MetisPostAction.UPDATE -> {
+                                MetisCrudAction.CREATE, MetisCrudAction.UPDATE -> {
                                     currentConversations[serverSentUpdate.conversation.id] =
                                         serverSentUpdate.conversation
                                 }
 
-                                MetisPostAction.NEW_MESSAGE -> {
+                                MetisCrudAction.NEW_MESSAGE -> {
                                     val isMetisContextVisible =
                                         visibleMetisContexts.value.any { visibleMetisContext ->
                                             val metisContext = visibleMetisContext.metisContext
@@ -305,7 +307,7 @@ class ConversationOverviewViewModel(
                                     }
                                 }
 
-                                MetisPostAction.DELETE -> {
+                                MetisCrudAction.DELETE -> {
                                     currentConversations.remove(serverSentUpdate.conversation.id)
                                 }
                             }

diff --git a/...e/metis/shared/content/MetisPostAction.kt → ...e/metis/shared/content/MetisCrudAction.kt b/...e/metis/shared/content/MetisPostAction.kt → ...e/metis/shared/content/MetisCrudAction.kt
@@ -3,9 +3,9 @@ package de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content
 import kotlinx.serialization.Serializable
 
 @Serializable
-enum class MetisPostAction(val value: String) {
+enum class MetisCrudAction(val value: String) {
     CREATE("CREATE"),
     UPDATE("UPDATE"),
     DELETE("DELETE"),
-    NEW_MESSAGE("NEW_MESSAGE")
+    NEW_MESSAGE("NEW_MESSAGE")    // Only used when for the first message in a new conversation.
 }
diff --git a/...n/de/tum/informatics/www1/artemis/native_app/feature/metis/shared/content/MetisPostDTO.kt b/...n/de/tum/informatics/www1/artemis/native_app/feature/metis/shared/content/MetisPostDTO.kt
@@ -4,4 +4,4 @@ import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.d
 import kotlinx.serialization.Serializable
 
 @Serializable
-data class MetisPostDTO(val post: StandalonePost, val action: MetisPostAction)
+data class MetisPostDTO(val post: StandalonePost, val action: MetisCrudAction)
diff --git a/...tics/www1/artemis/native_app/feature/metis/shared/content/dto/ConversationWebsocketDto.kt b/...tics/www1/artemis/native_app/feature/metis/shared/content/dto/ConversationWebsocketDto.kt
@@ -1,6 +1,6 @@
 package de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto
 
-import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisPostAction
+import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.MetisCrudAction
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.conversation.Conversation
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
@@ -9,5 +9,5 @@ import kotlinx.serialization.Serializable
 data class ConversationWebsocketDto(
     val conversation: Conversation,
     @SerialName("metisCrudAction")
-    val crudAction: MetisPostAction
+    val crudAction: MetisCrudAction
 )
diff --git a/...rtemis/native_app/feature/metis/shared/service/network/ConversationWebsocketExtensions.kt b/...rtemis/native_app/feature/metis/shared/service/network/ConversationWebsocketExtensions.kt
@@ -1,11 +1,12 @@
 package de.tum.informatics.www1.artemis.native_app.feature.metis.shared.service.network
 
 import de.tum.informatics.www1.artemis.native_app.core.websocket.WebsocketProvider
+import de.tum.informatics.www1.artemis.native_app.core.websocket.impl.WebsocketTopic
 import de.tum.informatics.www1.artemis.native_app.feature.metis.shared.content.dto.ConversationWebsocketDto
 import kotlinx.coroutines.flow.Flow
 
 fun WebsocketProvider.subscribeToConversationUpdates(userId: Long, courseId: Long): Flow<ConversationWebsocketDto> {
-    val topic = "/user/topic/metis/courses/$courseId/conversations/user/$userId"
+    val topic = WebsocketTopic.getConversationMetaUpdateTopic(courseId, userId)
 
     return subscribeMessage(topic, ConversationWebsocketDto.serializer())
 }