Skip to content

test: A $dynamicRef without anchor in fragment behaves identical to $ref #651

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

Merged
merged 1 commit into from
Mar 6, 2023
Merged

test: A $dynamicRef without anchor in fragment behaves identical to $ref #651

merged 1 commit into from
Mar 6, 2023

Conversation

santhosh-tekuri
Copy link
Contributor

from https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-8.2.3.2:

If the initially resolved starting point URI includes a fragment that was 
created by the "$dynamicAnchor" keyword, the initial URI MUST be r
eplaced by the URI (including the fragment) for the outermost schema 
resource in the dynamic scope that defines an identically named fragment 
with "$dynamicAnchor"

Otherwise, its behavior is identical to "$ref", and no runtime resolution is needed.

please ensure that my understanding is correct and the test case is valid

@santhosh-tekuri santhosh-tekuri requested a review from a team as a code owner March 4, 2023 19:33
@karenetheridge
Copy link
Member

My implementation errors on this:

evaluation generated an exception: {
  "error": "EXCEPTION: uri \"https://test.json-schema.org/typical-dynamic-resolution/list\" conflicts with an existing schema resource",
  "instanceLocation": "",
  "keywordLocation": ""
}

Please use a unique identifier for each new resource.

@santhosh-tekuri
Copy link
Contributor Author

santhosh-tekuri commented Mar 5, 2023

@karenetheridge corrected $id for uniqueness

@gregsdennis
Copy link
Member

Do we not already have a test for this? (Not at my computer right now) I seem to recall having something.

@gregsdennis
Copy link
Member

gregsdennis commented Mar 6, 2023

Just checking the current suite, we have

  • A $dynamicRef without a matching $dynamicAnchor in the same schema resource behaves like a normal $ref to $anchor
  • A $dynamicRef with a non-matching $dynamicAnchor in the same schema resource behaves like a normal $ref to $anchor
  • A $dynamicRef that initially resolves to a schema without a matching $dynamicAnchor behaves like a normal $ref to $anchor

These all declare $dynamicAnchor, but they're out of the dynamic scope. I'd say that covers this case, though indirectly.

@santhosh-tekuri
Copy link
Contributor Author

my impl https://github.com/santhosh-tekuri/boon passes all existing test cases, but it does not pass this test case.

below is the logic in my implementation validator.rs#L567-L574:

  1. find the initial target.
  2. if the initial target has $dynamicAnchor search for override in dynamic scope
  3. otherwise use initial target.

in step 1, it resolves to #/$defs/list/$defs/items from root resource.
this initial target has $dynamicAnchor property. so it searches for override in dynamic scope

since my impl does not pass this test, I wanted to confirm whether my understanding of spec is correct or not

@jdesrosiers
Copy link
Member

Do we not already have a test for this?

That was my first reaction as well, but I checked and this does appear to be unique. This is the only test that uses a JSON Pointer in a dynamic reference.

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works in my implementation.

@gregsdennis
Copy link
Member

This is the only test that uses a JSON Pointer in a dynamic reference. - @jdesrosiers

Should we refocus the test, then? If we're checking its functionality using a pointer, it should be focused on that, not that $dynamicRef behaves like $ref.

@jdesrosiers
Copy link
Member

If we're checking its functionality using a pointer, it should be focused on that, not that $dynamicRef behaves like $ref.

The functionality when using a pointer is that $dynamicRef will behave like $ref. I think the test is fine.

Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers okay.

Passes on mine, too.

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.

4 participants