From 777e4b3d968f5d85ca37262adbb9540d0ff42dd0 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 4 Feb 2014 14:04:18 +0200 Subject: [PATCH 01/53] Create initial module layout, based on the keyword router --- go/config.py | 4 ++ go/routers/app_multiplexer/__init__.py | 0 go/routers/app_multiplexer/definition.py | 8 +++ go/routers/app_multiplexer/tests/__init__.py | 0 .../app_multiplexer/tests/test_views.py | 24 +++++++ .../app_multiplexer/tests/test_vumi_app.py | 71 +++++++++++++++++++ go/routers/app_multiplexer/view_definition.py | 42 +++++++++++ go/routers/app_multiplexer/vumi_app.py | 37 ++++++++++ 8 files changed, 186 insertions(+) create mode 100644 go/routers/app_multiplexer/__init__.py create mode 100644 go/routers/app_multiplexer/definition.py create mode 100644 go/routers/app_multiplexer/tests/__init__.py create mode 100644 go/routers/app_multiplexer/tests/test_views.py create mode 100644 go/routers/app_multiplexer/tests/test_vumi_app.py create mode 100644 go/routers/app_multiplexer/view_definition.py create mode 100644 go/routers/app_multiplexer/vumi_app.py diff --git a/go/config.py b/go/config.py index 0687f4a38..7df334450 100644 --- a/go/config.py +++ b/go/config.py @@ -136,6 +136,10 @@ def get_router_definition(router_type, router=None): 'namespace': 'keyword', 'display_name': 'Keyword', }, + 'go.routers.app_multiplexer': { + 'namespace': 'application_multiplexer', + 'display_name': 'Application Multiplexer', + }, } _VUMI_OBSOLETE_ROUTERS = [ diff --git a/go/routers/app_multiplexer/__init__.py b/go/routers/app_multiplexer/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/go/routers/app_multiplexer/definition.py b/go/routers/app_multiplexer/definition.py new file mode 100644 index 000000000..de6dd684e --- /dev/null +++ b/go/routers/app_multiplexer/definition.py @@ -0,0 +1,8 @@ +from go.vumitools.router.definition import RouterDefinitionBase + + +class RouterDefinition(RouterDefinitionBase): + router_type = 'application_multiplexer' + + def configured_outbound_endpoints(self, config): + return list(set(config.get('keyword_endpoint_mapping', {}).values())) diff --git a/go/routers/app_multiplexer/tests/__init__.py b/go/routers/app_multiplexer/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/go/routers/app_multiplexer/tests/test_views.py b/go/routers/app_multiplexer/tests/test_views.py new file mode 100644 index 000000000..d6af33c34 --- /dev/null +++ b/go/routers/app_multiplexer/tests/test_views.py @@ -0,0 +1,24 @@ +from go.base.tests.helpers import GoDjangoTestCase +from go.routers.tests.view_helpers import RouterViewsHelper + + +class ApplicationMultiplexerViewTests(GoDjangoTestCase): + + def setUp(self): + self.router_helper = self.add_helper( + RouterViewsHelper(u'application_multiplexer') + ) + self.user_helper = self.router_helper.vumi_helper.get_or_create_user() + self.client = self.router_helper.get_client() + + def test_new_router(self): + router_store = self.user_helper.user_api.router_store + self.assertEqual([], router_store.list_routers()) + + response = self.client.post(self.router_helper.get_new_view_url(), { + 'name': u"myrouter", + 'router_type': u'application_multiplexer', + }) + [router_key] = router_store.list_routers() + rtr_helper = self.router_helper.get_router_helper_by_key(router_key) + self.assertRedirects(response, rtr_helper.get_view_url('edit')) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py new file mode 100644 index 000000000..62dd808bf --- /dev/null +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -0,0 +1,71 @@ +from twisted.internet.defer import inlineCallbacks + +from vumi.tests.helpers import VumiTestCase + +from go.routers.app_multiplexer.vumi_app import ApplicationMultiplexer +from go.routers.tests.helpers import RouterWorkerHelper + + +class TestApplicationMultiplexerRouter(VumiTestCase): + + router_class = ApplicationMultiplexer + + @inlineCallbacks + def setUp(self): + self.router_helper = self.add_helper( + RouterWorkerHelper(ApplicationMultiplexer) + ) + self.router_worker = yield self.router_helper.get_router_worker({}) + + @inlineCallbacks + def assert_routed_inbound(self, content, router, expected_endpoint): + msg = yield self.router_helper.ri.make_dispatch_inbound( + content, router=router) + emsg = msg.copy() + emsg.set_routing_endpoint(expected_endpoint) + rmsg = self.router_helper.ro.get_dispatched_inbound()[-1] + self.assertEqual(emsg, rmsg) + + @inlineCallbacks + def test_start(self): + router = yield self.router_helper.create_router() + self.assertTrue(router.stopped()) + self.assertFalse(router.running()) + + yield self.router_helper.start_router(router) + router = yield self.router_helper.get_router(router.key) + self.assertFalse(router.stopped()) + self.assertTrue(router.running()) + + @inlineCallbacks + def test_stop(self): + router = yield self.router_helper.create_router(started=True) + self.assertFalse(router.stopped()) + self.assertTrue(router.running()) + + yield self.router_helper.stop_router(router) + router = yield self.router_helper.get_router(router.key) + self.assertTrue(router.stopped()) + self.assertFalse(router.running()) + + @inlineCallbacks + def test_no_messages_processed_while_stopped(self): + router = yield self.router_helper.create_router() + + yield self.router_helper.ri.make_dispatch_inbound("foo", router=router) + self.assertEqual([], self.router_helper.ro.get_dispatched_inbound()) + + yield self.router_helper.ri.make_dispatch_ack(router=router) + self.assertEqual([], self.router_helper.ro.get_dispatched_events()) + + yield self.router_helper.ro.make_dispatch_outbound( + "foo", router=router) + self.assertEqual([], self.router_helper.ri.get_dispatched_outbound()) + [nack] = self.router_helper.ro.get_dispatched_events() + self.assertEqual(nack['event_type'], 'nack') + + @inlineCallbacks + def test_inbound_no_config(self): + router = yield self.router_helper.create_router(started=True) + yield self.assert_routed_inbound("foo bar", router, 'default') + yield self.assert_routed_inbound("baz quux", router, 'default') diff --git a/go/routers/app_multiplexer/view_definition.py b/go/routers/app_multiplexer/view_definition.py new file mode 100644 index 000000000..6443d7ef7 --- /dev/null +++ b/go/routers/app_multiplexer/view_definition.py @@ -0,0 +1,42 @@ +from django import forms + +from go.router.view_definition import RouterViewDefinitionBase, EditRouterView + + +class ApplicationMultiplexerForm(forms.Form): + keyword = forms.CharField() + target_endpoint = forms.CharField() + + +class BaseApplicationMultiplexerFormSet(forms.formsets.BaseFormSet): + @staticmethod + def initial_from_config(data): + return [{'keyword': k, 'target_endpoint': v} + for k, v in sorted(data.items())] + + def to_config(self): + keyword_endpoint_mapping = {} + for form in self: + if (not form.is_valid()) or form.cleaned_data['DELETE']: + continue + keyword = form.cleaned_data['keyword'] + target_endpoint = form.cleaned_data['target_endpoint'] + keyword_endpoint_mapping[keyword] = target_endpoint + return keyword_endpoint_mapping + + +ApplicationMultiplexerFormSet = forms.formsets.formset_factory( + ApplicationMultiplexerForm, + can_delete=True, + extra=1, + formset=BaseApplicationMultiplexerFormSet) + + +class EditApplicationMultiplexerView(EditRouterView): + edit_forms = ( + ('keyword_endpoint_mapping', ApplicationMultiplexerFormSet), + ) + + +class RouterViewDefinition(RouterViewDefinitionBase): + edit_view = EditApplicationMultiplexerView diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py new file mode 100644 index 000000000..6888ce578 --- /dev/null +++ b/go/routers/app_multiplexer/vumi_app.py @@ -0,0 +1,37 @@ +# -*- test-case-name: go.routers.keyword.tests.test_vumi_app -*- +# -*- coding: utf-8 -*- + +from vumi import log +from vumi.config import ConfigDict + +from go.vumitools.app_worker import GoRouterWorker + + +class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): + keyword_endpoint_mapping = ConfigDict( + "Mapping from case-insensitive keyword regex to endpoint name.", + default={}) + + +class ApplicationMultiplexer(GoRouterWorker): + """ + Router that splits inbound messages based on keywords. + """ + CONFIG_CLASS = ApplicationMultiplexerConfig + + worker_name = 'application_multiplexer' + + def lookup_target(self, config, msg): + first_word = ((msg['content'] or '').strip().split() + [''])[0] + for keyword, target in config.keyword_endpoint_mapping.iteritems(): + if keyword.lower() == first_word.lower(): + return target + return 'default' + + def handle_inbound(self, config, msg, conn_name): + log.debug("Handling inbound: %s" % (msg,)) + return self.publish_inbound(msg, self.lookup_target(config, msg)) + + def handle_outbound(self, config, msg, conn_name): + log.debug("Handling outbound: %s" % (msg,)) + return self.publish_outbound(msg, 'default') From 6a5379f9bf5f89b7ce5577b1f88ec99edcc08af6 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 7 Feb 2014 13:21:46 +0200 Subject: [PATCH 02/53] fix requirements.pip and add secret key --- go/settings.py | 2 +- requirements.pip | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go/settings.py b/go/settings.py index a4ec58a38..d669b2afe 100644 --- a/go/settings.py +++ b/go/settings.py @@ -104,7 +104,7 @@ def static_paths(paths): # Make this unique, and don't share it with anybody. # override this in production_settings.py -SECRET_KEY = "" +SECRET_KEY = "b0b3b3" # List of callables that know how to import templates from various sources. TEMPLATE_LOADERS = ( diff --git a/requirements.pip b/requirements.pip index f335e4c5a..78b8945fe 100644 --- a/requirements.pip +++ b/requirements.pip @@ -6,8 +6,8 @@ # This will ignore any existing repository clones. They can be updated manually # if desired. --e git+https://github.com/praekelt/vumi.git#egg=vumi --e git+https://github.com/praekelt/vxpolls.git#egg=vxpolls --e git+https://github.com/praekelt/vumi-wikipedia.git#egg=vumi-wikipedia +-e git+https://github.com/praekelt/vumi.git@develop#egg=vumi +-e git+https://github.com/praekelt/vxpolls.git@develop#egg=vxpolls +-e git+https://github.com/praekelt/vumi-wikipedia.git@develop#egg=vumi-wikipedia --e . \ No newline at end of file +-e . From eadd57d116385a19cb6daeb5cd942de9ac1ff523 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 7 Feb 2014 15:09:36 +0200 Subject: [PATCH 03/53] Tweak logging in settings.py --- go/settings.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/go/settings.py b/go/settings.py index f04128e21..8e5c9db22 100644 --- a/go/settings.py +++ b/go/settings.py @@ -212,10 +212,6 @@ def static_paths(paths): }, }, 'handlers': { - 'mail_admins': { - 'level': 'ERROR', - 'class': 'django.utils.log.AdminEmailHandler' - }, 'console': { 'level': 'DEBUG', 'class': 'logging.StreamHandler', @@ -230,7 +226,7 @@ def static_paths(paths): }, }, 'root': { - 'handlers': ['mail_admins'], + 'handlers': ['console'], 'level': 'ERROR', }, } From 8ada227043d4477ff69be2fd6fd953ca67a5a452 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 11 Feb 2014 20:10:22 +0200 Subject: [PATCH 04/53] Implemented django forms for the router detail view --- go/routers/app_multiplexer/definition.py | 2 +- go/routers/app_multiplexer/view_definition.py | 49 +++++++++++++++---- go/settings.py | 4 +- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/go/routers/app_multiplexer/definition.py b/go/routers/app_multiplexer/definition.py index de6dd684e..447700baf 100644 --- a/go/routers/app_multiplexer/definition.py +++ b/go/routers/app_multiplexer/definition.py @@ -5,4 +5,4 @@ class RouterDefinition(RouterDefinitionBase): router_type = 'application_multiplexer' def configured_outbound_endpoints(self, config): - return list(set(config.get('keyword_endpoint_mapping', {}).values())) + return list(set(config.get('endpoints', {}).values())) diff --git a/go/routers/app_multiplexer/view_definition.py b/go/routers/app_multiplexer/view_definition.py index 6443d7ef7..7df668e73 100644 --- a/go/routers/app_multiplexer/view_definition.py +++ b/go/routers/app_multiplexer/view_definition.py @@ -3,38 +3,69 @@ from go.router.view_definition import RouterViewDefinitionBase, EditRouterView +class ApplicationMultiplexerTitleForm(forms.Form): + menu_title = forms.CharField( + label="Menu Title", + max_length=32 + ) + + class ApplicationMultiplexerForm(forms.Form): - keyword = forms.CharField() - target_endpoint = forms.CharField() + application_title = forms.CharField( + label="Application Title", + max_length=12 + ) + target_endpoint = forms.CharField( + label="Endpoint" + ) class BaseApplicationMultiplexerFormSet(forms.formsets.BaseFormSet): + + def clean(self): + """ + Checks that no two applications have the same title. + """ + if any(self.errors): + return + titles = [] + for i in range(0, self.total_form_count()): + title = self.forms[i].cleaned_data['application_title'] + if title in titles: + raise forms.ValidationError( + "Application titles must be distinct." + ) + titles.append(title) + @staticmethod def initial_from_config(data): - return [{'keyword': k, 'target_endpoint': v} + return [{'application_title': k, 'target_endpoint': v} for k, v in sorted(data.items())] def to_config(self): - keyword_endpoint_mapping = {} - for form in self: + mappings = {} + for form in self.forms: if (not form.is_valid()) or form.cleaned_data['DELETE']: continue - keyword = form.cleaned_data['keyword'] + application_title = form.cleaned_data['application_title'] target_endpoint = form.cleaned_data['target_endpoint'] - keyword_endpoint_mapping[keyword] = target_endpoint - return keyword_endpoint_mapping + mappings[application_title] = target_endpoint + return mappings ApplicationMultiplexerFormSet = forms.formsets.formset_factory( ApplicationMultiplexerForm, can_delete=True, + max_num=8, extra=1, + can_order=True, formset=BaseApplicationMultiplexerFormSet) class EditApplicationMultiplexerView(EditRouterView): edit_forms = ( - ('keyword_endpoint_mapping', ApplicationMultiplexerFormSet), + ('menu_title', ApplicationMultiplexerTitleForm), + ('endpoints', ApplicationMultiplexerFormSet), ) diff --git a/go/settings.py b/go/settings.py index 8e5c9db22..6a7c341b1 100644 --- a/go/settings.py +++ b/go/settings.py @@ -1,4 +1,3 @@ - # Django settings for go project. import os import djcelery @@ -117,7 +116,8 @@ def static_paths(paths): 'debug_toolbar.middleware.DebugToolbarMiddleware', 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.middleware.csrf.CsrfViewMiddleware', + # NOTE: hack to bypass csrf errors + # 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'go.base.middleware.VumiUserApiMiddleware', From 4ba07eda5f1a7cb1ae1dea37856b68d77b18664f Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 14 Feb 2014 09:45:25 +0200 Subject: [PATCH 05/53] Improved validation of user input --- go/routers/app_multiplexer/common.py | 18 +++++++++++ go/routers/app_multiplexer/view_definition.py | 31 ++++++++++++++++--- 2 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 go/routers/app_multiplexer/common.py diff --git a/go/routers/app_multiplexer/common.py b/go/routers/app_multiplexer/common.py new file mode 100644 index 000000000..d16f5f7f0 --- /dev/null +++ b/go/routers/app_multiplexer/common.py @@ -0,0 +1,18 @@ +# helpers from GFM + + +def mkmenu(options, start=1, format='%s) %s'): + items = [format % (idx, opt) for idx, opt in enumerate(options, start)] + return '\n'.join(items) + + +def smart_truncate(content, k, sfx='...'): + """ + Useful for truncating text strings in the following manner: + + 'MyFancyTeam' => 'MyFancyT...' + """ + if len(content) <= k: + return content + else: + return content[:k - len(sfx)] + sfx diff --git a/go/routers/app_multiplexer/view_definition.py b/go/routers/app_multiplexer/view_definition.py index 7df668e73..b47f4a2c2 100644 --- a/go/routers/app_multiplexer/view_definition.py +++ b/go/routers/app_multiplexer/view_definition.py @@ -1,19 +1,19 @@ from django import forms from go.router.view_definition import RouterViewDefinitionBase, EditRouterView +from go.routers.app_multiplexer.common import mkmenu class ApplicationMultiplexerTitleForm(forms.Form): menu_title = forms.CharField( label="Menu Title", - max_length=32 + max_length=29 ) class ApplicationMultiplexerForm(forms.Form): application_title = forms.CharField( label="Application Title", - max_length=12 ) target_endpoint = forms.CharField( label="Endpoint" @@ -22,20 +22,42 @@ class ApplicationMultiplexerForm(forms.Form): class BaseApplicationMultiplexerFormSet(forms.formsets.BaseFormSet): + # TODO: move to a config file if appropriate + # We reserve 130 chars for the menu and 29 chars for the title or + # description of the menu + + MENU_CHAR_LENGTH = 130 + def clean(self): """ Checks that no two applications have the same title. + + We need to verify that application names, and endpoint + names are unique. Also throwing message size check for USSD. """ if any(self.errors): return + + # (Title, Endpoint) uniqueness check titles = [] + endpoints = [] for i in range(0, self.total_form_count()): title = self.forms[i].cleaned_data['application_title'] - if title in titles: + endpoint = self.forms[i].cleaned_data['target_endpoint'] + if title in titles or endpoint in endpoints: raise forms.ValidationError( - "Application titles must be distinct." + "Application titles and endpoints should be distinct." ) titles.append(title) + endpoints.append(endpoint) + + # Menu size check for the benefit of USSD which is usually limited + # to 160 chars + if len(mkmenu(titles)) > self.MENU_CHAR_LENGTH: + raise forms.ValidationError( + "The generated menu is too large. Either reduce the length of " + "application titles or the number of applications." + ) @staticmethod def initial_from_config(data): @@ -56,7 +78,6 @@ def to_config(self): ApplicationMultiplexerFormSet = forms.formsets.formset_factory( ApplicationMultiplexerForm, can_delete=True, - max_num=8, extra=1, can_order=True, formset=BaseApplicationMultiplexerFormSet) From 4430cf82d2844978d893cd08c17167af1c00b326 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 14 Feb 2014 09:45:55 +0200 Subject: [PATCH 06/53] add tests for router config validation --- .../app_multiplexer/tests/test_views.py | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/go/routers/app_multiplexer/tests/test_views.py b/go/routers/app_multiplexer/tests/test_views.py index d6af33c34..1c00f05b4 100644 --- a/go/routers/app_multiplexer/tests/test_views.py +++ b/go/routers/app_multiplexer/tests/test_views.py @@ -1,5 +1,6 @@ from go.base.tests.helpers import GoDjangoTestCase from go.routers.tests.view_helpers import RouterViewsHelper +from go.vumitools.api import VumiApiCommand class ApplicationMultiplexerViewTests(GoDjangoTestCase): @@ -22,3 +23,110 @@ def test_new_router(self): [router_key] = router_store.list_routers() rtr_helper = self.router_helper.get_router_helper_by_key(router_key) self.assertRedirects(response, rtr_helper.get_view_url('edit')) + + def test_show_stopped(self): + rtr_helper = self.router_helper.create_router_helper(name=u"myrouter") + response = self.client.get(rtr_helper.get_view_url('show')) + router = response.context[0].get('router') + self.assertEqual(router.name, u"myrouter") + self.assertContains(response, rtr_helper.get_view_url('start')) + self.assertNotContains(response, rtr_helper.get_view_url('stop')) + + def test_show_running(self): + rtr_helper = self.router_helper.create_router_helper( + name=u"myrouter", started=True) + response = self.client.get(rtr_helper.get_view_url('show')) + router = response.context[0].get('router') + self.assertEqual(router.name, u"myrouter") + self.assertNotContains(response, rtr_helper.get_view_url('start')) + self.assertContains(response, rtr_helper.get_view_url('stop')) + + def test_start(self): + rtr_helper = self.router_helper.create_router_helper(started=False) + + response = self.client.post(rtr_helper.get_view_url('start')) + self.assertRedirects(response, rtr_helper.get_view_url('show')) + router = rtr_helper.get_router() + self.assertTrue(router.starting()) + [start_cmd] = self.router_helper.get_api_commands_sent() + self.assertEqual( + start_cmd, VumiApiCommand.command( + '%s_router' % (router.router_type,), 'start', + user_account_key=router.user_account.key, + router_key=router.key)) + + def test_stop(self): + rtr_helper = self.router_helper.create_router_helper(started=True) + + response = self.client.post(rtr_helper.get_view_url('stop')) + self.assertRedirects(response, rtr_helper.get_view_url('show')) + router = rtr_helper.get_router() + self.assertTrue(router.stopping()) + [start_cmd] = self.router_helper.get_api_commands_sent() + self.assertEqual( + start_cmd, VumiApiCommand.command( + '%s_router' % (router.router_type,), 'stop', + user_account_key=router.user_account.key, + router_key=router.key)) + + def test_get_edit_empty_config(self): + rtr_helper = self.router_helper.create_router_helper(started=True) + response = self.client.get(rtr_helper.get_view_url('edit')) + self.assertEqual(response.status_code, 200) + + def test_get_edit_small_config(self): + rtr_helper = self.router_helper.create_router_helper( + started=True, config={ + 'endpoints': { + 'mykeyw[o0]rd': 'target_endpoint', + }, + }) + response = self.client.get(rtr_helper.get_view_url('edit')) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'mykeyw[o0]rd') + self.assertContains(response, 'target_endpoint') + + def test_config_validate_unique_endpoints(self): + rtr_helper = self.router_helper.create_router_helper(started=True) + router = rtr_helper.get_router() + self.assertEqual(router.config, {}) + response = self.client.post(rtr_helper.get_view_url('edit'), { + 'menu_title-menu_title': ['foo'], + + 'endpoints-TOTAL_FORMS': ['2'], + 'endpoints-INITIAL_FORMS': ['0'], + 'endpoints-MAX_NUM_FORMS': [''], + 'endpoints-0-application_title': ['foo'], + 'endpoints-0-target_endpoint': ['cat'], + 'endpoints-0-DELETE': [''], + 'endpoints-1-application_title': ['foo'], + 'endpoints-1-target_endpoint': ['dog'], + 'endpoints-1-DELETE': [''], + }) + self.assertEqual(response.status_code, 200) + self.assertContains(response, ('Application titles and endpoints ' + 'should be distinct.')) + + def test_config_validate_menu_length(self): + rtr_helper = self.router_helper.create_router_helper(started=True) + router = rtr_helper.get_router() + self.assertEqual(router.config, {}) + + fields = { + 'menu_title-menu_title': ['Please select:'], + 'endpoints-TOTAL_FORMS': ['60'], + 'endpoints-INITIAL_FORMS': ['0'], + 'endpoints-MAX_NUM_FORMS': [''], + } + for i in range(0, 60): + fields.update({ + 'endpoints-%s-application_title' % i: ['foo-%s' % i], + 'endpoints-%s-target_endpoint' % i: ['bar-%s' % i], + 'endpoints-%s-DELETE' % i: [''], + }) + response = self.client.post(rtr_helper.get_view_url('edit'), fields) + self.assertEqual(response.status_code, 200) + self.assertContains(response, + ("The generated menu is too large. Either reduce " + "the length of application titles or the number " + "of applications.")) From f4784c60d3cccf674959a5f1c69e7234bca52ca8 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 14 Feb 2014 09:47:35 +0200 Subject: [PATCH 07/53] hmm, hacked go.testsettings for some logging issue --- go/testsettings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/testsettings.py b/go/testsettings.py index 6eee4a023..375f85899 100644 --- a/go/testsettings.py +++ b/go/testsettings.py @@ -62,7 +62,7 @@ # disable console logging to avoid log messages messing up test output LOGGING['loggers']['go']['handlers'].remove('console') -LOGGING['root']['handlers'].remove('mail_admins') +#LOGGING['root']['handlers'].remove('mail_admins') # disable response-time middleware during tests DISALLOWED_MIDDLEWARE = set([ From cce9f7fe5978913c8e87e713242ada49f3bc585b Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 14 Feb 2014 09:48:17 +0200 Subject: [PATCH 08/53] Add non_field_errors to router edit template --- go/router/templates/router/edit.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/router/templates/router/edit.html b/go/router/templates/router/edit.html index 8c3f3f5c3..4e27b636e 100644 --- a/go/router/templates/router/edit.html +++ b/go/router/templates/router/edit.html @@ -24,6 +24,8 @@ {% for edit_form in edit_forms %}
{{ edit_form|crispy }} + + {{ edit_form.non_form_errors }}
{% endfor %} From a9b1875a974b038509336ca1c456c04cbd3b6ae7 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 17 Feb 2014 15:06:44 +0200 Subject: [PATCH 09/53] Rename multiplexer directory for more consistency --- .../{app_multiplexer => application_multiplexer}/__init__.py | 0 go/routers/{app_multiplexer => application_multiplexer}/common.py | 0 .../{app_multiplexer => application_multiplexer}/definition.py | 0 .../tests/__init__.py | 0 .../tests/test_views.py | 0 .../tests/test_vumi_app.py | 0 .../view_definition.py | 0 .../{app_multiplexer => application_multiplexer}/vumi_app.py | 0 8 files changed, 0 insertions(+), 0 deletions(-) rename go/routers/{app_multiplexer => application_multiplexer}/__init__.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/common.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/definition.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/tests/__init__.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/tests/test_views.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/tests/test_vumi_app.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/view_definition.py (100%) rename go/routers/{app_multiplexer => application_multiplexer}/vumi_app.py (100%) diff --git a/go/routers/app_multiplexer/__init__.py b/go/routers/application_multiplexer/__init__.py similarity index 100% rename from go/routers/app_multiplexer/__init__.py rename to go/routers/application_multiplexer/__init__.py diff --git a/go/routers/app_multiplexer/common.py b/go/routers/application_multiplexer/common.py similarity index 100% rename from go/routers/app_multiplexer/common.py rename to go/routers/application_multiplexer/common.py diff --git a/go/routers/app_multiplexer/definition.py b/go/routers/application_multiplexer/definition.py similarity index 100% rename from go/routers/app_multiplexer/definition.py rename to go/routers/application_multiplexer/definition.py diff --git a/go/routers/app_multiplexer/tests/__init__.py b/go/routers/application_multiplexer/tests/__init__.py similarity index 100% rename from go/routers/app_multiplexer/tests/__init__.py rename to go/routers/application_multiplexer/tests/__init__.py diff --git a/go/routers/app_multiplexer/tests/test_views.py b/go/routers/application_multiplexer/tests/test_views.py similarity index 100% rename from go/routers/app_multiplexer/tests/test_views.py rename to go/routers/application_multiplexer/tests/test_views.py diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py similarity index 100% rename from go/routers/app_multiplexer/tests/test_vumi_app.py rename to go/routers/application_multiplexer/tests/test_vumi_app.py diff --git a/go/routers/app_multiplexer/view_definition.py b/go/routers/application_multiplexer/view_definition.py similarity index 100% rename from go/routers/app_multiplexer/view_definition.py rename to go/routers/application_multiplexer/view_definition.py diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py similarity index 100% rename from go/routers/app_multiplexer/vumi_app.py rename to go/routers/application_multiplexer/vumi_app.py From cdb71d14800e769ac28aed7edfb18a9797c45e33 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 17 Feb 2014 18:14:11 +0200 Subject: [PATCH 10/53] Change various config-related field names --- .../view_definition.py | 78 ++++++------------- 1 file changed, 23 insertions(+), 55 deletions(-) diff --git a/go/routers/application_multiplexer/view_definition.py b/go/routers/application_multiplexer/view_definition.py index b47f4a2c2..9ed3d93ea 100644 --- a/go/routers/application_multiplexer/view_definition.py +++ b/go/routers/application_multiplexer/view_definition.py @@ -1,92 +1,60 @@ from django import forms from go.router.view_definition import RouterViewDefinitionBase, EditRouterView -from go.routers.app_multiplexer.common import mkmenu class ApplicationMultiplexerTitleForm(forms.Form): menu_title = forms.CharField( - label="Menu Title", - max_length=29 + label="Menu title", + max_length=100, ) class ApplicationMultiplexerForm(forms.Form): - application_title = forms.CharField( - label="Application Title", + application_label = forms.CharField( + label="Application label" ) - target_endpoint = forms.CharField( - label="Endpoint" + endpoint_name = forms.CharField( + label="Endpoint name" ) class BaseApplicationMultiplexerFormSet(forms.formsets.BaseFormSet): - # TODO: move to a config file if appropriate - # We reserve 130 chars for the menu and 29 chars for the title or - # description of the menu - - MENU_CHAR_LENGTH = 130 - - def clean(self): - """ - Checks that no two applications have the same title. - - We need to verify that application names, and endpoint - names are unique. Also throwing message size check for USSD. - """ - if any(self.errors): - return - - # (Title, Endpoint) uniqueness check - titles = [] - endpoints = [] - for i in range(0, self.total_form_count()): - title = self.forms[i].cleaned_data['application_title'] - endpoint = self.forms[i].cleaned_data['target_endpoint'] - if title in titles or endpoint in endpoints: - raise forms.ValidationError( - "Application titles and endpoints should be distinct." - ) - titles.append(title) - endpoints.append(endpoint) - - # Menu size check for the benefit of USSD which is usually limited - # to 160 chars - if len(mkmenu(titles)) > self.MENU_CHAR_LENGTH: - raise forms.ValidationError( - "The generated menu is too large. Either reduce the length of " - "application titles or the number of applications." - ) - @staticmethod def initial_from_config(data): - return [{'application_title': k, 'target_endpoint': v} - for k, v in sorted(data.items())] + initial_data = [] + for entry in data: + initial_data.append({ + 'application_label': entry['label'], + 'endpoint_name': entry['endpoint'], + }) + return initial_data def to_config(self): - mappings = {} - for form in self.forms: - if (not form.is_valid()) or form.cleaned_data['DELETE']: + entries = [] + for form in self.ordered_forms: + if not form.is_valid(): continue - application_title = form.cleaned_data['application_title'] - target_endpoint = form.cleaned_data['target_endpoint'] - mappings[application_title] = target_endpoint - return mappings + entries.append({ + "label": form.cleaned_data['application_label'], + "endpoint": form.cleaned_data['endpoint_name'], + }) + return entries ApplicationMultiplexerFormSet = forms.formsets.formset_factory( ApplicationMultiplexerForm, can_delete=True, - extra=1, can_order=True, + extra=1, formset=BaseApplicationMultiplexerFormSet) class EditApplicationMultiplexerView(EditRouterView): edit_forms = ( ('menu_title', ApplicationMultiplexerTitleForm), - ('endpoints', ApplicationMultiplexerFormSet), + ('entries', ApplicationMultiplexerFormSet), ) From dfa3e67dbffe2242df6a7ccf79d0cc4b9ceaaeee Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 17 Feb 2014 18:19:29 +0200 Subject: [PATCH 11/53] Move all form-related stuff to a new forms.py file --- go/routers/application_multiplexer/forms.py | 49 ++++++++++++++++++ .../view_definition.py | 51 +------------------ 2 files changed, 51 insertions(+), 49 deletions(-) create mode 100644 go/routers/application_multiplexer/forms.py diff --git a/go/routers/application_multiplexer/forms.py b/go/routers/application_multiplexer/forms.py new file mode 100644 index 000000000..7398a6bef --- /dev/null +++ b/go/routers/application_multiplexer/forms.py @@ -0,0 +1,49 @@ +from django import forms + + +class ApplicationMultiplexerTitleForm(forms.Form): + menu_title = forms.CharField( + label="Menu title", + max_length=100, + ) + + +class ApplicationMultiplexerForm(forms.Form): + application_label = forms.CharField( + label="Application label" + ) + endpoint_name = forms.CharField( + label="Endpoint name" + ) + + +class BaseApplicationMultiplexerFormSet(forms.formsets.BaseFormSet): + + @staticmethod + def initial_from_config(data): + initial_data = [] + for entry in data: + initial_data.append({ + 'application_label': entry['label'], + 'endpoint_name': entry['endpoint'], + }) + return initial_data + + def to_config(self): + entries = [] + for form in self.ordered_forms: + if not form.is_valid(): + continue + entries.append({ + "label": form.cleaned_data['application_label'], + "endpoint": form.cleaned_data['endpoint_name'], + }) + return entries + + +ApplicationMultiplexerFormSet = forms.formsets.formset_factory( + ApplicationMultiplexerForm, + can_delete=True, + can_order=True, + extra=1, + formset=BaseApplicationMultiplexerFormSet) diff --git a/go/routers/application_multiplexer/view_definition.py b/go/routers/application_multiplexer/view_definition.py index 9ed3d93ea..1aa22974d 100644 --- a/go/routers/application_multiplexer/view_definition.py +++ b/go/routers/application_multiplexer/view_definition.py @@ -1,54 +1,7 @@ -from django import forms - from go.router.view_definition import RouterViewDefinitionBase, EditRouterView - -class ApplicationMultiplexerTitleForm(forms.Form): - menu_title = forms.CharField( - label="Menu title", - max_length=100, - ) - - -class ApplicationMultiplexerForm(forms.Form): - application_label = forms.CharField( - label="Application label" - ) - endpoint_name = forms.CharField( - label="Endpoint name" - ) - - -class BaseApplicationMultiplexerFormSet(forms.formsets.BaseFormSet): - - @staticmethod - def initial_from_config(data): - initial_data = [] - for entry in data: - initial_data.append({ - 'application_label': entry['label'], - 'endpoint_name': entry['endpoint'], - }) - return initial_data - - def to_config(self): - entries = [] - for form in self.ordered_forms: - if not form.is_valid(): - continue - entries.append({ - "label": form.cleaned_data['application_label'], - "endpoint": form.cleaned_data['endpoint_name'], - }) - return entries - - -ApplicationMultiplexerFormSet = forms.formsets.formset_factory( - ApplicationMultiplexerForm, - can_delete=True, - can_order=True, - extra=1, - formset=BaseApplicationMultiplexerFormSet) +from go.routers.application_multiplexer import \ + (ApplicationMultiplexerTitleForm, ApplicationMultiplexerFormSet) class EditApplicationMultiplexerView(EditRouterView): From c2ab39d2ba75cdca2c5c10fe46e81e1a7ad4954f Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 17 Feb 2014 21:45:21 +0200 Subject: [PATCH 12/53] remove local dev hacks from the various configs --- go/settings.py | 14 ++++++++++---- go/testsettings.py | 4 +++- requirements.pip | 8 ++++---- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/go/settings.py b/go/settings.py index 6a7c341b1..a4ec58a38 100644 --- a/go/settings.py +++ b/go/settings.py @@ -1,3 +1,4 @@ + # Django settings for go project. import os import djcelery @@ -103,7 +104,7 @@ def static_paths(paths): # Make this unique, and don't share it with anybody. # override this in production_settings.py -SECRET_KEY = "b0b3b3" +SECRET_KEY = "" # List of callables that know how to import templates from various sources. TEMPLATE_LOADERS = ( @@ -116,8 +117,7 @@ def static_paths(paths): 'debug_toolbar.middleware.DebugToolbarMiddleware', 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', - # NOTE: hack to bypass csrf errors - # 'django.middleware.csrf.CsrfViewMiddleware', + 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'go.base.middleware.VumiUserApiMiddleware', @@ -158,6 +158,7 @@ def static_paths(paths): 'django.contrib.admindocs', 'south', 'gunicorn', + 'django_nose', 'djcelery', 'djcelery_email', 'crispy_forms', @@ -212,6 +213,10 @@ def static_paths(paths): }, }, 'handlers': { + 'mail_admins': { + 'level': 'ERROR', + 'class': 'django.utils.log.AdminEmailHandler' + }, 'console': { 'level': 'DEBUG', 'class': 'logging.StreamHandler', @@ -226,12 +231,13 @@ def static_paths(paths): }, }, 'root': { - 'handlers': ['console'], + 'handlers': ['mail_admins'], 'level': 'ERROR', }, } +TEST_RUNNER = 'django_nose.NoseTestSuiteRunner' SKIP_SOUTH_TESTS = True SOUTH_TESTS_MIGRATE = False SOUTH_MIGRATION_MODULES = { diff --git a/go/testsettings.py b/go/testsettings.py index 375f85899..30edfffa6 100644 --- a/go/testsettings.py +++ b/go/testsettings.py @@ -57,12 +57,14 @@ EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' +NOSE_ARGS = ['-evumitools', '-evumi_app', '-ehandlers', '-m^test'] + MESSAGE_STORAGE = 'django.contrib.messages.storage.cookie.CookieStorage' STATICFILES_STORAGE = 'pipeline.storage.NonPackagingPipelineStorage' # disable console logging to avoid log messages messing up test output LOGGING['loggers']['go']['handlers'].remove('console') -#LOGGING['root']['handlers'].remove('mail_admins') +LOGGING['root']['handlers'].remove('mail_admins') # disable response-time middleware during tests DISALLOWED_MIDDLEWARE = set([ diff --git a/requirements.pip b/requirements.pip index 78b8945fe..f335e4c5a 100644 --- a/requirements.pip +++ b/requirements.pip @@ -6,8 +6,8 @@ # This will ignore any existing repository clones. They can be updated manually # if desired. --e git+https://github.com/praekelt/vumi.git@develop#egg=vumi --e git+https://github.com/praekelt/vxpolls.git@develop#egg=vxpolls --e git+https://github.com/praekelt/vumi-wikipedia.git@develop#egg=vumi-wikipedia +-e git+https://github.com/praekelt/vumi.git#egg=vumi +-e git+https://github.com/praekelt/vxpolls.git#egg=vxpolls +-e git+https://github.com/praekelt/vumi-wikipedia.git#egg=vumi-wikipedia --e . +-e . \ No newline at end of file From 26ca348711ad26545d8337b989fb4011a46095aa Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 17 Feb 2014 21:56:15 +0200 Subject: [PATCH 13/53] Update tests as per changes in the view definition --- go/config.py | 2 +- .../tests/test_views.py | 72 +++++++------------ 2 files changed, 28 insertions(+), 46 deletions(-) diff --git a/go/config.py b/go/config.py index 9a1e58fa2..676eb7072 100644 --- a/go/config.py +++ b/go/config.py @@ -136,7 +136,7 @@ def get_router_definition(router_type, router=None): 'namespace': 'keyword', 'display_name': 'Keyword', }, - 'go.routers.app_multiplexer': { + 'go.routers.application_multiplexer': { 'namespace': 'application_multiplexer', 'display_name': 'Application Multiplexer', }, diff --git a/go/routers/application_multiplexer/tests/test_views.py b/go/routers/application_multiplexer/tests/test_views.py index 1c00f05b4..d204cd225 100644 --- a/go/routers/application_multiplexer/tests/test_views.py +++ b/go/routers/application_multiplexer/tests/test_views.py @@ -69,64 +69,46 @@ def test_stop(self): user_account_key=router.user_account.key, router_key=router.key)) - def test_get_edit_empty_config(self): - rtr_helper = self.router_helper.create_router_helper(started=True) - response = self.client.get(rtr_helper.get_view_url('edit')) - self.assertEqual(response.status_code, 200) - - def test_get_edit_small_config(self): + def test_initial_config(self): rtr_helper = self.router_helper.create_router_helper( started=True, config={ - 'endpoints': { - 'mykeyw[o0]rd': 'target_endpoint', - }, + 'menu_title': 'Please select an application', + 'entries': [ + { + 'label': 'Flappy Bird', + 'endpoint': 'flappy-bird', + + }, + ] }) response = self.client.get(rtr_helper.get_view_url('edit')) + self.assertContains(response, 'Please select an application') self.assertEqual(response.status_code, 200) - self.assertContains(response, 'mykeyw[o0]rd') - self.assertContains(response, 'target_endpoint') + self.assertContains(response, 'Flappy Bird') + self.assertContains(response, 'flappy-bird') - def test_config_validate_unique_endpoints(self): + def test_initial_config_empty(self): + rtr_helper = self.router_helper.create_router_helper(started=True) + response = self.client.get(rtr_helper.get_view_url('edit')) + self.assertEqual(response.status_code, 200) + + def test_user_input_good(self): rtr_helper = self.router_helper.create_router_helper(started=True) router = rtr_helper.get_router() self.assertEqual(router.config, {}) response = self.client.post(rtr_helper.get_view_url('edit'), { - 'menu_title-menu_title': ['foo'], - + 'menu_title-menu_title': ['Please select an application'], 'endpoints-TOTAL_FORMS': ['2'], 'endpoints-INITIAL_FORMS': ['0'], 'endpoints-MAX_NUM_FORMS': [''], - 'endpoints-0-application_title': ['foo'], - 'endpoints-0-target_endpoint': ['cat'], + 'endpoints-0-application_label': ['Flappy Bird'], + 'endpoints-0-endpoint_name': ['flappy-bird'], 'endpoints-0-DELETE': [''], - 'endpoints-1-application_title': ['foo'], - 'endpoints-1-target_endpoint': ['dog'], - 'endpoints-1-DELETE': [''], + 'endpoints-0-ORDER': ['0'], + 'endpoints-1-application_label': ['Mama'], + 'endpoints-1-endpoint_name': ['mama'], + 'endpoints-1-ORDER': ['1'], }) self.assertEqual(response.status_code, 200) - self.assertContains(response, ('Application titles and endpoints ' - 'should be distinct.')) - - def test_config_validate_menu_length(self): - rtr_helper = self.router_helper.create_router_helper(started=True) - router = rtr_helper.get_router() - self.assertEqual(router.config, {}) - - fields = { - 'menu_title-menu_title': ['Please select:'], - 'endpoints-TOTAL_FORMS': ['60'], - 'endpoints-INITIAL_FORMS': ['0'], - 'endpoints-MAX_NUM_FORMS': [''], - } - for i in range(0, 60): - fields.update({ - 'endpoints-%s-application_title' % i: ['foo-%s' % i], - 'endpoints-%s-target_endpoint' % i: ['bar-%s' % i], - 'endpoints-%s-DELETE' % i: [''], - }) - response = self.client.post(rtr_helper.get_view_url('edit'), fields) - self.assertEqual(response.status_code, 200) - self.assertContains(response, - ("The generated menu is too large. Either reduce " - "the length of application titles or the number " - "of applications.")) + self.assertContains(response, 'Flappy Bird') + self.assertContains(response, 'Mama') From 370dd5d59ccddb0da4e1f32ba6c7d71a550c066e Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 17 Feb 2014 23:12:08 +0200 Subject: [PATCH 14/53] Get tests working again after modifications to the view --- go/routers/application_multiplexer/forms.py | 4 +- .../tests/test_views.py | 46 +++++++++++-------- .../tests/test_vumi_app.py | 2 +- .../view_definition.py | 2 +- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/go/routers/application_multiplexer/forms.py b/go/routers/application_multiplexer/forms.py index 7398a6bef..d9ba3433b 100644 --- a/go/routers/application_multiplexer/forms.py +++ b/go/routers/application_multiplexer/forms.py @@ -2,9 +2,9 @@ class ApplicationMultiplexerTitleForm(forms.Form): - menu_title = forms.CharField( + content = forms.CharField( label="Menu title", - max_length=100, + max_length=100 ) diff --git a/go/routers/application_multiplexer/tests/test_views.py b/go/routers/application_multiplexer/tests/test_views.py index d204cd225..34e5b7b2f 100644 --- a/go/routers/application_multiplexer/tests/test_views.py +++ b/go/routers/application_multiplexer/tests/test_views.py @@ -72,18 +72,17 @@ def test_stop(self): def test_initial_config(self): rtr_helper = self.router_helper.create_router_helper( started=True, config={ - 'menu_title': 'Please select an application', + 'menu_title': {'content': 'Please select an application'}, 'entries': [ { 'label': 'Flappy Bird', 'endpoint': 'flappy-bird', }, - ] - }) + ]}) response = self.client.get(rtr_helper.get_view_url('edit')) - self.assertContains(response, 'Please select an application') self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Please select an application') self.assertContains(response, 'Flappy Bird') self.assertContains(response, 'flappy-bird') @@ -97,18 +96,29 @@ def test_user_input_good(self): router = rtr_helper.get_router() self.assertEqual(router.config, {}) response = self.client.post(rtr_helper.get_view_url('edit'), { - 'menu_title-menu_title': ['Please select an application'], - 'endpoints-TOTAL_FORMS': ['2'], - 'endpoints-INITIAL_FORMS': ['0'], - 'endpoints-MAX_NUM_FORMS': [''], - 'endpoints-0-application_label': ['Flappy Bird'], - 'endpoints-0-endpoint_name': ['flappy-bird'], - 'endpoints-0-DELETE': [''], - 'endpoints-0-ORDER': ['0'], - 'endpoints-1-application_label': ['Mama'], - 'endpoints-1-endpoint_name': ['mama'], - 'endpoints-1-ORDER': ['1'], + 'menu_title-content': ['Please select an application'], + 'entries-TOTAL_FORMS': ['2'], + 'entries-INITIAL_FORMS': ['0'], + 'entries-MAX_NUM_FORMS': [''], + 'entries-0-application_label': ['Flappy Bird'], + 'entries-0-endpoint_name': ['flappy-bird'], + 'entries-0-DELETE': [''], + 'entries-0-ORDER': ['0'], + 'entries-1-application_label': ['Mama'], + 'entries-1-endpoint_name': ['mama'], + 'entries-1-DELETE': [''], + 'entries-1-ORDER': ['1'], }) - self.assertEqual(response.status_code, 200) - self.assertContains(response, 'Flappy Bird') - self.assertContains(response, 'Mama') + self.assertRedirects(response, rtr_helper.get_view_url('show')) + router = rtr_helper.get_router() + self.assertEqual(router.config, { + u'menu_title': {u'content': u'Please select an application'}, + u'entries': [ + { + u'label': u'Flappy Bird', + u'endpoint': u'flappy-bird', + }, + { + u'label': u'Mama', + u'endpoint': u'mama', + }]}) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index 62dd808bf..f2c523b5e 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -2,7 +2,7 @@ from vumi.tests.helpers import VumiTestCase -from go.routers.app_multiplexer.vumi_app import ApplicationMultiplexer +from go.routers.application_multiplexer.vumi_app import ApplicationMultiplexer from go.routers.tests.helpers import RouterWorkerHelper diff --git a/go/routers/application_multiplexer/view_definition.py b/go/routers/application_multiplexer/view_definition.py index 1aa22974d..45e324835 100644 --- a/go/routers/application_multiplexer/view_definition.py +++ b/go/routers/application_multiplexer/view_definition.py @@ -1,6 +1,6 @@ from go.router.view_definition import RouterViewDefinitionBase, EditRouterView -from go.routers.application_multiplexer import \ +from go.routers.application_multiplexer.forms import \ (ApplicationMultiplexerTitleForm, ApplicationMultiplexerFormSet) From e4cf4412cc294ca4072422a8b2af51b8dfa319a7 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 18 Feb 2014 12:24:43 +0200 Subject: [PATCH 15/53] Only include non_field_errors if there actually errors --- go/router/templates/router/edit.html | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/go/router/templates/router/edit.html b/go/router/templates/router/edit.html index 4e27b636e..92e413a52 100644 --- a/go/router/templates/router/edit.html +++ b/go/router/templates/router/edit.html @@ -23,9 +23,10 @@
{% for edit_form in edit_forms %}
+ {% if edit_form.non_non_field_errors %} + {{ edit_form.non_field_errors }} + {% endif %} {{ edit_form|crispy }} - - {{ edit_form.non_form_errors }}
{% endfor %}
From a68f96c005abfb549ccf669f3bb4ac27cac023ab Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 18 Feb 2014 12:46:58 +0200 Subject: [PATCH 16/53] remove some obsolete Nose stuff which crept in via a bad merge --- go/settings.py | 3 --- go/testsettings.py | 2 -- 2 files changed, 5 deletions(-) diff --git a/go/settings.py b/go/settings.py index ca259ff49..518b8e1ae 100644 --- a/go/settings.py +++ b/go/settings.py @@ -1,4 +1,3 @@ - # Django settings for go project. import os import djcelery @@ -158,7 +157,6 @@ def static_paths(paths): 'django.contrib.admindocs', 'south', 'gunicorn', - 'django_nose', 'djcelery', 'djcelery_email', 'crispy_forms', @@ -241,7 +239,6 @@ def static_paths(paths): } -TEST_RUNNER = 'django_nose.NoseTestSuiteRunner' SKIP_SOUTH_TESTS = True SOUTH_TESTS_MIGRATE = False SOUTH_MIGRATION_MODULES = { diff --git a/go/testsettings.py b/go/testsettings.py index 59fc10c7f..52bfe024f 100644 --- a/go/testsettings.py +++ b/go/testsettings.py @@ -57,8 +57,6 @@ EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' -NOSE_ARGS = ['-evumitools', '-evumi_app', '-ehandlers', '-m^test'] - MESSAGE_STORAGE = 'django.contrib.messages.storage.cookie.CookieStorage' STATICFILES_STORAGE = 'pipeline.storage.NonPackagingPipelineStorage' From 3f2656a773a6b030703015a6e1166ee5829f298f Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 12:21:05 +0200 Subject: [PATCH 17/53] Flesh out the router worker to implement basic routing --- go/routers/application_multiplexer/common.py | 6 +- .../application_multiplexer/definition.py | 3 +- .../application_multiplexer/vumi_app.py | 224 ++++++++++++++++-- 3 files changed, 216 insertions(+), 17 deletions(-) diff --git a/go/routers/application_multiplexer/common.py b/go/routers/application_multiplexer/common.py index d16f5f7f0..5028d7761 100644 --- a/go/routers/application_multiplexer/common.py +++ b/go/routers/application_multiplexer/common.py @@ -1,4 +1,8 @@ -# helpers from GFM +# helpers lifted from GFM and Wikipedia + + +def clean(content): + return (content or '').strip() def mkmenu(options, start=1, format='%s) %s'): diff --git a/go/routers/application_multiplexer/definition.py b/go/routers/application_multiplexer/definition.py index 447700baf..dd7496b7b 100644 --- a/go/routers/application_multiplexer/definition.py +++ b/go/routers/application_multiplexer/definition.py @@ -5,4 +5,5 @@ class RouterDefinition(RouterDefinitionBase): router_type = 'application_multiplexer' def configured_outbound_endpoints(self, config): - return list(set(config.get('endpoints', {}).values())) + endpoints = [entry['endpoint'] for entry in config.get('entries', [])] + return list(set(endpoints)) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index 6888ce578..d14b44d75 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -1,16 +1,49 @@ -# -*- test-case-name: go.routers.keyword.tests.test_vumi_app -*- -# -*- coding: utf-8 -*- +# Local Variables: +# test-case-name: go.routers.application_multiplexer.tests.test_vumi_app +# coding: utf-8 +# End: + +from twisted.internet.defer import inlineCallbacks, returnValue from vumi import log -from vumi.config import ConfigDict +from vumi.config import ConfigDict, ConfigList, ConfigInt, ConfigText +from vumi.components.session import SessionManager +from vumi.message import TransportUserMessage from go.vumitools.app_worker import GoRouterWorker +from go.routers.application_multiplexer.common import mkmenu, clean class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): - keyword_endpoint_mapping = ConfigDict( - "Mapping from case-insensitive keyword regex to endpoint name.", - default={}) + + # Static configuration + session_expiry = ConfigInt( + "Maximum amount of time in seconds to keep session data around", + default=1800, static=True) + + session_data_version = ConfigInt( + "Just in case we need to modify the schema of session storage", + default=1, static=True) + + # Dynamic, per-message configuration + menu_title = ConfigDict( + "Content for the menu title", + default={'content': "Please select a choice."}) + entries = ConfigList( + "A list of application endpoints and associated labels", + default=[]) + keyword = ConfigText( + "Keyword to signal a request to return to the application menu", + default=':menu') + invalid_input_message = ConfigText( + "Prompt to display when warning about an invalid choice", + default=("That is an incorrect choice. Please enter the number " + "next to the menu item you wish to choose.\n\n 1) Try Again")) + error_message = ConfigText( + ("Prompt to display when a configuration change invalidates " + "an active session."), + default=("Oops! We experienced a temporary error. " + "Please dial the line again.")) class ApplicationMultiplexer(GoRouterWorker): @@ -21,17 +54,178 @@ class ApplicationMultiplexer(GoRouterWorker): worker_name = 'application_multiplexer' - def lookup_target(self, config, msg): - first_word = ((msg['content'] or '').strip().split() + [''])[0] - for keyword, target in config.keyword_endpoint_mapping.iteritems(): - if keyword.lower() == first_word.lower(): - return target - return 'default' + STATE_START = "start" + STATE_SELECT = "select" + STATE_SELECTED = "selected" + STATE_BAD_INPUT = "bad_input" + + HANDLERS = { + STATE_START: 'handle_state_start', + STATE_SELECT: 'handle_state_select', + STATE_SELECTED: 'handle_state_selected', + STATE_BAD_INPUT: 'handle_state_bad_input' + } + + def setup_router(self): + return super(ApplicationMultiplexer, self).setup_router() + def teardown_router(self): + return super(ApplicationMultiplexer, self).teardown_router() + + def session_manager(self, config): + """ + The implementation of SessionManager does the job of + appending ':session' to key names. + """ + return SessionManager.from_redis_config( + config.redis_manager, + max_session_length=config.session_expiry + ) + + def valid_target_endpoint(self, config, active_endpoint): + """ + Make sure the currently active endpoint is still valid. + """ + endpoints = set([entry['endpoint'] for entry in config.entries]) + if active_endpoint not in endpoints: + return False + return True + + @inlineCallbacks def handle_inbound(self, config, msg, conn_name): - log.debug("Handling inbound: %s" % (msg,)) - return self.publish_inbound(msg, self.lookup_target(config, msg)) + log.msg("Processing inbound message: %s" % (msg,)) + + user_id = msg['from_addr'] + session_manager = self.session_manager(config) + + session = yield session_manager.load_session(user_id) + if session is None: + log.msg("Creating session for user %s" % user_id) + state = self.STATE_START + session = { + 'version': config.session_data_version, + } + yield session_manager.create_session(user_id, **session) + else: + log.msg("Loading session for user %s: %s" % (session,)) + state = session['state'] + + try: + handler = getattr(self, self.HANDLERS[state]) + result = yield handler(config, session, msg) + if type(result) is tuple: + next_state = session['state'] = result[0] + session.update(result[1]) + else: + next_state = session['state'] = result + if state != next_state: + log.msg("State transition for user %s: %s => %s" + (user_id, state, next_state)) + except: + log.err() + yield session_manager.clear_session(user_id) + reply_msg = msg.reply( + config.error_message, + continue_session=False + ) + self.publish_outbound(reply_msg, 'default') + else: + yield session_manager.save_session(user_id, **session) + + @inlineCallbacks + def handle_state_start(self, config, session, msg): + reply_msg = msg.reply(self.create_menu(config)) + yield self.publish_outbound(reply_msg, 'default') + returnValue(self.STATE_CHOOSE) + + @inlineCallbacks + def handle_state_select(self, config, session, msg): + """ + NOTE: There is an edge case in which the user input no longer + matches an entry in the current config. The impact depends + on the number of users and how often the router config is modified. + + One solution would be to compare hashes of the menu configs as + they existed at the START and SELECT states. If there is a mismatch, + invalidate the current session. + """ + choice = self.get_menu_choice(msg, (1, len(config.entries))) + if choice is None: + reply_msg = msg.reply(config.invalid_input_message) + yield self.publish_outbound(reply_msg, 'default') + returnValue(self.STATE_BAD_CHOICE) + else: + endpoint = config.entries[choice - 1]['endpoint'] + forwarded_msg = TransportUserMessage(**msg.payload) + forwarded_msg['content'] = None + forwarded_msg['session_event'] = TransportUserMessage.SESSION_START + yield self.publish_inbound(forwarded_msg, endpoint) + log.msg("Switched to endpoint '%s' for user %s" % + (endpoint, msg['from_addr'])) + returnValue((self.STATE_SELECTED, dict(active_endpoint=endpoint))) + + @inlineCallbacks + def handle_state_selected(self, config, session, msg): + active_endpoint = session['active_endpoint'] + if not self.valid_target_endpoint(config, active_endpoint): + reply_msg = msg.reply( + config.error_message, + continue_session=False + ) + yield self.publish_outbound(reply_msg, 'default') + returnValue(None) + elif self.scan_for_keywords(config, msg, (config.keyword,)): + reply_msg = msg.reply(self.create_menu(config)) + yield self.publish_outbound(reply_msg, 'default') + + # Be polite and pass a SESSION_CLOSE to the active endpoint + close_msg = TransportUserMessage(**msg.payload) + close_msg['content'] = None + close_msg['session_event'] = TransportUserMessage.SESSION_CLOSE + yield self.publish_inbound(close_msg, active_endpoint) + returnValue((self.STATE_SELECT, dict(active_endpoint=None))) + else: + yield self.publish_inbound(msg, active_endpoint) + returnValue(self.STATE_SELECTED) + + @inlineCallbacks + def handle_state_bad_choice(self, config, session, msg): + choice = self.get_menu_choice(msg, (1, 1)) + if choice is None: + reply_msg = msg.reply(config.invalid_input_message) + yield self.publish_outbound(reply_msg, 'default') + returnValue(self.STATE_BAD_CHOICE) + else: + reply_msg = msg.reply(self.create_menu(config)) + yield self.publish_outbound(reply_msg, 'default') + returnValue(self.STATE_SELECT) def handle_outbound(self, config, msg, conn_name): - log.debug("Handling outbound: %s" % (msg,)) + """ + TODO: Go to SELECT state when session_event=close + """ + log.msg("Processing outbound message: %s" % (msg,)) return self.publish_outbound(msg, 'default') + + def scan_for_keywords(self, config, msg, keywords): + first_word = (clean(msg['content']).split() + [''])[0] + if first_word in keywords: + return True + return False + + def get_menu_choice(self, msg, valid_range): + """ + Parse user input for selecting a numeric menu choice + """ + try: + value = int(clean(msg['content'])) + except ValueError: + return None + else: + if value not in range(valid_range[0], valid_range[1] + 1): + return None + return value + + def create_menu(self, config): + labels = [entry['label'] for entry in config.entries] + return (config.menu_title['content'] + "\n" + mkmenu(labels)) From e2894194c37b1630a3771dbeb509c18a42d46de9 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 12:21:22 +0200 Subject: [PATCH 18/53] Add tests for various helpers --- .../tests/test_vumi_app.py | 67 ++++++++++++++++--- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index f2c523b5e..6ce8299ec 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -4,6 +4,7 @@ from go.routers.application_multiplexer.vumi_app import ApplicationMultiplexer from go.routers.tests.helpers import RouterWorkerHelper +from vumi.tests.helpers import PersistenceHelper, MessageHelper class TestApplicationMultiplexerRouter(VumiTestCase): @@ -13,9 +14,18 @@ class TestApplicationMultiplexerRouter(VumiTestCase): @inlineCallbacks def setUp(self): self.router_helper = self.add_helper( - RouterWorkerHelper(ApplicationMultiplexer) - ) - self.router_worker = yield self.router_helper.get_router_worker({}) + RouterWorkerHelper(ApplicationMultiplexer)) + + self.msg_helper = yield self.add_helper(MessageHelper()) + self.persistence_helper = yield self.add_helper(PersistenceHelper()) + self.parent_redis = yield self.persistence_helper.get_redis_manager() + self.router_worker = yield self.router_helper.get_router_worker({ + 'worker_name': 'application_multiplexer', + 'redis_manager': { + 'FAKE_REDIS': self.parent_redis, + 'key_prefix': self.parent_redis.get_key_prefix(), + } + }) @inlineCallbacks def assert_routed_inbound(self, content, router, expected_endpoint): @@ -64,8 +74,49 @@ def test_no_messages_processed_while_stopped(self): [nack] = self.router_helper.ro.get_dispatched_events() self.assertEqual(nack['event_type'], 'nack') - @inlineCallbacks - def test_inbound_no_config(self): - router = yield self.router_helper.create_router(started=True) - yield self.assert_routed_inbound("foo bar", router, 'default') - yield self.assert_routed_inbound("baz quux", router, 'default') + def test_get_menu_choice(self): + # good + msg = self.msg_helper.make_inbound(content='3 ') + choice = self.router_worker.get_menu_choice(msg, (1, 4)) + self.assertEqual(choice, 3) + + # bad - out of range + choice = self.router_worker.get_menu_choice(msg, (1, 2)) + self.assertEqual(choice, None) + + # bad - non-numeric input + msg = self.msg_helper.make_inbound(content='Foo ') + choice = self.router_worker.get_menu_choice(msg, (1, 2)) + self.assertEqual(choice, None) + + def test_scan_for_keywords(self): + config = self.router_worker.config + msg = self.msg_helper.make_inbound(content=':menu') + self.assertTrue(self.router_worker.scan_for_keywords( + config, + msg, (':menu',))) + msg = self.msg_helper.make_inbound(content='Foo bar baz') + self.assertFalse(self.router_worker.scan_for_keywords( + config, + msg, (':menu',))) + + def test_create_menu(self): + config = self.router_worker.config.copy() + config.update({ + 'menu_title': {'content': 'Please select a choice'}, + 'entries': [ + { + 'label': 'Flappy Bird', + 'endpoint': 'flappy-bird', + }, + { + 'label': 'Mama', + 'endpoint': 'mama', + } + ] + }) + config = self.router_worker.CONFIG_CLASS(config) + + text = self.router_worker.create_menu(config) + self.assertEqual(text, + 'Please select a choice\n1) Flappy Bird\n2) Mama') From 1eb8a7b7245ab9c3f8b3e0b970e50bbd7c2a8878 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 16:47:23 +0200 Subject: [PATCH 19/53] Remove empty setup/teardown routines that do nothing --- go/routers/application_multiplexer/vumi_app.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index d14b44d75..8735f2426 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -66,12 +66,6 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_BAD_INPUT: 'handle_state_bad_input' } - def setup_router(self): - return super(ApplicationMultiplexer, self).setup_router() - - def teardown_router(self): - return super(ApplicationMultiplexer, self).teardown_router() - def session_manager(self, config): """ The implementation of SessionManager does the job of From 581c142ccb062104c1a1b203fc91d3370e4bbce5 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 16:49:11 +0200 Subject: [PATCH 20/53] simplify verification of choosen endpoint --- go/routers/application_multiplexer/vumi_app.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index 8735f2426..3eb68ecd1 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -76,14 +76,11 @@ def session_manager(self, config): max_session_length=config.session_expiry ) - def valid_target_endpoint(self, config, active_endpoint): + def target_endpoints(self, config): """ Make sure the currently active endpoint is still valid. """ - endpoints = set([entry['endpoint'] for entry in config.entries]) - if active_endpoint not in endpoints: - return False - return True + return set([entry['endpoint'] for entry in config.entries]) @inlineCallbacks def handle_inbound(self, config, msg, conn_name): @@ -161,7 +158,7 @@ def handle_state_select(self, config, session, msg): @inlineCallbacks def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] - if not self.valid_target_endpoint(config, active_endpoint): + if active_endpoint not in self.target_endpoints(config): reply_msg = msg.reply( config.error_message, continue_session=False From 7dfe830c6148f98d676cd3592691e11c07dd20aa Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 16:51:27 +0200 Subject: [PATCH 21/53] Fix typo in the router edit template --- go/router/templates/router/edit.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/router/templates/router/edit.html b/go/router/templates/router/edit.html index 92e413a52..36697fa17 100644 --- a/go/router/templates/router/edit.html +++ b/go/router/templates/router/edit.html @@ -23,7 +23,7 @@
{% for edit_form in edit_forms %}
- {% if edit_form.non_non_field_errors %} + {% if edit_form.non_field_errors %} {{ edit_form.non_field_errors }} {% endif %} {{ edit_form|crispy }} From b5cee257763b2558a17ab4cb83a54a09e5244217 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 17:11:23 +0200 Subject: [PATCH 22/53] Revert changes to the magic comments in vumi_app.py --- go/routers/application_multiplexer/vumi_app.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index 3eb68ecd1..fbbdfc789 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -1,7 +1,4 @@ -# Local Variables: -# test-case-name: go.routers.application_multiplexer.tests.test_vumi_app -# coding: utf-8 -# End: +# -*- test-case-name: go.routers.application_multiplexer.tests.test_vumi_app -*- from twisted.internet.defer import inlineCallbacks, returnValue From e000b416f310c886a3f63abea068ad74f84e3a24 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Wed, 19 Feb 2014 17:39:59 +0200 Subject: [PATCH 23/53] Remove irrelevant whitespace fix from go.settings --- go/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/go/settings.py b/go/settings.py index 518b8e1ae..17d0d5453 100644 --- a/go/settings.py +++ b/go/settings.py @@ -1,3 +1,4 @@ + # Django settings for go project. import os import djcelery From 0c52ebfad499763027e9b4538d6d0d12de31be17 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Thu, 20 Feb 2014 13:07:27 +0200 Subject: [PATCH 24/53] Some cleanups and refactoring in the multiplexer worker --- .../application_multiplexer/vumi_app.py | 81 ++++++++++++------- 1 file changed, 52 insertions(+), 29 deletions(-) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index fbbdfc789..ddb7e9a9b 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -56,12 +56,17 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_SELECTED = "selected" STATE_BAD_INPUT = "bad_input" - HANDLERS = { - STATE_START: 'handle_state_start', - STATE_SELECT: 'handle_state_select', - STATE_SELECTED: 'handle_state_selected', - STATE_BAD_INPUT: 'handle_state_bad_input' - } + OUTBOUND_ENDPOINT = "default" + + def setup_router(self): + d = super(ApplicationMultiplexer, self).setup_router() + self.handlers = { + self.STATE_START: self.handle_state_start, + self.STATE_SELECT: self.handle_state_select, + self.STATE_SELECTED: self.handle_state_selected, + self.STATE_BAD_INPUT: self.handle_state_bad_input + } + return d def session_manager(self, config): """ @@ -99,8 +104,8 @@ def handle_inbound(self, config, msg, conn_name): state = session['state'] try: - handler = getattr(self, self.HANDLERS[state]) - result = yield handler(config, session, msg) + result = yield self.handlers[state](config, session, msg) + # update session with next state and updated session fields if type(result) is tuple: next_state = session['state'] = result[0] session.update(result[1]) @@ -116,15 +121,15 @@ def handle_inbound(self, config, msg, conn_name): config.error_message, continue_session=False ) - self.publish_outbound(reply_msg, 'default') + self.publish_outbound(reply_msg) else: yield session_manager.save_session(user_id, **session) @inlineCallbacks def handle_state_start(self, config, session, msg): reply_msg = msg.reply(self.create_menu(config)) - yield self.publish_outbound(reply_msg, 'default') - returnValue(self.STATE_CHOOSE) + yield self.publish_outbound(reply_msg) + returnValue(self.STATE_SELECT) @inlineCallbacks def handle_state_select(self, config, session, msg): @@ -140,17 +145,20 @@ def handle_state_select(self, config, session, msg): choice = self.get_menu_choice(msg, (1, len(config.entries))) if choice is None: reply_msg = msg.reply(config.invalid_input_message) - yield self.publish_outbound(reply_msg, 'default') - returnValue(self.STATE_BAD_CHOICE) + yield self.publish_outbound(reply_msg) + returnValue(self.STATE_BAD_INPUT) else: endpoint = config.entries[choice - 1]['endpoint'] - forwarded_msg = TransportUserMessage(**msg.payload) - forwarded_msg['content'] = None - forwarded_msg['session_event'] = TransportUserMessage.SESSION_START + forwarded_msg = self.forwarded_message( + msg, + content=None, + session_event=TransportUserMessage.SESSION_START + ) yield self.publish_inbound(forwarded_msg, endpoint) log.msg("Switched to endpoint '%s' for user %s" % (endpoint, msg['from_addr'])) - returnValue((self.STATE_SELECTED, dict(active_endpoint=endpoint))) + returnValue((self.STATE_SELECTED, + dict(active_endpoint=endpoint))) @inlineCallbacks def handle_state_selected(self, config, session, msg): @@ -160,32 +168,35 @@ def handle_state_selected(self, config, session, msg): config.error_message, continue_session=False ) - yield self.publish_outbound(reply_msg, 'default') + yield self.publish_outbound(reply_msg) returnValue(None) elif self.scan_for_keywords(config, msg, (config.keyword,)): reply_msg = msg.reply(self.create_menu(config)) - yield self.publish_outbound(reply_msg, 'default') + yield self.publish_outbound(reply_msg) # Be polite and pass a SESSION_CLOSE to the active endpoint - close_msg = TransportUserMessage(**msg.payload) - close_msg['content'] = None - close_msg['session_event'] = TransportUserMessage.SESSION_CLOSE - yield self.publish_inbound(close_msg, active_endpoint) - returnValue((self.STATE_SELECT, dict(active_endpoint=None))) + forwarded_msg = self.forwarded_message( + msg, + content=None, + session_event=TransportUserMessage.SESSION_CLOSE + ) + yield self.publish_inbound(forwarded_msg, active_endpoint) + returnValue((self.STATE_SELECT, + dict(active_endpoint=None))) else: yield self.publish_inbound(msg, active_endpoint) returnValue(self.STATE_SELECTED) @inlineCallbacks - def handle_state_bad_choice(self, config, session, msg): + def handle_state_bad_input(self, config, session, msg): choice = self.get_menu_choice(msg, (1, 1)) if choice is None: reply_msg = msg.reply(config.invalid_input_message) - yield self.publish_outbound(reply_msg, 'default') - returnValue(self.STATE_BAD_CHOICE) + yield self.publish_outbound(reply_msg) + returnValue(self.STATE_BAD_INPUT) else: reply_msg = msg.reply(self.create_menu(config)) - yield self.publish_outbound(reply_msg, 'default') + yield self.publish_outbound(reply_msg) returnValue(self.STATE_SELECT) def handle_outbound(self, config, msg, conn_name): @@ -193,7 +204,19 @@ def handle_outbound(self, config, msg, conn_name): TODO: Go to SELECT state when session_event=close """ log.msg("Processing outbound message: %s" % (msg,)) - return self.publish_outbound(msg, 'default') + return self.publish_outbound(msg) + + def publish_outbound(self, msg): + return super(ApplicationMultiplexer, self).publish_outbound( + msg, + self.OUTBOUND_ENDPOINT + ) + + def forwarded_message(self, msg, **kwargs): + copy = TransportUserMessage(**msg.payload) + for k, v in kwargs.items(): + copy[k] = v + return copy def scan_for_keywords(self, config, msg, keywords): first_word = (clean(msg['content']).split() + [''])[0] From eaa947d21c0b2b9db248ea5e8267b793625481fe Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Thu, 20 Feb 2014 13:23:53 +0200 Subject: [PATCH 25/53] Add a reference to a Vumi ticket in common.py --- go/routers/application_multiplexer/common.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/routers/application_multiplexer/common.py b/go/routers/application_multiplexer/common.py index 5028d7761..7baea08b0 100644 --- a/go/routers/application_multiplexer/common.py +++ b/go/routers/application_multiplexer/common.py @@ -1,4 +1,7 @@ # helpers lifted from GFM and Wikipedia +# +# TODO: move these common text helpers to Vumi Core +# https://github.com/praekelt/vumi/issues/727 def clean(content): From 9db74c74dcdea1f5e2c2d0a987b35d8560dfeb30 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Thu, 20 Feb 2014 17:57:14 +0200 Subject: [PATCH 26/53] Cleanup existing tests and sort out issues from review --- .../tests/test_views.py | 1 - .../tests/test_vumi_app.py | 30 +++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_views.py b/go/routers/application_multiplexer/tests/test_views.py index 34e5b7b2f..6458a6a38 100644 --- a/go/routers/application_multiplexer/tests/test_views.py +++ b/go/routers/application_multiplexer/tests/test_views.py @@ -77,7 +77,6 @@ def test_initial_config(self): { 'label': 'Flappy Bird', 'endpoint': 'flappy-bird', - }, ]}) response = self.client.get(rtr_helper.get_view_url('edit')) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index 6ce8299ec..16260f57b 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -4,7 +4,7 @@ from go.routers.application_multiplexer.vumi_app import ApplicationMultiplexer from go.routers.tests.helpers import RouterWorkerHelper -from vumi.tests.helpers import PersistenceHelper, MessageHelper +from vumi.tests.helpers import PersistenceHelper class TestApplicationMultiplexerRouter(VumiTestCase): @@ -16,7 +16,6 @@ def setUp(self): self.router_helper = self.add_helper( RouterWorkerHelper(ApplicationMultiplexer)) - self.msg_helper = yield self.add_helper(MessageHelper()) self.persistence_helper = yield self.add_helper(PersistenceHelper()) self.parent_redis = yield self.persistence_helper.get_redis_manager() self.router_worker = yield self.router_helper.get_router_worker({ @@ -27,6 +26,11 @@ def setUp(self): } }) + def dynamic_config(self, fields): + config = self.router_worker.config.copy() + config.update(fields) + return config + @inlineCallbacks def assert_routed_inbound(self, content, router, expected_endpoint): msg = yield self.router_helper.ri.make_dispatch_inbound( @@ -76,7 +80,7 @@ def test_no_messages_processed_while_stopped(self): def test_get_menu_choice(self): # good - msg = self.msg_helper.make_inbound(content='3 ') + msg = self.router_helper.make_inbound(content='3 ') choice = self.router_worker.get_menu_choice(msg, (1, 4)) self.assertEqual(choice, 3) @@ -85,24 +89,25 @@ def test_get_menu_choice(self): self.assertEqual(choice, None) # bad - non-numeric input - msg = self.msg_helper.make_inbound(content='Foo ') + msg = self.router_helper.make_inbound(content='Foo ') choice = self.router_worker.get_menu_choice(msg, (1, 2)) self.assertEqual(choice, None) def test_scan_for_keywords(self): - config = self.router_worker.config - msg = self.msg_helper.make_inbound(content=':menu') + config = self.dynamic_config({ + 'keyword': ':menu' + }) + msg = self.router_helper.make_inbound(content=':menu') self.assertTrue(self.router_worker.scan_for_keywords( config, msg, (':menu',))) - msg = self.msg_helper.make_inbound(content='Foo bar baz') + msg = self.router_helper.make_inbound(content='Foo bar baz') self.assertFalse(self.router_worker.scan_for_keywords( config, msg, (':menu',))) def test_create_menu(self): - config = self.router_worker.config.copy() - config.update({ + config = self.dynamic_config({ 'menu_title': {'content': 'Please select a choice'}, 'entries': [ { @@ -116,7 +121,8 @@ def test_create_menu(self): ] }) config = self.router_worker.CONFIG_CLASS(config) - text = self.router_worker.create_menu(config) - self.assertEqual(text, - 'Please select a choice\n1) Flappy Bird\n2) Mama') + self.assertEqual( + text, + 'Please select a choice\n1) Flappy Bird\n2) Mama' + ) From 52cceec9a337027b3e7d201c39ef29a895d011d1 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Thu, 20 Feb 2014 17:58:20 +0200 Subject: [PATCH 27/53] Cleanup/iterate on router configuration as per the the review --- go/routers/application_multiplexer/vumi_app.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index ddb7e9a9b..b28819f70 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -18,10 +18,6 @@ class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): "Maximum amount of time in seconds to keep session data around", default=1800, static=True) - session_data_version = ConfigInt( - "Just in case we need to modify the schema of session storage", - default=1, static=True) - # Dynamic, per-message configuration menu_title = ConfigDict( "Content for the menu title", @@ -31,16 +27,16 @@ class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): default=[]) keyword = ConfigText( "Keyword to signal a request to return to the application menu", - default=':menu') + default=None) invalid_input_message = ConfigText( "Prompt to display when warning about an invalid choice", default=("That is an incorrect choice. Please enter the number " - "next to the menu item you wish to choose.\n\n 1) Try Again")) + "of the menu item you wish to choose.\n\n 1) Try Again")) error_message = ConfigText( ("Prompt to display when a configuration change invalidates " "an active session."), default=("Oops! We experienced a temporary error. " - "Please dial the line again.")) + "Please try and dial the line again.")) class ApplicationMultiplexer(GoRouterWorker): @@ -94,11 +90,9 @@ def handle_inbound(self, config, msg, conn_name): session = yield session_manager.load_session(user_id) if session is None: log.msg("Creating session for user %s" % user_id) + session = {} state = self.STATE_START - session = { - 'version': config.session_data_version, - } - yield session_manager.create_session(user_id, **session) + yield session_manager.create_session(user_id) else: log.msg("Loading session for user %s: %s" % (session,)) state = session['state'] From 0be9e4c524092acea2ec96e28f6e72d984776186 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Thu, 20 Feb 2014 18:59:17 +0200 Subject: [PATCH 28/53] Add test for the START state --- .../tests/test_vumi_app.py | 40 ++++++++++++++----- .../application_multiplexer/vumi_app.py | 13 +++--- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index 16260f57b..a92e7979c 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -5,6 +5,7 @@ from go.routers.application_multiplexer.vumi_app import ApplicationMultiplexer from go.routers.tests.helpers import RouterWorkerHelper from vumi.tests.helpers import PersistenceHelper +from vumi.message import TransportUserMessage class TestApplicationMultiplexerRouter(VumiTestCase): @@ -16,15 +17,18 @@ def setUp(self): self.router_helper = self.add_helper( RouterWorkerHelper(ApplicationMultiplexer)) - self.persistence_helper = yield self.add_helper(PersistenceHelper()) - self.parent_redis = yield self.persistence_helper.get_redis_manager() - self.router_worker = yield self.router_helper.get_router_worker({ - 'worker_name': 'application_multiplexer', - 'redis_manager': { - 'FAKE_REDIS': self.parent_redis, - 'key_prefix': self.parent_redis.get_key_prefix(), - } - }) +# self.persistence_helper = yield self.add_helper(PersistenceHelper()) +# self.parent_redis = yield self.persistence_helper.get_redis_manager() +# self.router_worker = yield self.router_helper.get_router_worker({ +# 'worker_name': 'application_multiplexer', +# 'redis_manager': { +# 'FAKE_REDIS': self.parent_redis, +# 'key_prefix': self.parent_redis.get_key_prefix(), +# } + + self.router_worker = yield self.router_helper.get_router_worker({}) + +# }) def dynamic_config(self, fields): config = self.router_worker.config.copy() @@ -78,6 +82,24 @@ def test_no_messages_processed_while_stopped(self): [nack] = self.router_helper.ro.get_dispatched_events() self.assertEqual(nack['event_type'], 'nack') + @inlineCallbacks + def test_state_start(self): + router = yield self.router_helper.create_router(started=True, config={ + 'entries': [ + { + 'label': 'Flappy Bird', + 'endpoint': 'flappy-bird', + }, + ] + }) + yield self.router_helper.ri.make_dispatch_inbound( + None, + session_event=TransportUserMessage.SESSION_NEW, + router=router) + + [msg] = self.router_helper.ri.get_dispatched_outbound() + print msg['content'] + def test_get_menu_choice(self): # good msg = self.router_helper.make_inbound(content='3 ') diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index b28819f70..fc6823ffd 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -85,16 +85,17 @@ def handle_inbound(self, config, msg, conn_name): log.msg("Processing inbound message: %s" % (msg,)) user_id = msg['from_addr'] - session_manager = self.session_manager(config) + session_manager = yield self.session_manager(config) + session_event = msg['session_event'] session = yield session_manager.load_session(user_id) - if session is None: + if not session or session_event == TransportUserMessage.SESSION_NEW: log.msg("Creating session for user %s" % user_id) session = {} state = self.STATE_START yield session_manager.create_session(user_id) else: - log.msg("Loading session for user %s: %s" % (session,)) + log.msg("Loading session for user %s: %s" % (user_id, session,)) state = session['state'] try: @@ -106,7 +107,7 @@ def handle_inbound(self, config, msg, conn_name): else: next_state = session['state'] = result if state != next_state: - log.msg("State transition for user %s: %s => %s" + log.msg("State transition for user %s: %s => %s" % (user_id, state, next_state)) except: log.err() @@ -117,7 +118,7 @@ def handle_inbound(self, config, msg, conn_name): ) self.publish_outbound(reply_msg) else: - yield session_manager.save_session(user_id, **session) + yield session_manager.save_session(user_id, session) @inlineCallbacks def handle_state_start(self, config, session, msg): @@ -146,7 +147,7 @@ def handle_state_select(self, config, session, msg): forwarded_msg = self.forwarded_message( msg, content=None, - session_event=TransportUserMessage.SESSION_START + session_event=TransportUserMessage.SESSION_NEW ) yield self.publish_inbound(forwarded_msg, endpoint) log.msg("Switched to endpoint '%s' for user %s" % From db58f1568f6906d3e1288c87eaf9be0d9b5eff62 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 21 Feb 2014 00:24:02 +0200 Subject: [PATCH 29/53] Add test helper and a bunch of tests --- .../tests/test_vumi_app.py | 343 ++++++++++++++++-- 1 file changed, 317 insertions(+), 26 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index a92e7979c..ae91df149 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -1,34 +1,107 @@ +import copy + from twisted.internet.defer import inlineCallbacks from vumi.tests.helpers import VumiTestCase - from go.routers.application_multiplexer.vumi_app import ApplicationMultiplexer from go.routers.tests.helpers import RouterWorkerHelper -from vumi.tests.helpers import PersistenceHelper -from vumi.message import TransportUserMessage + + +def raise_error(*args, **kw): + raise RuntimeError("An anomaly has been detected") class TestApplicationMultiplexerRouter(VumiTestCase): router_class = ApplicationMultiplexer + ROUTER_CONFIG = { + 'invalid_input_message': 'Bad choice.\n1) Try Again', + 'error_message': 'Oops! Sorry!', + 'keyword': ':menu', + 'entries': [ + { + 'label': 'Flappy Bird', + 'endpoint': 'flappy-bird', + }, + ] + } + + @inlineCallbacks def setUp(self): self.router_helper = self.add_helper( RouterWorkerHelper(ApplicationMultiplexer)) + self.router_worker = yield self.router_helper.get_router_worker({}) -# self.persistence_helper = yield self.add_helper(PersistenceHelper()) -# self.parent_redis = yield self.persistence_helper.get_redis_manager() -# self.router_worker = yield self.router_helper.get_router_worker({ -# 'worker_name': 'application_multiplexer', -# 'redis_manager': { -# 'FAKE_REDIS': self.parent_redis, -# 'key_prefix': self.parent_redis.get_key_prefix(), -# } + @inlineCallbacks + def check_state(self, router, state): + """ + A helper to validate routing behavior. - self.router_worker = yield self.router_helper.get_router_worker({}) + The state dict describes the messages which need to sent, which + session data to initialize, and what data should be asserted + when the state handler completes execution. + + Messages are represented by a tuple (content, {field=value, ...}) + + This could be made into a generic test helper one day. + """ + + session_manager = yield self.router_worker.session_manager( + self.router_worker.CONFIG_CLASS(self.router_worker.config) + ) + + # Initialize session data + for user_id, data in state['session'].items(): + yield session_manager.save_session(user_id, data) + + # Send inbound message via ri + content, fields = state['ri_inbound'] + msg = yield self.router_helper.ri.make_dispatch_inbound( + content, + router=router, + **fields) + + # If required, send outbound message via ro + if 'ro_inbound' in state: + content, fields = state['ro_inbound'] + yield self.router_helper.ro.make_dispatch_reply( + msg, content, **fields) + + # If required, assert that an outbound message was dispatched to ro + if 'ro_outbound' in state['expect']: + content, fields = state['expect']['ro_outbound'] + [msg] = self.router_helper.ro.get_dispatched_inbound() + self.assertEqual(msg['content'], content, + msg="RO Inbound Message: Unexpected content") + for field, value in fields.items(): + self.assertEqual( + msg[field], value, + msg=("RO Inbound Message: Unexpected value For field '%s'" + % field) + ) + + # Assert that an expected message was dispatched via ri + [msg] = self.router_helper.ri.get_dispatched_outbound() + content, fields = state['expect']['ri_outbound'] + self.assertEqual(msg['content'], content, + msg="RI Outbound Message: Unexpected content") + for field, value in fields.items(): + self.assertEqual( + msg[field], value, + msg=("RI Outbound Message: Unexpected value For field '%s'" + % field) + ) + + # Assert that user session was updated correctly + for user_id, data in state['expect']['session'].iteritems(): + session = yield session_manager.load_session(user_id) + if 'created_at' in session: + del session['created_at'] + self.assertEqual(session, data, + msg="Unexpected session data") -# }) def dynamic_config(self, fields): config = self.router_worker.config.copy() @@ -83,22 +156,240 @@ def test_no_messages_processed_while_stopped(self): self.assertEqual(nack['event_type'], 'nack') @inlineCallbacks - def test_state_start(self): - router = yield self.router_helper.create_router(started=True, config={ - 'entries': [ - { - 'label': 'Flappy Bird', - 'endpoint': 'flappy-bird', + def test_state_start_to_select(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': (None, dict(from_addr='2323', session_event='new')), + 'session': {}, + 'expect': { + 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), + 'session': { + '2323': {'state': ApplicationMultiplexer.STATE_SELECT}, + } + } + }) + + @inlineCallbacks + def test_state_select_to_selected(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': ('1', dict(from_addr='2323', + session_event='resume')), + 'ro_inbound': ('Flappy Flappy!', dict(session_event='resume')), + 'session': { + '2323': {'state': ApplicationMultiplexer.STATE_SELECT}, + }, + 'expect': { + 'ri_outbound': ('Flappy Flappy!', {}), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird' + }, + } + } + }) + + @inlineCallbacks + def test_state_selected_to_selected(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': ('Up!', dict(from_addr='2323', + session_event='resume')), + 'ro_inbound': ('Game Over!', dict(session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird' }, - ] + }, + 'expect': { + 'ri_outbound': ('Game Over!', {}), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird' + }, + } + } }) - yield self.router_helper.ri.make_dispatch_inbound( - None, - session_event=TransportUserMessage.SESSION_NEW, - router=router) - [msg] = self.router_helper.ri.get_dispatched_outbound() - print msg['content'] + @inlineCallbacks + def test_state_selected_to_select(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': (':menu', dict(from_addr='2323', + session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird' + }, + }, + 'expect': { + 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), + 'ro_outbound': (None, dict(session_event='close')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECT, + # TODO: I should clear session keys which are no longer + # relevant in the SELECT state + 'active_endpoint': 'None' + }, + } + } + }) + + @inlineCallbacks + def test_state_select_to_bad_input(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': ('j8', dict(from_addr='2323', + session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECT, + }, + }, + 'expect': { + 'ri_outbound': ('Bad choice.\n1) Try Again', {}), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + }, + } + } + }) + + @inlineCallbacks + def test_state_bad_input_to_bad_input(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': ('2', dict(from_addr='2323', + session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + }, + }, + 'expect': { + 'ri_outbound': ('Bad choice.\n1) Try Again', {}), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + }, + } + } + }) + + @inlineCallbacks + def test_state_bad_input_to_select(self): + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + yield self.check_state(router, { + 'ri_inbound': ('1', dict(from_addr='2323', + session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + }, + }, + 'expect': { + 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECT, + }, + } + } + }) + + @inlineCallbacks + def test_runtime_exception(self): + """ + Verifies that the worker handles an arbitrary runtime error gracefully, + and sends an appropriate error message back to the user + """ + router = yield self.router_helper.create_router( + started=True, + config=self.ROUTER_CONFIG + ) + + # Make worker.target_endpoints raise an exception + self.patch(self.router_worker, + 'target_endpoints', + raise_error) + + yield self.check_state(router, { + 'ri_inbound': (':menu', dict(from_addr='2323', + session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird' + }, + }, + 'expect': { + 'ri_outbound': ('Oops! Sorry!', {}), + 'session': { + '2323': {}, + } + } + }) + errors = self.flushLoggedErrors(RuntimeError) + self.assertEqual(len(errors), 1) + + @inlineCallbacks + def test_session_invalidation(self): + """ + Verify that the router gracefully handles a configuration + update while there is an active user session. + + A session is invalidated if there is no longer an attached endpoint + to which it refers. + """ + config = copy.deepcopy(self.ROUTER_CONFIG) + config['entries'][0]['endpoint'] = 'mama' + router = yield self.router_helper.create_router( + started=True, + config=config + ) + yield self.check_state(router, { + 'ri_inbound': ('Up!', dict(from_addr='2323', + session_event='resume')), + 'session': { + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird' + }, + }, + 'expect': { + 'ri_outbound': ('Oops! Sorry!', dict(session_event='close')), + 'session': { + '2323': {}, + } + } + }) def test_get_menu_choice(self): # good From 97c9bb6ef4b573e241bd22f906b50d57a15cccdd Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 21 Feb 2014 00:24:38 +0200 Subject: [PATCH 30/53] More cleanups, and fixes for various bugs --- .../application_multiplexer/vumi_app.py | 63 ++++++++++++++----- 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index fc6823ffd..92b36c92e 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -41,8 +41,32 @@ class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): class ApplicationMultiplexer(GoRouterWorker): """ - Router that splits inbound messages based on keywords. - """ + Router that multiplexes between different endpoints + on the outbound path. + + State Diagram (for fun): + + +----------------+ + | | + | start | + | | + +----+-----------+ + | + | + +----*-----------+ +----------------+ + | *----+ | + | select | | bad_input | + | +----* | + +----+----*------+ +----------------+ + | | + | | + +----*----+------+ + | | + | selected | + | | + +----------------+ +""" + CONFIG_CLASS = ApplicationMultiplexerConfig worker_name = 'application_multiplexer' @@ -100,15 +124,26 @@ def handle_inbound(self, config, msg, conn_name): try: result = yield self.handlers[state](config, session, msg) - # update session with next state and updated session fields - if type(result) is tuple: - next_state = session['state'] = result[0] - session.update(result[1]) + if result is None: + # Halt session immediately + # The 'close' message should have already been sent back to + # the user at this point. + log.msg(("Router configuration change forced session abort " + "for user %s" % user_id)) + yield session_manager.clear_session(user_id) + return else: - next_state = session['state'] = result - if state != next_state: - log.msg("State transition for user %s: %s => %s" % - (user_id, state, next_state)) + if type(result) is tuple: + # Transition to next state AND mutate session data + next_state = session['state'] = result[0] + session.update(result[1]) + else: + # Transition to next state + next_state = session['state'] = result + if state != next_state: + log.msg("State transition for user %s: %s => %s" % + (user_id, state, next_state)) + yield session_manager.save_session(user_id, session) except: log.err() yield session_manager.clear_session(user_id) @@ -117,8 +152,6 @@ def handle_inbound(self, config, msg, conn_name): continue_session=False ) self.publish_outbound(reply_msg) - else: - yield session_manager.save_session(user_id, session) @inlineCallbacks def handle_state_start(self, config, session, msg): @@ -132,10 +165,6 @@ def handle_state_select(self, config, session, msg): NOTE: There is an edge case in which the user input no longer matches an entry in the current config. The impact depends on the number of users and how often the router config is modified. - - One solution would be to compare hashes of the menu configs as - they existed at the START and SELECT states. If there is a mismatch, - invalidate the current session. """ choice = self.get_menu_choice(msg, (1, len(config.entries))) if choice is None: @@ -143,6 +172,8 @@ def handle_state_select(self, config, session, msg): yield self.publish_outbound(reply_msg) returnValue(self.STATE_BAD_INPUT) else: + # TODO: Not urgent, but need to put in place a guard + # here for the reasons outlined in the docstring. endpoint = config.entries[choice - 1]['endpoint'] forwarded_msg = self.forwarded_message( msg, From 6577351fa6f24e35356808cf69b04194dac2f7fc Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 21 Feb 2014 10:38:04 +0200 Subject: [PATCH 31/53] More robust endpoint selection --- .../tests/test_vumi_app.py | 35 +++++-- .../application_multiplexer/vumi_app.py | 91 ++++++++++--------- 2 files changed, 75 insertions(+), 51 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index ae91df149..ea87d32f1 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -1,4 +1,5 @@ import copy +import json from twisted.internet.defer import inlineCallbacks @@ -39,7 +40,7 @@ def check_state(self, router, state): """ A helper to validate routing behavior. - The state dict describes the messages which need to sent, which + The state dict describes the messages which need to be sent, what session data to initialize, and what data should be asserted when the state handler completes execution. @@ -167,7 +168,10 @@ def test_state_start_to_select(self): 'expect': { 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), 'session': { - '2323': {'state': ApplicationMultiplexer.STATE_SELECT}, + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', + }, } } }) @@ -183,14 +187,18 @@ def test_state_select_to_selected(self): session_event='resume')), 'ro_inbound': ('Flappy Flappy!', dict(session_event='resume')), 'session': { - '2323': {'state': ApplicationMultiplexer.STATE_SELECT}, + '2323': { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', + }, }, 'expect': { 'ri_outbound': ('Flappy Flappy!', {}), 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird' + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }, } } @@ -209,7 +217,8 @@ def test_state_selected_to_selected(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird' + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }, }, 'expect': { @@ -217,7 +226,8 @@ def test_state_selected_to_selected(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird' + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }, } } @@ -235,7 +245,8 @@ def test_state_selected_to_select(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird' + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }, }, 'expect': { @@ -244,9 +255,10 @@ def test_state_selected_to_select(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECT, + 'active_endpoint': 'None', # TODO: I should clear session keys which are no longer - # relevant in the SELECT state - 'active_endpoint': 'None' + # relevant in this state + 'endpoints': '["flappy-bird"]', }, } } @@ -264,6 +276,7 @@ def test_state_select_to_bad_input(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', }, }, 'expect': { @@ -271,6 +284,7 @@ def test_state_select_to_bad_input(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + 'endpoints': '["flappy-bird"]', }, } } @@ -319,6 +333,7 @@ def test_state_bad_input_to_select(self): 'session': { '2323': { 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', }, } } @@ -365,7 +380,7 @@ def test_session_invalidation(self): Verify that the router gracefully handles a configuration update while there is an active user session. - A session is invalidated if there is no longer an attached endpoint + A session is aborted if there is no longer an attached endpoint to which it refers. """ config = copy.deepcopy(self.ROUTER_CONFIG) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/application_multiplexer/vumi_app.py index 92b36c92e..bdecf316a 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/application_multiplexer/vumi_app.py @@ -1,4 +1,5 @@ # -*- test-case-name: go.routers.application_multiplexer.tests.test_vumi_app -*- +import json from twisted.internet.defer import inlineCallbacks, returnValue @@ -76,8 +77,6 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_SELECTED = "selected" STATE_BAD_INPUT = "bad_input" - OUTBOUND_ENDPOINT = "default" - def setup_router(self): d = super(ApplicationMultiplexer, self).setup_router() self.handlers = { @@ -110,10 +109,9 @@ def handle_inbound(self, config, msg, conn_name): user_id = msg['from_addr'] session_manager = yield self.session_manager(config) - session_event = msg['session_event'] session = yield session_manager.load_session(user_id) - if not session or session_event == TransportUserMessage.SESSION_NEW: + if not session: log.msg("Creating session for user %s" % user_id) session = {} state = self.STATE_START @@ -126,12 +124,12 @@ def handle_inbound(self, config, msg, conn_name): result = yield self.handlers[state](config, session, msg) if result is None: # Halt session immediately - # The 'close' message should have already been sent back to + # The 'close' message has already been sent back to # the user at this point. log.msg(("Router configuration change forced session abort " "for user %s" % user_id)) yield session_manager.clear_session(user_id) - return + yield self.publish_error_reply(msg, config) else: if type(result) is tuple: # Transition to next state AND mutate session data @@ -147,54 +145,49 @@ def handle_inbound(self, config, msg, conn_name): except: log.err() yield session_manager.clear_session(user_id) - reply_msg = msg.reply( - config.error_message, - continue_session=False - ) - self.publish_outbound(reply_msg) + yield self.publish_error_reply(msg, config) @inlineCallbacks def handle_state_start(self, config, session, msg): + """ + When presenting the menu, we also store the list of endpoints + in the session data. Later, in the SELECT state, we load + these endpoints and retrieve the candidate endpoint based + on the user's menu choice. + """ reply_msg = msg.reply(self.create_menu(config)) yield self.publish_outbound(reply_msg) - returnValue(self.STATE_SELECT) + endpoints = json.dumps( + [entry['endpoint'] for entry in config.entries] + ) + returnValue((self.STATE_SELECT, dict(endpoints=endpoints))) @inlineCallbacks def handle_state_select(self, config, session, msg): - """ - NOTE: There is an edge case in which the user input no longer - matches an entry in the current config. The impact depends - on the number of users and how often the router config is modified. - """ - choice = self.get_menu_choice(msg, (1, len(config.entries))) - if choice is None: + endpoint = self.get_endpoint_for_choice(msg, session) + if endpoint is None: reply_msg = msg.reply(config.invalid_input_message) yield self.publish_outbound(reply_msg) returnValue(self.STATE_BAD_INPUT) else: - # TODO: Not urgent, but need to put in place a guard - # here for the reasons outlined in the docstring. - endpoint = config.entries[choice - 1]['endpoint'] - forwarded_msg = self.forwarded_message( - msg, - content=None, - session_event=TransportUserMessage.SESSION_NEW - ) - yield self.publish_inbound(forwarded_msg, endpoint) - log.msg("Switched to endpoint '%s' for user %s" % - (endpoint, msg['from_addr'])) - returnValue((self.STATE_SELECTED, - dict(active_endpoint=endpoint))) + if endpoint not in self.target_endpoints(config): + returnValue(None) + else: + forwarded_msg = self.forwarded_message( + msg, + content=None, + session_event=TransportUserMessage.SESSION_NEW + ) + yield self.publish_inbound(forwarded_msg, endpoint) + log.msg("Switched to endpoint '%s' for user %s" % + (endpoint, msg['from_addr'])) + returnValue((self.STATE_SELECTED, + dict(active_endpoint=endpoint))) @inlineCallbacks def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] if active_endpoint not in self.target_endpoints(config): - reply_msg = msg.reply( - config.error_message, - continue_session=False - ) - yield self.publish_outbound(reply_msg) returnValue(None) elif self.scan_for_keywords(config, msg, (config.keyword,)): reply_msg = msg.reply(self.create_menu(config)) @@ -221,9 +214,8 @@ def handle_state_bad_input(self, config, session, msg): yield self.publish_outbound(reply_msg) returnValue(self.STATE_BAD_INPUT) else: - reply_msg = msg.reply(self.create_menu(config)) - yield self.publish_outbound(reply_msg) - returnValue(self.STATE_SELECT) + result = yield self.handle_state_start(config, session, msg) + returnValue(result) def handle_outbound(self, config, msg, conn_name): """ @@ -235,9 +227,16 @@ def handle_outbound(self, config, msg, conn_name): def publish_outbound(self, msg): return super(ApplicationMultiplexer, self).publish_outbound( msg, - self.OUTBOUND_ENDPOINT + "default" ) + def publish_error_reply(self, msg, config): + reply_msg = msg.reply( + config.error_message, + continue_session=False + ) + return self.publish_outbound(reply_msg) + def forwarded_message(self, msg, **kwargs): copy = TransportUserMessage(**msg.payload) for k, v in kwargs.items(): @@ -250,6 +249,16 @@ def scan_for_keywords(self, config, msg, keywords): return True return False + def get_endpoint_for_choice(self, msg, session): + """ + Retrieves the candidate endpoint based on the user's numeric choice + """ + endpoints = json.loads(session['endpoints']) + index = self.get_menu_choice(msg, (1, len(endpoints))) + if index is None: + return None + return endpoints[index - 1] + def get_menu_choice(self, msg, valid_range): """ Parse user input for selecting a numeric menu choice From 216f447b9e1551d3a5bded6554667f4dc094d495 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 21 Feb 2014 18:23:41 +0200 Subject: [PATCH 32/53] Rewrite some of the tests as per Jeremy's suggestions --- .../tests/test_vumi_app.py | 152 +++++++++++------- 1 file changed, 98 insertions(+), 54 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index ea87d32f1..a1be0f754 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -28,7 +28,6 @@ class TestApplicationMultiplexerRouter(VumiTestCase): ] } - @inlineCallbacks def setUp(self): self.router_helper = self.add_helper( @@ -103,6 +102,24 @@ def check_state(self, router, state): self.assertEqual(session, data, msg="Unexpected session data") + @inlineCallbacks + def setup_session(self, user_id, data): + session_manager = yield self.router_worker.session_manager( + self.router_worker.CONFIG_CLASS(self.router_worker.config) + ) + # Initialize session data + yield session_manager.save_session(user_id, data) + + @inlineCallbacks + def assert_session_state(self, user_id, expected_session): + session_manager = yield self.router_worker.session_manager( + self.router_worker.CONFIG_CLASS(self.router_worker.config) + ) + session = yield session_manager.load_session(user_id) + if 'created_at' in session: + del session['created_at'] + self.assertEqual(session, expected_session, + msg="Unexpected session data") def dynamic_config(self, fields): config = self.router_worker.config.copy() @@ -162,18 +179,20 @@ def test_state_start_to_select(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': (None, dict(from_addr='2323', session_event='new')), - 'session': {}, - 'expect': { - 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECT, - 'endpoints': '["flappy-bird"]', - }, - } - } + # msg sent from user + yield self.router_helper.ri.make_dispatch_inbound( + None, + router=router, + from_addr='123') + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Please select a choice.\n1) Flappy Bird') + # assert that session data updated correctly + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks @@ -182,26 +201,37 @@ def test_state_select_to_selected(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': ('1', dict(from_addr='2323', - session_event='resume')), - 'ro_inbound': ('Flappy Flappy!', dict(session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECT, - 'endpoints': '["flappy-bird"]', - }, - }, - 'expect': { - 'ri_outbound': ('Flappy Flappy!', {}), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird', - 'endpoints': '["flappy-bird"]', - }, - } - } + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + '1', + router=router, + from_addr='123', + session_event='resume' + ) + + # assert that message is forwarded to application + [msg] = self.router_helper.ro.get_dispatched_inbound() + self.assertEqual(msg['content'], None) + self.assertEqual(msg['session_event'], 'new') + + # application sends reply + yield self.router_helper.ro.make_dispatch_reply(msg, 'Flappy Flappy!') + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Flappy Flappy!') + + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks @@ -210,27 +240,41 @@ def test_state_selected_to_selected(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': ('Up!', dict(from_addr='2323', - session_event='resume')), - 'ro_inbound': ('Game Over!', dict(session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird', - 'endpoints': '["flappy-bird"]', - }, - }, - 'expect': { - 'ri_outbound': ('Game Over!', {}), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird', - 'endpoints': '["flappy-bird"]', - }, - } - } + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + 'Up!', + router=router, + from_addr='123', + session_event='resume' + ) + + # assert that message is forwarded to application + [msg] = self.router_helper.ro.get_dispatched_inbound() + self.assertEqual(msg['content'], 'Up!') + self.assertEqual(msg['session_event'], 'resume') + + # application sends reply + yield self.router_helper.ro.make_dispatch_reply( + msg, + 'Game Over!\n1) Try Again!' + ) + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Game Over!\n1) Try Again!') + + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks From eafccbe600878ff4f645818b6d61d9dbd589b0a5 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Fri, 21 Feb 2014 18:56:16 +0200 Subject: [PATCH 33/53] Finish rewriting tests for the app multiplexor worker --- .../tests/test_vumi_app.py | 303 ++++++++---------- 1 file changed, 131 insertions(+), 172 deletions(-) diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/application_multiplexer/tests/test_vumi_app.py index a1be0f754..6f0ca561e 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/application_multiplexer/tests/test_vumi_app.py @@ -34,74 +34,6 @@ def setUp(self): RouterWorkerHelper(ApplicationMultiplexer)) self.router_worker = yield self.router_helper.get_router_worker({}) - @inlineCallbacks - def check_state(self, router, state): - """ - A helper to validate routing behavior. - - The state dict describes the messages which need to be sent, what - session data to initialize, and what data should be asserted - when the state handler completes execution. - - Messages are represented by a tuple (content, {field=value, ...}) - - This could be made into a generic test helper one day. - """ - - session_manager = yield self.router_worker.session_manager( - self.router_worker.CONFIG_CLASS(self.router_worker.config) - ) - - # Initialize session data - for user_id, data in state['session'].items(): - yield session_manager.save_session(user_id, data) - - # Send inbound message via ri - content, fields = state['ri_inbound'] - msg = yield self.router_helper.ri.make_dispatch_inbound( - content, - router=router, - **fields) - - # If required, send outbound message via ro - if 'ro_inbound' in state: - content, fields = state['ro_inbound'] - yield self.router_helper.ro.make_dispatch_reply( - msg, content, **fields) - - # If required, assert that an outbound message was dispatched to ro - if 'ro_outbound' in state['expect']: - content, fields = state['expect']['ro_outbound'] - [msg] = self.router_helper.ro.get_dispatched_inbound() - self.assertEqual(msg['content'], content, - msg="RO Inbound Message: Unexpected content") - for field, value in fields.items(): - self.assertEqual( - msg[field], value, - msg=("RO Inbound Message: Unexpected value For field '%s'" - % field) - ) - - # Assert that an expected message was dispatched via ri - [msg] = self.router_helper.ri.get_dispatched_outbound() - content, fields = state['expect']['ri_outbound'] - self.assertEqual(msg['content'], content, - msg="RI Outbound Message: Unexpected content") - for field, value in fields.items(): - self.assertEqual( - msg[field], value, - msg=("RI Outbound Message: Unexpected value For field '%s'" - % field) - ) - - # Assert that user session was updated correctly - for user_id, data in state['expect']['session'].iteritems(): - session = yield session_manager.load_session(user_id) - if 'created_at' in session: - del session['created_at'] - self.assertEqual(session, data, - msg="Unexpected session data") - @inlineCallbacks def setup_session(self, user_id, data): session_manager = yield self.router_worker.session_manager( @@ -283,29 +215,35 @@ def test_state_selected_to_select(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': (':menu', dict(from_addr='2323', - session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird', - 'endpoints': '["flappy-bird"]', - }, - }, - 'expect': { - 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), - 'ro_outbound': (None, dict(session_event='close')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECT, - 'active_endpoint': 'None', - # TODO: I should clear session keys which are no longer - # relevant in this state - 'endpoints': '["flappy-bird"]', - }, - } - } + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + ':menu', + router=router, + from_addr='123', + session_event='resume' + ) + + # assert that a close message is forwarded to application + [msg] = self.router_helper.ro.get_dispatched_inbound() + self.assertEqual(msg['content'], None) + self.assertEqual(msg['session_event'], 'close') + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Please select a choice.\n1) Flappy Bird') + + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'active_endpoint': 'None', + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks @@ -314,24 +252,28 @@ def test_state_select_to_bad_input(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': ('j8', dict(from_addr='2323', - session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECT, - 'endpoints': '["flappy-bird"]', - }, - }, - 'expect': { - 'ri_outbound': ('Bad choice.\n1) Try Again', {}), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_BAD_INPUT, - 'endpoints': '["flappy-bird"]', - }, - } - } + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + 'foo', + router=router, + from_addr='123', + session_event='resume' + ) + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Bad choice.\n1) Try Again') + + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks @@ -340,22 +282,28 @@ def test_state_bad_input_to_bad_input(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': ('2', dict(from_addr='2323', - session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_BAD_INPUT, - }, - }, - 'expect': { - 'ri_outbound': ('Bad choice.\n1) Try Again', {}), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_BAD_INPUT, - }, - } - } + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + 'foo', + router=router, + from_addr='123', + session_event='resume' + ) + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Bad choice.\n1) Try Again') + + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks @@ -364,23 +312,28 @@ def test_state_bad_input_to_select(self): started=True, config=self.ROUTER_CONFIG ) - yield self.check_state(router, { - 'ri_inbound': ('1', dict(from_addr='2323', - session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_BAD_INPUT, - }, - }, - 'expect': { - 'ri_outbound': ('Please select a choice.\n1) Flappy Bird', {}), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECT, - 'endpoints': '["flappy-bird"]', - }, - } - } + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_BAD_INPUT, + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + '1', + router=router, + from_addr='123', + session_event='resume' + ) + + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Please select a choice.\n1) Flappy Bird') + + yield self.assert_session_state('123', { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]', }) @inlineCallbacks @@ -399,22 +352,26 @@ def test_runtime_exception(self): 'target_endpoints', raise_error) - yield self.check_state(router, { - 'ri_inbound': (':menu', dict(from_addr='2323', - session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird' - }, - }, - 'expect': { - 'ri_outbound': ('Oops! Sorry!', {}), - 'session': { - '2323': {}, - } - } + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + 'Up!', + router=router, + from_addr='123', + session_event='resume' + ) + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Oops! Sorry!') + + yield self.assert_session_state('123', {}) + errors = self.flushLoggedErrors(RuntimeError) self.assertEqual(len(errors), 1) @@ -433,23 +390,25 @@ def test_session_invalidation(self): started=True, config=config ) - yield self.check_state(router, { - 'ri_inbound': ('Up!', dict(from_addr='2323', - session_event='resume')), - 'session': { - '2323': { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird' - }, - }, - 'expect': { - 'ri_outbound': ('Oops! Sorry!', dict(session_event='close')), - 'session': { - '2323': {}, - } - } + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', }) + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + 'Up!', + router=router, + from_addr='123', + session_event='resume' + ) + # assert that the user received a response + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], + 'Oops! Sorry!') + yield self.assert_session_state('123', {}) + def test_get_menu_choice(self): # good msg = self.router_helper.make_inbound(content='3 ') From b2be4617ad6688f5bd7439eecb0051f9a550ab36 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 11:53:55 +0200 Subject: [PATCH 34/53] Change app multiplexer module namespace to 'go.routers.app_multiplexer' --- go/config.py | 2 +- .../{application_multiplexer => app_multiplexer}/__init__.py | 0 .../{application_multiplexer => app_multiplexer}/common.py | 0 .../definition.py | 0 .../{application_multiplexer => app_multiplexer}/forms.py | 0 .../tests/__init__.py | 0 .../tests/test_views.py | 0 .../tests/test_vumi_app.py | 2 +- .../view_definition.py | 2 +- .../{application_multiplexer => app_multiplexer}/vumi_app.py | 4 ++-- 10 files changed, 5 insertions(+), 5 deletions(-) rename go/routers/{application_multiplexer => app_multiplexer}/__init__.py (100%) rename go/routers/{application_multiplexer => app_multiplexer}/common.py (100%) rename go/routers/{application_multiplexer => app_multiplexer}/definition.py (100%) rename go/routers/{application_multiplexer => app_multiplexer}/forms.py (100%) rename go/routers/{application_multiplexer => app_multiplexer}/tests/__init__.py (100%) rename go/routers/{application_multiplexer => app_multiplexer}/tests/test_views.py (100%) rename go/routers/{application_multiplexer => app_multiplexer}/tests/test_vumi_app.py (99%) rename go/routers/{application_multiplexer => app_multiplexer}/view_definition.py (88%) rename go/routers/{application_multiplexer => app_multiplexer}/vumi_app.py (98%) diff --git a/go/config.py b/go/config.py index 676eb7072..9a1e58fa2 100644 --- a/go/config.py +++ b/go/config.py @@ -136,7 +136,7 @@ def get_router_definition(router_type, router=None): 'namespace': 'keyword', 'display_name': 'Keyword', }, - 'go.routers.application_multiplexer': { + 'go.routers.app_multiplexer': { 'namespace': 'application_multiplexer', 'display_name': 'Application Multiplexer', }, diff --git a/go/routers/application_multiplexer/__init__.py b/go/routers/app_multiplexer/__init__.py similarity index 100% rename from go/routers/application_multiplexer/__init__.py rename to go/routers/app_multiplexer/__init__.py diff --git a/go/routers/application_multiplexer/common.py b/go/routers/app_multiplexer/common.py similarity index 100% rename from go/routers/application_multiplexer/common.py rename to go/routers/app_multiplexer/common.py diff --git a/go/routers/application_multiplexer/definition.py b/go/routers/app_multiplexer/definition.py similarity index 100% rename from go/routers/application_multiplexer/definition.py rename to go/routers/app_multiplexer/definition.py diff --git a/go/routers/application_multiplexer/forms.py b/go/routers/app_multiplexer/forms.py similarity index 100% rename from go/routers/application_multiplexer/forms.py rename to go/routers/app_multiplexer/forms.py diff --git a/go/routers/application_multiplexer/tests/__init__.py b/go/routers/app_multiplexer/tests/__init__.py similarity index 100% rename from go/routers/application_multiplexer/tests/__init__.py rename to go/routers/app_multiplexer/tests/__init__.py diff --git a/go/routers/application_multiplexer/tests/test_views.py b/go/routers/app_multiplexer/tests/test_views.py similarity index 100% rename from go/routers/application_multiplexer/tests/test_views.py rename to go/routers/app_multiplexer/tests/test_views.py diff --git a/go/routers/application_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py similarity index 99% rename from go/routers/application_multiplexer/tests/test_vumi_app.py rename to go/routers/app_multiplexer/tests/test_vumi_app.py index 6f0ca561e..594a8bae7 100644 --- a/go/routers/application_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -4,7 +4,7 @@ from twisted.internet.defer import inlineCallbacks from vumi.tests.helpers import VumiTestCase -from go.routers.application_multiplexer.vumi_app import ApplicationMultiplexer +from go.routers.app_multiplexer.vumi_app import ApplicationMultiplexer from go.routers.tests.helpers import RouterWorkerHelper diff --git a/go/routers/application_multiplexer/view_definition.py b/go/routers/app_multiplexer/view_definition.py similarity index 88% rename from go/routers/application_multiplexer/view_definition.py rename to go/routers/app_multiplexer/view_definition.py index 45e324835..e35324c76 100644 --- a/go/routers/application_multiplexer/view_definition.py +++ b/go/routers/app_multiplexer/view_definition.py @@ -1,6 +1,6 @@ from go.router.view_definition import RouterViewDefinitionBase, EditRouterView -from go.routers.application_multiplexer.forms import \ +from go.routers.app_multiplexer.forms import \ (ApplicationMultiplexerTitleForm, ApplicationMultiplexerFormSet) diff --git a/go/routers/application_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py similarity index 98% rename from go/routers/application_multiplexer/vumi_app.py rename to go/routers/app_multiplexer/vumi_app.py index bdecf316a..5bfea63db 100644 --- a/go/routers/application_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -1,4 +1,4 @@ -# -*- test-case-name: go.routers.application_multiplexer.tests.test_vumi_app -*- +# -*- test-case-name: go.routers.app_multiplexer.tests.test_vumi_app -*- import json from twisted.internet.defer import inlineCallbacks, returnValue @@ -9,7 +9,7 @@ from vumi.message import TransportUserMessage from go.vumitools.app_worker import GoRouterWorker -from go.routers.application_multiplexer.common import mkmenu, clean +from go.routers.app_multiplexer.common import mkmenu, clean class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): From b894b255a5278dbade82d00b6aeba9f2aa3681ef Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 12:10:45 +0200 Subject: [PATCH 35/53] Simplify state transition handling --- go/routers/app_multiplexer/vumi_app.py | 41 ++++++++++++-------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 5bfea63db..72e315a1d 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -58,14 +58,14 @@ class ApplicationMultiplexer(GoRouterWorker): | *----+ | | select | | bad_input | | +----* | - +----+----*------+ +----------------+ - | | - | | - +----*----+------+ - | | - | selected | - | | - +----------------+ + +----+----*--+---+ +----------------+ + | | \ - - + + | | \ + +----*----+------+ *----------------+ + | | | | + | selected +----* abort_session | + | | | | + +----------------+ +----------------+ """ CONFIG_CLASS = ApplicationMultiplexerConfig @@ -76,6 +76,7 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_SELECT = "select" STATE_SELECTED = "selected" STATE_BAD_INPUT = "bad_input" + STATE_ABORT_SESSION = "abort" def setup_router(self): d = super(ApplicationMultiplexer, self).setup_router() @@ -121,8 +122,9 @@ def handle_inbound(self, config, msg, conn_name): state = session['state'] try: - result = yield self.handlers[state](config, session, msg) - if result is None: + next_state, updated_session = yield self.handlers[state]( + config, session, msg) + if next_state == self.STATE_ABORT_SESSION: # Halt session immediately # The 'close' message has already been sent back to # the user at this point. @@ -131,13 +133,8 @@ def handle_inbound(self, config, msg, conn_name): yield session_manager.clear_session(user_id) yield self.publish_error_reply(msg, config) else: - if type(result) is tuple: - # Transition to next state AND mutate session data - next_state = session['state'] = result[0] - session.update(result[1]) - else: - # Transition to next state - next_state = session['state'] = result + session['state'] = next_state + session.update(updated_session) if state != next_state: log.msg("State transition for user %s: %s => %s" % (user_id, state, next_state)) @@ -168,10 +165,10 @@ def handle_state_select(self, config, session, msg): if endpoint is None: reply_msg = msg.reply(config.invalid_input_message) yield self.publish_outbound(reply_msg) - returnValue(self.STATE_BAD_INPUT) + returnValue((self.STATE_BAD_INPUT, {})) else: if endpoint not in self.target_endpoints(config): - returnValue(None) + returnValue((self.STATE_ABORT_SESSION, {})) else: forwarded_msg = self.forwarded_message( msg, @@ -188,7 +185,7 @@ def handle_state_select(self, config, session, msg): def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] if active_endpoint not in self.target_endpoints(config): - returnValue(None) + returnValue((self.STATE_ABORT_SESSION, {})) elif self.scan_for_keywords(config, msg, (config.keyword,)): reply_msg = msg.reply(self.create_menu(config)) yield self.publish_outbound(reply_msg) @@ -204,7 +201,7 @@ def handle_state_selected(self, config, session, msg): dict(active_endpoint=None))) else: yield self.publish_inbound(msg, active_endpoint) - returnValue(self.STATE_SELECTED) + returnValue((self.STATE_SELECTED, {})) @inlineCallbacks def handle_state_bad_input(self, config, session, msg): @@ -212,7 +209,7 @@ def handle_state_bad_input(self, config, session, msg): if choice is None: reply_msg = msg.reply(config.invalid_input_message) yield self.publish_outbound(reply_msg) - returnValue(self.STATE_BAD_INPUT) + returnValue((self.STATE_BAD_INPUT, {})) else: result = yield self.handle_state_start(config, session, msg) returnValue(result) From 078088919802d74cf490478c6fbac87187d413f0 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 13:23:03 +0200 Subject: [PATCH 36/53] Use a global session manager based on static config --- .../app_multiplexer/tests/test_vumi_app.py | 14 +++------ go/routers/app_multiplexer/vumi_app.py | 31 ++++++++----------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 594a8bae7..0ba67fef6 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -34,20 +34,16 @@ def setUp(self): RouterWorkerHelper(ApplicationMultiplexer)) self.router_worker = yield self.router_helper.get_router_worker({}) - @inlineCallbacks def setup_session(self, user_id, data): - session_manager = yield self.router_worker.session_manager( - self.router_worker.CONFIG_CLASS(self.router_worker.config) + return self.router_worker.session_manager.create_session( + user_id, + **data ) - # Initialize session data - yield session_manager.save_session(user_id, data) @inlineCallbacks def assert_session_state(self, user_id, expected_session): - session_manager = yield self.router_worker.session_manager( - self.router_worker.CONFIG_CLASS(self.router_worker.config) - ) - session = yield session_manager.load_session(user_id) + session = yield self.router_worker.session_manager.load_session( + user_id) if 'created_at' in session: del session['created_at'] self.assertEqual(session, expected_session, diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 72e315a1d..bc1ce9a01 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -78,25 +78,21 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_BAD_INPUT = "bad_input" STATE_ABORT_SESSION = "abort" + @inlineCallbacks def setup_router(self): - d = super(ApplicationMultiplexer, self).setup_router() + yield super(ApplicationMultiplexer, self).setup_router() + config = self.get_static_config() + self.session_manager = yield SessionManager.from_redis_config( + config.redis_manager, + key_prefix="application_multiplexer", + max_session_length=config.session_expiry + ) self.handlers = { self.STATE_START: self.handle_state_start, self.STATE_SELECT: self.handle_state_select, self.STATE_SELECTED: self.handle_state_selected, self.STATE_BAD_INPUT: self.handle_state_bad_input } - return d - - def session_manager(self, config): - """ - The implementation of SessionManager does the job of - appending ':session' to key names. - """ - return SessionManager.from_redis_config( - config.redis_manager, - max_session_length=config.session_expiry - ) def target_endpoints(self, config): """ @@ -109,14 +105,13 @@ def handle_inbound(self, config, msg, conn_name): log.msg("Processing inbound message: %s" % (msg,)) user_id = msg['from_addr'] - session_manager = yield self.session_manager(config) - session = yield session_manager.load_session(user_id) + session = yield self.session_manager.load_session(user_id) if not session: log.msg("Creating session for user %s" % user_id) session = {} state = self.STATE_START - yield session_manager.create_session(user_id) + yield self.session_manager.create_session(user_id) else: log.msg("Loading session for user %s: %s" % (user_id, session,)) state = session['state'] @@ -130,7 +125,7 @@ def handle_inbound(self, config, msg, conn_name): # the user at this point. log.msg(("Router configuration change forced session abort " "for user %s" % user_id)) - yield session_manager.clear_session(user_id) + yield self.session_manager.clear_session(user_id) yield self.publish_error_reply(msg, config) else: session['state'] = next_state @@ -138,10 +133,10 @@ def handle_inbound(self, config, msg, conn_name): if state != next_state: log.msg("State transition for user %s: %s => %s" % (user_id, state, next_state)) - yield session_manager.save_session(user_id, session) + yield self.session_manager.save_session(user_id, session) except: log.err() - yield session_manager.clear_session(user_id) + yield self.session_manager.clear_session(user_id) yield self.publish_error_reply(msg, config) @inlineCallbacks From aeceb8295c6b4b3850ac924ee43189d793d8a965 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 13:42:02 +0200 Subject: [PATCH 37/53] Remove unnecessary newlines from multiplexer tests --- .../app_multiplexer/tests/test_vumi_app.py | 89 ++++--------------- 1 file changed, 19 insertions(+), 70 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 0ba67fef6..45f776e2a 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -36,9 +36,7 @@ def setUp(self): def setup_session(self, user_id, data): return self.router_worker.session_manager.create_session( - user_id, - **data - ) + user_id, **data) @inlineCallbacks def assert_session_state(self, user_id, expected_session): @@ -104,14 +102,10 @@ def test_no_messages_processed_while_stopped(self): @inlineCallbacks def test_state_start_to_select(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) # msg sent from user yield self.router_helper.ri.make_dispatch_inbound( - None, - router=router, - from_addr='123') + None, router=router, from_addr='123') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() @@ -126,9 +120,7 @@ def test_state_start_to_select(self): @inlineCallbacks def test_state_select_to_selected(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_SELECT, @@ -137,11 +129,7 @@ def test_state_select_to_selected(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - '1', - router=router, - from_addr='123', - session_event='resume' - ) + '1', router=router, from_addr='123', session_event='resume') # assert that message is forwarded to application [msg] = self.router_helper.ro.get_dispatched_inbound() @@ -153,8 +141,7 @@ def test_state_select_to_selected(self): # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() - self.assertEqual(msg['content'], - 'Flappy Flappy!') + self.assertEqual(msg['content'], 'Flappy Flappy!') yield self.assert_session_state('123', { 'state': ApplicationMultiplexer.STATE_SELECTED, @@ -165,9 +152,7 @@ def test_state_select_to_selected(self): @inlineCallbacks def test_state_selected_to_selected(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_SELECTED, @@ -177,11 +162,7 @@ def test_state_selected_to_selected(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - 'Up!', - router=router, - from_addr='123', - session_event='resume' - ) + 'Up!', router=router, from_addr='123', session_event='resume') # assert that message is forwarded to application [msg] = self.router_helper.ro.get_dispatched_inbound() @@ -190,9 +171,7 @@ def test_state_selected_to_selected(self): # application sends reply yield self.router_helper.ro.make_dispatch_reply( - msg, - 'Game Over!\n1) Try Again!' - ) + msg, 'Game Over!\n1) Try Again!') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() @@ -208,9 +187,7 @@ def test_state_selected_to_selected(self): @inlineCallbacks def test_state_selected_to_select(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_SELECTED, @@ -220,11 +197,7 @@ def test_state_selected_to_select(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - ':menu', - router=router, - from_addr='123', - session_event='resume' - ) + ':menu', router=router, from_addr='123', session_event='resume') # assert that a close message is forwarded to application [msg] = self.router_helper.ro.get_dispatched_inbound() @@ -256,11 +229,7 @@ def test_state_select_to_bad_input(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - 'foo', - router=router, - from_addr='123', - session_event='resume' - ) + 'foo', router=router, from_addr='123', session_event='resume') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() @@ -286,11 +255,7 @@ def test_state_bad_input_to_bad_input(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - 'foo', - router=router, - from_addr='123', - session_event='resume' - ) + 'foo', router=router, from_addr='123', session_event='resume') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() @@ -316,11 +281,7 @@ def test_state_bad_input_to_select(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - '1', - router=router, - from_addr='123', - session_event='resume' - ) + '1', router=router, from_addr='123', session_event='resume') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() @@ -356,11 +317,7 @@ def test_runtime_exception(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - 'Up!', - router=router, - from_addr='123', - session_event='resume' - ) + 'Up!', router=router, from_addr='123', session_event='resume') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() self.assertEqual(msg['content'], @@ -383,9 +340,7 @@ def test_session_invalidation(self): config = copy.deepcopy(self.ROUTER_CONFIG) config['entries'][0]['endpoint'] = 'mama' router = yield self.router_helper.create_router( - started=True, - config=config - ) + started=True, config=config) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', @@ -394,11 +349,7 @@ def test_session_invalidation(self): # msg sent from user msg = yield self.router_helper.ri.make_dispatch_inbound( - 'Up!', - router=router, - from_addr='123', - session_event='resume' - ) + 'Up!', router=router, from_addr='123', session_event='resume') # assert that the user received a response [msg] = self.router_helper.ri.get_dispatched_outbound() self.assertEqual(msg['content'], @@ -426,12 +377,10 @@ def test_scan_for_keywords(self): }) msg = self.router_helper.make_inbound(content=':menu') self.assertTrue(self.router_worker.scan_for_keywords( - config, - msg, (':menu',))) + config, msg, (':menu',))) msg = self.router_helper.make_inbound(content='Foo bar baz') self.assertFalse(self.router_worker.scan_for_keywords( - config, - msg, (':menu',))) + config, msg, (':menu',))) def test_create_menu(self): config = self.dynamic_config({ From 83318010ceaabc061c6f0f8323e1b9aea925c574 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 13:46:05 +0200 Subject: [PATCH 38/53] Remove keyword scanning feature from multiplexer --- .../app_multiplexer/tests/test_vumi_app.py | 44 ------------------- go/routers/app_multiplexer/vumi_app.py | 22 ---------- 2 files changed, 66 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 45f776e2a..8f436e1a4 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -1,5 +1,4 @@ import copy -import json from twisted.internet.defer import inlineCallbacks @@ -19,7 +18,6 @@ class TestApplicationMultiplexerRouter(VumiTestCase): ROUTER_CONFIG = { 'invalid_input_message': 'Bad choice.\n1) Try Again', 'error_message': 'Oops! Sorry!', - 'keyword': ':menu', 'entries': [ { 'label': 'Flappy Bird', @@ -184,37 +182,6 @@ def test_state_selected_to_selected(self): 'endpoints': '["flappy-bird"]', }) - @inlineCallbacks - def test_state_selected_to_select(self): - router = yield self.router_helper.create_router( - started=True, config=self.ROUTER_CONFIG) - - yield self.setup_session('123', { - 'state': ApplicationMultiplexer.STATE_SELECTED, - 'active_endpoint': 'flappy-bird', - 'endpoints': '["flappy-bird"]', - }) - - # msg sent from user - msg = yield self.router_helper.ri.make_dispatch_inbound( - ':menu', router=router, from_addr='123', session_event='resume') - - # assert that a close message is forwarded to application - [msg] = self.router_helper.ro.get_dispatched_inbound() - self.assertEqual(msg['content'], None) - self.assertEqual(msg['session_event'], 'close') - - # assert that the user received a response - [msg] = self.router_helper.ri.get_dispatched_outbound() - self.assertEqual(msg['content'], - 'Please select a choice.\n1) Flappy Bird') - - yield self.assert_session_state('123', { - 'state': ApplicationMultiplexer.STATE_SELECT, - 'active_endpoint': 'None', - 'endpoints': '["flappy-bird"]', - }) - @inlineCallbacks def test_state_select_to_bad_input(self): router = yield self.router_helper.create_router( @@ -371,17 +338,6 @@ def test_get_menu_choice(self): choice = self.router_worker.get_menu_choice(msg, (1, 2)) self.assertEqual(choice, None) - def test_scan_for_keywords(self): - config = self.dynamic_config({ - 'keyword': ':menu' - }) - msg = self.router_helper.make_inbound(content=':menu') - self.assertTrue(self.router_worker.scan_for_keywords( - config, msg, (':menu',))) - msg = self.router_helper.make_inbound(content='Foo bar baz') - self.assertFalse(self.router_worker.scan_for_keywords( - config, msg, (':menu',))) - def test_create_menu(self): config = self.dynamic_config({ 'menu_title': {'content': 'Please select a choice'}, diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index bc1ce9a01..23220de3a 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -26,9 +26,6 @@ class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): entries = ConfigList( "A list of application endpoints and associated labels", default=[]) - keyword = ConfigText( - "Keyword to signal a request to return to the application menu", - default=None) invalid_input_message = ConfigText( "Prompt to display when warning about an invalid choice", default=("That is an incorrect choice. Please enter the number " @@ -181,19 +178,6 @@ def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] if active_endpoint not in self.target_endpoints(config): returnValue((self.STATE_ABORT_SESSION, {})) - elif self.scan_for_keywords(config, msg, (config.keyword,)): - reply_msg = msg.reply(self.create_menu(config)) - yield self.publish_outbound(reply_msg) - - # Be polite and pass a SESSION_CLOSE to the active endpoint - forwarded_msg = self.forwarded_message( - msg, - content=None, - session_event=TransportUserMessage.SESSION_CLOSE - ) - yield self.publish_inbound(forwarded_msg, active_endpoint) - returnValue((self.STATE_SELECT, - dict(active_endpoint=None))) else: yield self.publish_inbound(msg, active_endpoint) returnValue((self.STATE_SELECTED, {})) @@ -235,12 +219,6 @@ def forwarded_message(self, msg, **kwargs): copy[k] = v return copy - def scan_for_keywords(self, config, msg, keywords): - first_word = (clean(msg['content']).split() + [''])[0] - if first_word in keywords: - return True - return False - def get_endpoint_for_choice(self, msg, session): """ Retrieves the candidate endpoint based on the user's numeric choice From 7a812233fe8859ca479d21258a89cf6b538875a5 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 14:56:54 +0200 Subject: [PATCH 39/53] handle session 'close' messages in both directions --- go/routers/app_multiplexer/vumi_app.py | 50 +++++++++++++++++--------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 23220de3a..4405db481 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -60,7 +60,7 @@ class ApplicationMultiplexer(GoRouterWorker): | | \ +----*----+------+ *----------------+ | | | | - | selected +----* abort_session | + | selected +----* abort | | | | | +----------------+ +----------------+ """ @@ -73,7 +73,7 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_SELECT = "select" STATE_SELECTED = "selected" STATE_BAD_INPUT = "bad_input" - STATE_ABORT_SESSION = "abort" + STATE_ABORT = "abort" @inlineCallbacks def setup_router(self): @@ -88,7 +88,7 @@ def setup_router(self): self.STATE_START: self.handle_state_start, self.STATE_SELECT: self.handle_state_select, self.STATE_SELECTED: self.handle_state_selected, - self.STATE_BAD_INPUT: self.handle_state_bad_input + self.STATE_BAD_INPUT: self.handle_state_bad_input, } def target_endpoints(self, config): @@ -102,13 +102,16 @@ def handle_inbound(self, config, msg, conn_name): log.msg("Processing inbound message: %s" % (msg,)) user_id = msg['from_addr'] - session = yield self.session_manager.load_session(user_id) - if not session: + session_event = msg['session_event'] + if not session or session_event == TransportUserMessage.SESSION_NEW: log.msg("Creating session for user %s" % user_id) session = {} state = self.STATE_START yield self.session_manager.create_session(user_id) + elif session_event == TransportUserMessage.SESSION_CLOSE: + self.handle_session_close(self, config, session, msg) + return else: log.msg("Loading session for user %s: %s" % (user_id, session,)) state = session['state'] @@ -116,10 +119,8 @@ def handle_inbound(self, config, msg, conn_name): try: next_state, updated_session = yield self.handlers[state]( config, session, msg) - if next_state == self.STATE_ABORT_SESSION: + if next_state == self.STATE_ABORT: # Halt session immediately - # The 'close' message has already been sent back to - # the user at this point. log.msg(("Router configuration change forced session abort " "for user %s" % user_id)) yield self.session_manager.clear_session(user_id) @@ -160,7 +161,7 @@ def handle_state_select(self, config, session, msg): returnValue((self.STATE_BAD_INPUT, {})) else: if endpoint not in self.target_endpoints(config): - returnValue((self.STATE_ABORT_SESSION, {})) + returnValue((self.STATE_ABORT, {})) else: forwarded_msg = self.forwarded_message( msg, @@ -177,7 +178,7 @@ def handle_state_select(self, config, session, msg): def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] if active_endpoint not in self.target_endpoints(config): - returnValue((self.STATE_ABORT_SESSION, {})) + returnValue((self.STATE_ABORT, {})) else: yield self.publish_inbound(msg, active_endpoint) returnValue((self.STATE_SELECTED, {})) @@ -193,18 +194,33 @@ def handle_state_bad_input(self, config, session, msg): result = yield self.handle_state_start(config, session, msg) returnValue(result) + @inlineCallbacks def handle_outbound(self, config, msg, conn_name): - """ - TODO: Go to SELECT state when session_event=close - """ log.msg("Processing outbound message: %s" % (msg,)) - return self.publish_outbound(msg) + user_id = msg['to_addr'] + session_event = msg['session_event'] + session = yield self.session_manager.load_session(user_id) + if session and (session_event == TransportUserMessage.SESSION_CLOSE): + yield self.session_manager.clear_session(user_id) + yield self.publish_outbound(msg) + + @inlineCallbacks + def handle_session_close(self, config, session, msg): + user_id = msg['to_addr'] + session_event = msg['session_event'] + if session_event == self.STATE_SELECTED and \ + session['active_endpoint'] in self.target_endpoints(config): + yield self.publish_inbound( + self.forwarded_message( + msg, content=None, + session_event=TransportUserMessage.SESSION_CLOSE), + session['active_endpoint'] + ) + yield self.session_manager.clear_session(user_id) def publish_outbound(self, msg): return super(ApplicationMultiplexer, self).publish_outbound( - msg, - "default" - ) + msg, "default") def publish_error_reply(self, msg, config): reply_msg = msg.reply( From dd6d269553d0abb00310aa295bc34b3ca2dae444 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 15:22:03 +0200 Subject: [PATCH 40/53] Add tests for handling of 'close' messages --- .../app_multiplexer/tests/test_vumi_app.py | 27 +++++++++++++++++++ go/routers/app_multiplexer/vumi_app.py | 7 +++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 8f436e1a4..8dddd0824 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -323,6 +323,33 @@ def test_session_invalidation(self): 'Oops! Sorry!') yield self.assert_session_state('123', {}) + @inlineCallbacks + def test_state_selected_receive_close_inbound(self): + router = yield self.router_helper.create_router( + started=True, config=self.ROUTER_CONFIG) + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + None, router=router, from_addr='123', session_event='close') + + # assert app received forwarded 'close' message + [msg] = self.router_helper.ro.get_dispatched_inbound() + self.assertEqual(msg['content'], None) + self.assertEqual(msg['session_event'], 'close') + + # assert that no response sent to user + msgs = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msgs, []) + + # assert that session cleared + yield self.assert_session_state('123', {}) + def test_get_menu_choice(self): # good msg = self.router_helper.make_inbound(content='3 ') diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 4405db481..334ac2839 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -110,7 +110,7 @@ def handle_inbound(self, config, msg, conn_name): state = self.STATE_START yield self.session_manager.create_session(user_id) elif session_event == TransportUserMessage.SESSION_CLOSE: - self.handle_session_close(self, config, session, msg) + yield self.handle_session_close(config, session, msg) return else: log.msg("Loading session for user %s: %s" % (user_id, session,)) @@ -206,9 +206,8 @@ def handle_outbound(self, config, msg, conn_name): @inlineCallbacks def handle_session_close(self, config, session, msg): - user_id = msg['to_addr'] - session_event = msg['session_event'] - if session_event == self.STATE_SELECTED and \ + user_id = msg['from_addr'] + if session.get('state', None) == self.STATE_SELECTED and \ session['active_endpoint'] in self.target_endpoints(config): yield self.publish_inbound( self.forwarded_message( From 280404e7dde69b1087f5239e05aa3a26a2043a3c Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 15:30:57 +0200 Subject: [PATCH 41/53] Last batch of tests for handling 'close' messages --- .../app_multiplexer/tests/test_vumi_app.py | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 8dddd0824..6a50996ea 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -350,6 +350,63 @@ def test_state_selected_receive_close_inbound(self): # assert that session cleared yield self.assert_session_state('123', {}) + @inlineCallbacks + def test_receive_close_inbound(self): + """ + Same as the above test, but when in any state other than + STATE_SELECTED. + """ + router = yield self.router_helper.create_router( + started=True, config=self.ROUTER_CONFIG) + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECT, + 'endpoints': '["flappy-bird"]' + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + None, router=router, from_addr='123', session_event='close') + + # assert that no app received a forwarded 'close' message + msgs = self.router_helper.ro.get_dispatched_inbound() + self.assertEqual(msgs, []) + self.assertEqual(msg['session_event'], 'close') + + # assert that no response sent to user + msgs = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msgs, []) + + # assert that session cleared + yield self.assert_session_state('123', {}) + + @inlineCallbacks + def test_receive_close_outbound(self): + router = yield self.router_helper.create_router( + started=True, config=self.ROUTER_CONFIG) + + yield self.setup_session('123', { + 'state': ApplicationMultiplexer.STATE_SELECTED, + 'active_endpoint': 'flappy-bird', + 'endpoints': '["flappy-bird"]', + }) + + # msg sent from user + msg = yield self.router_helper.ri.make_dispatch_inbound( + "3", router=router, from_addr='123', session_event='resume') + + # application quits session + yield self.router_helper.ro.make_dispatch_reply( + msg, 'Game Over!', session_event='close') + + # assert that user receives the forwarded 'close' message + [msg] = self.router_helper.ri.get_dispatched_outbound() + self.assertEqual(msg['content'], 'Game Over!') + self.assertEqual(msg['session_event'], 'close') + + # assert that session cleared + yield self.assert_session_state('123', {}) + def test_get_menu_choice(self): # good msg = self.router_helper.make_inbound(content='3 ') From 22e2192e37d04f47df7f7ac5bb0b5b20b8802d9d Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 15:35:14 +0200 Subject: [PATCH 42/53] Minor test cleanups for multiplexer --- .../app_multiplexer/tests/test_vumi_app.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 6a50996ea..7a1d214ed 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -185,9 +185,7 @@ def test_state_selected_to_selected(self): @inlineCallbacks def test_state_select_to_bad_input(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_SELECT, @@ -211,9 +209,7 @@ def test_state_select_to_bad_input(self): @inlineCallbacks def test_state_bad_input_to_bad_input(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, @@ -237,9 +233,7 @@ def test_state_bad_input_to_bad_input(self): @inlineCallbacks def test_state_bad_input_to_select(self): router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) yield self.setup_session('123', { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, @@ -267,9 +261,7 @@ def test_runtime_exception(self): and sends an appropriate error message back to the user """ router = yield self.router_helper.create_router( - started=True, - config=self.ROUTER_CONFIG - ) + started=True, config=self.ROUTER_CONFIG) # Make worker.target_endpoints raise an exception self.patch(self.router_worker, From 29c73062bc42fdc1171a317d10782f19d1d7fb50 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 15:42:15 +0200 Subject: [PATCH 43/53] Remove unnecessary ABORT state --- go/routers/app_multiplexer/vumi_app.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 334ac2839..cdeadeb73 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -55,14 +55,14 @@ class ApplicationMultiplexer(GoRouterWorker): | *----+ | | select | | bad_input | | +----* | - +----+----*--+---+ +----------------+ - | | \ - - + - | | \ - +----*----+------+ *----------------+ - | | | | - | selected +----* abort | - | | | | - +----------------+ +----------------+ + +----+----*------+ +----------------+ + | | + | | + +----*----+------+ + | | + | selected + + | | + +----------------+ """ CONFIG_CLASS = ApplicationMultiplexerConfig @@ -73,7 +73,6 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_SELECT = "select" STATE_SELECTED = "selected" STATE_BAD_INPUT = "bad_input" - STATE_ABORT = "abort" @inlineCallbacks def setup_router(self): @@ -119,7 +118,7 @@ def handle_inbound(self, config, msg, conn_name): try: next_state, updated_session = yield self.handlers[state]( config, session, msg) - if next_state == self.STATE_ABORT: + if next_state is None: # Halt session immediately log.msg(("Router configuration change forced session abort " "for user %s" % user_id)) @@ -161,7 +160,7 @@ def handle_state_select(self, config, session, msg): returnValue((self.STATE_BAD_INPUT, {})) else: if endpoint not in self.target_endpoints(config): - returnValue((self.STATE_ABORT, {})) + returnValue((None, {})) else: forwarded_msg = self.forwarded_message( msg, @@ -178,7 +177,7 @@ def handle_state_select(self, config, session, msg): def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] if active_endpoint not in self.target_endpoints(config): - returnValue((self.STATE_ABORT, {})) + returnValue((None, {})) else: yield self.publish_inbound(msg, active_endpoint) returnValue((self.STATE_SELECTED, {})) From 4ce79e6f51ff531937270050e0f14ad4ac0818b7 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Mon, 24 Feb 2014 15:49:48 +0200 Subject: [PATCH 44/53] change multiplexer namespace to 'app_multiplexer' --- go/config.py | 2 +- go/routers/app_multiplexer/definition.py | 2 +- go/routers/app_multiplexer/tests/test_views.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/config.py b/go/config.py index 9a1e58fa2..7c7913e9e 100644 --- a/go/config.py +++ b/go/config.py @@ -137,7 +137,7 @@ def get_router_definition(router_type, router=None): 'display_name': 'Keyword', }, 'go.routers.app_multiplexer': { - 'namespace': 'application_multiplexer', + 'namespace': 'app_multiplexer', 'display_name': 'Application Multiplexer', }, } diff --git a/go/routers/app_multiplexer/definition.py b/go/routers/app_multiplexer/definition.py index dd7496b7b..89fc331c3 100644 --- a/go/routers/app_multiplexer/definition.py +++ b/go/routers/app_multiplexer/definition.py @@ -2,7 +2,7 @@ class RouterDefinition(RouterDefinitionBase): - router_type = 'application_multiplexer' + router_type = 'app_multiplexer' def configured_outbound_endpoints(self, config): endpoints = [entry['endpoint'] for entry in config.get('entries', [])] diff --git a/go/routers/app_multiplexer/tests/test_views.py b/go/routers/app_multiplexer/tests/test_views.py index 6458a6a38..277caee88 100644 --- a/go/routers/app_multiplexer/tests/test_views.py +++ b/go/routers/app_multiplexer/tests/test_views.py @@ -7,7 +7,7 @@ class ApplicationMultiplexerViewTests(GoDjangoTestCase): def setUp(self): self.router_helper = self.add_helper( - RouterViewsHelper(u'application_multiplexer') + RouterViewsHelper(u'app_multiplexer') ) self.user_helper = self.router_helper.vumi_helper.get_or_create_user() self.client = self.router_helper.get_client() @@ -18,7 +18,7 @@ def test_new_router(self): response = self.client.post(self.router_helper.get_new_view_url(), { 'name': u"myrouter", - 'router_type': u'application_multiplexer', + 'router_type': u'app_multiplexer', }) [router_key] = router_store.list_routers() rtr_helper = self.router_helper.get_router_helper_by_key(router_key) From b5f9dbff93102c11209afa5bfbfc65497baf9adc Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 12:20:41 +0200 Subject: [PATCH 45/53] Revert to using a dynamic session manager --- .../app_multiplexer/tests/test_vumi_app.py | 14 +++++--- go/routers/app_multiplexer/vumi_app.py | 36 ++++++++++--------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 7a1d214ed..603a68dcf 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -32,14 +32,20 @@ def setUp(self): RouterWorkerHelper(ApplicationMultiplexer)) self.router_worker = yield self.router_helper.get_router_worker({}) + @inlineCallbacks def setup_session(self, user_id, data): - return self.router_worker.session_manager.create_session( - user_id, **data) + session_manager = yield self.router_worker.session_manager( + self.router_worker.CONFIG_CLASS(self.router_worker.config) + ) + # Initialize session data + yield session_manager.save_session(user_id, data) @inlineCallbacks def assert_session_state(self, user_id, expected_session): - session = yield self.router_worker.session_manager.load_session( - user_id) + session_manager = yield self.router_worker.session_manager( + self.router_worker.CONFIG_CLASS(self.router_worker.config) + ) + session = yield session_manager.load_session(user_id) if 'created_at' in session: del session['created_at'] self.assertEqual(session, expected_session, diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index cdeadeb73..eb83dab38 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -74,21 +74,22 @@ class ApplicationMultiplexer(GoRouterWorker): STATE_SELECTED = "selected" STATE_BAD_INPUT = "bad_input" - @inlineCallbacks def setup_router(self): - yield super(ApplicationMultiplexer, self).setup_router() - config = self.get_static_config() - self.session_manager = yield SessionManager.from_redis_config( - config.redis_manager, - key_prefix="application_multiplexer", - max_session_length=config.session_expiry - ) + d = super(ApplicationMultiplexer, self).setup_router() self.handlers = { self.STATE_START: self.handle_state_start, self.STATE_SELECT: self.handle_state_select, self.STATE_SELECTED: self.handle_state_selected, self.STATE_BAD_INPUT: self.handle_state_bad_input, } + return d + + def session_manager(self, config): + return SessionManager.from_redis_config( + config.redis_manager, + key_prefix=self.worker_name, + max_session_length=config.session_expiry + ) def target_endpoints(self, config): """ @@ -101,13 +102,14 @@ def handle_inbound(self, config, msg, conn_name): log.msg("Processing inbound message: %s" % (msg,)) user_id = msg['from_addr'] - session = yield self.session_manager.load_session(user_id) + session_manager = yield self.session_manager(config) + session = yield session_manager.load_session(user_id) session_event = msg['session_event'] if not session or session_event == TransportUserMessage.SESSION_NEW: log.msg("Creating session for user %s" % user_id) session = {} state = self.STATE_START - yield self.session_manager.create_session(user_id) + yield session_manager.create_session(user_id) elif session_event == TransportUserMessage.SESSION_CLOSE: yield self.handle_session_close(config, session, msg) return @@ -122,7 +124,7 @@ def handle_inbound(self, config, msg, conn_name): # Halt session immediately log.msg(("Router configuration change forced session abort " "for user %s" % user_id)) - yield self.session_manager.clear_session(user_id) + yield session_manager.clear_session(user_id) yield self.publish_error_reply(msg, config) else: session['state'] = next_state @@ -130,10 +132,10 @@ def handle_inbound(self, config, msg, conn_name): if state != next_state: log.msg("State transition for user %s: %s => %s" % (user_id, state, next_state)) - yield self.session_manager.save_session(user_id, session) + yield session_manager.save_session(user_id, session) except: log.err() - yield self.session_manager.clear_session(user_id) + yield session_manager.clear_session(user_id) yield self.publish_error_reply(msg, config) @inlineCallbacks @@ -198,9 +200,10 @@ def handle_outbound(self, config, msg, conn_name): log.msg("Processing outbound message: %s" % (msg,)) user_id = msg['to_addr'] session_event = msg['session_event'] - session = yield self.session_manager.load_session(user_id) + session_manager = yield self.session_manager(config) + session = yield session_manager.load_session(user_id) if session and (session_event == TransportUserMessage.SESSION_CLOSE): - yield self.session_manager.clear_session(user_id) + yield session_manager.clear_session(user_id) yield self.publish_outbound(msg) @inlineCallbacks @@ -214,7 +217,8 @@ def handle_session_close(self, config, session, msg): session_event=TransportUserMessage.SESSION_CLOSE), session['active_endpoint'] ) - yield self.session_manager.clear_session(user_id) + session_manager = yield self.session_manager(config) + yield session_manager.clear_session(user_id) def publish_outbound(self, msg): return super(ApplicationMultiplexer, self).publish_outbound( From 7fe7f3e4360a49baf14e415848391e3d729745a4 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 12:23:16 +0200 Subject: [PATCH 46/53] remove unused smart_truncate function --- go/routers/app_multiplexer/common.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/go/routers/app_multiplexer/common.py b/go/routers/app_multiplexer/common.py index 7baea08b0..bb06e68d8 100644 --- a/go/routers/app_multiplexer/common.py +++ b/go/routers/app_multiplexer/common.py @@ -11,15 +11,3 @@ def clean(content): def mkmenu(options, start=1, format='%s) %s'): items = [format % (idx, opt) for idx, opt in enumerate(options, start)] return '\n'.join(items) - - -def smart_truncate(content, k, sfx='...'): - """ - Useful for truncating text strings in the following manner: - - 'MyFancyTeam' => 'MyFancyT...' - """ - if len(content) <= k: - return content - else: - return content[:k - len(sfx)] + sfx From 45f53b4544272bb5453ef41f6826081f714f7505 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 12:24:14 +0200 Subject: [PATCH 47/53] Reduce session expiration to 5min for multiplexor sessions --- go/routers/app_multiplexer/vumi_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index eb83dab38..9af3317d3 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -17,7 +17,7 @@ class ApplicationMultiplexerConfig(GoRouterWorker.CONFIG_CLASS): # Static configuration session_expiry = ConfigInt( "Maximum amount of time in seconds to keep session data around", - default=1800, static=True) + default=300, static=True) # Dynamic, per-message configuration menu_title = ConfigDict( From e3b47bf36ba889a1a24cae933533c9044ccea07c Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 12:53:56 +0200 Subject: [PATCH 48/53] Various minor cleanups as per review comments --- go/routers/app_multiplexer/tests/test_vumi_app.py | 12 ++---------- go/routers/app_multiplexer/view_definition.py | 4 ++-- go/routers/app_multiplexer/vumi_app.py | 4 ++-- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 603a68dcf..f32da8354 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -13,8 +13,6 @@ def raise_error(*args, **kw): class TestApplicationMultiplexerRouter(VumiTestCase): - router_class = ApplicationMultiplexer - ROUTER_CONFIG = { 'invalid_input_message': 'Bad choice.\n1) Try Again', 'error_message': 'Oops! Sorry!', @@ -51,11 +49,6 @@ def assert_session_state(self, user_id, expected_session): self.assertEqual(session, expected_session, msg="Unexpected session data") - def dynamic_config(self, fields): - config = self.router_worker.config.copy() - config.update(fields) - return config - @inlineCallbacks def assert_routed_inbound(self, content, router, expected_endpoint): msg = yield self.router_helper.ri.make_dispatch_inbound( @@ -421,7 +414,7 @@ def test_get_menu_choice(self): self.assertEqual(choice, None) def test_create_menu(self): - config = self.dynamic_config({ + router_worker = yield self.router_helper.get_router_worker({ 'menu_title': {'content': 'Please select a choice'}, 'entries': [ { @@ -434,8 +427,7 @@ def test_create_menu(self): } ] }) - config = self.router_worker.CONFIG_CLASS(config) - text = self.router_worker.create_menu(config) + text = router_worker.create_menu(self.router_worker.config) self.assertEqual( text, 'Please select a choice\n1) Flappy Bird\n2) Mama' diff --git a/go/routers/app_multiplexer/view_definition.py b/go/routers/app_multiplexer/view_definition.py index e35324c76..37a978888 100644 --- a/go/routers/app_multiplexer/view_definition.py +++ b/go/routers/app_multiplexer/view_definition.py @@ -1,7 +1,7 @@ from go.router.view_definition import RouterViewDefinitionBase, EditRouterView -from go.routers.app_multiplexer.forms import \ - (ApplicationMultiplexerTitleForm, ApplicationMultiplexerFormSet) +from go.routers.app_multiplexer.forms import (ApplicationMultiplexerTitleForm, + ApplicationMultiplexerFormSet) class EditApplicationMultiplexerView(EditRouterView): diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 9af3317d3..a588d407f 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -209,8 +209,8 @@ def handle_outbound(self, config, msg, conn_name): @inlineCallbacks def handle_session_close(self, config, session, msg): user_id = msg['from_addr'] - if session.get('state', None) == self.STATE_SELECTED and \ - session['active_endpoint'] in self.target_endpoints(config): + if (session.get('state', None) == self.STATE_SELECTED and + session['active_endpoint'] in self.target_endpoints(config)): yield self.publish_inbound( self.forwarded_message( msg, content=None, From 3727f564ac6a075477356fe2b5aa1134a4042d66 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 13:22:56 +0200 Subject: [PATCH 49/53] Just forward a 'close' message without trying to modify it --- go/routers/app_multiplexer/vumi_app.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index a588d407f..ab0605c93 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -211,12 +211,7 @@ def handle_session_close(self, config, session, msg): user_id = msg['from_addr'] if (session.get('state', None) == self.STATE_SELECTED and session['active_endpoint'] in self.target_endpoints(config)): - yield self.publish_inbound( - self.forwarded_message( - msg, content=None, - session_event=TransportUserMessage.SESSION_CLOSE), - session['active_endpoint'] - ) + yield self.publish_inbound(msg, session['active_endpoint']) session_manager = yield self.session_manager(config) yield session_manager.clear_session(user_id) From a2d2a0082d68b213308f73a0824b18e26e9ae330 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 13:58:30 +0200 Subject: [PATCH 50/53] Cleanup up some multiplexer tests, add test docs, etc --- .../app_multiplexer/tests/test_vumi_app.py | 66 +++++++++++++------ 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index f32da8354..1ec393033 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -49,15 +49,6 @@ def assert_session_state(self, user_id, expected_session): self.assertEqual(session, expected_session, msg="Unexpected session data") - @inlineCallbacks - def assert_routed_inbound(self, content, router, expected_endpoint): - msg = yield self.router_helper.ri.make_dispatch_inbound( - content, router=router) - emsg = msg.copy() - emsg.set_routing_endpoint(expected_endpoint) - rmsg = self.router_helper.ro.get_dispatched_inbound()[-1] - self.assertEqual(emsg, rmsg) - @inlineCallbacks def test_start(self): router = yield self.router_helper.create_router() @@ -97,7 +88,10 @@ def test_no_messages_processed_while_stopped(self): self.assertEqual(nack['event_type'], 'nack') @inlineCallbacks - def test_state_start_to_select(self): + def test_new_session_display_menu(self): + """ + Prompt user to choice an application endpoint. + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) # msg sent from user @@ -115,7 +109,11 @@ def test_state_start_to_select(self): }) @inlineCallbacks - def test_state_select_to_selected(self): + def test_select_application_endpoint(self): + """ + Retrieve endpoint choice from user and set currently active + endpoint. + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -147,7 +145,10 @@ def test_state_select_to_selected(self): }) @inlineCallbacks - def test_state_selected_to_selected(self): + def test_session_with_selected_endpoint(self): + """ + Tests an ongoing USSD session with a previously selected endpoint + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -182,7 +183,10 @@ def test_state_selected_to_selected(self): }) @inlineCallbacks - def test_state_select_to_bad_input(self): + def test_bad_input_for_endpoint_choice(self): + """ + User entered bad input for the endpoint selection menu. + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -206,7 +210,11 @@ def test_state_select_to_bad_input(self): }) @inlineCallbacks - def test_state_bad_input_to_bad_input(self): + def test_state_bad_input_for_bad_input_prompt(self): + """ + User entered bad input for the prompt telling the user + that they entered bad input (ha! recursive). + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -230,7 +238,11 @@ def test_state_bad_input_to_bad_input(self): }) @inlineCallbacks - def test_state_bad_input_to_select(self): + def test_state_good_input_for_bad_input_prompt(self): + """ + User entered good input for the prompt telling the user + that they entered bad input. + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -254,7 +266,7 @@ def test_state_bad_input_to_select(self): }) @inlineCallbacks - def test_runtime_exception(self): + def test_runtime_exception_in_selected_handler(self): """ Verifies that the worker handles an arbitrary runtime error gracefully, and sends an appropriate error message back to the user @@ -287,7 +299,7 @@ def test_runtime_exception(self): self.assertEqual(len(errors), 1) @inlineCallbacks - def test_session_invalidation(self): + def test_session_invalidation_in_state_handler(self): """ Verify that the router gracefully handles a configuration update while there is an active user session. @@ -316,6 +328,11 @@ def test_session_invalidation(self): @inlineCallbacks def test_state_selected_receive_close_inbound(self): + """ + User sends 'close' msg to the active endpoint via the router. + Verify that the message is forwarded and that the session for + the user is cleared. + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -344,8 +361,8 @@ def test_state_selected_receive_close_inbound(self): @inlineCallbacks def test_receive_close_inbound(self): """ - Same as the above test, but when in any state other than - STATE_SELECTED. + Same as the above test, but only for the case when + an active endpoint has not yet been selected. """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -373,6 +390,11 @@ def test_receive_close_inbound(self): @inlineCallbacks def test_receive_close_outbound(self): + """ + Application sends a 'close' message to the user via + the router. Verify that the message is forwarded correctly, + and that the session is terminated. + """ router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) @@ -399,6 +421,9 @@ def test_receive_close_outbound(self): yield self.assert_session_state('123', {}) def test_get_menu_choice(self): + """ + Verify that we parse user input correctly for menu prompts. + """ # good msg = self.router_helper.make_inbound(content='3 ') choice = self.router_worker.get_menu_choice(msg, (1, 4)) @@ -414,6 +439,9 @@ def test_get_menu_choice(self): self.assertEqual(choice, None) def test_create_menu(self): + """ + Create a menu prompt to choose between linked endpoints + """ router_worker = yield self.router_helper.get_router_worker({ 'menu_title': {'content': 'Please select a choice'}, 'entries': [ From f19ab981b99b23d7e1f2afa531adb8fe61fe9cc9 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 14:36:24 +0200 Subject: [PATCH 51/53] Use dict literals instead of dict constructors in the mutiplexer --- go/routers/app_multiplexer/vumi_app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index ab0605c93..8510b6dc5 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -151,7 +151,7 @@ def handle_state_start(self, config, session, msg): endpoints = json.dumps( [entry['endpoint'] for entry in config.entries] ) - returnValue((self.STATE_SELECT, dict(endpoints=endpoints))) + returnValue((self.STATE_SELECT, {'endpoints': endpoints})) @inlineCallbacks def handle_state_select(self, config, session, msg): @@ -173,7 +173,7 @@ def handle_state_select(self, config, session, msg): log.msg("Switched to endpoint '%s' for user %s" % (endpoint, msg['from_addr'])) returnValue((self.STATE_SELECTED, - dict(active_endpoint=endpoint))) + {'active_endpoint': endpoint})) @inlineCallbacks def handle_state_selected(self, config, session, msg): From aa55b10288745183d32ce46b3ffab63002d1d76c Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 14:52:03 +0200 Subject: [PATCH 52/53] De-inline the handlers for the 'None' next_state --- go/routers/app_multiplexer/vumi_app.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 8510b6dc5..66111cb26 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -99,6 +99,11 @@ def target_endpoints(self, config): @inlineCallbacks def handle_inbound(self, config, msg, conn_name): + """ + Main delegation point for handling inbound messages and + managing the state machine. + """ + log.msg("Processing inbound message: %s" % (msg,)) user_id = msg['from_addr'] @@ -121,11 +126,9 @@ def handle_inbound(self, config, msg, conn_name): next_state, updated_session = yield self.handlers[state]( config, session, msg) if next_state is None: - # Halt session immediately - log.msg(("Router configuration change forced session abort " - "for user %s" % user_id)) + # Session terminated (right now, just in the case of a + # administrator-initiated configuration change yield session_manager.clear_session(user_id) - yield self.publish_error_reply(msg, config) else: session['state'] = next_state session.update(updated_session) @@ -162,6 +165,9 @@ def handle_state_select(self, config, session, msg): returnValue((self.STATE_BAD_INPUT, {})) else: if endpoint not in self.target_endpoints(config): + log.msg(("Router configuration change forced session " + "termination for user %s" % msg['from_addr'])) + yield self.publish_error_reply(msg, config) returnValue((None, {})) else: forwarded_msg = self.forwarded_message( @@ -179,6 +185,9 @@ def handle_state_select(self, config, session, msg): def handle_state_selected(self, config, session, msg): active_endpoint = session['active_endpoint'] if active_endpoint not in self.target_endpoints(config): + log.msg(("Router configuration change forced session " + "termination for user %s" % msg['from_addr'])) + yield self.publish_error_reply(msg, config) returnValue((None, {})) else: yield self.publish_inbound(msg, active_endpoint) From 098b6cc67b5a5f11057d0827b6e9b9cc877da4b2 Mon Sep 17 00:00:00 2001 From: Vincent Geddes Date: Tue, 25 Feb 2014 15:29:13 +0200 Subject: [PATCH 53/53] Make the router key a component of session keys --- .../app_multiplexer/tests/test_vumi_app.py | 60 ++++++++++--------- go/routers/app_multiplexer/vumi_app.py | 2 +- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/go/routers/app_multiplexer/tests/test_vumi_app.py b/go/routers/app_multiplexer/tests/test_vumi_app.py index 1ec393033..77ec260ec 100644 --- a/go/routers/app_multiplexer/tests/test_vumi_app.py +++ b/go/routers/app_multiplexer/tests/test_vumi_app.py @@ -30,19 +30,21 @@ def setUp(self): RouterWorkerHelper(ApplicationMultiplexer)) self.router_worker = yield self.router_helper.get_router_worker({}) + def dynamic_config_with_router(self, router): + msg = self.router_helper.make_inbound(None, router=router) + return self.router_worker.get_config(msg) + @inlineCallbacks - def setup_session(self, user_id, data): - session_manager = yield self.router_worker.session_manager( - self.router_worker.CONFIG_CLASS(self.router_worker.config) - ) + def setup_session(self, router, user_id, data): + config = yield self.dynamic_config_with_router(router) + session_manager = yield self.router_worker.session_manager(config) # Initialize session data yield session_manager.save_session(user_id, data) @inlineCallbacks - def assert_session_state(self, user_id, expected_session): - session_manager = yield self.router_worker.session_manager( - self.router_worker.CONFIG_CLASS(self.router_worker.config) - ) + def assert_session(self, router, user_id, expected_session): + config = yield self.dynamic_config_with_router(router) + session_manager = yield self.router_worker.session_manager(config) session = yield session_manager.load_session(user_id) if 'created_at' in session: del session['created_at'] @@ -103,7 +105,7 @@ def test_new_session_display_menu(self): self.assertEqual(msg['content'], 'Please select a choice.\n1) Flappy Bird') # assert that session data updated correctly - yield self.assert_session_state('123', { + yield self.assert_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECT, 'endpoints': '["flappy-bird"]', }) @@ -117,7 +119,7 @@ def test_select_application_endpoint(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECT, 'endpoints': '["flappy-bird"]', }) @@ -138,7 +140,7 @@ def test_select_application_endpoint(self): [msg] = self.router_helper.ri.get_dispatched_outbound() self.assertEqual(msg['content'], 'Flappy Flappy!') - yield self.assert_session_state('123', { + yield self.assert_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -152,7 +154,7 @@ def test_session_with_selected_endpoint(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -176,7 +178,7 @@ def test_session_with_selected_endpoint(self): self.assertEqual(msg['content'], 'Game Over!\n1) Try Again!') - yield self.assert_session_state('123', { + yield self.assert_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -190,7 +192,7 @@ def test_bad_input_for_endpoint_choice(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECT, 'endpoints': '["flappy-bird"]', }) @@ -204,7 +206,7 @@ def test_bad_input_for_endpoint_choice(self): self.assertEqual(msg['content'], 'Bad choice.\n1) Try Again') - yield self.assert_session_state('123', { + yield self.assert_session(router, '123', { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, 'endpoints': '["flappy-bird"]', }) @@ -218,7 +220,7 @@ def test_state_bad_input_for_bad_input_prompt(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, 'endpoints': '["flappy-bird"]', }) @@ -232,7 +234,7 @@ def test_state_bad_input_for_bad_input_prompt(self): self.assertEqual(msg['content'], 'Bad choice.\n1) Try Again') - yield self.assert_session_state('123', { + yield self.assert_session(router, '123', { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, 'endpoints': '["flappy-bird"]', }) @@ -246,7 +248,7 @@ def test_state_good_input_for_bad_input_prompt(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_BAD_INPUT, 'endpoints': '["flappy-bird"]', }) @@ -260,7 +262,7 @@ def test_state_good_input_for_bad_input_prompt(self): self.assertEqual(msg['content'], 'Please select a choice.\n1) Flappy Bird') - yield self.assert_session_state('123', { + yield self.assert_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECT, 'endpoints': '["flappy-bird"]', }) @@ -279,7 +281,7 @@ def test_runtime_exception_in_selected_handler(self): 'target_endpoints', raise_error) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -293,7 +295,7 @@ def test_runtime_exception_in_selected_handler(self): self.assertEqual(msg['content'], 'Oops! Sorry!') - yield self.assert_session_state('123', {}) + yield self.assert_session(router, '123', {}) errors = self.flushLoggedErrors(RuntimeError) self.assertEqual(len(errors), 1) @@ -311,7 +313,7 @@ def test_session_invalidation_in_state_handler(self): config['entries'][0]['endpoint'] = 'mama' router = yield self.router_helper.create_router( started=True, config=config) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -324,7 +326,7 @@ def test_session_invalidation_in_state_handler(self): [msg] = self.router_helper.ri.get_dispatched_outbound() self.assertEqual(msg['content'], 'Oops! Sorry!') - yield self.assert_session_state('123', {}) + yield self.assert_session(router, '123', {}) @inlineCallbacks def test_state_selected_receive_close_inbound(self): @@ -336,7 +338,7 @@ def test_state_selected_receive_close_inbound(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -356,7 +358,7 @@ def test_state_selected_receive_close_inbound(self): self.assertEqual(msgs, []) # assert that session cleared - yield self.assert_session_state('123', {}) + yield self.assert_session(router, '123', {}) @inlineCallbacks def test_receive_close_inbound(self): @@ -367,7 +369,7 @@ def test_receive_close_inbound(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECT, 'endpoints': '["flappy-bird"]' }) @@ -386,7 +388,7 @@ def test_receive_close_inbound(self): self.assertEqual(msgs, []) # assert that session cleared - yield self.assert_session_state('123', {}) + yield self.assert_session(router, '123', {}) @inlineCallbacks def test_receive_close_outbound(self): @@ -398,7 +400,7 @@ def test_receive_close_outbound(self): router = yield self.router_helper.create_router( started=True, config=self.ROUTER_CONFIG) - yield self.setup_session('123', { + yield self.setup_session(router, '123', { 'state': ApplicationMultiplexer.STATE_SELECTED, 'active_endpoint': 'flappy-bird', 'endpoints': '["flappy-bird"]', @@ -418,7 +420,7 @@ def test_receive_close_outbound(self): self.assertEqual(msg['session_event'], 'close') # assert that session cleared - yield self.assert_session_state('123', {}) + yield self.assert_session(router, '123', {}) def test_get_menu_choice(self): """ diff --git a/go/routers/app_multiplexer/vumi_app.py b/go/routers/app_multiplexer/vumi_app.py index 66111cb26..65885ad75 100644 --- a/go/routers/app_multiplexer/vumi_app.py +++ b/go/routers/app_multiplexer/vumi_app.py @@ -87,7 +87,7 @@ def setup_router(self): def session_manager(self, config): return SessionManager.from_redis_config( config.redis_manager, - key_prefix=self.worker_name, + key_prefix=':'.join((self.worker_name, config.router.key)), max_session_length=config.session_expiry )