Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/aris/thread aware #4396

Merged
merged 10 commits into from
Nov 18, 2021
Merged

Feature/aris/thread aware #4396

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f00749
Thread awareness, map threads events to replies
ariskotsomitopoulos Oct 25, 2021
d1f3e3f
Thread awareness, map threads events to replies
ariskotsomitopoulos Oct 26, 2021
45a63b7
Make Android app thread aware. Handling also extreme cases like reall…
ariskotsomitopoulos Nov 2, 2021
8ee3f2c
Delete ThreadToReplyMapInterceptor
ariskotsomitopoulos Nov 3, 2021
4192c1c
Add changelog file
ariskotsomitopoulos Nov 3, 2021
ec366f1
PR Remarks
ariskotsomitopoulos Nov 4, 2021
88656ce
Use GetEventTask instead of session from the ThreadAwarenessHandler
ariskotsomitopoulos Nov 17, 2021
d463500
Remove unused import
ariskotsomitopoulos Nov 17, 2021
9972dbc
Use RealmConfiguration instead of Monarchy
ariskotsomitopoulos Nov 17, 2021
8015ffe
PR remrarks
ariskotsomitopoulos Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4246.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make Element Android Thread aware
4 changes: 4 additions & 0 deletions ...sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/RelationType.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ object RelationType {
/** Lets you define an event which references an existing event.*/
const val REFERENCE = "m.reference"

/** Lets you define an thread event that belongs to another existing event.*/
// const val THREAD = "m.thread" // m.thread is not yet released in the backend
const val THREAD = "io.element.thread" // io.element.thread will be replaced by m.thread when it is released

/** Lets you define an event which adds a response to an existing event.*/
const val RESPONSE = "org.matrix.response"
}
5 changes: 3 additions & 2 deletions ...x-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/EventEntity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.database.model
import io.realm.RealmObject
import io.realm.annotations.Index
import org.matrix.android.sdk.api.session.room.send.SendState
import org.matrix.android.sdk.api.util.JsonDict
import org.matrix.android.sdk.internal.crypto.MXEventDecryptionResult
import org.matrix.android.sdk.internal.crypto.algorithms.olm.OlmDecryptionResult
import org.matrix.android.sdk.internal.di.MoshiProvider
Expand Down Expand Up @@ -56,10 +57,10 @@ internal open class EventEntity(@Index var eventId: String = "",

companion object

fun setDecryptionResult(result: MXEventDecryptionResult) {
fun setDecryptionResult(result: MXEventDecryptionResult, clearEvent: JsonDict? = null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is strange, let me see the rest of the changes to understand why it is done

assertIsManaged()
val decryptionResult = OlmDecryptionResult(
payload = result.clearEvent,
payload = clearEvent ?: result.clearEvent,
senderKey = result.senderCurve25519Key,
keysClaimed = result.claimedEd25519Key?.let { mapOf("ed25519" to it) },
forwardingCurve25519KeyChain = result.forwardingCurve25519KeyChain
Expand Down
22 changes: 22 additions & 0 deletions ...id/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.realm.RealmConfiguration
import io.realm.RealmQuery
import io.realm.RealmResults
import io.realm.Sort
import kotlinx.coroutines.runBlocking
import org.matrix.android.sdk.api.MatrixCallback
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.extensions.tryOrNull
Expand All @@ -33,6 +34,7 @@ import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import org.matrix.android.sdk.api.session.room.timeline.TimelineSettings
import org.matrix.android.sdk.api.util.CancelableBag
import org.matrix.android.sdk.internal.database.RealmSessionProvider
import org.matrix.android.sdk.internal.database.mapper.EventMapper
import org.matrix.android.sdk.internal.database.mapper.TimelineEventMapper
import org.matrix.android.sdk.internal.database.model.ChunkEntity
import org.matrix.android.sdk.internal.database.model.RoomEntity
Expand All @@ -43,6 +45,7 @@ import org.matrix.android.sdk.internal.database.query.where
import org.matrix.android.sdk.internal.database.query.whereRoomId
import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask
import org.matrix.android.sdk.internal.session.sync.handler.room.ReadReceiptHandler
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
import org.matrix.android.sdk.internal.task.TaskExecutor
import org.matrix.android.sdk.internal.task.configureWith
import org.matrix.android.sdk.internal.util.Debouncer
Expand Down Expand Up @@ -72,6 +75,7 @@ internal class DefaultTimeline(
private val eventDecryptor: TimelineEventDecryptor,
private val realmSessionProvider: RealmSessionProvider,
private val loadRoomMembersTask: LoadRoomMembersTask,
private val threadsAwarenessHandler: ThreadsAwarenessHandler,
private val readReceiptHandler: ReadReceiptHandler
) : Timeline,
TimelineInput.Listener,
Expand Down Expand Up @@ -577,6 +581,10 @@ internal class DefaultTimeline(
} else {
nextDisplayIndex = offsetIndex + 1
}

// Prerequisite to in order for the ThreadsAwarenessHandler to work properly
fetchRootThreadEventsIfNeeded(offsetResults)

offsetResults.forEach { eventEntity ->

val timelineEvent = buildTimelineEvent(eventEntity)
Expand All @@ -601,6 +609,20 @@ internal class DefaultTimeline(
return offsetResults.size
}

/**
* This function is responsible to fetch and store the root event of a thread event
* in order to be able to display the event to the user appropriately
*/
private fun fetchRootThreadEventsIfNeeded(offsetResults: RealmResults<TimelineEventEntity>) = runBlocking {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use runBlocking

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, but we are already in a thread (and the UI will not freeze) and it's the only way to ensure that the next code block will have the required info, if you can suggest an alternative to that it's welcome. Furthermore keep in mind that this is a temporary fix and will be removed after the thread release

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes thats not really great but it's ok I guess for now. Note that my rework of the timeline authorise direct usage of suspend methods

val eventEntityList = offsetResults
.mapNotNull {
it?.root
}.map {
EventMapper.map(it)
}
threadsAwarenessHandler.fetchRootThreadEventsIfNeeded(eventEntityList)
}

private fun buildTimelineEvent(eventEntity: TimelineEventEntity): TimelineEvent {
return timelineEventMapper.map(
timelineEventEntity = eventEntity,
Expand Down
3 changes: 3 additions & 0 deletions ...main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimelineService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import org.matrix.android.sdk.internal.database.query.where
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask
import org.matrix.android.sdk.internal.session.sync.handler.room.ReadReceiptHandler
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
import org.matrix.android.sdk.internal.task.TaskExecutor

internal class DefaultTimelineService @AssistedInject constructor(
Expand All @@ -52,6 +53,7 @@ internal class DefaultTimelineService @AssistedInject constructor(
private val fetchTokenAndPaginateTask: FetchTokenAndPaginateTask,
private val timelineEventMapper: TimelineEventMapper,
private val loadRoomMembersTask: LoadRoomMembersTask,
private val threadsAwarenessHandler: ThreadsAwarenessHandler,
private val readReceiptHandler: ReadReceiptHandler
) : TimelineService {

Expand All @@ -75,6 +77,7 @@ internal class DefaultTimelineService @AssistedInject constructor(
fetchTokenAndPaginateTask = fetchTokenAndPaginateTask,
realmSessionProvider = realmSessionProvider,
loadRoomMembersTask = loadRoomMembersTask,
threadsAwarenessHandler = threadsAwarenessHandler,
readReceiptHandler = readReceiptHandler
)
}
Expand Down
19 changes: 15 additions & 4 deletions ...main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineEventDecryptor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.matrix.android.sdk.internal.crypto.model.event.EncryptedEventContent
import org.matrix.android.sdk.internal.database.model.EventEntity
import org.matrix.android.sdk.internal.database.query.where
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
import timber.log.Timber
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
Expand All @@ -34,7 +35,8 @@ import javax.inject.Inject
internal class TimelineEventDecryptor @Inject constructor(
@SessionDatabase
private val realmConfiguration: RealmConfiguration,
private val cryptoService: CryptoService
private val cryptoService: CryptoService,
private val threadsAwarenessHandler: ThreadsAwarenessHandler
) {

private val newSessionListener = object : NewSessionListener {
Expand Down Expand Up @@ -106,10 +108,19 @@ internal class TimelineEventDecryptor @Inject constructor(
val result = cryptoService.decryptEvent(request.event, timelineId)
Timber.v("Successfully decrypted event ${event.eventId}")
realm.executeTransaction {
val eventId = event.eventId ?: ""
EventEntity.where(it, eventId = eventId)
val eventId = event.eventId ?: return@executeTransaction
ariskotsomitopoulos marked this conversation as resolved.
Show resolved Hide resolved
val eventEntity = EventEntity
.where(it, eventId = eventId)
.findFirst()
?.setDecryptionResult(result)

eventEntity?.apply {
val decryptedPayload = threadsAwarenessHandler.handleIfNeededDuringDecryption(
it,
roomId = event.roomId,
event,
result)
setDecryptionResult(result, decryptedPayload)
}
}
} catch (e: MXCryptoError) {
Timber.v("Failed to decrypt event ${event.eventId} : ${e.localizedMessage}")
Expand Down
6 changes: 6 additions & 0 deletions ...android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import org.matrix.android.sdk.internal.session.sync.handler.PresenceSyncHandler
import org.matrix.android.sdk.internal.session.sync.handler.SyncResponsePostTreatmentAggregatorHandler
import org.matrix.android.sdk.internal.session.sync.handler.UserAccountDataSyncHandler
import org.matrix.android.sdk.internal.session.sync.handler.room.RoomSyncHandler
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
import org.matrix.android.sdk.internal.util.awaitTransaction
import org.matrix.android.sdk.internal.worker.WorkerParamsFactory
import timber.log.Timber
Expand All @@ -62,6 +63,7 @@ internal class SyncResponseHandler @Inject constructor(
private val tokenStore: SyncTokenStore,
private val processEventForPushTask: ProcessEventForPushTask,
private val pushRuleService: PushRuleService,
private val threadsAwarenessHandler: ThreadsAwarenessHandler,
private val presenceSyncHandler: PresenceSyncHandler
) {

Expand Down Expand Up @@ -94,6 +96,10 @@ internal class SyncResponseHandler @Inject constructor(
Timber.v("Finish handling toDevice in $it ms")
}
val aggregator = SyncResponsePostTreatmentAggregator()

// Prerequisite for thread events handling in RoomSyncHandler
threadsAwarenessHandler.fetchRootThreadEventsIfNeeded(syncResponse)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to do that even for the initial sync? It can take time if there are lots of events to retrieve

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, without this it will not be rendered appropriately. I think there will be never more than max 1/2 fetches per room worst case. Ofc this will be removed upon the first thread release.


// Start one big transaction
monarchy.awaitTransaction { realm ->
measureTimeMillis {
Expand Down
10 changes: 9 additions & 1 deletion ...rc/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
private val cryptoService: DefaultCryptoService,
private val roomMemberEventHandler: RoomMemberEventHandler,
private val roomTypingUsersHandler: RoomTypingUsersHandler,
private val threadsAwarenessHandler: ThreadsAwarenessHandler,
private val roomChangeMembershipStateDataSource: RoomChangeMembershipStateDataSource,
@UserId private val userId: String,
private val timelineInput: TimelineInput) {
Expand Down Expand Up @@ -362,10 +363,17 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
}
eventIds.add(event.eventId)

if (event.isEncrypted() && insertType != EventInsertType.INITIAL_SYNC) {
val isInitialSync = insertType == EventInsertType.INITIAL_SYNC

if (event.isEncrypted() && !isInitialSync) {
decryptIfNeeded(event, roomId)
}

threadsAwarenessHandler.handleIfNeeded(
realm = realm,
roomId = roomId,
event = event)

val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, insertType)
if (event.stateKey != null) {
Expand Down
Loading