Simplify the implementation.

This merges the SubscriptionManager and UnsubscriptionManager into a
single SubscriptionManager implementation that handles both register()
and unregister().  This allows us to use direct class-based adaptation
instead of the more clunky getAdapter() API.  We can also eliminate the
funky _get_workflow() implementation detail.

This has a couple of side-effects.  .confirm() must lookup the token in
the pendings database and pull out the pending type, dispatching to the
proper class depending on the type, or raising a LookupError if the
token is None or there is no pendable associated with the given token.
This feels like an acceptable trade-off.

However, this *also* means that IWorkflowStateManager must lose its
'name' argument in its methods.  That's because we won't actually know
the name until its too late.  Honestly, the name wasn't providing much
value anyway (it was always the subclass's name), so losing that seems
fine too.  The complication here is that the name was a primary key in
the 'workflowstate' table, so we need to add its removal in the database
migration.

4 files changed, 35 insertions, 65 deletions

diff --git a/src/mailman/model/docs/subscriptions.rst b/src/mailman/model/docs/subscriptions.rst
index 4c079d542..1e1810a7a 100644
--- a/src/mailman/model/docs/subscriptions.rst
+++ b/src/mailman/model/docs/subscriptions.rst
@@ -13,9 +13,8 @@ To begin, adapt a mailing list to an ``ISubscriptionManager``.  This is a
 named interface because the same interface manages both subscriptions and
 unsubscriptions.
 
-    >>> from zope.component import getAdapter
     >>> mlist = create_list('ant@example.com')
-    >>> manager = getAdapter(mlist, ISubscriptionManager, 'subscribe')
+    >>> manager = ISubscriptionManager(mlist)
 
 Either addresses or users with a preferred address can be registered.
 
@@ -114,7 +113,7 @@ that their email address is already verified, that step is not required.  To
 begin with unsubscribing, you need to adapt the mailing list to the same
 interface, but with a different name.
 
-    >>> manager = getAdapter(mlist, ISubscriptionManager, 'unsubscribe')
+    >>> manager = ISubscriptionManager(mlist)
 
 If the mailing list's unsubscription policy is open, unregistering the
 subscription takes effect immediately.
diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py
index 9d8315605..7e7f0b2eb 100644
--- a/src/mailman/model/pending.py
+++ b/src/mailman/model/pending.py
@@ -130,8 +130,8 @@ class Pendings:
         # Iterate on PendedKeyValue entries that are associated with the
         # pending object's ID.  Watch out for type conversions.
         for keyvalue in pending.key_values:
-            # The `type` key is special and served.  It is not JSONified.  See
-            # the IPendable interface for details.
+            # The `type` key is special and reserved.  It is not JSONified.
+            # See the IPendable interface for details.
             if keyvalue.key == 'type':
                 value = keyvalue.value
             else:
diff --git a/src/mailman/model/tests/test_workflow.py b/src/mailman/model/tests/test_workflow.py
index ee14f17a7..6aade980b 100644
--- a/src/mailman/model/tests/test_workflow.py
+++ b/src/mailman/model/tests/test_workflow.py
@@ -32,116 +32,88 @@ class TestWorkflow(unittest.TestCase):
 
     def test_save_restore_workflow(self):
         # Save and restore a workflow.
-        name = 'ant'
         token = 'bee'
         step = 'cat'
         data = 'dog'
-        self._manager.save(name, token, step, data)
-        state = self._manager.restore(name, token)
-        self.assertEqual(state.name, name)
+        self._manager.save(token, step, data)
+        state = self._manager.restore(token)
         self.assertEqual(state.token, token)
         self.assertEqual(state.step, step)
         self.assertEqual(state.data, data)
 
     def test_save_restore_workflow_without_step(self):
         # Save and restore a workflow that contains no step.
-        name = 'ant'
         token = 'bee'
         data = 'dog'
-        self._manager.save(name, token, data=data)
-        state = self._manager.restore(name, token)
-        self.assertEqual(state.name, name)
+        self._manager.save(token, data=data)
+        state = self._manager.restore(token)
         self.assertEqual(state.token, token)
         self.assertIsNone(state.step)
         self.assertEqual(state.data, data)
 
     def test_save_restore_workflow_without_data(self):
         # Save and restore a workflow that contains no data.
-        name = 'ant'
         token = 'bee'
         step = 'cat'
-        self._manager.save(name, token, step)
-        state = self._manager.restore(name, token)
-        self.assertEqual(state.name, name)
+        self._manager.save(token, step)
+        state = self._manager.restore(token)
         self.assertEqual(state.token, token)
         self.assertEqual(state.step, step)
         self.assertIsNone(state.data)
 
     def test_save_restore_workflow_without_step_or_data(self):
         # Save and restore a workflow that contains no step or data.
-        name = 'ant'
         token = 'bee'
-        self._manager.save(name, token)
-        state = self._manager.restore(name, token)
-        self.assertEqual(state.name, name)
+        self._manager.save(token)
+        state = self._manager.restore(token)
         self.assertEqual(state.token, token)
         self.assertIsNone(state.step)
         self.assertIsNone(state.data)
 
-    def test_restore_workflow_with_no_matching_name(self):
-        # Try to restore a workflow that has no matching name in the database.
-        name = 'ant'
-        token = 'bee'
-        self._manager.save(name, token)
-        state = self._manager.restore('ewe', token)
-        self.assertIsNone(state)
-
     def test_restore_workflow_with_no_matching_token(self):
         # Try to restore a workflow that has no matching token in the database.
-        name = 'ant'
-        token = 'bee'
-        self._manager.save(name, token)
-        state = self._manager.restore(name, 'fly')
-        self.assertIsNone(state)
-
-    def test_restore_workflow_with_no_matching_token_or_name(self):
-        # Try to restore a workflow that has no matching token or name in the
-        # database.
-        name = 'ant'
         token = 'bee'
-        self._manager.save(name, token)
-        state = self._manager.restore('ewe', 'fly')
+        self._manager.save(token)
+        state = self._manager.restore('fly')
         self.assertIsNone(state)
 
     def test_restore_removes_record(self):
-        name = 'ant'
         token = 'bee'
         self.assertEqual(self._manager.count, 0)
-        self._manager.save(name, token)
+        self._manager.save(token)
         self.assertEqual(self._manager.count, 1)
-        self._manager.restore(name, token)
+        self._manager.restore(token)
         self.assertEqual(self._manager.count, 0)
 
     def test_save_after_restore(self):
-        name = 'ant'
         token = 'bee'
         self.assertEqual(self._manager.count, 0)
-        self._manager.save(name, token)
+        self._manager.save(token)
         self.assertEqual(self._manager.count, 1)
-        self._manager.restore(name, token)
+        self._manager.restore(token)
         self.assertEqual(self._manager.count, 0)
-        self._manager.save(name, token)
+        self._manager.save(token)
         self.assertEqual(self._manager.count, 1)
 
     def test_discard(self):
         # Discard some workflow state.  This is use by
         # ISubscriptionManager.discard().
-        self._manager.save('ant', 'token', 'one')
-        self._manager.save('bee', 'token', 'two')
-        self._manager.save('ant', 'nekot', 'three')
-        self._manager.save('bee', 'nekot', 'four')
+        self._manager.save('token1', 'one')
+        self._manager.save('token2', 'two')
+        self._manager.save('token3', 'three')
+        self._manager.save('token4', 'four')
         self.assertEqual(self._manager.count, 4)
-        self._manager.discard('bee', 'token')
+        self._manager.discard('token2')
         self.assertEqual(self._manager.count, 3)
-        state = self._manager.restore('ant', 'token')
+        state = self._manager.restore('token1')
         self.assertEqual(state.step, 'one')
-        state = self._manager.restore('bee', 'token')
+        state = self._manager.restore('token2')
         self.assertIsNone(state)
-        state = self._manager.restore('ant', 'nekot')
+        state = self._manager.restore('token3')
         self.assertEqual(state.step, 'three')
-        state = self._manager.restore('bee', 'nekot')
+        state = self._manager.restore('token4')
         self.assertEqual(state.step, 'four')
 
     def test_discard_missing_workflow(self):
-        self._manager.discard('bogus-name', 'bogus-token')
+        self._manager.discard('bogus-token')
         self.assertEqual(self._manager.count, 0)
diff --git a/src/mailman/model/workflow.py b/src/mailman/model/workflow.py
index 1072fa548..53763a0e8 100644
--- a/src/mailman/model/workflow.py
+++ b/src/mailman/model/workflow.py
@@ -33,7 +33,6 @@ class WorkflowState(Model):
 
     __tablename__ = 'workflowstate'
 
-    name = Column(SAUnicode, primary_key=True)
     token = Column(SAUnicode, primary_key=True)
     step = Column(SAUnicode)
     data = Column(SAUnicode)
@@ -45,23 +44,23 @@ class WorkflowStateManager:
     """See `IWorkflowStateManager`."""
 
     @dbconnection
-    def save(self, store, name, token, step=None, data=None):
+    def save(self, store, token, step=None, data=None):
         """See `IWorkflowStateManager`."""
-        state = WorkflowState(name=name, token=token, step=step, data=data)
+        state = WorkflowState(token=token, step=step, data=data)
         store.add(state)
 
     @dbconnection
-    def restore(self, store, name, token):
+    def restore(self, store, token):
         """See `IWorkflowStateManager`."""
-        state = store.query(WorkflowState).get((name, token))
+        state = store.query(WorkflowState).get(token)
         if state is not None:
             store.delete(state)
         return state
 
     @dbconnection
-    def discard(self, store, name, token):
+    def discard(self, store, token):
         """See `IWorkflowStateManager`."""
-        state = store.query(WorkflowState).get((name, token))
+        state = store.query(WorkflowState).get(token)
         if state is not None:
             store.delete(state)