Skip to content

Commit

Permalink
fix: return jobStatus and not taskStatus(MAPCO-5662) (#101)
Browse files Browse the repository at this point in the history
* fix: return jobStatus and not taskStatus

* fix: ignore
  • Loading branch information
RonitKissis authored Dec 5, 2024
1 parent c2af06d commit 224d9d4
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,4 @@ reports
local.yaml

# Config
local.json
local*.json
6 changes: 6 additions & 0 deletions src/clients/jobManagerWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Tracer } from '@opentelemetry/api';
import { withSpanAsyncV4 } from '@map-colonies/telemetry';
import {
CreateExportJobBody,
GetJobResponse,
ICreateExportJobResponse,
IJobExportParameters,
ITaskParameters,
Expand Down Expand Up @@ -103,6 +104,11 @@ export class JobManagerWrapper extends JobManagerClient {
return tasks;
}

public async getJobByJobId(jobId: string): Promise<GetJobResponse> {
const job = await this.get<GetJobResponse>(`/jobs/${jobId}`);
return job;
}

public async findExportJob(
status: OperationStatus,
jobParams: JobExportDuplicationParams,
Expand Down
1 change: 1 addition & 0 deletions src/common/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export interface IGeometryRecord extends IGeometryRecordBase {
export type TaskResponse = ITaskResponse<ITaskParameters>;

// new API based on multi resolution
export type GetJobResponse = IJobResponse<IJobExportParameters, ITaskParameters>;
export type JobExportResponse = IJobResponse<IJobExportParameters, ITaskParameters>;
export type CreateExportJobBody = ICreateJobBody<IJobExportParameters, ITaskParameters>;
export type CreateFinalizeTaskBody = ICreateTaskBody<ITaskFinalizeParameters>;
Expand Down
6 changes: 3 additions & 3 deletions src/tasks/controllers/tasksController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { RequestHandler } from 'express';
import httpStatus from 'http-status-codes';
import { injectable, inject } from 'tsyringe';
import { SERVICES } from '../../common/constants';
import { ITaskStatusResponse, TasksManager } from '../models/tasksManager';
import { IJobStatusResponse, TasksManager } from '../models/tasksManager';

type GetTaskByJobIdHandler = RequestHandler<{ jobId: string }, ITaskStatusResponse, string>;
type GetTaskByJobIdHandler = RequestHandler<{ jobId: string }, IJobStatusResponse, string>;

@injectable()
export class TasksController {
Expand All @@ -14,7 +14,7 @@ export class TasksController {
public getTaskStatusByJobId: GetTaskByJobIdHandler = async (req, res, next) => {
const jobId: string = req.params.jobId;
try {
const taskStatus = await this.taskManager.getTaskStatusByJobId(jobId);
const taskStatus = await this.taskManager.getJobStatusByJobId(jobId);
return res.status(httpStatus.OK).json(taskStatus);
} catch (err) {
next(err);
Expand Down
17 changes: 17 additions & 0 deletions src/tasks/models/tasksManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ export interface ITaskStatusResponse {
status: OperationStatus;
}

export interface IJobStatusResponse {
percentage: number | undefined;
status: OperationStatus;
}

@injectable()
export class TasksManager {
private readonly gpkgsLocation: string;
Expand All @@ -54,6 +59,7 @@ export class TasksManager {
return job;
}

//This function is currently not in use and was switched to getJobStatusByJobId. wasn't deleted to enable future functionality if needed
@withSpanAsyncV4
public async getTaskStatusByJobId(jobId: string): Promise<ITaskStatusResponse> {
const tasks = await this.jobManagerClient.getTasksByJobId(jobId);
Expand All @@ -69,6 +75,17 @@ export class TasksManager {
return statusResponse;
}

@withSpanAsyncV4
public async getJobStatusByJobId(jobId: string): Promise<IJobStatusResponse> {
const job = await this.jobManagerClient.getJobByJobId(jobId);

const statusResponse: IJobStatusResponse = {
percentage: job.percentage,
status: job.status,
};
return statusResponse;
}

@withSpanAsyncV4
public async getExportJobsByTaskStatus(): Promise<IExportJobStatusResponse> {
const queryParams: IFindJobsRequest = {
Expand Down
2 changes: 1 addition & 1 deletion tests/configurations/unit/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ module.exports = {
coverageThreshold: {
global: {
branches: 74,
functions: 88,
functions: 87,
lines: 85,
statements: 85,
},
Expand Down
44 changes: 14 additions & 30 deletions tests/integration/createPackage/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@ import jsLogger from '@map-colonies/js-logger';
import { trace } from '@opentelemetry/api';
import httpStatusCodes from 'http-status-codes';
import { container } from 'tsyringe';
import { OperationStatus } from '@map-colonies/mc-priority-queue';
import { NotFoundError } from '@map-colonies/error-types';
import { getApp } from '../../../src/app';
import { SERVICES } from '../../../src/common/constants';
import { ITaskParameters, TaskResponse } from '../../../src/common/interfaces';
import { JobExportResponse, TaskResponse } from '../../../src/common/interfaces';
import { JobManagerWrapper } from '../../../src/clients/jobManagerWrapper';
import { mockCompletedJob } from '../../mocks/data/mockJob';
import { TasksSender } from './helpers/tasksSender';

describe('tasks', function () {
let requestSender: TasksSender;
let getTasksByJobIdSpy: jest.SpyInstance;
let getJobByJobIdSpy: jest.SpyInstance;

beforeEach(function () {
const app = getApp({
Expand All @@ -22,7 +23,7 @@ describe('tasks', function () {
useChild: true,
});
requestSender = new TasksSender(app);
getTasksByJobIdSpy = jest.spyOn(JobManagerWrapper.prototype, 'getTasksByJobId');
getJobByJobIdSpy = jest.spyOn(JobManagerWrapper.prototype, 'getJobByJobId');
});

afterEach(function () {
Expand All @@ -32,43 +33,26 @@ describe('tasks', function () {

describe('Happy Path', function () {
it('should return 200 status code and the tasks matched the jobId', async function () {
const tasksResponse: TaskResponse[] = [
{
id: '0a5552f7-01eb-40af-a912-eed8fa9e1561',
jobId: '0a5552f7-01eb-40af-a912-eed8fa9e1568',
type: 'export',
description: '',
parameters: {} as unknown as ITaskParameters,
status: OperationStatus.IN_PROGRESS,
percentage: 23,
reason: '',
attempts: 0,
resettable: true,
created: '2022-08-02T13:02:18.475Z',
updated: '2022-08-02T15:01:56.658Z',
},
];
const jobId = '09e29fa8-7283-4334-b3a4-99f75922de59';
getTasksByJobIdSpy.mockResolvedValue(tasksResponse);
const jobResponse = JSON.parse(JSON.stringify(mockCompletedJob)) as JobExportResponse;
getJobByJobIdSpy.mockResolvedValue(jobResponse);

const resposne = await requestSender.getTasksByJobId(jobId);
const resposne = await requestSender.getTasksByJobId(mockCompletedJob.id);

expect(resposne).toSatisfyApiSpec();
expect(getTasksByJobIdSpy).toHaveBeenCalledTimes(1);
expect(getJobByJobIdSpy).toHaveBeenCalledTimes(1);
expect(resposne.status).toBe(httpStatusCodes.OK);
});
});

describe('Sad Path', function () {
it('should return 404 status code due to invalid string format (uuid)', async function () {
const tasksResponse: TaskResponse[] = [];
it('should return 404 status code when no job was found', async function () {
const jobId = '09e29fa8-7283-4334-b3a4-99f75922de59';
getTasksByJobIdSpy.mockResolvedValue(tasksResponse);
getJobByJobIdSpy.mockRejectedValue(new NotFoundError('job not found'));

const resposne = await requestSender.getTasksByJobId(jobId);

expect(resposne).toSatisfyApiSpec();
expect(getTasksByJobIdSpy).toHaveBeenCalledTimes(1);
expect(getJobByJobIdSpy).toHaveBeenCalledTimes(1);
expect(resposne.status).toBe(httpStatusCodes.NOT_FOUND);
});
});
Expand All @@ -77,12 +61,12 @@ describe('tasks', function () {
it('should return 400 status code when jobId is not exists', async function () {
const tasksResponse: TaskResponse[] = [];
const jobId = 'string';
getTasksByJobIdSpy.mockResolvedValue(tasksResponse);
getJobByJobIdSpy.mockResolvedValue(tasksResponse);

const resposne = await requestSender.getTasksByJobId(jobId);

expect(resposne).toSatisfyApiSpec();
expect(getTasksByJobIdSpy).toHaveBeenCalledTimes(0);
expect(getJobByJobIdSpy).toHaveBeenCalledTimes(0);
expect(resposne.status).toBe(httpStatusCodes.BAD_REQUEST);
});
});
Expand Down
35 changes: 34 additions & 1 deletion tests/unit/createPackage/models/tasksModel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import jsLogger from '@map-colonies/js-logger';
import { OperationStatus } from '@map-colonies/mc-priority-queue';
import { NotFoundError } from '@map-colonies/error-types';
import { trace } from '@opentelemetry/api';
import { ITaskStatusResponse, TasksManager } from '../../../../src/tasks/models/tasksManager';
import { IJobStatusResponse, ITaskStatusResponse, TasksManager } from '../../../../src/tasks/models/tasksManager';
import {
CreateFinalizeTaskBody,
ICallbackExportData,
Expand All @@ -12,6 +12,7 @@ import {
JobExportResponse,
JobFinalizeResponse,
TaskResponse,
GetJobResponse,
} from '../../../../src/common/interfaces';
import { configMock, registerDefaultConfig } from '../../../mocks/config';
import { callbackClientMock, sendMock } from '../../../mocks/clients/callbackClient';
Expand All @@ -23,6 +24,7 @@ import { ArtifactType } from '../../../../src/common/enums';

let tasksManager: TasksManager;
let getTasksByJobIdStub: jest.Mock;
let getJobByJobIdStub: jest.Mock;

describe('TasksManager', () => {
beforeEach(() => {
Expand All @@ -37,6 +39,7 @@ describe('TasksManager', () => {
});

describe('ROI', () => {
// getTasksByJobId is currently not in use but left it and its tests to allow future functionality
describe('#getTaskStatusByJobId', () => {
it('should throw NotFoundError when jobId is not exists', async () => {
const emptyTasksResponse: TaskResponse[] = [];
Expand Down Expand Up @@ -82,6 +85,36 @@ describe('TasksManager', () => {
});
});

describe('#getJobStatusByJobId', () => {
it('should throw NotFoundError when jobId doesnt exist', async () => {
//const emptyTasksResponse: TaskResponse[] = [];

getJobByJobIdStub = jest.fn();
jobManagerWrapperMock.getJobByJobId = getJobByJobIdStub.mockRejectedValue(new NotFoundError('Job not found'));

const action = async () => tasksManager.getJobStatusByJobId('09e29fa8-7283-4334-b3a4-99f75922de59');

await expect(action).rejects.toThrow(NotFoundError);
expect(getJobByJobIdStub).toHaveBeenCalledTimes(1);
});

it('should successfully return job status by jobId', async () => {
const jobResponse = JSON.parse(JSON.stringify(mockCompletedJob)) as GetJobResponse;

getJobByJobIdStub = jest.fn();
jobManagerWrapperMock.getJobByJobId = getJobByJobIdStub.mockResolvedValue(jobResponse);

const result = tasksManager.getJobStatusByJobId(mockCompletedJob.id);
const expectedResult: IJobStatusResponse = {
percentage: jobResponse.percentage,
status: jobResponse.status,
};
await expect(result).resolves.not.toThrow();
await expect(result).resolves.toEqual(expectedResult);
expect(getJobByJobIdStub).toHaveBeenCalledTimes(1);
});
});

describe('#getJobsByTaskStatus', () => {
it('should return completed job with no failed jobs', async () => {
const jobs: JobFinalizeResponse[] = [];
Expand Down

0 comments on commit 224d9d4

Please sign in to comment.