Remove obsolete capabilities, improve routine profile fetch

This commit is contained in:
Scott Nonnenberg 2023-08-07 16:12:57 -07:00 committed by GitHub
parent 4ba3a7856c
commit 3299b8f323
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 147 additions and 212 deletions

View file

@ -1850,12 +1850,6 @@ export async function startApp(): Promise<void> {
// Note: we always have to register our capabilities all at once, so we do this // Note: we always have to register our capabilities all at once, so we do this
// after connect on every startup // after connect on every startup
await server.registerCapabilities({ await server.registerCapabilities({
announcementGroup: true,
giftBadges: true,
'gv2-3': true,
senderKey: true,
changeNumber: true,
stories: true,
pni: isPnpEnabled(), pni: isPnpEnabled(),
}); });
} catch (error) { } catch (error) {

View file

@ -3633,7 +3633,6 @@ export class ConversationModel extends window.Backbone
getRecipients({ getRecipients({
includePendingMembers, includePendingMembers,
extraConversationsForSend, extraConversationsForSend,
isStoryReply = false,
}: { }: {
includePendingMembers?: boolean; includePendingMembers?: boolean;
extraConversationsForSend?: ReadonlyArray<string>; extraConversationsForSend?: ReadonlyArray<string>;
@ -3642,7 +3641,6 @@ export class ConversationModel extends window.Backbone
return getRecipients(this.attributes, { return getRecipients(this.attributes, {
includePendingMembers, includePendingMembers,
extraConversationsForSend, extraConversationsForSend,
isStoryReply,
}); });
} }

View file

@ -7,7 +7,6 @@ import PQueue from 'p-queue';
import * as log from './logging/log'; import * as log from './logging/log';
import { assertDev } from './util/assert'; import { assertDev } from './util/assert';
import { sleep } from './util/sleep'; import { sleep } from './util/sleep';
import { missingCaseError } from './util/missingCaseError';
import { isNormalNumber } from './util/isNormalNumber'; import { isNormalNumber } from './util/isNormalNumber';
import { take } from './util/iterables'; import { take } from './util/iterables';
import type { ConversationModel } from './models/conversations'; import type { ConversationModel } from './models/conversations';
@ -15,13 +14,13 @@ import type { StorageInterface } from './types/Storage.d';
import * as Errors from './types/errors'; import * as Errors from './types/errors';
import { getProfile } from './util/getProfile'; import { getProfile } from './util/getProfile';
import { drop } from './util/drop'; import { drop } from './util/drop';
import { MINUTE, HOUR, DAY, WEEK, MONTH } from './util/durations'; import { MINUTE, HOUR, DAY, WEEK } from './util/durations';
import { isDirectConversation } from './util/whatTypeOfConversation';
const STORAGE_KEY = 'lastAttemptedToRefreshProfilesAt'; const STORAGE_KEY = 'lastAttemptedToRefreshProfilesAt';
const MAX_AGE_TO_BE_CONSIDERED_ACTIVE = MONTH; const MAX_AGE_TO_BE_CONSIDERED_RECENTLY_REFRESHED = 3 * DAY;
const MAX_AGE_TO_BE_CONSIDERED_RECENTLY_REFRESHED = DAY;
const MAX_CONVERSATIONS_TO_REFRESH = 50; const MAX_CONVERSATIONS_TO_REFRESH = 50;
const MIN_ELAPSED_DURATION_TO_REFRESH_AGAIN = 12 * HOUR; const MIN_ELAPSED_DURATION_TO_REFRESH_AGAIN = HOUR;
const MIN_REFRESH_DELAY = MINUTE; const MIN_REFRESH_DELAY = MINUTE;
let idCounter = 1; let idCounter = 1;
@ -200,60 +199,28 @@ function* getFilteredConversations(
conversations: ReadonlyArray<ConversationModel>, conversations: ReadonlyArray<ConversationModel>,
ourConversationId: string ourConversationId: string
): Iterable<ConversationModel> { ): Iterable<ConversationModel> {
const sorted = sortBy(conversations, c => c.get('active_at')); const filtered = conversations.filter(
c =>
const conversationIdsSeen = new Set<string>([ourConversationId]); isDirectConversation(c.attributes) &&
!c.isUnregisteredAndStale() &&
c.get('uuid')
);
const sorted = sortBy(filtered, c => c.get('profileLastFetchedAt') || 0);
for (const conversation of sorted) { for (const conversation of sorted) {
const type = conversation.get('type'); if (conversation.id === ourConversationId) {
switch (type) { if (conversation.hasProfileKeyCredentialExpired()) {
case 'private': yield conversation;
if ( }
conversation.hasProfileKeyCredentialExpired() && continue;
(conversation.id === ourConversationId || }
!conversationIdsSeen.has(conversation.id))
) {
conversationIdsSeen.add(conversation.id);
yield conversation;
break;
}
if ( if (!hasRefreshedProfileRecently(conversation)) {
!conversationIdsSeen.has(conversation.id) && yield conversation;
isConversationActive(conversation) &&
!hasRefreshedProfileRecently(conversation)
) {
conversationIdsSeen.add(conversation.id);
yield conversation;
}
break;
case 'group':
for (const member of conversation.getMembers()) {
if (
!conversationIdsSeen.has(member.id) &&
!hasRefreshedProfileRecently(member)
) {
conversationIdsSeen.add(member.id);
yield member;
}
}
break;
default:
throw missingCaseError(type);
} }
} }
} }
function isConversationActive(
conversation: Readonly<ConversationModel>
): boolean {
const activeAt = conversation.get('active_at');
return (
isNormalNumber(activeAt) &&
activeAt + MAX_AGE_TO_BE_CONSIDERED_ACTIVE > Date.now()
);
}
function hasRefreshedProfileRecently( function hasRefreshedProfileRecently(
conversation: Readonly<ConversationModel> conversation: Readonly<ConversationModel>
): boolean { ): boolean {

View file

@ -8,20 +8,36 @@ import { isConversationUnregistered } from '../../util/isConversationUnregistere
describe('isConversationUnregistered', () => { describe('isConversationUnregistered', () => {
it('returns false if passed an undefined discoveredUnregisteredAt', () => { it('returns false if passed an undefined discoveredUnregisteredAt', () => {
assert.isFalse(isConversationUnregistered({})); assert.isFalse(isConversationUnregistered({ uuid: 'uuid' }));
assert.isFalse( assert.isFalse(
isConversationUnregistered({ discoveredUnregisteredAt: undefined }) isConversationUnregistered({
uuid: 'uuid',
discoveredUnregisteredAt: undefined,
})
);
});
it('returns true if uuid is falsey', () => {
assert.isTrue(
isConversationUnregistered({
uuid: undefined,
discoveredUnregisteredAt: Date.now() + 123,
})
); );
}); });
it('returns true if passed a time fewer than 6 hours ago', () => { it('returns true if passed a time fewer than 6 hours ago', () => {
assert.isTrue( assert.isTrue(
isConversationUnregistered({ discoveredUnregisteredAt: Date.now() }) isConversationUnregistered({
uuid: 'uuid',
discoveredUnregisteredAt: Date.now(),
})
); );
const fiveHours = 1000 * 60 * 60 * 5; const fiveHours = 1000 * 60 * 60 * 5;
assert.isTrue( assert.isTrue(
isConversationUnregistered({ isConversationUnregistered({
uuid: 'uuid',
discoveredUnregisteredAt: Date.now() - fiveHours, discoveredUnregisteredAt: Date.now() - fiveHours,
}) })
); );
@ -29,19 +45,24 @@ describe('isConversationUnregistered', () => {
it('returns true if passed a time in the future', () => { it('returns true if passed a time in the future', () => {
assert.isTrue( assert.isTrue(
isConversationUnregistered({ discoveredUnregisteredAt: Date.now() + 123 }) isConversationUnregistered({
uuid: 'uuid',
discoveredUnregisteredAt: Date.now() + 123,
})
); );
}); });
it('returns false if passed a time more than 6 hours ago', () => { it('returns false if passed a time more than 6 hours ago', () => {
assert.isFalse( assert.isFalse(
isConversationUnregistered({ isConversationUnregistered({
uuid: 'uuid',
discoveredUnregisteredAt: discoveredUnregisteredAt:
Date.now() - 6 * durations.HOUR - durations.MINUTE, Date.now() - 6 * durations.HOUR - durations.MINUTE,
}) })
); );
assert.isFalse( assert.isFalse(
isConversationUnregistered({ isConversationUnregistered({
uuid: 'uuid',
discoveredUnregisteredAt: new Date(1999, 3, 20).getTime(), discoveredUnregisteredAt: new Date(1999, 3, 20).getTime(),
}) })
); );

View file

@ -6,7 +6,7 @@ import { times } from 'lodash';
import { ConversationModel } from '../models/conversations'; import { ConversationModel } from '../models/conversations';
import type { ConversationAttributesType } from '../model-types.d'; import type { ConversationAttributesType } from '../model-types.d';
import { UUID } from '../types/UUID'; import { UUID } from '../types/UUID';
import { DAY } from '../util/durations'; import { DAY, HOUR, MINUTE, MONTH } from '../util/durations';
import { routineProfileRefresh } from '../routineProfileRefresh'; import { routineProfileRefresh } from '../routineProfileRefresh';
@ -79,10 +79,10 @@ describe('routineProfileRefresh', () => {
}; };
} }
it('does nothing when the last refresh time is less than 12 hours ago', async () => { it('does nothing when the last refresh time is less one hour', async () => {
const conversation1 = makeConversation(); const conversation1 = makeConversation();
const conversation2 = makeConversation(); const conversation2 = makeConversation();
const storage = makeStorage(Date.now() - 1234); const storage = makeStorage(Date.now() - 47 * MINUTE);
await routineProfileRefresh({ await routineProfileRefresh({
allConversations: [conversation1, conversation2], allConversations: [conversation1, conversation2],
@ -120,15 +120,17 @@ describe('routineProfileRefresh', () => {
); );
}); });
it("skips conversations that haven't been active in 30 days", async () => { it('skips unregistered conversations and those fetched in the last three days', async () => {
const recentlyActive = makeConversation(); const normal = makeConversation();
const inactive = makeConversation({ const recentlyFetched = makeConversation({
active_at: Date.now() - 31 * 24 * 60 * 60 * 1000, profileLastFetchedAt: Date.now() - DAY * 2 - HOUR * 3,
});
const unregisteredAndStale = makeConversation({
firstUnregisteredAt: Date.now() - 2 * MONTH,
}); });
const neverActive = makeConversation({ active_at: undefined });
await routineProfileRefresh({ await routineProfileRefresh({
allConversations: [recentlyActive, inactive, neverActive], allConversations: [normal, recentlyFetched, unregisteredAndStale],
ourConversationId: UUID.generate().toString(), ourConversationId: UUID.generate().toString(),
storage: makeStorage(), storage: makeStorage(),
getProfileFn, getProfileFn,
@ -138,18 +140,18 @@ describe('routineProfileRefresh', () => {
sinon.assert.calledOnce(getProfileFn); sinon.assert.calledOnce(getProfileFn);
sinon.assert.calledWith( sinon.assert.calledWith(
getProfileFn, getProfileFn,
recentlyActive.get('uuid'), normal.get('uuid'),
recentlyActive.get('e164') normal.get('e164')
); );
sinon.assert.neverCalledWith( sinon.assert.neverCalledWith(
getProfileFn, getProfileFn,
inactive.get('uuid'), recentlyFetched.get('uuid'),
inactive.get('e164') recentlyFetched.get('e164')
); );
sinon.assert.neverCalledWith( sinon.assert.neverCalledWith(
getProfileFn, getProfileFn,
neverActive.get('uuid'), unregisteredAndStale.get('uuid'),
neverActive.get('e164') unregisteredAndStale.get('e164')
); );
}); });
@ -169,21 +171,56 @@ describe('routineProfileRefresh', () => {
sinon.assert.neverCalledWith(getProfileFn, me.get('uuid'), me.get('e164')); sinon.assert.neverCalledWith(getProfileFn, me.get('uuid'), me.get('e164'));
}); });
it('skips conversations that were refreshed in the last hour', async () => { it('includes your own conversation if profileKeyCredential is expired', async () => {
const neverRefreshed = makeConversation(); const notMe = makeConversation();
const recentlyFetched = makeConversation({ const me = makeConversation({
profileLastFetchedAt: Date.now() - 59 * 60 * 1000, profileKey: 'fakeProfileKey',
profileKeyCredential: undefined,
profileKeyCredentialExpiration: undefined,
}); });
await routineProfileRefresh({ await routineProfileRefresh({
allConversations: [neverRefreshed, recentlyFetched], allConversations: [notMe, me],
ourConversationId: me.id,
storage: makeStorage(),
getProfileFn,
id: 1,
});
sinon.assert.calledWith(getProfileFn, notMe.get('uuid'), notMe.get('e164'));
sinon.assert.calledWith(getProfileFn, me.get('uuid'), me.get('e164'));
});
it('skips conversations that were refreshed in last three days', async () => {
const neverRefreshed = makeConversation();
const refreshedToday = makeConversation({
profileLastFetchedAt: Date.now() - HOUR * 5,
});
const refreshedYesterday = makeConversation({
profileLastFetchedAt: Date.now() - DAY,
});
const refreshedTwoDaysAgo = makeConversation({
profileLastFetchedAt: Date.now() - DAY * 2,
});
const refreshedThreeDaysAgo = makeConversation({
profileLastFetchedAt: Date.now() - DAY * 3 - 1,
});
await routineProfileRefresh({
allConversations: [
neverRefreshed,
refreshedToday,
refreshedYesterday,
refreshedTwoDaysAgo,
refreshedThreeDaysAgo,
],
ourConversationId: UUID.generate().toString(), ourConversationId: UUID.generate().toString(),
storage: makeStorage(), storage: makeStorage(),
getProfileFn, getProfileFn,
id: 1, id: 1,
}); });
sinon.assert.calledOnce(getProfileFn); sinon.assert.calledTwice(getProfileFn);
sinon.assert.calledWith( sinon.assert.calledWith(
getProfileFn, getProfileFn,
neverRefreshed.get('uuid'), neverRefreshed.get('uuid'),
@ -191,105 +228,55 @@ describe('routineProfileRefresh', () => {
); );
sinon.assert.neverCalledWith( sinon.assert.neverCalledWith(
getProfileFn, getProfileFn,
recentlyFetched.get('uuid'), refreshedToday.get('uuid'),
recentlyFetched.get('e164') refreshedToday.get('e164')
);
});
it('"digs into" the members of an active group', async () => {
const privateConversation = makeConversation();
const recentlyActiveGroupMember = makeConversation();
const inactiveGroupMember = makeConversation({
active_at: Date.now() - 31 * 24 * 60 * 60 * 1000,
});
const memberWhoHasRecentlyRefreshed = makeConversation({
profileLastFetchedAt: Date.now() - 59 * 60 * 1000,
});
const groupConversation = makeGroup([
recentlyActiveGroupMember,
inactiveGroupMember,
memberWhoHasRecentlyRefreshed,
]);
await routineProfileRefresh({
allConversations: [
privateConversation,
recentlyActiveGroupMember,
inactiveGroupMember,
memberWhoHasRecentlyRefreshed,
groupConversation,
],
ourConversationId: UUID.generate().toString(),
storage: makeStorage(),
getProfileFn,
id: 1,
});
sinon.assert.calledWith(
getProfileFn,
privateConversation.get('uuid'),
privateConversation.get('e164')
);
sinon.assert.calledWith(
getProfileFn,
recentlyActiveGroupMember.get('uuid'),
recentlyActiveGroupMember.get('e164')
);
sinon.assert.calledWith(
getProfileFn,
inactiveGroupMember.get('uuid'),
inactiveGroupMember.get('e164')
); );
sinon.assert.neverCalledWith( sinon.assert.neverCalledWith(
getProfileFn, getProfileFn,
memberWhoHasRecentlyRefreshed.get('uuid'), refreshedYesterday.get('uuid'),
memberWhoHasRecentlyRefreshed.get('e164') refreshedYesterday.get('e164')
); );
sinon.assert.neverCalledWith( sinon.assert.neverCalledWith(
getProfileFn, getProfileFn,
groupConversation.get('uuid'), refreshedTwoDaysAgo.get('uuid'),
groupConversation.get('e164') refreshedTwoDaysAgo.get('e164')
);
sinon.assert.calledWith(
getProfileFn,
refreshedThreeDaysAgo.get('uuid'),
refreshedThreeDaysAgo.get('e164')
); );
}); });
it('only refreshes profiles for the 50 most recently active direct conversations', async () => { it('only refreshes profiles for the 50 conversations with the oldest profileLastFetchedAt', async () => {
const me = makeConversation(); const me = makeConversation();
const activeConversations = times(40, () => makeConversation()); const normalConversations = times(25, () => makeConversation());
const neverFetched = times(10, () =>
const inactiveGroupMembers = times(10, () =>
makeConversation({ makeConversation({
active_at: Date.now() - 999 * 24 * 60 * 60 * 1000, profileLastFetchedAt: undefined,
})
);
const unregisteredUsers = times(10, () =>
makeConversation({
firstUnregisteredAt: Date.now() - MONTH * 2,
}) })
); );
const recentlyActiveGroup = makeGroup(inactiveGroupMembers);
const shouldNotBeIncluded = [ const shouldNotBeIncluded = [
// Recently-active groups with no other members // Recently-active groups with no other members
makeGroup([]), makeGroup([]),
makeGroup([me]), makeGroup([me]),
// Old direct conversations ...unregisteredUsers,
...times(3, () =>
makeConversation({
active_at: Date.now() - 365 * 24 * 60 * 60 * 1000,
})
),
// Old groups
...times(3, () => makeGroup(inactiveGroupMembers)),
]; ];
await routineProfileRefresh({ await routineProfileRefresh({
allConversations: [ allConversations: [
me, me,
...activeConversations, ...unregisteredUsers,
...normalConversations,
recentlyActiveGroup, ...neverFetched,
...inactiveGroupMembers,
...shouldNotBeIncluded,
], ],
ourConversationId: me.id, ourConversationId: me.id,
storage: makeStorage(), storage: makeStorage(),
@ -297,7 +284,7 @@ describe('routineProfileRefresh', () => {
id: 1, id: 1,
}); });
[...activeConversations, ...inactiveGroupMembers].forEach(conversation => { [...normalConversations, ...neverFetched].forEach(conversation => {
sinon.assert.calledWith( sinon.assert.calledWith(
getProfileFn, getProfileFn,
conversation.get('uuid'), conversation.get('uuid'),

View file

@ -623,21 +623,9 @@ export type WebAPIConnectType = {
}; };
export type CapabilitiesType = { export type CapabilitiesType = {
announcementGroup: boolean;
giftBadges: boolean;
senderKey: boolean;
changeNumber: boolean;
stories: boolean;
pni: boolean; pni: boolean;
}; };
export type CapabilitiesUploadType = { export type CapabilitiesUploadType = {
announcementGroup: true;
giftBadges: true;
'gv2-3': true;
senderKey: true;
changeNumber: true;
stories: true;
// true in staging, false in production // true in staging, false in production
pni: boolean; pni: boolean;
}; };
@ -2073,12 +2061,6 @@ export function initialize({
accessKey, accessKey,
}: ConfirmCodeOptionsType) { }: ConfirmCodeOptionsType) {
const capabilities: CapabilitiesUploadType = { const capabilities: CapabilitiesUploadType = {
announcementGroup: true,
giftBadges: true,
'gv2-3': true,
senderKey: true,
changeNumber: true,
stories: true,
pni: isPnpEnabled(), pni: isPnpEnabled(),
}; };

View file

@ -15,7 +15,6 @@ export function getRecipients(
{ {
includePendingMembers, includePendingMembers,
extraConversationsForSend, extraConversationsForSend,
isStoryReply = false,
}: { }: {
includePendingMembers?: boolean; includePendingMembers?: boolean;
extraConversationsForSend?: ReadonlyArray<string>; extraConversationsForSend?: ReadonlyArray<string>;
@ -27,14 +26,10 @@ export function getRecipients(
return [getSendTarget(conversationAttributes)!]; return [getSendTarget(conversationAttributes)!];
} }
let members = getConversationMembers(conversationAttributes, { const members = getConversationMembers(conversationAttributes, {
includePendingMembers, includePendingMembers,
}); });
if (isStoryReply) {
members = members.filter(({ capabilities }) => capabilities?.stories);
}
// There are cases where we need to send to someone we just removed from the group, to // There are cases where we need to send to someone we just removed from the group, to
// let them know that we removed them. In that case, we need to send to more than // let them know that we removed them. In that case, we need to send to more than
// are currently in the group. // are currently in the group.

View file

@ -246,17 +246,10 @@ export async function onDecryptionError(
senderUuid, senderUuid,
'private' 'private'
); );
if (!conversation.get('capabilities')?.senderKey) {
await conversation.getProfiles();
}
const name = conversation.getTitle(); const name = conversation.getTitle();
maybeShowDecryptionToast(logId, name, senderDevice); maybeShowDecryptionToast(logId, name, senderDevice);
if ( if (RemoteConfig.isEnabled('desktop.senderKey.retry')) {
conversation.get('capabilities')?.senderKey &&
RemoteConfig.isEnabled('desktop.senderKey.retry')
) {
await requestResend(decryptionError); await requestResend(decryptionError);
} else { } else {
await startAutomaticSessionReset(decryptionError); await startAutomaticSessionReset(decryptionError);

View file

@ -7,23 +7,33 @@ import { HOUR, MONTH } from './durations';
const SIX_HOURS = 6 * HOUR; const SIX_HOURS = 6 * HOUR;
export function isConversationEverUnregistered({ export function isConversationEverUnregistered({
uuid,
discoveredUnregisteredAt, discoveredUnregisteredAt,
}: Readonly<{ discoveredUnregisteredAt?: number }>): boolean { }: Readonly<{ uuid?: string; discoveredUnregisteredAt?: number }>): boolean {
return discoveredUnregisteredAt !== undefined; return !uuid || discoveredUnregisteredAt !== undefined;
} }
export function isConversationUnregistered({ export function isConversationUnregistered({
uuid,
discoveredUnregisteredAt, discoveredUnregisteredAt,
}: Readonly<{ discoveredUnregisteredAt?: number }>): boolean { }: Readonly<{ uuid?: string; discoveredUnregisteredAt?: number }>): boolean {
return Boolean( return (
discoveredUnregisteredAt && !uuid ||
isMoreRecentThan(discoveredUnregisteredAt, SIX_HOURS) Boolean(
discoveredUnregisteredAt &&
isMoreRecentThan(discoveredUnregisteredAt, SIX_HOURS)
)
); );
} }
export function isConversationUnregisteredAndStale({ export function isConversationUnregisteredAndStale({
uuid,
firstUnregisteredAt, firstUnregisteredAt,
}: Readonly<{ firstUnregisteredAt?: number }>): boolean { }: Readonly<{ uuid?: string; firstUnregisteredAt?: number }>): boolean {
if (!uuid) {
return true;
}
return Boolean( return Boolean(
firstUnregisteredAt && isOlderThan(firstUnregisteredAt, MONTH) firstUnregisteredAt && isOlderThan(firstUnregisteredAt, MONTH)
); );

View file

@ -99,9 +99,7 @@ export async function sendEditedMessage(
const fromId = ourConversation.id; const fromId = ourConversation.id;
const recipientMaybeConversations = map( const recipientMaybeConversations = map(
conversation.getRecipients({ conversation.getRecipients(),
isStoryReply: false,
}),
identifier => window.ConversationController.get(identifier) identifier => window.ConversationController.get(identifier)
); );
const recipientConversations = filter(recipientMaybeConversations, isNotNil); const recipientConversations = filter(recipientMaybeConversations, isNotNil);

View file

@ -198,14 +198,9 @@ export async function sendContentMessageToGroup({
'sendContentMessageToGroup: textsecure.messaging not available!' 'sendContentMessageToGroup: textsecure.messaging not available!'
); );
const ourConversationId =
window.ConversationController.getOurConversationIdOrThrow();
const ourConversation = window.ConversationController.get(ourConversationId);
if ( if (
isEnabled('desktop.sendSenderKey3') && isEnabled('desktop.sendSenderKey3') &&
isEnabled('desktop.senderKey.send') && isEnabled('desktop.senderKey.send') &&
ourConversation?.get('capabilities')?.senderKey &&
sendTarget.isValid() sendTarget.isValid()
) { ) {
try { try {
@ -1148,11 +1143,6 @@ function isValidSenderKeyRecipient(
return false; return false;
} }
const capabilities = memberConversation.get('capabilities');
if (!capabilities?.senderKey) {
return false;
}
if (!getAccessKey(memberConversation.attributes, { story })) { if (!getAccessKey(memberConversation.attributes, { story })) {
return false; return false;
} }