diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 8b749adbaa6c..c3e65977b1ea 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -65,6 +65,7 @@ jobs: NODE_ENV: production RUN_COUNT: 10 ELECTRON_ENABLE_STACK_DUMPING: on + DEBUG: 'mock:benchmarks' ARTIFACTS_DIR: artifacts/startup - name: Run send benchmarks @@ -78,6 +79,7 @@ jobs: NODE_ENV: production RUN_COUNT: 100 ELECTRON_ENABLE_STACK_DUMPING: on + # DEBUG: 'mock:benchmarks' ARTIFACTS_DIR: artifacts/send - name: Run group send benchmarks @@ -93,6 +95,7 @@ jobs: RUN_COUNT: 100 CONVERSATION_SIZE: 500 ELECTRON_ENABLE_STACK_DUMPING: on + # DEBUG: 'mock:benchmarks' ARTIFACTS_DIR: artifacts/group-send - name: Run large group send benchmarks with delivery receipts @@ -112,6 +115,7 @@ jobs: RUN_COUNT: 20 CONVERSATION_SIZE: 50 ELECTRON_ENABLE_STACK_DUMPING: on + # DEBUG: 'mock:benchmarks' ARTIFACTS_DIR: artifacts/large-group-send - name: Run conversation open benchmarks @@ -126,6 +130,7 @@ jobs: NODE_ENV: production RUN_COUNT: 100 ELECTRON_ENABLE_STACK_DUMPING: on + # DEBUG: 'mock:benchmarks' ARTIFACTS_DIR: artifacts/convo-open - name: Upload benchmark logs on failure diff --git a/ts/groups.ts b/ts/groups.ts index bf8c0e535a69..369e9a548b65 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -751,7 +751,7 @@ export async function buildAddMembersChange( addPendingMembers.push(addPendingMemberAction); } - const doesMemberNeedUnban = conversation.bannedMembersV2?.find( + const doesMemberNeedUnban = conversation.bannedMembersV2?.some( bannedMember => bannedMember.serviceId === serviceId ); if (doesMemberNeedUnban) { @@ -1020,7 +1020,7 @@ export function _maybeBuildAddBannedMemberActions({ 'addMembersBanned' | 'deleteMembersBanned' > { const doesMemberNeedBan = - !group.bannedMembersV2?.find(member => member.serviceId === serviceId) && + !group.bannedMembersV2?.some(member => member.serviceId === serviceId) && serviceId !== ourAci; if (!doesMemberNeedBan) { return {}; @@ -1191,7 +1191,7 @@ export function buildAddMember({ actions.version = (group.revision || 0) + 1; actions.addMembers = [addMember]; - const doesMemberNeedUnban = group.bannedMembersV2?.find( + const doesMemberNeedUnban = group.bannedMembersV2?.some( member => member.serviceId === serviceId ); if (doesMemberNeedUnban) { @@ -3501,7 +3501,7 @@ async function getGroupUpdates({ const ourAci = window.storage.user.getCheckedAci(); const isInitialCreationMessage = isFirstFetch && newRevision === 0; - const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).find( + const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).some( item => item.aci === ourAci ); const isOneVersionUp = @@ -3565,18 +3565,18 @@ async function getGroupUpdates({ ); } - if ( - (!isFirstFetch || isNumber(newRevision)) && - window.Flags.GV2_ENABLE_CHANGE_PROCESSING - ) { + const areWeMember = (group.membersV2 || []).some(item => item.aci === ourAci); + const isReJoin = !isFirstFetch && !areWeMember; + + if (window.Flags.GV2_ENABLE_CHANGE_PROCESSING) { try { return await updateGroupViaLogs({ group, newRevision, }); } catch (error) { - const nextStep = isFirstFetch - ? `fetching logs since ${newRevision}` + const nextStep = isReJoin + ? 'attempting to fetch from re-join revision' : 'fetching full state'; if (error.code === TEMPORAL_AUTH_REJECTED_CODE) { @@ -3595,6 +3595,30 @@ async function getGroupUpdates({ } } + if (isReJoin && window.Flags.GV2_ENABLE_CHANGE_PROCESSING) { + try { + return await updateGroupViaLogs({ + group, + newRevision, + isReJoin, + }); + } catch (error) { + if (error.code === TEMPORAL_AUTH_REJECTED_CODE) { + // We will fail over to the updateGroupViaState call below + log.info( + `getGroupUpdates/${logId}: Temporal credential failure, now fetching full state` + ); + } else if (error.code === GROUP_ACCESS_DENIED_CODE) { + // We will fail over to the updateGroupViaState call below + log.info( + `getGroupUpdates/${logId}: Log access denied, now fetching full state` + ); + } else { + throw error; + } + } + } + if (window.Flags.GV2_ENABLE_STATE_PROCESSING) { try { return await updateGroupViaState({ @@ -3887,9 +3911,11 @@ async function updateGroupViaSingleChange({ async function updateGroupViaLogs({ group, newRevision, + isReJoin, }: { group: ConversationAttributesType; newRevision: number | undefined; + isReJoin?: boolean; }): Promise { const logId = idForLogging(group.groupId); const { publicParams, secretParams } = group; @@ -3900,15 +3926,15 @@ async function updateGroupViaLogs({ throw new Error('updateGroupViaLogs: group was missing secretParams!'); } + const currentRevision = isReJoin ? undefined : group.revision; + let includeFirstState = true; + log.info( `updateGroupViaLogs/${logId}: Getting group delta from ` + - `${group.revision ?? '?'} to ${newRevision ?? '?'} for group ` + + `${currentRevision ?? '?'} to ${newRevision ?? '?'} for group ` + `groupv2(${group.groupId})...` ); - const currentRevision = group.revision; - let includeFirstState = true; - // The range is inclusive so make sure that we always request the revision // that we are currently at since we might want the latest full state in // `integrateGroupChanges`. @@ -4217,9 +4243,13 @@ async function integrateGroupChange({ const isFirstFetch = !isNumber(group.revision); const ourAci = window.storage.user.getCheckedAci(); - const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).find( + const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).some( item => item.aci === ourAci ); + const weAreInGroup = (group.membersV2 || []).some( + item => item.aci === ourAci + ); + const isReJoin = !isFirstFetch && !weAreInGroup; // These need to be populated from the groupChange. But we might not get one! let isChangeSupported = false; @@ -4347,6 +4377,7 @@ async function integrateGroupChange({ isChangePresent: Boolean(groupChange), isChangeSupported, isFirstFetch, + isReJoin, isSameVersion, isMoreThanOneVersionUp, weAreAwaitingApproval, @@ -4366,13 +4397,14 @@ async function integrateGroupChange({ } = await applyGroupState({ group: attributes, groupState: decryptedGroupState, - sourceServiceId: isFirstFetch ? sourceServiceId : undefined, + sourceServiceId: isFirstFetch || isReJoin ? sourceServiceId : undefined, }); const groupChangeMessages = extractDiffs({ old: attributes, current: newAttributes, - sourceServiceId: isFirstFetch ? sourceServiceId : undefined, + sourceServiceId: isFirstFetch || isReJoin ? sourceServiceId : undefined, + isReJoin, }); const newProfileKeys = profileKeysToMap(newProfileKeysList); @@ -4419,15 +4451,17 @@ async function integrateGroupChange({ function extractDiffs({ current, dropInitialJoinMessage, + isReJoin, old, - sourceServiceId, promotedAciToPniMap, + sourceServiceId, }: { current: ConversationAttributesType; dropInitialJoinMessage?: boolean; + isReJoin?: boolean; old: ConversationAttributesType; - sourceServiceId?: ServiceIdString; promotedAciToPniMap?: ReadonlyMap; + sourceServiceId?: ServiceIdString; }): Array { const logId = idForLogging(old.groupId); const details: Array = []; @@ -4941,11 +4975,11 @@ function extractDiffs({ timerNotification = { ...generateBasicMessage(), type: 'timer-notification', - sourceServiceId, + sourceServiceId: isReJoin ? undefined : sourceServiceId, flags: Proto.DataMessage.Flags.EXPIRATION_TIMER_UPDATE, expirationTimerUpdate: { expireTimer, - sourceServiceId, + sourceServiceId: isReJoin ? undefined : sourceServiceId, }, }; } diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index a2eae042c730..7a8f04a2b24b 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -264,9 +264,9 @@ export class ConversationModel extends window.Backbone throttledBumpTyping?: () => void; - throttledFetchSMSOnlyUUID?: () => Promise | undefined; + throttledFetchSMSOnlyUUID?: () => Promise; - throttledMaybeMigrateV1Group?: () => Promise | undefined; + throttledMaybeMigrateV1Group?: () => Promise; throttledGetProfiles?: () => Promise; diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index a9a651ddd132..1f610cd598c9 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -4297,6 +4297,7 @@ function onConversationOpened( | SetQuotedMessageActionType > { return async (dispatch, getState) => { + const promises: Array> = []; const conversation = window.ConversationController.get(conversationId); if (!conversation) { throw new Error('onConversationOpened: Conversation not found'); @@ -4328,7 +4329,7 @@ function onConversationOpened( ); }; - drop(loadAndUpdate()); + promises.push(loadAndUpdate()); dispatch(setComposerFocus(conversation.id)); @@ -4341,17 +4342,17 @@ function onConversationOpened( ); } - drop(conversation.fetchLatestGroupV2Data()); + promises.push(conversation.fetchLatestGroupV2Data()); strictAssert( conversation.throttledMaybeMigrateV1Group !== undefined, 'Conversation model should be initialized' ); - drop(conversation.throttledMaybeMigrateV1Group()); + promises.push(conversation.throttledMaybeMigrateV1Group()); strictAssert( conversation.throttledFetchSMSOnlyUUID !== undefined, 'Conversation model should be initialized' ); - drop(conversation.throttledFetchSMSOnlyUUID()); + promises.push(conversation.throttledFetchSMSOnlyUUID()); const ourAci = window.textsecure.storage.user.getAci(); if ( @@ -4367,13 +4368,18 @@ function onConversationOpened( }); } - drop(conversation.updateVerified()); + promises.push(conversation.updateVerified()); replaceAttachments( conversation.get('id'), conversation.get('draftAttachments') || [] )(dispatch, getState, undefined); dispatch(resetComposer(conversationId)); + + await Promise.all(promises); + if (window.SignalCI) { + window.SignalCI.handleEvent('conversationOpenComplete', null); + } }; } diff --git a/ts/test-mock/benchmarks/group_send_bench.ts b/ts/test-mock/benchmarks/group_send_bench.ts index 4a70ef1ceff5..79ab785a8121 100644 --- a/ts/test-mock/benchmarks/group_send_bench.ts +++ b/ts/test-mock/benchmarks/group_send_bench.ts @@ -21,7 +21,6 @@ import { } from './fixtures'; import { stats } from '../../util/benchmark/stats'; import { sleep } from '../../util/sleep'; -import { MINUTE } from '../../util/durations'; import { typeIntoInput } from '../helpers'; const LAST_MESSAGE = 'start sending messages now'; @@ -31,8 +30,9 @@ Bootstrap.benchmark(async (bootstrap: Bootstrap): Promise => { const members = [...contacts].slice(0, GROUP_SIZE); + const GROUP_NAME = 'Mock Group'; const group = await phone.createGroup({ - title: 'Mock Group', + title: GROUP_NAME, members: [phone, ...members], }); @@ -115,10 +115,18 @@ Bootstrap.benchmark(async (bootstrap: Bootstrap): Promise => { const item = leftPane .locator( '.module-conversation-list__item--contact-or-conversation' + - `>> text="${LAST_MESSAGE}"` + `>> text="${GROUP_NAME}"` ) .first(); - await item.click({ timeout: 3 * MINUTE }); + await item.click(); + } + + debug('finding message in timeline'); + { + const item = window + .locator(`.module-message >> text="${LAST_MESSAGE}"`) + .first(); + await item.click(); } const deltaList = new Array(); diff --git a/ts/test-mock/playwright.ts b/ts/test-mock/playwright.ts index a183fa562d21..d90563d8064e 100644 --- a/ts/test-mock/playwright.ts +++ b/ts/test-mock/playwright.ts @@ -184,6 +184,10 @@ export class App extends EventEmitter { return this.waitForEvent('unlinkCleanupComplete'); } + public async waitForConversationOpenComplete(): Promise { + return this.waitForEvent('conversationOpenComplete'); + } + // EventEmitter types public override on(type: 'close', callback: () => void): this; diff --git a/ts/test-mock/pnp/accept_gv2_invite_test.ts b/ts/test-mock/pnp/accept_gv2_invite_test.ts index f213800c090c..5c0004b41403 100644 --- a/ts/test-mock/pnp/accept_gv2_invite_test.ts +++ b/ts/test-mock/pnp/accept_gv2_invite_test.ts @@ -84,6 +84,9 @@ describe('pnp/accept gv2 invite', function (this: Mocha.Suite) { debug('Opening group'); await leftPane.locator(`[data-testid="${group.id}"]`).click(); + + debug('Wait for conversation open fetches to complete'); + await app.waitForConversationOpenComplete(); }); afterEach(async function (this: Mocha.Context) {