From 7c1d4af0fd588c525b1673cd6597b4912aed2845 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 12 May 2026 18:56:05 -0600 Subject: [PATCH 01/13] feat(frontend): add Python UDF UI parameter inference --- frontend/package.json | 1 + frontend/src/app/app.module.ts | 2 + .../src/app/common/formly/formly-config.ts | 2 + .../ui-udf-parameters.component.html | 55 +++++ .../ui-udf-parameters.component.scss | 39 ++++ .../ui-udf-parameters.component.ts | 93 ++++++++ .../ui-udf-parameters-parser.service.spec.ts | 143 ++++++++++++ .../ui-udf-parameters-parser.service.ts | 213 ++++++++++++++++++ .../ui-udf-parameters-sync.service.spec.ts | 120 ++++++++++ .../ui-udf-parameters-sync.service.ts | 127 +++++++++++ .../types/workflow-compiling.interface.ts | 10 +- frontend/yarn.lock | 37 +++ 12 files changed, 841 insertions(+), 1 deletion(-) create mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html create mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss create mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts create mode 100644 frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts create mode 100644 frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts create mode 100644 frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts create mode 100644 frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts diff --git a/frontend/package.json b/frontend/package.json index 08b298260e3..481e720d976 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -38,6 +38,7 @@ "@codingame/monaco-vscode-java-default-extension": "8.0.4", "@codingame/monaco-vscode-python-default-extension": "8.0.4", "@codingame/monaco-vscode-r-default-extension": "8.0.4", + "@lezer/python": "1.1.18", "@ngneat/until-destroy": "8.1.4", "@ngx-formly/core": "6.3.12", "@ngx-formly/ng-zorro-antd": "6.3.12", diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index 21928b77039..78fc75d7cbc 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -156,6 +156,7 @@ import { NzTreeModule } from "ng-zorro-antd/tree"; import { NzTreeViewModule } from "ng-zorro-antd/tree-view"; import { NzNoAnimationModule } from "ng-zorro-antd/core/animation"; import { TreeModule } from "@ali-hm/angular-tree-component"; +import { UiUdfParametersComponent } from "./workspace/component/ui-udf-parameters/ui-udf-parameters.component"; import { ResultExportationComponent } from "./workspace/component/result-exportation/result-exportation.component"; import { ReportGenerationService } from "./workspace/service/report-generation/report-generation.service"; import { SearchBarComponent } from "./dashboard/component/user/search-bar/search-bar.component"; @@ -265,6 +266,7 @@ registerLocaleData(en); NzGridModule, ScrollingModule, FormlyRepeatDndComponent, + UiUdfParametersComponent, AdminGmailComponent, PublicProjectComponent, WorkspaceComponent, diff --git a/frontend/src/app/common/formly/formly-config.ts b/frontend/src/app/common/formly/formly-config.ts index c3995abb544..707ddfa7975 100644 --- a/frontend/src/app/common/formly/formly-config.ts +++ b/frontend/src/app/common/formly/formly-config.ts @@ -27,6 +27,7 @@ import { PresetWrapperComponent } from "./preset-wrapper/preset-wrapper.componen import { DatasetFileSelectorComponent } from "../../workspace/component/dataset-file-selector/dataset-file-selector.component"; import { CollabWrapperComponent } from "./collab-wrapper/collab-wrapper/collab-wrapper.component"; import { FormlyRepeatDndComponent } from "./repeat-dnd/repeat-dnd.component"; +import { UiUdfParametersComponent } from "../../workspace/component/ui-udf-parameters/ui-udf-parameters.component"; import { DatasetVersionSelectorComponent } from "../../workspace/component/dataset-version-selector/dataset-version-selector.component"; /** @@ -80,6 +81,7 @@ export const TEXERA_FORMLY_CONFIG = { { name: "inputautocomplete", component: DatasetFileSelectorComponent, wrappers: ["form-field"] }, { name: "datasetversionselector", component: DatasetVersionSelectorComponent, wrappers: ["form-field"] }, { name: "repeat-section-dnd", component: FormlyRepeatDndComponent }, + { name: "ui-udf-parameters", component: UiUdfParametersComponent, wrappers: ["form-field"] }, ], wrappers: [ { name: "preset-wrapper", component: PresetWrapperComponent }, diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html new file mode 100644 index 00000000000..0ecfb16a780 --- /dev/null +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html @@ -0,0 +1,55 @@ + +
+ +
+
Value
+
Name
+
Type
+
+ +
+ + +
+ + + +
+ + +
+ + + +
+ + +
+ + + +
+ +
+
diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss new file mode 100644 index 00000000000..901edaf1fc9 --- /dev/null +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss @@ -0,0 +1,39 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +.ui-udf-param-row { + display: grid; + grid-template-columns: 250px 250px 1fr; + gap: 12px; + align-items: start; +} + +.field-cell { + min-width: 0; +} + +/* Remove Formly/Ant label spacing */ +:host ::ng-deep .ant-form-item { + margin-bottom: 0; +} + +/* Hide Formly labels*/ +:host ::ng-deep .ant-form-item-label { + display: none; +} diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts new file mode 100644 index 00000000000..cabaef3604f --- /dev/null +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Component } from "@angular/core"; +import { NgFor, NgIf } from "@angular/common"; +import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; + +@Component({ + selector: "texera-ui-udf-parameters", + templateUrl: "./ui-udf-parameters.component.html", + styleUrls: ["./ui-udf-parameters.component.scss"], + imports: [NgIf, NgFor, FormlyModule], +}) +export class UiUdfParametersComponent extends FieldArrayType { + private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { + return rowField.fieldGroup?.find(f => f.key === key); + } + + private getAttributeChild(rowField: FormlyFieldConfig, childKey: string): FormlyFieldConfig | undefined { + const attributeGroup = this.getField(rowField, "attribute"); + return attributeGroup?.fieldGroup?.find(f => f.key === childKey); + } + + private setDisabled(field: FormlyFieldConfig | undefined, disabled: boolean): FormlyFieldConfig | undefined { + if (!field) return undefined; + + // 1) Modern Formly + field.props = { ...(field.props ?? {}), disabled }; + + // 2) Compatibility for templates/wrappers still using templateOptions + // (`as any` so we don't get nagged by the @deprecated JSDoc) + (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; + + // 3) Enforce at the reactive form level + if (field.formControl) { + if (disabled) { + field.formControl.disable({ emitEvent: false }); + } else { + field.formControl.enable({ emitEvent: false }); + } + } else { + // If control isn't created yet, disable it at init time. + const prevOnInit = field.hooks?.onInit; + field.hooks = { + ...(field.hooks ?? {}), + onInit: f => { + prevOnInit?.(f); + if (disabled) { + f.formControl?.disable({ emitEvent: false }); + } else { + f.formControl?.enable({ emitEvent: false }); + } + }, + }; + } + + return field; + } + + // Disable Name + getNameField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { + return this.setDisabled(this.getAttributeChild(rowField, "attributeName"), true); + } + + // Disable Type + getTypeField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { + return this.setDisabled(this.getAttributeChild(rowField, "attributeType"), true); + } + + // Value editable + getValueField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { + return this.setDisabled(this.getField(rowField, "value"), false); + } + + trackByParamName = (index: number, param: any): string | number => { + return param?.attribute?.attributeName ?? index; + }; +} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts new file mode 100644 index 00000000000..d0a887bc61f --- /dev/null +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -0,0 +1,143 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; + +describe("UiUdfParametersParserService", () => { + let service: UiUdfParametersParserService; + + beforeEach(() => { + service = new UiUdfParametersParserService(); + }); + + it("should parse Python-compatible positional and name-based arguments", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("count", AttributeType.INT) + self.UiParameter(type=AttributeType.STRING, name="name") + self.UiParameter(name="age", type=AttributeType.LONG) + self.UiParameter("score", AttributeType.DOUBLE) + self.UiParameter("created_at", type=AttributeType.TIMESTAMP) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, + { attribute: { attributeName: "name", attributeType: "string" }, value: "" }, + { attribute: { attributeName: "age", attributeType: "long" }, value: "" }, + { attribute: { attributeName: "score", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "created_at", attributeType: "timestamp" }, value: "" }, + ]); + }); + + it("should parse the attr_type keyword used by the Python API", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("count", attr_type=AttributeType.INT) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, + ]); + }); + + it("should ignore calls where name or type is missing", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter(name="a") + self.UiParameter(type=AttributeType.DOUBLE) + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should ignore invalid positional argument ordering", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter(AttributeType.INT, "count") + self.UiParameter(name="valid", type=AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, + ]); + }); + + it("should ignore legacy key= named argument", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter(type=AttributeType.DOUBLE, key="a") + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should ignore unsupported classes", () => { + const code = ` + class RandomClass(ABC): + def open(self): + self.UiParameter(type=AttributeType.DOUBLE, name="a") + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should ignore commented out UiParameter calls", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + # self.UiParameter("commented", AttributeType.INT) + self.UiParameter("active", AttributeType.INT) # self.UiParameter("trailing", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "active", attributeType: "integer" }, value: "" }, + ]); + }); + + it("should ignore UiParameter examples inside triple-quoted strings", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + """ + self.UiParameter("example", AttributeType.INT) + """ + self.UiParameter("active", AttributeType.DOUBLE) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "active", attributeType: "double" }, value: "" }, + ]); + }); + + it("should reject binary UiParameter types", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("payload", AttributeType.BINARY) + self.UiParameter("blob", AttributeType.LARGE_BINARY) + `; + + expect(service.parse(code)).toEqual([]); + }); +}); diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts new file mode 100644 index 00000000000..86d73da8d94 --- /dev/null +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Injectable } from "@angular/core"; +import { parser } from "@lezer/python"; +import { AttributeType, SchemaAttribute } from "../../types/workflow-compiling.interface"; + +export interface UiUdfParameter { + attribute: SchemaAttribute; + value: string; +} + +const CLASSES = new Set(["ProcessTupleOperator", "ProcessBatchOperator", "ProcessTableOperator", "GenerateOperator"]); + +// Java enum constant names (AttributeType.java) +const JAVA_ATTRIBUTE_TYPE_NAMES = [ + "STRING", + "INTEGER", + "LONG", + "DOUBLE", + "BOOLEAN", + "TIMESTAMP", + "BINARY", + "LARGE_BINARY", +] as const; + +type JavaAttributeTypeName = (typeof JAVA_ATTRIBUTE_TYPE_NAMES)[number]; + +// Python enum constant names (core.models.AttributeType) +const PYTHON_ATTRIBUTE_TYPE_NAMES = [ + "STRING", + "INT", + "LONG", + "DOUBLE", + "BOOL", + "TIMESTAMP", + "BINARY", + "LARGE_BINARY", +] as const; + +type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number]; + +type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName; +type ParserSyntaxNode = ReturnType["topNode"]; + +const TYPES: Readonly> = { + STRING: "string", + INTEGER: "integer", + INT: "integer", + LONG: "long", + DOUBLE: "double", + BOOLEAN: "boolean", + BOOL: "boolean", + TIMESTAMP: "timestamp", + BINARY: "binary", + LARGE_BINARY: "large_binary", +}; + +const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set(JAVA_ATTRIBUTE_TYPE_NAMES); +const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set(PYTHON_ATTRIBUTE_TYPE_NAMES); +const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set([ + "string", + "integer", + "long", + "double", + "boolean", + "timestamp", +]); + +@Injectable({ providedIn: "root" }) +export class UiUdfParametersParserService { + parse(code: string): UiUdfParameter[] { + if (!code) return []; + + const result: UiUdfParameter[] = []; + const seen = new Set(); + const add = (parameter?: UiUdfParameter): void => { + const name = parameter?.attribute.attributeName; + if (parameter && name && !seen.has(name)) { + seen.add(name); + result.push(parameter); + } + }; + + parser.parse(code).iterate({ + enter: ({ name, node }) => { + const className = node.getChild("VariableName"); + if (name !== "ClassDefinition" || !className || !CLASSES.has(code.slice(className.from, className.to))) return; + node + .cursor() + .iterate(ref => (ref.name === "CallExpression" ? (add(readCall(ref.node, code)), false) : undefined)); + return false; + }, + }); + + return result; + } +} + +function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefined { + const args = call.getChild("ArgList"); + if (!args || code.slice(call.from, args.from).replace(/\s+/g, "") !== "self.UiParameter") return undefined; + + let attributeName: string | undefined; + let attributeType: AttributeType | undefined; + let index = 0; + let sawNamed = false; + + for (const arg of splitArgs(code.slice(args.from + 1, args.to - 1))) { + const match = arg.match(/^([A-Za-z_]\w*)\s*=\s*([\s\S]+)$/); + const key = match?.[1]; + const value = match?.[2] ?? arg; + + if (match) sawNamed = true; + else if (sawNamed || index > 1) return undefined; + + if ((match ? key === "name" : index === 0) && !attributeName) attributeName = readString(value)?.trim(); + else if ((match ? key === "type" || key === "attr_type" : index === 1) && !attributeType) + attributeType = readType(value); + else return undefined; + + if (!match) index++; + if (!attributeName && (key === "name" || (!match && index === 1))) return undefined; + if (!attributeType && (key === "type" || key === "attr_type" || (!match && index === 2))) return undefined; + } + + return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; +} + +function splitArgs(input: string): string[] { + const result: string[] = []; + let current = ""; + let depth = 0; + let quote = ""; + let triple = false; + let escaped = false; + + for (let i = 0; i < input.length; i++) { + const char = input[i]; + current += char; + + if (quote) { + if (escaped) escaped = false; + else if (char === "\\") escaped = true; + else if (triple && input.slice(i, i + 3) === quote.repeat(3)) { + current += input.slice(i + 1, i + 3); + i += 2; + quote = ""; + triple = false; + } else if (!triple && char === quote) quote = ""; + continue; + } + + if (char === "'" || char === '"') { + quote = char; + triple = input.slice(i, i + 3) === char.repeat(3); + if (triple) { + current += input.slice(i + 1, i + 3); + i += 2; + } + } else if ("([{".includes(char)) depth++; + else if (")]}".includes(char)) depth--; + else if (char === "," && depth === 0) { + result.push(current.slice(0, -1).trim()); + current = ""; + } + } + + const tail = current.trim(); + return tail ? [...result, tail] : result; +} + +function readString(input: string): string | undefined { + return input + .trim() + .match(/^[rRuU]*(?:"""([\s\S]*)"""|'''([\s\S]*)'''|"((?:\\.|[^"\\])*)"|'((?:\\.|[^'\\])*)')$/) + ?.slice(1) + .find(value => value !== undefined); +} + +function readType(input: string): AttributeType | undefined { + const token = input + .trim() + .replace(/\s+/g, "") + .match(/^AttributeType\.([A-Za-z_]\w*)$/)?.[1] + .toUpperCase(); + if (!token) { + return undefined; + } + + if (!JAVA_ATTRIBUTE_TYPE_NAME_SET.has(token) && !PYTHON_ATTRIBUTE_TYPE_NAME_SET.has(token)) { + return undefined; + } + + const canonical = TYPES[token as ParserAttributeTypeToken]; + return SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES.has(canonical) ? canonical : undefined; +} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts new file mode 100644 index 00000000000..de92e7a1293 --- /dev/null +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -0,0 +1,120 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; +import { PYTHON_UDF_V2_OP_TYPE } from "../workflow-graph/model/workflow-graph"; +import { UiUdfParameter, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { UiUdfParametersSyncService } from "./ui-udf-parameters-sync.service"; +import { vi } from "vitest"; + +describe("UiUdfParametersSyncService", () => { + const operatorId = "operator-1"; + const code = "self.UiParameter(...)"; + + let service: UiUdfParametersSyncService; + let workflowActionServiceSpy: { getTexeraGraph: ReturnType }; + let parserServiceSpy: { parse: ReturnType }; + let operator: { + operatorType: string; + operatorProperties: { uiParameters: any[] }; + }; + + beforeEach(() => { + operator = { + operatorType: PYTHON_UDF_V2_OP_TYPE, + operatorProperties: { uiParameters: [] }, + }; + + workflowActionServiceSpy = { + getTexeraGraph: vi.fn().mockReturnValue({ + getOperator: vi.fn().mockImplementation((id: string) => (id === operatorId ? operator : undefined)), + }), + }; + + parserServiceSpy = { parse: vi.fn() }; + + service = new UiUdfParametersSyncService( + workflowActionServiceSpy as unknown as WorkflowActionService, + parserServiceSpy as unknown as UiUdfParametersParserService + ); + }); + + it("should emit parameters that preserve values from current and legacy parameter names", () => { + operator.operatorProperties.uiParameters = [ + createParameter("count", "integer", "42"), + { attribute: { name: "legacyName", attributeType: "string" }, value: "saved" }, + ]; + parserServiceSpy.parse.mockReturnValue([ + createParameter("count", "integer"), + createParameter("legacyName", "string"), + ]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(nextSpy).toHaveBeenCalledWith({ + operatorId, + parameters: [createParameter("count", "integer", "42"), createParameter("legacyName", "string", "saved")], + }); + expect(nextSpy).toHaveBeenCalledOnce(); + }); + + it("should emit updated parameters with preserved values and removed stale parameters", () => { + operator.operatorProperties.uiParameters = [ + createParameter("count", "integer", "42"), + createParameter("removed", "string", "stale"), + ]; + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(nextSpy).toHaveBeenCalledWith({ + operatorId, + parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], + }); + expect(nextSpy).toHaveBeenCalledOnce(); + }); + + it("should not emit when the merged parameters are unchanged", () => { + operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(nextSpy).not.toHaveBeenCalled(); + }); +}); + +function createParameter(name: string, type: UiUdfParameter["attribute"]["attributeType"], value = ""): UiUdfParameter { + return { + attribute: { + attributeName: name, + attributeType: type, + }, + value, + }; +} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts new file mode 100644 index 00000000000..fbeb3695f52 --- /dev/null +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -0,0 +1,127 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Injectable } from "@angular/core"; +import { isEqual } from "lodash-es"; +import { ReplaySubject } from "rxjs"; +import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; +import { UiUdfParameter, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { isDefined } from "../../../common/util/predicate"; +import { + DUAL_INPUT_PORTS_PYTHON_UDF_V2_OP_TYPE, + PYTHON_UDF_SOURCE_V2_OP_TYPE, + PYTHON_UDF_V2_OP_TYPE, +} from "../workflow-graph/model/workflow-graph"; +import { YType } from "../../types/shared-editing.interface"; +import type { Text as YText } from "yjs"; + +@Injectable({ providedIn: "root" }) +export class UiUdfParametersSyncService { + private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>( + 1 + ); + + readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable(); + + constructor( + private workflowActionService: WorkflowActionService, + private uiUdfParametersParserService: UiUdfParametersParserService + ) {} + + /** + * Attach directly to YText and sync whenever it changes + */ + attachToYCode(operatorId: string, yCode: YText): () => void { + const handler = () => { + const latestCode = yCode.toString(); + this.syncStructureFromCode(operatorId, latestCode); + }; + + yCode.observe(handler); + + handler(); + + // return cleanup function + return () => yCode.unobserve(handler); + } + + syncStructureFromCode(operatorId: string, codeFromEditor?: string): void { + const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId); + + if (!operator || !this.isSupportedPythonUdfType(operator.operatorType)) { + return; + } + + const code = codeFromEditor ?? this.getSharedCode(operatorId); + if (!isDefined(code)) { + return; + } + + const existingParameters = operator.operatorProperties?.uiParameters ?? []; + const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); + + if (isEqual(existingParameters, mergedUiParameters)) { + return; + } + + // Emit event so UI updates + this.uiParametersChangedSubject.next({ + operatorId, + parameters: mergedUiParameters, + }); + } + + private buildParsedShapeWithPreservedValues(code: string, existingParameters: any[]): UiUdfParameter[] { + const parsedParameters = this.uiUdfParametersParserService.parse(code); + + const existingValues = new Map(); + existingParameters.forEach((parameter: any) => { + const parameterName = parameter?.attribute?.attributeName ?? parameter?.attribute?.name; + + if (isDefined(parameterName) && isDefined(parameter?.value)) { + existingValues.set(parameterName, parameter.value); + } + }); + + return parsedParameters.map(parameter => ({ + ...parameter, + value: existingValues.get(parameter.attribute.attributeName) ?? "", + })); + } + + private getSharedCode(operatorId: string): string | undefined { + try { + const sharedOperatorType = this.workflowActionService.getTexeraGraph().getSharedOperatorType(operatorId); + + const operatorProperties = sharedOperatorType.get("operatorProperties") as YType< + Readonly<{ [key: string]: any }> + >; + + const yCode = operatorProperties.get("code") as YText; + return yCode?.toString(); + } catch { + return undefined; + } + } + + private isSupportedPythonUdfType(operatorType: string): boolean { + return [PYTHON_UDF_V2_OP_TYPE, PYTHON_UDF_SOURCE_V2_OP_TYPE, DUAL_INPUT_PORTS_PYTHON_UDF_V2_OP_TYPE].includes( + operatorType + ); + } +} diff --git a/frontend/src/app/workspace/types/workflow-compiling.interface.ts b/frontend/src/app/workspace/types/workflow-compiling.interface.ts index 8c4499d3d4c..307bf792bed 100644 --- a/frontend/src/app/workspace/types/workflow-compiling.interface.ts +++ b/frontend/src/app/workspace/types/workflow-compiling.interface.ts @@ -71,7 +71,15 @@ export type CompilationStateInfo = Readonly< } >; // possible types of an attribute -export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types +export type AttributeType = + | "string" + | "integer" + | "double" + | "boolean" + | "long" + | "timestamp" + | "binary" + | "large_binary"; // schema: an array of attribute names and types export interface SchemaAttribute extends Readonly<{ attributeName: string; diff --git a/frontend/yarn.lock b/frontend/yarn.lock index 0dc28f51477..0e8c34febe3 100644 --- a/frontend/yarn.lock +++ b/frontend/yarn.lock @@ -3612,6 +3612,42 @@ __metadata: languageName: node linkType: hard +"@lezer/common@npm:^1.0.0, @lezer/common@npm:^1.2.0, @lezer/common@npm:^1.3.0": + version: 1.5.2 + resolution: "@lezer/common@npm:1.5.2" + checksum: 10c0/e39b46d74899409eab549df7942f00cd8c7f46c81ef0e2f079654ca96d262fca009927328bcd500d69270f5f09986e74768bed19c0acaadbd22f1a6c7dd9bd85 + languageName: node + linkType: hard + +"@lezer/highlight@npm:^1.0.0": + version: 1.2.3 + resolution: "@lezer/highlight@npm:1.2.3" + dependencies: + "@lezer/common": "npm:^1.3.0" + checksum: 10c0/3bcb4fce7a1a45b5973895d7cb2be47970a0098700f2a0970aef9878ffd37f540285a2d7388ec1f524726ec90cc5196b5701bbb9764b7e7300786d772b7d2ce2 + languageName: node + linkType: hard + +"@lezer/lr@npm:^1.0.0": + version: 1.4.10 + resolution: "@lezer/lr@npm:1.4.10" + dependencies: + "@lezer/common": "npm:^1.0.0" + checksum: 10c0/15fac0ecc02a57f111432808c89f7cfb9fed0d78d0a98742607acb5859220a9c18526dd6d509d9fe17f5ef762aa73ce29fa6b930739abb857c1df8949a9003ea + languageName: node + linkType: hard + +"@lezer/python@npm:1.1.18": + version: 1.1.18 + resolution: "@lezer/python@npm:1.1.18" + dependencies: + "@lezer/common": "npm:^1.2.0" + "@lezer/highlight": "npm:^1.0.0" + "@lezer/lr": "npm:^1.0.0" + checksum: 10c0/8d984729e887808c75800f18ed54560adfd4e67094b301a1666bdcd49e8987ab45f04c515563a92dfb1377d4a04dcf6616adc50a75285afe9ab53ab90f659bd5 + languageName: node + linkType: hard + "@listr2/prompt-adapter-inquirer@npm:3.0.5": version: 3.0.5 resolution: "@listr2/prompt-adapter-inquirer@npm:3.0.5" @@ -11029,6 +11065,7 @@ __metadata: "@codingame/monaco-vscode-java-default-extension": "npm:8.0.4" "@codingame/monaco-vscode-python-default-extension": "npm:8.0.4" "@codingame/monaco-vscode-r-default-extension": "npm:8.0.4" + "@lezer/python": "npm:1.1.18" "@ngneat/until-destroy": "npm:8.1.4" "@ngx-formly/core": "npm:6.3.12" "@ngx-formly/ng-zorro-antd": "npm:6.3.12" From f009a3676b3cf2478f919ef253d46d701aa8e370 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 12 May 2026 22:04:16 -0600 Subject: [PATCH 02/13] fix(frontend): reject large binary UI parameters locally --- .../code-editor/ui-udf-parameters-parser.service.ts | 12 ++++++++---- .../workspace/types/workflow-compiling.interface.ts | 10 +--------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 86d73da8d94..f5e1c52a57b 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -57,9 +57,11 @@ const PYTHON_ATTRIBUTE_TYPE_NAMES = [ type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number]; type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName; +type UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY"; +type SupportedParserAttributeTypeToken = Exclude; type ParserSyntaxNode = ReturnType["topNode"]; -const TYPES: Readonly> = { +const TYPES: Readonly> = { STRING: "string", INTEGER: "integer", INT: "integer", @@ -68,8 +70,6 @@ const TYPES: Readonly> = { BOOLEAN: "boolean", BOOL: "boolean", TIMESTAMP: "timestamp", - BINARY: "binary", - LARGE_BINARY: "large_binary", }; const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set(JAVA_ATTRIBUTE_TYPE_NAMES); @@ -208,6 +208,10 @@ function readType(input: string): AttributeType | undefined { return undefined; } - const canonical = TYPES[token as ParserAttributeTypeToken]; + if (token === "BINARY" || token === "LARGE_BINARY") { + return undefined; + } + + const canonical = TYPES[token as SupportedParserAttributeTypeToken]; return SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES.has(canonical) ? canonical : undefined; } diff --git a/frontend/src/app/workspace/types/workflow-compiling.interface.ts b/frontend/src/app/workspace/types/workflow-compiling.interface.ts index 307bf792bed..8c4499d3d4c 100644 --- a/frontend/src/app/workspace/types/workflow-compiling.interface.ts +++ b/frontend/src/app/workspace/types/workflow-compiling.interface.ts @@ -71,15 +71,7 @@ export type CompilationStateInfo = Readonly< } >; // possible types of an attribute -export type AttributeType = - | "string" - | "integer" - | "double" - | "boolean" - | "long" - | "timestamp" - | "binary" - | "large_binary"; // schema: an array of attribute names and types +export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types export interface SchemaAttribute extends Readonly<{ attributeName: string; From 99b3a37a366228ec68deb768d6a017b0aad8d336 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 12 May 2026 22:33:14 -0600 Subject: [PATCH 03/13] test(frontend): cover multiline UI parameter parsing --- .../ui-udf-parameters-parser.service.spec.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index d0a887bc61f..f186931e262 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -58,6 +58,29 @@ describe("UiUdfParametersParserService", () => { ]); }); + it("should parse multiline UiParameter calls with split named arguments", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter( + name= + "threshold", + type= + AttributeType.DOUBLE, + ) + self.UiParameter( + "label", + type= + AttributeType.STRING, + ) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, + ]); + }); + it("should ignore calls where name or type is missing", () => { const code = ` class ProcessTupleOperator(UDFOperatorV2): @@ -115,6 +138,25 @@ describe("UiUdfParametersParserService", () => { ]); }); + it("should ignore commented out multiline UiParameter sections", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + # self.UiParameter( + # name="commented", + # type=AttributeType.INT, + # ) + self.UiParameter( + name="active", + type=AttributeType.STRING, + ) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "active", attributeType: "string" }, value: "" }, + ]); + }); + it("should ignore UiParameter examples inside triple-quoted strings", () => { const code = ` class ProcessTupleOperator(UDFOperatorV2): From 8cad906bd90eef666f2a763c4305610c2a5bf13e Mon Sep 17 00:00:00 2001 From: carloea2 Date: Mon, 18 May 2026 22:58:19 -0600 Subject: [PATCH 04/13] fix(frontend): address UI parameter review feedback --- .../ui-udf-parameters.component.spec.ts | 69 +++++++++ .../ui-udf-parameters.component.ts | 54 +++---- .../ui-udf-parameters-parser.service.spec.ts | 66 ++++++++- .../ui-udf-parameters-parser.service.ts | 138 ++++++++++-------- .../ui-udf-parameters-sync.service.spec.ts | 120 +++++++++++++-- .../ui-udf-parameters-sync.service.ts | 48 +++--- .../types/workflow-compiling.interface.ts | 6 + 7 files changed, 377 insertions(+), 124 deletions(-) create mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts new file mode 100644 index 00000000000..7cb209ad349 --- /dev/null +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { FormControl } from "@angular/forms"; +import { FormlyFieldConfig } from "@ngx-formly/core"; +import { UiUdfParametersComponent } from "./ui-udf-parameters.component"; + +describe("UiUdfParametersComponent", () => { + let component: UiUdfParametersComponent; + + beforeEach(() => { + component = new UiUdfParametersComponent(); + }); + + it("should disable name and type fields while leaving value editable", () => { + const valueControl = new FormControl({ value: "42", disabled: true }); + const nameControl = new FormControl("threshold"); + const typeControl = new FormControl("double"); + + const valueField: FormlyFieldConfig = { key: "value", formControl: valueControl }; + const nameField: FormlyFieldConfig = { key: "attributeName", formControl: nameControl }; + const typeField: FormlyFieldConfig = { key: "attributeType", formControl: typeControl }; + const rowField: FormlyFieldConfig = { + fieldGroup: [ + valueField, + { + key: "attribute", + fieldGroup: [nameField, typeField], + }, + ], + }; + + (component as any).field = { fieldGroup: [rowField] } as FormlyFieldConfig; + + component.ngOnInit(); + + expect(component.getValueField(rowField)).toBe(valueField); + expect(component.getNameField(rowField)).toBe(nameField); + expect(component.getTypeField(rowField)).toBe(typeField); + + expect(valueField.props?.disabled).toBe(false); + expect((valueField as any).templateOptions?.disabled).toBe(false); + expect(valueControl.enabled).toBe(true); + + expect(nameField.props?.disabled).toBe(true); + expect((nameField as any).templateOptions?.disabled).toBe(true); + expect(nameControl.disabled).toBe(true); + + expect(typeField.props?.disabled).toBe(true); + expect((typeField as any).templateOptions?.disabled).toBe(true); + expect(typeControl.disabled).toBe(true); + }); +}); diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index cabaef3604f..31335363843 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Component } from "@angular/core"; +import { Component, OnInit } from "@angular/core"; import { NgFor, NgIf } from "@angular/common"; import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; @@ -26,7 +26,15 @@ import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/cor styleUrls: ["./ui-udf-parameters.component.scss"], imports: [NgIf, NgFor, FormlyModule], }) -export class UiUdfParametersComponent extends FieldArrayType { +export class UiUdfParametersComponent extends FieldArrayType implements OnInit { + ngOnInit(): void { + this.field.fieldGroup?.forEach(rowField => { + this.configureDisabledState(this.getAttributeChild(rowField, "attributeName"), true); + this.configureDisabledState(this.getAttributeChild(rowField, "attributeType"), true); + this.configureDisabledState(this.getField(rowField, "value"), false); + }); + } + private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { return rowField.fieldGroup?.find(f => f.key === key); } @@ -36,55 +44,49 @@ export class UiUdfParametersComponent extends FieldArrayType { return attributeGroup?.fieldGroup?.find(f => f.key === childKey); } - private setDisabled(field: FormlyFieldConfig | undefined, disabled: boolean): FormlyFieldConfig | undefined { - if (!field) return undefined; + private configureDisabledState(field: FormlyFieldConfig | undefined, disabled: boolean): void { + if (!field) return; - // 1) Modern Formly field.props = { ...(field.props ?? {}), disabled }; - // 2) Compatibility for templates/wrappers still using templateOptions // (`as any` so we don't get nagged by the @deprecated JSDoc) (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; - // 3) Enforce at the reactive form level + const prevOnInit = field.hooks?.onInit; + field.hooks = { + ...(field.hooks ?? {}), + onInit: f => { + prevOnInit?.(f); + this.applyDisabledState(f, disabled); + }, + }; + + this.applyDisabledState(field, disabled); + } + + private applyDisabledState(field: FormlyFieldConfig, disabled: boolean): void { if (field.formControl) { if (disabled) { field.formControl.disable({ emitEvent: false }); } else { field.formControl.enable({ emitEvent: false }); } - } else { - // If control isn't created yet, disable it at init time. - const prevOnInit = field.hooks?.onInit; - field.hooks = { - ...(field.hooks ?? {}), - onInit: f => { - prevOnInit?.(f); - if (disabled) { - f.formControl?.disable({ emitEvent: false }); - } else { - f.formControl?.enable({ emitEvent: false }); - } - }, - }; } - - return field; } // Disable Name getNameField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.setDisabled(this.getAttributeChild(rowField, "attributeName"), true); + return this.getAttributeChild(rowField, "attributeName"); } // Disable Type getTypeField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.setDisabled(this.getAttributeChild(rowField, "attributeType"), true); + return this.getAttributeChild(rowField, "attributeType"); } // Value editable getValueField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.setDisabled(this.getField(rowField, "value"), false); + return this.getField(rowField, "value"); } trackByParamName = (index: number, param: any): string | number => { diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index f186931e262..fa21e88ffcb 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -58,7 +58,7 @@ describe("UiUdfParametersParserService", () => { ]); }); - it("should parse multiline UiParameter calls with split named arguments", () => { + it("should parse multiline UiParameter calls with named arguments split across lines", () => { const code = ` class ProcessTupleOperator(UDFOperatorV2): def open(self): @@ -125,6 +125,70 @@ describe("UiUdfParametersParserService", () => { expect(service.parse(code)).toEqual([]); }); + it("should ignore custom-named subclasses because injection targets template class names", () => { + const code = ` + class MyTupleOp(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) + + class MyWrappedTupleOp(ProcessTupleOperator): + def open(self): + self.UiParameter("label", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should parse supported UiParameter calls across multiple classes", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) + + class RandomClass(ABC): + def open(self): + self.UiParameter("ignored", AttributeType.STRING) + + class GenerateOperator(UDFSourceOperator): + def open(self): + self.UiParameter(name="batch_size", type=AttributeType.INT) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "batch_size", attributeType: "integer" }, value: "" }, + ]); + }); + + it("should ignore empty and extra positional arguments", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter() + self.UiParameter("too_many", AttributeType.STRING, "extra") + self.UiParameter("valid", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, + ]); + }); + + it("should keep the first duplicate parameter name", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) + self.UiParameter("threshold", AttributeType.STRING) + self.UiParameter("label", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, + ]); + }); + it("should ignore commented out UiParameter calls", () => { const code = ` class ProcessTupleOperator(UDFOperatorV2): diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index f5e1c52a57b..e63d2e044a9 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -19,13 +19,9 @@ import { Injectable } from "@angular/core"; import { parser } from "@lezer/python"; -import { AttributeType, SchemaAttribute } from "../../types/workflow-compiling.interface"; - -export interface UiUdfParameter { - attribute: SchemaAttribute; - value: string; -} +import { AttributeType, UiUdfParameter } from "../../types/workflow-compiling.interface"; +// Keep in sync with Python UDF template class names in PythonUDFOpDescV2, DualInputPortsPythonUDFOpDescV2, and PythonUDFSourceOpDescV2. const CLASSES = new Set(["ProcessTupleOperator", "ProcessBatchOperator", "ProcessTableOperator", "GenerateOperator"]); // Java enum constant names (AttributeType.java) @@ -60,6 +56,8 @@ type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName; type UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY"; type SupportedParserAttributeTypeToken = Exclude; type ParserSyntaxNode = ReturnType["topNode"]; +type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; +type UiParameterArgument = Readonly<{ kind: "name"; value: string }> | Readonly<{ kind: "type"; value: AttributeType }>; const TYPES: Readonly> = { STRING: "string", @@ -102,9 +100,11 @@ export class UiUdfParametersParserService { enter: ({ name, node }) => { const className = node.getChild("VariableName"); if (name !== "ClassDefinition" || !className || !CLASSES.has(code.slice(className.from, className.to))) return; - node - .cursor() - .iterate(ref => (ref.name === "CallExpression" ? (add(readCall(ref.node, code)), false) : undefined)); + node.cursor().iterate(ref => { + if (ref.name !== "CallExpression") return; + add(readCall(ref.node, code)); + return false; + }); return false; }, }); @@ -119,71 +119,91 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi let attributeName: string | undefined; let attributeType: AttributeType | undefined; - let index = 0; + let positionalIndex = 0; let sawNamed = false; - for (const arg of splitArgs(code.slice(args.from + 1, args.to - 1))) { - const match = arg.match(/^([A-Za-z_]\w*)\s*=\s*([\s\S]+)$/); - const key = match?.[1]; - const value = match?.[2] ?? arg; + for (const arg of readArguments(args, code)) { + if (arg.key) { + sawNamed = true; + } else if (sawNamed) { + return undefined; + } - if (match) sawNamed = true; - else if (sawNamed || index > 1) return undefined; + const parsedArg = readArgument(arg, positionalIndex, code); + if (!parsedArg) { + return undefined; + } - if ((match ? key === "name" : index === 0) && !attributeName) attributeName = readString(value)?.trim(); - else if ((match ? key === "type" || key === "attr_type" : index === 1) && !attributeType) - attributeType = readType(value); - else return undefined; + if (parsedArg.kind === "name") { + if (attributeName) { + return undefined; + } + attributeName = parsedArg.value; + } else { + if (attributeType) { + return undefined; + } + attributeType = parsedArg.value; + } - if (!match) index++; - if (!attributeName && (key === "name" || (!match && index === 1))) return undefined; - if (!attributeType && (key === "type" || key === "attr_type" || (!match && index === 2))) return undefined; + if (!arg.key) { + positionalIndex++; + } } return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; } -function splitArgs(input: string): string[] { - const result: string[] = []; - let current = ""; - let depth = 0; - let quote = ""; - let triple = false; - let escaped = false; - - for (let i = 0; i < input.length; i++) { - const char = input[i]; - current += char; - - if (quote) { - if (escaped) escaped = false; - else if (char === "\\") escaped = true; - else if (triple && input.slice(i, i + 3) === quote.repeat(3)) { - current += input.slice(i + 1, i + 3); - i += 2; - quote = ""; - triple = false; - } else if (!triple && char === quote) quote = ""; - continue; - } +function readArguments(args: ParserSyntaxNode, code: string): ParsedArgument[] { + const result: ParsedArgument[] = []; + const children = getChildren(args).filter(node => !["(", ")", ","].includes(node.name)); + + for (let index = 0; index < children.length; index++) { + const node = children[index]; - if (char === "'" || char === '"') { - quote = char; - triple = input.slice(i, i + 3) === char.repeat(3); - if (triple) { - current += input.slice(i + 1, i + 3); - i += 2; + if (node.name === "VariableName" && children[index + 1]?.name === "AssignOp") { + const value = children[index + 2]; + if (!value) { + return []; } - } else if ("([{".includes(char)) depth++; - else if (")]}".includes(char)) depth--; - else if (char === "," && depth === 0) { - result.push(current.slice(0, -1).trim()); - current = ""; + result.push({ key: code.slice(node.from, node.to), value }); + index += 2; + } else if (node.name !== "AssignOp") { + result.push({ value: node }); + } else { + return []; } } - const tail = current.trim(); - return tail ? [...result, tail] : result; + return result; +} + +function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { + const children: ParserSyntaxNode[] = []; + for (let child = node.firstChild; child; child = child.nextSibling) { + children.push(child); + } + return children; +} + +function readArgument(arg: ParsedArgument, positionalIndex: number, code: string): UiParameterArgument | undefined { + const key = arg.key; + const value = code.slice(arg.value.from, arg.value.to); + + if ((key === "name" || (!key && positionalIndex === 0)) && arg.value.name === "String") { + const name = readString(value)?.trim(); + return name ? { kind: "name", value: name } : undefined; + } + + if ( + (key === "type" || key === "attr_type" || (!key && positionalIndex === 1)) && + arg.value.name === "MemberExpression" + ) { + const type = readType(value); + return type ? { kind: "type", value: type } : undefined; + } + + return undefined; } function readString(input: string): string | undefined { diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts index de92e7a1293..beb7fa15adb 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -19,9 +19,11 @@ import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { PYTHON_UDF_V2_OP_TYPE } from "../workflow-graph/model/workflow-graph"; -import { UiUdfParameter, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; import { UiUdfParametersSyncService } from "./ui-udf-parameters-sync.service"; +import { UiUdfParameter } from "../../types/workflow-compiling.interface"; import { vi } from "vitest"; +import * as Y from "yjs"; describe("UiUdfParametersSyncService", () => { const operatorId = "operator-1"; @@ -30,9 +32,13 @@ describe("UiUdfParametersSyncService", () => { let service: UiUdfParametersSyncService; let workflowActionServiceSpy: { getTexeraGraph: ReturnType }; let parserServiceSpy: { parse: ReturnType }; + let graphSpy: { + getOperator: ReturnType; + getSharedOperatorType: ReturnType; + }; let operator: { operatorType: string; - operatorProperties: { uiParameters: any[] }; + operatorProperties: { uiParameters: UiUdfParameter[] }; }; beforeEach(() => { @@ -41,10 +47,13 @@ describe("UiUdfParametersSyncService", () => { operatorProperties: { uiParameters: [] }, }; + graphSpy = { + getOperator: vi.fn().mockImplementation((id: string) => (id === operatorId ? operator : undefined)), + getSharedOperatorType: vi.fn(), + }; + workflowActionServiceSpy = { - getTexeraGraph: vi.fn().mockReturnValue({ - getOperator: vi.fn().mockImplementation((id: string) => (id === operatorId ? operator : undefined)), - }), + getTexeraGraph: vi.fn().mockReturnValue(graphSpy), }; parserServiceSpy = { parse: vi.fn() }; @@ -55,15 +64,9 @@ describe("UiUdfParametersSyncService", () => { ); }); - it("should emit parameters that preserve values from current and legacy parameter names", () => { - operator.operatorProperties.uiParameters = [ - createParameter("count", "integer", "42"), - { attribute: { name: "legacyName", attributeType: "string" }, value: "saved" }, - ]; - parserServiceSpy.parse.mockReturnValue([ - createParameter("count", "integer"), - createParameter("legacyName", "string"), - ]); + it("should emit parameters that preserve values from current parameter names", () => { + operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); const nextSpy = vi.fn(); service.uiParametersChanged$.subscribe(nextSpy); @@ -72,7 +75,7 @@ describe("UiUdfParametersSyncService", () => { expect(nextSpy).toHaveBeenCalledWith({ operatorId, - parameters: [createParameter("count", "integer", "42"), createParameter("legacyName", "string", "saved")], + parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], }); expect(nextSpy).toHaveBeenCalledOnce(); }); @@ -107,6 +110,71 @@ describe("UiUdfParametersSyncService", () => { expect(nextSpy).not.toHaveBeenCalled(); }); + + it("should not parse code for non-Python UDF operators", () => { + operator.operatorType = "Projection"; + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(parserServiceSpy.parse).not.toHaveBeenCalled(); + expect(nextSpy).not.toHaveBeenCalled(); + }); + + it("should read code from the shared operator property when editor code is omitted", () => { + const sharedCode = 'self.UiParameter("count", AttributeType.INT)'; + graphSpy.getSharedOperatorType.mockReturnValue(createSharedOperatorType(sharedCode)); + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId); + + expect(parserServiceSpy.parse).toHaveBeenCalledWith(sharedCode); + expect(nextSpy).toHaveBeenCalledWith({ + operatorId, + parameters: [createParameter("count", "integer")], + }); + }); + + it("should debounce YText changes and clean up the observer", () => { + vi.useFakeTimers(); + try { + const yCode = createYText('self.UiParameter("count", AttributeType.INT)'); + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + const cleanup = service.attachToYCode(operatorId, yCode); + + expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); + expect(nextSpy).toHaveBeenCalledOnce(); + + yCode.insert(yCode.length, "\n# changed"); + + vi.advanceTimersByTime(199); + + expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); + + vi.advanceTimersByTime(1); + + expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); + expect(nextSpy).toHaveBeenCalledTimes(2); + + cleanup(); + yCode.insert(yCode.length, "\n# after cleanup"); + vi.advanceTimersByTime(200); + + expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); + expect(nextSpy).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); }); function createParameter(name: string, type: UiUdfParameter["attribute"]["attributeType"], value = ""): UiUdfParameter { @@ -118,3 +186,25 @@ function createParameter(name: string, type: UiUdfParameter["attribute"]["attrib value, }; } + +function createSharedOperatorType(code: string): Y.Map { + const doc = new Y.Doc(); + const sharedOperator = doc.getMap("operator"); + const operatorProperties = new Y.Map(); + const yCode = new Y.Text(); + + operatorProperties.set("code", yCode); + sharedOperator.set("operatorProperties", operatorProperties); + yCode.insert(0, code); + + return sharedOperator; +} + +function createYText(text: string): Y.Text { + const doc = new Y.Doc(); + const yMap = doc.getMap("root"); + const yText = new Y.Text(); + yMap.set("code", yText); + yText.insert(0, text); + return yText; +} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts index fbeb3695f52..0e45e5138af 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -18,17 +18,17 @@ */ import { Injectable } from "@angular/core"; import { isEqual } from "lodash-es"; -import { ReplaySubject } from "rxjs"; +import { ReplaySubject, Subject } from "rxjs"; +import { debounceTime } from "rxjs/operators"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; -import { UiUdfParameter, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; import { isDefined } from "../../../common/util/predicate"; -import { - DUAL_INPUT_PORTS_PYTHON_UDF_V2_OP_TYPE, - PYTHON_UDF_SOURCE_V2_OP_TYPE, - PYTHON_UDF_V2_OP_TYPE, -} from "../workflow-graph/model/workflow-graph"; +import { isPythonUdf } from "../workflow-graph/model/workflow-graph"; import { YType } from "../../types/shared-editing.interface"; import type { Text as YText } from "yjs"; +import { UiUdfParameter } from "../../types/workflow-compiling.interface"; + +const UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS = 200; @Injectable({ providedIn: "root" }) export class UiUdfParametersSyncService { @@ -47,23 +47,31 @@ export class UiUdfParametersSyncService { * Attach directly to YText and sync whenever it changes */ attachToYCode(operatorId: string, yCode: YText): () => void { - const handler = () => { - const latestCode = yCode.toString(); + const codeChanges = new Subject(); + const subscription = codeChanges.pipe(debounceTime(UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS)).subscribe(latestCode => { this.syncStructureFromCode(operatorId, latestCode); + }); + + const handler = () => { + codeChanges.next(yCode.toString()); }; yCode.observe(handler); - handler(); + this.syncStructureFromCode(operatorId, yCode.toString()); // return cleanup function - return () => yCode.unobserve(handler); + return () => { + yCode.unobserve(handler); + subscription.unsubscribe(); + codeChanges.complete(); + }; } syncStructureFromCode(operatorId: string, codeFromEditor?: string): void { const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId); - if (!operator || !this.isSupportedPythonUdfType(operator.operatorType)) { + if (!operator || !isPythonUdf(operator)) { return; } @@ -72,7 +80,7 @@ export class UiUdfParametersSyncService { return; } - const existingParameters = operator.operatorProperties?.uiParameters ?? []; + const existingParameters = (operator.operatorProperties?.uiParameters ?? []) as UiUdfParameter[]; const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); if (isEqual(existingParameters, mergedUiParameters)) { @@ -86,14 +94,14 @@ export class UiUdfParametersSyncService { }); } - private buildParsedShapeWithPreservedValues(code: string, existingParameters: any[]): UiUdfParameter[] { + private buildParsedShapeWithPreservedValues(code: string, existingParameters: UiUdfParameter[]): UiUdfParameter[] { const parsedParameters = this.uiUdfParametersParserService.parse(code); const existingValues = new Map(); - existingParameters.forEach((parameter: any) => { - const parameterName = parameter?.attribute?.attributeName ?? parameter?.attribute?.name; + existingParameters.forEach(parameter => { + const parameterName = parameter.attribute.attributeName; - if (isDefined(parameterName) && isDefined(parameter?.value)) { + if (isDefined(parameterName) && isDefined(parameter.value)) { existingValues.set(parameterName, parameter.value); } }); @@ -118,10 +126,4 @@ export class UiUdfParametersSyncService { return undefined; } } - - private isSupportedPythonUdfType(operatorType: string): boolean { - return [PYTHON_UDF_V2_OP_TYPE, PYTHON_UDF_SOURCE_V2_OP_TYPE, DUAL_INPUT_PORTS_PYTHON_UDF_V2_OP_TYPE].includes( - operatorType - ); - } } diff --git a/frontend/src/app/workspace/types/workflow-compiling.interface.ts b/frontend/src/app/workspace/types/workflow-compiling.interface.ts index 8c4499d3d4c..d9f21a20341 100644 --- a/frontend/src/app/workspace/types/workflow-compiling.interface.ts +++ b/frontend/src/app/workspace/types/workflow-compiling.interface.ts @@ -78,6 +78,12 @@ export interface SchemaAttribute attributeType: AttributeType; }> {} +export interface UiUdfParameter + extends Readonly<{ + attribute: SchemaAttribute; + value: string; + }> {} + export type PortSchema = ReadonlyArray; // schema of an operator: a map from serialized PortIdentity to port schema From 99bb449c31bce1e9c9b9feeda7f6e2c03ae774b8 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Mon, 18 May 2026 23:33:39 -0600 Subject: [PATCH 05/13] refactor(frontend): compact UI parameter foundation --- .../ui-udf-parameters.component.html | 55 ---- .../ui-udf-parameters.component.scss | 39 --- .../ui-udf-parameters.component.spec.ts | 56 +--- .../ui-udf-parameters.component.ts | 78 ++--- .../ui-udf-parameters-parser.service.spec.ts | 281 ++++-------------- .../ui-udf-parameters-parser.service.ts | 145 ++------- .../ui-udf-parameters-sync.service.spec.ts | 214 ++++--------- .../ui-udf-parameters-sync.service.ts | 72 ++--- 8 files changed, 203 insertions(+), 737 deletions(-) delete mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html delete mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html deleted file mode 100644 index 0ecfb16a780..00000000000 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html +++ /dev/null @@ -1,55 +0,0 @@ - -
- -
-
Value
-
Name
-
Type
-
- -
- - -
- - - -
- - -
- - - -
- - -
- - - -
- -
-
diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss deleted file mode 100644 index 901edaf1fc9..00000000000 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss +++ /dev/null @@ -1,39 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -.ui-udf-param-row { - display: grid; - grid-template-columns: 250px 250px 1fr; - gap: 12px; - align-items: start; -} - -.field-cell { - min-width: 0; -} - -/* Remove Formly/Ant label spacing */ -:host ::ng-deep .ant-form-item { - margin-bottom: 0; -} - -/* Hide Formly labels*/ -:host ::ng-deep .ant-form-item-label { - display: none; -} diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts index 7cb209ad349..a79b2d67281 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts @@ -16,54 +16,28 @@ * specific language governing permissions and limitations * under the License. */ - import { FormControl } from "@angular/forms"; import { FormlyFieldConfig } from "@ngx-formly/core"; import { UiUdfParametersComponent } from "./ui-udf-parameters.component"; describe("UiUdfParametersComponent", () => { - let component: UiUdfParametersComponent; - - beforeEach(() => { - component = new UiUdfParametersComponent(); - }); - - it("should disable name and type fields while leaving value editable", () => { - const valueControl = new FormControl({ value: "42", disabled: true }); - const nameControl = new FormControl("threshold"); - const typeControl = new FormControl("double"); - - const valueField: FormlyFieldConfig = { key: "value", formControl: valueControl }; - const nameField: FormlyFieldConfig = { key: "attributeName", formControl: nameControl }; - const typeField: FormlyFieldConfig = { key: "attributeType", formControl: typeControl }; - const rowField: FormlyFieldConfig = { - fieldGroup: [ - valueField, - { - key: "attribute", - fieldGroup: [nameField, typeField], - }, - ], + it("should lock inferred fields and keep value editable", () => { + const component = new UiUdfParametersComponent(); + const value = field("value", new FormControl({ value: "42", disabled: true })); + const name = field("attributeName", new FormControl("threshold")); + const type = field("attributeType", new FormControl("double")); + + (component as any).field = { + fieldGroup: [{ fieldGroup: [value, { key: "attribute", fieldGroup: [name, type] }] }], }; - - (component as any).field = { fieldGroup: [rowField] } as FormlyFieldConfig; - component.ngOnInit(); - expect(component.getValueField(rowField)).toBe(valueField); - expect(component.getNameField(rowField)).toBe(nameField); - expect(component.getTypeField(rowField)).toBe(typeField); - - expect(valueField.props?.disabled).toBe(false); - expect((valueField as any).templateOptions?.disabled).toBe(false); - expect(valueControl.enabled).toBe(true); - - expect(nameField.props?.disabled).toBe(true); - expect((nameField as any).templateOptions?.disabled).toBe(true); - expect(nameControl.disabled).toBe(true); - - expect(typeField.props?.disabled).toBe(true); - expect((typeField as any).templateOptions?.disabled).toBe(true); - expect(typeControl.disabled).toBe(true); + expect([value.props?.disabled, value.formControl?.enabled]).toEqual([false, true]); + expect([name.props?.disabled, name.formControl?.disabled]).toEqual([true, true]); + expect([type.props?.disabled, type.formControl?.disabled]).toEqual([true, true]); }); }); + +function field(key: string, formControl: FormControl): FormlyFieldConfig { + return { key, formControl }; +} diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index 31335363843..050f7031d78 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -16,80 +16,40 @@ * specific language governing permissions and limitations * under the License. */ +import { NgFor } from "@angular/common"; import { Component, OnInit } from "@angular/core"; -import { NgFor, NgIf } from "@angular/common"; import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; +const COLUMN_KEYS = ["value", "attributeName", "attributeType"] as const; + @Component({ selector: "texera-ui-udf-parameters", - templateUrl: "./ui-udf-parameters.component.html", - styleUrls: ["./ui-udf-parameters.component.scss"], - imports: [NgIf, NgFor, FormlyModule], + template: + '
', + styles: [ + ".ui-udf-param-row{display:grid;grid-template-columns:minmax(160px,250px) minmax(160px,250px) minmax(120px,1fr);gap:12px}:host ::ng-deep .ant-form-item{margin-bottom:0}:host ::ng-deep .ant-form-item-label{display:none}", + ], + imports: [NgFor, FormlyModule], }) export class UiUdfParametersComponent extends FieldArrayType implements OnInit { ngOnInit(): void { - this.field.fieldGroup?.forEach(rowField => { - this.configureDisabledState(this.getAttributeChild(rowField, "attributeName"), true); - this.configureDisabledState(this.getAttributeChild(rowField, "attributeType"), true); - this.configureDisabledState(this.getField(rowField, "value"), false); - }); + this.field.fieldGroup?.forEach(rowField => + COLUMN_KEYS.forEach(key => this.setDisabled(this.child(rowField, key), key !== "value")) + ); } - private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { - return rowField.fieldGroup?.find(f => f.key === key); + displayFields(rowField: FormlyFieldConfig): FormlyFieldConfig[] { + return COLUMN_KEYS.map(key => this.child(rowField, key)).filter((field): field is FormlyFieldConfig => !!field); } - private getAttributeChild(rowField: FormlyFieldConfig, childKey: string): FormlyFieldConfig | undefined { - const attributeGroup = this.getField(rowField, "attribute"); - return attributeGroup?.fieldGroup?.find(f => f.key === childKey); + private child(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { + if (key === "value") return rowField.fieldGroup?.find(field => field.key === key); + return rowField.fieldGroup?.find(field => field.key === "attribute")?.fieldGroup?.find(field => field.key === key); } - private configureDisabledState(field: FormlyFieldConfig | undefined, disabled: boolean): void { + private setDisabled(field: FormlyFieldConfig | undefined, disabled: boolean): void { if (!field) return; - field.props = { ...(field.props ?? {}), disabled }; - - // (`as any` so we don't get nagged by the @deprecated JSDoc) - (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; - - const prevOnInit = field.hooks?.onInit; - field.hooks = { - ...(field.hooks ?? {}), - onInit: f => { - prevOnInit?.(f); - this.applyDisabledState(f, disabled); - }, - }; - - this.applyDisabledState(field, disabled); - } - - private applyDisabledState(field: FormlyFieldConfig, disabled: boolean): void { - if (field.formControl) { - if (disabled) { - field.formControl.disable({ emitEvent: false }); - } else { - field.formControl.enable({ emitEvent: false }); - } - } + field.formControl?.[disabled ? "disable" : "enable"]({ emitEvent: false }); } - - // Disable Name - getNameField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.getAttributeChild(rowField, "attributeName"); - } - - // Disable Type - getTypeField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.getAttributeChild(rowField, "attributeType"); - } - - // Value editable - getValueField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.getField(rowField, "value"); - } - - trackByParamName = (index: number, param: any): string | number => { - return param?.attribute?.attributeName ?? index; - }; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index fa21e88ffcb..2249dd88fad 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -26,224 +26,65 @@ describe("UiUdfParametersParserService", () => { service = new UiUdfParametersParserService(); }); - it("should parse Python-compatible positional and name-based arguments", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("count", AttributeType.INT) - self.UiParameter(type=AttributeType.STRING, name="name") - self.UiParameter(name="age", type=AttributeType.LONG) - self.UiParameter("score", AttributeType.DOUBLE) - self.UiParameter("created_at", type=AttributeType.TIMESTAMP) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, - { attribute: { attributeName: "name", attributeType: "string" }, value: "" }, - { attribute: { attributeName: "age", attributeType: "long" }, value: "" }, - { attribute: { attributeName: "score", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "created_at", attributeType: "timestamp" }, value: "" }, - ]); - }); - - it("should parse the attr_type keyword used by the Python API", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("count", attr_type=AttributeType.INT) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, - ]); - }); - - it("should parse multiline UiParameter calls with named arguments split across lines", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter( - name= - "threshold", - type= - AttributeType.DOUBLE, - ) - self.UiParameter( - "label", - type= - AttributeType.STRING, - ) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, - ]); - }); - - it("should ignore calls where name or type is missing", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter(name="a") - self.UiParameter(type=AttributeType.DOUBLE) - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should ignore invalid positional argument ordering", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter(AttributeType.INT, "count") - self.UiParameter(name="valid", type=AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, - ]); - }); - - it("should ignore legacy key= named argument", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter(type=AttributeType.DOUBLE, key="a") - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should ignore unsupported classes", () => { - const code = ` - class RandomClass(ABC): - def open(self): - self.UiParameter(type=AttributeType.DOUBLE, name="a") - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should ignore custom-named subclasses because injection targets template class names", () => { - const code = ` - class MyTupleOp(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - - class MyWrappedTupleOp(ProcessTupleOperator): - def open(self): - self.UiParameter("label", AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should parse supported UiParameter calls across multiple classes", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - - class RandomClass(ABC): - def open(self): - self.UiParameter("ignored", AttributeType.STRING) - - class GenerateOperator(UDFSourceOperator): - def open(self): - self.UiParameter(name="batch_size", type=AttributeType.INT) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "batch_size", attributeType: "integer" }, value: "" }, - ]); - }); - - it("should ignore empty and extra positional arguments", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter() - self.UiParameter("too_many", AttributeType.STRING, "extra") - self.UiParameter("valid", AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, - ]); - }); - - it("should keep the first duplicate parameter name", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - self.UiParameter("threshold", AttributeType.STRING) - self.UiParameter("label", AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, - ]); - }); - - it("should ignore commented out UiParameter calls", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - # self.UiParameter("commented", AttributeType.INT) - self.UiParameter("active", AttributeType.INT) # self.UiParameter("trailing", AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "active", attributeType: "integer" }, value: "" }, - ]); - }); - - it("should ignore commented out multiline UiParameter sections", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - # self.UiParameter( - # name="commented", - # type=AttributeType.INT, - # ) - self.UiParameter( - name="active", - type=AttributeType.STRING, - ) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "active", attributeType: "string" }, value: "" }, - ]); - }); - - it("should ignore UiParameter examples inside triple-quoted strings", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - """ - self.UiParameter("example", AttributeType.INT) - """ - self.UiParameter("active", AttributeType.DOUBLE) - `; - - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "active", attributeType: "double" }, value: "" }, - ]); - }); - - it("should reject binary UiParameter types", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("payload", AttributeType.BINARY) - self.UiParameter("blob", AttributeType.LARGE_BINARY) - `; - - expect(service.parse(code)).toEqual([]); - }); + it.each([ + [ + [ + 'self.UiParameter("count", AttributeType.INT); self.UiParameter(type=AttributeType.STRING, name="name")', + 'self.UiParameter(name="age", type=AttributeType.LONG); self.UiParameter("score", AttributeType.DOUBLE)', + 'self.UiParameter("created_at", attr_type=AttributeType.TIMESTAMP)', + ], + ["count:integer", "name:string", "age:long", "score:double", "created_at:timestamp"], + ], + [ + [ + 'self.UiParameter(); self.UiParameter(name="missing_type"); self.UiParameter(type=AttributeType.DOUBLE)', + 'self.UiParameter(AttributeType.INT, "wrong_order"); self.UiParameter("too_many", AttributeType.STRING, "extra")', + 'self.UiParameter(type=AttributeType.DOUBLE, key="legacy"); self.UiParameter("payload", AttributeType.BINARY)', + 'self.UiParameter("blob", AttributeType.LARGE_BINARY); self.UiParameter("valid", AttributeType.STRING)', + 'self.UiParameter("threshold", AttributeType.DOUBLE); self.UiParameter("threshold", AttributeType.STRING)', + ], + ["valid:string", "threshold:double"], + ], + [ + [ + '# self.UiParameter("commented", AttributeType.INT)', + '# self.UiParameter(name="commented_multiline", type=AttributeType.INT)', + '""" self.UiParameter("example", AttributeType.STRING) """', + 'self.UiParameter("active", AttributeType.DOUBLE) # self.UiParameter("trailing", AttributeType.STRING)', + ], + ["active:double"], + ], + ])("should parse UDF parameters", (lines, expected) => { + expect(parsed(udf(...lines))).toEqual(expected); + }); + + it("should support multiline declarations and exact supported class names only", () => { + expect( + parsed(udf('self.UiParameter(name=\n "threshold",\n type=AttributeType.DOUBLE)')) + ).toEqual(["threshold:double"]); + expect( + parsed( + 'class MyWrappedTupleOp(ProcessTupleOperator):\n def open(self):\n self.UiParameter("ignored", AttributeType.STRING)' + ) + ).toEqual([]); + expect( + parsed( + 'class GenerateOperator(UDFSourceOperator):\n def open(self):\n self.UiParameter("batch_size", AttributeType.INT)' + ) + ).toEqual(["batch_size:integer"]); + }); + + function parsed(code: string): string[] { + return service + .parse(code) + .map(parameter => `${parameter.attribute.attributeName}:${parameter.attribute.attributeType}`); + } }); + +function udf(...lines: string[]): string { + return [ + "class ProcessTupleOperator(UDFOperatorV2):", + " def open(self):", + ...lines.map(line => ` ${line}`), + ].join("\n"); +} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index e63d2e044a9..79dfcd10e4c 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -22,44 +22,11 @@ import { parser } from "@lezer/python"; import { AttributeType, UiUdfParameter } from "../../types/workflow-compiling.interface"; // Keep in sync with Python UDF template class names in PythonUDFOpDescV2, DualInputPortsPythonUDFOpDescV2, and PythonUDFSourceOpDescV2. -const CLASSES = new Set(["ProcessTupleOperator", "ProcessBatchOperator", "ProcessTableOperator", "GenerateOperator"]); - -// Java enum constant names (AttributeType.java) -const JAVA_ATTRIBUTE_TYPE_NAMES = [ - "STRING", - "INTEGER", - "LONG", - "DOUBLE", - "BOOLEAN", - "TIMESTAMP", - "BINARY", - "LARGE_BINARY", -] as const; - -type JavaAttributeTypeName = (typeof JAVA_ATTRIBUTE_TYPE_NAMES)[number]; - -// Python enum constant names (core.models.AttributeType) -const PYTHON_ATTRIBUTE_TYPE_NAMES = [ - "STRING", - "INT", - "LONG", - "DOUBLE", - "BOOL", - "TIMESTAMP", - "BINARY", - "LARGE_BINARY", -] as const; - -type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number]; - -type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName; -type UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY"; -type SupportedParserAttributeTypeToken = Exclude; -type ParserSyntaxNode = ReturnType["topNode"]; -type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; -type UiParameterArgument = Readonly<{ kind: "name"; value: string }> | Readonly<{ kind: "type"; value: AttributeType }>; +const UDF_CLASSES = new Set( + "ProcessTupleOperator ProcessBatchOperator ProcessTableOperator GenerateOperator".split(" ") +); -const TYPES: Readonly> = { +const ATTRIBUTE_TYPES: Readonly> = { STRING: "string", INTEGER: "integer", INT: "integer", @@ -70,16 +37,8 @@ const TYPES: Readonly> TIMESTAMP: "timestamp", }; -const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set(JAVA_ATTRIBUTE_TYPE_NAMES); -const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set(PYTHON_ATTRIBUTE_TYPE_NAMES); -const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set([ - "string", - "integer", - "long", - "double", - "boolean", - "timestamp", -]); +type ParserSyntaxNode = ReturnType["topNode"]; +type RawArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; @Injectable({ providedIn: "root" }) export class UiUdfParametersParserService { @@ -98,8 +57,7 @@ export class UiUdfParametersParserService { parser.parse(code).iterate({ enter: ({ name, node }) => { - const className = node.getChild("VariableName"); - if (name !== "ClassDefinition" || !className || !CLASSES.has(code.slice(className.from, className.to))) return; + if (!isSupportedUdfClass(name, node, code)) return; node.cursor().iterate(ref => { if (ref.name !== "CallExpression") return; add(readCall(ref.node, code)); @@ -113,6 +71,11 @@ export class UiUdfParametersParserService { } } +function isSupportedUdfClass(name: string, node: ParserSyntaxNode, code: string): boolean { + const className = node.getChild("VariableName"); + return name === "ClassDefinition" && !!className && UDF_CLASSES.has(code.slice(className.from, className.to)); +} + function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefined { const args = call.getChild("ArgList"); if (!args || code.slice(call.from, args.from).replace(/\s+/g, "") !== "self.UiParameter") return undefined; @@ -120,52 +83,32 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi let attributeName: string | undefined; let attributeType: AttributeType | undefined; let positionalIndex = 0; - let sawNamed = false; for (const arg of readArguments(args, code)) { - if (arg.key) { - sawNamed = true; - } else if (sawNamed) { - return undefined; - } - - const parsedArg = readArgument(arg, positionalIndex, code); - if (!parsedArg) { - return undefined; - } - - if (parsedArg.kind === "name") { - if (attributeName) { - return undefined; - } - attributeName = parsedArg.value; - } else { - if (attributeType) { - return undefined; - } - attributeType = parsedArg.value; - } - - if (!arg.key) { - positionalIndex++; - } + const key = arg.key; + const value = code.slice(arg.value.from, arg.value.to); + const isName = key === "name" || (!key && positionalIndex === 0); + const isType = key === "type" || key === "attr_type" || (!key && positionalIndex === 1); + + if (isName && arg.value.name === "String" && !attributeName) attributeName = readString(value)?.trim(); + else if (isType && arg.value.name === "MemberExpression" && !attributeType) attributeType = readType(value); + else return undefined; + if (!arg.key) positionalIndex++; } return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; } -function readArguments(args: ParserSyntaxNode, code: string): ParsedArgument[] { - const result: ParsedArgument[] = []; - const children = getChildren(args).filter(node => !["(", ")", ","].includes(node.name)); +function readArguments(args: ParserSyntaxNode, code: string): RawArgument[] { + const result: RawArgument[] = []; + const children = childNodes(args).filter(node => !["(", ")", ","].includes(node.name)); for (let index = 0; index < children.length; index++) { const node = children[index]; if (node.name === "VariableName" && children[index + 1]?.name === "AssignOp") { const value = children[index + 2]; - if (!value) { - return []; - } + if (!value) return []; result.push({ key: code.slice(node.from, node.to), value }); index += 2; } else if (node.name !== "AssignOp") { @@ -178,7 +121,7 @@ function readArguments(args: ParserSyntaxNode, code: string): ParsedArgument[] { return result; } -function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { +function childNodes(node: ParserSyntaxNode): ParserSyntaxNode[] { const children: ParserSyntaxNode[] = []; for (let child = node.firstChild; child; child = child.nextSibling) { children.push(child); @@ -186,26 +129,6 @@ function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { return children; } -function readArgument(arg: ParsedArgument, positionalIndex: number, code: string): UiParameterArgument | undefined { - const key = arg.key; - const value = code.slice(arg.value.from, arg.value.to); - - if ((key === "name" || (!key && positionalIndex === 0)) && arg.value.name === "String") { - const name = readString(value)?.trim(); - return name ? { kind: "name", value: name } : undefined; - } - - if ( - (key === "type" || key === "attr_type" || (!key && positionalIndex === 1)) && - arg.value.name === "MemberExpression" - ) { - const type = readType(value); - return type ? { kind: "type", value: type } : undefined; - } - - return undefined; -} - function readString(input: string): string | undefined { return input .trim() @@ -220,18 +143,6 @@ function readType(input: string): AttributeType | undefined { .replace(/\s+/g, "") .match(/^AttributeType\.([A-Za-z_]\w*)$/)?.[1] .toUpperCase(); - if (!token) { - return undefined; - } - - if (!JAVA_ATTRIBUTE_TYPE_NAME_SET.has(token) && !PYTHON_ATTRIBUTE_TYPE_NAME_SET.has(token)) { - return undefined; - } - - if (token === "BINARY" || token === "LARGE_BINARY") { - return undefined; - } - - const canonical = TYPES[token as SupportedParserAttributeTypeToken]; - return SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES.has(canonical) ? canonical : undefined; + if (!token || token === "BINARY" || token === "LARGE_BINARY") return undefined; + return ATTRIBUTE_TYPES[token]; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts index beb7fa15adb..9a89821cc66 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -17,194 +17,106 @@ * under the License. */ -import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; +import * as Y from "yjs"; +import { vi } from "vitest"; import { PYTHON_UDF_V2_OP_TYPE } from "../workflow-graph/model/workflow-graph"; -import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; import { UiUdfParametersSyncService } from "./ui-udf-parameters-sync.service"; -import { UiUdfParameter } from "../../types/workflow-compiling.interface"; -import { vi } from "vitest"; -import * as Y from "yjs"; describe("UiUdfParametersSyncService", () => { const operatorId = "operator-1"; - const code = "self.UiParameter(...)"; - let service: UiUdfParametersSyncService; - let workflowActionServiceSpy: { getTexeraGraph: ReturnType }; - let parserServiceSpy: { parse: ReturnType }; - let graphSpy: { - getOperator: ReturnType; - getSharedOperatorType: ReturnType; - }; - let operator: { - operatorType: string; - operatorProperties: { uiParameters: UiUdfParameter[] }; - }; + let parser: any; + let graph: any; + let operator: any; beforeEach(() => { - operator = { - operatorType: PYTHON_UDF_V2_OP_TYPE, - operatorProperties: { uiParameters: [] }, - }; - - graphSpy = { - getOperator: vi.fn().mockImplementation((id: string) => (id === operatorId ? operator : undefined)), + operator = { operatorType: PYTHON_UDF_V2_OP_TYPE, operatorProperties: { uiParameters: [] } }; + graph = { + getOperator: vi.fn((id: string) => (id === operatorId ? operator : undefined)), getSharedOperatorType: vi.fn(), }; - - workflowActionServiceSpy = { - getTexeraGraph: vi.fn().mockReturnValue(graphSpy), - }; - - parserServiceSpy = { parse: vi.fn() }; - - service = new UiUdfParametersSyncService( - workflowActionServiceSpy as unknown as WorkflowActionService, - parserServiceSpy as unknown as UiUdfParametersParserService - ); - }); - - it("should emit parameters that preserve values from current parameter names", () => { - operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); - - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); - - service.syncStructureFromCode(operatorId, code); - - expect(nextSpy).toHaveBeenCalledWith({ - operatorId, - parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], - }); - expect(nextSpy).toHaveBeenCalledOnce(); + parser = { parse: vi.fn() }; + service = new UiUdfParametersSyncService({ getTexeraGraph: () => graph } as any, parser); }); - it("should emit updated parameters with preserved values and removed stale parameters", () => { - operator.operatorProperties.uiParameters = [ - createParameter("count", "integer", "42"), - createParameter("removed", "string", "stale"), - ]; - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); - - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + it("should merge parsed parameters with saved values and drop stale ones", () => { + operator.operatorProperties.uiParameters = [param("count", "integer", "42"), param("removed", "string", "stale")]; + parser.parse.mockReturnValue([param("count", "integer"), param("name", "string")]); - service.syncStructureFromCode(operatorId, code); + const next = subscribe(); + service.syncStructureFromCode(operatorId, "code"); - expect(nextSpy).toHaveBeenCalledWith({ + expect(next).toHaveBeenCalledWith({ operatorId, - parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], + parameters: [param("count", "integer", "42"), param("name", "string")], }); - expect(nextSpy).toHaveBeenCalledOnce(); }); - it("should not emit when the merged parameters are unchanged", () => { - operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); - - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); - - service.syncStructureFromCode(operatorId, code); - - expect(nextSpy).not.toHaveBeenCalled(); - }); - - it("should not parse code for non-Python UDF operators", () => { + it("should skip non-Python UDF operators", () => { operator.operatorType = "Projection"; - - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); - - service.syncStructureFromCode(operatorId, code); - - expect(parserServiceSpy.parse).not.toHaveBeenCalled(); - expect(nextSpy).not.toHaveBeenCalled(); + service.syncStructureFromCode(operatorId, "code"); + expect(parser.parse).not.toHaveBeenCalled(); }); - it("should read code from the shared operator property when editor code is omitted", () => { + it("should read shared code when editor code is omitted", () => { const sharedCode = 'self.UiParameter("count", AttributeType.INT)'; - graphSpy.getSharedOperatorType.mockReturnValue(createSharedOperatorType(sharedCode)); - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); - - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + graph.getSharedOperatorType.mockReturnValue(sharedOperator(sharedCode)); + parser.parse.mockReturnValue([param("count", "integer")]); service.syncStructureFromCode(operatorId); - expect(parserServiceSpy.parse).toHaveBeenCalledWith(sharedCode); - expect(nextSpy).toHaveBeenCalledWith({ - operatorId, - parameters: [createParameter("count", "integer")], - }); + expect(parser.parse).toHaveBeenCalledWith(sharedCode); }); it("should debounce YText changes and clean up the observer", () => { vi.useFakeTimers(); - try { - const yCode = createYText('self.UiParameter("count", AttributeType.INT)'); - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); - - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); - - const cleanup = service.attachToYCode(operatorId, yCode); - - expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); - expect(nextSpy).toHaveBeenCalledOnce(); - - yCode.insert(yCode.length, "\n# changed"); - - vi.advanceTimersByTime(199); - - expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); - - vi.advanceTimersByTime(1); - - expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); - expect(nextSpy).toHaveBeenCalledTimes(2); - - cleanup(); - yCode.insert(yCode.length, "\n# after cleanup"); - vi.advanceTimersByTime(200); - - expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); - expect(nextSpy).toHaveBeenCalledTimes(2); - } finally { - vi.useRealTimers(); - } + const yCode = yText('self.UiParameter("count", AttributeType.INT)'); + parser.parse.mockReturnValue([param("count", "integer")]); + const next = subscribe(); + const cleanup = service.attachToYCode(operatorId, yCode); + + yCode.insert(yCode.length, "\n# changed"); + vi.advanceTimersByTime(199); + expect(parser.parse).toHaveBeenCalledOnce(); + + vi.advanceTimersByTime(1); + expect(parser.parse).toHaveBeenCalledTimes(2); + expect(next).toHaveBeenCalledTimes(2); + + cleanup(); + yCode.insert(yCode.length, "\n# after cleanup"); + vi.advanceTimersByTime(200); + expect(parser.parse).toHaveBeenCalledTimes(2); + vi.useRealTimers(); }); + + function subscribe() { + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + return nextSpy; + } }); -function createParameter(name: string, type: UiUdfParameter["attribute"]["attributeType"], value = ""): UiUdfParameter { - return { - attribute: { - attributeName: name, - attributeType: type, - }, - value, - }; +function param(name: string, type: string, value = "") { + return { attribute: { attributeName: name, attributeType: type }, value }; } -function createSharedOperatorType(code: string): Y.Map { +function sharedOperator(code: string): Y.Map { const doc = new Y.Doc(); - const sharedOperator = doc.getMap("operator"); + const operator = doc.getMap("operator"); const operatorProperties = new Y.Map(); - const yCode = new Y.Text(); - - operatorProperties.set("code", yCode); - sharedOperator.set("operatorProperties", operatorProperties); - yCode.insert(0, code); - - return sharedOperator; + const codeText = new Y.Text(); + operatorProperties.set("code", codeText); + operator.set("operatorProperties", operatorProperties); + codeText.insert(0, code); + return operator; } -function createYText(text: string): Y.Text { +function yText(text: string): Y.Text { const doc = new Y.Doc(); - const yMap = doc.getMap("root"); - const yText = new Y.Text(); - yMap.set("code", yText); - yText.insert(0, text); - return yText; + const map = doc.getMap("root"); + const codeText = new Y.Text(); + map.set("code", codeText); + codeText.insert(0, text); + return codeText; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts index 0e45e5138af..c8afc86f5fd 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -22,19 +22,15 @@ import { ReplaySubject, Subject } from "rxjs"; import { debounceTime } from "rxjs/operators"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; -import { isDefined } from "../../../common/util/predicate"; import { isPythonUdf } from "../workflow-graph/model/workflow-graph"; -import { YType } from "../../types/shared-editing.interface"; import type { Text as YText } from "yjs"; import { UiUdfParameter } from "../../types/workflow-compiling.interface"; -const UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS = 200; +type UiParametersChanged = Readonly<{ operatorId: string; parameters: UiUdfParameter[] }>; @Injectable({ providedIn: "root" }) export class UiUdfParametersSyncService { - private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>( - 1 - ); + private readonly uiParametersChangedSubject = new ReplaySubject(1); readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable(); @@ -43,24 +39,16 @@ export class UiUdfParametersSyncService { private uiUdfParametersParserService: UiUdfParametersParserService ) {} - /** - * Attach directly to YText and sync whenever it changes - */ attachToYCode(operatorId: string, yCode: YText): () => void { const codeChanges = new Subject(); - const subscription = codeChanges.pipe(debounceTime(UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS)).subscribe(latestCode => { - this.syncStructureFromCode(operatorId, latestCode); - }); - - const handler = () => { - codeChanges.next(yCode.toString()); - }; + const subscription = codeChanges + .pipe(debounceTime(200)) + .subscribe(code => this.syncStructureFromCode(operatorId, code)); + const handler = () => codeChanges.next(yCode.toString()); yCode.observe(handler); - this.syncStructureFromCode(operatorId, yCode.toString()); - // return cleanup function return () => { yCode.unobserve(handler); subscription.unsubscribe(); @@ -71,55 +59,29 @@ export class UiUdfParametersSyncService { syncStructureFromCode(operatorId: string, codeFromEditor?: string): void { const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId); - if (!operator || !isPythonUdf(operator)) { - return; - } + if (!operator || !isPythonUdf(operator)) return; const code = codeFromEditor ?? this.getSharedCode(operatorId); - if (!isDefined(code)) { - return; - } + if (code === undefined) return; const existingParameters = (operator.operatorProperties?.uiParameters ?? []) as UiUdfParameter[]; - const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); - - if (isEqual(existingParameters, mergedUiParameters)) { - return; - } - - // Emit event so UI updates - this.uiParametersChangedSubject.next({ - operatorId, - parameters: mergedUiParameters, - }); - } - - private buildParsedShapeWithPreservedValues(code: string, existingParameters: UiUdfParameter[]): UiUdfParameter[] { - const parsedParameters = this.uiUdfParametersParserService.parse(code); - - const existingValues = new Map(); - existingParameters.forEach(parameter => { - const parameterName = parameter.attribute.attributeName; - - if (isDefined(parameterName) && isDefined(parameter.value)) { - existingValues.set(parameterName, parameter.value); - } - }); - - return parsedParameters.map(parameter => ({ + const existingValues = new Map( + existingParameters.map(parameter => [parameter.attribute.attributeName, parameter.value]) + ); + const mergedUiParameters = this.uiUdfParametersParserService.parse(code).map(parameter => ({ ...parameter, value: existingValues.get(parameter.attribute.attributeName) ?? "", })); + + if (isEqual(existingParameters, mergedUiParameters)) return; + + this.uiParametersChangedSubject.next({ operatorId, parameters: mergedUiParameters }); } private getSharedCode(operatorId: string): string | undefined { try { const sharedOperatorType = this.workflowActionService.getTexeraGraph().getSharedOperatorType(operatorId); - - const operatorProperties = sharedOperatorType.get("operatorProperties") as YType< - Readonly<{ [key: string]: any }> - >; - + const operatorProperties = sharedOperatorType.get("operatorProperties") as any; const yCode = operatorProperties.get("code") as YText; return yCode?.toString(); } catch { From 9812925b7863d5801b8c3a6277291d842b875fcf Mon Sep 17 00:00:00 2001 From: carloea2 Date: Mon, 18 May 2026 23:42:37 -0600 Subject: [PATCH 06/13] Revert "refactor(frontend): compact UI parameter foundation" This reverts commit 99bb449c31bce1e9c9b9feeda7f6e2c03ae774b8. --- .../ui-udf-parameters.component.html | 55 ++++ .../ui-udf-parameters.component.scss | 39 +++ .../ui-udf-parameters.component.spec.ts | 56 +++- .../ui-udf-parameters.component.ts | 78 +++-- .../ui-udf-parameters-parser.service.spec.ts | 281 ++++++++++++++---- .../ui-udf-parameters-parser.service.ts | 145 +++++++-- .../ui-udf-parameters-sync.service.spec.ts | 214 +++++++++---- .../ui-udf-parameters-sync.service.ts | 72 +++-- 8 files changed, 737 insertions(+), 203 deletions(-) create mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html create mode 100644 frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html new file mode 100644 index 00000000000..0ecfb16a780 --- /dev/null +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html @@ -0,0 +1,55 @@ + +
+ +
+
Value
+
Name
+
Type
+
+ +
+ + +
+ + + +
+ + +
+ + + +
+ + +
+ + + +
+ +
+
diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss new file mode 100644 index 00000000000..901edaf1fc9 --- /dev/null +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss @@ -0,0 +1,39 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +.ui-udf-param-row { + display: grid; + grid-template-columns: 250px 250px 1fr; + gap: 12px; + align-items: start; +} + +.field-cell { + min-width: 0; +} + +/* Remove Formly/Ant label spacing */ +:host ::ng-deep .ant-form-item { + margin-bottom: 0; +} + +/* Hide Formly labels*/ +:host ::ng-deep .ant-form-item-label { + display: none; +} diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts index a79b2d67281..7cb209ad349 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts @@ -16,28 +16,54 @@ * specific language governing permissions and limitations * under the License. */ + import { FormControl } from "@angular/forms"; import { FormlyFieldConfig } from "@ngx-formly/core"; import { UiUdfParametersComponent } from "./ui-udf-parameters.component"; describe("UiUdfParametersComponent", () => { - it("should lock inferred fields and keep value editable", () => { - const component = new UiUdfParametersComponent(); - const value = field("value", new FormControl({ value: "42", disabled: true })); - const name = field("attributeName", new FormControl("threshold")); - const type = field("attributeType", new FormControl("double")); - - (component as any).field = { - fieldGroup: [{ fieldGroup: [value, { key: "attribute", fieldGroup: [name, type] }] }], + let component: UiUdfParametersComponent; + + beforeEach(() => { + component = new UiUdfParametersComponent(); + }); + + it("should disable name and type fields while leaving value editable", () => { + const valueControl = new FormControl({ value: "42", disabled: true }); + const nameControl = new FormControl("threshold"); + const typeControl = new FormControl("double"); + + const valueField: FormlyFieldConfig = { key: "value", formControl: valueControl }; + const nameField: FormlyFieldConfig = { key: "attributeName", formControl: nameControl }; + const typeField: FormlyFieldConfig = { key: "attributeType", formControl: typeControl }; + const rowField: FormlyFieldConfig = { + fieldGroup: [ + valueField, + { + key: "attribute", + fieldGroup: [nameField, typeField], + }, + ], }; + + (component as any).field = { fieldGroup: [rowField] } as FormlyFieldConfig; + component.ngOnInit(); - expect([value.props?.disabled, value.formControl?.enabled]).toEqual([false, true]); - expect([name.props?.disabled, name.formControl?.disabled]).toEqual([true, true]); - expect([type.props?.disabled, type.formControl?.disabled]).toEqual([true, true]); + expect(component.getValueField(rowField)).toBe(valueField); + expect(component.getNameField(rowField)).toBe(nameField); + expect(component.getTypeField(rowField)).toBe(typeField); + + expect(valueField.props?.disabled).toBe(false); + expect((valueField as any).templateOptions?.disabled).toBe(false); + expect(valueControl.enabled).toBe(true); + + expect(nameField.props?.disabled).toBe(true); + expect((nameField as any).templateOptions?.disabled).toBe(true); + expect(nameControl.disabled).toBe(true); + + expect(typeField.props?.disabled).toBe(true); + expect((typeField as any).templateOptions?.disabled).toBe(true); + expect(typeControl.disabled).toBe(true); }); }); - -function field(key: string, formControl: FormControl): FormlyFieldConfig { - return { key, formControl }; -} diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index 050f7031d78..31335363843 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -16,40 +16,80 @@ * specific language governing permissions and limitations * under the License. */ -import { NgFor } from "@angular/common"; import { Component, OnInit } from "@angular/core"; +import { NgFor, NgIf } from "@angular/common"; import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; -const COLUMN_KEYS = ["value", "attributeName", "attributeType"] as const; - @Component({ selector: "texera-ui-udf-parameters", - template: - '
', - styles: [ - ".ui-udf-param-row{display:grid;grid-template-columns:minmax(160px,250px) minmax(160px,250px) minmax(120px,1fr);gap:12px}:host ::ng-deep .ant-form-item{margin-bottom:0}:host ::ng-deep .ant-form-item-label{display:none}", - ], - imports: [NgFor, FormlyModule], + templateUrl: "./ui-udf-parameters.component.html", + styleUrls: ["./ui-udf-parameters.component.scss"], + imports: [NgIf, NgFor, FormlyModule], }) export class UiUdfParametersComponent extends FieldArrayType implements OnInit { ngOnInit(): void { - this.field.fieldGroup?.forEach(rowField => - COLUMN_KEYS.forEach(key => this.setDisabled(this.child(rowField, key), key !== "value")) - ); + this.field.fieldGroup?.forEach(rowField => { + this.configureDisabledState(this.getAttributeChild(rowField, "attributeName"), true); + this.configureDisabledState(this.getAttributeChild(rowField, "attributeType"), true); + this.configureDisabledState(this.getField(rowField, "value"), false); + }); } - displayFields(rowField: FormlyFieldConfig): FormlyFieldConfig[] { - return COLUMN_KEYS.map(key => this.child(rowField, key)).filter((field): field is FormlyFieldConfig => !!field); + private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { + return rowField.fieldGroup?.find(f => f.key === key); } - private child(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { - if (key === "value") return rowField.fieldGroup?.find(field => field.key === key); - return rowField.fieldGroup?.find(field => field.key === "attribute")?.fieldGroup?.find(field => field.key === key); + private getAttributeChild(rowField: FormlyFieldConfig, childKey: string): FormlyFieldConfig | undefined { + const attributeGroup = this.getField(rowField, "attribute"); + return attributeGroup?.fieldGroup?.find(f => f.key === childKey); } - private setDisabled(field: FormlyFieldConfig | undefined, disabled: boolean): void { + private configureDisabledState(field: FormlyFieldConfig | undefined, disabled: boolean): void { if (!field) return; + field.props = { ...(field.props ?? {}), disabled }; - field.formControl?.[disabled ? "disable" : "enable"]({ emitEvent: false }); + + // (`as any` so we don't get nagged by the @deprecated JSDoc) + (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; + + const prevOnInit = field.hooks?.onInit; + field.hooks = { + ...(field.hooks ?? {}), + onInit: f => { + prevOnInit?.(f); + this.applyDisabledState(f, disabled); + }, + }; + + this.applyDisabledState(field, disabled); + } + + private applyDisabledState(field: FormlyFieldConfig, disabled: boolean): void { + if (field.formControl) { + if (disabled) { + field.formControl.disable({ emitEvent: false }); + } else { + field.formControl.enable({ emitEvent: false }); + } + } } + + // Disable Name + getNameField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { + return this.getAttributeChild(rowField, "attributeName"); + } + + // Disable Type + getTypeField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { + return this.getAttributeChild(rowField, "attributeType"); + } + + // Value editable + getValueField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { + return this.getField(rowField, "value"); + } + + trackByParamName = (index: number, param: any): string | number => { + return param?.attribute?.attributeName ?? index; + }; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index 2249dd88fad..fa21e88ffcb 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -26,65 +26,224 @@ describe("UiUdfParametersParserService", () => { service = new UiUdfParametersParserService(); }); - it.each([ - [ - [ - 'self.UiParameter("count", AttributeType.INT); self.UiParameter(type=AttributeType.STRING, name="name")', - 'self.UiParameter(name="age", type=AttributeType.LONG); self.UiParameter("score", AttributeType.DOUBLE)', - 'self.UiParameter("created_at", attr_type=AttributeType.TIMESTAMP)', - ], - ["count:integer", "name:string", "age:long", "score:double", "created_at:timestamp"], - ], - [ - [ - 'self.UiParameter(); self.UiParameter(name="missing_type"); self.UiParameter(type=AttributeType.DOUBLE)', - 'self.UiParameter(AttributeType.INT, "wrong_order"); self.UiParameter("too_many", AttributeType.STRING, "extra")', - 'self.UiParameter(type=AttributeType.DOUBLE, key="legacy"); self.UiParameter("payload", AttributeType.BINARY)', - 'self.UiParameter("blob", AttributeType.LARGE_BINARY); self.UiParameter("valid", AttributeType.STRING)', - 'self.UiParameter("threshold", AttributeType.DOUBLE); self.UiParameter("threshold", AttributeType.STRING)', - ], - ["valid:string", "threshold:double"], - ], - [ - [ - '# self.UiParameter("commented", AttributeType.INT)', - '# self.UiParameter(name="commented_multiline", type=AttributeType.INT)', - '""" self.UiParameter("example", AttributeType.STRING) """', - 'self.UiParameter("active", AttributeType.DOUBLE) # self.UiParameter("trailing", AttributeType.STRING)', - ], - ["active:double"], - ], - ])("should parse UDF parameters", (lines, expected) => { - expect(parsed(udf(...lines))).toEqual(expected); - }); - - it("should support multiline declarations and exact supported class names only", () => { - expect( - parsed(udf('self.UiParameter(name=\n "threshold",\n type=AttributeType.DOUBLE)')) - ).toEqual(["threshold:double"]); - expect( - parsed( - 'class MyWrappedTupleOp(ProcessTupleOperator):\n def open(self):\n self.UiParameter("ignored", AttributeType.STRING)' - ) - ).toEqual([]); - expect( - parsed( - 'class GenerateOperator(UDFSourceOperator):\n def open(self):\n self.UiParameter("batch_size", AttributeType.INT)' - ) - ).toEqual(["batch_size:integer"]); - }); - - function parsed(code: string): string[] { - return service - .parse(code) - .map(parameter => `${parameter.attribute.attributeName}:${parameter.attribute.attributeType}`); - } -}); + it("should parse Python-compatible positional and name-based arguments", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("count", AttributeType.INT) + self.UiParameter(type=AttributeType.STRING, name="name") + self.UiParameter(name="age", type=AttributeType.LONG) + self.UiParameter("score", AttributeType.DOUBLE) + self.UiParameter("created_at", type=AttributeType.TIMESTAMP) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, + { attribute: { attributeName: "name", attributeType: "string" }, value: "" }, + { attribute: { attributeName: "age", attributeType: "long" }, value: "" }, + { attribute: { attributeName: "score", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "created_at", attributeType: "timestamp" }, value: "" }, + ]); + }); + + it("should parse the attr_type keyword used by the Python API", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("count", attr_type=AttributeType.INT) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, + ]); + }); + + it("should parse multiline UiParameter calls with named arguments split across lines", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter( + name= + "threshold", + type= + AttributeType.DOUBLE, + ) + self.UiParameter( + "label", + type= + AttributeType.STRING, + ) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, + ]); + }); + + it("should ignore calls where name or type is missing", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter(name="a") + self.UiParameter(type=AttributeType.DOUBLE) + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should ignore invalid positional argument ordering", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter(AttributeType.INT, "count") + self.UiParameter(name="valid", type=AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, + ]); + }); + + it("should ignore legacy key= named argument", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter(type=AttributeType.DOUBLE, key="a") + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should ignore unsupported classes", () => { + const code = ` + class RandomClass(ABC): + def open(self): + self.UiParameter(type=AttributeType.DOUBLE, name="a") + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should ignore custom-named subclasses because injection targets template class names", () => { + const code = ` + class MyTupleOp(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) + + class MyWrappedTupleOp(ProcessTupleOperator): + def open(self): + self.UiParameter("label", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([]); + }); + + it("should parse supported UiParameter calls across multiple classes", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) + + class RandomClass(ABC): + def open(self): + self.UiParameter("ignored", AttributeType.STRING) + + class GenerateOperator(UDFSourceOperator): + def open(self): + self.UiParameter(name="batch_size", type=AttributeType.INT) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "batch_size", attributeType: "integer" }, value: "" }, + ]); + }); + + it("should ignore empty and extra positional arguments", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter() + self.UiParameter("too_many", AttributeType.STRING, "extra") + self.UiParameter("valid", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, + ]); + }); + + it("should keep the first duplicate parameter name", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) + self.UiParameter("threshold", AttributeType.STRING) + self.UiParameter("label", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, + ]); + }); -function udf(...lines: string[]): string { - return [ - "class ProcessTupleOperator(UDFOperatorV2):", - " def open(self):", - ...lines.map(line => ` ${line}`), - ].join("\n"); -} + it("should ignore commented out UiParameter calls", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + # self.UiParameter("commented", AttributeType.INT) + self.UiParameter("active", AttributeType.INT) # self.UiParameter("trailing", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "active", attributeType: "integer" }, value: "" }, + ]); + }); + + it("should ignore commented out multiline UiParameter sections", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + # self.UiParameter( + # name="commented", + # type=AttributeType.INT, + # ) + self.UiParameter( + name="active", + type=AttributeType.STRING, + ) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "active", attributeType: "string" }, value: "" }, + ]); + }); + + it("should ignore UiParameter examples inside triple-quoted strings", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + """ + self.UiParameter("example", AttributeType.INT) + """ + self.UiParameter("active", AttributeType.DOUBLE) + `; + + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "active", attributeType: "double" }, value: "" }, + ]); + }); + + it("should reject binary UiParameter types", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("payload", AttributeType.BINARY) + self.UiParameter("blob", AttributeType.LARGE_BINARY) + `; + + expect(service.parse(code)).toEqual([]); + }); +}); diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 79dfcd10e4c..e63d2e044a9 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -22,11 +22,44 @@ import { parser } from "@lezer/python"; import { AttributeType, UiUdfParameter } from "../../types/workflow-compiling.interface"; // Keep in sync with Python UDF template class names in PythonUDFOpDescV2, DualInputPortsPythonUDFOpDescV2, and PythonUDFSourceOpDescV2. -const UDF_CLASSES = new Set( - "ProcessTupleOperator ProcessBatchOperator ProcessTableOperator GenerateOperator".split(" ") -); +const CLASSES = new Set(["ProcessTupleOperator", "ProcessBatchOperator", "ProcessTableOperator", "GenerateOperator"]); + +// Java enum constant names (AttributeType.java) +const JAVA_ATTRIBUTE_TYPE_NAMES = [ + "STRING", + "INTEGER", + "LONG", + "DOUBLE", + "BOOLEAN", + "TIMESTAMP", + "BINARY", + "LARGE_BINARY", +] as const; + +type JavaAttributeTypeName = (typeof JAVA_ATTRIBUTE_TYPE_NAMES)[number]; + +// Python enum constant names (core.models.AttributeType) +const PYTHON_ATTRIBUTE_TYPE_NAMES = [ + "STRING", + "INT", + "LONG", + "DOUBLE", + "BOOL", + "TIMESTAMP", + "BINARY", + "LARGE_BINARY", +] as const; + +type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number]; + +type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName; +type UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY"; +type SupportedParserAttributeTypeToken = Exclude; +type ParserSyntaxNode = ReturnType["topNode"]; +type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; +type UiParameterArgument = Readonly<{ kind: "name"; value: string }> | Readonly<{ kind: "type"; value: AttributeType }>; -const ATTRIBUTE_TYPES: Readonly> = { +const TYPES: Readonly> = { STRING: "string", INTEGER: "integer", INT: "integer", @@ -37,8 +70,16 @@ const ATTRIBUTE_TYPES: Readonly> = { TIMESTAMP: "timestamp", }; -type ParserSyntaxNode = ReturnType["topNode"]; -type RawArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; +const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set(JAVA_ATTRIBUTE_TYPE_NAMES); +const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set(PYTHON_ATTRIBUTE_TYPE_NAMES); +const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set([ + "string", + "integer", + "long", + "double", + "boolean", + "timestamp", +]); @Injectable({ providedIn: "root" }) export class UiUdfParametersParserService { @@ -57,7 +98,8 @@ export class UiUdfParametersParserService { parser.parse(code).iterate({ enter: ({ name, node }) => { - if (!isSupportedUdfClass(name, node, code)) return; + const className = node.getChild("VariableName"); + if (name !== "ClassDefinition" || !className || !CLASSES.has(code.slice(className.from, className.to))) return; node.cursor().iterate(ref => { if (ref.name !== "CallExpression") return; add(readCall(ref.node, code)); @@ -71,11 +113,6 @@ export class UiUdfParametersParserService { } } -function isSupportedUdfClass(name: string, node: ParserSyntaxNode, code: string): boolean { - const className = node.getChild("VariableName"); - return name === "ClassDefinition" && !!className && UDF_CLASSES.has(code.slice(className.from, className.to)); -} - function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefined { const args = call.getChild("ArgList"); if (!args || code.slice(call.from, args.from).replace(/\s+/g, "") !== "self.UiParameter") return undefined; @@ -83,32 +120,52 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi let attributeName: string | undefined; let attributeType: AttributeType | undefined; let positionalIndex = 0; + let sawNamed = false; for (const arg of readArguments(args, code)) { - const key = arg.key; - const value = code.slice(arg.value.from, arg.value.to); - const isName = key === "name" || (!key && positionalIndex === 0); - const isType = key === "type" || key === "attr_type" || (!key && positionalIndex === 1); - - if (isName && arg.value.name === "String" && !attributeName) attributeName = readString(value)?.trim(); - else if (isType && arg.value.name === "MemberExpression" && !attributeType) attributeType = readType(value); - else return undefined; - if (!arg.key) positionalIndex++; + if (arg.key) { + sawNamed = true; + } else if (sawNamed) { + return undefined; + } + + const parsedArg = readArgument(arg, positionalIndex, code); + if (!parsedArg) { + return undefined; + } + + if (parsedArg.kind === "name") { + if (attributeName) { + return undefined; + } + attributeName = parsedArg.value; + } else { + if (attributeType) { + return undefined; + } + attributeType = parsedArg.value; + } + + if (!arg.key) { + positionalIndex++; + } } return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; } -function readArguments(args: ParserSyntaxNode, code: string): RawArgument[] { - const result: RawArgument[] = []; - const children = childNodes(args).filter(node => !["(", ")", ","].includes(node.name)); +function readArguments(args: ParserSyntaxNode, code: string): ParsedArgument[] { + const result: ParsedArgument[] = []; + const children = getChildren(args).filter(node => !["(", ")", ","].includes(node.name)); for (let index = 0; index < children.length; index++) { const node = children[index]; if (node.name === "VariableName" && children[index + 1]?.name === "AssignOp") { const value = children[index + 2]; - if (!value) return []; + if (!value) { + return []; + } result.push({ key: code.slice(node.from, node.to), value }); index += 2; } else if (node.name !== "AssignOp") { @@ -121,7 +178,7 @@ function readArguments(args: ParserSyntaxNode, code: string): RawArgument[] { return result; } -function childNodes(node: ParserSyntaxNode): ParserSyntaxNode[] { +function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { const children: ParserSyntaxNode[] = []; for (let child = node.firstChild; child; child = child.nextSibling) { children.push(child); @@ -129,6 +186,26 @@ function childNodes(node: ParserSyntaxNode): ParserSyntaxNode[] { return children; } +function readArgument(arg: ParsedArgument, positionalIndex: number, code: string): UiParameterArgument | undefined { + const key = arg.key; + const value = code.slice(arg.value.from, arg.value.to); + + if ((key === "name" || (!key && positionalIndex === 0)) && arg.value.name === "String") { + const name = readString(value)?.trim(); + return name ? { kind: "name", value: name } : undefined; + } + + if ( + (key === "type" || key === "attr_type" || (!key && positionalIndex === 1)) && + arg.value.name === "MemberExpression" + ) { + const type = readType(value); + return type ? { kind: "type", value: type } : undefined; + } + + return undefined; +} + function readString(input: string): string | undefined { return input .trim() @@ -143,6 +220,18 @@ function readType(input: string): AttributeType | undefined { .replace(/\s+/g, "") .match(/^AttributeType\.([A-Za-z_]\w*)$/)?.[1] .toUpperCase(); - if (!token || token === "BINARY" || token === "LARGE_BINARY") return undefined; - return ATTRIBUTE_TYPES[token]; + if (!token) { + return undefined; + } + + if (!JAVA_ATTRIBUTE_TYPE_NAME_SET.has(token) && !PYTHON_ATTRIBUTE_TYPE_NAME_SET.has(token)) { + return undefined; + } + + if (token === "BINARY" || token === "LARGE_BINARY") { + return undefined; + } + + const canonical = TYPES[token as SupportedParserAttributeTypeToken]; + return SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES.has(canonical) ? canonical : undefined; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts index 9a89821cc66..beb7fa15adb 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -17,106 +17,194 @@ * under the License. */ -import * as Y from "yjs"; -import { vi } from "vitest"; +import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { PYTHON_UDF_V2_OP_TYPE } from "../workflow-graph/model/workflow-graph"; +import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; import { UiUdfParametersSyncService } from "./ui-udf-parameters-sync.service"; +import { UiUdfParameter } from "../../types/workflow-compiling.interface"; +import { vi } from "vitest"; +import * as Y from "yjs"; describe("UiUdfParametersSyncService", () => { const operatorId = "operator-1"; + const code = "self.UiParameter(...)"; + let service: UiUdfParametersSyncService; - let parser: any; - let graph: any; - let operator: any; + let workflowActionServiceSpy: { getTexeraGraph: ReturnType }; + let parserServiceSpy: { parse: ReturnType }; + let graphSpy: { + getOperator: ReturnType; + getSharedOperatorType: ReturnType; + }; + let operator: { + operatorType: string; + operatorProperties: { uiParameters: UiUdfParameter[] }; + }; beforeEach(() => { - operator = { operatorType: PYTHON_UDF_V2_OP_TYPE, operatorProperties: { uiParameters: [] } }; - graph = { - getOperator: vi.fn((id: string) => (id === operatorId ? operator : undefined)), + operator = { + operatorType: PYTHON_UDF_V2_OP_TYPE, + operatorProperties: { uiParameters: [] }, + }; + + graphSpy = { + getOperator: vi.fn().mockImplementation((id: string) => (id === operatorId ? operator : undefined)), getSharedOperatorType: vi.fn(), }; - parser = { parse: vi.fn() }; - service = new UiUdfParametersSyncService({ getTexeraGraph: () => graph } as any, parser); + + workflowActionServiceSpy = { + getTexeraGraph: vi.fn().mockReturnValue(graphSpy), + }; + + parserServiceSpy = { parse: vi.fn() }; + + service = new UiUdfParametersSyncService( + workflowActionServiceSpy as unknown as WorkflowActionService, + parserServiceSpy as unknown as UiUdfParametersParserService + ); + }); + + it("should emit parameters that preserve values from current parameter names", () => { + operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(nextSpy).toHaveBeenCalledWith({ + operatorId, + parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], + }); + expect(nextSpy).toHaveBeenCalledOnce(); }); - it("should merge parsed parameters with saved values and drop stale ones", () => { - operator.operatorProperties.uiParameters = [param("count", "integer", "42"), param("removed", "string", "stale")]; - parser.parse.mockReturnValue([param("count", "integer"), param("name", "string")]); + it("should emit updated parameters with preserved values and removed stale parameters", () => { + operator.operatorProperties.uiParameters = [ + createParameter("count", "integer", "42"), + createParameter("removed", "string", "stale"), + ]; + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); - const next = subscribe(); - service.syncStructureFromCode(operatorId, "code"); + service.syncStructureFromCode(operatorId, code); - expect(next).toHaveBeenCalledWith({ + expect(nextSpy).toHaveBeenCalledWith({ operatorId, - parameters: [param("count", "integer", "42"), param("name", "string")], + parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], }); + expect(nextSpy).toHaveBeenCalledOnce(); }); - it("should skip non-Python UDF operators", () => { + it("should not emit when the merged parameters are unchanged", () => { + operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(nextSpy).not.toHaveBeenCalled(); + }); + + it("should not parse code for non-Python UDF operators", () => { operator.operatorType = "Projection"; - service.syncStructureFromCode(operatorId, "code"); - expect(parser.parse).not.toHaveBeenCalled(); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + service.syncStructureFromCode(operatorId, code); + + expect(parserServiceSpy.parse).not.toHaveBeenCalled(); + expect(nextSpy).not.toHaveBeenCalled(); }); - it("should read shared code when editor code is omitted", () => { + it("should read code from the shared operator property when editor code is omitted", () => { const sharedCode = 'self.UiParameter("count", AttributeType.INT)'; - graph.getSharedOperatorType.mockReturnValue(sharedOperator(sharedCode)); - parser.parse.mockReturnValue([param("count", "integer")]); + graphSpy.getSharedOperatorType.mockReturnValue(createSharedOperatorType(sharedCode)); + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); service.syncStructureFromCode(operatorId); - expect(parser.parse).toHaveBeenCalledWith(sharedCode); + expect(parserServiceSpy.parse).toHaveBeenCalledWith(sharedCode); + expect(nextSpy).toHaveBeenCalledWith({ + operatorId, + parameters: [createParameter("count", "integer")], + }); }); it("should debounce YText changes and clean up the observer", () => { vi.useFakeTimers(); - const yCode = yText('self.UiParameter("count", AttributeType.INT)'); - parser.parse.mockReturnValue([param("count", "integer")]); - const next = subscribe(); - const cleanup = service.attachToYCode(operatorId, yCode); - - yCode.insert(yCode.length, "\n# changed"); - vi.advanceTimersByTime(199); - expect(parser.parse).toHaveBeenCalledOnce(); - - vi.advanceTimersByTime(1); - expect(parser.parse).toHaveBeenCalledTimes(2); - expect(next).toHaveBeenCalledTimes(2); - - cleanup(); - yCode.insert(yCode.length, "\n# after cleanup"); - vi.advanceTimersByTime(200); - expect(parser.parse).toHaveBeenCalledTimes(2); - vi.useRealTimers(); - }); + try { + const yCode = createYText('self.UiParameter("count", AttributeType.INT)'); + parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); - function subscribe() { - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); - return nextSpy; - } + const nextSpy = vi.fn(); + service.uiParametersChanged$.subscribe(nextSpy); + + const cleanup = service.attachToYCode(operatorId, yCode); + + expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); + expect(nextSpy).toHaveBeenCalledOnce(); + + yCode.insert(yCode.length, "\n# changed"); + + vi.advanceTimersByTime(199); + + expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); + + vi.advanceTimersByTime(1); + + expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); + expect(nextSpy).toHaveBeenCalledTimes(2); + + cleanup(); + yCode.insert(yCode.length, "\n# after cleanup"); + vi.advanceTimersByTime(200); + + expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); + expect(nextSpy).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); }); -function param(name: string, type: string, value = "") { - return { attribute: { attributeName: name, attributeType: type }, value }; +function createParameter(name: string, type: UiUdfParameter["attribute"]["attributeType"], value = ""): UiUdfParameter { + return { + attribute: { + attributeName: name, + attributeType: type, + }, + value, + }; } -function sharedOperator(code: string): Y.Map { +function createSharedOperatorType(code: string): Y.Map { const doc = new Y.Doc(); - const operator = doc.getMap("operator"); + const sharedOperator = doc.getMap("operator"); const operatorProperties = new Y.Map(); - const codeText = new Y.Text(); - operatorProperties.set("code", codeText); - operator.set("operatorProperties", operatorProperties); - codeText.insert(0, code); - return operator; + const yCode = new Y.Text(); + + operatorProperties.set("code", yCode); + sharedOperator.set("operatorProperties", operatorProperties); + yCode.insert(0, code); + + return sharedOperator; } -function yText(text: string): Y.Text { +function createYText(text: string): Y.Text { const doc = new Y.Doc(); - const map = doc.getMap("root"); - const codeText = new Y.Text(); - map.set("code", codeText); - codeText.insert(0, text); - return codeText; + const yMap = doc.getMap("root"); + const yText = new Y.Text(); + yMap.set("code", yText); + yText.insert(0, text); + return yText; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts index c8afc86f5fd..0e45e5138af 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -22,15 +22,19 @@ import { ReplaySubject, Subject } from "rxjs"; import { debounceTime } from "rxjs/operators"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { isDefined } from "../../../common/util/predicate"; import { isPythonUdf } from "../workflow-graph/model/workflow-graph"; +import { YType } from "../../types/shared-editing.interface"; import type { Text as YText } from "yjs"; import { UiUdfParameter } from "../../types/workflow-compiling.interface"; -type UiParametersChanged = Readonly<{ operatorId: string; parameters: UiUdfParameter[] }>; +const UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS = 200; @Injectable({ providedIn: "root" }) export class UiUdfParametersSyncService { - private readonly uiParametersChangedSubject = new ReplaySubject(1); + private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>( + 1 + ); readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable(); @@ -39,16 +43,24 @@ export class UiUdfParametersSyncService { private uiUdfParametersParserService: UiUdfParametersParserService ) {} + /** + * Attach directly to YText and sync whenever it changes + */ attachToYCode(operatorId: string, yCode: YText): () => void { const codeChanges = new Subject(); - const subscription = codeChanges - .pipe(debounceTime(200)) - .subscribe(code => this.syncStructureFromCode(operatorId, code)); - const handler = () => codeChanges.next(yCode.toString()); + const subscription = codeChanges.pipe(debounceTime(UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS)).subscribe(latestCode => { + this.syncStructureFromCode(operatorId, latestCode); + }); + + const handler = () => { + codeChanges.next(yCode.toString()); + }; yCode.observe(handler); + this.syncStructureFromCode(operatorId, yCode.toString()); + // return cleanup function return () => { yCode.unobserve(handler); subscription.unsubscribe(); @@ -59,29 +71,55 @@ export class UiUdfParametersSyncService { syncStructureFromCode(operatorId: string, codeFromEditor?: string): void { const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId); - if (!operator || !isPythonUdf(operator)) return; + if (!operator || !isPythonUdf(operator)) { + return; + } const code = codeFromEditor ?? this.getSharedCode(operatorId); - if (code === undefined) return; + if (!isDefined(code)) { + return; + } const existingParameters = (operator.operatorProperties?.uiParameters ?? []) as UiUdfParameter[]; - const existingValues = new Map( - existingParameters.map(parameter => [parameter.attribute.attributeName, parameter.value]) - ); - const mergedUiParameters = this.uiUdfParametersParserService.parse(code).map(parameter => ({ + const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); + + if (isEqual(existingParameters, mergedUiParameters)) { + return; + } + + // Emit event so UI updates + this.uiParametersChangedSubject.next({ + operatorId, + parameters: mergedUiParameters, + }); + } + + private buildParsedShapeWithPreservedValues(code: string, existingParameters: UiUdfParameter[]): UiUdfParameter[] { + const parsedParameters = this.uiUdfParametersParserService.parse(code); + + const existingValues = new Map(); + existingParameters.forEach(parameter => { + const parameterName = parameter.attribute.attributeName; + + if (isDefined(parameterName) && isDefined(parameter.value)) { + existingValues.set(parameterName, parameter.value); + } + }); + + return parsedParameters.map(parameter => ({ ...parameter, value: existingValues.get(parameter.attribute.attributeName) ?? "", })); - - if (isEqual(existingParameters, mergedUiParameters)) return; - - this.uiParametersChangedSubject.next({ operatorId, parameters: mergedUiParameters }); } private getSharedCode(operatorId: string): string | undefined { try { const sharedOperatorType = this.workflowActionService.getTexeraGraph().getSharedOperatorType(operatorId); - const operatorProperties = sharedOperatorType.get("operatorProperties") as any; + + const operatorProperties = sharedOperatorType.get("operatorProperties") as YType< + Readonly<{ [key: string]: any }> + >; + const yCode = operatorProperties.get("code") as YText; return yCode?.toString(); } catch { From e37cc6e489507336c0055dc3409e6ff58ace4630 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 19 May 2026 01:30:49 -0600 Subject: [PATCH 07/13] fix(frontend): tighten UI parameter parser feedback --- .../ui-udf-parameters.component.html | 8 +- .../ui-udf-parameters.component.scss | 2 +- .../ui-udf-parameters.component.spec.ts | 21 +-- .../ui-udf-parameters.component.ts | 29 +-- .../ui-udf-parameters-parser.service.spec.ts | 75 ++++---- .../ui-udf-parameters-parser.service.ts | 142 +++++++------- .../ui-udf-parameters-sync.service.spec.ts | 177 ++++++++++-------- .../ui-udf-parameters-sync.service.ts | 20 +- .../types/workflow-compiling.interface.ts | 6 - 9 files changed, 245 insertions(+), 235 deletions(-) diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html index 0ecfb16a780..5d305c395d2 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html @@ -17,18 +17,18 @@ under the License. -->
-
+
Value
Name
Type
+ class="ui-udf-parameter-row" + *ngFor="let parameter of (model || []); let i = index; trackBy: trackByParameterName">
diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss index 901edaf1fc9..dff89283ab8 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss @@ -17,7 +17,7 @@ * under the License. */ -.ui-udf-param-row { +.ui-udf-parameter-row { display: grid; grid-template-columns: 250px 250px 1fr; gap: 12px; diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts index 7cb209ad349..1da883e37e8 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts @@ -54,16 +54,15 @@ describe("UiUdfParametersComponent", () => { expect(component.getNameField(rowField)).toBe(nameField); expect(component.getTypeField(rowField)).toBe(typeField); - expect(valueField.props?.disabled).toBe(false); - expect((valueField as any).templateOptions?.disabled).toBe(false); - expect(valueControl.enabled).toBe(true); - - expect(nameField.props?.disabled).toBe(true); - expect((nameField as any).templateOptions?.disabled).toBe(true); - expect(nameControl.disabled).toBe(true); - - expect(typeField.props?.disabled).toBe(true); - expect((typeField as any).templateOptions?.disabled).toBe(true); - expect(typeControl.disabled).toBe(true); + // templateOptions is deprecated, but some existing Formly wrappers still read it. + [ + [valueField, valueControl, false], + [nameField, nameControl, true], + [typeField, typeControl, true], + ].forEach(([field, control, disabled]) => { + expect((field as FormlyFieldConfig).props?.disabled).toBe(disabled); + expect((field as any).templateOptions?.disabled).toBe(disabled); + expect((control as FormControl).disabled).toBe(disabled); + }); }); }); diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index 31335363843..fc2bfb616a5 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -36,12 +36,12 @@ export class UiUdfParametersComponent extends FieldArrayType implements OnInit { } private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { - return rowField.fieldGroup?.find(f => f.key === key); + return rowField.fieldGroup?.find(fieldConfig => fieldConfig.key === key); } private getAttributeChild(rowField: FormlyFieldConfig, childKey: string): FormlyFieldConfig | undefined { const attributeGroup = this.getField(rowField, "attribute"); - return attributeGroup?.fieldGroup?.find(f => f.key === childKey); + return attributeGroup?.fieldGroup?.find(fieldConfig => fieldConfig.key === childKey); } private configureDisabledState(field: FormlyFieldConfig | undefined, disabled: boolean): void { @@ -49,15 +49,15 @@ export class UiUdfParametersComponent extends FieldArrayType implements OnInit { field.props = { ...(field.props ?? {}), disabled }; - // (`as any` so we don't get nagged by the @deprecated JSDoc) + // Keep deprecated templateOptions in sync for existing Formly wrappers that still read it. (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; - const prevOnInit = field.hooks?.onInit; + const previousOnInit = field.hooks?.onInit; field.hooks = { ...(field.hooks ?? {}), - onInit: f => { - prevOnInit?.(f); - this.applyDisabledState(f, disabled); + onInit: initializedField => { + previousOnInit?.(initializedField); + this.applyDisabledState(initializedField, disabled); }, }; @@ -65,31 +65,22 @@ export class UiUdfParametersComponent extends FieldArrayType implements OnInit { } private applyDisabledState(field: FormlyFieldConfig, disabled: boolean): void { - if (field.formControl) { - if (disabled) { - field.formControl.disable({ emitEvent: false }); - } else { - field.formControl.enable({ emitEvent: false }); - } - } + disabled ? field.formControl?.disable({ emitEvent: false }) : field.formControl?.enable({ emitEvent: false }); } - // Disable Name getNameField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { return this.getAttributeChild(rowField, "attributeName"); } - // Disable Type getTypeField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { return this.getAttributeChild(rowField, "attributeType"); } - // Value editable getValueField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { return this.getField(rowField, "value"); } - trackByParamName = (index: number, param: any): string | number => { - return param?.attribute?.attributeName ?? index; + trackByParameterName = (index: number, parameter: any): string | number => { + return parameter?.attribute?.attributeName ?? index; }; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index fa21e88ffcb..6adc2356690 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -17,7 +17,11 @@ * under the License. */ -import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { + UiUdfParametersParseError, + UiUdfParametersParserService, + type UiUdfParameter, +} from "./ui-udf-parameters-parser.service"; describe("UiUdfParametersParserService", () => { let service: UiUdfParametersParserService; @@ -38,11 +42,11 @@ describe("UiUdfParametersParserService", () => { `; expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, - { attribute: { attributeName: "name", attributeType: "string" }, value: "" }, - { attribute: { attributeName: "age", attributeType: "long" }, value: "" }, - { attribute: { attributeName: "score", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "created_at", attributeType: "timestamp" }, value: "" }, + parameter("count", "integer"), + parameter("name", "string"), + parameter("age", "long"), + parameter("score", "double"), + parameter("created_at", "timestamp"), ]); }); @@ -53,9 +57,7 @@ describe("UiUdfParametersParserService", () => { self.UiParameter("count", attr_type=AttributeType.INT) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "count", attributeType: "integer" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("count", "integer")]); }); it("should parse multiline UiParameter calls with named arguments split across lines", () => { @@ -75,10 +77,7 @@ describe("UiUdfParametersParserService", () => { ) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("threshold", "double"), parameter("label", "string")]); }); it("should ignore calls where name or type is missing", () => { @@ -100,9 +99,7 @@ describe("UiUdfParametersParserService", () => { self.UiParameter(name="valid", type=AttributeType.STRING) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("valid", "string")]); }); it("should ignore legacy key= named argument", () => { @@ -139,7 +136,7 @@ describe("UiUdfParametersParserService", () => { expect(service.parse(code)).toEqual([]); }); - it("should parse supported UiParameter calls across multiple classes", () => { + it("should parse supported UiParameter calls when unsupported classes are present", () => { const code = ` class ProcessTupleOperator(UDFOperatorV2): def open(self): @@ -148,16 +145,24 @@ describe("UiUdfParametersParserService", () => { class RandomClass(ABC): def open(self): self.UiParameter("ignored", AttributeType.STRING) + `; + + expect(service.parse(code)).toEqual([parameter("threshold", "double")]); + }); + + it("should raise an error for multiple supported UDF classes because execution expects one concrete operator", () => { + const code = ` + class ProcessTupleOperator(UDFOperatorV2): + def open(self): + self.UiParameter("threshold", AttributeType.DOUBLE) class GenerateOperator(UDFSourceOperator): def open(self): self.UiParameter(name="batch_size", type=AttributeType.INT) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "batch_size", attributeType: "integer" }, value: "" }, - ]); + expect(() => service.parse(code)).toThrow(UiUdfParametersParseError); + expect(() => service.parse(code)).toThrow("Only one Python UDF class can declare UiParameter values."); }); it("should ignore empty and extra positional arguments", () => { @@ -169,12 +174,10 @@ describe("UiUdfParametersParserService", () => { self.UiParameter("valid", AttributeType.STRING) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "valid", attributeType: "string" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("valid", "string")]); }); - it("should keep the first duplicate parameter name", () => { + it("should raise an error for duplicate parameter names", () => { const code = ` class ProcessTupleOperator(UDFOperatorV2): def open(self): @@ -183,10 +186,8 @@ describe("UiUdfParametersParserService", () => { self.UiParameter("label", AttributeType.STRING) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "threshold", attributeType: "double" }, value: "" }, - { attribute: { attributeName: "label", attributeType: "string" }, value: "" }, - ]); + expect(() => service.parse(code)).toThrow(UiUdfParametersParseError); + expect(() => service.parse(code)).toThrow("UiParameter name 'threshold' is declared more than once."); }); it("should ignore commented out UiParameter calls", () => { @@ -197,9 +198,7 @@ describe("UiUdfParametersParserService", () => { self.UiParameter("active", AttributeType.INT) # self.UiParameter("trailing", AttributeType.STRING) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "active", attributeType: "integer" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("active", "integer")]); }); it("should ignore commented out multiline UiParameter sections", () => { @@ -216,9 +215,7 @@ describe("UiUdfParametersParserService", () => { ) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "active", attributeType: "string" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("active", "string")]); }); it("should ignore UiParameter examples inside triple-quoted strings", () => { @@ -231,9 +228,7 @@ describe("UiUdfParametersParserService", () => { self.UiParameter("active", AttributeType.DOUBLE) `; - expect(service.parse(code)).toEqual([ - { attribute: { attributeName: "active", attributeType: "double" }, value: "" }, - ]); + expect(service.parse(code)).toEqual([parameter("active", "double")]); }); it("should reject binary UiParameter types", () => { @@ -247,3 +242,7 @@ describe("UiUdfParametersParserService", () => { expect(service.parse(code)).toEqual([]); }); }); + +function parameter(attributeName: string, attributeType: UiUdfParameter["attribute"]["attributeType"]): UiUdfParameter { + return { attribute: { attributeName, attributeType }, value: "" }; +} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index e63d2e044a9..0c4bf6062d6 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -19,47 +19,26 @@ import { Injectable } from "@angular/core"; import { parser } from "@lezer/python"; -import { AttributeType, UiUdfParameter } from "../../types/workflow-compiling.interface"; +import { AttributeType, SchemaAttribute } from "../../types/workflow-compiling.interface"; // Keep in sync with Python UDF template class names in PythonUDFOpDescV2, DualInputPortsPythonUDFOpDescV2, and PythonUDFSourceOpDescV2. -const CLASSES = new Set(["ProcessTupleOperator", "ProcessBatchOperator", "ProcessTableOperator", "GenerateOperator"]); - -// Java enum constant names (AttributeType.java) -const JAVA_ATTRIBUTE_TYPE_NAMES = [ - "STRING", - "INTEGER", - "LONG", - "DOUBLE", - "BOOLEAN", - "TIMESTAMP", - "BINARY", - "LARGE_BINARY", -] as const; - -type JavaAttributeTypeName = (typeof JAVA_ATTRIBUTE_TYPE_NAMES)[number]; - -// Python enum constant names (core.models.AttributeType) -const PYTHON_ATTRIBUTE_TYPE_NAMES = [ - "STRING", - "INT", - "LONG", - "DOUBLE", - "BOOL", - "TIMESTAMP", - "BINARY", - "LARGE_BINARY", -] as const; - -type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number]; - -type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName; -type UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY"; -type SupportedParserAttributeTypeToken = Exclude; +const SUPPORTED_CLASS_NAMES = new Set([ + "ProcessTupleOperator", + "ProcessBatchOperator", + "ProcessTableOperator", + "GenerateOperator", +]); + type ParserSyntaxNode = ReturnType["topNode"]; type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; type UiParameterArgument = Readonly<{ kind: "name"; value: string }> | Readonly<{ kind: "type"; value: AttributeType }>; -const TYPES: Readonly> = { +export type UiUdfParameter = Readonly<{ attribute: SchemaAttribute; value: string }>; + +export class UiUdfParametersParseError extends Error {} + +// Accept Java enum names (INTEGER, BOOLEAN) and Python enum aliases (INT, BOOL). +const ATTRIBUTE_TYPES_BY_TOKEN: Readonly> = { STRING: "string", INTEGER: "integer", INT: "integer", @@ -70,17 +49,6 @@ const TYPES: Readonly> TIMESTAMP: "timestamp", }; -const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set(JAVA_ATTRIBUTE_TYPE_NAMES); -const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set(PYTHON_ATTRIBUTE_TYPE_NAMES); -const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set([ - "string", - "integer", - "long", - "double", - "boolean", - "timestamp", -]); - @Injectable({ providedIn: "root" }) export class UiUdfParametersParserService { parse(code: string): UiUdfParameter[] { @@ -88,9 +56,15 @@ export class UiUdfParametersParserService { const result: UiUdfParameter[] = []; const seen = new Set(); - const add = (parameter?: UiUdfParameter): void => { + let supportedClassCount = 0; + let duplicateName: string | undefined; + const addParameter = (parameter?: UiUdfParameter): void => { const name = parameter?.attribute.attributeName; - if (parameter && name && !seen.has(name)) { + if (parameter && name) { + if (seen.has(name)) { + duplicateName = name; + return; + } seen.add(name); result.push(parameter); } @@ -99,54 +73,69 @@ export class UiUdfParametersParserService { parser.parse(code).iterate({ enter: ({ name, node }) => { const className = node.getChild("VariableName"); - if (name !== "ClassDefinition" || !className || !CLASSES.has(code.slice(className.from, className.to))) return; - node.cursor().iterate(ref => { - if (ref.name !== "CallExpression") return; - add(readCall(ref.node, code)); + if ( + name !== "ClassDefinition" || + !className || + !SUPPORTED_CLASS_NAMES.has(code.slice(className.from, className.to)) + ) + return; + supportedClassCount++; + node.cursor().iterate(cursorReference => { + if (cursorReference.name !== "CallExpression") return; + addParameter(readCall(cursorReference.node, code)); return false; }); return false; }, }); + if (supportedClassCount > 1) { + throw new UiUdfParametersParseError("Only one Python UDF class can declare UiParameter values."); + } + + if (duplicateName) { + throw new UiUdfParametersParseError(`UiParameter name '${duplicateName}' is declared more than once.`); + } + return result; } } function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefined { - const args = call.getChild("ArgList"); - if (!args || code.slice(call.from, args.from).replace(/\s+/g, "") !== "self.UiParameter") return undefined; + const argumentList = call.getChild("ArgList"); + if (!argumentList || code.slice(call.from, argumentList.from).replace(/\s+/g, "") !== "self.UiParameter") + return undefined; let attributeName: string | undefined; let attributeType: AttributeType | undefined; let positionalIndex = 0; let sawNamed = false; - for (const arg of readArguments(args, code)) { - if (arg.key) { + for (const argument of readArguments(argumentList, code)) { + if (argument.key) { sawNamed = true; } else if (sawNamed) { return undefined; } - const parsedArg = readArgument(arg, positionalIndex, code); - if (!parsedArg) { + const parsedArgument = readArgument(argument, positionalIndex, code); + if (!parsedArgument) { return undefined; } - if (parsedArg.kind === "name") { + if (parsedArgument.kind === "name") { if (attributeName) { return undefined; } - attributeName = parsedArg.value; + attributeName = parsedArgument.value; } else { if (attributeType) { return undefined; } - attributeType = parsedArg.value; + attributeType = parsedArgument.value; } - if (!arg.key) { + if (!argument.key) { positionalIndex++; } } @@ -154,9 +143,9 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; } -function readArguments(args: ParserSyntaxNode, code: string): ParsedArgument[] { +function readArguments(argumentList: ParserSyntaxNode, code: string): ParsedArgument[] { const result: ParsedArgument[] = []; - const children = getChildren(args).filter(node => !["(", ")", ","].includes(node.name)); + const children = getChildren(argumentList).filter(node => !["(", ")", ","].includes(node.name)); for (let index = 0; index < children.length; index++) { const node = children[index]; @@ -186,18 +175,22 @@ function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { return children; } -function readArgument(arg: ParsedArgument, positionalIndex: number, code: string): UiParameterArgument | undefined { - const key = arg.key; - const value = code.slice(arg.value.from, arg.value.to); +function readArgument( + argument: ParsedArgument, + positionalIndex: number, + code: string +): UiParameterArgument | undefined { + const key = argument.key; + const value = code.slice(argument.value.from, argument.value.to); - if ((key === "name" || (!key && positionalIndex === 0)) && arg.value.name === "String") { + if ((key === "name" || (!key && positionalIndex === 0)) && argument.value.name === "String") { const name = readString(value)?.trim(); return name ? { kind: "name", value: name } : undefined; } if ( (key === "type" || key === "attr_type" || (!key && positionalIndex === 1)) && - arg.value.name === "MemberExpression" + argument.value.name === "MemberExpression" ) { const type = readType(value); return type ? { kind: "type", value: type } : undefined; @@ -224,14 +217,5 @@ function readType(input: string): AttributeType | undefined { return undefined; } - if (!JAVA_ATTRIBUTE_TYPE_NAME_SET.has(token) && !PYTHON_ATTRIBUTE_TYPE_NAME_SET.has(token)) { - return undefined; - } - - if (token === "BINARY" || token === "LARGE_BINARY") { - return undefined; - } - - const canonical = TYPES[token as SupportedParserAttributeTypeToken]; - return SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES.has(canonical) ? canonical : undefined; + return ATTRIBUTE_TYPES_BY_TOKEN[token]; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts index beb7fa15adb..20b1afd38cb 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -19,22 +19,29 @@ import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { PYTHON_UDF_V2_OP_TYPE } from "../workflow-graph/model/workflow-graph"; -import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { UiUdfParametersParseError, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import type { UiUdfParameter } from "./ui-udf-parameters-parser.service"; import { UiUdfParametersSyncService } from "./ui-udf-parameters-sync.service"; -import { UiUdfParameter } from "../../types/workflow-compiling.interface"; -import { vi } from "vitest"; -import * as Y from "yjs"; +import type { Mock } from "vitest"; +import { vi as vitest } from "vitest"; +import * as Yjs from "yjs"; + +type MockFunction = Mock; + +function createMockFunction(): MockFunction { + return vitest.fn(); +} describe("UiUdfParametersSyncService", () => { const operatorId = "operator-1"; const code = "self.UiParameter(...)"; let service: UiUdfParametersSyncService; - let workflowActionServiceSpy: { getTexeraGraph: ReturnType }; - let parserServiceSpy: { parse: ReturnType }; - let graphSpy: { - getOperator: ReturnType; - getSharedOperatorType: ReturnType; + let workflowActionServiceMock: { getTexeraGraph: MockFunction }; + let parserServiceMock: { parse: MockFunction }; + let graphMock: { + getOperator: MockFunction; + getSharedOperatorType: MockFunction; }; let operator: { operatorType: string; @@ -47,37 +54,39 @@ describe("UiUdfParametersSyncService", () => { operatorProperties: { uiParameters: [] }, }; - graphSpy = { - getOperator: vi.fn().mockImplementation((id: string) => (id === operatorId ? operator : undefined)), - getSharedOperatorType: vi.fn(), + graphMock = { + getOperator: createMockFunction().mockImplementation((requestedOperatorId: string) => + requestedOperatorId === operatorId ? operator : undefined + ), + getSharedOperatorType: createMockFunction(), }; - workflowActionServiceSpy = { - getTexeraGraph: vi.fn().mockReturnValue(graphSpy), + workflowActionServiceMock = { + getTexeraGraph: createMockFunction().mockReturnValue(graphMock), }; - parserServiceSpy = { parse: vi.fn() }; + parserServiceMock = { parse: createMockFunction() }; service = new UiUdfParametersSyncService( - workflowActionServiceSpy as unknown as WorkflowActionService, - parserServiceSpy as unknown as UiUdfParametersParserService + workflowActionServiceMock as unknown as WorkflowActionService, + parserServiceMock as unknown as UiUdfParametersParserService ); }); it("should emit parameters that preserve values from current parameter names", () => { operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); + parserServiceMock.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + const parametersChangedObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); service.syncStructureFromCode(operatorId, code); - expect(nextSpy).toHaveBeenCalledWith({ + expect(parametersChangedObserver).toHaveBeenCalledWith({ operatorId, parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], }); - expect(nextSpy).toHaveBeenCalledOnce(); + expect(parametersChangedObserver).toHaveBeenCalledOnce(); }); it("should emit updated parameters with preserved values and removed stale parameters", () => { @@ -85,94 +94,114 @@ describe("UiUdfParametersSyncService", () => { createParameter("count", "integer", "42"), createParameter("removed", "string", "stale"), ]; - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); + parserServiceMock.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + const parametersChangedObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); service.syncStructureFromCode(operatorId, code); - expect(nextSpy).toHaveBeenCalledWith({ + expect(parametersChangedObserver).toHaveBeenCalledWith({ operatorId, parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], }); - expect(nextSpy).toHaveBeenCalledOnce(); + expect(parametersChangedObserver).toHaveBeenCalledOnce(); }); it("should not emit when the merged parameters are unchanged", () => { operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + parserServiceMock.parse.mockReturnValue([createParameter("count", "integer")]); - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + const parametersChangedObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); service.syncStructureFromCode(operatorId, code); - expect(nextSpy).not.toHaveBeenCalled(); + expect(parametersChangedObserver).not.toHaveBeenCalled(); + }); + + it("should emit parser errors without replacing the current parameters", () => { + operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; + parserServiceMock.parse.mockImplementation(() => { + throw new UiUdfParametersParseError("Only one Python UDF class can declare UiParameter values."); + }); + + const parametersChangedObserver = createMockFunction(); + const parseErrorObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); + service.uiParametersParseError$.subscribe(parseErrorObserver); + + service.syncStructureFromCode(operatorId, code); + + expect(parametersChangedObserver).not.toHaveBeenCalled(); + expect(parseErrorObserver).toHaveBeenCalledWith({ + operatorId, + message: "Only one Python UDF class can declare UiParameter values.", + }); }); it("should not parse code for non-Python UDF operators", () => { operator.operatorType = "Projection"; - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + const parametersChangedObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); service.syncStructureFromCode(operatorId, code); - expect(parserServiceSpy.parse).not.toHaveBeenCalled(); - expect(nextSpy).not.toHaveBeenCalled(); + expect(parserServiceMock.parse).not.toHaveBeenCalled(); + expect(parametersChangedObserver).not.toHaveBeenCalled(); }); it("should read code from the shared operator property when editor code is omitted", () => { const sharedCode = 'self.UiParameter("count", AttributeType.INT)'; - graphSpy.getSharedOperatorType.mockReturnValue(createSharedOperatorType(sharedCode)); - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + graphMock.getSharedOperatorType.mockReturnValue(createSharedOperatorType(sharedCode)); + parserServiceMock.parse.mockReturnValue([createParameter("count", "integer")]); - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + const parametersChangedObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); service.syncStructureFromCode(operatorId); - expect(parserServiceSpy.parse).toHaveBeenCalledWith(sharedCode); - expect(nextSpy).toHaveBeenCalledWith({ + expect(parserServiceMock.parse).toHaveBeenCalledWith(sharedCode); + expect(parametersChangedObserver).toHaveBeenCalledWith({ operatorId, parameters: [createParameter("count", "integer")], }); }); it("should debounce YText changes and clean up the observer", () => { - vi.useFakeTimers(); + vitest.useFakeTimers(); try { - const yCode = createYText('self.UiParameter("count", AttributeType.INT)'); - parserServiceSpy.parse.mockReturnValue([createParameter("count", "integer")]); + const sharedCodeText = createSharedText('self.UiParameter("count", AttributeType.INT)'); + parserServiceMock.parse.mockReturnValue([createParameter("count", "integer")]); - const nextSpy = vi.fn(); - service.uiParametersChanged$.subscribe(nextSpy); + const parametersChangedObserver = createMockFunction(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); - const cleanup = service.attachToYCode(operatorId, yCode); + const cleanup = service.attachToYCode(operatorId, sharedCodeText); - expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); - expect(nextSpy).toHaveBeenCalledOnce(); + expect(parserServiceMock.parse).toHaveBeenCalledOnce(); + expect(parametersChangedObserver).toHaveBeenCalledOnce(); - yCode.insert(yCode.length, "\n# changed"); + sharedCodeText.insert(sharedCodeText.length, "\n# changed"); - vi.advanceTimersByTime(199); + vitest.advanceTimersByTime(199); - expect(parserServiceSpy.parse).toHaveBeenCalledOnce(); + expect(parserServiceMock.parse).toHaveBeenCalledOnce(); - vi.advanceTimersByTime(1); + vitest.advanceTimersByTime(1); - expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); - expect(nextSpy).toHaveBeenCalledTimes(2); + expect(parserServiceMock.parse).toHaveBeenCalledTimes(2); + expect(parametersChangedObserver).toHaveBeenCalledTimes(2); cleanup(); - yCode.insert(yCode.length, "\n# after cleanup"); - vi.advanceTimersByTime(200); + sharedCodeText.insert(sharedCodeText.length, "\n# after cleanup"); + vitest.advanceTimersByTime(200); - expect(parserServiceSpy.parse).toHaveBeenCalledTimes(2); - expect(nextSpy).toHaveBeenCalledTimes(2); + expect(parserServiceMock.parse).toHaveBeenCalledTimes(2); + expect(parametersChangedObserver).toHaveBeenCalledTimes(2); } finally { - vi.useRealTimers(); + vitest.useRealTimers(); } }); }); @@ -187,24 +216,24 @@ function createParameter(name: string, type: UiUdfParameter["attribute"]["attrib }; } -function createSharedOperatorType(code: string): Y.Map { - const doc = new Y.Doc(); - const sharedOperator = doc.getMap("operator"); - const operatorProperties = new Y.Map(); - const yCode = new Y.Text(); +function createSharedOperatorType(code: string): Yjs.Map { + const yjsDocument = new Yjs.Doc(); + const sharedOperator = yjsDocument.getMap("operator"); + const operatorProperties = new Yjs.Map(); + const sharedCodeText = new Yjs.Text(); - operatorProperties.set("code", yCode); + operatorProperties.set("code", sharedCodeText); sharedOperator.set("operatorProperties", operatorProperties); - yCode.insert(0, code); + sharedCodeText.insert(0, code); return sharedOperator; } -function createYText(text: string): Y.Text { - const doc = new Y.Doc(); - const yMap = doc.getMap("root"); - const yText = new Y.Text(); - yMap.set("code", yText); - yText.insert(0, text); - return yText; +function createSharedText(text: string): Yjs.Text { + const yjsDocument = new Yjs.Doc(); + const sharedRootMap = yjsDocument.getMap("root"); + const sharedText = new Yjs.Text(); + sharedRootMap.set("code", sharedText); + sharedText.insert(0, text); + return sharedText; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts index 0e45e5138af..ebf680c20bb 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -21,12 +21,12 @@ import { isEqual } from "lodash-es"; import { ReplaySubject, Subject } from "rxjs"; import { debounceTime } from "rxjs/operators"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; -import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { UiUdfParametersParseError, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import type { UiUdfParameter } from "./ui-udf-parameters-parser.service"; import { isDefined } from "../../../common/util/predicate"; import { isPythonUdf } from "../workflow-graph/model/workflow-graph"; import { YType } from "../../types/shared-editing.interface"; import type { Text as YText } from "yjs"; -import { UiUdfParameter } from "../../types/workflow-compiling.interface"; const UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS = 200; @@ -35,8 +35,10 @@ export class UiUdfParametersSyncService { private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>( 1 ); + private readonly uiParametersParseErrorSubject = new ReplaySubject<{ operatorId: string; message?: string }>(1); readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable(); + readonly uiParametersParseError$ = this.uiParametersParseErrorSubject.asObservable(); constructor( private workflowActionService: WorkflowActionService, @@ -81,7 +83,19 @@ export class UiUdfParametersSyncService { } const existingParameters = (operator.operatorProperties?.uiParameters ?? []) as UiUdfParameter[]; - const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); + let mergedUiParameters: UiUdfParameter[]; + + try { + mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); + } catch (error) { + if (error instanceof UiUdfParametersParseError) { + this.uiParametersParseErrorSubject.next({ operatorId, message: error.message }); + return; + } + throw error; + } + + this.uiParametersParseErrorSubject.next({ operatorId }); if (isEqual(existingParameters, mergedUiParameters)) { return; diff --git a/frontend/src/app/workspace/types/workflow-compiling.interface.ts b/frontend/src/app/workspace/types/workflow-compiling.interface.ts index d9f21a20341..8c4499d3d4c 100644 --- a/frontend/src/app/workspace/types/workflow-compiling.interface.ts +++ b/frontend/src/app/workspace/types/workflow-compiling.interface.ts @@ -78,12 +78,6 @@ export interface SchemaAttribute attributeType: AttributeType; }> {} -export interface UiUdfParameter - extends Readonly<{ - attribute: SchemaAttribute; - value: string; - }> {} - export type PortSchema = ReadonlyArray; // schema of an operator: a map from serialized PortIdentity to port schema From 9de4c3ae3a380c64fdec7a869a1f621ce11086e6 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 19 May 2026 23:06:58 -0600 Subject: [PATCH 08/13] refactor(frontend): compact UI parameter foundation --- .../ui-udf-parameters.component.html | 36 +- .../ui-udf-parameters.component.scss | 2 - .../ui-udf-parameters.component.spec.ts | 14 +- .../ui-udf-parameters.component.ts | 35 +- .../ui-udf-parameters-parser.service.spec.ts | 375 ++++++++---------- .../ui-udf-parameters-parser.service.ts | 84 ++-- .../ui-udf-parameters-sync.service.spec.ts | 171 ++++---- .../ui-udf-parameters-sync.service.ts | 56 +-- 8 files changed, 314 insertions(+), 459 deletions(-) diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html index 5d305c395d2..c56391ab474 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html @@ -18,37 +18,25 @@ -->
- + *ngIf="model?.length">
-
Value
-
Name
-
Type
+
+ {{ column.label }} +
- -
- - - -
- - -
- - - -
- - -
- - - +
+
diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss index dff89283ab8..ab6b2ad6ee1 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.scss @@ -28,12 +28,10 @@ min-width: 0; } -/* Remove Formly/Ant label spacing */ :host ::ng-deep .ant-form-item { margin-bottom: 0; } -/* Hide Formly labels*/ :host ::ng-deep .ant-form-item-label { display: none; } diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts index 1da883e37e8..26325f8ad5d 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts @@ -50,16 +50,14 @@ describe("UiUdfParametersComponent", () => { component.ngOnInit(); - expect(component.getValueField(rowField)).toBe(valueField); - expect(component.getNameField(rowField)).toBe(nameField); - expect(component.getTypeField(rowField)).toBe(typeField); - // templateOptions is deprecated, but some existing Formly wrappers still read it. [ - [valueField, valueControl, false], - [nameField, nameControl, true], - [typeField, typeControl, true], - ].forEach(([field, control, disabled]) => { + { column: component.fieldColumns[0], field: valueField, control: valueControl }, + { column: component.fieldColumns[1], field: nameField, control: nameControl }, + { column: component.fieldColumns[2], field: typeField, control: typeControl }, + ].forEach(({ column, field, control }) => { + expect(component.getColumnField(rowField, column)).toBe(field); + const disabled = column.disabled; expect((field as FormlyFieldConfig).props?.disabled).toBe(disabled); expect((field as any).templateOptions?.disabled).toBe(disabled); expect((control as FormControl).disabled).toBe(disabled); diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index fc2bfb616a5..bbb8657500f 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -20,6 +20,8 @@ import { Component, OnInit } from "@angular/core"; import { NgFor, NgIf } from "@angular/common"; import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; +type UiUdfParameterColumn = Readonly<{ label: string; key: string; parentKey?: string; disabled: boolean }>; + @Component({ selector: "texera-ui-udf-parameters", templateUrl: "./ui-udf-parameters.component.html", @@ -27,21 +29,26 @@ import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/cor imports: [NgIf, NgFor, FormlyModule], }) export class UiUdfParametersComponent extends FieldArrayType implements OnInit { + readonly fieldColumns: UiUdfParameterColumn[] = [ + { label: "Value", key: "value", disabled: false }, + { label: "Name", key: "attributeName", parentKey: "attribute", disabled: true }, + { label: "Type", key: "attributeType", parentKey: "attribute", disabled: true }, + ]; + ngOnInit(): void { this.field.fieldGroup?.forEach(rowField => { - this.configureDisabledState(this.getAttributeChild(rowField, "attributeName"), true); - this.configureDisabledState(this.getAttributeChild(rowField, "attributeType"), true); - this.configureDisabledState(this.getField(rowField, "value"), false); + this.fieldColumns.forEach(column => { + this.configureDisabledState(this.getColumnField(rowField, column), column.disabled); + }); }); } - private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined { - return rowField.fieldGroup?.find(fieldConfig => fieldConfig.key === key); + getColumnField(rowField: FormlyFieldConfig, column: UiUdfParameterColumn): FormlyFieldConfig | undefined { + return this.getChildField(column.parentKey ? this.getChildField(rowField, column.parentKey) : rowField, column.key); } - private getAttributeChild(rowField: FormlyFieldConfig, childKey: string): FormlyFieldConfig | undefined { - const attributeGroup = this.getField(rowField, "attribute"); - return attributeGroup?.fieldGroup?.find(fieldConfig => fieldConfig.key === childKey); + private getChildField(rowField: FormlyFieldConfig | undefined, key: string): FormlyFieldConfig | undefined { + return rowField?.fieldGroup?.find(fieldConfig => fieldConfig.key === key); } private configureDisabledState(field: FormlyFieldConfig | undefined, disabled: boolean): void { @@ -68,18 +75,6 @@ export class UiUdfParametersComponent extends FieldArrayType implements OnInit { disabled ? field.formControl?.disable({ emitEvent: false }) : field.formControl?.enable({ emitEvent: false }); } - getNameField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.getAttributeChild(rowField, "attributeName"); - } - - getTypeField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.getAttributeChild(rowField, "attributeType"); - } - - getValueField(rowField: FormlyFieldConfig): FormlyFieldConfig | undefined { - return this.getField(rowField, "value"); - } - trackByParameterName = (index: number, parameter: any): string | number => { return parameter?.attribute?.attributeName ?? index; }; diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index 6adc2356690..e865df82c9a 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -23,6 +23,9 @@ import { type UiUdfParameter, } from "./ui-udf-parameters-parser.service"; +const MULTIPLE_SUPPORTED_CLASSES_ERROR = "Only one Python UDF class can declare UiParameter values."; +const DUPLICATE_NAME_ERROR = "UiParameter name 'threshold' is declared more than once."; + describe("UiUdfParametersParserService", () => { let service: UiUdfParametersParserService; @@ -30,218 +33,192 @@ describe("UiUdfParametersParserService", () => { service = new UiUdfParametersParserService(); }); - it("should parse Python-compatible positional and name-based arguments", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("count", AttributeType.INT) - self.UiParameter(type=AttributeType.STRING, name="name") - self.UiParameter(name="age", type=AttributeType.LONG) - self.UiParameter("score", AttributeType.DOUBLE) - self.UiParameter("created_at", type=AttributeType.TIMESTAMP) - `; - - expect(service.parse(code)).toEqual([ - parameter("count", "integer"), - parameter("name", "string"), - parameter("age", "long"), - parameter("score", "double"), - parameter("created_at", "timestamp"), - ]); - }); - - it("should parse the attr_type keyword used by the Python API", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("count", attr_type=AttributeType.INT) - `; - - expect(service.parse(code)).toEqual([parameter("count", "integer")]); - }); - - it("should parse multiline UiParameter calls with named arguments split across lines", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter( - name= - "threshold", - type= - AttributeType.DOUBLE, - ) - self.UiParameter( - "label", - type= - AttributeType.STRING, - ) - `; - - expect(service.parse(code)).toEqual([parameter("threshold", "double"), parameter("label", "string")]); - }); - - it("should ignore calls where name or type is missing", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter(name="a") - self.UiParameter(type=AttributeType.DOUBLE) - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should ignore invalid positional argument ordering", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter(AttributeType.INT, "count") - self.UiParameter(name="valid", type=AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([parameter("valid", "string")]); - }); - - it("should ignore legacy key= named argument", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter(type=AttributeType.DOUBLE, key="a") - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should ignore unsupported classes", () => { - const code = ` - class RandomClass(ABC): - def open(self): - self.UiParameter(type=AttributeType.DOUBLE, name="a") - `; - - expect(service.parse(code)).toEqual([]); - }); - - it("should ignore custom-named subclasses because injection targets template class names", () => { - const code = ` - class MyTupleOp(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - - class MyWrappedTupleOp(ProcessTupleOperator): - def open(self): - self.UiParameter("label", AttributeType.STRING) - `; + it("should parse supported positional, named, and attr_type arguments", () => { + expectParsed( + service, + ` + self.UiParameter("count", AttributeType.INT) + self.UiParameter(type=AttributeType.STRING, name="name") + self.UiParameter(name="age", type=AttributeType.LONG) + self.UiParameter("score", AttributeType.DOUBLE) + self.UiParameter("enabled", AttributeType.BOOL) + self.UiParameter("created_at", type=AttributeType.TIMESTAMP) + self.UiParameter("alias", attr_type=AttributeType.INTEGER) + `, + [ + parameter("count", "integer"), + parameter("name", "string"), + parameter("age", "long"), + parameter("score", "double"), + parameter("enabled", "boolean"), + parameter("created_at", "timestamp"), + parameter("alias", "integer"), + ] + ); + }); + + it("should parse multiline UiParameter calls with split arguments", () => { + expectParsed( + service, + ` + self.UiParameter( + name= + "threshold", + type= + AttributeType.DOUBLE, + ) + self.UiParameter( + "label", + type= + AttributeType.STRING, + ) + `, + [parameter("threshold", "double"), parameter("label", "string")] + ); + }); + + ( + [ + [ + "ignore calls where name or type is missing", + ` + self.UiParameter(name="a") + self.UiParameter(type=AttributeType.DOUBLE) + `, + [], + ], + [ + "ignore invalid positional argument ordering", + ` + self.UiParameter(AttributeType.INT, "count") + self.UiParameter(name="valid", type=AttributeType.STRING) + `, + [parameter("valid", "string")], + ], + ["ignore legacy key= named argument", 'self.UiParameter(type=AttributeType.DOUBLE, key="a")', []], + [ + "ignore empty and extra positional arguments", + ` + self.UiParameter() + self.UiParameter("too_many", AttributeType.STRING, "extra") + self.UiParameter("valid", AttributeType.STRING) + `, + [parameter("valid", "string")], + ], + [ + "ignore commented out UiParameter calls", + ` + # self.UiParameter("commented", AttributeType.INT) + self.UiParameter("active", AttributeType.INT) # self.UiParameter("trailing", AttributeType.STRING) + `, + [parameter("active", "integer")], + ], + [ + "ignore commented out multiline UiParameter sections", + ` + # self.UiParameter( + # name="commented", + # type=AttributeType.INT, + # ) + self.UiParameter(name="active", type=AttributeType.STRING) + `, + [parameter("active", "string")], + ], + [ + "ignore UiParameter examples inside triple-quoted strings", + ` + """ + self.UiParameter("example", AttributeType.INT) + """ + self.UiParameter("active", AttributeType.DOUBLE) + `, + [parameter("active", "double")], + ], + [ + "reject binary UiParameter types", + ` + self.UiParameter("payload", AttributeType.BINARY) + self.UiParameter("blob", AttributeType.LARGE_BINARY) + `, + [], + ], + ] as ReadonlyArray + ).forEach(([description, openBody, expectedParameters]) => { + it(`should ${description}`, () => { + expectParsed(service, openBody, expectedParameters); + }); + }); + + it("should ignore unsupported classes and custom-named subclasses", () => { + const code = [ + pythonClass('self.UiParameter(type=AttributeType.DOUBLE, name="a")', "RandomClass", "ABC"), + pythonClass('self.UiParameter("threshold", AttributeType.DOUBLE)', "MyTupleOp"), + pythonClass('self.UiParameter("label", AttributeType.STRING)', "MyWrappedTupleOp", "ProcessTupleOperator"), + ].join("\n"); expect(service.parse(code)).toEqual([]); }); it("should parse supported UiParameter calls when unsupported classes are present", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - - class RandomClass(ABC): - def open(self): - self.UiParameter("ignored", AttributeType.STRING) - `; + const code = [ + pythonClass('self.UiParameter("threshold", AttributeType.DOUBLE)'), + pythonClass('self.UiParameter("ignored", AttributeType.STRING)', "RandomClass", "ABC"), + ].join("\n"); expect(service.parse(code)).toEqual([parameter("threshold", "double")]); }); - it("should raise an error for multiple supported UDF classes because execution expects one concrete operator", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - - class GenerateOperator(UDFSourceOperator): - def open(self): - self.UiParameter(name="batch_size", type=AttributeType.INT) - `; - - expect(() => service.parse(code)).toThrow(UiUdfParametersParseError); - expect(() => service.parse(code)).toThrow("Only one Python UDF class can declare UiParameter values."); - }); - - it("should ignore empty and extra positional arguments", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter() - self.UiParameter("too_many", AttributeType.STRING, "extra") - self.UiParameter("valid", AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([parameter("valid", "string")]); - }); - - it("should raise an error for duplicate parameter names", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("threshold", AttributeType.DOUBLE) - self.UiParameter("threshold", AttributeType.STRING) - self.UiParameter("label", AttributeType.STRING) - `; - - expect(() => service.parse(code)).toThrow(UiUdfParametersParseError); - expect(() => service.parse(code)).toThrow("UiParameter name 'threshold' is declared more than once."); - }); - - it("should ignore commented out UiParameter calls", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - # self.UiParameter("commented", AttributeType.INT) - self.UiParameter("active", AttributeType.INT) # self.UiParameter("trailing", AttributeType.STRING) - `; - - expect(service.parse(code)).toEqual([parameter("active", "integer")]); - }); - - it("should ignore commented out multiline UiParameter sections", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - # self.UiParameter( - # name="commented", - # type=AttributeType.INT, - # ) - self.UiParameter( - name="active", - type=AttributeType.STRING, - ) - `; - - expect(service.parse(code)).toEqual([parameter("active", "string")]); + [ + { + description: "multiple supported UDF classes", + code: [ + pythonClass('self.UiParameter("threshold", AttributeType.DOUBLE)', "ProcessTupleOperator"), + pythonClass('self.UiParameter(name="batch_size", type=AttributeType.INT)', "GenerateOperator"), + ].join("\n"), + message: MULTIPLE_SUPPORTED_CLASSES_ERROR, + }, + { + description: "duplicate parameter names", + code: pythonClass(` + self.UiParameter("threshold", AttributeType.DOUBLE) + self.UiParameter("threshold", AttributeType.STRING) + self.UiParameter("label", AttributeType.STRING) + `), + message: DUPLICATE_NAME_ERROR, + }, + ].forEach(({ description, code, message }) => { + it(`should raise an error for ${description}`, () => { + expectParseError(service, code, message); + }); }); +}); - it("should ignore UiParameter examples inside triple-quoted strings", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - """ - self.UiParameter("example", AttributeType.INT) - """ - self.UiParameter("active", AttributeType.DOUBLE) - `; - - expect(service.parse(code)).toEqual([parameter("active", "double")]); - }); +function expectParsed( + service: UiUdfParametersParserService, + openBody: string, + expectedParameters: UiUdfParameter[] +): void { + expect(service.parse(pythonClass(openBody))).toEqual(expectedParameters); +} - it("should reject binary UiParameter types", () => { - const code = ` - class ProcessTupleOperator(UDFOperatorV2): - def open(self): - self.UiParameter("payload", AttributeType.BINARY) - self.UiParameter("blob", AttributeType.LARGE_BINARY) - `; +function expectParseError(service: UiUdfParametersParserService, code: string, message: string): void { + expect(() => service.parse(code)).toThrow(UiUdfParametersParseError); + expect(() => service.parse(code)).toThrow(message); +} - expect(service.parse(code)).toEqual([]); - }); -}); +function pythonClass(openBody: string, className = "ProcessTupleOperator", baseClass = "UDFOperatorV2"): string { + const openStatements = openBody + .trim() + .split("\n") + .map(line => ` ${line.trim()}`) + .join("\n"); + + return ` + class ${className}(${baseClass}): + def open(self): +${openStatements} + `; +} function parameter(attributeName: string, attributeType: UiUdfParameter["attribute"]["attributeType"]): UiUdfParameter { return { attribute: { attributeName, attributeType }, value: "" }; diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 0c4bf6062d6..37210c6c211 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -31,7 +31,6 @@ const SUPPORTED_CLASS_NAMES = new Set([ type ParserSyntaxNode = ReturnType["topNode"]; type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; -type UiParameterArgument = Readonly<{ kind: "name"; value: string }> | Readonly<{ kind: "type"; value: AttributeType }>; export type UiUdfParameter = Readonly<{ attribute: SchemaAttribute; value: string }>; @@ -89,13 +88,11 @@ export class UiUdfParametersParserService { }, }); - if (supportedClassCount > 1) { + if (supportedClassCount > 1) throw new UiUdfParametersParseError("Only one Python UDF class can declare UiParameter values."); - } - if (duplicateName) { + if (duplicateName) throw new UiUdfParametersParseError(`UiParameter name '${duplicateName}' is declared more than once.`); - } return result; } @@ -112,28 +109,20 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi let sawNamed = false; for (const argument of readArguments(argumentList, code)) { + const key = argument.key ?? (positionalIndex === 0 ? "name" : positionalIndex === 1 ? "type" : undefined); + if (argument.key) { sawNamed = true; } else if (sawNamed) { return undefined; } - const parsedArgument = readArgument(argument, positionalIndex, code); - if (!parsedArgument) { - return undefined; - } + if (key === "name" && !attributeName) attributeName = readName(argument.value, code); + else if ((key === "type" || key === "attr_type") && !attributeType) attributeType = readType(argument.value, code); + else return undefined; - if (parsedArgument.kind === "name") { - if (attributeName) { - return undefined; - } - attributeName = parsedArgument.value; - } else { - if (attributeType) { - return undefined; - } - attributeType = parsedArgument.value; - } + if ((key === "name" && !attributeName) || ((key === "type" || key === "attr_type") && !attributeType)) + return undefined; if (!argument.key) { positionalIndex++; @@ -152,9 +141,7 @@ function readArguments(argumentList: ParserSyntaxNode, code: string): ParsedArgu if (node.name === "VariableName" && children[index + 1]?.name === "AssignOp") { const value = children[index + 2]; - if (!value) { - return []; - } + if (!value) return []; result.push({ key: code.slice(node.from, node.to), value }); index += 2; } else if (node.name !== "AssignOp") { @@ -169,34 +156,24 @@ function readArguments(argumentList: ParserSyntaxNode, code: string): ParsedArgu function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { const children: ParserSyntaxNode[] = []; - for (let child = node.firstChild; child; child = child.nextSibling) { - children.push(child); - } + for (let child = node.firstChild; child; child = child.nextSibling) children.push(child); return children; } -function readArgument( - argument: ParsedArgument, - positionalIndex: number, - code: string -): UiParameterArgument | undefined { - const key = argument.key; - const value = code.slice(argument.value.from, argument.value.to); - - if ((key === "name" || (!key && positionalIndex === 0)) && argument.value.name === "String") { - const name = readString(value)?.trim(); - return name ? { kind: "name", value: name } : undefined; - } - - if ( - (key === "type" || key === "attr_type" || (!key && positionalIndex === 1)) && - argument.value.name === "MemberExpression" - ) { - const type = readType(value); - return type ? { kind: "type", value: type } : undefined; - } +function readName(value: ParserSyntaxNode, code: string): string | undefined { + const name = value.name === "String" ? readString(code.slice(value.from, value.to))?.trim() : undefined; + return name || undefined; +} - return undefined; +function readType(value: ParserSyntaxNode, code: string): AttributeType | undefined { + if (value.name !== "MemberExpression") return undefined; + const token = code + .slice(value.from, value.to) + .trim() + .replace(/\s+/g, "") + .match(/^AttributeType\.([A-Za-z_]\w*)$/)?.[1] + .toUpperCase(); + return token ? ATTRIBUTE_TYPES_BY_TOKEN[token] : undefined; } function readString(input: string): string | undefined { @@ -206,16 +183,3 @@ function readString(input: string): string | undefined { ?.slice(1) .find(value => value !== undefined); } - -function readType(input: string): AttributeType | undefined { - const token = input - .trim() - .replace(/\s+/g, "") - .match(/^AttributeType\.([A-Za-z_]\w*)$/)?.[1] - .toUpperCase(); - if (!token) { - return undefined; - } - - return ATTRIBUTE_TYPES_BY_TOKEN[token]; -} diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts index 20b1afd38cb..415bb27d4d5 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -26,94 +26,64 @@ import type { Mock } from "vitest"; import { vi as vitest } from "vitest"; import * as Yjs from "yjs"; -type MockFunction = Mock; - -function createMockFunction(): MockFunction { - return vitest.fn(); -} - describe("UiUdfParametersSyncService", () => { const operatorId = "operator-1"; const code = "self.UiParameter(...)"; let service: UiUdfParametersSyncService; - let workflowActionServiceMock: { getTexeraGraph: MockFunction }; - let parserServiceMock: { parse: MockFunction }; - let graphMock: { - getOperator: MockFunction; - getSharedOperatorType: MockFunction; - }; - let operator: { - operatorType: string; - operatorProperties: { uiParameters: UiUdfParameter[] }; - }; + let parserServiceMock: { parse: Mock }; + let graphMock: { getOperator: Mock; getSharedOperatorType: Mock }; + let operator: { operatorType: string; operatorProperties: { uiParameters: UiUdfParameter[] } }; beforeEach(() => { - operator = { - operatorType: PYTHON_UDF_V2_OP_TYPE, - operatorProperties: { uiParameters: [] }, - }; - + operator = { operatorType: PYTHON_UDF_V2_OP_TYPE, operatorProperties: { uiParameters: [] } }; graphMock = { - getOperator: createMockFunction().mockImplementation((requestedOperatorId: string) => - requestedOperatorId === operatorId ? operator : undefined - ), - getSharedOperatorType: createMockFunction(), + getOperator: vitest + .fn() + .mockImplementation((requestedOperatorId: string) => + requestedOperatorId === operatorId ? operator : undefined + ), + getSharedOperatorType: vitest.fn(), }; - - workflowActionServiceMock = { - getTexeraGraph: createMockFunction().mockReturnValue(graphMock), - }; - - parserServiceMock = { parse: createMockFunction() }; - + parserServiceMock = { parse: vitest.fn() }; service = new UiUdfParametersSyncService( - workflowActionServiceMock as unknown as WorkflowActionService, + { getTexeraGraph: vitest.fn().mockReturnValue(graphMock) } as unknown as WorkflowActionService, parserServiceMock as unknown as UiUdfParametersParserService ); }); - it("should emit parameters that preserve values from current parameter names", () => { - operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; - parserServiceMock.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); - - const parametersChangedObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); - - service.syncStructureFromCode(operatorId, code); - - expect(parametersChangedObserver).toHaveBeenCalledWith({ - operatorId, - parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], - }); - expect(parametersChangedObserver).toHaveBeenCalledOnce(); - }); - - it("should emit updated parameters with preserved values and removed stale parameters", () => { - operator.operatorProperties.uiParameters = [ - createParameter("count", "integer", "42"), - createParameter("removed", "string", "stale"), - ]; - parserServiceMock.parse.mockReturnValue([createParameter("count", "integer"), createParameter("name", "string")]); + [ + { + description: "preserve values from current parameter names", + existingParameters: [parameter("count", "integer", "42")], + parsedParameters: [parameter("count", "integer"), parameter("name", "string")], + expectedParameters: [parameter("count", "integer", "42"), parameter("name", "string", "")], + }, + { + description: "remove stale parameters while preserving retained values", + existingParameters: [parameter("count", "integer", "42"), parameter("removed", "string", "stale")], + parsedParameters: [parameter("count", "integer"), parameter("name", "string")], + expectedParameters: [parameter("count", "integer", "42"), parameter("name", "string", "")], + }, + ].forEach(({ description, existingParameters, parsedParameters, expectedParameters }) => { + it(`should ${description}`, () => { + operator.operatorProperties.uiParameters = existingParameters; + parserServiceMock.parse.mockReturnValue(parsedParameters); - const parametersChangedObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); + const parametersChangedObserver = observeParameterChanges(); - service.syncStructureFromCode(operatorId, code); + service.syncStructureFromCode(operatorId, code); - expect(parametersChangedObserver).toHaveBeenCalledWith({ - operatorId, - parameters: [createParameter("count", "integer", "42"), createParameter("name", "string", "")], + expect(parametersChangedObserver).toHaveBeenCalledWith({ operatorId, parameters: expectedParameters }); + expect(parametersChangedObserver).toHaveBeenCalledOnce(); }); - expect(parametersChangedObserver).toHaveBeenCalledOnce(); }); it("should not emit when the merged parameters are unchanged", () => { - operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; - parserServiceMock.parse.mockReturnValue([createParameter("count", "integer")]); + operator.operatorProperties.uiParameters = [parameter("count", "integer", "42")]; + parserServiceMock.parse.mockReturnValue([parameter("count", "integer")]); - const parametersChangedObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); + const parametersChangedObserver = observeParameterChanges(); service.syncStructureFromCode(operatorId, code); @@ -121,14 +91,13 @@ describe("UiUdfParametersSyncService", () => { }); it("should emit parser errors without replacing the current parameters", () => { - operator.operatorProperties.uiParameters = [createParameter("count", "integer", "42")]; + operator.operatorProperties.uiParameters = [parameter("count", "integer", "42")]; parserServiceMock.parse.mockImplementation(() => { throw new UiUdfParametersParseError("Only one Python UDF class can declare UiParameter values."); }); - const parametersChangedObserver = createMockFunction(); - const parseErrorObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); + const parametersChangedObserver = observeParameterChanges(); + const parseErrorObserver = vitest.fn(); service.uiParametersParseError$.subscribe(parseErrorObserver); service.syncStructureFromCode(operatorId, code); @@ -143,8 +112,7 @@ describe("UiUdfParametersSyncService", () => { it("should not parse code for non-Python UDF operators", () => { operator.operatorType = "Projection"; - const parametersChangedObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); + const parametersChangedObserver = observeParameterChanges(); service.syncStructureFromCode(operatorId, code); @@ -154,86 +122,81 @@ describe("UiUdfParametersSyncService", () => { it("should read code from the shared operator property when editor code is omitted", () => { const sharedCode = 'self.UiParameter("count", AttributeType.INT)'; - graphMock.getSharedOperatorType.mockReturnValue(createSharedOperatorType(sharedCode)); - parserServiceMock.parse.mockReturnValue([createParameter("count", "integer")]); + graphMock.getSharedOperatorType.mockReturnValue(sharedOperatorType(sharedCode)); + parserServiceMock.parse.mockReturnValue([parameter("count", "integer")]); - const parametersChangedObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); + const parametersChangedObserver = observeParameterChanges(); service.syncStructureFromCode(operatorId); expect(parserServiceMock.parse).toHaveBeenCalledWith(sharedCode); expect(parametersChangedObserver).toHaveBeenCalledWith({ operatorId, - parameters: [createParameter("count", "integer")], + parameters: [parameter("count", "integer")], }); }); it("should debounce YText changes and clean up the observer", () => { vitest.useFakeTimers(); try { - const sharedCodeText = createSharedText('self.UiParameter("count", AttributeType.INT)'); - parserServiceMock.parse.mockReturnValue([createParameter("count", "integer")]); - - const parametersChangedObserver = createMockFunction(); - service.uiParametersChanged$.subscribe(parametersChangedObserver); + const sharedCodeText = sharedText('self.UiParameter("count", AttributeType.INT)'); + parserServiceMock.parse.mockReturnValue([parameter("count", "integer")]); + const parametersChangedObserver = observeParameterChanges(); const cleanup = service.attachToYCode(operatorId, sharedCodeText); expect(parserServiceMock.parse).toHaveBeenCalledOnce(); expect(parametersChangedObserver).toHaveBeenCalledOnce(); sharedCodeText.insert(sharedCodeText.length, "\n# changed"); - vitest.advanceTimersByTime(199); - expect(parserServiceMock.parse).toHaveBeenCalledOnce(); vitest.advanceTimersByTime(1); - expect(parserServiceMock.parse).toHaveBeenCalledTimes(2); expect(parametersChangedObserver).toHaveBeenCalledTimes(2); cleanup(); sharedCodeText.insert(sharedCodeText.length, "\n# after cleanup"); vitest.advanceTimersByTime(200); - expect(parserServiceMock.parse).toHaveBeenCalledTimes(2); expect(parametersChangedObserver).toHaveBeenCalledTimes(2); } finally { vitest.useRealTimers(); } }); + + function observeParameterChanges(): Mock { + const parametersChangedObserver = vitest.fn(); + service.uiParametersChanged$.subscribe(parametersChangedObserver); + return parametersChangedObserver; + } }); -function createParameter(name: string, type: UiUdfParameter["attribute"]["attributeType"], value = ""): UiUdfParameter { - return { - attribute: { - attributeName: name, - attributeType: type, - }, - value, - }; +function parameter( + attributeName: string, + attributeType: UiUdfParameter["attribute"]["attributeType"], + value = "" +): UiUdfParameter { + return { attribute: { attributeName, attributeType }, value }; } -function createSharedOperatorType(code: string): Yjs.Map { +function sharedOperatorType(code: string): Yjs.Map { const yjsDocument = new Yjs.Doc(); const sharedOperator = yjsDocument.getMap("operator"); const operatorProperties = new Yjs.Map(); - const sharedCodeText = new Yjs.Text(); - operatorProperties.set("code", sharedCodeText); + operatorProperties.set("code", sharedText(code)); sharedOperator.set("operatorProperties", operatorProperties); - sharedCodeText.insert(0, code); - return sharedOperator; } -function createSharedText(text: string): Yjs.Text { +function sharedText(text: string): Yjs.Text { const yjsDocument = new Yjs.Doc(); const sharedRootMap = yjsDocument.getMap("root"); - const sharedText = new Yjs.Text(); - sharedRootMap.set("code", sharedText); - sharedText.insert(0, text); - return sharedText; + const codeText = new Yjs.Text(); + + sharedRootMap.set("code", codeText); + codeText.insert(0, text); + return codeText; } diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts index ebf680c20bb..8ef42462c17 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -25,7 +25,6 @@ import { UiUdfParametersParseError, UiUdfParametersParserService } from "./ui-ud import type { UiUdfParameter } from "./ui-udf-parameters-parser.service"; import { isDefined } from "../../../common/util/predicate"; import { isPythonUdf } from "../workflow-graph/model/workflow-graph"; -import { YType } from "../../types/shared-editing.interface"; import type { Text as YText } from "yjs"; const UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS = 200; @@ -45,24 +44,16 @@ export class UiUdfParametersSyncService { private uiUdfParametersParserService: UiUdfParametersParserService ) {} - /** - * Attach directly to YText and sync whenever it changes - */ attachToYCode(operatorId: string, yCode: YText): () => void { const codeChanges = new Subject(); - const subscription = codeChanges.pipe(debounceTime(UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS)).subscribe(latestCode => { - this.syncStructureFromCode(operatorId, latestCode); - }); - - const handler = () => { - codeChanges.next(yCode.toString()); - }; + const subscription = codeChanges + .pipe(debounceTime(UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS)) + .subscribe(latestCode => this.syncStructureFromCode(operatorId, latestCode)); + const handler = () => codeChanges.next(yCode.toString()); yCode.observe(handler); - this.syncStructureFromCode(operatorId, yCode.toString()); - // return cleanup function return () => { yCode.unobserve(handler); subscription.unsubscribe(); @@ -73,20 +64,16 @@ export class UiUdfParametersSyncService { syncStructureFromCode(operatorId: string, codeFromEditor?: string): void { const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId); - if (!operator || !isPythonUdf(operator)) { - return; - } + if (!operator || !isPythonUdf(operator)) return; const code = codeFromEditor ?? this.getSharedCode(operatorId); - if (!isDefined(code)) { - return; - } + if (!isDefined(code)) return; const existingParameters = (operator.operatorProperties?.uiParameters ?? []) as UiUdfParameter[]; - let mergedUiParameters: UiUdfParameter[]; + let mergedParameters: UiUdfParameter[]; try { - mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); + mergedParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters); } catch (error) { if (error instanceof UiUdfParametersParseError) { this.uiParametersParseErrorSubject.next({ operatorId, message: error.message }); @@ -97,28 +84,16 @@ export class UiUdfParametersSyncService { this.uiParametersParseErrorSubject.next({ operatorId }); - if (isEqual(existingParameters, mergedUiParameters)) { - return; - } + if (isEqual(existingParameters, mergedParameters)) return; - // Emit event so UI updates - this.uiParametersChangedSubject.next({ - operatorId, - parameters: mergedUiParameters, - }); + this.uiParametersChangedSubject.next({ operatorId, parameters: mergedParameters }); } private buildParsedShapeWithPreservedValues(code: string, existingParameters: UiUdfParameter[]): UiUdfParameter[] { const parsedParameters = this.uiUdfParametersParserService.parse(code); - - const existingValues = new Map(); - existingParameters.forEach(parameter => { - const parameterName = parameter.attribute.attributeName; - - if (isDefined(parameterName) && isDefined(parameter.value)) { - existingValues.set(parameterName, parameter.value); - } - }); + const existingValues = new Map( + existingParameters.map(parameter => [parameter.attribute.attributeName, parameter.value] as const) + ); return parsedParameters.map(parameter => ({ ...parameter, @@ -130,10 +105,7 @@ export class UiUdfParametersSyncService { try { const sharedOperatorType = this.workflowActionService.getTexeraGraph().getSharedOperatorType(operatorId); - const operatorProperties = sharedOperatorType.get("operatorProperties") as YType< - Readonly<{ [key: string]: any }> - >; - + const operatorProperties = sharedOperatorType.get("operatorProperties") as any; const yCode = operatorProperties.get("code") as YText; return yCode?.toString(); } catch { From 7756cbdcadfce8ca44bdb6f7ecede70070098c4e Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 19 May 2026 23:17:45 -0600 Subject: [PATCH 09/13] fix(frontend): satisfy UI parameter lint --- .../component/ui-udf-parameters/ui-udf-parameters.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index bbb8657500f..1481a00705b 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -72,7 +72,8 @@ export class UiUdfParametersComponent extends FieldArrayType implements OnInit { } private applyDisabledState(field: FormlyFieldConfig, disabled: boolean): void { - disabled ? field.formControl?.disable({ emitEvent: false }) : field.formControl?.enable({ emitEvent: false }); + if (disabled) field.formControl?.disable({ emitEvent: false }); + else field.formControl?.enable({ emitEvent: false }); } trackByParameterName = (index: number, parameter: any): string | number => { From e954e2e35aa6b702feb40f944abf401702ecf3b8 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 19 May 2026 23:39:37 -0600 Subject: [PATCH 10/13] refactor(frontend): read UI parameter member paths from AST --- .../ui-udf-parameters-parser.service.spec.ts | 9 +++++++ .../ui-udf-parameters-parser.service.ts | 27 ++++++++++++------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index e865df82c9a..e9fc4e34948 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -96,6 +96,15 @@ describe("UiUdfParametersParserService", () => { [parameter("valid", "string")], ], ["ignore legacy key= named argument", 'self.UiParameter(type=AttributeType.DOUBLE, key="a")', []], + [ + "ignore non-self calls and non-AttributeType members", + ` + some.UiParameter("not_self", AttributeType.INT) + self.UiParameter("bad_type", OtherType.INT) + self.UiParameter("valid", AttributeType.STRING) + `, + [parameter("valid", "string")], + ], [ "ignore empty and extra positional arguments", ` diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 37210c6c211..238794806b7 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -100,8 +100,8 @@ export class UiUdfParametersParserService { function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefined { const argumentList = call.getChild("ArgList"); - if (!argumentList || code.slice(call.from, argumentList.from).replace(/\s+/g, "") !== "self.UiParameter") - return undefined; + const callee = call.getChild("MemberExpression"); + if (!argumentList || !isMemberPath(callee, code, ["self", "UiParameter"])) return undefined; let attributeName: string | undefined; let attributeType: AttributeType | undefined; @@ -166,16 +166,25 @@ function readName(value: ParserSyntaxNode, code: string): string | undefined { } function readType(value: ParserSyntaxNode, code: string): AttributeType | undefined { - if (value.name !== "MemberExpression") return undefined; - const token = code - .slice(value.from, value.to) - .trim() - .replace(/\s+/g, "") - .match(/^AttributeType\.([A-Za-z_]\w*)$/)?.[1] - .toUpperCase(); + const parts = readMemberPath(value, code); + if (parts?.length !== 2 || parts[0] !== "AttributeType") return undefined; + const token = parts[1].toUpperCase(); return token ? ATTRIBUTE_TYPES_BY_TOKEN[token] : undefined; } +function isMemberPath(node: ParserSyntaxNode | null, code: string, expectedParts: string[]): boolean { + const parts = node ? readMemberPath(node, code) : undefined; + return parts?.length === expectedParts.length && parts.every((part, index) => part === expectedParts[index]); +} + +function readMemberPath(node: ParserSyntaxNode, code: string): string[] | undefined { + if (node.name !== "MemberExpression") return undefined; + const parts = getChildren(node) + .filter(child => child.name === "VariableName" || child.name === "PropertyName") + .map(child => code.slice(child.from, child.to)); + return parts.length ? parts : undefined; +} + function readString(input: string): string | undefined { return input .trim() From 5afd325c2886d254ab75101012345c5a9fa0a303 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Tue, 19 May 2026 23:59:06 -0600 Subject: [PATCH 11/13] refactor(frontend): centralize Python parser node names --- .../ui-udf-parameters-parser.service.ts | 57 +++++++++++++------ 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 238794806b7..31266518b91 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -29,6 +29,24 @@ const SUPPORTED_CLASS_NAMES = new Set([ "GenerateOperator", ]); +const PYTHON_NODE = { + ARG_LIST: "ArgList", + ASSIGN_OP: "AssignOp", + CALL_EXPRESSION: "CallExpression", + CLASS_DEFINITION: "ClassDefinition", + MEMBER_EXPRESSION: "MemberExpression", + PROPERTY_NAME: "PropertyName", + STRING: "String", + VARIABLE_NAME: "VariableName", +} as const; +const ARGUMENT_DELIMITER_NODES = new Set(["(", ")", ","]); + +const UI_PARAMETER_CALLEE = ["self", "UiParameter"]; +const ATTRIBUTE_TYPE_RECEIVER = "AttributeType"; +const ARGUMENT_NAME = "name"; +const ARGUMENT_TYPE = "type"; +const ARGUMENT_ATTR_TYPE = "attr_type"; + type ParserSyntaxNode = ReturnType["topNode"]; type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; @@ -71,16 +89,16 @@ export class UiUdfParametersParserService { parser.parse(code).iterate({ enter: ({ name, node }) => { - const className = node.getChild("VariableName"); + const className = node.getChild(PYTHON_NODE.VARIABLE_NAME); if ( - name !== "ClassDefinition" || + name !== PYTHON_NODE.CLASS_DEFINITION || !className || !SUPPORTED_CLASS_NAMES.has(code.slice(className.from, className.to)) ) return; supportedClassCount++; node.cursor().iterate(cursorReference => { - if (cursorReference.name !== "CallExpression") return; + if (cursorReference.name !== PYTHON_NODE.CALL_EXPRESSION) return; addParameter(readCall(cursorReference.node, code)); return false; }); @@ -99,9 +117,9 @@ export class UiUdfParametersParserService { } function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefined { - const argumentList = call.getChild("ArgList"); - const callee = call.getChild("MemberExpression"); - if (!argumentList || !isMemberPath(callee, code, ["self", "UiParameter"])) return undefined; + const argumentList = call.getChild(PYTHON_NODE.ARG_LIST); + const callee = call.getChild(PYTHON_NODE.MEMBER_EXPRESSION); + if (!argumentList || !isMemberPath(callee, code, UI_PARAMETER_CALLEE)) return undefined; let attributeName: string | undefined; let attributeType: AttributeType | undefined; @@ -109,7 +127,8 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi let sawNamed = false; for (const argument of readArguments(argumentList, code)) { - const key = argument.key ?? (positionalIndex === 0 ? "name" : positionalIndex === 1 ? "type" : undefined); + const key = + argument.key ?? (positionalIndex === 0 ? ARGUMENT_NAME : positionalIndex === 1 ? ARGUMENT_TYPE : undefined); if (argument.key) { sawNamed = true; @@ -117,11 +136,15 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi return undefined; } - if (key === "name" && !attributeName) attributeName = readName(argument.value, code); - else if ((key === "type" || key === "attr_type") && !attributeType) attributeType = readType(argument.value, code); + if (key === ARGUMENT_NAME && !attributeName) attributeName = readName(argument.value, code); + else if ((key === ARGUMENT_TYPE || key === ARGUMENT_ATTR_TYPE) && !attributeType) + attributeType = readType(argument.value, code); else return undefined; - if ((key === "name" && !attributeName) || ((key === "type" || key === "attr_type") && !attributeType)) + if ( + (key === ARGUMENT_NAME && !attributeName) || + ((key === ARGUMENT_TYPE || key === ARGUMENT_ATTR_TYPE) && !attributeType) + ) return undefined; if (!argument.key) { @@ -134,17 +157,17 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi function readArguments(argumentList: ParserSyntaxNode, code: string): ParsedArgument[] { const result: ParsedArgument[] = []; - const children = getChildren(argumentList).filter(node => !["(", ")", ","].includes(node.name)); + const children = getChildren(argumentList).filter(node => !ARGUMENT_DELIMITER_NODES.has(node.name)); for (let index = 0; index < children.length; index++) { const node = children[index]; - if (node.name === "VariableName" && children[index + 1]?.name === "AssignOp") { + if (node.name === PYTHON_NODE.VARIABLE_NAME && children[index + 1]?.name === PYTHON_NODE.ASSIGN_OP) { const value = children[index + 2]; if (!value) return []; result.push({ key: code.slice(node.from, node.to), value }); index += 2; - } else if (node.name !== "AssignOp") { + } else if (node.name !== PYTHON_NODE.ASSIGN_OP) { result.push({ value: node }); } else { return []; @@ -161,13 +184,13 @@ function getChildren(node: ParserSyntaxNode): ParserSyntaxNode[] { } function readName(value: ParserSyntaxNode, code: string): string | undefined { - const name = value.name === "String" ? readString(code.slice(value.from, value.to))?.trim() : undefined; + const name = value.name === PYTHON_NODE.STRING ? readString(code.slice(value.from, value.to))?.trim() : undefined; return name || undefined; } function readType(value: ParserSyntaxNode, code: string): AttributeType | undefined { const parts = readMemberPath(value, code); - if (parts?.length !== 2 || parts[0] !== "AttributeType") return undefined; + if (parts?.length !== 2 || parts[0] !== ATTRIBUTE_TYPE_RECEIVER) return undefined; const token = parts[1].toUpperCase(); return token ? ATTRIBUTE_TYPES_BY_TOKEN[token] : undefined; } @@ -178,9 +201,9 @@ function isMemberPath(node: ParserSyntaxNode | null, code: string, expectedParts } function readMemberPath(node: ParserSyntaxNode, code: string): string[] | undefined { - if (node.name !== "MemberExpression") return undefined; + if (node.name !== PYTHON_NODE.MEMBER_EXPRESSION) return undefined; const parts = getChildren(node) - .filter(child => child.name === "VariableName" || child.name === "PropertyName") + .filter(child => child.name === PYTHON_NODE.VARIABLE_NAME || child.name === PYTHON_NODE.PROPERTY_NAME) .map(child => code.slice(child.from, child.to)); return parts.length ? parts : undefined; } From 222250994c53ba21237b18b1242f91254332e242 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Wed, 20 May 2026 00:04:27 -0600 Subject: [PATCH 12/13] refactor(frontend): normalize UI parameter arguments --- .../ui-udf-parameters-parser.service.ts | 64 ++++++++++++------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 31266518b91..489d1338c02 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -46,9 +46,13 @@ const ATTRIBUTE_TYPE_RECEIVER = "AttributeType"; const ARGUMENT_NAME = "name"; const ARGUMENT_TYPE = "type"; const ARGUMENT_ATTR_TYPE = "attr_type"; +const POSITIONAL_ARGUMENT_KEYS = [ARGUMENT_NAME, ARGUMENT_TYPE] as const; type ParserSyntaxNode = ReturnType["topNode"]; type ParsedArgument = Readonly<{ key?: string; value: ParserSyntaxNode }>; +type UiParameterArgument = + | Readonly<{ kind: typeof ARGUMENT_NAME; value: string }> + | Readonly<{ kind: typeof ARGUMENT_TYPE; value: AttributeType }>; export type UiUdfParameter = Readonly<{ attribute: SchemaAttribute; value: string }>; @@ -123,36 +127,50 @@ function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | undefi let attributeName: string | undefined; let attributeType: AttributeType | undefined; - let positionalIndex = 0; - let sawNamed = false; + const uiParameterArguments = readUiParameterArguments(argumentList, code); + if (!uiParameterArguments) return undefined; - for (const argument of readArguments(argumentList, code)) { - const key = - argument.key ?? (positionalIndex === 0 ? ARGUMENT_NAME : positionalIndex === 1 ? ARGUMENT_TYPE : undefined); + for (const argument of uiParameterArguments) { + if (argument.kind === ARGUMENT_NAME && !attributeName) attributeName = argument.value; + else if (argument.kind === ARGUMENT_TYPE && !attributeType) attributeType = argument.value; + else return undefined; + } - if (argument.key) { - sawNamed = true; - } else if (sawNamed) { - return undefined; - } + return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; +} - if (key === ARGUMENT_NAME && !attributeName) attributeName = readName(argument.value, code); - else if ((key === ARGUMENT_TYPE || key === ARGUMENT_ATTR_TYPE) && !attributeType) - attributeType = readType(argument.value, code); - else return undefined; +function readUiParameterArguments(argumentList: ParserSyntaxNode, code: string): UiParameterArgument[] | undefined { + const result: UiParameterArgument[] = []; + let positionalIndex = 0; + let sawNamedArgument = false; - if ( - (key === ARGUMENT_NAME && !attributeName) || - ((key === ARGUMENT_TYPE || key === ARGUMENT_ATTR_TYPE) && !attributeType) - ) - return undefined; + for (const argument of readArguments(argumentList, code)) { + if (argument.key) sawNamedArgument = true; + else if (sawNamedArgument) return undefined; - if (!argument.key) { - positionalIndex++; - } + const key = argument.key ?? POSITIONAL_ARGUMENT_KEYS[positionalIndex++]; + const parsedArgument = readUiParameterArgument(key, argument.value, code); + if (!parsedArgument) return undefined; + result.push(parsedArgument); } - return attributeName && attributeType ? { attribute: { attributeName, attributeType }, value: "" } : undefined; + return result; +} + +function readUiParameterArgument( + key: string | undefined, + value: ParserSyntaxNode, + code: string +): UiParameterArgument | undefined { + if (key === ARGUMENT_NAME) { + const attributeName = readName(value, code); + return attributeName ? { kind: ARGUMENT_NAME, value: attributeName } : undefined; + } + if (key === ARGUMENT_TYPE || key === ARGUMENT_ATTR_TYPE) { + const attributeType = readType(value, code); + return attributeType ? { kind: ARGUMENT_TYPE, value: attributeType } : undefined; + } + return undefined; } function readArguments(argumentList: ParserSyntaxNode, code: string): ParsedArgument[] { From bc27c112b08a9e20dcb66910d0f069e492b849e7 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Wed, 20 May 2026 18:47:40 -0600 Subject: [PATCH 13/13] fix(frontend): address ui parameter follow-up review --- .../ui-udf-parameters.component.spec.ts | 85 +++++++++++++++---- .../ui-udf-parameters.component.ts | 58 ++++++++++--- .../ui-udf-parameters-parser.service.ts | 7 ++ .../ui-udf-parameters-sync.service.spec.ts | 20 +++++ .../ui-udf-parameters-sync.service.ts | 31 ++++++- 5 files changed, 169 insertions(+), 32 deletions(-) diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts index 26325f8ad5d..74b876d3218 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.spec.ts @@ -33,28 +33,33 @@ describe("UiUdfParametersComponent", () => { const nameControl = new FormControl("threshold"); const typeControl = new FormControl("double"); - const valueField: FormlyFieldConfig = { key: "value", formControl: valueControl }; - const nameField: FormlyFieldConfig = { key: "attributeName", formControl: nameControl }; - const typeField: FormlyFieldConfig = { key: "attributeType", formControl: typeControl }; - const rowField: FormlyFieldConfig = { - fieldGroup: [ - valueField, - { - key: "attribute", - fieldGroup: [nameField, typeField], - }, - ], - }; + const rowField = rowConfig([ + { key: "value", formControl: valueControl }, + { key: "attributeName", formControl: nameControl }, + { key: "attributeType", formControl: typeControl }, + ]); - (component as any).field = { fieldGroup: [rowField] } as FormlyFieldConfig; + (component as any).field = { model: [{}], fieldGroup: [rowField] } as FormlyFieldConfig; - component.ngOnInit(); + component.onPopulate((component as any).field); // templateOptions is deprecated, but some existing Formly wrappers still read it. [ - { column: component.fieldColumns[0], field: valueField, control: valueControl }, - { column: component.fieldColumns[1], field: nameField, control: nameControl }, - { column: component.fieldColumns[2], field: typeField, control: typeControl }, + { + column: component.fieldColumns[0], + field: component.getColumnField(rowField, component.fieldColumns[0]), + control: valueControl, + }, + { + column: component.fieldColumns[1], + field: component.getColumnField(rowField, component.fieldColumns[1]), + control: nameControl, + }, + { + column: component.fieldColumns[2], + field: component.getColumnField(rowField, component.fieldColumns[2]), + control: typeControl, + }, ].forEach(({ column, field, control }) => { expect(component.getColumnField(rowField, column)).toBe(field); const disabled = column.disabled; @@ -63,4 +68,50 @@ describe("UiUdfParametersComponent", () => { expect((control as FormControl).disabled).toBe(disabled); }); }); + + it("should apply disabled state to rows generated from the field array template", () => { + const field: FormlyFieldConfig = { + model: [{ value: "42", attribute: { attributeName: "threshold", attributeType: "double" } }], + fieldArray: rowConfig([{ key: "value" }, { key: "attributeName" }, { key: "attributeType" }]), + fieldGroup: [], + }; + + component.onPopulate(field); + + const generatedRow = field.fieldGroup?.[0] as FormlyFieldConfig; + const valueControl = new FormControl({ value: "42", disabled: true }); + const nameControl = new FormControl("threshold"); + const typeControl = new FormControl("double"); + + [ + { column: component.fieldColumns[0], control: valueControl }, + { column: component.fieldColumns[1], control: nameControl }, + { column: component.fieldColumns[2], control: typeControl }, + ].forEach(({ column, control }) => { + const columnField = component.getColumnField(generatedRow, column) as FormlyFieldConfig; + Object.assign(columnField, { formControl: control }); + columnField.hooks?.onInit?.(columnField); + + expect(columnField.props?.disabled).toBe(column.disabled); + expect((columnField as any).templateOptions?.disabled).toBe(column.disabled); + expect(control.disabled).toBe(column.disabled); + }); + }); }); + +function rowConfig(fields: ReadonlyArray<{ key: string; formControl?: FormControl }>): FormlyFieldConfig { + const [valueField, nameField, typeField] = fields.map(field => ({ + key: field.key, + formControl: field.formControl, + })); + + return { + fieldGroup: [ + valueField, + { + key: "attribute", + fieldGroup: [nameField, typeField], + }, + ], + }; +} diff --git a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts index 1481a00705b..d725004c582 100644 --- a/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts +++ b/frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts @@ -16,48 +16,74 @@ * specific language governing permissions and limitations * under the License. */ -import { Component, OnInit } from "@angular/core"; +import { Component } from "@angular/core"; import { NgFor, NgIf } from "@angular/common"; import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; type UiUdfParameterColumn = Readonly<{ label: string; key: string; parentKey?: string; disabled: boolean }>; +/** Renders inferred Python UDF UI parameters with editable values and locked name/type columns. */ @Component({ selector: "texera-ui-udf-parameters", templateUrl: "./ui-udf-parameters.component.html", styleUrls: ["./ui-udf-parameters.component.scss"], imports: [NgIf, NgFor, FormlyModule], }) -export class UiUdfParametersComponent extends FieldArrayType implements OnInit { +export class UiUdfParametersComponent extends FieldArrayType { + private readonly disabledStateConfigured = new WeakMap(); + readonly fieldColumns: UiUdfParameterColumn[] = [ { label: "Value", key: "value", disabled: false }, { label: "Name", key: "attributeName", parentKey: "attribute", disabled: true }, { label: "Type", key: "attributeType", parentKey: "attribute", disabled: true }, ]; - ngOnInit(): void { - this.field.fieldGroup?.forEach(rowField => { - this.fieldColumns.forEach(column => { - this.configureDisabledState(this.getColumnField(rowField, column), column.disabled); - }); - }); + override onPopulate(field: FormlyFieldConfig): void { + this.configureRowTemplate(this.getFieldArrayTemplate(field)); + super.onPopulate(field); + field.fieldGroup?.forEach(rowField => this.configureRowFields(rowField)); } + /** Finds the Formly field config that backs one visible column in a parameter row. */ getColumnField(rowField: FormlyFieldConfig, column: UiUdfParameterColumn): FormlyFieldConfig | undefined { return this.getChildField(column.parentKey ? this.getChildField(rowField, column.parentKey) : rowField, column.key); } + private getFieldArrayTemplate(field: FormlyFieldConfig): FormlyFieldConfig | undefined { + return typeof field.fieldArray === "function" ? undefined : field.fieldArray; + } + + private configureRowTemplate(rowField: FormlyFieldConfig | undefined): void { + this.configureRowColumns(rowField, this.setDisabledMetadata.bind(this)); + } + + private configureRowFields(rowField: FormlyFieldConfig | undefined): void { + this.configureRowColumns(rowField, this.configureDisabledState.bind(this)); + } + + private configureRowColumns( + rowField: FormlyFieldConfig | undefined, + configureColumn: (field: FormlyFieldConfig | undefined, disabled: boolean) => void + ): void { + if (!rowField) return; + + this.fieldColumns.forEach(column => configureColumn(this.getColumnField(rowField, column), column.disabled)); + } + private getChildField(rowField: FormlyFieldConfig | undefined, key: string): FormlyFieldConfig | undefined { return rowField?.fieldGroup?.find(fieldConfig => fieldConfig.key === key); } + /** Sets Formly disabled metadata and keeps controls created later in sync through an onInit hook. */ private configureDisabledState(field: FormlyFieldConfig | undefined, disabled: boolean): void { if (!field) return; - field.props = { ...(field.props ?? {}), disabled }; + this.setDisabledMetadata(field, disabled); - // Keep deprecated templateOptions in sync for existing Formly wrappers that still read it. - (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; + if (this.disabledStateConfigured.get(field) === disabled) { + this.applyDisabledState(field, disabled); + return; + } const previousOnInit = field.hooks?.onInit; field.hooks = { @@ -68,9 +94,19 @@ export class UiUdfParametersComponent extends FieldArrayType implements OnInit { }, }; + this.disabledStateConfigured.set(field, disabled); this.applyDisabledState(field, disabled); } + private setDisabledMetadata(field: FormlyFieldConfig | undefined, disabled: boolean): void { + if (!field) return; + + field.props = { ...(field.props ?? {}), disabled }; + + // Keep deprecated templateOptions in sync for existing Formly wrappers that still read it. + (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled }; + } + private applyDisabledState(field: FormlyFieldConfig, disabled: boolean): void { if (disabled) field.formControl?.disable({ emitEvent: false }); else field.formControl?.enable({ emitEvent: false }); diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts index 489d1338c02..5c0acb81d09 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts @@ -54,8 +54,10 @@ type UiParameterArgument = | Readonly<{ kind: typeof ARGUMENT_NAME; value: string }> | Readonly<{ kind: typeof ARGUMENT_TYPE; value: AttributeType }>; +/** UI parameter row inferred from Python code, with backend-compatible attribute metadata and an editable value. */ export type UiUdfParameter = Readonly<{ attribute: SchemaAttribute; value: string }>; +/** Raised when supported Python UDF code declares UI parameters that cannot be represented safely in the UI. */ export class UiUdfParametersParseError extends Error {} // Accept Java enum names (INTEGER, BOOLEAN) and Python enum aliases (INT, BOOL). @@ -70,8 +72,13 @@ const ATTRIBUTE_TYPES_BY_TOKEN: Readonly> = { TIMESTAMP: "timestamp", }; +/** Parses Python UDF source code and infers supported self.UiParameter(...) declarations for the property panel. */ @Injectable({ providedIn: "root" }) export class UiUdfParametersParserService { + /** + * Returns UI parameters from the single supported Python UDF class in the source. + * Throws UiUdfParametersParseError for duplicate parameter names or multiple supported UDF classes. + */ parse(code: string): UiUdfParameter[] { if (!code) return []; diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts index 415bb27d4d5..d755978ca6a 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.spec.ts @@ -136,6 +136,26 @@ describe("UiUdfParametersSyncService", () => { }); }); + it("should warn and skip sync when shared code cannot be read", () => { + const sharedCodeError = new Error("missing shared operator"); + const consoleWarnSpy = vitest.spyOn(console, "warn").mockImplementation(() => undefined); + graphMock.getSharedOperatorType.mockImplementation(() => { + throw sharedCodeError; + }); + + try { + service.syncStructureFromCode(operatorId); + + expect(parserServiceMock.parse).not.toHaveBeenCalled(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + "Unable to read Python UDF code from shared operator properties.", + sharedCodeError + ); + } finally { + consoleWarnSpy.mockRestore(); + } + }); + it("should debounce YText changes and clean up the observer", () => { vitest.useFakeTimers(); try { diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts index 8ef42462c17..519d54aaa97 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts @@ -26,9 +26,16 @@ import type { UiUdfParameter } from "./ui-udf-parameters-parser.service"; import { isDefined } from "../../../common/util/predicate"; import { isPythonUdf } from "../workflow-graph/model/workflow-graph"; import type { Text as YText } from "yjs"; +import type { YType } from "../../types/shared-editing.interface"; +type SharedOperatorProperties = Readonly<{ code?: string; [key: string]: unknown }>; + +/** + * Waits briefly after shared-code edits so typing does not parse the full UDF body on every keystroke. + */ const UI_PARAMETER_SYNC_DEBOUNCE_TIME_MS = 200; +/** Keeps Python UDF UI parameter structure in sync with the code editor and workflow graph. */ @Injectable({ providedIn: "root" }) export class UiUdfParametersSyncService { private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>( @@ -36,7 +43,10 @@ export class UiUdfParametersSyncService { ); private readonly uiParametersParseErrorSubject = new ReplaySubject<{ operatorId: string; message?: string }>(1); + /** Emits when parsed UI parameter structure changes; consumers should write the parameters back to operatorProperties. */ readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable(); + + /** Emits parser errors; an event without message clears the current parse error for that operator. */ readonly uiParametersParseError$ = this.uiParametersParseErrorSubject.asObservable(); constructor( @@ -44,6 +54,10 @@ export class UiUdfParametersSyncService { private uiUdfParametersParserService: UiUdfParametersParserService ) {} + /** + * Observes a shared YText code buffer and syncs the initial and debounced future contents. + * Each call attaches an independent observer; call the returned cleanup function to detach it. + */ attachToYCode(operatorId: string, yCode: YText): () => void { const codeChanges = new Subject(); const subscription = codeChanges @@ -61,6 +75,10 @@ export class UiUdfParametersSyncService { }; } + /** + * Parses Python UDF code for a known Python UDF operator and emits merged parameter rows when the shape changes. + * If codeFromEditor is omitted, reads from Yjs; does nothing if the operator or code is unavailable. + */ syncStructureFromCode(operatorId: string, codeFromEditor?: string): void { const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId); @@ -82,7 +100,7 @@ export class UiUdfParametersSyncService { throw error; } - this.uiParametersParseErrorSubject.next({ operatorId }); + this.clearParseError(operatorId); if (isEqual(existingParameters, mergedParameters)) return; @@ -105,11 +123,16 @@ export class UiUdfParametersSyncService { try { const sharedOperatorType = this.workflowActionService.getTexeraGraph().getSharedOperatorType(operatorId); - const operatorProperties = sharedOperatorType.get("operatorProperties") as any; - const yCode = operatorProperties.get("code") as YText; + const operatorProperties = sharedOperatorType.get("operatorProperties") as YType; + const yCode = operatorProperties.get("code") as unknown as YText; return yCode?.toString(); - } catch { + } catch (error) { + console.warn("Unable to read Python UDF code from shared operator properties.", error); return undefined; } } + + private clearParseError(operatorId: string): void { + this.uiParametersParseErrorSubject.next({ operatorId }); + } }