[Zodb-checkins] CVS: Zope3/lib/python/ZODB - Connection.py:1.60.6.3

Jeremy Hylton jeremy@zope.com
Thu, 21 Feb 2002 18:25:55 -0500


Update of /cvs-repository/Zope3/lib/python/ZODB
In directory cvs.zope.org:/tmp/cvs-serv15451

Modified Files:
      Tag: Zope-3x-branch
	Connection.py 
Log Message:
Lots of refactoring plus fixes for BTrees.

Fix new_persistent_id() to work with new-style types.  A BTree object,
for example, has an _p_oid attribute.  The BTree _type_ also has a
_p_oid attribute -- the descriptor for the BTree objects.  So the
simple hasattr() test in the pre-2.2 version of the code was
insufficient.  Unfortunately, it's not obvious how to ask if something
is a descriptor, so I extended the test in a different way.

new_persistent_id() does not consider an object to be a persistent
object unless it has an _p_oid attribute *and* that attribute is a
string.  we may need to modify the test in the future, but it works
for now.

Refactoring:
    Replace apply() with extended call syntax.
    Add whitespace when needed.
    Refactor commit() by adding helper method commit_store().



=== Zope3/lib/python/ZODB/Connection.py 1.60.6.2 => 1.60.6.3 ===
 from zLOG import LOG, ERROR, BLATHER
 from ConflictResolution import ResolvedSerial
+from ZODB.utils import U64
 
 from cPickle import Unpickler, Pickler
 from cStringIO import StringIO
 import sys
 from time import time
-from types import StringType, ClassType
+from types import StringType, ClassType, TupleType
 
 from Transaction import get_transaction
 
@@ -78,13 +79,12 @@
         try: del self.cacheGC
         except: pass
 
-    def __getitem__(self, oid,
-                    tt=type(())):
-        cache=self._cache
-        object=cache.get(oid, self)
-        if object is not self: return object
+    def __getitem__(self, oid):
+        object = self._cache.get(oid, self)
+        if object is not self:
+            return object
 
-        __traceback_info__ = (oid)
+        __traceback_info__ = (oid,)
         p, serial = self._storage.load(oid, self._version)
         __traceback_info__ = (oid, p)
         file=StringIO(p)
@@ -99,7 +99,7 @@
 
         klass, args = object
 
-        if type(klass) is tt:
+        if isinstance(klass, TupleType):
             module, name = klass
             klass=self._db._classFactory(self, module, name)
         
@@ -110,17 +110,18 @@
             # The contacts are a bit unclear.
             object=__new__(klass)
         else:
-            object=apply(klass,args)
-            if klass is not ExtensionKlass:
+            object = klass(*args)
+            if klass is not type:
                 object.__dict__.clear()
 
-        object._p_oid=oid
-        object._p_jar=self
-        object._p_changed=None
-        object._p_serial=serial
-
-        cache[oid]=object
-        if oid=='\0\0\0\0\0\0\0\0': self._root_=object # keep a ref
+        object._p_oid = oid
+        object._p_jar = self
+        object._p_changed = None
+        object._p_serial = serial
+
+        self._cache[oid] = object
+        if oid == '\0\0\0\0\0\0\0\0':
+            self._root_ = object # keep a ref
         return object
 
     ######################################################################
@@ -313,7 +314,7 @@
             # We registered ourself.  Execute a commit action, if any.
             if self.__onCommitActions is not None:
                 method_name, args, kw = self.__onCommitActions.pop(0)
-                apply(getattr(self, method_name), (transaction,) + args, kw)
+                getattr(self, method_name)(transaction, *args, **kw)
             return
         oid=object._p_oid
         invalid=self._invalid
@@ -337,80 +338,82 @@
             # Nothing to do
             return
 
-        stack=[object]
+        stack = [object]
         
-        file=StringIO()
-        seek=file.seek
-        pickler=Pickler(file,1)
-        pickler.persistent_id=new_persistent_id(self, stack.append)
-        dbstore=self._storage.store
-        file=file.getvalue
-        cache=self._cache
-        get=cache.get
-        dump=pickler.dump
-        clear_memo=pickler.clear_memo
+        file = StringIO()
+        pickler = Pickler(file, 1)
+        pickler.persistent_id = new_persistent_id(self, stack)
 
-
-        version=self._version
+        version = self._version
         
         while stack:
-            object=stack[-1]
-            del stack[-1]
-            oid=object._p_oid
-            serial=getattr(object, '_p_serial', '\0\0\0\0\0\0\0\0')
-            if serial == '\0\0\0\0\0\0\0\0':
-                # new object
-                self._creating.append(oid)
+            self.commit_store(stack.pop(), file, pickler, transaction)
+
+    def commit_store(self, pobject, file, pickler, transaction):
+        # XXX the file and pickler get reset each time through...
+        # Maybe just create new ones each time?  Except I'm not sure
+        # how that interacts with the persistent_id attribute.
+        oid = pobject._p_oid
+        serial = getattr(pobject, '_p_serial', '\0\0\0\0\0\0\0\0')
+        if serial == '\0\0\0\0\0\0\0\0':
+            # new object
+            self._creating.append(oid)
+        else:
+            #XXX We should never get here
+            #jer: Don't understand previous comment.
+            if ((self._invalidated.has_key(oid) and
+                 not hasattr(pobject, '_p_resolveConflict'))
+                or self._invalidated.has_key(None)):
+                raise ConflictError, `oid`
+            self._invalidating.append(oid)
+
+
+        klass = pobject.__class__
+
+        # It's unclear what exactly this conditional is testing and
+        # why it takes one branch or the other.  It looks like a
+        # Persistent object will never has it's __getstate__() called.
+        if pobject is type:
+            dict = {}
+            dict.update(pobject.__dict__)
+            # XXX we delete the _p_jar, but not, say, _p_oid?
+            del dict['_p_jar']
+            args = pobject.__name__, pobject.__bases__, dict
+            state = None
+        else:
+            # XXX For what kind of object is this path taken?
+            if hasattr(klass, '__getinitargs__'):
+                args = pobject.__getinitargs__()
+                len(args) # XXX Assert it's a sequence
             else:
-                #XXX We should never get here
-                if (
-                    (invalid(oid) and
-                     not hasattr(object, '_p_resolveConflict'))
-                    or
-                    invalid(None)
-                    ):
-                    raise ConflictError, `oid`
-                self._invalidating.append(oid)
-                
-            klass = object.__class__
-        
-            if klass is ExtensionKlass:
-                # Yee Ha!
-                dict={}
-                dict.update(object.__dict__)
-                del dict['_p_jar']
-                args=object.__name__, object.__bases__, dict
-                state=None
+                args = None # New no-constructor protocol!
+
+            module = getattr(klass,'__module__','')
+            if module:
+                klass = module, klass.__name__
+            __traceback_info__ = klass, oid, self._version
+            state = pobject.__getstate__()
+
+        file.seek(0)
+        pickler.clear_memo()
+        pickler.dump((klass, args))
+        pickler.dump(state)
+        p = file.getvalue(1)
+        s = self._storage.store(oid, serial, p, self._version, transaction)
+        # 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] = pobject
+        except:
+            # Dang, I bet its wrapped:
+            # jer: Why does this mean?
+            if hasattr(pobject, 'aq_base'):
+                self._cache[oid] = pobject.aq_base
             else:
-                if hasattr(klass, '__getinitargs__'):
-                    args = object.__getinitargs__()
-                    len(args) # XXX Assert it's a sequence
-                else:
-                    args = None # New no-constructor protocol!
+                raise
+        self._handle_serial(s, oid)
         
-                module=getattr(klass,'__module__','')
-                if module: klass=module, klass.__name__
-                __traceback_info__=klass, oid, self._version
-                state=object.__getstate__()
-        
-            seek(0)
-            clear_memo()
-            dump((klass,args))
-            dump(state)
-            p=file(1)
-            s=dbstore(oid,serial,p,version,transaction)
-            # 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: cache[oid]=object
-            except:
-                # Dang, I bet its wrapped:
-                if hasattr(object, 'aq_base'):
-                    cache[oid]=object.aq_base
-                else:
-                    raise
-
-            self._handle_serial(s, oid)
 
     def commit_sub(self, t):
         """Commit all work done in subtransactions"""
@@ -429,7 +432,6 @@
         load=src.load
         store=tmp.store
         dest=self._version
-        get=self._cache.get
         oids=src._index.keys()
 
         # Copy invalidating and creating info from temporary storage:
@@ -485,11 +487,13 @@
         self._invalidated[oid]=1
 
     def modifiedInVersion(self, oid):
-        try: return self._db.modifiedInVersion(oid)
+        try:
+            return self._db.modifiedInVersion(oid)
         except KeyError:
             return self._version
 
-    def root(self): return self['\0\0\0\0\0\0\0\0']
+    def root(self):
+        return self['\0\0\0\0\0\0\0\0']
 
     def oldstate(self, object, serial):
         """Return the state of an object as it existed in some time in the past
@@ -520,20 +524,20 @@
     
             klass, args = copy
     
-            if klass is not ExtensionKlass:
+            if klass is not type:
                 LOG('ZODB',ERROR,
-                    "Unexpected klass when setting class state on %s"
+                    "Unexpected class when setting class state on %s"
                     % getattr(object,'__name__','(?)'))
                 return
             
-            copy=apply(klass,args)
+            copy = klass(*args)
             object.__dict__.clear()
             object.__dict__.update(copy.__dict__)
     
-            object._p_oid=oid
-            object._p_jar=self
-            object._p_changed=0
-            object._p_serial=serial
+            object._p_oid = oid
+            object._p_jar = self
+            object._p_changed = 0
+            object._p_serial = serial
         except:
             LOG('ZODB',ERROR, 'setklassstate failed', error=sys.exc_info())
             raise
@@ -542,9 +546,8 @@
         if self.__onCommitActions is not None:
             del self.__onCommitActions
         self._storage.tpc_abort(transaction)
-        cache=self._cache
-        cache.invalidateMany(self._invalidated)
-        cache.invalidateMany(self._invalidating)
+        self._cache.invalidateMany(self._invalidated)
+        self._cache.invalidateMany(self._invalidating)
         self._invalidate_creating()
 
     def tpc_begin(self, transaction, sub=None):
@@ -671,34 +674,53 @@
     def close(self):
         self._breakcr()
 
-def new_persistent_id(self, stackup):
-
-    # Create a special persistent_id that passes T and the subobject
-    # stack along:
+def new_persistent_id(self, stack):
+    # XXX need a doc string.  not sure if the one for persistent_id()
+    # below is correct.
+
+    # Create a special persistent_id that captures T and the subobject
+    # stack in a closure.
+
+    def persistent_id(object):
+        """Test if an object is persistent, returning an oid if it is.
+
+        This function is used by the pickler to test whether an object
+        is persistent.  If it isn't, the function returns None and the
+        object is included in the pickle for the current persistent
+        object.
 
-    def persistent_id(object, self=self, stackup=stackup):
-
-        new_oid = self.new_oid
+        If it is persistent, it returns the oid and sometimes a tuple
+        with other stuff.
+        """
         
         if (not hasattr(object, '_p_oid') or
-            type(object) is ClassType): return None
-
-        oid=object._p_oid
+            isinstance(object, ClassType)):
+            return None
+        
+        oid = object._p_oid
 
+        # I'd like to write something like this --
+        # if isinstance(oid, types.MemberDescriptor):
+        # -- but I can't because the type doesn't have a canonical name.
+        # Instead, we'll assert that an oid must always be a string
+        if not (isinstance(oid, StringType) or oid is None):
+            return None
+        
         if oid is None or object._p_jar is not self:
             oid = self.new_oid()
-            object._p_jar=self
-            object._p_oid=oid
-            stackup(object)
-
-        klass=object.__class__
-
-        if klass is ExtensionKlass: return oid
-
-        if hasattr(klass, '__getinitargs__'): return oid
-
-        module=getattr(klass,'__module__','')
-        if module: klass=module, klass.__name__
+            object._p_jar = self
+            object._p_oid = oid
+            stack.append(object)
+
+        klass = object.__class__
+
+        if klass is type:
+            return oid
+        if hasattr(klass, '__getinitargs__'):
+            return oid
+        module = getattr(klass,'__module__', '')
+        if module:
+            klass = module, klass.__name__
 
         return oid, klass