fix: annotate resolved schemas with dialect-aware id keyword#912
Open
penyaskito wants to merge 3 commits into
Open
fix: annotate resolved schemas with dialect-aware id keyword#912penyaskito wants to merge 3 commits into
penyaskito wants to merge 3 commits into
Conversation
UriRetriever::retrieve() unconditionally stamped resolved schemas with the draft-04 "id" keyword. Draft-6 renamed this to "$id", but the annotation was never updated. Detect the dialect from the root schema's $schema keyword before pointer resolution. Draft-3/4 schemas get "id"; Draft-6+ and schemas without an explicit $schema keyword get "$id" (matching the library's default). Internally safe because SchemaStorage::findSchemaIdInObject() already reads both keywords. Fixes the issue for external consumers of resolved schemas (e.g. strict Draft-7 validators like Ajv that reject the unknown "id" keyword). Fixes jsonrainbow#911 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
penyaskito
commented
May 27, 2026
Comment on lines
-342
to
+343
| $this->assertEquals('454f423bd7edddf0bc77af4130ed9161', md5(json_encode($schema))); | ||
| $this->assertEquals('object', $schema->type); | ||
| $this->assertObjectHasAttribute('$id', $schema); |
Contributor
Author
There was a problem hiding this comment.
$id instead of id?
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes UriRetriever::retrieve() unconditionally stamping the Draft-4 id keyword onto every URI-resolved schema. It now inspects the loaded root schema's $schema keyword and uses id only for Draft-3/4, defaulting to $id otherwise (matching the library's Draft-6 default dialect). The corresponding test no longer relies on a brittle md5 of the serialized schema.
Changes:
- Import
DraftIdentifiersinUriRetrieverand detect the dialect from the root schema before pointer resolution. - Stamp the resolved URI onto either
id(Draft-3/4) or$id(everything else) inUriRetriever::retrieve(). - Replace the md5-of-JSON assertion in
testRetrieveSchemaFromPackagewith structural assertions ontypeand the presence of$id.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/JsonSchema/Uri/UriRetriever.php | Detect dialect from root schema's $schema and annotate with id for Draft-3/4, $id otherwise. |
| tests/Uri/UriRetrieverTest.php | Switch the package-retrieval test from a brittle md5 hash to structural assertions reflecting the new $id annotation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…word The dialect comparison now strips the trailing # from the $schema value and compares against withoutFragment() constants, so schemas declaring e.g. "http://json-schema.org/draft-04/schema" (without #) correctly get `id` instead of falling through to `$id`. Adds 5 tests covering Draft-04 and Draft-07 with/without fragment, plus the no-$schema default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UriRetriever::retrieve()now reads the root schema's$schemakeyword to determine whether to annotate withid(Draft-3/4) or$id(Draft-6+)$schemakeyword default to$id, matching the library's Draft-6 default dialecttestRetrieveSchemaFromPackageto assert structure instead of a brittle md5 hashContext
UriRetriever::retrieve()unconditionally stamped$jsonSchema->id = $resolvedUrion every resolved schema. Draft-6 renamedidto$id, but the annotation was never updated. This leaks a Draft-4 keyword into schemas resolved under Draft-6/7, causing errors in strict validators (e.g. Ajv:NOT SUPPORTED: keyword "id", use "$id" for schema ID).The dialect is detected from the root schema before pointer resolution, so sub-schemas inherit the correct keyword from their parent document.
Internally safe —
SchemaStorage::findSchemaIdInObject()already reads bothidand$id.Test plan
id, Draft-07 →$id, no$schema→$idFixes #911