-
-
Notifications
You must be signed in to change notification settings - Fork 233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fails to resolve correct path/filename for extended $ref #200
Comments
I should also mention the issue described here: #168. Based on the draft 07 spec, it seems like the behavior of objects with $ref and additional properties appears to be undefined. I think the decision on how to proceed with this behavior going forward may inform the decision made for this issue as well. |
This feels like the same problem that I've had, but I'm not sure what an extended ref is! The problem I've had is that when it comes time to resolve (using your example):
... I've lost the context that these $refs are in So I've patched I'm still testing this myself, but it seems promising. Here's the patch if it sounds like it might address this issue for you. diff --git a/node_modules/@apidevtools/json-schema-ref-parser/lib/resolve-external.js b/node_modules/@apidevtools/json-schema-ref-parser/lib/resolve-external.js
index c7238fb..d1f1824 100644
--- a/node_modules/@apidevtools/json-schema-ref-parser/lib/resolve-external.js
+++ b/node_modules/@apidevtools/json-schema-ref-parser/lib/resolve-external.js
@@ -51,7 +51,7 @@ function resolveExternal (parser, options) {
* If any of the JSON references point to files that contain additional JSON references,
* then the corresponding promise will internally reference an array of promises.
*/
-function crawl (obj, path, $refs, options) {
+function crawl (obj, path, $refs, options, external) {
let promises = [];
if (obj && typeof obj === "object" && !ArrayBuffer.isView(obj)) {
@@ -59,16 +59,17 @@ function crawl (obj, path, $refs, options) {
promises.push(resolve$Ref(obj, path, $refs, options));
}
else {
+ if (external && $Ref.is$Ref(obj)) {
+ /* Correct the reference in the external document so we can resolve it */
+ let withoutHash = url.stripHash(path);
+ obj.$ref = withoutHash + obj.$ref;
+ }
+
for (let key of Object.keys(obj)) {
let keyPath = Pointer.join(path, key);
let value = obj[key];
- if ($Ref.isExternal$Ref(value)) {
- promises.push(resolve$Ref(value, keyPath, $refs, options));
- }
- else {
- promises = promises.concat(crawl(value, keyPath, $refs, options));
- }
+ promises = promises.concat(crawl(value, keyPath, $refs, options, external));
}
}
}
@@ -107,7 +108,7 @@ async function resolve$Ref ($ref, path, $refs, options) {
// Crawl the parsed value
// console.log('Resolving $ref pointers in %s', withoutHash);
- let promises = crawl(result, withoutHash + "#", $refs, options);
+ let promises = crawl(result, withoutHash + "#", $refs, options, true);
return Promise.all(promises);
} |
Possibly corrects APIDevTools#200. This however causes tests to fail as they’re not expecting the references to be resolved. I’m not sure yet how to fix the tests. I thought I’d wait to see if the fix is valid!
@daniellubovich Thanks for reporting this. I almost gave up until I saw the issue you created. I believe I run into the same issue here. Went with a workaround nesting the $ref inside an oneOf to keep the description. Not sure if there's a better way but it fixes the resolving issues I run into. {
"parameters": {
"description": "My parameter description ...",
"oneOf": [
{
"$ref": "./external.schema.json#/definitions/ParameterDefinition"
}
]
}
} |
@daniellubovich if I get the problem correctly, it seems to be a case of nested references to external files? That is: main file --- references ---> external file 1 --- references ---> external file 2 At least this seems to be the issue that I'm getting and guessing that this is the underlying problem. I'm in a similar position where I'd be happy to help but would appreciate some guidance on it. From my perspective it seems there is at least a need to be able to propagate referencing context such that at the time of dereferencing the correct value can be obtained from the correct reference location |
@btruhand if you get a moment, your issue sounds like the issue that I've resolved. I've published a version of |
@karlvr just got back to replying on this. I actually realized my problem is elsewhere (at least for the case that I was testing). Though I'll try to give your solution a whirl and see how it goes and get back with results for future reference |
@karlvr where can I find your published package? I am facing an issue where |
@benjamin-mogensen Right, hopefully it resolves the issue. I published it https://www.npmjs.com/package/@openapi-generator-plus/json-schema-ref-parser for interoperability with my https://www.npmjs.com/package/openapi-generator-plus. The source code is https://github.com/karlvr/json-schema-ref-parser. The master branch contains what's published to npm. I've also recently fixed the bundle task as well as I'm using that to convert schemas referencing external files into a single self-contained schema for use with tools that don't seem to support external references. (I've fixed the reference to where I'd published it above; I was publishing a few things at the time!) |
Thanks a lot for getting back on this. I tried using your package, bundled the schemas and then checked if the $ref -> $ref problem had been fixed. Unfortunately not so AJV still cannot compile the bundled schemas and I am forced to hard code a fix that replaces these "broken" paths in the $ref so that AJV can compile without errors. |
@benjamin-mogensen interesting; can you post an example file that it doesn't fix? My use case is resolved but I'm happy to look if this is related to what I've fixed. |
@karlvr thanks for your reply - The schemas I bundle are very big and takes many $ref which in turn has many other $ref so it is a bit difficult to make a simple example. I will see if I can one of the coming days though. The result I get (in a super stripped down version) is like the link I posted above. But I guess the end result will not help us much finding out if we can fix this in json-schema-ref-parser :) |
I am having this same issue. It's disappointing that it's still causing problems. If I have a CampaignResponse:
allOf:
- $ref: './Campaign.yml' # works just fine, no problem
required:
- id
- state
- candidates
properties:
id:
type: string
state:
type: string
revision:
type: number
publishedBy:
$ref: './ChangeLogEntry.yml' # ERROR! Can't find file! (even though it DEFINITELY exists in the specified location)
candidates:
type: array
items:
$ref: './Candidate.yml' As I'm sure you're all aware, this is incredibly frustrating because it's forcing me to have to duplicate data structures in multiple places just to get it to work properly. It shouldn't have to be this way. All explicit references should simply work. Any suggestions/comments/hacks would be warmly welcomed and greatly appreciated 🙏🏾 |
@y0n3r this isn't a simple thing, and lets not just stamp our feet and talk about being disappointed. All time spent on this is entirely voluntary, and I already run a reforestation charity with about 40 hours of my free time each week, so sometimes issues dont get seen to as fast as we'd all like. @karlvr thank you for submitting a pull request, I'll talk to you over there about getting it moving. Everyone else, if you're seeing this issue, please give https://www.npmjs.com/package/@openapi-generator-plus/json-schema-ref-parser a try and see if it fixes that issue. We'll get the changes merged upstream ASAP. |
Possibly corrects APIDevTools#200. This however causes tests to fail as they’re not expecting the references to be resolved. I’m not sure yet how to fix the tests. I thought I’d wait to see if the fix is valid!
I am having a similar issue with both packages. The folder structure looks like:
location-created.json {
"$schema": "http://json-schema.org/draft-07/schema",
"title": "LocationCreated",
"type": "object",
"properties": {
"data": {
"type": "object",
"properties": {
"id": {
"type": "string"
},
"name": {
"type": "string"
},
"physicalAddress": {
"$ref": "schemas/src/common/physical-address.json"
}
},
"required": ["id", "name", "physicalAddress"]
}
},
"required": ["data"]
} location-created.json {
"$schema": "http://json-schema.org/draft-07/schema",
"title": "PhysicalAddress",
"type": "object",
"properties": {
"streetNumber": {
"type": "string"
},
"streetName": {
"type": "string"
},
"streetDetails": {
"type": "string"
},
"city": {
"type": "string"
},
"state": {
"$ref": "enums/us-states.json"
},
"countryCode": {
"type": "string",
"enum": ["US"]
},
"postalCode": {
"type": "string"
}
},
"required": ["streetNumber", "streetName", "city", "state", "countryCode", "postalCode"]
} us-states.json {
"$schema": "http://json-schema.org/draft-07/schema",
"title": "USStates",
"type": "string",
"enum": [
"AL",
"AK",
"AZ",
"AR",
"CA",
"CO",
"CT",
"DE",
"FL",
"GA",
"HI",
"ID",
"IL",
"IN",
"IA",
"KS",
"KY",
"LA",
"ME",
"MD",
"MA",
"MI",
"MN",
"MS",
"MO",
"MT",
"NE",
"NV",
"NH",
"NJ",
"NM",
"NY",
"NC",
"ND",
"OH",
"OK",
"OR",
"PA",
"RI",
"SC",
"SD",
"TN",
"TX",
"UT",
"VT",
"VA",
"WA",
"WV",
"WI",
"WY",
"DC",
"AS",
"GU",
"MP",
"PR",
"UM",
"VI"
]
} In path from base dir{
"physicalAddress": {
"$ref": "schemas/src/common/physical-address.json"
}
} Which seems to duplicate the root directory and gives the error:
path from dir where
|
I'll add that I think I'm seeing the same issue when parsing an external doc that references its own definitions relatively via https://raw.githubusercontent.com/CycloneDX/specification/master/schema/bom-1.4.schema.json fails with My workaround is to fully qualify the internal references in
Update: I take this back, I believe the my problem is due to an incompatibility with another change in the downstream |
@JamesMessinger @P0lip has anyone looked into this? Maybe PR #220 is good? Seems like a big issue affecting a lot of people. |
Hi there, can someone help to write tests for this feature? I've tested with There is a starting point for the tests here: e121717 (I've added a branch with changes from this PR on top of some changes done for ISSUE-283 for tests on Windows.) |
I had this issue and was able to implement a quick fix by adding the |
Yup wanted to thank you @philsturgeon for your work. |
I recently ran into an issue testing some of my more complex schema documents where combination schema $refs cannot be resolved properly if they are part of an "extended" $ref. It is a little hard to explain the case, but I think I have found a simple enough schema that causes the problem to occur.
The error that I see when trying to resolve this looks like:
Error while processing route: dynamic-form Token "cstring" does not exist. MissingPointerError: Token "cstring" does not exist.
This only seems to happen when the intermediate schema object is an "extended" ref. For example, if we remove the description from
astring
then we are able to dereference without issue. From doing some debugging it looks to me like our issue is that when resolvingcstring
, the path we resolve isdefs.json#/definitions/cstring
instead of looking indefs2.json
. I think this is because when we resolve extended $refs, we explicitly do not update the pointer's path.For reference:
json-schema-ref-parser/lib/pointer.js
Lines 240 to 245 in 13b0092
I would like to propose a fix, but feel ill-equipped to do so without some guidance. It does seem like updating the path in the above code fixes my specific issue, but clearly that decision was made for a reason so I'd rather not step on any toes without understanding the intent. Any thoughts would be greatly appreciated!
The text was updated successfully, but these errors were encountered: