refactoring: introduce componentSpecFromYaml (#1460)

## Description

Added a new `componentSpecFromYaml` function in `utils/yaml.ts` that safely parses YAML text into a ComponentSpec object with proper validation. This function replaces the direct use of `yaml.load()` in the component service, adding type checking and validation to prevent invalid component specifications.

## Type of Change

- [x] Improvement
- [x] Cleanup/Refactor

## Checklist

- [x] I have tested this does not break current pipelines / runs functionality
- [ ] I have tested the changes on staging

## Test Instructions

1. No functional changes to the app
2. No regression should be found across the app

Author: maxy-shpfy

src/services/componentService.test.ts

@@ -78,13 +78,15 @@ describe("componentService", () => {
 
   describe("parseComponentData", () => {
     it("should parse valid YAML data", () => {
-      const yamlData = "name: test-component\nversion: 1.2";
-      const result = parseComponentData(yamlData);
-
-      expect(result).toEqual({
+      const spec = {
         name: "test-component",
         version: 1.2,
-      });
+        implementation: { container: { image: "test" } },
+      };
+      const yamlData = yaml.dump(spec);
+      const result = parseComponentData(yamlData);
+
+      expect(result).toEqual(spec);
     });
 
     it("should return null for invalid YAML", () => {

src/services/componentService.ts

@@ -1,5 +1,3 @@
-import yaml from "js-yaml";
-
 import { getAppSettings } from "@/appSettings";
 import {
   type ComponentFolder,
@@ -36,6 +34,7 @@ import {
   saveComponent,
   type UserComponent,
 } from "@/utils/localforage";
+import { componentSpecFromYaml } from "@/utils/yaml";
 
 export interface ExistingAndNewComponent {
   existingComponent: UserComponent | undefined;
@@ -154,7 +153,7 @@ export const fetchAndStoreComponentByUrl = async (
       const text = await fetchComponentTextFromUrl(url);
       if (text) {
         try {
-          return yaml.load(text) as ComponentSpec;
+          return componentSpecFromYaml(text);
         } catch (error) {
           console.error(`Error parsing component at ${url}:`, error);
         }
@@ -182,7 +181,7 @@ export const fetchAndStoreComponentByUrl = async (
     });
 
     try {
-      return yaml.load(text) as ComponentSpec;
+      return componentSpecFromYaml(text);
     } catch (error) {
       console.error(`Error parsing component at ${url}:`, error);
       return null;
@@ -263,7 +262,7 @@ const parseTextToSpec = async (
 ): Promise<ComponentSpec | null> => {
   if (text) {
     try {
-      return yaml.load(text) as ComponentSpec;
+      return componentSpecFromYaml(text);
     } catch (error) {
       console.error("Error parsing component text:", error);
     }
@@ -316,7 +315,7 @@ export const fetchComponentTextFromUrl = async (
  */
 export const parseComponentData = (data: string): ComponentSpec | null => {
   try {
-    return yaml.load(data) as ComponentSpec;
+    return componentSpecFromYaml(data);
   } catch (error) {
     console.error("Error parsing component data:", error);
     return null;
@@ -445,7 +444,7 @@ async function hydrateFromPartialContentfulComponentReference(
     : component.text;
 
   const spec = isTextOnlyComponentReference(component)
-    ? (yaml.load(component.text) as ComponentSpec)
+    ? componentSpecFromYaml(component.text)
     : component.spec;
 
   if (!text || !spec) {

src/services/importPipeline.test.ts

@@ -1,41 +1,37 @@
+import yaml from "js-yaml";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
-import * as componentSpecModule from "@/utils/componentSpec";
 import * as componentStore from "@/utils/componentStore";
 import { USER_PIPELINES_LIST_NAME } from "@/utils/constants";
 
 import { importPipelineFromYaml } from "./pipelineService";
 
-vi.mock("@/utils/componentStore", () => ({
-  componentSpecToYaml: vi.fn(),
-  getComponentFileFromList: vi.fn(),
-  writeComponentToFileListFromText: vi.fn().mockResolvedValue({}),
-}));
-
-vi.mock("@/utils/componentSpec", () => ({
-  isGraphImplementation: vi.fn(),
-}));
-
 describe("importPipelineFromYaml", () => {
-  const validYamlContent = `
-name: Test Pipeline
-metadata:
-  annotations:
-    sdk: https://cloud-pipelines.net/pipeline-editor/
-implementation:
-  graph:
-    tasks: {}
-    outputValues: {}
-`;
+  const validYamlObject = {
+    name: "Test Pipeline",
+    metadata: {
+      annotations: {
+        sdk: "https://cloud-pipelines.net/pipeline-editor/",
+      },
+    },
+    implementation: {
+      graph: {
+        tasks: [],
+        outputValues: [],
+      },
+    },
+  };
+
+  const validYamlContent = yaml.dump(validYamlObject);
 
   beforeEach(() => {
     vi.clearAllMocks();
-    // Default to returning true for valid tests
-    vi.mocked(componentSpecModule.isGraphImplementation).mockReturnValue(true);
-    // Default for componentSpecToYaml
-    vi.mocked(componentStore.componentSpecToYaml).mockReturnValue(
-      "mocked-yaml",
+    vi.spyOn(console, "error").mockImplementation(() => {});
+
+    vi.spyOn(componentStore, "getComponentFileFromList").mockResolvedValue(
+      null,
     );
+    vi.spyOn(componentStore, "writeComponentToFileListFromText");
   });
 
   afterEach(() => {
@@ -48,17 +44,8 @@ implementation:
 
     const result = await importPipelineFromYaml(validYamlContent);
 
-    // Expect the function to call componentSpecToYaml
-    expect(componentStore.componentSpecToYaml).toHaveBeenCalled();
-
     // Expect writeComponentToFileListFromText to be called with correct parameters
-    expect(
-      componentStore.writeComponentToFileListFromText,
-    ).toHaveBeenCalledWith(
-      USER_PIPELINES_LIST_NAME,
-      "Test Pipeline",
-      "mocked-yaml",
-    );
+    expect(componentStore.writeComponentToFileListFromText).toHaveBeenCalled();
 
     // Expect successful result
     expect(result).toEqual({
@@ -79,9 +66,6 @@ implementation:
       },
     );
 
-    // Mock the console.error to prevent test output pollution
-    vi.spyOn(console, "error").mockImplementation(() => {});
-
     const result = await importPipelineFromYaml(validYamlContent, false);
 
     // Since we're now renaming rather than erroring, expect a successful result
@@ -95,7 +79,7 @@ implementation:
     ).toHaveBeenCalledWith(
       USER_PIPELINES_LIST_NAME,
       "Test Pipeline (1)",
-      "mocked-yaml",
+      expect.stringContaining("name: Test Pipeline (1)"),
     );
   });
 
@@ -114,9 +98,6 @@ implementation:
       },
     );
 
-    // Mock the console.error
-    vi.spyOn(console, "error").mockImplementation(() => {});
-
     const result = await importPipelineFromYaml(validYamlContent, false);
 
     // Expect a successful result with the name incremented to (3)
@@ -129,14 +110,14 @@ implementation:
     ).toHaveBeenCalledWith(
       USER_PIPELINES_LIST_NAME,
       "Test Pipeline (3)",
-      "mocked-yaml",
+      yaml.dump({
+        ...validYamlObject,
+        name: "Test Pipeline (3)",
+      }),
     );
   });
 
   it("should handle invalid YAML content", async () => {
-    // Mock the console.error to prevent test output pollution
-    vi.spyOn(console, "error").mockImplementation(() => {});
-
     const result = await importPipelineFromYaml("invalid: yaml: content: -");
 
     // Expect unsuccessful result
@@ -146,19 +127,16 @@ implementation:
   });
 
   it("should handle non-graph pipelines", async () => {
-    // Mock isGraphImplementation to return false
-    vi.mocked(componentSpecModule.isGraphImplementation).mockReturnValue(false);
-
-    // Mock the console.error
-    vi.spyOn(console, "error").mockImplementation(() => {});
-
-    const containerPipeline = `
-name: Container Pipeline
-implementation:
-  container:
-    image: test-image
-    command: ["echo", "hello"]
-`;
+    const containerPipelineObj = {
+      name: "Container Pipeline",
+      implementation: {
+        container: {
+          image: "test-image",
+          command: ["echo", "hello"],
+        },
+      },
+    };
+    const containerPipeline = yaml.dump(containerPipelineObj);
 
     const result = await importPipelineFromYaml(containerPipeline);
 
@@ -173,18 +151,21 @@ implementation:
   });
 
   it("should use default name for unnamed pipelines", async () => {
-    // Mock isGraphImplementation to return true
-    vi.mocked(componentSpecModule.isGraphImplementation).mockReturnValue(true);
-
-    const unnamedYaml = `
-metadata:
-  annotations:
-    sdk: https://cloud-pipelines.net/pipeline-editor/
-implementation:
-  graph:
-    tasks: {}
-    outputValues: {}
-`;
+    const unnamedPipelineSpec = {
+      metadata: {
+        annotations: {
+          sdk: "https://cloud-pipelines.net/pipeline-editor/",
+        },
+      },
+      implementation: {
+        graph: {
+          tasks: {},
+          outputValues: {},
+        },
+      },
+    };
+
+    const unnamedYaml = yaml.dump(unnamedPipelineSpec);
 
     vi.mocked(componentStore.getComponentFileFromList).mockResolvedValue(null);
 
@@ -196,7 +177,7 @@ implementation:
     ).toHaveBeenCalledWith(
       USER_PIPELINES_LIST_NAME,
       "Imported Pipeline",
-      "mocked-yaml",
+      unnamedYaml,
     );
 
     expect(result.name).toBe("Imported Pipeline");

src/services/pipelineService.ts

@@ -17,6 +17,7 @@ import {
   writeComponentToFileListFromText,
 } from "@/utils/componentStore";
 import { USER_PIPELINES_LIST_NAME } from "@/utils/constants";
+import { componentSpecFromYaml } from "@/utils/yaml";
 
 export const deletePipeline = async (name: string, onDelete?: () => void) => {
   try {
@@ -178,19 +179,7 @@ export async function importPipelineFromYaml(
 ): Promise<ImportResult> {
   try {
     // Parse the YAML content to get the component spec
-    const componentSpec = yaml.load(yamlContent) as ComponentSpec;
-
-    if (!componentSpec || typeof componentSpec !== "object") {
-      const errorMessage =
-        "Invalid YAML content. Could not parse as a component spec.";
-      console.error(errorMessage);
-      return {
-        name: "",
-        overwritten: false,
-        successful: false,
-        errorMessage,
-      };
-    }
+    const componentSpec = componentSpecFromYaml(yamlContent);
 
     // Validate the component spec has the required structure
     if (

src/utils/componentStore.ts

@@ -6,9 +6,9 @@ import { fetchComponentTextFromUrl } from "@/services/componentService";
 import type { DownloadDataType } from "./cache";
 import { downloadDataWithCache } from "./cache";
 import type { ComponentReference, ComponentSpec } from "./componentSpec";
-import { isValidComponentSpec } from "./componentSpec";
 import { USER_COMPONENTS_LIST_NAME } from "./constants";
 import { getIdOrTitleFromPath } from "./URL";
+import { componentSpecFromYaml } from "./yaml";
 
 // IndexedDB: DB and table names
 const DB_NAME = "components";
@@ -74,16 +74,7 @@ export const loadComponentAsRefFromText = async (
       ? new TextEncoder().encode(componentText)
       : componentText;
 
-  const loadedObj = yaml.load(componentString);
-  if (typeof loadedObj !== "object" || loadedObj === null) {
-    throw Error(`componentText is not a YAML-encoded object: ${loadedObj}`);
-  }
-  if (!isValidComponentSpec(loadedObj)) {
-    throw Error(
-      `componentText does not encode a valid pipeline component: ${loadedObj}`,
-    );
-  }
-  const componentSpec: ComponentSpec = loadedObj;
+  const componentSpec = componentSpecFromYaml(componentString);
 
   const digest = await generateDigest(componentBytes as ArrayBuffer);
   const componentRef: ComponentReferenceWithSpec = {

src/utils/submitPipeline.ts

@@ -1,5 +1,3 @@
-import yaml from "js-yaml";
-
 import type { BodyCreateApiPipelineRunsPost } from "@/api/types.gen";
 import { getArgumentsFromInputs } from "@/components/shared/ReactFlow/FlowCanvas/utils/getArgumentsFromInputs";
 import {
@@ -9,6 +7,7 @@ import {
 import type { PipelineRun } from "@/types/pipelineRun";
 
 import type { ComponentReference, ComponentSpec } from "./componentSpec";
+import { componentSpecFromYaml } from "./yaml";
 
 export async function submitPipelineRun(
   componentSpec: ComponentSpec,
@@ -159,13 +158,7 @@ const parseComponentYaml = (text: string): ComponentSpec => {
     throw new Error("Received empty component specification");
   }
 
-  const loadedSpec = yaml.load(text) as ComponentSpec;
-
-  if (!loadedSpec || typeof loadedSpec !== "object") {
-    throw new Error("Invalid component specification format");
-  }
-
-  return loadedSpec;
+  return componentSpecFromYaml(text);
 };
 
 // Fetch component with timeout to avoid hanging on unresponsive URLs