From 80df22ebb30eb621bc37135bbf9b38feefc085e2 Mon Sep 17 00:00:00 2001 From: Arthur Bayr Date: Thu, 17 Oct 2024 16:44:26 +0200 Subject: [PATCH 01/12] https://github.com/strawberry-graphql/strawberry/issues/3596 --- strawberry/permission.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/strawberry/permission.py b/strawberry/permission.py index b52fdce102..47a4c0a94f 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -158,9 +158,12 @@ def __init__( def apply(self, field: StrawberryField) -> None: """Applies all of the permission directives 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 - ) + # Dedupe multiple directives + # https://github.com/strawberry-graphql/strawberry/issues/3596 + permission_directives = {p.schema_directive for p in self.permissions if p.schema_directive} + existing_field_directives = set(field.directives) + extend_directives = permission_directives - existing_field_directives + field.directives.extend(extend_directives) # We can only fail silently if the field is optional or a list if self.fail_silently: if isinstance(field.type, StrawberryOptional): From a736a61081f20ceccf71fb595efe12f2376d4ebc Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 17 Oct 2024 14:54:35 +0000 Subject: [PATCH 02/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- strawberry/permission.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/strawberry/permission.py b/strawberry/permission.py index 47a4c0a94f..ba76b0c8c4 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -160,7 +160,9 @@ def apply(self, field: StrawberryField) -> None: if self.use_directives: # Dedupe multiple directives # https://github.com/strawberry-graphql/strawberry/issues/3596 - permission_directives = {p.schema_directive for p in self.permissions if p.schema_directive} + permission_directives = { + p.schema_directive for p in self.permissions if p.schema_directive + } existing_field_directives = set(field.directives) extend_directives = permission_directives - existing_field_directives field.directives.extend(extend_directives) From 9b4e9c9c9de6121cb18edaf9c831793b176e5e9b Mon Sep 17 00:00:00 2001 From: Arthur Bayr Date: Mon, 21 Oct 2024 08:44:49 +0200 Subject: [PATCH 03/12] keep order --- strawberry/permission.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/strawberry/permission.py b/strawberry/permission.py index ba76b0c8c4..dce1d6f57a 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -156,16 +156,15 @@ 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: - # Dedupe multiple directives - # https://github.com/strawberry-graphql/strawberry/issues/3596 - permission_directives = { - p.schema_directive for p in self.permissions if p.schema_directive - } - existing_field_directives = set(field.directives) - extend_directives = permission_directives - existing_field_directives - field.directives.extend(extend_directives) + 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): From e89afd18a2fc0019b9b218490309fa5815a9ed29 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 21 Oct 2024 06:46:20 +0000 Subject: [PATCH 04/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- strawberry/permission.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/strawberry/permission.py b/strawberry/permission.py index dce1d6f57a..7c001f1d28 100644 --- a/strawberry/permission.py +++ b/strawberry/permission.py @@ -158,7 +158,11 @@ def __init__( def apply(self, field: StrawberryField) -> None: """Applies all of the permission directives (deduped) to the schema and sets up silent permissions.""" if self.use_directives: - permission_directives = [perm.schema_directive for perm in self.permissions if perm.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 From d77bc51d9a8ac6fff230ff0dfb47262e2b1f1c93 Mon Sep 17 00:00:00 2001 From: Arthur Bayr Date: Mon, 21 Oct 2024 09:15:55 +0200 Subject: [PATCH 05/12] added a test --- tests/test_printer/test_schema_directives.py | 61 ++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 2b3be70ad3..1a87577367 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -4,6 +4,8 @@ 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 @@ -531,6 +533,65 @@ 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) -> bool: + return True + + @strawberry.interface + class MyInterface: + id: strawberry.ID + + @strawberry.field(extensions=[PermissionExtension(permissions=[MemberRoleRequired()])]) + def hello(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(info: Info) -> MyInterface: + return MyType1(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() + def test_print_directive_on_union(): @strawberry.type From be4faa03852474193a8a03c874c28e88080eea34 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 21 Oct 2024 07:16:11 +0000 Subject: [PATCH 06/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_printer/test_schema_directives.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 1a87577367..f25f9a6260 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -533,6 +533,7 @@ class Query: assert print_schema(schema) == textwrap.dedent(expected_output).strip() + def test_dedupe_multiple_equal_directives(): class MemberRoleRequired(BasePermission): message = "Keine Rechte" @@ -544,9 +545,11 @@ def has_permission(self, source, info: Info, **kwargs) -> bool: class MyInterface: id: strawberry.ID - @strawberry.field(extensions=[PermissionExtension(permissions=[MemberRoleRequired()])]) + @strawberry.field( + extensions=[PermissionExtension(permissions=[MemberRoleRequired()])] + ) def hello(info: Info) -> str: - return 'world' + return "world" @strawberry.type class MyType1(MyInterface): @@ -558,10 +561,9 @@ class MyType2(MyInterface): @strawberry.type class Query: - @strawberry.field def my_type(info: Info) -> MyInterface: - return MyType1(id='1', name='Hello') + return MyType1(id="1", name="Hello") expected_output = """ directive @memberRoleRequired on FIELD_DEFINITION From 315f2a13958cdfcef9064a77e56613356d3717a8 Mon Sep 17 00:00:00 2001 From: Arthur Bayr Date: Mon, 21 Oct 2024 12:46:07 +0200 Subject: [PATCH 07/12] fix untyped **kwargs --- tests/test_printer/test_schema_directives.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index f25f9a6260..17cfce6f16 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -538,7 +538,7 @@ def test_dedupe_multiple_equal_directives(): class MemberRoleRequired(BasePermission): message = "Keine Rechte" - def has_permission(self, source, info: Info, **kwargs) -> bool: + def has_permission(self, source, info: Info) -> bool: return True @strawberry.interface From 96a2ae5279d888fb71b6d327894afb620d230983 Mon Sep 17 00:00:00 2001 From: Arthur Bayr Date: Mon, 21 Oct 2024 12:49:26 +0200 Subject: [PATCH 08/12] keep 'Any' from inheritance --- tests/test_printer/test_schema_directives.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 17cfce6f16..39f1c3067f 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -1,6 +1,6 @@ import textwrap from enum import Enum -from typing import List, Optional, Union +from typing import List, Optional, Union, Any from typing_extensions import Annotated import strawberry @@ -538,7 +538,7 @@ def test_dedupe_multiple_equal_directives(): class MemberRoleRequired(BasePermission): message = "Keine Rechte" - def has_permission(self, source, info: Info) -> bool: + def has_permission(self, source, info: Info, **kwargs: Any) -> bool: return True @strawberry.interface From 35015d66aabc454a187dcd1094c286ab789713a4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 21 Oct 2024 10:49:43 +0000 Subject: [PATCH 09/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_printer/test_schema_directives.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index 39f1c3067f..f7153241e0 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -1,6 +1,6 @@ import textwrap from enum import Enum -from typing import List, Optional, Union, Any +from typing import Any, List, Optional, Union from typing_extensions import Annotated import strawberry From 8a820090c4f9848fd786f5f05c70fd70b39618d0 Mon Sep 17 00:00:00 2001 From: Arthur Bayr Date: Mon, 21 Oct 2024 12:53:51 +0200 Subject: [PATCH 10/12] add a RELEASE.md --- RELEASE.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000000..78c48cc9d2 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,4 @@ +Release type: patch + +This pull request addresses a bug where directives were being added multiple times, causing VSCode errors. +The fix involves deduplicating directives when applying extensions/permissions to a field, ensuring that each directive is only added once. From 0c83f474c49de1076ec05835123a641d01d5210c Mon Sep 17 00:00:00 2001 From: Thiago Bellini Ribeiro Date: Mon, 21 Oct 2024 10:45:12 -0300 Subject: [PATCH 11/12] Update RELEASE.md --- RELEASE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index 78c48cc9d2..caa51f3d22 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,4 +1,5 @@ Release type: patch -This pull request addresses a bug where directives were being added multiple times, causing VSCode errors. +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. From c7389863dbccd806247d397e15a764962bad60bc Mon Sep 17 00:00:00 2001 From: Thiago Bellini Ribeiro Date: Mon, 21 Oct 2024 10:55:26 -0300 Subject: [PATCH 12/12] Execute the schema to increase coverage --- tests/test_printer/test_schema_directives.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_printer/test_schema_directives.py b/tests/test_printer/test_schema_directives.py index f7153241e0..ac0fb2c6a3 100644 --- a/tests/test_printer/test_schema_directives.py +++ b/tests/test_printer/test_schema_directives.py @@ -548,7 +548,7 @@ class MyInterface: @strawberry.field( extensions=[PermissionExtension(permissions=[MemberRoleRequired()])] ) - def hello(info: Info) -> str: + def hello(self, info: Info) -> str: return "world" @strawberry.type @@ -562,8 +562,8 @@ class MyType2(MyInterface): @strawberry.type class Query: @strawberry.field - def my_type(info: Info) -> MyInterface: - return MyType1(id="1", name="Hello") + def my_type(self, info: Info) -> MyInterface: + return MyType1(id=strawberry.ID("1"), name="Hello") expected_output = """ directive @memberRoleRequired on FIELD_DEFINITION @@ -594,6 +594,10 @@ def my_type(info: Info) -> MyInterface: 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