Skip to content

Commit 65f402b

Browse files
committed
fix: renew now cares about if the fs ttl was 30s
1 parent 3df4703 commit 65f402b

File tree

10 files changed

+106
-56
lines changed

10 files changed

+106
-56
lines changed

ts/interactions/avatar-interactions/nts-avatar-interactions.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
UserConfigWrapperActions,
1111
} from '../../webworker/workers/browser/libsession_worker_interface';
1212
import { UserUtils } from '../../session/utils';
13-
import { fromBase64ToArray } from '../../session/utils/String';
13+
import { fromHexToArray } from '../../session/utils/String';
1414

1515
export async function uploadAndSetOurAvatarShared({
1616
decryptedAvatarData,
@@ -43,10 +43,10 @@ export async function uploadAndSetOurAvatarShared({
4343
encryptionKey = encryptedContent.encryptionKey;
4444
} else {
4545
// if this is a reupload, reuse the current profile key. Otherwise generate a new one
46-
const existingProfileKey = ourConvo.getProfileKey();
46+
const existingProfileKeyHex = ourConvo.getProfileKeyHex();
4747
const profileKey =
48-
context === 'reuploadAvatar' && existingProfileKey
49-
? fromBase64ToArray(existingProfileKey)
48+
context === 'reuploadAvatar' && existingProfileKeyHex
49+
? fromHexToArray(existingProfileKeyHex)
5050
: randombytes_buf(32);
5151
encryptedData = await encryptProfile(mainAvatarDetails.outputBuffer, profileKey);
5252
encryptionKey = profileKey;
@@ -103,6 +103,6 @@ export async function uploadAndSetOurAvatarShared({
103103

104104
return {
105105
avatarPointer: ourConvo.getAvatarPointer(),
106-
profileKey: ourConvo.getProfileKey(),
106+
profileKey: ourConvo.getProfileKeyHex(),
107107
};
108108
}

ts/models/conversation.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -215,16 +215,9 @@ type SetSessionProfileReturn = {
215215
* We need to do some extra processing for private actions, as they have a updatedAtSeconds field.
216216
*/
217217
function isSetProfileWithUpdatedAtSeconds<T extends SetSessionProfileDetails>(
218-
_action: T
219-
): _action is Extract<T, { profileUpdatedAtSeconds: number }> {
220-
/**
221-
* We temporarily want to not write the profileUpdatedAtSeconds as we want this behavior to
222-
* be used only once a user has updated their profile picture (and resized it).
223-
*/
224-
window.log.debug('isSetProfileWithUpdatedAtSeconds forced to return false for now');
225-
return false;
226-
227-
// return 'profileUpdatedAtSeconds' in action;
218+
action: T
219+
): action is Extract<T, { profileUpdatedAtSeconds: number }> {
220+
return 'profileUpdatedAtSeconds' in action;
228221
}
229222

230223
/**
@@ -1545,7 +1538,7 @@ export class ConversationModel extends Model<ConversationAttributes> {
15451538
this.getAvatarInProfilePath() ||
15461539
this.getFallbackAvatarInProfilePath() ||
15471540
this.getAvatarPointer() ||
1548-
this.getProfileKey()
1541+
this.getProfileKeyHex()
15491542
) {
15501543
this.set({
15511544
avatarInProfile: undefined,
@@ -1573,7 +1566,7 @@ export class ConversationModel extends Model<ConversationAttributes> {
15731566
: to_hex(newProfile.profileKey);
15741567

15751568
const existingAvatarPointer = this.getAvatarPointer();
1576-
const existingProfileKeyHex = this.getProfileKey();
1569+
const existingProfileKeyHex = this.getProfileKeyHex();
15771570
const hasAvatarInNewProfile = !!newProfile.avatarPointer || !!newProfileKeyHex;
15781571
// if no changes are needed, return early
15791572
if (
@@ -1603,7 +1596,7 @@ export class ConversationModel extends Model<ConversationAttributes> {
16031596
: to_hex(newProfile.profileKey);
16041597

16051598
const existingAvatarPointer = this.getAvatarPointer();
1606-
const existingProfileKeyHex = this.getProfileKey();
1599+
const existingProfileKeyHex = this.getProfileKeyHex();
16071600
const originalAvatar = this.getAvatarInProfilePath();
16081601
const originalFallbackAvatar = this.getFallbackAvatarInProfilePath();
16091602

@@ -1730,7 +1723,7 @@ export class ConversationModel extends Model<ConversationAttributes> {
17301723
* Returns the profile key attributes of this instance.
17311724
* If the attribute is unset, empty, or not a string, returns `undefined`.
17321725
*/
1733-
public getProfileKey(): string | undefined {
1726+
public getProfileKeyHex(): string | undefined {
17341727
const profileKey = this.get('profileKey');
17351728
if (!profileKey || !isString(profileKey)) {
17361729
return undefined;
@@ -1768,11 +1761,11 @@ export class ConversationModel extends Model<ConversationAttributes> {
17681761
}
17691762
const avatarPointer = this.getAvatarPointer() ?? null;
17701763
const displayName = this.getRealSessionUsername() ?? '';
1771-
const profileKey = this.getProfileKey() ?? null;
1764+
const profileKeyHex = this.getProfileKeyHex() ?? null;
17721765
const updatedAtSeconds = this.getProfileUpdatedSeconds();
17731766

17741767
return new OutgoingUserProfile({
1775-
profilePic: { url: avatarPointer, key: profileKey ? from_hex(profileKey) : null },
1768+
profilePic: { url: avatarPointer, key: profileKeyHex ? from_hex(profileKeyHex) : null },
17761769
displayName,
17771770
updatedAtSeconds,
17781771
});

ts/session/apis/file_server_api/FileServerApi.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -197,30 +197,28 @@ export const getLatestReleaseFromFileServer = async (
197197
*
198198
*/
199199
export const extendFileExpiry = async (fileId: string, fsTarget: FILE_SERVER_TARGET_TYPE) => {
200-
// TODO: remove this once QA is done
201-
202-
if (!FS.supportsFsExtend(fsTarget)) {
203-
throw new Error('extendFileExpiry: only works with potato for now');
204-
}
205200
if (window.sessionFeatureFlags?.debugServerRequests) {
206201
window.log.info(`about to renew expiry of file: "${fileId}"`);
207202
}
208203

209204
const method = 'POST';
210205
const endpoint = `/file/${fileId}/extend`;
206+
const headers: Record<string, string> = window.sessionFeatureFlags.fsTTL30s
207+
? { 'X-FS-TTL': '30' }
208+
: {};
211209
const params = {
212210
abortSignal: new AbortController().signal,
213211
endpoint,
214212
method,
215213
stringifiedBody: null,
216-
headers: {},
214+
headers,
217215
timeoutMs: 10 * DURATION.SECONDS,
218216
target: fsTarget,
219217
};
220218

221219
const result = await OnionSending.sendJsonViaOnionV4ToFileServer(params);
222220

223-
if (!batchGlobalIsSuccess(result) || OnionV4.parseStatusCodeFromV4Request(result) !== 200) {
221+
if (!batchGlobalIsSuccess(result)) {
224222
return null;
225223
}
226224

ts/session/apis/snode_api/swarm_polling_config/SwarmPollingGroupConfig.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ async function scheduleAvatarDownloadJobIfNeeded(groupPk: GroupPubkeyType) {
279279

280280
if (!profileUrl || !profileKeyHex) {
281281
// no avatar set for this group: make sure we also remove the one we might have locally.
282-
if (conversation.getAvatarPointer() || conversation.getProfileKey()) {
282+
if (conversation.getAvatarPointer() || conversation.getProfileKeyHex()) {
283283
await conversation.setSessionProfile({
284284
type: 'resetAvatarGroup',
285285
displayName: null,
@@ -291,7 +291,7 @@ async function scheduleAvatarDownloadJobIfNeeded(groupPk: GroupPubkeyType) {
291291

292292
// here, an avatar for this group is set. First we need to make sure if that's the same as we already have
293293
const prevPointer = conversation.getAvatarPointer();
294-
const prevProfileKey = conversation.getProfileKey();
294+
const prevProfileKey = conversation.getProfileKeyHex();
295295

296296
if (prevPointer !== profileUrl || prevProfileKey !== profileKeyHex) {
297297
// set the avatar for this group, it will be downloaded by the job scheduled below

ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function shouldAddAvatarDownloadJob({ conversationId }: { conversationId:
3636
return false;
3737
}
3838
const prevPointer = conversation.getAvatarPointer();
39-
const profileKey = conversation.getProfileKey();
39+
const profileKey = conversation.getProfileKeyHex();
4040
const hasNoAvatar = isEmpty(prevPointer) || isEmpty(profileKey);
4141

4242
if (hasNoAvatar) {
@@ -111,7 +111,7 @@ class AvatarDownloadJob extends PersistedJob<AvatarDownloadPersistedData> {
111111
}
112112
let changes = false;
113113
const toDownloadPointer = conversation.getAvatarPointer();
114-
const toDownloadProfileKey = conversation.getProfileKey();
114+
const toDownloadProfileKey = conversation.getProfileKeyHex();
115115

116116
// if there is an avatar and profileKey for that user/group ('', null and undefined excluded), download, decrypt and save the avatar locally.
117117
if (toDownloadPointer && toDownloadProfileKey) {

ts/session/utils/job_runners/jobs/AvatarMigrateJob.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class AvatarMigrateJob extends PersistedJob<AvatarMigratePersistedData> {
109109
window.log.warn('AvatarMigrateJob: no avatar pointer found for conversation');
110110
return RunJobResult.Success;
111111
}
112-
const existingProfileKeyHex = conversation.getProfileKey();
112+
const existingProfileKeyHex = conversation.getProfileKeyHex();
113113
if (!existingProfileKeyHex) {
114114
window.log.warn('AvatarMigrateJob: no profileKey found for conversation');
115115
return RunJobResult.Success;

ts/session/utils/job_runners/jobs/AvatarReuploadJob.ts

Lines changed: 79 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { from_hex } from 'libsodium-wrappers-sumo';
12
import { isNumber } from 'lodash';
23
import { v4 } from 'uuid';
34
import { UserUtils } from '../..';
@@ -19,7 +20,6 @@ import { maxAvatarDetails } from '../../../../util/attachment/attachmentSizes';
1920
import { UserConfigWrapperActions } from '../../../../webworker/workers/browser/libsession_worker_interface';
2021
import { extendFileExpiry } from '../../../apis/file_server_api/FileServerApi';
2122
import { fileServerUrlToFileId } from '../../../apis/file_server_api/types';
22-
import { NetworkTime } from '../../../../util/NetworkTime';
2323
import { DURATION, DURATION_SECONDS } from '../../../constants';
2424
import { uploadAndSetOurAvatarShared } from '../../../../interactions/avatar-interactions/nts-avatar-interactions';
2525
import { FS } from '../../../apis/file_server_api/FileServerTarget';
@@ -63,6 +63,39 @@ async function fetchLocalAvatarDetails(currentMainPath: string) {
6363
}
6464
}
6565

66+
/**
67+
* Returns the current timestamp in seconds.
68+
* Note: this is not the network time, but our local time with an offset, potentially.
69+
* We want to use that one here, as the UserProfile actions are not based on the network timestamp either.k
70+
*/
71+
function nowSeconds() {
72+
return Math.floor(Date.now() / 1000);
73+
}
74+
75+
function shouldSkipRenew({
76+
ourProfileLastUpdatedSeconds,
77+
}: {
78+
ourProfileLastUpdatedSeconds: number;
79+
}) {
80+
if (window.sessionFeatureFlags.fsTTL30s) {
81+
// this is in dev
82+
return Date.now() / 1000 - ourProfileLastUpdatedSeconds <= 10 * DURATION_SECONDS.SECONDS;
83+
}
84+
// this is in prod
85+
return nowSeconds() - ourProfileLastUpdatedSeconds <= 2 * DURATION_SECONDS.HOURS;
86+
}
87+
88+
function shouldSkipReupload({
89+
ourProfileLastUpdatedSeconds,
90+
}: {
91+
ourProfileLastUpdatedSeconds: number;
92+
}) {
93+
if (window.sessionFeatureFlags.fsTTL30s) {
94+
return nowSeconds() - ourProfileLastUpdatedSeconds <= 10 * DURATION_SECONDS.SECONDS;
95+
}
96+
return nowSeconds() - ourProfileLastUpdatedSeconds <= 12 * DURATION_SECONDS.DAYS;
97+
}
98+
6699
class AvatarReuploadJob extends PersistedJob<AvatarReuploadPersistedData> {
67100
constructor({
68101
conversationId,
@@ -91,7 +124,7 @@ class AvatarReuploadJob extends PersistedJob<AvatarReuploadPersistedData> {
91124
public async run(): Promise<RunJobResult> {
92125
const convoId = this.persistedData.conversationId;
93126
window.log.debug(
94-
`running job ${this.persistedData.jobType} id:"${this.persistedData.identifier}" `
127+
`[avatarReupload] running job ${this.persistedData.jobType} id:"${this.persistedData.identifier}" `
95128
);
96129

97130
if (!this.persistedData.identifier) {
@@ -104,14 +137,14 @@ class AvatarReuploadJob extends PersistedJob<AvatarReuploadPersistedData> {
104137
let conversation = ConvoHub.use().get(convoId);
105138
if (!conversation || !conversation.isMe()) {
106139
// Note: if we add the groupv2 case here, we'd need to add a profile_updated timestamp to the metagroup wrapper
107-
window.log.warn('AvatarReuploadJob did not find corresponding conversation, or not us');
140+
window.log.warn('[avatarReupload] did not find corresponding conversation, or not us');
108141

109142
return RunJobResult.PermanentFailure;
110143
}
111144
const ourProfileLastUpdatedSeconds = await UserConfigWrapperActions.getProfileUpdatedSeconds();
112145
const currentMainPath = conversation.getAvatarInProfilePath();
113146
const avatarPointer = conversation.getAvatarPointer();
114-
const profileKey = conversation.getProfileKey();
147+
const profileKey = conversation.getProfileKeyHex();
115148
const { fileId, fullUrl } = fileServerUrlToFileId(avatarPointer);
116149
if (!currentMainPath || !avatarPointer || !profileKey || !fullUrl) {
117150
// we do not have an avatar to reupload, nothing to do.
@@ -130,58 +163,84 @@ class AvatarReuploadJob extends PersistedJob<AvatarReuploadPersistedData> {
130163
if (
131164
ourProfileLastUpdatedSeconds !== 0 &&
132165
metadata.width <= maxAvatarDetails.maxSidePlanReupload &&
133-
metadata.height <= maxAvatarDetails.maxSidePlanReupload &&
134-
metadata.format === 'webp'
166+
metadata.height <= maxAvatarDetails.maxSidePlanReupload
135167
) {
136168
const target = FS.fileUrlToFileTarget(fullUrl?.toString());
137169
window.log.debug(
138-
`[avatarReupload] main avatar is already the right size and format for ${ed25519Str(conversation.id)}, just renewing it on fs: ${target}`
170+
`[avatarReupload] main avatar is already the right size for ${ed25519Str(conversation.id)} target:${target}`
171+
);
172+
if (shouldSkipRenew({ ourProfileLastUpdatedSeconds })) {
173+
// we don't want to call `renew` too often. Only once every 2hours (or more when the fsTTL30s feature is enabled)
174+
window.log.debug(
175+
`[avatarReupload] not trying to renew avatar for ${ed25519Str(conversation.id)} of file ${fileId} as we did one recently`
176+
);
177+
// considering this to be a success
178+
return RunJobResult.Success;
179+
}
180+
window.log.debug(
181+
`[avatarReupload] renewing avatar on fs: ${target} for ${ed25519Str(conversation.id)} and file:${fileId}`
139182
);
140183
const expiryRenewResult = await extendFileExpiry(fileId, target);
141184

142185
if (expiryRenewResult) {
143186
window.log.debug(
144-
`[avatarReupload] expiry renew for ${ed25519Str(conversation.id)} of file ${fileId} was successful`
187+
`[avatarReupload] expiry renew for ${ed25519Str(conversation.id)} of file:${fileId} on fs: ${target} was successful`
145188
);
189+
190+
await UserConfigWrapperActions.getProfilePic();
191+
192+
await UserConfigWrapperActions.setReuploadProfilePic({
193+
key: from_hex(profileKey),
194+
url: avatarPointer,
195+
});
196+
146197
return RunJobResult.Success;
147198
}
199+
window.log.debug(
200+
`[avatarReupload] expiry renew for ${ed25519Str(conversation.id)} of file:${fileId} on fs: ${target} failed`
201+
);
202+
203+
// AUDRIC: expiry renew for (...efb27b5b) of file:Ff1CvAQIo1BXCeoV3DwTjYEzSoBPZW56FeExk8qij79h on fs: POTATO failed
204+
// keep failing even whe it shouldnt
148205

149-
if (ourProfileLastUpdatedSeconds > NetworkTime.nowSeconds() - 12 * DURATION_SECONDS.DAYS) {
150-
// `renew` failed but our last reupload was less than 12 days ago, so we don't want to retry
206+
if (shouldSkipReupload({ ourProfileLastUpdatedSeconds })) {
151207
window.log.debug(
152-
`[avatarReupload] expiry renew for ${ed25519Str(conversation.id)} of file ${fileId} failed but our last reupload was less than 12 days ago, so we don't want to retry`
208+
`[avatarReupload] ${ed25519Str(conversation.id)} last reupload was recent enough, so we don't want to reupload it`
153209
);
154210
// considering this to be a success
155211
return RunJobResult.Success;
156212
}
157-
// renew failed, but our last reupload was more than 12 days ago, so we want to reprocess and
158-
// reupload our current avatar, see below
213+
// renew failed, and our last reupload was not too recent, so we want to reprocess and
214+
// reupload our current avatar, see below...
159215
}
160216

161217
// here,
162218
// - either the format or the size is wrong
163219
// - or we do not have a ourProfileLastUpdatedSeconds yet
164-
// - or the expiry renew failed and our last reupload was more than 12 days ago
220+
// - or the expiry renew failed and our last reupload not recent
165221
// In all those cases, we want to reprocess our current avatar, and reupload it.
166222

167-
window.log.info(`[profileupdate] about to auto scale avatar for convo ${conversation.id}`);
223+
window.log.info(
224+
`[avatarReupload] about to auto scale avatar for convo ${ed25519Str(conversation.id)}`
225+
);
168226

169227
conversation = ConvoHub.use().getOrThrow(convoId);
170228

171229
// Reprocess the avatar content, and reupload it
172230
// This will pick the correct file server depending on the env variables set.
173-
await uploadAndSetOurAvatarShared({
231+
const details = await uploadAndSetOurAvatarShared({
174232
decryptedAvatarData,
175233
ourConvo: conversation,
176234
context: 'reuploadAvatar',
177235
});
236+
window.log.info(
237+
`[avatarReupload] reupload done for ${ed25519Str(conversation.id)}: ${details?.avatarPointer}`
238+
);
239+
return RunJobResult.Success;
178240
} catch (e) {
179-
window.log.warn(`[profileReupload] failed with ${e.message}`);
241+
window.log.warn(`[avatarReupload] failed with ${e.message}`);
180242
return RunJobResult.RetryJobIfPossible;
181243
}
182-
183-
// return true so this job is marked as a success
184-
return RunJobResult.Success;
185244
}
186245

187246
public serializeJob(): AvatarReuploadPersistedData {

ts/session/utils/libsession/libsession_utils_contacts.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ async function insertContactFromDBIntoWrapperAndRefresh(
6262
const dbName = foundConvo.getRealSessionUsername() || undefined;
6363
const dbNickname = foundConvo.get('nickname');
6464
const dbProfileUrl = foundConvo.getAvatarPointer() || undefined;
65-
const dbProfileKey = foundConvo.getProfileKey() || undefined;
65+
const dbProfileKey = foundConvo.getProfileKeyHex() || undefined;
6666
const dbApproved = !!foundConvo.get('isApproved');
6767
const dbApprovedMe = !!foundConvo.get('didApproveMe');
6868
const dbBlocked = foundConvo.isBlocked();

ts/state/ducks/metaGroups.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,7 @@ async function handleClearAvatarFromUI({ groupPk }: WithGroupPubkey) {
11201120
isNil(convo.getAvatarPointer()) &&
11211121
isNil(convo.getAvatarInProfilePath()) &&
11221122
isNil(convo.getFallbackAvatarInProfilePath()) &&
1123-
isNil(convo.getProfileKey())
1123+
isNil(convo.getProfileKeyHex())
11241124
) {
11251125
return;
11261126
}

ts/state/ducks/user.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ const clearOurAvatar = createAsyncThunk('user/clearOurAvatar', async () => {
6969
isNil(convo.getAvatarPointer()) &&
7070
isNil(convo.getAvatarInProfilePath()) &&
7171
isNil(convo.getFallbackAvatarInProfilePath()) &&
72-
isNil(convo.getProfileKey())
72+
isNil(convo.getProfileKeyHex())
7373
) {
7474
return;
7575
}

0 commit comments

Comments
 (0)