Add tests, docs, and a bunch of small cleanups.

Add get() as preferred synonym for __getitem__.  Change several uses
    of subscripting to get().
Change add() to register new object with transaction.
Change several methods to raise RuntimeError if they are called when
    the Connection is closed.
Add experimental epydoc markup to doc strings.
Extend doc strings and write new tests for "User Methods."

src/ZODB/Connection.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """Database connection support
 
-$Id: Connection.py,v 1.124 2004/02/25 19:09:14 jeremy Exp $"""
+$Id: Connection.py,v 1.125 2004/02/26 22:55:51 jeremy Exp $"""
 
 import logging
 import sys
@@ -91,11 +91,31 @@ class Connection(ExportImport, object):
     XXX We should document an intended API for using a Connection via
     multiple threads.
 
-    $Id: Connection.py,v 1.124 2004/02/25 19:09:14 jeremy Exp $
+    XXX We should explain that the Connection has a cache and that
+    multiple calls to get() will return a reference to the same
+    object, provided that one of the earlier objects is still
+    referenced.  Object identity is preserved within a connection, but
+    not across connections.
+
+    XXX Mention the database pool.
+
+    @group User Methods: root, get, add, close, db, sync, isReadOnly,
+        cacheFullSweep, cacheMinimize, getVersion, modifiedInVersion
+    @group Experimental Methods: setLocalTransaction, getTransaction,
+        onCloseCallbacks
+    @group Transaction Data Manager Methods: tpc_begin, tpc_vote,
+        tpc_finish, tpc_abort, sortKey, abort, commit, commit_sub,
+        abort_sub
+    @group Database Invalidation Methods: invalidate, _setDB
+    @group IPersistentDataManager Methods: setstate, register,
+        setklassstate
+    @group Other Methods: oldstate, exchange, getDebugInfo, setDebugInfo,
+        getTransferCounts
+
+    $Id: Connection.py,v 1.125 2004/02/26 22:55:51 jeremy Exp $
     """
 
     _tmp = None
-    _debug_info = ()
     _opened = None
     _code_timestamp = 0
     _transaction = None
@@ -103,9 +123,15 @@ class Connection(ExportImport, object):
 
     def __init__(self, version='', cache_size=400,
                  cache_deactivate_after=60, mvcc=True):
-        """Create a new Connection"""
+        """Create a new Connection.
+
+        A Connection instance should by instantiated by the DB
+        instance that it connects to.
+        """
 
         self._log = logging.getLogger("zodb.conn")
+        self._storage = None
+        self._debug_info = ()
 
         self._version = version
         self._cache = cache = PickleCache(self, cache_size)
@@ -188,7 +214,23 @@ class Connection(ExportImport, object):
             ver = ''
         return '<Connection at %08x%s>' % (id(self), ver)
 
-    def __getitem__(self, oid):
+    def get(self, oid):
+        """Return the persistent object with oid C{oid}.
+
+        If the object was not in the cache and the object's class is
+        ghostable, then a ghost will be returned.  If the object is
+        already in the cache, a reference to the cached object will be
+        returned.
+
+        @return: persistent object
+        @rtype: L{IPersistent}
+        @raise KeyError: If C{oid} does not exist.
+        @raise RuntimeError: If the connection is closed.
+        """
+        if self._storage is None:
+            # XXX Should this be a ZODB-specific exception?
+            raise RuntimeError("The database connection is closed")
+        
         obj = self._cache.get(oid, None)
         if obj is not None:
             return obj
@@ -207,7 +249,35 @@ class Connection(ExportImport, object):
         self._cache[oid] = obj
         return obj
 
+    # deprecate this method?
+    __getitem__ = get
+
     def add(self, obj):
+        """Add a new object C{obj} to the database and assign it an oid.
+
+        A persistent object is normally added to the database and
+        assigned an oid when it becomes reachable an object already in
+        the database.  In some cases, it is useful to create a new
+        object and uses its oid (C{_p_oid}) in a single transaction.
+
+        This method assigns a new oid regardless of whether the object
+        is reachable.
+
+        The object is added when the transaction commits.  The object
+        must implement the L{IPersisent} interface and must not
+        already be associated with a L{Connection}.
+
+        @param obj: the object to add
+        @type obj: L{IPersistent}
+        @raise TypeError: If C{obj} is not a persistent object.
+        @raise InvalidObjectReference: If C{obj} is already associated
+            with another connection.
+        @raise RuntimeError: If the connection is closed.
+        """
+        if self._storage is None:
+            # XXX Should this be a ZODB-specific exception?
+            raise RuntimeError("The database connection is closed")
+        
         marker = object()
         oid = getattr(obj, "_p_oid", marker)
         if oid is marker:
@@ -220,6 +290,7 @@ class Connection(ExportImport, object):
             self._added[oid] = obj
             if self._added_during_commit is not None:
                 self._added_during_commit.append(obj)
+            self.getTransaction().register(obj)
         elif obj._p_jar is not self:
             raise InvalidObjectReference(obj, obj._p_jar)
 
@@ -279,21 +350,49 @@ class Connection(ExportImport, object):
             else:
                 self._cache.invalidate(object._p_oid)
 
+    # XXX should there be a way to call incrgc directly?
+    # perhaps "full sweep" should do that?
+
+    # XXX we should test what happens when these methods are called
+    # mid-transaction.
+
     def cacheFullSweep(self, dt=0):
+        # XXX needs doc string
+        warnings.warn("cacheFullSweep is deprecated. "
+                      "Use cacheMinimize instead.", PendingDeprecationWarning)
         self._cache.full_sweep(dt)
 
-    def cacheMinimize(self, dt=0):
-        # dt is ignored
+    def cacheMinimize(self, dt=None):
+        # XXX needs doc string
+        if dt is not None:
+            warnings.warn("The dt argument to cacheMinimize is ignored.",
+                          PendingDeprecationWarning)
         self._cache.minimize()
 
     __onCloseCallbacks = None
 
     def onCloseCallback(self, f):
+        """Register a callable C{f} to be called by L{close}.
+
+        The callable C{f} will be called at most once, the next time
+        the Connection is closed.
+        """
         if self.__onCloseCallbacks is None:
             self.__onCloseCallbacks = []
         self.__onCloseCallbacks.append(f)
 
     def close(self):
+        """Close the C{Connection}.
+
+        A closed C{Connection} should not be used by client code.  It
+        can't load or store objects.  Objects in the cache are not
+        freed, because C{Connections} are re-used and the cache are
+        expected to be useful to the next client.
+
+        When the Connection is closed, all callbacks registered by
+        L{onCloseCallbacks} are invoked and the cache is scanned for
+        old objects.
+        """
         if self._incrgc is not None:
             self._incrgc() # This is a good time to do some GC
 
@@ -310,7 +409,9 @@ class Connection(ExportImport, object):
         self._storage = self._tmp = self.new_oid = self._opened = None
         self._debug_info = ()
         # Return the connection to the pool.
-        self._db._closeConnection(self)
+        if self._db is not None:
+            self._db._closeConnection(self)
+            self._db = None
 
     def commit(self, object, transaction):
         if object is self:
@@ -454,9 +555,15 @@ class Connection(ExportImport, object):
         return self._db
 
     def getVersion(self):
+        if self._storage is None:
+            # XXX Should this be a ZODB-specific exception?
+            raise RuntimeError("The database connection is closed")
         return self._version
 
     def isReadOnly(self):
+        if self._storage is None:
+            # XXX Should this be a ZODB-specific exception?
+            raise RuntimeError("The database connection is closed")
         return self._storage.isReadOnly()
 
     def invalidate(self, tid, oids):
@@ -515,7 +622,12 @@ class Connection(ExportImport, object):
         self.getTransaction().register(object)
 
     def root(self):
-        return self[z64]
+        """Get the database root object.
+
+        @return: the database root object
+        @rtype: C{persistent.dict.PersistentDict}
+        """
+        return self.get(z64)
 
     def setstate(self, obj):
         oid = obj._p_oid
@@ -639,8 +751,19 @@ class Connection(ExportImport, object):
             self.getTransaction().register(obj)
             raise ReadConflictError(object=obj)
 
-    def oldstate(self, obj, serial):
-        p = self._storage.loadSerial(obj._p_oid, serial)
+    def oldstate(self, obj, tid):
+        """Return copy of C{obj} that was written by C{tid}.
+
+        @param obj: the persistent object to retrieve an old revision of
+        @type obj: L{IPersistent}
+        @param tid: id of transaction that wrote revision
+        @type tid: C{string}
+
+        @raise KeyError: If C{tid} does not exist or if C{tid} deleted
+            a revision of C{obj}
+        """
+        assert obj._p_jar is self
+        p = self._storage.loadSerial(obj._p_oid, tid)
         return self._reader.getState(p)
 
     def setklassstate(self, obj):

src/ZODB/ExportImport.py

@@ -74,7 +74,7 @@ class ExportImport:
         t.commit(1)
         # Return the root imported object.
         if return_oid_list:
-            return self[return_oid_list[0]]
+            return self.get(return_oid_list[0])
         else:
             return None
 

src/ZODB/serialize.py

@@ -465,7 +465,7 @@ class ConnectionObjectReader(BaseObjectReader):
             except TypeError:
                 # Couldn't create the instance.  Maybe there's more
                 # current data in the object's actual record!
-                return self._conn[oid]
+                return self._conn.get(oid)
 
             # XXX should be done by connection
             obj._p_oid = oid
@@ -489,7 +489,7 @@ class ConnectionObjectReader(BaseObjectReader):
         obj = self._cache.get(oid, None)
         if obj is not None:
             return obj
-        return self._conn[oid]
+        return self._conn.get(oid)
 
 def referencesf(p, rootl=None):
 

src/ZODB/tests/testConnection.py

@@ -11,11 +11,14 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-"""Unit tests for the Connection class.
-"""
+"""Unit tests for the Connection class."""
 
+import doctest
 import unittest
+
 from persistent import Persistent
+from ZODB.config import databaseFromString
+from ZODB.utils import p64, u64
 
 class ConnectionDotAdd(unittest.TestCase):
 
@@ -34,7 +37,7 @@ class ConnectionDotAdd(unittest.TestCase):
         self.datamgr.add(obj)
         self.assert_(obj._p_oid is not None)
         self.assert_(obj._p_jar is self.datamgr)
-        self.assert_(self.datamgr[obj._p_oid] is obj)
+        self.assert_(self.datamgr.get(obj._p_oid) is obj)
 
         # Only first-class persistent objects may be added.
         self.assertRaises(TypeError, self.datamgr.add, object())
@@ -59,7 +62,7 @@ class ConnectionDotAdd(unittest.TestCase):
         self.datamgr.abort(obj, self.transaction)
         self.assert_(obj._p_oid is None)
         self.assert_(obj._p_jar is None)
-        self.assertRaises(KeyError, self.datamgr.__getitem__, oid)
+        self.assertRaises(KeyError, self.datamgr.get, oid)
 
     def checkResetOnTpcAbort(self):
         obj = StubObject()
@@ -73,7 +76,7 @@ class ConnectionDotAdd(unittest.TestCase):
         self.datamgr.tpc_abort(self.transaction)
         self.assert_(obj._p_oid is None)
         self.assert_(obj._p_jar is None)
-        self.assertRaises(KeyError, self.datamgr.__getitem__, oid)
+        self.assertRaises(KeyError, self.datamgr.get, oid)
 
     def checkTcpAbortAfterCommit(self):
         obj = StubObject()
@@ -86,7 +89,7 @@ class ConnectionDotAdd(unittest.TestCase):
         self.datamgr.tpc_abort(self.transaction)
         self.assert_(obj._p_oid is None)
         self.assert_(obj._p_jar is None)
-        self.assertRaises(KeyError, self.datamgr.__getitem__, oid)
+        self.assertRaises(KeyError, self.datamgr.get, oid)
         self.assertEquals(self.db._storage._stored, [oid])
 
     def checkCommit(self):
@@ -138,6 +141,200 @@ class ConnectionDotAdd(unittest.TestCase):
         self.datamgr.tpc_finish(self.transaction)
         self.assert_(obj._p_oid not in self.datamgr._storage._stored)
 
+class UserMethodTests(unittest.TestCase):
+
+    # XXX add isn't tested here, because there are a bunch of traditional
+    # unit tests for it.
+
+    # XXX the version tests would require a storage that supports versions
+    # which is a bit more work.
+
+    def test_root(self):
+        r"""doctest of root() method
+
+        The root() method is simple, and the tests are pretty minimal.
+        Ensure that a new database has a root and that it is a
+        PersistentMapping.
+        
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+        >>> root = cn.root()
+        >>> type(root).__name__
+        'PersistentMapping'
+        >>> root._p_oid
+        '\x00\x00\x00\x00\x00\x00\x00\x00'
+        >>> root._p_jar is cn
+        True
+        >>> db.close()
+        """
+
+    def test_get(self):
+        r"""doctest of get() method
+
+        The get() method return the persistent object corresponding to
+        an oid.
+
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+        >>> obj = cn.get(p64(0))
+        >>> obj._p_oid
+        '\x00\x00\x00\x00\x00\x00\x00\x00'
+
+        The object is a ghost.
+        
+        >>> obj._p_state 
+        -1
+
+        And multiple calls with the same oid, return the same object.
+        
+        >>> obj2 = cn.get(p64(0))
+        >>> obj is obj2
+        True
+
+        If all references to the object are released, then a new
+        object will be returned. The cache doesn't keep unreferenced
+        ghosts alive.  (The next object returned my still have the
+        same id, because Python may re-use the same memory.)
+        
+        >>> del obj, obj2
+        >>> cn._cache.get(p64(0), None)
+
+        If the object is unghosted, then it will stay in the cache
+        after the last reference is released.  (This is true only if
+        there is room in the cache and the object is recently used.)
+
+        >>> obj = cn.get(p64(0))
+        >>> obj._p_activate()
+        >>> y = id(obj)
+        >>> del obj
+        >>> obj = cn.get(p64(0))
+        >>> id(obj) == y
+        True
+        >>> obj._p_state
+        0
+
+        A request for an object that doesn't exist will raise a KeyError.
+
+        >>> cn.get(p64(1))
+        Traceback (most recent call last):
+          ...
+        KeyError: '\x00\x00\x00\x00\x00\x00\x00\x01'
+        """
+
+    def test_close(self):
+        r"""doctest of close() method
+
+        This is a minimal test, because most of the interesting
+        effects on closing a connection involved its interaction the
+        database and transaction.
+
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+
+        It's safe to close a connection multiple times.
+        >>> cn.close()
+        >>> cn.close()
+        >>> cn.close()
+
+        It's not possible to load or store objects once the storage is
+        closed.
+        
+        >>> cn.get(p64(0))
+        Traceback (most recent call last):
+          ...
+        RuntimeError: The database connection is closed
+        >>> p = Persistent()
+        >>> cn.add(p)
+        Traceback (most recent call last):
+          ...
+        RuntimeError: The database connection is closed
+        """
+
+    def test_onCloseCallbacks(self):
+        r"""doctest of onCloseCallback() method
+
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+
+        Every function registered is called, even if it raises an
+        exception.  They are only called once.
+
+        >>> L = []
+        >>> def f():
+        ...     L.append("f")
+        >>> def g():
+        ...     L.append("g")
+        ...     return 1 / 0
+        >>> cn.onCloseCallback(g)
+        >>> cn.onCloseCallback(f)
+        >>> cn.close()
+        >>> L
+        ['g', 'f']
+        >>> del L[:]
+        >>> cn.close()
+        >>> L
+        []
+
+        The implementation keeps a list of callbacks that is reset
+        to a class variable (which is bound to None) after the connection
+        is closed.
+        
+        >>> cn._Connection__onCloseCallbacks
+        """
+
+    def test_db(self):
+        r"""doctest of db() method
+
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+        >>> cn.db() is db
+        True
+        >>> cn.close()
+        >>> cn.db()
+        """
+
+    def test_isReadOnly(self):
+        r"""doctest of isReadOnly() method
+
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+        >>> cn.isReadOnly()
+        False
+        >>> cn.close()
+        >>> cn.isReadOnly()
+        Traceback (most recent call last):
+          ...
+        RuntimeError: The database connection is closed
+
+        An expedient way to create a read-only storage:
+        
+        >>> db._storage._is_read_only = True
+        >>> cn = db.open()
+        >>> cn.isReadOnly()
+        True
+        """
+
+    def test_cache(self):
+        r"""doctest of cacheMinimize() and cacheFullSweep() methods.
+
+        These tests are fairly minimal, just verifying that the
+        methods can be called and have some effect.  We need other
+        tests that verify the cache works as intended.
+
+        >>> db = databaseFromString("<zodb>\n<mappingstorage/>\n</zodb>")
+        >>> cn = db.open()
+        >>> r = cn.root()
+        >>> r._p_activate()
+        >>> cn.cacheFullSweep()
+        >>> r._p_state
+        0
+        >>> cn.cacheMinimize()
+        >>> r._p_state
+        -1
+        >>> cn.cacheFullSweep(12)
+        >>> cn.cacheMinimize(12)
+        """
+
 # ---- stubs
 
 class StubObject(Persistent):
@@ -262,4 +459,6 @@ class StubDatabase:
 
 
 def test_suite():
-    return unittest.makeSuite(ConnectionDotAdd, 'check')
+    s = unittest.makeSuite(ConnectionDotAdd, 'check')
+    s.addTest(doctest.DocTestSuite())
+    return s