Avoid raising execeptions during tpc_finish

Commit (send) email from tpc_vote rather than tpc_finish.  The
documentation for ``transaction`` says that tpc_finish should never
fail.  Data managers which really have only "one-phase-commit" semantics
(and whose commit operation may fail) should do the deed in
tpc_vote.  (At least, that's what zope.sqlalchemy does.)

Refs:
  https://zodb.readthedocs.org/en/latest/transactions.html#tpc-finish

CHANGES.rst

@@ -12,6 +12,13 @@ Unreleased
   failed-commit situations (masking the original exception
   responsible for the failed transaction.)
 
+- ``MailDataManger`` now sends mail from ``tpc_vote`` rather than
+  ``tpc_finish``.  According to the `transaction documentation`_,
+  ``tpc_finish`` should never fail.
+
+.. _transaction documentation:
+   https://zodb.readthedocs.org/en/latest/transactions.html#tpc-finish
+
 4.2 (2014-02-17)
 ----------------
 

repoze/sendmail/delivery.py

@@ -119,7 +119,10 @@ def abort(self, trans):
         self._do_abort()
 
     def sortKey(self):
-        return str(id(self))
+        # Currently, we only support "one-phase-commit" semantics.
+        # Try to sort last, so as to give all other data manager a
+        # chance to vote before email is sent.
+        return "~~~repoze.sendmail:%d" % id(self)
 
     def savepoint(self):
         """Create a custom `MailDataSavepoint` object
@@ -150,6 +153,7 @@ def tpc_vote(self, trans):
         if self.tpc_phase != 1:
             raise ValueError("TPC phase error: %d" % self.tpc_phase)
         self.tpc_phase = 2
+        self.callable(*self.args)
 
     def tpc_finish(self, trans):
         if self.transaction is None:
@@ -158,7 +162,6 @@ def tpc_finish(self, trans):
             raise ValueError("In a different transaction")
         if self.tpc_phase != 2:
             raise ValueError("TPC phase error: %d" % self.tpc_phase)
-        self.callable(*self.args)
         self._finish(MailDataManagerState.TPC_FINISHED)
 
     def tpc_abort(self, trans):

repoze/sendmail/tests/test_delivery.py

@@ -142,7 +142,10 @@ def _onAbort():
 
     def test_sortKey(self):
         mdm = self._makeOne()
-        self.assertEqual(mdm.sortKey(), str(id(mdm)))
+        sort_key = mdm.sortKey()
+        self.assertTrue(sort_key > 'z')
+        self.assertTrue(sort_key > '42')
+        self.assertTrue(sort_key > '~sqlalchemy:42')
 
     def test_savepoint_wo_transaction(self):
         mdm = self._makeOne()
@@ -207,12 +210,16 @@ def test_tpc_vote_not_already_tpc(self):
         self.assertRaises(ValueError, mdm.tpc_vote, txn)
 
     def test_tpc_vote_ok(self):
-        mdm = self._makeOne()
+        _called = []
+        def _callable(*args):
+            _called.append(args)
+        mdm = self._makeOne(_callable, (1, 2))
         txn = DummyTransaction()
         mdm.join_transaction(txn)
         mdm.tpc_phase = 1
         mdm.tpc_vote(txn)
         self.assertEqual(mdm.tpc_phase, 2)
+        self.assertEqual(_called, [(1, 2)])
 
     def test_tpc_finish_wo_transaction(self):
         mdm = self._makeOne()
@@ -249,7 +256,7 @@ def _callable(*args):
         mdm.join_transaction(txn)
         mdm.tpc_phase = 2
         mdm.tpc_finish(txn)
-        self.assertEqual(_called, [(1, 2)])
+        self.assertEqual(_called, [])
         self.assertEqual(mdm.state, MailDataManagerState.TPC_FINISHED)
 
     def test_tpc_abort_wo_transaction(self):