diff --git a/src/attachments/attachments.v4.controller.spec.ts b/src/attachments/attachments.v4.controller.spec.ts new file mode 100644 index 000000000..52b685607 --- /dev/null +++ b/src/attachments/attachments.v4.controller.spec.ts @@ -0,0 +1,112 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { AttachmentsV4Controller } from "./attachments.v4.controller"; +import { AttachmentsV4Service } from "./attachments.v4.service"; +import { HttpException, HttpStatus } from "@nestjs/common"; +import { PartialUpdateAttachmentV4Dto } from "./dto/update-attachment.v4.dto"; +import { Attachment } from "./schemas/attachment.schema"; +import * as jmp from "json-merge-patch"; +import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; +import { PoliciesGuard } from "src/casl/guards/policies.guard"; + +describe("AttachmentsController - findOneAndUpdate", () => { + let controller: AttachmentsV4Controller; + let service: AttachmentsV4Service; + + const mockAttachment: Attachment = { + _id: "123", + name: "Test Attachment", + description: "Initial", + updatedAt: new Date("2025-09-01T10:00:00Z"), + // other fields... + }; + + const mockUpdatedAttachment = { + ...mockAttachment, + description: "Updated", + }; + + const mockCaslAbilityFactory = { + createForUser: jest.fn().mockReturnValue({ + can: jest.fn().mockReturnValue(true), // or false depending on test + }), + }; + + const mockAttachmentsV4Service = { + findOneAndUpdate: jest.fn(), + }; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [AttachmentsV4Controller], + providers: [ + { + provide: AttachmentsV4Service, + useValue: mockAttachmentsV4Service, + useValue: { + findOneAndUpdate: jest + .fn() + .mockResolvedValue(mockUpdatedAttachment), + }, + }, + { + provide: CaslAbilityFactory, + useValue: mockCaslAbilityFactory, + }, + PoliciesGuard, + ], + }).compile(); + + controller = module.get(AttachmentsV4Controller); + service = module.get(AttachmentsV4Service); + + // Mock permission check + jest + .spyOn(controller, "checkPermissionsForAttachment") + .mockResolvedValue(mockAttachment); + }); + + it("should update attachment with application/json", async () => { + const dto: PartialUpdateAttachmentV4Dto = { description: "Updated" }; + const headers = { "content-type": "application/json" }; + + const result = await controller.findOneAndUpdate( + { headers }, + "123", + headers, + dto, + ); + + expect(result).toEqual(mockUpdatedAttachment); + expect(service.findOneAndUpdate).toHaveBeenCalledWith({ _id: "123" }, dto); + }); + + it("should update attachment with application/merge-patch+json", async () => { + const dto = { description: null }; + const headers = { "content-type": "application/merge-patch+json" }; + + await controller.findOneAndUpdate({ headers }, "123", headers, dto); + + const expectedPatched = jmp.apply(mockAttachment, dto); + expect(service.findOneAndUpdate).toHaveBeenCalledWith( + { _id: "123" }, + expectedPatched, + ); + }); + + it("should throw PRECONDITION_FAILED if If-Unmodified-Since is older than updatedAt", async () => { + const dto = { name: "Should Fail" }; + const headers = { + "content-type": "application/json", + "if-unmodified-since": "2000-01-01T00:00:00Z", + }; + + await expect( + controller.findOneAndUpdate({ headers }, "123", headers, dto), + ).rejects.toThrow( + new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ), + ); + }); +}); diff --git a/src/attachments/attachments.v4.controller.ts b/src/attachments/attachments.v4.controller.ts index c06b76ea2..5c077d86b 100644 --- a/src/attachments/attachments.v4.controller.ts +++ b/src/attachments/attachments.v4.controller.ts @@ -15,6 +15,8 @@ import { Patch, Put, HttpCode, + Headers, + HttpException, } from "@nestjs/common"; import { ApiBearerAuth, @@ -370,21 +372,36 @@ Set \`content-type\` header to \`application/merge-patch+json\` if you would lik async findOneAndUpdate( @Req() request: Request, @Param("aid") aid: string, + @Headers() headers: Record, @Body() updateAttachmentDto: PartialUpdateAttachmentV4Dto, ): Promise { - const foundAattachment = await this.checkPermissionsForAttachment( + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; + + const foundAttachment = await this.checkPermissionsForAttachment( request, aid, Action.AttachmentUpdateEndpoint, ); const updateAttachmentDtoForservice = request.headers["content-type"] === "application/merge-patch+json" - ? jmp.apply(foundAattachment, updateAttachmentDto) + ? jmp.apply(foundAttachment, updateAttachmentDto) : updateAttachmentDto; - return this.attachmentsService.findOneAndUpdate( - { _id: aid }, - updateAttachmentDtoForservice, - ); + + if (headerDate && headerDate <= foundAttachment.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } else { + return this.attachmentsService.findOneAndUpdate( + { _id: aid }, + updateAttachmentDtoForservice, + ); + } } // PUT /attachments/:aid diff --git a/src/datasets/datasets.controller.spec.ts b/src/datasets/datasets.controller.spec.ts index d605f06a8..56f573c6b 100644 --- a/src/datasets/datasets.controller.spec.ts +++ b/src/datasets/datasets.controller.spec.ts @@ -8,23 +8,37 @@ import { HistoryService } from "src/history/history.service"; import { LogbooksService } from "src/logbooks/logbooks.service"; import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { ConfigModule } from "@nestjs/config"; +import { + ForbiddenException, + HttpException, + NotFoundException, +} from "@nestjs/common"; +import { DatasetType } from "./types/dataset-type.enum"; +import { Request } from "express"; class AttachmentsServiceMock {} class DatablocksServiceMock {} -class DatasetsServiceMock {} - class OrigDatablocksServiceMock {} class LogbooksServiceMock {} -class CaslAbilityFactoryMock {} +class DatasetsServiceMock { + findOne = jest.fn(); + findByIdAndUpdate = jest.fn(); +} + +class CaslAbilityFactoryMock { + datasetInstanceAccess = jest.fn(); +} class HistoryServiceMock {} describe("DatasetsController", () => { let controller: DatasetsController; + let datasetsService: DatasetsServiceMock; + let caslAbilityFactory: CaslAbilityFactoryMock; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -42,9 +56,137 @@ describe("DatasetsController", () => { }).compile(); controller = module.get(DatasetsController); + datasetsService = module.get(DatasetsService); + caslAbilityFactory = module.get(CaslAbilityFactory); }); it("should be defined", () => { expect(controller).toBeDefined(); }); + + describe("findByIdAndUpdate", () => { + it("should throw NotFoundException if dataset not found", async () => { + datasetsService.findOne.mockResolvedValue(null); + + await expect( + controller.findByIdAndUpdate( + { user: {} } as Request, + "some-pid", + { "if-unmodified-since": "2023-01-01T00:00:00Z" }, + {}, + ), + ).rejects.toThrow(NotFoundException); + }); + + it("should throw PreconditionFailed if header date <= updatedAt", async () => { + const mockDataset = { + pid: "some-pid", + updatedAt: new Date("2023-01-02T00:00:00Z"), + type: DatasetType.Raw, + }; + datasetsService.findOne.mockResolvedValue(mockDataset); + + await expect( + controller.findByIdAndUpdate( + { user: {} } as Request, + "some-pid", + { "if-unmodified-since": "2023-01-01T00:00:00Z" }, + {}, + ), + ).rejects.toThrow(HttpException); + }); + + it("should throw ForbiddenException if user cannot update", async () => { + const mockDataset = { + pid: "some-pid", + updatedAt: new Date("2023-01-01T00:00:00Z"), + type: DatasetType.Raw, + }; + datasetsService.findOne.mockResolvedValue(mockDataset); + jest.spyOn(controller, "validateDatasetObsolete").mockResolvedValue({}); + jest + .spyOn(controller, "generateDatasetInstanceForPermissions") + .mockResolvedValue({}); + caslAbilityFactory.datasetInstanceAccess.mockReturnValue({ + can: () => false, + }); + + await expect( + controller.findByIdAndUpdate( + { user: {} } as Request, + "some-pid", + { "if-unmodified-since": "2023-01-02T00:00:00Z" }, + {}, + ), + ).rejects.toThrow(ForbiddenException); + }); + + it("should return updated dataset if all conditions pass", async () => { + const mockDataset = { + pid: "some-pid", + updatedAt: new Date("2023-01-01T00:00:00Z"), + type: DatasetType.Raw, + }; + const updatedDataset = { pid: "some-pid", name: "Updated" }; + + datasetsService.findOne.mockResolvedValue(mockDataset); + jest.spyOn(controller, "validateDatasetObsolete").mockResolvedValue({}); + jest + .spyOn(controller, "generateDatasetInstanceForPermissions") + .mockResolvedValue({}); + caslAbilityFactory.datasetInstanceAccess.mockReturnValue({ + can: () => true, + }); + jest + .spyOn(controller, "convertObsoleteToCurrentSchema") + .mockReturnValue({}); + datasetsService.findByIdAndUpdate.mockResolvedValue(updatedDataset); + jest + .spyOn(controller, "convertCurrentToObsoleteSchema") + .mockReturnValue(updatedDataset); + + const result = await controller.findByIdAndUpdate( + { user: {} } as Request, + "some-pid", + { "if-unmodified-since": "2023-01-02T00:00:00Z" }, + {}, + ); + + expect(result).toEqual(updatedDataset); + }); + + it("should proceed with update if if-unmodified-since header is missing or invalid", async () => { + const mockDataset = { + pid: "some-pid", + updatedAt: new Date("2023-01-01T00:00:00Z"), + type: DatasetType.Raw, + }; + const updatedDataset = { pid: "some-pid", name: "Updated" }; + + datasetsService.findOne.mockResolvedValue(mockDataset); + jest.spyOn(controller, "validateDatasetObsolete").mockResolvedValue({}); + jest + .spyOn(controller, "generateDatasetInstanceForPermissions") + .mockResolvedValue({}); + caslAbilityFactory.datasetInstanceAccess.mockReturnValue({ + can: () => true, + }); + jest + .spyOn(controller, "convertObsoleteToCurrentSchema") + .mockReturnValue({}); + datasetsService.findByIdAndUpdate.mockResolvedValue(updatedDataset); + jest + .spyOn(controller, "convertCurrentToObsoleteSchema") + .mockReturnValue(updatedDataset); + + const result = await controller.findByIdAndUpdate( + { user: {} } as Request, + "some-pid", + { "if-unmodified-since": "not-a-valid-date" }, // invalid header + {}, + ); + + expect(result).toEqual(updatedDataset); + }); + }); }); diff --git a/src/datasets/datasets.controller.ts b/src/datasets/datasets.controller.ts index 8bd7945e5..051413633 100644 --- a/src/datasets/datasets.controller.ts +++ b/src/datasets/datasets.controller.ts @@ -1414,11 +1414,19 @@ export class DatasetsController { @Req() request: Request, @Param("pid") pid: string, @Body() + @Headers() + headers: Record, updateDatasetObsoleteDto: | PartialUpdateRawDatasetObsoleteDto | PartialUpdateDerivedDatasetObsoleteDto | PartialUpdateDatasetDto, ): Promise { + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; + const foundDataset = await this.datasetsService.findOne({ where: { pid }, }); @@ -1427,52 +1435,59 @@ export class DatasetsController { throw new NotFoundException(); } - // NOTE: Default validation pipe does not validate union types. So we need custom validation. - let dtoType; - switch (foundDataset.type) { - case DatasetType.Raw: - dtoType = PartialUpdateRawDatasetObsoleteDto; - break; - case DatasetType.Derived: - dtoType = PartialUpdateDerivedDatasetObsoleteDto; - break; - default: - dtoType = PartialUpdateDatasetDto; - break; - } - const validatedUpdateDatasetObsoleteDto = - (await this.validateDatasetObsolete( - updateDatasetObsoleteDto, - dtoType, - )) as - | PartialUpdateRawDatasetObsoleteDto - | PartialUpdateDerivedDatasetObsoleteDto - | PartialUpdateDatasetDto; - - // NOTE: We need DatasetClass instance because casl module can not recognize the type from dataset mongo database model. If other fields are needed can be added later. - const datasetInstance = - await this.generateDatasetInstanceForPermissions(foundDataset); + if (headerDate && headerDate <= foundDataset.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } else { + // NOTE: Default validation pipe does not validate union types. So we need custom validation. + let dtoType; + switch (foundDataset.type) { + case DatasetType.Raw: + dtoType = PartialUpdateRawDatasetObsoleteDto; + break; + case DatasetType.Derived: + dtoType = PartialUpdateDerivedDatasetObsoleteDto; + break; + default: + dtoType = PartialUpdateDatasetDto; + break; + } + const validatedUpdateDatasetObsoleteDto = + (await this.validateDatasetObsolete( + updateDatasetObsoleteDto, + dtoType, + )) as + | PartialUpdateRawDatasetObsoleteDto + | PartialUpdateDerivedDatasetObsoleteDto + | PartialUpdateDatasetDto; + + // NOTE: We need DatasetClass instance because casl module can not recognize the type from dataset mongo database model. If other fields are needed can be added later. + const datasetInstance = + await this.generateDatasetInstanceForPermissions(foundDataset); + + // instantiate the casl matrix for the user + const user: JWTUser = request.user as JWTUser; + const ability = this.caslAbilityFactory.datasetInstanceAccess(user); + // check if he/she can create this dataset + const canUpdate = + ability.can(Action.DatasetUpdateAny, DatasetClass) || + ability.can(Action.DatasetUpdateOwner, datasetInstance); + + if (!canUpdate) { + throw new ForbiddenException("Unauthorized to update this dataset"); + } - // instantiate the casl matrix for the user - const user: JWTUser = request.user as JWTUser; - const ability = this.caslAbilityFactory.datasetInstanceAccess(user); - // check if he/she can create this dataset - const canUpdate = - ability.can(Action.DatasetUpdateAny, DatasetClass) || - ability.can(Action.DatasetUpdateOwner, datasetInstance); + const updateDatasetDto = this.convertObsoleteToCurrentSchema( + validatedUpdateDatasetObsoleteDto, + ) as UpdateDatasetDto; - if (!canUpdate) { - throw new ForbiddenException("Unauthorized to update this dataset"); + const res = this.convertCurrentToObsoleteSchema( + await this.datasetsService.findByIdAndUpdate(pid, updateDatasetDto), + ); + return res; } - - const updateDatasetDto = this.convertObsoleteToCurrentSchema( - validatedUpdateDatasetObsoleteDto, - ) as UpdateDatasetDto; - - const res = await this.convertCurrentToObsoleteSchema( - await this.datasetsService.findByIdAndUpdate(pid, updateDatasetDto), - ); - return res; } // PUT /datasets/:id diff --git a/src/datasets/datasets.v4.controller.spec.ts b/src/datasets/datasets.v4.controller.spec.ts index 612016461..d3bbc795c 100644 --- a/src/datasets/datasets.v4.controller.spec.ts +++ b/src/datasets/datasets.v4.controller.spec.ts @@ -5,6 +5,8 @@ import { ConfigModule } from "@nestjs/config"; import { DatasetsV4Controller } from "./datasets.v4.controller"; import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { HttpModule } from "@nestjs/axios"; +import { PartialUpdateDatasetDto } from "./dto/update-dataset.dto"; +import { Request } from "express"; class DatasetsServiceMock {} @@ -33,3 +35,143 @@ describe("DatasetsController", () => { expect(controller).toBeDefined(); }); }); + +describe("DatasetsController (manual instantiate)", () => { + let controller: DatasetsV4Controller; + + // ---- Mocks ---- + + const mockUser = { id: "u-1", roles: ["admin"] }; + + const datasetsService = { + findOne: jest.fn().mockResolvedValue({ + pid: "test-dataset-id", + updatedAt: new Date("2025-01-01T00:00:00Z"), + scientificMetadata: { + temperature: { value: 295, unit: "K" }, + }, + }), + findByIdAndUpdate: jest + .fn() + .mockImplementation((pid: string, updateDto) => ({ + pid, + ...updateDto, + })), + } as unknown as jest.Mocked; + + // CASL factory mock (kept for completeness; permission check is stubbed anyway) + const caslAbilityFactory = { + createForUser: jest.fn().mockReturnValue({ + can: () => true, + cannot: () => false, + }), + datasetInstanceAccess: jest.fn().mockReturnValue(true), + } as unknown as jest.Mocked; + + class LogbooksServiceMock {} + + let checkSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + + controller = new DatasetsV4Controller( + datasetsService, + new LogbooksServiceMock(), + caslAbilityFactory, + ); + + // Force the permission check to always pass in these unit tests + checkSpy = jest + .spyOn(controller, "checkPermissionsForDatasetExtended") + .mockResolvedValue(undefined); + }); + + it("should update dataset and return updated result (happy path with fresh header)", async () => { + const pid = "test-dataset-id"; + const updateDto: PartialUpdateDatasetDto = { + scientificMetadata: { temperature: { value: 300, unit: "K" } }, + }; + + const req = { + headers: { "content-type": "application/json" }, + user: mockUser, + } as unknown as Request; + + // Header is *after* updatedAt -> allowed + const headers = { "if-unmodified-since": "2026-01-01T00:00:00Z" }; + + const result = await controller.findByIdAndUpdate( + req, + pid, + headers, + updateDto, + ); + + expect(result).toBeDefined(); + expect(result.pid).toBe(pid); + expect(result.scientificMetadata.temperature.value).toBe(300); + + expect(datasetsService.findOne).toHaveBeenCalledWith({ where: { pid } }); + expect(datasetsService.findByIdAndUpdate).toHaveBeenCalledTimes(1); + expect(checkSpy).toHaveBeenCalledTimes(1); + }); + + it.each([ + { title: "missing header", header: {} as Record }, + { + title: "invalid header", + header: { "if-unmodified-since": "not-a-date" } as Record, + }, + ])("should update when if-unmodified-since is $title", async ({ header }) => { + const pid = "test-dataset-id"; + const updateDto: PartialUpdateDatasetDto = { + scientificMetadata: { temperature: { value: 305, unit: "K" } }, + }; + + const req = { + headers: { "content-type": "application/json" }, + user: mockUser, + } as unknown as Request; + + const result = await controller.findByIdAndUpdate( + req, + pid, + header, + updateDto, + ); + + expect(result).toBeDefined(); + expect(result.pid).toBe(pid); + expect(result.scientificMetadata.temperature.value).toBe(305); + + expect(datasetsService.findOne).toHaveBeenCalledWith({ where: { pid } }); + expect(datasetsService.findByIdAndUpdate).toHaveBeenCalledTimes(1); + expect(checkSpy).toHaveBeenCalledTimes(1); + }); + + it("should NOT update and throw 412 when if-unmodified-since is older than updatedAt", async () => { + const pid = "test-dataset-id"; + const updateDto: PartialUpdateDatasetDto = { + scientificMetadata: { temperature: { value: 310, unit: "K" } }, + }; + + const req = { + headers: { "content-type": "application/json" }, + user: mockUser, + } as unknown as Request; + + // Header is *before* updatedAt -> should fail with PRECONDITION_FAILED (412) + const headers = { "if-unmodified-since": "2024-12-31T12:00:00Z" }; + + const promise = controller.findByIdAndUpdate(req, pid, headers, updateDto); + + await expect(promise).rejects.toThrow( + "Update error due to failed if-modified-since condition", + ); + + expect(datasetsService.findOne).toHaveBeenCalledWith({ where: { pid } }); + expect(datasetsService.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(checkSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/datasets/datasets.v4.controller.ts b/src/datasets/datasets.v4.controller.ts index 00f6ba3fc..2cee3e426 100644 --- a/src/datasets/datasets.v4.controller.ts +++ b/src/datasets/datasets.v4.controller.ts @@ -1,24 +1,26 @@ import { - BadRequestException, Body, - ConflictException, Controller, Delete, - ForbiddenException, Get, - HttpCode, - HttpStatus, - InternalServerErrorException, - NotFoundException, Param, Patch, Post, Put, Query, - Req, UseGuards, UseInterceptors, + HttpCode, + HttpStatus, + NotFoundException, + Req, + BadRequestException, + ForbiddenException, + InternalServerErrorException, + ConflictException, UsePipes, + Headers, + HttpException, } from "@nestjs/common"; import { ApiBearerAuth, @@ -784,9 +786,31 @@ Set \`content-type\` header to \`application/merge-patch+json\` if you would lik async findByIdAndUpdate( @Req() request: Request, @Param("pid") pid: string, + @Headers() headers: Record, @Body() updateDatasetDto: PartialUpdateDatasetDto, ): Promise { + return this.findByIdAndUpdateInternal( + request, + pid, + headers, + updateDatasetDto, + ); + } + + async findByIdAndUpdateInternal( + @Req() request: Request, + @Param("pid") pid: string, + @Headers() headers: Record, + @Body() + updateDatasetDto: PartialUpdateDatasetDto, + ): Promise { + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; + const foundDataset = await this.datasetsService.findOne({ where: { pid }, }); @@ -813,15 +837,22 @@ Set \`content-type\` header to \`application/merge-patch+json\` if you would lik ); } - const updateDatasetDtoForService = - request.headers["content-type"] === "application/merge-patch+json" - ? jmp.apply(foundDataset, updateDatasetDto) - : updateDatasetDto; - const updatedDataset = await this.datasetsService.findByIdAndUpdate( - pid, - updateDatasetDtoForService, - ); - return updatedDataset; + if (headerDate && headerDate <= foundDataset.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } else { + const updateDatasetDtoForService = + request.headers["content-type"] === "application/merge-patch+json" + ? jmp.apply(foundDataset, updateDatasetDto) + : updateDatasetDto; + const updatedDataset = await this.datasetsService.findByIdAndUpdate( + pid, + updateDatasetDtoForService, + ); + return updatedDataset; + } } // GET /datasets/:id/datasetlifecycle diff --git a/src/instruments/instruments.controller.spec.ts b/src/instruments/instruments.controller.spec.ts index 0d19d48c4..ee84b7df2 100644 --- a/src/instruments/instruments.controller.spec.ts +++ b/src/instruments/instruments.controller.spec.ts @@ -3,13 +3,30 @@ import { Test, TestingModule } from "@nestjs/testing"; import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { InstrumentsController } from "./instruments.controller"; import { InstrumentsService } from "./instruments.service"; +import { + NotFoundException, + HttpException, + ConflictException, +} from "@nestjs/common"; +import { MongoError } from "mongodb"; -class InstrumentsServiceMock {} +class InstrumentsServiceMock { + findOne = jest.fn(); + update = jest.fn(); +} class CaslAbilityFactoryMock {} describe("InstrumentsController", () => { let controller: InstrumentsController; + let service: InstrumentsServiceMock; + + const mockInstrument = { + _id: "123", + updatedAt: new Date("2025-09-01T10:00:00Z"), + }; + + const mockUpdateDto = { name: "Updated Instrument" }; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -22,9 +39,59 @@ describe("InstrumentsController", () => { }).compile(); controller = module.get(InstrumentsController); + service = module.get(InstrumentsService); }); it("should be defined", () => { expect(controller).toBeDefined(); }); + + it("should throw NotFoundException if instrument not found", async () => { + service.findOne.mockResolvedValue(null); + + await expect(controller.update("123", mockUpdateDto, {})).rejects.toThrow( + NotFoundException, + ); + }); + + it("should update if header is missing", async () => { + service.findOne.mockResolvedValue(mockInstrument); + service.update.mockResolvedValue({ ...mockInstrument, ...mockUpdateDto }); + + const result = await controller.update("123", mockUpdateDto, {}); + expect(result).toEqual({ ...mockInstrument, ...mockUpdateDto }); + }); + + it("should throw PRECONDITION_FAILED if header date <= updatedAt", async () => { + service.findOne.mockResolvedValue(mockInstrument); + + const headers = { + "if-unmodified-since": "2025-09-01T09:00:00Z", + }; + + await expect( + controller.update("123", mockUpdateDto, headers), + ).rejects.toThrow(HttpException); + }); + + it("should update if header date > updatedAt", async () => { + service.findOne.mockResolvedValue(mockInstrument); + service.update.mockResolvedValue({ ...mockInstrument, ...mockUpdateDto }); + + const headers = { + "if-unmodified-since": "2025-09-02T10:00:00Z", + }; + + const result = await controller.update("123", mockUpdateDto, headers); + expect(result).toEqual({ ...mockInstrument, ...mockUpdateDto }); + }); + + it("should throw ConflictException on duplicate key error", async () => { + service.findOne.mockResolvedValue(mockInstrument); + service.update.mockRejectedValue({ code: 11000 } as MongoError); + + await expect(controller.update("123", mockUpdateDto, {})).rejects.toThrow( + ConflictException, + ); + }); }); diff --git a/src/instruments/instruments.controller.ts b/src/instruments/instruments.controller.ts index cf98c234e..5a702d2b5 100644 --- a/src/instruments/instruments.controller.ts +++ b/src/instruments/instruments.controller.ts @@ -1,16 +1,20 @@ import { + Body, + ConflictException, Controller, + Delete, Get, - Post, - Body, - Patch, + Headers, + HttpException, + HttpStatus, + InternalServerErrorException, + NotFoundException, Param, - Delete, - UseGuards, + Patch, + Post, Query, + UseGuards, UseInterceptors, - InternalServerErrorException, - ConflictException, } from "@nestjs/common"; import { MongoError } from "mongodb"; import { InstrumentsService } from "./instruments.service"; @@ -157,26 +161,53 @@ export class InstrumentsController { async update( @Param("id") id: string, @Body() updateInstrumentDto: PartialUpdateInstrumentDto, + @Headers() headers: Record, ): Promise { - try { - const instrument = await this.instrumentsService.update( - { _id: id }, - updateInstrumentDto, - ); + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; - return instrument; - } catch (error) { - if ((error as MongoError).code === 11000) { - throw new ConflictException( - "Instrument with the same unique name already exists", - ); - } else { - throw new InternalServerErrorException( - "Something went wrong. Please try again later.", - { cause: error }, - ); - } - } + return this.instrumentsService + .findOne({ where: { _id: id } }) + .then((instrument) => { + if (!instrument) { + throw new NotFoundException("Instrument not found"); + } + + // If header is missing, always update + if (!headerDate) { + return this.instrumentsService.update( + { _id: id }, + updateInstrumentDto, + ); + } + + if (headerDate && headerDate <= instrument.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } else { + return this.instrumentsService.update( + { _id: id }, + updateInstrumentDto, + ); + } + }) + .then((updatedInstrument) => { + return updatedInstrument; + }) + .catch((error) => { + if ((error as MongoError).code === 11000) { + throw new ConflictException( + "Instrument with the same unique name already exists", + ); + } else { + throw error; + } + }); } @UseGuards(PoliciesGuard) diff --git a/src/instruments/schemas/instrument.schema.ts b/src/instruments/schemas/instrument.schema.ts index f7a35d9e2..8cd4e9ec3 100644 --- a/src/instruments/schemas/instrument.schema.ts +++ b/src/instruments/schemas/instrument.schema.ts @@ -2,6 +2,7 @@ import { Prop, Schema, SchemaFactory } from "@nestjs/mongoose"; import { ApiProperty } from "@nestjs/swagger"; import { Document } from "mongoose"; import { v4 as uuidv4 } from "uuid"; +import { QueryableClass } from "../../common/schemas/queryable.schema"; export type InstrumentDocument = Instrument & Document; @@ -13,7 +14,7 @@ export type InstrumentDocument = Instrument & Document; getters: true, }, }) -export class Instrument { +export class Instrument extends QueryableClass { @ApiProperty({ type: String, default: function genUUID(): string { diff --git a/src/origdatablocks/origdatablocks.controller.spec.ts b/src/origdatablocks/origdatablocks.controller.spec.ts index fc3275a67..31abc78a9 100644 --- a/src/origdatablocks/origdatablocks.controller.spec.ts +++ b/src/origdatablocks/origdatablocks.controller.spec.ts @@ -4,8 +4,13 @@ import { OrigDatablocksService } from "src/origdatablocks/origdatablocks.service import { DatasetsService } from "src/datasets/datasets.service"; import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { ConfigModule } from "@nestjs/config"; +import { NotFoundException, HttpException } from "@nestjs/common"; +import { Request } from "express"; -class OrigDatablocksServiceMock {} +class OrigDatablocksServiceMock { + findOne = jest.fn(); + findByIdAndUpdate = jest.fn(); +} class DatasetsServiceMock {} @@ -13,6 +18,7 @@ class CaslAbilityFactoryMock {} describe("OrigDatablocksController", () => { let controller: OrigDatablocksController; + let origDatablocksService: OrigDatablocksServiceMock; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -26,9 +32,128 @@ describe("OrigDatablocksController", () => { }).compile(); controller = module.get(OrigDatablocksController); + origDatablocksService = module.get( + OrigDatablocksService, + ); + + // Mock internal methods + controller["checkPermissionsForOrigDatablock"] = jest.fn(); + controller["updateDatasetSizeAndFiles"] = jest.fn(); }); it("should be defined", () => { expect(controller).toBeDefined(); }); + + describe("update", () => { + const mockRequest = {} as Request; + const mockHeaders = { + "if-unmodified-since": new Date().toUTCString(), + }; + const mockDto = { name: "Updated Name" }; + const mockDatablock = { + _id: "123", + updatedAt: new Date(Date.now() - 1000), + datasetId: "ds1", + }; + + it("should throw NotFoundException if datablock not found before update", async () => { + origDatablocksService.findOne.mockResolvedValue(null); + + await expect( + controller.update(mockRequest, "123", mockHeaders, mockDto), + ).rejects.toThrow(NotFoundException); + }); + + it("should throw PreconditionFailed if header date <= updatedAt", async () => { + origDatablocksService.findOne.mockResolvedValue({ + ...mockDatablock, + updatedAt: new Date(), + }); + + await expect( + controller.update(mockRequest, "123", mockHeaders, mockDto), + ).rejects.toThrow(HttpException); + }); + + it("should throw NotFoundException if datablock not found after update", async () => { + origDatablocksService.findOne.mockResolvedValue(mockDatablock); + origDatablocksService.findByIdAndUpdate.mockResolvedValue(null); + + await expect( + controller.update(mockRequest, "123", mockHeaders, mockDto), + ).rejects.toThrow(NotFoundException); + }); + + it("should return updated datablock on success", async () => { + const updatedDatablock = { ...mockDatablock, name: "Updated Name" }; + + origDatablocksService.findOne.mockResolvedValue(mockDatablock); + origDatablocksService.findByIdAndUpdate.mockResolvedValue( + updatedDatablock, + ); + + const result = await controller.update( + mockRequest, + "123", + mockHeaders, + mockDto, + ); + + expect(result).toEqual(updatedDatablock); + expect(controller["updateDatasetSizeAndFiles"]).toHaveBeenCalledWith( + "ds1", + ); + }); + + describe("update", () => { + const mockRequest = {} as Request; + const mockDto = { name: "Updated Name" }; + const mockDatablock = { + _id: "123", + updatedAt: new Date(), + datasetId: "ds1", + }; + const updatedDatablock = { ...mockDatablock, name: "Updated Name" }; + + beforeEach(() => { + origDatablocksService.findOne.mockResolvedValue(mockDatablock); + origDatablocksService.findByIdAndUpdate.mockResolvedValue( + updatedDatablock, + ); + }); + + it("should proceed with update if 'if-unmodified-since' header is missing", async () => { + const headers = {}; // No header + + const result = await controller.update( + mockRequest, + "123", + headers, + mockDto, + ); + + expect(result).toEqual(updatedDatablock); + expect(controller["updateDatasetSizeAndFiles"]).toHaveBeenCalledWith( + "ds1", + ); + }); + + it("should proceed with update if 'if-unmodified-since' header is malformed", async () => { + const headers = { "if-unmodified-since": "not-a-date" }; // Invalid date format + + const result = await controller.update( + mockRequest, + "123", + headers, + mockDto, + ); + + expect(result).toEqual(updatedDatablock); + expect(controller["updateDatasetSizeAndFiles"]).toHaveBeenCalledWith( + "ds1", + ); + }); + }); + }); }); diff --git a/src/origdatablocks/origdatablocks.controller.ts b/src/origdatablocks/origdatablocks.controller.ts index a8904a1a3..2a3e30668 100644 --- a/src/origdatablocks/origdatablocks.controller.ts +++ b/src/origdatablocks/origdatablocks.controller.ts @@ -13,6 +13,8 @@ import { Req, ForbiddenException, NotFoundException, + Headers, + HttpException, } from "@nestjs/common"; import { Request } from "express"; import { OrigDatablocksService } from "./origdatablocks.service"; @@ -643,6 +645,7 @@ export class OrigDatablocksController { async update( @Req() request: Request, @Param("id") id: string, + @Headers() headers: Record, @Body() updateOrigDatablockDto: PartialUpdateOrigDatablockDto, ): Promise { await this.checkPermissionsForOrigDatablock( @@ -651,14 +654,35 @@ export class OrigDatablocksController { Action.OrigdatablockUpdate, ); - const origdatablock = (await this.origDatablocksService.update( - { _id: id }, - updateOrigDatablockDto, - )) as OrigDatablock; + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; - await this.updateDatasetSizeAndFiles(origdatablock.datasetId); + const datablock = await this.origDatablocksService.findOne({ + where: { _id: id }, + }); + if (!datablock) { + throw new NotFoundException("Datablock not found"); + } - return origdatablock; + if (headerDate && headerDate <= datablock.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } else { + const origdatablock = await this.origDatablocksService.findByIdAndUpdate( + id, + updateOrigDatablockDto, + ); + if (!origdatablock) { + throw new NotFoundException("Datablock not found"); + } + await this.updateDatasetSizeAndFiles(origdatablock.datasetId); + return origdatablock; + } } // DELETE /origdatablocks/:id diff --git a/src/origdatablocks/origdatablocks.v4.controller.spec.ts b/src/origdatablocks/origdatablocks.v4.controller.spec.ts index 294c419ca..2753a42c6 100644 --- a/src/origdatablocks/origdatablocks.v4.controller.spec.ts +++ b/src/origdatablocks/origdatablocks.v4.controller.spec.ts @@ -4,15 +4,24 @@ import { OrigDatablocksService } from "src/origdatablocks/origdatablocks.service import { DatasetsService } from "src/datasets/datasets.service"; import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { ConfigModule } from "@nestjs/config"; +import { NotFoundException, HttpException } from "@nestjs/common"; +import { Request } from "express"; -class OrigDatablocksServiceMock {} +class OrigDatablocksServiceMock { + findOne = jest.fn(); + findByIdAndUpdate = jest.fn(); + findOneComplete = jest.fn(); +} -class DatasetsServiceMock {} +class DatasetsServiceMock { + findOneComplete = jest.fn(); +} class CaslAbilityFactoryMock {} describe("OrigDatablocksV4Controller", () => { let controller: OrigDatablocksV4Controller; + let origDatablocksService: OrigDatablocksServiceMock; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -28,9 +37,144 @@ describe("OrigDatablocksV4Controller", () => { controller = module.get( OrigDatablocksV4Controller, ); + origDatablocksService = module.get(OrigDatablocksService); }); it("should be defined", () => { expect(controller).toBeDefined(); }); + + describe("findByIdAndUpdate", () => { + const mockRequest = { + user: { id: "user123" }, + } as unknown as Request; + + const mockHeaders = { + "if-unmodified-since": new Date().toISOString(), + }; + + const mockUpdateDto = { + name: "Updated Name", + }; + + const mockDatablock = { + _id: "db123", + updatedAt: new Date(Date.now() - 1000), + datasetId: "ds123", + }; + + const updatedDatablock = { + ...mockDatablock, + name: "Updated Name", + }; + + beforeEach(() => { + jest + .spyOn(controller, "checkPermissionsForOrigDatablockWrite") + .mockResolvedValue(mockDatablock); + jest + .spyOn(controller, "updateDatasetSizeAndFiles") + .mockResolvedValue(undefined); + }); + + it("should throw NotFoundException if datablock not found", async () => { + jest.spyOn(origDatablocksService, "findOne").mockResolvedValue(null); + + await expect( + controller.findByIdAndUpdate( + mockRequest, + "db123", + mockHeaders, + mockUpdateDto, + ), + ).rejects.toThrow(NotFoundException); + }); + + it("should throw HttpException if header date is older than updatedAt", async () => { + jest.spyOn(origDatablocksService, "findOne").mockResolvedValue({ + ...mockDatablock, + updatedAt: new Date(), + }); + + const oldDate = new Date(Date.now() - 10000).toISOString(); + const headers = { "if-unmodified-since": oldDate }; + + await expect( + controller.findByIdAndUpdate( + mockRequest, + "db123", + headers, + mockUpdateDto, + ), + ).rejects.toThrow(HttpException); + }); + + it("should throw NotFoundException if update returns null", async () => { + jest + .spyOn(origDatablocksService, "findOne") + .mockResolvedValue(mockDatablock); + jest + .spyOn(origDatablocksService, "findByIdAndUpdate") + .mockResolvedValue(null); + + await expect( + controller.findByIdAndUpdate(mockRequest, "db123", {}, mockUpdateDto), + ).rejects.toThrow(NotFoundException); + }); + + it("should return updated datablock on success", async () => { + jest + .spyOn(origDatablocksService, "findOne") + .mockResolvedValue(mockDatablock); + jest + .spyOn(origDatablocksService, "findByIdAndUpdate") + .mockResolvedValue(updatedDatablock); + + const result = await controller.findByIdAndUpdate( + mockRequest, + "db123", + {}, + mockUpdateDto, + ); + expect(result).toEqual(updatedDatablock); + }); + + it("should succeed if 'if-unmodified-since' header is missing", async () => { + jest + .spyOn(origDatablocksService, "findOne") + .mockResolvedValue(mockDatablock); + jest + .spyOn(origDatablocksService, "findByIdAndUpdate") + .mockResolvedValue(updatedDatablock); + + const result = await controller.findByIdAndUpdate( + mockRequest, + "db123", + {}, + mockUpdateDto, + ); + expect(result).toEqual(updatedDatablock); + }); + + it("should succeed if 'if-unmodified-since' header is malformed", async () => { + const malformedHeaders = { + "if-unmodified-since": "not-a-date", + }; + + jest + .spyOn(origDatablocksService, "findOne") + .mockResolvedValue(mockDatablock); + jest + .spyOn(origDatablocksService, "findByIdAndUpdate") + .mockResolvedValue(updatedDatablock); + + const result = await controller.findByIdAndUpdate( + mockRequest, + "db123", + malformedHeaders, + mockUpdateDto, + ); + expect(result).toEqual(updatedDatablock); + }); + }); }); diff --git a/src/origdatablocks/origdatablocks.v4.controller.ts b/src/origdatablocks/origdatablocks.v4.controller.ts index f76518fe5..264827eb1 100644 --- a/src/origdatablocks/origdatablocks.v4.controller.ts +++ b/src/origdatablocks/origdatablocks.v4.controller.ts @@ -14,6 +14,8 @@ import { ForbiddenException, NotFoundException, InternalServerErrorException, + Headers, + HttpException, } from "@nestjs/common"; import { Request } from "express"; import { OrigDatablocksService } from "./origdatablocks.service"; @@ -770,6 +772,7 @@ export class OrigDatablocksV4Controller { async findByIdAndUpdate( @Req() request: Request, @Param("id") id: string, + @Headers() headers: Record, @Body() updateOrigDatablockDto: PartialUpdateOrigDatablockDto, ): Promise { await this.checkPermissionsForOrigDatablockWrite( @@ -778,16 +781,35 @@ export class OrigDatablocksV4Controller { Action.OrigdatablockUpdate, ); - const origdatablock = await this.origDatablocksService.findByIdAndUpdate( - id, - updateOrigDatablockDto, - ); + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; - if (origdatablock) { - await this.updateDatasetSizeAndFiles(origdatablock.datasetId); + const datablock = await this.origDatablocksService.findOne({ + where: { _id: id }, + }); + if (!datablock) { + throw new NotFoundException("Datablock not found"); } - return origdatablock; + if (headerDate && headerDate <= datablock.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } else { + const origdatablock = await this.origDatablocksService.findByIdAndUpdate( + id, + updateOrigDatablockDto, + ); + if (!origdatablock) { + throw new NotFoundException("Datablock not found"); + } + await this.updateDatasetSizeAndFiles(origdatablock.datasetId); + return origdatablock; + } } // DELETE /origdatablocks/:id diff --git a/src/proposals/proposals.controller.spec.ts b/src/proposals/proposals.controller.spec.ts index 44d84080e..ad0969b05 100644 --- a/src/proposals/proposals.controller.spec.ts +++ b/src/proposals/proposals.controller.spec.ts @@ -4,17 +4,24 @@ import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { DatasetsService } from "src/datasets/datasets.service"; import { ProposalsController } from "./proposals.controller"; import { ProposalsService } from "./proposals.service"; +import { NotFoundException, HttpException } from "@nestjs/common"; +import { PartialUpdateProposalDto } from "./dto/update-proposal.dto"; +import { ProposalClass } from "./schemas/proposal.schema"; class AttachmentsServiceMock {} class DatasetsServiceMock {} -class ProposalsServiceMock {} +class ProposalsServiceMock { + findOne = jest.fn(); + update = jest.fn(); +} class CaslAbilityFactoryMock {} describe("ProposalsController", () => { let controller: ProposalsController; + let proposalsService: ProposalsServiceMock; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -28,9 +35,111 @@ describe("ProposalsController", () => { }).compile(); controller = module.get(ProposalsController); + proposalsService = module.get(ProposalsService); + + // Mock permission check + controller["checkPermissionsForProposal"] = jest.fn(); }); it("should be defined", () => { expect(controller).toBeDefined(); }); + + describe("update", () => { + it("should throw NotFoundException if proposal not found", async () => { + proposalsService.findOne.mockResolvedValue(null); + + await expect( + controller.update( + {}, + "proposal-id", + {}, + {} as PartialUpdateProposalDto, + ), + ).rejects.toThrow(NotFoundException); + }); + + it("should throw HttpException if headerDate <= proposal.updatedAt", async () => { + const proposal = { updatedAt: new Date("2023-01-01") } as ProposalClass; + proposalsService.findOne.mockResolvedValue(proposal); + + const headers = { + "if-unmodified-since": "2022-12-31", + }; + + await expect( + controller.update( + {}, + "proposal-id", + headers, + {} as PartialUpdateProposalDto, + ), + ).rejects.toThrow(HttpException); + }); + + it("should call update and return updated proposal", async () => { + const proposal = { updatedAt: new Date("2022-12-31") } as ProposalClass; + const updatedProposal = { + ...proposal, + title: "Updated", + } as ProposalClass; + + proposalsService.findOne.mockResolvedValue(proposal); + proposalsService.update.mockResolvedValue(updatedProposal); + + const headers = { + "if-unmodified-since": "2023-01-01", + }; + + const result = await controller.update({}, "proposal-id", headers, { + title: "Updated", + }); + + expect(result).toEqual(updatedProposal); + expect(proposalsService.update).toHaveBeenCalledWith( + { proposalId: "proposal-id" }, + { title: "Updated" }, + ); + }); + + it("should proceed with update if header is missing", async () => { + const proposal = { updatedAt: new Date("2023-01-01") } as ProposalClass; + const updatedProposal = { + ...proposal, + title: "Updated", + } as ProposalClass; + + proposalsService.findOne.mockResolvedValue(proposal); + proposalsService.update.mockResolvedValue(updatedProposal); + + const headers = {}; // No 'if-unmodified-since' + + const result = await controller.update({}, "proposal-id", headers, { + title: "Updated", + }); + + expect(result).toEqual(updatedProposal); + }); + + it("should proceed with update if header is invalid date string", async () => { + const proposal = { updatedAt: new Date("2023-01-01") } as ProposalClass; + const updatedProposal = { + ...proposal, + title: "Updated", + } as ProposalClass; + + proposalsService.findOne.mockResolvedValue(proposal); + proposalsService.update.mockResolvedValue(updatedProposal); + + const headers = { + "if-unmodified-since": "not-a-valid-date", + }; + + const result = await controller.update({}, "proposal-id", headers, { + title: "Updated", + }); + + expect(result).toEqual(updatedProposal); + }); + }); }); diff --git a/src/proposals/proposals.controller.ts b/src/proposals/proposals.controller.ts index 8acf07a3a..126e551c2 100644 --- a/src/proposals/proposals.controller.ts +++ b/src/proposals/proposals.controller.ts @@ -17,6 +17,8 @@ import { Logger, InternalServerErrorException, NotFoundException, + Headers, + HttpException, } from "@nestjs/common"; import { Request } from "express"; import { ProposalsService } from "./proposals.service"; @@ -720,17 +722,43 @@ export class ProposalsController { async update( @Req() request: Request, @Param("pid") proposalId: string, + @Headers() headers: Record, @Body() updateProposalDto: PartialUpdateProposalDto, ): Promise { + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; + await this.checkPermissionsForProposal( request, proposalId, Action.ProposalsUpdate, ); - return this.proposalsService.update( - { proposalId: proposalId }, - updateProposalDto, - ); + + return this.proposalsService + .findOne({ where: { _id: proposalId } }) + .then((proposal: ProposalClass | null) => { + if (!proposal) { + throw new NotFoundException("Proposal not found"); + } + if (headerDate && headerDate <= proposal.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } + { + return this.proposalsService.update( + { proposalId: proposalId }, + updateProposalDto, + ); + } + }) + .catch((error) => { + throw error; + }); } // DELETE /proposals/:id diff --git a/src/published-data/published-data.v4.controller.ts b/src/published-data/published-data.v4.controller.ts index 81cfd51a1..1597c53b0 100644 --- a/src/published-data/published-data.v4.controller.ts +++ b/src/published-data/published-data.v4.controller.ts @@ -12,6 +12,7 @@ import { HttpStatus, NotFoundException, Req, + Headers, } from "@nestjs/common"; import { ApiBearerAuth, @@ -405,6 +406,7 @@ export class PublishedDataV4Controller { async publish( @Req() request: Request, @Param("id") id: string, + @Headers() headers: Record, ): Promise { const filter = this.getAccessBasedFilters(request, id); const publishedData = await this.publishedDataService.findOne(filter); @@ -426,9 +428,14 @@ export class PublishedDataV4Controller { const datasetPids = publishedData.datasetPids; await Promise.all( datasetPids.map(async (pid) => { - await this.datasetsController.findByIdAndUpdate(request, pid, { - isPublished: true, - }); + await this.datasetsController.findByIdAndUpdateInternal( + request, + pid, + headers, + { + isPublished: true, + }, + ); }), ); @@ -563,6 +570,7 @@ export class PublishedDataV4Controller { async register( @Req() request: Request, @Param("id") id: string, + @Headers() headers: Record, ): Promise { const filter = this.getAccessBasedFilters(request, id); const publishedData = await this.publishedDataService.findOne(filter); @@ -585,10 +593,15 @@ export class PublishedDataV4Controller { await Promise.all( publishedData.datasetPids.map(async (pid) => { - await this.datasetsController.findByIdAndUpdate(request, pid, { - isPublished: true, - datasetlifecycle: { publishedOn: data.registeredTime }, - }); + await this.datasetsController.findByIdAndUpdateInternal( + request, + pid, + headers, + { + isPublished: true, + datasetlifecycle: { publishedOn: data.registeredTime }, + }, + ); }), ); const registerDoiUri = this.configService.get("registerDoiUri"); diff --git a/src/samples/samples.controller.spec.ts b/src/samples/samples.controller.spec.ts index 3a96725f4..f70d05097 100644 --- a/src/samples/samples.controller.spec.ts +++ b/src/samples/samples.controller.spec.ts @@ -4,17 +4,25 @@ import { CaslAbilityFactory } from "src/casl/casl-ability.factory"; import { DatasetsService } from "src/datasets/datasets.service"; import { SamplesController } from "./samples.controller"; import { SamplesService } from "./samples.service"; +import { NotFoundException, HttpException } from "@nestjs/common"; +import { Request } from "express"; +import { SampleClass } from "./schemas/sample.schema"; +import { PartialUpdateSampleDto } from "./dto/update-sample.dto"; class AttachmentsServiceMock {} class DatasetsServiceMock {} -class SamplesServiceMock {} - class CaslAbilityFactoryMock {} +class SamplesServiceMock { + findOne = jest.fn(); + update = jest.fn(); +} + describe("SamplesController", () => { let controller: SamplesController; + let samplesService: SamplesService; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -28,9 +36,86 @@ describe("SamplesController", () => { }).compile(); controller = module.get(SamplesController); + samplesService = module.get(SamplesService); + jest.spyOn(controller, "checkPermissionsForSample").mockResolvedValue(true); }); it("should be defined", () => { expect(controller).toBeDefined(); }); + + describe("update", () => { + const sampleId = "sample123"; + const updateDto: PartialUpdateSampleDto = { name: "Updated Sample" }; + const mockRequest = {} as Request; + + it("should update sample when header is missing", async () => { + const sample = { _id: sampleId, updatedAt: new Date() } as SampleClass; + samplesService.findOne.mockResolvedValue(sample); + samplesService.update.mockResolvedValue({ ...sample, ...updateDto }); + + const result = await controller.update( + mockRequest, + sampleId, + updateDto, + {}, + ); + expect(result).toEqual({ ...sample, ...updateDto }); + }); + + it("should throw NotFoundException if sample not found", async () => { + samplesService.findOne.mockResolvedValue(null); + + await expect( + controller.update(mockRequest, sampleId, updateDto, {}), + ).rejects.toThrow(NotFoundException); + }); + + it("should throw PreconditionFailed if header date is older than updatedAt", async () => { + const sample = { + _id: sampleId, + updatedAt: new Date("2023-01-01"), + } as SampleClass; + samplesService.findOne.mockResolvedValue(sample); + + const headers = { + "if-unmodified-since": new Date("2022-01-01").toUTCString(), + }; + + await expect( + controller.update(mockRequest, sampleId, updateDto, headers), + ).rejects.toThrow(HttpException); + }); + it("should update sample if header date is invalid", async () => { + const sample = { _id: sampleId, updatedAt: new Date() } as SampleClass; + samplesService.findOne.mockResolvedValue(sample); + samplesService.update.mockResolvedValue({ ...sample, ...updateDto }); + + const headers = { + "if-unmodified-since": "invalid-date-string", + }; + + const result = await controller.update( + mockRequest, + sampleId, + updateDto, + headers, + ); + expect(result).toEqual({ ...sample, ...updateDto }); + }); + + it("should update sample if header date is not present", async () => { + const sample = { _id: sampleId, updatedAt: new Date() } as SampleClass; + samplesService.findOne.mockResolvedValue(sample); + samplesService.update.mockResolvedValue({ ...sample, ...updateDto }); + + const result = await controller.update( + mockRequest, + sampleId, + updateDto, + {}, + ); + expect(result).toEqual({ ...sample, ...updateDto }); + }); + }); }); diff --git a/src/samples/samples.controller.ts b/src/samples/samples.controller.ts index 6ad35bb17..0e4167917 100644 --- a/src/samples/samples.controller.ts +++ b/src/samples/samples.controller.ts @@ -18,6 +18,8 @@ import { Req, Header, NotFoundException, + Headers, + HttpException, } from "@nestjs/common"; import { SamplesService } from "./samples.service"; import { CreateSampleDto } from "./dto/create-sample.dto"; @@ -697,9 +699,33 @@ export class SamplesController { @Req() request: Request, @Param("id") id: string, @Body() updateSampleDto: PartialUpdateSampleDto, + @Headers() headers: Record, ): Promise { await this.checkPermissionsForSample(request, id, Action.SampleUpdate); - return this.samplesService.update({ sampleId: id }, updateSampleDto); + + const headerDateString = headers["if-unmodified-since"]; + const headerDate = + headerDateString && !isNaN(new Date(headerDateString).getTime()) + ? new Date(headerDateString) + : null; + + return this.samplesService + .findOne({ where: { _id: id } }) + .then((sample: SampleClass | null) => { + if (!sample) { + throw new NotFoundException("Sample not found"); + } + // If header is missing, always update , If header is present, compare with updatedAt + if (headerDate && headerDate <= sample.updatedAt) { + throw new HttpException( + "Update error due to failed if-modified-since condition", + HttpStatus.PRECONDITION_FAILED, + ); + } + { + return this.samplesService.update({ sampleId: id }, updateSampleDto); + } + }); } // DELETE /samples/:id