From e6f9cb9929eac5afc274baace01c65c59ef7a854 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 25 Mar 2020 18:43:02 -0400 Subject: [PATCH] Remove TextSecurePreferences.getAvatarId() --- .../RecipientPreferenceActivity.java | 2 +- .../contacts/avatars/ProfileContactPhoto.java | 49 ++++++++++----- .../insights/InsightsRepository.java | 2 +- .../securesms/jobs/JobManagerFactories.java | 2 + .../securesms/jobs/ProfileUploadJob.java | 4 +- .../jobs/RetrieveProfileAvatarJob.java | 4 -- .../migrations/ApplicationMigrations.java | 7 ++- .../AvatarIdRemovalMigrationJob.java | 59 +++++++++++++++++++ .../widgets/ProfilePreference.java | 3 +- .../profiles/edit/EditProfileRepository.java | 2 +- .../securesms/recipients/Recipient.java | 2 +- .../securesms/util/AvatarUtil.java | 4 +- .../securesms/util/TextSecurePreferences.java | 8 --- .../api/SignalServiceAccountManager.java | 4 +- .../internal/push/PushServiceSocket.java | 12 ++-- 15 files changed, 120 insertions(+), 44 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/migrations/AvatarIdRemovalMigrationJob.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/RecipientPreferenceActivity.java b/app/src/main/java/org/thoughtcrime/securesms/RecipientPreferenceActivity.java index 764d8295a2..27221a7fb7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/RecipientPreferenceActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/RecipientPreferenceActivity.java @@ -221,7 +221,7 @@ public class RecipientPreferenceActivity extends PassphraseRequiredActionBarActi } private void setHeader(@NonNull Recipient recipient) { - ContactPhoto contactPhoto = recipient.isLocalNumber() ? new ProfileContactPhoto(recipient.getId(), String.valueOf(TextSecurePreferences.getProfileAvatarId(this))) + ContactPhoto contactPhoto = recipient.isLocalNumber() ? new ProfileContactPhoto(recipient, recipient.getProfileAvatar()) : recipient.getContactPhoto(); FallbackContactPhoto fallbackPhoto = recipient.isLocalNumber() ? new ResourceContactPhoto(R.drawable.ic_profile_outline_40, R.drawable.ic_profile_outline_20, R.drawable.ic_person_large) : recipient.getFallbackContactPhoto(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/avatars/ProfileContactPhoto.java b/app/src/main/java/org/thoughtcrime/securesms/contacts/avatars/ProfileContactPhoto.java index 76a16e585e..b6daaa8191 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/avatars/ProfileContactPhoto.java +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/avatars/ProfileContactPhoto.java @@ -6,32 +6,37 @@ import android.net.Uri; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.logging.Log; import org.thoughtcrime.securesms.profiles.AvatarHelper; +import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; +import org.whispersystems.libsignal.util.ByteUtil; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.security.MessageDigest; +import java.util.Objects; public class ProfileContactPhoto implements ContactPhoto { - private final @NonNull RecipientId recipient; - private final @NonNull String avatarObject; + private final @NonNull Recipient recipient; + private final @NonNull String avatarObject; - public ProfileContactPhoto(@NonNull RecipientId recipient, @NonNull String avatarObject) { + public ProfileContactPhoto(@NonNull Recipient recipient, @Nullable String avatarObject) { this.recipient = recipient; - this.avatarObject = avatarObject; + this.avatarObject = avatarObject == null ? "" : avatarObject; } @Override public @NonNull InputStream openInputStream(Context context) throws IOException { - return AvatarHelper.getInputStreamFor(context, recipient); + return AvatarHelper.getInputStreamFor(context, recipient.getId()); } @Override public @Nullable Uri getUri(@NonNull Context context) { - File avatarFile = AvatarHelper.getAvatarFile(context, recipient); + File avatarFile = AvatarHelper.getAvatarFile(context, recipient.getId()); return avatarFile.exists() ? Uri.fromFile(avatarFile) : null; } @@ -42,21 +47,37 @@ public class ProfileContactPhoto implements ContactPhoto { @Override public void updateDiskCacheKey(@NonNull MessageDigest messageDigest) { - messageDigest.update(recipient.serialize().getBytes()); + messageDigest.update(recipient.getId().serialize().getBytes()); messageDigest.update(avatarObject.getBytes()); + messageDigest.update(ByteUtil.longToByteArray(getFileLastModified())); } @Override - public boolean equals(Object other) { - if (other == null || !(other instanceof ProfileContactPhoto)) return false; - - ProfileContactPhoto that = (ProfileContactPhoto)other; - - return this.recipient.equals(that.recipient) && this.avatarObject.equals(that.avatarObject); + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ProfileContactPhoto that = (ProfileContactPhoto) o; + return recipient.equals(that.recipient) && + avatarObject.equals(that.avatarObject) && + getFileLastModified() == that.getFileLastModified(); } @Override public int hashCode() { - return recipient.hashCode() ^ avatarObject.hashCode(); + return Objects.hash(recipient, avatarObject, getFileLastModified()); + } + + private long getFileLastModified() { + if (!recipient.isLocalNumber()) { + return 0; + } + + File avatarFile = AvatarHelper.getAvatarFile(ApplicationDependencies.getApplication(), recipient.getId()); + + if (avatarFile.exists()) { + return avatarFile.lastModified(); + } else { + return 0; + } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/insights/InsightsRepository.java b/app/src/main/java/org/thoughtcrime/securesms/insights/InsightsRepository.java index 0d112d3831..e02ff84962 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/insights/InsightsRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/insights/InsightsRepository.java @@ -74,7 +74,7 @@ public class InsightsRepository implements InsightsDashboardViewModel.Repository fallbackColor = ContactColors.generateFor(name); } - return new InsightsUserAvatar(new ProfileContactPhoto(self.getId(), String.valueOf(TextSecurePreferences.getProfileAvatarId(context))), + return new InsightsUserAvatar(new ProfileContactPhoto(self, self.getProfileAvatar()), fallbackColor, new GeneratedContactPhoto(name, R.drawable.ic_profile_outline_40)); }, avatarConsumer::accept); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java index 149a7a9a1c..afbe60cae5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java @@ -20,6 +20,7 @@ import org.thoughtcrime.securesms.jobmanager.migrations.RecipientIdFollowUpJobMi import org.thoughtcrime.securesms.jobmanager.migrations.RecipientIdFollowUpJobMigration2; import org.thoughtcrime.securesms.jobmanager.migrations.RecipientIdJobMigration; import org.thoughtcrime.securesms.jobmanager.migrations.SendReadReceiptsJobMigration; +import org.thoughtcrime.securesms.migrations.AvatarIdRemovalMigrationJob; import org.thoughtcrime.securesms.migrations.PassingMigrationJob; import org.thoughtcrime.securesms.migrations.AvatarMigrationJob; import org.thoughtcrime.securesms.migrations.CachedAttachmentsMigrationJob; @@ -108,6 +109,7 @@ public final class JobManagerFactories { put(ProfileUploadJob.KEY, new ProfileUploadJob.Factory()); // Migrations + put(AvatarIdRemovalMigrationJob.KEY, new AvatarIdRemovalMigrationJob.Factory()); put(AvatarMigrationJob.KEY, new AvatarMigrationJob.Factory()); put(CachedAttachmentsMigrationJob.KEY, new CachedAttachmentsMigrationJob.Factory()); put(DatabaseMigrationJob.KEY, new DatabaseMigrationJob.Factory()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileUploadJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileUploadJob.java index 186239ce06..a9e68df9a6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileUploadJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileUploadJob.java @@ -54,10 +54,10 @@ public final class ProfileUploadJob extends BaseJob { try (StreamDetails avatar = AvatarHelper.getSelfProfileAvatarStream(context)) { if (FeatureFlags.VERSIONED_PROFILES) { - avatarPath = accountManager.setVersionedProfile(Recipient.self().getUuid().get(), profileKey, profileName.serialize(), avatar); + avatarPath = accountManager.setVersionedProfile(Recipient.self().getUuid().get(), profileKey, profileName.serialize(), avatar).orNull(); } else { accountManager.setProfileName(profileKey, profileName.serialize()); - avatarPath = accountManager.setProfileAvatar(profileKey, avatar); + avatarPath = accountManager.setProfileAvatar(profileKey, avatar).orNull(); } } 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 c93326a9a2..96bf2fdc78 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java @@ -120,10 +120,6 @@ public class RetrieveProfileAvatarJob extends BaseJob { } database.setProfileAvatar(recipient.getId(), profileAvatar); - - if (recipient.isLocalNumber()) { - TextSecurePreferences.setProfileAvatarId(context, Util.getSecureRandom().nextInt()); - } } @Override diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java index b7bae93f01..81c66c92ac 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -39,7 +39,7 @@ public class ApplicationMigrations { private static final int LEGACY_CANONICAL_VERSION = 455; - public static final int CURRENT_VERSION = 12; + public static final int CURRENT_VERSION = 13; private static final class Version { static final int LEGACY = 1; @@ -54,6 +54,7 @@ public class ApplicationMigrations { static final int SWOON_STICKERS = 10; static final int STORAGE_SERVICE = 11; static final int STORAGE_KEY_ROTATE = 12; + static final int REMOVE_AVATAR_ID = 13; } /** @@ -215,6 +216,10 @@ public class ApplicationMigrations { jobs.put(Version.STORAGE_KEY_ROTATE, new StorageKeyRotationMigrationJob()); } + if (lastSeenVersion < Version.REMOVE_AVATAR_ID) { + jobs.put(Version.REMOVE_AVATAR_ID, new AvatarIdRemovalMigrationJob()); + } + return jobs; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/AvatarIdRemovalMigrationJob.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/AvatarIdRemovalMigrationJob.java new file mode 100644 index 0000000000..d822153b28 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/AvatarIdRemovalMigrationJob.java @@ -0,0 +1,59 @@ +package org.thoughtcrime.securesms.migrations; + +import androidx.annotation.NonNull; + +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.jobmanager.Data; +import org.thoughtcrime.securesms.jobmanager.Job; +import org.thoughtcrime.securesms.jobmanager.JobManager; +import org.thoughtcrime.securesms.jobs.MultiDeviceKeysUpdateJob; +import org.thoughtcrime.securesms.jobs.RefreshOwnProfileJob; +import org.thoughtcrime.securesms.jobs.StorageSyncJob; +import org.thoughtcrime.securesms.logging.Log; +import org.thoughtcrime.securesms.util.TextSecurePreferences; + +/** + * We just want to make sure that the user has a profile avatar set in the RecipientDatabase, so + * we're refreshing their own profile. + */ +public class AvatarIdRemovalMigrationJob extends MigrationJob { + + private static final String TAG = Log.tag(AvatarIdRemovalMigrationJob.class); + + public static final String KEY = "AvatarIdRemovalMigrationJob"; + + AvatarIdRemovalMigrationJob() { + this(new Parameters.Builder().build()); + } + + private AvatarIdRemovalMigrationJob(@NonNull Parameters parameters) { + super(parameters); + } + + @Override + public boolean isUiBlocking() { + return false; + } + + @Override + public @NonNull String getFactoryKey() { + return KEY; + } + + @Override + public void performMigration() { + ApplicationDependencies.getJobManager().add(new RefreshOwnProfileJob()); + } + + @Override + boolean shouldRetry(@NonNull Exception e) { + return false; + } + + public static class Factory implements Job.Factory { + @Override + public @NonNull AvatarIdRemovalMigrationJob create(@NonNull Parameters parameters, @NonNull Data data) { + return new AvatarIdRemovalMigrationJob(parameters); + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/widgets/ProfilePreference.java b/app/src/main/java/org/thoughtcrime/securesms/preferences/widgets/ProfilePreference.java index 3a32228665..c4bf1c4747 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/widgets/ProfilePreference.java +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/widgets/ProfilePreference.java @@ -19,6 +19,7 @@ import org.thoughtcrime.securesms.contacts.avatars.ResourceContactPhoto; import org.thoughtcrime.securesms.mms.GlideApp; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.util.TextSecurePreferences; +import org.thoughtcrime.securesms.util.Util; public class ProfilePreference extends Preference { @@ -68,7 +69,7 @@ public class ProfilePreference extends Preference { final String profileName = Recipient.self().getProfileName().toString(); GlideApp.with(getContext().getApplicationContext()) - .load(new ProfileContactPhoto(self.getId(), String.valueOf(TextSecurePreferences.getProfileAvatarId(getContext())))) + .load(new ProfileContactPhoto(self, self.getProfileAvatar())) .error(new ResourceContactPhoto(R.drawable.ic_camera_solid_white_24).asDrawable(getContext(), getContext().getResources().getColor(R.color.grey_400))) .circleCrop() .diskCacheStrategy(DiskCacheStrategy.ALL) diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileRepository.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileRepository.java index 9259bba6ce..dbfcce1aaf 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/edit/EditProfileRepository.java @@ -20,6 +20,7 @@ import org.thoughtcrime.securesms.profiles.ProfileName; import org.thoughtcrime.securesms.profiles.SystemProfileUtil; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; +import org.thoughtcrime.securesms.util.Base64; import org.thoughtcrime.securesms.util.ProfileUtil; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.Util; @@ -106,7 +107,6 @@ class EditProfileRepository { try { AvatarHelper.setAvatar(context, Recipient.self().getId(), avatar); - TextSecurePreferences.setProfileAvatarId(context, new SecureRandom().nextInt()); } catch (IOException e) { return UploadResult.ERROR_FILE_IO; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java index 6cbe999371..6d92dfdcca 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java @@ -608,7 +608,7 @@ public class Recipient { if (localNumber) return null; else if (isGroupInternal() && groupAvatarId.isPresent()) return new GroupRecordContactPhoto(groupId, groupAvatarId.get()); else if (systemContactPhoto != null) return new SystemContactPhoto(id, systemContactPhoto, 0); - else if (profileAvatar != null) return new ProfileContactPhoto(id, profileAvatar); + else if (profileAvatar != null) return new ProfileContactPhoto(this, profileAvatar); else return null; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/AvatarUtil.java b/app/src/main/java/org/thoughtcrime/securesms/util/AvatarUtil.java index c7c600938d..2028013de8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/AvatarUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/AvatarUtil.java @@ -47,14 +47,14 @@ public final class AvatarUtil { public static GlideRequest getSelfAvatarOrFallbackIcon(@NonNull Context context, @DrawableRes int fallbackIcon) { return GlideApp.with(context) .asDrawable() - .load(new ProfileContactPhoto(Recipient.self().getId(), String.valueOf(TextSecurePreferences.getProfileAvatarId(context)))) + .load(new ProfileContactPhoto(Recipient.self(), Recipient.self().getProfileAvatar())) .error(fallbackIcon) .circleCrop() .diskCacheStrategy(DiskCacheStrategy.ALL); } private static GlideRequest request(@NonNull GlideRequest glideRequest, @NonNull Context context, @NonNull Recipient recipient) { - return glideRequest.load(new ProfileContactPhoto(recipient.getId(), String.valueOf(TextSecurePreferences.getProfileAvatarId(context)))) + return glideRequest.load(new ProfileContactPhoto(recipient, recipient.getProfileAvatar())) .error(getFallback(context, recipient)) .circleCrop() .diskCacheStrategy(DiskCacheStrategy.ALL); diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java b/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java index f437d3e08a..1624b411ae 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/TextSecurePreferences.java @@ -434,14 +434,6 @@ public class TextSecurePreferences { setBooleanPreference(context, GIF_GRID_LAYOUT, isGrid); } - public static void setProfileAvatarId(Context context, int id) { - setIntegerPrefrence(context, PROFILE_AVATAR_ID_PREF, id); - } - - public static int getProfileAvatarId(Context context) { - return getIntegerPreference(context, PROFILE_AVATAR_ID_PREF, 0); - } - public static int getNotificationPriority(Context context) { return Integer.valueOf(getStringPreference(context, NOTIFICATION_PRIORITY_PREF, String.valueOf(NotificationCompat.PRIORITY_HIGH))); } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountManager.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountManager.java index c6b51acfa6..4de6c6cb92 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountManager.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountManager.java @@ -641,7 +641,7 @@ public class SignalServiceAccountManager { this.pushServiceSocket.setProfileName(ciphertextName); } - public String setProfileAvatar(ProfileKey key, StreamDetails avatar) + public Optional setProfileAvatar(ProfileKey key, StreamDetails avatar) throws IOException { if (FeatureFlags.VERSIONED_PROFILES) { @@ -663,7 +663,7 @@ public class SignalServiceAccountManager { /** * @return The avatar URL path, if one was written. */ - public String setVersionedProfile(UUID uuid, ProfileKey profileKey, String name, StreamDetails avatar) + public Optional setVersionedProfile(UUID uuid, ProfileKey profileKey, String name, StreamDetails avatar) throws IOException { if (!FeatureFlags.VERSIONED_PROFILES) { diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java index 44181b1759..eb74f0c72f 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java @@ -604,7 +604,7 @@ public class PushServiceSocket { makeServiceRequest(String.format(PROFILE_PATH, "name/" + (name == null ? "" : URLEncoder.encode(name))), "PUT", ""); } - public String setProfileAvatar(ProfileAvatarData profileAvatar) + public Optional setProfileAvatar(ProfileAvatarData profileAvatar) throws NonSuccessfulResponseCodeException, PushNetworkException { if (FeatureFlags.VERSIONED_PROFILES) { @@ -629,16 +629,16 @@ public class PushServiceSocket { profileAvatar.getContentType(), profileAvatar.getDataLength(), profileAvatar.getOutputStreamFactory(), null, null); - return formAttributes.getKey(); + return Optional.of(formAttributes.getKey()); } - return null; + return Optional.absent(); } /** * @return The avatar URL path, if one was written. */ - public String writeProfile(SignalServiceProfileWrite signalServiceProfileWrite, ProfileAvatarData profileAvatar) + public Optional writeProfile(SignalServiceProfileWrite signalServiceProfileWrite, ProfileAvatarData profileAvatar) throws NonSuccessfulResponseCodeException, PushNetworkException { if (!FeatureFlags.VERSIONED_PROFILES) { @@ -665,10 +665,10 @@ public class PushServiceSocket { profileAvatar.getContentType(), profileAvatar.getDataLength(), profileAvatar.getOutputStreamFactory(), null, null); - return formAttributes.getKey(); + return Optional.of(formAttributes.getKey()); } - return null; + return Optional.absent(); } public void setUsername(String username) throws IOException {