From 3bbdddf51debe80c2fdf6eb5d6507e4f949568b7 Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 2 Mar 2026 22:45:44 +1100 Subject: [PATCH] [API] Enforce pk ordering for API endpoints (#11446) * Add unit test to detect unreliable pagination * Enforce PK field ordering - Append 'pk' ordering to InvenTreeOrderingFilter * Use our ordering filter everywhere * Simplify ordering options * Enforce list * Use last term for ordering checks * Individual delete to fix mysql issue --- src/backend/InvenTree/InvenTree/filters.py | 34 +++++----- src/backend/InvenTree/build/api.py | 8 +-- src/backend/InvenTree/common/api.py | 8 +-- src/backend/InvenTree/company/api.py | 8 +-- src/backend/InvenTree/order/api.py | 22 +++---- src/backend/InvenTree/part/api.py | 8 +-- src/backend/InvenTree/stock/api.py | 7 +- src/backend/InvenTree/stock/test_api.py | 75 ++++++++++++++++++++++ 8 files changed, 119 insertions(+), 51 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/filters.py b/src/backend/InvenTree/InvenTree/filters.py index 78c5c5e455..44319b843f 100644 --- a/src/backend/InvenTree/InvenTree/filters.py +++ b/src/backend/InvenTree/InvenTree/filters.py @@ -111,9 +111,11 @@ class InvenTreeOrderingFilter(filters.OrderingFilter): def get_ordering(self, request, queryset, view): """Override ordering for supporting aliases.""" - ordering = super().get_ordering(request, queryset, view) + ordering = list(super().get_ordering(request, queryset, view) or []) aliases = getattr(view, 'ordering_field_aliases', None) + lookup_field = getattr(view, 'lookup_field', 'pk') + lookup_reversed = len(ordering) > 0 and ordering[-1].startswith('-') # Attempt to map ordering fields based on provided aliases if ordering is not None and aliases is not None: @@ -123,9 +125,8 @@ class InvenTreeOrderingFilter(filters.OrderingFilter): ordering = [] for field in ordering_initial: - reverse = field.startswith('-') - - if reverse: + field_reversed = field.startswith('-') + if field_reversed: field = field[1:] # Are aliases defined for this field? @@ -153,11 +154,22 @@ class InvenTreeOrderingFilter(filters.OrderingFilter): continue for a in alias: - if reverse: + if field_reversed: a = '-' + a ordering.append(a) + # Ensure that any API filtering appends the primary-key field + # This is to prevent "ambiguous ordering" errors across pagination boundaries + # Ref: https://github.com/inventree/InvenTree/issues/11442 + if lookup_field and not any( + field in ordering for field in [lookup_field, f'-{lookup_field}'] + ): + if lookup_reversed: + ordering.append(f'-{lookup_field}') + else: + ordering.append(lookup_field) + return ordering @@ -220,18 +232,10 @@ class NumericInFilter(rest_filters.BaseInFilter): return super().filter(qs, numeric_values) -SEARCH_ORDER_FILTER = [ - drf_backend.DjangoFilterBackend, - InvenTreeSearchFilter, - filters.OrderingFilter, -] +ORDER_FILTER = [drf_backend.DjangoFilterBackend, InvenTreeOrderingFilter] -SEARCH_ORDER_FILTER_ALIAS = [ +SEARCH_ORDER_FILTER = [ drf_backend.DjangoFilterBackend, InvenTreeSearchFilter, InvenTreeOrderingFilter, ] - -ORDER_FILTER = [drf_backend.DjangoFilterBackend, filters.OrderingFilter] - -ORDER_FILTER_ALIAS = [drf_backend.DjangoFilterBackend, InvenTreeOrderingFilter] diff --git a/src/backend/InvenTree/build/api.py b/src/backend/InvenTree/build/api.py index b155602b50..e94590e67f 100644 --- a/src/backend/InvenTree/build/api.py +++ b/src/backend/InvenTree/build/api.py @@ -27,7 +27,7 @@ from generic.states.api import StatusView from InvenTree.api import BulkDeleteMixin, ParameterListMixin, meta_path from InvenTree.fields import InvenTreeOutputOption, OutputConfiguration from InvenTree.filters import ( - SEARCH_ORDER_FILTER_ALIAS, + SEARCH_ORDER_FILTER, InvenTreeDateFilter, NumberOrNullFilter, ) @@ -343,7 +343,7 @@ class BuildList( output_options = BuildListOutputOptions filterset_class = BuildFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_fields = [ 'reference', 'part', @@ -594,7 +594,7 @@ class BuildLineList( """API endpoint for accessing a list of BuildLine objects.""" filterset_class = BuildLineFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = BuildLineOutputOptions ordering_fields = [ 'part', @@ -951,7 +951,7 @@ class BuildItemList( output_options = BuildItemOutputOptions filterset_class = BuildItemFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER def get_queryset(self): """Override the queryset method, to perform custom prefetch.""" diff --git a/src/backend/InvenTree/common/api.py b/src/backend/InvenTree/common/api.py index eccd885d98..3b6cc14de5 100644 --- a/src/backend/InvenTree/common/api.py +++ b/src/backend/InvenTree/common/api.py @@ -49,11 +49,7 @@ from InvenTree.api import ( meta_path, ) from InvenTree.config import CONFIG_LOOKUPS -from InvenTree.filters import ( - ORDER_FILTER, - SEARCH_ORDER_FILTER, - SEARCH_ORDER_FILTER_ALIAS, -) +from InvenTree.filters import ORDER_FILTER, SEARCH_ORDER_FILTER from InvenTree.helpers import inheritors, str2bool from InvenTree.helpers_email import send_email from InvenTree.mixins import ( @@ -1079,7 +1075,7 @@ class ParameterList( """List API endpoint for Parameter objects.""" filterset_class = ParameterFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_fields = ['name', 'data', 'units', 'template', 'updated', 'updated_by'] diff --git a/src/backend/InvenTree/company/api.py b/src/backend/InvenTree/company/api.py index 8c1fe8db05..9ce3eaf27c 100644 --- a/src/backend/InvenTree/company/api.py +++ b/src/backend/InvenTree/company/api.py @@ -11,7 +11,7 @@ import part.models from data_exporter.mixins import DataExportViewMixin from InvenTree.api import ListCreateDestroyAPIView, ParameterListMixin, meta_path from InvenTree.fields import InvenTreeOutputOption, OutputConfiguration -from InvenTree.filters import SEARCH_ORDER_FILTER, SEARCH_ORDER_FILTER_ALIAS +from InvenTree.filters import SEARCH_ORDER_FILTER from InvenTree.mixins import ( ListCreateAPI, OutputOptionsMixin, @@ -197,7 +197,7 @@ class ManufacturerPartList( """ filterset_class = ManufacturerPartFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = ManufacturerOutputOptions ordering_fields = ['part', 'IPN', 'MPN', 'manufacturer'] @@ -360,7 +360,7 @@ class SupplierPartList( """ filterset_class = SupplierPartFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = SupplierPartOutputOptions ordering_fields = [ @@ -475,7 +475,7 @@ class SupplierPriceBreakList( output_options = SupplierPriceBreakOutputOptions filterset_class = SupplierPriceBreakFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_fields = ['quantity', 'supplier', 'SKU', 'price'] search_fields = ['part__SKU', 'part__supplier__name'] diff --git a/src/backend/InvenTree/order/api.py b/src/backend/InvenTree/order/api.py index a7bdee7568..b8e214a0ac 100644 --- a/src/backend/InvenTree/order/api.py +++ b/src/backend/InvenTree/order/api.py @@ -35,11 +35,7 @@ from InvenTree.api import ( meta_path, ) from InvenTree.fields import InvenTreeOutputOption, OutputConfiguration -from InvenTree.filters import ( - SEARCH_ORDER_FILTER, - SEARCH_ORDER_FILTER_ALIAS, - InvenTreeDateFilter, -) +from InvenTree.filters import SEARCH_ORDER_FILTER, InvenTreeDateFilter from InvenTree.helpers import str2bool from InvenTree.helpers_model import construct_absolute_url, get_base_url from InvenTree.mixins import ( @@ -399,7 +395,7 @@ class PurchaseOrderList( """ filterset_class = PurchaseOrderFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = PurchaseOrderOutputOptions ordering_field_aliases = { @@ -709,7 +705,7 @@ class PurchaseOrderLineItemList( serializer.data, status=status.HTTP_201_CREATED, headers=headers ) - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_field_aliases = { 'MPN': 'part__manufacturer_part__MPN', @@ -868,7 +864,7 @@ class SalesOrderList( """ filterset_class = SalesOrderFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = SalesOrderOutputOptions ordering_field_aliases = { @@ -1053,7 +1049,7 @@ class SalesOrderLineItemList( filterset_class = SalesOrderLineItemFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = SalesOrderLineItemOutputOptions @@ -1299,7 +1295,7 @@ class SalesOrderAllocationList( """API endpoint for listing SalesOrderAllocation objects.""" filterset_class = SalesOrderAllocationFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = SalesOrderAllocationOutputOptions ordering_fields = [ @@ -1409,7 +1405,7 @@ class SalesOrderShipmentList(SalesOrderShipmentMixin, ListCreateAPI): """API list endpoint for SalesOrderShipment model.""" filterset_class = SalesOrderShipmentFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_fields = ['reference', 'delivery_date', 'shipment_date', 'allocated_items'] search_fields = [ @@ -1538,7 +1534,7 @@ class ReturnOrderList( """API endpoint for accessing a list of ReturnOrder objects.""" filterset_class = ReturnOrderFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = ReturnOrderOutputOptions @@ -1685,7 +1681,7 @@ class ReturnOrderLineItemList( filterset_class = ReturnOrderLineItemFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER output_options = ReturnOrderLineItemOutputOptions diff --git a/src/backend/InvenTree/part/api.py b/src/backend/InvenTree/part/api.py index db8eefd5e6..31169bf878 100644 --- a/src/backend/InvenTree/part/api.py +++ b/src/backend/InvenTree/part/api.py @@ -24,9 +24,7 @@ from InvenTree.api import ( from InvenTree.fields import InvenTreeOutputOption, OutputConfiguration from InvenTree.filters import ( ORDER_FILTER, - ORDER_FILTER_ALIAS, SEARCH_ORDER_FILTER, - SEARCH_ORDER_FILTER_ALIAS, InvenTreeDateFilter, InvenTreeSearchFilter, NumberOrNullFilter, @@ -302,7 +300,7 @@ class CategoryTree(ListAPI): queryset = PartCategory.objects.all() serializer_class = part_serializers.CategoryTree - filter_backends = ORDER_FILTER_ALIAS + filter_backends = ORDER_FILTER ordering_fields = ['level', 'name', 'subcategories'] @@ -1080,7 +1078,7 @@ class PartList( filterset_class = PartFilter is_create = True - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_fields = [ 'name', @@ -1442,7 +1440,7 @@ class BomList( output_options = BomOutputOptions filterset_class = BomFilter - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER search_fields = [ 'reference', diff --git a/src/backend/InvenTree/stock/api.py b/src/backend/InvenTree/stock/api.py index 9b2fd32a3b..bef906f6e5 100644 --- a/src/backend/InvenTree/stock/api.py +++ b/src/backend/InvenTree/stock/api.py @@ -37,9 +37,8 @@ from InvenTree.api import ( ) from InvenTree.fields import InvenTreeOutputOption, OutputConfiguration from InvenTree.filters import ( - ORDER_FILTER_ALIAS, + ORDER_FILTER, SEARCH_ORDER_FILTER, - SEARCH_ORDER_FILTER_ALIAS, InvenTreeDateFilter, NumberOrNullFilter, ) @@ -455,7 +454,7 @@ class StockLocationTree(ListAPI): queryset = StockLocation.objects.all() serializer_class = StockSerializers.LocationTreeSerializer - filter_backends = ORDER_FILTER_ALIAS + filter_backends = ORDER_FILTER ordering_fields = ['level', 'name', 'sublocations'] @@ -1260,7 +1259,7 @@ class StockList( headers=self.get_success_headers(serializer.data), ) - filter_backends = SEARCH_ORDER_FILTER_ALIAS + filter_backends = SEARCH_ORDER_FILTER ordering_field_aliases = { 'part': 'part__name', diff --git a/src/backend/InvenTree/stock/test_api.py b/src/backend/InvenTree/stock/test_api.py index 405f90a6b8..cbaea852f9 100644 --- a/src/backend/InvenTree/stock/test_api.py +++ b/src/backend/InvenTree/stock/test_api.py @@ -575,6 +575,81 @@ class StockItemListTest(StockAPITestCase): for ordering in ['part', 'location', 'stock', 'status', 'IPN', 'MPN', 'SKU']: self.run_ordering_test(self.list_url, ordering) + def test_pagination(self): + """Test that pagination boundaries are observed correctly. + + Ref: https://github.com/inventree/InvenTree/issues/11442 + """ + location = StockLocation.objects.first() + part = Part.objects.first() + + items = [] + + # Delete all existing stock item objects + for item in StockItem.objects.all(): + item.delete() + + for idx in range(1000): + items.append( + StockItem( + part=part, + location=location, + quantity=idx % 10, + level=0, + lft=0, + rght=0, + tree_id=0, + ) + ) + + if len(items) >= 100: + StockItem.objects.bulk_create(items) + items = [] + + self.assertEqual(StockItem.objects.count(), 1000) + + url = reverse('api-stock-list') + + # Keep track of the unique PKs we have seen in the results + unique_pks = set() + + for idx in range(0, 100, 10): + data = self.get(url, {'limit': 10, 'offset': idx}).data + self.assertEqual(data['count'], 1000) + self.assertEqual(len(data['results']), 10) + + for item in data['results']: + self.assertNotIn( + item['pk'], + unique_pks, + f'Duplicate PK {item["pk"]} found in paginated results @ page {idx // 10}', + ) + unique_pks.add(item['pk']) + + self.assertEqual( + len(unique_pks), 100, 'Expected to see 100 unique PKs in paginated results' + ) + + # Run same test again, with reverse ordering on part IPN + unique_pks = set() + + for idx in range(0, 100, 10): + data = self.get(url, {'limit': 10, 'offset': idx, 'ordering': '-IPN'}).data + self.assertEqual(data['count'], 1000) + self.assertEqual(len(data['results']), 10) + + for item in data['results']: + self.assertNotIn( + item['pk'], + unique_pks, + f'Duplicate PK {item["pk"]} found in paginated results @ page {idx // 10} with reverse ordering', + ) + unique_pks.add(item['pk']) + + self.assertEqual( + len(unique_pks), 100, 'Expected to see 100 unique PKs in paginated results' + ) + def test_top_level_filtering(self): """Test filtering against "top level" stock location.""" # No filters, should return *all* items