Skip to content

Commit

Permalink
Merge branch 'main' into N21-2317-migration-wizard-skip-errors
Browse files Browse the repository at this point in the history
  • Loading branch information
MarvinOehlerkingCap authored Dec 13, 2024
2 parents 8781c62 + 32bc14a commit 56d65ea
Show file tree
Hide file tree
Showing 20 changed files with 222 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export interface SchulconnexRestClientOptions {

clientSecret?: string;

personInfoTimeoutInMs?: number;

personenInfoTimeoutInMs?: number;

policiesInfoTimeoutInMs?: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -100,6 +101,7 @@ describe(SchulconnexRestClient.name, () => {
Authorization: `Bearer ${accessToken}`,
'Accept-Encoding': 'gzip',
},
timeout: options.personInfoTimeoutInMs,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 8 additions & 5 deletions apps/server/src/modules/idp-console/idp-console.config.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
},
});
});
});
});
Original file line number Diff line number Diff line change
@@ -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,
},
};
}
}
1 change: 1 addition & 0 deletions apps/server/src/modules/provisioning/loggable/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ export { FetchingPoliciesInfoFailedLoggable } from './fetching-policies-info-fai
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';
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -58,7 +58,8 @@ export class SanisProvisioningStrategy extends SchulconnexProvisioningStrategy {
schulconnexLicenseProvisioningService,
schulconnexToolProvisioningService,
groupService,
configService
configService,
logger
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,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) {
Expand All @@ -144,7 +155,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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
legacySchoolDoFactory,
userDoFactory,
} from '@shared/testing';
import { Logger } from '@src/core/logger';
import {
ExternalGroupDto,
ExternalSchoolDto,
Expand Down Expand Up @@ -98,6 +99,10 @@ describe(SchulconnexProvisioningStrategy.name, () => {
get: jest.fn().mockImplementation((key: keyof ProvisioningConfig) => config[key]),
},
},
{
provide: Logger,
useValue: createMock<Logger>(),
},
],
}).compile();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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(),
})
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { RoleName } from '@shared/domain/interface';
import { UUID } from 'bson';
import { Factory } from 'fishery';
import { ExternalGroupUserDto } from '../dto';

export const externalGroupUserDtoFactory = Factory.define<ExternalGroupUserDto>(
() =>
new ExternalGroupUserDto({
externalUserId: new UUID().toString(),
roleName: RoleName.TEACHER,
})
);
Loading

0 comments on commit 56d65ea

Please sign in to comment.