In accord with the endless debate on zodb-dev, remove the ``order=``

argument from addBeforeCommitHook().

NEWS.txt

@@ -10,12 +10,10 @@ Zope3 development).  These are the dates of the internal releases:
 Commit hooks
 ------------
 
-- (3.6a1) A new ``addBeforeCommitHook()`` method of transactions generalizes
-  the older ``beforeCommitHook()`` method, with a more-robust signature
-  and an optional new ``order`` argument to influence the order in which
-  commit hooks are invoked.  ``beforeCommitHook()`` is now deprecated, and
-  will be removed in ZODB 3.7.  Thanks to Julien Anguenot for contributing
-  code and tests.
+- (3.6a1) The ``beforeCommitHook()`` method has been replaced by the new
+  ``addBeforeCommitHook()`` method, with a more-robust signature.
+  ``beforeCommitHook()`` is now deprecated, and will be removed in ZODB 3.8.
+  Thanks to Julien Anguenot for contributing code and tests.
 
 PersistentMapping
 -----------------

src/transaction/_transaction.py

@@ -146,7 +146,6 @@ committed or aborted.  The methods are passed the current Transaction
 as their only argument.
 """
 
-import bisect
 import logging
 import sys
 import thread
@@ -239,16 +238,8 @@ class Transaction(object):
         # raised, incorporating this traceback.
         self._failure_traceback = None
 
-        # List of (order, index, hook, args, kws) tuples added by
-        # addbeforeCommitHook().  `index` is used to resolve ties on equal
-        # `order` values, preserving the order in which the hooks were
-        # registered.  Each time we append a tuple to _before_commit,
-        # the current value of _before_commit_index is used for the
-        # index, and then the latter is incremented by 1.
-        # TODO: in Python 2.4, change to collections.deque; lists can be
-        # inefficient for FIFO access of this kind.
+        # List of (hook, args, kws) tuples added by addbeforeCommitHook().
         self._before_commit = []
-        self._before_commit_index = 0
 
     # Raise TransactionFailedError, due to commit()/join()/register()
     # getting called when the current transaction has already suffered
@@ -415,34 +406,27 @@ class Transaction(object):
         raise t, v, tb
 
     def getBeforeCommitHooks(self):
-        # Don't return the hook order and index values because of
-        # backward compatibility, and because they're internal details.
-        return iter([x[2:] for x in self._before_commit])
-
-    def addBeforeCommitHook(self, hook, args=(), kws=None, order=0):
-        if not isinstance(order, int):
-            raise ValueError("An integer value is required "
-                             "for the order argument")
+        return iter(self._before_commit)
+
+    def addBeforeCommitHook(self, hook, args=(), kws=None):
         if kws is None:
             kws = {}
-        bisect.insort(self._before_commit, (order, self._before_commit_index,
-                                            hook, tuple(args), kws))
-        self._before_commit_index += 1
+        self._before_commit.append((hook, tuple(args), kws))
 
     def beforeCommitHook(self, hook, *args, **kws):
         from ZODB.utils import deprecated38
 
         deprecated38("Use addBeforeCommitHook instead of beforeCommitHook.")
-        # Default order is zero.
-        self.addBeforeCommitHook(hook, args, kws, order=0)
+        self.addBeforeCommitHook(hook, args, kws)
 
     def _callBeforeCommitHooks(self):
         # Call all hooks registered, allowing further registrations
-        # during processing.
-        while self._before_commit:
-            order, index, hook, args, kws = self._before_commit.pop(0)
+        # during processing.  Note that calls to addBeforeCommitHook() may
+        # add additional hooks while hooks are running, and iterating over a
+        # growing list is well-defined in Python.
+        for hook, args, kws in self._before_commit:
             hook(*args, **kws)
-        self._before_commit_index = 0
+        self._before_commit = []
 
     def _commitResources(self):
         # Execute the two-phase commit protocol.

src/transaction/interfaces.py

@@ -195,7 +195,7 @@ class ITransaction(zope.interface.Interface):
         instead.
         """
 
-    def addBeforeCommitHook(hook, args=(), kws=None, order=0):
+    def addBeforeCommitHook(hook, args=(), kws=None):
         """Register a hook to call before the transaction is committed.
 
         The specified hook function will be called after the transaction's
@@ -208,20 +208,10 @@ class ITransaction(zope.interface.Interface):
         arguments are passed).
 
         Multiple hooks can be registered and will be called in the order they
-        were registered (first registered, first called), except that
-        hooks registered with different `order` arguments are invoked from
-        smallest `order` value to largest.  `order` must be an integer,
-        and defaults to 0.
-
-        For instance, a hook registered with order=1 will be invoked after
-        another hook registered with order=-1 and before another registered
-        with order=2, regardless of which was registered first.  When two
-        hooks are registered with the same order, the first one registered is
-        called first.
-
-        This method can also be called from a hook:  an executing hook can
-        register more hooks.  Applications should take care to avoid creating
-        infinite loops by recursively registering hooks.
+        were registered (first registered, first called).  This method can
+        also be called from a hook:  an executing hook can register more
+        hooks.  Applications should take care to avoid creating infinite loops
+        by recursively registering hooks.
 
         Hooks are called only for a top-level commit.  A subtransaction
         commit or savepoint creation does not call any hooks.  If the

src/transaction/tests/test_transaction.py

@@ -414,28 +414,6 @@ def test_join():
 def hook():
     pass
 
-class BeforeCommitHookTests(unittest.TestCase):
-
-    def test_01_beforecommithook_order_exceptions(self):
-        # string
-        t = transaction.Transaction()
-        self.assertRaises(ValueError, t.addBeforeCommitHook,
-                          hook, order='string')
-
-    def test_02_beforecommithook_order_exceptions(self):
-        # float
-        t = transaction.Transaction()
-        self.assertRaises(ValueError, t.addBeforeCommitHook,
-                          hook, order=1.2)
-
-    def test_03_beforecommithook_order_exceptions(self):
-        # object
-        t = transaction.Transaction()
-        class foo:
-            pass
-        self.assertRaises(ValueError, t.addBeforeCommitHook,
-                          hook, order=foo())
-
 # deprecated38; remove this then
 def test_beforeCommitHook():
     """Test beforeCommitHook.
@@ -613,7 +591,7 @@ def test_beforeCommitHook():
     """
 
 def test_addBeforeCommitHook():
-    """Test addBeforeCommitHook, without order arguments.
+    """Test addBeforeCommitHook.
 
     Let's define a hook to call, and a way to see that it was called.
 
@@ -752,92 +730,11 @@ def test_addBeforeCommitHook():
       >>> reset_log()
     """
 
-def test_addBeforeCommitHookOrder():
-    """Test addBeforeCommitHook with order arguments.
-
-    Register a hook with an order explicitly equal to 0 (the default value):
-
-      >>> import transaction
-      >>> t = transaction.begin()
-      >>> t.addBeforeCommitHook(hook, '1', order=0)
-
-    We can see that the hook is indeed registered.
-
-      >>> [(hook.func_name, args, kws)
-      ...  for hook, args, kws in t.getBeforeCommitHooks()]
-      [('hook', ('1',), {})]
-
-    Let's add another one with a smaller order. It will be registered
-    to be called first.
-
-      >>> t.addBeforeCommitHook(hook, '2', order=-999999)
-      >>> [(hook.func_name, args, kws)
-      ...  for hook, args, kws in t.getBeforeCommitHooks()]
-      [('hook', ('2',), {}), ('hook', ('1',), {})]
-
-    Let's add another one with a bigger order.  It will be registered
-    to be called last.
-
-      >>> t.addBeforeCommitHook(hook, '3', order=999999)
-      >>> for hook, args, kws in t.getBeforeCommitHooks():
-      ...     print (hook.func_name, args, kws)
-      ('hook', ('2',), {})
-      ('hook', ('1',), {})
-      ('hook', ('3',), {})
-
-    Above, we checked that the order parameter works as expected.
-    Now check that insertion with the same order values  respects the order
-    of registration.
-
-      >>> t.addBeforeCommitHook(hook, '4') # order=0 implied
-      >>> for hook, args, kws in t.getBeforeCommitHooks():
-      ...     print (hook.func_name, args, kws)
-      ('hook', ('2',), {})
-      ('hook', ('1',), {})
-      ('hook', ('4',), {})
-      ('hook', ('3',), {})
-
-      >>> t.addBeforeCommitHook(hook, '5', order=999999)
-      >>> for hook, args, kws in t.getBeforeCommitHooks():
-      ...     print (hook.func_name, args, kws)
-      ('hook', ('2',), {})
-      ('hook', ('1',), {})
-      ('hook', ('4',), {})
-      ('hook', ('3',), {})
-      ('hook', ('5',), {})
-
-      >>> t.addBeforeCommitHook(hook, '6', order=-999999)
-      >>> for hook, args, kws in t.getBeforeCommitHooks():
-      ...     print (hook.func_name, args, kws)
-      ('hook', ('2',), {})
-      ('hook', ('6',), {})
-      ('hook', ('1',), {})
-      ('hook', ('4',), {})
-      ('hook', ('3',), {})
-      ('hook', ('5',), {})
-
-      >>> def hook2():
-      ...     pass
-
-      >>> t.addBeforeCommitHook(hook2, '8', order=0)
-      >>> for hook, args, kws in t.getBeforeCommitHooks():
-      ...     print (hook.func_name, args, kws)
-      ('hook', ('2',), {})
-      ('hook', ('6',), {})
-      ('hook', ('1',), {})
-      ('hook', ('4',), {})
-      ('hook2', ('8',), {})
-      ('hook', ('3',), {})
-      ('hook', ('5',), {})
-
-    """
-
 def test_suite():
     from zope.testing.doctest import DocTestSuite
     return unittest.TestSuite((
         DocTestSuite(),
         unittest.makeSuite(TransactionTests),
-        unittest.makeSuite(BeforeCommitHookTests),
         ))
 
 if __name__ == '__main__':