Schema-aware validation and schema drift detection (0.2.0) - #1
Merged
Conversation
The groundwork for checking queries against a schema. A schema is read with graphql-core's own full introspection query, because the minimal one the schema keywords used cannot build a schema object at all: build_client_schema needs each type's kind, the ofType chain, field types, arguments, interfaces and possible types, and rejects anything partial. That query is expensive - 595 KB against the endpoint this was measured on, where the minimal one was 4.6 KB - so the result is cached. The cache is keyed on the same connection parameters as the client cache, so aliases against one endpoint share a schema exactly as they already share a connection pool, and a closed client takes its schema with it rather than leaving a stale one behind for the next session on that endpoint. fetch_schema lives in its own module rather than on either keyword class: both the executing keywords and the schema keywords need it, and neither owns the other. SchemaUnavailable is deliberately not GraphQLResponseError. Callers have to be able to tell 'the server would not describe itself' apart from 'the server reported errors', because the two deserve opposite responses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parsing catches a query that is not GraphQL. It cannot catch one that is valid GraphQL but wrong for this server: a misspelled field, an argument that does not exist, a selection missing its subfields. That check needs the schema, is part of the specification rather than an extra, and graphql-core already implements all 27 rules - so this is mostly a matter of calling it. On by default, since a query that cannot succeed is worth catching whether or not a suite remembered to ask. validate_against_schema=False turns it off, and validate_queries=False turns it off with the other local checks. Two behaviours make 'on by default' safe rather than obstructive: A server that will not describe itself is not a failure. Introspection is commonly disabled outside development, so failing there would break every suite pointed at staging. The check is skipped with a warning instead. It is retried once after an operation succeeds. Found against a real endpoint: the first query on a session is very often the login, and a schema behind authentication answers 401 until that has run. Latching on the first failure left validation off for the whole suite, which is precisely where it was wanted. Attempts are capped, so a server that genuinely refuses is asked twice rather than before every query. Failures raise ValueError, matching the convention for a bad call caught before anything is sent. GraphQLResponseError would be wrong: nothing was sent, so there is no response. Execute Raw Request still bypasses all of this, as documented. The shared unit-test fixture switches validation off, because it puts a request in front of the one each of those tests reads back. The default path has its own tests rather than being left untested behind a disabled fixture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he schema Save Schema Snapshot writes the schema as SDL; Get Schema Breaking Changes, Get Schema Dangerous Changes and Schema Should Have No Breaking Changes compare a live endpoint against one. SDL rather than the introspection JSON, because a snapshot is only worth committing if the diff is readable - a removed field then shows up in review rather than as a puzzling failure months later. Breaking and dangerous are reported apart because they answer different questions: a removed field breaks a query outright, a value added to an enum only breaks a suite that switches on it. The comparison builds both sides the same way. Comparing snapshot SDL against a live introspected schema reported a change that was not one: a server built on graphql-js describes @deprecated with four locations, while graphql-core's built-in adds DIRECTIVE_DEFINITION, so every comparison against a real JS server claimed a directive location had been removed. Printing the live schema and reading it back leaves only real differences. Found by running this against a real endpoint; the acceptance server never showed it. The existing keywords now read the cached schema object instead of raw introspection JSON, which is what removed the hand-written introspection query. Names, arguments, return values and error messages are unchanged, so atest/schema_tests.robot passes untouched - that suite is the regression test for this part. The unit tests build their fake introspection by running graphql-core's introspection query against a schema built from SDL. The hand-rolled fixture they used before cannot build a client schema, which is the same limitation that made the old introspection query unusable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also corrects the note on the graphql-core dependency, which claimed the library called `validate` directly when it only ever called `parse`. That is finally true, and the note now lists what is actually imported. Co-Authored-By: Claude Opus 5 (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
1 participant
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.
Adds the half of GraphQL validation the library was missing, plus drift detection against a committed snapshot.
Why
Validate Queryonly ever parsed. It passed{ user(id: 1) { nickname } }against a schema with nonicknamefield, because catching that needs the schema. Validation against a schema is GraphQL specification §5 rather than an extra, andgraphql-core— already a dependency — implements all 27 rules. The library simply never called them.What
validate_against_schema=Falseturns it off.Query Should Be Valid Against Schemafor an explicit check.Validate Queryis unchanged and now documents that it is syntax-only.Save Schema Snapshot,Get Schema Breaking Changes,Get Schema Dangerous Changes,Schema Should Have No Breaking Changes.Refresh Graphql Schema.Two things found only by testing against a real server
Neither reproduced against the acceptance server:
A schema behind authentication was never picked up. The first query on a session is often the login itself, so introspection 401s. Latching on that first failure left validation off for the entire suite — exactly where it was wanted. It is now retried once after an operation succeeds, with a bounded attempt count so a server that genuinely refuses is not asked before every query.
Drift detection reported a false positive. A snapshot compared against itself claimed
DIRECTIVE_DEFINITION was removed from deprecated, because graphql-js describes@deprecatedwith four locations while graphql-core's built-in adds a fifth. Every comparison against a real JS server would have hit it. Both sides are now built the same way.Verification
atest/schema_tests.robotunchanged — the regression test proving the rewritten schema keywords kept their contracttwine checkpassesA server with introspection disabled degrades to a warning rather than failing, since that is a normal thing to test against.
🤖 Generated with Claude Code