fix: considering header field if-unmodified-since for different patch requests

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>(AttachmentsV4Controller);
+    service = module.get<AttachmentsV4Service>(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,
+      ),
+    );
+  });
+});

src/attachments/attachments.v4.controller.ts

@@ -15,6 +15,8 @@ import {
   Patch,
   Put,
   HttpCode,
+  Headers,
+  HttpException,
 } from "@nestjs/common";
 import {
   ApiBearerAuth,
@@ -365,21 +367,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<string, string>,
     @Body() updateAttachmentDto: PartialUpdateAttachmentV4Dto,
   ): Promise<OutputAttachmentV4Dto | null> {
-    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

src/datasets/datasets.controller.spec.ts

@@ -7,21 +7,35 @@ import { DatasetsService } from "./datasets.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();
+}
 
 describe("DatasetsController", () => {
   let controller: DatasetsController;
+  let datasetsService: DatasetsServiceMock;
+  let caslAbilityFactory: CaslAbilityFactoryMock;
 
   beforeEach(async () => {
     const module: TestingModule = await Test.createTestingModule({
@@ -38,9 +52,137 @@ describe("DatasetsController", () => {
     }).compile();
 
     controller = module.get<DatasetsController>(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);
+    });
+  });
 });