5 files changed, 47 insertions, 26 deletions

diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py
index 49686af01..f446213e5 100644
--- a/src/mailman/database/tests/test_migrations.py
+++ b/src/mailman/database/tests/test_migrations.py
@@ -30,6 +30,7 @@ from mailman.config import config
 from mailman.database.alembic import alembic_cfg
 from mailman.database.helpers import exists_in_db
 from mailman.database.model import Model
+from mailman.database.transaction import transaction
 from mailman.testing.layers import ConfigLayer
 
 
@@ -136,20 +137,21 @@ class TestMigrations(unittest.TestCase):
                     ])
             return results
         # Start at the previous revision
-        alembic.command.downgrade(alembic_cfg, '33bc0099223')
-        for i in range(1, 6):
-            config.db.store.execute(pended_table.insert().values(id=i))
-        config.db.store.execute(keyvalue_table.insert().values([
-            {'pended_id': 1, 'key': 'member_id', 'value': 'test-value'},
-            {'pended_id': 2, 'key': 'token_owner', 'value': 'test-value'},
-            {'pended_id': 3, 'key': '_mod_message_id', 'value': 'test-value'},
-            {'pended_id': 4, 'key': 'type', 'value': '"held message"'},
-            {'pended_id': 5, 'key': 'type', 'value': 'registration'},
-            ]))
-        config.db.store.commit()
+        with transaction():
+            alembic.command.downgrade(alembic_cfg, '33bc0099223')
+            for i in range(1, 6):
+                config.db.store.execute(pended_table.insert().values(id=i))
+            config.db.store.execute(keyvalue_table.insert().values([
+                {'pended_id': 1, 'key': 'member_id', 'value': 'test-value'},
+                {'pended_id': 2, 'key': 'token_owner', 'value': 'test-value'},
+                {'pended_id': 3, 'key': '_mod_message_id', 'value': 'test-value'},
+                {'pended_id': 4, 'key': 'type', 'value': '"held message"'},
+                {'pended_id': 5, 'key': 'type', 'value': 'registration'},
+                ]))
         # Upgrading.
-        alembic.command.upgrade(alembic_cfg, '47294d3a604')
-        results = get_from_db()
+        with transaction():
+            alembic.command.upgrade(alembic_cfg, '47294d3a604')
+            results = get_from_db()
         for i in range(1, 5):
             self.assertIn('type', results[i])
         self.assertEqual(results[1]['type'], 'probe')
@@ -157,10 +159,10 @@ class TestMigrations(unittest.TestCase):
         self.assertEqual(results[3]['type'], 'data')
         self.assertEqual(results[4]['type'], 'held message')
         self.assertEqual(results[5]['type'], 'registration')
-        config.db.store.commit()
         # Downgrading.
-        alembic.command.downgrade(alembic_cfg, '33bc0099223')
-        results = get_from_db()
+        with transaction():
+            alembic.command.downgrade(alembic_cfg, '33bc0099223')
+            results = get_from_db()
         for i in range(1, 4):
             self.assertNotIn('type', results[i])
         self.assertEqual(results[4]['type'], '"held message"')
diff --git a/src/mailman/interfaces/pending.py b/src/mailman/interfaces/pending.py
index d71322e77..152946b66 100644
--- a/src/mailman/interfaces/pending.py
+++ b/src/mailman/interfaces/pending.py
@@ -97,6 +97,16 @@ class IPendings(Interface):
     def evict():
         """Remove all pended items whose lifetime has expired."""
 
+    def find(mlist=None, pend_type=None):
+        """Search for the pendables matching the given criteria.
+
+        :param mlist: The MailingList object that the pendables must be
+            related to.
+        :param pend_type: The type of the pendables that are looked for, this
+            corresponds to the `PEND_TYPE` attribute.
+        :return: An iterator over 2-tuples of the form (token, dict).
+        """
+
     def __iter__():
         """An iterator over all pendables.
 
diff --git a/src/mailman/model/docs/pending.rst b/src/mailman/model/docs/pending.rst
index 4accf5f3f..9c21a09eb 100644
--- a/src/mailman/model/docs/pending.rst
+++ b/src/mailman/model/docs/pending.rst
@@ -43,8 +43,13 @@ There's exactly one entry in the pendings database now.
     1
 
 You can *confirm* the pending, which means returning the `IPendable` structure
-(as a dictionary) from the database that matches the token.  If the token
-isn't in the database, None is returned.
+(as a dictionary) from the database that matches the token.
+
+All `IPendable` classes have a `PEND_TYPE` attribute which must be a string. It
+is used to identify and query pendables in the database, and will be returned
+as the `type` key in the dictionary.
+
+If the token isn't in the database, None is returned.
 
     >>> pendable = pendingdb.confirm(b'missing')
     >>> print(pendable)
diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py
index c99ac8ee1..61b526ece 100644
--- a/src/mailman/model/pending.py
+++ b/src/mailman/model/pending.py
@@ -105,10 +105,13 @@ class Pendings:
         pending = Pended(
             token=token,
             expiration_date=now() + lifetime)
-        pendable_type = pendable.pop('type', pendable.PEND_TYPE)
+        pendable_type = pendable.get('type', pendable.PEND_TYPE)
         pending.key_values.append(
             PendedKeyValue(key='type', value=pendable_type))
         for key, value in pendable.items():
+            # The type has been handled above.
+            if key == 'type':
+                continue
             # Both keys and values must be strings.
             if isinstance(key, bytes):
                 key = key.decode('utf-8')
@@ -154,7 +157,7 @@ class Pendings:
                 store.delete(pending)
 
     @dbconnection
-    def find(self, store, mlist=None, type=None):
+    def find(self, store, mlist=None, pend_type=None):
         query = store.query(Pended)
         if mlist is not None:
             pkv_alias_mlist = aliased(PendedKeyValue)
@@ -162,11 +165,11 @@ class Pendings:
                 pkv_alias_mlist.key == 'list_id',
                 pkv_alias_mlist.value == json.dumps(mlist.list_id)
                 ))
-        if type is not None:
+        if pend_type is not None:
             pkv_alias_type = aliased(PendedKeyValue)
             query = query.join(pkv_alias_type).filter(and_(
                 pkv_alias_type.key == 'type',
-                pkv_alias_type.value == type
+                pkv_alias_type.value == pend_type
                 ))
         for pending in query:
             yield pending.token, self.confirm(pending.token, expunge=False)
diff --git a/src/mailman/model/tests/test_pending.py b/src/mailman/model/tests/test_pending.py
index 723d27be0..3fe381cf1 100644
--- a/src/mailman/model/tests/test_pending.py
+++ b/src/mailman/model/tests/test_pending.py
@@ -81,12 +81,12 @@ class TestPendings(unittest.TestCase):
         token_4 = pendingdb.add(subscription_4)
         self.assertEqual(pendingdb.count, 4)
         # Find the pending subscription in list1.
-        pendings = list(pendingdb.find(mlist=mlist, type='subscription'))
+        pendings = list(pendingdb.find(mlist=mlist, pend_type='subscription'))
         self.assertEqual(len(pendings), 1)
         self.assertEqual(pendings[0][0], token_1)
         self.assertEqual(pendings[0][1]['list_id'], 'list1.example.com')
         # Find all pending hold requests.
-        pendings = list(pendingdb.find(type='hold request'))
+        pendings = list(pendingdb.find(pend_type='hold request'))
         self.assertEqual(len(pendings), 2)
         self.assertSetEqual(
             set((p[0], p[1]['list_id']) for p in pendings),
@@ -96,6 +96,7 @@ class TestPendings(unittest.TestCase):
         pendings = list(pendingdb.find(mlist=mlist))
         self.assertEqual(len(pendings), 2)
         self.assertSetEqual(
-            set((p[0], p[1]['list_id']) for p in pendings),
-            {(token_1, 'list1.example.com'), (token_3, 'list1.example.com')}
+            set((p[0], p[1]['list_id'], p[1]['type']) for p in pendings),
+            {(token_1, 'list1.example.com', 'subscription'),
+             (token_3, 'list1.example.com', 'hold request')}
             )