jonny
/
Signal-Desktop
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Optimize profile avatar uploads and sync urls 

Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
This commit is contained in:
automated-signal 2022-03-16 10:03:12 -07:00 committed by GitHub
parent 36bb201537
commit 09d9cd477d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 147 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
ts/components/AvatarPreview.tsx
View File

@ -2,7 +2,7 @@
// SPDX-License-Identifier: AGPL-3.0-only
import type { CSSProperties } from 'react';
import React, { useEffect, useRef, useState } from 'react';
import React, { useEffect, useState } from 'react';
import { noop } from 'lodash';
import * as log from '../logging/log';
@ -46,10 +46,6 @@ export const AvatarPreview = ({
onClick,
style = {},
}: PropsType): JSX.Element => {
const startingAvatarPathRef = useRef<undefined | string>(
avatarValue ? undefined : avatarPath
);
const [avatarPreview, setAvatarPreview] = useState<Uint8Array | undefined>();
// Loads the initial avatarPath if one is provided, but only if we're in editable mode.
@ -60,8 +56,7 @@ export const AvatarPreview = ({
return;
}
const startingAvatarPath = startingAvatarPathRef.current;
if (!startingAvatarPath) {
if (!avatarPath) {
return noop;
}
@ -69,14 +64,12 @@ export const AvatarPreview = ({
(async () => {
try {
const buffer = await imagePathToBytes(startingAvatarPath);
const buffer = await imagePathToBytes(avatarPath);
if (shouldCancel) {
return;
}
setAvatarPreview(buffer);
if (onAvatarLoaded) {
onAvatarLoaded(buffer);
}
onAvatarLoaded?.(buffer);
} catch (err) {
if (shouldCancel) {
return;
@ -92,7 +85,7 @@ export const AvatarPreview = ({
return () => {
shouldCancel = true;
};
}, [onAvatarLoaded, isEditable]);
}, [avatarPath, onAvatarLoaded, isEditable]);
// Ensures that when avatarValue changes we generate new URLs
useEffect(() => {

4
ts/components/ProfileEditor.stories.tsx
View File

@ -25,7 +25,7 @@ const stories = storiesOf('Components/ProfileEditor', module);
const createProps = (overrideProps: Partial<PropsType> = {}): PropsType => ({
aboutEmoji: overrideProps.aboutEmoji,
aboutText: text('about', overrideProps.aboutText || ''),
avatarPath: overrideProps.avatarPath,
profileAvatarPath: overrideProps.profileAvatarPath,
clearUsernameSave: action('clearUsernameSave'),
conversationId: '123',
color: overrideProps.color || getRandomColor(),
@ -64,7 +64,7 @@ stories.add('Full Set', () => {
{...createProps({
aboutEmoji: '🙏',
aboutText: 'Live. Laugh. Love',
avatarPath: '/fixtures/kitten-3-64-64.jpg',
profileAvatarPath: '/fixtures/kitten-3-64-64.jpg',
onSetSkinTone: setSkinTone,
familyName: getLastName(),
skinTone,

53
ts/components/ProfileEditor.tsx
View File

@ -9,6 +9,7 @@ import type { AvatarColorType } from '../types/Colors';
import { AvatarColors } from '../types/Colors';
import type {
AvatarDataType,
AvatarUpdateType,
DeleteAvatarFromDiskActionType,
ReplaceAvatarActionType,
SaveAvatarToDiskActionType,
@ -58,14 +59,14 @@ type PropsExternalType = {
onEditStateChanged: (editState: EditState) => unknown;
onProfileChanged: (
profileData: ProfileDataType,
avatarBuffer?: Uint8Array
avatar: AvatarUpdateType
) => unknown;
};
export type PropsDataType = {
aboutEmoji?: string;
aboutText?: string;
avatarPath?: string;
profileAvatarPath?: string;
color?: AvatarColorType;
conversationId: string;
familyName?: string;
@ -211,7 +212,7 @@ function mapSaveStateToEditState({
export const ProfileEditor = ({
aboutEmoji,
aboutText,
avatarPath,
profileAvatarPath,
clearUsernameSave,
color,
conversationId,
@ -254,9 +255,16 @@ export const ProfileEditor = ({
UsernameEditState.Editing
);
const [startingAvatarPath, setStartingAvatarPath] =
useState(profileAvatarPath);
const [oldAvatarBuffer, setOldAvatarBuffer] = useState<
Uint8Array | undefined
>(undefined);
const [avatarBuffer, setAvatarBuffer] = useState<Uint8Array | undefined>(
undefined
);
const [isLoadingAvatar, setIsLoadingAvatar] = useState(true);
const [stagedProfile, setStagedProfile] = useState<ProfileDataType>({
aboutEmoji,
aboutText,
@ -285,6 +293,9 @@ export const ProfileEditor = ({
// To make AvatarEditor re-render less often
const handleAvatarChanged = useCallback(
(avatar: Uint8Array | undefined) => {
// Do not display stale avatar from disk anymore.
setStartingAvatarPath(undefined);
setAvatarBuffer(avatar);
setEditState(EditState.None);
onProfileChanged(
@ -295,10 +306,11 @@ export const ProfileEditor = ({
? trim(stagedProfile.familyName)
: undefined,
},
avatar
{ oldAvatar: oldAvatarBuffer, newAvatar: avatar }
);
setOldAvatarBuffer(avatar);
},
[onProfileChanged, stagedProfile]
[onProfileChanged, stagedProfile, oldAvatarBuffer]
);
const getFullNameText = () => {
@ -405,9 +417,14 @@ export const ProfileEditor = ({
};
// To make AvatarEditor re-render less often
const handleAvatarLoaded = useCallback(avatar => {
setAvatarBuffer(avatar);
}, []);
const handleAvatarLoaded = useCallback(
avatar => {
setAvatarBuffer(avatar);
setOldAvatarBuffer(avatar);
setIsLoadingAvatar(false);
},
[setAvatarBuffer, setOldAvatarBuffer, setIsLoadingAvatar]
);
let content: JSX.Element;
@ -415,7 +432,7 @@ export const ProfileEditor = ({
content = (
<AvatarEditor
avatarColor={color || AvatarColors[0]}
avatarPath={avatarPath}
avatarPath={startingAvatarPath}
avatarValue={avatarBuffer}
conversationId={conversationId}
conversationTitle={getFullNameText()}
@ -430,6 +447,7 @@ export const ProfileEditor = ({
);
} else if (editState === EditState.ProfileName) {
const shouldDisableSave =
isLoadingAvatar ||
!stagedProfile.firstName ||
(stagedProfile.firstName === fullName.firstName &&
stagedProfile.familyName === fullName.familyName) ||
@ -502,7 +520,10 @@ export const ProfileEditor = ({
familyName: stagedProfile.familyName,
});
onProfileChanged(stagedProfile, avatarBuffer);
onProfileChanged(stagedProfile, {
oldAvatar: oldAvatarBuffer,
newAvatar: avatarBuffer,
});
handleBack();
}}
>
@ -513,8 +534,9 @@ export const ProfileEditor = ({
);
} else if (editState === EditState.Bio) {
const shouldDisableSave =
stagedProfile.aboutText === fullBio.aboutText &&
stagedProfile.aboutEmoji === fullBio.aboutEmoji;
isLoadingAvatar ||
(stagedProfile.aboutText === fullBio.aboutText &&
stagedProfile.aboutEmoji === fullBio.aboutEmoji);
content = (
<>
@ -613,7 +635,10 @@ export const ProfileEditor = ({
aboutText: stagedProfile.aboutText,
});
onProfileChanged(stagedProfile, avatarBuffer);
onProfileChanged(stagedProfile, {
oldAvatar: oldAvatarBuffer,
newAvatar: avatarBuffer,
});
handleBack();
}}
>
@ -689,7 +714,7 @@ export const ProfileEditor = ({
<>
<AvatarPreview
avatarColor={color}
avatarPath={avatarPath}
avatarPath={startingAvatarPath}
avatarValue={avatarBuffer}
conversationTitle={getFullNameText()}
i18n={i18n}

3
ts/components/ProfileEditorModal.tsx
View File

@ -7,6 +7,7 @@ import { ConfirmationDialog } from './ConfirmationDialog';
import type { PropsType as ProfileEditorPropsType } from './ProfileEditor';
import { ProfileEditor, EditState } from './ProfileEditor';
import type { ProfileDataType } from '../state/ducks/conversations';
import type { AvatarUpdateType } from '../types/Avatar';
export type PropsDataType = {
hasError: boolean;
@ -15,7 +16,7 @@ export type PropsDataType = {
type PropsType = {
myProfileChanged: (
profileData: ProfileDataType,
avatarBuffer?: Uint8Array
avatar: AvatarUpdateType
) => unknown;
toggleProfileEditor: () => unknown;
toggleProfileEditorHasError: () => unknown;

6
ts/models/conversations.ts
View File

@ -1812,6 +1812,7 @@ export class ConversationModel extends window.Backbone
avatarPath: this.getAbsoluteAvatarPath(),
avatarHash: this.getAvatarHash(),
unblurredAvatarPath: this.getAbsoluteUnblurredAvatarPath(),
profileAvatarPath: this.getAbsoluteProfileAvatarPath(),
color,
conversationColor: this.getConversationColor(),
customColor,
@ -4947,6 +4948,11 @@ export class ConversationModel extends window.Backbone
return avatarPath ? getAbsoluteAttachmentPath(avatarPath) : undefined;
}
getAbsoluteProfileAvatarPath(): string | undefined {
const avatarPath = this.get('profileAvatar')?.path;
return avatarPath ? getAbsoluteAttachmentPath(avatarPath) : undefined;
}
getAbsoluteUnblurredAvatarPath(): string | undefined {
const unblurredAvatarPath = this.get('unblurredAvatarPath');
return unblurredAvatarPath

13
ts/services/storageRecordOps.ts
View File

@ -178,7 +178,10 @@ export function toAccountRecord(
if (conversation.get('profileFamilyName')) {
accountRecord.familyName = conversation.get('profileFamilyName') || '';
}
accountRecord.avatarUrl = window.storage.get('avatarUrl') || '';
const avatarUrl = window.storage.get('avatarUrl');
if (avatarUrl !== undefined) {
accountRecord.avatarUrl = avatarUrl;
}
accountRecord.noteToSelfArchived = Boolean(conversation.get('isArchived'));
accountRecord.noteToSelfMarkedUnread = Boolean(
conversation.get('markedUnread')
@ -895,7 +898,6 @@ export async function mergeAccountRecord(
): Promise<MergeResultType> {
let details = new Array<string>();
const {
avatarUrl,
linkPreviews,
notDiscoverableByPhoneNumber,
noteToSelfArchived,
@ -1146,10 +1148,9 @@ export async function mergeAccountRecord(
{ viaStorageServiceSync: true }
);
if (avatarUrl) {
await conversation.setProfileAvatar(avatarUrl, profileKey);
window.storage.put('avatarUrl', avatarUrl);
}
const avatarUrl = dropNull(accountRecord.avatarUrl);
await conversation.setProfileAvatar(avatarUrl, profileKey);
window.storage.put('avatarUrl', avatarUrl);
}
const { hasConflict, details: extraDetails } = doesRecordHavePendingChanges(

53
ts/services/writeProfile.ts
View File

@ -11,10 +11,11 @@ import { getProfile } from '../util/getProfile';
import { singleProtoJobQueue } from '../jobs/singleProtoJobQueue';
import { strictAssert } from '../util/assert';
import { isWhitespace } from '../util/whitespaceStringUtil';
import type { AvatarUpdateType } from '../types/Avatar';
export async function writeProfile(
conversation: ConversationType,
avatarBuffer?: Uint8Array
avatar: AvatarUpdateType
): Promise<void> {
// Before we write anything we request the user's profile so that we can
// have an up-to-date paymentAddress to be able to include it when we write
@ -41,7 +42,7 @@ export async function writeProfile(
const [profileData, encryptedAvatarData] = await encryptProfileData(
conversation,
avatarBuffer
avatar
);
const avatarRequestHeaders = await window.textsecure.messaging.putProfile(
profileData
@ -50,37 +51,49 @@ export async function writeProfile(
// Upload the avatar if provided
// delete existing files on disk if avatar has been removed
// update the account's avatar path and hash if it's a new avatar
let profileAvatar:
| {
hash: string;
path: string;
}
| undefined;
if (avatarRequestHeaders && encryptedAvatarData && avatarBuffer) {
await window.textsecure.messaging.uploadAvatar(
const { newAvatar } = avatar;
let maybeProfileAvatarUpdate: {
profileAvatar?:
| {
hash: string;
path: string;
}
| undefined;
} = {};
if (profileData.sameAvatar) {
log.info('writeProfile: not updating avatar');
} else if (avatarRequestHeaders && encryptedAvatarData && newAvatar) {
log.info('writeProfile: uploading new avatar');
const avatarUrl = await window.textsecure.messaging.uploadAvatar(
avatarRequestHeaders,
encryptedAvatarData
);
const hash = await computeHash(avatarBuffer);
const hash = await computeHash(newAvatar);
if (hash !== avatarHash) {
log.info('writeProfile: removing old avatar and saving the new one');
const [path] = await Promise.all([
window.Signal.Migrations.writeNewAttachmentData(avatarBuffer),
window.Signal.Migrations.writeNewAttachmentData(newAvatar),
avatarPath
? window.Signal.Migrations.deleteAttachmentData(avatarPath)
: undefined,
]);
profileAvatar = {
hash,
path,
maybeProfileAvatarUpdate = {
profileAvatar: { hash, path },
};
}
} else if (avatarPath) {
await window.Signal.Migrations.deleteAttachmentData(avatarPath);
}
const profileAvatarData = profileAvatar ? { profileAvatar } : {};
await window.storage.put('avatarUrl', avatarUrl);
} else if (avatarPath) {
log.info('writeProfile: removing avatar');
await Promise.all([
window.Signal.Migrations.deleteAttachmentData(avatarPath),
window.storage.put('avatarUrl', undefined),
]);
maybeProfileAvatarUpdate = { profileAvatar: undefined };
}
// Update backbone, update DB, run storage service upload
model.set({
@ -88,7 +101,7 @@ export async function writeProfile(
aboutEmoji,
profileName: firstName,
profileFamilyName: familyName,
...profileAvatarData,
...maybeProfileAvatarUpdate,
});
dataInterface.updateConversation(model.attributes);

7
ts/state/ducks/conversations.ts
View File

@ -63,7 +63,7 @@ import {
getMe,
getUsernameSaveState,
} from '../selectors/conversations';
import type { AvatarDataType } from '../../types/Avatar';
import type { AvatarDataType, AvatarUpdateType } from '../../types/Avatar';
import { getDefaultAvatars } from '../../types/Avatar';
import { getAvatarData } from '../../util/getAvatarData';
import { isSameAvatarData } from '../../util/isSameAvatarData';
@ -121,6 +121,7 @@ export type ConversationType = {
avatars?: Array<AvatarDataType>;
avatarPath?: string;
avatarHash?: string;
profileAvatarPath?: string;
unblurredAvatarPath?: string;
areWeAdmin?: boolean;
areWePending?: boolean;
@ -1094,7 +1095,7 @@ function saveUsername({
function myProfileChanged(
profileData: ProfileDataType,
avatarBuffer?: Uint8Array
avatar: AvatarUpdateType
): ThunkAction<
void,
RootStateType,
@ -1110,7 +1111,7 @@ function myProfileChanged(
...conversation,
...profileData,
},
avatarBuffer
avatar
);
// writeProfile above updates the backbone model which in turn updates

4
ts/state/smart/ProfileEditorModal.ts
View File

@ -17,7 +17,7 @@ function mapStateToProps(
): Omit<PropsDataType, 'onEditStateChange' | 'onProfileChanged'> &
ProfileEditorModalPropsType {
const {
avatarPath,
profileAvatarPath,
avatars: userAvatarData = [],
aboutText,
aboutEmoji,
@ -34,7 +34,7 @@ function mapStateToProps(
return {
aboutEmoji,
aboutText,
avatarPath,
profileAvatarPath,
color,
conversationId,
familyName,

35
ts/test-electron/util/encryptProfileData_test.ts
View File

@ -10,13 +10,17 @@ import {
decryptProfileName,
decryptProfile,
} from '../../Crypto';
import type { ConversationType } from '../../state/ducks/conversations';
import { UUID } from '../../types/UUID';
import { encryptProfileData } from '../../util/encryptProfileData';
describe('encryptProfileData', () => {
it('encrypts and decrypts properly', async () => {
const keyBuffer = getRandomBytes(32);
const conversation = {
let keyBuffer: Uint8Array;
let conversation: ConversationType;
beforeEach(() => {
keyBuffer = getRandomBytes(32);
conversation = {
aboutEmoji: '🐢',
aboutText: 'I like turtles',
familyName: 'Kid',
@ -33,8 +37,13 @@ describe('encryptProfileData', () => {
title: '',
type: 'direct' as const,
};
});
const [encrypted] = await encryptProfileData(conversation);
it('encrypts and decrypts properly', async () => {
const [encrypted] = await encryptProfileData(conversation, {
oldAvatar: undefined,
newAvatar: undefined,
});
assert.isDefined(encrypted.version);
assert.isDefined(encrypted.name);
@ -83,4 +92,22 @@ describe('encryptProfileData', () => {
assert.isDefined(encrypted.aboutEmoji);
}
});
it('sets sameAvatar to true when avatars are the same', async () => {
const [encrypted] = await encryptProfileData(conversation, {
oldAvatar: new Uint8Array([1, 2, 3]),
newAvatar: new Uint8Array([1, 2, 3]),
});
assert.isTrue(encrypted.sameAvatar);
});
it('sets sameAvatar to false when avatars are different', async () => {
const [encrypted] = await encryptProfileData(conversation, {
oldAvatar: new Uint8Array([1, 2, 3]),
newAvatar: new Uint8Array([4, 5, 6, 7]),
});
assert.isFalse(encrypted.sameAvatar);
});
});

1
ts/textsecure/WebAPI.ts
View File

@ -701,6 +701,7 @@ export type ProfileRequestDataType = {
about: string | null;
aboutEmoji: string | null;
avatar: boolean;
sameAvatar: boolean;
commitment: string;
name: string;
paymentAddress: string | null;

5
ts/types/Avatar.ts
View File

@ -65,6 +65,11 @@ export type SaveAvatarToDiskActionType = (
conversationId?: string
) => unknown;
export type AvatarUpdateType = Readonly<{
oldAvatar: Uint8Array | undefined;
newAvatar: Uint8Array | undefined;
}>;
const groupIconColors = [
'A180',
'A120',

2
ts/types/Storage.d.ts vendored
View File

@ -107,7 +107,7 @@ export type StorageAccessType = {
typingIndicators: boolean;
sealedSenderIndicators: boolean;
storageFetchComplete: boolean;
avatarUrl: string;
avatarUrl: string | undefined;
manifestVersion: number;
storageCredentials: StorageServiceCredentials;
'storage-service-error-records': Array<UnknownRecord>;

12
ts/util/encryptProfileData.ts
View File

@ -10,11 +10,12 @@ import {
encryptProfile,
encryptProfileItemWithPadding,
} from '../Crypto';
import type { AvatarUpdateType } from '../types/Avatar';
import { deriveProfileKeyCommitment, deriveProfileKeyVersion } from './zkgroup';
export async function encryptProfileData(
conversation: ConversationType,
avatarBuffer?: Uint8Array
{ oldAvatar, newAvatar }: AvatarUpdateType
): Promise<[ProfileRequestDataType, Uint8Array | undefined]> {
const {
aboutEmoji,
@ -55,10 +56,12 @@ export async function encryptProfileData(
)
: null;
const encryptedAvatarData = avatarBuffer
? encryptProfile(avatarBuffer, keyBuffer)
const encryptedAvatarData = newAvatar
? encryptProfile(newAvatar, keyBuffer)
: undefined;
const sameAvatar = Bytes.areEqual(oldAvatar, newAvatar);
const profileData = {
version: deriveProfileKeyVersion(profileKey, uuid),
name: Bytes.toBase64(bytesName),
@ -66,7 +69,8 @@ export async function encryptProfileData(
aboutEmoji: bytesAboutEmoji ? Bytes.toBase64(bytesAboutEmoji) : null,
badgeIds: (badges || []).map(({ id }) => id),
paymentAddress: window.storage.get('paymentAddress') || null,
avatar: Boolean(avatarBuffer),
avatar: Boolean(newAvatar),
sameAvatar,
commitment: deriveProfileKeyCommitment(profileKey, uuid),
};

9
ts/util/lint/exceptions.json
View File

@ -7226,13 +7226,6 @@
"reasonCategory": "falseMatch",
"updated": "2021-05-05T23:11:22.692Z"
},
{
"rule": "React-useRef",
"path": "ts/components/AvatarPreview.tsx",
"line": " const startingAvatarPathRef = useRef<undefined | string>(",
"reasonCategory": "usageTrusted",
"updated": "2021-08-03T21:17:38.615Z"
},
{
"rule": "React-useRef",
"path": "ts/components/AvatarTextEditor.tsx",
@ -8079,4 +8072,4 @@
"reasonCategory": "usageTrusted",
"updated": "2021-09-17T21:02:59.414Z"
}
]
]