Repository navigation
Conversation
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Parameter inheritance and server overrides introduce request-execution failures through lost parameter references, same-name parameter collisions, unexpanded server variables, and unresolved relative URLs.
4 open issues
What changed in this PR
This PR hardens the SDK’s OpenAPI request resolution, addressing server selection and inherited parameter handling described in #1831.
Changes:
- Default Swagger 2 servers to HTTPS when
schemesis absent. - Merge path-level and operation-level parameters, with operation-level definitions taking precedence.
- Honor OpenAPI 3 operation and path server overrides, with tests for precedence and parameter merging.
| File | Description |
|---|---|
impl/openapi/src/test/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinitionTest.java |
Adds tests for HTTPS fallback, parameter merging, and server precedence. |
impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/UnifiedOpenAPI.java |
Guards missing schemes and models inherited parameters and server overrides. |
impl/openapi/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OperationDefinition.java |
Merges parameter definitions and selects servers by precedence. |
Review effort: Balanced
fjtirado
reviewed
Nov 13, 2026
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
# Conflicts: # impl/core/src/main/java/org/openworkflow/sdk/impl/executors/openapi/OpenAPIExecutor.java # impl/core/src/test/resources/schema/openapi/parameter-refs.json # impl/core/src/test/resources/schema/swagger/parameter-refs.json # impl/test/src/test/java/org/openworkflow/sdk/impl/test/OpenAPITest.java
mathecruz
marked this pull request as ready for review
November 13, 2026 14:36
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
getServers()uses schemes.stream().findFirst() with guarding for nullgetParameters()reads now operation-level parameter and path-level parameterUnifiedOpenAPI.getServers()when a Swagger 2 document has a host but no schemes (previously threw NPE)Closes #1831