[Zodb-checkins] CVS: ZODB3/ZODB - Connection.py:1.114.2.6

[Gintautas Miliauskas]

Update of /cvs-repository/ZODB3/ZODB
In directory cvs.zope.org:/tmp/cvs-serv29045

Modified Files:
      Tag: zope3-zodb3-devel-branch
	Connection.py 
Log Message:
Fixed the implementation of Connection.add() to not lose objects that are
created during a __getstate__ and are also added to the Connection with
add(). (Objects that were created in __getstate__ but not added with add()
were just fine!)

Added tests for this.

SteveA + Gintas


=== ZODB3/ZODB/Connection.py 1.114.2.5 => 1.114.2.6 ===
--- ZODB3/ZODB/Connection.py:1.114.2.5	Sun Feb  1 13:38:38 2004
+++ ZODB3/ZODB/Connection.py	Tue Feb  3 14:14:41 2004
@@ -18,6 +18,7 @@
 import logging
 import sys
 import threading
+import itertools
 from time import time
 from utils import u64
 
@@ -61,6 +62,7 @@
     _opened = None
     _code_timestamp = 0
     _transaction = None
+    _added_during_commit = None
 
     def __init__(self, version='', cache_size=400,
                  cache_deactivate_after=60, mvcc=True):
@@ -172,6 +174,8 @@
             oid = obj._p_oid = self._storage.new_oid()
             obj._p_jar = self
             self._added[oid] = obj
+            if self._added_during_commit is not None:
+                self._added_during_commit.append(obj)
         elif obj._p_jar is not self:
             raise InvalidObjectReference(obj, obj._p_jar)
 
@@ -291,7 +295,8 @@
         #     the oid is appended to _creating.
         #     However, this ought to be unnecessary because the _p_serial
         #     of the object will be z64 or None, so it will be appended
-        #     to _creating about 30 lines down.
+        #     to _creating about 30 lines down. The removal from _added
+        #     ought likewise to be unnecessary.
         if oid is None or object._p_jar is not self:
             # new object
             oid = self.new_oid()
@@ -299,7 +304,8 @@
             object._p_oid = oid
             self._creating.append(oid) # maybe don't need this
         elif oid in self._added:
-            self._creating.append(oid) # maybe don't need this
+            # maybe don't need these
+            self._creating.append(oid)
             del self._added[oid]
         elif object._p_changed:
             if invalid(oid):
@@ -307,42 +313,48 @@
                 if resolve is None:
                     raise ConflictError(object=object)
             self._modified.append(oid)
-
         else:
             # Nothing to do
             return
 
         w = ObjectWriter(object)
-        for obj in w:
-            oid = obj._p_oid
-            serial = getattr(obj, '_p_serial', z64)
-
-            # XXX which one? z64 or None? Why do I have to check both?
-            if serial == z64 or serial is None:
-                # new object
-                self._creating.append(oid)
-            else:
-                #XXX We should never get here
-                if invalid(oid) and not hasattr(object, '_p_resolveConflict'):
-                    raise ConflictError(object=obj)
-                self._modified.append(oid)
-
-            p = w.serialize(obj)
-            s = self._storage.store(oid, serial, p, self._version, transaction)
-            self._store_count = self._store_count + 1
-            # Put the object in the cache before handling the
-            # response, just in case the response contains the
-            # serial number for a newly created object
-            try:
-                self._cache[oid] = obj
-            except:
-                # Dang, I bet its wrapped:
-                if hasattr(obj, 'aq_base'):
-                    self._cache[oid] = obj.aq_base
+        self._added_during_commit = []
+        try:
+            for obj in itertools.chain(w, self._added_during_commit):
+                oid = obj._p_oid
+                serial = getattr(obj, '_p_serial', z64)
+
+                # XXX which one? z64 or None? Why do I have to check both?
+                if serial == z64 or serial is None:
+                    # new object
+                    self._creating.append(oid)
+                    # If this object was added, it is now in _creating, so can
+                    # be removed from _added.
+                    self._added.pop(oid, None)
                 else:
-                    raise
+                    if invalid(oid) and not hasattr(object, '_p_resolveConflict'):
+                        raise ConflictError(object=obj)
+                    self._modified.append(oid)
+                p = w.serialize(obj)  # This calls __getstate__ of obj
+                
+                s = self._storage.store(oid, serial, p, self._version, transaction)
+                self._store_count = self._store_count + 1
+                # Put the object in the cache before handling the
+                # response, just in case the response contains the
+                # serial number for a newly created object
+                try:
+                    self._cache[oid] = obj
+                except:
+                    # Dang, I bet its wrapped:
+                    if hasattr(obj, 'aq_base'):
+                        self._cache[oid] = obj.aq_base
+                    else:
+                        raise
 
-            self._handle_serial(s, oid)
+                self._handle_serial(s, oid)
+        finally:
+            del self._added_during_commit
+       
 
     def commit_sub(self, t):
         """Commit all work done in all subtransactions for this transaction"""
@@ -707,10 +719,6 @@
 
         self._conflicts.clear()
         self._flush_invalidations()
-        # self._added should be empty at this point, because each object
-        # that add() was called for will have been either committed or
-        # aborted, or tpc_abort() will have been called.
-        assert not self._added, 'self._added not empty at end of tpc_finish'
 
     def sync(self):
         self.getTransaction().abort()