Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Access schema include paths #140

Closed
justjacksonn opened this issue Mar 19, 2015 · 19 comments · Fixed by #176
Closed

Access schema include paths #140

justjacksonn opened this issue Mar 19, 2015 · 19 comments · Fixed by #176

Comments

@justjacksonn
Copy link

Need to add the ability to get the actual !include path for schemas.

@dmartinezg
Copy link

Hi @justjacksonn I am not sure what this issue is about, can you clarify?

@blakeembrey
Copy link
Contributor

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

@mbixby
Copy link

mbixby commented Aug 11, 2015

+1

@blakeembrey
Copy link
Contributor

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.

Edit 2: https://www.npmjs.com/package/json-schema-deref

@blakeembrey blakeembrey self-assigned this Aug 11, 2015
@mbixby
Copy link

mbixby commented Aug 11, 2015

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.

@blakeembrey
Copy link
Contributor

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

@martnst
Copy link

martnst commented Jan 6, 2016

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

@przezor
Copy link

przezor commented Mar 9, 2016

+1

@alvassin
Copy link
Contributor

alvassin commented Apr 18, 2016

@blakeembrey +1 blocked by this issue. Any progress on this?

@alvassin
Copy link
Contributor

@martnst How you were able to achieve that outside of raml-js-parser?

@alvassin
Copy link
Contributor

alvassin commented Apr 18, 2016

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

@martnst
Copy link

martnst commented Apr 18, 2016

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.

@blakeembrey
Copy link
Contributor

@alvassin I do not work on this project anymore, @dmartinezg should be able to help you out. It sounds reasonable though.

@dmartinezg
Copy link

@alvassin, yes, if you can add the feature flag, we would be able to accept the patch.

@dmartinezg
Copy link

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 ?

@alvassin
Copy link
Contributor

@dmartinezg Thanks for suggestion! I'll update code and add additional tests 👍

@alvassin
Copy link
Contributor

alvassin commented Apr 20, 2016

@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?

@dmartinezg
Copy link

As long as all !included files have been already de-referenced and parsed, I don't see an issue with cwd (I think)

@alvassin
Copy link
Contributor

alvassin commented Apr 21, 2016

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants