From 2c47fe9f0d959f2d9e97af6d7b8c2b7f69635669 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 29 Apr 2020 11:47:33 +0200 Subject: [PATCH 01/21] typo --- .../session/room/timeline/DefaultTimeline.kt | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt index f2bee734ce..82729f092d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt @@ -383,7 +383,7 @@ internal class DefaultTimeline( } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results * @return true if createSnapshot should be posted */ private fun paginateInternal(startDisplayIndex: Int?, @@ -446,7 +446,7 @@ internal class DefaultTimeline( } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results */ private fun handleInitialLoad() { var shouldFetchInitialEvent = false @@ -478,7 +478,7 @@ internal class DefaultTimeline( } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results */ private fun handleUpdates(results: RealmResults, changeSet: OrderedCollectionChangeSet) { // If changeSet has deletion we are having a gap, so we clear everything @@ -516,7 +516,7 @@ internal class DefaultTimeline( } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results */ private fun executePaginationTask(direction: Timeline.Direction, limit: Int) { val token = getTokenLive(direction) @@ -560,23 +560,22 @@ internal class DefaultTimeline( } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results */ - private fun getTokenLive(direction: Timeline.Direction): String? { val chunkEntity = getLiveChunk() ?: return null return if (direction == Timeline.Direction.BACKWARDS) chunkEntity.prevToken else chunkEntity.nextToken } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results */ private fun getLiveChunk(): ChunkEntity? { return nonFilteredEvents.firstOrNull()?.chunk?.firstOrNull() } /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results * @return number of items who have been added */ private fun buildTimelineEvents(startDisplayIndex: Int?, @@ -628,7 +627,7 @@ internal class DefaultTimeline( ) /** - * This has to be called on TimelineThread as it access realm live results + * This has to be called on TimelineThread as it accesses realm live results */ private fun getOffsetResults(startDisplayIndex: Int, direction: Timeline.Direction, From 2697800deb7a60000ae13707710a170ebd9c7127 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 29 Apr 2020 11:48:40 +0200 Subject: [PATCH 02/21] Doc and cleanup --- .../android/internal/database/helper/ChunkEntityHelper.kt | 7 +++---- .../matrix/android/internal/database/model/ChunkEntity.kt | 2 ++ 2 files changed, 5 insertions(+), 4 deletions(-) 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 80376fb6ee..d86151e694 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 @@ -60,10 +60,9 @@ internal fun ChunkEntity.merge(roomId: String, chunkToMerge: ChunkEntity, direct chunkToMerge.stateEvents.forEach { stateEvent -> addStateEvent(roomId, stateEvent, direction) } - return eventsToMerge - .forEach { - addTimelineEventFromMerge(localRealm, it, direction) - } + eventsToMerge.forEach { + addTimelineEventFromMerge(localRealm, it, direction) + } } internal fun ChunkEntity.addStateEvent(roomId: String, stateEvent: EventEntity, direction: PaginationDirection) { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt index 2d294e6783..9c6bf5f757 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt @@ -23,9 +23,11 @@ import io.realm.annotations.Index import io.realm.annotations.LinkingObjects internal open class ChunkEntity(@Index var prevToken: String? = null, + // Because of gaps we can have several chunks with nextToken == null @Index var nextToken: String? = null, var stateEvents: RealmList = RealmList(), var timelineEvents: RealmList = RealmList(), + // Only one chunk will have isLastForward == true @Index var isLastForward: Boolean = false, @Index var isLastBackward: Boolean = false ) : RealmObject() { From 7e955ef0e4e1b1f0cad74e580dbbe6de33b0452a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 29 Apr 2020 11:49:08 +0200 Subject: [PATCH 03/21] Add possibility to create clear room --- .../matrix/android/common/CryptoTestHelper.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CryptoTestHelper.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CryptoTestHelper.kt index 9278bed918..e4aa7872aa 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CryptoTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CryptoTestHelper.kt @@ -53,17 +53,19 @@ class CryptoTestHelper(private val mTestHelper: CommonTestHelper) { /** * @return alice session */ - fun doE2ETestWithAliceInARoom(): CryptoTestData { + fun doE2ETestWithAliceInARoom(encryptedRoom: Boolean = true): CryptoTestData { val aliceSession = mTestHelper.createAccount(TestConstants.USER_ALICE, defaultSessionParams) val roomId = mTestHelper.doSync { aliceSession.createRoom(CreateRoomParams(name = "MyRoom"), it) } - val room = aliceSession.getRoom(roomId)!! + if (encryptedRoom) { + val room = aliceSession.getRoom(roomId)!! - mTestHelper.doSync { - room.enableEncryption(callback = it) + mTestHelper.doSync { + room.enableEncryption(callback = it) + } } return CryptoTestData(aliceSession, roomId) @@ -72,8 +74,8 @@ class CryptoTestHelper(private val mTestHelper: CommonTestHelper) { /** * @return alice and bob sessions */ - fun doE2ETestWithAliceAndBobInARoom(): CryptoTestData { - val cryptoTestData = doE2ETestWithAliceInARoom() + fun doE2ETestWithAliceAndBobInARoom(encryptedRoom: Boolean = true): CryptoTestData { + val cryptoTestData = doE2ETestWithAliceInARoom(encryptedRoom) val aliceSession = cryptoTestData.firstSession val aliceRoomId = cryptoTestData.roomId From bfd847179f8b55e77ad7759ac0eff85b56f460eb Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 29 Apr 2020 11:49:42 +0200 Subject: [PATCH 04/21] Wait more --- .../java/im/vector/matrix/android/common/CommonTestHelper.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt index 5bc8653f3d..ad12fb6bba 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt @@ -143,7 +143,8 @@ class CommonTestHelper(context: Context) { for (i in 0 until nbOfMessages) { room.sendTextMessage(message + " #" + (i + 1)) } - await(latch) + // Wait 3 second more per message + await(latch, timeout = TestConstants.timeOutMillis + 3_000L * nbOfMessages) timeline.removeListener(timelineListener) timeline.dispose() From 20b726819ff8136f1dd2aa5e3ffce099166dc85f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Apr 2020 17:35:48 +0200 Subject: [PATCH 05/21] Rename "LastLive" -> "LastForward" --- .../android/internal/database/query/ChunkEntityQueries.kt | 2 +- .../matrix/android/internal/database/query/ReadQueries.kt | 2 +- .../internal/database/query/TimelineEventEntityQueries.kt | 2 +- .../session/room/timeline/TokenChunkEventPersistor.kt | 8 ++++---- .../android/internal/session/sync/RoomSyncHandler.kt | 5 +++-- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ChunkEntityQueries.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ChunkEntityQueries.kt index 009ee4b7fe..5efb84a105 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ChunkEntityQueries.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ChunkEntityQueries.kt @@ -41,7 +41,7 @@ internal fun ChunkEntity.Companion.find(realm: Realm, roomId: String, prevToken: return query.findFirst() } -internal fun ChunkEntity.Companion.findLastLiveChunkFromRoom(realm: Realm, roomId: String): ChunkEntity? { +internal fun ChunkEntity.Companion.findLastForwardChunkOfRoom(realm: Realm, roomId: String): ChunkEntity? { return where(realm, roomId) .equalTo(ChunkEntityFields.IS_LAST_FORWARD, true) .findFirst() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ReadQueries.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ReadQueries.kt index 1b83577a8c..9c73dff1dd 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ReadQueries.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/ReadQueries.kt @@ -36,7 +36,7 @@ internal fun isEventRead(monarchy: Monarchy, var isEventRead = false monarchy.doWithRealm { realm -> - val liveChunk = ChunkEntity.findLastLiveChunkFromRoom(realm, roomId) ?: return@doWithRealm + val liveChunk = ChunkEntity.findLastForwardChunkOfRoom(realm, roomId) ?: return@doWithRealm val eventToCheck = liveChunk.timelineEvents.find(eventId) isEventRead = if (eventToCheck == null || eventToCheck.root?.sender == userId) { true diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/TimelineEventEntityQueries.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/TimelineEventEntityQueries.kt index 5168d0728e..c3e9a8288b 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/TimelineEventEntityQueries.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/query/TimelineEventEntityQueries.kt @@ -59,7 +59,7 @@ internal fun TimelineEventEntity.Companion.latestEvent(realm: Realm, filterTypes: List = emptyList()): TimelineEventEntity? { val roomEntity = RoomEntity.where(realm, roomId).findFirst() ?: return null val sendingTimelineEvents = roomEntity.sendingTimelineEvents.where().filterTypes(filterTypes) - val liveEvents = ChunkEntity.findLastLiveChunkFromRoom(realm, roomId)?.timelineEvents?.where()?.filterTypes(filterTypes) + val liveEvents = ChunkEntity.findLastForwardChunkOfRoom(realm, roomId)?.timelineEvents?.where()?.filterTypes(filterTypes) if (filterContentRelation) { liveEvents ?.not()?.like(TimelineEventEntityFields.ROOT.CONTENT, FilterContent.EDIT_TYPE) 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 164626224b..f6fd88f816 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 @@ -35,7 +35,7 @@ import im.vector.matrix.android.internal.database.query.copyToRealmOrIgnore import im.vector.matrix.android.internal.database.query.create import im.vector.matrix.android.internal.database.query.find import im.vector.matrix.android.internal.database.query.findAllIncludingEvents -import im.vector.matrix.android.internal.database.query.findLastLiveChunkFromRoom +import im.vector.matrix.android.internal.database.query.findLastForwardChunkOfRoom import im.vector.matrix.android.internal.database.query.getOrCreate import im.vector.matrix.android.internal.database.query.latestEvent import im.vector.matrix.android.internal.database.query.where @@ -169,10 +169,10 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy private fun handleReachEnd(realm: Realm, roomId: String, direction: PaginationDirection, currentChunk: ChunkEntity) { Timber.v("Reach end of $roomId") if (direction == PaginationDirection.FORWARDS) { - val currentLiveChunk = ChunkEntity.findLastLiveChunkFromRoom(realm, roomId) - if (currentChunk != currentLiveChunk) { + val currentLastForwardChunk = ChunkEntity.findLastForwardChunkOfRoom(realm, roomId) + if (currentChunk != currentLastForwardChunk) { currentChunk.isLastForward = true - currentLiveChunk?.deleteOnCascade() + currentLastForwardChunk?.deleteOnCascade() RoomSummaryEntity.where(realm, roomId).findFirst()?.apply { latestPreviewableEvent = TimelineEventEntity.latestEvent( realm, diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/RoomSyncHandler.kt index 70c1e39334..8c21d23a8c 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/RoomSyncHandler.kt @@ -36,7 +36,7 @@ import im.vector.matrix.android.internal.database.model.CurrentStateEventEntity import im.vector.matrix.android.internal.database.model.RoomEntity import im.vector.matrix.android.internal.database.query.copyToRealmOrIgnore import im.vector.matrix.android.internal.database.query.find -import im.vector.matrix.android.internal.database.query.findLastLiveChunkFromRoom +import im.vector.matrix.android.internal.database.query.findLastForwardChunkOfRoom import im.vector.matrix.android.internal.database.query.getOrCreate import im.vector.matrix.android.internal.database.query.getOrNull import im.vector.matrix.android.internal.database.query.where @@ -220,12 +220,13 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle prevToken: String? = null, isLimited: Boolean = true, syncLocalTimestampMillis: Long): ChunkEntity { - val lastChunk = ChunkEntity.findLastLiveChunkFromRoom(realm, roomEntity.roomId) + val lastChunk = ChunkEntity.findLastForwardChunkOfRoom(realm, roomEntity.roomId) val chunkEntity = if (!isLimited && lastChunk != null) { lastChunk } else { realm.createObject().apply { this.prevToken = prevToken } } + // Only one chunk has isLastForward set to true lastChunk?.isLastForward = false chunkEntity.isLastForward = true From a61434ae0877d829d51eb72582606a25517415e9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Apr 2020 17:37:58 +0200 Subject: [PATCH 06/21] doc --- .../vector/matrix/android/api/session/room/timeline/Timeline.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/Timeline.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/Timeline.kt index d7d6682046..19ff65dbe2 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/Timeline.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/Timeline.kt @@ -58,7 +58,7 @@ interface Timeline { /** * Check if the timeline can be enriched by paginating. - * @param the direction to check in + * @param direction the direction to check in * @return true if timeline can be enriched */ fun hasMoreToLoad(direction: Direction): Boolean From becc5a7b541680d65ff2b4acf6de4def0cba199f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Apr 2020 17:39:01 +0200 Subject: [PATCH 07/21] Add assertion in debug --- .../android/api/session/room/timeline/TimelineEvent.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt index 0c8a04db36..7adc438e20 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt @@ -16,6 +16,7 @@ package im.vector.matrix.android.api.session.room.timeline +import im.vector.matrix.android.BuildConfig 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.api.session.events.model.RelationType @@ -45,6 +46,12 @@ data class TimelineEvent( val readReceipts: List = emptyList() ) { + init { + if (BuildConfig.DEBUG) { + assert(eventId == root.eventId) + } + } + val metadata = HashMap() /** From 8966e249256d47fd0e81767df9a883d3637b7420 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Apr 2020 18:23:18 +0200 Subject: [PATCH 08/21] Create a debug method to send x times the same event --- .../session/room/send/DefaultSendService.kt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt index 1037b7c79c..eee1c01295 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt @@ -74,6 +74,19 @@ internal class DefaultSendService @AssistedInject constructor( return sendEvent(event) } + // For test only + private fun sendTextMessages(text: CharSequence, msgType: String, autoMarkdown: Boolean, times: Int): Cancelable { + return CancelableBag().apply { + // Send the event several times + repeat(times) { i -> + val event = localEchoEventFactory.createTextEvent(roomId, msgType, "$text - $i", autoMarkdown).also { + createLocalEcho(it) + } + add(sendEvent(event)) + } + } + } + override fun sendFormattedTextMessage(text: String, formattedText: String, msgType: String): Cancelable { val event = localEchoEventFactory.createFormattedTextEvent(roomId, TextContent(text, formattedText), msgType).also { createLocalEcho(it) From f3c3c07d468e04820cc1e465aec1bc5410aef40f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Apr 2020 18:36:43 +0200 Subject: [PATCH 09/21] Kotlin sugar --- .../session/room/send/DefaultSendService.kt | 77 +++++++++---------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt index eee1c01295..9c8723af05 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/DefaultSendService.kt @@ -68,10 +68,9 @@ internal class DefaultSendService @AssistedInject constructor( private val workerFutureListenerExecutor = Executors.newSingleThreadExecutor() override fun sendTextMessage(text: CharSequence, msgType: String, autoMarkdown: Boolean): Cancelable { - val event = localEchoEventFactory.createTextEvent(roomId, msgType, text, autoMarkdown).also { - createLocalEcho(it) - } - return sendEvent(event) + return localEchoEventFactory.createTextEvent(roomId, msgType, text, autoMarkdown) + .also { createLocalEcho(it) } + .let { sendEvent(it) } } // For test only @@ -79,33 +78,30 @@ internal class DefaultSendService @AssistedInject constructor( return CancelableBag().apply { // Send the event several times repeat(times) { i -> - val event = localEchoEventFactory.createTextEvent(roomId, msgType, "$text - $i", autoMarkdown).also { - createLocalEcho(it) - } - add(sendEvent(event)) + localEchoEventFactory.createTextEvent(roomId, msgType, "$text - $i", autoMarkdown) + .also { createLocalEcho(it) } + .let { sendEvent(it) } + .also { add(it) } } } } override fun sendFormattedTextMessage(text: String, formattedText: String, msgType: String): Cancelable { - val event = localEchoEventFactory.createFormattedTextEvent(roomId, TextContent(text, formattedText), msgType).also { - createLocalEcho(it) - } - return sendEvent(event) + return localEchoEventFactory.createFormattedTextEvent(roomId, TextContent(text, formattedText), msgType) + .also { createLocalEcho(it) } + .let { sendEvent(it) } } override fun sendPoll(question: String, options: List): Cancelable { - val event = localEchoEventFactory.createPollEvent(roomId, question, options).also { - createLocalEcho(it) - } - return sendEvent(event) + return localEchoEventFactory.createPollEvent(roomId, question, options) + .also { createLocalEcho(it) } + .let { sendEvent(it) } } override fun sendOptionsReply(pollEventId: String, optionIndex: Int, optionValue: String): Cancelable { - val event = localEchoEventFactory.createOptionsReplyEvent(roomId, pollEventId, optionIndex, optionValue).also { - createLocalEcho(it) - } - return sendEvent(event) + return localEchoEventFactory.createOptionsReplyEvent(roomId, pollEventId, optionIndex, optionValue) + .also { createLocalEcho(it) } + .let { sendEvent(it) } } private fun sendEvent(event: Event): Cancelable { @@ -132,8 +128,8 @@ internal class DefaultSendService @AssistedInject constructor( override fun redactEvent(event: Event, reason: String?): Cancelable { // TODO manage media/attachements? - val redactWork = createRedactEventWork(event, reason) - return timelineSendEventWorkCommon.postWork(roomId, redactWork) + return createRedactEventWork(event, reason) + .let { timelineSendEventWorkCommon.postWork(roomId, it) } } override fun resendTextMessage(localEcho: TimelineEvent): Cancelable? { @@ -276,31 +272,30 @@ internal class DefaultSendService @AssistedInject constructor( private fun createEncryptEventWork(event: Event, startChain: Boolean): OneTimeWorkRequest { // Same parameter - val params = EncryptEventWorker.Params(sessionId, event) - val sendWorkData = WorkerParamsFactory.toData(params) - - return workManagerProvider.matrixOneTimeWorkRequestBuilder() - .setConstraints(WorkManagerProvider.workConstraints) - .setInputData(sendWorkData) - .startChain(startChain) - .setBackoffCriteria(BackoffPolicy.LINEAR, WorkManagerProvider.BACKOFF_DELAY, TimeUnit.MILLISECONDS) - .build() + return EncryptEventWorker.Params(sessionId, event) + .let { WorkerParamsFactory.toData(it) } + .let { + workManagerProvider.matrixOneTimeWorkRequestBuilder() + .setConstraints(WorkManagerProvider.workConstraints) + .setInputData(it) + .startChain(startChain) + .setBackoffCriteria(BackoffPolicy.LINEAR, WorkManagerProvider.BACKOFF_DELAY, TimeUnit.MILLISECONDS) + .build() + } } private fun createSendEventWork(event: Event, startChain: Boolean): OneTimeWorkRequest { - val sendContentWorkerParams = SendEventWorker.Params(sessionId, event) - val sendWorkData = WorkerParamsFactory.toData(sendContentWorkerParams) - - return timelineSendEventWorkCommon.createWork(sendWorkData, startChain) + return SendEventWorker.Params(sessionId, event) + .let { WorkerParamsFactory.toData(it) } + .let { timelineSendEventWorkCommon.createWork(it, startChain) } } private fun createRedactEventWork(event: Event, reason: String?): OneTimeWorkRequest { - val redactEvent = localEchoEventFactory.createRedactEvent(roomId, event.eventId!!, reason).also { - createLocalEcho(it) - } - val sendContentWorkerParams = RedactEventWorker.Params(sessionId, redactEvent.eventId!!, roomId, event.eventId, reason) - val redactWorkData = WorkerParamsFactory.toData(sendContentWorkerParams) - return timelineSendEventWorkCommon.createWork(redactWorkData, true) + return localEchoEventFactory.createRedactEvent(roomId, event.eventId!!, reason) + .also { createLocalEcho(it) } + .let { RedactEventWorker.Params(sessionId, it.eventId!!, roomId, event.eventId, reason) } + .let { WorkerParamsFactory.toData(it) } + .let { timelineSendEventWorkCommon.createWork(it, true) } } private fun createUploadMediaWork(allLocalEchos: List, From 86fba283131ab9b5315694ece8af42ba9f6bde78 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 5 May 2020 02:40:50 +0200 Subject: [PATCH 10/21] After jump to unread, newer messages are never loaded (#1008) --- CHANGES.md | 2 +- .../internal/database/model/ChunkEntity.kt | 3 +++ .../session/room/timeline/DefaultTimeline.kt | 27 ++++++++++++++++++- .../room/timeline/TokenChunkEventPersistor.kt | 9 ++++++- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f0cf30af2d..bc04df2e39 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ Improvements 🙌: - Bugfix 🐛: - - + - After jump to unread, newer messages are never loaded (#1008) Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt index 9c6bf5f757..19bf72970c 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/model/ChunkEntity.kt @@ -34,6 +34,9 @@ internal open class ChunkEntity(@Index var prevToken: String? = null, fun identifier() = "${prevToken}_$nextToken" + // If true, then this chunk was previously a last forward chunk + fun hasBeenALastForwardChunk() = nextToken == null && !isLastForward + @LinkingObjects("chunks") val room: RealmResults? = null diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt index 82729f092d..8cfd498cc8 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt @@ -559,6 +559,28 @@ internal class DefaultTimeline( .executeBy(taskExecutor) } + // For debug purpose only + private fun dumpAndLogChunks() { + val liveChunk = getLiveChunk() + Timber.w("Live chunk: $liveChunk") + + Realm.getInstance(realmConfiguration).use { realm -> + ChunkEntity.where(realm, roomId).findAll() + .also { Timber.w("Found ${it.size} chunks") } + .forEach { + Timber.w("") + Timber.w("ChunkEntity: $it") + Timber.w("prevToken: ${it.prevToken}") + Timber.w("nextToken: ${it.nextToken}") + Timber.w("isLastBackward: ${it.isLastBackward}") + Timber.w("isLastForward: ${it.isLastForward}") + it.timelineEvents.forEach { tle -> + Timber.w(" TLE: ${tle.root?.content}") + } + } + } + } + /** * This has to be called on TimelineThread as it accesses realm live results */ @@ -569,6 +591,7 @@ internal class DefaultTimeline( /** * This has to be called on TimelineThread as it accesses realm live results + * Return the current Chunk */ private fun getLiveChunk(): ChunkEntity? { return nonFilteredEvents.firstOrNull()?.chunk?.firstOrNull() @@ -576,7 +599,7 @@ internal class DefaultTimeline( /** * This has to be called on TimelineThread as it accesses realm live results - * @return number of items who have been added + * @return the number of items who have been added */ private fun buildTimelineEvents(startDisplayIndex: Int?, direction: Timeline.Direction, @@ -617,6 +640,8 @@ internal class DefaultTimeline( } val time = System.currentTimeMillis() - start Timber.v("Built ${offsetResults.size} items from db in $time ms") + // For the case where wo reach the lastForward chunk + updateLoadingStates(filteredEvents) return offsetResults.size } 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 f6fd88f816..6161e8e5fb 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 @@ -224,11 +224,18 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy currentChunk.addTimelineEvent(roomId, eventEntity, direction, roomMemberContentsByUser) } + // Find all the chunks which contain at least one event from the list of eventIds val chunks = ChunkEntity.findAllIncludingEvents(realm, eventIds) + Timber.d("Found ${chunks.size} chunks containing at least one of the eventIds") val chunksToDelete = ArrayList() chunks.forEach { if (it != currentChunk) { - currentChunk.merge(roomId, it, direction) + if (direction == PaginationDirection.FORWARDS && it.hasBeenALastForwardChunk()) { + Timber.d("Do not merge $it") + } else { + Timber.d("Merge $it") + currentChunk.merge(roomId, it, direction) + } chunksToDelete.add(it) } } From 697eaec197b338e735b48095c7b5424e9df67908 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 1 May 2020 00:36:01 +0200 Subject: [PATCH 11/21] TI: After jump to unread, newer messages are never loaded (#1008) --- .../matrix/android/common/CommonTestHelper.kt | 24 ++- .../timeline/TimelineForwardPaginationTest.kt | 175 ++++++++++++++++++ 2 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt index ad12fb6bba..2529be9547 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt @@ -28,10 +28,10 @@ import im.vector.matrix.android.api.auth.data.LoginFlowResult import im.vector.matrix.android.api.auth.registration.RegistrationResult import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.events.model.EventType -import im.vector.matrix.android.api.session.events.model.LocalEcho import im.vector.matrix.android.api.session.events.model.toModel import im.vector.matrix.android.api.session.room.Room import im.vector.matrix.android.api.session.room.model.message.MessageContent +import im.vector.matrix.android.api.session.room.send.SendState import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.matrix.android.api.session.room.timeline.TimelineSettings @@ -116,7 +116,7 @@ class CommonTestHelper(context: Context) { */ fun sendTextMessage(room: Room, message: String, nbOfMessages: Int): List { val sentEvents = ArrayList(nbOfMessages) - val latch = CountDownLatch(nbOfMessages) + val latch = CountDownLatch(1) val timelineListener = object : Timeline.Listener { override fun onTimelineFailure(throwable: Throwable) { } @@ -127,7 +127,7 @@ class CommonTestHelper(context: Context) { override fun onTimelineUpdated(snapshot: List) { val newMessages = snapshot - .filter { LocalEcho.isLocalEchoId(it.eventId).not() } + .filter { it.root.sendState == SendState.SYNCED } .filter { it.root.getClearType() == EventType.MESSAGE } .filter { it.root.getClearContent().toModel()?.body?.startsWith(message) == true } @@ -292,6 +292,24 @@ class CommonTestHelper(context: Context) { return requestFailure!! } + fun createEventListener(latch: CountDownLatch, predicate: (List) -> Boolean): Timeline.Listener { + return object : Timeline.Listener { + override fun onTimelineFailure(throwable: Throwable) { + // noop + } + + override fun onNewTimelineEvents(eventIds: List) { + // noop + } + + override fun onTimelineUpdated(snapshot: List) { + if (predicate(snapshot)) { + latch.countDown() + } + } + } + } + /** * Await for a latch and ensure the result is true * diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt new file mode 100644 index 0000000000..1540f9f99b --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt @@ -0,0 +1,175 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * + * 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 im.vector.matrix.android.session.room.timeline + +import im.vector.matrix.android.InstrumentedTest +import im.vector.matrix.android.api.session.events.model.EventType +import im.vector.matrix.android.api.session.room.timeline.Timeline +import im.vector.matrix.android.api.session.room.timeline.TimelineSettings +import im.vector.matrix.android.common.CommonTestHelper +import im.vector.matrix.android.common.CryptoTestHelper +import org.amshove.kluent.shouldBeFalse +import org.amshove.kluent.shouldBeTrue +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.runners.MethodSorters +import timber.log.Timber +import java.util.concurrent.CountDownLatch + +@RunWith(JUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +class TimelineForwardPaginationTest : InstrumentedTest { + + private val commonTestHelper = CommonTestHelper(context()) + private val cryptoTestHelper = CryptoTestHelper(commonTestHelper) + + /** + * This test ensure that if we click to permalink, we will be able to go back to the live + */ + @Test + fun forwardPaginationTest() { + val numberOfMessagesToSend = 90 + val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceInARoom(false) + + val aliceSession = cryptoTestData.firstSession + val aliceRoomId = cryptoTestData.roomId + + aliceSession.cryptoService().setWarnOnUnknownDevices(false) + + val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!! + + // Alice sends X messages + val sentMessages = commonTestHelper.sendTextMessage( + roomFromAlicePOV, + "Message from Alice, long enough to observe the problem, if it is not long enough, there is not always the problem", + numberOfMessagesToSend) + + // Alice clear the cache + commonTestHelper.doSync { + aliceSession.clearCache(it) + } + + aliceSession.startSync(true) + + val aliceTimeline = roomFromAlicePOV.createTimeline(null, TimelineSettings(30)) + aliceTimeline.start() + + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Alice timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root.content}") + } + + // Ok, we have the 10 first messages of the initial sync + snapshot.size == 10 + } + + // Open the timeline at last sent message + aliceTimeline.addListener(eventsListener) + commonTestHelper.await(lock) + aliceTimeline.removeAllListeners() + + aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() + aliceTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + } + + run { + val lock = CountDownLatch(1) + val aliceEventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Alice timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root.content}") + } + + // The event is not in db, so it is fetch alone + snapshot.size == 1 + } + + aliceTimeline.addListener(aliceEventsListener) + + // Restart the timeline to the first sent event + aliceTimeline.restartWithEventId(sentMessages.last().eventId) + + commonTestHelper.await(lock) + aliceTimeline.removeAllListeners() + + aliceTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() + aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() + } + + run { + val lock = CountDownLatch(1) + val aliceEventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Alice timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root.content}") + } + + // Alice can see the first event of the room (so Back pagination has worked) + snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE + // 6 for room creation item (backward pagination), 1 for the context, and 50 for the forward pagination + && snapshot.size == 6 + 1 + 50 + } + + aliceTimeline.addListener(aliceEventsListener) + + // Restart the timeline to the first sent event + // We ask to load event backward and forward + aliceTimeline.paginate(Timeline.Direction.BACKWARDS, 50) + aliceTimeline.paginate(Timeline.Direction.FORWARDS, 50) + + commonTestHelper.await(lock) + aliceTimeline.removeAllListeners() + + aliceTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() + aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + run { + val lock = CountDownLatch(1) + val aliceEventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Alice timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root.content}") + } + + // 6 for room creation item (backward pagination),and numberOfMessagesToSend (all the message of the room) + snapshot.size == 6 + numberOfMessagesToSend + } + + aliceTimeline.addListener(aliceEventsListener) + + // Ask for a forward pagination + aliceTimeline.paginate(Timeline.Direction.FORWARDS, 50) + + commonTestHelper.await(lock) + aliceTimeline.removeAllListeners() + + // The timeline is fully loaded + aliceTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + aliceTimeline.dispose() + + cryptoTestData.cleanUp(commonTestHelper) + } +} From 92befcde5d747a5f22c63a0e1ecf9af3920b5d1f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 1 May 2020 01:45:06 +0200 Subject: [PATCH 12/21] Add test to cover previous last forward case (passing) --- .../TimelinePreviousLastForwardTest.kt | 236 ++++++++++++++++++ 1 file changed, 236 insertions(+) create mode 100644 matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt new file mode 100644 index 0000000000..7ae307ec8b --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt @@ -0,0 +1,236 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * + * 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 im.vector.matrix.android.session.room.timeline + +import im.vector.matrix.android.InstrumentedTest +import im.vector.matrix.android.api.extensions.orFalse +import im.vector.matrix.android.api.session.events.model.EventType +import im.vector.matrix.android.api.session.events.model.toModel +import im.vector.matrix.android.api.session.room.model.message.MessageContent +import im.vector.matrix.android.api.session.room.timeline.Timeline +import im.vector.matrix.android.api.session.room.timeline.TimelineSettings +import im.vector.matrix.android.common.CommonTestHelper +import im.vector.matrix.android.common.CryptoTestHelper +import org.amshove.kluent.shouldBeFalse +import org.amshove.kluent.shouldBeTrue +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.runners.MethodSorters +import timber.log.Timber +import java.util.concurrent.CountDownLatch + +@RunWith(JUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +class TimelinePreviousLastForwardTest : InstrumentedTest { + + private val commonTestHelper = CommonTestHelper(context()) + private val cryptoTestHelper = CryptoTestHelper(commonTestHelper) + + /** + * This test ensure that if we have a chunk in the timeline which is due to a sync, and we click to permalink, we will be able to go back to the live + */ + @Test + fun previousLastForwardTest() { + val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(false) + + val aliceSession = cryptoTestData.firstSession + val bobSession = cryptoTestData.secondSession!! + val aliceRoomId = cryptoTestData.roomId + + aliceSession.cryptoService().setWarnOnUnknownDevices(false) + bobSession.cryptoService().setWarnOnUnknownDevices(false) + + val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!! + val roomFromBobPOV = bobSession.getRoom(aliceRoomId)!! + + val bobTimeline = roomFromBobPOV.createTimeline(null, TimelineSettings(30)) + bobTimeline.start() + + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Ok, we have the 8 first messages of the initial sync (room creation and bob join event) + snapshot.size == 8 + } + + bobTimeline.addListener(eventsListener) + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + } + + // Bob stop to sync + bobSession.stopSync() + + // Alice sends 30 messages + val firstMessageFromAliceId = commonTestHelper.sendTextMessage( + roomFromAlicePOV, + "First messages from Alice", + 30) + .last() + .eventId + + // Bob start to sync + bobSession.startSync(true) + + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Ok, we have the 10 last messages from Alice. This will be our future previous lastForward chunk + snapshot.size == 10 + && snapshot.all { it.root.content.toModel()?.body?.startsWith("First messages from Alice").orFalse() } + } + + bobTimeline.addListener(eventsListener) + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + } + + // Bob stop to sync + bobSession.stopSync() + + // Alice sends again 30 messages + commonTestHelper.sendTextMessage( + roomFromAlicePOV, + "Second messages from Alice", + 30) + + // Bob start to sync + bobSession.startSync(true) + + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Ok, we have the 10 last messages from Alice. This will be our future previous lastForward chunk + snapshot.size == 10 + && snapshot.all { it.root.content.toModel()?.body?.startsWith("Second messages from Alice").orFalse() } + } + + bobTimeline.addListener(eventsListener) + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + } + + // Bob navigate to the first message sent from Alice + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // The event is not in db, so it is fetch + snapshot.size == 1 + } + + bobTimeline.addListener(eventsListener) + + // Restart the timeline to the first sent event, and paginate in both direction + bobTimeline.restartWithEventId(firstMessageFromAliceId) + bobTimeline.paginate(Timeline.Direction.BACKWARDS, 50) + bobTimeline.paginate(Timeline.Direction.FORWARDS, 50) + + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() + } + + // Paginate in both direction + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + snapshot.size == 8 + 1 + 35 + } + + bobTimeline.addListener(eventsListener) + + // Paginate in both direction + bobTimeline.paginate(Timeline.Direction.BACKWARDS, 50) + // Ensure the chunk in the middle is included in the next pagination + bobTimeline.paginate(Timeline.Direction.FORWARDS, 35) + + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + // Bob scroll to the future, till the live + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Bob can see the first event of the room (so Back pagination has worked) + snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE + // 8 for room creation item 60 message from Alice + && snapshot.size == 8 + 60 + } + + bobTimeline.addListener(eventsListener) + + bobTimeline.paginate(Timeline.Direction.FORWARDS, 50) + + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + bobTimeline.dispose() + + cryptoTestData.cleanUp(commonTestHelper) + } +} From 2b9d3960b384279662d43665d6ba189750eeb8a2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 4 May 2020 15:02:09 +0200 Subject: [PATCH 13/21] Improve tests --- .../timeline/TimelineForwardPaginationTest.kt | 17 +++++++++++++++-- .../timeline/TimelinePreviousLastForwardTest.kt | 2 +- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt index 1540f9f99b..ae6a9f8d42 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt @@ -17,7 +17,10 @@ package im.vector.matrix.android.session.room.timeline import im.vector.matrix.android.InstrumentedTest +import im.vector.matrix.android.api.extensions.orFalse import im.vector.matrix.android.api.session.events.model.EventType +import im.vector.matrix.android.api.session.events.model.toModel +import im.vector.matrix.android.api.session.room.model.message.MessageContent import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.matrix.android.api.session.room.timeline.TimelineSettings import im.vector.matrix.android.common.CommonTestHelper @@ -57,7 +60,7 @@ class TimelineForwardPaginationTest : InstrumentedTest { // Alice sends X messages val sentMessages = commonTestHelper.sendTextMessage( roomFromAlicePOV, - "Message from Alice, long enough to observe the problem, if it is not long enough, there is not always the problem", + "Message from Alice", numberOfMessagesToSend) // Alice clear the cache @@ -65,11 +68,13 @@ class TimelineForwardPaginationTest : InstrumentedTest { aliceSession.clearCache(it) } + // And restarts the sync aliceSession.startSync(true) val aliceTimeline = roomFromAlicePOV.createTimeline(null, TimelineSettings(30)) aliceTimeline.start() + // Alice sees the 10 last message of the room, and can only navigate BACKWARD run { val lock = CountDownLatch(1) val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> @@ -78,8 +83,9 @@ class TimelineForwardPaginationTest : InstrumentedTest { Timber.w(" event ${it.root.content}") } - // Ok, we have the 10 first messages of the initial sync + // Ok, we have the 10 last messages of the initial sync snapshot.size == 10 + && snapshot.all { it.root.content.toModel()?.body?.startsWith("Message from Alice").orFalse() } } // Open the timeline at last sent message @@ -91,6 +97,8 @@ class TimelineForwardPaginationTest : InstrumentedTest { aliceTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() } + // Alice navigates to the first message of the room, which is not in its database. A GET /context is performed + // Then she can paginate BACKWARD and FORWARD run { val lock = CountDownLatch(1) val aliceEventsListener = commonTestHelper.createEventListener(lock) { snapshot -> @@ -101,6 +109,7 @@ class TimelineForwardPaginationTest : InstrumentedTest { // The event is not in db, so it is fetch alone snapshot.size == 1 + && snapshot.all { it.root.content.toModel()?.body?.startsWith("Message from Alice").orFalse() } } aliceTimeline.addListener(aliceEventsListener) @@ -115,6 +124,8 @@ class TimelineForwardPaginationTest : InstrumentedTest { aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() } + // Alice paginates BACKWARD and FORWARD of 50 events each + // Then she can only navigate FORWARD run { val lock = CountDownLatch(1) val aliceEventsListener = commonTestHelper.createEventListener(lock) { snapshot -> @@ -143,6 +154,8 @@ class TimelineForwardPaginationTest : InstrumentedTest { aliceTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() } + // Alice paginates once again FORWARD for 50 events + // All the timeline is retrieved, she cannot paginate anymore in both direction run { val lock = CountDownLatch(1) val aliceEventsListener = commonTestHelper.createEventListener(lock) { snapshot -> diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt index 7ae307ec8b..0ca5cfdda0 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt @@ -70,7 +70,7 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { Timber.w(" event ${it.root}") } - // Ok, we have the 8 first messages of the initial sync (room creation and bob join event) + // Ok, we have the 8 first messages of the initial sync (room creation and bob invite and join events) snapshot.size == 8 } From 53583c691f9e4da12aab493b943df1f2ed801c60 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 4 May 2020 17:15:01 +0200 Subject: [PATCH 14/21] Add some logs --- .../internal/crypto/store/db/RealmCryptoStoreMigration.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt index c1897c76d9..6ff8a49e28 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt @@ -196,6 +196,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi } private fun migrateTo3(realm: DynamicRealm) { + Timber.d("Step 2 -> 3") Timber.d("Updating CryptoMetadataEntity table") realm.schema.get("CryptoMetadataEntity") ?.addField(CryptoMetadataEntityFields.KEY_BACKUP_RECOVERY_KEY, String::class.java) @@ -203,6 +204,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi } private fun migrateTo4(realm: DynamicRealm) { + Timber.d("Step 3 -> 4") Timber.d("Updating KeyInfoEntity table") val keyInfoEntities = realm.where("KeyInfoEntity").findAll() try { @@ -217,6 +219,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi } private fun migrateTo5(realm: DynamicRealm) { + Timber.d("Step 4 -> 5") realm.schema.create("MyDeviceLastSeenInfoEntity") .addField(MyDeviceLastSeenInfoEntityFields.DEVICE_ID, String::class.java) .addPrimaryKey(MyDeviceLastSeenInfoEntityFields.DEVICE_ID) From 17ddb5ce43fedebe818e5e15e0fa60063ca65451 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 4 May 2020 18:19:49 +0200 Subject: [PATCH 15/21] if all events are rendered in the timeline (developer mode), render the room creation event. --- .../src/main/res/values/strings.xml | 1 + .../timeline/factory/RoomCreateItemFactory.kt | 24 ++++++++++++------- .../timeline/factory/TimelineItemFactory.kt | 2 +- .../timeline/format/NoticeEventFormatter.kt | 8 +++++++ 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/matrix-sdk-android/src/main/res/values/strings.xml b/matrix-sdk-android/src/main/res/values/strings.xml index 50169fd982..69907e5835 100644 --- a/matrix-sdk-android/src/main/res/values/strings.xml +++ b/matrix-sdk-android/src/main/res/values/strings.xml @@ -5,6 +5,7 @@ %1$s sent a sticker. %s\'s invitation + %1$s created the room %1$s invited %2$s %1$s invited you %1$s joined the room diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/RoomCreateItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/RoomCreateItemFactory.kt index bf3b82ab4d..d5471d7f4f 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/RoomCreateItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/RoomCreateItemFactory.kt @@ -21,21 +21,21 @@ import im.vector.matrix.android.api.session.events.model.toModel import im.vector.matrix.android.api.session.room.model.create.RoomCreateContent import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.riotx.R -import im.vector.riotx.core.resources.ColorProvider +import im.vector.riotx.core.epoxy.VectorEpoxyModel import im.vector.riotx.core.resources.StringProvider +import im.vector.riotx.core.resources.UserPreferencesProvider import im.vector.riotx.features.home.room.detail.timeline.TimelineEventController -import im.vector.riotx.features.home.room.detail.timeline.item.RoomCreateItem import im.vector.riotx.features.home.room.detail.timeline.item.RoomCreateItem_ import me.gujun.android.span.span import javax.inject.Inject -class RoomCreateItemFactory @Inject constructor(private val colorProvider: ColorProvider, - private val stringProvider: StringProvider) { +class RoomCreateItemFactory @Inject constructor(private val stringProvider: StringProvider, + private val userPreferencesProvider: UserPreferencesProvider, + private val noticeItemFactory: NoticeItemFactory) { - fun create(event: TimelineEvent, callback: TimelineEventController.Callback?): RoomCreateItem? { - val createRoomContent = event.root.getClearContent().toModel() - ?: return null - val predecessorId = createRoomContent.predecessor?.roomId ?: return null + fun create(event: TimelineEvent, callback: TimelineEventController.Callback?): VectorEpoxyModel<*>? { + val createRoomContent = event.root.getClearContent().toModel() ?: return null + val predecessorId = createRoomContent.predecessor?.roomId ?: return defaultRendering(event, callback) val roomLink = PermalinkFactory.createPermalink(predecessorId) ?: return null val text = span { +stringProvider.getString(R.string.room_tombstone_continuation_description) @@ -48,4 +48,12 @@ class RoomCreateItemFactory @Inject constructor(private val colorProvider: Color return RoomCreateItem_() .text(text) } + + private fun defaultRendering(event: TimelineEvent, callback: TimelineEventController.Callback?): VectorEpoxyModel<*>? { + return if (userPreferencesProvider.shouldShowHiddenEvents()) { + noticeItemFactory.create(event, false, callback) + } else { + null + } + } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/TimelineItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/TimelineItemFactory.kt index 7e6c387934..f2ac7018aa 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/TimelineItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/TimelineItemFactory.kt @@ -58,7 +58,7 @@ class TimelineItemFactory @Inject constructor(private val messageItemFactory: Me EventType.CALL_HANGUP, EventType.CALL_ANSWER, EventType.REACTION, - EventType.REDACTION -> noticeItemFactory.create(event, highlight, callback) + EventType.REDACTION -> noticeItemFactory.create(event, highlight, callback) EventType.STATE_ROOM_ENCRYPTION -> { encryptionItemFactory.create(event, highlight, callback) } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/format/NoticeEventFormatter.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/format/NoticeEventFormatter.kt index 39e17b7c35..f29bd72e0a 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/format/NoticeEventFormatter.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/format/NoticeEventFormatter.kt @@ -32,6 +32,7 @@ import im.vector.matrix.android.api.session.room.model.RoomMemberContent import im.vector.matrix.android.api.session.room.model.RoomNameContent import im.vector.matrix.android.api.session.room.model.RoomTopicContent import im.vector.matrix.android.api.session.room.model.call.CallInviteContent +import im.vector.matrix.android.api.session.room.model.create.RoomCreateContent import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.matrix.android.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM import im.vector.matrix.android.internal.crypto.model.event.EncryptionEventContent @@ -47,6 +48,7 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active fun format(timelineEvent: TimelineEvent): CharSequence? { return when (val type = timelineEvent.root.getClearType()) { EventType.STATE_ROOM_JOIN_RULES -> formatJoinRulesEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName()) + EventType.STATE_ROOM_CREATE -> formatRoomCreateEvent(timelineEvent.root) EventType.STATE_ROOM_NAME -> formatRoomNameEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName()) EventType.STATE_ROOM_TOPIC -> formatRoomTopicEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName()) EventType.STATE_ROOM_MEMBER -> formatRoomMemberEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName()) @@ -98,6 +100,12 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active return "{ \"type\": ${event.getClearType()} }" } + private fun formatRoomCreateEvent(event: Event): CharSequence? { + return event.getClearContent().toModel() + ?.takeIf { it.creator.isNullOrBlank().not() } + ?.let { sp.getString(R.string.notice_room_created, it.creator) } + } + private fun formatRoomNameEvent(event: Event, senderName: String?): CharSequence? { val content = event.getClearContent().toModel() ?: return null return if (content.name.isNullOrBlank()) { From fcee85a6829557ac4169f04b137718493e6e7a47 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 5 May 2020 00:19:40 +0200 Subject: [PATCH 16/21] Cleanup and doc --- .../riotx/core/extensions/Collections.kt | 20 +++++++++++++++++++ .../timeline/TimelineEventController.kt | 2 +- .../factory/MergedHeaderItemFactory.kt | 14 ++++++------- .../helper/TimelineDisplayableEvents.kt | 8 -------- 4 files changed, 28 insertions(+), 16 deletions(-) create mode 100644 vector/src/main/java/im/vector/riotx/core/extensions/Collections.kt diff --git a/vector/src/main/java/im/vector/riotx/core/extensions/Collections.kt b/vector/src/main/java/im/vector/riotx/core/extensions/Collections.kt new file mode 100644 index 0000000000..af5d5babb6 --- /dev/null +++ b/vector/src/main/java/im/vector/riotx/core/extensions/Collections.kt @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * + * 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 im.vector.riotx.core.extensions + +inline fun List.nextOrNull(index: Int) = getOrNull(index + 1) +inline fun List.prevOrNull(index: Int) = getOrNull(index - 1) diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt index addbfab43c..e074af1da6 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt @@ -35,6 +35,7 @@ import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.riotx.core.date.VectorDateFormatter import im.vector.riotx.core.epoxy.LoadingItem_ import im.vector.riotx.core.extensions.localDateTime +import im.vector.riotx.core.extensions.nextOrNull import im.vector.riotx.features.home.room.detail.RoomDetailAction import im.vector.riotx.features.home.room.detail.RoomDetailViewState import im.vector.riotx.features.home.room.detail.UnreadState @@ -45,7 +46,6 @@ import im.vector.riotx.features.home.room.detail.timeline.helper.ReadMarkerVisib import im.vector.riotx.features.home.room.detail.timeline.helper.TimelineEventDiffUtilCallback import im.vector.riotx.features.home.room.detail.timeline.helper.TimelineEventVisibilityStateChangedListener import im.vector.riotx.features.home.room.detail.timeline.helper.TimelineMediaSizeProvider -import im.vector.riotx.features.home.room.detail.timeline.helper.nextOrNull import im.vector.riotx.features.home.room.detail.timeline.item.BaseEventItem import im.vector.riotx.features.home.room.detail.timeline.item.BasedMergedItem import im.vector.riotx.features.home.room.detail.timeline.item.DaySeparatorItem diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt index 03c273800a..ec6a975178 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt @@ -22,7 +22,7 @@ import im.vector.matrix.android.api.session.room.model.create.RoomCreateContent import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.matrix.android.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM import im.vector.matrix.android.internal.crypto.model.event.EncryptionEventContent -import im.vector.riotx.core.di.ActiveSessionHolder +import im.vector.riotx.core.extensions.prevOrNull import im.vector.riotx.features.home.AvatarRenderer import im.vector.riotx.features.home.room.detail.timeline.TimelineEventController import im.vector.riotx.features.home.room.detail.timeline.helper.AvatarSizeProvider @@ -37,15 +37,15 @@ import im.vector.riotx.features.home.room.detail.timeline.item.MergedRoomCreatio import im.vector.riotx.features.home.room.detail.timeline.item.MergedRoomCreationItem_ import javax.inject.Inject -class MergedHeaderItemFactory @Inject constructor(private val sessionHolder: ActiveSessionHolder, - private val avatarRenderer: AvatarRenderer, +class MergedHeaderItemFactory @Inject constructor(private val avatarRenderer: AvatarRenderer, private val avatarSizeProvider: AvatarSizeProvider) { private val collapsedEventIds = linkedSetOf() private val mergeItemCollapseStates = HashMap() /** - * Note: nextEvent is an older event than event + * @param nextEvent is an older event than event + * @param items all known items, sorted from newer event to oldest event */ fun create(event: TimelineEvent, nextEvent: TimelineEvent?, @@ -127,9 +127,9 @@ class MergedHeaderItemFactory @Inject constructor(private val sessionHolder: Act eventIdToHighlight: String?, requestModelBuild: () -> Unit, callback: TimelineEventController.Callback?): MergedRoomCreationItem_? { - var prevEvent = if (currentPosition > 0) items[currentPosition - 1] else null + var prevEvent = items.prevOrNull(currentPosition) var tmpPos = currentPosition - 1 - val mergedEvents = ArrayList().also { it.add(event) } + val mergedEvents = mutableListOf(event) var hasEncryption = false var encryptionAlgorithm: String? = null while (prevEvent != null && prevEvent.isRoomConfiguration(null)) { @@ -139,7 +139,7 @@ class MergedHeaderItemFactory @Inject constructor(private val sessionHolder: Act } mergedEvents.add(prevEvent) tmpPos-- - prevEvent = if (tmpPos >= 0) items[tmpPos] else null + prevEvent = items.getOrNull(tmpPos) } return if (mergedEvents.size > 2) { var highlighted = false diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/TimelineDisplayableEvents.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/TimelineDisplayableEvents.kt index f1106d276e..daf0100bbb 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/TimelineDisplayableEvents.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/TimelineDisplayableEvents.kt @@ -106,11 +106,3 @@ fun List.prevSameTypeEvents(index: Int, minSize: Int): List.nextOrNull(index: Int): TimelineEvent? { - return if (index >= size - 1) { - null - } else { - subList(index + 1, this.size).firstOrNull() - } -} From db77e7b8178dcc3aab09b29b10dd9c31cf50e357 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 5 May 2020 00:30:49 +0200 Subject: [PATCH 17/21] Create a fun --- .../factory/MergedHeaderItemFactory.kt | 111 ++++++++++-------- 1 file changed, 60 insertions(+), 51 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt index ec6a975178..9529693e6b 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt @@ -64,60 +64,69 @@ class MergedHeaderItemFactory @Inject constructor(private val avatarRenderer: Av } else if (!event.canBeMerged() || (nextEvent?.root?.getClearType() == event.root.getClearType() && !addDaySeparator)) { null } else { - val prevSameTypeEvents = items.prevSameTypeEvents(currentPosition, 2) - if (prevSameTypeEvents.isEmpty()) { - null - } else { - var highlighted = false - val mergedEvents = (prevSameTypeEvents + listOf(event)).asReversed() - val mergedData = ArrayList(mergedEvents.size) - mergedEvents.forEach { mergedEvent -> - if (!highlighted && mergedEvent.root.eventId == eventIdToHighlight) { - highlighted = true - } - val senderAvatar = mergedEvent.senderAvatar - val senderName = mergedEvent.getDisambiguatedDisplayName() - val data = BasedMergedItem.Data( - userId = mergedEvent.root.senderId ?: "", - avatarUrl = senderAvatar, - memberName = senderName, - localId = mergedEvent.localId, - eventId = mergedEvent.root.eventId ?: "" - ) - mergedData.add(data) + buildMembershipEventsMergedSummary(currentPosition, items, event, eventIdToHighlight, requestModelBuild, callback) + } + } + + private fun buildMembershipEventsMergedSummary(currentPosition: Int, + items: List, + event: TimelineEvent, + eventIdToHighlight: String?, + requestModelBuild: () -> Unit, + callback: TimelineEventController.Callback?): MergedMembershipEventsItem_? { + val prevSameTypeEvents = items.prevSameTypeEvents(currentPosition, 2) + return if (prevSameTypeEvents.isEmpty()) { + null + } else { + var highlighted = false + val mergedEvents = (prevSameTypeEvents + listOf(event)).asReversed() + val mergedData = ArrayList(mergedEvents.size) + mergedEvents.forEach { mergedEvent -> + if (!highlighted && mergedEvent.root.eventId == eventIdToHighlight) { + highlighted = true } - val mergedEventIds = mergedEvents.map { it.localId } - // We try to find if one of the item id were used as mergeItemCollapseStates key - // => handle case where paginating from mergeable events and we get more - val previousCollapseStateKey = mergedEventIds.intersect(mergeItemCollapseStates.keys).firstOrNull() - val initialCollapseState = mergeItemCollapseStates.remove(previousCollapseStateKey) - ?: true - val isCollapsed = mergeItemCollapseStates.getOrPut(event.localId) { initialCollapseState } - if (isCollapsed) { - collapsedEventIds.addAll(mergedEventIds) - } else { - collapsedEventIds.removeAll(mergedEventIds) - } - val mergeId = mergedEventIds.joinToString(separator = "_") { it.toString() } - val attributes = MergedMembershipEventsItem.Attributes( - isCollapsed = isCollapsed, - mergeData = mergedData, - avatarRenderer = avatarRenderer, - onCollapsedStateChanged = { - mergeItemCollapseStates[event.localId] = it - requestModelBuild() - }, - readReceiptsCallback = callback + val senderAvatar = mergedEvent.senderAvatar + val senderName = mergedEvent.getDisambiguatedDisplayName() + val data = BasedMergedItem.Data( + userId = mergedEvent.root.senderId ?: "", + avatarUrl = senderAvatar, + memberName = senderName, + localId = mergedEvent.localId, + eventId = mergedEvent.root.eventId ?: "" ) - MergedMembershipEventsItem_() - .id(mergeId) - .leftGuideline(avatarSizeProvider.leftGuideline) - .highlighted(isCollapsed && highlighted) - .attributes(attributes) - .also { - it.setOnVisibilityStateChanged(MergedTimelineEventVisibilityStateChangedListener(callback, mergedEvents)) - } + mergedData.add(data) } + val mergedEventIds = mergedEvents.map { it.localId } + // We try to find if one of the item id were used as mergeItemCollapseStates key + // => handle case where paginating from mergeable events and we get more + val previousCollapseStateKey = mergedEventIds.intersect(mergeItemCollapseStates.keys).firstOrNull() + val initialCollapseState = mergeItemCollapseStates.remove(previousCollapseStateKey) + ?: true + val isCollapsed = mergeItemCollapseStates.getOrPut(event.localId) { initialCollapseState } + if (isCollapsed) { + collapsedEventIds.addAll(mergedEventIds) + } else { + collapsedEventIds.removeAll(mergedEventIds) + } + val mergeId = mergedEventIds.joinToString(separator = "_") { it.toString() } + val attributes = MergedMembershipEventsItem.Attributes( + isCollapsed = isCollapsed, + mergeData = mergedData, + avatarRenderer = avatarRenderer, + onCollapsedStateChanged = { + mergeItemCollapseStates[event.localId] = it + requestModelBuild() + }, + readReceiptsCallback = callback + ) + MergedMembershipEventsItem_() + .id(mergeId) + .leftGuideline(avatarSizeProvider.leftGuideline) + .highlighted(isCollapsed && highlighted) + .attributes(attributes) + .also { + it.setOnVisibilityStateChanged(MergedTimelineEventVisibilityStateChangedListener(callback, mergedEvents)) + } } } From ffeae7ec83ffbed426ef3982e0b84501f7528ece Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 5 May 2020 02:38:30 +0200 Subject: [PATCH 18/21] Fix timeline navigation when opening an event in a previous lastForward chunk. In this case, we do not have a nextToken, but there are more event to load. So we perform a GET /context on the last known event. Not sure it is correct to do that though... --- .../TimelineBackToPreviousLastForwardTest.kt | 206 ++++++++++++++++++ .../session/room/timeline/DefaultTimeline.kt | 6 + .../room/timeline/TokenChunkEventPersistor.kt | 16 +- 3 files changed, 225 insertions(+), 3 deletions(-) create mode 100644 matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt new file mode 100644 index 0000000000..d55087a8c7 --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt @@ -0,0 +1,206 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * + * 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 im.vector.matrix.android.session.room.timeline + +import im.vector.matrix.android.InstrumentedTest +import im.vector.matrix.android.api.extensions.orFalse +import im.vector.matrix.android.api.session.events.model.EventType +import im.vector.matrix.android.api.session.events.model.toModel +import im.vector.matrix.android.api.session.room.model.message.MessageContent +import im.vector.matrix.android.api.session.room.timeline.Timeline +import im.vector.matrix.android.api.session.room.timeline.TimelineSettings +import im.vector.matrix.android.common.CommonTestHelper +import im.vector.matrix.android.common.CryptoTestHelper +import org.amshove.kluent.shouldBeFalse +import org.amshove.kluent.shouldBeTrue +import org.junit.Assert.assertTrue +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.runners.MethodSorters +import timber.log.Timber +import java.util.concurrent.CountDownLatch + +@RunWith(JUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +class TimelineBackToPreviousLastForwardTest : InstrumentedTest { + + private val commonTestHelper = CommonTestHelper(context()) + private val cryptoTestHelper = CryptoTestHelper(commonTestHelper) + + /** + * This test ensure that if we have a chunk in the timeline which is due to a sync, and we click to permalink of an + * even contained in a previous lastForward chunk, we will be able to go back to the live + */ + @Test + fun backToPreviousLastForwardTest() { + val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(false) + + val aliceSession = cryptoTestData.firstSession + val bobSession = cryptoTestData.secondSession!! + val aliceRoomId = cryptoTestData.roomId + + aliceSession.cryptoService().setWarnOnUnknownDevices(false) + bobSession.cryptoService().setWarnOnUnknownDevices(false) + + val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!! + val roomFromBobPOV = bobSession.getRoom(aliceRoomId)!! + + val bobTimeline = roomFromBobPOV.createTimeline(null, TimelineSettings(30)) + bobTimeline.start() + + var roomCreationEventId: String? = null + + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + roomCreationEventId = snapshot.lastOrNull()?.root?.eventId + // Ok, we have the 8 first messages of the initial sync (room creation and bob join event) + snapshot.size == 8 + } + + bobTimeline.addListener(eventsListener) + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + } + + // Bob stop to sync + bobSession.stopSync() + + // Alice sends 30 messages + commonTestHelper.sendTextMessage( + roomFromAlicePOV, + "First messages from Alice", + 30) + + // Bob start to sync + bobSession.startSync(true) + + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Ok, we have the 10 last messages from Alice. + snapshot.size == 10 + && snapshot.all { it.root.content.toModel()?.body?.startsWith("First messages from Alice").orFalse() } + } + + bobTimeline.addListener(eventsListener) + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + } + + // Bob navigate to the first event (room creation event), so inside the previous last forward chunk + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // The event is in db, so it is fetch and auto pagination occurs, half of the number of events we have for this chunk (?) + snapshot.size == 4 + } + + bobTimeline.addListener(eventsListener) + + // Restart the timeline to the first sent event, which is already in the database, so pagination should start automatically + assertTrue(roomFromBobPOV.getTimeLineEvent(roomCreationEventId!!) != null) + + bobTimeline.restartWithEventId(roomCreationEventId) + + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + // Bob scroll to the future + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Bob can see the first event of the room (so Back pagination has worked) + snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE + // 8 for room creation item, and 30 for the forward pagination + && snapshot.size == 8 + } + + bobTimeline.addListener(eventsListener) + + bobTimeline.paginate(Timeline.Direction.FORWARDS, 50) + + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + // Do it again, now we should have a next token, so we can paginate FORWARD + run { + val lock = CountDownLatch(1) + val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> + Timber.e("Bob timeline updated: with ${snapshot.size} events:") + snapshot.forEach { + Timber.w(" event ${it.root}") + } + + // Bob can see the first event of the room (so Back pagination has worked) + snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE + // 8 for room creation item, and 30 for the forward pagination + && snapshot.size == 8 + 30 + } + + bobTimeline.addListener(eventsListener) + + bobTimeline.paginate(Timeline.Direction.FORWARDS, 50) + + commonTestHelper.await(lock) + bobTimeline.removeAllListeners() + + bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() + bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() + } + + bobTimeline.dispose() + + cryptoTestData.cleanUp(commonTestHelper) + } +} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt index 8cfd498cc8..ba46b10228 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt @@ -521,6 +521,12 @@ internal class DefaultTimeline( private fun executePaginationTask(direction: Timeline.Direction, limit: Int) { val token = getTokenLive(direction) if (token == null) { + val currentChunk = getLiveChunk() + if (direction == Timeline.Direction.FORWARDS && currentChunk?.hasBeenALastForwardChunk() == true) { + // We are in the case that next event exists, but we do not know the next token. + // Fetch (again) the last event to get a nextToken + currentChunk.timelineEvents.lastOrNull()?.eventId?.let { fetchEvent(it) } + } updateState(direction) { it.copy(isPaginating = false, requestedPaginationCount = 0) } return } 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 6161e8e5fb..3f5e5ccf06 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 @@ -231,12 +231,20 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy chunks.forEach { if (it != currentChunk) { if (direction == PaginationDirection.FORWARDS && it.hasBeenALastForwardChunk()) { - Timber.d("Do not merge $it") + // Maybe it was a trick to get a nextToken + if (receivedChunk.events.size == 1) { + Timber.d("Receiving a new nextToken") + it.nextToken = receivedChunk.end + chunksToDelete.add(currentChunk) + } else { + Timber.d("Do not merge $it") + chunksToDelete.add(it) + } } else { Timber.d("Merge $it") currentChunk.merge(roomId, it, direction) + chunksToDelete.add(it) } - chunksToDelete.add(it) } } val shouldUpdateSummary = chunksToDelete.isNotEmpty() && currentChunk.isLastForward && direction == PaginationDirection.FORWARDS @@ -253,6 +261,8 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy ) roomSummaryEntity.latestPreviewableEvent = latestPreviewableEvent } - RoomEntity.where(realm, roomId).findFirst()?.addOrUpdate(currentChunk) + if (currentChunk.isValid) { + RoomEntity.where(realm, roomId).findFirst()?.addOrUpdate(currentChunk) + } } } From 458e3ee5e8b9b6a56371f0bd373a56000bb119ec Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 15 May 2020 20:18:07 +0200 Subject: [PATCH 19/21] Timeline: fetch next token with the help of getContext when required --- .../internal/session/room/RoomModule.kt | 5 + .../session/room/timeline/DefaultTimeline.kt | 100 +++++++++++------- .../room/timeline/DefaultTimelineService.kt | 4 +- .../timeline/FetchNextTokenAndPaginateTask.kt | 66 ++++++++++++ .../room/timeline/TokenChunkEventPersistor.kt | 18 +--- 5 files changed, 138 insertions(+), 55 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/FetchNextTokenAndPaginateTask.kt diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomModule.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomModule.kt index 6b003b5ba2..b0a60480e3 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomModule.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomModule.kt @@ -56,8 +56,10 @@ import im.vector.matrix.android.internal.session.room.reporting.DefaultReportCon import im.vector.matrix.android.internal.session.room.reporting.ReportContentTask import im.vector.matrix.android.internal.session.room.state.DefaultSendStateTask import im.vector.matrix.android.internal.session.room.state.SendStateTask +import im.vector.matrix.android.internal.session.room.timeline.DefaultFetchNextTokenAndPaginateTask import im.vector.matrix.android.internal.session.room.timeline.DefaultGetContextOfEventTask import im.vector.matrix.android.internal.session.room.timeline.DefaultPaginationTask +import im.vector.matrix.android.internal.session.room.timeline.FetchNextTokenAndPaginateTask import im.vector.matrix.android.internal.session.room.timeline.GetContextOfEventTask import im.vector.matrix.android.internal.session.room.timeline.PaginationTask import im.vector.matrix.android.internal.session.room.typing.DefaultSendTypingTask @@ -143,6 +145,9 @@ internal abstract class RoomModule { @Binds abstract fun bindPaginationTask(task: DefaultPaginationTask): PaginationTask + @Binds + abstract fun bindFetchNextTokenAndPaginateTask(task: DefaultFetchNextTokenAndPaginateTask): FetchNextTokenAndPaginateTask + @Binds abstract fun bindFetchEditHistoryTask(task: DefaultFetchEditHistoryTask): FetchEditHistoryTask diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt index ba46b10228..76cdf8c485 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt @@ -17,6 +17,7 @@ package im.vector.matrix.android.internal.session.room.timeline import im.vector.matrix.android.api.MatrixCallback +import im.vector.matrix.android.api.extensions.orFalse import im.vector.matrix.android.api.session.events.model.EventType import im.vector.matrix.android.api.session.events.model.RelationType import im.vector.matrix.android.api.session.events.model.toModel @@ -71,6 +72,7 @@ internal class DefaultTimeline( private val realmConfiguration: RealmConfiguration, private val taskExecutor: TaskExecutor, private val contextOfEventTask: GetContextOfEventTask, + private val fetchNextTokenAndPaginateTask: FetchNextTokenAndPaginateTask, private val paginationTask: PaginationTask, private val timelineEventMapper: TimelineEventMapper, private val settings: TimelineSettings, @@ -519,50 +521,44 @@ internal class DefaultTimeline( * This has to be called on TimelineThread as it accesses realm live results */ private fun executePaginationTask(direction: Timeline.Direction, limit: Int) { - val token = getTokenLive(direction) + val currentChunk = getLiveChunk() + val token = if (direction == Timeline.Direction.BACKWARDS) currentChunk?.prevToken else currentChunk?.nextToken if (token == null) { - val currentChunk = getLiveChunk() - if (direction == Timeline.Direction.FORWARDS && currentChunk?.hasBeenALastForwardChunk() == true) { + if (direction == Timeline.Direction.FORWARDS && currentChunk?.hasBeenALastForwardChunk().orFalse()) { // We are in the case that next event exists, but we do not know the next token. // Fetch (again) the last event to get a nextToken - currentChunk.timelineEvents.lastOrNull()?.eventId?.let { fetchEvent(it) } - } - updateState(direction) { it.copy(isPaginating = false, requestedPaginationCount = 0) } - return - } - val params = PaginationTask.Params(roomId = roomId, - from = token, - direction = direction.toPaginationDirection(), - limit = limit) - - Timber.v("Should fetch $limit items $direction") - cancelableBag += paginationTask - .configureWith(params) { - this.callback = object : MatrixCallback { - override fun onSuccess(data: TokenChunkEventPersistor.Result) { - when (data) { - TokenChunkEventPersistor.Result.SUCCESS -> { - Timber.v("Success fetching $limit items $direction from pagination request") - } - TokenChunkEventPersistor.Result.REACHED_END -> { - postSnapshot() - } - TokenChunkEventPersistor.Result.SHOULD_FETCH_MORE -> - // Database won't be updated, so we force pagination request - BACKGROUND_HANDLER.post { - executePaginationTask(direction, limit) - } + val lastKnownEventId = nonFilteredEvents.firstOrNull()?.eventId + if (lastKnownEventId == null) { + updateState(direction) { it.copy(isPaginating = false, requestedPaginationCount = 0) } + } else { + val params = FetchNextTokenAndPaginateTask.Params( + roomId = roomId, + limit = limit, + lastKnownEventId = lastKnownEventId + ) + cancelableBag += fetchNextTokenAndPaginateTask + .configureWith(params) { + this.callback = createPaginationCallback(limit, direction) } - } - - override fun onFailure(failure: Throwable) { - updateState(direction) { it.copy(isPaginating = false, requestedPaginationCount = 0) } - postSnapshot() - Timber.v("Failure fetching $limit items $direction from pagination request") - } - } + .executeBy(taskExecutor) } - .executeBy(taskExecutor) + } else { + updateState(direction) { it.copy(isPaginating = false, requestedPaginationCount = 0) } + } + } else { + val params = PaginationTask.Params( + roomId = roomId, + from = token, + direction = direction.toPaginationDirection(), + limit = limit + ) + Timber.v("Should fetch $limit items $direction") + cancelableBag += paginationTask + .configureWith(params) { + this.callback = createPaginationCallback(limit, direction) + } + .executeBy(taskExecutor) + } } // For debug purpose only @@ -743,6 +739,32 @@ internal class DefaultTimeline( forwardsState.set(State()) } + private fun createPaginationCallback(limit: Int, direction: Timeline.Direction): MatrixCallback { + return object : MatrixCallback { + override fun onSuccess(data: TokenChunkEventPersistor.Result) { + when (data) { + TokenChunkEventPersistor.Result.SUCCESS -> { + Timber.v("Success fetching $limit items $direction from pagination request") + } + TokenChunkEventPersistor.Result.REACHED_END -> { + postSnapshot() + } + TokenChunkEventPersistor.Result.SHOULD_FETCH_MORE -> + // Database won't be updated, so we force pagination request + BACKGROUND_HANDLER.post { + executePaginationTask(direction, limit) + } + } + } + + override fun onFailure(failure: Throwable) { + updateState(direction) { it.copy(isPaginating = false, requestedPaginationCount = 0) } + postSnapshot() + Timber.v("Failure fetching $limit items $direction from pagination request") + } + } + } + // Extension methods *************************************************************************** private fun Timeline.Direction.toPaginationDirection(): PaginationDirection { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimelineService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimelineService.kt index c02bb915ef..ffa282d088 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimelineService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimelineService.kt @@ -42,6 +42,7 @@ internal class DefaultTimelineService @AssistedInject constructor(@Assisted priv private val contextOfEventTask: GetContextOfEventTask, private val eventDecryptor: TimelineEventDecryptor, private val paginationTask: PaginationTask, + private val fetchNextTokenAndPaginateTask: FetchNextTokenAndPaginateTask, private val timelineEventMapper: TimelineEventMapper, private val readReceiptsSummaryMapper: ReadReceiptsSummaryMapper ) : TimelineService { @@ -63,7 +64,8 @@ internal class DefaultTimelineService @AssistedInject constructor(@Assisted priv settings = settings, hiddenReadReceipts = TimelineHiddenReadReceipts(readReceiptsSummaryMapper, roomId, settings), eventBus = eventBus, - eventDecryptor = eventDecryptor + eventDecryptor = eventDecryptor, + fetchNextTokenAndPaginateTask = fetchNextTokenAndPaginateTask ) } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/FetchNextTokenAndPaginateTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/FetchNextTokenAndPaginateTask.kt new file mode 100644 index 0000000000..1189e627c4 --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/FetchNextTokenAndPaginateTask.kt @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * + * 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 im.vector.matrix.android.internal.session.room.timeline + +import com.zhuinden.monarchy.Monarchy +import im.vector.matrix.android.internal.database.model.ChunkEntity +import im.vector.matrix.android.internal.database.query.findIncludingEvent +import im.vector.matrix.android.internal.network.executeRequest +import im.vector.matrix.android.internal.session.filter.FilterRepository +import im.vector.matrix.android.internal.session.room.RoomAPI +import im.vector.matrix.android.internal.task.Task +import im.vector.matrix.android.internal.util.awaitTransaction +import org.greenrobot.eventbus.EventBus +import javax.inject.Inject + +internal interface FetchNextTokenAndPaginateTask : Task { + + data class Params( + val roomId: String, + val lastKnownEventId: String, + val limit: Int + ) +} + +internal class DefaultFetchNextTokenAndPaginateTask @Inject constructor( + private val roomAPI: RoomAPI, + private val monarchy: Monarchy, + private val filterRepository: FilterRepository, + private val paginationTask: PaginationTask, + private val eventBus: EventBus +) : FetchNextTokenAndPaginateTask { + + override suspend fun execute(params: FetchNextTokenAndPaginateTask.Params): TokenChunkEventPersistor.Result { + val filter = filterRepository.getRoomFilter() + val response = executeRequest(eventBus) { + apiCall = roomAPI.getContextOfEvent(params.roomId, params.lastKnownEventId, 0, filter) + } + if (response.end == null) { + throw IllegalStateException("No next token found") + } + monarchy.awaitTransaction { + ChunkEntity.findIncludingEvent(it, params.lastKnownEventId)?.nextToken = response.end + } + val paginationParams = PaginationTask.Params( + roomId = params.roomId, + from = response.end, + direction = PaginationDirection.FORWARDS, + limit = params.limit + ) + return paginationTask.execute(paginationParams) + } +} 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 3f5e5ccf06..f7411b3bf1 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 @@ -230,21 +230,9 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy val chunksToDelete = ArrayList() chunks.forEach { if (it != currentChunk) { - if (direction == PaginationDirection.FORWARDS && it.hasBeenALastForwardChunk()) { - // Maybe it was a trick to get a nextToken - if (receivedChunk.events.size == 1) { - Timber.d("Receiving a new nextToken") - it.nextToken = receivedChunk.end - chunksToDelete.add(currentChunk) - } else { - Timber.d("Do not merge $it") - chunksToDelete.add(it) - } - } else { - Timber.d("Merge $it") - currentChunk.merge(roomId, it, direction) - chunksToDelete.add(it) - } + Timber.d("Merge $it") + currentChunk.merge(roomId, it, direction) + chunksToDelete.add(it) } } val shouldUpdateSummary = chunksToDelete.isNotEmpty() && currentChunk.isLastForward && direction == PaginationDirection.FORWARDS From cad14c93d0f1631a2e2c88c1b3377a35e67649ec Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 19 May 2020 14:39:42 +0200 Subject: [PATCH 20/21] Timeline: fix tests and add message order check --- .../matrix/android/common/CommonTestHelper.kt | 10 +++++ .../TimelineBackToPreviousLastForwardTest.kt | 38 ++++--------------- .../timeline/TimelineForwardPaginationTest.kt | 8 ++-- .../TimelinePreviousLastForwardTest.kt | 13 +++++-- 4 files changed, 32 insertions(+), 37 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt index 2529be9547..9b3ebad03b 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/common/CommonTestHelper.kt @@ -368,3 +368,13 @@ class CommonTestHelper(context: Context) { session.close() } } + +fun List.checkSendOrder(baseTextMessage: String, numberOfMessages: Int, startIndex: Int): Boolean { + return drop(startIndex) + .take(numberOfMessages) + .foldRightIndexed(true) { index, timelineEvent, acc -> + val body = timelineEvent.root.content.toModel()?.body + val currentMessageSuffix = numberOfMessages - index + acc && (body == null || body.startsWith(baseTextMessage) && body.endsWith("#$currentMessageSuffix")) + } +} diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt index d55087a8c7..22e9c2f353 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt @@ -25,6 +25,7 @@ import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.matrix.android.api.session.room.timeline.TimelineSettings import im.vector.matrix.android.common.CommonTestHelper import im.vector.matrix.android.common.CryptoTestHelper +import im.vector.matrix.android.common.checkSendOrder import org.amshove.kluent.shouldBeFalse import org.amshove.kluent.shouldBeTrue import org.junit.Assert.assertTrue @@ -90,10 +91,12 @@ class TimelineBackToPreviousLastForwardTest : InstrumentedTest { // Bob stop to sync bobSession.stopSync() + val messageRoot = "First messages from Alice" + // Alice sends 30 messages commonTestHelper.sendTextMessage( roomFromAlicePOV, - "First messages from Alice", + messageRoot, 30) // Bob start to sync @@ -109,7 +112,7 @@ class TimelineBackToPreviousLastForwardTest : InstrumentedTest { // Ok, we have the 10 last messages from Alice. snapshot.size == 10 - && snapshot.all { it.root.content.toModel()?.body?.startsWith("First messages from Alice").orFalse() } + && snapshot.all { it.root.content.toModel()?.body?.startsWith(messageRoot).orFalse() } } bobTimeline.addListener(eventsListener) @@ -159,33 +162,8 @@ class TimelineBackToPreviousLastForwardTest : InstrumentedTest { // Bob can see the first event of the room (so Back pagination has worked) snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE // 8 for room creation item, and 30 for the forward pagination - && snapshot.size == 8 - } - - bobTimeline.addListener(eventsListener) - - bobTimeline.paginate(Timeline.Direction.FORWARDS, 50) - - commonTestHelper.await(lock) - bobTimeline.removeAllListeners() - - bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeTrue() - bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() - } - - // Do it again, now we should have a next token, so we can paginate FORWARD - run { - val lock = CountDownLatch(1) - val eventsListener = commonTestHelper.createEventListener(lock) { snapshot -> - Timber.e("Bob timeline updated: with ${snapshot.size} events:") - snapshot.forEach { - Timber.w(" event ${it.root}") - } - - // Bob can see the first event of the room (so Back pagination has worked) - snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE - // 8 for room creation item, and 30 for the forward pagination - && snapshot.size == 8 + 30 + && snapshot.size == 38 + && snapshot.checkSendOrder(messageRoot, 30, 0) } bobTimeline.addListener(eventsListener) @@ -197,8 +175,8 @@ class TimelineBackToPreviousLastForwardTest : InstrumentedTest { bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() - } + } bobTimeline.dispose() cryptoTestData.cleanUp(commonTestHelper) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt index ae6a9f8d42..adb5c81378 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineForwardPaginationTest.kt @@ -25,6 +25,7 @@ import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.matrix.android.api.session.room.timeline.TimelineSettings import im.vector.matrix.android.common.CommonTestHelper import im.vector.matrix.android.common.CryptoTestHelper +import im.vector.matrix.android.common.checkSendOrder import org.amshove.kluent.shouldBeFalse import org.amshove.kluent.shouldBeTrue import org.junit.FixMethodOrder @@ -58,9 +59,10 @@ class TimelineForwardPaginationTest : InstrumentedTest { val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!! // Alice sends X messages + val message = "Message from Alice" val sentMessages = commonTestHelper.sendTextMessage( roomFromAlicePOV, - "Message from Alice", + message, numberOfMessagesToSend) // Alice clear the cache @@ -85,7 +87,7 @@ class TimelineForwardPaginationTest : InstrumentedTest { // Ok, we have the 10 last messages of the initial sync snapshot.size == 10 - && snapshot.all { it.root.content.toModel()?.body?.startsWith("Message from Alice").orFalse() } + && snapshot.all { it.root.content.toModel()?.body?.startsWith(message).orFalse() } } // Open the timeline at last sent message @@ -163,9 +165,9 @@ class TimelineForwardPaginationTest : InstrumentedTest { snapshot.forEach { Timber.w(" event ${it.root.content}") } - // 6 for room creation item (backward pagination),and numberOfMessagesToSend (all the message of the room) snapshot.size == 6 + numberOfMessagesToSend + && snapshot.checkSendOrder(message, numberOfMessagesToSend, 0) } aliceTimeline.addListener(aliceEventsListener) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt index 0ca5cfdda0..3e673e4c08 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelinePreviousLastForwardTest.kt @@ -25,6 +25,7 @@ import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.matrix.android.api.session.room.timeline.TimelineSettings import im.vector.matrix.android.common.CommonTestHelper import im.vector.matrix.android.common.CryptoTestHelper +import im.vector.matrix.android.common.checkSendOrder import org.amshove.kluent.shouldBeFalse import org.amshove.kluent.shouldBeTrue import org.junit.FixMethodOrder @@ -85,10 +86,11 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { // Bob stop to sync bobSession.stopSync() + val firstMessage = "First messages from Alice" // Alice sends 30 messages val firstMessageFromAliceId = commonTestHelper.sendTextMessage( roomFromAlicePOV, - "First messages from Alice", + firstMessage, 30) .last() .eventId @@ -106,7 +108,7 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { // Ok, we have the 10 last messages from Alice. This will be our future previous lastForward chunk snapshot.size == 10 - && snapshot.all { it.root.content.toModel()?.body?.startsWith("First messages from Alice").orFalse() } + && snapshot.all { it.root.content.toModel()?.body?.startsWith(firstMessage).orFalse() } } bobTimeline.addListener(eventsListener) @@ -120,10 +122,11 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { // Bob stop to sync bobSession.stopSync() + val secondMessage = "Second messages from Alice" // Alice sends again 30 messages commonTestHelper.sendTextMessage( roomFromAlicePOV, - "Second messages from Alice", + secondMessage, 30) // Bob start to sync @@ -139,7 +142,7 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { // Ok, we have the 10 last messages from Alice. This will be our future previous lastForward chunk snapshot.size == 10 - && snapshot.all { it.root.content.toModel()?.body?.startsWith("Second messages from Alice").orFalse() } + && snapshot.all { it.root.content.toModel()?.body?.startsWith(secondMessage).orFalse() } } bobTimeline.addListener(eventsListener) @@ -216,6 +219,8 @@ class TimelinePreviousLastForwardTest : InstrumentedTest { snapshot.lastOrNull()?.root?.getClearType() == EventType.STATE_ROOM_CREATE // 8 for room creation item 60 message from Alice && snapshot.size == 8 + 60 + && snapshot.checkSendOrder(secondMessage, 30, 0) + && snapshot.checkSendOrder(firstMessage, 30, 30) } bobTimeline.addListener(eventsListener) From 01484978bd3b9392df75cf0035fca06d1a98daa9 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 19 May 2020 15:24:36 +0200 Subject: [PATCH 21/21] Fix lint --- .../room/timeline/TimelineBackToPreviousLastForwardTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt index 22e9c2f353..7c7de8170b 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/session/room/timeline/TimelineBackToPreviousLastForwardTest.kt @@ -175,7 +175,6 @@ class TimelineBackToPreviousLastForwardTest : InstrumentedTest { bobTimeline.hasMoreToLoad(Timeline.Direction.FORWARDS).shouldBeFalse() bobTimeline.hasMoreToLoad(Timeline.Direction.BACKWARDS).shouldBeFalse() - } bobTimeline.dispose()