Allow arbitrary JSON in google.protobuf.Struct fields during validation

common/protos/BUILD

@@ -10,6 +10,7 @@ ts_library(
     deps = [
         "//:modules-fix",
         "//common/strings",
+        "//protos:ts",
         "@npm//protobufjs",
     ],
 )

common/protos/index.ts

@@ -1,9 +1,13 @@
 import { util } from "protobufjs";
 
+import { google } from "df/protos/ts";
+
 const CONFIGS_PROTO_DOCUMENTATION_URL =
   "https://dataform-co.github.io/dataform/docs/configs-reference";
 const REPORT_ISSUE_URL = "https://github.com/dataform-co/dataform/issues";
 
+
+
 export interface IProtoClass<IProto, Proto> {
   new (): Proto;
 
@@ -41,54 +45,115 @@ export function verifyObjectMatchesProto<Proto>(
     throw ReferenceError(`Expected a top-level object, but found an array`);
   }
 
-  // Calling toObject on the object/JSON creates a version only contains the valid proto fields.
+  // 1. Single Pass Create
   const proto = protoType.create(object);
-  const protoCastObject = protoType.toObject(proto);
-
-  function checkFields(present: { [k: string]: any }, desired: { [k: string]: any }) {
-    // Only the entries of `present` need to be iterated through as `desired` is guaranteed to be a
-    // strict subset of `present`.
-    Object.entries(present).forEach(([presentKey, presentValue]) => {
-      const desiredValue = desired[presentKey];
-      if (typeof desiredValue !== typeof presentValue) {
-        if (Array.isArray(presentValue) && presentValue.length === 0) {
-          // Empty arrays are assigned to empty proto array fields by ProtobufJS.
-          return;
-        }
-        if (!presentValue) {
-          throw ReferenceError(
-            `Unexpected empty value for "${presentKey}".` +
-              maybeGetDocsLinkPrefix(errorBehaviour, protoType)
-          );
-        }
-        if (typeof presentValue === "object" && Object.keys(presentValue).length === 0) {
-          // Empty objects are assigned to empty object fields by ProtobufJS.
-          return;
-        }
-        if (errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) {
-          throw ReferenceError(
-            `Unexpected property "${presentKey}" for "${protoType
-              .getTypeUrl("")
-              .replace("/", "")}", please report this to the Dataform team at ` +
-              `${REPORT_ISSUE_URL}.`
-          );
+  const probeObject = (protoType as any).toObject(proto, { defaults: true });
+
+  // 2. Validate and Convert In-Place
+  checkAndConvertFields(object, probeObject, proto, errorBehaviour, protoType);
+
+  return proto;
+}
+
+function checkAndConvertFields(
+  raw: { [k: string]: any },
+  probe: { [k: string]: any },
+  protoInstance: any,
+  errorBehaviour: VerifyProtoErrorBehaviour,
+  protoType: any
+) {
+  const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType);
+  Object.entries(raw).forEach(([rawKey, rawValue]) => {
+    if (rawValue === undefined) {
+      return;
+    }
+    if (
+      rawValue === null &&
+      errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM
+    ) {
+      return;
+    }
+
+    let probeKey = rawKey;
+    if (probe[rawKey] === undefined) {
+      if (probe[toSnakeCase(rawKey)] !== undefined) {
+        probeKey = toSnakeCase(rawKey);
+      } else if (probe[toCamelCase(rawKey)] !== undefined) {
+        probeKey = toCamelCase(rawKey);
+      }
+    }
+    const probeValue = probe[probeKey];
+
+    if (
+      Array.isArray(probeValue) &&
+      rawValue === null &&
+      errorBehaviour === VerifyProtoErrorBehaviour.SHOW_DOCS_LINK
+    ) {
+      throw ReferenceError(`Unexpected empty value for "${rawKey}".${docLinkPrefix}`);
+    }
+
+    // Heuristic 1: Object Struct Detection
+    if (
+      typeof rawValue === "object" &&
+      !Array.isArray(rawValue) &&
+      probeValue &&
+      typeof probeValue === "object" &&
+      probeValue.fields &&
+      typeof probeValue.fields === "object" &&
+      Object.keys(probeValue.fields).length === 0 &&
+      !rawValue.fields
+    ) {
+      protoInstance[probeKey] = unknownToValue(rawValue).structValue;
+      return;
+    }
+
+    // Heuristic 2: Array List/Struct Detection
+    if (
+      Array.isArray(rawValue) &&
+      rawValue.length > 0 &&
+      probeValue &&
+      Array.isArray(probeValue) &&
+      probeValue.length === 0
+    ) {
+      protoInstance[probeKey] = {
+        listValue: {
+          values: rawValue.map(item => unknownToValue(item))
         }
+      };
+      return;
+    }
+
+    if (typeof probeValue !== typeof rawValue) {
+      if (Array.isArray(rawValue) && rawValue.length === 0) {
+        return;
+      }
+      if (!rawValue) {
         throw ReferenceError(
-          `Unexpected property "${presentKey}", or property value type of ` +
-            `"${typeof presentValue}" is incorrect.` +
-            maybeGetDocsLinkPrefix(errorBehaviour, protoType)
+          `Unexpected empty value for "${rawKey}".${docLinkPrefix}`
         );
       }
-      if (typeof presentValue === "object") {
-        checkFields(presentValue, desiredValue);
+      if (typeof rawValue === "object" && Object.keys(rawValue).length === 0) {
+        return;
       }
-    });
-  }
-
-  checkFields(object, protoCastObject);
-  return proto;
+      if (errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) {
+        throw ReferenceError(
+          `Unexpected property "${rawKey}" for "${protoType
+            .getTypeUrl("")
+            .replace("/", "")}", please report this to the Dataform team at ${REPORT_ISSUE_URL}.`
+        );
+      }
+      throw ReferenceError(
+        `Unexpected property "${rawKey}", or property value type of "${typeof rawValue}" is incorrect.${docLinkPrefix}`
+      );
+    }
+
+    if (typeof rawValue === "object" && rawValue !== null) {
+      checkAndConvertFields(rawValue, probeValue, protoInstance[probeKey], errorBehaviour, protoType);
+    }
+  });
 }
 
+
 function maybeGetDocsLinkPrefix<Proto>(
   errorBehaviour: VerifyProtoErrorBehaviour,
   protoType: IProtoClass<any, Proto>
@@ -140,3 +205,41 @@ function fromBase64(value: string): Uint8Array {
   util.base64.decode(value, buf, 0);
   return buf;
 }
+
+export function unknownToValue(raw: unknown): google.protobuf.IValue {
+  if (raw === null || typeof raw === "undefined") {
+    return { nullValue: 0 };
+  }
+  if (typeof raw === "string") {
+    return { stringValue: raw };
+  }
+  if (typeof raw === "number") {
+    return { numberValue: raw };
+  }
+  if (typeof raw === "boolean") {
+    return { boolValue: raw };
+  }
+  if (Array.isArray(raw)) {
+    return { listValue: { values: raw.map(unknownToValue) } };
+  }
+  if (typeof raw === "object") {
+    return {
+      structValue: {
+        fields: Object.fromEntries(
+          Object.entries(raw as object).map(([key, value]) => [key, unknownToValue(value)])
+        )
+      }
+    };
+  }
+  throw new Error(`Unsupported value: ${raw}`);
+}
+
+
+
+function toSnakeCase(str: string): string {
+  return str.replace(/([a-z0-9])([A-Z])/g, "$1_$2").toLowerCase();
+}
+
+function toCamelCase(str: string): string {
+  return str.replace(/_([a-z])/g, (_, char) => char.toUpperCase());
+}

core/actions/table_test.ts

@@ -191,6 +191,120 @@ SELECT 1`
         );
       });
     });
+
+    test("tables can be configured with a plain object for extraProperties", () => {
+      const projectDir = tmpDirFixture.createNewTmpDir();
+      fs.writeFileSync(
+        path.join(projectDir, "workflow_settings.yaml"),
+        VALID_WORKFLOW_SETTINGS_YAML
+      );
+      fs.mkdirSync(path.join(projectDir, "definitions"));
+      fs.writeFileSync(
+        path.join(projectDir, "definitions/table.sqlx"),
+        `config {
+    type: "table",
+    metadata: {
+        extraProperties: {
+            priority: "high"
+        }
+    }
+}
+SELECT 1`
+      );
+
+      const result = runMainInVm(coreExecutionRequestFromPath(projectDir));
+
+      expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]);
+      expect(
+        asPlainObject(result.compile.compiledGraph.tables[0].actionDescriptor.metadata)
+      ).deep.equals({
+        extraProperties: {
+          fields: {
+            priority: { stringValue: "high" }
+          }
+        }
+      });
+    });
+
+    test("tables can be configured with a complex nested plain object for extraProperties", () => {
+      const projectDir = tmpDirFixture.createNewTmpDir();
+      fs.writeFileSync(
+        path.join(projectDir, "workflow_settings.yaml"),
+        VALID_WORKFLOW_SETTINGS_YAML
+      );
+      fs.mkdirSync(path.join(projectDir, "definitions"));
+      fs.writeFileSync(
+        path.join(projectDir, "definitions/table.sqlx"),
+        `config {
+      type: "table",
+      metadata: {
+        overview: "The test overview",
+        extraProperties: {
+          glossary_terms: [
+            {
+              column_name: "trip_id",
+              project: "project_identifier",
+              location: "us-central1"
+            },
+            {
+              project: "project_identifier",
+              glossary_id: "jebmjilij-9c85ee94"
+            }
+          ],
+          generic: {
+            system: "my custom system value",
+            type: "my custom type value"
+          }
+        }
+      }
+    }
+    SELECT 1`
+      );
+
+      const result = runMainInVm(coreExecutionRequestFromPath(projectDir));
+
+      expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]);
+      const metadata = result.compile.compiledGraph.tables[0].actionDescriptor.metadata;
+      expect(metadata.overview).equals("The test overview");
+
+      expect(asPlainObject(metadata.extraProperties)).deep.equals({
+        fields: {
+          glossary_terms: {
+            listValue: {
+              values: [
+                {
+                  structValue: {
+                    fields: {
+                      column_name: { stringValue: "trip_id" },
+                      project: { stringValue: "project_identifier" },
+                      location: { stringValue: "us-central1" }
+                    }
+                  }
+                },
+                {
+                  structValue: {
+                    fields: {
+                      project: { stringValue: "project_identifier" },
+                      glossary_id: { stringValue: "jebmjilij-9c85ee94" }
+                    }
+                  }
+                }
+              ]
+            }
+          },
+          generic: {
+            structValue: {
+              fields: {
+                system: { stringValue: "my custom system value" },
+                type: { stringValue: "my custom type value" }
+              }
+            }
+          }
+        }
+      });
+    });
+
+
   });
 
   test("action config options", () => {

core/main_test.ts

@@ -1702,7 +1702,7 @@ dataform.jitData("key", {test: () => {}});
 
         expect(
           result.compile.compiledGraph.graphErrors.compilationErrors.map(e => e.message)
-        ).to.deep.equal(["Unsupported context object: () => {}"]);
+        ).to.deep.equal(["Unsupported value: () => {}"]);
       });
     });
 

core/session.ts

@@ -1,6 +1,6 @@
 import { default as TarjanGraphConstructor, Graph as TarjanGraph } from "tarjan-graph";
 
-import { encode64, verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos";
+import { encode64, unknownToValue, verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos";
 import { Action, ActionProto, ILegacyTableConfig, TableType } from "df/core/actions";
 import { AContextable, Assertion, AssertionContext } from "df/core/actions/assertion";
 import {
@@ -413,40 +413,6 @@ export class Session {
   }
 
   public jitData(key: string, data: unknown): void {
-    function unknownToValue(raw: unknown): google.protobuf.Value {
-      if (raw === null || typeof raw === "undefined") {
-        return google.protobuf.Value.create({ nullValue: google.protobuf.NullValue.NULL_VALUE });
-      }
-      if (typeof raw === "string") {
-        return google.protobuf.Value.create({ stringValue: raw as string });
-      }
-      if (typeof raw === "number") {
-        return google.protobuf.Value.create({ numberValue: raw as number });
-      }
-      if (typeof raw === "boolean") {
-        return google.protobuf.Value.create({ boolValue: raw as boolean });
-      }
-      if (typeof raw === "object" && raw instanceof Array) {
-        return google.protobuf.Value.create({
-          listValue: google.protobuf.ListValue.create({
-            values: (raw as unknown[]).map(unknownToValue)
-          })
-        });
-      }
-      if (typeof raw === "object") {
-        return google.protobuf.Value.create({
-          structValue: google.protobuf.Struct.create({
-            fields: Object.fromEntries(Object.entries(raw).map(
-              ([fieldKey, fieldValue]) => ([
-                fieldKey,
-                unknownToValue(fieldValue)
-              ])
-            ))
-          })
-        })
-      }
-      throw new Error(`Unsupported context object: ${raw}`);
-    }
 
     if (this.jitContextData.fields[key] !== undefined) {
       throw new Error(`JiT context data with key ${key} already exists.`);