Merge lp:~free.ekanayaka/python-zope-fixtures/adapter-fixture into lp:python-zope-fixtures

[Free Ekanayaka]

I realized that the ComponentsFixture might not be a good idea, or at least it probably doesn't match the typical use cases one would have in a Zope-based application.

The reason is that it uses the low-level sethook() API to override the current registry (aka site manager), while Zope actually supports an higher-level API in the zope.components.hooks module that seems precisely meant to support thread-local/request-specific overrides via the setHooks() and setSite() APIs. This higher-level API is indeed implemented on top of sethook().

I believe ComponentsFixture should be dropped and maybe replaced with a SiteFixture which would use the setHooks/setSite APIs to offer a similar behavior, but more in line with what the Zope publisher does, and hence more suited for writing unit-tests.

Said that, this branch heads into that direction and re-implements UtilityFixture without using ComponentsFixture under the hood, and merely using whatever site manager is currently in-force (thus letting test code be in control of the site manager). It also adds an AdapterFixture which pretty much does the same, but with adapters.

It should be possible to virtually get the same features of ComponentsFixture using UtilityFixture and AdapterFixture.

[Free Ekanayaka (free.ekanayaka)]

FWIW more information on the canonical use of hooks in a Zope application can be found in zope/component/hooks.txt and zope/site/site.txt.

[Robert Collins (lifeless)]

I think this:
76 +
77 + # Check if an utility for the same interface and name exists, if so
78 + # restore it upon cleanup, otherwise just unregister our one.
79 + original = sm.queryUtility(self._provided, self._name)
80 + if original is not None:
81 + self.addCleanup(sm.registerUtility, original, self._provided,
82 + self._name)
83 + else:
84 + self.addCleanup(sm.unregisterUtility, self._component,
85 + self._provided, self._name)
86 +
87 + # Register the utility
88 + sm.registerUtility(self._component, self._provided, self._name)
might be clearer as
76 +
79 + original = sm.queryUtility(self._provided, self._name)
88 + sm.registerUtility(self._component, self._provided, self._name)
80 + if original is not None:
77 + # Restore any existing utility.
81 + self.addCleanup(sm.registerUtility, original, self._provided,
82 + self._name)
87 + # Register the utility
84 + self.addCleanup(sm.unregisterUtility, self._component,
85 + self._provided, self._name)

Which due to the LIFO nature of cleanups will just always unregister the utility we are using, and restore any previous utility - this is easier to reason about - it doesn't depend on the silent-replace aspect of registerUtility, and the perf cost is trivial (one call to unregisterUtility).

48 + The utility will be unregistered upon cleanUp, or the original utility
49 + will be restored if there was one.

Perhaps: "The registration will be reverted upon cleanUp, restoring the previous utility (or None)."

same pattern in adapterFixture. I wonder if we can pull that logic sideways and make it independent.

I'll merge what you have and see about a refactoring branch.

review: Approve

[Robert Collins (lifeless)]

Ok, so it comes together okish, but not enough better to worry about just yet - I think if callmany were more easily accessible it would be that little bit better and worth doing.

[Robert Collins (lifeless)]

Also, you forgot a NEWS entry ;).

[Robert Collins (lifeless)]

BTW, do you have more changes, or should we release this?

[Free Ekanayaka (free.ekanayaka)]

Hi Robert,

thanks for the careful review. Yeah, I thought too about unifying the logic but it just looked not enough worth yet as you point. I'm fine with the proposed change in the addCleanup calls, if you feel like committing it go ahead. Good catch on the NEWS entry, and yes please release it, I don't have further changes.

=== modified file 'lib/zope_fixtures/__init__.py'
--- lib/zope_fixtures/__init__.py	2012-04-26 02:58:05 +0000
+++ lib/zope_fixtures/__init__.py	2012-09-13 12:45:41 +0000
@@ -36,9 +36,11 @@
 __all__ = [
     'ComponentsFixture',
     'UtilityFixture',
+    'AdapterFixture',
     ]
 
-from zope_fixtures.components import ComponentsFixture, UtilityFixture
+from zope_fixtures.components import (
+    ComponentsFixture, UtilityFixture, AdapterFixture)
 
 
 def test_suite():

=== modified file 'lib/zope_fixtures/components.py'
--- lib/zope_fixtures/components.py	2012-04-26 02:52:29 +0000
+++ lib/zope_fixtures/components.py	2012-09-13 12:45:41 +0000
@@ -16,6 +16,7 @@
 __all__ = [
     'ComponentsFixture',
     'UtilityFixture',
+    'AdapterFixture',
     ]
 
 from fixtures import Fixture
@@ -23,6 +24,9 @@
 
 Components = try_import("zope.component.registry.Components")
 getSiteManager = try_import("zope.component.getSiteManager")
+_getUtilityProvided = try_import("zope.component.registry._getUtilityProvided")
+_getAdapterProvided = try_import("zope.component.registry._getAdapterProvided")
+_getAdapterRequired = try_import("zope.component.registry._getAdapterRequired")
 
 
 class ComponentsFixture(Fixture):
@@ -65,19 +69,83 @@
 
 
 class UtilityFixture(Fixture):
-    """Convenience around ComponentsFixture to override a single utility."""
-
-    def __init__(self, *args, **kwargs):
+    """Fixture for registering or overriding a single utility.
+
+    The utility will be unregistered upon cleanUp, or the original utility
+    will be restored if there was one.
+
+    The registration is performed agaist the current site manager.
+    """
+
+    def __init__(self, component, provided=None, name=u""):
         """Create an UtilityFixture.
 
-        The args and kwargs parameters are the same supported by
-        registerUtility and will be passed to it.
+        The parameters are the same supported by registerUtility and will
+        be passed to it.
         """
         super(UtilityFixture, self).__init__()
-        self.args = args
-        self.kwargs = kwargs
+        self._component = component
+        self._provided = provided or _getUtilityProvided(component)
+        self._name = name
 
     def setUp(self):
         super(UtilityFixture, self).setUp()
-        components = self.useFixture(ComponentsFixture())
-        components.registry.registerUtility(*self.args, **self.kwargs)
+        # We use the current site manager so we honor whatever hooks have
+        # been set
+        sm = getSiteManager()
+
+        # Check if an utility for the same interface and name exists, if so
+        # restore it upon cleanup, otherwise just unregister our one.
+        original = sm.queryUtility(self._provided, self._name)
+        if original is not None:
+            self.addCleanup(sm.registerUtility, original, self._provided,
+                            self._name)
+        else:
+            self.addCleanup(sm.unregisterUtility, self._component,
+                            self._provided, self._name)
+
+        # Register the utility
+        sm.registerUtility(self._component, self._provided, self._name)
+
+
+class AdapterFixture(Fixture):
+    """Fixture for registering or overriding a single adapter.
+
+    The adapter will be unregistered upon cleanUp, or the original adapter
+    will be restored if there was one.
+
+    The registration is performed agaist the current site manager.
+    """
+
+    def __init__(self, factory, required=None, provided=None, name=u""):
+        """Create an AdapterFixture.
+
+        The parameters are the same supported by registerAdpater and will
+        be passed to it.
+        """
+        super(AdapterFixture, self).__init__()
+        self._factory = factory
+        self._required = _getAdapterRequired(factory, required)
+        self._provided = provided or _getAdapterProvided(factory)
+        self._name = name
+
+    def setUp(self):
+        super(AdapterFixture, self).setUp()
+        # We use the current site manager so we honor whatever hooks have
+        # been set
+        sm = getSiteManager()
+
+        # Check if an utility for the same interface and name exists, if so
+        # restore it upon cleanup, otherwise just unregister our one.
+        original = sm.adapters.registered(self._required, self._provided,
+                                          self._name)
+        if original is not None:
+            self.addCleanup(sm.registerAdapter, original, self._required,
+                            self._provided, self._name)
+        else:
+            self.addCleanup(sm.unregisterAdapter, self._factory, self._required,
+                            self._provided, self._name)
+
+        # Register the utility
+        sm.registerAdapter(self._factory, self._required, self._provided,
+                           self._name)

=== modified file 'lib/zope_fixtures/tests/test_components.py'
--- lib/zope_fixtures/tests/test_components.py	2012-04-26 02:52:29 +0000
+++ lib/zope_fixtures/tests/test_components.py	2012-09-13 12:45:41 +0000
@@ -16,10 +16,12 @@
 import testtools
 
 from zope.interface import Interface, implements
-from zope.component import provideUtility, queryUtility, getSiteManager
+from zope.component import (
+    provideUtility, queryUtility, getSiteManager, queryAdapter, adapts)
 from zope.component.registry import Components
 
-from zope_fixtures import ComponentsFixture, UtilityFixture
+from zope_fixtures import (
+    ComponentsFixture, UtilityFixture, AdapterFixture)
 
 
 class ITestUtility(Interface):
@@ -30,6 +32,26 @@
     implements(ITestUtility)
 
 
+class ITestContext(Interface):
+    pass
+
+
+class TestContext(object):
+    implements(ITestContext)
+
+
+class ITestAdapter(Interface):
+    pass
+
+
+class TestAdapter(object):
+    implements(ITestAdapter)
+    adapts(ITestContext)
+
+    def __init__(self, context):
+        self.context = context
+
+
 class TestComponentsFixture(testtools.TestCase):
 
     def test_overlays_site_manager(self):
@@ -98,14 +120,60 @@
         self.useFixture(fixture)
         self.assertIs(utility, queryUtility(ITestUtility))
 
-    def test_restore_utility(self):
+    def test_unregister_utility(self):
         utility = TestUtility()
         with UtilityFixture(utility):
             self.assertIs(utility, queryUtility(ITestUtility))
         self.assertIs(None, queryUtility(ITestUtility))
 
+    def test_restore_original_utility(self):
+        original = TestUtility()
+        sm = getSiteManager()
+        sm.registerUtility(original)
+        self.addCleanup(sm.unregisterUtility, original)
+        utility = TestUtility()
+        with UtilityFixture(utility):
+            self.assertIs(utility, queryUtility(ITestUtility))
+        self.assertIs(original, queryUtility(ITestUtility))
+
     def test_arguments_passthrough(self):
         utility = TestUtility()
         fixture = UtilityFixture(utility, name="foo")
         self.useFixture(fixture)
         self.assertIs(utility, queryUtility(ITestUtility, name="foo"))
+
+
+class TestAdapterFixture(testtools.TestCase):
+
+    def test_provide_adapter(self):
+        fixture = AdapterFixture(TestAdapter)
+        self.useFixture(fixture)
+        context = TestContext()
+        adapter = queryAdapter(context)
+        self.assertIs(context, adapter.context)
+
+    def test_unregister_adapter(self):
+        context = TestContext()
+        with AdapterFixture(TestAdapter):
+            adapter = queryAdapter(context)
+            self.assertIs(context, adapter.context)
+        self.assertIs(None, queryAdapter(context))
+
+    def test_restore_original_adapter(self):
+
+        class TestAdapter2(TestAdapter):
+            pass
+
+        sm = getSiteManager()
+        sm.registerAdapter(TestAdapter2)
+        self.addCleanup(sm.unregisterAdapter, TestAdapter2)
+        context = TestContext()
+        with AdapterFixture(TestAdapter):
+            self.assertIsInstance(queryAdapter(context), TestAdapter)
+        self.assertIsInstance(queryAdapter(context), TestAdapter2)
+
+    def test_arguments_passthrough(self):
+        fixture = AdapterFixture(TestAdapter, name="foo")
+        self.useFixture(fixture)
+        context = TestContext()
+        self.assertIsInstance(queryAdapter(context, name="foo"), TestAdapter)