[z3-checkins] r22132 - in z3/sqlos/branch/jinty-connection/src/sqlos: . ftests testing tests

jinty at codespeak.net jinty at codespeak.net
Sat Jan 14 07:52:06 CET 2006


Author: jinty
Date: Sat Jan 14 07:52:00 2006
New Revision: 22132

Modified:
   z3/sqlos/branch/jinty-connection/src/sqlos/_transaction.py
   z3/sqlos/branch/jinty-connection/src/sqlos/ftests/test_transaction.py
   z3/sqlos/branch/jinty-connection/src/sqlos/testing/testdb.py
   z3/sqlos/branch/jinty-connection/src/sqlos/tests/test_transaction.py
Log:
Clean up the transaction manager so that it doesn't expire changed objects
anymore. Then fix the resuting test breakage.

A caveat of this work is that you should never, never keep a reference to a
SQLOS object over a transaction. Just don't do it.


Modified: z3/sqlos/branch/jinty-connection/src/sqlos/_transaction.py
==============================================================================
--- z3/sqlos/branch/jinty-connection/src/sqlos/_transaction.py	(original)
+++ z3/sqlos/branch/jinty-connection/src/sqlos/_transaction.py	Sat Jan 14 07:52:00 2006
@@ -78,27 +78,6 @@
     if not obj.sqlmeta._obsolete:
         obj.sync()
 
-def expireSQLObject(obj):
-    """Expire the SQLObject.
-
-    Try to make sure none of the data makes it to the next transaction.
-
-    This function should not cause SQL to be sent as it is not defined whether
-    the SQL connection will commit before or after this is executed.
-    """
-
-    # Expire object values. The transaction has either been aborted or
-    # committed.
-    obj.expire()
-
-    # Calling sync() would commit the changes because expire()
-    # doesn't clear _SO_createValues.
-    # XXX this is a workaround for a bug fixed in SQLObject 0.7.1 it's
-    # kept around for compatibility until we decide not to support 0.7
-    # anymore
-    unproxied = removeSecurityProxy(obj)
-    unproxied._SO_createValues = {}
-
 
 class SQLObjectTransactionManager:
     """
@@ -136,7 +115,6 @@
     Monkeypatch some methods to ease testing:
 
         >>> synced = []
-        >>> expired = []
 
         >>> oldsync = SamplePerson.sync
         >>> def sync(self):
@@ -145,13 +123,6 @@
         ...     oldsync(self)
         >>> SamplePerson.sync = sync
 
-        >>> oldexpire = SamplePerson.expire
-        >>> def expire(self):
-        ...     expired.append(self.username)
-        ...     expired.sort()
-        ...     oldexpire(self)
-        >>> SamplePerson.expire = expire
-
     Now, we check the initial DataManager state:
 
         >>> person.dirty
@@ -178,16 +149,11 @@
 
         >>> synced
         []
-        >>> expired
-        []
         >>> get().commit()
         >>> synced
         ['jinty', 'sidnei']
-        >>> expired
-        ['jinty', 'sidnei']
 
         >>> synced[:] = []
-        >>> expired[:] = []
 
     Check the state:
 
@@ -208,18 +174,13 @@
 
         >>> synced
         []
-        >>> expired
-        []
         >>> get().abort()
         >>> synced
         []
-        >>> expired
-        ['sidnei']
 
     And then cleanup the monkeypatched method:
 
         >>> SamplePerson.sync = oldsync
-        >>> SamplePerson.expire = oldexpire
 
     And finally call tearDown and cleanup:
 
@@ -236,25 +197,12 @@
     def prepare(self, txn):
         return True
 
-    def _expireObjects(self):
-        for obj in self._objects:
-            expireSQLObject(obj)
-        self._objects.clear()
-
     def abort(self, txn):
-        self._expireObjects()
+        self._objects.clear()
         self._joined_txn = False
 
     def commit(self, txn):
-        # Excerpt from a mail by Stuart Bishop:
-        # >>|  * Why do we expire objects in a successful transaction? I can't think
-        # >>|    of a reason why we need to.
-        #
-        # You need to expire objects after a successful transaction so you can see
-        # database changes made by other processes or threads. This is particularly
-        # important for Zope where you have a number of handler threads and possibly
-        # multiple application servers in a ZEO environment.
-        self._expireObjects()
+        self._objects.clear()
         self._joined_txn = False
 
     def register(self, obj):
@@ -267,9 +215,6 @@
             txn = get()
             txn.addBeforeCommitHook(beforeCommitHook, (obj, ))
             if not self._joined_txn:
-                # register the Zope DA connection as well, becuse when we prepare,
-                # and cause DB activity, it might try to register as well. which
-                # fails. not very nice at all.
                 txn.join(self)
                 self._joined_txn = True
 

Modified: z3/sqlos/branch/jinty-connection/src/sqlos/ftests/test_transaction.py
==============================================================================
--- z3/sqlos/branch/jinty-connection/src/sqlos/ftests/test_transaction.py	(original)
+++ z3/sqlos/branch/jinty-connection/src/sqlos/ftests/test_transaction.py	Sat Jan 14 07:52:00 2006
@@ -26,17 +26,19 @@
     def setUp(self):
         super(TestTransaction, self).setUp()
         SamplePerson.createTable(ifNotExists=True)
-        self.person = SamplePerson(fullname='Sidnei da Silva',
-                                   username='sidnei',
-                                   password='test')
+        person = SamplePerson(fullname='Sidnei da Silva',
+                              username='sidnei',
+                              password='test')
+        self.personid = person.id
         # Commit what we have done
         get().commit()
+        begin()
 
     def supportTransactions(self):
         return SamplePerson._connection.supportTransactions
 
     def testChange(self):
-        person = self.person
+        person = SamplePerson.get(self.personid)
         self.assertEqual(person.fullname, 'Sidnei da Silva')
         self.assertEqual(person.username, 'sidnei')
         self.assertEqual(person.password, 'test')
@@ -51,7 +53,7 @@
         self.assertEqual(person.password, 'pass')
 
     def testCommit(self):
-        person = self.person
+        person = SamplePerson.get(self.personid)
         self.assertEqual(person.fullname, 'Sidnei da Silva')
         self.assertEqual(person.username, 'sidnei')
         self.assertEqual(person.password, 'test')
@@ -59,13 +61,14 @@
         person.username = 'dreamcatcher'
         person.password = 'pass'
         get().commit()
-        person.syncUpdate() # Let's see what is in the database
+        begin()
+        person = SamplePerson.get(self.personid)
         self.assertEqual(person.fullname, 'Sidnei Silva')
         self.assertEqual(person.username, 'dreamcatcher')
         self.assertEqual(person.password, 'pass')
 
     def testAbort(self):
-        person = self.person
+        person = SamplePerson.get(self.personid)
         self.assertEqual(person.fullname, 'Sidnei da Silva')
         self.assertEqual(person.username, 'sidnei')
         self.assertEqual(person.password, 'test')
@@ -74,7 +77,8 @@
         person.password = 'pass'
         person.sync() # Sunc to make sure that the DB is sent the statements
         get().abort()
-        person.syncUpdate() # Let's see what is in the database
+        begin()
+        person = SamplePerson.get(self.personid)
         if self.supportTransactions():
             self.assertEqual(person.fullname, 'Sidnei da Silva')
             self.assertEqual(person.username, 'sidnei')
@@ -98,15 +102,13 @@
             warnings.warn('Warning, not testing Cache Isolation')
             return # this test is NOT going to work against the default test
                    # database
-        person = self.person
-        personid = person.id
         # warm up the cache by getting an instance
-        person2 = SamplePerson.get(personid)
+        person2 = SamplePerson.get(self.personid)
         self.assertEqual(person2.fullname, 'Sidnei da Silva')
         self.assertEqual(person2.username, 'sidnei')
         self.assertEqual(person2.password, 'test')
         def changePerson():
-            person1 = SamplePerson.get(personid)
+            person1 = SamplePerson.get(self.personid)
             person1.fullname = 'Another Person'
             person1.username = 'notsidnei'
             person1.password = 'nottest'
@@ -126,10 +128,8 @@
             warnings.warn('Warning, not testing Cache Isolation')
             return # this test is NOT going to work against the default test
                    # database
-        person = self.person
-        personid = person.id
         # warm up the cache by getting an instance
-        person2 = SamplePerson.get(personid)
+        person2 = SamplePerson.get(self.personid)
         self.assertEqual(person2.fullname, 'Sidnei da Silva')
         self.assertEqual(person2.username, 'sidnei')
         self.assertEqual(person2.password, 'test')
@@ -137,7 +137,7 @@
         get().commit()
         # get change and commit the person in another thread
         def changePerson():
-            person1 = SamplePerson.get(personid)
+            person1 = SamplePerson.get(self.personid)
             person1.fullname = 'Another Person'
             person1.username = 'notsidnei'
             person1.password = 'nottest'
@@ -150,15 +150,18 @@
         get().commit()
         begin()
         # If we get a new person, we should see the changed person
-        person3 = SamplePerson.get(personid)
+        person3 = SamplePerson.get(self.personid)
         self.assertEqual(person3.fullname, 'Another Person')
         self.assertEqual(person3.username, 'notsidnei')
         self.assertEqual(person3.password, 'nottest')
 
     def tearDown(self):
-        self.person.destroySelf()
+        person = SamplePerson.get(self.personid)
+        person.destroySelf()
         super(TestTransaction, self).tearDown()
         SamplePerson.dropTable()
+        get().commit()
+        begin()
 
 def test_suite():
     suite = unittest.TestSuite()

Modified: z3/sqlos/branch/jinty-connection/src/sqlos/testing/testdb.py
==============================================================================
--- z3/sqlos/branch/jinty-connection/src/sqlos/testing/testdb.py	(original)
+++ z3/sqlos/branch/jinty-connection/src/sqlos/testing/testdb.py	Sat Jan 14 07:52:00 2006
@@ -68,7 +68,7 @@
 
     def getConnection(self):
         if self.conn is None:
-            self.conn = self.connectionAdapterFactory().transaction()
+            self.conn = self.connectionAdapterFactory()
         return self.conn
 
     def tearDown(self, dropTables=True):

Modified: z3/sqlos/branch/jinty-connection/src/sqlos/tests/test_transaction.py
==============================================================================
--- z3/sqlos/branch/jinty-connection/src/sqlos/tests/test_transaction.py	(original)
+++ z3/sqlos/branch/jinty-connection/src/sqlos/tests/test_transaction.py	Sat Jan 14 07:52:00 2006
@@ -2,7 +2,7 @@
 import doctest
 
 from zope.testing.doctestunit import DocTestSuite
-from transaction import get
+from transaction import get, begin
 
 def doctest_KeepValuesOverExpireSync():
     """Regression test for a SQLObject bug in expire.
@@ -25,14 +25,20 @@
 
         >>> person = SamplePerson(fullname='Andres', username='andres',
         ... password='789')
+        >>> personid = person.id
         >>> get().commit()
+        >>> txn = begin()
 
     And make sure the abort/commit cycle works:
 
+        >>> person = SamplePerson.get(personid)
         >>> person.username = "yourname"
         >>> get().abort()
+        >>> txn = begin()
+        >>> person = SamplePerson.get(personid)
         >>> person.fullname = "Andres Freund"
         >>> get().commit()
+        >>> txn = begin()
         >>> person.username
         'andres'
 


More information about the z3-checkins mailing list