Support bidirectional mapping for Change Metadata (#6906)

# Description of Changes The Change Metadata tool was missed from the bidirectional mappings added in #6867. This PR adds it to the list of supported tools.
2026-09-03 05:10:16 +03:00 · 2026-07-08 12:47:11 +00:00
parent c8f238ae60
commit 8150d16b6f
3 changed files with 234 additions and 56 deletions
@@ -1,5 +1,12 @@
-import { buildChangeMetadataFormData } from "@app/hooks/tools/changeMetadata/useChangeMetadataOperation";
-import { ChangeMetadataParameters } from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters";
+import {
+  buildChangeMetadataFormData,
+  changeMetadataToApiParams,
+  changeMetadataFromApiParams,
+} from "@app/hooks/tools/changeMetadata/useChangeMetadataOperation";
+import {
+  ChangeMetadataParameters,
+  defaultParameters,
+} from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters";
 import { TrappedStatus } from "@app/types/metadata";
 import { describe, expect, test } from "vitest";

@@ -142,3 +149,78 @@ describe("buildChangeMetadataFormData", () => {
    expect(formData.get("allRequestParams[customValue1]")).toBe("Engineering");
  });
 });
+
+describe("changeMetadata mappers", () => {
+  const fullApi: Parameters<typeof changeMetadataFromApiParams>[0] = {
+    title: "Title",
+    author: "Author",
+    subject: "Subject",
+    keywords: "a, b",
+    creator: "Creator",
+    producer: "Producer",
+    creationDate: "2024/01/15 10:30:00",
+    modificationDate: "2024/02/20 14:05:09",
+    trapped: "True",
+    deleteAll: false,
+    allRequestParams: {
+      customKey1: "Department",
+      customValue1: "Engineering",
+      customKey2: "Project",
+      customValue2: "Falcon",
+    },
+  };
+
+  test("round-trips a full request through fromApiParams and back", () => {
+    const roundTripped = changeMetadataToApiParams({
+      ...defaultParameters,
+      ...changeMetadataFromApiParams(fullApi),
+    });
+
+    expect(roundTripped).toEqual(fullApi);
+  });
+
+  test("reconstructs dates in local time and clears absent ones", () => {
+    const params = changeMetadataFromApiParams(fullApi);
+    expect(params.creationDate).toEqual(new Date(2024, 0, 15, 10, 30, 0));
+    expect(params.modificationDate).toEqual(new Date(2024, 1, 20, 14, 5, 9));
+
+    const cleared = changeMetadataFromApiParams({
+      creationDate: "",
+      modificationDate: undefined,
+    });
+    expect(cleared.creationDate).toBeNull();
+    expect(cleared.modificationDate).toBeNull();
+  });
+
+  test.each([TrappedStatus.TRUE, TrappedStatus.FALSE, TrappedStatus.UNKNOWN])(
+    "round-trips trapped=%s",
+    (trapped) => {
+      const api = changeMetadataToApiParams({ ...defaultParameters, trapped });
+      expect(api.trapped).toBe(trapped);
+      expect(changeMetadataFromApiParams(api).trapped).toBe(trapped);
+    },
+  );
+
+  test("falls back to the default for an unrecognised trapped value", () => {
+    const params = changeMetadataFromApiParams({
+      trapped: "Bogus",
+    } as unknown as Parameters<typeof changeMetadataFromApiParams>[0]);
+    expect(params.trapped).toBe(defaultParameters.trapped);
+  });
+
+  test("reconstructs custom metadata, tolerating non-contiguous indices", () => {
+    const params = changeMetadataFromApiParams({
+      allRequestParams: {
+        customKey1: "Department",
+        customValue1: "Engineering",
+        customKey3: "Project",
+        customValue3: "Falcon",
+      },
+    });
+
+    expect(params.customMetadata).toEqual([
+      { key: "Department", value: "Engineering", id: "custom1" },
+      { key: "Project", value: "Falcon", id: "custom3" },
+    ]);
+  });
+});
@@ -3,13 +3,22 @@ import {
  useToolOperation,
  defineSingleFileTool,
 } from "@app/hooks/tools/shared/useToolOperation";
+import {
+  objectToFormData,
+  type ToolApiParams,
+  type ToolEndpoint,
+} from "@app/hooks/tools/shared/toolApiMapping";
 import { createStandardErrorHandler } from "@app/utils/toolErrorHandler";
+import { TrappedStatus, CustomMetadataEntry } from "@app/types/metadata";
 import {
  ChangeMetadataParameters,
  defaultParameters,
 } from "@app/hooks/tools/changeMetadata/useChangeMetadataParameters";

-// Helper function to format Date object to string
+const ENDPOINT = "/api/v1/misc/update-metadata" satisfies ToolEndpoint;
+type ChangeMetadataApiParams = ToolApiParams[typeof ENDPOINT];
+
+// Backend date format (yyyy/MM/dd HH:mm:ss), in local time to mirror the parser.
 const formatDateForBackend = (date: Date | null): string => {
  if (!date) return "";
  const year = date.getFullYear();
@@ -21,64 +30,135 @@ const formatDateForBackend = (date: Date | null): string => {
  return `${year}/${month}/${day} ${hours}:${minutes}:${seconds}`;
 };

+// Inverse of formatDateForBackend; returns null for empty or unparseable input.
+const parseDateFromBackend = (value: string | undefined): Date | null => {
+  if (!value) return null;
+  const match = /^(\d{4})\/(\d{2})\/(\d{2}) (\d{2}):(\d{2}):(\d{2})$/.exec(
+    value,
+  );
+  if (!match) return null;
+  const [, year, month, day, hours, minutes, seconds] = match;
+  return new Date(
+    Number(year),
+    Number(month) - 1,
+    Number(day),
+    Number(hours),
+    Number(minutes),
+    Number(seconds),
+  );
+};
+
+// Custom metadata is carried in the request's allRequestParams map as paired
+// customKey<N>/customValue<N> entries; the backend rejoins them by the shared
+// index N (see MetadataController). Only entries with a non-blank key and value
+// are sent, and they are re-numbered from 1 so the indices stay contiguous.
+const buildCustomMetadataMap = (
+  customMetadata: CustomMetadataEntry[],
+): Record<string, string> | undefined => {
+  const validEntries = customMetadata.filter(
+    (entry) => entry.key.trim() && entry.value.trim(),
+  );
+  if (validEntries.length === 0) return undefined;
+
+  const map: Record<string, string> = {};
+  validEntries.forEach((entry, index) => {
+    const n = index + 1;
+    map[`customKey${n}`] = entry.key.trim();
+    map[`customValue${n}`] = entry.value.trim();
+  });
+  return map;
+};
+
+// Rebuild the UI's custom metadata list from the allRequestParams map by pairing
+// customKey<N>/customValue<N> on their shared index N. Mirrors the backend
+// (MetadataController), which pairs by index across all entries and does not
+// assume the indices are contiguous, so a non-contiguous stored map round-trips.
+const parseCustomMetadataMap = (
+  allRequestParams: ChangeMetadataApiParams["allRequestParams"],
+): CustomMetadataEntry[] => {
+  if (!allRequestParams) return [];
+  return Object.keys(allRequestParams)
+    .map((key) => /^customKey(\d+)$/.exec(key)?.[1])
+    .filter((n): n is string => n !== undefined)
+    .map(Number)
+    .sort((a, b) => a - b)
+    .map((n) => ({
+      key: allRequestParams[`customKey${n}`] ?? "",
+      value: allRequestParams[`customValue${n}`] ?? "",
+      id: `custom${n}`,
+    }));
+};
+
+// Map the backend's trapped string onto the UI enum, validating against the
+// actual enum values so an unrecognised value falls back to the default instead
+// of being force-cast.
+const parseTrapped = (value: string | undefined): TrappedStatus =>
+  Object.values(TrappedStatus).find((status) => status === value) ??
+  defaultParameters.trapped;
+
+// Convert the tool's UI parameters into the update-metadata request body. The
+// return type is the generated backend model, so a spec change that renames or
+// drops a field breaks the build here.
+export const changeMetadataToApiParams = (
+  parameters: ChangeMetadataParameters,
+): ChangeMetadataApiParams => ({
+  title: parameters.title,
+  author: parameters.author,
+  subject: parameters.subject,
+  keywords: parameters.keywords,
+  creator: parameters.creator,
+  producer: parameters.producer,
+  creationDate: formatDateForBackend(parameters.creationDate),
+  modificationDate: formatDateForBackend(parameters.modificationDate),
+  trapped: parameters.trapped,
+  deleteAll: parameters.deleteAll,
+  allRequestParams: buildCustomMetadataMap(parameters.customMetadata),
+});
+
+// Reconstruct the tool's UI parameters from an update-metadata request body, so
+// a stored or AI-authored step can be re-rendered in the settings UI.
+export const changeMetadataFromApiParams = (
+  apiParams: ChangeMetadataApiParams,
+): Partial<ChangeMetadataParameters> => ({
+  title: apiParams.title ?? defaultParameters.title,
+  author: apiParams.author ?? defaultParameters.author,
+  subject: apiParams.subject ?? defaultParameters.subject,
+  keywords: apiParams.keywords ?? defaultParameters.keywords,
+  creator: apiParams.creator ?? defaultParameters.creator,
+  producer: apiParams.producer ?? defaultParameters.producer,
+  creationDate: parseDateFromBackend(apiParams.creationDate),
+  modificationDate: parseDateFromBackend(apiParams.modificationDate),
+  trapped: parseTrapped(apiParams.trapped),
+  deleteAll: apiParams.deleteAll ?? defaultParameters.deleteAll,
+  customMetadata: parseCustomMetadataMap(apiParams.allRequestParams),
+});
+
 // Static function that can be used by both the hook and automation executor
 export const buildChangeMetadataFormData = (
  parameters: ChangeMetadataParameters,
  file: File,
 ): FormData => {
-  const formData = new FormData();
-  formData.append("fileInput", file);
-
-  // Standard metadata fields
-  formData.append("title", parameters.title || "");
-  formData.append("author", parameters.author || "");
-  formData.append("subject", parameters.subject || "");
-  formData.append("keywords", parameters.keywords || "");
-  formData.append("creator", parameters.creator || "");
-  formData.append("producer", parameters.producer || "");
-
-  // Date fields - convert Date objects to strings
-  formData.append(
-    "creationDate",
-    formatDateForBackend(parameters.creationDate),
-  );
-  formData.append(
-    "modificationDate",
-    formatDateForBackend(parameters.modificationDate),
-  );
-
-  // Trapped status
-  formData.append("trapped", parameters.trapped || "");
-
-  // Delete all metadata flag
-  formData.append("deleteAll", parameters.deleteAll.toString());
-
-  // Custom metadata - backend expects them as values to 'allRequestParams[customKeyX/customValueX]'
-  let keyNumber = 0;
-  if (parameters.customMetadata && Array.isArray(parameters.customMetadata)) {
-    parameters.customMetadata.forEach((entry) => {
-      if (entry.key.trim() && entry.value.trim()) {
-        keyNumber += 1;
-        formData.append(
-          `allRequestParams[customKey${keyNumber}]`,
-          entry.key.trim(),
-        );
-        formData.append(
-          `allRequestParams[customValue${keyNumber}]`,
-          entry.value.trim(),
-        );
-      }
-    });
+  // allRequestParams is a Spring-bound map: objectToFormData only serializes
+  // primitives, so the scalar fields go through it and the map is flattened into
+  // allRequestParams[<key>] form fields separately.
+  const { allRequestParams, ...scalarParams } =
+    changeMetadataToApiParams(parameters);
+  const formData = objectToFormData(scalarParams, { fileInput: file });
+  for (const [key, value] of Object.entries(allRequestParams ?? {})) {
+    if (value !== undefined) {
+      formData.append(`allRequestParams[${key}]`, value);
+    }
  }
-
  return formData;
 };

 // Static configuration object
 export const changeMetadataOperationConfig = defineSingleFileTool({
  buildFormData: buildChangeMetadataFormData,
+  toApiParams: changeMetadataToApiParams,
+  fromApiParams: changeMetadataFromApiParams,
  operationType: "changeMetadata",
-  endpoint: "/api/v1/misc/update-metadata",
+  endpoint: ENDPOINT,
  defaultParameters,
 });

@@ -1,5 +1,8 @@
 import { describe, expect, test } from "vitest";
-import { type RegistryToolOperationConfig } from "@app/hooks/tools/shared/toolOperationTypes";
+import {
+  ToolType,
+  type RegistryToolOperationConfig,
+} from "@app/hooks/tools/shared/toolOperationTypes";
 import { objectToFormData } from "@app/hooks/tools/shared/toolApiMapping";

 // Pilot tools.
@@ -17,6 +20,7 @@ import { adjustPageScaleOperationConfig } from "@app/hooks/tools/adjustPageScale
 import { autoRenameOperationConfig } from "@app/hooks/tools/autoRename/useAutoRenameOperation";
 import { bookletImpositionOperationConfig } from "@app/hooks/tools/bookletImposition/useBookletImpositionOperation";
 import { certSignOperationConfig } from "@app/hooks/tools/certSign/useCertSignOperation";
+import { changeMetadataOperationConfig } from "@app/hooks/tools/changeMetadata/useChangeMetadataOperation";
 import { changePermissionsOperationConfig } from "@app/hooks/tools/changePermissions/useChangePermissionsOperation";
 import { cropOperationConfig } from "@app/hooks/tools/crop/useCropOperation";
 import { editTableOfContentsOperationConfig } from "@app/hooks/tools/editTableOfContents/useEditTableOfContentsOperation";
@@ -57,6 +61,7 @@ const MIGRATED_CONFIGS = [
  autoRenameOperationConfig,
  bookletImpositionOperationConfig,
  certSignOperationConfig,
+  changeMetadataOperationConfig,
  changePermissionsOperationConfig,
  cropOperationConfig,
  editTableOfContentsOperationConfig,
@@ -102,15 +107,26 @@ describe("migrated tool mappers (sweep)", () => {
      expect(config.toApiParams).toBeDefined();
      expect(config.fromApiParams).toBeDefined();

-      // toApiParams(defaults) must produce a body objectToFormData can serialize
-      // (i.e. only primitives / arrays of primitives). A mapper that leaked a
-      // structured value would throw here.
+      // Serialize the defaults through the tool's own buildFormData - the real
+      // path the executor uses - so a tool whose toApiParams carries a structured
+      // field that buildFormData flattens itself (e.g. changeMetadata's
+      // allRequestParams map) is exercised too, not just tools whose mapper
+      // output is directly objectToFormData-able. Custom tools have no
+      // buildFormData, so fall back to serializing the mapper output directly.
      const params =
        config.defaultParameters ?? FALLBACK_PARAMS[config.operationType] ?? {};
-      const apiParams = config.toApiParams!(params);
-      expect(() =>
-        objectToFormData(apiParams, { fileInput: file }),
-      ).not.toThrow();
+      if (config.toolType === ToolType.multiFile) {
+        const build = config.buildFormData;
+        expect(() => build(params, [file])).not.toThrow();
+      } else if (config.toolType === ToolType.singleFile) {
+        const build = config.buildFormData;
+        expect(() => build(params, file)).not.toThrow();
+      } else {
+        const toApiParams = config.toApiParams!;
+        expect(() =>
+          objectToFormData(toApiParams(params), { fileInput: file }),
+        ).not.toThrow();
+      }
    },
  );
 });