tristanpenman / valijson

Header-only C++ library for JSON Schema validation, with support for many popular parsers
BSD 2-Clause "Simplified" License
350 stars 105 forks source link

Support for URN document references #132

Closed rayvincent2 closed 3 years ago

rayvincent2 commented 3 years ago

I'm currently unable to have valijson reference a document by a urn.

tristanpenman commented 3 years ago

~Hi @rayvincent2 are you able to provide some more context for the problem? If possible an example of how you're using the code could help.~

Just saw that you've raised a PR. Thanks for the contribution! I'll try to test and review that ASAP.

rayvincent2 commented 3 years ago

Thanks, apologies that the issue I raised was devoid of the exact use cases that I was running into. I supposed I was leaning on my PR to do that for me. Let me know if you have any questions.

The crux of the problem that I ran into is that the fetchDocument method passed to the valijson::SchemaParser::populateSchema method was not being called unless the value of $ref was either an absolute http uri or a relative file path. The way we're adding $refs to our json docs is to include the unique urns for the json schemas since our application needs to run offline and we don't want to have our json schemas have any knowledge of file structure. We want the applications to be responsible for knowing the file structure and being able to fetch the appropriate local document based on the unique urn.

Adding support for urns makes your library the one stop shop for us!!

The only doubts that I have about the PR are ensuring that the regex handles all cases of the RFC 8141

I spent some time creating the regex to validate a URN from the syntax in the RFC 8141 spec: https://datatracker.ietf.org/doc/html/rfc8141#section-2

Here's my regex tests that I used to validate the regex I created: https://regex101.com/r/hxoRGh/1

rayvincent2 commented 3 years ago

I found a much better regex expression for a URN which is much better than the one I crafted. Here it is with plenty of examples: https://regex101.com/r/ZSOWyt/1

tristanpenman commented 3 years ago

Going to close this issue, since your changes have been merged at ad7dac7.

Thanks for taking the time to add this feature!