From c368aea4e94d0f67166d74f6d23f3e0006132a49 Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Thu, 16 Apr 2020 17:03:39 +0200 Subject: [PATCH 1/8] feat: Check for missing team member when no clients are left --- .../conversation/ClientMismatchHandler.ts | 74 ++++++++++--------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index d7494460d5d..ed4bfedace5 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -73,8 +73,8 @@ export class ClientMismatchHandler { eventInfoEntity.conversationId !== '' ? await this.conversationRepository.get_conversation_by_id(eventInfoEntity.conversationId) : undefined; - this.handleRedundant(redundant, payload, conversationEntity); - this.handleDeleted(deleted, payload, conversationEntity); + await this.handleRedundant(redundant, payload, conversationEntity); + await this.handleDeleted(deleted, payload, conversationEntity); return this.handleMissing(missing, payload, conversationEntity, eventInfoEntity); } @@ -84,19 +84,23 @@ export class ClientMismatchHandler { * @note Contains clients of which the backend is sure that they should not be recipient of a message and verified they no longer exist. * @param recipients User client map containing redundant clients * @param payload Payload of the request - * @param [conversationEntity] Conversation entity + * @param conversationEntity Conversation entity * @returns Resolves when the payload got updated */ - private handleDeleted(recipients: UserClients, payload: NewOTRMessage, conversationEntity?: Conversation): void { + private async handleDeleted( + recipients: UserClients, + payload: NewOTRMessage, + conversationEntity?: Conversation, + ): Promise { if (Object.entries(recipients).length === 0) { return; } this.logger.debug(`Message contains deleted clients of '${Object.keys(recipients).length}' users`, recipients); - const removeDeletedClient = (userId: string, clientId: string) => { + const removeDeletedClient = async (userId: string, clientId: string) => { delete payload.recipients[userId][clientId]; - this.userRepository.removeClientFromUser(userId, clientId); + await this.userRepository.removeClientFromUser(userId, clientId); }; - this.removePayload(recipients, removeDeletedClient, conversationEntity, payload, false); + await this.removePayload(recipients, removeDeletedClient, conversationEntity, payload); } /** @@ -104,7 +108,7 @@ export class ClientMismatchHandler { * * @param recipients User client map containing redundant clients * @param payload Payload of the request - * @param [conversationEntity] Conversation entity + * @param conversationEntity Conversation entity * @param eventInfoEntity Info about event * @returns Resolves with the updated payload */ @@ -117,7 +121,7 @@ export class ClientMismatchHandler { const missingUserIds = Object.keys(recipients); if (!missingUserIds.length) { - return Promise.resolve(payload); + return payload; } this.logger.debug(`Message is missing clients of '${missingUserIds.length}' users`, recipients); @@ -157,16 +161,20 @@ export class ClientMismatchHandler { * Sometimes clients of the self user are listed. Thus we cannot remove the payload for all the clients of a user without checking. * @param recipients User client map containing redundant clients * @param payload Payload of the request - * @param [conversationEntity] Conversation entity + * @param conversationEntity Conversation entity * @returns Resolves when the payload got updated */ - private handleRedundant(recipients: UserClients, payload: NewOTRMessage, conversationEntity?: Conversation): void { + private async handleRedundant( + recipients: UserClients, + payload: NewOTRMessage, + conversationEntity?: Conversation, + ): Promise { if (Object.entries(recipients).length === 0) { return; } this.logger.debug(`Message contains redundant clients of '${Object.keys(recipients).length}' users`, recipients); const removeRedundantClient = (userId: string, clientId: string) => delete payload.recipients[userId][clientId]; - this.removePayload(recipients, removeRedundantClient, conversationEntity, payload, true); + await this.removePayload(recipients, removeRedundantClient, conversationEntity, payload); } /** @@ -176,40 +184,40 @@ export class ClientMismatchHandler { * @param clientFn Function to remove clients * @param conversationEntity Conversation entity * @param payload Initial payload resulting in a 412 - * @param verifyDeletedUsers Verifies if users still exist on the backend by checking their active clients * @returns Function array */ - removePayload( + async removePayload( recipients: UserClients, - clientFn: Function, + clientFn: (userId: string, clientId: string) => any | Promise, conversationEntity: Conversation = undefined, payload: NewOTRMessage, - verifyDeletedUsers: boolean, - ): void[] { - const result: void[] = []; - - const removeDeletedUser = (userId: string) => { + ): Promise { + const removeDeletedUser = async (userId: string): Promise => { const clientIdsOfUser = Object.keys(payload.recipients[userId]); const noRemainingClients = !clientIdsOfUser.length; - if (noRemainingClients) { - if (conversationEntity !== undefined) { - if (conversationEntity.isGroup() && verifyDeletedUsers) { - const timestamp = this.serverTimeHandler.toServerTimestamp(); - const event = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); - this.eventRepository.injectEvent(event); + if (noRemainingClients && typeof conversationEntity !== 'undefined' && conversationEntity.isGroup()) { + try { + const users = await this.userRepository.fetchUsersById([userId]); + if (users.length === 0) { + throw new Error('No users found'); } + } catch (error) { + const timestamp = this.serverTimeHandler.toServerTimestamp(); + const memberLeaveEvent = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); + this.eventRepository.injectEvent(memberLeaveEvent); } - - delete payload.recipients[userId]; } + + delete payload.recipients[userId]; }; - Object.entries(recipients).forEach(([userId, clientIds = []]) => { - clientIds.forEach(clientId => result.push(clientFn(userId, clientId))); - result.push(removeDeletedUser(userId)); - }); + for (const [userId, clientIds = []] of Object.entries(recipients)) { + for (const clientId of clientIds) { + await clientFn(userId, clientId); + } - return result; + await removeDeletedUser(userId); + } } } From 0144c904b9e363475289e8c3af093d1f7b73e861 Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Thu, 16 Apr 2020 18:08:06 +0200 Subject: [PATCH 2/8] remove comment [ci skip] --- src/script/conversation/ClientMismatchHandler.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index ed4bfedace5..c28fb92ec44 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -184,7 +184,6 @@ export class ClientMismatchHandler { * @param clientFn Function to remove clients * @param conversationEntity Conversation entity * @param payload Initial payload resulting in a 412 - * @returns Function array */ async removePayload( recipients: UserClients, From 7edead5e2a1db79053f804101399b876529ee25f Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Thu, 16 Apr 2020 19:07:43 +0200 Subject: [PATCH 3/8] check if users are deleted --- src/script/conversation/ClientMismatchHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index c28fb92ec44..a352017a38e 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -198,7 +198,7 @@ export class ClientMismatchHandler { if (noRemainingClients && typeof conversationEntity !== 'undefined' && conversationEntity.isGroup()) { try { const users = await this.userRepository.fetchUsersById([userId]); - if (users.length === 0) { + if (users.length === 0 || users.every(user => user && !!user.isDeleted)) { throw new Error('No users found'); } } catch (error) { From b7eabf32ca33ea9e65ca91ff358770bf470ba22d Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Thu, 16 Apr 2020 19:10:20 +0200 Subject: [PATCH 4/8] remove try-catch --- src/script/conversation/ClientMismatchHandler.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index a352017a38e..5acade505b5 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -196,12 +196,8 @@ export class ClientMismatchHandler { const noRemainingClients = !clientIdsOfUser.length; if (noRemainingClients && typeof conversationEntity !== 'undefined' && conversationEntity.isGroup()) { - try { - const users = await this.userRepository.fetchUsersById([userId]); - if (users.length === 0 || users.every(user => user && !!user.isDeleted)) { - throw new Error('No users found'); - } - } catch (error) { + const users = await this.userRepository.fetchUsersById([userId]); + if (users.length === 0 || users.every(user => user && !!user.isDeleted)) { const timestamp = this.serverTimeHandler.toServerTimestamp(); const memberLeaveEvent = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); this.eventRepository.injectEvent(memberLeaveEvent); From 9c7e5622ef97da072dd1d4762a2ebfa45aff2fcd Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Fri, 17 Apr 2020 08:25:53 +0200 Subject: [PATCH 5/8] use user service --- .../conversation/ClientMismatchHandler.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index 5acade505b5..a60a5e26f7a 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -29,6 +29,7 @@ import {UserRepository} from '../user/UserRepository'; import {EventInfoEntity} from './EventInfoEntity'; import {NewOTRMessage} from '@wireapp/api-client/dist/conversation'; import {Conversation} from '../entity/Conversation'; +import {BackendClientError} from '../error/BackendClientError'; export class ClientMismatchHandler { private readonly conversationRepository: ConversationRepository; @@ -196,11 +197,17 @@ export class ClientMismatchHandler { const noRemainingClients = !clientIdsOfUser.length; if (noRemainingClients && typeof conversationEntity !== 'undefined' && conversationEntity.isGroup()) { - const users = await this.userRepository.fetchUsersById([userId]); - if (users.length === 0 || users.every(user => user && !!user.isDeleted)) { - const timestamp = this.serverTimeHandler.toServerTimestamp(); - const memberLeaveEvent = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); - this.eventRepository.injectEvent(memberLeaveEvent); + try { + await this.userRepository['userService'].getUser('userId'); + } catch (error) { + const isNotFound = error.code === BackendClientError.STATUS_CODE.NOT_FOUND; + if (isNotFound) { + const timestamp = this.serverTimeHandler.toServerTimestamp(); + const memberLeaveEvent = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); + this.eventRepository.injectEvent(memberLeaveEvent); + } else { + throw error; + } } } From fd939483f58da27c3c2aa448d2f507c19c59598e Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Fri, 17 Apr 2020 09:40:11 +0200 Subject: [PATCH 6/8] Update src/script/conversation/ClientMismatchHandler.ts [ci skip] Co-Authored-By: Benny Neugebauer --- src/script/conversation/ClientMismatchHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index a60a5e26f7a..751319f43f0 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -188,7 +188,7 @@ export class ClientMismatchHandler { */ async removePayload( recipients: UserClients, - clientFn: (userId: string, clientId: string) => any | Promise, + clientFn: (userId: string, clientId: string) => void | Promise, conversationEntity: Conversation = undefined, payload: NewOTRMessage, ): Promise { From 27f89a8e53f1c30956c70f9f449fe8959deeb342 Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Fri, 17 Apr 2020 09:41:04 +0200 Subject: [PATCH 7/8] PR review --- src/script/conversation/ClientMismatchHandler.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index 751319f43f0..f6655476da8 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -97,7 +97,7 @@ export class ClientMismatchHandler { return; } this.logger.debug(`Message contains deleted clients of '${Object.keys(recipients).length}' users`, recipients); - const removeDeletedClient = async (userId: string, clientId: string) => { + const removeDeletedClient = async (userId: string, clientId: string): Promise => { delete payload.recipients[userId][clientId]; await this.userRepository.removeClientFromUser(userId, clientId); }; @@ -174,7 +174,9 @@ export class ClientMismatchHandler { return; } this.logger.debug(`Message contains redundant clients of '${Object.keys(recipients).length}' users`, recipients); - const removeRedundantClient = (userId: string, clientId: string) => delete payload.recipients[userId][clientId]; + const removeRedundantClient = (userId: string, clientId: string) => { + delete payload.recipients[userId][clientId]; + }; await this.removePayload(recipients, removeRedundantClient, conversationEntity, payload); } From 42d00722f8d2dcae6eccf1cba147820bb2b5502a Mon Sep 17 00:00:00 2001 From: Florian Keller Date: Fri, 17 Apr 2020 12:18:18 +0200 Subject: [PATCH 8/8] backend returns deleted --- .../conversation/ClientMismatchHandler.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/script/conversation/ClientMismatchHandler.ts b/src/script/conversation/ClientMismatchHandler.ts index f6655476da8..fab0ee93165 100644 --- a/src/script/conversation/ClientMismatchHandler.ts +++ b/src/script/conversation/ClientMismatchHandler.ts @@ -199,17 +199,12 @@ export class ClientMismatchHandler { const noRemainingClients = !clientIdsOfUser.length; if (noRemainingClients && typeof conversationEntity !== 'undefined' && conversationEntity.isGroup()) { - try { - await this.userRepository['userService'].getUser('userId'); - } catch (error) { - const isNotFound = error.code === BackendClientError.STATUS_CODE.NOT_FOUND; - if (isNotFound) { - const timestamp = this.serverTimeHandler.toServerTimestamp(); - const memberLeaveEvent = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); - this.eventRepository.injectEvent(memberLeaveEvent); - } else { - throw error; - } + const result = await this.userRepository['userService'].getUser(userId); + const isDeleted = result?.deleted === true; + if (isDeleted) { + const timestamp = this.serverTimeHandler.toServerTimestamp(); + const memberLeaveEvent = EventBuilder.buildMemberLeave(conversationEntity, userId, false, timestamp); + this.eventRepository.injectEvent(memberLeaveEvent); } }