From 0e7f675c727b62d4d170482cbe544df2119a1e92 Mon Sep 17 00:00:00 2001 From: Jannis Mattheis Date: Fri, 14 Jun 2024 14:44:21 +0200 Subject: [PATCH] fix: image loading when using markdown img and bigImageUrl When receiving an image with the same image in the markdown body and in the extras client::notification.bigImageUrl, then there is a clash on the file system. One request succeeds and the other fails with the following error. This commit ensures that there is only one cail image loader instance, so that there shouldn't be file system race conditions. WebSocket(1): received message {"id":845,"appid":21,...} Failed - http://192.168.178.2:8000/1.jpg?v=1718369188 - java.lang.IllegalStateException: closed java.lang.IllegalStateException: closed at okio.RealBufferedSource.rangeEquals(RealBufferedSource.kt:466) at okio.RealBufferedSource.rangeEquals(RealBufferedSource.kt:130) at coil.decode.SvgDecodeUtils.isSvg(DecodeUtils.kt:19) at coil.decode.SvgDecoder$Factory.isApplicable(SvgDecoder.kt:104) at coil.decode.SvgDecoder$Factory.create(SvgDecoder.kt:99) at coil.ComponentRegistry.newDecoder(ComponentRegistry.kt:100) at coil.intercept.EngineInterceptor.decode(EngineInterceptor.kt:197) at coil.intercept.EngineInterceptor.access$decode(EngineInterceptor.kt:42) at coil.intercept.EngineInterceptor$execute$executeResult$1.invokeSuspend(EngineInterceptor.kt:131) at coil.intercept.EngineInterceptor$execute$executeResult$1.invoke(EngineInterceptor.kt) at coil.intercept.EngineInterceptor$execute$executeResult$1.invoke(EngineInterceptor.kt) at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:78) at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:167) at kotlinx.coroutines.BuildersKt.withContext(Unknown Source) at coil.intercept.EngineInterceptor.execute(EngineInterceptor.kt:130) at coil.intercept.EngineInterceptor.access$execute(EngineInterceptor.kt:42) at coil.intercept.EngineInterceptor$execute$1.invokeSuspend(EngineInterceptor.kt) at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108) at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115) at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103) at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584) at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793) at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697) at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684) Successful (NETWORK) - http://192.168.178.2:8000/1.jpg?v=1718369188 --- .../{CoilHandler.kt => CoilInstance.kt} | 70 ++++++++++++------- .../kotlin/com/github/gotify/SSLSettings.kt | 2 +- .../gotify/messages/MessagesActivity.kt | 11 ++- .../github/gotify/messages/MessagesModel.kt | 2 - .../github/gotify/service/WebSocketService.kt | 10 ++- 5 files changed, 53 insertions(+), 42 deletions(-) rename app/src/main/kotlin/com/github/gotify/{CoilHandler.kt => CoilInstance.kt} (57%) diff --git a/app/src/main/kotlin/com/github/gotify/CoilHandler.kt b/app/src/main/kotlin/com/github/gotify/CoilInstance.kt similarity index 57% rename from app/src/main/kotlin/com/github/gotify/CoilHandler.kt rename to app/src/main/kotlin/com/github/gotify/CoilInstance.kt index 94781115..611e6fbd 100644 --- a/app/src/main/kotlin/com/github/gotify/CoilHandler.kt +++ b/app/src/main/kotlin/com/github/gotify/CoilInstance.kt @@ -16,40 +16,26 @@ import java.io.IOException import okhttp3.OkHttpClient import org.tinylog.kotlin.Logger -internal class CoilHandler(private val context: Context, private val settings: Settings) { - private val imageLoader = makeImageLoader() - - private fun makeImageLoader(): ImageLoader { - val builder = OkHttpClient.Builder() - CertUtils.applySslSettings(builder, settings.sslSettings()) - return ImageLoader.Builder(context) - .okHttpClient(builder.build()) - .diskCache { - DiskCache.Builder() - .directory(context.cacheDir.resolve("coil-cache")) - .build() - } - .components { - add(SvgDecoder.Factory()) - } - .build() - } +object CoilInstance { + private var holder: Pair? = null @Throws(IOException::class) - fun getImageFromUrl(url: String?): Bitmap { + fun getImageFromUrl(context: Context, url: String?): Bitmap { val request = ImageRequest.Builder(context) .data(url) .build() - return (imageLoader.executeBlocking(request).drawable as BitmapDrawable).bitmap + return (get(context).executeBlocking(request).drawable as BitmapDrawable).bitmap } - fun getIcon(app: Application?): Bitmap { + fun getIcon(context: Context, app: Application?): Bitmap { if (app == null) { return BitmapFactory.decodeResource(context.resources, R.drawable.gotify) } + val baseUrl = Settings(context).url try { return getImageFromUrl( - Utils.resolveAbsoluteUrl("${settings.url}/", app.image) + context, + Utils.resolveAbsoluteUrl("$baseUrl/", app.image) ) } catch (e: IOException) { Logger.error(e, "Could not load image for notification") @@ -57,15 +43,45 @@ internal class CoilHandler(private val context: Context, private val settings: S return BitmapFactory.decodeResource(context.resources, R.drawable.gotify) } - fun get() = imageLoader - @OptIn(ExperimentalCoilApi::class) - fun evict() { + fun evict(context: Context) { try { - imageLoader.diskCache?.clear() - imageLoader.memoryCache?.clear() + get(context).apply { + diskCache?.clear() + memoryCache?.clear() + } } catch (e: IOException) { Logger.error(e, "Problem evicting Coil cache") } } + + @Synchronized + fun get(context: Context): ImageLoader { + val newSettings = Settings(context).sslSettings() + val copy = holder + if (copy != null && copy.first == newSettings) { + return copy.second + } + return makeImageLoader(context, newSettings).also { holder = it }.second + } + + private fun makeImageLoader( + context: Context, + sslSettings: SSLSettings + ): Pair { + val builder = OkHttpClient.Builder() + CertUtils.applySslSettings(builder, sslSettings) + val loader = ImageLoader.Builder(context) + .okHttpClient(builder.build()) + .diskCache { + DiskCache.Builder() + .directory(context.cacheDir.resolve("coil-cache")) + .build() + } + .components { + add(SvgDecoder.Factory()) + } + .build() + return sslSettings to loader + } } diff --git a/app/src/main/kotlin/com/github/gotify/SSLSettings.kt b/app/src/main/kotlin/com/github/gotify/SSLSettings.kt index cc88a607..eb900b66 100644 --- a/app/src/main/kotlin/com/github/gotify/SSLSettings.kt +++ b/app/src/main/kotlin/com/github/gotify/SSLSettings.kt @@ -1,6 +1,6 @@ package com.github.gotify -internal class SSLSettings( +internal data class SSLSettings( val validateSSL: Boolean, val caCertPath: String?, val clientCertPath: String?, diff --git a/app/src/main/kotlin/com/github/gotify/messages/MessagesActivity.kt b/app/src/main/kotlin/com/github/gotify/messages/MessagesActivity.kt index 514acf22..421adb36 100644 --- a/app/src/main/kotlin/com/github/gotify/messages/MessagesActivity.kt +++ b/app/src/main/kotlin/com/github/gotify/messages/MessagesActivity.kt @@ -31,6 +31,7 @@ import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView import coil.request.ImageRequest import com.github.gotify.BuildConfig +import com.github.gotify.CoilInstance import com.github.gotify.MissedMessageUtil import com.github.gotify.R import com.github.gotify.Utils @@ -102,7 +103,7 @@ internal class MessagesActivity : listMessageAdapter = ListMessageAdapter( this, viewModel.settings, - viewModel.coilHandler.get() + CoilInstance.get(this) ) { message -> scheduleDeletion(message) } @@ -169,13 +170,13 @@ internal class MessagesActivity : } private fun refreshAll() { - viewModel.coilHandler.evict() + CoilInstance.evict(this) startActivity(Intent(this, InitializationActivity::class.java)) finish() } private fun onRefresh() { - viewModel.coilHandler.evict() + CoilInstance.evict(this) viewModel.messages.clear() launchCoroutine { loadMore(viewModel.appId).forEachIndexed { index, message -> @@ -211,9 +212,7 @@ internal class MessagesActivity : .size(100, 100) .target(t) .build() - viewModel.coilHandler - .get() - .enqueue(request) + CoilInstance.get(this).enqueue(request) } selectAppInMenu(selectedItem) } diff --git a/app/src/main/kotlin/com/github/gotify/messages/MessagesModel.kt b/app/src/main/kotlin/com/github/gotify/messages/MessagesModel.kt index a7383069..e4ae8e4c 100644 --- a/app/src/main/kotlin/com/github/gotify/messages/MessagesModel.kt +++ b/app/src/main/kotlin/com/github/gotify/messages/MessagesModel.kt @@ -3,7 +3,6 @@ package com.github.gotify.messages import android.app.Activity import androidx.lifecycle.ViewModel import coil.target.Target -import com.github.gotify.CoilHandler import com.github.gotify.Settings import com.github.gotify.api.ClientFactory import com.github.gotify.client.api.MessageApi @@ -13,7 +12,6 @@ import com.github.gotify.messages.provider.MessageState internal class MessagesModel(parentView: Activity) : ViewModel() { val settings = Settings(parentView) - val coilHandler = CoilHandler(parentView, settings) val client = ClientFactory.clientToken(settings) val appsHolder = ApplicationHolder(parentView, client) val messages = MessageFacade(client.createService(MessageApi::class.java), appsHolder) diff --git a/app/src/main/kotlin/com/github/gotify/service/WebSocketService.kt b/app/src/main/kotlin/com/github/gotify/service/WebSocketService.kt index 402aa808..7ad0078c 100644 --- a/app/src/main/kotlin/com/github/gotify/service/WebSocketService.kt +++ b/app/src/main/kotlin/com/github/gotify/service/WebSocketService.kt @@ -17,7 +17,7 @@ import androidx.annotation.RequiresApi import androidx.core.app.NotificationCompat import androidx.core.content.ContextCompat import com.github.gotify.BuildConfig -import com.github.gotify.CoilHandler +import com.github.gotify.CoilInstance import com.github.gotify.MarkwonFactory import com.github.gotify.MissedMessageUtil import com.github.gotify.NotificationSupport @@ -62,7 +62,6 @@ internal class WebSocketService : Service() { private val lastReceivedMessage = AtomicLong(NOT_LOADED) private lateinit var missingMessageUtil: MissedMessageUtil - private lateinit var coilHandler: CoilHandler private lateinit var markwon: Markwon override fun onCreate() { @@ -71,8 +70,7 @@ internal class WebSocketService : Service() { val client = ClientFactory.clientToken(settings) missingMessageUtil = MissedMessageUtil(client.createService(MessageApi::class.java)) Logger.info("Create ${javaClass.simpleName}") - coilHandler = CoilHandler(this, settings) - markwon = MarkwonFactory.createForNotification(this, coilHandler.get()) + markwon = MarkwonFactory.createForNotification(this, CoilInstance.get(this)) } override fun onDestroy() { @@ -377,7 +375,7 @@ internal class WebSocketService : Service() { .setDefaults(Notification.DEFAULT_ALL) .setWhen(System.currentTimeMillis()) .setSmallIcon(R.drawable.ic_gotify) - .setLargeIcon(coilHandler.getIcon(appIdToApp[appId])) + .setLargeIcon(CoilInstance.getIcon(this, appIdToApp[appId])) .setTicker("${getString(R.string.app_name)} - $title") .setGroup(NotificationSupport.Group.MESSAGES) .setContentTitle(title) @@ -406,7 +404,7 @@ internal class WebSocketService : Service() { try { b.setStyle( NotificationCompat.BigPictureStyle() - .bigPicture(coilHandler.getImageFromUrl(notificationImageUrl)) + .bigPicture(CoilInstance.getImageFromUrl(this, notificationImageUrl)) ) } catch (e: Exception) { Logger.error(e, "Error loading bigImageUrl")