From f215535f75b5fc07045c0fbd3b692748c5fb888d Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 10 Feb 2023 16:30:02 +1100 Subject: [PATCH] fix: set and get profile picture from libsession --- ts/receiver/configMessage.ts | 2 +- ts/session/profile_manager/ProfileManager.ts | 7 ++-- ts/session/utils/job_runners/JobRunner.ts | 39 ++++++++++++++++--- .../job_runners/jobs/AvatarDownloadJob.ts | 20 ++++++++-- .../job_runners/jobs/ConfigurationSyncJob.ts | 14 ++----- .../utils/libsession/libsession_utils.ts | 1 - .../libsession_wrapper_contacts_test.ts | 6 +-- .../libsession_wrapper_test.ts | 15 +++++-- .../node/libsession/libsession.worker.ts | 2 +- 9 files changed, 73 insertions(+), 33 deletions(-) diff --git a/ts/receiver/configMessage.ts b/ts/receiver/configMessage.ts index 737bc3c1a..59547d623 100644 --- a/ts/receiver/configMessage.ts +++ b/ts/receiver/configMessage.ts @@ -37,6 +37,7 @@ async function mergeConfigsWithIncomingUpdates( const wrapperId = LibSessionUtil.kindToVariant(kind); await GenericWrapperActions.merge(wrapperId, toMerge); + const needsPush = await GenericWrapperActions.needsPush(wrapperId); const needsDump = await GenericWrapperActions.needsDump(wrapperId); const messageHashes = [incomingConfig.messageHash]; @@ -59,7 +60,6 @@ async function handleUserProfileUpdate(result: IncomingConfResult): Promise(jobRunner: JobRunnerType, job: PersistedJob) { + return `id: "${job.persistedData.identifier}" (type: "${jobRunner}")`; +} + /** * This class is used to plan jobs and make sure they are retried until the success. * By having a specific type, we can find the logic to be run by that type of job. @@ -79,7 +83,7 @@ export class PersistedJobRunner { if (this.jobsScheduled.find(j => j.persistedData.identifier === job.persistedData.identifier)) { window.log.info( - `job runner has already a job with id:"${job.persistedData.identifier}" planned so not adding another one` + `job runner (${this.jobRunnerType}) has already a job with id:"${job.persistedData.identifier}" planned so not adding another one` ); return 'identifier_exists'; } @@ -168,7 +172,7 @@ export class PersistedJobRunner { private async writeJobsToDB() { const serialized = this.getSerializedJobs(); - window.log.warn('writing to db', serialized); + window.log.warn(`writing to db for "${this.jobRunnerType}": `, serialized); await Data.createOrUpdateItem({ id: this.getJobRunnerItemId(), value: JSON.stringify(serialized), @@ -176,7 +180,6 @@ export class PersistedJobRunner { } private async addJobUnchecked(job: PersistedJob) { - console.warn('job', job); this.jobsScheduled.push(cloneDeep(job)); this.sortJobsList(); await this.writeJobsToDB(); @@ -254,7 +257,12 @@ export class PersistedJobRunner { private deleteJobsByIdentifier(identifiers: Array) { identifiers.forEach(identifier => { const jobIndex = this.jobsScheduled.findIndex(f => f.persistedData.identifier === identifier); - window.log.info('deleteJobsByIdentifier job', identifier, ' index', jobIndex); + window.log.info( + `removing job ${jobToLogId( + this.jobRunnerType, + this.jobsScheduled[jobIndex] + )} at ${jobIndex}` + ); if (jobIndex >= 0) { this.jobsScheduled.splice(jobIndex, 1); @@ -290,21 +298,40 @@ export class PersistedJobRunner { success = await timeout(this.currentJob.runJob(), this.currentJob.getJobTimeoutMs()); if (success !== RunJobResult.Success) { - throw new Error(`job ${nextJob.persistedData.identifier} failed`); + throw new Error('return result was not "Success"'); } // here the job did not throw and didn't return false. Consider it OK then and remove it from the list of jobs to run. this.deleteJobsByIdentifier([this.currentJob.persistedData.identifier]); await this.writeJobsToDB(); } catch (e) { - window.log.warn(`JobRunner current ${this.jobRunnerType} failed with ${e.message}`); + window.log.info(`${jobToLogId(this.jobRunnerType, nextJob)} failed with "${e.message}"`); if ( success === RunJobResult.PermanentFailure || nextJob.persistedData.currentRetry >= nextJob.persistedData.maxAttempts - 1 ) { + if (success === RunJobResult.PermanentFailure) { + window.log.info( + `${jobToLogId(this.jobRunnerType, nextJob)}:${ + nextJob.persistedData.currentRetry + } permament failure for job` + ); + } else { + window.log.info( + `Too many failures for ${jobToLogId( + this.jobRunnerType, + nextJob + )} out of nextJob.persistedData.maxAttempts` + ); + } // we cannot restart this job anymore. Remove the entry completely this.deleteJobsByIdentifier([nextJob.persistedData.identifier]); } else { + window.log.info( + `Rescheduling ${jobToLogId(this.jobRunnerType, nextJob)} in ${ + nextJob.persistedData.delayBetweenRetries + }...` + ); nextJob.persistedData.currentRetry = nextJob.persistedData.currentRetry + 1; // that job can be restarted. Plan a retry later with the already defined retry nextJob.persistedData.nextAttemptTimestamp = diff --git a/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts b/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts index b91c70851..b6f7c9d06 100644 --- a/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts +++ b/ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts @@ -60,11 +60,11 @@ async function addAvatarDownloadJobIfNeeded({ profileKeyHex: string | null | undefined; }) { if (profileKeyHex && shouldAddAvatarDownloadJob({ pubkey, profileUrl, profileKeyHex })) { - debugger; const avatarDownloadJob = new AvatarDownloadJob({ conversationId: pubkey, profileKeyHex, profilePictureUrl: profileUrl || null, + nextAttemptTimestamp: Date.now(), }); window.log.debug( `addAvatarDownloadJobIfNeeded: adding job download for ${pubkey}:${profileUrl}:${profileKeyHex} ` @@ -130,8 +130,6 @@ class AvatarDownloadJob extends PersistedJob { return RunJobResult.PermanentFailure; } - debugger; - let conversation = getConversationController().get(convoId); if (!conversation) { // return true so we do not retry this task. @@ -171,12 +169,26 @@ class AvatarDownloadJob extends PersistedJob { }); 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; try { const profileKeyArrayBuffer = fromHexToArray(this.persistedData.profileKeyHex); - const decryptedData = await decryptProfile(downloaded.data, profileKeyArrayBuffer); + 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; + } window.log.info( `[profileupdate] about to auto scale avatar for convo ${conversation.id}` diff --git a/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts b/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts index 1bcfabf3e..05f3fbddd 100644 --- a/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts +++ b/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts @@ -1,4 +1,3 @@ -import { from_string } from 'libsodium-wrappers-sumo'; import { compact, groupBy, isArray, isEmpty, isNumber, isString, uniq } from 'lodash'; import { v4 } from 'uuid'; import { UserUtils } from '../..'; @@ -12,6 +11,7 @@ import { getConversationController } from '../../../conversations'; import { SharedConfigMessage } from '../../../messages/outgoing/controlMessage/SharedConfigMessage'; import { MessageSender } from '../../../sending/MessageSender'; import { LibSessionUtil, OutgoingConfResult } from '../../libsession/libsession_utils'; +import { fromHexToArray } from '../../String'; import { runners } from '../JobRunner'; import { AddJobCheckReturn, @@ -116,10 +116,6 @@ function resultsToSuccessfulChange( request.destination ) { // a message was posted. We need to add it to the tracked list of hashes - console.warn( - `messagePostedHashes for j:${j}; didDeleteOldConfigMessages:${didDeleteOldConfigMessages}: `, - messagePostedHashes - ); const updatedHashes: Array = didDeleteOldConfigMessages ? [messagePostedHashes] : uniq(compact([...request.allOldHashes, messagePostedHashes])); @@ -132,7 +128,6 @@ function resultsToSuccessfulChange( } } } catch (e) { - console.warn('eeee', e); throw e; } @@ -153,7 +148,6 @@ async function buildAndSaveDumpsToDB(changes: Array): Promise< continue; } const dump = await GenericWrapperActions.dump(variant); - console.warn('change.updatedHash', change.updatedHash); await ConfigDumpData.saveConfigDump({ data: dump, publicKey: change.publicKey, @@ -205,7 +199,8 @@ class ConfigurationSyncJob extends PersistedJob await UserConfigWrapperActions.setName(name || ''); if (profileKey && pointer) { - await UserConfigWrapperActions.setProfilePicture(pointer, from_string(profileKey)); + const profileKeyArray = fromHexToArray(profileKey); + await UserConfigWrapperActions.setProfilePicture(pointer, profileKeyArray); } else { await UserConfigWrapperActions.setProfilePicture('', new Uint8Array()); } @@ -235,9 +230,6 @@ class ConfigurationSyncJob extends PersistedJob }) ); - console.warn( - `ConfigurationSyncJob sendToPubKeyNonDurably ${this.persistedData.identifier} returned: "${allResults}"` - ); // we do a sequence call here. If we do not have the right expected number of results, consider it if (!isArray(allResults) || allResults.length !== singleDestChanges.length) { diff --git a/ts/session/utils/libsession/libsession_utils.ts b/ts/session/utils/libsession/libsession_utils.ts index d0ffae464..4151ceb53 100644 --- a/ts/session/utils/libsession/libsession_utils.ts +++ b/ts/session/utils/libsession/libsession_utils.ts @@ -160,7 +160,6 @@ async function pendingChangesForPubkey(pubkey: string): Promise = []; - debugger; for (let index = 0; index < dumps.length; index++) { const dump = dumps[index]; const variant = dump.variant; diff --git a/ts/test/session/unit/libsession_wrapper/libsession_wrapper_contacts_test.ts b/ts/test/session/unit/libsession_wrapper/libsession_wrapper_contacts_test.ts index ae747e37b..f3811ca65 100644 --- a/ts/test/session/unit/libsession_wrapper/libsession_wrapper_contacts_test.ts +++ b/ts/test/session/unit/libsession_wrapper/libsession_wrapper_contacts_test.ts @@ -5,7 +5,7 @@ import { from_hex, from_string } from 'libsodium-wrappers-sumo'; // tslint:disable: chai-vague-errors no-unused-expression no-http-string no-octal-literal whitespace no-require-imports variable-name import * as SessionUtilWrapper from 'session_util_wrapper'; -describe('libsession_wrapper_contacts ', () => { +describe('libsession_contacts', () => { // Note: To run this test, you need to compile the libsession wrapper for node (and not for electron). // To do this, you can cd to the node_module/libsession_wrapper folder and do // yarn configure && yarn build @@ -13,7 +13,7 @@ describe('libsession_wrapper_contacts ', () => { // We have to disable it by filename as nodejs tries to load the module during the import step above, and fails as it is not compiled for nodejs but for electron. - it('[config][contacts]', () => { + it('libsession_contacts', () => { const edSecretKey = from_hex( '0123456789abcdef0123456789abcdef000000000000000000000000000000004cb76fdc6d32278e3f83dbf608360ecc6b65727934b85d2fb86862ff98c46ab7' ); @@ -186,7 +186,7 @@ describe('libsession_wrapper_contacts ', () => { expect(nicknames2).to.be.deep.eq(['(N/A)', 'Nickname 3']); }); - it('[config][contacts][c]', () => { + it('libsession_contacts_c', () => { const edSecretKey = from_hex( '0123456789abcdef0123456789abcdef000000000000000000000000000000004cb76fdc6d32278e3f83dbf608360ecc6b65727934b85d2fb86862ff98c46ab7' ); diff --git a/ts/test/session/unit/libsession_wrapper/libsession_wrapper_test.ts b/ts/test/session/unit/libsession_wrapper/libsession_wrapper_test.ts index 59fe4c38e..47eec9493 100644 --- a/ts/test/session/unit/libsession_wrapper/libsession_wrapper_test.ts +++ b/ts/test/session/unit/libsession_wrapper/libsession_wrapper_test.ts @@ -7,7 +7,7 @@ import { concatUInt8Array } from '../../../../session/crypto'; // tslint:disable: chai-vague-errors no-unused-expression no-http-string no-octal-literal whitespace describe('libsession_wrapper', () => { - it('[config][user_profile][c]', () => { + it('libsession_user', () => { // Note: To run this test, you need to compile the libsession wrapper for node (and not for electron). // To do this, you can cd to the node_module/libsession_wrapper folder and do // yarn configure && yarn build @@ -46,7 +46,14 @@ describe('libsession_wrapper', () => { expect(picResult.key).to.be.null; // Now let's go set a profile name and picture: - conf.setProfilePicture('http://example.org/omg-pic-123.bmp', stringToUint8Array('secret')); + conf.setProfilePicture( + 'http://example.org/omg-pic-123.bmp', + new Uint8Array([115, 101, 99, 114, 101, 116]) // 'secret' in ascii + ); + expect(conf.getProfilePicture().key).to.be.deep.eq( + new Uint8Array([115, 101, 99, 114, 101, 116]) // 'secret' in ascii + ); + conf.setName('Kallie'); // Retrieve them just to make sure they set properly: @@ -58,7 +65,9 @@ describe('libsession_wrapper', () => { const picture = conf.getProfilePicture(); expect(picture.url).to.be.eq('http://example.org/omg-pic-123.bmp'); - expect(picture.key).to.be.deep.eq(stringToUint8Array('secret')); + expect(picture.key).to.be.deep.eq( + new Uint8Array([115, 101, 99, 114, 101, 116]) // 'secret' in ascii + ); // Since we've made changes, we should need to push new config to the swarm, *and* should need // to dump the updated state: diff --git a/ts/webworker/workers/node/libsession/libsession.worker.ts b/ts/webworker/workers/node/libsession/libsession.worker.ts index 0a85469c8..5f904d80c 100644 --- a/ts/webworker/workers/node/libsession/libsession.worker.ts +++ b/ts/webworker/workers/node/libsession/libsession.worker.ts @@ -95,8 +95,8 @@ onmessage = async (e: { data: [number, ConfigWrapperObjectTypes, string, ...any] } const wrapper = await getCorrespondingWrapper(config); - const fn = (wrapper as any)[action]; + if (!fn) { throw new Error( `Worker: job "${jobId}" did not find function "${action}" on config "${config}"`