🔀 Commit

No more disappearing avatars when storage runs low
Commite925bd8e57ea2b4dfb401747196633a28525b5f5
AuthorJabJab <noreply@xmpp.tel>
Date2026-08-10
Parent359ddbaa
commit e925bd8e57ea2b4dfb401747196633a28525b5f5
Author: JabJab <noreply@xmpp.tel>
Date:   Mon Aug 10 12:06:48 2026 +0300

    No more disappearing avatars when storage runs low
---
 build.gradle                                       |   4 +-
 .../tel/xmpp/jabjab/persistance/FileBackend.java   |  30 ++----
 .../tel/xmpp/jabjab/services/AvatarService.java    | 115 ++++++++++++++++++++-
 .../jabjab/services/XmppConnectionService.java     |   1 -
 4 files changed, 120 insertions(+), 30 deletions(-)

diff --git a/build.gradle b/build.gradle
index 8d10109..5974681 100644
--- a/build.gradle
+++ b/build.gradle
@@ -113,8 +113,8 @@ android {
 
     defaultConfig {
         minSdkVersion 23
-        versionCode 42310
-        versionName "1.0.6"
+        versionCode 42311
+        versionName "1.0.7"
         applicationId "tel.xmpp.jabjab"
         resValue "string", "applicationId", applicationId
         def appName = "JabJab"
diff --git a/src/main/java/tel/xmpp/jabjab/persistance/FileBackend.java b/src/main/java/tel/xmpp/jabjab/persistance/FileBackend.java
index 62cb60a..b98ca8f 100644
--- a/src/main/java/tel/xmpp/jabjab/persistance/FileBackend.java
+++ b/src/main/java/tel/xmpp/jabjab/persistance/FileBackend.java
@@ -1163,34 +1163,18 @@ public class FileBackend {
         return rendered;
     }
 
-    public void deleteHistoricAvatarPath() {
-        delete(getHistoricAvatarPath());
-    }
-
-    private void delete(final File file) {
-        if (file.isDirectory()) {
-            final File[] files = file.listFiles();
-            if (files != null) {
-                for (final File f : files) {
-                    delete(f);
-                }
-            }
-        }
-        if (file.delete()) {
-            Log.d(Config.LOGTAG, "deleted " + file.getAbsolutePath());
-        }
-    }
-
-    private File getHistoricAvatarPath() {
-        return new File(mXmppConnectionService.getFilesDir(), "/avatars/");
-    }
-
     public File getAvatarFile(final String avatar) {
         return getAvatarFile(mXmppConnectionService, avatar);
     }
 
     public static File getAvatarFile(Context context, final String avatar) {
-        return new File(context.getCacheDir(), "/avatars/" + avatar);
+        // getFilesDir(), not getCacheDir() — same reasoning as LinkPreviewFetcher's
+        // thumbCacheFile() and RecordingActivity's staging dir: Android can wipe the cache
+        // dir at any time under storage pressure with no warning. Nothing re-fetches a
+        // missing avatar file automatically, so an OS-triggered eviction here silently and
+        // permanently breaks that avatar until some unrelated event (a presence/PEP push)
+        // happens to re-trigger a fetch.
+        return new File(context.getFilesDir(), "/avatars/" + avatar);
     }
 
     public Uri getAvatarUri(String avatar) {
diff --git a/src/main/java/tel/xmpp/jabjab/services/AvatarService.java b/src/main/java/tel/xmpp/jabjab/services/AvatarService.java
index c4c5f52..5dd0a72 100644
--- a/src/main/java/tel/xmpp/jabjab/services/AvatarService.java
+++ b/src/main/java/tel/xmpp/jabjab/services/AvatarService.java
@@ -13,6 +13,7 @@ import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
 import android.net.Uri;
 import android.util.DisplayMetrics;
+import android.util.Log;
 import androidx.annotation.ColorInt;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -26,6 +27,10 @@ import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
+import com.google.common.util.concurrent.FutureCallback;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.MoreExecutors;
+import tel.xmpp.jabjab.Config;
 import tel.xmpp.jabjab.R;
 import tel.xmpp.jabjab.entities.Account;
 import tel.xmpp.jabjab.entities.Contact;
@@ -39,11 +44,14 @@ import tel.xmpp.jabjab.entities.Room;
 import tel.xmpp.jabjab.persistance.FileBackend;
 import tel.xmpp.jabjab.utils.UIHelper;
 import tel.xmpp.jabjab.xmpp.Jid;
+import tel.xmpp.jabjab.xmpp.XmppConnection;
+import tel.xmpp.jabjab.xmpp.manager.AvatarManager;
 import tel.xmpp.jabjab.xmpp.manager.MultiUserChatManager;
 import im.conversations.android.model.Bookmark;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class AvatarService {
 
@@ -69,6 +77,12 @@ public class AvatarService {
             CacheBuilder.newBuilder().maximumSize(256).build();
     private final Set<Integer> sizes = new HashSet<>();
     private final Multimap<String, String> conversationDependentKeys = ArrayListMultimap.create();
+    // getByHashOrFallback() dedup: a known hash whose local file has gone missing (e.g. wiped
+    // by the OS while briefly stored under getCacheDir(), or any other disk loss) would
+    // otherwise show the letter placeholder forever — the client only normally re-fetches on
+    // a hash *change*, never on a merely-absent file. One re-fetch attempt per (jid, hash)
+    // per process lifetime is enough to self-heal without risking a retry storm.
+    private final Set<String> missingAvatarRefetchAttempted = ConcurrentHashMap.newKeySet();
 
     protected final XmppConnectionService mXmppConnectionService;
 
@@ -173,7 +187,22 @@ public class AvatarService {
                 return avatar;
             }
         }
-        final var avatar = getByHashOrFallback(contact, contact.getAvatar(), surface, size);
+        final var avatar =
+                getByHashOrFallback(
+                        contact,
+                        contact.getAvatar(),
+                        surface,
+                        size,
+                        () ->
+                                triggerMissingAvatarRefetch(
+                                        contact.getAddress(),
+                                        contact.getAvatar(),
+                                        contact.getAccount().getXmppConnection(),
+                                        () -> {
+                                            clear(contact);
+                                            mXmppConnectionService.updateConversationUi();
+                                            mXmppConnectionService.updateRosterUi();
+                                        }));
         this.cache.put(KEY, avatar);
         return avatar;
     }
@@ -289,7 +318,22 @@ public class AvatarService {
         if (cached != null || cachedOnly) {
             return cached;
         }
-        final var avatar = getByHashOrFallback(user, user.getAvatar(), Surface.REGULAR, size);
+        final var avatar =
+                getByHashOrFallback(
+                        user,
+                        user.getAvatar(),
+                        Surface.REGULAR,
+                        size,
+                        () ->
+                                triggerMissingAvatarRefetch(
+                                        user.getFullJid(),
+                                        user.getAvatar(),
+                                        user.getAccount().getXmppConnection(),
+                                        () -> {
+                                            clear(user);
+                                            mXmppConnectionService.updateConversationUi();
+                                            mXmppConnectionService.updateMucRosterUi();
+                                        }));
         this.cache.put(KEY, avatar);
         return avatar;
     }
@@ -298,16 +342,64 @@ public class AvatarService {
             final Avatar avatar,
             @Nullable final String hash,
             final Surface surface,
-            final int size) {
+            final int size,
+            @Nullable final Runnable onMissingFile) {
         if (hash != null) {
             final var byHash = mXmppConnectionService.getFileBackend().getAvatar(hash, size);
             if (byHash != null) {
                 return modifyForSurface(byHash, surface);
             }
+            if (onMissingFile != null) {
+                onMissingFile.run();
+            }
         }
         return getImpl(getFirstLetter(avatar), avatar.getAvatarBackgroundColor(), surface, size);
     }
 
+    /**
+     * Fires (at most once per (jid, hash) per process lifetime) a real re-fetch of an avatar
+     * whose hash we already know but whose local file is missing. Runs {@code onRefetched} —
+     * expected to clear this avatar's own cache entries and refresh the relevant UI — once the
+     * fetch actually lands, since {@link AvatarManager}'s own post-fetch update path is gated
+     * on the hash having *changed*, which it hasn't here.
+     */
+    private void triggerMissingAvatarRefetch(
+            final Jid jid,
+            final String hash,
+            @Nullable final XmppConnection connection,
+            final Runnable onRefetched) {
+        if (connection == null) {
+            return;
+        }
+        if (!missingAvatarRefetchAttempted.add(jid + "\0" + hash)) {
+            return;
+        }
+        Log.d(
+                Config.LOGTAG,
+                "AVATAR_LOAD missing local file for known hash, re-fetching " + jid);
+        final var future = connection.getManager(AvatarManager.class).fetchAndStore(jid);
+        Futures.addCallback(
+                future,
+                new FutureCallback<Void>() {
+                    @Override
+                    public void onSuccess(final Void result) {
+                        Log.d(
+                                Config.LOGTAG,
+                                "AVATAR_LOAD re-fetch succeeded for " + jid);
+                        onRefetched.run();
+                    }
+
+                    @Override
+                    public void onFailure(@NonNull final Throwable t) {
+                        Log.d(
+                                Config.LOGTAG,
+                                "AVATAR_LOAD re-fetch failed for " + jid,
+                                t);
+                    }
+                },
+                MoreExecutors.directExecutor());
+    }
+
     public void clear(final Contact contact) {
         synchronized (this.sizes) {
             for (final Integer size : sizes) {
@@ -542,7 +634,22 @@ public class AvatarService {
         if (cached != null || cachedOnly) {
             return cached;
         }
-        final var avatar = getByHashOrFallback(account, account.getAvatar(), surface, size);
+        final var avatar =
+                getByHashOrFallback(
+                        account,
+                        account.getAvatar(),
+                        surface,
+                        size,
+                        () ->
+                                triggerMissingAvatarRefetch(
+                                        account.getJid().asBareJid(),
+                                        account.getAvatar(),
+                                        account.getXmppConnection(),
+                                        () -> {
+                                            clear(account);
+                                            mXmppConnectionService.updateConversationUi();
+                                            mXmppConnectionService.updateAccountUi();
+                                        }));
         this.cache.put(KEY, avatar);
         return avatar;
     }
diff --git a/src/main/java/tel/xmpp/jabjab/services/XmppConnectionService.java b/src/main/java/tel/xmpp/jabjab/services/XmppConnectionService.java
index 3ef9de7..bab5e45 100644
--- a/src/main/java/tel/xmpp/jabjab/services/XmppConnectionService.java
+++ b/src/main/java/tel/xmpp/jabjab/services/XmppConnectionService.java
@@ -1275,7 +1275,6 @@ public class XmppConnectionService extends Service {
                         == PackageManager.PERMISSION_GRANTED) {
             startContactObserver();
         }
-        FILE_OBSERVER_EXECUTOR.execute(fileBackend::deleteHistoricAvatarPath);
         if (Compatibility.hasStoragePermission(this)) {
             Log.d(Config.LOGTAG, "starting file observer");
             FILE_OBSERVER_EXECUTOR.execute(this.fileObserver::restartWatching);

Built with passion for open-source software, XMPP, privacy, and security —
the belief that people deserve communication tools that serve them, not surveil them.

Dedicated to every brave man and woman who stands up against tyranny and oppression across the world.
And in memory of those who were silenced before they could speak freely.