Commits

Waldemar Kornewald committed 52de756

Adds test cases that assert that ordering happens only once for each save (and also happens *before* any data type conversions). Also fixes the code accordingly. Contributed by Jonas Haag.

  • Participants
  • Parent commits 640a13a

Comments (0)

Files changed (2)

File djangotoolbox/fields.py

                             "not of type %r" %  type(self.ordering))
         super(ListField, self).__init__(*args, **kwargs)
 
-    def _convert(self, func, values, *args, **kwargs):
-        values = super(ListField, self)._convert(func, values, *args, **kwargs)
-        if values is not None and self.ordering is not None:
+    def pre_save(self, model_instance, add):
+        values = getattr(model_instance, self.attname)
+        if values is None:
+            return None
+        if values and self.ordering:
             values.sort(key=self.ordering)
-        return values
+        return super(ListField, self).pre_save(model_instance, add)
 
 class SetField(AbstractIterableField):
     """

File djangotoolbox/tests.py

 from django.test import TestCase
 from django.utils import unittest
 
+def count_calls(func):
+    def wrapper(*args, **kwargs):
+        wrapper.calls += 1
+        return func(*args, **kwargs)
+    wrapper.calls = 0
+    return wrapper
+
 class Target(models.Model):
     index = models.IntegerField()
 
 
 class OrderedListModel(models.Model):
     ordered_ints = ListField(models.IntegerField(max_length=500), default=[],
-                             ordering=lambda x: x, null=True)
+                             ordering=count_calls(lambda x:x), null=True)
     ordered_nullable = ListField(ordering=lambda x:x, null=True)
 
 class SetModel(models.Model):
         self.assertEqual(ListModel().names_with_default, [])
 
     def test_ordering(self):
-        OrderedListModel(ordered_ints=self.unordered_ints).save()
+        f = OrderedListModel._meta.fields[1]
+        f.ordering.calls = 0
+
+        # ensure no ordering happens on assignment
+        obj = OrderedListModel()
+        obj.ordered_ints = self.unordered_ints
+        self.assertEqual(f.ordering.calls, 0)
+
+        obj.save()
         self.assertEqual(OrderedListModel.objects.get().ordered_ints,
                          sorted(self.unordered_ints))
+        # ordering should happen only once, i.e. the order function may be
+        # called N times at most (N being the number of items in the list)
+        self.assertLessEqual(f.ordering.calls, len(self.unordered_ints))
 
     def test_gt(self):
         # test gt on list