diff --git a/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-cronjob-configmap.yml.j2 b/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-cronjob-configmap.yml.j2 index 73c1790ca37..0ef22e06573 100644 --- a/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-cronjob-configmap.yml.j2 +++ b/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-cronjob-configmap.yml.j2 @@ -23,5 +23,3 @@ data: TSP_SYNC_DATA_LIMIT: "{{ TSP_SYNC_DATA_LIMIT }}" TSP_SYNC_SCHOOL_DAYS_TO_FETCH: "-1" TSP_SYNC_DATA_DAYS_TO_FETCH: "-1" - FEATURE_TSP_MIGRATION_ENABLED: "{{ FEATURE_TSP_MIGRATION_ENABLED }}" - TSP_SYNC_MIGRATION_LIMIT: "{{ TSP_SYNC_MIGRATION_LIMIT }}" diff --git a/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-init-configmap.yml.j2 b/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-init-configmap.yml.j2 index e32a6f2b1d7..984698440e7 100644 --- a/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-init-configmap.yml.j2 +++ b/ansible/roles/schulcloud-server-tspsync/templates/api-tsp-sync-init-configmap.yml.j2 @@ -23,5 +23,3 @@ data: TSP_SYNC_DATA_LIMIT: "{{ TSP_SYNC_DATA_LIMIT }}" TSP_SYNC_SCHOOL_DAYS_TO_FETCH: "-1" TSP_SYNC_DATA_DAYS_TO_FETCH: "-1" - FEATURE_TSP_MIGRATION_ENABLED: "{{ FEATURE_TSP_MIGRATION_ENABLED }}" - TSP_SYNC_MIGRATION_LIMIT: "{{ TSP_SYNC_MIGRATION_LIMIT }}" diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/index.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/index.ts index e4b36f662f5..1da5e907eb1 100644 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/index.ts +++ b/apps/server/src/infra/sync/strategy/tsp/loggable/index.ts @@ -1,7 +1,4 @@ export { TspDataFetchedLoggable } from './tsp-data-fetched.loggable'; -export { TspLegacyMigrationStartLoggable } from './tsp-legacy-migration-start.loggable'; -export { TspLegacySchoolMigrationCountLoggable } from './tsp-legacy-school-migration-count.loggable'; -export { TspLegacySchoolMigrationSuccessLoggable } from './tsp-legacy-school-migration-success.loggable'; export { TspMissingExternalIdLoggable } from './tsp-missing-external-id.loggable'; export { TspSchoolsFetchedLoggable } from './tsp-schools-fetched.loggable'; export { TspSchoolsSyncedLoggable } from './tsp-schools-synced.loggable'; @@ -9,7 +6,6 @@ export { TspSchulnummerMissingLoggable } from './tsp-schulnummer-missing.loggabl export { TspSyncedUsersLoggable } from './tsp-synced-users.loggable'; export { TspSyncingUsersLoggable } from './tsp-syncing-users.loggable'; export { TspSystemNotFoundLoggableException } from './tsp-system-not-found.loggable-exception'; -export { TspUsersMigratedLoggable } from './tsp-users-migrated.loggable'; export { TspClassSyncSummaryLoggable } from './tsp-class-sync-summary.loggable'; export { TspClassSyncStartLoggable } from './tsp-class-sync-start.loggable'; export { TspClassSyncBatchLoggable } from './tsp-class-sync-batch.loggable'; diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-start.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-start.loggable.spec.ts deleted file mode 100644 index a8b309293a2..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-start.loggable.spec.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { TspLegacyMigrationStartLoggable } from './tsp-legacy-migration-start.loggable'; - -describe(TspLegacyMigrationStartLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const expected = { - message: 'Running migration of legacy tsp data.', - }; - - return { expected }; - }; - - it('should return a log message', () => { - const { expected } = setup(); - - const loggable = new TspLegacyMigrationStartLoggable(); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-start.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-start.loggable.ts deleted file mode 100644 index 6df32ccd8b8..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-start.loggable.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Loggable, LogMessage } from '@core/logger'; - -export class TspLegacyMigrationStartLoggable implements Loggable { - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: 'Running migration of legacy tsp data.', - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-system-missing.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-system-missing.loggable.spec.ts deleted file mode 100644 index bd450a4ac91..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-system-missing.loggable.spec.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { TspLegacyMigrationSystemMissingLoggable } from './tsp-legacy-migration-system-missing.loggable'; - -describe(TspLegacyMigrationSystemMissingLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const expected = { - message: 'No legacy system found', - }; - - return { expected }; - }; - - it('should return a log message', () => { - const { expected } = setup(); - - const loggable = new TspLegacyMigrationSystemMissingLoggable(); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts deleted file mode 100644 index 9d7dcca7115..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-migration-system-missing.loggable.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Loggable, LogMessage } from '@core/logger'; - -export class TspLegacyMigrationSystemMissingLoggable implements Loggable { - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: 'No legacy system found', - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-count.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-count.loggable.spec.ts deleted file mode 100644 index 9d02234799f..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-count.loggable.spec.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { faker } from '@faker-js/faker'; -import { TspLegacySchoolMigrationCountLoggable } from './tsp-legacy-school-migration-count.loggable'; - -describe(TspLegacySchoolMigrationCountLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const total = faker.number.int(); - - const expected = { - message: `Found ${total} legacy tsp schools to migrate`, - data: { - total, - }, - }; - - return { total, expected }; - }; - - it('should return a log message', () => { - const { total, expected } = setup(); - - const loggable = new TspLegacySchoolMigrationCountLoggable(total); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts deleted file mode 100644 index a68ce81733f..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-count.loggable.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Loggable, LogMessage } from '@core/logger'; - -export class TspLegacySchoolMigrationCountLoggable implements Loggable { - constructor(private readonly total: number) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: `Found ${this.total} legacy tsp schools to migrate`, - data: { - total: this.total, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-success.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-success.loggable.spec.ts deleted file mode 100644 index 5bb1fa5383c..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-success.loggable.spec.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { faker } from '@faker-js/faker'; -import { TspLegacySchoolMigrationSuccessLoggable } from './tsp-legacy-school-migration-success.loggable'; - -describe(TspLegacySchoolMigrationSuccessLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const total = faker.number.int(); - const migrated = faker.number.int(); - - const expected = { - message: `Legacy tsp data migration finished. Total schools: ${total}, migrated schools: ${migrated}`, - data: { - total, - migrated, - }, - }; - - return { total, migrated, expected }; - }; - - it('should return a log message', () => { - const { total, migrated, expected } = setup(); - - const loggable = new TspLegacySchoolMigrationSuccessLoggable(total, migrated); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts deleted file mode 100644 index 3e8ce6c2bd8..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-legacy-school-migration-success.loggable.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Loggable, LogMessage } from '@core/logger'; - -export class TspLegacySchoolMigrationSuccessLoggable implements Loggable { - constructor(private readonly total: number, private readonly migrated: number) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - message: `Legacy tsp data migration finished. Total schools: ${this.total}, migrated schools: ${this.migrated}`, - data: { - total: this.total, - migrated: this.migrated, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migration-batch-summary.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migration-batch-summary.loggable.spec.ts deleted file mode 100644 index 0efda5bc894..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migration-batch-summary.loggable.spec.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { faker } from '@faker-js/faker'; -import { TspMigrationBatchSummaryLoggable } from './tsp-migration-batch-summary.loggable'; - -describe(TspMigrationBatchSummaryLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const batchSize = faker.number.int(); - const usersUpdated = faker.number.int(); - const accountsUpdated = faker.number.int(); - const totalDone = faker.number.int(); - const totalMigrations = faker.number.int(); - - const expected = { - message: `Migrated ${usersUpdated} users and ${accountsUpdated} accounts in batch of size ${batchSize} (total done: ${totalDone}, total migrations: ${totalMigrations})`, - data: { - batchSize, - usersUpdated, - accountsUpdated, - totalDone, - totalMigrations, - }, - }; - - return { - batchSize, - usersUpdated, - accountsUpdated, - totalDone, - totalMigrations, - expected, - }; - }; - - it('should return a log message', () => { - const { batchSize, usersUpdated, accountsUpdated, totalDone, totalMigrations, expected } = setup(); - - const loggable = new TspMigrationBatchSummaryLoggable( - batchSize, - usersUpdated, - accountsUpdated, - totalDone, - totalMigrations - ); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migration-batch-summary.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migration-batch-summary.loggable.ts deleted file mode 100644 index 74da5a5a68e..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migration-batch-summary.loggable.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { Loggable, LogMessage } from '@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/strategy/tsp/loggable/tsp-migrations-fetched.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migrations-fetched.loggable.spec.ts deleted file mode 100644 index eacfcf022c6..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migrations-fetched.loggable.spec.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { faker } from '@faker-js/faker'; -import { TspMigrationsFetchedLoggable } from './tsp-migrations-fetched.loggable'; - -describe(TspMigrationsFetchedLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const tspUserMigrationCount = faker.number.int(); - - const expected = { - message: `Fetched ${tspUserMigrationCount} users for migration from TSP`, - data: { - tspUserMigrationCount, - }, - }; - - return { tspUserMigrationCount, expected }; - }; - - it('should return a log message', () => { - const { tspUserMigrationCount, expected } = setup(); - - const loggable = new TspMigrationsFetchedLoggable(tspUserMigrationCount); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migrations-fetched.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migrations-fetched.loggable.ts deleted file mode 100644 index 5432c904a30..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-migrations-fetched.loggable.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Loggable, LogMessage } from '@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/strategy/tsp/loggable/tsp-users-migrated.loggable.spec.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-users-migrated.loggable.spec.ts deleted file mode 100644 index 9fa93d6f276..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-users-migrated.loggable.spec.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { faker } from '@faker-js/faker'; -import { TspUsersMigratedLoggable } from './tsp-users-migrated.loggable'; - -describe(TspUsersMigratedLoggable.name, () => { - describe('getLogMessage is called', () => { - const setup = () => { - const totalMigrations = faker.number.int(); - const migratedUsers = faker.number.int(); - const migratedAccounts = faker.number.int(); - - const expected = { - message: `Migrated ${migratedUsers} users and ${migratedAccounts} accounts. Total amount of migrations requested: ${totalMigrations}`, - data: { - totalMigrations, - migratedUsers, - migratedAccounts, - }, - }; - - return { totalMigrations, migratedUsers, migratedAccounts, expected }; - }; - - it('should return a log message', () => { - const { totalMigrations, migratedUsers, migratedAccounts, expected } = setup(); - - const loggable = new TspUsersMigratedLoggable(totalMigrations, migratedUsers, migratedAccounts); - - expect(loggable.getLogMessage()).toEqual(expected); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-users-migrated.loggable.ts b/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-users-migrated.loggable.ts deleted file mode 100644 index 428da0118fd..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/loggable/tsp-users-migrated.loggable.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Loggable, LogMessage } from '@core/logger'; - -export class TspUsersMigratedLoggable implements Loggable { - constructor( - private readonly totalMigrations: number, - private readonly migratedUsers: number, - private readonly migratedAccounts: number - ) {} - - public getLogMessage(): LogMessage { - const message: LogMessage = { - 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, - }, - }; - - return message; - } -} diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.spec.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.spec.ts index f60f0097b7d..a9686371320 100644 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.spec.ts +++ b/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.spec.ts @@ -6,18 +6,14 @@ import { ExportApiInterface, RobjExportKlasse, RobjExportLehrer, - RobjExportLehrerMigration, RobjExportSchueler, - RobjExportSchuelerMigration, RobjExportSchule, TspClientFactory, } from '@infra/tsp-client'; import { robjExportKlasseFactory, robjExportLehrerFactory, - robjExportLehrerMigrationFactory, robjExportSchuelerFactory, - robjExportSchuelerMigrationFactory, robjExportSchuleFactory, } from '@infra/tsp-client/testing'; import { OauthConfigMissingLoggableException } from '@modules/oauth/loggable'; @@ -103,25 +99,11 @@ describe(TspFetchService.name, () => { data: classes, }); - const tspTeacherMigration = robjExportLehrerMigrationFactory.build(); - const teacherMigrations = [tspTeacherMigration]; - const responseTeacherMigrations = createMock>>({ - data: teacherMigrations, - }); - - const tspStudentMigration = robjExportSchuelerMigrationFactory.build(); - const studentMigrations = [tspStudentMigration]; - const responseStudentMigrations = createMock>>({ - data: studentMigrations, - }); - const exportApiMock = createMock(); exportApiMock.exportSchuleList.mockResolvedValueOnce(responseSchools); exportApiMock.exportLehrerList.mockResolvedValueOnce(responseTeachers); exportApiMock.exportSchuelerList.mockResolvedValueOnce(responseStudents); exportApiMock.exportKlasseList.mockResolvedValueOnce(responseClasses); - exportApiMock.exportLehrerListMigration.mockResolvedValueOnce(responseTeacherMigrations); - exportApiMock.exportSchuelerListMigration.mockResolvedValueOnce(responseStudentMigrations); tspClientFactory.createExportClient.mockReturnValueOnce(exportApiMock); @@ -135,8 +117,6 @@ describe(TspFetchService.name, () => { teachers, students, classes, - teacherMigrations, - studentMigrations, }; }; @@ -272,72 +252,6 @@ describe(TspFetchService.name, () => { }); }); - describe('fetchTspTeacherMigrations', () => { - describe('when tsp teacher migrations are fetched', () => { - it('should create the client', async () => { - const { clientId, clientSecret, tokenEndpoint, system } = setupTspClient(); - - await sut.fetchTspTeacherMigrations(system); - - expect(tspClientFactory.createExportClient).toHaveBeenCalledWith({ - clientId, - clientSecret, - tokenEndpoint, - }); - }); - - it('should call exportLehrerListMigration', async () => { - const { system, exportApiMock } = setupTspClient(); - - await sut.fetchTspTeacherMigrations(system); - - expect(exportApiMock.exportLehrerListMigration).toHaveBeenCalledTimes(1); - }); - - it('should return an array of teacher migrations', async () => { - const { system } = setupTspClient(); - - const result = await sut.fetchTspTeacherMigrations(system); - - expect(result).toBeDefined(); - expect(result).toBeInstanceOf(Array); - }); - }); - }); - - describe('fetchTspStudentMigrations', () => { - describe('when tsp student migrations are fetched', () => { - it('should create the client', async () => { - const { clientId, clientSecret, tokenEndpoint, system } = setupTspClient(); - - await sut.fetchTspStudentMigrations(system); - - expect(tspClientFactory.createExportClient).toHaveBeenCalledWith({ - clientId, - clientSecret, - tokenEndpoint, - }); - }); - - it('should call exportSchuelerListMigration', async () => { - const { system, exportApiMock } = setupTspClient(); - - await sut.fetchTspStudentMigrations(system); - - expect(exportApiMock.exportSchuelerListMigration).toHaveBeenCalledTimes(1); - }); - - it('should return an array of student migrations', async () => { - const { system } = setupTspClient(); - - const result = await sut.fetchTspStudentMigrations(system); - - expect(result).toBeDefined(); - expect(result).toBeInstanceOf(Array); - }); - }); - }); - describe('fetchTsp', () => { describe('when AxiosError is thrown', () => { const setup = () => { diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.ts index d57f36760a0..402ac954383 100644 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.ts +++ b/apps/server/src/infra/sync/strategy/tsp/tsp-fetch.service.ts @@ -4,9 +4,7 @@ import { ExportApiInterface, RobjExportKlasse, RobjExportLehrer, - RobjExportLehrerMigration, RobjExportSchueler, - RobjExportSchuelerMigration, RobjExportSchule, TspClientFactory, } from '@infra/tsp-client'; @@ -51,18 +49,6 @@ export class TspFetchService { return classes; } - public fetchTspTeacherMigrations(system: System): Promise { - const migrations = this.fetchTsp(system, (client) => client.exportLehrerListMigration(), []); - - return migrations; - } - - public fetchTspStudentMigrations(system: System): Promise { - const migrations = this.fetchTsp(system, (client) => client.exportSchuelerListMigration(), []); - - return migrations; - } - private async fetchTsp( system: System, fetchFunction: (client: ExportApiInterface) => Promise>, diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-legacy-migration.service.integration.spec.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-legacy-migration.service.integration.spec.ts deleted file mode 100644 index 1154249d031..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-legacy-migration.service.integration.spec.ts +++ /dev/null @@ -1,108 +0,0 @@ -import { Logger } from '@core/logger'; -import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { EntityManager } from '@mikro-orm/mongodb'; -import { SchoolFeature } from '@modules/school/domain'; -import { SchoolEntity } from '@modules/school/repo'; -import { schoolEntityFactory } from '@modules/school/testing'; -import { SystemType } from '@modules/system'; -import { systemEntityFactory } from '@modules/system/testing'; -import { Test, TestingModule } from '@nestjs/testing'; -import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; -import { cleanupCollections } from '@testing/cleanup-collections'; -import { MongoMemoryDatabaseModule } from '@testing/database'; -import { TspLegacyMigrationSystemMissingLoggable } from './loggable/tsp-legacy-migration-system-missing.loggable'; -import { TspLegacyMigrationService } from './tsp-legacy-migration.service'; - -describe(TspLegacyMigrationService.name, () => { - let module: TestingModule; - let em: EntityManager; - let sut: TspLegacyMigrationService; - let logger: DeepMocked; - - beforeAll(async () => { - module = await Test.createTestingModule({ - imports: [MongoMemoryDatabaseModule.forRoot({ entities: [SchoolEntity] })], - providers: [ - TspLegacyMigrationService, - { - provide: Logger, - useValue: createMock(), - }, - ], - }).compile(); - sut = module.get(TspLegacyMigrationService); - em = module.get(EntityManager); - logger = module.get(Logger); - }); - - afterAll(async () => { - await module.close(); - }); - - afterEach(async () => { - jest.resetAllMocks(); - jest.clearAllMocks(); - jest.restoreAllMocks(); - await cleanupCollections(em); - }); - - describe('prepareLegacySyncDataForNewSync', () => { - describe('when legacy system is not found', () => { - it('should log TspLegacyMigrationSystemMissingLoggable', async () => { - await sut.prepareLegacySyncDataForNewSync(''); - - expect(logger.info).toHaveBeenCalledWith(new TspLegacyMigrationSystemMissingLoggable()); - }); - }); - - describe('when migrating legacy data', () => { - const setup = async () => { - const legacySystem = systemEntityFactory.buildWithId({ - type: 'tsp-school', - }); - const newSystem = systemEntityFactory.buildWithId({ - type: SystemType.OAUTH, - provisioningStrategy: SystemProvisioningStrategy.TSP, - }); - - const schoolIdentifier = '123'; - const legacySchool = schoolEntityFactory.buildWithId({ - systems: [legacySystem], - features: [], - }); - - await em.persistAndFlush([legacySystem, newSystem, legacySchool]); - em.clear(); - - await em.getCollection('schools').findOneAndUpdate( - { - systems: [legacySystem._id], - }, - { - $set: { - sourceOptions: { - schoolIdentifier, - }, - source: 'tsp', - }, - } - ); - - return { legacySystem, newSystem, legacySchool, schoolId: schoolIdentifier }; - }; - - it('should update the school to the new format', async () => { - const { newSystem, legacySchool, schoolId: schoolIdentifier } = await setup(); - - await sut.prepareLegacySyncDataForNewSync(newSystem.id); - - const migratedSchool = await em.findOne(SchoolEntity.name, { - id: legacySchool.id, - }); - expect(migratedSchool?.externalId).toBe(schoolIdentifier); - expect(migratedSchool?.systems[0].id).toBe(newSystem.id); - expect(migratedSchool?.features).toContain(SchoolFeature.OAUTH_PROVISIONING_ENABLED); - }); - }); - }); -}); diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-legacy-migration.service.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-legacy-migration.service.ts deleted file mode 100644 index bf3d071e1fd..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-legacy-migration.service.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { Logger } from '@core/logger'; -import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; -import { SchoolFeature } from '@modules/school/domain'; -import { Injectable } from '@nestjs/common'; -import { EntityId } from '@shared/domain/types'; -import { TspLegacyMigrationStartLoggable } from './loggable/tsp-legacy-migration-start.loggable'; -import { TspLegacyMigrationSystemMissingLoggable } from './loggable/tsp-legacy-migration-system-missing.loggable'; -import { TspLegacySchoolMigrationCountLoggable } from './loggable/tsp-legacy-school-migration-count.loggable'; -import { TspLegacySchoolMigrationSuccessLoggable } from './loggable/tsp-legacy-school-migration-success.loggable'; - -type LegacyTspSchoolProperties = { - sourceOptions: { - schoolIdentifier: number; - }; -}; - -const TSP_LEGACY_SYSTEM_TYPE = 'tsp-school'; -const TSP_LEGACY_SOURCE_TYPE = 'tsp'; -const SCHOOLS_COLLECTION = 'schools'; -const SYSTEMS_COLLECTION = 'systems'; - -@Injectable() -export class TspLegacyMigrationService { - constructor(private readonly em: EntityManager, private readonly logger: Logger) { - logger.setContext(TspLegacyMigrationService.name); - } - - public async prepareLegacySyncDataForNewSync(newSystemId: EntityId): Promise { - this.logger.info(new TspLegacyMigrationStartLoggable()); - - const legacySystemId = await this.findLegacySystemId(); - - if (!legacySystemId) { - this.logger.info(new TspLegacyMigrationSystemMissingLoggable()); - return; - } - - const schoolIds = await this.findIdsOfLegacyTspSchools(legacySystemId); - - this.logger.info(new TspLegacySchoolMigrationCountLoggable(schoolIds.length)); - - const promises = schoolIds.map(async (oldId): Promise => { - const legacySchoolFilter = { - systems: [legacySystemId], - source: TSP_LEGACY_SOURCE_TYPE, - sourceOptions: { - schoolIdentifier: oldId, - }, - }; - - const featureUpdateCount = await this.em.nativeUpdate(SCHOOLS_COLLECTION, legacySchoolFilter, { - $addToSet: { - features: SchoolFeature.OAUTH_PROVISIONING_ENABLED, - }, - }); - const idUpdateCount = await this.em.nativeUpdate(SCHOOLS_COLLECTION, legacySchoolFilter, { - ldapSchoolIdentifier: oldId, - systems: [new ObjectId(newSystemId)], - }); - - return featureUpdateCount === 1 && idUpdateCount === 1 ? 1 : 0; - }); - - const results = await Promise.allSettled(promises); - const successfulMigrations = results - .filter((r) => r.status === 'fulfilled') - .map((r) => r.value) - .reduce((previousValue, currentValue) => previousValue + currentValue, 0); - - this.logger.info(new TspLegacySchoolMigrationSuccessLoggable(schoolIds.length, successfulMigrations)); - } - - private async findLegacySystemId(): Promise { - const tspLegacySystem = await this.em.getCollection(SYSTEMS_COLLECTION).findOne({ - type: TSP_LEGACY_SYSTEM_TYPE, - }); - - return tspLegacySystem?._id; - } - - private async findIdsOfLegacyTspSchools(legacySystemId: ObjectId): Promise { - const schools = await this.em - .getCollection(SCHOOLS_COLLECTION) - .find({ - systems: [legacySystemId], - source: TSP_LEGACY_SOURCE_TYPE, - }) - .toArray(); - - const schoolIds = schools.map((school) => school.sourceOptions.schoolIdentifier); - - return schoolIds; - } -} diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-sync-migration.service.spec.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-sync-migration.service.spec.ts deleted file mode 100644 index df8be66451b..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-sync-migration.service.spec.ts +++ /dev/null @@ -1,140 +0,0 @@ -import { Logger } from '@core/logger'; -import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { AccountService } from '@modules/account'; -import { accountDoFactory } from '@modules/account/testing'; -import { systemFactory } from '@modules/system/testing'; -import { UserService, UserSourceOptions } from '@modules/user'; -import { userDoFactory } from '@modules/user/testing'; -import { ConfigService } from '@nestjs/config'; -import { Test, TestingModule } from '@nestjs/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/strategy/tsp/tsp-sync-migration.service.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-sync-migration.service.ts deleted file mode 100644 index 5f6046fc51d..00000000000 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-sync-migration.service.ts +++ /dev/null @@ -1,149 +0,0 @@ -import { Logger } from '@core/logger'; -import { Account, AccountService } from '@modules/account'; -import { BadDataLoggableException } from '@modules/provisioning/loggable'; -import { System } from '@modules/system'; -import { UserDo, UserService, UserSourceOptions } from '@modules/user'; -import { Injectable } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; -import { TspMigrationBatchSummaryLoggable } from './loggable/tsp-migration-batch-summary.loggable'; -import { TspMigrationsFetchedLoggable } from './loggable/tsp-migrations-fetched.loggable'; -import { TspSyncConfig } from './tsp-sync.config'; - -@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) { - this.logger.warning( - new BadDataLoggableException(`Can't migrate TSP User. Old tspUid is not set for TSP User: ${user.id ?? ''}`, { - userId: user.id, - }) - ); - return; - } - - const newUid = oldToNewMappings.get(oldId); - - if (!newUid) { - this.logger.warning( - new BadDataLoggableException(`Can't migrate TSP User. No new Uid is given for oldId: ${oldId}`, { - oldId, - }) - ); - 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', { infer: true }); - - 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/strategy/tsp/tsp-sync.config.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-sync.config.ts index 5cf20ec8376..8723ac3c858 100644 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-sync.config.ts +++ b/apps/server/src/infra/sync/strategy/tsp/tsp-sync.config.ts @@ -5,6 +5,4 @@ export interface TspSyncConfig extends SyncConfig { TSP_SYNC_SCHOOL_DAYS_TO_FETCH: number; TSP_SYNC_DATA_LIMIT: number; TSP_SYNC_DATA_DAYS_TO_FETCH: number; - TSP_SYNC_MIGRATION_LIMIT: number; - FEATURE_TSP_MIGRATION_ENABLED: boolean; } diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.spec.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.spec.ts index 544881ed63a..59191cdb763 100644 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.spec.ts +++ b/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.spec.ts @@ -1,20 +1,11 @@ import { Logger } from '@core/logger'; import { faker } from '@faker-js/faker'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { - RobjExportKlasse, - RobjExportLehrer, - RobjExportLehrerMigration, - RobjExportSchueler, - RobjExportSchuelerMigration, - RobjExportSchule, -} from '@infra/tsp-client'; +import { RobjExportKlasse, RobjExportLehrer, RobjExportSchueler, RobjExportSchule } from '@infra/tsp-client'; import { robjExportKlasseFactory, robjExportLehrerFactory, - robjExportLehrerMigrationFactory, robjExportSchuelerFactory, - robjExportSchuelerMigrationFactory, robjExportSchuleFactory, } from '@infra/tsp-client/testing'; import { Account } from '@modules/account'; @@ -39,10 +30,8 @@ import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; import { SyncStrategyTarget } from '../../sync-strategy.types'; import { TspFetchService } from './tsp-fetch.service'; -import { TspLegacyMigrationService } from './tsp-legacy-migration.service'; import { TspOauthDataMapper, TspUserInfo } from './tsp-oauth-data.mapper'; import { TspSchoolService } from './tsp-school.service'; -import { TspSyncMigrationService } from './tsp-sync-migration.service'; import { TspSyncConfig } from './tsp-sync.config'; import { TspSyncStrategy } from './tsp-sync.strategy'; @@ -53,8 +42,6 @@ describe(TspSyncStrategy.name, () => { let tspFetchService: DeepMocked; let provisioningService: DeepMocked; let tspOauthDataMapper: DeepMocked; - let tspLegacyMigrationService: DeepMocked; - let tspSyncMigrationService: DeepMocked; let configService: DeepMocked>; let systemService: DeepMocked; @@ -86,14 +73,6 @@ describe(TspSyncStrategy.name, () => { provide: TspOauthDataMapper, useValue: createMock(), }, - { - provide: TspLegacyMigrationService, - useValue: createMock(), - }, - { - provide: TspSyncMigrationService, - useValue: createMock(), - }, { provide: SystemService, useValue: createMock(), @@ -106,8 +85,6 @@ describe(TspSyncStrategy.name, () => { tspFetchService = module.get(TspFetchService); provisioningService = module.get(TspProvisioningService); tspOauthDataMapper = module.get(TspOauthDataMapper); - tspLegacyMigrationService = module.get(TspLegacyMigrationService); - tspSyncMigrationService = module.get(TspSyncMigrationService); configService = module.get(ConfigService); systemService = module.get(SystemService); }); @@ -139,8 +116,6 @@ describe(TspSyncStrategy.name, () => { fetchedClasses?: RobjExportKlasse[]; fetchedTeachers?: RobjExportLehrer[]; fetchedStudents?: RobjExportSchueler[]; - fetchedTeacherMigrations?: RobjExportLehrerMigration[]; - fetchedStudentMigrations?: RobjExportSchuelerMigration[]; foundSchool?: School; foundSystemSchools?: School[]; foundTspUidUser?: UserDo | null; @@ -150,11 +125,6 @@ describe(TspSyncStrategy.name, () => { updatedAccount?: Account; updatedUser?: UserDo; configValues?: unknown[]; - migrationResult?: { - totalAmount: number; - totalUsers: number; - totalAccounts: number; - }; processBatchSize?: number; classBatchResult?: { classCreationCount: number; classUpdateCount: number }; }) => { @@ -162,8 +132,6 @@ describe(TspSyncStrategy.name, () => { tspFetchService.fetchTspClasses.mockResolvedValueOnce(params.fetchedClasses ?? []); tspFetchService.fetchTspStudents.mockResolvedValueOnce(params.fetchedStudents ?? []); tspFetchService.fetchTspTeachers.mockResolvedValueOnce(params.fetchedTeachers ?? []); - tspFetchService.fetchTspTeacherMigrations.mockResolvedValueOnce(params.fetchedTeacherMigrations ?? []); - tspFetchService.fetchTspStudentMigrations.mockResolvedValueOnce(params.fetchedStudentMigrations ?? []); tspSyncService.findSchool.mockResolvedValueOnce(params.foundSchool ?? undefined); tspSyncService.findAllSchoolsForSystem.mockResolvedValueOnce(params.foundSystemSchools ?? []); @@ -178,14 +146,6 @@ describe(TspSyncStrategy.name, () => { 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(), - } - ); - provisioningService.provisionUserBatch.mockResolvedValueOnce(params.processBatchSize ?? 0); provisioningService.provisionClassBatch.mockResolvedValueOnce( params.classBatchResult ?? { classCreationCount: 0, classUpdateCount: 0 } @@ -215,10 +175,6 @@ describe(TspSyncStrategy.name, () => { const oauthDataDto = oauthDataDtoFactory.build({ system: systemDto, externalUser, externalSchool }); - const tspTeacherMigration = robjExportLehrerMigrationFactory.build(); - - const tspStudentMigration = robjExportSchuelerMigrationFactory.build(); - const tspSchool = robjExportSchuleFactory.build({ schuleNummer: externalSchool.externalId, }); @@ -238,15 +194,13 @@ describe(TspSyncStrategy.name, () => { fetchedClasses: [tspClasses], fetchedTeachers: [tspTeachers], fetchedStudents: [tspStudents], - fetchedStudentMigrations: [tspStudentMigration], - fetchedTeacherMigrations: [tspTeacherMigration], mappedOauthDto: { oauthDataDtos: [oauthDataDto], usersOfClasses: new Map(), }, foundSystemSchools: [school], foundSystem: system, - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); return { @@ -256,8 +210,6 @@ describe(TspSyncStrategy.name, () => { tspTeachers, tspStudents, tspClasses, - tspStudentMigration, - tspTeacherMigration, }; }; @@ -269,17 +221,6 @@ describe(TspSyncStrategy.name, () => { expect(systemService.find).toHaveBeenCalledTimes(1); }); - it('should migrate the legacy data', async () => { - const { oauthDataDto } = setup(); - - await sut.sync(); - - expect(tspLegacyMigrationService.prepareLegacySyncDataForNewSync).toHaveBeenCalledTimes(1); - expect(tspLegacyMigrationService.prepareLegacySyncDataForNewSync).toHaveBeenCalledWith( - oauthDataDto.system.systemId - ); - }); - it('should fetch the schools', async () => { const { system } = setup(); @@ -295,13 +236,13 @@ describe(TspSyncStrategy.name, () => { await sut.sync(); expect(tspFetchService.fetchTspTeachers).toHaveBeenCalledTimes(1); - expect(tspFetchService.fetchTspTeachers).toHaveBeenCalledWith(system, 10); + expect(tspFetchService.fetchTspTeachers).toHaveBeenCalledWith(system, 15); expect(tspFetchService.fetchTspStudents).toHaveBeenCalledTimes(1); - expect(tspFetchService.fetchTspStudents).toHaveBeenCalledWith(system, 10); + expect(tspFetchService.fetchTspStudents).toHaveBeenCalledWith(system, 15); expect(tspFetchService.fetchTspClasses).toHaveBeenCalledTimes(1); - expect(tspFetchService.fetchTspClasses).toHaveBeenCalledWith(system, 10); + expect(tspFetchService.fetchTspClasses).toHaveBeenCalledWith(system, 15); }); it('should load all schools', async () => { @@ -346,41 +287,6 @@ describe(TspSyncStrategy.name, () => { expect(provisioningService.provisionClassBatch).toHaveBeenCalledTimes(1); }); - - describe('when feature tsp migration is enabled', () => { - it('should fetch teacher migrations', async () => { - const { system } = setup(); - - await sut.sync(); - - expect(tspFetchService.fetchTspTeacherMigrations).toHaveBeenCalledTimes(1); - expect(tspFetchService.fetchTspTeacherMigrations).toHaveBeenCalledWith(system); - }); - - it('should fetch student migrations', async () => { - const { system } = setup(); - - await sut.sync(); - - expect(tspFetchService.fetchTspStudentMigrations).toHaveBeenCalledTimes(1); - expect(tspFetchService.fetchTspStudentMigrations).toHaveBeenCalledWith(system); - }); - - it('should call tspSyncMigrationService', async () => { - const { system, tspStudentMigration, tspTeacherMigration } = setup(); - - await sut.sync(); - - expect(tspSyncMigrationService.migrateTspUsers).toHaveBeenCalledTimes(1); - expect(tspSyncMigrationService.migrateTspUsers).toHaveBeenCalledWith( - system, - new Map([ - [tspStudentMigration.schuelerUidAlt, tspStudentMigration.schuelerUidNeu], - [tspTeacherMigration.lehrerUidAlt, tspTeacherMigration.lehrerUidNeu], - ]) - ); - }); - }); }); describe('when tsp system is not found', () => { @@ -409,7 +315,7 @@ describe(TspSyncStrategy.name, () => { setupMockServices({ fetchedSchools: tspSchools, foundSystem: system, - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); return { system, tspSchool }; @@ -439,7 +345,7 @@ describe(TspSyncStrategy.name, () => { }), fetchedSchools: tspSchools, foundSchool: school, - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); return { school, tspSchool }; @@ -468,7 +374,7 @@ describe(TspSyncStrategy.name, () => { oauthConfig: systemOauthConfigFactory.build(), }), fetchedSchools: tspSchools, - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); }; @@ -492,7 +398,7 @@ describe(TspSyncStrategy.name, () => { oauthConfig: systemOauthConfigFactory.build(), }), foundSystemSchools: schoolFactory.buildList(2), - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); }; @@ -522,7 +428,7 @@ describe(TspSyncStrategy.name, () => { }), usersOfClasses: new Map(), }, - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); }; @@ -552,7 +458,7 @@ describe(TspSyncStrategy.name, () => { [faker.string.uuid(), [{ externalId: faker.string.uuid(), role: RoleName.TEACHER }]], ]), }, - configValues: [1, 10, true, 10, 1, 50], + configValues: [1, 10, 15, 50], }); }; it('should throw NotFoundLoggableException', async () => { diff --git a/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.ts b/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.ts index eeef53f6c16..b51edddb0ed 100644 --- a/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.ts +++ b/apps/server/src/infra/sync/strategy/tsp/tsp-sync.strategy.ts @@ -25,12 +25,9 @@ import { TspSchoolsSyncedLoggable } from './loggable/tsp-schools-synced.loggable import { TspSchulnummerMissingLoggable } from './loggable/tsp-schulnummer-missing.loggable'; import { TspSyncedUsersLoggable } from './loggable/tsp-synced-users.loggable'; import { TspSyncingUsersLoggable } from './loggable/tsp-syncing-users.loggable'; -import { TspUsersMigratedLoggable } from './loggable/tsp-users-migrated.loggable'; import { TspFetchService } from './tsp-fetch.service'; -import { TspLegacyMigrationService } from './tsp-legacy-migration.service'; import { TspOauthDataMapper, TspUserInfo } from './tsp-oauth-data.mapper'; import { TspSchoolService } from './tsp-school.service'; -import { TspSyncMigrationService } from './tsp-sync-migration.service'; import { TspSyncConfig } from './tsp-sync.config'; type TspSchoolData = { @@ -46,11 +43,9 @@ export class TspSyncStrategy extends SyncStrategy { private readonly tspSyncService: TspSchoolService, private readonly tspFetchService: TspFetchService, private readonly tspOauthDataMapper: TspOauthDataMapper, - private readonly tspLegacyMigrationService: TspLegacyMigrationService, private readonly configService: ConfigService, private readonly systemService: SystemService, - private readonly provisioningService: TspProvisioningService, - private readonly tspSyncMigrationService: TspSyncMigrationService + private readonly provisioningService: TspProvisioningService ) { super(); this.logger.setContext(TspSyncStrategy.name); @@ -64,16 +59,10 @@ export class TspSyncStrategy extends SyncStrategy { // Please keep the order of this steps/methods as each relies on the data processed in the ones before. const system = await this.findTspSystemOrFail(); - await this.tspLegacyMigrationService.prepareLegacySyncDataForNewSync(system.id); - await this.syncTspSchools(system); const schools = await this.tspSyncService.findAllSchoolsForSystem(system); - if (this.configService.getOrThrow('FEATURE_TSP_MIGRATION_ENABLED', { infer: true })) { - await this.runMigrationOfExistingUsers(system); - } - await this.syncDataOfSyncedTspSchools(system, schools); } @@ -252,34 +241,6 @@ export class TspSyncStrategy extends SyncStrategy { return { tspTeachers, tspStudents, tspClasses }; } - private async runMigrationOfExistingUsers(system: System): Promise { - const oldToNewMappings = new Map(); - const [teacherMigrations, studentsMigrations] = await Promise.all([ - this.tspFetchService.fetchTspTeacherMigrations(system), - this.tspFetchService.fetchTspStudentMigrations(system), - ]); - - teacherMigrations.forEach(({ lehrerUidAlt, lehrerUidNeu }) => { - if (lehrerUidAlt && lehrerUidNeu) { - oldToNewMappings.set(lehrerUidAlt, lehrerUidNeu); - } - }); - 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 - ) - ); - } - private async findTspSystemOrFail(): Promise { const systems = ( await this.systemService.find({ diff --git a/apps/server/src/infra/sync/sync.module.ts b/apps/server/src/infra/sync/sync.module.ts index 64b41528694..a73f39a5d85 100644 --- a/apps/server/src/infra/sync/sync.module.ts +++ b/apps/server/src/infra/sync/sync.module.ts @@ -21,9 +21,7 @@ import { VidisSyncService, VidisSyncStrategy } from './media-licenses'; import { MediaMetadataSyncStrategy } from './media-metadata'; import { SyncService } from './service/sync.service'; import { TspFetchService } from './strategy/tsp/tsp-fetch.service'; -import { TspLegacyMigrationService } from './strategy/tsp/tsp-legacy-migration.service'; import { TspOauthDataMapper } from './strategy/tsp/tsp-oauth-data.mapper'; -import { TspSyncMigrationService } from './strategy/tsp/tsp-sync-migration.service'; import { TspSchoolService } from './strategy/tsp/tsp-school.service'; import { TspSyncStrategy } from './strategy/tsp/tsp-sync.strategy'; import { SyncUc } from './uc/sync.uc'; @@ -62,14 +60,7 @@ import { SyncUc } from './uc/sync.uc'; VidisSyncService, VidisSyncStrategy, ...((Configuration.get('FEATURE_TSP_SYNC_ENABLED') as boolean) - ? [ - TspSyncStrategy, - TspSchoolService, - TspOauthDataMapper, - TspFetchService, - TspLegacyMigrationService, - TspSyncMigrationService, - ] + ? [TspSyncStrategy, TspSchoolService, TspOauthDataMapper, TspFetchService] : []), ...((Configuration.get('FEATURE_MEDIA_METADATA_SYNC_ENABLED') as boolean) ? [MediaMetadataSyncStrategy] : []), ], diff --git a/apps/server/src/infra/tsp-client/testing/index.ts b/apps/server/src/infra/tsp-client/testing/index.ts index 84d95f3ceaa..ac76607fb26 100644 --- a/apps/server/src/infra/tsp-client/testing/index.ts +++ b/apps/server/src/infra/tsp-client/testing/index.ts @@ -2,5 +2,3 @@ export { robjExportSchuleFactory } from './robj-export-schule.factory'; export { robjExportKlasseFactory } from './robj-export-klasse.factory'; export { robjExportLehrerFactory } from './robj-export-lehrer.factory'; export { robjExportSchuelerFactory } from './robj-export-schueler.factory'; -export { robjExportLehrerMigrationFactory } from './robj-export-lehrer-migration.factory'; -export { robjExportSchuelerMigrationFactory } from './robj-export-schueler-migration.factory'; diff --git a/apps/server/src/infra/tsp-client/testing/robj-export-lehrer-migration.factory.ts b/apps/server/src/infra/tsp-client/testing/robj-export-lehrer-migration.factory.ts deleted file mode 100644 index 86f65ac4b03..00000000000 --- a/apps/server/src/infra/tsp-client/testing/robj-export-lehrer-migration.factory.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { ObjectId } from '@mikro-orm/mongodb'; -import { RobjExportLehrerMigration } from '@infra/tsp-client'; -import { Factory } from 'fishery'; - -export const robjExportLehrerMigrationFactory = Factory.define( - () => { - return { - lehrerUidAlt: new ObjectId().toHexString(), - lehrerUidNeu: new ObjectId().toHexString(), - }; - } -); diff --git a/apps/server/src/infra/tsp-client/testing/robj-export-schueler-migration.factory.ts b/apps/server/src/infra/tsp-client/testing/robj-export-schueler-migration.factory.ts deleted file mode 100644 index 7d0fc0d5f9e..00000000000 --- a/apps/server/src/infra/tsp-client/testing/robj-export-schueler-migration.factory.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { ObjectId } from '@mikro-orm/mongodb'; -import { RobjExportSchuelerMigration } from '@infra/tsp-client'; -import { Factory } from 'fishery'; - -export const robjExportSchuelerMigrationFactory = Factory.define< - RobjExportSchuelerMigration, - RobjExportSchuelerMigration ->(() => { - return { - schuelerUidAlt: new ObjectId().toHexString(), - schuelerUidNeu: new ObjectId().toHexString(), - }; -}); diff --git a/apps/server/src/modules/server/server.config.ts b/apps/server/src/modules/server/server.config.ts index a7bf527c161..3050aeaf7ca 100644 --- a/apps/server/src/modules/server/server.config.ts +++ b/apps/server/src/modules/server/server.config.ts @@ -321,8 +321,6 @@ const config: ServerConfig = { TSP_SYNC_SCHOOL_DAYS_TO_FETCH: Configuration.get('TSP_SYNC_SCHOOL_DAYS_TO_FETCH') as number, TSP_SYNC_DATA_LIMIT: Configuration.get('TSP_SYNC_DATA_LIMIT') as number, TSP_SYNC_DATA_DAYS_TO_FETCH: Configuration.get('TSP_SYNC_DATA_DAYS_TO_FETCH') as number, - TSP_SYNC_MIGRATION_LIMIT: Configuration.get('TSP_SYNC_MIGRATION_LIMIT') as number, - FEATURE_TSP_MIGRATION_ENABLED: Configuration.get('FEATURE_TSP_MIGRATION_ENABLED') as boolean, 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/apps/server/src/modules/user/domain/do/index.ts b/apps/server/src/modules/user/domain/do/index.ts index d3018feb302..b0bd405b263 100644 --- a/apps/server/src/modules/user/domain/do/index.ts +++ b/apps/server/src/modules/user/domain/do/index.ts @@ -1,5 +1,4 @@ export { Consent } from './consent'; export { ParentConsent } from './parent-consent'; export { UserConsent } from './user-consent'; -export { UserSourceOptions } from './user-source-options'; export { SecondarySchoolReference, UserDo } from './user.do'; diff --git a/apps/server/src/modules/user/domain/do/user-source-options.spec.ts b/apps/server/src/modules/user/domain/do/user-source-options.spec.ts deleted file mode 100644 index e0eebc52a33..00000000000 --- a/apps/server/src/modules/user/domain/do/user-source-options.spec.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { UserSourceOptions } from './user-source-options'; - -describe(UserSourceOptions.name, () => { - describe('constructor', () => { - describe('When a constructor is called', () => { - const setup = () => { - const domainObject = new UserSourceOptions({ tspUid: '12345' }); - - return { domainObject }; - }; - - it('should create empty object', () => { - const domainObject = new UserSourceOptions({}); - - expect(domainObject).toEqual(expect.objectContaining({})); - }); - - it('should contain valid tspUid ', () => { - const { domainObject } = setup(); - - const userSourceOptionsDo: UserSourceOptions = new UserSourceOptions(domainObject); - - expect(userSourceOptionsDo.tspUid).toEqual(domainObject.tspUid); - }); - }); - }); - describe('getters', () => { - describe('When getters are used', () => { - it('getters should return proper value', () => { - const props = { - tspUid: '12345', - }; - - const userSourceOptionsDo = new UserSourceOptions(props); - const gettersValues = { - tspUid: userSourceOptionsDo.tspUid, - }; - - expect(gettersValues).toEqual(props); - }); - }); - }); -}); diff --git a/apps/server/src/modules/user/domain/do/user-source-options.ts b/apps/server/src/modules/user/domain/do/user-source-options.ts deleted file mode 100644 index 49ab42d5071..00000000000 --- a/apps/server/src/modules/user/domain/do/user-source-options.ts +++ /dev/null @@ -1,15 +0,0 @@ -export interface UserSourceOptionsProps { - tspUid?: string; -} - -export class UserSourceOptions { - protected props: UserSourceOptionsProps; - - constructor(props: UserSourceOptionsProps) { - this.props = props; - } - - get tspUid(): string | undefined { - return this.props.tspUid; - } -} diff --git a/apps/server/src/modules/user/domain/do/user.do.ts b/apps/server/src/modules/user/domain/do/user.do.ts index eef4df911ca..bd83985deab 100644 --- a/apps/server/src/modules/user/domain/do/user.do.ts +++ b/apps/server/src/modules/user/domain/do/user.do.ts @@ -2,7 +2,6 @@ import { BaseDO, RoleReference } from '@shared/domain/domainobject'; import { LanguageType } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { Consent } from './consent'; -import { UserSourceOptions } from './user-source-options'; export class SecondarySchoolReference { public schoolId: EntityId; @@ -70,8 +69,6 @@ export class UserDo extends BaseDO { public source?: string; - public sourceOptions?: UserSourceOptions; - public lastSyncedAt?: Date; constructor(domainObject: UserDo) { @@ -104,7 +101,6 @@ export class UserDo extends BaseDO { this.birthday = domainObject.birthday; this.consent = domainObject.consent; this.source = domainObject.source; - this.sourceOptions = domainObject.sourceOptions; this.lastSyncedAt = domainObject.lastSyncedAt; } } diff --git a/apps/server/src/modules/user/domain/interface/user-do.repo.interface.ts b/apps/server/src/modules/user/domain/interface/user-do.repo.interface.ts index 0a9c359b341..0ecd86c4411 100644 --- a/apps/server/src/modules/user/domain/interface/user-do.repo.interface.ts +++ b/apps/server/src/modules/user/domain/interface/user-do.repo.interface.ts @@ -22,8 +22,6 @@ export interface UserDoRepo { findByEmail(email: string): Promise; - findByTspUids(tspUids: string[]): Promise; - save(domainObject: UserDo): Promise; saveAll(domainObjects: UserDo[]): Promise; diff --git a/apps/server/src/modules/user/domain/query/user-query.ts b/apps/server/src/modules/user/domain/query/user-query.ts index f4c1abfc9f9..cfd32269138 100644 --- a/apps/server/src/modules/user/domain/query/user-query.ts +++ b/apps/server/src/modules/user/domain/query/user-query.ts @@ -13,5 +13,4 @@ export type UserQuery = Partial> & { lastLoginSystemChangeSmallerThan?: Date; lastLoginSystemChangeBetweenStart?: Date; lastLoginSystemChangeBetweenEnd?: Date; - tspUid?: string; }; diff --git a/apps/server/src/modules/user/domain/service/user.service.spec.ts b/apps/server/src/modules/user/domain/service/user.service.spec.ts index 104088f16cc..15e10d30d6c 100644 --- a/apps/server/src/modules/user/domain/service/user.service.spec.ts +++ b/apps/server/src/modules/user/domain/service/user.service.spec.ts @@ -1266,24 +1266,4 @@ 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/domain/service/user.service.ts b/apps/server/src/modules/user/domain/service/user.service.ts index 63c55d00328..ee72bea0c20 100644 --- a/apps/server/src/modules/user/domain/service/user.service.ts +++ b/apps/server/src/modules/user/domain/service/user.service.ts @@ -359,12 +359,6 @@ export class UserService implements DeletionService, IEventHandler { - const userDOs = this.userDoRepo.findByTspUids(tspUids); - - return userDOs; - } - public findForImportUser( school: SchoolEntity, userName?: UserName, diff --git a/apps/server/src/modules/user/index.ts b/apps/server/src/modules/user/index.ts index 9dfce89dd0a..623c2a953c8 100644 --- a/apps/server/src/modules/user/index.ts +++ b/apps/server/src/modules/user/index.ts @@ -1,3 +1,3 @@ -export { Consent, ParentConsent, UserConsent, UserDo, UserService, UserSourceOptions } from './domain'; +export { Consent, ParentConsent, UserConsent, UserDo, UserService } from './domain'; export { UserConfig } from './user.config'; export { UserModule } from './user.module'; diff --git a/apps/server/src/modules/user/repo/scope/user.scope.spec.ts b/apps/server/src/modules/user/repo/scope/user.scope.spec.ts index 6ebae64d62f..a0aabd3eab3 100644 --- a/apps/server/src/modules/user/repo/scope/user.scope.spec.ts +++ b/apps/server/src/modules/user/repo/scope/user.scope.spec.ts @@ -243,23 +243,4 @@ describe('UserScope', () => { }); }); }); - - describe('byTspUid is called', () => { - describe('when tsp parameter is defined', () => { - it('should return scope with added tspUid to query', () => { - const tspUid = 'tspUid'; - - scope.byTspUid(tspUid); - - expect(scope.query).toEqual({ sourceOptions: { tspUid } }); - }); - }); - - describe('when tsp parameter is undefined', () => { - it('should return scope without added tspUid to query', () => { - scope.byTspUid(undefined); - expect(scope.query).toEqual({}); - }); - }); - }); }); diff --git a/apps/server/src/modules/user/repo/scope/user.scope.ts b/apps/server/src/modules/user/repo/scope/user.scope.ts index 71d46bf0d1d..cbdd72d8512 100644 --- a/apps/server/src/modules/user/repo/scope/user.scope.ts +++ b/apps/server/src/modules/user/repo/scope/user.scope.ts @@ -73,11 +73,4 @@ export class UserScope extends Scope { } return this; } - - public byTspUid(tspUid?: string): UserScope { - if (tspUid !== undefined) { - this.addQuery({ sourceOptions: { tspUid } }); - } - return this; - } } diff --git a/apps/server/src/modules/user/repo/user-do.repo.integration.spec.ts b/apps/server/src/modules/user/repo/user-do.repo.integration.spec.ts index ff2355031ed..edae119167a 100644 --- a/apps/server/src/modules/user/repo/user-do.repo.integration.spec.ts +++ b/apps/server/src/modules/user/repo/user-do.repo.integration.spec.ts @@ -15,7 +15,7 @@ import { RoleReference } from '@shared/domain/domainobject'; import { type IFindOptions, LanguageType, SortOrder } from '@shared/domain/interface'; import { cleanupCollections } from '@testing/cleanup-collections'; import { MongoMemoryDatabaseModule } from '@testing/database'; -import { MultipleUsersFoundLoggableException, UserDiscoverableQuery, type UserDo, UserSourceOptions } from '../domain'; +import { MultipleUsersFoundLoggableException, UserDiscoverableQuery, type UserDo } from '../domain'; import { userDoFactory, userFactory } from '../testing'; import { UserDoMikroOrmRepo } from './user-do.repo'; import { User } from './user.entity'; @@ -159,7 +159,7 @@ describe('UserRepo', () => { }); }); - describe('when multiple users were found', () => { + describe('when multiple users were found with the same system', () => { const externalId = 'externalId'; let system: SystemEntity; let school: SchoolEntity; @@ -180,6 +180,31 @@ describe('UserRepo', () => { ); }); }); + + describe('when two users have the same external id but different systems', () => { + it('should return the user of the requested system', async () => { + const system1 = systemEntityFactory.buildWithId(); + const system2 = systemEntityFactory.buildWithId(); + const school1 = schoolEntityFactory.buildWithId(); + const school2 = schoolEntityFactory.buildWithId(); + school1.systems.add(system1); + school2.systems.add(system2); + const user1 = userFactory.buildWithId({ externalId: 'externalId', school: school1 }); + const user2 = userFactory.buildWithId({ externalId: 'externalId', school: school2 }); + + await em.persistAndFlush([system1, system2, school1, school2, user1, user2]); + + const result = await repo.findByExternalId(user2.externalId as string, system2.id); + + expect(result).toEqual( + expect.objectContaining({ + id: user2.id, + externalId: user2.externalId, + schoolId: school2.id, + }) + ); + }); + }); }); describe('findByExternalIdOrFail', () => { @@ -836,32 +861,4 @@ describe('UserRepo', () => { }); }); }); - - describe('findByTspUids', () => { - describe('when users are found', () => { - const setup = async () => { - const tspUid = new ObjectId().toHexString(); - - const 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/modules/user/repo/user-do.repo.ts b/apps/server/src/modules/user/repo/user-do.repo.ts index 140d3ede7bb..f1ee2309f0a 100644 --- a/apps/server/src/modules/user/repo/user-do.repo.ts +++ b/apps/server/src/modules/user/repo/user-do.repo.ts @@ -10,14 +10,12 @@ import { BaseDORepo } from '@shared/repo/base.do.repo'; import { Scope } from '@shared/repo/scope'; import { ObjectId } from 'bson'; import { Consent, MultipleUsersFoundLoggableException, ParentConsent, UserConsent, type UserDoRepo } from '../domain'; -import { UserSourceOptions } from '../domain/do/user-source-options'; import { SecondarySchoolReference, UserDo } from '../domain/do/user.do'; import { UserQuery } from '../domain/query/user-query'; import { ConsentEntity } from './consent.entity'; import { ParentConsentEntity } from './parent-consent.entity'; import { UserScope } from './scope/user.scope'; import { UserConsentEntity } from './user-consent.entity'; -import { UserSourceOptionsEntity } from './user-source-options-entity'; import { User, UserSchoolEmbeddable } from './user.entity'; @Injectable() @@ -31,7 +29,6 @@ export class UserDoMikroOrmRepo extends BaseDORepo implements User const order: QueryOrderMap = this.createQueryOrderMap(options?.order || {}); const scope: Scope = new UserScope() .bySchoolId(query.schoolId) - .byTspUid(query.tspUid) .byRoleId(query.roleId) .withDiscoverableTrue(query.discoverable) .isOutdated(query.isOutdated) @@ -118,15 +115,18 @@ export class UserDoMikroOrmRepo extends BaseDORepo implements User public async findByExternalId(externalId: string, systemId: string): Promise { const userEntitys: User[] = await this._em.find(User, { externalId }, { populate: ['school.systems'] }); - if (userEntitys.length > 1) { - throw new MultipleUsersFoundLoggableException(externalId); - } - const userEntity: User | undefined = userEntitys.find((user: User): boolean => { + const users: User[] = userEntitys.filter((user: User): boolean => { const { systems } = user.school; return systems && !!systems.getItems().find((system): boolean => system.id === systemId); }); + if (users.length > 1) { + throw new MultipleUsersFoundLoggableException(externalId); + } + if (users.length === 0) { + return null; + } - const userDo: UserDo | null = userEntity ? this.mapEntityToDO(userEntity) : null; + const userDo = this.mapEntityToDO(users[0]); return userDo; } @@ -170,7 +170,6 @@ export class UserDoMikroOrmRepo extends BaseDORepo implements User outdatedSince: entity.outdatedSince, previousExternalId: entity.previousExternalId, birthday: entity.birthday, - sourceOptions: entity.sourceOptions ? new UserSourceOptions({ tspUid: entity.sourceOptions.tspUid }) : undefined, lastSyncedAt: entity.lastSyncedAt, consent: entity.consent ? this.mapConsentEntityToDo(entity.consent) : undefined, source: entity.source, @@ -219,31 +218,12 @@ export class UserDoMikroOrmRepo extends BaseDORepo implements User outdatedSince: entityDO.outdatedSince, previousExternalId: entityDO.previousExternalId, birthday: entityDO.birthday, - sourceOptions: entityDO.sourceOptions - ? new UserSourceOptionsEntity({ tspUid: entityDO.sourceOptions.tspUid }) - : undefined, lastSyncedAt: entityDO.lastSyncedAt, consent: entityDO.consent ? this.mapConsentToEntity(entityDO.consent) : undefined, source: entityDO.source, }; } - 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 mapConsentEntityToDo(consentEntity: ConsentEntity): Consent { const consent = new Consent({ userConsent: consentEntity.userConsent ? this.mapUserConsentEntityToDo(consentEntity.userConsent) : undefined, diff --git a/apps/server/src/modules/user/repo/user-source-options-entity.spec.ts b/apps/server/src/modules/user/repo/user-source-options-entity.spec.ts deleted file mode 100644 index 2bb17cfcdbe..00000000000 --- a/apps/server/src/modules/user/repo/user-source-options-entity.spec.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { UserSourceOptionsEntity } from './user-source-options-entity'; - -describe(UserSourceOptionsEntity.name, () => { - describe('constructor', () => { - describe('When a contructor is called', () => { - const setup = () => { - const entityProps = { tspUid: 'tspUid' }; - - return { entityProps }; - }; - - it('should contain valid tspUid ', () => { - const { entityProps } = setup(); - - const userSourceOptionsEntity: UserSourceOptionsEntity = new UserSourceOptionsEntity(entityProps); - - expect(userSourceOptionsEntity.tspUid).toEqual(entityProps.tspUid); - }); - }); - }); -}); diff --git a/apps/server/src/modules/user/repo/user-source-options-entity.ts b/apps/server/src/modules/user/repo/user-source-options-entity.ts deleted file mode 100644 index 4a85e0e1fa2..00000000000 --- a/apps/server/src/modules/user/repo/user-source-options-entity.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Embeddable, Property } from '@mikro-orm/core'; - -export interface UserSourceOptionsEntityProps { - tspUid?: string; -} - -@Embeddable() -export class UserSourceOptionsEntity { - @Property({ nullable: true, unique: true }) - tspUid?: string; - - constructor(props: UserSourceOptionsEntityProps) { - if (props.tspUid !== undefined) { - this.tspUid = props.tspUid; - } - } -} diff --git a/apps/server/src/modules/user/repo/user.entity.spec.ts b/apps/server/src/modules/user/repo/user.entity.spec.ts index be7b37ffd22..d13df3fcf36 100644 --- a/apps/server/src/modules/user/repo/user.entity.spec.ts +++ b/apps/server/src/modules/user/repo/user.entity.spec.ts @@ -40,7 +40,6 @@ describe('User Entity', () => { school, roles: [], source: 'ldap', - sourceOptions: { tspUid: '123' }, }); expect(user).toBeInstanceOf(User); diff --git a/apps/server/src/modules/user/repo/user.entity.ts b/apps/server/src/modules/user/repo/user.entity.ts index cda940363a6..b5e6ef7b179 100644 --- a/apps/server/src/modules/user/repo/user.entity.ts +++ b/apps/server/src/modules/user/repo/user.entity.ts @@ -8,6 +8,7 @@ import { ManyToMany, ManyToOne, Property, + Unique, wrap, } from '@mikro-orm/core'; import { RoleName } from '@modules/role'; @@ -19,7 +20,6 @@ import { LanguageType, Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { ConsentEntity } from './consent.entity'; import { UserParentsEntity } from './user-parents.entity'; -import { UserSourceOptionsEntity } from './user-source-options-entity'; export interface UserProperties { email: string; @@ -45,7 +45,6 @@ export interface UserProperties { lastSyncedAt?: Date; consent?: ConsentEntity; source?: string; - sourceOptions?: UserSourceOptionsEntity; } interface UserInfo { @@ -74,6 +73,11 @@ export class UserSchoolEmbeddable { @Entity({ tableName: 'users' }) @Index({ properties: ['id', 'email'] }) @Index({ properties: ['firstName', 'lastName'] }) +@Unique({ + properties: ['externalId', 'source'], + options: { partialFilterExpression: { source: { $exists: true } } }, + // TODO: LDAP, Moin.Schule and BRB-IDM have to set source as well. +}) @Index({ properties: ['externalId', 'school'] }) @Index({ properties: ['school', 'ldapDn'] }) @Index({ properties: ['school', 'roles'] }) @@ -166,9 +170,6 @@ export class User extends BaseEntityWithTimestamps { @Index() source?: string; - @Embedded(() => UserSourceOptionsEntity, { object: true, nullable: true }) - sourceOptions?: UserSourceOptionsEntity; - constructor(props: UserProperties) { super(); this.firstName = props.firstName; @@ -196,10 +197,6 @@ export class User extends BaseEntityWithTimestamps { if (props.source !== undefined) { this.source = props.source; } - - if (props.sourceOptions !== undefined) { - this.sourceOptions = props.sourceOptions; - } } public resolvePermissions(): string[] { diff --git a/apps/server/src/modules/user/repo/user.repo.integration.spec.ts b/apps/server/src/modules/user/repo/user.repo.integration.spec.ts index b46119456c2..3e1409908aa 100644 --- a/apps/server/src/modules/user/repo/user.repo.integration.spec.ts +++ b/apps/server/src/modules/user/repo/user.repo.integration.spec.ts @@ -20,7 +20,7 @@ describe('user repo', () => { beforeAll(async () => { module = await Test.createTestingModule({ - imports: [MongoMemoryDatabaseModule.forRoot({ entities: [User] })], + imports: [MongoMemoryDatabaseModule.forRoot({ entities: [User], ensureIndexes: true })], providers: [UserMikroOrmRepo], }).compile(); repo = module.get(UserMikroOrmRepo); @@ -69,7 +69,6 @@ describe('user repo', () => { 'school', 'secondarySchools', 'source', - 'sourceOptions', '_id', 'ldapDn', 'externalId', @@ -220,7 +219,6 @@ describe('user repo', () => { 'birthday', 'consent', 'source', - 'sourceOptions', 'discoverable', ].sort() ); @@ -724,4 +722,34 @@ describe('user repo', () => { }); }); }); + + describe('externalId uniqueness', () => { + it('should fail to safe the same externalId with the same source', async () => { + const externalId = '123'; + const first = userFactory.build({ externalId, source: 'same' }); + const second = userFactory.build({ externalId, source: 'same' }); + + await expect(() => em.persistAndFlush([first, second])).rejects.toThrow(); + }); + + it('should successfully save the same externalId with a different source', async () => { + const externalId = '456'; + const first = userFactory.build({ externalId, source: 'same' }); + const second = userFactory.build({ externalId, source: 'different' }); + + await em.persistAndFlush([first, second]); + + expect('we made it here').toBeTruthy(); + }); + + it('should successfully save the same externalId twice without a source', async () => { + const externalId = '789'; + const first = userFactory.build({ externalId }); + const second = userFactory.build({ externalId }); + + await em.persistAndFlush([first, second]); + + expect('we made it here').toBeTruthy(); + }); + }); }); diff --git a/config/default.schema.json b/config/default.schema.json index 6a4b6873e88..d5b33c875d3 100644 --- a/config/default.schema.json +++ b/config/default.schema.json @@ -8,12 +8,7 @@ }, "NODE_ENV": { "type": "string", - "enum": [ - "development", - "test", - "production", - "migration" - ], + "enum": ["development", "test", "production", "migration"], "default": "production" }, "REQUEST_OPTION": { @@ -72,10 +67,7 @@ "DEFAULT_LANGUAGE": { "type": "string", "default": "de", - "enum": [ - "de", - "en" - ], + "enum": ["de", "en"], "description": "Value for the default language" }, "DEFAULT_TIMEZONE": { @@ -103,23 +95,13 @@ "TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION": { "type": "string", "default": "disabled", - "enum": [ - "disabled", - "opt-in", - "opt-out", - "enabled" - ], + "enum": ["disabled", "opt-in", "opt-out", "enabled"], "description": "defines wheter external team invitation shows teachers from different schools or not. if enabled system wide there are options general enabled or opt-in/-out by user required." }, "STUDENT_TEAM_CREATION": { "type": "string", "default": "opt-out", - "enum": [ - "disabled", - "opt-in", - "opt-out", - "enabled" - ], + "enum": ["disabled", "opt-in", "opt-out", "enabled"], "description": "defines wheter students may create teams or not. if enabled system wide there are options general enabled or opt-in/-out by school admin required." }, "REDIS_URI": { @@ -201,16 +183,6 @@ "default": "1", "description": "The amount of days for which the sync fetches school data from the TSP." }, - "TSP_SYNC_MIGRATION_LIMIT": { - "type": "number", - "default": "50", - "description": "The amount of users the sync migrates at once." - }, - "FEATURE_TSP_MIGRATION_ENABLED": { - "type": "boolean", - "default": false, - "description": "Feature toggle for TSP migration." - }, "FEATURE_TSP_SYNC_ENABLED": { "type": "boolean", "default": false, @@ -265,13 +237,7 @@ "FILES_STORAGE": { "type": "object", "description": "Files storage server properties, required always to be defined", - "required": [ - "S3_ENDPOINT", - "S3_REGION", - "S3_BUCKET", - "S3_ACCESS_KEY_ID", - "S3_SECRET_ACCESS_KEY" - ], + "required": ["S3_ENDPOINT", "S3_REGION", "S3_BUCKET", "S3_ACCESS_KEY_ID", "S3_SECRET_ACCESS_KEY"], "properties": { "SERVICE_BASE_URL": { "type": "string", @@ -337,13 +303,7 @@ "FWU_CONTENT": { "type": "object", "description": "Properties of the S3 storage containing FWU content", - "required": [ - "S3_ENDPOINT", - "S3_REGION", - "S3_BUCKET", - "S3_ACCESS_KEY", - "S3_SECRET_KEY" - ], + "required": ["S3_ENDPOINT", "S3_REGION", "S3_BUCKET", "S3_ACCESS_KEY", "S3_SECRET_KEY"], "properties": { "S3_ENDPOINT": { "type": "string", @@ -382,12 +342,7 @@ "H5P_EDITOR": { "type": "object", "description": "Properties of the H5P server microservice and library management job", - "required": [ - "S3_ENDPOINT", - "S3_REGION", - "S3_BUCKET_CONTENT", - "S3_BUCKET_LIBRARIES" - ], + "required": ["S3_ENDPOINT", "S3_REGION", "S3_BUCKET_CONTENT", "S3_BUCKET_LIBRARIES"], "default": {}, "properties": { "S3_ENDPOINT": { @@ -447,12 +402,7 @@ "H5P_Library": { "type": "object", "description": "Properties of the H5P server microservice", - "required": [ - "S3_ENDPOINT", - "S3_BUCKET_LIBRARIES", - "S3_ACCESS_KEY_ID", - "S3_SECRET_ACCESS_KEY" - ], + "required": ["S3_ENDPOINT", "S3_BUCKET_LIBRARIES", "S3_ACCESS_KEY_ID", "S3_SECRET_ACCESS_KEY"], "default": {}, "properties": { "S3_ENDPOINT": { @@ -866,9 +816,7 @@ "ETHERPAD": { "type": "object", "description": "Etherpad settings", - "required": [ - "PAD_URI" - ], + "required": ["PAD_URI"], "properties": { "URI": { "type": "string", @@ -1021,9 +969,7 @@ "API_VALIDATION_WHITELIST_EXTENSION": { "type": "string", "description": "when set, this is interpreted as a regex to extend the ignorelist for the API validation with any routes matching the regex.", - "examples": [ - ".*/courses/[0-9a-f]{24}($|/$)" - ] + "examples": [".*/courses/[0-9a-f]{24}($|/$)"] }, "FEATURE_PROMETHEUS_METRICS_ENABLED": { "type": "boolean", @@ -1098,31 +1044,13 @@ "type": "string", "default": "error", "description": "Log level for api.", - "enum": [ - "emerg", - "alert", - "crit", - "error", - "warning", - "notice", - "info", - "debug" - ] + "enum": ["emerg", "alert", "crit", "error", "warning", "notice", "info", "debug"] }, "NEST_LOG_LEVEL": { "type": "string", "default": "notice", "description": "Nest Log level for api. The http flag is for request logging. The http flag do only work by api methods with added 'request logging interceptor'.", - "enum": [ - "emerg", - "alert", - "crit", - "error", - "warning", - "notice", - "info", - "debug" - ] + "enum": ["emerg", "alert", "crit", "error", "warning", "notice", "info", "debug"] }, "EXIT_ON_ERROR": { "type": "boolean", @@ -1133,12 +1061,7 @@ "type": "string", "default": "requestError", "description": "Special logs.", - "enum": [ - "requestError", - "systemLogs", - "request", - "sendRequests" - ] + "enum": ["requestError", "systemLogs", "request", "sendRequests"] }, "SYNC_QUEUE_NAME": { "type": "string", @@ -1340,11 +1263,7 @@ "SAME_SITE": { "type": "string", "default": "none", - "enum": [ - "none", - "lax", - "strict" - ], + "enum": ["none", "lax", "strict"], "description": "Value for cookies sameSite property. When SECURE flag is false, 'None' is not allowed in SAME_SITE and Lax should be used as default instead" }, "HTTP_ONLY": { @@ -1368,13 +1287,7 @@ "description": "Expiration in seconds from now" } }, - "required": [ - "SAME_SITE", - "HTTP_ONLY", - "HOST_ONLY", - "SECURE", - "EXPIRES_SECONDS" - ], + "required": ["SAME_SITE", "HTTP_ONLY", "HOST_ONLY", "SECURE", "EXPIRES_SECONDS"], "allOf": [ { "$ref": "#/properties/COOKIE/definitions/SAME_SITE_SECURE_VALID" @@ -1392,10 +1305,7 @@ "then": { "properties": { "SAME_SITE": { - "enum": [ - "lax", - "strict" - ] + "enum": ["lax", "strict"] } } } @@ -1587,16 +1497,12 @@ "API_URL": { "type": "string", "description": "Base URL of the schulconnex API (from dof)", - "examples": [ - "https://api-dienste.stage.niedersachsen-login.schule/v1/" - ] + "examples": ["https://api-dienste.stage.niedersachsen-login.schule/v1/"] }, "TOKEN_ENDPOINT": { "type": "string", "description": "Token endpoint of the schulconnex API (from dof)", - "examples": [ - "https://api-dienste.stage.niedersachsen-login.schule/v1/oauth2/token" - ] + "examples": ["https://api-dienste.stage.niedersachsen-login.schule/v1/oauth2/token"] }, "CLIENT_ID": { "type": "string", @@ -1672,9 +1578,7 @@ "type": "string", "default": "", "description": "URL for fetching policies info from moin.schule schulconnex", - "examples": [ - "https://api-dienste.stage.niedersachsen-login.schule/v1/policies-info" - ] + "examples": ["https://api-dienste.stage.niedersachsen-login.schule/v1/policies-info"] }, "PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT": { "type": "number",