Simulation: index 'delivery' categories in ZODB

Use of catalog to get related simulation movements from delivery lines/cells is
unreliable.

Until now, for any new code written for simulation, we often had to be careful
not to call getDeliveryRelatedValueList too early, usually by deferring code
to activities with complicated dependencies. Race conditions are difficult to
avoid by developping this way, because for a given delivery, there are
potentially so many events that happen at the same time, involving:
- simulation, amongst causality, expand, building, solving (including split)
- alarms, user actions, external interfaces, chains of activities
- several related deliveries and simulation trees

This commit enables ZODB-indexing of related documents for 'delivery' base
category, making getDeliveryRelatedValueList safe and fixing unlink of
deleted delivery lines/cells.

Existing activity dependencies are left unchanged because builders only uses
catalog and local building needs to find all simulation movements.

product/ERP5/Document/Delivery.py

@@ -394,6 +394,7 @@ class Delivery(XMLObject, ImmobilisationDelivery, SimulableMixin,
       if calculate:
         path = self.getPath()
         self.activate(
+          # after_tag to built: could be removed
           after_tag=('built:'+path, 'expand:'+path),
           after_path_and_method_id=(path, '_localBuild'),
           ).updateCausalityState()
@@ -713,6 +714,10 @@ class Delivery(XMLObject, ImmobilisationDelivery, SimulableMixin,
         after_tag = [after_tag]
       else:
         after_tag = list(after_tag) if after_tag else []
+      # Now that 'delivery' category relation are indexed in ZODB, this is the
+      # only method that depends on built: tag (via _updateSimulation), which
+      # is still required because builders only use catalog to find buildable
+      # movements and we don't want to miss any for local building.
       after_tag.append('expand:' + self.getPath())
       sm = getSecurityManager()
       newSecurityManager(None, nobody)
@@ -857,12 +862,11 @@ class Delivery(XMLObject, ImmobilisationDelivery, SimulableMixin,
 
       return disconnected_simulation_movement_list
 
-    def _getAllRelatedSimulationMovementList(self, **kw):
-      movement_uid_list = [x.getUid() for x in self.getMovementList()]
-      return movement_uid_list and \
-        self.getPortalObject().portal_catalog.unrestrictedSearchResults(
-          portal_type='Simulation Movement',
-          delivery_uid=movement_uid_list, **kw)
+    def _getAllRelatedSimulationMovementList(self):
+      result = []
+      for movement in self.getMovementList():
+        result += movement.getDeliveryRelatedValueList()
+      return result
 
     def getDivergentTesterAndSimulationMovementList(self):
       """

product/ERP5/Document/DeliveryRootSimulationRule.py

@@ -101,35 +101,11 @@ class DeliveryRuleMovementGenerator(MovementGeneratorMixin):
       return []
     else:
       result = []
-      existing_movement_list = self._applied_rule.objectValues()
       for movement in delivery.getMovementList(
         portal_type=self._getPortalDeliveryMovementTypeList()):
-        simulation_movement = self._getDeliveryRelatedSimulationMovement(movement)
-        if simulation_movement is None or \
-               simulation_movement in existing_movement_list:
+        simulation_movement_list = movement.getDeliveryRelatedValueList()
+        if not simulation_movement_list or self._applied_rule in (
+            simulation_movement.getParentValue()
+            for simulation_movement in simulation_movement_list):
           result.append(movement)
       return result
-
-  def _getDeliveryRelatedSimulationMovement(self, delivery_movement):
-    """Helper method to get the delivery related simulation movement.
-    This method is more robust than simply calling getDeliveryRelatedValue
-    which will not work if simulation movements are not indexed.
-    """
-    simulation_movement = delivery_movement.getDeliveryRelatedValue()
-    if simulation_movement is not None:
-      return simulation_movement
-    # simulation movement was not found, maybe simply because it's not indexed
-    # yet. We'll look in the simulation tree and try to find it anyway before
-    # creating another simulation movement.
-    # Try to find the one from trade model rule, which is the most common case
-    # where we may expand again before indexation of simulation movements is
-    # finished.
-    delivery = delivery_movement.getExplanationValue()
-    for movement in delivery.getMovementList():
-      related_simulation_movement = movement.getDeliveryRelatedValue()
-      if related_simulation_movement is not None:
-        for applied_rule in related_simulation_movement.contentValues():
-          for simulation_movement in applied_rule.contentValues():
-            if simulation_movement.getDeliveryValue() == delivery_movement:
-              return simulation_movement
-    return None

product/ERP5/Document/InvoiceRootSimulationRule.py

@@ -28,14 +28,12 @@
 """
 """
 
-import zope.interface
 from AccessControl import ClassSecurityInfo
-from Products.ERP5Type import Permissions, PropertySheet, interfaces
-from Products.ERP5.mixin.rule import RuleMixin, MovementGeneratorMixin
-from Products.ERP5.mixin.movement_collection_updater import \
-     MovementCollectionUpdaterMixin
+from Products.ERP5Type import Permissions
+from Products.ERP5.Document.DeliveryRootSimulationRule \
+     import DeliveryRootSimulationRule, DeliveryRuleMovementGenerator
 
-class InvoiceRootSimulationRule(RuleMixin, MovementCollectionUpdaterMixin):
+class InvoiceRootSimulationRule(DeliveryRootSimulationRule):
   """
   """
   # CMF Type Definition
@@ -46,24 +44,6 @@ class InvoiceRootSimulationRule(RuleMixin, MovementCollectionUpdaterMixin):
   security = ClassSecurityInfo()
   security.declareObjectProtected(Permissions.AccessContentsInformation)
 
-  # Declarative interfaces
-  zope.interface.implements(interfaces.IRule,
-                            interfaces.IDivergenceController,
-                            interfaces.IMovementCollectionUpdater,)
-
-  # Default Properties
-  property_sheets = (
-    PropertySheet.Base,
-    PropertySheet.XMLObject,
-    PropertySheet.CategoryCore,
-    PropertySheet.DublinCore,
-    PropertySheet.Task,
-    PropertySheet.Predicate,
-    PropertySheet.Reference,
-    PropertySheet.Version,
-    PropertySheet.Rule
-    )
-
   security.declareProtected(Permissions.AccessContentsInformation,
                             'isAccountable')
   def isAccountable(self, movement):
@@ -81,62 +61,9 @@ class InvoiceRootSimulationRule(RuleMixin, MovementCollectionUpdaterMixin):
     """
     return InvoiceRuleMovementGenerator(applied_rule=context, rule=self)
 
-  def _getMovementGeneratorContext(self, context):
-    """
-    Return the movement generator context to use for expand
-    """
-    return context
+class InvoiceRuleMovementGenerator(DeliveryRuleMovementGenerator):
 
-  def _getMovementGeneratorMovementList(self, context):
-    """
-    Return the movement lists to provide to the movement generator
-    """
-    return []
-
-  def _isProfitAndLossMovement(self, movement):
-    # For a kind of trade rule, a profit and loss movement lacks source
-    # or destination.
-    return (movement.getSource() is None or movement.getDestination() is None)
-
-class InvoiceRuleMovementGenerator(MovementGeneratorMixin):
-
-  def _getInputMovementList(self, movement_list=None, rounding=None):
-    """Input movement list comes from delivery"""
-    delivery = self._applied_rule.getDefaultCausalityValue()
-    if delivery is None:
-      return []
-    else:
-      ret = []
-      existing_movement_list = self._applied_rule.objectValues()
-      for movement in delivery.getMovementList(
-        portal_type=(delivery.getPortalInvoiceMovementTypeList() + \
-                     delivery.getPortalTaxMovementTypeList())): # This is bad XXX-JPS - use use
-        simulation_movement = self._getDeliveryRelatedSimulationMovement(movement)
-        if simulation_movement is None or \
-               simulation_movement in existing_movement_list:
-          ret.append(movement)
-      return ret
-
-  def _getDeliveryRelatedSimulationMovement(self, delivery_movement):
-    """Helper method to get the delivery related simulation movement.
-    This method is more robust than simply calling getDeliveryRelatedValue
-    which will not work if simulation movements are not indexed.
-    """
-    simulation_movement = delivery_movement.getDeliveryRelatedValue()
-    if simulation_movement is not None:
-      return simulation_movement
-    # simulation movement was not found, maybe simply because it's not indexed
-    # yet. We'll look in the simulation tree and try to find it anyway before
-    # creating another simulation movement.
-    # Try to find the one from trade model rule, which is the most common case
-    # where we may expand again before indexation of simulation movements is
-    # finished.
-    delivery = delivery_movement.getExplanationValue()
-    for movement in delivery.getMovementList():
-      related_simulation_movement = movement.getDeliveryRelatedValue()
-      if related_simulation_movement is not None:
-        for applied_rule in related_simulation_movement.contentValues():
-          for simulation_movement in applied_rule.contentValues():
-            if simulation_movement.getDeliveryValue() == delivery_movement:
-              return simulation_movement
-    return None
+  def _getPortalDeliveryMovementTypeList(self): # This is bad XXX-JPS - use use
+    portal = self._rule.getPortalObject()
+    return portal.getPortalInvoiceMovementTypeList() + \
+           portal.getPortalTaxMovementTypeList()

product/ERP5/Document/Movement.py

@@ -41,7 +41,6 @@ from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
 from Products.ERP5.mixin.amount_generator import AmountGeneratorMixin
 from Products.ERP5.mixin.composition import CompositionMixin
 from Products.ERP5.Document.Amount import Amount
-from Products.ERP5.Document.SimulatedDeliveryBuilder import BUILDING_KEY
 
 from zLOG import LOG, WARNING
 
@@ -595,11 +594,8 @@ class Movement(XMLObject, Amount, CompositionMixin, AmountGeneratorMixin):
       simulation movement.
     """
     simulation_movement = self.getDeliveryRelatedValue()
-    if simulation_movement is not None and \
-       not simulation_movement.getParentValue().isRootAppliedRule():
-      return True
-    building = getTransactionalVariable().get(BUILDING_KEY, ())
-    return self in building or self.getRootDeliveryValue() in building
+    return simulation_movement is not None and \
+       not simulation_movement.getParentValue().isRootAppliedRule()
 
   # New Causality API
   security.declareProtected( Permissions.AccessContentsInformation,

product/ERP5/Document/SimulatedDeliveryBuilder.py

@@ -35,11 +35,6 @@ from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
 from Products.ERP5Type.UnrestrictedMethod import UnrestrictedMethod
 from Products.ERP5Type.CopySupport import CopyError, tryMethodCallWithTemporaryPermission
 
-# Quite ugly way to avoid useless expand when building lines
-# in a delivery that already have lines and a root applied rule.
-# For example, it's normally useless to expand after building
-# accounting lines in an invoice with manually created invoice lines.
-BUILDING_KEY = 'building_from_portal_simulation'
 
 class SimulatedDeliveryBuilder(BuilderMixin):
   """
@@ -145,9 +140,6 @@ class SimulatedDeliveryBuilder(BuilderMixin):
       and delivery movement.
     """
     delivery = delivery_movement.getExplanationValue()
-    building = getTransactionalVariable()[BUILDING_KEY]
-    if delivery in building:
-      building.add(delivery_movement)
     simulation_movement.recursiveReindexObject(activate_kw=dict(
       activate_kw or (), tag='built:'+delivery.getPath()))
     BuilderMixin._setDeliveryMovementProperties(
@@ -380,25 +372,6 @@ class SimulatedDeliveryBuilder(BuilderMixin):
 
     return delivery
 
-  def _processDeliveryLineGroup(self, delivery, movement_group_node,
-                                *args, **kw):
-    building = getTransactionalVariable().setdefault(BUILDING_KEY, set())
-    if None in building:
-      super(SimulatedDeliveryBuilder, self)._processDeliveryLineGroup(
-          delivery, movement_group_node, *args, **kw)
-      return
-    building.add(None)
-    try:
-      for movement in movement_group_node.getMovementList():
-        if not movement.isTempDocument():
-          building.add(delivery)
-          break
-      super(SimulatedDeliveryBuilder, self)._processDeliveryLineGroup(
-        delivery, movement_group_node, *args, **kw)
-    finally:
-      building.remove(None)
-      building.discard(delivery)
-
   def _createDeliveryLine(self, delivery, movement_list, activate_kw):
     """
       Refer to the docstring in GeneratedDeliveryBuilder.

product/ERP5/bootstrap/erp5_core/CategoryTemplateItem/portal_categories/delivery.xml

@@ -76,7 +76,9 @@
         </item>
         <item>
             <key> <string>description</string> </key>
-            <value> <string></string> </value>
+            <value>
+              <none/>
+            </value>
         </item>
         <item>
             <key> <string>fallback_base_category</string> </key>
@@ -128,6 +130,10 @@
               <none/>
             </value>
         </item>
+        <item>
+            <key> <string>related_locally_indexed</string> </key>
+            <value> <int>1</int> </value>
+        </item>
         <item>
             <key> <string>rid</string> </key>
             <value>

product/ERP5/bootstrap/erp5_core/bt/revision

-41061
\ No newline at end of file
+41062
\ No newline at end of file

product/ERP5/mixin/movement_collection_updater.py

@@ -192,4 +192,8 @@ class MovementCollectionUpdaterMixin:
       del movement.__dict__
       movement = newMovement()
       d.update(movement.__dict__)
+      categories = d.pop('categories')
       movement.__dict__ = d
+      # Force update of local indexes on linked objects
+      # (important for 'delivery').
+      movement._setCategoryList(categories)

product/ERP5/mixin/rule.py

@@ -509,7 +509,9 @@ class SimulableMixin(Base):
               activity='SQLQueue',
               group_method_id='portal_rules/updateSimulation',
               tag='expand:' + path,
-              after_tag='built:'+ path, # see SimulatedDeliveryBuilder
+              # Now that we don't rely on catalog anymore, this after_tag could
+              # moved to _localBuild, which currently only depends on 'expand:'.
+              after_tag='built:'+ path,
               priority=3,
               )._updateSimulation(**kw)
         del tv[key]

product/ERP5/tests/testRule.py

@@ -26,7 +26,6 @@
 ##############################################################################
 
 import unittest
-from Products.ERP5Type.tests.backportUnittest import expectedFailure
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
 from Products.ERP5Type.tests.utils import createZODBPythonScript
 from Products.ERP5.tests.testOrder import TestOrderMixin
@@ -690,10 +689,12 @@ return context.generatePredicate(
       for m in message_list if m.method_id == 'immediateReindexObject'))
     root_applied_rule, = self.pl.getCausalityRelatedValueList()
     sm, = root_applied_rule.objectValues()
-    self.assertEqual([], sm.getDeliveryValue().getDeliveryRelatedList())
+    line = sm.getDeliveryValue()
+    self.assertEqual([sm], line.getDeliveryRelatedValueList())
+    self.assertEqual([], [x.getObject() for x in self.portal.portal_catalog
+      .unrestrictedSearchResults(delivery_uid=line.getUid())])
     return root_applied_rule
 
-  @expectedFailure
   def test_13_unlinkSimulation(self):
     """
     When a root delivery line is deleted, the related simulation movement