7 files changed, 221 insertions, 89 deletions

diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py
index 1e45da43e..8cd507bd2 100644
--- a/src/mailman/app/subscriptions.py
+++ b/src/mailman/app/subscriptions.py
@@ -24,16 +24,10 @@ __all__ = [
     ]
 
 
-import json
-from collections import deque
-from operator import attrgetter
-from sqlalchemy import and_, or_
-from uuid import UUID
-from zope.component import getUtility
-from zope.interface import implementer
 
 from mailman.app.membership import add_member, delete_member
 from mailman.app.moderator import hold_subscription
+from mailman.app.workflow import Workflow
 from mailman.core.constants import system_preferences
 from mailman.database.transaction import dbconnection
 from mailman.interfaces.address import IAddress
@@ -45,9 +39,13 @@ from mailman.interfaces.subscriptions import (
     ISubscriptionService, MissingUserError, RequestRecord)
 from mailman.interfaces.user import IUser
 from mailman.interfaces.usermanager import IUserManager
-from mailman.interfaces.workflowstate import IWorkflowStateManager
 from mailman.model.member import Member
 from mailman.utilities.datetime import now
+from operator import attrgetter
+from sqlalchemy import and_, or_
+from uuid import UUID
+from zope.component import getUtility
+from zope.interface import implementer
 
 
 def _membership_sort_key(member):
@@ -59,68 +57,6 @@ def _membership_sort_key(member):
     return (member.list_id, member.address.email, member.role.value)
 
 
-class Workflow:
-    """Generic workflow."""
-    # TODO: move this class to a more generic module
-
-    _save_key = ""
-    _save_attributes = []
-    _initial_state = []
-
-    def __init__(self):
-        self._next = deque(self._initial_state)
-
-    def __iter__(self):
-        return self
-
-    def _pop(self):
-        name = self._next.popleft()
-        step = getattr(self, '_step_{}'.format(name))
-        return step, name
-
-    def __next__(self):
-        try:
-            step, name = self._pop()
-            step()
-        except IndexError:
-            raise StopIteration
-        except:
-            raise
-
-    def save_state(self):
-        state_manager = getUtility(IWorkflowStateManager)
-        data = {attr: getattr(self, attr) for attr in self._save_attributes}
-        # Note: only the next step is saved, not the whole stack. Not an issue
-        # since there's never more than a single step in the queue anyway.
-        # If we want to support more than a single step in the queue AND want
-        # to support state saving/restoring, change this method and the
-        # restore_state() method.
-        if len(self._next) == 0:
-            step = None
-        elif len(self._next) == 1:
-            step = self._next[0]
-        else:
-            raise AssertionError(
-                "Can't save a workflow state with more than one step "
-                "in the queue")
-        state_manager.save(
-            self.__class__.__name__,
-            self._save_key,
-            step,
-            json.dumps(data))
-
-    def restore_state(self):
-        state_manager = getUtility(IWorkflowStateManager)
-        state = state_manager.restore(self.__class__.__name__, self._save_key)
-        if state is not None:
-            self._next.clear()
-            if state.step:
-                self._next.append(state.step)
-            if state.data is not None:
-                for attr, value in json.loads(state.data).items():
-                    setattr(self, attr, value)
-
-
 class SubscriptionWorkflow(Workflow):
     """Workflow of a subscription request."""
 
diff --git a/src/mailman/app/workflow.py b/src/mailman/app/workflow.py
new file mode 100644
index 000000000..91c8c9f84
--- /dev/null
+++ b/src/mailman/app/workflow.py
@@ -0,0 +1,91 @@
+# Copyright (C) 2015 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Generic workflow."""
+
+__all__ = [
+    'Workflow',
+    ]
+
+
+import json
+
+from collections import deque
+from mailman.interfaces.workflow import IWorkflowStateManager
+from zope.component import getUtility
+
+
+
+class Workflow:
+    """Generic workflow."""
+
+    _save_key = ''
+    _save_attributes = []
+    _initial_state = []
+
+    def __init__(self):
+        self._next = deque(self._initial_state)
+
+    def __iter__(self):
+        return self
+
+    def _pop(self):
+        name = self._next.popleft()
+        step = getattr(self, '_step_{}'.format(name))
+        return step, name
+
+    def __next__(self):
+        try:
+            step, name = self._pop()
+            step()
+        except IndexError:
+            raise StopIteration
+        except:
+            raise
+
+    def save_state(self):
+        state_manager = getUtility(IWorkflowStateManager)
+        data = {attr: getattr(self, attr) for attr in self._save_attributes}
+        # Note: only the next step is saved, not the whole stack. Not an issue
+        # since there's never more than a single step in the queue anyway.
+        # If we want to support more than a single step in the queue AND want
+        # to support state saving/restoring, change this method and the
+        # restore_state() method.
+        if len(self._next) == 0:
+            step = None
+        elif len(self._next) == 1:
+            step = self._next[0]
+        else:
+            raise AssertionError(
+                "Can't save a workflow state with more than one step "
+                "in the queue")
+        state_manager.save(
+            self.__class__.__name__,
+            self._save_key,
+            step,
+            json.dumps(data))
+
+    def restore_state(self):
+        state_manager = getUtility(IWorkflowStateManager)
+        state = state_manager.restore(self.__class__.__name__, self._save_key)
+        if state is not None:
+            self._next.clear()
+            if state.step:
+                self._next.append(state.step)
+            if state.data is not None:
+                for attr, value in json.loads(state.data).items():
+                    setattr(self, attr, value)
diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml
index f6cb6dac1..1f2283b02 100644
--- a/src/mailman/config/configure.zcml
+++ b/src/mailman/config/configure.zcml
@@ -118,8 +118,8 @@
    />
 
   <utility
-    provides="mailman.interfaces.workflowstate.IWorkflowStateManager"
-    factory="mailman.model.workflowstate.WorkflowStateManager"
+    provides="mailman.interfaces.workflow.IWorkflowStateManager"
+    factory="mailman.model.workflow.WorkflowStateManager"
     />
 
 </configure>
diff --git a/src/mailman/database/alembic/versions/2bb9b382198_workflow_state_table.py b/src/mailman/database/alembic/versions/2bb9b382198_workflow_state_table.py
index e6608b6ce..59cb1121e 100644
--- a/src/mailman/database/alembic/versions/2bb9b382198_workflow_state_table.py
+++ b/src/mailman/database/alembic/versions/2bb9b382198_workflow_state_table.py
@@ -21,7 +21,7 @@ def upgrade():
         sa.Column('step', sa.Unicode(), nullable=True),
         sa.Column('data', sa.Unicode(), nullable=True),
         sa.PrimaryKeyConstraint('name', 'key')
-    )
+        )
 
 
 def downgrade():
diff --git a/src/mailman/interfaces/workflowstate.py b/src/mailman/interfaces/workflow.py
index 186386170..b5aeec093 100644
--- a/src/mailman/interfaces/workflowstate.py
+++ b/src/mailman/interfaces/workflow.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2007-2015 by the Free Software Foundation, Inc.
+# Copyright (C) 2015 by the Free Software Foundation, Inc.
 #
 # This file is part of GNU Mailman.
 #
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License along with
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Interface describing the state of a workflow."""
+"""Interfaces describing the state of a workflow."""
 
 __all__ = [
     'IWorkflowState',
@@ -23,24 +23,20 @@ __all__ = [
     ]
 
 
-from zope.interface import Interface, Attribute
+from zope.interface import Attribute, Interface
 
 
 
 class IWorkflowState(Interface):
     """The state of a workflow."""
 
-    name = Attribute(
-        """The name of the workflow.""")
+    name = Attribute('The name of the workflow.')
 
-    key = Attribute(
-        """A unique key identifying the workflow instance.""")
+    key = Attribute('A unique key identifying the workflow instance.')
 
-    step = Attribute(
-        """This workflow's next step.""")
+    step = Attribute("This workflow's next step.")
 
-    data = Attribute(
-        """Additional data (may be JSON-encodedeJSON .""")
+    data = Attribute('Additional data (may be JSON-encodedeJSON .')
 
 
 
diff --git a/src/mailman/model/tests/test_workflow.py b/src/mailman/model/tests/test_workflow.py
new file mode 100644
index 000000000..b5e915df4
--- /dev/null
+++ b/src/mailman/model/tests/test_workflow.py
@@ -0,0 +1,110 @@
+# Copyright (C) 2015 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the workflow model."""
+
+__all__ = [
+    'TestWorkflow',
+    ]
+
+
+import unittest
+
+from mailman.interfaces.workflow import IWorkflowStateManager
+from mailman.testing.layers import ConfigLayer
+from zope.component import getUtility
+
+
+
+class TestWorkflow(unittest.TestCase):
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._manager = getUtility(IWorkflowStateManager)
+
+    def test_save_restore_workflow(self):
+        # Save and restore a workflow.
+        name = 'ant'
+        key = 'bee'
+        step = 'cat'
+        data = 'dog'
+        self._manager.save(name, key, step, data)
+        workflow = self._manager.restore(name, key)
+        self.assertEqual(workflow.name, name)
+        self.assertEqual(workflow.key, key)
+        self.assertEqual(workflow.step, step)
+        self.assertEqual(workflow.data, data)
+
+    def test_save_restore_workflow_without_step(self):
+        # Save and restore a workflow that contains no step.
+        name = 'ant'
+        key = 'bee'
+        data = 'dog'
+        self._manager.save(name, key, data=data)
+        workflow = self._manager.restore(name, key)
+        self.assertEqual(workflow.name, name)
+        self.assertEqual(workflow.key, key)
+        self.assertIsNone(workflow.step)
+        self.assertEqual(workflow.data, data)
+
+    def test_save_restore_workflow_without_data(self):
+        # Save and restore a workflow that contains no data.
+        name = 'ant'
+        key = 'bee'
+        step = 'cat'
+        self._manager.save(name, key, step)
+        workflow = self._manager.restore(name, key)
+        self.assertEqual(workflow.name, name)
+        self.assertEqual(workflow.key, key)
+        self.assertEqual(workflow.step, step)
+        self.assertIsNone(workflow.data)
+
+    def test_save_restore_workflow_without_step_or_data(self):
+        # Save and restore a workflow that contains no step or data.
+        name = 'ant'
+        key = 'bee'
+        self._manager.save(name, key)
+        workflow = self._manager.restore(name, key)
+        self.assertEqual(workflow.name, name)
+        self.assertEqual(workflow.key, key)
+        self.assertIsNone(workflow.step)
+        self.assertIsNone(workflow.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'
+        key = 'bee'
+        self._manager.save(name, key)
+        workflow = self._manager.restore('ewe', key)
+        self.assertIsNone(workflow)
+
+    def test_restore_workflow_with_no_matching_key(self):
+        # Try to restore a workflow that has no matching key in the database.
+        name = 'ant'
+        key = 'bee'
+        self._manager.save(name, key)
+        workflow = self._manager.restore(name, 'fly')
+        self.assertIsNone(workflow)
+
+    def test_restore_workflow_with_no_matching_key_or_name(self):
+        # Try to restore a workflow that has no matching key or name in the
+        # database.
+        name = 'ant'
+        key = 'bee'
+        self._manager.save(name, key)
+        workflow = self._manager.restore('ewe', 'fly')
+        self.assertIsNone(workflow)
diff --git a/src/mailman/model/workflowstate.py b/src/mailman/model/workflow.py
index 229a2240b..53c9f05ea 100644
--- a/src/mailman/model/workflowstate.py
+++ b/src/mailman/model/workflow.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2007-2015 by the Free Software Foundation, Inc.
+# Copyright (C) 2015 by the Free Software Foundation, Inc.
 #
 # This file is part of GNU Mailman.
 #
@@ -25,8 +25,7 @@ __all__ = [
 
 from mailman.database.model import Model
 from mailman.database.transaction import dbconnection
-from mailman.interfaces.workflowstate import (
-    IWorkflowState, IWorkflowStateManager)
+from mailman.interfaces.workflow import IWorkflowState, IWorkflowStateManager
 from sqlalchemy import Column, Unicode
 from zope.interface import implementer
 
@@ -54,8 +53,8 @@ class WorkflowStateManager:
         """See `IWorkflowStateManager`."""
         state = store.query(WorkflowState).get((name, key))
         if state is None:
-            state = store.add(WorkflowState(
-                name=name, key=key, step=step, data=data))
+            state = WorkflowState(name=name, key=key, step=step, data=data)
+            store.add(state)
         else:
             state.step = step
             state.data = data