Просмотр исходного кода

Assert the collation reaches the query, and clarify the documented behaviour

The existing tests assert on filter results, which stay correct for ASCII values
even when the collation is never applied. Add a test which asserts on the
lookup's own compiled output, so that the mechanism failing open is caught
rather than passing silently.

Explain why the placeholder is compared literally: a field declaring its own
get_placeholder() compiles to something other than '%s', and splicing a COLLATE
clause into that is not safe, so any other right-hand side is left alone.

The documentation note described the folding as specific to the German
eszett. It is the common example rather than the rule: the collation treats a
character as equivalent to the sequence it expands to in upper case, which also
covers ligatures. Note too that a case-insensitive exact match on a collated
field may now return more than one object.
Jason Novinger 12 часов назад
Родитель
Сommit
9782be4cd5
3 измененных файлов с 37 добавлено и 3 удалено
  1. 6 3
      docs/reference/filtering.md
  2. 23 0
      netbox/dcim/tests/test_filtersets.py
  3. 8 0
      netbox/extras/lookups.py

+ 6 - 3
docs/reference/filtering.md

@@ -104,9 +104,12 @@ GET /api/dcim/devices/?name__ic=switch
 !!! note "Case-insensitive matching depends on the field's collation"
     Most `name` fields use a database collation which sorts them in natural order, so that
     `device-2` precedes `device-10`. Case-insensitive matching on those fields follows the
-    same collation, which treats the German `ß` and `ss` as equivalent: a search for
-    `Strasse` matches a device named `Straße`, and vice versa. Fields which do not use this
-    collation, such as `serial` and `description`, match these characters literally.
+    same collation, which treats a character as equivalent to the sequence it expands to in
+    upper case. The German `ß` is the common example: a search for `Strasse` matches a
+    device named `Straße`, and vice versa. Ligatures such as `fi` behave the same way. One
+    consequence is that a case-insensitive exact match on such a field may return more than
+    one object. Fields which do not use this collation, such as `serial` and `description`,
+    match these characters literally.
 
 ### Foreign Keys & Other Fields
 

+ 23 - 0
netbox/dcim/tests/test_filtersets.py

@@ -2,6 +2,7 @@ from decimal import Decimal
 
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
+from django.db import DEFAULT_DB_ALIAS, connection
 from django.test import TestCase
 
 from circuits.models import Circuit, CircuitTermination, CircuitType, Provider
@@ -3464,6 +3465,28 @@ class DeviceCollatedFilterTestCase(TestCase):
         qs = Device.objects.annotate(collated=CollateAsChar('name')).filter(collated__icontains='switch')
         self.assertEqual(qs.count(), 2)
 
+    def test_collation_is_applied_to_parameter(self):
+        # The tests above assert on results, which stay correct for ASCII values even if
+        # the collation is never applied. This asserts on the lookup's own output instead,
+        # so that the mechanism failing open is caught rather than passing silently.
+        for lookup in ('icontains', 'iexact', 'istartswith', 'iendswith'):
+            with self.subTest(lookup=lookup):
+                self.assertEqual(
+                    self._compiled_rhs(Device, 'name', lookup),
+                    '%s COLLATE "natural_sort"'
+                )
+                self.assertEqual(self._compiled_rhs(Device, 'serial', lookup), '%s')
+
+    @staticmethod
+    def _compiled_rhs(model, field_name, lookup):
+        """
+        Compile a single filter's right-hand side and return its SQL.
+        """
+        query = model.objects.filter(**{f'{field_name}__{lookup}': 'x'}).query
+        compiler = query.get_compiler(using=DEFAULT_DB_ALIAS)
+        rhs, _ = query.where.children[0].process_rhs(compiler, connection)
+        return rhs
+
 
 class ModuleTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
     queryset = Module.objects.all()

+ 8 - 0
netbox/extras/lookups.py

@@ -144,6 +144,9 @@ class CollatedCaseInsensitiveMixin:
 
     The COLLATE clause must sit inside UPPER(), not after the comparison, or it applies to
     the comparison's result rather than to its operand and has no effect.
+
+    Tested in dcim.tests.test_filtersets.DeviceCollatedFilterTestCase, which is where the
+    collated fields these lookups act upon are defined.
     """
     def process_rhs(self, compiler, connection):
         rhs, params = super().process_rhs(compiler, connection)
@@ -155,6 +158,11 @@ class CollatedCaseInsensitiveMixin:
         # one comparison. Requiring a Col also avoids reading a collation from an
         # annotation's output_field which the annotation itself does not carry, as Concat()
         # and Coalesce() both do.
+        #
+        # The placeholder is compared literally rather than inspected structurally: a field
+        # declaring its own get_placeholder() compiles to something other than '%s', and
+        # splicing a COLLATE clause into that is not safe. Any other rhs is a deliberate
+        # opt-out which leaves the lookup at its previous behaviour.
         if collation == NATURAL_SORT_COLLATION and rhs == '%s' and isinstance(self.lhs, Col):
             # The collation name cannot be passed as a query parameter, but it originates
             # from the field definition rather than from user input.