From 2732e36c7ff150e7caf50e4b87c43a688c2ed0b6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 23 Jan 2024 11:26:53 +0100 Subject: [PATCH] fix: don't load self convo's messages into memory [WPB-6166] (#16589) (#16594) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: don't load self convo's messages into memory * [tmp] * test: update tests * refactor: improve naming * test: fix tests * test: simplify generating a convo Co-authored-by: Patryk Górka --- src/script/backup/BackupRepository.test.ts | 75 +++++++++++-------- src/script/backup/BackupRepository.ts | 5 +- .../conversation/ConversationSelectors.ts | 14 ++++ src/script/conversation/ConversationState.ts | 18 +---- 4 files changed, 65 insertions(+), 47 deletions(-) diff --git a/src/script/backup/BackupRepository.test.ts b/src/script/backup/BackupRepository.test.ts index c682d9aa80b..1d67115a4ce 100644 --- a/src/script/backup/BackupRepository.test.ts +++ b/src/script/backup/BackupRepository.test.ts @@ -17,9 +17,12 @@ * */ +import {CONVERSATION_TYPE} from '@wireapp/api-client/lib/conversation'; import {container} from 'tsyringe'; import {omit} from 'underscore'; +import {generateConversation} from 'test/helper/ConversationGenerator'; +import {TestFactory} from 'test/helper/TestFactory'; import {generateAPIUser} from 'test/helper/UserGenerator'; import {noop} from 'Util/util'; import {createUuid} from 'Util/uuid'; @@ -31,7 +34,6 @@ import {BackupService} from './BackupService'; import {CancelError, DifferentAccountError, IncompatiblePlatformError} from './Error'; import {handleZipEvent} from './zipWorker'; -import {ConversationRepository} from '../conversation/ConversationRepository'; import {User} from '../entity/User'; import {ClientEvent} from '../event/Client'; import {DatabaseTypes, createStorageEngine} from '../service/StoreEngineProvider'; @@ -39,22 +41,14 @@ import {StorageService} from '../storage'; import {StorageSchemata} from '../storage/StorageSchemata'; const conversationId = '35a9a89d-70dc-4d9e-88a2-4d8758458a6a'; -const conversation = { - accessModes: ['private'], - accessRole: 'private', - archived_state: false, - archived_timestamp: 0, - creator: '1ccd93e0-0f4b-4a73-b33f-05c464b88439', - id: conversationId, - last_event_timestamp: 2, - last_server_timestamp: 2, - muted_state: false, - muted_timestamp: 0, - name: 'Tom @ Staging', - others: ['a7122859-3f16-4870-b7f2-5cbca5572ab2'], - status: 0, - type: 2, -}; + +const conversation = generateConversation({ + id: {id: conversationId, domain: 'test.wire.link'}, + overwites: { + status: 0, + type: 2, + }, +}).serialize(); const messages = [ { @@ -81,14 +75,15 @@ async function buildBackupRepository() { storageService.init(engine); const backupService = new BackupService(storageService); - const conversationRepository = { - initAllLocal1To1Conversations: jest.fn(), - getAllLocalConversations: jest.fn(), - checkForDeletedConversations: jest.fn(), - mapConnections: jest.fn().mockImplementation(() => []), - updateConversationStates: jest.fn().mockImplementation(conversations => conversations), - updateConversations: jest.fn().mockImplementation(async () => {}), - } as unknown as ConversationRepository; + + const testFactory = new TestFactory(); + const conversationRepository = await testFactory.exposeConversationActors(); + + jest + .spyOn(conversationRepository, 'mapConversations') + .mockImplementation(conversations => conversations.map(c => generateConversation({type: c.type, overwites: c}))); + jest.spyOn(conversationRepository, 'updateConversationStates'); + jest.spyOn(conversationRepository, 'updateConversations'); return [ new BackupRepository(backupService, conversationRepository), {backupService, conversationRepository, storageService}, @@ -137,8 +132,8 @@ describe('BackupRepository', () => { }; const importSpy = jest.spyOn(backupService, 'importEntities'); - await storageService.save(StorageSchemata.OBJECT_STORE.EVENTS, undefined, verificationEvent); - await storageService.save(StorageSchemata.OBJECT_STORE.EVENTS, undefined, textEvent); + await storageService.save(StorageSchemata.OBJECT_STORE.EVENTS, '', verificationEvent); + await storageService.save(StorageSchemata.OBJECT_STORE.EVENTS, '', textEvent); const blob = await backupRepository.generateHistory(user, 'client1', noop, password); await backupRepository.importHistory(new User('user1'), blob, noop, noop); @@ -153,7 +148,7 @@ describe('BackupRepository', () => { const [backupRepository, {storageService}] = await buildBackupRepository(); const password = ''; await Promise.all([ - ...messages.map(message => storageService.save(eventStoreName, undefined, message)), + ...messages.map(message => storageService.save(eventStoreName, '', message)), storageService.save('conversations', conversationId, conversation), ]); @@ -201,9 +196,19 @@ describe('BackupRepository', () => { const metadata = {...backupRepository.createMetaData(user, 'client1'), version: mockedDBVersion}; + const conversation = generateConversation({ + id: {id: 'conversation1', domain: 'staging2'}, + type: CONVERSATION_TYPE.ONE_TO_ONE, + }).serialize(); + + const selfConversation = generateConversation({ + id: {id: 'conversation2', domain: 'staging2'}, + type: CONVERSATION_TYPE.SELF, + }).serialize(); + const files = { [Filename.METADATA]: JSON.stringify(metadata), - [Filename.CONVERSATIONS]: JSON.stringify([conversation]), + [Filename.CONVERSATIONS]: JSON.stringify([conversation, selfConversation]), [Filename.EVENTS]: JSON.stringify(messages), [Filename.USERS]: JSON.stringify(users), }; @@ -212,7 +217,17 @@ describe('BackupRepository', () => { await backupRepository.importHistory(user, zip, noop, noop); - expect(conversationRepository.updateConversationStates).toHaveBeenCalledWith([conversation]); + expect(conversationRepository.updateConversationStates).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({id: conversation.id})]), + ); + + expect(conversationRepository.updateConversations).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({id: conversation.id})]), + ); + expect(conversationRepository.updateConversations).not.toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({id: selfConversation.id})]), + ); + expect(importSpy).toHaveBeenCalledWith( StorageSchemata.OBJECT_STORE.EVENTS, messages.map(message => omit(message, 'primary_key')), diff --git a/src/script/backup/BackupRepository.ts b/src/script/backup/BackupRepository.ts index 8c5fb03f5d2..fd4da44f6fa 100644 --- a/src/script/backup/BackupRepository.ts +++ b/src/script/backup/BackupRepository.ts @@ -41,6 +41,7 @@ import { import {preprocessConversations, preprocessEvents, preprocessUsers} from './recordPreprocessors'; import type {ConversationRepository} from '../conversation/ConversationRepository'; +import {isReadableConversation} from '../conversation/ConversationSelectors'; import type {Conversation} from '../entity/Conversation'; import {User} from '../entity/User'; import {EventRecord, UserRecord} from '../storage'; @@ -381,7 +382,9 @@ export class BackupRepository { // Run all the database migrations on the imported data await this.backupService.runDbSchemaUpdates(archiveVersion); - await this.conversationRepository.updateConversations(importedConversations); + const readableConversations = importedConversations.filter(isReadableConversation); + + await this.conversationRepository.updateConversations(readableConversations); await this.conversationRepository.initAllLocal1To1Conversations(); // doesn't need to be awaited void this.conversationRepository.checkForDeletedConversations(); diff --git a/src/script/conversation/ConversationSelectors.ts b/src/script/conversation/ConversationSelectors.ts index 8c359758291..73659f9c7fa 100644 --- a/src/script/conversation/ConversationSelectors.ts +++ b/src/script/conversation/ConversationSelectors.ts @@ -17,6 +17,7 @@ * */ +import {ConnectionStatus} from '@wireapp/api-client/lib/connection/'; import {CONVERSATION_TYPE, ConversationProtocol} from '@wireapp/api-client/lib/conversation/'; import {QualifiedId} from '@wireapp/api-client/lib/user/'; @@ -100,3 +101,16 @@ export const isProteus1to1ConversationWithUser = (userId: QualifiedId) => export const isMLS1to1ConversationWithUser = (userId: QualifiedId) => is1to1ConversationWithUser(userId, ConversationProtocol.MLS); + +export const isReadableConversation = (conversation: Conversation): boolean => { + const states_to_filter = [ + ConnectionStatus.MISSING_LEGAL_HOLD_CONSENT, + ConnectionStatus.BLOCKED, + ConnectionStatus.CANCELLED, + ConnectionStatus.PENDING, + ]; + + const connection = conversation.connection(); + + return !(isSelfConversation(conversation) || (connection && states_to_filter.includes(connection.status()))); +}; diff --git a/src/script/conversation/ConversationState.ts b/src/script/conversation/ConversationState.ts index 7b22dfaf115..c8cb1197785 100644 --- a/src/script/conversation/ConversationState.ts +++ b/src/script/conversation/ConversationState.ts @@ -17,7 +17,6 @@ * */ -import {ConnectionStatus} from '@wireapp/api-client/lib/connection/'; import {QualifiedId} from '@wireapp/api-client/lib/user'; import ko from 'knockout'; import {container, singleton} from 'tsyringe'; @@ -32,6 +31,7 @@ import { isMLSConversation, isProteus1to1ConversationWithUser, isSelfConversation, + isReadableConversation, } from './ConversationSelectors'; import {Conversation} from '../entity/Conversation'; @@ -103,21 +103,7 @@ export class ConversationState { }); this.filteredConversations = ko.pureComputed(() => { - return this.conversations().filter(conversationEntity => { - const states_to_filter = [ - ConnectionStatus.MISSING_LEGAL_HOLD_CONSENT, - ConnectionStatus.BLOCKED, - ConnectionStatus.CANCELLED, - ConnectionStatus.PENDING, - ]; - - const connection = conversationEntity.connection(); - - return !( - isSelfConversation(conversationEntity) || - (connection && states_to_filter.includes(connection.status())) - ); - }); + return this.conversations().filter(isReadableConversation); }); this.connectedUsers = ko.pureComputed(() => {