diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt index 722e14a790..f5418c76c9 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt @@ -17,6 +17,7 @@ import io.element.android.libraries.matrix.api.room.BaseRoom import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.matrix.api.timeline.item.event.toEventOrTransactionId import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -27,10 +28,11 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.onEach +import timber.log.Timber import java.util.concurrent.atomic.AtomicBoolean interface MediaGalleryDataSource { - fun start() + fun start(coroutineScope: CoroutineScope) fun groupedMediaItemsFlow(): Flow> fun getLastData(): AsyncData suspend fun loadMore(direction: Timeline.PaginationDirection) @@ -58,7 +60,7 @@ class TimelineMediaGalleryDataSource( private val isStarted = AtomicBoolean(false) @OptIn(ExperimentalCoroutinesApi::class) - override fun start() { + override fun start(coroutineScope: CoroutineScope) { if (!isStarted.compareAndSet(false, true)) { return } @@ -96,9 +98,12 @@ class TimelineMediaGalleryDataSource( groupedMediaItemsFlow.emit(AsyncData.Success(groupedMediaItems)) } .onCompletion { - timeline?.close() + timeline?.let { + Timber.d("Timeline media gallery data source flow completed for room ${room.roomId}, closing timeline") + it.close() + } } - .launchIn(room.roomCoroutineScope) + .launchIn(coroutineScope) } override suspend fun loadMore(direction: Timeline.PaginationDirection) { diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt index ac9b365099..e7caa12f2e 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt @@ -78,7 +78,7 @@ class MediaGalleryPresenter( .collectAsState(AsyncData.Uninitialized) LaunchedEffect(Unit) { - mediaGalleryDataSource.start() + mediaGalleryDataSource.start(this) } val permissions by room.permissionsAsState(MediaPermissions.DEFAULT) { perms -> diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt index a9fb5d645c..24e48531f0 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt @@ -35,6 +35,7 @@ import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.withContext @@ -62,11 +63,12 @@ class MediaViewerDataSource( private val localMediaStates: MutableMap>> = mutableMapOf() - fun setup() { - galleryDataSource.start() + fun setup(coroutineScope: CoroutineScope) { + galleryDataSource.start(coroutineScope) } fun dispose() { + Timber.d("Disposing MediaViewerDataSource, closing ${mediaFiles.size} media files") mediaFiles.forEach { it.close() } mediaFiles.clear() localMediaStates.clear() diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt index b7631a7039..60f03bb1e0 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt @@ -88,7 +88,7 @@ class MediaViewerPresenter( var mediaBottomSheetState by remember { mutableStateOf(MediaBottomSheetState.Hidden) } DisposableEffect(Unit) { - dataSource.setup() + dataSource.setup(coroutineScope) onDispose { dataSource.dispose() } diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSource.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSource.kt index f243ac4fd7..7bbf397171 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSource.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSource.kt @@ -20,12 +20,13 @@ import io.element.android.libraries.mediaviewer.impl.datasource.MediaGalleryData import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems import io.element.android.libraries.mediaviewer.impl.model.MediaItem import kotlinx.collections.immutable.persistentListOf +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.flowOf class SingleMediaGalleryDataSource( private val data: GroupedMediaItems, ) : MediaGalleryDataSource { - override fun start() = Unit + override fun start(coroutineScope: CoroutineScope) = Unit override fun groupedMediaItemsFlow() = flowOf(AsyncData.Success(data)) override fun getLastData(): AsyncData = AsyncData.Success(data) diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/FakeMediaGalleryDataSource.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/FakeMediaGalleryDataSource.kt index c612bba1bc..4a64f02a33 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/FakeMediaGalleryDataSource.kt +++ b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/FakeMediaGalleryDataSource.kt @@ -13,6 +13,7 @@ import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems import io.element.android.tests.testutils.lambda.lambdaError +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -21,7 +22,7 @@ class FakeMediaGalleryDataSource( private val loadMoreLambda: (Timeline.PaginationDirection) -> Unit = { lambdaError() }, private val deleteItemLambda: (EventId) -> Unit = { lambdaError() }, ) : MediaGalleryDataSource { - override fun start() = startLambda() + override fun start(coroutineScope: CoroutineScope) = startLambda() private val groupedMediaItemsFlow = MutableSharedFlow>( replay = 1 diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/TimelineMediaGalleryDataSourceTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/TimelineMediaGalleryDataSourceTest.kt index bb8419dde5..528fc1da70 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/TimelineMediaGalleryDataSourceTest.kt +++ b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/TimelineMediaGalleryDataSourceTest.kt @@ -80,7 +80,7 @@ class TimelineMediaGalleryDataSourceTest { roomCoroutineScope = backgroundScope, ) ) - sut.start() + sut.start(backgroundScope) assertThat(sut.getLastData()).isEqualTo(AsyncData.Uninitialized) sut.groupedMediaItemsFlow().test { assertThat(awaitItem().isLoading()).isTrue() @@ -95,7 +95,7 @@ class TimelineMediaGalleryDataSourceTest { ) assertThat(sut.getLastData().isSuccess()).isTrue() // Also test that starting again should have no effect - sut.start() + sut.start(backgroundScope) } } // Ensure that the timeline has been closed on flow completion @@ -117,7 +117,7 @@ class TimelineMediaGalleryDataSourceTest { roomCoroutineScope = backgroundScope, ) ) - sut.start() + sut.start(backgroundScope) sut.groupedMediaItemsFlow().test { skipItems(2) sut.loadMore(Timeline.PaginationDirection.BACKWARDS) @@ -140,7 +140,7 @@ class TimelineMediaGalleryDataSourceTest { roomCoroutineScope = backgroundScope, ) ) - sut.start() + sut.start(backgroundScope) sut.groupedMediaItemsFlow().test { skipItems(2) sut.deleteItem(AN_EVENT_ID) @@ -159,7 +159,7 @@ class TimelineMediaGalleryDataSourceTest { roomCoroutineScope = backgroundScope, ) ) - sut.start() + sut.start(backgroundScope) sut.groupedMediaItemsFlow().test { assertThat(awaitItem().isLoading()).isTrue() assertThat(sut.getLastData().isLoading()).isTrue() @@ -181,7 +181,7 @@ class TimelineMediaGalleryDataSourceTest { roomCoroutineScope = backgroundScope, ) ) - sut.start() + sut.start(backgroundScope) sut.groupedMediaItemsFlow().test { assertThat(awaitItem().isLoading()).isTrue() assertThat(sut.getLastData().isLoading()).isTrue() diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSourceTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSourceTest.kt index 44eed2733f..c3f1ab9a0d 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSourceTest.kt +++ b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSourceTest.kt @@ -50,7 +50,7 @@ class MediaViewerDataSourceTest { val sut = createMediaViewerDataSource( galleryDataSource = galleryDataSource, ) - sut.setup() + sut.setup(backgroundScope) startLambda.assertions().isCalledOnce() } diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSourceTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSourceTest.kt index c6460cb70a..8c0a7c05d0 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSourceTest.kt +++ b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSourceTest.kt @@ -37,9 +37,9 @@ class SingleMediaGalleryDataSourceTest { val warmUpRule = WarmUpRule() @Test - fun `function start is no op`() { + fun `function start is no op`() = runTest { val sut = SingleMediaGalleryDataSource(aGroupedMediaItems()) - sut.start() + sut.start(backgroundScope) } @Test