From 7202537d2e9d2dfec286ab772cb174b043db8273 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Wed, 2 Sep 2026 13:06:44 -0700 Subject: [PATCH 1/3] feat(gui): offer a workflow as a Python script A button on the workflow toolbar asks the compiling service to translate the canvas into a script and shows what comes back, with a copy control so the script can be taken somewhere that runs it. The translation is a round trip, so the button disables itself while one is in flight. A workflow the translator cannot express comes back with a reason rather than an empty modal, and that reason is what the notification says. The route is added in the two places a browser reaches the compiling service from: the dev server's proxy and the single-node nginx. Without them the button reaches nothing. Co-Authored-By: Claude Opus 5 (1M context) --- bin/single-node/nginx.conf | 6 + frontend/junit.xml | 213 ++++++++++++++++++ frontend/proxy.config.json | 5 + .../workflow-to-python.service.ts | 58 +++++ .../component/menu/menu.component.html | 27 +++ .../component/menu/menu.component.scss | 25 ++ .../component/menu/menu.component.spec.ts | 50 ++++ .../component/menu/menu.component.ts | 52 ++++- 8 files changed, 435 insertions(+), 1 deletion(-) create mode 100644 frontend/junit.xml create mode 100644 frontend/src/app/dashboard/service/user/workflow-to-python/workflow-to-python.service.ts diff --git a/bin/single-node/nginx.conf b/bin/single-node/nginx.conf index 688e256d46d..f0d6970f11b 100644 --- a/bin/single-node/nginx.conf +++ b/bin/single-node/nginx.conf @@ -33,6 +33,12 @@ http { proxy_set_header X-Real-IP $remote_addr; } + location /api/workflow-to-python { + proxy_pass http://workflow-compiling-service:9090; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + } + location /api/dataset { proxy_pass http://file-service:9092; proxy_set_header Host $host; diff --git a/frontend/junit.xml b/frontend/junit.xml new file mode 100644 index 00000000000..914f23d6c20 --- /dev/null +++ b/frontend/junit.xml @@ -0,0 +1,213 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/frontend/proxy.config.json b/frontend/proxy.config.json index 7801e0c256f..27bef687b15 100755 --- a/frontend/proxy.config.json +++ b/frontend/proxy.config.json @@ -30,6 +30,11 @@ "secure": false, "changeOrigin": true }, + "/api/workflow-to-python": { + "target": "http://localhost:9090", + "secure": false, + "changeOrigin": true + }, "/api/dataset": { "target": "http://localhost:9092", "secure": false, diff --git a/frontend/src/app/dashboard/service/user/workflow-to-python/workflow-to-python.service.ts b/frontend/src/app/dashboard/service/user/workflow-to-python/workflow-to-python.service.ts new file mode 100644 index 00000000000..301515d9568 --- /dev/null +++ b/frontend/src/app/dashboard/service/user/workflow-to-python/workflow-to-python.service.ts @@ -0,0 +1,58 @@ +/** + * 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 { HttpClient, HttpHeaders } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import { Observable } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; +import { LogicalPlan } from "../../../../workspace/types/execute-workflow.interface"; + +export const WORKFLOW_TO_PYTHON_ENDPOINT = "workflow-to-python"; + +export interface WorkflowToPythonResponse { + type: "success" | "failure"; + pythonCode?: string; + errorMessage?: string; +} + +@Injectable({ + providedIn: "root", +}) +export class WorkflowToPythonService { + constructor(private httpClient: HttpClient) {} + + public convertToPython(logicalPlan: LogicalPlan): Observable { + const body = { + operators: logicalPlan.operators, + links: logicalPlan.links, + opsToReuseResult: [], + opsToViewResult: [], + }; + + return this.httpClient.post( + `${AppSettings.getApiEndpoint()}/${WORKFLOW_TO_PYTHON_ENDPOINT}`, + body, + { + headers: new HttpHeaders({ + "Content-Type": "application/json", + }), + } + ); + } +} diff --git a/frontend/src/app/workspace/component/menu/menu.component.html b/frontend/src/app/workspace/component/menu/menu.component.html index cfdeea77105..445996e90b1 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.html +++ b/frontend/src/app/workspace/component/menu/menu.component.html @@ -129,6 +129,15 @@ nz-icon nzType="download"> + + + 
{{ pythonCodeForModal }}
+ + diff --git a/frontend/src/app/workspace/component/menu/menu.component.scss b/frontend/src/app/workspace/component/menu/menu.component.scss index deb31c02586..adf267f232f 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.scss +++ b/frontend/src/app/workspace/component/menu/menu.component.scss @@ -210,6 +210,31 @@ texera-coeditor-user-icon { } } +.workflow-python-modal { + display: flex; + flex-direction: column; + gap: 8px; +} + +.workflow-python-modal-toolbar { + display: flex; + justify-content: flex-end; +} + +.workflow-python-code { + max-height: 70vh; + overflow: auto; + margin: 0; + padding: 12px; + white-space: pre-wrap; + word-break: break-word; + background: #f6f8fa; + border: 1px solid #d9d9d9; + border-radius: 4px; + font-size: 12px; + line-height: 1.5; +} + .jupyter-notebook-icon { height: 1.1em; width: auto; diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index c726bfec659..84c50ecf131 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -55,6 +55,10 @@ import { USER_WORKFLOW } from "../../../app-routing.constant"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; +import { + WorkflowToPythonResponse, + WorkflowToPythonService, +} from "../../../dashboard/service/user/workflow-to-python/workflow-to-python.service"; import type { Mocked } from "vitest"; vi.mock("file-saver", () => ({ saveAs: vi.fn() })); @@ -544,6 +548,52 @@ describe("MenuComponent", () => { }); }); + describe("export as Python", () => { + it("shows the returned script in a modal", () => { + const toPython = TestBed.inject(WorkflowToPythonService); + vi.spyOn(toPython, "convertToPython").mockReturnValue( + of({ type: "success", pythonCode: "print('hello')" } as WorkflowToPythonResponse) + ); + const fakeModalRef = { afterClose: of(undefined) } as unknown as NzModalRef; + const createSpy = vi.spyOn(modalService, "create").mockReturnValue(fakeModalRef); + + component.onClickExportAsPython(); + + expect(component.pythonCodeForModal).toBe("print('hello')"); + expect(createSpy).toHaveBeenCalledTimes(1); + // the button is re-enabled whichever way the request ends, so the user is not locked out of a second try + expect(component.isTranslatingToPython).toBe(false); + }); + + it("reports the reason the translation gave rather than opening an empty modal", () => { + const toPython = TestBed.inject(WorkflowToPythonService); + vi.spyOn(toPython, "convertToPython").mockReturnValue( + of({ type: "failure", errorMessage: "Sink is not supported" } as WorkflowToPythonResponse) + ); + const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); + const createSpy = vi.spyOn(modalService, "create"); + + component.onClickExportAsPython(); + + expect(errorSpy).toHaveBeenCalledWith("Sink is not supported"); + expect(createSpy).not.toHaveBeenCalled(); + expect(component.isTranslatingToPython).toBe(false); + }); + + it("copyPythonCodeToClipboard writes the generated Python script to the clipboard", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + vi.stubGlobal("navigator", { clipboard: { writeText } }); + const successSpy = vi.spyOn(notificationService, "success").mockImplementation(() => {}); + component.pythonCodeForModal = "print('hello')"; + + await component.copyPythonCodeToClipboard(); + + expect(writeText).toHaveBeenCalledWith("print('hello')"); + expect(successSpy).toHaveBeenCalledWith("Python script copied to clipboard"); + vi.unstubAllGlobals(); + }); + }); + describe("version history", () => { it("onClickGetAllVersions delegates to workflowVersionService.displayWorkflowVersions", () => { const displaySpy = vi.spyOn(workflowVersionService, "displayWorkflowVersions").mockImplementation(() => {}); diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index b5b209b3eb0..173eb300369 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -18,7 +18,7 @@ */ import { DatePipe, Location, NgIf, NgFor, NgTemplateOutlet, AsyncPipe } from "@angular/common"; -import { Component, ElementRef, Input, OnDestroy, OnInit, ViewChild } from "@angular/core"; +import { Component, ElementRef, Input, OnDestroy, OnInit, TemplateRef, ViewChild } from "@angular/core"; import { Router, RouterLink } from "@angular/router"; import { UserService } from "../../../common/service/user/user.service"; import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; @@ -43,6 +43,7 @@ import { EMPTY, firstValueFrom, of, timer } from "rxjs"; import { NzModalService } from "ng-zorro-antd/modal"; import { ResultExportationComponent } from "../result-exportation/result-exportation.component"; import { ReportGenerationService } from "../../service/report-generation/report-generation.service"; +import { WorkflowToPythonService } from "../../../dashboard/service/user/workflow-to-python/workflow-to-python.service"; import { ShareAccessComponent } from "src/app/dashboard/component/user/share-access/share-access.component"; import { PanelService } from "../../service/panel/panel.service"; import { USER_WORKFLOW } from "../../../app-routing.constant"; @@ -129,6 +130,9 @@ export class MenuComponent implements OnInit, OnDestroy { public ComputingUnitState = ComputingUnitState; // make Angular HTML access enum definition public isWorkflowValid: boolean = true; // this will check whether the workflow error or not public isWorkflowEmpty: boolean = false; + // the translation is a round trip to the compiling service, so the button stays disabled while one is in flight + public isTranslatingToPython: boolean = false; + public pythonCodeForModal: string = ""; public isSaving: boolean = false; public isWorkflowModifiable: boolean = false; public workflowId?: number; @@ -148,6 +152,7 @@ export class MenuComponent implements OnInit, OnDestroy { @Input() public currentExecutionName: string = ""; // reset executionName @Input() public particularVersionDate: string = ""; // placeholder for the metadata information of a particular workflow version @ViewChild("workflowNameInput") workflowNameInput: ElementRef | undefined; + @ViewChild("workflowPythonScriptModal", { static: true }) workflowPythonScriptModal?: TemplateRef; // variable bound with HTML to decide if the running spinner should show public runButtonText = "Run"; @@ -188,6 +193,7 @@ export class MenuComponent implements OnInit, OnDestroy { private computingUnitStatusService: ComputingUnitStatusService, protected config: GuiConfigService, private router: Router, + private workflowToPythonService: WorkflowToPythonService, private jupyterPanelService: JupyterPanelService ) { workflowWebsocketService @@ -615,6 +621,50 @@ export class MenuComponent implements OnInit, OnDestroy { this.jupyterPanelService.openJupyterNotebookPanel(); } + public onClickExportAsPython(): void { + const logicalPlan = ExecuteWorkflowService.getLogicalPlanRequest( + this.validationWorkflowService.getValidTexeraGraph() + ); + + this.isTranslatingToPython = true; + this.workflowToPythonService + .convertToPython(logicalPlan) + .pipe(untilDestroyed(this)) + .subscribe({ + next: response => { + this.isTranslatingToPython = false; + if (response.type === "success") { + this.pythonCodeForModal = response.pythonCode ?? ""; + this.modalService.create({ + nzTitle: "Workflow as Python Script", + nzContent: this.workflowPythonScriptModal ?? "", + nzFooter: null, + nzWidth: 800, + }); + } else { + this.notificationService.error(response.errorMessage ?? "Failed to translate workflow to Python."); + } + }, + error: () => { + this.isTranslatingToPython = false; + this.notificationService.error("Request failed while translating workflow to Python."); + }, + }); + } + + public async copyPythonCodeToClipboard(): Promise { + if (!this.pythonCodeForModal) { + return; + } + + try { + await navigator.clipboard.writeText(this.pythonCodeForModal); + this.notificationService.success("Python script copied to clipboard"); + } catch (error) { + this.notificationService.error("Failed to copy Python script"); + } + } + public onClickExportWorkflow(): void { const workflowContent: WorkflowContent = this.workflowActionService.getWorkflowContent(); const workflowContentJson = JSON.stringify(workflowContent, null, 2); From 91ed19e8d8e63433dcaa60fa970f91ff9e48c30f Mon Sep 17 00:00:00 2001 From: kary zheng Date: Wed, 2 Sep 2026 13:07:02 -0700 Subject: [PATCH 2/3] chore: drop a test report that a local run left behind Co-Authored-By: Claude Opus 5 (1M context) --- frontend/junit.xml | 213 --------------------------------------------- 1 file changed, 213 deletions(-) delete mode 100644 frontend/junit.xml diff --git a/frontend/junit.xml b/frontend/junit.xml deleted file mode 100644 index 914f23d6c20..00000000000 --- a/frontend/junit.xml +++ /dev/null @@ -1,213 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From dd463c2b6317885be80ba2b85e0bf52d0df11a69 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Wed, 2 Sep 2026 14:19:33 -0700 Subject: [PATCH 3/3] feat(gui): carry the rest of the property panel and the run controls Three changes that were open as separate PRs land here instead, so the frontend arrives in one piece rather than as four reviews of the same files. A workflow that fails to compile cannot be executed, so the run button reads that state alongside the schema and port validation it already read, and running to an operator is offered only when everything upstream of it compiled too. The property panel reads the constraints the schema declares. A column property is checked against the attribute type rule for every column it names, not only the first. A hyperparameter's value renders as the control its parameter implies: a dropdown where the set is known, a number input where it is numeric, a text box otherwise. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/app/common/formly/formly-config.ts | 2 + .../app/common/formly/formly-utils.spec.ts | 323 +++++++++++++++++- .../src/app/common/formly/formly-utils.ts | 133 +++++++- .../constrained-value.component.spec.ts | 136 ++++++++ .../constrained-value.component.ts | 95 ++++++ .../component/menu/menu.component.spec.ts | 110 +++--- .../component/menu/menu.component.ts | 28 +- ...ator-property-edit-frame.component.spec.ts | 148 +++++++- .../operator-property-edit-frame.component.ts | 117 +++++-- .../context-menu.component.spec.ts | 40 ++- .../context-menu/context-menu.component.ts | 19 +- .../types/custom-json-schema.interface.ts | 35 ++ 12 files changed, 1096 insertions(+), 90 deletions(-) create mode 100644 frontend/src/app/workspace/component/constrained-value/constrained-value.component.spec.ts create mode 100644 frontend/src/app/workspace/component/constrained-value/constrained-value.component.ts diff --git a/frontend/src/app/common/formly/formly-config.ts b/frontend/src/app/common/formly/formly-config.ts index c4fc54fd77f..1ea2b679833 100644 --- a/frontend/src/app/common/formly/formly-config.ts +++ b/frontend/src/app/common/formly/formly-config.ts @@ -28,6 +28,7 @@ import { DatasetFileSelectorComponent } from "../../workspace/component/dataset- 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 { ConstrainedValueComponent } from "../../workspace/component/constrained-value/constrained-value.component"; import { DatasetVersionSelectorComponent } from "../../workspace/component/dataset-version-selector/dataset-version-selector.component"; import { HuggingFaceImageUploadComponent } from "../../workspace/component/hugging-face-image-upload/hugging-face-image-upload.component"; import { HuggingFaceComponent } from "../../workspace/component/hugging-face/hugging-face.component"; @@ -88,6 +89,7 @@ export const TEXERA_FORMLY_CONFIG = { { name: "huggingface-image-upload", component: HuggingFaceImageUploadComponent, wrappers: ["form-field"] }, { name: "repeat-section-dnd", component: FormlyRepeatDndComponent }, { name: "ui-udf-parameters", component: UiUdfParametersComponent, wrappers: ["form-field"] }, + { name: "constrainedvalue", component: ConstrainedValueComponent, wrappers: ["form-field"] }, ], wrappers: [ { name: "preset-wrapper", component: PresetWrapperComponent }, diff --git a/frontend/src/app/common/formly/formly-utils.spec.ts b/frontend/src/app/common/formly/formly-utils.spec.ts index 4bec75a6692..6f8feea682f 100644 --- a/frontend/src/app/common/formly/formly-utils.spec.ts +++ b/frontend/src/app/common/formly/formly-utils.spec.ts @@ -17,14 +17,26 @@ * under the License. */ -import { FormlyFieldConfig } from "@ngx-formly/core"; +import { FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; import { createOutputFormChangeEventStream, createShouldHideFieldFunc, + createValueRulesValidator, getFieldByName, + matchingValueRule, setChildTypeDependency, setHideExpression, + setValueRules, + valueRulesValidationMessage, } from "./formly-utils"; +import { ValueRuleSet } from "../../workspace/types/custom-json-schema.interface"; +import { Component } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { NoopAnimationsModule } from "@angular/platform-browser/animations"; +import { AbstractControl, FormGroup, ReactiveFormsModule } from "@angular/forms"; +import { FormlyNgZorroAntdModule } from "@ngx-formly/ng-zorro-antd"; +import { TEXERA_FORMLY_CONFIG } from "./formly-config"; import { Subject } from "rxjs"; import { FORM_DEBOUNCE_TIME_MS } from "../../workspace/service/execute-workflow/execute-workflow.service"; import { PortSchema } from "../../workspace/types/workflow-compiling.interface"; @@ -206,3 +218,312 @@ describe("createOutputFormChangeEventStream", () => { expect(modelCheck).toHaveBeenCalledTimes(2); }); }); + +describe("valueRules", () => { + // the shape the sklearn trainers emit: one branch per hyperparameter, keyed on the + // `parameter` chosen beside the value in the same row + const rules: ValueRuleSet = { + allOf: [ + { + if: { parameter: { valEnum: ["C"] } }, + then: { type: "number", exclusiveMinimum: 0, examples: ["1.0"] }, + }, + { + if: { parameter: { valEnum: ["degree"] } }, + then: { type: "integer", minimum: 0, examples: ["3"] }, + }, + { if: { parameter: { valEnum: ["coef0"] } }, then: { type: "number", examples: ["0.0"] } }, + // gamma takes either of two words or a number, which no type names + { + if: { parameter: { valEnum: ["gamma"] } }, + then: { pattern: "^\\s*(?:scale|auto|[-+]?[0-9]*\\.?[0-9]+)\\s*$", examples: ["scale"] }, + }, + // an accepted set carries no example: it already names every value worth offering, and + // the estimator's own default leads + { + if: { parameter: { valEnum: ["kernel"] } }, + then: { enum: ["rbf", "linear", "poly", "sigmoid", "precomputed"] }, + }, + ], + }; + + const rowField = (row: unknown): FormlyFieldConfig => ({ parent: { model: row } }) as FormlyFieldConfig; + const control = (value: unknown) => ({ value }) as any; + const check = (parameter: string, value: unknown) => + createValueRulesValidator(rules)(control(value), rowField({ parameter })); + + describe("matchingValueRule", () => { + it("selects the branch the sibling's value names", () => { + expect(matchingValueRule(rules, { parameter: "kernel" })?.enum).toEqual([ + "rbf", + "linear", + "poly", + "sigmoid", + "precomputed", + ]); + expect(matchingValueRule(rules, { parameter: "degree" })?.type).toBe("integer"); + }); + + it("selects nothing when the sibling holds a value no branch names", () => { + expect(matchingValueRule(rules, { parameter: "metric_params" })).toBeUndefined(); + }); + + it("selects nothing before the row has a sibling value at all", () => { + expect(matchingValueRule(rules, {})).toBeUndefined(); + expect(matchingValueRule(rules, undefined)).toBeUndefined(); + expect(matchingValueRule(undefined, { parameter: "C" })).toBeUndefined(); + }); + }); + + describe("createValueRulesValidator", () => { + it("accepts a value the chosen parameter's set contains", () => { + expect(check("kernel", "rbf")).toBe(true); + }); + + it("rejects a value outside that set, including one of another parameter's", () => { + expect(check("kernel", "1")).toBe(false); + expect(check("kernel", "uniform")).toBe(false); + }); + + it("holds a numeric parameter to a number", () => { + expect(check("C", "1.0")).toBe(true); + // coef0 carries no bound, so it is where number-ness alone can be checked + expect(check("coef0", "-2.5e3")).toBe(true); + expect(check("C", "abc")).toBe(false); + }); + + it("holds a whole-number parameter to a whole number", () => { + expect(check("degree", "3")).toBe(true); + // int() raises on this, so the form should not let it reach the operator + expect(check("degree", "1.5")).toBe(false); + expect(check("coef0", "-1")).toBe(true); + }); + + it("leaves emptiness to the required rule rather than answering twice", () => { + expect(check("C", "")).toBe(true); + expect(check("C", null)).toBe(true); + expect(check("kernel", undefined)).toBe(true); + }); + + it("accepts anything for a parameter no branch constrains", () => { + expect(check("metric_params", "whatever")).toBe(true); + }); + + it("holds a value to the bound the estimator puts on it", () => { + // C is open at zero, degree is closed at it, and coef0 has no bound at all + expect(check("C", "0")).toBe(false); + expect(check("C", "-1")).toBe(false); + expect(check("C", "0.0001")).toBe(true); + expect(check("degree", "0")).toBe(true); + expect(check("degree", "-1")).toBe(false); + expect(check("coef0", "-100")).toBe(true); + }); + + it("holds a parameter with a pattern to the shape it declares", () => { + // both halves of the union it describes + expect(check("gamma", "scale")).toBe(true); + expect(check("gamma", "auto")).toBe(true); + expect(check("gamma", "0.1")).toBe(true); + expect(check("gamma", " 1 ")).toBe(true); + expect(check("gamma", "abc")).toBe(false); + expect(check("gamma", "scaleauto")).toBe(false); + }); + + it("judges the same value against whichever parameter the row now holds", () => { + // a value typed for one parameter is usually wrong for the next, and stays visible. That + // the form asks again when the parameter changes is the rendered form's test below + expect(check("C", "1.0")).toBe(true); + expect(check("kernel", "1.0")).toBe(false); + }); + }); + + describe("valueRulesValidationMessage", () => { + const field = (parameter: string): FormlyFieldConfig => + ({ props: { valueRules: rules }, parent: { model: { parameter } } }) as FormlyFieldConfig; + + it("names the accepted values when there is a set", () => { + expect(valueRulesValidationMessage(null, field("kernel"))).toBe( + "must be one of rbf, linear, poly, sigmoid, precomputed" + ); + }); + + it("distinguishes a whole number from a number, and names the bound where there is one", () => { + expect(valueRulesValidationMessage(null, field("degree"))).toBe("must be a whole number of at least 0"); + expect(valueRulesValidationMessage(null, field("C"))).toBe("must be a number greater than 0"); + expect(valueRulesValidationMessage(null, field("coef0"))).toBe("must be a number"); + }); + + it("points at a working value where a pattern is what the branch declares", () => { + expect(valueRulesValidationMessage(null, field("gamma"))).toBe( + "is not a value this parameter takes, such as scale" + ); + }); + + it("says only what it knows when a pattern branch offers no example", () => { + const noExample: ValueRuleSet = { + allOf: [{ if: { parameter: { valEnum: ["gamma"] } }, then: { pattern: "^scale$" } }], + }; + const bare = { props: { valueRules: noExample }, parent: { model: { parameter: "gamma" } } }; + expect(valueRulesValidationMessage(null, bare as FormlyFieldConfig)).toBe("is not a value this parameter takes"); + }); + + it("falls back to the numeric wording when no branch applies at all", () => { + // reached when the row's parameter changes between the check failing and the message + // being read, so the message must still say something rather than throw + expect(valueRulesValidationMessage(null, field("metric_params"))).toBe("must be a number"); + }); + }); + + /** + * Through a rendered form rather than a hand-made field, because what the field carries is + * only half of it: the other half is when Angular runs a validator, which is when the control + * carrying it changes and not when the parameter beside it does. + */ + describe("setValueRules in a rendered form", () => { + /** One `paraList` row: the parameter dropdown and the value field that follows it. */ + @Component({ + standalone: true, + imports: [ReactiveFormsModule, FormlyModule], + template: `
+ +
`, + }) + class RowHost { + readonly form = new FormGroup({}); + readonly model: Record = { + paraList: [ + { parameter: "C", value: "1.0" }, + { parameter: "kernel", value: "rbf" }, + ], + }; + readonly fields: FormlyFieldConfig[] = [ + { + key: "paraList", + type: "array", + fieldArray: { + fieldGroup: [ + { + key: "parameter", + type: "enum", + props: { + options: ["C", "degree", "gamma", "kernel", "metric_params"].map(p => ({ label: p, value: p })), + }, + }, + valueField, + ], + }, + }, + ]; + } + + let valueField: FormlyFieldConfig; + let fixture: ComponentFixture; + let parameter: AbstractControl; + let value: AbstractControl; + + beforeEach(async () => { + valueField = { key: "value" }; + setValueRules(valueField, rules); + + await TestBed.configureTestingModule({ + imports: [RowHost, NoopAnimationsModule, FormlyModule.forRoot(TEXERA_FORMLY_CONFIG), FormlyNgZorroAntdModule], + }).compileComponents(); + + fixture = TestBed.createComponent(RowHost); + fixture.detectChanges(); + parameter = rowControl(0, "parameter"); + value = rowControl(0, "value"); + }); + + const rowControl = (row: number, key: string): AbstractControl => + fixture.componentInstance.form.get(`paraList.${row}.${key}`)!; + + /** What the user does: picks a parameter, leaving whatever value the row already held. */ + const choose = (parameterName: string) => { + parameter.setValue(parameterName); + fixture.detectChanges(); + }; + + it("gives the value field the control and the validator the rules call for", () => { + expect(valueField.type).toBe("constrainedvalue"); + expect(value.valid).toBe(true); + expect(fixture.debugElement.query(By.css("texera-constrained-value"))).not.toBeNull(); + }); + + it("re-judges a value the row already holds when the parameter changes under it", () => { + // 1.0 is a C, and no kernel at all + expect(value.valid).toBe(true); + + choose("kernel"); + + expect(value.valid).toBe(false); + expect(value.hasError("valueRules")).toBe(true); + }); + + it("clears the error once the parameter changes to one the value suits", () => { + choose("kernel"); + expect(value.valid).toBe(false); + + choose("gamma"); + + expect(value.valid).toBe(true); + }); + + it("re-judges when the new parameter constrains the value no rule did before", () => { + choose("metric_params"); + value.setValue("1.5"); + expect(value.valid).toBe(true); + + choose("degree"); + + expect(value.valid).toBe(false); + }); + + it("leaves an empty value to the required rule whichever parameter it sits under", () => { + value.setValue(""); + choose("kernel"); + expect(value.valid).toBe(true); + }); + + it("gives the message of the parameter now chosen, not the one judged against", () => { + choose("kernel"); + value.markAsTouched(); + fixture.detectChanges(); + + expect(fixture.nativeElement.textContent).toContain("must be one of rbf, linear, poly, sigmoid, precomputed"); + }); + + it("follows the parameter with the control the branch calls for", () => { + // scoped to the row's value field, so that neither the parameter's own dropdown nor the + // second row is what is seen + const valueControl = (selector: string) => + fixture.debugElement.queryAll(By.css("texera-constrained-value"))[0].query(By.css(selector)); + expect(valueControl("input[nz-input]").nativeElement.type).toBe("number"); + expect(valueControl("nz-select")).toBeNull(); + + choose("kernel"); + + expect(valueControl("nz-select")).not.toBeNull(); + expect(valueControl("input[nz-input]")).toBeNull(); + }); + + it("re-judges only the row whose parameter changed", () => { + const otherValue = rowControl(1, "value"); + expect(otherValue.valid).toBe(true); + + choose("kernel"); + + // rbf is still a kernel, whatever the row above holds + expect(value.valid).toBe(false); + expect(otherValue.valid).toBe(true); + + rowControl(1, "parameter").setValue("degree"); + fixture.detectChanges(); + + expect(otherValue.valid).toBe(false); + }); + }); +}); diff --git a/frontend/src/app/common/formly/formly-utils.ts b/frontend/src/app/common/formly/formly-utils.ts index cb80abe2bd5..15c25fb071a 100644 --- a/frontend/src/app/common/formly/formly-utils.ts +++ b/frontend/src/app/common/formly/formly-utils.ts @@ -22,9 +22,10 @@ import { isDefined } from "../util/predicate"; import { Observable } from "rxjs"; import { FORM_DEBOUNCE_TIME_MS } from "../../workspace/service/execute-workflow/execute-workflow.service"; -import { debounceTime, distinctUntilChanged, filter, share } from "rxjs/operators"; -import { HideType } from "../../workspace/types/custom-json-schema.interface"; +import { debounceTime, distinctUntilChanged, filter, share, tap } from "rxjs/operators"; +import { HideType, ValueRuleSet } from "../../workspace/types/custom-json-schema.interface"; import { PortSchema } from "../../workspace/types/workflow-compiling.interface"; +import { AbstractControl } from "@angular/forms"; export function getFieldByName(fieldName: string, fields: FormlyFieldConfig[]): FormlyFieldConfig | undefined { return fields.filter((field, _, __) => field.key === fieldName)[0]; @@ -39,6 +40,134 @@ export function setHideExpression(toggleHidden: string[], fields: FormlyFieldCon }); } +type ValueRule = ValueRuleSet["allOf"][number]["then"]; + +/** + * The one branch of `valueRules` that the row's current contents select, or undefined where + * none does. A branch names its sibling fields and the values of theirs it applies to, so the + * row model is what decides; `field.parent.model` is that row for an array item and the + * operator itself for a top-level field. + */ +export function matchingValueRule(rules: ValueRuleSet | undefined, rowModel: any): ValueRule | undefined { + if (!isDefined(rules) || !isDefined(rowModel)) { + return undefined; + } + return rules.allOf.find(branch => + Object.entries(branch.if).every(([sibling, condition]) => (condition.valEnum ?? []).includes(rowModel[sibling])) + )?.then; +} + +/** + * Validator holding a field to whichever branch of `valueRules` currently applies. + * + * An empty value passes: whether emptiness is allowed is `required`'s business, and a field + * that answers twice would report the wrong thing once. The numeric branches accept what + * JavaScript reads as a number, which is slightly narrower than the Python converters on the + * other end (they take `1_000` and `inf`); erring narrow here would be wrong for a field whose + * accepted set is open, but these two are bounded and the values it turns away are ones no one + * types into a hyperparameter. + */ +export function createValueRulesValidator(rules: ValueRuleSet) { + return (control: AbstractControl, field: FormlyFieldConfig): boolean => { + const rule = matchingValueRule(rules, field?.parent?.model); + if (!isDefined(rule)) { + return true; + } + const value = control.value; + if (value === null || value === undefined || value === "") { + return true; + } + const text = String(value).trim(); + if (isDefined(rule.enum)) { + return rule.enum.includes(text); + } + if (isDefined(rule.pattern)) { + // anchored the way the declaration writes it, so the same expression judges the value + // here, in the operator's own tests and in the generated Python + return new RegExp(rule.pattern).test(String(value)); + } + if (rule.type === "integer" && !/^[-+]?\d+$/.test(text)) { + return false; + } + if (rule.type === "number" && !(text.length > 0 && Number.isFinite(Number(text)))) { + return false; + } + if (isDefined(rule.type)) { + // the estimator's own bound, which it would otherwise raise on after the run started + const value = Number(text); + if (isDefined(rule.minimum) && value < rule.minimum) { + return false; + } + if (isDefined(rule.exclusiveMinimum) && value <= rule.exclusiveMinimum) { + return false; + } + } + return true; + }; +} + +/** Says what the field will take, naming the branch rather than the rule that rejected it. */ +export function valueRulesValidationMessage(_err: unknown, field: FormlyFieldConfig): string { + const rule = matchingValueRule(field?.props?.valueRules, field?.parent?.model); + if (isDefined(rule?.enum)) { + return `must be one of ${rule.enum.join(", ")}`; + } + if (isDefined(rule?.pattern)) { + // a pattern covers shapes no short phrase names, so point at a value that works instead + const example = rule.examples?.[0]; + return isDefined(example) + ? `is not a value this parameter takes, such as ${example}` + : "is not a value this parameter takes"; + } + const kind = rule?.type === "integer" ? "a whole number" : "a number"; + if (isDefined(rule?.minimum)) { + return `must be ${kind} of at least ${rule.minimum}`; + } + if (isDefined(rule?.exclusiveMinimum)) { + return `must be ${kind} greater than ${rule.exclusiveMinimum}`; + } + return `must be ${kind}`; +} + +/** + * Gives a field whose accepted values follow a sibling's both the control they call for and the + * validator holding it to them. + * + * Angular re-runs a validator only when the control carrying it changes, so the field has to be + * re-judged when the sibling deciding its rule changes. The hook reads formly's own event rather + * than the sibling control's because formly writes the row model before emitting, and the row + * model is what picks the branch. + */ +export function setValueRules(field: FormlyFieldConfig, rules: ValueRuleSet): void { + const siblings = new Set(rules.allOf.flatMap(branch => Object.keys(branch.if))); + field.type = "constrainedvalue"; + // written into the existing object rather than over it: `props` and `templateOptions` are two + // names for one object, and replacing it leaves them pointing at different ones + field.props = field.props ?? {}; + (field.props as Record).valueRules = rules; + field.validators = { + ...field.validators, + valueRules: { + expression: createValueRulesValidator(rules), + message: valueRulesValidationMessage, + }, + }; + field.hooks = { + ...field.hooks, + // returned rather than subscribed, so that formly ends it with the field + onInit: valueField => + valueField.options?.fieldChanges?.pipe( + filter( + change => + change.type === "valueChanges" && + change.field.parent === valueField.parent && + siblings.has(String(change.field.key)) + ), + tap(() => valueField.formControl?.updateValueAndValidity()) + ), + }; +} + /* Factory function to make functions that hide expressions for a particular field */ export function createShouldHideFieldFunc( hideTarget: string, diff --git a/frontend/src/app/workspace/component/constrained-value/constrained-value.component.spec.ts b/frontend/src/app/workspace/component/constrained-value/constrained-value.component.spec.ts new file mode 100644 index 00000000000..ea5221a0734 --- /dev/null +++ b/frontend/src/app/workspace/component/constrained-value/constrained-value.component.spec.ts @@ -0,0 +1,136 @@ +/** + * 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 { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { FormlyFieldConfig } from "@ngx-formly/core"; +import { NoopAnimationsModule } from "@angular/platform-browser/animations"; +import { ValueRuleSet } from "../../types/custom-json-schema.interface"; +import { ConstrainedValueComponent } from "./constrained-value.component"; + +describe("ConstrainedValueComponent", () => { + // one branch of each shape a rule can take, keyed on the sibling `parameter` + const rules: ValueRuleSet = { + allOf: [ + { + if: { parameter: { valEnum: ["kernel"] } }, + then: { enum: ["rbf", "linear", "poly", "sigmoid", "precomputed"] }, + }, + { if: { parameter: { valEnum: ["C"] } }, then: { type: "number", examples: ["1.0"] } }, + { if: { parameter: { valEnum: ["degree"] } }, then: { type: "integer", examples: ["3"] } }, + { + if: { parameter: { valEnum: ["gamma"] } }, + then: { pattern: "^\\s*(?:scale|auto|[-+]?[0-9]*\\.?[0-9]+)\\s*$", examples: ["scale"] }, + }, + ], + }; + + let fixture: ComponentFixture; + let component: ConstrainedValueComponent; + + /** Puts the component in the row a real `paraList` item would give it. */ + const showFor = (parameter: string, value: string = ""): FormControl => { + const formControl = new FormControl(value); + (component as any).field = { + key: "value", + formControl, + props: { valueRules: rules }, + parent: { model: { parameter } }, + } as FormlyFieldConfig; + fixture.detectChanges(); + return formControl; + }; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ConstrainedValueComponent, NoopAnimationsModule], + }).compileComponents(); + + fixture = TestBed.createComponent(ConstrainedValueComponent); + component = fixture.componentInstance; + }); + + it("offers the accepted values as a dropdown when the parameter is chosen from a set", () => { + showFor("kernel"); + expect(component.acceptedValues).toEqual(["rbf", "linear", "poly", "sigmoid", "precomputed"]); + expect(fixture.debugElement.query(By.css("nz-select"))).not.toBeNull(); + // nz-select carries a hidden input of its own, so look for ours rather than for any + expect(fixture.debugElement.query(By.css("input[nz-input]"))).toBeNull(); + }); + + it("gives a numeric parameter a number input instead", () => { + showFor("C"); + expect(component.acceptedValues).toEqual([]); + expect(component.inputType).toBe("number"); + expect(fixture.debugElement.query(By.css("nz-select"))).toBeNull(); + expect(fixture.debugElement.query(By.css("input[nz-input]")).nativeElement.type).toBe("number"); + }); + + it("keeps a parameter described by a pattern on a text input, since it may hold a word", () => { + showFor("gamma"); + expect(component.inputType).toBe("text"); + expect(fixture.debugElement.query(By.css("input[nz-input]")).nativeElement.type).toBe("text"); + }); + + it("leaves a parameter no branch names as a plain text box", () => { + showFor("metric_params"); + expect(component.acceptedValues).toEqual([]); + expect(component.inputType).toBe("text"); + }); + + it("follows the row when the parameter beside it changes", () => { + showFor("kernel"); + expect(fixture.debugElement.query(By.css("nz-select"))).not.toBeNull(); + + (component as any).field.parent.model.parameter = "C"; + fixture.detectChanges(); + + expect(fixture.debugElement.query(By.css("nz-select"))).toBeNull(); + expect(fixture.debugElement.query(By.css("input[nz-input]")).nativeElement.type).toBe("number"); + }); + + it("writes the control as a string whichever control produced the value", () => { + const control = showFor("C"); + // a number input yields a number once its text parses + component.write(0.1); + expect(control.value).toBe("0.1"); + expect(typeof control.value).toBe("string"); + }); + + it("writes an empty string when a dropdown is cleared", () => { + const control = showFor("kernel", "rbf"); + component.write(null); + expect(control.value).toBe(""); + }); + + it("marks the control touched so the error shows on the first bad value", () => { + const control = showFor("C"); + expect(control.touched).toBe(false); + component.write("abc"); + expect(control.dirty).toBe(true); + expect(control.touched).toBe(true); + }); + + it("reads an unset control as an empty string rather than null", () => { + const control = showFor("C"); + control.setValue(null); + expect(component.current).toBe(""); + }); +}); diff --git a/frontend/src/app/workspace/component/constrained-value/constrained-value.component.ts b/frontend/src/app/workspace/component/constrained-value/constrained-value.component.ts new file mode 100644 index 00000000000..87e305458e8 --- /dev/null +++ b/frontend/src/app/workspace/component/constrained-value/constrained-value.component.ts @@ -0,0 +1,95 @@ +/** + * 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 { ChangeDetectionStrategy, Component } from "@angular/core"; +import { CommonModule } from "@angular/common"; +import { FormsModule } from "@angular/forms"; +import { FieldType, FieldTypeConfig, FormlyModule } from "@ngx-formly/core"; +import { NzInputModule } from "ng-zorro-antd/input"; +import { NzSelectModule } from "ng-zorro-antd/select"; +import { matchingValueRule } from "../../../common/formly/formly-utils"; + +/** + * A field whose accepted values depend on what a sibling field holds: a chosen-from-a-set + * parameter renders as a dropdown, a numeric one as a number input, and anything the rules do + * not cover stays a plain text box. + * + * The value stays a string whichever control is showing. Operators that read one of these put + * the text through a converter of their own, so handing them a JSON number instead would only + * move the coercion somewhere less visible. + */ +@Component({ + selector: "texera-constrained-value", + standalone: true, + imports: [CommonModule, FormsModule, FormlyModule, NzInputModule, NzSelectModule], + changeDetection: ChangeDetectionStrategy.Default, + template: ` + + + + + + + + `, +}) +export class ConstrainedValueComponent extends FieldType { + /** The branch of the rules that the sibling's current value selects, if any. */ + private get rule() { + return matchingValueRule(this.props.valueRules, this.field?.parent?.model); + } + + get acceptedValues(): ReadonlyArray { + return this.rule?.enum ?? []; + } + + /** A number input where the rules call for a number, so a keyboard offers digits and the + * browser refuses most of what the converter would reject. + */ + get inputType(): string { + return this.rule?.type === undefined ? "text" : "number"; + } + + get current(): string { + return this.formControl.value ?? ""; + } + + /** Writes the control as a string whatever the control was. `nz-select` clears to null and a + * number input yields a number once its value parses, and both reach an operator expecting + * text. + */ + write(raw: unknown): void { + this.formControl.setValue(raw === null || raw === undefined ? "" : String(raw)); + this.formControl.markAsDirty(); + this.formControl.markAsTouched(); + } +} diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index 84c50ecf131..0f1d53ebabc 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -55,10 +55,8 @@ import { USER_WORKFLOW } from "../../../app-routing.constant"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; -import { - WorkflowToPythonResponse, - WorkflowToPythonService, -} from "../../../dashboard/service/user/workflow-to-python/workflow-to-python.service"; +import { WorkflowCompilingService } from "../../service/compile-workflow/workflow-compiling.service"; +import { CompilationState } from "../../types/workflow-compiling.interface"; import type { Mocked } from "vitest"; vi.mock("file-saver", () => ({ saveAs: vi.fn() })); @@ -76,8 +74,10 @@ describe("MenuComponent", () => { let notificationService: NotificationService; let location: Location; let validationStream$: BehaviorSubject; + let compilationStream$: Subject; beforeEach(async () => { + compilationStream$ = new Subject(); await TestBed.configureTestingModule({ imports: [MenuComponent, HttpClientTestingModule, RouterTestingModule.withRoutes([]), NzModalModule], providers: [ @@ -94,6 +94,11 @@ describe("MenuComponent", () => { }, }, { provide: UserService, useClass: StubUserService }, + { + // stubbed so the debounced compile request of the real service does not outlive the test injector + provide: WorkflowCompilingService, + useValue: { getCompilationStateInfoChangedStream: () => compilationStream$.asObservable() }, + }, ...commonTestProviders, ], }).compileComponents(); @@ -135,6 +140,18 @@ describe("MenuComponent", () => { expect(behavior.disable).toBe(true); }); + it("returns 'Invalid Workflow' when the workflow does not compile", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.isWorkflowCompilable = false; + + const behavior = component.getRunButtonBehavior(); + + expect(behavior.text).toBe("Invalid Workflow"); + expect(behavior.icon).toBe("warning"); + expect(behavior.disable).toBe(true); + }); + it("returns 'Empty Workflow' when the workflow has no operators", () => { component.isWorkflowValid = true; component.isWorkflowEmpty = true; @@ -420,6 +437,19 @@ describe("MenuComponent", () => { expect(component.computingUnitSelectionComponent.showAddComputeUnitModalVisible).not.toHaveBeenCalled(); }); + it("does nothing when the workflow does not compile", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.isWorkflowCompilable = false; + component.computingUnitStatus = ComputingUnitState.Running; + const executeSpy = vi.spyOn(executeWorkflowService, "executeWorkflowWithEmailNotification"); + + component.runWorkflow(); + + expect(executeSpy).not.toHaveBeenCalled(); + expect(component.computingUnitSelectionComponent.showAddComputeUnitModalVisible).not.toHaveBeenCalled(); + }); + it("does nothing when the workflow is empty", () => { component.isWorkflowValid = true; component.isWorkflowEmpty = true; @@ -548,50 +578,17 @@ describe("MenuComponent", () => { }); }); - describe("export as Python", () => { - it("shows the returned script in a modal", () => { - const toPython = TestBed.inject(WorkflowToPythonService); - vi.spyOn(toPython, "convertToPython").mockReturnValue( - of({ type: "success", pythonCode: "print('hello')" } as WorkflowToPythonResponse) - ); - const fakeModalRef = { afterClose: of(undefined) } as unknown as NzModalRef; - const createSpy = vi.spyOn(modalService, "create").mockReturnValue(fakeModalRef); + it("copyPythonCodeToClipboard writes the generated Python script to the clipboard", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + vi.stubGlobal("navigator", { clipboard: { writeText } }); + const successSpy = vi.spyOn(notificationService, "success").mockImplementation(() => {}); + component.pythonCodeForModal = "print('hello')"; - component.onClickExportAsPython(); + await component.copyPythonCodeToClipboard(); - expect(component.pythonCodeForModal).toBe("print('hello')"); - expect(createSpy).toHaveBeenCalledTimes(1); - // the button is re-enabled whichever way the request ends, so the user is not locked out of a second try - expect(component.isTranslatingToPython).toBe(false); - }); - - it("reports the reason the translation gave rather than opening an empty modal", () => { - const toPython = TestBed.inject(WorkflowToPythonService); - vi.spyOn(toPython, "convertToPython").mockReturnValue( - of({ type: "failure", errorMessage: "Sink is not supported" } as WorkflowToPythonResponse) - ); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const createSpy = vi.spyOn(modalService, "create"); - - component.onClickExportAsPython(); - - expect(errorSpy).toHaveBeenCalledWith("Sink is not supported"); - expect(createSpy).not.toHaveBeenCalled(); - expect(component.isTranslatingToPython).toBe(false); - }); - - it("copyPythonCodeToClipboard writes the generated Python script to the clipboard", async () => { - const writeText = vi.fn().mockResolvedValue(undefined); - vi.stubGlobal("navigator", { clipboard: { writeText } }); - const successSpy = vi.spyOn(notificationService, "success").mockImplementation(() => {}); - component.pythonCodeForModal = "print('hello')"; - - await component.copyPythonCodeToClipboard(); - - expect(writeText).toHaveBeenCalledWith("print('hello')"); - expect(successSpy).toHaveBeenCalledWith("Python script copied to clipboard"); - vi.unstubAllGlobals(); - }); + expect(writeText).toHaveBeenCalledWith("print('hello')"); + expect(successSpy).toHaveBeenCalledWith("Python script copied to clipboard"); + vi.unstubAllGlobals(); }); describe("version history", () => { @@ -1480,6 +1477,27 @@ describe("MenuComponent", () => { } }); + it("re-applies the run button behavior on every compilation state event", () => { + component.isWorkflowValid = true; + component.isWorkflowEmpty = false; + component.computingUnitStatus = ComputingUnitState.Running; + component.executionState = ExecutionState.Uninitialized; + Object.defineProperty(component.workflowWebsocketService, "isConnected", { + get: () => true, + configurable: true, + }); + + compilationStream$.next(CompilationState.Failed); + expect(component.isWorkflowCompilable).toBe(false); + expect(component.runButtonText).toBe("Invalid Workflow"); + expect(component.runDisable).toBe(true); + + compilationStream$.next(CompilationState.Succeeded); + expect(component.isWorkflowCompilable).toBe(true); + expect(component.runButtonText).toBe("Run"); + expect(component.runDisable).toBe(false); + }); + it("deactivates the export button unless the feature is on and results exist", () => { const guiConfig = TestBed.inject(GuiConfigService); const results$ = component.workflowResultExportService.hasResultToExportOnAllOperators; diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index 173eb300369..77b3f65a37d 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -72,6 +72,8 @@ import { NzSwitchComponent } from "ng-zorro-antd/switch"; import { NzBadgeComponent } from "ng-zorro-antd/badge"; import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; +import { WorkflowCompilingService } from "../../service/compile-workflow/workflow-compiling.service"; +import { CompilationState } from "../../types/workflow-compiling.interface"; /** * MenuComponent is the top level menu bar that shows @@ -130,9 +132,9 @@ export class MenuComponent implements OnInit, OnDestroy { public ComputingUnitState = ComputingUnitState; // make Angular HTML access enum definition public isWorkflowValid: boolean = true; // this will check whether the workflow error or not public isWorkflowEmpty: boolean = false; - // the translation is a round trip to the compiling service, so the button stays disabled while one is in flight - public isTranslatingToPython: boolean = false; - public pythonCodeForModal: string = ""; + // whether the last compilation of the workflow failed. A workflow that cannot compile cannot be executed, + // so it is treated the same way as one that fails the schema / port validation. + public isWorkflowCompilable: boolean = true; public isSaving: boolean = false; public isWorkflowModifiable: boolean = false; public workflowId?: number; @@ -178,6 +180,7 @@ export class MenuComponent implements OnInit, OnDestroy { private location: Location, public undoRedoService: UndoRedoService, public validationWorkflowService: ValidationWorkflowService, + private workflowCompilingService: WorkflowCompilingService, public workflowPersistService: WorkflowPersistService, public workflowVersionService: WorkflowVersionService, public userService: UserService, @@ -242,6 +245,16 @@ export class MenuComponent implements OnInit, OnDestroy { this.applyRunButtonBehavior(this.getRunButtonBehavior()); }); + // the compilation errors are reported per operator on the canvas and in the result panel, but a workflow + // that cannot compile cannot be executed either, so the run button has to reflect the compilation state too + this.workflowCompilingService + .getCompilationStateInfoChangedStream() + .pipe(untilDestroyed(this)) + .subscribe(state => { + this.isWorkflowCompilable = state !== CompilationState.Failed; + this.applyRunButtonBehavior(this.getRunButtonBehavior()); + }); + // Subscribe to WorkflowResultExportService observable this.workflowResultExportService .getExportOnAllOperatorsStatusStream() @@ -362,8 +375,8 @@ export class MenuComponent implements OnInit, OnDestroy { disable: boolean; onClick: () => void; } { - // If workflow is invalid, always disable and show "Invalid Workflow" - if (!this.isWorkflowValid) { + // If workflow is invalid or does not compile, always disable and show "Invalid Workflow" + if (!this.isWorkflowValid || !this.isWorkflowCompilable) { return { text: "Invalid Workflow", icon: "warning", @@ -621,6 +634,9 @@ export class MenuComponent implements OnInit, OnDestroy { this.jupyterPanelService.openJupyterNotebookPanel(); } + public isTranslatingToPython = false; + public pythonCodeForModal = ""; + public onClickExportAsPython(): void { const logicalPlan = ExecuteWorkflowService.getLogicalPlanRequest( this.validationWorkflowService.getValidTexeraGraph() @@ -847,7 +863,7 @@ export class MenuComponent implements OnInit, OnDestroy { */ runWorkflow(): void { // Use the existing flags that were already updated via subscriptions - if (!this.isWorkflowValid || this.isWorkflowEmpty) { + if (!this.isWorkflowValid || !this.isWorkflowCompilable || this.isWorkflowEmpty) { return; } diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts index 2831a9943f4..d62b24c7b87 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts @@ -29,7 +29,7 @@ import { FORM_DEBOUNCE_TIME_MS } from "../../../service/execute-workflow/execute import { DatePipe } from "@angular/common"; import { By } from "@angular/platform-browser"; import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; -import { FormControl, FormGroup, FormsModule, ReactiveFormsModule } from "@angular/forms"; +import { AbstractControl, FormControl, FormGroup, FormsModule, ReactiveFormsModule } from "@angular/forms"; import { FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; import { TEXERA_FORMLY_CONFIG } from "../../../../common/formly/formly-config"; import { HttpClientTestingModule } from "@angular/common/http/testing"; @@ -1796,6 +1796,62 @@ describe("OperatorPropertyEditFrameComponent", () => { vi.spyOn(compiling, "getOperatorInputAttributeType").mockReturnValue("string"); expect(validator.expression({ value: { attr: "colA", mode: "loose" } } as any, rootField())).toBe(true); }); + + // A property that takes several columns holds a list of names, and the rule + // has to reach each of them rather than the list as a whole. + const multiColumnSchema: CustomJSONSchema7 = { + type: "object", + properties: { attrs: { type: "array", items: { type: "string" }, autofillAttributeOnPort: 0 } }, + attributeTypeRules: { attrs: { enum: ["integer"] } }, + }; + + it("enum rule passes when every column a multi-column property names matches", () => { + const validator = bindSchema(multiColumnSchema); + const spy = vi.spyOn(compiling, "getOperatorInputAttributeType").mockReturnValue("integer"); + + expect(validator.expression({ value: { attrs: ["colA", "colB"] } } as any, rootField())).toBe(true); + expect(spy).toHaveBeenCalledWith("attr-rules-op", 0, "colA"); + expect(spy).toHaveBeenCalledWith("attr-rules-op", 0, "colB"); + }); + + it("enum rule names the one column of a multi-column property that violates it", () => { + const validator = bindSchema(multiColumnSchema); + vi.spyOn(compiling, "getOperatorInputAttributeType").mockImplementation((_id, _port, name) => + name === "colA" ? "integer" : "string" + ); + const field = rootField(); + + expect(validator.expression({ value: { attrs: ["colA", "colB"] } } as any, field)).toBe(false); + expect((field as any).validators.checkAttributeType.message).toContain( + "The type of 'colB' is string, but it's expected to be integer" + ); + }); + + it("enum rule names the timestamp among columns a trainer cannot fit together", () => { + // The shape the sklearn advanced trainers are in: a timestamp fits on its own but + // raises DTypePromotionError beside any other column, so the accepted set leaves it + // out and the warning has to name the timestamp rather than the numeric column. + const validator = bindSchema({ + type: "object", + properties: { attrs: { type: "array", items: { type: "string" }, autofillAttributeOnPort: 0 } }, + attributeTypeRules: { attrs: { enum: ["integer", "long", "double", "boolean"] } }, + }); + vi.spyOn(compiling, "getOperatorInputAttributeType").mockImplementation((_id, _port, name) => + name === "when" ? "timestamp" : "double" + ); + const field = rootField(); + + expect(validator.expression({ value: { attrs: ["amount", "when"] } } as any, field)).toBe(false); + expect((field as any).validators.checkAttributeType.message).toContain("The type of 'when' is timestamp"); + }); + + it("enum rule is skipped when a multi-column property names nothing", () => { + const validator = bindSchema(multiColumnSchema); + const spy = vi.spyOn(compiling, "getOperatorInputAttributeType"); + + expect(validator.expression({ value: { attrs: [] } } as any, rootField())).toBe(true); + expect(spy).not.toHaveBeenCalled(); + }); }); // ────────────────────────────────────────────────────────────────────────── @@ -1820,6 +1876,36 @@ describe("OperatorPropertyEditFrameComponent", () => { ); }); + it("adds a uniqueAmongRows validator that rejects a value another row already holds", () => { + component.setFormlyFormBinding({ + type: "object", + properties: { + paraList: { + type: "array", + items: { + type: "object", + properties: { parameter: { type: "string", uniqueAmongRows: true } }, + }, + }, + }, + } as CustomJSONSchema7); + // A row's own fields exist only once formly is asked to build a row. + const arrayField = getField("paraList")!; + const rowField = (arrayField.fieldArray as (root: FormlyFieldConfig) => FormlyFieldConfig)(arrayField); + const validator = rowField.fieldGroup?.find(f => f.key === "parameter")?.validators?.["uniqueAmongRows"]; + expect(validator).toBeDefined(); + + const twoRowsSettingC = { + key: "parameter", + parent: { parent: { model: [{ parameter: "C" }, { parameter: "C" }] } }, + } as any; + expect(validator!.expression({ value: "C" } as any, twoRowsSettingC)).toBe(false); + expect(validator!.expression({ value: "kernel" } as any, twoRowsSettingC)).toBe(true); + expect(validator!.message(null, { formControl: { value: "C" } } as any)).toBe( + '"C" is already set by another row' + ); + }); + it("maps datasetVersionPath to the datasetversionselector field type", () => { component.setFormlyFormBinding({ type: "object", @@ -2224,6 +2310,66 @@ describe("OperatorPropertyEditFrameComponent", () => { setupPreview({ kind: "text", title: "T", pills: [] }); expect(realFixture.debugElement.query(By.css(".hf-task-preview-pills"))).toBeNull(); }); + + // Two rows holding one parameter mark each other, so the row that resolves the duplicate + // has to clear the row it left behind. Only a rendered form has the second row to clear. + describe("uniqueAmongRows across rendered rows", () => { + function renderTwoRows(first: string, second: string): void { + // A form the frame holds locked is disabled, and Angular does not validate a disabled control. + realComponent.interactive = true; + realComponent.setFormlyFormBinding({ + type: "object", + properties: { + paraList: { + type: "array", + items: { + type: "object", + properties: { parameter: { type: "string", uniqueAmongRows: true } }, + }, + }, + }, + } as CustomJSONSchema7); + realComponent.formData = { paraList: [{ parameter: first }, { parameter: second }] }; + realFixture.detectChanges(); + } + + function rowControl(index: number): AbstractControl { + return realComponent.formlyFormGroup!.get(["paraList", String(index), "parameter"])!; + } + + function typeIntoRow(index: number, parameter: string): void { + const input = realFixture.debugElement.queryAll(By.css("input"))[index].nativeElement as HTMLInputElement; + input.value = parameter; + input.dispatchEvent(new Event("input")); + realFixture.detectChanges(); + } + + it("marks both rows that name one parameter", () => { + renderTwoRows("C", "C"); + expect(realFixture.debugElement.queryAll(By.css("input")).length).toBe(2); + expect(rowControl(0).hasError("uniqueAmongRows")).toBe(true); + expect(rowControl(1).hasError("uniqueAmongRows")).toBe(true); + }); + + it("clears the row left behind when the other row picks a free parameter", () => { + renderTwoRows("C", "C"); + + typeIntoRow(1, "kernel"); + + expect(rowControl(0).hasError("uniqueAmongRows")).toBe(false); + expect(rowControl(1).hasError("uniqueAmongRows")).toBe(false); + }); + + it("marks the row already holding the parameter a row is changed onto", () => { + renderTwoRows("C", "kernel"); + expect(rowControl(0).hasError("uniqueAmongRows")).toBe(false); + + typeIntoRow(1, "C"); + + expect(rowControl(0).hasError("uniqueAmongRows")).toBe(true); + expect(rowControl(1).hasError("uniqueAmongRows")).toBe(true); + }); + }); }); describe("onFormChanges null handling", () => { diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts index b6379255192..14f8d64a53d 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts @@ -42,8 +42,10 @@ import { WorkflowCompilingService } from "../../../service/compile-workflow/work import { createOutputFormChangeEventStream, createShouldHideFieldFunc, + createValueRulesValidator, setChildTypeDependency, setHideExpression, + setValueRules, } from "src/app/common/formly/formly-utils"; import { TYPE_CASTING_OPERATOR_TYPE, @@ -899,6 +901,12 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On }; } + // a field whose accepted values follow a sibling's: give it the control those values + // call for, and hold it to them before the workflow can be run + if (isDefined(mapSource.valueRules)) { + setValueRules(mappedField, mapSource.valueRules); + } + // if the title is fileName, then change it to custom autocomplete input template if (mappedField.key === "fileName") { mappedField.type = "inputautocomplete"; @@ -1198,6 +1206,42 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On }; } + // A field the schema marks unique holds a meaning the enclosing list cannot repeat. + // uniqueItems cannot say this: two hyperparameter rows naming one parameter differ in + // their other fields, so they are distinct items while still emitting one keyword twice. + if (mapSource.uniqueAmongRows === true) { + mappedField.validators.uniqueAmongRows = { + expression: (control: AbstractControl, field: FormlyFieldConfig) => { + const rows = field.parent?.parent?.model; + const key = field.key; + if (!isDefined(control?.value) || !Array.isArray(rows) || typeof key !== "string") { + return true; + } + return rows.filter(row => isDefined(row) && row[key] === control.value).length <= 1; + }, + message: (error: any, field: FormlyFieldConfig) => + `"${field.formControl?.value}" is already set by another row`, + }; + // Whether a row repeats another is a property of the whole column, but Angular reruns a + // validator only on the control that changed. A change is answered by rechecking every + // row, so the row that resolves a duplicate clears the one it left behind, and a row + // changed onto a parameter another row holds marks that row too. + mappedField.hooks = { + ...mappedField.hooks, + onInit: (field: FormlyFieldConfig) => { + field.formControl?.valueChanges + .pipe(untilDestroyed(this)) + .subscribe(() => + field.parent?.parent?.fieldGroup?.forEach(row => + row.fieldGroup + ?.find(sibling => sibling.key === field.key) + ?.formControl?.updateValueAndValidity({ emitEvent: false }) + ) + ); + }, + }; + } + // Add custom validators for attribute type if (isDefined(mapSource.attributeTypeRules)) { mappedField.validators.checkAttributeType = { @@ -1212,7 +1256,18 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On return true; } - const findAttributeType = (propertyName: string): AttributeType | undefined => { + // A property that takes several columns holds a list of names rather + // than one, so both shapes are read as a list here and each name is + // then checked on its own. + const selectedAttributeNames = (propertyName: string): string[] => { + const value = control.value[propertyName]; + if (Array.isArray(value)) { + return value.filter(name => typeof name === "string"); + } + return typeof value === "string" ? [value] : []; + }; + + const findAttributeType = (propertyName: string, attributeName: string): AttributeType | undefined => { if ( !isDefined(this.currentOperatorId) || !isDefined(mapSource.properties) || @@ -1224,7 +1279,6 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On if (!isDefined(portIndex)) { return undefined; } - const attributeName: string = control.value[propertyName]; return this.workflowCompilingService.getOperatorInputAttributeType( this.currentOperatorId, portIndex, @@ -1246,14 +1300,17 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On if (!isDefined(data)) { return; } - const dataAttributeType = findAttributeType(data); + // Every rule written so far compares against a single-column + // property, so the first name is that property's whole value. + const dataAttributeName = selectedAttributeNames(data)[0]; + const dataAttributeType = isDefined(dataAttributeName) + ? findAttributeType(data, dataAttributeName) + : undefined; if (!isDefined(dataAttributeType)) { // if data attribute type is not defined, then data attribute is not yet selected. skip validation return; } if (inputAttributeType !== dataAttributeType) { - // get data attribute name for error message - const dataAttributeName = control.value[data]; throw TypeError(`it's expected to be the same type as '${dataAttributeName}' (${dataAttributeType}).`); } }; @@ -1294,21 +1351,30 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On // Get the type of constrains for each property in AttributeTypeRuleSchema const checkConstraint = (propertyName: string, constraint: AttributeTypeRuleSet) => { - const inputAttributeType = findAttributeType(propertyName); + for (const attributeName of selectedAttributeNames(propertyName)) { + const inputAttributeType = findAttributeType(propertyName, attributeName); - if (!isDefined(inputAttributeType)) { - // when inputAttributeType is undefined, it means the property is not set - return; - } - if (isDefined(constraint.enum)) { - checkEnumConstraint(inputAttributeType, constraint.enum); - } + if (!isDefined(inputAttributeType)) { + // when inputAttributeType is undefined, it means the property is not set + continue; + } + try { + if (isDefined(constraint.enum)) { + checkEnumConstraint(inputAttributeType, constraint.enum); + } - if (isDefined(constraint.const)) { - checkConstConstraint(inputAttributeType, constraint.const); - } - if (isDefined(constraint.allOf)) { - checkAllOfConstraint(inputAttributeType, constraint.allOf); + if (isDefined(constraint.const)) { + checkConstConstraint(inputAttributeType, constraint.const); + } + if (isDefined(constraint.allOf)) { + checkAllOfConstraint(inputAttributeType, constraint.allOf); + } + } catch (err) { + // The checks above describe the expectation, and only this loop + // knows which of several columns broke it. + // @ts-ignore + throw TypeError(`The type of '${attributeName}' is ${inputAttributeType}, but ${err.message}`); + } } }; @@ -1317,22 +1383,11 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On try { checkConstraint(prop, constraint); } catch (err) { - // have to get the type, attribute name and property name again - // should consider reusing the part in findAttributeType() - const attributeName = control.value[prop]; - const port = (mapSource.properties[prop] as CustomJSONSchema7).autofillAttributeOnPort as number; - const inputAttributeType = this.workflowCompilingService.getOperatorInputAttributeType( - this.currentOperatorId, - port, - attributeName - ); - // @ts-ignore - const message = err.message; if (field.validators === undefined) { field.validators = {}; } - field.validators.checkAttributeType.message = - `Warning: The type of '${attributeName}' is ${inputAttributeType}, but ` + message; + // @ts-ignore + field.validators.checkAttributeType.message = `Warning: ${err.message}`; return false; } } diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts index 2903abc1fb7..e180ae358c1 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts @@ -33,6 +33,7 @@ import { ReactiveFormsModule } from "@angular/forms"; import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { NzDropDownModule } from "ng-zorro-antd/dropdown"; import { ValidationWorkflowService } from "src/app/workspace/service/validation/validation-workflow.service"; +import { WorkflowCompilingService } from "src/app/workspace/service/compile-workflow/workflow-compiling.service"; import { NzModalModule, NzModalService } from "ng-zorro-antd/modal"; import { commonTestProviders } from "../../../../../common/testing/test-utils"; // Import NzModalModule and NzModalService import type { Mocked } from "vitest"; @@ -56,6 +57,7 @@ describe("ContextMenuComponent", () => { let operatorMenuService: Mocked; let jointGraphWrapperSpy: Mocked; let validationWorkflowService: Mocked; + let workflowCompilingService: Mocked; let highlightedOperatorsSubject: BehaviorSubject; let highlightedCommentBoxesSubject: BehaviorSubject; @@ -71,7 +73,12 @@ describe("ContextMenuComponent", () => { jointGraphWrapperSpy.getCurrentHighlightedCommentBoxIDs.mockReturnValue([]); jointGraphWrapperSpy.getCurrentHighlightedLinkIDs.mockReturnValue([]); - const texeraGraphSpy = { isOperatorDisabled: vi.fn(), hasLinkWithID: vi.fn(), bundleActions: vi.fn() }; + const texeraGraphSpy = { + isOperatorDisabled: vi.fn(), + hasLinkWithID: vi.fn(), + bundleActions: vi.fn(), + getSubDAG: vi.fn(), + }; const workflowActionServiceSpy = { getJointGraphWrapper: vi.fn(), @@ -92,6 +99,7 @@ describe("ContextMenuComponent", () => { // Set up TexeraGraph spy return values texeraGraphSpy.hasLinkWithID.mockReturnValue(false); + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [], links: [] }); texeraGraphSpy.bundleActions.mockImplementation((callback: Function) => callback()); const workflowResultServiceSpy = { getResultService: vi.fn(), hasAnyResult: vi.fn() }; @@ -121,6 +129,9 @@ describe("ContextMenuComponent", () => { const validationWorkflowServiceSpy = { validateOperator: vi.fn() }; + const workflowCompilingServiceSpy = { getWorkflowCompilationErrors: vi.fn() }; + workflowCompilingServiceSpy.getWorkflowCompilationErrors.mockReturnValue({}); + await TestBed.configureTestingModule({ providers: [ { provide: OperatorMetadataService, useClass: StubOperatorMetadataService }, @@ -129,6 +140,7 @@ describe("ContextMenuComponent", () => { { provide: WorkflowResultExportService, useValue: workflowResultExportServiceSpy }, { provide: OperatorMenuService, useValue: operatorMenuService }, { provide: ValidationWorkflowService, useValue: validationWorkflowServiceSpy }, + { provide: WorkflowCompilingService, useValue: workflowCompilingServiceSpy }, NzModalService, // Provide NzModalService ...commonTestProviders, ], @@ -151,6 +163,7 @@ describe("ContextMenuComponent", () => { validationWorkflowService = TestBed.inject( ValidationWorkflowService ) as unknown as Mocked; + workflowCompilingService = TestBed.inject(WorkflowCompilingService) as unknown as Mocked; fixture = TestBed.createComponent(ContextMenuComponent); component = fixture.componentInstance; @@ -263,6 +276,31 @@ describe("ContextMenuComponent", () => { expect(texeraGraphSpy.isOperatorDisabled).toHaveBeenCalledWith("op1"); }); + it("should return false when the target operator failed to compile", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [{ operatorID: "op1" }], links: [] } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ op1: {} as any }); + + expect(component.canExecuteOperator()).toBe(false); + expect(texeraGraphSpy.getSubDAG).toHaveBeenCalledWith("op1"); + }); + + it("should return false when an upstream operator failed to compile", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ + operators: [{ operatorID: "op1" }, { operatorID: "upstream" }], + links: [], + } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ upstream: {} as any }); + + expect(component.canExecuteOperator()).toBe(false); + }); + + it("should return true when the compilation error is outside the target's sub-DAG", () => { + texeraGraphSpy.getSubDAG.mockReturnValue({ operators: [{ operatorID: "op1" }], links: [] } as any); + workflowCompilingService.getWorkflowCompilationErrors.mockReturnValue({ unrelated: {} as any }); + + expect(component.canExecuteOperator()).toBe(true); + }); + it("should check disabled status only for valid operators", () => { // First test with invalid operator validationWorkflowService.validateOperator.mockReturnValue({ isValid: false, messages: {} }); diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts index 019f77f7afa..8f0f6321bca 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts @@ -26,6 +26,7 @@ import { WorkflowResultExportService } from "src/app/workspace/service/workflow- import { NzModalService } from "ng-zorro-antd/modal"; import { ResultExportationComponent } from "../../../result-exportation/result-exportation.component"; import { ValidationWorkflowService } from "src/app/workspace/service/validation/validation-workflow.service"; +import { WorkflowCompilingService } from "src/app/workspace/service/compile-workflow/workflow-compiling.service"; import { GuiConfigService } from "../../../../../common/service/gui-config.service"; import { NzMenuDirective, NzMenuItemComponent } from "ng-zorro-antd/menu"; import { NgIf } from "@angular/common"; @@ -51,7 +52,8 @@ export class ContextMenuComponent { protected config: GuiConfigService, private workflowResultService: WorkflowResultService, private modalService: NzModalService, - private validationWorkflowService: ValidationWorkflowService + private validationWorkflowService: ValidationWorkflowService, + private workflowCompilingService: WorkflowCompilingService ) { this.registerWorkflowModifiableChangedHandler(); this.operatorMenuService.highlightedOperators$ @@ -82,10 +84,23 @@ export class ContextMenuComponent { private isOperatorExecutable(operatorID: string): boolean { return ( this.validationWorkflowService.validateOperator(operatorID).isValid && - !this.workflowActionService.getTexeraGraph().isOperatorDisabled(operatorID) + !this.workflowActionService.getTexeraGraph().isOperatorDisabled(operatorID) && + this.isSubDAGCompilable(operatorID) ); } + /** + * Executing to an operator runs the operator together with everything upstream of it, so the entry has to be + * disabled when any operator in that sub-DAG failed to compile, not only when the target operator itself did. + */ + private isSubDAGCompilable(operatorID: string): boolean { + const compilationErrors = this.workflowCompilingService.getWorkflowCompilationErrors(); + return this.workflowActionService + .getTexeraGraph() + .getSubDAG(operatorID) + .operators.every(operator => !(operator.operatorID in compilationErrors)); + } + public hasHighlightedLinks(): boolean { return this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedLinkIDs().length > 0; } diff --git a/frontend/src/app/workspace/types/custom-json-schema.interface.ts b/frontend/src/app/workspace/types/custom-json-schema.interface.ts index 50edb681618..c89ac1be64c 100644 --- a/frontend/src/app/workspace/types/custom-json-schema.interface.ts +++ b/frontend/src/app/workspace/types/custom-json-schema.interface.ts @@ -46,6 +46,37 @@ export type AttributeTypeRuleSchema = Readonly<{ [key: string]: AttributeTypeRuleSet; }>; +/** + * What one field may hold, given what a sibling holds. Borrows `attributeTypeRules`' grammar + * and, like it, sits under a key of Texera's own rather than as a JSON-Schema `allOf`: the + * form builder merges the members of an `allOf` into a single field, which would leave one + * control carrying every branch's constraints at once. + */ +export type ValueRuleSet = Readonly<{ + allOf: ReadonlyArray<{ + if: { + [siblingField: string]: { + valEnum?: string[]; + }; + }; + then: { + // the accepted set, where the value is chosen from one + enum?: ReadonlyArray; + // otherwise how the value is read, in JSON Schema's names, with the bound the estimator + // puts on it where it has one + type?: "number" | "integer"; + minimum?: number; + exclusiveMinimum?: number; + // or, where the value is a choice between a set and a number and no type names it, + // the shape it takes + pattern?: string; + // a value that is accepted, for a reader that has to supply one; the form does not + // render it, the same as everywhere else `examples` is declared + examples?: ReadonlyArray; + }; + }>; +}>; + export interface CustomJSONSchema7 extends JSONSchema7 { propertyOrder?: number; properties?: { @@ -57,6 +88,7 @@ export interface CustomJSONSchema7 extends JSONSchema7 { autofill?: "attributeName" | "attributeNameList"; autofillAttributeOnPort?: number; attributeTypeRules?: AttributeTypeRuleSchema; + valueRules?: ValueRuleSet; "enable-presets"?: boolean; // include property in schema of preset @@ -69,4 +101,7 @@ export interface CustomJSONSchema7 extends JSONSchema7 { hideOnNull?: boolean; additionalEnumValue?: string; + + // no two rows of the enclosing list may hold the same value for this field + uniqueAmongRows?: boolean; }