JSON-Schema-Test-Suite icon indicating copy to clipboard operation
JSON-Schema-Test-Suite copied to clipboard

Test for valid use of empty fragment in "$id"

Open handrews opened this issue 6 years ago • 4 comments
trafficstars

  • [ ] $id with trailing empty fragment equivalent to $id without a fragment
  • [ ] "$id": "#" is a no-op

handrews avatar Nov 11 '19 20:11 handrews

I don't think either of these test cases ended up being covered by the final version of #341 (but the first one might be covered elsewhere).

karenetheridge avatar Jun 19 '20 18:06 karenetheridge

Both of these cases are implemented.

However, I don't remember why "$id": "#" is a no-op is a test case. Here is the case as implemented:

            {     
                "description": "Identifier name with base URI change in subschema",
                "data": {
                    "$id": "http://localhost:1234/root",
                    "$ref": "http://localhost:1234/nested.json#/$defs/B",
                    "$defs": {
                        "A": {
                            "$id": "nested.json",
                            "$defs": {
                                "B": {
                                    "$id": "#",
                                    "type": "integer"
                                }         
                            }             
                        }             
                    }             
                },            
                "valid": true
            }         

The problem here is that this identifies both A and B as http://localhost:1234/nested.json, which makes the reference ambiguous. I think what I meant was using "$id": "#" in the document root, in which case the schema URI is the retrieval URI, which is exactly what happens if you don't have $id at all. So I think that's the "no-op" I meant. But I don't think the test suite can set retrieval URIs so this weird case is probably un-testable.

@Julian would you be OK with me making a PR to delete that case across all relevant drafts? If there's a way to set an external base URI I can replace it with the document root one, but otherwise this is more-or-less equivalent to having two identical $anchor values, which the spec notes has undefined behavior (see the last paragraph of the linked section).

handrews avatar Aug 12 '22 04:08 handrews

If the test is wrong yes definitely a PR to remove would be appreciated!

We have a way to communicate retrieval URIs, it's basically by putting the remote at a corresponding path inside remotes/ -- so putting foo.json in there is saying the retrieval URI is http://localhost:1234/foo.json even when it doesn't contain a $id (in fact some tests I have for a PR shortly are about noticing we don't test cases where that retrieval URI is different from the one in $id)

But yeah I think it sounds like that should work for this scenario?

Julian avatar Aug 12 '22 08:08 Julian

@Julian sounds good, I'll make a PR for that.

The test as it stands runs afoul of §9.1.2 "Loading a referenced schema" (last paragraph):

A schema MAY (and likely will) have multiple IRIs, but there is no way for an IRI to identify more than one schema. When multiple schemas try to identify as the same IRI, validators SHOULD raise an error condition.

(I've filed json-schema-org/json-schema-spec#1271 to consolidate this and §8.1.2 which I linked in the comments, since one of them has a MAY around raising an error and the other a SHOULD).

So it's not wrong to pass that test as written, but it would be equally correct to fail (resolving http://localhost:1234/nested.json#/$defs/B to be equivalent to http://localhost:1234/root#/$defs/A/$defs/B/$defs/B instead of http://localhost:1234/root#/$defs/A/$defs/B. And it would actually be more correct to error out upon schema load.

Unless the test suite can test for error conditions, in which case we could make that an optional test for raising an error, it should just be removed.

handrews avatar Aug 12 '22 14:08 handrews