From 90eeb68d362cd1be0ef6592f7dd904c3cb67cf7a Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 20 Sep 2019 17:22:04 +0200 Subject: [PATCH] Timeline: fix permalink towards an hidden event --- .../api/session/room/timeline/Timeline.kt | 4 +- .../session/room/timeline/DefaultTimeline.kt | 79 +++++++++++++------ .../room/timeline/TokenChunkEventPersistor.kt | 10 +-- .../home/room/detail/RoomDetailFragment.kt | 1 + .../home/room/detail/RoomDetailViewModel.kt | 23 ++---- .../ScrollOnHighlightedEventCallback.kt | 7 +- .../timeline/TimelineEventController.kt | 5 +- 7 files changed, 81 insertions(+), 48 deletions(-) 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 d0f4bff74b..13eca813c7 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 @@ -44,7 +44,6 @@ interface Timeline { */ fun dispose() - fun restartWithEventId(eventId: String?) @@ -73,8 +72,9 @@ interface Timeline { fun getTimelineEventWithId(eventId: String?): TimelineEvent? + fun getFirstDisplayableEventId(eventId: String): String? - interface Listener { + interface Listener { /** * Call when the timeline has been updated through pagination or sync. * @param snapshot the most uptodate snapshot 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 88c13cc056..b3d83a2286 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 @@ -99,7 +99,8 @@ internal class DefaultTimeline( private val cancelableBag = CancelableBag() private val debouncer = Debouncer(mainHandler) - private lateinit var liveEvents: RealmResults + private lateinit var nonFilteredEvents: RealmResults + private lateinit var filteredEvents: RealmResults private lateinit var eventRelations: RealmResults private var roomEntity: RoomEntity? = null @@ -128,9 +129,9 @@ internal class DefaultTimeline( } changeSet.insertionRanges.forEach { range -> val (startDisplayIndex, direction) = if (range.startIndex == 0) { - Pair(liveEvents[range.length - 1]!!.root!!.displayIndex, Timeline.Direction.FORWARDS) + Pair(filteredEvents[range.length - 1]!!.root!!.displayIndex, Timeline.Direction.FORWARDS) } else { - Pair(liveEvents[range.startIndex]!!.root!!.displayIndex, Timeline.Direction.BACKWARDS) + Pair(filteredEvents[range.startIndex]!!.root!!.displayIndex, Timeline.Direction.BACKWARDS) } val state = getPaginationState(direction) if (state.isPaginating) { @@ -218,9 +219,9 @@ internal class DefaultTimeline( } } - liveEvents = buildEventQuery(realm) + nonFilteredEvents = buildEventQuery(realm).sort(TimelineEventEntityFields.ROOT.DISPLAY_INDEX, Sort.DESCENDING).findAll() + filteredEvents = nonFilteredEvents.where() .filterEventsWithSettings() - .sort(TimelineEventEntityFields.ROOT.DISPLAY_INDEX, Sort.DESCENDING) .findAllAsync() .also { it.addChangeListener(eventsChangeListener) } @@ -229,9 +230,9 @@ internal class DefaultTimeline( .also { it.addChangeListener(relationsListener) } if (settings.buildReadReceipts) { - hiddenReadReceipts.start(realm, liveEvents, this) + hiddenReadReceipts.start(realm, filteredEvents, this) } - hiddenReadMarker.start(realm, liveEvents, this) + hiddenReadMarker.start(realm, filteredEvents, this) isReady.set(true) } } @@ -246,7 +247,7 @@ internal class DefaultTimeline( BACKGROUND_HANDLER.post { roomEntity?.sendingTimelineEvents?.removeAllChangeListeners() eventRelations.removeAllChangeListeners() - liveEvents.removeAllChangeListeners() + filteredEvents.removeAllChangeListeners() hiddenReadMarker.dispose() if (settings.buildReadReceipts) { hiddenReadReceipts.dispose() @@ -267,25 +268,56 @@ internal class DefaultTimeline( postSnapshot() } - override fun getIndexOfEvent(eventId: String?): Int? { - return builtEventsIdMap[eventId] - } - override fun getTimelineEventAtIndex(index: Int): TimelineEvent? { return builtEvents.getOrNull(index) } + override fun getIndexOfEvent(eventId: String?): Int? { + return builtEventsIdMap[eventId] + } + override fun getTimelineEventWithId(eventId: String?): TimelineEvent? { return builtEventsIdMap[eventId]?.let { getTimelineEventAtIndex(it) } } + override fun getFirstDisplayableEventId(eventId: String): String? { + // If the item is built, the id is obviously displayable + val builtIndex = builtEventsIdMap[eventId] + if (builtIndex != null) { + return eventId + } + // Otherwise, we should check if the event is in the db, but is hidden because of filters + return Realm.getInstance(realmConfiguration).use { localRealm -> + val nonFilteredEvents = buildEventQuery(localRealm).sort(TimelineEventEntityFields.ROOT.DISPLAY_INDEX, Sort.DESCENDING).findAll() + val nonFilteredEvent = nonFilteredEvents.where().equalTo(TimelineEventEntityFields.EVENT_ID, eventId).findFirst() + val filteredEvents = nonFilteredEvents.where().filterEventsWithSettings().findAll() + val isEventInDb = nonFilteredEvent != null + + val isHidden = isEventInDb && filteredEvents.where().equalTo(TimelineEventEntityFields.EVENT_ID, eventId).findFirst() == null + if (isHidden) { + val displayIndex = nonFilteredEvent?.root?.displayIndex + if (displayIndex != null) { + // Then we are looking for the first displayable event after the hidden one + val firstDisplayedEvent = filteredEvents.where() + .lessThanOrEqualTo(TimelineEventEntityFields.ROOT.DISPLAY_INDEX, displayIndex) + .findFirst() + firstDisplayedEvent?.eventId + } else { + null + } + } else { + null + } + } + } + override fun hasMoreToLoad(direction: Timeline.Direction): Boolean { return hasMoreInCache(direction) || !hasReachedEnd(direction) } - // TimelineHiddenReadReceipts.Delegate +// TimelineHiddenReadReceipts.Delegate override fun rebuildEvent(eventId: String, readReceipts: List): Boolean { return rebuildEvent(eventId) { te -> @@ -297,7 +329,7 @@ internal class DefaultTimeline( postSnapshot() } - // TimelineHiddenReadMarker.Delegate +// TimelineHiddenReadMarker.Delegate override fun rebuildEvent(eventId: String, hasReadMarker: Boolean): Boolean { return rebuildEvent(eventId) { te -> @@ -309,7 +341,7 @@ internal class DefaultTimeline( postSnapshot() } - // Private methods ***************************************************************************** +// Private methods ***************************************************************************** private fun rebuildEvent(eventId: String, builder: (TimelineEvent) -> TimelineEvent): Boolean { return builtEventsIdMap[eventId]?.let { builtIndex -> @@ -415,22 +447,23 @@ internal class DefaultTimeline( */ private fun handleInitialLoad() { var shouldFetchInitialEvent = false - val initialDisplayIndex = if (initialEventId == null) { - liveEvents.firstOrNull()?.root?.displayIndex + val currentInitialEventId = initialEventId + val initialDisplayIndex = if (currentInitialEventId == null) { + filteredEvents.firstOrNull()?.root?.displayIndex } else { - val initialEvent = liveEvents.where() + val initialEvent = nonFilteredEvents.where() .equalTo(TimelineEventEntityFields.EVENT_ID, initialEventId) .findFirst() + shouldFetchInitialEvent = initialEvent == null initialEvent?.root?.displayIndex } prevDisplayIndex = initialDisplayIndex nextDisplayIndex = initialDisplayIndex - val currentInitialEventId = initialEventId if (currentInitialEventId != null && shouldFetchInitialEvent) { fetchEvent(currentInitialEventId) } else { - val count = min(settings.initialSize, liveEvents.size) + val count = min(settings.initialSize, filteredEvents.size) if (initialEventId == null) { paginateInternal(initialDisplayIndex, Timeline.Direction.BACKWARDS, count, strict = false) } else { @@ -494,7 +527,7 @@ internal class DefaultTimeline( * This has to be called on TimelineThread as it access realm live results */ private fun getLiveChunk(): ChunkEntity? { - return liveEvents.firstOrNull()?.chunk?.firstOrNull() + return filteredEvents.firstOrNull()?.chunk?.firstOrNull() } /** @@ -552,7 +585,7 @@ internal class DefaultTimeline( direction: Timeline.Direction, count: Long, strict: Boolean): RealmResults { - val offsetQuery = liveEvents.where() + val offsetQuery = filteredEvents.where() if (direction == Timeline.Direction.BACKWARDS) { offsetQuery.sort(TimelineEventEntityFields.ROOT.DISPLAY_INDEX, Sort.DESCENDING) if (strict) { @@ -631,7 +664,7 @@ internal class DefaultTimeline( } -// Extension methods *************************************************************************** + // Extension methods *************************************************************************** private fun Timeline.Direction.toPaginationDirection(): PaginationDirection { return if (this == Timeline.Direction.BACKWARDS) PaginationDirection.BACKWARDS else PaginationDirection.FORWARDS 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 af845040ae..0305002959 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 @@ -112,7 +112,7 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy Timber.v("Start persisting ${receivedChunk.events.size} events in $roomId towards $direction") val roomEntity = RoomEntity.where(realm, roomId).findFirst() - ?: realm.createObject(roomId) + ?: realm.createObject(roomId) val nextToken: String? val prevToken: String? @@ -141,7 +141,7 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy } else { nextChunk?.apply { this.prevToken = prevToken } } - ?: ChunkEntity.create(realm, prevToken, nextToken) + ?: ChunkEntity.create(realm, prevToken, nextToken) if (receivedChunk.events.isEmpty() && receivedChunk.end == receivedChunk.start) { Timber.v("Reach end of $roomId") @@ -163,8 +163,8 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy currentChunk = handleMerge(roomEntity, direction, currentChunk, nextChunk) } else { val newEventIds = receivedChunk.events.mapNotNull { it.eventId } - ChunkEntity - .findAllIncludingEvents(realm, newEventIds) + val overlappedChunks = ChunkEntity.findAllIncludingEvents(realm, newEventIds) + overlappedChunks .filter { it != currentChunk } .forEach { overlapped -> currentChunk = handleMerge(roomEntity, direction, currentChunk, overlapped) @@ -194,7 +194,7 @@ internal class TokenChunkEventPersistor @Inject constructor(private val monarchy // We always merge the bottom chunk into top chunk, so we are always merging backwards Timber.v("Merge ${currentChunk.prevToken} | ${currentChunk.nextToken} with ${otherChunk.prevToken} | ${otherChunk.nextToken}") - return if (direction == PaginationDirection.BACKWARDS) { + return if (direction == PaginationDirection.BACKWARDS && !otherChunk.isLastForward) { currentChunk.merge(roomEntity.roomId, otherChunk, PaginationDirection.BACKWARDS) roomEntity.deleteOnCascade(otherChunk) currentChunk diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt index 58ab211eb4..f6d50046a9 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt @@ -702,6 +702,7 @@ class RoomDetailFragment : val summary = state.asyncRoomSummary() val inviter = state.asyncInviter() if (summary?.membership == Membership.JOIN) { + scrollOnHighlightedEventCallback.timeline = state.timeline timelineEventController.update(state) inviteView.visibility = View.GONE val uid = session.myUserId diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt index 401411dfde..72a09fbad6 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt @@ -43,6 +43,7 @@ import im.vector.matrix.android.api.session.room.model.Membership import im.vector.matrix.android.api.session.room.model.message.MessageContent import im.vector.matrix.android.api.session.room.model.message.MessageType import im.vector.matrix.android.api.session.room.model.message.getFileUrl +import im.vector.matrix.android.api.session.room.model.relation.ReactionContent import im.vector.matrix.android.api.session.room.model.tombstone.RoomTombstoneContent import im.vector.matrix.android.api.session.room.send.UserDraft import im.vector.matrix.android.api.session.room.timeline.TimelineEvent @@ -613,18 +614,18 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro } - private fun handleNavigateToEvent(action: RoomDetailActions.NavigateToEvent) { - val targetEventId = action.eventId - val indexOfEvent = timeline.getIndexOfEvent(targetEventId) + val targetEventId: String = action.eventId + val correctedEventId = timeline.getFirstDisplayableEventId(targetEventId) ?: targetEventId + val indexOfEvent = timeline.getIndexOfEvent(correctedEventId) if (indexOfEvent == null) { // Event is not already in RAM timeline.restartWithEventId(targetEventId) } if (action.highlight) { - setState { copy(highlightedEventId = targetEventId) } + setState { copy(highlightedEventId = correctedEventId) } } - _navigateToEvent.postLiveEvent(targetEventId) + _navigateToEvent.postLiveEvent(correctedEventId) } private fun handleResendEvent(action: RoomDetailActions.ResendMessage) { @@ -683,17 +684,7 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro } private fun handleSetReadMarkerAction(action: RoomDetailActions.SetReadMarkerAction) = withState { state -> - var readMarkerId = action.eventId - if (readMarkerId == state.asyncRoomSummary()?.readMarkerId) { - val indexOfEvent = timeline.getIndexOfEvent(action.eventId) - // force to set the read marker on the next event - if (indexOfEvent != null) { - timeline.getTimelineEventAtIndex(indexOfEvent - 1)?.root?.eventId?.also { eventIdOfNext -> - readMarkerId = eventIdOfNext - } - } - } - room.setReadMarker(readMarkerId, callback = object : MatrixCallback {}) + room.setReadMarker(action.eventId, callback = object : MatrixCallback {}) } private fun handleMarkAllAsRead() { diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/ScrollOnHighlightedEventCallback.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/ScrollOnHighlightedEventCallback.kt index c272e611a0..62d80408d2 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/ScrollOnHighlightedEventCallback.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/ScrollOnHighlightedEventCallback.kt @@ -17,6 +17,7 @@ package im.vector.riotx.features.home.room.detail import androidx.recyclerview.widget.LinearLayoutManager +import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.riotx.core.platform.DefaultListUpdateCallback import im.vector.riotx.features.home.room.detail.timeline.TimelineEventController import timber.log.Timber @@ -27,9 +28,13 @@ class ScrollOnHighlightedEventCallback(private val layoutManager: LinearLayoutMa private val scheduledEventId = AtomicReference() + var timeline: Timeline? = null + override fun onChanged(position: Int, count: Int, tag: Any?) { val eventId = scheduledEventId.get() ?: return - val positionToScroll = timelineEventController.searchPositionOfEvent(eventId) + val nonNullTimeline = timeline ?: return + val correctedEventId = nonNullTimeline.getFirstDisplayableEventId(eventId) + val positionToScroll = timelineEventController.searchPositionOfEvent(correctedEventId) if (positionToScroll != null) { val firstVisibleItem = layoutManager.findFirstCompletelyVisibleItemPosition() val lastVisibleItem = layoutManager.findLastCompletelyVisibleItemPosition() 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 3aeac06f12..701e412b1d 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 @@ -296,8 +296,11 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec } } - fun searchPositionOfEvent(eventId: String): Int? = synchronized(modelCache) { + fun searchPositionOfEvent(eventId: String?): Int? = synchronized(modelCache) { // Search in the cache + if (eventId == null) { + return null + } var realPosition = 0 if (showingForwardLoader) { realPosition++