From de90cbe73e675fc4afadf4b08c1f49f6954370ed Mon Sep 17 00:00:00 2001
From: ganfra <francoisg@matrix.org>
Date: Mon, 7 Jan 2019 19:38:49 +0100
Subject: [PATCH] Timeline : fix merging issues

---
 .../session/room/timeline/ChunkEntityTest.kt  | 18 ++++----
 .../database/helper/ChunkEntityHelper.kt      | 41 ++++++++++---------
 .../room/timeline/TokenChunkEventPersistor.kt | 17 +++++---
 3 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/ChunkEntityTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/ChunkEntityTest.kt
index 181f0c1ed9..7460f5b535 100644
--- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/ChunkEntityTest.kt
+++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/ChunkEntityTest.kt
@@ -4,7 +4,11 @@ import com.zhuinden.monarchy.Monarchy
 import im.vector.matrix.android.InstrumentedTest
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.session.events.model.EventType
-import im.vector.matrix.android.internal.database.helper.*
+import im.vector.matrix.android.internal.database.helper.add
+import im.vector.matrix.android.internal.database.helper.addAll
+import im.vector.matrix.android.internal.database.helper.isUnlinked
+import im.vector.matrix.android.internal.database.helper.lastStateIndex
+import im.vector.matrix.android.internal.database.helper.merge
 import im.vector.matrix.android.internal.database.model.ChunkEntity
 import im.vector.matrix.android.internal.session.room.timeline.PaginationDirection
 import io.realm.Realm
@@ -102,7 +106,7 @@ internal class ChunkEntityTest : InstrumentedTest {
             val chunk2: ChunkEntity = realm.createObject()
             chunk1.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS)
             chunk2.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS)
-            chunk1.merge(chunk2, PaginationDirection.BACKWARDS)
+            chunk1.merge("roomId", chunk2, PaginationDirection.BACKWARDS)
             chunk1.events.size shouldEqual 60
         }
     }
@@ -118,7 +122,7 @@ internal class ChunkEntityTest : InstrumentedTest {
             chunk2.isLast = false
             chunk1.addAll("roomId", eventsForChunk1, PaginationDirection.FORWARDS)
             chunk2.addAll("roomId", eventsForChunk2, PaginationDirection.BACKWARDS)
-            chunk1.merge(chunk2, PaginationDirection.BACKWARDS)
+            chunk1.merge("roomId", chunk2, PaginationDirection.BACKWARDS)
             chunk1.events.size shouldEqual 40
             chunk1.isLast.shouldBeTrue()
         }
@@ -131,7 +135,7 @@ internal class ChunkEntityTest : InstrumentedTest {
             val chunk2: ChunkEntity = realm.createObject()
             chunk1.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
             chunk2.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = false)
-            chunk1.merge(chunk2, PaginationDirection.BACKWARDS)
+            chunk1.merge("roomId", chunk2, PaginationDirection.BACKWARDS)
             chunk1.isUnlinked().shouldBeFalse()
         }
     }
@@ -143,7 +147,7 @@ internal class ChunkEntityTest : InstrumentedTest {
             val chunk2: ChunkEntity = realm.createObject()
             chunk1.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
             chunk2.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
-            chunk1.merge(chunk2, PaginationDirection.BACKWARDS)
+            chunk1.merge("roomId", chunk2, PaginationDirection.BACKWARDS)
             chunk1.isUnlinked().shouldBeTrue()
         }
     }
@@ -157,7 +161,7 @@ internal class ChunkEntityTest : InstrumentedTest {
             chunk1.prevToken = prevToken
             chunk1.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
             chunk2.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
-            chunk1.merge(chunk2, PaginationDirection.FORWARDS)
+            chunk1.merge("roomId", chunk2, PaginationDirection.FORWARDS)
             chunk1.prevToken shouldEqual prevToken
         }
     }
@@ -171,7 +175,7 @@ internal class ChunkEntityTest : InstrumentedTest {
             chunk1.nextToken = nextToken
             chunk1.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
             chunk2.addAll("roomId", createFakeListOfEvents(30), PaginationDirection.BACKWARDS, isUnlinked = true)
-            chunk1.merge(chunk2, PaginationDirection.BACKWARDS)
+            chunk1.merge("roomId", chunk2, PaginationDirection.BACKWARDS)
             chunk1.nextToken shouldEqual nextToken
         }
     }
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/helper/ChunkEntityHelper.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/helper/ChunkEntityHelper.kt
index 519e69e1c9..514a015825 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/helper/ChunkEntityHelper.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/helper/ChunkEntityHelper.kt
@@ -2,6 +2,7 @@ package im.vector.matrix.android.internal.database.helper
 
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.session.events.model.EventType
+import im.vector.matrix.android.internal.database.mapper.asDomain
 import im.vector.matrix.android.internal.database.mapper.toEntity
 import im.vector.matrix.android.internal.database.mapper.updateWith
 import im.vector.matrix.android.internal.database.model.ChunkEntity
@@ -12,18 +13,21 @@ import im.vector.matrix.android.internal.session.room.timeline.PaginationDirecti
 import io.realm.Sort
 
 internal fun ChunkEntity.deleteOnCascade() {
+    assertIsManaged()
     this.events.deleteAllFromRealm()
     this.deleteFromRealm()
 }
 
 // By default if a chunk is empty we consider it unlinked
 internal fun ChunkEntity.isUnlinked(): Boolean {
+    assertIsManaged()
     return events.where().equalTo(EventEntityFields.IS_UNLINKED, false).findAll().isEmpty()
 }
 
-internal fun ChunkEntity.merge(chunkToMerge: ChunkEntity,
+internal fun ChunkEntity.merge(roomId: String,
+                               chunkToMerge: ChunkEntity,
                                direction: PaginationDirection) {
-
+    assertIsManaged()
     val isChunkToMergeUnlinked = chunkToMerge.isUnlinked()
     val isCurrentChunkUnlinked = this.isUnlinked()
     val isUnlinked = isCurrentChunkUnlinked && isChunkToMergeUnlinked
@@ -41,7 +45,7 @@ internal fun ChunkEntity.merge(chunkToMerge: ChunkEntity,
         eventsToMerge = chunkToMerge.events
     }
     eventsToMerge.forEach {
-        add(it, direction, isUnlinked = isUnlinked)
+        add(roomId, it.asDomain(), direction, isUnlinked = isUnlinked)
     }
 }
 
@@ -50,7 +54,7 @@ internal fun ChunkEntity.addAll(roomId: String,
                                 direction: PaginationDirection,
                                 stateIndexOffset: Int = 0,
                                 isUnlinked: Boolean = false) {
-
+    assertIsManaged()
     events.forEach { event ->
         add(roomId, event, direction, stateIndexOffset, isUnlinked)
     }
@@ -66,22 +70,12 @@ internal fun ChunkEntity.add(roomId: String,
                              stateIndexOffset: Int = 0,
                              isUnlinked: Boolean = false) {
 
-    add(event.toEntity(roomId), direction, stateIndexOffset, isUnlinked)
-}
-
-internal fun ChunkEntity.add(eventEntity: EventEntity,
-                             direction: PaginationDirection,
-                             stateIndexOffset: Int = 0,
-                             isUnlinked: Boolean = false) {
-    if (!isManaged) {
-        throw IllegalStateException("Chunk entity should be managed to use fast contains")
-    }
-
-    if (eventEntity.eventId.isEmpty() || events.fastContains(eventEntity.eventId)) {
+    assertIsManaged()
+    if (event.eventId.isNullOrEmpty() || events.fastContains(event.eventId)) {
         return
     }
     var currentStateIndex = lastStateIndex(direction, defaultValue = stateIndexOffset)
-    if (direction == PaginationDirection.FORWARDS && EventType.isStateEvent(eventEntity.type)) {
+    if (direction == PaginationDirection.FORWARDS && EventType.isStateEvent(event.type)) {
         currentStateIndex += 1
     } else if (direction == PaginationDirection.BACKWARDS && events.isNotEmpty()) {
         val lastEventType = events.last()?.type ?: ""
@@ -89,14 +83,21 @@ internal fun ChunkEntity.add(eventEntity: EventEntity,
             currentStateIndex -= 1
         }
     }
+    val eventEntity = event.toEntity(roomId)
     eventEntity.updateWith(currentStateIndex, isUnlinked)
     val position = if (direction == PaginationDirection.FORWARDS) 0 else this.events.size
     events.add(position, eventEntity)
 }
 
+private fun ChunkEntity.assertIsManaged() {
+    if (!isManaged) {
+        throw IllegalStateException("Chunk entity should be managed to use this function")
+    }
+}
+
 internal fun ChunkEntity.lastStateIndex(direction: PaginationDirection, defaultValue: Int = 0): Int {
     return when (direction) {
-        PaginationDirection.FORWARDS -> events.where().sort(EventEntityFields.STATE_INDEX, Sort.DESCENDING).findFirst()?.stateIndex
-        PaginationDirection.BACKWARDS -> events.where().sort(EventEntityFields.STATE_INDEX, Sort.ASCENDING).findFirst()?.stateIndex
-    } ?: defaultValue
+               PaginationDirection.FORWARDS  -> events.where().sort(EventEntityFields.STATE_INDEX, Sort.DESCENDING).findFirst()?.stateIndex
+               PaginationDirection.BACKWARDS -> events.where().sort(EventEntityFields.STATE_INDEX, Sort.ASCENDING).findFirst()?.stateIndex
+           } ?: defaultValue
 }
\ No newline at end of file
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TokenChunkEventPersistor.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TokenChunkEventPersistor.kt
index cd2b486842..68f26bcc53 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TokenChunkEventPersistor.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TokenChunkEventPersistor.kt
@@ -2,7 +2,12 @@ package im.vector.matrix.android.internal.session.room.timeline
 
 import arrow.core.Try
 import com.zhuinden.monarchy.Monarchy
-import im.vector.matrix.android.internal.database.helper.*
+import im.vector.matrix.android.internal.database.helper.addAll
+import im.vector.matrix.android.internal.database.helper.addOrUpdate
+import im.vector.matrix.android.internal.database.helper.addStateEvents
+import im.vector.matrix.android.internal.database.helper.deleteOnCascade
+import im.vector.matrix.android.internal.database.helper.isUnlinked
+import im.vector.matrix.android.internal.database.helper.merge
 import im.vector.matrix.android.internal.database.model.ChunkEntity
 import im.vector.matrix.android.internal.database.model.RoomEntity
 import im.vector.matrix.android.internal.database.query.create
@@ -21,7 +26,7 @@ internal class TokenChunkEventPersistor(private val monarchy: Monarchy) {
         return monarchy
                 .tryTransactionSync { realm ->
                     val roomEntity = RoomEntity.where(realm, roomId).findFirst()
-                            ?: throw IllegalStateException("You shouldn't use this method without a room")
+                                     ?: throw IllegalStateException("You shouldn't use this method without a room")
 
                     val nextToken: String?
                     val prevToken: String?
@@ -41,10 +46,10 @@ internal class TokenChunkEventPersistor(private val monarchy: Monarchy) {
 
                     var currentChunk = if (direction == PaginationDirection.FORWARDS) {
                         prevChunk?.apply { this.nextToken = nextToken }
-                                ?: ChunkEntity.create(realm, prevToken, nextToken)
+                        ?: ChunkEntity.create(realm, prevToken, nextToken)
                     } else {
                         nextChunk?.apply { this.prevToken = prevToken }
-                                ?: ChunkEntity.create(realm, prevToken, nextToken)
+                        ?: ChunkEntity.create(realm, prevToken, nextToken)
                     }
 
                     currentChunk.addAll(roomId, receivedChunk.events, direction, isUnlinked = currentChunk.isUnlinked())
@@ -75,11 +80,11 @@ internal class TokenChunkEventPersistor(private val monarchy: Monarchy) {
 
         // We always merge the bottom chunk into top chunk, so we are always merging backwards
         return if (direction == PaginationDirection.BACKWARDS) {
-            currentChunk.merge(otherChunk, PaginationDirection.BACKWARDS)
+            currentChunk.merge(roomEntity.roomId, otherChunk, PaginationDirection.BACKWARDS)
             roomEntity.deleteOnCascade(otherChunk)
             currentChunk
         } else {
-            otherChunk.merge(currentChunk, PaginationDirection.BACKWARDS)
+            otherChunk.merge(roomEntity.roomId, currentChunk, PaginationDirection.BACKWARDS)
             roomEntity.deleteOnCascade(currentChunk)
             otherChunk
         }