Normalize relative ref paths to avoid duplicating schemas #2105
Conversation
ctreatma
force-pushed
the
collapse-equivalent-refs
branch
from
6f1d634 to
18d5450
Compare
modules/swagger-parser-v3/src/test/resources/oas3.fetched/components/parameters/Exclude.yaml
Outdated
Show resolved
Hide resolved
|
Just ran into this issue today. Glad to see a fix is on the way (hopefully soon). Thanks! |
ctreatma
force-pushed
the
collapse-equivalent-refs
branch
2 times, most recently
from
d25dc19 to
e2fd955
Compare
ctreatma
force-pushed
the
collapse-equivalent-refs
branch
from
e2fd955 to
8174b8f
Compare
|
I rebased on the latest from |
ctreatma
force-pushed
the
collapse-equivalent-refs
branch
from
8174b8f to
47a4931
Compare
ctreatma
force-pushed
the
collapse-equivalent-refs
branch
from
47a4931 to
26e34ae
Compare
| @@ -92,6 +93,20 @@ public String processRefToExternalSchema(String $ref, RefFormat refFormat) { | |||
| return renamedRef; | |||
| } | |||
|
|
|||
| RefFormat format = computeRefFormat($ref); | |||
| if (format.equals(RefFormat.RELATIVE)) { | |||
| String normalizedRef = "./" + Paths.get($ref).normalize().toString(); | |||
There was a problem hiding this comment.
To minimize impact on existing tests, this normalizes with a leading ./. It might be preferable to leave that out in normalized paths, but that would require changes to a number of tests that currently explicitly check for paths starting with ./.
There was a problem hiding this comment.
It looks like this matches existing behavior so should be OK as-is; the deserializer adds a leading ./ to relative references.
Given the existing behavior, an alternative approach to fixing this path resolution issue might be to normalize the relative path in mungeRef instead, although I'm not clear if that change would have to happen in only one or in both of OpenAPIDeserializer.mungeRef and RefUtils.mungeRef.
There was a problem hiding this comment.
Update to use RefUtils.mungeRef; I tried moving the path normalization elsewhere and it didn't work, so that approach will need more investigation (for me, at least).
Currently, if an OpenAPI spec contains multiple relative refs to the same file, but those refs are located in different files and use different relative paths to reach the one file, swagger-parser will create a separate, duplicate schema for each relative path rather than reusing the same schema across all equivalent paths.
For example, given a spec with the following refs:
$ref: ./components/schemas/Thing.yaml$ref: ../../components/schemas/Thing.yamlThe parser will produce a
Thingand aThing_1schema object instead of reusingThingfor the second, equivalent reference.This updates the ref processor to resolve relative paths before processing relative refs in order to produce a single
Thingschema that is reused for all equivalent references.Fixes #2016, fixes #1518, and maybe others.