From f898f226bef16184785ad49cb3045a46f84274a7 Mon Sep 17 00:00:00 2001 From: JORGE Date: Wed, 11 Dec 2024 12:01:52 -0400 Subject: [PATCH] [TM-1531] some changes for dtos --- .../src/jobs/delayed-jobs.controller.spec.ts | 215 ++++++++---------- .../src/jobs/delayed-jobs.controller.ts | 45 ++-- .../src/jobs/dto/delayed-job-update.dto.ts | 22 +- .../src/lib/entities/delayed-job.entity.ts | 6 +- 4 files changed, 140 insertions(+), 148 deletions(-) diff --git a/apps/job-service/src/jobs/delayed-jobs.controller.spec.ts b/apps/job-service/src/jobs/delayed-jobs.controller.spec.ts index d519acd..7d2416c 100644 --- a/apps/job-service/src/jobs/delayed-jobs.controller.spec.ts +++ b/apps/job-service/src/jobs/delayed-jobs.controller.spec.ts @@ -1,10 +1,9 @@ -import { DelayedJobsController } from './delayed-jobs.controller'; import { Test, TestingModule } from '@nestjs/testing'; -import { Logger, NotFoundException } from '@nestjs/common'; -import { DelayedJobFactory } from '@terramatch-microservices/database/factories'; -import { Resource } from '@terramatch-microservices/common/util'; +import { DelayedJobsController } from './delayed-jobs.controller'; import { DelayedJob } from '@terramatch-microservices/database/entities'; -import { JobBulkUpdateBodyDto } from './dto/delayed-job-update.dto'; +import { DelayedJobBulkUpdateBodyDto } from './dto/delayed-job-update.dto'; +import { v4 as uuidv4 } from 'uuid'; +import { BadRequestException, NotFoundException, UnauthorizedException } from '@nestjs/common'; describe('DelayedJobsController', () => { let controller: DelayedJobsController; @@ -14,6 +13,7 @@ describe('DelayedJobsController', () => { where: {}, truncate: true }); + const module: TestingModule = await Test.createTestingModule({ controllers: [DelayedJobsController] }).compile(); @@ -25,157 +25,136 @@ describe('DelayedJobsController', () => { jest.restoreAllMocks(); }); - it('should throw not found if the delayed job does not exist', async () => { - await expect(controller.findOne('asdf')).rejects - .toThrow(NotFoundException); - }); - - it('should return the job definition when the delayed job does exist', async () => { - const { uuid, statusCode, payload, totalContent, processedContent, progressMessage } = await DelayedJobFactory.create(); - const result = await controller.findOne(uuid); - const resource = result.data as Resource; - expect(resource.type).toBe('delayedJobs'); - expect(resource.id).toBe(uuid); - expect(resource.attributes.statusCode).toBe(statusCode); - expect(resource.attributes.payload).toMatchObject(payload); - expect(resource.attributes.totalContent).toBe(totalContent); - expect(resource.attributes.processedContent).toBe(processedContent); - expect(resource.attributes.progressMessage).toBe(progressMessage); - }); describe('bulkClearJobs', () => { - it('should clear non-pending jobs in bulk for the authenticated user and return the updated jobs', async () => { - const job1 = await DelayedJobFactory.create({ - createdBy: 130999, + it('should successfully clear multiple jobs', async () => { + const authenticatedUserId = 130999; + + const job1 = await DelayedJob.create({ + uuid: uuidv4(), + createdBy: authenticatedUserId, isAcknowledged: false, status: 'completed' }); - const job2 = await DelayedJobFactory.create({ - createdBy: 130999, + const job2 = await DelayedJob.create({ + uuid: uuidv4(), + createdBy: authenticatedUserId, isAcknowledged: false, status: 'failed' }); - const job3 = await DelayedJobFactory.create({ - createdBy: 130999, - isAcknowledged: false, - status: 'pending' - }); - - const request = { authenticatedUserId: 130999 }; - const payload: JobBulkUpdateBodyDto = { + + const payload: DelayedJobBulkUpdateBodyDto = { data: [ { - type: 'jobs', + type: 'delayedJobs', uuid: job1.uuid, - attributes: { - isAcknowledged: true, - }, + attributes: { isAcknowledged: true } }, { - type: 'jobs', + type: 'delayedJobs', uuid: job2.uuid, - attributes: { - isAcknowledged: true, - }, - }, - ], + attributes: { isAcknowledged: true } + } + ] }; - + + const request = { authenticatedUserId }; + const result = await controller.bulkClearJobs(payload, request); - + expect(result.data).toHaveLength(2); - expect(result.data[0]).toMatchObject({ - type: 'jobs', - uuid: job1.uuid, - attributes: { - isAcknowledged: true, - }, - }); - expect(result.data[1]).toMatchObject({ - type: 'jobs', - uuid: job2.uuid, - attributes: { - isAcknowledged: true, - }, - }); + expect(result.data[0].id).toBe(job1.uuid); + expect(result.data[1].id).toBe(job2.uuid); + + const updatedJob1 = await DelayedJob.findOne({ where: { uuid: job1.uuid } }); + const updatedJob2 = await DelayedJob.findOne({ where: { uuid: job2.uuid } }); + expect(updatedJob1.isAcknowledged).toBe(true); + expect(updatedJob2.isAcknowledged).toBe(true); }); - - it('should return an empty array when no jobs can be cleared in bulk', async () => { - const job = await DelayedJobFactory.create({ - createdBy: 130999, - isAcknowledged: false, - status: 'pending' - }); - + + it('should throw BadRequestException when no jobs are provided', async () => { + const payload: DelayedJobBulkUpdateBodyDto = { data: [] }; const request = { authenticatedUserId: 130999 }; - const payload: JobBulkUpdateBodyDto = { + + await expect(controller.bulkClearJobs(payload, request)) + .rejects.toThrow(BadRequestException); + }); + + it('should throw NotFoundException for non-existent job', async () => { + const payload: DelayedJobBulkUpdateBodyDto = { data: [ { - type: 'jobs', - uuid: job.uuid, - attributes: { - isAcknowledged: true, - }, - }, - ], + type: 'delayedJobs', + uuid: 'non-existent-uuid', + attributes: { isAcknowledged: true } + } + ] }; - - const result = await controller.bulkClearJobs(payload, request); - - expect(result.data).toHaveLength(0); - }); - }); - + const request = { authenticatedUserId: 130999 }; - describe('findOne', () => { - it('should handle non-existent job uuid', async () => { - await expect(controller.findOne('non-existent-uuid')).rejects.toThrow(NotFoundException); + await expect(controller.bulkClearJobs(payload, request)) + .rejects.toThrow(NotFoundException); }); - it('should handle null or undefined uuid', async () => { - await expect(controller.findOne(null)).rejects.toThrow(); - await expect(controller.findOne(undefined)).rejects.toThrow(); - }); - }); + it('should not update jobs created by other users', async () => { + const authenticatedUserId = 130999; + const otherUserId = 999999; + const otherUserJob = await DelayedJob.create({ + uuid: uuidv4(), + createdBy: otherUserId, + isAcknowledged: false, + status: 'completed' + }); - describe('findOne', () => { - it('should handle non-existent job uuid', async () => { - await expect(controller.findOne('non-existent-uuid')).rejects.toThrow(NotFoundException); + const payload: DelayedJobBulkUpdateBodyDto = { + data: [ + { + type: 'delayedJobs', + uuid: otherUserJob.uuid, + attributes: { isAcknowledged: true } + } + ] + }; + const request = { authenticatedUserId }; + + await expect(controller.bulkClearJobs(payload, request)) + .rejects.toThrow(NotFoundException); }); - it('should handle null or undefined uuid', async () => { - await expect(controller.findOne(null)).rejects.toThrow(); - await expect(controller.findOne(undefined)).rejects.toThrow(); + it('should throw UnauthorizedException for missing authenticated id', async () => { + const payload: DelayedJobBulkUpdateBodyDto = { + data: [ + { type: 'delayedJobs', uuid: uuidv4(), attributes: { isAcknowledged: true } } + ] + }; + + await expect(controller.bulkClearJobs(payload, { authenticatedUserId: null })) + .rejects.toThrow(UnauthorizedException); }); - }); - describe('getRunningJobs', () => { - it('should handle jobs with different statuses', async () => { + it('should not update jobs with status "pending"', async () => { const authenticatedUserId = 130999; - const request = { authenticatedUserId }; - // Create jobs with different statuses but same user - await DelayedJobFactory.create({ - createdBy: authenticatedUserId, - isAcknowledged: false, - status: 'completed' - }); - await DelayedJobFactory.create({ + const pendingJob = await DelayedJob.create({ + uuid: uuidv4(), createdBy: authenticatedUserId, isAcknowledged: false, status: 'pending' }); - await DelayedJobFactory.create({ - createdBy: authenticatedUserId, - isAcknowledged: true, - status: 'failed' - }); - const result = await controller.getRunningJobs(request); - const resources = result.data as Resource[]; + const payload: DelayedJobBulkUpdateBodyDto = { + data: [ + { + type: 'delayedJobs', + uuid: pendingJob.uuid, + attributes: { isAcknowledged: true } + } + ] + }; + const request = { authenticatedUserId }; - // Should only return non-cleared jobs - expect(resources).toHaveLength(2); + await expect(controller.bulkClearJobs(payload, request)) + .rejects.toThrow(NotFoundException); }); }); }); diff --git a/apps/job-service/src/jobs/delayed-jobs.controller.ts b/apps/job-service/src/jobs/delayed-jobs.controller.ts index 1c77e64..719bf87 100644 --- a/apps/job-service/src/jobs/delayed-jobs.controller.ts +++ b/apps/job-service/src/jobs/delayed-jobs.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, NotFoundException, Param, UnauthorizedException, Request, Patch, BadRequestException, Body } from '@nestjs/common'; +import { Controller, Get, NotFoundException, Param, UnauthorizedException, Request, Patch, BadRequestException, Body, Logger } from '@nestjs/common'; import { ApiException } from '@nanogiants/nestjs-swagger-api-exception-decorator'; import { ApiBody, ApiOperation } from '@nestjs/swagger'; import { Op } from 'sequelize'; @@ -9,7 +9,7 @@ import { } from '@terramatch-microservices/common/util'; import { DelayedJobDto } from './dto/delayed-job.dto'; import { DelayedJob } from '@terramatch-microservices/database/entities'; -import { JobBulkUpdateBodyDto, JobData } from './dto/delayed-job-update.dto'; +import { DelayedJobBulkUpdateBodyDto } from './dto/delayed-job-update.dto'; @Controller('jobs/v3/delayedJobs') export class DelayedJobsController { @@ -73,7 +73,7 @@ export class DelayedJobsController { }) @ApiBody({ description: 'JSON:API bulk update payload for jobs', - type: JobBulkUpdateBodyDto, + type: DelayedJobBulkUpdateBodyDto, examples: { example: { value: { @@ -97,19 +97,23 @@ export class DelayedJobsController { }, }, }) + @JsonApiResponse({ data: { type: DelayedJobDto } }) @ApiException(() => UnauthorizedException, { description: 'Authentication failed.' }) @ApiException(() => BadRequestException, { description: 'Invalid payload or IDs provided.' }) @ApiException(() => NotFoundException, { description: 'One or more jobs specified in the payload could not be found.' }) async bulkClearJobs( - @Body() bulkClearJobsDto: JobBulkUpdateBodyDto, + @Body() bulkClearJobsDto: DelayedJobBulkUpdateBodyDto, @Request() { authenticatedUserId } - ): Promise<{ data: JobData[] }> { + ): Promise { const jobUpdates = bulkClearJobsDto.data; - + if (!jobUpdates || jobUpdates.length === 0) { throw new BadRequestException('No jobs provided in the payload.'); } - + + if (!authenticatedUserId) { + throw new UnauthorizedException('Authentication failed.'); + } const updatePromises = jobUpdates.map(async (job) => { const [updatedCount] = await DelayedJob.update( { isAcknowledged: job.attributes.isAcknowledged }, @@ -119,21 +123,28 @@ export class DelayedJobsController { createdBy: authenticatedUserId, status: { [Op.ne]: 'pending' }, }, - } - ); - + }); + if (updatedCount === 0) { throw new NotFoundException(`Job with UUID ${job.uuid} could not be updated.`); } + + const updatedJob = await DelayedJob.findOne({ + where: { uuid: job.uuid }, + }); - return job; + return updatedJob; }); - + const updatedJobs = await Promise.all(updatePromises); - - return { - data: updatedJobs, - }; + + + const jsonApiBuilder = buildJsonApi(); + updatedJobs.forEach((job) => { + jsonApiBuilder.addData(job.uuid, new DelayedJobDto(job)); + }); + + return jsonApiBuilder.serialize(); + } - } \ No newline at end of file diff --git a/apps/job-service/src/jobs/dto/delayed-job-update.dto.ts b/apps/job-service/src/jobs/dto/delayed-job-update.dto.ts index 0745021..87e951f 100644 --- a/apps/job-service/src/jobs/dto/delayed-job-update.dto.ts +++ b/apps/job-service/src/jobs/dto/delayed-job-update.dto.ts @@ -2,30 +2,30 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsArray, IsBoolean, IsUUID, ValidateNested } from 'class-validator'; import { Type } from 'class-transformer'; -class JobAttributes { +class DelayedJobAttributes { @IsBoolean() @ApiProperty({ description: 'Value to set for isAcknowledged', example: true }) isAcknowledged: boolean; } -export class JobData { - @ApiProperty({ enum: ['jobs'], description: 'Type of the resource', example: 'jobs' }) - type: 'jobs'; +export class DelayedJobData { + @ApiProperty({ enum: ['delayedJobs'], description: 'Type of the resource', example: 'delayedJobs' }) + type: 'delayedJobs'; @IsUUID() @ApiProperty({ format: 'uuid', description: 'UUID of the job', example: 'uuid-1' }) uuid: string; @ValidateNested() - @Type(() => JobAttributes) - @ApiProperty({ description: 'Attributes to update for the job', type: JobAttributes }) - attributes: JobAttributes; + @Type(() => DelayedJobAttributes) + @ApiProperty({ description: 'Attributes to update for the job', type: DelayedJobAttributes }) + attributes: DelayedJobAttributes; } -export class JobBulkUpdateBodyDto { +export class DelayedJobBulkUpdateBodyDto { @IsArray() @ValidateNested({ each: true }) - @Type(() => JobData) - @ApiProperty({ description: 'List of jobs to update', type: [JobData] }) - data: JobData[]; + @Type(() => DelayedJobData) + @ApiProperty({ description: 'List of jobs to update isAcknowledged', type: [DelayedJobData] }) + data: DelayedJobData[]; } diff --git a/libs/database/src/lib/entities/delayed-job.entity.ts b/libs/database/src/lib/entities/delayed-job.entity.ts index 86cc173..a062596 100644 --- a/libs/database/src/lib/entities/delayed-job.entity.ts +++ b/libs/database/src/lib/entities/delayed-job.entity.ts @@ -1,5 +1,6 @@ -import { AllowNull, AutoIncrement, Column, Default, Index, Model, PrimaryKey, Table } from "sequelize-typescript"; +import { AllowNull, AutoIncrement, Column, Default, ForeignKey, Index, Model, PrimaryKey, Table } from "sequelize-typescript"; import { BIGINT, BOOLEAN, INTEGER, JSON, STRING, UUID } from "sequelize"; +import { User } from "./user.entity"; @Table({ tableName: "delayed_jobs", underscored: true }) export class DelayedJob extends Model { @@ -36,8 +37,9 @@ export class DelayedJob extends Model { @Column(STRING) progressMessage: string | null + @ForeignKey(() => User) @AllowNull - @Column(BIGINT) + @Column(BIGINT.UNSIGNED) createdBy: number | null; @Column(BOOLEAN)