Fixes #18900: raise QuerySetNotOrdered exception when trying to paginate unordered API querysets (#19943)

* Fixes #18900: introduce/raise QuerySetNotOrdered exception

Defines a new exception, `QuerySetNotOrdered`, and raises it in
`OptionalLimitOffsetPagination.paginate_queryset` in the right
conditions:
- the iterable to be paginated is a QuerySet isinstance
- the `queryset.ordered` flag is not truthy

* Don't try to reapply ordering if ordering is already present

* Add ordering for failing tagged-objects list API endpoint

I chose to implement this here for TaggedItemViewSet, rather than on the
model, because any meaningful ordering is going to be done on the
related Tag instance and I didn't want to introduce potential, not well
understood side-effects by applying a model-wide ordering via a related
model field.

* Add default Token ordering behavior

* Adds basic tests for raising QuerySetNotOrdered

* Note why ordering is not applied in TaggedItem.Meta

Author: jnovinger

netbox/extras/api/views.py

@@ -185,7 +185,9 @@ class TagViewSet(NetBoxModelViewSet):
 
 
 class TaggedItemViewSet(RetrieveModelMixin, ListModelMixin, BaseViewSet):
-    queryset = TaggedItem.objects.prefetch_related('content_type', 'content_object', 'tag')
+    queryset = TaggedItem.objects.prefetch_related(
+        'content_type', 'content_object', 'tag'
+    ).order_by('tag__weight', 'tag__name')
     serializer_class = serializers.TaggedItemSerializer
     filterset_class = filtersets.TaggedItemFilterSet
 

netbox/extras/models/tags.py

@@ -83,3 +83,6 @@ class Meta:
         indexes = [models.Index(fields=["content_type", "object_id"])]
         verbose_name = _('tagged item')
         verbose_name_plural = _('tagged items')
+        # Note: while there is no ordering applied here (because it would basically be done on fields
+        # of the related `tag`), there is an ordering applied to extras.api.views.TaggedItemViewSet
+        # to allow for proper pagination.

netbox/netbox/api/exceptions.py

@@ -12,3 +12,7 @@ class SerializerNotFound(Exception):
 
 class GraphQLTypeNotFound(Exception):
     pass
+
+
+class QuerySetNotOrdered(Exception):
+    pass

netbox/netbox/api/pagination.py

@@ -1,6 +1,7 @@
 from django.db.models import QuerySet
 from rest_framework.pagination import LimitOffsetPagination
 
+from netbox.api.exceptions import QuerySetNotOrdered
 from netbox.config import get_config
 
 
@@ -15,6 +16,12 @@ def __init__(self):
 
     def paginate_queryset(self, queryset, request, view=None):
 
+        if isinstance(queryset, QuerySet) and not queryset.ordered:
+            raise QuerySetNotOrdered(
+                "Paginating over an unordered queryset is unreliable. Ensure that a minimal "
+                "ordering has been applied to the queryset for this API endpoint."
+            )
+
         if isinstance(queryset, QuerySet):
             self.count = self.get_queryset_count(queryset)
         else:

netbox/netbox/tests/test_api.py

@@ -1,8 +1,13 @@
 import uuid
 
+from django.test import RequestFactory, TestCase
 from django.urls import reverse
+from rest_framework.request import Request
 
+from netbox.api.exceptions import QuerySetNotOrdered
+from netbox.api.pagination import OptionalLimitOffsetPagination
 from utilities.testing import APITestCase
+from users.models import Token
 
 
 class AppTest(APITestCase):
@@ -26,3 +31,40 @@ def test_status(self):
         response = self.client.get(f'{url}?format=api', **self.header)
 
         self.assertEqual(response.status_code, 200)
+
+
+class OptionalLimitOffsetPaginationTest(TestCase):
+
+    def setUp(self):
+        self.paginator = OptionalLimitOffsetPagination()
+        self.factory = RequestFactory()
+
+    def _make_drf_request(self, path='/', query_params=None):
+        """Helper to create a proper DRF Request object"""
+        return Request(self.factory.get(path, query_params or {}))
+
+    def test_raises_exception_for_unordered_queryset(self):
+        """Should raise QuerySetNotOrdered for unordered QuerySet"""
+        queryset = Token.objects.all().order_by()
+        request = self._make_drf_request()
+
+        with self.assertRaises(QuerySetNotOrdered) as cm:
+            self.paginator.paginate_queryset(queryset, request)
+
+        error_msg = str(cm.exception)
+        self.assertIn("Paginating over an unordered queryset is unreliable", error_msg)
+        self.assertIn("Ensure that a minimal ordering has been applied", error_msg)
+
+    def test_allows_ordered_queryset(self):
+        """Should not raise exception for ordered QuerySet"""
+        queryset = Token.objects.all().order_by('created')
+        request = self._make_drf_request()
+
+        self.paginator.paginate_queryset(queryset, request)  # Should not raise exception
+
+    def test_allows_non_queryset_iterables(self):
+        """Should not raise exception for non-QuerySet iterables"""
+        iterable = [1, 2, 3, 4, 5]
+        request = self._make_drf_request()
+
+        self.paginator.paginate_queryset(iterable, request)  # Should not raise exception

netbox/users/migrations/0010_add_token_meta_ordering.py

@@ -0,0 +1,17 @@
+# Generated by Django 5.2.4 on 2025-07-23 17:28
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0009_update_group_perms'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='token',
+            options={'ordering': ('-created',)},
+        ),
+    ]

netbox/users/models/tokens.py

@@ -74,6 +74,7 @@ class Token(models.Model):
     class Meta:
         verbose_name = _('token')
         verbose_name_plural = _('tokens')
+        ordering = ('-created',)
 
     def __str__(self):
         return self.key if settings.ALLOW_TOKEN_RETRIEVAL else self.partial

netbox/utilities/query.py

@@ -67,5 +67,8 @@ def reapply_model_ordering(queryset: QuerySet) -> QuerySet:
     # MPTT-based models are exempt from this; use caution when annotating querysets of these models
     if any(isinstance(manager, TreeManager) for manager in queryset.model._meta.local_managers):
         return queryset
+    elif queryset.ordered:
+        return queryset
+
     ordering = queryset.model._meta.ordering
     return queryset.order_by(*ordering)