From a9ab32c8e367a107f60931804bbb64d89bb5abc4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 30 Aug 2024 12:53:44 +0200 Subject: [PATCH 1/4] Rework: extract AvatarFactory and MediaRequestDataFactory from CoilMediaFetcher --- .../ui/media/AvatarDataFetcherFactory.kt | 42 +++++++++++++++++++ .../matrix/ui/media/CoilMediaFetcher.kt | 41 ------------------ .../matrix/ui/media/ImageLoaderFactories.kt | 4 +- .../media/MediaRequestDataFetcherFactory.kt | 41 ++++++++++++++++++ 4 files changed, 85 insertions(+), 43 deletions(-) create mode 100644 libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatarDataFetcherFactory.kt create mode 100644 libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaRequestDataFetcherFactory.kt diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatarDataFetcherFactory.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatarDataFetcherFactory.kt new file mode 100644 index 00000000000..e6e71c3465e --- /dev/null +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatarDataFetcherFactory.kt @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2024 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 + * + * https://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.matrix.ui.media + +import android.content.Context +import coil.ImageLoader +import coil.fetch.Fetcher +import coil.request.Options +import io.element.android.libraries.designsystem.components.avatar.AvatarData +import io.element.android.libraries.matrix.api.MatrixClient + +internal class AvatarDataFetcherFactory( + private val context: Context, + private val client: MatrixClient +) : Fetcher.Factory { + override fun create( + data: AvatarData, + options: Options, + imageLoader: ImageLoader + ): Fetcher { + return CoilMediaFetcher( + scalingFunction = { context.resources.displayMetrics.density * it }, + mediaLoader = client.mediaLoader, + mediaData = data.toMediaRequestData(), + options = options + ) + } +} diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt index 455041aabdc..b47f927d124 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt @@ -16,16 +16,12 @@ package io.element.android.libraries.matrix.ui.media -import android.content.Context -import coil.ImageLoader import coil.decode.DataSource import coil.decode.ImageSource import coil.fetch.FetchResult import coil.fetch.Fetcher import coil.fetch.SourceResult import coil.request.Options -import io.element.android.libraries.designsystem.components.avatar.AvatarData -import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.media.MatrixMediaLoader import io.element.android.libraries.matrix.api.media.MediaSource import io.element.android.libraries.matrix.api.media.toFile @@ -102,41 +98,4 @@ internal class CoilMediaFetcher( dataSource = DataSource.MEMORY ) } - - class MediaRequestDataFactory( - private val context: Context, - private val client: MatrixClient - ) : - Fetcher.Factory { - override fun create( - data: MediaRequestData, - options: Options, - imageLoader: ImageLoader - ): Fetcher { - return CoilMediaFetcher( - scalingFunction = { context.resources.displayMetrics.density * it }, - mediaLoader = client.mediaLoader, - mediaData = data, - options = options - ) - } - } - - class AvatarFactory( - private val context: Context, - private val client: MatrixClient - ) : Fetcher.Factory { - override fun create( - data: AvatarData, - options: Options, - imageLoader: ImageLoader - ): Fetcher { - return CoilMediaFetcher( - scalingFunction = { context.resources.displayMetrics.density * it }, - mediaLoader = client.mediaLoader, - mediaData = data.toMediaRequestData(), - options = options - ) - } - } } diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderFactories.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderFactories.kt index 95b23f54a19..a3c1a0642a5 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderFactories.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderFactories.kt @@ -52,8 +52,8 @@ class DefaultLoggedInImageLoaderFactory @Inject constructor( } add(AvatarDataKeyer()) add(MediaRequestDataKeyer()) - add(CoilMediaFetcher.AvatarFactory(context, matrixClient)) - add(CoilMediaFetcher.MediaRequestDataFactory(context, matrixClient)) + add(AvatarDataFetcherFactory(context, matrixClient)) + add(MediaRequestDataFetcherFactory(context, matrixClient)) } .build() } diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaRequestDataFetcherFactory.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaRequestDataFetcherFactory.kt new file mode 100644 index 00000000000..a06a92ee3e0 --- /dev/null +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaRequestDataFetcherFactory.kt @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2024 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 + * + * https://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.matrix.ui.media + +import android.content.Context +import coil.ImageLoader +import coil.fetch.Fetcher +import coil.request.Options +import io.element.android.libraries.matrix.api.MatrixClient + +internal class MediaRequestDataFetcherFactory( + private val context: Context, + private val client: MatrixClient +) : Fetcher.Factory { + override fun create( + data: MediaRequestData, + options: Options, + imageLoader: ImageLoader + ): Fetcher { + return CoilMediaFetcher( + scalingFunction = { context.resources.displayMetrics.density * it }, + mediaLoader = client.mediaLoader, + mediaData = data, + options = options + ) + } +} From cef31d370427e1deb57c975016eaba1b6f959bbc Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 30 Aug 2024 13:06:26 +0200 Subject: [PATCH 2/4] Log more errors. --- .../android/libraries/matrix/ui/media/CoilMediaFetcher.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt index b47f927d124..78df94ef883 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt @@ -38,7 +38,9 @@ internal class CoilMediaFetcher( private val options: Options ) : Fetcher { override suspend fun fetch(): FetchResult? { - if (mediaData?.source == null) return null + if (mediaData?.source == null) return null.also { + Timber.e("MediaData source is null") + } return when (mediaData.kind) { is MediaRequestData.Kind.Content -> fetchContent(mediaData.source, options) is MediaRequestData.Kind.Thumbnail -> fetchThumbnail(mediaData.source, mediaData.kind, options) @@ -72,6 +74,8 @@ internal class CoilMediaFetcher( source = mediaSource, ).map { byteArray -> byteArray.asSourceResult(options) + }.onFailure { + Timber.e(it) }.getOrNull() } @@ -82,6 +86,8 @@ internal class CoilMediaFetcher( height = scalingFunction(kind.height.toFloat()).roundToLong(), ).map { byteArray -> byteArray.asSourceResult(options) + }.onFailure { + Timber.e(it) }.getOrNull() } From 34ba0a5e98ec2054f63a5310550c09ef172dab97 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 30 Aug 2024 13:07:39 +0200 Subject: [PATCH 3/4] CoilMediaFetcher.mediaData cannot be null. --- .../android/libraries/matrix/ui/media/CoilMediaFetcher.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt index 78df94ef883..5f669571b41 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/CoilMediaFetcher.kt @@ -34,12 +34,13 @@ import kotlin.math.roundToLong internal class CoilMediaFetcher( private val scalingFunction: (Float) -> Float, private val mediaLoader: MatrixMediaLoader, - private val mediaData: MediaRequestData?, + private val mediaData: MediaRequestData, private val options: Options ) : Fetcher { override suspend fun fetch(): FetchResult? { - if (mediaData?.source == null) return null.also { + if (mediaData.source == null) { Timber.e("MediaData source is null") + return null } return when (mediaData.kind) { is MediaRequestData.Kind.Content -> fetchContent(mediaData.source, options) From e35ba7af8f7facd5ca2b5b193da148910a3ba823 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 30 Aug 2024 13:12:03 +0200 Subject: [PATCH 4/4] `AvatarData.toMediaRequestData()` can be internal. --- .../android/libraries/matrix/ui/media/AvatatarDataExt.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatatarDataExt.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatatarDataExt.kt index 39912cb443f..0aa9330ff82 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatatarDataExt.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatatarDataExt.kt @@ -20,7 +20,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarData import io.element.android.libraries.matrix.api.media.MediaSource import kotlin.math.roundToLong -fun AvatarData.toMediaRequestData(): MediaRequestData { +internal fun AvatarData.toMediaRequestData(): MediaRequestData { return MediaRequestData( source = url?.let { MediaSource(it) }, kind = MediaRequestData.Kind.Thumbnail(size.dp.value.roundToLong())