diff --git a/ts/components/registration/RegistrationStages.tsx b/ts/components/registration/RegistrationStages.tsx index a8f422bfe..5a06f87f8 100644 --- a/ts/components/registration/RegistrationStages.tsx +++ b/ts/components/registration/RegistrationStages.tsx @@ -128,7 +128,7 @@ export async function signInWithLinking(signInDetails: { userRecoveryPhrase: str }, 60000); if (displayNameFromNetwork.length) { // display name, avatars, groups and contacts should already be handled when this event was triggered. - window?.log?.info('We got a displayName from network: '); + window?.log?.info(`We got a displayName from network: "${displayNameFromNetwork}"`); } else { window?.log?.info('Got a config message from network but without a displayName...'); throw new Error('Got a config message from network but without a displayName...'); diff --git a/ts/receiver/configMessage.ts b/ts/receiver/configMessage.ts index 9bb7b3797..7b2dd6d4b 100644 --- a/ts/receiver/configMessage.ts +++ b/ts/receiver/configMessage.ts @@ -19,7 +19,7 @@ import { getConversationController } from '../session/conversations'; import { IncomingMessage } from '../session/messages/incoming/IncomingMessage'; import { ProfileManager } from '../session/profile_manager/ProfileManager'; import { PubKey } from '../session/types'; -import { UserUtils } from '../session/utils'; +import { StringUtils, UserUtils } from '../session/utils'; import { toHex } from '../session/utils/String'; import { ConfigurationSync } from '../session/utils/job_runners/jobs/ConfigurationSyncJob'; import { IncomingConfResult, LibSessionUtil } from '../session/utils/libsession/libsession_utils'; @@ -53,6 +53,8 @@ import { queueAllCachedFromSource } from './receiver'; import { EnvelopePlus } from './types'; import { deleteAllMessagesByConvoIdNoConfirmation } from '../interactions/conversationInteractions'; +const printDumpsForDebugging = false; + function groupByVariant( incomingConfigs: Array> ) { @@ -97,6 +99,12 @@ async function mergeConfigsWithIncomingUpdates( data: msg.message.data, hash: msg.messageHash, })); + if (printDumpsForDebugging) { + window.log.info( + `printDumpsForDebugging: before merge of ${variant}:`, + StringUtils.toHex(await GenericWrapperActions.dump(variant)) + ); + } const mergedCount = await GenericWrapperActions.merge(variant, toMerge); const needsPush = await GenericWrapperActions.needsPush(variant); const needsDump = await GenericWrapperActions.needsDump(variant); @@ -106,6 +114,12 @@ async function mergeConfigsWithIncomingUpdates( `${variant}: "${publicKey}" needsPush:${needsPush} needsDump:${needsDump}; mergedCount:${mergedCount} ` ); + if (printDumpsForDebugging) { + window.log.info( + `printDumpsForDebugging: after merge of ${variant}:`, + StringUtils.toHex(await GenericWrapperActions.dump(variant)) + ); + } const incomingConfResult: IncomingConfResult = { needsDump, needsPush, diff --git a/ts/session/profile_manager/ProfileManager.ts b/ts/session/profile_manager/ProfileManager.ts index de418c279..32ac9bbae 100644 --- a/ts/session/profile_manager/ProfileManager.ts +++ b/ts/session/profile_manager/ProfileManager.ts @@ -1,8 +1,8 @@ import { isEmpty } from 'lodash'; import { getConversationController } from '../conversations'; import { UserUtils } from '../utils'; -import { AvatarDownload } from '../utils/job_runners/jobs/AvatarDownloadJob'; import { toHex } from '../utils/String'; +import { AvatarDownload } from '../utils/job_runners/jobs/AvatarDownloadJob'; /** * This can be used to update our conversation display name with the given name right away, and plan an AvatarDownloadJob to retrieve the new avatar if needed to download it @@ -42,21 +42,45 @@ async function updateProfileOfContact( window.log.warn('updateProfileOfContact can only be used for existing and private convos'); return; } - + let changes = false; const existingDisplayName = conversation.get('displayNameInProfile'); // avoid setting the display name to an invalid value if (existingDisplayName !== displayName && !isEmpty(displayName)) { conversation.set('displayNameInProfile', displayName || undefined); - await conversation.commit(); + changes = true; } - // add an avatar download job only if needed + const profileKeyHex = !profileKey || isEmpty(profileKey) ? null : toHex(profileKey); - await AvatarDownload.addAvatarDownloadJobIfNeeded({ - profileKeyHex, - profileUrl, - pubkey, - }); + + let avatarChanged = false; + // trust whatever we get as an update. It either comes from a shared config wrapper or one of that user's message. But in any case we should trust it, even if it gets resetted. + const prevPointer = conversation.get('avatarPointer'); + const prevProfileKey = conversation.get('profileKey'); + + // we have to set it right away and not in the async download job, as the next .commit will save it to the + // database and wrapper (and we do not want to override anything in the wrapper's content + // with what we have locally, so we need the commit to have already the right values in pointer and profileKey) + if (prevPointer !== profileUrl || prevProfileKey !== profileKeyHex) { + conversation.set({ + avatarPointer: profileUrl || undefined, + profileKey: profileKeyHex || undefined, + }); + + // if the avatar data we had before is not the same of what we received, we need to schedule a new avatar download job. + avatarChanged = true; // allow changes from strings to null/undefined to trigger a AvatarDownloadJob. If that happens, we want to remove the local attachment file. + } + + if (changes) { + await conversation.commit(); + } + + if (avatarChanged) { + // this call will download the new avatar or reset the local filepath if needed + await AvatarDownload.addAvatarDownloadJob({ + conversationId: pubkey, + }); + } } export const ProfileManager = { diff --git a/ts/session/utils/job_runners/JobRunner.ts b/ts/session/utils/job_runners/JobRunner.ts index 8a63a0c46..ce5fb6bfc 100644 --- a/ts/session/utils/job_runners/JobRunner.ts +++ b/ts/session/utils/job_runners/JobRunner.ts @@ -98,16 +98,6 @@ export class PersistedJobRunner { return 'type_exists'; } - // if addJobCheck returned 'removeJobsFromQueue it means that job logic estimates some jobs have to remove before adding that one. - // so let's grab the jobs to remove, remove them, and then add that new job nevertheless - if (addJobChecks === 'removeJobsFromQueue') { - // fetch all the jobs which we should remove and remove them - const toRemove = job.nonRunningJobsToRemove(serializedNonRunningJobs); - this.deleteJobsByIdentifier(toRemove.map(m => m.identifier)); - this.sortJobsList(); - await this.writeJobsToDB(); - } - // make sure there is no job with that same identifier already . window.log.info(`job runner adding type :"${job.persistedData.jobType}" `); diff --git a/ts/session/utils/job_runners/PersistedJob.ts b/ts/session/utils/job_runners/PersistedJob.ts index a21495289..b266760d9 100644 --- a/ts/session/utils/job_runners/PersistedJob.ts +++ b/ts/session/utils/job_runners/PersistedJob.ts @@ -29,8 +29,6 @@ export interface FakeSleepForMultiJobData extends PersistedJobData { export interface AvatarDownloadPersistedData extends PersistedJobData { jobType: 'AvatarDownloadJobType'; conversationId: string; - profileKeyHex: string | null; - profilePictureUrl: string | null; } export interface ConfigurationSyncPersistedData extends PersistedJobData { @@ -43,11 +41,7 @@ export type TypeOfPersistedData = | FakeSleepJobData | FakeSleepForMultiJobData; -export type AddJobCheckReturn = - | 'skipAddSameJobPresent' - | 'removeJobsFromQueue' - | 'sameJobDataAlreadyInQueue' - | null; +export type AddJobCheckReturn = 'skipAddSameJobPresent' | 'sameJobDataAlreadyInQueue' | null; export enum RunJobResult { Success = 1, diff --git a/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts b/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts index de7398fa2..fb6c5af6d 100644 --- a/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts +++ b/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts @@ -1,4 +1,4 @@ -import { isEmpty, isEqual, isNumber, isString } from 'lodash'; +import { isEmpty, isNumber, isString } from 'lodash'; import { v4 } from 'uuid'; import { UserUtils } from '../..'; import { downloadAttachment } from '../../../../receiver/attachments'; @@ -20,18 +20,11 @@ const defaultMsBetweenRetries = 10000; const defaultMaxAttemps = 3; /** - * Returns true if given those details we should add an Avatar Download Job to the list of jobs to run + * Returns true if the provided conversationId is a private chat and that we should add an Avatar Download Job to the list of jobs to run. + * Before calling this function, you have to update the related conversation profileKey and avatarPointer fields with the urls which should be downloaded, or reset them if you wanted them reset. */ -function shouldAddAvatarDownloadJob({ - profileKeyHex, - profileUrl, - pubkey, -}: { - pubkey: string; - profileUrl: string | null | undefined; - profileKeyHex: string | null | undefined; -}) { - const conversation = getConversationController().get(pubkey); +export function shouldAddAvatarDownloadJob({ conversationId }: { conversationId: string }) { + const conversation = getConversationController().get(conversationId); if (!conversation) { // return true so we do not retry this task. window.log.warn('shouldAddAvatarDownloadJob did not corresponding conversation'); @@ -43,37 +36,24 @@ function shouldAddAvatarDownloadJob({ return false; } const prevPointer = conversation.get('avatarPointer'); + const profileKey = conversation.get('profileKey'); + const hasNoAvatar = isEmpty(prevPointer) || isEmpty(profileKey); - if (!isEmpty(profileUrl) && !isEmpty(profileKeyHex) && !isEqual(prevPointer, profileUrl)) { - return true; + if (hasNoAvatar) { + return false; } - return false; + + return true; } -async function addAvatarDownloadJobIfNeeded({ - profileKeyHex, - profileUrl, - pubkey, -}: { - pubkey: string; - profileUrl: string | null | undefined; - profileKeyHex: string | null | undefined; -}) { - if (profileKeyHex && shouldAddAvatarDownloadJob({ pubkey, profileUrl, profileKeyHex })) { +async function addAvatarDownloadJob({ conversationId }: { conversationId: string }) { + if (shouldAddAvatarDownloadJob({ conversationId })) { const avatarDownloadJob = new AvatarDownloadJob({ - conversationId: pubkey, - profileKeyHex, - profilePictureUrl: profileUrl || null, + conversationId, nextAttemptTimestamp: Date.now(), }); - window.log.debug( - `addAvatarDownloadJobIfNeeded: adding job download for ${pubkey}:${profileUrl}:${profileKeyHex} ` - ); + window.log.debug(`addAvatarDownloadJobIfNeeded: adding job download for ${conversationId} `); await runners.avatarDownloadRunner.addJob(avatarDownloadJob); - } else { - // window.log.debug( - // `addAvatarDownloadJobIfNeeded: no download required for ${pubkey}:${profileUrl}:${profileKeyHex} ` - // ); } } @@ -89,12 +69,9 @@ class AvatarDownloadJob extends PersistedJob { nextAttemptTimestamp, maxAttempts, currentRetry, - profileKeyHex, - profilePictureUrl, identifier, - }: Pick & { - conversationId: string; - } & Partial< + }: Pick & + Partial< Pick< AvatarDownloadPersistedData, | 'nextAttemptTimestamp' @@ -112,8 +89,6 @@ class AvatarDownloadJob extends PersistedJob { maxAttempts: isNumber(maxAttempts) ? maxAttempts : defaultMaxAttemps, nextAttemptTimestamp: nextAttemptTimestamp || Date.now() + defaultMsBetweenRetries, currentRetry: isNumber(currentRetry) ? currentRetry : 0, - profileKeyHex, - profilePictureUrl, }); } @@ -142,103 +117,81 @@ class AvatarDownloadJob extends PersistedJob { return RunJobResult.PermanentFailure; } let changes = false; + const toDownloadPointer = conversation.get('avatarPointer'); + const toDownloadProfileKey = conversation.get('profileKey'); + + // if there is an avatar and profileKey for that user ('', null and undefined excluded), download, decrypt and save the avatar locally. + if (toDownloadPointer && toDownloadProfileKey) { + try { + window.log.debug(`[profileupdate] starting downloading task for ${conversation.id}`); + const downloaded = await downloadAttachment({ + url: toDownloadPointer, + isRaw: true, + }); + conversation = getConversationController().getOrThrow(convoId); - const shouldRunJob = shouldAddAvatarDownloadJob({ - pubkey: convoId, - profileKeyHex: this.persistedData.profileKeyHex, - profileUrl: this.persistedData.profilePictureUrl, - }); - if (!shouldRunJob) { - // return true so we do not retry this task. - window.log.warn('AvatarDownloadJob shouldAddAvatarDownloadJob said no'); - - return RunJobResult.PermanentFailure; - } + if (!downloaded.data.byteLength) { + window.log.debug(`[profileupdate] downloaded data is empty for ${conversation.id}`); + return RunJobResult.RetryJobIfPossible; // so we retry this job + } - if (this.persistedData.profilePictureUrl && this.persistedData.profileKeyHex) { - const prevPointer = conversation.get('avatarPointer'); - const needsUpdate = - !prevPointer || !isEqual(prevPointer, this.persistedData.profilePictureUrl); + // null => use placeholder with color and first letter + let path = null; - if (needsUpdate) { try { - window.log.debug(`[profileupdate] starting downloading task for ${conversation.id}`); - const downloaded = await downloadAttachment({ - url: this.persistedData.profilePictureUrl, - isRaw: true, - }); - conversation = getConversationController().getOrThrow(convoId); - - if (!downloaded.data.byteLength) { - window.log.debug(`[profileupdate] downloaded data is empty for ${conversation.id}`); - return RunJobResult.RetryJobIfPossible; // so we retry this job - } - - // null => use placeholder with color and first letter - let path = null; - + const profileKeyArrayBuffer = fromHexToArray(toDownloadProfileKey); + let decryptedData: ArrayBuffer; try { - const profileKeyArrayBuffer = fromHexToArray(this.persistedData.profileKeyHex); - let decryptedData: ArrayBuffer; - try { - decryptedData = await decryptProfile(downloaded.data, profileKeyArrayBuffer); - } catch (decryptError) { - window.log.info( - `[profileupdate] failed to decrypt downloaded data ${conversation.id} with provided profileKey` - ); - // if we cannot decrypt the content, there is no need to keep retrying. - return RunJobResult.PermanentFailure; - } - + decryptedData = await decryptProfile(downloaded.data, profileKeyArrayBuffer); + } catch (decryptError) { window.log.info( - `[profileupdate] about to auto scale avatar for convo ${conversation.id}` + `[profileupdate] failed to decrypt downloaded data ${conversation.id} with provided profileKey` ); + // if we got content, but cannot decrypt it with the provided profileKey, there is no need to keep retrying. + return RunJobResult.PermanentFailure; + } - const scaledData = await autoScaleForIncomingAvatar(decryptedData); + window.log.info( + `[profileupdate] about to auto scale avatar for convo ${conversation.id}` + ); - const upgraded = await processNewAttachment({ - data: await scaledData.blob.arrayBuffer(), - contentType: MIME.IMAGE_UNKNOWN, // contentType is mostly used to generate previews and screenshot. We do not care for those in this case. - }); - conversation = getConversationController().getOrThrow(convoId); + // we autoscale incoming avatars because our app keeps decrypted avatars in memory and some platforms allows large avatars to be uploaded. + const scaledData = await autoScaleForIncomingAvatar(decryptedData); - // Only update the convo if the download and decrypt is a success - conversation.set('avatarPointer', this.persistedData.profilePictureUrl); - conversation.set('profileKey', this.persistedData.profileKeyHex || undefined); - ({ path } = upgraded); - } catch (e) { - window?.log?.error(`[profileupdate] Could not decrypt profile image: ${e}`); - return RunJobResult.RetryJobIfPossible; // so we retry this job - } + const upgraded = await processNewAttachment({ + data: await scaledData.blob.arrayBuffer(), + contentType: MIME.IMAGE_UNKNOWN, // contentType is mostly used to generate previews and screenshot. We do not care for those in this case. + }); + conversation = getConversationController().getOrThrow(convoId); + ({ path } = upgraded); + } catch (e) { + window?.log?.error(`[profileupdate] Could not decrypt profile image: ${e}`); + return RunJobResult.RetryJobIfPossible; // so we retry this job + } - conversation.set({ avatarInProfile: path || undefined }); + conversation.set({ avatarInProfile: path || undefined }); - changes = true; - } catch (e) { - if (isString(e.message) && (e.message as string).includes('404')) { - window.log.warn( - `[profileupdate] Failed to download attachment at ${this.persistedData.profilePictureUrl}. We got 404 error: "${e.message}"` - ); - return RunJobResult.PermanentFailure; - } + changes = true; + } catch (e) { + // TODO would be nice to throw a specific exception here instead of relying on the error string. + if (isString(e.message) && (e.message as string).includes('404')) { window.log.warn( - `[profileupdate] Failed to download attachment at ${this.persistedData.profilePictureUrl}. Maybe it expired? ${e.message}` + `[profileupdate] Failed to download attachment at ${toDownloadPointer}. We got 404 error: "${e.message}"` ); - return RunJobResult.RetryJobIfPossible; + return RunJobResult.PermanentFailure; } + window.log.warn( + `[profileupdate] Failed to download attachment at ${toDownloadPointer}. Maybe it expired? ${e.message}` + ); + return RunJobResult.RetryJobIfPossible; } } else { - if ( - conversation.get('avatarInProfile') || - conversation.get('avatarPointer') || - conversation.get('profileKey') - ) { - changes = true; + // there is no valid avatar to download, make sure the local file of the avatar of that user is removed + if (conversation.get('avatarInProfile')) { conversation.set({ avatarInProfile: undefined, - avatarPointer: undefined, - profileKey: undefined, }); + changes = true; } } @@ -270,27 +223,20 @@ class AvatarDownloadJob extends PersistedJob { return super.serializeBase(); } - public nonRunningJobsToRemove(jobs: Array) { - // for an avatar download job, we want to remove any job matching the same conversationID. - return jobs.filter(j => j.conversationId === this.persistedData.conversationId); + public nonRunningJobsToRemove(_jobs: Array) { + return []; } public addJobCheck(jobs: Array): AddJobCheckReturn { // avoid adding the same job if the exact same one is already planned const hasSameJob = jobs.some(j => { - return ( - j.conversationId === this.persistedData.conversationId && - j.profileKeyHex === this.persistedData.profileKeyHex && - j.profilePictureUrl === this.persistedData.profilePictureUrl - ); + return j.conversationId === this.persistedData.conversationId; }); if (hasSameJob) { return 'skipAddSameJobPresent'; } - if (this.nonRunningJobsToRemove(jobs).length) { - return 'removeJobsFromQueue'; - } + return null; } @@ -301,5 +247,5 @@ class AvatarDownloadJob extends PersistedJob { export const AvatarDownload = { AvatarDownloadJob, - addAvatarDownloadJobIfNeeded, + addAvatarDownloadJob, };