#1480 Output key validation

src/devTools/editor/hooks/usePipelineField.ts

@@ -21,70 +21,49 @@ import { useCallback } from "react";
 import { BlockPipeline } from "@/blocks/types";
 import { useField, useFormikContext, setNestedObjectValues } from "formik";
 import { TraceError } from "@/telemetry/trace";
-import { isInputValidationError } from "@/blocks/errors";
-import { OutputUnit } from "@cfworker/json-schema";
 import { useAsyncEffect } from "use-async-effect";
-import { joinName } from "@/utils";
-import { set } from "lodash";
+import validateOutputKey from "@/devTools/editor/validation/validateOutputKey";
+import applyTraceError from "@/devTools/editor/validation/applyTraceError";
+import { isEmpty } from "lodash";
+import { BlocksMap } from "@/devTools/editor/tabs/editTab/editTabTypes";
 
-const REQUIRED_FIELD_REGEX = /^Instance does not have required property "(?<property>.+)"\.$/;
+/*
+ * PipelineErrors is Formik error... thing.
+ * It can be a string, a record of strings, or a record of records... i.e. it is dynamic and depends on the level of the state tree where the error happens.
+ * Speaking about `PipelineErrors[0]`, the error state normally is not an array, but since the pipeline is an array we use numbers (index) to get the error related to a block.
+ * Despite it looks like an array (the top-level may look like an array - have numbers for property names), it is an object.
+ * For instance, it doesn't have a `length` property.
+ */
+export type PipelineErrors = string | Record<string | number, unknown>;
+
+const pipelineBlocksFieldName = "extension.blockPipeline";
 
-function usePipelineField(): {
+function usePipelineField(
+  allBlocks: BlocksMap
+): {
   blockPipeline: BlockPipeline;
-  blockPipelineErrors: unknown[];
-  setBlockPipeline: (value: BlockPipeline, shouldValidate: boolean) => void;
-  traceError: TraceError;
+  blockPipelineErrors: PipelineErrors;
+  errorTraceEntry: TraceError;
 } {
-  const traceError = useSelector(selectTraceError);
+  const errorTraceEntry = useSelector(selectTraceError);
 
   const validatePipelineBlocks = useCallback(
-    (pipeline: BlockPipeline) => {
-      if (!traceError) {
-        return;
-      }
+    (pipeline: BlockPipeline): void | PipelineErrors => {
+      const formikErrors: Record<string, unknown> = {};
 
-      const { error, blockInstanceId } = traceError;
-      const blockIndex = pipeline.findIndex(
-        (block) => block.instanceId === blockInstanceId
-      );
-      if (blockIndex === -1) {
-        return;
-      }
-
-      const errors: Record<string, unknown> = {};
-      if (isInputValidationError(error)) {
-        for (const unit of (error.errors as unknown) as OutputUnit[]) {
-          let propertyNameInPipeline: string;
-          let errorMessage: string;
-
-          const property = REQUIRED_FIELD_REGEX.exec(unit.error)?.groups
-            .property;
-          if (property) {
-            propertyNameInPipeline = joinName(
-              String(blockIndex),
-              "config",
-              property
-            );
-            errorMessage = "Error from the last run: This field is required";
-          } else {
-            propertyNameInPipeline = String(blockIndex);
-            errorMessage = error.message;
-          }
-
-          set(errors, propertyNameInPipeline, errorMessage);
-        }
-      } else {
-        // eslint-disable-next-line security/detect-object-injection
-        errors[blockIndex] = error.message;
-      }
+      validateOutputKey(formikErrors, pipeline, allBlocks);
+      applyTraceError(formikErrors, errorTraceEntry, pipeline);
 
-      return errors;
+      return isEmpty(formikErrors) ? undefined : formikErrors;
     },
-    [traceError]
+    [errorTraceEntry, allBlocks]
   );
 
-  const formikField = useField<BlockPipeline>({
-    name: "extension.blockPipeline",
+  const [
+    { value: blockPipeline },
+    { error: blockPipelineErrors },
+  ] = useField<BlockPipeline>({
+    name: pipelineBlocksFieldName,
     // @ts-expect-error working with nested errors
     validate: validatePipelineBlocks,
   });
@@ -101,14 +80,13 @@ function usePipelineField(): {
         formikContext.setTouched(setNestedObjectValues(validationErrors, true));
       }
     },
-    [traceError]
+    [errorTraceEntry]
   );
 
   return {
-    blockPipeline: formikField[0].value,
-    blockPipelineErrors: (formikField[1].error as unknown) as unknown[],
-    setBlockPipeline: formikField[2].setValue,
-    traceError,
+    blockPipeline,
+    blockPipelineErrors,
+    errorTraceEntry,
   };
 }
 

src/devTools/editor/tabs/editTab/EditTab.tsx

@@ -28,9 +28,8 @@ import { ADAPTERS } from "@/devTools/editor/extensionPoints/adapter";
 import { BlockType, defaultBlockConfig, getType } from "@/blocks/util";
 import { useAsyncState } from "@/hooks/common";
 import blockRegistry from "@/blocks/registry";
-import { compact, zip } from "lodash";
+import { compact, isEmpty } from "lodash";
 import { IBlock, OutputKey, UUID } from "@/core";
-import hash from "object-hash";
 import { produce } from "immer";
 import EditorNodeConfigPanel from "@/devTools/editor/tabs/editTab/editorNodeConfigPanel/EditorNodeConfigPanel";
 import styles from "./EditTab.module.scss";
@@ -49,17 +48,7 @@ import useExtensionTrace from "@/devTools/editor/hooks/useExtensionTrace";
 import FoundationDataPanel from "@/devTools/editor/tabs/editTab/dataPanel/FoundationDataPanel";
 import { produceExcludeUnusedDependencies } from "@/components/fields/schemaFields/ServiceField";
 import usePipelineField from "@/devTools/editor/hooks/usePipelineField";
-
-async function filterBlocks(
-  blocks: IBlock[],
-  { excludeTypes = [] }: { excludeTypes: BlockType[] }
-): Promise<IBlock[]> {
-  const types = await Promise.all(blocks.map(async (block) => getType(block)));
-  // Exclude null to exclude foundations
-  return zip(blocks, types)
-    .filter(([, type]) => type != null && !excludeTypes.includes(type))
-    .map(([block]) => block);
-}
+import { BlocksMap } from "./editTabTypes";
 
 const blockConfigTheme: ThemeProps = {
   layout: "horizontal",
@@ -69,7 +58,6 @@ const EditTab: React.FC<{
   eventKey: string;
 }> = ({ eventKey }) => {
   useExtensionTrace();
-  // ToDo Figure out how to properly bind field validation errors to Formik state // useRuntimeErrors(pipelineFieldName);
 
   const { values, setValues: setFormValues } = useFormikContext<FormState>();
   const { extensionPoint, type: elementType } = values;
@@ -82,7 +70,30 @@ const EditTab: React.FC<{
 
   const { label, icon, EditorNode: FoundationNode } = ADAPTERS.get(elementType);
 
-  const { blockPipeline, blockPipelineErrors, traceError } = usePipelineField();
+  // Load once
+  const [allBlocks] = useAsyncState<BlocksMap>(
+    async () => {
+      const blocksMap: BlocksMap = {};
+      const blocks = await blockRegistry.all();
+      for (const block of blocks) {
+        blocksMap[block.id] = {
+          block,
+          // eslint-disable-next-line no-await-in-loop
+          type: await getType(block),
+        };
+      }
+
+      return blocksMap;
+    },
+    [],
+    {}
+  );
+
+  const {
+    blockPipeline,
+    blockPipelineErrors,
+    errorTraceEntry,
+  } = usePipelineField(allBlocks);
 
   const [activeNodeId, setActiveNodeId] = useState<NodeId>(FOUNDATION_NODE_ID);
   const activeBlockIndex = useMemo(() => {
@@ -100,35 +111,22 @@ const EditTab: React.FC<{
     [activeBlockIndex]
   );
 
-  // Load once
-  const [allBlocks] = useAsyncState(async () => blockRegistry.all(), [], []);
-
-  const pipelineIdHash = hash(blockPipeline.map((x) => x.id));
-  const resolvedBlocks = useMemo(
-    () =>
-      blockPipeline.map(({ id }) =>
-        (allBlocks ?? []).find((block) => block.id === id)
-      ),
-    // eslint-disable-next-line react-hooks/exhaustive-deps -- using actionsHash since we only use the actions ids
-    [allBlocks, pipelineIdHash]
-  );
-
   const [showAppendNode] = useAsyncState(
     async () => {
-      if (!resolvedBlocks || resolvedBlocks.length === 0) {
+      if (isEmpty(allBlocks)) {
         return true;
       }
 
       const lastId = blockPipeline[blockPipeline.length - 1].id;
-      const lastResolved = resolvedBlocks.find((block) => block.id === lastId);
-      if (!lastResolved) {
+      // eslint-disable-next-line security/detect-object-injection
+      const lastBlock = allBlocks[lastId];
+      if (!lastBlock?.block) {
         return true;
       }
 
-      const lastType = await getType(lastResolved);
-      return lastType !== "renderer";
+      return lastBlock.type !== "renderer";
     },
-    [resolvedBlocks, blockPipeline],
+    [allBlocks, blockPipeline],
     false
   );
 
@@ -160,8 +158,7 @@ const EditTab: React.FC<{
 
   const blockNodes: LayoutNodeProps[] = blockPipeline.map(
     (blockConfig, index) => {
-      // eslint-disable-next-line security/detect-object-injection
-      const block = resolvedBlocks[index];
+      const block = allBlocks[blockConfig.id]?.block;
       const nodeId = blockConfig.instanceId;
       return block
         ? {
@@ -181,9 +178,13 @@ const EditTab: React.FC<{
                 faIconClass={styles.brickFaIcon}
               />
             ),
-            // eslint-disable-next-line security/detect-object-injection -- uuid
-            hasError: Boolean(blockPipelineErrors?.[index]),
-            hasWarning: traceError?.blockInstanceId === blockConfig.instanceId,
+            hasError:
+              // If blockPipelineErrors is a string, it means the error is on the pipeline level
+              typeof blockPipelineErrors !== "string" &&
+              // eslint-disable-next-line security/detect-object-injection
+              Boolean(blockPipelineErrors?.[index]),
+            hasWarning:
+              errorTraceEntry?.blockInstanceId === blockConfig.instanceId,
             onClick: () => {
               setActiveNodeId(blockConfig.instanceId);
             },
@@ -211,12 +212,15 @@ const EditTab: React.FC<{
   const nodes: LayoutNodeProps[] = [foundationNode, ...blockNodes];
 
   const [relevantBlocksToAdd] = useAsyncState(async () => {
-    const excludeTypes: BlockType[] = ["actionPanel", "panel"].includes(
+    const excludeType: BlockType = ["actionPanel", "panel"].includes(
       elementType
     )
-      ? ["effect"]
-      : ["renderer"];
-    return filterBlocks(allBlocks, { excludeTypes });
+      ? "effect"
+      : "renderer";
+
+    return Object.values(allBlocks)
+      .filter(({ type }) => type != null && type !== excludeType)
+      .map(({ block }) => block);
   }, [allBlocks, elementType]);
 
   const addBlock = useCallback(

src/devTools/editor/tabs/editTab/editTabTypes.ts

@@ -0,0 +1,10 @@
+import { BlockType } from "@/blocks/util";
+import { IBlock, RegistryId } from "@/core";
+
+export type BlocksMap = Record<
+  RegistryId,
+  {
+    block: IBlock;
+    type: BlockType;
+  }
+>;

src/devTools/editor/validation/applyTraceBlockError.test.ts

@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2021 PixieBrix, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import { traceErrorFactory } from "@/tests/factories";
+import applyTraceBlockError from "./applyTraceBlockError";
+
+test("sets block error", () => {
+  const pipelineErrors: Record<string, unknown> = {};
+  const errorTraceEntry = traceErrorFactory();
+
+  applyTraceBlockError(pipelineErrors, errorTraceEntry, 0);
+
+  expect(pipelineErrors[0]).toBe(errorTraceEntry.error.message);
+});
+
+test("doesn't override nested error", () => {
+  const errorTraceEntry = traceErrorFactory();
+  const blockIndex = 5;
+
+  const nestedBlockError = {
+    config: {
+      name: "Nested Error",
+    },
+  };
+  const pipelineErrors = {
+    [blockIndex]: nestedBlockError,
+  };
+
+  applyTraceBlockError(pipelineErrors, errorTraceEntry, blockIndex);
+
+  // eslint-disable-next-line security/detect-object-injection
+  expect(pipelineErrors[blockIndex]).toBe(nestedBlockError);
+});

src/devTools/editor/validation/applyTraceBlockError.ts

@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2021 PixieBrix, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import { TraceError } from "@/telemetry/trace";
+
+function applyTraceBlockError(
+  pipelineErrors: Record<string, unknown>,
+  errorTraceEntry: TraceError,
+  blockIndex: number
+) {
+  // eslint-disable-next-line security/detect-object-injection
+  if (!pipelineErrors[blockIndex]) {
+    // eslint-disable-next-line security/detect-object-injection
+    pipelineErrors[blockIndex] = errorTraceEntry.error.message;
+  }
+}
+
+export default applyTraceBlockError;