Skip to content
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

Review removed cref regarding $anchor's affect on dynamic scope resolution #1056

Closed
jdesrosiers opened this issue Dec 8, 2020 · 7 comments
Closed
Labels

Comments

@jdesrosiers
Copy link
Member

jdesrosiers commented Dec 8, 2020

We removed this cref regarding $dynamicRef/$dynamicAnchor from draft 2020-12. It appears to be wrong, but we haven't had enough eyes on it to be 100% sure and we don't want it to get lost if it turns out that it does say something important. This issue is to remind us to come back to this after a few more people have implemented $dynamicRef/$dynamicAnchor and/or feel ready to contribute their thoughts.

Requiring both the initial and final URI fragment to be defined
by "$dynamicAnchor" ensures that the more common "$anchor"
never unexpectedly changes the dynamic resolution process
due to a naming conflict across resources. Users of
"$dynamicAnchor" are expected to be aware of the possibility
of such name collisions, while users of "$anchor" are not.

I see no way that an $anchor can affect dynamic scope resolution and even if it could, it's not clear how bookending solves that problem.

Source: #1041 (comment)

@marksparkza
Copy link

marksparkza commented Apr 6, 2021

As far as I can tell, the only way $anchor might affect dynamic ref resolution is if, either:

  1. the implementation is faulty; or
  2. the schema is invalid against the spec

Problem 1 might occur if, for example, the implementation happens (as in my case) to use the same cache for storing (sub)schemas against URIs for $id, $anchor and $dynamicAnchor and, while searching the dynamic scope for schemas cached against possible target URIs, does not check that a found schema actually has the required $dynamicAnchor value. I'm only speculating, but perhaps this 'caveat' is what originally motivated the CREF. Regardless, the CREF is incorrect in putting the responsibility on schema users/authors to beware. It's an implementation problem, and the resolution is an implementation detail. And even then, bookending makes no difference, while checking for the required $dynamicAnchor value in a potential target schema is trivial.

Problem 2 might occur if there is a naming conflict within the same resource which - as in my implementation - might cause an $anchor schema to replace a $dynamicAnchor schema in the URI-schema cache. However, this situation not actually covered by the CREF (which refers to naming conflicts 'across resources', and is in any case expressly precluded by section 8.2.2 of the spec, which says:

The effect of specifying the same fragment name multiple times within the same resource, using any combination of "$anchor" and/or "$dynamicAnchor", is undefined. Implementations MAY raise an error if such usage is detected.

@handrews
Copy link
Contributor

It's because if you $dynamicRef: #foo and there's an $anchor: foo in the current resource, it should find that, and stop, and never examine the dynamic reference. Or at least that's what I wrote, I know @jdesrosiers was looking at changing that process so maybe this really no longer makes sense. I would not be surprised if $dynamic* could be better!

@jdesrosiers
Copy link
Member Author

if you $dynamicRef: #foo and there's an $anchor: foo in the current resource, it should find that, and stop, and never examine the dynamic reference. Or at least that's what I wrote,

I don't see how that behavior is what is defined in the spec. First, it does initial resolution (which has not effect in this example), then it checks dynamic scope. If dynamic scope is found, it will resolve there. If there is no dynamic scope, then it behaves like a $ref and will resolve to the local $anchor. $anchor only comes into play if there is no dynamic scope resolution.

Maybe there was another algorithm you intended?

@handrews handrews added the core label May 10, 2021
@handrews
Copy link
Contributor

  1. $dynamicAnchor and $anchor both create a totally normal fragment for their context resource. If both are present with the same value in a resource, which one sets the fragment is undefined.
  2. $dynamicAnchor also (in some way) labels the fragment as one that can be used in dynamic resolution.

So when you $ref a fragment, it's just a fragment. It doesn't matter which keyword creates it. It's not that $dynamicAnchor has extra features making it usable by both, it's that fragment resolution is just fragment resolution. RFC 3986 doesn't care how the fragment got there, that's a media-type concern.

  1. The first step in $dynamicRef is plain old RFC 3986 reference resolution, just like $ref.
  2. The second step is to check and see if the resolved fragment was created by $dynamicAnchor (now we're outside of 3986 and into our own process).
    3a. If it was, the third step is to examine the dynamic scope for other $dynamicAnchor-created fragments.
    3b. If it was not (it was just created by $anchor), then you are done: that is the final result even though it is a $dynamicRef and there may be some $dynamicAnchor of the correct name elsewhere in the dynamic scope.

The key thing here is that you only get the extra, non-RFC 3986 behavior if the initial anchor was created by $dynamicAnchor. Again, if you have $anchor and $dynamicAnchor with the same value in the same resource, the results are undefined (it could go either way, or just fail).

@jdesrosiers
Copy link
Member Author

@handrews, now I see where you're coming from. To express the problem as code, you expect anchor information to be collected like this,

// "$dynamicAnchor": "aaa"
anchors["aaa"] = { pointer: "/$defs/foo", dynamic: true };

// "$anchor": "aaa"
anchors["aaa"] = { pointer: "/$defs/bar", dynamic: false };

That could definitely result in the presence of an $anchor causing dynamic references to be nondeterministic because one keyword can completely overwrite the other.

The way I (and it seems everyone else) interpreted it is more like this,

// "$dynamicAnchor": "aaa"
dynamicAnchors["aaa"] = "https://example.com/my-schema#/$defs/foo";
anchors["aaa"] = "/$defs/foo";

// "$anchor": "aaa"
anchors["aaa"] = "/$defs/bar";

A $dynamicAnchor defines an anchor as well as a dynamic anchor, while $anchor just defines an anchor. Therefore, dynamic scope resolution is not affected by $anchor, but regular reference resolution could be affected by $dynamicAnchor. I think it was @ssilverman who suggested in slack conversation that $dynamicAnchor just defines a dynamic anchor and $anchor just defines an $anchor. That way neither dynamic nor static reference resolution can be affected by the other. I think that's something we should consider, but that's another issue.


I originally said,

I see no way that an $anchor can affect dynamic scope resolution and even if it could, it's not clear how bookending solves that problem.

Now I see how $anchor can affect dynamic scope resolution, but I still don't see how bookending solves that problem.

@gregsdennis
Copy link
Member

Reading this again, so much of the "bookending requirement" debate suddenly makes sense.

It seems that the difference in opinion comes down to whether you think that $dynamic* and $ref/$anchor are isolated referencing mechanisms or they interact (i.e. $dynamicRef first operates as $ref and respects $anchor).

If they interact, then, yes, you need bookending. Having $anchor: foo and $dynamicAnchor: foo in the same resource would be invalid because they would both define the anchor #foo. But having $anchor at the reference site subverts the dynamic behavior and makes it just a $ref. Thus the only way to guarantee dynamic behavior is by ensuring that the resource doesn't define $anchor by requiring $dynamicAnchor instead.

Conversely, if they're isolated referencing mechanisms, then $dynamicRef can never resolve to $anchor. They're both valid in the same resource with the same value, and you don't need to define $dynamicAnchor at the reference site.

Not looking at what the spec actually says, I tend to prefer having isolated referencing mechanisms. I think it's simpler to understand and to implement. $ref resolves to $id/$anchor; $dynamicRef only resolves to $dynamicAnchor.

That said, there was probably a reason to have $dynamicRef (and probably the previous $recursiveRef) first behave like $ref, and I'm currently investigating that.

@gregsdennis
Copy link
Member

For the stable release, we're separating the two referencing mechanisms completely. $anchor no longer affects $dynamic*. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants