[Zope-Checkins] SVN: Zope/branches/2.9/lib/python/Shared/DC/ZRDB/ -
replace nasty and ineffective hack for dynamic connections
ids in ZSQL Method cache which is tested and works
Chris Withers
chris at simplistix.co.uk
Fri Nov 17 08:22:41 EST 2006
Log message for revision 71151:
- replace nasty and ineffective hack for dynamic connections ids in ZSQL Method cache which is tested and works
- simplify tests as a result
- make sure full chain of DA.__call__ and DA._cached_result is execercised
Changed:
U Zope/branches/2.9/lib/python/Shared/DC/ZRDB/DA.py
U Zope/branches/2.9/lib/python/Shared/DC/ZRDB/tests/test_caching.py
-=-
Modified: Zope/branches/2.9/lib/python/Shared/DC/ZRDB/DA.py
===================================================================
--- Zope/branches/2.9/lib/python/Shared/DC/ZRDB/DA.py 2006-11-17 12:30:35 UTC (rev 71150)
+++ Zope/branches/2.9/lib/python/Shared/DC/ZRDB/DA.py 2006-11-17 13:22:41 UTC (rev 71151)
@@ -352,11 +352,8 @@
def _searchable_result_columns(self): return self._col
- def _cached_result(self, DB__, query):
- pure_query = query
- # we need to munge the incoming query key in the cache
- # so that the same request to a different db is returned
- query = query + ('\nDBConnId: %s' % self.connection_hook, )
+ def _cached_result(self, DB__, query, max_rows, conn_id):
+ cache_key = query,max_rows,conn_id
# Try to fetch from cache
if hasattr(self,'_v_cache'): cache=self._v_cache
@@ -376,15 +373,15 @@
del cache[q]
del keys[-1]
- if cache.has_key(query):
- k, r = cache[query]
+ if cache.has_key(cache_key):
+ k, r = cache[cache_key]
if k > t: return r
# call the pure query
- result=apply(DB__.query, pure_query)
+ result=DB__.query(query,max_rows)
if self.cache_time_ > 0:
- tcache[int(now)]=query
- cache[query]= now, result
+ tcache[int(now)]=cache_key
+ cache[cache_key]= now, result
return result
@@ -450,8 +447,9 @@
if src__: return query
if self.cache_time_ > 0 and self.max_cache_ > 0:
- result=self._cached_result(DB__, (query, self.max_rows_))
- else: result=DB__.query(query, self.max_rows_)
+ result=self._cached_result(DB__, query, self.max_rows_, c)
+ else:
+ result=DB__.query(query, self.max_rows_)
if hasattr(self, '_v_brain'): brain=self._v_brain
else:
Modified: Zope/branches/2.9/lib/python/Shared/DC/ZRDB/tests/test_caching.py
===================================================================
--- Zope/branches/2.9/lib/python/Shared/DC/ZRDB/tests/test_caching.py 2006-11-17 12:30:35 UTC (rev 71150)
+++ Zope/branches/2.9/lib/python/Shared/DC/ZRDB/tests/test_caching.py 2006-11-17 13:22:41 UTC (rev 71151)
@@ -5,9 +5,13 @@
class DummyDB:
conn_num = 1
+
+ result = None
- def query(self,*args):
- return ('result for:',)+args
+ def query(self,query,max_rows):
+ if self.result:
+ return self.result
+ return 'result for ' + query
def hook_method(self):
conn_to_use = 'conn'+str(self.conn_num)
@@ -34,13 +38,13 @@
self.da.cache_time_ = 10
self.da.max_cache_ = 2
- def _do_query(self,query,time):
+ def _do_query(self,query,t):
try:
- self.DA.time = DummyTime(time)
- result = self.da._cached_result(DummyDB(),query)
+ self.DA.time = DummyTime(t)
+ result = self.da._cached_result(DummyDB(),query,1,'conn_id')
finally:
self.DA.time = time
- self.assertEqual(result,('result for:',)+query)
+ self.assertEqual(result,'result for '+query)
def _check_mapping(self,expected,actual):
missing = []
@@ -101,10 +105,10 @@
# query, but where the item returned is always in the cache
self._check_cache({},{})
for t in range(1,6):
- self._do_query(('query',),t)
+ self._do_query('query',t)
self._check_cache(
- {('query', '\nDBConnId: None'): (1,('result for:', 'query'))},
- {1: ('query', '\nDBConnId: None')}
+ {('query',1,'conn_id'): (1,'result for query')},
+ {1: ('query',1,'conn_id')}
)
def test_same_query_same_second(self):
@@ -115,10 +119,10 @@
self._check_cache({},{})
for t in range(11,16,1):
t = float(t)/10
- self._do_query(('query',),t)
+ self._do_query('query',t)
self._check_cache(
- {('query', '\nDBConnId: None'): (1.1,('result for:', 'query'))},
- {1: ('query', '\nDBConnId: None')}
+ {('query',1,'conn_id'): (1.1,'result for query')},
+ {1: ('query',1,'conn_id')}
)
def test_different_queries_different_second(self):
@@ -128,49 +132,49 @@
# dumped due to the replacement of Bucket with dict
self._check_cache({},{})
# one
- self._do_query(('query1',),1.1)
+ self._do_query('query1',1.1)
self._check_cache(
- {('query1', '\nDBConnId: None'): (1.1,('result for:', 'query1'))},
- {1: ('query1', '\nDBConnId: None')}
+ {('query1',1,'conn_id'): (1.1,'result for query1')},
+ {1: ('query1',1,'conn_id')}
)
# two
- self._do_query( ('query2',),3.2)
+ self._do_query( 'query2',3.2)
self._check_cache(
- {('query1', '\nDBConnId: None'): (1.1,('result for:', 'query1')),
- ('query2', '\nDBConnId: None'): (3.2,('result for:', 'query2')),},
- {1: ('query1', '\nDBConnId: None'),
- 3: ('query2', '\nDBConnId: None'),}
+ {('query1',1,'conn_id'): (1.1,'result for query1'),
+ ('query2',1,'conn_id'): (3.2,'result for query2'),},
+ {1: ('query1',1,'conn_id'),
+ 3: ('query2',1,'conn_id'),}
)
# three
- self._do_query(('query3',),4.3)
+ self._do_query('query3',4.3)
self._check_cache(
- {('query1', '\nDBConnId: None'): (1.1,('result for:', 'query1')),
- ('query2', '\nDBConnId: None'): (3.2,('result for:', 'query2')),
- ('query3', '\nDBConnId: None'): (4.3,('result for:', 'query3')),},
- {1: ('query1', '\nDBConnId: None'),
- 3: ('query2', '\nDBConnId: None'),
- 4: ('query3', '\nDBConnId: None'),}
+ {('query1',1,'conn_id'): (1.1,'result for query1'),
+ ('query2',1,'conn_id'): (3.2,'result for query2'),
+ ('query3',1,'conn_id'): (4.3,'result for query3'),},
+ {1: ('query1',1,'conn_id'),
+ 3: ('query2',1,'conn_id'),
+ 4: ('query3',1,'conn_id'),}
)
# four - now we drop our first cache entry, this is an off-by-one error
- self._do_query(('query4',),8.4)
+ self._do_query('query4',8.4)
self._check_cache(
- {('query2', '\nDBConnId: None'): (3.2,('result for:', 'query2')),
- ('query3', '\nDBConnId: None'): (4.3,('result for:', 'query3')),
- ('query4', '\nDBConnId: None'): (8.4,('result for:', 'query4')),},
- {3: ('query2', '\nDBConnId: None'),
- 4: ('query3', '\nDBConnId: None'),
- 8: ('query4', '\nDBConnId: None'),}
+ {('query2',1,'conn_id'): (3.2,'result for query2'),
+ ('query3',1,'conn_id'): (4.3,'result for query3'),
+ ('query4',1,'conn_id'): (8.4,'result for query4'),},
+ {3: ('query2',1,'conn_id'),
+ 4: ('query3',1,'conn_id'),
+ 8: ('query4',1,'conn_id'),}
)
# five - now we drop another cache entry
- self._do_query(('query5',),9.5)
+ self._do_query('query5',9.5)
# XXX oops - because dicts have an arbitary ordering, we dumped the wrong key!
self._check_cache(
- {('query3', '\nDBConnId: None'): (4.3,('result for:', 'query3')),
- ('query2', '\nDBConnId: None'): (3.2,('result for:', 'query2')),
- ('query5', '\nDBConnId: None'): (9.5,('result for:', 'query5')),},
- {4: ('query3', '\nDBConnId: None'),
- 3: ('query2', '\nDBConnId: None'),
- 9: ('query5', '\nDBConnId: None'),}
+ {('query3',1,'conn_id'): (4.3,'result for query3'),
+ ('query2',1,'conn_id'): (3.2,'result for query2'),
+ ('query5',1,'conn_id'): (9.5,'result for query5'),},
+ {4: ('query3',1,'conn_id'),
+ 3: ('query2',1,'conn_id'),
+ 9: ('query5',1,'conn_id'),}
)
def test_different_queries_same_second(self):
@@ -179,77 +183,54 @@
# XXX The demonstrates a memory leak in the cache code
self._check_cache({},{})
# one
- self._do_query(('query1',),1.0)
+ self._do_query('query1',1.0)
self._check_cache(
- {('query1', '\nDBConnId: None'): (1.0,('result for:', 'query1'))},
- {1: ('query1', '\nDBConnId: None')}
+ {('query1',1,'conn_id'): (1.0,'result for query1')},
+ {1: ('query1',1,'conn_id')}
)
# two
- self._do_query( ('query2',),1.1)
+ self._do_query( 'query2',1.1)
self._check_cache(
- {('query1', '\nDBConnId: None'): (1.0,('result for:', 'query1')),
- ('query2', '\nDBConnId: None'): (1.1,('result for:', 'query2')),},
- {1.0: ('query2', '\nDBConnId: None'),}
+ {('query1',1,'conn_id'): (1.0,'result for query1'),
+ ('query2',1,'conn_id'): (1.1,'result for query2'),},
+ {1.0: ('query2',1,'conn_id'),}
)
# three
- self._do_query(('query3',),1.2)
+ self._do_query('query3',1.2)
self._check_cache(
- {('query1', '\nDBConnId: None'): (1,('result for:', 'query1')),
- ('query2', '\nDBConnId: None'): (1.1,('result for:', 'query2')),
- ('query3', '\nDBConnId: None'): (1.2,('result for:', 'query3')),},
- {1: ('query3', '\nDBConnId: None'),}
+ {('query1',1,'conn_id'): (1,'result for query1'),
+ ('query2',1,'conn_id'): (1.1,'result for query2'),
+ ('query3',1,'conn_id'): (1.2,'result for query3'),},
+ {1: ('query3',1,'conn_id'),}
)
# four - now we drop our first cache entry, this is an off-by-one error
- self._do_query(('query4',),1.3)
+ self._do_query('query4',1.3)
self._check_cache(
# XXX - oops, why is query1 here still?
- {('query1', '\nDBConnId: None'): (1,('result for:', 'query1')),
- ('query2', '\nDBConnId: None'): (1.1,('result for:', 'query2')),
- ('query3', '\nDBConnId: None'): (1.2,('result for:', 'query3')),
- ('query4', '\nDBConnId: None'): (1.3,('result for:', 'query4')),},
- {1: ('query4', '\nDBConnId: None'),}
+ {('query1',1,'conn_id'): (1,'result for query1'),
+ ('query2',1,'conn_id'): (1.1,'result for query2'),
+ ('query3',1,'conn_id'): (1.2,'result for query3'),
+ ('query4',1,'conn_id'): (1.3,'result for query4'),},
+ {1: ('query4',1,'conn_id'),}
)
# five - now we drop another cache entry
- self._do_query(('query5',),1.4)
+ self._do_query('query5',1.4)
self._check_cache(
# XXX - oops, why are query1 and query2 here still?
- {('query1', '\nDBConnId: None'): (1,('result for:', 'query1')),
- ('query2', '\nDBConnId: None'): (1.1,('result for:', 'query2')),
- ('query3', '\nDBConnId: None'): (1.2,('result for:', 'query3')),
- ('query4', '\nDBConnId: None'): (1.3,('result for:', 'query4')),
- ('query5', '\nDBConnId: None'): (1.4,('result for:', 'query5')),},
- {1: ('query5', '\nDBConnId: None'),}
+ {('query1',1,'conn_id'): (1,'result for query1'),
+ ('query2',1,'conn_id'): (1.1,'result for query2'),
+ ('query3',1,'conn_id'): (1.2,'result for query3'),
+ ('query4',1,'conn_id'): (1.3,'result for query4'),
+ ('query5',1,'conn_id'): (1.4,'result for query5'),},
+ {1: ('query5',1,'conn_id'),}
)
- def test_connection_hook(self):
- # XXX excercises the nonsense of the connection id cache descriminator
- self._do_query(('query1',),1.1)
- # XXX this should be '\nDBConnId: conn_id'
- self._check_cache(
- {('query1', '\nDBConnId: None'): (1.1,('result for:', 'query1'))},
- {1: ('query1', '\nDBConnId: None')}
- )
- del self.da._v_cache
- self.da.connection_hook='hook_method'
- # XXX this should be '\nDBConnId: conn1'
- self._do_query(('query1',),1.1)
- self._check_cache(
- {('query1', '\nDBConnId: hook_method'): (1.1,('result for:', 'query1'))},
- {1: ('query1', '\nDBConnId: hook_method')}
- )
- del self.da._v_cache
- # XXX this should be '\nDBConnId: conn2'
- self._do_query(('query1',),1.1)
- self._check_cache(
- {('query1', '\nDBConnId: hook_method'): (1.1,('result for:', 'query1'))},
- {1: ('query1', '\nDBConnId: hook_method')}
- )
-
class DummyDA:
def __call__(self):
- # we return None here, because this should never actually be called
- return None
+ conn = DummyDB()
+ conn.result = ((),())
+ return conn
sql_quote__ = "I don't know what this is."
@@ -264,8 +245,8 @@
class TestCacheKeys(TestCase):
- def _cached_result(self,DB__,query):
- self.cache_key = query
+ def _cached_result(self,DB__,query,row_count,conn_id):
+ self.cache_key = query,row_count,conn_id
# we return something that can be safely turned into an empty Result
return ((),())
@@ -280,27 +261,40 @@
def test_default(self):
self.da()
- self.assertEqual(self.cache_key,('some sql',1000))
+ self.assertEqual(self.cache_key,('some sql',1000,'conn_id'))
def test_different_max_rows(self):
self.da.max_rows_ = 123
self.da()
- self.assertEqual(self.cache_key,('some sql',123))
+ self.assertEqual(self.cache_key,('some sql',123,'conn_id'))
def test_connection_hook(self):
self.da.connection_hook = 'hook_method'
self.da.hook_method = Hook()
self.da.conn1 = DummyDA()
self.da()
- # XXX the connection id should be added to the cache key here
- self.assertEqual(self.cache_key,('some sql',1000))
+ self.assertEqual(self.cache_key,('some sql',1000,'conn1'))
self.da.conn2 = DummyDA()
self.da()
- # XXX the connection id should be added to the cache key here
- self.assertEqual(self.cache_key,('some sql',1000))
+ self.assertEqual(self.cache_key,('some sql',1000,'conn2'))
+class TestFullChain(TestCase):
+
+ def test_full_chain(self):
+ from Shared.DC.ZRDB.DA import DA
+ self.da = DA('da','title','conn_id','arg1 arg2','some sql')
+ self.da.conn_id = DummyDA()
+ # These need to be set so DA.__call__ tries for a cached result
+ self.da.cache_time_ = 1
+ self.da.max_cache_ = 1
+ # run the method, exercising both DA.__call__ and DA._cached_result
+ # currently all this checks is that DA._cached_result's call signature
+ # matches that expected by DA.__call__
+ self.da()
+
def test_suite():
suite = TestSuite()
suite.addTest(makeSuite(TestCaching))
suite.addTest(makeSuite(TestCacheKeys))
+ suite.addTest(makeSuite(TestFullChain))
return suite
More information about the Zope-Checkins
mailing list