From a1fd6cc1d8791294dbde3e7a419d710abd74d65f Mon Sep 17 00:00:00 2001 From: ThomasSession Date: Tue, 25 Feb 2025 01:06:04 +0200 Subject: [PATCH] Avatar reupload config update (#971) * Making sure we update the config when reuploading an avatar as a way to keep the file server image alive * Wrong error type * PR feedback - cleaned up download usage --- .../preferences/SettingsViewModel.kt | 5 ++-- .../messaging/jobs/AttachmentDownloadJob.kt | 4 ++- .../jobs/RetrieveProfileAvatarJob.kt | 9 +++++- .../libsession/utilities/DownloadUtilities.kt | 29 ++++++++++++------- .../utilities/ProfilePictureUtilities.kt | 20 +++++++++---- 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt index 76514df5e2..20e52b1eac 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsViewModel.kt @@ -197,7 +197,7 @@ class SettingsViewModel @Inject constructor( try { // Grab the profile key and kick of the promise to update the profile picture val encodedProfileKey = ProfileKeyUtil.generateEncodedProfileKey(context) - ProfilePictureUtilities.upload(profilePicture, encodedProfileKey, context) + val url = ProfilePictureUtilities.upload(profilePicture, encodedProfileKey, context) // If the online portion of the update succeeded then update the local state AvatarHelper.setAvatar( @@ -217,11 +217,10 @@ class SettingsViewModel @Inject constructor( ProfileKeyUtil.setEncodedProfileKey(context, encodedProfileKey) // Attempt to grab the details we require to update the profile picture - val url = prefs.getProfilePictureURL() val profileKey = ProfileKeyUtil.getProfileKey(context) // If we have a URL and a profile key then set the user's profile picture - if (!url.isNullOrEmpty() && profileKey.isNotEmpty()) { + if (url.isNotEmpty() && profileKey.isNotEmpty()) { configFactory.withMutableUserConfigs { it.userProfile.setPic(UserPic(url, profileKey)) } diff --git a/libsession/src/main/java/org/session/libsession/messaging/jobs/AttachmentDownloadJob.kt b/libsession/src/main/java/org/session/libsession/messaging/jobs/AttachmentDownloadJob.kt index 559f7c4cfa..b5c7a21b20 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/jobs/AttachmentDownloadJob.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/jobs/AttachmentDownloadJob.kt @@ -15,6 +15,7 @@ import org.session.libsession.snode.utilities.await import org.session.libsession.utilities.DecodedAudio import org.session.libsession.utilities.DownloadUtilities import org.session.libsession.utilities.InputStreamMediaDataSource +import org.session.libsignal.exceptions.NonRetryableException import org.session.libsignal.streams.AttachmentCipherInputStream import org.session.libsignal.utilities.Base64 import org.session.libsignal.utilities.Log @@ -74,7 +75,8 @@ class AttachmentDownloadJob(val attachmentID: Long, val databaseMessageID: Long) val threadID = storage.getThreadIdForMms(databaseMessageID) val handleFailure: (java.lang.Exception, attachmentId: AttachmentId?) -> Unit = { exception, attachment -> - if (exception == Error.NoAttachment + if (exception is NonRetryableException || + exception == Error.NoAttachment || exception == Error.NoThread || exception == Error.NoSender || (exception is OnionRequestAPI.HTTPRequestFailedAtDestinationException && exception.statusCode == 400)) { diff --git a/libsession/src/main/java/org/session/libsession/messaging/jobs/RetrieveProfileAvatarJob.kt b/libsession/src/main/java/org/session/libsession/messaging/jobs/RetrieveProfileAvatarJob.kt index fbdd151b60..bd39c353d9 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/jobs/RetrieveProfileAvatarJob.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/jobs/RetrieveProfileAvatarJob.kt @@ -10,6 +10,7 @@ import org.session.libsession.utilities.TextSecurePreferences.Companion.setProfi import org.session.libsession.utilities.Util.copy import org.session.libsession.utilities.Util.equals import org.session.libsession.utilities.recipients.Recipient +import org.session.libsignal.exceptions.NonRetryableException import org.session.libsignal.streams.ProfileCipherInputStream import org.session.libsignal.utilities.Log import org.session.libsignal.utilities.Util.SECURE_RANDOM @@ -91,7 +92,13 @@ class RetrieveProfileAvatarJob( } storage.setProfilePicture(recipient, profileAvatar, profileKey) - } catch (e: Exception) { + } + catch (e: NonRetryableException){ + Log.e("Loki", "Failed to download profile avatar from non-retryable error", e) + errorUrls += profileAvatar + return delegate.handleJobFailedPermanently(this, dispatcherName, e) + } + catch (e: Exception) { Log.e("Loki", "Failed to download profile avatar", e) if (failureCount + 1 >= maxFailureCount) { errorUrls += profileAvatar diff --git a/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt b/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt index fd53ec5a16..f159d20907 100644 --- a/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt +++ b/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt @@ -6,6 +6,7 @@ import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import org.session.libsession.messaging.file_server.FileServerApi import org.session.libsession.snode.utilities.await +import org.session.libsignal.exceptions.NonRetryableException import org.session.libsignal.utilities.HTTP import org.session.libsignal.utilities.Log import java.io.File @@ -18,20 +19,28 @@ object DownloadUtilities { * Blocks the calling thread. */ suspend fun downloadFile(destination: File, url: String) { - val outputStream = FileOutputStream(destination) // Throws var remainingAttempts = 2 var exception: Exception? = null - while (remainingAttempts > 0) { - remainingAttempts -= 1 - try { - downloadFile(outputStream, url) - exception = null - break - } catch (e: Exception) { - exception = e + + destination.outputStream().use { outputStream -> + while (remainingAttempts > 0) { + remainingAttempts -= 1 + + try { + downloadFile(outputStream, url) + return // return on success + } catch (e: HTTP.HTTPRequestFailedException) { + if (e.statusCode == 404) { + throw NonRetryableException("404 response trying to download file: $url", e) + } + exception = e + } catch (e: Exception) { + exception = e + } } } - if (exception != null) { throw exception } + + throw exception ?: NonRetryableException("Couldn't download file: $url") } /** diff --git a/libsession/src/main/java/org/session/libsession/utilities/ProfilePictureUtilities.kt b/libsession/src/main/java/org/session/libsession/utilities/ProfilePictureUtilities.kt index a7dc316f1e..c5e4e82d53 100644 --- a/libsession/src/main/java/org/session/libsession/utilities/ProfilePictureUtilities.kt +++ b/libsession/src/main/java/org/session/libsession/utilities/ProfilePictureUtilities.kt @@ -5,8 +5,10 @@ import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch +import network.loki.messenger.libsession_util.util.UserPic import okio.Buffer import org.session.libsession.avatars.AvatarHelper +import org.session.libsession.messaging.MessagingModuleConfiguration import org.session.libsession.messaging.file_server.FileServerApi import org.session.libsession.snode.utilities.await import org.session.libsession.utilities.Address.Companion.fromSerialized @@ -19,11 +21,10 @@ import org.session.libsignal.streams.ProfileCipherOutputStream import org.session.libsignal.streams.ProfileCipherOutputStreamFactory import org.session.libsignal.utilities.Log import org.session.libsignal.utilities.ProfileAvatarData -import org.session.libsignal.utilities.ThreadUtils.queue import org.session.libsignal.utilities.retryIfNeeded import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream -import java.util.* +import java.util.Date object ProfilePictureUtilities { @@ -39,8 +40,7 @@ object ProfilePictureUtilities { // Don't generate a new profile key here; we do that when the user changes their profile picture Log.d("Loki-Avatar", "Uploading Avatar Started") - val encodedProfileKey = - getProfileKey(context) + val encodedProfileKey = getProfileKey(context) try { // Read the file into a byte array val inputStream = AvatarHelper.getInputStreamFor( @@ -58,7 +58,7 @@ object ProfilePictureUtilities { baos.flush() val profilePicture = baos.toByteArray() // Re-upload it - upload( + val url = upload( profilePicture, encodedProfileKey!!, context @@ -70,6 +70,12 @@ object ProfilePictureUtilities { Date().time ) + // update config with new URL for reuploaded file + val profileKey = ProfileKeyUtil.getProfileKey(context) + MessagingModuleConfiguration.shared.configFactory.withMutableUserConfigs { + it.userProfile.setPic(UserPic(url, profileKey)) + } + Log.d("Loki-Avatar", "Uploading Avatar Finished") } catch (e: Exception) { Log.e("Loki-Avatar", "Uploading avatar failed.") @@ -77,7 +83,7 @@ object ProfilePictureUtilities { } } - suspend fun upload(profilePicture: ByteArray, encodedProfileKey: String, context: Context) { + suspend fun upload(profilePicture: ByteArray, encodedProfileKey: String, context: Context): String { val inputStream = ByteArrayInputStream(profilePicture) val outputStream = ProfileCipherOutputStream.getCiphertextLength(profilePicture.size.toLong()) @@ -108,5 +114,7 @@ object ProfilePictureUtilities { TextSecurePreferences.setLastProfilePictureUpload(context, Date().time) val url = "${FileServerApi.server}/file/$id" TextSecurePreferences.setProfilePictureURL(context, url) + + return url } } \ No newline at end of file