diff --git a/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.spec.ts b/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.spec.ts index 28f975540e1..610f9bbaa26 100644 --- a/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.spec.ts +++ b/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.spec.ts @@ -471,6 +471,57 @@ describe("WorkflowCompilingService.setOperatorInputAttrs / restoreOperatorInputA ).toEqual(["col_a", "col_b"]); }); + it("does not append an empty option for a required property inside a referenced definition", () => { + const schema = makeOperatorSchema({ + type: "object", + required: ["attributes"], + properties: { + attributes: { + type: "array", + items: { $ref: "#/definitions/SortCriteriaUnit" }, + }, + }, + definitions: { + SortCriteriaUnit: { + type: "object", + required: ["attribute"], + properties: { + attribute: { type: "string", autofill: "attributeName", autofillAttributeOnPort: 0 }, + }, + }, + }, + }); + + const result = WorkflowCompilingService.setOperatorInputAttrs(schema, inputPortSchemaMap); + const definition = (result.jsonSchema.definitions as any).SortCriteriaUnit; + expect(definition.properties.attribute.enum).toEqual(["col_a", "col_b"]); + }); + + it("retains an empty option for an optional nested property when the root requires the same name", () => { + const schema = makeOperatorSchema({ + type: "object", + required: ["attribute"], + properties: { + aggregations: { + type: "array", + items: { $ref: "#/definitions/Aggregation" }, + }, + }, + definitions: { + Aggregation: { + type: "object", + properties: { + attribute: { type: "string", autofill: "attributeName", autofillAttributeOnPort: 0 }, + }, + }, + }, + }); + + const result = WorkflowCompilingService.setOperatorInputAttrs(schema, inputPortSchemaMap); + const definition = (result.jsonSchema.definitions as any).Aggregation; + expect(definition.properties.attribute.enum).toEqual(["col_a", "col_b", ""]); + }); + it("includes additionalEnumValue before the optional empty option", () => { const schema = makeOperatorSchema({ type: "object", diff --git a/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.ts b/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.ts index dae47a0088b..dd8f6f6ba3c 100644 --- a/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.ts +++ b/frontend/src/app/workspace/service/compile-workflow/workflow-compiling.service.ts @@ -338,7 +338,11 @@ export class WorkflowCompilingService { let newJsonSchema = operatorSchema.jsonSchema; - const getAttrNames = (attrName: string, v: CustomJSONSchema7): string[] | undefined => { + const getAttrNames = ( + attrName: string, + v: CustomJSONSchema7, + ownerSchema: CustomJSONSchema7 + ): string[] | undefined => { const i = v.autofillAttributeOnPort; if (i === undefined || i === null || !Number.isInteger(i)) { return undefined; @@ -359,7 +363,7 @@ export class WorkflowCompilingService { // https://github.com/ajv-validator/ajv/issues/1471 // the null -> "" change is done by Ajv.validate() with useDefault set to true. // It is converted during the property editor form initialization and workflow validation, instead of during schema propagation. - if (!operatorSchema.jsonSchema.required?.includes(attrName)) { + if (!ownerSchema.required?.includes(attrName)) { if (v.default) { if (typeof v.default !== "string") { throw new Error("default value must be a string"); @@ -377,10 +381,10 @@ export class WorkflowCompilingService { newJsonSchema = DynamicSchemaService.mutateProperty( newJsonSchema, (k, v) => v.autofill === "attributeName", - (attrName, old) => ({ + (attrName, old, ownerSchema) => ({ ...old, type: "string", - enum: getAttrNames(attrName, old), + enum: getAttrNames(attrName, old, ownerSchema), uniqueItems: true, }) ); @@ -388,14 +392,14 @@ export class WorkflowCompilingService { newJsonSchema = DynamicSchemaService.mutateProperty( newJsonSchema, (k, v) => v.autofill === "attributeNameList", - (attrName, old) => ({ + (attrName, old, ownerSchema) => ({ ...old, type: "array", uniqueItems: true, items: { ...(old.items as CustomJSONSchema7), type: "string", - enum: getAttrNames(attrName, old), + enum: getAttrNames(attrName, old, ownerSchema), }, }) ); diff --git a/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.spec.ts b/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.spec.ts index 2bfde185429..50fc039dca2 100644 --- a/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.spec.ts +++ b/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.spec.ts @@ -190,6 +190,39 @@ describe("DynamicSchemaService.mutateProperty", () => { expect((nested.properties!.deepTarget as CustomJSONSchema7).description).toEqual("mutated"); }); + it("should pass the schema that owns a nested property to both callbacks", () => { + const original = { + type: "object", + required: ["root"], + properties: { + nested: { + type: "object", + required: ["deepTarget"], + properties: { + deepTarget: { type: "string" }, + }, + }, + }, + } as CustomJSONSchema7; + const matchSpy = vi.fn( + (propertyName: string, _: CustomJSONSchema7, ownerSchema: CustomJSONSchema7) => + propertyName === "deepTarget" && ownerSchema.required?.includes(propertyName) === true + ); + const mutationSpy = vi.fn( + (_: string, propertyValue: CustomJSONSchema7, ownerSchema: CustomJSONSchema7): CustomJSONSchema7 => ({ + ...propertyValue, + description: ownerSchema.required?.join(","), + }) + ); + + const result = DynamicSchemaService.mutateProperty(original, matchSpy, mutationSpy); + + const nested = result.properties!.nested as CustomJSONSchema7; + expect((nested.properties!.deepTarget as CustomJSONSchema7).description).toEqual("deepTarget"); + expect(matchSpy).toHaveBeenCalledWith("deepTarget", expect.any(Object), nested); + expect(mutationSpy).toHaveBeenCalledWith("deepTarget", expect.any(Object), nested); + }); + it("should recurse into definitions to find the matched property", () => { const original = { type: "object", diff --git a/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.ts b/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.ts index 9425de4a60f..b49f33b25e2 100644 --- a/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.ts +++ b/frontend/src/app/workspace/service/dynamic-schema/dynamic-schema.service.ts @@ -149,15 +149,19 @@ export class DynamicSchemaService { /** * Helper function to change a property in a json schema of an operator schema. * It recursively walks through the property field of a JSON schema, and tries to find the property name. - * Once it finds the property name, it invokes the mutationFunction to get the new property and replaces the old property. - * The mutationFunction optionally takes a input with current property of the propertyName and outputs the new mutated property. + * Once it finds the property name, it invokes the mutation function to get the new property and replaces the old property. + * Both callbacks receive the property's name and value plus the schema object that owns the property. * * Returns a new object containing the new json schema property. */ public static mutateProperty( jsonSchemaToChange: CustomJSONSchema7, - matchFunc: (propertyName: string, propertyValue: CustomJSONSchema7) => boolean, - mutationFunc: (propertyName: string, propertyValue: CustomJSONSchema7) => CustomJSONSchema7 + matchFunc: (propertyName: string, propertyValue: CustomJSONSchema7, ownerSchema: CustomJSONSchema7) => boolean, + mutationFunc: ( + propertyName: string, + propertyValue: CustomJSONSchema7, + ownerSchema: CustomJSONSchema7 + ) => CustomJSONSchema7 ): CustomJSONSchema7 { // recursively walks the JSON schema property tree to find the property name const mutatePropertyRecurse = (jsonSchema: JSONSchema7) => { @@ -171,8 +175,9 @@ export class DynamicSchemaService { if (typeof propertyValue === "boolean") { return; } - if (matchFunc(propertyName, propertyValue as CustomJSONSchema7)) { - objectProperty[propertyName] = mutationFunc(propertyName, propertyValue as CustomJSONSchema7); + const ownerSchema = jsonSchema as CustomJSONSchema7; + if (matchFunc(propertyName, propertyValue as CustomJSONSchema7, ownerSchema)) { + objectProperty[propertyName] = mutationFunc(propertyName, propertyValue as CustomJSONSchema7, ownerSchema); } else { mutatePropertyRecurse(propertyValue); }