Test relative reference resolution when ID is not present

[matt-allan]

This PR adds a test to ensure that relative references are resolved against the URI of the current schema, as outlined in section 7.1 of the specification and section 4 of the JSON Reference specification.

The initial resolution scope of a schema is the URI of the schema itself, if any, or the empty URI if the schema was not loaded from a URI.

If the URI contained in the JSON Reference value is a relative URI,
then the base URI resolution MUST be calculated according to
[RFC3986], section 5.2. Resolution is performed relative to the
referring document.

We currently have the "change resolution scope" test which uses a relative reference, but because the schema has an ID it does not prove that an implementation uses the current schema's URI as the initial resolution scope.

[epoberezkin]

lgtm, can somebody else review as well? @Relequestual @handrews @Julian

[epoberezkin]

@handrews we both approved it and I merged it, but I think we've done it wrong.

The convention in these tests is that the files are available at http://localhost:1234, so subSchemas.json full URI is http://localhost:1234/subSchemas.json. But subschemas.json doesn't have id in it, so there is no base URI. I don't think that the file is available at http://localhost:1234 should make it its base URI if there is no id attribute - I think for this test to be correct id attribute should be added to it. Do you agree?

[epoberezkin]

And it also broke build. I will revert it.

[matt-allan]

Hey @epoberezkin,

I don't think that the file is available at http://localhost:1234 should make it its base URI if there is no id attribute

That is what I interpreted this part as saying:

The initial resolution scope of a schema is the URI of the schema itself, if any, or the empty URI if the schema was not loaded from a URI.

The qualifier 'or the empty URI if the schema was not loaded from a URI' seems like it implies the initial 'URI of the schema itself' is the URI it was loaded from if it was loaded from a URI.

Maybe I should open an issue at the json-schema-spec repo?

[handrews]

If we fetch the file from http://localhost:1234/subSchemas.json then per RFC 3986 that is the document's base URI. We should be testing that implementations use the correct base URI in this situation, in which case we cannot include an "id"/"$id" for such a test. Is that what we're testing here?

The "empty URI if the schema was not loaded from a URI" is from the JSON Reference spec which is relevant to Draft 04 but not to later drafts (which don't seem to have that language). In RFC 3986 §5.1 terms, this would be the application-specific default base URI.

[matt-allan]

Is that what we're testing here?

Yep, exactly. My validator passed the test suite but did not set a base URI, so relative references did not work if the schema did not include an id. All of the existing relative reference tests set an id, so this scenario is never encountered.

[epoberezkin]

On another hand, the spec says that ID is schema URI, but ID doesn't mean it can be downloaded there. See #114 (comment)

So I am not sure. @Julian's validator failed this test too, that's why I reverted.

If we decide that it should work this way as this test required, the validator should be fixed before we can add this test.

[handrews]

@epoberezkin this is not ambiguous. From RFC 3986 section 5.1:

     .----------------------------------------------------------.
     |  .----------------------------------------------------.  |
     |  |  .----------------------------------------------.  |  |
     |  |  |  .----------------------------------------.  |  |  |
     |  |  |  |  .----------------------------------.  |  |  |  |
     |  |  |  |  |       <relative-reference>       |  |  |  |  |
     |  |  |  |  `----------------------------------'  |  |  |  |
     |  |  |  | (5.1.1) Base URI embedded in content   |  |  |  |
     |  |  |  `----------------------------------------'  |  |  |
     |  |  | (5.1.2) Base URI of the encapsulating entity |  |  |
     |  |  |         (message, representation, or none)   |  |  |
     |  |  `----------------------------------------------'  |  |
     |  | (5.1.3) URI used to retrieve the entity            |  |
     |  `----------------------------------------------------'  |
     | (5.1.4) Default Base URI (application-dependent)         |
     `----------------------------------------------------------'

If there is an id, then we are in the situation marked "(5.1.1) Base URI embedded in content". If we retrieved the JSON Schema document via a URI, then "(5.1.3) URI used to retrieve the entity". The only case where there is anything for the JSON Schema spec to decide is if we did not use a URI to retrieve the document, and there is no URI in the document.

@epoberezkin, it doesn't matter whether the document can be fetched at the "$id" URI, because in that situation we already have the URI within the document. If we have a "$ref" URI that we used to fetch the referenced schema, which presumably we do because we're testing the use of "$ref", then that is the URI used to retrieve the entity.

[epoberezkin]

@handrews ok, I have no objection to that. I also thought it should work so I merged it. But then it turned out the validator we have here to test the tests doesn't support it - that was the main reason to revert it, as otherwise we'd lose the ability to test anything else.

So if @Julian could implement the support for it we can re-add it. Or we can reduce what Julian's validator tests to draft-03 and test the rest with Ajv (actually not 100% sure if it supports it but I will fix if not).

Just let me know how you want to proceed. In any case, having broken tests is not an option I think.

[Julian]

I haven't been following this issue but if there's something we think that's valid I definitely recommend merging it, we can make the tests pass again even if my library doesn't yet conform.

[handrews]

It seems clear to me that the library conformance is what is broken, so we should fix that (thanks for supporting this, @Julian ). We should not remove tests because the validator missed a (rather obscure) part of the spec.