diff --git a/Makefile b/Makefile index 31e5c937c..79b20d112 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,10 @@ dev-setup: tests: PYTHONPATH=. pytest graphene_django --cov=graphene_django -vv +.PHONY: tests-repeat ## Run unit tests 100 times to possibly identify flaky unit tests (and run them in parallel) +tests-repeat: + PYTHONPATH=. pytest graphene_django --cov=graphene_django -vv --count 100 -n logical + .PHONY: format ## Format code format: black graphene_django examples setup.py diff --git a/docs/settings.rst b/docs/settings.rst index d38d0c9c8..20cf04100 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -6,7 +6,7 @@ Graphene-Django can be customised using settings. This page explains each settin Usage ----- -Add settings to your Django project by creating a Dictonary with name ``GRAPHENE`` in the project's ``settings.py``: +Add settings to your Django project by creating a Dictionary with name ``GRAPHENE`` in the project's ``settings.py``: .. code:: python diff --git a/examples/cookbook/dummy_data.json b/examples/cookbook/dummy_data.json index c585bfcdf..c661846c2 100644 --- a/examples/cookbook/dummy_data.json +++ b/examples/cookbook/dummy_data.json @@ -231,7 +231,7 @@ "fields": { "category": 3, "name": "Newt", - "notes": "Braised and Confuesd" + "notes": "Braised and Confused" }, "model": "ingredients.ingredient", "pk": 5 diff --git a/examples/starwars/data.py b/examples/starwars/data.py index 6bdbf579c..f5d266757 100644 --- a/examples/starwars/data.py +++ b/examples/starwars/data.py @@ -28,17 +28,17 @@ def initialize(): # Yeah, technically it's Corellian. But it flew in the service of the rebels, # so for the purposes of this demo it's a rebel ship. - falcon = Ship(id="4", name="Millenium Falcon", faction=rebels) + falcon = Ship(id="4", name="Millennium Falcon", faction=rebels) falcon.save() - homeOne = Ship(id="5", name="Home One", faction=rebels) - homeOne.save() + home_one = Ship(id="5", name="Home One", faction=rebels) + home_one.save() - tieFighter = Ship(id="6", name="TIE Fighter", faction=empire) - tieFighter.save() + tie_fighter = Ship(id="6", name="TIE Fighter", faction=empire) + tie_fighter.save() - tieInterceptor = Ship(id="7", name="TIE Interceptor", faction=empire) - tieInterceptor.save() + tie_interceptor = Ship(id="7", name="TIE Interceptor", faction=empire) + tie_interceptor.save() executor = Ship(id="8", name="Executor", faction=empire) executor.save() diff --git a/examples/starwars/tests/test_mutation.py b/examples/starwars/tests/test_mutation.py index e24bf8ae1..46b8fc351 100644 --- a/examples/starwars/tests/test_mutation.py +++ b/examples/starwars/tests/test_mutation.py @@ -40,7 +40,7 @@ def test_mutations(): {"node": {"id": "U2hpcDox", "name": "X-Wing"}}, {"node": {"id": "U2hpcDoy", "name": "Y-Wing"}}, {"node": {"id": "U2hpcDoz", "name": "A-Wing"}}, - {"node": {"id": "U2hpcDo0", "name": "Millenium Falcon"}}, + {"node": {"id": "U2hpcDo0", "name": "Millennium Falcon"}}, {"node": {"id": "U2hpcDo1", "name": "Home One"}}, {"node": {"id": "U2hpcDo5", "name": "Peter"}}, ] diff --git a/graphene_django/debug/exception/formating.py b/graphene_django/debug/exception/formatting.py similarity index 100% rename from graphene_django/debug/exception/formating.py rename to graphene_django/debug/exception/formatting.py diff --git a/graphene_django/debug/middleware.py b/graphene_django/debug/middleware.py index de0d72d18..6c18b1a1e 100644 --- a/graphene_django/debug/middleware.py +++ b/graphene_django/debug/middleware.py @@ -1,6 +1,6 @@ from django.db import connections -from .exception.formating import wrap_exception +from .exception.formatting import wrap_exception from .sql.tracking import unwrap_cursor, wrap_cursor from .types import DjangoDebug diff --git a/graphene_django/filter/tests/conftest.py b/graphene_django/filter/tests/conftest.py index 9f5d36667..a4097b183 100644 --- a/graphene_django/filter/tests/conftest.py +++ b/graphene_django/filter/tests/conftest.py @@ -44,7 +44,7 @@ class Meta: "name": ["exact", "contains"], } - # Those are actually usable with our Query fixture bellow + # Those are actually usable with our Query fixture below tags__contains = ArrayFilter(field_name="tags", lookup_expr="contains") tags__overlap = ArrayFilter(field_name="tags", lookup_expr="overlap") tags = ArrayFilter(field_name="tags", lookup_expr="exact") diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index df3b97acb..b9c8df465 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -789,7 +789,7 @@ class Query(ObjectType): query = """ query NodeFilteringQuery { - allReporters(orderBy: "-firtsnaMe") { + allReporters(orderBy: "-firstname") { edges { node { firstName @@ -802,7 +802,7 @@ class Query(ObjectType): assert result.errors -def test_order_by_is_perserved(): +def test_order_by_is_preserved(): class ReporterType(DjangoObjectType): class Meta: model = Reporter diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index 3dd835fc3..ac94fa167 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -8,19 +8,102 @@ from .filterset import custom_filterset_factory, setup_filterset -def get_field_type(registry, model, field_name): +def get_field_type_from_registry(registry, model, field_name): """ - Try to get a model field corresponding Graphql type from the DjangoObjectType. + Try to get a model field corresponding GraphQL type from the DjangoObjectType. """ object_type = registry.get_type_for_model(model) - if object_type: - object_type_field = object_type._meta.fields.get(field_name) - if object_type_field: - field_type = object_type_field.type - if isinstance(field_type, graphene.NonNull): - field_type = field_type.of_type - return field_type - return None + if not object_type: + return None + + object_type_field = object_type._meta.fields.get(field_name) + if not object_type_field: + return None + + field_type = object_type_field.type + if isinstance(field_type, graphene.NonNull): + field_type = field_type.of_type + return field_type + + +def get_field_type_from_model_field(model_field, form_field, registry): + """ + Get the field type from the model field. + + If the model field is a foreign key, then we need to get the type from the related model. + """ + if ( + isinstance(form_field, forms.ModelChoiceField) + or isinstance(form_field, forms.ModelMultipleChoiceField) + or isinstance(form_field, GlobalIDMultipleChoiceField) + or isinstance(form_field, GlobalIDFormField) + ): + # Foreign key have dynamic types and filtering on a foreign key actually means filtering on its ID. + return get_field_type_from_registry(registry, model_field.related_model, "id") + + return get_field_type_from_registry(registry, model_field.model, model_field.name) + + +def get_form_field(model_field, filter_field, required): + """ + Retrieve the form field to use for the filter. + + Get the form field either from: + # 1. the formfield corresponding to the model field + # 2. the field defined on filter + + Returns None if no form field can be found. + """ + form_field = None + if hasattr(model_field, "formfield"): + form_field = model_field.formfield(required=required) + if not form_field: + form_field = filter_field.field + return form_field + + +def get_field_type_and_form_field_for_implicit_filter( + model, filter_type, filter_field, registry, required +): + """ + Get the filter type for filters that are not explicitly declared. + + Returns a tuple of (field_type, form_field) where: + - field_type is the type of the filter argument + - form_field is the form field to use to validate the input value + """ + if filter_type == "isnull": + # Filter type is boolean, no form field. + return (graphene.Boolean, None) + + model_field = get_model_field(model, filter_field.field_name) + form_field = get_form_field(model_field, filter_field, required) + + # First try to get the matching field type from the GraphQL DjangoObjectType + if model_field: + field_type = get_field_type_from_model_field(model_field, form_field, registry) + return (field_type, form_field) + + return (None, None) + + +def get_field_type_for_explicit_filter(filter_field, form_field): + """ + Fallback on converting the form field either because: + - it's an explicitly declared filters + - we did not manage to get the type from the model type + """ + from ..forms.converter import convert_form_field + + form_field = form_field or filter_field.field + return convert_form_field(form_field).get_type() + + +def is_filter_list_or_range(filter_field): + """ + Determine if the filter is a ListFilter or RangeFilter. + """ + return isinstance(filter_field, ListFilter) or isinstance(filter_field, RangeFilter) def get_filtering_args_from_filterset(filterset_class, type): @@ -28,7 +111,6 @@ def get_filtering_args_from_filterset(filterset_class, type): Inspect a FilterSet and produce the arguments to pass to a Graphene Field. These arguments will be available to filter against in the GraphQL API. """ - from ..forms.converter import convert_form_field args = {} model = filterset_class._meta.model @@ -43,55 +125,27 @@ def get_filtering_args_from_filterset(filterset_class, type): isinstance(filter_field, TypedFilter) and filter_field.input_type is not None ): - # First check if the filter input type has been explicitely given + # First check if the filter input type has been explicitly given field_type = filter_field.input_type else: if name not in filterset_class.declared_filters or isinstance( filter_field, TypedFilter ): - # Get the filter field for filters that are no explicitly declared. - if filter_type == "isnull": - field_type = graphene.Boolean - else: - model_field = get_model_field(model, filter_field.field_name) - - # Get the form field either from: - # 1. the formfield corresponding to the model field - # 2. the field defined on filter - if hasattr(model_field, "formfield"): - form_field = model_field.formfield(required=required) - if not form_field: - form_field = filter_field.field - - # First try to get the matching field type from the GraphQL DjangoObjectType - if model_field: - if ( - isinstance(form_field, forms.ModelChoiceField) - or isinstance(form_field, forms.ModelMultipleChoiceField) - or isinstance(form_field, GlobalIDMultipleChoiceField) - or isinstance(form_field, GlobalIDFormField) - ): - # Foreign key have dynamic types and filtering on a foreign key actually means filtering on its ID. - field_type = get_field_type( - registry, model_field.related_model, "id" - ) - else: - field_type = get_field_type( - registry, model_field.model, model_field.name - ) + ( + field_type, + form_field, + ) = get_field_type_and_form_field_for_implicit_filter( + model, filter_type, filter_field, registry, required + ) if not field_type: - # Fallback on converting the form field either because: - # - it's an explicitly declared filters - # - we did not manage to get the type from the model type - form_field = form_field or filter_field.field - field_type = convert_form_field(form_field).get_type() - - if isinstance(filter_field, ListFilter) or isinstance( - filter_field, RangeFilter - ): - # Replace InFilter/RangeFilter filters (`in`, `range`) argument type to be a list of - # the same type as the field. See comments in `replace_csv_filters` method for more details. + field_type = get_field_type_for_explicit_filter( + filter_field, form_field + ) + + # Replace InFilter/RangeFilter filters (`in`, `range`) argument type to be a list of + # the same type as the field. See comments in `replace_csv_filters` method for more details. + if is_filter_list_or_range(filter_field): field_type = graphene.List(field_type) args[name] = graphene.Argument( diff --git a/graphene_django/forms/types.py b/graphene_django/forms/types.py index b370afd84..0e311e5d6 100644 --- a/graphene_django/forms/types.py +++ b/graphene_django/forms/types.py @@ -4,7 +4,7 @@ from graphene.utils.str_converters import to_camel_case from ..converter import BlankValueField -from ..types import ErrorType # noqa Import ErrorType for backwards compatability +from ..types import ErrorType # noqa Import ErrorType for backwards compatibility from .mutation import fields_for_form @@ -60,7 +60,7 @@ def mutate(_root, _info, data): and isinstance(object_type._meta.fields[name], BlankValueField) ): # Field type BlankValueField here means that field - # with choises have been converted to enum + # with choices have been converted to enum # (BlankValueField is using only for that task ?) setattr(cls, name, cls.get_enum_cnv_cls_instance(name, object_type)) elif ( diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py index 4afbbbce7..67e266731 100644 --- a/graphene_django/tests/models.py +++ b/graphene_django/tests/models.py @@ -97,7 +97,7 @@ class Meta: class APNewsReporter(Reporter): """ - This class only inherits from Reporter for testing multi table inheritence + This class only inherits from Reporter for testing multi table inheritance similar to what you'd see in django-polymorphic """ diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index 34828dbb4..b30c6793d 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -30,7 +30,7 @@ class ArticleConnection(Connection): test = String() - def resolve_test(): + def resolve_test(self): return "test" class Meta: diff --git a/graphene_django/types.py b/graphene_django/types.py index 163fe3f39..1d742ff3d 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -62,10 +62,7 @@ def construct_fields( return fields -def validate_fields(type_, model, fields, only_fields, exclude_fields): - # Validate the given fields against the model's fields and custom fields - all_field_names = set(fields.keys()) - only_fields = only_fields if only_fields is not ALL_FIELDS else () +def validate_only_fields(only_fields, all_field_names, model, type_): for name in only_fields or (): if name in all_field_names: continue @@ -83,20 +80,22 @@ def validate_fields(type_, model, fields, only_fields, exclude_fields): type_=type_, ) ) + continue - else: - warnings.warn( - ( - 'Field name "{field_name}" doesn\'t exist on Django model "{app_label}.{object_name}". ' - 'Consider removing the field from the "fields" list of DjangoObjectType "{type_}" because it has no effect.' - ).format( - field_name=name, - app_label=model._meta.app_label, - object_name=model._meta.object_name, - type_=type_, - ) + warnings.warn( + ( + 'Field name "{field_name}" doesn\'t exist on Django model "{app_label}.{object_name}". ' + 'Consider removing the field from the "fields" list of DjangoObjectType "{type_}" because it has no effect.' + ).format( + field_name=name, + app_label=model._meta.app_label, + object_name=model._meta.object_name, + type_=type_, ) + ) + +def validate_exclude_fields(exclude_fields, all_field_names, model, type_): # Validate exclude fields for name in exclude_fields or (): if name in all_field_names: @@ -107,19 +106,29 @@ def validate_fields(type_, model, fields, only_fields, exclude_fields): 'Either remove the custom field or remove the field from the "exclude" list.' ).format(field_name=name, type_=type_) ) - else: - if not hasattr(model, name): - warnings.warn( - ( - 'Django model "{app_label}.{object_name}" does not have a field or attribute named "{field_name}". ' - 'Consider removing the field from the "exclude" list of DjangoObjectType "{type_}" because it has no effect' - ).format( - field_name=name, - app_label=model._meta.app_label, - object_name=model._meta.object_name, - type_=type_, - ) + continue + + if not hasattr(model, name): + warnings.warn( + ( + 'Django model "{app_label}.{object_name}" does not have a field or attribute named "{field_name}". ' + 'Consider removing the field from the "exclude" list of DjangoObjectType "{type_}" because it has no effect' + ).format( + field_name=name, + app_label=model._meta.app_label, + object_name=model._meta.object_name, + type_=type_, ) + ) + + +def validate_fields(type_, model, fields, only_fields, exclude_fields): + # Validate the given fields against the model's fields and custom fields + all_field_names = set(fields.keys()) + only_fields = only_fields if only_fields is not ALL_FIELDS else () + + validate_only_fields(only_fields, all_field_names, model, type_) + validate_exclude_fields(exclude_fields, all_field_names, model, type_) class DjangoObjectTypeOptions(ObjectTypeOptions): diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py index ad9ff35f8..e1e52f53a 100644 --- a/graphene_django/utils/testing.py +++ b/graphene_django/utils/testing.py @@ -128,7 +128,7 @@ def _client(self, client): ) self.client = client - def assertResponseNoErrors(self, resp, msg=None): + def assert_response_no_errors(self, resp, msg=None): """ Assert that the call went through correctly. 200 means the syntax is ok, if there are no `errors`, the call was fine. @@ -138,7 +138,7 @@ def assertResponseNoErrors(self, resp, msg=None): self.assertEqual(resp.status_code, 200, msg or content) self.assertNotIn("errors", list(content.keys()), msg or content) - def assertResponseHasErrors(self, resp, msg=None): + def assert_response_has_errors(self, resp, msg=None): """ Assert that the call was failing. Take care: Even with errors, GraphQL returns status 200! :resp HttpResponse: Response diff --git a/graphene_django/views.py b/graphene_django/views.py index ce08d26a9..d83d7c471 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -103,16 +103,15 @@ def __init__( ): if not schema: schema = graphene_settings.SCHEMA + self.schema = schema or self.schema if middleware is None: middleware = graphene_settings.MIDDLEWARE + if isinstance(middleware, MiddlewareManager): + self.middleware = middleware + else: + self.middleware = list(instantiate_middleware(middleware)) - self.schema = schema or self.schema - if middleware is not None: - if isinstance(middleware, MiddlewareManager): - self.middleware = middleware - else: - self.middleware = list(instantiate_middleware(middleware)) self.root_value = root_value self.pretty = pretty or self.pretty self.graphiql = graphiql or self.graphiql @@ -285,6 +284,25 @@ def parse_body(self, request): return {} + def validate_query_request_type( + self, request, document, operation_name, show_graphiql + ): + if request.method.lower() == "get": + operation_ast = get_operation_ast(document, operation_name) + if ( + operation_ast + and operation_ast.operation != OperationType.QUERY + and not show_graphiql + ): + raise HttpError( + HttpResponseNotAllowed( + ["POST"], + "Can only perform a {} operation from a POST request.".format( + operation_ast.operation.value + ), + ) + ) + def execute_graphql_request( self, request, data, query, variables, operation_name, show_graphiql=False ): @@ -298,20 +316,12 @@ def execute_graphql_request( except Exception as e: return ExecutionResult(errors=[e]) - if request.method.lower() == "get": - operation_ast = get_operation_ast(document, operation_name) - if operation_ast and operation_ast.operation != OperationType.QUERY: - if show_graphiql: - return None + self.validate_query_request_type( + request, document, operation_name, show_graphiql + ) + if show_graphiql: + return None - raise HttpError( - HttpResponseNotAllowed( - ["POST"], - "Can only perform a {} operation from a POST request.".format( - operation_ast.operation.value - ), - ) - ) try: extra_options = {} if self.execution_context_class: @@ -328,14 +338,7 @@ def execute_graphql_request( options.update(extra_options) operation_ast = get_operation_ast(document, operation_name) - if ( - operation_ast - and operation_ast.operation == OperationType.MUTATION - and ( - graphene_settings.ATOMIC_MUTATIONS is True - or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True - ) - ): + if self.is_atomic_mutation_enabled(operation_ast, connection): with transaction.atomic(): result = self.schema.execute(**options) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: @@ -400,3 +403,14 @@ def get_content_type(request): meta = request.META content_type = meta.get("CONTENT_TYPE", meta.get("HTTP_CONTENT_TYPE", "")) return content_type.split(";", 1)[0].lower() + + @staticmethod + def is_atomic_mutation_enabled(operation_ast, connection): + return ( + operation_ast + and operation_ast.operation == OperationType.MUTATION + and ( + graphene_settings.ATOMIC_MUTATIONS is True + or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True + ) + ) diff --git a/setup.py b/setup.py index 51ed63779..21d6fc248 100644 --- a/setup.py +++ b/setup.py @@ -22,6 +22,8 @@ "pytz", "django-filter>=22.1", "pytest-django>=4.5.2", + "pytest-repeat>=0.9.1", + "pytest-xdist>=3.3.1", ] + rest_framework_require