From be8fe75b0f0fdb7d47668f7875b29df7291cf673 Mon Sep 17 00:00:00 2001 From: Spencer Bliven Date: Wed, 18 Dec 2024 11:44:13 +0100 Subject: [PATCH] Support dataset validation in ValidateAction During a `create` operation, the `validate` action can take a `datasets` option listing path/schema pairs that will be applied to any datasets in jobParams.datasetList. Datasets are fetched from the database during the DTO validation step if needed. Validation of archive/retrieve/public lifecycle properties is no longer done automatically, but must be configured in the jobConfig file. Details: - Separate ValidateCreateJobAction (create) and ValidateJobAction (update) - Fix 'Internal Server Error' from job controller if a session timed out. If the session times out then only public jobs will be visible. - Update documentation for validate action - Remove checkDatasetState from the job controller. This must now be configured with a validate action - Add unit test - Remove hard-coded job types and dataset states --- jobConfig.example.yaml | 23 +- .../actions/corejobactioncreators.module.ts | 8 +- .../validateaction.interface.ts | 23 +- .../validateaction/validateaction.module.ts | 6 +- .../validateaction/validateaction.service.ts | 18 +- .../validateaction/validateaction.spec.ts | 372 +++++++++++++----- .../actions/validateaction/validateaction.ts | 210 +++++++--- src/datasets/datasets.service.spec.ts | 10 +- src/jobs/dto/create-job.dto.ts | 4 +- src/jobs/jobs.controller.ts | 106 +---- src/jobs/types/job-types.enum.ts | 12 - test/Jobs.js | 2 +- test/config/jobconfig.yaml | 15 + 13 files changed, 533 insertions(+), 276 deletions(-) diff --git a/jobConfig.example.yaml b/jobConfig.example.yaml index df987f76b..c34aae57c 100644 --- a/jobConfig.example.yaml +++ b/jobConfig.example.yaml @@ -2,9 +2,12 @@ configVersion: v1.0 2024-03-01 6f3f38 jobs: - jobType: archive create: - auth: "#all" + auth: "#datasetOwner" actions: - - actionType: log + - actionType: validate + datasets: + "datasetlifecycle.archivable": + const: true - actionType: url url: http://localhost:3000/api/v3/health?jobid={{id}} headers: @@ -36,6 +39,17 @@ jobs: exchange: jobs.write queue: client.jobs.write key: jobqueue + - jobType: retrieve + create: + auth: "#datasetOwner" + actions: + - actionType: validate + datasets: + "datasetlifecycle.retrievable": + const: true + statusUpdate: + auth: "archivemanager" + actions: [] - jobType: public create: auth: "#all" @@ -47,5 +61,8 @@ jobs: required: - pid - files + datasets: + isPublished: + const: true statusUpdate: - auth: "#all" + auth: "archivemanager" diff --git a/src/config/job-config/actions/corejobactioncreators.module.ts b/src/config/job-config/actions/corejobactioncreators.module.ts index ebecb2be3..c57c9a3cd 100644 --- a/src/config/job-config/actions/corejobactioncreators.module.ts +++ b/src/config/job-config/actions/corejobactioncreators.module.ts @@ -7,7 +7,7 @@ import { actionType as logActionType } from "./logaction/logaction.interface"; import { actionType as emailActionType } from "./emailaction/emailaction.interface"; import { ValidateJobActionModule } from "./validateaction/validateaction.module"; import { actionType as validateActionType } from "./validateaction/validateaction.interface"; -import { ValidateJobActionCreator } from "./validateaction/validateaction.service"; +import { ValidateCreateJobActionCreator, ValidateJobActionCreator } from "./validateaction/validateaction.service"; import { URLJobActionModule } from "./urlaction/urlaction.module"; import { URLJobActionCreator } from "./urlaction/urlaction.service"; import { actionType as urlActionType } from "./urlaction/urlaction.interface"; @@ -39,14 +39,14 @@ import { useFactory: ( logJobActionCreator, emailJobActionCreator, - validateJobActionCreator, + validateCreateJobActionCreator, urlJobActionCreator, rabbitMQJobActionCreator, ) => { return { [logActionType]: logJobActionCreator, [emailActionType]: emailJobActionCreator, - [validateActionType]: validateJobActionCreator, + [validateActionType]: validateCreateJobActionCreator, [urlActionType]: urlJobActionCreator, [rabbitmqActionType]: rabbitMQJobActionCreator, }; @@ -54,7 +54,7 @@ import { inject: [ LogJobActionCreator, EmailJobActionCreator, - ValidateJobActionCreator, + ValidateCreateJobActionCreator, URLJobActionCreator, RabbitMQJobActionCreator, ], diff --git a/src/config/job-config/actions/validateaction/validateaction.interface.ts b/src/config/job-config/actions/validateaction/validateaction.interface.ts index aea837f2c..16bae02f2 100644 --- a/src/config/job-config/actions/validateaction/validateaction.interface.ts +++ b/src/config/job-config/actions/validateaction/validateaction.interface.ts @@ -4,7 +4,13 @@ export const actionType = "validate"; export interface ValidateJobActionOptions extends JobActionOptions { actionType: typeof actionType; - request: Record; + request?: Record; +} + +export interface ValidateCreateJobActionOptions extends ValidateJobActionOptions { + actionType: typeof actionType; + request?: Record; + datasets?: Record; } /** @@ -15,7 +21,20 @@ export function isValidateJobActionOptions( ): options is ValidateJobActionOptions { if (typeof options === "object" && options !== null) { const opts = options as ValidateJobActionOptions; - return opts.actionType === actionType && typeof opts.request === "object"; + return opts.actionType === actionType && (opts.request === undefined || typeof opts.request === "object"); + } + return false; +} + +/** + * Type guard for EmailJobActionOptions + */ +export function isValidateCreateJobActionOptions( + options: unknown, +): options is ValidateJobActionOptions { + if (typeof options === "object" && options !== null) { + const opts = options as ValidateCreateJobActionOptions; + return opts.actionType === actionType && (opts.request === undefined || typeof opts.request === "object") && (opts.datasets === undefined || typeof opts.datasets === "object"); } return false; } diff --git a/src/config/job-config/actions/validateaction/validateaction.module.ts b/src/config/job-config/actions/validateaction/validateaction.module.ts index af4c77bdc..23f873fe1 100644 --- a/src/config/job-config/actions/validateaction/validateaction.module.ts +++ b/src/config/job-config/actions/validateaction/validateaction.module.ts @@ -1,8 +1,8 @@ import { Module } from "@nestjs/common"; -import { ValidateJobActionCreator } from "./validateaction.service"; +import { ValidateCreateJobActionCreator, ValidateJobActionCreator } from "./validateaction.service"; @Module({ - providers: [ValidateJobActionCreator], - exports: [ValidateJobActionCreator], + providers: [ValidateJobActionCreator, ValidateCreateJobActionCreator], + exports: [ValidateJobActionCreator, ValidateCreateJobActionCreator], }) export class ValidateJobActionModule {} diff --git a/src/config/job-config/actions/validateaction/validateaction.service.ts b/src/config/job-config/actions/validateaction/validateaction.service.ts index 7d2233e40..c46310c52 100644 --- a/src/config/job-config/actions/validateaction/validateaction.service.ts +++ b/src/config/job-config/actions/validateaction/validateaction.service.ts @@ -4,12 +4,14 @@ import { JobActionOptions, JobDto, } from "../../jobconfig.interface"; -import { ValidateJobAction } from "./validateaction"; +import { ValidateCreateJobAction, ValidateJobAction } from "./validateaction"; import { isValidateJobActionOptions } from "./validateaction.interface"; +import { DatasetsService } from "src/datasets/datasets.service"; +import { CreateJobDto } from "src/jobs/dto/create-job.dto"; @Injectable() export class ValidateJobActionCreator implements JobActionCreator { - constructor() {} + constructor(private datasetService: DatasetsService) {} public create(options: Options) { if (!isValidateJobActionOptions(options)) { @@ -18,3 +20,15 @@ export class ValidateJobActionCreator implements JobActionCreator { return new ValidateJobAction(options); } } + +@Injectable() +export class ValidateCreateJobActionCreator implements JobActionCreator { + constructor(private datasetService: DatasetsService) {} + + public create(options: Options) { + if (!isValidateJobActionOptions(options)) { + throw new Error("Invalid options for ValidateJobAction."); + } + return new ValidateCreateJobAction(this.datasetService, options); + } +} diff --git a/src/config/job-config/actions/validateaction/validateaction.spec.ts b/src/config/job-config/actions/validateaction/validateaction.spec.ts index fddf96200..dd19328ab 100644 --- a/src/config/job-config/actions/validateaction/validateaction.spec.ts +++ b/src/config/job-config/actions/validateaction/validateaction.spec.ts @@ -1,6 +1,9 @@ -import { ValidateJobAction } from "./validateaction"; +import { ValidateCreateJobAction, ValidateJobAction } from "./validateaction"; import { CreateJobDto } from "../../../../jobs/dto/create-job.dto"; -import { ValidateJobActionOptions } from "./validateaction.interface"; +import { ValidateCreateJobActionOptions, ValidateJobActionOptions } from "./validateaction.interface"; +import { DatasetsService } from "src/datasets/datasets.service"; +import { Test, TestingModule } from "@nestjs/testing"; +import { ValidateCreateJobActionCreator } from "./validateaction.service"; const createJobBase = { type: "validate", @@ -9,127 +12,284 @@ const createJobBase = { contactEmail: "email@example.com", }; -describe("ValiateAction", () => { - const config: ValidateJobActionOptions = { - actionType: "validate", - request: { - "jobParams.stringVal": { type: "string" }, - "jobParams.requiredArray[*]": { type: "string" }, - "jobParams.numberVal": { type: "number" }, - jobParams: { required: ["nonNull"] }, - }, - }; - const action = new ValidateJobAction(config); - it("should be configured successfully", async () => { - expect(action).toBeDefined(); - }); +const mockDataset = { + _id: "testId", + pid: "testPid", + owner: "testOwner", + ownerEmail: "testOwner@email.com", + instrumentIds: ["testInstrumentId"], + orcidOfOwner: "https://0000.0000.0000.0001", + contactEmail: "testContact@email.com", + sourceFolder: "/nfs/groups/beamlines/test/123456", + sourceFolderHost: "https://fileserver.site.com", + size: 1000000, + packedSize: 1000000, + numberOfFiles: 1, + numberOfFilesArchived: 1, + creationTime: new Date("2021-11-11T12:29:02.083Z"), + type: "raw", + validationStatus: "string", + keywords: [], + description: "Test dataset.", + datasetName: "Test Dataset", + classification: "string", + license: "string", + version: "string", + isPublished: false, + history: [], + datasetlifecycle: { + id: "testId", + archivable: true, + retrievable: false, + publishable: true, + dateOfDiskPurging: new Date("2031-11-11T12:29:02.083Z"), + archiveRetentionTime: new Date("2031-11-11T12:29:02.083Z"), + dateOfPublishing: new Date("2024-11-11T12:29:02.083Z"), + publishedOn: new Date("2024-11-11T12:29:02.083Z"), + isOnCentralDisk: true, + archiveReturnMessage: {}, + retrieveReturnMessage: {}, + archiveStatusMessage: "string", + retrieveStatusMessage: "string", + exportedTo: "string", + retrieveIntegrityCheck: false, + }, + createdAt: new Date("2021-11-11T12:29:02.083Z"), + updatedAt: new Date("2021-11-11T12:29:02.083Z"), + techniques: [], + principalInvestigator: "testInvestigator", + endTime: new Date("2021-12-11T12:29:02.083Z"), + creationLocation: "test", + dataFormat: "Test Format", + scientificMetadata: {}, + proposalIds: ["ABCDEF"], + sampleIds: ["testSampleId"], + accessGroups: [], + createdBy: "test user", + ownerGroup: "test", + relationships: [], + sharedWith: [], + updatedBy: "test", + instrumentGroup: "test", + inputDatasets: [], + usedSoftware: [], + jobParameters: {}, + jobLogData: "", + comment: "", + dataQualityMetrics: 1, +}; - it("should pass if required params are present", async () => { - const dto: CreateJobDto = { - ...createJobBase, - jobParams: { - stringVal: "ok", - numberVal: 1, - nonNull: "value1", - requiredArray: ["ok"], - }, - }; +describe("ValidateAction", () => { + describe("request validation", () => { + let action: ValidateJobAction; - await expect(action.validate(dto)).resolves.toBeUndefined(); - }); + beforeAll(() => { + const config: ValidateJobActionOptions = { + actionType: "validate", + request: { + "jobParams.stringVal": { type: "string" }, + "jobParams.requiredArray[*]": { type: "string" }, + "jobParams.numberVal": { type: "number" }, + jobParams: { required: ["nonNull"] }, + }, + }; + action = new ValidateJobAction(config); + }); - it("should fail if nonNull is missing", async () => { - const dto: CreateJobDto = { - ...createJobBase, - jobParams: { - stringVal: "ok", - numberVal: 1, - //nonNull: "value1", - requiredArray: ["ok"], - }, - }; + it("should be configured successfully", async () => { + expect(action).toBeDefined(); + expect(action["request"]).toBeDefined(); + }); - await expect(action.validate(dto)).rejects.toThrow( - "Invalid request. Invalid value for 'jobParams'", - ); - }); + it("should pass if required params are present", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: "ok", + numberVal: 1, + nonNull: "value1", + requiredArray: ["ok"], + }, + }; - it("should fail if string type is wrong", async () => { - const dto: CreateJobDto = { - ...createJobBase, - jobParams: { - stringVal: 0xdeadbeef, // wrong type - numberVal: 1, - nonNull: "value1", - requiredArray: ["ok"], - }, - }; + await expect(action.validate(dto)).resolves.toBeUndefined(); + }); - await expect(action.validate(dto)).rejects.toThrow( - "Invalid request. Invalid value for 'jobParams.stringVal", - ); - }); + it("should fail if nonNull is missing", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: "ok", + numberVal: 1, + //nonNull: "value1", + requiredArray: ["ok"], + }, + }; - it("should fail if number type is wrong", async () => { - const dto: CreateJobDto = { - ...createJobBase, - jobParams: { - stringVal: "ok", - numberVal: "error", - nonNull: "value1", - requiredArray: ["ok"], - }, - }; + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Invalid value for 'jobParams'", + ); + }); - await expect(action.validate(dto)).rejects.toThrow( - "Invalid request. Invalid value for 'jobParams.numberVal'", - ); - }); + it("should fail if string type is wrong", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: 0xdeadbeef, // wrong type + numberVal: 1, + nonNull: "value1", + requiredArray: ["ok"], + }, + }; - it("should fail if requiredArray is ommitted", async () => { - const dto: CreateJobDto = { - ...createJobBase, - jobParams: { - stringVal: "ok", - numberVal: 1, - nonNull: "value1", - //requiredArray: ["ok"], - }, - }; + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Invalid value for 'jobParams.stringVal", + ); + }); - await expect(action.validate(dto)).rejects.toThrow( - "Invalid request. Requires 'jobParams.requiredArray[*]'", - ); - }); + it("should fail if number type is wrong", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: "ok", + numberVal: "error", + nonNull: "value1", + requiredArray: ["ok"], + }, + }; - it("should fail if requiredArray is empty", async () => { - const dto: CreateJobDto = { - ...createJobBase, - jobParams: { - stringVal: "ok", - numberVal: 1, - nonNull: "value1", - requiredArray: [], - }, - }; - await expect(action.validate(dto)).rejects.toThrow( - "Invalid request. Requires 'jobParams.requiredArray[*]'", - ); + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Invalid value for 'jobParams.numberVal'", + ); + }); + + it("should fail if requiredArray is ommitted", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: "ok", + numberVal: 1, + nonNull: "value1", + //requiredArray: ["ok"], + }, + }; + + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Requires 'jobParams.requiredArray[*]'", + ); + }); + + it("should fail if requiredArray is empty", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: "ok", + numberVal: 1, + nonNull: "value1", + requiredArray: [], + }, + }; + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Requires 'jobParams.requiredArray[*]'", + ); + }); + + it("should fail if requiredArray has the wrong type", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: { + stringVal: "ok", + numberVal: "error", + nonNull: "value1", + requiredArray: [0xdeadbeef], + }, + }; + + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Invalid value for 'jobParams.requiredArray[*]'", + ); + }); }); - it("should fail if requiredArray has the wrong type", async () => { - const dto: CreateJobDto = { + describe("dataset validation", () => { + let creator: ValidateCreateJobActionCreator; + let action: ValidateCreateJobAction; + const findAll = jest.fn().mockResolvedValue([mockDataset]); + const mockCreateDto: CreateJobDto = { ...createJobBase, jobParams: { - stringVal: "ok", - numberVal: "error", - nonNull: "value1", - requiredArray: [0xdeadbeef], + datasetList: [ + { + pid: mockDataset.pid, + files: [], + }, + ], }, }; - await expect(action.validate(dto)).rejects.toThrow( - "Invalid request. Invalid value for 'jobParams.requiredArray[*]'", - ); + beforeAll(async () => { + // Create mock dataset + const module: TestingModule = await Test.createTestingModule({ + providers: [ + { + provide: DatasetsService, + useValue: { + findAll: findAll, + }, + }, + ValidateCreateJobActionCreator, + ], + }).compile(); + + const config: ValidateCreateJobActionOptions = { + actionType: "validate", + datasets: { + "datasetlifecycle.archivable": { const: true }, + }, + } + + creator = await module.resolve(ValidateCreateJobActionCreator); + action = creator.create(config); + }); + + it("should be configured successfully", async () => { + expect(action).toBeDefined(); + expect(action["datasets"]).toBeDefined(); + expect(action["datasetsService"]).toBeDefined(); + }); + + it("should fail without a dataset", async () => { + const dto: CreateJobDto = { + ...createJobBase, + jobParams: {}, + }; + + await expect(action.validate(dto)).rejects.toThrow( + "'jobParams.datasetList' is required.", + ); + }); + + it("should pass when referencing an archivable dataset", async () => { + const dto: CreateJobDto = { + ...mockCreateDto, + }; + + await expect(action.validate(dto)).resolves.toBeUndefined(); + }); + + it("should fail when referencing a non-archivable dataset", async () => { + const dto: CreateJobDto = { + ...mockCreateDto, + }; + const unarchivableDataset = { + ...mockDataset, + }; + unarchivableDataset.datasetlifecycle.archivable = false; + + findAll.mockResolvedValueOnce([unarchivableDataset]); + await expect(action.validate(dto)).rejects.toThrow( + "Invalid request. Invalid value for 'datasetlifecycle.archivable'", + ); + }); }); }); diff --git a/src/config/job-config/actions/validateaction/validateaction.ts b/src/config/job-config/actions/validateaction/validateaction.ts index a7182fee2..c3d374785 100644 --- a/src/config/job-config/actions/validateaction/validateaction.ts +++ b/src/config/job-config/actions/validateaction/validateaction.ts @@ -1,40 +1,54 @@ -import { HttpException, HttpStatus, NotFoundException } from "@nestjs/common"; +import { HttpException, HttpStatus, Logger, NotFoundException } from "@nestjs/common"; import { JobAction, JobDto } from "../../jobconfig.interface"; import { JobClass } from "../../../../jobs/schemas/job.schema"; -import { JSONPath } from "jsonpath-plus"; +import { JSONPath, JSONPathOptions } from "jsonpath-plus"; import Ajv, { ValidateFunction } from "ajv"; import { actionType, + ValidateCreateJobActionOptions, ValidateJobActionOptions, } from "./validateaction.interface"; +import { CreateJobDto } from "src/jobs/dto/create-job.dto"; +import { DatasetsService } from "src/datasets/datasets.service"; +import { JobParams } from "src/jobs/types/job-types.enum"; +import { DatasetListDto } from "src/jobs/dto/dataset-list.dto"; +type JSONData = JSONPathOptions["json"]; /** - * Validates the job DTO for the presence of required fields. Can also check types or + * Validates the job for the presence of required fields. Can also check types or * validate against a JSON Schema. * - * ValidateAction is configured with a single parameter, `request`, which is checked - * against the request body (aka the DTO). The config file will look like this: + * The config file for a validate action will look like this: * - * 
{
- *   "actionType": "validate",
- *   "request": {
- *     "path": typecheck,
- *     ...
- *   }
- * }
+ * 
+ * actionType: validate
+ * request:
+ *   path: typecheck
+ *
* * Usually `path` will be a dot-delimited field in the DTO, eg. "jobParams.name". - * Technically it is a JSONPath-Plus expression; see https://github.com/JSONPath-Plus/JSONPath. + * Technically it is a JSONPath-Plus expression; see + * https://github.com/JSONPath-Plus/JSONPath. * * Here are some example typecheck expressions: - * 
{
- *   "actionType": "validate",
- *   "request": {
- *     "name": { "type": "string"}, // match simple types
- *     "answers[*]": { "enum": ["yes","no"]}, // literal values (here applied to an array)
- *     "metadata": {"$ref": "https://json.schemastore.org/schema-org-thing.json"}, // JSON Schema
- *   }
- * }
+ * 
+ * actionType: validate
+ * request:
+ *   name:
+ *     type: string
+ *   answers[*]:
+ *     enum: ["yes", "no" ]
+ *   metadata:
+ *     "$ref": https://json.schemastore.org/schema-org-thing.json
+ *
+ * + * This base class includes only the `request` field, which validates the request body + * received from the client (aka the DTO). It is registed for the `update` job + * operation. + * + * ValidateCreateAction extends this with an additional `datasets` field, which works + * identically to `request` but is applied to any datasets mentioned in + * `jobParams.datasetList`. This is used for the `create` job operation. */ export class ValidateJobAction implements JobAction { private request: Record>; @@ -43,20 +57,25 @@ export class ValidateJobAction implements JobAction { return actionType; } - constructor(options: ValidateJobActionOptions) { - if (!("request" in options)) { - throw new NotFoundException( - `Missing connection parameter in 'validate' action: 'request'`, - ); + constructor(options: ValidateJobActionOptions, ajv?: Ajv) { + ajv = + ajv || + new Ajv({ + strictSchema: false, + strictTypes: false, + }); + if ("request" in options) { + const request = options.request || {}; + this.request = this.compileSchemas(ajv, request); } - const request = options["request"] as Record; + } - const ajv = new Ajv({ - strictSchema: false, - strictTypes: false, - }); - this.request = Object.fromEntries( - Object.entries(request).map(([path, schema]) => { + protected compileSchemas( + ajv: Ajv, + schemasMap: Record, + ): Record> { + return Object.fromEntries( + Object.entries(schemasMap).map(([path, schema]) => { if (typeof schema !== "object" || schema === null) { throw new Error("Schema must be a valid object."); } @@ -67,28 +86,28 @@ export class ValidateJobAction implements JobAction { } async validate(dto: T) { - for (const [path, schema] of Object.entries(this.request)) { - const result: unknown[] = JSONPath({ path: path, json: dto }); + if (this.request) { + // validate request body + this.validateJson(dto, this.request); + } + } + + protected validateJson( + json: JSONData, + schemaMap: Record>, + ) { + for (const [path, schema] of Object.entries(schemaMap)) { + const result: JSONData[] = JSONPath({ path, json }); if (result !== null && result?.length > 0) { result.forEach((entry) => { if (!schema(entry)) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: `Invalid request. Invalid value for '${path}'`, - }, - HttpStatus.BAD_REQUEST, + throw makeHttpException( + `Invalid request. Invalid value for '${path}'`, ); } }); } else { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: `Invalid request. Requires '${path}'`, - }, - HttpStatus.BAD_REQUEST, - ); + throw makeHttpException(`Invalid request. Requires '${path}'`); } } } @@ -100,3 +119,98 @@ export class ValidateJobAction implements JobAction { // eslint-disable-next-line @typescript-eslint/no-unused-vars async performJob(job: JobClass) {} } + +/** + * Validates a request to create a new job for the presence of required fields. Can + * check types or validate against a JSON Schema. + * + * The config file for a validate action will look like this: + * + * 
+ * actionType: validate
+ * request:
+ *   path: typecheck
+ * datasets:
+ *   path: typecheck
+ *
+ * + * The constraints in the 'datasets' section are applied to all datasets listed in the + * `jobParams.datasetList` array. Since the dataset list needs to be set at job + * creation, this form of the action is only applicable to the `CreateJobDto` and cannot + * be included during an update operation. + * + * See ValidateAction for more information about the form of `path` and `typecheck`. + */ +export class ValidateCreateJobAction extends ValidateJobAction { + private datasets?: Record>; + + constructor(private datasetsService: DatasetsService, options: ValidateCreateJobActionOptions, ajv?: Ajv) { + ajv = + ajv || + new Ajv({ + strictSchema: false, + strictTypes: false, + }); + super(options, ajv); + if ("datasets" in options) { + const datasets = options.datasets || {}; + this.datasets = this.compileSchemas(ajv, datasets); + } + } + + async validate(dto: CreateJobDto): Promise { + await super.validate(dto); + if (!this.datasets) { + return; + } + const datasets = await this.loadDatasets(dto); + await Promise.all( + datasets.map((dataset) => this.validateJson(dataset, this.datasets!)), + ); + } + + private async loadDatasets(dto: CreateJobDto) { + // Require datasetList + if (!(JobParams.DatasetList in dto.jobParams)) { + throw makeHttpException( + `'jobParams.${JobParams.DatasetList}' is required.`, + ); + } + const datasetList = dto.jobParams[ + JobParams.DatasetList + ] as DatasetListDto[]; + const datasetIds = datasetList.map((x) => x.pid); + + // Load linked datasets + const filter = { + where: { + pid: { + $in: datasetIds, + }, + }, + }; + if ( this.datasetsService === undefined || this.datasetsService.findAll === undefined) { + Logger.error(`NestJS error. Dependency injection not working in ValidateCreateAction`); + throw makeHttpException("NestJS error. Dependency injection not working in ValidateCreateAction.", HttpStatus.INTERNAL_SERVER_ERROR); + } + const result = await this.datasetsService.findAll(filter); + if (result.length != datasetIds.length) { + Logger.error( + `Unable to get a dataset for job (${JSON.stringify(datasetIds)})`, + ); + throw makeHttpException(`Unable to get a dataset.`); + } + return result; + } +} + +function makeHttpException(message: string, status?: number): HttpException { + status = status || HttpStatus.BAD_REQUEST; + return new HttpException( + { + status: status, + message: message, + }, + status, + ); +} diff --git a/src/datasets/datasets.service.spec.ts b/src/datasets/datasets.service.spec.ts index 9cea8700e..5f3afce13 100644 --- a/src/datasets/datasets.service.spec.ts +++ b/src/datasets/datasets.service.spec.ts @@ -19,7 +19,7 @@ const mockDataset: DatasetClass = { pid: "testPid", owner: "testOwner", ownerEmail: "testOwner@email.com", - instrumentId: "testInstrumentId", + instrumentIds: ["testInstrumentId"], orcidOfOwner: "https://0000.0000.0000.0001", contactEmail: "testContact@email.com", sourceFolder: "/nfs/groups/beamlines/test/123456", @@ -64,19 +64,15 @@ const mockDataset: DatasetClass = { creationLocation: "test", dataFormat: "Test Format", scientificMetadata: {}, - proposalId: "ABCDEF", - sampleId: "testSampleId", - attachments: [], + proposalIds: ["ABCDEF"], + sampleIds: ["testSampleId"], accessGroups: [], createdBy: "test user", - datablocks: [], - origdatablocks: [], ownerGroup: "test", relationships: [], sharedWith: [], updatedBy: "test", instrumentGroup: "test", - investigator: "test", inputDatasets: [], usedSoftware: [], jobParameters: {}, diff --git a/src/jobs/dto/create-job.dto.ts b/src/jobs/dto/create-job.dto.ts index 775e30951..50b01b180 100644 --- a/src/jobs/dto/create-job.dto.ts +++ b/src/jobs/dto/create-job.dto.ts @@ -23,7 +23,7 @@ export class CreateJobDto { type: String, required: false, description: - "User that this job belongs to. Applicable only if requesting user has dequate permissions level", + "User that this job belongs to. Applicable only if requesting user has adequate permissions level", }) @IsString() @IsOptional() @@ -33,7 +33,7 @@ export class CreateJobDto { type: String, required: false, description: - "Group that this job belongs to. Applicable only if requesting user has dequate permissions level", + "Group that this job belongs to. Applicable only if requesting user has adequate permissions level", }) @IsString() @IsOptional() diff --git a/src/jobs/jobs.controller.ts b/src/jobs/jobs.controller.ts index f0d8f6380..e109ceff7 100644 --- a/src/jobs/jobs.controller.ts +++ b/src/jobs/jobs.controller.ts @@ -86,7 +86,7 @@ export class JobsController { if (parseBoolean(this.configService.get("rabbitMq.enabled"))) { // TODO: This should publish the job to the message broker. // job.publishJob(ctx.instance, "jobqueue"); - console.log("Saved Job %s#%s and published to message broker"); + Logger.log("Saved Job %s#%s and published to message broker"); } } @@ -109,7 +109,6 @@ export class JobsController { */ async validateDatasetList( jobParams: Record, - jobType: string, ): Promise { const datasetList = jobParams[ JobParams.DatasetList @@ -158,8 +157,6 @@ export class JobsController { // check that all requested pids exist await this.checkDatasetPids(datasetListDtos); - // check that dataset state is compatible with the job type - await this.checkDatasetState(datasetListDtos, jobType); // check that all requested files exist await this.checkDatasetFiles(datasetListDtos); @@ -199,79 +196,6 @@ export class JobsController { return; } - /** - * Check that datasets are in a state at which the job can be performed: - * For retrieve jobs all datasets must be in state retrievable - * For archive jobs all datasets must be in state archivable - * For public jobs all datasets must be published - */ - async checkDatasetState( - datasetList: DatasetListDto[], - jobType: string, - ): Promise { - const datasetIds = datasetList.map((x) => x.pid); - switch (jobType) { - case JobType.Retrieve: // Intentional fall through - case JobType.Archive: - // can I archive some files of a dataset or is it always every file? - { - const filter = { - fields: { - pid: true, - }, - where: { - [`datasetlifecycle.${DatasetState[jobType]}`]: false, - pid: { - $in: datasetIds, - }, - }, - }; - const result = await this.datasetsService.findAll(filter); - if (result.length > 0) { - throw new HttpException( - { - status: HttpStatus.CONFLICT, - message: `The following datasets are not in ${DatasetState[jobType]} state for a ${jobType} job.`, - error: JSON.stringify(result.map(({ pid }) => ({ pid }))), - }, - HttpStatus.CONFLICT, - ); - } - } - break; - case JobType.Public: - // isPublished applies to the full dataset not the files - { - const filter = { - fields: { - pid: true, - }, - where: { - [DatasetState.public]: true, - pid: { - $in: datasetIds, - }, - }, - }; - const result = await this.datasetsService.findAll(filter); - if (result.length !== datasetIds.length) { - throw new HttpException( - { - status: HttpStatus.CONFLICT, - message: "The following datasets are not public.", - error: JSON.stringify(result.map(({ pid }) => ({ pid }))), - }, - HttpStatus.CONFLICT, - ); - } - } - break; - default: - // Do not check for other job types - break; - } - } - /** * Check that the dataset files are valid */ @@ -407,10 +331,7 @@ export class JobsController { // validate datasetList, if it exists in jobParams let datasetList: DatasetListDto[] = []; if (JobParams.DatasetList in jobCreateDto.jobParams) { - datasetList = await this.validateDatasetList( - jobCreateDto.jobParams, - jobCreateDto.type, - ); + datasetList = await this.validateDatasetList(jobCreateDto.jobParams); jobInstance.jobParams = { ...jobInstance.jobParams, [JobParams.DatasetList]: datasetList, @@ -507,12 +428,25 @@ export class JobsController { if (jobConfiguration.create.auth === "#datasetPublic") { datasetsWhere["where"]["isPublished"] = true; } else if (jobConfiguration.create.auth === "#datasetAccess") { - datasetsWhere["where"]["$or"] = [ - { ownerGroup: { $in: user.currentGroups } }, - { accessGroups: { $in: user.currentGroups } }, - { isPublished: true }, - ]; + if (user) { + datasetsWhere["where"]["$or"] = [ + { ownerGroup: { $in: user.currentGroups } }, + { accessGroups: { $in: user.currentGroups } }, + { isPublished: true }, + ]; + } else { + datasetsWhere["where"]["isPublished"] = true; + } } else if (jobConfiguration.create.auth === "#datasetOwner") { + if (!user) { + throw new HttpException( + { + status: HttpStatus.UNAUTHORIZED, + message: "User not authenticated", + }, + HttpStatus.UNAUTHORIZED, + ); + } datasetsWhere["where"]["ownerGroup"] = { $in: user.currentGroups }; } const numberOfDatasetsWithAccess = diff --git a/src/jobs/types/job-types.enum.ts b/src/jobs/types/job-types.enum.ts index 494ea25c7..e2fe2074d 100644 --- a/src/jobs/types/job-types.enum.ts +++ b/src/jobs/types/job-types.enum.ts @@ -1,15 +1,3 @@ -export enum JobType { - Archive = "archive", - Retrieve = "retrieve", - Public = "public", -} - -export enum DatasetState { - retrieve = "retrievable", - archive = "archivable", - public = "isPublished", -} - export enum JobParams { DatasetList = "datasetList", Pid = "pid", diff --git a/test/Jobs.js b/test/Jobs.js index 51defbdf1..bf906261d 100644 --- a/test/Jobs.js +++ b/test/Jobs.js @@ -1040,7 +1040,7 @@ describe("1100: Jobs: Test New Job Model", () => { .send(newDataset) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.ConflictStatusCode) + .expect(TestData.BadRequestStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); diff --git a/test/config/jobconfig.yaml b/test/config/jobconfig.yaml index cd61bea70..f9da012ca 100644 --- a/test/config/jobconfig.yaml +++ b/test/config/jobconfig.yaml @@ -40,16 +40,31 @@ jobs: - jobType: archive create: auth: "#all" + actions: + - actionType: validate + datasets: + datasetlifecycle.archivable: + const: true statusUpdate: auth: "#all" - jobType: retrieve create: auth: "#all" + actions: + - actionType: validate + datasets: + datasetlifecycle.retrievable: + const: true statusUpdate: auth: "#all" - jobType: public create: auth: "#all" + actions: + - actionType: validate + datasets: + isPublished: + const: true statusUpdate: auth: "#all" - jobType: validate