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

add description and examples for $anchor keyword #113

Merged
merged 7 commits into from
Feb 17, 2024

Conversation

AgniveshChaubey
Copy link
Collaborator

This PR addresses #53.

Copy link
Member

@jviotti jviotti left a comment

Choose a reason for hiding this comment

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

Good take! I left some comments but indeed grasp the idea of anchors very well!

content/2020-12/core/anchor.markdown Outdated Show resolved Hide resolved
content/2020-12/core/anchor.markdown Outdated Show resolved Hide resolved

The `$anchor` keyword is used to assign a unique identifier to a subschema within the scope of its containing schema. This identifier can then be referenced elsewhere in the schema using the `$ref` keyword.

* Its value must be a valid identifier starting with a letter and containing letters, digits, hyphens, underscores, colons, or periods.
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually the case? I guess any valid URI fragment would do, but I might be wrong. i.e. 1234 or even something that looks like a JSON Pointer might in the theory do.

Can you cross check this with a few implementations and with the URI RFC?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, let me verify

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image
https://json-schema.org/draft/2019-09/json-schema-core#rfc.section.8.2.3

I tried with hyperhump schema validator which allows any string values (including special characters) without checking any format (which I think is not an expected behavior), whereas json everything strictly follows the above rule and only allows the $achors with above-mentioned format.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, great

Choose a reason for hiding this comment

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

If I remember correctly (and if it matters) this constraint didn't always exist. I think it was added in 2019-09. Sorry, for not looking it up myself.

content/2020-12/core/anchor.markdown Outdated Show resolved Hide resolved
content/2020-12/core/anchor.markdown Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I think another point worth mentioning is that defining two equal anchors on the SAME schema resource is considered to be an error.

However, defining two equal anchors on DIFFERENT schema resources (i.e. subschemas with their own $id) is valid. Maybe show an example of that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean something like this? ---
Valid:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "https://example.com/base",
  "properties": {
    "foo": {
      "$id": "one",
      "$anchor": "bar",
      "type": "integer"
    },
    "bar": {
      "$id": "two",
      "$anchor": "bar",
      "type": "string"
    }
  }
}

Invalid:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "https://example.com/base",
  "properties": {
    "foo": {
      "$anchor": "bar",
      "type": "integer"
    },
    "bar": {
      "$anchor": "bar",
      "type": "string"
    }
  }
}

However, the implementations do not throw any error for the second schema and the fragmented URI reference is resolved to the last $anchor

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "https://example.com/base",
  "properties": {
    "foo": {
      "$anchor": "bar",
      "type": "integer"
    },
    "bar": {
      "$anchor": "bar",
      "type": "string"
    },
    "baz": {
      "$ref": "#bar"
    }
  }

Instance: {"foo": 22, "bar": "aaaa", "baz": "aaaa" }

Copy link
Member

Choose a reason for hiding this comment

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

However, the implementations do not throw any error for the second schema and the fragmented URI reference is resolved to the last $anchor

That's strange. Which implementations are you testing with?

Copy link
Member

Choose a reason for hiding this comment

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

@jdesrosiers Shouldn't Hyperjump fail for the latter schema? i.e. same anchor twice in the same schema resource?

Choose a reason for hiding this comment

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

Having duplicate anchors doesn't make the schema invalid. It's just nonsense to do so and the behavior is undefined. It's like having a JSON value with duplicate properties ({ "a": 1, "a": 2 }). It's not invalid JSON, but you can't count on different implementations handling it the same way.

So, definitely tell users not to do that, but don't tell them they should expect an error.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, interesting. I thought it would result in an error. Then probably fine to ignore all this @AgniveshChaubey

@AgniveshChaubey
Copy link
Collaborator Author

Apologies for the delayed response. I've been dealing with some family issues, which is making me a bit distracted.

@jviotti
Copy link
Member

jviotti commented Feb 16, 2024

No hurries at all! Take your time :)

Copy link
Member

@jviotti jviotti left a comment

Choose a reason for hiding this comment

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

Approving it already, and we can sort out the case with conflicting anchors separately?

@AgniveshChaubey
Copy link
Collaborator Author

Sure! 👍

@AgniveshChaubey AgniveshChaubey merged commit 828b446 into main Feb 17, 2024
1 check passed
@AgniveshChaubey AgniveshChaubey deleted the keyword-anchor branch February 17, 2024 04:55
@AgniveshChaubey AgniveshChaubey mentioned this pull request Mar 30, 2024
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.

3 participants