Remove TextSecurePreferences.getAvatarId()

signalapp · Mar 26, 2020 · e6f9cb9 · e6f9cb9
1 parent 6aac3ba
commit e6f9cb9
Show file tree

Hide file tree

Showing 15 changed files with 120 additions and 44 deletions.
diff --git a/app/src/main/java/org/thoughtcrime/securesms/RecipientPreferenceActivity.java b/app/src/main/java/org/thoughtcrime/securesms/RecipientPreferenceActivity.java
@@ -221,7 +221,7 @@ private void initializeToolbar() {
   }
 
   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
@@ -6,32 +6,37 @@
 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 boolean isProfilePhoto() {
 
   @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
@@ -74,7 +74,7 @@ public void getUserAvatar(@NonNull Consumer<InsightsUserAvatar> avatarConsumer)
         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
@@ -20,6 +20,7 @@
 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 static Map<String, Job.Factory> getJobFactories(@NonNull Application appl
       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
@@ -54,10 +54,10 @@ protected void onRun() throws Exception {
 
     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
@@ -120,10 +120,6 @@ public void onRun() throws IOException {
     }
 
     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
@@ -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 @@ private static final class Version {
     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 @@ private static LinkedHashMap<Integer, MigrationJob> getMigrationJobs(@NonNull Co
       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
@@ -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<AvatarIdRemovalMigrationJob> {
+    @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
@@ -19,6 +19,7 @@
 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 void refresh() {
     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
@@ -20,6 +20,7 @@
 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 @@ void uploadProfile(@NonNull ProfileName profileName, @Nullable byte[] avatar, @N
 
       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
@@ -608,7 +608,7 @@ public boolean isPushGroup() {
     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
@@ -47,14 +47,14 @@ public static IconCompat getIconForNotification(@NonNull Context context, @NonNu
   public static GlideRequest<Drawable> 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 <T> GlideRequest<T> request(@NonNull GlideRequest<T> 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
@@ -434,14 +434,6 @@ public static void setIsGifSearchInGridLayout(Context context, boolean isGrid) {
     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/...rvice/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountManager.java b/...rvice/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountManager.java
@@ -641,7 +641,7 @@ public void setProfileName(ProfileKey key, String name)
     this.pushServiceSocket.setProfileName(ciphertextName);
   }
 
-  public String setProfileAvatar(ProfileKey key, StreamDetails avatar)
+  public Optional<String> setProfileAvatar(ProfileKey key, StreamDetails avatar)
       throws IOException
   {
     if (FeatureFlags.VERSIONED_PROFILES) {
@@ -663,7 +663,7 @@ public String setProfileAvatar(ProfileKey key, StreamDetails avatar)
   /**
    * @return The avatar URL path, if one was written.
    */
-  public String setVersionedProfile(UUID uuid, ProfileKey profileKey, String name, StreamDetails avatar)
+  public Optional<String> setVersionedProfile(UUID uuid, ProfileKey profileKey, String name, StreamDetails avatar)
     throws IOException
   {
     if (!FeatureFlags.VERSIONED_PROFILES) {

diff --git a/...rvice/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java b/...rvice/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java
@@ -604,7 +604,7 @@ public void setProfileName(String name) throws NonSuccessfulResponseCodeExceptio
     makeServiceRequest(String.format(PROFILE_PATH, "name/" + (name == null ? "" : URLEncoder.encode(name))), "PUT", "");
   }
 
-  public String setProfileAvatar(ProfileAvatarData profileAvatar)
+  public Optional<String> setProfileAvatar(ProfileAvatarData profileAvatar)
       throws NonSuccessfulResponseCodeException, PushNetworkException
   {
     if (FeatureFlags.VERSIONED_PROFILES) {
@@ -629,16 +629,16 @@ public String setProfileAvatar(ProfileAvatarData profileAvatar)
                   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<String> writeProfile(SignalServiceProfileWrite signalServiceProfileWrite, ProfileAvatarData profileAvatar)
     throws NonSuccessfulResponseCodeException, PushNetworkException
   {
     if (!FeatureFlags.VERSIONED_PROFILES) {
@@ -665,10 +665,10 @@ public String writeProfile(SignalServiceProfileWrite signalServiceProfileWrite,
                   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 {