URL preview: improve fix regarding message edition

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/media/MediaService.kt

@@ -24,10 +24,9 @@ interface MediaService {
     /**
      * Extract URLs from a TimelineEvent.
      * @param event TimelineEvent to extract the URL from.
-     * @param forceExtract Should be used for edited events. If true, URL will be extracted again even it is already in the cache.
      * @return the list of URLs contains in the body of the TimelineEvent. It does not mean that URLs in this list have UrlPreview data
      */
-    fun extractUrls(event: TimelineEvent, forceExtract: Boolean = false): List<String>
+    fun extractUrls(event: TimelineEvent): List<String>
 
     /**
      * Get Raw Url Preview data from the homeserver. There is no cache management for this request

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/timeline/TimelineEvent.kt

@@ -89,6 +89,17 @@ data class TimelineEvent(
  */
 fun TimelineEvent.hasBeenEdited() = annotations?.editSummary != null
 
+/**
+ * Get the latest known eventId for an edited event, or the eventId for an Event which has not been edited
+ */
+fun TimelineEvent.getLatestEventId(): String {
+    return annotations
+            ?.editSummary
+            ?.sourceEvents
+            ?.lastOrNull()
+            ?: eventId
+}
+
 /**
  * Get the relation content if any
  */

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/media/DefaultMediaService.kt

@@ -18,10 +18,10 @@ package org.matrix.android.sdk.internal.session.media
 
 import androidx.collection.LruCache
 import org.matrix.android.sdk.api.cache.CacheStrategy
-import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.media.MediaService
 import org.matrix.android.sdk.api.session.media.PreviewUrlData
 import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
+import org.matrix.android.sdk.api.session.room.timeline.getLatestEventId
 import org.matrix.android.sdk.api.util.JsonDict
 import org.matrix.android.sdk.internal.util.getOrPut
 import javax.inject.Inject
@@ -35,12 +35,12 @@ internal class DefaultMediaService @Inject constructor(
     // Cache of extracted URLs
     private val extractedUrlsCache = LruCache<String, List<String>>(1_000)
 
-    override fun extractUrls(event: TimelineEvent, forceExtract: Boolean): List<String> {
+    override fun extractUrls(event: TimelineEvent): List<String> {
-        if (forceExtract) extractedUrlsCache.remove(event.root.cacheKey())
+        return extractedUrlsCache.getOrPut(event.cacheKey()) { urlsExtractor.extract(event) }
-        return extractedUrlsCache.getOrPut(event.root.cacheKey()) { urlsExtractor.extract(event) }
     }
 
-    private fun Event.cacheKey() = "${eventId ?: ""}-${roomId ?: ""}"
+    // Use the id of the latest Event edition
+    private fun TimelineEvent.cacheKey() = "${getLatestEventId()}-${root.roomId ?: ""}"
 
     override suspend fun getRawPreviewUrl(url: String, timestamp: Long?): JsonDict {
         return getRawPreviewUrlTask.execute(GetRawPreviewUrlTask.Params(url, timestamp))

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/url/PreviewUrlRetriever.kt

@@ -24,12 +24,19 @@ import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.cache.CacheStrategy
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
-import org.matrix.android.sdk.api.session.room.timeline.hasBeenEdited
+import org.matrix.android.sdk.api.session.room.timeline.getLatestEventId
 
 class PreviewUrlRetriever(session: Session) {
     private val mediaService = session.mediaService()
 
-    private val data = mutableMapOf<String, PreviewUrlUiState>()
+    private data class EventIdPreviewUrlUiState(
+            // Id of the latest event in the case of an edited event, or the eventId for an event which has not been edited
+            val latestEventId: String,
+            val previewUrlUiState: PreviewUrlUiState
+    )
+
+    // Keys are the main eventId
+    private val data = mutableMapOf<String, EventIdPreviewUrlUiState>()
     private val listeners = mutableMapOf<String, MutableSet<PreviewUrlRetrieverListener>>()
     private val uiHandler = Handler(Looper.getMainLooper())
 
@@ -38,19 +45,22 @@ class PreviewUrlRetriever(session: Session) {
 
     fun getPreviewUrl(event: TimelineEvent, coroutineScope: CoroutineScope) {
         val eventId = event.root.eventId ?: return
+        val latestEventId = event.getLatestEventId()
 
         synchronized(data) {
-            val isEditedEvent = event.hasBeenEdited()
+            val current = data[eventId]
-            if (data[eventId] == null || isEditedEvent) {
+            if (current?.latestEventId != latestEventId) {
+                // The event is not known or it has been edited
                 // Keep only the first URL for the moment
-                val url = mediaService.extractUrls(event, forceExtract = isEditedEvent)
+                val url = mediaService.extractUrls(event)
                         .firstOrNull()
                         ?.takeIf { it !in blockedUrl }
                 if (url == null) {
-                    updateState(eventId, PreviewUrlUiState.NoUrl)
+                    updateState(eventId, latestEventId, PreviewUrlUiState.NoUrl)
-                    url
+                    null
-                } else if (url != (data[eventId] as? PreviewUrlUiState.Data)?.url) {
+                } else if (url != (current?.previewUrlUiState as? PreviewUrlUiState.Data)?.url) {
-                    updateState(eventId, PreviewUrlUiState.Loading)
+                    // There is a not known URL, or the Event has been edited and the URL has changed
+                    updateState(eventId, latestEventId, PreviewUrlUiState.Loading)
                     url
                 } else {
                     // Already handled
@@ -73,15 +83,15 @@ class PreviewUrlRetriever(session: Session) {
                             synchronized(data) {
                                 // Blocked after the request has been sent?
                                 if (urlToRetrieve in blockedUrl) {
-                                    updateState(eventId, PreviewUrlUiState.NoUrl)
+                                    updateState(eventId, latestEventId, PreviewUrlUiState.NoUrl)
                                 } else {
-                                    updateState(eventId, PreviewUrlUiState.Data(eventId, urlToRetrieve, it))
+                                    updateState(eventId, latestEventId, PreviewUrlUiState.Data(eventId, urlToRetrieve, it))
                                 }
                             }
                         },
                         {
                             synchronized(data) {
-                                updateState(eventId, PreviewUrlUiState.Error(it))
+                                updateState(eventId, latestEventId, PreviewUrlUiState.Error(it))
                             }
                         }
                 )
@@ -95,15 +105,15 @@ class PreviewUrlRetriever(session: Session) {
         // Notify the listener
         synchronized(data) {
             data[eventId]
-                    ?.takeIf { it is PreviewUrlUiState.Data && it.url == url }
+                    ?.takeIf { it.previewUrlUiState is PreviewUrlUiState.Data && it.previewUrlUiState.url == url }
                     ?.let {
-                        updateState(eventId, PreviewUrlUiState.NoUrl)
+                        updateState(eventId, it.latestEventId, PreviewUrlUiState.NoUrl)
                     }
         }
     }
 
-    private fun updateState(eventId: String, state: PreviewUrlUiState) {
+    private fun updateState(eventId: String, latestEventId: String, state: PreviewUrlUiState) {
-        data[eventId] = state
+        data[eventId] = EventIdPreviewUrlUiState(latestEventId, state)
         // Notify the listener
         uiHandler.post {
             listeners[eventId].orEmpty().forEach {
@@ -118,7 +128,7 @@ class PreviewUrlRetriever(session: Session) {
 
         // Give the current state if any
         synchronized(data) {
-            listener.onStateUpdated(data[key] ?: PreviewUrlUiState.Unknown)
+            listener.onStateUpdated(data[key]?.previewUrlUiState ?: PreviewUrlUiState.Unknown)
         }
     }