From 13d317f60b446b74d3ba88eaaed1452424dbdaf8 Mon Sep 17 00:00:00 2001 From: Bartosz Nowicki <116367402+bn-pass@users.noreply.github.com> Date: Fri, 22 Sep 2023 17:23:59 +0200 Subject: [PATCH 01/10] BC-4970 legacy user files retrieval (#4429) * add files service method to find files accessible by user with given userId * modify some tests descriptions * add files service method to find files owned by user with given userId --- .../files/service/files.service.spec.ts | 92 +++++++++++++++++++ .../modules/files/service/files.service.ts | 9 ++ 2 files changed, 101 insertions(+) diff --git a/apps/server/src/modules/files/service/files.service.spec.ts b/apps/server/src/modules/files/service/files.service.spec.ts index 5ee07897565..98ddc788d34 100644 --- a/apps/server/src/modules/files/service/files.service.spec.ts +++ b/apps/server/src/modules/files/service/files.service.spec.ts @@ -39,6 +39,51 @@ describe(FilesService.name, () => { await module.close(); }); + describe('findFilesAccessibleByUser', () => { + describe('when called with a userId of a user that', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const accessibleFiles: FileEntity[] = []; + + for (let i = 0; i < 5; i += 1) { + accessibleFiles.push( + fileEntityFactory.build({ + permissions: [filePermissionEntityFactory.build({ refId: userId })], + }) + ); + } + + return { userId, accessibleFiles }; + }; + + describe("doesn't have an access to any files", () => { + it('should return an empty array', async () => { + const { userId } = setup(); + + repo.findByPermissionRefId.mockResolvedValueOnce([]); + + const result = await service.findFilesAccessibleByUser(userId); + + expect(repo.findByPermissionRefId).toBeCalledWith(userId); + expect(result).toEqual([]); + }); + }); + + describe('does have an access to some files', () => { + it('should return an array containing proper file entities', async () => { + const { userId, accessibleFiles } = setup(); + + repo.findByPermissionRefId.mockResolvedValueOnce(accessibleFiles); + + const result = await service.findFilesAccessibleByUser(userId); + + expect(repo.findByPermissionRefId).toBeCalledWith(userId); + expect(result).toEqual(accessibleFiles); + }); + }); + }); + }); + describe('removeUserPermissionsToAnyFiles', () => { it('should not modify any files if there are none that user has permission to access', async () => { const userId = new ObjectId().toHexString(); @@ -110,6 +155,53 @@ describe(FilesService.name, () => { }); }); + describe('findFilesOwnedByUser', () => { + describe('when called with a userId of a user that', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const ownedFiles: FileEntity[] = []; + + for (let i = 0; i < 5; i += 1) { + ownedFiles.push( + fileEntityFactory.build({ + ownerId: userId, + creatorId: userId, + permissions: [filePermissionEntityFactory.build({ refId: userId })], + }) + ); + } + + return { userId, ownedFiles }; + }; + + describe("doesn't own any files", () => { + it('should return an empty array', async () => { + const { userId } = setup(); + + repo.findByOwnerUserId.mockResolvedValueOnce([]); + + const result = await service.findFilesOwnedByUser(userId); + + expect(repo.findByOwnerUserId).toBeCalledWith(userId); + expect(result).toEqual([]); + }); + }); + + describe('does own some files', () => { + it('should return an array containing proper file entities', async () => { + const { userId, ownedFiles } = setup(); + + repo.findByOwnerUserId.mockResolvedValueOnce(ownedFiles); + + const result = await service.findFilesOwnedByUser(userId); + + expect(repo.findByOwnerUserId).toBeCalledWith(userId); + expect(result).toEqual(ownedFiles); + }); + }); + }); + }); + describe('markFilesOwnedByUserForDeletion', () => { const verifyEntityChanges = (entity: FileEntity) => { expect(entity.deleted).toEqual(true); diff --git a/apps/server/src/modules/files/service/files.service.ts b/apps/server/src/modules/files/service/files.service.ts index b3d3bd352cc..b2faef58fd1 100644 --- a/apps/server/src/modules/files/service/files.service.ts +++ b/apps/server/src/modules/files/service/files.service.ts @@ -1,11 +1,16 @@ import { Injectable } from '@nestjs/common'; import { EntityId } from '@shared/domain'; import { FilesRepo } from '../repo'; +import { FileEntity } from '../entity'; @Injectable() export class FilesService { constructor(private readonly repo: FilesRepo) {} + async findFilesAccessibleByUser(userId: EntityId): Promise { + return this.repo.findByPermissionRefId(userId); + } + async removeUserPermissionsToAnyFiles(userId: EntityId): Promise { const entities = await this.repo.findByPermissionRefId(userId); @@ -20,6 +25,10 @@ export class FilesService { return entities.length; } + async findFilesOwnedByUser(userId: EntityId): Promise { + return this.repo.findByOwnerUserId(userId); + } + async markFilesOwnedByUserForDeletion(userId: EntityId): Promise { const entities = await this.repo.findByOwnerUserId(userId); From 69b15285a0e53c72064ef3783450b6c80c1a13ea Mon Sep 17 00:00:00 2001 From: Max Bischof <106820326+bischofmax@users.noreply.github.com> Date: Mon, 25 Sep 2023 10:14:12 +0200 Subject: [PATCH 02/10] BC-3800 - Add caption of file (#4433) --- .../board/file-element.do.spec.ts | 56 +++++++++++++++++++ .../domainobject/board/file-element.do.ts | 2 +- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/apps/server/src/shared/domain/domainobject/board/file-element.do.spec.ts b/apps/server/src/shared/domain/domainobject/board/file-element.do.spec.ts index a659be88ee1..6de914f6751 100644 --- a/apps/server/src/shared/domain/domainobject/board/file-element.do.spec.ts +++ b/apps/server/src/shared/domain/domainobject/board/file-element.do.spec.ts @@ -4,6 +4,62 @@ import { FileElement } from './file-element.do'; import { BoardCompositeVisitor, BoardCompositeVisitorAsync } from './types'; describe(FileElement.name, () => { + describe('get caption', () => { + describe('when caption is set', () => { + it('should return the caption', () => { + const fileElement = fileElementFactory.build(); + + expect(fileElement.caption).toEqual(fileElement.caption); + }); + }); + + describe('when caption is not set', () => { + it('should return an empty string', () => { + const fileElement = fileElementFactory.build({ caption: undefined }); + + expect(fileElement.caption).toEqual(''); + }); + }); + }); + + describe('set caption', () => { + it('should set caption', () => { + const fileElement = fileElementFactory.build(); + const text = 'new caption'; + fileElement.caption = text; + + expect(fileElement.caption).toEqual(text); + }); + }); + + describe('get alternative text', () => { + describe('when alternative text is set', () => { + it('should return the alternative text', () => { + const fileElement = fileElementFactory.build(); + + expect(fileElement.alternativeText).toEqual(fileElement.alternativeText); + }); + }); + + describe('when alternative text is not set', () => { + it('should return an empty string', () => { + const fileElement = fileElementFactory.build({ alternativeText: undefined }); + + expect(fileElement.alternativeText).toEqual(''); + }); + }); + }); + + describe('set alternative text', () => { + it('should set alternative text', () => { + const fileElement = fileElementFactory.build(); + const text = 'new alternative text'; + fileElement.alternativeText = text; + + expect(fileElement.alternativeText).toEqual(text); + }); + }); + describe('when trying to add a child to a file element', () => { it('should throw an error ', () => { const fileElement = fileElementFactory.build(); diff --git a/apps/server/src/shared/domain/domainobject/board/file-element.do.ts b/apps/server/src/shared/domain/domainobject/board/file-element.do.ts index ab209fbe346..4d58aa300ea 100644 --- a/apps/server/src/shared/domain/domainobject/board/file-element.do.ts +++ b/apps/server/src/shared/domain/domainobject/board/file-element.do.ts @@ -3,7 +3,7 @@ import type { BoardCompositeVisitor, BoardCompositeVisitorAsync } from './types' export class FileElement extends BoardComposite { get caption(): string { - return this.props.caption; + return this.props.caption || ''; } set caption(value: string) { From b1c09d49f2bb0b4d3d2e656f9c0030e50a0f072f Mon Sep 17 00:00:00 2001 From: virgilchiriac <17074330+virgilchiriac@users.noreply.github.com> Date: Mon, 25 Sep 2023 11:43:22 +0200 Subject: [PATCH 03/10] BC-4375 - fix api generator (#4435) --- .../controller/api-test/submission-item-lookup.api.spec.ts | 2 +- .../src/modules/board/controller/dto/submission-item/index.ts | 4 +++- .../controller/mapper/submission-item-response.mapper.ts | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts b/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts index 4686f953d29..6f31d5a4e5a 100644 --- a/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts @@ -15,7 +15,7 @@ import { userFactory, } from '@shared/testing'; import { ServerTestModule } from '@src/modules/server'; -import { SubmissionsResponse } from '../dto'; +import { SubmissionsResponse } from '../dto/submission-item/submissions.response'; const baseRouteName = '/board-submissions'; describe('submission item lookup (api)', () => { diff --git a/apps/server/src/modules/board/controller/dto/submission-item/index.ts b/apps/server/src/modules/board/controller/dto/submission-item/index.ts index 294c722dbef..b009f3e0560 100644 --- a/apps/server/src/modules/board/controller/dto/submission-item/index.ts +++ b/apps/server/src/modules/board/controller/dto/submission-item/index.ts @@ -2,5 +2,7 @@ export * from './submission-container.url.params'; export * from './create-submission-item.body.params'; export * from './submission-item.response'; export * from './submission-item.url.params'; -export * from './submissions.response'; +// TODO for some reason, api generator messes up the types +// import it directly, not via this index seems to fix it +// export * from './submissions.response'; export * from './update-submission-item.body.params'; diff --git a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts index 53efb37a482..82d2292ba11 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts @@ -1,5 +1,6 @@ import { SubmissionItem, UserBoardRoles } from '@shared/domain'; -import { SubmissionItemResponse, SubmissionsResponse, TimestampsResponse, UserDataResponse } from '../dto'; +import { SubmissionsResponse } from '../dto/submission-item/submissions.response'; +import { SubmissionItemResponse, TimestampsResponse, UserDataResponse } from '../dto'; export class SubmissionItemResponseMapper { private static instance: SubmissionItemResponseMapper; From 8501d4f24c7b90140197fd847df4520f3f2cb94b Mon Sep 17 00:00:00 2001 From: Alexander Weber <103171324+alweber-cap@users.noreply.github.com> Date: Mon, 25 Sep 2023 13:32:44 +0200 Subject: [PATCH 04/10] Ew 604 Handle Users without accounts and proper naming (#4431) * add account check and lowercase username * add test * fix lint * fix account get flow --- .../user-import/uc/user-import.uc.spec.ts | 17 +++++++++------- .../modules/user-import/uc/user-import.uc.ts | 20 +++++++++++++++++-- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts b/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts index 3617b5c4e3d..8dcccaf84e2 100644 --- a/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts +++ b/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts @@ -483,6 +483,7 @@ describe('[ImportUserModule]', () => { userMatch1 = userFactory.buildWithId({ school }); userMatch2 = userFactory.buildWithId({ school }); + importUser1 = importUserFactory.buildWithId({ school, user: userMatch1, @@ -505,13 +506,15 @@ describe('[ImportUserModule]', () => { userRepoFlushSpy = userRepo.flush.mockResolvedValueOnce(); permissionServiceSpy = authorizationService.checkAllPermissions.mockReturnValue(); importUserRepoFindImportUsersSpy = importUserRepo.findImportUsers.mockResolvedValue([[], 0]); - accountServiceFindByUserIdSpy = accountService.findByUserIdOrFail.mockResolvedValue({ - id: 'dummyId', - userId: currentUser.id, - username: currentUser.email, - createdAt: new Date(), - updatedAt: new Date(), - }); + accountServiceFindByUserIdSpy = accountService.findByUserId + .mockResolvedValue({ + id: 'dummyId', + userId: currentUser.id, + username: currentUser.email, + createdAt: new Date(), + updatedAt: new Date(), + }) + .mockResolvedValueOnce(null); importUserRepoDeleteImportUsersBySchoolSpy = importUserRepo.deleteImportUsersBySchool.mockResolvedValue(); importUserRepoDeleteImportUserSpy = importUserRepo.delete.mockResolvedValue(); schoolServiceSaveSpy = schoolService.save.mockReturnValueOnce(Promise.resolve(createMockSchoolDo(school))); diff --git a/apps/server/src/modules/user-import/uc/user-import.uc.ts b/apps/server/src/modules/user-import/uc/user-import.uc.ts index 3812b945cc6..ecef86207a0 100644 --- a/apps/server/src/modules/user-import/uc/user-import.uc.ts +++ b/apps/server/src/modules/user-import/uc/user-import.uc.ts @@ -23,6 +23,7 @@ import { AccountService } from '@src/modules/account/services/account.service'; import { AccountDto } from '@src/modules/account/services/dto/account.dto'; import { AuthorizationService } from '@src/modules/authorization'; import { LegacySchoolService } from '@src/modules/legacy-school'; +import { AccountSaveDto } from '../../account/services/dto'; import { MigrationMayBeCompleted, MigrationMayNotBeCompleted, @@ -281,17 +282,32 @@ export class UserImportUc { user.ldapDn = importUser.ldapDn; user.externalId = importUser.externalId; - const account: AccountDto = await this.accountService.findByUserIdOrFail(user.id); + const account: AccountDto = await this.getAccount(user); account.systemId = importUser.system.id; account.password = undefined; - account.username = `${school.externalId}/${importUser.loginName}`; + account.username = `${school.externalId}/${importUser.loginName}`.toLowerCase(); await this.userRepo.save(user); await this.accountService.save(account); await this.importUserRepo.delete(importUser); } + private async getAccount(user: User): Promise { + let account: AccountDto | null = await this.accountService.findByUserId(user.id); + + if (!account) { + const newAccount: AccountSaveDto = new AccountSaveDto({ + userId: user.id, + username: user.email, + }); + + await this.accountService.saveWithValidation(newAccount); + account = await this.accountService.findByUserIdOrFail(user.id); + } + return account; + } + private async getMigrationSystem(): Promise { const systemId = Configuration.get('FEATURE_USER_MIGRATION_SYSTEM_ID') as string; const system = await this.systemRepo.findById(systemId); From ee43c77b8e36315b72f6399d9f82a6f7bdb1fd7d Mon Sep 17 00:00:00 2001 From: WojciechGrancow <116577704+WojciechGrancow@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:31:40 +0200 Subject: [PATCH 05/10] BC-4973-rewrite-data-retrival-in-course (#4422) * changes in lesson module * add method for services in lesson, course and coursegroup * litle addition in testcase * changes in tests * change method names in service after review * small changes in test --- .../learnroom/service/course.service.spec.ts | 37 +++++++++++++++++ .../learnroom/service/course.service.ts | 8 +++- .../service/coursegroup.service.spec.ts | 34 +++++++++++++++ .../learnroom/service/coursegroup.service.ts | 8 +++- .../lesson/service/lesson.service.spec.ts | 41 +++++++++++++++++++ .../modules/lesson/service/lesson.service.ts | 6 +++ 6 files changed, 132 insertions(+), 2 deletions(-) diff --git a/apps/server/src/modules/learnroom/service/course.service.spec.ts b/apps/server/src/modules/learnroom/service/course.service.spec.ts index 67c581d6818..fbf2f09b698 100644 --- a/apps/server/src/modules/learnroom/service/course.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/course.service.spec.ts @@ -56,6 +56,43 @@ describe('CourseService', () => { }); }); + describe('findAllCoursesByUserId', () => { + describe('when finding by userId', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const course1 = courseFactory.buildWithId({ students: [user] }); + const course2 = courseFactory.buildWithId({ teachers: [user] }); + const course3 = courseFactory.buildWithId({ substitutionTeachers: [user] }); + const allCourses = [course1, course2, course3]; + + userRepo.findById.mockResolvedValue(user); + courseRepo.findAllByUserId.mockResolvedValue([allCourses, allCourses.length]); + + return { + user, + allCourses, + }; + }; + + it('should call courseRepo.findAllByUserId', async () => { + const { user } = setup(); + + await courseService.findAllCoursesByUserId(user.id); + + expect(courseRepo.findAllByUserId).toBeCalledWith(user.id); + }); + + it('should return array of courses with userId', async () => { + const { user, allCourses } = setup(); + + const [courses] = await courseService.findAllCoursesByUserId(user.id); + + expect(courses.length).toEqual(3); + expect(courses).toEqual(allCourses); + }); + }); + }); + describe('when deleting by userId', () => { const setup = () => { const user = userFactory.buildWithId(); diff --git a/apps/server/src/modules/learnroom/service/course.service.ts b/apps/server/src/modules/learnroom/service/course.service.ts index c2e7364ffe9..76b441b42d8 100644 --- a/apps/server/src/modules/learnroom/service/course.service.ts +++ b/apps/server/src/modules/learnroom/service/course.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { CourseRepo } from '@shared/repo'; -import { Course, EntityId } from '@shared/domain'; +import { Counted, Course, EntityId } from '@shared/domain'; @Injectable() export class CourseService { @@ -10,6 +10,12 @@ export class CourseService { return this.repo.findById(courseId); } + public async findAllCoursesByUserId(userId: EntityId): Promise> { + const [courses, count] = await this.repo.findAllByUserId(userId); + + return [courses, count]; + } + public async deleteUserDataFromCourse(userId: EntityId): Promise { const [courses, count] = await this.repo.findAllByUserId(userId); diff --git a/apps/server/src/modules/learnroom/service/coursegroup.service.spec.ts b/apps/server/src/modules/learnroom/service/coursegroup.service.spec.ts index 5a445792a23..48caf02c378 100644 --- a/apps/server/src/modules/learnroom/service/coursegroup.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/coursegroup.service.spec.ts @@ -38,6 +38,40 @@ describe('CourseGroupService', () => { jest.clearAllMocks(); }); + describe('findAllCourseGroupsByUserId', () => { + describe('when finding by userId', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const courseGroups = courseGroupFactory.buildListWithId(2, { students: [user] }); + + userRepo.findById.mockResolvedValue(user); + courseGroupRepo.findByUserId.mockResolvedValue([courseGroups, courseGroups.length]); + + return { + user, + courseGroups, + }; + }; + + it('should call courseGroupRepo.findByUserId', async () => { + const { user } = setup(); + + await courseGroupService.findAllCourseGroupsByUserId(user.id); + + expect(courseGroupRepo.findByUserId).toBeCalledWith(user.id); + }); + + it('should return array with coursesGroup with userId', async () => { + const { user, courseGroups } = setup(); + + const [courseGroup] = await courseGroupService.findAllCourseGroupsByUserId(user.id); + + expect(courseGroup.length).toEqual(2); + expect(courseGroup).toEqual(courseGroups); + }); + }); + }); + describe('when deleting by userId', () => { const setup = () => { const user = userFactory.buildWithId(); diff --git a/apps/server/src/modules/learnroom/service/coursegroup.service.ts b/apps/server/src/modules/learnroom/service/coursegroup.service.ts index acd8f5b9b42..622cdbc5e9e 100644 --- a/apps/server/src/modules/learnroom/service/coursegroup.service.ts +++ b/apps/server/src/modules/learnroom/service/coursegroup.service.ts @@ -1,11 +1,17 @@ import { Injectable } from '@nestjs/common'; -import { EntityId } from '@shared/domain'; +import { Counted, CourseGroup, EntityId } from '@shared/domain'; import { CourseGroupRepo } from '@shared/repo'; @Injectable() export class CourseGroupService { constructor(private readonly repo: CourseGroupRepo) {} + public async findAllCourseGroupsByUserId(userId: EntityId): Promise> { + const [courseGroups, count] = await this.repo.findByUserId(userId); + + return [courseGroups, count]; + } + public async deleteUserDataFromCourseGroup(userId: EntityId): Promise { const [courseGroups, count] = await this.repo.findByUserId(userId); diff --git a/apps/server/src/modules/lesson/service/lesson.service.spec.ts b/apps/server/src/modules/lesson/service/lesson.service.spec.ts index bd5431b326d..7f0179640b5 100644 --- a/apps/server/src/modules/lesson/service/lesson.service.spec.ts +++ b/apps/server/src/modules/lesson/service/lesson.service.spec.ts @@ -77,6 +77,47 @@ describe('LessonService', () => { }); }); + describe('findAllLessonsByUserId', () => { + describe('when finding by userId', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const contentExample: IComponentProperties = { + title: 'title', + hidden: false, + user: userId, + component: ComponentType.TEXT, + content: { text: 'test of content' }, + }; + const lesson1 = lessonFactory.buildWithId({ contents: [contentExample] }); + const lesson2 = lessonFactory.buildWithId({ contents: [contentExample] }); + const lessons = [lesson1, lesson2]; + + lessonRepo.findByUserId.mockResolvedValue(lessons); + + return { + userId, + lessons, + }; + }; + + it('should call findByCourseIds from lesson repo', async () => { + const { userId } = setup(); + + await expect(lessonService.findAllLessonsByUserId(userId)).resolves.not.toThrow(); + expect(lessonRepo.findByUserId).toBeCalledWith(userId); + }); + + it('should return array of lessons with userId', async () => { + const { userId, lessons } = setup(); + + const result = await lessonService.findAllLessonsByUserId(userId); + + expect(result).toHaveLength(2); + expect(result).toEqual(lessons); + }); + }); + }); + describe('deleteUserDataFromTeams', () => { describe('when deleting by userId', () => { const setup = () => { diff --git a/apps/server/src/modules/lesson/service/lesson.service.ts b/apps/server/src/modules/lesson/service/lesson.service.ts index 5f8acb03e38..5a69a19f305 100644 --- a/apps/server/src/modules/lesson/service/lesson.service.ts +++ b/apps/server/src/modules/lesson/service/lesson.service.ts @@ -24,6 +24,12 @@ export class LessonService { return this.lessonRepo.findAllByCourseIds(courseIds); } + async findAllLessonsByUserId(userId: EntityId): Promise { + const lessons = await this.lessonRepo.findByUserId(userId); + + return lessons; + } + async deleteUserDataFromLessons(userId: EntityId): Promise { const lessons = await this.lessonRepo.findByUserId(userId); From 4e3b8b9e97fcee4675c4a20d0f1fb8b270e57b20 Mon Sep 17 00:00:00 2001 From: Martin Schuhmacher <55735359+MartinSchuhmacher@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:09:58 +0200 Subject: [PATCH 06/10] BC-4374 - make submission due date optional (#4378) For the response FE needs the dueDate to be set so that vue can properly watch it. Therefor, the response will set it to null if undefined. Co-authored-by: virgilchiriac --- .../content-element-create.api.spec.ts | 5 +- .../content-element-update-content.spec.ts | 139 +++++++++++++++--- .../controller/dto/card/card.response.ts | 8 +- .../submission-container-element.response.ts | 8 +- .../update-element-content.body.params.ts | 8 +- ...ssion-container-element-response.mapper.ts | 8 +- .../board/repo/board-do.builder-impl.ts | 6 +- .../board/repo/recursive-save.visitor.ts | 5 +- .../service/content-element-update.visitor.ts | 2 +- .../board/content-element.factory.ts | 2 - .../board/submission-container-element.do.ts | 6 +- ...ubmission-container-element-node.entity.ts | 6 +- ...bmission-container-element-node.factory.ts | 5 +- 13 files changed, 165 insertions(+), 43 deletions(-) diff --git a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts index f6bcd16fd24..0f7a3795580 100644 --- a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts @@ -12,7 +12,7 @@ import { UserAndAccountTestFactory, } from '@shared/testing'; import { ServerTestModule } from '@src/modules/server/server.module'; -import { AnyContentElementResponse } from '../dto'; +import { AnyContentElementResponse, SubmissionContainerElementResponse } from '../dto'; const baseRouteName = '/cards'; @@ -91,7 +91,7 @@ describe(`content element create (api)`, () => { expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.EXTERNAL_TOOL); }); - it('should return the created content element of type SUBMISSION_CONTAINER', async () => { + it('should return the created content element of type SUBMISSION_CONTAINER with dueDate set to null', async () => { const { loggedInClient, cardNode } = await setup(); const response = await loggedInClient.post(`${cardNode.id}/elements`, { @@ -99,6 +99,7 @@ describe(`content element create (api)`, () => { }); expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.SUBMISSION_CONTAINER); + expect((response.body as SubmissionContainerElementResponse).content.dueDate).toBeNull(); }); it('should actually create the content element', async () => { diff --git a/apps/server/src/modules/board/controller/api-test/content-element-update-content.spec.ts b/apps/server/src/modules/board/controller/api-test/content-element-update-content.spec.ts index 09e3f8c046b..bee1ad63f0f 100644 --- a/apps/server/src/modules/board/controller/api-test/content-element-update-content.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/content-element-update-content.spec.ts @@ -8,10 +8,9 @@ import { FileElementNode, InputFormat, RichTextElementNode, + SubmissionContainerElementNode, } from '@shared/domain'; import { - TestApiClient, - UserAndAccountTestFactory, cardNodeFactory, cleanupCollections, columnBoardNodeFactory, @@ -19,6 +18,9 @@ import { courseFactory, fileElementNodeFactory, richTextElementNodeFactory, + submissionContainerElementNodeFactory, + TestApiClient, + UserAndAccountTestFactory, } from '@shared/testing'; import { ServerTestModule } from '@src/modules/server/server.module'; @@ -59,8 +61,15 @@ describe(`content element update content (api)`, () => { const column = columnNodeFactory.buildWithId({ parent: columnBoardNode }); const parentCard = cardNodeFactory.buildWithId({ parent: column }); - const richTextelement = richTextElementNodeFactory.buildWithId({ parent: parentCard }); + const richTextElement = richTextElementNodeFactory.buildWithId({ parent: parentCard }); const fileElement = fileElementNodeFactory.buildWithId({ parent: parentCard }); + const submissionContainerElement = submissionContainerElementNodeFactory.buildWithId({ parent: parentCard }); + + const tomorrow = new Date(Date.now() + 86400000); + const submissionContainerElementWithDueDate = submissionContainerElementNodeFactory.buildWithId({ + parent: parentCard, + dueDate: tomorrow, + }); await em.persistAndFlush([ teacherAccount, @@ -68,20 +77,28 @@ describe(`content element update content (api)`, () => { parentCard, column, columnBoardNode, - richTextelement, + richTextElement, fileElement, + submissionContainerElement, + submissionContainerElementWithDueDate, ]); em.clear(); const loggedInClient = await testApiClient.login(teacherAccount); - return { loggedInClient, richTextelement, fileElement }; + return { + loggedInClient, + richTextElement, + fileElement, + submissionContainerElement, + submissionContainerElementWithDueDate, + }; }; it('should return status 204', async () => { - const { loggedInClient, richTextelement } = await setup(); + const { loggedInClient, richTextElement } = await setup(); - const response = await loggedInClient.patch(`${richTextelement.id}/content`, { + const response = await loggedInClient.patch(`${richTextElement.id}/content`, { data: { content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 }, type: ContentElementType.RICH_TEXT, @@ -92,30 +109,30 @@ describe(`content element update content (api)`, () => { }); it('should actually change content of the element', async () => { - const { loggedInClient, richTextelement } = await setup(); + const { loggedInClient, richTextElement } = await setup(); - await loggedInClient.patch(`${richTextelement.id}/content`, { + await loggedInClient.patch(`${richTextElement.id}/content`, { data: { content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 }, type: ContentElementType.RICH_TEXT, }, }); - const result = await em.findOneOrFail(RichTextElementNode, richTextelement.id); + const result = await em.findOneOrFail(RichTextElementNode, richTextElement.id); expect(result.text).toEqual('hello world'); }); it('should sanitize rich text before changing content of the element', async () => { - const { loggedInClient, richTextelement } = await setup(); + const { loggedInClient, richTextElement } = await setup(); const text = ' some more text'; const sanitizedText = sanitizeRichText(text, InputFormat.RICH_TEXT_CK5); - await loggedInClient.patch(`${richTextelement.id}/content`, { + await loggedInClient.patch(`${richTextElement.id}/content`, { data: { content: { text, inputFormat: InputFormat.RICH_TEXT_CK5 }, type: ContentElementType.RICH_TEXT }, }); - const result = await em.findOneOrFail(RichTextElementNode, richTextelement.id); + const result = await em.findOneOrFail(RichTextElementNode, richTextElement.id); expect(result.text).toEqual(sanitizedText); }); @@ -146,6 +163,76 @@ describe(`content element update content (api)`, () => { expect(result.alternativeText).toEqual('rich text 1 some more text'); }); + + it('should return status 204 (nothing changed) without dueDate parameter for submission container element', async () => { + const { loggedInClient, submissionContainerElement } = await setup(); + + const response = await loggedInClient.patch(`${submissionContainerElement.id}/content`, { + data: { + content: {}, + type: 'submissionContainer', + }, + }); + + expect(response.statusCode).toEqual(204); + }); + + it('should not change dueDate value without dueDate parameter for submission container element', async () => { + const { loggedInClient, submissionContainerElement } = await setup(); + + await loggedInClient.patch(`${submissionContainerElement.id}/content`, { + data: { + content: {}, + type: 'submissionContainer', + }, + }); + const result = await em.findOneOrFail(SubmissionContainerElementNode, submissionContainerElement.id); + + expect(result.dueDate).toBeUndefined(); + }); + + it('should set dueDate value when dueDate parameter is provided for submission container element', async () => { + const { loggedInClient, submissionContainerElement } = await setup(); + + const inThreeDays = new Date(Date.now() + 259200000); + + await loggedInClient.patch(`${submissionContainerElement.id}/content`, { + data: { + content: { dueDate: inThreeDays }, + type: 'submissionContainer', + }, + }); + const result = await em.findOneOrFail(SubmissionContainerElementNode, submissionContainerElement.id); + + expect(result.dueDate).toEqual(inThreeDays); + }); + + it('should unset dueDate value when dueDate parameter is not provided for submission container element', async () => { + const { loggedInClient, submissionContainerElementWithDueDate } = await setup(); + + await loggedInClient.patch(`${submissionContainerElementWithDueDate.id}/content`, { + data: { + content: {}, + type: 'submissionContainer', + }, + }); + const result = await em.findOneOrFail(SubmissionContainerElementNode, submissionContainerElementWithDueDate.id); + + expect(result.dueDate).toBeUndefined(); + }); + + it('should return status 400 for wrong date format for submission container element', async () => { + const { loggedInClient, submissionContainerElement } = await setup(); + + const response = await loggedInClient.patch(`${submissionContainerElement.id}/content`, { + data: { + content: { dueDate: 'hello world' }, + type: 'submissionContainer', + }, + }); + + expect(response.statusCode).toEqual(400); + }); }); describe('with invalid user', () => { @@ -163,24 +250,38 @@ describe(`content element update content (api)`, () => { const column = columnNodeFactory.buildWithId({ parent: columnBoardNode }); const parentCard = cardNodeFactory.buildWithId({ parent: column }); - const element = richTextElementNodeFactory.buildWithId({ parent: parentCard }); + const richTextElement = richTextElementNodeFactory.buildWithId({ parent: parentCard }); + const submissionContainerElement = submissionContainerElementNodeFactory.buildWithId({ parent: parentCard }); - await em.persistAndFlush([parentCard, column, columnBoardNode, element]); + await em.persistAndFlush([parentCard, column, columnBoardNode, richTextElement, submissionContainerElement]); em.clear(); const loggedInClient = await testApiClient.login(invalidTeacherAccount); - return { loggedInClient, element }; + return { loggedInClient, richTextElement, submissionContainerElement }; }; - it('should return status 403', async () => { - const { loggedInClient, element } = await setup(); + it('should return status 403 for rich text element', async () => { + const { loggedInClient, richTextElement } = await setup(); - const response = await loggedInClient.patch(`${element.id}/content`, { + const response = await loggedInClient.patch(`${richTextElement.id}/content`, { data: { content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 }, type: 'richText' }, }); expect(response.statusCode).toEqual(HttpStatus.FORBIDDEN); }); + + it('should return status 403 for submission container element', async () => { + const { loggedInClient, submissionContainerElement } = await setup(); + + const response = await loggedInClient.patch(`${submissionContainerElement.id}/content`, { + data: { + content: {}, + type: 'submissionContainer', + }, + }); + + expect(response.statusCode).toEqual(HttpStatus.FORBIDDEN); + }); }); }); diff --git a/apps/server/src/modules/board/controller/dto/card/card.response.ts b/apps/server/src/modules/board/controller/dto/card/card.response.ts index 8ff034ad093..44ee426fb6b 100644 --- a/apps/server/src/modules/board/controller/dto/card/card.response.ts +++ b/apps/server/src/modules/board/controller/dto/card/card.response.ts @@ -1,6 +1,6 @@ import { ApiExtraModels, ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; import { DecodeHtmlEntities } from '@shared/controller'; -import { AnyContentElementResponse } from '../element'; +import { AnyContentElementResponse, FileElementResponse, SubmissionContainerElementResponse } from '../element'; import { RichTextElementResponse } from '../element/rich-text-element.response'; import { TimestampsResponse } from '../timestamps.response'; import { VisibilitySettingsResponse } from './visibility-settings.response'; @@ -31,7 +31,11 @@ export class CardResponse { @ApiProperty({ type: 'array', items: { - oneOf: [{ $ref: getSchemaPath(RichTextElementResponse) }], + oneOf: [ + { $ref: getSchemaPath(RichTextElementResponse) }, + { $ref: getSchemaPath(FileElementResponse) }, + { $ref: getSchemaPath(SubmissionContainerElementResponse) }, + ], }, }) elements: AnyContentElementResponse[]; diff --git a/apps/server/src/modules/board/controller/dto/element/submission-container-element.response.ts b/apps/server/src/modules/board/controller/dto/element/submission-container-element.response.ts index 642d5e44818..e6f0d1364ef 100644 --- a/apps/server/src/modules/board/controller/dto/element/submission-container-element.response.ts +++ b/apps/server/src/modules/board/controller/dto/element/submission-container-element.response.ts @@ -7,8 +7,12 @@ export class SubmissionContainerElementContent { this.dueDate = dueDate; } - @ApiProperty() - dueDate: Date; + @ApiProperty({ + type: Date, + description: 'The dueDate as date string or null of not set', + example: '2023-08-17T14:17:51.958+00:00', + }) + dueDate: Date | null; } export class SubmissionContainerElementResponse { diff --git a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts index 1f2a320119e..05856e9ef5f 100644 --- a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts @@ -53,8 +53,12 @@ export class RichTextElementContentBody extends ElementContentBody { export class SubmissionContainerContentBody { @IsDate() - @ApiProperty() - dueDate!: Date; + @IsOptional() + @ApiPropertyOptional({ + required: false, + description: 'The point in time until when a submission can be handed in.', + }) + dueDate?: Date; } export class SubmissionContainerElementContentBody extends ElementContentBody { diff --git a/apps/server/src/modules/board/controller/mapper/submission-container-element-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-container-element-response.mapper.ts index 30acafa298f..8b3dc6ae54f 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-container-element-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-container-element-response.mapper.ts @@ -18,9 +18,15 @@ export class SubmissionContainerElementResponseMapper implements BaseResponseMap id: element.id, timestamps: new TimestampsResponse({ lastUpdatedAt: element.updatedAt, createdAt: element.createdAt }), type: ContentElementType.SUBMISSION_CONTAINER, - content: new SubmissionContainerElementContent({ dueDate: element.dueDate }), + content: new SubmissionContainerElementContent({ + dueDate: element.dueDate || null, + }), }); + if (element.dueDate) { + result.content = new SubmissionContainerElementContent({ dueDate: element.dueDate }); + } + return result; } diff --git a/apps/server/src/modules/board/repo/board-do.builder-impl.ts b/apps/server/src/modules/board/repo/board-do.builder-impl.ts index 87dc4382798..af58280b33f 100644 --- a/apps/server/src/modules/board/repo/board-do.builder-impl.ts +++ b/apps/server/src/modules/board/repo/board-do.builder-impl.ts @@ -125,11 +125,15 @@ export class BoardDoBuilderImpl implements BoardDoBuilder { const element = new SubmissionContainerElement({ id: boardNode.id, - dueDate: boardNode.dueDate, children: elements, createdAt: boardNode.createdAt, updatedAt: boardNode.updatedAt, }); + + if (boardNode.dueDate) { + element.dueDate = boardNode.dueDate; + } + return element; } diff --git a/apps/server/src/modules/board/repo/recursive-save.visitor.ts b/apps/server/src/modules/board/repo/recursive-save.visitor.ts index 082cc73c4f4..5561e636267 100644 --- a/apps/server/src/modules/board/repo/recursive-save.visitor.ts +++ b/apps/server/src/modules/board/repo/recursive-save.visitor.ts @@ -128,11 +128,14 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { const boardNode = new SubmissionContainerElementNode({ id: submissionContainerElement.id, - dueDate: submissionContainerElement.dueDate, parent: parentData?.boardNode, position: parentData?.position, }); + if (submissionContainerElement.dueDate) { + boardNode.dueDate = submissionContainerElement.dueDate; + } + this.createOrUpdateBoardNode(boardNode); this.visitChildren(submissionContainerElement, boardNode); } diff --git a/apps/server/src/modules/board/service/content-element-update.visitor.ts b/apps/server/src/modules/board/service/content-element-update.visitor.ts index d660fbee98c..dfd430aa250 100644 --- a/apps/server/src/modules/board/service/content-element-update.visitor.ts +++ b/apps/server/src/modules/board/service/content-element-update.visitor.ts @@ -59,7 +59,7 @@ export class ContentElementUpdateVisitor implements BoardCompositeVisitor { visitSubmissionContainerElement(submissionContainerElement: SubmissionContainerElement): void { if (this.content instanceof SubmissionContainerContentBody) { - submissionContainerElement.dueDate = this.content.dueDate; + submissionContainerElement.dueDate = this.content.dueDate ?? undefined; } else { this.throwNotHandled(submissionContainerElement); } diff --git a/apps/server/src/shared/domain/domainobject/board/content-element.factory.ts b/apps/server/src/shared/domain/domainobject/board/content-element.factory.ts index ea268206559..fb476d2dbd0 100644 --- a/apps/server/src/shared/domain/domainobject/board/content-element.factory.ts +++ b/apps/server/src/shared/domain/domainobject/board/content-element.factory.ts @@ -63,10 +63,8 @@ export class ContentElementFactory { } private buildSubmissionContainer() { - const tomorrow = new Date(Date.now() + 86400000); const element = new SubmissionContainerElement({ id: new ObjectId().toHexString(), - dueDate: tomorrow, children: [], createdAt: new Date(), updatedAt: new Date(), diff --git a/apps/server/src/shared/domain/domainobject/board/submission-container-element.do.ts b/apps/server/src/shared/domain/domainobject/board/submission-container-element.do.ts index 3b9a85600c6..09756153a90 100644 --- a/apps/server/src/shared/domain/domainobject/board/submission-container-element.do.ts +++ b/apps/server/src/shared/domain/domainobject/board/submission-container-element.do.ts @@ -3,11 +3,11 @@ import { SubmissionItem } from './submission-item.do'; import type { AnyBoardDo, BoardCompositeVisitor, BoardCompositeVisitorAsync } from './types'; export class SubmissionContainerElement extends BoardComposite { - get dueDate(): Date { + get dueDate(): Date | undefined { return this.props.dueDate; } - set dueDate(value: Date) { + set dueDate(value: Date | undefined) { this.props.dueDate = value; } @@ -26,7 +26,7 @@ export class SubmissionContainerElement extends BoardComposite(SubmissionContainerElementNode, () => { - const inThreeDays = new Date(Date.now() + 259200000); - return { - dueDate: inThreeDays, - }; + return {}; }); From fbd698b2b535b60957ef7495b27f15a5a945b712 Mon Sep 17 00:00:00 2001 From: Tomasz Wiaderek Date: Mon, 25 Sep 2023 19:18:55 +0200 Subject: [PATCH 07/10] code refactor: --- .../api-test/tldraw.controller.api.spec.ts} | 10 +- .../src/modules/tldraw/controller/index.ts | 1 + .../tldraw.controller.ts} | 8 +- .../tldraw.gateway.spec.ts | 19 ++-- .../src/modules/tldraw/gateway/index.ts | 1 - .../src/modules/tldraw/tldraw.module.ts | 4 +- apps/server/src/modules/tldraw/types/index.ts | 1 + .../tldraw/types/ws-close-code-enum.ts | 3 + .../src/modules/tldraw/types/ws-shared-doc.ts | 62 +++++++++++ apps/server/src/modules/tldraw/utils/index.ts | 1 + apps/server/src/modules/tldraw/utils/utils.ts | 101 +----------------- .../src/modules/tldraw/utils/ydoc-utils.ts | 44 ++++++++ 12 files changed, 137 insertions(+), 118 deletions(-) rename apps/server/src/modules/tldraw/{gateway/api-test/tldraw.gateway.api.spec.ts => controller/api-test/tldraw.controller.api.spec.ts} (94%) create mode 100644 apps/server/src/modules/tldraw/controller/index.ts rename apps/server/src/modules/tldraw/{gateway/tldraw.gateway.ts => controller/tldraw.controller.ts} (86%) rename apps/server/src/modules/tldraw/{gateway => controller}/tldraw.gateway.spec.ts (95%) delete mode 100644 apps/server/src/modules/tldraw/gateway/index.ts create mode 100644 apps/server/src/modules/tldraw/types/ws-close-code-enum.ts create mode 100644 apps/server/src/modules/tldraw/types/ws-shared-doc.ts create mode 100644 apps/server/src/modules/tldraw/utils/ydoc-utils.ts diff --git a/apps/server/src/modules/tldraw/gateway/api-test/tldraw.gateway.api.spec.ts b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts similarity index 94% rename from apps/server/src/modules/tldraw/gateway/api-test/tldraw.gateway.api.spec.ts rename to apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts index 8aae861446c..c0f7f06def9 100644 --- a/apps/server/src/modules/tldraw/gateway/api-test/tldraw.gateway.api.spec.ts +++ b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts @@ -8,11 +8,11 @@ import { ConfigModule } from '@nestjs/config'; import { createConfigModuleOptions } from '@src/config'; import { config } from '@src/modules/tldraw/config'; import * as Utils from '../../utils/utils'; -import { TldrawGateway } from '../tldraw.gateway'; +import { TldrawController } from '../tldraw.controller'; -describe('WebSocketGateway (WsAdapter)', () => { +describe('WebSocketController (WsAdapter)', () => { let app: INestApplication; - let gateway: TldrawGateway; + let gateway: TldrawController; let ws: WebSocket; const gatewayPort = 3346; @@ -44,9 +44,9 @@ describe('WebSocketGateway (WsAdapter)', () => { const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; const testingModule = await Test.createTestingModule({ imports, - providers: [TldrawGateway], + providers: [TldrawController], }).compile(); - gateway = testingModule.get(TldrawGateway); + gateway = testingModule.get(TldrawController); app = testingModule.createNestApplication(); app.useWebSocketAdapter(new WsAdapter(app)); await app.init(); diff --git a/apps/server/src/modules/tldraw/controller/index.ts b/apps/server/src/modules/tldraw/controller/index.ts new file mode 100644 index 00000000000..61d6fc675d9 --- /dev/null +++ b/apps/server/src/modules/tldraw/controller/index.ts @@ -0,0 +1 @@ +export * from './tldraw.controller'; diff --git a/apps/server/src/modules/tldraw/gateway/tldraw.gateway.ts b/apps/server/src/modules/tldraw/controller/tldraw.controller.ts similarity index 86% rename from apps/server/src/modules/tldraw/gateway/tldraw.gateway.ts rename to apps/server/src/modules/tldraw/controller/tldraw.controller.ts index 1737a8e9075..cb758bb9527 100644 --- a/apps/server/src/modules/tldraw/gateway/tldraw.gateway.ts +++ b/apps/server/src/modules/tldraw/controller/tldraw.controller.ts @@ -3,10 +3,11 @@ import { Server, WebSocket } from 'ws'; import { MongodbPersistence } from 'y-mongodb-provider'; import { ConfigService } from '@nestjs/config'; import { TldrawConfig, SOCKET_PORT } from '@src/modules/tldraw/config'; +import { WsCloseCodeEnum } from '@src/modules/tldraw/types/ws-close-code-enum'; import { setupWSConnection, setPersistence, updateDocument } from '../utils'; @WebSocketGateway(SOCKET_PORT) -export class TldrawGateway implements OnGatewayInit, OnGatewayConnection { +export class TldrawController implements OnGatewayInit, OnGatewayConnection { @WebSocketServer() server!: Server; @@ -22,7 +23,10 @@ export class TldrawGateway implements OnGatewayInit, OnGatewayConnection { if (docName.length > 0 && this.configService.get('FEATURE_TLDRAW_ENABLED')) { setupWSConnection(client, docName); } else { - client.close(4000, 'Document name is mandatory in url or Tldraw Tool is turned off.'); + client.close( + WsCloseCodeEnum.WS_CUSTOM_CLIENT_CLOSE_CODE, + 'Document name is mandatory in url or Tldraw Tool is turned off.' + ); } } diff --git a/apps/server/src/modules/tldraw/gateway/tldraw.gateway.spec.ts b/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts similarity index 95% rename from apps/server/src/modules/tldraw/gateway/tldraw.gateway.spec.ts rename to apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts index ff5ecf9511b..f8c5b804546 100644 --- a/apps/server/src/modules/tldraw/gateway/tldraw.gateway.spec.ts +++ b/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts @@ -8,16 +8,17 @@ import { ConfigModule } from '@nestjs/config'; import { createConfigModuleOptions } from '@src/config'; import { config } from '@src/modules/tldraw/config'; import * as Utils from '@src/modules/tldraw/utils/utils'; +import * as YjsUtils from '@src/modules/tldraw/utils/ydoc-utils'; import { WSSharedDoc } from '@src/modules/tldraw/utils/utils'; import { TextEncoder } from 'util'; import * as SyncProtocols from 'y-protocols/sync'; import * as AwarenessProtocol from 'y-protocols/awareness'; import { encoding } from 'lib0'; -import { TldrawGateway } from '.'; +import { TldrawController } from '.'; describe('TldrawGateway', () => { let app: INestApplication; - let gateway: TldrawGateway; + let gateway: TldrawController; let ws: WebSocket; const gatewayPort = 3346; @@ -38,10 +39,10 @@ describe('TldrawGateway', () => { const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; const testingModule = await Test.createTestingModule({ imports, - providers: [TldrawGateway], + providers: [TldrawController], }).compile(); - gateway = testingModule.get(TldrawGateway); + gateway = testingModule.get(TldrawController); app = testingModule.createNestApplication(); app.useWebSocketAdapter(new WsAdapter(app)); jest.useFakeTimers({ advanceTimers: true, doNotFake: ['setInterval', 'clearInterval', 'setTimeout'] }); @@ -75,7 +76,7 @@ describe('TldrawGateway', () => { }; }; - it('should gateway properties be defined', async () => { + it('should controller properties be defined', async () => { await app.init(); expect(gateway).toBeDefined(); @@ -472,7 +473,7 @@ describe('TldrawGateway', () => { }; const storeUpdateSpy = jest.spyOn(mdb, 'storeUpdate'); const byteArray = new TextEncoder().encode(testMessage); - const calculateDiffSpy = jest.spyOn(Utils, 'calculateDiff').mockImplementation(() => 1); + const calculateDiffSpy = jest.spyOn(YjsUtils, 'calculateDiff').mockImplementation(() => 1); return { mdb, @@ -486,7 +487,7 @@ describe('TldrawGateway', () => { it('should store on db', async () => { const { mdb, doc, byteArray, storeUpdateSpy, calculateDiffSpy } = await setup(); - await Utils.updateDocument(mdb as MongodbPersistence, 'TEST', doc); + await YjsUtils.updateDocument(mdb as MongodbPersistence, 'TEST', doc); doc.emit('update', [byteArray, undefined, doc]); await delay(200); expect(storeUpdateSpy).toHaveBeenCalled(); @@ -512,7 +513,7 @@ describe('TldrawGateway', () => { storeUpdate: () => {}, }; const storeUpdateSpy = jest.spyOn(mdb, 'storeUpdate'); - const calculateDiffSpy = jest.spyOn(Utils, 'calculateDiff'); + const calculateDiffSpy = jest.spyOn(YjsUtils, 'calculateDiff'); return { mdb, @@ -525,7 +526,7 @@ describe('TldrawGateway', () => { it('should not update db with diff', async () => { const { mdb, doc, storeUpdateSpy, calculateDiffSpy } = await setup(); - await Utils.updateDocument(mdb as MongodbPersistence, 'TEST2', doc); + await YjsUtils.updateDocument(mdb as MongodbPersistence, 'TEST2', doc); await delay(200); expect(storeUpdateSpy).toHaveBeenCalledTimes(0); expect(calculateDiffSpy).toReturnWith(0); diff --git a/apps/server/src/modules/tldraw/gateway/index.ts b/apps/server/src/modules/tldraw/gateway/index.ts deleted file mode 100644 index eedf1eb18ca..00000000000 --- a/apps/server/src/modules/tldraw/gateway/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './tldraw.gateway'; diff --git a/apps/server/src/modules/tldraw/tldraw.module.ts b/apps/server/src/modules/tldraw/tldraw.module.ts index ed774d35f24..c4a55ed89b9 100644 --- a/apps/server/src/modules/tldraw/tldraw.module.ts +++ b/apps/server/src/modules/tldraw/tldraw.module.ts @@ -3,13 +3,13 @@ import { ConfigModule } from '@nestjs/config'; import { createConfigModuleOptions } from '@src/config'; import { CoreModule } from '@src/core'; import { Logger } from '@src/core/logger'; -import { TldrawGateway } from './gateway'; +import { TldrawController } from './controller'; import { config } from './config'; const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; @Module({ imports, - providers: [Logger, TldrawGateway], + providers: [Logger, TldrawController], }) export class TldrawModule {} diff --git a/apps/server/src/modules/tldraw/types/index.ts b/apps/server/src/modules/tldraw/types/index.ts index d56917ab845..c5241935553 100644 --- a/apps/server/src/modules/tldraw/types/index.ts +++ b/apps/server/src/modules/tldraw/types/index.ts @@ -1,3 +1,4 @@ export * from './connection-enum'; export * from './persistence-type'; export * from './request-options-type'; +export * from './ws-shared-doc'; diff --git a/apps/server/src/modules/tldraw/types/ws-close-code-enum.ts b/apps/server/src/modules/tldraw/types/ws-close-code-enum.ts new file mode 100644 index 00000000000..14e6b08e6a5 --- /dev/null +++ b/apps/server/src/modules/tldraw/types/ws-close-code-enum.ts @@ -0,0 +1,3 @@ +export enum WsCloseCodeEnum { + WS_CUSTOM_CLIENT_CLOSE_CODE = 4000, +} diff --git a/apps/server/src/modules/tldraw/types/ws-shared-doc.ts b/apps/server/src/modules/tldraw/types/ws-shared-doc.ts new file mode 100644 index 00000000000..cab1da67bea --- /dev/null +++ b/apps/server/src/modules/tldraw/types/ws-shared-doc.ts @@ -0,0 +1,62 @@ +import { Doc } from 'yjs'; +import WebSocket from 'ws'; +import { Awareness, encodeAwarenessUpdate } from 'y-protocols/awareness'; +import { encoding } from 'lib0'; +import { Configuration } from '@hpi-schul-cloud/commons'; +import { WSMessageType } from '@src/modules/tldraw/types/connection-enum'; +import { send, updateHandler } from '@src/modules/tldraw/utils'; + +// disable gc when using snapshots! +const gcEnabled: boolean = Configuration.get('TLDRAW__GC_ENABLED') as boolean; + +export class WSSharedDoc extends Doc { + name: string; + + conns: Map>; + + awareness: Awareness; + + /** + * @param {string} name + */ + constructor(name: string) { + super({ gc: gcEnabled }); + this.name = name; + this.conns = new Map(); + this.awareness = new Awareness(this); + this.awareness.setLocalState(null); + + this.awareness.on('update', this.awarenessChangeHandler); + this.on('update', updateHandler); + } + + /** + * @param {{ added: Array, updated: Array, removed: Array }} changes + * @param {WebSocket | null} conn Origin is the connection that made the change + */ + awarenessChangeHandler = ( + { added, updated, removed }: { added: Array; updated: Array; removed: Array }, + conn: WebSocket | null + ) => { + const changedClients = added.concat(updated, removed); + if (conn !== null) { + const connControlledIDs = this.conns.get(conn) as Set; + if (connControlledIDs !== undefined) { + added.forEach((clientID) => { + connControlledIDs.add(clientID); + }); + removed.forEach((clientID) => { + connControlledIDs.delete(clientID); + }); + } + } + // broadcast awareness update + const encoder = encoding.createEncoder(); + encoding.writeVarUint(encoder, WSMessageType.AWARENESS); + encoding.writeVarUint8Array(encoder, encodeAwarenessUpdate(this.awareness, changedClients)); + const buff = encoding.toUint8Array(encoder); + this.conns.forEach((_, c) => { + send(this, c, buff); + }); + }; +} diff --git a/apps/server/src/modules/tldraw/utils/index.ts b/apps/server/src/modules/tldraw/utils/index.ts index 04bca77e0de..4c985cea0ed 100644 --- a/apps/server/src/modules/tldraw/utils/index.ts +++ b/apps/server/src/modules/tldraw/utils/index.ts @@ -1 +1,2 @@ export * from './utils'; +export * from './ydoc-utils'; diff --git a/apps/server/src/modules/tldraw/utils/utils.ts b/apps/server/src/modules/tldraw/utils/utils.ts index a3f6aac640f..086c8cbc95a 100644 --- a/apps/server/src/modules/tldraw/utils/utils.ts +++ b/apps/server/src/modules/tldraw/utils/utils.ts @@ -1,16 +1,12 @@ import { setInterval, clearInterval } from 'timers'; -import { applyUpdate, Doc, encodeStateAsUpdate, encodeStateVector } from 'yjs'; -import { Awareness, encodeAwarenessUpdate, removeAwarenessStates, applyAwarenessUpdate } from 'y-protocols/awareness'; +import { encodeAwarenessUpdate, removeAwarenessStates, applyAwarenessUpdate } from 'y-protocols/awareness'; import { encoding, decoding, map } from 'lib0'; import { writeUpdate, readSyncMessage, writeSyncStep1 } from 'y-protocols/sync'; import { Configuration } from '@hpi-schul-cloud/commons'; import WebSocket from 'ws'; -import { MongodbPersistence } from 'y-mongodb-provider'; -import { WSMessageType, WSConnectionState, Persitence } from '../types'; +import { WSMessageType, WSConnectionState, Persitence, WSSharedDoc } from '../types'; const pingTimeout: number = (Configuration.get('TLDRAW__PING_TIMEOUT') as number) ?? 10000; -// disable gc when using snapshots! -const gcEnabled: boolean = Configuration.get('TLDRAW__GC_ENABLED') as boolean; /** * @type {{bindState: function(string,WSSharedDoc):void, writeState:function(string,WSSharedDoc):Promise, provider: any}|null} @@ -92,58 +88,6 @@ export const updateHandler = (update: Uint8Array, origin, doc: WSSharedDoc) => { doc.conns.forEach((_, conn) => send(doc, conn, message)); }; -export class WSSharedDoc extends Doc { - name: string; - - conns: Map>; - - awareness: Awareness; - - /** - * @param {string} name - */ - constructor(name: string) { - super({ gc: gcEnabled }); - this.name = name; - this.conns = new Map(); - this.awareness = new Awareness(this); - this.awareness.setLocalState(null); - - this.awareness.on('update', this.awarenessChangeHandler); - this.on('update', updateHandler); - } - - /** - * @param {{ added: Array, updated: Array, removed: Array }} changes - * @param {WebSocket | null} conn Origin is the connection that made the change - */ - awarenessChangeHandler = ( - { added, updated, removed }: { added: Array; updated: Array; removed: Array }, - conn: WebSocket | null - ) => { - const changedClients = added.concat(updated, removed); - if (conn !== null) { - const connControlledIDs = this.conns.get(conn) as Set; - if (connControlledIDs !== undefined) { - added.forEach((clientID) => { - connControlledIDs.add(clientID); - }); - removed.forEach((clientID) => { - connControlledIDs.delete(clientID); - }); - } - } - // broadcast awareness update - const encoder = encoding.createEncoder(); - encoding.writeVarUint(encoder, WSMessageType.AWARENESS); - encoding.writeVarUint8Array(encoder, encodeAwarenessUpdate(this.awareness, changedClients)); - const buff = encoding.toUint8Array(encoder); - this.conns.forEach((_, c) => { - send(this, c, buff); - }); - }; -} - /** * Gets a Y.Doc by name, whether in memory or on disk * @@ -255,44 +199,3 @@ export const setupWSConnection = (ws: WebSocket, docName = 'GLOBAL') => { } } }; - -// eslint-disable-next-line consistent-return -export const getYDocFromMdb = async (mdb: MongodbPersistence, docName: string) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access - const yDoc = await mdb.getYDoc(docName); - if (yDoc instanceof Doc) { - return yDoc; - } -}; - -export const calculateDiff = (diff: Uint8Array) => - diff.reduce((previousValue, currentValue) => previousValue + currentValue, 0); - -export const updateStoredDocWithDiff = (mdb: MongodbPersistence, docName: string, diff: Uint8Array) => { - if (calculateDiff(diff) > 0) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access - mdb.storeUpdate(docName, diff); - } -}; - -export const updateDocument = async (mdb: MongodbPersistence, docName: string, ydoc: WSSharedDoc) => { - const persistedYdoc = await getYDocFromMdb(mdb, docName); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - const persistedStateVector = encodeStateVector(persistedYdoc); - const diff = encodeStateAsUpdate(ydoc, persistedStateVector); - updateStoredDocWithDiff(mdb, docName, diff); - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - applyUpdate(ydoc, encodeStateAsUpdate(persistedYdoc)); - - ydoc.on('update', (update) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access - mdb.storeUpdate(docName, update); - }); - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - persistedYdoc.destroy(); -}; diff --git a/apps/server/src/modules/tldraw/utils/ydoc-utils.ts b/apps/server/src/modules/tldraw/utils/ydoc-utils.ts new file mode 100644 index 00000000000..f5c4ec80693 --- /dev/null +++ b/apps/server/src/modules/tldraw/utils/ydoc-utils.ts @@ -0,0 +1,44 @@ +import { applyUpdate, Doc, encodeStateAsUpdate, encodeStateVector } from 'yjs'; +import { MongodbPersistence } from 'y-mongodb-provider'; +import { WSSharedDoc } from '@src/modules/tldraw/types/ws-shared-doc'; + +// eslint-disable-next-line consistent-return +export const getYDocFromMdb = async (mdb: MongodbPersistence, docName: string) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access + const yDoc = await mdb.getYDoc(docName); + if (yDoc instanceof Doc) { + return yDoc; + } +}; + +export const calculateDiff = (diff: Uint8Array) => + diff.reduce((previousValue, currentValue) => previousValue + currentValue, 0); + +export const updateStoredDocWithDiff = (mdb: MongodbPersistence, docName: string, diff: Uint8Array) => { + if (calculateDiff(diff) > 0) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access + mdb.storeUpdate(docName, diff); + } +}; + +export const updateDocument = async (mdb: MongodbPersistence, docName: string, ydoc: WSSharedDoc) => { + const persistedYdoc = await getYDocFromMdb(mdb, docName); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const persistedStateVector = encodeStateVector(persistedYdoc); + const diff = encodeStateAsUpdate(ydoc, persistedStateVector); + updateStoredDocWithDiff(mdb, docName, diff); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + applyUpdate(ydoc, encodeStateAsUpdate(persistedYdoc)); + + ydoc.on('update', (update) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access + mdb.storeUpdate(docName, update); + }); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + persistedYdoc.destroy(); +}; From 832630071a859299a4db0591cc48e7aab001d625 Mon Sep 17 00:00:00 2001 From: Tomasz Wiaderek Date: Mon, 25 Sep 2023 19:26:08 +0200 Subject: [PATCH 08/10] code refactor --- apps/server/src/modules/tldraw/types/persistence-type.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/modules/tldraw/types/persistence-type.ts b/apps/server/src/modules/tldraw/types/persistence-type.ts index fa36d8c20a6..b880eae6544 100644 --- a/apps/server/src/modules/tldraw/types/persistence-type.ts +++ b/apps/server/src/modules/tldraw/types/persistence-type.ts @@ -1,4 +1,4 @@ -import { WSSharedDoc } from '@src/modules/tldraw/utils'; +import { WSSharedDoc } from '@src/modules/tldraw/types/ws-shared-doc'; export type Persitence = { bindState: (docName: string, ydoc: WSSharedDoc) => Promise; From a4784bf805d72f0e503d3aac153df9a803fc0f5f Mon Sep 17 00:00:00 2001 From: Tomasz Wiaderek Date: Mon, 25 Sep 2023 19:52:25 +0200 Subject: [PATCH 09/10] refactor tests --- .../api-test/tldraw.controller.api.spec.ts | 4 +- .../tldraw/controller/tldraw.gateway.spec.ts | 56 +++++++------------ 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts index c0f7f06def9..6fa1131a1d0 100644 --- a/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts +++ b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts @@ -67,7 +67,7 @@ describe('WebSocketController (WsAdapter)', () => { describe('when tldraw is correctly setup', () => { const setup = async () => { const handleConnectionSpy = jest.spyOn(gateway, 'handleConnection'); - jest.spyOn(Uint8Array.prototype, 'reduce').mockReturnValue(1); + jest.spyOn(Uint8Array.prototype, 'reduce').mockReturnValueOnce(1); await setupWs('TEST'); @@ -137,7 +137,7 @@ describe('WebSocketController (WsAdapter)', () => { describe('when tldraw is not correctly setup', () => { const setup = async () => { const handleConnectionSpy = jest.spyOn(gateway, 'handleConnection'); - const utilsSpy = jest.spyOn(Utils, 'messageHandler').mockReturnValue(); + const utilsSpy = jest.spyOn(Utils, 'messageHandler').mockReturnValueOnce(); await setupWs(); diff --git a/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts b/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts index f8c5b804546..03b37a9d384 100644 --- a/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts +++ b/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts @@ -9,7 +9,7 @@ import { createConfigModuleOptions } from '@src/config'; import { config } from '@src/modules/tldraw/config'; import * as Utils from '@src/modules/tldraw/utils/utils'; import * as YjsUtils from '@src/modules/tldraw/utils/ydoc-utils'; -import { WSSharedDoc } from '@src/modules/tldraw/utils/utils'; +import { WSSharedDoc } from '@src/modules/tldraw/types'; import { TextEncoder } from 'util'; import * as SyncProtocols from 'y-protocols/sync'; import * as AwarenessProtocol from 'y-protocols/awareness'; @@ -35,7 +35,7 @@ describe('TldrawGateway', () => { jest.useFakeTimers(); - beforeEach(async () => { + beforeAll(async () => { const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; const testingModule = await Test.createTestingModule({ imports, @@ -46,9 +46,10 @@ describe('TldrawGateway', () => { app = testingModule.createNestApplication(); app.useWebSocketAdapter(new WsAdapter(app)); jest.useFakeTimers({ advanceTimers: true, doNotFake: ['setInterval', 'clearInterval', 'setTimeout'] }); + await app.init(); }); - afterEach(async () => { + afterAll(async () => { await app.close(); }); @@ -76,9 +77,7 @@ describe('TldrawGateway', () => { }; }; - it('should controller properties be defined', async () => { - await app.init(); - + it('should controller properties be defined', () => { expect(gateway).toBeDefined(); expect(gateway.configService).toBeDefined(); expect(gateway.server).toBeDefined(); @@ -88,7 +87,6 @@ describe('TldrawGateway', () => { describe('when client is not connected to WS', () => { const setup = async () => { - await app.init(); await setupWs(); const closeConSpy = jest.spyOn(Utils, 'closeConn').mockImplementationOnce(() => {}); @@ -119,9 +117,7 @@ describe('TldrawGateway', () => { }); describe('when websocket has ready state different than 0 or 1', () => { - const setup = async () => { - await app.init(); - + const setup = () => { const closeConSpy = jest.spyOn(Utils, 'closeConn'); const sendSpy = jest.spyOn(Utils, 'send'); const doc: { conns: Map> } = { conns: new Map() }; @@ -137,8 +133,8 @@ describe('TldrawGateway', () => { }; }; - it('should close connection if websocket has ready state different than 0 or 1', async () => { - const { closeConSpy, sendSpy, doc, socketMock, byteArray } = await setup(); + it('should close connection if websocket has ready state different than 0 or 1', () => { + const { closeConSpy, sendSpy, doc, socketMock, byteArray } = setup(); Utils.send(doc as WSSharedDoc, socketMock as WebSocket, byteArray); @@ -153,7 +149,6 @@ describe('TldrawGateway', () => { describe('when websocket has ready state 0', () => { const setup = async () => { - await app.init(); await setupWs(); const sendSpy = jest.spyOn(Utils, 'send'); @@ -186,7 +181,6 @@ describe('TldrawGateway', () => { describe('when awareness change was called', () => { const setup = async () => { - await app.init(); await setupWs(); class MockAwareness { @@ -205,7 +199,7 @@ describe('TldrawGateway', () => { doc.awareness.states = states; doc.awareness.meta = mockMeta; - const sendSpy = jest.spyOn(Utils, 'send').mockReturnValue(); + const sendSpy = jest.spyOn(Utils, 'send').mockReturnValueOnce(); const mockIDs = new Set(); const mockConns = new Map>(); @@ -226,7 +220,7 @@ describe('TldrawGateway', () => { }; }; - it('should correctly update awwereness', async () => { + it('should correctly update awareness', async () => { const { sendSpy, doc, mockIDs, awarenessUpdate } = await setup(); doc.awarenessChangeHandler(awarenessUpdate, ws); @@ -244,12 +238,11 @@ describe('TldrawGateway', () => { describe('when received message of specific type', () => { const setup = async (messageValues: number[]) => { - await app.init(); await setupWs('TEST'); const sendSpy = jest.spyOn(Utils, 'send'); const applyAwarenessUpdateSpy = jest.spyOn(AwarenessProtocol, 'applyAwarenessUpdate'); - jest.spyOn(SyncProtocols, 'readSyncMessage').mockImplementation((dec, enc) => { + jest.spyOn(SyncProtocols, 'readSyncMessage').mockImplementationOnce((dec, enc) => { enc.bufs = [new Uint8Array(2), new Uint8Array(2)]; return 1; }); @@ -302,11 +295,10 @@ describe('TldrawGateway', () => { describe('when message is sent', () => { const setup = async (messageValues: number[]) => { - await app.init(); await setupWs('TEST'); const messageHandlerSpy = jest.spyOn(Utils, 'messageHandler'); - jest.spyOn(SyncProtocols, 'readSyncMessage').mockImplementation((dec, enc) => { + jest.spyOn(SyncProtocols, 'readSyncMessage').mockImplementationOnce((dec, enc) => { enc.bufs = [new Uint8Array(2), new Uint8Array(2)]; return 1; }); @@ -333,11 +325,10 @@ describe('TldrawGateway', () => { describe('when error is thrown', () => { const setup = async () => { - await app.init(); await setupWs(); const sendSpy = jest.spyOn(Utils, 'send'); - jest.spyOn(SyncProtocols, 'readSyncMessage').mockImplementation(() => { + jest.spyOn(SyncProtocols, 'readSyncMessage').mockImplementationOnce(() => { throw new Error('error'); }); const doc = new WSSharedDoc('TEST'); @@ -364,7 +355,6 @@ describe('TldrawGateway', () => { describe('when trying to close already closed connection', () => { const setup = async () => { - await app.init(); await setupWs(); jest.spyOn(ws, 'close').mockImplementationOnce(() => { @@ -387,12 +377,11 @@ describe('TldrawGateway', () => { describe('when ping failed', () => { const setup = async () => { - await app.init(); await setupWs('TEST'); - const messageHandlerSpy = jest.spyOn(Utils, 'messageHandler').mockImplementation(() => {}); + const messageHandlerSpy = jest.spyOn(Utils, 'messageHandler').mockImplementationOnce(() => {}); const closeConnSpy = jest.spyOn(Utils, 'closeConn'); - jest.spyOn(ws, 'ping').mockImplementation(() => { + jest.spyOn(ws, 'ping').mockImplementationOnce(() => { throw new Error('error'); }); @@ -419,7 +408,6 @@ describe('TldrawGateway', () => { describe('when awareness states size greater then one', () => { const setup = async () => { - await app.init(); await setupWs('TEST'); const doc = new WSSharedDoc('TEST'); @@ -427,11 +415,11 @@ describe('TldrawGateway', () => { doc.awareness.states.set(1, ['test1']); doc.awareness.states.set(2, ['test2']); - const messageHandlerSpy = jest.spyOn(Utils, 'messageHandler').mockImplementation(() => {}); + const messageHandlerSpy = jest.spyOn(Utils, 'messageHandler').mockImplementationOnce(() => {}); const sendSpy = jest.spyOn(Utils, 'send'); - const getYDocSpy = jest.spyOn(Utils, 'getYDoc').mockImplementation(() => doc); + const getYDocSpy = jest.spyOn(Utils, 'getYDoc').mockImplementationOnce(() => doc); const { msg } = createMessage([0, 1]); - jest.spyOn(AwarenessProtocol, 'encodeAwarenessUpdate').mockImplementation(() => msg); + jest.spyOn(AwarenessProtocol, 'encodeAwarenessUpdate').mockImplementationOnce(() => msg); return { messageHandlerSpy, @@ -447,7 +435,7 @@ describe('TldrawGateway', () => { await delay(200); - expect(sendSpy).toHaveBeenCalledTimes(2); + expect(sendSpy).toHaveBeenCalledTimes(3); ws.close(); messageHandlerSpy.mockRestore(); @@ -458,8 +446,6 @@ describe('TldrawGateway', () => { describe('when document receive update', () => { const setup = async () => { - await app.init(); - const doc = new WSSharedDoc('TEST'); await setupWs('TEST'); const wsSet = new Set(); @@ -473,7 +459,7 @@ describe('TldrawGateway', () => { }; const storeUpdateSpy = jest.spyOn(mdb, 'storeUpdate'); const byteArray = new TextEncoder().encode(testMessage); - const calculateDiffSpy = jest.spyOn(YjsUtils, 'calculateDiff').mockImplementation(() => 1); + const calculateDiffSpy = jest.spyOn(YjsUtils, 'calculateDiff').mockImplementationOnce(() => 1); return { mdb, @@ -499,8 +485,6 @@ describe('TldrawGateway', () => { describe('when document receive empty update', () => { const setup = async () => { - await app.init(); - const doc = new WSSharedDoc('TEST2'); await setupWs('TEST2'); const wsSet = new Set(); From 97f5b6c974c8548b5358158dfb63bbc192e4482a Mon Sep 17 00:00:00 2001 From: blazejpass Date: Mon, 25 Sep 2023 21:07:29 +0200 Subject: [PATCH 10/10] Change naming tldraw controller --- .../controller/api-test/tldraw.controller.api.spec.ts | 8 ++++---- apps/server/src/modules/tldraw/controller/index.ts | 2 +- ...aw.gateway.spec.ts => tldraw-ws.controller.spec.ts} | 10 +++++----- .../{tldraw.controller.ts => tldraw-ws.controller.ts} | 2 +- apps/server/src/modules/tldraw/tldraw.module.ts | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) rename apps/server/src/modules/tldraw/controller/{tldraw.gateway.spec.ts => tldraw-ws.controller.spec.ts} (98%) rename apps/server/src/modules/tldraw/controller/{tldraw.controller.ts => tldraw-ws.controller.ts} (96%) diff --git a/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts index 6fa1131a1d0..fbd5fa47cc4 100644 --- a/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts +++ b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.api.spec.ts @@ -8,11 +8,11 @@ import { ConfigModule } from '@nestjs/config'; import { createConfigModuleOptions } from '@src/config'; import { config } from '@src/modules/tldraw/config'; import * as Utils from '../../utils/utils'; -import { TldrawController } from '../tldraw.controller'; +import { TldrawWsController } from '../tldraw-ws.controller'; describe('WebSocketController (WsAdapter)', () => { let app: INestApplication; - let gateway: TldrawController; + let gateway: TldrawWsController; let ws: WebSocket; const gatewayPort = 3346; @@ -44,9 +44,9 @@ describe('WebSocketController (WsAdapter)', () => { const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; const testingModule = await Test.createTestingModule({ imports, - providers: [TldrawController], + providers: [TldrawWsController], }).compile(); - gateway = testingModule.get(TldrawController); + gateway = testingModule.get(TldrawWsController); app = testingModule.createNestApplication(); app.useWebSocketAdapter(new WsAdapter(app)); await app.init(); diff --git a/apps/server/src/modules/tldraw/controller/index.ts b/apps/server/src/modules/tldraw/controller/index.ts index 61d6fc675d9..af9f4b335f5 100644 --- a/apps/server/src/modules/tldraw/controller/index.ts +++ b/apps/server/src/modules/tldraw/controller/index.ts @@ -1 +1 @@ -export * from './tldraw.controller'; +export * from './tldraw-ws.controller'; diff --git a/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts b/apps/server/src/modules/tldraw/controller/tldraw-ws.controller.spec.ts similarity index 98% rename from apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts rename to apps/server/src/modules/tldraw/controller/tldraw-ws.controller.spec.ts index 03b37a9d384..9bfb41ffec3 100644 --- a/apps/server/src/modules/tldraw/controller/tldraw.gateway.spec.ts +++ b/apps/server/src/modules/tldraw/controller/tldraw-ws.controller.spec.ts @@ -14,11 +14,11 @@ import { TextEncoder } from 'util'; import * as SyncProtocols from 'y-protocols/sync'; import * as AwarenessProtocol from 'y-protocols/awareness'; import { encoding } from 'lib0'; -import { TldrawController } from '.'; +import { TldrawWsController } from '.'; -describe('TldrawGateway', () => { +describe('TldrawWSConytoller', () => { let app: INestApplication; - let gateway: TldrawController; + let gateway: TldrawWsController; let ws: WebSocket; const gatewayPort = 3346; @@ -39,10 +39,10 @@ describe('TldrawGateway', () => { const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; const testingModule = await Test.createTestingModule({ imports, - providers: [TldrawController], + providers: [TldrawWsController], }).compile(); - gateway = testingModule.get(TldrawController); + gateway = testingModule.get(TldrawWsController); app = testingModule.createNestApplication(); app.useWebSocketAdapter(new WsAdapter(app)); jest.useFakeTimers({ advanceTimers: true, doNotFake: ['setInterval', 'clearInterval', 'setTimeout'] }); diff --git a/apps/server/src/modules/tldraw/controller/tldraw.controller.ts b/apps/server/src/modules/tldraw/controller/tldraw-ws.controller.ts similarity index 96% rename from apps/server/src/modules/tldraw/controller/tldraw.controller.ts rename to apps/server/src/modules/tldraw/controller/tldraw-ws.controller.ts index cb758bb9527..7b120552976 100644 --- a/apps/server/src/modules/tldraw/controller/tldraw.controller.ts +++ b/apps/server/src/modules/tldraw/controller/tldraw-ws.controller.ts @@ -7,7 +7,7 @@ import { WsCloseCodeEnum } from '@src/modules/tldraw/types/ws-close-code-enum'; import { setupWSConnection, setPersistence, updateDocument } from '../utils'; @WebSocketGateway(SOCKET_PORT) -export class TldrawController implements OnGatewayInit, OnGatewayConnection { +export class TldrawWsController implements OnGatewayInit, OnGatewayConnection { @WebSocketServer() server!: Server; diff --git a/apps/server/src/modules/tldraw/tldraw.module.ts b/apps/server/src/modules/tldraw/tldraw.module.ts index c4a55ed89b9..a6e05901d5e 100644 --- a/apps/server/src/modules/tldraw/tldraw.module.ts +++ b/apps/server/src/modules/tldraw/tldraw.module.ts @@ -3,13 +3,13 @@ import { ConfigModule } from '@nestjs/config'; import { createConfigModuleOptions } from '@src/config'; import { CoreModule } from '@src/core'; import { Logger } from '@src/core/logger'; -import { TldrawController } from './controller'; +import { TldrawWsController } from './controller'; import { config } from './config'; const imports = [CoreModule, ConfigModule.forRoot(createConfigModuleOptions(config))]; @Module({ imports, - providers: [Logger, TldrawController], + providers: [Logger, TldrawWsController], }) export class TldrawModule {}