From fef965bcb57f2c670f9a61cc9880623f334878af Mon Sep 17 00:00:00 2001 From: Al Lansley Date: Wed, 3 Apr 2024 09:48:57 +1100 Subject: [PATCH] SES-1356 - List of recently used reaction emojis is not accurate (#1400) * WIP * Further WIP * Push prior to cleanup * Fixes #1015 * Added limiting to the count of recently used emoji that we store * Put back adjusted reaction pill layout to standard * Adjusted recently used reaction emojis already in list to go to start of list --------- Co-authored-by: = <=> --- .../emoji/RecentEmojiPageModel.java | 132 +++++++----------- .../conversation/v2/ConversationActivityV2.kt | 2 + app/src/main/res/layout/reactions_pill.xml | 2 +- 3 files changed, 50 insertions(+), 86 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/emoji/RecentEmojiPageModel.java b/app/src/main/java/org/thoughtcrime/securesms/components/emoji/RecentEmojiPageModel.java index 6cc39c8d14..0e2de9068c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/emoji/RecentEmojiPageModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/emoji/RecentEmojiPageModel.java @@ -3,133 +3,95 @@ package org.thoughtcrime.securesms.components.emoji; import android.content.Context; import android.content.SharedPreferences; import android.net.Uri; -import android.os.AsyncTask; import android.preference.PreferenceManager; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.annimon.stream.Stream; -import com.fasterxml.jackson.databind.type.CollectionType; -import com.fasterxml.jackson.databind.type.TypeFactory; import org.session.libsignal.utilities.JsonUtil; import org.session.libsignal.utilities.Log; -import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Iterator; -import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; import network.loki.messenger.R; public class RecentEmojiPageModel implements EmojiPageModel { private static final String TAG = RecentEmojiPageModel.class.getSimpleName(); - private static final String EMOJI_LRU_PREFERENCE = "pref_recent_emoji2"; - private static final int EMOJI_LRU_SIZE = 50; - public static final String KEY = "Recents"; - public static final List DEFAULT_REACTIONS_LIST = - Arrays.asList("\ud83d\ude02", "\ud83e\udd70", "\ud83d\ude22", "\ud83d\ude21", "\ud83d\ude2e", "\ud83d\ude08"); + public static final String RECENT_EMOJIS_KEY = "Recents"; - private final SharedPreferences prefs; - private final LinkedHashSet recentlyUsed; + public static final LinkedList DEFAULT_REACTION_EMOJIS_LIST = new LinkedList<>(Arrays.asList( + "\ud83d\ude02", + "\ud83e\udd70", + "\ud83d\ude22", + "\ud83d\ude21", + "\ud83d\ude2e", + "\ud83d\ude08")); + + public static final String DEFAULT_REACTION_EMOJIS_JSON_STRING = JsonUtil.toJson(new LinkedList<>(DEFAULT_REACTION_EMOJIS_LIST)); + private static SharedPreferences prefs; + private static LinkedList recentlyUsed; public RecentEmojiPageModel(Context context) { - this.prefs = PreferenceManager.getDefaultSharedPreferences(context); - this.recentlyUsed = getPersistedCache(); - } + prefs = PreferenceManager.getDefaultSharedPreferences(context); - private LinkedHashSet getPersistedCache() { - String serialized = prefs.getString(EMOJI_LRU_PREFERENCE, "[]"); - try { - CollectionType collectionType = TypeFactory.defaultInstance() - .constructCollectionType(LinkedHashSet.class, String.class); - return JsonUtil.getMapper().readValue(serialized, collectionType); - } catch (IOException e) { - Log.w(TAG, e); - return new LinkedHashSet<>(); - } + // Note: Do NOT try to populate or update the persisted recent emojis in the constructor - the + // `getEmoji` method ends up getting called half-way through in a race-condition manner. } @Override - public String getKey() { - return KEY; - } + public String getKey() { return RECENT_EMOJIS_KEY; } - @Override public int getIconAttr() { - return R.attr.emoji_category_recent; - } + @Override public int getIconAttr() { return R.attr.emoji_category_recent; } @Override public List getEmoji() { - List recent = new ArrayList<>(recentlyUsed); - List out = new ArrayList<>(DEFAULT_REACTIONS_LIST.size()); - - for (int i = 0; i < DEFAULT_REACTIONS_LIST.size(); i++) { - if (recent.size() > i) { - out.add(recent.get(i)); - } else { - out.add(DEFAULT_REACTIONS_LIST.get(i)); + // Populate our recently used list if required (i.e., on first run) + if (recentlyUsed == null) { + try { + String recentlyUsedEmjoiJsonString = prefs.getString(RECENT_EMOJIS_KEY, DEFAULT_REACTION_EMOJIS_JSON_STRING); + recentlyUsed = JsonUtil.fromJson(recentlyUsedEmjoiJsonString, LinkedList.class); + } catch (Exception e) { + Log.w(TAG, e); + Log.d(TAG, "Default reaction emoji data was corrupt (likely via key re-use on app upgrade) - rewriting fresh data."); + boolean writeSuccess = prefs.edit().putString(RECENT_EMOJIS_KEY, DEFAULT_REACTION_EMOJIS_JSON_STRING).commit(); + if (!writeSuccess) { Log.w(TAG, "Failed to update recently used emojis in shared prefs."); } + recentlyUsed = DEFAULT_REACTION_EMOJIS_LIST; } } - - return out; + return new ArrayList<>(recentlyUsed); } @Override public List getDisplayEmoji() { return Stream.of(getEmoji()).map(Emoji::new).toList(); } - @Override public boolean hasSpriteMap() { - return false; - } + @Override public boolean hasSpriteMap() { return false; } @Nullable @Override - public Uri getSpriteUri() { - return null; - } + public Uri getSpriteUri() { return null; } - @Override public boolean isDynamic() { - return true; - } + @Override public boolean isDynamic() { return true; } - public void onCodePointSelected(String emoji) { - recentlyUsed.remove(emoji); - recentlyUsed.add(emoji); + public static void onCodePointSelected(String emoji) { + // If the emoji is already in the recently used list then remove it.. + if (recentlyUsed.contains(emoji)) { recentlyUsed.removeFirstOccurrence(emoji); } - if (recentlyUsed.size() > EMOJI_LRU_SIZE) { - Iterator iterator = recentlyUsed.iterator(); - iterator.next(); - iterator.remove(); - } + // ..and then regardless of whether the emoji used was already in the recently used list or not + // it gets placed as the first element in the list.. + recentlyUsed.addFirst(emoji); - final LinkedHashSet latestRecentlyUsed = new LinkedHashSet<>(recentlyUsed); - new AsyncTask() { + // Ensure that we only ever store data for a maximum of 6 recently used emojis (this code will + // execute if if we did NOT remove any occurrence of a previously used emoji but then added the + // new emoji to the front of the list). + while (recentlyUsed.size() > 6) { recentlyUsed.removeLast(); } - @Override - protected Void doInBackground(Void... params) { - try { - String serialized = JsonUtil.toJsonThrows(latestRecentlyUsed); - prefs.edit() - .putString(EMOJI_LRU_PREFERENCE, serialized) - .apply(); - } catch (IOException e) { - Log.w(TAG, e); - } - - return null; - } - }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); - } - - private String[] toReversePrimitiveArray(@NonNull LinkedHashSet emojiSet) { - String[] emojis = new String[emojiSet.size()]; - int i = emojiSet.size() - 1; - for (String emoji : emojiSet) { - emojis[i--] = emoji; - } - return emojis; + // ..which we then save to shared prefs. + String recentlyUsedAsJsonString = JsonUtil.toJson(recentlyUsed); + boolean writeSuccess = prefs.edit().putString(RECENT_EMOJIS_KEY, recentlyUsedAsJsonString).commit(); + if (!writeSuccess) { Log.w(TAG, "Failed to update recently used emojis in shared prefs."); } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt index 39c7bf4196..d0a0298e1c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt @@ -106,6 +106,7 @@ import org.thoughtcrime.securesms.ApplicationContext import org.thoughtcrime.securesms.PassphraseRequiredActionBarActivity import org.thoughtcrime.securesms.attachments.ScreenshotObserver import org.thoughtcrime.securesms.audio.AudioRecorder +import org.thoughtcrime.securesms.components.emoji.RecentEmojiPageModel import org.thoughtcrime.securesms.contacts.SelectContactsActivity.Companion.selectedContactsKey import org.thoughtcrime.securesms.conversation.ConversationActionBarDelegate import org.thoughtcrime.securesms.conversation.disappearingmessages.DisappearingMessagesActivity @@ -1335,6 +1336,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe sendEmojiRemoval(emoji, messageRecord) } else { sendEmojiReaction(emoji, messageRecord) + RecentEmojiPageModel.onCodePointSelected(emoji) // Save to recently used reaction emojis } } diff --git a/app/src/main/res/layout/reactions_pill.xml b/app/src/main/res/layout/reactions_pill.xml index c87f29d82c..a88673cba8 100644 --- a/app/src/main/res/layout/reactions_pill.xml +++ b/app/src/main/res/layout/reactions_pill.xml @@ -15,7 +15,7 @@ android:layout_width="17dp" android:layout_height="17dp" /> -