Skip to content

Commit

Permalink
Merge branch 'BC-3513-remove-i-prefix-in-interfaces' of github.com:hp…
Browse files Browse the repository at this point in the history
…i-schul-cloud/schulcloud-server into BC-3513-remove-i-prefix-in-interfaces
  • Loading branch information
wolfganggreschus committed Nov 16, 2023
2 parents 4cb6514 + b0b84db commit 87c3979
Show file tree
Hide file tree
Showing 10 changed files with 210 additions and 161 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,104 +109,93 @@ describe('CommonToolValidationService', () => {
});
});

describe('checkForDuplicateParameters', () => {
describe('when given parameters has a case sensitive duplicate', () => {
const setup = () => {
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
parameters: [
{ name: 'nameDuplicate', value: 'value' },
{ name: 'nameDuplicate', value: 'value' },
],
});
describe('checkCustomParameterEntries', () => {
const createTools = (
externalToolMock?: Partial<ExternalTool>,
schoolExternalToolMock?: Partial<SchoolExternalTool>,
contextExternalToolMock?: Partial<ContextExternalTool>
) => {
const externalTool: ExternalTool = new ExternalTool({
...externalToolFactory.buildWithId(),
...externalToolMock,
});
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
...schoolExternalToolFactory.buildWithId(),
...schoolExternalToolMock,
});
const schoolExternalToolId = schoolExternalTool.id as string;
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({
...contextExternalToolFactory.buildWithId(),
...contextExternalToolMock,
});

return {
schoolExternalTool,
};
return {
externalTool,
schoolExternalTool,
schoolExternalToolId,
contextExternalTool,
};
};

it('should throw error', () => {
const { schoolExternalTool } = setup();

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

expect(func).toThrow('tool_param_duplicate');
});
});

describe('when given parameters has case insensitive duplicate', () => {
describe('when a parameter is a duplicate', () => {
const setup = () => {
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
const externalTool: ExternalTool = externalToolFactory.buildWithId({
parameters: [
{ name: 'nameDuplicate', value: 'value' },
{ name: 'nameduplicate', value: 'value' },
customParameterFactory.build({
name: 'duplicate',
scope: CustomParameterScope.SCHOOL,
type: CustomParameterType.STRING,
isOptional: true,
}),
],
});
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({
toolId: externalTool.id,
parameters: [
{ name: 'duplicate', value: undefined },
{ name: 'duplicate', value: undefined },
],
});

return {
externalTool,
schoolExternalTool,
};
};

it('should throw error when given parameters has case insensitive duplicate', () => {
const { schoolExternalTool } = setup();
it('should throw error', () => {
const { externalTool, schoolExternalTool } = setup();

const func = () => service.checkForDuplicateParameters(schoolExternalTool);
const func = () => service.checkCustomParameterEntries(externalTool, schoolExternalTool);

expect(func).toThrowError('tool_param_duplicate');
});
});

describe('when given parameters has no duplicates', () => {
describe('when a parameter is unknown', () => {
const setup = () => {
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
parameters: [
{ name: 'nameNoDuplicate1', value: 'value' },
{ name: 'nameNoDuplicate2', value: 'value' },
],
const externalTool: ExternalTool = externalToolFactory.buildWithId({
parameters: [],
});
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({
toolId: externalTool.id,
parameters: [{ name: 'unknownParameter', value: undefined }],
});

return {
externalTool,
schoolExternalTool,
};
};

it('when given parameters has no duplicates should return without error', () => {
const { schoolExternalTool } = setup();
it('should throw error', () => {
const { externalTool, schoolExternalTool } = setup();

const func = () => service.checkForDuplicateParameters(schoolExternalTool);
const func = () => service.checkCustomParameterEntries(externalTool, schoolExternalTool);

expect(func).not.toThrowError('tool_param_duplicate');
expect(func).toThrowError('tool_param_unknown');
});
});
});

describe('checkCustomParameterEntries', () => {
const createTools = (
externalToolMock?: Partial<ExternalTool>,
schoolExternalToolMock?: Partial<SchoolExternalTool>,
contextExternalToolMock?: Partial<ContextExternalTool>
) => {
const externalTool: ExternalTool = new ExternalTool({
...externalToolFactory.buildWithId(),
...externalToolMock,
});
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
...schoolExternalToolFactory.buildWithId(),
...schoolExternalToolMock,
});
const schoolExternalToolId = schoolExternalTool.id as string;
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({
...contextExternalToolFactory.buildWithId(),
...contextExternalToolMock,
});

return {
externalTool,
schoolExternalTool,
schoolExternalToolId,
contextExternalTool,
};
};

describe('when checking parameter is required', () => {
describe('and given parameter is not optional and parameter value is empty', () => {
Expand Down Expand Up @@ -280,16 +269,23 @@ describe('CommonToolValidationService', () => {

describe('when parameter is not school or context', () => {
const setup = () => {
const notSchoolParam: CustomParameter = customParameterFactory.build({
name: 'notSchoolParam',
scope: CustomParameterScope.GLOBAL,
type: CustomParameterType.BOOLEAN,
});

const { externalTool, schoolExternalTool } = createTools(
{ parameters: [notSchoolParam] },
{
parameters: [{ name: 'name', value: 'true' }],
parameters: [
customParameterFactory.build({
name: 'notSchoolParam',
scope: CustomParameterScope.GLOBAL,
type: CustomParameterType.BOOLEAN,
}),
customParameterFactory.build({
name: 'schoolParam',
scope: CustomParameterScope.SCHOOL,
type: CustomParameterType.BOOLEAN,
}),
],
},
{
parameters: [{ name: 'schoolParam', value: 'true' }],
}
);

Expand Down Expand Up @@ -320,7 +316,7 @@ describe('CommonToolValidationService', () => {
const { externalTool, schoolExternalTool } = createTools(
{ parameters: [missingParam] },
{
parameters: [{ name: 'anotherParam', value: 'value' }],
parameters: [],
}
);

Expand All @@ -339,18 +335,26 @@ describe('CommonToolValidationService', () => {
});
});

describe('when parameter is optional but is missing on params', () => {
describe('when parameter is optional and was not defined', () => {
const setup = () => {
const param: CustomParameter = customParameterFactory.build({
name: 'notChecked',
scope: CustomParameterScope.SCHOOL,
isOptional: true,
});

const { externalTool, schoolExternalTool } = createTools(
{ parameters: [param] },
{
parameters: [{ name: 'anotherParam', value: 'value' }],
parameters: [
customParameterFactory.build({
name: 'optionalParameter',
scope: CustomParameterScope.SCHOOL,
isOptional: true,
}),
customParameterFactory.build({
name: 'requiredParameter',
scope: CustomParameterScope.SCHOOL,
type: CustomParameterType.STRING,
isOptional: false,
}),
],
},
{
parameters: [{ name: 'requiredParameter', value: 'value' }],
}
);

Expand All @@ -360,7 +364,7 @@ describe('CommonToolValidationService', () => {
};
};

it('should return without error ', () => {
it('should return without error', () => {
const { externalTool, schoolExternalTool } = setup();

const func = () => service.checkCustomParameterEntries(externalTool, schoolExternalTool);
Expand All @@ -385,7 +389,7 @@ describe('CommonToolValidationService', () => {
},
undefined,
{
parameters: [{ name: 'anotherParam', value: 'value' }],
parameters: [],
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,41 +29,59 @@ export class CommonToolValidationService {
return isValid;
}

public checkForDuplicateParameters(validatableTool: ValidatableTool): void {
const caseInsensitiveNames: string[] = validatableTool.parameters.map(({ name }: CustomParameterEntry) =>
name.toLowerCase()
public checkCustomParameterEntries(loadedExternalTool: ExternalTool, validatableTool: ValidatableTool): void {
this.checkForDuplicateParameters(validatableTool);

const parametersForScope: CustomParameter[] = (loadedExternalTool.parameters ?? []).filter(
(param: CustomParameter) =>
(validatableTool instanceof SchoolExternalTool && param.scope === CustomParameterScope.SCHOOL) ||
(validatableTool instanceof ContextExternalTool && param.scope === CustomParameterScope.CONTEXT)
);

this.checkForUnknownParameters(validatableTool, parametersForScope);

this.checkValidityOfParameters(validatableTool, parametersForScope);
}

private checkForDuplicateParameters(validatableTool: ValidatableTool): void {
const caseInsensitiveNames: string[] = validatableTool.parameters.map(({ name }: CustomParameterEntry) => name);

const uniqueNames: Set<string> = new Set(caseInsensitiveNames);

if (uniqueNames.size !== validatableTool.parameters.length) {
throw new ValidationError(
`tool_param_duplicate: The tool ${validatableTool.id ?? ''} contains multiple of the same custom parameters.`
);
}
}

public checkCustomParameterEntries(loadedExternalTool: ExternalTool, validatableTool: ValidatableTool): void {
if (loadedExternalTool.parameters) {
for (const param of loadedExternalTool.parameters) {
this.checkScopeAndValidateParameter(validatableTool, param);
private checkForUnknownParameters(validatableTool: ValidatableTool, parametersForScope: CustomParameter[]): void {
for (const entry of validatableTool.parameters) {
const foundParameter: CustomParameter | undefined = parametersForScope.find(
({ name }: CustomParameter): boolean => name === entry.name
);

if (!foundParameter) {
throw new ValidationError(
`tool_param_unknown: The parameter with name ${entry.name} is not part of this tool.`
);
}
}
}

private checkScopeAndValidateParameter(validatableTool: ValidatableTool, param: CustomParameter): void {
const foundEntry: CustomParameterEntry | undefined = validatableTool.parameters.find(
({ name }: CustomParameterEntry): boolean => name.toLowerCase() === param.name.toLowerCase()
);
private checkValidityOfParameters(validatableTool: ValidatableTool, parametersForScope: CustomParameter[]): void {
for (const param of parametersForScope) {
const foundEntry: CustomParameterEntry | undefined = validatableTool.parameters.find(
({ name }: CustomParameterEntry): boolean => name === param.name
);

if (param.scope === CustomParameterScope.SCHOOL && validatableTool instanceof SchoolExternalTool) {
this.validateParameter(param, foundEntry);
} else if (param.scope === CustomParameterScope.CONTEXT && validatableTool instanceof ContextExternalTool) {
this.validateParameter(param, foundEntry);
}
}

private validateParameter(param: CustomParameter, foundEntry: CustomParameterEntry | undefined): void {
this.checkOptionalParameter(param, foundEntry);

if (foundEntry) {
this.checkParameterType(foundEntry, param);
this.checkParameterRegex(foundEntry, param);
Expand Down
Loading

0 comments on commit 87c3979

Please sign in to comment.