From 9947fee7a548fd191a1bd1410315ac475a0f392f Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 21 Oct 2024 16:09:28 +0200 Subject: [PATCH] Fix interfaces with duplicate directives (#3674) Co-authored-by: Arthur Bayr Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Thiago Bellini Ribeiro Co-authored-by: Thiago Bellini Ribeiro --- RELEASE.md | 5 ++ strawberry/permission.py | 16 +++-- tests/test_printer/test_schema_directives.py | 69 +++++++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000000..caa51f3d22 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,5 @@ +Release type: patch + +This release addresses a bug where directives were being added multiple times when defined in an interface which multiple objects inherits from. + +The fix involves deduplicating directives when applying extensions/permissions to a field, ensuring that each directive is only added once. diff --git a/strawberry/permission.py b/strawberry/permission.py index b52fdce102..7c001f1d28 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -156,11 +156,19 @@ def __init__( self.use_directives = use_directives def apply(self, field: StrawberryField) -> None: - """Applies all of the permission directives to the schema and sets up silent permissions.""" + """Applies all of the permission directives (deduped) to the schema and sets up silent permissions.""" if self.use_directives: - field.directives.extend( - p.schema_directive for p in self.permissions if p.schema_directive - ) + permission_directives = [ + perm.schema_directive + for perm in self.permissions + if perm.schema_directive + ] + # Iteration, because we want to keep order + for perm_directive in permission_directives: + # Dedupe multiple directives + if perm_directive in field.directives: + continue + field.directives.append(perm_directive) # We can only fail silently if the field is optional or a list if self.fail_silently: if isinstance(field.type, StrawberryOptional): diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 2b3be70ad3..ac0fb2c6a3 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -1,9 +1,11 @@ import textwrap from enum import Enum -from typing import List, Optional, Union +from typing import Any, List, Optional, Union from typing_extensions import Annotated import strawberry +from strawberry import BasePermission, Info +from strawberry.permission import PermissionExtension from strawberry.printer import print_schema from strawberry.schema.config import StrawberryConfig from strawberry.schema_directive import Location @@ -532,6 +534,71 @@ class Query: assert print_schema(schema) == textwrap.dedent(expected_output).strip() +def test_dedupe_multiple_equal_directives(): + class MemberRoleRequired(BasePermission): + message = "Keine Rechte" + + def has_permission(self, source, info: Info, **kwargs: Any) -> bool: + return True + + @strawberry.interface + class MyInterface: + id: strawberry.ID + + @strawberry.field( + extensions=[PermissionExtension(permissions=[MemberRoleRequired()])] + ) + def hello(self, info: Info) -> str: + return "world" + + @strawberry.type + class MyType1(MyInterface): + name: str + + @strawberry.type + class MyType2(MyInterface): + age: int + + @strawberry.type + class Query: + @strawberry.field + def my_type(self, info: Info) -> MyInterface: + return MyType1(id=strawberry.ID("1"), name="Hello") + + expected_output = """ + directive @memberRoleRequired on FIELD_DEFINITION + + interface MyInterface { + id: ID! + hello: String! @memberRoleRequired + } + + type MyType1 implements MyInterface { + id: ID! + hello: String! @memberRoleRequired + name: String! + } + + type MyType2 implements MyInterface { + id: ID! + hello: String! @memberRoleRequired + age: Int! + } + + type Query { + myType: MyInterface! + } + """ + + schema = strawberry.Schema(Query, types=[MyType1, MyType2]) + + assert print_schema(schema) == textwrap.dedent(expected_output).strip() + + retval = schema.execute_sync("{ myType { id hello } }") + assert retval.errors is None + assert retval.data == {"myType": {"id": "1", "hello": "world"}} + + def test_print_directive_on_union(): @strawberry.type class A: