[PBE-5181] fix pin expiration & pin inconsistency (#5329)

* AUTOMATION: Changelog update * [PBE-5181] pinning with expire date has no effect * [PBE-5250] Remove `url` property from `Attachment` class (#5325) * Remove `url` property from `Attachment` class * Update CHANGELOG.md * Increase database version * [PBE-5181] clear cache on message unpin --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jc Miñarro <[email protected]>
GetStream · Jul 23, 2024 · 80569f3 · 80569f3
1 parent 2a08e7d
commit 80569f3
Show file tree

Hide file tree

Showing 15 changed files with 145 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 
 ## stream-chat-android-client
 ### 🐞 Fixed
+- Fixed pinned message expire date validation. [#5329](https://github.com/GetStream/stream-chat-android/pull/5329)
 
 ### ⬆️ Improved
 
@@ -36,6 +37,7 @@
 
 ## stream-chat-android-state
 ### 🐞 Fixed
+- Fixed channel cache not being updated on message unpin operation. [#5329](https://github.com/GetStream/stream-chat-android/pull/5329)
 
 ### ⬆️ Improved
 
@@ -107,6 +109,7 @@
 ## stream-chat-android-compose
 ### 🐞 Fixed
 - Fixed quoted message styling. [#5316](https://github.com/GetStream/stream-chat-android/pull/5316)
+- Fixed checkbox visibility in `MessageComposer`'s footer when dark theme is used. [#5318](https://github.com/GetStream/stream-chat-android/pull/5318)
 
 ### ⬆️ Improved
 - Sent messages from the MessageComposer are marked as read. [#5322](https://github.com/GetStream/stream-chat-android/pull/5322)
@@ -161,7 +164,6 @@
 ### 🐞 Fixed
 - Fixed deleted pinned messages being highlighted as pinned. [#5315](https://github.com/GetStream/stream-chat-android/pull/5315)
 - Fixed the url used when click on the link. [#5314](https://github.com/GetStream/stream-chat-android/pull/5314)
-- Fixed checkbox visibility in `MessageComposer`'s footer when dark theme is used. [#5318](https://github.com/GetStream/stream-chat-android/pull/5318)
 
 ### ⬆️ Improved
 - Enabled Strong Skipping Mode for Compose compiler and improved Compose performance. [#5303](https://github.com/GetStream/stream-chat-android/pull/5303)

diff --git a/stream-chat-android-client/api/stream-chat-android-client.api b/stream-chat-android-client/api/stream-chat-android-client.api
@@ -2867,7 +2867,6 @@ public final class io/getstream/chat/android/client/utils/message/MessageUtils {
 	public static final fun isGiphy (Lio/getstream/chat/android/models/Message;)Z
 	public static final fun isGiphyEphemeral (Lio/getstream/chat/android/models/Message;)Z
 	public static final fun isModerationError (Lio/getstream/chat/android/models/Message;Ljava/lang/String;)Z
-	public static final fun isPinExpired (Lio/getstream/chat/android/models/Message;Lkotlin/jvm/functions/Function0;)Z
 	public static final fun isPinnedAndNotDeleted (Lio/getstream/chat/android/models/Message;)Z
 	public static final fun isRegular (Lio/getstream/chat/android/models/Message;)Z
 	public static final fun isReply (Lio/getstream/chat/android/models/Message;)Z

diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
@@ -1932,6 +1932,10 @@ internal constructor(
      */
     @CheckResult
     public fun pinMessage(message: Message, expirationDate: Date? = null): Call<Message> {
+        logger.d {
+            "[pinMessage] message: Message(id=${message.id}, text=${message.text})" +
+                ", expirationDate: $expirationDate"
+        }
         val set: MutableMap<String, Any> = LinkedHashMap()
         set["pinned"] = true
         expirationDate?.let { set["pin_expires"] = it }
@@ -1951,6 +1955,10 @@ internal constructor(
      */
     @CheckResult
     public fun pinMessage(message: Message, timeout: Int): Call<Message> {
+        logger.d {
+            "[pinMessage] message: Message(id=${message.id}, text=${message.text})" +
+                ", timeout: $timeout seconds"
+        }
         val calendar = Calendar.getInstance().apply {
             add(Calendar.SECOND, timeout)
         }
@@ -1972,6 +1980,7 @@ internal constructor(
      */
     @CheckResult
     public fun unpinMessage(message: Message): Call<Message> {
+        logger.d { "[unpinMessage] message: Message(text=${message.text}, id=${message.id})" }
         return partialUpdateMessage(
             messageId = message.id,
             set = mapOf("pinned" to false),

diff --git a/...droid-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.kt b/...droid-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.kt
@@ -97,8 +97,8 @@ public inline fun Message.isPinned(
 /**
  * @return If the message is a valid pinned message.
  */
-@PublishedApi
-internal inline fun Message.isPinExpired(now: () -> Long): Boolean = pinExpires?.let { it.time > now() } ?: false
+@InternalStreamChatApi
+public inline fun Message.isPinExpired(now: () -> Long): Boolean = pinExpires?.let { it.time < now() } ?: false
 
 /**
  * @return If the message type is regular.

diff --git a/...d-client/src/test/java/io/getstream/chat/android/client/utils/message/MessageUtilsTest.kt b/...d-client/src/test/java/io/getstream/chat/android/client/utils/message/MessageUtilsTest.kt
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2014-2024 Stream.io Inc. All rights reserved.
+ *
+ * Licensed under the Stream License;
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    https://github.com/GetStream/stream-chat-android/blob/main/LICENSE
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.getstream.chat.android.client.utils.message
+
+import io.getstream.chat.android.randomMessage
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.currentTime
+import kotlinx.coroutines.test.runTest
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+import java.util.Date
+
+@ExperimentalCoroutinesApi
+internal class MessageUtilsTest {
+
+    @Test
+    fun `validate message_isPinExpired`() = runTest {
+        val message = randomMessage(
+            pinned = true,
+            pinExpires = Date(currentTime + 1000),
+        )
+
+        advanceTimeBy(2000)
+
+        message.isPinExpired { currentTime } shouldBeEqualTo true
+    }
+
+    @Test
+    fun `validate message_isPinned when pin expires`() = runTest {
+        val message = randomMessage(
+            pinned = true,
+            pinExpires = Date(currentTime + 1000),
+        )
+
+        advanceTimeBy(2000)
+
+        message.isPinned { currentTime } shouldBeEqualTo false
+    }
+
+    @Test
+    fun `validate message_isPinned when message gets deleted`() = runTest {
+        val message = randomMessage(
+            pinned = true,
+            deletedAt = Date(currentTime),
+        )
+
+        advanceTimeBy(2000)
+
+        message.isPinned { currentTime } shouldBeEqualTo false
+    }
+
+    @Test
+    fun `validate message_isPinned when message gets unpinned`() = runTest {
+        val message = randomMessage(
+            pinned = false,
+        )
+
+        message.isPinned { currentTime } shouldBeEqualTo false
+    }
+}
diff --git a/stream-chat-android-offline/api/stream-chat-android-offline.api b/stream-chat-android-offline/api/stream-chat-android-offline.api
@@ -1,5 +1,7 @@
 public final class io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory : io/getstream/chat/android/client/persistance/repository/factory/RepositoryFactory$Provider, io/getstream/chat/android/client/plugin/factory/PluginFactory {
 	public fun <init> (Landroid/content/Context;)V
+	public fun <init> (Landroid/content/Context;Lkotlin/jvm/functions/Function0;)V
+	public synthetic fun <init> (Landroid/content/Context;Lkotlin/jvm/functions/Function0;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun createRepositoryFactory (Lio/getstream/chat/android/models/User;)Lio/getstream/chat/android/client/persistance/repository/factory/RepositoryFactory;
 	public fun get (Lio/getstream/chat/android/models/User;)Lio/getstream/chat/android/client/plugin/Plugin;
 }

diff --git a/.../main/java/io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory.kt b/.../main/java/io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory.kt
@@ -60,7 +60,10 @@ import kotlin.reflect.KClass
  *
  * @param appContext [Context]
  */
-public class StreamOfflinePluginFactory(private val appContext: Context) : PluginFactory, RepositoryFactory.Provider {
+public class StreamOfflinePluginFactory @JvmOverloads constructor(
+    private val appContext: Context,
+    private val now: () -> Long = { System.currentTimeMillis() },
+) : PluginFactory, RepositoryFactory.Provider {
 
     private val logger by taggedLogger("Chat:OfflinePluginFactory")
 
@@ -77,6 +80,7 @@ public class StreamOfflinePluginFactory(private val appContext: Context) : Plugi
             database = createDatabase(appContext, user),
             currentUser = user,
             scope = ChatClient.instance().inheritScope { SupervisorJob(it) },
+            now = now,
         )
     }
 

diff --git a/...ream/chat/android/offline/repository/domain/channel/internal/DatabaseChannelRepository.kt b/...ream/chat/android/offline/repository/domain/channel/internal/DatabaseChannelRepository.kt
@@ -19,7 +19,7 @@ package io.getstream.chat.android.offline.repository.domain.channel.internal
 import android.util.LruCache
 import io.getstream.chat.android.client.extensions.syncUnreadCountWithReads
 import io.getstream.chat.android.client.persistance.repository.ChannelRepository
-import io.getstream.chat.android.client.utils.message.isDeleted
+import io.getstream.chat.android.client.utils.message.isPinned
 import io.getstream.chat.android.core.utils.date.maxOf
 import io.getstream.chat.android.core.utils.date.minOf
 import io.getstream.chat.android.models.Channel
@@ -42,6 +42,7 @@ internal class DatabaseChannelRepository(
     private val channelDao: ChannelDao,
     private val getUser: suspend (userId: String) -> User,
     private val getMessage: suspend (messageId: String) -> Message?,
+    private val now: () -> Long = { System.currentTimeMillis() },
     cacheSize: Int = 1000,
 ) : ChannelRepository {
 
@@ -92,11 +93,12 @@ internal class DatabaseChannelRepository(
      */
     override suspend fun deleteChannel(cid: String) {
         logger.v { "[deleteChannel] cid: $cid" }
-        channelCache.remove(cid)
+        removeFromCache(cid)
         scope.launchWithMutex(dbMutex) { channelDao.delete(cid) }
     }
 
     override suspend fun deleteChannelMessage(message: Message) {
+        logger.v { "[deleteChannelMessage] message.id: ${message.id}, message.text: ${message.text}" }
         channelCache[message.cid]?.let { cachedChannel ->
             val updatedChannel = cachedChannel.copy(
                 messages = cachedChannel.messages.filter { it.id != message.id },
@@ -107,12 +109,13 @@ internal class DatabaseChannelRepository(
     }
 
     override suspend fun updateChannelMessage(message: Message) {
+        logger.v { "[updateChannelMessage] message.id: ${message.id}, message.text: ${message.text}" }
         channelCache[message.cid]?.let { cachedChannel ->
             val updatedChannel = cachedChannel.copy(
                 messages = cachedChannel.messages.map { if (it.id == message.id) message else it },
                 pinnedMessages = cachedChannel.pinnedMessages.mapNotNull { existing ->
                     when (existing.id == message.id) {
-                        true -> message.takeUnless { it.isDeleted() }
+                        true -> message.takeIf { it.isPinned(now) }
                         else -> existing
                     }
                 },
@@ -172,7 +175,7 @@ internal class DatabaseChannelRepository(
      * @param deletedAt Date.
      */
     override suspend fun setChannelDeletedAt(cid: String, deletedAt: Date) {
-        channelCache.remove(cid)
+        removeFromCache(cid)
         scope.launchWithMutex(dbMutex) { channelDao.setDeletedAt(cid, deletedAt) }
     }
 
@@ -184,7 +187,7 @@ internal class DatabaseChannelRepository(
      * @param hideMessagesBefore Date.
      */
     override suspend fun setHiddenForChannel(cid: String, hidden: Boolean, hideMessagesBefore: Date) {
-        channelCache.remove(cid)
+        removeFromCache(cid)
         scope.launchWithMutex(dbMutex) { channelDao.setHidden(cid, hidden, hideMessagesBefore) }
     }
 
@@ -195,7 +198,7 @@ internal class DatabaseChannelRepository(
      * @param hidden Date.
      */
     override suspend fun setHiddenForChannel(cid: String, hidden: Boolean) {
-        channelCache.remove(cid)
+        removeFromCache(cid)
         scope.launchWithMutex(dbMutex) { channelDao.setHidden(cid, hidden) }
     }
 
@@ -269,6 +272,11 @@ internal class DatabaseChannelRepository(
 
     override suspend fun evictChannel(cid: String) {
         logger.v { "[evictChannel] cid: $cid" }
+        removeFromCache(cid)
+    }
+
+    private fun removeFromCache(cid: String) {
+        logger.v { "[removeFromCache] cid: $cid" }
         channelCache.remove(cid)
     }
 

diff --git a/...o/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.kt b/...o/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.kt
@@ -42,6 +42,7 @@ internal class DatabaseRepositoryFactory(
     private val database: ChatDatabase,
     private val currentUser: User,
     private val scope: CoroutineScope,
+    private val now: () -> Long = { System.currentTimeMillis() },
 ) : RepositoryFactory {
 
     private var repositoriesCache: MutableMap<Class<out Any>, Any> = mutableMapOf()
@@ -74,7 +75,7 @@ internal class DatabaseRepositoryFactory(
         val databaseChannelRepository = repositoriesCache[ChannelRepository::class.java] as? DatabaseChannelRepository?
 
         return databaseChannelRepository ?: run {
-            DatabaseChannelRepository(scope, database.channelStateDao(), getUser, getMessage)
+            DatabaseChannelRepository(scope, database.channelStateDao(), getUser, getMessage, now)
                 .also { repository ->
                     repositoriesCache[ChannelRepository::class.java] = repository
                 }

diff --git a/...ain/java/io/getstream/chat/android/state/event/handler/internal/EventHandlerSequential.kt b/...ain/java/io/getstream/chat/android/state/event/handler/internal/EventHandlerSequential.kt
@@ -723,6 +723,9 @@ internal class EventHandlerSequential(
                         repos.markMessageAsDeleted(event.message)
                     }
                 }
+                is MessageUpdatedEvent -> {
+                    repos.updateChannelMessage(event.message)
+                }
                 is MemberRemovedEvent -> {
                     repos.evictChannel(event.cid)
                 }

diff --git a/...n/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt b/...n/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt
@@ -28,6 +28,7 @@ import io.getstream.chat.android.client.events.UserStopWatchingEvent
 import io.getstream.chat.android.client.extensions.internal.NEVER
 import io.getstream.chat.android.client.setup.state.ClientState
 import io.getstream.chat.android.client.utils.message.isDeleted
+import io.getstream.chat.android.client.utils.message.isPinExpired
 import io.getstream.chat.android.client.utils.message.isPinned
 import io.getstream.chat.android.client.utils.message.isReply
 import io.getstream.chat.android.models.Channel
@@ -236,7 +237,8 @@ internal class ChannelStateLogic(
 
     override fun delsertPinnedMessage(message: Message) {
         logger.d {
-            "[delsertPinnedMessage] pinned: ${message.pinned}, deleted: ${message.isDeleted()}" +
+            "[delsertPinnedMessage] pinned: ${message.pinned}, pinExpired: ${message.isPinExpired(now)}" +
+                ", deleted: ${message.isDeleted()}" +
                 ", message.id: ${message.id}, message.text: ${message.text}"
         }
         if (message.isPinned(now)) {

diff --git a/...java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt b/...java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt
@@ -455,12 +455,12 @@ internal class ChannelMutableState(
     fun deleteMessage(message: Message) {
         logger.v { "[deleteMessage] message.id: ${message.id}" }
         _messages?.apply { value = value - message.id }
-        _pinnedMessages?.apply { value = value - message.id }
+        setPinned { pinned -> pinned - message.id }
     }
 
     fun deletePinnedMessage(message: Message) {
-        logger.v { "[deletePinnedMessage] message.id: ${message.id}" }
-        _pinnedMessages?.apply { value = value - message.id }
+        logger.v { "[deletePinnedMessage] message.id=${message.id}, message.text=${message.text}" }
+        setPinned { pinned -> pinned - message.id }
     }
 
     fun upsertWatchers(watchers: List<User>, watchersCount: Int) {
@@ -529,8 +529,9 @@ internal class ChannelMutableState(
         } ?: false
 
     fun removeMessagesBefore(date: Date) {
+        logger.d { "[removeMessagesBefore] date: $date" }
         _messages?.apply { value = value.filter { it.value.wasCreatedAfter(date) } }
-        _pinnedMessages?.apply { value = value.filter { it.value.wasCreatedAfter(date) } }
+        setPinned { pinned -> pinned.filter { it.value.wasCreatedAfter(date) } }
     }
 
     fun upsertMessages(updatedMessages: Collection<Message>) {
@@ -542,11 +543,21 @@ internal class ChannelMutableState(
     }
 
     fun setPinnedMessages(messages: List<Message>) {
-        _pinnedMessages?.value = messages.associateBy(Message::id)
+        logger.d { "[setPinnedMessages] messages.size: ${messages.size}" }
+        setPinned { messages.associateBy(Message::id) }
     }
 
     fun upsertPinnedMessages(messages: Collection<Message>) {
-        _pinnedMessages?.apply { value += messages.associateBy(Message::id) }
+        logger.d { "[upsertPinnedMessages] messages.size: ${messages.size}" }
+        setPinned { pinned -> pinned + messages.associateBy(Message::id) }
+    }
+
+    private inline fun setPinned(producer: (Map<String, Message>) -> Map<String, Message>) {
+        val curPinnedMessages = _pinnedMessages?.value
+        curPinnedMessages ?: return
+        val newPinnedMessages = producer(curPinnedMessages)
+        logger.v { "[setPinned] pinned.size: ${curPinnedMessages.size} => ${newPinnedMessages.size}" }
+        _pinnedMessages?.value = newPinnedMessages
     }
 
     private fun cacheLatestMessages() {

diff --git a/stream-chat-android-ui-common/api/stream-chat-android-ui-common.api b/stream-chat-android-ui-common/api/stream-chat-android-ui-common.api
@@ -182,6 +182,8 @@ public final class io/getstream/chat/android/ui/common/feature/messages/list/Mes
 	public final fun performGiphyAction (Lio/getstream/chat/android/ui/common/state/messages/list/GiphyAction;)V
 	public final fun performMessageAction (Lio/getstream/chat/android/ui/common/state/messages/MessageAction;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public final fun pinMessage (Lio/getstream/chat/android/models/Message;)V
+	public final fun pinMessage (Lio/getstream/chat/android/models/Message;Ljava/util/Date;)V
+	public static synthetic fun pinMessage$default (Lio/getstream/chat/android/ui/common/feature/messages/list/MessageListController;Lio/getstream/chat/android/models/Message;Ljava/util/Date;ILjava/lang/Object;)V
 	public final fun reactToMessage (Lio/getstream/chat/android/models/Reaction;Lio/getstream/chat/android/models/Message;)V
 	public final fun removeAttachment (Ljava/lang/String;Lio/getstream/chat/android/models/Attachment;)V
 	public final fun removeOverlay ()V
@@ -701,6 +703,8 @@ public final class io/getstream/chat/android/ui/common/state/messages/MessageMod
 public final class io/getstream/chat/android/ui/common/state/messages/MessageMode$Normal : io/getstream/chat/android/ui/common/state/messages/MessageMode {
 	public static final field $stable I
 	public static final field INSTANCE Lio/getstream/chat/android/ui/common/state/messages/MessageMode$Normal;
+	public fun equals (Ljava/lang/Object;)Z
+	public fun hashCode ()I
 	public fun toString ()Ljava/lang/String;
 }