Ask an upload row which source it is on, then show only that branch
The row offered both ways of giving a file at once and greyed out the one not in use, which still leaves the reader working out which half is live. It reads better as what it is: one choice, then the fields that choice needs - an input name, or which global input holds the file. Greying was all the schema could express, so this adds the annotation for showing a field only in the state it belongs to. The distinction earns the second annotation: a field that still tells the reader something while unavailable should stay and grey, but the branch nobody picked is not unavailable, it is irrelevant. A row saved before the choice existed says which branch it is on by what it carries, so it is stamped on read rather than left reading as the default and demanding an input name it never had. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
fed7141c06
commit
03db2dee5c
|
|
@ -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<Class<?>, Map<String, Structural>> structuralMap = collectStructuralMetadata(type);
|
||||
Map<Class<?>, Map<String, UiOptionalGroup>> uiOptionalGroupMap = collectUiOptionalGroupMetadata(type);
|
||||
Map<Class<?>, Map<String, UiEnabledWhen>> uiEnabledWhenMap = collectUiEnabledWhenMetadata(type);
|
||||
Map<Class<?>, Map<String, UiVisibleWhen>> uiVisibleWhenMap = collectUiVisibleWhenMetadata(type);
|
||||
Map<Class<?>, Map<String, UiOrder>> uiOrderMap = collectUiOrderMetadata(type);
|
||||
Map<Class<?>, Map<String, UiOptionsFromNode>> uiOptionsFromNodeMap = collectUiOptionsFromNodeMetadata(type);
|
||||
Map<Class<?>, Map<String, UiRequiredWhen>> 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<Class<?>, Map<String, UiVisibleWhen>> collectUiVisibleWhenMetadata(Class<?> rootClass) {
|
||||
Map<Class<?>, Map<String, UiVisibleWhen>> result = new HashMap<>();
|
||||
Set<Class<?>> visited = new HashSet<>();
|
||||
Queue<Class<?>> queue = new ArrayDeque<>();
|
||||
queue.add(rootClass);
|
||||
|
||||
while (!queue.isEmpty()) {
|
||||
Class<?> current = queue.poll();
|
||||
if (current == null || !visited.add(current) || isTerminalType(current)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
Map<String, UiVisibleWhen> 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<String, UiVisibleWhen> metadata) {
|
||||
if (metadata == null || metadata.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
JsonNode propertiesNode = classSchema.get("properties");
|
||||
if (!(propertiesNode instanceof ObjectNode properties)) {
|
||||
return;
|
||||
}
|
||||
for (Entry<String, UiVisibleWhen> 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<Class<?>, Map<String, UiEnabledWhen>> collectUiEnabledWhenMetadata(Class<?> rootClass) {
|
||||
Map<Class<?>, Map<String, UiEnabledWhen>> result = new HashMap<>();
|
||||
Set<Class<?>> 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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 "";
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
*
|
||||
* <p>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 {};
|
||||
}
|
||||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue