Skip to content

Commit

Permalink
fix: don't load self convo's messages into memory [WPB-6166] (#16589) (
Browse files Browse the repository at this point in the history
…#16594)

* 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 <[email protected]>
  • Loading branch information
github-actions[bot] and PatrykBuniX authored Jan 23, 2024
1 parent 6d34620 commit 2732e36
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 47 deletions.
75 changes: 45 additions & 30 deletions src/script/backup/BackupRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -31,30 +34,21 @@ 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';
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 = [
{
Expand All @@ -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},
Expand Down Expand Up @@ -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);
Expand All @@ -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),
]);

Expand Down Expand Up @@ -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),
};
Expand All @@ -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')),
Expand Down
5 changes: 4 additions & 1 deletion src/script/backup/BackupRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down
14 changes: 14 additions & 0 deletions src/script/conversation/ConversationSelectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/';

Expand Down Expand Up @@ -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())));
};
18 changes: 2 additions & 16 deletions src/script/conversation/ConversationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -32,6 +31,7 @@ import {
isMLSConversation,
isProteus1to1ConversationWithUser,
isSelfConversation,
isReadableConversation,
} from './ConversationSelectors';

import {Conversation} from '../entity/Conversation';
Expand Down Expand Up @@ -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(() => {
Expand Down

0 comments on commit 2732e36

Please sign in to comment.