Skip to content

Validate schema directive applications - #4607

Open
patrick91 wants to merge 1 commit into
mainfrom
validate-schema-directive-applications
Open

Validate schema directive applications#4607
patrick91 wants to merge 1 commit into
mainfrom
validate-schema-directive-applications

Conversation

@patrick91

@patrick91 patrick91 commented Aug 31, 2026

Copy link
Copy Markdown
Member

Summary

  • validate attached schema directives against every supported type-system location during schema construction
  • reject duplicate applications of non-repeatable directives per GraphQL name and schema element
  • preserve repeatable directive order and valid reuse on separate elements
  • simplify the SDL printer to emit validated applications without location filtering
  • preserve schema-directive discovery, registration, introspection, and Federation behavior from Expose attached schema directives through introspection #4598
  • add source-aware errors, focused regression coverage, documentation, and a patch release note

Compatibility

This intentionally surfaces schema directive applications that were previously omitted from SDL or emitted as SDL rejected by GraphQL tools. Generated input mutation fields now require directives attached to them to declare INPUT_FIELD_DEFINITION.

Test plan

  • affected schema, printer, and Federation suites: 858 passed, 27 skipped, 2 xfailed
  • focused graphql-core 3.2.6, 3.2.11, and 3.3.0rc0 compatibility checks
  • rebased focused suite: 123 passed, 5 skipped
  • ruff check and format check
  • targeted mypy
  • pre-commit on all files

Summary by Sourcery

Validate schema directive locations and repeatability during schema construction to prevent invalid applications from being omitted or emitted as invalid SDL.

Bug Fixes:

  • Validate schema directive applications against their declared GraphQL locations during schema construction.
  • Reject duplicate applications of non-repeatable directives on the same schema element while preserving valid repeatable and cross-element reuse.
  • Report actionable, source-aware errors for invalid directive locations and duplicate applications.

Enhancements:

  • Simplify SDL printing to emit the validated directive applications directly.
  • Preserve Federation directive behavior, including repeatable compose directives and multiple key applications.

Documentation:

  • Document schema directive location and repeatability validation, including upgrade guidance for newly exposed invalid applications.

Tests:

  • Add regression coverage for directive validation across schema, type, field, argument, enum value, input field, and Federation scenarios.

Chores:

  • Add patch release notes for schema directive validation changes.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for adding the RELEASE.md file!

Below is the changelog that will be used for the release.


This release fixes validation of schema directive applications.

Strawberry now validates every supported type-system location during schema
construction, including the distinctions between objects and interfaces and between
output and input fields. Applying a directive outside its declared locations raises
an actionable error that identifies the directive, actual location, and schema
element.

Applying a non-repeatable directive more than once to the same schema element now
also raises an error. Repeatable directives retain their Python attachment order,
and non-repeatable directives can still be reused on separate elements. Valid
Federation applications such as multiple @key and @tag directives continue to
work.

This may expose invalid applications that Strawberry previously omitted from SDL or
printed as SDL rejected by GraphQL tools. Move or remove those applications, add the
intended location to the directive definition, or set repeatable=True when repeated
applications are intentional.

This release was contributed by @patrick91 in #4607

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@botberry

Copy link
Copy Markdown
Member

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🟢
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR validates schema-directive locations and repeatability during schema construction, adds source-aware errors, and lets the SDL printer emit the resulting validated applications directly.

  • Covers schema, type, field, argument, enum-value, scalar, union, interface, and input locations.
  • Preserves repeatable application order and updates Federation compose directives and regression coverage.
  • Documents the compatibility change and adds a patch release note.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking concern that unvalidated post-construction metadata or alternate BaseSchema implementations can now produce invalid SDL.

Construction-time validation comprehensively covers the normal Schema path, but removing all printer checks leaves the public printing boundary dependent on directive metadata remaining unchanged and on callers having used the concrete Schema collector.

Files Needing Attention: strawberry/printer/printer.py

Important Files Changed

Filename Overview
strawberry/schema/directive_collector.py Adds location-aware traversal and per-element repeatability validation for directive applications throughout the converted GraphQL type graph.
strawberry/printer/printer.py Removes printer-side location filtering in favor of construction-time validation, leaving a non-blocking validation-boundary concern for mutation and alternate BaseSchema implementations.
strawberry/exceptions/schema_directive.py Adds actionable, source-aware exceptions for invalid locations and duplicate non-repeatable directives.
strawberry/federation/schema_directives.py Marks composeDirective repeatable to preserve valid multiple schema applications.
tests/schema/test_schema_directives.py Adds broad registration, SDL validity, location rejection, duplicate rejection, and separate-element reuse coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Python schema declarations] --> B[GraphQL conversion]
    B --> C[SchemaDirectiveCollector]
    C --> D{Location and repeatability valid?}
    D -- No --> E[Raise source-aware schema error]
    D -- Yes --> F[Register directive definitions]
    F --> G[Construct GraphQL schema]
    G --> H[Print validated directive applications to SDL]
Loading

Reviews (1): Last reviewed commit: "Validate schema directive applications" | Re-trigger Greptile

Comment on lines 216 to +218
return "".join(
print_schema_directive(directive, schema=schema, extras=extras)
for directive in directives
for directive in field.directives

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Preserve printer location validation

The printer now emits directives from mutable field metadata without validating their declared locations. Post-construction mutations and alternate BaseSchema implementations bypass SchemaDirectiveCollector, so printing such metadata produces SDL that GraphQL tooling rejects; retain a location assertion at this boundary or make the validated state immutable.

Knowledge Base Used: Schema conversion and printing

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wonder if we can consider that use case supported 🤔

maybe we do a public api to do this (modify directives?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I’m open to supporting post-construction changes, but mutating field.directives directly cannot be considered supported by this PR: it bypasses SchemaDirectiveCollector, so the GraphQLSchema directive registry, introspection, repeatability/location checks, and Federation-derived metadata can become stale. A public API could make this safe, but it would need to validate the application and invalidate or rebuild the affected schema state—not just append to the list. Until that exists, I’d keep the printer defensive here, or make the directive collections immutable, so raw metadata mutation cannot produce invalid SDL.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure how much use case is there for these things, what are the mutation that one could do? could we have field.add_directive for example?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree the post-construction mutation use case is probably small. The realistic cases are extensions or decorators that add directives while assembling a type, generated fields, or code that runs after @strawberry.type but before Schema construction. Those already work by mutating field.directives, and the collector validates the final state when the schema is built.

A field.add_directive(directive) method could make that supported usage clearer, but I would scope it to pre-schema-construction mutation. It cannot safely support mutation after Schema creation: the GraphQL field, directive registry/introspection, repeatability checks, and Federation metadata may already have been materialized. Supporting that would require a schema-level API that rebuilds or invalidates those structures, rather than simply appending to the Strawberry field.

I would not add field.add_directive as part of this PR unless we want the explicit API/documentation. If we do add it, the collector should remain the validation boundary, and the printer can rely on validated applications rather than reimplementing location filtering.

@codspeed-hq

codspeed-hq Bot commented Aug 31, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 34 untouched benchmarks


Comparing validate-schema-directive-applications (3476aef) with main (4837f62)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (215bf4b) during the generation of this report, so 4837f62 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@patrick91
patrick91 requested a review from bellini666 August 31, 2026 22:58
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.

2 participants