Skip to content

Test dynamic behavior of "$recursiveRef" and "$recursiveAnchor" #298

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

Closed
2 tasks
handrews opened this issue Nov 11, 2019 · 12 comments
Closed
2 tasks

Test dynamic behavior of "$recursiveRef" and "$recursiveAnchor" #298

handrews opened this issue Nov 11, 2019 · 12 comments
Labels
missing test A request to add a test to the suite that is currently not covered elsewhere.
Milestone

Comments

@handrews
Copy link
Contributor

Test where "$recursiveRef" targets a "$recursiveAnchor": true schema and there is also an outer scope "$recursiveAnchor": true to which the $recursiveRef should actually resolve.

Should probably have at least two cases:

  • the outer scope is the entry point (as with our multi-vocabulary meta-schemas that include the applicator vocabulary)
  • the outer scope is intermediate (such as when using links.json, the schema for hyper-schema's Link Description Object, as the entry point).
@handrews handrews added this to the 2019-09 milestone Nov 11, 2019
@Julian Julian added the missing test A request to add a test to the suite that is currently not covered elsewhere. label Nov 29, 2019
@karenetheridge
Copy link
Member

karenetheridge commented May 29, 2020

I don't know if we want to do this for draft2019-09, given that the semantics of these keywords are changing soon, but we should also test what happens with this schema:

{
  "$id": "http://localhost:1234",
  "type": "object",
  "properties": {
    "myprop": {
      "$recursiveAnchor": true,
      "anyOf": [
        { "type": "string" },
        {
          "type": "object",
          "additionalProperties": { "$recursiveRef": "#" },
        }
      ]
    }
  }
}

..and also the more interesting case where $recursiveRef is not "#", but the fragment is not empty (either a json pointer or a plain-name fragment serves equally well for this case) -- the fragment has to be resolved against the uri marking the position of the $recursiveAnchor, but there is no $id there, so how do we perform resolution? I suspect that with the way the spec is written now, this can only blow up, since we have no provision for "resolve one fragment against the other by concatenating them together" or anything like that.

(@handrews I haven't read the new keyword proposals in great detail yet so I don't know if this case has been considered yet.. I just mention it without prejudice in the sense that this sort of scenario should be covered too.)

@handrews
Copy link
Contributor Author

@karenetheridge in the example schema you give, I believe the $recursiveRef functions just as $ref, because there is no $recursiveAnchor located at the initial target (the root schema). The presence of $recursiveAnchor anywhere else is irrelevant.

Regarding fragments, this is one reason why $recursiveRef MUST target a resource root schema. In URI reference resolutions, fragments are replaced, never combined.

@karenetheridge
Copy link
Member

karenetheridge commented May 30, 2020

Regarding fragments, this is one reason why $recursiveRef MUST target a resource root schema.

Sorry for being obtuse, but there's a few weird edge cases here and I want to completely understand. Does this mean that the final target for a $recursiveRef must be a resource root schema (which I believe means there MUST always be an $id keyword at the same location)? Or that the intermediary target (i.e. the location of the outermost $recursiveAnchor) must be a resource root schema?

For example, is this legal?

{
  "$id": "foo.json",
  "$recursiveAnchor": true,
  "$defs": {
    "my_def" : { ... },
  },
 ...

  ..and somewhere else in the schema:
  { "$recursiveRef": "#/defs/my_def", ... }
}

This would mean that the $recursiveRef is always targeting a definition with a specific name within the referenced resource, but that referenced resource could change depending on which $recursiveAnchor is in effect at the source of the $recursiveRef.

At the moment, I am throwing an exception on the combination of: $recursiveRef has a non-empty fragment, AND the canonical uri of the active $recursiveAnchor has a fragment (which would be any subschema that lacks an $id).

I tried for a little bit to concoct a test case with $recursiveRef having a non-empty path in the URI, and had to lie down from dizziness. :/

@handrews
Copy link
Contributor Author

Does this mean that the final target for a $recursiveRef must be a resource root schema (which I believe means there MUST always be an $id keyword at the same location)? Or that the intermediary target (i.e. the location of the outermost $recursiveAnchor) must be a resource root schema?

both.

@handrews
Copy link
Contributor Author

AND the canonical uri of the active $recursiveAnchor has a fragment (which would include things with an $anchor).

The presence of $anchor alongside $recursiveAnchor is irrelevant. # is a JSON Pointer fragment. Of course this all changes in the next draft, and I'm hoping that since OAS 3.1 will be on the next draft, people will deprecate 2019-09 and not use it in production at all.

@karenetheridge
Copy link
Member

I guess I'll not worry too much about $recursiveAnchor and $recursiveRef in 2019-09 then :) I got the multi-step resolution working, although I had difficulty coming up with some good tests that went beyond just using "#" in $recursiveRef.

@epoberezkin
Copy link
Member

Hi all. Am I right that there will be no tests for recursiveRef? And that it will be dynamicRef? Is there any timeline when it would stabilize?

@jdesrosiers
Copy link
Member

@karenetheridge wrote some tests for $recursiveRef/$recursiveAnchor #391. I'm not sure why progress on that PR stalled out, but it would be great to see that finished and merged.

$recrusiveRef/$recursiveAnchor is set in stone in draft 2019-09. It's not going to change other than that they will be replaced with $dynamicRef/$dynamicAnchor in the next draft. At this point there's no reason to expect any additional changes to $dynamicRef/$dynamicAnchor before the next draft is published.

@epoberezkin
Copy link
Member

epoberezkin commented Oct 28, 2020

I haven't read the next one yet - is it just renaming them? Or do they also extend "recursive" in some way?

@jdesrosiers
Copy link
Member

It's more than a rename. $dynamicAnchor is a string ({ "$dynamicRef": "foo" }) instead of a boolean ({ "$recursiveRef": true }) and is allowed anywhere in the schema instead of only at the top level. $dynamicRef works pretty much the same way as $recursiveRef except that it can reference a specific $dynamicAnchor ({ "$dynamicRef": "#foo" }) rather than only a top level schema ({ "$recursiveRef": "#" }).

This change makes it so $refing an $anchor works exactly like $dynamicRefing a $dynamicAnchor except for the dynamic resolution part.

@handrews
Copy link
Contributor Author

It looks like this is now tested quite thoroughly. @Julian @karenetheridge any reason not to go ahead and close this?

@Julian
Copy link
Member

Julian commented Aug 12, 2022

Yup, indeed. Closing, thanks folks.

@Julian Julian closed this as completed Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing test A request to add a test to the suite that is currently not covered elsewhere.
Projects
None yet
Development

No branches or pull requests

5 participants