Expose attached schema directives through introspection - #4598
Conversation
|
Thanks for adding the Below is the changelog that will be used for the release. This release fixes introspection for custom schema directives. Schema directives attached to types, fields, arguments, and other schema elements Federation directives and custom composed directives used on field arguments are A directive reused across the schema is defined only once. Input, enum, and scalar Because these directives and argument types are now part of the runtime schema, This release was contributed by @patrick91 in #4598 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="strawberry/schema/schema.py" line_range="559-585" />
<code_context>
+ for graphql_type in self._schema.type_map.values():
</code_context>
<issue_to_address>
**issue (broader_impact):** Schema directives attached to input objects, input fields, or other newly discovered directive-argument types are never registered when those types are reachable only through an attached directive argument. The method scans the initial schema type map, rebuilds the schema after discovering a directive, and does not rescan the newly added argument types, so introspection omits directives attached within nested directive input types.
**Triggers:** When an attached schema directive takes an input-object argument whose nested input object has its own attached schema directive and that input object is not otherwise used by the schema.
**Suggested fix:** After rebuilding the schema with newly discovered directive types, rescan the expanded type map (or recursively collect attached directives from Strawberry definitions before rebuilding).
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: strawberry/schema/schema.py:585
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Apollo Federation Subgraph Compatibility Results
Learn more: |
Greptile SummaryThe PR exposes attached schema directives and their supporting input-type graphs through runtime introspection while keeping SDL generation consistent.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the fixed-point collector now discovers directives on nested input types introduced solely through directive arguments before GraphQLSchema construction.
|
| Filename | Overview |
|---|---|
| strawberry/schema/directive_collector.py | Adds fixed-point traversal of reachable GraphQL types, directive applications, and directive argument input graphs. |
| strawberry/schema/schema.py | Integrates complete directive collection and conflict validation before the single GraphQLSchema construction. |
| strawberry/schema/schema_converter.py | Resolves schema-directive argument annotations during construction and raises the established unresolved-type error. |
| strawberry/printer/printer.py | Reuses registered directive definitions and filters private support types while retaining definitions required by visible SDL. |
| strawberry/federation/schema.py | Moves generated Federation link and composition directives into the pre-construction collection lifecycle. |
| strawberry/federation/types.py | Attaches runtime scalar metadata to Federation support types while keeping private definitions out of ordinary SDL. |
| strawberry/permission.py | Stabilizes automatically generated permission directive identity across instances and concurrent schema construction. |
| tests/schema/test_schema_directives.py | Covers transitive discovery through directive-only nested input graphs, including directives on nested input types and fields. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Root and explicit Strawberry types] --> B[Convert GraphQL types]
B --> C[Collect attached directives]
C --> D[Convert directive arguments]
D --> E[Queue referenced input graphs]
E --> C
C --> F[Prepare generated Federation directives]
F --> C
C --> G[Build and validate GraphQLSchema]
G --> H[Introspection]
G --> I[SDL printer]
Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/schema-dire..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="strawberry/schema/schema.py" line_range="1397-1400" />
<code_context>
pass
+ def _should_register_schema_directive(self, directive: object) -> bool:
+ return True
+
</code_context>
<issue_to_address>
**issue (broader_impact):** Federation directives attached to schema types are explicitly excluded from `_register_schema_directives`, so they are absent from `schema._schema.directives` and cannot be discovered through `__schema.directives` introspection when using `strawberry.federation.Schema`.
**Triggers:** When a federation directive such as `@key`, `@external`, or `@tag` is attached to a federated schema element.
**Suggested fix:** Return `True` for federation schemas or otherwise add federation directives to the GraphQL schema directive registry while continuing to filter their definitions from federation SDL.
</issue_to_address>
### Comment 2
<location path="strawberry/schema/schema.py" line_range="428" />
<code_context>
GraphQLCoreConverter.DEFINITION_BACKREF: self,
},
)
+ self._register_schema_directives()
except TypeError as error:
</code_context>
<issue_to_address>
**issue (bug_risk):** Schema directives added by the federation constructor after `super().__init__` returns—such as generated `@link` and `@composeDirective` instances—are never passed through `_register_schema_directives`, so they remain unavailable through GraphQL introspection even if they should be registered.
**Triggers:** When a federation schema uses composed directives or generates federation link directives.
**Suggested fix:** Run directive registration after federation adds its generated directives, or register those directives as part of the federation schema construction before finalizing the GraphQL schema.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: strawberry/schema/schema.py:1400, strawberry/schema/schema.py:428
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Move the attached-directive discovery into a small SchemaDirectiveCollector class with explicit state instead of nested closures, and pass the converted root types and explicit directive classes to the collect helpers as arguments instead of storing write-once scaffolding attributes on the Schema instance. Extract the directive-name registry and the @OneOf compatibility check into named module-level helpers, and normalize enum, argument, union and scalar directives to tuples at definition time so schema construction no longer needs to materialize one-shot iterables and write them back onto user definitions. Claude-Session: https://claude.ai/code/session_01TeQw3R6G4xK7qJxUTuKotZ
Sourcery withdrew this approval because the latest commits introduced blocking findings.
|
|
||
| ## Hiding integration support types from SDL | ||
|
|
||
| Integration authors can pass `print_definition=False` to `strawberry.enum` for |
There was a problem hiding this comment.
Wondering if print_definition is clear enough, or some kind of internal flag would be better? Not sure 🤔
There was a problem hiding this comment.
yeah, I opened an issue in order to change this, maybe for now we avoid documenting it :D
| if schema.config.enable_experimental_incremental_execution: | ||
| directives.append( | ||
| "directive @defer(if: Boolean, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT" | ||
| ) | ||
| directives.append( | ||
| "directive @stream(if: Boolean, label: String, initialCount: Int = 0) on FIELD" | ||
| ) |
There was a problem hiding this comment.
Is it possible to import the symbols from graphql-core and print those, instead of hardcoding how they look like?
Sourcery withdrew this approval because the latest commits introduced blocking findings.
|
This PR was published as 0.326.0. Thank you for contributing! |
3 participants
Summary
types=SDL and release impact
This is a minor release change. Input, enum, and scalar types referenced only by directive arguments are now ordinary schema types and may appear in generated SDL in normal sorted order.
Federation-private directive support types such as
_FieldSetremain available to runtime introspection but are omitted fromstrawberry.federation.Schema.as_str()as before._Anyremains in Federation SDL because it is part of the executable Federation API.print_definition=Falsestill omits the directive definition itself, while the runtime directive and its supporting types remain available to introspection. Using Federation directives withstrawberry.Schemaretains its existing warning behavior;strawberry.federation.Schemaregisters them for introspection.Testing
uv run pytest tests/schema tests/test_printer tests/federation/printer tests/federation/test_schema.py -qwith graphql-core 3.2.11 (812 passed, 27 skipped, 2 xfailed)uv run ruff check .uv run ruff format --check .uv run mypy --config-file mypy.ini strawberry/schema/base.py strawberry/schema/schema.py strawberry/printer/printer.py strawberry/federation/schema.pySummary by Sourcery
Expose attached schema directives through GraphQL introspection while constructing and validating their complete supporting type graph in a single schema build.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: