Propagate activate_kw when edit successor

See merge request nexedi/slapos.core!581

master/bt5/slapos_cloud/ExtensionTemplateItem/portal_components/extension.erp5.SlapOSCloud.py

@@ -25,6 +25,7 @@
 #
 ##############################################################################
 
+from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
 from AccessControl.SecurityManagement import getSecurityManager, \
              setSecurityManager, newSecurityManager
 from Products.ERP5Security import SUPER_USER
@@ -283,6 +284,13 @@ def Computer_checkComputeNodeMigrationConsistency(self, fixit=False):
 
   return error_list
 
+def Base_setTransactionalTag(self, tag, REQUEST=None):
+  getTransactionalVariable()[str(tag)] = str(tag)
+
+def Base_getTransactionalTag(self, tag, REQUEST=None):
+  if REQUEST is not None:
+    raise Unauthorized
+  return getTransactionalVariable().get(tag, None) 
 
 def Base_updateRelatedContentWithoutReindextion(self, previous_category_url, new_category_url, REQUEST=None):
   """ This method indeed reimplements the updateRelatedContent but it uses 

master/bt5/slapos_cloud/ExtensionTemplateItem/portal_components/extension.erp5.SlapOSCloud.xml

@@ -6,12 +6,6 @@
     </pickle>
     <pickle>
       <dictionary>
-        <item>
-            <key> <string>_recorded_property_dict</string> </key>
-            <value>
-              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
-            </value>
-        </item>
         <item>
             <key> <string>default_reference</string> </key>
             <value> <string>SlapOSCloud</string> </value>
@@ -55,28 +49,13 @@
         <item>
             <key> <string>workflow_history</string> </key>
             <value>
-              <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
+              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
             </value>
         </item>
       </dictionary>
     </pickle>
   </record>
   <record id="2" aka="AAAAAAAAAAI=">
-    <pickle>
-      <global name="PersistentMapping" module="Persistence.mapping"/>
-    </pickle>
-    <pickle>
-      <dictionary>
-        <item>
-            <key> <string>data</string> </key>
-            <value>
-              <dictionary/>
-            </value>
-        </item>
-      </dictionary>
-    </pickle>
-  </record>
-  <record id="3" aka="AAAAAAAAAAM=">
     <pickle>
       <global name="PersistentMapping" module="Persistence.mapping"/>
     </pickle>
@@ -89,7 +68,7 @@
                 <item>
                     <key> <string>component_validation_workflow</string> </key>
                     <value>
-                      <persistent> <string encoding="base64">AAAAAAAAAAQ=</string> </persistent>
+                      <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
                     </value>
                 </item>
               </dictionary>
@@ -98,7 +77,7 @@
       </dictionary>
     </pickle>
   </record>
-  <record id="4" aka="AAAAAAAAAAQ=">
+  <record id="3" aka="AAAAAAAAAAM=">
     <pickle>
       <global name="WorkflowHistoryList" module="Products.ERP5Type.Workflow"/>
     </pickle>

master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Base_getTransactionalTag.xml

+<?xml version="1.0"?>
+<ZopeData>
+  <record id="1" aka="AAAAAAAAAAE=">
+    <pickle>
+      <global name="ExternalMethod" module="Products.ExternalMethod.ExternalMethod"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>_function</string> </key>
+            <value> <string>Base_getTransactionalTag</string> </value>
+        </item>
+        <item>
+            <key> <string>_module</string> </key>
+            <value> <string>SlapOSCloud</string> </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>Base_getTransactionalTag</string> </value>
+        </item>
+        <item>
+            <key> <string>title</string> </key>
+            <value> <string></string> </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+</ZopeData>

master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Base_setTransactionalTag.xml

+<?xml version="1.0"?>
+<ZopeData>
+  <record id="1" aka="AAAAAAAAAAE=">
+    <pickle>
+      <global name="ExternalMethod" module="Products.ExternalMethod.ExternalMethod"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>_function</string> </key>
+            <value> <string>Base_setTransactionalTag</string> </value>
+        </item>
+        <item>
+            <key> <string>_module</string> </key>
+            <value> <string>SlapOSCloud</string> </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>Base_setTransactionalTag</string> </value>
+        </item>
+        <item>
+            <key> <string>title</string> </key>
+            <value> <string></string> </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+</ZopeData>

master/bt5/slapos_cloud/TestTemplateItem/portal_components/test.erp5.testSlapOSCloudInstanceSlapInterfaceWorkflow.py

@@ -20,7 +20,6 @@
 ##############################################################################
 from erp5.component.test.SlapOSTestCaseMixin import SlapOSTestCaseMixin
 import transaction
-from unittest import expectedFailure
 from time import sleep
 from zExceptions import Unauthorized
 
@@ -441,7 +440,6 @@ class TestSlapOSCoreInstanceSlapInterfaceWorkflow(SlapOSTestCaseMixin):
     self.assertRaises(NotImplementedError, B_instance.requestInstance,
         **request_kw)
 
-  @expectedFailure
   def test_request_tree_change_same_transaction(self):
     """Checks tree change forced by request
 
@@ -472,6 +470,66 @@ class TestSlapOSCoreInstanceSlapInterfaceWorkflow(SlapOSTestCaseMixin):
     self.assertRaises(NotImplementedError, B_instance.requestInstance,
         **request_kw)
 
+  def test_request_tree_edit_parameters_change_not_indexed(self):
+    """Checks tree change forced by request
+
+    For a tree like:
+
+    A
+    |
+    A
+    |\
+    B C
+    |
+    D
+
+    When C requests D tree in a while C requests D, but before indexation,
+    the system shall disallow the operation."""
+    request_kw = self.request_kw.copy()
+
+    request_kw['software_title'] = self.generateNewSoftwareTitle()
+    self.software_instance.requestInstance(**request_kw)
+    B_instance = self.software_instance.REQUEST.get('request_instance')
+
+    request_kw['software_title'] = self.generateNewSoftwareTitle()
+    self.software_instance.requestInstance(**request_kw)
+    C_instance = self.software_instance.REQUEST.get('request_instance')
+
+    request_kw['software_title'] = self.generateNewSoftwareTitle()
+    B_instance.requestInstance(**request_kw)
+    D_instance = self.software_instance.REQUEST.get('request_instance')
+
+    self.assertSameSet(
+        self.software_instance.getSuccessorList(),
+        [B_instance.getRelativeUrl(), C_instance.getRelativeUrl()])
+
+    self.assertSameSet(
+        B_instance.getSuccessorList(), [D_instance.getRelativeUrl()])
+
+    # Ensure all is indexed first
+    self.tic()
+  
+    # B edits twice w/o a problem w/o indexation problems
+    request_kw['instance_xml'] = self.generateSafeXml()
+    B_instance.requestInstance(**request_kw)
+    
+    transaction.commit()
+    C_instance.requestInstance(**request_kw)
+    
+    transaction.commit()
+    self.assertSameSet(
+        C_instance.getSuccessorList(), [D_instance.getRelativeUrl()])
+
+    self.assertSameSet(
+        B_instance.getSuccessorList(), [])
+
+
+    # B request must fails since indexation didnt finished up properly
+    # which would lead to dupplicated set on sucessor
+    self.assertRaises(NotImplementedError, B_instance.requestInstance,
+        **request_kw)
+
+
   def test_request_started_stopped_destroyed(self):
     request_kw = self.request_kw.copy()
 
@@ -706,7 +764,6 @@ class TestSlapOSCoreInstanceSlapInterfaceWorkflow(SlapOSTestCaseMixin):
     self.assertRaises(NotImplementedError, B_instance.requestInstance,
         **request_kw)
 
-  @expectedFailure
   def test_request_tree_change_same_transaction_shared(self):
     """Checks tree change forced by request
 
@@ -1160,7 +1217,6 @@ class TestSlapOSCoreInstanceSlapInterfaceWorkflowTransfer(SlapOSTestCaseMixin):
     self.assertTrue(serial in self.software_instance.getSslCertificate())
     self.assertTrue(certificate_login.getReference() in \
        self.software_instance.getSslCertificate().decode('string_escape'))
-
     self.assertRaises(ValueError, self.software_instance.generateCertificate)
 
   def test_revokeCertificate(self):

master/bt5/slapos_cloud/WorkflowTemplateItem/portal_workflow/instance_slap_interface_workflow/script_RequesterInstance_request.py

@@ -30,7 +30,7 @@ else:
 # Prevent creating two instances with the same title
 instance_tree.serialize()
 tag = "%s_%s_inProgress" % (instance_tree.getUid(), software_title)
-if (portal.portal_activities.countMessageWithTag(tag) > 0):
+if (portal.portal_activities.countMessageWithTag(tag) > 0) or context.Base_getTransactionalTag(tag):
   # The software instance is already under creation but can not be fetched from catalog
   # As it is not possible to fetch informations, it is better to raise an error
   raise NotImplementedError(tag)
@@ -74,6 +74,7 @@ if (request_software_instance is None):
   else:
     instance_found = True
     # First time that the software instance is requested
+    successor = None
 
     # Create a new one
     reference = "SOFTINST-%s" % portal.portal_ids.generateNewId(
@@ -105,8 +106,8 @@ else:
   successor = request_software_instance.getSuccessorRelatedValue(portal_type="Software Instance")
   if (successor is None):
     # Check if the precessor is a Instance Tree
-    instance_tree_precessesor = request_software_instance.getSuccessorRelatedValue(portal_type="Instance Tree")
-    if (requester_instance.getPortalType() != "Instance Tree" and instance_tree_precessesor is not None):
+    instance_tree_successor = request_software_instance.getSuccessorRelatedValue(portal_type="Instance Tree")
+    if (requester_instance.getPortalType() != "Instance Tree" and instance_tree_successor is not None):
       raise ValueError('It is disallowed to request root software instance %s' % request_software_instance.getRelativeUrl())
     else:
       successor = requester_instance
@@ -114,10 +115,14 @@ else:
       if request_software_instance.getUid() not in graph:
         graph[request_software_instance.getUid()] = request_software_instance.getSuccessorUidList()
 
-  successor_uid_list = successor.getSuccessorUidList()
-  if request_software_instance.getUid() in successor_uid_list:
-    successor_uid_list.remove(request_software_instance.getUid())
-    successor.edit(successor_uid_list=successor_uid_list)
+  successor_uid_list = successor.getSuccessorUidList()  
+  if successor != requester_instance:
+    if request_software_instance.getUid() in successor_uid_list:
+      successor_uid_list.remove(request_software_instance.getUid())
+      successor.edit(
+        successor_uid_list=successor_uid_list,
+        activate_kw={'tag': tag}
+      )
   graph[successor.getUid()] = successor_uid_list
 
 if instance_found:
@@ -132,6 +137,7 @@ if instance_found:
   }
   request_software_instance_url = request_software_instance.getRelativeUrl()
   context.REQUEST.set('request_instance', request_software_instance)
+  context.Base_setTransactionalTag(tag)
   if (root_state == "started"):
     request_software_instance.requestStart(**promise_kw)
   elif (root_state == "stopped"):
@@ -142,7 +148,11 @@ if instance_found:
   else:
     raise ValueError, "state should be started, stopped or destroyed"
 
-  successor_list = requester_instance.getSuccessorList() + [request_software_instance_url]
+  successor_list = requester_instance.getSuccessorList()
+  successor_uid_list = requester_instance.getSuccessorUidList()
+  if successor != requester_instance:
+    successor_list.append(request_software_instance_url)
+    successor_uid_list.append(request_software_instance.getUid())
   uniq_successor_list = list(set(successor_list))
   successor_list.sort()
   uniq_successor_list.sort()
@@ -150,13 +160,16 @@ if instance_found:
   assert successor_list == uniq_successor_list, "%s != %s" % (successor_list, uniq_successor_list)
 
   # update graph to reflect requested operation
-  graph[requester_instance.getUid()] = requester_instance.getSuccessorUidList() + [request_software_instance.getUid()]
+  graph[requester_instance.getUid()] = successor_uid_list
 
   # check if all elements are still connected and if there is no cycle
   request_software_instance.checkConnected(graph, instance_tree.getUid())
   request_software_instance.checkNotCyclic(graph)
 
-  requester_instance.edit(successor_list=successor_list)
-
+  if successor != requester_instance:
+    requester_instance.edit(
+      successor_list=successor_list,
+      activate_kw={'tag': tag}
+    )
 else:
   context.REQUEST.set('request_instance', None)