Merge pull request #2503 from plone/issue-2502-51-combine-bundles-reg…

…istry-directory

Fixed combine-bundles step to work with keyword in registry directory.

CHANGES.rst

@@ -19,6 +19,10 @@ New features:
 
 Bug fixes:
 
+- Let the ``combine-bundles`` import step also work when the ``IBundleRegistry`` keyword is not in ``registry.xml``, but in a ``registry`` directory.
+  `Issue 2520 <https://github.com/plone/Products.CMFPlone/issues/2502>`_.
+  [maurits]
+
 - Get rid of obsolete ``X-UA-Compatible`` header.
   [hvelarde]
 

Products/CMFPlone/resources/exportimport/bundles.py

@@ -14,22 +14,37 @@ def combine(context):
         logger.info("Cannot find registry")
         return
 
-    body = context.readDataFile('registry.xml')
-    if body and "IBundleRegistry" in body:
-        # Calling combine_bundles will have as side effect that the
-        # Content-Type header of the response is set to application/javascript,
-        # which we do not want.  So we reset it to the original at the end.
-        site = context.getSite()
-        request = getattr(site, 'REQUEST', getRequest())
-        if request is not None:
-            # Easily happens in tests.
-            orig_header = request.response.getHeader('Content-Type')
-        combine_bundles(site)
-        if request is not None:
-            new_header = request.response.getHeader('Content-Type')
-            if new_header != orig_header:
-                if orig_header is None:
-                    # Setting it to None would result in the string 'None'.
-                    # So pick a saner one.
-                    orig_header = 'text/html'
-                request.response.setHeader('Content-Type', orig_header)
+    # Look for a keyword in registry.xml or the registry directory.
+    filepaths = ['registry.xml']
+    if context.isDirectory('registry'):
+        for filename in context.listDirectory('registry'):
+            filepaths.append('registry/' + filename)
+    found = False
+    for filepath in filepaths:
+        body = context.readDataFile(filepath)
+        if body is not None and 'IBundleRegistry' in body:
+            found = True
+            break
+    if not found:
+        return
+
+    # Calling combine_bundles will have as side effect that the
+    # Content-Type header of the response is set to application/javascript,
+    # which we do not want.  So we reset it to the original at the end.
+    site = context.getSite()
+    request = getattr(site, 'REQUEST', getRequest())
+    # In tests the request can easily be None.
+    if request is not None:
+        orig_header = request.response.getHeader('Content-Type')
+    combine_bundles(site)
+    if request is None:
+        # we are done
+        return
+    new_header = request.response.getHeader('Content-Type')
+    if new_header == orig_header:
+        return
+    if orig_header is None:
+        # Setting it to None would result in the string 'None'.
+        # So pick a saner one.
+        orig_header = 'text/html'
+    request.response.setHeader('Content-Type', orig_header)

Products/CMFPlone/tests/test_metabundles.py

@@ -10,6 +10,20 @@
     PRODUCTION_RESOURCE_DIRECTORY,
     combine_bundles,
 )
+from Products.GenericSetup.tests import common
+
+
+class DummyImportContext(common.DummyImportContext):
+    # Copied from plone.app.registry tests.
+    # This expands the context with directories.
+
+    _directories = {}
+
+    def listDirectory(self, path):
+        return self._directories.get(path, [])
+
+    def isDirectory(self, path):
+        return path in self._directories
 
 
 class ProductsCMFPloneSetupTest(PloneTestCase):
@@ -51,3 +65,76 @@ def test_overrides(self):
             'alert("Overrided legacy!");',
             self.production_folder.readFile('default.js')
         )
+
+    def test_import(self):
+        # If IBundleRegistry is in registry.xml, the combine-bundles import step
+        # will call combine_bundles.
+        from Products.CMFPlone.resources.exportimport.bundles import combine
+        # from Products.CMFPlone.resources.browser.combine import get_override_directory
+        # from Products.CMFPlone.resources.browser.combine import PRODUCTION_RESOURCE_DIRECTORY
+
+        # Prepare some registry xml files with or without the key IBundleRegistry.
+        xml_with = '<registry>with IBundleRegistry</registry>'
+        xml_without = '<registry>without bundle registry</registry>'
+        xml_without2 = '<registry>without bundle registry</registry>'
+        context = DummyImportContext(self.portal, purge=False)
+
+        def get_timestamp():
+            # If combine_bundles is run, a timestamp is updated.
+            return self.production_folder.readFile('timestamp.txt')
+
+        ts1 = get_timestamp()
+        self.assertTrue(ts1)
+
+        # call the import step on a file without bundles
+        context._files = {'registry.xml': xml_without}
+        combine(context)
+        ts2 = get_timestamp()
+        self.assertEqual(ts1, ts2)
+
+        # call the import step on a file with bundles
+        context._files = {'registry.xml': xml_with}
+        combine(context)
+        ts3 = get_timestamp()
+        self.assertLess(ts2, ts3)
+
+        # call the import step on a file without bundles
+        context._files = {'registry.xml': xml_without2}
+        combine(context)
+        ts4 = get_timestamp()
+        self.assertEqual(ts3, ts4)
+
+        # Since Plone 5.1 the registry xml can also be a directory.
+        # Set one file with bundles.
+        context._files = {
+            'registry.xml': xml_without,
+            'registry/foo2.xml': xml_with,
+            'registry/foo3.xml': xml_without2,
+        }
+        context._directories = {
+            'registry': [
+                'foo2.xml',
+                'foo3.xml',
+            ]
+        }
+        combine(context)
+        ts10 = get_timestamp()
+        self.assertLess(ts4, ts10)
+
+        # The registry.xml file itself may be missing.
+        context._files = {
+            'registry/foo2.xml': xml_with,
+            'registry/foo3.xml': xml_without2,
+        }
+        combine(context)
+        ts11 = get_timestamp()
+        self.assertLess(ts10, ts11)
+
+        # Now without any bundle info.
+        context._files = {
+            'registry/foo2.xml': xml_without,
+            'registry/foo3.xml': xml_without2,
+        }
+        combine(context)
+        ts12 = get_timestamp()
+        self.assertEqual(ts11, ts12)