From f65b654b4573f128610eb4155535a4df51aa18f9 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Mon, 22 Apr 2024 17:50:15 +0200 Subject: [PATCH 01/14] Add delete pad to recursive visitor --- .../infra/etherpad-client/etherpad-client.adapter.ts | 10 ++++++++-- .../infra/etherpad-client/etherpad-client.module.ts | 9 ++++++++- apps/server/src/modules/board/board.module.ts | 2 ++ .../src/modules/board/repo/recursive-delete.vistor.ts | 5 ++++- .../service/collaborative-text-editor.service.ts | 4 ++++ 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts index f73c841c671..b96f86150ed 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts @@ -10,7 +10,7 @@ import { InlineResponse2006, InlineResponse2006Data, } from './etherpad-api-client'; -import { AuthorApi, GroupApi, SessionApi } from './etherpad-api-client/api'; +import { AuthorApi, GroupApi, PadApi, SessionApi } from './etherpad-api-client/api'; import { AuthorId, EtherpadErrorType, EtherpadParams, EtherpadResponse, GroupId, PadId, SessionId } from './interface'; import { EtherpadResponseMapper } from './mappers'; @@ -19,7 +19,8 @@ export class EtherpadClientAdapter { constructor( private readonly groupApi: GroupApi, private readonly sessionApi: SessionApi, - private readonly authorApi: AuthorApi + private readonly authorApi: AuthorApi, + private readonly padApi: PadApi ) {} public async getOrCreateAuthorId(userId: EntityId, username: string): Promise { @@ -188,6 +189,11 @@ export class EtherpadClientAdapter { } } + public async deletePad(padId: PadId): Promise { + await this.padApi.deletePadUsingPOST(padId); + // TODO: Error Mapper + } + private handleResponse( axiosResponse: AxiosResponse, payload: EtherpadParams diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.module.ts b/apps/server/src/infra/etherpad-client/etherpad-client.module.ts index 3230fcf1c48..3368cd1970c 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.module.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.module.ts @@ -1,5 +1,5 @@ import { DynamicModule, Module } from '@nestjs/common'; -import { AuthorApi, GroupApi, SessionApi } from './etherpad-api-client/api'; +import { AuthorApi, GroupApi, PadApi, SessionApi } from './etherpad-api-client/api'; import { Configuration, ConfigurationParameters } from './etherpad-api-client/configuration'; import { EtherpadClientAdapter } from './etherpad-client.adapter'; @@ -34,6 +34,13 @@ export class EtherpadClientModule { return new AuthorApi(configuration); }, }, + { + provide: PadApi, + useFactory: () => { + const configuration = new Configuration(config); + return new PadApi(configuration); + }, + }, ]; return { diff --git a/apps/server/src/modules/board/board.module.ts b/apps/server/src/modules/board/board.module.ts index 959e7eb1650..6dd75961a14 100644 --- a/apps/server/src/modules/board/board.module.ts +++ b/apps/server/src/modules/board/board.module.ts @@ -11,6 +11,7 @@ import { CqrsModule } from '@nestjs/cqrs'; import { ContentElementFactory } from '@shared/domain/domainobject'; import { CourseRepo } from '@shared/repo'; import { LoggerModule } from '@src/core/logger'; +import { CollaborativeTextEditorModule } from '../collaborative-text-editor/collaborative-text-editor.module'; import { BoardDoRepo, BoardNodeRepo, RecursiveDeleteVisitor } from './repo'; import { BoardDoAuthorizableService, @@ -40,6 +41,7 @@ import { ColumnBoardCopyService } from './service/column-board-copy.service'; ToolConfigModule, TldrawClientModule, CqrsModule, + CollaborativeTextEditorModule, ], providers: [ // TODO: move BoardDoAuthorizableService, BoardDoRepo, BoardDoService, BoardNodeRepo in separate module and move mediaboard related services in mediaboard module diff --git a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts index a4b082097bc..b17896dbe9d 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -23,6 +23,7 @@ import type { SubmissionItem, } from '@shared/domain/domainobject'; import { BoardNode } from '@shared/domain/entity'; +import { CollaborativeTextEditorService } from '@src/modules/collaborative-text-editor/service/collaborative-text-editor.service'; @Injectable() export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { @@ -30,7 +31,8 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { private readonly em: EntityManager, private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService, private readonly contextExternalToolService: ContextExternalToolService, - private readonly drawingElementAdapterService: DrawingElementAdapterService + private readonly drawingElementAdapterService: DrawingElementAdapterService, + private readonly collaborativeTextEditorService: CollaborativeTextEditorService ) {} async visitColumnBoardAsync(columnBoard: ColumnBoard): Promise { @@ -104,6 +106,7 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { async visitCollaborativeTextEditorElementAsync( collaborativeTextEditorElement: CollaborativeTextEditorElement ): Promise { + await this.collaborativeTextEditorService.deleteCollaborativeTextEditor(collaborativeTextEditorElement.id); this.deleteNode(collaborativeTextEditorElement); await this.visitChildrenAsync(collaborativeTextEditorElement); } diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts index fa0ea1d3316..eaa100eecca 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts @@ -45,6 +45,10 @@ export class CollaborativeTextEditorService { }; } + async deleteCollaborativeTextEditor(parentId: string): Promise { + await this.collaborativeTextEditorAdapter.deletePad(parentId); + } + private removeDuplicateSessions(sessions: string[]): string[] { const uniqueSessions = [...new Set(sessions)]; From b7f5ddcf0875b11c7a50bbb2498b3300c19f46ed Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Tue, 23 Apr 2024 09:38:34 +0200 Subject: [PATCH 02/14] Add recursive delete visitor test --- .../repo/recursive-delete.visitor.spec.ts | 89 +++++++++++++++---- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts index e4215553641..cb998fbfb39 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts @@ -21,6 +21,7 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; +import { CollaborativeTextEditorService } from '@src/modules/collaborative-text-editor/service/collaborative-text-editor.service'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; describe(RecursiveDeleteVisitor.name, () => { @@ -31,6 +32,7 @@ describe(RecursiveDeleteVisitor.name, () => { let filesStorageClientAdapterService: DeepMocked; let contextExternalToolService: DeepMocked; let drawingElementAdapterService: DeepMocked; + let collaborativeTextEditorService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -40,6 +42,7 @@ describe(RecursiveDeleteVisitor.name, () => { { provide: FilesStorageClientAdapterService, useValue: createMock() }, { provide: ContextExternalToolService, useValue: createMock() }, { provide: DrawingElementAdapterService, useValue: createMock() }, + { provide: CollaborativeTextEditorService, useValue: createMock() }, ], }).compile(); @@ -48,6 +51,7 @@ describe(RecursiveDeleteVisitor.name, () => { filesStorageClientAdapterService = module.get(FilesStorageClientAdapterService); contextExternalToolService = module.get(ContextExternalToolService); drawingElementAdapterService = module.get(DrawingElementAdapterService); + collaborativeTextEditorService = module.get(CollaborativeTextEditorService); await setupEntities(); }); @@ -444,26 +448,81 @@ describe(RecursiveDeleteVisitor.name, () => { }); describe('visitCollaborativeTextEditorAsync', () => { - const setup = () => { - const childCollaborativeTextEditorElement = collaborativeTextEditorElementFactory.build(); - const collaborativeTextEditorElement = collaborativeTextEditorElementFactory.build({ - children: [childCollaborativeTextEditorElement], + describe('WHEN collaborative text editor element is deleted successfully', () => { + const setup = () => { + const childCollaborativeTextEditorElement = collaborativeTextEditorElementFactory.build(); + const collaborativeTextEditorElement = collaborativeTextEditorElementFactory.build({ + children: [childCollaborativeTextEditorElement], + }); + + return { collaborativeTextEditorElement, childCollaborativeTextEditorElement }; + }; + + it('should call deleteCollaborativeTextEditor', async () => { + const { collaborativeTextEditorElement } = setup(); + + await service.visitCollaborativeTextEditorElementAsync(collaborativeTextEditorElement); + + expect(collaborativeTextEditorService.deleteCollaborativeTextEditor).toHaveBeenCalledWith( + collaborativeTextEditorElement.id + ); }); - return { collaborativeTextEditorElement, childCollaborativeTextEditorElement }; - }; + it('should call entity remove', async () => { + const { collaborativeTextEditorElement, childCollaborativeTextEditorElement } = setup(); - it('should call entity remove', async () => { - const { collaborativeTextEditorElement, childCollaborativeTextEditorElement } = setup(); + await service.visitCollaborativeTextEditorElementAsync(collaborativeTextEditorElement); - await service.visitCollaborativeTextEditorElementAsync(collaborativeTextEditorElement); + expect(em.remove).toHaveBeenCalledWith( + em.getReference(collaborativeTextEditorElement.constructor, collaborativeTextEditorElement.id) + ); + expect(em.remove).toHaveBeenCalledWith( + em.getReference(childCollaborativeTextEditorElement.constructor, childCollaborativeTextEditorElement.id) + ); + }); + }); - expect(em.remove).toHaveBeenCalledWith( - em.getReference(collaborativeTextEditorElement.constructor, collaborativeTextEditorElement.id) - ); - expect(em.remove).toHaveBeenCalledWith( - em.getReference(childCollaborativeTextEditorElement.constructor, childCollaborativeTextEditorElement.id) - ); + describe('WHEN deleteCollaborativeTextEditor returns error', () => { + const setup = () => { + const childCollaborativeTextEditorElement = collaborativeTextEditorElementFactory.build(); + const collaborativeTextEditorElement = collaborativeTextEditorElementFactory.build({ + children: [childCollaborativeTextEditorElement], + }); + const error = new Error('testError'); + collaborativeTextEditorService.deleteCollaborativeTextEditor.mockRejectedValueOnce(error); + + return { collaborativeTextEditorElement, childCollaborativeTextEditorElement, error }; + }; + + it('should return error', async () => { + const { collaborativeTextEditorElement, error } = setup(); + + await expect( + service.visitCollaborativeTextEditorElementAsync(collaborativeTextEditorElement) + ).rejects.toThrowError(error); + }); + }); + + describe('WHEN visitChildrenAsync returns error', () => { + const setup = () => { + const childCollaborativeTextEditorElement = collaborativeTextEditorElementFactory.build(); + const collaborativeTextEditorElement = collaborativeTextEditorElementFactory.build({ + children: [childCollaborativeTextEditorElement], + }); + const error = new Error('testError'); + collaborativeTextEditorService.deleteCollaborativeTextEditor.mockResolvedValueOnce(); + collaborativeTextEditorService.deleteCollaborativeTextEditor.mockRejectedValueOnce(error); + + return { collaborativeTextEditorElement, childCollaborativeTextEditorElement, error }; + }; + + it('should return error', async () => { + const { collaborativeTextEditorElement, error } = setup(); + + await expect( + service.visitCollaborativeTextEditorElementAsync(collaborativeTextEditorElement) + ).rejects.toThrowError(error); + }); }); }); }); From a56743052d67f15281bb2db46d7d1e70bba571cb Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Tue, 23 Apr 2024 09:40:11 +0200 Subject: [PATCH 03/14] Add service test --- .../collaborative-text-editor.service.spec.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts index 59945ebcd93..fe3bbd54750 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts @@ -261,4 +261,27 @@ describe('CollaborativeTextEditorService', () => { }); }); }); + + describe('deleteCollaborativeTextEditor', () => { + describe('WHEN etherpadClientAdapter.deletePad returns successfully', () => { + it('should call etherpadClientAdapter.deletePad with correct parameter', async () => { + const parentId = 'parentId'; + + await service.deleteCollaborativeTextEditor(parentId); + + expect(etherpadClientAdapter.deletePad).toHaveBeenCalledWith(parentId); + }); + }); + + describe('WHEN etherpadClientAdapter.deletePad throws an error', () => { + it('should throw an error', async () => { + const parentId = 'parentId'; + const error = new Error('error'); + + etherpadClientAdapter.deletePad.mockRejectedValueOnce(error); + + await expect(service.deleteCollaborativeTextEditor(parentId)).rejects.toThrowError(error); + }); + }); + }); }); From 3daec9db23adc1c1106cc32452be86f96ef32b5f Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Tue, 23 Apr 2024 18:37:05 +0200 Subject: [PATCH 04/14] Improve etherpad response handling & remove unsave casting & add catch to delete --- .../etherpad-client.adapter.spec.ts | 63 ++++++++++++++++++- .../etherpad-client.adapter.ts | 47 ++++++++++---- 2 files changed, 95 insertions(+), 15 deletions(-) diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts index 53e69c3926d..ae06aa3b166 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts @@ -1,4 +1,5 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { AxiosResponse } from 'axios'; import { @@ -10,6 +11,7 @@ import { InlineResponse2003, InlineResponse2004, InlineResponse2006, + PadApi, SessionApi, } from './etherpad-api-client'; import { EtherpadClientAdapter } from './etherpad-client.adapter'; @@ -39,6 +41,10 @@ describe(EtherpadClientAdapter.name, () => { provide: AuthorApi, useValue: createMock(), }, + { + provide: PadApi, + useValue: createMock(), + }, ], }).compile(); @@ -67,6 +73,7 @@ describe(EtherpadClientAdapter.name, () => { const username = 'username'; const response = createMock>({ data: { + code: 0, data: { authorID: 'authorId' }, }, }); @@ -98,6 +105,7 @@ describe(EtherpadClientAdapter.name, () => { const username = 'username'; const response = createMock>({ data: { + code: 0, data: {}, }, }); @@ -142,12 +150,14 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const response = createMock>({ data: { + code: 0, data: { sessionID: 'sessionId' }, }, }); const listSessionsResponse = createMock>({ data: { + code: 0, data: { // @ts-expect-error wrong type mapping 'session-id-1': { groupID: groupId, authorID: authorId }, @@ -187,12 +197,14 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const response = createMock>({ data: { + code: 0, data: { sessionID: 'sessionId' }, }, }); const listSessionsResponse = createMock>({ data: { + code: 0, data: {}, }, }); @@ -231,6 +243,7 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const listSessionsResponse = createMock>({ data: { + code: 0, data: {}, }, }); @@ -239,6 +252,7 @@ describe(EtherpadClientAdapter.name, () => { const response = createMock>({ data: { + code: 0, data: {}, }, }); @@ -264,6 +278,7 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const listSessionsResponse = createMock>({ data: { + code: 0, data: {}, }, }); @@ -290,6 +305,7 @@ describe(EtherpadClientAdapter.name, () => { const authorId = 'authorId'; const response = createMock>({ data: { + code: 0, // @ts-expect-error wrong type mapping data: { 'session-id-1': { groupID: 'groupId', authorID: authorId } }, }, @@ -313,6 +329,7 @@ describe(EtherpadClientAdapter.name, () => { const authorId = 'authorId'; const response = createMock>({ data: { + code: 0, data: {}, }, }); @@ -345,6 +362,30 @@ describe(EtherpadClientAdapter.name, () => { await expect(service.listSessionIdsOfAuthor(authorId)).rejects.toThrowError(EtherpadErrorLoggableException); }); }); + + describe('when InlineResponse2006Data is not an object', () => { + const setup = () => { + const authorId = 'authorId'; + const response = createMock>({ + data: { + code: 0, + // @ts-expect-error wrong type mapping + data: [], + }, + }); + + authorApi.listSessionsOfAuthorUsingGET.mockResolvedValue(response); + return authorId; + }; + + it('should throw an error', async () => { + const authorId = setup(); + + await expect(service.listSessionIdsOfAuthor(authorId)).rejects.toThrowError( + 'Etherpad session ids response is not an object' + ); + }); + }); }); describe('getOrCreateGroupId', () => { @@ -353,6 +394,7 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const response = createMock>({ data: { + code: 0, data: { groupID: 'groupId' }, }, }); @@ -382,7 +424,7 @@ describe(EtherpadClientAdapter.name, () => { const setup = () => { const parentId = 'parentId'; const response = createMock>({ - data: {}, + data: { code: 0 }, }); groupApi.createGroupIfNotExistsForUsingGET.mockResolvedValue(response); @@ -420,12 +462,14 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const response = createMock>({ data: { + code: 0, data: { padID: 'padId' }, }, }); const listPadsResponse = createMock>({ data: { + code: 0, data: { padIDs: [] }, }, }); @@ -458,6 +502,7 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const response = createMock>({ data: { + code: 0, data: { padIDs: ['groupId$parentId'] }, }, }); @@ -489,11 +534,13 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const listPadsResponse = createMock>({ data: { + code: 0, data: { padIDs: [] }, }, }); const response = createMock>({ data: { + code: 0, data: {}, }, }); @@ -516,6 +563,7 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const listPadsResponse = createMock>({ data: { + code: 0, data: { padIDs: [] }, }, }); @@ -555,7 +603,7 @@ describe(EtherpadClientAdapter.name, () => { }); }); - describe('handleResponse', () => { + describe('handleEtherpadResponse', () => { const setup = (code = 0) => { const parentId = 'parentId'; const response = createMock>({ @@ -622,5 +670,16 @@ describe(EtherpadClientAdapter.name, () => { await expect(result).rejects.toThrowError(EtherpadErrorLoggableException); }); }); + + describe('when status code is other than known code', () => { + it('should throw an error', async () => { + const parentId = setup(5); + + const result = service.getOrCreateGroupId(parentId); + + const exception = new InternalServerErrorException('Etherpad Response Code unknown'); + await expect(result).rejects.toThrowError(exception); + }); + }); }); }); diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts index b96f86150ed..969d5c9cd2f 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, InternalServerErrorException } from '@nestjs/common'; import { EntityId } from '@shared/domain/types'; import { AxiosResponse } from 'axios'; import { @@ -25,7 +25,7 @@ export class EtherpadClientAdapter { public async getOrCreateAuthorId(userId: EntityId, username: string): Promise { const response = await this.tryCreateAuthor(userId, username); - const user = this.handleResponse(response, { userId }); + const user = this.handleEtherpadResponse(response, { userId }); const authorId = EtherpadResponseMapper.mapToAuthorResponse(user); @@ -56,7 +56,7 @@ export class EtherpadClientAdapter { } const response = await this.tryCreateSession(groupId, authorId, sessionCookieExpire); - const session = this.handleResponse(response, { parentId }); + const session = this.handleEtherpadResponse(response, { parentId }); sessionId = EtherpadResponseMapper.mapToSessionResponse(session); @@ -85,7 +85,7 @@ export class EtherpadClientAdapter { let sessionId: SessionId | undefined; const response = await this.tryListSessionsOfAuthor(authorId); - const sessions = this.handleResponse(response, { groupId, authorId }); + const sessions = this.handleEtherpadResponse(response, { groupId, authorId }); if (sessions) { sessionId = this.findSessionId(sessions, groupId, authorId); @@ -109,13 +109,22 @@ export class EtherpadClientAdapter { public async listSessionIdsOfAuthor(authorId: AuthorId): Promise { const response = await this.tryListSessionsOfAuthor(authorId); - const sessions = this.handleResponse(response, { authorId }); + const sessions = this.handleEtherpadResponse(response, { authorId }); - const sessionIds = Object.keys(sessions as object); + // InlineResponse2006Data has the wrong type definition. Therefore we savely cast it to an object. + if (!this.isObject(sessions)) { + throw new InternalServerErrorException('Etherpad session ids response is not an object'); + } + + const sessionIds = Object.keys(sessions); return sessionIds; } + private isObject(value: any): value is object { + return typeof value === 'object' && !Array.isArray(value) && value !== null; + } + private async tryListSessionsOfAuthor(authorId: string): Promise> { try { const response = await this.authorApi.listSessionsOfAuthorUsingGET(authorId); @@ -128,7 +137,7 @@ export class EtherpadClientAdapter { public async getOrCreateGroupId(parentId: EntityId): Promise { const groupResponse = await this.tryGetOrCreateGroup(parentId); - const group = this.handleResponse(groupResponse, { parentId }); + const group = this.handleEtherpadResponse(groupResponse, { parentId }); const groupId = EtherpadResponseMapper.mapToGroupResponse(group); @@ -152,7 +161,7 @@ export class EtherpadClientAdapter { if (!padId) { const padResponse = await this.tryCreateEtherpad(groupId, parentId); - const pad = this.handleResponse(padResponse, { parentId }); + const pad = this.handleEtherpadResponse(padResponse, { parentId }); padId = EtherpadResponseMapper.mapToPadResponse(pad); } @@ -172,7 +181,7 @@ export class EtherpadClientAdapter { private async getPadId(groupId: GroupId, parentId: EntityId): Promise { const padsResponse = await this.tryListPads(groupId); - const pads = this.handleResponse(padsResponse, { parentId }); + const pads = this.handleEtherpadResponse(padsResponse, { parentId }); const padId = pads?.padIDs?.find((id: string) => id.includes(`${groupId}$${parentId}`)); @@ -190,17 +199,29 @@ export class EtherpadClientAdapter { } public async deletePad(padId: PadId): Promise { - await this.padApi.deletePadUsingPOST(padId); - // TODO: Error Mapper + const response = await this.tryDeletePad(padId); + this.handleEtherpadResponse(response, { padId }); + } + + private async tryDeletePad(padId: string): Promise> { + try { + const response = await this.padApi.deletePadUsingPOST(padId); + + return response; + } catch (error) { + throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.CONNECTION_ERROR, { padId }, error); + } } - private handleResponse( + private handleEtherpadResponse( axiosResponse: AxiosResponse, payload: EtherpadParams ): T['data'] { const response = axiosResponse.data; switch (response.code) { + case 0: + return response.data; case 1: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.BAD_REQUEST, payload, response); case 2: @@ -210,7 +231,7 @@ export class EtherpadClientAdapter { case 4: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.WRONG_API_KEY, payload, response); default: - return response.data as T['data']; + throw new InternalServerErrorException('Etherpad Response Code unknown'); } } } From 572674ad6f7a282e7a5c9d40cf0a5bc176c3eda7 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 07:41:14 +0200 Subject: [PATCH 05/14] Add etherpad response code enum --- .../etherpad-client.adapter.ts | 21 +++++++++++++------ .../interface/etherpad-response-code.enum.ts | 8 +++++++ .../infra/etherpad-client/interface/index.ts | 1 + 3 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 apps/server/src/infra/etherpad-client/interface/etherpad-response-code.enum.ts diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts index 969d5c9cd2f..564d48da92e 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts @@ -11,7 +11,16 @@ import { InlineResponse2006Data, } from './etherpad-api-client'; import { AuthorApi, GroupApi, PadApi, SessionApi } from './etherpad-api-client/api'; -import { AuthorId, EtherpadErrorType, EtherpadParams, EtherpadResponse, GroupId, PadId, SessionId } from './interface'; +import { + AuthorId, + EtherpadErrorType, + EtherpadParams, + EtherpadResponse, + EtherpadResponseCode, + GroupId, + PadId, + SessionId, +} from './interface'; import { EtherpadResponseMapper } from './mappers'; @Injectable() @@ -220,15 +229,15 @@ export class EtherpadClientAdapter { const response = axiosResponse.data; switch (response.code) { - case 0: + case EtherpadResponseCode.OK: return response.data; - case 1: + case EtherpadResponseCode.BAD_REQUEST: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.BAD_REQUEST, payload, response); - case 2: + case EtherpadResponseCode.INTERNAL_ERROR: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.INTERNAL_ERROR, payload, response); - case 3: + case EtherpadResponseCode.FUNCTION_NOT_FOUND: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.FUNCTION_NOT_FOUND, payload, response); - case 4: + case EtherpadResponseCode.WRONG_API_KEY: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.WRONG_API_KEY, payload, response); default: throw new InternalServerErrorException('Etherpad Response Code unknown'); diff --git a/apps/server/src/infra/etherpad-client/interface/etherpad-response-code.enum.ts b/apps/server/src/infra/etherpad-client/interface/etherpad-response-code.enum.ts new file mode 100644 index 00000000000..ed17c5e46f8 --- /dev/null +++ b/apps/server/src/infra/etherpad-client/interface/etherpad-response-code.enum.ts @@ -0,0 +1,8 @@ +// For more information please see etherpad documentation +export enum EtherpadResponseCode { + OK = 0, + BAD_REQUEST = 1, + INTERNAL_ERROR = 2, + FUNCTION_NOT_FOUND = 3, + WRONG_API_KEY = 4, +} diff --git a/apps/server/src/infra/etherpad-client/interface/index.ts b/apps/server/src/infra/etherpad-client/interface/index.ts index e087de15d39..fd428f19f74 100644 --- a/apps/server/src/infra/etherpad-client/interface/index.ts +++ b/apps/server/src/infra/etherpad-client/interface/index.ts @@ -1,2 +1,3 @@ export * from './etherpad-error-type.enum'; +export * from './etherpad-response-code.enum'; export * from './payloads.interface'; From 878aedf0def63354ed17add0fffa46a4e371680d Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 08:32:31 +0200 Subject: [PATCH 06/14] Add CollaborativeTextEditorService dependency to BoardDoRepo --- apps/server/src/modules/board/repo/board-do.repo.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/server/src/modules/board/repo/board-do.repo.spec.ts b/apps/server/src/modules/board/repo/board-do.repo.spec.ts index 2e18e9cdf9e..e178975244e 100644 --- a/apps/server/src/modules/board/repo/board-do.repo.spec.ts +++ b/apps/server/src/modules/board/repo/board-do.repo.spec.ts @@ -42,6 +42,7 @@ import { richTextElementFactory, richTextElementNodeFactory, } from '@shared/testing'; +import { CollaborativeTextEditorService } from '@src/modules/collaborative-text-editor/service/collaborative-text-editor.service'; import { ContextExternalTool } from '../../tool/context-external-tool/domain'; import { ContextExternalToolEntity } from '../../tool/context-external-tool/entity'; import { BoardDoRepo } from './board-do.repo'; @@ -65,6 +66,7 @@ describe(BoardDoRepo.name, () => { { provide: FilesStorageClientAdapterService, useValue: createMock() }, { provide: ContextExternalToolService, useValue: createMock() }, { provide: DrawingElementAdapterService, useValue: createMock() }, + { provide: CollaborativeTextEditorService, useValue: createMock() }, ], }).compile(); repo = module.get(BoardDoRepo); From cb7bd764a251ba8c69eb4de8bf5edd8e4474f75d Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 12:43:14 +0200 Subject: [PATCH 07/14] Add adapter test --- .../etherpad-client.adapter.spec.ts | 69 ++++++++++++++++++- .../etherpad-client.adapter.ts | 2 +- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts index ae06aa3b166..006cd5157d0 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts @@ -15,7 +15,7 @@ import { SessionApi, } from './etherpad-api-client'; import { EtherpadClientAdapter } from './etherpad-client.adapter'; -import { EtherpadErrorType } from './interface'; +import { EtherpadErrorType, EtherpadResponseCode } from './interface'; import { EtherpadErrorLoggableException } from './loggable'; describe(EtherpadClientAdapter.name, () => { @@ -24,6 +24,7 @@ describe(EtherpadClientAdapter.name, () => { let groupApi: DeepMocked; let sessionApi: DeepMocked; let authorApi: DeepMocked; + let padApi: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -52,6 +53,7 @@ describe(EtherpadClientAdapter.name, () => { sessionApi = module.get(SessionApi); authorApi = module.get(AuthorApi); groupApi = module.get(GroupApi); + padApi = module.get(PadApi); }); afterAll(async () => { @@ -603,6 +605,69 @@ describe(EtherpadClientAdapter.name, () => { }); }); + describe('deletePad', () => { + describe('when deletePadUsingPOST returns successfull', () => { + const setup = () => { + const padId = 'padId'; + const response = createMock>({ + data: { + code: EtherpadResponseCode.OK, + data: {}, + }, + }); + + padApi.deletePadUsingPOST.mockResolvedValue(response); + return padId; + }; + + it('should call deletePadUsingGET with correct params', async () => { + const padId = setup(); + + await service.deletePad(padId); + + expect(padApi.deletePadUsingPOST).toBeCalledWith(padId); + }); + }); + + describe('when deletePadUsingPOST returns etherpad error code', () => { + const setup = () => { + const padId = 'padId'; + const response = createMock>({ + data: { + code: EtherpadResponseCode.BAD_REQUEST, + data: {}, + }, + }); + + padApi.deletePadUsingPOST.mockResolvedValue(response); + return padId; + }; + + it('should throw EtherpadErrorLoggableException', async () => { + const padId = setup(); + + const exception = new EtherpadErrorLoggableException(EtherpadErrorType.BAD_REQUEST, { padId }, {}); + await expect(service.deletePad(padId)).rejects.toThrowError(exception); + }); + }); + + describe('when deletePadUsingGET returns error', () => { + const setup = () => { + const padId = 'padId'; + + padApi.deletePadUsingPOST.mockRejectedValueOnce(new Error('error')); + + return padId; + }; + + it('should throw EtherpadErrorLoggableException', async () => { + const padId = setup(); + + await expect(service.deletePad(padId)).rejects.toThrowError(EtherpadErrorLoggableException); + }); + }); + }); + describe('handleEtherpadResponse', () => { const setup = (code = 0) => { const parentId = 'parentId'; @@ -677,7 +742,7 @@ describe(EtherpadClientAdapter.name, () => { const result = service.getOrCreateGroupId(parentId); - const exception = new InternalServerErrorException('Etherpad Response Code unknown'); + const exception = new InternalServerErrorException('Etherpad response code unknown'); await expect(result).rejects.toThrowError(exception); }); }); diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts index 564d48da92e..464d82bcea0 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts @@ -240,7 +240,7 @@ export class EtherpadClientAdapter { case EtherpadResponseCode.WRONG_API_KEY: throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.WRONG_API_KEY, payload, response); default: - throw new InternalServerErrorException('Etherpad Response Code unknown'); + throw new InternalServerErrorException('Etherpad response code unknown'); } } } From 5e43dbe2cafd5f6fec37dc5e1a935c09321df0e7 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 14:30:43 +0200 Subject: [PATCH 08/14] Delete group and not pad --- .../etherpad-client.adapter.spec.ts | 51 +++++++++---------- .../etherpad-client.adapter.ts | 19 ++++--- .../collaborative-text-editor.service.ts | 4 +- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts index 006cd5157d0..27074ea5821 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts @@ -11,7 +11,6 @@ import { InlineResponse2003, InlineResponse2004, InlineResponse2006, - PadApi, SessionApi, } from './etherpad-api-client'; import { EtherpadClientAdapter } from './etherpad-client.adapter'; @@ -24,7 +23,6 @@ describe(EtherpadClientAdapter.name, () => { let groupApi: DeepMocked; let sessionApi: DeepMocked; let authorApi: DeepMocked; - let padApi: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -42,10 +40,6 @@ describe(EtherpadClientAdapter.name, () => { provide: AuthorApi, useValue: createMock(), }, - { - provide: PadApi, - useValue: createMock(), - }, ], }).compile(); @@ -53,7 +47,6 @@ describe(EtherpadClientAdapter.name, () => { sessionApi = module.get(SessionApi); authorApi = module.get(AuthorApi); groupApi = module.get(GroupApi); - padApi = module.get(PadApi); }); afterAll(async () => { @@ -605,10 +598,10 @@ describe(EtherpadClientAdapter.name, () => { }); }); - describe('deletePad', () => { - describe('when deletePadUsingPOST returns successfull', () => { + describe('deleteGroup', () => { + describe('when deleteGroupUsingPOST returns successfull', () => { const setup = () => { - const padId = 'padId'; + const groupId = 'groupId'; const response = createMock>({ data: { code: EtherpadResponseCode.OK, @@ -616,22 +609,23 @@ describe(EtherpadClientAdapter.name, () => { }, }); - padApi.deletePadUsingPOST.mockResolvedValue(response); - return padId; + groupApi.deleteGroupUsingPOST.mockResolvedValue(response); + + return groupId; }; it('should call deletePadUsingGET with correct params', async () => { - const padId = setup(); + const groupId = setup(); - await service.deletePad(padId); + await service.deleteGroup(groupId); - expect(padApi.deletePadUsingPOST).toBeCalledWith(padId); + expect(groupApi.deleteGroupUsingPOST).toBeCalledWith(groupId); }); }); - describe('when deletePadUsingPOST returns etherpad error code', () => { + describe('when deleteGroupUsingPOST returns etherpad error code', () => { const setup = () => { - const padId = 'padId'; + const groupId = 'groupId'; const response = createMock>({ data: { code: EtherpadResponseCode.BAD_REQUEST, @@ -639,31 +633,32 @@ describe(EtherpadClientAdapter.name, () => { }, }); - padApi.deletePadUsingPOST.mockResolvedValue(response); - return padId; + groupApi.deleteGroupUsingPOST.mockResolvedValue(response); + + return groupId; }; it('should throw EtherpadErrorLoggableException', async () => { - const padId = setup(); + const groupId = setup(); - const exception = new EtherpadErrorLoggableException(EtherpadErrorType.BAD_REQUEST, { padId }, {}); - await expect(service.deletePad(padId)).rejects.toThrowError(exception); + const exception = new EtherpadErrorLoggableException(EtherpadErrorType.BAD_REQUEST, { padId: groupId }, {}); + await expect(service.deleteGroup(groupId)).rejects.toThrowError(exception); }); }); - describe('when deletePadUsingGET returns error', () => { + describe('when deleteGroupUsingPOST returns error', () => { const setup = () => { - const padId = 'padId'; + const groupId = 'padId'; - padApi.deletePadUsingPOST.mockRejectedValueOnce(new Error('error')); + groupApi.deleteGroupUsingPOST.mockRejectedValueOnce(new Error('error')); - return padId; + return groupId; }; it('should throw EtherpadErrorLoggableException', async () => { - const padId = setup(); + const groupId = setup(); - await expect(service.deletePad(padId)).rejects.toThrowError(EtherpadErrorLoggableException); + await expect(service.deleteGroup(groupId)).rejects.toThrowError(EtherpadErrorLoggableException); }); }); }); diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts index 464d82bcea0..58d02502cfe 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.ts @@ -10,7 +10,7 @@ import { InlineResponse2006, InlineResponse2006Data, } from './etherpad-api-client'; -import { AuthorApi, GroupApi, PadApi, SessionApi } from './etherpad-api-client/api'; +import { AuthorApi, GroupApi, SessionApi } from './etherpad-api-client/api'; import { AuthorId, EtherpadErrorType, @@ -28,8 +28,7 @@ export class EtherpadClientAdapter { constructor( private readonly groupApi: GroupApi, private readonly sessionApi: SessionApi, - private readonly authorApi: AuthorApi, - private readonly padApi: PadApi + private readonly authorApi: AuthorApi ) {} public async getOrCreateAuthorId(userId: EntityId, username: string): Promise { @@ -106,7 +105,7 @@ export class EtherpadClientAdapter { private findSessionId(sessions: InlineResponse2006Data, groupId: string, authorId: string): string | undefined { const sessionEntries = Object.entries(sessions); const sessionId = sessionEntries.map(([key, value]: [string, { groupID: string; authorID: string }]) => { - if (value.groupID === groupId && value.authorID === authorId) { + if (value?.groupID === groupId && value?.authorID === authorId) { return key; } @@ -207,18 +206,18 @@ export class EtherpadClientAdapter { } } - public async deletePad(padId: PadId): Promise { - const response = await this.tryDeletePad(padId); - this.handleEtherpadResponse(response, { padId }); + public async deleteGroup(groupId: GroupId): Promise { + const response = await this.tryDeleteGroup(groupId); + this.handleEtherpadResponse(response, { groupId }); } - private async tryDeletePad(padId: string): Promise> { + private async tryDeleteGroup(groupId: string): Promise> { try { - const response = await this.padApi.deletePadUsingPOST(padId); + const response = await this.groupApi.deleteGroupUsingPOST(groupId); return response; } catch (error) { - throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.CONNECTION_ERROR, { padId }, error); + throw EtherpadResponseMapper.mapResponseToException(EtherpadErrorType.CONNECTION_ERROR, { groupId }, error); } } diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts index eaa100eecca..35d190b841e 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts @@ -46,7 +46,9 @@ export class CollaborativeTextEditorService { } async deleteCollaborativeTextEditor(parentId: string): Promise { - await this.collaborativeTextEditorAdapter.deletePad(parentId); + const groupId = await this.collaborativeTextEditorAdapter.getOrCreateGroupId(parentId); + + await this.collaborativeTextEditorAdapter.deleteGroup(groupId); } private removeDuplicateSessions(sessions: string[]): string[] { From 7a334b804387fc7631d5d951bd16ce55e227e0b8 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 14:34:22 +0200 Subject: [PATCH 09/14] Remove padApi from client module --- .../src/infra/etherpad-client/etherpad-client.module.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.module.ts b/apps/server/src/infra/etherpad-client/etherpad-client.module.ts index 3368cd1970c..3230fcf1c48 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.module.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.module.ts @@ -1,5 +1,5 @@ import { DynamicModule, Module } from '@nestjs/common'; -import { AuthorApi, GroupApi, PadApi, SessionApi } from './etherpad-api-client/api'; +import { AuthorApi, GroupApi, SessionApi } from './etherpad-api-client/api'; import { Configuration, ConfigurationParameters } from './etherpad-api-client/configuration'; import { EtherpadClientAdapter } from './etherpad-client.adapter'; @@ -34,13 +34,6 @@ export class EtherpadClientModule { return new AuthorApi(configuration); }, }, - { - provide: PadApi, - useFactory: () => { - const configuration = new Configuration(config); - return new PadApi(configuration); - }, - }, ]; return { From 06b992c578d6c4f1f8d936ee1288b331806055e5 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 14:40:53 +0200 Subject: [PATCH 10/14] Use EtherpadResponseCode in tests --- .../etherpad-client.adapter.spec.ts | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts index 27074ea5821..546cf74f2e5 100644 --- a/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts +++ b/apps/server/src/infra/etherpad-client/etherpad-client.adapter.spec.ts @@ -68,7 +68,7 @@ describe(EtherpadClientAdapter.name, () => { const username = 'username'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { authorID: 'authorId' }, }, }); @@ -100,7 +100,7 @@ describe(EtherpadClientAdapter.name, () => { const username = 'username'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -145,14 +145,14 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { sessionID: 'sessionId' }, }, }); const listSessionsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { // @ts-expect-error wrong type mapping 'session-id-1': { groupID: groupId, authorID: authorId }, @@ -192,14 +192,14 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { sessionID: 'sessionId' }, }, }); const listSessionsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -238,7 +238,7 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const listSessionsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -247,7 +247,7 @@ describe(EtherpadClientAdapter.name, () => { const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -273,7 +273,7 @@ describe(EtherpadClientAdapter.name, () => { const sessionCookieExpire = new Date(); const listSessionsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -300,7 +300,7 @@ describe(EtherpadClientAdapter.name, () => { const authorId = 'authorId'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, // @ts-expect-error wrong type mapping data: { 'session-id-1': { groupID: 'groupId', authorID: authorId } }, }, @@ -324,7 +324,7 @@ describe(EtherpadClientAdapter.name, () => { const authorId = 'authorId'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -363,7 +363,7 @@ describe(EtherpadClientAdapter.name, () => { const authorId = 'authorId'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, // @ts-expect-error wrong type mapping data: [], }, @@ -389,7 +389,7 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { groupID: 'groupId' }, }, }); @@ -419,7 +419,7 @@ describe(EtherpadClientAdapter.name, () => { const setup = () => { const parentId = 'parentId'; const response = createMock>({ - data: { code: 0 }, + data: { code: EtherpadResponseCode.OK }, }); groupApi.createGroupIfNotExistsForUsingGET.mockResolvedValue(response); @@ -457,14 +457,14 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { padID: 'padId' }, }, }); const listPadsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { padIDs: [] }, }, }); @@ -497,7 +497,7 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { padIDs: ['groupId$parentId'] }, }, }); @@ -529,13 +529,13 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const listPadsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { padIDs: [] }, }, }); const response = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: {}, }, }); @@ -558,7 +558,7 @@ describe(EtherpadClientAdapter.name, () => { const parentId = 'parentId'; const listPadsResponse = createMock>({ data: { - code: 0, + code: EtherpadResponseCode.OK, data: { padIDs: [] }, }, }); @@ -664,7 +664,7 @@ describe(EtherpadClientAdapter.name, () => { }); describe('handleEtherpadResponse', () => { - const setup = (code = 0) => { + const setup = (code: number) => { const parentId = 'parentId'; const response = createMock>({ data: { @@ -677,9 +677,9 @@ describe(EtherpadClientAdapter.name, () => { return parentId; }; - describe('wehn status code is 0', () => { + describe('wehn status code is EtherpadResponseCode.OK', () => { it('should return data', async () => { - const parentId = setup(); + const parentId = setup(EtherpadResponseCode.OK); const result = await service.getOrCreateGroupId(parentId); @@ -687,9 +687,9 @@ describe(EtherpadClientAdapter.name, () => { }); }); - describe('when status code is 1', () => { + describe('when status code is BAD_REQUEST', () => { it('should throw an error', async () => { - const parentId = setup(1); + const parentId = setup(EtherpadResponseCode.BAD_REQUEST); const result = service.getOrCreateGroupId(parentId); @@ -698,9 +698,9 @@ describe(EtherpadClientAdapter.name, () => { }); }); - describe('when status code is 2', () => { + describe('when status code is INTERNAL_ERROR', () => { it('should throw an error', async () => { - const parentId = setup(2); + const parentId = setup(EtherpadResponseCode.INTERNAL_ERROR); const result = service.getOrCreateGroupId(parentId); @@ -709,9 +709,9 @@ describe(EtherpadClientAdapter.name, () => { }); }); - describe('when status code is 3', () => { + describe('when status code is FUNCTION_NOT_FOUND', () => { it('should throw an error', async () => { - const parentId = setup(3); + const parentId = setup(EtherpadResponseCode.FUNCTION_NOT_FOUND); const result = service.getOrCreateGroupId(parentId); @@ -720,9 +720,9 @@ describe(EtherpadClientAdapter.name, () => { }); }); - describe('when status code is 4', () => { + describe('when status code is WRONG_API_KEY', () => { it('should throw an error', async () => { - const parentId = setup(4); + const parentId = setup(EtherpadResponseCode.WRONG_API_KEY); const result = service.getOrCreateGroupId(parentId); From a5edd8125c6d2c189449c2401601efc11c8bcd5f Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 15:41:40 +0200 Subject: [PATCH 11/14] Fix service test --- .../service/collaborative-text-editor.service.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts index fe3bbd54750..642650870ef 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts @@ -263,22 +263,22 @@ describe('CollaborativeTextEditorService', () => { }); describe('deleteCollaborativeTextEditor', () => { - describe('WHEN etherpadClientAdapter.deletePad returns successfully', () => { + describe('WHEN etherpadClientAdapter.deleteGroup returns successfully', () => { it('should call etherpadClientAdapter.deletePad with correct parameter', async () => { const parentId = 'parentId'; await service.deleteCollaborativeTextEditor(parentId); - expect(etherpadClientAdapter.deletePad).toHaveBeenCalledWith(parentId); + expect(etherpadClientAdapter.deleteGroup).toHaveBeenCalledWith(parentId); }); }); - describe('WHEN etherpadClientAdapter.deletePad throws an error', () => { + describe('WHEN etherpadClientAdapter.deleteGroup throws an error', () => { it('should throw an error', async () => { const parentId = 'parentId'; const error = new Error('error'); - etherpadClientAdapter.deletePad.mockRejectedValueOnce(error); + etherpadClientAdapter.deleteGroup.mockRejectedValueOnce(error); await expect(service.deleteCollaborativeTextEditor(parentId)).rejects.toThrowError(error); }); From 70aefd1b2ea89c222c7d907a68cb2cd268d1d44b Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Wed, 24 Apr 2024 15:58:22 +0200 Subject: [PATCH 12/14] Fix service test --- .../collaborative-text-editor.service.spec.ts | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts index 642650870ef..6ddd83049af 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts @@ -264,22 +264,56 @@ describe('CollaborativeTextEditorService', () => { describe('deleteCollaborativeTextEditor', () => { describe('WHEN etherpadClientAdapter.deleteGroup returns successfully', () => { - it('should call etherpadClientAdapter.deletePad with correct parameter', async () => { + const setup = () => { const parentId = 'parentId'; + const groupId = 'groupId'; + + etherpadClientAdapter.getOrCreateGroupId.mockResolvedValueOnce(groupId); + + return { parentId, groupId }; + }; + + it('should call etherpadClientAdapter.getOrCreateGroupId with correct parameter', async () => { + const { parentId } = setup(); await service.deleteCollaborativeTextEditor(parentId); - expect(etherpadClientAdapter.deleteGroup).toHaveBeenCalledWith(parentId); + expect(etherpadClientAdapter.getOrCreateGroupId).toHaveBeenCalledWith(parentId); }); }); - describe('WHEN etherpadClientAdapter.deleteGroup throws an error', () => { + describe('WHEN etherpadClientAdapter.getOrCreateGroupId throws an error', () => { + const setup = () => { + const parentId = 'parentId'; + const error = new Error('error'); + + etherpadClientAdapter.getOrCreateGroupId.mockRejectedValueOnce(error); + + return { parentId, error }; + }; + it('should throw an error', async () => { + const { parentId, error } = setup(); + + await expect(service.deleteCollaborativeTextEditor(parentId)).rejects.toThrowError(error); + }); + }); + + describe('WHEN etherpadClientAdapter.deleteGroup throws an error', () => { + const setup = () => { const parentId = 'parentId'; + const groupId = 'groupId'; const error = new Error('error'); + etherpadClientAdapter.getOrCreateGroupId.mockResolvedValueOnce(groupId); etherpadClientAdapter.deleteGroup.mockRejectedValueOnce(error); + return { parentId, groupId, error }; + }; + + it('should throw an error', async () => { + const { parentId, error } = setup(); + await expect(service.deleteCollaborativeTextEditor(parentId)).rejects.toThrowError(error); }); }); From 348de857ad5aef206f620c6fa5bc03093f378392 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Thu, 25 Apr 2024 10:29:24 +0200 Subject: [PATCH 13/14] Rename service function to deleteCollaborativeTextEditorByParentId --- .../board/repo/recursive-delete.visitor.spec.ts | 12 ++++++------ .../modules/board/repo/recursive-delete.vistor.ts | 4 +++- .../collaborative-text-editor.service.spec.ts | 8 ++++---- .../service/collaborative-text-editor.service.ts | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts index cb998fbfb39..43d71124f05 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts @@ -458,12 +458,12 @@ describe(RecursiveDeleteVisitor.name, () => { return { collaborativeTextEditorElement, childCollaborativeTextEditorElement }; }; - it('should call deleteCollaborativeTextEditor', async () => { + it('should call deleteCollaborativeTextEditorByParentId', async () => { const { collaborativeTextEditorElement } = setup(); await service.visitCollaborativeTextEditorElementAsync(collaborativeTextEditorElement); - expect(collaborativeTextEditorService.deleteCollaborativeTextEditor).toHaveBeenCalledWith( + expect(collaborativeTextEditorService.deleteCollaborativeTextEditorByParentId).toHaveBeenCalledWith( collaborativeTextEditorElement.id ); }); @@ -482,14 +482,14 @@ describe(RecursiveDeleteVisitor.name, () => { }); }); - describe('WHEN deleteCollaborativeTextEditor returns error', () => { + describe('WHEN deleteCollaborativeTextEditorByParentId returns error', () => { const setup = () => { const childCollaborativeTextEditorElement = collaborativeTextEditorElementFactory.build(); const collaborativeTextEditorElement = collaborativeTextEditorElementFactory.build({ children: [childCollaborativeTextEditorElement], }); const error = new Error('testError'); - collaborativeTextEditorService.deleteCollaborativeTextEditor.mockRejectedValueOnce(error); + collaborativeTextEditorService.deleteCollaborativeTextEditorByParentId.mockRejectedValueOnce(error); return { collaborativeTextEditorElement, childCollaborativeTextEditorElement, error }; }; @@ -510,8 +510,8 @@ describe(RecursiveDeleteVisitor.name, () => { children: [childCollaborativeTextEditorElement], }); const error = new Error('testError'); - collaborativeTextEditorService.deleteCollaborativeTextEditor.mockResolvedValueOnce(); - collaborativeTextEditorService.deleteCollaborativeTextEditor.mockRejectedValueOnce(error); + collaborativeTextEditorService.deleteCollaborativeTextEditorByParentId.mockResolvedValueOnce(); + collaborativeTextEditorService.deleteCollaborativeTextEditorByParentId.mockRejectedValueOnce(error); return { collaborativeTextEditorElement, childCollaborativeTextEditorElement, error }; }; diff --git a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts index b17896dbe9d..ac54aa4a1c8 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -106,7 +106,9 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { async visitCollaborativeTextEditorElementAsync( collaborativeTextEditorElement: CollaborativeTextEditorElement ): Promise { - await this.collaborativeTextEditorService.deleteCollaborativeTextEditor(collaborativeTextEditorElement.id); + await this.collaborativeTextEditorService.deleteCollaborativeTextEditorByParentId( + collaborativeTextEditorElement.id + ); this.deleteNode(collaborativeTextEditorElement); await this.visitChildrenAsync(collaborativeTextEditorElement); } diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts index 6ddd83049af..a3df8602161 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.spec.ts @@ -262,7 +262,7 @@ describe('CollaborativeTextEditorService', () => { }); }); - describe('deleteCollaborativeTextEditor', () => { + describe('deleteCollaborativeTextEditorByParentId', () => { describe('WHEN etherpadClientAdapter.deleteGroup returns successfully', () => { const setup = () => { const parentId = 'parentId'; @@ -276,7 +276,7 @@ describe('CollaborativeTextEditorService', () => { it('should call etherpadClientAdapter.getOrCreateGroupId with correct parameter', async () => { const { parentId } = setup(); - await service.deleteCollaborativeTextEditor(parentId); + await service.deleteCollaborativeTextEditorByParentId(parentId); expect(etherpadClientAdapter.getOrCreateGroupId).toHaveBeenCalledWith(parentId); }); @@ -295,7 +295,7 @@ describe('CollaborativeTextEditorService', () => { it('should throw an error', async () => { const { parentId, error } = setup(); - await expect(service.deleteCollaborativeTextEditor(parentId)).rejects.toThrowError(error); + await expect(service.deleteCollaborativeTextEditorByParentId(parentId)).rejects.toThrowError(error); }); }); @@ -314,7 +314,7 @@ describe('CollaborativeTextEditorService', () => { it('should throw an error', async () => { const { parentId, error } = setup(); - await expect(service.deleteCollaborativeTextEditor(parentId)).rejects.toThrowError(error); + await expect(service.deleteCollaborativeTextEditorByParentId(parentId)).rejects.toThrowError(error); }); }); }); diff --git a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts index 35d190b841e..38aae97cff5 100644 --- a/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts +++ b/apps/server/src/modules/collaborative-text-editor/service/collaborative-text-editor.service.ts @@ -45,7 +45,7 @@ export class CollaborativeTextEditorService { }; } - async deleteCollaborativeTextEditor(parentId: string): Promise { + async deleteCollaborativeTextEditorByParentId(parentId: string): Promise { const groupId = await this.collaborativeTextEditorAdapter.getOrCreateGroupId(parentId); await this.collaborativeTextEditorAdapter.deleteGroup(groupId); From ee6debfb803c31972a7b0ec76783520c1f673989 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Thu, 25 Apr 2024 13:25:56 +0200 Subject: [PATCH 14/14] Fix module imports --- apps/server/src/modules/board/board.module.ts | 2 +- apps/server/src/modules/board/repo/board-do.repo.spec.ts | 2 +- .../src/modules/board/repo/recursive-delete.visitor.spec.ts | 2 +- apps/server/src/modules/board/repo/recursive-delete.vistor.ts | 2 +- apps/server/src/modules/collaborative-text-editor/index.ts | 3 ++- apps/server/src/modules/server/server.module.ts | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/server/src/modules/board/board.module.ts b/apps/server/src/modules/board/board.module.ts index 6dd75961a14..27b29f0ccbf 100644 --- a/apps/server/src/modules/board/board.module.ts +++ b/apps/server/src/modules/board/board.module.ts @@ -1,4 +1,5 @@ import { ConsoleWriterModule } from '@infra/console'; +import { CollaborativeTextEditorModule } from '@modules/collaborative-text-editor'; import { CopyHelperModule } from '@modules/copy-helper'; import { FilesStorageClientModule } from '@modules/files-storage-client'; import { TldrawClientModule } from '@modules/tldraw-client'; @@ -11,7 +12,6 @@ import { CqrsModule } from '@nestjs/cqrs'; import { ContentElementFactory } from '@shared/domain/domainobject'; import { CourseRepo } from '@shared/repo'; import { LoggerModule } from '@src/core/logger'; -import { CollaborativeTextEditorModule } from '../collaborative-text-editor/collaborative-text-editor.module'; import { BoardDoRepo, BoardNodeRepo, RecursiveDeleteVisitor } from './repo'; import { BoardDoAuthorizableService, diff --git a/apps/server/src/modules/board/repo/board-do.repo.spec.ts b/apps/server/src/modules/board/repo/board-do.repo.spec.ts index e178975244e..08a8d107862 100644 --- a/apps/server/src/modules/board/repo/board-do.repo.spec.ts +++ b/apps/server/src/modules/board/repo/board-do.repo.spec.ts @@ -2,6 +2,7 @@ import { createMock } from '@golevelup/ts-jest'; import { MongoMemoryDatabaseModule } from '@infra/database'; import { NotFoundError } from '@mikro-orm/core'; import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { CollaborativeTextEditorService } from '@modules/collaborative-text-editor'; import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { DrawingElementAdapterService } from '@modules/tldraw-client'; import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; @@ -42,7 +43,6 @@ import { richTextElementFactory, richTextElementNodeFactory, } from '@shared/testing'; -import { CollaborativeTextEditorService } from '@src/modules/collaborative-text-editor/service/collaborative-text-editor.service'; import { ContextExternalTool } from '../../tool/context-external-tool/domain'; import { ContextExternalToolEntity } from '../../tool/context-external-tool/entity'; import { BoardDoRepo } from './board-do.repo'; diff --git a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts index 43d71124f05..e0373dad5d2 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts @@ -1,6 +1,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { FileRecordParentType } from '@infra/rabbitmq'; import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { CollaborativeTextEditorService } from '@modules/collaborative-text-editor'; import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { DrawingElementAdapterService } from '@modules/tldraw-client'; import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; @@ -21,7 +22,6 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; -import { CollaborativeTextEditorService } from '@src/modules/collaborative-text-editor/service/collaborative-text-editor.service'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; describe(RecursiveDeleteVisitor.name, () => { diff --git a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts index ac54aa4a1c8..3b29e82edd5 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -1,4 +1,5 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { CollaborativeTextEditorService } from '@modules/collaborative-text-editor'; import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { DrawingElementAdapterService } from '@modules/tldraw-client'; import type { ContextExternalTool } from '@modules/tool/context-external-tool/domain'; @@ -23,7 +24,6 @@ import type { SubmissionItem, } from '@shared/domain/domainobject'; import { BoardNode } from '@shared/domain/entity'; -import { CollaborativeTextEditorService } from '@src/modules/collaborative-text-editor/service/collaborative-text-editor.service'; @Injectable() export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { diff --git a/apps/server/src/modules/collaborative-text-editor/index.ts b/apps/server/src/modules/collaborative-text-editor/index.ts index 274f33d5408..5c5b448ca3b 100644 --- a/apps/server/src/modules/collaborative-text-editor/index.ts +++ b/apps/server/src/modules/collaborative-text-editor/index.ts @@ -1,2 +1,3 @@ -export { CollaborativeTextEditorApiModule } from './collaborative-text-editor-api.module'; export { CollaborativeTextEditorConfig } from './collaborative-text-editor.config'; +export { CollaborativeTextEditorModule } from './collaborative-text-editor.module'; +export { CollaborativeTextEditorService } from './service/collaborative-text-editor.service'; diff --git a/apps/server/src/modules/server/server.module.ts b/apps/server/src/modules/server/server.module.ts index ff8b86cd649..ea0e6d348d7 100644 --- a/apps/server/src/modules/server/server.module.ts +++ b/apps/server/src/modules/server/server.module.ts @@ -10,7 +10,7 @@ import { AuthenticationApiModule } from '@modules/authentication/authentication- import { BoardApiModule } from '@modules/board/board-api.module'; import { MediaBoardApiModule } from '@modules/board/media-board-api.module'; import { CollaborativeStorageModule } from '@modules/collaborative-storage'; -import { CollaborativeTextEditorApiModule } from '@modules/collaborative-text-editor'; +import { CollaborativeTextEditorApiModule } from '@modules/collaborative-text-editor/collaborative-text-editor-api.module'; import { FilesStorageClientModule } from '@modules/files-storage-client'; import { GroupApiModule } from '@modules/group/group-api.module'; import { LearnroomApiModule } from '@modules/learnroom/learnroom-api.module';