Merge branch 'master' into SWAP-4629-fix-affected-dataset-with-size-0

Author: martin-trajanovski

package-lock.json

@@ -46,6 +46,7 @@
     "@nestjs/platform-express": "^11",
     "@nestjs/swagger": "^11.1.1",
     "@nestjs/terminus": "^11",
+    "@types/json-merge-patch": "^1.0.0",
     "@user-office-software/duo-logger": "^2.1.1",
     "@user-office-software/duo-message-broker": "^1.4.0",
     "ajv": "^8.12.0",
@@ -58,6 +59,7 @@
     "express-session": "^1.17.3",
     "handlebars": "^4.7.7",
     "js-yaml": "^4.1.0",
+    "json-merge-patch": "^1.0.2",
     "jsonpath-plus": "^10.1.0",
     "lodash": "^4.17.21",
     "luxon": "^3.2.1",

package.json

@@ -19,13 +19,15 @@ import {
 import {
   ApiBearerAuth,
   ApiBody,
+  ApiConsumes,
   ApiOperation,
   ApiParam,
   ApiQuery,
   ApiResponse,
   ApiTags,
 } from "@nestjs/swagger";
 import { Request } from "express";
+import * as jmp from "json-merge-patch";
 import { PoliciesGuard } from "src/casl/guards/policies.guard";
 import { CheckPolicies } from "src/casl/decorators/check-policies.decorator";
 import { AppAbility, CaslAbilityFactory } from "src/casl/casl-ability.factory";
@@ -325,13 +327,14 @@ export class AttachmentsV4Controller {
   @ApiOperation({
     summary: "It updates the attachment.",
     description:
-      "It updates the attachment specified through the id specified. it updates only the specified fields.",
+      "It updates the attachment specified through the id specified. It updates only the specified fields. Set `content-type` to `application/merge-patch+json` if you would like to update nested objects. Warning! `application/merge-patch+json` doesn’t support updating a specific item in an array — the result will always replace the entire target if it’s not an object.",
   })
   @ApiParam({
     name: "aid",
     description: "ID of the attachment to modify",
     type: String,
   })
+  @ApiConsumes("application/json", "application/merge-patch+json")
   @ApiBody({
     type: PartialUpdateAttachmentV4Dto,
   })
@@ -347,14 +350,18 @@ export class AttachmentsV4Controller {
     @Param("aid") aid: string,
     @Body() updateAttachmentDto: PartialUpdateAttachmentV4Dto,
   ): Promise<OutputAttachmentV4Dto | null> {
-    await this.checkPermissionsForAttachment(
+    const foundAattachment = await this.checkPermissionsForAttachment(
       request,
       aid,
       Action.AttachmentUpdateEndpoint,
     );
+    const updateAttachmentDtoForservice =
+      request.headers["content-type"] === "application/merge-patch+json"
+        ? jmp.apply(foundAattachment, updateAttachmentDto)
+        : updateAttachmentDto;
     return this.attachmentsService.findOneAndUpdate(
       { _id: aid },
-      updateAttachmentDto,
+      updateAttachmentDtoForservice,
     );
   }
 

src/attachments/attachments.v4.controller.ts

@@ -10,11 +10,6 @@ export class FullFacetFilters {
   fields?: string;
 }
 
-export class SampleCountFilters {
-  @ApiPropertyOptional()
-  fields?: string;
-}
-
 export class FullQueryFilters {
   @ApiPropertyOptional()
   limits?: string;

src/common/types.ts

@@ -216,9 +216,7 @@ export const mapScientificQuery = (
 /**Check if input is object or a physical quantity */
 const isObject = (x: unknown) => {
   if (
-    x &&
-    typeof x === "object" &&
-    !Array.isArray(x) &&
+    IsRecord(x) &&
     !(x as Record<string, unknown>).unit &&
     (x as Record<string, unknown>).unit !== "" &&
     !(x as Record<string, unknown>).u &&
@@ -229,6 +227,18 @@ const isObject = (x: unknown) => {
   return false;
 };
 
+export const IsRecord = (x: unknown): x is Record<string, unknown> => {
+  // checks if value is (nested) object
+  return x !== null && typeof x === "object" && !Array.isArray(x);
+};
+
+export const IsValueUnitObject = (
+  x: unknown,
+): x is { value?: unknown; unit?: unknown } => {
+  // checks if the argument is object and contains either property 'value' or 'unit'
+  return IsRecord(x) && ("value" in x || "unit" in x);
+};
+
 export const extractMetadataKeys = <T>(
   instances: T[],
   prop: keyof T,
@@ -959,7 +969,7 @@ export const datasetsFullQueryDescriptionFields =
   </pre>';
 
 export const jobsFullQueryExampleFields =
-  '{"ownerGroup":["group1"], "statusCode": "jobCreated"}';
+  '{"ownerGroup": "group1", "statusCode": "jobCreated"}';
 
 export const jobsFullQueryDescriptionFields =
   '<pre>\n  \

src/common/utils.ts

@@ -14,13 +14,15 @@ import {
   HttpStatus,
   NotFoundException,
   Req,
+  BadRequestException,
   ForbiddenException,
   InternalServerErrorException,
   ConflictException,
 } from "@nestjs/common";
 import {
   ApiBearerAuth,
   ApiBody,
+  ApiConsumes,
   ApiExtraModels,
   ApiOperation,
   ApiParam,
@@ -30,6 +32,8 @@ import {
 } from "@nestjs/swagger";
 import { Request } from "express";
 import { MongoError } from "mongodb";
+import * as jmp from "json-merge-patch";
+import { IsRecord, IsValueUnitObject } from "../common/utils";
 import { DatasetsService } from "./datasets.service";
 import { DatasetClass, DatasetDocument } from "./schemas/dataset.schema";
 import { PoliciesGuard } from "src/casl/guards/policies.guard";
@@ -236,6 +240,47 @@ export class DatasetsV4Controller {
     return filter;
   }
 
+  findInvalidValueUnitUpdates(
+    updateDto: Record<string, unknown>,
+    dataset: Record<string, unknown>,
+    path: string[] = [],
+  ): string[] {
+    // collect properties that have both 'value' and 'unit' in original dataset but one of either is missing in the updateDto body
+    const unmatched: string[] = [];
+
+    for (const key in updateDto) {
+      const value = updateDto[key];
+      const currentPath = [...path, key];
+      if (IsValueUnitObject(value)) {
+        const datasetAtKey = currentPath.reduce<unknown>(
+          (obj, k) => (IsRecord(obj) ? obj[k] : undefined),
+          dataset,
+        );
+        if (IsValueUnitObject(datasetAtKey)) {
+          // check if current object's 'value' or 'unit' are not undefined in original dataset and passed updateDto
+          const originalHasValue = datasetAtKey.value !== undefined;
+          const originalHasUnit = datasetAtKey.unit !== undefined;
+          const updateHasValue = value.value !== undefined;
+          const updateHasUnit = value.unit !== undefined;
+          if (
+            originalHasValue &&
+            originalHasUnit &&
+            !(updateHasValue && updateHasUnit)
+          ) {
+            unmatched.push(currentPath.join("."));
+          }
+        }
+      }
+      // recursively go through the (scientificMetadata) object
+      if (IsRecord(value)) {
+        unmatched.push(
+          ...this.findInvalidValueUnitUpdates(value, dataset, currentPath),
+        );
+      }
+    }
+    return unmatched;
+  }
+
   // POST /api/v4/datasets
   @UseGuards(PoliciesGuard)
   @CheckPolicies("datasets", (ability: AppAbility) =>
@@ -670,13 +715,14 @@ export class DatasetsV4Controller {
   @ApiOperation({
     summary: "It partially updates the dataset.",
     description:
-      "It updates the dataset through the pid specified. It updates only the specified fields.",
+      "It updates the dataset through the pid specified. It updates only the specified fields. Set `content-type` to `application/merge-patch+json` if you would like to update nested objects. Warning! `application/merge-patch+json` doesn’t support updating a specific item in an array — the result will always replace the entire target if it’s not an object.",
   })
   @ApiParam({
     name: "pid",
     description: "Id of the dataset to modify",
     type: String,
   })
+  @ApiConsumes("application/merge-patch+json", "application/json")
   @ApiBody({
     description:
       "Fields that needs to be updated in the dataset. Only the fields that needs to be updated have to be passed in.",
@@ -705,11 +751,30 @@ export class DatasetsV4Controller {
       Action.DatasetUpdate,
     );
 
+    if (foundDataset && IsRecord(updateDatasetDto) && IsRecord(foundDataset)) {
+      const mismatchedPaths = this.findInvalidValueUnitUpdates(
+        updateDatasetDto,
+        foundDataset,
+      );
+      if (mismatchedPaths.length > 0) {
+        throw new BadRequestException(
+          `Original dataset ${pid} contains both value and unit in ${mismatchedPaths.join(", ")}. Please provide both when updating.`,
+        );
+      }
+    } else {
+      throw new BadRequestException(
+        `Failed to compare scientific metadata to include both value and units`,
+      );
+    }
+
+    const updateDatasetDtoForService =
+      request.headers["content-type"] === "application/merge-patch+json"
+        ? jmp.apply(foundDataset, updateDatasetDto)
+        : updateDatasetDto;
     const updatedDataset = await this.datasetsService.findByIdAndUpdate(
       pid,
-      updateDatasetDto,
+      updateDatasetDtoForService,
     );
-
     return updatedDataset;
   }