Repository navigation
fix(resolver): allow circular reference placeholders in allOf for Ope… - #4312
Open
jochenberger wants to merge 1 commit into
Open
jochenberger wants to merge 1 commit into
jochenberger wants to merge 1 commit into
Conversation
…nAPI 3.1/3.2 When resolving with useCircularStructures=false, circular references are replaced by a generic ObjectElement containing only `$ref` (see circularReplacer). The AllOfVisitor only accepted SchemaElement items and therefore reported "Elements in allOf must be objects" for these placeholders, leaving allOf unmerged. Accept such placeholders in allOf and merge them like any other item, which matches the behavior of the OpenAPI 2.0/3.0 specmap resolver. Refs swagger-api#3858 Refs swagger-api/swagger-ui#10583
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.
Description
When resolving OpenAPI 3.1/3.2 definitions with
useCircularStructures: false(the default), the dereference strategy runs withcircular: 'replace'. Each detected circular reference is replaced by thecircularReplacerwith a genericObjectElementthat contains only$ref. This is intentional, so the placeholder carries no Schema Object semantics.The
AllOfVisitorthen checksallOf.content.every(isSchemaElement). That check rejects the very placeholders the resolver inserted, so any circular reference insideallOfproducesTypeError: Elements in allOf must be objectsand leavesallOfunmerged.This PR makes the
AllOfVisitor(in both theopenapi-3-1-swagger-clientandopenapi-3-2-swagger-clientstrategies) accept these circular-reference placeholders: non-semanticObjectElements containing$ref. They are then merged like any otherallOfitem. The result is a schema that keeps its$refalongside its sibling keywords, which matches how the OpenAPI 2.0/3.0 specmap resolver already behaves. Genuinely invalidallOfitems (e.g. primitives) are still reported as errors.Motivation and Context
Fixes #3858
Refs swagger-api/swagger-ui#10583 (same root cause, reported against Swagger UI 5.28.0)
Minimal reproduction from #3858:
A more realistic case from swagger-ui#10583, a self-referencing schema whose
allOfadds constraints:Before this change,
friends.itemsstayed as an unmergedallOfand an error was reported. After this change, it resolves to{ $ref: '…/User', type: 'object', required: ['id'] }with no errors.How Has This Been Tested?
Added
test/bugs/3858.js. It runs for both OpenAPI 3.1.0 and 3.2.0 and goes through the public API (SwaggerClient.resolveandSwaggerClient.resolveSubtree). The public API is needed here because the existing visitor unit tests use apidom's defaultcircular: 'ignore'mode and never hit the swagger-clientcircularReplacer.The tests cover:
allOfdefinition from OpenAPI 3.1.0: error when resolving schemas with allOf and circular references #3858, both as a full resolve and as a subtree resolve of each schema: no errors, andallOfis merged;allOffrom swagger-ui#10583, full and subtree: no errors, and sibling keywords (type,required) are retained;allOfstill producesElements in allOf must be objects.Without the fix, 12 of the 14 new tests fail. The two negative-case tests pass either way, as intended. With the fix, all 14 pass. The existing resolver test suite and the full unit test suite pass as well.
Screenshots (if appropriate):
N/A. The error shown in the screenshot in #3858 no longer appears.
Types of changes
package.json)Checklist: