Skip to content

fix: bypass attempt with isValid as true does not indicate for an error (MAPCO-10606) #90

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

Merged
vitaligi merged 2 commits into from
May 13, 2026
+63 −52
Merged

fix: bypass attempt with isValid as true does not indicate for an error (MAPCO-10606) #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2502841
fix: throw UnsupportedEntityError on invalid task or suspended job
vitaligi May 12, 2026
7c4c191
test: fix test name error type
vitaligi May 13, 2026
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
75 changes: 43 additions & 32 deletions src/ingestion/models/ingestionManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { relative } from 'node:path';
import { randomUUID } from 'node:crypto';
import { BadRequestError, ConflictError, NotFoundError } from '@map-colonies/error-types';
import { ConflictError, NotFoundError } from '@map-colonies/error-types';
import { Logger } from '@map-colonies/js-logger';
import {
IFindJobsByCriteriaBody,
Expand Down Expand Up @@ -234,37 +234,7 @@ export class IngestionManager {
const job: IJobResponse<IngestionBaseJobParams, unknown> = await this.jobManagerWrapper.getJob<IngestionBaseJobParams, unknown>(jobId);
const validationTask = await this.getValidationTask(jobId, { ...logCtx });

if (validationTask.parameters.isValid === true) {
return;
}
if (job.status !== OperationStatus.SUSPENDED) {
throw new BadRequestError('cannot bypass validation errors when the job is not suspended');
}
if (validationTask.parameters.errorsSummary === undefined) {
throw new UnsupportedEntityError('cannot bypass validation errors when there are no validation errors in task params');
}

const errorsSummary = validationTask.parameters.errorsSummary;
const exceededResolutionThreshold = errorsSummary.thresholds.resolution.exceeded;

for (const [errorType, errorCount] of Object.entries(errorsSummary.errorsCount)) {
if (errorCount > 0 && !allowedValidationErrors.includes(errorType)) {
throw new UnsupportedEntityError('validation task has additional errors that are not in the allowed list');
}
}
if (exceededResolutionThreshold) {
throw new UnsupportedEntityError('cannot bypass validation error of type: resolution, because the resolution exceeded threshold');
}

const existingChecksums = validationTask.parameters.checksums;
const metadataShapefilePath = await this.validateAndGetAbsoluteInputFiles(job.parameters.inputFiles);
const newChecksums = await this.getChecksum(metadataShapefilePath.metadataShapefilePath);
if (this.isChecksumChanged(existingChecksums, newChecksums)) {
throw new ConflictError(
'cannot bypass validation errors because the metadata shapefile has been changed since the validation was performed,re-run the process'
);
}

await this.canBypassValidationTask({ ...body, ingestionValidationTaskParams: validationTask.parameters, job });
await this.makeValidationTaskCompleted(validationTask);
await this.jobManagerWrapper.updateJob(jobId, {
status: OperationStatus.IN_PROGRESS,
Expand Down Expand Up @@ -783,6 +753,47 @@ export class IngestionManager {
}));
}

private async canBypassValidationTask(
options: {
ingestionValidationTaskParams: IngestionValidationTaskParams;
job: IJobResponse<IngestionBaseJobParams, unknown>;
} & IBypassValidationErrorsRequestBody
): Promise<void> {
const { allowedValidationErrors, ingestionValidationTaskParams, job } = options;

if (ingestionValidationTaskParams.isValid === true) {
throw new UnsupportedEntityError('cannot bypass validation errors since the validation was already successfully completed');
}
if (job.status !== OperationStatus.SUSPENDED) {
throw new UnsupportedEntityError('cannot bypass validation errors when the job is not suspended');
}
if (ingestionValidationTaskParams.errorsSummary === undefined) {
throw new UnsupportedEntityError('cannot bypass validation errors when there are no validation errors in task params');
}

const errorsSummary = ingestionValidationTaskParams.errorsSummary;
const exceededResolutionThreshold = errorsSummary.thresholds.resolution.exceeded;

if (exceededResolutionThreshold) {
throw new UnsupportedEntityError('cannot bypass validation error of type: resolution, because the resolution exceeded threshold');
}

for (const [errorType, errorCount] of Object.entries(errorsSummary.errorsCount)) {
if (errorCount > 0 && !allowedValidationErrors.includes(errorType)) {
throw new UnsupportedEntityError('validation task has additional errors that are not in the allowed list');
}
}

const existingChecksums = ingestionValidationTaskParams.checksums;
const metadataShapefilePath = await this.validateAndGetAbsoluteInputFiles(job.parameters.inputFiles);
const newChecksums = await this.getChecksum(metadataShapefilePath.metadataShapefilePath);
if (this.isChecksumChanged(existingChecksums, newChecksums)) {
throw new ConflictError(
'cannot bypass validation errors because the metadata shapefile has been changed since the validation was performed, re-run the process'
);
}
}

private async makeValidationTaskCompleted(task: ITaskResponse<IngestionValidationTaskParams>): Promise<void> {
try {
await this.jobManagerWrapper.updateTask(task.jobId, task.id, {
Expand Down
30 changes: 14 additions & 16 deletions tests/integration/ingestion/ingestion.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2260,25 +2260,26 @@ describe('Ingestion', () => {
nock(configMock.get<string>('services.jobTrackerServiceURL')).post(`/tasks/${taskId}/notify`).reply(httpStatusCodes.OK);

const response = await requestSender.bypassValidationErrors(jobId, requestBody);
console.log('BYPASS RES:', response.body);

expect(response).toSatisfyApiSpec();
expect(response.status).toBe(httpStatusCodes.OK);
});
});

it('should return 200 when task is valid', async () => {
describe('Bad Path', () => {
it('should return 422 UNPROCESSABLE_ENTITY when job is not suspended', async () => {
const jobId = faker.string.uuid();
const taskId = faker.string.uuid();
const bypassJob = createBypassJob({ jobId });
const bypassJob = createBypassJob({ jobId, status: OperationStatus.FAILED });
const requestBody = { allowedValidationErrors: ['resolution'], approver: 'approverName' };

const validationTask = {
id: taskId,
jobId,
type: configMock.get<string>('jobManager.validationTaskType'),
status: OperationStatus.SUSPENDED,
status: OperationStatus.FAILED,
parameters: {
isValid: true,
isValid: false,
checksums: validInputFiles.checksums,
errorsSummary: {
errorsCount: { resolution: 1 },
Expand All @@ -2293,28 +2294,25 @@ describe('Ingestion', () => {
const response = await requestSender.bypassValidationErrors(jobId, requestBody);

expect(response).toSatisfyApiSpec();
expect(response.status).toBe(httpStatusCodes.OK);
expect(response.status).toBe(httpStatusCodes.UNPROCESSABLE_ENTITY);
});
});

describe('Bad Path', () => {
it('should return 400 BAD_REQUEST when job is not suspended', async () => {
it('should return 422 UNPROCESSABLE_ENTITY when validation task was already successfully accomplished', async () => {
const jobId = faker.string.uuid();
const taskId = faker.string.uuid();
const bypassJob = createBypassJob({ jobId, status: OperationStatus.PENDING });
const bypassJob = createBypassJob({ jobId, status: OperationStatus.COMPLETED });
const requestBody = { allowedValidationErrors: ['resolution'], approver: 'approverName' };

const validationTask = {
id: taskId,
jobId,
type: configMock.get<string>('jobManager.validationTaskType'),
status: OperationStatus.FAILED,
status: OperationStatus.COMPLETED,
parameters: {
isValid: false,
isValid: true,
checksums: validInputFiles.checksums,
errorsSummary: {
errorsCount: { resolution: 1 },
thresholds: { resolution: { exceeded: false } },
errorsCount: { resolution: 0, geometryValidity: 0, metadata: 0, smallGeometries: 0, smallHoles: 0, unknown: 0, vertices: 0 },
thresholds: { resolution: { exceeded: false }, smallGeometries: { exceeded: false }, smallHoles: { count: 0, exceeded: false } },
},
},
};
Expand All @@ -2325,7 +2323,7 @@ describe('Ingestion', () => {
const response = await requestSender.bypassValidationErrors(jobId, requestBody);

expect(response).toSatisfyApiSpec();
expect(response.status).toBe(httpStatusCodes.BAD_REQUEST);
expect(response.status).toBe(httpStatusCodes.UNPROCESSABLE_ENTITY);
});

it('should return 422 UNPROCESSABLE_ENTITY when there are no validation errors in task params', async () => {
Expand Down
10 changes: 6 additions & 4 deletions tests/unit/ingestion/models/ingestionManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ describe('IngestionManager', () => {
expect(mockJobTrackerClient.notify).toHaveBeenCalledWith(mockTask);
});

it('should return and do nothing when task is valid', async () => {
it('should throw UnsupportedEntityError when task is valid', async () => {
const mockJobId = faker.string.uuid();
const mockJob = generateMockJob({ status: OperationStatus.SUSPENDED });
const mockTask = {
Expand All @@ -1235,11 +1235,12 @@ describe('IngestionManager', () => {
getJobSpy.mockResolvedValue(mockJob);
getTasksForJobSpy.mockResolvedValue([mockTask]);

await ingestionManager.bypassValidationErrors(body, mockJobId);
const response = ingestionManager.bypassValidationErrors(body, mockJobId);
expect(mockJobTrackerClient.notify).not.toHaveBeenCalled();
await expect(response).rejects.toThrow(UnsupportedEntityError);
});

it('should throw BadRequestError if job is not suspended', async () => {
it('should throw UnsupportedEntityError if job is not suspended', async () => {
const mockJobId = faker.string.uuid();
const mockJob = generateMockJob({ status: OperationStatus.PENDING });
const mockTask = {
Expand All @@ -1265,7 +1266,8 @@ describe('IngestionManager', () => {
getJobSpy.mockResolvedValue(mockJob);
getTasksForJobSpy.mockResolvedValue([mockTask]);

await expect(ingestionManager.bypassValidationErrors(body, mockJobId)).rejects.toThrow(BadRequestError);
const response = ingestionManager.bypassValidationErrors(body, mockJobId);
await expect(response).rejects.toThrow(UnsupportedEntityError);
});

it('should throw UnsupportedEntityError if task has unallowed errors', async () => {
Expand Down
Loading