From 1ba42951054cdb9c59df675fb581f2e814215531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Thu, 15 Feb 2024 19:14:27 +0100 Subject: [PATCH 01/11] Execute queries over GET --- ariadne/asgi/handlers/http.py | 56 +++++++++++++++++++++++++++++++---- ariadne/graphql.py | 33 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/ariadne/asgi/handlers/http.py b/ariadne/asgi/handlers/http.py index 8b49802b2..561b507b3 100644 --- a/ariadne/asgi/handlers/http.py +++ b/ariadne/asgi/handlers/http.py @@ -35,6 +35,7 @@ class GraphQLHTTPHandler(GraphQLHttpHandlerBase): def __init__( self, + execute_get_queries: bool = False, extensions: Optional[Extensions] = None, middleware: Optional[Middlewares] = None, middleware_manager_class: Optional[Type[MiddlewareManager]] = None, @@ -43,6 +44,9 @@ def __init__( # Optional arguments + `execute_get_queries`: a `bool` that controls if `query` operations + sent using the `GET` method should be executed. Defaults to `False`. + `extensions`: an `Extensions` list or callable returning a list of extensions server should use during query execution. Defaults to no extensions. @@ -58,6 +62,7 @@ def __init__( """ super().__init__() + self.execute_get_queries = execute_get_queries self.extensions = extensions self.middleware = middleware self.middleware_manager_class = middleware_manager_class or MiddlewareManager @@ -114,9 +119,12 @@ async def handle_request(self, request: Request) -> Response: `request`: the `Request` instance from Starlette or FastAPI. """ - if request.method == "GET" and self.introspection and self.explorer: - # only render explorer when introspection is enabled - return await self.render_explorer(request, self.explorer) + if request.method == "GET": + if self.execute_get_queries and request.query_params.get("query"): + return await self.graphql_http_server(request) + elif self.introspection and self.explorer: + # only render explorer when introspection is enabled + return await self.render_explorer(request, self.explorer) if request.method == "POST": return await self.graphql_http_server(request) @@ -182,6 +190,12 @@ async def extract_data_from_request(self, request: Request): return await self.extract_data_from_json_request(request) if content_type == DATA_TYPE_MULTIPART: return await self.extract_data_from_multipart_request(request) + if ( + request.method == "GET" + and self.execute_get_queries + and request.query_params.get("query") + ): + return await self.extract_data_from_get_request(request) raise HttpBadRequestError( "Posted content must be of type {} or {}".format( @@ -189,7 +203,7 @@ async def extract_data_from_request(self, request: Request): ) ) - async def extract_data_from_json_request(self, request: Request): + async def extract_data_from_json_request(self, request: Request) -> dict: """Extracts GraphQL data from JSON request. Returns a `dict` with GraphQL query data that was not yet validated. @@ -203,7 +217,9 @@ async def extract_data_from_json_request(self, request: Request): except (TypeError, ValueError) as ex: raise HttpBadRequestError("Request body is not a valid JSON") from ex - async def extract_data_from_multipart_request(self, request: Request): + async def extract_data_from_multipart_request( + self, request: Request + ) -> dict | list: """Extracts GraphQL data from `multipart/form-data` request. Returns an unvalidated `dict` with GraphQL query data. @@ -240,6 +256,35 @@ async def extract_data_from_multipart_request(self, request: Request): return combine_multipart_data(operations, files_map, request_files) + async def extract_data_from_get_request(self, request: Request) -> dict: + """Extracts GraphQL data from GET request's querystring. + + Returns a `dict` with GraphQL query data that was not yet validated. + + # Required arguments + + `request`: the `Request` instance from Starlette or FastAPI. + """ + query = request.query_params["query"].strip() + operation_name = request.query_params.get("operationName", "").strip() + variables = request.query_params.get("variables", "").strip() + + clean_variables = None + + if variables: + try: + clean_variables = json.loads(variables) + except (TypeError, ValueError) as ex: + raise HttpBadRequestError( + "Variables query arg is not a valid JSON" + ) from ex + + return { + "query": query, + "operationName": operation_name or None, + "variables": clean_variables, + } + async def execute_graphql_query( self, request: Any, @@ -284,6 +329,7 @@ async def execute_graphql_query( query_validator=self.query_validator, query_document=query_document, validation_rules=self.validation_rules, + require_query=request.method == "GET", debug=self.debug, introspection=self.introspection, logger=self.logger, diff --git a/ariadne/graphql.py b/ariadne/graphql.py index f3d25a733..1c5ee1428 100644 --- a/ariadne/graphql.py +++ b/ariadne/graphql.py @@ -22,6 +22,7 @@ GraphQLError, GraphQLSchema, MiddlewareManager, + OperationDefinitionNode, TypeInfo, execute, execute_sync, @@ -71,6 +72,7 @@ async def graphql( introspection: bool = True, logger: Union[None, str, Logger, LoggerAdapter] = None, validation_rules: Optional[ValidationRules] = None, + require_query: bool = False, error_formatter: ErrorFormatter = format_error, middleware: MiddlewareList = None, middleware_manager_class: Optional[Type[MiddlewareManager]] = None, @@ -123,6 +125,9 @@ async def graphql( `validation_rules`: a `list` of or callable returning list of custom validation rules to use to validate query before it's executed. + `require_query`: a `bool` controlling if GraphQL operation to execute must be + a query (vs. mutation or subscription). + `error_formatter`: an `ErrorFormatter` callable to use to convert GraphQL errors encountered during query execution to JSON-serializable format. @@ -169,6 +174,7 @@ async def graphql( enable_introspection=introspection, query_validator=query_validator, ) + if validation_errors: return handle_graphql_errors( validation_errors, @@ -178,6 +184,9 @@ async def graphql( extension_manager=extension_manager, ) + if require_query and not validation_errors: + validate_operation_is_query(document, operation_name) + if callable(root_value): try: root_value = root_value( # type: ignore @@ -639,3 +648,27 @@ def validate_variables(variables) -> None: def validate_operation_name(operation_name) -> None: if operation_name is not None and not isinstance(operation_name, str): raise GraphQLError('"%s" is not a valid operation name.' % operation_name) + + +def validate_operation_is_query(document_ast: DocumentNode, operation_name: str): + query_operations: List[Optional[str]] = [] + for definition in document_ast.definitions: + if ( + isinstance(definition, OperationDefinitionNode) + and definition.operation.name == "QUERY" + ): + if definition.name: + query_operations.append(definition.name.value) + else: + query_operations.append(None) + + if operation_name: + if query_operations not in query_operations: + raise GraphQLError( + f"Operation '{operation_name}' can't be executed using the GET " + "HTTP method. Use POST instead." + ) + elif len(query_operations) != 1: + raise GraphQLError( + f"'operationName' is required if 'query' defines multiple operations." + ) From 30cedbdebfe53be04d023872e717769354a00b0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 11:38:34 +0100 Subject: [PATCH 02/11] Fix tests --- ariadne/asgi/handlers/http.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ariadne/asgi/handlers/http.py b/ariadne/asgi/handlers/http.py index 561b507b3..c171d283d 100644 --- a/ariadne/asgi/handlers/http.py +++ b/ariadne/asgi/handlers/http.py @@ -319,6 +319,11 @@ async def execute_graphql_query( if self.schema is None: raise TypeError("schema is not set, call configure method to initialize it") + + if isinstance(request, Request): + require_query = request.method == "GET" + else: + require_query = False return await graphql( self.schema, @@ -329,7 +334,7 @@ async def execute_graphql_query( query_validator=self.query_validator, query_document=query_document, validation_rules=self.validation_rules, - require_query=request.method == "GET", + require_query=require_query, debug=self.debug, introspection=self.introspection, logger=self.logger, From a996aaf657a0175af404ac59d769f9bcc5ccf832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 11:42:32 +0100 Subject: [PATCH 03/11] Fix linters --- ariadne/asgi/handlers/http.py | 4 ++-- ariadne/graphql.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ariadne/asgi/handlers/http.py b/ariadne/asgi/handlers/http.py index c171d283d..1dd292036 100644 --- a/ariadne/asgi/handlers/http.py +++ b/ariadne/asgi/handlers/http.py @@ -122,7 +122,7 @@ async def handle_request(self, request: Request) -> Response: if request.method == "GET": if self.execute_get_queries and request.query_params.get("query"): return await self.graphql_http_server(request) - elif self.introspection and self.explorer: + if self.introspection and self.explorer: # only render explorer when introspection is enabled return await self.render_explorer(request, self.explorer) @@ -319,7 +319,7 @@ async def execute_graphql_query( if self.schema is None: raise TypeError("schema is not set, call configure method to initialize it") - + if isinstance(request, Request): require_query = request.method == "GET" else: diff --git a/ariadne/graphql.py b/ariadne/graphql.py index 1c5ee1428..4db4af080 100644 --- a/ariadne/graphql.py +++ b/ariadne/graphql.py @@ -663,12 +663,12 @@ def validate_operation_is_query(document_ast: DocumentNode, operation_name: str) query_operations.append(None) if operation_name: - if query_operations not in query_operations: + if operation_name not in query_operations: raise GraphQLError( f"Operation '{operation_name}' can't be executed using the GET " "HTTP method. Use POST instead." ) elif len(query_operations) != 1: raise GraphQLError( - f"'operationName' is required if 'query' defines multiple operations." + "'operationName' is required if 'query' defines multiple operations." ) From 2ebfb8cdf76333e1cb753460172db56054388e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 11:45:59 +0100 Subject: [PATCH 04/11] Fix type annotation for py 3.8 --- ariadne/asgi/handlers/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ariadne/asgi/handlers/http.py b/ariadne/asgi/handlers/http.py index 1dd292036..15363725d 100644 --- a/ariadne/asgi/handlers/http.py +++ b/ariadne/asgi/handlers/http.py @@ -219,7 +219,7 @@ async def extract_data_from_json_request(self, request: Request) -> dict: async def extract_data_from_multipart_request( self, request: Request - ) -> dict | list: + ) -> Union[dict, list]: """Extracts GraphQL data from `multipart/form-data` request. Returns an unvalidated `dict` with GraphQL query data. From f4b41b2d3f077d8e7f289cbabffdd3cb73cda426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 11:48:37 +0100 Subject: [PATCH 05/11] Add missing import --- ariadne/asgi/handlers/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ariadne/asgi/handlers/http.py b/ariadne/asgi/handlers/http.py index 15363725d..dca234eb8 100644 --- a/ariadne/asgi/handlers/http.py +++ b/ariadne/asgi/handlers/http.py @@ -1,6 +1,6 @@ import json from inspect import isawaitable -from typing import Any, Optional, Type, cast +from typing import Any, Optional, Type, Union, cast from graphql import DocumentNode, MiddlewareManager from starlette.datastructures import UploadFile From b5314be9ff5a3efe0c4fbc0ffce598e402946767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 18:12:13 +0100 Subject: [PATCH 06/11] Execute queries over GET in WSGI --- ariadne/graphql.py | 18 ++- ariadne/wsgi.py | 77 +++++++++++- tests/wsgi/test_configuration.py | 195 ++++++++++++++++++++++++++++--- 3 files changed, 267 insertions(+), 23 deletions(-) diff --git a/ariadne/graphql.py b/ariadne/graphql.py index 4db4af080..36e4498c6 100644 --- a/ariadne/graphql.py +++ b/ariadne/graphql.py @@ -174,7 +174,6 @@ async def graphql( enable_introspection=introspection, query_validator=query_validator, ) - if validation_errors: return handle_graphql_errors( validation_errors, @@ -184,7 +183,7 @@ async def graphql( extension_manager=extension_manager, ) - if require_query and not validation_errors: + if require_query: validate_operation_is_query(document, operation_name) if callable(root_value): @@ -246,6 +245,7 @@ def graphql_sync( introspection: bool = True, logger: Union[None, str, Logger, LoggerAdapter] = None, validation_rules: Optional[ValidationRules] = None, + require_query: bool = False, error_formatter: ErrorFormatter = format_error, middleware: MiddlewareList = None, middleware_manager_class: Optional[Type[MiddlewareManager]] = None, @@ -298,6 +298,9 @@ def graphql_sync( `validation_rules`: a `list` of or callable returning list of custom validation rules to use to validate query before it's executed. + `require_query`: a `bool` controlling if GraphQL operation to execute must be + a query (vs. mutation or subscription). + `error_formatter`: an `ErrorFormatter` callable to use to convert GraphQL errors encountered during query execution to JSON-serializable format. @@ -353,6 +356,9 @@ def graphql_sync( extension_manager=extension_manager, ) + if require_query: + validate_operation_is_query(document, operation_name) + if callable(root_value): try: root_value = root_value( # type: ignore @@ -650,7 +656,9 @@ def validate_operation_name(operation_name) -> None: raise GraphQLError('"%s" is not a valid operation name.' % operation_name) -def validate_operation_is_query(document_ast: DocumentNode, operation_name: str): +def validate_operation_is_query( + document_ast: DocumentNode, operation_name: Optional[str] +): query_operations: List[Optional[str]] = [] for definition in document_ast.definitions: if ( @@ -665,8 +673,8 @@ def validate_operation_is_query(document_ast: DocumentNode, operation_name: str) if operation_name: if operation_name not in query_operations: raise GraphQLError( - f"Operation '{operation_name}' can't be executed using the GET " - "HTTP method. Use POST instead." + f"Operation '{operation_name}' is not defined or " + "is not of a 'query' type." ) elif len(query_operations) != 1: raise GraphQLError( diff --git a/ariadne/wsgi.py b/ariadne/wsgi.py index ead0bb46e..6e1ae3186 100644 --- a/ariadne/wsgi.py +++ b/ariadne/wsgi.py @@ -1,6 +1,7 @@ import json from inspect import isawaitable from typing import Any, Callable, Dict, List, Optional, Type, Union, cast +from urllib.parse import parse_qsl from graphql import ( ExecutionContext, @@ -75,6 +76,7 @@ def __init__( explorer: Optional[Explorer] = None, logger: Optional[str] = None, error_formatter: ErrorFormatter = format_error, + execute_get_queries: bool = False, extensions: Optional[Extensions] = None, middleware: Optional[Middlewares] = None, middleware_manager_class: Optional[Type[MiddlewareManager]] = None, @@ -125,6 +127,9 @@ def __init__( GraphQL errors returned to clients. If not set, default formatter implemented by Ariadne is used. + `execute_get_queries`: a `bool` that controls if `query` operations + sent using the `GET` method should be executed. Defaults to `False`. + `extensions`: an `Extensions` list or callable returning a list of extensions server should use during query execution. Defaults to no extensions. @@ -152,6 +157,7 @@ def __init__( self.introspection = introspection self.logger = logger self.error_formatter = error_formatter + self.execute_get_queries = execute_get_queries self.extensions = extensions self.middleware = middleware self.middleware_manager_class = middleware_manager_class or MiddlewareManager @@ -234,7 +240,7 @@ def handle_request(self, environ: dict, start_response: Callable) -> List[bytes] `start_response`: a callable used to begin new HTTP response. """ - if environ["REQUEST_METHOD"] == "GET" and self.introspection: + if environ["REQUEST_METHOD"] == "GET": return self.handle_get(environ, start_response) if environ["REQUEST_METHOD"] == "POST": return self.handle_post(environ, start_response) @@ -242,7 +248,62 @@ def handle_request(self, environ: dict, start_response: Callable) -> List[bytes] return self.handle_not_allowed_method(environ, start_response) def handle_get(self, environ: dict, start_response) -> List[bytes]: - """Handles WSGI HTTP GET request and returns a a response to the client. + """Handles WSGI HTTP GET request and returns a response to the client. + + Returns list of bytes with response body. + + # Required arguments + + `environ`: a WSGI environment dictionary. + + `start_response`: a callable used to begin new HTTP response. + """ + query_params = parse_query_string(environ) + if self.execute_get_queries and query_params and query_params.get("query"): + return self.handle_get_query(environ, start_response, query_params) + if self.introspection: + return self.handle_get_explorer(environ, start_response) + + return self.handle_not_allowed_method(environ, start_response) + + def handle_get_query( + self, environ: dict, start_response, query_params: dict + ) -> List[bytes]: + data = self.extract_data_from_get(query_params) + result = self.execute_query(environ, data) + return self.return_response_from_result(start_response, result) + + def extract_data_from_get(self, query_params: dict) -> dict: + """Extracts GraphQL data from GET request's querystring. + + Returns a `dict` with GraphQL query data that was not yet validated. + + # Required arguments + + `query_params`: a `dict` with parsed query string. + """ + query = query_params["query"].strip() + operation_name = query_params.get("operationName", "").strip() + variables = query_params.get("variables", "").strip() + + clean_variables = None + + if variables: + try: + clean_variables = json.loads(variables) + except (TypeError, ValueError, json.JSONDecodeError) as ex: + raise HttpBadRequestError( + "Variables query arg is not a valid JSON" + ) from ex + + return { + "query": query, + "operationName": operation_name or None, + "variables": clean_variables, + } + + def handle_get_explorer(self, environ: dict, start_response) -> List[bytes]: + """Handles WSGI HTTP GET explorer request and returns a response to the client. Returns list of bytes with response body. @@ -413,6 +474,7 @@ def execute_query(self, environ: dict, data: dict) -> GraphQLResult: query_parser=self.query_parser, query_validator=self.query_validator, validation_rules=self.validation_rules, + require_query=environ["REQUEST_METHOD"] == "GET", debug=self.debug, introspection=self.introspection, logger=self.logger, @@ -588,6 +650,17 @@ def __call__(self, environ: dict, start_response: Callable) -> List[bytes]: return self.graphql_app(environ, start_response) +def parse_query_string(environ: dict) -> Optional[dict]: + query_string = environ.get("QUERY_STRING") + if not query_string: + return None + + try: + return {k: v for k, v in parse_qsl(query_string)} + except (TypeError, ValueError): + return None + + def parse_multipart_request(environ: dict) -> "FormData": content_type = environ.get("CONTENT_TYPE") headers = {"Content-Type": content_type} diff --git a/tests/wsgi/test_configuration.py b/tests/wsgi/test_configuration.py index 67f85e75e..583fb135a 100644 --- a/tests/wsgi/test_configuration.py +++ b/tests/wsgi/test_configuration.py @@ -27,21 +27,27 @@ def __init__(self, app): def test_custom_context_value_is_passed_to_resolvers(schema): app = GraphQL(schema, context_value={"test": "TEST-CONTEXT"}) - _, result = app.execute_query({}, {"query": "{ testContext }"}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ testContext }"}, + ) assert result == {"data": {"testContext": "TEST-CONTEXT"}} def test_custom_context_value_function_is_set_and_called_by_app(schema): get_context_value = Mock(return_value=True) app = GraphQL(schema, context_value=get_context_value) - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) get_context_value.assert_called_once() def test_custom_context_value_function_is_called_with_request_value(schema): get_context_value = Mock(return_value=True) app = GraphQL(schema, context_value=get_context_value) - request = {"CONTENT_TYPE": DATA_TYPE_JSON} + request = {"CONTENT_TYPE": DATA_TYPE_JSON, "REQUEST_METHOD": "POST"} app.execute_query(request, {"query": "{ status }"}) get_context_value.assert_called_once_with(request, {"query": "{ status }"}) @@ -49,7 +55,10 @@ def test_custom_context_value_function_is_called_with_request_value(schema): def test_custom_context_value_function_result_is_passed_to_resolvers(schema): get_context_value = Mock(return_value={"test": "TEST-CONTEXT"}) app = GraphQL(schema, context_value=get_context_value) - _, result = app.execute_query({}, {"query": "{ testContext }"}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ testContext }"}, + ) assert result == {"data": {"testContext": "TEST-CONTEXT"}} @@ -62,19 +71,28 @@ def get_context_value(request): app = GraphQL(schema, context_value=get_context_value) with pytest.deprecated_call(): - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) def test_custom_root_value_is_passed_to_resolvers(schema): app = GraphQL(schema, root_value={"test": "TEST-ROOT"}) - _, result = app.execute_query({}, {"query": "{ testRoot }"}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ testRoot }"}, + ) assert result == {"data": {"testRoot": "TEST-ROOT"}} def test_custom_root_value_function_is_set_and_called_by_app(schema): get_root_value = Mock(return_value=True) app = GraphQL(schema, root_value=get_root_value) - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) get_root_value.assert_called_once() @@ -83,7 +101,10 @@ def test_custom_root_value_function_is_called_with_context_value(schema): app = GraphQL( schema, context_value={"test": "TEST-CONTEXT"}, root_value=get_root_value ) - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) get_root_value.assert_called_once_with({"test": "TEST-CONTEXT"}, None, None, ANY) @@ -98,13 +119,19 @@ def get_root_value(_context, _document): ) with pytest.deprecated_call(): - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) def test_custom_query_parser_is_used(schema): mock_parser = Mock(return_value=parse("{ status }")) app = GraphQL(schema, query_parser=mock_parser) - _, result = app.execute_query({}, {"query": "{ testContext }"}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ testContext }"}, + ) assert result == {"data": {"status": True}} mock_parser.assert_called_once() @@ -119,7 +146,10 @@ def test_custom_query_parser_is_used(schema): def test_custom_query_validator_is_used(schema, errors): mock_validator = Mock(return_value=errors) app = GraphQL(schema, query_validator=mock_validator) - _, result = app.execute_query({}, {"query": "{ testContext }"}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ testContext }"}, + ) if errors: assert result == {"errors": [{"message": "Nope"}]} else: @@ -132,7 +162,10 @@ def test_custom_validation_rule_is_called_by_query_validation( ): spy_validation_rule = mocker.spy(validation_rule, "__init__") app = GraphQL(schema, validation_rules=[validation_rule]) - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) spy_validation_rule.assert_called_once() @@ -142,7 +175,10 @@ def test_custom_validation_rules_function_is_set_and_called_on_query_execution( spy_validation_rule = mocker.spy(validation_rule, "__init__") get_validation_rules = Mock(return_value=[validation_rule]) app = GraphQL(schema, validation_rules=get_validation_rules) - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) get_validation_rules.assert_called_once() spy_validation_rule.assert_called_once() @@ -156,10 +192,131 @@ def test_custom_validation_rules_function_is_called_with_context_value( context_value={"test": "TEST-CONTEXT"}, validation_rules=get_validation_rules, ) - app.execute_query({}, {"query": "{ status }"}) + app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": "{ status }"}, + ) get_validation_rules.assert_called_once_with({"test": "TEST-CONTEXT"}, ANY, ANY) +def test_query_over_get_is_executed_if_enabled(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=True) + response = app( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": "query={status}", + }, + send_response + ) + send_response.assert_called_once_with( + "200 OK", [("Content-Type", "application/json; charset=UTF-8")] + ) + assert json.loads(response[0]) == {"data": {"status": True}} + + +def test_query_over_get_is_executed_with_variables(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=True) + response = app( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": ( + "query=query Hello($name:String) {hello(name: $name)}" + "&operationName=Hello" + "&variables={\"name\": \"John\"}" + ), + }, + send_response + ) + send_response.assert_called_once_with( + "200 OK", [("Content-Type", "application/json; charset=UTF-8")] + ) + assert json.loads(response[0]) == {"data": {"hello": "Hello, John!"}} + + +def test_query_over_get_is_executed_without_operation_name(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=True) + response = app( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": ( + "query=query Hello($name:String) {hello(name: $name)}" + "&variables={\"name\": \"John\"}" + ), + }, + send_response + ) + send_response.assert_called_once_with( + "200 OK", [("Content-Type", "application/json; charset=UTF-8")] + ) + assert json.loads(response[0]) == {"data": {"hello": "Hello, John!"}} + + +def test_query_over_get_fails_if_operation_name_is_invalid(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=True) + response = app( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": ( + "query=query Hello($name:String) {hello(name: $name)}" + "&operationName=Invalid" + "&variables={\"name\": \"John\"}" + ), + }, + send_response + ) + send_response.assert_called_once_with( + "400 Bad Request", [("Content-Type", "application/json; charset=UTF-8")] + ) + assert json.loads(response[0]) == { + "errors": [ + { + "message": ( + "Operation 'Invalid' is not defined or is not of a 'query' type." + ) + } + ] + } + + +def test_query_over_get_fails_if_variables_are_not_json_serialized(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=True) + response = app( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": ( + "query=query Hello($name:String) {hello(name: $name)}" + "&operationName=Hello" + "&variables={\"name\" \"John\"}" + ), + }, + send_response + ) + send_response.assert_called_once_with( + "400 Bad Request", [("Content-Type", "text/plain; charset=UTF-8")] + ) + assert response[0] == b"Variables query arg is not a valid JSON" + + +def test_query_over_get_is_not_executed_if_not_enabled(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=False) + app.handle_request( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": "query={status}", + }, + send_response + ) + send_response.assert_called_once_with( + "200 OK", [("Content-Type", "text/html; charset=UTF-8")] + ) + + def execute_failing_query(app): data = json.dumps({"query": "{ error }"}) app( @@ -244,7 +401,10 @@ def middleware(next_fn, *args, **kwargs): def test_middlewares_are_passed_to_query_executor(schema): app = GraphQL(schema, middleware=[middleware]) - _, result = app.execute_query({}, {"query": '{ hello(name: "BOB") }'}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": '{ hello(name: "BOB") }'}, + ) assert result == {"data": {"hello": "**Hello, BOB!**"}} @@ -253,7 +413,10 @@ def get_middleware(*_): return [middleware] app = GraphQL(schema, middleware=get_middleware) - _, result = app.execute_query({}, {"query": '{ hello(name: "BOB") }'}) + _, result = app.execute_query( + {"REQUEST_METHOD": "POST"}, + {"query": '{ hello(name: "BOB") }'}, + ) assert result == {"data": {"hello": "**Hello, BOB!**"}} From 70662dea13655ebb067f54f8fa9c64aca56b281b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 18:14:14 +0100 Subject: [PATCH 07/11] Fix linter --- ariadne/wsgi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ariadne/wsgi.py b/ariadne/wsgi.py index 6e1ae3186..b11b92cc5 100644 --- a/ariadne/wsgi.py +++ b/ariadne/wsgi.py @@ -656,7 +656,7 @@ def parse_query_string(environ: dict) -> Optional[dict]: return None try: - return {k: v for k, v in parse_qsl(query_string)} + return dict(parse_qsl(query_string)) except (TypeError, ValueError): return None From f21ae7e8c8b54c398c2f6e4910eabc57ba71fceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 18:16:30 +0100 Subject: [PATCH 08/11] Format code with black --- tests/wsgi/test_configuration.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/wsgi/test_configuration.py b/tests/wsgi/test_configuration.py index 583fb135a..4506d3f0e 100644 --- a/tests/wsgi/test_configuration.py +++ b/tests/wsgi/test_configuration.py @@ -207,7 +207,7 @@ def test_query_over_get_is_executed_if_enabled(schema): "REQUEST_METHOD": "GET", "QUERY_STRING": "query={status}", }, - send_response + send_response, ) send_response.assert_called_once_with( "200 OK", [("Content-Type", "application/json; charset=UTF-8")] @@ -224,10 +224,10 @@ def test_query_over_get_is_executed_with_variables(schema): "QUERY_STRING": ( "query=query Hello($name:String) {hello(name: $name)}" "&operationName=Hello" - "&variables={\"name\": \"John\"}" + '&variables={"name": "John"}' ), }, - send_response + send_response, ) send_response.assert_called_once_with( "200 OK", [("Content-Type", "application/json; charset=UTF-8")] @@ -243,10 +243,10 @@ def test_query_over_get_is_executed_without_operation_name(schema): "REQUEST_METHOD": "GET", "QUERY_STRING": ( "query=query Hello($name:String) {hello(name: $name)}" - "&variables={\"name\": \"John\"}" + '&variables={"name": "John"}' ), }, - send_response + send_response, ) send_response.assert_called_once_with( "200 OK", [("Content-Type", "application/json; charset=UTF-8")] @@ -263,10 +263,10 @@ def test_query_over_get_fails_if_operation_name_is_invalid(schema): "QUERY_STRING": ( "query=query Hello($name:String) {hello(name: $name)}" "&operationName=Invalid" - "&variables={\"name\": \"John\"}" + '&variables={"name": "John"}' ), }, - send_response + send_response, ) send_response.assert_called_once_with( "400 Bad Request", [("Content-Type", "application/json; charset=UTF-8")] @@ -291,10 +291,10 @@ def test_query_over_get_fails_if_variables_are_not_json_serialized(schema): "QUERY_STRING": ( "query=query Hello($name:String) {hello(name: $name)}" "&operationName=Hello" - "&variables={\"name\" \"John\"}" + '&variables={"name" "John"}' ), }, - send_response + send_response, ) send_response.assert_called_once_with( "400 Bad Request", [("Content-Type", "text/plain; charset=UTF-8")] @@ -310,7 +310,7 @@ def test_query_over_get_is_not_executed_if_not_enabled(schema): "REQUEST_METHOD": "GET", "QUERY_STRING": "query={status}", }, - send_response + send_response, ) send_response.assert_called_once_with( "200 OK", [("Content-Type", "text/html; charset=UTF-8")] From e16a15524f086c4d2d4a7edcf1a358ad0862d228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 18:34:42 +0100 Subject: [PATCH 09/11] Add tests for query over get execution --- ariadne/asgi/graphql.py | 6 +++ ariadne/asgi/handlers/base.py | 3 ++ ariadne/asgi/handlers/http.py | 9 +--- tests/asgi/test_configuration.py | 86 ++++++++++++++++++++++++++++++++ tests/wsgi/test_configuration.py | 28 +++++++++++ 5 files changed, 125 insertions(+), 7 deletions(-) diff --git a/ariadne/asgi/graphql.py b/ariadne/asgi/graphql.py index 8114a9d7f..797ccb107 100644 --- a/ariadne/asgi/graphql.py +++ b/ariadne/asgi/graphql.py @@ -39,6 +39,7 @@ def __init__( query_parser: Optional[QueryParser] = None, query_validator: Optional[QueryValidator] = None, validation_rules: Optional[ValidationRules] = None, + execute_get_queries: bool = False, debug: bool = False, introspection: bool = True, explorer: Optional[Explorer] = None, @@ -73,6 +74,9 @@ def __init__( list of extra validation rules server should use to validate the GraphQL queries. Defaults to `None`. + `execute_get_queries`: a `bool` that controls if `query` operations + sent using the `GET` method should be executed. Defaults to `False`. + `debug`: a `bool` controlling in server should run in debug mode or not. Controls details included in error data returned to clients. Defaults to `False`. @@ -126,6 +130,7 @@ def __init__( query_parser, query_validator, validation_rules, + execute_get_queries, debug, introspection, explorer, @@ -140,6 +145,7 @@ def __init__( query_parser, query_validator, validation_rules, + execute_get_queries, debug, introspection, explorer, diff --git a/ariadne/asgi/handlers/base.py b/ariadne/asgi/handlers/base.py index 3d626ddc4..18f5fb554 100644 --- a/ariadne/asgi/handlers/base.py +++ b/ariadne/asgi/handlers/base.py @@ -40,6 +40,7 @@ def __init__(self) -> None: self.query_parser: Optional[QueryParser] = None self.query_validator: Optional[QueryValidator] = None self.validation_rules: Optional[ValidationRules] = None + self.execute_get_queries: bool = False self.execution_context_class: Optional[Type[ExecutionContext]] = None self.middleware_manager_class: Optional[Type[MiddlewareManager]] = None @@ -79,6 +80,7 @@ def configure( query_parser: Optional[QueryParser] = None, query_validator: Optional[QueryValidator] = None, validation_rules: Optional[ValidationRules] = None, + execute_get_queries: bool = False, debug: bool = False, introspection: bool = True, explorer: Optional[Explorer] = None, @@ -94,6 +96,7 @@ def configure( self.context_value = context_value self.debug = debug self.error_formatter = error_formatter + self.execute_get_queries = execute_get_queries self.execution_context_class = execution_context_class self.introspection = introspection self.explorer = explorer diff --git a/ariadne/asgi/handlers/http.py b/ariadne/asgi/handlers/http.py index dca234eb8..b00ee114e 100644 --- a/ariadne/asgi/handlers/http.py +++ b/ariadne/asgi/handlers/http.py @@ -35,7 +35,6 @@ class GraphQLHTTPHandler(GraphQLHttpHandlerBase): def __init__( self, - execute_get_queries: bool = False, extensions: Optional[Extensions] = None, middleware: Optional[Middlewares] = None, middleware_manager_class: Optional[Type[MiddlewareManager]] = None, @@ -44,9 +43,6 @@ def __init__( # Optional arguments - `execute_get_queries`: a `bool` that controls if `query` operations - sent using the `GET` method should be executed. Defaults to `False`. - `extensions`: an `Extensions` list or callable returning a list of extensions server should use during query execution. Defaults to no extensions. @@ -62,7 +58,6 @@ def __init__( """ super().__init__() - self.execute_get_queries = execute_get_queries self.extensions = extensions self.middleware = middleware self.middleware_manager_class = middleware_manager_class or MiddlewareManager @@ -195,7 +190,7 @@ async def extract_data_from_request(self, request: Request): and self.execute_get_queries and request.query_params.get("query") ): - return await self.extract_data_from_get_request(request) + return self.extract_data_from_get_request(request) raise HttpBadRequestError( "Posted content must be of type {} or {}".format( @@ -256,7 +251,7 @@ async def extract_data_from_multipart_request( return combine_multipart_data(operations, files_map, request_files) - async def extract_data_from_get_request(self, request: Request) -> dict: + def extract_data_from_get_request(self, request: Request) -> dict: """Extracts GraphQL data from GET request's querystring. Returns a `dict` with GraphQL query data that was not yet validated. diff --git a/tests/asgi/test_configuration.py b/tests/asgi/test_configuration.py index eb7e1a8ed..e06e435fa 100644 --- a/tests/asgi/test_configuration.py +++ b/tests/asgi/test_configuration.py @@ -317,6 +317,92 @@ def test_custom_validation_rules_function_is_called_with_context_value( get_validation_rules.assert_called_once_with({"test": "TEST-CONTEXT"}, ANY, ANY) +def test_query_over_get_is_executed_if_enabled(schema): + app = GraphQL(schema, execute_get_queries=True) + client = TestClient(app) + response = client.get("/?query={status}") + assert response.json() == {"data": {"status": True}} + + +def test_query_over_get_is_executed_with_variables(schema): + app = GraphQL(schema, execute_get_queries=True) + client = TestClient(app) + response = client.get( + "/?query=query Hello($name:String) {hello(name: $name)}" + "&operationName=Hello" + '&variables={"name": "John"}' + ) + assert response.json() == {"data": {"hello": "Hello, John!"}} + + +def test_query_over_get_is_executed_without_operation_name(schema): + app = GraphQL(schema, execute_get_queries=True) + client = TestClient(app) + response = client.get( + "/?query=query Hello($name:String) {hello(name: $name)}" + '&variables={"name": "John"}' + ) + assert response.json() == {"data": {"hello": "Hello, John!"}} + + +def test_query_over_get_fails_if_operation_name_is_invalid(schema): + app = GraphQL(schema, execute_get_queries=True) + client = TestClient(app) + response = client.get( + "/?query=query Hello($name:String) {hello(name: $name)}" + "&operationName=Invalid" + '&variables={"name": "John"}' + ) + assert response.json() == { + "errors": [ + { + "message": ( + "Operation 'Invalid' is not defined or is not of a 'query' type." + ) + } + ] + } + + +def test_query_over_get_fails_if_operation_is_mutation(schema): + app = GraphQL(schema, execute_get_queries=True) + client = TestClient(app) + response = client.get( + "/?query=mutation Echo($text:String!) {echo(text: $text)}" + "&operationName=Echo" + '&variables={"text": "John"}' + ) + assert response.json() == { + "errors": [ + { + "message": ( + "Operation 'Echo' is not defined or is not of a 'query' type." + ) + } + ] + } + + +def test_query_over_get_fails_if_variables_are_not_json_serialized(schema): + app = GraphQL(schema, execute_get_queries=True) + client = TestClient(app) + response = client.get( + "/?query=query Hello($name:String) {hello(name: $name)}" + "&operationName=Hello" + '&variables={"name" "John"}' + ) + assert response.status_code == 400 + assert response.content == b"Variables query arg is not a valid JSON" + + +def test_query_over_get_is_not_executed_if_not_enabled(schema): + app = GraphQL(schema, execute_get_queries=False) + client = TestClient(app) + response = client.get("/?query={ status }") + assert response.status_code == 200 + assert response.headers["CONTENT-TYPE"] == "text/html; charset=utf-8" + + def execute_failing_query(app): client = TestClient(app) client.post("/", json={"query": "{ error }"}) diff --git a/tests/wsgi/test_configuration.py b/tests/wsgi/test_configuration.py index 4506d3f0e..aaeb874a2 100644 --- a/tests/wsgi/test_configuration.py +++ b/tests/wsgi/test_configuration.py @@ -282,6 +282,34 @@ def test_query_over_get_fails_if_operation_name_is_invalid(schema): } +def test_query_over_get_fails_if_operation_is_mutation(schema): + send_response = Mock() + app = GraphQL(schema, execute_get_queries=True) + response = app( + { + "REQUEST_METHOD": "GET", + "QUERY_STRING": ( + "query=mutation Echo($text:String!) {echo(text: $text)}" + "&operationName=Echo" + '&variables={"text": "John"}' + ), + }, + send_response, + ) + send_response.assert_called_once_with( + "400 Bad Request", [("Content-Type", "application/json; charset=UTF-8")] + ) + assert json.loads(response[0]) == { + "errors": [ + { + "message": ( + "Operation 'Echo' is not defined or is not of a 'query' type." + ) + } + ] + } + + def test_query_over_get_fails_if_variables_are_not_json_serialized(schema): send_response = Mock() app = GraphQL(schema, execute_get_queries=True) From 0c90a8f94cb36d9ff1e71c961964b32be325d0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 18:39:11 +0100 Subject: [PATCH 10/11] Add changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84039c460..1fad9c58f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # CHANGELOG +## 0.23 (UNRELEASED) + +- Added `execute_get_queries` option to the `GraphQL` apps that controls execution of the GraphQL "query" type operations made with GET requests. Defaults to `False`. +- Added support for the Apollo Federation versions up to 2.6. + + ## 0.22 (2024-01-31) - Deprecated `EnumType.bind_to_default_values` method. It will be removed in a future release. From 2cd2d01dffdb953fe194d7f095b34a4e982d55a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Pito=C5=84?= Date: Mon, 19 Feb 2024 18:40:19 +0100 Subject: [PATCH 11/11] Reword changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fad9c58f..829c70dca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.23 (UNRELEASED) -- Added `execute_get_queries` option to the `GraphQL` apps that controls execution of the GraphQL "query" type operations made with GET requests. Defaults to `False`. +- Added `execute_get_queries` setting to the `GraphQL` apps that controls execution of the GraphQL "query" operations made with GET requests. Defaults to `False`. - Added support for the Apollo Federation versions up to 2.6.