From d74dc3a3151aa104c9394ea1258f448635e2b17b Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Tue, 8 Oct 2024 15:59:59 +0200 Subject: [PATCH 1/9] Add school part to sync. --- apps/server/src/infra/sync/sync.module.ts | 21 +++- ...ystem-not-found.loggable-exception.spec.ts | 24 +++++ ...tsp-system-not-found.loggable-exception.ts | 25 +++++ .../src/infra/sync/tsp/tsp-sync.service.ts | 98 +++++++++++++++++++ .../src/infra/sync/tsp/tsp-sync.strategy.ts | 46 ++++++++- .../types/federal-state-names.enum.ts | 1 + .../src/modules/school/domain/do/school.ts | 4 + 7 files changed, 210 insertions(+), 9 deletions(-) create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts create mode 100644 apps/server/src/infra/sync/tsp/tsp-sync.service.ts diff --git a/apps/server/src/infra/sync/sync.module.ts b/apps/server/src/infra/sync/sync.module.ts index 2516e4b13df..5a8b9207d12 100644 --- a/apps/server/src/infra/sync/sync.module.ts +++ b/apps/server/src/infra/sync/sync.module.ts @@ -1,19 +1,30 @@ +import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { ConsoleWriterModule } from '@infra/console'; +import { TspClientModule } from '@infra/tsp-client/tsp-client.module'; +import { LegacySchoolModule } from '@modules/legacy-school'; +import { SchoolModule } from '@modules/school'; +import { SystemModule } from '@modules/system'; import { Module } from '@nestjs/common'; import { LoggerModule } from '@src/core/logger'; -import { ConsoleWriterModule } from '@infra/console'; -import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { SyncConsole } from './console/sync.console'; -import { SyncUc } from './uc/sync.uc'; import { SyncService } from './service/sync.service'; +import { TspSyncService } from './tsp/tsp-sync.service'; import { TspSyncStrategy } from './tsp/tsp-sync.strategy'; +import { SyncUc } from './uc/sync.uc'; @Module({ - imports: [LoggerModule, ConsoleWriterModule], + imports: [ + LoggerModule, + ConsoleWriterModule, + ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) + ? [TspClientModule, SystemModule, SchoolModule, LegacySchoolModule] + : []), + ], providers: [ SyncConsole, SyncUc, SyncService, - ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) ? [TspSyncStrategy] : []), + ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) ? [TspSyncStrategy, TspSyncService] : []), ], exports: [SyncConsole], }) diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts new file mode 100644 index 00000000000..1e015b0698d --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts @@ -0,0 +1,24 @@ +import { TspSystemNotFoundLoggableException } from './tsp-system-not-found.loggable-exception'; + +describe(TspSystemNotFoundLoggableException.name, () => { + let loggable: TspSystemNotFoundLoggableException; + + beforeAll(() => { + loggable = new TspSystemNotFoundLoggableException(); + }); + + describe('when loggable is initialized', () => { + it('should be defined', () => { + expect(loggable).toBeDefined(); + }); + }); + + describe('getLogMessage', () => { + it('should return a log message', () => { + expect(loggable.getLogMessage()).toEqual({ + type: 'TSP_SYSTEM_NOT_FOUND', + stack: expect.any(String), + }); + }); + }); +}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts new file mode 100644 index 00000000000..a2e15ff7db8 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts @@ -0,0 +1,25 @@ +import { HttpStatus } from '@nestjs/common'; +import { BusinessError, ErrorLogMessage } from '@shared/common'; +import { Loggable } from '@src/core/logger'; + +export class TspSystemNotFoundLoggableException extends BusinessError implements Loggable { + constructor() { + super( + { + type: 'TSP_SYSTEM_NOT_FOUND', + title: 'The TSP system could not be found', + defaultMessage: 'The TSP system could not be found during the sync', + }, + HttpStatus.BAD_REQUEST + ); + } + + getLogMessage(): ErrorLogMessage { + const message: ErrorLogMessage = { + type: this.type, + stack: this.stack, + }; + + return message; + } +} diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts new file mode 100644 index 00000000000..b2e37405c73 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -0,0 +1,98 @@ +import { TspClientFactory } from '@infra/tsp-client'; +import { FederalStateService, SchoolYearService } from '@modules/legacy-school'; +import { FederalStateNames } from '@src/modules/legacy-school/types'; +import { School, SchoolService } from '@modules/school'; +import { System, SystemService, SystemType } from '@modules/system'; +import { Injectable } from '@nestjs/common'; +import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; +import { SchoolFeature } from '@shared/domain/types'; +import { FederalState } from '@src/modules/school/domain'; +import { SchoolFactory } from '@src/modules/school/domain/factory'; +import { FederalStateEntityMapper, SchoolYearEntityMapper } from '@src/modules/school/repo/mikro-orm/mapper'; +import { ObjectId } from 'bson'; +import { TspSystemNotFoundLoggableException } from './loggable/tsp-system-not-found.loggable-exception'; + +@Injectable() +export class TspSyncService { + private federalState: FederalState | undefined; + + constructor( + private readonly tspClientFactory: TspClientFactory, + private readonly systemService: SystemService, + private readonly schoolService: SchoolService, + private readonly federalStateService: FederalStateService, + private readonly schoolYearService: SchoolYearService + ) {} + + public async findTspSystemOrFail(): Promise { + const systems = ( + await this.systemService.find({ + types: [SystemType.OIDC], + }) + ).filter((system) => system.provisioningStrategy === SystemProvisioningStrategy.TSP); + + if (systems.length === 0) { + throw new TspSystemNotFoundLoggableException(); + } + + return systems[0]; + } + + public async fetchTspSchools() { + const client = this.tspClientFactory.createExportClient(); + const schools = (await client.exportSchuleList()).data; + + return schools; + } + + public async findSchool(system: System, identifier: string): Promise { + const schools = await this.schoolService.getSchools({ + externalId: identifier, + systemId: system.id, + }); + + if (schools.length === 0) { + return undefined; + } + return schools[0]; + } + + public async updateSchool(school: School, name: string): Promise { + school.name = name; + + const updatedSchool = await this.schoolService.save(school); + + return updatedSchool; + } + + public async createSchool(system: System, identifier: string, name: string): Promise { + const schoolYearEntity = await this.schoolYearService.getCurrentSchoolYear(); + const schoolYear = SchoolYearEntityMapper.mapToDo(schoolYearEntity); + + const school = SchoolFactory.build({ + externalId: identifier, + name, + systemIds: [system.id], + federalState: await this.findFederalState(), + currentYear: schoolYear, + features: new Set([SchoolFeature.OAUTH_PROVISIONING_ENABLED]), + createdAt: new Date(), + updatedAt: new Date(), + id: new ObjectId().toHexString(), + }); + + const savedSchool = await this.schoolService.save(school); + + return savedSchool; + } + + private async findFederalState(): Promise { + if (this.federalState) { + return this.federalState; + } + + const federalStateEntity = await this.federalStateService.findFederalStateByName(FederalStateNames.THUERINGEN); + this.federalState = FederalStateEntityMapper.mapToDo(federalStateEntity); + return this.federalState; + } +} 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 5f7eb3a44c5..71bb56cb9d4 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts @@ -1,15 +1,53 @@ +import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { School } from '@modules/school'; import { Injectable } from '@nestjs/common'; +import { System } from '@src/modules/system'; +import pLimit from 'p-limit'; import { SyncStrategy } from '../strategy/sync-strategy'; import { SyncStrategyTarget } from '../sync-strategy.types'; +import { TspSyncService } from './tsp-sync.service'; @Injectable() export class TspSyncStrategy extends SyncStrategy { - getType(): SyncStrategyTarget { + private readonly schoolLimit: pLimit.Limit; + + constructor(private readonly tspSyncService: TspSyncService) { + super(); + this.schoolLimit = pLimit(Configuration.get('TSP_SYNC_SCHOOL_LIMIT') as number); + } + + public override getType(): SyncStrategyTarget { return SyncStrategyTarget.TSP; } - sync(): Promise { - // implementation - return Promise.resolve(); + public async sync(): Promise { + const system = await this.tspSyncService.findTspSystemOrFail(); + + await this.syncSchools(system); + } + + private async syncSchools(system: System): Promise { + const tspSchools = await this.tspSyncService.fetchTspSchools(); + + const schoolPromises = tspSchools.map((school) => + this.schoolLimit(async () => { + const existingSchool = await this.tspSyncService.findSchool(system, school.schuleNummer ?? ''); + + if (existingSchool) { + const updatedSchool = await this.tspSyncService.updateSchool(existingSchool, school.schuleName ?? ''); + return updatedSchool; + } + + const createdSchool = await this.tspSyncService.createSchool( + system, + school.schuleNummer ?? '', + school.schuleName ?? '' + ); + return createdSchool; + }) + ); + + const scSchools = await Promise.all(schoolPromises); + return scSchools; } } diff --git a/apps/server/src/modules/legacy-school/types/federal-state-names.enum.ts b/apps/server/src/modules/legacy-school/types/federal-state-names.enum.ts index e6d426e45f6..b9b74ff4656 100644 --- a/apps/server/src/modules/legacy-school/types/federal-state-names.enum.ts +++ b/apps/server/src/modules/legacy-school/types/federal-state-names.enum.ts @@ -1,3 +1,4 @@ export enum FederalStateNames { NIEDERSACHEN = 'Niedersachsen', + THUERINGEN = 'Thüringen', } diff --git a/apps/server/src/modules/school/domain/do/school.ts b/apps/server/src/modules/school/domain/do/school.ts index eaa237827d9..5a94772db20 100644 --- a/apps/server/src/modules/school/domain/do/school.ts +++ b/apps/server/src/modules/school/domain/do/school.ts @@ -40,6 +40,10 @@ export class School extends DomainObject { this.props.ldapLastSync = ldapLastSync; } + set name(name: string) { + this.props.name = name; + } + public getInfo(): SchoolInfo { const info = { id: this.props.id, From f3265dcacea829b5eba6eb8a9aef5d614411903b Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Tue, 8 Oct 2024 16:20:59 +0200 Subject: [PATCH 2/9] use client --- apps/server/src/infra/sync/tsp/tsp-sync.service.ts | 8 ++++++-- apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) 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 b2e37405c73..795c4d3ae26 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -38,8 +38,12 @@ export class TspSyncService { return systems[0]; } - public async fetchTspSchools() { - const client = this.tspClientFactory.createExportClient(); + public async fetchTspSchools(system: System) { + const client = this.tspClientFactory.createExportClient({ + clientId: system.oauthConfig?.clientId ?? '', + clientSecret: system.oauthConfig?.clientSecret ?? '', + tokenEndpoint: system.oauthConfig?.tokenEndpoint ?? '', + }); const schools = (await client.exportSchuleList()).data; return schools; 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 71bb56cb9d4..4b5029ee1a2 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts @@ -27,7 +27,7 @@ export class TspSyncStrategy extends SyncStrategy { } private async syncSchools(system: System): Promise { - const tspSchools = await this.tspSyncService.fetchTspSchools(); + const tspSchools = await this.tspSyncService.fetchTspSchools(system); const schoolPromises = tspSchools.map((school) => this.schoolLimit(async () => { From 96ef0103a927e72c22ffd0b803a036f4f24b3fab Mon Sep 17 00:00:00 2001 From: Alexander Weber Date: Tue, 8 Oct 2024 17:19:55 +0200 Subject: [PATCH 3/9] fix dependencies and schema json --- apps/server/src/infra/sync/sync.module.ts | 2 ++ apps/server/src/infra/sync/tsp/tsp-sync.service.ts | 13 ++++++++++--- config/default.schema.json | 5 +++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/apps/server/src/infra/sync/sync.module.ts b/apps/server/src/infra/sync/sync.module.ts index 5a8b9207d12..59ffee92b44 100644 --- a/apps/server/src/infra/sync/sync.module.ts +++ b/apps/server/src/infra/sync/sync.module.ts @@ -6,6 +6,7 @@ import { SchoolModule } from '@modules/school'; import { SystemModule } from '@modules/system'; import { Module } from '@nestjs/common'; import { LoggerModule } from '@src/core/logger'; +import { RabbitMQWrapperModule } from '@infra/rabbitmq'; import { SyncConsole } from './console/sync.console'; import { SyncService } from './service/sync.service'; import { TspSyncService } from './tsp/tsp-sync.service'; @@ -14,6 +15,7 @@ import { SyncUc } from './uc/sync.uc'; @Module({ imports: [ + RabbitMQWrapperModule, LoggerModule, ConsoleWriterModule, ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) 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 795c4d3ae26..15629bf2cb5 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -1,4 +1,4 @@ -import { TspClientFactory } from '@infra/tsp-client'; +import { RobjExportSchule, TspClientFactory } from '@infra/tsp-client'; import { FederalStateService, SchoolYearService } from '@modules/legacy-school'; import { FederalStateNames } from '@src/modules/legacy-school/types'; import { School, SchoolService } from '@modules/school'; @@ -10,6 +10,7 @@ import { FederalState } from '@src/modules/school/domain'; import { SchoolFactory } from '@src/modules/school/domain/factory'; import { FederalStateEntityMapper, SchoolYearEntityMapper } from '@src/modules/school/repo/mikro-orm/mapper'; import { ObjectId } from 'bson'; +import moment from 'moment/moment'; import { TspSystemNotFoundLoggableException } from './loggable/tsp-system-not-found.loggable-exception'; @Injectable() @@ -27,7 +28,7 @@ export class TspSyncService { public async findTspSystemOrFail(): Promise { const systems = ( await this.systemService.find({ - types: [SystemType.OIDC], + types: [SystemType.OAUTH, SystemType.OIDC], }) ).filter((system) => system.provisioningStrategy === SystemProvisioningStrategy.TSP); @@ -44,7 +45,10 @@ export class TspSyncService { clientSecret: system.oauthConfig?.clientSecret ?? '', tokenEndpoint: system.oauthConfig?.tokenEndpoint ?? '', }); - const schools = (await client.exportSchuleList()).data; + const lastChangeDate = moment(new Date(0)).format('YYYY-MM-DD HH:mm:ss.SSS'); + const schools: RobjExportSchule[] = (await client.exportSchuleList(lastChangeDate)).data; + + console.log(`Found ${schools.length} schools in TSP`); return schools; } @@ -65,6 +69,7 @@ export class TspSyncService { school.name = name; const updatedSchool = await this.schoolService.save(school); + console.log(`Updated School: ${updatedSchool.getInfo().name}`); return updatedSchool; } @@ -87,6 +92,8 @@ export class TspSyncService { const savedSchool = await this.schoolService.save(school); + console.log(`Created School: ${savedSchool.getInfo().name}`); + return savedSchool; } diff --git a/config/default.schema.json b/config/default.schema.json index 387e7331b1a..8bc787bde6b 100644 --- a/config/default.schema.json +++ b/config/default.schema.json @@ -179,6 +179,11 @@ "default": "30000", "description": "The TSP token lifetime in milliseconds." }, + "TSP_SYNC_SCHOOL_LIMIT": { + "type": "number", + "default": "10", + "description": "The amount of schools the sync handles at once." + }, "FEATURE_TSP_ENABLED": { "type": "boolean", "default": false, From bd309492dde2d3de913aeea14b7e144161622dbe Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Wed, 9 Oct 2024 11:42:33 +0200 Subject: [PATCH 4/9] work on school sync --- apps/server/src/infra/sync/index.ts | 1 + apps/server/src/infra/sync/tsp/index.ts | 2 + .../loggable/tsp-schools-fetched.loggable.ts | 16 ++++++++ .../loggable/tsp-schools-synced.loggable.ts | 22 +++++++++++ .../src/infra/sync/tsp/tsp-sync.config.ts | 3 ++ .../src/infra/sync/tsp/tsp-sync.service.ts | 14 +++---- .../src/infra/sync/tsp/tsp-sync.strategy.ts | 38 +++++++++++++------ .../src/modules/server/server.config.ts | 3 ++ 8 files changed, 81 insertions(+), 18 deletions(-) create mode 100644 apps/server/src/infra/sync/index.ts create mode 100644 apps/server/src/infra/sync/tsp/index.ts create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts create mode 100644 apps/server/src/infra/sync/tsp/tsp-sync.config.ts diff --git a/apps/server/src/infra/sync/index.ts b/apps/server/src/infra/sync/index.ts new file mode 100644 index 00000000000..0a3165f6f52 --- /dev/null +++ b/apps/server/src/infra/sync/index.ts @@ -0,0 +1 @@ +export * from './tsp'; diff --git a/apps/server/src/infra/sync/tsp/index.ts b/apps/server/src/infra/sync/tsp/index.ts new file mode 100644 index 00000000000..86513d07df8 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/index.ts @@ -0,0 +1,2 @@ +export { TspSyncConfig } from './tsp-sync.config'; +export { TspSyncStrategy } from './tsp-sync.strategy'; diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts new file mode 100644 index 00000000000..7022ab06d07 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts @@ -0,0 +1,16 @@ +import { Loggable, LogMessage } from '@src/core/logger'; + +export class TspSchoolsFetchedLoggable implements Loggable { + constructor(private readonly tspSchoolCount: number) {} + + getLogMessage(): LogMessage { + const message: LogMessage = { + message: `Fetched ${this.tspSchoolCount} schools from TSP`, + data: { + tspSchoolCount: this.tspSchoolCount, + }, + }; + + return message; + } +} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts new file mode 100644 index 00000000000..868348991b6 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts @@ -0,0 +1,22 @@ +import { Loggable, LogMessage } from '@src/core/logger'; + +export class TspSchoolsSyncedLoggable implements Loggable { + constructor( + private readonly tspSchoolCount: number, + private readonly createdSchools: number, + private readonly updatedSchools: number + ) {} + + getLogMessage(): LogMessage { + const message: LogMessage = { + message: `Synced schools: Of ${this.tspSchoolCount} schools, ${this.createdSchools} were created and ${this.updatedSchools} were updated`, + data: { + tspSchoolCount: this.tspSchoolCount, + createdSchools: this.createdSchools, + updatedSchools: this.updatedSchools, + }, + }; + + return message; + } +} diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.config.ts b/apps/server/src/infra/sync/tsp/tsp-sync.config.ts new file mode 100644 index 00000000000..6c0d5ceae56 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/tsp-sync.config.ts @@ -0,0 +1,3 @@ +export interface TspSyncConfig { + TSP_SYNC_SCHOOL_LIMIT: number; +} 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 15629bf2cb5..ba47bff9fea 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -1,11 +1,11 @@ import { RobjExportSchule, TspClientFactory } from '@infra/tsp-client'; import { FederalStateService, SchoolYearService } from '@modules/legacy-school'; -import { FederalStateNames } from '@src/modules/legacy-school/types'; import { School, SchoolService } from '@modules/school'; import { System, SystemService, SystemType } from '@modules/system'; import { Injectable } from '@nestjs/common'; import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; import { SchoolFeature } from '@shared/domain/types'; +import { FederalStateNames } from '@src/modules/legacy-school/types'; import { FederalState } from '@src/modules/school/domain'; import { SchoolFactory } from '@src/modules/school/domain/factory'; import { FederalStateEntityMapper, SchoolYearEntityMapper } from '@src/modules/school/repo/mikro-orm/mapper'; @@ -45,10 +45,9 @@ export class TspSyncService { clientSecret: system.oauthConfig?.clientSecret ?? '', tokenEndpoint: system.oauthConfig?.tokenEndpoint ?? '', }); - const lastChangeDate = moment(new Date(0)).format('YYYY-MM-DD HH:mm:ss.SSS'); - const schools: RobjExportSchule[] = (await client.exportSchuleList(lastChangeDate)).data; - console.log(`Found ${schools.length} schools in TSP`); + const lastChangeDate = this.formatChangeDate(new Date(0)); + const schools: RobjExportSchule[] = (await client.exportSchuleList(lastChangeDate)).data; return schools; } @@ -69,7 +68,6 @@ export class TspSyncService { school.name = name; const updatedSchool = await this.schoolService.save(school); - console.log(`Updated School: ${updatedSchool.getInfo().name}`); return updatedSchool; } @@ -92,8 +90,6 @@ export class TspSyncService { const savedSchool = await this.schoolService.save(school); - console.log(`Created School: ${savedSchool.getInfo().name}`); - return savedSchool; } @@ -106,4 +102,8 @@ export class TspSyncService { this.federalState = FederalStateEntityMapper.mapToDo(federalStateEntity); return this.federalState; } + + private formatChangeDate(date: Date): string { + return moment(date).format('YYYY-MM-DD HH:mm:ss.SSS'); + } } 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 4b5029ee1a2..377ea45ed4a 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts @@ -1,19 +1,28 @@ -import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { School } from '@modules/school'; import { Injectable } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { Logger } from '@src/core/logger'; import { System } from '@src/modules/system'; import pLimit from 'p-limit'; +import { TspSyncConfig } from './tsp-sync.config'; import { SyncStrategy } from '../strategy/sync-strategy'; import { SyncStrategyTarget } from '../sync-strategy.types'; +import { TspSchoolsFetchedLoggable } from './loggable/tsp-schools-fetched.loggable'; +import { TspSchoolsSyncedLoggable } from './loggable/tsp-schools-synced.loggable'; import { TspSyncService } from './tsp-sync.service'; @Injectable() export class TspSyncStrategy extends SyncStrategy { private readonly schoolLimit: pLimit.Limit; - constructor(private readonly tspSyncService: TspSyncService) { + constructor( + private readonly logger: Logger, + private readonly tspSyncService: TspSyncService, + configService: ConfigService + ) { super(); - this.schoolLimit = pLimit(Configuration.get('TSP_SYNC_SCHOOL_LIMIT') as number); + this.logger.setContext(TspSyncStrategy.name); + this.schoolLimit = pLimit(configService.getOrThrow('TSP_SYNC_SCHOOL_LIMIT')); } public override getType(): SyncStrategyTarget { @@ -28,26 +37,33 @@ export class TspSyncStrategy extends SyncStrategy { private async syncSchools(system: System): Promise { const tspSchools = await this.tspSyncService.fetchTspSchools(system); + this.logger.info(new TspSchoolsFetchedLoggable(tspSchools.length)); - const schoolPromises = tspSchools.map((school) => + const schoolPromises = tspSchools.map((tspSchool) => this.schoolLimit(async () => { - const existingSchool = await this.tspSyncService.findSchool(system, school.schuleNummer ?? ''); + const existingSchool = await this.tspSyncService.findSchool(system, tspSchool.schuleNummer ?? ''); if (existingSchool) { - const updatedSchool = await this.tspSyncService.updateSchool(existingSchool, school.schuleName ?? ''); - return updatedSchool; + const updatedSchool = await this.tspSyncService.updateSchool(existingSchool, tspSchool.schuleName ?? ''); + return { school: updatedSchool, created: false }; } const createdSchool = await this.tspSyncService.createSchool( system, - school.schuleNummer ?? '', - school.schuleName ?? '' + tspSchool.schuleNummer ?? '', + tspSchool.schuleName ?? '' ); - return createdSchool; + return { school: createdSchool, created: true }; }) ); const scSchools = await Promise.all(schoolPromises); - return scSchools; + + const total = scSchools.length; + const createdSchools = scSchools.filter((scSchool) => scSchool.created).length; + const updatedSchools = total - createdSchools; + this.logger.info(new TspSchoolsSyncedLoggable(total, createdSchools, updatedSchools)); + + return scSchools.map((scSchool) => scSchool.school); } } diff --git a/apps/server/src/modules/server/server.config.ts b/apps/server/src/modules/server/server.config.ts index ca5aa6f9d02..bbec4f6df39 100644 --- a/apps/server/src/modules/server/server.config.ts +++ b/apps/server/src/modules/server/server.config.ts @@ -3,6 +3,7 @@ import { XApiKeyConfig } from '@infra/auth-guard'; import type { IdentityManagementConfig } from '@infra/identity-management'; import type { MailConfig } from '@infra/mail/interfaces/mail-config'; import type { SchulconnexClientConfig } from '@infra/schulconnex-client'; +import { TspSyncConfig } from '@infra/sync'; import type { TspClientConfig } from '@infra/tsp-client'; import type { AccountConfig } from '@modules/account'; import { AlertConfig } from '@modules/alert'; @@ -70,6 +71,7 @@ export interface ServerConfig VideoConferenceConfig, BbbConfig, TspClientConfig, + TspSyncConfig, AlertConfig, ShdConfig { NODE_ENV: NodeEnvType; @@ -310,6 +312,7 @@ const config: ServerConfig = { FEATURE_ROOMS_ENABLED: Configuration.get('FEATURE_ROOMS_ENABLED') as boolean, TSP_API_BASE_URL: Configuration.get('TSP_API_BASE_URL') as string, TSP_API_TOKEN_LIFETIME_MS: Configuration.get('TSP_API_TOKEN_LIFETIME_MS') as number, + TSP_SYNC_SCHOOL_LIMIT: Configuration.get('TSP_SYNC_SCHOOL_LIMIT') as number, ROCKET_CHAT_URI: Configuration.get('ROCKET_CHAT_URI') as string, ROCKET_CHAT_ADMIN_ID: Configuration.get('ROCKET_CHAT_ADMIN_ID') as string, ROCKET_CHAT_ADMIN_TOKEN: Configuration.get('ROCKET_CHAT_ADMIN_TOKEN') as string, From 1bf0dbb16fbb86f8f460603be6839de522c7e357 Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Wed, 9 Oct 2024 17:15:48 +0200 Subject: [PATCH 5/9] Add tests. --- .../tsp-schools-fetched.loggable.spec.ts | 26 ++ .../tsp-schools-synced.loggable.spec.ts | 28 ++ .../infra/sync/tsp/tsp-sync.service.spec.ts | 313 ++++++++++++++++++ .../src/infra/sync/tsp/tsp-sync.service.ts | 3 +- .../infra/sync/tsp/tsp-sync.strategy.spec.ts | 113 ++++++- 5 files changed, 473 insertions(+), 10 deletions(-) create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts create mode 100644 apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts new file mode 100644 index 00000000000..af9e36d79cb --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts @@ -0,0 +1,26 @@ +import { TspSchoolsFetchedLoggable } from './tsp-schools-fetched.loggable'; + +describe(TspSchoolsFetchedLoggable.name, () => { + let loggable: TspSchoolsFetchedLoggable; + + beforeAll(() => { + loggable = new TspSchoolsFetchedLoggable(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 schools from TSP`, + data: { + tspSchoolCount: 10, + }, + }); + }); + }); +}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts new file mode 100644 index 00000000000..08c85f65c6c --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts @@ -0,0 +1,28 @@ +import { TspSchoolsSyncedLoggable } from './tsp-schools-synced.loggable'; + +describe(TspSchoolsSyncedLoggable.name, () => { + let loggable: TspSchoolsSyncedLoggable; + + beforeAll(() => { + loggable = new TspSchoolsSyncedLoggable(10, 5, 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: `Synced schools: Of 10 schools, 5 were created and 5 were updated`, + data: { + tspSchoolCount: 10, + createdSchools: 5, + updatedSchools: 5, + }, + }); + }); + }); +}); 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 new file mode 100644 index 00000000000..e550ae5637f --- /dev/null +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts @@ -0,0 +1,313 @@ +import { faker } from '@faker-js/faker'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ExportApiInterface, RobjExportSchule, TspClientFactory } from '@infra/tsp-client'; +import { School, SchoolService } from '@modules/school'; +import { SystemService, SystemType } from '@modules/system'; +import { Test, TestingModule } from '@nestjs/testing'; +import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; +import { federalStateFactory, schoolYearFactory } from '@shared/testing'; +import { FederalStateService, SchoolYearService } from '@src/modules/legacy-school'; +import { SchoolProps } from '@src/modules/school/domain'; +import { FederalStateEntityMapper, SchoolYearEntityMapper } from '@src/modules/school/repo/mikro-orm/mapper'; +import { schoolFactory } from '@src/modules/school/testing'; +import { systemFactory } from '@src/modules/system/testing'; +import { AxiosResponse } from 'axios'; +import { TspSyncService } from './tsp-sync.service'; + +describe(TspSyncService.name, () => { + let module: TestingModule; + let sut: TspSyncService; + let tspClientFactory: DeepMocked; + let systemService: DeepMocked; + let schoolService: DeepMocked; + let federalStateService: DeepMocked; + let schoolYearService: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + TspSyncService, + { + provide: TspClientFactory, + useValue: createMock(), + }, + { + provide: SystemService, + useValue: createMock(), + }, + { + provide: SchoolService, + useValue: createMock(), + }, + { + provide: FederalStateService, + useValue: createMock(), + }, + { + provide: SchoolYearService, + useValue: createMock(), + }, + ], + }).compile(); + + sut = module.get(TspSyncService); + tspClientFactory = module.get(TspClientFactory); + systemService = module.get(SystemService); + schoolService = module.get(SchoolService); + federalStateService = module.get(FederalStateService); + schoolYearService = module.get(SchoolYearService); + }); + + afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + }); + + afterAll(async () => { + await module.close(); + }); + + describe('when sync service is initialized', () => { + it('should be defined', () => { + expect(sut).toBeDefined(); + }); + }); + + describe('findTspSystemOrFail', () => { + describe('when tsp system is found', () => { + const setup = () => { + const system = systemFactory.build({ + type: SystemType.OAUTH, + provisioningStrategy: SystemProvisioningStrategy.TSP, + }); + + systemService.find.mockResolvedValueOnce([system]); + }; + + it('should be returned', async () => { + setup(); + + const system = await sut.findTspSystemOrFail(); + + expect(system).toBeDefined(); + }); + }); + + describe('when tsp system is not found', () => { + const setup = () => { + systemService.find.mockResolvedValueOnce([]); + }; + + it('should throw', async () => { + setup(); + + await expect(sut.findTspSystemOrFail()).rejects.toThrow(); + }); + }); + }); + + describe('fetchTspSchools', () => { + describe('when tsp schools are fetched', () => { + const setup = () => { + const clientId = faker.string.alpha(); + const clientSecret = faker.string.alpha(); + const tokenEndpoint = faker.internet.url(); + const system = systemFactory.build({ + oauthConfig: { + clientId, + clientSecret, + tokenEndpoint, + }, + }); + + const tspSchool: RobjExportSchule = { + schuleName: faker.string.alpha(), + schuleNummer: faker.string.alpha(), + }; + const schools = [tspSchool]; + const response = createMock>>({ + data: schools, + }); + + const exportApiMock = createMock(); + exportApiMock.exportSchuleList.mockResolvedValueOnce(response); + tspClientFactory.createExportClient.mockReturnValueOnce(exportApiMock); + + return { clientId, clientSecret, tokenEndpoint, system, exportApiMock, schools }; + }; + + it('should use the oauthConfig to create the client', async () => { + const { clientId, clientSecret, tokenEndpoint, system } = setup(); + + await sut.fetchTspSchools(system); + + expect(tspClientFactory.createExportClient).toHaveBeenCalledWith({ + clientId, + clientSecret, + tokenEndpoint, + }); + }); + + it('should call exportSchuleList', async () => { + const { system, exportApiMock } = setup(); + + await sut.fetchTspSchools(system); + + expect(exportApiMock.exportSchuleList).toHaveBeenCalledTimes(1); + }); + + it('should return an array of schools', async () => { + const { system } = setup(); + + const schools = await sut.fetchTspSchools(system); + + expect(schools).toBeDefined(); + expect(schools).toBeInstanceOf(Array); + }); + }); + }); + + describe('findSchool', () => { + describe('when school is found', () => { + const setup = () => { + const externalId = faker.string.alpha(); + const system = systemFactory.build(); + const school = schoolFactory.build(); + + schoolService.getSchools.mockResolvedValueOnce([school]); + + return { externalId, system }; + }; + + it('should return the school', async () => { + const { externalId, system } = setup(); + + const result = await sut.findSchool(system, externalId); + + expect(result).toBeInstanceOf(School); + }); + }); + + describe('when school is not found', () => { + const setup = () => { + const externalId = faker.string.alpha(); + const system = systemFactory.build(); + + schoolService.getSchools.mockResolvedValueOnce([]); + + return { externalId, system }; + }; + + it('should return undefined', async () => { + const { externalId, system } = setup(); + + const result = await sut.findSchool(system, externalId); + + expect(result).toBeUndefined(); + }); + }); + }); + + describe('updateSchool', () => { + describe('when school is updated', () => { + const setup = () => { + const newName = faker.string.alpha(); + const oldName = faker.string.alpha(); + const school = schoolFactory.build({ + name: oldName, + }); + + return { newName, school }; + }; + + it('should set the new name', async () => { + const { newName, school } = setup(); + + await sut.updateSchool(school, newName); + + expect(schoolService.save).toHaveBeenCalledWith({ + props: expect.objectContaining>({ + name: newName, + }) as Partial, + }); + }); + }); + }); + + describe('createSchool', () => { + describe('when school is created', () => { + const setup = () => { + const system = systemFactory.build(); + const name = faker.string.alpha(); + const externalId = faker.string.alpha(); + + const schoolYearEntity = schoolYearFactory.build(); + const schoolYear = SchoolYearEntityMapper.mapToDo(schoolYearEntity); + schoolYearService.getCurrentSchoolYear.mockResolvedValueOnce(schoolYearEntity); + + const federalStateEntity = federalStateFactory.build(); + const federalState = FederalStateEntityMapper.mapToDo(federalStateEntity); + federalStateService.findFederalStateByName.mockResolvedValueOnce(federalStateEntity); + + schoolService.save.mockResolvedValueOnce(schoolFactory.build()); + + return { system, name, externalId, schoolYear, federalState }; + }; + + it('should be returned', async () => { + const { system, name, externalId, schoolYear, federalState } = setup(); + + const school = await sut.createSchool(system, externalId, name); + + expect(school).toBeDefined(); + expect(schoolService.save).toHaveBeenCalledWith({ + props: expect.objectContaining>({ + name, + externalId, + systemIds: [system.id], + federalState, + currentYear: schoolYear, + }) as Partial, + }); + }); + }); + + describe('when federalState is already cached', () => { + const setup = () => { + const system = systemFactory.build(); + const name = faker.string.alpha(); + const externalId = faker.string.alpha(); + + const schoolYearEntity = schoolYearFactory.build(); + const schoolYear = SchoolYearEntityMapper.mapToDo(schoolYearEntity); + schoolYearService.getCurrentSchoolYear.mockResolvedValueOnce(schoolYearEntity); + + const federalStateEntity = federalStateFactory.build(); + const federalState = FederalStateEntityMapper.mapToDo(federalStateEntity); + Reflect.set(sut, 'federalState', federalState); + + schoolService.save.mockResolvedValueOnce(schoolFactory.build()); + + return { system, name, externalId, schoolYear, federalState }; + }; + + it('should be used and not loaded again', async () => { + const { system, name, externalId, schoolYear, federalState } = setup(); + + const school = await sut.createSchool(system, externalId, name); + + expect(school).toBeDefined(); + expect(schoolService.save).toHaveBeenCalledWith({ + props: expect.objectContaining>({ + name, + externalId, + systemIds: [system.id], + federalState, + currentYear: schoolYear, + }) as Partial, + }); + expect(federalStateService.findFederalStateByName).not.toHaveBeenCalled(); + }); + }); + }); +}); 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 ba47bff9fea..7b4ed11d77e 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -75,12 +75,13 @@ export class TspSyncService { public async createSchool(system: System, identifier: string, name: string): Promise { const schoolYearEntity = await this.schoolYearService.getCurrentSchoolYear(); const schoolYear = SchoolYearEntityMapper.mapToDo(schoolYearEntity); + const federalState = await this.findFederalState(); const school = SchoolFactory.build({ externalId: identifier, name, systemIds: [system.id], - federalState: await this.findFederalState(), + federalState, currentYear: schoolYear, features: new Set([SchoolFeature.OAUTH_PROVISIONING_ENABLED]), createdAt: new Date(), 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 fce598102eb..8ae2bea850a 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 @@ -1,17 +1,55 @@ -import { TestingModule, Test } from '@nestjs/testing'; +import { faker } from '@faker-js/faker'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { RobjExportSchule } from '@infra/tsp-client'; +import { ConfigService } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { Logger } from '@src/core/logger'; +import { schoolFactory } from '@src/modules/school/testing'; import { SyncStrategyTarget } from '../sync-strategy.types'; +import { TspSyncConfig } from './tsp-sync.config'; +import { TspSyncService } from './tsp-sync.service'; import { TspSyncStrategy } from './tsp-sync.strategy'; describe(TspSyncStrategy.name, () => { let module: TestingModule; - let strategy: TspSyncStrategy; + let sut: TspSyncStrategy; + let tspSyncService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ - providers: [TspSyncStrategy], + providers: [ + TspSyncStrategy, + { + provide: TspSyncService, + useValue: createMock(), + }, + { + provide: Logger, + useValue: createMock(), + }, + { + provide: ConfigService, + useValue: createMock>({ + getOrThrow: (key: string) => { + switch (key) { + case 'TSP_SYNC_SCHOOL_LIMIT': + return 10; + default: + throw new Error(`Unknown key: ${key}`); + } + }, + }), + }, + ], }).compile(); - strategy = module.get(TspSyncStrategy); + sut = module.get(TspSyncStrategy); + tspSyncService = module.get(TspSyncService); + }); + + afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); }); afterAll(async () => { @@ -20,23 +58,80 @@ describe(TspSyncStrategy.name, () => { describe('when tsp sync strategy is initialized', () => { it('should be defined', () => { - expect(strategy).toBeDefined(); + expect(sut).toBeDefined(); }); }); describe('getType', () => { describe('when tsp sync strategy is initialized', () => { it('should return tsp', () => { - expect(strategy.getType()).toBe(SyncStrategyTarget.TSP); + expect(sut.getType()).toBe(SyncStrategyTarget.TSP); }); }); }); describe('sync', () => { - it('should return a promise', () => { - const result = strategy.sync(); + describe('when sync is called', () => { + const setup = () => { + tspSyncService.fetchTspSchools.mockResolvedValueOnce([]); + }; + + it('should find the tsp system', async () => { + await sut.sync(); + + expect(tspSyncService.findTspSystemOrFail).toHaveBeenCalled(); + }); + + it('should fetch the schools', async () => { + setup(); + + await sut.sync(); + + expect(tspSyncService.fetchTspSchools).toHaveBeenCalled(); + }); + }); + + describe('when school does not exist', () => { + const setup = () => { + const tspSchool: RobjExportSchule = { + schuleNummer: faker.string.alpha(), + schuleName: faker.string.alpha(), + }; + const tspSchools = [tspSchool]; + tspSyncService.fetchTspSchools.mockResolvedValueOnce(tspSchools); - expect(result).toBeInstanceOf(Promise); + tspSyncService.findSchool.mockResolvedValueOnce(undefined); + }; + + it('should create the school', async () => { + setup(); + + await sut.sync(); + + expect(tspSyncService.createSchool).toHaveBeenCalled(); + }); + }); + + describe('when school does exist', () => { + const setup = () => { + const tspSchool: RobjExportSchule = { + schuleNummer: faker.string.alpha(), + schuleName: faker.string.alpha(), + }; + const tspSchools = [tspSchool]; + tspSyncService.fetchTspSchools.mockResolvedValueOnce(tspSchools); + + const school = schoolFactory.build(); + tspSyncService.findSchool.mockResolvedValueOnce(school); + }; + + it('should update the school', async () => { + setup(); + + await sut.sync(); + + expect(tspSyncService.updateSchool).toHaveBeenCalled(); + }); }); }); }); From 5d1e29c65d46d5bd819b452fa69bd974d76fb4ff Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Thu, 10 Oct 2024 09:30:49 +0200 Subject: [PATCH 6/9] only import rabbitmq for tsp. --- apps/server/src/infra/sync/sync.module.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/server/src/infra/sync/sync.module.ts b/apps/server/src/infra/sync/sync.module.ts index 59ffee92b44..5295e22a922 100644 --- a/apps/server/src/infra/sync/sync.module.ts +++ b/apps/server/src/infra/sync/sync.module.ts @@ -15,11 +15,10 @@ import { SyncUc } from './uc/sync.uc'; @Module({ imports: [ - RabbitMQWrapperModule, LoggerModule, ConsoleWriterModule, ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) - ? [TspClientModule, SystemModule, SchoolModule, LegacySchoolModule] + ? [TspClientModule, SystemModule, SchoolModule, LegacySchoolModule, RabbitMQWrapperModule] : []), ], providers: [ From 8221d2a32924e365e92cca13f2a3c7417de368ed Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Fri, 11 Oct 2024 11:55:41 +0200 Subject: [PATCH 7/9] Code review comments. --- .../tsp-schools-fetched.loggable.spec.ts | 5 ++-- .../loggable/tsp-schools-fetched.loggable.ts | 5 ++-- .../tsp-schools-synced.loggable.spec.ts | 5 ++-- .../loggable/tsp-schools-synced.loggable.ts | 4 ++- .../tsp-schulnummer-missing.loggable.spec.ts | 23 ++++++++++++++ .../tsp-schulnummer-missing.loggable.ts | 11 +++++++ ...ystem-not-found.loggable-exception.spec.ts | 1 + ...tsp-system-not-found.loggable-exception.ts | 7 +++-- .../src/infra/sync/tsp/tsp-sync.config.ts | 1 + .../infra/sync/tsp/tsp-sync.service.spec.ts | 6 ++-- .../src/infra/sync/tsp/tsp-sync.service.ts | 8 ++--- .../infra/sync/tsp/tsp-sync.strategy.spec.ts | 23 ++++++++++++++ .../src/infra/sync/tsp/tsp-sync.strategy.ts | 30 ++++++++++++------- .../src/modules/server/server.config.ts | 1 + config/default.schema.json | 5 ++++ 15 files changed, 108 insertions(+), 27 deletions(-) create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts create mode 100644 apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts index af9e36d79cb..c97e2609e7c 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.spec.ts @@ -4,7 +4,7 @@ describe(TspSchoolsFetchedLoggable.name, () => { let loggable: TspSchoolsFetchedLoggable; beforeAll(() => { - loggable = new TspSchoolsFetchedLoggable(10); + loggable = new TspSchoolsFetchedLoggable(10, 5); }); describe('when loggable is initialized', () => { @@ -16,9 +16,10 @@ describe(TspSchoolsFetchedLoggable.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ - message: `Fetched 10 schools from TSP`, + message: `Fetched 10 schools for the last 5 days from TSP`, data: { tspSchoolCount: 10, + daysFetched: 5, }, }); }); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts index 7022ab06d07..62843fee0c3 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-fetched.loggable.ts @@ -1,13 +1,14 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspSchoolsFetchedLoggable implements Loggable { - constructor(private readonly tspSchoolCount: number) {} + constructor(private readonly tspSchoolCount: number, private readonly daysFetched: number) {} getLogMessage(): LogMessage { const message: LogMessage = { - message: `Fetched ${this.tspSchoolCount} schools from TSP`, + message: `Fetched ${this.tspSchoolCount} schools for the last ${this.daysFetched} days from TSP`, data: { tspSchoolCount: this.tspSchoolCount, + daysFetched: this.daysFetched, }, }; diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts index 08c85f65c6c..c74ce2ade74 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.spec.ts @@ -4,7 +4,7 @@ describe(TspSchoolsSyncedLoggable.name, () => { let loggable: TspSchoolsSyncedLoggable; beforeAll(() => { - loggable = new TspSchoolsSyncedLoggable(10, 5, 5); + loggable = new TspSchoolsSyncedLoggable(10, 10, 5, 5); }); describe('when loggable is initialized', () => { @@ -16,9 +16,10 @@ describe(TspSchoolsSyncedLoggable.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ - message: `Synced schools: Of 10 schools, 5 were created and 5 were updated`, + message: `Synced schools: Of 10 schools 10 were processed. 5 were created and 5 were updated`, data: { tspSchoolCount: 10, + processedSchools: 10, createdSchools: 5, updatedSchools: 5, }, diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts index 868348991b6..e10ef69d194 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schools-synced.loggable.ts @@ -3,15 +3,17 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspSchoolsSyncedLoggable implements Loggable { constructor( private readonly tspSchoolCount: number, + private readonly processedSchools: number, private readonly createdSchools: number, private readonly updatedSchools: number ) {} getLogMessage(): LogMessage { const message: LogMessage = { - message: `Synced schools: Of ${this.tspSchoolCount} schools, ${this.createdSchools} were created and ${this.updatedSchools} were updated`, + message: `Synced schools: Of ${this.tspSchoolCount} schools ${this.processedSchools} were processed. ${this.createdSchools} were created and ${this.updatedSchools} were updated`, data: { tspSchoolCount: this.tspSchoolCount, + processedSchools: this.processedSchools, createdSchools: this.createdSchools, updatedSchools: this.updatedSchools, }, diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts new file mode 100644 index 00000000000..99bedb630c9 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts @@ -0,0 +1,23 @@ +import { TspSchulnummerMissingLoggable } from './tsp-schulnummer-missing.loggable'; + +describe(TspSchulnummerMissingLoggable.name, () => { + let loggable: TspSchulnummerMissingLoggable; + + beforeAll(() => { + loggable = new TspSchulnummerMissingLoggable(); + }); + + 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: `A TSP school is missing a Schulnummer. Skipping school.`, + }); + }); + }); +}); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts new file mode 100644 index 00000000000..35462042a46 --- /dev/null +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts @@ -0,0 +1,11 @@ +import { Loggable, LogMessage } from '@src/core/logger'; + +export class TspSchulnummerMissingLoggable implements Loggable { + getLogMessage(): LogMessage { + const message: LogMessage = { + message: `A TSP school is missing a Schulnummer. Skipping school.`, + }; + + return message; + } +} diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts index 1e015b0698d..4b1655271b5 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.spec.ts @@ -16,6 +16,7 @@ describe(TspSystemNotFoundLoggableException.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ + message: 'The TSP system could not be found during the sync', type: 'TSP_SYSTEM_NOT_FOUND', stack: expect.any(String), }); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts index a2e15ff7db8..d7225c1cafa 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-system-not-found.loggable-exception.ts @@ -1,6 +1,6 @@ import { HttpStatus } from '@nestjs/common'; import { BusinessError, ErrorLogMessage } from '@shared/common'; -import { Loggable } from '@src/core/logger'; +import { Loggable, LogMessage } from '@src/core/logger'; export class TspSystemNotFoundLoggableException extends BusinessError implements Loggable { constructor() { @@ -14,8 +14,9 @@ export class TspSystemNotFoundLoggableException extends BusinessError implements ); } - getLogMessage(): ErrorLogMessage { - const message: ErrorLogMessage = { + getLogMessage(): LogMessage | ErrorLogMessage { + const message: LogMessage | ErrorLogMessage = { + message: this.message, type: this.type, stack: this.stack, }; diff --git a/apps/server/src/infra/sync/tsp/tsp-sync.config.ts b/apps/server/src/infra/sync/tsp/tsp-sync.config.ts index 6c0d5ceae56..9b1e8064337 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.config.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.config.ts @@ -1,3 +1,4 @@ export interface TspSyncConfig { TSP_SYNC_SCHOOL_LIMIT: number; + TSP_SYNC_SCHOOL_DAYS_TO_FETCH: number; } 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 e550ae5637f..727e3bc9b68 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 @@ -139,7 +139,7 @@ describe(TspSyncService.name, () => { it('should use the oauthConfig to create the client', async () => { const { clientId, clientSecret, tokenEndpoint, system } = setup(); - await sut.fetchTspSchools(system); + await sut.fetchTspSchools(system, 1); expect(tspClientFactory.createExportClient).toHaveBeenCalledWith({ clientId, @@ -151,7 +151,7 @@ describe(TspSyncService.name, () => { it('should call exportSchuleList', async () => { const { system, exportApiMock } = setup(); - await sut.fetchTspSchools(system); + await sut.fetchTspSchools(system, 1); expect(exportApiMock.exportSchuleList).toHaveBeenCalledTimes(1); }); @@ -159,7 +159,7 @@ describe(TspSyncService.name, () => { it('should return an array of schools', async () => { const { system } = setup(); - const schools = await sut.fetchTspSchools(system); + const schools = await sut.fetchTspSchools(system, 1); expect(schools).toBeDefined(); expect(schools).toBeInstanceOf(Array); 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 7b4ed11d77e..e9529671b49 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -39,14 +39,14 @@ export class TspSyncService { return systems[0]; } - public async fetchTspSchools(system: System) { + public async fetchTspSchools(system: System, daysToFetch: number) { const client = this.tspClientFactory.createExportClient({ clientId: system.oauthConfig?.clientId ?? '', clientSecret: system.oauthConfig?.clientSecret ?? '', tokenEndpoint: system.oauthConfig?.tokenEndpoint ?? '', }); - const lastChangeDate = this.formatChangeDate(new Date(0)); + const lastChangeDate = this.formatChangeDate(daysToFetch); const schools: RobjExportSchule[] = (await client.exportSchuleList(lastChangeDate)).data; return schools; @@ -104,7 +104,7 @@ export class TspSyncService { return this.federalState; } - private formatChangeDate(date: Date): string { - return moment(date).format('YYYY-MM-DD HH:mm:ss.SSS'); + private formatChangeDate(daysToFetch: number): string { + return moment(new Date()).subtract(daysToFetch, 'days').subtract(1, 'hours').format('YYYY-MM-DD HH:mm:ss.SSS'); } } 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 8ae2bea850a..ae1767a934c 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 @@ -34,6 +34,8 @@ describe(TspSyncStrategy.name, () => { switch (key) { case 'TSP_SYNC_SCHOOL_LIMIT': return 10; + case 'TSP_SYNC_SCHOOL_DAYS_TO_FETCH': + return 1; default: throw new Error(`Unknown key: ${key}`); } @@ -133,5 +135,26 @@ describe(TspSyncStrategy.name, () => { expect(tspSyncService.updateSchool).toHaveBeenCalled(); }); }); + + describe('when tsp school does not have a schulnummer', () => { + const setup = () => { + const tspSchool: RobjExportSchule = { + schuleNummer: undefined, + schuleName: faker.string.alpha(), + }; + const tspSchools = [tspSchool]; + tspSyncService.fetchTspSchools.mockResolvedValueOnce(tspSchools); + }; + + it('should skip the school', async () => { + setup(); + + await sut.sync(); + + expect(tspSyncService.findSchool).not.toHaveBeenCalled(); + expect(tspSyncService.updateSchool).not.toHaveBeenCalled(); + expect(tspSyncService.createSchool).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 377ea45ed4a..eac205a4a83 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts @@ -4,17 +4,20 @@ import { ConfigService } from '@nestjs/config'; import { Logger } from '@src/core/logger'; import { System } from '@src/modules/system'; import pLimit from 'p-limit'; -import { TspSyncConfig } from './tsp-sync.config'; import { SyncStrategy } from '../strategy/sync-strategy'; import { SyncStrategyTarget } from '../sync-strategy.types'; 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 { TspSyncConfig } from './tsp-sync.config'; import { TspSyncService } from './tsp-sync.service'; @Injectable() export class TspSyncStrategy extends SyncStrategy { private readonly schoolLimit: pLimit.Limit; + private readonly schoolDaysToFetch: number; + constructor( private readonly logger: Logger, private readonly tspSyncService: TspSyncService, @@ -23,6 +26,7 @@ export class TspSyncStrategy extends SyncStrategy { 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); } public override getType(): SyncStrategyTarget { @@ -36,12 +40,17 @@ export class TspSyncStrategy extends SyncStrategy { } private async syncSchools(system: System): Promise { - const tspSchools = await this.tspSyncService.fetchTspSchools(system); - this.logger.info(new TspSchoolsFetchedLoggable(tspSchools.length)); + const tspSchools = await this.tspSyncService.fetchTspSchools(system, this.schoolDaysToFetch); + this.logger.info(new TspSchoolsFetchedLoggable(tspSchools.length, this.schoolDaysToFetch)); const schoolPromises = tspSchools.map((tspSchool) => this.schoolLimit(async () => { - const existingSchool = await this.tspSyncService.findSchool(system, tspSchool.schuleNummer ?? ''); + if (!tspSchool.schuleNummer) { + this.logger.warning(new TspSchulnummerMissingLoggable()); + return null; + } + + const existingSchool = await this.tspSyncService.findSchool(system, tspSchool.schuleNummer); if (existingSchool) { const updatedSchool = await this.tspSyncService.updateSchool(existingSchool, tspSchool.schuleName ?? ''); @@ -50,7 +59,7 @@ export class TspSyncStrategy extends SyncStrategy { const createdSchool = await this.tspSyncService.createSchool( system, - tspSchool.schuleNummer ?? '', + tspSchool.schuleNummer, tspSchool.schuleName ?? '' ); return { school: createdSchool, created: true }; @@ -59,11 +68,12 @@ export class TspSyncStrategy extends SyncStrategy { const scSchools = await Promise.all(schoolPromises); - const total = scSchools.length; - const createdSchools = scSchools.filter((scSchool) => scSchool.created).length; - const updatedSchools = total - createdSchools; - this.logger.info(new TspSchoolsSyncedLoggable(total, createdSchools, updatedSchools)); + const total = tspSchools.length; + const totalProcessed = scSchools.filter((scSchool) => scSchool != null).length; + const createdSchools = scSchools.filter((scSchool) => scSchool != null && scSchool.created).length; + const updatedSchools = scSchools.filter((scSchool) => scSchool != null && !scSchool.created).length; + this.logger.info(new TspSchoolsSyncedLoggable(total, totalProcessed, createdSchools, updatedSchools)); - return scSchools.map((scSchool) => scSchool.school); + return scSchools.filter((scSchool) => scSchool != null).map((scSchool) => scSchool.school); } } diff --git a/apps/server/src/modules/server/server.config.ts b/apps/server/src/modules/server/server.config.ts index bbec4f6df39..c967ba2a5de 100644 --- a/apps/server/src/modules/server/server.config.ts +++ b/apps/server/src/modules/server/server.config.ts @@ -313,6 +313,7 @@ const config: ServerConfig = { TSP_API_BASE_URL: Configuration.get('TSP_API_BASE_URL') as string, TSP_API_TOKEN_LIFETIME_MS: Configuration.get('TSP_API_TOKEN_LIFETIME_MS') as number, TSP_SYNC_SCHOOL_LIMIT: Configuration.get('TSP_SYNC_SCHOOL_LIMIT') as number, + TSP_SYNC_SCHOOL_DAYS_TO_FETCH: Configuration.get('TSP_SYNC_SCHOOL_DAYS_TO_FETCH') as number, ROCKET_CHAT_URI: Configuration.get('ROCKET_CHAT_URI') as string, ROCKET_CHAT_ADMIN_ID: Configuration.get('ROCKET_CHAT_ADMIN_ID') as string, ROCKET_CHAT_ADMIN_TOKEN: Configuration.get('ROCKET_CHAT_ADMIN_TOKEN') as string, diff --git a/config/default.schema.json b/config/default.schema.json index 8bc787bde6b..d13131bf0e9 100644 --- a/config/default.schema.json +++ b/config/default.schema.json @@ -184,6 +184,11 @@ "default": "10", "description": "The amount of schools the sync handles at once." }, + "TSP_SYNC_SCHOOL_DAYS_TO_FETCH": { + "type": "number", + "default": "1", + "description": "The amount of days for which the sync fetches schools from the TSP." + }, "FEATURE_TSP_ENABLED": { "type": "boolean", "default": false, From c1a5ab105ec126d8afdefdd61685297453e08bf9 Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Fri, 11 Oct 2024 14:57:21 +0200 Subject: [PATCH 8/9] code review --- .../sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts | 2 +- .../infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts | 2 +- apps/server/src/infra/sync/tsp/tsp-sync.service.spec.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts index 99bedb630c9..86b69aaa55f 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts @@ -16,7 +16,7 @@ describe(TspSchulnummerMissingLoggable.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ - message: `A TSP school is missing a Schulnummer. Skipping school.`, + message: `A TSP school is missing a Schulnummer. This school is skipped.`, }); }); }); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts index 35462042a46..8641c5f2194 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts @@ -3,7 +3,7 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspSchulnummerMissingLoggable implements Loggable { getLogMessage(): LogMessage { const message: LogMessage = { - message: `A TSP school is missing a Schulnummer. Skipping school.`, + message: `A TSP school is missing a Schulnummer. This school is skipped.`, }; return message; 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 727e3bc9b68..737c81acf6c 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 @@ -98,7 +98,7 @@ describe(TspSyncService.name, () => { systemService.find.mockResolvedValueOnce([]); }; - it('should throw', async () => { + it('should throw a TspSystemNotFound exception', async () => { setup(); await expect(sut.findTspSystemOrFail()).rejects.toThrow(); From a1a059f282a8203a14ae4b1a1a8d0d0569381477 Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Mon, 14 Oct 2024 13:25:16 +0200 Subject: [PATCH 9/9] code review. --- .../tsp-schulnummer-missing.loggable.spec.ts | 7 +++++-- .../tsp-schulnummer-missing.loggable.ts | 7 ++++++- .../infra/sync/tsp/tsp-sync.service.spec.ts | 20 +++++++++++++++++++ .../src/infra/sync/tsp/tsp-sync.service.ts | 6 +++++- .../src/infra/sync/tsp/tsp-sync.strategy.ts | 2 +- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts index 86b69aaa55f..4627ff49634 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.spec.ts @@ -4,7 +4,7 @@ describe(TspSchulnummerMissingLoggable.name, () => { let loggable: TspSchulnummerMissingLoggable; beforeAll(() => { - loggable = new TspSchulnummerMissingLoggable(); + loggable = new TspSchulnummerMissingLoggable('Schule'); }); describe('when loggable is initialized', () => { @@ -16,7 +16,10 @@ describe(TspSchulnummerMissingLoggable.name, () => { describe('getLogMessage', () => { it('should return a log message', () => { expect(loggable.getLogMessage()).toEqual({ - message: `A TSP school is missing a Schulnummer. This school is skipped.`, + message: `The TSP school 'Schule' is missing a Schulnummer. This school is skipped.`, + data: { + schulName: 'Schule', + }, }); }); }); diff --git a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts index 8641c5f2194..ddd8e68f9df 100644 --- a/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts +++ b/apps/server/src/infra/sync/tsp/loggable/tsp-schulnummer-missing.loggable.ts @@ -1,9 +1,14 @@ import { Loggable, LogMessage } from '@src/core/logger'; export class TspSchulnummerMissingLoggable implements Loggable { + constructor(private readonly schulName?: string) {} + getLogMessage(): LogMessage { const message: LogMessage = { - message: `A TSP school is missing a Schulnummer. This school is skipped.`, + message: `The TSP school '${this.schulName ?? ''}' is missing a Schulnummer. This school is skipped.`, + data: { + schulName: this.schulName, + }, }; return message; 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 737c81acf6c..486d4c2bb57 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 @@ -232,6 +232,26 @@ describe(TspSyncService.name, () => { }); }); }); + + describe('when school name is undefined', () => { + const setup = () => { + const newName = undefined; + const oldName = faker.string.alpha(); + const school = schoolFactory.build({ + name: oldName, + }); + + return { newName, school }; + }; + + it('should not update school', async () => { + const { newName, school } = setup(); + + await sut.updateSchool(school, newName); + + expect(schoolService.save).not.toHaveBeenCalled(); + }); + }); }); describe('createSchool', () => { 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 e9529671b49..00ff1d2429e 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.service.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.service.ts @@ -64,7 +64,11 @@ export class TspSyncService { return schools[0]; } - public async updateSchool(school: School, name: string): Promise { + public async updateSchool(school: School, name?: string): Promise { + if (!name) { + return school; + } + school.name = name; const updatedSchool = await this.schoolService.save(school); 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 eac205a4a83..c0cb551c3e3 100644 --- a/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/tsp/tsp-sync.strategy.ts @@ -53,7 +53,7 @@ export class TspSyncStrategy extends SyncStrategy { const existingSchool = await this.tspSyncService.findSchool(system, tspSchool.schuleNummer); if (existingSchool) { - const updatedSchool = await this.tspSyncService.updateSchool(existingSchool, tspSchool.schuleName ?? ''); + const updatedSchool = await this.tspSyncService.updateSchool(existingSchool, tspSchool.schuleName); return { school: updatedSchool, created: false }; }