Access schema include paths
justjacksonn opened this issue · 19 comments
Need to add the ability to get the actual !include path for schemas.
Hi @justjacksonn I am not sure what this issue is about, can you clarify?
Hi @dmartinezg! I asked him to log the issue and it was about being able to get the original !include path from a schema lookup. For example, schema: !include schemas/user.json loses the reference to user.json file and we can no longer do things with the original file (including validating that the $refs load/exist, etc).
Edit: Maybe we can look at a higher level mode for exposing the refs in a transparent object instance that will still JSON stringify to the same structure? It'd also be useful for getting the local reference which are lost (e.g. schema: schemaName).
+1
Side note: I think the best way to keep backwards compatibility with the current parser would be to have the parser expand JSON schemas inline. It wouldn't require much of a change to support either.
Edit: In fact, first result for "json expander" is https://github.com/br-adam-newman/raml-jsonschema-expander.
Thanks for the links, I think I'll just override fetchLocalFileAsync in raml.coffee to get it working until there's a final decision.
By the way, with this kind of preprocessing the underlying issue is still there, now you're not only hiding !include paths but also hiding $ref paths from the included schemas (mutating the schemas in the process).
I'd be okay if there was some new schemaUri attribute pointing to a virtual location that the user has to understand. You could then have some simple default method of resolving this virtual location into the full schema (like just using file paths) and provide option to override the behavior with something more advanced (json-schema-deref or some server). Resolved schema would be still under the schema attribute.
@mbixby I definitely understand and it's something that will be fixed in future parsers. Keeping backward compat right now doesn't give us much choice until the next major release which will break the current parser. I think this is the lesser of two evils, for now.
+1
What's the current progress on this? I am currently struggling with schema $ref not getting resolved. Looks like https://github.com/bojand/json-schema-deref-sync (also mentioned in #155) is the perfect library to resolve this issue.
Update: I was able to enhance my schemas using json-schema-deref-sync outside of raml-js-parser in less than an hour of work.
+1
@blakeembrey +1 blocked by this issue. Any progress on this?
@dmartinezg @blakeembrey I have to keep my schemas (i have a lot!) in different folders, so this issue is very important for me.
I can add some additional setting resolveReferencedSchemas, which would cause raml-js-parser to return expanded schemas (with substituted $refs to referenced schemas). Is this solution acceptable?
I can make pull request & tests with resolving referenced schemas without changing current interface of parser, so no one will be affected.
Please let me know if it is that acceptable solution. If it is, i will make pull request.
I have a rather complex setup include forks of dependencies here and there. Basically I have a node script that wraps everything up. It's all in the context of raml2hml. Therefor I am not sure my solution would help.
@alvassin I do not work on this project anymore, @dmartinezg should be able to help you out. It sounds reasonable though.
@alvassin, yes, if you can add the feature flag, we would be able to accept the patch.
I don't think it would be that simple to do while loading the document, because at that stage, the actual root schema key may not even be loaded, or may be included in another file. I think that this approach will fail for including root schemas.
Instead of doing this step in the YAML loading (in raml.coffee), it could be done as a composing step, in the composeRamlTree function at https://github.com/raml-org/raml-js-parser/blob/master/src/composer.coffee#L48 , where at that stage, you already have the YAML document read and validated, and can potentially access the entire AST by the root node.
The only remaining issue is handling the async nature of dereferencing the files, so maybe, the bit that loads the JSON files can remain at the async level, and the de-referencing can be done at the composing layer.
Makes sense @alvassin ?
@dmartinezg Thanks for suggestion! I'll update code and add additional tests 👍
@dmartinezg I implemented one more stage, that works after all fragment files are loade, but before composer (i would like to provide to composer ready object model for further validation & processing) - by adding one more then (fullyAssembledTree) callback.
Currently all cases are fine except inline defined schemas with $refs: process.cwd() usually differs from raml file path, so json-schema-ref-parser tries to find $referenced files in wrong place.
E.g. for call parser.load('tests/raml-files/expand-schemas.raml') process.cwd() will return /Users/alvassin/Sites/raml-js-parser, but raml file is located at /Users/alvassin/Sites/raml-js-parser/tests/raml-files/expand-schemas.raml
I found issue, where is said that json-schema-ref-parser itself does not provide ability to pass path for resolving $refs. In this issue it was suggested to use process.cwd as temporary solution.
I could change process.cwd (only) for the dereferencing stage, and then returning its value back. Is that acceptable?
As long as all !included files have been already de-referenced and parsed, I don't see an issue with cwd (I think)
The problem is not with !include-ed files (they work fine), but with inline-defined schemas, that contain $ref-s with relative paths (relative to raml file).