Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 7 additions & 3 deletions .github/workflows/build_and_push.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@ permissions:

jobs:
build_and_push_docker:
uses: MapColonies/shared-workflows/.github/workflows/build-and-push-docker.yaml@v2
uses: MapColonies/shared-workflows/.github/workflows/build-and-push-docker.yaml@v4
secrets: inherit

with:
scope: common

build_and_push_helm:
uses: MapColonies/shared-workflows/.github/workflows/build-and-push-helm.yaml@v2
uses: MapColonies/shared-workflows/.github/workflows/build-and-push-helm.yaml@v4
secrets: inherit
with:
scope: common
11 changes: 11 additions & 0 deletions openapi3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ paths:
/jobs/{jobId}/tasks:
parameters:
- $ref: '#/components/parameters/jobId'
- $ref: '#/components/parameters/shouldExcludeParameters'
get:
operationId: getTasks
tags:
Expand Down Expand Up @@ -560,6 +561,13 @@ components:
required: true
schema:
$ref: '#/components/schemas/jobId'
shouldExcludeParameters:
in: query
name: shouldExcludeParameters
description: Whether to exclude the 'parameters' column from the response
required: false
schema:
$ref: '#/components/schemas/shouldExcludeParameters'
taskId:
in: path
name: taskId
Expand Down Expand Up @@ -785,6 +793,9 @@ components:
jobId:
type: string
format: uuid
shouldExcludeParameters:
type: boolean
default: false
taskId:
type: string
format: uuid
Expand Down
10 changes: 7 additions & 3 deletions src/DAL/repositories/jobRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import { ConflictError, NotFoundError } from '@map-colonies/error-types';
import { OperationStatus } from '@map-colonies/mc-priority-queue';
import { DBConstraintError } from '../../common/errors';
import { SERVICES } from '../../common/constants';
import { JobEntity } from '../entity/job';
import { paramsQueryBuilder } from '../../common/utils';
import { excludeColumns, paramsQueryBuilder } from '../../common/utils';
import {
FindJobsResponse,
ICreateJobBody,
Expand All @@ -18,6 +17,7 @@ import {
IFindJobsByCriteriaBody,
} from '../../common/dataModels/jobs';
import { JobModelConvertor } from '../convertors/jobModelConverter';
import { JobEntity } from '../entity/job';
import { GeneralRepository } from './generalRepository';

export type JobParameters = Record<string, unknown>;
Expand Down Expand Up @@ -60,7 +60,11 @@ export class JobRepository extends GeneralRepository<JobEntity> {
}
}

const options: FindManyOptions<JobEntity> = { where: filter };
const options: FindManyOptions<JobEntity> = {
where: filter,
select: excludeColumns(JobEntity, ['parameters']),
};

if (req.shouldReturnTasks !== false) {
options.relations = ['tasks'];
}
Expand Down
17 changes: 11 additions & 6 deletions src/DAL/repositories/taskRepository.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { EntityRepository, LessThan, Brackets, UpdateResult, In } from 'typeorm';
import { EntityRepository, LessThan, Brackets, UpdateResult, In, FindManyOptions } from 'typeorm';
import { container } from 'tsyringe';
import { Logger } from '@map-colonies/js-logger';
import { ConflictError, NotFoundError } from '@map-colonies/error-types';
import { OperationStatus } from '@map-colonies/mc-priority-queue';
import { SERVICES } from '../../common/constants';
import { TaskEntity } from '../entity/task';
import { TaskModelConvertor } from '../convertors/taskModelConvertor';
import {
CreateTasksRequest,
CreateTasksResponse,
Expand All @@ -20,8 +18,11 @@ import {
ITaskType,
IUpdateTaskRequest,
} from '../../common/dataModels/tasks';
import { JobEntity } from '../entity/job';
import { IJobAndTaskStatus } from '../../common/interfaces';
import { excludeColumns } from '../../common/utils';
import { TaskModelConvertor } from '../convertors/taskModelConvertor';
import { JobEntity } from '../entity/job';
import { TaskEntity } from '../entity/task';
import { GeneralRepository } from './generalRepository';

declare type SqlRawResponse = [unknown[], number];
Expand All @@ -38,8 +39,12 @@ export class TaskRepository extends GeneralRepository<TaskEntity> {
this.taskConvertor = container.resolve(TaskModelConvertor);
}

public async getTasks(req: IAllTasksParams): Promise<GetTasksResponse> {
const entities = await this.find(req);
public async getTasks(req: IAllTasksParams, shouldExcludeParameters = false): Promise<GetTasksResponse> {
const options: FindManyOptions<TaskEntity> = {
where: req,
select: shouldExcludeParameters ? excludeColumns(TaskEntity, ['parameters']) : undefined,
};
const entities = await this.find(options);
const models = entities.map((entity) => this.taskConvertor.entityToModel(entity));
return models;
}
Expand Down
4 changes: 4 additions & 0 deletions src/common/dataModels/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,7 @@ export interface IFindTasksRequest extends Partial<ICreateTaskBody> {
creationTime?: Date;
updateTime?: Date;
}

export interface IGetTasksQueryParams {
shouldExcludeParameters?: boolean;
}
8 changes: 8 additions & 0 deletions src/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ export const paramsQueryBuilder = (params: Record<string, unknown>): string => {
}
return fullQuery;
};

/* eslint-disable @typescript-eslint/naming-convention */
export const excludeColumns = <T extends object>(EntityClass: new () => T, exclude: string[]): (keyof T)[] => {
const entityInstance = new EntityClass();
const allColumns = Object.keys(entityInstance) as (keyof T)[];
return allColumns.filter((column) => !exclude.includes(column as string));
};
/* eslint-enable @typescript-eslint/naming-convention */
7 changes: 5 additions & 2 deletions src/jobs/controllers/taskController.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
import { ErrorResponse } from '@map-colonies/error-express-handler';
import { Logger } from '@map-colonies/js-logger';
import { NotFoundError } from '@map-colonies/error-types';
Expand All @@ -17,12 +18,13 @@ import {
CreateTasksRequest,
IFindTasksRequest,
IGetTasksStatus,
IGetTasksQueryParams,
} from '../../common/dataModels/tasks';
import { DefaultResponse } from '../../common/interfaces';
import { TaskManager } from '../models/taskManager';

type CreateResourceHandler = RequestHandler<IAllTasksParams, CreateTasksResponse, CreateTasksBody>;
type GetResourcesHandler = RequestHandler<IAllTasksParams, GetTasksResponse>;
type GetResourcesHandler = RequestHandler<IAllTasksParams, GetTasksResponse, undefined, IGetTasksQueryParams>;
type GetResourceHandler = RequestHandler<ISpecificTaskParams, IGetTaskResponse>;
type DeleteResourceHandler = RequestHandler<ISpecificTaskParams, DefaultResponse>;
type UpdateResourceHandler = RequestHandler<ISpecificTaskParams, DefaultResponse, IUpdateTaskBody>;
Expand Down Expand Up @@ -61,9 +63,10 @@ export class TaskController {
return next(err);
}
};

public getResources: GetResourcesHandler = async (req, res, next) => {
try {
const tasksRes = await this.manager.getAllTasks(req.params);
const tasksRes = await this.manager.getAllTasks(req.params, req.query.shouldExcludeParameters);
return res.status(httpStatus.OK).json(tasksRes);
} catch (err) {
return next(err);
Expand Down
4 changes: 2 additions & 2 deletions src/jobs/models/taskManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ export class TaskManager {
) {}

@withSpanAsyncV4
public async getAllTasks(req: IAllTasksParams): Promise<GetTasksResponse> {
public async getAllTasks(req: IAllTasksParams, shouldExcludeParameters = false): Promise<GetTasksResponse> {
const repo = await this.getRepository();
const res = await repo.getTasks(req);
const res = await repo.getTasks(req, shouldExcludeParameters);
return res;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/integration/jobs/helpers/tasksRequestSender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { IFindTasksRequest } from '../../../../src/common/dataModels/tasks';
export class TasksRequestSender {
public constructor(private readonly app: Express.Application) {}

public async getAllResources(jobId: string): Promise<supertest.Response> {
return supertest.agent(this.app).get(`/jobs/${jobId}/tasks`).set('Content-Type', 'application/json');
public async getAllResources(jobId: string, shouldExcludeParameters = false): Promise<supertest.Response> {
return supertest.agent(this.app).get(`/jobs/${jobId}/tasks`).query({ shouldExcludeParameters }).set('Content-Type', 'application/json');
}

public async getResource(jobId: string, taskId: string): Promise<supertest.Response> {
Expand Down
19 changes: 15 additions & 4 deletions tests/integration/jobs/jobs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import {
} from '../../mocks/DBMock';
import { getApp } from '../../../src/app';
import { FindJobsResponse, IAvailableActions, IGetJobResponse } from '../../../src/common/dataModels/jobs';
import { ResponseCodes } from '../../../src/common/constants';
import { TaskRepository } from '../../../src/DAL/repositories/taskRepository';
import { TaskEntity } from '../../../src/DAL/entity/task';
import { ResponseCodes } from '../../../src/common/constants';
import { JobManager } from '../../../src/jobs/models/jobManager';
import { JobsRequestSender, SearchJobsParams } from './helpers/jobsRequestSender';

Expand Down Expand Up @@ -418,7 +418,11 @@ describe('job', function () {

expect(response.status).toBe(httpStatusCodes.OK);
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({ relations: ['tasks'], where: {} });
expect(jobsFindMock).toHaveBeenCalledWith({
relations: ['tasks'],
where: {},
select: expect.not.arrayContaining(['parameters']) as string[],
});

const jobs = response.body as unknown;
expect(jobs).toEqual([jobModel]);
Expand Down Expand Up @@ -487,7 +491,11 @@ describe('job', function () {

expect(response.status).toBe(httpStatusCodes.OK);
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({ relations: ['tasks'], where: { internalId: '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d' } });
expect(jobsFindMock).toHaveBeenCalledWith({
relations: ['tasks'],
where: { internalId: '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d' },
select: expect.not.arrayContaining(['parameters']) as string[],
});

const jobs = response.body as unknown;
expect(jobs).toEqual([jobModel]);
Expand All @@ -508,6 +516,7 @@ describe('job', function () {
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({
where: { updateTime: 'moreThanOrEqualMock' },
select: expect.not.arrayContaining(['parameters']) as string[],
});
expect(moreThanOrEqualMock).toHaveBeenCalledTimes(1);
expect(moreThanOrEqualMock).toHaveBeenCalledWith('2000-01-01T00:00:00Z');
Expand All @@ -533,6 +542,7 @@ describe('job', function () {
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({
where: { updateTime: 'lessThanOrEqualMock' },
select: expect.not.arrayContaining(['parameters']) as string[],
});
expect(moreThanOrEqualMock).toHaveBeenCalledTimes(0);
expect(lessThanOrEqualMock).toHaveBeenCalledTimes(1);
Expand All @@ -557,6 +567,7 @@ describe('job', function () {
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({
where: { updateTime: 'betweenMock' },
select: expect.not.arrayContaining(['parameters']) as string[],
});
expect(moreThanOrEqualMock).toHaveBeenCalledTimes(0);
expect(lessThanOrEqualMock).toHaveBeenCalledTimes(0);
Expand Down Expand Up @@ -584,7 +595,7 @@ describe('job', function () {
expect(response.body).toEqual([]);
expect(response.status).toBe(httpStatusCodes.OK);
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({ where: filter });
expect(jobsFindMock).toHaveBeenCalledWith({ where: filter, select: expect.not.arrayContaining(['parameters']) as string[] });
expect(response).toSatisfyApiSpec();
});
});
Expand Down
38 changes: 36 additions & 2 deletions tests/integration/jobs/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { JobRepository } from '../../../src/DAL/repositories/jobRepository';
import { ICreateTaskBody, IGetTaskResponse, IGetTasksStatus } from '../../../src/common/dataModels/tasks';
import { IFindTasksRequest } from '../../../src/common/dataModels/tasks';
import { JobEntity } from '../../../src/DAL/entity/job';

import { getApp } from '../../../src/app';
import { ResponseCodes } from '../../../src/common/constants';
import { TasksRequestSender } from './helpers/tasksRequestSender';
Expand Down Expand Up @@ -171,10 +170,44 @@ describe('tasks', function () {
expect(response.status).toBe(httpStatusCodes.OK);
expect(taskFindMock).toHaveBeenCalledTimes(1);
expect(taskFindMock).toHaveBeenCalledWith({
where: { jobId: jobId },
});
const tasks = response.body as IGetTaskResponse[];
const entityFromResponse = convertTaskResponseToEntity(tasks[0]);
expect(entityFromResponse).toEqual(entityFromResponse);

expect(response).toSatisfyApiSpec();
});

it('should get all tasks without parameters and return 200', async function () {
const taskEntity = {
jobId: jobId,
id: taskId,
creationTime: new Date(Date.UTC(2000, 1, 2)),
updateTime: new Date(Date.UTC(2000, 1, 2)),
attempts: 0,
description: '1',
reason: '3',
percentage: 4,
type: '5',
status: OperationStatus.IN_PROGRESS,
resettable: false,
} as unknown as TaskEntity;

const taskFindMock = taskRepositoryMocks.findMock;
taskFindMock.mockResolvedValue([taskEntity]);

const response = await requestSender.getAllResources(jobId, true);

expect(response.status).toBe(httpStatusCodes.OK);
expect(taskFindMock).toHaveBeenCalledTimes(1);
expect(taskFindMock).toHaveBeenCalledWith({
where: { jobId: jobId },
select: expect.not.arrayContaining(['parameters']) as string[],
});
const tasks = response.body as IGetTaskResponse[];
const entityFromResponse = convertTaskResponseToEntity(tasks[0]);
expect(entityFromResponse).not.toHaveProperty('parameters');
expect(entityFromResponse).toEqual(entityFromResponse);

expect(response).toSatisfyApiSpec();
Expand All @@ -191,7 +224,7 @@ describe('tasks', function () {
expect(response.status).toBe(httpStatusCodes.OK);
expect(jobsFindMock).toHaveBeenCalledTimes(1);
expect(jobsFindMock).toHaveBeenCalledWith({
jobId: jobId,
where: { jobId: jobId },
});
});

Expand Down Expand Up @@ -361,6 +394,7 @@ describe('tasks', function () {
expect(response).toSatisfyApiSpec();
});
});

describe('Bad Path', function () {
it('should return status code 400 on PUT request with invalid body', async function () {
const taskCountMock = taskRepositoryMocks.countMock;
Expand Down
Loading