diff --git a/package-lock.json b/package-lock.json index a8e9f45..1cb3857 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,7 @@ "@map-colonies/mc-priority-queue": "^9.1.0", "@map-colonies/mc-utils": "^5.1.0", "@map-colonies/openapi-express-viewer": "^3.0.0", - "@map-colonies/raster-shared": "^8.0.0-alpha", + "@map-colonies/raster-shared": "^8.1.0", "@map-colonies/read-pkg": "0.0.1", "@map-colonies/shapefile-reader": "^1.0.1", "@map-colonies/storage-explorer-middleware": "^1.3.0", @@ -7275,9 +7275,9 @@ "license": "ISC" }, "node_modules/@map-colonies/raster-shared": { - "version": "8.0.0-alpha", - "resolved": "https://registry.npmjs.org/@map-colonies/raster-shared/-/raster-shared-8.0.0-alpha.tgz", - "integrity": "sha512-W3qFta4ecvvLiZS+RCu/9YTErhBwODdcPJ6oy3v+OFPwGGr7Ztk7eyUE5+x8qXY1U0KH7CNT4sKE4StkJsYDJA==", + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/@map-colonies/raster-shared/-/raster-shared-8.1.0.tgz", + "integrity": "sha512-D/otgsJ7X00NnxlMJUSuj6rC9U779ReZDe4pAmtGsIOuQvASRnjjKIGIj/9nFJth6lbOaptppp0RRGImGd0jIw==", "license": "ISC", "dependencies": { "@map-colonies/mc-priority-queue": "^9.1.0", diff --git a/package.json b/package.json index 13e2671..6c06948 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "@map-colonies/mc-priority-queue": "^9.1.0", "@map-colonies/mc-utils": "^5.1.0", "@map-colonies/openapi-express-viewer": "^3.0.0", - "@map-colonies/raster-shared": "^8.0.0-alpha", + "@map-colonies/raster-shared": "^8.1.0", "@map-colonies/read-pkg": "0.0.1", "@map-colonies/shapefile-reader": "^1.0.1", "@map-colonies/storage-explorer-middleware": "^1.3.0", diff --git a/src/ingestion/models/ingestionManager.ts b/src/ingestion/models/ingestionManager.ts index 95fe2de..11bb734 100644 --- a/src/ingestion/models/ingestionManager.ts +++ b/src/ingestion/models/ingestionManager.ts @@ -1,5 +1,5 @@ -import { relative } from 'node:path'; import { randomUUID } from 'node:crypto'; +import { relative } from 'node:path'; import { ConflictError, NotFoundError } from '@map-colonies/error-types'; import { Logger } from '@map-colonies/js-logger'; import { @@ -12,15 +12,15 @@ import { } from '@map-colonies/mc-priority-queue'; import { getMapServingLayerName, + ingestionValidationTaskParamsSchema, inputFilesSchema, rasterProductTypeSchema, resourceIdSchema, - ingestionValidationTaskParamsSchema, - type IngestionValidationTaskParams, type Checksum as IChecksum, type IngestionNewJobParams, type IngestionSwapUpdateJobParams, type IngestionUpdateJobParams, + type IngestionValidationTaskParams, type InputFiles, type RasterProductTypes, } from '@map-colonies/raster-shared'; @@ -32,22 +32,23 @@ import { IConfig, ISupportedIngestionSwapTypes, LogContext } from '../../common/ import { InfoManager } from '../../info/models/infoManager'; import { CatalogClient } from '../../serviceClients/catalogClient'; import { JobManagerWrapper } from '../../serviceClients/jobManagerWrapper'; +import { JobTrackerClient } from '../../serviceClients/jobTrackerClient'; import { MapProxyClient } from '../../serviceClients/mapProxyClient'; +import { PolygonPartsManagerClient } from '../../serviceClients/polygonPartsManagerClient'; import { Checksum } from '../../utils/hash/checksum'; import { getAbsolutePathInputFiles } from '../../utils/paths'; import { getShapefileFiles } from '../../utils/shapefile'; +import { isKeyOf } from '../../utils/typeGuards'; import { ZodValidator } from '../../utils/validation/zodValidator'; import { ValidateManager } from '../../validate/models/validateManager'; import { ChecksumError, throwInvalidJobStatusError, UnsupportedEntityError } from '../errors/ingestionErrors'; +import type { IBypassValidationErrorsRequestBody, IngestionBaseJobParams, ResponseId } from '../interfaces'; import { IngestionOperation } from '../interfaces'; -import type { IngestionBaseJobParams, ResponseId, IBypassValidationErrorsRequestBody } from '../interfaces'; import type { RasterLayerMetadata } from '../schemas/layerCatalogSchema'; import type { IngestionNewLayer } from '../schemas/newLayerSchema'; import type { IngestionUpdateLayer } from '../schemas/updateLayerSchema'; import { GeoValidator } from '../validators/geoValidator'; import { SourceValidator } from '../validators/sourceValidator'; -import { PolygonPartsManagerClient } from '../../serviceClients/polygonPartsManagerClient'; -import { JobTrackerClient } from '../../serviceClients/jobTrackerClient'; import { ProductManager } from './productManager'; type ReplaceValuesOfKey, Key extends keyof T, Value> = { @@ -779,7 +780,11 @@ export class IngestionManager { } for (const [errorType, errorCount] of Object.entries(errorsSummary.errorsCount)) { - if (errorCount > 0 && !allowedValidationErrors.includes(errorType)) { + if ( + errorCount > 0 && + !allowedValidationErrors.includes(errorType) && + !(isKeyOf(errorType, errorsSummary.thresholds) && !errorsSummary.thresholds[errorType].exceeded) + ) { throw new UnsupportedEntityError('validation task has additional errors that are not in the allowed list'); } } diff --git a/src/utils/typeGuards.ts b/src/utils/typeGuards.ts new file mode 100644 index 0000000..f47526a --- /dev/null +++ b/src/utils/typeGuards.ts @@ -0,0 +1,3 @@ +export function isKeyOf>(key: PropertyKey, object: T): key is keyof T { + return key in object; +} diff --git a/tests/integration/ingestion/ingestion.spec.ts b/tests/integration/ingestion/ingestion.spec.ts index d6357f5..1d900ad 100644 --- a/tests/integration/ingestion/ingestion.spec.ts +++ b/tests/integration/ingestion/ingestion.spec.ts @@ -2264,6 +2264,40 @@ describe('Ingestion', () => { expect(response).toSatisfyApiSpec(); expect(response.status).toBe(httpStatusCodes.OK); }); + + it('should return 200 status code when successfully bypassing validation errors and ignoring un-allowed and below threshold errors', async () => { + const jobId = faker.string.uuid(); + const taskId = faker.string.uuid(); + const bypassJob = createBypassJob({ jobId }); + const requestBody = { allowedValidationErrors: ['resolution'], approver: 'approverName' }; + + const errorsSummary = { + errorsCount: { resolution: 1, smallHoles: 1 }, + thresholds: { resolution: { exceeded: false }, smallHoles: 1 }, + }; + const validationTask = { + id: taskId, + jobId, + type: configMock.get('jobManager.validationTaskType'), + status: OperationStatus.SUSPENDED, + parameters: { + isValid: false, + checksums: validInputFiles.checksums, + errorsSummary, + }, + }; + + nock(jobManagerURL).get(`/jobs/${jobId}`).query({ shouldReturnTasks: false }).reply(httpStatusCodes.OK, bypassJob); + nock(jobManagerURL).get(`/jobs/${jobId}/tasks`).reply(httpStatusCodes.OK, [validationTask]); + nock(jobManagerURL).put(`/jobs/${jobId}/tasks/${taskId}`).reply(httpStatusCodes.OK); + nock(jobManagerURL).put(`/jobs/${jobId}`).reply(httpStatusCodes.OK); + nock(configMock.get('services.jobTrackerServiceURL')).post(`/tasks/${taskId}/notify`).reply(httpStatusCodes.OK); + + const response = await requestSender.bypassValidationErrors(jobId, requestBody); + + expect(response).toSatisfyApiSpec(); + expect(response.status).toBe(httpStatusCodes.OK); + }); }); describe('Bad Path', () => { @@ -2382,6 +2416,36 @@ describe('Ingestion', () => { expect(response.status).toBe(httpStatusCodes.UNPROCESSABLE_ENTITY); }); + it('should return 422 UNPROCESSABLE_ENTITY when there are additional errors that are not in the allowed list that exceeded error threshold', async () => { + const jobId = faker.string.uuid(); + const taskId = faker.string.uuid(); + const bypassJob = createBypassJob({ jobId }); + const requestBody = { allowedValidationErrors: ['resolution'], approver: 'approverName' }; + + const validationTask = { + id: taskId, + jobId, + type: configMock.get('jobManager.validationTaskType'), + status: OperationStatus.SUSPENDED, + parameters: { + isValid: false, + checksums: validInputFiles.checksums, + errorsSummary: { + errorsCount: { resolution: 0, smallGeometries: 1 }, + thresholds: { resolution: { exceeded: false }, smallGeometries: { exceeded: true } }, + }, + }, + }; + + nock(jobManagerURL).get(`/jobs/${jobId}`).query({ shouldReturnTasks: false }).reply(httpStatusCodes.OK, bypassJob); + nock(jobManagerURL).get(`/jobs/${jobId}/tasks`).reply(httpStatusCodes.OK, [validationTask]); + + const response = await requestSender.bypassValidationErrors(jobId, requestBody); + + expect(response).toSatisfyApiSpec(); + expect(response.status).toBe(httpStatusCodes.UNPROCESSABLE_ENTITY); + }); + it('should return 422 UNPROCESSABLE_ENTITY when exceeded resolution threshold', async () => { const jobId = faker.string.uuid(); const taskId = faker.string.uuid(); diff --git a/tests/unit/ingestion/models/ingestionManager.spec.ts b/tests/unit/ingestion/models/ingestionManager.spec.ts index 561277c..cb4dc0f 100644 --- a/tests/unit/ingestion/models/ingestionManager.spec.ts +++ b/tests/unit/ingestion/models/ingestionManager.spec.ts @@ -1124,6 +1124,7 @@ describe('IngestionManager', () => { expect(mockPolygonPartsManagerClient.deleteValidationEntity).not.toHaveBeenCalled(); }); }); + describe('bypassValidationErrors', () => { let getJobSpy: jest.SpyInstance; let getTasksForJobSpy: jest.SpyInstance; @@ -1353,6 +1354,38 @@ describe('IngestionManager', () => { await expect(ingestionManager.bypassValidationErrors(body, mockJobId)).rejects.toThrow(UnsupportedEntityError); }); + it('should throw UnsupportedEntityError when un-allowed error exceeded threshold', async () => { + const mockJobId = faker.string.uuid(); + const mockJob = generateMockJob({ status: OperationStatus.SUSPENDED }); + const mockTask = { + id: faker.string.uuid(), + type: configMock.get('jobManager.validationTaskType'), + status: OperationStatus.SUSPENDED, + parameters: { + isValid: false, + errorsSummary: { + thresholds: { + resolution: { exceeded: false }, + smallHoles: { exceeded: true }, + }, + errorsCount: { errorType1: 1, smallHoles: 10 }, + }, + }, + jobId: mockJobId, + }; + + const body = { + allowedValidationErrors: ['errorType1'], + approver: 'admin', + jobId: mockJobId, + }; + + getJobSpy.mockResolvedValue(mockJob); + getTasksForJobSpy.mockResolvedValue([mockTask]); + + await expect(ingestionManager.bypassValidationErrors(body, mockJobId)).rejects.toThrow(UnsupportedEntityError); + }); + it('should throw ConflictError when checksums have changed', async () => { const mockJobId = faker.string.uuid(); const mockJob = generateMockJob({ status: OperationStatus.SUSPENDED });