From 2b9ffb4849c422b03bf149b8e6c1c15480810d03 Mon Sep 17 00:00:00 2001 From: davotoula Date: Fri, 3 Jul 2026 22:44:20 +0200 Subject: [PATCH] Code review: - extract shared File.deleteOrWarn helper for cache eviction --- .../service/images/ThumbnailDiskCache.kt | 7 +-- .../loggedIn/home/VoiceReplyViewModel.kt | 5 +-- .../amethyst/commons/util/FileDeletion.kt | 44 +++++++++++++++++++ .../amethyst/napplethost/NappletBlobCache.kt | 16 ++++--- 4 files changed, 57 insertions(+), 15 deletions(-) create mode 100644 commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/FileDeletion.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt index 208150293b..32b1c1fe70 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.images import android.graphics.Bitmap import android.graphics.BitmapFactory import android.graphics.Matrix +import com.vitorpamplona.amethyst.commons.util.deleteOrWarn import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.sha256.sha256 @@ -193,11 +194,7 @@ class ThumbnailDiskCache( files .sortedBy { it.lastModified() } .take(files.size - maxEntries) - .forEach { - if (!it.delete()) { - Log.w("ThumbnailDiskCache") { "Failed to evict thumbnail ${it.absolutePath}" } - } - } + .forEach { it.deleteOrWarn("ThumbnailDiskCache", "thumbnail") } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt index 4113ec5a08..ce3e28411b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt @@ -28,6 +28,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.commons.util.deleteOrWarn import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.service.uploads.CompressorQuality import com.vitorpamplona.amethyst.service.uploads.UploadOrchestrator @@ -156,10 +157,8 @@ class VoiceReplyViewModel : ViewModel() { voiceLocalFile?.let { file -> try { if (file.exists()) { - if (file.delete()) { + if (file.deleteOrWarn("VoiceReplyViewModel", "voice file")) { Log.d("VoiceReplyViewModel") { "Deleted voice file: ${file.absolutePath}" } - } else { - Log.w("VoiceReplyViewModel") { "Failed to delete voice file: ${file.absolutePath}" } } } } catch (e: Exception) { diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/FileDeletion.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/FileDeletion.kt new file mode 100644 index 0000000000..6ca8b99870 --- /dev/null +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/FileDeletion.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.util + +import com.vitorpamplona.quartz.utils.Log +import java.io.File + +/** + * Deletes this file, warning on real failures only. [File.delete] returns false + * both when deletion genuinely fails and when the file is already gone (e.g. + * removed by a concurrent eviction in another thread or process); only the + * former deserves a warning. + * + * @param tag the log tag of the calling component + * @param what a short noun for the log message, e.g. "thumbnail" or "blob" + * @return true when the file no longer exists, whether this call deleted it or + * it was already gone; false when it still exists and could not be deleted. + */ +fun File.deleteOrWarn( + tag: String, + what: String, +): Boolean { + if (delete() || !exists()) return true + Log.w(tag) { "Failed to delete $what $absolutePath" } + return false +} diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBlobCache.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBlobCache.kt index ecd61878c8..baf1109759 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBlobCache.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBlobCache.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.napplethost +import com.vitorpamplona.amethyst.commons.util.deleteOrWarn import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.sha256.sha256 @@ -64,16 +65,17 @@ class NappletBlobCache( /** Best-effort eviction: if the store exceeds [maxBytes], delete oldest blobs until under it. */ fun trimToSize(maxBytes: Long) { runCatching { - val files = dir.listFiles()?.filter { it.isFile && !it.name.contains(".tmp.") } ?: return - var total = files.sumOf { it.length() } + val files = + dir + .listFiles() + ?.filter { it.isFile && !it.name.contains(".tmp.") } + ?.map { it to it.length() } ?: return + var total = files.sumOf { it.second } if (total <= maxBytes) return - files.sortedBy { it.lastModified() }.forEach { f -> + files.sortedBy { it.first.lastModified() }.forEach { (f, length) -> if (total <= maxBytes) return - val length = f.length() - if (f.delete()) { + if (f.deleteOrWarn("NappletBlobCache", "blob")) { total -= length - } else { - Log.w("NappletBlobCache") { "Failed to evict blob ${f.absolutePath}" } } } }