From 6649a9a745bccea2d6c6ac5de565d9b8070b9be6 Mon Sep 17 00:00:00 2001 From: Harris Date: Tue, 15 Mar 2022 09:24:15 +1100 Subject: [PATCH] Generate placeholder avatars from two characters, re-fetch missed avatars (#856) * feat: splitting names in the avatar generation * fix: re-fetch avatars if initial downloads fail * fix: remove shadowed name, add tests for common labels --- .../securesms/jobmanager/JobManager.java | 2 +- .../jobs/RetrieveProfileAvatarJob.java | 2 +- .../util/AvatarPlaceholderGenerator.kt | 31 +++++++++++++------ .../recipients/AvatarGeneratorTest.kt | 26 ++++++++++++++++ .../libsession/avatars/AvatarHelper.java | 5 +++ .../ReceivedMessageHandler.kt | 18 +++++++---- 6 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/recipients/AvatarGeneratorTest.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobManager.java b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobManager.java index 440f37508b..5906afd292 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobManager.java @@ -254,7 +254,7 @@ public class JobManager implements ConstraintObserver.Notifier { public static class Builder { private ExecutorFactory executorFactory = new DefaultExecutorFactory(); - private int jobThreadCount = Math.max(2, Math.min(Runtime.getRuntime().availableProcessors() - 1, 4)); + private int jobThreadCount = 1; private Map jobFactories = new HashMap<>(); private Map constraintFactories = new HashMap<>(); private List constraintObservers = new ArrayList<>(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java index 897b98e97f..8b24dfc485 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java @@ -84,7 +84,7 @@ public class RetrieveProfileAvatarJob extends BaseJob { return; } - if (Util.equals(profileAvatar, recipient.resolve().getProfileAvatar())) { + if (AvatarHelper.avatarFileExists(context, recipient.resolve().getAddress()) && Util.equals(profileAvatar, recipient.resolve().getProfileAvatar())) { Log.w(TAG, "Already retrieved profile avatar: " + profileAvatar); return; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/AvatarPlaceholderGenerator.kt b/app/src/main/java/org/thoughtcrime/securesms/util/AvatarPlaceholderGenerator.kt index b93a4a0905..b55f82eae7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/AvatarPlaceholderGenerator.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/AvatarPlaceholderGenerator.kt @@ -1,14 +1,20 @@ package org.thoughtcrime.securesms.util import android.content.Context -import android.graphics.* +import android.graphics.Bitmap +import android.graphics.Canvas +import android.graphics.Color +import android.graphics.Paint +import android.graphics.Rect +import android.graphics.RectF +import android.graphics.Typeface import android.graphics.drawable.BitmapDrawable import android.text.TextPaint import android.text.TextUtils import network.loki.messenger.R import java.math.BigInteger import java.security.MessageDigest -import java.util.* +import java.util.Locale object AvatarPlaceholderGenerator { @@ -28,7 +34,7 @@ object AvatarPlaceholderGenerator { val colorPrimary = colorArray[(hash % colorArray.size).toInt()] val labelText = when { - !TextUtils.isEmpty(displayName) -> extractLabel(displayName!!.capitalize()) + !TextUtils.isEmpty(displayName) -> extractLabel(displayName!!.capitalize(Locale.ROOT)) !TextUtils.isEmpty(hashString) -> extractLabel(hashString) else -> EMPTY_LABEL } @@ -57,14 +63,19 @@ object AvatarPlaceholderGenerator { return BitmapDrawable(context.resources, bitmap) } - private fun extractLabel(content: String): String { - var content = content.trim() - if (content.isEmpty()) return EMPTY_LABEL - return if (content.length > 2 && content.startsWith("05")) { - content[2].toString().toUpperCase(Locale.ROOT) + fun extractLabel(content: String): String { + val trimmedContent = content.trim() + if (trimmedContent.isEmpty()) return EMPTY_LABEL + return if (trimmedContent.length > 2 && trimmedContent.startsWith("05")) { + trimmedContent[2].toString() } else { - content.first().toString().toUpperCase(Locale.ROOT) - } + val splitWords = trimmedContent.split(Regex("\\W")) + if (splitWords.size < 2) { + trimmedContent.take(2) + } else { + splitWords.filter { word -> word.isNotEmpty() }.take(2).map { it.first() }.joinToString("") + } + }.uppercase() } private fun getSha512(input: String): String { diff --git a/app/src/test/java/org/thoughtcrime/securesms/recipients/AvatarGeneratorTest.kt b/app/src/test/java/org/thoughtcrime/securesms/recipients/AvatarGeneratorTest.kt new file mode 100644 index 0000000000..909442277f --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/recipients/AvatarGeneratorTest.kt @@ -0,0 +1,26 @@ +package org.thoughtcrime.securesms.recipients + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.thoughtcrime.securesms.util.AvatarPlaceholderGenerator + +class AvatarGeneratorTest { + + @Test + fun testCommonAvatarFormats() { + val testNamesAndResults = mapOf( + "H " to "H", + "Test Name" to "TN", + "test name" to "TN", + "howdy partner" to "HP", + "testname" to "TE", // + "05aaapubkey" to "A", // pubkey values only return first non-05 character + "Test" to "TE" + ) + testNamesAndResults.forEach { (test, expected) -> + val processed = AvatarPlaceholderGenerator.extractLabel(test) + assertEquals(expected, processed) + } + } + +} \ No newline at end of file diff --git a/libsession/src/main/java/org/session/libsession/avatars/AvatarHelper.java b/libsession/src/main/java/org/session/libsession/avatars/AvatarHelper.java index 1215757f26..1588289017 100644 --- a/libsession/src/main/java/org/session/libsession/avatars/AvatarHelper.java +++ b/libsession/src/main/java/org/session/libsession/avatars/AvatarHelper.java @@ -46,6 +46,11 @@ public class AvatarHelper { return new File(avatarDirectory, new File(address.serialize()).getName()); } + public static boolean avatarFileExists(@NonNull Context context , @NonNull Address address) { + File avatarFile = getAvatarFile(context, address); + return avatarFile.exists(); + } + public static void setAvatar(@NonNull Context context, @NonNull Address address, @Nullable byte[] data) throws IOException { diff --git a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt index baa95a875b..999129aa33 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt @@ -1,6 +1,7 @@ package org.session.libsession.messaging.sending_receiving import android.text.TextUtils +import org.session.libsession.avatars.AvatarHelper import org.session.libsession.messaging.MessagingModuleConfiguration import org.session.libsession.messaging.jobs.AttachmentDownloadJob import org.session.libsession.messaging.jobs.JobQueue @@ -188,28 +189,33 @@ fun MessageReceiver.handleVisibleMessage(message: VisibleMessage, proto: SignalS val storage = MessagingModuleConfiguration.shared.storage val context = MessagingModuleConfiguration.shared.context val userPublicKey = storage.getUserPublicKey() + val messageSender: String? = message.sender // Get or create thread // FIXME: In case this is an open group this actually * doesn't * create the thread if it doesn't yet // exist. This is intentional, but it's very non-obvious. val threadID = storage.getOrCreateThreadIdFor(message.syncTarget - ?: message.sender!!, message.groupPublicKey, openGroupID) + ?: messageSender!!, message.groupPublicKey, openGroupID) if (threadID < 0) { // Thread doesn't exist; should only be reached in a case where we are processing open group messages for a no longer existent thread throw MessageReceiver.Error.NoThread } // Update profile if needed - val recipient = Recipient.from(context, Address.fromSerialized(message.sender!!), false) + val recipient = Recipient.from(context, Address.fromSerialized(messageSender!!), false) val profile = message.profile - if (profile != null && userPublicKey != message.sender) { + if (profile != null && userPublicKey != messageSender) { val profileManager = SSKEnvironment.shared.profileManager val name = profile.displayName!! if (name.isNotEmpty()) { profileManager.setName(context, recipient, name) } val newProfileKey = profile.profileKey - if (newProfileKey?.isNotEmpty() == true && (newProfileKey.size == 16 || newProfileKey.size == 32) && profile.profilePictureURL?.isNotEmpty() == true - && (recipient.profileKey == null || !MessageDigest.isEqual(recipient.profileKey, newProfileKey))) { - profileManager.setProfileKey(context, recipient, newProfileKey) + + val needsProfilePicture = !AvatarHelper.avatarFileExists(context, Address.fromSerialized(messageSender)) + val profileKeyValid = newProfileKey?.isNotEmpty() == true && (newProfileKey.size == 16 || newProfileKey.size == 32) && profile.profilePictureURL?.isNotEmpty() == true + val profileKeyChanged = (recipient.profileKey == null || !MessageDigest.isEqual(recipient.profileKey, newProfileKey)) + + if ((profileKeyValid && profileKeyChanged) || (profileKeyValid && needsProfilePicture)) { + profileManager.setProfileKey(context, recipient, newProfileKey!!) profileManager.setUnidentifiedAccessMode(context, recipient, Recipient.UnidentifiedAccessMode.UNKNOWN) profileManager.setProfilePictureURL(context, recipient, profile.profilePictureURL!!) }