diff --git a/.gitignore b/.gitignore index c5a9ad2183f..af88458f081 100644 --- a/.gitignore +++ b/.gitignore @@ -88,3 +88,6 @@ build /.idea/ /apps/server/src/modules/board/loadtest/**/*.html /apps/server/src/modules/board/loadtest/artilleryreport.json + +# Docker +docker-compose.yml diff --git a/apps/server/src/infra/sync/sync.module.ts b/apps/server/src/infra/sync/sync.module.ts index 0bf0c2dc0ec..342d604bbf7 100644 --- a/apps/server/src/infra/sync/sync.module.ts +++ b/apps/server/src/infra/sync/sync.module.ts @@ -18,6 +18,7 @@ import { TspSyncService } from './tsp/tsp-sync.service'; import { TspSyncStrategy } from './tsp/tsp-sync.strategy'; import { SyncUc } from './uc/sync.uc'; import { TspFetchService } from './tsp/tsp-fetch.service'; +import { TspSyncMigrationService } from './tsp/tsp-sync-migration.service'; @Module({ imports: [ @@ -41,7 +42,14 @@ import { TspFetchService } from './tsp/tsp-fetch.service'; SyncUc, SyncService, ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) - ? [TspSyncStrategy, TspSyncService, TspOauthDataMapper, TspFetchService, TspLegacyMigrationService] + ? [ + TspSyncStrategy, + TspSyncService, + TspOauthDataMapper, + TspFetchService, + TspLegacyMigrationService, + TspSyncMigrationService, + ] : []), ], exports: [SyncConsole], diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-start.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-start.loggable.ts index c3af20dc0c7..d70e760a188 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-start.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-start.loggable.ts @@ -1,7 +1,7 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspLegacyMigrationStartLoggable implements Loggable { - getLogMessage(): LogMessage { + public getLogMessage(): LogMessage { const message: LogMessage = { message: 'Running migration of legacy tsp data.', }; diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts index fcdf3b26d0a..b7c4dbf9b25 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts @@ -1,7 +1,7 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspLegacyMigrationSystemMissingLoggable implements Loggable { - getLogMessage(): LogMessage { + public getLogMessage(): LogMessage { const message: LogMessage = { message: 'No legacy system found', }; diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts index c04fc6b5a53..bea2c2a91b4 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts @@ -3,7 +3,7 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspLegacySchoolMigrationCountLoggable implements Loggable { constructor(private readonly total: number) {} - getLogMessage(): LogMessage { + public getLogMessage(): LogMessage { const message: LogMessage = { message: `Found ${this.total} legacy tsp schools to migrate`, data: { diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts index b6ac7b247e2..bfd70da05d2 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts @@ -3,7 +3,7 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspLegacySchoolMigrationSuccessLoggable implements Loggable { constructor(private readonly total: number, private readonly migrated: number) {} - getLogMessage(): LogMessage { + public getLogMessage(): LogMessage { const message: LogMessage = { message: `Legacy tsp data migration finished. Total schools: ${this.total}, migrated schools: ${this.migrated}`, data: { diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-migration-batch-summary.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-migration-batch-summary.loggable.spec.ts new file mode 100644 index 00000000000..88f1bffc3d9 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-migration-batch-summary.loggable.spec.ts @@ -0,0 +1,30 @@ +import { TspMigrationBatchSummaryLoggable } from './tsp-migration-batch-summary.loggable'; + +describe(TspMigrationBatchSummaryLoggable.name, () => { + let loggable: TspMigrationBatchSummaryLoggable; + + beforeAll(() => { + loggable = new TspMigrationBatchSummaryLoggable(1, 2, 3, 4, 5); + }); + + describe('when loggable is initialized', () => { + it('should be defined', () => { + expect(loggable).toBeDefined(); + }); + }); + + describe('getLogMessage', () => { + it('should return a log message', () => { + expect(loggable.getLogMessage()).toEqual({ + message: `Migrated 2 users and 3 accounts in batch of size 1 (total done: 4, total migrations: 5)`, + data: { + batchSize: 1, + usersUpdated: 2, + accountsUpdated: 3, + totalDone: 4, + totalMigrations: 5, + }, + }); + }); + }); +}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-migration-batch-summary.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-migration-batch-summary.loggable.ts new file mode 100644 index 00000000000..475458d0d5f --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-migration-batch-summary.loggable.ts @@ -0,0 +1,26 @@ +import { Loggable, LogMessage } from '@src/core/logger'; + +export class TspMigrationBatchSummaryLoggable implements Loggable { + constructor( + private readonly batchSize: number, + private readonly usersUpdated: number, + private readonly accountsUpdated: number, + private readonly totalDone: number, + private readonly totalMigrations: number + ) {} + + public getLogMessage(): LogMessage { + const message: LogMessage = { + message: `Migrated ${this.usersUpdated} users and ${this.accountsUpdated} accounts in batch of size ${this.batchSize} (total done: ${this.totalDone}, total migrations: ${this.totalMigrations})`, + data: { + batchSize: this.batchSize, + usersUpdated: this.usersUpdated, + accountsUpdated: this.accountsUpdated, + totalDone: this.totalDone, + totalMigrations: this.totalMigrations, + }, + }; + + return message; + } +} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-students-fetched.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-migrations-fetched.loggable.spec.ts similarity index 50% rename from apps/server/src/infra/sync/tsp/loggable/tsp-students-fetched.loggable.spec.ts rename to apps/server/src/infra/sync/tsp/loggable/tsp-migrations-fetched.loggable.spec.ts index eeef45f5d8a..2f4f734fc91 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-students-fetched.loggable.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-migrations-fetched.loggable.spec.ts @@ -1,10 +1,10 @@ -import { TspStudentsFetchedLoggable } from './tsp-students-fetched.loggable'; +import { TspMigrationsFetchedLoggable } from './tsp-migrations-fetched.loggable'; -describe(TspStudentsFetchedLoggable.name, () => { - let loggable: TspStudentsFetchedLoggable; +describe(TspMigrationsFetchedLoggable.name, () => { + let loggable: TspMigrationsFetchedLoggable; beforeAll(() => { - loggable = new TspStudentsFetchedLoggable(10); + loggable = new TspMigrationsFetchedLoggable(10); }); describe('when loggable is initialized', () => { @@ -16,9 +16,9 @@ describe(TspStudentsFetchedLoggable.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ - message: `Fetched 10 students for migration from TSP`, + message: `Fetched 10 users for migration from TSP`, data: { - tspStudentCount: 10, + tspUserMigrationCount: 10, }, }); }); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-migrations-fetched.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-migrations-fetched.loggable.ts new file mode 100644 index 00000000000..34e55dc87bf --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-migrations-fetched.loggable.ts @@ -0,0 +1,16 @@ +import { Loggable, LogMessage } from '@src/core/logger'; + +export class TspMigrationsFetchedLoggable implements Loggable { + constructor(private readonly tspUserMigrationCount: number) {} + + public getLogMessage(): LogMessage { + const message: LogMessage = { + message: `Fetched ${this.tspUserMigrationCount} users for migration from TSP`, + data: { + tspUserMigrationCount: this.tspUserMigrationCount, + }, + }; + + return message; + } +} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-students-fetched.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-students-fetched.loggable.ts deleted file mode 100644 index 5a8af5758b3..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-students-fetched.loggable.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Loggable, LogMessage } from '@src/core/logger'; - -export class TspStudentsFetchedLoggable implements Loggable { - constructor(private readonly tspStudentCount: number) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: `Fetched ${this.tspStudentCount} students for migration from TSP`, - data: { - tspStudentCount: this.tspStudentCount, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-students-migrated.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-students-migrated.loggable.spec.ts deleted file mode 100644 index 725a1b845a8..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-students-migrated.loggable.spec.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { TspStudentsMigratedLoggable } from './tsp-students-migrated.loggable'; - -describe(TspStudentsMigratedLoggable.name, () => { - let loggable: TspStudentsMigratedLoggable; - - beforeAll(() => { - loggable = new TspStudentsMigratedLoggable(10); - }); - - describe('when loggable is initialized', () => { - it('should be defined', () => { - expect(loggable).toBeDefined(); - }); - }); - - describe('getLogMessage', () => { - it('should return a log message', () => { - expect(loggable.getLogMessage()).toEqual({ - message: `Migrated students: 10 students migrated`, - data: { - migratedStudents: 10, - }, - }); - }); - }); -}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-students-migrated.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-students-migrated.loggable.ts deleted file mode 100644 index 5937433ea6e..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-students-migrated.loggable.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Loggable, LogMessage } from '@src/core/logger'; - -export class TspStudentsMigratedLoggable implements Loggable { - constructor(private readonly migratedStudents: number) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: `Migrated students: ${this.migratedStudents} students migrated`, - data: { - migratedStudents: this.migratedStudents, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-fetched.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-fetched.loggable.spec.ts deleted file mode 100644 index cf272bbf0f0..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-fetched.loggable.spec.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { TspTeachersFetchedLoggable } from './tsp-teachers-fetched.loggable'; - -describe(TspTeachersFetchedLoggable.name, () => { - let loggable: TspTeachersFetchedLoggable; - - beforeAll(() => { - loggable = new TspTeachersFetchedLoggable(10); - }); - - describe('when loggable is initialized', () => { - it('should be defined', () => { - expect(loggable).toBeDefined(); - }); - }); - - describe('getLogMessage', () => { - it('should return a log message', () => { - expect(loggable.getLogMessage()).toEqual({ - message: `Fetched 10 teachers for migration from TSP`, - data: { - tspTeacherCount: 10, - }, - }); - }); - }); -}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-fetched.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-fetched.loggable.ts deleted file mode 100644 index 476327462bc..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-fetched.loggable.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Loggable, LogMessage } from '@src/core/logger'; - -export class TspTeachersFetchedLoggable implements Loggable { - constructor(private readonly tspTeacherCount: number) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: `Fetched ${this.tspTeacherCount} teachers for migration from TSP`, - data: { - tspTeacherCount: this.tspTeacherCount, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-migrated.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-migrated.loggable.spec.ts deleted file mode 100644 index df525470f2a..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-migrated.loggable.spec.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { TspTeachersMigratedLoggable } from './tsp-teachers-migrated.loggable'; - -describe(TspTeachersMigratedLoggable.name, () => { - let loggable: TspTeachersMigratedLoggable; - - beforeAll(() => { - loggable = new TspTeachersMigratedLoggable(10); - }); - - describe('when loggable is initialized', () => { - it('should be defined', () => { - expect(loggable).toBeDefined(); - }); - }); - - describe('getLogMessage', () => { - it('should return a log message', () => { - expect(loggable.getLogMessage()).toEqual({ - message: `Migrated teachers: 10 teachers migrated`, - data: { - migratedTeachers: 10, - }, - }); - }); - }); -}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-migrated.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-migrated.loggable.ts deleted file mode 100644 index ebbe515d06c..00000000000 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-teachers-migrated.loggable.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Loggable, LogMessage } from '@src/core/logger'; - -export class TspTeachersMigratedLoggable implements Loggable { - constructor(private readonly migratedTeachers: number) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: `Migrated teachers: ${this.migratedTeachers} teachers migrated`, - data: { - migratedTeachers: this.migratedTeachers, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.spec.ts index 22eb67a8ad9..658aef28519 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.spec.ts @@ -4,7 +4,7 @@ describe(TspUsersMigratedLoggable.name, () => { let loggable: TspUsersMigratedLoggable; beforeAll(() => { - loggable = new TspUsersMigratedLoggable(10); + loggable = new TspUsersMigratedLoggable(10, 5, 4); }); describe('when loggable is initialized', () => { @@ -16,9 +16,11 @@ describe(TspUsersMigratedLoggable.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ - message: `Migrated users: 10 users migrated`, + message: `Migrated 5 users and 4 accounts. Total amount of migrations requested: 10`, data: { - migratedUsers: 10, + totalMigrations: 10, + migratedUsers: 5, + migratedAccounts: 4, }, }); }); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.ts index 9000de6b9bc..b92cedb192b 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-users-migrated.loggable.ts @@ -1,13 +1,19 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspUsersMigratedLoggable implements Loggable { - constructor(private readonly migratedUsers: number) {} + constructor( + private readonly totalMigrations: number, + private readonly migratedUsers: number, + private readonly migratedAccounts: number + ) {} public getLogMessage(): LogMessage { const message: LogMessage = { - message: `Migrated users: ${this.migratedUsers} users migrated`, + message: `Migrated ${this.migratedUsers} users and ${this.migratedAccounts} accounts. Total amount of migrations requested: ${this.totalMigrations}`, data: { + totalMigrations: this.totalMigrations, migratedUsers: this.migratedUsers, + migratedAccounts: this.migratedAccounts, }, }; diff --git a/apps/server/src/infra/sync/tsp/tsp-fetch.service.ts b/apps/server/src/infra/sync/tsp/tsp-fetch.service.ts index 9285fbfd9a8..13976ec0259 100644 --- a/apps/server/src/infra/sync/tsp/tsp-fetch.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-fetch.service.ts @@ -1,7 +1,16 @@ import { Injectable } from '@nestjs/common'; import { AxiosErrorLoggable, ErrorLoggable } from '@src/core/error/loggable'; import { Logger } from '@src/core/logger'; -import { ExportApiInterface, TspClientFactory } from '@src/infra/tsp-client'; +import { + ExportApiInterface, + RobjExportKlasse, + RobjExportLehrer, + RobjExportLehrerMigration, + RobjExportSchueler, + RobjExportSchuelerMigration, + RobjExportSchule, + TspClientFactory, +} from '@src/infra/tsp-client'; import { OauthConfigMissingLoggableException } from '@src/modules/oauth/loggable'; import { System } from '@src/modules/system'; import { AxiosError, AxiosResponse } from 'axios'; @@ -13,41 +22,41 @@ export class TspFetchService { this.logger.setContext(TspFetchService.name); } - public async fetchTspSchools(system: System, daysToFetch: number) { + public fetchTspSchools(system: System, daysToFetch: number): Promise { const lastChangeDate = this.formatChangeDate(daysToFetch); - const schools = await this.fetchTsp(system, (client) => client.exportSchuleList(lastChangeDate), []); + const schools = this.fetchTsp(system, (client) => client.exportSchuleList(lastChangeDate), []); return schools; } - public async fetchTspTeachers(system: System, daysToFetch: number) { + public fetchTspTeachers(system: System, daysToFetch: number): Promise { const lastChangeDate = this.formatChangeDate(daysToFetch); - const teachers = await this.fetchTsp(system, (client) => client.exportLehrerList(lastChangeDate), []); + const teachers = this.fetchTsp(system, (client) => client.exportLehrerList(lastChangeDate), []); return teachers; } - public async fetchTspStudents(system: System, daysToFetch: number) { + public fetchTspStudents(system: System, daysToFetch: number): Promise { const lastChangeDate = this.formatChangeDate(daysToFetch); - const students = await this.fetchTsp(system, (client) => client.exportSchuelerList(lastChangeDate), []); + const students = this.fetchTsp(system, (client) => client.exportSchuelerList(lastChangeDate), []); return students; } - public async fetchTspClasses(system: System, daysToFetch: number) { + public fetchTspClasses(system: System, daysToFetch: number): Promise { const lastChangeDate = this.formatChangeDate(daysToFetch); - const classes = await this.fetchTsp(system, (client) => client.exportKlasseList(lastChangeDate), []); + const classes = this.fetchTsp(system, (client) => client.exportKlasseList(lastChangeDate), []); return classes; } - public async fetchTspTeacherMigrations(system: System) { + public fetchTspTeacherMigrations(system: System): Promise { const migrations = this.fetchTsp(system, (client) => client.exportLehrerListMigration(), []); return migrations; } - public async fetchTspStudentMigrations(system: System) { + public fetchTspStudentMigrations(system: System): Promise { const migrations = this.fetchTsp(system, (client) => client.exportSchuelerListMigration(), []); return migrations; @@ -78,7 +87,7 @@ export class TspFetchService { return moment(new Date()).subtract(daysToFetch, 'days').subtract(1, 'hours').format('YYYY-MM-DD HH:mm:ss.SSS'); } - private createClient(system: System) { + private createClient(system: System): ExportApiInterface { if (!system.oauthConfig) { throw new OauthConfigMissingLoggableException(system.id); } diff --git a/apps/server/src/infra/sync/tsp/tsp-sync-migration.service.spec.ts b/apps/server/src/infra/sync/tsp/tsp-sync-migration.service.spec.ts new file mode 100644 index 00000000000..fb3d1586757 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/tsp-sync-migration.service.spec.ts @@ -0,0 +1,141 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { AccountService } from '@modules/account'; +import { systemFactory } from '@modules/system/testing'; +import { UserService } from '@modules/user'; +import { ConfigService } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { UserSourceOptions } from '@shared/domain/domainobject/user-source-options.do'; +import { userDoFactory } from '@shared/testing'; +import { Logger } from '@src/core/logger'; +import { accountDoFactory } from '@src/modules/account/testing'; +import { TspSyncMigrationService } from './tsp-sync-migration.service'; +import { TspSyncConfig } from './tsp-sync.config'; + +describe(TspSyncMigrationService.name, () => { + let module: TestingModule; + let sut: TspSyncMigrationService; + let userService: DeepMocked; + let accountService: DeepMocked; + let configService: DeepMocked>; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + TspSyncMigrationService, + { + provide: Logger, + useValue: createMock(), + }, + { + provide: UserService, + useValue: createMock(), + }, + { + provide: AccountService, + useValue: createMock(), + }, + { + provide: ConfigService, + useValue: createMock>(), + }, + ], + }).compile(); + + sut = module.get(TspSyncMigrationService); + userService = module.get(UserService); + accountService = module.get(AccountService); + configService = module.get(ConfigService); + }); + + afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + }); + + afterAll(async () => { + await module.close(); + }); + + describe('when sync migration service is initialized', () => { + it('should be defined', () => { + expect(sut).toBeDefined(); + }); + }); + + describe('migrateTspUsers', () => { + describe('when old id, new id and user exist', () => { + const setup = () => { + const system = systemFactory.build(); + const oldToNewMappings = new Map(); + oldToNewMappings.set('oldId', 'newId'); + + const user = userDoFactory.build(); + user.sourceOptions = new UserSourceOptions({ tspUid: 'oldId' }); + + configService.getOrThrow.mockReturnValueOnce(1); + userService.findByTspUids.mockResolvedValueOnce([user]); + accountService.findMultipleByUserId.mockResolvedValueOnce(accountDoFactory.buildList(1)); + + return { system, oldToNewMappings }; + }; + + it('should migrate the tsp users', async () => { + const { system, oldToNewMappings } = setup(); + const result = await sut.migrateTspUsers(system, oldToNewMappings); + + expect(result).toBeDefined(); + expect(result.totalAmount).toBe(1); + expect(result.totalUsers).toBe(1); + expect(result.totalAccounts).toBe(1); + }); + }); + + describe('when tsp user does not have a tspUid', () => { + const setup = () => { + const system = systemFactory.build(); + const oldToNewMappings = new Map(); + oldToNewMappings.set('oldId', 'newId'); + + const user = userDoFactory.build(); + user.sourceOptions = new UserSourceOptions({ tspUid: undefined }); + + configService.getOrThrow.mockReturnValueOnce(1); + userService.findByTspUids.mockResolvedValueOnce([user]); + accountService.findMultipleByUserId.mockResolvedValueOnce(accountDoFactory.buildList(1)); + + return { system, oldToNewMappings }; + }; + + it('should not migrate the tsp user', async () => { + const { system, oldToNewMappings } = setup(); + const result = await sut.migrateTspUsers(system, oldToNewMappings); + + expect(result.totalUsers).toBe(0); + }); + }); + + describe('when newUid does not exist for a user', () => { + const setup = () => { + const system = systemFactory.build(); + const oldToNewMappings = new Map(); + oldToNewMappings.set('oldId', 'newId'); + + const user = userDoFactory.build(); + user.sourceOptions = new UserSourceOptions({ tspUid: 'oldIdWithoutNewIdMapping' }); + + configService.getOrThrow.mockReturnValueOnce(1); + userService.findByTspUids.mockResolvedValueOnce([user]); + accountService.findMultipleByUserId.mockResolvedValueOnce(accountDoFactory.buildList(1)); + + return { system, oldToNewMappings }; + }; + + it('should not migrate the tsp user', async () => { + const { system, oldToNewMappings } = setup(); + const result = await sut.migrateTspUsers(system, oldToNewMappings); + + expect(result.totalUsers).toBe(0); + }); + }); + }); +}); diff --git a/apps/server/src/infra/sync/tsp/tsp-sync-migration.service.ts b/apps/server/src/infra/sync/tsp/tsp-sync-migration.service.ts new file mode 100644 index 00000000000..05b30a81e54 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/tsp-sync-migration.service.ts @@ -0,0 +1,140 @@ +import { Account, AccountService } from '@modules/account'; +import { System } from '@modules/system'; +import { UserService } from '@modules/user'; +import { Injectable } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { UserDO } from '@shared/domain/domainobject'; +import { UserSourceOptions } from '@shared/domain/domainobject/user-source-options.do'; +import { Logger } from '@src/core/logger'; +import { TspMigrationsFetchedLoggable } from './loggable/tsp-migrations-fetched.loggable'; +import { TspSyncConfig } from './tsp-sync.config'; +import { TspMigrationBatchSummaryLoggable } from './loggable/tsp-migration-batch-summary.loggable'; + +@Injectable() +export class TspSyncMigrationService { + constructor( + private readonly logger: Logger, + private readonly userService: UserService, + private readonly accountService: AccountService, + private readonly configService: ConfigService + ) { + this.logger.setContext(TspSyncMigrationService.name); + } + + public async migrateTspUsers( + system: System, + oldToNewMappings: Map + ): Promise<{ + totalAmount: number; + totalUsers: number; + totalAccounts: number; + }> { + const totalIdCount = oldToNewMappings.size; + this.logger.info(new TspMigrationsFetchedLoggable(totalIdCount)); + + const batches = this.getOldIdBatches(oldToNewMappings); + + let totalAmount = 0; + let totalUsers = 0; + let totalAccounts = 0; + for await (const oldIdsBatch of batches) { + const { users, accounts, accountsForUserId } = await this.loadUsersAndAccounts(oldIdsBatch); + const updated = this.updateUsersAndAccounts(system.id, oldToNewMappings, users, accountsForUserId); + await this.saveUsersAndAccounts(users, accounts); + + totalAmount += oldIdsBatch.length; + totalUsers += updated.usersUpdated; + totalAccounts += updated.accountsUpdated; + + this.logger.info( + new TspMigrationBatchSummaryLoggable( + oldIdsBatch.length, + updated.usersUpdated, + updated.accountsUpdated, + totalAmount, + totalIdCount + ) + ); + } + + return { + totalAmount, + totalUsers, + totalAccounts, + }; + } + + private updateUsersAndAccounts( + systemId: string, + oldToNewMappings: Map, + users: UserDO[], + accountsForUserId: Map + ): { usersUpdated: number; accountsUpdated: number } { + let usersUpdated = 0; + let accountsUpdated = 0; + users.forEach((user) => { + const oldId = user.sourceOptions?.tspUid; + + if (!oldId) { + return; + } + + const newUid = oldToNewMappings.get(oldId); + + if (!newUid) { + return; + } + + const newEmailAndUsername = `${newUid}@schul-cloud.org`; + + user.email = newEmailAndUsername; + user.externalId = newUid; + user.previousExternalId = oldId; + user.sourceOptions = new UserSourceOptions({ tspUid: newUid }); + usersUpdated += 1; + + const account = accountsForUserId.get(user.id ?? ''); + if (account) { + account.username = newEmailAndUsername; + account.systemId = systemId; + accountsUpdated += 1; + } + }); + + return { usersUpdated, accountsUpdated }; + } + + private getOldIdBatches(oldToNewMappings: Map): string[][] { + const oldIds = Array.from(oldToNewMappings.keys()); + const batchSize = this.configService.getOrThrow('TSP_SYNC_MIGRATION_LIMIT'); + + const batchCount = Math.ceil(oldIds.length / batchSize); + const batches: string[][] = []; + for (let i = 0; i < batchCount; i += 1) { + const start = i * batchSize; + const end = Math.min((i + 1) * batchSize, oldIds.length); + batches.push(oldIds.slice(start, end)); + } + + return batches; + } + + private async loadUsersAndAccounts( + tspUids: string[] + ): Promise<{ users: UserDO[]; accounts: Account[]; accountsForUserId: Map }> { + const users = await this.userService.findByTspUids(tspUids); + + const userIds = users.map((user) => user.id ?? ''); + const accounts = await this.accountService.findMultipleByUserId(userIds); + + const accountsForUserId = new Map(); + accounts.forEach((account) => accountsForUserId.set(account.userId ?? '', account)); + + return { users, accounts, accountsForUserId }; + } + + private async saveUsersAndAccounts(users: UserDO[], accounts: Account[]): Promise { + await this.userService.saveAll(users); + await this.accountService.saveAll(accounts); + } +} diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts b/apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts index b86d44430f7..106565cdf7b 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts @@ -1,13 +1,10 @@ import { faker } from '@faker-js/faker'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { AccountService } from '@modules/account'; import { School, SchoolService } from '@modules/school'; import { SystemService, SystemType } from '@modules/system'; -import { UserService } from '@modules/user'; import { Test, TestingModule } from '@nestjs/testing'; import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; -import { federalStateFactory, schoolYearFactory, userDoFactory } from '@shared/testing'; -import { accountDoFactory } from '@src/modules/account/testing'; +import { federalStateFactory, schoolYearFactory } from '@shared/testing'; import { FederalStateService, SchoolYearService } from '@src/modules/legacy-school'; import { FileStorageType, SchoolProps } from '@src/modules/school/domain'; import { FederalStateEntityMapper, SchoolYearEntityMapper } from '@src/modules/school/repo/mikro-orm/mapper'; @@ -22,8 +19,6 @@ describe(TspSyncService.name, () => { let schoolService: DeepMocked; let federalStateService: DeepMocked; let schoolYearService: DeepMocked; - let userService: DeepMocked; - let accountService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -45,14 +40,6 @@ describe(TspSyncService.name, () => { provide: SchoolYearService, useValue: createMock(), }, - { - provide: UserService, - useValue: createMock(), - }, - { - provide: AccountService, - useValue: createMock(), - }, ], }).compile(); @@ -61,8 +48,6 @@ describe(TspSyncService.name, () => { schoolService = module.get(SchoolService); federalStateService = module.get(FederalStateService); schoolYearService = module.get(SchoolYearService); - userService = module.get(UserService); - accountService = module.get(AccountService); }); afterEach(() => { @@ -299,137 +284,4 @@ describe(TspSyncService.name, () => { }); }); }); - - describe('findUserByTspUid', () => { - describe('when user is found', () => { - const setup = () => { - const tspUid = faker.string.alpha(); - const user = userDoFactory.build(); - - userService.findUsers.mockResolvedValueOnce({ data: [user], total: 1 }); - - return { tspUid, user }; - }; - - it('should return the user', async () => { - const { tspUid, user } = setup(); - - const result = await sut.findUserByTspUid(tspUid); - - expect(result).toBe(user); - }); - }); - - describe('when user is not found', () => { - const setup = () => { - const tspUid = faker.string.alpha(); - - userService.findUsers.mockResolvedValueOnce({ data: [], total: 0 }); - - return { tspUid }; - }; - - it('should return null', async () => { - const { tspUid } = setup(); - - const result = await sut.findUserByTspUid(tspUid); - - expect(result).toBeNull(); - }); - }); - }); - - describe('findAccountByExternalId', () => { - describe('when account is found', () => { - const setup = () => { - const externalId = faker.string.alpha(); - const systemId = faker.string.alpha(); - - const user = userDoFactory.build(); - const account = accountDoFactory.build(); - - user.id = faker.string.alpha(); - user.externalId = externalId; - account.userId = user.id; - - userService.findByExternalId.mockResolvedValueOnce(user); - accountService.findByUserId.mockResolvedValueOnce(account); - - return { externalId, systemId, account }; - }; - - it('should return the account', async () => { - const { externalId, systemId, account } = setup(); - - const result = await sut.findAccountByExternalId(externalId, systemId); - - expect(result).toBe(account); - }); - }); - - describe('when account is not found', () => { - const setup = () => { - const externalId = faker.string.alpha(); - const systemId = faker.string.alpha(); - - userService.findByExternalId.mockResolvedValueOnce(null); - accountService.findByUserId.mockResolvedValueOnce(null); - - return { externalId, systemId }; - }; - - it('should return null', async () => { - const { externalId, systemId } = setup(); - - const result = await sut.findAccountByExternalId(externalId, systemId); - - expect(result).toBeNull(); - }); - }); - }); - - describe('updateUser', () => { - describe('when user is updated', () => { - const setup = () => { - const oldUid = faker.string.alpha(); - const newUid = faker.string.alpha(); - const email = faker.internet.email(); - const user = userDoFactory.build(); - - userService.save.mockResolvedValueOnce(user); - - return { oldUid, newUid, email, user }; - }; - - it('should return the updated user', async () => { - const { oldUid, newUid, email, user } = setup(); - - const result = await sut.updateUser(user, email, newUid, oldUid); - - expect(result).toBe(user); - }); - }); - }); - - describe('updateAccount', () => { - describe('when account is updated', () => { - const setup = () => { - const username = faker.internet.userName(); - const systemId = faker.string.alpha(); - const account = accountDoFactory.build(); - - accountService.save.mockResolvedValueOnce(account); - - return { username, systemId, account }; - }; - - it('should return the updated account', async () => { - const { username, systemId, account } = setup(); - - const result = await sut.updateAccount(account, username, systemId); - - expect(result).toBe(account); - }); - }); - }); }); diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts index c79e1165343..51e4b6e78ae 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -2,17 +2,13 @@ import { FederalStateService, SchoolYearService } from '@modules/legacy-school'; import { School, SchoolService } from '@modules/school'; import { System, SystemService, SystemType } from '@modules/system'; import { Injectable } from '@nestjs/common'; -import { UserDO } from '@shared/domain/domainobject'; -import { UserSourceOptions } from '@shared/domain/domainobject/user-source-options.do'; import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; -import { EntityId, SchoolFeature } from '@shared/domain/types'; -import { Account, AccountService } from '@src/modules/account'; +import { SchoolFeature } from '@shared/domain/types'; import { FederalStateNames } from '@src/modules/legacy-school/types'; import { FederalState, FileStorageType } from '@src/modules/school/domain'; import { SchoolFactory } from '@src/modules/school/domain/factory'; import { SchoolPermissions } from '@src/modules/school/domain/type'; import { FederalStateEntityMapper, SchoolYearEntityMapper } from '@src/modules/school/repo/mikro-orm/mapper'; -import { UserService } from '@src/modules/user'; import { ObjectId } from 'bson'; import { TspSystemNotFoundLoggableException } from './loggable/tsp-system-not-found.loggable-exception'; @@ -24,9 +20,7 @@ export class TspSyncService { private readonly systemService: SystemService, private readonly schoolService: SchoolService, private readonly federalStateService: FederalStateService, - private readonly schoolYearService: SchoolYearService, - private readonly userService: UserService, - private readonly accountService: AccountService + private readonly schoolYearService: SchoolYearService ) {} public async findTspSystemOrFail(): Promise { @@ -114,47 +108,4 @@ export class TspSyncService { this.federalState = FederalStateEntityMapper.mapToDo(federalStateEntity); return this.federalState; } - - public async findUserByTspUid(tspUid: string): Promise { - const tspUser = await this.userService.findUsers({ tspUid }); - - if (tspUser.data.length === 0) { - return null; - } - - return tspUser.data[0]; - } - - public async findAccountByExternalId(externalId: string, systemId: EntityId): Promise { - const user = await this.userService.findByExternalId(externalId, systemId); - - if (!user || !user.id) { - return null; - } - - const account = await this.accountService.findByUserId(user.id); - - return account; - } - - public async updateUser( - user: UserDO, - email: string, - externalId: string, - previousExternalId: string - ): Promise { - user.email = email; - user.externalId = externalId; - user.previousExternalId = previousExternalId; - user.sourceOptions = new UserSourceOptions({ tspUid: user.externalId }); - - return this.userService.save(user); - } - - public async updateAccount(account: Account, username: string, systemId: string): Promise { - account.username = username; - account.systemId = systemId; - - return this.accountService.save(account); - } } diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.spec.ts b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.spec.ts index b4303779eef..d1a9942cc98 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.spec.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.spec.ts @@ -12,10 +12,8 @@ import { ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; import { UserDO } from '@shared/domain/domainobject'; import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; -import { userDoFactory } from '@shared/testing'; import { Logger } from '@src/core/logger'; import { Account } from '@src/modules/account'; -import { accountDoFactory } from '@src/modules/account/testing'; import { ExternalUserDto, OauthDataDto, ProvisioningService, ProvisioningSystemDto } from '@src/modules/provisioning'; import { School } from '@src/modules/school'; import { schoolFactory } from '@src/modules/school/testing'; @@ -28,6 +26,7 @@ import { TspOauthDataMapper } from './tsp-oauth-data.mapper'; import { TspSyncConfig } from './tsp-sync.config'; import { TspSyncService } from './tsp-sync.service'; import { TspSyncStrategy } from './tsp-sync.strategy'; +import { TspSyncMigrationService } from './tsp-sync-migration.service'; describe(TspSyncStrategy.name, () => { let module: TestingModule; @@ -37,6 +36,8 @@ describe(TspSyncStrategy.name, () => { let provisioningService: DeepMocked; let tspOauthDataMapper: DeepMocked; let tspLegacyMigrationService: DeepMocked; + let tspSyncMigrationService: DeepMocked; + let configService: DeepMocked>; beforeAll(async () => { module = await Test.createTestingModule({ @@ -56,26 +57,7 @@ describe(TspSyncStrategy.name, () => { }, { provide: ConfigService, - useValue: createMock>({ - getOrThrow: (key: string) => { - switch (key) { - case 'TSP_SYNC_SCHOOL_LIMIT': - return 10; - case 'TSP_SYNC_SCHOOL_DAYS_TO_FETCH': - return 1; - case 'TSP_SYNC_DATA_LIMIT': - return 10; - case 'TSP_SYNC_DATA_DAYS_TO_FETCH': - return 1; - case 'TSP_SYNC_MIGRATION_LIMIT': - return 10; - case 'FEATURE_TSP_MIGRATION_ENABLED': - return true; - default: - throw new Error(`Unknown key: ${key}`); - } - }, - }), + useValue: createMock>(), }, { provide: ProvisioningService, @@ -89,6 +71,10 @@ describe(TspSyncStrategy.name, () => { provide: TspLegacyMigrationService, useValue: createMock(), }, + { + provide: TspSyncMigrationService, + useValue: createMock(), + }, ], }).compile(); @@ -98,11 +84,14 @@ describe(TspSyncStrategy.name, () => { provisioningService = module.get(ProvisioningService); tspOauthDataMapper = module.get(TspOauthDataMapper); tspLegacyMigrationService = module.get(TspLegacyMigrationService); + tspSyncMigrationService = module.get(TspSyncMigrationService); + configService = module.get(ConfigService); }); afterEach(() => { jest.clearAllMocks(); jest.resetAllMocks(); + jest.restoreAllMocks(); }); afterAll(async () => { @@ -138,6 +127,12 @@ describe(TspSyncStrategy.name, () => { foundSystem?: System; updatedAccount?: Account; updatedUser?: UserDO; + configValues?: unknown[]; + migrationResult?: { + totalAmount: number; + totalUsers: number; + totalAccounts: number; + }; }) => { tspFetchService.fetchTspSchools.mockResolvedValueOnce(params.fetchedSchools ?? []); tspFetchService.fetchTspClasses.mockResolvedValueOnce(params.fetchedClasses ?? []); @@ -148,17 +143,19 @@ describe(TspSyncStrategy.name, () => { tspSyncService.findSchool.mockResolvedValue(params.foundSchool ?? undefined); tspSyncService.findSchoolsForSystem.mockResolvedValueOnce(params.foundSystemSchools ?? []); - tspSyncService.findUserByTspUid.mockResolvedValueOnce( - params.foundTspUidUser !== undefined ? params.foundTspUidUser : userDoFactory.build() - ); - tspSyncService.updateUser.mockResolvedValueOnce(params.updatedUser ?? userDoFactory.build()); - tspSyncService.findAccountByExternalId.mockResolvedValueOnce( - params.foundTspUidAccount !== undefined ? params.foundTspUidAccount : accountDoFactory.build() - ); - tspSyncService.updateAccount.mockResolvedValueOnce(params.updatedAccount ?? accountDoFactory.build()); tspSyncService.findTspSystemOrFail.mockResolvedValueOnce(params.foundSystem ?? systemFactory.build()); tspOauthDataMapper.mapTspDataToOauthData.mockReturnValueOnce(params.mappedOauthDto ?? []); + + params.configValues?.forEach((value) => configService.getOrThrow.mockReturnValueOnce(value)); + + tspSyncMigrationService.migrateTspUsers.mockResolvedValueOnce( + params.migrationResult ?? { + totalAccounts: faker.number.int(), + totalAmount: faker.number.int(), + totalUsers: faker.number.int(), + } + ); }; describe('sync', () => { @@ -188,12 +185,15 @@ describe(TspSyncStrategy.name, () => { fetchedStudentMigrations: [tspStudent], fetchedTeacherMigrations: [tspTeacher], mappedOauthDto: [oauthDataDto], + configValues: [1, 10, true, 10, 1, 50], }); return { oauthDataDto }; }; it('should find the tsp system', async () => { + setup(); + await sut.sync(); expect(tspSyncService.findTspSystemOrFail).toHaveBeenCalled(); @@ -266,36 +266,12 @@ describe(TspSyncStrategy.name, () => { expect(tspFetchService.fetchTspStudentMigrations).toHaveBeenCalled(); }); - it('find user by tsp Uid', async () => { - setup(); - - await sut.sync(); - - expect(tspSyncService.findUserByTspUid).toHaveBeenCalled(); - }); - - it('should update user', async () => { - setup(); - - await sut.sync(); - - expect(tspSyncService.updateUser).toHaveBeenCalled(); - }); - - it('should find account by tsp Uid', async () => { + it('should call tspSyncMigrationService', async () => { setup(); await sut.sync(); - expect(tspSyncService.findAccountByExternalId).toHaveBeenCalled(); - }); - - it('should update account', async () => { - setup(); - - await sut.sync(); - - expect(tspSyncService.updateAccount).toHaveBeenCalled(); + expect(tspSyncMigrationService.migrateTspUsers).toHaveBeenCalled(); }); }); }); @@ -310,6 +286,7 @@ describe(TspSyncStrategy.name, () => { setupMockServices({ fetchedSchools: tspSchools, + configValues: [1, 10, true, 10, 1, 50], }); }; @@ -334,6 +311,7 @@ describe(TspSyncStrategy.name, () => { setupMockServices({ fetchedSchools: tspSchools, foundSchool: school, + configValues: [1, 10, true, 10, 1, 50], }); }; @@ -356,6 +334,7 @@ describe(TspSyncStrategy.name, () => { setupMockServices({ fetchedSchools: tspSchools, + configValues: [1, 10, true, 10, 1, 50], }); }; @@ -369,77 +348,5 @@ describe(TspSyncStrategy.name, () => { expect(tspSyncService.createSchool).not.toHaveBeenCalled(); }); }); - - describe('when UidAlt or UidNeu is missing during migration', () => { - const setup = () => { - const tspTeacher: RobjExportLehrerMigration = { - lehrerUidAlt: undefined, - lehrerUidNeu: faker.string.alpha(), - }; - const tspStudent: RobjExportSchuelerMigration = { - schuelerUidAlt: faker.string.alpha(), - schuelerUidNeu: undefined, - }; - - setupMockServices({ - fetchedStudentMigrations: [tspStudent], - fetchedTeacherMigrations: [tspTeacher], - }); - }; - - it('should return false and not call findUserByTspUid', async () => { - setup(); - - await sut.sync(); - - expect(tspSyncService.findUserByTspUid).not.toHaveBeenCalled(); - }); - }); - - describe('when no user is found during migration', () => { - const setup = () => { - const tspTeacher: RobjExportLehrerMigration = { - lehrerUidAlt: faker.string.alpha(), - lehrerUidNeu: faker.string.alpha(), - }; - - setupMockServices({ - fetchedTeacherMigrations: [tspTeacher], - foundTspUidUser: null, - }); - - return { tspTeacher }; - }; - - it('should throw and not call updateUser', async () => { - setup(); - - await sut.sync(); - - expect(tspSyncService.updateUser).not.toHaveBeenCalled(); - }); - }); - - describe('when no account is found during migration', () => { - const setup = () => { - const tspTeacher: RobjExportLehrerMigration = { - lehrerUidAlt: faker.string.alpha(), - lehrerUidNeu: faker.string.alpha(), - }; - - setupMockServices({ - fetchedTeacherMigrations: [tspTeacher], - foundTspUidAccount: null, - }); - }; - - it('should throw and not call updateAccount', async () => { - setup(); - - await sut.sync(); - - expect(tspSyncService.updateAccount).not.toHaveBeenCalled(); - }); - }); }); }); diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts index 07469f5abd6..8ed097e6b27 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts @@ -1,10 +1,7 @@ import { School } from '@modules/school'; import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; -import { NotFoundLoggableException } from '@shared/common/loggable-exception'; -import { UserDO } from '@shared/domain/domainobject'; import { Logger } from '@src/core/logger'; -import { Account } from '@src/modules/account'; import { ProvisioningService } from '@src/modules/provisioning'; import { System } from '@src/modules/system'; import pLimit from 'p-limit'; @@ -14,53 +11,30 @@ import { TspDataFetchedLoggable } from './loggable/tsp-data-fetched.loggable'; import { TspSchoolsFetchedLoggable } from './loggable/tsp-schools-fetched.loggable'; import { TspSchoolsSyncedLoggable } from './loggable/tsp-schools-synced.loggable'; import { TspSchulnummerMissingLoggable } from './loggable/tsp-schulnummer-missing.loggable'; -import { TspStudentsFetchedLoggable } from './loggable/tsp-students-fetched.loggable'; -import { TspStudentsMigratedLoggable } from './loggable/tsp-students-migrated.loggable'; import { TspSyncedUsersLoggable } from './loggable/tsp-synced-users.loggable'; import { TspSyncingUsersLoggable } from './loggable/tsp-syncing-users.loggable'; -import { TspTeachersFetchedLoggable } from './loggable/tsp-teachers-fetched.loggable'; -import { TspTeachersMigratedLoggable } from './loggable/tsp-teachers-migrated.loggable'; import { TspUsersMigratedLoggable } from './loggable/tsp-users-migrated.loggable'; +import { TspFetchService } from './tsp-fetch.service'; +import { TspLegacyMigrationService } from './tsp-legacy-migration.service'; import { TspOauthDataMapper } from './tsp-oauth-data.mapper'; +import { TspSyncMigrationService } from './tsp-sync-migration.service'; import { TspSyncConfig } from './tsp-sync.config'; import { TspSyncService } from './tsp-sync.service'; -import { TspLegacyMigrationService } from './tsp-legacy-migration.service'; -import { TspFetchService } from './tsp-fetch.service'; @Injectable() export class TspSyncStrategy extends SyncStrategy { - private readonly schoolLimit: pLimit.Limit; - - private readonly dataLimit: pLimit.Limit; - - private readonly migrationLimit: pLimit.Limit; - - private readonly schoolDaysToFetch: number; - - private readonly schoolDataDaysToFetch: number; - - private readonly migrationEnabled: boolean; - constructor( private readonly logger: Logger, private readonly tspSyncService: TspSyncService, private readonly tspFetchService: TspFetchService, private readonly tspOauthDataMapper: TspOauthDataMapper, private readonly tspLegacyMigrationService: TspLegacyMigrationService, - configService: ConfigService, - private readonly provisioningService: ProvisioningService + private readonly configService: ConfigService, + private readonly provisioningService: ProvisioningService, + private readonly tspSyncMigrationService: TspSyncMigrationService ) { super(); this.logger.setContext(TspSyncStrategy.name); - - this.schoolLimit = pLimit(configService.getOrThrow('TSP_SYNC_SCHOOL_LIMIT')); - this.schoolDaysToFetch = configService.get('TSP_SYNC_SCHOOL_DAYS_TO_FETCH', 1); - - this.dataLimit = pLimit(configService.getOrThrow('TSP_SYNC_DATA_LIMIT')); - this.schoolDataDaysToFetch = configService.get('TSP_SYNC_DATA_DAYS_TO_FETCH', 1); - - this.migrationLimit = pLimit(configService.getOrThrow('TSP_SYNC_MIGRATION_LIMIT')); - this.migrationEnabled = configService.get('FEATURE_TSP_MIGRATION_ENABLED', false); } public override getType(): SyncStrategyTarget { @@ -76,22 +50,23 @@ export class TspSyncStrategy extends SyncStrategy { const schools = await this.tspSyncService.findSchoolsForSystem(system); - if (this.migrationEnabled) { - const teacherMigrationResult = await this.migrateTspTeachers(system); - const studentMigrationResult = await this.migrateTspStudents(system); - const totalMigrations = teacherMigrationResult.total + studentMigrationResult.total; - this.logger.info(new TspUsersMigratedLoggable(totalMigrations)); + if (this.configService.getOrThrow('FEATURE_TSP_MIGRATION_ENABLED')) { + await this.runMigration(system); } await this.syncData(system, schools); } private async syncSchools(system: System): Promise { - const tspSchools = await this.tspFetchService.fetchTspSchools(system, this.schoolDaysToFetch); - this.logger.info(new TspSchoolsFetchedLoggable(tspSchools.length, this.schoolDaysToFetch)); + const schoolDaysToFetch = this.configService.getOrThrow('TSP_SYNC_SCHOOL_DAYS_TO_FETCH'); + const tspSchools = await this.tspFetchService.fetchTspSchools(system, schoolDaysToFetch); + this.logger.info(new TspSchoolsFetchedLoggable(tspSchools.length, schoolDaysToFetch)); + + const schoolLimit = this.configService.getOrThrow('TSP_SYNC_SCHOOL_LIMIT'); + const schoolLimitFn = pLimit(schoolLimit); const schoolPromises = tspSchools.map((tspSchool) => - this.schoolLimit(async () => { + schoolLimitFn(async () => { if (!tspSchool.schuleNummer) { this.logger.warning(new TspSchulnummerMissingLoggable(tspSchool.schuleName)); return null; @@ -125,11 +100,12 @@ export class TspSyncStrategy extends SyncStrategy { } private async syncData(system: System, schools: School[]): Promise { - const tspTeachers = await this.tspFetchService.fetchTspTeachers(system, this.schoolDataDaysToFetch); - const tspStudents = await this.tspFetchService.fetchTspStudents(system, this.schoolDataDaysToFetch); - const tspClasses = await this.tspFetchService.fetchTspClasses(system, this.schoolDataDaysToFetch); + const schoolDataDaysToFetch = this.configService.getOrThrow('TSP_SYNC_DATA_DAYS_TO_FETCH'); + const tspTeachers = await this.tspFetchService.fetchTspTeachers(system, schoolDataDaysToFetch); + const tspStudents = await this.tspFetchService.fetchTspStudents(system, schoolDataDaysToFetch); + const tspClasses = await this.tspFetchService.fetchTspClasses(system, schoolDataDaysToFetch); this.logger.info( - new TspDataFetchedLoggable(tspTeachers.length, tspStudents.length, tspClasses.length, this.schoolDataDaysToFetch) + new TspDataFetchedLoggable(tspTeachers.length, tspStudents.length, tspClasses.length, schoolDataDaysToFetch) ); const oauthDataDtos = this.tspOauthDataMapper.mapTspDataToOauthData( @@ -142,8 +118,11 @@ export class TspSyncStrategy extends SyncStrategy { this.logger.info(new TspSyncingUsersLoggable(oauthDataDtos.length)); + const dataLimit = this.configService.getOrThrow('TSP_SYNC_DATA_LIMIT'); + const dataLimitFn = pLimit(dataLimit); + const dataPromises = oauthDataDtos.map((oauthDataDto) => - this.dataLimit(() => this.provisioningService.provisionData(oauthDataDto)) + dataLimitFn(() => this.provisioningService.provisionData(oauthDataDto)) ); const results = await Promise.allSettled(dataPromises); @@ -151,74 +130,29 @@ export class TspSyncStrategy extends SyncStrategy { this.logger.info(new TspSyncedUsersLoggable(results.length)); } - private async migrateTspTeachers(system: System): Promise<{ total: number }> { - const tspTeacherIds = await this.tspFetchService.fetchTspTeacherMigrations(system); - this.logger.info(new TspTeachersFetchedLoggable(tspTeacherIds.length)); - - const teacherMigrationPromises = tspTeacherIds.map(({ lehrerUidAlt, lehrerUidNeu }) => - this.migrationLimit(async () => { - if (lehrerUidAlt && lehrerUidNeu) { - await this.migrateTspUser(lehrerUidAlt, lehrerUidNeu, system.id); - return true; - } - return false; - }) - ); - - const migratedTspTeachers = await Promise.allSettled(teacherMigrationPromises); - - const total = migratedTspTeachers.filter((result) => result.status === 'fulfilled' && result.value === true).length; - this.logger.info(new TspTeachersMigratedLoggable(total)); - - return { total }; - } - - private async migrateTspStudents(system: System): Promise<{ total: number }> { - const tspStudentIds = await this.tspFetchService.fetchTspStudentMigrations(system); - this.logger.info(new TspStudentsFetchedLoggable(tspStudentIds.length)); - - const studentMigrationPromises = tspStudentIds.map(({ schuelerUidAlt, schuelerUidNeu }) => - this.migrationLimit(async () => { - if (schuelerUidAlt && schuelerUidNeu) { - await this.migrateTspUser(schuelerUidAlt, schuelerUidNeu, system.id); - return true; - } - return false; - }) + private async runMigration(system: System): Promise { + const oldToNewMappings = new Map(); + const teacherMigrations = await this.tspFetchService.fetchTspTeacherMigrations(system); + teacherMigrations.forEach(({ lehrerUidAlt, lehrerUidNeu }) => { + if (lehrerUidAlt && lehrerUidNeu) { + oldToNewMappings.set(lehrerUidAlt, lehrerUidNeu); + } + }); + + const studentsMigrations = await this.tspFetchService.fetchTspStudentMigrations(system); + studentsMigrations.forEach(({ schuelerUidAlt, schuelerUidNeu }) => { + if (schuelerUidAlt && schuelerUidNeu) { + oldToNewMappings.set(schuelerUidAlt, schuelerUidNeu); + } + }); + + const migrationResult = await this.tspSyncMigrationService.migrateTspUsers(system, oldToNewMappings); + this.logger.info( + new TspUsersMigratedLoggable( + migrationResult.totalAmount, + migrationResult.totalUsers, + migrationResult.totalAccounts + ) ); - - const migratedStudents = await Promise.allSettled(studentMigrationPromises); - - const total = migratedStudents.filter((result) => result.status === 'fulfilled' && result.value === true).length; - this.logger.info(new TspStudentsMigratedLoggable(total)); - - return { total }; - } - - private async migrateTspUser( - oldUid: string, - newUid: string, - systemId: string - ): Promise<{ updatedUser: UserDO; updatedAccount: Account }> { - const newEmailAndUsername = `${newUid}@schul-cloud.org`; - const user = await this.tspSyncService.findUserByTspUid(oldUid); - - if (!user) { - throw new NotFoundLoggableException(UserDO.name, { oldUid }); - } - - const newEmail = newEmailAndUsername; - const updatedUser = await this.tspSyncService.updateUser(user, newEmail, newUid, oldUid); - - const account = await this.tspSyncService.findAccountByExternalId(newUid, systemId); - - if (!account) { - throw new NotFoundLoggableException(Account.name, { oldUid }); - } - - const newUsername = newEmailAndUsername; - const updatedAccount = await this.tspSyncService.updateAccount(account, newUsername, systemId); - - return { updatedUser, updatedAccount }; } } diff --git a/apps/server/src/modules/account/domain/services/account-db.service.spec.ts b/apps/server/src/modules/account/domain/services/account-db.service.spec.ts index 4b5911267a8..9334a0ca937 100644 --- a/apps/server/src/modules/account/domain/services/account-db.service.spec.ts +++ b/apps/server/src/modules/account/domain/services/account-db.service.spec.ts @@ -323,7 +323,7 @@ describe('AccountDbService', () => { if (mockTeacherUser.id === userId) { return Promise.resolve(mockTeacherAccount); } - throw new EntityNotFoundError(AccountEntity.name); + return Promise.reject(new EntityNotFoundError(AccountEntity.name)); }); return {}; }; @@ -691,6 +691,57 @@ describe('AccountDbService', () => { }); }); + describe('saveAll', () => { + describe('when given account that does not exist', () => { + const setup = () => { + const account = accountDoFactory.build({ + id: undefined, + }); + const savedAccount = accountDoFactory.build({ + ...account, + id: new ObjectId().toHexString(), + }); + + accountRepo.saveAll.mockResolvedValueOnce([savedAccount]); + + return { account, savedAccount }; + }; + + it('should save it', async () => { + const { account, savedAccount } = setup(); + + const result = await accountService.saveAll([account]); + + expect(result.length).toBe(1); + expect(result[0]).toStrictEqual(savedAccount); + expect(accountRepo.saveAll).toHaveBeenCalledTimes(1); + }); + }); + + describe('when given account that exist', () => { + const setup = () => { + const account = accountDoFactory.build(); + const foundAccount = accountDoFactory.build(); + const updateSpy = jest.spyOn(foundAccount, 'update'); + + accountRepo.findById.mockResolvedValueOnce(foundAccount); + accountRepo.saveAll.mockResolvedValueOnce([foundAccount]); + + return { account, foundAccount, updateSpy }; + }; + + it('should update it', async () => { + const { account, foundAccount, updateSpy } = setup(); + + const result = await accountService.saveAll([account]); + + expect(updateSpy).toHaveBeenCalledTimes(1); + expect(result.length).toBe(1); + expect(result[0].id).toBe(foundAccount.id); + }); + }); + }); + describe('updateUsername', () => { describe('when updating username', () => { const setup = () => { diff --git a/apps/server/src/modules/account/domain/services/account-db.service.ts b/apps/server/src/modules/account/domain/services/account-db.service.ts index 36df25fb82b..23fb2d6cdca 100644 --- a/apps/server/src/modules/account/domain/services/account-db.service.ts +++ b/apps/server/src/modules/account/domain/services/account-db.service.ts @@ -21,29 +21,29 @@ export class AccountServiceDb extends AbstractAccountService { super(); } - async findById(id: EntityId): Promise { + public async findById(id: EntityId): Promise { const internalId = await this.getInternalId(id); return this.accountRepo.findById(internalId); } - async findMultipleByUserId(userIds: EntityId[]): Promise { + public findMultipleByUserId(userIds: EntityId[]): Promise { return this.accountRepo.findMultipleByUserId(userIds); } - async findByUserId(userId: EntityId): Promise { + public findByUserId(userId: EntityId): Promise { return this.accountRepo.findByUserId(userId); } - async findByUserIdOrFail(userId: EntityId): Promise { + public findByUserIdOrFail(userId: EntityId): Promise { return this.accountRepo.findByUserIdOrFail(userId); } - async findByUsernameAndSystemId(username: string, systemId: EntityId | ObjectId): Promise { + public findByUsernameAndSystemId(username: string, systemId: EntityId | ObjectId): Promise { return this.accountRepo.findByUsernameAndSystemId(username, systemId); } - async save(accountSave: AccountSave): Promise { + public async save(accountSave: AccountSave): Promise { let account: Account; if (accountSave.id) { const internalId = await this.getInternalId(accountSave.id); @@ -56,7 +56,28 @@ export class AccountServiceDb extends AbstractAccountService { return this.accountRepo.save(account); } - async updateUsername(accountId: EntityId, username: string): Promise { + public async saveAll(accountSaves: AccountSave[]): Promise { + const updatedAccounts = await Promise.all( + accountSaves.map(async (accountSave) => { + let account: Account; + if (accountSave.id) { + const internalId = await this.getInternalId(accountSave.id); + + account = await this.accountRepo.findById(internalId); + } else { + account = this.createAccount(accountSave); + } + await account.update(accountSave); + return account; + }) + ); + + const savedAccounts = this.accountRepo.saveAll(updatedAccounts); + + return savedAccounts; + } + + public async updateUsername(accountId: EntityId, username: string): Promise { const internalId = await this.getInternalId(accountId); const account = await this.accountRepo.findById(internalId); account.username = username; @@ -64,7 +85,7 @@ export class AccountServiceDb extends AbstractAccountService { return account; } - async updateLastLogin(accountId: EntityId, lastLogin: Date): Promise { + public async updateLastLogin(accountId: EntityId, lastLogin: Date): Promise { const internalId = await this.getInternalId(accountId); const account = await this.accountRepo.findById(internalId); account.lastLogin = lastLogin; @@ -72,7 +93,7 @@ export class AccountServiceDb extends AbstractAccountService { return account; } - async updateLastTriedFailedLogin(accountId: EntityId, lastTriedFailedLogin: Date): Promise { + public async updateLastTriedFailedLogin(accountId: EntityId, lastTriedFailedLogin: Date): Promise { const internalId = await this.getInternalId(accountId); const account = await this.accountRepo.findById(internalId); account.lasttriedFailedLogin = lastTriedFailedLogin; @@ -80,7 +101,7 @@ export class AccountServiceDb extends AbstractAccountService { return account; } - async updatePassword(accountId: EntityId, password: string): Promise { + public async updatePassword(accountId: EntityId, password: string): Promise { const internalId = await this.getInternalId(accountId); const account = await this.accountRepo.findById(internalId); account.password = await this.encryptPassword(password); @@ -89,24 +110,24 @@ export class AccountServiceDb extends AbstractAccountService { return account; } - async delete(id: EntityId): Promise { + public async delete(id: EntityId): Promise { const internalId = await this.getInternalId(id); return this.accountRepo.deleteById(internalId); } - async deleteByUserId(userId: EntityId): Promise { + public deleteByUserId(userId: EntityId): Promise { return this.accountRepo.deleteByUserId(userId); } - async searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise> { + public searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise> { return this.accountRepo.searchByUsernamePartialMatch(userName, skip, limit); } - async searchByUsernameExactMatch(userName: string): Promise> { + public searchByUsernameExactMatch(userName: string): Promise> { return this.accountRepo.searchByUsernameExactMatch(userName); } - validatePassword(account: Account, comparePassword: string): Promise { + public validatePassword(account: Account, comparePassword: string): Promise { if (!account.password) { return Promise.resolve(false); } @@ -137,7 +158,7 @@ export class AccountServiceDb extends AbstractAccountService { return bcrypt.hash(password, 10); } - async findMany(offset = 0, limit = 100): Promise { + public findMany(offset = 0, limit = 100): Promise { return this.accountRepo.findMany(offset, limit); } diff --git a/apps/server/src/modules/account/domain/services/account-idm.service.spec.ts b/apps/server/src/modules/account/domain/services/account-idm.service.spec.ts index 492b3269bf5..8baa91c6808 100644 --- a/apps/server/src/modules/account/domain/services/account-idm.service.spec.ts +++ b/apps/server/src/modules/account/domain/services/account-idm.service.spec.ts @@ -218,6 +218,37 @@ describe('AccountIdmService', () => { }); }); + describe('saveAll', () => { + describe('when saving multiple accounts', () => { + const setup = () => { + const mockAccountSaves = [ + { + id: mockIdmAccountRefId, + username: 'testUserName1', + userId: 'userId1', + systemId: 'systemId1', + } as AccountSave, + { + id: undefined, + username: 'testUserName2', + userId: 'userId2', + systemId: 'systemId2', + } as AccountSave, + ]; + + return { mockAccountSaves }; + }; + + it('should save all accounts', async () => { + const { mockAccountSaves } = setup(); + + const ret = await accountIdmService.saveAll(mockAccountSaves); + + expect(ret).toHaveLength(mockAccountSaves.length); + }); + }); + }); + describe('updateUsername', () => { describe('when update Username', () => { const setup = () => { diff --git a/apps/server/src/modules/account/domain/services/account-idm.service.ts b/apps/server/src/modules/account/domain/services/account-idm.service.ts index c20b1ec7b14..3fa1c84e2d2 100644 --- a/apps/server/src/modules/account/domain/services/account-idm.service.ts +++ b/apps/server/src/modules/account/domain/services/account-idm.service.ts @@ -24,13 +24,13 @@ export class AccountServiceIdm extends AbstractAccountService { super(); } - async findById(id: EntityId): Promise { + public async findById(id: EntityId): Promise { const result = await this.identityManager.findAccountById(id); const account = this.accountIdmToDoMapper.mapToDo(result); return account; } - async findMultipleByUserId(userIds: EntityId[]): Promise { + public async findMultipleByUserId(userIds: EntityId[]): Promise { const resultAccounts = new Array(); const promises = userIds.map((userId) => this.identityManager.findAccountByDbcUserId(userId).catch(() => null)); @@ -48,7 +48,7 @@ export class AccountServiceIdm extends AbstractAccountService { return resultAccounts; } - async findByUserId(userId: EntityId): Promise { + public async findByUserId(userId: EntityId): Promise { try { const result = await this.identityManager.findAccountByDbcUserId(userId); return this.accountIdmToDoMapper.mapToDo(result); @@ -58,7 +58,7 @@ export class AccountServiceIdm extends AbstractAccountService { } } - async findByUserIdOrFail(userId: EntityId): Promise { + public async findByUserIdOrFail(userId: EntityId): Promise { try { const result = await this.identityManager.findAccountByDbcUserId(userId); return this.accountIdmToDoMapper.mapToDo(result); @@ -67,25 +67,29 @@ export class AccountServiceIdm extends AbstractAccountService { } } - async findByUsernameAndSystemId(username: string, systemId: EntityId | ObjectId): Promise { + public async findByUsernameAndSystemId(username: string, systemId: EntityId | ObjectId): Promise { const [accounts] = await this.searchByUsernameExactMatch(username); const account = accounts.find((tempAccount) => tempAccount.systemId === systemId) || null; return account; } - async searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise> { + public async searchByUsernamePartialMatch( + userName: string, + skip: number, + limit: number + ): Promise> { const [results, total] = await this.identityManager.findAccountsByUsername(userName, { skip, limit, exact: false }); const accounts = results.map((result) => this.accountIdmToDoMapper.mapToDo(result)); return [accounts, total]; } - async searchByUsernameExactMatch(userName: string): Promise> { + public async searchByUsernameExactMatch(userName: string): Promise> { const [results, total] = await this.identityManager.findAccountsByUsername(userName, { exact: true }); const accounts = results.map((result) => this.accountIdmToDoMapper.mapToDo(result)); return [accounts, total]; } - async updateLastTriedFailedLogin(accountId: EntityId, lastTriedFailedLogin: Date): Promise { + public async updateLastTriedFailedLogin(accountId: EntityId, lastTriedFailedLogin: Date): Promise { const attributeName = 'lastTriedFailedLogin'; const id = await this.getIdmAccountId(accountId); await this.identityManager.setUserAttribute(id, attributeName, lastTriedFailedLogin.toISOString()); @@ -93,7 +97,7 @@ export class AccountServiceIdm extends AbstractAccountService { return this.accountIdmToDoMapper.mapToDo(updatedAccount); } - async save(accountSave: AccountSave): Promise { + public async save(accountSave: AccountSave): Promise { let accountId: string; const idmAccount: IdmAccountUpdate = { username: accountSave.username, @@ -117,6 +121,13 @@ export class AccountServiceIdm extends AbstractAccountService { return this.accountIdmToDoMapper.mapToDo(updatedAccount); } + public saveAll(accountSaves: AccountSave[]): Promise { + const savePromises = accountSaves.map((accountSave) => this.save(accountSave)); + const savedAccounts = Promise.all(savePromises); + + return savedAccounts; + } + private async updateAccount(idmAccountId: string, idmAccount: IdmAccountUpdate, password?: string): Promise { const updatedAccountId = await this.identityManager.updateAccount(idmAccountId, idmAccount); if (password) { @@ -130,31 +141,31 @@ export class AccountServiceIdm extends AbstractAccountService { return accountId; } - async updateUsername(accountRefId: EntityId, username: string): Promise { + public async updateUsername(accountRefId: EntityId, username: string): Promise { const id = await this.getIdmAccountId(accountRefId); await this.identityManager.updateAccount(id, { username }); const updatedAccount = await this.identityManager.findAccountById(id); return this.accountIdmToDoMapper.mapToDo(updatedAccount); } - async updatePassword(accountRefId: EntityId, password: string): Promise { + public async updatePassword(accountRefId: EntityId, password: string): Promise { const id = await this.getIdmAccountId(accountRefId); await this.identityManager.updateAccountPassword(id, password); const updatedAccount = await this.identityManager.findAccountById(id); return this.accountIdmToDoMapper.mapToDo(updatedAccount); } - async validatePassword(account: Account, comparePassword: string): Promise { + public async validatePassword(account: Account, comparePassword: string): Promise { const jwt = await this.idmOauthService.resourceOwnerPasswordGrant(account.username, comparePassword); return jwt !== undefined; } - async delete(accountRefId: EntityId): Promise { + public async delete(accountRefId: EntityId): Promise { const id = await this.getIdmAccountId(accountRefId); await this.identityManager.deleteAccountById(id); } - async deleteByUserId(userId: EntityId): Promise { + public async deleteByUserId(userId: EntityId): Promise { const idmAccount = await this.identityManager.findAccountByDbcUserId(userId); const deletedAccountId = await this.identityManager.deleteAccountById(idmAccount.id); @@ -162,8 +173,8 @@ export class AccountServiceIdm extends AbstractAccountService { } // eslint-disable-next-line @typescript-eslint/require-await, @typescript-eslint/no-unused-vars - async findMany(_offset: number, _limit: number): Promise { - throw new NotImplementedException(); + public findMany(_offset: number, _limit: number): Promise { + return Promise.reject(new NotImplementedException()); } private async getOptionalIdmAccount(accountId: string): Promise { diff --git a/apps/server/src/modules/account/domain/services/account.service.abstract.ts b/apps/server/src/modules/account/domain/services/account.service.abstract.ts index 80a48cd718b..5d75ed6c657 100644 --- a/apps/server/src/modules/account/domain/services/account.service.abstract.ts +++ b/apps/server/src/modules/account/domain/services/account.service.abstract.ts @@ -7,45 +7,51 @@ export abstract class AbstractAccountService { * The following methods are only needed by nest */ - abstract searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise>; + public abstract searchByUsernamePartialMatch( + userName: string, + skip: number, + limit: number + ): Promise>; - abstract validatePassword(account: Account, comparePassword: string): Promise; + public abstract validatePassword(account: Account, comparePassword: string): Promise; /** * @deprecated For migration purpose only */ - abstract findMany(offset?: number, limit?: number): Promise; + public abstract findMany(offset?: number, limit?: number): Promise; /* * The following methods are also needed by feathers */ - abstract findById(id: EntityId): Promise; + public abstract findById(id: EntityId): Promise; - abstract findMultipleByUserId(userIds: EntityId[]): Promise; + public abstract findMultipleByUserId(userIds: EntityId[]): Promise; - abstract findByUserId(userId: EntityId): Promise; + public abstract findByUserId(userId: EntityId): Promise; - abstract findByUserIdOrFail(userId: EntityId): Promise; + public abstract findByUserIdOrFail(userId: EntityId): Promise; // HINT: it would be preferable to use entityId here. Needs to be checked if this is blocked by lecacy code - abstract findByUsernameAndSystemId(username: string, systemId: EntityId | ObjectId): Promise; + public abstract findByUsernameAndSystemId(username: string, systemId: EntityId | ObjectId): Promise; - abstract save(accountSave: AccountSave): Promise; + public abstract save(accountSave: AccountSave): Promise; - abstract updateUsername(accountId: EntityId, username: string): Promise; + public abstract saveAll(accountSaves: AccountSave[]): Promise; + + public abstract updateUsername(accountId: EntityId, username: string): Promise; /** * @deprecated Used for brute force detection, but will become subject to IDM thus be removed. */ - abstract updateLastTriedFailedLogin(accountId: EntityId, lastTriedFailedLogin: Date): Promise; + public abstract updateLastTriedFailedLogin(accountId: EntityId, lastTriedFailedLogin: Date): Promise; - abstract updatePassword(accountId: EntityId, password: string): Promise; + public abstract updatePassword(accountId: EntityId, password: string): Promise; - abstract delete(id: EntityId): Promise; + public abstract delete(id: EntityId): Promise; - abstract deleteByUserId(userId: EntityId): Promise; + public abstract deleteByUserId(userId: EntityId): Promise; - abstract searchByUsernameExactMatch(userName: string): Promise>; + public abstract searchByUsernameExactMatch(userName: string): Promise>; - abstract isUniqueEmail(email: string): Promise; + public abstract isUniqueEmail(email: string): Promise; } diff --git a/apps/server/src/modules/account/domain/services/account.service.spec.ts b/apps/server/src/modules/account/domain/services/account.service.spec.ts index e7cc9a001e9..63d56f28f00 100644 --- a/apps/server/src/modules/account/domain/services/account.service.spec.ts +++ b/apps/server/src/modules/account/domain/services/account.service.spec.ts @@ -125,7 +125,13 @@ describe('AccountService', () => { describe('findById', () => { describe('When calling findById in accountService', () => { + const setup = () => { + accountServiceDb.findById.mockResolvedValueOnce(accountDoFactory.build()); + }; + it('should call findById in accountServiceDb', async () => { + setup(); + await expect(accountService.findById('id')).resolves.not.toThrow(); expect(accountServiceDb.findById).toHaveBeenCalledTimes(1); }); @@ -134,6 +140,8 @@ describe('AccountService', () => { describe('When identity management is primary', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.findById.mockResolvedValueOnce(accountDoFactory.build()); + return newAccountService(); }; @@ -147,7 +155,13 @@ describe('AccountService', () => { describe('findByUserId', () => { describe('When calling findByUserId in accountService', () => { + const setup = () => { + accountServiceDb.findByUserId.mockResolvedValueOnce(accountDoFactory.build()); + }; + it('should call findByUserId in accountServiceDb', async () => { + setup(); + await expect(accountService.findByUserId('userId')).resolves.not.toThrow(); expect(accountServiceDb.findByUserId).toHaveBeenCalledTimes(1); }); @@ -156,6 +170,8 @@ describe('AccountService', () => { describe('When identity management is primary', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.findByUserId.mockResolvedValueOnce(accountDoFactory.build()); + return newAccountService(); }; @@ -169,7 +185,13 @@ describe('AccountService', () => { describe('findByUsernameAndSystemId', () => { describe('When calling findByUsernameAndSystemId in accountService', () => { + const setup = () => { + accountServiceDb.findByUsernameAndSystemId.mockResolvedValueOnce(accountDoFactory.build()); + }; + it('should call findByUsernameAndSystemId in accountServiceDb', async () => { + setup(); + await expect(accountService.findByUsernameAndSystemId('username', 'systemId')).resolves.not.toThrow(); expect(accountServiceDb.findByUsernameAndSystemId).toHaveBeenCalledTimes(1); }); @@ -178,6 +200,8 @@ describe('AccountService', () => { describe('when identity management is primary', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.findByUsernameAndSystemId.mockResolvedValueOnce(accountDoFactory.build()); + return newAccountService(); }; @@ -191,7 +215,13 @@ describe('AccountService', () => { describe('findMultipleByUserId', () => { describe('When calling findMultipleByUserId in accountService', () => { + const setup = () => { + accountServiceDb.findMultipleByUserId.mockResolvedValueOnce([accountDoFactory.build()]); + }; + it('should call findMultipleByUserId in accountServiceDb', async () => { + setup(); + await expect(accountService.findMultipleByUserId(['userId1, userId2'])).resolves.not.toThrow(); expect(accountServiceDb.findMultipleByUserId).toHaveBeenCalledTimes(1); }); @@ -200,6 +230,8 @@ describe('AccountService', () => { describe('When identity management is primary', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.findMultipleByUserId.mockResolvedValueOnce([accountDoFactory.build()]); + return newAccountService(); }; @@ -213,7 +245,13 @@ describe('AccountService', () => { describe('findByUserIdOrFail', () => { describe('When calling findByUserIdOrFail in accountService', () => { + const setup = () => { + accountServiceDb.findByUserIdOrFail.mockResolvedValueOnce(accountDoFactory.build()); + }; + it('should call findByUserIdOrFail in accountServiceDb', async () => { + setup(); + await expect(accountService.findByUserIdOrFail('userId')).resolves.not.toThrow(); expect(accountServiceDb.findByUserIdOrFail).toHaveBeenCalledTimes(1); }); @@ -222,6 +260,8 @@ describe('AccountService', () => { describe('When identity management is primary', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.findByUserIdOrFail.mockResolvedValueOnce(accountDoFactory.build()); + return newAccountService(); }; @@ -340,6 +380,30 @@ describe('AccountService', () => { }); }); + describe('saveAll', () => { + describe('when saving accounts', () => { + const setup = () => { + const accounts = accountDoFactory.buildList(1); + + configService.get.mockReturnValue(true); + accountServiceDb.saveAll.mockResolvedValueOnce(accounts); + accountServiceIdm.saveAll.mockResolvedValueOnce(accounts); + + return { accounts, sut: newAccountService() }; + }; + + it('should delegate to db and idm service', async () => { + const { accounts, sut } = setup(); + + const result = await sut.saveAll(accounts); + + expect(result).toBeDefined(); + expect(accountServiceDb.saveAll).toHaveBeenCalledTimes(1); + expect(accountServiceIdm.saveAll).toHaveBeenCalledTimes(1); + }); + }); + }); + describe('saveWithValidation', () => { describe('When calling with an empty username', () => { it('should throw an ValidationError', async () => { @@ -690,15 +754,25 @@ describe('AccountService', () => { }); describe('validatePassword', () => { - it('should call validatePassword in accountServiceDb', async () => { - await expect(accountService.validatePassword({} as Account, 'password')).resolves.not.toThrow(); - expect(accountServiceIdm.validatePassword).toHaveBeenCalledTimes(0); - expect(accountServiceDb.validatePassword).toHaveBeenCalledTimes(1); + describe('when validating a password', () => { + const setup = () => { + accountServiceDb.validatePassword.mockResolvedValueOnce(true); + }; + + it('should call validatePassword in accountServiceDb', async () => { + setup(); + + await expect(accountService.validatePassword({} as Account, 'password')).resolves.not.toThrow(); + expect(accountServiceIdm.validatePassword).toHaveBeenCalledTimes(0); + expect(accountServiceDb.validatePassword).toHaveBeenCalledTimes(1); + }); }); describe('When calling validatePassword in accountService if feature is enabled', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.validatePassword.mockResolvedValueOnce(true); + return newAccountService(); }; @@ -846,7 +920,13 @@ describe('AccountService', () => { describe('findMany', () => { describe('When calling findMany in accountService', () => { + const setup = () => { + accountServiceDb.findMany.mockResolvedValueOnce(accountDoFactory.buildList(1)); + }; + it('should call findMany in accountServiceDb', async () => { + setup(); + await expect(accountService.findMany()).resolves.not.toThrow(); expect(accountServiceDb.findMany).toHaveBeenCalledTimes(1); }); @@ -855,7 +935,13 @@ describe('AccountService', () => { describe('searchByUsernamePartialMatch', () => { describe('When calling searchByUsernamePartialMatch in accountService', () => { + const setup = () => { + accountServiceDb.searchByUsernamePartialMatch.mockResolvedValueOnce([accountDoFactory.buildList(1), 1]); + }; + it('should call searchByUsernamePartialMatch in accountServiceDb', async () => { + setup(); + await expect(accountService.searchByUsernamePartialMatch('username', 1, 1)).resolves.not.toThrow(); expect(accountServiceDb.searchByUsernamePartialMatch).toHaveBeenCalledTimes(1); }); @@ -863,7 +949,13 @@ describe('AccountService', () => { }); describe('searchByUsernameExactMatch', () => { + const setup = () => { + accountServiceDb.searchByUsernameExactMatch.mockResolvedValueOnce([accountDoFactory.buildList(1), 1]); + }; + it('should call searchByUsernameExactMatch in accountServiceDb', async () => { + setup(); + await expect(accountService.searchByUsernameExactMatch('username')).resolves.not.toThrow(); expect(accountServiceDb.searchByUsernameExactMatch).toHaveBeenCalledTimes(1); }); @@ -873,6 +965,8 @@ describe('AccountService', () => { describe('findById', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.findById.mockResolvedValueOnce(accountDoFactory.build()); + return newAccountService(); }; @@ -886,6 +980,8 @@ describe('AccountService', () => { describe('searchByUsernamePartialMatch', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.searchByUsernamePartialMatch.mockResolvedValueOnce([accountDoFactory.buildList(1), 1]); + return newAccountService(); }; @@ -899,7 +995,13 @@ describe('AccountService', () => { describe('searchByUsernameExactMatch', () => { describe('When calling searchByUsernameExactMatch in accountService', () => { + const setup = () => { + accountServiceDb.searchByUsernameExactMatch.mockResolvedValueOnce([accountDoFactory.buildList(1), 1]); + }; + it('should call searchByUsernameExactMatch in accountServiceDb', async () => { + setup(); + await expect(accountService.searchByUsernameExactMatch('username')).resolves.not.toThrow(); expect(accountServiceDb.searchByUsernameExactMatch).toHaveBeenCalledTimes(1); }); @@ -908,6 +1010,8 @@ describe('AccountService', () => { describe('when identity management is primary', () => { const setup = () => { configService.get.mockReturnValue(true); + accountServiceIdm.searchByUsernameExactMatch.mockResolvedValueOnce([accountDoFactory.buildList(1), 1]); + return newAccountService(); }; diff --git a/apps/server/src/modules/account/domain/services/account.service.ts b/apps/server/src/modules/account/domain/services/account.service.ts index c2f79223a5b..f13751da38b 100644 --- a/apps/server/src/modules/account/domain/services/account.service.ts +++ b/apps/server/src/modules/account/domain/services/account.service.ts @@ -72,7 +72,7 @@ export class AccountService extends AbstractAccountService implements DeletionSe } } - public async updateMyAccount(user: User, account: Account, updateData: UpdateMyAccount) { + public async updateMyAccount(user: User, account: Account, updateData: UpdateMyAccount): Promise { await this.checkUpdateMyAccountPrerequisites(updateData, account); const accountSave = new AccountSave({ @@ -144,7 +144,7 @@ export class AccountService extends AbstractAccountService implements DeletionSe return updateUserName; } - private async checkUpdateMyAccountPrerequisites(updateData: UpdateMyAccount, account: Account) { + private async checkUpdateMyAccountPrerequisites(updateData: UpdateMyAccount, account: Account): Promise { if (account.systemId) { throw new ForbiddenOperationError('External account details can not be changed.'); } @@ -244,35 +244,35 @@ export class AccountService extends AbstractAccountService implements DeletionSe await this.eventBus.publish(new DataDeletedEvent(deletionRequestId, dataDeleted)); } - async findById(id: string): Promise { + public findById(id: string): Promise { return this.accountImpl.findById(id); } - async findMultipleByUserId(userIds: string[]): Promise { + public findMultipleByUserId(userIds: string[]): Promise { return this.accountImpl.findMultipleByUserId(userIds); } - async findByUserId(userId: string): Promise { + public findByUserId(userId: string): Promise { return this.accountImpl.findByUserId(userId); } - async findByUserIdOrFail(userId: string): Promise { + public findByUserIdOrFail(userId: string): Promise { return this.accountImpl.findByUserIdOrFail(userId); } - async findByUsernameAndSystemId(username: string, systemId: string | ObjectId): Promise { + public findByUsernameAndSystemId(username: string, systemId: string | ObjectId): Promise { return this.accountImpl.findByUsernameAndSystemId(username, systemId); } - async searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise> { + public searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise> { return this.accountImpl.searchByUsernamePartialMatch(userName, skip, limit); } - async searchByUsernameExactMatch(userName: string): Promise> { + public searchByUsernameExactMatch(userName: string): Promise> { return this.accountImpl.searchByUsernameExactMatch(userName); } - async save(accountSave: AccountSave): Promise { + public async save(accountSave: AccountSave): Promise { const ret = await this.accountDb.save(accountSave); const newAccount = new AccountSave({ ...accountSave, @@ -299,7 +299,35 @@ export class AccountService extends AbstractAccountService implements DeletionSe return new Account({ ...ret.getProps(), idmReferenceId: idmAccount?.idmReferenceId }); } - async validateAccountBeforeSaveOrReject(accountSave: AccountSave) { + public async saveAll(accountSaves: AccountSave[]): Promise { + const savedDbAccounts = await this.accountDb.saveAll(accountSaves); + + const newAccounts = savedDbAccounts.map((ret, idx) => { + const accountSave = accountSaves[idx]; + + return new AccountSave({ + ...accountSave, + id: accountSave.id, + idmReferenceId: ret.id, + password: accountSave.password, + }); + }); + + const idmAccounts = await this.executeIdmMethod(async () => { + const account = await this.accountIdm.saveAll(newAccounts); + + return account; + }); + + const combinedAccounts = savedDbAccounts.map((ret, idx) => { + const idmReferenceId = idmAccounts ? idmAccounts[idx].idmReferenceId : undefined; + return new Account({ ...ret.getProps(), idmReferenceId }); + }); + + return combinedAccounts; + } + + public async validateAccountBeforeSaveOrReject(accountSave: AccountSave): Promise { // if username is undefined or empty, throw error ✔ if (!accountSave.username || !isNotEmpty(accountSave.username)) { throw new ValidationError('username can not be empty'); @@ -335,12 +363,12 @@ export class AccountService extends AbstractAccountService implements DeletionSe // } } - async saveWithValidation(accountSave: AccountSave): Promise { + public async saveWithValidation(accountSave: AccountSave): Promise { await this.validateAccountBeforeSaveOrReject(accountSave); await this.save(accountSave); } - async updateUsername(accountId: string, username: string): Promise { + public async updateUsername(accountId: string, username: string): Promise { const ret = await this.accountDb.updateUsername(accountId, username); const idmAccount = await this.executeIdmMethod(async () => { this.logger.debug(new UpdatingAccountUsernameLoggable(accountId)); @@ -351,11 +379,11 @@ export class AccountService extends AbstractAccountService implements DeletionSe return new Account({ ...ret.getProps(), idmReferenceId: idmAccount?.idmReferenceId }); } - async updateLastLogin(accountId: string, lastLogin: Date): Promise { + public async updateLastLogin(accountId: string, lastLogin: Date): Promise { await this.accountDb.updateLastLogin(accountId, lastLogin); } - async updateLastTriedFailedLogin(accountId: string, lastTriedFailedLogin: Date): Promise { + public async updateLastTriedFailedLogin(accountId: string, lastTriedFailedLogin: Date): Promise { const ret = await this.accountDb.updateLastTriedFailedLogin(accountId, lastTriedFailedLogin); const idmAccount = await this.executeIdmMethod(async () => { this.logger.debug(new UpdatingLastFailedLoginLoggable(accountId)); @@ -366,7 +394,7 @@ export class AccountService extends AbstractAccountService implements DeletionSe return new Account({ ...ret.getProps(), idmReferenceId: idmAccount?.idmReferenceId }); } - async updatePassword(accountId: string, password: string): Promise { + public async updatePassword(accountId: string, password: string): Promise { const ret = await this.accountDb.updatePassword(accountId, password); const idmAccount = await this.executeIdmMethod(async () => { this.logger.debug(new UpdatingAccountPasswordLoggable(accountId)); @@ -377,11 +405,11 @@ export class AccountService extends AbstractAccountService implements DeletionSe return new Account({ ...ret.getProps(), idmReferenceId: idmAccount?.idmReferenceId }); } - async validatePassword(account: Account, comparePassword: string): Promise { + public validatePassword(account: Account, comparePassword: string): Promise { return this.accountImpl.validatePassword(account, comparePassword); } - async delete(accountId: string): Promise { + public async delete(accountId: string): Promise { await this.accountDb.delete(accountId); await this.executeIdmMethod(async () => { this.logger.debug(new DeletingAccountLoggable(accountId)); @@ -418,11 +446,11 @@ export class AccountService extends AbstractAccountService implements DeletionSe /** * @deprecated For migration purpose only */ - async findMany(offset = 0, limit = 100): Promise { + public findMany(offset = 0, limit = 100): Promise { return this.accountDb.findMany(offset, limit); } - private async executeIdmMethod(idmCallback: () => Promise) { + private async executeIdmMethod(idmCallback: () => Promise): Promise { if (this.configService.get('FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED') === true) { try { return await idmCallback(); @@ -439,7 +467,7 @@ export class AccountService extends AbstractAccountService implements DeletionSe } } - async findByUserIdsAndSystemId(usersIds: string[], systemId: string): Promise { + public async findByUserIdsAndSystemId(usersIds: string[], systemId: string): Promise { const foundAccounts = await this.accountRepo.findByUserIdsAndSystemId(usersIds, systemId); return foundAccounts; diff --git a/apps/server/src/modules/account/repo/micro-orm/account.repo.integration.spec.ts b/apps/server/src/modules/account/repo/micro-orm/account.repo.integration.spec.ts index 76aff5bad1a..a20ee96bf16 100644 --- a/apps/server/src/modules/account/repo/micro-orm/account.repo.integration.spec.ts +++ b/apps/server/src/modules/account/repo/micro-orm/account.repo.integration.spec.ts @@ -68,6 +68,26 @@ describe('account repo', () => { }); }); + describe('saveAll', () => { + describe('When multiple accounts are given', () => { + const setup = () => { + const accounts = accountDoFactory.buildList(3); + const accountIds = accounts.map((account) => account.id); + + return { accounts, accountIds }; + }; + + it('should save all accounts', async () => { + const { accounts, accountIds } = setup(); + + await repo.saveAll(accounts); + + const foundAccounts = await em.find(AccountEntity, { id: { $in: accountIds } }); + expect(foundAccounts.length).toBe(accounts.length); + }); + }); + }); + describe('findById', () => { describe('When the account exists', () => { const setup = async () => { diff --git a/apps/server/src/modules/account/repo/micro-orm/account.repo.ts b/apps/server/src/modules/account/repo/micro-orm/account.repo.ts index 3f62fc2fefd..54641b91530 100644 --- a/apps/server/src/modules/account/repo/micro-orm/account.repo.ts +++ b/apps/server/src/modules/account/repo/micro-orm/account.repo.ts @@ -13,7 +13,7 @@ import { AccountScope } from './scope/account-scope'; export class AccountRepo { constructor(private readonly em: EntityManager) {} - get entityName() { + get entityName(): typeof AccountEntity { return AccountEntity; } @@ -33,6 +33,13 @@ export class AccountRepo { return AccountEntityToDoMapper.mapToDo(saved); } + public async saveAll(accounts: Account[]): Promise { + const savedAccounts = await Promise.all(accounts.map((account) => this.saveWithoutFlush(account))); + await this.flush(); + + return savedAccounts; + } + public async findById(id: EntityId | ObjectId): Promise { const entity = await this.em.findOneOrFail(this.entityName, id as FilterQuery); @@ -86,22 +93,26 @@ export class AccountRepo { return AccountEntityToDoMapper.mapToDo(entity); } - getObjectReference>( + public getObjectReference>( entityName: EntityName, id: Primary | Primary[] ): Entity { return this.em.getReference(entityName, id); } - public async saveWithoutFlush(account: Account): Promise { + public async saveWithoutFlush(account: Account): Promise { const saveEntity = AccountDoToEntityMapper.mapToEntity(account); const existing = await this.em.findOne(AccountEntity, { id: account.id }); + let saved: AccountEntity; if (existing) { - this.em.assign(existing, saveEntity); + saved = this.em.assign(existing, saveEntity); } else { this.em.persist(saveEntity); + saved = saveEntity; } + + return AccountEntityToDoMapper.mapToDo(saved); } public async flush(): Promise { @@ -175,7 +186,7 @@ export class AccountRepo { return AccountEntityToDoMapper.mapEntitiesToDos(result); } - async findByUserIdsAndSystemId(userIds: string[], systemId: string): Promise { + public async findByUserIdsAndSystemId(userIds: string[], systemId: string): Promise { const scope = new AccountScope(); const userIdScope = new AccountScope(); diff --git a/apps/server/src/modules/provisioning/service/provisioning.service.ts b/apps/server/src/modules/provisioning/service/provisioning.service.ts index ef059ff6620..013206ada7f 100644 --- a/apps/server/src/modules/provisioning/service/provisioning.service.ts +++ b/apps/server/src/modules/provisioning/service/provisioning.service.ts @@ -46,6 +46,7 @@ export class ProvisioningService { const strategy: ProvisioningStrategy = this.getProvisioningStrategy(system.provisioningStrategy); const data: OauthDataDto = await strategy.getData(input); + return data; } diff --git a/apps/server/src/modules/user/service/user.service.spec.ts b/apps/server/src/modules/user/service/user.service.spec.ts index 48e0706635d..ec71f6a8486 100644 --- a/apps/server/src/modules/user/service/user.service.spec.ts +++ b/apps/server/src/modules/user/service/user.service.spec.ts @@ -1150,4 +1150,24 @@ describe('UserService', () => { }); }); }); + + describe('findByTspUids', () => { + describe('when looking for users with tspUids', () => { + const setup = () => { + const user = userDoFactory.build(); + userDORepo.findByTspUids.mockResolvedValueOnce([user]); + + return { user }; + }; + + it('should delegate to the userRepo', async () => { + const { user } = setup(); + + const result = await service.findByTspUids(['tspUid']); + + expect(result).toStrictEqual([user]); + expect(userDORepo.findByTspUids).toHaveBeenCalledTimes(1); + }); + }); + }); }); diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index 811cedbfea8..920ec2d37da 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -340,4 +340,10 @@ export class UserService implements DeletionService, IEventHandler { + const userDOs = this.userDORepo.findByTspUids(tspUids); + + return userDOs; + } } diff --git a/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts b/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts index 4828117220d..fc3beb920b4 100644 --- a/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts @@ -9,6 +9,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { EntityNotFoundError } from '@shared/common'; import { RoleReference } from '@shared/domain/domainobject'; import { Page } from '@shared/domain/domainobject/page'; +import { UserSourceOptions } from '@shared/domain/domainobject/user-source-options.do'; import { UserDO } from '@shared/domain/domainobject/user.do'; import { Role, SchoolEntity, User } from '@shared/domain/entity'; import { IFindOptions, LanguageType, RoleName, SortOrder } from '@shared/domain/interface'; @@ -845,4 +846,32 @@ describe('UserRepo', () => { }); }); }); + + describe('findByTspUids', () => { + describe('when users are found', () => { + const setup = async () => { + const tspUid = new ObjectId().toHexString(); + + const user: User = userFactory.buildWithId({ + sourceOptions: new UserSourceOptions({ + tspUid, + }), + }); + + await em.persistAndFlush([user]); + em.clear(); + + return { tspUid, user }; + }; + + it('should return mapped users', async () => { + const { tspUid, user } = await setup(); + + const result = await repo.findByTspUids([tspUid]); + + expect(result.length).toBe(1); + expect(result[0].id).toBe(user.id); + }); + }); + }); }); diff --git a/apps/server/src/shared/repo/user/user-do.repo.ts b/apps/server/src/shared/repo/user/user-do.repo.ts index 6115948aa60..c06c9c5b961 100644 --- a/apps/server/src/shared/repo/user/user-do.repo.ts +++ b/apps/server/src/shared/repo/user/user-do.repo.ts @@ -19,7 +19,7 @@ export class UserDORepo extends BaseDORepo { return User; } - async find(query: UserQuery, options?: IFindOptions) { + public async find(query: UserQuery, options?: IFindOptions) { const pagination: Pagination = options?.pagination || {}; const order: QueryOrderMap = this.createQueryOrderMap(options?.order || {}); const scope: Scope = new UserScope() @@ -49,7 +49,7 @@ export class UserDORepo extends BaseDORepo { return page; } - async findById(id: EntityId, populate = false): Promise { + public async findById(id: EntityId, populate = false): Promise { const userEntity: User = await this._em.findOneOrFail(this.entityName, id as FilterQuery); if (populate) { @@ -60,7 +60,7 @@ export class UserDORepo extends BaseDORepo { return this.mapEntityToDO(userEntity); } - async findByIds(ids: string[], populate = false): Promise { + public async findByIds(ids: string[], populate = false): Promise { const users = await this._em.find(User, { id: { $in: ids } }); if (populate) { @@ -83,7 +83,7 @@ export class UserDORepo extends BaseDORepo { return userDOs; } - async findByIdOrNull(id: EntityId, populate = false): Promise { + public async findByIdOrNull(id: EntityId, populate = false): Promise { const user: User | null = await this._em.findOne(this.entityName, id as FilterQuery); if (!user) { @@ -100,7 +100,7 @@ export class UserDORepo extends BaseDORepo { return domainObject; } - async findByExternalIdOrFail(externalId: string, systemId: string): Promise { + public async findByExternalIdOrFail(externalId: string, systemId: string): Promise { const userDo: UserDO | null = await this.findByExternalId(externalId, systemId); if (userDo) { return userDo; @@ -108,7 +108,7 @@ export class UserDORepo extends BaseDORepo { throw new EntityNotFoundError('User'); } - async findByExternalId(externalId: string, systemId: string): Promise { + public async findByExternalId(externalId: string, systemId: string): Promise { const userEntitys: User[] = await this._em.find(User, { externalId }, { populate: ['school.systems'] }); if (userEntitys.length > 1) { @@ -123,7 +123,7 @@ export class UserDORepo extends BaseDORepo { return userDo; } - async findByEmail(email: string): Promise { + public async findByEmail(email: string): Promise { // find mail case-insensitive by regex const userEntitys: User[] = await this._em.find(User, { email: new RegExp(`^${email.replace(/\W/g, '\\$&')}$`, 'i'), @@ -136,7 +136,7 @@ export class UserDORepo extends BaseDORepo { return userDos; } - mapEntityToDO(entity: User): UserDO { + public mapEntityToDO(entity: User): UserDO { const user: UserDO = new UserDO({ id: entity.id, createdAt: entity.createdAt, @@ -184,7 +184,7 @@ export class UserDORepo extends BaseDORepo { return user; } - mapDOToEntityProperties(entityDO: UserDO): EntityData { + public mapDOToEntityProperties(entityDO: UserDO): EntityData { return { email: entityDO.email, firstName: entityDO.firstName, @@ -214,6 +214,22 @@ export class UserDORepo extends BaseDORepo { }; } + public async findByTspUids(tspUids: string[]): Promise { + const users = await this._em.find( + User, + { + sourceOptions: { tspUid: { $in: tspUids } }, + }, + { + populate: ['roles', 'school.systems', 'school.currentYear', 'school.name', 'secondarySchools.role'], + } + ); + + const userDOs = users.map((user) => this.mapEntityToDO(user)); + + return userDOs; + } + private createQueryOrderMap(sort: SortOrderMap): QueryOrderMap { const queryOrderMap: QueryOrderMap = { _id: sort.id,