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

Added migration page overview #1061

Draft
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

kwennB
Copy link
Contributor

@kwennB kwennB commented Oct 24, 2024

What kind of change does this PR introduce?

Documentation

Issue Number:

This is the overview page for Migration, which is part of the effort to create an easy transition between dialect upgrades.

Does this PR introduce a breaking change?

No

@kwennB kwennB added 📝 Documentation Indicates improvements or additions to documentation. GSoD Google Season of Docs labels Oct 24, 2024
@kwennB kwennB self-assigned this Oct 24, 2024
@kwennB kwennB requested a review from a team as a code owner October 24, 2024 10:46
Copy link

github-actions bot commented Oct 24, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
website ✅ Ready (View Log) Visit Preview f0fd4c2

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5c0ef1b) to head (f0fd4c2).
Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1061   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          373       373           
  Branches        94        94           
=========================================
  Hits           373       373           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kwennB kwennB marked this pull request as draft October 24, 2024 10:50
pages/specification/migration/_index.md Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
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.

I found a few errors in the table. But, also there are several cases that are quite complicated. Hopefully my suggestions fill in some of that history. Feel free to express any of the complicated cases however you see best. My suggestions are just suggestions. If you want to ignore some of the chaos in name of simplifying things a bit, I'm ok with that.

pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
</Infobox>

<Infobox label="Note">
Before Draft 3, the JSON Schema only included the Core specification, which outlined the foundational elements for schema structure. With Draft 4, the specification expanded to incorporate Validation, establishing rules for data format, structure, and type requirements. This addition enabled schema authors to define and enforce validation constraints directly within their schemas, making it easier for implementers to ensure data integrity and compatibility as schemas evolved.
Copy link
Member

Choose a reason for hiding this comment

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

This isn't correct. There was originally only one spec that included everything. Everything that we now consider Core, Validation, and Hyper-Schema was included in that one spec. In draft-04, the one spec was split into the three parts we're familiar with today. So, the spec didn't expand in draft-04 to include validation. Validation was always there. It just split that content into a different document.

This may be too much detail, but what has gone in each document has changed over time as well. The Content vocabulary and a few Meta-Data keywords moved from Hyper-Schema to Validation (draft-07) and the Applicator vocabulary moved from Validation to Core (2019-09).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything that we now consider Core, Validation, and Hyper-Schema was included in that one spec

What was the one spec?

If you also stated that there was originally only one specification, at what point did the Validation become necessary?

In draft-04, the one spec was split into the three parts we're familiar with today. So, the spec didn't expand in draft-04 to include validation. Validation was always there. It just split that content into a different document.

Thank you. I will phrase this better.

The Content vocabulary and a few Meta-Data keywords moved from Hyper-Schema to Validation (draft-07) and the Applicator vocabulary moved from Validation to Core (2019-09).

Are there any more data for scenarios like this?

To better represent this Spec note, what would be the most nearly accurate detail?

Copy link
Member

Choose a reason for hiding this comment

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

What was the one spec?

I'm not sure what you mean. Draft-01, draft-02, and draft-03 were all just one spec that included what is now Core, Validation, and Hyper-Schema. It didn't really have another name like the three do now. It was just the JSON Schema spec.

If you also stated that there was originally only one specification, at what point did the Validation become necessary?

Validation was always part of the spec. It was included from the first draft. It was always necessary.

Are there any more data for scenarios like this?

I think this kind of information is in the release notes for each draft. The older drafts don't have release notes, but those didn't have these kinds of changes.

To better represent this Spec note, what would be the most nearly accurate detail?

Honestly, I'm not sure what this spec note is trying to communicate. Is it just that there are now three specs instead of one?1 That was just a change in the way the spec was maintained. It has little affect on what people can and can't do with JSON Schema. However, splitting off Hyper-Schema meant draft-04 was the first draft where you could implement JSON Schema without including Hyper-Schema support. Implementers could implement Core and Validation and treat Hyper-Schema as an extension that they don't support.

Footnotes

  1. Actually, it was broken into four specs: Core, Validation, Hyper-Schema, and JSON Reference. But, JSON Reference wasn't maintained by us. JSON Reference was merged back into Core in draft-06.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for clarifying about the initial "JSON Schema Spec".

pages/draft-04/migration-notes.md Outdated Show resolved Hide resolved
pages/draft-04/migration-notes.md Show resolved Hide resolved
| ----------------- | ----------------- | ------------- | ----------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `$schema` | `$schema` | Core | Identifier | Change in the dialect (Uses the latest Draft4 dialect) |
| `type` | `type` | Validation | Assertion | This change no longer accepts the `any` type, restricting instances to the seven core primitive types only. |
| `disallow` | removed | Validation | Applicator | The `disallow` keyword was removed, eliminating the ability to specify types or schemas that an instance must not match. |
Copy link
Member

Choose a reason for hiding this comment

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

The ability to specify types or schemas that an instance must not match wasn't eliminated, it's just done using not now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you confirm that disallow had that functionality before? That way, I can add that the not keyword replaced it in functionality.

Copy link
Member

Choose a reason for hiding this comment

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

I can confirm that not can do everything disallow could do, just with a little different syntax.

  • { "disallow": "string" } => { "not": { "type": "string" } }
  • { "disallow": { "type": "number", "minimum": 0 } } => { "not": { "type": "number", "minimum": 0 } }

pages/draft-04/migration-notes.md Outdated Show resolved Hide resolved
Comment on lines +35 to +45
<Infobox label="Note"> Starting with Draft 4, schema identifiers that use an empty URI "" or a fragment-only URI "#" are no longer allowed.

In Draft 3, these identifiers were considered valid:

```
id: ""
id: "#"
```

However, this format is now prohibited from Draft 4 onwards.
</Infobox>
Copy link
Member

Choose a reason for hiding this comment

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

Should id be included in the table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it was only used as a reference example.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I didn't understand your response. The infobox is describing a change to how id behaves from draft-03 to draft-04. The table above lists all keyword changes that occurred between draft-03 and draft-04. So, I don't see why id wouldn't be included.

Copy link
Contributor Author

@kwennB kwennB Nov 21, 2024

Choose a reason for hiding this comment

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

This is what I was trying to communicate with the infobox.

On Draft 3, it was defined as :
This attribute defines a URI of a schema that contains the full
representation of this schema. When a validator encounters this
attribute, it SHOULD replace the current schema with the schema
referenced by the value's URI (if known and available) and re-validate
the instance. This URI MAY be relative or absolute, and relative URIs
SHOULD be resolved against the URI of the current schema.

And on Draft 4:
The value for this keyword MUST be a string, and MUST be a valid URI. This URI MUST be normalized, and SHOULD NOT be an empty fragment (#) or the empty URI.

--
So it was not only about id.

But thank you for pointing out id; I checked and noticed I skipped adding it here because it didn't change to $id until draft 6. So I will include it here.

Copy link
Member

Choose a reason for hiding this comment

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

So it was not only about id.

Maybe I'm missing something, but I don't think that's true. You've quoted the definition of $ref for draft-03, not id. Maybe that's a source of confusion? Both definitions describe the behavior of id and nothing more. The draft-03 definition is pretty vague and the draft-04 definition isn't great either, but the only clear difference is that empty URIs and empty fragments are discouraged.

Actually, the spec says "SHOULD", not "MUST". So, technically those empty values aren't disallowed, it's just recommended that schema authors don't use them. I think that's a good argument for not considering this not a change and leaving it out of the migration page altogether. Implementations in both drafts should consider those values to effectively be a no-op. The only difference is that draft-04 tells schema authors not to use that particular nonsense value. I'd argue that nothing actually changed in the behavior of id from draft-03 to draft-04.

pages/draft-04/migration-notes.md Outdated Show resolved Hide resolved
pages/draft-04/migration-notes.md Outdated Show resolved Hide resolved
Comment on lines 66 to 68
#### Step 3: Refactor $ref Usage

References in Draft 4 (`$ref`) are more standardized, which enhances schema modularity and reusability. If your schema relies on definitions within or across schemas, make sure the `$ref` keyword is used consistently as per Draft 4’s enhanced JSON Reference guidelines.
Copy link
Member

Choose a reason for hiding this comment

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

There were no changes to $ref in draft-04. It was moved into its own spec, but nothing changed about the way it worked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then there's no need to keep this as Step 3.

Comment on lines 70 to 76
#### Step 4: Validate Your Updated Schemas

Test your updated schemas using a JSON Schema validator compatible with Draft 4 ([Another JSON Validator](https://ajv.js.org/) (AJV)). Pay particular attention to behaviours of `type`, `properties`, and new combinatory keywords (`allOf`, `anyOf`), as validation behaviour may differ from Draft 3.

#### Step 5: Test in Your Application Context

After the schema updates, integrate them into your application and test the end-to-end functionality. Ensure that the new schema definitions meet your data requirements and interact appropriately with application code, especially in cases involving complex validation rules and nested references.
Copy link
Member

Choose a reason for hiding this comment

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

These last two steps effectively just tell people to test the changes to their schemas. That's general and obvious software development practice that I don't think needs to be said explicitly.

In any case, don't suggest using AJV. It has poor spec compliance and isn't actively maintained.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you suggest any tool?

We can also say, "validate and test your updated schemas with your preferred tool".

Copy link
Member

Choose a reason for hiding this comment

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

I like suggesting they use their preferred tool. We could link to the tooling page to help them find a tool if they don't have a preferred tool yet.

@kwennB
Copy link
Contributor Author

kwennB commented Nov 20, 2024

I see you chose to remove the (legacy) keyword entries. That's a perfectly fine choice, but if you remove them, it doesn't make sense to keep referencing them in the "Changed" column. Without the (legacy) keyword entries, "Replaced by"/"Replaced" doesn't make sense.

I think it would make sense to use the draft identifier of the release where the change was introduced similar to how we are using the "Removed" column. Depending on how much information you want to put in that column, you could also include a short sentence describing the change or a link to a resource describing the change in detail.

Are you suggesting removing the entire 'Changed' column or to put dashes in places that carry 'Replace, or Replaced by'.

@jdesrosiers
Copy link
Member

Are you suggesting removing the entire 'Changed' column or to put dashes in places that carry 'Replace, or Replaced by'.

No, not at all. The only entries I was suggesting you change are the ones that reference a (legacy) keyword such as Replaced $ref (legacy). You can't say it replaced $ref (legacy) because you removed the listing for $ref (legacy). Here's an example of what my suggestion was. It uses the draft identifier for when the keyword changed in the "Changed" column.

| All Keywords | Specification | Draft introduction | Removed | Changed                                                    |
| ------------ | ------------- | ------------------ | ------- | ---------------------------------------------------------- |
| `$ref`       | Core          | 03                 | No      | 2019-09 (???)                                              |
| `type`       | Validation    | 01                 | No      | 03 (???)                                                   |
| `format`     | Validation    | 01                 | No      | 04 (???), 06 (???), 07 (???), 2019-09 (???), 2020-12 (???) |

This communicates the draft the keyword was introduced, the draft(s) the keyword changed, and the draft the keyword was removed (if it was removed). (???) is a placeholder if you want to put a short description of what changed. If not, you can just list the draft identifiers.

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.

Thanks for your hard work on this PR. The history of JSON Schema is complicated, so this is a hard one to get right.

pages/draft-04/migration-notes.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved
pages/specification/migration/_index.md Outdated Show resolved Hide resolved

| Keyword(Draft 2) | Keyword(Draft 3) | Specification | Keyword type | Behavior Details |
| ----------------- | ------------------- | ------------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `$schema` | `$schema` | `core` | Identifier | Change in the dialect |
Copy link
Member

Choose a reason for hiding this comment

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

$schema didn't exist in draft-02. It was introduced in draft-03.

pages/draft-03/migration-notes.md Outdated Show resolved Hide resolved
pages/draft-03/migration-notes.md Outdated Show resolved Hide resolved
| `$schema` | `$schema` | `core` | Identifier | Change in the dialect |
| not present | `$ref` | `core` | Applicator | `$ref` key references an external schema URI for validation, allowing re-validation against the referenced schema. |
| not present | `id` | `core` | Identifier | This attribute defines the schema's current URI (a "self" link). The URI can be relative or absolute and is resolved against the parent schema's URI. If id is missing, the parent's URI is used. |
| `optional` | `required` | `core` | Assertion | This change ensures that properties must have defined values for validation. |
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 this description needs work. In draft-02, object properties defined with properties were considered to be required by default. The optional keyword existed to make a property optional. In draft-03, it changed so that object properties defined with properties were optional by default. So, required was replaced with optional to reflect the change to the default behavior. Since properties are optional by default, we no longer needed an optional keyword, we need a required keyword.

Hopefully, you can rewrite that in a more concise way for this description.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the feedback Jason!,
I would be more descriptive and rephrase this part.

pages/draft-03/migration-notes.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝 Documentation Indicates improvements or additions to documentation. GSoD Google Season of Docs Status: In Progress This issue is being worked on, and has someone assigned.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

8 participants