diff --git a/releases/api_views.py b/releases/api_views.py index ccf95c4..4ffbc33 100644 --- a/releases/api_views.py +++ b/releases/api_views.py @@ -1,11 +1,21 @@ from rest_framework import viewsets from rest_framework.exceptions import ValidationError -from projects.models import Project -from .models import Release, ordered_releases +from bugsink.api_pagination import AscDescCursorPagination + +from .models import Release from .serializers import ReleaseListSerializer, ReleaseDetailSerializer, ReleaseCreateSerializer +class ReleasePagination(AscDescCursorPagination): + # Cursor pagination requires an indexed, mostly-stable ordering field. We use `digest_order`: We require + # ?project= and have a composite (project_id, date_released) index, so ORDER BY date_released after filtering by + # project is fast and cursor-stable. (also note that date_released generally comes in in-order). + base_ordering = ("date_released",) + page_size = 250 + default_direction = "desc" + + class ReleaseViewSet(viewsets.ModelViewSet): """ LIST requires: ?project= @@ -15,6 +25,7 @@ class ReleaseViewSet(viewsets.ModelViewSet): queryset = Release.objects.all() serializer_class = ReleaseListSerializer http_method_names = ["get", "post", "head", "options"] + pagination_class = ReleasePagination def filter_queryset(self, queryset): queryset = super().filter_queryset(queryset) @@ -26,10 +37,7 @@ class ReleaseViewSet(viewsets.ModelViewSet): if not project_id: raise ValidationError({"project": ["This field is required."]}) - # application-ordering (as opposed to in-db); will have performance implications, but we do "correct first, fast - # later": - project = Project.objects.get(pk=project_id) - return ordered_releases(project=project) + return queryset.filter(project=project_id) def get_serializer_class(self): if self.action == "create": diff --git a/releases/migrations/0004_fix_indexes.py b/releases/migrations/0004_fix_indexes.py new file mode 100644 index 0000000..f5a90ed --- /dev/null +++ b/releases/migrations/0004_fix_indexes.py @@ -0,0 +1,29 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("projects", "0014_alter_projectmembership_project"), + ("releases", "0003_alter_release_project"), + ] + + operations = [ + migrations.RemoveIndex( + model_name="release", + name="releases_re_sort_ep_5c07c8_idx", + ), + migrations.AddIndex( + model_name="release", + index=models.Index( + fields=["project", "sort_epoch"], name="releases_re_project_1ceb8b_idx" + ), + ), + migrations.AddIndex( + model_name="release", + index=models.Index( + fields=["project", "date_released"], + name="releases_re_project_b17273_idx", + ), + ), + ] diff --git a/releases/models.py b/releases/models.py index a0e7592..8e03be9 100644 --- a/releases/models.py +++ b/releases/models.py @@ -83,7 +83,8 @@ class Release(models.Model): unique_together = ("project", "version") indexes = [ - models.Index(fields=["sort_epoch"]), + models.Index(fields=["project", "sort_epoch"]), + models.Index(fields=["project", "date_released"]), ] def get_short_version(self): diff --git a/releases/test_api.py b/releases/test_api.py index 7d7f603..b9de8e2 100644 --- a/releases/test_api.py +++ b/releases/test_api.py @@ -1,10 +1,12 @@ from django.test import TestCase as DjangoTestCase from django.urls import reverse +from django.utils import timezone from rest_framework.test import APIClient from bsmain.models import AuthToken from projects.models import Project -from releases.models import ordered_releases +from releases.models import Release +from releases.api_views import ReleaseViewSet class ReleaseApiTests(DjangoTestCase): @@ -25,19 +27,6 @@ class ReleaseApiTests(DjangoTestCase): self.assertEqual(response.status_code, 400) self.assertEqual({"project": ["This field is required."]}, response.json()) - def test_list_uses_ordered_releases(self): - # Create in arbitrary order - self._create("1.0.0") - self._create("1.0.0+build") - self._create("1.0.1") - - response = self.client.get(reverse("api:release-list"), {"project": str(self.project.id)}) - self.assertEqual(response.status_code, 200) - - versions_from_api = [row["version"] for row in response.json()["results"]] - versions_expected = [r.version for r in ordered_releases(project=self.project)] - self.assertEqual(versions_from_api, versions_expected) - def test_create_new_returns_201_and_detail_shape(self): response = self._create("1.2.3", timestamp="2024-01-01T00:00:00Z") self.assertEqual(response.status_code, 201) @@ -90,3 +79,58 @@ class ReleaseApiTests(DjangoTestCase): self.assertEqual(put_response.status_code, 405) self.assertEqual(patch_response.status_code, 405) self.assertEqual(delete_response.status_code, 405) + + +class ReleasePaginationTests(DjangoTestCase): + def setUp(self): + self.client = APIClient() + token = AuthToken.objects.create() + self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {token.token}") + self.old_size = ReleaseViewSet.pagination_class.page_size + ReleaseViewSet.pagination_class.page_size = 2 + + def tearDown(self): + ReleaseViewSet.pagination_class.page_size = self.old_size + + def _make_releases(self, project, deltas): + base = timezone.now().replace(microsecond=0) + releases = [] + for i, delta in enumerate(deltas): + rel = Release.objects.create( + project=project, + version=f"v{i}", + date_released=base + delta, + ) + releases.append(rel) + return releases + + def _ids(self, resp): + return [row["id"] for row in resp.json()["results"]] + + def test_date_released_desc_two_pages(self): + proj = Project.objects.create(name="P") + releases = self._make_releases( + proj, [timezone.timedelta(days=i) for i in range(5)] + ) + + r1 = self.client.get( + reverse("api:release-list"), {"project": str(proj.id), "order": "desc"} + ) + self.assertEqual(self._ids(r1), [str(releases[4].id), str(releases[3].id)]) + + r2 = self.client.get(r1.json()["next"]) + self.assertEqual(self._ids(r2), [str(releases[2].id), str(releases[1].id)]) + + def test_date_released_asc_two_pages(self): + proj = Project.objects.create(name="P2") + releases = self._make_releases( + proj, [timezone.timedelta(days=i) for i in range(5)] + ) + + r1 = self.client.get( + reverse("api:release-list"), {"project": str(proj.id), "order": "asc"} + ) + self.assertEqual(self._ids(r1), [str(releases[0].id), str(releases[1].id)]) + + r2 = self.client.get(r1.json()["next"]) + self.assertEqual(self._ids(r2), [str(releases[2].id), str(releases[3].id)])