From bd86e71732b03768644135aa1de7933ad4dd67d9 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Thu, 19 Mar 2026 21:28:42 +0000 Subject: [PATCH 01/41] Allow arbitrary JSON in google.protobuf.Struct fields during validation --- common/protos/index.ts | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 5f805b1d1..2990f4fed 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -41,8 +41,13 @@ export function verifyObjectMatchesProto( throw ReferenceError(`Expected a top-level object, but found an array`); } + let objectToVerify = object; + if (protoType.getTypeUrl("").endsWith("google.protobuf.Struct")) { + objectToVerify = jsonToStruct(object); + } + // Calling toObject on the object/JSON creates a version only contains the valid proto fields. - const proto = protoType.create(object); + const proto = protoType.create(objectToVerify); const protoCastObject = protoType.toObject(proto); function checkFields(present: { [k: string]: any }, desired: { [k: string]: any }) { @@ -140,3 +145,32 @@ function fromBase64(value: string): Uint8Array { util.base64.decode(value, buf, 0); return buf; } + +function jsonToStruct(obj: any): any { + if (obj === null || typeof obj !== "object" || Array.isArray(obj)) { + throw new Error(`Expected a JSON object for Struct, but found ${typeof obj}`); + } + const fields: { [key: string]: any } = {}; + for (const [key, value] of Object.entries(obj)) { + fields[key] = jsonToValue(value); + } + return { fields }; +} + +function jsonToValue(value: any): any { + if (value === null) { + return { nullValue: 0 }; + } else if (typeof value === "number") { + return { numberValue: value }; + } else if (typeof value === "string") { + return { stringValue: value }; + } else if (typeof value === "boolean") { + return { boolValue: value }; + } else if (Array.isArray(value)) { + return { listValue: { values: value.map(jsonToValue) } }; + } else if (typeof value === "object") { + return { structValue: jsonToStruct(value) }; + } else { + throw new Error(`Unsupported JSON value type: ${typeof value}`); + } +} From 937e221944b6d3d303c2415803c6fd20c301a700 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Fri, 20 Mar 2026 20:54:22 +0000 Subject: [PATCH 02/41] Add compilation test cases. Reuse the same logic in index.ts and session.ts --- common/protos/index.ts | 6 +++--- core/actions/table.ts | 7 +++++-- core/actions/table_test.ts | 26 +++++++++++++++++++++++++ core/main_test.ts | 2 +- core/session.ts | 39 ++------------------------------------ 5 files changed, 37 insertions(+), 43 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 2990f4fed..bbb97983c 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -146,7 +146,7 @@ function fromBase64(value: string): Uint8Array { return buf; } -function jsonToStruct(obj: any): any { +export function jsonToStruct(obj: any): any { if (obj === null || typeof obj !== "object" || Array.isArray(obj)) { throw new Error(`Expected a JSON object for Struct, but found ${typeof obj}`); } @@ -157,8 +157,8 @@ function jsonToStruct(obj: any): any { return { fields }; } -function jsonToValue(value: any): any { - if (value === null) { +export function jsonToValue(value: any): any { + if (value === null || typeof value === "undefined") { return { nullValue: 0 }; } else if (typeof value === "number") { return { numberValue: value }; diff --git a/core/actions/table.ts b/core/actions/table.ts index ee1562527..98d90af48 100644 --- a/core/actions/table.ts +++ b/core/actions/table.ts @@ -1,4 +1,4 @@ -import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos"; +import { jsonToStruct, verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos"; import { ActionBuilder, checkConfigAdditionalOptionsOverlap, @@ -133,7 +133,10 @@ export class Table extends ActionBuilder { if (!unverifiedConfig) { return; } - const config = this.verifyConfig(unverifiedConfig); + if (this.unverifiedConfig.metadata?.extraProperties) { + this.unverifiedConfig.metadata.extraProperties = jsonToStruct(this.unverifiedConfig.metadata.extraProperties); + } + const config = this.verifyConfig(this.unverifiedConfig); if (!config.name) { config.name = Path.basename(config.filename); diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index 701fd9deb..e71f2dda7 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -193,6 +193,32 @@ SELECT 1` }); }); + test("plain JSON in extraProperties works", () => { + 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.js"), + `publish("name", { + type: "table", + metadata: { + extraProperties: { + priority: "high" + } + } + }).query("SELECT 1")` + ); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(asPlainObject(result.compile.compiledGraph.tables[0].actionDescriptor.metadata.extraProperties)).deep.equals({ + fields: { + priority: { stringValue: "high" } + } + }); + }); + test("action config options", () => { const projectDir = tmpDirFixture.createNewTmpDir(); fs.writeFileSync(path.join(projectDir, "workflow_settings.yaml"), VALID_WORKFLOW_SETTINGS_YAML); diff --git a/core/main_test.ts b/core/main_test.ts index 82d5c8693..3aa60db7a 100644 --- a/core/main_test.ts +++ b/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 JSON value type: function"]); }); }); diff --git a/core/session.ts b/core/session.ts index 87eb0eed1..841dd2f36 100644 --- a/core/session.ts +++ b/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, jsonToValue, 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,46 +413,11 @@ 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.`); } - this.jitContextData.fields[key] = unknownToValue(data); + this.jitContextData.fields[key] = google.protobuf.Value.create(jsonToValue(data)); } public compileError(err: Error | string, path?: string, actionTarget?: dataform.ITarget) { From 85bb25458bdc6f0bf4d067d8e24633039f48cd6c Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 23 Mar 2026 22:27:53 +0000 Subject: [PATCH 03/41] Revert "Add compilation test cases. Reuse the same logic in index.ts and session.ts" This reverts commit 686023060eafe3f7311adc709bc2ff70297c44d4. --- common/protos/index.ts | 6 +++--- core/actions/table.ts | 7 ++----- core/actions/table_test.ts | 26 ------------------------- core/main_test.ts | 2 +- core/session.ts | 39 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 43 insertions(+), 37 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index bbb97983c..2990f4fed 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -146,7 +146,7 @@ function fromBase64(value: string): Uint8Array { return buf; } -export function jsonToStruct(obj: any): any { +function jsonToStruct(obj: any): any { if (obj === null || typeof obj !== "object" || Array.isArray(obj)) { throw new Error(`Expected a JSON object for Struct, but found ${typeof obj}`); } @@ -157,8 +157,8 @@ export function jsonToStruct(obj: any): any { return { fields }; } -export function jsonToValue(value: any): any { - if (value === null || typeof value === "undefined") { +function jsonToValue(value: any): any { + if (value === null) { return { nullValue: 0 }; } else if (typeof value === "number") { return { numberValue: value }; diff --git a/core/actions/table.ts b/core/actions/table.ts index 98d90af48..ee1562527 100644 --- a/core/actions/table.ts +++ b/core/actions/table.ts @@ -1,4 +1,4 @@ -import { jsonToStruct, verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos"; +import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos"; import { ActionBuilder, checkConfigAdditionalOptionsOverlap, @@ -133,10 +133,7 @@ export class Table extends ActionBuilder { if (!unverifiedConfig) { return; } - if (this.unverifiedConfig.metadata?.extraProperties) { - this.unverifiedConfig.metadata.extraProperties = jsonToStruct(this.unverifiedConfig.metadata.extraProperties); - } - const config = this.verifyConfig(this.unverifiedConfig); + const config = this.verifyConfig(unverifiedConfig); if (!config.name) { config.name = Path.basename(config.filename); diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index e71f2dda7..701fd9deb 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -193,32 +193,6 @@ SELECT 1` }); }); - test("plain JSON in extraProperties works", () => { - 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.js"), - `publish("name", { - type: "table", - metadata: { - extraProperties: { - priority: "high" - } - } - }).query("SELECT 1")` - ); - - const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); - - expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); - expect(asPlainObject(result.compile.compiledGraph.tables[0].actionDescriptor.metadata.extraProperties)).deep.equals({ - fields: { - priority: { stringValue: "high" } - } - }); - }); - test("action config options", () => { const projectDir = tmpDirFixture.createNewTmpDir(); fs.writeFileSync(path.join(projectDir, "workflow_settings.yaml"), VALID_WORKFLOW_SETTINGS_YAML); diff --git a/core/main_test.ts b/core/main_test.ts index 3aa60db7a..82d5c8693 100644 --- a/core/main_test.ts +++ b/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 JSON value type: function"]); + ).to.deep.equal(["Unsupported context object: () => {}"]); }); }); diff --git a/core/session.ts b/core/session.ts index 841dd2f36..87eb0eed1 100644 --- a/core/session.ts +++ b/core/session.ts @@ -1,6 +1,6 @@ import { default as TarjanGraphConstructor, Graph as TarjanGraph } from "tarjan-graph"; -import { encode64, jsonToValue, verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos"; +import { encode64, 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,11 +413,46 @@ 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.`); } - this.jitContextData.fields[key] = google.protobuf.Value.create(jsonToValue(data)); + this.jitContextData.fields[key] = unknownToValue(data); } public compileError(err: Error | string, path?: string, actionTarget?: dataform.ITarget) { From d840850e566238913b72f77769f93552a657a3b4 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 23 Mar 2026 22:50:47 +0000 Subject: [PATCH 04/41] move the unknownToValue logic from session.ts to index.ts and reuse the same logic --- common/protos/index.ts | 53 ++++++++++++++++++++++-------------------- core/session.ts | 36 +--------------------------- 2 files changed, 29 insertions(+), 60 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 2990f4fed..d600ad7f8 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -43,7 +43,11 @@ export function verifyObjectMatchesProto( let objectToVerify = object; if (protoType.getTypeUrl("").endsWith("google.protobuf.Struct")) { - objectToVerify = jsonToStruct(object); + const converted = unknownToValue(object); + if (!converted.structValue) { + throw new Error(`Expected a JSON object for Struct, but found ${typeof object}`); + } + objectToVerify = converted.structValue; } // Calling toObject on the object/JSON creates a version only contains the valid proto fields. @@ -146,31 +150,30 @@ function fromBase64(value: string): Uint8Array { return buf; } -function jsonToStruct(obj: any): any { - if (obj === null || typeof obj !== "object" || Array.isArray(obj)) { - throw new Error(`Expected a JSON object for Struct, but found ${typeof obj}`); +export function unknownToValue(raw: unknown): any { + if (raw === null || typeof raw === "undefined") { + return { nullValue: 0 }; } - const fields: { [key: string]: any } = {}; - for (const [key, value] of Object.entries(obj)) { - fields[key] = jsonToValue(value); + if (typeof raw === "string") { + return { stringValue: raw }; } - return { fields }; -} - -function jsonToValue(value: any): any { - if (value === null) { - return { nullValue: 0 }; - } else if (typeof value === "number") { - return { numberValue: value }; - } else if (typeof value === "string") { - return { stringValue: value }; - } else if (typeof value === "boolean") { - return { boolValue: value }; - } else if (Array.isArray(value)) { - return { listValue: { values: value.map(jsonToValue) } }; - } else if (typeof value === "object") { - return { structValue: jsonToStruct(value) }; - } else { - throw new Error(`Unsupported JSON value type: ${typeof value}`); + 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}`); } diff --git a/core/session.ts b/core/session.ts index 87eb0eed1..b0d28ab0a 100644 --- a/core/session.ts +++ b/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.`); From 7e9e4c717a7c327a25e6d7e5c6053e047b508d2e Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 23 Mar 2026 23:02:50 +0000 Subject: [PATCH 05/41] Add compilation test. Add return type. --- common/protos/index.ts | 11 ++++++++++- core/actions/table_test.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index d600ad7f8..7fe464f21 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -150,7 +150,16 @@ function fromBase64(value: string): Uint8Array { return buf; } -export function unknownToValue(raw: unknown): any { +export interface IValue { + nullValue?: number; + stringValue?: string; + numberValue?: number; + boolValue?: boolean; + structValue?: { fields: { [key: string]: IValue } }; + listValue?: { values: IValue[] }; +} + +export function unknownToValue(raw: unknown): IValue { if (raw === null || typeof raw === "undefined") { return { nullValue: 0 }; } diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index 701fd9deb..128a140f9 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -193,6 +193,32 @@ SELECT 1` }); }); + test("plain JSON in metadata.extraProperties is converted to Struct", () => { + 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.js"), + `publish("name", { + type: "table", + metadata: { + extraProperties: { + priority: "high" + } + } + }).query(ctx => "SELECT 1")` + ); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(asPlainObject(result.compile.compiledGraph.tables[0].actionDescriptor.metadata.extraProperties)).deep.equals({ + fields: { + priority: { stringValue: "high" } + } + }); + }); + test("action config options", () => { const projectDir = tmpDirFixture.createNewTmpDir(); fs.writeFileSync(path.join(projectDir, "workflow_settings.yaml"), VALID_WORKFLOW_SETTINGS_YAML); From 670fa35df3c625be4bd0337fcefedd6ed346c3c0 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 00:39:53 +0000 Subject: [PATCH 06/41] fix the test error. Using google.protobuf.IValue instead of create the new interface. --- common/protos/BUILD | 1 + common/protos/index.ts | 12 ++---------- core/actions/table_test.ts | 26 -------------------------- core/main_test.ts | 2 +- 4 files changed, 4 insertions(+), 37 deletions(-) diff --git a/common/protos/BUILD b/common/protos/BUILD index 87efbbe1c..6f12d80ed 100644 --- a/common/protos/BUILD +++ b/common/protos/BUILD @@ -10,6 +10,7 @@ ts_library( deps = [ "//:modules-fix", "//common/strings", + "//protos:ts", "@npm//protobufjs", ], ) diff --git a/common/protos/index.ts b/common/protos/index.ts index 7fe464f21..aa160271d 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -1,4 +1,5 @@ import { util } from "protobufjs"; +import { google } from "df/protos/ts"; const CONFIGS_PROTO_DOCUMENTATION_URL = "https://dataform-co.github.io/dataform/docs/configs-reference"; @@ -150,16 +151,7 @@ function fromBase64(value: string): Uint8Array { return buf; } -export interface IValue { - nullValue?: number; - stringValue?: string; - numberValue?: number; - boolValue?: boolean; - structValue?: { fields: { [key: string]: IValue } }; - listValue?: { values: IValue[] }; -} - -export function unknownToValue(raw: unknown): IValue { +export function unknownToValue(raw: unknown): google.protobuf.IValue { if (raw === null || typeof raw === "undefined") { return { nullValue: 0 }; } diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index 128a140f9..701fd9deb 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -193,32 +193,6 @@ SELECT 1` }); }); - test("plain JSON in metadata.extraProperties is converted to Struct", () => { - 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.js"), - `publish("name", { - type: "table", - metadata: { - extraProperties: { - priority: "high" - } - } - }).query(ctx => "SELECT 1")` - ); - - const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); - - expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); - expect(asPlainObject(result.compile.compiledGraph.tables[0].actionDescriptor.metadata.extraProperties)).deep.equals({ - fields: { - priority: { stringValue: "high" } - } - }); - }); - test("action config options", () => { const projectDir = tmpDirFixture.createNewTmpDir(); fs.writeFileSync(path.join(projectDir, "workflow_settings.yaml"), VALID_WORKFLOW_SETTINGS_YAML); diff --git a/core/main_test.ts b/core/main_test.ts index 82d5c8693..1b53e0e91 100644 --- a/core/main_test.ts +++ b/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: () => {}"]); }); }); From 42dbe96731e7eb5637da7830b7e3b3af328770aa Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 00:44:42 +0000 Subject: [PATCH 07/41] change the import format --- common/protos/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/common/protos/index.ts b/common/protos/index.ts index aa160271d..2a6e67100 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -1,4 +1,5 @@ import { util } from "protobufjs"; + import { google } from "df/protos/ts"; const CONFIGS_PROTO_DOCUMENTATION_URL = From 4098a253f8dc61d854cf21c4602c8b0d26811318 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 01:59:36 +0000 Subject: [PATCH 08/41] add the compilation test --- common/protos/index.ts | 3 +++ core/actions/table_test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/common/protos/index.ts b/common/protos/index.ts index 2a6e67100..0c90f6d30 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -60,6 +60,9 @@ export function verifyObjectMatchesProto( // 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]) => { + if (presentKey === "extraProperties") { + return; + } const desiredValue = desired[presentKey]; if (typeof desiredValue !== typeof presentValue) { if (Array.isArray(presentValue) && presentValue.length === 0) { diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index 701fd9deb..40023b186 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -191,6 +191,40 @@ 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("action config options", () => { From ca9750330503c925157b85f5426575a45e02ad73 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 21:53:01 +0000 Subject: [PATCH 09/41] add recursive logic --- common/protos/index.ts | 46 +++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 0c90f6d30..6146db670 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -5,6 +5,8 @@ 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"; +const STRUCT_FIELD_NAMES = new Set(["extraProperties", "contexts", "glossary_terms"]); + export interface IProtoClass { new (): Proto; @@ -37,20 +39,14 @@ export enum VerifyProtoErrorBehaviour { export function verifyObjectMatchesProto( protoType: IProtoClass, object: object, - errorBehaviour: VerifyProtoErrorBehaviour = VerifyProtoErrorBehaviour.DEFAULT + errorBehaviour: VerifyProtoErrorBehaviour = VerifyProtoErrorBehaviour.DEFAULT, + structFieldNames: Set = STRUCT_FIELD_NAMES ): Proto { if (Array.isArray(object)) { throw ReferenceError(`Expected a top-level object, but found an array`); } - let objectToVerify = object; - if (protoType.getTypeUrl("").endsWith("google.protobuf.Struct")) { - const converted = unknownToValue(object); - if (!converted.structValue) { - throw new Error(`Expected a JSON object for Struct, but found ${typeof object}`); - } - objectToVerify = converted.structValue; - } + const objectToVerify = applyStructConversions(object); // Calling toObject on the object/JSON creates a version only contains the valid proto fields. const proto = protoType.create(objectToVerify); @@ -60,7 +56,7 @@ export function verifyObjectMatchesProto( // 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]) => { - if (presentKey === "extraProperties") { + if (structFieldNames.has(presentKey)) { return; } const desiredValue = desired[presentKey]; @@ -103,6 +99,36 @@ export function verifyObjectMatchesProto( return proto; } +function applyStructConversions(obj: any): any { + if (!obj || typeof obj !== "object" || Array.isArray(obj)) { + return obj; + } + + const result = { ...obj }; + + for (const key of Object.keys(obj)) { + const value = obj[key]; + if (value === undefined || value === null) continue; + + if (STRUCT_FIELD_NAMES.has(key)) { + if (Array.isArray(value)) { + result[key] = { + listValue: { + values: value.map(item => unknownToValue(item)) + } + }; + } else if (typeof value === "object" && !value.fields) { + const converted = unknownToValue(value); + result[key] = converted.structValue; + } + } else if (typeof value === "object") { + result[key] = applyStructConversions(value); + } + } + + return result; +} + function maybeGetDocsLinkPrefix( errorBehaviour: VerifyProtoErrorBehaviour, protoType: IProtoClass From da0c07171c37ec61e35d9c4e25a31a7e4515696c Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 22:00:44 +0000 Subject: [PATCH 10/41] add a complex test --- core/actions/table_test.ts | 80 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index 40023b186..c6ed1fbd5 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -225,6 +225,86 @@ SELECT 1` } }); }); + + 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: "bq-dataworkeragent-test", + location: "us-central1" + }, + { + project: "bq-dataworkeragent-test", + glossary_id: "jebmjilij-9c85ee94" + } + ], + generic: { + system: "yo", + type: "yo yo" + } + } + } + } + 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: "bq-dataworkeragent-test" }, + location: { stringValue: "us-central1" } + } + } + }, + { + structValue: { + fields: { + project: { stringValue: "bq-dataworkeragent-test" }, + glossary_id: { stringValue: "jebmjilij-9c85ee94" } + } + } + } + ] + } + }, + generic: { + structValue: { + fields: { + system: { stringValue: "yo" }, + type: { stringValue: "yo yo" } + } + } + } + } + }); + }); + + }); test("action config options", () => { From b1a54b5950f773b4fdc6efb0205c7efea7c6c270 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 22:07:09 +0000 Subject: [PATCH 11/41] fix a lint bug --- common/protos/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 6146db670..7433f0a29 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -108,7 +108,9 @@ function applyStructConversions(obj: any): any { for (const key of Object.keys(obj)) { const value = obj[key]; - if (value === undefined || value === null) continue; + if (value === undefined || value === null) { + continue; + } if (STRUCT_FIELD_NAMES.has(key)) { if (Array.isArray(value)) { From a2bc7b6e9a4075b1cfcd7a78d7bbb3e168626ede Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 25 Mar 2026 22:18:27 +0000 Subject: [PATCH 12/41] fix the test fail --- core/main_test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/main_test.ts b/core/main_test.ts index 1b53e0e91..4fb249242 100644 --- a/core/main_test.ts +++ b/core/main_test.ts @@ -310,10 +310,10 @@ publish("name")` expect( result.compile.compiledGraph.graphErrors.compilationErrors?.map(error => error.message) ).deep.equals([ - `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"`, - `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"`, - `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"`, - `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"` + `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"`, + `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"`, + `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"`, + `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"` ]); }); From 792f2b58fc6c95612d57c471d5c9c9b592965cc9 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 31 Mar 2026 21:31:42 +0000 Subject: [PATCH 13/41] Refactor verifyObjectMatchesProto with Dynamic Struct Detection (Double-Pass) --- common/protos/index.ts | 106 +++++++++++++++++++++++++++++++++-------- 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 7433f0a29..5b2890fe9 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -5,7 +5,7 @@ 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"; -const STRUCT_FIELD_NAMES = new Set(["extraProperties", "contexts", "glossary_terms"]); + export interface IProtoClass { @@ -39,30 +39,41 @@ export enum VerifyProtoErrorBehaviour { export function verifyObjectMatchesProto( protoType: IProtoClass, object: object, - errorBehaviour: VerifyProtoErrorBehaviour = VerifyProtoErrorBehaviour.DEFAULT, - structFieldNames: Set = STRUCT_FIELD_NAMES + errorBehaviour: VerifyProtoErrorBehaviour = VerifyProtoErrorBehaviour.DEFAULT ): Proto { if (Array.isArray(object)) { throw ReferenceError(`Expected a top-level object, but found an array`); } - const objectToVerify = applyStructConversions(object); + // 1. First Pass (The Probe) + const probeProto = protoType.create(object); + const probeObject = (protoType as any).toObject(probeProto, { defaults: true }); + + // 2. Detection + const lostPaths = findLostPaths(object, probeObject); + const lostPathsSet = new Set(lostPaths); + + // 3. Targeted Conversion + const convertedObject = applyStructConversions(object, lostPathsSet); - // Calling toObject on the object/JSON creates a version only contains the valid proto fields. - const proto = protoType.create(objectToVerify); - const protoCastObject = protoType.toObject(proto); + // 4. Final Build + const finalProto = protoType.create(convertedObject); + const finalProtoObject = protoType.toObject(finalProto); 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]) => { - if (structFieldNames.has(presentKey)) { - return; + let desiredKey = presentKey; + if (desired[presentKey] === undefined) { + if (desired[toSnakeCase(presentKey)] !== undefined) { + desiredKey = toSnakeCase(presentKey); + } else if (desired[toCamelCase(presentKey)] !== undefined) { + desiredKey = toCamelCase(presentKey); + } } - const desiredValue = desired[presentKey]; + const desiredValue = desired[desiredKey]; + 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) { @@ -72,7 +83,6 @@ export function verifyObjectMatchesProto( ); } 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) { @@ -95,11 +105,11 @@ export function verifyObjectMatchesProto( }); } - checkFields(object, protoCastObject); - return proto; + checkFields(convertedObject, finalProtoObject); + return finalProto; } -function applyStructConversions(obj: any): any { +function applyStructConversions(obj: any, lostPaths: Set, currentPath: string = ""): any { if (!obj || typeof obj !== "object" || Array.isArray(obj)) { return obj; } @@ -112,7 +122,9 @@ function applyStructConversions(obj: any): any { continue; } - if (STRUCT_FIELD_NAMES.has(key)) { + const path = currentPath ? `${currentPath}.${key}` : key; + + if (lostPaths.has(path)) { if (Array.isArray(value)) { result[key] = { listValue: { @@ -124,7 +136,7 @@ function applyStructConversions(obj: any): any { result[key] = converted.structValue; } } else if (typeof value === "object") { - result[key] = applyStructConversions(value); + result[key] = applyStructConversions(value, lostPaths, path); } } @@ -210,3 +222,59 @@ export function unknownToValue(raw: unknown): google.protobuf.IValue { } throw new Error(`Unsupported value: ${raw}`); } + +function findLostPaths(raw: any, probe: any, path: string = ""): string[] { + if (!raw || typeof raw !== "object" || Array.isArray(raw)) { + return []; + } + + let lost: string[] = []; + + for (const key of Object.keys(raw)) { + const rawVal = raw[key]; + if (!rawVal || typeof rawVal !== "object" || Object.keys(rawVal).length === 0) { + continue; + } + + const currentPath = path ? `${path}.${key}` : key; + const probeVal = probe ? (probe[key] || probe[toSnakeCase(key)] || probe[toCamelCase(key)]) : undefined; + + if (probeVal === undefined) { + lost.push(currentPath); + } else { + // Heuristic 1: If raw is an object, probe has a 'fields' property that is an empty object, + // and raw does not have 'fields', it's likely a plain JSON object for a Struct. + // Heuristic 2: If raw is a non-empty array and probe is an empty array or missing, it's lost. + if ( + typeof rawVal === "object" && + !Array.isArray(rawVal) && + probeVal && + typeof probeVal === "object" && + probeVal.fields && + typeof probeVal.fields === "object" && + Object.keys(probeVal.fields).length === 0 && + !rawVal.fields + ) { + lost.push(currentPath); + } else if ( + Array.isArray(rawVal) && + rawVal.length > 0 && + (!probeVal || (Array.isArray(probeVal) && probeVal.length === 0)) + ) { + lost.push(currentPath); + } else { + lost.push(...findLostPaths(rawVal, probeVal, currentPath)); + } + } + } + + return lost; +} + +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()); +} From f0e9f6c7ed6da1f588354fd8624912859b177248 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 31 Mar 2026 21:39:52 +0000 Subject: [PATCH 14/41] fix lint error --- common/protos/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 5b2890fe9..bf225a9fb 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -228,7 +228,7 @@ function findLostPaths(raw: any, probe: any, path: string = ""): string[] { return []; } - let lost: string[] = []; + const lost: string[] = []; for (const key of Object.keys(raw)) { const rawVal = raw[key]; From eb4720a769e3b290aebd377506607ce30fd31b70 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 1 Apr 2026 23:18:26 +0000 Subject: [PATCH 15/41] Do incremental conversion only for struct fields --- common/protos/index.ts | 229 ++++++++++++++++++----------------------- core/main_test.ts | 8 +- 2 files changed, 102 insertions(+), 135 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index bf225a9fb..3cf339f46 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -45,104 +45,117 @@ export function verifyObjectMatchesProto( throw ReferenceError(`Expected a top-level object, but found an array`); } - // 1. First Pass (The Probe) - const probeProto = protoType.create(object); - const probeObject = (protoType as any).toObject(probeProto, { defaults: true }); - - // 2. Detection - const lostPaths = findLostPaths(object, probeObject); - const lostPathsSet = new Set(lostPaths); - - // 3. Targeted Conversion - const convertedObject = applyStructConversions(object, lostPathsSet); - - // 4. Final Build - const finalProto = protoType.create(convertedObject); - const finalProtoObject = protoType.toObject(finalProto); - - function checkFields(present: { [k: string]: any }, desired: { [k: string]: any }) { - Object.entries(present).forEach(([presentKey, presentValue]) => { - let desiredKey = presentKey; - if (desired[presentKey] === undefined) { - if (desired[toSnakeCase(presentKey)] !== undefined) { - desiredKey = toSnakeCase(presentKey); - } else if (desired[toCamelCase(presentKey)] !== undefined) { - desiredKey = toCamelCase(presentKey); - } - } - const desiredValue = desired[desiredKey]; + // 1. Single Pass Create + const proto = protoType.create(object); + const probeObject = (protoType as any).toObject(proto, { defaults: true }); - if (typeof desiredValue !== typeof presentValue) { - if (Array.isArray(presentValue) && presentValue.length === 0) { - return; - } - if (!presentValue) { - throw ReferenceError( - `Unexpected empty value for "${presentKey}".` + - maybeGetDocsLinkPrefix(errorBehaviour, protoType) - ); - } - if (typeof presentValue === "object" && Object.keys(presentValue).length === 0) { - 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}.` - ); - } - throw ReferenceError( - `Unexpected property "${presentKey}", or property value type of ` + - `"${typeof presentValue}" is incorrect.` + - maybeGetDocsLinkPrefix(errorBehaviour, protoType) - ); - } - if (typeof presentValue === "object") { - checkFields(presentValue, desiredValue); - } - }); - } + // 2. Validate and Convert In-Place + checkAndConvertFields(object, probeObject, proto, errorBehaviour, protoType); - checkFields(convertedObject, finalProtoObject); - return finalProto; + return proto; } -function applyStructConversions(obj: any, lostPaths: Set, currentPath: string = ""): any { - if (!obj || typeof obj !== "object" || Array.isArray(obj)) { - return obj; - } +function checkAndConvertFields( + raw: { [k: string]: any }, + probe: { [k: string]: any }, + protoInstance: any, + errorBehaviour: VerifyProtoErrorBehaviour, + protoType: any +) { + Object.entries(raw).forEach(([rawKey, rawValue]) => { + if (rawValue === undefined) { + return; + } + if ( + rawValue === null && + errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM + ) { + return; + } - const result = { ...obj }; + 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 + ) { + const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); + throw ReferenceError(`Unexpected empty value for "${rawKey}".${docLinkPrefix}`); + } - for (const key of Object.keys(obj)) { - const value = obj[key]; - if (value === undefined || value === null) { - continue; + // 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; } - const path = currentPath ? `${currentPath}.${key}` : key; - - if (lostPaths.has(path)) { - if (Array.isArray(value)) { - result[key] = { - listValue: { - values: value.map(item => unknownToValue(item)) - } - }; - } else if (typeof value === "object" && !value.fields) { - const converted = unknownToValue(value); - result[key] = converted.structValue; + // 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) { + const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); + throw ReferenceError( + `Unexpected empty value for "${rawKey}".${docLinkPrefix}` + ); + } + if (typeof rawValue === "object" && Object.keys(rawValue).length === 0) { + return; } - } else if (typeof value === "object") { - result[key] = applyStructConversions(value, lostPaths, path); + 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}.` + ); + } + const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); + throw ReferenceError( + `Unexpected property "${rawKey}", or property value type of "${typeof rawValue}" is incorrect.${docLinkPrefix}` + ); } - } - return result; + if (typeof rawValue === "object" && rawValue !== null) { + checkAndConvertFields(rawValue, probeValue, protoInstance[probeKey], errorBehaviour, protoType); + } + }); } + function maybeGetDocsLinkPrefix( errorBehaviour: VerifyProtoErrorBehaviour, protoType: IProtoClass @@ -223,53 +236,7 @@ export function unknownToValue(raw: unknown): google.protobuf.IValue { throw new Error(`Unsupported value: ${raw}`); } -function findLostPaths(raw: any, probe: any, path: string = ""): string[] { - if (!raw || typeof raw !== "object" || Array.isArray(raw)) { - return []; - } - - const lost: string[] = []; - - for (const key of Object.keys(raw)) { - const rawVal = raw[key]; - if (!rawVal || typeof rawVal !== "object" || Object.keys(rawVal).length === 0) { - continue; - } - - const currentPath = path ? `${path}.${key}` : key; - const probeVal = probe ? (probe[key] || probe[toSnakeCase(key)] || probe[toCamelCase(key)]) : undefined; - - if (probeVal === undefined) { - lost.push(currentPath); - } else { - // Heuristic 1: If raw is an object, probe has a 'fields' property that is an empty object, - // and raw does not have 'fields', it's likely a plain JSON object for a Struct. - // Heuristic 2: If raw is a non-empty array and probe is an empty array or missing, it's lost. - if ( - typeof rawVal === "object" && - !Array.isArray(rawVal) && - probeVal && - typeof probeVal === "object" && - probeVal.fields && - typeof probeVal.fields === "object" && - Object.keys(probeVal.fields).length === 0 && - !rawVal.fields - ) { - lost.push(currentPath); - } else if ( - Array.isArray(rawVal) && - rawVal.length > 0 && - (!probeVal || (Array.isArray(probeVal) && probeVal.length === 0)) - ) { - lost.push(currentPath); - } else { - lost.push(...findLostPaths(rawVal, probeVal, currentPath)); - } - } - } - return lost; -} function toSnakeCase(str: string): string { return str.replace(/([a-z0-9])([A-Z])/g, "$1_$2").toLowerCase(); diff --git a/core/main_test.ts b/core/main_test.ts index 4fb249242..1b53e0e91 100644 --- a/core/main_test.ts +++ b/core/main_test.ts @@ -310,10 +310,10 @@ publish("name")` expect( result.compile.compiledGraph.graphErrors.compilationErrors?.map(error => error.message) ).deep.equals([ - `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"`, - `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"`, - `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"`, - `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"name\":\"name\",\"schema\":\"otherDataset\",\"database\":\"defaultProject\"}\"` + `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"`, + `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"`, + `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"`, + `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n\"{\"schema\":\"otherDataset\",\"name\":\"name\",\"database\":\"defaultProject\"}\"` ]); }); From 7e6aed1f7e854df331ababe645b723406b5a2ee6 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 1 Apr 2026 23:25:16 +0000 Subject: [PATCH 16/41] fix lint error --- common/protos/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 3cf339f46..992311494 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -62,6 +62,7 @@ function checkAndConvertFields( errorBehaviour: VerifyProtoErrorBehaviour, protoType: any ) { + const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); Object.entries(raw).forEach(([rawKey, rawValue]) => { if (rawValue === undefined) { return; @@ -88,7 +89,6 @@ function checkAndConvertFields( rawValue === null && errorBehaviour === VerifyProtoErrorBehaviour.SHOW_DOCS_LINK ) { - const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); throw ReferenceError(`Unexpected empty value for "${rawKey}".${docLinkPrefix}`); } @@ -128,7 +128,6 @@ function checkAndConvertFields( return; } if (!rawValue) { - const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); throw ReferenceError( `Unexpected empty value for "${rawKey}".${docLinkPrefix}` ); @@ -143,7 +142,6 @@ function checkAndConvertFields( .replace("/", "")}", please report this to the Dataform team at ${REPORT_ISSUE_URL}.` ); } - const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); throw ReferenceError( `Unexpected property "${rawKey}", or property value type of "${typeof rawValue}" is incorrect.${docLinkPrefix}` ); From 23840da8fb5bdee4ec6533b01abaa828c94632f6 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 1 Apr 2026 23:34:14 +0000 Subject: [PATCH 17/41] resolved comments on test --- core/actions/table_test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index c6ed1fbd5..9f2be88c4 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -243,17 +243,17 @@ SELECT 1` glossary_terms: [ { column_name: "trip_id", - project: "bq-dataworkeragent-test", + project: "project_identifier", location: "us-central1" }, { - project: "bq-dataworkeragent-test", + project: "project_identifier", glossary_id: "jebmjilij-9c85ee94" } ], generic: { - system: "yo", - type: "yo yo" + system: "my custom system value", + type: "my custom type value" } } } @@ -276,7 +276,7 @@ SELECT 1` structValue: { fields: { column_name: { stringValue: "trip_id" }, - project: { stringValue: "bq-dataworkeragent-test" }, + project: { stringValue: "project_identifier" }, location: { stringValue: "us-central1" } } } @@ -284,7 +284,7 @@ SELECT 1` { structValue: { fields: { - project: { stringValue: "bq-dataworkeragent-test" }, + project: { stringValue: "project_identifier" }, glossary_id: { stringValue: "jebmjilij-9c85ee94" } } } @@ -295,8 +295,8 @@ SELECT 1` generic: { structValue: { fields: { - system: { stringValue: "yo" }, - type: { stringValue: "yo yo" } + system: { stringValue: "my custom system value" }, + type: { stringValue: "my custom type value" } } } } From 8a5257c8e21427aa7e03365b7a32332aa6c2055d Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Thu, 2 Apr 2026 18:25:43 +0000 Subject: [PATCH 18/41] Add test for struct input already --- core/actions/table_test.ts | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index 9f2be88c4..bda7b8092 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -226,6 +226,42 @@ SELECT 1` }); }); + test("tables can be configured with a Struct already 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: { + fields: { + priority: { stringValue: "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( From 8dee580684eb24a2cd40f852e31804458e75909b Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Thu, 2 Apr 2026 20:16:53 +0000 Subject: [PATCH 19/41] resolve comments --- common/protos/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 992311494..b0831bdf3 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -226,7 +226,7 @@ export function unknownToValue(raw: unknown): google.protobuf.IValue { return { structValue: { fields: Object.fromEntries( - Object.entries(raw as object).map(([key, value]) => [key, unknownToValue(value)]) + Object.entries(raw).map(([key, value]) => [key, unknownToValue(value)]) ) } }; From 3bd115b036a65e33a8db68f44f413286dc926027 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Thu, 2 Apr 2026 20:51:24 +0000 Subject: [PATCH 20/41] remove blank line --- common/protos/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index b0831bdf3..bfc4e3f3b 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -6,8 +6,6 @@ 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 { new (): Proto; From edec4b3b24b85def948f33a41f36a9da15ec2d06 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Thu, 2 Apr 2026 21:05:09 +0000 Subject: [PATCH 21/41] Add glossary terms to the test case for struct already for extraProperties --- core/actions/table_test.ts | 50 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/core/actions/table_test.ts b/core/actions/table_test.ts index bda7b8092..38a010d1e 100644 --- a/core/actions/table_test.ts +++ b/core/actions/table_test.ts @@ -240,7 +240,30 @@ SELECT 1` metadata: { extraProperties: { fields: { - priority: { stringValue: "high" } + priority: { stringValue: "high" }, + 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" } + } + } + } + ] + } + } } } } @@ -256,7 +279,30 @@ SELECT 1` ).deep.equals({ extraProperties: { fields: { - priority: { stringValue: "high" } + priority: { stringValue: "high" }, + 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" } + } + } + } + ] + } + } } } }); From 984fd65532467b1b092ef3ed9efe812fd4b8ea33 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Fri, 3 Apr 2026 23:14:08 +0000 Subject: [PATCH 22/41] refactor the logic to check uncoverted struct and list, add detailed comments --- common/protos/index.ts | 65 ++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index bfc4e3f3b..275683b5f 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -91,31 +91,16 @@ function checkAndConvertFields( } // 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 - ) { + if (isUnconvertedStruct(rawValue, probeValue)) { 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 - ) { + if (isUnconvertedList(rawValue, probeValue)) { protoInstance[probeKey] = { listValue: { - values: rawValue.map(item => unknownToValue(item)) + values: rawValue.map((item: any) => unknownToValue(item)) } }; return; @@ -241,3 +226,47 @@ function toSnakeCase(str: string): string { function toCamelCase(str: string): string { return str.replace(/_([a-z])/g, (_, char) => char.toUpperCase()); } + +/** + * Heuristic to detect if a raw value should be converted to a google.protobuf.Struct. + * We use this heuristic because our static-stripped Protobuf build removes $type and + * fields metadata at runtime, making reflection impossible. + */ +function isUnconvertedStruct(raw: any, probe: any): boolean { + return ( + // 1. The user provided a plain object (not an array). + typeof raw === "object" && + !Array.isArray(raw) && + // 2. The probe (created via ProtobufJS) is an object. + probe && + typeof probe === "object" && + // 3. The probe has a 'fields' property which is an object. This is how + // ProtobufJS represents an empty/default google.protobuf.Struct. + probe.fields && + typeof probe.fields === "object" && + // 4. The 'fields' object is empty, indicating ProtobufJS couldn't map the raw data. + Object.keys(probe.fields).length === 0 && + // 5. The user didn't already provide a pre-converted struct (with 'fields'). + !raw.fields + ); +} + +/** + * Heuristic to detect if a raw value should be converted to a google.protobuf.ListValue. + * We use this heuristic because our static-stripped Protobuf build removes $type and + * fields metadata at runtime, making reflection impossible. + */ +function isUnconvertedList(raw: any, probe: any): boolean { + return ( + // 1. The user provided a non-empty array. + Array.isArray(raw) && + raw.length > 0 && + // 2. The probe exists and is an array. + probe && + Array.isArray(probe) && + // 3. The probe is empty. This suggests that the field is defined as a + // google.protobuf.ListValue, and ProtobufJS fallback to an empty default + // array because it couldn't map the raw array directly. + probe.length === 0 + ); +} From 453fa477d550fdebe967c3c51e5bca3db6b09b70 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Fri, 3 Apr 2026 23:41:11 +0000 Subject: [PATCH 23/41] refactor the logic to check if a field is given empty by user --- common/protos/index.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 275683b5f..8fc31be82 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -107,7 +107,8 @@ function checkAndConvertFields( } if (typeof probeValue !== typeof rawValue) { - if (Array.isArray(rawValue) && rawValue.length === 0) { + // Ignore empty containers (arrays or objects) as they are valid fallback states. + if (isEmptyObjectOrArray(rawValue)) { return; } if (!rawValue) { @@ -115,9 +116,6 @@ function checkAndConvertFields( `Unexpected empty value for "${rawKey}".${docLinkPrefix}` ); } - if (typeof rawValue === "object" && Object.keys(rawValue).length === 0) { - return; - } if (errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) { throw ReferenceError( `Unexpected property "${rawKey}" for "${protoType @@ -270,3 +268,18 @@ function isUnconvertedList(raw: any, probe: any): boolean { probe.length === 0 ); } + +/** + * Checks if a value is an empty array or an empty object. + * We ignore these during type mismatch checks to allow users to provide empty + * containers as valid fallback values. + */ +function isEmptyObjectOrArray(val: any): boolean { + if (Array.isArray(val)) { + return val.length === 0; + } + if (typeof val === "object" && val !== null) { + return Object.keys(val).length === 0; + } + return false; +} From 0f09773eafa461d7d116215d154aabeed2ae07fd Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Sat, 4 Apr 2026 00:04:51 +0000 Subject: [PATCH 24/41] remove the logic convert camel case and snake case --- common/protos/index.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 8fc31be82..ff88ce209 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -73,13 +73,6 @@ function checkAndConvertFields( } 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 ( @@ -215,16 +208,6 @@ export function unknownToValue(raw: unknown): google.protobuf.IValue { 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()); -} - /** * Heuristic to detect if a raw value should be converted to a google.protobuf.Struct. * We use this heuristic because our static-stripped Protobuf build removes $type and From a9a60b8495098497399fe24219fa506124f71a85 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Sat, 4 Apr 2026 00:10:40 +0000 Subject: [PATCH 25/41] change proto type from any --- common/protos/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index ff88ce209..401b2bcf4 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -58,7 +58,7 @@ function checkAndConvertFields( probe: { [k: string]: any }, protoInstance: any, errorBehaviour: VerifyProtoErrorBehaviour, - protoType: any + protoType: IProtoClass ) { const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); Object.entries(raw).forEach(([rawKey, rawValue]) => { From bd504f0fa3e34542db3fbbcf77f576403ebdd435 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Sat, 4 Apr 2026 00:20:05 +0000 Subject: [PATCH 26/41] reduce the argument in checkAndConvertFields --- common/protos/index.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 401b2bcf4..ec27403f0 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -48,26 +48,30 @@ export function verifyObjectMatchesProto( const probeObject = (protoType as any).toObject(proto, { defaults: true }); // 2. Validate and Convert In-Place - checkAndConvertFields(object, probeObject, proto, errorBehaviour, protoType); + checkAndConvertFields(object, probeObject, proto, { errorBehaviour, protoType }); return proto; } +interface ValidationContext { + errorBehaviour: VerifyProtoErrorBehaviour; + protoType: IProtoClass; +} + function checkAndConvertFields( raw: { [k: string]: any }, probe: { [k: string]: any }, protoInstance: any, - errorBehaviour: VerifyProtoErrorBehaviour, - protoType: IProtoClass + context: ValidationContext ) { - const docLinkPrefix = maybeGetDocsLinkPrefix(errorBehaviour, protoType); + const docLinkPrefix = maybeGetDocsLinkPrefix(context.errorBehaviour, context.protoType); Object.entries(raw).forEach(([rawKey, rawValue]) => { if (rawValue === undefined) { return; } if ( rawValue === null && - errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM + context.errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM ) { return; } @@ -78,7 +82,7 @@ function checkAndConvertFields( if ( Array.isArray(probeValue) && rawValue === null && - errorBehaviour === VerifyProtoErrorBehaviour.SHOW_DOCS_LINK + context.errorBehaviour === VerifyProtoErrorBehaviour.SHOW_DOCS_LINK ) { throw ReferenceError(`Unexpected empty value for "${rawKey}".${docLinkPrefix}`); } @@ -109,9 +113,9 @@ function checkAndConvertFields( `Unexpected empty value for "${rawKey}".${docLinkPrefix}` ); } - if (errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) { + if (context.errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) { throw ReferenceError( - `Unexpected property "${rawKey}" for "${protoType + `Unexpected property "${rawKey}" for "${context.protoType .getTypeUrl("") .replace("/", "")}", please report this to the Dataform team at ${REPORT_ISSUE_URL}.` ); @@ -122,7 +126,7 @@ function checkAndConvertFields( } if (typeof rawValue === "object" && rawValue !== null) { - checkAndConvertFields(rawValue, probeValue, protoInstance[probeKey], errorBehaviour, protoType); + checkAndConvertFields(rawValue, probeValue, protoInstance[probeKey], context); } }); } From 7ca5c498bb574af8a7f1c876e8e3cf7b5ab41615 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Sat, 4 Apr 2026 00:24:29 +0000 Subject: [PATCH 27/41] fix lint error --- common/protos/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index ec27403f0..4cdd1bed5 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -53,7 +53,7 @@ export function verifyObjectMatchesProto( return proto; } -interface ValidationContext { +interface IValidationContext { errorBehaviour: VerifyProtoErrorBehaviour; protoType: IProtoClass; } @@ -62,7 +62,7 @@ function checkAndConvertFields( raw: { [k: string]: any }, probe: { [k: string]: any }, protoInstance: any, - context: ValidationContext + context: IValidationContext ) { const docLinkPrefix = maybeGetDocsLinkPrefix(context.errorBehaviour, context.protoType); Object.entries(raw).forEach(([rawKey, rawValue]) => { @@ -76,7 +76,7 @@ function checkAndConvertFields( return; } - let probeKey = rawKey; + const probeKey = rawKey; const probeValue = probe[probeKey]; if ( From c018a6e0c38908ed49e87e361e5167f538a826e6 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Sat, 4 Apr 2026 00:30:58 +0000 Subject: [PATCH 28/41] Add test --- common/protos/BUILD | 21 ++++++++++++++ common/protos/index.spec.ts | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 common/protos/index.spec.ts diff --git a/common/protos/BUILD b/common/protos/BUILD index 6f12d80ed..2fbaec065 100644 --- a/common/protos/BUILD +++ b/common/protos/BUILD @@ -1,11 +1,13 @@ package(default_visibility = ["//visibility:public"]) load("//tools:ts_library.bzl", "ts_library") +load("@df//testing:index.bzl", "ts_test_suite") ts_library( name = "protos", srcs = glob( ["*.ts"], + exclude = ["*.spec.ts"], ), deps = [ "//:modules-fix", @@ -15,6 +17,25 @@ ts_library( ], ) +ts_test_suite( + name = "tests", + srcs = glob(["*.spec.ts"]), + data = [ + "@npm//source-map-support", + ], + templated_args = [ + "--node_options=--require=source-map-support/register", + "--bazel_patch_module_resolver", + ], + deps = [ + ":protos", + "@df//testing", + "@npm//@types/chai", + "@npm//@types/node", + "@npm//chai", + ], +) + load("//testing:build_test.bzl", "build_test") build_test( diff --git a/common/protos/index.spec.ts b/common/protos/index.spec.ts new file mode 100644 index 000000000..56e388305 --- /dev/null +++ b/common/protos/index.spec.ts @@ -0,0 +1,56 @@ +import { expect } from "chai"; +import { suite, test } from "df/testing"; +import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "./index"; + +suite("verifyObjectMatchesProto", () => { + test("throws error when top-level object is an array", () => { + const MockProto = {} as any; + expect(() => { + verifyObjectMatchesProto(MockProto, [] as any); + }).to.throw(ReferenceError, "Expected a top-level object, but found an array"); + }); + + test("throws error when null value provided for array field and SHOW_DOCS_LINK", () => { + const MockProto = { + create: (obj: any) => obj, + toObject: (proto: any, opts: any) => ({ repeatedField: [] as any[] }), + getTypeUrl: () => "MockProto" + } as any; + + expect(() => { + verifyObjectMatchesProto( + MockProto, + { repeatedField: null } as any, + VerifyProtoErrorBehaviour.SHOW_DOCS_LINK + ); + }).to.throw(ReferenceError, /Unexpected empty value for "repeatedField"/); + }); + + test("throws error on type mismatch with SUGGEST_REPORTING_TO_DATAFORM_TEAM", () => { + const MockProto = { + create: (obj: any) => obj, + toObject: (proto: any, opts: any) => ({ stringField: "default" }), + getTypeUrl: () => "MockProto" + } as any; + + expect(() => { + verifyObjectMatchesProto( + MockProto, + { stringField: 123 } as any, + VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM + ); + }).to.throw(ReferenceError, /Unexpected property "stringField" for "MockProto".*please report this to the Dataform team/); + }); + + test("throws error on default type mismatch", () => { + const MockProto = { + create: (obj: any) => obj, + toObject: (proto: any, opts: any) => ({ stringField: "default" }), + getTypeUrl: () => "MockProto" + } as any; + + expect(() => { + verifyObjectMatchesProto(MockProto, { stringField: 123 } as any); + }).to.throw(ReferenceError, /Unexpected property "stringField", or property value type of "number" is incorrect/); + }); +}); From bdf7ac291c996b9ffe31d82b5682876ceb5af7d1 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 6 Apr 2026 21:35:10 +0000 Subject: [PATCH 29/41] fix lint error --- common/protos/index.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/common/protos/index.spec.ts b/common/protos/index.spec.ts index 56e388305..a8e6acd4c 100644 --- a/common/protos/index.spec.ts +++ b/common/protos/index.spec.ts @@ -1,4 +1,5 @@ import { expect } from "chai"; + import { suite, test } from "df/testing"; import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "./index"; From 08b44164af52efed1fc2173360cd25a75c22192d Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 6 Apr 2026 21:49:35 +0000 Subject: [PATCH 30/41] modify import order --- common/protos/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/protos/index.spec.ts b/common/protos/index.spec.ts index a8e6acd4c..0ef7b1889 100644 --- a/common/protos/index.spec.ts +++ b/common/protos/index.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; +import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "./index"; import { suite, test } from "df/testing"; -import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "./index"; suite("verifyObjectMatchesProto", () => { test("throws error when top-level object is an array", () => { From 7d90a90c8099593fec713817b2c30dc6fd40036e Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 7 Apr 2026 23:05:20 +0000 Subject: [PATCH 31/41] resolve comments about input type --- common/protos/index.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 4cdd1bed5..9da9a5ab5 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -14,7 +14,7 @@ export interface IProtoClass { encode(proto: IProto | Proto): { finish(): Uint8Array }; decode(bytes: Uint8Array): Proto; - toObject(proto: Proto): { [k: string]: any }; + toObject(proto: Proto, options?: any): { [k: string]: any }; fromObject(obj: { [k: string]: any }): Proto; getTypeUrl(prefix: string): string; @@ -45,7 +45,7 @@ export function verifyObjectMatchesProto( // 1. Single Pass Create const proto = protoType.create(object); - const probeObject = (protoType as any).toObject(proto, { defaults: true }); + const probeObject = protoType.toObject(proto, { defaults: true }); // 2. Validate and Convert In-Place checkAndConvertFields(object, probeObject, proto, { errorBehaviour, protoType }); @@ -53,16 +53,16 @@ export function verifyObjectMatchesProto( return proto; } -interface IValidationContext { +interface IValidationContext { errorBehaviour: VerifyProtoErrorBehaviour; - protoType: IProtoClass; + protoType: IProtoClass; } -function checkAndConvertFields( +function checkAndConvertFields( raw: { [k: string]: any }, probe: { [k: string]: any }, - protoInstance: any, - context: IValidationContext + protoInstance: Proto, + context: IValidationContext ) { const docLinkPrefix = maybeGetDocsLinkPrefix(context.errorBehaviour, context.protoType); Object.entries(raw).forEach(([rawKey, rawValue]) => { @@ -89,13 +89,13 @@ function checkAndConvertFields( // Heuristic 1: Object Struct Detection if (isUnconvertedStruct(rawValue, probeValue)) { - protoInstance[probeKey] = unknownToValue(rawValue).structValue; + (protoInstance as any)[probeKey] = unknownToValue(rawValue).structValue; return; } // Heuristic 2: Array List/Struct Detection if (isUnconvertedList(rawValue, probeValue)) { - protoInstance[probeKey] = { + (protoInstance as any)[probeKey] = { listValue: { values: rawValue.map((item: any) => unknownToValue(item)) } @@ -126,7 +126,7 @@ function checkAndConvertFields( } if (typeof rawValue === "object" && rawValue !== null) { - checkAndConvertFields(rawValue, probeValue, protoInstance[probeKey], context); + checkAndConvertFields(rawValue, probeValue, (protoInstance as any)[probeKey], context); } }); } From bfc58e47bfd28fdbcc8177b57d6a09de7e6ded1e Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 8 Apr 2026 00:13:50 +0000 Subject: [PATCH 32/41] resolve comments about index_test --- common/protos/BUILD | 5 ++-- common/protos/index.spec.ts | 57 ------------------------------------- common/protos/index_test.ts | 45 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 59 deletions(-) delete mode 100644 common/protos/index.spec.ts create mode 100644 common/protos/index_test.ts diff --git a/common/protos/BUILD b/common/protos/BUILD index 2fbaec065..ffb46ca4d 100644 --- a/common/protos/BUILD +++ b/common/protos/BUILD @@ -7,7 +7,7 @@ ts_library( name = "protos", srcs = glob( ["*.ts"], - exclude = ["*.spec.ts"], + exclude = ["*_test.ts", "*.spec.ts"], ), deps = [ "//:modules-fix", @@ -19,7 +19,7 @@ ts_library( ts_test_suite( name = "tests", - srcs = glob(["*.spec.ts"]), + srcs = glob(["*_test.ts"]), data = [ "@npm//source-map-support", ], @@ -29,6 +29,7 @@ ts_test_suite( ], deps = [ ":protos", + "//protos:ts", "@df//testing", "@npm//@types/chai", "@npm//@types/node", diff --git a/common/protos/index.spec.ts b/common/protos/index.spec.ts deleted file mode 100644 index 0ef7b1889..000000000 --- a/common/protos/index.spec.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { expect } from "chai"; -import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "./index"; - -import { suite, test } from "df/testing"; - -suite("verifyObjectMatchesProto", () => { - test("throws error when top-level object is an array", () => { - const MockProto = {} as any; - expect(() => { - verifyObjectMatchesProto(MockProto, [] as any); - }).to.throw(ReferenceError, "Expected a top-level object, but found an array"); - }); - - test("throws error when null value provided for array field and SHOW_DOCS_LINK", () => { - const MockProto = { - create: (obj: any) => obj, - toObject: (proto: any, opts: any) => ({ repeatedField: [] as any[] }), - getTypeUrl: () => "MockProto" - } as any; - - expect(() => { - verifyObjectMatchesProto( - MockProto, - { repeatedField: null } as any, - VerifyProtoErrorBehaviour.SHOW_DOCS_LINK - ); - }).to.throw(ReferenceError, /Unexpected empty value for "repeatedField"/); - }); - - test("throws error on type mismatch with SUGGEST_REPORTING_TO_DATAFORM_TEAM", () => { - const MockProto = { - create: (obj: any) => obj, - toObject: (proto: any, opts: any) => ({ stringField: "default" }), - getTypeUrl: () => "MockProto" - } as any; - - expect(() => { - verifyObjectMatchesProto( - MockProto, - { stringField: 123 } as any, - VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM - ); - }).to.throw(ReferenceError, /Unexpected property "stringField" for "MockProto".*please report this to the Dataform team/); - }); - - test("throws error on default type mismatch", () => { - const MockProto = { - create: (obj: any) => obj, - toObject: (proto: any, opts: any) => ({ stringField: "default" }), - getTypeUrl: () => "MockProto" - } as any; - - expect(() => { - verifyObjectMatchesProto(MockProto, { stringField: 123 } as any); - }).to.throw(ReferenceError, /Unexpected property "stringField", or property value type of "number" is incorrect/); - }); -}); diff --git a/common/protos/index_test.ts b/common/protos/index_test.ts new file mode 100644 index 000000000..241c79a36 --- /dev/null +++ b/common/protos/index_test.ts @@ -0,0 +1,45 @@ +import { expect } from "chai"; +import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "./index"; + +import { dataform } from "df/protos/ts"; +import { suite, test } from "df/testing"; + +suite("verifyObjectMatchesProto", () => { + test("throws error when top-level object is an array", () => { + expect(() => { + verifyObjectMatchesProto(dataform.Target, [] as any); + }).to.throw(ReferenceError, "Expected a top-level object, but found an array"); + }); + + test("throws error when null value provided for array field and SHOW_DOCS_LINK", () => { + expect(() => { + verifyObjectMatchesProto( + dataform.Table, + { dependencyTargets: null } as any, + VerifyProtoErrorBehaviour.SHOW_DOCS_LINK + ); + }).to.throw(ReferenceError, /Unexpected empty value for "dependencyTargets"/); + }); + + test("throws error on type mismatch with SUGGEST_REPORTING_TO_DATAFORM_TEAM", () => { + expect(() => { + verifyObjectMatchesProto( + dataform.Table, + { actionDescriptor: 123 } as any, + VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM + ); + }).to.throw( + ReferenceError, + /Unexpected property "actionDescriptor" for ".*Table".*please report this to the Dataform team/ + ); + }); + + test("throws error on default type mismatch", () => { + expect(() => { + verifyObjectMatchesProto(dataform.Table, { actionDescriptor: 123 } as any); + }).to.throw( + ReferenceError, + /Unexpected property "actionDescriptor", or property value type of "number" is incorrect/ + ); + }); +}); From ddd216c8af336d3a02b7702cf721b1ef710e9175 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 13 Apr 2026 22:36:39 +0000 Subject: [PATCH 33/41] monkey patching for verify method --- common/protos/index.ts | 208 +++++++++++++---------------------------- 1 file changed, 66 insertions(+), 142 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 9da9a5ab5..0c35971ea 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -1,6 +1,6 @@ import { util } from "protobufjs"; -import { google } from "df/protos/ts"; +import { google, dataform } from "df/protos/ts"; const CONFIGS_PROTO_DOCUMENTATION_URL = "https://dataform-co.github.io/dataform/docs/configs-reference"; @@ -14,9 +14,11 @@ export interface IProtoClass { encode(proto: IProto | Proto): { finish(): Uint8Array }; decode(bytes: Uint8Array): Proto; - toObject(proto: Proto, options?: any): { [k: string]: any }; + toObject(proto: Proto): { [k: string]: any }; fromObject(obj: { [k: string]: any }): Proto; + verify(obj: any): (string | null); + getTypeUrl(prefix: string): string; } @@ -26,6 +28,27 @@ export enum VerifyProtoErrorBehaviour { SHOW_DOCS_LINK } +const Struct = google.protobuf.Struct; + +// Save references to the original generated methods +const originalVerify = Struct.verify; + +// ------------------------------------------------------------------------ +// Monkey Patching Methods +// ------------------------------------------------------------------------ + +Struct.verify = function (object: any) { + if (object && typeof object === "object" && !("fields" in object)) { + const fields: { [key: string]: any } = {}; + for (const [k, v] of Object.entries(object)) { + fields[k] = unknownToValue(v); + } + Object.keys(object).forEach(key => delete object[key]); + object["fields"] = fields; + } + return originalVerify.call(this, object); +}; + // This is a minimalist Typescript equivalent for the validation part of Profobuf's JsonFormat's // mergeMessage method: // https://github.com/protocolbuffers/protobuf/blob/670e0c2a0d0b64c994f743a73ee9b8926c47580d/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java#L1455 @@ -43,95 +66,55 @@ export function verifyObjectMatchesProto( throw ReferenceError(`Expected a top-level object, but found an array`); } - // 1. Single Pass Create + // Calling toObject on the object/JSON creates a version only contains the valid proto fields. + protoType.verify(object); const proto = protoType.create(object); - const probeObject = protoType.toObject(proto, { defaults: true }); - - // 2. Validate and Convert In-Place - checkAndConvertFields(object, probeObject, proto, { errorBehaviour, protoType }); - - return proto; -} - -interface IValidationContext { - errorBehaviour: VerifyProtoErrorBehaviour; - protoType: IProtoClass; -} - -function checkAndConvertFields( - raw: { [k: string]: any }, - probe: { [k: string]: any }, - protoInstance: Proto, - context: IValidationContext -) { - const docLinkPrefix = maybeGetDocsLinkPrefix(context.errorBehaviour, context.protoType); - Object.entries(raw).forEach(([rawKey, rawValue]) => { - if (rawValue === undefined) { - return; - } - if ( - rawValue === null && - context.errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM - ) { - return; - } - - const probeKey = rawKey; - const probeValue = probe[probeKey]; - - if ( - Array.isArray(probeValue) && - rawValue === null && - context.errorBehaviour === VerifyProtoErrorBehaviour.SHOW_DOCS_LINK - ) { - throw ReferenceError(`Unexpected empty value for "${rawKey}".${docLinkPrefix}`); - } - - // Heuristic 1: Object Struct Detection - if (isUnconvertedStruct(rawValue, probeValue)) { - (protoInstance as any)[probeKey] = unknownToValue(rawValue).structValue; - return; - } - - // Heuristic 2: Array List/Struct Detection - if (isUnconvertedList(rawValue, probeValue)) { - (protoInstance as any)[probeKey] = { - listValue: { - values: rawValue.map((item: any) => unknownToValue(item)) + 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}.` + ); } - }; - return; - } - - if (typeof probeValue !== typeof rawValue) { - // Ignore empty containers (arrays or objects) as they are valid fallback states. - if (isEmptyObjectOrArray(rawValue)) { - return; - } - if (!rawValue) { throw ReferenceError( - `Unexpected empty value for "${rawKey}".${docLinkPrefix}` + `Unexpected property "${presentKey}", or property value type of ` + + `"${typeof presentValue}" is incorrect.` + + maybeGetDocsLinkPrefix(errorBehaviour, protoType) ); } - if (context.errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) { - throw ReferenceError( - `Unexpected property "${rawKey}" for "${context.protoType - .getTypeUrl("") - .replace("/", "")}", please report this to the Dataform team at ${REPORT_ISSUE_URL}.` - ); + if (typeof presentValue === "object") { + checkFields(presentValue, desiredValue); } - 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 as any)[probeKey], context); - } - }); + checkFields(object, protoCastObject); + return proto; } - function maybeGetDocsLinkPrefix( errorBehaviour: VerifyProtoErrorBehaviour, protoType: IProtoClass @@ -210,63 +193,4 @@ export function unknownToValue(raw: unknown): google.protobuf.IValue { }; } throw new Error(`Unsupported value: ${raw}`); -} - -/** - * Heuristic to detect if a raw value should be converted to a google.protobuf.Struct. - * We use this heuristic because our static-stripped Protobuf build removes $type and - * fields metadata at runtime, making reflection impossible. - */ -function isUnconvertedStruct(raw: any, probe: any): boolean { - return ( - // 1. The user provided a plain object (not an array). - typeof raw === "object" && - !Array.isArray(raw) && - // 2. The probe (created via ProtobufJS) is an object. - probe && - typeof probe === "object" && - // 3. The probe has a 'fields' property which is an object. This is how - // ProtobufJS represents an empty/default google.protobuf.Struct. - probe.fields && - typeof probe.fields === "object" && - // 4. The 'fields' object is empty, indicating ProtobufJS couldn't map the raw data. - Object.keys(probe.fields).length === 0 && - // 5. The user didn't already provide a pre-converted struct (with 'fields'). - !raw.fields - ); -} - -/** - * Heuristic to detect if a raw value should be converted to a google.protobuf.ListValue. - * We use this heuristic because our static-stripped Protobuf build removes $type and - * fields metadata at runtime, making reflection impossible. - */ -function isUnconvertedList(raw: any, probe: any): boolean { - return ( - // 1. The user provided a non-empty array. - Array.isArray(raw) && - raw.length > 0 && - // 2. The probe exists and is an array. - probe && - Array.isArray(probe) && - // 3. The probe is empty. This suggests that the field is defined as a - // google.protobuf.ListValue, and ProtobufJS fallback to an empty default - // array because it couldn't map the raw array directly. - probe.length === 0 - ); -} - -/** - * Checks if a value is an empty array or an empty object. - * We ignore these during type mismatch checks to allow users to provide empty - * containers as valid fallback values. - */ -function isEmptyObjectOrArray(val: any): boolean { - if (Array.isArray(val)) { - return val.length === 0; - } - if (typeof val === "object" && val !== null) { - return Object.keys(val).length === 0; - } - return false; -} +} \ No newline at end of file From 5e75b80f79a52a166bdad0fa13a6f5711b5a7f66 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Mon, 13 Apr 2026 22:45:00 +0000 Subject: [PATCH 34/41] fix lint error --- common/protos/index.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 0c35971ea..1529a6c98 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -1,6 +1,6 @@ import { util } from "protobufjs"; -import { google, dataform } from "df/protos/ts"; +import { google } from "df/protos/ts"; const CONFIGS_PROTO_DOCUMENTATION_URL = "https://dataform-co.github.io/dataform/docs/configs-reference"; @@ -33,10 +33,7 @@ const Struct = google.protobuf.Struct; // Save references to the original generated methods const originalVerify = Struct.verify; -// ------------------------------------------------------------------------ // Monkey Patching Methods -// ------------------------------------------------------------------------ - Struct.verify = function (object: any) { if (object && typeof object === "object" && !("fields" in object)) { const fields: { [key: string]: any } = {}; @@ -44,7 +41,7 @@ Struct.verify = function (object: any) { fields[k] = unknownToValue(v); } Object.keys(object).forEach(key => delete object[key]); - object["fields"] = fields; + object.fields = fields; } return originalVerify.call(this, object); }; From 028a602cf7af246087f463face5cb4276bf2a312 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 14 Apr 2026 23:08:05 +0000 Subject: [PATCH 35/41] have a separate function here that won't be using recursion because verify itself will handle recursive transformations --- common/protos/index.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 1529a6c98..2bf2777b2 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -38,7 +38,7 @@ Struct.verify = function (object: any) { if (object && typeof object === "object" && !("fields" in object)) { const fields: { [key: string]: any } = {}; for (const [k, v] of Object.entries(object)) { - fields[k] = unknownToValue(v); + fields[k] = unknownToValueShallow(v); } Object.keys(object).forEach(key => delete object[key]); object.fields = fields; @@ -164,6 +164,28 @@ function fromBase64(value: string): Uint8Array { return buf; } +function unknownToValueShallow(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(unknownToValueShallow) } }; + } + if (typeof raw === "object") { + return { structValue: raw as any }; + } + throw new Error(`Unsupported value: ${raw}`); +} + export function unknownToValue(raw: unknown): google.protobuf.IValue { if (raw === null || typeof raw === "undefined") { return { nullValue: 0 }; From c22d352c2059fddfd50a4120b43808075d0cf0aa Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 14 Apr 2026 23:19:40 +0000 Subject: [PATCH 36/41] get rid of the checkFields function below by recursively patching all verify calls --- common/protos/index.ts | 113 +++++++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 45 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 2bf2777b2..d942ec198 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -45,6 +45,47 @@ Struct.verify = function (object: any) { } return originalVerify.call(this, object); }; +function patchNamespace(ns: any) { + if (!ns || typeof ns !== "object") return; + + if (typeof ns.verify === "function" && !ns.verify._isPatched) { + const dummy = new ns(); + const validKeys = new Set(Object.keys(ns.toObject(dummy, { defaults: true }))); + const originalVerify = ns.verify; + + ns.verify = function(message: any): string | null { + if (message && typeof message === "object" && !Array.isArray(message)) { + for (const key of Object.keys(message)) { + if (key.startsWith('$')) continue; + if (message[key] === null || message[key] === undefined) { + return `Unexpected empty value for "${key}"`; + } + } + } + + const err = originalVerify.call(this, message); + if (err) return err; + + if (message && typeof message === "object" && !Array.isArray(message)) { + for (const key of Object.keys(message)) { + if (key.startsWith('$')) continue; + if (!validKeys.has(key)) { + return `Unexpected property "${key}"`; + } + } + } + return null; + }; + ns.verify._isPatched = true; + } + + for (const key in ns) { + if (Object.prototype.hasOwnProperty.call(ns, key)) { + patchNamespace(ns[key]); + } + } +} + // This is a minimalist Typescript equivalent for the validation part of Profobuf's JsonFormat's // mergeMessage method: @@ -63,53 +104,35 @@ export function verifyObjectMatchesProto( 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. - protoType.verify(object); - 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}.` - ); - } - throw ReferenceError( - `Unexpected property "${presentKey}", or property value type of ` + - `"${typeof presentValue}" is incorrect.` + - maybeGetDocsLinkPrefix(errorBehaviour, protoType) - ); - } - if (typeof presentValue === "object") { - checkFields(presentValue, desiredValue); - } - }); + // Patch the proto type and its nested types lazily. + patchNamespace(protoType); + + const err = protoType.verify(object); + if (err) { + if (err.startsWith("Unexpected")) { + throw ReferenceError(err + maybeGetDocsLinkPrefix(errorBehaviour, protoType)); + } + + const parts = err.split(/[:.]/); + const fieldName = parts[0]; + + if (errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) { + throw ReferenceError( + `Unexpected property "${fieldName}" for "${protoType + .getTypeUrl("") + .replace("/", "")}", please report this to the Dataform team at ` + + `https://github.com/dataform-co/dataform/issues.` + ); + } + + throw ReferenceError( + `Unexpected property "${fieldName}", or property value type of ` + + `"${typeof (object as any)[fieldName]}" is incorrect.` + + maybeGetDocsLinkPrefix(errorBehaviour, protoType) + ); } - checkFields(object, protoCastObject); - return proto; + return protoType.create(object); } function maybeGetDocsLinkPrefix( From a54a42c8a2a68aad094b56111874aeb96a8e5df0 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 14 Apr 2026 23:26:26 +0000 Subject: [PATCH 37/41] fix lint error --- common/protos/index.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index d942ec198..6ed58de3c 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -46,24 +46,28 @@ Struct.verify = function (object: any) { return originalVerify.call(this, object); }; function patchNamespace(ns: any) { - if (!ns || typeof ns !== "object") return; + if (!ns || typeof ns !== "object") { + return; + } if (typeof ns.verify === "function" && !ns.verify._isPatched) { const dummy = new ns(); const validKeys = new Set(Object.keys(ns.toObject(dummy, { defaults: true }))); - const originalVerify = ns.verify; + const baseVerify = ns.verify; ns.verify = function(message: any): string | null { if (message && typeof message === "object" && !Array.isArray(message)) { for (const key of Object.keys(message)) { - if (key.startsWith('$')) continue; + if (key.startsWith('$')) { + continue; + } if (message[key] === null || message[key] === undefined) { return `Unexpected empty value for "${key}"`; } } } - const err = originalVerify.call(this, message); + const err = baseVerify.call(this, message); if (err) return err; if (message && typeof message === "object" && !Array.isArray(message)) { From 2538d67025deea2a7da8047958c901f0689431c1 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Tue, 14 Apr 2026 23:30:58 +0000 Subject: [PATCH 38/41] fix lint error --- common/protos/index.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 6ed58de3c..b8b42ccb4 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -68,11 +68,15 @@ function patchNamespace(ns: any) { } const err = baseVerify.call(this, message); - if (err) return err; + if (err) { + return err; + } if (message && typeof message === "object" && !Array.isArray(message)) { for (const key of Object.keys(message)) { - if (key.startsWith('$')) continue; + if (key.startsWith('$')) { + continue; + } if (!validKeys.has(key)) { return `Unexpected property "${key}"`; } From 092deeca7428795fe0a293716f0f457c2a26f195 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 15 Apr 2026 00:07:28 +0000 Subject: [PATCH 39/41] Revert "fix lint error" This reverts commit 2538d67025deea2a7da8047958c901f0689431c1. --- common/protos/index.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index b8b42ccb4..6ed58de3c 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -68,15 +68,11 @@ function patchNamespace(ns: any) { } const err = baseVerify.call(this, message); - if (err) { - return err; - } + if (err) return err; if (message && typeof message === "object" && !Array.isArray(message)) { for (const key of Object.keys(message)) { - if (key.startsWith('$')) { - continue; - } + if (key.startsWith('$')) continue; if (!validKeys.has(key)) { return `Unexpected property "${key}"`; } From 481a241128fb3da9f832b819a3f79365e4449dd6 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 15 Apr 2026 00:08:03 +0000 Subject: [PATCH 40/41] Revert "fix lint error" This reverts commit a54a42c8a2a68aad094b56111874aeb96a8e5df0. --- common/protos/index.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index 6ed58de3c..d942ec198 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -46,28 +46,24 @@ Struct.verify = function (object: any) { return originalVerify.call(this, object); }; function patchNamespace(ns: any) { - if (!ns || typeof ns !== "object") { - return; - } + if (!ns || typeof ns !== "object") return; if (typeof ns.verify === "function" && !ns.verify._isPatched) { const dummy = new ns(); const validKeys = new Set(Object.keys(ns.toObject(dummy, { defaults: true }))); - const baseVerify = ns.verify; + const originalVerify = ns.verify; ns.verify = function(message: any): string | null { if (message && typeof message === "object" && !Array.isArray(message)) { for (const key of Object.keys(message)) { - if (key.startsWith('$')) { - continue; - } + if (key.startsWith('$')) continue; if (message[key] === null || message[key] === undefined) { return `Unexpected empty value for "${key}"`; } } } - const err = baseVerify.call(this, message); + const err = originalVerify.call(this, message); if (err) return err; if (message && typeof message === "object" && !Array.isArray(message)) { From de3a9267b7c7920de0539ce480489e6470641a88 Mon Sep 17 00:00:00 2001 From: zhipingliu Date: Wed, 15 Apr 2026 00:08:05 +0000 Subject: [PATCH 41/41] Revert "get rid of the checkFields function below by recursively patching all verify calls" This reverts commit c22d352c2059fddfd50a4120b43808075d0cf0aa. --- common/protos/index.ts | 113 ++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 68 deletions(-) diff --git a/common/protos/index.ts b/common/protos/index.ts index d942ec198..2bf2777b2 100644 --- a/common/protos/index.ts +++ b/common/protos/index.ts @@ -45,47 +45,6 @@ Struct.verify = function (object: any) { } return originalVerify.call(this, object); }; -function patchNamespace(ns: any) { - if (!ns || typeof ns !== "object") return; - - if (typeof ns.verify === "function" && !ns.verify._isPatched) { - const dummy = new ns(); - const validKeys = new Set(Object.keys(ns.toObject(dummy, { defaults: true }))); - const originalVerify = ns.verify; - - ns.verify = function(message: any): string | null { - if (message && typeof message === "object" && !Array.isArray(message)) { - for (const key of Object.keys(message)) { - if (key.startsWith('$')) continue; - if (message[key] === null || message[key] === undefined) { - return `Unexpected empty value for "${key}"`; - } - } - } - - const err = originalVerify.call(this, message); - if (err) return err; - - if (message && typeof message === "object" && !Array.isArray(message)) { - for (const key of Object.keys(message)) { - if (key.startsWith('$')) continue; - if (!validKeys.has(key)) { - return `Unexpected property "${key}"`; - } - } - } - return null; - }; - ns.verify._isPatched = true; - } - - for (const key in ns) { - if (Object.prototype.hasOwnProperty.call(ns, key)) { - patchNamespace(ns[key]); - } - } -} - // This is a minimalist Typescript equivalent for the validation part of Profobuf's JsonFormat's // mergeMessage method: @@ -104,35 +63,53 @@ export function verifyObjectMatchesProto( throw ReferenceError(`Expected a top-level object, but found an array`); } - // Patch the proto type and its nested types lazily. - patchNamespace(protoType); - - const err = protoType.verify(object); - if (err) { - if (err.startsWith("Unexpected")) { - throw ReferenceError(err + maybeGetDocsLinkPrefix(errorBehaviour, protoType)); - } - - const parts = err.split(/[:.]/); - const fieldName = parts[0]; - - if (errorBehaviour === VerifyProtoErrorBehaviour.SUGGEST_REPORTING_TO_DATAFORM_TEAM) { - throw ReferenceError( - `Unexpected property "${fieldName}" for "${protoType - .getTypeUrl("") - .replace("/", "")}", please report this to the Dataform team at ` + - `https://github.com/dataform-co/dataform/issues.` - ); - } - - throw ReferenceError( - `Unexpected property "${fieldName}", or property value type of ` + - `"${typeof (object as any)[fieldName]}" is incorrect.` + - maybeGetDocsLinkPrefix(errorBehaviour, protoType) - ); + // Calling toObject on the object/JSON creates a version only contains the valid proto fields. + protoType.verify(object); + 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}.` + ); + } + throw ReferenceError( + `Unexpected property "${presentKey}", or property value type of ` + + `"${typeof presentValue}" is incorrect.` + + maybeGetDocsLinkPrefix(errorBehaviour, protoType) + ); + } + if (typeof presentValue === "object") { + checkFields(presentValue, desiredValue); + } + }); } - return protoType.create(object); + checkFields(object, protoCastObject); + return proto; } function maybeGetDocsLinkPrefix(