diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c63742a..c2cdc99 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,15 +8,11 @@ jobs: strategy: max-parallel: 4 matrix: - django: ["2.2", "3.0", "3.1", "3.2"] - python-version: ["3.6", "3.7", "3.8", "3.9"] + django: ["3.2", "4.0", "4.1"] + python-version: ["3.8", "3.9", "3.10"] include: - django: "3.2" - python-version: "3.10" - - django: "4.0" - python-version: "3.10" - - django: "main" - python-version: "3.10" + python-version: "3.7" steps: - uses: actions/checkout@v2 - name: Set up Python ${{ matrix.python-version }} diff --git a/Makefile b/Makefile index 6ecb25a..391c454 100644 --- a/Makefile +++ b/Makefile @@ -1,22 +1,21 @@ +.PHONY: help +help: + @echo "Please use \`make ' where is one of" + @grep -E '^\.PHONY: [a-zA-Z_-]+ .*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = "(: |##)"}; {printf "\033[36m%-30s\033[0m %s\n", $$2, $$3}' + .PHONY: dev-setup ## Install development dependencies dev-setup: pip install -e ".[dev]" -.PHONY: install-dev -install-dev: dev-setup # Alias install-dev -> dev-setup - -.PHONY: tests +.PHONY: tests ## Run unit tests tests: py.test graphene_django --cov=graphene_django -vv -.PHONY: test -test: tests # Alias test -> tests - -.PHONY: format +.PHONY: format ## Format code format: black graphene_django examples setup.py -.PHONY: lint +.PHONY: lint ## Lint code lint: flake8 graphene_django examples diff --git a/docs/tutorial-relay.rst b/docs/tutorial-relay.rst index 3de9022..a27e255 100644 --- a/docs/tutorial-relay.rst +++ b/docs/tutorial-relay.rst @@ -151,7 +151,7 @@ Create ``cookbook/ingredients/schema.py`` and type the following: interfaces = (relay.Node, ) - class Query(graphene.ObjectType): + class Query(ObjectType): category = relay.Node.Field(CategoryNode) all_categories = DjangoFilterConnectionField(CategoryNode) diff --git a/graphene_django/__init__.py b/graphene_django/__init__.py index 999f3de..93a697a 100644 --- a/graphene_django/__init__.py +++ b/graphene_django/__init__.py @@ -1,7 +1,7 @@ from .fields import DjangoConnectionField, DjangoListField from .types import DjangoObjectType -__version__ = "3.0.0b7" +__version__ = "3.0.0b8" __all__ = [ "__version__", diff --git a/graphene_django/converter.py b/graphene_django/converter.py index c243e82..338ab6d 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -24,6 +24,7 @@ from graphene import ( Decimal, ) from graphene.types.json import JSONString +from graphene.types.scalars import BigInt from graphene.utils.str_converters import to_camel_case from graphql import GraphQLError, assert_valid_name from graphql.pyutils import register_description @@ -186,10 +187,14 @@ def convert_field_to_uuid(field, registry=None): ) +@convert_django_field.register(models.BigIntegerField) +def convert_big_int_field(field, registry=None): + return BigInt(description=field.help_text, required=not field.null) + + @convert_django_field.register(models.PositiveIntegerField) @convert_django_field.register(models.PositiveSmallIntegerField) @convert_django_field.register(models.SmallIntegerField) -@convert_django_field.register(models.BigIntegerField) @convert_django_field.register(models.IntegerField) def convert_field_to_int(field, registry=None): return Int(description=get_django_field_description(field), required=not field.null) @@ -205,7 +210,9 @@ def convert_field_to_boolean(field, registry=None): @convert_django_field.register(models.DecimalField) def convert_field_to_decimal(field, registry=None): - return Decimal(description=field.help_text, required=not field.null) + return Decimal( + description=get_django_field_description(field), required=not field.null + ) @convert_django_field.register(models.FloatField) @@ -301,7 +308,24 @@ def convert_field_to_djangomodel(field, registry=None): if not _type: return - return Field( + class CustomField(Field): + def wrap_resolve(self, parent_resolver): + """ + Implements a custom resolver which go through the `get_node` method to insure that + it goes through the `get_queryset` method of the DjangoObjectType. + """ + resolver = super().wrap_resolve(parent_resolver) + + def custom_resolver(root, info, **args): + fk_obj = resolver(root, info, **args) + if fk_obj is None: + return None + else: + return _type.get_node(info, fk_obj.pk) + + return custom_resolver + + return CustomField( _type, description=get_django_field_description(field), required=not field.null, diff --git a/graphene_django/fields.py b/graphene_django/fields.py index eb932c1..3c48595 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -69,7 +69,10 @@ class DjangoListField(Field): _type = _type.of_type django_object_type = _type.of_type.of_type return partial( - self.list_resolver, django_object_type, resolver, self.get_manager(), + self.list_resolver, + django_object_type, + resolver, + self.get_manager(), ) @@ -146,36 +149,40 @@ class DjangoConnectionField(ConnectionField): iterable = maybe_queryset(iterable) if isinstance(iterable, QuerySet): - list_length = iterable.count() + array_length = iterable.count() else: - list_length = len(iterable) - list_slice_length = ( - min(max_limit, list_length) if max_limit is not None else list_length - ) + array_length = len(iterable) - # If after is higher than list_length, connection_from_list_slice + # If after is higher than array_length, connection_from_array_slice # would try to do a negative slicing which makes django throw an # AssertionError - after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length) + slice_start = min( + get_offset_with_default(args.get("after"), -1) + 1, + array_length, + ) + array_slice_length = array_length - slice_start - if max_limit is not None and args.get("first", None) is None: - if args.get("last", None) is not None: - after = list_length - args["last"] - else: - args["first"] = max_limit + # Impose the maximum limit via the `first` field if neither first or last are already provided + # (note that if any of them is provided they must be under max_limit otherwise an error is raised). + if ( + max_limit is not None + and args.get("first", None) is None + and args.get("last", None) is None + ): + args["first"] = max_limit connection = connection_from_array_slice( - iterable[after:], + iterable[slice_start:], args, - slice_start=after, - array_length=list_length, - array_slice_length=list_slice_length, + slice_start=slice_start, + array_length=array_length, + array_slice_length=array_slice_length, connection_type=partial(connection_adapter, connection), edge_type=connection.Edge, page_info_type=page_info_adapter, ) connection.iterable = iterable - connection.length = list_length + connection.length = array_length return connection @classmethod diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index c6dd50e..eeb197e 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -30,7 +30,7 @@ def convert_enum(data): class DjangoFilterConnectionField(DjangoConnectionField): def __init__( self, - type, + type_, fields=None, order_by=None, extra_filter_meta=None, @@ -44,7 +44,7 @@ class DjangoFilterConnectionField(DjangoConnectionField): self._filtering_args = None self._extra_filter_meta = extra_filter_meta self._base_args = None - super(DjangoFilterConnectionField, self).__init__(type, *args, **kwargs) + super(DjangoFilterConnectionField, self).__init__(type_, *args, **kwargs) @property def args(self): diff --git a/graphene_django/filter/filters/global_id_filter.py b/graphene_django/filter/filters/global_id_filter.py index a612a8a..da16585 100644 --- a/graphene_django/filter/filters/global_id_filter.py +++ b/graphene_django/filter/filters/global_id_filter.py @@ -13,7 +13,7 @@ class GlobalIDFilter(Filter): field_class = GlobalIDFormField def filter(self, qs, value): - """ Convert the filter value to a primary key before filtering """ + """Convert the filter value to a primary key before filtering""" _id = None if value is not None: _, _id = from_global_id(value) diff --git a/graphene_django/filter/filterset.py b/graphene_django/filter/filterset.py index b3333bf..57c35af 100644 --- a/graphene_django/filter/filterset.py +++ b/graphene_django/filter/filterset.py @@ -18,8 +18,8 @@ GRAPHENE_FILTER_SET_OVERRIDES = { class GrapheneFilterSetMixin(BaseFilterSet): - """ A django_filters.filterset.BaseFilterSet with default filter overrides - to handle global IDs """ + """A django_filters.filterset.BaseFilterSet with default filter overrides + to handle global IDs""" FILTER_DEFAULTS = dict( itertools.chain( @@ -29,8 +29,7 @@ class GrapheneFilterSetMixin(BaseFilterSet): def setup_filterset(filterset_class): - """ Wrap a provided filterset in Graphene-specific functionality - """ + """Wrap a provided filterset in Graphene-specific functionality""" return type( "Graphene{}".format(filterset_class.__name__), (filterset_class, GrapheneFilterSetMixin), @@ -39,8 +38,7 @@ def setup_filterset(filterset_class): def custom_filterset_factory(model, filterset_base_class=FilterSet, **meta): - """ Create a filterset for the given model using the provided meta data - """ + """Create a filterset for the given model using the provided meta data""" meta.update({"model": model}) meta_class = type(str("Meta"), (object,), meta) filterset = type( diff --git a/graphene_django/filter/tests/conftest.py b/graphene_django/filter/tests/conftest.py index 57924af..e2bba68 100644 --- a/graphene_django/filter/tests/conftest.py +++ b/graphene_django/filter/tests/conftest.py @@ -89,10 +89,10 @@ def Query(EventType): def resolve_events(self, info, **kwargs): events = [ - Event(name="Live Show", tags=["concert", "music", "rock"],), - Event(name="Musical", tags=["movie", "music"],), - Event(name="Ballet", tags=["concert", "dance"],), - Event(name="Speech", tags=[],), + Event(name="Live Show", tags=["concert", "music", "rock"]), + Event(name="Musical", tags=["movie", "music"]), + Event(name="Ballet", tags=["concert", "dance"]), + Event(name="Speech", tags=[]), ] STORE["events"] = events diff --git a/graphene_django/filter/tests/test_array_field_exact_filter.py b/graphene_django/filter/tests/test_array_field_exact_filter.py index cd72868..10e32ef 100644 --- a/graphene_django/filter/tests/test_array_field_exact_filter.py +++ b/graphene_django/filter/tests/test_array_field_exact_filter.py @@ -120,10 +120,7 @@ def test_array_field_filter_schema_type(Query): "randomField": "[Boolean!]", } filters_str = ", ".join( - [ - f"{filter_field}: {gql_type} = null" - for filter_field, gql_type in filters.items() - ] + [f"{filter_field}: {gql_type}" for filter_field, gql_type in filters.items()] ) assert ( f"type Query {{\n events({filters_str}): EventTypeConnection\n}}" in schema_str diff --git a/graphene_django/filter/tests/test_enum_filtering.py b/graphene_django/filter/tests/test_enum_filtering.py index 09c69b3..a284d08 100644 --- a/graphene_django/filter/tests/test_enum_filtering.py +++ b/graphene_django/filter/tests/test_enum_filtering.py @@ -54,13 +54,13 @@ def reporter_article_data(): first_name="Jane", last_name="Doe", email="janedoe@example.com", a_choice=2 ) Article.objects.create( - headline="Article Node 1", reporter=john, editor=john, lang="es", + headline="Article Node 1", reporter=john, editor=john, lang="es" ) Article.objects.create( - headline="Article Node 2", reporter=john, editor=john, lang="en", + headline="Article Node 2", reporter=john, editor=john, lang="en" ) Article.objects.create( - headline="Article Node 3", reporter=jane, editor=jane, lang="en", + headline="Article Node 3", reporter=jane, editor=jane, lang="en" ) @@ -80,7 +80,13 @@ def test_filter_enum_on_connection(schema, reporter_article_data): } """ - expected = {"allArticles": {"edges": [{"node": {"headline": "Article Node 1"}},]}} + expected = { + "allArticles": { + "edges": [ + {"node": {"headline": "Article Node 1"}}, + ] + } + } result = schema.execute(query) assert not result.errors @@ -152,9 +158,6 @@ def test_filter_enum_field_schema_type(schema): "reporter_AChoice_In": "[TestsReporterAChoiceChoices]", } filters_str = ", ".join( - [ - f"{filter_field}: {gql_type} = null" - for filter_field, gql_type in filters.items() - ] + [f"{filter_field}: {gql_type}" for filter_field, gql_type in filters.items()] ) assert f" allArticles({filters_str}): ArticleTypeConnection\n" in schema_str diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 7d440f4..fe4ae87 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -1008,7 +1008,7 @@ def test_integer_field_filter_type(): assert str(schema) == dedent( """\ type Query { - pets(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null): PetTypeConnection + pets(offset: Int, before: String, after: String, first: Int, last: Int, age: Int): PetTypeConnection } type PetTypeConnection { @@ -1056,8 +1056,7 @@ def test_integer_field_filter_type(): interface Node { \"""The ID of the object\""" id: ID! - } - """ + }""" ) @@ -1077,7 +1076,7 @@ def test_other_filter_types(): assert str(schema) == dedent( """\ type Query { - pets(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null, age_Isnull: Boolean = null, age_Lt: Int = null): PetTypeConnection + pets(offset: Int, before: String, after: String, first: Int, last: Int, age: Int, age_Isnull: Boolean, age_Lt: Int): PetTypeConnection } type PetTypeConnection { @@ -1125,8 +1124,7 @@ def test_other_filter_types(): interface Node { \"""The ID of the object\""" id: ID! - } - """ + }""" ) @@ -1226,7 +1224,7 @@ def test_filter_filterset_based_on_mixin(): } } - result = schema.execute(query, variable_values={"email": reporter_1.email},) + result = schema.execute(query, variable_values={"email": reporter_1.email}) assert not result.errors assert result.data == expected @@ -1267,13 +1265,23 @@ def test_filter_string_contains(): result = schema.execute(query, variables={"filter": "Ja"}) assert not result.errors assert result.data == { - "people": {"edges": [{"node": {"name": "Jack"}}, {"node": {"name": "Jane"}},]} + "people": { + "edges": [ + {"node": {"name": "Jack"}}, + {"node": {"name": "Jane"}}, + ] + } } result = schema.execute(query, variables={"filter": "o"}) assert not result.errors assert result.data == { - "people": {"edges": [{"node": {"name": "Joe"}}, {"node": {"name": "Bob"}},]} + "people": { + "edges": [ + {"node": {"name": "Joe"}}, + {"node": {"name": "Bob"}}, + ] + } } diff --git a/graphene_django/filter/tests/test_typed_filter.py b/graphene_django/filter/tests/test_typed_filter.py index b903b59..a7edc56 100644 --- a/graphene_django/filter/tests/test_typed_filter.py +++ b/graphene_django/filter/tests/test_typed_filter.py @@ -98,20 +98,14 @@ def test_typed_filter_schema(schema): ) for filter_field, gql_type in filters.items(): - assert "{}: {} = null".format(filter_field, gql_type) in all_articles_filters + assert "{}: {}".format(filter_field, gql_type) in all_articles_filters def test_typed_filters_work(schema): reporter = Reporter.objects.create(first_name="John", last_name="Doe", email="") - Article.objects.create( - headline="A", reporter=reporter, editor=reporter, lang="es", - ) - Article.objects.create( - headline="B", reporter=reporter, editor=reporter, lang="es", - ) - Article.objects.create( - headline="C", reporter=reporter, editor=reporter, lang="en", - ) + Article.objects.create(headline="A", reporter=reporter, editor=reporter, lang="es") + Article.objects.create(headline="B", reporter=reporter, editor=reporter, lang="es") + Article.objects.create(headline="C", reporter=reporter, editor=reporter, lang="en") query = "query { articles (lang_In: [ES]) { edges { node { headline } } } }" diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index cd05a87..ebd2a00 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -94,7 +94,9 @@ def get_filtering_args_from_filterset(filterset_class, type): field_type = graphene.List(field_type) args[name] = graphene.Argument( - field_type, description=filter_field.label, required=required, + field_type, + description=filter_field.label, + required=required, ) return args diff --git a/graphene_django/forms/mutation.py b/graphene_django/forms/mutation.py index 5a3d8e7..13e9863 100644 --- a/graphene_django/forms/mutation.py +++ b/graphene_django/forms/mutation.py @@ -117,7 +117,7 @@ class DjangoModelFormMutation(BaseDjangoFormMutation): class Meta: abstract = True - errors = graphene.List(ErrorType) + errors = graphene.List(graphene.NonNull(ErrorType), required=True) @classmethod def __init_subclass_with_meta__( diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py index 7b76cd3..c26a6d8 100644 --- a/graphene_django/tests/models.py +++ b/graphene_django/tests/models.py @@ -13,6 +13,9 @@ class Person(models.Model): class Pet(models.Model): name = models.CharField(max_length=30) age = models.PositiveIntegerField() + owner = models.ForeignKey( + "Person", on_delete=models.CASCADE, null=True, blank=True, related_name="pets" + ) class FilmDetails(models.Model): diff --git a/graphene_django/tests/test_command.py b/graphene_django/tests/test_command.py index 70116b8..11a15bc 100644 --- a/graphene_django/tests/test_command.py +++ b/graphene_django/tests/test_command.py @@ -53,6 +53,5 @@ def test_generate_graphql_file_on_call_graphql_schema(): """\ type Query { hi: String - } - """ + }""" ) diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py index afd744f..9158b12 100644 --- a/graphene_django/tests/test_converter.py +++ b/graphene_django/tests/test_converter.py @@ -10,6 +10,7 @@ from graphene import NonNull from graphene.relay import ConnectionField, Node from graphene.types.datetime import Date, DateTime, Time from graphene.types.json import JSONString +from graphene.types.scalars import BigInt from ..compat import ( ArrayField, @@ -140,8 +141,8 @@ def test_should_small_integer_convert_int(): assert_conversion(models.SmallIntegerField, graphene.Int) -def test_should_big_integer_convert_int(): - assert_conversion(models.BigIntegerField, graphene.Int) +def test_should_big_integer_convert_big_int(): + assert_conversion(models.BigIntegerField, BigInt) def test_should_integer_convert_int(): diff --git a/graphene_django/tests/test_get_queryset.py b/graphene_django/tests/test_get_queryset.py new file mode 100644 index 0000000..b2647c3 --- /dev/null +++ b/graphene_django/tests/test_get_queryset.py @@ -0,0 +1,355 @@ +import pytest + +import graphene +from graphene.relay import Node + +from graphql_relay import to_global_id + +from ..fields import DjangoConnectionField +from ..types import DjangoObjectType + +from .models import Article, Reporter + + +class TestShouldCallGetQuerySetOnForeignKey: + """ + Check that the get_queryset method is called in both forward and reversed direction + of a foreignkey on types. + (see issue #1111) + """ + + @pytest.fixture(autouse=True) + def setup_schema(self): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + + @classmethod + def get_queryset(cls, queryset, info): + if info.context and info.context.get("admin"): + return queryset + raise Exception("Not authorized to access reporters.") + + class ArticleType(DjangoObjectType): + class Meta: + model = Article + + @classmethod + def get_queryset(cls, queryset, info): + return queryset.exclude(headline__startswith="Draft") + + class Query(graphene.ObjectType): + reporter = graphene.Field(ReporterType, id=graphene.ID(required=True)) + article = graphene.Field(ArticleType, id=graphene.ID(required=True)) + + def resolve_reporter(self, info, id): + return ( + ReporterType.get_queryset(Reporter.objects, info) + .filter(id=id) + .last() + ) + + def resolve_article(self, info, id): + return ( + ArticleType.get_queryset(Article.objects, info).filter(id=id).last() + ) + + self.schema = graphene.Schema(query=Query) + + self.reporter = Reporter.objects.create(first_name="Jane", last_name="Doe") + + self.articles = [ + Article.objects.create( + headline="A fantastic article", + reporter=self.reporter, + editor=self.reporter, + ), + Article.objects.create( + headline="Draft: My next best seller", + reporter=self.reporter, + editor=self.reporter, + ), + ] + + def test_get_queryset_called_on_field(self): + # If a user tries to access an article it is fine as long as it's not a draft one + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + } + } + """ + # Non-draft + result = self.schema.execute(query, variables={"id": self.articles[0].id}) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + } + # Draft + result = self.schema.execute(query, variables={"id": self.articles[1].id}) + assert not result.errors + assert result.data["article"] is None + + # If a non admin user tries to access a reporter they should get our authorization error + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute(query, variables={"id": self.reporter.id}) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute( + query, variables={"id": self.reporter.id}, context_value={"admin": True}, + ) + assert not result.errors + assert result.data == {"reporter": {"firstName": "Jane"}} + + def test_get_queryset_called_on_foreignkey(self): + # If a user tries to access a reporter through an article they should get our authorization error + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute(query, variables={"id": self.articles[0].id}) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters through an article + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute( + query, variables={"id": self.articles[0].id}, context_value={"admin": True}, + ) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + "reporter": {"firstName": "Jane"}, + } + + # An admin user should not be able to access draft article through a reporter + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + articles { + headline + } + } + } + """ + + result = self.schema.execute( + query, variables={"id": self.reporter.id}, context_value={"admin": True}, + ) + assert not result.errors + assert result.data["reporter"] == { + "firstName": "Jane", + "articles": [{"headline": "A fantastic article"}], + } + + +class TestShouldCallGetQuerySetOnForeignKeyNode: + """ + Check that the get_queryset method is called in both forward and reversed direction + of a foreignkey on types using a node interface. + (see issue #1111) + """ + + @pytest.fixture(autouse=True) + def setup_schema(self): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + @classmethod + def get_queryset(cls, queryset, info): + if info.context and info.context.get("admin"): + return queryset + raise Exception("Not authorized to access reporters.") + + class ArticleType(DjangoObjectType): + class Meta: + model = Article + interfaces = (Node,) + + @classmethod + def get_queryset(cls, queryset, info): + return queryset.exclude(headline__startswith="Draft") + + class Query(graphene.ObjectType): + reporter = Node.Field(ReporterType) + article = Node.Field(ArticleType) + + self.schema = graphene.Schema(query=Query) + + self.reporter = Reporter.objects.create(first_name="Jane", last_name="Doe") + + self.articles = [ + Article.objects.create( + headline="A fantastic article", + reporter=self.reporter, + editor=self.reporter, + ), + Article.objects.create( + headline="Draft: My next best seller", + reporter=self.reporter, + editor=self.reporter, + ), + ] + + def test_get_queryset_called_on_node(self): + # If a user tries to access an article it is fine as long as it's not a draft one + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + } + } + """ + # Non-draft + result = self.schema.execute( + query, variables={"id": to_global_id("ArticleType", self.articles[0].id)} + ) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + } + # Draft + result = self.schema.execute( + query, variables={"id": to_global_id("ArticleType", self.articles[1].id)} + ) + assert not result.errors + assert result.data["article"] is None + + # If a non admin user tries to access a reporter they should get our authorization error + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute( + query, variables={"id": to_global_id("ReporterType", self.reporter.id)} + ) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute( + query, + variables={"id": to_global_id("ReporterType", self.reporter.id)}, + context_value={"admin": True}, + ) + assert not result.errors + assert result.data == {"reporter": {"firstName": "Jane"}} + + def test_get_queryset_called_on_foreignkey(self): + # If a user tries to access a reporter through an article they should get our authorization error + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute( + query, variables={"id": to_global_id("ArticleType", self.articles[0].id)} + ) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters through an article + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute( + query, + variables={"id": to_global_id("ArticleType", self.articles[0].id)}, + context_value={"admin": True}, + ) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + "reporter": {"firstName": "Jane"}, + } + + # An admin user should not be able to access draft article through a reporter + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + articles { + edges { + node { + headline + } + } + } + } + } + """ + + result = self.schema.execute( + query, + variables={"id": to_global_id("ReporterType", self.reporter.id)}, + context_value={"admin": True}, + ) + assert not result.errors + assert result.data["reporter"] == { + "firstName": "Jane", + "articles": {"edges": [{"node": {"headline": "A fantastic article"}}]}, + } diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index aabe19c..e6ae64f 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -15,7 +15,7 @@ from ..compat import IntegerRangeField, MissingType from ..fields import DjangoConnectionField from ..types import DjangoObjectType from ..utils import DJANGO_FILTER_INSTALLED -from .models import Article, CNNReporter, Film, FilmDetails, Reporter +from .models import Article, CNNReporter, Film, FilmDetails, Person, Pet, Reporter def test_should_query_only_fields(): @@ -251,8 +251,8 @@ def test_should_node(): def test_should_query_onetoone_fields(): - film = Film(id=1) - film_details = FilmDetails(id=1, film=film) + film = Film.objects.create(id=1) + film_details = FilmDetails.objects.create(id=1, film=film) class FilmNode(DjangoObjectType): class Meta: @@ -1243,6 +1243,7 @@ def test_should_have_next_page(graphene_settings): } +@pytest.mark.parametrize("max_limit", [100, 4]) class TestBackwardPagination: def setup_schema(self, graphene_settings, max_limit): graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit @@ -1261,8 +1262,8 @@ class TestBackwardPagination: schema = graphene.Schema(query=Query) return schema - def do_queries(self, schema): - # Simply last 3 + def test_query_last(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) query_last = """ query { allReporters(last: 3) { @@ -1282,7 +1283,8 @@ class TestBackwardPagination: e["node"]["firstName"] for e in result.data["allReporters"]["edges"] ] == ["First 3", "First 4", "First 5"] - # Use a combination of first and last + def test_query_first_and_last(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) query_first_and_last = """ query { allReporters(first: 4, last: 3) { @@ -1302,7 +1304,8 @@ class TestBackwardPagination: e["node"]["firstName"] for e in result.data["allReporters"]["edges"] ] == ["First 1", "First 2", "First 3"] - # Use a combination of first and last and after + def test_query_first_last_and_after(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) query_first_last_and_after = """ query queryAfter($after: String) { allReporters(first: 4, last: 3, after: $after) { @@ -1317,7 +1320,8 @@ class TestBackwardPagination: after = base64.b64encode(b"arrayconnection:0").decode() result = schema.execute( - query_first_last_and_after, variable_values=dict(after=after) + query_first_last_and_after, + variable_values=dict(after=after), ) assert not result.errors assert len(result.data["allReporters"]["edges"]) == 3 @@ -1325,20 +1329,35 @@ class TestBackwardPagination: e["node"]["firstName"] for e in result.data["allReporters"]["edges"] ] == ["First 2", "First 3", "First 4"] - def test_should_query(self, graphene_settings): + def test_query_last_and_before(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) + query_first_last_and_after = """ + query queryAfter($before: String) { + allReporters(last: 1, before: $before) { + edges { + node { + firstName + } + } + } + } """ - Backward pagination should work as expected - """ - schema = self.setup_schema(graphene_settings, max_limit=100) - self.do_queries(schema) - def test_should_query_with_low_max_limit(self, graphene_settings): - """ - When doing backward pagination (using last) in combination with a max limit higher than the number of objects - we should really retrieve the last ones. - """ - schema = self.setup_schema(graphene_settings, max_limit=4) - self.do_queries(schema) + result = schema.execute( + query_first_last_and_after, + ) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 1 + assert result.data["allReporters"]["edges"][0]["node"]["firstName"] == "First 5" + + before = base64.b64encode(b"arrayconnection:5").decode() + result = schema.execute( + query_first_last_and_after, + variable_values=dict(before=before), + ) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 1 + assert result.data["allReporters"]["edges"][0]["node"]["firstName"] == "First 4" def test_should_preserve_prefetch_related(django_assert_num_queries): @@ -1480,7 +1499,11 @@ def test_connection_should_enable_offset_filtering(): result = schema.execute(query) assert not result.errors expected = { - "allReporters": {"edges": [{"node": {"firstName": "Some", "lastName": "Guy"}},]} + "allReporters": { + "edges": [ + {"node": {"firstName": "Some", "lastName": "Guy"}}, + ] + } } assert result.data == expected @@ -1521,7 +1544,9 @@ def test_connection_should_enable_offset_filtering_higher_than_max_limit( assert not result.errors expected = { "allReporters": { - "edges": [{"node": {"firstName": "Some", "lastName": "Lady"}},] + "edges": [ + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] } } assert result.data == expected @@ -1590,6 +1615,149 @@ def test_connection_should_allow_offset_filtering_with_after(): result = schema.execute(query, variable_values=dict(after=after)) assert not result.errors expected = { - "allReporters": {"edges": [{"node": {"firstName": "Jane", "lastName": "Roe"}},]} + "allReporters": { + "edges": [ + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + ] + } } assert result.data == expected + + +def test_connection_should_succeed_if_last_higher_than_number_of_objects(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + fields = "__all__" + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query ReporterPromiseConnectionQuery ($last: Int) { + allReporters(last: $last) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + result = schema.execute(query, variable_values=dict(last=2)) + assert not result.errors + expected = {"allReporters": {"edges": []}} + assert result.data == expected + + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + Reporter.objects.create(first_name="Jane", last_name="Roe") + Reporter.objects.create(first_name="Some", last_name="Lady") + + result = schema.execute(query, variable_values=dict(last=2)) + assert not result.errors + expected = { + "allReporters": { + "edges": [ + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] + } + } + assert result.data == expected + + result = schema.execute(query, variable_values=dict(last=4)) + assert not result.errors + expected = { + "allReporters": { + "edges": [ + {"node": {"firstName": "John", "lastName": "Doe"}}, + {"node": {"firstName": "Some", "lastName": "Guy"}}, + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] + } + } + assert result.data == expected + + result = schema.execute(query, variable_values=dict(last=20)) + assert not result.errors + expected = { + "allReporters": { + "edges": [ + {"node": {"firstName": "John", "lastName": "Doe"}}, + {"node": {"firstName": "Some", "lastName": "Guy"}}, + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] + } + } + assert result.data == expected + + +def test_should_query_nullable_foreign_key(): + class PetType(DjangoObjectType): + class Meta: + model = Pet + + class PersonType(DjangoObjectType): + class Meta: + model = Person + + class Query(graphene.ObjectType): + pet = graphene.Field(PetType, name=graphene.String(required=True)) + person = graphene.Field(PersonType, name=graphene.String(required=True)) + + def resolve_pet(self, info, name): + return Pet.objects.filter(name=name).first() + + def resolve_person(self, info, name): + return Person.objects.filter(name=name).first() + + schema = graphene.Schema(query=Query) + + person = Person.objects.create(name="Jane") + pets = [ + Pet.objects.create(name="Stray dog", age=1), + Pet.objects.create(name="Jane's dog", owner=person, age=1), + ] + + query_pet = """ + query getPet($name: String!) { + pet(name: $name) { + owner { + name + } + } + } + """ + result = schema.execute(query_pet, variables={"name": "Stray dog"}) + assert not result.errors + assert result.data["pet"] == { + "owner": None, + } + + result = schema.execute(query_pet, variables={"name": "Jane's dog"}) + assert not result.errors + assert result.data["pet"] == { + "owner": {"name": "Jane"}, + } + + query_owner = """ + query getOwner($name: String!) { + person(name: $name) { + pets { + name + } + } + } + """ + result = schema.execute(query_owner, variables={"name": "Jane"}) + assert not result.errors + assert result.data["person"] == { + "pets": [{"name": "Jane's dog"}], + } diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index bde72c7..4885917 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -183,7 +183,7 @@ def test_schema_representation(): pets: [Reporter!]! aChoice: TestsReporterAChoiceChoices reporterType: TestsReporterReporterTypeChoices - articles(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null): ArticleConnection! + articles(offset: Int, before: String, after: String, first: Int, last: Int): ArticleConnection! } \"""An enumeration.\""" @@ -244,8 +244,7 @@ def test_schema_representation(): \"""The ID of the object\""" id: ID! ): Node - } - """ + }""" ) assert str(schema) == expected @@ -525,8 +524,7 @@ class TestDjangoObjectType: id: ID! kind: String! cuteness: Int! - } - """ + }""" ) def test_django_objecttype_convert_choices_enum_list(self, PetModel): @@ -560,8 +558,7 @@ class TestDjangoObjectType: \"""Dog\""" DOG - } - """ + }""" ) def test_django_objecttype_convert_choices_enum_empty_list(self, PetModel): @@ -586,8 +583,7 @@ class TestDjangoObjectType: id: ID! kind: String! cuteness: Int! - } - """ + }""" ) def test_django_objecttype_convert_choices_enum_naming_collisions( @@ -621,8 +617,7 @@ class TestDjangoObjectType: \"""Dog\""" DOG - } - """ + }""" ) def test_django_objecttype_choices_custom_enum_name( @@ -660,8 +655,7 @@ class TestDjangoObjectType: \"""Dog\""" DOG - } - """ + }""" ) diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index 945fa87..c2f18c3 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -109,12 +109,10 @@ def test_reports_validation_errors(client): { "message": "Cannot query field 'unknownOne' on type 'QueryRoot'.", "locations": [{"line": 1, "column": 9}], - "path": None, }, { "message": "Cannot query field 'unknownTwo' on type 'QueryRoot'.", "locations": [{"line": 1, "column": 21}], - "path": None, }, ] } @@ -135,8 +133,6 @@ def test_errors_when_missing_operation_name(client): "errors": [ { "message": "Must provide operation name if query contains multiple operations.", - "locations": None, - "path": None, } ] } @@ -477,7 +473,6 @@ def test_handles_syntax_errors_caught_by_graphql(client): { "locations": [{"column": 1, "line": 1}], "message": "Syntax Error: Unexpected Name 'syntaxerror'.", - "path": None, } ] } diff --git a/graphene_django/types.py b/graphene_django/types.py index d272412..c256f1d 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -216,7 +216,7 @@ class DjangoObjectType(ObjectType): "Creating a DjangoObjectType without either the `fields` " "or the `exclude` option is deprecated. Add an explicit `fields " "= '__all__'` option on DjangoObjectType {class_name} to use all " - "fields".format(class_name=cls.__name__,), + "fields".format(class_name=cls.__name__), DeprecationWarning, stacklevel=2, ) diff --git a/graphene_django/utils/tests/test_str_converters.py b/graphene_django/utils/tests/test_str_converters.py index 6460c4e..d3d33c2 100644 --- a/graphene_django/utils/tests/test_str_converters.py +++ b/graphene_django/utils/tests/test_str_converters.py @@ -6,4 +6,4 @@ def test_to_const(): def test_to_const_unicode(): - assert to_const(u"Skoða þetta unicode stöff") == "SKODA_THETTA_UNICODE_STOFF" + assert to_const("Skoða þetta unicode stöff") == "SKODA_THETTA_UNICODE_STOFF" diff --git a/graphene_django/views.py b/graphene_django/views.py index c23b020..f533f70 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -11,7 +11,6 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.generic import View from graphql import OperationType, get_operation_ast, parse, validate from graphql.error import GraphQLError -from graphql.error import format_error as format_graphql_error from graphql.execution import ExecutionResult from graphene import Schema @@ -387,7 +386,7 @@ class GraphQLView(View): @staticmethod def format_error(error): if isinstance(error, GraphQLError): - return format_graphql_error(error) + return error.formatted return {"message": str(error)} diff --git a/setup.py b/setup.py index 1407be4..3a46d24 100644 --- a/setup.py +++ b/setup.py @@ -14,22 +14,22 @@ rest_framework_require = ["djangorestframework>=3.6.3"] tests_require = [ - "pytest>=3.6.3", + "pytest>=7.1.3", "pytest-cov", "pytest-random-order", "coveralls", "mock", "pytz", - "django-filter>=2", - "pytest-django>=3.3.2", + "django-filter>=22.1", + "pytest-django>=4.5.2", ] + rest_framework_require dev_requires = [ - "black==22.3.0", - "flake8>=4,<5", + "black==22.8.0", + "flake8==5.0.4", "flake8-black==0.3.3", - "flake8-bugbear==20.1.4", + "flake8-bugbear==22.9.11", ] + tests_require setup( @@ -46,24 +46,24 @@ setup( "Intended Audience :: Developers", "Topic :: Software Development :: Libraries", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", "Programming Language :: Python :: Implementation :: PyPy", "Framework :: Django", - "Framework :: Django :: 2.2", - "Framework :: Django :: 3.0", - "Framework :: Django :: 3.1", "Framework :: Django :: 3.2", + "Framework :: Django :: 4.0", + "Framework :: Django :: 4.1", ], keywords="api graphql protocol rest relay graphene", packages=find_packages(exclude=["tests", "examples", "examples.*"]), install_requires=[ - "graphene>=3.0,<4", + # "graphene>=3.0,<4", + "graphene @ git+https://github.com/loft-orbital/graphene.git@loft-v3-1.0#egg=graphene", "graphql-core>=3.1.0,<4", "graphql-relay>=3.1.1,<4", - "Django>=2.2", + "Django>=3.2", "promise>=2.1", "text-unidecode", ], diff --git a/tox.ini b/tox.ini index 6a8f6c6..47fd1e5 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,11 @@ [tox] envlist = - py{36,37,38,39}-django{22,30,31}, - py{36,37,38,39,310}-django32, - py{38,39,310}-django{40,main}, + py{37,38,39,310}-django32, + py{38,39,310}-django{40,41,main}, pre-commit [gh-actions] python = - 3.6: py36 3.7: py37 3.8: py38 3.9: py39 @@ -15,11 +13,9 @@ python = [gh-actions:env] DJANGO = - 2.2: django22 - 3.0: django30 - 3.1: django31 3.2: django32 4.0: django40 + 4.1: django41 main: djangomain [testenv] @@ -30,11 +26,9 @@ setenv = deps = -e.[test] psycopg2-binary - django22: Django>=2.2,<3.0 - django30: Django>=3.0,<3.1 - django31: Django>=3.1,<3.2 django32: Django>=3.2,<4.0 django40: Django>=4.0,<4.1 + django41: Django>=4.1,<4.2 djangomain: https://github.com/django/django/archive/main.zip commands = {posargs:py.test --cov=graphene_django graphene_django examples}