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.

7 files changed, 45 insertions, 85 deletions

diff --git a/src/mailman/app/docs/moderator.rst b/src/mailman/app/docs/moderator.rst
index e183d050c..ce25a4711 100644
--- a/src/mailman/app/docs/moderator.rst
+++ b/src/mailman/app/docs/moderator.rst
@@ -217,12 +217,12 @@ the unsubscribing address is required.
 Fred is a member of the mailing list...
 
     >>> from mailman.interfaces.usermanager import IUserManager
-    >>> from zope.component import getAdapter, getUtility
+    >>> from zope.component import getUtility
     >>> mlist.send_welcome_message = False
     >>> fred = getUtility(IUserManager).create_address(
     ...     'fred@example.com', 'Fred Person')
     >>> from mailman.interfaces.subscriptions import ISubscriptionManager
-    >>> registrar = getAdapter(mlist, ISubscriptionManager, name='subscribe')
+    >>> registrar = ISubscriptionManager(mlist)
     >>> token, token_owner, member = registrar.register(
     ...     fred, pre_verified=True, pre_confirmed=True, pre_approved=True)
     >>> member
diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py
index 2790a6751..15df6b3f3 100644
--- a/src/mailman/app/subscriptions.py
+++ b/src/mailman/app/subscriptions.py
@@ -297,7 +297,9 @@ class SubscriptionWorkflow(Workflow):
         # We can immediately subscribe the user to the mailing list.
         self.member = self.mlist.subscribe(self.subscriber)
         # This workflow is done so throw away any associated state.
-        getUtility(IWorkflowStateManager).restore(self.name, self.token)
+        if self.token is not None:
+            getUtility(IWorkflowStateManager).discard(self.token)
+            self._set_token(TokenOwner.no_one)
 
     def _step_send_confirmation(self):
         self._set_token(TokenOwner.subscriber)
@@ -533,7 +535,9 @@ class UnSubscriptionWorkflow(Workflow):
             pass
         self.member = None
         # This workflow is done so throw away any associated state.
-        getUtility(IWorkflowStateManager).restore(self.name, self.token)
+        if self.token is not None:
+            getUtility(IWorkflowStateManager).discard(self.token)
+            self._set_token(TokenOwner.no_one)
 
     def _step_unsubscribe_from_restored(self):
         # Prevent replay attacks.
@@ -546,48 +550,11 @@ class UnSubscriptionWorkflow(Workflow):
         self.push('do_unsubscription')
 
 
-class BaseSubscriptionManager:
-    """Base class to handle registration and un-registration workflows."""
-
-    def __init__(self, mlist):
-        self._mlist = mlist
-
-    def _get_workflow(self):
-        raise NotImplementedError
-
-    def register(self, subscriber=None, *,
-                 pre_verified=False, pre_confirmed=False, pre_approved=False):
-        raise NotImplementedError
-
-    def unregister(self, subscriber=None, *,
-                   pre_confirmed=False, pre_approved=False):
-        raise NotImplementedError
-
-    def confirm(self, token):
-        workflow = self._get_workflow()
-        workflow.token = token
-        workflow.restore()
-        # In order to just run the whole workflow, all we need to do
-        # is iterate over the workflow object. On calling the __next__
-        # over the workflow iterator it automatically executes the steps
-        # that needs to be done.
-        list(workflow)
-        return workflow.token, workflow.token_owner, workflow.member
-
-    def discard(self, token):
-        with flush():
-            getUtility(IPendings).confirm(token)
-            getUtility(IWorkflowStateManager).discard(
-                self._get_workflow().name, token)
-
-
 @public
 @implementer(ISubscriptionManager)
-class SubscriptionManager(BaseSubscriptionManager):
-    """Handle registrations and confirmations for subscriptions."""
-
-    def _get_workflow(self):
-        return SubscriptionWorkflow(self._mlist)
+class SubscriptionManager:
+    def __init__(self, mlist):
+        self._mlist = mlist
 
     def register(self, subscriber=None, *,
                  pre_verified=False, pre_confirmed=False, pre_approved=False):
@@ -600,15 +567,6 @@ class SubscriptionManager(BaseSubscriptionManager):
         list(workflow)
         return workflow.token, workflow.token_owner, workflow.member
 
-
-@public
-@implementer(ISubscriptionManager)
-class UnsubscriptionManager(BaseSubscriptionManager):
-    """Handle un-subscriptions and confirmations for un-subscriptions."""
-
-    def _get_workflow(self):
-        return UnSubscriptionWorkflow(self._mlist)
-
     def unregister(self, subscriber=None, *,
                    pre_confirmed=False, pre_approved=False):
         workflow = UnSubscriptionWorkflow(
@@ -618,6 +576,32 @@ class UnsubscriptionManager(BaseSubscriptionManager):
         list(workflow)
         return workflow.token, workflow.token_owner, workflow.member
 
+    def confirm(self, token):
+        if token is None:
+            raise LookupError
+        pendable = getUtility(IPendings).confirm(token, expunge=False)
+        if pendable is None:
+            raise LookupError
+        workflow_type = pendable.get('type')
+        assert workflow_type in (PendableSubscription.PEND_TYPE,
+                                 PendableUnsubscription.PEND_TYPE)
+        workflow = (SubscriptionWorkflow
+                    if workflow_type == PendableSubscription.PEND_TYPE
+                    else UnSubscriptionWorkflow)(self._mlist)
+        workflow.token = token
+        workflow.restore()
+        # In order to just run the whole workflow, all we need to do
+        # is iterate over the workflow object. On calling the __next__
+        # over the workflow iterator it automatically executes the steps
+        # that needs to be done.
+        list(workflow)
+        return workflow.token, workflow.token_owner, workflow.member
+
+    def discard(self, token):
+        with flush():
+            getUtility(IPendings).confirm(token)
+            getUtility(IWorkflowStateManager).discard(token)
+
 
 def _handle_confirmation_needed_events(event, template_name):
     subject = 'confirm {}'.format(event.token)
diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py
index 63b665948..90485bfad 100644
--- a/src/mailman/app/tests/test_moderation.py
+++ b/src/mailman/app/tests/test_moderation.py
@@ -35,7 +35,7 @@ from mailman.testing.helpers import (
     specialized_message_from_string as mfs)
 from mailman.testing.layers import SMTPLayer
 from mailman.utilities.datetime import now
-from zope.component import getAdapter, getUtility
+from zope.component import getUtility
 
 
 class TestModeration(unittest.TestCase):
@@ -153,8 +153,7 @@ class TestUnsubscription(unittest.TestCase):
 
     def setUp(self):
         self._mlist = create_list('test@example.com')
-        self._registrar = getAdapter(
-            self._mlist, ISubscriptionManager, name='subscribe')
+        self._registrar = ISubscriptionManager(self._mlist)
 
     def test_unsubscribe_defer(self):
         # When unsubscriptions must be approved by the moderator, but the
diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py
index 9f02593a9..a6a00d80e 100644
--- a/src/mailman/app/tests/test_subscriptions.py
+++ b/src/mailman/app/tests/test_subscriptions.py
@@ -359,8 +359,6 @@ class TestSubscriptionWorkflow(unittest.TestCase):
                                         pre_verified=True,
                                         pre_confirmed=True,
                                         pre_approved=True)
-        # Cache the token.
-        token = workflow.token
         # Consume the entire state machine.
         list(workflow)
         # Anne is now a member of the mailing list.
@@ -370,13 +368,6 @@ class TestSubscriptionWorkflow(unittest.TestCase):
         # The workflow is done, so it has no token.
         self.assertIsNone(workflow.token)
         self.assertEqual(workflow.token_owner, TokenOwner.no_one)
-        # The pendable associated with the token has been evicted.
-        self.assertIsNone(getUtility(IPendings).confirm(token, expunge=False))
-        # There is no saved workflow associated with the token.  This shows up
-        # as an exception when we try to restore the workflow.
-        new_workflow = SubscriptionWorkflow(self._mlist)
-        new_workflow.token = token
-        self.assertRaises(LookupError, new_workflow.restore)
 
     def test_moderator_approves(self):
         # The workflow runs until moderator approval is required, at which
diff --git a/src/mailman/app/tests/test_unsubscriptions.py b/src/mailman/app/tests/test_unsubscriptions.py
index b7693f774..d8dbc18cb 100644
--- a/src/mailman/app/tests/test_unsubscriptions.py
+++ b/src/mailman/app/tests/test_unsubscriptions.py
@@ -184,7 +184,7 @@ class TestUnSubscriptionWorkflow(unittest.TestCase):
         self.assertIsNone(workflow.token)
         self.assertEqual(workflow.token_owner, TokenOwner.no_one)
 
-    def test_do_unsubscription_pre_approved_pre_onfirmed(self):
+    def test_do_unsubscription_pre_approved_pre_confirmed(self):
         # A moderation-requiring un-subscription policy plus a pre-appvoed
         # address means the user gets un-subscribed to the mailing list without
         # any further confirmations or approvals.
@@ -201,14 +201,12 @@ class TestUnSubscriptionWorkflow(unittest.TestCase):
         self.assertEqual(workflow.token_owner, TokenOwner.no_one)
 
     def test_do_unsubscription_cleanups(self):
-        # Once the user is un-subscribed, the token and its associated pending
+        # Once the user is unsubscribed, the token and its associated pending
         # database record will be removed from the database.
         self._mlist.unsubscription_policy = SubscriptionPolicy.open
         workflow = UnSubscriptionWorkflow(self._mlist, self.anne,
                                           pre_approved=True,
                                           pre_confirmed=True)
-        # Cache the token.
-        token = workflow.token
         # Run the workflow.
         list(workflow)
         # Anne is now un-subscribed from the list.
@@ -217,13 +215,6 @@ class TestUnSubscriptionWorkflow(unittest.TestCase):
         # Workflow is done, so it has no token.
         self.assertIsNone(workflow.token)
         self.assertEqual(workflow.token_owner, TokenOwner.no_one)
-        # The pendable associated with the token as been evicted.
-        self.assertIsNone(getUtility(IPendings).confirm(token, expunge=False))
-        # There is no workflow associated with the token. This shows up as an
-        # exception when trying to restore the workflow.
-        new_workflow = UnSubscriptionWorkflow(self._mlist)
-        new_workflow.token = token
-        self.assertRaises(LookupError, new_workflow.restore)
 
     def test_moderator_approves(self):
         # The workflow runs until moderator approval is required, at which
diff --git a/src/mailman/app/tests/test_workflowmanager.py b/src/mailman/app/tests/test_workflowmanager.py
index f72486111..4193beefe 100644
--- a/src/mailman/app/tests/test_workflowmanager.py
+++ b/src/mailman/app/tests/test_workflowmanager.py
@@ -28,7 +28,7 @@ from mailman.interfaces.usermanager import IUserManager
 from mailman.testing.helpers import get_queue_messages
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.datetime import now
-from zope.component import getAdapter, getUtility
+from zope.component import getUtility
 
 
 class TestRegistrar(unittest.TestCase):
@@ -38,8 +38,7 @@ class TestRegistrar(unittest.TestCase):
 
     def setUp(self):
         self._mlist = create_list('ant@example.com')
-        self._registrar = getAdapter(
-            self._mlist, ISubscriptionManager, name='subscribe')
+        self._registrar = ISubscriptionManager(self._mlist)
         self._pendings = getUtility(IPendings)
         self._anne = getUtility(IUserManager).create_address(
             'anne@example.com')
diff --git a/src/mailman/app/workflow.py b/src/mailman/app/workflow.py
index eec8a14a8..762b0fd91 100644
--- a/src/mailman/app/workflow.py
+++ b/src/mailman/app/workflow.py
@@ -132,15 +132,11 @@ class Workflow:
             raise AssertionError(
                 "Can't save a workflow state with more than one step "
                 "in the queue")
-        state_manager.save(
-            self.__class__.__name__,
-            self.token,
-            step,
-            json.dumps(data))
+        state_manager.save(self.token, step, json.dumps(data))
 
     def restore(self):
         state_manager = getUtility(IWorkflowStateManager)
-        state = state_manager.restore(self.__class__.__name__, self.token)
+        state = state_manager.restore(self.token)
         if state is None:
             # The token doesn't exist in the database.
             raise LookupError(self.token)