BusinessTemplate: update tool if they have properties

Since 248f59e5 (Business Template: Likewise ERP5Site.addERP5Tool(), do
not re-create Tool if it already exists., 2020-06-22) we no longer
update tools, because of the problem that business template does not
currently handle updating objects with lots of sub-objects.

But we realized that we really need to update tools when they contain
configuration as object attributes, like mimetypes_registry, where the
problem was observed.

Instead of unconditionnally skipping any tool during update, we inspect
the tool we are about to install and only skip the tool if it looks
already OK: the __class__ is already correct and the new tool does not
have any properties in their __dict__

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testBusinessTemplate.py

@@ -5140,6 +5140,157 @@ class TestBusinessTemplate(BusinessTemplateMixin):
     sequence_list.addSequenceString(sequence_string)
     sequence_list.play(self)
 
+  def test_tool_update(self):
+    self.portal.newContent(
+        id='dummy_tool',
+        portal_type="Alarm Tool")
+
+    # export and install a first version
+    bt = self.portal.portal_templates.newContent(
+        portal_type='Business Template',
+        title='test_bt_%s' % self.id(),
+        template_tool_id_list=('dummy_tool', ))
+    self.tic()
+    bt.build()
+    self.tic()
+    self.portal.manage_delObjects(ids=['dummy_tool'])
+    self.commit()
+    export_dir = tempfile.mkdtemp()
+    try:
+      bt.export(path=export_dir, local=True)
+      self.tic()
+      self.portal.portal_templates.updateBusinessTemplateFromUrl(
+          download_url='file:/%s' % export_dir)
+    finally:
+      shutil.rmtree(export_dir)
+
+    self.tic()
+
+    # build a new version of this business template, where the tool export some
+    # configuration (as some plain object attributes). In this case, we expect
+    # that updating the business template will set this configuration on the tool.
+    self.portal.dummy_tool._some_configuration = "saved in business template"
+    bt.build()
+    self.tic()
+    # undo the change in title
+    del self.portal.dummy_tool._some_configuration
+
+    export_dir = tempfile.mkdtemp()
+    try:
+      bt.export(path=export_dir, local=True)
+      self.tic()
+      new_bt = self.portal.portal_templates.updateBusinessTemplateFromUrl(
+          download_url='file:/%s' % export_dir)
+    finally:
+      shutil.rmtree(export_dir)
+
+    try:
+      self.assertEqual(self.portal.dummy_tool._some_configuration, "saved in business template")
+    finally:
+      new_bt.uninstall()
+      self.assertIsNone(self.portal._getOb('dummy_tool', None))
+
+  def test_move_tool_to_another_business_template(self):
+    """test the case of a tool that was initially in a business template, but later
+    was moved to another business template.
+
+    This test update business templates. In the first state we have:
+     - bt1 which contain a "dummy_tool" tool
+     - bt2 which contain nothing
+    in the second state, we update so that:
+     - bt1 contain nothing
+     - bt2 contain the "dummy_tool"
+
+    Because this tool did not really change in business templates, we expect that updating
+    bt2 will not update the tool.
+    In a real-life scenario, we also expect that updating b1 does not remove the tool, but
+    this is not really automatic, this rely on the developer to correctly package bt1 and
+    list the tool in the "keep paths".
+    """
+    self.portal.newContent(
+        id='dummy_tool',
+        portal_type="Alarm Tool")
+
+    bt1_initial = self.portal.portal_templates.newContent(
+        portal_type='Business Template',
+        title='test_bt1_%s' % self.id(),
+        template_tool_id_list=('dummy_tool', ))
+    self.tic()
+    bt1_initial.build()
+    self.tic()
+
+    # install a first version of test_bt2 where dummy_tool was not here,
+    # to check the update case.
+    bt2_initial = self.portal.portal_templates.newContent(
+        portal_type='Business Template',
+        title='test_bt2_%s' % self.id())
+    bt2_initial.build()
+    self.tic()
+    bt2_initial.install()
+    self.tic()
+
+    bt2 = self.portal.portal_templates.newContent(
+        portal_type='Business Template',
+        title='test_bt2_%s' % self.id(),
+        template_tool_id_list=('dummy_tool', ))
+    self.tic()
+    bt2.build()
+    self.tic()
+
+    self.portal.manage_delObjects(ids=['dummy_tool'])
+    self.commit()
+    bt1 = self.portal.portal_templates.newContent(
+        portal_type='Business Template',
+        title='test_bt1_%s' % self.id(),
+        template_tool_id_list=(),
+        template_keep_path_list=('dummy_tool',)
+    )
+    self.tic()
+    bt1.build()
+    self.tic()
+
+    export_dir = tempfile.mkdtemp()
+    try:
+      bt1_initial.export(path=export_dir, local=True)
+      self.tic()
+      self.portal.portal_templates.updateBusinessTemplateFromUrl(
+          download_url='file:/%s' % export_dir)
+    finally:
+      shutil.rmtree(export_dir)
+
+    self.portal.dummy_tool._touched_in_update = False
+    self.commit()
+    # install the new bt2 and check that this does not re-install the tool.
+    # This is especially important for tool which contain lots of documents (such as portal_simulation)
+    export_dir = tempfile.mkdtemp()
+    try:
+      bt2.export(path=export_dir, local=True)
+      self.tic()
+      new_bt2 = self.portal.portal_templates.updateBusinessTemplateFromUrl(
+          download_url='file:/%s' % export_dir)
+    finally:
+      shutil.rmtree(export_dir)
+
+    self.assertFalse(self.portal.dummy_tool._touched_in_update)
+
+    # install the new version of bt1, where tool was removed, this
+    # does not remove the tool (because we made the business template correctly and did not forget
+    # to list the tool in template_keep_path_list)
+    export_dir = tempfile.mkdtemp()
+    try:
+      bt1.export(path=export_dir, local=True)
+      self.tic()
+      self.portal.portal_templates.updateBusinessTemplateFromUrl(
+          download_url='file:/%s' % export_dir)
+    finally:
+      shutil.rmtree(export_dir)
+    self.assertIsNotNone(self.portal._getOb('dummy_tool', None))
+
+    # cleanup
+    new_bt2.uninstall()
+    self.commit()
+    self.assertIsNone(self.portal._getOb('dummy_tool', None))
+
   def test_21_CategoryIncludeSubobjects(self):
     """Test Category includes subobjects"""
     sequence_list = SequenceList()

product/ERP5/Document/BusinessTemplate.py

@@ -1813,18 +1813,54 @@ class PathTemplateItem(ObjectTemplateItem):
 
 class ToolTemplateItem(PathTemplateItem):
   """This class is used only for making a distinction between other objects
-  and tools, because tools may not be backed up."""
+  and tools, because tools may not be backed up.
+
+  Also, some tools can contain lots of sub-documents, like for example
+  simulation tool which contain all applied rules, currently we are not
+  able to update them, because business template does support updating a
+  document with so many documents (that's why we special case ModuleTemplateItem).
+  But we want to update tools, so we have a special preinstall method that
+  will skip tools if they already "look up to date".
+  """
+
   def _backupObject(self, action, trashbin, container_path, object_id, **kw):
     """Fake as if a trashbin is not available."""
-    return PathTemplateItem._backupObject(self, action, None, container_path,
+    return super(ToolTemplateItem, self)._backupObject(action, None, container_path,
                                           object_id, **kw)
 
   def preinstall(self, context, installed_item, **kw):
-    object_dict = ObjectTemplateItem.preinstall(self, context, installed_item, **kw)
+    """Try to not install/update the tool, unless it already exists
+    and seems different.
+    """
+    object_dict = super(ToolTemplateItem, self).preinstall(context, installed_item, **kw)
     portal_base = aq_base(context.getPortalObject())
+
+    def canIgnoreExistingToolUpdate(existing_object, new_object):
+      # When a path already exist, we usually update it, but for tool which
+      # contain documents, we want to be a bit more clever so that we
+      # don't update the tool if it is already up to date. This allows to
+      # support the case of tools that are moved from one business template to another.
+
+      # if the new object does not actually exist, we can not ignore.
+      if new_object is None:
+        return False
+
+      # if classes are different, we definitely needs to update existing tool
+      if existing_object.__class__ != new_object.__class__:
+        return False
+
+      # If the new tool have more properties in its __dict__ than the
+      # default properties from all ERP5 documents, we must update it.
+      new_object._p_activate()
+      new_object_dict = new_object.__dict__
+      new_object_dict = new_object_dict.get('__Broken_state__', new_object_dict)
+      return not any(
+          k for k in new_object_dict.keys()
+          if k not in set(('__ac_local_roles__', '_count', '_mt_index', '_tree', 'portal_type', 'id', 'uid', 'title')))
+
     for path, (action, type_name) in object_dict.items():
       obj = getattr(portal_base, path, None)
-      if obj is not None:
+      if obj is not None and canIgnoreExistingToolUpdate(obj, self._objects.get(path)):
         if action == 'New':
           del object_dict[path]
         elif action == 'Modified':