From 4844c724157b52b8097d19afae4436ed016a28d9 Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Tue, 9 Sep 2025 12:08:30 +0200 Subject: [PATCH] Events API See #146 --- bugsink/test_api.py | 6 ++-- events/api_views.py | 57 +++++++++++++++++++++++++++++++++--- events/serializers.py | 20 ++++++++----- events/test_api.py | 67 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 events/test_api.py diff --git a/bugsink/test_api.py b/bugsink/test_api.py index 2cf0602..8ebcc5a 100644 --- a/bugsink/test_api.py +++ b/bugsink/test_api.py @@ -9,9 +9,9 @@ class BearerAuthRouterTests(unittest.TestCase): self.client = APIClient() def test_ok_on_event_list(self): - tok = AuthToken.objects.create() - self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {tok.token}") - resp = self.client.get(reverse("api:event-list")) + token = AuthToken.objects.create() + self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {token.token}") + resp = self.client.get(reverse("api:event-list"), {"issue": "00000000-0000-0000-0000-000000000000"}) self.assertEqual(resp.status_code, 200) def test_missing_on_event_list(self): diff --git a/events/api_views.py b/events/api_views.py index 874fa4c..188bcef 100644 --- a/events/api_views.py +++ b/events/api_views.py @@ -1,11 +1,60 @@ +from django.shortcuts import get_object_or_404 from rest_framework import viewsets +from rest_framework.exceptions import ValidationError + +from bugsink.utils import assert_ from .models import Event -from .serializers import EventSerializer +from .serializers import EventListSerializer, EventDetailSerializer class EventViewSet(viewsets.ReadOnlyModelViewSet): - queryset = Event.objects.all().order_by('digest_order') - serializer_class = EventSerializer + """ + LIST requires: ?issue= + Optional: ?order=asc|desc (default: desc) + LIST omits `data`, ordered by digest_order + RETRIEVE includes `data` (pure PK lookup; no filters/order applied) + """ + queryset = Event.objects.all() # router requirement for basename inference + serializer_class = EventListSerializer - # TODO: the idea of required filter-fields when listing. + def filter_queryset(self, queryset): + query_params = self.request.query_params + + if "issue" not in query_params: + raise ValidationError({"issue": ["This field is required."]}) + + order = query_params.get("order", "desc") + if order not in ("asc", "desc"): + raise ValidationError({"order": ["Must be 'asc' or 'desc'."]}) + + ordering = "digest_order" if order == "asc" else "-digest_order" + + # (issue, digest_order) is a db-index + return queryset.filter(issue=query_params["issue"]).order_by(ordering) + + def get_object(self): + """ + DRF's get_object(), but we intentionally bypass filter_queryset for detail routes to keep PK lookups + db-index-friendly (no WHERE filters other than the PK which is already indexed). + """ + queryset = self.get_queryset() # no filter_queryset() here + + lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field + assert_(lookup_url_kwarg in self.kwargs, ( + 'Expected view %s to be called with a URL keyword argument ' + 'named "%s". Fix your URL conf, or set the `.lookup_field` ' + 'attribute on the view correctly.' % + (self.__class__.__name__, lookup_url_kwarg) + )) + + filter_kwargs = {self.lookup_field: self.kwargs[lookup_url_kwarg]} + obj = get_object_or_404(queryset, **filter_kwargs) + + # May raise a permission denied + self.check_object_permissions(self.request, obj) + + return obj + + def get_serializer_class(self): + return EventDetailSerializer if self.action == "retrieve" else EventListSerializer diff --git a/events/serializers.py b/events/serializers.py index ae9bb26..66f2e44 100644 --- a/events/serializers.py +++ b/events/serializers.py @@ -3,14 +3,11 @@ from rest_framework import serializers from .models import Event -class EventSerializer(serializers.ModelSerializer): +class EventListSerializer(serializers.ModelSerializer): + """Lightweight list view: excludes the (potentially large) `data` field.""" + class Meta: model = Event - - # TODO better wording: - # This is the first attempt at getting the list of fields right. My belief is: this is a nice minimal list. - # it _does_ contain `data`, which is typically quite "fat", but I'd say that's the most useful field to have. - # and when you're actually in the business of looking at a specific event, you want to see the data. fields = [ "id", "ingested_at", @@ -19,7 +16,16 @@ class EventSerializer(serializers.ModelSerializer): "grouping", "event_id", "project", - "data", # TODO fetch from disk if so-configured "timestamp", "digest_order", ] + + +class EventDetailSerializer(serializers.ModelSerializer): + """Detail view: includes full `data` payload.""" + + class Meta: + model = Event + fields = EventListSerializer.Meta.fields + [ + "data", + ] diff --git a/events/test_api.py b/events/test_api.py new file mode 100644 index 0000000..c6fcf64 --- /dev/null +++ b/events/test_api.py @@ -0,0 +1,67 @@ +from django.test import TestCase as DjangoTestCase +from django.urls import reverse +from rest_framework.test import APIClient + +from projects.models import Project +from bsmain.models import AuthToken +from events.factories import create_event +from issues.factories import get_or_create_issue + + +class EventApiTests(DjangoTestCase): + def setUp(self): + self.client = APIClient() + token = AuthToken.objects.create() + self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {token.token}") + + self.project = Project.objects.create(name="Test Project") + + self.issue, _ = get_or_create_issue(project=self.project) + self.event = create_event(issue=self.issue) + + def test_list_requires_scope(self): + response = self.client.get(reverse("api:event-list")) + + self.assertEqual(response.status_code, 400) + self.assertEqual({'issue': ['This field is required.']}, response.json()) + + def test_detail_by_id(self): + url = reverse("api:event-detail", args=[self.event.id]) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + detail = response.json() + self.assertEqual(detail["id"], str(self.event.id)) + self.assertIn("data", detail) + + def test_list_by_issue_is_light_payload(self): + response = self.client.get(reverse("api:event-list"), {"issue": str(self.issue.id)}) + self.assertEqual(response.status_code, 200) + self.assertNotIn("data", response.json()["results"][0]) + + def test_detail_not_found_is_404(self): + url = reverse("api:event-detail", args=["00000000-0000-0000-0000-000000000000"]) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + def test_list_rejects_bad_order(self): + response = self.client.get(reverse("api:event-list"), {"issue": str(self.issue.id), "order": "sideways"}) + self.assertEqual(response.status_code, 400) + self.assertEqual({'order': ["Must be 'asc' or 'desc'."]}, response.json()) + + def test_list_order_default_desc(self): + e0 = self.event + e1 = create_event(issue=self.issue) + response = self.client.get(reverse("api:event-list"), {"issue": str(self.issue.id)}) + self.assertEqual(response.status_code, 200) + ids = [item["id"] for item in response.json()["results"]] + self.assertEqual(ids[0], str(e1.id)) + self.assertEqual(ids[1], str(e0.id)) + + def test_list_order_asc(self): + e0 = self.event + e1 = create_event(issue=self.issue) + response = self.client.get(reverse("api:event-list"), {"issue": str(self.issue.id), "order": "asc"}) + self.assertEqual(response.status_code, 200) + ids = [item["id"] for item in response.json()["results"]] + self.assertEqual(ids[0], str(e0.id)) + self.assertEqual(ids[1], str(e1.id))