mirror of
https://github.com/inventree/InvenTree.git
synced 2026-03-04 03:11:46 +00:00
[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
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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']
|
||||
|
||||
|
||||
@@ -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']
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user