[Zodb-checkins] SVN: ZODB/branches/gocept-iteration/src/Z Fix the issues that Jim pointed out when reviewing this branch, namely:

Christian Theune ct at gocept.com
Tue Aug 26 12:05:55 EDT 2008


Log message for revision 90352:
  Fix the issues that Jim pointed out when reviewing this branch, namely:
  
  - wildly improved efficiency of getting a data record iterator
  - merge error marshalling code for the StorageServer's store/restore methods
  - use 1-character codes for differentiating the store type in the commit log
  - garbage collect iterators at notifyDisconnected
  - Undid change to extension data pickling which caused test breakage and
    changes to the FileStorage data format.
  - Removed explicit out-side calls to `close` on the FileStorageIterator. Those
    still should be called from the inside now and might cause temporary
    breakage on Windows.
  - added BBB alias for the `RecordIterator` symbol that was imported to
    ZODB.FileStorage
  
  

Changed:
  U   ZODB/branches/gocept-iteration/src/ZEO/ClientStorage.py
  U   ZODB/branches/gocept-iteration/src/ZEO/ServerStub.py
  U   ZODB/branches/gocept-iteration/src/ZEO/StorageServer.py
  U   ZODB/branches/gocept-iteration/src/ZEO/tests/IterationTests.py
  U   ZODB/branches/gocept-iteration/src/ZEO/tests/testZEO.py
  U   ZODB/branches/gocept-iteration/src/ZODB/BaseStorage.py
  U   ZODB/branches/gocept-iteration/src/ZODB/DemoStorage.py
  U   ZODB/branches/gocept-iteration/src/ZODB/FileStorage/FileStorage.py
  U   ZODB/branches/gocept-iteration/src/ZODB/FileStorage/__init__.py
  U   ZODB/branches/gocept-iteration/src/ZODB/interfaces.py
  U   ZODB/branches/gocept-iteration/src/ZODB/tests/IteratorStorage.py
  U   ZODB/branches/gocept-iteration/src/ZODB/tests/PackableStorage.py
  U   ZODB/branches/gocept-iteration/src/ZODB/tests/RecoveryStorage.py
  U   ZODB/branches/gocept-iteration/src/ZODB/utils.py

-=-
Modified: ZODB/branches/gocept-iteration/src/ZEO/ClientStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZEO/ClientStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZEO/ClientStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -30,7 +30,7 @@
 import logging
 import weakref
 
-from zope.interface import implements
+import zope.interface
 from ZEO import ServerStub
 from ZEO.cache import ClientCache
 from ZEO.TransactionBuffer import TransactionBuffer
@@ -101,8 +101,9 @@
     tpc_begin().
     """
 
-    implements(ZODB.interfaces.IBlobStorage,
-               ZODB.interfaces.IStorageIteration)
+    # ClientStorage does not declare any interfaces here. Interfaces are
+    # declared according to the server's storage once a connection is
+    # established.
 
     # Classes we instantiate.  A subclass might override.
     TransactionBufferClass = TransactionBuffer
@@ -274,7 +275,7 @@
         self._verification_invalidations = None
 
         self._info = {'length': 0, 'size': 0, 'name': 'ZEO Client',
-                      'supportsUndo':0}
+                      'supportsUndo': 0, 'interfaces': ()}
 
         self._tbuf = self.TransactionBufferClass()
         self._db = None
@@ -533,6 +534,15 @@
 
         self._handle_extensions()
 
+        # Decorate ClientStorage with all interfaces that the backend storage
+        # supports.
+        remote_interfaces = []
+        for module_name, interface_name in self._info['interfaces']:
+            module = __import__(module_name, globals(), locals(), [interface_name])
+            interface = getattr(module, interface_name)
+            remote_interfaces.append(interface)
+        zope.interface.directlyProvides(self, remote_interfaces)
+
     def _handle_extensions(self):
         for name in self.getExtensionMethods().keys():
             if not hasattr(self, name):
@@ -639,6 +649,7 @@
         self._ready.clear()
         self._server = disconnected_stub
         self._midtxn_disconnect = 1
+        self._iterator_gc()
 
     def __len__(self):
         """Return the size of the storage."""
@@ -1182,8 +1193,8 @@
         assert not version
         self._check_trans(transaction)
         self._server.restorea(oid, serial, data, prev_txn, id(transaction))
-        # XXX I'm not updating the transaction buffer here because I can't
-        # exactly predict how invalidation should work with restore. :/
+        # Don't update the transaction buffer, because current data are
+        # unaffected.
         return self._check_serials()
 
     # Below are methods invoked by the StorageServer
@@ -1267,24 +1278,10 @@
         # iids are "iterator IDs" that can be used to query an iterator whose
         # status is held on the server.
         iid = self._server.iterator_start(start, stop)
-        return self._setup_iterator(self._iterator, iid)
+        return self._setup_iterator(TransactionIterator, iid)
 
-    def _iterator(self, iid):
-        while True:
-            item = self._server.iterator_next(iid)
-            if item is None:
-                # The iterator is exhausted, and the server has already
-                # disposed it.
-                self._forget_iterator(iid)
-                break
-
-            tid = item[0]
-            riid = self._server.iterator_record_start(tid)
-            yield self._setup_iterator(ClientStorageTransactionInformation,
-                                       riid, self, *item)
-
     def _setup_iterator(self, factory, iid, *args):
-        self._iterators[iid] = iterator = factory(iid, *args)
+        self._iterators[iid] = iterator = factory(self, iid, *args)
         self._iterator_ids.add(iid)
         return iterator
 
@@ -1307,12 +1304,38 @@
         self._iterator_ids -= iids
 
 
+class TransactionIterator(object):
+
+    def __init__(self, storage, iid, *args):
+        self._storage = storage 
+        self._iid = iid
+        self._ended = False
+
+    def __iter__(self):
+        return self
+
+    def next(self):
+        if self._ended:
+            raise ZODB.interfaces.StorageStopIteration()
+
+        tx_data = self._storage._server.iterator_next(self._iid)
+        if tx_data is None:
+            # The iterator is exhausted, and the server has already
+            # disposed it.
+            self._ended = True
+            self._storage._forget_iterator(self._iid)
+            raise ZODB.interfaces.StorageStopIteration()
+
+        return ClientStorageTransactionInformation(self._storage, self, *tx_data)
+
+
 class ClientStorageTransactionInformation(ZODB.BaseStorage.TransactionRecord):
 
-    def __init__(self, riid, storage, tid, status, user, description, extension):
+    def __init__(self, storage, txiter, tid, status, user, description, extension):
         self._storage = storage
-        self._riid = riid
+        self._txiter = txiter
         self._completed = False
+        self._riid = None
 
         self.tid = tid
         self.status = status
@@ -1321,18 +1344,29 @@
         self.extension = extension
 
     def __iter__(self):
+        riid = self._storage._server.iterator_record_start(self._txiter._iid, self.tid)
+        return self._storage._setup_iterator(RecordIterator, riid)
+
+
+class RecordIterator(object):
+
+    def __init__(self, storage, riid):
+        self._riid = riid
+        self._completed = False
+        self._storage = storage
+
+    def __iter__(self):
         return self
 
     def next(self):
         if self._completed:
             # We finished iteration once already and the server can't know
             # about the iteration anymore.
-            raise StopIteration
+            raise ZODB.interfaces.StorageStopIteration()
         item = self._storage._server.iterator_record_next(self._riid)
         if item is None:
             # The iterator is exhausted, and the server has already
             # disposed it.
-            self._storage._forget_iterator(self._riid)
             self._completed = True
-            raise StopIteration
+            raise ZODB.interfaces.StorageStopIteration()
         return ZODB.BaseStorage.DataRecord(*item)

Modified: ZODB/branches/gocept-iteration/src/ZEO/ServerStub.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZEO/ServerStub.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZEO/ServerStub.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -302,8 +302,8 @@
     def iterator_next(self, iid):
         return self.rpc.call('iterator_next', iid)
 
-    def iterator_record_start(self, tid):
-        return self.rpc.call('iterator_record_start', tid)
+    def iterator_record_start(self, txn_iid, tid):
+        return self.rpc.call('iterator_record_start', txn_iid, tid)
 
     def iterator_record_next(self, iid):
         return self.rpc.call('iterator_record_next', iid)

Modified: ZODB/branches/gocept-iteration/src/ZEO/StorageServer.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZEO/StorageServer.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZEO/StorageServer.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -36,6 +36,7 @@
 import ZODB.serialize
 import ZEO.zrpc.error
 
+import zope.interface
 from ZEO import ClientStub
 from ZEO.CommitLog import CommitLog
 from ZEO.monitor import StorageStats, StatsServer
@@ -112,6 +113,9 @@
             self._extensions[func.func_name] = None
         self._iterators = {}
         self._iterator_ids = itertools.count()
+        # Stores the last item that was handed out for a
+        # transaction iterator.
+        self._txn_iterators_last = {}
 
     def finish_auth(self, authenticated):
         if not self.auth_realm:
@@ -274,6 +278,12 @@
         else:
             supportsUndo = supportsUndo()
 
+        # Communicate the backend storage interfaces to the client
+        storage_provides = zope.interface.providedBy(storage)
+        interfaces = []
+        for candidate in storage_provides.__iro__:
+            interfaces.append((candidate.__module__, candidate.__name__))
+
         return {'length': len(storage),
                 'size': storage.getSize(),
                 'name': storage.getName(),
@@ -281,6 +291,7 @@
                 'supportsVersions': False,
                 'extensionMethods': self.getExtensionMethods(),
                 'supports_record_iternext': hasattr(self, 'record_iternext'),
+                'interfaces': tuple(interfaces),
                 }
 
     def get_size_info(self):
@@ -551,16 +562,7 @@
                 # Unexpected errors are logged and passed to the client
                 self.log("store error: %s, %s" % sys.exc_info()[:2],
                          logging.ERROR, exc_info=True)
-            # Try to pickle the exception.  If it can't be pickled,
-            # the RPC response would fail, so use something else.
-            pickler = cPickle.Pickler()
-            pickler.fast = 1
-            try:
-                pickler.dump(err, 1)
-            except:
-                msg = "Couldn't pickle storage exception: %s" % repr(err)
-                self.log(msg, logging.ERROR)
-                err = StorageServerError(msg)
+            err = self._marshal_error(err)
             # The exception is reported back as newserial for this oid
             newserial = [(oid, err)]
         else:
@@ -594,21 +596,25 @@
                 # Unexpected errors are logged and passed to the client
                 self.log("store error: %s, %s" % sys.exc_info()[:2],
                          logging.ERROR, exc_info=True)
-            # Try to pickle the exception.  If it can't be pickled,
-            # the RPC response would fail, so use something else.
-            pickler = cPickle.Pickler()
-            pickler.fast = 1
-            try:
-                pickler.dump(err, 1)
-            except:
-                msg = "Couldn't pickle storage exception: %s" % repr(err)
-                self.log(msg, logging.ERROR)
-                err = StorageServerError(msg)
+            err = self._marshal_error(err)
             # The exception is reported back as newserial for this oid
             self.serials.append((oid, err))
 
         return err is None
 
+    def _marshal_error(self, error):
+        # Try to pickle the exception.  If it can't be pickled,
+        # the RPC response would fail, so use something that can be pickled.
+        pickler = cPickle.Pickler()
+        pickler.fast = 1
+        try:
+            pickler.dump(error, 1)
+        except:
+            msg = "Couldn't pickle storage exception: %s" % repr(error)
+            self.log(msg, logging.ERROR)
+            error = StorageServerError(msg)
+        return error
+
     def _vote(self):
         if not self.store_failed:
             # Only call tpc_vote of no store call failed, otherwise
@@ -665,10 +671,12 @@
             store_type = store[0]
             store_args = store[1:]
 
-            if store_type == 'store':
+            if store_type == 's':
                 do_store = self._store
-            elif store_type == 'restore':
+            elif store_type == 'r':
                 do_store = self._restore
+            else:
+                raise ValueError('Invalid store type: %r' % store_type)
 
             if not do_store(*store_args):
                 break
@@ -739,20 +747,27 @@
         except StopIteration:
             del self._iterators[iid]
             item = None
+            if iid in self._txn_iterators_last:
+                del self._txn_iterators_last[iid]
         else:
             item = (info.tid,
                     info.status,
                     info.user,
                     info.description,
                     info.extension)
+            # Keep a reference to the last iterator result to allow starting a
+            # record iterator off it.
+            self._txn_iterators_last[iid] = info
         return item
 
-    def iterator_record_start(self, tid):
-        iid = self._iterator_ids.next()
-        txn_infos = list(self.storage.iterator(tid, tid))
-        assert len(txn_infos) == 1, "%s" % txn_infos
-        self._iterators[iid] = iter(txn_infos[0])
-        return iid
+    def iterator_record_start(self, txn_iid, tid):
+        record_iid = self._iterator_ids.next()
+        txn_info = self._txn_iterators_last[txn_iid]
+        if txn_info.tid != tid:
+            raise Exception(
+                'Out-of-order request for record iterator for transaction %r' % tid)
+        self._iterators[record_iid] = iter(txn_info)
+        return record_iid
 
     def iterator_record_next(self, iid):
         iterator = self._iterators[iid]

Modified: ZODB/branches/gocept-iteration/src/ZEO/tests/IterationTests.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZEO/tests/IterationTests.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZEO/tests/IterationTests.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -75,6 +75,17 @@
         self.assertEquals(0, len(self._storage._iterator_ids))
         self.assertRaises(KeyError, self._storage._server.iterator_next, iid)
 
+    def checkIteratorGCStorageDisconnect(self):
+        self._storage.iterator()
+        iid = list(self._storage._iterator_ids)[0]
+        t = transaction.Transaction()
+        self._storage.tpc_begin(t)
+        # Show that after disconnecting, the client side GCs the iterators
+        # as well. I'm calling this directly to avoid accidentally
+        # calling tpc_abort implicitly.
+        self._storage.notifyDisconnected()
+        self.assertEquals(0, len(self._storage._iterator_ids))
+
     def checkIteratorParallel(self):
         self._dostore()
         self._dostore()

Modified: ZODB/branches/gocept-iteration/src/ZEO/tests/testZEO.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZEO/tests/testZEO.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZEO/tests/testZEO.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -330,6 +330,20 @@
         </filestorage>
         """ % filename
 
+    def checkInterfaceFromRemoteStorage(self):
+        # ClientStorage itself doesn't implement IStorageIteration, but the
+        # FileStorage on the other end does, and thus the ClientStorage
+        # instance that is connected to it reflects this.
+        self.failIf(ZODB.interfaces.IStorageIteration.implementedBy(
+            ZEO.ClientStorage.ClientStorage))
+        self.failUnless(ZODB.interfaces.IStorageIteration.providedBy(
+            self._storage))
+        # This is communicated using ClientStorage's _info object:
+        self.assertEquals((('ZODB.interfaces', 'IStorageIteration'),
+                           ('zope.interface', 'Interface')),
+                          self._storage._info['interfaces'])
+
+
 class MappingStorageTests(GenericTests):
     """ZEO backed by a Mapping storage."""
 

Modified: ZODB/branches/gocept-iteration/src/ZODB/BaseStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/BaseStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/BaseStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -188,7 +188,11 @@
             user = transaction.user
             desc = transaction.description
             ext = transaction._extension
-            ext = cPickle.dumps(ext, 1)
+            if ext:
+                ext = cPickle.dumps(ext, 1)
+            else:
+                ext = ""
+
             self._ude = user, desc, ext
 
             if tid is None:
@@ -349,12 +353,7 @@
         dest.tpc_vote(transaction)
         dest.tpc_finish(transaction)
 
-    if hasattr(fiter, 'close'):
-        # XXX close is not part of the iterator interface but FileStorage's
-        # iterator has this method to get rid of a file handle.
-        fiter.close()
 
-
 class TransactionRecord(object):
     """Abstract base class for iterator protocol"""
 

Modified: ZODB/branches/gocept-iteration/src/ZODB/DemoStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/DemoStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/DemoStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -84,6 +84,8 @@
 import base64, time
 
 import ZODB.BaseStorage
+import ZODB.interfaces
+import zope.interface
 from ZODB import POSException
 from ZODB.utils import z64, oid_repr
 from persistent.TimeStamp import TimeStamp
@@ -106,6 +108,7 @@
     
     """
     
+    zope.interface.implements(ZODB.interfaces.IStorageIteration)
 
     def __init__(self, name='Demo Storage', base=None, quota=None):
         ZODB.BaseStorage.BaseStorage.__init__(self, name, base)
@@ -575,3 +578,27 @@
     def close(self):
         if self._base is not None:
             self._base.close()
+
+    def iterator(self, start=None, end=None):
+        # First iterate over the base storage
+        if self._base is not None:
+            for transaction in self._base.iterator(start, end):
+                yield transaction
+        # Then iterate over our local transactions
+        for tid, transaction in self._data.items():
+            if tid >= start and tid <= end:
+                yield TransactionRecord(tid, transaction)
+
+
+class TransactionRecord(ZODB.BaseStorage.TransactionRecord):
+    
+    def __init__(self, tid, transaction):
+        packed, user, description, extension, records = transaction
+        super(TransactionRecord, self).__init__(
+            tid, packed, user, description, extension)
+        self.records = transaction
+
+    def __iter__(self):
+        for record in self.records:
+            oid, prev, version, data, tid = record
+            yield ZODB.BaseStorage.DataRecord(oid, tid, data, version, prev)

Modified: ZODB/branches/gocept-iteration/src/ZODB/FileStorage/FileStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/FileStorage/FileStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/FileStorage/FileStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -29,6 +29,8 @@
 # Not all platforms have fsync
 fsync = getattr(os, "fsync", None)
 
+import zope.interface
+import ZODB.interfaces
 from ZODB import BaseStorage, ConflictResolution, POSException
 from ZODB.POSException import UndoError, POSKeyError, MultipleUndoErrors
 from persistent.TimeStamp import TimeStamp
@@ -88,6 +90,8 @@
                   ConflictResolution.ConflictResolvingStorage,
                   FileStorageFormatter):
 
+    zope.interface.implements(ZODB.interfaces.IStorageIteration)
+
     # Set True while a pack is in progress; undo is blocked for the duration.
     _pack_is_in_progress = False
 
@@ -1683,7 +1687,7 @@
 
             return result
 
-        raise StopIteration
+        raise ZODB.interfaces.StorageStopIteration()
 
 
 class TransactionRecord(BaseStorage.TransactionRecord, FileStorageFormatter):
@@ -1731,7 +1735,7 @@
 
             return Record(h.oid, h.tid, data, prev_txn, pos)
 
-        raise StopIteration
+        raise ZODB.interfaces.StorageStopIteration()
 
 
 class Record(BaseStorage.DataRecord):

Modified: ZODB/branches/gocept-iteration/src/ZODB/FileStorage/__init__.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/FileStorage/__init__.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/FileStorage/__init__.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -2,3 +2,7 @@
 
 from ZODB.FileStorage.FileStorage import FileStorage, TransactionRecord
 from ZODB.FileStorage.FileStorage import FileIterator, Record, packed_version
+
+
+# BBB Alias for compatibility
+RecordIterator = TransactionRecord

Modified: ZODB/branches/gocept-iteration/src/ZODB/interfaces.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/interfaces.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/interfaces.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -286,6 +286,7 @@
         begins or until the connection os reopned.
         """
 
+
 class IStorageDB(Interface):
     """Database interface exposed to storages
 
@@ -418,6 +419,7 @@
         should also close all the Connections.
         """
 
+
 class IStorage(Interface):
     """A storage is responsible for storing and retrieving data of objects.
     """
@@ -710,6 +712,7 @@
 
         """
 
+
 class IStorageRestoreable(IStorage):
     """Copying Transactions
 
@@ -775,6 +778,7 @@
         Nothing is returned.
         """
 
+
 class IStorageRecordInformation(Interface):
     """Provide information about a single storage record
     """
@@ -938,6 +942,7 @@
         
         """
 
+
 class IBlob(Interface):
     """A BLOB supports efficient handling of large data within ZODB."""
 
@@ -992,5 +997,12 @@
         If Blobs use this, then commits can be performed with a simple rename.
         """
 
+
 class BlobError(Exception):
     pass
+
+
+class StorageStopIteration(IndexError, StopIteration):
+    """A combination of StopIteration and IndexError to provide a
+    backwards-compatible exception.
+    """

Modified: ZODB/branches/gocept-iteration/src/ZODB/tests/IteratorStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/tests/IteratorStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/tests/IteratorStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -15,6 +15,7 @@
 
 Any storage that supports the iterator() method should be able to pass
 all these tests.
+
 """
 
 from ZODB.tests.MinPO import MinPO
@@ -23,13 +24,16 @@
 
 from transaction import Transaction
 
+import itertools
+
+
 class IteratorCompare:
 
     def iter_verify(self, txniter, revids, val0):
         eq = self.assertEqual
         oid = self._oid
         val = val0
-        for reciter, revid in zip(txniter, revids + [None]):
+        for reciter, revid in itertools.izip(txniter, revids + [None]):
             eq(reciter.tid, revid)
             for rec in reciter:
                 eq(rec.oid, oid)
@@ -37,10 +41,8 @@
                 eq(zodb_unpickle(rec.data), MinPO(val))
                 val = val + 1
         eq(val, val0 + len(revids))
-        if hasattr(txniter, 'close'):
-            # XXX See bug #191573
-            txniter.close()
 
+
 class IteratorStorage(IteratorCompare):
 
     def checkSimpleIteration(self):
@@ -53,15 +55,6 @@
         txniter = self._storage.iterator()
         self.iter_verify(txniter, [revid1, revid2, revid3], 11)
 
-    def checkClose(self):
-        self._oid = oid = self._storage.new_oid()
-        revid1 = self._dostore(oid, data=MinPO(11))
-        txniter = self._storage.iterator()
-        if hasattr(txniter, 'close'):
-            # XXX See bug #191573
-            txniter.close()
-            self.assertRaises(IOError, txniter.next)
-
     def checkUndoZombie(self):
         oid = self._storage.new_oid()
         revid = self._dostore(oid, data=MinPO(94))
@@ -202,30 +195,36 @@
         txniter = self._storage.iterator(revid3, revid3)
         self.iter_verify(txniter, [revid3], 13)
 
+
 class IteratorDeepCompare:
+
     def compare(self, storage1, storage2):
         eq = self.assertEqual
         iter1 = storage1.iterator()
         iter2 = storage2.iterator()
-        for txn1, txn2 in zip(iter1, iter2):
+        for txn1, txn2 in itertools.izip(iter1, iter2):
             eq(txn1.tid,         txn2.tid)
             eq(txn1.status,      txn2.status)
             eq(txn1.user,        txn2.user)
             eq(txn1.description, txn2.description)
             eq(txn1.extension,  txn2.extension)
-            for rec1, rec2 in zip(txn1, txn2):
+            itxn1 = iter(txn1)
+            itxn2 = iter(txn2)
+            for rec1, rec2 in itertools.izip(itxn1, itxn2):
                 eq(rec1.oid,     rec2.oid)
                 eq(rec1.tid,  rec2.tid)
                 eq(rec1.data,    rec2.data)
             # Make sure there are no more records left in rec1 and rec2,
             # meaning they were the same length.
-            self.assertRaises(StopIteration, txn1.next)
-            self.assertRaises(StopIteration, txn2.next)
+            # Additionally, check that we're backwards compatible to the
+            # IndexError we used to raise before.
+            self.assertRaises(IndexError, itxn1.next)
+            self.assertRaises(IndexError, itxn2.next)
+            self.assertRaises(StopIteration, itxn1.next)
+            self.assertRaises(StopIteration, itxn2.next)
         # Make sure ther are no more records left in txn1 and txn2, meaning
         # they were the same length
+        self.assertRaises(IndexError, iter1.next)
+        self.assertRaises(IndexError, iter2.next)
         self.assertRaises(StopIteration, iter1.next)
         self.assertRaises(StopIteration, iter2.next)
-        if hasattr(iter1, 'close'):
-            iter1.close()
-        if hasattr(iter2, 'close'):
-            iter2.close()

Modified: ZODB/branches/gocept-iteration/src/ZODB/tests/PackableStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/tests/PackableStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/tests/PackableStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -30,6 +30,7 @@
 from persistent import Persistent
 from persistent.mapping import PersistentMapping
 import transaction
+import ZODB.interfaces
 from ZODB import DB
 from ZODB.serialize import referencesf
 from ZODB.tests.MinPO import MinPO
@@ -149,7 +150,7 @@
 
     def _sanity_check(self):
         # Iterate over the storage to make sure it's sane.
-        if not hasattr(self._storage, "iterator"):
+        if not ZODB.interfaces.IStorageIteration.providedBy(self._storage):
             return
         it = self._storage.iterator()
         for txn in it:

Modified: ZODB/branches/gocept-iteration/src/ZODB/tests/RecoveryStorage.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/tests/RecoveryStorage.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/tests/RecoveryStorage.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -22,6 +22,7 @@
 
 import time
 
+
 class RecoveryStorage(IteratorDeepCompare):
     # Requires a setUp() that creates a self._dst destination storage
     def checkSimpleRecovery(self):
@@ -49,12 +50,16 @@
         # copy the final transaction manually.  even though there
         # was a pack, the restore() ought to succeed.
         it = self._storage.iterator()
-        final = list(it)[-1]
+        # Get the last transaction and its record iterator. Record iterators
+        # can't be accessed out-of-order, so we need to do this in a bit
+        # complicated way:
+        for final  in it:
+            records = list(final)
+
         self._dst.tpc_begin(final, final.tid, final.status)
-        for r in final:
+        for r in records:
             self._dst.restore(r.oid, r.tid, r.data, '', r.data_txn,
                               final)
-        it.close()
         self._dst.tpc_vote(final)
         self._dst.tpc_finish(final)
 

Modified: ZODB/branches/gocept-iteration/src/ZODB/utils.py
===================================================================
--- ZODB/branches/gocept-iteration/src/ZODB/utils.py	2008-08-26 16:01:45 UTC (rev 90351)
+++ ZODB/branches/gocept-iteration/src/ZODB/utils.py	2008-08-26 16:05:55 UTC (rev 90352)
@@ -295,5 +295,3 @@
     handle, filename = mkstemp(dir=dir)
     os.close(handle)
     return filename
-
-



More information about the Zodb-checkins mailing list