-
Notifications
You must be signed in to change notification settings - Fork 764
Optimize views #1439
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
Changes from all commits
4b42599
314c68d
33b3426
94550ac
7d84e87
ba70076
f8af0eb
b24ed31
f48d54b
2e4c6a2
352db54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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) | ||
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
Rather than calling There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]) | ||
|
||
|
There was a problem hiding this comment.
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