Skip to content

Add Draft 07 support#50

Merged
jba merged 10 commits intogoogle:mainfrom
baptmont:draft-07
Nov 20, 2025
Merged

Add Draft 07 support#50
jba merged 10 commits intogoogle:mainfrom
baptmont:draft-07

Conversation

@baptmont
Copy link
Contributor

For #4
Supersedes #6
Added draft-7 conformance tests, including format.json which is ignored for draft 2020.
Added Draft to resolved.
When marshaling, kept @slimslenderslacks logic to use prefix items + items. Added logic to separate dependencies field, separate id behavior into id and anchor, replace Ref to items.
When resolving and validating ignore other properties of ref objects

@baptmont
Copy link
Contributor Author

baptmont commented Nov 18, 2025

The previous commit was umarshaling the draft7 fields, and adapting it to the draft2020 new field. For example splitting the dependencies field into the DependentRequired and DependentSchemas, and then use those for the validation.
Now I remove this and modified it to use the draft7 fields for validation and resolving.
I remove the guessing from the detectDraft and now it only uses the schema field and defaults to draft2020.

I had to introduce 2 helper structs to map the union types for the Dependencies field and for the Items field

@baptmont baptmont requested a review from jba November 18, 2025 16:47
{"/allOf/1", s.AllOf[1]},
{"/definitions/~1~0", s.Definitions["/~"]},
{"/definitions/~01", s.Definitions["~1"]},
{"/items/0", s.ItemsArray[0]},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a case for /items when Items is non-nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Items and ItemsArray are mutually exclusive, which is one of the validations done in basicChecks.
I was already checking non-nil items in {"/items/1/items", s.ItemsArray[1].Items}.
Adding an Items value to the root of the test Schema will break the "/items/0" tests, since lookupSchemaField will retrieve the Items Schema when processing the "items" segment.
It will not throw an error since the basicCheck happens during resolution which we are skipping here.

Copy link
Collaborator

@jba jba left a comment

Choose a reason for hiding this comment

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

Also there should be a comment somewhere that notes why the schema field map has the right value for "dependencies". Maybe it has to do with the order they're listed somewhere? If so, note that the order matters, and why.

@baptmont
Copy link
Contributor Author

The value of "dependencies" is defined after a sort of schemaFieldInfos. This sort is unstable and is comparing the json.names of DependencyStrings and DependencySchemas which are both "dependencies". Since the sort is unstable it cannot be guarantied that "dependencies" has the correct value and it should be treated as an edge case.
This is why instead of checking the schemaFieldMap the following was required

if name == "dependencies" {
	return v.FieldByName("DependencySchemas")
}
if sf, ok := schemaFieldMap[name]; ok {

I'll add a quick comment explaining that then

@jba jba merged commit 98a387e into google:main Nov 20, 2025
6 checks passed
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