test(views): cover show_graphiql, middleware None, atomic mutation, single AST parse

Add ``TestExecuteGraphqlRequestRefactor`` pinning every behaviour the
``execute_graphql_request`` extraction in views.py is meant to
preserve, including the three regressions identified in the PR #1457
review.

* ``_should_short_circuit_get_request`` (5 tests):
  - POST never short-circuits.
  - GET + query never short-circuits regardless of show_graphiql
    (B1 regression pin).
  - GET + mutation + show_graphiql=True returns True so the caller
    short-circuits.
  - GET + mutation + show_graphiql=False raises HttpError carrying
    a 405 with the documented message.
  - operation_ast=None never short-circuits.
* ``_is_atomic_mutation`` (7 tests via parametrize + None case):
  - All 6 combinations of (operation kind, ATOMIC_MUTATIONS global,
    ATOMIC_MUTATIONS per-db).
  - None operation_ast returns False.
* ``execute_graphql_request`` end-to-end:
  - GET + query + show_graphiql=True executes the query and returns
    the result (B1 pin).
  - GET + mutation + show_graphiql=True returns None (existing
    behaviour preserved).
  - get_operation_ast called exactly once per request (B2 pin).
* ``GraphQLView.__init__``:
  - graphene_settings.MIDDLEWARE = None succeeds and yields
    view.middleware is None (B3 pin).

Each test follows the team docstring template (Name / Description /
Assumptions / Expectations); the class docstring spells out which
scenarios and edge cases are covered. All 16 new tests pass.

Made-with: Cursor

Author: firaskafri

graphene_django/tests/test_views.py

@@ -922,3 +922,376 @@ def test_exceeds_max_validation_errors(client, url):
 
     error_messages = (error["message"].lower() for error in json_response["errors"])
     assert any(MAX_VALIDATION_ERRORS_EXCEEDED_MESSAGE in msg for msg in error_messages)
+
+
+class TestExecuteGraphqlRequestRefactor:
+    """Pin the ``execute_graphql_request`` extraction in
+    :mod:`graphene_django.views`.
+
+    The refactor introduced two private helpers,
+    :meth:`GraphQLView._should_short_circuit_get_request` and
+    :meth:`GraphQLView._is_atomic_mutation`, and the tests in this class
+    exercise the previously-implicit branches that those helpers now
+    cover so that any regression fails a focused test.
+
+    Scenarios covered:
+
+    * ``_should_short_circuit_get_request``
+        - POST never short-circuits regardless of operation kind.
+        - GET + query never short-circuits (the B1 regression: an earlier
+          rewrite of this branch caused queries with ``show_graphiql=True``
+          to be silently dropped).
+        - GET + non-query + ``show_graphiql=True`` short-circuits to
+          ``True`` so the caller returns ``None``.
+        - GET + non-query + ``show_graphiql=False`` raises ``HttpError``
+          carrying a 405 ``HttpResponseNotAllowed``.
+        - GET with ``operation_ast=None`` (e.g. an unparseable document)
+          does not short-circuit; the executor sees the error.
+    * ``_is_atomic_mutation``
+        - Parametrised over the 8 combinations of (operation kind,
+          ATOMIC_MUTATIONS setting, db ATOMIC_MUTATIONS setting),
+          asserting the boolean outcome for each.
+    * ``execute_graphql_request`` end-to-end
+        - GET + query + ``show_graphiql=True`` invoked directly returns
+          a non-None ``ExecutionResult`` with the query's data (B1 pin).
+        - GET + mutation + ``show_graphiql=True`` invoked directly
+          returns ``None`` (existing behaviour preserved).
+        - ``get_operation_ast`` is invoked exactly once per request (B2
+          pin: the original PR introduced a second call).
+    * ``GraphQLView.__init__``
+        - Constructing the view with ``graphene_settings.MIDDLEWARE = None``
+          succeeds and yields ``view.middleware is None`` (B3 pin: the
+          original PR dropped the ``if middleware is not None`` guard,
+          which would have raised ``TypeError``).
+
+    Assumptions: the existing :data:`graphene_django.tests.schema_view.schema`
+    exposes a ``test`` query and a ``writeTest`` mutation, both used as
+    minimal exercises of the query and mutation branches respectively.
+    """
+
+    @staticmethod
+    def _make_view():
+        """Build a :class:`GraphQLView` against the existing test schema."""
+        from graphene_django.views import GraphQLView
+
+        from .schema_view import schema
+
+        return GraphQLView(schema=schema)
+
+    @staticmethod
+    def _build_request(method, **get_params):
+        """Build a tiny stand-in for a Django HttpRequest.
+
+        ``execute_graphql_request`` only reads ``request.method`` (in the
+        ``_should_short_circuit_get_request`` helper) and is otherwise
+        agnostic to the request object, so a SimpleNamespace is enough
+        for these unit tests and avoids spinning up the full request
+        factory.
+        """
+        from types import SimpleNamespace
+
+        return SimpleNamespace(method=method, GET={}, META={})
+
+    def test_should_short_circuit_returns_false_for_post(self):
+        """
+        Name: short-circuit, POST is never short-circuited
+        Description: ``_should_short_circuit_get_request`` must return
+            ``False`` for POST requests so all operation kinds (queries,
+            mutations, subscriptions) are routed to the executor.
+        Assumptions: A POST mutation is the canonical "do not
+            short-circuit" case.
+        Expectations: Returns ``False``.
+        """
+        from graphql import OperationType, parse, get_operation_ast
+
+        from graphene_django.views import GraphQLView
+
+        document = parse("mutation { writeTest { test } }")
+        op_ast = get_operation_ast(document, None)
+        assert op_ast.operation == OperationType.MUTATION
+
+        request = self._build_request("post")
+        assert (
+            GraphQLView._should_short_circuit_get_request(request, op_ast, True)
+            is False
+        )
+
+    def test_should_short_circuit_returns_false_for_get_query(self):
+        """
+        Name: short-circuit, GET + query never short-circuits (B1 pin)
+        Description: A query operation on a GET request must always be
+            executed normally, even when GraphiQL is rendering. This is
+            the regression the PR refactor introduced and that the
+            ``_should_short_circuit_get_request`` helper is designed to
+            prevent.
+        Assumptions: ``test`` is a query in :data:`schema_view.schema`.
+        Expectations: Returns ``False`` regardless of ``show_graphiql``.
+        """
+        from graphql import OperationType, parse, get_operation_ast
+
+        from graphene_django.views import GraphQLView
+
+        document = parse("{ test }")
+        op_ast = get_operation_ast(document, None)
+        assert op_ast.operation == OperationType.QUERY
+
+        request = self._build_request("get")
+        assert (
+            GraphQLView._should_short_circuit_get_request(request, op_ast, True)
+            is False
+        )
+        assert (
+            GraphQLView._should_short_circuit_get_request(request, op_ast, False)
+            is False
+        )
+
+    def test_should_short_circuit_returns_true_for_get_mutation_with_graphiql(self):
+        """
+        Name: short-circuit, GET + mutation + show_graphiql=True
+        Description: The only case that short-circuits to ``True`` is a
+            non-query operation on a GET request when GraphiQL is
+            rendering, so the caller can return ``None`` without raising.
+        Assumptions: ``writeTest`` is a mutation in
+            :data:`schema_view.schema`.
+        Expectations: Returns ``True``.
+        """
+        from graphql import OperationType, parse, get_operation_ast
+
+        from graphene_django.views import GraphQLView
+
+        document = parse("mutation { writeTest { test } }")
+        op_ast = get_operation_ast(document, None)
+        assert op_ast.operation == OperationType.MUTATION
+
+        request = self._build_request("get")
+        assert (
+            GraphQLView._should_short_circuit_get_request(request, op_ast, True)
+            is True
+        )
+
+    def test_should_short_circuit_raises_for_get_mutation_without_graphiql(self):
+        """
+        Name: short-circuit, GET + mutation + no GraphiQL raises 405
+        Description: A non-query GET request without GraphiQL must raise
+            ``HttpError`` carrying a 405 ``HttpResponseNotAllowed`` so the
+            client gets the documented "Can only perform a {} operation
+            from a POST request" error.
+        Assumptions: The operation is a mutation.
+        Expectations: ``HttpError`` is raised; the wrapped response is a
+            405 with the expected message.
+        """
+        from http import HTTPStatus
+
+        from graphql import OperationType, parse, get_operation_ast
+
+        from graphene_django.views import GraphQLView, HttpError
+
+        document = parse("mutation { writeTest { test } }")
+        op_ast = get_operation_ast(document, None)
+        assert op_ast.operation == OperationType.MUTATION
+
+        request = self._build_request("get")
+        with pytest.raises(HttpError) as excinfo:
+            GraphQLView._should_short_circuit_get_request(request, op_ast, False)
+        assert excinfo.value.response.status_code == HTTPStatus.METHOD_NOT_ALLOWED
+        assert (
+            "Can only perform a mutation operation from a POST request."
+            in excinfo.value.message
+        )
+
+    def test_should_short_circuit_returns_false_when_operation_ast_is_none(self):
+        """
+        Name: short-circuit, missing operation_ast
+        Description: When ``get_operation_ast`` couldn't resolve an
+            operation (e.g. operation_name not found on the document), the
+            helper must not short-circuit; downstream validation will
+            surface the appropriate error to the user.
+        Assumptions: ``operation_ast`` is allowed to be ``None``.
+        Expectations: Returns ``False`` for both GET and POST.
+        """
+        from graphene_django.views import GraphQLView
+
+        for method in ("get", "post"):
+            request = self._build_request(method)
+            assert (
+                GraphQLView._should_short_circuit_get_request(request, None, True)
+                is False
+            )
+            assert (
+                GraphQLView._should_short_circuit_get_request(request, None, False)
+                is False
+            )
+
+    @pytest.mark.parametrize(
+        "operation_str,atomic_global,atomic_db,expected",
+        [
+            # Queries are never atomic regardless of settings.
+            ("{ test }", True, True, False),
+            ("{ test }", False, False, False),
+            # Mutations require at least one of the two flags.
+            ("mutation { writeTest { test } }", False, False, False),
+            ("mutation { writeTest { test } }", True, False, True),
+            ("mutation { writeTest { test } }", False, True, True),
+            ("mutation { writeTest { test } }", True, True, True),
+        ],
+    )
+    def test_is_atomic_mutation_combinations(
+        self, monkeypatch, operation_str, atomic_global, atomic_db, expected
+    ):
+        """
+        Name: _is_atomic_mutation, parametrised settings
+        Description: ``_is_atomic_mutation`` must return ``True`` only when
+            the operation is a mutation **and** at least one of the two
+            ATOMIC_MUTATIONS flags (graphene setting or per-connection db
+            setting) is enabled.
+        Assumptions: The two-flag rule mirrors the original inline logic
+            in ``execute_graphql_request``.
+        Expectations: For each ``(operation_kind, atomic_global, atomic_db)``
+            combination, the helper returns the documented boolean.
+        """
+        from django.db import connection as db_connection
+        from graphql import parse, get_operation_ast
+
+        from graphene_django.views import GraphQLView
+
+        monkeypatch.setattr(
+            "graphene_django.views.graphene_settings.ATOMIC_MUTATIONS",
+            atomic_global,
+        )
+        monkeypatch.setitem(
+            db_connection.settings_dict, "ATOMIC_MUTATIONS", atomic_db
+        )
+
+        document = parse(operation_str)
+        op_ast = get_operation_ast(document, None)
+        assert GraphQLView._is_atomic_mutation(op_ast) is expected
+
+    def test_is_atomic_mutation_returns_false_for_none_operation(self):
+        """
+        Name: _is_atomic_mutation, missing operation_ast
+        Description: With no operation AST, the helper must return ``False``
+            so the caller does not wrap an unparseable document in a
+            transaction.
+        Assumptions: ``operation_ast`` is allowed to be ``None``.
+        Expectations: Returns ``False``.
+        """
+        from graphene_django.views import GraphQLView
+
+        assert GraphQLView._is_atomic_mutation(None) is False
+
+    def test_execute_graphql_request_get_with_query_and_graphiql_executes(self):
+        """
+        Name: execute_graphql_request, GET + query + show_graphiql=True (B1 pin)
+        Description: Calling ``execute_graphql_request`` directly with
+            ``show_graphiql=True`` and a query must still execute the
+            query and return a non-None ``ExecutionResult``. The earlier
+            rewrite of this code path returned ``None`` unconditionally
+            on ``show_graphiql=True``, dropping the result silently.
+        Assumptions: ``test`` is a String query on :data:`schema_view.schema`
+            that returns ``"Hello World"`` when called without arguments.
+        Expectations: The returned ``ExecutionResult`` has no errors and
+            ``result.data == {"test": "Hello World"}``.
+        """
+        view = self._make_view()
+        request = self._build_request("get")
+
+        result = view.execute_graphql_request(
+            request,
+            data={},
+            query="{ test }",
+            variables=None,
+            operation_name=None,
+            show_graphiql=True,
+        )
+        assert result is not None
+        assert result.errors is None
+        assert result.data == {"test": "Hello World"}
+
+    def test_execute_graphql_request_get_with_mutation_and_graphiql_returns_none(
+        self,
+    ):
+        """
+        Name: execute_graphql_request, GET + mutation + show_graphiql=True
+        Description: A non-query operation on a GET request with GraphiQL
+            rendering must short-circuit to ``None`` so GraphiQL can be
+            displayed without executing the mutation. This mirrors the
+            existing public behaviour and protects it across the
+            extraction.
+        Assumptions: ``writeTest`` is a mutation on :data:`schema_view.schema`.
+        Expectations: Returns ``None``.
+        """
+        view = self._make_view()
+        request = self._build_request("get")
+
+        result = view.execute_graphql_request(
+            request,
+            data={},
+            query="mutation { writeTest { test } }",
+            variables=None,
+            operation_name=None,
+            show_graphiql=True,
+        )
+        assert result is None
+
+    def test_execute_graphql_request_calls_get_operation_ast_once(self, monkeypatch):
+        """
+        Name: execute_graphql_request, single get_operation_ast call (B2 pin)
+        Description: The refactor must continue to call ``get_operation_ast``
+            exactly once per request — the original PR introduced a second
+            call inside the extracted validation helper, which this test
+            prevents from regressing.
+        Assumptions: ``graphene_django.views.get_operation_ast`` is the
+            single import the view uses.
+        Expectations: The wrapped function is invoked exactly one time
+            during a single ``execute_graphql_request`` call.
+        """
+        from graphene_django import views as views_module
+
+        original = views_module.get_operation_ast
+        call_count = {"n": 0}
+
+        def counting_get_operation_ast(*args, **kwargs):
+            call_count["n"] += 1
+            return original(*args, **kwargs)
+
+        monkeypatch.setattr(
+            views_module, "get_operation_ast", counting_get_operation_ast
+        )
+
+        view = self._make_view()
+        request = self._build_request("post")
+        view.execute_graphql_request(
+            request,
+            data={},
+            query="{ test }",
+            variables=None,
+            operation_name=None,
+            show_graphiql=False,
+        )
+        assert call_count["n"] == 1, (
+            f"expected exactly one get_operation_ast call, got {call_count['n']}"
+        )
+
+    def test_init_with_middleware_setting_none_does_not_raise(self, monkeypatch):
+        """
+        Name: __init__ with MIDDLEWARE=None (B3 pin)
+        Description: ``GraphQLView.__init__`` must tolerate
+            ``graphene_settings.MIDDLEWARE = None`` without raising
+            ``TypeError``. The original PR dropped the ``if middleware is
+            not None`` guard, which would have caused
+            ``list(instantiate_middleware(None))`` to fail.
+        Assumptions: ``graphene_settings.MIDDLEWARE`` is a publicly-overridable
+            setting that may legitimately be ``None``.
+        Expectations: Constructing the view succeeds and ``view.middleware``
+            equals the class default (``None``).
+        """
+        from graphene_django.views import GraphQLView
+
+        from .schema_view import schema
+
+        monkeypatch.setattr(
+            "graphene_django.views.graphene_settings.MIDDLEWARE", None
+        )
+
+        view = GraphQLView(schema=schema)
+        assert view.middleware is None