From 4b4259917f5f2f6e0587e9f3cfdb96bb289005a3 Mon Sep 17 00:00:00 2001 From: Dan Sutich Date: Thu, 3 Aug 2023 09:48:41 -0400 Subject: [PATCH 1/8] Optimize execute_graphql_request --- graphene_django/views.py | 75 +++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index 3fb87d410..caf50ed79 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -9,13 +9,12 @@ from django.utils.decorators import method_decorator from django.views.decorators.csrf import ensure_csrf_cookie from django.views.generic import View -from graphql import OperationType, get_operation_ast, parse +from graphql import ExecutionResult, OperationType, execute, get_operation_ast, parse from graphql.error import GraphQLError -from graphql.execution import ExecutionResult - -from graphene import Schema from graphql.execution.middleware import MiddlewareManager +from graphql.validation import validate +from graphene import Schema from graphene_django.constants import MUTATION_ERRORS_FLAG from graphene_django.utils.utils import set_rollback @@ -299,51 +298,63 @@ 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 + operation_ast = get_operation_ast(document, operation_name) - raise HttpError( - HttpResponseNotAllowed( - ["POST"], - "Can only perform a {} operation from a POST request.".format( - operation_ast.operation.value - ), - ) + op_error = None + if not operation_ast: + op_error = "Must provide a valid operation." + elif operation_ast.operation == OperationType.SUBSCRIPTION: + op_error = "The 'subscription' operation is not supported." + + if op_error: + return ExecutionResult(errors=[GraphQLError(op_error)]) + + if ( + request.method.lower() == "get" + and operation_ast.operation != OperationType.QUERY + ): + 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: - extra_options["execution_context_class"] = self.execution_context_class + ) + + execute_args = (self.schema.graphql_schema, document) + + if validation_errors := validate(*execute_args): + return ExecutionResult(data=None, errors=validation_errors) - options = { - "source": query, + try: + execute_options = { "root_value": self.get_root_value(request), + "context_value": self.get_context(request), "variable_values": variables, "operation_name": operation_name, - "context_value": self.get_context(request), "middleware": self.get_middleware(request), } - options.update(extra_options) + if self.execution_context_class: + execute_options[ + "execution_context_class" + ] = self.execution_context_class - operation_ast = get_operation_ast(document, operation_name) if ( - operation_ast + graphene_settings.ATOMIC_MUTATIONS is True + or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True and operation_ast.operation == OperationType.MUTATION - and ( - graphene_settings.ATOMIC_MUTATIONS is True - or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True - ) ): with transaction.atomic(): - result = self.schema.execute(**options) + result = execute(*execute_args, **execute_options) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: transaction.set_rollback(True) return result - return self.schema.execute(**options) + return execute(*execute_args, **execute_options) except Exception as e: return ExecutionResult(errors=[e]) From 314c68d2d1e28b66d367fd7d1a00ae3b2a9a2891 Mon Sep 17 00:00:00 2001 From: Dan Sutich Date: Thu, 3 Aug 2023 09:49:23 -0400 Subject: [PATCH 2/8] Require operation_ast to be found by view handler --- graphene_django/views.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index caf50ed79..0cb557469 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -12,6 +12,7 @@ from graphql import ExecutionResult, OperationType, execute, get_operation_ast, parse from graphql.error import GraphQLError from graphql.execution.middleware import MiddlewareManager +from graphql.language import OperationDefinitionNode from graphql.validation import validate from graphene import Schema @@ -300,13 +301,23 @@ def execute_graphql_request( operation_ast = get_operation_ast(document, operation_name) - op_error = None if not operation_ast: - op_error = "Must provide a valid operation." - elif operation_ast.operation == OperationType.SUBSCRIPTION: - op_error = "The 'subscription' operation is not supported." + ops_count = len( + [ + x + for x in document.definitions + if isinstance(x, OperationDefinitionNode) + ] + ) + if ops_count > 1: + op_error = ( + "Must provide operation name if query contains multiple operations." + ) + elif operation_name: + op_error = f"Unknown operation named '{operation_name}'." + else: + op_error = "Must provide a valid operation." - if op_error: return ExecutionResult(errors=[GraphQLError(op_error)]) if ( @@ -346,8 +357,7 @@ def execute_graphql_request( if ( graphene_settings.ATOMIC_MUTATIONS is True or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True - and operation_ast.operation == OperationType.MUTATION - ): + ) and operation_ast.operation == OperationType.MUTATION: with transaction.atomic(): result = execute(*execute_args, **execute_options) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: From 33b3426092a2c6ceea35026276087f9c203e53ab Mon Sep 17 00:00:00 2001 From: Dan Sutich Date: Thu, 3 Aug 2023 12:33:50 -0400 Subject: [PATCH 3/8] Remove unused show_graphiql kwarg --- graphene_django/views.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index 0cb557469..f6634bb09 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -186,7 +186,7 @@ def dispatch(self, request, *args, **kwargs): or 200 ) else: - result, status_code = self.get_response(request, data, show_graphiql) + result, status_code = self.get_response(request, data) return HttpResponse( status=status_code, content=result, content_type="application/json" @@ -200,11 +200,11 @@ def dispatch(self, request, *args, **kwargs): ) return response - def get_response(self, request, data, show_graphiql=False): + def get_response(self, request, data): query, variables, operation_name, id = self.get_graphql_params(request, data) execution_result = self.execute_graphql_request( - request, data, query, variables, operation_name, show_graphiql + request, data, query, variables, operation_name ) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: @@ -231,7 +231,7 @@ def get_response(self, request, data, show_graphiql=False): response["id"] = id response["status"] = status_code - result = self.json_encode(request, response, pretty=show_graphiql) + result = self.json_encode(request, response) else: result = None @@ -286,12 +286,8 @@ def parse_body(self, request): return {} - def execute_graphql_request( - self, request, data, query, variables, operation_name, show_graphiql=False - ): + def execute_graphql_request(self, request, data, query, variables, operation_name): if not query: - if show_graphiql: - return None raise HttpError(HttpResponseBadRequest("Must provide query string.")) try: @@ -324,14 +320,12 @@ def execute_graphql_request( request.method.lower() == "get" and operation_ast.operation != OperationType.QUERY ): - if show_graphiql: - return None - raise HttpError( HttpResponseNotAllowed( ["POST"], - "Can only perform a {} operation from a POST request.".format( - operation_ast.operation.value + ( + f"Can only perform a {operation_ast.operation.value} operation " + "from a POST request." ), ) ) From 94550ac1e5bfc9f85d7719267ece6f7e57e99ea1 Mon Sep 17 00:00:00 2001 From: Dan Sutich Date: Thu, 3 Aug 2023 17:04:18 -0400 Subject: [PATCH 4/8] Old style if syntax --- graphene_django/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index f6634bb09..84f565c88 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -331,8 +331,9 @@ def execute_graphql_request(self, request, data, query, variables, operation_nam ) execute_args = (self.schema.graphql_schema, document) + validation_errors = validate(*execute_args) - if validation_errors := validate(*execute_args): + if validation_errors: return ExecutionResult(data=None, errors=validation_errors) try: From b24ed316511fca49d8b2d10c627ae71d16a6eb66 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 26 Oct 2023 20:07:11 +0800 Subject: [PATCH 5/8] Revert "Remove unused show_graphiql kwarg" This reverts commit 33b3426092a2c6ceea35026276087f9c203e53ab. --- graphene_django/views.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index 4d2c17b42..b696020c7 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -188,7 +188,7 @@ def dispatch(self, request, *args, **kwargs): or 200 ) else: - result, status_code = self.get_response(request, data) + result, status_code = self.get_response(request, data, show_graphiql) return HttpResponse( status=status_code, content=result, content_type="application/json" @@ -202,11 +202,11 @@ def dispatch(self, request, *args, **kwargs): ) return response - def get_response(self, request, data): + def get_response(self, request, data, show_graphiql=False): query, variables, operation_name, id = self.get_graphql_params(request, data) execution_result = self.execute_graphql_request( - request, data, query, variables, operation_name + request, data, query, variables, operation_name, show_graphiql ) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: @@ -233,7 +233,7 @@ def get_response(self, request, data): response["id"] = id response["status"] = status_code - result = self.json_encode(request, response) + result = self.json_encode(request, response, pretty=show_graphiql) else: result = None @@ -288,8 +288,12 @@ def parse_body(self, request): return {} - def execute_graphql_request(self, request, data, query, variables, operation_name): + def execute_graphql_request( + self, request, data, query, variables, operation_name, show_graphiql=False + ): if not query: + if show_graphiql: + return None raise HttpError(HttpResponseBadRequest("Must provide query string.")) try: @@ -322,12 +326,14 @@ def execute_graphql_request(self, request, data, query, variables, operation_nam request.method.lower() == "get" and operation_ast.operation != OperationType.QUERY ): + if show_graphiql: + return None + raise HttpError( HttpResponseNotAllowed( ["POST"], - ( - f"Can only perform a {operation_ast.operation.value} operation " - "from a POST request." + "Can only perform a {} operation from a POST request.".format( + operation_ast.operation.value ), ) ) From f48d54b64c83d70f5015d0527cec49f0c2b7d731 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 26 Oct 2023 19:51:19 +0800 Subject: [PATCH 6/8] Add missing schema validation step --- graphene_django/views.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index b696020c7..d7d74b8e9 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -9,7 +9,14 @@ from django.utils.decorators import method_decorator from django.views.decorators.csrf import ensure_csrf_cookie from django.views.generic import View -from graphql import ExecutionResult, OperationType, execute, get_operation_ast, parse +from graphql import ( + ExecutionResult, + OperationType, + execute, + get_operation_ast, + parse, + validate_schema, +) from graphql.error import GraphQLError from graphql.execution.middleware import MiddlewareManager from graphql.language import OperationDefinitionNode @@ -296,6 +303,10 @@ def execute_graphql_request( return None raise HttpError(HttpResponseBadRequest("Must provide query string.")) + schema_validation_errors = validate_schema(self.schema.graphql_schema) + if schema_validation_errors: + return ExecutionResult(data=None, errors=schema_validation_errors) + try: document = parse(query) except Exception as e: From 2e4c6a25dc95ee99b96a46a4a832d871a9dd0623 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Sun, 29 Oct 2023 12:08:58 +0800 Subject: [PATCH 7/8] Pass args directly to improve clarity --- graphene_django/views.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index d7d74b8e9..c095a35c5 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -303,7 +303,9 @@ def execute_graphql_request( return None raise HttpError(HttpResponseBadRequest("Must provide query string.")) - schema_validation_errors = validate_schema(self.schema.graphql_schema) + schema = self.schema.graphql_schema + + schema_validation_errors = validate_schema(schema) if schema_validation_errors: return ExecutionResult(data=None, errors=schema_validation_errors) @@ -349,8 +351,7 @@ def execute_graphql_request( ) ) - execute_args = (self.schema.graphql_schema, document) - validation_errors = validate(*execute_args) + validation_errors = validate(schema, document) if validation_errors: return ExecutionResult(data=None, errors=validation_errors) @@ -373,12 +374,12 @@ def execute_graphql_request( or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True ) and operation_ast.operation == OperationType.MUTATION: with transaction.atomic(): - result = execute(*execute_args, **execute_options) + result = execute(schema, document, **execute_options) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: transaction.set_rollback(True) return result - return execute(*execute_args, **execute_options) + return execute(schema, document, **execute_options) except Exception as e: return ExecutionResult(errors=[e]) From 352db54f02b5395e7c5c8f08f4520a03cf498c75 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Sun, 29 Oct 2023 12:11:55 +0800 Subject: [PATCH 8/8] Remove duplicated operation_ast not None check --- graphene_django/views.py | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index c095a35c5..9fc617207 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -19,7 +19,6 @@ ) from graphql.error import GraphQLError from graphql.execution.middleware import MiddlewareManager -from graphql.language import OperationDefinitionNode from graphql.validation import validate from graphene import Schema @@ -316,27 +315,9 @@ def execute_graphql_request( operation_ast = get_operation_ast(document, operation_name) - if not operation_ast: - ops_count = len( - [ - x - for x in document.definitions - if isinstance(x, OperationDefinitionNode) - ] - ) - if ops_count > 1: - op_error = ( - "Must provide operation name if query contains multiple operations." - ) - elif operation_name: - op_error = f"Unknown operation named '{operation_name}'." - else: - op_error = "Must provide a valid operation." - - return ExecutionResult(errors=[GraphQLError(op_error)]) - if ( request.method.lower() == "get" + and operation_ast is not None and operation_ast.operation != OperationType.QUERY ): if show_graphiql: @@ -370,9 +351,13 @@ def execute_graphql_request( ] = self.execution_context_class if ( - graphene_settings.ATOMIC_MUTATIONS is True - or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True - ) and operation_ast.operation == OperationType.MUTATION: + operation_ast is not None + and operation_ast.operation == OperationType.MUTATION + and ( + graphene_settings.ATOMIC_MUTATIONS is True + or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True + ) + ): with transaction.atomic(): result = execute(schema, document, **execute_options) if getattr(request, MUTATION_ERRORS_FLAG, False) is True: