diff --git a/.github/workflows/build_and_push.yaml b/.github/workflows/build_and_push.yaml index 1a290f0..5925aef 100644 --- a/.github/workflows/build_and_push.yaml +++ b/.github/workflows/build_and_push.yaml @@ -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 diff --git a/openapi3.yaml b/openapi3.yaml index ebe92ed..e46aaec 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -245,6 +245,7 @@ paths: /jobs/{jobId}/tasks: parameters: - $ref: '#/components/parameters/jobId' + - $ref: '#/components/parameters/shouldExcludeParameters' get: operationId: getTasks tags: @@ -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 @@ -785,6 +793,9 @@ components: jobId: type: string format: uuid + shouldExcludeParameters: + type: boolean + default: false taskId: type: string format: uuid diff --git a/src/DAL/repositories/jobRepository.ts b/src/DAL/repositories/jobRepository.ts index a22df31..4c5032a 100644 --- a/src/DAL/repositories/jobRepository.ts +++ b/src/DAL/repositories/jobRepository.ts @@ -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, @@ -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; @@ -60,7 +60,11 @@ export class JobRepository extends GeneralRepository { } } - const options: FindManyOptions = { where: filter }; + const options: FindManyOptions = { + where: filter, + select: excludeColumns(JobEntity, ['parameters']), + }; + if (req.shouldReturnTasks !== false) { options.relations = ['tasks']; } diff --git a/src/DAL/repositories/taskRepository.ts b/src/DAL/repositories/taskRepository.ts index 9450410..434ff89 100644 --- a/src/DAL/repositories/taskRepository.ts +++ b/src/DAL/repositories/taskRepository.ts @@ -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, @@ -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]; @@ -38,8 +39,12 @@ export class TaskRepository extends GeneralRepository { this.taskConvertor = container.resolve(TaskModelConvertor); } - public async getTasks(req: IAllTasksParams): Promise { - const entities = await this.find(req); + public async getTasks(req: IAllTasksParams, shouldExcludeParameters = false): Promise { + const options: FindManyOptions = { + 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; } diff --git a/src/common/dataModels/tasks.ts b/src/common/dataModels/tasks.ts index b3829d8..88ba118 100644 --- a/src/common/dataModels/tasks.ts +++ b/src/common/dataModels/tasks.ts @@ -98,3 +98,7 @@ export interface IFindTasksRequest extends Partial { creationTime?: Date; updateTime?: Date; } + +export interface IGetTasksQueryParams { + shouldExcludeParameters?: boolean; +} diff --git a/src/common/utils.ts b/src/common/utils.ts index 8cf3fbf..65c72dd 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -14,3 +14,11 @@ export const paramsQueryBuilder = (params: Record): string => { } return fullQuery; }; + +/* eslint-disable @typescript-eslint/naming-convention */ +export const excludeColumns = (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 */ diff --git a/src/jobs/controllers/taskController.ts b/src/jobs/controllers/taskController.ts index ca5dc8e..7fd704b 100644 --- a/src/jobs/controllers/taskController.ts +++ b/src/jobs/controllers/taskController.ts @@ -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'; @@ -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; -type GetResourcesHandler = RequestHandler; +type GetResourcesHandler = RequestHandler; type GetResourceHandler = RequestHandler; type DeleteResourceHandler = RequestHandler; type UpdateResourceHandler = RequestHandler; @@ -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); diff --git a/src/jobs/models/taskManager.ts b/src/jobs/models/taskManager.ts index f816e4d..08772a4 100644 --- a/src/jobs/models/taskManager.ts +++ b/src/jobs/models/taskManager.ts @@ -32,9 +32,9 @@ export class TaskManager { ) {} @withSpanAsyncV4 - public async getAllTasks(req: IAllTasksParams): Promise { + public async getAllTasks(req: IAllTasksParams, shouldExcludeParameters = false): Promise { const repo = await this.getRepository(); - const res = await repo.getTasks(req); + const res = await repo.getTasks(req, shouldExcludeParameters); return res; } diff --git a/tests/integration/jobs/helpers/tasksRequestSender.ts b/tests/integration/jobs/helpers/tasksRequestSender.ts index cb4532b..93c9dc4 100644 --- a/tests/integration/jobs/helpers/tasksRequestSender.ts +++ b/tests/integration/jobs/helpers/tasksRequestSender.ts @@ -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 { - return supertest.agent(this.app).get(`/jobs/${jobId}/tasks`).set('Content-Type', 'application/json'); + public async getAllResources(jobId: string, shouldExcludeParameters = false): Promise { + return supertest.agent(this.app).get(`/jobs/${jobId}/tasks`).query({ shouldExcludeParameters }).set('Content-Type', 'application/json'); } public async getResource(jobId: string, taskId: string): Promise { diff --git a/tests/integration/jobs/jobs.spec.ts b/tests/integration/jobs/jobs.spec.ts index 2d36502..da6f829 100644 --- a/tests/integration/jobs/jobs.spec.ts +++ b/tests/integration/jobs/jobs.spec.ts @@ -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'; @@ -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]); @@ -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]); @@ -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'); @@ -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); @@ -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); @@ -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(); }); }); diff --git a/tests/integration/jobs/tasks.spec.ts b/tests/integration/jobs/tasks.spec.ts index d13c870..f6fef3c 100644 --- a/tests/integration/jobs/tasks.spec.ts +++ b/tests/integration/jobs/tasks.spec.ts @@ -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'; @@ -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(); @@ -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 }, }); }); @@ -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;