You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: resolve external refs nested in array items of external schemas (#2409) - #2410
When an externalized schema has an array property whose items is a $ref to another external file, and that is the only $ref in the file, the items ref is left as a raw relative path (./schemas/Item.yaml) instead of #/components/schemas/Item. It also stays unresolved with resolveFully.
The second pass in ComponentsProcessor.processSchemas only reprocessed a model when one of its direct properties had a $ref, so a pending ref one level down (inside array items) was skipped. It worked only when the same file also had a bare property $ref, which triggered a reprocess of the whole model.
The second pass now reprocesses a model once when it has a pending external ref in its properties, items, additionalProperties or allOf/anyOf/oneOf, at any depth, including models without properties such as a top-level array or a bare allOf. not and discriminator mapping are left out: ExternalRefProcessor does not rewrite not refs relative to the external file, and the second pass does not write mapping values back, so reprocessing them would only fail or do nothing. A visited set keeps the check safe on recursive schemas. This also stops the model from being reprocessed once per property with a $ref.
Thanks for picking this up and adding the #2409 case as a test. I read through the change and two things stood out (from reading the code, not from running it):
ComponentsProcessor.java:261 only re-processes a model when model.getProperties() != null. An external schema whose top level is an array (type: array with items: {$ref: ./Item.yaml}) or a bare allOf/oneOf has no properties, so its nested relative refs still aren't resolved. The description says "at any depth", so either drop that gate or narrow the description.
hasUnresolvedExternalRef also descends into not (line 294), but ExternalRefProcessor never rewrites not refs (there's no getNot() in it). So for a schema in a subdirectory like schemas/Foo.yaml with properties.x.not.$ref: ./Other.yaml, the ref stays relative to schemas/. On master the second pass skips it and parsing finishes. With this PR the model gets re-processed, ./Other.yaml is looked up from the root, and I'd expect "Unable to load RELATIVE ref" plus the resolve stopping early. Skipping not in the check, or rewriting not in ExternalRefProcessor the same way as items/additionalProperties, would avoid that.
Smaller: the discriminator mapping is detected but processDiscriminatorSchema works on a temporary Schema, so the mapping values aren't actually rewritten. A test for a top-level array and one asserting getMessages() is empty would help catch both points above.
Thanks, all three points were right. I added tests for each one first, and they failed on the previous commit.
Dropped the getProperties() != null gate. A top-level array (ItemList.yaml) and an array inside a bare allOf (Composed.yaml) now resolve to #/components/schemas/Item.
Removed not from the check. schemas/Negated.yaml has properties.value.not.$ref: ./Item.yaml. With the previous commit it produced "Unable to load RELATIVE ref" and stopped the resolve early, which also broke the resolveFully test. Now the parse has no messages. Rewriting not in ExternalRefProcessor is a separate gap and I left it out of this PR.
Removed the discriminator mapping check. processDiscriminatorSchema does not write the mapping values back, so reprocessing for it did nothing.
Fixed in ebffc23. The full swagger-parser-v3 suite passes (704 tests). I updated the description to match.
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
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 an externalized schema has an array property whose
itemsis a$refto another external file, and that is the only$refin the file, theitemsref is left as a raw relative path (./schemas/Item.yaml) instead of#/components/schemas/Item. It also stays unresolved withresolveFully.The second pass in
ComponentsProcessor.processSchemasonly reprocessed a model when one of its direct properties had a$ref, so a pending ref one level down (inside arrayitems) was skipped. It worked only when the same file also had a bare property$ref, which triggered a reprocess of the whole model.The second pass now reprocesses a model once when it has a pending external ref in its properties,
items,additionalPropertiesorallOf/anyOf/oneOf, at any depth, including models withoutpropertiessuch as a top-level array or a bareallOf.notand discriminator mapping are left out:ExternalRefProcessordoes not rewritenotrefs relative to the external file, and the second pass does not write mapping values back, so reprocessing them would only fail or do nothing. A visited set keeps the check safe on recursive schemas. This also stops the model from being reprocessed once per property with a$ref.Fixes: #2409
Type of Change
Checklist
Screenshots / Additional Context
Issue2409Testuses the reproduction from the issue plus these variants:entries), to show the property name is not a factoritemsref to another schema of the same external fileallOfnotref in an external schema in a subdirectory, asserting the parse has no messagesresolveFully, where the items schema is now inlinedThe full
swagger-parser-v3test suite passes (704 tests).