Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: image loading when using markdown img and bigImageUrl #353

Merged
merged 1 commit into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,56 +16,72 @@ import java.io.IOException
import okhttp3.OkHttpClient
import org.tinylog.kotlin.Logger

internal class CoilHandler(private val context: Context, private val settings: Settings) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyb3rko Would you mind reviewing this PR, it's related to #337.

Copy link
Contributor

@cyb3rko cyb3rko Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
But the edge cases were not tested because I do not have that kind of setup for testing it atm, I trust you there :)

I've added a smaller comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyb3rko Thanks for the review, what smaller comment do you mean?

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<SSLSettings, ImageLoader>? = 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")
}
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<SSLSettings, ImageLoader> {
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
}
}
2 changes: 1 addition & 1 deletion app/src/main/kotlin/com/github/gotify/SSLSettings.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.github.gotify

internal class SSLSettings(
internal data class SSLSettings(
val validateSSL: Boolean,
val caCertPath: String?,
val clientCertPath: String?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -102,7 +103,7 @@ internal class MessagesActivity :
listMessageAdapter = ListMessageAdapter(
this,
viewModel.settings,
viewModel.coilHandler.get()
CoilInstance.get(this)
) { message ->
scheduleDeletion(message)
}
Expand Down Expand Up @@ -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 ->
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
Loading