Merge lp:~free.ekanayaka/storm/zstorm-access into lp:storm

[Free Ekanayaka]

This branch adds a ZStorm.of static method similar to Store.of, that lets Storm object access the ZStorm they belong to without relying on global variables.

It also adds a ZStorm.settings instance variable that application code can use to expose configuration values and resources to Storm objects without using global variables.

[Gustavo Niemeyer (niemeyer)]

[1]

What's the reasoning for these features?

[2]

This is adding a reference cycle between every store and zstorm.

[Gustavo Niemeyer (niemeyer)]

Btw, on second thought, I have a bad feeling about this. This is attaching a blob of data onto ZStorm
without any apparent reason, and ZStorm.of seems to be a way to get to that blob of data if I understand
it correctly. It feels like a workaround to an issue of code organization.

review: Needs Information

[Free Ekanayaka (free.ekanayaka)]

Hi Gustavo, thanks for reviewing.

[1]

The use case is to be able to access the stores managed by a certain ZStorm instance from a Storm object. Let's say you have two stores, with a storm class Foo belonging to the first and a storm class Bar belonging to the second. Now you want to write a Foo.egg() method with some logic that accesses the second store to find some instances of the Bar class. Right now what one would probably do is to run a queryUtility(IZStorm) to get a global ZStorm instance and get the second store object from it.

The problem with this is that I don't see a way to have two ZStorm instances managing separate sets of stores for two entirely different models in the same process (because queryUtility returns you a global per-process object, unless you register your two ZStorm instances as named utilities, but then you introduce some coupling between the two models). Web frameworks like Pyramid let you run multiple applications in the same process, but if one uses ZStorm in the way described, it might be problematic, see also:

http://docs.pylonsproject.org/projects/pyramid/dev/narr/zca.html#using-the-zca-global-api-in-a-pyramid-application

Beside this, it can be useful for parallel testing.

The alternative I see is to pass around the particular ZStorm instance you want and add it as parameter to the Foo.egg() method, but that sounds more laborious than being able to access it via ZStorm.of().

As for the blob of data, maybe it's to open, what I'd use it for is to let storm objects access a non-global Zope registry to query for application components in a non-global way.

[2]

Sure, what problem does the reference cycle bring? If it has bad implications maybe there's a better way.

Unmerged revisions

420. By Free Ekanayaka on 2011-11-08

Add ZStorm.of and ZStorm.settings

=== modified file 'storm/zope/testing.py'
--- storm/zope/testing.py	2011-08-16 10:00:16 +0000
+++ storm/zope/testing.py	2011-11-08 11:11:03 +0000
@@ -20,7 +20,11 @@
 #
 import transaction
 from testresources import TestResourceManager
-from zope.component import provideUtility, getUtility
+try:
+    from zope.component import provideUtility, getUtility
+except ImportError:
+    # zope.compnent is optional, see ZStormResourceManager.auto_provide_utility
+    pass
 
 from storm.zope.zstorm import ZStorm
 from storm.zope.interfaces import IZStorm
@@ -40,11 +44,14 @@
         - 'schema-uri', optionally an alternate URI to use for applying the
           schema, if not given it defaults to 'uri'.
 
-    @ivar force_delete: If C{True} for running L{Schema.delete} on a L{Store}
+    @ivar force_delete: If C{True} force running L{Schema.delete} on a L{Store}
         even if no commit was performed by the test. Useful when running a test
         in a subprocess that might commit behind our back.
+    @ivar auto_provide_utility: If C{True} (the default), register the zstorm
+        resource in the global ZCA registry.
     """
     force_delete = False
+    auto_provide_utility = True
 
     def __init__(self, databases):
         super(ZStormResourceManager, self).__init__()
@@ -88,14 +95,16 @@
                 # with an empty db
                 schema.delete(schema_store)
 
-            provideUtility(zstorm)
+            if self.auto_provide_utility:
+                provideUtility(zstorm)
             self._zstorm = zstorm
             self._schema_zstorm = schema_zstorm
 
-        elif getUtility(IZStorm) is not self._zstorm:
-            # This probably means that the test code has overwritten our
-            # utility, let's re-register it.
-            provideUtility(self._zstorm)
+        elif self.auto_provide_utility:
+            if getUtility(IZStorm) is not self._zstorm:
+                # This probably means that the test code has overwritten our
+                # utility, let's re-register it.
+                provideUtility(self._zstorm)
 
         return self._zstorm
 

=== modified file 'storm/zope/zstorm.py'
--- storm/zope/zstorm.py	2010-02-09 12:53:53 +0000
+++ storm/zope/zstorm.py	2011-11-08 11:11:03 +0000
@@ -56,6 +56,10 @@
       from storm.zope.interfaces import IZStorm
 
       store = getUtility(IZStorm).get('main')
+
+    @ivar settings: A C{dict} that user code can use to store configuration
+        values or resources that Storm objects can access by getting the
+        L{ZStorm} instance they belong to with L{ZStorm.of}.
     """
 
     implements(IZStorm)
@@ -68,6 +72,7 @@
         self._local = threading.local()
         self._default_databases = {}
         self._default_uris = {}
+        self.settings = {}
 
     def _reset(self):
         for name, store in list(self.iterstores()):
@@ -135,6 +140,7 @@
         store._event.hook(
             "register-transaction", register_store_with_transaction,
             weakref.ref(self))
+        store._zstorm = self
 
         self._stores[id(store)] = store
         if name is not None:
@@ -181,6 +187,7 @@
         store._event.unhook(
             "register-transaction", register_store_with_transaction,
             weakref.ref(self))
+        store._zstorm = None
 
     def iterstores(self):
         """Iterate C{name, store} 2-tuples."""
@@ -202,6 +209,17 @@
         """
         return self._default_uris.copy()
 
+    @staticmethod
+    def of(context):
+        """Get the L{ZStorm} the given context is associated with.
+
+        If the given context is not associated with any L{ZStorm}, return None.
+        """
+        store = context
+        if not isinstance(store, Store):
+            store = Store.of(context)
+        return getattr(store, "_zstorm", None)
+
 
 def register_store_with_transaction(store, zstorm_ref):
     zstorm = zstorm_ref()

=== modified file 'tests/zope/testing.py'
--- tests/zope/testing.py	2011-09-13 10:43:40 +0000
+++ tests/zope/testing.py	2011-11-08 11:11:03 +0000
@@ -28,7 +28,7 @@
 from storm.exceptions import IntegrityError
 
 if has_transaction and has_zope_component and has_testresources:
-    from zope.component import provideUtility, getUtility
+    from zope.component import provideUtility, getUtility, queryUtility
     from storm.zope.zstorm import ZStorm
     from storm.zope.interfaces import IZStorm
     from storm.zope.schema import ZSchema
@@ -66,6 +66,7 @@
         self.store = Store(create_database(uri))
 
     def tearDown(self):
+        provideUtility(None, IZStorm)
         del sys.modules["patch_package"]
         sys.path.remove(self._package_dir)
         if "patch_1" in sys.modules:
@@ -117,6 +118,15 @@
         self.resource.make([])
         self.assertIs(zstorm, getUtility(IZStorm))
 
+    def test_make_no_izstorm(self):
+        """
+        L{ZStormResourceManager.make} registers its own ZStorm again if a test
+        has registered a new ZStorm utility overwriting the resource one.
+        """
+        self.resource.auto_provide_utility = False
+        self.resource.make([])
+        self.assertIs(None, queryUtility(IZStorm))
+
     def test_clean_flush(self):
         """
         L{ZStormResourceManager.clean} tries to flush the stores to make sure

=== modified file 'tests/zope/zstorm.py'
--- tests/zope/zstorm.py	2011-09-14 15:27:02 +0000
+++ tests/zope/zstorm.py	2011-11-08 11:11:03 +0000
@@ -34,7 +34,7 @@
     from zope.component import provideUtility, getUtility
 
 from storm.exceptions import OperationalError
-from storm.locals import Store
+from storm.locals import Store, Int
 
 
 class ZStormTest(TestHelper):
@@ -130,6 +130,54 @@
         store = self.zstorm.create("name", "sqlite:")
         self.assertEquals(self.zstorm.get_name(store), "name")
 
+    def test_of(self):
+        """
+        L{ZStorm.of} returns the L{ZStorm} instance a L{Store} is linked to.
+        """
+        store = self.zstorm.create("name", "sqlite:")
+        self.assertIs(self.zstorm, ZStorm.of(store))
+
+    def test_of_with_no_zstorm_associated(self):
+        """
+        L{ZStorm.of} returns L{None} of the L{Store} is not linked to
+        any L{ZStorm} instance.
+        """
+        store = self.zstorm.create("name", "sqlite:")
+        self.zstorm.remove(store)
+        self.assertIs(None, ZStorm.of(store))
+
+    def test_of_with_object(self):
+        """
+        L{ZStorm.of} works also when passed a Storm object.
+        """
+        store = self.zstorm.create("name", "sqlite:")
+        store.execute("CREATE TABLE test (id INTEGER)")
+
+        class Test(object):
+            __storm_table__ = "test"
+            id = Int(primary=True)
+
+        test = Test()
+        store.add(test)
+        self.assertIs(self.zstorm, ZStorm.of(test))
+
+    def test_of_with_object_and_settings(self):
+        """
+        L{ZStorm.settings} can be used to conveniently expose application
+        settings to Storm objects
+        """
+        self.zstorm.settings["foo"] = "bar"
+        store = self.zstorm.create("name", "sqlite:")
+        store.execute("CREATE TABLE test (id INTEGER)")
+
+        class Test(object):
+            __storm_table__ = "test"
+            id = Int(primary=True)
+
+        test = Test()
+        store.add(test)
+        self.assertEqual("bar", ZStorm.of(test).settings["foo"])
+
     def test_get_name_with_removed_store(self):
         store = self.zstorm.create("name", "sqlite:")
         self.assertEquals(self.zstorm.get_name(store), "name")