code review

element-hq · Nov 23, 2022 · 0feb4ed · 0feb4ed
1 parent 6d56967
commit 0feb4ed
Show file tree

Hide file tree

Showing 14 changed files with 366 additions and 100 deletions.
diff --git a/...roid/src/main/java/org/matrix/android/sdk/api/session/events/model/AggregatedRelations.kt b/...roid/src/main/java/org/matrix/android/sdk/api/session/events/model/AggregatedRelations.kt
@@ -19,7 +19,7 @@ import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 
 /**
- * <code>
+ * ```
  *  {
  *       "m.annotation": {
  *          "chunk": [
@@ -43,7 +43,7 @@ import com.squareup.moshi.JsonClass
  *           "count": 1
  *           }
  *      }
- * </code>
+ * ```
  */
 
 @JsonClass(generateAdapter = true)

diff --git a/...roid/src/main/java/org/matrix/android/sdk/api/session/events/model/ValidDecryptedEvent.kt b/...roid/src/main/java/org/matrix/android/sdk/api/session/events/model/ValidDecryptedEvent.kt
@@ -16,6 +16,9 @@
 
 package org.matrix.android.sdk.api.session.events.model
 
+import org.matrix.android.sdk.api.session.room.model.message.MessageRelationContent
+import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent
+
 data class ValidDecryptedEvent(
         val type: String,
         val eventId: String,
@@ -29,25 +32,6 @@ data class ValidDecryptedEvent(
         val algorithm: String,
 )
 
-fun Event.toValidDecryptedEvent(): ValidDecryptedEvent? {
-    if (!this.isEncrypted()) return null
-    val decryptedContent = this.getDecryptedContent() ?: return null
-    val eventId = this.eventId ?: return null
-    val roomId = this.roomId ?: return null
-    val type = this.getDecryptedType() ?: return null
-    val senderKey = this.getSenderKey() ?: return null
-    val algorithm = this.content?.get("algorithm") as? String ?: return null
-
-    return ValidDecryptedEvent(
-            type = type,
-            eventId = eventId,
-            clearContent = decryptedContent,
-            prevContent = this.prevContent,
-            originServerTs = this.originServerTs ?: 0,
-            cryptoSenderKey = senderKey,
-            roomId = roomId,
-            unsignedData = this.unsignedData,
-            redacts = this.redacts,
-            algorithm = algorithm
-    )
+fun ValidDecryptedEvent.getRelationContent(): RelationDefaultContent? {
+    return clearContent.toModel<MessageRelationContent?>()?.relatesTo
 }
diff --git a/...java/org/matrix/android/sdk/internal/database/mapper/EditAggregatedSummaryEntityMapper.kt b/...java/org/matrix/android/sdk/internal/database/mapper/EditAggregatedSummaryEntityMapper.kt
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2020 The Matrix.org Foundation C.I.C.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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 org.matrix.android.sdk.internal.database.mapper
+
+import org.matrix.android.sdk.api.session.room.model.EditAggregatedSummary
+import org.matrix.android.sdk.internal.database.model.EditAggregatedSummaryEntity
+import org.matrix.android.sdk.internal.database.model.EditionOfEvent
+
+internal object EditAggregatedSummaryEntityMapper {
+
+    fun map(summary: EditAggregatedSummaryEntity?): EditAggregatedSummary? {
+        summary ?: return null
+        /**
+         * The most recent event is determined by comparing origin_server_ts;
+         * if two or more replacement events have identical origin_server_ts,
+         * the event with the lexicographically largest event_id is treated as more recent.
+         */
+        val latestEdition = summary.editions.sortedWith(compareBy<EditionOfEvent> { it.timestamp }.thenBy { it.eventId })
+                .lastOrNull() ?: return null
+        val editEvent = latestEdition.event
+
+        return EditAggregatedSummary(
+                latestEdit = editEvent?.asDomain(),
+                sourceEvents = summary.editions.filter { editionOfEvent -> !editionOfEvent.isLocalEcho }
+                        .map { editionOfEvent -> editionOfEvent.eventId },
+                localEchos = summary.editions.filter { editionOfEvent -> editionOfEvent.isLocalEcho }
+                        .map { editionOfEvent -> editionOfEvent.eventId },
+                lastEditTs = latestEdition.timestamp
+        )
+    }
+}
diff --git a/...ain/java/org/matrix/android/sdk/internal/database/mapper/EventAnnotationsSummaryMapper.kt b/...ain/java/org/matrix/android/sdk/internal/database/mapper/EventAnnotationsSummaryMapper.kt
@@ -16,11 +16,9 @@
 
 package org.matrix.android.sdk.internal.database.mapper
 
-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.EditionOfEvent
 import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntity
 
 internal object EventAnnotationsSummaryMapper {
@@ -36,27 +34,7 @@ internal object EventAnnotationsSummaryMapper {
                             it.sourceLocalEcho.toList()
                     )
                 },
-                editSummary = annotationsSummary.editSummary
-                        ?.let { summary ->
-                            /**
-                             * The most recent event is determined by comparing origin_server_ts;
-                             * if two or more replacement events have identical origin_server_ts,
-                             * the event with the lexicographically largest event_id is treated as more recent.
-                             */
-                            val latestEdition = summary.editions.sortedWith(compareBy<EditionOfEvent> { it.timestamp }.thenBy { it.eventId })
-                                    .lastOrNull() ?: return@let null
-                            // get the event and validate?
-                            val editEvent = latestEdition.event
-
-                            EditAggregatedSummary(
-                                    latestEdit = editEvent?.asDomain(),
-                                    sourceEvents = summary.editions.filter { editionOfEvent -> !editionOfEvent.isLocalEcho }
-                                            .map { editionOfEvent -> editionOfEvent.eventId },
-                                    localEchos = summary.editions.filter { editionOfEvent -> editionOfEvent.isLocalEcho }
-                                            .map { editionOfEvent -> editionOfEvent.eventId },
-                                    lastEditTs = latestEdition.timestamp
-                            )
-                        },
+                editSummary = EditAggregatedSummaryEntityMapper.map(annotationsSummary.editSummary),
                 referencesAggregatedSummary = annotationsSummary.referencesSummaryEntity?.let {
                     ReferencesAggregatedSummary(
                             ContentMapper.map(it.content),

diff --git a/...d/src/main/java/org/matrix/android/sdk/internal/database/migration/MigrateSessionTo008.kt b/...d/src/main/java/org/matrix/android/sdk/internal/database/migration/MigrateSessionTo008.kt
@@ -25,11 +25,11 @@ internal class MigrateSessionTo008(realm: DynamicRealm) : RealmMigrator(realm, 8
 
     override fun doMigrate(realm: DynamicRealm) {
         val editionOfEventSchema = realm.schema.create("EditionOfEvent")
-                .addField("content"/**EditionOfEventFields.CONTENT*/, String::class.java)
+                .addField("content", String::class.java)
                 .addField(EditionOfEventFields.EVENT_ID, String::class.java)
                 .setRequired(EditionOfEventFields.EVENT_ID, true)
-                .addField("senderId" /*EditionOfEventFields.SENDER_ID*/, String::class.java)
-                .setRequired("senderId" /*EditionOfEventFields.SENDER_ID*/, true)
+                .addField("senderId", String::class.java)
+                .setRequired("senderId", true)
                 .addField(EditionOfEventFields.TIMESTAMP, Long::class.java)
                 .addField(EditionOfEventFields.IS_LOCAL_ECHO, Boolean::class.java)
 

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/events/EventExt.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/events/EventExt.kt
@@ -17,6 +17,7 @@
 package org.matrix.android.sdk.internal.session.events
 
 import org.matrix.android.sdk.api.session.events.model.Event
+import org.matrix.android.sdk.api.session.events.model.ValidDecryptedEvent
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
 
@@ -33,3 +34,32 @@ internal fun Event.getFixedRoomMemberContent(): RoomMemberContent? {
         content
     }
 }
+
+fun Event.toValidDecryptedEvent(): ValidDecryptedEvent? {
+    if (!this.isEncrypted()) return null
+    val decryptedContent = this.getDecryptedContent() ?: return null
+    val eventId = this.eventId ?: return null
+    val roomId = this.roomId ?: return null
+    val type = this.getDecryptedType() ?: return null
+    val senderKey = this.getSenderKey() ?: return null
+    val algorithm = this.content?.get("algorithm") as? String ?: return null
+
+    // copy the relation as it's in clear in the encrypted content
+    val updatedContent = this.content.get("m.relates_to")?.let {
+        decryptedContent.toMutableMap().apply {
+            put("m.relates_to", it)
+        }
+    } ?: decryptedContent
+    return ValidDecryptedEvent(
+            type = type,
+            eventId = eventId,
+            clearContent = updatedContent,
+            prevContent = this.prevContent,
+            originServerTs = this.originServerTs ?: 0,
+            cryptoSenderKey = senderKey,
+            roomId = roomId,
+            unsignedData = this.unsignedData,
+            redacts = this.redacts,
+            algorithm = algorithm
+    )
+}
diff --git a/...-android/src/main/java/org/matrix/android/sdk/internal/session/room/EventEditValidator.kt b/...-android/src/main/java/org/matrix/android/sdk/internal/session/room/EventEditValidator.kt
@@ -17,11 +17,14 @@
 package org.matrix.android.sdk.internal.session.room
 
 import org.matrix.android.sdk.api.session.events.model.Event
+import org.matrix.android.sdk.api.session.events.model.LocalEcho
 import org.matrix.android.sdk.api.session.events.model.RelationType
+import org.matrix.android.sdk.api.session.events.model.getRelationContent
 import org.matrix.android.sdk.api.session.events.model.toModel
-import org.matrix.android.sdk.api.session.events.model.toValidDecryptedEvent
 import org.matrix.android.sdk.api.session.room.model.message.MessageContent
 import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
+import org.matrix.android.sdk.internal.session.events.toValidDecryptedEvent
+import timber.log.Timber
 import javax.inject.Inject
 
 internal class EventEditValidator @Inject constructor(val cryptoStore: IMXCryptoStore) {
@@ -47,12 +50,18 @@ internal class EventEditValidator @Inject constructor(val cryptoStore: IMXCrypto
      * If the original event was encrypted, the replacement should be too.
      */
     fun validateEdit(originalEvent: Event?, replaceEvent: Event): EditValidity {
+        Timber.v("###REPLACE valide event $originalEvent replaced $replaceEvent")
         // we might not know the original event at that time. In this case we can't perform the validation
         // Edits should be revalidated when the original event is received
         if (originalEvent == null) {
             return EditValidity.Unknown
         }
 
+        if (LocalEcho.isLocalEchoId(replaceEvent.eventId.orEmpty())) {
+            // Don't validate local echo
+            return EditValidity.Unknown
+        }
+
         if (originalEvent.roomId != replaceEvent.roomId) {
             return EditValidity.Invalid("original event and replacement event must have the same room_id")
         }
@@ -71,7 +80,7 @@ internal class EventEditValidator @Inject constructor(val cryptoStore: IMXCrypto
             val originalCryptoSenderId = cryptoStore.deviceWithIdentityKey(originalDecrypted.cryptoSenderKey)?.userId
             val editCryptoSenderId = cryptoStore.deviceWithIdentityKey(replaceDecrypted.cryptoSenderKey)?.userId
 
-            if (originalDecrypted.clearContent.toModel<MessageContent>()?.relatesTo?.type == RelationType.REPLACE) {
+            if (originalDecrypted.getRelationContent()?.type == RelationType.REPLACE) {
                 return EditValidity.Invalid("The original event must not, itself, have a rel_type of m.replace ")
             }
 
@@ -96,7 +105,7 @@ internal class EventEditValidator @Inject constructor(val cryptoStore: IMXCrypto
                 return EditValidity.Invalid("replacement event must have an m.new_content property")
             }
         } else {
-            if (originalEvent.content.toModel<MessageContent>()?.relatesTo?.type == RelationType.REPLACE) {
+            if (originalEvent.getRelationContent()?.type == RelationType.REPLACE) {
                 return EditValidity.Invalid("The original event must not, itself, have a rel_type of m.replace ")
             }
 

diff --git a/...n/java/org/matrix/android/sdk/internal/session/room/EventRelationsAggregationProcessor.kt b/...n/java/org/matrix/android/sdk/internal/session/room/EventRelationsAggregationProcessor.kt
@@ -23,6 +23,7 @@ import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.events.model.EventType
 import org.matrix.android.sdk.api.session.events.model.LocalEcho
 import org.matrix.android.sdk.api.session.events.model.RelationType
+import org.matrix.android.sdk.api.session.events.model.content.EncryptedEventContent
 import org.matrix.android.sdk.api.session.events.model.getRelationContent
 import org.matrix.android.sdk.api.session.events.model.toContent
 import org.matrix.android.sdk.api.session.events.model.toModel
@@ -81,13 +82,12 @@ internal class EventRelationsAggregationProcessor @Inject constructor(
             EventType.REDACTION,
             EventType.REACTION,
             // The aggregator handles verification events but just to render tiles in the timeline
-            // It's not participating in verfication it's self, just timeline display
+            // It's not participating in verification itself, just timeline display
             EventType.KEY_VERIFICATION_DONE,
             EventType.KEY_VERIFICATION_CANCEL,
             EventType.KEY_VERIFICATION_ACCEPT,
             EventType.KEY_VERIFICATION_START,
             EventType.KEY_VERIFICATION_MAC,
-            // TODO Add ?
             EventType.KEY_VERIFICATION_READY,
             EventType.KEY_VERIFICATION_KEY,
             EventType.ENCRYPTED
@@ -168,28 +168,28 @@ internal class EventRelationsAggregationProcessor @Inject constructor(
                 // As for now Live event processors are not receiving UTD events.
                 // They will get an update if the event is decrypted later
                 EventType.ENCRYPTED -> {
-//                    // Relation type is in clear, it might be possible to do some things?
-//                    // Notice that if the event is decrypted later, process be called again
-//                    val encryptedEventContent = event.content.toModel<EncryptedEventContent>()
-//                    when (encryptedEventContent?.relatesTo?.type) {
-//                        RelationType.REPLACE -> {
-//                            Timber.v("###REPLACE in room $roomId for event ${event.eventId}")
-//                            // A replace!
-//                            handleReplace(realm, event, roomId, isLocalEcho, encryptedEventContent.relatesTo.eventId)
-//                        }
-//                        RelationType.RESPONSE -> {
-//                            // can we / should we do we something for UTD response??
-//                            Timber.w("## UTD response in room $roomId related to ${encryptedEventContent.relatesTo.eventId}")
-//                        }
-//                        RelationType.REFERENCE -> {
-//                            // can we / should we do we something for UTD reference??
-//                            Timber.w("## UTD reference in room $roomId related to ${encryptedEventContent.relatesTo.eventId}")
-//                        }
-//                        RelationType.ANNOTATION -> {
-//                            // can we / should we do we something for UTD annotation??
-//                            Timber.w("## UTD annotation in room $roomId related to ${encryptedEventContent.relatesTo.eventId}")
-//                        }
-//                    }
+                    // Relation type is in clear, it might be possible to do some things?
+                    // Notice that if the event is decrypted later, process be called again
+                    val encryptedEventContent = event.content.toModel<EncryptedEventContent>()
+                    when (encryptedEventContent?.relatesTo?.type) {
+                        RelationType.REPLACE -> {
+                            Timber.v("###REPLACE in room $roomId for event ${event.eventId}")
+                            // A replace!
+                            handleReplace(realm, event, roomId, isLocalEcho, encryptedEventContent.relatesTo.eventId)
+                        }
+                        RelationType.RESPONSE -> {
+                            // can we / should we do we something for UTD response??
+                            Timber.w("## UTD response in room $roomId related to ${encryptedEventContent.relatesTo.eventId}")
+                        }
+                        RelationType.REFERENCE -> {
+                            // can we / should we do we something for UTD reference??
+                            Timber.w("## UTD reference in room $roomId related to ${encryptedEventContent.relatesTo.eventId}")
+                        }
+                        RelationType.ANNOTATION -> {
+                            // can we / should we do we something for UTD annotation??
+                            Timber.w("## UTD annotation in room $roomId related to ${encryptedEventContent.relatesTo.eventId}")
+                        }
+                    }
                 }
                 EventType.REDACTION -> {
                     val eventToPrune = event.redacts?.let { EventEntity.where(realm, eventId = it).findFirst() }
@@ -256,7 +256,7 @@ internal class EventRelationsAggregationProcessor @Inject constructor(
             relatedEventId: String?
     ) {
         val eventId = event.eventId ?: return
-        val targetEventId = relatedEventId ?: return // ?: content.relatesTo?.eventId ?: return
+        val targetEventId = relatedEventId ?: return
         val editedEvent = EventEntity.where(realm, targetEventId).findFirst()
 
         when (val validity = editValidator.validateEdit(editedEvent?.asDomain(), event)) {
@@ -301,15 +301,16 @@ internal class EventRelationsAggregationProcessor @Inject constructor(
                 // ok it has already been managed
                 Timber.v("###REPLACE Receiving remote echo of edit (edit already done)")
                 existingSummary.editions.firstOrNull { it.eventId == txId }?.let {
-                    it.eventId = event.eventId
+                    it.eventId = eventId
                     it.timestamp = event.originServerTs ?: clock.epochMillis()
                     it.isLocalEcho = false
+                    it.event = EventEntity.where(realm, eventId).findFirst()
                 }
             } else {
                 Timber.v("###REPLACE Computing aggregated edit summary (isLocalEcho:$isLocalEcho)")
                 existingSummary.editions.add(
                         EditionOfEvent(
-                                eventId = event.eventId,
+                                eventId = eventId,
                                 event = EventEntity.where(realm, eventId).findFirst(),
                                 timestamp = if (isLocalEcho) {
                                     clock.epochMillis()
@@ -343,7 +344,8 @@ internal class EventRelationsAggregationProcessor @Inject constructor(
      * @param editions list of edition of event
      */
     private fun handleThreadSummaryEdition(
-            editedEvent: EventEntity?, replaceEvent: TimelineEventEntity?,
+            editedEvent: EventEntity?,
+            replaceEvent: TimelineEventEntity?,
             editions: List<EditionOfEvent>?
     ) {
         replaceEvent ?: return