From 62a367520e2c068dd5f84b9452b0e72c15c0db84 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 1 Aug 2023 10:53:41 +0200 Subject: [PATCH] RoomList: use same logic than Timeline for caching built items. (#1013) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * RoomList: use same logic than Timeline for caching built items. Extract into reusable components. * RoomList: fix tests * Fix `DiffCacheUpdater` docs --------- Co-authored-by: ganfra Co-authored-by: Jorge Martín --- features/messages/impl/build.gradle.kts | 1 - .../impl/timeline/diff/CacheInvalidator.kt | 56 ----------- .../diff/TimelineItemsCacheInvalidator.kt | 63 ++++++++++++ .../factories/TimelineItemsFactory.kt | 47 ++++----- .../impl/datasource/RoomListDataSource.kt | 96 +++++++++++++------ .../roomlist/impl/RoomListPresenterTests.kt | 26 +++-- libraries/androidutils/build.gradle.kts | 1 + .../androidutils/diff/DefaultDiffCallback.kt | 21 ++-- .../libraries/androidutils/diff/DiffCache.kt | 67 +++++++++++++ .../androidutils/diff/DiffCacheInvalidator.kt | 63 ++++++++++++ .../androidutils/diff/DiffCacheUpdater.kt | 70 ++++++++++++++ 11 files changed, 371 insertions(+), 140 deletions(-) delete mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt create mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/TimelineItemsCacheInvalidator.kt rename features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/MatrixTimelineItemsDiffCallback.kt => libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DefaultDiffCallback.kt (70%) create mode 100644 libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCache.kt create mode 100644 libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheInvalidator.kt create mode 100644 libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheUpdater.kt diff --git a/features/messages/impl/build.gradle.kts b/features/messages/impl/build.gradle.kts index b5edaec78e..948bac4c57 100644 --- a/features/messages/impl/build.gradle.kts +++ b/features/messages/impl/build.gradle.kts @@ -52,7 +52,6 @@ dependencies { implementation(libs.coil.compose) implementation(libs.datetime) implementation(libs.accompanist.flowlayout) - implementation(libs.androidx.recyclerview) implementation(libs.jsoup) implementation(libs.androidx.constraintlayout) implementation(libs.androidx.constraintlayout.compose) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt deleted file mode 100644 index 9aa3ab5e02..0000000000 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (c) 2023 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 io.element.android.features.messages.impl.timeline.diff - -import androidx.recyclerview.widget.ListUpdateCallback -import io.element.android.features.messages.impl.timeline.model.TimelineItem -import io.element.android.features.messages.impl.timeline.util.invalidateLast -import timber.log.Timber - -internal class CacheInvalidator(private val itemStatesCache: MutableList) : - ListUpdateCallback { - - override fun onChanged(position: Int, count: Int, payload: Any?) { - Timber.d("onChanged(position= $position, count= $count)") - (position until position + count).forEach { - // Invalidate cache - itemStatesCache[it] = null - } - } - - override fun onMoved(fromPosition: Int, toPosition: Int) { - Timber.d("onMoved(fromPosition= $fromPosition, toPosition= $toPosition)") - val model = itemStatesCache.removeAt(fromPosition) - itemStatesCache.add(toPosition, model) - } - - override fun onInserted(position: Int, count: Int) { - Timber.d("onInserted(position= $position, count= $count)") - itemStatesCache.invalidateLast() - repeat(count) { - itemStatesCache.add(position, null) - } - } - - override fun onRemoved(position: Int, count: Int) { - Timber.d("onRemoved(position= $position, count= $count)") - itemStatesCache.invalidateLast() - repeat(count) { - itemStatesCache.removeAt(position) - } - } -} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/TimelineItemsCacheInvalidator.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/TimelineItemsCacheInvalidator.kt new file mode 100644 index 0000000000..a7a3bea00e --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/TimelineItemsCacheInvalidator.kt @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2023 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 io.element.android.features.messages.impl.timeline.diff + +import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.libraries.androidutils.diff.DefaultDiffCacheInvalidator +import io.element.android.libraries.androidutils.diff.DiffCacheInvalidator +import io.element.android.libraries.androidutils.diff.MutableDiffCache + +/** + * [DiffCacheInvalidator] implementation for [TimelineItem]. + * It uses [DefaultDiffCacheInvalidator] and invalidate the cache around the updated item so that those items are computed again. + * This is needed because a timeline item is computed based on the previous and next items. + */ +internal class TimelineItemsCacheInvalidator : DiffCacheInvalidator { + + private val delegate = DefaultDiffCacheInvalidator() + + override fun onChanged(position: Int, count: Int, cache: MutableDiffCache) { + delegate.onChanged(position, count, cache) + } + + override fun onMoved(fromPosition: Int, toPosition: Int, cache: MutableDiffCache) { + delegate.onMoved(fromPosition, toPosition, cache) + } + + override fun onInserted(position: Int, count: Int, cache: MutableDiffCache) { + cache.invalidateAround(position) + delegate.onInserted(position, count, cache) + } + + override fun onRemoved(position: Int, count: Int, cache: MutableDiffCache) { + cache.invalidateAround(position) + delegate.onRemoved(position, count, cache) + } +} + +/** + * Invalidate the cache around the given position. + * It invalidates the previous and next items. + */ +private fun MutableDiffCache<*>.invalidateAround(position: Int) { + if (position > 0) { + set(position - 1, null) + } + if (position < indices().last) { + set(position + 1, null) + } +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt index aa9786c945..d664d3f26c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt @@ -19,13 +19,13 @@ package io.element.android.features.messages.impl.timeline.factories import androidx.compose.runtime.Composable import androidx.compose.runtime.State import androidx.compose.runtime.collectAsState -import androidx.recyclerview.widget.DiffUtil -import io.element.android.features.messages.impl.timeline.diff.CacheInvalidator -import io.element.android.features.messages.impl.timeline.diff.MatrixTimelineItemsDiffCallback +import io.element.android.features.messages.impl.timeline.diff.TimelineItemsCacheInvalidator import io.element.android.features.messages.impl.timeline.factories.event.TimelineItemEventFactory import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.libraries.androidutils.diff.DiffCacheUpdater +import io.element.android.libraries.androidutils.diff.MutableListDiffCache import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import kotlinx.collections.immutable.ImmutableList @@ -35,9 +35,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext -import timber.log.Timber import javax.inject.Inject -import kotlin.system.measureTimeMillis class TimelineItemsFactory @Inject constructor( private val dispatchers: CoroutineDispatchers, @@ -46,13 +44,20 @@ class TimelineItemsFactory @Inject constructor( private val timelineItemGrouper: TimelineItemGrouper, ) { private val timelineItems = MutableStateFlow(persistentListOf()) - private val timelineItemsCache = arrayListOf() - - // Items from rust sdk, used for diffing - private var matrixTimelineItems: List = emptyList() private val lock = Mutex() - private val cacheInvalidator = CacheInvalidator(timelineItemsCache) + private val diffCache = MutableListDiffCache() + private val diffCacheUpdater = DiffCacheUpdater( + diffCache = diffCache, + detectMoves = false, + cacheInvalidator = TimelineItemsCacheInvalidator() + ) { old, new -> + if (old is MatrixTimelineItem.Event && new is MatrixTimelineItem.Event) { + old.uniqueId == new.uniqueId + } else { + false + } + } @Composable fun collectItemsAsState(): State> { @@ -63,15 +68,15 @@ class TimelineItemsFactory @Inject constructor( timelineItems: List, ) = withContext(dispatchers.computation) { lock.withLock { - calculateAndApplyDiff(timelineItems) + diffCacheUpdater.updateWith(timelineItems) buildAndEmitTimelineItemStates(timelineItems) } } private suspend fun buildAndEmitTimelineItemStates(timelineItems: List) { val newTimelineItemStates = ArrayList() - for (index in timelineItemsCache.indices.reversed()) { - val cacheItem = timelineItemsCache[index] + for (index in diffCache.indices().reversed()) { + val cacheItem = diffCache.get(index) if (cacheItem == null) { buildAndCacheItem(timelineItems, index)?.also { timelineItemState -> newTimelineItemStates.add(timelineItemState) @@ -84,20 +89,6 @@ class TimelineItemsFactory @Inject constructor( this.timelineItems.emit(result) } - private fun calculateAndApplyDiff(newTimelineItems: List) { - val timeToDiff = measureTimeMillis { - val diffCallback = - MatrixTimelineItemsDiffCallback( - oldList = matrixTimelineItems, - newList = newTimelineItems - ) - val diffResult = DiffUtil.calculateDiff(diffCallback, false) - matrixTimelineItems = newTimelineItems - diffResult.dispatchUpdatesTo(cacheInvalidator) - } - Timber.v("Time to apply diff on new list of ${newTimelineItems.size} items: $timeToDiff ms") - } - private fun buildAndCacheItem( timelineItems: List, index: Int @@ -108,7 +99,7 @@ class TimelineItemsFactory @Inject constructor( is MatrixTimelineItem.Virtual -> virtualItemFactory.create(currentTimelineItem) MatrixTimelineItem.Other -> null } - timelineItemsCache[index] = timelineItemState + diffCache[index] = timelineItemState return timelineItemState } } diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt index 4eb7bc9d72..714f6e2e11 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt @@ -18,6 +18,8 @@ package io.element.android.features.roomlist.impl.datasource import io.element.android.features.roomlist.impl.model.RoomListRoomSummary import io.element.android.features.roomlist.impl.model.RoomListRoomSummaryPlaceholders +import io.element.android.libraries.androidutils.diff.DiffCacheUpdater +import io.element.android.libraries.androidutils.diff.MutableListDiffCache import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.core.extensions.orEmpty import io.element.android.libraries.dateformatter.api.LastMessageTimestampFormatter @@ -36,6 +38,8 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import javax.inject.Inject @@ -50,15 +54,17 @@ class RoomListDataSource @Inject constructor( private val _allRooms = MutableStateFlow>(persistentListOf()) private val _filteredRooms = MutableStateFlow>(persistentListOf()) + private val lock = Mutex() + private val diffCache = MutableListDiffCache() + private val diffCacheUpdater = DiffCacheUpdater(diffCache = diffCache, detectMoves = true) { old, new -> + old?.identifier() == new?.identifier() + } + fun launchIn(coroutineScope: CoroutineScope) { roomSummaryDataSource .allRooms() .onEach { roomSummaries -> - _allRooms.value = if (roomSummaries.isEmpty()) { - RoomListRoomSummaryPlaceholders.createFakeList(16) - } else { - mapRoomSummaries(roomSummaries) - }.toImmutableList() + replaceWith(roomSummaries) } .launchIn(coroutineScope) @@ -85,33 +91,63 @@ class RoomListDataSource @Inject constructor( val allRooms: StateFlow> = _allRooms val filteredRooms: StateFlow> = _filteredRooms - private suspend fun mapRoomSummaries( - roomSummaries: List - ): List = withContext(coroutineDispatchers.computation) { - roomSummaries.map { roomSummary -> - when (roomSummary) { - is RoomSummary.Empty -> RoomListRoomSummaryPlaceholders.create(roomSummary.identifier) - is RoomSummary.Filled -> { - val avatarData = AvatarData( - id = roomSummary.identifier(), - name = roomSummary.details.name, - url = roomSummary.details.avatarURLString, - size = AvatarSize.RoomListItem, - ) - val roomIdentifier = roomSummary.identifier() - RoomListRoomSummary( - id = roomSummary.identifier(), - roomId = RoomId(roomIdentifier), - name = roomSummary.details.name, - hasUnread = roomSummary.details.unreadNotificationCount > 0, - timestamp = lastMessageTimestampFormatter.format(roomSummary.details.lastMessageTimestamp), - lastMessage = roomSummary.details.lastMessage?.let { message -> - roomLastMessageFormatter.format(message.event, roomSummary.details.isDirect) - }.orEmpty(), - avatarData = avatarData, - ) + private suspend fun replaceWith(roomSummaries: List) = withContext(coroutineDispatchers.computation) { + lock.withLock { + diffCacheUpdater.updateWith(roomSummaries) + buildAndEmitAllRooms(roomSummaries) + } + } + + private suspend fun buildAndEmitAllRooms(roomSummaries: List) { + if (diffCache.isEmpty()) { + _allRooms.emit( + RoomListRoomSummaryPlaceholders.createFakeList(16).toImmutableList() + ) + } else { + val roomListRoomSummaries = ArrayList() + for (index in diffCache.indices()) { + val cacheItem = diffCache.get(index) + if (cacheItem == null) { + buildAndCacheItem(roomSummaries, index)?.also { timelineItemState -> + roomListRoomSummaries.add(timelineItemState) + } + } else { + roomListRoomSummaries.add(cacheItem) } } + _allRooms.emit(roomListRoomSummaries.toImmutableList()) + } + } + + private fun buildAndCacheItem( + roomSummaries: List, + index: Int + ): RoomListRoomSummary? { + val roomListRoomSummary = when (val roomSummary = roomSummaries.getOrNull(index)) { + is RoomSummary.Empty -> RoomListRoomSummaryPlaceholders.create(roomSummary.identifier) + is RoomSummary.Filled -> { + val avatarData = AvatarData( + id = roomSummary.identifier(), + name = roomSummary.details.name, + url = roomSummary.details.avatarURLString, + size = AvatarSize.RoomListItem, + ) + val roomIdentifier = roomSummary.identifier() + RoomListRoomSummary( + id = roomSummary.identifier(), + roomId = RoomId(roomIdentifier), + name = roomSummary.details.name, + hasUnread = roomSummary.details.unreadNotificationCount > 0, + timestamp = lastMessageTimestampFormatter.format(roomSummary.details.lastMessageTimestamp), + lastMessage = roomSummary.details.lastMessage?.let { message -> + roomLastMessageFormatter.format(message.event, roomSummary.details.isDirect) + }.orEmpty(), + avatarData = avatarData, + ) + } + null -> null } + diffCache[index] = roomListRoomSummary + return roomListRoomSummary } } diff --git a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt index ce8e83c70d..d2d52aecc8 100644 --- a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt +++ b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt @@ -50,6 +50,7 @@ import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.FakeRoomSummaryDataSource import io.element.android.libraries.matrix.test.room.aRoomSummaryFilled import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService +import io.element.android.tests.testutils.consumeItemsUntilPredicate import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.TestScope @@ -118,13 +119,12 @@ class RoomListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - skipItems(1) - val initialState = awaitItem() + val initialState = consumeItemsUntilPredicate { state -> state.roomList.size == 16 }.last() // Room list is loaded with 16 placeholders Truth.assertThat(initialState.roomList.size).isEqualTo(16) Truth.assertThat(initialState.roomList.all { it.isPlaceholder }).isTrue() roomSummaryDataSource.postAllRooms(listOf(aRoomSummaryFilled())) - val withRoomState = awaitItem() + val withRoomState = consumeItemsUntilPredicate { state -> state.roomList.size == 1 }.last() Truth.assertThat(withRoomState.roomList.size).isEqualTo(1) Truth.assertThat(withRoomState.roomList.first()) .isEqualTo(aRoomListRoomSummary) @@ -142,21 +142,19 @@ class RoomListPresenterTests { presenter.present() }.test { roomSummaryDataSource.postAllRooms(listOf(aRoomSummaryFilled())) - skipItems(1) - val loadedState = awaitItem() + val loadedState = consumeItemsUntilPredicate { state -> state.roomList.size == 1 }.last() // Test filtering with result loadedState.eventSink.invoke(RoomListEvents.UpdateFilter(A_ROOM_NAME.substring(0, 3))) - skipItems(1) // Filter update - val withNotFilteredRoomState = awaitItem() - Truth.assertThat(withNotFilteredRoomState.filter).isEqualTo(A_ROOM_NAME.substring(0, 3)) - Truth.assertThat(withNotFilteredRoomState.filteredRoomList.size).isEqualTo(1) - Truth.assertThat(withNotFilteredRoomState.filteredRoomList.first()) + val withFilteredRoomState = consumeItemsUntilPredicate { state -> state.filteredRoomList.size == 1 }.last() + Truth.assertThat(withFilteredRoomState.filter).isEqualTo(A_ROOM_NAME.substring(0, 3)) + Truth.assertThat(withFilteredRoomState.filteredRoomList.size).isEqualTo(1) + Truth.assertThat(withFilteredRoomState.filteredRoomList.first()) .isEqualTo(aRoomListRoomSummary) // Test filtering without result - withNotFilteredRoomState.eventSink.invoke(RoomListEvents.UpdateFilter("tada")) - skipItems(1) // Filter update - Truth.assertThat(awaitItem().filter).isEqualTo("tada") - Truth.assertThat(awaitItem().filteredRoomList).isEmpty() + withFilteredRoomState.eventSink.invoke(RoomListEvents.UpdateFilter("tada")) + val withNotFilteredRoomState = consumeItemsUntilPredicate { state -> state.filteredRoomList.size == 0 }.last() + Truth.assertThat(withNotFilteredRoomState.filter).isEqualTo("tada") + Truth.assertThat(withNotFilteredRoomState.filteredRoomList).isEmpty() } } diff --git a/libraries/androidutils/build.gradle.kts b/libraries/androidutils/build.gradle.kts index 92e3c46126..57e4ca3569 100644 --- a/libraries/androidutils/build.gradle.kts +++ b/libraries/androidutils/build.gradle.kts @@ -37,6 +37,7 @@ dependencies { implementation(libs.timber) implementation(libs.androidx.corektx) implementation(libs.androidx.activity.activity) + implementation(libs.androidx.recyclerview) implementation(libs.androidx.exifinterface) implementation(libs.androidx.security.crypto) implementation(libs.androidx.browser) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/MatrixTimelineItemsDiffCallback.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DefaultDiffCallback.kt similarity index 70% rename from features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/MatrixTimelineItemsDiffCallback.kt rename to libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DefaultDiffCallback.kt index 4a78447bd7..219441d5e6 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/MatrixTimelineItemsDiffCallback.kt +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DefaultDiffCallback.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 New Vector Ltd + * Copyright (c) 2023 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. @@ -14,14 +14,17 @@ * limitations under the License. */ -package io.element.android.features.messages.impl.timeline.diff +package io.element.android.libraries.androidutils.diff import androidx.recyclerview.widget.DiffUtil -import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem -internal class MatrixTimelineItemsDiffCallback( - private val oldList: List, - private val newList: List +/** + * Default implementation of [DiffUtil.Callback] that uses [areItemsTheSame] to compare items. + */ +internal class DefaultDiffCallback( + private val oldList: List, + private val newList: List, + private val areItemsTheSame: (oldItem: T?, newItem: T?) -> Boolean, ) : DiffUtil.Callback() { override fun getOldListSize(): Int { @@ -35,11 +38,7 @@ internal class MatrixTimelineItemsDiffCallback( override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { val oldItem = oldList.getOrNull(oldItemPosition) val newItem = newList.getOrNull(newItemPosition) - return if (oldItem is MatrixTimelineItem.Event && newItem is MatrixTimelineItem.Event) { - oldItem.uniqueId == newItem.uniqueId - } else { - false - } + return areItemsTheSame(oldItem, newItem) } override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCache.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCache.kt new file mode 100644 index 0000000000..3d1161e2e0 --- /dev/null +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCache.kt @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2023 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 io.element.android.libraries.androidutils.diff + +/** + * A cache that can be used to store some data that can be invalidated when a diff is applied. + * The cache is invalidated by the [DiffCacheInvalidator]. + */ +interface DiffCache { + fun get(index: Int): E? + fun indices(): IntRange + fun isEmpty(): Boolean +} + +/** + * A [DiffCache] that can be mutated by adding, removing or updating elements. + */ +interface MutableDiffCache : DiffCache { + fun removeAt(index: Int): E? + fun add(index: Int, element: E?) + operator fun set(index: Int, element: E?) +} + +/** + * A [MutableDiffCache] backed by a [MutableList]. + * + */ +class MutableListDiffCache(private val mutableList: MutableList = ArrayList()) : MutableDiffCache { + + override fun removeAt(index: Int): E? { + return mutableList.removeAt(index) + } + + override fun get(index: Int): E? { + return mutableList.getOrNull(index) + } + + override fun indices(): IntRange { + return mutableList.indices + } + + override fun isEmpty(): Boolean { + return mutableList.isEmpty() + } + + override operator fun set(index: Int, element: E?) { + mutableList[index] = element + } + + override fun add(index: Int, element: E?) { + mutableList.add(index, element) + } +} diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheInvalidator.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheInvalidator.kt new file mode 100644 index 0000000000..d9f378c8fa --- /dev/null +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheInvalidator.kt @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2023 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 io.element.android.libraries.androidutils.diff + +/** + * [DiffCacheInvalidator] is used to invalidate the cache when the list is updated. + * It is used by [DiffCacheUpdater]. + * Check the default implementation [DefaultDiffCacheInvalidator]. + */ +interface DiffCacheInvalidator { + fun onChanged(position: Int, count: Int, cache: MutableDiffCache) + + fun onMoved(fromPosition: Int, toPosition: Int, cache: MutableDiffCache) + + fun onInserted(position: Int, count: Int, cache: MutableDiffCache) + + fun onRemoved(position: Int, count: Int, cache: MutableDiffCache) +} + +/** + * Default implementation of [DiffCacheInvalidator]. + * It invalidates the cache by setting values to null. + */ +class DefaultDiffCacheInvalidator : DiffCacheInvalidator { + + override fun onChanged(position: Int, count: Int, cache: MutableDiffCache) { + (position until position + count).forEach { + // Invalidate cache + cache[it] = null + } + } + + override fun onMoved(fromPosition: Int, toPosition: Int, cache: MutableDiffCache) { + val model = cache.removeAt(fromPosition) + cache.add(toPosition, model) + } + + override fun onInserted(position: Int, count: Int, cache: MutableDiffCache) { + repeat(count) { + cache.add(position, null) + } + } + + override fun onRemoved(position: Int, count: Int, cache: MutableDiffCache) { + repeat(count) { + cache.removeAt(position) + } + } +} diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheUpdater.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheUpdater.kt new file mode 100644 index 0000000000..500edcb135 --- /dev/null +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/diff/DiffCacheUpdater.kt @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2023 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 io.element.android.libraries.androidutils.diff + +import androidx.recyclerview.widget.DiffUtil +import androidx.recyclerview.widget.ListUpdateCallback +import timber.log.Timber +import kotlin.system.measureTimeMillis + +/** + * Class in charge of updating a [MutableDiffCache] according to the cache invalidation rules provided by the [DiffCacheInvalidator]. + * @param ListItem the type of the items in the list + * @param CachedItem the type of the items in the cache + * @param diffCache the cache to update + * @param detectMoves true if DiffUtil should try to detect moved items, false otherwise + * @param cacheInvalidator the invalidator to use to update the cache + * @param areItemsTheSame the function to use to compare items + */ +class DiffCacheUpdater( + private val diffCache: MutableDiffCache, + private val detectMoves: Boolean = false, + private val cacheInvalidator: DiffCacheInvalidator = DefaultDiffCacheInvalidator(), + private val areItemsTheSame: (oldItem: ListItem?, newItem: ListItem?) -> Boolean, +) { + + private val lock = Object() + private var prevOriginalList: List = emptyList() + + private val listUpdateCallback = object : ListUpdateCallback { + override fun onInserted(position: Int, count: Int) { + cacheInvalidator.onInserted(position, count, diffCache) + } + + override fun onRemoved(position: Int, count: Int) { + cacheInvalidator.onRemoved(position, count, diffCache) + } + + override fun onMoved(fromPosition: Int, toPosition: Int) { + cacheInvalidator.onMoved(fromPosition, toPosition, diffCache) + } + + override fun onChanged(position: Int, count: Int, payload: Any?) { + cacheInvalidator.onChanged(position, count, diffCache) + } + } + + fun updateWith(newOriginalList: List) = synchronized(lock) { + val timeToDiff = measureTimeMillis { + val diffCallback = DefaultDiffCallback(prevOriginalList, newOriginalList, areItemsTheSame) + val diffResult = DiffUtil.calculateDiff(diffCallback, detectMoves) + prevOriginalList = newOriginalList + diffResult.dispatchUpdatesTo(listUpdateCallback) + } + Timber.v("Time to apply diff on new list of ${newOriginalList.size} items: $timeToDiff ms") + } +}