diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts new file mode 100644 index 00000000000..52b9d19ed99 --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts @@ -0,0 +1,293 @@ +/** + * 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 { FormlyFieldConfig } from "@ngx-formly/core"; +import { FormlyJsonschema } from "@ngx-formly/core/json-schema"; +import { applySafeNumericParser, parseNumericInput } from "./numeric-input-parser.util"; + +describe("parseNumericInput", () => { + it("returns the number for a numeric value", () => { + expect(parseNumericInput(10)).toEqual(10); + expect(parseNumericInput(0)).toEqual(0); + expect(parseNumericInput(-5)).toEqual(-5); + }); + + it("parses numeric strings", () => { + expect(parseNumericInput("10")).toEqual(10); + expect(parseNumericInput("-5")).toEqual(-5); + expect(parseNumericInput("0")).toEqual(0); + expect(parseNumericInput("3.5")).toEqual(3.5); + }); + + it("treats empty / null / undefined as null", () => { + expect(parseNumericInput("")).toBeNull(); + expect(parseNumericInput(null)).toBeNull(); + expect(parseNumericInput(undefined)).toBeNull(); + }); + + it("returns null for non-numeric input instead of throwing", () => { + expect(parseNumericInput("abc")).toBeNull(); + expect(parseNumericInput("1a")).toBeNull(); + expect(parseNumericInput({})).toBeNull(); + }); + + it("preserves negative and zero values (does not clamp to 1)", () => { + // Guards the reported bug: a negative limit must pass through unchanged here, + // not be turned into 1 or dropped. + expect(parseNumericInput(-100)).toEqual(-100); + expect(parseNumericInput("-1")).toEqual(-1); + }); + + it("parses negative decimals and whitespace-padded numbers", () => { + expect(parseNumericInput("-3.5")).toEqual(-3.5); + expect(parseNumericInput(" 10 ")).toEqual(10); + }); + + it("treats whitespace-only strings as null", () => { + expect(parseNumericInput(" ")).toBeNull(); + expect(parseNumericInput(" ")).toBeNull(); + expect(parseNumericInput("\t")).toBeNull(); + }); + + it("rejects NaN and infinities as null", () => { + expect(parseNumericInput(NaN)).toBeNull(); + expect(parseNumericInput(Infinity)).toBeNull(); + expect(parseNumericInput(-Infinity)).toBeNull(); + expect(parseNumericInput("Infinity")).toBeNull(); + }); + + it("rejects non-number/non-string values (boolean, array, object) as null", () => { + expect(parseNumericInput(true)).toBeNull(); + expect(parseNumericInput(false)).toBeNull(); + expect(parseNumericInput([5])).toBeNull(); + expect(parseNumericInput([1, 2])).toBeNull(); + }); + + it("preserves large finite numbers", () => { + expect(parseNumericInput(1_000_000_000)).toEqual(1_000_000_000); + expect(parseNumericInput("2000000")).toEqual(2000000); + }); + + it("never touches the DOM (safe to call without an element)", () => { + // The whole point of the replacement: no document.querySelector / .validity access. + expect(() => parseNumericInput("42")).not.toThrow(); + expect(parseNumericInput("42")).toEqual(42); + }); +}); + +describe("applySafeNumericParser", () => { + it("replaces a numeric field's existing (crash-prone) parser with the safe one", () => { + // Mimics @ngx-formly's parser that throws on nz-input-number when the value is null. + const crashingParser = () => { + throw new TypeError("Cannot read properties of undefined (reading 'badInput')"); + }; + const field: FormlyFieldConfig = { type: "integer", parsers: [crashingParser] }; + + applySafeNumericParser(field); + + // the crash-prone parser is gone + expect(field.parsers).toHaveLength(1); + expect(field.parsers?.[0]).not.toBe(crashingParser); + // the null intermediate state that used to crash is now handled safely + expect(() => (field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)(null, field)).not.toThrow(); + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)(null, field)).toBeNull(); + // and it still converts real values + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("10", field)).toEqual(10); + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("-5", field)).toEqual(-5); + }); + + it("replaces the parser for type 'number' too (not just 'integer')", () => { + const field: FormlyFieldConfig = { + type: "number", + parsers: [ + () => { + throw new Error("original numeric parser should not run"); + }, + ], + }; + + applySafeNumericParser(field); + + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("3.5", field)).toEqual(3.5); + }); + + it("does NOT touch an enum (select) field's parser", () => { + // Attribute selectors are 'enum' fields; they carry a (string) parser but render as a + // select, not nz-input-number, so they must be left alone. + const enumParser = (v: unknown) => v; + const field: FormlyFieldConfig = { type: "enum", parsers: [enumParser] }; + + applySafeNumericParser(field); + + expect(field.parsers?.[0]).toBe(enumParser); + }); + + it("does NOT touch a string field's parser (regression: text inputs must keep working)", () => { + // FormlyJsonschema also sets a parser on string fields. Replacing it with the numeric + // parser would turn typed text into null and break every text property across all + // operators, so string fields must be left completely alone. + const stringParser = (v: unknown) => v; + const field: FormlyFieldConfig = { type: "string", parsers: [stringParser] }; + + applySafeNumericParser(field); + + expect(field.parsers).toHaveLength(1); + expect(field.parsers?.[0]).toBe(stringParser); // exact same parser, untouched + // typed text still passes through unchanged + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("hello", field)).toEqual("hello"); + }); + + it("leaves non-numeric fields (without parsers) untouched", () => { + const field: FormlyFieldConfig = { type: "string" }; + + applySafeNumericParser(field); + + expect(field.parsers).toBeUndefined(); + }); + + it("does not add parsers to a field that had none", () => { + const field: FormlyFieldConfig = { key: "someBoolean", type: "boolean" }; + + applySafeNumericParser(field); + + expect(field.parsers).toBeUndefined(); + }); + + it("the replacement parser treats the clear-field cases ('' and null) as null", () => { + const field: FormlyFieldConfig = { + type: "integer", + parsers: [ + () => { + throw new Error("original parser should not run"); + }, + ], + }; + + applySafeNumericParser(field); + const parser = field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown; + + expect(parser("", field)).toBeNull(); + expect(parser(null, field)).toBeNull(); + }); + + it("collapses multiple existing parsers into a single safe one", () => { + const field: FormlyFieldConfig = { type: "integer", parsers: [() => 1, () => 2] }; + + applySafeNumericParser(field); + + expect(field.parsers).toHaveLength(1); + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("7", field)).toEqual(7); + }); + + it("leaves an empty parsers array untouched (nothing to replace)", () => { + const field: FormlyFieldConfig = { type: "integer", parsers: [] }; + + applySafeNumericParser(field); + + expect(field.parsers).toEqual([]); + }); +}); + +describe("applySafeNumericParser with the real FormlyJsonschema (integration)", () => { + // Uses the actual @ngx-formly/core json-schema conversion (not hand-built fields) to + // prove, against real library behavior, that only numeric fields are affected. This is + // the exact concern behind the earlier "all operators can't input values" regression: + // FormlyJsonschema sets a parser on BOTH string and number fields, so the guard must + // touch only the numeric one. + const jsonSchema = new FormlyJsonschema(); + + it("replaces a real integer field's parser but leaves a real string field's parser intact", () => { + const stringField = jsonSchema.toFieldConfig({ type: "string" }); + const integerField = jsonSchema.toFieldConfig({ type: "integer" }); + + // premise: the library really does set parsers on both, and the types are as expected + expect(stringField.type).toEqual("string"); + expect(integerField.type).toEqual("integer"); + expect((stringField.parsers ?? []).length).toBeGreaterThan(0); + expect((integerField.parsers ?? []).length).toBeGreaterThan(0); + + const originalStringParser = stringField.parsers![0]; + + applySafeNumericParser(stringField); + applySafeNumericParser(integerField); + + // the string field is completely untouched → text still passes through unchanged, + // i.e. every text property on every operator keeps working + expect(stringField.parsers![0]).toBe(originalStringParser); + expect( + (stringField.parsers![0] as (v: unknown, f: FormlyFieldConfig) => unknown)("hello world", stringField) + ).toEqual("hello world"); + + // the integer field is switched to the safe numeric parser + expect((integerField.parsers![0] as (v: unknown, f: FormlyFieldConfig) => unknown)("10", integerField)).toEqual(10); + expect((integerField.parsers![0] as (v: unknown, f: FormlyFieldConfig) => unknown)(null, integerField)).toBeNull(); + }); + + it("leaves a real string field WITH an enum (attribute-selector style) intact", () => { + // An attribute selector is a string schema with an enum → the library makes it type + // 'enum'; its parser (set during the string phase) must not be swapped. + const enumField = jsonSchema.toFieldConfig({ type: "string", enum: ["a", "b", "c"] }); + expect(enumField.type).toEqual("enum"); + const originalParser = enumField.parsers?.[0]; + + applySafeNumericParser(enumField); + + expect(enumField.parsers?.[0]).toBe(originalParser); + }); + + it("invariant across every JSON-schema field type: only number/integer are changed", () => { + // Operators are just combinations of these property types, so proving the invariant + // per type proves it for all (hundreds of) operators: applySafeNumericParser must + // change field.parsers ONLY for number/integer, and leave the parsers reference + // byte-for-byte identical for every other type. + const schemas: Record = { + string: { type: "string" }, + "string+minLength": { type: "string", minLength: 2 }, + "string+pattern": { type: "string", pattern: "^a" }, + "string+enum": { type: "string", enum: ["a", "b"] }, + "nullable-string": { type: ["string", "null"] }, + boolean: { type: "boolean" }, + array: { type: "array", items: { type: "string" } }, + "array-of-numbers": { type: "array", items: { type: "integer" } }, + object: { type: "object", properties: { x: { type: "string" } } }, + const: { const: "fixed" }, + integer: { type: "integer" }, + number: { type: "number" }, + "nullable-integer": { type: ["integer", "null"] }, + }; + + for (const [name, schema] of Object.entries(schemas)) { + const field = jsonSchema.toFieldConfig(schema as never); + const parsersBefore = field.parsers; + const isNumeric = field.type === "number" || field.type === "integer"; + + applySafeNumericParser(field); + + if (isNumeric) { + // numeric fields get the safe parser (when the library gave them one) + if (parsersBefore && parsersBefore.length > 0) { + expect(field.parsers?.[0], `numeric type "${name}" should be replaced`).not.toBe(parsersBefore[0]); + } + } else { + // every non-numeric type keeps the exact same parsers reference (fully untouched) + expect(field.parsers, `non-numeric type "${name}" must be untouched`).toBe(parsersBefore); + } + } + }); +}); diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts new file mode 100644 index 00000000000..d7bfcc53503 --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts @@ -0,0 +1,64 @@ +/** + * 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 { FormlyFieldConfig } from "@ngx-formly/core"; + +/** + * Null-safe parser for numeric (integer/number) property fields. + * + * It replaces @ngx-formly/core's json-schema integer/number parser, which reads + * `document.querySelector('#' + field.id).validity.badInput`. That code assumes the + * field id is on the native ``, but ng-zorro's `nz-input-number` puts the id + * on its host element (which has no `.validity`), so the parser throws + * "Cannot read properties of undefined (reading 'badInput')" and typed edits never + * commit. This version does no DOM access: it just converts the value to a number, or + * null when it is empty/undefined/not a number. + */ +export function parseNumericInput(value: unknown): number | null { + if (typeof value === "number") { + // Reject NaN and ±Infinity — only real, finite numbers are valid. + return Number.isFinite(value) ? value : null; + } + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed === "") { + return null; + } + const parsed = Number(trimmed); + return Number.isFinite(parsed) ? parsed : null; + } + // null, undefined, boolean, object, array, etc. are not valid numeric input. + return null; +} + +/** + * Replace a numeric field's crash-prone Formly parser with {@link parseNumericInput}. + * + * FormlyJsonschema sets `parsers` on integer/number fields (the crash-prone one) but + * ALSO on string fields (a different, harmless one). Only the numeric parser causes the + * nz-input-number crash, so restrict the swap to number/integer fields — replacing a + * string field's parser with a numeric one would drop all typed text and break every + * text property. Leave every other field untouched. + */ +export function applySafeNumericParser(field: FormlyFieldConfig): void { + const isNumericField = field.type === "number" || field.type === "integer"; + if (isNumericField && field.parsers && field.parsers.length > 0) { + field.parsers = [value => parseNumericInput(value)]; + } +} 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 2512fecdacb..38b5bde66e6 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 @@ -74,6 +74,7 @@ import { WorkflowPveService } from "../../../service/virtual-environment/virtual import { ComputingUnitStatusService } from "../../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { of } from "rxjs"; import { map, switchMap, take } from "rxjs/operators"; +import { applySafeNumericParser } from "./numeric-input-parser.util"; Quill.register("modules/cursors", QuillCursors); @@ -488,6 +489,15 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On }, }; + // @ngx-formly/core's json-schema parser for integer/number fields reads + // document.querySelector('#' + field.id).validity.badInput, assuming the id is on + // the native . ng-zorro's nz-input-number puts the id on its host element + // (which has no `.validity`), so that parser throws + // "Cannot read properties of undefined (reading 'badInput')" and typed edits never + // commit — the field gets stuck (only the +/- steppers work). FormlyJsonschema only + // sets `parsers` for numeric fields, so replace it with a null-safe parser here. + applySafeNumericParser(mappedField); + // Disable dummy operator for user if (mappedField.key === "dummyOperator") { mappedField.expressions = {