4 files changed, 53 insertions, 17 deletions

diff --git a/src/mailman_pgp/mta/bulk.py b/src/mailman_pgp/mta/bulk.py
index cd517e8..a2cc8c6 100644
--- a/src/mailman_pgp/mta/bulk.py
+++ b/src/mailman_pgp/mta/bulk.py
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
-""""""
+"""PGP enabled BulkDelivery."""
 import copy
 
 from mailman.mta.bulk import BulkDelivery
@@ -28,7 +28,7 @@ from mailman_pgp.utils.email import overwrite_message
 
 
 class CallbackBulkDelivery(BulkDelivery):
-    """"""
+    """Bulk delivery that has a list of callbacks to run for each chunk."""
 
     def __init__(self, max_recipients=None):
         super().__init__(max_recipients=max_recipients)
@@ -40,19 +40,27 @@ class CallbackBulkDelivery(BulkDelivery):
         for recipients in self.chunkify(msgdata.get('recipients', set())):
             message_copy = copy.deepcopy(msg)
             msgdata_copy = msgdata.copy()
+            recipients_copy = set(recipients)
+
             for callback in self.callbacks:
-                callback(mlist, message_copy, msgdata_copy, recipients)
+                callback(mlist, message_copy, msgdata_copy, recipients_copy)
+            callback_refused = dict(
+                    (recipient, (444, BaseException))
+                    for recipient in recipients - recipients_copy)
+            refused.update(callback_refused)
+
             chunk_refused = self._deliver_to_recipients(
-                    mlist, message_copy, msgdata_copy, recipients)
+                    mlist, message_copy, msgdata_copy, recipients_copy)
             refused.update(chunk_refused)
         return refused
 
 
 class PGPBulkMixin:
-    """"""
+    """Bulk encryption and signing Delivery mixin."""
 
     def sign_encrypt(self, mlist, msg, msgdata, recipients):
         """
+        Sign and encrypt the outgoing message to the recipients.
 
         :param mlist:
         :type mlist: mailman.model.mailinglist.MailingList
@@ -61,7 +69,7 @@ class PGPBulkMixin:
         :param msgdata:
         :type msgdata: dict
         :param recipients:
-        :type recipients: list
+        :type recipients: set
         """
         pgp_list = PGPMailingList.for_list(mlist)
         if not pgp_list:
@@ -71,13 +79,13 @@ class PGPBulkMixin:
             return
 
         keys = []
-        for recipient in recipients:
+        for recipient in set(recipients):
             pgp_address = PGPAddress.for_email(recipient)
             if pgp_address is None:
-                # TODO: log failure here
+                recipients.remove(recipient)
                 continue
             if pgp_address.key is None or not pgp_address.key_confirmed:
-                # TODO: log failure here?
+                recipients.remove(recipient)
                 continue
             keys.append(pgp_address.key)
 
@@ -96,7 +104,7 @@ class PGPBulkMixin:
 
 @public
 class PGPBulkDelivery(CallbackBulkDelivery, PGPBulkMixin):
-    """"""
+    """Bulk PGP enabled delivery."""
 
     def __init__(self, max_recipients=None):
         super().__init__(max_recipients=max_recipients)
diff --git a/src/mailman_pgp/mta/personalized.py b/src/mailman_pgp/mta/personalized.py
index c2c9a15..a89301f 100644
--- a/src/mailman_pgp/mta/personalized.py
+++ b/src/mailman_pgp/mta/personalized.py
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
-""""""
+"""PGP enabled IndividualDelivery."""
 from mailman.mta.base import IndividualDelivery
 from mailman.mta.decorating import DecoratingMixin
 from mailman.mta.personalized import PersonalizedMixin
@@ -29,10 +29,11 @@ from mailman_pgp.utils.email import overwrite_message
 
 
 class PGPIndividualMixin:
-    """"""
+    """Individual encryption and signing Delivery mixin."""
 
     def sign_encrypt(self, mlist, msg, msgdata):
         """
+        Sign and encrypt the outgoing message to the recipient.
 
         :param mlist:
         :type mlist: mailman.model.mailinglist.MailingList
@@ -74,7 +75,7 @@ class PGPIndividualMixin:
 @public
 class PGPPersonalizedDelivery(IndividualDelivery, VERPMixin, DecoratingMixin,
                               PersonalizedMixin, PGPIndividualMixin):
-    """"""
+    """Individual PGP enabled delivery."""
 
     def __init__(self):
         super().__init__()
diff --git a/src/mailman_pgp/mta/tests/test_bulk.py b/src/mailman_pgp/mta/tests/test_bulk.py
index bd0db9e..ccb4988 100644
--- a/src/mailman_pgp/mta/tests/test_bulk.py
+++ b/src/mailman_pgp/mta/tests/test_bulk.py
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
-""""""
+"""Test the PGP enabled BulkDelivery."""
 import unittest
 
 from mailman.app.lifecycle import create_list
@@ -34,7 +34,7 @@ from mailman_pgp.utils.pgp import verifies
 
 
 class BulkDeliveryTester(PGPBulkDelivery):
-    """"""
+    """Save deliveries made by the PGPBulkDelivery class."""
 
     def __init__(self, max_recipients=None):
         super().__init__(max_recipients=max_recipients)
@@ -166,3 +166,30 @@ Subject: test
         wrapped = PGPWrapper(out_msg)
         self.assertFalse(wrapped.is_signed())
         self.assertFalse(wrapped.is_encrypted())
+
+    def test_no_pgp_list(self):
+        ordinary_list = create_list('ordinary@example.com')
+        msgdata = dict(recipients=['anne@example.org', 'bart@example.org'])
+        agent = BulkDeliveryTester(2)
+        refused = agent.deliver(ordinary_list, self.msg, msgdata)
+
+        self.assertEqual(len(refused), 0)
+        self.assertEqual(len(agent.deliveries), 1)
+
+    def test_no_pgp_address(self):
+        msgdata = dict(recipients=['anne@example.org', 'someone@example.org'])
+        agent = BulkDeliveryTester(2)
+        refused = agent.deliver(self.mlist, self.msg, msgdata)
+
+        self.assertEqual(len(refused), 1)
+        self.assertEqual(len(agent.deliveries), 1)
+
+    def test_no_key(self):
+        with transaction():
+            self.pgp_bart.key = None
+        msgdata = dict(recipients=['anne@example.org', 'bart@example.org'])
+        agent = BulkDeliveryTester(2)
+        refused = agent.deliver(self.mlist, self.msg, msgdata)
+
+        self.assertEqual(len(refused), 1)
+        self.assertEqual(len(agent.deliveries), 1)
diff --git a/src/mailman_pgp/mta/tests/test_personalized.py b/src/mailman_pgp/mta/tests/test_personalized.py
index 9200a3b..0357757 100644
--- a/src/mailman_pgp/mta/tests/test_personalized.py
+++ b/src/mailman_pgp/mta/tests/test_personalized.py
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
-""""""
+"""Test the PGP enabled IndividualDelivery."""
 import unittest
 
 from mailman.app.lifecycle import create_list
@@ -34,7 +34,7 @@ from mailman_pgp.utils.pgp import verifies
 
 
 class PersonalizedDeliveryTester(PGPPersonalizedDelivery):
-    """"""
+    """Save the deliveries made by the PGPPersonalizedDelivery class."""
 
     def __init__(self):
         super().__init__()