Skip to content

Fix for ignoring the result of scheme mapping #1157

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TotenTraum
Copy link

When loading a schema that uses the "$schema" property set to "http://json-schema.org/schema#" that references the latest version of the specification, the following exception is thrown

java.io.FileNotFoundException: classpath:schema

Afterwards I decided to add mapping for such a case, but it still throws the same exception

final VersionFlag versionFlag = VersionFlag.V7;
final String schemaIri = JsonSchemaFactory.checkVersion(versionFlag).getInstance().getIri();

final JsonSchemaFactory jsonSchemaFactory = JsonSchemaFactory.getInstance(versionFlag, builder ->
     builder.schemaMappers(schemaMappers -> schemaMappers.mappings(Map.of("http://json-schema.org/schema", schemaIri)))
);

It turned out that the code of the DefaultSchemaLoader class ignores the mapping result

...
AbsoluteIri mappedResult = absoluteIri; <-----
for (SchemaMapper mapper : schemaMappers) {
    AbsoluteIri mapped = mapper.map(mappedResult);
    if (mapped != null) {
         mappedResult = mapped;
       }
    }
    AbsoluteIri mapped = META_SCHEMA_MAPPER.map(absoluteIri); <----
    if (mapped != null) {
        mappedResult = mapped;
    }
...

@justin-tay
Copy link
Contributor

Just a note that the use of http://json-schema.org/schema# to refer to the latest version is deprecated and should not be used. See https://json-schema.org/understanding-json-schema/reference/schema. Dialects are not backward compatible so you may get unexpected results.

That said you need to add a unit test for the pull request for this to be reviewed if this is still something you want.

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.

2 participants