From f0a3c3b40de594288101781b96983dfff8c83696 Mon Sep 17 00:00:00 2001 From: PatrykBuniX Date: Mon, 6 Nov 2023 12:10:19 +0100 Subject: [PATCH] runfix: joining after migration is finalised --- src/script/backup/BackupRepository.test.ts | 5 +- .../conversation/ConversationFilter.test.ts | 2 + src/script/conversation/ConversationMapper.ts | 17 ++- .../conversation/ConversationRepository.ts | 14 +-- src/script/entity/Conversation.ts | 3 + src/script/main/app.ts | 9 +- ...rsationsAfterMigrationFinalisation.test.ts | 104 ++++-------------- ...ConversationsAfterMigrationFinalisation.ts | 34 ++---- src/script/storage/StorageSchemata.ts | 13 +++ .../storage/record/ConversationRecord.ts | 1 + 10 files changed, 73 insertions(+), 129 deletions(-) diff --git a/src/script/backup/BackupRepository.test.ts b/src/script/backup/BackupRepository.test.ts index 78b4cd053bb..42f00ccf893 100644 --- a/src/script/backup/BackupRepository.test.ts +++ b/src/script/backup/BackupRepository.test.ts @@ -198,11 +198,12 @@ describe('BackupRepository', () => { it('successfully imports a backup', async () => { const [backupRepository, {backupService, conversationRepository}] = await buildBackupRepository(); const user = new User('user1'); - jest.spyOn(backupService, 'getDatabaseVersion').mockReturnValue(15); + const mockedDBVersion = 20; + jest.spyOn(backupService, 'getDatabaseVersion').mockReturnValue(mockedDBVersion); const importSpy = jest.spyOn(backupService, 'importEntities').mockResolvedValue(1); const users = [generateAPIUser(), generateAPIUser()]; - const metadata = {...backupRepository.createMetaData(user, 'client1'), version: 15}; + const metadata = {...backupRepository.createMetaData(user, 'client1'), version: mockedDBVersion}; const files = { [Filename.METADATA]: JSON.stringify(metadata), diff --git a/src/script/conversation/ConversationFilter.test.ts b/src/script/conversation/ConversationFilter.test.ts index b1487d67b0f..88535417933 100644 --- a/src/script/conversation/ConversationFilter.test.ts +++ b/src/script/conversation/ConversationFilter.test.ts @@ -65,6 +65,7 @@ describe('ConversationFilter', () => { name: 'Florian@Staging11', others: ['71e25be1-5433-4647-964d-03a5d9e7c970'], protocol: ConversationProtocol.PROTEUS, + initial_protocol: ConversationProtocol.PROTEUS, qualified_others: undefined, receipt_mode: null, roles: {}, @@ -114,6 +115,7 @@ describe('ConversationFilter', () => { muted_timestamp: 0, name: 'Florian@Staging11', others: ['71e25be1-5433-4647-964d-03a5d9e7c970'], + initial_protocol: ConversationProtocol.PROTEUS, protocol: ConversationProtocol.PROTEUS, qualified_others: undefined, receipt_mode: null, diff --git a/src/script/conversation/ConversationMapper.ts b/src/script/conversation/ConversationMapper.ts index 8c37364c19e..bfe49db054c 100644 --- a/src/script/conversation/ConversationMapper.ts +++ b/src/script/conversation/ConversationMapper.ts @@ -234,8 +234,20 @@ export class ConversationMapper { throw new ConversationError(BASE_ERROR_TYPE.INVALID_PARAMETER, BaseError.MESSAGE.INVALID_PARAMETER); } - const {creator, id, members, name, others, qualified_others, type, group_id, epoch, protocol, cipher_suite} = - conversationData; + const { + creator, + id, + members, + name, + others, + qualified_others, + type, + group_id, + epoch, + protocol, + cipher_suite, + initial_protocol, + } = conversationData; let conversationEntity = new Conversation( id, @@ -246,6 +258,7 @@ export class ConversationMapper { conversationEntity.creator = creator; conversationEntity.groupId = group_id; + conversationEntity.initialProtocol = initial_protocol; conversationEntity.epoch = epoch ?? -1; conversationEntity.cipherSuite = cipher_suite; conversationEntity.type(type); diff --git a/src/script/conversation/ConversationRepository.ts b/src/script/conversation/ConversationRepository.ts index db1142c7e62..1ce743a8356 100644 --- a/src/script/conversation/ConversationRepository.ts +++ b/src/script/conversation/ConversationRepository.ts @@ -579,15 +579,14 @@ export class ConversationRepository { /** * Will load all the conversations in memory - * @param initialLocalConversations conversations stored in the local database * @returns all the conversations from backend merged with the locally stored conversations and loaded into memory */ - public async loadConversations(initialLocalConversations?: ConversationDatabaseData[]): Promise { + public async loadConversations(): Promise { const remoteConversations = await this.conversationService.getAllConversations().catch(error => { this.logger.error(`Failed to get all conversations from backend: ${error.message}`); return {found: []} as RemoteConversations; }); - return this.loadRemoteConversations(remoteConversations, initialLocalConversations); + return this.loadRemoteConversations(remoteConversations); } /** @@ -613,13 +612,8 @@ export class ConversationRepository { * @param remoteConversations new conversations fetched from backend * @returns the new conversations from backend merged with the locally stored conversations */ - private async loadRemoteConversations( - remoteConversations: RemoteConversations, - initialLocalConversations?: ConversationDatabaseData[], - ): Promise { - const localConversations = - initialLocalConversations || - (await this.conversationService.loadConversationStatesFromDb()); + private async loadRemoteConversations(remoteConversations: RemoteConversations): Promise { + const localConversations = await this.conversationService.loadConversationStatesFromDb(); let conversationsData: any[]; if (!remoteConversations.found?.length) { diff --git a/src/script/entity/Conversation.ts b/src/script/entity/Conversation.ts index 202b845b59a..9abab507f27 100644 --- a/src/script/entity/Conversation.ts +++ b/src/script/entity/Conversation.ts @@ -106,6 +106,8 @@ export class Conversation { public groupId?: string; public epoch: number = -1; public cipherSuite: number = 1; + // Initial protocol is a protocol that was known by a webapp before any protocol update happened. For newly created conversations it is the same as protocol. + public initialProtocol: ConversationProtocol = this.protocol; public readonly display_name: ko.PureComputed; public readonly firstUserEntity: ko.PureComputed; public readonly globalMessageTimer: ko.Observable; @@ -1005,6 +1007,7 @@ export class Conversation { epoch: this.epoch, global_message_timer: this.globalMessageTimer(), group_id: this.groupId, + initial_protocol: this.initialProtocol, id: this.id, is_guest: this.isGuest(), last_event_timestamp: this.last_event_timestamp(), diff --git a/src/script/main/app.ts b/src/script/main/app.ts index dfaf49e70ed..ba389ef816f 100644 --- a/src/script/main/app.ts +++ b/src/script/main/app.ts @@ -56,7 +56,6 @@ import {ClientRepository, ClientService} from '../client'; import {Configuration} from '../Config'; import {ConnectionRepository} from '../connection/ConnectionRepository'; import {ConnectionService} from '../connection/ConnectionService'; -import {ConversationDatabaseData} from '../conversation/ConversationMapper'; import {ConversationRepository} from '../conversation/ConversationRepository'; import {ConversationService} from '../conversation/ConversationService'; import {MessageRepository} from '../conversation/MessageRepository'; @@ -423,10 +422,7 @@ export class App { const connections = await connectionRepository.getConnections(); telemetry.addStatistic(AppInitStatisticsValue.CONNECTIONS, connections.length, 50); - const initialLocalConversations = - await this.service.conversation.loadConversationStatesFromDb(); - - const conversations = await conversationRepository.loadConversations(initialLocalConversations); + const conversations = await conversationRepository.loadConversations(); await userRepository.loadUsers(selfUser, connections, conversations, teamMembers); @@ -463,8 +459,7 @@ export class App { if (supportsMLSMigration()) { //join all the mls groups that are known by the user but were migrated to mls await joinConversationsAfterMigrationFinalisation({ - updatedConversations: conversations, - initialDatabaseConversations: initialLocalConversations, + conversations, core: this.core, conversationRepository: conversationRepository, }); diff --git a/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.test.ts b/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.test.ts index 06ccddad670..b7648612ab7 100644 --- a/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.test.ts +++ b/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.test.ts @@ -25,8 +25,7 @@ import { } from '@wireapp/api-client/lib/conversation'; import {container} from 'tsyringe'; -import {ConversationDatabaseData} from 'src/script/conversation/ConversationMapper'; -import {Conversation} from 'src/script/entity/Conversation'; +import {ConversationDatabaseData, ConversationMapper} from 'src/script/conversation/ConversationMapper'; import {User} from 'src/script/entity/User'; import {CONVERSATION} from 'src/script/event/Client'; import {Core} from 'src/script/service/CoreSingleton'; @@ -38,6 +37,7 @@ import {joinConversationsAfterMigrationFinalisation} from './'; const createMockedDBConversationEntry = ( id: string, domain: string, + initialProtocol: ConversationProtocol, protocol: ConversationProtocol, type: CONVERSATION_TYPE, ): ConversationDatabaseData => ({ @@ -65,6 +65,7 @@ const createMockedDBConversationEntry = ( global_message_timer: 0, group_id: 'AAEAAGzYgfBo4k5Eti33a4ZZ78cAYW50YS53aXJlLmxpbms=', id, + initial_protocol: initialProtocol, is_guest: false, last_event_timestamp: 1688640266515, last_read_timestamp: 1688640266515, @@ -98,12 +99,15 @@ const createMockedDBConversationEntry = ( const createConversation = ( id: string, domain: string, + initialProtocol: ConversationProtocol, protocol: ConversationProtocol, type: CONVERSATION_TYPE, selfUser: User, groupId?: string, ) => { - const conversation = new Conversation(id, domain, protocol); + const conversationRecord = createMockedDBConversationEntry(id, domain, initialProtocol, protocol, type); + + const [conversation] = ConversationMapper.mapConversations([conversationRecord]); conversation.type(type); if (protocol === ConversationProtocol.MLS) { @@ -115,20 +119,6 @@ const createConversation = ( return conversation; }; -const createdMigratedConversationEntities = ( - id: string, - domain: string, - type: CONVERSATION_TYPE, - selfUser: User, - protocols: {localStore: ConversationProtocol; backend: ConversationProtocol}, - groupId?: string, -) => { - return { - conversationDatabaseData: createMockedDBConversationEntry(id, domain, protocols.localStore, type), - updatedConversation: createConversation(id, domain, protocols.backend, type, selfUser, groupId), - }; -}; - const testFactory = new TestFactory(); describe('joinConversationsAfterMigrationFinalisation', () => { @@ -148,21 +138,18 @@ describe('joinConversationsAfterMigrationFinalisation', () => { const conversationGroupId = 'groupId1'; const selfUser = new User(createUuid()); - const {updatedConversation, conversationDatabaseData} = createdMigratedConversationEntities( + const mockedConversation = createConversation( conversationId, mockDomain, + ConversationProtocol.PROTEUS, + ConversationProtocol.MLS, CONVERSATION_TYPE.REGULAR, selfUser, - { - localStore: ConversationProtocol.PROTEUS, - backend: ConversationProtocol.MLS, - }, conversationGroupId, ); await joinConversationsAfterMigrationFinalisation({ - updatedConversations: [updatedConversation], - initialDatabaseConversations: [conversationDatabaseData], + conversations: [mockedConversation], core: mockCore, conversationRepository, }); @@ -195,21 +182,18 @@ describe('joinConversationsAfterMigrationFinalisation', () => { const conversationGroupId = 'groupId1'; const selfUser = new User(createUuid()); - const {updatedConversation, conversationDatabaseData} = createdMigratedConversationEntities( + const mockedConversations = createConversation( conversationId, mockDomain, + ConversationProtocol.PROTEUS, + ConversationProtocol.MLS, CONVERSATION_TYPE.ONE_TO_ONE, selfUser, - { - localStore: ConversationProtocol.PROTEUS, - backend: ConversationProtocol.MLS, - }, conversationGroupId, ); await joinConversationsAfterMigrationFinalisation({ - updatedConversations: [updatedConversation], - initialDatabaseConversations: [conversationDatabaseData], + conversations: [mockedConversations], core: mockCore, conversationRepository, }); @@ -231,57 +215,18 @@ describe('joinConversationsAfterMigrationFinalisation', () => { const conversationGroupId = 'groupId1'; const selfUser = new User(createUuid()); - const {updatedConversation, conversationDatabaseData} = createdMigratedConversationEntities( - conversationId, - mockDomain, - CONVERSATION_TYPE.REGULAR, - selfUser, - { - localStore: ConversationProtocol.MLS, - backend: ConversationProtocol.MLS, - }, - conversationGroupId, - ); - - await joinConversationsAfterMigrationFinalisation({ - updatedConversations: [updatedConversation], - initialDatabaseConversations: [conversationDatabaseData], - core: mockCore, - conversationRepository, - }); - - expect(mockCore.service?.conversation.joinByExternalCommit).not.toHaveBeenCalled(); - - expect(conversationRepository['eventRepository'].injectEvent).not.toHaveBeenCalled(); - }); - - it('Should not join MLS conversation if conversation was not in the store before', async () => { - const mockCore = container.resolve(Core); - const conversationRepository = await testFactory.exposeConversationActors(); - - jest.spyOn(mockCore.service!.conversation, 'mlsGroupExistsLocally').mockResolvedValue(false); - jest.spyOn(conversationRepository['eventRepository'], 'injectEvent'); - - const conversationId = 'conversation1'; - const mockDomain = 'anta.wire.link'; - const conversationGroupId = 'groupId1'; - const selfUser = new User(createUuid()); - - const {updatedConversation} = createdMigratedConversationEntities( + const mockedConversation = createConversation( conversationId, mockDomain, + ConversationProtocol.MLS, + ConversationProtocol.MLS, CONVERSATION_TYPE.REGULAR, selfUser, - { - localStore: ConversationProtocol.PROTEUS, - backend: ConversationProtocol.MLS, - }, conversationGroupId, ); await joinConversationsAfterMigrationFinalisation({ - updatedConversations: [updatedConversation], - initialDatabaseConversations: [], + conversations: [mockedConversation], core: mockCore, conversationRepository, }); @@ -303,21 +248,18 @@ describe('joinConversationsAfterMigrationFinalisation', () => { const conversationGroupId = 'groupId1'; const selfUser = new User(createUuid()); - const {updatedConversation, conversationDatabaseData} = createdMigratedConversationEntities( + const mockedConversation = createConversation( conversationId, mockDomain, + ConversationProtocol.PROTEUS, + ConversationProtocol.PROTEUS, CONVERSATION_TYPE.REGULAR, selfUser, - { - localStore: ConversationProtocol.PROTEUS, - backend: ConversationProtocol.PROTEUS, - }, conversationGroupId, ); await joinConversationsAfterMigrationFinalisation({ - updatedConversations: [updatedConversation], - initialDatabaseConversations: [conversationDatabaseData], + conversations: [mockedConversation], core: mockCore, conversationRepository, }); diff --git a/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.ts b/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.ts index fcaca531fea..dd3ff7771ba 100644 --- a/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.ts +++ b/src/script/mls/MLSMigration/finaliseMigration/joinConversationsAfterMigrationFinalisation/joinConversationsAfterMigrationFinalisation.ts @@ -21,7 +21,6 @@ import {ConversationProtocol} from '@wireapp/api-client/lib/conversation'; import {Account} from '@wireapp/core'; -import {ConversationDatabaseData} from 'src/script/conversation/ConversationMapper'; import {ConversationRepository} from 'src/script/conversation/ConversationRepository'; import {MLSConversation, isMLSConversation} from 'src/script/conversation/ConversationSelectors'; import {Conversation} from 'src/script/entity/Conversation'; @@ -32,17 +31,14 @@ import {initMLSConversations} from 'src/script/mls/MLSConversations'; * Will join the MLS group and send a system message to the conversation if the conversation was previously using proteus and is now using MLS. * Should be called before we join new unestablished MLS conversations, otherwise we would join without insterting the system message. * - * @param updatedConversations - list of conversations updated from the backend - * @param initialDatabaseConversations - list of conversations stored in the local database before being updated from the backend + * @param conversations - list of conversations updated from the backend */ export const joinConversationsAfterMigrationFinalisation = async ({ - updatedConversations, - initialDatabaseConversations, + conversations, conversationRepository, core, }: { - updatedConversations: Conversation[]; - initialDatabaseConversations: ConversationDatabaseData[]; + conversations: Conversation[]; conversationRepository: ConversationRepository; core: Account; }) => { @@ -50,10 +46,7 @@ export const joinConversationsAfterMigrationFinalisation = async ({ //if such conversations were previously using proteus, and now are using MLS, //it means that the self user did not take part in the migration and is joining a conversation late //we have to join the conversation with external commit and let user know that they might have missed some messages - const alreadyMigratedConversations = filterGroupConversationsAlreadyMigratedToMLS( - updatedConversations, - initialDatabaseConversations, - ); + const alreadyMigratedConversations = filterGroupConversationsAlreadyMigratedToMLS(conversations); await initMLSConversations( alreadyMigratedConversations, @@ -62,25 +55,12 @@ export const joinConversationsAfterMigrationFinalisation = async ({ ); }; -const filterGroupConversationsAlreadyMigratedToMLS = ( - updatedConversations: Conversation[], - initialDatabaseConversations: ConversationDatabaseData[], -) => { - return updatedConversations.filter((conversation): conversation is MLSConversation => { +const filterGroupConversationsAlreadyMigratedToMLS = (conversations: Conversation[]) => { + return conversations.filter((conversation): conversation is MLSConversation => { if (!conversation.isGroup()) { return false; } - const localConversation = initialDatabaseConversations.find(localConversation => { - return localConversation.id === conversation.id; - }); - - if (!localConversation) { - return false; - } - - const isConversationMigratedToMLS = isMLSConversation(conversation); - - return localConversation.protocol === ConversationProtocol.PROTEUS && isConversationMigratedToMLS; + return isMLSConversation(conversation) && conversation.initialProtocol !== ConversationProtocol.MLS; }); }; diff --git a/src/script/storage/StorageSchemata.ts b/src/script/storage/StorageSchemata.ts index 8e847e311a9..1003927d781 100644 --- a/src/script/storage/StorageSchemata.ts +++ b/src/script/storage/StorageSchemata.ts @@ -21,6 +21,8 @@ import type {Dexie, Transaction} from 'dexie'; import {base64ToArray} from 'Util/util'; +import {ConversationRecord} from './record'; + import {categoryFromEvent} from '../message/MessageCategorization'; interface DexieSchema { @@ -414,6 +416,17 @@ export class StorageSchemata { schema: {}, version: 19, }, + { + schema: {}, + upgrade: (transaction: Transaction) => + transaction + .table(StorageSchemata.OBJECT_STORE.CONVERSATIONS) + .toCollection() + .modify((conversation: ConversationRecord) => { + conversation.initial_protocol = conversation.protocol; + }), + version: 20, + }, ]; } } diff --git a/src/script/storage/record/ConversationRecord.ts b/src/script/storage/record/ConversationRecord.ts index d1ce32b467b..62d768778a8 100644 --- a/src/script/storage/record/ConversationRecord.ts +++ b/src/script/storage/record/ConversationRecord.ts @@ -47,6 +47,7 @@ export interface ConversationRecord { group_id: string; epoch: number; id: string; + initial_protocol: ConversationProtocol; is_guest: boolean; last_event_timestamp: number; last_read_timestamp: number;