Merge anguenot-ordering-beforecommitsubscribers branch.

addBeforeCommitHook() is new, a generalization of the
now-deprecated beforeCommitHook() that allows influencing
the order in which commit hooks are called.

src/transaction/_transaction.py

@@ -105,13 +105,13 @@ commit will start with a commit_sub() call instead of a tpc_begin()
 call.
 
 Before-commit hook
----------------
+------------------
 
 Sometimes, applications want to execute some code when a transaction is
 committed.  For example, one might want to delay object indexing until a
 transaction commits, rather than indexing every time an object is changed.
 Or someone might want to check invariants only after a set of operations.  A
-pre-commit hook is available for such use cases, just use beforeCommitHook()
+pre-commit hook is available for such use cases:  use addBeforeCommitHook(),
 passing it a callable and arguments.  The callable will be called with its
 arguments at the start of the commit (but not for substransaction commits).
 
@@ -146,6 +146,7 @@ committed or aborted.  The methods are passed the current Transaction
 as their only argument.
 """
 
+import bisect
 import logging
 import sys
 import thread
@@ -238,10 +239,16 @@ class Transaction(object):
         # raised, incorporating this traceback.
         self._failure_traceback = None
 
-        # Holds (hook, args, kws) triples added by beforeCommitHook.
+        # 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.
         self._before_commit = []
+        self._before_commit_index = 0
 
     # Raise TransactionFailedError, due to commit()/join()/register()
     # getting called when the current transaction has already suffered
@@ -408,17 +415,34 @@ class Transaction(object):
         raise t, v, tb
 
     def getBeforeCommitHooks(self):
-        return iter(self._before_commit)
+        # 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")
+        if kws is None:
+            kws = {}
+        bisect.insort(self._before_commit, (order, self._before_commit_index,
+                                            hook, tuple(args), kws))
+        self._before_commit_index += 1
 
     def beforeCommitHook(self, hook, *args, **kws):
-        self._before_commit.append((hook, args, kws))
+        from ZODB.utils import deprecated37
+
+        deprecated37("Use addBeforeCommitHook instead of beforeCommitHook.")
+        # Default order is zero.
+        self.addBeforeCommitHook(hook, args, kws, order=0)
 
     def _callBeforeCommitHooks(self):
         # Call all hooks registered, allowing further registrations
         # during processing.
         while self._before_commit:
-            hook, args, kws = self._before_commit.pop(0)
+            order, index, hook, args, kws = self._before_commit.pop(0)
             hook(*args, **kws)
+        self._before_commit_index = 0
 
     def _commitResources(self):
         # Execute the two-phase commit protocol.

src/transaction/interfaces.py

@@ -167,9 +167,12 @@ class ITransaction(zope.interface.Interface):
         raise an exception, or remove `<name, value>` pairs).
         """
 
-    def beforeCommitHook(hook, *args, **kws):
+    # deprecated37
+    def beforeCommitHook(__hook, *args, **kws):
         """Register a hook to call before the transaction is committed.
 
+        THIS IS DEPRECATED IN ZODB 3.5.  Use addBeforeCommitHook() instead.
+
         The specified hook function will be called after the transaction's
         commit method has been called, but before the commit process has been
         started.  The hook will be passed the specified positional and keyword
@@ -192,11 +195,51 @@ class ITransaction(zope.interface.Interface):
         instead.
         """
 
+    def addBeforeCommitHook(hook, args=(), kws=None, order=0):
+        """Register a hook to call before the transaction is committed.
+
+        The specified hook function will be called after the transaction's
+        commit method has been called, but before the commit process has been
+        started.  The hook will be passed the specified positional (`args`)
+        and keyword (`kws`) arguments.  `args` is a sequence of positional
+        arguments to be passed, defaulting to an empty tuple (no positional
+        arguments are passed).  `kws` is a dictionary of keyword argument
+        names and values to be passed, or the default None (no keyword
+        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.
+
+        Hooks are called only for a top-level commit.  A subtransaction
+        commit or savepoint creation does not call any hooks.  If the
+        transaction is aborted, hooks are not called, and are discarded.
+        Calling a hook "consumes" its registration too:  hook registrations
+        do not persist across transactions.  If it's desired to call the same
+        hook on every transaction commit, then addBeforeCommitHook() must be
+        called with that hook during every transaction; in such a case
+        consider registering a synchronizer object via a TransactionManager's
+        registerSynch() method instead.
+        """
+
     def getBeforeCommitHooks():
-        """Return iterable producing the registered beforeCommit hooks.
+        """Return iterable producing the registered addBeforeCommit hooks.
 
         A triple (hook, args, kws) is produced for each registered hook.
-        The hooks are produced in the order in which they were registered.
+        The hooks are produced in the order in which they would be invoked
+        by a top-level transaction commit.
         """
 
 class ITransactionDeprecated(zope.interface.Interface):

src/transaction/tests/test_transaction.py

@@ -44,6 +44,7 @@ import warnings
 
 import transaction
 from ZODB.utils import positive_id
+from ZODB.tests.warnhook import WarningsHook
 
 # deprecated37  remove when subtransactions go away
 # Don't complain about subtxns in these tests.
@@ -410,8 +411,34 @@ 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())
+
+# deprecated37; remove this then
 def test_beforeCommitHook():
-    """Test the beforeCommitHook.
+    """Test beforeCommitHook.
 
     Let's define a hook to call, and a way to see that it was called.
 
@@ -422,12 +449,37 @@ def test_beforeCommitHook():
       >>> def hook(arg='no_arg', kw1='no_kw1', kw2='no_kw2'):
       ...     log.append("arg %r kw1 %r kw2 %r" % (arg, kw1, kw2))
 
+    beforeCommitHook is deprecated, so we need cruft to suppress the
+    warnings.
+
+      >>> whook = WarningsHook()
+      >>> whook.install()
+
+    Fool the warnings module into delivering the warnings despite that
+    they've been seen before; this is needed in case this test is run
+    more than once.
+
+      >>> import warnings
+      >>> warnings.filterwarnings("always", category=DeprecationWarning)
+
     Now register the hook with a transaction.
 
       >>> import transaction
       >>> t = transaction.begin()
       >>> t.beforeCommitHook(hook, '1')
 
+    Make sure it triggered a deprecation warning:
+
+      >>> len(whook.warnings)
+      1
+      >>> message, category, filename, lineno = whook.warnings[0]
+      >>> print message
+      This will be removed in ZODB 3.7:
+      Use addBeforeCommitHook instead of beforeCommitHook.
+      >>> category.__name__
+      'DeprecationWarning'
+      >>> whook.clear()
+
     We can see that the hook is indeed registered.
 
       >>> [(hook.func_name, args, kws)
@@ -548,6 +600,236 @@ def test_beforeCommitHook():
                "arg '-' kw1 'no_kw1' kw2 'no_kw2'",
        'rec0']
       >>> reset_log()
+
+    We have to uninstall the warnings hook so that other warnings don't get
+    lost.
+
+      >>> whook.uninstall()
+
+    Obscure:  There is no API call for removing the filter we added, but
+    filters appears to be a public variable.
+
+      >>> del warnings.filters[0]
+    """
+
+def test_addBeforeCommitHook():
+    """Test addBeforeCommitHook, without order arguments.
+
+    Let's define a hook to call, and a way to see that it was called.
+
+      >>> log = []
+      >>> def reset_log():
+      ...     del log[:]
+
+      >>> def hook(arg='no_arg', kw1='no_kw1', kw2='no_kw2'):
+      ...     log.append("arg %r kw1 %r kw2 %r" % (arg, kw1, kw2))
+
+    Now register the hook with a transaction.
+
+      >>> import transaction
+      >>> t = transaction.begin()
+      >>> t.addBeforeCommitHook(hook, '1')
+
+    We can see that the hook is indeed registered.
+
+      >>> [(hook.func_name, args, kws)
+      ...  for hook, args, kws in t.getBeforeCommitHooks()]
+      [('hook', ('1',), {})]
+
+    When transaction commit starts, the hook is called, with its
+    arguments.
+
+      >>> log
+      []
+      >>> t.commit()
+      >>> log
+      ["arg '1' kw1 'no_kw1' kw2 'no_kw2'"]
+      >>> reset_log()
+
+    A hook's registration is consumed whenever the hook is called.  Since
+    the hook above was called, it's no longer registered:
+
+      >>> len(list(t.getBeforeCommitHooks()))
+      0
+      >>> transaction.commit()
+      >>> log
+      []
+
+    The hook is only called for a full commit, not for a savepoint or
+    subtransaction.
+
+      >>> t = transaction.begin()
+      >>> t.addBeforeCommitHook(hook, 'A', dict(kw1='B'))
+      >>> dummy = t.savepoint()
+      >>> log
+      []
+      >>> t.commit(subtransaction=True)
+      >>> log
+      []
+      >>> t.commit()
+      >>> log
+      ["arg 'A' kw1 'B' kw2 'no_kw2'"]
+      >>> reset_log()
+
+    If a transaction is aborted, no hook is called.
+
+      >>> t = transaction.begin()
+      >>> t.addBeforeCommitHook(hook, ["OOPS!"])
+      >>> transaction.abort()
+      >>> log
+      []
+      >>> transaction.commit()
+      >>> log
+      []
+
+    The hook is called before the commit does anything, so even if the
+    commit fails the hook will have been called.  To provoke failures in
+    commit, we'll add failing resource manager to the transaction.
+
+      >>> class CommitFailure(Exception):
+      ...     pass
+      >>> class FailingDataManager:
+      ...     def tpc_begin(self, txn, sub=False):
+      ...         raise CommitFailure
+      ...     def abort(self, txn):
+      ...         pass
+
+      >>> t = transaction.begin()
+      >>> t.join(FailingDataManager())
+
+      >>> t.addBeforeCommitHook(hook, '2')
+      >>> t.commit()
+      Traceback (most recent call last):
+      ...
+      CommitFailure
+      >>> log
+      ["arg '2' kw1 'no_kw1' kw2 'no_kw2'"]
+      >>> reset_log()
+
+    Let's register several hooks.
+
+      >>> t = transaction.begin()
+      >>> t.addBeforeCommitHook(hook, '4', dict(kw1='4.1'))
+      >>> t.addBeforeCommitHook(hook, '5', dict(kw2='5.2'))
+
+    They are returned in the same order by getBeforeCommitHooks.
+
+      >>> [(hook.func_name, args, kws)     #doctest: +NORMALIZE_WHITESPACE
+      ...  for hook, args, kws in t.getBeforeCommitHooks()]
+      [('hook', ('4',), {'kw1': '4.1'}),
+       ('hook', ('5',), {'kw2': '5.2'})]
+
+    And commit also calls them in this order.
+
+      >>> t.commit()
+      >>> len(log)
+      2
+      >>> log  #doctest: +NORMALIZE_WHITESPACE
+      ["arg '4' kw1 '4.1' kw2 'no_kw2'",
+       "arg '5' kw1 'no_kw1' kw2 '5.2'"]
+      >>> reset_log()
+
+    While executing, a hook can itself add more hooks, and they will all
+    be called before the real commit starts.
+
+      >>> def recurse(txn, arg):
+      ...     log.append('rec' + str(arg))
+      ...     if arg:
+      ...         txn.addBeforeCommitHook(hook, '-')
+      ...         txn.addBeforeCommitHook(recurse, (txn, arg-1))
+
+      >>> t = transaction.begin()
+      >>> t.addBeforeCommitHook(recurse, (t, 3))
+      >>> transaction.commit()
+      >>> log  #doctest: +NORMALIZE_WHITESPACE
+      ['rec3',
+               "arg '-' kw1 'no_kw1' kw2 'no_kw2'",
+       'rec2',
+               "arg '-' kw1 'no_kw1' kw2 'no_kw2'",
+       'rec1',
+               "arg '-' kw1 'no_kw1' kw2 'no_kw2'",
+       'rec0']
+      >>> 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():
@@ -555,8 +837,8 @@ def test_suite():
     return unittest.TestSuite((
         DocTestSuite(),
         unittest.makeSuite(TransactionTests),
+        unittest.makeSuite(BeforeCommitHookTests),
         ))
 
-
 if __name__ == '__main__':
     unittest.TextTestRunner().run(test_suite())