From 0582d0f641b1dfafb55518eb81a728dad5649c36 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 4 Oct 2019 12:12:39 +0200 Subject: [PATCH 1/6] Timeline: fix some crashes --- .../session/room/timeline/DefaultTimeline.kt | 5 +- .../timeline/factory/DefaultItemFactory.kt | 29 ++++++----- .../timeline/factory/MessageItemFactory.kt | 49 ++++++++++--------- 3 files changed, 45 insertions(+), 38 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 3afde4efb5..da26839fa4 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 @@ -176,8 +176,8 @@ internal class DefaultTimeline( override fun start() { if (isStarted.compareAndSet(false, true)) { Timber.v("Start timeline for roomId: $roomId and eventId: $initialEventId") - eventDecryptor.start() BACKGROUND_HANDLER.post { + eventDecryptor.start() val realm = Realm.getInstance(realmConfiguration) backgroundRealm.set(realm) clearUnlinkedEvents(realm) @@ -212,7 +212,6 @@ internal class DefaultTimeline( isReady.set(false) Timber.v("Dispose timeline for roomId: $roomId and eventId: $initialEventId") cancelableBag.cancel() - BACKGROUND_HANDLER.removeCallbacksAndMessages(null) BACKGROUND_HANDLER.post { roomEntity?.sendingTimelineEvents?.removeAllChangeListeners() eventRelations.removeAllChangeListeners() @@ -225,8 +224,8 @@ internal class DefaultTimeline( backgroundRealm.getAndSet(null).also { it.close() } + eventDecryptor.destroy() } - eventDecryptor.destroy() } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt index 959079bf8b..f0671decd8 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt @@ -23,24 +23,17 @@ import im.vector.riotx.features.home.room.detail.timeline.TimelineEventControlle import im.vector.riotx.features.home.room.detail.timeline.helper.MessageInformationDataFactory import im.vector.riotx.features.home.room.detail.timeline.item.DefaultItem import im.vector.riotx.features.home.room.detail.timeline.item.DefaultItem_ +import im.vector.riotx.features.home.room.detail.timeline.item.MessageInformationData import javax.inject.Inject class DefaultItemFactory @Inject constructor(private val avatarSizeProvider: AvatarSizeProvider, private val avatarRenderer: AvatarRenderer, private val informationDataFactory: MessageInformationDataFactory) { - fun create(event: TimelineEvent, + fun create(text: String, + informationData: MessageInformationData, highlight: Boolean, - readMarkerVisible: Boolean, - callback: TimelineEventController.Callback?, - exception: Exception? = null): DefaultItem? { - val text = if (exception == null) { - "${event.root.getClearType()} events are not yet handled" - } else { - "an exception occurred when rendering the event ${event.root.eventId}" - } - - val informationData = informationDataFactory.create(event, null, readMarkerVisible) + callback: TimelineEventController.Callback?): DefaultItem? { return DefaultItem_() .leftGuideline(avatarSizeProvider.leftGuideline) @@ -52,4 +45,18 @@ class DefaultItemFactory @Inject constructor(private val avatarSizeProvider: Ava .readReceiptsCallback(callback) } + fun create(event: TimelineEvent, + highlight: Boolean, + readMarkerVisible: Boolean, + callback: TimelineEventController.Callback?, + exception: Exception? = null): DefaultItem? { + val text = if (exception == null) { + "${event.root.getClearType()} events are not yet handled" + } else { + "an exception occurred when rendering the event ${event.root.eventId}" + } + val informationData = informationDataFactory.create(event, null, readMarkerVisible) + return create(text, informationData, highlight, callback) + } + } \ No newline at end of file diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MessageItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MessageItemFactory.kt index 35ca7d7b9a..91b60c6fbe 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MessageItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MessageItemFactory.kt @@ -73,6 +73,7 @@ class MessageItemFactory @Inject constructor( private val messageInformationDataFactory: MessageInformationDataFactory, private val messageItemAttributesFactory: MessageItemAttributesFactory, private val contentUploadStateTrackerBinder: ContentUploadStateTrackerBinder, + private val defaultItemFactory: DefaultItemFactory, private val noticeItemFactory: NoticeItemFactory, private val avatarSizeProvider: AvatarSizeProvider) { @@ -93,13 +94,13 @@ class MessageItemFactory @Inject constructor( return buildRedactedItem(attributes, highlight) } - val messageContent: MessageContent = - event.getLastMessageContent() - ?: //Malformed content, we should echo something on screen - return DefaultItem_().text(stringProvider.getString(R.string.malformed_message)) - + val messageContent = event.getLastMessageContent() + if (messageContent == null) { + val malformedText = stringProvider.getString(R.string.malformed_message) + return defaultItemFactory.create(malformedText, informationData, highlight, callback) + } if (messageContent.relatesTo?.type == RelationType.REPLACE - || event.isEncrypted() && event.root.content.toModel()?.relatesTo?.type == RelationType.REPLACE + || event.isEncrypted() && event.root.content.toModel()?.relatesTo?.type == RelationType.REPLACE ) { // This is an edit event, we should it when debugging as a notice event return noticeItemFactory.create(event, highlight, readMarkerVisible, callback) @@ -110,21 +111,21 @@ class MessageItemFactory @Inject constructor( // val ev = all.toModel() return when (messageContent) { is MessageEmoteContent -> buildEmoteMessageItem(messageContent, - informationData, - highlight, - callback, - attributes) + informationData, + highlight, + callback, + attributes) is MessageTextContent -> buildTextMessageItem(messageContent, - informationData, - highlight, - callback, - attributes) + informationData, + highlight, + callback, + attributes) is MessageImageContent -> buildImageMessageItem(messageContent, informationData, highlight, callback, attributes) is MessageNoticeContent -> buildNoticeMessageItem(messageContent, informationData, highlight, callback, attributes) is MessageVideoContent -> buildVideoMessageItem(messageContent, informationData, highlight, callback, attributes) is MessageFileContent -> buildFileMessageItem(messageContent, informationData, highlight, callback, attributes) is MessageAudioContent -> buildAudioMessageItem(messageContent, informationData, highlight, callback, attributes) - else -> buildNotHandledMessageItem(messageContent, highlight) + else -> buildNotHandledMessageItem(messageContent, informationData, highlight, callback) } } @@ -166,12 +167,12 @@ class MessageItemFactory @Inject constructor( })) } - private fun buildNotHandledMessageItem(messageContent: MessageContent, highlight: Boolean): DefaultItem? { + private fun buildNotHandledMessageItem(messageContent: MessageContent, + informationData: MessageInformationData, + highlight: Boolean, + callback: TimelineEventController.Callback?): DefaultItem? { val text = "${messageContent.type} message events are not yet handled" - return DefaultItem_() - .leftGuideline(avatarSizeProvider.leftGuideline) - .text(text) - .highlighted(highlight) + return defaultItemFactory.create(text, informationData, highlight, callback) } private fun buildImageMessageItem(messageContent: MessageImageContent, @@ -216,7 +217,7 @@ class MessageItemFactory @Inject constructor( val thumbnailData = ImageContentRenderer.Data( filename = messageContent.body, url = messageContent.videoInfo?.thumbnailFile?.url - ?: messageContent.videoInfo?.thumbnailUrl, + ?: messageContent.videoInfo?.thumbnailUrl, elementToDecrypt = messageContent.videoInfo?.thumbnailFile?.toElementToDecrypt(), height = messageContent.videoInfo?.height, maxHeight = maxHeight, @@ -297,9 +298,9 @@ class MessageItemFactory @Inject constructor( //nop } }, - editStart, - editEnd, - Spanned.SPAN_INCLUSIVE_EXCLUSIVE) + editStart, + editEnd, + Spanned.SPAN_INCLUSIVE_EXCLUSIVE) return spannable } From 4e4fb4c5659e720eb8b39ac996644c18ca762c7f Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 4 Oct 2019 19:36:10 +0200 Subject: [PATCH 2/6] Crypto store: fix potential issue with realm open/close process --- .../internal/crypto/store/db/Helper.kt | 31 +++++++++---------- .../crypto/store/db/RealmCryptoStore.kt | 4 +-- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt index 4ba484ae1a..b1e1cb4426 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt @@ -32,41 +32,38 @@ import java.util.zip.GZIPInputStream * Get realm, invoke the action, close realm, and return the result of the action */ fun doWithRealm(realmConfiguration: RealmConfiguration, action: (Realm) -> T): T { - val realm = Realm.getInstance(realmConfiguration) - val result = action.invoke(realm) - realm.close() - return result + return Realm.getInstance(realmConfiguration).use { realm -> + action.invoke(realm) + } } /** * Get realm, do the query, copy from realm, close realm, and return the copied result */ fun doRealmQueryAndCopy(realmConfiguration: RealmConfiguration, action: (Realm) -> T?): T? { - val realm = Realm.getInstance(realmConfiguration) - val result = action.invoke(realm) - val copiedResult = result?.let { realm.copyFromRealm(result) } - realm.close() - return copiedResult + return Realm.getInstance(realmConfiguration).use { realm -> + val result = action.invoke(realm) + result?.let { realm.copyFromRealm(result) } + } } /** * Get realm, do the list query, copy from realm, close realm, and return the copied result */ fun doRealmQueryAndCopyList(realmConfiguration: RealmConfiguration, action: (Realm) -> Iterable): Iterable { - val realm = Realm.getInstance(realmConfiguration) - val result = action.invoke(realm) - val copiedResult = realm.copyFromRealm(result) - realm.close() - return copiedResult + return Realm.getInstance(realmConfiguration).use { realm -> + val result = action.invoke(realm) + return realm.copyFromRealm(result) + } } /** * Get realm instance, invoke the action in a transaction and close realm */ fun doRealmTransaction(realmConfiguration: RealmConfiguration, action: (Realm) -> Unit) { - val realm = Realm.getInstance(realmConfiguration) - realm.executeTransaction { action.invoke(it) } - realm.close() + Realm.getInstance(realmConfiguration).use { realm -> + realm.executeTransaction { action.invoke(realm) } + } } /** diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt index 9865614c9f..87e8b55b50 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt @@ -360,7 +360,7 @@ internal class RealmCryptoStore(private val realmConfiguration: RealmConfigurati return } - doRealmTransaction(realmConfiguration) { + doRealmTransaction(realmConfiguration) { realm -> sessions.forEach { session -> var sessionIdentifier: String? = null @@ -387,7 +387,7 @@ internal class RealmCryptoStore(private val realmConfiguration: RealmConfigurati putInboundGroupSession(session) } - it.insertOrUpdate(realmOlmInboundGroupSession) + realm.insertOrUpdate(realmOlmInboundGroupSession) } } } From 9c5987b6822816d2385127d18167e683aead7101 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 4 Oct 2019 19:36:22 +0200 Subject: [PATCH 3/6] SAS: fix potential crash --- .../features/crypto/verification/SASVerificationActivity.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/SASVerificationActivity.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/SASVerificationActivity.kt index 2c695cad6e..de10e8d3b2 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/verification/SASVerificationActivity.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/SASVerificationActivity.kt @@ -97,8 +97,9 @@ class SASVerificationActivity : SimpleFragmentActivity() { } if (isIncoming) { - val incoming = viewModel.transaction as IncomingSasVerificationTransaction - when (incoming.uxState) { + val incoming = viewModel.transaction as? IncomingSasVerificationTransaction + when (incoming?.uxState) { + null, IncomingSasVerificationTransaction.UxState.UNKNOWN, IncomingSasVerificationTransaction.UxState.SHOW_ACCEPT, IncomingSasVerificationTransaction.UxState.WAIT_FOR_KEY_AGREEMENT -> { From 1931a1a4a46497cfbd24a896bb6a3e8f4997bab0 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 4 Oct 2019 19:37:23 +0200 Subject: [PATCH 4/6] Sync: use some suspending function where it makes sense --- .../android/internal/session/sync/GroupSyncHandler.kt | 5 +++-- .../android/internal/session/sync/RoomSyncHandler.kt | 7 ++++--- .../android/internal/session/sync/SyncResponseHandler.kt | 2 +- .../matrix/android/internal/session/sync/SyncTask.kt | 2 +- .../internal/session/sync/UserAccountDataSyncHandler.kt | 5 +++-- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/GroupSyncHandler.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/GroupSyncHandler.kt index 9355384ef5..2fc733f4d5 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/GroupSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/GroupSyncHandler.kt @@ -25,6 +25,7 @@ import im.vector.matrix.android.internal.session.DefaultInitialSyncProgressServi import im.vector.matrix.android.internal.session.mapWithProgress import im.vector.matrix.android.internal.session.sync.model.GroupsSyncResponse import im.vector.matrix.android.internal.session.sync.model.InvitedGroupSync +import im.vector.matrix.android.internal.util.awaitTransaction import io.realm.Realm import javax.inject.Inject @@ -36,8 +37,8 @@ internal class GroupSyncHandler @Inject constructor(private val monarchy: Monarc data class LEFT(val data: Map) : HandlingStrategy() } - fun handle(roomsSyncResponse: GroupsSyncResponse, reporter: DefaultInitialSyncProgressService? = null) { - monarchy.runTransactionSync { realm -> + suspend fun handle(roomsSyncResponse: GroupsSyncResponse, reporter: DefaultInitialSyncProgressService? = null) { + monarchy.awaitTransaction { realm -> handleGroupSync(realm, HandlingStrategy.JOINED(roomsSyncResponse.join), reporter) handleGroupSync(realm, HandlingStrategy.INVITED(roomsSyncResponse.invite), reporter) handleGroupSync(realm, HandlingStrategy.LEFT(roomsSyncResponse.leave), reporter) 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 91119f6376..43d6462899 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 @@ -51,6 +51,7 @@ import im.vector.matrix.android.internal.session.sync.model.RoomsSyncResponse import im.vector.matrix.android.internal.session.user.UserEntityFactory import im.vector.matrix.android.internal.task.TaskExecutor import im.vector.matrix.android.internal.task.configureWith +import im.vector.matrix.android.internal.util.awaitTransaction import io.realm.Realm import io.realm.kotlin.createObject import timber.log.Timber @@ -73,13 +74,12 @@ internal class RoomSyncHandler @Inject constructor(private val monarchy: Monarch data class LEFT(val data: Map) : HandlingStrategy() } - fun handle(roomsSyncResponse: RoomsSyncResponse, isInitialSync: Boolean, reporter: DefaultInitialSyncProgressService? = null) { - monarchy.runTransactionSync { realm -> + suspend fun handle(roomsSyncResponse: RoomsSyncResponse, isInitialSync: Boolean, reporter: DefaultInitialSyncProgressService? = null) { + monarchy.awaitTransaction { realm -> handleRoomSync(realm, HandlingStrategy.JOINED(roomsSyncResponse.join), isInitialSync, reporter) handleRoomSync(realm, HandlingStrategy.INVITED(roomsSyncResponse.invite), isInitialSync, reporter) handleRoomSync(realm, HandlingStrategy.LEFT(roomsSyncResponse.leave), isInitialSync, reporter) } - //handle event for bing rule checks checkPushRules(roomsSyncResponse) @@ -126,6 +126,7 @@ internal class RoomSyncHandler @Inject constructor(private val monarchy: Monarch roomSync: RoomSync, isInitialSync: Boolean): RoomEntity { + Timber.v("Handle join sync for room $roomId") if (roomSync.ephemeral != null && roomSync.ephemeral.events.isNotEmpty()) { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncResponseHandler.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncResponseHandler.kt index 1b843bda19..1e2bf359ec 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncResponseHandler.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncResponseHandler.kt @@ -32,7 +32,7 @@ internal class SyncResponseHandler @Inject constructor(private val roomSyncHandl private val cryptoService: DefaultCryptoService, private val initialSyncProgressService: DefaultInitialSyncProgressService) { - suspend fun handleResponse(syncResponse: SyncResponse, fromToken: String?, isCatchingUp: Boolean) { + suspend fun handleResponse(syncResponse: SyncResponse, fromToken: String?) { val isInitialSync = fromToken == null Timber.v("Start handling sync, is InitialSync: $isInitialSync") val reporter = initialSyncProgressService.takeIf { isInitialSync } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncTask.kt index f9cbd05d26..df4d5f3aa9 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/SyncTask.kt @@ -77,7 +77,7 @@ internal class DefaultSyncTask @Inject constructor(private val syncAPI: SyncAPI, } throw throwable } - syncResponseHandler.handleResponse(syncResponse, token, false) + syncResponseHandler.handleResponse(syncResponse, token) syncTokenStore.saveToken(syncResponse.nextBatch) if (isInitialSync) { initialSyncProgressService.endAll() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/UserAccountDataSyncHandler.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/UserAccountDataSyncHandler.kt index 87b4c2d1c1..6446efb348 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/UserAccountDataSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/UserAccountDataSyncHandler.kt @@ -34,6 +34,7 @@ import im.vector.matrix.android.internal.session.user.accountdata.DirectChatsHel import im.vector.matrix.android.internal.session.user.accountdata.UpdateUserAccountDataTask import im.vector.matrix.android.internal.task.TaskExecutor import im.vector.matrix.android.internal.task.configureWith +import im.vector.matrix.android.internal.util.awaitTransaction import io.realm.Realm import timber.log.Timber import javax.inject.Inject @@ -62,8 +63,8 @@ internal class UserAccountDataSyncHandler @Inject constructor(private val monarc savePushRulesTask.execute(SavePushRulesTask.Params(userAccountDataPushRules.content)) } - private fun handleDirectChatRooms(directMessages: UserAccountDataDirectMessages) { - monarchy.runTransactionSync { realm -> + private suspend fun handleDirectChatRooms(directMessages: UserAccountDataDirectMessages) { + monarchy.awaitTransaction { realm -> val oldDirectRooms = RoomSummaryEntity.getDirectRooms(realm) oldDirectRooms.forEach { it.isDirect = false From 77de059dc9a82691a5a186d8bf5938d1e5f8409d Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 4 Oct 2019 19:37:44 +0200 Subject: [PATCH 5/6] Timeline: fix potential issues when starting/disposing the timeline --- .../internal/session/room/RoomModule.kt | 5 ++ .../room/timeline/ClearUnlinkedEventsTask.kt | 50 +++++++++++++++++++ .../session/room/timeline/DefaultTimeline.kt | 38 +++++++------- .../room/timeline/DefaultTimelineService.kt | 4 +- .../room/timeline/TimelineEventDecryptor.kt | 21 +++++--- .../timeline/TimelineHiddenReadReceipts.kt | 4 +- 6 files changed, 92 insertions(+), 30 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/ClearUnlinkedEventsTask.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 95edf3dbaa..b3db84c9c6 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 @@ -50,6 +50,8 @@ import im.vector.matrix.android.internal.session.room.relation.FindReactionEvent import im.vector.matrix.android.internal.session.room.relation.UpdateQuickReactionTask 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.* +import im.vector.matrix.android.internal.session.room.timeline.ClearUnlinkedEventsTask 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.GetContextOfEventTask @@ -120,6 +122,9 @@ internal abstract class RoomModule { @Binds abstract fun bindGetContextOfEventTask(getContextOfEventTask: DefaultGetContextOfEventTask): GetContextOfEventTask + @Binds + abstract fun bindClearUnlinkedEventsTask(clearUnlinkedEventsTask: DefaultClearUnlinkedEventsTask): ClearUnlinkedEventsTask + @Binds abstract fun bindPaginationTask(paginationTask: DefaultPaginationTask): PaginationTask diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/ClearUnlinkedEventsTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/ClearUnlinkedEventsTask.kt new file mode 100644 index 0000000000..df7a0cc886 --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/ClearUnlinkedEventsTask.kt @@ -0,0 +1,50 @@ +/* + + * Copyright 2019 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.helper.deleteOnCascade +import im.vector.matrix.android.internal.database.model.ChunkEntity +import im.vector.matrix.android.internal.database.model.ChunkEntityFields +import im.vector.matrix.android.internal.database.model.EventEntityFields +import im.vector.matrix.android.internal.database.query.where +import im.vector.matrix.android.internal.task.Task +import im.vector.matrix.android.internal.util.awaitTransaction +import javax.inject.Inject + +internal interface ClearUnlinkedEventsTask : Task { + + data class Params(val roomId: String) + +} + +internal class DefaultClearUnlinkedEventsTask @Inject constructor(private val monarchy: Monarchy) : ClearUnlinkedEventsTask { + + override suspend fun execute(params: ClearUnlinkedEventsTask.Params) { + monarchy.awaitTransaction { localRealm -> + val unlinkedChunks = ChunkEntity + .where(localRealm, roomId = params.roomId) + .equalTo("${ChunkEntityFields.TIMELINE_EVENTS.ROOT}.${EventEntityFields.IS_UNLINKED}", true) + .findAll() + unlinkedChunks.forEach { + it.deleteOnCascade() + } + } + } + +} 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 da26839fa4..63012b65ca 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 @@ -53,6 +53,8 @@ import io.realm.RealmConfiguration import io.realm.RealmQuery import io.realm.RealmResults import io.realm.Sort +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.launch import timber.log.Timber import java.util.* import java.util.concurrent.atomic.AtomicBoolean @@ -71,6 +73,7 @@ internal class DefaultTimeline( private val realmConfiguration: RealmConfiguration, private val taskExecutor: TaskExecutor, private val contextOfEventTask: GetContextOfEventTask, + private val clearUnlinkedEventsTask: ClearUnlinkedEventsTask, private val paginationTask: PaginationTask, private val cryptoService: CryptoService, private val timelineEventMapper: TimelineEventMapper, @@ -180,7 +183,6 @@ internal class DefaultTimeline( eventDecryptor.start() val realm = Realm.getInstance(realmConfiguration) backgroundRealm.set(realm) - clearUnlinkedEvents(realm) roomEntity = RoomEntity.where(realm, roomId = roomId).findFirst()?.also { it.sendingTimelineEvents.addChangeListener { _ -> @@ -212,20 +214,28 @@ internal class DefaultTimeline( isReady.set(false) Timber.v("Dispose timeline for roomId: $roomId and eventId: $initialEventId") cancelableBag.cancel() + BACKGROUND_HANDLER.removeCallbacksAndMessages(null) BACKGROUND_HANDLER.post { roomEntity?.sendingTimelineEvents?.removeAllChangeListeners() - eventRelations.removeAllChangeListeners() - filteredEvents.removeAllChangeListeners() + if (this::eventRelations.isInitialized) { + eventRelations.removeAllChangeListeners() + } + if (this::filteredEvents.isInitialized) { + filteredEvents.removeAllChangeListeners() + } hiddenReadMarker.dispose() if (settings.buildReadReceipts) { hiddenReadReceipts.dispose() } clearAllValues() backgroundRealm.getAndSet(null).also { - it.close() + it?.close() } eventDecryptor.destroy() } + clearUnlinkedEventsTask + .configureWith(ClearUnlinkedEventsTask.Params(roomId)) + .executeBy(taskExecutor) } } @@ -499,9 +509,9 @@ internal class DefaultTimeline( return } val params = PaginationTask.Params(roomId = roomId, - from = token, - direction = direction.toPaginationDirection(), - limit = limit) + from = token, + direction = direction.toPaginationDirection(), + limit = limit) Timber.v("Should fetch $limit items $direction") cancelableBag += paginationTask @@ -577,7 +587,7 @@ internal class DefaultTimeline( val timelineEvent = buildTimelineEvent(eventEntity) if (timelineEvent.isEncrypted() - && timelineEvent.root.mxDecryptionResult == null) { + && timelineEvent.root.mxDecryptionResult == null) { timelineEvent.root.eventId?.let { eventDecryptor.requestDecryption(it) } } @@ -639,18 +649,6 @@ internal class DefaultTimeline( } } - private fun clearUnlinkedEvents(realm: Realm) { - realm.executeTransaction { localRealm -> - val unlinkedChunks = ChunkEntity - .where(localRealm, roomId = roomId) - .equalTo("${ChunkEntityFields.TIMELINE_EVENTS.ROOT}.${EventEntityFields.IS_UNLINKED}", true) - .findAll() - unlinkedChunks.forEach { - it.deleteOnCascade() - } - } - } - private fun fetchEvent(eventId: String) { val params = GetContextOfEventTask.Params(roomId, eventId) cancelableBag += contextOfEventTask.configureWith(params).executeBy(taskExecutor) 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 4c4b08ce4f..1e3bcbc089 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 @@ -41,7 +41,8 @@ internal class DefaultTimelineService @AssistedInject constructor(@Assisted priv private val cryptoService: CryptoService, private val paginationTask: PaginationTask, private val timelineEventMapper: TimelineEventMapper, - private val readReceiptsSummaryMapper: ReadReceiptsSummaryMapper + private val readReceiptsSummaryMapper: ReadReceiptsSummaryMapper, + private val clearUnlinkedEventsTask: ClearUnlinkedEventsTask ) : TimelineService { @AssistedInject.Factory @@ -55,6 +56,7 @@ internal class DefaultTimelineService @AssistedInject constructor(@Assisted priv monarchy.realmConfiguration, taskExecutor, contextOfEventTask, + clearUnlinkedEventsTask, paginationTask, cryptoService, timelineEventMapper, diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineEventDecryptor.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineEventDecryptor.kt index 64d5f120da..14c044f42d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineEventDecryptor.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineEventDecryptor.kt @@ -26,6 +26,7 @@ import im.vector.matrix.android.internal.database.query.where import io.realm.Realm import io.realm.RealmConfiguration import timber.log.Timber +import java.util.concurrent.ExecutorService import java.util.concurrent.Executors @@ -53,18 +54,20 @@ internal class TimelineEventDecryptor( } - private val executor = Executors.newSingleThreadExecutor() + private var executor: ExecutorService? = null private val existingRequests = HashSet() private val unknownSessionsFailure = HashMap>() fun start() { + executor = Executors.newSingleThreadExecutor() cryptoService.addNewSessionListener(newSessionListener) } fun destroy() { cryptoService.removeSessionListener(newSessionListener) - executor.shutdownNow() + executor?.shutdownNow() + executor = null unknownSessionsFailure.clear() existingRequests.clear() } @@ -85,11 +88,9 @@ internal class TimelineEventDecryptor( } } } - executor.execute { + executor?.execute { Realm.getInstance(realmConfiguration).use { realm -> - realm.executeTransaction { - processDecryptRequest(eventId, it) - } + processDecryptRequest(eventId, realm) } } } @@ -104,12 +105,16 @@ internal class TimelineEventDecryptor( try { val result = cryptoService.decryptEvent(event, timelineId) Timber.v("Successfully decrypted event ${eventId}") - eventEntity.setDecryptionResult(result) + realm.executeTransaction { + eventEntity.setDecryptionResult(result) + } } catch (e: MXCryptoError) { Timber.v("Failed to decrypt event ${eventId} ${e}") if (e is MXCryptoError.Base && e.errorType == MXCryptoError.ErrorType.UNKNOWN_INBOUND_SESSION_ID) { //Keep track of unknown sessions to automatically try to decrypt on new session - eventEntity.decryptionErrorCode = e.errorType.name + realm.executeTransaction { + eventEntity.decryptionErrorCode = e.errorType.name + } event.content?.toModel()?.let { content -> content.sessionId?.let { sessionId -> synchronized(unknownSessionsFailure) { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineHiddenReadReceipts.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineHiddenReadReceipts.kt index 39c2535282..1a4958e656 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineHiddenReadReceipts.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineHiddenReadReceipts.kt @@ -132,7 +132,9 @@ internal class TimelineHiddenReadReceipts constructor(private val readReceiptsSu * Dispose the realm query subscription. Has to be called on an HandlerThread */ fun dispose() { - this.hiddenReadReceipts.removeAllChangeListeners() + if (this::hiddenReadReceipts.isInitialized) { + this.hiddenReadReceipts.removeAllChangeListeners() + } } /** From 8e39fd2a703502f4d5e1da8b2c500b1a2486aa9b Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 7 Oct 2019 14:45:58 +0200 Subject: [PATCH 6/6] Clean after benoit's review --- .../vector/matrix/android/internal/crypto/store/db/Helper.kt | 4 ++-- .../home/room/detail/timeline/factory/DefaultItemFactory.kt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt index b1e1cb4426..3f9aea3369 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/Helper.kt @@ -43,7 +43,7 @@ fun doWithRealm(realmConfiguration: RealmConfiguration, action: (Realm) -> T fun doRealmQueryAndCopy(realmConfiguration: RealmConfiguration, action: (Realm) -> T?): T? { return Realm.getInstance(realmConfiguration).use { realm -> val result = action.invoke(realm) - result?.let { realm.copyFromRealm(result) } + result?.let { realm.copyFromRealm(it) } } } @@ -53,7 +53,7 @@ fun doRealmQueryAndCopy(realmConfiguration: RealmConfiguration fun doRealmQueryAndCopyList(realmConfiguration: RealmConfiguration, action: (Realm) -> Iterable): Iterable { return Realm.getInstance(realmConfiguration).use { realm -> val result = action.invoke(realm) - return realm.copyFromRealm(result) + realm.copyFromRealm(result) } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt index f0671decd8..cfa689b7ff 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/DefaultItemFactory.kt @@ -33,7 +33,7 @@ class DefaultItemFactory @Inject constructor(private val avatarSizeProvider: Ava fun create(text: String, informationData: MessageInformationData, highlight: Boolean, - callback: TimelineEventController.Callback?): DefaultItem? { + callback: TimelineEventController.Callback?): DefaultItem { return DefaultItem_() .leftGuideline(avatarSizeProvider.leftGuideline) @@ -49,7 +49,7 @@ class DefaultItemFactory @Inject constructor(private val avatarSizeProvider: Ava highlight: Boolean, readMarkerVisible: Boolean, callback: TimelineEventController.Callback?, - exception: Exception? = null): DefaultItem? { + exception: Exception? = null): DefaultItem { val text = if (exception == null) { "${event.root.getClearType()} events are not yet handled" } else {