Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N21-1336 change tool status determination method #4534

Merged
merged 14 commits into from
Nov 14, 2023
Merged

N21-1336 change tool status determination method #4534

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
562c7f0
coompute tool status via paramter validation and feature flag
IgorCapCoder Nov 8, 2023
0d292a7
unit test and refactoring
IgorCapCoder Nov 8, 2023
66faaae
Merge branch 'main' into N21-1336-change-tool-versioning
IgorCapCoder Nov 8, 2023
a3abd6e
coverage up, merge main
IgorCapCoder Nov 8, 2023
2028064
add feature flag to tool config and fix unit tests
IgorCapCoder Nov 9, 2023
8902f34
add feature flag dependent status determination in school service
IgorCapCoder Nov 9, 2023
9944948
request changes
IgorCapCoder Nov 9, 2023
de2e1b9
Merge branch 'main' into N21-1336-change-tool-versioning
IgorCapCoder Nov 9, 2023
30787e1
request changes, TODO adjust unit tests
IgorCapCoder Nov 9, 2023
e12b824
unit test adjustments for requested changes
IgorCapCoder Nov 13, 2023
f7d65ac
Merge branch 'main' into N21-1336-change-tool-versioning
IgorCapCoder Nov 13, 2023
dc3bf1d
fix unit test and merge main
IgorCapCoder Nov 13, 2023
9e1c878
requested changes
IgorCapCoder Nov 14, 2023
c1144fa
Merge branch 'main' into N21-1336-change-tool-versioning
IgorCapCoder Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { ObjectId } from '@mikro-orm/mongodb';
import { Test, TestingModule } from '@nestjs/testing';
import { contextExternalToolFactory, externalToolFactory, schoolExternalToolFactory } from '@shared/testing';
import { Configuration } from '@hpi-schul-cloud/commons';
import { ToolConfigurationStatus } from '../../common/enum';
import { CommonToolService } from '../../common/service';
import { ExternalToolLogoService, ExternalToolService } from '../../external-tool/service';
import { SchoolExternalToolService } from '../../school-external-tool/service';
import { SchoolExternalToolService, SchoolExternalToolValidationService } from '../../school-external-tool/service';
import { ToolReference } from '../domain';
import { ContextExternalToolService } from './context-external-tool.service';
import { ToolReferenceService } from './tool-reference.service';
import { ContextExternalToolValidationService } from './context-external-tool-validation.service';

describe('ToolReferenceService', () => {
let module: TestingModule;
Expand All @@ -19,6 +21,8 @@ describe('ToolReferenceService', () => {
let contextExternalToolService: DeepMocked<ContextExternalToolService>;
let commonToolService: DeepMocked<CommonToolService>;
let externalToolLogoService: DeepMocked<ExternalToolLogoService>;
let contextExternalToolValidationService: DeepMocked<ContextExternalToolValidationService>;
let schoolExternalToolValidationService: DeepMocked<SchoolExternalToolValidationService>;

beforeAll(async () => {
module = await Test.createTestingModule({
Expand All @@ -44,6 +48,14 @@ describe('ToolReferenceService', () => {
provide: ExternalToolLogoService,
useValue: createMock<ExternalToolLogoService>(),
},
{
provide: ContextExternalToolValidationService,
useValue: createMock<ContextExternalToolValidationService>(),
},
{
provide: SchoolExternalToolValidationService,
useValue: createMock<SchoolExternalToolValidationService>(),
},
],
}).compile();

Expand All @@ -53,6 +65,8 @@ describe('ToolReferenceService', () => {
contextExternalToolService = module.get(ContextExternalToolService);
commonToolService = module.get(CommonToolService);
externalToolLogoService = module.get(ExternalToolLogoService);
contextExternalToolValidationService = module.get(ContextExternalToolValidationService);
schoolExternalToolValidationService = module.get(SchoolExternalToolValidationService);
});

afterAll(async () => {
Expand Down Expand Up @@ -82,6 +96,8 @@ describe('ToolReferenceService', () => {
commonToolService.determineToolConfigurationStatus.mockReturnValue(ToolConfigurationStatus.OUTDATED);
externalToolLogoService.buildLogoUrl.mockReturnValue(logoUrl);

Configuration.set('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED', false);

return {
contextExternalToolId,
externalTool,
Expand Down Expand Up @@ -128,5 +144,42 @@ describe('ToolReferenceService', () => {
});
});
});

describe('when FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED is true', () => {
const setup = () => {
const contextExternalToolId = new ObjectId().toHexString();
const externalTool = externalToolFactory.buildWithId();
const schoolExternalTool = schoolExternalToolFactory.buildWithId({
toolId: externalTool.id as string,
});
const contextExternalTool = contextExternalToolFactory
.withSchoolExternalToolRef(schoolExternalTool.id as string)
.buildWithId(undefined, contextExternalToolId);
const logoUrl = 'logoUrl';

contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool);
schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool);
externalToolService.findById.mockResolvedValueOnce(externalTool);
commonToolService.determineToolConfigurationStatus.mockReturnValue(ToolConfigurationStatus.OUTDATED);
externalToolLogoService.buildLogoUrl.mockReturnValue(logoUrl);

Configuration.set('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED', true);

return {
contextExternalToolId,
schoolExternalTool,
contextExternalTool,
};
};

it('should determine the tool status through validation', async () => {
const { contextExternalToolId, schoolExternalTool, contextExternalTool } = setup();

await service.getToolReference(contextExternalToolId);

expect(schoolExternalToolValidationService.validate).toHaveBeenCalledWith(schoolExternalTool);
expect(contextExternalToolValidationService.validate).toHaveBeenCalledWith(contextExternalTool);
});
});
});
});
36 changes: 29 additions & 7 deletions apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { Injectable } from '@nestjs/common';
import { EntityId } from '@shared/domain';
import { Configuration } from '@hpi-schul-cloud/commons';
import { ToolConfigurationStatus } from '../../common/enum';
import { CommonToolService } from '../../common/service';
import { ExternalTool } from '../../external-tool/domain';
import { ExternalToolLogoService, ExternalToolService } from '../../external-tool/service';
import { SchoolExternalTool } from '../../school-external-tool/domain';
import { SchoolExternalToolService } from '../../school-external-tool/service';
import { SchoolExternalToolService, SchoolExternalToolValidationService } from '../../school-external-tool/service';
import { ContextExternalTool, ToolReference } from '../domain';
import { ToolReferenceMapper } from '../mapper';
import { ContextExternalToolService } from './context-external-tool.service';
import { ContextExternalToolValidationService } from './context-external-tool-validation.service';

@Injectable()
export class ToolReferenceService {
Expand All @@ -17,7 +19,9 @@ export class ToolReferenceService {
private readonly schoolExternalToolService: SchoolExternalToolService,
private readonly contextExternalToolService: ContextExternalToolService,
private readonly commonToolService: CommonToolService,
private readonly externalToolLogoService: ExternalToolLogoService
private readonly externalToolLogoService: ExternalToolLogoService,
private readonly contextExternalToolValidationService: ContextExternalToolValidationService,
private readonly schoolExternalToolValidationService: SchoolExternalToolValidationService
) {}

async getToolReference(contextExternalToolId: EntityId): Promise<ToolReference> {
Expand All @@ -29,11 +33,16 @@ export class ToolReferenceService {
);
const externalTool: ExternalTool = await this.externalToolService.findById(schoolExternalTool.toolId);

const status: ToolConfigurationStatus = this.commonToolService.determineToolConfigurationStatus(
externalTool,
schoolExternalTool,
contextExternalTool
);
let status: ToolConfigurationStatus;
if (Configuration.get('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED')) {
IgorCapCoder marked this conversation as resolved.
Show resolved Hide resolved
status = await this.determineToolConfigurationStatusThroughValidation(schoolExternalTool, contextExternalTool);
} else {
status = this.commonToolService.determineToolConfigurationStatus(
externalTool,
schoolExternalTool,
contextExternalTool
);
}

const toolReference: ToolReference = ToolReferenceMapper.mapToToolReference(
externalTool,
Expand All @@ -47,4 +56,17 @@ export class ToolReferenceService {

return toolReference;
}

private async determineToolConfigurationStatusThroughValidation(
IgorCapCoder marked this conversation as resolved.
Show resolved Hide resolved
schoolExternalTool: SchoolExternalTool,
contextExternalTool: ContextExternalTool
): Promise<ToolConfigurationStatus> {
try {
await this.schoolExternalToolValidationService.validate(schoolExternalTool);
await this.contextExternalToolValidationService.validate(contextExternalTool);
return ToolConfigurationStatus.LATEST;
} catch (err) {
return ToolConfigurationStatus.OUTDATED;
IgorCapCoder marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
42 changes: 41 additions & 1 deletion ...modules/tool/school-external-tool/service/school-external-tool-validation.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { Test, TestingModule } from '@nestjs/testing';
import { externalToolFactory, schoolExternalToolFactory } from '@shared/testing/factory/domainobject/tool';
import { Configuration } from '@hpi-schul-cloud/commons';
import { CommonToolValidationService } from '../../common/service';
import { ExternalTool } from '../../external-tool/domain';
import { ExternalToolService } from '../../external-tool/service';
Expand Down Expand Up @@ -51,8 +52,12 @@ describe('SchoolExternalToolValidationService', () => {
...externalToolFactory.buildWithId(),
...externalToolDoMock,
});
externalToolService.findById.mockResolvedValue(externalTool);

const schoolExternalToolId = schoolExternalTool.id as string;

externalToolService.findById.mockResolvedValue(externalTool);
Configuration.set('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED', true);

return {
schoolExternalTool,
ExternalTool,
Expand Down Expand Up @@ -87,9 +92,44 @@ describe('SchoolExternalToolValidationService', () => {
schoolExternalTool
);
});

it('should not throw error', async () => {
const { schoolExternalTool } = setup({ version: 8383 }, { toolVersion: 1337 });

const func = () => service.validate(schoolExternalTool);

await expect(func()).resolves.not.toThrow();
});
});
});

describe('validate with FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED on false', () => {
describe('when version of externalTool and schoolExternalTool are different', () => {
const setup = (
externalToolDoMock?: Partial<ExternalTool>,
schoolExternalToolDoMock?: Partial<SchoolExternalTool>
) => {
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
...schoolExternalToolFactory.buildWithId(),
...schoolExternalToolDoMock,
});
const externalTool: ExternalTool = new ExternalTool({
...externalToolFactory.buildWithId(),
...externalToolDoMock,
});

const schoolExternalToolId = schoolExternalTool.id as string;

externalToolService.findById.mockResolvedValue(externalTool);
Configuration.set('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED', false);

return {
schoolExternalTool,
ExternalTool,
schoolExternalToolId,
};
};

it('should throw error', async () => {
const { schoolExternalTool } = setup({ version: 8383 }, { toolVersion: 1337 });

Expand Down
6 changes: 4 additions & 2 deletions .../src/modules/tool/school-external-tool/service/school-external-tool-validation.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Injectable } from '@nestjs/common';
import { ValidationError } from '@shared/common';
import { Configuration } from '@hpi-schul-cloud/commons';
import { CommonToolValidationService } from '../../common/service';
import { ExternalTool } from '../../external-tool/domain';
import { ExternalToolService } from '../../external-tool/service';
Expand All @@ -17,8 +18,9 @@ export class SchoolExternalToolValidationService {

const loadedExternalTool: ExternalTool = await this.externalToolService.findById(schoolExternalTool.toolId);

this.checkVersionMatch(schoolExternalTool.toolVersion, loadedExternalTool.version);

if (!Configuration.get('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED')) {
this.checkVersionMatch(schoolExternalTool.toolVersion, loadedExternalTool.version);
}
this.commonToolValidationService.checkCustomParameterEntries(loadedExternalTool, schoolExternalTool);
}

Expand Down
73 changes: 71 additions & 2 deletions apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import {
externalToolFactory,
schoolExternalToolFactory,
} from '@shared/testing';
import { Configuration } from '@hpi-schul-cloud/commons';
import { ToolConfigType, ToolConfigurationStatus } from '../../common/enum';
import { CommonToolService } from '../../common/service';
import { ContextExternalTool } from '../../context-external-tool/domain';
import { BasicToolConfig, ExternalTool } from '../../external-tool/domain';
import { ExternalToolService } from '../../external-tool/service';
import { SchoolExternalTool } from '../../school-external-tool/domain';
import { SchoolExternalToolService } from '../../school-external-tool/service';
import { SchoolExternalToolService, SchoolExternalToolValidationService } from '../../school-external-tool/service';
import { ToolStatusOutdatedLoggableException } from '../error';
import { LaunchRequestMethod, ToolLaunchData, ToolLaunchDataType, ToolLaunchRequest } from '../types';
import {
Expand All @@ -23,6 +24,7 @@ import {
OAuth2ToolLaunchStrategy,
} from './launch-strategy';
import { ToolLaunchService } from './tool-launch.service';
import { ContextExternalToolValidationService } from '../../context-external-tool/service';

describe('ToolLaunchService', () => {
let module: TestingModule;
Expand All @@ -32,6 +34,8 @@ describe('ToolLaunchService', () => {
let externalToolService: DeepMocked<ExternalToolService>;
let basicToolLaunchStrategy: DeepMocked<BasicToolLaunchStrategy>;
let commonToolService: DeepMocked<CommonToolService>;
let contextExternalToolValidationService: DeepMocked<ContextExternalToolValidationService>;
let schoolExternalToolValidationService: DeepMocked<SchoolExternalToolValidationService>;

beforeEach(async () => {
module = await Test.createTestingModule({
Expand Down Expand Up @@ -61,6 +65,14 @@ describe('ToolLaunchService', () => {
provide: CommonToolService,
useValue: createMock<CommonToolService>(),
},
{
provide: ContextExternalToolValidationService,
useValue: createMock<ContextExternalToolValidationService>(),
},
{
provide: SchoolExternalToolValidationService,
useValue: createMock<SchoolExternalToolValidationService>(),
},
],
}).compile();

Expand All @@ -69,6 +81,8 @@ describe('ToolLaunchService', () => {
externalToolService = module.get(ExternalToolService);
basicToolLaunchStrategy = module.get(BasicToolLaunchStrategy);
commonToolService = module.get(CommonToolService);
contextExternalToolValidationService = module.get(ContextExternalToolValidationService);
schoolExternalToolValidationService = module.get(SchoolExternalToolValidationService);
});

afterAll(async () => {
Expand Down Expand Up @@ -109,6 +123,8 @@ describe('ToolLaunchService', () => {
basicToolLaunchStrategy.createLaunchData.mockResolvedValue(launchDataDO);
commonToolService.determineToolConfigurationStatus.mockReturnValueOnce(ToolConfigurationStatus.LATEST);

Configuration.set('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED', false);

return {
launchDataDO,
launchParams,
Expand Down Expand Up @@ -146,6 +162,14 @@ describe('ToolLaunchService', () => {

expect(externalToolService.findById).toHaveBeenCalledWith(launchParams.schoolExternalTool.toolId);
});

it('should call determineToolConfigurationStatus', async () => {
const { launchParams } = setup();

await service.getLaunchData('userId', launchParams.contextExternalTool);

expect(commonToolService.determineToolConfigurationStatus).toHaveBeenCalled();
});
});

describe('when the tool config type is unknown', () => {
Expand Down Expand Up @@ -213,7 +237,6 @@ describe('ToolLaunchService', () => {
commonToolService.determineToolConfigurationStatus.mockReturnValueOnce(ToolConfigurationStatus.OUTDATED);

return {
launchDataDO,
launchParams,
userId,
contextExternalToolId: contextExternalTool.id as string,
Expand All @@ -228,6 +251,52 @@ describe('ToolLaunchService', () => {
await expect(func).rejects.toThrow(new ToolStatusOutdatedLoggableException(userId, contextExternalToolId));
});
});

describe('when FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED is true', () => {
const setup = () => {
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId();
const contextExternalTool: ContextExternalTool = contextExternalToolFactory
.withSchoolExternalToolRef(schoolExternalTool.id as string)
.build();
const basicToolConfigDO: BasicToolConfig = basicToolConfigFactory.build();
const externalTool: ExternalTool = externalToolFactory.build({
config: basicToolConfigDO,
});

const launchDataDO: ToolLaunchData = new ToolLaunchData({
type: ToolLaunchDataType.BASIC,
baseUrl: 'https://www.basic-baseurl.com/',
properties: [],
openNewTab: false,
});

const launchParams: IToolLaunchParams = {
externalTool,
schoolExternalTool,
contextExternalTool,
};

schoolExternalToolService.findById.mockResolvedValue(schoolExternalTool);
externalToolService.findById.mockResolvedValue(externalTool);
basicToolLaunchStrategy.createLaunchData.mockResolvedValue(launchDataDO);
commonToolService.determineToolConfigurationStatus.mockReturnValueOnce(ToolConfigurationStatus.LATEST);

Configuration.set('FEATURE_COMPUTE_TOOL_STATUS_WITHOUT_VERSIONS_ENABLED', true);

return {
launchParams,
};
};

it('should should call validate', async () => {
const { launchParams } = setup();

await service.getLaunchData('userId', launchParams.contextExternalTool);

expect(schoolExternalToolValidationService.validate).toHaveBeenCalled();
expect(contextExternalToolValidationService.validate).toHaveBeenCalled();
});
});
});

describe('generateLaunchRequest', () => {
Expand Down
Loading
Loading