From 097668b762d7426c4e1ca175f1e09ac0c9e2e4ce Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 1 Mar 2021 19:59:55 +0100 Subject: [PATCH] Rework edition of event management - step 2 --- CHANGES.md | 2 +- .../database/RealmSessionStoreMigration.kt | 30 ++++++- .../database/helper/ChunkEntityHelper.kt | 1 + .../mapper/EventAnnotationsSummaryMapper.kt | 55 ++---------- .../model/EditAggregatedSummaryEntity.kt | 19 +++-- .../model/EventAnnotationsSummaryEntity.kt | 16 ++++ .../database/model/SessionRealmModule.kt | 1 + .../EventRelationsAggregationProcessor.kt | 84 ++++++++----------- 8 files changed, 102 insertions(+), 106 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5125d158ec..d9c2c01bea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,7 +25,7 @@ Test: - Other changes: - - + - Rework edition of event management Changes in Element 1.1.0 (2021-02-19) =================================================== diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmSessionStoreMigration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmSessionStoreMigration.kt index 57002b5a60..820588b1ab 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmSessionStoreMigration.kt +++ b/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) + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt index c2b729e593..e262b40419 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt +++ b/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] diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventAnnotationsSummaryMapper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventAnnotationsSummaryMapper.kt index 9ed2664068..f4d3d74150 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventAnnotationsSummaryMapper.kt +++ b/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), - it.sourceEvents.toList(), - it.sourceLocalEchoEvents.toList(), - it.lastEditTs + ContentMapper.map(latestEdition?.content), + it.editions.filter { editionOfEvent -> !editionOfEvent.isLocalEcho } + .map { editionOfEvent -> editionOfEvent.eventId }, + 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().apply { addAll(it.sourceEvents) }, - RealmList().apply { addAll(it.localEchos) }, - it.lastEditTs - ) - } - eventAnnotationsSummaryEntity.reactionsSummary = annotationsSummary.reactionsSummary.let { - RealmList().apply { - addAll(it.map { - ReactionAggregatedSummaryEntity( - it.key, - it.count, - it.addedByMe, - it.firstTimestamp, - RealmList().apply { addAll(it.sourceEvents) }, - RealmList().apply { addAll(it.localEchoEvents) } - ) - }) - } - } - eventAnnotationsSummaryEntity.referencesSummaryEntity = annotationsSummary.referencesAggregatedSummary?.let { - ReferencesAggregatedSummaryEntity( - it.eventId, - ContentMapper.map(it.content), - RealmList().apply { addAll(it.sourceEvents) }, - RealmList().apply { addAll(it.localEchos) } - ) - } - eventAnnotationsSummaryEntity.pollResponseSummary = annotationsSummary.pollResponseSummary?.let { - PollResponseAggregatedSummaryEntityMapper.map(it) - } - return eventAnnotationsSummaryEntity - } } internal fun EventAnnotationsSummaryEntity.asDomain(): EventAnnotationsSummary { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EditAggregatedSummaryEntity.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EditAggregatedSummaryEntity.kt index 604afc1ab1..0ed927a6b8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EditAggregatedSummaryEntity.kt +++ b/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 eventIDs used to build the summary (might be out of sync if chunked received from message chunk) - var sourceEvents: RealmList = RealmList(), - var sourceLocalEchoEvents: RealmList = RealmList(), - var lastEditTs: Long = 0 + // The list of the editions used to build the summary (might be out of sync if chunked received from message chunk) + var editions: RealmList = RealmList() ) : 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() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EventAnnotationsSummaryEntity.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EventAnnotationsSummaryEntity.kt index 33f26d439f..3e88130420 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EventAnnotationsSummaryEntity.kt +++ b/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 } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/SessionRealmModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/SessionRealmModule.kt index bca2c42c9e..6e6096cf8a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/SessionRealmModule.kt +++ b/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, diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/EventRelationsAggregationProcessor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/EventRelationsAggregationProcessor.kt index c274ccb244..8269643d39 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/EventRelationsAggregationProcessor.kt +++ b/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) - if (isLocalEcho) { - editSummary.lastEditTs = 0 - editSummary.sourceLocalEchoEvents.add(eventId) - } else { - editSummary.lastEditTs = event.originServerTs ?: 0 - editSummary.sourceEvents.add(eventId) - } + editSummary.editions.add( + EditionOfEvent( + senderId = event.senderId ?: "", + eventId = event.eventId, + content = ContentMapper.map(newContent), + timestamp = if (isLocalEcho) 0 else event.originServerTs ?: 0, + 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.sourceEvents.add(event.eventId) - } else if ( - isLocalEcho // do not rely on ts for local echo, take it - || 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) + existingSummary.editions.firstOrNull { it.eventId == txId }?.let { + it.eventId = event.eventId + it.timestamp = event.originServerTs ?: System.currentTimeMillis() + it.isLocalEcho = false } } else { - // ignore this event for the summary (back paginate) - if (!isLocalEcho) { - existingSummary.sourceEvents.add(eventId) - } - Timber.v("###REPLACE ignoring event for summary, it's to old $eventId") + Timber.v("###REPLACE Computing aggregated edit summary (isLocalEcho:$isLocalEcho)") + existingSummary.editions.add( + EditionOfEvent( + senderId = event.senderId ?: "", + 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 = sourceEvents?.indexOf(redacted.eventId) + val sourceToDiscard = eventSummary.editSummary?.editions?.firstOrNull {it.eventId == 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 - sourceEvents.removeAt(sourceToDiscard) - 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()?.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 - } - } + // Need to remove this event from the edition list + sourceToDiscard.deleteFromRealm() } private fun handleReactionRedact(eventToPrune: EventEntity, realm: Realm, userId: String) {