reduce avatar cache invalidation

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Contact.java               | 11 
src/main/java/eu/siacs/conversations/entities/MucOptions.java            | 19 
src/main/java/eu/siacs/conversations/parser/MessageParser.java           | 22 
src/main/java/eu/siacs/conversations/parser/PresenceParser.java          | 54 
src/main/java/eu/siacs/conversations/services/AvatarService.java         | 54 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 28 
src/quicksy/java/eu/siacs/conversations/ui/EnterNameActivity.java        |  2 
7 files changed, 106 insertions(+), 84 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Contact.java 🔗

@@ -448,21 +448,22 @@ public class Contact implements ListItem, Blockable {
         return getJid().getDomain().toString();
     }
 
-    public void setAvatar(Avatar avatar) {
-        setAvatar(avatar, false);
+    public boolean setAvatar(final Avatar avatar) {
+        return setAvatar(avatar, false);
     }
 
-    public void setAvatar(Avatar avatar, boolean previouslyOmittedPepFetch) {
+    public boolean setAvatar(final Avatar avatar, final boolean previouslyOmittedPepFetch) {
         if (this.avatar != null && this.avatar.equals(avatar)) {
-            return;
+            return false;
         }
         if (!previouslyOmittedPepFetch
                 && this.avatar != null
                 && this.avatar.origin == Avatar.Origin.PEP
                 && avatar.origin == Avatar.Origin.VCARD) {
-            return;
+            return false;
         }
         this.avatar = avatar;
+        return true;
     }
 
     public String getAvatarFilename() {

src/main/java/eu/siacs/conversations/entities/MucOptions.java 🔗

@@ -60,7 +60,7 @@ public class MucOptions {
         return this.conversation.getAccount();
     }
 
-    public boolean setSelf(User user) {
+    public boolean setSelf(final User user) {
         this.self = user;
         final boolean roleChanged = this.conversation.setAttribute("role", user.role.toString());
         final boolean affiliationChanged =
@@ -68,8 +68,8 @@ public class MucOptions {
         return roleChanged || affiliationChanged;
     }
 
-    public void changeAffiliation(Jid jid, Affiliation affiliation) {
-        User user = findUserByRealJid(jid);
+    public void changeAffiliation(final Jid jid, final Affiliation affiliation) {
+        final User user = findUserByRealJid(jid);
         synchronized (users) {
             if (user != null && user.getRole() == Role.NONE) {
                 users.remove(user);
@@ -440,14 +440,15 @@ public class MucOptions {
         }
     }
 
-    public List<User> getUsers(int max) {
-        ArrayList<User> subset = new ArrayList<>();
-        HashSet<Jid> jids = new HashSet<>();
-        jids.add(account.getJid().asBareJid());
+    public List<User> getUsers(final int max) {
+        final ArrayList<User> subset = new ArrayList<>();
+        final HashSet<Jid> addresses = new HashSet<>();
+        addresses.add(account.getJid().asBareJid());
         synchronized (users) {
             for (User user : users) {
                 if (user.getRealJid() == null
-                        || (user.getRealJid().getLocal() != null && jids.add(user.getRealJid()))) {
+                        || (user.getRealJid().getLocal() != null
+                                && addresses.add(user.getRealJid()))) {
                     subset.add(user);
                 }
                 if (subset.size() >= max) {
@@ -877,7 +878,7 @@ public class MucOptions {
             }
         }
 
-        public boolean setAvatar(Avatar avatar) {
+        public boolean setAvatar(final Avatar avatar) {
             if (this.avatar != null && this.avatar.equals(avatar)) {
                 return false;
             } else {

src/main/java/eu/siacs/conversations/parser/MessageParser.java 🔗

@@ -266,11 +266,12 @@ public class MessageParser extends AbstractParser
                         mXmppConnectionService.updateAccountUi();
                     } else {
                         final Contact contact = account.getRoster().getContact(from);
-                        contact.setAvatar(avatar);
-                        mXmppConnectionService.syncRoster(account);
-                        mXmppConnectionService.getAvatarService().clear(contact);
-                        mXmppConnectionService.updateConversationUi();
-                        mXmppConnectionService.updateRosterUi();
+                        if (contact.setAvatar(avatar)) {
+                            mXmppConnectionService.syncRoster(account);
+                            mXmppConnectionService.getAvatarService().clear(contact);
+                            mXmppConnectionService.updateConversationUi();
+                            mXmppConnectionService.updateRosterUi();
+                        }
                     }
                 } else if (mXmppConnectionService.isDataSaverDisabled()) {
                     mXmppConnectionService.fetchAvatar(account, avatar);
@@ -1174,7 +1175,7 @@ public class MessageParser extends AbstractParser
                             // ignored
                         }
                     } else if ("item".equals(child.getName())) {
-                        MucOptions.User user = AbstractParser.parseItem(conversation, child);
+                        final var user = AbstractParser.parseItem(conversation, child);
                         Log.d(
                                 Config.LOGTAG,
                                 account.getJid()
@@ -1185,8 +1186,13 @@ public class MessageParser extends AbstractParser
                                         + " in "
                                         + conversation.getJid().asBareJid());
                         if (!user.realJidMatchesAccount()) {
-                            boolean isNew = conversation.getMucOptions().updateUser(user);
-                            mXmppConnectionService.getAvatarService().clear(conversation);
+                            final var mucOptions = conversation.getMucOptions();
+                            final boolean isNew = mucOptions.updateUser(user);
+                            final var avatarService = mXmppConnectionService.getAvatarService();
+                            if (Strings.isNullOrEmpty(mucOptions.getAvatar())) {
+                                avatarService.clear(mucOptions);
+                            }
+                            avatarService.clear(user);
                             mXmppConnectionService.updateMucRosterUi();
                             mXmppConnectionService.updateConversationUi();
                             Contact contact = user.getContact();

src/main/java/eu/siacs/conversations/parser/PresenceParser.java 🔗

@@ -1,6 +1,7 @@
 package eu.siacs.conversations.parser;
 
 import android.util.Log;
+import com.google.common.base.Strings;
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.crypto.PgpEngine;
 import eu.siacs.conversations.crypto.axolotl.AxolotlService;
@@ -37,22 +38,24 @@ public class PresenceParser extends AbstractParser
                 packet.getFrom() == null
                         ? null
                         : mXmppConnectionService.find(account, packet.getFrom().asBareJid());
-        if (conversation != null) {
-            final MucOptions mucOptions = conversation.getMucOptions();
-            boolean before = mucOptions.online();
-            int count = mucOptions.getUserCount();
-            final List<MucOptions.User> tileUserBefore = mucOptions.getUsers(5);
-            processConferencePresence(packet, conversation);
-            final List<MucOptions.User> tileUserAfter = mucOptions.getUsers(5);
-            if (!tileUserAfter.equals(tileUserBefore)) {
-                mXmppConnectionService.getAvatarService().clear(mucOptions);
-            }
-            if (before != mucOptions.online()
-                    || (mucOptions.online() && count != mucOptions.getUserCount())) {
-                mXmppConnectionService.updateConversationUi();
-            } else if (mucOptions.online()) {
-                mXmppConnectionService.updateMucRosterUi();
-            }
+        if (conversation == null) {
+            return;
+        }
+        final MucOptions mucOptions = conversation.getMucOptions();
+        boolean before = mucOptions.online();
+        int count = mucOptions.getUserCount();
+        final List<MucOptions.User> tileUserBefore = mucOptions.getUsers(5);
+        processConferencePresence(packet, conversation);
+        final List<MucOptions.User> tileUserAfter = mucOptions.getUsers(5);
+        if (Strings.isNullOrEmpty(mucOptions.getAvatar())
+                && !tileUserAfter.equals(tileUserBefore)) {
+            mXmppConnectionService.getAvatarService().clear(mucOptions);
+        }
+        if (before != mucOptions.online()
+                || (mucOptions.online() && count != mucOptions.getUserCount())) {
+            mXmppConnectionService.updateConversationUi();
+        } else if (mucOptions.online()) {
+            mXmppConnectionService.updateMucRosterUi();
         }
     }
 
@@ -152,9 +155,11 @@ public class PresenceParser extends AbstractParser
                                                     .getAccount()
                                                     .getRoster()
                                                     .getContact(user.getRealJid());
-                                    c.setAvatar(avatar);
-                                    mXmppConnectionService.syncRoster(conversation.getAccount());
-                                    mXmppConnectionService.getAvatarService().clear(c);
+                                    if (c.setAvatar(avatar)) {
+                                        mXmppConnectionService.syncRoster(
+                                                conversation.getAccount());
+                                        mXmppConnectionService.getAvatarService().clear(c);
+                                    }
                                     mXmppConnectionService.updateRosterUi();
                                 }
                             } else if (mXmppConnectionService.isDataSaverDisabled()) {
@@ -329,11 +334,12 @@ public class PresenceParser extends AbstractParser
                         mXmppConnectionService.updateConversationUi();
                         mXmppConnectionService.updateAccountUi();
                     } else {
-                        contact.setAvatar(avatar);
-                        mXmppConnectionService.syncRoster(account);
-                        mXmppConnectionService.getAvatarService().clear(contact);
-                        mXmppConnectionService.updateConversationUi();
-                        mXmppConnectionService.updateRosterUi();
+                        if (contact.setAvatar(avatar)) {
+                            mXmppConnectionService.syncRoster(account);
+                            mXmppConnectionService.getAvatarService().clear(contact);
+                            mXmppConnectionService.updateConversationUi();
+                            mXmppConnectionService.updateRosterUi();
+                        }
                     }
                 } else if (mXmppConnectionService.isDataSaverDisabled()) {
                     mXmppConnectionService.fetchAvatar(account, avatar);

src/main/java/eu/siacs/conversations/services/AvatarService.java 🔗

@@ -15,10 +15,10 @@ import android.net.Uri;
 import android.text.TextUtils;
 import android.util.DisplayMetrics;
 import android.util.Log;
-import android.util.LruCache;
 import androidx.annotation.ColorInt;
 import androidx.annotation.Nullable;
 import androidx.core.content.res.ResourcesCompat;
+import com.google.common.base.Strings;
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.R;
 import eu.siacs.conversations.entities.Account;
@@ -57,6 +57,7 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded {
     private static final String CHANNEL_SYMBOL = "#";
 
     private final Set<Integer> sizes = new HashSet<>();
+    // TODO refactor to multimap
     private final HashMap<String, Set<String>> conversationDependentKeys = new HashMap<>();
 
     protected XmppConnectionService mXmppConnectionService = null;
@@ -261,23 +262,27 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded {
         return avatar;
     }
 
-    public void clear(Contact contact) {
+    public void clear(final Contact contact) {
         synchronized (this.sizes) {
             for (final Integer size : sizes) {
                 this.mXmppConnectionService.getBitmapCache().remove(key(contact, size));
             }
         }
-        for (Conversation conversation : mXmppConnectionService.findAllConferencesWith(contact)) {
-            MucOptions.User user =
-                    conversation.getMucOptions().findUserByRealJid(contact.getJid().asBareJid());
+        for (final Conversation conversation :
+                mXmppConnectionService.findAllConferencesWith(contact)) {
+            final var mucOptions = conversation.getMucOptions();
+            final var user = mucOptions.findUserByRealJid(contact.getJid().asBareJid());
             if (user != null) {
                 clear(user);
             }
-            clear(conversation);
+            if (Strings.isNullOrEmpty(mucOptions.getAvatar())
+                    && mucOptions.isPrivateAndNonAnonymous()) {
+                clear(mucOptions);
+            }
         }
     }
 
-    private String key(Contact contact, int size) {
+    private String key(final Contact contact, final int size) {
         synchronized (this.sizes) {
             this.sizes.add(size);
         }
@@ -345,26 +350,15 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded {
         }
     }
 
-    public void clear(Conversation conversation) {
+    public void clear(final Conversation conversation) {
         if (conversation.getMode() == Conversation.MODE_SINGLE) {
             clear(conversation.getContact());
         } else {
             clear(conversation.getMucOptions());
-            synchronized (this.conversationDependentKeys) {
-                Set<String> keys = this.conversationDependentKeys.get(conversation.getUuid());
-                if (keys == null) {
-                    return;
-                }
-                LruCache<String, Bitmap> cache = this.mXmppConnectionService.getBitmapCache();
-                for (String key : keys) {
-                    cache.remove(key);
-                }
-                keys.clear();
-            }
         }
     }
 
-    private Bitmap get(MucOptions mucOptions, int size, boolean cachedOnly) {
+    private Bitmap get(final MucOptions mucOptions, final int size, final boolean cachedOnly) {
         final String KEY = key(mucOptions, size);
         Bitmap bitmap = this.mXmppConnectionService.getBitmapCache().get(KEY);
         if (bitmap != null || cachedOnly) {
@@ -377,7 +371,7 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded {
             Conversation c = mucOptions.getConversation();
             if (mucOptions.isPrivateAndNonAnonymous()) {
                 final List<MucOptions.User> users = mucOptions.getUsersRelevantForNameAndAvatar();
-                if (users.size() == 0) {
+                if (users.isEmpty()) {
                     bitmap =
                             getImpl(
                                     c.getName().toString(),
@@ -456,12 +450,12 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded {
         return PREFIX_CONVERSATION + "_" + options.getConversation().getUuid() + "_" + size;
     }
 
-    private String key(List<MucOptions.User> users, int size) {
+    private String key(final List<MucOptions.User> users, final int size) {
         final Conversation conversation = users.get(0).getConversation();
         StringBuilder builder = new StringBuilder("TILE_");
         builder.append(conversation.getUuid());
 
-        for (MucOptions.User user : users) {
+        for (final MucOptions.User user : users) {
             builder.append("\0");
             builder.append(emptyOnNull(user.getRealJid()));
             builder.append("\0");
@@ -551,12 +545,24 @@ public class AvatarService implements OnAdvancedStreamFeaturesLoaded {
         }
     }
 
-    public void clear(MucOptions.User user) {
+    public void clear(final MucOptions.User user) {
         synchronized (this.sizes) {
             for (Integer size : sizes) {
                 this.mXmppConnectionService.getBitmapCache().remove(key(user, size));
             }
         }
+        synchronized (this.conversationDependentKeys) {
+            final Set<String> keys =
+                    this.conversationDependentKeys.get(user.getConversation().getUuid());
+            if (keys == null) {
+                return;
+            }
+            final var cache = this.mXmppConnectionService.getBitmapCache();
+            for (final String key : keys) {
+                cache.remove(key);
+            }
+            keys.clear();
+        }
     }
 
     private String key(Account account, int size) {

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java 🔗

@@ -3455,15 +3455,15 @@ public class XmppConnectionService extends Service {
     }
 
     public boolean checkListeners() {
-        return (this.mOnAccountUpdates.size() == 0
-                && this.mOnConversationUpdates.size() == 0
-                && this.mOnRosterUpdates.size() == 0
-                && this.mOnCaptchaRequested.size() == 0
-                && this.mOnMucRosterUpdate.size() == 0
-                && this.mOnUpdateBlocklist.size() == 0
-                && this.mOnShowErrorToasts.size() == 0
-                && this.onJingleRtpConnectionUpdate.size() == 0
-                && this.mOnKeyStatusUpdated.size() == 0);
+        return (this.mOnAccountUpdates.isEmpty()
+                && this.mOnConversationUpdates.isEmpty()
+                && this.mOnRosterUpdates.isEmpty()
+                && this.mOnCaptchaRequested.isEmpty()
+                && this.mOnMucRosterUpdate.isEmpty()
+                && this.mOnUpdateBlocklist.isEmpty()
+                && this.mOnShowErrorToasts.isEmpty()
+                && this.onJingleRtpConnectionUpdate.isEmpty()
+                && this.mOnKeyStatusUpdated.isEmpty());
     }
 
     private void switchToForeground() {
@@ -3815,7 +3815,8 @@ public class XmppConnectionService extends Service {
                         }
                         ++i;
                         if (i >= affiliations.length) {
-                            List<Jid> members = conversation.getMucOptions().getMembers(true);
+                            final var mucOptions = conversation.getMucOptions();
+                            final var members = mucOptions.getMembers(true);
                             if (success) {
                                 List<Jid> cryptoTargets = conversation.getAcceptedCryptoTargets();
                                 boolean changed = false;
@@ -3840,7 +3841,7 @@ public class XmppConnectionService extends Service {
                                     updateConversation(conversation);
                                 }
                             }
-                            getAvatarService().clear(conversation);
+                            getAvatarService().clear(mucOptions);
                             updateMucRosterUi();
                             updateConversationUi();
                         }
@@ -4487,8 +4488,9 @@ public class XmppConnectionService extends Service {
                 request,
                 (response) -> {
                     if (response.getType() == Iq.Type.RESULT) {
-                        conference.getMucOptions().changeAffiliation(jid, affiliation);
-                        getAvatarService().clear(conference);
+                        final var mucOptions = conference.getMucOptions();
+                        mucOptions.changeAffiliation(jid, affiliation);
+                        getAvatarService().clear(mucOptions);
                         if (callback != null) {
                             callback.onAffiliationChangedSuccessful(jid);
                         } else {

src/quicksy/java/eu/siacs/conversations/ui/EnterNameActivity.java 🔗

@@ -54,7 +54,7 @@ public class EnterNameActivity extends XmppActivity
     }
 
     @Override
-    public void onSaveInstanceState(Bundle savedInstanceState) {
+    public void onSaveInstanceState(final Bundle savedInstanceState) {
         savedInstanceState.putBoolean("set_nick", this.setNick.get());
         super.onSaveInstanceState(savedInstanceState);
     }