diff --git a/app/common/src/main/java/stirling/software/common/config/swagger/ToolIOOperationCustomizer.java b/app/common/src/main/java/stirling/software/common/config/swagger/ToolIOOperationCustomizer.java index c7ea1b4f7c..b5c3a3ef87 100644 --- a/app/common/src/main/java/stirling/software/common/config/swagger/ToolIOOperationCustomizer.java +++ b/app/common/src/main/java/stirling/software/common/config/swagger/ToolIOOperationCustomizer.java @@ -1,5 +1,6 @@ package stirling.software.common.config.swagger; +import java.lang.reflect.Method; import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; @@ -18,6 +19,7 @@ import stirling.software.common.model.tool.ToolFormat; import stirling.software.common.model.tool.ToolIO; import stirling.software.common.model.tool.ToolIOCase; import stirling.software.common.model.tool.ToolIOWhen; +import stirling.software.common.service.ToolIOParameterDefaults; /** * Publishes each {@link ToolIO} into the spec as {@code x-stirling-io}, which is how the frontend @@ -49,40 +51,42 @@ public class ToolIOOperationCustomizer if (declaration == null) { return operation; } - operation.addExtension(EXTENSION_NAME, toExtension(declaration)); + operation.addExtension(EXTENSION_NAME, toExtension(declaration, handlerMethod.getMethod())); operation.setDescription(appendSummaryLine(operation.getDescription(), declaration)); return operation; } - private static Map toExtension(ToolIO declaration) { + private static Map toExtension(ToolIO declaration, Method handler) { Map extension = new LinkedHashMap<>(); extension.put("accepts", names(declaration.accepts())); extension.put("produces", declaration.produces().name()); extension.put("arity", declaration.arity().name()); if (declaration.cases().length > 0) { - extension.put("cases", cases(declaration)); + extension.put("cases", cases(declaration, handler)); } return extension; } - private static List> cases(ToolIO declaration) { - return Arrays.stream(declaration.cases()).map(ToolIOOperationCustomizer::toCase).toList(); + private static List> cases(ToolIO declaration, Method handler) { + return Arrays.stream(declaration.cases()).map(rule -> toCase(rule, handler)).toList(); } - private static Map toCase(ToolIOCase rule) { + private static Map toCase(ToolIOCase rule, Method handler) { Map entry = new LinkedHashMap<>(); - entry.put( - "when", - Arrays.stream(rule.when()).map(ToolIOOperationCustomizer::toCondition).toList()); + entry.put("when", Arrays.stream(rule.when()).map(c -> toCondition(c, handler)).toList()); entry.put("produces", rule.produces().name()); entry.put("arity", rule.arity().name()); return entry; } - private static Map toCondition(ToolIOWhen condition) { + private static Map toCondition(ToolIOWhen condition, Method handler) { Map entry = new LinkedHashMap<>(); entry.put("param", condition.param()); entry.put("matches", List.of(condition.matches())); + // The default the endpoint uses when this parameter is absent, so a step that never sends + // it still resolves. Omitted when the parameter is required with none. + ToolIOParameterDefaults.resolve(handler, condition.param()) + .ifPresent(value -> entry.put("default", value)); return entry; } diff --git a/app/common/src/main/java/stirling/software/common/model/tool/ToolIOSpec.java b/app/common/src/main/java/stirling/software/common/model/tool/ToolIOSpec.java index 146ae85133..cf627d97ef 100644 --- a/app/common/src/main/java/stirling/software/common/model/tool/ToolIOSpec.java +++ b/app/common/src/main/java/stirling/software/common/model/tool/ToolIOSpec.java @@ -5,13 +5,18 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Set; /** The runtime form of a {@link ToolIO} declaration, read off a handler method once at startup. */ public record ToolIOSpec( Set accepts, ToolFormat produces, ToolArity arity, List cases) { - public record When(String param, List matches) { + /** + * @param paramDefault the value used when the parameter is absent, or null when it has no + * default - an absent parameter then leaves the case unresolved rather than defaulted. + */ + public record When(String param, List matches, String paramDefault) { boolean holdsFor(Object value) { String normalised = normalise(value); @@ -19,6 +24,14 @@ public record ToolIOSpec( } } + /** Supplies the default value a request parameter takes when a caller omits it. */ + @FunctionalInterface + public interface ParameterDefaults { + Optional defaultFor(String param); + + ParameterDefaults NONE = param -> Optional.empty(); + } + /** * Both sides of a condition are normalised at comparison, not at construction: the declaration * reaches the frontend and the engine as published data, and normalising only one side there @@ -44,25 +57,33 @@ public record ToolIOSpec( } public static ToolIOSpec from(ToolIO annotation) { + return from(annotation, ParameterDefaults.NONE); + } + + public static ToolIOSpec from(ToolIO annotation, ParameterDefaults defaults) { return new ToolIOSpec( new LinkedHashSet<>(Arrays.asList(annotation.accepts())), annotation.produces(), annotation.arity(), - Arrays.stream(annotation.cases()).map(ToolIOSpec::toCase).toList()); + Arrays.stream(annotation.cases()).map(rule -> toCase(rule, defaults)).toList()); } - private static Case toCase(ToolIOCase rule) { - List when = Arrays.stream(rule.when()).map(ToolIOSpec::toWhen).toList(); + private static Case toCase(ToolIOCase rule, ParameterDefaults defaults) { + List when = Arrays.stream(rule.when()).map(c -> toWhen(c, defaults)).toList(); return new Case(when, rule.produces(), rule.arity()); } - private static When toWhen(ToolIOWhen condition) { - return new When(condition.param(), List.of(condition.matches())); + private static When toWhen(ToolIOWhen condition, ParameterDefaults defaults) { + return new When( + condition.param(), + List.of(condition.matches()), + defaults.defaultFor(condition.param()).orElse(null)); } /** - * First matching {@link Case} wins. If none match but one reads a parameter we cannot see, the - * declared output comes back uncertain: a value we never saw might have picked another branch. + * First matching {@link Case} wins. A parameter the caller omitted resolves to its declared + * default; only a parameter with no default leaves the output uncertain, since an unseen value + * might then have picked another branch. * * @param parameters the step's configured parameters, or null when not known */ @@ -71,12 +92,17 @@ public record ToolIOSpec( for (Case rule : cases) { boolean allHold = true; for (When condition : rule.when()) { - if (parameters == null || !parameters.containsKey(condition.param())) { + Object value; + if (parameters != null && parameters.containsKey(condition.param())) { + value = parameters.get(condition.param()); + } else if (condition.paramDefault() != null) { + value = condition.paramDefault(); + } else { sawUnknownParam = true; allHold = false; continue; } - allHold &= condition.holdsFor(parameters.get(condition.param())); + allHold &= condition.holdsFor(value); } if (allHold) { return new Output(rule.produces(), rule.arity(), true); diff --git a/app/common/src/main/java/stirling/software/common/service/ToolIOParameterDefaults.java b/app/common/src/main/java/stirling/software/common/service/ToolIOParameterDefaults.java new file mode 100644 index 0000000000..d77ca63de6 --- /dev/null +++ b/app/common/src/main/java/stirling/software/common/service/ToolIOParameterDefaults.java @@ -0,0 +1,92 @@ +package stirling.software.common.service; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.Optional; + +import io.swagger.v3.oas.annotations.media.Schema; + +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotNull; + +import lombok.extern.slf4j.Slf4j; + +/** + * The value a request parameter takes when the caller omits it, read from the request model so a + * {@code @ToolIOCase} can be resolved even for a step that never sends the parameter it branches + * on. The default is read from the field so it cannot drift. + */ +@Slf4j +public final class ToolIOParameterDefaults { + + // Swagger's sentinel for an unset @Schema string member; not a real default value. + private static final String SCHEMA_UNSET = "##default"; + + private ToolIOParameterDefaults() {} + + /** + * The default {@code param} resolves to when absent, or empty when the parameter is required + * with no declared default - in which case an unset value leaves the output genuinely unknown + * rather than defaulted, and the chain reports it as uncertain. + * + *

Precedence: an explicit {@code @Schema(defaultValue)}, then the field's own value (a + * primitive's language default, or an initializer), then the empty string for an optional field + * left null, and finally empty for a required field with none of the above. + */ + public static Optional resolve(Method handler, String param) { + for (Parameter parameter : handler.getParameters()) { + Field field = findField(parameter.getType(), param); + if (field != null) { + return fromField(parameter.getType(), field); + } + } + return Optional.empty(); + } + + private static Optional fromField(Class owner, Field field) { + Schema schema = field.getAnnotation(Schema.class); + if (schema != null + && !schema.defaultValue().isEmpty() + && !SCHEMA_UNSET.equals(schema.defaultValue())) { + return Optional.of(schema.defaultValue()); + } + Object value = readField(owner, field); + if (value != null) { + return Optional.of(String.valueOf(value)); + } + return isRequired(field, schema) ? Optional.empty() : Optional.of(""); + } + + private static Object readField(Class owner, Field field) { + try { + Object instance = owner.getDeclaredConstructor().newInstance(); + field.setAccessible(true); + return field.get(instance); + } catch (ReflectiveOperationException | RuntimeException e) { + // A request model we cannot instantiate leaves the default unknown, which the check + // treats conservatively as uncertain. Never break startup over it. + log.warn("Could not read default of {}.{}", owner.getSimpleName(), field.getName(), e); + return null; + } + } + + private static boolean isRequired(Field field, Schema schema) { + if (schema != null && schema.requiredMode() == Schema.RequiredMode.REQUIRED) { + return true; + } + return field.isAnnotationPresent(NotNull.class) + || field.isAnnotationPresent(NotBlank.class); + } + + private static Field findField(Class type, String name) { + for (Class c = type; c != null && c != Object.class; c = c.getSuperclass()) { + try { + return c.getDeclaredField(name); + } catch (NoSuchFieldException ignored) { + // Try the superclass; request models extend a shared file-input base. + } + } + return null; + } +} diff --git a/app/common/src/main/java/stirling/software/common/service/ToolIORegistry.java b/app/common/src/main/java/stirling/software/common/service/ToolIORegistry.java index 754bfb8019..c2219536d1 100644 --- a/app/common/src/main/java/stirling/software/common/service/ToolIORegistry.java +++ b/app/common/src/main/java/stirling/software/common/service/ToolIORegistry.java @@ -67,7 +67,10 @@ public class ToolIORegistry implements ToolMetadataService, ToolIOSource { if (annotation == null) { return; } - ToolIOSpec spec = ToolIOSpec.from(annotation); + Method method = handler.getMethod(); + ToolIOSpec spec = + ToolIOSpec.from( + annotation, param -> ToolIOParameterDefaults.resolve(method, param)); for (String pattern : extractPatterns(info)) { target.put(pattern, spec); } diff --git a/app/common/src/test/java/stirling/software/common/service/ToolChainValidatorConformanceTest.java b/app/common/src/test/java/stirling/software/common/service/ToolChainValidatorConformanceTest.java index 67ba8be612..78c15d751d 100644 --- a/app/common/src/test/java/stirling/software/common/service/ToolChainValidatorConformanceTest.java +++ b/app/common/src/test/java/stirling/software/common/service/ToolChainValidatorConformanceTest.java @@ -101,7 +101,12 @@ class ToolChainValidatorConformanceTest { for (JsonNode match : condition.get("matches")) { matches.add(match.asString()); } - when.add(new ToolIOSpec.When(condition.get("param").asString(), matches)); + JsonNode paramDefault = condition.get("default"); + when.add( + new ToolIOSpec.When( + condition.get("param").asString(), + matches, + paramDefault == null ? null : paramDefault.asString())); } cases.add( new ToolIOSpec.Case( diff --git a/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java b/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java index 120bee6a2b..f6fe52f2a6 100644 --- a/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java @@ -26,6 +26,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import stirling.software.common.model.tool.ToolFormat; import stirling.software.common.model.tool.ToolIO; import stirling.software.common.model.tool.ToolIOSpec; +import stirling.software.common.service.ToolIOParameterDefaults; /** * Every document-transforming endpoint must declare its I/O, or it becomes a hole in the @@ -233,6 +234,32 @@ class ToolIODeclarationCoverageTest { assertEquals(Set.of("ps", "pcl", "xps"), declared); } + @Test + void anAbsentParameterResolvesToItsRequestModelDefault() { + // A pipeline step often omits a parameter a case branches on. The default is read from the + // request model, so the output resolves anyway instead of coming back uncertain. + + // Auto Rotate never sends dryRun; its default (false) means the JSON branch cannot fire. + assertEquals( + ToolFormat.PDF, + spec("/api/v1/misc/auto-rotate-pdf").resolveOutput(Map.of()).format()); + assertTrue(spec("/api/v1/misc/auto-rotate-pdf").resolveOutput(Map.of()).certain()); + + // Change Permissions posts to add-password with no password fields; both default to blank, + // so the unencrypted branch fires and it is not mistaken for producing an encrypted PDF. + assertEquals( + ToolFormat.PDF, + spec("/api/v1/security/add-password").resolveOutput(Map.of()).format()); + assertTrue(spec("/api/v1/security/add-password").resolveOutput(Map.of()).certain()); + } + + @Test + void aRequiredParameterWithNoDefaultStaysUncertainWhenAbsent() { + // pdf/text branches on outputFormat, which is required with no default. Absent, its output + // is genuinely txt-or-rtf-dependent, so it must remain uncertain rather than assume TEXT. + assertFalse(spec("/api/v1/convert/pdf/text").resolveOutput(Map.of()).certain()); + } + @Test void onlyRemovePasswordAcceptsAnEncryptedDocument() { assertTrue( @@ -288,7 +315,11 @@ class ToolIODeclarationCoverageTest { } required.add(full); if (declaration != null) { - declared.put(full, ToolIOSpec.from(declaration)); + declared.put( + full, + ToolIOSpec.from( + declaration, + param -> ToolIOParameterDefaults.resolve(method, param))); } } } diff --git a/engine/scripts/generate_tool_models.py b/engine/scripts/generate_tool_models.py index 2a5139a263..7587d8c62f 100644 --- a/engine/scripts/generate_tool_models.py +++ b/engine/scripts/generate_tool_models.py @@ -65,6 +65,8 @@ class ToolIOWhen(ApiModel): param: str matches: list[str] + # The value the endpoint uses when this parameter is absent; None when it has none. + default: str | None = None class ToolIOCase(ApiModel): @@ -378,7 +380,10 @@ def collect_tool_io(spec: dict[str, Any]) -> dict[str, dict[str, Any]]: def _render_when(condition: dict[str, Any]) -> str: - return f"ToolIOWhen(param={json.dumps(condition['param'])}, matches={json.dumps(condition['matches'])})" + parts = [f"param={json.dumps(condition['param'])}", f"matches={json.dumps(condition['matches'])}"] + if "default" in condition: + parts.append(f"default={json.dumps(condition['default'])}") + return f"ToolIOWhen({', '.join(parts)})" def _render_case(case: dict[str, Any]) -> str: diff --git a/engine/src/stirling/models/tool_io.py b/engine/src/stirling/models/tool_io.py index 8c95be59a1..da0cd5b9ae 100644 --- a/engine/src/stirling/models/tool_io.py +++ b/engine/src/stirling/models/tool_io.py @@ -58,6 +58,8 @@ class ToolIOWhen(ApiModel): param: str matches: list[str] + # The value the endpoint uses when this parameter is absent; None when it has none. + default: str | None = None class ToolIOCase(ApiModel): @@ -101,7 +103,7 @@ TOOL_IO: dict[ToolEndpoint, ToolIOSpec] = { arity=ToolArity.SIMO, cases=[ ToolIOCase( - when=[ToolIOWhen(param="singleOrMultiple", matches=["single"])], + when=[ToolIOWhen(param="singleOrMultiple", matches=["single"], default="multiple")], produces=ToolFormat.IMAGE, arity=ToolArity.SISO, ) @@ -130,15 +132,19 @@ TOOL_IO: dict[ToolEndpoint, ToolIOSpec] = { arity=ToolArity.SISO, cases=[ ToolIOCase( - when=[ToolIOWhen(param="outputFormat", matches=["ps"])], + when=[ToolIOWhen(param="outputFormat", matches=["ps"], default="eps")], produces=ToolFormat.POSTSCRIPT, arity=ToolArity.SISO, ), ToolIOCase( - when=[ToolIOWhen(param="outputFormat", matches=["pcl"])], produces=ToolFormat.PCL, arity=ToolArity.SISO + when=[ToolIOWhen(param="outputFormat", matches=["pcl"], default="eps")], + produces=ToolFormat.PCL, + arity=ToolArity.SISO, ), ToolIOCase( - when=[ToolIOWhen(param="outputFormat", matches=["xps"])], produces=ToolFormat.XPS, arity=ToolArity.SISO + when=[ToolIOWhen(param="outputFormat", matches=["xps"], default="eps")], + produces=ToolFormat.XPS, + arity=ToolArity.SISO, ), ], ), @@ -151,7 +157,7 @@ TOOL_IO: dict[ToolEndpoint, ToolIOSpec] = { arity=ToolArity.MIMO, cases=[ ToolIOCase( - when=[ToolIOWhen(param="combineIntoSinglePdf", matches=["true"])], + when=[ToolIOWhen(param="combineIntoSinglePdf", matches=["true"], default="false")], produces=ToolFormat.PDF, arity=ToolArity.MISO, ) @@ -202,7 +208,9 @@ TOOL_IO: dict[ToolEndpoint, ToolIOSpec] = { arity=ToolArity.SISO, cases=[ ToolIOCase( - when=[ToolIOWhen(param="dryRun", matches=["true"])], produces=ToolFormat.JSON, arity=ToolArity.SISO + when=[ToolIOWhen(param="dryRun", matches=["true"], default="false")], + produces=ToolFormat.JSON, + arity=ToolArity.SISO, ) ], ), @@ -223,7 +231,9 @@ TOOL_IO: dict[ToolEndpoint, ToolIOSpec] = { arity=ToolArity.SISO, cases=[ ToolIOCase( - when=[ToolIOWhen(param="sidecar", matches=["true"])], produces=ToolFormat.ZIP, arity=ToolArity.SISO + when=[ToolIOWhen(param="sidecar", matches=["true"], default="false")], + produces=ToolFormat.ZIP, + arity=ToolArity.SISO, ) ], ), @@ -242,7 +252,10 @@ TOOL_IO: dict[ToolEndpoint, ToolIOSpec] = { arity=ToolArity.SISO, cases=[ ToolIOCase( - when=[ToolIOWhen(param="password", matches=[""]), ToolIOWhen(param="ownerPassword", matches=[""])], + when=[ + ToolIOWhen(param="password", matches=[""], default=""), + ToolIOWhen(param="ownerPassword", matches=[""], default=""), + ], produces=ToolFormat.PDF, arity=ToolArity.SISO, ) diff --git a/engine/src/stirling/services/tool_io_compat.py b/engine/src/stirling/services/tool_io_compat.py index b2bb5566e9..f65f0efc22 100644 --- a/engine/src/stirling/services/tool_io_compat.py +++ b/engine/src/stirling/services/tool_io_compat.py @@ -89,11 +89,16 @@ def resolve_output(spec: ToolIOSpec, parameters: dict[str, object] | None) -> Re for rule in spec.cases: all_hold = True for condition in rule.when: - if parameters is None or condition.param not in parameters: + if parameters is not None and condition.param in parameters: + raw: object = parameters[condition.param] + elif condition.default is not None: + # The caller omitted it, so it takes the endpoint's default. + raw = condition.default + else: saw_unknown_param = True all_hold = False continue - normalised = _normalise(parameters[condition.param]) + normalised = _normalise(raw) all_hold = all_hold and any(_normalise(m) == normalised for m in condition.matches) if all_hold: return ResolvedOutput(format=rule.produces, arity=rule.arity, certain=True) diff --git a/frontend/editor/scripts/generate-tool-api-types.mts b/frontend/editor/scripts/generate-tool-api-types.mts index a805f2dfa3..d5990096af 100644 --- a/frontend/editor/scripts/generate-tool-api-types.mts +++ b/frontend/editor/scripts/generate-tool-api-types.mts @@ -258,6 +258,8 @@ export type ToolArity = ${union(vocabulary.arities as string[])}; export interface ToolIOWhen { param: string; matches: string[]; + /** The value the endpoint uses when this parameter is absent; omitted when it has none. */ + default?: string; } /** An output that applies when every condition in \`when\` holds. */ diff --git a/frontend/editor/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts b/frontend/editor/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts index ac5f9a7d96..fa249a6162 100644 --- a/frontend/editor/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts +++ b/frontend/editor/src/core/hooks/tools/changePermissions/useChangePermissionsOperation.ts @@ -80,6 +80,13 @@ export const changePermissionsOperationConfig = defineSingleFileTool({ operationType: "changePermissions", endpoint: ENDPOINT, // Change Permissions is a fake endpoint for the Add Password tool defaultParameters, + // Both tools post to add-password. A permissions-only step carries none of the encryption + // fields, so it is this tool and not Add Password; keyLength, always sent by Add Password, + // is the reliable tell even when a password happens to be blank. + claimsStoredStep: (apiParams) => + !("password" in apiParams) && + !("ownerPassword" in apiParams) && + !("keyLength" in apiParams), }); export const useChangePermissionsOperation = () => { diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts index f4cb974692..467e3ea5f7 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts @@ -24,6 +24,8 @@ import { SPLIT_METHODS } from "@app/constants/splitConstants"; import { redactOperationConfig } from "@app/hooks/tools/redact/useRedactOperation"; import { autoRotateOperationConfig } from "@app/hooks/tools/autoRotate/useAutoRotateOperation"; import { defaultParameters as autoRotateDefaults } from "@app/hooks/tools/autoRotate/useAutoRotateParameters"; +import { addPasswordOperationConfig } from "@app/hooks/tools/addPassword/useAddPasswordOperation"; +import { changePermissionsOperationConfig } from "@app/hooks/tools/changePermissions/useChangePermissionsOperation"; function entry(over: Partial): ToolRegistryEntry { return { @@ -239,6 +241,46 @@ describe("serialize/deserialize round-trip", () => { }); }); +describe("shared-endpoint disambiguation", () => { + const addPassword = entry({ + name: "Add Password", + automationSettings: NoopSettings, + operationConfig: asRegistryConfig(addPasswordOperationConfig), + }); + const changePermissions = entry({ + name: "Change Permissions", + automationSettings: NoopSettings, + operationConfig: asRegistryConfig(changePermissionsOperationConfig), + }); + const ADD_PASSWORD = "/api/v1/security/add-password"; + + // Permissions only, no encryption fields: this is Change Permissions. + const permsOnly = { + operation: ADD_PASSWORD, + parameters: { preventPrinting: true }, + }; + // Carries keyLength (and a password): this is Add Password, even with a blank owner password. + const withPassword = { + operation: ADD_PASSWORD, + parameters: { password: "s3cret", ownerPassword: "", keyLength: 256 }, + }; + + // Both share an endpoint, so the wrong one would win by registry order without a discriminator. + for (const [label, registry] of [ + ["add-password declared first", { addPassword, changePermissions }], + ["change-permissions declared first", { changePermissions, addPassword }], + ] as const) { + test(`each stored step reloads as its own tool (${label})`, () => { + expect(deserializeToolStep(permsOnly, registry).toolId).toBe( + "changePermissions", + ); + expect(deserializeToolStep(withPassword, registry).toolId).toBe( + "addPassword", + ); + }); + } +}); + describe("stepRequiresUpload", () => { const step = (params: Record): WorkingToolStep => ({ toolId: "compress" as ToolId, diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts index d3874bc840..aab9e1ab10 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts @@ -226,11 +226,13 @@ function findToolByEndpoint( step: ToolApiStep, registry: Partial, ): [ToolId, ToolRegistryEntry] | undefined { + const staticMatches: [ToolId, ToolRegistryEntry][] = []; let dynamic: [ToolId, ToolRegistryEntry] | undefined; for (const [id, entry] of Object.entries(registry)) { const endpoint = entry?.operationConfig?.endpoint; if (typeof endpoint === "string") { - if (endpoint === step.operation) return [id as ToolId, entry]; + if (endpoint === step.operation) + staticMatches.push([id as ToolId, entry]); } else if (typeof endpoint === "function" && !dynamic) { const declared = entry?.operationConfig?.endpoints; const matched = declared @@ -239,9 +241,33 @@ function findToolByEndpoint( if (matched) dynamic = [id as ToolId, entry]; } } + if (staticMatches.length > 0) { + return disambiguateStaticMatches(staticMatches, step.parameters); + } return dynamic; } +/** + * Most endpoints belong to one tool, so the single match is returned unchanged. When several + * share an endpoint (Add Password and its permissions-only alias Change Permissions), prefer the + * specialised tool that claims the stored parameters; otherwise fall back to the general owner + * that declares no such claim. + */ +function disambiguateStaticMatches( + matches: [ToolId, ToolRegistryEntry][], + parameters: Record, +): [ToolId, ToolRegistryEntry] { + if (matches.length === 1) return matches[0]; + const claimed = matches.find(([, entry]) => + entry.operationConfig?.claimsStoredStep?.(parameters), + ); + if (claimed) return claimed; + const general = matches.find( + ([, entry]) => !entry.operationConfig?.claimsStoredStep, + ); + return general ?? matches[0]; +} + /** A stored step kept verbatim because its endpoint maps to no known tool. */ function unmappedStep(step: ToolApiStep): UnknownToolStep { return { diff --git a/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts b/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts index 60453c576f..d1f306a3f0 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolOperationTypes.ts @@ -101,6 +101,13 @@ interface BaseToolOperationConfig { */ fromApiParams?(apiParams: ToolApiParams[TEndpoint]): Partial; + /** + * Whether a stored step belongs to this tool, used only to tell apart tools that share an endpoint. + * Receives the raw stored request body. Absent means the tool is the general owner of its + * endpoint and claims any step no specialised sibling claims. + */ + claimsStoredStep?(apiParams: Record): boolean; + /** * For custom tools: if true, success implies all input files were successfully processed. * Use this for tools like Automate or Merge where Many-to-One relationships exist diff --git a/frontend/editor/src/core/types/toolIO.ts b/frontend/editor/src/core/types/toolIO.ts index f8a034dcee..96572c65f4 100644 --- a/frontend/editor/src/core/types/toolIO.ts +++ b/frontend/editor/src/core/types/toolIO.ts @@ -72,6 +72,8 @@ export type ToolArity = "SISO" | "SIMO" | "MISO" | "MIMO"; export interface ToolIOWhen { param: string; matches: string[]; + /** The value the endpoint uses when this parameter is absent; omitted when it has none. */ + default?: string; } /** An output that applies when every condition in `when` holds. */ @@ -168,7 +170,13 @@ export const TOOL_IO: ToolIOTable = { arity: "SIMO", cases: [ { - when: [{ param: "singleOrMultiple", matches: ["single"] }], + when: [ + { + param: "singleOrMultiple", + matches: ["single"], + default: "multiple", + }, + ], produces: "IMAGE", arity: "SISO", }, @@ -207,17 +215,17 @@ export const TOOL_IO: ToolIOTable = { arity: "SISO", cases: [ { - when: [{ param: "outputFormat", matches: ["ps"] }], + when: [{ param: "outputFormat", matches: ["ps"], default: "eps" }], produces: "POSTSCRIPT", arity: "SISO", }, { - when: [{ param: "outputFormat", matches: ["pcl"] }], + when: [{ param: "outputFormat", matches: ["pcl"], default: "eps" }], produces: "PCL", arity: "SISO", }, { - when: [{ param: "outputFormat", matches: ["xps"] }], + when: [{ param: "outputFormat", matches: ["xps"], default: "eps" }], produces: "XPS", arity: "SISO", }, @@ -244,7 +252,13 @@ export const TOOL_IO: ToolIOTable = { arity: "MIMO", cases: [ { - when: [{ param: "combineIntoSinglePdf", matches: ["true"] }], + when: [ + { + param: "combineIntoSinglePdf", + matches: ["true"], + default: "false", + }, + ], produces: "PDF", arity: "MISO", }, @@ -432,7 +446,7 @@ export const TOOL_IO: ToolIOTable = { arity: "SISO", cases: [ { - when: [{ param: "dryRun", matches: ["true"] }], + when: [{ param: "dryRun", matches: ["true"], default: "false" }], produces: "JSON", arity: "SISO", }, @@ -485,7 +499,7 @@ export const TOOL_IO: ToolIOTable = { arity: "SISO", cases: [ { - when: [{ param: "sidecar", matches: ["true"] }], + when: [{ param: "sidecar", matches: ["true"], default: "false" }], produces: "ZIP", arity: "SISO", }, @@ -534,8 +548,8 @@ export const TOOL_IO: ToolIOTable = { cases: [ { when: [ - { param: "password", matches: [""] }, - { param: "ownerPassword", matches: [""] }, + { param: "password", matches: [""], default: "" }, + { param: "ownerPassword", matches: [""], default: "" }, ], produces: "PDF", arity: "SISO", diff --git a/frontend/editor/src/core/utils/toolIOCompat.ts b/frontend/editor/src/core/utils/toolIOCompat.ts index a5f9ea0fe9..473195ff7b 100644 --- a/frontend/editor/src/core/utils/toolIOCompat.ts +++ b/frontend/editor/src/core/utils/toolIOCompat.ts @@ -102,12 +102,18 @@ export function resolveOutput( for (const rule of spec.cases ?? []) { let allHold = true; for (const condition of rule.when) { - if (!parameters || !(condition.param in parameters)) { + let raw: unknown; + if (parameters && condition.param in parameters) { + raw = parameters[condition.param]; + } else if (condition.default !== undefined) { + // The caller omitted it, so it takes the endpoint's default. + raw = condition.default; + } else { sawUnknownParam = true; allHold = false; continue; } - const value = normalise(parameters[condition.param]); + const value = normalise(raw); allHold &&= condition.matches.some((match) => normalise(match) === value); } if (allHold) { diff --git a/testing/tool-io-cases.json b/testing/tool-io-cases.json index 59c2802f8d..996b2b693b 100644 --- a/testing/tool-io-cases.json +++ b/testing/tool-io-cases.json @@ -70,6 +70,33 @@ "arity": "SISO" } ] + }, + "autoRotateShape": { + "accepts": ["PDF"], + "produces": "PDF", + "arity": "SISO", + "cases": [ + { + "when": [{ "param": "dryRun", "matches": ["true"], "default": "false" }], + "produces": "JSON", + "arity": "SISO" + } + ] + }, + "changePermsShape": { + "accepts": ["PDF"], + "produces": "PDF_ENCRYPTED", + "arity": "SISO", + "cases": [ + { + "when": [ + { "param": "password", "matches": [""], "default": "" }, + { "param": "ownerPassword", "matches": [""], "default": "" } + ], + "produces": "PDF", + "arity": "SISO" + } + ] } }, "cases": [ @@ -250,6 +277,35 @@ ], "expected": [{ "stepIndex": 1, "severity": "INFO", "code": "fan-in" }] }, + { + "name": "an absent parameter takes its default, which here does not trigger the case", + "steps": [{ "spec": "autoRotateShape" }, { "spec": "pdfToPdf" }], + "expected": [] + }, + { + "name": "an explicit value overrides the default and triggers the case", + "steps": [ + { "spec": "autoRotateShape", "parameters": { "dryRun": "true" } }, + { "spec": "pdfToPdf" } + ], + "expected": [{ "stepIndex": 1, "severity": "ERROR", "code": "format-mismatch" }] + }, + { + "name": "an absent parameter whose default triggers the case resolves to that branch", + "steps": [{ "spec": "changePermsShape" }, { "spec": "pdfToPdf" }], + "expected": [] + }, + { + "name": "setting the branch parameter away from its default flips the outcome", + "steps": [ + { + "spec": "changePermsShape", + "parameters": { "password": "hunter2", "ownerPassword": "hunter2" } + }, + { "spec": "pdfToPdf" } + ], + "expected": [{ "stepIndex": 1, "severity": "ERROR", "code": "format-mismatch" }] + }, { "name": "an undeclared step warns and stops the chain being checked past it", "steps": [{ "spec": "addPassword" }, { "spec": null }, { "spec": "pdfToPdf" }],