Rework edition of event management - step 2

CHANGES.md

@@ -25,7 +25,7 @@ Test:
  -
 
 Other changes:
- -
+ - Rework edition of event management
 
 Changes in Element 1.1.0 (2021-02-19)
 ===================================================

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmSessionStoreMigration.kt

@@ -18,6 +18,8 @@ package org.matrix.android.sdk.internal.database
 
 import io.realm.DynamicRealm
 import io.realm.RealmMigration
+import org.matrix.android.sdk.internal.database.model.EditAggregatedSummaryEntityFields
+import org.matrix.android.sdk.internal.database.model.EditionOfEventFields
 import org.matrix.android.sdk.internal.database.model.HomeServerCapabilitiesEntityFields
 import org.matrix.android.sdk.internal.database.model.PendingThreePidEntityFields
 import org.matrix.android.sdk.internal.database.model.PreviewUrlCacheEntityFields
@@ -30,7 +32,7 @@ import javax.inject.Inject
 class RealmSessionStoreMigration @Inject constructor() : RealmMigration {
 
     companion object {
-        const val SESSION_STORE_SCHEMA_VERSION = 7L
+        const val SESSION_STORE_SCHEMA_VERSION = 8L
     }
 
     override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
@@ -43,6 +45,7 @@ class RealmSessionStoreMigration @Inject constructor() : RealmMigration {
         if (oldVersion <= 4) migrateTo5(realm)
         if (oldVersion <= 5) migrateTo6(realm)
         if (oldVersion <= 6) migrateTo7(realm)
+        if (oldVersion <= 7) migrateTo8(realm)
     }
 
     private fun migrateTo1(realm: DynamicRealm) {
@@ -122,4 +125,29 @@ class RealmSessionStoreMigration @Inject constructor() : RealmMigration {
                 }
                 ?.removeField("areAllMembersLoaded")
     }
+
+    private fun migrateTo8(realm: DynamicRealm) {
+        Timber.d("Step 7 -> 8")
+
+        val editionOfEventSchema = realm.schema.create("EditionOfEvent")
+                .apply {
+                    // setEmbedded does not return `this`...
+                    isEmbedded = true
+                }
+                .addField(EditionOfEventFields.CONTENT, String::class.java)
+                .addField(EditionOfEventFields.EVENT_ID, String::class.java)
+                .setRequired(EditionOfEventFields.EVENT_ID, true)
+                .addField(EditionOfEventFields.SENDER_ID, String::class.java)
+                .setRequired(EditionOfEventFields.SENDER_ID, true)
+                .addField(EditionOfEventFields.TIMESTAMP, Long::class.java)
+                .addField(EditionOfEventFields.IS_LOCAL_ECHO, Boolean::class.java)
+
+
+        realm.schema.get("EditAggregatedSummaryEntity")
+                ?.removeField("aggregatedContent")
+                ?.removeField("sourceEvents")
+                ?.removeField("lastEditTs")
+                ?.removeField("sourceLocalEchoEvents")
+                ?.addRealmListField(EditAggregatedSummaryEntityFields.EDITIONS.`$`, editionOfEventSchema)
+    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt

@@ -98,6 +98,7 @@ internal fun ChunkEntity.addTimelineEvent(roomId: String,
         this.eventId = eventId
         this.roomId = roomId
         this.annotations = EventAnnotationsSummaryEntity.where(realm, roomId, eventId).findFirst()
+                ?.also { it.cleanUp(eventEntity.sender) }
         this.readReceipts = readReceiptsSummaryEntity
         this.displayIndex = displayIndex
         val roomMemberContent = roomMemberContentsByUser[senderId]

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventAnnotationsSummaryMapper.kt

@@ -20,11 +20,7 @@ import org.matrix.android.sdk.api.session.room.model.EditAggregatedSummary
 import org.matrix.android.sdk.api.session.room.model.EventAnnotationsSummary
 import org.matrix.android.sdk.api.session.room.model.ReactionAggregatedSummary
 import org.matrix.android.sdk.api.session.room.model.ReferencesAggregatedSummary
-import org.matrix.android.sdk.internal.database.model.EditAggregatedSummaryEntity
 import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntity
-import org.matrix.android.sdk.internal.database.model.ReactionAggregatedSummaryEntity
-import org.matrix.android.sdk.internal.database.model.ReferencesAggregatedSummaryEntity
-import io.realm.RealmList
 
 internal object EventAnnotationsSummaryMapper {
     fun map(annotationsSummary: EventAnnotationsSummaryEntity): EventAnnotationsSummary {
@@ -41,11 +37,14 @@ internal object EventAnnotationsSummaryMapper {
                     )
                 },
                 editSummary = annotationsSummary.editSummary?.let {
+                    val latestEdition = it.editions.maxByOrNull { editionOfEvent -> editionOfEvent.timestamp }
                     EditAggregatedSummary(
-                            ContentMapper.map(it.aggregatedContent),
+                            ContentMapper.map(latestEdition?.content),
-                            it.sourceEvents.toList(),
+                            it.editions.filter { editionOfEvent -> !editionOfEvent.isLocalEcho }
-                            it.sourceLocalEchoEvents.toList(),
+                                    .map { editionOfEvent -> editionOfEvent.eventId },
-                            it.lastEditTs
+                            it.editions.filter { editionOfEvent -> editionOfEvent.isLocalEcho }
+                                    .map { editionOfEvent -> editionOfEvent.eventId },
+                            latestEdition?.timestamp ?: 0L
                     )
                 },
                 referencesAggregatedSummary = annotationsSummary.referencesSummaryEntity?.let {
@@ -62,46 +61,6 @@ internal object EventAnnotationsSummaryMapper {
 
         )
     }
-
-    fun map(annotationsSummary: EventAnnotationsSummary, roomId: String): EventAnnotationsSummaryEntity {
-        val eventAnnotationsSummaryEntity = EventAnnotationsSummaryEntity()
-        eventAnnotationsSummaryEntity.eventId = annotationsSummary.eventId
-        eventAnnotationsSummaryEntity.roomId = roomId
-        eventAnnotationsSummaryEntity.editSummary = annotationsSummary.editSummary?.let {
-            EditAggregatedSummaryEntity(
-                    ContentMapper.map(it.aggregatedContent),
-                    RealmList<String>().apply { addAll(it.sourceEvents) },
-                    RealmList<String>().apply { addAll(it.localEchos) },
-                    it.lastEditTs
-            )
-        }
-        eventAnnotationsSummaryEntity.reactionsSummary = annotationsSummary.reactionsSummary.let {
-            RealmList<ReactionAggregatedSummaryEntity>().apply {
-                addAll(it.map {
-                    ReactionAggregatedSummaryEntity(
-                            it.key,
-                            it.count,
-                            it.addedByMe,
-                            it.firstTimestamp,
-                            RealmList<String>().apply { addAll(it.sourceEvents) },
-                            RealmList<String>().apply { addAll(it.localEchoEvents) }
-                    )
-                })
-            }
-        }
-        eventAnnotationsSummaryEntity.referencesSummaryEntity = annotationsSummary.referencesAggregatedSummary?.let {
-            ReferencesAggregatedSummaryEntity(
-                    it.eventId,
-                    ContentMapper.map(it.content),
-                    RealmList<String>().apply { addAll(it.sourceEvents) },
-                    RealmList<String>().apply { addAll(it.localEchos) }
-            )
-        }
-        eventAnnotationsSummaryEntity.pollResponseSummary = annotationsSummary.pollResponseSummary?.let {
-            PollResponseAggregatedSummaryEntityMapper.map(it)
-        }
-        return eventAnnotationsSummaryEntity
-    }
 }
 
 internal fun EventAnnotationsSummaryEntity.asDomain(): EventAnnotationsSummary {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EditAggregatedSummaryEntity.kt

@@ -17,17 +17,24 @@ package org.matrix.android.sdk.internal.database.model
 
 import io.realm.RealmList
 import io.realm.RealmObject
+import io.realm.annotations.RealmClass
 
 /**
- * Keep the latest state of edition of a message
+ * Keep all the editions of a message
  */
 internal open class EditAggregatedSummaryEntity(
-        var aggregatedContent: String? = null,
+        // The list of the editions used to build the summary (might be out of sync if chunked received from message chunk)
-        // The list of the eventIDs used to build the summary (might be out of sync if chunked received from message chunk)
+        var editions: RealmList<EditionOfEvent> = RealmList()
-        var sourceEvents: RealmList<String> = RealmList(),
-        var sourceLocalEchoEvents: RealmList<String> = RealmList(),
-        var lastEditTs: Long = 0
 ) : RealmObject() {
 
     companion object
 }
+
+@RealmClass(embedded = true)
+internal open class EditionOfEvent(
+        var senderId: String = "",
+        var eventId: String = "",
+        var content: String? = null,
+        var timestamp: Long = 0,
+        var isLocalEcho: Boolean = false
+) : RealmObject()

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EventAnnotationsSummaryEntity.kt

@@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.database.model
 import io.realm.RealmList
 import io.realm.RealmObject
 import io.realm.annotations.PrimaryKey
+import timber.log.Timber
 
 internal open class EventAnnotationsSummaryEntity(
         @PrimaryKey
@@ -29,6 +30,21 @@ internal open class EventAnnotationsSummaryEntity(
         var pollResponseSummary: PollResponseAggregatedSummaryEntity? = null
 ) : RealmObject() {
 
+    /**
+     * Cleanup undesired editions, done by users different from the originalEventSender
+     */
+    fun cleanUp(originalEventSenderId: String?) {
+        originalEventSenderId ?: return
+
+        editSummary?.editions?.filter {
+            it.senderId != originalEventSenderId
+        }
+                ?.forEach {
+                    Timber.w("Deleting an edition from ${it.senderId} of event sent by $originalEventSenderId")
+                    it.deleteFromRealm()
+                }
+    }
+
     companion object
 }
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/SessionRealmModule.kt

@@ -43,6 +43,7 @@ import io.realm.annotations.RealmModule
             EventAnnotationsSummaryEntity::class,
             ReactionAggregatedSummaryEntity::class,
             EditAggregatedSummaryEntity::class,
+            EditionOfEvent::class,
             PollResponseAggregatedSummaryEntity::class,
             ReferencesAggregatedSummaryEntity::class,
             PushRulesEntity::class,

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/EventRelationsAggregationProcessor.kt

@@ -36,6 +36,7 @@ import org.matrix.android.sdk.internal.crypto.verification.toState
 import org.matrix.android.sdk.internal.database.mapper.ContentMapper
 import org.matrix.android.sdk.internal.database.mapper.EventMapper
 import org.matrix.android.sdk.internal.database.model.EditAggregatedSummaryEntity
+import org.matrix.android.sdk.internal.database.model.EditionOfEvent
 import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntity
 import org.matrix.android.sdk.internal.database.model.EventEntity
 import org.matrix.android.sdk.internal.database.model.EventInsertType
@@ -219,49 +220,48 @@ internal class EventRelationsAggregationProcessor @Inject constructor(@UserId pr
             // create the edit summary
             eventAnnotationsSummaryEntity.editSummary = realm.createObject(EditAggregatedSummaryEntity::class.java)
                     .also { editSummary ->
-                        editSummary.aggregatedContent = ContentMapper.map(newContent)
+                        editSummary.editions.add(
-                        if (isLocalEcho) {
+                                EditionOfEvent(
-                            editSummary.lastEditTs = 0
+                                        senderId = event.senderId ?: "",
-                            editSummary.sourceLocalEchoEvents.add(eventId)
+                                        eventId = event.eventId,
-                        } else {
+                                        content = ContentMapper.map(newContent),
-                            editSummary.lastEditTs = event.originServerTs ?: 0
+                                        timestamp = if (isLocalEcho) 0 else event.originServerTs ?: 0,
-                            editSummary.sourceEvents.add(eventId)
+                                        isLocalEcho = isLocalEcho
-                        }
+                                )
+                        )
                     }
         } else {
-            if (existingSummary.sourceEvents.contains(eventId)) {
+            if (existingSummary.editions.any { it.eventId == eventId }) {
                 // ignore this event, we already know it (??)
                 Timber.v("###REPLACE ignoring event for summary, it's known $eventId")
                 return
             }
             val txId = event.unsignedData?.transactionId
             // is it a remote echo?
-            if (!isLocalEcho && existingSummary.sourceLocalEchoEvents.contains(txId)) {
+            if (!isLocalEcho && existingSummary.editions.any { it.eventId == txId }) {
                 // ok it has already been managed
                 Timber.v("###REPLACE Receiving remote echo of edit (edit already done)")
-                existingSummary.sourceLocalEchoEvents.remove(txId)
+                existingSummary.editions.firstOrNull { it.eventId == txId }?.let {
-                existingSummary.sourceEvents.add(event.eventId)
+                    it.eventId = event.eventId
-            } else if (
+                    it.timestamp = event.originServerTs ?: System.currentTimeMillis()
-                    isLocalEcho // do not rely on ts for local echo, take it
+                    it.isLocalEcho = false
-                    || event.originServerTs ?: 0 >= existingSummary.lastEditTs
-            ) {
-                Timber.v("###REPLACE Computing aggregated edit summary (isLocalEcho:$isLocalEcho)")
-                if (!isLocalEcho) {
-                    // Do not take local echo originServerTs here, could mess up ordering (keep old ts)
-                    existingSummary.lastEditTs = event.originServerTs ?: System.currentTimeMillis()
-                }
-                existingSummary.aggregatedContent = ContentMapper.map(newContent)
-                if (isLocalEcho) {
-                    existingSummary.sourceLocalEchoEvents.add(eventId)
-                } else {
-                    existingSummary.sourceEvents.add(eventId)
                 }
             } else {
-                // ignore this event for the summary (back paginate)
+                Timber.v("###REPLACE Computing aggregated edit summary (isLocalEcho:$isLocalEcho)")
-                if (!isLocalEcho) {
+                existingSummary.editions.add(
-                    existingSummary.sourceEvents.add(eventId)
+                        EditionOfEvent(
-                }
+                                senderId = event.senderId ?: "",
-                Timber.v("###REPLACE ignoring event for summary, it's to old $eventId")
+                                eventId = event.eventId,
+                                content = ContentMapper.map(newContent),
+                                timestamp = if (isLocalEcho) {
+                                    System.currentTimeMillis()
+                                } else {
+                                    // Do not take local echo originServerTs here, could mess up ordering (keep old ts)
+                                    event.originServerTs ?: System.currentTimeMillis()
+                                },
+                                isLocalEcho = isLocalEcho
+                        )
+                )
             }
         }
     }
@@ -448,29 +448,13 @@ internal class EventRelationsAggregationProcessor @Inject constructor(@UserId pr
             Timber.w("Redaction of a replace targeting an unknown event $relatedEventId")
             return
         }
-        val sourceEvents = eventSummary.editSummary?.sourceEvents
+        val sourceToDiscard = eventSummary.editSummary?.editions?.firstOrNull {it.eventId == redacted.eventId }
-        val sourceToDiscard = sourceEvents?.indexOf(redacted.eventId)
         if (sourceToDiscard == null) {
             Timber.w("Redaction of a replace that was not known in aggregation $sourceToDiscard")
             return
         }
-        // Need to remove this event from the redaction list and compute new aggregation state
+        // Need to remove this event from the edition list
-        sourceEvents.removeAt(sourceToDiscard)
+        sourceToDiscard.deleteFromRealm()
-        val previousEdit = sourceEvents.mapNotNull { EventEntity.where(realm, it).findFirst() }.sortedBy { it.originServerTs }.lastOrNull()
-        if (previousEdit == null) {
-            // revert to original
-            eventSummary.editSummary?.deleteFromRealm()
-        } else {
-            // I have the last event
-            ContentMapper.map(previousEdit.content)?.toModel<MessageContent>()?.newContent?.let { newContent ->
-                eventSummary.editSummary?.lastEditTs = previousEdit.originServerTs
-                        ?: System.currentTimeMillis()
-                eventSummary.editSummary?.aggregatedContent = ContentMapper.map(newContent)
-            } ?: run {
-                Timber.e("Failed to udate edited summary")
-                // TODO how to reccover that
-            }
-        }
     }
 
     private fun handleReactionRedact(eventToPrune: EventEntity, realm: Realm, userId: String) {