N21-2313 Improve schulconnex group provisioning runtime (#5394)

(cherry picked from commit 32bc14a)

apps/server/src/infra/schulconnex-client/schulconnex-client-config.ts

@@ -1,4 +1,5 @@
 export interface SchulconnexClientConfig {
+	SCHULCONNEX_CLIENT__PERSON_INFO_TIMEOUT_IN_MS: number;
 	SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS: number;
 	SCHULCONNEX_CLIENT__POLICIES_INFO_TIMEOUT_IN_MS: number;
 	SCHULCONNEX_CLIENT__API_URL?: string;

apps/server/src/infra/schulconnex-client/schulconnex-client.module.ts

@@ -8,7 +8,7 @@ import { SchulconnexRestClientOptions } from './schulconnex-rest-client-options'
 
 @Module({})
 export class SchulconnexClientModule {
-	static registerAsync(): DynamicModule {
+	public static registerAsync(): DynamicModule {
 		return {
 			imports: [HttpModule, LoggerModule],
 			module: SchulconnexClientModule,
@@ -27,6 +27,7 @@ export class SchulconnexClientModule {
 							tokenEndpoint: configService.get<string>('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT'),
 							clientId: configService.get<string>('SCHULCONNEX_CLIENT__CLIENT_ID'),
 							clientSecret: configService.get<string>('SCHULCONNEX_CLIENT__CLIENT_SECRET'),
+							personInfoTimeoutInMs: configService.get<number>('SCHULCONNEX_CLIENT__PERSON_INFO_TIMEOUT_IN_MS'),
 							personenInfoTimeoutInMs: configService.get<number>('SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS'),
 							policiesInfoTimeoutInMs: configService.get<number>('SCHULCONNEX_CLIENT__POLICIES_INFO_TIMEOUT_IN_MS'),
 						};

apps/server/src/infra/schulconnex-client/schulconnex-rest-client-options.ts

@@ -7,6 +7,8 @@ export interface SchulconnexRestClientOptions {
 
 	clientSecret?: string;
 
+	personInfoTimeoutInMs?: number;
+
 	personenInfoTimeoutInMs?: number;
 
 	policiesInfoTimeoutInMs?: number;

apps/server/src/infra/schulconnex-client/schulconnex-rest-client.spec.ts

@@ -25,8 +25,9 @@ describe(SchulconnexRestClient.name, () => {
 		clientId: 'clientId',
 		clientSecret: 'clientSecret',
 		tokenEndpoint: 'https://schulconnex.url/token',
-		personenInfoTimeoutInMs: 30000,
-		policiesInfoTimeoutInMs: 30000,
+		personInfoTimeoutInMs: 30001,
+		personenInfoTimeoutInMs: 30002,
+		policiesInfoTimeoutInMs: 30003,
 	};
 
 	beforeAll(() => {
@@ -100,6 +101,7 @@ describe(SchulconnexRestClient.name, () => {
 						Authorization: `Bearer ${accessToken}`,
 						'Accept-Encoding': 'gzip',
 					},
+					timeout: options.personInfoTimeoutInMs,
 				});
 			});
 

apps/server/src/infra/schulconnex-client/schulconnex-rest-client.ts

@@ -30,10 +30,14 @@ export class SchulconnexRestClient implements SchulconnexApiInterface {
 		this.SCHULCONNEX_API_BASE_URL = options.apiUrl || '';
 	}
 
-	public async getPersonInfo(accessToken: string, options?: { overrideUrl: string }): Promise<SchulconnexResponse> {
+	public getPersonInfo(accessToken: string, options?: { overrideUrl: string }): Promise<SchulconnexResponse> {
 		const url: URL = new URL(options?.overrideUrl ?? `${this.SCHULCONNEX_API_BASE_URL}/person-info`);
 
-		const response: Promise<SchulconnexResponse> = this.getRequest<SchulconnexResponse>(url, accessToken);
+		const response: Promise<SchulconnexResponse> = this.getRequest<SchulconnexResponse>(
+			url,
+			accessToken,
+			this.options.personInfoTimeoutInMs
+		);
 
 		return response;
 	}

apps/server/src/modules/idp-console/idp-console.config.ts

@@ -1,12 +1,12 @@
+import { Configuration } from '@hpi-schul-cloud/commons';
 import { ConsoleWriterConfig } from '@infra/console';
-import { LoggerConfig } from '@src/core/logger';
+import { RabbitMqConfig } from '@infra/rabbitmq';
+import { SchulconnexClientConfig } from '@infra/schulconnex-client';
 import { AccountConfig } from '@modules/account';
-import { UserConfig } from '@modules/user';
 import { SynchronizationConfig } from '@modules/synchronization';
-import { SchulconnexClientConfig } from '@infra/schulconnex-client';
-import { Configuration } from '@hpi-schul-cloud/commons';
+import { UserConfig } from '@modules/user';
 import { LanguageType } from '@shared/domain/interface';
-import { RabbitMqConfig } from '@infra/rabbitmq';
+import { LoggerConfig } from '@src/core/logger';
 
 export interface IdpConsoleConfig
 	extends ConsoleWriterConfig,
@@ -33,6 +33,9 @@ const config: IdpConsoleConfig = {
 	TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION: Configuration.get(
 		'TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION'
 	) as string,
+	SCHULCONNEX_CLIENT__PERSON_INFO_TIMEOUT_IN_MS: Configuration.get(
+		'SCHULCONNEX_CLIENT__PERSON_INFO_TIMEOUT_IN_MS'
+	) as number,
 	SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS: Configuration.get(
 		'SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS'
 	) as number,

apps/server/src/modules/provisioning/loggable/group-provisioning-info.loggable.spec.ts

@@ -0,0 +1,38 @@
+import { externalGroupDtoFactory, externalGroupUserDtoFactory } from '../testing';
+import { GroupProvisioningInfoLoggable } from './group-provisioning-info.loggable';
+
+describe(GroupProvisioningInfoLoggable.name, () => {
+	describe('getLogMessage', () => {
+		const setup = () => {
+			const groupCount = 2;
+			const otherUserCount = 5;
+			const totalUserCount = groupCount * otherUserCount + groupCount;
+			const externalGroups = externalGroupDtoFactory.buildList(groupCount, {
+				otherUsers: externalGroupUserDtoFactory.buildList(otherUserCount),
+			});
+
+			const loggable = new GroupProvisioningInfoLoggable(externalGroups, 100);
+
+			return {
+				loggable,
+				totalUserCount,
+				groupCount,
+			};
+		};
+
+		it('should return a loggable message', () => {
+			const { loggable, totalUserCount, groupCount } = setup();
+
+			const message = loggable.getLogMessage();
+
+			expect(message).toEqual({
+				message: 'Group provisioning has finished.',
+				data: {
+					groupCount,
+					userCount: totalUserCount,
+					durationMs: 100,
+				},
+			});
+		});
+	});
+});

apps/server/src/modules/provisioning/loggable/group-provisioning-info.loggable.ts

@@ -0,0 +1,22 @@
+import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
+import { ExternalGroupDto } from '../dto';
+
+export class GroupProvisioningInfoLoggable implements Loggable {
+	constructor(private readonly groups: ExternalGroupDto[], private readonly durationMs: number) {}
+
+	public getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
+		const userCount = this.groups.reduce(
+			(count: number, group: ExternalGroupDto) => count + (group.otherUsers?.length ?? 0),
+			this.groups.length
+		);
+
+		return {
+			message: 'Group provisioning has finished.',
+			data: {
+				groupCount: this.groups.length,
+				userCount,
+				durationMs: this.durationMs,
+			},
+		};
+	}
+}

apps/server/src/modules/provisioning/loggable/index.ts

@@ -6,3 +6,6 @@ export * from './group-role-unknown.loggable';
 export { SchoolExternalToolCreatedLoggable } from './school-external-tool-created.loggable';
 export { FetchingPoliciesInfoFailedLoggable } from './fetching-policies-info-failed.loggable';
 export { PoliciesInfoErrorResponseLoggable } from './policies-info-error-response-loggable';
+export { UserRoleUnknownLoggableException } from './user-role-unknown.loggable-exception';
+export { SchoolMissingLoggableException } from './school-missing.loggable-exception';
+export { GroupProvisioningInfoLoggable } from './group-provisioning-info.loggable';

apps/server/src/modules/provisioning/provisioning.config.ts

@@ -2,6 +2,7 @@ export interface ProvisioningConfig {
 	FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED: boolean;
 	FEATURE_SCHULCONNEX_MEDIA_LICENSE_ENABLED: boolean;
 	PROVISIONING_SCHULCONNEX_POLICIES_INFO_URL: string;
+	PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT?: number;
 	FEATURE_SANIS_GROUP_PROVISIONING_ENABLED: boolean;
 	FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED: boolean;
 }

apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts

@@ -14,6 +14,7 @@ import {
 	legacySchoolDoFactory,
 	userDoFactory,
 } from '@shared/testing';
+import { Logger } from '@src/core/logger';
 import {
 	ExternalGroupDto,
 	ExternalSchoolDto,
@@ -98,6 +99,10 @@ describe(SchulconnexProvisioningStrategy.name, () => {
 						get: jest.fn().mockImplementation((key: keyof ProvisioningConfig) => config[key]),
 					},
 				},
+				{
+					provide: Logger,
+					useValue: createMock<Logger>(),
+				},
 			],
 		}).compile();
 

apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts

@@ -2,7 +2,9 @@ import { Group, GroupService } from '@modules/group';
 import { Injectable } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { LegacySchoolDo, UserDO } from '@shared/domain/domainobject';
+import { Logger } from '@src/core/logger';
 import { ExternalGroupDto, OauthDataDto, ProvisioningDto } from '../../dto';
+import { GroupProvisioningInfoLoggable } from '../../loggable';
 import { ProvisioningConfig } from '../../provisioning.config';
 import { ProvisioningStrategy } from '../base.strategy';
 import {
@@ -24,7 +26,8 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate
 		protected readonly schulconnexLicenseProvisioningService: SchulconnexLicenseProvisioningService,
 		protected readonly schulconnexToolProvisioningService: SchulconnexToolProvisioningService,
 		protected readonly groupService: GroupService,
-		protected readonly configService: ConfigService<ProvisioningConfig, true>
+		protected readonly configService: ConfigService<ProvisioningConfig, true>,
+		protected readonly logger: Logger
 	) {
 		super();
 	}
@@ -61,6 +64,8 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate
 	}
 
 	private async provisionGroups(data: OauthDataDto, school?: LegacySchoolDo): Promise<void> {
+		const startTime = performance.now();
+
 		await this.removeUserFromGroups(data);
 
 		if (data.externalGroups) {
@@ -96,6 +101,9 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate
 
 			await Promise.all(groupProvisioningPromises);
 		}
+
+		const endTime = performance.now();
+		this.logger.warning(new GroupProvisioningInfoLoggable(data.externalGroups ?? [], endTime - startTime));
 	}
 
 	private async removeUserFromGroups(data: OauthDataDto): Promise<void> {

apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.ts

@@ -46,9 +46,9 @@ export class SanisProvisioningStrategy extends SchulconnexProvisioningStrategy {
 		protected readonly schulconnexLicenseProvisioningService: SchulconnexLicenseProvisioningService,
 		protected readonly schulconnexToolProvisioningService: SchulconnexToolProvisioningService,
 		protected readonly configService: ConfigService<ProvisioningConfig, true>,
+		protected readonly logger: Logger,
 		private readonly responseMapper: SchulconnexResponseMapper,
-		private readonly schulconnexRestClient: SchulconnexRestClient,
-		private readonly logger: Logger
+		private readonly schulconnexRestClient: SchulconnexRestClient
 	) {
 		super(
 			schulconnexSchoolProvisioningService,
@@ -58,7 +58,8 @@ export class SanisProvisioningStrategy extends SchulconnexProvisioningStrategy {
 			schulconnexLicenseProvisioningService,
 			schulconnexToolProvisioningService,
 			groupService,
-			configService
+			configService,
+			logger
 		);
 	}
 

apps/server/src/modules/provisioning/strategy/sanis/schulconnex-response-mapper.spec.ts

@@ -47,6 +47,11 @@ describe(SchulconnexResponseMapper.name, () => {
 		mapper = module.get(SchulconnexResponseMapper);
 	});
 
+	beforeEach(() => {
+		config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = false;
+		config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = undefined;
+	});
+
 	describe('mapToExternalSchoolDto', () => {
 		describe('when a schulconnex response is provided', () => {
 			const setup = () => {
@@ -316,6 +321,8 @@ describe(SchulconnexResponseMapper.name, () => {
 
 		describe('when other participants have unknown roles', () => {
 			const setup = () => {
+				config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
+
 				const schulconnexResponse: SchulconnexResponse = schulconnexResponseFactory.build();
 				schulconnexResponse.personenkontexte[0].gruppen![0]!.sonstige_gruppenzugehoerige = [
 					{
@@ -514,6 +521,56 @@ describe(SchulconnexResponseMapper.name, () => {
 				);
 			});
 		});
+
+		describe('when there are too many users in groups', () => {
+			const setup = () => {
+				config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
+				config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = 1;
+
+				const schulconnexResponse: SchulconnexResponse = schulconnexResponseFactory.build();
+
+				return {
+					schulconnexResponse,
+				};
+			};
+
+			it('should not map other group users', () => {
+				const { schulconnexResponse } = setup();
+
+				const result: ExternalGroupDto[] | undefined = mapper.mapToExternalGroupDtos(schulconnexResponse);
+
+				expect(result).toEqual([
+					expect.objectContaining<Partial<ExternalGroupDto>>({
+						otherUsers: undefined,
+					}),
+				]);
+			});
+		});
+
+		describe('when there are not too many users in groups', () => {
+			const setup = () => {
+				config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
+				config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = 10;
+
+				const schulconnexResponse: SchulconnexResponse = schulconnexResponseFactory.build();
+
+				return {
+					schulconnexResponse,
+				};
+			};
+
+			it('should not map other group users', () => {
+				const { schulconnexResponse } = setup();
+
+				const result: ExternalGroupDto[] | undefined = mapper.mapToExternalGroupDtos(schulconnexResponse);
+
+				expect(result).not.toEqual([
+					expect.objectContaining({
+						otherUsers: undefined,
+					}),
+				]);
+			});
+		});
 	});
 
 	describe('mapLernperiode', () => {

apps/server/src/modules/provisioning/strategy/sanis/schulconnex-response-mapper.ts

@@ -118,14 +118,25 @@ export class SchulconnexResponseMapper {
 			return undefined;
 		}
 
+		const usersInGroupsCount: number = groups.reduce(
+			(count: number, group: SchulconnexGruppenResponse) => count + (group.sonstige_gruppenzugehoerige?.length ?? 0),
+			groups.length
+		);
+		const limit: number | undefined = this.configService.get('PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT');
+		const shouldProvisionOtherUsers: boolean = limit === undefined || usersInGroupsCount < limit;
+
 		const mapped: ExternalGroupDto[] = groups
-			.map((group) => this.mapExternalGroup(source, group))
-			.filter((group): group is ExternalGroupDto => group !== null);
+			.map((group: SchulconnexGruppenResponse) => this.mapExternalGroup(source, group, shouldProvisionOtherUsers))
+			.filter((group: ExternalGroupDto | null): group is ExternalGroupDto => group !== null);
 
 		return mapped;
 	}
 
-	private mapExternalGroup(source: SchulconnexResponse, group: SchulconnexGruppenResponse): ExternalGroupDto | null {
+	private mapExternalGroup(
+		source: SchulconnexResponse,
+		group: SchulconnexGruppenResponse,
+		shouldProvisionOtherUsers: boolean
+	): ExternalGroupDto | null {
 		const groupType: GroupTypes | undefined = GroupTypeMapping[group.gruppe.typ];
 
 		if (!groupType) {
@@ -142,7 +153,7 @@ export class SchulconnexResponseMapper {
 		}
 
 		let otherUsers: ExternalGroupUserDto[] | undefined;
-		if (this.configService.get('FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED')) {
+		if (this.configService.get('FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED') && shouldProvisionOtherUsers) {
 			otherUsers = group.sonstige_gruppenzugehoerige
 				? group.sonstige_gruppenzugehoerige
 						.map((relation): ExternalGroupUserDto | null => this.mapToExternalGroupUser(relation))

apps/server/src/modules/provisioning/testing/external-group-dto.factory.ts

@@ -0,0 +1,18 @@
+import { UUID } from 'bson';
+import { Factory } from 'fishery';
+import { GroupTypes } from '../../group';
+import { ExternalGroupDto } from '../dto';
+import { externalGroupUserDtoFactory } from './external-group-user-dto.factory';
+
+export const externalGroupDtoFactory = Factory.define<ExternalGroupDto>(
+	({ sequence }) =>
+		new ExternalGroupDto({
+			type: GroupTypes.CLASS,
+			name: `External Group ${sequence}`,
+			externalId: new UUID().toString(),
+			user: externalGroupUserDtoFactory.build(),
+			otherUsers: externalGroupUserDtoFactory.buildList(2),
+			from: new Date(),
+			until: new Date(),
+		})
+);