This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7551-eefd8600b6cf75da9e5d3067bc531ac3bc4faba9 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 4afe8d4ecdf6b4eff007ce55b859b09553163c43 Author: carloea2 <[email protected]> AuthorDate: Thu Aug 13 18:26:57 2026 +0000 feat(frontend): add UDF parameters from the panel (#7551) <img width="807" height="175" alt="image" src="https://github.com/user-attachments/assets/9145a9be-6ab4-458d-834b-057c0686549a"; /> <img width="589" height="108" alt="image" src="https://github.com/user-attachments/assets/06e6f5c6-9bd4-4b47-8070-d04d45b6836f"; /> ### What changes were proposed in this PR? This PR lets users add Python UDF parameters directly from the properties panel. The parameter list now includes a dashed **Add parameter** button. Clicking it opens an inline draft row where the user can enter a name, select a supported type, and confirm with the check button or Enter, or cancel with the close button or Escape. Confirming the draft inserts a `self.UiParameter(...)` declaration into the operator's shared Python code. The code remains the single source of truth: the parameter row appears through the existing parse-and-sync flow instead of being written directly into the form model. The insertion logic: - groups new declarations with existing `UiParameter` declarations; - inserts into an existing `open()` method when available; - creates `open()` when it is absent; - handles docstrings, decorators, `pass`, comments, and empty class bodies; - converts display names into safe Python assignment targets while preserving the exact parameter name; - rejects missing names, duplicate declarations, unsupported types, and unsupported UDF code shapes. <!-- Add before/after properties-panel screenshots here before marking the PR ready. --> ### Any related issues, documentation, discussions? Closes #7552 ### How was this PR tested? Manual verification by the author covered the add, confirm/cancel, declaration insertion, and properties-panel synchronization flows. Frontend CI formatting and lint: ```bash cd frontend yarn format:ci ``` The command passes. The updated specifications cover component actions and error states, shared-code insertion and synchronization, declaration placement, identifier sanitization, duplicate names, unsupported types, unsupported classes, and malformed class bodies. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code Fable 5 Ultra, OpenAI Codex 5.6 Ultra, and Me --- .../ui-udf-parameters.component.html | 63 ++++++- .../ui-udf-parameters.component.scss | 28 +++ .../ui-udf-parameters.component.spec.ts | 85 ++++++++- .../ui-udf-parameters.component.ts | 49 ++++- .../ui-udf-parameters-parser.service.spec.ts | 187 ++++++++++++++++++ .../ui-udf-parameters-parser.service.ts | 209 +++++++++++++++++---- .../ui-udf-parameters-sync.service.spec.ts | 45 ++++- .../code-editor/ui-udf-parameters-sync.service.ts | 32 +++- 8 files changed, 649 insertions(+), 49 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 26ea63b827..e8220aa973 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,9 +17,23 @@ under the License. --> <div class="ui-udf-parameters-field"> + <button + class="add-parameter-button" + *ngIf="workflowModificationEnabled && !draftVisible" + nz-button + nzType="dashed" + nzBlock + type="button" + (click)="draftVisible = true"> + <span + nz-icon + nzType="plus"></span> + Add parameter + </button> + <div class="ui-udf-parameter-list" - *ngIf="model?.length"> + *ngIf="model?.length || draftVisible"> <div class="ui-udf-parameter-row header"> <div class="field-cell" @@ -28,6 +42,53 @@ </div> </div> + <div + class="ui-udf-parameter-row draft" + *ngIf="draftVisible"> + <div class="field-cell"></div> + <div class="field-cell"> + <input + #parameterNameInput + type="text" + placeholder="Parameter name" + autofocus + (keyup.enter)="addParameter(parameterNameInput, parameterTypeSelect.value)" + (keyup.escape)="draftVisible = false" /> + </div> + <div class="field-cell draft-actions"> + <select #parameterTypeSelect> + <option + *ngFor="let parameterType of addParameterTypeOptions" + [value]="parameterType"> + {{ parameterType }} + </option> + </select> + <button + nz-button + nzType="primary" + nzShape="circle" + nzSize="small" + type="button" + title="Add parameter" + (click)="addParameter(parameterNameInput, parameterTypeSelect.value)"> + <span + nz-icon + nzType="check"></span> + </button> + <button + nz-button + nzShape="circle" + nzSize="small" + type="button" + title="Cancel" + (click)="draftVisible = false"> + <span + nz-icon + nzType="close"></span> + </button> + </div> + </div> + <div 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 ab6b2ad6ee..a4ccd6039f 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 @@ -35,3 +35,31 @@ :host ::ng-deep .ant-form-item-label { display: none; } + +.ui-udf-parameter-row.draft { + align-items: center; + + input, + select { + width: 100%; + height: 32px; + padding: 0 11px; + border: 1px solid #d9d9d9; + border-radius: 2px; + background: transparent; + } + + .draft-actions { + display: flex; + gap: 8px; + align-items: center; + + select { + flex: 1; + } + } +} + +.add-parameter-button { + margin-bottom: 8px; +} 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 74b876d321..6813293bc3 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 @@ -18,14 +18,71 @@ */ import { FormControl } from "@angular/forms"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; import { FormlyFieldConfig } from "@ngx-formly/core"; +import type { Mock } from "vitest"; +import { vi as vitest } from "vitest"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { UiUdfParametersEditError } from "../../service/code-editor/ui-udf-parameters-parser.service"; +import { UiUdfParametersSyncService } from "../../service/code-editor/ui-udf-parameters-sync.service"; +import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; import { UiUdfParametersComponent } from "./ui-udf-parameters.component"; describe("UiUdfParametersComponent", () => { + const operatorId = "operator-1"; + + let fixture: ComponentFixture<UiUdfParametersComponent>; let component: UiUdfParametersComponent; + let workflowActionServiceMock: { + checkWorkflowModificationEnabled: Mock; + getJointGraphWrapper: Mock; + }; + let syncServiceMock: { addParameter: Mock }; + let notificationServiceMock: { error: Mock }; + + beforeEach(async () => { + workflowActionServiceMock = { + checkWorkflowModificationEnabled: vitest.fn().mockReturnValue(true), + getJointGraphWrapper: vitest.fn().mockReturnValue({ + getCurrentHighlightedOperatorIDs: () => [operatorId], + }), + }; + syncServiceMock = { addParameter: vitest.fn() }; + notificationServiceMock = { error: vitest.fn() }; + + await TestBed.configureTestingModule({ + imports: [UiUdfParametersComponent], + providers: [ + { provide: WorkflowActionService, useValue: workflowActionServiceMock }, + { provide: UiUdfParametersSyncService, useValue: syncServiceMock }, + { provide: NotificationService, useValue: notificationServiceMock }, + ], + }).compileComponents(); - beforeEach(() => { - component = new UiUdfParametersComponent(); + fixture = TestBed.createComponent(UiUdfParametersComponent); + component = fixture.componentInstance; + }); + + it("should render the add control and draft row before existing parameters", () => { + (component as any).field = { + model: [{ value: "42", attribute: { attributeName: "threshold", attributeType: "double" } }], + fieldGroup: [{}], + } as FormlyFieldConfig; + + fixture.detectChanges(); + + const addButton = fixture.nativeElement.querySelector(".add-parameter-button") as HTMLElement; + const parameterList = fixture.nativeElement.querySelector(".ui-udf-parameter-list") as HTMLElement; + expect(addButton.compareDocumentPosition(parameterList) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + + component.draftVisible = true; + fixture.detectChanges(); + + const draftRow = fixture.nativeElement.querySelector(".ui-udf-parameter-row.draft") as HTMLElement; + const existingRow = fixture.nativeElement.querySelector( + ".ui-udf-parameter-row:not(.header):not(.draft)" + ) as HTMLElement; + expect(draftRow.compareDocumentPosition(existingRow) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); }); it("should disable name and type fields while leaving value editable", () => { @@ -97,6 +154,30 @@ describe("UiUdfParametersComponent", () => { expect(control.disabled).toBe(column.disabled); }); }); + + it("should add a parameter for the highlighted operator and close the draft row", () => { + component.draftVisible = true; + + component.addParameter({ value: "threshold" } as HTMLInputElement, "double"); + + expect(syncServiceMock.addParameter).toHaveBeenCalledWith(operatorId, "threshold", "double"); + expect(component.draftVisible).toBe(false); + expect(notificationServiceMock.error).not.toHaveBeenCalled(); + }); + + it("should surface edit errors and keep the draft row open", () => { + component.draftVisible = true; + syncServiceMock.addParameter.mockImplementation(() => { + throw new UiUdfParametersEditError("UiParameter name 'threshold' is declared already."); + }); + + component.addParameter({ value: "threshold" } as HTMLInputElement, "double"); + + expect(notificationServiceMock.error).toHaveBeenCalledWith( + "Could not add UDF parameter: UiParameter name 'threshold' is declared already." + ); + expect(component.draftVisible).toBe(true); + }); }); function rowConfig(fields: ReadonlyArray<{ key: string; formControl?: FormControl }>): FormlyFieldConfig { 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 d725004c58..d1aebb0cb9 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 @@ -19,6 +19,18 @@ import { Component } from "@angular/core"; import { NgFor, NgIf } from "@angular/common"; import { FieldArrayType, FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; +import { NzButtonComponent } from "ng-zorro-antd/button"; +import { NzWaveDirective } from "ng-zorro-antd/core/wave"; +import { ɵNzTransitionPatchDirective } from "ng-zorro-antd/core/transition-patch"; +import { NzIconDirective } from "ng-zorro-antd/icon"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; +import { + UiUdfParametersEditError, + UiUdfParametersParseError, +} from "../../service/code-editor/ui-udf-parameters-parser.service"; +import { UiUdfParametersSyncService } from "../../service/code-editor/ui-udf-parameters-sync.service"; +import type { AttributeType } from "../../types/workflow-compiling.interface"; type UiUdfParameterColumn = Readonly<{ label: string; key: string; parentKey?: string; disabled: boolean }>; @@ -27,7 +39,15 @@ type UiUdfParameterColumn = Readonly<{ label: string; key: string; parentKey?: s selector: "texera-ui-udf-parameters", templateUrl: "./ui-udf-parameters.component.html", styleUrls: ["./ui-udf-parameters.component.scss"], - imports: [NgIf, NgFor, FormlyModule], + imports: [ + NgIf, + NgFor, + FormlyModule, + NzButtonComponent, + NzWaveDirective, + ɵNzTransitionPatchDirective, + NzIconDirective, + ], }) export class UiUdfParametersComponent extends FieldArrayType<FormlyFieldConfig> { private readonly disabledStateConfigured = new WeakMap<FormlyFieldConfig, boolean>(); @@ -38,6 +58,33 @@ export class UiUdfParametersComponent extends FieldArrayType<FormlyFieldConfig> { label: "Type", key: "attributeType", parentKey: "attribute", disabled: true }, ]; + readonly addParameterTypeOptions: AttributeType[] = ["string", "integer", "long", "double", "boolean", "timestamp"]; + draftVisible = false; + + constructor( + private workflowActionService: WorkflowActionService, + private uiUdfParametersSyncService: UiUdfParametersSyncService, + private notificationService: NotificationService + ) { + super(); + } + + get workflowModificationEnabled(): boolean { + return this.workflowActionService.checkWorkflowModificationEnabled(); + } + + /** Inserts the declaration into the operator's Python code; the row then appears through the normal code sync. */ + addParameter(nameInput: HTMLInputElement, attributeType: string): void { + const operatorId = this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs()[0]; + try { + this.uiUdfParametersSyncService.addParameter(operatorId, nameInput.value, attributeType as AttributeType); + this.draftVisible = false; + } catch (error) { + if (!(error instanceof UiUdfParametersEditError) && !(error instanceof UiUdfParametersParseError)) throw error; + this.notificationService.error(`Could not add UDF parameter: ${error.message}`); + } + } + override onPopulate(field: FormlyFieldConfig): void { this.configureRowTemplate(this.getFieldArrayTemplate(field)); super.onPopulate(field); 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 e9fc4e3494..66a808c108 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 @@ -18,13 +18,16 @@ */ import { + UiUdfParametersEditError, UiUdfParametersParseError, UiUdfParametersParserService, type UiUdfParameter, } from "./ui-udf-parameters-parser.service"; +import type { AttributeType } from "../../types/workflow-compiling.interface"; 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."; +const PASS_ONLY_OPEN = "class ProcessTupleOperator(UDFOperatorV2):\n def open(self):\n pass\n"; describe("UiUdfParametersParserService", () => { let service: UiUdfParametersParserService; @@ -202,6 +205,190 @@ describe("UiUdfParametersParserService", () => { }); }); +describe("UiUdfParametersParserService.computeParameterInsertion", () => { + let service: UiUdfParametersParserService; + + beforeEach(() => { + service = new UiUdfParametersParserService(); + }); + + // Each case is the expected file, with the inserted lines marked ">": the input is the same + // file without them. Every case also re-parses the result to prove the round trip. + ( + [ + [ + "insert before existing UiParameter declarations", + "b", + "double", + [ + "class ProcessTupleOperator(UDFOperatorV2):", + " def open(self):", + '> self.b = self.UiParameter(name="b", type=AttributeType.DOUBLE).value', + ' self.a = self.UiParameter(name="a", type=AttributeType.INT).value', + " self.other = 1", + ], + ], + [ + "insert after an open() docstring and before executable statements", + "b", + "integer", + [ + "class ProcessTupleOperator(UDFOperatorV2):", + " def open(self):", + ' """Load resources."""', + '> self.b = self.UiParameter(name="b", type=AttributeType.INT).value', + " self.other = 1", + ], + ], + [ + "insert before a header-based UiParameter and its following compound statement", + "limit", + "string", + [ + "class ProcessTupleOperator(UDFOperatorV2):", + " def open(self):", + '> self.limit = self.UiParameter(name="limit", type=AttributeType.STRING).value', + ' if self.UiParameter(name="debug", type=AttributeType.BOOL).value:', + " self.x = 1", + " for i in range(3):", + " pass", + ], + ], + [ + "create a decorated open() before the first method when the code uses @overrides", + "b", + "double", + [ + "class ProcessTupleOperator(UDFOperatorV2):", + "> @overrides", + "> def open(self) -> None:", + '> self.b = self.UiParameter(name="b", type=AttributeType.DOUBLE).value', + ">", + " @overrides", + " def process_tuple(self, tuple_, port):", + " yield tuple_", + ], + ], + [ + "create an undecorated open() after a class docstring", + "b", + "boolean", + [ + "class ProcessTupleOperator(UDFOperatorV2):", + ' """Doc."""', + ">", + "> def open(self) -> None:", + '> self.b = self.UiParameter(name="b", type=AttributeType.BOOL).value', + ], + ], + [ + // Exact user flow: only the import and the class header line of the template are + // uncommented, so the class body holds nothing but comments and lezer's error node. + "start the class body with open() when the template body is still commented out", + "carlos", + "string", + [ + "from pytexera import *", + "# ", + "class ProcessTupleOperator(UDFOperatorV2):", + "> def open(self) -> None:", + '> self.carlos = self.UiParameter(name="carlos", type=AttributeType.STRING).value', + "# ", + "# @overrides", + "# def process_tuple(self, tuple_: Tuple, port: int) -> Iterator[Optional[TupleLike]]:", + "# yield tuple_", + "# ", + "# class ProcessBatchOperator(UDFBatchOperator):", + ], + ], + [ + "start the open() body when its statements are still commented out", + "b", + "long", + [ + "class ProcessTupleOperator(UDFOperatorV2):", + " def open(self):", + '> self.b = self.UiParameter(name="b", type=AttributeType.LONG).value', + " # self.a = 1", + "", + " def process_tuple(self, tuple_, port):", + " yield tuple_", + ], + ], + ] as ReadonlyArray<readonly [string, string, AttributeType, string[]]> + ).forEach(([description, name, attributeType, annotatedLines]) => { + it(`should ${description}`, () => { + const input = pythonLines(...annotatedLines.filter(line => !line.startsWith(">"))); + const expected = pythonLines(...annotatedLines.map(line => (line.startsWith(">") ? line.slice(1) : line))); + + const updatedCode = insertParameter(service, input, name, attributeType); + + expect(updatedCode).toBe(expected); + expect(service.parse(updatedCode).map(parsed => parsed.attribute.attributeName)).toContain(name); + }); + }); + + it("should sanitize assignment targets while keeping exact names, and accumulate declarations", () => { + let code = PASS_ONLY_OPEN; + code = insertParameter(service, code, "my param-1", "timestamp"); + code = insertParameter(service, code, "class"); + + expect(code).toContain('self.my_param_1 = self.UiParameter(name="my param-1", type=AttributeType.TIMESTAMP).value'); + expect(code).toContain('self.class_ = self.UiParameter(name="class", type=AttributeType.DOUBLE).value'); + expect(service.parse(code)).toEqual([ + { attribute: { attributeName: "class", attributeType: "double" }, value: "" }, + { attribute: { attributeName: "my param-1", attributeType: "timestamp" }, value: "" }, + ]); + }); + + ( + [ + [ + "no supported UDF class", + "class RandomClass(ABC):\n def open(self):\n pass", + "b", + "double", + "No supported Python UDF class", + ], + [ + "a duplicate parameter name", + PASS_ONLY_OPEN.replace("pass", 'self.b = self.UiParameter(name="b", type=AttributeType.DOUBLE).value'), + "b", + "double", + "UiParameter name 'b' is declared already.", + ], + [ + "a single-line open() body", + "class ProcessTupleOperator(UDFOperatorV2):\n def open(self): pass", + "b", + "double", + "need an indented block body", + ], + ["an empty parameter name", PASS_ONLY_OPEN, " ", "double", "UiParameter name is required."], + ["an unsupported parameter type", PASS_ONLY_OPEN, "b", "binary", "UiParameter type 'binary' is not supported."], + ] as ReadonlyArray<readonly [string, string, string, AttributeType, string]> + ).forEach(([description, code, name, attributeType, message]) => { + it(`should raise an error for ${description}`, () => { + expect(() => service.computeParameterInsertion(code, name, attributeType)).toThrow(UiUdfParametersEditError); + expect(() => service.computeParameterInsertion(code, name, attributeType)).toThrow(message); + }); + }); +}); + +function insertParameter( + service: UiUdfParametersParserService, + code: string, + name: string, + attributeType: AttributeType = "double" +): string { + const edit = service.computeParameterInsertion(code, name, attributeType); + return code.slice(0, edit.offset) + edit.text + code.slice(edit.offset); +} + +function pythonLines(...lines: string[]): string { + return `${lines.join("\n")}\n`; +} + function expectParsed( service: UiUdfParametersParserService, openBody: string, 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 5c0acb81d0..3db18b05a9 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 @@ -32,14 +32,19 @@ const SUPPORTED_CLASS_NAMES = new Set([ const PYTHON_NODE = { ARG_LIST: "ArgList", ASSIGN_OP: "AssignOp", + BODY: "Body", CALL_EXPRESSION: "CallExpression", CLASS_DEFINITION: "ClassDefinition", + EXPRESSION_STATEMENT: "ExpressionStatement", + FUNCTION_DEFINITION: "FunctionDefinition", MEMBER_EXPRESSION: "MemberExpression", PROPERTY_NAME: "PropertyName", STRING: "String", VARIABLE_NAME: "VariableName", } as const; const ARGUMENT_DELIMITER_NODES = new Set(["(", ")", ","]); +// "⚠" is lezer's error node: a body whose real statements are still commented out ends in one. +const NON_STATEMENT_BODY_NODES = new Set([":", "Comment", "⚠"]); const UI_PARAMETER_CALLEE = ["self", "UiParameter"]; const ATTRIBUTE_TYPE_RECEIVER = "AttributeType"; @@ -60,6 +65,9 @@ export type UiUdfParameter = Readonly<{ attribute: SchemaAttribute; value: strin /** Raised when supported Python UDF code declares UI parameters that cannot be represented safely in the UI. */ export class UiUdfParametersParseError extends Error {} +/** Raised when a new UiParameter declaration cannot be inserted into the Python UDF code. */ +export class UiUdfParametersEditError extends Error {} + // Accept Java enum names (INTEGER, BOOLEAN) and Python enum aliases (INT, BOOL). const ATTRIBUTE_TYPES_BY_TOKEN: Readonly<Record<string, AttributeType>> = { STRING: "string", @@ -72,6 +80,24 @@ const ATTRIBUTE_TYPES_BY_TOKEN: Readonly<Record<string, AttributeType>> = { TIMESTAMP: "timestamp", }; +// Python AttributeType member emitted when generating declarations; matches the pytexera enum and template style. +const PYTHON_TOKENS_BY_ATTRIBUTE_TYPE: Readonly<Partial<Record<AttributeType, string>>> = { + string: "STRING", + integer: "INT", + long: "LONG", + double: "DOUBLE", + boolean: "BOOL", + timestamp: "TIMESTAMP", +}; + +// Hard keywords only; soft keywords (match, case, type) are valid attribute names. +const PYTHON_KEYWORDS = new Set( + ( + "False None True and as assert async await break class continue def del elif else except finally " + + "for from global if import in is lambda nonlocal not or pass raise return try while with yield" + ).split(" ") +); + /** Parses Python UDF source code and infers supported self.UiParameter(...) declarations for the property panel. */ @Injectable({ providedIn: "root" }) export class UiUdfParametersParserService { @@ -83,48 +109,153 @@ export class UiUdfParametersParserService { if (!code) return []; const result: UiUdfParameter[] = []; - const seen = new Set<string>(); - let supportedClassCount = 0; - let duplicateName: string | undefined; - const addParameter = (parameter?: UiUdfParameter): void => { - const name = parameter?.attribute.attributeName; - if (parameter && name) { - if (seen.has(name)) { - duplicateName = name; - return; - } - seen.add(name); + const supportedClass = findSupportedClass(code); + if (supportedClass) + forEachUiParameterCall(supportedClass, code, parameter => { + if (result.some(existing => existing.attribute.attributeName === parameter.attribute.attributeName)) + throw new UiUdfParametersParseError( + `UiParameter name '${parameter.attribute.attributeName}' is declared more than once.` + ); result.push(parameter); - } - }; - - parser.parse(code).iterate({ - enter: ({ name, node }) => { - const className = node.getChild(PYTHON_NODE.VARIABLE_NAME); - if ( - 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 !== PYTHON_NODE.CALL_EXPRESSION) 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; } + + /** + * Computes the text insertion that declares a new self.UiParameter(...) inside open() of the + * supported Python UDF class, creating open() when the class does not define one. + * Throws UiUdfParametersEditError when the declaration cannot be placed. + */ + computeParameterInsertion( + code: string, + name: string, + attributeType: AttributeType + ): Readonly<{ offset: number; text: string }> { + const attributeName = name.trim(); + const pythonToken = PYTHON_TOKENS_BY_ATTRIBUTE_TYPE[attributeType]; + if (!attributeName) throw new UiUdfParametersEditError("UiParameter name is required."); + if (!pythonToken) throw new UiUdfParametersEditError(`UiParameter type '${attributeType}' is not supported.`); + if (this.parse(code).some(parameter => parameter.attribute.attributeName === attributeName)) + throw new UiUdfParametersEditError(`UiParameter name '${attributeName}' is declared already.`); + + const declaration = + `self.${toPythonIdentifier(attributeName)} = ` + + `self.UiParameter(name=${JSON.stringify(attributeName)}, type=AttributeType.${pythonToken}).value`; + const supportedClass = findSupportedClass(code); + if (!supportedClass) + throw new UiUdfParametersEditError( + "No supported Python UDF class (such as ProcessTupleOperator) was found in the code." + ); + + const openMethod = findOpenMethod(supportedClass, code); + if (openMethod) return insertIntoBody(code, openMethod, [declaration]); + return insertIntoBody(code, supportedClass, [ + ...(/^\s*@overrides\b/m.test(code) ? ["@overrides"] : []), + "def open(self) -> None:", + ` ${declaration}`, + ]); + } +} + +/** Returns the single supported UDF class; throws when several declare UiParameter-capable classes. */ +function findSupportedClass(code: string): ParserSyntaxNode | undefined { + const classes: ParserSyntaxNode[] = []; + parser.parse(code).iterate({ + enter: ({ name, node }) => { + const className = name === PYTHON_NODE.CLASS_DEFINITION ? node.getChild(PYTHON_NODE.VARIABLE_NAME) : null; + if (!className || !SUPPORTED_CLASS_NAMES.has(code.slice(className.from, className.to))) return; + classes.push(node); + return false; + }, + }); + if (classes.length > 1) + throw new UiUdfParametersParseError("Only one Python UDF class can declare UiParameter values."); + return classes[0]; +} + +function forEachUiParameterCall( + supportedClass: ParserSyntaxNode, + code: string, + visit: (parameter: UiUdfParameter, call: ParserSyntaxNode) => void +): void { + supportedClass.cursor().iterate(cursorReference => { + if (cursorReference.name !== PYTHON_NODE.CALL_EXPRESSION) return; + const parameter = readCall(cursorReference.node, code); + if (parameter) visit(parameter, cursorReference.node); + return false; + }); +} + +function findOpenMethod(supportedClass: ParserSyntaxNode, code: string): ParserSyntaxNode | undefined { + const body = supportedClass.getChild(PYTHON_NODE.BODY); + for (const statement of body ? getChildren(body) : []) { + const definition = + statement.name === PYTHON_NODE.FUNCTION_DEFINITION + ? statement + : statement.getChild(PYTHON_NODE.FUNCTION_DEFINITION); + const definitionName = definition?.getChild(PYTHON_NODE.VARIABLE_NAME); + if (definition && definitionName && code.slice(definitionName.from, definitionName.to) === "open") + return definition; + } + return undefined; +} + +/** + * Inserts lines at the start of a class or def body while preserving a leading docstring. + * When the body has no real statement yet (for example a template whose statements are all + * commented out), the lines go right after the header. + */ +function insertIntoBody( + code: string, + definition: ParserSyntaxNode, + lines: string[] +): Readonly<{ offset: number; text: string }> { + const body = definition.getChild(PYTHON_NODE.BODY); + const statements = body ? getChildren(body).filter(child => !NON_STATEMENT_BODY_NODES.has(child.name)) : []; + const first = statements[0]; + if (body && first && code.slice(body.from, first.from).includes("\n")) { + const indent = lineIndentation(code, first.from); + if (isDocstringStatement(first)) { + const block = lines.map(line => `${indent}${line}`).join("\n"); + const leadingSeparator = lines.length > 1 ? "\n\n" : "\n"; + const trailingSeparator = lines.length > 1 && statements.length > 1 ? "\n" : ""; + return { + offset: lineEnd(code, Math.max(first.from, first.to - 1)), + text: `${leadingSeparator}${block}${trailingSeparator}`, + }; + } + const block = lines.map(line => `${indent}${line}\n`).join(""); + // A synthesized open() gets a blank separator line before the statement that follows it. + return { offset: lineStart(code, first.from), text: lines.length > 1 ? `${block}\n` : block }; + } + if (first || !body) + throw new UiUdfParametersEditError( + "The Python UDF class and open() need an indented block body to declare UiParameter values." + ); + const indent = `${lineIndentation(code, definition.from)} `; + return { offset: lineEnd(code, body.from), text: lines.map(line => `\n${indent}${line}`).join("") }; +} + +function isDocstringStatement(statement: ParserSyntaxNode): boolean { + return statement.name === PYTHON_NODE.EXPRESSION_STATEMENT && statement.getChild(PYTHON_NODE.STRING) !== null; +} + +function toPythonIdentifier(name: string): string { + const identifier = name.replace(/\W/g, "_").replace(/^(?=\d)/, "_") || "parameter"; + return PYTHON_KEYWORDS.has(identifier) ? `${identifier}_` : identifier; +} + +function lineStart(code: string, position: number): number { + return code.lastIndexOf("\n", position - 1) + 1; +} + +function lineEnd(code: string, position: number): number { + const newline = code.indexOf("\n", position); + return newline === -1 ? code.length : newline; +} + +function lineIndentation(code: string, position: number): string { + return code.slice(lineStart(code, position), position).match(/^[ \t]*/)?.[0] ?? ""; } function readCall(call: ParserSyntaxNode, code: string): UiUdfParameter | 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 3d3613fa5a..e07f565256 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,7 +19,11 @@ import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; import { PYTHON_UDF_V2_OP_TYPE } from "../workflow-graph/model/workflow-graph"; -import { UiUdfParametersParseError, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { + UiUdfParametersEditError, + 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 type { Mock } from "vitest"; @@ -31,7 +35,7 @@ describe("UiUdfParametersSyncService", () => { const code = "self.UiParameter(...)"; let service: UiUdfParametersSyncService; - let parserServiceMock: { parse: Mock }; + let parserServiceMock: { parse: Mock; computeParameterInsertion: Mock }; let graphMock: { getOperator: Mock; getSharedOperatorType: Mock }; let operator: { operatorType: string; operatorProperties: { uiParameters: UiUdfParameter[] } }; @@ -45,7 +49,7 @@ describe("UiUdfParametersSyncService", () => { ), getSharedOperatorType: vitest.fn(), }; - parserServiceMock = { parse: vitest.fn() }; + parserServiceMock = { parse: vitest.fn(), computeParameterInsertion: vitest.fn() }; service = new UiUdfParametersSyncService( { getTexeraGraph: vitest.fn().mockReturnValue(graphMock) } as unknown as WorkflowActionService, parserServiceMock as unknown as UiUdfParametersParserService @@ -207,6 +211,41 @@ describe("UiUdfParametersSyncService", () => { } }); + it("should insert the computed parameter edit into shared code and re-sync the structure", () => { + const sharedCode = "class ProcessTupleOperator(UDFOperatorV2):\n pass\n"; + const sharedOperator = sharedOperatorType(sharedCode); + graphMock.getSharedOperatorType.mockReturnValue(sharedOperator); + parserServiceMock.computeParameterInsertion.mockReturnValue({ offset: 0, text: "# inserted\n" }); + parserServiceMock.parse.mockReturnValue([parameter("count", "integer")]); + + const parametersChangedObserver = observeParameterChanges(); + + service.addParameter(operatorId, "count", "integer"); + + expect(parserServiceMock.computeParameterInsertion).toHaveBeenCalledWith(sharedCode, "count", "integer"); + const yCode = (sharedOperator.get("operatorProperties") as Yjs.Map<unknown>).get("code") as Yjs.Text; + expect(yCode.toString()).toBe(`# inserted\n${sharedCode}`); + expect(parserServiceMock.parse).toHaveBeenCalledWith(`# inserted\n${sharedCode}`); + expect(parametersChangedObserver).toHaveBeenCalledWith({ + operatorId, + parameters: [parameter("count", "integer")], + }); + }); + + it("should throw without editing when shared code is unavailable", () => { + const consoleWarnSpy = vitest.spyOn(console, "warn").mockImplementation(() => undefined); + graphMock.getSharedOperatorType.mockImplementation(() => { + throw new Error("missing shared operator"); + }); + + try { + expect(() => service.addParameter(operatorId, "count", "integer")).toThrow(UiUdfParametersEditError); + expect(parserServiceMock.computeParameterInsertion).not.toHaveBeenCalled(); + } finally { + consoleWarnSpy.mockRestore(); + } + }); + function observeParameterChanges(): Mock { const parametersChangedObserver = vitest.fn(); service.uiParametersChanged$.subscribe(parametersChangedObserver); 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 bb3c832420..54aa5ec363 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,17 @@ import { isEqual } from "lodash-es"; import { Subject } from "rxjs"; import { debounceTime } from "rxjs/operators"; import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service"; -import { UiUdfParametersParseError, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service"; +import { + UiUdfParametersEditError, + 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 type { Text as YText } from "yjs"; import type { YType } from "../../types/shared-editing.interface"; +import type { AttributeType } from "../../types/workflow-compiling.interface"; type SharedOperatorProperties = Readonly<{ code?: string; [key: string]: unknown }>; @@ -117,13 +122,34 @@ export class UiUdfParametersSyncService { })); } + /** + * Inserts a new self.UiParameter declaration into the operator's shared Python code + * (creating open() when missing) and immediately re-syncs the parameter rows. + * Throws UiUdfParametersEditError or UiUdfParametersParseError when the code cannot be edited. + */ + addParameter(operatorId: string, attributeName: string, attributeType: AttributeType): void { + const yCode = this.getSharedYCode(operatorId); + if (!yCode) throw new UiUdfParametersEditError("Python UDF code is not available for this operator."); + + const edit = this.uiUdfParametersParserService.computeParameterInsertion( + yCode.toString(), + attributeName, + attributeType + ); + yCode.insert(edit.offset, edit.text); + this.syncStructureFromCode(operatorId); + } + private getSharedCode(operatorId: string): string | undefined { + return this.getSharedYCode(operatorId)?.toString(); + } + + private getSharedYCode(operatorId: string): YText | undefined { try { const sharedOperatorType = this.workflowActionService.getTexeraGraph().getSharedOperatorType(operatorId); const operatorProperties = sharedOperatorType.get("operatorProperties") as YType<SharedOperatorProperties>; - const yCode = operatorProperties.get("code") as unknown as YText; - return yCode?.toString(); + return operatorProperties.get("code") as unknown as YText; } catch (error) { console.warn("Unable to read Python UDF code from shared operator properties.", error); return undefined;
