From 0a85dace16b3f0b84d6e33fef5ed0ce7fb4b80e0 Mon Sep 17 00:00:00 2001 From: AmirW Date: Sun, 14 May 2023 18:39:30 +0330 Subject: [PATCH 1/4] get_field_type's job became more clear --- grapple/actions.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/grapple/actions.py b/grapple/actions.py index 6a0a39d9..011514ef 100644 --- a/grapple/actions.py +++ b/grapple/actions.py @@ -1,7 +1,7 @@ import inspect from collections.abc import Iterable from types import MethodType -from typing import Any, Dict, Type, Union +from typing import Any, Dict, Type, Union, Callable import graphene from django.apps import apps @@ -19,6 +19,7 @@ from wagtail.snippets.models import get_snippet_models from .helpers import field_middlewares, streamfield_types +from .models import GraphQLField from .registry import registry from .settings import grapple_settings from .types.documents import DocumentObjectType @@ -154,7 +155,20 @@ def get_fields_and_properties(cls): return fields + properties -def get_field_type(field): +ComplicatedField = Any # QuerySetList, TagList, ... + + +def get_field_type( + field: Union[ + GraphQLField, + tuple[GraphQLField, ComplicatedField], + Callable[[], GraphQLField], + tuple[Callable[[], GraphQLField], ComplicatedField], + tuple[Callable[[], tuple[GraphQLField, ComplicatedField]]], + ] +) -> tuple[GraphQLField, Any]: + if callable(field): + field = field() # If a tuple is returned then obj[1] wraps obj[0] field_wrapper = None if isinstance(field, tuple): @@ -317,9 +331,6 @@ class Meta: methods = {} if hasattr(cls, "graphql_fields"): for field in cls.graphql_fields: - if callable(field): - field = field() - # Add field to GQL type with correct field-type field, field_type = get_field_type(field) type_meta[field.field_name] = field_type @@ -460,9 +471,6 @@ class Meta: # Add any custom fields to node if they are defined. if hasattr(cls, "graphql_fields"): for item in cls.graphql_fields: - if callable(item): - item = item() - # Get correct types from field field, field_type = get_field_type(item) From 11a6fb10b25ce6cf53d51922f302bbf6a20f231d Mon Sep 17 00:00:00 2001 From: AmirW Date: Mon, 15 May 2023 16:13:49 +0330 Subject: [PATCH 2/4] if a field does not appear in graphql_fields then it should not be queryable -if adding it to Meta.exclude works then do it, if not then return a dummy data --- grapple/actions.py | 72 +++++++++++++------ grapple/models.py | 7 ++ grapple/utils.py | 11 +++ tests/test_grapple.py | 48 ++++++++++++- .../migrations/0003_authorpage_nickname.py | 17 +++++ tests/testapp/models/core.py | 9 ++- 6 files changed, 140 insertions(+), 24 deletions(-) create mode 100644 tests/testapp/migrations/0003_authorpage_nickname.py diff --git a/grapple/actions.py b/grapple/actions.py index 011514ef..1336dc3b 100644 --- a/grapple/actions.py +++ b/grapple/actions.py @@ -19,7 +19,7 @@ from wagtail.snippets.models import get_snippet_models from .helpers import field_middlewares, streamfield_types -from .models import GraphQLField +from .models import GraphQLField, DefaultField from .registry import registry from .settings import grapple_settings from .types.documents import DocumentObjectType @@ -27,6 +27,7 @@ from .types.pages import Page, PageInterface from .types.rich_text import RichText as RichTextType from .types.streamfield import generate_streamfield_union +from .utils import resolve_not_exposed_exception if apps.is_installed("wagtailmedia"): from wagtailmedia.models import AbstractMedia @@ -311,39 +312,66 @@ class Meta: type_meta = {"Meta": Meta, "id": graphene.ID(), "name": type_name} exclude_fields = [] + exclude_meta_fields = [] + graphql_fields = getattr(cls, "graphql_fields", []) + default_graphql_fields = [ + i.field_name for i in graphql_fields if isinstance(i, DefaultField) + ] + custom_graphql_fields = [ + get_field_type(i) + for i in graphql_fields + if not isinstance(i, DefaultField) + ] base_type_for_exclusion_checks = ( base_type if not issubclass(cls, WagtailPage) else WagtailPage ) for field in get_fields_and_properties(cls): - # Filter out any fields that are defined on the interface of base type to prevent the - # 'Excluding the custom field "" on DjangoObjectType "" has no effect. - # Either remove the custom field or remove the field from the "exclude" list.' warning - if ( + if field in default_graphql_fields and not ( + hasattr(interface, field) + or hasattr(base_type_for_exclusion_checks, field) + ): + raise TypeError( + f"{field} is not part of grapple default implementation" + ) + + if not ( field == "id" or hasattr(interface, field) or hasattr(base_type_for_exclusion_checks, field) ): - continue - - exclude_fields.append(field) + # see #105 + # Filter out any fields that are defined on the interface of base type to prevent the + # 'Excluding the custom field "" on DjangoObjectType "" has no effect. + # Either remove the custom field or remove the field from the "exclude" list.' warning + exclude_meta_fields.append(field) + else: + if ( + default_graphql_fields + and field not in default_graphql_fields + ): + # Filter out any fields that are not acquired by the user + exclude_fields.append(field) # Add any custom fields to node if they are defined. methods = {} - if hasattr(cls, "graphql_fields"): - for field in cls.graphql_fields: - # Add field to GQL type with correct field-type - field, field_type = get_field_type(field) - type_meta[field.field_name] = field_type - - # Remove field from excluded list - if field.field_name in exclude_fields: - exclude_fields.remove(field.field_name) - - # Add a custom resolver for each field - methods["resolve_" + field.field_name] = model_resolver(field) - + for field, field_type in custom_graphql_fields: + # Add field to GQL type with correct field-type + type_meta[field.field_name] = field_type + + # Remove field from excluded lists + if field.field_name in exclude_fields: + exclude_fields.remove(field.field_name) + if field.field_name in exclude_meta_fields: + exclude_meta_fields.remove(field.field_name) + + # Add a custom resolver for each field + methods["resolve_" + field.field_name] = model_resolver(field) + for i in exclude_fields: + # because of #105 we can't remove them from the schema, so just raise error in case of access + methods["resolve_" + i] = resolve_not_exposed_exception + + type_meta["Meta"].exclude_fields = exclude_meta_fields # Replace stud node with real thing - type_meta["Meta"].exclude_fields = exclude_fields node = type(type_name, (base_type,), type_meta) # Add custom resolvers for fields diff --git a/grapple/models.py b/grapple/models.py index c02502b9..dd237791 100644 --- a/grapple/models.py +++ b/grapple/models.py @@ -4,6 +4,13 @@ from .registry import registry +class DefaultField: + field_name: str + + def __init__(self, field_name: str): + self.field_name = field_name + + # Classes used to define what the Django field should look like in the GQL type class GraphQLField: field_name: str diff --git a/grapple/utils.py b/grapple/utils.py index db252fbf..40aa5a53 100644 --- a/grapple/utils.py +++ b/grapple/utils.py @@ -1,6 +1,8 @@ +import graphql from django.conf import settings from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator from django.db import connection +from graphql import GraphQLError from wagtail.models import Site from wagtail.search.index import class_is_indexed from wagtail.search.models import Query @@ -216,3 +218,12 @@ def get_media_item_url(cls): if url[0] == "/": return settings.BASE_URL + url return url + + +def resolve_not_exposed_exception(cls, instance, info, **kwargs): + if info.return_type.of_type == graphql.GraphQLInt: + return 0 + if info.return_type.of_type == graphql.GraphQLString: + return "this field is not exposed" + # Add other types here + raise GraphQLError("this field is not exposed") diff --git a/tests/test_grapple.py b/tests/test_grapple.py index 263f8688..4afc204c 100644 --- a/tests/test_grapple.py +++ b/tests/test_grapple.py @@ -9,7 +9,7 @@ from django.db import connection from django.test import RequestFactory, TestCase, override_settings from graphene.test import Client -from testapp.factories import BlogPageFactory +from testapp.factories import BlogPageFactory, AuthorPageFactory from testapp.models import GlobalSocialMediaSettings, HomePage, SocialMediaSettings from wagtail.documents import get_document_model from wagtail.images import get_image_model @@ -1684,3 +1684,49 @@ def test_query_all_settings_with_site_filter(self): } }, ) + + +class ExcludeFieldTest(BaseGrappleTest): + def setUp(self): + super().setUp() + self.factory = RequestFactory() + self.author = AuthorPageFactory(parent=self.home, nickname="the_nickname") + + def test_returned_value_of_excluded_field(self): + query = """ + { + pages(contentType: "testapp.AuthorPage") { + id + ...on AuthorPage{ + contentType + numchild + } + } + } + """ + + executed = self.client.execute(query) + self.assertEqual( + executed["data"]["pages"][0]["contentType"], "this field is not exposed" + ) + self.assertEqual(executed["data"]["pages"][0]["numchild"], 0) + + def test_returned_value_of_meta_excluded_field(self): + query = """ + { + pages(contentType: "testapp.AuthorPage") { + id + ...on AuthorPage{ + nickname + } + } + } + """ + + executed = self.client.execute(query) + for e in executed["errors"]: + if "Cannot query field" in e["message"] and "nickname" in e["message"]: + "an assertion that indicates that the test is passed" + self.assertTrue(True) + return + self.fail("Expected error message not found in executed errors") diff --git a/tests/testapp/migrations/0003_authorpage_nickname.py b/tests/testapp/migrations/0003_authorpage_nickname.py new file mode 100644 index 00000000..d0983884 --- /dev/null +++ b/tests/testapp/migrations/0003_authorpage_nickname.py @@ -0,0 +1,17 @@ +# Generated by Django 4.1.9 on 2023-05-15 11:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("testapp", "0002_create_homepage"), + ] + + operations = [ + migrations.AddField( + model_name="authorpage", + name="nickname", + field=models.CharField(blank=True, max_length=255), + ), + ] diff --git a/tests/testapp/models/core.py b/tests/testapp/models/core.py index 09f4e7a7..2a109b54 100644 --- a/tests/testapp/models/core.py +++ b/tests/testapp/models/core.py @@ -37,6 +37,7 @@ GraphQLStreamfield, GraphQLString, GraphQLTag, + DefaultField, ) from grapple.utils import resolve_paginated_queryset @@ -76,10 +77,16 @@ class HomePage(Page): class AuthorPage(Page): name = models.CharField(max_length=255) + nickname = models.CharField(max_length=255, blank=True) content_panels = Page.content_panels + [FieldPanel("name")] - graphql_fields = [GraphQLString("name")] + graphql_fields = [ + DefaultField("id"), + DefaultField("title"), + DefaultField("slug"), + GraphQLString("name"), + ] class BlogPageTag(TaggedItemBase): From 6bf766b5bda236868969a21cd20376f52fa7e208 Mon Sep 17 00:00:00 2001 From: AmirW Date: Tue, 16 May 2023 17:40:33 +0330 Subject: [PATCH 3/4] fix python3.8 compatibility (tuple[T]) --- grapple/actions.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/grapple/actions.py b/grapple/actions.py index 1336dc3b..c438f239 100644 --- a/grapple/actions.py +++ b/grapple/actions.py @@ -1,7 +1,7 @@ import inspect from collections.abc import Iterable from types import MethodType -from typing import Any, Dict, Type, Union, Callable +from typing import Any, Dict, Type, Union, Callable, Tuple import graphene from django.apps import apps @@ -162,12 +162,12 @@ def get_fields_and_properties(cls): def get_field_type( field: Union[ GraphQLField, - tuple[GraphQLField, ComplicatedField], + Tuple[GraphQLField, ComplicatedField], Callable[[], GraphQLField], - tuple[Callable[[], GraphQLField], ComplicatedField], - tuple[Callable[[], tuple[GraphQLField, ComplicatedField]]], + Tuple[Callable[[], GraphQLField], ComplicatedField], + Tuple[Callable[[], Tuple[GraphQLField, ComplicatedField]]], ] -) -> tuple[GraphQLField, Any]: +) -> Tuple[GraphQLField, Any]: if callable(field): field = field() # If a tuple is returned then obj[1] wraps obj[0] From 0ea509d88610b40a06bcabd87a3049465d2de1ba Mon Sep 17 00:00:00 2001 From: AmirW Date: Wed, 17 May 2023 09:26:05 +0330 Subject: [PATCH 4/4] fix isort-pre-commit --- grapple/actions.py | 4 ++-- tests/test_grapple.py | 2 +- tests/testapp/models/core.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/grapple/actions.py b/grapple/actions.py index c438f239..b032a210 100644 --- a/grapple/actions.py +++ b/grapple/actions.py @@ -1,7 +1,7 @@ import inspect from collections.abc import Iterable from types import MethodType -from typing import Any, Dict, Type, Union, Callable, Tuple +from typing import Any, Callable, Dict, Tuple, Type, Union import graphene from django.apps import apps @@ -19,7 +19,7 @@ from wagtail.snippets.models import get_snippet_models from .helpers import field_middlewares, streamfield_types -from .models import GraphQLField, DefaultField +from .models import DefaultField, GraphQLField from .registry import registry from .settings import grapple_settings from .types.documents import DocumentObjectType diff --git a/tests/test_grapple.py b/tests/test_grapple.py index 4afc204c..121fec5e 100644 --- a/tests/test_grapple.py +++ b/tests/test_grapple.py @@ -9,7 +9,7 @@ from django.db import connection from django.test import RequestFactory, TestCase, override_settings from graphene.test import Client -from testapp.factories import BlogPageFactory, AuthorPageFactory +from testapp.factories import AuthorPageFactory, BlogPageFactory from testapp.models import GlobalSocialMediaSettings, HomePage, SocialMediaSettings from wagtail.documents import get_document_model from wagtail.images import get_image_model diff --git a/tests/testapp/models/core.py b/tests/testapp/models/core.py index 2a109b54..eee00b10 100644 --- a/tests/testapp/models/core.py +++ b/tests/testapp/models/core.py @@ -25,6 +25,7 @@ ) from grapple.middleware import IsAnonymousMiddleware from grapple.models import ( + DefaultField, GraphQLCollection, GraphQLDocument, GraphQLField, @@ -37,7 +38,6 @@ GraphQLStreamfield, GraphQLString, GraphQLTag, - DefaultField, ) from grapple.utils import resolve_paginated_queryset