diff --git a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/JsonSchemaProducer.java b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/JsonSchemaProducer.java index bdc51cd..3ee8e1b 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/JsonSchemaProducer.java +++ b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/JsonSchemaProducer.java @@ -45,6 +45,7 @@ import it.cnr.isti.workflow.manager.configurations.annotations.SchemaAllowedValu import it.cnr.isti.workflow.manager.configurations.annotations.UiDescription; import it.cnr.isti.workflow.manager.configurations.annotations.UiRequiredWhen; import it.cnr.isti.workflow.manager.configurations.annotations.UiEnabledWhen; +import it.cnr.isti.workflow.manager.configurations.annotations.UiVisibleWhen; import it.cnr.isti.workflow.manager.configurations.annotations.UiLabel; import it.cnr.isti.workflow.manager.configurations.annotations.UiOrder; import it.cnr.isti.workflow.manager.configurations.annotations.UiOptionsFromNode; @@ -99,6 +100,7 @@ public class JsonSchemaProducer { Map, Map> structuralMap = collectStructuralMetadata(type); Map, Map> uiOptionalGroupMap = collectUiOptionalGroupMetadata(type); Map, Map> uiEnabledWhenMap = collectUiEnabledWhenMetadata(type); + Map, Map> uiVisibleWhenMap = collectUiVisibleWhenMetadata(type); Map, Map> uiOrderMap = collectUiOrderMetadata(type); Map, Map> uiOptionsFromNodeMap = collectUiOptionsFromNodeMetadata(type); Map, Map> uiRequiredWhenMap = collectUiRequiredWhenMetadata(type); @@ -116,6 +118,7 @@ public class JsonSchemaProducer { applyStructuralMetadata(root, getMergedMetadata(structuralMap, type)); applyUiOptionalGroupMetadata(root, getMergedMetadata(uiOptionalGroupMap, type)); applyUiEnabledWhenMetadata(root, getMergedMetadata(uiEnabledWhenMap, type)); + applyUiVisibleWhenMetadata(root, getMergedMetadata(uiVisibleWhenMap, type)); applyUiOrderMetadata(root, type, getMergedMetadata(uiOrderMap, type)); applyUiOptionsFromNodeMetadata(root, getMergedMetadata(uiOptionsFromNodeMap, type)); applyUiRequiredWhenMetadata(root, getMergedMetadata(uiRequiredWhenMap, type)); @@ -162,6 +165,7 @@ public class JsonSchemaProducer { applyStructuralMetadata(classSchema, getMergedMetadata(structuralMap, matchedClass)); applyUiOptionalGroupMetadata(classSchema, getMergedMetadata(uiOptionalGroupMap, matchedClass)); applyUiEnabledWhenMetadata(classSchema, getMergedMetadata(uiEnabledWhenMap, matchedClass)); + applyUiVisibleWhenMetadata(classSchema, getMergedMetadata(uiVisibleWhenMap, matchedClass)); applyUiOrderMetadata(classSchema, matchedClass, getMergedMetadata(uiOrderMap, matchedClass)); applyUiOptionsFromNodeMetadata(classSchema, getMergedMetadata(uiOptionsFromNodeMap, matchedClass)); applyUiRequiredWhenMetadata(classSchema, getMergedMetadata(uiRequiredWhenMap, matchedClass)); @@ -1311,6 +1315,73 @@ public class JsonSchemaProducer { } } + private Map, Map> collectUiVisibleWhenMetadata(Class rootClass) { + Map, Map> result = new HashMap<>(); + Set> visited = new HashSet<>(); + Queue> queue = new ArrayDeque<>(); + queue.add(rootClass); + + while (!queue.isEmpty()) { + Class current = queue.poll(); + if (current == null || !visited.add(current) || isTerminalType(current)) { + continue; + } + + Map metadata = new LinkedHashMap<>(); + for (Field field : current.getDeclaredFields()) { + UiVisibleWhen annotation = field.getAnnotation(UiVisibleWhen.class); + if (annotation != null) { + metadata.put(field.getName(), annotation); + } + enqueueRelatedTypes(queue, field.getGenericType(), field.getType()); + } + + if (current.isRecord()) { + for (RecordComponent component : current.getRecordComponents()) { + UiVisibleWhen annotation = component.getAnnotation(UiVisibleWhen.class); + if (annotation != null) { + metadata.put(component.getName(), annotation); + } + enqueueRelatedTypes(queue, component.getGenericType(), component.getType()); + } + } + + if (!metadata.isEmpty()) { + result.put(current, metadata); + } + } + + return result; + } + + private void applyUiVisibleWhenMetadata(ObjectNode classSchema, Map metadata) { + if (metadata == null || metadata.isEmpty()) { + return; + } + JsonNode propertiesNode = classSchema.get("properties"); + if (!(propertiesNode instanceof ObjectNode properties)) { + return; + } + for (Entry entry : metadata.entrySet()) { + JsonNode propNode = properties.get(entry.getKey()); + if (!(propNode instanceof ObjectNode propertySchema)) { + continue; + } + UiVisibleWhen dependency = entry.getValue(); + ObjectNode visibleWhen = propertySchema.putObject("x-ui-visible-when"); + visibleWhen.put("field", dependency.field()); + if (!dependency.equals().isBlank()) { + visibleWhen.put("equals", dependency.equals()); + } + if (dependency.equalsAny().length > 0) { + ArrayNode equalsAny = visibleWhen.putArray("in"); + for (String value : dependency.equalsAny()) { + equalsAny.add(value); + } + } + } + } + private Map, Map> collectUiEnabledWhenMetadata(Class rootClass) { Map, Map> result = new HashMap<>(); Set> visited = new HashSet<>(); @@ -1466,8 +1537,6 @@ public class JsonSchemaProducer { } if (dependency.present()) { enabledWhen.put("present", true); - } else if (dependency.absent()) { - enabledWhen.put("present", false); } } } @@ -1527,8 +1596,6 @@ public class JsonSchemaProducer { } if (dependency.present()) { requiredWhen.put("present", true); - } else if (dependency.absent()) { - requiredWhen.put("present", false); } } } diff --git a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInput.java b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInput.java index 00894d6..94d3618 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInput.java +++ b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInput.java @@ -8,30 +8,37 @@ import it.cnr.isti.workflow.manager.configurations.annotations.FieldRetriever; import it.cnr.isti.workflow.manager.configurations.annotations.SchemaAllowedValues; import it.cnr.isti.workflow.manager.configurations.annotations.UiContextKeys; import it.cnr.isti.workflow.manager.configurations.annotations.UiDescription; -import it.cnr.isti.workflow.manager.configurations.annotations.UiEnabledWhen; import it.cnr.isti.workflow.manager.configurations.annotations.UiLabel; import it.cnr.isti.workflow.manager.configurations.annotations.UiRequiredWhen; +import it.cnr.isti.workflow.manager.configurations.annotations.UiVisibleWhen; import jakarta.validation.constraints.Size; /** * Declares a file that is forwarded with an MCP agent query. * *

Where the file comes from is the one thing this has to say, and the two ways are alternatives: - * a port of its own, named by {@link #name}, or a global input of the flow, named by - * {@link #globalInput}. Filling either one greys out the other, so the row states one source rather - * than leaving both on offer and the reader guessing which wins. + * a port of its own, uploaded to the step, or a global input of the flow. {@link #source} says + * which, and only that branch is shown - the fields of the other one are not unavailable, they are + * irrelevant, and leaving them on screen asks the reader to work out which half of the row is live. */ public record MCPAgentUploadInput( /** - * The port this file is uploaded to, which is why it is only meaningful without a global: - * it is the input's name, and an attachment taken from a global has no port to name. + * Which of the two ways this file arrives. The default lives in {@link #effectiveSource()} + * and is declared here too, so the editor can say which one an untouched row is using rather + * than only that it is using one. */ + @SchemaAllowedValues(value = { SOURCE_INPUT, SOURCE_GLOBAL }, defaultValue = SOURCE_INPUT) + @UiLabel("File comes from") + @UiDescription("Uploaded to an input of this step, or taken from a global input of the flow.") + @JsonProperty(required = false) String source, + + /** The port this file is uploaded to: the input's name. */ @Size(max = 64) @UiLabel("Input name") @UiDescription("Name of the input this file is uploaded to.") - @UiEnabledWhen(field = "globalInput", absent = true) - @UiRequiredWhen(field = "globalInput", absent = true) + @UiVisibleWhen(field = "source", equalsAny = { SOURCE_INPUT, "" }) + @UiRequiredWhen(field = "source", equalsAny = { SOURCE_INPUT, "" }) @JsonProperty(required = false) String name, /** @@ -44,47 +51,67 @@ public record MCPAgentUploadInput( @SchemaAllowedValues({ "IMAGE", "DOCUMENT" }) @UiLabel("Expected kind") @UiDescription("Filters the file picker. What the file is taken to be is read from the file itself.") - @UiEnabledWhen(field = "globalInput", absent = true) + @UiVisibleWhen(field = "source", equalsAny = { SOURCE_INPUT, "" }) @JsonProperty(required = false) MCPAgentUploadKind kind, @UiLabel("Several files") - @UiEnabledWhen(field = "globalInput", absent = true) + @UiVisibleWhen(field = "source", equalsAny = { SOURCE_INPUT, "" }) @JsonProperty(required = false) Boolean multiple, /** - * The flow global input this attachment is taken from, instead of a port of its own. + * The flow global input this attachment is taken from. * - *

Left blank, the block grows an input port and the file is uploaded to that step. Named, - * the file is whatever the run's global input holds - so one document uploaded once at the + *

The file is whatever the run's global input holds, so one document uploaded once at the * start of a run can be attached by every agent that needs it, rather than being uploaded * again per step. A global input reaches a block only by being named like this; it never - * feeds a port. + * feeds a port, which is why this branch grows no input on the block. */ - @UiLabel("From global input") - @UiDescription("Take the file from a global input of the flow instead of uploading it to this step.") - @UiEnabledWhen(field = "name", absent = true) + @UiLabel("Global input") + @UiDescription("Which global input of the flow holds the file.") + @UiVisibleWhen(field = "source", equals = SOURCE_GLOBAL) + @UiRequiredWhen(field = "source", equals = SOURCE_GLOBAL) @FieldRetriever(name = "GlobalInputs", url = "/secure-retriever/GlobalInputs/inputs/items?type=FILE", dependsOn = { UiContextKeys.FLOW_ID }, requiresAuth = true) @JsonProperty(required = false) String globalInput) { + public static final String SOURCE_INPUT = "INPUT"; + public static final String SOURCE_GLOBAL = "GLOBAL"; + /** * Absent means single, which is what a half-filled row means while it is being configured. The * component was a primitive, so the editor posting a row before that box had been touched failed * to deserialize at all, and the block update came back as a bad request naming "multiple" - * with nothing in the editor to connect that to the row being filled in. + * + *

A row written before the choice existed says which branch it is on by what it carries, so + * it is stamped here rather than left reading as the default and demanding an input name it + * never had. */ public MCPAgentUploadInput { multiple = multiple != null && multiple; + if ((source == null || source.isBlank()) && globalInput != null && !globalInput.isBlank()) { + source = SOURCE_GLOBAL; + } } public MCPAgentUploadInput(String name, MCPAgentUploadKind kind, boolean multiple) { - this(name, kind, multiple, null); + this(null, name, kind, multiple, null); + } + + public MCPAgentUploadInput(String name, MCPAgentUploadKind kind, boolean multiple, String globalInput) { + this(null, name, kind, multiple, globalInput); + } + + @JsonIgnore + public String effectiveSource() { + return source == null || source.isBlank() ? SOURCE_INPUT : source; } /** Whether this attachment comes from a global input rather than a port of its own. */ @JsonIgnore public boolean isFromGlobalInput() { - return globalInput != null && !globalInput.isBlank(); + return SOURCE_GLOBAL.equalsIgnoreCase(effectiveSource()) + && globalInput != null && !globalInput.isBlank(); } /** diff --git a/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiEnabledWhen.java b/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiEnabledWhen.java index 3598ad2..05f7e53 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiEnabledWhen.java +++ b/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiEnabledWhen.java @@ -16,14 +16,6 @@ public @interface UiEnabledWhen { boolean present() default false; - /** - * Enabled only while the referenced field is empty, which is how two fields that are - * alternatives to each other say so: filling either one greys out the other. Distinct from - * {@link #present()} because a primitive boolean cannot tell "not specified" from "false", so - * the absent case needed a flag of its own. - */ - boolean absent() default false; - String group() default ""; } diff --git a/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiRequiredWhen.java b/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiRequiredWhen.java index 50b7181..3685d4e 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiRequiredWhen.java +++ b/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiRequiredWhen.java @@ -16,7 +16,4 @@ public @interface UiRequiredWhen { boolean present() default false; - /** Required only while the referenced field is empty. See UiEnabledWhen#absent. */ - boolean absent() default false; - } diff --git a/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiVisibleWhen.java b/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiVisibleWhen.java new file mode 100644 index 0000000..cc0344f --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiVisibleWhen.java @@ -0,0 +1,25 @@ +package it.cnr.isti.workflow.manager.configurations.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Shows a field only in the state it belongs to, rather than greying it out there. + * + *

The difference is worth an annotation of its own. {@link UiEnabledWhen} suits a field that + * still tells the reader something while it is unavailable - a name that will be used once the mode + * changes. It suits a branch of a choice badly: the fields of the branch nobody picked are not + * unavailable, they are irrelevant, and leaving them on screen greyed asks the reader to work out + * which half of the form is the live one. + */ +@Target({ ElementType.FIELD, ElementType.RECORD_COMPONENT }) +@Retention(RetentionPolicy.RUNTIME) +public @interface UiVisibleWhen { + String field(); + + String equals() default ""; + + String[] equalsAny() default {}; +} diff --git a/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadGlobalSchemaTest.java b/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadGlobalSchemaTest.java index 1ba07ef..2be7e7b 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadGlobalSchemaTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadGlobalSchemaTest.java @@ -2,7 +2,6 @@ package it.cnr.isti.workflow.manager.blocks.configurations; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -80,23 +79,26 @@ public class MCPAgentUploadGlobalSchemaTest { } @Test - public void theTwoWaysToGiveAFileAreOfferedAsAlternatives() { - // Filling either greys out the other, so the row states one source instead of leaving both - // on offer with nothing saying which one wins. + public void theRowAsksWhichOfTheTwoSourcesItIsOn() { + // One choice up front, then only that branch: the other branch's fields are not unavailable, + // they are irrelevant. JsonNode upload = schemaProducer.generateSchemaNode(MCPAgentBlockConfiguration.class) .get("definitions").get("MCPAgentUploadInput").get("properties"); - assertEquals("globalInput", upload.get("name").get("x-ui-enabled-when").get("field").asString()); - assertFalse(upload.get("name").get("x-ui-enabled-when").get("present").asBoolean()); - assertEquals("globalInput", upload.get("kind").get("x-ui-enabled-when").get("field").asString()); - assertEquals("globalInput", upload.get("multiple").get("x-ui-enabled-when").get("field").asString()); + assertEquals("INPUT", upload.get("source").get("default").asString()); - assertEquals("name", upload.get("globalInput").get("x-ui-enabled-when").get("field").asString()); - assertFalse(upload.get("globalInput").get("x-ui-enabled-when").get("present").asBoolean()); + for (String branchField : new String[] { "name", "kind", "multiple" }) { + JsonNode rule = upload.get(branchField).get("x-ui-visible-when"); + assertEquals("source", rule.get("field").asString(), branchField); + assertEquals("INPUT", rule.get("in").get(0).asString(), branchField); + } - // And a port with no name is the one thing a row cannot be saved as, so it is required only - // while no global is chosen. - assertEquals("globalInput", upload.get("name").get("x-ui-required-when").get("field").asString()); - assertFalse(upload.get("name").get("x-ui-required-when").get("present").asBoolean()); + JsonNode globalRule = upload.get("globalInput").get("x-ui-visible-when"); + assertEquals("source", globalRule.get("field").asString()); + assertEquals("GLOBAL", globalRule.get("equals").asString()); + + // Each branch asks for the one thing it cannot do without, and only on its own branch. + assertEquals("source", upload.get("name").get("x-ui-required-when").get("field").asString()); + assertEquals("GLOBAL", upload.get("globalInput").get("x-ui-required-when").get("equals").asString()); } } diff --git a/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInputJsonTest.java b/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInputJsonTest.java index 3904d29..78da801 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInputJsonTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/MCPAgentUploadInputJsonTest.java @@ -94,4 +94,24 @@ public class MCPAgentUploadInputJsonTest { assertEquals(MCPAgentUploadInput.MCPAgentUploadKind.DOCUMENT, upload.kind()); } + + @Test + public void stampsARowWrittenBeforeTheChoiceExisted() { + // Saved flows carry rows with no source at all. What such a row holds says which branch it + // is on, so it must not read as the default and demand an input name it never had. + MCPAgentUploadInput upload = mapper.readValue( + "{\"globalInput\":\"document\"}", MCPAgentUploadInput.class); + + assertEquals(MCPAgentUploadInput.SOURCE_GLOBAL, upload.source()); + assertTrue(upload.isFromGlobalInput()); + } + + @Test + public void aRowWithNoSourceAndNoGlobalIsAnInputRow() { + MCPAgentUploadInput upload = mapper.readValue( + "{\"name\":\"planDoc\"}", MCPAgentUploadInput.class); + + assertEquals(MCPAgentUploadInput.SOURCE_INPUT, upload.effectiveSource()); + assertFalse(upload.isFromGlobalInput()); + } }