Release: fix 500s on /api/auth/user/info/ and /api/search/aimodel/

.github/workflows/django-tests.yml

@@ -0,0 +1,46 @@
+name: Django Tests
+
+# The SDK workflows run a handful of tests with -p no:django, so nothing ever
+# exercised the Django side: views, schema, permissions. This runs that suite.
+on:
+  push:
+    branches: [dev, main]
+  pull_request:
+    branches: [dev, main]
+  workflow_call:
+
+permissions:
+  contents: read
+
+jobs:
+  django-tests:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+          cache: pip
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+          # Not in requirements.txt, which covers the running service only.
+          pip install pytest pytest-django psycopg2-binary setuptools
+
+      - name: Prepare environment
+        run: |
+          cp .env.example .env
+          mkdir -p logs
+
+      # tests/object_types/charts and the dataset metadata tests were left
+      # behind by the chart consolidation and fail on main: see #189.
+      - name: Run tests
+        run: |
+          pytest tests/ \
+            --ignore=tests/object_types/charts \
+            --deselect tests/schema/test_dataset_schema.py::TestAddUpdateDatasetMetadata

.github/workflows/pr-gate.yml

@@ -17,6 +17,11 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
+  # Unit tests for this repo's own code, alongside the browser/API suite.
+  django-tests:
+    name: Django Tests
+    uses: ./.github/workflows/django-tests.yml
+
   full-suite:
     name: Full Suite (dev)
     uses: CivicDataLab/CivicDataSpace-test/.github/workflows/run-smoke.yml@CI

api/views/auth.py

@@ -1,5 +1,5 @@
 from rest_framework import status, views
-from rest_framework.permissions import AllowAny
+from rest_framework.permissions import AllowAny, IsAuthenticated
 from rest_framework.request import Request
 from rest_framework.response import Response
 from rest_framework_simplejwt.tokens import RefreshToken
@@ -83,6 +83,11 @@ class UserInfoView(views.APIView):
     View for getting the current user's information.
     """
 
+    # Without this the project-wide AllowAny default lets an anonymous request
+    # through, and reading .email off AnonymousUser raises a 500 instead of
+    # returning 401.
+    permission_classes = [IsAuthenticated]
+
     def get(self, request: Request) -> Response:
         user = request.user
         return Response(

api/views/paginated_elastic_view.py

@@ -4,13 +4,33 @@
 from django.core.cache import cache
 from django.http import HttpRequest, HttpResponse
 from elasticsearch_dsl import Search
+from elasticsearch_dsl.utils import AttrDict, AttrList
 from rest_framework.permissions import AllowAny
 from rest_framework.response import Response
 from rest_framework.serializers import Serializer
 from rest_framework.views import APIView
 
 from api.signals.dataset_signals import SEARCH_CACHE_VERSION_KEY
 
+def as_plain_data(value: Any) -> Any:
+    """Convert Elasticsearch wrapper objects into plain Python containers.
+
+    Serialized hits keep AttrList/AttrDict/InnerDoc values for nested fields.
+    Those classes are rebuilt per document type, so pickling one raises
+    "it's not the same object as elasticsearch_dsl.document.InnerDoc" and the
+    cache write fails, turning the whole response into a 500.
+    """
+    if isinstance(value, AttrList):
+        return [as_plain_data(item) for item in value]
+    if isinstance(value, AttrDict):
+        return as_plain_data(value.to_dict())
+    if isinstance(value, dict):
+        return {key: as_plain_data(item) for key, item in value.items()}
+    if isinstance(value, (list, tuple)):
+        return [as_plain_data(item) for item in value]
+    return value
+
+
 T = TypeVar("T")
 SearchType = TypeVar("SearchType", bound=Search)
 SerializerType = TypeVar("SerializerType", bound=Serializer)
@@ -173,11 +193,13 @@ def get(self, request: HttpRequest) -> Response:
                 for agg in is_individual_usecase_agg:
                     aggregations["is_individual_usecase"][agg["key"]] = agg["doc_count"]
 
-            result: Dict[str, Any] = {
-                "results": serializer.data,
-                "total": response.hits.total.value,  # type: ignore
-                "aggregations": aggregations,
-            }
+            result: Dict[str, Any] = as_plain_data(
+                {
+                    "results": serializer.data,
+                    "total": response.hits.total.value,  # type: ignore
+                    "aggregations": aggregations,
+                }
+            )
 
             # Cache the result
             cache.set(cache_key, result, timeout=3600)  # Cache for 1 hour

tests/object_types/charts/test_grouped_bar_chart.py

@@ -65,8 +65,7 @@ def test_vertical_grouped_bar_chart(self):
         }
         chart_details = MockResourceChartDetails(
             chart_type="BAR",
-            options={**options, "allow_multi_series": true},
-            options=options,
+            options={**options, "allow_multi_series": True},
         )
         chart = UnifiedChart(chart_details, self.test_data)
         result = chart.create_chart()
@@ -104,8 +103,7 @@ def test_styling_options(self):
         }
         chart_details = MockResourceChartDetails(
             chart_type="BAR",
-            options={**options, "allow_multi_series": true},
-            options=options,
+            options={**options, "allow_multi_series": True},
         )
         chart = UnifiedChart(chart_details, self.test_data)
         result = chart.create_chart()
@@ -136,8 +134,7 @@ def test_value_aggregation(self):
         }
         chart_details = MockResourceChartDetails(
             chart_type="BAR",
-            options={**options, "allow_multi_series": true},
-            options=options,
+            options={**options, "allow_multi_series": True},
         )
         chart = UnifiedChart(chart_details, test_data)
         result = chart.create_chart()
@@ -156,8 +153,7 @@ def test_time_based_grouped_bar_chart(self):
         }
         chart_details = MockResourceChartDetails(
             chart_type="BAR",
-            options={**options, "allow_multi_series": true},
-            options=options,
+            options={**options, "allow_multi_series": True},
         )
         chart = UnifiedChart(chart_details, self.test_data)
         result = chart.create_chart()
@@ -190,8 +186,7 @@ def test_value_mapping(self):
         }
         chart_details = MockResourceChartDetails(
             chart_type="BAR",
-            options={**options, "allow_multi_series": true},
-            options=options,
+            options={**options, "allow_multi_series": True},
         )
         chart = UnifiedChart(chart_details, test_data)
         result = chart.create_chart()

tests/test_search_cache_and_user_info.py

@@ -0,0 +1,67 @@
+"""Regression tests for the two 500s on /api/auth/user/info/ and /api/search/aimodel/."""
+
+import pickle
+
+import pytest
+from elasticsearch_dsl import InnerDoc
+from elasticsearch_dsl.utils import AttrDict, AttrList
+from rest_framework.test import APIRequestFactory
+
+from api.views.auth import UserInfoView
+from api.views.paginated_elastic_view import as_plain_data
+
+
+def nested_doc(**fields: object) -> InnerDoc:
+    """Build a nested doc the way elasticsearch_dsl does for a nested field.
+
+    The class is rebuilt per document type rather than being the module-level
+    InnerDoc, which is exactly what pickle refuses to serialize.
+    """
+    cls = type("InnerDoc", (InnerDoc,), {})
+    cls.__module__ = "elasticsearch_dsl.document"
+    doc = cls()
+    for name, value in fields.items():
+        setattr(doc, name, value)
+    return doc
+
+
+@pytest.mark.django_db
+def test_user_info_rejects_anonymous_request() -> None:
+    """Anonymous callers get 401, not a 500 from reading .email off AnonymousUser.
+
+    The view is called directly: routing it through the test client hides the
+    bug, because the test settings drop the Keycloak middleware.
+    """
+    request = APIRequestFactory().get("/api/auth/user/info/")
+    response = UserInfoView.as_view()(request)
+    assert response.status_code == 401
+
+
+def test_as_plain_data_makes_search_results_picklable() -> None:
+    """Nested hits must survive cache.set, which pickles the cached value."""
+    result = {
+        "results": [
+            {
+                "all_providers": AttrList([nested_doc(provider="GPT")]),
+                "name": AttrDict({"raw": "x"}),
+            }
+        ],
+        "total": 1,
+    }
+
+    plain = as_plain_data(result)
+    assert pickle.loads(pickle.dumps(plain)) == plain
+    assert plain["results"][0]["all_providers"] == [{"provider": "GPT"}]
+    assert plain["results"][0]["name"] == {"raw": "x"}
+    assert not _holds_elastic_objects(plain)
+
+
+def _holds_elastic_objects(value: object) -> bool:
+    """The cache pickles what it is given, so no wrapper may survive anywhere."""
+    if isinstance(value, (AttrDict, AttrList, InnerDoc)):
+        return True
+    if isinstance(value, dict):
+        return any(_holds_elastic_objects(item) for item in value.values())
+    if isinstance(value, (list, tuple)):
+        return any(_holds_elastic_objects(item) for item in value)
+    return False