feat!: added final job statuses middleware handler on crud operations (MAPCO-9859)

openapi3.yaml

@@ -141,6 +141,12 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/errorMessage'
+        '409':
+          description: Conflict
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/errorMessage'
         '404':
           description: Job not found
           content:
@@ -236,6 +242,12 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/errorMessage'
+        '409':
+          description: Conflict
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/errorMessage'
         '500':
           description: Internal Server Error
           content:
@@ -550,6 +562,12 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/errorMessage'
+        '409':
+          description: Conflict
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/errorMessage'
       tags:
         - tasksManagement
 components:

src/DAL/repositories/jobRepository.ts

@@ -269,14 +269,4 @@ export class JobRepository extends GeneralRepository<JobEntity> {
 
     return resettableJobsCount > 0;
   }
-
-  public async isJobHasPendingTasks(jobId: string): Promise<boolean> {
-    const pendingTasksCount = await this.createQueryBuilder('job')
-      .leftJoinAndSelect('job.tasks', 'task')
-      .where('job.id = :jobId', { jobId })
-      .andWhere('task.status = :status', { status: OperationStatus.PENDING })
-      .getCount();
-
-    return pendingTasksCount > 0;
-  }
 }

src/common/dataModels/jobs.ts

@@ -99,7 +99,7 @@ export interface IGetJobResponse {
   tasks?: GetTasksResponse;
   created: Date;
   updated: Date;
-  status?: OperationStatus;
+  status: OperationStatus;
   percentage?: number;
   isCleaned: boolean;
   priority?: number;

src/common/middlewares/validateJobStatusMiddleware.ts

@@ -0,0 +1,28 @@
+import { Request, Response, NextFunction } from 'express';
+import { OperationStatus } from '@map-colonies/mc-priority-queue';
+import { ConflictError } from '@map-colonies/error-types';
+import { Logger } from '@map-colonies/js-logger';
+import { container } from 'tsyringe';
+import { SERVICES } from '../constants';
+import { JobManager } from '../../jobs/models/jobManager';
+
+export const validateJobStatusMiddleware = async (req: Request<{ jobId: string }>, res: Response, next: NextFunction): Promise<void> => {
+  const jobManager = container.resolve(JobManager);
+  const logger = container.resolve<Logger>(SERVICES.LOGGER);
+
+  const finalStateStatuses: OperationStatus[] = [OperationStatus.COMPLETED, OperationStatus.ABORTED, OperationStatus.EXPIRED];
+  try {
+    const jobId = req.params.jobId;
+    const job = await jobManager.getJob({ jobId }, { shouldReturnTasks: false, shouldReturnAvailableActions: false });
+
+    if (finalStateStatuses.includes(job.status)) {
+      const errorMessage = `Cannot perform the requested operation on job with final state status: ${job.status}`;
+      logger.error({ msg: errorMessage, jobId, status: job.status });
+      throw new ConflictError(errorMessage);
+    }
+
+    next();
+  } catch (error) {
+    next(error);
+  }
+};

src/jobs/routes/jobRouter.ts

@@ -1,7 +1,9 @@
+/* eslint-disable @typescript-eslint/no-misused-promises */
 import { Router } from 'express';
 import { FactoryFunction } from 'tsyringe';
 import { JobController } from '../controllers/jobController';
 import { TaskController } from '../controllers/taskController';
+import { validateJobStatusMiddleware } from '../../common/middlewares/validateJobStatusMiddleware';
 
 const jobRouterFactory: FactoryFunction<Router> = (dependencyContainer) => {
   const router = Router();
@@ -13,13 +15,13 @@ const jobRouterFactory: FactoryFunction<Router> = (dependencyContainer) => {
   router.post('/', jobsController.createResource);
   router.get('/parameters', jobsController.getJobByJobsParameters);
   router.get('/:jobId', jobsController.getResource);
-  router.put('/:jobId', jobsController.updateResource);
+  router.put('/:jobId', validateJobStatusMiddleware, jobsController.updateResource);
   router.delete('/:jobId', jobsController.deleteResource);
   router.post('/:jobId/resettable', jobsController.isResettable);
-  router.post('/:jobId/reset', jobsController.resetJob);
+  router.post('/:jobId/reset', validateJobStatusMiddleware, jobsController.resetJob);
 
   router.get('/:jobId/tasks', tasksController.getResources);
-  router.post('/:jobId/tasks', tasksController.createResource);
+  router.post('/:jobId/tasks', validateJobStatusMiddleware, tasksController.createResource);
   router.get('/:jobId/tasks/:taskId', tasksController.getResource);
   router.put('/:jobId/tasks/:taskId', tasksController.updateResource);
   router.delete('/:jobId/tasks/:taskId', tasksController.deleteResource);

src/taskManagement/models/taskManagementManger.ts

@@ -1,5 +1,5 @@
 import { Logger } from '@map-colonies/js-logger';
-import { NotFoundError, BadRequestError } from '@map-colonies/error-types';
+import { NotFoundError } from '@map-colonies/error-types';
 import { Tracer } from '@opentelemetry/api';
 import { withSpanAsyncV4 } from '@map-colonies/telemetry';
 import { inject, injectable } from 'tsyringe';
@@ -61,11 +61,7 @@ export class TaskManagementManager {
       this.logger.error({ jobId: req.jobId, msg: message });
       throw new NotFoundError(message);
     }
-    if ((jobEntity.status as OperationStatus) === OperationStatus.COMPLETED || (jobEntity.status as OperationStatus) === OperationStatus.ABORTED) {
-      const message = 'Job abort request failed, job status cannot be one of: "Completed" or "Aborted"';
-      this.logger.error({ jobStatus: jobEntity.status, msg: message });
-      throw new BadRequestError(message);
-    }
+
     await jobRepo.updateJob({ jobId: req.jobId, status: OperationStatus.ABORTED });
     const taskRepo = await this.getTaskRepository();
     await taskRepo.abortJobTasks(req.jobId);

src/taskManagement/routes/taskManagerRouter.ts

@@ -1,7 +1,9 @@
+/* eslint-disable @typescript-eslint/no-misused-promises */
 import { Router } from 'express';
 import { FactoryFunction } from 'tsyringe';
 import { TaskManagementController } from '../controllers/taskManagementController';
 import { TaskController } from '../../jobs/controllers/taskController';
+import { validateJobStatusMiddleware } from '../../common/middlewares/validateJobStatusMiddleware';
 
 const taskManagerRouterFactory: FactoryFunction<Router> = (dependencyContainer) => {
   const router = Router();
@@ -15,7 +17,7 @@ const taskManagerRouterFactory: FactoryFunction<Router> = (dependencyContainer)
   router.post('/findInactive', tasksManagementController.findInactiveTasks);
   router.post('/releaseInactive', tasksManagementController.releaseInactive);
   router.post('/updateExpiredStatus', tasksManagementController.updateExpiredStatus);
-  router.post('/abort/:jobId', tasksManagementController.abort);
+  router.post('/abort/:jobId', validateJobStatusMiddleware, tasksManagementController.abort);
 
   return router;
 };

tests/integration/jobs/jobs.spec.ts

@@ -140,13 +140,6 @@ function createJobDataForGetJob(): unknown {
   return jobModel;
 }
 
-function isJobHasPendingTasksMock(tasks: TaskEntity[] | undefined): boolean {
-  if (tasks === undefined) {
-    return false;
-  }
-  return tasks.some((task) => task.status === OperationStatus.PENDING);
-}
-
 function createJobDataForAvailableActionsWithoutAbortableJob(): unknown {
   const taskModel = {
     jobId: '170dd8c0-8bad-498b-bb26-671dcf19aa3c',
@@ -308,6 +301,8 @@ describe('job', function () {
   });
 
   describe('Happy Path', function () {
+    const statusCases = [OperationStatus.PENDING, OperationStatus.IN_PROGRESS, OperationStatus.SUSPENDED, OperationStatus.FAILED];
+
     describe('createJob', () => {
       it('should create job with tasks and return status code 201 and the created job and tasks ids', async function () {
         const createTaskModel1 = {
@@ -438,8 +433,6 @@ describe('job', function () {
         const jobModel = createJobDataForFind();
         const jobEntity = jobModelToEntity(jobModel);
         const jobsFindMock = jobRepositoryMocks.findMock;
-        const isJobHasPendingTasksSpy = jest.spyOn(JobRepository.prototype, 'isJobHasPendingTasks');
-        isJobHasPendingTasksSpy.mockResolvedValue(true);
         jobRepositoryMocks.queryMock.mockResolvedValue([{ unResettableTasks: '1', failedTasks: '3' }]);
         const findJobsSpy = jest.spyOn(JobManager.prototype, 'findJobs');
         jobsFindMock.mockResolvedValue([jobEntity]);
@@ -772,8 +765,6 @@ describe('job', function () {
         const getJobSpy = jest.spyOn(JobManager.prototype, 'getJob');
         delete jobEntity.tasks;
         jobsFindOneMock.mockResolvedValue(jobEntity);
-        const isJobHasPendingTasksSpy = jest.spyOn(JobRepository.prototype, 'isJobHasPendingTasks');
-        isJobHasPendingTasksSpy.mockResolvedValue(true);
         const expectedAvailableActions: IAvailableActions = {
           isAbortable: true,
           isResumable: false,
@@ -805,9 +796,6 @@ describe('job', function () {
         const jobsFindOneMock = jobRepositoryMocks.findOneMock;
         jobRepositoryMocks.queryMock.mockResolvedValue([{ unResettableTasks: '1', failedTasks: '3' }]);
         const getJobSpy = jest.spyOn(JobManager.prototype, 'getJob');
-        const isJobHasPendingTasksSpy = jest.spyOn(JobRepository.prototype, 'isJobHasPendingTasks');
-        const condition = isJobHasPendingTasksMock(jobEntity.tasks);
-        isJobHasPendingTasksSpy.mockResolvedValue(condition);
         delete jobEntity.tasks;
         jobsFindOneMock.mockResolvedValue(jobEntity);
         const expectedAvailableActions: IAvailableActions = {
@@ -848,9 +836,6 @@ describe('job', function () {
           jobRepositoryMocks.queryMock.mockResolvedValue([{ unResettableTasks: '1', failedTasks: '3' }]);
 
           const getJobSpy = jest.spyOn(JobManager.prototype, 'getJob');
-          const isJobHasPendingTasksSpy = jest.spyOn(JobRepository.prototype, 'isJobHasPendingTasks');
-          const condition = isJobHasPendingTasksMock(jobEntity.tasks);
-          isJobHasPendingTasksSpy.mockResolvedValue(condition);
 
           delete jobEntity.tasks;
           jobsFindOneMock.mockResolvedValue(jobEntity);
@@ -876,7 +861,6 @@ describe('job', function () {
           expect(response).toSatisfyApiSpec();
 
           getJobSpy.mockRestore();
-          isJobHasPendingTasksSpy.mockRestore();
         }
       );
 
@@ -910,23 +894,24 @@ describe('job', function () {
         getJobSpy.mockRestore();
       });
 
-      it('should update job status and return 200', async function () {
+      it.each(statusCases)('should update job status to %s and return 200', async function (status) {
         const jobCountMock = jobRepositoryMocks.countMock;
         const jobSaveMock = jobRepositoryMocks.saveMock;
 
+        jobRepositoryMocks.findOneMock.mockResolvedValue({ status: OperationStatus.PENDING });
         jobCountMock.mockResolvedValue(1);
         jobSaveMock.mockResolvedValue({});
 
         const response = await requestSender.updateResource('170dd8c0-8bad-498b-bb26-671dcf19aa3c', {
-          status: 'In-Progress',
+          status,
         });
 
         expect(response.status).toBe(httpStatusCodes.OK);
         expect(response.body).toEqual({ code: ResponseCodes.JOB_UPDATED });
         expect(jobSaveMock).toHaveBeenCalledTimes(1);
         expect(jobSaveMock).toHaveBeenCalledWith({
           id: '170dd8c0-8bad-498b-bb26-671dcf19aa3c',
-          status: 'In-Progress',
+          status,
         });
         expect(response).toSatisfyApiSpec();
       });
@@ -1114,6 +1099,14 @@ describe('job', function () {
         jobRepositoryMocks.queryMock.mockResolvedValue([{ unResettableTasks: '1', failedTasks: '3' }]);
         const id = 'dabf6137-8160-4b62-9110-2d1c1195398b';
 
+        jobRepositoryMocks.findOneMock.mockResolvedValue({
+          id,
+          status: OperationStatus.FAILED,
+          isCleaned: false,
+        });
+        jobRepositoryMocks.countMock.mockResolvedValue(0);
+        jobRepositoryMocks.queryBuilder.getCount.mockResolvedValue(0);
+
         const body = {
           newExpirationDate: undefined,
         };
@@ -1122,7 +1115,7 @@ describe('job', function () {
         expect(res.status).toBe(httpStatusCodes.BAD_REQUEST);
         expect(queryRunnerMocks.connect).toHaveBeenCalledTimes(1);
         expect(queryRunnerMocks.startTransaction).toHaveBeenCalledTimes(1);
-        expect(queryRunnerMocks.manager.getCustomRepository).toHaveBeenCalledTimes(1);
+        expect(queryRunnerMocks.manager.getCustomRepository).toHaveBeenCalledTimes(2);
         expect(queryRunnerMocks.commitTransaction).toHaveBeenCalledTimes(0);
         expect(queryRunnerMocks.rollbackTransaction).toHaveBeenCalledTimes(1);
         expect(queryRunnerMocks.release).toHaveBeenCalledTimes(1);
@@ -1147,23 +1140,33 @@ describe('job', function () {
     });
 
     it('should return status code 404 on PUT request for non existing job', async function () {
-      const jobCountMock = jobRepositoryMocks.countMock;
-      const jobSaveMock = jobRepositoryMocks.saveMock;
-      jobCountMock.mockResolvedValue(0);
+      jobRepositoryMocks.findOneMock.mockResolvedValue(undefined);
 
       const response = await requestSender.updateResource('170dd8c0-8bad-498b-bb26-671dcf19aa3c', {
         status: 'Pending',
       });
 
-      expect(jobCountMock).toHaveBeenCalledTimes(1);
-      expect(jobCountMock).toHaveBeenCalledWith({
-        id: '170dd8c0-8bad-498b-bb26-671dcf19aa3c',
-      });
-      expect(jobSaveMock).toHaveBeenCalledTimes(0);
       expect(response.status).toBe(httpStatusCodes.NOT_FOUND);
       expect(response).toSatisfyApiSpec();
     });
 
+    it.each([OperationStatus.COMPLETED, OperationStatus.EXPIRED, OperationStatus.ABORTED])(
+      'should return status conflict error on update job attempt when trying to update job with status %s',
+      async function (status) {
+        jobRepositoryMocks.findOneMock.mockResolvedValue({
+          id: '170dd8c0-8bad-498b-bb26-671dcf19aa3c',
+          status: status,
+        });
+
+        const response = await requestSender.updateResource('170dd8c0-8bad-498b-bb26-671dcf19aa3c', {
+          status: OperationStatus.COMPLETED,
+        });
+
+        expect(response.status).toBe(httpStatusCodes.CONFLICT);
+        expect(response).toSatisfyApiSpec();
+      }
+    );
+
     it('should return status code 404 on DELETE request for non existing job', async function () {
       const jobCountMock = jobRepositoryMocks.countMock;
       const jobDeleteMock = jobRepositoryMocks.deleteMock;
@@ -1203,10 +1206,16 @@ describe('job', function () {
     describe('reset', () => {
       it('returns 400 when job is not resettable', async () => {
         const id = 'dabf6137-8160-4b62-9110-2d1c1195398b';
+        jobRepositoryMocks.findOneMock.mockResolvedValue({
+          id,
+          status: OperationStatus.FAILED,
+          isCleaned: false,
+        });
         jobRepositoryMocks.queryBuilder.getCount.mockResolvedValue(0);
         const body = {
           newExpirationDate: undefined,
         };
+
         const res = await requestSender.reset(id, body);
 
         expect(res.status).toBe(httpStatusCodes.BAD_REQUEST);
@@ -1217,10 +1226,39 @@ describe('job', function () {
         expect(queryRunnerMocks.release).toHaveBeenCalledTimes(1);
 
         expect(jobRepositoryMocks.queryBuilder.getCount).toHaveBeenCalledTimes(1);
-        expect(jobRepositoryMocks.findOneMock).toHaveBeenCalledTimes(0);
+        expect(jobRepositoryMocks.findOneMock).toHaveBeenCalledTimes(1);
         expect(taskRepositoryMocks.queryBuilder.execute).toHaveBeenCalledTimes(0);
         expect(res).toSatisfyApiSpec();
       });
+
+      it.each([OperationStatus.COMPLETED, OperationStatus.EXPIRED, OperationStatus.ABORTED])(
+        'should return conflict error when job status is: %s while resetting',
+        async function (status) {
+          const id = 'dabf6137-8160-4b62-9110-2d1c1195398b';
+
+          jobRepositoryMocks.findOneMock.mockResolvedValue({
+            id,
+            status,
+            isCleaned: false,
+          });
+
+          jobRepositoryMocks.countMock.mockResolvedValue(1);
+          jobRepositoryMocks.queryBuilder.getCount.mockResolvedValue(1);
+
+          const body = {
+            newExpirationDate: undefined,
+          };
+
+          const res = await requestSender.reset(id, body);
+
+          expect(res.status).toBe(httpStatusCodes.CONFLICT);
+          expect(jobRepositoryMocks.findOneMock).toHaveBeenCalledTimes(1);
+          expect(jobRepositoryMocks.findOneMock).toHaveBeenCalledWith(id);
+          expect(queryRunnerMocks.connect).toHaveBeenCalledTimes(0);
+          expect(taskRepositoryMocks.queryBuilder.execute).toHaveBeenCalledTimes(0);
+          expect(res).toSatisfyApiSpec();
+        }
+      );
     });
   });
 });