Skip to content
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.

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

fix: return jobStatus and not taskStatus(MAPCO-5662) #101

Merged
merged 2 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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
RonitKissis marked this conversation as resolved.
Show resolved Hide resolved
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
Loading