Skip to content
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

feat: Add URL mappings #125

Merged
merged 2 commits into from
Apr 1, 2019
Merged

feat: Add URL mappings #125

merged 2 commits into from
Apr 1, 2019

Conversation

rhwood
Copy link
Contributor

@rhwood rhwood commented Mar 22, 2019

URL mappings allows public, normally internet accessible URLs (such as might be externally referenced in a schema) to be mapped to local URLs that might be on the filesystem, at a different external URL, or embedded within a JAR file.

This allows the JsonSchema validator to validate a schema with an external reference to validate against the local copy of that reference when the external reference is not available. Note that when using a mapping, the local copy is always used, and the external reference is not queried.

URL mappings allows public, normally internet accessible URLs (such as might be externally referenced in a schema) to be mapped to local URLs that might be on the filesystem, at a different external URL, or embedded within a JAR file.

This allows the JsonSchema validator to validate a schema with an external reference to validate against the local copy of that reference when the external reference is not available. Note that when using a mapping, the local copy is always used, and the external reference is not queried.
@rhwood
Copy link
Contributor Author

rhwood commented Mar 22, 2019

This includes tests that the URL mappings work, both in terms of being schema valid, and by referencing a non-existent URL.

@stevehu two questions:

  • Where should the schema for a JSON file containing mappings be published? 22efdfe uses https://raw.githubusercontent.com/networknt/json-schema-validator/master/src/main/resources/url-mapping.schema.json which, while being valid if this PR is merged from that commit without changes, feels like it should be published elsewhere (or use a different URL even if not published at that URL).
  • Should the mapping file src/test/resources/tests/url_mapping/url-mapping.json be included in sources, and not just tests, as a usable mapping file to avoid queries against the Draft 4 schema and the mapping schema?

Fixes #124

@stevehu stevehu changed the base branch from master to develop March 22, 2019 13:58
@stevehu
Copy link
Contributor

stevehu commented Mar 22, 2019

@rhwood I like the solution which gives users more flexibility for other schema remote references. Regarding the configuration, we have an external config file and config class already and wondering if you could use that instead. This will give users a consistent way for configuration and allow them to leverage the feature easily.

https://github.com/rhwood/json-schema-validator/blob/master/src/main/java/com/networknt/schema/SchemaValidatorsConfig.java

The config module we are using is part of the light-4j framework and it is tiny.

https://github.com/networknt/light-4j/tree/master/config

https://doc.networknt.com/concern/config/

- Add URL mappings to `SchemaValidatorsConfig`
- Remove methods to add mappings in `JsonSchemaFactory`
- Remove methods to add mappings from URL or JSON in `JsonSchemaFactory.Builder`
- Use a JSON mappings file and its schema in tests (both as a JSON object to be validated and to load schema locations that needs to be tested for redirection)
@rhwood
Copy link
Contributor Author

rhwood commented Mar 24, 2019

@stevehu 3c27e9b changes this to use a SchemaValidatorsConfig to provide the mapping. It also allows the JsonSchemaFactory.Builder to provide the mapping, merging the two mappings together if both a Builder and a Config provide the mapping.

I've removed mechanisms other than providing the mapping as a Map<URL, URL> to a SchemaValidatorsConfig or JsonSchemaFactory.Builder from the code.

@jiachen1120
Copy link
Contributor

jiachen1120 commented Mar 28, 2019

@stevehu Currently, we cannot use the config module inside the Json-schema-validator, since the Json-schema-validator is not depend on light-4j, but light-4j depend on Json-schema-validator.

Therefore, in order to make this url-mapping configurable, maybe we should load a config called "url-mapping.yml" in light-rest-4j/openapi-validator and forward this map into an overload method JsonSchema.validate(content, url-mapping).

Otherwise, we can use @rhwood's method, to parse a Json file of url-mapping (getUrlMappingsFromUrl() in test. Then let user provide a url-mapping.json If they want use external schema. In this way, no need to use config module.

What do you think?

@rhwood
Copy link
Contributor Author

rhwood commented Mar 30, 2019 via email

@stevehu
Copy link
Contributor

stevehu commented Apr 1, 2019

@jiachen1120 I got your point. For the light-4j frameworks, we can load single or multiple configuration files without any issue with light-4j config module. However, it would be very hard for other users to load config files if they are just using the json-schema-validator. I have reviewed SchemaValidatorsConfig and found that some of the attributes are useful for independent users. I would prefer to have a single config file like @rhwood implemented. This is the second option you have described above. Do you see any problem with this approach? If not, could you please approve the PR and I will merge it.

@stevehu stevehu merged commit e97d280 into networknt:develop Apr 1, 2019
@stevehu
Copy link
Contributor

stevehu commented Apr 1, 2019

@rhwood Thanks a lot for your help.

stevehu added a commit that referenced this pull request Apr 1, 2019
* Add link to Javadocs (#123)

* feat: Add URL mappings (#125)

* feat: Add URL mappings

URL mappings allows public, normally internet accessible URLs (such as might be externally referenced in a schema) to be mapped to local URLs that might be on the filesystem, at a different external URL, or embedded within a JAR file.

This allows the JsonSchema validator to validate a schema with an external reference to validate against the local copy of that reference when the external reference is not available. Note that when using a mapping, the local copy is always used, and the external reference is not queried.

* Add URL mappings to SchemaValidatorsConfig

- Add URL mappings to `SchemaValidatorsConfig`
- Remove methods to add mappings in `JsonSchemaFactory`
- Remove methods to add mappings from URL or JSON in `JsonSchemaFactory.Builder`
- Use a JSON mappings file and its schema in tests (both as a JSON object to be validated and to load schema locations that needs to be tested for redirection)
stevehu added a commit that referenced this pull request Apr 1, 2019
* Add link to Javadocs (#123)

* feat: Add URL mappings (#125)

* feat: Add URL mappings

URL mappings allows public, normally internet accessible URLs (such as might be externally referenced in a schema) to be mapped to local URLs that might be on the filesystem, at a different external URL, or embedded within a JAR file.

This allows the JsonSchema validator to validate a schema with an external reference to validate against the local copy of that reference when the external reference is not available. Note that when using a mapping, the local copy is always used, and the external reference is not queried.

* Add URL mappings to SchemaValidatorsConfig

- Add URL mappings to `SchemaValidatorsConfig`
- Remove methods to add mappings in `JsonSchemaFactory`
- Remove methods to add mappings from URL or JSON in `JsonSchemaFactory.Builder`
- Use a JSON mappings file and its schema in tests (both as a JSON object to be validated and to load schema locations that needs to be tested for redirection)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants