Skip to content

fix: resolve external refs nested in array items of external schemas (#2409) - #2410

Open
vinirib wants to merge 5 commits into
swagger-api:masterfrom
vinirib:issue-2409
Open

vinirib wants to merge 5 commits into
swagger-api:masterfrom
vinirib:issue-2409

Conversation

@vinirib

@vinirib vinirib commented Oct 29, 2026 •

Copy link
Copy Markdown

Description

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.

Fixes: #2409

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Refactor (non-breaking change)
  • 🧪 Tests
  • 📝 Documentation
  • 🧹 Chore (build or tooling)

Checklist

  • I have added/updated tests as needed
  • I have added/updated documentation where applicable
  • The PR title is descriptive
  • The code builds and passes tests locally
  • I have linked related issues (if any)

Screenshots / Additional Context

Issue2409Test uses the reproduction from the issue plus these variants:

  • the array property renamed (entries), to show the property name is not a factor
  • an items ref to another schema of the same external file
  • a top-level array schema and an array inside allOf
  • a not ref in an external schema in a subdirectory, asserting the parse has no messages
  • resolveFully, where the items schema is now inlined

The full swagger-parser-v3 test suite passes (704 tests).

@xiaoyu2er

Copy link
Copy Markdown

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):

  1. 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.

  2. 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.

@vinirib

vinirib commented Nov 13, 2026

Copy link
Copy Markdown
Author

Thanks, all three points were right. I added tests for each one first, and they failed on the previous commit.

  1. 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.
  2. 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.
  3. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Array items $ref to an external file is not resolved to #/components/schemas/... when it's the only $ref in the referencing file

2 participants