SVN: ZODB/trunk/src/ Bugs fixed:

Jim Fulton <[email protected]>
Newsgroups gmane.comp.python.zope.zodb.cvs
Message-ID <20100909155002.93741941EF__6502.83734216845$1284047408$gmane$org@cvs.zope.org>
Log message for revision 116273:
  Bugs fixed:
    When objects were added in savepoints and either the savepoint was
    rolled back (https://bugs.launchpad.net/zodb/+bug/143560) or the
    transaction was aborted
    (https://mail.zope.org/pipermail/zodb-dev/2010-June/013488.html)
    The objects' _p_oid and _p_jar variables weren't cleared, leading to
    surprizing errors.
  

Changed:
  U   ZODB/trunk/src/CHANGES.txt
  U   ZODB/trunk/src/ZODB/Connection.py
  U   ZODB/trunk/src/ZODB/tests/testConnection.py

-=-
Modified: ZODB/trunk/src/CHANGES.txt
===================================================================
--- ZODB/trunk/src/CHANGES.txt	2010-09-09 15:10:57 UTC (rev 116272)
+++ ZODB/trunk/src/CHANGES.txt	2010-09-09 15:50:02 UTC (rev 116273)
@@ -2,6 +2,20 @@
  Change History
 ================
 
+3.10.0b7 (2010-09-??)
+=====================
+
+Bugs fixed
+----------
+
+- When objects were added in savepoints and either the savepoint was
+  rolled back (https://bugs.launchpad.net/zodb/+bug/143560) or the
+  transaction was aborted
+  (https://mail.zope.org/pipermail/zodb-dev/2010-June/013488.html)
+  The objects' _p_oid and _p_jar variables weren't cleared, leading to
+  surprizing errors.
+
+
 3.10.0b6 (2010-09-08)
 =====================
 

Modified: ZODB/trunk/src/ZODB/Connection.py
===================================================================
--- ZODB/trunk/src/ZODB/Connection.py	2010-09-09 15:10:57 UTC (rev 116272)
+++ ZODB/trunk/src/ZODB/Connection.py	2010-09-09 15:50:02 UTC (rev 116273)
@@ -442,6 +442,8 @@
             assert oid is not None
             if oid in self._added:
                 del self._added[oid]
+                if self._cache.get(oid) is not None:
+                    del self._cache[oid]
                 del obj._p_jar
                 del obj._p_oid
                 self._db.save_oid(oid)
@@ -742,7 +744,6 @@
         """Disown any objects newly saved in an uncommitted transaction."""
         if creating is None:
             creating = self._creating
-            self._creating = {}
 
         for oid in creating:
             self._db.save_oid(oid)
@@ -752,6 +753,8 @@
                 del o._p_jar
                 del o._p_oid
 
+        creating.clear()
+
     def tpc_vote(self, transaction):
         """Verify that a data manager can commit the transaction."""
         try:
@@ -1104,7 +1107,10 @@
         self._creating.clear()
         self._registered_objects = []
 
-        state = self._storage.position, self._storage.index.copy()
+        state = (self._storage.position,
+                 self._storage.index.copy(),
+                 self._storage.creating.copy(),
+                 )
         result = Savepoint(self, state)
         # While the interface doesn't guarantee this, savepoints are
         # sometimes used just to "break up" very long transactions, and as
@@ -1117,6 +1123,7 @@
         self._abort()
         self._registered_objects = []
         src = self._storage
+        self._invalidate_creating(src.creating)
         index = src.index
         src.reset(*state)
         self._cache.invalidate(index)
@@ -1163,6 +1170,7 @@
     def _abort_savepoint(self):
         """Discard all savepoint data."""
         src = self._savepoint_storage
+        self._invalidate_creating(src.creating)
         self._storage = self._normal_storage
         self._savepoint_storage = None
 
@@ -1182,8 +1190,11 @@
         # by another thread, so the risk of a reread is pretty low.
         # It's really not worth the effort to pursue this.
 
+        # Note that we do this *after* reseting the storage so that, if
+        # data are read, we read it from the reset storage!
+
         self._cache.invalidate(src.index)
-        self._invalidate_creating(src.creating)
+
         src.close()
 
     # Savepoint support
@@ -1311,7 +1322,7 @@
     def temporaryDirectory(self):
         return self._storage.temporaryDirectory()
 
-    def reset(self, position, index):
+    def reset(self, position, index, creating):
         self._file.truncate(position)
         self.position = position
         # Caution:  We're typically called as part of a savepoint rollback.
@@ -1324,6 +1335,7 @@
         # a copy of the index here.  An alternative would be to ensure that
         # all callers pass copies.  As is, our callers do not make copies.
         self.index = index.copy()
+        self.creating = creating
 
 class RootConvenience(object):
 

Modified: ZODB/trunk/src/ZODB/tests/testConnection.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testConnection.py	2010-09-09 15:10:57 UTC (rev 116272)
+++ ZODB/trunk/src/ZODB/tests/testConnection.py	2010-09-09 15:50:02 UTC (rev 116273)
@@ -759,12 +759,76 @@
 
     """
 
-# check interaction w savepoint
-# check call in read-only trans followed by write trans
+class C_invalidations_of_new_objects_work_after_savepoint(Persistent):
+    def __init__(self):
+        self.settings = 1
 
+    def _p_invalidate(self):
+        print 'INVALIDATE', self.settings
+        Persistent._p_invalidate(self)
+        print self.settings   # POSKeyError here
 
+def abort_of_savepoint_creating_new_objects_w_exotic_invalidate_doesnt_break():
+    r"""
+    Before, the following would fail with a POSKeyError, which was
+    somewhat surprizing, in a very edgy sort of way. :)
 
+    Really, when an object add is aborted, the object should be "removed" from
+    the db and its invalidatuon method shouldm't even be called:
 
+    >>> conn = ZODB.connection(None)
+    >>> conn.root.x = x = C_invalidations_of_new_objects_work_after_savepoint()
+    >>> _ = transaction.savepoint()
+    >>> x._p_oid
+    '\x00\x00\x00\x00\x00\x00\x00\x01'
+
+    >>> x._p_jar is conn
+    True
+
+    >>> transaction.abort()
+
+After the abort, the oid and jar are None:
+
+    >>> x._p_oid
+    >>> x._p_jar
+
+    """
+
+class Clp9460655(Persistent):
+    def __init__(self, word, id):
+        super(Clp9460655, self).__init__()
+	self.id = id
+        self._word = word
+
+def lp9460655():
+    r"""
+    >>> conn = ZODB.connection(None)
+    >>> root = conn.root()
+    >>> Word = Clp9460655
+
+    >>> from BTrees.OOBTree import OOBTree
+    >>> data = root['data'] = OOBTree()
+
+    >>> commonWords = []
+    >>> count = "0"
+    >>> for x in ('hello', 'world', 'how', 'are', 'you'):
+    ...         commonWords.append(Word(x, count))
+    ...         count = str(int(count) + 1)
+
+    >>> sv = transaction.savepoint()
+    >>> for word in commonWords:
+    ...         sv2 = transaction.savepoint()
+    ...         data[word.id] = word
+
+    >>> sv.rollback()
+    >>> print commonWords[1].id  # raises POSKeyError
+    1
+
+    """
+
+
+
+
 class _PlayPersistent(Persistent):
     def setValueWithSize(self, size=0): self.value = size*' '
     __init__ = setValueWithSize
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.