Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve event builder #17756

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 64 additions & 6 deletions src/script/conversation/ConversationRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ import {
OnConversationVerificationStateChange,
} from './ConversationVerificationStateHandler/shared';
import {EventMapper} from './EventMapper';
import {createBaseEvent} from './EventNew';
import {MessageRepository} from './MessageRepository';
import {NOTIFICATION_STATE} from './NotificationSetting';

Expand All @@ -122,6 +123,7 @@ import {ConnectionEntity} from '../connection/ConnectionEntity';
import {ConnectionRepository} from '../connection/ConnectionRepository';
import {ConnectionState} from '../connection/ConnectionState';
import {
AllVerifiedEvent,
AssetAddEvent,
ButtonActionConfirmationEvent,
ClientConversationEvent,
Expand Down Expand Up @@ -151,6 +153,7 @@ import {isMemberMessage} from '../guards/Message';
import * as LegalHoldEvaluator from '../legal-hold/LegalHoldEvaluator';
import {MessageCategory} from '../message/MessageCategory';
import {SystemMessageType} from '../message/SystemMessageType';
import {VerificationMessageType} from '../message/VerificationMessageType';
import {addOtherSelfClientsToMLSConversation} from '../mls';
import {PropertiesRepository} from '../properties/PropertiesRepository';
import {SelfRepository} from '../self/SelfRepository';
Expand Down Expand Up @@ -926,11 +929,51 @@ export class ConversationRepository {
conversationEntity.withAllTeamMembers(allTeamMembersParticipate);
}

const creationEvent = conversationEntity.isGroup()
? EventBuilder.buildGroupCreation(conversationEntity, isTemporaryGuest, timestamp)
: EventBuilder.build1to1Creation(conversationEntity);
let creationEventNew;

await this.eventRepository.injectEvent(creationEvent, eventSource);
if (conversationEntity.isGroup()) {
const selfUser = conversationEntity.selfUser();

if (!selfUser) {
this.logger.error('Failed to get self user');
return;
}

const {creator: creatorId} = conversationEntity;
const selfUserId = selfUser.id;

const userIds = conversationEntity.participating_user_ids().slice();
const createdBySelf = creatorId === selfUserId || isTemporaryGuest;
if (!createdBySelf) {
userIds.push(selfUser.qualifiedId);
}

creationEventNew = createBaseEvent<GroupCreationEvent>({
conversation: conversationEntity,
eventType: ClientEvent.CONVERSATION.ONE2ONE_CREATION,
additionalData: {
userIds,
name: conversationEntity.name(),
allTeamMembers: conversationEntity.withAllTeamMembers(),
},
from: isTemporaryGuest ? selfUserId : creatorId,
timestamp: 0,
});
} else {
creationEventNew = createBaseEvent<OneToOneCreationEvent>({
conversation: conversationEntity,
eventType: ClientEvent.CONVERSATION.ONE2ONE_CREATION,
additionalData: {userIds: conversationEntity.participating_user_ids()},
from: conversationEntity.creator,
timestamp: 0,
});
}

// const creationEvent = conversationEntity.isGroup()
// ? EventBuilder.buildGroupCreation(conversationEntity, isTemporaryGuest, timestamp)
// : EventBuilder.build1to1Creation(conversationEntity);
Comment on lines +972 to +974
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 what to do with this?


await this.eventRepository.injectEvent(creationEventNew, eventSource);
}

/**
Expand Down Expand Up @@ -2311,7 +2354,12 @@ export class ConversationRepository {
}) => {
switch (conversationVerificationState) {
case ConversationVerificationState.VERIFIED:
const allVerifiedEvent = EventBuilder.buildAllVerified(conversationEntity);
// const allVerifiedEvent = EventBuilder.buildAllVerified(conversationEntity);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also unneeded?

const allVerifiedEvent = createBaseEvent<AllVerifiedEvent>({
conversation: conversationEntity,
eventType: ClientEvent.CONVERSATION.VERIFICATION,
additionalData: {type: VerificationMessageType.VERIFIED},
});
await this.eventRepository.injectEvent(allVerifiedEvent);
break;
case ConversationVerificationState.DEGRADED:
Expand Down Expand Up @@ -2647,9 +2695,19 @@ export class ConversationRepository {
// TODO: Can this even have a response? in the API Client it look like it always returns `void`
const hasResponse = response?.event;
const currentTimestamp = this.serverTimeHandler.toServerTimestamp();
// const event = hasResponse
// ? response.event
// : EventBuilder.buildMemberLeave(conversationEntity, [user], this.userState.self().id, currentTimestamp);

const event = hasResponse
? response.event
: EventBuilder.buildMemberLeave(conversationEntity, [user], this.userState.self().id, currentTimestamp);
: createBaseEvent<MemberLeaveEvent>({
conversation: conversationEntity,
eventType: CONVERSATION_EVENT.MEMBER_LEAVE,
additionalData: {qualified_user_ids: [user], user_ids: [user].map(({id}) => id)},
from: this.userState.self().id,
timestamp: currentTimestamp,
});

this.eventRepository.injectEvent(event, EventRepository.SOURCE.BACKEND_RESPONSE);
return event;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
expect(conversationB.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(conversationAB.is_verified()).toBeDefined();
expect(conversationAB.is_verified()).toBeTruthy();
expect(EventBuilder.buildAllVerified).not.toHaveBeenCalled();
// expect(EventBuilder.buildAllVerified).not.toHaveBeenCalled();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guess this should be replaced with createBaseEvent

expect(testFactory.event_repository.injectEvent).not.toHaveBeenCalled();
});
});
Expand Down Expand Up @@ -169,8 +169,8 @@
expect(conversationAB.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(conversationB.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(conversationC.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(EventBuilder.buildAllVerified).toHaveBeenCalledTimes(3);
// expect(EventBuilder.buildAllVerified).toHaveBeenCalledTimes(3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

expect(testFactory.event_repository.injectEvent).toHaveBeenCalledWith(verifiedEvent);

Check failure on line 173 in src/script/conversation/ConversationVerificationStateHandler/Proteus/ProteusStateHandler.test.ts

View workflow job for this annotation

GitHub Actions / test

ProteusConversationVerificationStateHandler › onClientRemoved › should change state from DEGRADED to VERIFIED if last unverified client was removed

expect(spy).toHaveBeenCalledWith(...expected) Expected: {"type": "verified"} Received 1 Object { - "type": "verified", + "type": "degraded", }, 2 Object { - "type": "verified", + "type": "degraded", }, 3 Object { - "type": "verified", + "type": "degraded", }, Number of calls: 6 at Object.toHaveBeenCalledWith (src/script/conversation/ConversationVerificationStateHandler/Proteus/ProteusStateHandler.test.ts:173:56)
});
});

Expand Down Expand Up @@ -199,8 +199,8 @@
expect(conversationAB.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(conversationB.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(conversationC.verification_state()).toBe(ConversationVerificationState.VERIFIED);
expect(EventBuilder.buildAllVerified).toHaveBeenCalledTimes(3);
// expect(EventBuilder.buildAllVerified).toHaveBeenCalledTimes(3);
expect(testFactory.event_repository.injectEvent).toHaveBeenCalledWith(verifiedEvent);

Check failure on line 203 in src/script/conversation/ConversationVerificationStateHandler/Proteus/ProteusStateHandler.test.ts

View workflow job for this annotation

GitHub Actions / test

ProteusConversationVerificationStateHandler › onClientsUpdated › should change state from DEGRADED to VERIFIED if last unverified client was removed by other user

expect(spy).toHaveBeenCalledWith(...expected) Expected: {"type": "verified"} Received 1 Object { - "type": "verified", + "type": "degraded", }, 2 Object { - "type": "verified", + "type": "degraded", }, 3 Object { - "type": "verified", + "type": "degraded", }, Number of calls: 6 at Object.toHaveBeenCalledWith (src/script/conversation/ConversationVerificationStateHandler/Proteus/ProteusStateHandler.test.ts:203:56)
});
});

Expand Down
12 changes: 10 additions & 2 deletions src/script/conversation/EventBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import type {QualifiedId} from '@wireapp/api-client/lib/user/';

import {EventBuilder} from 'src/script/conversation/EventBuilder';
import {AllVerifiedEvent, EventBuilder} from 'src/script/conversation/EventBuilder';
import {EventMapper} from 'src/script/conversation/EventMapper';
import {Conversation} from 'src/script/entity/Conversation';
import {User} from 'src/script/entity/User';
Expand All @@ -28,6 +28,8 @@ import {SuperType} from 'src/script/message/SuperType';
import {VerificationMessageType} from 'src/script/message/VerificationMessageType';
import {createUuid} from 'Util/uuid';

import {createBaseEvent} from './EventNew';

import {VerificationMessage} from '../entity/message/VerificationMessage';

describe('EventBuilder', () => {
Expand All @@ -46,7 +48,13 @@ describe('EventBuilder', () => {
});

it('buildAllVerified', () => {
const event = EventBuilder.buildAllVerified(conversation_et);
// const event = EventBuilder.buildAllVerified(conversation_et);
const event = createBaseEvent<AllVerifiedEvent>({
conversation: conversation_et,
eventType: ClientEvent.CONVERSATION.VERIFICATION,
additionalData: {type: VerificationMessageType.VERIFIED},
from: conversation_et.selfUser().id,
});
const messageEntity = event_mapper.mapJsonEvent(event as any, conversation_et) as VerificationMessage;
expect(messageEntity).toBeDefined();
expect(messageEntity.super_type).toBe(SuperType.VERIFICATION);
Expand Down
19 changes: 0 additions & 19 deletions src/script/conversation/EventBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,25 +466,6 @@ export const EventBuilder = {
};
},

buildIncomingMessageTooBig(
event: ConversationOtrMessageAddEvent,
messageError: Error,
errorCode: number,
): ErrorEvent {
const {qualified_conversation: conversationId, conversation, data: eventData, from, time} = event;

return {
...buildQualifiedId(conversationId || conversation),
data: undefined,
error: `${messageError.message} (${eventData.sender})`,
error_code: errorCode,
from,
id: createUuid(),
time,
type: ClientEvent.CONVERSATION.INCOMING_MESSAGE_TOO_BIG,
};
},

buildLegalHoldMessage(
conversationId: QualifiedId,
userId: QualifiedId,
Expand Down
57 changes: 57 additions & 0 deletions src/script/conversation/EventNew.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Wire
* Copyright (C) 2024 Wire Swiss GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*
*/

import type {QualifiedId} from '@wireapp/api-client/lib/user';

import {createUuid} from 'Util/uuid';

import type {Conversation} from '../entity/Conversation';

function buildQualifiedId(conversation: QualifiedId | string) {
const qualifiedId = typeof conversation === 'string' ? {domain: '', id: conversation} : conversation;
return {
conversation: qualifiedId.id,
qualified_conversation: {domain: qualifiedId.domain, id: qualifiedId.id},
};
}

type EventInput = {
conversation: Conversation;
eventType: string;
additionalData?: {};
from?: string;
timestamp?: number;
};

export function createBaseEvent<T extends {type: string; data?: unknown}>({
conversation,
eventType,
additionalData = {},
from,
timestamp,
}: EventInput): T {
return {
...buildQualifiedId(conversation),
data: additionalData,
from: from ?? conversation.selfUser()?.id,
id: createUuid(),
time: new Date(timestamp || conversation.getNextTimestamp()).toISOString(),
type: eventType,
} as unknown as T;
}
16 changes: 14 additions & 2 deletions src/script/event/preprocessor/ServiceMiddleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@
*/

import {ConversationRepository} from 'src/script/conversation/ConversationRepository';
import {EventBuilder} from 'src/script/conversation/EventBuilder';
import {EventBuilder, OneToOneCreationEvent} from 'src/script/conversation/EventBuilder';
import {Conversation} from 'src/script/entity/Conversation';
import {User} from 'src/script/entity/User';
import {UserRepository} from 'src/script/user/UserRepository';
import {createUuid} from 'Util/uuid';

import {ServiceMiddleware} from './ServiceMiddleware';

import {createBaseEvent} from '../../conversation/EventNew';
import {ClientEvent} from '../Client';

function buildServiceMiddleware() {
const selfUser = new User(createUuid());
const conversationRepository = {getConversationById: jest.fn()} as unknown as jest.Mocked<ConversationRepository>;
Expand Down Expand Up @@ -119,7 +122,16 @@ describe('ServiceMiddleware', () => {

it('adds meta when services are present in the event with qualified user ids', async () => {
const [serviceMiddleware, {userRepository}] = buildServiceMiddleware();
const event = EventBuilder.build1to1Creation(conversation);
// const event = EventBuilder.build1to1Creation(conversation);
const event = createBaseEvent<OneToOneCreationEvent>({
conversation,
eventType: ClientEvent.CONVERSATION.ONE2ONE_CREATION,
additionalData: {
userIds: conversation.participating_user_ids(),
},
from: conversation.creator,
timestamp: 0,
});

const service = new User();
service.isService = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
*
*/

import {FEDERATION_EVENT} from '@wireapp/api-client/lib/event';
import {CONVERSATION_EVENT, FEDERATION_EVENT} from '@wireapp/api-client/lib/event';
import {container} from 'tsyringe';
import {debounce} from 'underscore';

import {ConversationState} from 'src/script/conversation/ConversationState';
import {ConversationStatus} from 'src/script/conversation/ConversationStatus';
import {EventBuilder} from 'src/script/conversation/EventBuilder';
import {FederationStopEvent, MemberLeaveEvent} from 'src/script/conversation/EventBuilder';
import {Conversation} from 'src/script/entity/Conversation';
import {User} from 'src/script/entity/User';
import {ServerTimeHandler} from 'src/script/time/serverTimeHandler';
Expand All @@ -33,6 +33,8 @@ import {
getFederationDeleteEventUpdates,
} from './ConversationFederationUtils';

import {createBaseEvent} from '../../../conversation/EventNew';
import {CONVERSATION} from '../../Client';
import {EventProcessor, IncomingEvent} from '../../EventProcessor';
import {EventRepository} from '../../EventRepository';

Expand Down Expand Up @@ -131,7 +133,16 @@ export class FederationEventProcessor implements EventProcessor {

private async insertFederationStopSystemMessage(conversation: Conversation, domains: string[]) {
const currentTimestamp = this.serverTimeHandler.toServerTimestamp();
const event = EventBuilder.buildFederationStop(conversation, this.selfUser, domains, currentTimestamp);
// const event = EventBuilder.buildFederationStop(conversation, this.selfUser, domains, currentTimestamp);
const event = createBaseEvent<FederationStopEvent>({
conversation,
eventType: CONVERSATION.FEDERATION_STOP,
additionalData: {
domains,
},
from: this.selfUser.id,
timestamp: currentTimestamp,
});
await this.eventRepository.injectEvent(event, EventRepository.SOURCE.INJECTED);
}

Expand All @@ -142,12 +153,26 @@ export class FederationEventProcessor implements EventProcessor {
*/
private async removeMembers(conversation: Conversation, users: User[]) {
const currentTimestamp = this.serverTimeHandler.toServerTimestamp();
const event = EventBuilder.buildMemberLeave(
// const event = EventBuilder.buildMemberLeave(
// conversation,
// users.map(user => user.qualifiedId),
// '',
// currentTimestamp,
// );

const userIds = users.map(user => user.qualifiedId);

const event = createBaseEvent<MemberLeaveEvent>({
conversation,
users.map(user => user.qualifiedId),
'',
currentTimestamp,
);
eventType: CONVERSATION_EVENT.MEMBER_LEAVE,
additionalData: {
qualified_user_ids: userIds,
user_ids: userIds.map(({id}) => id),
},
from: '',
timestamp: currentTimestamp,
});

// Injecting the event will trigger all the handlers that will then actually remove the users from the conversation
await this.eventRepository.injectEvent(event);
}
Expand Down
Loading