Xiao-zhen-Liu commented on code in PR #5043:
URL: https://github.com/apache/texera/pull/5043#discussion_r3261286198
##########
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",
Review Comment:
Since this is a new library, please note the license of this library in the
PR description. @bobbai00 is there any protocol we should follow from now on
when introducing new dependencies, given our recent release experience?
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,217 @@
+/**
+ * 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 UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY";
+type SupportedParserAttributeTypeToken = Exclude<ParserAttributeTypeToken,
UnsupportedParserAttributeTypeToken>;
+type ParserSyntaxNode = ReturnType<typeof parser.parse>["topNode"];
+
+const TYPES: Readonly<Record<SupportedParserAttributeTypeToken,
AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new
Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new
Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "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<string>();
+ 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[] = [];
Review Comment:
This function parses the argument list character by character, but Lezer's
`ArgList` node already provides typed child nodes for each argument: `String`
for literals, `MemberExpression` for things like `AttributeType.INT`, and a
separate node type for `key=value` arguments. Using those nodes directly would
remove around 40 lines from this file, handle cases like f-strings, nested
calls, and multiline arguments automatically, and make it easier to extend the
parser in later PRs (for example to support default values or constraints).
Better to do this now rather than carry `splitArgs` forward into PR #2.
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,217 @@
+/**
+ * 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 UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY";
+type SupportedParserAttributeTypeToken = Exclude<ParserAttributeTypeToken,
UnsupportedParserAttributeTypeToken>;
+type ParserSyntaxNode = ReturnType<typeof parser.parse>["topNode"];
+
+const TYPES: Readonly<Record<SupportedParserAttributeTypeToken,
AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new
Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new
Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "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<string>();
+ 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;
Review Comment:
The `false` here is semantically important. It tells Lezer not to descend
further into the node. Putting it inside a comma expression makes it look like
an accident. A small block body is slightly longer but reads more clearly:
```ts
.iterate(ref => {
if (ref.name !== "CallExpression") return;
add(readCall(ref.node, code));
return false;
});
```
##########
frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html:
##########
@@ -0,0 +1,55 @@
+<!--
+ 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.
+-->
+<div
+ class="ui-udf-param-list"
+ *ngIf="(model?.length ?? 0) > 0">
+ <!-- Optional header row -->
+ <div class="ui-udf-param-row header">
+ <div class="field-cell"><span class="col-title">Value</span></div>
+ <div class="field-cell"><span class="col-title">Name</span></div>
+ <div class="field-cell"><span class="col-title">Type</span></div>
+ </div>
+
+ <div
+ class="ui-udf-param-row"
+ *ngFor="let param of (model || []); let i = index; trackBy:
trackByParamName">
+ <ng-container *ngIf="field.fieldGroup?.[i] as rowField">
+ <!-- Value -->
+ <div class="field-cell">
+ <ng-container *ngIf="getValueField(rowField) as valueField">
Review Comment:
These getters (`getValueField`, `getNameField`, `getTypeField`) re-run
`setDisabled` on every change-detection cycle. Each call assigns a fresh
`props` object and re-calls `disable()`. Disabling a control should happen once
per row, not on every cycle. Please move this logic into each row's
`FormlyFieldConfig.hooks.onInit` hook, or better, set `disabled: true` directly
in the field schema when the schema is built.
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,217 @@
+/**
+ * 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"]);
Review Comment:
Two things here. (1) This matches the literal class name, so `class
MyOp(ProcessTupleOperator):` won't have its parameters parsed. Is that
intentional? If yes, please add a test that documents the behavior. If no, the
parser should also check base classes. (2) These four names are coupled to the
Python SDK. Please add a one-line comment pointing at where they're defined
upstream, so future maintainers know to keep them in sync.
##########
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 {
Review Comment:
Every keystroke triggers a full Lezer parse, a scan of the parsed tree, and
an `isEqual` check. Lezer is fast, but at typing speed in a long UDF body this
does more work than necessary. Please wrap the handler with `debounceTime(200)`
or similar. This is the standard pattern at the editor-to-graph boundary.
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,217 @@
+/**
+ * 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 UnsupportedParserAttributeTypeToken = "BINARY" | "LARGE_BINARY";
+type SupportedParserAttributeTypeToken = Exclude<ParserAttributeTypeToken,
UnsupportedParserAttributeTypeToken>;
+type ParserSyntaxNode = ReturnType<typeof parser.parse>["topNode"];
+
+const TYPES: Readonly<Record<SupportedParserAttributeTypeToken,
AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new
Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new
Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "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<string>();
+ 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;
+ }
Review Comment:
This block is hard to read: three pieces of state (`match`, `key`, `index`)
and four interleaved conditions, with two post-block guards that fire *after*
`index` has been incremented, so they check at a different point than the
in-block branches. A future edit (like "now handle a third argument") would be
very risky.
If the AST suggestion from the `splitArgs` comment is applied, most of this
complexity disappears, since the loop will iterate over typed child nodes and
assign to a small accumulator. If not, please split this into a small helper
that parses one argument and returns `{ kind: 'name'|'type', value }`, and have
the loop dispatch on the result.
##########
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<string, string>();
+ existingParameters.forEach((parameter: any) => {
+ const parameterName = parameter?.attribute?.attributeName ??
parameter?.attribute?.name;
Review Comment:
What shape is the `?? parameter?.attribute?.name` branch handling? Is it
saved workflows from earlier in development? If so, please either write a
one-time migration and delete the fallback, or leave a TODO with a target
removal date. Carrying a legacy reader in brand-new code without a removal
target tends to stick around indefinitely. Also, please type
`existingParameters` as `UiUdfParameter[]` instead of `any[]`. TypeScript can
then catch shape drift on this boundary.
##########
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<string, string>();
+ 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;
+ }
+ }
+
Review Comment:
This duplicates the operator-type list in `isPythonUdf(operator)` at
`workflow-graph.ts:84`. If a new Python UDF type gets added later, only one of
the two lists will get updated. Please refactor `isPythonUdf` to also accept an
operatorType string (or call it with the operator object that was already
fetched above), and delete this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
