diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index de95fc97cc21..dc43114b65bf 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -93,6 +93,10 @@ const COLORS = [ const THREE_HOURS = 3 * 60 * 60 * 1000; const FIVE_MINUTES = 1000 * 60 * 5; +const ATTRIBUTES_THAT_DONT_INVALIDATE_PROPS_CACHE = new Set([ + 'profileLastFetchedAt', +]); + type CustomError = Error & { identifier?: string; number?: string; @@ -289,6 +293,13 @@ export class ConversationModel extends window.Backbone // We clear our cached props whenever we change so that the next call to format() will // result in refresh via a getProps() call. See format() below. this.on('change', () => { + const isPropsCacheStillValid = Object.keys(this.changed).every(key => + ATTRIBUTES_THAT_DONT_INVALIDATE_PROPS_CACHE.has(key) + ); + if (isPropsCacheStillValid) { + return; + } + if (this.cachedProps) { this.oldCachedProps = this.cachedProps; } @@ -1342,6 +1353,8 @@ export class ConversationModel extends window.Backbone // maintains a cache, and protects against reentrant calls. // Note: When writing code inside this function, do not call .format() on a conversation // unless you are sure that it's not this very same conversation. + // Note: If you start relying on an attribute that is in + // `ATTRIBUTES_THAT_DONT_INVALIDATE_PROPS_CACHE`, remove it from that list. private getProps(): ConversationType { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const color = this.getColor()!; diff --git a/ts/routineProfileRefresh.ts b/ts/routineProfileRefresh.ts index aa5329ecc814..ebc8c2ae83fe 100644 --- a/ts/routineProfileRefresh.ts +++ b/ts/routineProfileRefresh.ts @@ -1,13 +1,17 @@ // Copyright 2021 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +// We use `for ... of` to deal with iterables in several places in this file. +/* eslint-disable no-restricted-syntax */ + import { isNil, sortBy } from 'lodash'; +import PQueue from 'p-queue'; import * as log from './logging/log'; import { assert } from './util/assert'; import { missingCaseError } from './util/missingCaseError'; import { isNormalNumber } from './util/isNormalNumber'; -import { map, take } from './util/iterables'; +import { take } from './util/iterables'; import { isOlderThan } from './util/timestamp'; import { ConversationModel } from './models/conversations'; @@ -52,23 +56,37 @@ export async function routineProfileRefresh({ let totalCount = 0; let successCount = 0; - await Promise.all( - map(conversationsToRefresh, async (conversation: ConversationModel) => { - totalCount += 1; - try { - await conversation.getProfile( - conversation.get('uuid'), - conversation.get('e164') - ); - successCount += 1; - } catch (err) { - window.log.error( - 'routineProfileRefresh: failed to fetch a profile', - err?.stack || err - ); - } - }) - ); + + async function refreshConversation( + conversation: ConversationModel + ): Promise { + window.log.info( + `routineProfileRefresh: refreshing profile for ${conversation.idForLogging()}` + ); + + totalCount += 1; + try { + await conversation.getProfile( + conversation.get('uuid'), + conversation.get('e164') + ); + window.log.info( + `routineProfileRefresh: refreshed profile for ${conversation.idForLogging()}` + ); + successCount += 1; + } catch (err) { + window.log.error( + `routineProfileRefresh: refreshed profile for ${conversation.idForLogging()}`, + err?.stack || err + ); + } + } + + const refreshQueue = new PQueue({ concurrency: 5, timeout: 1000 * 60 * 2 }); + for (const conversation of conversationsToRefresh) { + refreshQueue.add(() => refreshConversation(conversation)); + } + await refreshQueue.onIdle(); log.info( `routineProfileRefresh: successfully refreshed ${successCount} out of ${totalCount} conversation(s)` @@ -112,9 +130,6 @@ function* getFilteredConversations( const conversationIdsSeen = new Set([ourConversationId]); - // We use a `for` loop (instead of something like `forEach`) because we want to be able - // to yield. We use `for ... of` for readability. - // eslint-disable-next-line no-restricted-syntax for (const conversation of sorted) { const type = conversation.get('type'); switch (type) { @@ -129,7 +144,6 @@ function* getFilteredConversations( } break; case 'group': - // eslint-disable-next-line no-restricted-syntax for (const member of conversation.getMembers()) { if ( !conversationIdsSeen.has(member.id) && diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index b843011ebba4..1bb7f0e45f8f 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -1410,8 +1410,9 @@ export function reducer( let { showArchived } = state; const existing = conversationLookup[id]; - // In the change case we only modify the lookup if we already had that conversation - if (!existing) { + // We only modify the lookup if we already had that conversation and the conversation + // changed. + if (!existing || data === existing) { return state; }