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

Proposed by Free Ekanayaka
Status: Needs review
Proposed branch: lp:~free.ekanayaka/storm/zstorm-access
Merge into: lp:storm
Diff against target: 216 lines (+94/-9)
4 files modified
storm/zope/testing.py (+16/-7)
storm/zope/zstorm.py (+18/-0)
tests/zope/testing.py (+11/-1)
tests/zope/zstorm.py (+49/-1)
To merge this branch: bzr merge lp:~free.ekanayaka/storm/zstorm-access
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Needs Information
Storm Developers Pending
Review via email: mp+81556@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

What's the reasoning for these features?

[2]

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

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

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
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

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

Add ZStorm.of and ZStorm.settings

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
20#20#
21import transaction21import transaction
22from testresources import TestResourceManager22from testresources import TestResourceManager
23from zope.component import provideUtility, getUtility23try:
24 from zope.component import provideUtility, getUtility
25except ImportError:
26 # zope.compnent is optional, see ZStormResourceManager.auto_provide_utility
27 pass
2428
25from storm.zope.zstorm import ZStorm29from storm.zope.zstorm import ZStorm
26from storm.zope.interfaces import IZStorm30from storm.zope.interfaces import IZStorm
@@ -40,11 +44,14 @@
40 - 'schema-uri', optionally an alternate URI to use for applying the44 - 'schema-uri', optionally an alternate URI to use for applying the
41 schema, if not given it defaults to 'uri'.45 schema, if not given it defaults to 'uri'.
4246
43 @ivar force_delete: If C{True} for running L{Schema.delete} on a L{Store}47 @ivar force_delete: If C{True} force running L{Schema.delete} on a L{Store}
44 even if no commit was performed by the test. Useful when running a test48 even if no commit was performed by the test. Useful when running a test
45 in a subprocess that might commit behind our back.49 in a subprocess that might commit behind our back.
50 @ivar auto_provide_utility: If C{True} (the default), register the zstorm
51 resource in the global ZCA registry.
46 """52 """
47 force_delete = False53 force_delete = False
54 auto_provide_utility = True
4855
49 def __init__(self, databases):56 def __init__(self, databases):
50 super(ZStormResourceManager, self).__init__()57 super(ZStormResourceManager, self).__init__()
@@ -88,14 +95,16 @@
88 # with an empty db95 # with an empty db
89 schema.delete(schema_store)96 schema.delete(schema_store)
9097
91 provideUtility(zstorm)98 if self.auto_provide_utility:
99 provideUtility(zstorm)
92 self._zstorm = zstorm100 self._zstorm = zstorm
93 self._schema_zstorm = schema_zstorm101 self._schema_zstorm = schema_zstorm
94102
95 elif getUtility(IZStorm) is not self._zstorm:103 elif self.auto_provide_utility:
96 # This probably means that the test code has overwritten our104 if getUtility(IZStorm) is not self._zstorm:
97 # utility, let's re-register it.105 # This probably means that the test code has overwritten our
98 provideUtility(self._zstorm)106 # utility, let's re-register it.
107 provideUtility(self._zstorm)
99108
100 return self._zstorm109 return self._zstorm
101110
102111
=== 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 @@
56 from storm.zope.interfaces import IZStorm56 from storm.zope.interfaces import IZStorm
5757
58 store = getUtility(IZStorm).get('main')58 store = getUtility(IZStorm).get('main')
59
60 @ivar settings: A C{dict} that user code can use to store configuration
61 values or resources that Storm objects can access by getting the
62 L{ZStorm} instance they belong to with L{ZStorm.of}.
59 """63 """
6064
61 implements(IZStorm)65 implements(IZStorm)
@@ -68,6 +72,7 @@
68 self._local = threading.local()72 self._local = threading.local()
69 self._default_databases = {}73 self._default_databases = {}
70 self._default_uris = {}74 self._default_uris = {}
75 self.settings = {}
7176
72 def _reset(self):77 def _reset(self):
73 for name, store in list(self.iterstores()):78 for name, store in list(self.iterstores()):
@@ -135,6 +140,7 @@
135 store._event.hook(140 store._event.hook(
136 "register-transaction", register_store_with_transaction,141 "register-transaction", register_store_with_transaction,
137 weakref.ref(self))142 weakref.ref(self))
143 store._zstorm = self
138144
139 self._stores[id(store)] = store145 self._stores[id(store)] = store
140 if name is not None:146 if name is not None:
@@ -181,6 +187,7 @@
181 store._event.unhook(187 store._event.unhook(
182 "register-transaction", register_store_with_transaction,188 "register-transaction", register_store_with_transaction,
183 weakref.ref(self))189 weakref.ref(self))
190 store._zstorm = None
184191
185 def iterstores(self):192 def iterstores(self):
186 """Iterate C{name, store} 2-tuples."""193 """Iterate C{name, store} 2-tuples."""
@@ -202,6 +209,17 @@
202 """209 """
203 return self._default_uris.copy()210 return self._default_uris.copy()
204211
212 @staticmethod
213 def of(context):
214 """Get the L{ZStorm} the given context is associated with.
215
216 If the given context is not associated with any L{ZStorm}, return None.
217 """
218 store = context
219 if not isinstance(store, Store):
220 store = Store.of(context)
221 return getattr(store, "_zstorm", None)
222
205223
206def register_store_with_transaction(store, zstorm_ref):224def register_store_with_transaction(store, zstorm_ref):
207 zstorm = zstorm_ref()225 zstorm = zstorm_ref()
208226
=== 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 @@
28from storm.exceptions import IntegrityError28from storm.exceptions import IntegrityError
2929
30if has_transaction and has_zope_component and has_testresources:30if has_transaction and has_zope_component and has_testresources:
31 from zope.component import provideUtility, getUtility31 from zope.component import provideUtility, getUtility, queryUtility
32 from storm.zope.zstorm import ZStorm32 from storm.zope.zstorm import ZStorm
33 from storm.zope.interfaces import IZStorm33 from storm.zope.interfaces import IZStorm
34 from storm.zope.schema import ZSchema34 from storm.zope.schema import ZSchema
@@ -66,6 +66,7 @@
66 self.store = Store(create_database(uri))66 self.store = Store(create_database(uri))
6767
68 def tearDown(self):68 def tearDown(self):
69 provideUtility(None, IZStorm)
69 del sys.modules["patch_package"]70 del sys.modules["patch_package"]
70 sys.path.remove(self._package_dir)71 sys.path.remove(self._package_dir)
71 if "patch_1" in sys.modules:72 if "patch_1" in sys.modules:
@@ -117,6 +118,15 @@
117 self.resource.make([])118 self.resource.make([])
118 self.assertIs(zstorm, getUtility(IZStorm))119 self.assertIs(zstorm, getUtility(IZStorm))
119120
121 def test_make_no_izstorm(self):
122 """
123 L{ZStormResourceManager.make} registers its own ZStorm again if a test
124 has registered a new ZStorm utility overwriting the resource one.
125 """
126 self.resource.auto_provide_utility = False
127 self.resource.make([])
128 self.assertIs(None, queryUtility(IZStorm))
129
120 def test_clean_flush(self):130 def test_clean_flush(self):
121 """131 """
122 L{ZStormResourceManager.clean} tries to flush the stores to make sure132 L{ZStormResourceManager.clean} tries to flush the stores to make sure
123133
=== 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 @@
34 from zope.component import provideUtility, getUtility34 from zope.component import provideUtility, getUtility
3535
36from storm.exceptions import OperationalError36from storm.exceptions import OperationalError
37from storm.locals import Store37from storm.locals import Store, Int
3838
3939
40class ZStormTest(TestHelper):40class ZStormTest(TestHelper):
@@ -130,6 +130,54 @@
130 store = self.zstorm.create("name", "sqlite:")130 store = self.zstorm.create("name", "sqlite:")
131 self.assertEquals(self.zstorm.get_name(store), "name")131 self.assertEquals(self.zstorm.get_name(store), "name")
132132
133 def test_of(self):
134 """
135 L{ZStorm.of} returns the L{ZStorm} instance a L{Store} is linked to.
136 """
137 store = self.zstorm.create("name", "sqlite:")
138 self.assertIs(self.zstorm, ZStorm.of(store))
139
140 def test_of_with_no_zstorm_associated(self):
141 """
142 L{ZStorm.of} returns L{None} of the L{Store} is not linked to
143 any L{ZStorm} instance.
144 """
145 store = self.zstorm.create("name", "sqlite:")
146 self.zstorm.remove(store)
147 self.assertIs(None, ZStorm.of(store))
148
149 def test_of_with_object(self):
150 """
151 L{ZStorm.of} works also when passed a Storm object.
152 """
153 store = self.zstorm.create("name", "sqlite:")
154 store.execute("CREATE TABLE test (id INTEGER)")
155
156 class Test(object):
157 __storm_table__ = "test"
158 id = Int(primary=True)
159
160 test = Test()
161 store.add(test)
162 self.assertIs(self.zstorm, ZStorm.of(test))
163
164 def test_of_with_object_and_settings(self):
165 """
166 L{ZStorm.settings} can be used to conveniently expose application
167 settings to Storm objects
168 """
169 self.zstorm.settings["foo"] = "bar"
170 store = self.zstorm.create("name", "sqlite:")
171 store.execute("CREATE TABLE test (id INTEGER)")
172
173 class Test(object):
174 __storm_table__ = "test"
175 id = Int(primary=True)
176
177 test = Test()
178 store.add(test)
179 self.assertEqual("bar", ZStorm.of(test).settings["foo"])
180
133 def test_get_name_with_removed_store(self):181 def test_get_name_with_removed_store(self):
134 store = self.zstorm.create("name", "sqlite:")182 store = self.zstorm.create("name", "sqlite:")
135 self.assertEquals(self.zstorm.get_name(store), "name")183 self.assertEquals(self.zstorm.get_name(store), "name")

Subscribers

People subscribed via source and target branches

to status/vote changes: