From 5aae99421e2c63e571d60c9f31424ae2c6dd6b55 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 14:53:33 +0000 Subject: [PATCH 01/18] Expose attached schema directives through introspection Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- RELEASE.md | 22 ++ strawberry/printer/printer.py | 43 +++- strawberry/schema/base.py | 1 + strawberry/schema/schema.py | 198 +++++++++++++-- tests/schema/test_basic.py | 9 +- tests/schema/test_extensions.py | 13 +- tests/schema/test_schema_directives.py | 337 +++++++++++++++++++++++++ 7 files changed, 584 insertions(+), 39 deletions(-) create mode 100644 RELEASE.md create mode 100644 tests/schema/test_schema_directives.py diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000000..0997a9e1a5 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,22 @@ +--- +release type: patch +social_messages: + x: >- + {project_name} {version} is out! Custom schema directives attached to types and + fields now appear in GraphQL introspection. 🍓 + https://strawberry.rocks/release/{version} + linkedin: >- + {project_name} {version} is out. GraphQL tools can now discover custom schema + directives attached throughout a Strawberry schema using standard + introspection. 🍓 +--- + +This release fixes introspection for custom schema directives. + +Schema directives attached to types, fields, arguments, and other schema elements +now appear in standard GraphQL introspection. Schema explorers, IDEs, code +generators, and other tools can discover each directive's description, arguments, +allowed locations, repeatability, and any input types it uses. + +Existing Strawberry SDL output stays compatible, and a directive reused across +the schema is defined only once. diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index a349cfe00b..c1ad7166e9 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -10,10 +10,17 @@ overload, ) -from graphql import GraphQLInputField, GraphQLObjectType, GraphQLSchema, is_union_type +from graphql import ( + GraphQLInputField, + GraphQLObjectType, + GraphQLSchema, + get_named_type, + is_union_type, +) from graphql.language.printer import print_ast from graphql.type import ( is_enum_type, + is_input_object_type, is_input_type, is_interface_type, is_object_type, @@ -49,6 +56,7 @@ GraphQLArgument, GraphQLEnumType, GraphQLEnumValue, + GraphQLInputObjectType, GraphQLNamedType, GraphQLScalarType, GraphQLUnionType, @@ -609,7 +617,8 @@ def print_schema(schema: BaseSchema) -> str: types = [ type_ for type_name in sorted(type_map) - if is_defined_type(type_ := type_map[type_name]) + if type_name not in schema._schema_directive_argument_types + and is_defined_type(type_ := type_map[type_name]) ] types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] @@ -619,6 +628,7 @@ def print_schema(schema: BaseSchema) -> str: printed_directive for directive in filtered_directives if (printed_directive := print_directive(directive, schema=schema)) is not None + and printed_directive not in extras.directives ] if schema.config.enable_experimental_incremental_execution: @@ -638,6 +648,8 @@ def _name_getter(type_: Any) -> str: def _print_extra_types() -> Iterable[str]: # Make sure extra types are ordered for predictive printing + graphql_types: dict[str, GraphQLNamedType] = {} + for type_ in sorted(extras.types, key=_name_getter): graphql_type = cast( "GraphQLNamedType", schema.schema_converter.from_type(type_) @@ -646,9 +658,34 @@ def _print_extra_types() -> Iterable[str]: # Skip types that are already part of the schema's type map, otherwise # they'd be printed twice (e.g. an enum used both as a regular type and # as a schema directive field), producing invalid SDL. - if graphql_type.name in type_map: + if ( + graphql_type.name in type_map + and graphql_type.name not in schema._schema_directive_argument_types + ): continue + graphql_types[graphql_type.name] = graphql_type + + def add_referenced_types(graphql_type: GraphQLNamedType) -> None: + if not is_input_object_type(graphql_type): + return + + input_type = cast("GraphQLInputObjectType", graphql_type) + for field in input_type.fields.values(): + field_type = get_named_type(field.type) + if field_type.name in graphql_types or ( + field_type.name in type_map + and field_type.name not in schema._schema_directive_argument_types + ): + continue + + graphql_types[field_type.name] = field_type + add_referenced_types(field_type) + + for graphql_type in tuple(graphql_types.values()): + add_referenced_types(graphql_type) + + for graphql_type in graphql_types.values(): yield _print_type(graphql_type, schema, extras=extras) return "\n\n".join( diff --git a/strawberry/schema/base.py b/strawberry/schema/base.py index 526aeb5c35..74d717bbee 100644 --- a/strawberry/schema/base.py +++ b/strawberry/schema/base.py @@ -38,6 +38,7 @@ class BaseSchema(Protocol): mutation: type[WithStrawberryObjectDefinition] | None subscription: type[WithStrawberryObjectDefinition] | None schema_directives: list[object] + _schema_directive_argument_types: set[str] exception_handlers: tuple[ExceptionHandler[Any], ...] @abstractmethod diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index c3d8de122e..410ead132e 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -3,7 +3,14 @@ import asyncio import warnings from asyncio import ensure_future -from collections.abc import AsyncGenerator, AsyncIterator, Awaitable, Callable, Iterable +from collections.abc import ( + AsyncGenerator, + AsyncIterator, + Awaitable, + Callable, + Iterable, + Iterator, +) from functools import lru_cache from inspect import isawaitable from typing import ( @@ -21,6 +28,7 @@ FieldNode, FragmentDefinitionNode, GraphQLBoolean, + GraphQLDirective, GraphQLError, GraphQLField, GraphQLNamedType, @@ -346,7 +354,7 @@ class Query: exception_handlers=self.exception_handlers, ) - self.directives = directives + self.directives = tuple(directives) self.schema_directives = list(schema_directives) query_type = self.schema_converter.from_object( @@ -373,16 +381,16 @@ class Query: else None ) - graphql_directives = [ - self.schema_converter.from_directive(directive) for directive in directives + self._operation_graphql_directives = [ + self.schema_converter.from_directive(directive) + for directive in self.directives ] graphql_types = [] + explicit_schema_directive_types = [] for type_ in types: if compat.is_schema_directive(type_): - graphql_directives.append( - self.schema_converter.from_schema_directive(type_) - ) + explicit_schema_directive_types.append(type_) else: if ( has_object_definition(type_) @@ -396,22 +404,18 @@ class Query: raise TypeError(f"{graphql_type} is not a named GraphQL Type") graphql_types.append(graphql_type) + self._graphql_query_type = query_type + self._graphql_mutation_type = mutation_type + self._graphql_subscription_type = subscription_type + self._graphql_types = graphql_types + self._explicit_schema_directive_types = tuple(explicit_schema_directive_types) + self._schema_directive_argument_types: set[str] = set() + try: - directives = specified_directives + tuple(graphql_directives) # type: ignore - - if self.config.enable_experimental_incremental_execution: - directives = tuple(directives) + tuple(incremental_execution_directives) - - self._schema = GraphQLSchema( - query=query_type, - mutation=mutation_type, - subscription=subscription_type if subscription else None, - directives=directives, # type: ignore - types=graphql_types, - extensions={ - GraphQLCoreConverter.DEFINITION_BACKREF: self, - }, + self._schema = self._create_graphql_schema( + self._explicit_schema_directive_types ) + self._register_schema_directives() except TypeError as error: # GraphQL core throws a TypeError if there's any exception raised @@ -439,6 +443,158 @@ class Query: formatted_errors = "\n\n".join(f"❌ {error.message}" for error in errors) raise ValueError(f"Invalid Schema. Errors:\n\n{formatted_errors}") + def _create_graphql_schema( + self, schema_directive_types: Iterable[type] + ) -> GraphQLSchema: + directives = list(specified_directives) + directive_definitions: dict[str, object] = { + directive.name: directive for directive in directives + } + directive_sources = { + directive.name: "the built-in GraphQL directive" for directive in directives + } + + def add_directive( + directive: GraphQLDirective, + definition: object, + source: str, + ) -> None: + name = directive.name + if name not in directive_definitions: + directive_definitions[name] = definition + directive_sources[name] = source + directives.append(directive) + return + + if directive_definitions[name] is definition: + return + + raise ValueError( + f"Schema directive '@{name}' is defined by both " + f"{directive_sources[name]} and {source}. Use a different " + "GraphQL name for one of them." + ) + + for directive in self._operation_graphql_directives: + strawberry_directive = directive.extensions[ + GraphQLCoreConverter.DEFINITION_BACKREF + ] + add_directive( + directive, + strawberry_directive, + f"operation directive '{strawberry_directive.python_name}'", + ) + + from strawberry.schema_directives import OneOf + + for directive_type in schema_directive_types: + strawberry_directive = cast("Any", directive_type).__strawberry_directive__ + directive_name = self.config.name_converter.from_directive( + strawberry_directive + ) + + # Strawberry's OneOf schema directive predates graphql-core exposing + # the specified @oneOf directive. Keep its SDL behavior while using + # the canonical runtime directive. + if ( + directive_type is OneOf + and directive_name == "oneOf" + and directive_name in directive_definitions + ): + continue + + add_directive( + self.schema_converter.from_schema_directive(directive_type), + directive_type, + ( + "schema directive " + f"'{directive_type.__module__}.{directive_type.__qualname__}'" + ), + ) + + if self.config.enable_experimental_incremental_execution: + for directive in incremental_execution_directives: + add_directive( + directive, + directive, + f"the experimental GraphQL directive '@{directive.name}'", + ) + + return GraphQLSchema( + query=self._graphql_query_type, + mutation=self._graphql_mutation_type, + subscription=self._graphql_subscription_type, + directives=tuple(directives), + types=self._graphql_types, + extensions={ + GraphQLCoreConverter.DEFINITION_BACKREF: self, + }, + ) + + def _register_schema_directives(self) -> None: + schema_directive_types = list(self._explicit_schema_directive_types) + seen_directive_types = set(schema_directive_types) + + def add_directive(directive: object) -> None: + directive_type = directive.__class__ + if ( + compat.is_schema_directive(directive_type) + and directive_type not in seen_directive_types + ): + seen_directive_types.add(directive_type) + schema_directive_types.append(directive_type) + + def add_directives(owner: object) -> None: + attached_directives = getattr(owner, "directives", ()) or () + if isinstance(attached_directives, Iterator): + attached_directives = list(attached_directives) + owner.directives = attached_directives # type: ignore[attr-defined] + + for directive in attached_directives: + add_directive(directive) + + for directive in self.schema_directives: + add_directive(directive) + + for graphql_type in self._schema.type_map.values(): + type_definition = (getattr(graphql_type, "extensions", None) or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if type_definition is not None: + add_directives(type_definition) + + for field in getattr(graphql_type, "fields", {}).values(): + field_definition = (field.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if field_definition is not None: + add_directives(field_definition) + + for argument in getattr(field, "args", {}).values(): + argument_definition = (argument.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if argument_definition is not None: + add_directives(argument_definition) + + for value in getattr(graphql_type, "values", {}).values(): + value_definition = (value.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if value_definition is not None: + add_directives(value_definition) + + self._schema_directive_types = tuple(schema_directive_types) + + if self._schema_directive_types == self._explicit_schema_directive_types: + return + + existing_type_names = set(self._schema.type_map) + self._schema = self._create_graphql_schema(self._schema_directive_types) + self._schema_directive_argument_types.update( + set(self._schema.type_map) - existing_type_names + ) + def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning # is emitted once at ``Schema.__init__``; users are expected to migrate diff --git a/tests/schema/test_basic.py b/tests/schema/test_basic.py index cd36de2724..4dfbf8f0cb 100644 --- a/tests/schema/test_basic.py +++ b/tests/schema/test_basic.py @@ -512,15 +512,16 @@ class Input: class Query: foo: int - @strawberry.schema_directive(locations=[Location.SCALAR], name="specifiedBy") - class SpecifiedBy: + @strawberry.schema_directive(locations=[Location.SCALAR], name="customDirective") + class CustomDirective: name: str schema = strawberry.Schema( - query=Query, types=[Type, Interface, Input, Base64, ID, str, int, SpecifiedBy] + query=Query, + types=[Type, Interface, Input, Base64, ID, str, int, CustomDirective], ) expected = ''' - directive @specifiedBy(name: String!) on SCALAR + directive @customDirective(name: String!) on SCALAR """ Represents binary data as Base64-encoded strings, using the standard alphabet. diff --git a/tests/schema/test_extensions.py b/tests/schema/test_extensions.py index d5ef1d2783..cdb50880be 100644 --- a/tests/schema/test_extensions.py +++ b/tests/schema/test_extensions.py @@ -14,7 +14,6 @@ from strawberry.scalars import JSON from strawberry.schema.schema_converter import GraphQLCoreConverter from strawberry.schema_directive import Location -from strawberry.types.base import get_object_definition DEFINITION_BACKREF = GraphQLCoreConverter.DEFINITION_BACKREF @@ -34,16 +33,8 @@ class Query: # Schema assert graphql_schema.extensions[DEFINITION_BACKREF] is schema - # TODO: Apparently I stumbled on a bug: - # SchemaDirective are used on schema.__str__(), - # but aren't added to graphql_schema.directives - # maybe graphql_schema_directive = graphql_schema.get_directive("schemaDirective") - - directives = get_object_definition(Query, strict=True).directives - assert directives is not None - graphql_schema_directive = schema.schema_converter.from_schema_directive( - directives[0] - ) + graphql_schema_directive = graphql_schema.get_directive("schemaDirective") + assert graphql_schema_directive is not None assert ( graphql_schema_directive.extensions[DEFINITION_BACKREF] is SchemaDirective.__strawberry_directive__ diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py new file mode 100644 index 0000000000..cb86445fd2 --- /dev/null +++ b/tests/schema/test_schema_directives.py @@ -0,0 +1,337 @@ +from enum import Enum +from typing import Annotated + +import pytest +from graphql import ( + GraphQLEnumType, + GraphQLInputObjectType, + GraphQLScalarType, + build_schema, + get_named_type, +) + +import strawberry +from strawberry.schema_directive import Location + + +def test_registers_reused_directives_and_preserves_application_order(): + @strawberry.schema_directive( + locations=[Location.SCHEMA, Location.OBJECT, Location.FIELD_DEFINITION], + repeatable=True, + ) + class Marker: + name: str + + @strawberry.type(directives=[Marker(name="object-1"), Marker(name="object-2")]) + class Query: + name: str = strawberry.field( + default="Patrick", directives=[Marker(name="field")] + ) + + schema = strawberry.Schema(query=Query, schema_directives=[Marker(name="schema")]) + + directives = [ + directive + for directive in schema._schema.directives + if directive.name == "marker" + ] + assert len(directives) == 1 + + sdl = schema.as_str() + assert sdl.count("directive @marker") == 1 + assert 'schema @marker(name: "schema")' in sdl + assert 'type Query @marker(name: "object-1") @marker(name: "object-2")' in sdl + assert 'name: String! @marker(name: "field")' in sdl + build_schema(sdl) + + +def test_introspection_exposes_schema_directive_metadata(): + @strawberry.schema_directive( + name="accessPolicy", + description="Controls access to a schema element.", + locations=[Location.OBJECT, Location.FIELD_DEFINITION], + repeatable=True, + ) + class AccessPolicy: + role: str + level: int = 1 + + @strawberry.type(directives=[AccessPolicy(role="admin")]) + class Query: + name: str + + schema = strawberry.Schema(query=Query) + result = schema.execute_sync( + """ + { + __schema { + directives { + name + description + isRepeatable + locations + args { + name + defaultValue + type { + kind + name + ofType { + kind + name + } + } + } + } + } + } + """ + ) + + assert result.errors is None + directive = next( + directive + for directive in result.data["__schema"]["directives"] + if directive["name"] == "accessPolicy" + ) + assert directive == { + "name": "accessPolicy", + "description": "Controls access to a schema element.", + "isRepeatable": True, + "locations": ["OBJECT", "FIELD_DEFINITION"], + "args": [ + { + "name": "role", + "defaultValue": None, + "type": { + "kind": "NON_NULL", + "name": None, + "ofType": {"kind": "SCALAR", "name": "String"}, + }, + }, + { + "name": "level", + "defaultValue": "1", + "type": { + "kind": "NON_NULL", + "name": None, + "ofType": {"kind": "SCALAR", "name": "Int"}, + }, + }, + ], + } + + +def test_explicit_directive_type_is_reconciled_with_attached_uses(): + @strawberry.schema_directive(locations=[Location.OBJECT]) + class Marker: ... + + @strawberry.type(directives=[Marker()]) + class Query: + name: str + + schema = strawberry.Schema(query=Query, types=[Marker, Marker]) + + assert ( + sum(directive.name == "marker" for directive in schema._schema.directives) == 1 + ) + sdl = schema.as_str() + assert sdl.count("directive @marker") == 1 + build_schema(sdl) + + +def test_registers_nested_directive_argument_input_types(): + @strawberry.enum + class Mode(Enum): + PRIVATE = "private" + + Secret = strawberry.scalar(str, name="Secret") + + @strawberry.input + class Rule: + value: str + + @strawberry.input + class Policy: + rule: Rule + mode: Mode + secret: Secret + + @strawberry.schema_directive(locations=[Location.FIELD_DEFINITION]) + class Protected: + policy: Policy + + @strawberry.type + class Query: + name: str = strawberry.field( + default="Patrick", + directives=[ + Protected( + policy=Policy( + rule=Rule(value="private"), + mode=Mode.PRIVATE, + secret="token", + ) + ) + ], + ) + + schema = strawberry.Schema(query=Query) + + policy_type = schema._schema.get_type("Policy") + rule_type = schema._schema.get_type("Rule") + assert isinstance(policy_type, GraphQLInputObjectType) + assert isinstance(rule_type, GraphQLInputObjectType) + assert isinstance(schema._schema.get_type("Mode"), GraphQLEnumType) + assert isinstance(schema._schema.get_type("Secret"), GraphQLScalarType) + assert get_named_type(policy_type.fields["rule"].type) is rule_type + + sdl = schema.as_str() + assert sdl.count("input Policy") == 1 + assert sdl.count("input Rule") == 1 + assert sdl.count("enum Mode") == 1 + assert sdl.count("scalar Secret") == 1 + build_schema(sdl) + + +def test_print_definition_false_remains_available_to_introspection(): + @strawberry.input + class HiddenConfig: + reason: str + + @strawberry.schema_directive( + locations=[Location.FIELD_DEFINITION], print_definition=False + ) + class Hidden: + config: HiddenConfig + + @strawberry.type + class Query: + name: str = strawberry.field( + default="Patrick", + directives=[Hidden(config=HiddenConfig(reason="private"))], + ) + + schema = strawberry.Schema(query=Query) + + assert schema._schema.get_directive("hidden") is not None + assert schema._schema.get_type("HiddenConfig") is not None + + sdl = schema.as_str() + assert "directive @hidden" not in sdl + assert "input HiddenConfig" not in sdl + assert "@hidden(config:" in sdl + assert 'reason: "private"' in sdl + + +def test_registers_directives_from_all_type_system_attachment_points(): + def directive(name: str, location: Location) -> type: + @strawberry.schema_directive(name=name, locations=[location]) + class Directive: ... + + return Directive + + InterfaceDirective = directive("onInterface", Location.INTERFACE) + UnionDirective = directive("onUnion", Location.UNION) + EnumDirective = directive("onEnum", Location.ENUM) + EnumValueDirective = directive("onEnumValue", Location.ENUM_VALUE) + ScalarDirective = directive("onScalar", Location.SCALAR) + InputDirective = directive("onInput", Location.INPUT_OBJECT) + InputFieldDirective = directive("onInputField", Location.INPUT_FIELD_DEFINITION) + ArgumentDirective = directive("onArgument", Location.ARGUMENT_DEFINITION) + + @strawberry.interface(directives=[InterfaceDirective()]) + class Node: + id: strawberry.ID + + @strawberry.type + class Item(Node): + name: str + + @strawberry.type + class Other: + value: str + + Result = Annotated[ + Item | Other, + strawberry.union("Result", directives=[UnionDirective()]), + ] + + @strawberry.enum(directives=[EnumDirective()]) + class Choice(Enum): + FIRST = strawberry.enum_value("first", directives=[EnumValueDirective()]) + + CustomScalar = strawberry.scalar( + str, name="CustomScalar", directives=[ScalarDirective()] + ) + + @strawberry.input(directives=[InputDirective()]) + class Filter: + term: str = strawberry.field(directives=[InputFieldDirective()]) + + @strawberry.type + class Query: + node: Node + result: Result + choice: Choice + custom_scalar: CustomScalar + + @strawberry.field + def search( + self, + filter: Filter, + term: Annotated[str, strawberry.argument(directives=[ArgumentDirective()])], + ) -> str: + return filter.term + term + + schema = strawberry.Schema(query=Query, types=[Item]) + directive_names = {item.name for item in schema._schema.directives} + + assert { + "onInterface", + "onUnion", + "onEnum", + "onEnumValue", + "onScalar", + "onInput", + "onInputField", + "onArgument", + } <= directive_names + + +def test_rejects_conflicting_schema_directive_names(): + @strawberry.schema_directive(name="conflict", locations=[Location.OBJECT]) + class First: ... + + @strawberry.schema_directive(name="conflict", locations=[Location.FIELD_DEFINITION]) + class Second: ... + + @strawberry.type(directives=[First()]) + class Query: + name: str = strawberry.field(default="Patrick", directives=[Second()]) + + with pytest.raises( + ValueError, + match=( + r"Schema directive '@conflict' is defined by both .*First.* and .*Second" + ), + ): + strawberry.Schema(query=Query) + + +def test_rejects_schema_directive_collisions_with_specified_directives(): + @strawberry.schema_directive(name="skip", locations=[Location.OBJECT]) + class CustomSkip: ... + + @strawberry.type(directives=[CustomSkip()]) + class Query: + name: str + + with pytest.raises( + ValueError, + match=( + r"Schema directive '@skip' is defined by both the built-in GraphQL " + "directive and schema directive .*CustomSkip" + ), + ): + strawberry.Schema(query=Query) From 9013ef7c3e61b1d2ad4f565237fb1fdf7c970abb Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 15:12:14 +0000 Subject: [PATCH 02/18] Print directive argument types as schema types Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- RELEASE.md | 7 ++-- strawberry/printer/printer.py | 35 ++----------------- strawberry/schema/base.py | 1 - strawberry/schema/schema.py | 5 --- .../printer/test_additional_directives.py | 4 +++ .../printer/test_compose_directive.py | 4 +++ tests/federation/printer/test_entities.py | 2 ++ tests/federation/printer/test_inaccessible.py | 2 ++ tests/federation/printer/test_interface.py | 2 ++ .../printer/test_interface_object.py | 2 ++ tests/federation/printer/test_keys.py | 2 ++ tests/federation/printer/test_link.py | 24 +++++++++++++ tests/federation/printer/test_override.py | 4 +++ tests/federation/printer/test_provides.py | 4 +++ tests/federation/printer/test_requires.py | 2 ++ tests/federation/printer/test_shareable.py | 2 ++ tests/schema/test_schema_directives.py | 2 +- tests/test_printer/test_schema_directives.py | 26 +++++++------- 18 files changed, 74 insertions(+), 56 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 0997a9e1a5..9d913b0f71 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,5 +1,5 @@ --- -release type: patch +release type: minor social_messages: x: >- {project_name} {version} is out! Custom schema directives attached to types and @@ -18,5 +18,6 @@ now appear in standard GraphQL introspection. Schema explorers, IDEs, code generators, and other tools can discover each directive's description, arguments, allowed locations, repeatability, and any input types it uses. -Existing Strawberry SDL output stays compatible, and a directive reused across -the schema is defined only once. +A directive reused across the schema is defined only once. Input, enum, and scalar +types referenced by directive arguments are now part of the schema and may appear +in generated SDL even when they are not used by fields. diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index c1ad7166e9..ce6dfee11a 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -14,13 +14,11 @@ GraphQLInputField, GraphQLObjectType, GraphQLSchema, - get_named_type, is_union_type, ) from graphql.language.printer import print_ast from graphql.type import ( is_enum_type, - is_input_object_type, is_input_type, is_interface_type, is_object_type, @@ -56,7 +54,6 @@ GraphQLArgument, GraphQLEnumType, GraphQLEnumValue, - GraphQLInputObjectType, GraphQLNamedType, GraphQLScalarType, GraphQLUnionType, @@ -617,8 +614,7 @@ def print_schema(schema: BaseSchema) -> str: types = [ type_ for type_name in sorted(type_map) - if type_name not in schema._schema_directive_argument_types - and is_defined_type(type_ := type_map[type_name]) + if is_defined_type(type_ := type_map[type_name]) ] types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] @@ -648,8 +644,6 @@ def _name_getter(type_: Any) -> str: def _print_extra_types() -> Iterable[str]: # Make sure extra types are ordered for predictive printing - graphql_types: dict[str, GraphQLNamedType] = {} - for type_ in sorted(extras.types, key=_name_getter): graphql_type = cast( "GraphQLNamedType", schema.schema_converter.from_type(type_) @@ -658,34 +652,9 @@ def _print_extra_types() -> Iterable[str]: # Skip types that are already part of the schema's type map, otherwise # they'd be printed twice (e.g. an enum used both as a regular type and # as a schema directive field), producing invalid SDL. - if ( - graphql_type.name in type_map - and graphql_type.name not in schema._schema_directive_argument_types - ): + if graphql_type.name in type_map: continue - graphql_types[graphql_type.name] = graphql_type - - def add_referenced_types(graphql_type: GraphQLNamedType) -> None: - if not is_input_object_type(graphql_type): - return - - input_type = cast("GraphQLInputObjectType", graphql_type) - for field in input_type.fields.values(): - field_type = get_named_type(field.type) - if field_type.name in graphql_types or ( - field_type.name in type_map - and field_type.name not in schema._schema_directive_argument_types - ): - continue - - graphql_types[field_type.name] = field_type - add_referenced_types(field_type) - - for graphql_type in tuple(graphql_types.values()): - add_referenced_types(graphql_type) - - for graphql_type in graphql_types.values(): yield _print_type(graphql_type, schema, extras=extras) return "\n\n".join( diff --git a/strawberry/schema/base.py b/strawberry/schema/base.py index 74d717bbee..526aeb5c35 100644 --- a/strawberry/schema/base.py +++ b/strawberry/schema/base.py @@ -38,7 +38,6 @@ class BaseSchema(Protocol): mutation: type[WithStrawberryObjectDefinition] | None subscription: type[WithStrawberryObjectDefinition] | None schema_directives: list[object] - _schema_directive_argument_types: set[str] exception_handlers: tuple[ExceptionHandler[Any], ...] @abstractmethod diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 410ead132e..072444ce9d 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -409,7 +409,6 @@ class Query: self._graphql_subscription_type = subscription_type self._graphql_types = graphql_types self._explicit_schema_directive_types = tuple(explicit_schema_directive_types) - self._schema_directive_argument_types: set[str] = set() try: self._schema = self._create_graphql_schema( @@ -589,11 +588,7 @@ def add_directives(owner: object) -> None: if self._schema_directive_types == self._explicit_schema_directive_types: return - existing_type_names = set(self._schema.type_map) self._schema = self._create_graphql_schema(self._schema_directive_types) - self._schema_directive_argument_types.update( - set(self._schema.type_map) - existing_type_names - ) def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning diff --git a/tests/federation/printer/test_additional_directives.py b/tests/federation/printer/test_additional_directives.py index e0d4b4ef16..e4dd04bfcc 100644 --- a/tests/federation/printer/test_additional_directives.py +++ b/tests/federation/printer/test_additional_directives.py @@ -43,6 +43,8 @@ class Query: union _Entity = FederatedType + scalar _FieldSet + type _Service { sdl: String! } @@ -99,6 +101,8 @@ class Query: union _Entity = FederatedType + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_compose_directive.py b/tests/federation/printer/test_compose_directive.py index 780cccb559..54704c7a2c 100644 --- a/tests/federation/printer/test_compose_directive.py +++ b/tests/federation/printer/test_compose_directive.py @@ -55,6 +55,8 @@ class Query: union _Entity = FederatedType + scalar _FieldSet + type _Service { sdl: String! } @@ -119,6 +121,8 @@ class Query: union _Entity = FederatedType + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_entities.py b/tests/federation/printer/test_entities.py index 28df060bed..f7a6641efc 100644 --- a/tests/federation/printer/test_entities.py +++ b/tests/federation/printer/test_entities.py @@ -124,6 +124,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_inaccessible.py b/tests/federation/printer/test_inaccessible.py index b0aa7c9b77..8acbe607d5 100644 --- a/tests/federation/printer/test_inaccessible.py +++ b/tests/federation/printer/test_inaccessible.py @@ -81,6 +81,8 @@ def top_products( union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_interface.py b/tests/federation/printer/test_interface.py index 27ecbb36eb..0ec435a3ef 100644 --- a/tests/federation/printer/test_interface.py +++ b/tests/federation/printer/test_interface.py @@ -44,6 +44,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_interface_object.py b/tests/federation/printer/test_interface_object.py index 3011fedf79..dccfe8d710 100644 --- a/tests/federation/printer/test_interface_object.py +++ b/tests/federation/printer/test_interface_object.py @@ -28,6 +28,8 @@ class SomeInterface: union _Entity = SomeInterface + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_keys.py b/tests/federation/printer/test_keys.py index 1de6f8589a..59741d53ab 100644 --- a/tests/federation/printer/test_keys.py +++ b/tests/federation/printer/test_keys.py @@ -62,6 +62,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product | Review + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_link.py b/tests/federation/printer/test_link.py index 6151a133be..90fea0fd35 100644 --- a/tests/federation/printer/test_link.py +++ b/tests/federation/printer/test_link.py @@ -34,6 +34,13 @@ class Query: type _Service { sdl: String! } + + scalar link__Import + + enum link__Purpose { + SECURITY + EXECUTION + } """ assert schema.as_str() == textwrap.dedent(expected).strip() @@ -90,6 +97,13 @@ class Query: type _Service { sdl: String! } + + scalar link__Import + + enum link__Purpose { + SECURITY + EXECUTION + } """ assert schema.as_str() == textwrap.dedent(expected).strip() @@ -125,6 +139,8 @@ class Query: union _Entity = User + scalar _FieldSet + type _Service { sdl: String! } @@ -168,6 +184,8 @@ class Query: scalar _Any + scalar _FieldSet + type _Service { sdl: String! } @@ -251,6 +269,8 @@ class Query: union _Entity = User + scalar _FieldSet + type _Service { sdl: String! } @@ -289,6 +309,8 @@ class Query: union _Entity = User + scalar _FieldSet + type _Service { sdl: String! } @@ -336,6 +358,8 @@ class Query: union _Entity = User + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_override.py b/tests/federation/printer/test_override.py index 82364dfd75..2d75095318 100644 --- a/tests/federation/printer/test_override.py +++ b/tests/federation/printer/test_override.py @@ -47,6 +47,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } @@ -99,6 +101,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_provides.py b/tests/federation/printer/test_provides.py index 0403b0947b..2428b86f70 100644 --- a/tests/federation/printer/test_provides.py +++ b/tests/federation/printer/test_provides.py @@ -67,6 +67,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } @@ -138,6 +140,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_requires.py b/tests/federation/printer/test_requires.py index 00980b31e8..98279508af 100644 --- a/tests/federation/printer/test_requires.py +++ b/tests/federation/printer/test_requires.py @@ -70,6 +70,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/federation/printer/test_shareable.py b/tests/federation/printer/test_shareable.py index 2555ee75e9..0029c1ed2d 100644 --- a/tests/federation/printer/test_shareable.py +++ b/tests/federation/printer/test_shareable.py @@ -46,6 +46,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index cb86445fd2..3c82451035 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -219,7 +219,7 @@ class Query: sdl = schema.as_str() assert "directive @hidden" not in sdl - assert "input HiddenConfig" not in sdl + assert "input HiddenConfig" in sdl assert "@hidden(config:" in sdl assert 'reason: "private"' in sdl diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 055d6e57cd..26e270ee32 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -132,17 +132,17 @@ def user(self, input: Input) -> User: user(input: Input!): User! } + input SensitiveValue { + key: String! + value: String! + } + type User @sensitiveData(reason: "GDPR") { firstName: String! age: Int! phone: String! @sensitiveData(reason: "PRIVATE", meta: [{ key: "can_share_field", value: "phone_share_accepted" }]) phoneShareAccepted: Boolean! } - - input SensitiveValue { - key: String! - value: String! - } """ schema = strawberry.Schema(query=Query) @@ -831,14 +831,14 @@ def foo(self, info: strawberry.Info) -> str: ... expected_output = """ directive @fooDirective(input: FooInput!, optionalInput: FooInput) on FIELD_DEFINITION - type Query { - foo: String! @fooDirective(input: { a: "something" }) - } - input FooInput { a: String b: String } + + type Query { + foo: String! @fooDirective(input: { a: "something" }) + } """ schema = strawberry.Schema(query=Query) @@ -872,14 +872,14 @@ def foo(self, info: strawberry.Info) -> str: ... expected_output = """ directive @fooDirective(input: FooInput!, optionalInput: FooInput) on FIELD_DEFINITION - type Query { - foo: String! @fooDirective(input: { hello: "hello", helloWorld: "hello world" }) - } - input FooInput { hello: String! helloWorld: String! } + + type Query { + foo: String! @fooDirective(input: { hello: "hello", helloWorld: "hello world" }) + } """ schema = strawberry.Schema(query=Query) From c58a8f10130f81ec63fe2c5ec233b8c0ecec52aa Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 15:22:11 +0000 Subject: [PATCH 03/18] Discover directives on directive argument types Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/federation/schema.py | 3 ++ strawberry/schema/schema.py | 63 ++++++++++++++++---------- tests/federation/test_schema.py | 2 + tests/schema/test_schema_directives.py | 14 +++++- 4 files changed, 55 insertions(+), 27 deletions(-) diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index 4a6d935c42..869334a067 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -367,6 +367,9 @@ def _warn_for_federation_directives(self) -> None: pass + def _should_register_schema_directive(self, directive: object) -> bool: + return True + def _get_entity_type( query: type[WithStrawberryObjectDefinition] | None, diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 072444ce9d..a190ea825f 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -537,7 +537,8 @@ def _register_schema_directives(self) -> None: def add_directive(directive: object) -> None: directive_type = directive.__class__ if ( - compat.is_schema_directive(directive_type) + self._should_register_schema_directive(directive) + and compat.is_schema_directive(directive_type) and directive_type not in seen_directive_types ): seen_directive_types.add(directive_type) @@ -555,40 +556,47 @@ def add_directives(owner: object) -> None: for directive in self.schema_directives: add_directive(directive) - for graphql_type in self._schema.type_map.values(): - type_definition = (getattr(graphql_type, "extensions", None) or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if type_definition is not None: - add_directives(type_definition) + registered_directive_types = self._explicit_schema_directive_types - for field in getattr(graphql_type, "fields", {}).values(): - field_definition = (field.extensions or {}).get( + # Directive arguments can add types that have their own attached directives. + # Rebuild and rescan until every reachable directive has been registered. + while True: + for graphql_type in self._schema.type_map.values(): + type_definition = (getattr(graphql_type, "extensions", None) or {}).get( GraphQLCoreConverter.DEFINITION_BACKREF ) - if field_definition is not None: - add_directives(field_definition) + if type_definition is not None: + add_directives(type_definition) - for argument in getattr(field, "args", {}).values(): - argument_definition = (argument.extensions or {}).get( + for field in getattr(graphql_type, "fields", {}).values(): + field_definition = (field.extensions or {}).get( GraphQLCoreConverter.DEFINITION_BACKREF ) - if argument_definition is not None: - add_directives(argument_definition) + if field_definition is not None: + add_directives(field_definition) - for value in getattr(graphql_type, "values", {}).values(): - value_definition = (value.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if value_definition is not None: - add_directives(value_definition) + for argument in getattr(field, "args", {}).values(): + argument_definition = (argument.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if argument_definition is not None: + add_directives(argument_definition) + + for value in getattr(graphql_type, "values", {}).values(): + value_definition = (value.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if value_definition is not None: + add_directives(value_definition) - self._schema_directive_types = tuple(schema_directive_types) + discovered_directive_types = tuple(schema_directive_types) + if discovered_directive_types == registered_directive_types: + break - if self._schema_directive_types == self._explicit_schema_directive_types: - return + self._schema = self._create_graphql_schema(discovered_directive_types) + registered_directive_types = discovered_directive_types - self._schema = self._create_graphql_schema(self._schema_directive_types) + self._schema_directive_types = registered_directive_types def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning @@ -1352,6 +1360,11 @@ def _resolve_node_ids(self) -> None: if not has_custom_resolve_id: origin.resolve_id_attr() + def _should_register_schema_directive(self, directive: object) -> bool: + from strawberry.federation.schema_directives import FederationDirective + + return not isinstance(directive, FederationDirective) + def _warn_for_federation_directives(self) -> None: """Raises a warning if the schema has any federation directives.""" from strawberry.federation.schema_directives import FederationDirective diff --git a/tests/federation/test_schema.py b/tests/federation/test_schema.py index ab596c3513..9113c9e1e7 100644 --- a/tests/federation/test_schema.py +++ b/tests/federation/test_schema.py @@ -101,6 +101,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product + scalar _FieldSet + type _Service { sdl: String! } diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index 3c82451035..541706ba74 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -141,15 +141,21 @@ class Query: def test_registers_nested_directive_argument_input_types(): + @strawberry.schema_directive(locations=[Location.INPUT_OBJECT]) + class OnNestedInput: ... + + @strawberry.schema_directive(locations=[Location.INPUT_FIELD_DEFINITION]) + class OnNestedInputField: ... + @strawberry.enum class Mode(Enum): PRIVATE = "private" Secret = strawberry.scalar(str, name="Secret") - @strawberry.input + @strawberry.input(directives=[OnNestedInput()]) class Rule: - value: str + value: str = strawberry.field(directives=[OnNestedInputField()]) @strawberry.input class Policy: @@ -185,8 +191,12 @@ class Query: assert isinstance(schema._schema.get_type("Mode"), GraphQLEnumType) assert isinstance(schema._schema.get_type("Secret"), GraphQLScalarType) assert get_named_type(policy_type.fields["rule"].type) is rule_type + assert schema._schema.get_directive("onNestedInput") is not None + assert schema._schema.get_directive("onNestedInputField") is not None sdl = schema.as_str() + assert sdl.count("directive @onNestedInput on INPUT_OBJECT") == 1 + assert sdl.count("directive @onNestedInputField on INPUT_FIELD_DEFINITION") == 1 assert sdl.count("input Policy") == 1 assert sdl.count("input Rule") == 1 assert sdl.count("enum Mode") == 1 From 4363a0c6a931d6f8a8c2dd2818f1cfd9538cad94 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 15:45:44 +0000 Subject: [PATCH 04/18] Collect nested directives before rebuilding schema Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/schema/schema.py | 91 +++++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 34 deletions(-) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index a190ea825f..31d4861272 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -38,6 +38,7 @@ GraphQLSchema, OperationDefinitionNode, get_introspection_query, + get_named_type, parse, validate_schema, ) @@ -96,7 +97,7 @@ from graphql.language import DocumentNode from graphql.pyutils import Path - from graphql.type import GraphQLResolveInfo + from graphql.type import GraphQLInputType, GraphQLResolveInfo from graphql.validation import ASTValidationRule from strawberry.directive import StrawberryDirective @@ -533,6 +534,7 @@ def add_directive( def _register_schema_directives(self) -> None: schema_directive_types = list(self._explicit_schema_directive_types) seen_directive_types = set(schema_directive_types) + seen_directive_argument_types: set[str] = set() def add_directive(directive: object) -> None: directive_type = directive.__class__ @@ -553,50 +555,71 @@ def add_directives(owner: object) -> None: for directive in attached_directives: add_directive(directive) - for directive in self.schema_directives: - add_directive(directive) - - registered_directive_types = self._explicit_schema_directive_types + def add_directives_from_graphql_type( + graphql_type: GraphQLNamedType, *, recurse_fields: bool = False + ) -> None: + type_definition = (graphql_type.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if type_definition is not None: + add_directives(type_definition) - # Directive arguments can add types that have their own attached directives. - # Rebuild and rescan until every reachable directive has been registered. - while True: - for graphql_type in self._schema.type_map.values(): - type_definition = (getattr(graphql_type, "extensions", None) or {}).get( + for field in getattr(graphql_type, "fields", {}).values(): + field_definition = (field.extensions or {}).get( GraphQLCoreConverter.DEFINITION_BACKREF ) - if type_definition is not None: - add_directives(type_definition) + if field_definition is not None: + add_directives(field_definition) - for field in getattr(graphql_type, "fields", {}).values(): - field_definition = (field.extensions or {}).get( + for argument in getattr(field, "args", {}).values(): + argument_definition = (argument.extensions or {}).get( GraphQLCoreConverter.DEFINITION_BACKREF ) - if field_definition is not None: - add_directives(field_definition) + if argument_definition is not None: + add_directives(argument_definition) - for argument in getattr(field, "args", {}).values(): - argument_definition = (argument.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if argument_definition is not None: - add_directives(argument_definition) + if recurse_fields: + add_directives_from_argument_type(field.type) - for value in getattr(graphql_type, "values", {}).values(): - value_definition = (value.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if value_definition is not None: - add_directives(value_definition) + for value in getattr(graphql_type, "values", {}).values(): + value_definition = (value.extensions or {}).get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if value_definition is not None: + add_directives(value_definition) - discovered_directive_types = tuple(schema_directive_types) - if discovered_directive_types == registered_directive_types: - break + def add_directives_from_argument_type(type_: GraphQLInputType) -> None: + graphql_type = get_named_type(type_) + if graphql_type.name in seen_directive_argument_types: + return + + seen_directive_argument_types.add(graphql_type.name) + add_directives_from_graphql_type(graphql_type, recurse_fields=True) + + for directive in self.schema_directives: + add_directive(directive) - self._schema = self._create_graphql_schema(discovered_directive_types) - registered_directive_types = discovered_directive_types + for graphql_type in self._schema.type_map.values(): + add_directives_from_graphql_type(graphql_type) + + # Directive arguments can introduce input types with more attached + # directives. Collect that closure before rebuilding the schema once. + directive_index = 0 + while directive_index < len(schema_directive_types): + directive_type = schema_directive_types[directive_index] + graphql_directive = self.schema_converter.from_schema_directive( + directive_type + ) + for argument in graphql_directive.args.values(): + add_directives_from_argument_type(argument.type) + directive_index += 1 + + self._schema_directive_types = tuple(schema_directive_types) + + if self._schema_directive_types == self._explicit_schema_directive_types: + return - self._schema_directive_types = registered_directive_types + self._schema = self._create_graphql_schema(self._schema_directive_types) def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning From 6e321f544a1f68dd1a24e4580b796e2c702c1ca2 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:01:14 +0000 Subject: [PATCH 05/18] Assert complete directive schemas Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- tests/schema/test_schema_directives.py | 96 ++++++++++++++++++-------- 1 file changed, 68 insertions(+), 28 deletions(-) diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index 541706ba74..52bb78ff4e 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -1,3 +1,4 @@ +import textwrap from enum import Enum from typing import Annotated @@ -37,11 +38,20 @@ class Query: ] assert len(directives) == 1 + expected = """ + directive @marker(name: String!) repeatable on SCHEMA | OBJECT | FIELD_DEFINITION + + schema @marker(name: "schema") { + query: Query + } + + type Query @marker(name: "object-1") @marker(name: "object-2") { + name: String! @marker(name: "field") + } + """ + sdl = schema.as_str() - assert sdl.count("directive @marker") == 1 - assert 'schema @marker(name: "schema")' in sdl - assert 'type Query @marker(name: "object-1") @marker(name: "object-2")' in sdl - assert 'name: String! @marker(name: "field")' in sdl + assert sdl == textwrap.dedent(expected).strip() build_schema(sdl) @@ -135,8 +145,17 @@ class Query: assert ( sum(directive.name == "marker" for directive in schema._schema.directives) == 1 ) + + expected = """ + directive @marker on OBJECT + + type Query @marker { + name: String! + } + """ + sdl = schema.as_str() - assert sdl.count("directive @marker") == 1 + assert sdl == textwrap.dedent(expected).strip() build_schema(sdl) @@ -165,21 +184,13 @@ class Policy: @strawberry.schema_directive(locations=[Location.FIELD_DEFINITION]) class Protected: - policy: Policy + policy: Policy | None = strawberry.UNSET @strawberry.type class Query: name: str = strawberry.field( default="Patrick", - directives=[ - Protected( - policy=Policy( - rule=Rule(value="private"), - mode=Mode.PRIVATE, - secret="token", - ) - ) - ], + directives=[Protected()], ) schema = strawberry.Schema(query=Query) @@ -194,13 +205,36 @@ class Query: assert schema._schema.get_directive("onNestedInput") is not None assert schema._schema.get_directive("onNestedInputField") is not None + expected = """ + directive @onNestedInput on INPUT_OBJECT + + directive @onNestedInputField on INPUT_FIELD_DEFINITION + + directive @protected(policy: Policy) on FIELD_DEFINITION + + enum Mode { + PRIVATE + } + + input Policy { + rule: Rule! + mode: Mode! + secret: Secret! + } + + type Query { + name: String! @protected + } + + input Rule @onNestedInput { + value: String! @onNestedInputField + } + + scalar Secret + """ + sdl = schema.as_str() - assert sdl.count("directive @onNestedInput on INPUT_OBJECT") == 1 - assert sdl.count("directive @onNestedInputField on INPUT_FIELD_DEFINITION") == 1 - assert sdl.count("input Policy") == 1 - assert sdl.count("input Rule") == 1 - assert sdl.count("enum Mode") == 1 - assert sdl.count("scalar Secret") == 1 + assert sdl == textwrap.dedent(expected).strip() build_schema(sdl) @@ -213,13 +247,13 @@ class HiddenConfig: locations=[Location.FIELD_DEFINITION], print_definition=False ) class Hidden: - config: HiddenConfig + config: HiddenConfig | None = strawberry.UNSET @strawberry.type class Query: name: str = strawberry.field( default="Patrick", - directives=[Hidden(config=HiddenConfig(reason="private"))], + directives=[Hidden()], ) schema = strawberry.Schema(query=Query) @@ -227,11 +261,17 @@ class Query: assert schema._schema.get_directive("hidden") is not None assert schema._schema.get_type("HiddenConfig") is not None - sdl = schema.as_str() - assert "directive @hidden" not in sdl - assert "input HiddenConfig" in sdl - assert "@hidden(config:" in sdl - assert 'reason: "private"' in sdl + expected = """ + input HiddenConfig { + reason: String! + } + + type Query { + name: String! @hidden + } + """ + + assert schema.as_str() == textwrap.dedent(expected).strip() def test_registers_directives_from_all_type_system_attachment_points(): From 83d9540590ddb02d06f02df6e0c7e2b4f3b09333 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:08:50 +0000 Subject: [PATCH 06/18] Keep Federation support types out of SDL Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/federation/schema.py | 12 ++++++ strawberry/printer/printer.py | 4 ++ strawberry/schema/base.py | 6 ++- strawberry/schema/schema.py | 3 ++ .../printer/test_additional_directives.py | 4 -- .../printer/test_compose_directive.py | 4 -- tests/federation/printer/test_entities.py | 2 - tests/federation/printer/test_inaccessible.py | 2 - tests/federation/printer/test_interface.py | 2 - .../printer/test_interface_object.py | 2 - tests/federation/printer/test_keys.py | 2 - tests/federation/printer/test_link.py | 43 +++++++++---------- tests/federation/printer/test_override.py | 4 -- tests/federation/printer/test_provides.py | 4 -- tests/federation/printer/test_requires.py | 2 - tests/federation/printer/test_shareable.py | 2 - tests/federation/test_schema.py | 9 ++-- 17 files changed, 50 insertions(+), 57 deletions(-) diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index 869334a067..4745519a4b 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -32,6 +32,7 @@ if TYPE_CHECKING: from graphql import ExecutionContext as GraphQLExecutionContext + from graphql import GraphQLNamedType from strawberry.extensions import SchemaExtension from strawberry.federation.schema_directives import ComposeDirective @@ -44,6 +45,14 @@ FederationAny = NewType("FederationAny", object) """Represents the _Any scalar type used in federation entity resolution.""" +_PRIVATE_FEDERATION_TYPES = frozenset( + { + "_FieldSet", + "link__Import", + "link__Purpose", + } +) + class Schema(BaseSchema): def __init__( # noqa: PLR0917 @@ -370,6 +379,9 @@ def _warn_for_federation_directives(self) -> None: def _should_register_schema_directive(self, directive: object) -> bool: return True + def _should_include_type_in_sdl(self, graphql_type: "GraphQLNamedType") -> bool: + return graphql_type.name not in _PRIVATE_FEDERATION_TYPES + def _get_entity_type( query: type[WithStrawberryObjectDefinition] | None, diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index ce6dfee11a..6fba96b094 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -615,6 +615,7 @@ def print_schema(schema: BaseSchema) -> str: type_ for type_name in sorted(type_map) if is_defined_type(type_ := type_map[type_name]) + and schema._should_include_type_in_sdl(type_) ] types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] @@ -649,6 +650,9 @@ def _print_extra_types() -> Iterable[str]: "GraphQLNamedType", schema.schema_converter.from_type(type_) ) + if not schema._should_include_type_in_sdl(graphql_type): + continue + # Skip types that are already part of the schema's type map, otherwise # they'd be printed twice (e.g. an enum used both as a regular type and # as a schema directive field), producing invalid SDL. diff --git a/strawberry/schema/base.py b/strawberry/schema/base.py index 526aeb5c35..892202ecdc 100644 --- a/strawberry/schema/base.py +++ b/strawberry/schema/base.py @@ -9,7 +9,7 @@ if TYPE_CHECKING: from collections.abc import Iterable - from graphql import GraphQLError + from graphql import GraphQLError, GraphQLNamedType from strawberry.directive import StrawberryDirective from strawberry.schema.exception_handlers import ExceptionHandler @@ -111,6 +111,10 @@ def get_directive_by_name(self, graphql_name: str) -> StrawberryDirective | None def as_str(self) -> str: raise NotImplementedError + @abstractmethod + def _should_include_type_in_sdl(self, graphql_type: GraphQLNamedType) -> bool: + raise NotImplementedError + @staticmethod def remove_field_suggestion(error: GraphQLError) -> None: if ( diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 31d4861272..9ad90b0b0e 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -1427,6 +1427,9 @@ def as_str(self) -> str: __str__ = as_str + def _should_include_type_in_sdl(self, graphql_type: GraphQLNamedType) -> bool: + return True + def introspect(self) -> dict[str, Any]: """Return the introspection query result for the current schema. diff --git a/tests/federation/printer/test_additional_directives.py b/tests/federation/printer/test_additional_directives.py index e4dd04bfcc..e0d4b4ef16 100644 --- a/tests/federation/printer/test_additional_directives.py +++ b/tests/federation/printer/test_additional_directives.py @@ -43,8 +43,6 @@ class Query: union _Entity = FederatedType - scalar _FieldSet - type _Service { sdl: String! } @@ -101,8 +99,6 @@ class Query: union _Entity = FederatedType - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_compose_directive.py b/tests/federation/printer/test_compose_directive.py index 54704c7a2c..780cccb559 100644 --- a/tests/federation/printer/test_compose_directive.py +++ b/tests/federation/printer/test_compose_directive.py @@ -55,8 +55,6 @@ class Query: union _Entity = FederatedType - scalar _FieldSet - type _Service { sdl: String! } @@ -121,8 +119,6 @@ class Query: union _Entity = FederatedType - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_entities.py b/tests/federation/printer/test_entities.py index f7a6641efc..28df060bed 100644 --- a/tests/federation/printer/test_entities.py +++ b/tests/federation/printer/test_entities.py @@ -124,8 +124,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_inaccessible.py b/tests/federation/printer/test_inaccessible.py index 8acbe607d5..b0aa7c9b77 100644 --- a/tests/federation/printer/test_inaccessible.py +++ b/tests/federation/printer/test_inaccessible.py @@ -81,8 +81,6 @@ def top_products( union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_interface.py b/tests/federation/printer/test_interface.py index 0ec435a3ef..27ecbb36eb 100644 --- a/tests/federation/printer/test_interface.py +++ b/tests/federation/printer/test_interface.py @@ -44,8 +44,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_interface_object.py b/tests/federation/printer/test_interface_object.py index dccfe8d710..3011fedf79 100644 --- a/tests/federation/printer/test_interface_object.py +++ b/tests/federation/printer/test_interface_object.py @@ -28,8 +28,6 @@ class SomeInterface: union _Entity = SomeInterface - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_keys.py b/tests/federation/printer/test_keys.py index 59741d53ab..1de6f8589a 100644 --- a/tests/federation/printer/test_keys.py +++ b/tests/federation/printer/test_keys.py @@ -62,8 +62,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product | Review - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_link.py b/tests/federation/printer/test_link.py index 90fea0fd35..95680df720 100644 --- a/tests/federation/printer/test_link.py +++ b/tests/federation/printer/test_link.py @@ -34,16 +34,30 @@ class Query: type _Service { sdl: String! } + """ - scalar link__Import + assert schema.as_str() == textwrap.dedent(expected).strip() - enum link__Purpose { - SECURITY - EXECUTION + result = schema.execute_sync( + """ + { + importType: __type(name: "link__Import") { + kind + name + } + purposeType: __type(name: "link__Purpose") { + kind + name + } } - """ + """ + ) - assert schema.as_str() == textwrap.dedent(expected).strip() + assert result.errors is None + assert result.data == { + "importType": {"kind": "SCALAR", "name": "link__Import"}, + "purposeType": {"kind": "ENUM", "name": "link__Purpose"}, + } @skip_if_gql_32("formatting is different in gql 3.2") @@ -97,13 +111,6 @@ class Query: type _Service { sdl: String! } - - scalar link__Import - - enum link__Purpose { - SECURITY - EXECUTION - } """ assert schema.as_str() == textwrap.dedent(expected).strip() @@ -139,8 +146,6 @@ class Query: union _Entity = User - scalar _FieldSet - type _Service { sdl: String! } @@ -184,8 +189,6 @@ class Query: scalar _Any - scalar _FieldSet - type _Service { sdl: String! } @@ -269,8 +272,6 @@ class Query: union _Entity = User - scalar _FieldSet - type _Service { sdl: String! } @@ -309,8 +310,6 @@ class Query: union _Entity = User - scalar _FieldSet - type _Service { sdl: String! } @@ -358,8 +357,6 @@ class Query: union _Entity = User - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_override.py b/tests/federation/printer/test_override.py index 2d75095318..82364dfd75 100644 --- a/tests/federation/printer/test_override.py +++ b/tests/federation/printer/test_override.py @@ -47,8 +47,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } @@ -101,8 +99,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_provides.py b/tests/federation/printer/test_provides.py index 2428b86f70..0403b0947b 100644 --- a/tests/federation/printer/test_provides.py +++ b/tests/federation/printer/test_provides.py @@ -67,8 +67,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } @@ -140,8 +138,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_requires.py b/tests/federation/printer/test_requires.py index 98279508af..00980b31e8 100644 --- a/tests/federation/printer/test_requires.py +++ b/tests/federation/printer/test_requires.py @@ -70,8 +70,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/printer/test_shareable.py b/tests/federation/printer/test_shareable.py index 0029c1ed2d..2555ee75e9 100644 --- a/tests/federation/printer/test_shareable.py +++ b/tests/federation/printer/test_shareable.py @@ -46,8 +46,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } diff --git a/tests/federation/test_schema.py b/tests/federation/test_schema.py index 9113c9e1e7..33f56937df 100644 --- a/tests/federation/test_schema.py +++ b/tests/federation/test_schema.py @@ -101,8 +101,6 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover union _Entity = Product - scalar _FieldSet - type _Service { sdl: String! } @@ -118,6 +116,10 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover name } } + fieldSet: __type(name: "_FieldSet") { + kind + name + } } """ @@ -126,7 +128,8 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover assert not result.errors assert result.data == { - "__type": {"kind": "UNION", "possibleTypes": [{"name": "Product"}]} + "__type": {"kind": "UNION", "possibleTypes": [{"name": "Product"}]}, + "fieldSet": {"kind": "SCALAR", "name": "_FieldSet"}, } From b2598df2a5dce5d7caf45ae71c5166fbb58891e4 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:20:48 +0000 Subject: [PATCH 07/18] Mark private Federation SDL types Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/federation/schema.py | 45 ++++++++++++++++++++------------- strawberry/printer/printer.py | 7 +++-- strawberry/schema/base.py | 6 +---- strawberry/schema/schema.py | 3 --- tests/federation/test_schema.py | 38 +++++++++++++++++++++++++++- 5 files changed, 71 insertions(+), 28 deletions(-) diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index 4745519a4b..06e9bf8516 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -14,6 +14,7 @@ from strawberry.annotation import StrawberryAnnotation from strawberry.printer import print_schema +from strawberry.printer.printer import PRINT_DEFINITION from strawberry.schema import Schema as BaseSchema from strawberry.types.base import ( StrawberryContainer, @@ -27,12 +28,11 @@ from strawberry.utils.inspect import get_func_args from .schema_directive import StrawberryFederationSchemaDirective -from .types import FieldSet, LinkImport +from .types import FieldSet, LinkImport, LinkPurpose from .versions import format_version, parse_version if TYPE_CHECKING: from graphql import ExecutionContext as GraphQLExecutionContext - from graphql import GraphQLNamedType from strawberry.extensions import SchemaExtension from strawberry.federation.schema_directives import ComposeDirective @@ -45,14 +45,6 @@ FederationAny = NewType("FederationAny", object) """Represents the _Any scalar type used in federation entity resolution.""" -_PRIVATE_FEDERATION_TYPES = frozenset( - { - "_FieldSet", - "link__Import", - "link__Purpose", - } -) - class Schema(BaseSchema): def __init__( # noqa: PLR0917 @@ -96,16 +88,26 @@ def __init__( # noqa: PLR0917 types = [*types, FederationAny] # Add federation scalars to scalar_overrides so they can be recognized + field_set_scalar = scalar( + name="_FieldSet", serialize=lambda v: v, parse_value=str + ) + link_import_scalar = scalar( + name="link__Import", serialize=lambda v: v, parse_value=lambda v: v + ) + private_type_definitions = ( + field_set_scalar, + link_import_scalar, + LinkPurpose.__strawberry_definition__, # type: ignore[attr-defined] + ) + federation_scalar_overrides: dict[ object, type | ScalarDefinition | ScalarWrapper ] = { FederationAny: scalar( name="_Any", serialize=lambda v: v, parse_value=lambda v: v ), - FieldSet: scalar(name="_FieldSet", serialize=lambda v: v, parse_value=str), - LinkImport: scalar( - name="link__Import", serialize=lambda v: v, parse_value=lambda v: v - ), + FieldSet: field_set_scalar, + LinkImport: link_import_scalar, } if scalar_overrides: federation_scalar_overrides.update(scalar_overrides) @@ -124,6 +126,18 @@ def __init__( # noqa: PLR0917 exception_handlers=exception_handlers, ) + # These types are imported from Federation specs. Keep them in the runtime + # schema for directive introspection without defining them in subgraph SDL. + for graphql_type in self._schema.type_map.values(): + strawberry_definition = graphql_type.extensions.get( + self.schema_converter.DEFINITION_BACKREF + ) + if any( + strawberry_definition is definition + for definition in private_type_definitions + ): + graphql_type.extensions[PRINT_DEFINITION] = False + self.schema_directives = list(schema_directives) # Validate directive compatibility with federation version @@ -379,9 +393,6 @@ def _warn_for_federation_directives(self) -> None: def _should_register_schema_directive(self, directive: object) -> bool: return True - def _should_include_type_in_sdl(self, graphql_type: "GraphQLNamedType") -> bool: - return graphql_type.name not in _PRIVATE_FEDERATION_TYPES - def _get_entity_type( query: type[WithStrawberryObjectDefinition] | None, diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index 6fba96b094..464658390c 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -66,6 +66,9 @@ _T = TypeVar("_T") +# GraphQL type extension controlling SDL output without changing the runtime schema. +PRINT_DEFINITION = "strawberry-print-definition" + @dataclasses.dataclass class PrintExtras: @@ -615,7 +618,7 @@ def print_schema(schema: BaseSchema) -> str: type_ for type_name in sorted(type_map) if is_defined_type(type_ := type_map[type_name]) - and schema._should_include_type_in_sdl(type_) + and type_.extensions.get(PRINT_DEFINITION, True) ] types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] @@ -650,7 +653,7 @@ def _print_extra_types() -> Iterable[str]: "GraphQLNamedType", schema.schema_converter.from_type(type_) ) - if not schema._should_include_type_in_sdl(graphql_type): + if not graphql_type.extensions.get(PRINT_DEFINITION, True): continue # Skip types that are already part of the schema's type map, otherwise diff --git a/strawberry/schema/base.py b/strawberry/schema/base.py index 892202ecdc..526aeb5c35 100644 --- a/strawberry/schema/base.py +++ b/strawberry/schema/base.py @@ -9,7 +9,7 @@ if TYPE_CHECKING: from collections.abc import Iterable - from graphql import GraphQLError, GraphQLNamedType + from graphql import GraphQLError from strawberry.directive import StrawberryDirective from strawberry.schema.exception_handlers import ExceptionHandler @@ -111,10 +111,6 @@ def get_directive_by_name(self, graphql_name: str) -> StrawberryDirective | None def as_str(self) -> str: raise NotImplementedError - @abstractmethod - def _should_include_type_in_sdl(self, graphql_type: GraphQLNamedType) -> bool: - raise NotImplementedError - @staticmethod def remove_field_suggestion(error: GraphQLError) -> None: if ( diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 9ad90b0b0e..31d4861272 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -1427,9 +1427,6 @@ def as_str(self) -> str: __str__ = as_str - def _should_include_type_in_sdl(self, graphql_type: GraphQLNamedType) -> bool: - return True - def introspect(self) -> dict[str, Any]: """Return the introspection query result for the current schema. diff --git a/tests/federation/test_schema.py b/tests/federation/test_schema.py index 33f56937df..1aac0fe843 100644 --- a/tests/federation/test_schema.py +++ b/tests/federation/test_schema.py @@ -1,6 +1,6 @@ import textwrap import warnings -from typing import Generic, TypeVar +from typing import Generic, NewType, TypeVar import pytest @@ -161,6 +161,42 @@ def top_products(self, first: int) -> list[Example]: # pragma: no cover assert result.data == {"__type": {"kind": "SCALAR"}} +def test_user_type_named_like_private_federation_type_is_printed(): + UserFieldSet = NewType("UserFieldSet", str) + + @strawberry.type + class Query: + field_set: UserFieldSet + + schema = strawberry.federation.Schema( + query=Query, + scalar_overrides={ + UserFieldSet: strawberry.scalar( + name="_FieldSet", + serialize=lambda value: value, + parse_value=str, + ) + }, + ) + + expected_sdl = textwrap.dedent(""" + type Query { + _service: _Service! + fieldSet: _FieldSet! + } + + scalar _Any + + scalar _FieldSet + + type _Service { + sdl: String! + } + """).strip() + + assert schema.as_str() == expected_sdl + + def test_service(): @strawberry.federation.type class Product: From 12c011c12b17650dd22c9395a77948b02d6bff3b Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:36:43 +0000 Subject: [PATCH 08/18] Construct non-printing Federation types Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/federation/schema.py | 41 +++++++++++---------------------- strawberry/federation/types.py | 2 +- strawberry/printer/printer.py | 14 ++++++----- strawberry/types/enum.py | 11 +++++++++ strawberry/types/scalar.py | 10 ++++++++ 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index 06e9bf8516..e1b92fa06c 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -14,7 +14,6 @@ from strawberry.annotation import StrawberryAnnotation from strawberry.printer import print_schema -from strawberry.printer.printer import PRINT_DEFINITION from strawberry.schema import Schema as BaseSchema from strawberry.types.base import ( StrawberryContainer, @@ -28,7 +27,7 @@ from strawberry.utils.inspect import get_func_args from .schema_directive import StrawberryFederationSchemaDirective -from .types import FieldSet, LinkImport, LinkPurpose +from .types import FieldSet, LinkImport from .versions import format_version, parse_version if TYPE_CHECKING: @@ -88,26 +87,24 @@ def __init__( # noqa: PLR0917 types = [*types, FederationAny] # Add federation scalars to scalar_overrides so they can be recognized - field_set_scalar = scalar( - name="_FieldSet", serialize=lambda v: v, parse_value=str - ) - link_import_scalar = scalar( - name="link__Import", serialize=lambda v: v, parse_value=lambda v: v - ) - private_type_definitions = ( - field_set_scalar, - link_import_scalar, - LinkPurpose.__strawberry_definition__, # type: ignore[attr-defined] - ) - federation_scalar_overrides: dict[ object, type | ScalarDefinition | ScalarWrapper ] = { FederationAny: scalar( name="_Any", serialize=lambda v: v, parse_value=lambda v: v ), - FieldSet: field_set_scalar, - LinkImport: link_import_scalar, + FieldSet: scalar( + name="_FieldSet", + serialize=lambda v: v, + parse_value=str, + print_definition=False, + ), + LinkImport: scalar( + name="link__Import", + serialize=lambda v: v, + parse_value=lambda v: v, + print_definition=False, + ), } if scalar_overrides: federation_scalar_overrides.update(scalar_overrides) @@ -126,18 +123,6 @@ def __init__( # noqa: PLR0917 exception_handlers=exception_handlers, ) - # These types are imported from Federation specs. Keep them in the runtime - # schema for directive introspection without defining them in subgraph SDL. - for graphql_type in self._schema.type_map.values(): - strawberry_definition = graphql_type.extensions.get( - self.schema_converter.DEFINITION_BACKREF - ) - if any( - strawberry_definition is definition - for definition in private_type_definitions - ): - graphql_type.extensions[PRINT_DEFINITION] = False - self.schema_directives = list(schema_directives) # Validate directive compatibility with federation version diff --git a/strawberry/federation/types.py b/strawberry/federation/types.py index 2389dc7c98..85ed044595 100644 --- a/strawberry/federation/types.py +++ b/strawberry/federation/types.py @@ -10,7 +10,7 @@ """Represents an import for the @link directive.""" -@enum(name="link__Purpose") +@enum(name="link__Purpose", print_definition=False) class LinkPurpose(Enum): SECURITY = "SECURITY" EXECUTION = "EXECUTION" diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index 464658390c..12d5efef10 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -66,9 +66,6 @@ _T = TypeVar("_T") -# GraphQL type extension controlling SDL output without changing the runtime schema. -PRINT_DEFINITION = "strawberry-print-definition" - @dataclasses.dataclass class PrintExtras: @@ -600,6 +597,12 @@ def is_builtin_directive(directive: GraphQLDirective) -> bool: return False +def _should_print_type(type_: GraphQLNamedType) -> bool: + strawberry_definition = type_.extensions.get("strawberry-definition") + + return getattr(strawberry_definition, "print_definition", True) + + def print_schema(schema: BaseSchema) -> str: graphql_core_schema = cast( "GraphQLSchema", @@ -617,8 +620,7 @@ def print_schema(schema: BaseSchema) -> str: types = [ type_ for type_name in sorted(type_map) - if is_defined_type(type_ := type_map[type_name]) - and type_.extensions.get(PRINT_DEFINITION, True) + if is_defined_type(type_ := type_map[type_name]) and _should_print_type(type_) ] types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] @@ -653,7 +655,7 @@ def _print_extra_types() -> Iterable[str]: "GraphQLNamedType", schema.schema_converter.from_type(type_) ) - if not graphql_type.extensions.get(PRINT_DEFINITION, True): + if not _should_print_type(graphql_type): continue # Skip types that are already part of the schema's type map, otherwise diff --git a/strawberry/types/enum.py b/strawberry/types/enum.py index d3178b8d8f..f591e2e7eb 100644 --- a/strawberry/types/enum.py +++ b/strawberry/types/enum.py @@ -27,6 +27,7 @@ class StrawberryEnumDefinition(StrawberryType): values: list[EnumValue] description: str | None directives: Iterable[object] = () + print_definition: bool = True def __hash__(self) -> int: # TODO: Is this enough for unique-ness? @@ -111,6 +112,7 @@ class EnumAnnotation: description: str | None = None directives: Iterable[object] = () graphql_name_from: GraphqlEnumNameFrom = "key" + print_definition: bool = True def __call__(self, cls: EnumType) -> EnumType: return _process_enum( @@ -119,6 +121,7 @@ def __call__(self, cls: EnumType) -> EnumType: self.description, directives=self.directives, graphql_name_from=self.graphql_name_from, + print_definition=self.print_definition, ) @@ -128,6 +131,8 @@ def _process_enum( description: str | None = None, directives: Iterable[object] = (), graphql_name_from: GraphqlEnumNameFrom = "key", + *, + print_definition: bool = True, ) -> EnumType: if not isinstance(cls, EnumMeta): raise ObjectIsNotAnEnumError(cls) @@ -180,6 +185,7 @@ def _process_enum( values=values, description=description, directives=directives, + print_definition=print_definition, ) return cls @@ -193,6 +199,7 @@ def enum( description: str | None = None, directives: Iterable[object] = (), graphql_name_from: GraphqlEnumNameFrom = "key", + print_definition: bool = True, ) -> EnumType: ... @@ -204,6 +211,7 @@ def enum( description: str | None = None, directives: Iterable[object] = (), graphql_name_from: GraphqlEnumNameFrom = "key", + print_definition: bool = True, ) -> Callable[[EnumType], EnumType]: ... @@ -214,6 +222,7 @@ def enum( description: str | None = None, directives: Iterable[object] = (), graphql_name_from: GraphqlEnumNameFrom = "key", + print_definition: bool = True, ) -> EnumType | Callable[[EnumType], EnumType]: """Annotates an Enum class a GraphQL enum. @@ -227,6 +236,7 @@ def enum( description: The description of the GraphQL enum. directives: The directives to attach to the GraphQL enum. graphql_name_from: Whether to use the names (key) or values of the Python enums in GraphQL. + print_definition: Whether to include the enum definition in generated SDL. Returns: The decorated Enum class. @@ -260,6 +270,7 @@ class MyEnum(Enum): description=description, directives=directives, graphql_name_from=graphql_name_from, + print_definition=print_definition, ) if not cls: diff --git a/strawberry/types/scalar.py b/strawberry/types/scalar.py index 9365648fea..d66d884a7f 100644 --- a/strawberry/types/scalar.py +++ b/strawberry/types/scalar.py @@ -51,6 +51,7 @@ class ScalarDefinition(StrawberryType): # used for better error messages _source_file: str | None = None _source_line: int | None = None + print_definition: bool = True def copy_with( self, type_var_map: Mapping[str, StrawberryType | type] @@ -92,6 +93,7 @@ def _process_scalar( parse_value: GraphQLScalarValueParser | None = None, parse_literal: GraphQLScalarLiteralParser | None = None, directives: Iterable[object] = (), + print_definition: bool = True, ) -> ScalarWrapper: from strawberry.exceptions.handler import should_use_rich_exceptions @@ -115,6 +117,7 @@ def _process_scalar( parse_literal=parse_literal, parse_value=parse_value, directives=directives, + print_definition=print_definition, origin=cls, # type: ignore[arg-type] _source_file=_source_file, _source_line=_source_line, @@ -134,6 +137,7 @@ def scalar( parse_value: GraphQLScalarValueParser | None = None, parse_literal: GraphQLScalarLiteralParser | None = None, directives: Iterable[object] = (), + print_definition: bool = True, ) -> ScalarDefinition: ... @@ -148,6 +152,7 @@ def scalar( parse_value: GraphQLScalarValueParser | None = None, parse_literal: GraphQLScalarLiteralParser | None = None, directives: Iterable[object] = (), + print_definition: bool = True, ) -> Callable[[_T], _T]: ... @@ -162,6 +167,7 @@ def scalar( parse_value: GraphQLScalarValueParser | None = None, parse_literal: GraphQLScalarLiteralParser | None = None, directives: Iterable[object] = (), + print_definition: bool = True, ) -> _T: ... @@ -178,6 +184,7 @@ def scalar( parse_value: GraphQLScalarValueParser | None = None, parse_literal: GraphQLScalarLiteralParser | None = None, directives: Iterable[object] = (), + print_definition: bool = True, ) -> Any: """Annotates a class or type as a GraphQL custom scalar. @@ -204,6 +211,7 @@ def scalar( parse_value: The function to parse the value. parse_literal: The function to parse the literal. directives: The directives to apply to the scalar. + print_definition: Whether to include the scalar definition in generated SDL. Returns: A `ScalarDefinition` when called with `name` only, a decorator function @@ -265,6 +273,7 @@ def scalar( parse_literal=parse_literal, parse_value=parse_value, directives=directives, + print_definition=print_definition, origin=None, _source_file=_source_file, _source_line=_source_line, @@ -292,6 +301,7 @@ def wrap(cls: _T) -> ScalarWrapper: parse_value=parse_value, parse_literal=parse_literal, directives=directives, + print_definition=print_definition, ) if cls is None: From d4fc361a973b0c6c85553a62b7092eeaa842a485 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:44:12 +0000 Subject: [PATCH 09/18] Separate directive collection from schema creation Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/schema/schema.py | 87 ++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 35 deletions(-) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 31d4861272..7456d767fe 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -136,6 +136,45 @@ ) +class _GraphQLDirectiveRegistry: + def __init__(self) -> None: + self._directives = list(specified_directives) + self._entries: dict[str, tuple[object, str]] = { + directive.name: (directive, "the built-in GraphQL directive") + for directive in self._directives + } + + def __contains__(self, name: str) -> bool: + return name in self._entries + + @property + def directives(self) -> tuple[GraphQLDirective, ...]: + return tuple(self._directives) + + def add( + self, + directive: GraphQLDirective, + definition: object, + source: str, + ) -> None: + name = directive.name + existing = self._entries.get(name) + if existing is None: + self._entries[name] = (definition, source) + self._directives.append(directive) + return + + existing_definition, existing_source = existing + if existing_definition is definition: + return + + raise ValueError( + f"Schema directive '@{name}' is defined by both " + f"{existing_source} and {source}. Use a different " + "GraphQL name for one of them." + ) + + # TODO: merge with below def validate_document( schema: GraphQLSchema, @@ -443,43 +482,16 @@ class Query: formatted_errors = "\n\n".join(f"❌ {error.message}" for error in errors) raise ValueError(f"Invalid Schema. Errors:\n\n{formatted_errors}") - def _create_graphql_schema( + def _collect_graphql_directives( self, schema_directive_types: Iterable[type] - ) -> GraphQLSchema: - directives = list(specified_directives) - directive_definitions: dict[str, object] = { - directive.name: directive for directive in directives - } - directive_sources = { - directive.name: "the built-in GraphQL directive" for directive in directives - } - - def add_directive( - directive: GraphQLDirective, - definition: object, - source: str, - ) -> None: - name = directive.name - if name not in directive_definitions: - directive_definitions[name] = definition - directive_sources[name] = source - directives.append(directive) - return - - if directive_definitions[name] is definition: - return - - raise ValueError( - f"Schema directive '@{name}' is defined by both " - f"{directive_sources[name]} and {source}. Use a different " - "GraphQL name for one of them." - ) + ) -> tuple[GraphQLDirective, ...]: + registry = _GraphQLDirectiveRegistry() for directive in self._operation_graphql_directives: strawberry_directive = directive.extensions[ GraphQLCoreConverter.DEFINITION_BACKREF ] - add_directive( + registry.add( directive, strawberry_directive, f"operation directive '{strawberry_directive.python_name}'", @@ -499,11 +511,11 @@ def add_directive( if ( directive_type is OneOf and directive_name == "oneOf" - and directive_name in directive_definitions + and directive_name in registry ): continue - add_directive( + registry.add( self.schema_converter.from_schema_directive(directive_type), directive_type, ( @@ -514,17 +526,22 @@ def add_directive( if self.config.enable_experimental_incremental_execution: for directive in incremental_execution_directives: - add_directive( + registry.add( directive, directive, f"the experimental GraphQL directive '@{directive.name}'", ) + return registry.directives + + def _create_graphql_schema( + self, schema_directive_types: Iterable[type] + ) -> GraphQLSchema: return GraphQLSchema( query=self._graphql_query_type, mutation=self._graphql_mutation_type, subscription=self._graphql_subscription_type, - directives=tuple(directives), + directives=self._collect_graphql_directives(schema_directive_types), types=self._graphql_types, extensions={ GraphQLCoreConverter.DEFINITION_BACKREF: self, From 79a2524facf2f582fcfd5ee9d9fbeaed2e8c8806 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:47:18 +0000 Subject: [PATCH 10/18] Use ordered map for directive registry Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/schema/schema.py | 81 +++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 7456d767fe..38016f79b1 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -134,45 +134,30 @@ ProcessErrors: TypeAlias = ( "Callable[[list[GraphQLError], ExecutionContext | None], None]" ) +_DirectiveEntry: TypeAlias = tuple[GraphQLDirective, object, str] + + +def _add_graphql_directive( + directives: dict[str, _DirectiveEntry], + directive: GraphQLDirective, + definition: object, + source: str, +) -> None: + name = directive.name + existing = directives.get(name) + if existing is None: + directives[name] = (directive, definition, source) + return + _, existing_definition, existing_source = existing + if existing_definition is definition: + return -class _GraphQLDirectiveRegistry: - def __init__(self) -> None: - self._directives = list(specified_directives) - self._entries: dict[str, tuple[object, str]] = { - directive.name: (directive, "the built-in GraphQL directive") - for directive in self._directives - } - - def __contains__(self, name: str) -> bool: - return name in self._entries - - @property - def directives(self) -> tuple[GraphQLDirective, ...]: - return tuple(self._directives) - - def add( - self, - directive: GraphQLDirective, - definition: object, - source: str, - ) -> None: - name = directive.name - existing = self._entries.get(name) - if existing is None: - self._entries[name] = (definition, source) - self._directives.append(directive) - return - - existing_definition, existing_source = existing - if existing_definition is definition: - return - - raise ValueError( - f"Schema directive '@{name}' is defined by both " - f"{existing_source} and {source}. Use a different " - "GraphQL name for one of them." - ) + raise ValueError( + f"Schema directive '@{name}' is defined by both " + f"{existing_source} and {source}. Use a different " + "GraphQL name for one of them." + ) # TODO: merge with below @@ -485,13 +470,21 @@ class Query: def _collect_graphql_directives( self, schema_directive_types: Iterable[type] ) -> tuple[GraphQLDirective, ...]: - registry = _GraphQLDirectiveRegistry() + directives: dict[str, _DirectiveEntry] = { + directive.name: ( + directive, + directive, + "the built-in GraphQL directive", + ) + for directive in specified_directives + } for directive in self._operation_graphql_directives: strawberry_directive = directive.extensions[ GraphQLCoreConverter.DEFINITION_BACKREF ] - registry.add( + _add_graphql_directive( + directives, directive, strawberry_directive, f"operation directive '{strawberry_directive.python_name}'", @@ -511,11 +504,12 @@ def _collect_graphql_directives( if ( directive_type is OneOf and directive_name == "oneOf" - and directive_name in registry + and directive_name in directives ): continue - registry.add( + _add_graphql_directive( + directives, self.schema_converter.from_schema_directive(directive_type), directive_type, ( @@ -526,13 +520,14 @@ def _collect_graphql_directives( if self.config.enable_experimental_incremental_execution: for directive in incremental_execution_directives: - registry.add( + _add_graphql_directive( + directives, directive, directive, f"the experimental GraphQL directive '@{directive.name}'", ) - return registry.directives + return tuple(directive for directive, _, _ in directives.values()) def _create_graphql_schema( self, schema_directive_types: Iterable[type] From d814faed76b233d035f7a8676d1775a45ca28312 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 16:51:26 +0000 Subject: [PATCH 11/18] Simplify GraphQL schema construction Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/schema/schema.py | 97 ++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 38016f79b1..dcd1d23852 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -137,29 +137,6 @@ _DirectiveEntry: TypeAlias = tuple[GraphQLDirective, object, str] -def _add_graphql_directive( - directives: dict[str, _DirectiveEntry], - directive: GraphQLDirective, - definition: object, - source: str, -) -> None: - name = directive.name - existing = directives.get(name) - if existing is None: - directives[name] = (directive, definition, source) - return - - _, existing_definition, existing_source = existing - if existing_definition is definition: - return - - raise ValueError( - f"Schema directive '@{name}' is defined by both " - f"{existing_source} and {source}. Use a different " - "GraphQL name for one of them." - ) - - # TODO: merge with below def validate_document( schema: GraphQLSchema, @@ -436,8 +413,17 @@ class Query: self._explicit_schema_directive_types = tuple(explicit_schema_directive_types) try: - self._schema = self._create_graphql_schema( - self._explicit_schema_directive_types + self._schema = GraphQLSchema( + query=self._graphql_query_type, + mutation=self._graphql_mutation_type, + subscription=self._graphql_subscription_type, + directives=self._collect_graphql_directives( + self._explicit_schema_directive_types + ), + types=self._graphql_types, + extensions={ + GraphQLCoreConverter.DEFINITION_BACKREF: self, + }, ) self._register_schema_directives() @@ -467,10 +453,31 @@ class Query: formatted_errors = "\n\n".join(f"❌ {error.message}" for error in errors) raise ValueError(f"Invalid Schema. Errors:\n\n{formatted_errors}") + def _add_graphql_directive( + self, + directive: GraphQLDirective, + definition: object, + source: str, + ) -> None: + name = directive.name + if (existing := self._graphql_directives.get(name)) is None: + self._graphql_directives[name] = (directive, definition, source) + return + + _, existing_definition, existing_source = existing + if existing_definition is definition: + return + + raise ValueError( + f"Schema directive '@{name}' is defined by both " + f"{existing_source} and {source}. Use a different " + "GraphQL name for one of them." + ) + def _collect_graphql_directives( self, schema_directive_types: Iterable[type] ) -> tuple[GraphQLDirective, ...]: - directives: dict[str, _DirectiveEntry] = { + self._graphql_directives: dict[str, _DirectiveEntry] = { directive.name: ( directive, directive, @@ -483,8 +490,7 @@ def _collect_graphql_directives( strawberry_directive = directive.extensions[ GraphQLCoreConverter.DEFINITION_BACKREF ] - _add_graphql_directive( - directives, + self._add_graphql_directive( directive, strawberry_directive, f"operation directive '{strawberry_directive.python_name}'", @@ -504,12 +510,11 @@ def _collect_graphql_directives( if ( directive_type is OneOf and directive_name == "oneOf" - and directive_name in directives + and directive_name in self._graphql_directives ): continue - _add_graphql_directive( - directives, + self._add_graphql_directive( self.schema_converter.from_schema_directive(directive_type), directive_type, ( @@ -520,28 +525,13 @@ def _collect_graphql_directives( if self.config.enable_experimental_incremental_execution: for directive in incremental_execution_directives: - _add_graphql_directive( - directives, + self._add_graphql_directive( directive, directive, f"the experimental GraphQL directive '@{directive.name}'", ) - return tuple(directive for directive, _, _ in directives.values()) - - def _create_graphql_schema( - self, schema_directive_types: Iterable[type] - ) -> GraphQLSchema: - return GraphQLSchema( - query=self._graphql_query_type, - mutation=self._graphql_mutation_type, - subscription=self._graphql_subscription_type, - directives=self._collect_graphql_directives(schema_directive_types), - types=self._graphql_types, - extensions={ - GraphQLCoreConverter.DEFINITION_BACKREF: self, - }, - ) + return tuple(directive for directive, _, _ in self._graphql_directives.values()) def _register_schema_directives(self) -> None: schema_directive_types = list(self._explicit_schema_directive_types) @@ -631,7 +621,16 @@ def add_directives_from_argument_type(type_: GraphQLInputType) -> None: if self._schema_directive_types == self._explicit_schema_directive_types: return - self._schema = self._create_graphql_schema(self._schema_directive_types) + self._schema = GraphQLSchema( + query=self._graphql_query_type, + mutation=self._graphql_mutation_type, + subscription=self._graphql_subscription_type, + directives=self._collect_graphql_directives(self._schema_directive_types), + types=self._graphql_types, + extensions={ + GraphQLCoreConverter.DEFINITION_BACKREF: self, + }, + ) def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning From 1b8af009ded157d8b731d319fb89fc4fabfaf534 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 17:06:30 +0000 Subject: [PATCH 12/18] Register generated federation directives Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- RELEASE.md | 4 ++- strawberry/federation/schema.py | 4 +++ strawberry/schema/schema.py | 10 ++++++-- .../printer/test_compose_directive.py | 25 +++++++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 9d913b0f71..8fc8459fe2 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -16,7 +16,9 @@ This release fixes introspection for custom schema directives. Schema directives attached to types, fields, arguments, and other schema elements now appear in standard GraphQL introspection. Schema explorers, IDEs, code generators, and other tools can discover each directive's description, arguments, -allowed locations, repeatability, and any input types it uses. +allowed locations, repeatability, and any input types it uses. Federation directives, +including generated `@link` and `@composeDirective` applications, are discoverable +in the same way. A directive reused across the schema is defined only once. Input, enum, and scalar types referenced by directive arguments are now part of the schema and may appear diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index e1b92fa06c..b55b3558cb 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -131,6 +131,10 @@ def __init__( # noqa: PLR0917 composed_directives = self._add_compose_directives() self._add_link_directives(composed_directives) # type: ignore + # Compose and link directives are generated after the base schema has + # collected attached directives, so include them in the runtime schema too. + self._register_schema_directives() + def _get_federation_query_type( self, query: type[WithStrawberryObjectDefinition] | None, diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index dcd1d23852..536ce6afec 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -534,7 +534,12 @@ def _collect_graphql_directives( return tuple(directive for directive, _, _ in self._graphql_directives.values()) def _register_schema_directives(self) -> None: - schema_directive_types = list(self._explicit_schema_directive_types) + registered_schema_directive_types = getattr( + self, + "_schema_directive_types", + self._explicit_schema_directive_types, + ) + schema_directive_types = list(registered_schema_directive_types) seen_directive_types = set(schema_directive_types) seen_directive_argument_types: set[str] = set() @@ -618,7 +623,7 @@ def add_directives_from_argument_type(type_: GraphQLInputType) -> None: self._schema_directive_types = tuple(schema_directive_types) - if self._schema_directive_types == self._explicit_schema_directive_types: + if self._schema_directive_types == registered_schema_directive_types: return self._schema = GraphQLSchema( @@ -631,6 +636,7 @@ def add_directives_from_argument_type(type_: GraphQLInputType) -> None: GraphQLCoreConverter.DEFINITION_BACKREF: self, }, ) + self._schema._strawberry_schema = self # type: ignore def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning diff --git a/tests/federation/printer/test_compose_directive.py b/tests/federation/printer/test_compose_directive.py index 780cccb559..99d31ba904 100644 --- a/tests/federation/printer/test_compose_directive.py +++ b/tests/federation/printer/test_compose_directive.py @@ -66,6 +66,31 @@ class Query: assert schema.as_str() == textwrap.dedent(expected_type).strip() + result = schema.execute_sync( + """ + { + __schema { + directives { + name + } + } + } + """ + ) + + assert result.errors is None + directive_names = { + directive["name"] for directive in result.data["__schema"]["directives"] + } + assert { + "cacheControl", + "sensitive", + "key", + "shareable", + "composeDirective", + "link", + } <= directive_names + def test_schema_directives_and_compose_schema_custom_import_url(): @strawberry.federation.schema_directive( From 9d5e153aa99a1dcc0906574b3e744c48d556e07e Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sat, 29 Aug 2026 21:37:04 +0000 Subject: [PATCH 13/18] Address schema directive review findings Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- RELEASE.md | 7 + docs/types/enums.md | 18 ++ docs/types/scalars.md | 20 ++ docs/types/schema-directives.md | 38 ++++ .../exceptions/unresolved_field_type.py | 10 +- strawberry/federation/schema.py | 45 +---- strawberry/federation/schema_directives.py | 5 + strawberry/permission.py | 17 +- strawberry/printer/printer.py | 75 +++++++- strawberry/schema/schema.py | 173 ++++++++---------- strawberry/schema/schema_converter.py | 6 +- .../printer/test_compose_directive.py | 22 ++- tests/federation/test_schema.py | 40 ++++ tests/schema/test_permission.py | 27 +++ tests/schema/test_schema_directives.py | 153 ++++++++++++++++ 15 files changed, 510 insertions(+), 146 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 8fc8459fe2..8fda84c431 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -23,3 +23,10 @@ in the same way. A directive reused across the schema is defined only once. Input, enum, and scalar types referenced by directive arguments are now part of the schema and may appear in generated SDL even when they are not used by fields. + +Because these directives and argument types are now part of the runtime schema, +their GraphQL names must be unique. Schema construction reports a clear error when +different directive definitions share a name, a custom directive replaces a +built-in directive such as `@skip`, or a directive argument type conflicts with +another schema type. Compatible custom `@oneOf` definitions continue to use +GraphQL's built-in directive. diff --git a/docs/types/enums.md b/docs/types/enums.md index 6a85a58407..f553597ceb 100644 --- a/docs/types/enums.md +++ b/docs/types/enums.md @@ -187,3 +187,21 @@ When querying, the custom name will be used in the response: Note that the Python enum member name (`CHOCOLATE_COOKIE`) is still used in your Python code, while the custom name (`chocolateCookie`) is used in the GraphQL schema and responses. + +## Hiding integration support types from SDL + +Integration authors can pass `print_definition=False` to `strawberry.enum` for +an enum that should be available to runtime introspection without normally +appearing in Strawberry's generated SDL: + +```python +@strawberry.enum(print_definition=False) +class InternalPurpose(Enum): + SECURITY = "security" + EXECUTION = "execution" +``` + +If a regular field, argument, or printed directive definition uses the enum, +Strawberry prints its definition to keep the generated SDL valid. This option is +primarily intended for types used only by hidden schema directives and framework +integrations. diff --git a/docs/types/scalars.md b/docs/types/scalars.md index 6cc80cb7db..d32bb25090 100644 --- a/docs/types/scalars.md +++ b/docs/types/scalars.md @@ -170,6 +170,26 @@ from strawberry.scalars import Base16, Base32, Base64 +### Hiding integration support types from SDL + +Integration authors can pass `print_definition=False` to `strawberry.scalar` for +a support type that should be available to runtime introspection without +normally appearing in Strawberry's generated SDL: + +```python +InternalIDScalar = strawberry.scalar( + name="InternalID", + serialize=str, + parse_value=str, + print_definition=False, +) +``` + +If a regular field, argument, or printed directive definition uses the scalar, +Strawberry prints its definition to keep the generated SDL valid. This option is +primarily intended for types used only by hidden schema directives and framework +integrations. + ## Example: Custom Object Scalar Suppose we would like to use a Pillow `Image` as a scalar that serializes diff --git a/docs/types/schema-directives.md b/docs/types/schema-directives.md index d8ec5c971f..1f87847ae1 100644 --- a/docs/types/schema-directives.md +++ b/docs/types/schema-directives.md @@ -48,6 +48,44 @@ type User @keys(fields: "id") { } ``` +## Introspection + +Directives attached to a Strawberry schema are included in standard GraphQL +introspection. Tools can discover their descriptions, arguments, default values, +locations, and repeatability with a query such as: + +```graphql +{ + __schema { + directives { + name + description + locations + isRepeatable + args { + name + defaultValue + } + } + } +} +``` + +Input objects, enums, and scalars used only by directive arguments are also part +of the runtime schema. Their GraphQL names must therefore be distinct from other +types in the schema. Directive names must likewise be unique and cannot replace +built-in directives such as `@skip` or `@deprecated`. Compatible legacy `@oneOf` +definitions use GraphQL's built-in directive automatically. + +Directive argument annotations are resolved when the schema is built, in the +same way as field and argument annotations elsewhere in the schema. Types +imported only under `TYPE_CHECKING` should use +[`strawberry.lazy`](/docs/types/lazy) so Strawberry can resolve them at runtime. + +Setting `print_definition=False` on `@strawberry.schema_directive` keeps its +definition out of Strawberry's generated SDL, but the directive and its argument +types stay available to runtime introspection. + ## Overriding field names You can use `strawberry.directive_field` to override the name of a field: diff --git a/strawberry/exceptions/unresolved_field_type.py b/strawberry/exceptions/unresolved_field_type.py index 8468031492..6f62013572 100644 --- a/strawberry/exceptions/unresolved_field_type.py +++ b/strawberry/exceptions/unresolved_field_type.py @@ -8,6 +8,7 @@ from .exception import StrawberryException if TYPE_CHECKING: + from strawberry.schema_directive import StrawberrySchemaDirective from strawberry.types.field import StrawberryField from strawberry.types.object_type import StrawberryObjectDefinition @@ -17,7 +18,7 @@ class UnresolvedFieldTypeError(StrawberryException): def __init__( self, - type_definition: StrawberryObjectDefinition, + type_definition: StrawberryObjectDefinition | StrawberrySchemaDirective, field: StrawberryField, ) -> None: self.type_definition = type_definition @@ -44,10 +45,11 @@ def exception_source(self) -> ExceptionSource | None: source_finder = SourceFinder() # field could be attached to the class or not + origin = self.type_definition.origin + if origin is None: + return None - source = source_finder.find_class_attribute_from_object( - self.type_definition.origin, self.field.name - ) + source = source_finder.find_class_attribute_from_object(origin, self.field.name) if source is not None: return source diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index b55b3558cb..52dcacac95 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -1,7 +1,5 @@ from collections import defaultdict from collections.abc import Callable, Iterable, Mapping -from functools import cached_property -from itertools import chain from typing import ( TYPE_CHECKING, Any, @@ -123,18 +121,6 @@ def __init__( # noqa: PLR0917 exception_handlers=exception_handlers, ) - self.schema_directives = list(schema_directives) - - # Validate directive compatibility with federation version - self._validate_directive_compatibility() - - composed_directives = self._add_compose_directives() - self._add_link_directives(composed_directives) # type: ignore - - # Compose and link directives are generated after the base schema has - # collected attached directives, so include them in the runtime schema too. - self._register_schema_directives() - def _get_federation_query_type( self, query: type[WithStrawberryObjectDefinition] | None, @@ -246,27 +232,9 @@ def entities_resolver( return results - @cached_property + @property def schema_directives_in_use(self) -> list[object]: - all_graphql_types = self._schema.type_map.values() - - directives: list[object] = [] - - for type_ in all_graphql_types: - strawberry_definition = type_.extensions.get("strawberry-definition") - - if not strawberry_definition: - continue - - directives.extend(strawberry_definition.directives) - - fields = getattr(strawberry_definition, "fields", []) - values = getattr(strawberry_definition, "values", []) - - for field in chain(fields, values): - directives.extend(field.directives) - - return directives + return self._schema_directives_in_use def _add_link_for_composed_directive( self, @@ -306,13 +274,13 @@ def _add_link_for_federation_directive( directive_by_url[url].add(f"@{name}") def _add_link_directives( - self, additional_directives: list[object] | None = None + self, additional_directives: Iterable[object] | None = None ) -> None: from .schema_directives import Link directive_by_url: defaultdict[str, set[str]] = defaultdict(set) - additional_directives = additional_directives or [] + additional_directives = list(additional_directives or ()) for directive in self.schema_directives_in_use + additional_directives: definition = directive.__strawberry_directive__ # type: ignore @@ -379,6 +347,11 @@ def _warn_for_federation_directives(self) -> None: pass + def _prepare_schema_directives(self) -> None: + self._validate_directive_compatibility() + composed_directives = self._add_compose_directives() + self._add_link_directives(composed_directives) + def _should_register_schema_directive(self, directive: object) -> bool: return True diff --git a/strawberry/federation/schema_directives.py b/strawberry/federation/schema_directives.py index 2585aff5c3..1d2147370f 100644 --- a/strawberry/federation/schema_directives.py +++ b/strawberry/federation/schema_directives.py @@ -28,6 +28,8 @@ def with_version(self, version: str) -> "ImportedFrom": class FederationDirective: imported_from: ClassVar[ImportedFrom] minimum_version: ClassVar[FederationVersion] + # The base Schema does not install Federation's supporting scalar mappings. + __strawberry_register_definition__: ClassVar[bool] = False @schema_directive( @@ -94,6 +96,9 @@ class Shareable(FederationDirective): locations=[Location.SCHEMA], name="link", repeatable=True, print_definition=False ) class Link: + # The base Schema does not install Federation's supporting scalar mappings. + __strawberry_register_definition__: ClassVar[bool] = False + url: str | None as_: str | None = directive_field(name="as") for_: LinkPurpose | None = directive_field(name="for") diff --git a/strawberry/permission.py b/strawberry/permission.py index 7d10a023f5..85659079bd 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -7,6 +7,7 @@ from typing import ( TYPE_CHECKING, Any, + ClassVar, ) from strawberry.exceptions import StrawberryGraphQLError @@ -54,7 +55,7 @@ def has_permission(self, source, info, **kwargs): error_class: type[GraphQLError] = StrawberryGraphQLError - _schema_directive: object | None = None + _schema_directive: ClassVar[object | None] = None @abc.abstractmethod def has_permission( @@ -106,19 +107,23 @@ def on_unauthorized(self) -> None: @property def schema_directive(self) -> object: - if not self._schema_directive: + permission_class = self.__class__ + if ( + schema_directive := permission_class.__dict__.get("_schema_directive") + ) is None: class AutoDirective: __strawberry_directive__ = StrawberrySchemaDirective( - self.__class__.__name__, - self.__class__.__name__, + permission_class.__name__, + permission_class.__name__, [Location.FIELD_DEFINITION], [], ) - self._schema_directive = AutoDirective() + schema_directive = AutoDirective() + permission_class._schema_directive = schema_directive - return self._schema_directive + return schema_directive class PermissionExtension(FieldExtension): diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index 12d5efef10..0a0c9b4bff 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -14,6 +14,7 @@ GraphQLInputField, GraphQLObjectType, GraphQLSchema, + get_named_type, is_union_type, ) from graphql.language.printer import print_ast @@ -159,7 +160,11 @@ def print_schema_directive( "StrawberrySchemaDirective", directive.__class__.__strawberry_directive__ ) schema_converter = schema.schema_converter - gql_directive = schema_converter.from_schema_directive(directive.__class__) + gql_directive = getattr(schema, "_schema_graphql_directives", {}).get( + directive.__class__ + ) + if gql_directive is None: + gql_directive = schema_converter.from_schema_directive(directive.__class__) params = print_schema_directive_params( gql_directive, { @@ -597,10 +602,68 @@ def is_builtin_directive(directive: GraphQLDirective) -> bool: return False -def _should_print_type(type_: GraphQLNamedType) -> bool: +def _should_print_type(type_: GraphQLNamedType, schema_type_names: set[str]) -> bool: strawberry_definition = type_.extensions.get("strawberry-definition") - return getattr(strawberry_definition, "print_definition", True) + return ( + getattr(strawberry_definition, "print_definition", True) + or type_.name in schema_type_names + ) + + +def _get_schema_type_names(schema: BaseSchema) -> set[str]: + graphql_schema = cast("GraphQLSchema", schema._schema) # type: ignore[attr-defined] + stack = [ + graphql_schema.query_type, + graphql_schema.mutation_type, + graphql_schema.subscription_type, + *schema._graphql_types, # type: ignore[attr-defined] + ] + + for type_ in graphql_schema.type_map.values(): + strawberry_definition = type_.extensions.get("strawberry-definition") + if is_defined_type(type_) and getattr( + strawberry_definition, "print_definition", True + ): + stack.append(type_) + + for directive in graphql_schema.directives: + if is_builtin_directive(directive): + continue + + strawberry_definition = directive.extensions.get("strawberry-definition") + if ( + isinstance(strawberry_definition, StrawberrySchemaDirective) + and not strawberry_definition.print_definition + ): + continue + + stack.extend(argument.type for argument in directive.args.values()) + + type_names: set[str] = set() + + while stack: + type_ = stack.pop() + if type_ is None: + continue + + named_type = get_named_type(type_) + if named_type.name in type_names: + continue + + type_names.add(named_type.name) + stack.extend(getattr(named_type, "interfaces", ())) + stack.extend(getattr(named_type, "types", ())) + if is_interface_type(named_type): + stack.extend(graphql_schema.get_possible_types(named_type)) + + for field in getattr(named_type, "fields", {}).values(): + stack.append(field.type) + stack.extend( + argument.type for argument in getattr(field, "args", {}).values() + ) + + return type_names def print_schema(schema: BaseSchema) -> str: @@ -617,10 +680,12 @@ def print_schema(schema: BaseSchema) -> str: ] type_map = graphql_core_schema.type_map + schema_type_names = _get_schema_type_names(schema) types = [ type_ for type_name in sorted(type_map) - if is_defined_type(type_ := type_map[type_name]) and _should_print_type(type_) + if is_defined_type(type_ := type_map[type_name]) + and _should_print_type(type_, schema_type_names) ] types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] @@ -655,7 +720,7 @@ def _print_extra_types() -> Iterable[str]: "GraphQLNamedType", schema.schema_converter.from_type(type_) ) - if not _should_print_type(graphql_type): + if not _should_print_type(graphql_type, schema_type_names): continue # Skip types that are already part of the schema's type map, otherwise diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 536ce6afec..ca9aa39a3b 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -38,7 +38,6 @@ GraphQLSchema, OperationDefinitionNode, get_introspection_query, - get_named_type, parse, validate_schema, ) @@ -97,7 +96,7 @@ from graphql.language import DocumentNode from graphql.pyutils import Path - from graphql.type import GraphQLInputType, GraphQLResolveInfo + from graphql.type import GraphQLResolveInfo from graphql.validation import ASTValidationRule from strawberry.directive import StrawberryDirective @@ -413,19 +412,21 @@ class Query: self._explicit_schema_directive_types = tuple(explicit_schema_directive_types) try: + self._schema_directive_types = self._collect_schema_directives( + self._explicit_schema_directive_types + ) self._schema = GraphQLSchema( query=self._graphql_query_type, mutation=self._graphql_mutation_type, subscription=self._graphql_subscription_type, directives=self._collect_graphql_directives( - self._explicit_schema_directive_types + self._schema_directive_types ), types=self._graphql_types, extensions={ GraphQLCoreConverter.DEFINITION_BACKREF: self, }, ) - self._register_schema_directives() except TypeError as error: # GraphQL core throws a TypeError if there's any exception raised @@ -496,26 +497,29 @@ def _collect_graphql_directives( f"operation directive '{strawberry_directive.python_name}'", ) - from strawberry.schema_directives import OneOf - for directive_type in schema_directive_types: + graphql_directive = self._schema_graphql_directives[directive_type] strawberry_directive = cast("Any", directive_type).__strawberry_directive__ directive_name = self.config.name_converter.from_directive( strawberry_directive ) # Strawberry's OneOf schema directive predates graphql-core exposing - # the specified @oneOf directive. Keep its SDL behavior while using - # the canonical runtime directive. + # the specified @oneOf directive. Treat compatible definitions as the + # canonical runtime directive regardless of the Python class used. if ( - directive_type is OneOf - and directive_name == "oneOf" - and directive_name in self._graphql_directives + directive_name == "oneOf" + and (existing := self._graphql_directives.get(directive_name)) + is not None + and existing[0] is existing[1] + and graphql_directive.locations == existing[0].locations + and graphql_directive.args.keys() == existing[0].args.keys() + and graphql_directive.is_repeatable == existing[0].is_repeatable ): continue self._add_graphql_directive( - self.schema_converter.from_schema_directive(directive_type), + graphql_directive, directive_type, ( "schema directive " @@ -533,17 +537,19 @@ def _collect_graphql_directives( return tuple(directive for directive, _, _ in self._graphql_directives.values()) - def _register_schema_directives(self) -> None: - registered_schema_directive_types = getattr( - self, - "_schema_directive_types", - self._explicit_schema_directive_types, - ) - schema_directive_types = list(registered_schema_directive_types) + def _collect_schema_directives( + self, explicit_schema_directive_types: Iterable[type] + ) -> tuple[type, ...]: + schema_directive_types = list(dict.fromkeys(explicit_schema_directive_types)) seen_directive_types = set(schema_directive_types) - seen_directive_argument_types: set[str] = set() + seen_type_definitions: set[int] = set() + self._schema_graphql_directives: dict[type, GraphQLDirective] = {} + self._schema_directives_in_use: list[object] = [] + + def add_directive(directive: object, *, track_application: bool) -> None: + if track_application: + self._schema_directives_in_use.append(directive) - def add_directive(directive: object) -> None: directive_type = directive.__class__ if ( self._should_register_schema_directive(directive) @@ -553,90 +559,68 @@ def add_directive(directive: object) -> None: seen_directive_types.add(directive_type) schema_directive_types.append(directive_type) - def add_directives(owner: object) -> None: + def add_directives(owner: object, *, track_application: bool) -> None: attached_directives = getattr(owner, "directives", ()) or () if isinstance(attached_directives, Iterator): attached_directives = list(attached_directives) owner.directives = attached_directives # type: ignore[attr-defined] for directive in attached_directives: - add_directive(directive) + add_directive(directive, track_application=track_application) - def add_directives_from_graphql_type( - graphql_type: GraphQLNamedType, *, recurse_fields: bool = False - ) -> None: - type_definition = (graphql_type.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if type_definition is not None: - add_directives(type_definition) + for directive in self.schema_directives: + add_directive(directive, track_application=False) - for field in getattr(graphql_type, "fields", {}).values(): - field_definition = (field.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if field_definition is not None: - add_directives(field_definition) + directive_index = 0 - for argument in getattr(field, "args", {}).values(): - argument_definition = (argument.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF + def collect_new_definitions() -> None: + nonlocal directive_index + + while True: + made_progress = False + + for concrete_type in tuple(self.schema_converter.type_map.values()): + definition = concrete_type.definition + if id(definition) in seen_type_definitions: + continue + + seen_type_definitions.add(id(definition)) + made_progress = True + + # Resolve lazy fields and unions so their reachable types are + # added to the converter map before GraphQLSchema is built. + tuple(getattr(concrete_type.implementation, "fields", {}).values()) + tuple(getattr(concrete_type.implementation, "types", ())) + + add_directives(definition, track_application=True) + for field in getattr(definition, "fields", ()): + add_directives(field, track_application=True) + for argument in getattr(field, "arguments", ()): + add_directives(argument, track_application=True) + for value in getattr(definition, "values", ()): + add_directives(value, track_application=True) + + while directive_index < len(schema_directive_types): + directive_type = schema_directive_types[directive_index] + self._schema_graphql_directives[directive_type] = ( + self.schema_converter.from_schema_directive(directive_type) ) - if argument_definition is not None: - add_directives(argument_definition) - - if recurse_fields: - add_directives_from_argument_type(field.type) - - for value in getattr(graphql_type, "values", {}).values(): - value_definition = (value.extensions or {}).get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if value_definition is not None: - add_directives(value_definition) + directive_index += 1 + made_progress = True - def add_directives_from_argument_type(type_: GraphQLInputType) -> None: - graphql_type = get_named_type(type_) - if graphql_type.name in seen_directive_argument_types: - return + if not made_progress: + return - seen_directive_argument_types.add(graphql_type.name) - add_directives_from_graphql_type(graphql_type, recurse_fields=True) + collect_new_definitions() + self._prepare_schema_directives() + # Subclasses can add schema applications such as Federation's generated + # @link and @composeDirective directives during preparation. for directive in self.schema_directives: - add_directive(directive) - - for graphql_type in self._schema.type_map.values(): - add_directives_from_graphql_type(graphql_type) + add_directive(directive, track_application=False) + collect_new_definitions() - # Directive arguments can introduce input types with more attached - # directives. Collect that closure before rebuilding the schema once. - directive_index = 0 - while directive_index < len(schema_directive_types): - directive_type = schema_directive_types[directive_index] - graphql_directive = self.schema_converter.from_schema_directive( - directive_type - ) - for argument in graphql_directive.args.values(): - add_directives_from_argument_type(argument.type) - directive_index += 1 - - self._schema_directive_types = tuple(schema_directive_types) - - if self._schema_directive_types == registered_schema_directive_types: - return - - self._schema = GraphQLSchema( - query=self._graphql_query_type, - mutation=self._graphql_mutation_type, - subscription=self._graphql_subscription_type, - directives=self._collect_graphql_directives(self._schema_directive_types), - types=self._graphql_types, - extensions={ - GraphQLCoreConverter.DEFINITION_BACKREF: self, - }, - ) - self._schema._strawberry_schema = self # type: ignore + return tuple(schema_directive_types) def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning @@ -1400,10 +1384,13 @@ def _resolve_node_ids(self) -> None: if not has_custom_resolve_id: origin.resolve_id_attr() - def _should_register_schema_directive(self, directive: object) -> bool: - from strawberry.federation.schema_directives import FederationDirective + def _prepare_schema_directives(self) -> None: + pass - return not isinstance(directive, FederationDirective) + def _should_register_schema_directive(self, directive: object) -> bool: + # Integrations can opt out when their directives require schema-specific + # support types. Their Schema subclass can override this policy. + return getattr(directive, "__strawberry_register_definition__", True) def _warn_for_federation_directives(self) -> None: """Raises a warning if the schema has any federation directives.""" diff --git a/strawberry/schema/schema_converter.py b/strawberry/schema/schema_converter.py index b40801077f..0fb06608ea 100644 --- a/strawberry/schema/schema_converter.py +++ b/strawberry/schema/schema_converter.py @@ -539,13 +539,17 @@ def from_schema_directive(self, cls: type) -> GraphQLDirective: if default == dataclasses.MISSING: default = UNSET + field_type = field.resolve_type() + if field_type is UNRESOLVED: + raise UnresolvedFieldTypeError(strawberry_directive, field) + name = self.config.name_converter.get_graphql_name(field) args[name] = self.from_argument( StrawberryArgument( python_name=field.python_name or field.name, graphql_name=None, type_annotation=StrawberryAnnotation( - annotation=field.type, + annotation=field_type, namespace=module.__dict__, ), default=default, diff --git a/tests/federation/printer/test_compose_directive.py b/tests/federation/printer/test_compose_directive.py index 99d31ba904..f44fc9a712 100644 --- a/tests/federation/printer/test_compose_directive.py +++ b/tests/federation/printer/test_compose_directive.py @@ -1,10 +1,11 @@ import textwrap import strawberry +import strawberry.schema.schema as schema_module from strawberry.schema_directive import Location -def test_schema_directives_and_compose_schema(): +def test_schema_directives_and_compose_schema(monkeypatch): @strawberry.federation.schema_directive( locations=[Location.OBJECT], name="cacheControl", @@ -60,10 +61,29 @@ class Query: } """ + graphql_schema_calls = 0 + graphql_schema = schema_module.GraphQLSchema + validated_schemas: list[object] = [] + validate_schema = schema_module.validate_schema + + def counting_graphql_schema(*args: object, **kwargs: object): + nonlocal graphql_schema_calls + graphql_schema_calls += 1 + return graphql_schema(*args, **kwargs) + + def tracking_validate_schema(schema: object): + validated_schemas.append(schema) + return validate_schema(schema) + + monkeypatch.setattr(schema_module, "GraphQLSchema", counting_graphql_schema) + monkeypatch.setattr(schema_module, "validate_schema", tracking_validate_schema) + schema = strawberry.federation.Schema( query=Query, ) + assert graphql_schema_calls == 1 + assert validated_schemas == [schema._schema] assert schema.as_str() == textwrap.dedent(expected_type).strip() result = schema.execute_sync( diff --git a/tests/federation/test_schema.py b/tests/federation/test_schema.py index 1aac0fe843..5196a1960f 100644 --- a/tests/federation/test_schema.py +++ b/tests/federation/test_schema.py @@ -3,8 +3,10 @@ from typing import Generic, NewType, TypeVar import pytest +from graphql import build_schema import strawberry +from strawberry.federation.types import FieldSet, LinkImport, LinkPurpose def test_entities_type_when_no_type_has_keys(): @@ -197,6 +199,44 @@ class Query: assert schema.as_str() == expected_sdl +def test_private_federation_types_are_printed_when_used_by_fields(): + @strawberry.type + class Query: + field_set: FieldSet + link_import: LinkImport + link_purpose: LinkPurpose + + schema = strawberry.federation.Schema(query=Query) + + expected_sdl = textwrap.dedent(""" + type Query { + _service: _Service! + fieldSet: _FieldSet! + linkImport: link__Import! + linkPurpose: link__Purpose! + } + + scalar _Any + + scalar _FieldSet + + type _Service { + sdl: String! + } + + scalar link__Import + + enum link__Purpose { + SECURITY + EXECUTION + } + """).strip() + + sdl = schema.as_str() + assert sdl == expected_sdl + build_schema(sdl) + + def test_service(): @strawberry.federation.type class Product: diff --git a/tests/schema/test_permission.py b/tests/schema/test_permission.py index 144d549f9b..ae2d64d49e 100644 --- a/tests/schema/test_permission.py +++ b/tests/schema/test_permission.py @@ -531,6 +531,33 @@ def name(self) -> str: # pragma: no cover assert print_schema(schema) == textwrap.dedent(expected_output).strip() +def test_permission_directives_reused_across_fields(): + class IsAuthorized(BasePermission): + def has_permission(self, source, info, **kwargs: typing.Any) -> bool: + return True + + @strawberry.type + class Query: + first: str = strawberry.field( + extensions=[PermissionExtension([IsAuthorized()])] + ) + second: str = strawberry.field( + extensions=[PermissionExtension([IsAuthorized()])] + ) + + schema = strawberry.Schema(query=Query) + + expected_output = """ + directive @isAuthorized on FIELD_DEFINITION + + type Query { + first: String! @isAuthorized + second: String! @isAuthorized + } + """ + assert print_schema(schema) == textwrap.dedent(expected_output).strip() + + def test_permission_directives_not_added_on_field(): class IsAuthorized(BasePermission): message = "User is not authorized" diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index 52bb78ff4e..ad88b02f94 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -12,6 +12,9 @@ ) import strawberry +import strawberry.schema.schema as schema_module +from strawberry.exceptions import DuplicatedTypeName, UnresolvedFieldTypeError +from strawberry.schema.schema_converter import GraphQLCoreConverter from strawberry.schema_directive import Location @@ -274,6 +277,37 @@ class Query: assert schema.as_str() == textwrap.dedent(expected).strip() +def test_hidden_argument_types_are_printed_with_visible_directive_definitions(): + HiddenValue = strawberry.scalar(str, name="HiddenValue", print_definition=False) + + @strawberry.schema_directive(locations=[Location.FIELD_DEFINITION]) + class Marker: + value: HiddenValue + + @strawberry.type + class Query: + name: str = strawberry.field( + default="Patrick", + directives=[Marker(value="example")], + ) + + schema = strawberry.Schema(query=Query) + + expected = """ + directive @marker(value: HiddenValue!) on FIELD_DEFINITION + + scalar HiddenValue + + type Query { + name: String! @marker(value: "example") + } + """ + + sdl = schema.as_str() + assert sdl == textwrap.dedent(expected).strip() + build_schema(sdl) + + def test_registers_directives_from_all_type_system_attachment_points(): def directive(name: str, location: Location) -> type: @strawberry.schema_directive(name=name, locations=[location]) @@ -385,3 +419,122 @@ class Query: ), ): strawberry.Schema(query=Query) + + +def test_compatible_custom_one_of_uses_specified_directive(): + @strawberry.schema_directive(name="oneOf", locations=[Location.INPUT_OBJECT]) + class LegacyOneOf: ... + + @strawberry.input(directives=[LegacyOneOf()]) + class Choice: + value: str | None + + @strawberry.type + class Query: + @strawberry.field + def choose(self, choice: Choice) -> str: + return choice.value or "" + + schema = strawberry.Schema(query=Query) + + expected = """ + directive @oneOf on INPUT_OBJECT + + input Choice @oneOf { + value: String + } + + type Query { + choose(choice: Choice!): String! + } + """ + + assert schema.as_str() == textwrap.dedent(expected).strip() + assert schema._schema.get_directive("oneOf") is not None + + +def test_rejects_directive_argument_type_name_conflicts(): + @strawberry.input(name="Conflict") + class DirectiveInput: + value: str + + @strawberry.type(name="Conflict") + class RegularType: + value: str + + @strawberry.schema_directive(locations=[Location.OBJECT]) + class Marker: + config: DirectiveInput + + @strawberry.type(directives=[Marker(config=DirectiveInput(value="directive"))]) + class Query: + value: RegularType + + with pytest.raises( + DuplicatedTypeName, + match="Type Conflict is defined multiple times in the schema", + ): + strawberry.Schema(query=Query) + + +def test_constructs_graphql_schema_and_directive_definitions_once(monkeypatch): + @strawberry.schema_directive(locations=[Location.INPUT_OBJECT]) + class OnConfig: ... + + @strawberry.input(directives=[OnConfig()]) + class Config: + value: str + + @strawberry.schema_directive(locations=[Location.OBJECT]) + class Marker: + config: Config + + @strawberry.type(directives=[Marker(config=Config(value="example"))]) + class Query: + value: str + + graphql_schema_calls = 0 + converted_directives: list[type] = [] + graphql_schema = schema_module.GraphQLSchema + from_schema_directive = GraphQLCoreConverter.from_schema_directive + + def counting_graphql_schema(*args: object, **kwargs: object): + nonlocal graphql_schema_calls + graphql_schema_calls += 1 + return graphql_schema(*args, **kwargs) + + def counting_from_schema_directive(self, directive_type): + converted_directives.append(directive_type) + return from_schema_directive(self, directive_type) + + monkeypatch.setattr(schema_module, "GraphQLSchema", counting_graphql_schema) + monkeypatch.setattr( + GraphQLCoreConverter, + "from_schema_directive", + counting_from_schema_directive, + ) + + schema = strawberry.Schema(query=Query) + schema.as_str() + + assert graphql_schema_calls == 1 + assert converted_directives == [Marker, OnConfig] + + +def test_reports_unresolved_directive_argument_types(): + @strawberry.schema_directive(locations=[Location.OBJECT]) + class Marker: + config: "MissingConfig" # noqa: F821 + + @strawberry.type(directives=[Marker(config=None)]) # type: ignore[arg-type] + class Query: + name: str + + with pytest.raises( + UnresolvedFieldTypeError, + match=( + r"Could not resolve the type of 'config'\. Check that the class is " + r"accessible from the global module scope\." + ), + ): + strawberry.Schema(query=Query) From 31ac8fa49ace57b3613b51298a5287adb5e5e231 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sun, 30 Aug 2026 09:53:07 +0000 Subject: [PATCH 14/18] Address follow-up directive review feedback Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- RELEASE.md | 4 + strawberry/federation/schema.py | 25 ++++- strawberry/permission.py | 45 +++++---- strawberry/printer/printer.py | 65 +++++++++---- strawberry/schema/schema.py | 46 ++++++++- strawberry/schema/schema_converter.py | 8 +- .../printer/test_compose_directive.py | 49 ++++++++++ tests/federation/test_schema.py | 50 ++++++++++ tests/schema/test_permission.py | 95 +++++++++++++++++++ tests/schema/test_schema_directives.py | 33 +++++++ tests/test_printer/test_basic.py | 74 +++++++++++++++ tests/test_printer/test_schema_directives.py | 39 ++++++++ 12 files changed, 489 insertions(+), 44 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 8fda84c431..3d884a037c 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -20,6 +20,10 @@ allowed locations, repeatability, and any input types it uses. Federation direct including generated `@link` and `@composeDirective` applications, are discoverable in the same way. +Federation directives and custom composed directives used on field arguments are +also included in the generated subgraph metadata, so routers can recognize those +argument annotations without additional schema configuration. + A directive reused across the schema is defined only once. Input, enum, and scalar types referenced by directive arguments are now part of the schema and may appear in generated SDL even when they are not used by fields. diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index 52dcacac95..4de3ef9fc8 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -84,7 +84,12 @@ def __init__( # noqa: PLR0917 # Add FederationAny to types so it appears in the schema types = [*types, FederationAny] - # Add federation scalars to scalar_overrides so they can be recognized + # graphql-core needs Federation's directive argument types in the runtime + # schema for validation and introspection. Federation SDL imports the + # directive definitions through @link instead of printing them locally, + # so mark their private support types as hidden by default. The printer + # still emits one when a regular field makes that type part of the public + # schema. federation_scalar_overrides: dict[ object, type | ScalarDefinition | ScalarWrapper ] = { @@ -234,6 +239,11 @@ def entities_resolver( @property def schema_directives_in_use(self) -> list[object]: + # Federation needs this list to generate @link and @composeDirective + # applications before the GraphQLSchema exists. The base collector tracks + # it while traversing the same definitions used for schema construction, + # including argument and nested input definitions that the old post-build + # GraphQL type-map scan could not see. return self._schema_directives_in_use def _add_link_for_composed_directive( @@ -348,12 +358,23 @@ def _warn_for_federation_directives(self) -> None: pass def _prepare_schema_directives(self) -> None: + # This hook runs inside the base collector, before GraphQLSchema is built. + # Generated directives are therefore included in that single schema + # construction and in the validation of the schema that will be served. self._validate_directive_compatibility() composed_directives = self._add_compose_directives() self._add_link_directives(composed_directives) def _should_register_schema_directive(self, directive: object) -> bool: - return True + from .schema_directives import FederationDirective, Link + + # The base policy skips Federation definitions because a plain Schema + # does not install their private argument types. This subclass does, so + # its built-ins and generated @link directives are safe to register. + # Delegate all other directives to preserve integration opt-outs. + return isinstance( + directive, (FederationDirective, Link) + ) or super()._should_register_schema_directive(directive) def _get_entity_type( diff --git a/strawberry/permission.py b/strawberry/permission.py index 85659079bd..5136873993 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -56,6 +56,33 @@ def has_permission(self, source, info, **kwargs): error_class: type[GraphQLError] = StrawberryGraphQLError _schema_directive: ClassVar[object | None] = None + _auto_schema_directive: ClassVar[object] + + def __init_subclass__(cls, **kwargs: Any) -> None: + super().__init_subclass__(**kwargs) + + # Every instance of one permission class must use the same directive + # definition; otherwise two uses look like conflicting definitions with + # the same GraphQL name during schema collection. Build it when the + # permission subclass is created rather than lazily, which also avoids + # concurrent schema construction racing to create two definitions. Keep + # it separate from `_schema_directive` so instance, class, and inherited + # overrides work. + class AutoDirective: + __strawberry_directive__ = StrawberrySchemaDirective( + cls.__name__, + cls.__name__, + [Location.FIELD_DEFINITION], + [], + ) + + # Without this metadata a collision would report the local + # `AutoDirective` class twice. Make diagnostics point to the permission + # classes that users can actually rename. + AutoDirective.__name__ = cls.__name__ + AutoDirective.__qualname__ = cls.__qualname__ + AutoDirective.__module__ = cls.__module__ + cls._auto_schema_directive = AutoDirective() @abc.abstractmethod def has_permission( @@ -107,23 +134,7 @@ def on_unauthorized(self) -> None: @property def schema_directive(self) -> object: - permission_class = self.__class__ - if ( - schema_directive := permission_class.__dict__.get("_schema_directive") - ) is None: - - class AutoDirective: - __strawberry_directive__ = StrawberrySchemaDirective( - permission_class.__name__, - permission_class.__name__, - [Location.FIELD_DEFINITION], - [], - ) - - schema_directive = AutoDirective() - permission_class._schema_directive = schema_directive - - return schema_directive + return self._schema_directive or self._auto_schema_directive class PermissionExtension(FieldExtension): diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index 0a0c9b4bff..f1ce2e02b9 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -66,6 +66,7 @@ _T = TypeVar("_T") +_SCHEMA_TYPE_NAMES_CACHE_KEY = "strawberry-schema-type-names" @dataclasses.dataclass @@ -160,6 +161,10 @@ def print_schema_directive( "StrawberrySchemaDirective", directive.__class__.__strawberry_directive__ ) schema_converter = schema.schema_converter + # Registered directives were already converted while constructing the + # GraphQLSchema. Reuse that object so argument types and defaults match + # introspection exactly. The fallback is for definitions deliberately kept + # out of the runtime schema but still supported by Strawberry's SDL printer. gql_directive = getattr(schema, "_schema_graphql_directives", {}).get( directive.__class__ ) @@ -605,6 +610,9 @@ def is_builtin_directive(directive: GraphQLDirective) -> bool: def _should_print_type(type_: GraphQLNamedType, schema_type_names: set[str]) -> bool: strawberry_definition = type_.extensions.get("strawberry-definition") + # `print_definition=False` hides a private support type only while nothing + # visible refers to it. Reachability wins so a field can never produce SDL + # that names a type whose definition was omitted. return ( getattr(strawberry_definition, "print_definition", True) or type_.name in schema_type_names @@ -613,11 +621,27 @@ def _should_print_type(type_: GraphQLNamedType, schema_type_names: set[str]) -> def _get_schema_type_names(schema: BaseSchema) -> set[str]: graphql_schema = cast("GraphQLSchema", schema._schema) # type: ignore[attr-defined] + + # A schema is finalized before it can be printed, and Federation's _service + # resolver can print the same instance repeatedly. Cache this traversal on + # the GraphQLSchema instead of walking every field for each request. + if ( + cached_type_names := graphql_schema.extensions.get(_SCHEMA_TYPE_NAMES_CACHE_KEY) + ) is not None: + return cast("set[str]", cached_type_names) + + # graphql-core includes every named type referenced by a registered + # directive argument. Strawberry sometimes omits a directive definition from + # SDL (notably Federation directives imported with @link), and should omit + # private types used only by that hidden definition as well. Start from the + # roots, explicit/visible types, and visible directive arguments to determine + # which definitions the printed document really needs. A hidden support type + # remains visible when an ordinary field makes it reachable. stack = [ graphql_schema.query_type, graphql_schema.mutation_type, graphql_schema.subscription_type, - *schema._graphql_types, # type: ignore[attr-defined] + *getattr(schema, "_graphql_types", ()), ] for type_ in graphql_schema.type_map.values(): @@ -663,6 +687,8 @@ def _get_schema_type_names(schema: BaseSchema) -> set[str]: argument.type for argument in getattr(field, "args", {}).values() ) + graphql_schema.extensions[_SCHEMA_TYPE_NAMES_CACHE_KEY] = type_names + return type_names @@ -691,21 +717,6 @@ def print_schema(schema: BaseSchema) -> str: types_printed = [_print_type(type_, schema, extras=extras) for type_ in types] schema_definition = print_schema_definition(schema, extras=extras) - directives = [ - printed_directive - for directive in filtered_directives - if (printed_directive := print_directive(directive, schema=schema)) is not None - and printed_directive not in extras.directives - ] - - 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" - ) - def _name_getter(type_: Any) -> str: if hasattr(type_, "name"): return type_.name @@ -731,13 +742,33 @@ def _print_extra_types() -> Iterable[str]: yield _print_type(graphql_type, schema, extras=extras) + # Printing an extra directive argument type can discover directive + # applications attached to that type. Materialize extras before filtering + # the runtime directive list so each definition is emitted exactly once; + # relying on generator evaluation order would make this dedup accidental. + extra_types_printed = list(_print_extra_types()) + directives = [ + printed_directive + for directive in filtered_directives + if (printed_directive := print_directive(directive, schema=schema)) is not None + and printed_directive not in extras.directives + ] + + 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" + ) + return "\n\n".join( chain( sorted(extras.directives), filter(None, [schema_definition]), directives, types_printed, - _print_extra_types(), + extra_types_printed, ) ) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index ca9aa39a3b..0cc9f7c79f 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -412,6 +412,11 @@ class Query: self._explicit_schema_directive_types = tuple(explicit_schema_directive_types) try: + # graphql-core adds the argument types of every directive passed to + # GraphQLSchema to its type map. It cannot, however, discover + # directives stored only in Strawberry definitions. Collect and + # convert those directives first so the one GraphQLSchema we build + # receives the complete directive and type graph. self._schema_directive_types = self._collect_schema_directives( self._explicit_schema_directive_types ) @@ -478,6 +483,12 @@ def _add_graphql_directive( def _collect_graphql_directives( self, schema_directive_types: Iterable[type] ) -> tuple[GraphQLDirective, ...]: + # GraphQLSchema expects its directive collection to be canonical. Seed + # this registry with the specified directives so a custom definition + # cannot silently replace a built-in, then keep one insertion-ordered + # entry per GraphQL name. Reusing the same Strawberry definition is + # harmless while distinct definitions produce an error that identifies + # both Python sources. self._graphql_directives: dict[str, _DirectiveEntry] = { directive.name: ( directive, @@ -507,6 +518,9 @@ def _collect_graphql_directives( # Strawberry's OneOf schema directive predates graphql-core exposing # the specified @oneOf directive. Treat compatible definitions as the # canonical runtime directive regardless of the Python class used. + # Legacy definitions normally have no description; only accept a + # supplied description when it exactly matches the specified one so + # custom metadata is not silently discarded. if ( directive_name == "oneOf" and (existing := self._graphql_directives.get(directive_name)) @@ -515,6 +529,7 @@ def _collect_graphql_directives( and graphql_directive.locations == existing[0].locations and graphql_directive.args.keys() == existing[0].args.keys() and graphql_directive.is_repeatable == existing[0].is_repeatable + and graphql_directive.description in (None, existing[0].description) ): continue @@ -540,6 +555,12 @@ def _collect_graphql_directives( def _collect_schema_directives( self, explicit_schema_directive_types: Iterable[type] ) -> tuple[type, ...]: + # Directive definitions are not edges in graphql-core's type graph until + # they have been converted and passed to GraphQLSchema. Conversion can + # add directive argument input types to the converter map, and those + # input types may themselves have attached directives. Alternate between + # scanning definitions and converting newly found directives until both + # collections stop growing. schema_directive_types = list(dict.fromkeys(explicit_schema_directive_types)) seen_directive_types = set(schema_directive_types) seen_type_definitions: set[int] = set() @@ -548,6 +569,9 @@ def _collect_schema_directives( def add_directive(directive: object, *, track_application: bool) -> None: if track_application: + # Federation derives @link and @composeDirective applications + # from uses, including definitions opted out of runtime + # registration, so track applications independently. self._schema_directives_in_use.append(directive) directive_type = directive.__class__ @@ -562,6 +586,9 @@ def add_directive(directive: object, *, track_application: bool) -> None: def add_directives(owner: object, *, track_application: bool) -> None: attached_directives = getattr(owner, "directives", ()) or () if isinstance(attached_directives, Iterator): + # Strawberry accepts arbitrary iterables for directives. Later + # consumers such as Federation and the SDL printer need to see + # the same applications, so preserve one-shot iterators here. attached_directives = list(attached_directives) owner.directives = attached_directives # type: ignore[attr-defined] @@ -587,8 +614,10 @@ def collect_new_definitions() -> None: seen_type_definitions.add(id(definition)) made_progress = True - # Resolve lazy fields and unions so their reachable types are - # added to the converter map before GraphQLSchema is built. + # graphql-core would normally resolve these lazy field and + # union thunks while building its type map. We need to inspect + # Strawberry definitions before GraphQLSchema exists, so force + # that same reachability expansion through the converter now. tuple(getattr(concrete_type.implementation, "fields", {}).values()) tuple(getattr(concrete_type.implementation, "types", ())) @@ -600,6 +629,9 @@ def collect_new_definitions() -> None: for value in getattr(definition, "values", ()): add_directives(value, track_application=True) + # Converting a directive walks its arguments and adds their input + # graph to the converter map. The next outer iteration then scans + # those newly reachable definitions for more attached directives. while directive_index < len(schema_directive_types): directive_type = schema_directive_types[directive_index] self._schema_graphql_directives[directive_type] = ( @@ -615,7 +647,9 @@ def collect_new_definitions() -> None: self._prepare_schema_directives() # Subclasses can add schema applications such as Federation's generated - # @link and @composeDirective directives during preparation. + # @link and @composeDirective directives during preparation. Scan once + # more so their definitions and support types are included in the final + # GraphQLSchema rather than added after it has already been validated. for directive in self.schema_directives: add_directive(directive, track_application=False) collect_new_definitions() @@ -1388,8 +1422,10 @@ def _prepare_schema_directives(self) -> None: pass def _should_register_schema_directive(self, directive: object) -> bool: - # Integrations can opt out when their directives require schema-specific - # support types. Their Schema subclass can override this policy. + # Integrations can opt out when a plain Schema does not install the + # support types required by their directives. This controls runtime + # registration only: attached applications remain available to an + # integration's schema preparation and Strawberry's SDL printer. return getattr(directive, "__strawberry_register_definition__", True) def _warn_for_federation_directives(self) -> None: diff --git a/strawberry/schema/schema_converter.py b/strawberry/schema/schema_converter.py index 0fb06608ea..ba5f7097c6 100644 --- a/strawberry/schema/schema_converter.py +++ b/strawberry/schema/schema_converter.py @@ -2,7 +2,6 @@ import dataclasses import inspect -import sys import typing from functools import partial, reduce from typing import ( @@ -531,7 +530,6 @@ def from_schema_directive(self, cls: type) -> GraphQLDirective: "StrawberrySchemaDirective", cls.__strawberry_directive__, # type: ignore[attr-defined] ) - module = sys.modules[cls.__module__] args: dict[str, GraphQLArgument] = {} for field in strawberry_directive.fields: @@ -539,6 +537,11 @@ def from_schema_directive(self, cls: type) -> GraphQLDirective: if default == dataclasses.MISSING: default = UNSET + # Attached schema directives are now converted during schema + # construction instead of only when SDL happens to be printed. + # Resolve their annotations at the Strawberry boundary so unresolved + # forward references raise the usual actionable Strawberry error + # instead of surfacing later as a graphql-core type error. field_type = field.resolve_type() if field_type is UNRESOLVED: raise UnresolvedFieldTypeError(strawberry_directive, field) @@ -550,7 +553,6 @@ def from_schema_directive(self, cls: type) -> GraphQLDirective: graphql_name=None, type_annotation=StrawberryAnnotation( annotation=field_type, - namespace=module.__dict__, ), default=default, ) diff --git a/tests/federation/printer/test_compose_directive.py b/tests/federation/printer/test_compose_directive.py index f44fc9a712..aaf10a8158 100644 --- a/tests/federation/printer/test_compose_directive.py +++ b/tests/federation/printer/test_compose_directive.py @@ -1,4 +1,5 @@ import textwrap +from typing import Annotated import strawberry import strawberry.schema.schema as schema_module @@ -174,3 +175,51 @@ class Query: ) assert schema.as_str() == textwrap.dedent(expected_type).strip() + + +def test_composes_directives_attached_to_arguments(): + @strawberry.federation.schema_directive( + locations=[Location.ARGUMENT_DEFINITION], + name="validate", + compose=True, + import_url="https://example.com/validate/v1.0", + ) + class Validate: + pattern: str + + @strawberry.type + class Query: + @strawberry.field + def search( + self, + term: Annotated[ + str, + strawberry.federation.argument( + directives=[Validate(pattern="letters-only")] + ), + ], + ) -> str: + return term + + schema = strawberry.federation.Schema(query=Query) + + expected_type = """ + directive @validate(pattern: String!) on ARGUMENT_DEFINITION + + schema @composeDirective(name: "@validate") @link(url: "https://example.com/validate/v1.0", import: ["@validate"]) @link(url: "https://specs.apollo.dev/federation/v2.11", import: ["@composeDirective"]) { + query: Query + } + + type Query { + _service: _Service! + search(term: String! @validate(pattern: "letters-only")): String! + } + + scalar _Any + + type _Service { + sdl: String! + } + """ + + assert schema.as_str() == textwrap.dedent(expected_type).strip() diff --git a/tests/federation/test_schema.py b/tests/federation/test_schema.py index 5196a1960f..031ec08ab4 100644 --- a/tests/federation/test_schema.py +++ b/tests/federation/test_schema.py @@ -7,6 +7,7 @@ import strawberry from strawberry.federation.types import FieldSet, LinkImport, LinkPurpose +from strawberry.schema_directive import Location def test_entities_type_when_no_type_has_keys(): @@ -135,6 +136,55 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover } +def test_runtime_registration_opt_out_does_not_hide_federation_directives(): + @strawberry.schema_directive(locations=[Location.OBJECT]) + class RuntimeOptOut: + __strawberry_register_definition__ = False + + @strawberry.federation.type(keys=["upc"], directives=[RuntimeOptOut()]) + class Product: + upc: str + + @strawberry.type + class Query: + product: Product + + schema = strawberry.federation.Schema(query=Query) + + assert schema._schema.get_directive("runtimeOptOut") is None + assert schema._schema.get_directive("key") is not None + assert schema._schema.get_directive("link") is not None + + expected_sdl = textwrap.dedent(""" + directive @runtimeOptOut on OBJECT + + schema @link(url: "https://specs.apollo.dev/federation/v2.11", import: ["@key"]) { + query: Query + } + + type Product @runtimeOptOut @key(fields: "upc") { + upc: String! + } + + type Query { + _entities(representations: [_Any!]!): [_Entity]! + _service: _Service! + product: Product! + } + + scalar _Any + + union _Entity = Product + + type _Service { + sdl: String! + } + """).strip() + + sdl = schema.as_str() + assert sdl == expected_sdl + + def test_additional_scalars(): @strawberry.federation.type(keys=["upc"]) class Example: diff --git a/tests/schema/test_permission.py b/tests/schema/test_permission.py index ae2d64d49e..d7d944577d 100644 --- a/tests/schema/test_permission.py +++ b/tests/schema/test_permission.py @@ -11,6 +11,7 @@ ) from strawberry.permission import BasePermission, PermissionExtension from strawberry.printer import print_schema +from strawberry.schema_directive import Location from strawberry.utils.aio import aclosing @@ -558,6 +559,100 @@ class Query: assert print_schema(schema) == textwrap.dedent(expected_output).strip() +def test_permission_directive_can_be_overridden_on_an_instance(): + @strawberry.schema_directive( + name="requiresRole", locations=[Location.FIELD_DEFINITION] + ) + class RequiresRole: + role: str + + class IsAuthorized(BasePermission): + def has_permission(self, source, info, **kwargs: typing.Any) -> bool: + return True + + permission = IsAuthorized() + permission._schema_directive = RequiresRole(role="member") + + @strawberry.type + class Query: + name: str = strawberry.field(extensions=[PermissionExtension([permission])]) + + schema = strawberry.Schema(query=Query) + + expected_output = """ + directive @requiresRole(role: String!) on FIELD_DEFINITION + + type Query { + name: String! @requiresRole(role: "member") + } + """ + assert print_schema(schema) == textwrap.dedent(expected_output).strip() + + +def test_permission_directive_can_be_inherited(): + @strawberry.schema_directive( + name="requiresRole", locations=[Location.FIELD_DEFINITION] + ) + class RequiresRole: + role: str + + class IsAuthorized(BasePermission): + _schema_directive = RequiresRole(role="member") + + def has_permission(self, source, info, **kwargs: typing.Any) -> bool: + return True + + class IsMember(IsAuthorized): ... + + @strawberry.type + class Query: + name: str = strawberry.field(extensions=[PermissionExtension([IsMember()])]) + + schema = strawberry.Schema(query=Query) + + expected_output = """ + directive @requiresRole(role: String!) on FIELD_DEFINITION + + type Query { + name: String! @requiresRole(role: "member") + } + """ + assert print_schema(schema) == textwrap.dedent(expected_output).strip() + + +def test_permission_directive_collision_names_permission_classes(): + def has_permission( + self: BasePermission, source, info, **kwargs: typing.Any + ) -> bool: + return True + + First = type( + "CanAccess", + (BasePermission,), + {"__module__": "permissions.first", "has_permission": has_permission}, + ) + Second = type( + "CanAccess", + (BasePermission,), + {"__module__": "permissions.second", "has_permission": has_permission}, + ) + + @strawberry.type + class Query: + first: str = strawberry.field(extensions=[PermissionExtension([First()])]) + second: str = strawberry.field(extensions=[PermissionExtension([Second()])]) + + with pytest.raises( + ValueError, + match=( + r"Schema directive '@canAccess' is defined by both schema directive " + r"'permissions\.first\.CanAccess' and schema directive " + r"'permissions\.second\.CanAccess'" + ), + ): + strawberry.Schema(query=Query) + + def test_permission_directives_not_added_on_field(): class IsAuthorized(BasePermission): message = "User is not authorized" diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index ad88b02f94..23d0727824 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -9,6 +9,7 @@ GraphQLScalarType, build_schema, get_named_type, + specified_directives, ) import strawberry @@ -453,6 +454,38 @@ def choose(self, choice: Choice) -> str: assert schema._schema.get_directive("oneOf") is not None +@pytest.mark.skipif( + not any(directive.name == "oneOf" for directive in specified_directives), + reason="graphql-core does not define the specified @oneOf directive", +) +def test_rejects_custom_one_of_with_a_different_description(): + @strawberry.schema_directive( + name="oneOf", + description="A custom oneOf definition.", + locations=[Location.INPUT_OBJECT], + ) + class CustomOneOf: ... + + @strawberry.input(directives=[CustomOneOf()]) + class Choice: + value: str | None + + @strawberry.type + class Query: + @strawberry.field + def choose(self, choice: Choice) -> str: + return choice.value or "" + + with pytest.raises( + ValueError, + match=( + r"Schema directive '@oneOf' is defined by both the built-in GraphQL " + r"directive and schema directive .*CustomOneOf" + ), + ): + strawberry.Schema(query=Query) + + def test_rejects_directive_argument_type_name_conflicts(): @strawberry.input(name="Conflict") class DirectiveInput: diff --git a/tests/test_printer/test_basic.py b/tests/test_printer/test_basic.py index 6101b04356..d14ed838e0 100644 --- a/tests/test_printer/test_basic.py +++ b/tests/test_printer/test_basic.py @@ -1,4 +1,5 @@ import textwrap +from types import SimpleNamespace from uuid import UUID import pytest @@ -40,6 +41,79 @@ class Query: assert print_schema(schema) == textwrap.dedent(expected_type).strip() +def test_prints_base_schema_without_explicit_graphql_types(): + @strawberry.type + class Query: + name: str + + schema = strawberry.Schema(query=Query) + custom_schema = SimpleNamespace( + _schema=schema._schema, + config=schema.config, + mutation=schema.mutation, + query=schema.query, + schema_converter=schema.schema_converter, + schema_directives=schema.schema_directives, + subscription=schema.subscription, + ) + + expected_type = """ + type Query { + name: String! + } + """ + + assert print_schema(custom_schema) == textwrap.dedent(expected_type).strip() + + +def test_caches_schema_type_reachability(monkeypatch): + @strawberry.interface + class Node: + id: strawberry.ID + + @strawberry.type + class User(Node): + name: str + + @strawberry.type + class Query: + node: Node + + schema = strawberry.Schema(query=Query, types=[User]) + get_possible_types = schema._schema.get_possible_types + calls = 0 + + def counting_get_possible_types(type_): + nonlocal calls + calls += 1 + return get_possible_types(type_) + + monkeypatch.setattr( + schema._schema, "get_possible_types", counting_get_possible_types + ) + + expected_type = """ + interface Node { + id: ID! + } + + type Query { + node: Node! + } + + type User implements Node { + id: ID! + name: String! + } + """ + expected_type = textwrap.dedent(expected_type).strip() + + assert print_schema(schema) == expected_type + assert calls == 1 + assert print_schema(schema) == expected_type + assert calls == 1 + + def test_printer_with_camel_case_on(): @strawberry.type class Query: diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 26e270ee32..967ac0d9e2 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -514,6 +514,45 @@ def run(self, config: Config) -> bool: assert print_schema(schema) == textwrap.dedent(expected_output).strip() +def test_deduplicates_directives_discovered_while_printing_extra_types(): + @strawberry.schema_directive(locations=[Location.INPUT_OBJECT]) + class OnConfig: ... + + @strawberry.input(directives=[OnConfig()]) + class Config: + value: str + + @strawberry.schema_directive(locations=[Location.OBJECT]) + class WithConfig: + __strawberry_register_definition__ = False + + config: Config | None = strawberry.UNSET + + @strawberry.type(directives=[WithConfig()]) + class Query: + name: str + + schema = strawberry.Schema(query=Query, types=[OnConfig]) + + expected_output = """ + directive @onConfig on INPUT_OBJECT + + directive @withConfig(config: Config) on OBJECT + + type Query @withConfig { + name: String! + } + + input Config @onConfig { + value: String! + } + """ + expected_output = textwrap.dedent(expected_output).strip() + + assert print_schema(schema) == expected_output + assert print_schema(schema) == expected_output + + def test_does_not_print_definition(): @strawberry.schema_directive( locations=[Location.FIELD_DEFINITION], print_definition=False From da8b33eb67aff3b62a6d5d9af91411cfe53ff41a Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sun, 30 Aug 2026 10:07:17 +0000 Subject: [PATCH 15/18] Simplify hidden SDL type reachability Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- strawberry/printer/printer.py | 11 +++++------ tests/test_printer/test_basic.py | 19 ++++++------------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index f1ce2e02b9..83164d5474 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -610,9 +610,9 @@ def is_builtin_directive(directive: GraphQLDirective) -> bool: def _should_print_type(type_: GraphQLNamedType, schema_type_names: set[str]) -> bool: strawberry_definition = type_.extensions.get("strawberry-definition") - # `print_definition=False` hides a private support type only while nothing - # visible refers to it. Reachability wins so a field can never produce SDL - # that names a type whose definition was omitted. + # `print_definition=False` omits otherwise-private support types from SDL. + # If a visible field or printed directive definition reaches one, its + # definition is restored so SDL never references an undefined type. return ( getattr(strawberry_definition, "print_definition", True) or type_.name in schema_type_names @@ -634,14 +634,13 @@ def _get_schema_type_names(schema: BaseSchema) -> set[str]: # directive argument. Strawberry sometimes omits a directive definition from # SDL (notably Federation directives imported with @link), and should omit # private types used only by that hidden definition as well. Start from the - # roots, explicit/visible types, and visible directive arguments to determine - # which definitions the printed document really needs. A hidden support type + # roots, visible types, and visible directive arguments to determine which + # definitions the printed document really needs. A hidden support type # remains visible when an ordinary field makes it reachable. stack = [ graphql_schema.query_type, graphql_schema.mutation_type, graphql_schema.subscription_type, - *getattr(schema, "_graphql_types", ()), ] for type_ in graphql_schema.type_map.values(): diff --git a/tests/test_printer/test_basic.py b/tests/test_printer/test_basic.py index d14ed838e0..b0de2528d6 100644 --- a/tests/test_printer/test_basic.py +++ b/tests/test_printer/test_basic.py @@ -1,5 +1,4 @@ import textwrap -from types import SimpleNamespace from uuid import UUID import pytest @@ -41,21 +40,14 @@ class Query: assert print_schema(schema) == textwrap.dedent(expected_type).strip() -def test_prints_base_schema_without_explicit_graphql_types(): +def test_does_not_print_unreferenced_hidden_explicit_type(): + Hidden = strawberry.scalar(str, name="Hidden", print_definition=False) + @strawberry.type class Query: name: str - schema = strawberry.Schema(query=Query) - custom_schema = SimpleNamespace( - _schema=schema._schema, - config=schema.config, - mutation=schema.mutation, - query=schema.query, - schema_converter=schema.schema_converter, - schema_directives=schema.schema_directives, - subscription=schema.subscription, - ) + schema = strawberry.Schema(query=Query, types=[Hidden]) expected_type = """ type Query { @@ -63,7 +55,8 @@ class Query: } """ - assert print_schema(custom_schema) == textwrap.dedent(expected_type).strip() + assert schema._schema.get_type("Hidden") is not None + assert print_schema(schema) == textwrap.dedent(expected_type).strip() def test_caches_schema_type_reachability(monkeypatch): From 6614832d76faa3a835d406dc1e4678adfb0a3e81 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sun, 30 Aug 2026 10:30:47 +0000 Subject: [PATCH 16/18] Address final schema directive review Amp-Thread-ID: https://ampcode.com/threads/T-01a04d3e-60f6-7768-ba15-4f10d54eea17 --- RELEASE.md | 4 +- strawberry/federation/schema.py | 34 +--- strawberry/federation/schema_directives.py | 5 - strawberry/federation/types.py | 13 ++ strawberry/printer/printer.py | 24 ++- strawberry/schema/schema.py | 194 ++++++++++++------- tests/federation/test_schema.py | 17 +- tests/schema/test_schema_directives.py | 29 +++ tests/test_printer/test_schema_directives.py | 14 +- 9 files changed, 203 insertions(+), 131 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3d884a037c..09cd4408f5 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -33,4 +33,6 @@ their GraphQL names must be unique. Schema construction reports a clear error wh different directive definitions share a name, a custom directive replaces a built-in directive such as `@skip`, or a directive argument type conflicts with another schema type. Compatible custom `@oneOf` definitions continue to use -GraphQL's built-in directive. +GraphQL's built-in directive. Strawberry now also resolves attached directive +argument annotations during schema construction, so unresolved forward references +are reported when the schema is created instead of later when its SDL is printed. diff --git a/strawberry/federation/schema.py b/strawberry/federation/schema.py index 4de3ef9fc8..b2426d611e 100644 --- a/strawberry/federation/schema.py +++ b/strawberry/federation/schema.py @@ -25,7 +25,6 @@ from strawberry.utils.inspect import get_func_args from .schema_directive import StrawberryFederationSchemaDirective -from .types import FieldSet, LinkImport from .versions import format_version, parse_version if TYPE_CHECKING: @@ -84,30 +83,16 @@ def __init__( # noqa: PLR0917 # Add FederationAny to types so it appears in the schema types = [*types, FederationAny] - # graphql-core needs Federation's directive argument types in the runtime - # schema for validation and introspection. Federation SDL imports the - # directive definitions through @link instead of printing them locally, - # so mark their private support types as hidden by default. The printer - # still emits one when a regular field makes that type part of the public - # schema. + # _Any belongs to Federation's entity resolver rather than a directive, + # so this schema installs its scalar mapping. Directive support types + # such as _FieldSet carry their definitions on the annotations themselves; + # that lets both Schema classes register attached Federation directives. federation_scalar_overrides: dict[ object, type | ScalarDefinition | ScalarWrapper ] = { FederationAny: scalar( name="_Any", serialize=lambda v: v, parse_value=lambda v: v ), - FieldSet: scalar( - name="_FieldSet", - serialize=lambda v: v, - parse_value=str, - print_definition=False, - ), - LinkImport: scalar( - name="link__Import", - serialize=lambda v: v, - parse_value=lambda v: v, - print_definition=False, - ), } if scalar_overrides: federation_scalar_overrides.update(scalar_overrides) @@ -365,17 +350,6 @@ def _prepare_schema_directives(self) -> None: composed_directives = self._add_compose_directives() self._add_link_directives(composed_directives) - def _should_register_schema_directive(self, directive: object) -> bool: - from .schema_directives import FederationDirective, Link - - # The base policy skips Federation definitions because a plain Schema - # does not install their private argument types. This subclass does, so - # its built-ins and generated @link directives are safe to register. - # Delegate all other directives to preserve integration opt-outs. - return isinstance( - directive, (FederationDirective, Link) - ) or super()._should_register_schema_directive(directive) - def _get_entity_type( query: type[WithStrawberryObjectDefinition] | None, diff --git a/strawberry/federation/schema_directives.py b/strawberry/federation/schema_directives.py index 1d2147370f..2585aff5c3 100644 --- a/strawberry/federation/schema_directives.py +++ b/strawberry/federation/schema_directives.py @@ -28,8 +28,6 @@ def with_version(self, version: str) -> "ImportedFrom": class FederationDirective: imported_from: ClassVar[ImportedFrom] minimum_version: ClassVar[FederationVersion] - # The base Schema does not install Federation's supporting scalar mappings. - __strawberry_register_definition__: ClassVar[bool] = False @schema_directive( @@ -96,9 +94,6 @@ class Shareable(FederationDirective): locations=[Location.SCHEMA], name="link", repeatable=True, print_definition=False ) class Link: - # The base Schema does not install Federation's supporting scalar mappings. - __strawberry_register_definition__: ClassVar[bool] = False - url: str | None as_: str | None = directive_field(name="as") for_: LinkPurpose | None = directive_field(name="for") diff --git a/strawberry/federation/types.py b/strawberry/federation/types.py index 85ed044595..b465c3e0b0 100644 --- a/strawberry/federation/types.py +++ b/strawberry/federation/types.py @@ -2,12 +2,25 @@ from typing import NewType from strawberry.types.enum import enum +from strawberry.types.scalar import scalar FieldSet = NewType("FieldSet", str) """Represents a selection set for federation @requires, @provides, @key directives.""" +FieldSet._scalar_definition = scalar( # type: ignore[attr-defined] + name="_FieldSet", + serialize=lambda value: value, + parse_value=str, + print_definition=False, +) LinkImport = NewType("LinkImport", object) """Represents an import for the @link directive.""" +LinkImport._scalar_definition = scalar( # type: ignore[attr-defined] + name="link__Import", + serialize=lambda value: value, + parse_value=lambda value: value, + print_definition=False, +) @enum(name="link__Purpose", print_definition=False) diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index 83164d5474..c12627571d 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -55,8 +55,10 @@ GraphQLArgument, GraphQLEnumType, GraphQLEnumValue, + GraphQLInterfaceType, GraphQLNamedType, GraphQLScalarType, + GraphQLType, GraphQLUnionType, ) from graphql.type.directives import GraphQLDirective @@ -637,18 +639,18 @@ def _get_schema_type_names(schema: BaseSchema) -> set[str]: # roots, visible types, and visible directive arguments to determine which # definitions the printed document really needs. A hidden support type # remains visible when an ordinary field makes it reachable. - stack = [ + stack: list[GraphQLType | None] = [ graphql_schema.query_type, graphql_schema.mutation_type, graphql_schema.subscription_type, ] - for type_ in graphql_schema.type_map.values(): - strawberry_definition = type_.extensions.get("strawberry-definition") - if is_defined_type(type_) and getattr( + for schema_type in graphql_schema.type_map.values(): + strawberry_definition = schema_type.extensions.get("strawberry-definition") + if is_defined_type(schema_type) and getattr( strawberry_definition, "print_definition", True ): - stack.append(type_) + stack.append(schema_type) for directive in graphql_schema.directives: if is_builtin_directive(directive): @@ -666,11 +668,11 @@ def _get_schema_type_names(schema: BaseSchema) -> set[str]: type_names: set[str] = set() while stack: - type_ = stack.pop() - if type_ is None: + graphql_type = stack.pop() + if graphql_type is None: continue - named_type = get_named_type(type_) + named_type = get_named_type(graphql_type) if named_type.name in type_names: continue @@ -678,7 +680,11 @@ def _get_schema_type_names(schema: BaseSchema) -> set[str]: stack.extend(getattr(named_type, "interfaces", ())) stack.extend(getattr(named_type, "types", ())) if is_interface_type(named_type): - stack.extend(graphql_schema.get_possible_types(named_type)) + stack.extend( + graphql_schema.get_possible_types( + cast("GraphQLInterfaceType", named_type) + ) + ) for field in getattr(named_type, "fields", {}).values(): stack.append(field.type) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 0cc9f7c79f..35af9d2d57 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -38,6 +38,7 @@ GraphQLSchema, OperationDefinitionNode, get_introspection_query, + get_named_type, parse, validate_schema, ) @@ -535,7 +536,7 @@ def _collect_graphql_directives( self._add_graphql_directive( graphql_directive, - directive_type, + strawberry_directive, ( "schema directive " f"'{directive_type.__module__}.{directive_type.__qualname__}'" @@ -555,33 +556,52 @@ def _collect_graphql_directives( def _collect_schema_directives( self, explicit_schema_directive_types: Iterable[type] ) -> tuple[type, ...]: - # Directive definitions are not edges in graphql-core's type graph until - # they have been converted and passed to GraphQLSchema. Conversion can - # add directive argument input types to the converter map, and those - # input types may themselves have attached directives. Alternate between - # scanning definitions and converting newly found directives until both - # collections stop growing. - schema_directive_types = list(dict.fromkeys(explicit_schema_directive_types)) - seen_directive_types = set(schema_directive_types) - seen_type_definitions: set[int] = set() + # graphql-core can walk fields and directive argument types once it has a + # GraphQLSchema, but attached Strawberry directives are not edges in that + # graph. Walk the same reachable GraphQL types before constructing the + # schema, and add each converted directive's argument graph to the queue. + # This finds directives on nested directive-only input types without + # repeatedly rescanning the converter's complete type map. + schema_directive_types: list[type] = [] + seen_directive_types: set[type] = set() + directive_types_by_definition: dict[int, type] = {} + directive_type_aliases: dict[type, type] = {} self._schema_graphql_directives: dict[type, GraphQLDirective] = {} self._schema_directives_in_use: list[object] = [] + def add_directive_type(directive_type: type) -> None: + if directive_type in seen_directive_types: + return + + seen_directive_types.add(directive_type) + strawberry_directive = cast("Any", directive_type).__strawberry_directive__ + + # A plain subclass inherits its parent's Strawberry directive + # definition. It is another Python spelling for the same GraphQL + # directive, not a conflicting definition that needs conversion. + definition_id = id(strawberry_directive) + if ( + canonical_type := directive_types_by_definition.get(definition_id) + ) is not None: + directive_type_aliases[directive_type] = canonical_type + return + + directive_types_by_definition[definition_id] = directive_type + schema_directive_types.append(directive_type) + + for directive_type in explicit_schema_directive_types: + add_directive_type(directive_type) + def add_directive(directive: object, *, track_application: bool) -> None: if track_application: # Federation derives @link and @composeDirective applications - # from uses, including definitions opted out of runtime - # registration, so track applications independently. + # from uses, so keep their order independently from the + # deduplicated definition collection. self._schema_directives_in_use.append(directive) directive_type = directive.__class__ - if ( - self._should_register_schema_directive(directive) - and compat.is_schema_directive(directive_type) - and directive_type not in seen_directive_types - ): - seen_directive_types.add(directive_type) - schema_directive_types.append(directive_type) + if compat.is_schema_directive(directive_type): + add_directive_type(directive_type) def add_directives(owner: object, *, track_application: bool) -> None: attached_directives = getattr(owner, "directives", ()) or () @@ -598,61 +618,102 @@ def add_directives(owner: object, *, track_application: bool) -> None: for directive in self.schema_directives: add_directive(directive, track_application=False) + graphql_types: list[GraphQLNamedType] = [] + seen_graphql_types: set[int] = set() + + def add_graphql_type(graphql_type: Any) -> None: + if graphql_type is None: + return + + named_type = get_named_type(graphql_type) + if id(named_type) in seen_graphql_types: + return + + seen_graphql_types.add(id(named_type)) + graphql_types.append(named_type) + + add_graphql_type(self._graphql_query_type) + add_graphql_type(self._graphql_mutation_type) + add_graphql_type(self._graphql_subscription_type) + for graphql_type in self._graphql_types: + add_graphql_type(graphql_type) + + graphql_type_index = 0 directive_index = 0 def collect_new_definitions() -> None: - nonlocal directive_index - - while True: - made_progress = False - - for concrete_type in tuple(self.schema_converter.type_map.values()): - definition = concrete_type.definition - if id(definition) in seen_type_definitions: - continue - - seen_type_definitions.add(id(definition)) - made_progress = True - - # graphql-core would normally resolve these lazy field and - # union thunks while building its type map. We need to inspect - # Strawberry definitions before GraphQLSchema exists, so force - # that same reachability expansion through the converter now. - tuple(getattr(concrete_type.implementation, "fields", {}).values()) - tuple(getattr(concrete_type.implementation, "types", ())) - - add_directives(definition, track_application=True) - for field in getattr(definition, "fields", ()): - add_directives(field, track_application=True) - for argument in getattr(field, "arguments", ()): - add_directives(argument, track_application=True) - for value in getattr(definition, "values", ()): - add_directives(value, track_application=True) - - # Converting a directive walks its arguments and adds their input - # graph to the converter map. The next outer iteration then scans - # those newly reachable definitions for more attached directives. + nonlocal directive_index, graphql_type_index + + while graphql_type_index < len(graphql_types) or directive_index < len( + schema_directive_types + ): + while graphql_type_index < len(graphql_types): + graphql_type = graphql_types[graphql_type_index] + graphql_type_index += 1 + + # Resolve graphql-core's lazy thunks before reading the + # Strawberry definition. Field extensions can attach schema + # directives while fields are converted (permissions do + # this), so inspecting the definition first would miss them. + graphql_fields = getattr(graphql_type, "fields", {}) + interfaces = getattr(graphql_type, "interfaces", ()) + member_types = getattr(graphql_type, "types", ()) + + definition = graphql_type.extensions.get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if definition is not None: + add_directives(definition, track_application=True) + for field in getattr(definition, "fields", ()): + add_directives(field, track_application=True) + for argument in getattr(field, "arguments", ()): + add_directives(argument, track_application=True) + for value in getattr(definition, "values", ()): + add_directives(value, track_application=True) + + # Queue the resolved edges explicitly so definitions reached + # through regular fields and directive arguments use one + # traversal and one source of attachment metadata. + for field in graphql_fields.values(): + add_graphql_type(field.type) + for argument in getattr(field, "args", {}).values(): + add_graphql_type(argument.type) + for interface in interfaces: + add_graphql_type(interface) + for member_type in member_types: + add_graphql_type(member_type) + + # Conversion exposes argument input types that graphql-core + # cannot contribute until the directive is registered. Queue + # those types, then continue the same traversal to discover any + # directives attached inside their nested input graph. while directive_index < len(schema_directive_types): directive_type = schema_directive_types[directive_index] - self._schema_graphql_directives[directive_type] = ( - self.schema_converter.from_schema_directive(directive_type) + graphql_directive = self.schema_converter.from_schema_directive( + directive_type ) + self._schema_graphql_directives[directive_type] = graphql_directive directive_index += 1 - made_progress = True - - if not made_progress: - return + for argument in graphql_directive.args.values(): + add_graphql_type(argument.type) collect_new_definitions() + schema_directive_count = len(self.schema_directives) self._prepare_schema_directives() - # Subclasses can add schema applications such as Federation's generated - # @link and @composeDirective directives during preparation. Scan once - # more so their definitions and support types are included in the final - # GraphQLSchema rather than added after it has already been validated. - for directive in self.schema_directives: - add_directive(directive, track_application=False) - collect_new_definitions() + if len(self.schema_directives) > schema_directive_count: + # Federation prepares generated @link and @composeDirective + # applications here. Only process the new tail; a plain Schema does + # no second traversal, and the final GraphQLSchema still receives + # every generated directive and support type before validation. + for directive in self.schema_directives[schema_directive_count:]: + add_directive(directive, track_application=False) + collect_new_definitions() + + for alias, canonical_type in directive_type_aliases.items(): + self._schema_graphql_directives[alias] = self._schema_graphql_directives[ + canonical_type + ] return tuple(schema_directive_types) @@ -1421,13 +1482,6 @@ def _resolve_node_ids(self) -> None: def _prepare_schema_directives(self) -> None: pass - def _should_register_schema_directive(self, directive: object) -> bool: - # Integrations can opt out when a plain Schema does not install the - # support types required by their directives. This controls runtime - # registration only: attached applications remain available to an - # integration's schema preparation and Strawberry's SDL printer. - return getattr(directive, "__strawberry_register_definition__", True) - def _warn_for_federation_directives(self) -> None: """Raises a warning if the schema has any federation directives.""" from strawberry.federation.schema_directives import FederationDirective diff --git a/tests/federation/test_schema.py b/tests/federation/test_schema.py index 031ec08ab4..ff7bb351f5 100644 --- a/tests/federation/test_schema.py +++ b/tests/federation/test_schema.py @@ -136,12 +136,11 @@ def top_products(self, first: int) -> list[Product]: # pragma: no cover } -def test_runtime_registration_opt_out_does_not_hide_federation_directives(): +def test_registers_custom_and_federation_directives(): @strawberry.schema_directive(locations=[Location.OBJECT]) - class RuntimeOptOut: - __strawberry_register_definition__ = False + class Custom: ... - @strawberry.federation.type(keys=["upc"], directives=[RuntimeOptOut()]) + @strawberry.federation.type(keys=["upc"], directives=[Custom()]) class Product: upc: str @@ -151,18 +150,18 @@ class Query: schema = strawberry.federation.Schema(query=Query) - assert schema._schema.get_directive("runtimeOptOut") is None + assert schema._schema.get_directive("custom") is not None assert schema._schema.get_directive("key") is not None assert schema._schema.get_directive("link") is not None expected_sdl = textwrap.dedent(""" - directive @runtimeOptOut on OBJECT + directive @custom on OBJECT schema @link(url: "https://specs.apollo.dev/federation/v2.11", import: ["@key"]) { query: Query } - type Product @runtimeOptOut @key(fields: "upc") { + type Product @custom @key(fields: "upc") { upc: String! } @@ -461,7 +460,7 @@ class ProductFed: weight: int | None with pytest.warns(UserWarning) as record: # noqa: PT030 - strawberry.Schema( + schema = strawberry.Schema( query=ProductFed, ) @@ -470,6 +469,8 @@ class ProductFed: "Use `strawberry.federation.Schema` instead of `strawberry.Schema`." in [str(r.message) for r in record] ) + assert schema._schema.get_directive("key") is not None + assert schema._schema.get_type("_FieldSet") is not None def test_does_not_warn_when_using_federation_schema(): diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index 23d0727824..f80b0c7360 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -163,6 +163,35 @@ class Query: build_schema(sdl) +def test_plain_subclass_reuses_inherited_directive_definition(): + @strawberry.schema_directive(locations=[Location.OBJECT, Location.FIELD_DEFINITION]) + class Marker: ... + + class FieldMarker(Marker): ... + + @strawberry.type(directives=[Marker()]) + class Query: + name: str = strawberry.field(default="Patrick", directives=[FieldMarker()]) + + schema = strawberry.Schema(query=Query) + + assert ( + sum(directive.name == "marker" for directive in schema._schema.directives) == 1 + ) + + expected = """ + directive @marker on OBJECT | FIELD_DEFINITION + + type Query @marker { + name: String! @marker + } + """ + + sdl = schema.as_str() + assert sdl == textwrap.dedent(expected).strip() + build_schema(sdl) + + def test_registers_nested_directive_argument_input_types(): @strawberry.schema_directive(locations=[Location.INPUT_OBJECT]) class OnNestedInput: ... diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 967ac0d9e2..2a84479af6 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -514,7 +514,7 @@ def run(self, config: Config) -> bool: assert print_schema(schema) == textwrap.dedent(expected_output).strip() -def test_deduplicates_directives_discovered_while_printing_extra_types(): +def test_deduplicates_directives_discovered_in_directive_argument_types(): @strawberry.schema_directive(locations=[Location.INPUT_OBJECT]) class OnConfig: ... @@ -524,28 +524,26 @@ class Config: @strawberry.schema_directive(locations=[Location.OBJECT]) class WithConfig: - __strawberry_register_definition__ = False - config: Config | None = strawberry.UNSET @strawberry.type(directives=[WithConfig()]) class Query: name: str - schema = strawberry.Schema(query=Query, types=[OnConfig]) + schema = strawberry.Schema(query=Query) expected_output = """ directive @onConfig on INPUT_OBJECT directive @withConfig(config: Config) on OBJECT - type Query @withConfig { - name: String! - } - input Config @onConfig { value: String! } + + type Query @withConfig { + name: String! + } """ expected_output = textwrap.dedent(expected_output).strip() From 1221d6420ef3831db7a28720fd4107632bfa3853 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sun, 30 Aug 2026 12:59:53 +0200 Subject: [PATCH 17/18] Simplify schema directive collection 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 --- strawberry/schema/directive_collector.py | 150 ++++++++++ strawberry/schema/schema.py | 365 +++++++---------------- strawberry/types/arguments.py | 4 +- strawberry/types/enum.py | 8 + strawberry/types/scalar.py | 2 +- strawberry/types/union.py | 2 +- tests/schema/test_schema_directives.py | 75 +++++ 7 files changed, 345 insertions(+), 261 deletions(-) create mode 100644 strawberry/schema/directive_collector.py diff --git a/strawberry/schema/directive_collector.py b/strawberry/schema/directive_collector.py new file mode 100644 index 0000000000..c536a35647 --- /dev/null +++ b/strawberry/schema/directive_collector.py @@ -0,0 +1,150 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, cast + +from graphql import get_named_type + +from .compat import is_schema_directive +from .schema_converter import GraphQLCoreConverter + +if TYPE_CHECKING: + from collections.abc import Iterable + + from graphql import GraphQLDirective, GraphQLNamedType, GraphQLType + + +class SchemaDirectiveCollector: + """Discovers the schema directives attached to Strawberry definitions. + + graphql-core adds the argument types of every directive passed to + ``GraphQLSchema`` to its type map, but it cannot discover directives that + are stored only on Strawberry definitions. This collector walks the same + GraphQL types graphql-core will reach, records every attached directive, + converts each directive class once, and queues the converted directive's + argument types so directives attached inside nested directive-only input + types are found as well. + """ + + def __init__(self, converter: GraphQLCoreConverter) -> None: + self._converter = converter + + # Directive classes in discovery order. A plain subclass inherits its + # parent's definition, so it is another Python spelling of the same + # GraphQL directive and is recorded as an alias instead. + self.directive_types: list[type] = [] + self.graphql_directives: dict[type, GraphQLDirective] = {} + self._seen_directive_types: set[type] = set() + self._types_by_definition: dict[int, type] = {} + self._aliases: dict[type, type] = {} + + # Every application found on a type, field, argument or enum value, in + # traversal order. Federation derives @link and @composeDirective from + # these, so the order is kept independent of the definitions above. + self.directives_in_use: list[object] = [] + + self._graphql_types: list[GraphQLNamedType] = [] + self._seen_graphql_types: set[int] = set() + self._type_cursor = 0 + self._directive_cursor = 0 + + def add_directive_type(self, directive_type: type) -> None: + if directive_type in self._seen_directive_types: + return + + self._seen_directive_types.add(directive_type) + definition = cast("Any", directive_type).__strawberry_directive__ + canonical_type = self._types_by_definition.setdefault( + id(definition), directive_type + ) + if canonical_type is not directive_type: + self._aliases[directive_type] = canonical_type + return + + self.directive_types.append(directive_type) + + def add_schema_directives(self, directives: Iterable[object]) -> None: + """Register directives applied to the schema definition itself.""" + for directive in directives: + self._add_directive_type_of(directive) + + def add_graphql_types(self, graphql_types: Iterable[GraphQLType | None]) -> None: + for graphql_type in graphql_types: + if graphql_type is not None: + self._queue_graphql_type(graphql_type) + + def collect(self) -> None: + """Process queued types and directives until nothing new is found.""" + while self._type_cursor < len(self._graphql_types) or ( + self._directive_cursor < len(self.directive_types) + ): + while self._type_cursor < len(self._graphql_types): + graphql_type = self._graphql_types[self._type_cursor] + self._type_cursor += 1 + self._visit_graphql_type(graphql_type) + + # Conversion exposes argument input types that graphql-core can only + # contribute once the directive is registered. Queue them so any + # directives attached inside that input graph are discovered too. + while self._directive_cursor < len(self.directive_types): + directive_type = self.directive_types[self._directive_cursor] + self._directive_cursor += 1 + graphql_directive = self._converter.from_schema_directive( + directive_type + ) + self.graphql_directives[directive_type] = graphql_directive + for argument in graphql_directive.args.values(): + self._queue_graphql_type(argument.type) + + for alias, canonical_type in self._aliases.items(): + self.graphql_directives[alias] = self.graphql_directives[canonical_type] + + def _add_directive_type_of(self, directive: object) -> None: + directive_type = directive.__class__ + if is_schema_directive(directive_type): + self.add_directive_type(directive_type) + + def _record_applied_directives(self, owner: object) -> None: + for directive in getattr(owner, "directives", None) or (): + self.directives_in_use.append(directive) + self._add_directive_type_of(directive) + + def _queue_graphql_type(self, graphql_type: GraphQLType) -> None: + named_type = get_named_type(graphql_type) + if id(named_type) in self._seen_graphql_types: + return + + self._seen_graphql_types.add(id(named_type)) + self._graphql_types.append(named_type) + + def _visit_graphql_type(self, graphql_type: GraphQLNamedType) -> None: + # Resolve graphql-core's lazy thunks before reading the Strawberry + # definition. Field extensions can attach schema directives while fields + # are converted (permissions do this), so inspecting the definition + # first would miss them. + graphql_fields = getattr(graphql_type, "fields", {}) + interfaces = getattr(graphql_type, "interfaces", ()) + member_types = getattr(graphql_type, "types", ()) + + definition = graphql_type.extensions.get( + GraphQLCoreConverter.DEFINITION_BACKREF + ) + if definition is not None: + self._record_applied_directives(definition) + for field in getattr(definition, "fields", ()): + self._record_applied_directives(field) + for argument in getattr(field, "arguments", ()): + self._record_applied_directives(argument) + for value in getattr(definition, "values", ()): + self._record_applied_directives(value) + + for field in graphql_fields.values(): + self._queue_graphql_type(field.type) + for argument in getattr(field, "args", {}).values(): + self._queue_graphql_type(argument.type) + for interface in interfaces: + self._queue_graphql_type(interface) + for member_type in member_types: + self._queue_graphql_type(member_type) + + +__all__ = ["SchemaDirectiveCollector"] diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 35af9d2d57..7b7cc37c61 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -3,14 +3,7 @@ import asyncio import warnings from asyncio import ensure_future -from collections.abc import ( - AsyncGenerator, - AsyncIterator, - Awaitable, - Callable, - Iterable, - Iterator, -) +from collections.abc import AsyncGenerator, AsyncIterator, Awaitable, Callable, Iterable from functools import lru_cache from inspect import isawaitable from typing import ( @@ -38,7 +31,6 @@ GraphQLSchema, OperationDefinitionNode, get_introspection_query, - get_named_type, parse, validate_schema, ) @@ -89,6 +81,7 @@ ) from .base import BaseSchema from .config import StrawberryConfig +from .directive_collector import SchemaDirectiveCollector from .exceptions import CannotGetOperationTypeError, InvalidOperationTypeError if TYPE_CHECKING: @@ -137,6 +130,57 @@ _DirectiveEntry: TypeAlias = tuple[GraphQLDirective, object, str] +def _register_graphql_directive( + registry: dict[str, _DirectiveEntry], + directive: GraphQLDirective, + definition: object, + source: str, +) -> None: + """Add ``directive`` to ``registry`` unless its name is already taken. + + Registering the same Strawberry definition twice is harmless; two distinct + definitions sharing a GraphQL name raise an error naming both sources. + """ + if (existing := registry.get(directive.name)) is None: + registry[directive.name] = (directive, definition, source) + return + + _, existing_definition, existing_source = existing + if existing_definition is definition: + return + + raise ValueError( + f"Schema directive '@{directive.name}' is defined by both " + f"{existing_source} and {source}. Use a different " + "GraphQL name for one of them." + ) + + +def _is_specified_one_of_directive( + directive: GraphQLDirective, registry: dict[str, _DirectiveEntry] +) -> bool: + """Whether ``directive`` is a compatible spelling of graphql-core's @oneOf. + + Strawberry's OneOf schema directive (and user-defined ones) predate + graphql-core exposing the specified @oneOf directive. A compatible + definition stands in for the built-in regardless of the Python class used. + Legacy definitions normally have no description; a supplied description is + only accepted when it matches the specified one so custom metadata is not + silently discarded. + """ + if directive.name != "oneOf" or (existing := registry.get("oneOf")) is None: + return False + + specified, definition, _ = existing + return ( + specified is definition + and directive.locations == specified.locations + and directive.args.keys() == specified.args.keys() + and directive.is_repeatable == specified.is_repeatable + and directive.description in (None, specified.description) + ) + + # TODO: merge with below def validate_document( schema: GraphQLSchema, @@ -383,16 +427,11 @@ class Query: else None ) - self._operation_graphql_directives = [ - self.schema_converter.from_directive(directive) - for directive in self.directives - ] - - graphql_types = [] - explicit_schema_directive_types = [] + graphql_types: list[GraphQLNamedType] = [] + explicit_directive_types: list[type] = [] for type_ in types: if compat.is_schema_directive(type_): - explicit_schema_directive_types.append(type_) + explicit_directive_types.append(type_) else: if ( has_object_definition(type_) @@ -406,29 +445,22 @@ class Query: raise TypeError(f"{graphql_type} is not a named GraphQL Type") graphql_types.append(graphql_type) - self._graphql_query_type = query_type - self._graphql_mutation_type = mutation_type - self._graphql_subscription_type = subscription_type - self._graphql_types = graphql_types - self._explicit_schema_directive_types = tuple(explicit_schema_directive_types) - try: # graphql-core adds the argument types of every directive passed to # GraphQLSchema to its type map. It cannot, however, discover # directives stored only in Strawberry definitions. Collect and # convert those directives first so the one GraphQLSchema we build # receives the complete directive and type graph. - self._schema_directive_types = self._collect_schema_directives( - self._explicit_schema_directive_types + schema_directive_types = self._collect_schema_directives( + explicit_directive_types, + [query_type, mutation_type, subscription_type, *graphql_types], ) self._schema = GraphQLSchema( - query=self._graphql_query_type, - mutation=self._graphql_mutation_type, - subscription=self._graphql_subscription_type, - directives=self._collect_graphql_directives( - self._schema_directive_types - ), - types=self._graphql_types, + query=query_type, + mutation=mutation_type, + subscription=subscription_type, + directives=self._collect_graphql_directives(schema_directive_types), + types=graphql_types, extensions={ GraphQLCoreConverter.DEFINITION_BACKREF: self, }, @@ -460,83 +492,60 @@ class Query: formatted_errors = "\n\n".join(f"❌ {error.message}" for error in errors) raise ValueError(f"Invalid Schema. Errors:\n\n{formatted_errors}") - def _add_graphql_directive( + def _collect_schema_directives( self, - directive: GraphQLDirective, - definition: object, - source: str, - ) -> None: - name = directive.name - if (existing := self._graphql_directives.get(name)) is None: - self._graphql_directives[name] = (directive, definition, source) - return - - _, existing_definition, existing_source = existing - if existing_definition is definition: - return + explicit_directive_types: Iterable[type], + graphql_types: Iterable[GraphQLNamedType | None], + ) -> list[type]: + collector = SchemaDirectiveCollector(self.schema_converter) + for directive_type in explicit_directive_types: + collector.add_directive_type(directive_type) + collector.add_schema_directives(self.schema_directives) + collector.add_graphql_types(graphql_types) + collector.collect() + + # Federation reads the applications found so far to generate @link and + # @composeDirective, appending them to ``schema_directives``. Fold what + # it added into the same collection so the single GraphQLSchema built + # from it is validated with every generated directive and support type. + self._schema_directives_in_use = collector.directives_in_use + prepared_count = len(self.schema_directives) + self._prepare_schema_directives() + collector.add_schema_directives(self.schema_directives[prepared_count:]) + collector.collect() - raise ValueError( - f"Schema directive '@{name}' is defined by both " - f"{existing_source} and {source}. Use a different " - "GraphQL name for one of them." - ) + self._schema_graphql_directives = collector.graphql_directives + return collector.directive_types def _collect_graphql_directives( self, schema_directive_types: Iterable[type] ) -> tuple[GraphQLDirective, ...]: # GraphQLSchema expects its directive collection to be canonical. Seed - # this registry with the specified directives so a custom definition + # the registry with the specified directives so a custom definition # cannot silently replace a built-in, then keep one insertion-ordered - # entry per GraphQL name. Reusing the same Strawberry definition is - # harmless while distinct definitions produce an error that identifies - # both Python sources. - self._graphql_directives: dict[str, _DirectiveEntry] = { - directive.name: ( - directive, - directive, - "the built-in GraphQL directive", - ) + # entry per GraphQL name. + registry: dict[str, _DirectiveEntry] = { + directive.name: (directive, directive, "the built-in GraphQL directive") for directive in specified_directives } - for directive in self._operation_graphql_directives: - strawberry_directive = directive.extensions[ - GraphQLCoreConverter.DEFINITION_BACKREF - ] - self._add_graphql_directive( + for directive in self.directives: + _register_graphql_directive( + registry, + self.schema_converter.from_directive(directive), directive, - strawberry_directive, - f"operation directive '{strawberry_directive.python_name}'", + f"operation directive '{directive.python_name}'", ) for directive_type in schema_directive_types: graphql_directive = self._schema_graphql_directives[directive_type] - strawberry_directive = cast("Any", directive_type).__strawberry_directive__ - directive_name = self.config.name_converter.from_directive( - strawberry_directive - ) - - # Strawberry's OneOf schema directive predates graphql-core exposing - # the specified @oneOf directive. Treat compatible definitions as the - # canonical runtime directive regardless of the Python class used. - # Legacy definitions normally have no description; only accept a - # supplied description when it exactly matches the specified one so - # custom metadata is not silently discarded. - if ( - directive_name == "oneOf" - and (existing := self._graphql_directives.get(directive_name)) - is not None - and existing[0] is existing[1] - and graphql_directive.locations == existing[0].locations - and graphql_directive.args.keys() == existing[0].args.keys() - and graphql_directive.is_repeatable == existing[0].is_repeatable - and graphql_directive.description in (None, existing[0].description) - ): + if _is_specified_one_of_directive(graphql_directive, registry): continue - self._add_graphql_directive( + _register_graphql_directive( + registry, graphql_directive, - strawberry_directive, + cast("Any", directive_type).__strawberry_directive__, ( "schema directive " f"'{directive_type.__module__}.{directive_type.__qualname__}'" @@ -545,177 +554,14 @@ def _collect_graphql_directives( if self.config.enable_experimental_incremental_execution: for directive in incremental_execution_directives: - self._add_graphql_directive( + _register_graphql_directive( + registry, directive, directive, f"the experimental GraphQL directive '@{directive.name}'", ) - return tuple(directive for directive, _, _ in self._graphql_directives.values()) - - def _collect_schema_directives( - self, explicit_schema_directive_types: Iterable[type] - ) -> tuple[type, ...]: - # graphql-core can walk fields and directive argument types once it has a - # GraphQLSchema, but attached Strawberry directives are not edges in that - # graph. Walk the same reachable GraphQL types before constructing the - # schema, and add each converted directive's argument graph to the queue. - # This finds directives on nested directive-only input types without - # repeatedly rescanning the converter's complete type map. - schema_directive_types: list[type] = [] - seen_directive_types: set[type] = set() - directive_types_by_definition: dict[int, type] = {} - directive_type_aliases: dict[type, type] = {} - self._schema_graphql_directives: dict[type, GraphQLDirective] = {} - self._schema_directives_in_use: list[object] = [] - - def add_directive_type(directive_type: type) -> None: - if directive_type in seen_directive_types: - return - - seen_directive_types.add(directive_type) - strawberry_directive = cast("Any", directive_type).__strawberry_directive__ - - # A plain subclass inherits its parent's Strawberry directive - # definition. It is another Python spelling for the same GraphQL - # directive, not a conflicting definition that needs conversion. - definition_id = id(strawberry_directive) - if ( - canonical_type := directive_types_by_definition.get(definition_id) - ) is not None: - directive_type_aliases[directive_type] = canonical_type - return - - directive_types_by_definition[definition_id] = directive_type - schema_directive_types.append(directive_type) - - for directive_type in explicit_schema_directive_types: - add_directive_type(directive_type) - - def add_directive(directive: object, *, track_application: bool) -> None: - if track_application: - # Federation derives @link and @composeDirective applications - # from uses, so keep their order independently from the - # deduplicated definition collection. - self._schema_directives_in_use.append(directive) - - directive_type = directive.__class__ - if compat.is_schema_directive(directive_type): - add_directive_type(directive_type) - - def add_directives(owner: object, *, track_application: bool) -> None: - attached_directives = getattr(owner, "directives", ()) or () - if isinstance(attached_directives, Iterator): - # Strawberry accepts arbitrary iterables for directives. Later - # consumers such as Federation and the SDL printer need to see - # the same applications, so preserve one-shot iterators here. - attached_directives = list(attached_directives) - owner.directives = attached_directives # type: ignore[attr-defined] - - for directive in attached_directives: - add_directive(directive, track_application=track_application) - - for directive in self.schema_directives: - add_directive(directive, track_application=False) - - graphql_types: list[GraphQLNamedType] = [] - seen_graphql_types: set[int] = set() - - def add_graphql_type(graphql_type: Any) -> None: - if graphql_type is None: - return - - named_type = get_named_type(graphql_type) - if id(named_type) in seen_graphql_types: - return - - seen_graphql_types.add(id(named_type)) - graphql_types.append(named_type) - - add_graphql_type(self._graphql_query_type) - add_graphql_type(self._graphql_mutation_type) - add_graphql_type(self._graphql_subscription_type) - for graphql_type in self._graphql_types: - add_graphql_type(graphql_type) - - graphql_type_index = 0 - directive_index = 0 - - def collect_new_definitions() -> None: - nonlocal directive_index, graphql_type_index - - while graphql_type_index < len(graphql_types) or directive_index < len( - schema_directive_types - ): - while graphql_type_index < len(graphql_types): - graphql_type = graphql_types[graphql_type_index] - graphql_type_index += 1 - - # Resolve graphql-core's lazy thunks before reading the - # Strawberry definition. Field extensions can attach schema - # directives while fields are converted (permissions do - # this), so inspecting the definition first would miss them. - graphql_fields = getattr(graphql_type, "fields", {}) - interfaces = getattr(graphql_type, "interfaces", ()) - member_types = getattr(graphql_type, "types", ()) - - definition = graphql_type.extensions.get( - GraphQLCoreConverter.DEFINITION_BACKREF - ) - if definition is not None: - add_directives(definition, track_application=True) - for field in getattr(definition, "fields", ()): - add_directives(field, track_application=True) - for argument in getattr(field, "arguments", ()): - add_directives(argument, track_application=True) - for value in getattr(definition, "values", ()): - add_directives(value, track_application=True) - - # Queue the resolved edges explicitly so definitions reached - # through regular fields and directive arguments use one - # traversal and one source of attachment metadata. - for field in graphql_fields.values(): - add_graphql_type(field.type) - for argument in getattr(field, "args", {}).values(): - add_graphql_type(argument.type) - for interface in interfaces: - add_graphql_type(interface) - for member_type in member_types: - add_graphql_type(member_type) - - # Conversion exposes argument input types that graphql-core - # cannot contribute until the directive is registered. Queue - # those types, then continue the same traversal to discover any - # directives attached inside their nested input graph. - while directive_index < len(schema_directive_types): - directive_type = schema_directive_types[directive_index] - graphql_directive = self.schema_converter.from_schema_directive( - directive_type - ) - self._schema_graphql_directives[directive_type] = graphql_directive - directive_index += 1 - for argument in graphql_directive.args.values(): - add_graphql_type(argument.type) - - collect_new_definitions() - schema_directive_count = len(self.schema_directives) - self._prepare_schema_directives() - - if len(self.schema_directives) > schema_directive_count: - # Federation prepares generated @link and @composeDirective - # applications here. Only process the new tail; a plain Schema does - # no second traversal, and the final GraphQLSchema still receives - # every generated directive and support type before validation. - for directive in self.schema_directives[schema_directive_count:]: - add_directive(directive, track_application=False) - collect_new_definitions() - - for alias, canonical_type in directive_type_aliases.items(): - self._schema_graphql_directives[alias] = self._schema_graphql_directives[ - canonical_type - ] - - return tuple(schema_directive_types) + return tuple(directive for directive, _, _ in registry.values()) def get_extensions(self, sync: bool = False) -> list[SchemaExtension]: # Deprecated instances are passed through as-is. The DeprecationWarning @@ -1480,7 +1326,12 @@ def _resolve_node_ids(self) -> None: origin.resolve_id_attr() def _prepare_schema_directives(self) -> None: - pass + """Hook for subclasses to append generated ``schema_directives``. + + Runs after the attached directives have been collected (so + ``_schema_directives_in_use`` is populated) and before the GraphQLSchema + is built, so anything appended here is part of the served schema. + """ def _warn_for_federation_directives(self) -> None: """Raises a warning if the schema has any federation directives.""" diff --git a/strawberry/types/arguments.py b/strawberry/types/arguments.py index aaa3d7755f..f6ed6951e0 100644 --- a/strawberry/types/arguments.py +++ b/strawberry/types/arguments.py @@ -76,7 +76,7 @@ def __init__( # noqa: PLR0917 self.description = description self.type_annotation = type_annotation self.deprecation_reason = deprecation_reason - self.directives = directives + self.directives = tuple(directives) self.metadata = metadata or {} # TODO: Consider moving this logic to a function @@ -108,7 +108,7 @@ def __init__( # noqa: PLR0917 self.description = arg.description self.graphql_name = arg.name self.deprecation_reason = arg.deprecation_reason - self.directives = arg.directives + self.directives = tuple(arg.directives) self.metadata = arg.metadata if arg.graphql_type is not None: self.type_annotation = StrawberryAnnotation( diff --git a/strawberry/types/enum.py b/strawberry/types/enum.py index f591e2e7eb..875aae90d9 100644 --- a/strawberry/types/enum.py +++ b/strawberry/types/enum.py @@ -19,6 +19,11 @@ class EnumValue: directives: Iterable[object] = () description: str | None = None + def __post_init__(self) -> None: + # Directives are read by several consumers (schema construction, + # federation, the SDL printer), so one-shot iterables are materialized. + self.directives = tuple(self.directives) + @dataclasses.dataclass class StrawberryEnumDefinition(StrawberryType): @@ -29,6 +34,9 @@ class StrawberryEnumDefinition(StrawberryType): directives: Iterable[object] = () print_definition: bool = True + def __post_init__(self) -> None: + self.directives = tuple(self.directives) + def __hash__(self) -> int: # TODO: Is this enough for unique-ness? return hash(self.name) diff --git a/strawberry/types/scalar.py b/strawberry/types/scalar.py index d66d884a7f..fccd96cc0e 100644 --- a/strawberry/types/scalar.py +++ b/strawberry/types/scalar.py @@ -116,7 +116,7 @@ def _process_scalar( serialize=serialize, parse_literal=parse_literal, parse_value=parse_value, - directives=directives, + directives=tuple(directives), print_definition=print_definition, origin=cls, # type: ignore[arg-type] _source_file=_source_file, diff --git a/strawberry/types/union.py b/strawberry/types/union.py index c37cd54da6..d9e5d04989 100644 --- a/strawberry/types/union.py +++ b/strawberry/types/union.py @@ -57,7 +57,7 @@ def __init__( self.graphql_name = name self.type_annotations = type_annotations self.description = description - self.directives = directives + self.directives = tuple(directives) self._source_file = None self._source_line = None self.concrete_of: StrawberryUnion | None = None diff --git a/tests/schema/test_schema_directives.py b/tests/schema/test_schema_directives.py index f80b0c7360..60ce8ef1c2 100644 --- a/tests/schema/test_schema_directives.py +++ b/tests/schema/test_schema_directives.py @@ -413,6 +413,81 @@ def search( } <= directive_names +def test_directives_passed_as_one_shot_iterables_are_kept(): + @strawberry.schema_directive( + locations=[ + Location.ARGUMENT_DEFINITION, + Location.ENUM, + Location.ENUM_VALUE, + Location.SCALAR, + Location.UNION, + ] + ) + class Marker: ... + + def markers(): + yield Marker() + + @strawberry.enum(directives=markers()) + class Choice(Enum): + FIRST = strawberry.enum_value("first", directives=markers()) + + CustomScalar = strawberry.scalar(str, name="CustomScalar", directives=markers()) + + @strawberry.type + class Item: + name: str + + @strawberry.type + class Other: + value: str + + Result = Annotated[Item | Other, strawberry.union("Result", directives=markers())] + + @strawberry.type + class Query: + result: Result + custom_scalar: CustomScalar + + @strawberry.field + def choice( + self, + choice: Annotated[Choice, strawberry.argument(directives=markers())], + ) -> Choice: + return choice + + schema = strawberry.Schema(query=Query) + + expected = """ + directive @marker on ARGUMENT_DEFINITION | ENUM | ENUM_VALUE | SCALAR | UNION + + enum Choice @marker { + FIRST @marker + } + + scalar CustomScalar @marker + + type Item { + name: String! + } + + type Other { + value: String! + } + + type Query { + result: Result! + customScalar: CustomScalar! + choice(choice: Choice! @marker): Choice! + } + + union Result @marker = Item | Other + """ + + assert schema.as_str() == textwrap.dedent(expected).strip() + assert schema._schema.get_directive("marker") is not None + + def test_rejects_conflicting_schema_directive_names(): @strawberry.schema_directive(name="conflict", locations=[Location.OBJECT]) class First: ... From c0b9fbc2c819069560e5289f512cb0ef6744e8df Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Mon, 31 Aug 2026 19:35:22 +0200 Subject: [PATCH 18/18] Address Thiago's feedback --- docs/types/enums.md | 18 --------------- docs/types/scalars.md | 20 ----------------- docs/types/schema-directives.md | 4 ---- strawberry/printer/printer.py | 14 +++++------- strawberry/types/enum.py | 3 +-- strawberry/types/scalar.py | 3 +-- tests/test_printer/test_defer_stream.py | 29 +++++++++++++++++++++---- 7 files changed, 32 insertions(+), 59 deletions(-) diff --git a/docs/types/enums.md b/docs/types/enums.md index f553597ceb..6a85a58407 100644 --- a/docs/types/enums.md +++ b/docs/types/enums.md @@ -187,21 +187,3 @@ When querying, the custom name will be used in the response: Note that the Python enum member name (`CHOCOLATE_COOKIE`) is still used in your Python code, while the custom name (`chocolateCookie`) is used in the GraphQL schema and responses. - -## Hiding integration support types from SDL - -Integration authors can pass `print_definition=False` to `strawberry.enum` for -an enum that should be available to runtime introspection without normally -appearing in Strawberry's generated SDL: - -```python -@strawberry.enum(print_definition=False) -class InternalPurpose(Enum): - SECURITY = "security" - EXECUTION = "execution" -``` - -If a regular field, argument, or printed directive definition uses the enum, -Strawberry prints its definition to keep the generated SDL valid. This option is -primarily intended for types used only by hidden schema directives and framework -integrations. diff --git a/docs/types/scalars.md b/docs/types/scalars.md index d32bb25090..6cc80cb7db 100644 --- a/docs/types/scalars.md +++ b/docs/types/scalars.md @@ -170,26 +170,6 @@ from strawberry.scalars import Base16, Base32, Base64 -### Hiding integration support types from SDL - -Integration authors can pass `print_definition=False` to `strawberry.scalar` for -a support type that should be available to runtime introspection without -normally appearing in Strawberry's generated SDL: - -```python -InternalIDScalar = strawberry.scalar( - name="InternalID", - serialize=str, - parse_value=str, - print_definition=False, -) -``` - -If a regular field, argument, or printed directive definition uses the scalar, -Strawberry prints its definition to keep the generated SDL valid. This option is -primarily intended for types used only by hidden schema directives and framework -integrations. - ## Example: Custom Object Scalar Suppose we would like to use a Pillow `Image` as a scalar that serializes diff --git a/docs/types/schema-directives.md b/docs/types/schema-directives.md index 1f87847ae1..3aeda2cec9 100644 --- a/docs/types/schema-directives.md +++ b/docs/types/schema-directives.md @@ -82,10 +82,6 @@ same way as field and argument annotations elsewhere in the schema. Types imported only under `TYPE_CHECKING` should use [`strawberry.lazy`](/docs/types/lazy) so Strawberry can resolve them at runtime. -Setting `print_definition=False` on `@strawberry.schema_directive` keeps its -definition out of Strawberry's generated SDL, but the directive and its argument -types stay available to runtime introspection. - ## Overriding field names You can use `strawberry.directive_field` to override the name of a field: diff --git a/strawberry/printer/printer.py b/strawberry/printer/printer.py index c12627571d..32facafe7c 100644 --- a/strawberry/printer/printer.py +++ b/strawberry/printer/printer.py @@ -34,6 +34,7 @@ print_implemented_interfaces, print_specified_by_url, ) +from graphql.utilities.print_schema import print_directive as original_print_directive from graphql.utilities.print_schema import print_type as original_print_type from strawberry.schema_directive import Location, StrawberrySchemaDirective @@ -580,7 +581,10 @@ def print_schema_definition(schema: BaseSchema, *, extras: PrintExtras) -> str | def print_directive(directive: GraphQLDirective, *, schema: BaseSchema) -> str | None: strawberry_directive = directive.extensions.get("strawberry-definition") - if strawberry_directive is None or ( + if strawberry_directive is None: + return original_print_directive(directive) + + if ( isinstance(strawberry_directive, StrawberrySchemaDirective) and not strawberry_directive.print_definition ): @@ -759,14 +763,6 @@ def _print_extra_types() -> Iterable[str]: and printed_directive not in extras.directives ] - 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" - ) - return "\n\n".join( chain( sorted(extras.directives), diff --git a/strawberry/types/enum.py b/strawberry/types/enum.py index 875aae90d9..00a06d88e6 100644 --- a/strawberry/types/enum.py +++ b/strawberry/types/enum.py @@ -223,7 +223,7 @@ def enum( ) -> Callable[[EnumType], EnumType]: ... -def enum( +def enum( # noqa: D417 cls: EnumType | None = None, *, name: str | None = None, @@ -244,7 +244,6 @@ def enum( description: The description of the GraphQL enum. directives: The directives to attach to the GraphQL enum. graphql_name_from: Whether to use the names (key) or values of the Python enums in GraphQL. - print_definition: Whether to include the enum definition in generated SDL. Returns: The decorated Enum class. diff --git a/strawberry/types/scalar.py b/strawberry/types/scalar.py index fccd96cc0e..6da773d73d 100644 --- a/strawberry/types/scalar.py +++ b/strawberry/types/scalar.py @@ -174,7 +174,7 @@ def scalar( # TODO: We are tricking pyright into thinking that we are returning the given type # here or else it won't let us use any custom scalar to annotate attributes in # dataclasses/types. This should be properly solved when implementing StrawberryScalar -def scalar( +def scalar( # noqa: D417 cls: _T | None = None, *, name: str | None = None, @@ -211,7 +211,6 @@ def scalar( parse_value: The function to parse the value. parse_literal: The function to parse the literal. directives: The directives to apply to the scalar. - print_definition: Whether to include the scalar definition in generated SDL. Returns: A `ScalarDefinition` when called with `name` only, a decorator function diff --git a/tests/test_printer/test_defer_stream.py b/tests/test_printer/test_defer_stream.py index 41022799eb..089b975751 100644 --- a/tests/test_printer/test_defer_stream.py +++ b/tests/test_printer/test_defer_stream.py @@ -35,14 +35,35 @@ def test_prints_defer_and_stream_directives_when_experimental_execution_is_enabl config=StrawberryConfig(enable_experimental_incremental_execution=True), ) - expected_type = """ - directive @defer(if: Boolean, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT + expected_type = ''' + """ + Directs the executor to defer this fragment when the `if` argument is true or undefined. + """ + directive @defer( + """Deferred when true or undefined.""" + if: Boolean! = true - directive @stream(if: Boolean, label: String, initialCount: Int = 0) on FIELD + """Unique name""" + label: String + ) on FRAGMENT_SPREAD | INLINE_FRAGMENT + + """ + Directs the executor to stream plural fields when the `if` argument is true or undefined. + """ + directive @stream( + """Stream when true or undefined.""" + if: Boolean! = true + + """Unique name""" + label: String + + """Number of items to return immediately""" + initialCount: Int = 0 + ) on FIELD type Query { hello: String! } - """ + ''' assert str(schema) == textwrap.dedent(expected_type).strip()