Collector 1822.

Make undo{Log,Info} arguments act like Python slice indices when
both are non-negative.  The code used to do that before ZODB 3.4a9,
but changed to match ZODB's UML documentation.  Alas, some
(untested) code in Zope relied on the actual behavior (see the
collector report).  Changed code, docs, and tests to bless the
old behavior in these cases.

DemoStorage.UndoLog:  this was wrong in several ways.  I'm still
unsure about why it skips "packed" transactions.  That doesn't seem
right, but I don't have time to wonder about that now.

NEWS.txt

-What's new in ZODB3 3.4.1a2?
+What's new in ZODB3 3.4.1a3?
 ============================
 Release date: DD-MMM-2005
 
 Following are dates of internal releases (to support ongoing Zope 2
 development) since ZODB 3.4's last public release:
 
+- 3.4.1a2 29-Jun-2005
 - 3.4.1a1 27-Jun-2005
 
 Savepoints
@@ -18,11 +19,21 @@ Savepoints
   could cause spurious errors if a transaction with a pending savepoint
   needed to fetch an older revision of some object.
 
-FileStorage.UndoSearch
-----------------------
+FileStorage
+-----------
 
-- (3.4.1a1) The ``_readnext()`` method now returns the transaction size as
-  the value of the "size" key.  Thanks to Dieter Maurer for the patch, from
+- (3.4.1a2) Collector #1822.  The ``undoLog()`` and ``undoInfo()`` methods
+  were changed in 3.4a9 to return the documented results.  Alas, some pieces
+  of (non-ZODB) code relied on the actual behavior.  When the `first` and
+  `last` arguments are both >= 0, these methods now treat them as if they
+  were Python slice indices, including the `first` index but excluding the
+  `last` index.  This matches former behavior, although it contradicts older
+  ZODB UML documentation.  The documentation in
+  ``ZODB.interfaces.IStorageUndoable`` was changed to match the new intent.
+
+- (3.4.1a1) The ``UndoSearch._readnext()`` method now returns the transaction
+  size as the value of the "size" key.  Thanks to Dieter Maurer for the
+  patch, from
   http://mail.zope.org/pipermail/zodb-dev/2003-October/006157.html. "This is
   very valuable when you want to spot strange transaction sizes via Zope's
   'Undo' tab".
@@ -39,6 +50,11 @@ ThreadedAsync.LoopCallback
   example, debugging prints added to Python's ``asyncore.loop`` won't be
   lost anymore).
 
+DemoStorage
+-----------
+
+- The implementation of ``undoLog()`` was wrong in several ways; repaired.
+
 
 What's new in ZODB3 3.4?
 ========================

src/ZODB/DemoStorage.py

@@ -337,21 +337,19 @@ class DemoStorage(BaseStorage):
         self._ltid = self._tid
 
     def undoLog(self, first, last, filter=None):
-        if last < 0:
-            last = first - last + 1
+        if last < 0:  # abs(last) is an upper bound on the # to return
+            last = first - last
         self._lock_acquire()
         try:
-            # Unsure:  shouldn we sort this?
             transactions = self._data.items()
             pos = len(transactions)
             r = []
             i = 0
             while i < last and pos:
-                pos = pos - 1
-                if i < first:
-                    i = i + 1
-                    continue
+                pos -= 1
                 tid, (p, u, d, e, t) = transactions[pos]
+                # Bug alert:  why do we skip this if the transaction
+                # has been packed?
                 if p:
                     continue
                 d = {'id': base64.encodestring(tid)[:-1],
@@ -359,10 +357,10 @@ class DemoStorage(BaseStorage):
                      'user_name': u, 'description': d}
                 if e:
                     d.update(loads(e))
-
                 if filter is None or filter(d):
+                    if i >= first:
                         r.append(d)
-                    i = i + 1
+                    i += 1
             return r
         finally:
             self._lock_release()

src/ZODB/FileStorage/FileStorage.py

@@ -1070,11 +1070,11 @@ class FileStorage(BaseStorage.BaseStorage,
     def undoLog(self, first=0, last=-20, filter=None):
         if last < 0:
             # -last is supposed to be the max # of transactions.  Convert to
-            # a positive index.  Should have x - first + 1 = -last, which
-            # means x = first - last - 1.  This is spelled out here because
+            # a positive index.  Should have x - first = -last, which
+            # means x = first - last.  This is spelled out here because
             # the normalization code was incorrect for years (used +1
-            # instead -- off by 2), until ZODB 3.4.
-            last = first - last - 1
+            # instead -- off by 1), until ZODB 3.4.
+            last = first - last
         self._lock_acquire()
         try:
             if self._pack_is_in_progress:
@@ -2043,7 +2043,7 @@ class UndoSearch:
         self.filter = filter
         # self.i is the index of the transaction we're _going_ to find
         # next.  When it reaches self.first, we should start appending
-        # to self.results.  When it reaches self.last + 1, we're done
+        # to self.results.  When it reaches self.last, we're done
         # (although we may finish earlier).
         self.i = 0
         self.results = []
@@ -2052,7 +2052,7 @@ class UndoSearch:
     def finished(self):
         """Return True if UndoSearch has found enough records."""
         # BAW: Why 39 please?  This makes no sense (see also below).
-        return self.i > self.last or self.pos < 39 or self.stop
+        return self.i >= self.last or self.pos < 39 or self.stop
 
     def search(self):
         """Search for another record."""

src/ZODB/UndoLogCompatible.py

@@ -18,12 +18,20 @@ class UndoLogCompatible:
 
     def undoInfo(self, first=0, last=-20, specification=None):
         if specification:
+            # filter(desc) returns true iff `desc` is a "superdict"
+            # of `specification`, meaning that `desc` contains the same
+            # (key, value) pairs as `specification`, and possibly additional
+            # (key, value) pairs.  Another way to do this might be
+            #    d = desc.copy()
+            #    d.update(specification)
+            #    return d == desc
             def filter(desc, spec=specification.items()):
-                get=desc.get
+                get = desc.get
                 for k, v in spec:
                     if get(k, None) != v:
                         return 0
                 return 1
-        else: filter=None
+        else:
+            filter = None
 
         return self.undoLog(first, last, filter)

src/ZODB/interfaces.py

@@ -490,13 +490,15 @@ class IStorageUndoable(IStorage):
 
             `first`:  This is the index of the first transaction description
                       in the slice.  It must be >= 0.
-            `last`:  If >= 0, this is the index of the last transaction
-                     description in the slice, and `last` should be at least
-                     as large as `first` in this case.  If `last` is less than
-                     0, then abs(last) is taken to be the maximum number
-                     of descriptions in the slice (which still begins at
-                     index `first`).  When `last` < 0, the same effect could
-                     be gotten by passing the positive first-last-1 for
+            `last`:  If >= 0, first:last acts like a Python slice, selecting
+                     the descriptions at indices `first`, first+1, ..., up to
+                     but not including index `last`.  At most last-first
+                     descriptions are in the slice, and `last` should be at
+                     least as large as `first` in this case.  If `last` is
+                     less than 0, then abs(last) is taken to be the maximum
+                     number of descriptions in the slice (which still begins
+                     at index `first`).  When `last` < 0, the same effect
+                     could be gotten by passing the positive first-last for
                      `last` instead.
         """
 

src/ZODB/tests/TransactionalUndoStorage.py

@@ -755,7 +755,7 @@ class TransactionalUndoStorage:
         self.assertEqual(default, allofem[:20])
 
         # If we ask for only one, we should get only the most recent.
-        fresh = info_func(last=0)
+        fresh = info_func(last=1)
         self.assertEqual(len(fresh), 1)
         self.assertEqual(fresh[0], allofem[0])
 
@@ -765,11 +765,11 @@ class TransactionalUndoStorage:
 
         # Try a slice that doesn't start at 0.
         oddball = info_func(first=11, last=17)
-        self.assertEqual(len(oddball), 17-11+1)
+        self.assertEqual(len(oddball), 17-11)
         self.assertEqual(oddball, allofem[11 : 11+len(oddball)])
 
         # And another way to spell the same thing.
-        redundant = info_func(first=11, last=-7)
+        redundant = info_func(first=11, last=-6)
         self.assertEqual(oddball, redundant)
 
         cn.close()