Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize views #1439

Merged
merged 11 commits into from Oct 29, 2023
72 changes: 46 additions & 26 deletions graphene_django/views.py
Expand Up @@ -9,10 +9,17 @@
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,
validate_schema,
)
from graphql.error import GraphQLError
from graphql.execution import ExecutionResult
from graphql.execution.middleware import MiddlewareManager
from graphql.validation import validate

from graphene import Schema
from graphene_django.constants import MUTATION_ERRORS_FLAG
Expand Down Expand Up @@ -295,56 +302,69 @@ def execute_graphql_request(
return None
raise HttpError(HttpResponseBadRequest("Must provide query string."))

schema = self.schema.graphql_schema

schema_validation_errors = validate_schema(schema)
if schema_validation_errors:
return ExecutionResult(data=None, errors=schema_validation_errors)

try:
document = parse(query)
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the old logic would at some point call get_operation_ast so I hoisted it up


raise HttpError(
HttpResponseNotAllowed(
["POST"],
"Can only perform a {} operation from a POST request.".format(
operation_ast.operation.value
),
)
if (
request.method.lower() == "get"
and operation_ast is not None
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
)

validation_errors = validate(schema, document)

if validation_errors:
return ExecutionResult(data=None, errors=validation_errors)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing version called self.schema.execute which is basically just a wrapper to calling graphql_impl which performs the following steps:

  1. validate_schema -> we are doing this here
  2. parse -> we already did this operation a few lines up. This operation is surprisingly expensive to we get a nice optimization by not having to redundantly call it
  3. validate -> This actually also calls validate_schema (though schema validation is cached so that particular piece isn't done redundantly)

Rather than calling self.schema.execute I am reimplementing the validate call and then calling execute directly rather than redundantly parsing and validating the request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put this part in a separate PR? I can review and merge this part now.


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
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 = self.schema.execute(**options)
result = execute(schema, document, **execute_options)
if getattr(request, MUTATION_ERRORS_FLAG, False) is True:
transaction.set_rollback(True)
return result

return self.schema.execute(**options)
return execute(schema, document, **execute_options)
except Exception as e:
return ExecutionResult(errors=[e])

Expand Down