Accessor: keep order in Set type category accessor.

Note that Set type setter still keeps the default one if exists.

/reviewed-on nexedi/erp5!862

product/ERP5Type/Accessor/Category.py

@@ -27,6 +27,7 @@
 ##############################################################################
 
 
+from collections import OrderedDict
 from Base import func_code, type_definition, list_types, ATTRIBUTE_PREFIX, Setter as BaseSetter, Getter as BaseGetter
 from zLOG import LOG
 from Products.ERP5Type.PsycoWrapper import psyco
@@ -98,8 +99,7 @@ class SetSetter(ListSetter):
       We should take care that the provided argument has no
       duplicate values
       """
-      if type(value) not in (set, frozenset):
-        value = frozenset(value)
+      value = tuple(OrderedDict.fromkeys(value))
       instance._setCategoryMembership(self._key, value,
                                       spec=kw.get('spec',()),
                                       filter=kw.get('filter', None),
@@ -164,7 +164,7 @@ class SetGetter(ListGetter):
     Gets a category value set
     """
     def __call__(self, instance, *args, **kw):
-      return list(set(ListGetter.__call__(self, instance, *args, **kw)))
+      return list(OrderedDict.fromkeys(ListGetter.__call__(self, instance, *args, **kw)))
 
 
 # ItemList is outdated XXX -> ItemList

product/ERP5Type/Accessor/Value.py

@@ -26,6 +26,7 @@
 #
 ##############################################################################
 
+from collections import OrderedDict
 from operator import methodcaller
 from Base import func_code, type_definition, list_types, ATTRIBUTE_PREFIX, Setter as BaseSetter, Getter as BaseGetter
 from zLOG import LOG
@@ -46,10 +47,11 @@ class SetSetter(BaseSetter):
       self._key = key
       self._warning = warning
 
-    def __call__(self, instance, *args, **kw):
+    def __call__(self, instance, value, *args, **kw):
       if self._warning:
         LOG("ERP5Type Deprecated Setter Id:",0, self._id)
-      instance._setValue(self._key, set(args[0]),
+      value = tuple(OrderedDict.fromkeys(value))
+      instance._setValue(self._key, value,
                                                 spec=kw.get('spec',()),
                                                 filter=kw.get('filter', None),
                                                 portal_type=kw.get('portal_type',()),
@@ -164,7 +166,7 @@ class SetGetter(ListGetter):
     """
     def __call__(self, instance, *args, **kw):
       r = ListGetter.__call__(self, instance, **kw)
-      return list(set(r)) if r or not args else args[0]
+      return list(OrderedDict.fromkeys(r)) if r or not args else args[0]
 
 
 def defMethodGetter(key, method=None):

product/ERP5Type/tests/testERP5Type.py

@@ -643,23 +643,27 @@ class TestERP5Type(PropertySheetTestCase, LogInterceptor):
       person.setRegionValueSet([alpha, alpha])
       self.assertEqual(person.getRegionList(), ['alpha'])
       self.assertEqual(person.getRegionSet(), ['alpha'])
+      person.setRegionValueList([beta, alpha, beta])
+      self.assertEqual(person.getRegionList(), ['beta', 'alpha', 'beta'])
+      self.assertEqual(person.getRegionSet(), ['beta', 'alpha']) # Order is kept in Set getter.
+      self.assertEqual(person.getRegionValueSet(), [beta, alpha]) # Order is kept in Set getter.
       person.setRegionValueList([alpha, beta, alpha])
       self.assertEqual(person.getRegionList(), ['alpha', 'beta', 'alpha'])
+      self.assertEqual(person.getRegionSet(), ['alpha', 'beta'])
+      self.assertEqual(person.getRegionValueSet(), [alpha, beta])
       person.setRegionValueSet([alpha, beta, alpha])
-      result = person.getRegionSet()
-      result.sort()
-      self.assertEqual(result, ['alpha', 'beta'])
+      self.assertEqual(person.getRegionList(), ['alpha', 'beta'])
+      person.setRegionValueSet([beta, alpha, zeta, alpha])
+      self.assertEqual(person.getRegionList(), ['alpha', 'beta', 'zeta']) # Default is kept, then order is kept in Set setter.
       person.setDefaultRegionValue(beta)
       self.assertEqual(person.getDefaultRegion(), 'beta')
-      result = person.getRegionSet()
-      result.sort()
-      self.assertEqual(result, ['alpha', 'beta'])
+      self.assertEqual(person.getRegionList(), ['beta', 'alpha', 'zeta'])
+      person.setRegion(None)
+      person.setRegionValueSet([beta, alpha, alpha])
       self.assertEqual(person.getRegionList(), ['beta', 'alpha'])
       person.setDefaultRegionValue(alpha)
       self.assertEqual(person.getDefaultRegion(), 'alpha')
-      result = person.getRegionSet()
-      result.sort()
-      self.assertEqual(result, ['alpha', 'beta'])
+      self.assertEqual(person.getRegionSet(), ['alpha', 'beta'])
       self.assertEqual(person.getRegionList(), ['alpha', 'beta'])
       # Test accessor on documents rather than on categories
       person.setDefaultRegionValue(person)
@@ -679,8 +683,8 @@ class TestERP5Type(PropertySheetTestCase, LogInterceptor):
       person.setRegionList(['alpha', 'alpha'])
       self.assertEqual(person.getRegionList(), ['alpha', 'alpha'])
       self.assertEqual(person.getRegionSet(), ['alpha'])
-      person.setRegionSet(['beta', 'alpha', 'alpha'])
-      self.assertEqual(person.getRegionList(), ['alpha', 'beta'])
+      person.setRegionSet(['beta', 'alpha', 'zeta', 'alpha'])
+      self.assertEqual(person.getRegionList(), ['alpha', 'beta', 'zeta'])
       person.setRegionList(['beta', 'alpha', 'alpha'])
       self.assertEqual(person.getRegionList(), ['beta', 'alpha', 'alpha'])
       # at this point the person have a default region set to the first item in