From 8cdc40683d21a6e19c85b05b464eda7e7d589920 Mon Sep 17 00:00:00 2001 From: Matheus Cruz Date: Thu, 12 Nov 2026 17:44:46 -0300 Subject: [PATCH 1/2] Harden OpenAPI server and parameter resolution Signed-off-by: Matheus Cruz --- .../openapi/OperationDefinition.java | 58 +++++++--- .../executors/openapi/UnifiedOpenAPI.java | 12 +- .../openapi/OperationDefinitionTest.java | 104 ++++++++++++++++++ 3 files changed, 156 insertions(+), 18 deletions(-) create mode 100644 impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java diff --git a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java index 33c50dd7a..aebac2d80 100644 --- a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java +++ b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java @@ -16,6 +16,7 @@ package org.openworkflow.sdk.impl.executors.openapi; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -23,14 +24,20 @@ class OperationDefinition { private final UnifiedOpenAPI.Operation operation; + private final UnifiedOpenAPI.PathItem pathItem; private final String method; private final UnifiedOpenAPI openAPI; private final String path; OperationDefinition( - UnifiedOpenAPI openAPI, UnifiedOpenAPI.Operation operation, String path, String method) { + UnifiedOpenAPI openAPI, + UnifiedOpenAPI.PathItem pathItem, + UnifiedOpenAPI.Operation operation, + String path, + String method) { this.openAPI = Objects.requireNonNull(openAPI, "openAPI cannot be null"); + this.pathItem = pathItem; this.operation = Objects.requireNonNull(operation, "operation cannot be null"); this.path = Objects.requireNonNull(path, "path cannot be null"); this.method = Objects.requireNonNull(method, "method cannot be null"); @@ -49,30 +56,49 @@ UnifiedOpenAPI.Operation getOperation() { } List getServers() { - if (openAPI.getServers() == null) { - return List.of(); + if (openAPI.swaggerVersion() == UnifiedOpenAPI.SwaggerVersion.OPENAPI_V3) { + if (operation.servers() != null && !operation.servers().isEmpty()) { + return toUrls(operation.servers()); + } + if (pathItem != null && pathItem.servers() != null && !pathItem.servers().isEmpty()) { + return toUrls(pathItem.servers()); + } } - return openAPI.getServers(); + List servers = openAPI.getServers(); + return servers == null ? List.of() : servers; + } + + private static List toUrls(List servers) { + return servers.stream().map(UnifiedOpenAPI.Server::url).toList(); + } + + private List mergedParameters() { + Map merged = new LinkedHashMap<>(); + if (pathItem != null && pathItem.parameters() != null) { + pathItem.parameters().forEach(p -> merged.put(p.in() + ":" + p.name(), p)); + } + if (operation.hasParameters()) { + operation.parameters().forEach(p -> merged.put(p.in() + ":" + p.name(), p)); + } + return new ArrayList<>(merged.values()); } List getParameters() { + List allParameters = mergedParameters(); List paramDefinitions = new ArrayList<>(); - if (operation.hasParameters()) { - for (UnifiedOpenAPI.Parameter parameter : operation.parameters()) { - if ("body".equals(parameter.in())) { - continue; // body parameters are handled separately - } - - paramDefinitions.add( - new ParameterDefinition( - parameter.name(), parameter.in(), parameter.required(), parameter.schema())); + for (UnifiedOpenAPI.Parameter parameter : allParameters) { + if ("body".equals(parameter.in())) { + continue; // body parameters are handled separately } + + paramDefinitions.add( + new ParameterDefinition( + parameter.name(), parameter.in(), parameter.required(), parameter.schema())); } - if (openAPI.swaggerVersion() == UnifiedOpenAPI.SwaggerVersion.SWAGGER_V2 - && operation.hasParameters()) { - operation.parameters().stream() + if (openAPI.swaggerVersion() == UnifiedOpenAPI.SwaggerVersion.SWAGGER_V2) { + allParameters.stream() .filter(p -> "body".equals(p.in())) .forEach( p -> { diff --git a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java index 4f45c4a39..6f8413241 100644 --- a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java +++ b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java @@ -59,6 +59,7 @@ Optional findOperationById(String operationId) { return Optional.of( new OperationDefinition( this, + pathItem, httpOperation.operation(), path, httpOperation.method().toUpperCase(Locale.ROOT))); @@ -79,7 +80,8 @@ public List getServers() { base += basePath; } - return List.of(schemes.stream().findFirst().orElse("https") + "://" + base); + String scheme = schemes == null ? "https" : schemes.stream().findFirst().orElse("https"); + return List.of(scheme + "://" + base); } if (servers == null || servers.isEmpty()) { @@ -132,6 +134,8 @@ private Schema resolveRefSwaggerV2(String ref) { public record Server(String url) {} public record PathItem( + List servers, + List parameters, Operation get, Operation post, Operation put, @@ -154,7 +158,11 @@ Set methods() { public record HttpOperation(String method, Operation operation) {} - public record Operation(String operationId, List parameters, RequestBody requestBody) { + public record Operation( + String operationId, + List servers, + List parameters, + RequestBody requestBody) { public boolean hasParameters() { return parameters != null && !parameters.isEmpty(); diff --git a/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java b/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java new file mode 100644 index 000000000..cd730f26f --- /dev/null +++ b/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java @@ -0,0 +1,104 @@ +/* + * Copyright 2020-Present The Open Workflow Specification Authors + * + * Licensed 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. + */ +package org.openworkflow.sdk.impl.executors.openapi; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class OperationDefinitionTest { + + private static UnifiedOpenAPI.Parameter param(String name, String in, boolean required) { + return new UnifiedOpenAPI.Parameter(name, in, required, null); + } + + private static UnifiedOpenAPI openAPI3( + List servers, UnifiedOpenAPI.PathItem pathItem) { + return new UnifiedOpenAPI( + "3.0.0", servers, null, null, null, Map.of("/pets", pathItem), null, null); + } + + private static UnifiedOpenAPI.PathItem pathItem( + List servers, + List parameters, + UnifiedOpenAPI.Operation get) { + return new UnifiedOpenAPI.PathItem( + servers, parameters, get, null, null, null, null, null, null); + } + + @Test + void swaggerV2WithoutSchemesDefaultsToHttps() { + UnifiedOpenAPI openAPI = + new UnifiedOpenAPI("2.0", null, "example.com", "/v2", null, Map.of(), null, null); + assertEquals(List.of("https://example.com/v2"), openAPI.getServers()); + } + + @Test + void pathLevelParametersAreMergedAndOverridden() { + UnifiedOpenAPI.Operation op = + new UnifiedOpenAPI.Operation( + "listPets", + null, + List.of(param("limit", "query", true), param("id", "path", false)), + null); + UnifiedOpenAPI.PathItem item = + pathItem(null, List.of(param("id", "path", true), param("trace", "header", false)), op); + + OperationDefinition definition = + openAPI3(null, item).findOperationById("listPets").orElseThrow(); + List params = definition.getParameters(); + + assertEquals(3, params.size()); + ParameterDefinition id = + params.stream().filter(p -> p.name().equals("id")).findFirst().orElseThrow(); + assertFalse(id.required(), "operation-level parameter must override path-level one"); + assertTrue(params.stream().anyMatch(p -> p.name().equals("trace"))); + assertTrue(params.stream().anyMatch(p -> p.name().equals("limit"))); + } + + @Test + void serverOverridesFollowPrecedence() { + List root = List.of(new UnifiedOpenAPI.Server("https://root")); + List path = List.of(new UnifiedOpenAPI.Server("https://path")); + List oper = List.of(new UnifiedOpenAPI.Server("https://operation")); + + UnifiedOpenAPI.Operation plain = new UnifiedOpenAPI.Operation("op", null, null, null); + UnifiedOpenAPI.Operation overriding = new UnifiedOpenAPI.Operation("op", oper, null, null); + + assertEquals( + List.of("https://root"), + openAPI3(root, pathItem(null, null, plain)) + .findOperationById("op") + .orElseThrow() + .getServers()); + assertEquals( + List.of("https://path"), + openAPI3(root, pathItem(path, null, plain)) + .findOperationById("op") + .orElseThrow() + .getServers()); + assertEquals( + List.of("https://operation"), + openAPI3(root, pathItem(path, null, overriding)) + .findOperationById("op") + .orElseThrow() + .getServers()); + } +} From 085fc88c85bf815c43fcefd5bc88b87b61bdf777 Mon Sep 17 00:00:00 2001 From: Matheus Cruz Date: Fri, 13 Nov 2026 11:20:36 -0300 Subject: [PATCH 2/2] Fix OpenAPI parameter binding and resolve parameter references Signed-off-by: Matheus Cruz --- .../jackson/JacksonUnifiedOpenAPIMixIn.java | 3 +- .../executors/openapi/OpenAPIExecutor.java | 18 +- .../openapi/OperationDefinition.java | 9 +- .../executors/openapi/UnifiedOpenAPI.java | 53 +++++- .../openapi/OpenAPIProcessorTest.java | 160 ++++++++++++++++++ .../openapi/OperationDefinitionTest.java | 104 ------------ .../schema/openapi/parameter-refs.json | 77 +++++++++ .../schema/swagger/parameter-refs.json | 67 ++++++++ .../sdk/impl/test/OpenAPITest.java | 29 ++++ .../openapi/openapi-shared-param-name.yaml | 29 ++++ .../openapi/get-item-shared-param-name.yaml | 14 ++ 11 files changed, 446 insertions(+), 117 deletions(-) delete mode 100644 impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java create mode 100644 impl/openapi/src/test/resources/schema/openapi/parameter-refs.json create mode 100644 impl/openapi/src/test/resources/schema/swagger/parameter-refs.json create mode 100644 impl/test/src/test/resources/schema/openapi/openapi-shared-param-name.yaml create mode 100644 impl/test/src/test/resources/workflows-samples/openapi/get-item-shared-param-name.yaml diff --git a/impl/openapi-jackson/src/main/java/org/openworkflow/sdk/impl/executors/openapi/jackson/JacksonUnifiedOpenAPIMixIn.java b/impl/openapi-jackson/src/main/java/org/openworkflow/sdk/impl/executors/openapi/jackson/JacksonUnifiedOpenAPIMixIn.java index 6f43e9086..5108fbc81 100644 --- a/impl/openapi-jackson/src/main/java/org/openworkflow/sdk/impl/executors/openapi/jackson/JacksonUnifiedOpenAPIMixIn.java +++ b/impl/openapi-jackson/src/main/java/org/openworkflow/sdk/impl/executors/openapi/jackson/JacksonUnifiedOpenAPIMixIn.java @@ -29,7 +29,8 @@ public class Server {} public class RequestBody {} @JsonIgnoreProperties(ignoreUnknown = true) - public class Parameter {} + public record Parameter( + String name, String in, boolean required, Schema schema, @JsonProperty("$ref") String ref) {} @JsonIgnoreProperties(ignoreUnknown = true) public class PathItem {} diff --git a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIExecutor.java b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIExecutor.java index b7108c4d9..edabf3ea9 100644 --- a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIExecutor.java +++ b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIExecutor.java @@ -104,20 +104,23 @@ private void fillHttpBuilder(WorkflowApplication application, OperationDefinitio Map pathParameters = new HashMap<>(); Set missingParams = new HashSet<>(); - Map bodyParameters = new HashMap<>(parameters); + // Parameters are unique per (in, name) + Set consumedParams = new HashSet<>(); for (ParameterDefinition parameter : operation.getParameters()) { switch (parameter.in()) { case "header": - param(parameter, bodyParameters, headersMap, missingParams); + param(parameter, headersMap, consumedParams, missingParams); break; case "path": - param(parameter, bodyParameters, pathParameters, missingParams); + param(parameter, pathParameters, consumedParams, missingParams); break; case "query": - param(parameter, bodyParameters, queryMap, missingParams); + param(parameter, queryMap, consumedParams, missingParams); break; } } + Map bodyParameters = new HashMap<>(parameters); + bodyParameters.keySet().removeAll(consumedParams); if (!missingParams.isEmpty()) { throw new IllegalArgumentException( @@ -137,12 +140,13 @@ private void fillHttpBuilder(WorkflowApplication application, OperationDefinitio private void param( ParameterDefinition parameter, - Map origMap, Map collectorMap, + Set consumedParams, Set missingParams) { String name = parameter.name(); - if (origMap.containsKey(name)) { - collectorMap.put(parameter.name(), origMap.remove(name)); + if (parameters.containsKey(name)) { + collectorMap.put(name, parameters.get(name)); + consumedParams.add(name); } else if (parameter.required()) { UnifiedOpenAPI.Schema schema = parameter.schema(); diff --git a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java index aebac2d80..f12d0c22e 100644 --- a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java +++ b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java @@ -76,14 +76,19 @@ private static List toUrls(List servers) { private List mergedParameters() { Map merged = new LinkedHashMap<>(); if (pathItem != null && pathItem.parameters() != null) { - pathItem.parameters().forEach(p -> merged.put(p.in() + ":" + p.name(), p)); + pathItem.parameters().forEach(p -> merge(merged, p)); } if (operation.hasParameters()) { - operation.parameters().forEach(p -> merged.put(p.in() + ":" + p.name(), p)); + operation.parameters().forEach(p -> merge(merged, p)); } return new ArrayList<>(merged.values()); } + private void merge(Map merged, UnifiedOpenAPI.Parameter p) { + UnifiedOpenAPI.Parameter resolved = openAPI.resolveParameter(p); + merged.put(resolved.in() + ":" + resolved.name(), resolved); + } + List getParameters() { List allParameters = mergedParameters(); List paramDefinitions = new ArrayList<>(); diff --git a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java index 6f8413241..30e9db5b1 100644 --- a/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java +++ b/impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java @@ -15,6 +15,7 @@ */ package org.openworkflow.sdk.impl.executors.openapi; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -29,7 +30,8 @@ public record UnifiedOpenAPI( List schemes, Map paths, Components components, - Map definitions) { + Map definitions, + Map parameters) { public enum SwaggerVersion { SWAGGER_V2, @@ -131,6 +133,46 @@ private Schema resolveRefSwaggerV2(String ref) { return definitions.get(name); } + /** + * Resolves a parameter reference, following chained references. + * + *

Parameters that are not references are returned as-is. + * + * @throws IllegalArgumentException if the reference cannot be resolved or is circular + */ + public Parameter resolveParameter(Parameter parameter) { + Set visited = new HashSet<>(); + while (parameter != null && parameter.hasRef()) { + String ref = parameter.ref(); + if (!visited.add(ref)) { + throw new IllegalArgumentException("Circular OpenAPI parameter reference: " + ref); + } + Parameter resolved = + isSwaggerV2() ? resolveParameterSwaggerV2(ref) : resolveParameterOpenAPI(ref); + if (resolved == null) { + throw new IllegalArgumentException("Unable to resolve OpenAPI parameter reference: " + ref); + } + parameter = resolved; + } + return parameter; + } + + private Parameter resolveParameterOpenAPI(String ref) { + if (!ref.startsWith("#/components/parameters/") + || components == null + || components.parameters() == null) { + return null; + } + return components.parameters().get(ref.substring("#/components/parameters/".length())); + } + + private Parameter resolveParameterSwaggerV2(String ref) { + if (!ref.startsWith("#/parameters/") || parameters == null) { + return null; + } + return parameters.get(ref.substring("#/parameters/".length())); + } + public record Server(String url) {} public record PathItem( @@ -173,7 +215,12 @@ public boolean hasRequestBody() { } } - public record Parameter(String name, String in, boolean required, Schema schema) {} + public record Parameter(String name, String in, boolean required, Schema schema, String ref) { + + public boolean hasRef() { + return ref != null && !ref.isBlank(); + } + } public record RequestBody(Content content) {} @@ -185,7 +232,7 @@ public boolean isApplicationJson() { public record MediaType(Schema schema) {} - public record Components(Map schemas) {} + public record Components(Map schemas, Map parameters) {} public record Schema( String type, diff --git a/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIProcessorTest.java b/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIProcessorTest.java index 34939c315..44aae68da 100644 --- a/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIProcessorTest.java +++ b/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIProcessorTest.java @@ -17,11 +17,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.io.UncheckedIOException; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.openworkflow.sdk.impl.resources.ClasspathResource; import org.openworkflow.sdk.impl.resources.ExternalResourceHandler; @@ -198,6 +200,164 @@ public void testLoginUser(UnifiedOpenAPI json) { assertTrue(param2.required()); } + @Test + public void testInheritedParameterReferenceIsResolvedSwaggerV2() { + testInheritedParameterReferenceIsResolved(readResource("schema/swagger/parameter-refs.json")); + } + + @Test + public void testInheritedParameterReferenceIsResolvedOpenAPI() { + testInheritedParameterReferenceIsResolved(readResource("schema/openapi/parameter-refs.json")); + } + + public void testInheritedParameterReferenceIsResolved(UnifiedOpenAPI openAPI) { + OperationDefinition definition = new OpenAPIProcessor("getItem").parse(openAPI); + + List params = definition.getParameters(); + assertEquals(2, params.size()); + ParameterDefinition traceId = params.get(0); + assertEquals("header", traceId.in()); + assertEquals("X-Trace-Id", traceId.name()); + assertFalse(traceId.required()); + ParameterDefinition itemId = params.get(1); + assertEquals("path", itemId.in()); + assertEquals("itemId", itemId.name()); + } + + @Test + public void testInheritedParameterReferenceIsOverriddenInlineSwaggerV2() { + testInheritedParameterReferenceIsOverriddenInline( + readResource("schema/swagger/parameter-refs.json")); + } + + @Test + public void testInheritedParameterReferenceIsOverriddenInlineOpenAPI() { + testInheritedParameterReferenceIsOverriddenInline( + readResource("schema/openapi/parameter-refs.json")); + } + + public void testInheritedParameterReferenceIsOverriddenInline(UnifiedOpenAPI openAPI) { + OperationDefinition definition = new OpenAPIProcessor("deleteItem").parse(openAPI); + + List params = definition.getParameters(); + assertEquals(2, params.size()); + ParameterDefinition traceId = + params.stream().filter(p -> "X-Trace-Id".equals(p.name())).findFirst().orElseThrow(); + assertEquals("header", traceId.in()); + assertTrue(traceId.required(), "operation-level parameter must override the referenced one"); + } + + @Test + public void testSwaggerV2WithoutSchemesDefaultsToHttps() { + UnifiedOpenAPI openAPI = + new UnifiedOpenAPI("2.0", null, "example.com", "/v2", null, Map.of(), null, null, null); + assertEquals(List.of("https://example.com/v2"), openAPI.getServers()); + } + + @Test + public void testPathLevelParametersAreMergedAndOverridden() { + UnifiedOpenAPI.Operation op = + new UnifiedOpenAPI.Operation( + "listPets", + null, + List.of(param("limit", "query", true), param("id", "path", false)), + null); + UnifiedOpenAPI.PathItem item = + pathItem(null, List.of(param("id", "path", true), param("trace", "header", false)), op); + + OperationDefinition definition = new OpenAPIProcessor("listPets").parse(openAPI3(null, item)); + List params = definition.getParameters(); + + assertEquals(3, params.size()); + ParameterDefinition id = + params.stream().filter(p -> p.name().equals("id")).findFirst().orElseThrow(); + assertFalse(id.required(), "operation-level parameter must override path-level one"); + assertTrue(params.stream().anyMatch(p -> p.name().equals("trace"))); + assertTrue(params.stream().anyMatch(p -> p.name().equals("limit"))); + } + + @Test + public void testServerOverridesFollowPrecedence() { + List root = List.of(new UnifiedOpenAPI.Server("https://root")); + List path = List.of(new UnifiedOpenAPI.Server("https://path")); + List oper = List.of(new UnifiedOpenAPI.Server("https://operation")); + + UnifiedOpenAPI.Operation plain = new UnifiedOpenAPI.Operation("op", null, null, null); + UnifiedOpenAPI.Operation overriding = new UnifiedOpenAPI.Operation("op", oper, null, null); + OpenAPIProcessor processor = new OpenAPIProcessor("op"); + + assertEquals( + List.of("https://root"), + processor.parse(openAPI3(root, pathItem(null, null, plain))).getServers()); + assertEquals( + List.of("https://path"), + processor.parse(openAPI3(root, pathItem(path, null, plain))).getServers()); + assertEquals( + List.of("https://operation"), + processor.parse(openAPI3(root, pathItem(path, null, overriding))).getServers()); + } + + @Test + public void testUnresolvableParameterReferenceFails() { + UnifiedOpenAPI.Operation op = new UnifiedOpenAPI.Operation("op", null, null, null); + UnifiedOpenAPI openAPI = + openAPI3(null, pathItem(null, List.of(ref("#/components/parameters/Missing")), op)); + + OperationDefinition definition = new OpenAPIProcessor("op").parse(openAPI); + IllegalArgumentException e = + assertThrows(IllegalArgumentException.class, definition::getParameters); + assertTrue(e.getMessage().contains("#/components/parameters/Missing")); + } + + @Test + public void testCircularParameterReferenceFails() { + UnifiedOpenAPI.Operation op = new UnifiedOpenAPI.Operation("op", null, null, null); + UnifiedOpenAPI.Components components = + new UnifiedOpenAPI.Components( + null, + Map.of( + "A", ref("#/components/parameters/B"), + "B", ref("#/components/parameters/A"))); + UnifiedOpenAPI openAPI = + new UnifiedOpenAPI( + "3.0.0", + null, + null, + null, + null, + Map.of("/pets", pathItem(null, List.of(ref("#/components/parameters/A")), op)), + components, + null, + null); + + OperationDefinition definition = new OpenAPIProcessor("op").parse(openAPI); + IllegalArgumentException e = + assertThrows(IllegalArgumentException.class, definition::getParameters); + assertTrue(e.getMessage().contains("Circular")); + } + + private static UnifiedOpenAPI.Parameter param(String name, String in, boolean required) { + return new UnifiedOpenAPI.Parameter(name, in, required, null, null); + } + + private static UnifiedOpenAPI.Parameter ref(String ref) { + return new UnifiedOpenAPI.Parameter(null, null, false, null, ref); + } + + private static UnifiedOpenAPI openAPI3( + List servers, UnifiedOpenAPI.PathItem pathItem) { + return new UnifiedOpenAPI( + "3.0.0", servers, null, null, null, Map.of("/pets", pathItem), null, null, null); + } + + private static UnifiedOpenAPI.PathItem pathItem( + List servers, + List parameters, + UnifiedOpenAPI.Operation get) { + return new UnifiedOpenAPI.PathItem( + servers, parameters, get, null, null, null, null, null, null); + } + private boolean checkServer(List servers, String expected) { for (String server : servers) { if (server.equals(expected)) { diff --git a/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java b/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java deleted file mode 100644 index cd730f26f..000000000 --- a/impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright 2020-Present The Open Workflow Specification Authors - * - * Licensed 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. - */ -package org.openworkflow.sdk.impl.executors.openapi; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.util.List; -import java.util.Map; -import org.junit.jupiter.api.Test; - -class OperationDefinitionTest { - - private static UnifiedOpenAPI.Parameter param(String name, String in, boolean required) { - return new UnifiedOpenAPI.Parameter(name, in, required, null); - } - - private static UnifiedOpenAPI openAPI3( - List servers, UnifiedOpenAPI.PathItem pathItem) { - return new UnifiedOpenAPI( - "3.0.0", servers, null, null, null, Map.of("/pets", pathItem), null, null); - } - - private static UnifiedOpenAPI.PathItem pathItem( - List servers, - List parameters, - UnifiedOpenAPI.Operation get) { - return new UnifiedOpenAPI.PathItem( - servers, parameters, get, null, null, null, null, null, null); - } - - @Test - void swaggerV2WithoutSchemesDefaultsToHttps() { - UnifiedOpenAPI openAPI = - new UnifiedOpenAPI("2.0", null, "example.com", "/v2", null, Map.of(), null, null); - assertEquals(List.of("https://example.com/v2"), openAPI.getServers()); - } - - @Test - void pathLevelParametersAreMergedAndOverridden() { - UnifiedOpenAPI.Operation op = - new UnifiedOpenAPI.Operation( - "listPets", - null, - List.of(param("limit", "query", true), param("id", "path", false)), - null); - UnifiedOpenAPI.PathItem item = - pathItem(null, List.of(param("id", "path", true), param("trace", "header", false)), op); - - OperationDefinition definition = - openAPI3(null, item).findOperationById("listPets").orElseThrow(); - List params = definition.getParameters(); - - assertEquals(3, params.size()); - ParameterDefinition id = - params.stream().filter(p -> p.name().equals("id")).findFirst().orElseThrow(); - assertFalse(id.required(), "operation-level parameter must override path-level one"); - assertTrue(params.stream().anyMatch(p -> p.name().equals("trace"))); - assertTrue(params.stream().anyMatch(p -> p.name().equals("limit"))); - } - - @Test - void serverOverridesFollowPrecedence() { - List root = List.of(new UnifiedOpenAPI.Server("https://root")); - List path = List.of(new UnifiedOpenAPI.Server("https://path")); - List oper = List.of(new UnifiedOpenAPI.Server("https://operation")); - - UnifiedOpenAPI.Operation plain = new UnifiedOpenAPI.Operation("op", null, null, null); - UnifiedOpenAPI.Operation overriding = new UnifiedOpenAPI.Operation("op", oper, null, null); - - assertEquals( - List.of("https://root"), - openAPI3(root, pathItem(null, null, plain)) - .findOperationById("op") - .orElseThrow() - .getServers()); - assertEquals( - List.of("https://path"), - openAPI3(root, pathItem(path, null, plain)) - .findOperationById("op") - .orElseThrow() - .getServers()); - assertEquals( - List.of("https://operation"), - openAPI3(root, pathItem(path, null, overriding)) - .findOperationById("op") - .orElseThrow() - .getServers()); - } -} diff --git a/impl/openapi/src/test/resources/schema/openapi/parameter-refs.json b/impl/openapi/src/test/resources/schema/openapi/parameter-refs.json new file mode 100644 index 000000000..9e25547eb --- /dev/null +++ b/impl/openapi/src/test/resources/schema/openapi/parameter-refs.json @@ -0,0 +1,77 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "Parameter references", + "version": "1.0.0" + }, + "servers": [ + { + "url": "https://example.com/api" + } + ], + "paths": { + "/items/{itemId}": { + "parameters": [ + { + "$ref": "#/components/parameters/TraceId" + } + ], + "get": { + "operationId": "getItem", + "parameters": [ + { + "name": "itemId", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK" + } + } + }, + "delete": { + "operationId": "deleteItem", + "parameters": [ + { + "name": "itemId", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "X-Trace-Id", + "in": "header", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "204": { + "description": "Deleted" + } + } + } + } + }, + "components": { + "parameters": { + "TraceId": { + "name": "X-Trace-Id", + "in": "header", + "required": false, + "schema": { + "type": "string" + } + } + } + } +} diff --git a/impl/openapi/src/test/resources/schema/swagger/parameter-refs.json b/impl/openapi/src/test/resources/schema/swagger/parameter-refs.json new file mode 100644 index 000000000..f150e3643 --- /dev/null +++ b/impl/openapi/src/test/resources/schema/swagger/parameter-refs.json @@ -0,0 +1,67 @@ +{ + "swagger": "2.0", + "info": { + "title": "Parameter references", + "version": "1.0.0" + }, + "host": "example.com", + "basePath": "/api", + "schemes": [ + "https" + ], + "paths": { + "/items/{itemId}": { + "parameters": [ + { + "$ref": "#/parameters/TraceId" + } + ], + "get": { + "operationId": "getItem", + "parameters": [ + { + "name": "itemId", + "in": "path", + "required": true, + "type": "string" + } + ], + "responses": { + "200": { + "description": "OK" + } + } + }, + "delete": { + "operationId": "deleteItem", + "parameters": [ + { + "name": "itemId", + "in": "path", + "required": true, + "type": "string" + }, + { + "name": "X-Trace-Id", + "in": "header", + "required": true, + "type": "string" + } + ], + "responses": { + "204": { + "description": "Deleted" + } + } + } + } + }, + "parameters": { + "TraceId": { + "name": "X-Trace-Id", + "in": "header", + "required": false, + "type": "string" + } + } +} diff --git a/impl/test/src/test/java/org/openworkflow/sdk/impl/test/OpenAPITest.java b/impl/test/src/test/java/org/openworkflow/sdk/impl/test/OpenAPITest.java index 9f923656c..34e39a469 100644 --- a/impl/test/src/test/java/org/openworkflow/sdk/impl/test/OpenAPITest.java +++ b/impl/test/src/test/java/org/openworkflow/sdk/impl/test/OpenAPITest.java @@ -259,6 +259,35 @@ private void assertData(Map result) throws InterruptedException assertEquals("severus.calix@hive-terra.example.com", data.get("email")); } + @Test + public void testOpenAPISameNameBoundToPathAndInheritedQuery() throws Exception { + Workflow workflow = + readWorkflowFromClasspath("workflows-samples/openapi/get-item-shared-param-name.yaml"); + + Buffer schema = new Buffer(); + try (InputStream is = + OpenAPITest.class.getResourceAsStream("/schema/openapi/openapi-shared-param-name.yaml")) { + schema.write(is.readAllBytes()); + } + openApiServer.enqueue( + new MockResponse() + .setBody(schema) + .setHeader("Content-Type", "application/yaml") + .setResponseCode(200)); + + restServer.enqueue( + new MockResponse() + .setBody("{\"success\": true}") + .setHeader("Content-Type", "application/json") + .setResponseCode(200)); + + app.workflowDefinition(workflow).instance().start().get(); + + RecordedRequest restRequest = restServer.takeRequest(); + assertEquals("GET", restRequest.getMethod()); + assertEquals("/api/v1/items/abc?id=abc", restRequest.getPath()); + } + @Test public void testOpenAPIGetWithPositiveResponseAndVars() throws Exception { Workflow workflow = diff --git a/impl/test/src/test/resources/schema/openapi/openapi-shared-param-name.yaml b/impl/test/src/test/resources/schema/openapi/openapi-shared-param-name.yaml new file mode 100644 index 000000000..88015940e --- /dev/null +++ b/impl/test/src/test/resources/schema/openapi/openapi-shared-param-name.yaml @@ -0,0 +1,29 @@ +openapi: 3.0.3 +info: + title: Shared parameter name API + version: 1.0.0 +servers: + - url: http://127.0.0.1:8886/api/v1 +paths: + /items/{id}: + parameters: + - name: id + in: query + required: false + schema: + type: string + get: + operationId: getItemById + parameters: + - name: id + in: path + required: true + schema: + type: string + responses: + '200': + description: OK + content: + application/json: + schema: + type: object diff --git a/impl/test/src/test/resources/workflows-samples/openapi/get-item-shared-param-name.yaml b/impl/test/src/test/resources/workflows-samples/openapi/get-item-shared-param-name.yaml new file mode 100644 index 000000000..4c28d302a --- /dev/null +++ b/impl/test/src/test/resources/workflows-samples/openapi/get-item-shared-param-name.yaml @@ -0,0 +1,14 @@ +document: + dsl: '1.0.1' + namespace: test + name: openapi-get-item-shared-param-name + version: '0.1.0' +do: + - getItem: + call: openapi + with: + document: + endpoint: http://localhost:8887/shared-param-name.yaml + operationId: getItemById + parameters: + id: abc