[Zodb-checkins] SVN: ZODB/branches/3.4/ Collector 1822.

Tim Peters tim.one at comcast.net
Wed Jun 29 16:10:47 EDT 2005


Log message for revision 30944:
  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.
  

Changed:
  U   ZODB/branches/3.4/NEWS.txt
  U   ZODB/branches/3.4/src/ZODB/DemoStorage.py
  U   ZODB/branches/3.4/src/ZODB/FileStorage/FileStorage.py
  U   ZODB/branches/3.4/src/ZODB/UndoLogCompatible.py
  U   ZODB/branches/3.4/src/ZODB/interfaces.py
  U   ZODB/branches/3.4/src/ZODB/tests/TransactionalUndoStorage.py

-=-
Modified: ZODB/branches/3.4/NEWS.txt
===================================================================
--- ZODB/branches/3.4/NEWS.txt	2005-06-29 20:08:45 UTC (rev 30943)
+++ ZODB/branches/3.4/NEWS.txt	2005-06-29 20:10:47 UTC (rev 30944)
@@ -1,10 +1,11 @@
-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 @@
   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,7 +50,12 @@
   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?
 ========================
 Release date: 09-Jun-2005

Modified: ZODB/branches/3.4/src/ZODB/DemoStorage.py
===================================================================
--- ZODB/branches/3.4/src/ZODB/DemoStorage.py	2005-06-29 20:08:45 UTC (rev 30943)
+++ ZODB/branches/3.4/src/ZODB/DemoStorage.py	2005-06-29 20:10:47 UTC (rev 30944)
@@ -337,21 +337,19 @@
         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 @@
                      'user_name': u, 'description': d}
                 if e:
                     d.update(loads(e))
-
                 if filter is None or filter(d):
-                    r.append(d)
-                    i = i + 1
+                    if i >= first:
+                        r.append(d)
+                    i += 1
             return r
         finally:
             self._lock_release()

Modified: ZODB/branches/3.4/src/ZODB/FileStorage/FileStorage.py
===================================================================
--- ZODB/branches/3.4/src/ZODB/FileStorage/FileStorage.py	2005-06-29 20:08:45 UTC (rev 30943)
+++ ZODB/branches/3.4/src/ZODB/FileStorage/FileStorage.py	2005-06-29 20:10:47 UTC (rev 30944)
@@ -1070,11 +1070,11 @@
     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 @@
         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 @@
     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."""

Modified: ZODB/branches/3.4/src/ZODB/UndoLogCompatible.py
===================================================================
--- ZODB/branches/3.4/src/ZODB/UndoLogCompatible.py	2005-06-29 20:08:45 UTC (rev 30943)
+++ ZODB/branches/3.4/src/ZODB/UndoLogCompatible.py	2005-06-29 20:10:47 UTC (rev 30944)
@@ -18,12 +18,20 @@
 
     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)

Modified: ZODB/branches/3.4/src/ZODB/interfaces.py
===================================================================
--- ZODB/branches/3.4/src/ZODB/interfaces.py	2005-06-29 20:08:45 UTC (rev 30943)
+++ ZODB/branches/3.4/src/ZODB/interfaces.py	2005-06-29 20:10:47 UTC (rev 30944)
@@ -490,13 +490,15 @@
 
             `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.
         """
 

Modified: ZODB/branches/3.4/src/ZODB/tests/TransactionalUndoStorage.py
===================================================================
--- ZODB/branches/3.4/src/ZODB/tests/TransactionalUndoStorage.py	2005-06-29 20:08:45 UTC (rev 30943)
+++ ZODB/branches/3.4/src/ZODB/tests/TransactionalUndoStorage.py	2005-06-29 20:10:47 UTC (rev 30944)
@@ -755,7 +755,7 @@
         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 @@
 
         # 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()



More information about the Zodb-checkins mailing list