Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion netbox/extras/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions netbox/extras/models/tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
4 changes: 4 additions & 0 deletions netbox/netbox/api/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,7 @@ class SerializerNotFound(Exception):

class GraphQLTypeNotFound(Exception):
pass


class QuerySetNotOrdered(Exception):
pass
7 changes: 7 additions & 0 deletions netbox/netbox/api/pagination.py
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -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:
Expand Down
42 changes: 42 additions & 0 deletions netbox/netbox/tests/test_api.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
17 changes: 17 additions & 0 deletions netbox/users/migrations/0010_add_token_meta_ordering.py
Original file line number Diff line number Diff line change
@@ -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',)},
),
]
1 change: 1 addition & 0 deletions netbox/users/models/tokens.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions netbox/utilities/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)