Amount Generator: make sorting by dependency resolution really stable

product/ERP5/mixin/amount_generator.py

@@ -168,7 +168,25 @@ class BaseAmountDict(Implicit):
 
 class BaseAmountResolver(BaseAmountDict):
 
-  _dummy_property_dict = {'_index': 0},
+  class _node(set):
+
+    contribution_dict = None
+
+    def __init__(self, property_dict, contribution_dict):
+      self.property_dict = property_dict
+      self.contribution_dict = contribution_dict
+
+    def __call__(self):
+      if self.contribution_dict:
+        contribution_dict_get = self.contribution_dict.get
+        del self.contribution_dict
+        for application in list(self):
+          for node in contribution_dict_get(application, ()):
+            self |= node()
+      return self
+
+    def __lt__(self, other):
+      return not other().isdisjoint(self.property_dict['_contribution'])
 
   def __init__(self, cache, method_kw):
     self._dict = cache.setdefault(None, {})
@@ -180,25 +198,18 @@ class BaseAmountResolver(BaseAmountDict):
       recurseApplicationDependencies = \
         self.__of__(delivery_amount).getGeneratedAmountQuantity
       contribution_dict = defaultdict(list)
+      node_list = []
       for property_dict in property_dict_list:
+        node = self._resolving = self._node(property_dict, contribution_dict)
+        node_list.append(node)
         self._amount_generator_line = property_dict[None]
-        self._resolving = property_dict['_index'] = set()
         for variated_base_amount in property_dict['_application']:
           recurseApplicationDependencies(*variated_base_amount)
         for variated_base_amount in property_dict['_contribution']:
-          contribution_dict[variated_base_amount].append(property_dict)
+          contribution_dict[variated_base_amount].append(node)
       del self._resolving
-      contribution_dict.default_factory = lambda: self._dummy_property_dict
-      def sort_key(property_dict):
-        score = property_dict['_index']
-        if type(score) is set:
-          score = property_dict['_index'] = 1 + max(sort_key(x)
-            for x in score
-            for x in contribution_dict[x])
-        return score
-      property_dict_list.sort(key=sort_key)
-      for property_dict in property_dict_list:
-        del property_dict['_index']
+      node_list.sort()
+      property_dict_list[:] = (node.property_dict for node in node_list)
 
   def getBaseAmountList(self):
     return ()

product/ERP5/tests/testTradeModelLine.py

@@ -884,33 +884,46 @@ return context""" % (base_amount, base_amount))
   def test_05_dependencyResolution(self):
     from Products.ERP5Type.Document import newTempAmount, newTempTradeModelLine
     from Products.ERP5.mixin.amount_generator import BaseAmountResolver
-    resolver = BaseAmountResolver({}, {})
+    delivery_amount = newTempAmount(self.portal, '')
     trade_model_line = newTempTradeModelLine(self.portal, '')
+    def case(*lines):
+      return BaseAmountResolver({}, {}), [{
+        None: trade_model_line,
+        'index': index,
+        '_application': [(x, ()) for x in application],
+        '_contribution': [(x, ()) for x in contribution],
+      } for index, application, contribution in lines]
+    def check():
+      resolver(delivery_amount, property_dict_list)
+      self.assertEqual(range(len(property_dict_list)),
+                       [x['index'] for x in property_dict_list])
+
+    # Case 1: calculation of some base_amount depends on others.
     trade_model_line.getBaseAmountQuantity = \
       lambda delivery_amount, base_amount: sum(map(
         delivery_amount.getGeneratedAmountQuantity,
         application_dict.get(base_amount, base_amount)))
     application_dict = dict(B='bf', C='c', E='Bef')
-    property_dict_list = [{
-        None: trade_model_line,
-        'index': index,
-        '_application': [(x, ()) for x in application],
-        '_contribution': [(x, ()) for x in contribution],
-      } for index, application, contribution in (
-        (2, 'C', 'e'),
-        (3, 'dE', ''),
-        (0, 'a', 'b'),
-        (1, 'B', 'cd'),
-      )]
-    delivery_amount = newTempAmount(self.portal, '')
-    resolver(delivery_amount, property_dict_list)
-    self.assertEqual(range(len(property_dict_list)),
-                     [x['index'] for x in property_dict_list])
+    resolver, property_dict_list = case((2, 'C', 'e'),
+                                        (3, 'dE', ''),
+                                        (0, 'a', 'b'),
+                                        (1, 'B', 'cd'))
+    check()
     # Retry with cache already filled.
     property_dict_list.reverse()
-    resolver(delivery_amount, property_dict_list)
-    self.assertEqual(range(len(property_dict_list)),
-                     [x['index'] for x in property_dict_list])
+    check()
+    del trade_model_line.getBaseAmountQuantity
+
+    # Case 2: sorting by dependency resolution must be stable.
+    # This is important for compatibility in case that calculation of D
+    # applies d conditionally, whereas the user took care of ordering 3 after 2
+    # with indices: 3 must remain after 2.
+    resolver, property_dict_list = case((0, 'a', 'b'),
+                                        (1, 'b', 'c'),
+                                        (2, 'c', 'd'),
+                                        (3, 'bD', ''))
+    check()
+
 
   def test_tradeModelLineWithFixedPrice(self):
     """