From ae19e32af9394a69903daacdf921cfd594e5bbdd Mon Sep 17 00:00:00 2001 From: "Marvin Rode (Cap)" <127723478+marode-cap@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:10:42 +0100 Subject: [PATCH 1/2] SPSH-1626 itslearning correlation-IDs (#839) * Add syncID to itslearning requests and logs * Fix linter warnings * Fix coverage --- ...arning-organisations.event-handler.spec.ts | 155 +++++++++++------- ...itslearning-organisations.event-handler.ts | 82 +++++---- .../itslearning-persons.event-handler.spec.ts | 128 ++++++++++----- .../itslearning-persons.event-handler.ts | 100 ++++++----- .../itslearning-sync.event-handler.spec.ts | 76 ++++++--- .../itslearning-sync.event-handler.ts | 58 ++++--- .../itslearning/itslearning.service.spec.ts | 20 +++ .../itslearning/itslearning.service.ts | 14 +- .../repo/itslearning-group.repo.spec.ts | 40 +++-- .../repo/itslearning-group.repo.ts | 18 +- .../repo/itslearning-membership.repo.spec.ts | 40 +++-- .../repo/itslearning-membership.repo.ts | 30 +++- .../repo/itslearning-person.repo.spec.ts | 38 +++-- .../repo/itslearning-person.repo.ts | 37 +++-- src/shared/events/base-event.ts | 5 + 15 files changed, 551 insertions(+), 290 deletions(-) diff --git a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts index 5a4f0d2ab..a6a0b45f6 100644 --- a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts +++ b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts @@ -68,13 +68,18 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams]>({ - id: event.id, - name: event.name!, - parentId: event.administriertVon!, - type: 'Unspecified', - }); - expect(loggerMock.info).toHaveBeenLastCalledWith(`Klasse with ID ${event.id} created.`); + expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams, string]>( + { + id: event.id, + name: event.name!, + parentId: event.administriertVon!, + type: 'Unspecified', + }, + `${event.eventID}-KLASSE-CREATED`, + ); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Klasse with ID ${event.id} created.`, + ); }); it('should skip event, if not enabled', async () => { @@ -87,7 +92,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -100,7 +105,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no parent organisation. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`, + ); }); it('should log error, if the klasse has no name', async () => { @@ -112,7 +119,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no name. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); }); it('should log info, if the parent school is not itslearning enabled', async () => { @@ -126,7 +133,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); }); @@ -142,7 +149,9 @@ describe('ItsLearning Organisations Event Handler', () => { ); // CreateGroupAction await sut.createKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenLastCalledWith('Could not create Klasse in itsLearning: Error'); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Could not create Klasse in itsLearning: Error`, + ); }); }); @@ -157,13 +166,18 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams]>({ - id: event.organisationId, - name: event.name, - parentId: event.administriertVon!, - type: 'Unspecified', - }); - expect(loggerMock.info).toHaveBeenLastCalledWith(`Klasse with ID ${event.organisationId} was updated.`); + expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams, string]>( + { + id: event.organisationId, + name: event.name, + parentId: event.administriertVon!, + type: 'Unspecified', + }, + `${event.eventID}-KLASSE-UPDATED`, + ); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was updated.`, + ); }); it('should skip event, if not enabled', async () => { @@ -176,7 +190,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -189,7 +203,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no parent organisation. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`, + ); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -198,7 +214,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no name. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -213,7 +229,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); }); @@ -230,7 +246,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenLastCalledWith('Could not update Klasse in itsLearning: Error'); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Could not update Klasse in itsLearning: Error`, + ); }); }); @@ -241,8 +259,13 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.deletedKlasseEventHandler(event); - expect(itslearningGroupRepoMock.deleteGroup).toHaveBeenLastCalledWith(event.organisationId); - expect(loggerMock.info).toHaveBeenLastCalledWith(`Klasse with ID ${event.organisationId} was deleted.`); + expect(itslearningGroupRepoMock.deleteGroup).toHaveBeenLastCalledWith( + event.organisationId, + `${event.eventID}-KLASSE-DELETED`, + ); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was deleted.`, + ); }); it('should skip event, if not enabled', async () => { @@ -251,7 +274,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.deletedKlasseEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.deleteGroup).not.toHaveBeenCalled(); }); @@ -261,7 +284,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.deletedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenLastCalledWith('Could not delete Klasse in itsLearning: Error'); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Could not delete Klasse in itsLearning: Error`, + ); }); }); @@ -277,7 +302,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.createOrUpdateGroups).not.toHaveBeenCalled(); }); @@ -292,7 +317,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `The organisation with ID ${event.organisationId} is not of type "SCHULE"!`, + `[EventID: ${event.eventID}] The organisation with ID ${event.organisationId} is not of type "SCHULE"!`, ); expect(itslearningGroupRepoMock.createOrUpdateGroups).not.toHaveBeenCalled(); }); @@ -310,7 +335,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Ersatzschule, ignoring.'); + expect(loggerMock.error).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Ersatzschule, ignoring.`); expect(itslearningGroupRepoMock.createOrUpdateGroups).not.toHaveBeenCalled(); }); @@ -332,7 +357,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: Error`, + `[EventID: ${event.eventID}] Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: Error`, ); }); @@ -360,28 +385,31 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Schule with ID ${event.organisationId} and its 2 Klassen were created.`, + `[EventID: ${event.eventID}] Schule with ID ${event.organisationId} and its 2 Klassen were created.`, + ); + expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[], string]>( + [ + { + id: event.organisationId, + name: `${event.kennung} (${event.name})`, + type: 'School', + parentId: sut.ROOT_OEFFENTLICH, + }, + { + id: klasse1.id, + name: `${klasse1.name}`, + type: 'Unspecified', + parentId: event.organisationId, + }, + { + id: klasse2.id, + name: `Unbenannte Klasse`, + type: 'Unspecified', + parentId: event.organisationId, + }, + ], + `${event.eventID}-SCHULE-SYNC`, ); - expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[]]>([ - { - id: event.organisationId, - name: `${event.kennung} (${event.name})`, - type: 'School', - parentId: sut.ROOT_OEFFENTLICH, - }, - { - id: klasse1.id, - name: `${klasse1.name}`, - type: 'Unspecified', - parentId: event.organisationId, - }, - { - id: klasse2.id, - name: `Unbenannte Klasse`, - type: 'Unspecified', - parentId: event.organisationId, - }, - ]); }); it('should set default name for schule', async () => { @@ -400,16 +428,19 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Schule with ID ${event.organisationId} and its 0 Klassen were created.`, + `[EventID: ${event.eventID}] Schule with ID ${event.organisationId} and its 0 Klassen were created.`, + ); + expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[], string]>( + [ + { + id: event.organisationId, + name: `${event.kennung} (Unbenannte Schule)`, + type: 'School', + parentId: sut.ROOT_OEFFENTLICH, + }, + ], + `${event.eventID}-SCHULE-SYNC`, ); - expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[]]>([ - { - id: event.organisationId, - name: `${event.kennung} (Unbenannte Schule)`, - type: 'School', - parentId: sut.ROOT_OEFFENTLICH, - }, - ]); }); }); }); diff --git a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts index 60734b4c5..e0c792fe9 100644 --- a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts +++ b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts @@ -38,19 +38,19 @@ export class ItsLearningOrganisationsEventHandler { @EventHandler(KlasseCreatedEvent) public async createKlasseEventHandler(event: KlasseCreatedEvent): Promise { - this.logger.info(`Received KlasseCreatedEvent, ID: ${event.id}`); + this.logger.info(`[EventID: ${event.eventID}] Received KlasseCreatedEvent, ID: ${event.id}`); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } if (!event.administriertVon) { - return this.logger.error('Klasse has no parent organisation. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`); } if (!event.name) { - return this.logger.error('Klasse has no name. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); } { @@ -58,7 +58,7 @@ export class ItsLearningOrganisationsEventHandler { const parent: Option> = await this.organisationRepo.findById(event.administriertVon); if (!parent?.itslearningEnabled) { return this.logger.info( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); } } @@ -70,30 +70,37 @@ export class ItsLearningOrganisationsEventHandler { parentId: event.administriertVon, }; - const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup(params); + const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup( + params, + `${event.eventID}-KLASSE-CREATED`, + ); if (createError) { - return this.logger.error(`Could not create Klasse in itsLearning: ${createError.message}`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not create Klasse in itsLearning: ${createError.message}`, + ); } - this.logger.info(`Klasse with ID ${event.id} created.`); + this.logger.info(`[EventID: ${event.eventID}] Klasse with ID ${event.id} created.`); } @EventHandler(KlasseUpdatedEvent) public async updatedKlasseEventHandler(event: KlasseUpdatedEvent): Promise { - this.logger.info(`Received KlasseUpdatedEvent, ID: ${event.organisationId}, new name: ${event.name}`); + this.logger.info( + `[EventID: ${event.eventID}] Received KlasseUpdatedEvent, ID: ${event.organisationId}, new name: ${event.name}`, + ); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } if (!event.administriertVon) { - return this.logger.error('Klasse has no parent organisation. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`); } if (!event.name) { - return this.logger.error('Klasse has no name. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); } { @@ -101,7 +108,7 @@ export class ItsLearningOrganisationsEventHandler { const parent: Option> = await this.organisationRepo.findById(event.administriertVon); if (!parent?.itslearningEnabled) { return this.logger.info( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); } } @@ -113,44 +120,58 @@ export class ItsLearningOrganisationsEventHandler { parentId: event.administriertVon, }; - const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup(params); + const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup( + params, + `${event.eventID}-KLASSE-UPDATED`, + ); if (createError) { - return this.logger.error(`Could not update Klasse in itsLearning: ${createError.message}`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not update Klasse in itsLearning: ${createError.message}`, + ); } - this.logger.info(`Klasse with ID ${event.organisationId} was updated.`); + this.logger.info(`[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was updated.`); } @EventHandler(KlasseDeletedEvent) public async deletedKlasseEventHandler(event: KlasseDeletedEvent): Promise { - this.logger.info(`Received KlasseUpdatedEvent, ID: ${event.organisationId}`); + this.logger.info(`[EventID: ${event.eventID}] Received KlasseUpdatedEvent, ID: ${event.organisationId}`); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } - const deleteError: Option = await this.itslearningGroupRepo.deleteGroup(event.organisationId); + const deleteError: Option = await this.itslearningGroupRepo.deleteGroup( + event.organisationId, + `${event.eventID}-KLASSE-DELETED`, + ); if (deleteError) { - return this.logger.error(`Could not delete Klasse in itsLearning: ${deleteError.message}`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not delete Klasse in itsLearning: ${deleteError.message}`, + ); } - this.logger.info(`Klasse with ID ${event.organisationId} was deleted.`); + this.logger.info(`[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was deleted.`); } @EventHandler(SchuleItslearningEnabledEvent) public async schuleItslearningEnabledEventHandler(event: SchuleItslearningEnabledEvent): Promise { - this.logger.info(`Received EnableSchuleItslearningEvent, ID: ${event.organisationId}`); + this.logger.info( + `[EventID: ${event.eventID}] Received EnableSchuleItslearningEvent, ID: ${event.organisationId}`, + ); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } if (event.typ !== OrganisationsTyp.SCHULE) { - this.logger.error(`The organisation with ID ${event.organisationId} is not of type "SCHULE"!`); + this.logger.error( + `[EventID: ${event.eventID}] The organisation with ID ${event.organisationId} is not of type "SCHULE"!`, + ); return; } @@ -160,7 +181,7 @@ export class ItsLearningOrganisationsEventHandler { ]); if (rootType === RootDirectChildrenType.ERSATZ) { - this.logger.error('Ersatzschule, ignoring.'); + this.logger.error(`[EventID: ${event.eventID}] Ersatzschule, ignoring.`); return; } @@ -182,14 +203,19 @@ export class ItsLearningOrganisationsEventHandler { parentId: this.ROOT_OEFFENTLICH, }); - const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroups(createParams); + const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroups( + createParams, + `${event.eventID}-SCHULE-SYNC`, + ); if (createError) { return this.logger.error( - `Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: ${createError.message}`, + `[EventID: ${event.eventID}] Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: ${createError.message}`, ); } - this.logger.info(`Schule with ID ${event.organisationId} and its ${klassen.length} Klassen were created.`); + this.logger.info( + `[EventID: ${event.eventID}] Schule with ID ${event.organisationId} and its ${klassen.length} Klassen were created.`, + ); } } diff --git a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts index 9782d6ae6..a6f3a7a8a 100644 --- a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts +++ b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts @@ -86,12 +86,13 @@ describe('ItsLearning Persons Event Handler', () => { ok: true, value: { deleted: 0, updated: 1 }, } satisfies Result); + const eventID: string = faker.string.uuid(); - await sut.updateMemberships(personId, currentKontexte); + await sut.updateMemberships(personId, currentKontexte, eventID); expect(itslearningMembershipRepoMock.setMemberships).toHaveBeenCalledTimes(1); expect(loggerMock.info).toHaveBeenCalledWith( - `Set ${currentKontexte.length} memberships for person ${personId}`, + `[EventID: ${eventID}] Set ${currentKontexte.length} memberships for person ${personId}`, ); }); @@ -103,12 +104,13 @@ describe('ItsLearning Persons Event Handler', () => { SetMembershipsResult, DomainError >); + const eventID: string = faker.string.uuid(); - await sut.updateMemberships(personId, currentKontexte); + await sut.updateMemberships(personId, currentKontexte, eventID); expect(itslearningMembershipRepoMock.setMemberships).toHaveBeenCalledTimes(1); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not set ${currentKontexte.length} memberships for person ${personId}`, + `[EventID: ${eventID}] Could not set ${currentKontexte.length} memberships for person ${personId}`, error, ); }); @@ -118,22 +120,24 @@ describe('ItsLearning Persons Event Handler', () => { it('should delete person in itsLearning', async () => { const personID: string = faker.string.uuid(); itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(undefined); + const eventID: string = faker.string.uuid(); - await sut.deletePerson(personID); + await sut.deletePerson(personID, eventID); expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith(personID); - expect(loggerMock.info).toHaveBeenCalledWith(`Person with ID ${personID} deleted.`); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${eventID}] Person with ID ${personID} deleted.`); }); it('should log error if person could not be deleted', async () => { const personID: string = faker.string.uuid(); itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(new ItsLearningError('Test Error')); + const eventID: string = faker.string.uuid(); - await sut.deletePerson(personID); + await sut.deletePerson(personID, eventID); expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith(personID); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not delete person with ID ${personID} from itsLearning.`, + `[EventID: ${eventID}] Could not delete person with ID ${personID} from itsLearning.`, ); }); }); @@ -161,38 +165,48 @@ describe('ItsLearning Persons Event Handler', () => { const [person, personResponse]: [Person, PersonResponse] = createPersonAndResponse(); itslearningPersonRepoMock.readPerson.mockResolvedValueOnce(personResponse); // Read person itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(undefined); // Create person + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); - expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: personResponse.institutionRole, - }); - expect(loggerMock.info).toHaveBeenCalledWith(`Person with ID ${person.id} updated in itsLearning!`); + expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: personResponse.institutionRole, + }, + `${event.eventID}-PERSON-RENAMED-UPDATE`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${person.id} updated in itsLearning!`, + ); }); it('should log error if person could not be updated', async () => { const [person, personResponse]: [Person, PersonResponse] = createPersonAndResponse(); itslearningPersonRepoMock.readPerson.mockResolvedValueOnce(personResponse); // Read person itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(new ItsLearningError('Test Error')); // Create person + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Person with ID ${person.id} could not be updated in itsLearning!`, + `[EventID: ${event.eventID}] Person with ID ${person.id} could not be updated in itsLearning!`, ); }); describe('when person is invalid', () => { it('should log error, if person has no referrer', async () => { const [person]: [Person, PersonResponse] = createPersonAndResponse({ referrer: undefined }); + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${person.id} has no username!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${person.id} has no username!`, + ); }); }); @@ -200,11 +214,12 @@ describe('ItsLearning Persons Event Handler', () => { it('should log info', async () => { const [person]: [Person, PersonResponse] = createPersonAndResponse(); itslearningPersonRepoMock.readPerson.mockResolvedValueOnce(undefined); // Read person + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Person with ID ${person.id} is not in itslearning, ignoring.`, + `[EventID: ${event.eventID}] Person with ID ${person.id} is not in itslearning, ignoring.`, ); }); }); @@ -212,10 +227,11 @@ describe('ItsLearning Persons Event Handler', () => { it('should skip event, if not enabled', async () => { sut.ENABLED = false; const [person]: [Person, PersonResponse] = createPersonAndResponse(); + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); }); }); @@ -230,27 +246,34 @@ describe('ItsLearning Persons Event Handler', () => { it('should send person to itsLearning', async () => { const kontextData: PersonenkontextUpdatedData = makeKontextEventData({ rolle: RollenArt.LERN }); itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(undefined); + const eventID: string = faker.string.uuid(); - await sut.updatePerson(person, [kontextData]); + await sut.updatePerson(person, [kontextData], eventID); - expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: IMSESInstitutionRoleType.STUDENT, - }); - expect(loggerMock.info).toHaveBeenCalledWith(`Person with ID ${person.id} created in itsLearning!`); + expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: IMSESInstitutionRoleType.STUDENT, + }, + eventID, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${eventID}] Person with ID ${person.id} created in itsLearning!`, + ); }); it('should log error if person could not be created', async () => { const kontextData: PersonenkontextUpdatedData = makeKontextEventData({ rolle: RollenArt.LERN }); itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(new ItsLearningError('Test Error')); + const eventID: string = faker.string.uuid(); - await sut.updatePerson(person, [kontextData]); + await sut.updatePerson(person, [kontextData], eventID); expect(loggerMock.error).toHaveBeenCalledWith( - `Person with ID ${person.id} could not be sent to itsLearning! Error: Test Error`, + `[EventID: ${eventID}] Person with ID ${person.id} could not be sent to itsLearning! Error: Test Error`, ); }); @@ -258,10 +281,13 @@ describe('ItsLearning Persons Event Handler', () => { it('should log error, if person has no referrer', async () => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { referrer, ...personWithoutReferrer }: PersonenkontextUpdatedPersonData = person; + const eventID: string = faker.string.uuid(); - await sut.updatePerson(personWithoutReferrer, [createMock()]); + await sut.updatePerson(personWithoutReferrer, [createMock()], eventID); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${person.id} has no username!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${eventID}] Person with ID ${person.id} has no username!`, + ); }); }); }); @@ -284,8 +310,14 @@ describe('ItsLearning Persons Event Handler', () => { await sut.oxUserChangedEventHandler(generatedEvent); - expect(itslearningPersonRepoMock.updateEmail).toHaveBeenCalledWith(personId, email); - expect(loggerMock.info).toHaveBeenCalledWith(`Updated E-Mail for person with ID ${personId}!`); + expect(itslearningPersonRepoMock.updateEmail).toHaveBeenCalledWith( + personId, + email, + `${generatedEvent.eventID}-EMAIL-UPDATE`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${generatedEvent.eventID}] Updated E-Mail for person with ID ${personId}!`, + ); }); it('should log error, if email could not be updated', async () => { @@ -293,7 +325,9 @@ describe('ItsLearning Persons Event Handler', () => { await sut.oxUserChangedEventHandler(generatedEvent); - expect(loggerMock.error).toHaveBeenCalledWith(`Could not update E-Mail for person with ID ${personId}!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${generatedEvent.eventID}] Could not update E-Mail for person with ID ${personId}!`, + ); }); it('should skip event, if not enabled', async () => { @@ -302,7 +336,9 @@ describe('ItsLearning Persons Event Handler', () => { await sut.oxUserChangedEventHandler(generatedEvent); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring email update.'); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${generatedEvent.eventID}] Not enabled, ignoring email update.`, + ); }); }); @@ -366,7 +402,11 @@ describe('ItsLearning Persons Event Handler', () => { await sut.updatePersonenkontexteEventHandler(event); expect(updateMembershipsSpy).toHaveBeenCalledTimes(1); - expect(updateMembershipsSpy).toHaveBeenCalledWith(event.person.id, expect.objectContaining({ length: 2 })); + expect(updateMembershipsSpy).toHaveBeenCalledWith( + event.person.id, + expect.objectContaining({ length: 2 }), + `${event.eventID}-UPDATE-MEMBERSHIPS`, + ); }); it('should not call updatePerson, if no relevant kontext exists', async () => { @@ -412,7 +452,7 @@ describe('ItsLearning Persons Event Handler', () => { await sut.updatePersonenkontexteEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); }); }); }); diff --git a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts index 796ee8e44..19cf2f161 100644 --- a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts +++ b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts @@ -43,58 +43,71 @@ export class ItsLearningPersonsEventHandler { @EventHandler(PersonRenamedEvent) public async personRenamedEventHandler(event: PersonRenamedEvent): Promise { await this.personUpdateMutex.runExclusive(async () => { - this.logger.info(`Received PersonRenamedEvent, ${event.personId}`); + this.logger.info(`[EventID: ${event.eventID}] Received PersonRenamedEvent, ${event.personId}`); if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring event.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); } if (!event.referrer) { - return this.logger.error(`Person with ID ${event.personId} has no username!`); + return this.logger.error( + `[EventID: ${event.eventID}] Person with ID ${event.personId} has no username!`, + ); } const readPersonResult: Option = await this.itslearningPersonRepo.readPerson( event.personId, + `${event.eventID}-PERSON-EXISTS-CHECK`, ); if (!readPersonResult) { - return this.logger.info(`Person with ID ${event.personId} is not in itslearning, ignoring.`); + return this.logger.info( + `[EventID: ${event.eventID}] Person with ID ${event.personId} is not in itslearning, ignoring.`, + ); } - const updatePersonError: Option = await this.itslearningPersonRepo.createOrUpdatePerson({ - id: event.personId, - firstName: event.vorname, - lastName: event.familienname, - username: event.referrer, - institutionRoleType: readPersonResult.institutionRole, - }); + const updatePersonError: Option = await this.itslearningPersonRepo.createOrUpdatePerson( + { + id: event.personId, + firstName: event.vorname, + lastName: event.familienname, + username: event.referrer, + institutionRoleType: readPersonResult.institutionRole, + }, + `${event.eventID}-PERSON-RENAMED-UPDATE`, + ); if (updatePersonError) { - return this.logger.error(`Person with ID ${event.personId} could not be updated in itsLearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Person with ID ${event.personId} could not be updated in itsLearning!`, + ); } - this.logger.info(`Person with ID ${event.personId} updated in itsLearning!`); + this.logger.info(`[EventID: ${event.eventID}] Person with ID ${event.personId} updated in itsLearning!`); }); } @EventHandler(OxUserChangedEvent) public async oxUserChangedEventHandler(event: OxUserChangedEvent): Promise { if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring email update.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring email update.`); } await this.personUpdateMutex.runExclusive(async () => { - this.logger.info(`Received OxUserChangedEvent, ${event.personId}`); + this.logger.info(`[EventID: ${event.eventID}] Received OxUserChangedEvent, ${event.personId}`); const updateError: Option = await this.itslearningPersonRepo.updateEmail( event.personId, event.primaryEmail, + `${event.eventID}-EMAIL-UPDATE`, ); if (updateError) { - this.logger.error(`Could not update E-Mail for person with ID ${event.personId}!`); + this.logger.error( + `[EventID: ${event.eventID}] Could not update E-Mail for person with ID ${event.personId}!`, + ); } else { - this.logger.info(`Updated E-Mail for person with ID ${event.personId}!`); + this.logger.info(`[EventID: ${event.eventID}] Updated E-Mail for person with ID ${event.personId}!`); } }); } @@ -102,10 +115,10 @@ export class ItsLearningPersonsEventHandler { @EventHandler(PersonenkontextUpdatedEvent) public async updatePersonenkontexteEventHandler(event: PersonenkontextUpdatedEvent): Promise { await this.personUpdateMutex.runExclusive(async () => { - this.logger.info(`Received PersonenkontextUpdatedEvent, ${event.person.id}`); + this.logger.info(`[EventID: ${event.eventID}] Received PersonenkontextUpdatedEvent, ${event.person.id}`); if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring event.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); } // Collect all itslearning-orgas @@ -123,30 +136,35 @@ export class ItsLearningPersonsEventHandler { if (currentKontexte.length > 0) { // Person should have itslearning, create/update them as necessary - await this.updatePerson(event.person, currentKontexte); + await this.updatePerson(event.person, currentKontexte, `${event.eventID}-UPDATE-PERSON`); // Synchronize memberships - await this.updateMemberships(event.person.id, currentKontexte); + await this.updateMemberships(event.person.id, currentKontexte, `${event.eventID}-UPDATE-MEMBERSHIPS`); } else { // Delete person - await this.deletePerson(event.person.id); + await this.deletePerson(event.person.id, `${event.eventID}-DELETE-PERSON`); } }); } - public async updateMemberships(personId: PersonID, currentKontexte: PersonenkontextUpdatedData[]): Promise { + public async updateMemberships( + personId: PersonID, + currentKontexte: PersonenkontextUpdatedData[], + eventID: string, + ): Promise { const setMembershipsResult: Result = await this.itslearningMembershipRepo.setMemberships( personId, currentKontexte.map((pk: PersonenkontextUpdatedData) => ({ organisationId: pk.orgaId, role: pk.rolle })), + eventID, ); if (!setMembershipsResult.ok) { this.logger.error( - `Could not set ${currentKontexte.length} memberships for person ${personId}`, + `[EventID: ${eventID}] Could not set ${currentKontexte.length} memberships for person ${personId}`, setMembershipsResult.error, ); } else { - this.logger.info(`Set ${currentKontexte.length} memberships for person ${personId}`); + this.logger.info(`[EventID: ${eventID}] Set ${currentKontexte.length} memberships for person ${personId}`); } } @@ -156,43 +174,47 @@ export class ItsLearningPersonsEventHandler { public async updatePerson( person: PersonenkontextUpdatedPersonData, currentPersonenkontexte: PersonenkontextUpdatedData[], + eventID: string, ): Promise { if (!person.referrer) { - return this.logger.error(`Person with ID ${person.id} has no username!`); + return this.logger.error(`[EventID: ${eventID}] Person with ID ${person.id} has no username!`); } const targetRole: IMSESInstitutionRoleType = rollenartToIMSESInstitutionRole( determineHighestRollenart(currentPersonenkontexte.map((pk: PersonenkontextUpdatedData) => pk.rolle)), ); - const createError: Option = await this.itslearningPersonRepo.createOrUpdatePerson({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: targetRole, - email: person.email, - }); + const createError: Option = await this.itslearningPersonRepo.createOrUpdatePerson( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: targetRole, + email: person.email, + }, + eventID, + ); if (createError) { return this.logger.error( - `Person with ID ${person.id} could not be sent to itsLearning! Error: ${createError.message}`, + `[EventID: ${eventID}] Person with ID ${person.id} could not be sent to itsLearning! Error: ${createError.message}`, ); } - return this.logger.info(`Person with ID ${person.id} created in itsLearning!`); + return this.logger.info(`[EventID: ${eventID}] Person with ID ${person.id} created in itsLearning!`); } /** * Delete this person in itslearning */ - public async deletePerson(personID: PersonID): Promise { + public async deletePerson(personID: PersonID, eventID: string): Promise { const deleteError: Option = await this.itslearningPersonRepo.deletePerson(personID); if (!deleteError) { - this.logger.info(`Person with ID ${personID} deleted.`); + this.logger.info(`[EventID: ${eventID}] Person with ID ${personID} deleted.`); } else { - this.logger.error(`Could not delete person with ID ${personID} from itsLearning.`); + this.logger.error(`[EventID: ${eventID}] Could not delete person with ID ${personID} from itsLearning.`); } } diff --git a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts index 3906fb340..29b6571e0 100644 --- a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts +++ b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts @@ -167,16 +167,22 @@ describe('ItsLearning Persons Event Handler', () => { it('should create or update user', async () => { itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(undefined); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); - - expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: rollenartToIMSESInstitutionRole(rolleWithItslearning.rollenart), - }); - expect(loggerMock.info).toHaveBeenCalledWith(`Updated person with ID ${person.id} in itslearning!`); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); + + expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: rollenartToIMSESInstitutionRole(rolleWithItslearning.rollenart), + }, + `${event.eventID}-SYNC-PERSON`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Updated person with ID ${person.id} in itslearning!`, + ); }); it('should log error if creation failed', async () => { @@ -184,11 +190,12 @@ describe('ItsLearning Persons Event Handler', () => { new ItsLearningError('Error Test'), ); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledTimes(1); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not create/update person with ID ${person.id} in itslearning!`, + `[EventID: ${event.eventID}] Could not create/update person with ID ${person.id} in itslearning!`, ); }); @@ -199,7 +206,8 @@ describe('ItsLearning Persons Event Handler', () => { value: { deleted: 0, updated: 1 }, } satisfies Result); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(itslearningMembershipRepoMock.setMemberships).toHaveBeenCalledWith( person.id, @@ -213,9 +221,10 @@ describe('ItsLearning Persons Event Handler', () => { role: rolleWithItslearning.rollenart, }, ]), + `${event.eventID}-SYNC-PERSON-MEMBERSHIPS`, ); expect(loggerMock.info).toHaveBeenCalledWith( - `Created/Updated 1 and deleted 0 memberships for person with ID ${person.id} to itslearning!`, + `[EventID: ${event.eventID}] Created/Updated 1 and deleted 0 memberships for person with ID ${person.id} to itslearning!`, ); }); @@ -226,10 +235,11 @@ describe('ItsLearning Persons Event Handler', () => { error: new ItsLearningError('Error Test'), } satisfies Result); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not delete person with ID ${person.id} from itslearning!`, + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, ); }); }); @@ -258,18 +268,23 @@ describe('ItsLearning Persons Event Handler', () => { it('should delete person', async () => { itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(undefined); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); - expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith(person.id); + expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith( + person.id, + `${event.eventID}-DELETE`, + ); }); it('should log error if deletion failed', async () => { itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(new ItsLearningError('Error Test')); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not delete person with ID ${person.id} from itslearning!`, + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, ); }); }); @@ -279,18 +294,24 @@ describe('ItsLearning Persons Event Handler', () => { const personId: string = faker.string.uuid(); personRepoMock.findById.mockResolvedValueOnce(undefined); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(personId)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(personId); + await sut.personExternalSystemSyncEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${personId} could not be found!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${personId} could not be found!`, + ); }); it('should log error, if person has no username', async () => { const personId: string = faker.string.uuid(); personRepoMock.findById.mockResolvedValueOnce(DoFactory.createPerson(true, { referrer: undefined })); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(personId)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(personId); + await sut.personExternalSystemSyncEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${personId} has no username!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${personId} has no username!`, + ); }); }); @@ -298,9 +319,12 @@ describe('ItsLearning Persons Event Handler', () => { it('should log info and return', async () => { sut.ENABLED = false; - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(faker.string.uuid())); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(faker.string.uuid()); + await sut.personExternalSystemSyncEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Not enabled, ignoring event.`, + ); }); }); }); diff --git a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts index af60a25dc..fa7727eb7 100644 --- a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts +++ b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts @@ -51,21 +51,23 @@ export class ItsLearningSyncEventHandler { @EventHandler(PersonExternalSystemsSyncEvent) public async personExternalSystemSyncEventHandler(event: PersonExternalSystemsSyncEvent): Promise { - this.logger.info(`Received PersonExternalSystemsSyncEvent, ${event.personId}`); + this.logger.info(`[EventID: ${event.eventID}] Received PersonExternalSystemsSyncEvent, ${event.personId}`); if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring event.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); } // Retrieve the person from the DB const person: Option> = await this.personRepo.findById(event.personId); if (!person) { - return this.logger.error(`Person with ID ${event.personId} could not be found!`); + return this.logger.error( + `[EventID: ${event.eventID}] Person with ID ${event.personId} could not be found!`, + ); } // Check if person has a username if (!person.referrer) { - return this.logger.error(`Person with ID ${event.personId} has no username!`); + return this.logger.error(`[EventID: ${event.eventID}] Person with ID ${event.personId} has no username!`); } // Get all personenkontexte for this person @@ -123,20 +125,25 @@ export class ItsLearningSyncEventHandler { ); // Create or update the person in itslearning - const creationError: Option = await this.itslearningPersonRepo.createOrUpdatePerson({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: rollenartToIMSESInstitutionRole(targetRole), - email: person.email, - }); + const creationError: Option = await this.itslearningPersonRepo.createOrUpdatePerson( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: rollenartToIMSESInstitutionRole(targetRole), + email: person.email, + }, + `${event.eventID}-SYNC-PERSON`, + ); if (creationError) { - return this.logger.error(`Could not create/update person with ID ${person.id} in itslearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not create/update person with ID ${person.id} in itslearning!`, + ); } - this.logger.info(`Updated person with ID ${person.id} in itslearning!`); + this.logger.info(`[EventID: ${event.eventID}] Updated person with ID ${person.id} in itslearning!`); // Set the memberships const memberships: SetMembershipParams[] = relevantKontexte.map((pk: Personenkontext) => ({ @@ -147,25 +154,36 @@ export class ItsLearningSyncEventHandler { })); const setMembershipsResult: Result = - await this.itslearningMembershipRepo.setMemberships(person.id, memberships); + await this.itslearningMembershipRepo.setMemberships( + person.id, + memberships, + `${event.eventID}-SYNC-PERSON-MEMBERSHIPS`, + ); if (!setMembershipsResult.ok) { - return this.logger.error(`Could not delete person with ID ${person.id} from itslearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, + ); } this.logger.info( - `Created/Updated ${setMembershipsResult.value.updated} and deleted ${setMembershipsResult.value.deleted} memberships for person with ID ${person.id} to itslearning!`, + `[EventID: ${event.eventID}] Created/Updated ${setMembershipsResult.value.updated} and deleted ${setMembershipsResult.value.deleted} memberships for person with ID ${person.id} to itslearning!`, ); } else { this.logger.info( - `Deleting person with ID ${person.id} from itslearning (if they exist), because they have no relevant personenkontexte!`, + `[EventID: ${event.eventID}] Deleting person with ID ${person.id} from itslearning (if they exist), because they have no relevant personenkontexte!`, ); // We don't have any relevant personenkontexte for this person, so we delete it - const deleteError: Option = await this.itslearningPersonRepo.deletePerson(person.id); + const deleteError: Option = await this.itslearningPersonRepo.deletePerson( + person.id, + `${event.eventID}-DELETE`, + ); if (deleteError) { - return this.logger.error(`Could not delete person with ID ${person.id} from itslearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, + ); } } } diff --git a/src/modules/itslearning/itslearning.service.spec.ts b/src/modules/itslearning/itslearning.service.spec.ts index f0b49b227..8675e6c5d 100644 --- a/src/modules/itslearning/itslearning.service.spec.ts +++ b/src/modules/itslearning/itslearning.service.spec.ts @@ -58,6 +58,26 @@ describe('ItsLearningIMSESService', () => { }, ); }); + it('should include syncID if given', async () => { + const mockAction: DeepMocked> = createMock>(); + mockAction.buildRequest.mockReturnValueOnce({}); + mockAction.action = 'testAction'; + httpServiceMock.post.mockReturnValueOnce(of({} as AxiosResponse)); + const syncID: string = 'sync-id-test'; + + await sut.send(mockAction, syncID); + + expect(httpServiceMock.post).toHaveBeenCalledWith( + 'https://itslearning-test.example.com', + expect.stringContaining(syncID), + { + headers: { + 'Content-Type': 'text/xml;charset=UTF-8', + SOAPAction: `"testAction"`, + }, + }, + ); + }); it('should call parseResponse of action and return result', async () => { const mockAction: DeepMocked> = createMock>(); diff --git a/src/modules/itslearning/itslearning.service.ts b/src/modules/itslearning/itslearning.service.ts index d3c5c60e9..7a88e1f0d 100644 --- a/src/modules/itslearning/itslearning.service.ts +++ b/src/modules/itslearning/itslearning.service.ts @@ -10,6 +10,7 @@ import { ItsLearningConfig, ServerConfig } from '../../shared/config/index.js'; import { DomainError, ItsLearningError } from '../../shared/error/index.js'; import { IMSESAction } from './actions/base-action.js'; import { IMSESMassAction } from './actions/base-mass-action.js'; +import { IMS_MESS_BIND_SCHEMA } from './schemas.js'; @Injectable() export class ItsLearningIMSESService { @@ -34,9 +35,10 @@ export class ItsLearningIMSESService { public async send( action: IMSESAction | IMSESMassAction, + syncId?: string, ): Promise> { const body: object = action.buildRequest(); - const message: string = this.createMessage(body); + const message: string = this.createMessage(body, syncId); try { const response: AxiosResponse = await lastValueFrom( @@ -57,25 +59,29 @@ export class ItsLearningIMSESService { } } - private createMessage(body: object): string { + private createMessage(body: object, syncId?: string): string { return this.xmlBuilder.build({ 'soapenv:Envelope': { '@_xmlns:soapenv': 'http://schemas.xmlsoap.org/soap/envelope/', - 'soapenv:Header': this.createSecurityObject(), + 'soapenv:Header': this.createSecurityObject(syncId), 'soapenv:Body': body, }, }) as string; } - private createSecurityObject(): object { + private createSecurityObject(syncId?: string): object { const now: string = new Date().toISOString(); const nHash: Hash = createHash('sha1'); nHash.update(now + Math.random()); const nonce: string = nHash.digest('base64'); return { + 'ims:syncRequestHeaderInfo': syncId && { + '@_xmlns:ims': IMS_MESS_BIND_SCHEMA, + 'ims:messageIdentifier': syncId, + }, 'wsse:Security': { '@_xmlns:wsse': 'http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd', '@_xmlns:wsu': 'http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd', diff --git a/src/modules/itslearning/repo/itslearning-group.repo.spec.ts b/src/modules/itslearning/repo/itslearning-group.repo.spec.ts index 4f6a07abe..b2846fff9 100644 --- a/src/modules/itslearning/repo/itslearning-group.repo.spec.ts +++ b/src/modules/itslearning/repo/itslearning-group.repo.spec.ts @@ -46,11 +46,15 @@ describe('Itslearning Group Repo', () => { describe('readGroup', () => { it('should call the itslearning API', async () => { const organisationId: string = faker.string.uuid(); + const syncID: string = faker.string.uuid(); - await sut.readGroup(organisationId); + await sut.readGroup(organisationId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: organisationId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadGroupAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ id: organisationId }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadGroupAction), syncID); }); it('should return the result', async () => { @@ -93,11 +97,15 @@ describe('Itslearning Group Repo', () => { ok: true, value: undefined, }); // CreateGroupAction + const syncID: string = faker.string.uuid(); - await sut.createOrUpdateGroup(createParams); + await sut.createOrUpdateGroup(createParams, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: createParams })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: createParams }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupAction), syncID); }); it('should not return error on success', async () => { @@ -150,11 +158,15 @@ describe('Itslearning Group Repo', () => { ok: true, value: undefined, }); // CreateGroupAction + const syncID: string = faker.string.uuid(); - await sut.createOrUpdateGroups(createParams); + await sut.createOrUpdateGroups(createParams, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: createParams })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupsAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: createParams }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupsAction), syncID); }); it('should not return error on success', async () => { @@ -204,11 +216,15 @@ describe('Itslearning Group Repo', () => { ok: true, value: undefined, }); // DeletePersonAction + const syncID: string = faker.string.uuid(); - await sut.deleteGroup(organisationId); + await sut.deleteGroup(organisationId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: organisationId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteGroupAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ id: organisationId }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteGroupAction), syncID); }); it('should not return error on success', async () => { diff --git a/src/modules/itslearning/repo/itslearning-group.repo.ts b/src/modules/itslearning/repo/itslearning-group.repo.ts index 9a69f075e..f9ee4bfff 100644 --- a/src/modules/itslearning/repo/itslearning-group.repo.ts +++ b/src/modules/itslearning/repo/itslearning-group.repo.ts @@ -13,9 +13,10 @@ import { ItsLearningIMSESService } from '../itslearning.service.js'; export class ItslearningGroupRepo { public constructor(private readonly itslearningService: ItsLearningIMSESService) {} - public async readGroup(id: OrganisationID): Promise> { + public async readGroup(id: OrganisationID, syncId?: string): Promise> { const groupResult: Result = await this.itslearningService.send( new ReadGroupAction(id), + syncId, ); if (!groupResult.ok) { @@ -25,10 +26,10 @@ export class ItslearningGroupRepo { return groupResult.value; } - public async createOrUpdateGroup(params: CreateGroupParams): Promise> { + public async createOrUpdateGroup(params: CreateGroupParams, syncId?: string): Promise> { const createAction: CreateGroupAction = new CreateGroupAction(params); - const createResult: Result = await this.itslearningService.send(createAction); + const createResult: Result = await this.itslearningService.send(createAction, syncId); if (!createResult.ok) { return createResult.error; @@ -37,10 +38,10 @@ export class ItslearningGroupRepo { return undefined; } - public async createOrUpdateGroups(params: CreateGroupParams[]): Promise> { + public async createOrUpdateGroups(params: CreateGroupParams[], syncId?: string): Promise> { const createAction: CreateGroupsAction = new CreateGroupsAction(params); - const createResult: Result = await this.itslearningService.send(createAction); + const createResult: Result = await this.itslearningService.send(createAction, syncId); if (!createResult.ok) { return createResult.error; @@ -49,8 +50,11 @@ export class ItslearningGroupRepo { return undefined; } - public async deleteGroup(id: OrganisationID): Promise> { - const deleteResult: Result = await this.itslearningService.send(new DeleteGroupAction(id)); + public async deleteGroup(id: OrganisationID, syncId?: string): Promise> { + const deleteResult: Result = await this.itslearningService.send( + new DeleteGroupAction(id), + syncId, + ); if (!deleteResult.ok) { return deleteResult.error; diff --git a/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts b/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts index c0bf749d7..16e8cfbb4 100644 --- a/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts +++ b/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts @@ -45,11 +45,15 @@ describe('Itslearning Person Repo', () => { describe('readMembershipsForPerson', () => { it('should call the itslearning API', async () => { const personId: string = faker.string.uuid(); + const syncID: string = faker.string.uuid(); - await sut.readMembershipsForPerson(personId); + await sut.readMembershipsForPerson(personId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadMembershipsForPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.any(ReadMembershipsForPersonAction), + syncID, + ); }); it('should return the result', async () => { @@ -83,11 +87,15 @@ describe('Itslearning Person Repo', () => { roleType: faker.helpers.enumValue(IMSESRoleType), }, ]; + const syncID: string = faker.string.uuid(); - await sut.createMemberships(memberships); + await sut.createMemberships(memberships, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: memberships })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateMembershipsAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: memberships }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateMembershipsAction), syncID); }); it('should not return error on success', async () => { @@ -133,11 +141,15 @@ describe('Itslearning Person Repo', () => { describe('removeMemberships', () => { it('should call the itslearning API', async () => { const membershipIDs: string[] = [faker.string.uuid()]; + const syncID: string = faker.string.uuid(); - await sut.removeMemberships(membershipIDs); + await sut.removeMemberships(membershipIDs, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ membershipIDs })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteMembershipsAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ membershipIDs }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteMembershipsAction), syncID); }); it('should not return error on success', async () => { @@ -171,11 +183,15 @@ describe('Itslearning Person Repo', () => { it('should read current memberships for person', async () => { const personId: string = faker.string.uuid(); itsLearningServiceMock.send.mockResolvedValueOnce({ ok: true, value: [] }); // Read Memberships + const syncID: string = faker.string.uuid(); - await sut.setMemberships(personId, []); + await sut.setMemberships(personId, [], syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadMembershipsForPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.any(ReadMembershipsForPersonAction), + syncID, + ); }); it('should abort if memberships can not be read', async () => { diff --git a/src/modules/itslearning/repo/itslearning-membership.repo.ts b/src/modules/itslearning/repo/itslearning-membership.repo.ts index 51bcc2522..e50be729a 100644 --- a/src/modules/itslearning/repo/itslearning-membership.repo.ts +++ b/src/modules/itslearning/repo/itslearning-membership.repo.ts @@ -37,14 +37,23 @@ export class ItslearningMembershipRepo { this.ROOT_NAMES = [itslearningConfig.ROOT, itslearningConfig.ROOT_OEFFENTLICH, itslearningConfig.ROOT_ERSATZ]; } - public readMembershipsForPerson(personId: PersonID): Promise> { - return this.itslearningService.send(new ReadMembershipsForPersonAction(personId)); + public readMembershipsForPerson( + personId: PersonID, + syncId?: string, + ): Promise> { + return this.itslearningService.send(new ReadMembershipsForPersonAction(personId), syncId); } - public async createMemberships(memberships: CreateMembershipParams[]): Promise> { + public async createMemberships( + memberships: CreateMembershipParams[], + syncId?: string, + ): Promise> { const createMembershipsAction: CreateMembershipsAction = new CreateMembershipsAction(memberships); - const createResult: Result = await this.itslearningService.send(createMembershipsAction); + const createResult: Result = await this.itslearningService.send( + createMembershipsAction, + syncId, + ); if (!createResult.ok) { return createResult.error; @@ -53,9 +62,10 @@ export class ItslearningMembershipRepo { return undefined; } - public async removeMemberships(membershipIDs: string[]): Promise> { + public async removeMemberships(membershipIDs: string[], syncId?: string): Promise> { const deleteResult: Result = await this.itslearningService.send( new DeleteMembershipsAction(membershipIDs), + syncId, ); if (!deleteResult.ok) { @@ -68,14 +78,17 @@ export class ItslearningMembershipRepo { public async setMemberships( personId: PersonID, memberships: SetMembershipParams[], + syncId?: string, ): Promise> { const returnResult: SetMembershipsResult = { updated: 0, deleted: 0, }; - const currentMemberships: Result = - await this.readMembershipsForPerson(personId); + const currentMemberships: Result = await this.readMembershipsForPerson( + personId, + syncId, + ); if (!currentMemberships.ok) { return { @@ -111,6 +124,7 @@ export class ItslearningMembershipRepo { if (membershipsToBeRemoved.length > 0) { const deleteError: Option = await this.removeMemberships( membershipsToBeRemoved.map((mr: MembershipResponse) => mr.id), + syncId, ); if (deleteError) { @@ -134,7 +148,7 @@ export class ItslearningMembershipRepo { ); if (membershipsToCreateOrUpdate.length > 0) { - const createError: Option = await this.createMemberships(membershipsToCreateOrUpdate); + const createError: Option = await this.createMemberships(membershipsToCreateOrUpdate, syncId); if (createError) { this.logger.error( diff --git a/src/modules/itslearning/repo/itslearning-person.repo.spec.ts b/src/modules/itslearning/repo/itslearning-person.repo.spec.ts index 5243a6bb2..23e25b8ed 100644 --- a/src/modules/itslearning/repo/itslearning-person.repo.spec.ts +++ b/src/modules/itslearning/repo/itslearning-person.repo.spec.ts @@ -45,11 +45,12 @@ describe('Itslearning Person Repo', () => { describe('readPerson', () => { it('should call the itslearning API', async () => { const personId: string = faker.string.uuid(); + const syncID: string = faker.string.uuid(); - await sut.readPerson(personId); + await sut.readPerson(personId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadPersonAction), syncID); }); it('should return the result', async () => { @@ -101,11 +102,16 @@ describe('Itslearning Person Repo', () => { ok: true, value: undefined, }); // CreatePersonAction + const syncID: string = faker.string.uuid(); - await sut.updateEmail(personId, email); + await sut.updateEmail(personId, email, syncID); - expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(1, expect.objectContaining({ id: personId })); - expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(1, expect.any(ReadPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ id: personId }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(1, expect.any(ReadPersonAction), syncID); expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith( 2, expect.objectContaining({ @@ -118,8 +124,9 @@ describe('Itslearning Person Repo', () => { email, }, }), + syncID, ); - expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(2, expect.any(CreatePersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(2, expect.any(CreatePersonAction), syncID); }); it('should not update email, if person was not found', async () => { @@ -150,11 +157,15 @@ describe('Itslearning Person Repo', () => { ok: true, value: undefined, }); // CreatePersonAction + const syncID: string = faker.string.uuid(); - await sut.createOrUpdatePerson(createParams); + await sut.createOrUpdatePerson(createParams, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: createParams })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreatePersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: createParams }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreatePersonAction), syncID); }); it('should not return error on success', async () => { @@ -202,11 +213,12 @@ describe('Itslearning Person Repo', () => { ok: true, value: undefined, }); // DeletePersonAction + const syncID: string = faker.string.uuid(); - await sut.deletePerson(personId); + await sut.deletePerson(personId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeletePersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeletePersonAction), syncID); }); it('should not return error on success', async () => { diff --git a/src/modules/itslearning/repo/itslearning-person.repo.ts b/src/modules/itslearning/repo/itslearning-person.repo.ts index 219da1013..fc52458bd 100644 --- a/src/modules/itslearning/repo/itslearning-person.repo.ts +++ b/src/modules/itslearning/repo/itslearning-person.repo.ts @@ -11,9 +11,10 @@ import { ItsLearningIMSESService } from '../itslearning.service.js'; export class ItslearningPersonRepo { public constructor(private readonly itslearningService: ItsLearningIMSESService) {} - public async readPerson(id: PersonID): Promise> { + public async readPerson(id: PersonID, syncId?: string): Promise> { const personResult: Result = await this.itslearningService.send( new ReadPersonAction(id), + syncId, ); if (!personResult.ok) { @@ -23,10 +24,10 @@ export class ItslearningPersonRepo { return personResult.value; } - public async createOrUpdatePerson(params: CreatePersonParams): Promise> { + public async createOrUpdatePerson(params: CreatePersonParams, syncId?: string): Promise> { const createAction: CreatePersonAction = new CreatePersonAction(params); - const createResult: Result = await this.itslearningService.send(createAction); + const createResult: Result = await this.itslearningService.send(createAction, syncId); if (!createResult.ok) { return createResult.error; @@ -35,24 +36,30 @@ export class ItslearningPersonRepo { return undefined; } - public async updateEmail(personId: PersonID, email: string): Promise> { - const person: Option = await this.readPerson(personId); + public async updateEmail(personId: PersonID, email: string, syncId?: string): Promise> { + const person: Option = await this.readPerson(personId, syncId); // Person is not in itslearning, should not update the e-mail if (!person) return new ItsLearningError(`[updateEmail] person with ID ${personId} not found.`); - return this.createOrUpdatePerson({ - id: personId, - firstName: person.firstName, - lastName: person.lastName, - institutionRoleType: person.institutionRole, - username: person.username, - email, - }); + return this.createOrUpdatePerson( + { + id: personId, + firstName: person.firstName, + lastName: person.lastName, + institutionRoleType: person.institutionRole, + username: person.username, + email, + }, + syncId, + ); } - public async deletePerson(id: PersonID): Promise> { - const deleteResult: Result = await this.itslearningService.send(new DeletePersonAction(id)); + public async deletePerson(id: PersonID, syncId?: string): Promise> { + const deleteResult: Result = await this.itslearningService.send( + new DeletePersonAction(id), + syncId, + ); if (!deleteResult.ok) { return deleteResult.error; diff --git a/src/shared/events/base-event.ts b/src/shared/events/base-event.ts index 6b6e6658f..fd7e18504 100644 --- a/src/shared/events/base-event.ts +++ b/src/shared/events/base-event.ts @@ -1,3 +1,5 @@ +import { randomUUID } from 'crypto'; + declare const EVENT_MARKER: unique symbol; export abstract class BaseEvent { @@ -6,4 +8,7 @@ export abstract class BaseEvent { public readonly [EVENT_MARKER]!: never; public readonly createdAt: Date = new Date(); + + // For now generate random ID. In the future this should be replaced with the sessions correlation-ID or similar + public readonly eventID: string = randomUUID(); } From 7c955d4080d2d467758dc61fe5ab2b45e4796bb2 Mon Sep 17 00:00:00 2001 From: Cornelius <144817755+DPDS93CT@users.noreply.github.com> Date: Fri, 13 Dec 2024 06:09:40 +0100 Subject: [PATCH 2/2] SPSH-1583: Refaktorisierung OX-User-Blacklist (#829) * create OxUserBlacklistEntry, OxUserBlacklistRepo * use OxUserBlacklistRepo in UsernameGeneratorService * fix test-cases which use UsernameGenerator to provide OxUserBlacklistRepo * add test-cases for OxUserBlacklistRepo * fix imports in modules and test-files * rm unused import --- src/console/console.module.spec.ts | 5 +- src/console/console.module.ts | 2 - ...le-mocked-db-seed-repo.integration-spec.ts | 2 + .../db-seed.console.integration-spec.ts | 3 +- .../db-seed.service.integration-spec.ts | 3 +- .../email/persistence/email.repo.spec.ts | 2 + .../person/domain/ox-user-blacklist-entry.ts | 27 +++ ...name-generator.service.integration-spec.ts | 22 +- .../domain/username-generator.service.ts | 37 ++- .../ox-user-blacklist.repo.spec.ts | 216 ++++++++++++++++++ .../persistence/ox-user-blacklist.repo.ts | 104 +++++++++ src/modules/person/person.module.ts | 2 + ...onenkontext.controller.integration-spec.ts | 4 + ...m-personenkontext.repo.integration-spec.ts | 2 + ...m-personenkontext.repo.integration-spec.ts | 2 + ...kontext-specifications.integration-spec.ts | 4 + src/shared/types/ox-ids.types.ts | 3 + 17 files changed, 411 insertions(+), 29 deletions(-) create mode 100644 src/modules/person/domain/ox-user-blacklist-entry.ts create mode 100644 src/modules/person/persistence/ox-user-blacklist.repo.spec.ts create mode 100644 src/modules/person/persistence/ox-user-blacklist.repo.ts diff --git a/src/console/console.module.spec.ts b/src/console/console.module.spec.ts index 89818d0a7..c0f0bf291 100644 --- a/src/console/console.module.spec.ts +++ b/src/console/console.module.spec.ts @@ -2,13 +2,16 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ConsoleModule } from './console.module.js'; import { DbConsole } from './db.console.js'; import { DbInitConsole } from './db-init.console.js'; +import { LoggingTestModule } from '../../test/utils/index.js'; +import { PersonModule } from '../modules/person/person.module.js'; +import { KeycloakAdministrationModule } from '../modules/keycloak-administration/keycloak-administration.module.js'; describe('ConsoleModule', () => { let module: TestingModule; beforeAll(async () => { module = await Test.createTestingModule({ - imports: [ConsoleModule], + imports: [PersonModule, KeycloakAdministrationModule, ConsoleModule, LoggingTestModule], }).compile(); }); diff --git a/src/console/console.module.ts b/src/console/console.module.ts index 512ed8491..6aba134c7 100644 --- a/src/console/console.module.ts +++ b/src/console/console.module.ts @@ -11,7 +11,6 @@ import { DbConsole } from './db.console.js'; import { DbInitConsole } from './db-init.console.js'; import { LoggerModule } from '../core/logging/logger.module.js'; import { KeycloakAdministrationModule } from '../modules/keycloak-administration/keycloak-administration.module.js'; -import { UsernameGeneratorService } from '../modules/person/domain/username-generator.service.js'; import { KeycloakConfigModule } from '../modules/keycloak-administration/keycloak-config.module.js'; import { OrganisationModule } from '../modules/organisation/organisation.module.js'; import { RolleModule } from '../modules/rolle/rolle.module.js'; @@ -93,7 +92,6 @@ import { KeycloakConsoleModule } from './keycloak/keycloak-console.module.js'; DbInitMigrationConsole, DbCreateMigrationConsole, DbApplyMigrationConsole, - UsernameGeneratorService, DbSeedDataGeneratorConsole, ], }) diff --git a/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts b/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts index 1d31758f2..6a9447b2f 100644 --- a/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts +++ b/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts @@ -25,6 +25,7 @@ import { DBiamPersonenkontextService } from '../../modules/personenkontext/domai import { DbSeedReferenceRepo } from './repo/db-seed-reference.repo.js'; import { PersonenKontextModule } from '../../modules/personenkontext/personenkontext.module.js'; import { LdapClient } from '../../core/ldap/domain/ldap-client.js'; +import { OxUserBlacklistRepo } from '../../modules/person/persistence/ox-user-blacklist.repo.js'; describe('DbSeedConsoleMockedDbSeedRepo', () => { let module: TestingModule; @@ -50,6 +51,7 @@ describe('DbSeedConsoleMockedDbSeedRepo', () => { providers: [ UsernameGeneratorService, DBiamPersonenkontextRepo, + OxUserBlacklistRepo, DbSeedConsole, DbSeedService, DBiamPersonenkontextService, diff --git a/src/console/dbseed/db-seed.console.integration-spec.ts b/src/console/dbseed/db-seed.console.integration-spec.ts index b449da2d8..67c12f9b6 100644 --- a/src/console/dbseed/db-seed.console.integration-spec.ts +++ b/src/console/dbseed/db-seed.console.integration-spec.ts @@ -19,6 +19,7 @@ import { RolleModule } from '../../modules/rolle/rolle.module.js'; import { ServiceProviderModule } from '../../modules/service-provider/service-provider.module.js'; import { DbSeedModule } from './db-seed.module.js'; import { PersonenKontextModule } from '../../modules/personenkontext/personenkontext.module.js'; +import { OxUserBlacklistRepo } from '../../modules/person/persistence/ox-user-blacklist.repo.js'; describe('DbSeedConsoleIntegration', () => { let module: TestingModule; @@ -41,7 +42,7 @@ describe('DbSeedConsoleIntegration', () => { ServiceProviderModule, PersonenKontextModule, ], - providers: [UsernameGeneratorService, DBiamPersonenkontextRepo], + providers: [UsernameGeneratorService, DBiamPersonenkontextRepo, OxUserBlacklistRepo], }) .overrideModule(KeycloakConfigModule) .useModule(KeycloakConfigTestModule.forRoot({ isKeycloakRequired: true })) diff --git a/src/console/dbseed/domain/db-seed.service.integration-spec.ts b/src/console/dbseed/domain/db-seed.service.integration-spec.ts index db936bad3..cf1ae8f90 100644 --- a/src/console/dbseed/domain/db-seed.service.integration-spec.ts +++ b/src/console/dbseed/domain/db-seed.service.integration-spec.ts @@ -21,6 +21,7 @@ import { PersonModule } from '../../../modules/person/person.module.js'; import { DbSeedModule } from '../db-seed.module.js'; import { PersonenKontextModule } from '../../../modules/personenkontext/personenkontext.module.js'; import { VornameForPersonWithTrailingSpaceError } from '../../../modules/person/domain/vorname-with-trailing-space.error.js'; +import { OxUserBlacklistRepo } from '../../../modules/person/persistence/ox-user-blacklist.repo.js'; describe('DbSeedServiceIntegration', () => { let module: TestingModule; @@ -42,7 +43,7 @@ describe('DbSeedServiceIntegration', () => { LoggingTestModule, PersonenKontextModule, ], - providers: [UsernameGeneratorService, DBiamPersonenkontextRepo], + providers: [UsernameGeneratorService, DBiamPersonenkontextRepo, OxUserBlacklistRepo], }) .overrideModule(KeycloakConfigModule) .useModule(KeycloakConfigTestModule.forRoot({ isKeycloakRequired: true })) diff --git a/src/modules/email/persistence/email.repo.spec.ts b/src/modules/email/persistence/email.repo.spec.ts index da0a88c36..c62b0e662 100644 --- a/src/modules/email/persistence/email.repo.spec.ts +++ b/src/modules/email/persistence/email.repo.spec.ts @@ -27,6 +27,7 @@ import { PersonAlreadyHasEnabledEmailAddressError } from '../error/person-alread import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { PersonEmailResponse } from '../../person/api/person-email-response.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; import { OrganisationID, PersonID } from '../../../shared/types/aggregate-ids.types.js'; describe('EmailRepo', () => { @@ -45,6 +46,7 @@ describe('EmailRepo', () => { imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], providers: [ UsernameGeneratorService, + OxUserBlacklistRepo, EmailRepo, EmailFactory, PersonFactory, diff --git a/src/modules/person/domain/ox-user-blacklist-entry.ts b/src/modules/person/domain/ox-user-blacklist-entry.ts new file mode 100644 index 000000000..95c4adcbb --- /dev/null +++ b/src/modules/person/domain/ox-user-blacklist-entry.ts @@ -0,0 +1,27 @@ +import { OXEmail, OXUserName } from '../../../shared/types/ox-ids.types.js'; + +export class OxUserBlacklistEntry { + public constructor( + public id: Persisted, + public readonly createdAt: Persisted, + public readonly updatedAt: Persisted, + public email: OXEmail, + public name: string, + public username: OXUserName, + ) {} + + public static construct( + id: string, + createdAt: Date, + updatedAt: Date, + email: OXEmail, + name: string, + username: OXUserName, + ): OxUserBlacklistEntry { + return new OxUserBlacklistEntry(id, createdAt, updatedAt, email, name, username); + } + + public static createNew(email: OXEmail, name: string, username: OXUserName): OxUserBlacklistEntry { + return new OxUserBlacklistEntry(undefined, undefined, undefined, email, name, username); + } +} diff --git a/src/modules/person/domain/username-generator.service.integration-spec.ts b/src/modules/person/domain/username-generator.service.integration-spec.ts index 77e00397b..29d1046dc 100644 --- a/src/modules/person/domain/username-generator.service.integration-spec.ts +++ b/src/modules/person/domain/username-generator.service.integration-spec.ts @@ -18,11 +18,14 @@ import { ConfigTestModule, } from '../../../../test/utils/index.js'; import { MikroORM } from '@mikro-orm/core'; +import { OxUserBlacklistRepo } from '../persistence/ox-user-blacklist.repo.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; -describe('The UsernameGenerator Service', () => { +describe('UsernameGeneratorService', () => { let module: TestingModule; let service: UsernameGeneratorService; let kcUserService: DeepMocked; + let loggerMock: DeepMocked; let em: EntityManager; let orm: MikroORM; @@ -31,12 +34,21 @@ describe('The UsernameGenerator Service', () => { imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], providers: [ UsernameGeneratorService, - { provide: KeycloakUserService, useValue: createMock() }, + OxUserBlacklistRepo, + { + provide: KeycloakUserService, + useValue: createMock(), + }, + { + provide: ClassLogger, + useValue: createMock(), + }, ], }).compile(); orm = module.get(MikroORM); service = module.get(UsernameGeneratorService); kcUserService = module.get(KeycloakUserService); + loggerMock = module.get(ClassLogger); em = module.get(EntityManager); await DatabaseTestModule.setupDatabase(orm); @@ -127,6 +139,7 @@ describe('The UsernameGenerator Service', () => { .mockResolvedValueOnce({ ok: true, value: createMock>() }) .mockResolvedValueOnce({ ok: false, error: new EntityNotFoundError('Not found') }); const generatedUsername: Result = await service.generateUsername('Max', 'Meyer'); + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:mmeyer1`); expect(generatedUsername).toEqual({ ok: true, value: 'mmeyer1' }); }); @@ -137,6 +150,7 @@ describe('The UsernameGenerator Service', () => { } else return Promise.resolve({ ok: false, error: new EntityNotFoundError('Not found') }); }); const generatedUsername: Result = await service.generateUsername('Max', 'Meyer'); + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:mmeyer2`); expect(generatedUsername).toEqual({ ok: true, value: 'mmeyer2' }); }); @@ -148,6 +162,7 @@ describe('The UsernameGenerator Service', () => { .mockResolvedValueOnce({ ok: false, error: new EntityNotFoundError('Not found') }) .mockResolvedValueOnce({ ok: true, value: createMock>() }); const generatedUsername: Result = await service.generateUsername('Renate', 'Bergmann'); + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:rbergmann3`); expect(generatedUsername).toEqual({ ok: true, value: 'rbergmann3' }); }); @@ -156,6 +171,7 @@ describe('The UsernameGenerator Service', () => { await expect(service.generateUsername('Maximilian', 'Mustermann')).rejects.toStrictEqual( new KeycloakClientError('Could not reach'), ); + expect(loggerMock.info).toHaveBeenCalledTimes(0); }); it('should return error if username can not be generated (cleaned names are of length 0)', async () => { @@ -199,6 +215,8 @@ describe('The UsernameGenerator Service', () => { const generatedUsername: Result = await service.generateUsername('Max', 'Meyer'); // Assert: The generated username should have the counter appended + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:mmeyer1`); + expect(generatedUsername).toEqual({ ok: true, value: 'mmeyer1' }); }); }); diff --git a/src/modules/person/domain/username-generator.service.ts b/src/modules/person/domain/username-generator.service.ts index 0a869038e..dbe0148b0 100644 --- a/src/modules/person/domain/username-generator.service.ts +++ b/src/modules/person/domain/username-generator.service.ts @@ -8,14 +8,16 @@ import { InvalidAttributeLengthError, } from '../../../shared/error/index.js'; import { isDIN91379A, toDIN91379SearchForm } from '../../../shared/util/din-91379-validation.js'; -import { OxUserBlacklistEntity } from '../persistence/ox-user-blacklist.entity.js'; -import { EntityManager } from '@mikro-orm/postgresql'; +import { OxUserBlacklistRepo } from '../persistence/ox-user-blacklist.repo.js'; +import { OxUserBlacklistEntry } from './ox-user-blacklist-entry.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; @Injectable() export class UsernameGeneratorService { public constructor( - private readonly em: EntityManager, + private readonly logger: ClassLogger, private kcUserService: KeycloakUserService, + private oxUserBlacklistRepo: OxUserBlacklistRepo, ) {} public async generateUsername(firstname: string, lastname: string): Promise> { @@ -80,22 +82,17 @@ export class UsernameGeneratorService { while (await this.usernameExists(calculatedUsername + counter)) { counter = counter + 1; } - /* eslint-disable no-await-in-loop */ - return calculatedUsername + counter; - } - - public async findByOxUsername(username: string): Promise { - const person: Option = await this.em.findOne(OxUserBlacklistEntity, { - username: username, - }); - if (person) { - return person; - } + this.logger.info(`Next Available Username Is:${calculatedUsername + counter}`); - return null; + return calculatedUsername + counter; } - public async usernameExists(username: string): Promise { + /** + * This method can throw errors e.g. if Keycloak search fails. + * @param username + * @private + */ + private async usernameExists(username: string): Promise { // Check Keycloak const searchResult: Result, DomainError> = await this.kcUserService.findOne({ username }); if (searchResult.ok) { @@ -105,12 +102,8 @@ export class UsernameGeneratorService { } // Check OX Blacklist for the username. If it exists then return true. - const oxUser: OxUserBlacklistEntity | null = await this.findByOxUsername(username); - - if (oxUser) { - return true; // Username exists in the blacklist - } + const oxUser: Option> = await this.oxUserBlacklistRepo.findByOxUsername(username); - return false; // Username is available + return !!oxUser; } } diff --git a/src/modules/person/persistence/ox-user-blacklist.repo.spec.ts b/src/modules/person/persistence/ox-user-blacklist.repo.spec.ts new file mode 100644 index 000000000..f6568268c --- /dev/null +++ b/src/modules/person/persistence/ox-user-blacklist.repo.spec.ts @@ -0,0 +1,216 @@ +import { faker } from '@faker-js/faker'; +import { Test, TestingModule } from '@nestjs/testing'; +import { + ConfigTestModule, + DatabaseTestModule, + DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, +} from '../../../../test/utils/index.js'; +import { EntityManager, MikroORM } from '@mikro-orm/core'; +import { mapAggregateToData, OxUserBlacklistRepo } from './ox-user-blacklist.repo.js'; +import { OxUserBlacklistEntry } from '../domain/ox-user-blacklist-entry.js'; +import { OxUserBlacklistEntity } from './ox-user-blacklist.entity.js'; +import { OXEmail, OXUserName } from '../../../shared/types/ox-ids.types.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; +import { createMock } from '@golevelup/ts-jest'; +import { DomainError } from '../../../shared/error/domain.error.js'; +import { EntityNotFoundError } from '../../../shared/error/entity-not-found.error.js'; + +describe('OxUserBlacklistRepo', () => { + let module: TestingModule; + let sut: OxUserBlacklistRepo; + let orm: MikroORM; + let em: EntityManager; + + beforeAll(async () => { + module = await Test.createTestingModule({ + imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], + providers: [ + OxUserBlacklistRepo, + { + provide: ClassLogger, + useValue: createMock(), + }, + ], + }).compile(); + sut = module.get(OxUserBlacklistRepo); + orm = module.get(MikroORM); + em = module.get(EntityManager); + + await DatabaseTestModule.setupDatabase(orm); + }, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS); + + async function createEntity(email?: OXEmail, name?: string, username?: OXUserName): Promise { + const oxUserBlacklistEntity: OxUserBlacklistEntity = new OxUserBlacklistEntity(); + oxUserBlacklistEntity.email = email ?? faker.internet.email(); + oxUserBlacklistEntity.name = name ?? faker.person.lastName(); + oxUserBlacklistEntity.username = username ?? faker.internet.userName(); + await em.persistAndFlush(oxUserBlacklistEntity); + } + + async function createOxUserBlacklistEntry( + email?: OXEmail, + name?: string, + username?: OXUserName, + ): Promise { + const oxUserBlacklistEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.createNew( + email ?? faker.internet.email(), + name ?? faker.person.lastName(), + username ?? faker.internet.userName(), + ); + const mappedOxUserBlacklistEntity: OxUserBlacklistEntity = em.create( + OxUserBlacklistEntity, + mapAggregateToData(oxUserBlacklistEntry), + ); + await em.persistAndFlush(mappedOxUserBlacklistEntity); + + return mappedOxUserBlacklistEntity; + } + + afterAll(async () => { + await orm.close(); + await module.close(); + }); + + beforeEach(async () => { + await DatabaseTestModule.clearDatabase(orm); + }); + + it('should be defined', () => { + expect(sut).toBeDefined(); + }); + + describe('findByEmail', () => { + describe('when entity can be found by email', () => { + it('should return OxUserBlacklistEntry', async () => { + const fakeEmail: OXEmail = faker.internet.email(); + await createEntity(fakeEmail); + + const findResult: Option> = await sut.findByEmail(fakeEmail); + + if (!findResult) throw Error(); + expect(findResult.email).toStrictEqual(fakeEmail); + }); + }); + + describe('when entity CANNOT be found by email', () => { + it('should return null', async () => { + const findResult: Option> = await sut.findByEmail(faker.internet.email()); + + expect(findResult).toBeNull(); + }); + }); + }); + + describe('findByOxUsername', () => { + describe('when entity can be found by username', () => { + it('should return OxUserBlacklistEntry', async () => { + const fakeUsername: OXUserName = faker.internet.userName(); + await createEntity(undefined, undefined, fakeUsername); + + const findResult: Option> = await sut.findByOxUsername(fakeUsername); + + if (!findResult) throw Error(); + expect(findResult.username).toStrictEqual(fakeUsername); + }); + }); + + describe('when entity CANNOT be found by username', () => { + it('should return null', async () => { + const findResult: Option> = await sut.findByOxUsername( + faker.internet.userName(), + ); + + expect(findResult).toBeNull(); + }); + }); + }); + + describe('save', () => { + beforeEach(() => { + jest.restoreAllMocks(); + }); + describe('when OxUserBlacklistEntry has an id and can be found', () => { + it('should call the update method and return the updated OxUserBlacklistEntry', async () => { + const existingEntity: OxUserBlacklistEntity = await createOxUserBlacklistEntry(); + const updatedEmail: OXEmail = faker.internet.email(); + const updatedName: string = faker.person.lastName(); + const updatedUsername: OXUserName = faker.internet.userName(); + + const updatedEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.construct( + existingEntity.id, + existingEntity.createdAt, + existingEntity.updatedAt, + updatedEmail, + updatedName, + updatedUsername, + ); + + const result: OxUserBlacklistEntry | DomainError = await sut.save(updatedEntry); + if (result instanceof DomainError) throw Error(); + + const foundOxUserBlacklistEntity: Option = await em.findOne( + OxUserBlacklistEntity, + { + id: existingEntity.id, + }, + ); + if (!foundOxUserBlacklistEntity) throw Error(); + + expect(foundOxUserBlacklistEntity.id).toStrictEqual(existingEntity.id); + expect(foundOxUserBlacklistEntity.email).toStrictEqual(updatedEmail); + expect(foundOxUserBlacklistEntity.name).toStrictEqual(updatedName); + expect(foundOxUserBlacklistEntity.username).toStrictEqual(updatedUsername); + }); + }); + + describe('when OxUserBlacklistEntry has an id and CANNOT be found', () => { + it('should call the update method and return EntityNotFoundError', async () => { + const updatedEmail: OXEmail = faker.internet.email(); + const updatedName: string = faker.person.lastName(); + const updatedUsername: OXUserName = faker.internet.userName(); + + const updatedEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.construct( + faker.string.uuid(), + faker.date.past(), + faker.date.recent(), + updatedEmail, + updatedName, + updatedUsername, + ); + + const result: OxUserBlacklistEntry | DomainError = await sut.save(updatedEntry); + if (result instanceof OxUserBlacklistEntry) throw Error(); + + expect(result).toStrictEqual(new EntityNotFoundError('OxUserBlacklistEntity')); + }); + }); + + describe('when OxUserBlacklistEntry does not have an id', () => { + it('should call the create method and return the created OxUserBlacklistEntry', async () => { + const newEmail: OXEmail = faker.internet.email(); + const newName: string = faker.person.lastName(); + const newUsername: OXUserName = faker.internet.userName(); + const newEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.createNew( + newEmail, + newName, + newUsername, + ); + + const result: OxUserBlacklistEntry | DomainError = await sut.save(newEntry); + if (result instanceof DomainError) throw Error(); + + const foundOxUserBlacklistEntity: Option = await em.findOne( + OxUserBlacklistEntity, + { + email: newEmail, + }, + ); + if (!foundOxUserBlacklistEntity) throw Error(); + + expect(foundOxUserBlacklistEntity.email).toStrictEqual(newEmail); + expect(foundOxUserBlacklistEntity.name).toStrictEqual(newName); + expect(foundOxUserBlacklistEntity.username).toStrictEqual(newUsername); + }); + }); + }); +}); diff --git a/src/modules/person/persistence/ox-user-blacklist.repo.ts b/src/modules/person/persistence/ox-user-blacklist.repo.ts new file mode 100644 index 000000000..10df9c2e1 --- /dev/null +++ b/src/modules/person/persistence/ox-user-blacklist.repo.ts @@ -0,0 +1,104 @@ +import { EntityManager, RequiredEntityData } from '@mikro-orm/core'; +import { Injectable } from '@nestjs/common'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; +import { OxUserBlacklistEntry } from '../domain/ox-user-blacklist-entry.js'; +import { OxUserBlacklistEntity } from './ox-user-blacklist.entity.js'; +import { DomainError, EntityNotFoundError } from '../../../shared/error/index.js'; +import { OXEmail, OXUserName } from '../../../shared/types/ox-ids.types.js'; + +export function mapAggregateToData( + oxUserBlacklistEntry: OxUserBlacklistEntry, +): RequiredEntityData { + return { + // Don't assign createdAt and updatedAt, they are auto-generated! + id: oxUserBlacklistEntry.id, + email: oxUserBlacklistEntry.email, + name: oxUserBlacklistEntry.name, + username: oxUserBlacklistEntry.username, + }; +} + +function mapEntityToAggregate(entity: OxUserBlacklistEntity): OxUserBlacklistEntry { + return new OxUserBlacklistEntry( + entity.id, + entity.createdAt, + entity.updatedAt, + entity.email, + entity.name, + entity.username, + ); +} + +@Injectable() +export class OxUserBlacklistRepo { + public constructor( + private readonly em: EntityManager, + private readonly logger: ClassLogger, + ) {} + + public async findByEmail(email: OXEmail): Promise>> { + const oxUserBlacklistEntity: Option = await this.em.findOne(OxUserBlacklistEntity, { + email, + }); + if (oxUserBlacklistEntity) { + return mapEntityToAggregate(oxUserBlacklistEntity); + } + + return null; + } + + public async findByOxUsername(oxUsername: OXUserName): Promise>> { + const oxUserBlacklistEntity: Option = await this.em.findOne(OxUserBlacklistEntity, { + username: oxUsername, + }); + if (oxUserBlacklistEntity) { + return mapEntityToAggregate(oxUserBlacklistEntity); + } + + return null; + } + + /** + * Creates or updates OxUserBlacklistEntity in database. + * @param oxUserBlacklistEntry + */ + public async save( + oxUserBlacklistEntry: OxUserBlacklistEntry, + ): Promise | DomainError> { + if (oxUserBlacklistEntry.id) { + return this.update(oxUserBlacklistEntry); + } else { + return this.create(oxUserBlacklistEntry); + } + } + + private async create(oxUserBlacklistEntry: OxUserBlacklistEntry): Promise> { + const oxUserBlacklistEntity: OxUserBlacklistEntity = this.em.create( + OxUserBlacklistEntity, + mapAggregateToData(oxUserBlacklistEntry), + ); + await this.em.persistAndFlush(oxUserBlacklistEntity); + + return mapEntityToAggregate(oxUserBlacklistEntity); + } + + private async update( + oxUserBlacklistEntry: OxUserBlacklistEntry, + ): Promise | DomainError> { + const oxUserBlacklistEntity: Option = await this.em.findOne(OxUserBlacklistEntity, { + id: oxUserBlacklistEntry.id, + }); + + if (!oxUserBlacklistEntity) { + this.logger.error( + `Could Not Find OxUserBlacklistEntity, oxUserBlacklistEntryId:${oxUserBlacklistEntry.id}`, + ); + return new EntityNotFoundError('OxUserBlacklistEntity'); + } + + oxUserBlacklistEntity.assign(mapAggregateToData(oxUserBlacklistEntry)); + await this.em.persistAndFlush(oxUserBlacklistEntity); + + return mapEntityToAggregate(oxUserBlacklistEntity); + } +} diff --git a/src/modules/person/person.module.ts b/src/modules/person/person.module.ts index 45afb2505..5854b2cca 100644 --- a/src/modules/person/person.module.ts +++ b/src/modules/person/person.module.ts @@ -10,6 +10,7 @@ import { RolleFactory } from '../rolle/domain/rolle.factory.js'; import { ServiceProviderRepo } from '../service-provider/repo/service-provider.repo.js'; import { OrganisationRepository } from '../organisation/persistence/organisation.repository.js'; import { EventModule } from '../../core/eventbus/event.module.js'; +import { OxUserBlacklistRepo } from './persistence/ox-user-blacklist.repo.js'; @Module({ imports: [KeycloakAdministrationModule, LoggerModule.register(PersonModule.name), EventModule], providers: [ @@ -21,6 +22,7 @@ import { EventModule } from '../../core/eventbus/event.module.js'; OrganisationRepository, RolleFactory, ServiceProviderRepo, + OxUserBlacklistRepo, ], exports: [PersonService, PersonFactory, PersonRepository], }) diff --git a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts index def47061c..7949c00fd 100644 --- a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts +++ b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts @@ -13,6 +13,7 @@ import { DatabaseTestModule, DoFactory, KeycloakConfigTestModule, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { GlobalValidationPipe } from '../../../shared/validation/index.js'; @@ -36,6 +37,7 @@ import { PersonFactory } from '../../person/domain/person.factory.js'; import { UsernameGeneratorService } from '../../person/domain/username-generator.service.js'; import { PersonenkontextMigrationRuntype } from '../domain/personenkontext.enums.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; describe('dbiam Personenkontext API', () => { let app: INestApplication; @@ -56,6 +58,7 @@ describe('dbiam Personenkontext API', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), PersonenKontextApiModule, KeycloakAdministrationModule, + LoggingTestModule, ], providers: [ { @@ -82,6 +85,7 @@ describe('dbiam Personenkontext API', () => { }, PersonFactory, UsernameGeneratorService, + OxUserBlacklistRepo, ], }) .overrideModule(KeycloakConfigModule) diff --git a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts index 31431ec91..4de57a5eb 100644 --- a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts @@ -41,6 +41,7 @@ import { } from '../../../../test/utils/organisation-test-helper.js'; import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; describe('dbiam Personenkontext Repo', () => { let module: TestingModule; @@ -94,6 +95,7 @@ describe('dbiam Personenkontext Repo', () => { PersonFactory, PersonRepository, UsernameGeneratorService, + OxUserBlacklistRepo, RolleFactory, RolleRepo, ServiceProviderRepo, diff --git a/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts index bbc54f564..9edc6cbcc 100644 --- a/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts @@ -26,6 +26,7 @@ import { ServiceProviderRepo } from '../../service-provider/repo/service-provide import { DBiamPersonenkontextRepoInternal } from './internal-dbiam-personenkontext.repo.js'; import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; describe('dbiam Personenkontext Repo', () => { let module: TestingModule; @@ -74,6 +75,7 @@ describe('dbiam Personenkontext Repo', () => { PersonFactory, PersonRepository, UsernameGeneratorService, + OxUserBlacklistRepo, RolleFactory, RolleRepo, ServiceProviderRepo, diff --git a/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts b/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts index 6f354d629..ab7e868ba 100644 --- a/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts +++ b/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts @@ -3,6 +3,7 @@ import { ConfigTestModule, DatabaseTestModule, KeycloakConfigTestModule, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; @@ -27,6 +28,7 @@ import { OrganisationRepository } from '../../organisation/persistence/organisat import { EventService } from '../../../core/eventbus/index.js'; import { EmailRepo } from '../../email/persistence/email.repo.js'; import { Organisation } from '../../organisation/domain/organisation.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; function createPersonenkontext( this: void, @@ -69,11 +71,13 @@ describe('PersonenkontextSpecifications Integration', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), KeycloakAdministrationModule, MapperTestModule, + LoggingTestModule, ], providers: [ PersonRepository, PersonFactory, UsernameGeneratorService, + OxUserBlacklistRepo, PersonenkontextFactory, { provide: EmailRepo, diff --git a/src/shared/types/ox-ids.types.ts b/src/shared/types/ox-ids.types.ts index 8b06b7158..947c0bc36 100644 --- a/src/shared/types/ox-ids.types.ts +++ b/src/shared/types/ox-ids.types.ts @@ -1,5 +1,8 @@ import { Flavor } from './flavor.types.js'; +declare const oxEmailSymbol: unique symbol; +export type OXEmail = Flavor; + declare const oxUserIdSymbol: unique symbol; export type OXUserID = Flavor;