Refactor advanced task args argument validators

services/api/src/resources/task/advancedtasktoolbox.test.ts

@@ -12,9 +12,9 @@ interface IPermissionsMockItem {
 }
 
 const areIKeycloakAuthAttributesEqual = (a: IKeycloakAuthAttributes, b: IKeycloakAuthAttributes) => {
-    const userSort = R.partial(R.sort, [(a, b) => { return a - b;}]);
+    const userSort = R.partial(R.sort, [(a, b) => { return a - b; }]);
     return R.equals(a.project, b.project) && R.equals(a.group, b.group) &&
-    R.equals(userSort(a.users || []), userSort(b.users || []));
+        R.equals(userSort(a.users || []), userSort(b.users || []));
 }
 
 //Mock out the hasPermissions function
@@ -23,14 +23,14 @@ const mockHasPermission = (permissions: Array<IPermissionsMockItem>) => {
     return async (resource, scope, attributes: IKeycloakAuthAttributes = {}) => {
         let match = false;
         permissions.forEach(element => {
-            if(element.resource == resource &&
+            if (element.resource == resource &&
                 scope == element.scope &&
                 areIKeycloakAuthAttributesEqual(element.attributes, attributes)
-                ) {
+            ) {
                 match = true;
             }
         });
-        if(match) { return true; }
+        if (match) { return true; }
         throw new KeycloakUnauthorizedError(`Unauthorized: You don't have permission to "${scope}" on "${resource}".`);
     }
 }
@@ -41,26 +41,26 @@ describe('advancedtasktoolbox', () => {
     describe('canUserSeeTaskDefinition', () => {
 
         let environmentById = jest.fn((id: number) => {
-            if(id == 1) {
-                return {project: 1};
+            if (id == 1) {
+                return { project: 1 };
             }
-            return {project: 2};
+            return { project: 2 };
         });
 
         let environmentHelpers = {
             getEnvironmentById: environmentById,
         };
 
         //This user has permission to view tasks on
-        let hasPermissions = mockHasPermission([{resource: 'task', scope: 'view', attributes: {project: 1}}])
+        let hasPermissions = mockHasPermission([{ resource: 'task', scope: 'view', attributes: { project: 1 } }])
         let ath = advancedTaskFunctionFactory({}, hasPermissions, {}, {}, environmentHelpers, {});
 
         test('test user is granted permission when invoking a project she has access to', () => {
-            return expect(ath.permissions.canUserSeeTaskDefinition({environment: 1})).resolves.toBe(true);
+            return expect(ath.permissions.canUserSeeTaskDefinition({ environment: 1 })).resolves.toBe(true);
         });
 
         test('test user is denied permission to a project she doesnt have access to', () => {
-            return expect(ath.permissions.canUserSeeTaskDefinition({environment: 2})).resolves.toBe(false);
+            return expect(ath.permissions.canUserSeeTaskDefinition({ environment: 2 })).resolves.toBe(false);
         });
     });
 });
@@ -69,12 +69,12 @@ describe('advancedtasktoolbox', () => {
 //Let's quickly ensure our test functions are working as expected
 describe('testSystemsMetaTest', () => {
     const usersPermissions = [
-        {resource: 'task', scope: 'view', attributes: {users: [1,2,3]}},
-        {resource: 'nothing', scope: 'whatever', attributes: {}},
+        { resource: 'task', scope: 'view', attributes: { users: [1, 2, 3] } },
+        { resource: 'nothing', scope: 'whatever', attributes: {} },
     ];
 
     test('should match our users permissions when running haspermissions', () => {
         let hasPermission = mockHasPermission(usersPermissions);
-        return expect(hasPermission('task', 'view', {users: [2,1,3]})).resolves.toBe(true);
+        return expect(hasPermission('task', 'view', { users: [2, 1, 3] })).resolves.toBe(true);
     });
 });

services/api/src/resources/task/models/advancedTaskDefinitionArgument.test.ts

@@ -0,0 +1,57 @@
+import { AdvancedTaskArgumentValidator, NumberArgument, EnvironmentSourceArgument, OtherEnvironmentSourceNamesArgument, StringArgument } from "./advancedTaskDefinitionArgument";
+
+describe("Testing AdvancedTaskArgumentValidator class as a whole", () => {
+
+    let arglist = [
+        { name: "stringarg", type: StringArgument.typeName(), range: "", default: "" },
+        { name: "envarg", type: EnvironmentSourceArgument.typeName(), range: "", default: "" },
+    ];
+
+    let environmentNames = ["main", "dev", "qa"];
+
+    let myEnvironmentName = "main";
+
+    let advancedTaskValidator = new AdvancedTaskArgumentValidator(1, myEnvironmentName, environmentNames, arglist);
+
+    test("string validator", () => {
+        return expect(advancedTaskValidator.validateArgument("stringarg", "arbitrary string")).toBe(true);
+    });
+
+    test("environment source argument validator", () => {
+        return expect(advancedTaskValidator.validateArgument("envarg", "dev")).toBe(true);
+    });
+
+    test("environment source argument validator - not in list", () => {
+        return expect(advancedTaskValidator.validateArgument("envarg", "notInEnvList")).toBe(false);
+    });
+
+});
+
+describe("advanced task def arguments individual validator tests", () => {
+
+    test("numeric validator", () => {
+        let validator = new NumberArgument();
+        return expect(validator.validateInput("5")).toBe(true);
+    });
+
+
+    let environmentSourceArgument = new EnvironmentSourceArgument(["a", "b", "c"]);
+    test("EnvironmentSourceArgument with valid input", () => {
+        return expect(environmentSourceArgument.validateInput("a")).toBe(true);
+    });
+
+    test("EnvironmentSourceArgument with invalid input", () => {
+        return expect(environmentSourceArgument.validateInput("not-in-env-list")).toBe(false);
+    });
+
+    let otherEnvironmentSourceNamesArgument = new OtherEnvironmentSourceNamesArgument("myname", ["myname", "dev", "qa", "main"]);
+    test("OtherEnvironmentSourceNamesArgument test name that isn't mine is valid", () => {
+        return expect(otherEnvironmentSourceNamesArgument.validateInput("main")).toBe(true);
+    });
+
+    test("OtherEnvironmentSourceNamesArgument test name that is mine is invalid", () => {
+        // since this should filter all environments _except_ the target, this should fail
+        return expect(otherEnvironmentSourceNamesArgument.validateInput("myname")).toBe(false);
+    });
+
+});

services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts

@@ -1,98 +1,80 @@
+import { sqlClientPool } from '../../../clients/sqlClient';
 import { query } from '../../../util/db';
 import * as R from 'ramda';
 
 export class ArgumentBase {
-    async validateInput(input): Promise<boolean> {
+    validateInput(input): boolean {
         return true;
     }
 
     public static typeName() {
         return "BASE";
     }
 
-    public async getArgumentRange() {
+    public getArgumentRange() {
         return [];
     }
 
 }
 
 export class EnvironmentSourceArgument extends ArgumentBase {
-
-    protected sqlClientPool;
-    protected environmentId;
     protected environmentNameList = [];
 
-    constructor(sqlClientPool, environmentId) {
+    constructor(environmentNameList) {
         super();
-        this.sqlClientPool = sqlClientPool;
-        this.environmentId = environmentId;
+        this.environmentNameList = environmentNameList;
     }
 
     public static typeName() {
         return "ENVIRONMENT_SOURCE_NAME";
     }
 
-    public async getArgumentRange() {
-        await this.loadEnvNames();
+    public getArgumentRange() {
         return this.environmentNameList;
     }
 
-    protected async loadEnvNames() {
-        const rows = await query(
-            this.sqlClientPool,
-            `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${this.environmentId}`
-          );
-        this.environmentNameList = R.pluck('name')(rows);
-    }
 
     /**
      *
      * @param input Environment name
      * @returns boolean
      */
-    async validateInput(input): Promise<boolean>  {
-        await this.loadEnvNames();
+    validateInput(input): boolean {
         return this.environmentNameList.includes(input);
     }
 }
 
 
 export class OtherEnvironmentSourceNamesArgument extends ArgumentBase {
 
-    protected sqlClientPool;
-    protected environmentId;
+    protected environmentName;
     protected environmentNameList = [];
 
-    constructor(sqlClientPool, environmentId) {
+    constructor(environmentName, environmentNameList) {
         super();
-        this.sqlClientPool = sqlClientPool;
-        this.environmentId = environmentId;
+        this.environmentName = environmentName;
+        // We simply filter out the target environment name here
+        this.environmentNameList = environmentNameList.filter((i) => i != environmentName);
     }
 
     public static typeName() {
         return "ENVIRONMENT_SOURCE_NAME_EXCLUDE_SELF";
     }
 
-    public async getArgumentRange() {
-        await this.loadEnvNames();
+    public getArgumentRange() {
         return this.environmentNameList;
     }
 
-    protected async loadEnvNames() {
-        const rows = await query(
-            this.sqlClientPool,
-            `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${this.environmentId} and e.id != ${this.environmentId}`
-          );
-        this.environmentNameList = R.pluck('name')(rows);
-    }
 
     /**
      *
      * @param input Environment name
      * @returns boolean
      */
-    async validateInput(input): Promise<boolean>  {
-        await this.loadEnvNames();
+    validateInput(input): boolean {
+        if (this.environmentName == input) {
+            return false; // we shouldn't match our own name - just anything that appears in the env list
+        }
         return this.environmentNameList.includes(input);
     }
 }
@@ -104,11 +86,11 @@ export class StringArgument extends ArgumentBase {
         return "STRING";
     }
 
-    async validateInput(input): Promise<boolean>  {
+    validateInput(input): boolean {
         return true;
     }
 
-    public async getArgumentRange() {
+    public getArgumentRange() {
         return null;
     }
 }
@@ -120,36 +102,86 @@ export class NumberArgument {
         return "NUMERIC";
     }
 
-    async validateInput(input): Promise<boolean>  {
+    validateInput(input): boolean {
         return /^[0-9\.]+$/.test(input);
     }
 
-    public async getArgumentRange() {
+    public getArgumentRange() {
         return null;
     }
 }
 
 
+export const getAdvancedTaskArgumentValidator = async (sqlClientPool, environment, taskArguments) => {
+    const rows = await query(
+        sqlClientPool,
+        `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id}`
+    );
+    let environmentNameList = R.pluck('name')(rows);
+    return new AdvancedTaskArgumentValidator(environment.id, environment.name, environmentNameList, taskArguments);
+}
+
+export class AdvancedTaskArgumentValidator {
+
+    protected environmentId;
+    protected environmentName;
+    protected relatedEnvironments;
+    protected taskArguments;
+
+    constructor(environmentId: number, environmentName: string, relatedEnvironments, taskArguments) {
+        this.environmentId = environmentId;
+        this.environmentName = environmentName;
+        this.relatedEnvironments = relatedEnvironments;
+        this.taskArguments = taskArguments;
+    }
+
+    public validateArgument(argumentName, argumentValue) {
+        let advancedTaskDefinitionArgument = R.find(R.propEq("name", argumentName), this.taskArguments);
+        if (advancedTaskDefinitionArgument == null || advancedTaskDefinitionArgument == undefined) {
+            throw new Error(`Unable to find argument ${argumentName} in argument list for task`);
+        }
+
+        //@ts-ignore
+        let typename = advancedTaskDefinitionArgument.type;
+        let validator = this.getValidatorForArg(typename);
+        return validator.validateInput(argumentValue);
+    }
+
+    public validatorForArgument(argumentName) {
+        let advancedTaskDefinitionArgument = R.find(R.propEq("name", argumentName), this.taskArguments);
+        if (advancedTaskDefinitionArgument == null || advancedTaskDefinitionArgument == undefined) {
+            throw new Error(`Unable to find argument ${argumentName} in argument list for task`);
+        }
+
+        //@ts-ignore
+        let typename = advancedTaskDefinitionArgument.type;
+        let validator = this.getValidatorForArg(typename);
+        return validator;
+    }
+
+    protected getValidatorForArg(typename) {
+
+        let validator = null;
+
+        switch (typename) {
+            case (EnvironmentSourceArgument.typeName()):
+                validator = new EnvironmentSourceArgument(this.relatedEnvironments);
+                break;
+            case (StringArgument.typeName()):
+                validator = new StringArgument();
+                break;
+            case (NumberArgument.typeName()):
+                validator = new NumberArgument();
+                break;
+            case (OtherEnvironmentSourceNamesArgument.typeName()):
+                validator = new OtherEnvironmentSourceNamesArgument(this.environmentName, this.relatedEnvironments);
+                break;
+            default:
+                throw new Error(`Unable to find AdvancedTaskDefinitionType ${typename}`);
+                break;
+        }
 
-/**
- * @param name The name of the advancedTaskDefinition type (stored in field)
- */
-export const advancedTaskDefinitionTypeFactory = (sqlClientPool, task, environment) => (name) => {
-    switch(name) {
-        case(EnvironmentSourceArgument.typeName()):
-            return new EnvironmentSourceArgument(sqlClientPool, environment);
-        break;
-        case(StringArgument.typeName()):
-            return new StringArgument();
-        break;
-        case(NumberArgument.typeName()):
-            return new NumberArgument();
-        break;
-        case(OtherEnvironmentSourceNamesArgument.typeName()):
-            return new OtherEnvironmentSourceNamesArgument(sqlClientPool, environment);
-        break;
-        default:
-            throw new Error(`Unable to find AdvancedTaskDefinitionType ${name}`);
-        break;
+        return validator;
     }
+
 }