Merge pull request #16 from zopefoundation/maurits-apply-dependency-o…

…nly-once

Apply profile dependencies only once.

Products/GenericSetup/events.py

@@ -50,5 +50,6 @@ def handleProfileImportedEvent(event):
 
     version = event.tool.getVersionForProfile(profile_id)
     if version and version != 'unknown':
-        profile_id = profile_id[len('profile-'):]
+        prefix = 'profile-'
+        profile_id = profile_id[len(prefix):]
         event.tool.setLastVersionForProfile(profile_id, version)

Products/GenericSetup/registry.py

@@ -644,6 +644,12 @@ def __init__(self):
     def getProfileInfo(self, profile_id, for_=None):
         """ See IProfileRegistry.
         """
+        prefixes = ('profile-', 'snapshot-')
+        for prefix in prefixes:
+            if profile_id.startswith(prefix):
+                profile_id = profile_id[len(prefix):]
+                break
+
         result = self._registered.get(profile_id)
         if result is None:
             raise KeyError(profile_id)

Products/GenericSetup/tests/test_registry.py

@@ -1056,6 +1056,13 @@ def test_registerProfile_normal( self ):
         self.assertEqual( info[ 'type' ], PROFILE_TYPE )
         self.assertEqual( info[ 'for' ], None )
 
+        # We strip off any 'profile-' or 'snapshot-' at the beginning
+        # of the profile id.
+        info2 = registry.getProfileInfo( 'profile-' + PROFILE_ID )
+        self.assertEqual(info, info2)
+        info3 = registry.getProfileInfo( 'snapshot-' + PROFILE_ID )
+        self.assertEqual(info, info3)
+
     def test_registerProfile_duplicate( self ):
 
         NAME = 'one'

Products/GenericSetup/tests/test_tool.py

@@ -28,6 +28,7 @@
 from Products.GenericSetup.interfaces import IBeforeProfileImportEvent
 from Products.GenericSetup.interfaces import IProfileImportedEvent
 from Products.GenericSetup.testing import ExportImportZCMLLayer
+from Products.GenericSetup.tests.common import _makeTestFile
 from Products.GenericSetup.tests.common import BaseRegistryTests
 from Products.GenericSetup.tests.common import DummyExportContext
 from Products.GenericSetup.tests.common import DummyImportContext
@@ -67,6 +68,20 @@ def handleProfileImportedEvent(event):
   </dependencies>
 </metadata>
 """
+_DOUBLE_METADATA_XML = """<?xml version="1.0"?>
+<metadata>
+  <version>1.0</version>
+  <dependencies>
+    <dependency>profile-other:bar</dependency>
+    <dependency>profile-other:ham</dependency>
+  </dependencies>
+</metadata>
+"""
+_PLAIN_METADATA_XML = """<?xml version="1.0"?>
+<metadata>
+  <version>1.0</version>
+</metadata>
+"""
 
 
 class SetupToolTests(FilesystemTestBase, TarballTester, ConformsToISetupTool):
@@ -75,6 +90,7 @@ class SetupToolTests(FilesystemTestBase, TarballTester, ConformsToISetupTool):
 
     _PROFILE_PATH = '/tmp/STT_test'
     _PROFILE_PATH2 = '/tmp/STT_test2'
+    _PROFILE_PATH3 = '/tmp/STT_test3'
 
     def afterSetUp(self):
         from Products.GenericSetup.upgrade import _upgrade_registry
@@ -558,6 +574,131 @@ def applyContext(context):
                                           ignore_dependencies=False)
         self.assertEqual(_imported, [self._PROFILE_PATH2, self._PROFILE_PATH])
 
+    def _setup_dependency_strategy_test_tool(self):
+        # If we add a dependency profile in our metadata.xml, and this
+        # dependency was already applied, then we do not need to apply
+        # it yet again.  Once is quite enough, thank you.  Running any
+        # upgrade steps would be nice though.  There are options.
+        # Setup a tool and profiles for testing dependency strategies.
+        from Products.GenericSetup.metadata import METADATA_XML
+        self._makeFile(METADATA_XML, _DOUBLE_METADATA_XML)
+        _makeTestFile(METADATA_XML, self._PROFILE_PATH2, _PLAIN_METADATA_XML)
+        _makeTestFile(METADATA_XML, self._PROFILE_PATH3, _PLAIN_METADATA_XML)
+        site = self._makeSite()
+        tool = self._makeOne('setup_tool').__of__(site)
+
+        # Register main profile and two dependency profiles.
+        profile_registry.registerProfile('foo', 'Foo', '', self._PROFILE_PATH)
+        profile_registry.registerProfile('bar', 'Bar', '', self._PROFILE_PATH2)
+        profile_registry.registerProfile('ham', 'Ham', '', self._PROFILE_PATH3)
+
+        # Apply the second profile.
+        tool.runAllImportStepsFromProfile('profile-other:bar')
+
+        # Register an upgrade step.  Note that applying this step will
+        # set the profile version to 1.1, even though the metadata of
+        # the profile really says 1.0.  We will use this to check
+        # whether the upgrade step has been applied (version is 1.1)
+        # or the full profile has been applied (version is 1.0).
+        step_bar = UpgradeStep(
+            "Upgrade", "other:bar", '1.0', '1.1', '', dummy_upgrade, None, "1")
+        _registerUpgradeStep(step_bar)
+        # And another one.
+        step_ham = UpgradeStep(
+            "Upgrade", "other:ham", '1.0', '1.1', '', dummy_upgrade, None, "1")
+        _registerUpgradeStep(step_ham)
+
+        # Gather list of imported profiles.
+        tool._imported = []
+
+        def applyContext(context):
+            tool._imported.append(context._profile_path)
+
+        tool.applyContext = applyContext
+
+        return tool
+
+    def test_runAllImportStepsFromProfile_with_default_strategy(self):
+        # Default strategy: apply new profiles, upgrade old profiles.
+        tool = self._setup_dependency_strategy_test_tool()
+
+        # Run the main profile.
+        tool.runAllImportStepsFromProfile('profile-other:foo')
+        # The main and third profile have been applied.
+        self.assertEqual(tool._imported,
+                         [self._PROFILE_PATH3, self._PROFILE_PATH])
+        # The upgrade step of the second profile has been applied,
+        # pushing it to version 1.1.
+        self.assertEqual(tool.getLastVersionForProfile('other:bar'),
+                         ('1', '1'))
+        # Third profile is at 1.0.
+        self.assertEqual(tool.getLastVersionForProfile('other:ham'),
+                         ('1', '0'))
+
+    def test_runAllImportStepsFromProfile_with_reapply_strategy(self):
+        # You can choose the old behavior of always applying the
+        # dependencies.  This ignores any upgrade steps.
+        tool = self._setup_dependency_strategy_test_tool()
+
+        # Run the main profile.
+        from Products.GenericSetup.tool import DEPENDENCY_STRATEGY_REAPPLY
+        tool.runAllImportStepsFromProfile(
+            'profile-other:foo',
+            dependency_strategy=DEPENDENCY_STRATEGY_REAPPLY)
+        # All three profiles have been applied.
+        self.assertEqual(tool._imported,
+                         [self._PROFILE_PATH2, self._PROFILE_PATH3,
+                          self._PROFILE_PATH])
+        self.assertEqual(tool.getLastVersionForProfile('other:bar'),
+                         ('1', '0'))
+        self.assertEqual(tool.getLastVersionForProfile('other:ham'),
+                         ('1', '0'))
+
+    def test_runAllImportStepsFromProfile_with_new_strategy(self):
+        # You can choose to be happy with any applied version and
+        # ignore any upgrade steps.
+        tool = self._setup_dependency_strategy_test_tool()
+
+        # Run the main profile.
+        from Products.GenericSetup.tool import DEPENDENCY_STRATEGY_NEW
+        tool.runAllImportStepsFromProfile(
+            'profile-other:foo',
+            dependency_strategy=DEPENDENCY_STRATEGY_NEW)
+        # The main and third profile have been applied.
+        self.assertEqual(tool._imported,
+                         [self._PROFILE_PATH3, self._PROFILE_PATH])
+        # Second profile stays at 1.0.
+        self.assertEqual(tool.getLastVersionForProfile('other:bar'),
+                         ('1', '0'))
+        self.assertEqual(tool.getLastVersionForProfile('other:ham'),
+                         ('1', '0'))
+
+    def test_runAllImportStepsFromProfile_with_ignore_strategy(self):
+        # You can choose to be ignore all dependency profiles.
+        tool = self._setup_dependency_strategy_test_tool()
+
+        # Run the main profile.
+        from Products.GenericSetup.tool import DEPENDENCY_STRATEGY_IGNORE
+        tool.runAllImportStepsFromProfile(
+            'profile-other:foo',
+            dependency_strategy=DEPENDENCY_STRATEGY_IGNORE)
+        # Only the main profile has been applied.
+        self.assertEqual(tool._imported,
+                         [self._PROFILE_PATH])
+        # Second profile stays at 1.0.
+        self.assertEqual(tool.getLastVersionForProfile('other:bar'),
+                         ('1', '0'))
+        # Third profile is not applied.
+        self.assertEqual(tool.getLastVersionForProfile('other:ham'),
+                         ('unknown'))
+
+    def test_runAllImportStepsFromProfile_unknown_strategy(self):
+        site = self._makeSite()
+        tool = self._makeOne('setup_tool').__of__(site)
+        profile_registry.registerProfile('foo', 'Foo', '', self._PROFILE_PATH)
+        self.assertRaises(ValueError, tool.runAllImportStepsFromProfile,
+                          'profile-other:foo', dependency_strategy='random')
+
     def test_runAllImportStepsFromProfile_set_last_profile_version(self):
         from Products.GenericSetup.metadata import METADATA_XML
 
@@ -987,6 +1128,21 @@ def test_profileVersioning(self):
         self.assertEqual(tool.getLastVersionForProfile(profile_id),
                          ('1', '1'))
 
+    def test_get_and_setLastVersionForProfile(self):
+        site = self._makeSite()
+        site.setup_tool = self._makeOne('setup_tool')
+        tool = site.setup_tool
+        self.assertEqual(tool._profile_upgrade_versions, {})
+        # Any 'profile-' is stripped off in these calls.
+        self.assertEqual(tool.getLastVersionForProfile('foo'), 'unknown')
+        self.assertEqual(tool.getLastVersionForProfile('profile-foo'), 'unknown')
+        tool.setLastVersionForProfile('foo', '1.0')
+        self.assertEqual(tool.getLastVersionForProfile('foo'), ('1', '0'))
+        self.assertEqual(tool.getLastVersionForProfile('profile-foo'), ('1', '0'))
+        tool.setLastVersionForProfile('profile-foo', '2.0')
+        self.assertEqual(tool.getLastVersionForProfile('foo'), ('2', '0'))
+        self.assertEqual(tool.getLastVersionForProfile('profile-foo'), ('2', '0'))
+
     def test_manage_doUpgrades_no_profile_id_or_updates(self):
         site = self._makeSite()
         site.setup_tool = self._makeOne('setup_tool')

Products/GenericSetup/tests/test_zcml.py

@@ -166,6 +166,24 @@ def test_registerUpgradeStep(self):
       ('1', '1')
       >>> step.handler
       <function dummy_upgrade at ...>
+      >>> step_id = step.id
+      >>> step_id == keys[0]
+      True
+
+    Get the step in a different way::
+
+      >>> step2 = _ur.getUpgradeStep('default', step_id)
+      >>> step == step2
+      True
+
+    We strip off any 'profile-' at the beginning of the profile id::
+
+      >>> profile_steps2 = _ur.getUpgradeStepsForProfile('profile-default')
+      >>> profile_steps == profile_steps2
+      True
+      >>> step3 = _ur.getUpgradeStep('profile-default', step_id)
+      >>> step == step3
+      True
 
     Clean up and make sure the cleanup works::