slapos_slap_tool: do not trigger computer xml recalcultation if not needed

master/bt5/slapos_slap_tool/ExtensionTemplateItem/portal_components/extension.erp5.SlapOSSlapTool.py

@@ -43,3 +43,15 @@ def Item_activateFillComputerInformationCache(state_change):
         computer_reference, computer_reference)
   finally:
     setSecurityManager(sm)
+
+
+@UnrestrictedMethod
+def reindexPartition(item):
+  partition = item.getAggregateValue(portal_type='Computer Partition')
+  if partition is not None:
+    partition.reindexObject()
+
+
+def Instance_reindexComputerPartition(state_change):
+  item = state_change['object']
+  reindexPartition(item)

master/bt5/slapos_slap_tool/TestTemplateItem/portal_components/test.erp5.testSlapOSSlapTool.py

@@ -14,8 +14,15 @@ import xml.dom.ext.reader.Sax
 import xml.dom.ext
 import StringIO
 import difflib
+import hashlib
+from binascii import hexlify
 from OFS.Traversable import NotFound
 
+
+def hashData(data):
+  return hexlify(hashlib.sha1(data).digest())
+
+
 class Simulator:
   def __init__(self, outfile, method):
     self.outfile = outfile
@@ -63,6 +70,166 @@ class TestSlapOSSlapToolMixin(SlapOSTestCaseMixin):
     self.unpinDateTime()
     self._cleaupREQUEST()
 
+
+class TestSlapOSSlapToolgetFullComputerInformation(TestSlapOSSlapToolMixin):
+  def test_activate_getFullComputerInformation_first_access(self):
+    self._makeComplexComputer(with_slave=True)
+    self.portal.REQUEST['disable_isTestRun'] = True
+    self.tic()
+
+    self.login(self.computer_user_id)
+
+    # First access.
+    # Cache has been filled by interaction workflow
+    # (luckily, it seems the cache is filled after everything is indexed)
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    first_etag = self.portal_slap._calculateRefreshEtag()
+    first_body_fingerprint = hashData(
+      self.portal_slap._getCacheComputerInformation(self.computer_id,
+                                                    self.computer_id)
+    )
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    self.assertEqual(first_etag, response.headers.get('etag'))
+    self.assertEqual(first_body_fingerprint, hashData(response.body))
+    self.assertEqual(0, len(self.portal.portal_activities.getMessageList()))
+
+    # Trigger the computer reindexation
+    # This should trigger a new etag, but the body should be the same
+    self.computer.reindexObject()
+    self.commit()
+
+    # Second access
+    # Check that the result is stable, as the indexation timestamp is not changed yet
+    current_activity_count = len(self.portal.portal_activities.getMessageList())
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    self.assertEqual(first_etag, response.headers.get('etag'))
+    self.assertEqual(first_body_fingerprint, hashData(response.body))
+    self.assertEqual(current_activity_count, len(self.portal.portal_activities.getMessageList()))
+
+    self.tic()
+
+    # Third access, new calculation expected
+    # The retrieved informations comes from the cache
+    # But a new cache modification activity is triggered
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    second_etag = self.portal_slap._calculateRefreshEtag()
+    second_body_fingerprint = hashData(
+      self.portal_slap._getCacheComputerInformation(self.computer_id,
+                                                    self.computer_id)
+    )
+    self.assertNotEqual(first_etag, second_etag)
+    # The indexation timestamp does not impact the response body
+    self.assertEqual(first_body_fingerprint, second_body_fingerprint)
+    self.assertEqual(first_etag, response.headers.get('etag'))
+    self.assertEqual(first_body_fingerprint, hashData(response.body))
+    self.assertEqual(1, len(self.portal.portal_activities.getMessageList()))
+
+    # Execute the cache modification activity
+    self.tic()
+
+    # 4th access
+    # The new etag value is now used
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    self.assertEqual(second_etag, response.headers.get('etag'))
+    self.assertEqual(first_body_fingerprint, hashData(response.body))
+    self.assertEqual(0, len(self.portal.portal_activities.getMessageList()))
+
+    # Edit the instance
+    # This should trigger a new etag and a new body
+    self.stop_requested_software_instance.edit(text_content=self.generateSafeXml())
+    self.commit()
+
+    # 5th access
+    # Check that the result is stable, as the indexation timestamp is not changed yet
+    current_activity_count = len(self.portal.portal_activities.getMessageList())
+    # Edition does not impact the etag
+    self.assertEqual(second_etag, self.portal_slap._calculateRefreshEtag())
+    third_body_fingerprint = hashData(
+      self.portal_slap._getCacheComputerInformation(self.computer_id,
+                                                    self.computer_id)
+    )
+    # The edition impacts the response body
+    self.assertNotEqual(first_body_fingerprint, third_body_fingerprint)
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    self.assertEqual(second_etag, response.headers.get('etag'))
+    self.assertEqual(first_body_fingerprint, hashData(response.body))
+    self.assertEqual(current_activity_count, len(self.portal.portal_activities.getMessageList()))
+
+    self.tic()
+
+    # 6th, the instance edition triggered an interaction workflow
+    # which updated the cache
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    third_etag = self.portal_slap._calculateRefreshEtag()
+    self.assertNotEqual(second_etag, third_etag)
+    self.assertEqual(third_etag, response.headers.get('etag'))
+    self.assertEqual(third_body_fingerprint, hashData(response.body))
+    self.assertEqual(0, len(self.portal.portal_activities.getMessageList()))
+
+    # Remove the slave link to the partition
+    # Computer should loose permission to access the slave instance
+    self.start_requested_slave_instance.setAggregate('')
+    self.commit()
+
+    # 7th access
+    # Check that the result is stable, as the indexation timestamp is not changed yet
+    current_activity_count = len(self.portal.portal_activities.getMessageList())
+    # Edition does not impact the etag
+    self.assertEqual(third_etag, self.portal_slap._calculateRefreshEtag())
+    # The edition does not impact the response body yet, as the aggregate relation
+    # is not yet unindex
+    self.assertEqual(third_body_fingerprint, hashData(
+      self.portal_slap._getCacheComputerInformation(self.computer_id,
+                                                    self.computer_id)
+    ))
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    self.assertEqual(third_etag, response.headers.get('etag'))
+    self.assertEqual(third_body_fingerprint, hashData(response.body))
+    self.assertEqual(current_activity_count, len(self.portal.portal_activities.getMessageList()))
+
+    self.tic()
+
+    # 8th access
+    # changing the aggregate relation trigger the partition reindexation
+    # which trigger cache modification activity
+    # So, we should get the correct cached value
+    response = self.portal_slap.getFullComputerInformation(self.computer_id)
+    self.commit()
+    self.assertEqual(200, response.status)
+    self.assertTrue('last-modified' not in response.headers)
+    fourth_etag = self.portal_slap._calculateRefreshEtag()
+    fourth_body_fingerprint = hashData(
+      self.portal_slap._getCacheComputerInformation(self.computer_id,
+                                                    self.computer_id)
+    )
+    self.assertNotEqual(third_etag, fourth_etag)
+    # The indexation timestamp does not impact the response body
+    self.assertNotEqual(third_body_fingerprint, fourth_body_fingerprint)
+    self.assertEqual(fourth_etag, response.headers.get('etag'))
+    self.assertEqual(fourth_body_fingerprint, hashData(response.body))
+    self.assertEqual(0, len(self.portal.portal_activities.getMessageList()))
+
+
 class TestSlapOSSlapToolComputerAccess(TestSlapOSSlapToolMixin):
   def test_getFullComputerInformation(self):
     self._makeComplexComputer(with_slave=True)
@@ -81,7 +248,7 @@ class TestSlapOSSlapToolComputerAccess(TestSlapOSSlapToolMixin):
         response.headers.get('cache-control'))
     self.assertEqual('REMOTE_USER',
         response.headers.get('vary'))
-    self.assertTrue('last-modified' in response.headers)
+    self.assertFalse('etag' in response.headers)
     self.assertEqual('text/xml; charset=utf-8',
         response.headers.get('content-type'))
 
@@ -992,7 +1159,7 @@ class TestSlapOSSlapToolInstanceAccess(TestSlapOSSlapToolMixin):
         response.headers.get('cache-control'))
     self.assertEqual('REMOTE_USER',
         response.headers.get('vary'))
-    self.assertTrue('last-modified' in response.headers)
+    self.assertFalse('etag' in response.headers)
     self.assertEqual('text/xml; charset=utf-8',
         response.headers.get('content-type'))
     # check returned XML

master/bt5/slapos_slap_tool/ToolComponentTemplateItem/portal_components/tool.erp5.SlapTool.py

@@ -201,6 +201,7 @@ class SlapTool(BaseTool):
       self._getCachePlugin().set(key, DEFAULT_CACHE_SCOPE,
         dict (
           time=time.time(),
+          refresh_etag=self._calculateRefreshEtag(),
           data=self._getCacheComputerInformation(computer_id, user),
         ),
         cache_duration=self.getPortalObject().portal_caches\
@@ -275,8 +276,23 @@ class SlapTool(BaseTool):
         )
       )
 
-  def _getComputerInformation(self, computer_id, user):
-    user_document = _assertACI(self.getPortalObject().portal_catalog.unrestrictedGetResultValue(
+  def _calculateRefreshEtag(self):
+    # check max indexation timestamp
+    # it is unlikely to get an empty catalog
+    last_indexed_entry = self.getPortalObject().portal_catalog(
+      select_list=['indexation_timestamp'],
+      portal_type=['Computer', 'Computer Partition',
+                   'Software Instance', 'Slave Instance',
+                   'Software Installation'],
+      sort_on=[('indexation_timestamp', 'DESC')],
+      limit=1,
+    )[0]
+    return '%s_%s' % (last_indexed_entry.uid,
+                      last_indexed_entry.indexation_timestamp)
+
+  def _getComputerInformation(self, computer_id, user, refresh_etag):
+    portal = self.getPortalObject()
+    user_document = _assertACI(portal.portal_catalog.unrestrictedGetResultValue(
       reference=user, portal_type=['Person', 'Computer', 'Software Instance']))
     user_type = user_document.getPortalType()
     self.REQUEST.response.setHeader('Content-Type', 'text/xml; charset=utf-8')
@@ -287,21 +303,26 @@ class SlapTool(BaseTool):
     if user_type in ('Computer', 'Person'):
       if not self._isTestRun():
         cache_plugin = self._getCachePlugin()
+        key = '%s_%s' % (computer_id, user)
         try:
-          key = '%s_%s' % (computer_id, user)
           entry = cache_plugin.get(key, DEFAULT_CACHE_SCOPE)
         except KeyError:
           entry = None
+
         if entry is not None and isinstance(entry.getValue(), dict):
-          result = entry.getValue()['data']
-          self._activateFillComputerInformationCache(computer_id, user)
-          return result
+          cached_dict = entry.getValue()
+          cached_etag = cached_dict.get('refresh_etag', None)
+          if (refresh_etag != cached_etag):
+            # Do not recalculate the computer information
+            # if nothing changed
+            self._activateFillComputerInformationCache(computer_id, user)
+          return cached_dict['data'], cached_etag
         else:
           self._activateFillComputerInformationCache(computer_id, user)
           self.REQUEST.response.setStatus(503)
-          return self.REQUEST.response
+          return self.REQUEST.response, None
       else:
-        return self._getCacheComputerInformation(computer_id, user)
+        return self._getCacheComputerInformation(computer_id, user), None
     else:
       slap_computer._software_release_list = []
 
@@ -319,7 +340,7 @@ class SlapTool(BaseTool):
                     portal_type="Computer Partition")
 
     self._calculateSlapComputerInformation(slap_computer, computer_partition_list)
-    return dumps(slap_computer)
+    return dumps(slap_computer), None
 
   @UnrestrictedMethod
   def _getHostingSubscriptionIpList(self, computer_id, computer_partition_id):
@@ -360,7 +381,8 @@ class SlapTool(BaseTool):
     user = self.getPortalObject().portal_membership.getAuthenticatedMember().getUserName()
     if str(user) == computer_id:
       self._logAccess(user, user, '#access %s' % computer_id)
-    result = self._getComputerInformation(computer_id, user)
+    refresh_etag = self._calculateRefreshEtag()
+    body, etag = self._getComputerInformation(computer_id, user, refresh_etag)
 
     if self.REQUEST.response.getStatus() == 200:
       # Keep in cache server for 7 days
@@ -368,11 +390,12 @@ class SlapTool(BaseTool):
                                       'public, max-age=1, stale-if-error=604800')
       self.REQUEST.response.setHeader('Vary',
                                       'REMOTE_USER')
-      self.REQUEST.response.setHeader('Last-Modified', rfc1123_date(DateTime()))
-      self.REQUEST.response.setBody(result)
+      if etag is not None:
+        self.REQUEST.response.setHeader('Etag', etag)
+      self.REQUEST.response.setBody(body)
       return self.REQUEST.response
     else:
-      return result
+      return body
 
   security.declareProtected(Permissions.AccessContentsInformation,
     'getHostingSubscriptionIpList')

master/bt5/slapos_slap_tool/WorkflowTemplateItem/portal_workflow/slapos_slap_tool_interaction_workflow/interactions/Instance_setAggregate.xml

+<?xml version="1.0"?>
+<ZopeData>
+  <record id="1" aka="AAAAAAAAAAE=">
+    <pickle>
+      <global name="InteractionDefinition" module="Products.ERP5.Interaction"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>actbox_category</string> </key>
+            <value> <string>workflow</string> </value>
+        </item>
+        <item>
+            <key> <string>actbox_name</string> </key>
+            <value> <string></string> </value>
+        </item>
+        <item>
+            <key> <string>actbox_url</string> </key>
+            <value> <string></string> </value>
+        </item>
+        <item>
+            <key> <string>activate_script_name</string> </key>
+            <value>
+              <tuple/>
+            </value>
+        </item>
+        <item>
+            <key> <string>after_script_name</string> </key>
+            <value>
+              <tuple/>
+            </value>
+        </item>
+        <item>
+            <key> <string>before_commit_script_name</string> </key>
+            <value>
+              <tuple/>
+            </value>
+        </item>
+        <item>
+            <key> <string>description</string> </key>
+            <value> <string></string> </value>
+        </item>
+        <item>
+            <key> <string>guard</string> </key>
+            <value>
+              <none/>
+            </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>Instance_setAggregate</string> </value>
+        </item>
+        <item>
+            <key> <string>method_id</string> </key>
+            <value>
+              <list>
+                <string>_setAggregate.*</string>
+              </list>
+            </value>
+        </item>
+        <item>
+            <key> <string>once_per_transaction</string> </key>
+            <value> <int>1</int> </value>
+        </item>
+        <item>
+            <key> <string>portal_type_filter</string> </key>
+            <value>
+              <list>
+                <string>Slave Instance</string>
+                <string>Software Installation</string>
+              </list>
+            </value>
+        </item>
+        <item>
+            <key> <string>portal_type_group_filter</string> </key>
+            <value>
+              <none/>
+            </value>
+        </item>
+        <item>
+            <key> <string>script_name</string> </key>
+            <value>
+              <list>
+                <string>Instance_reindexComputerPartition</string>
+              </list>
+            </value>
+        </item>
+        <item>
+            <key> <string>temporary_document_disallowed</string> </key>
+            <value> <int>1</int> </value>
+        </item>
+        <item>
+            <key> <string>title</string> </key>
+            <value> <string></string> </value>
+        </item>
+        <item>
+            <key> <string>trigger_type</string> </key>
+            <value> <int>2</int> </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+</ZopeData>

master/bt5/slapos_slap_tool/WorkflowTemplateItem/portal_workflow/slapos_slap_tool_interaction_workflow/scripts/Instance_reindexComputerPartition.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>Instance_reindexComputerPartition</string> </value>
+        </item>
+        <item>
+            <key> <string>_module</string> </key>
+            <value> <string>SlapOSSlapTool</string> </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>Instance_reindexComputerPartition</string> </value>
+        </item>
+        <item>
+            <key> <string>title</string> </key>
+            <value> <string></string> </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+</ZopeData>