-
Notifications
You must be signed in to change notification settings - Fork 20
Application Multiplexer #844
Changes from 10 commits
777e4b3
6a5379f
8c97624
eadd57d
e920803
8ada227
4ba07ed
4430cf8
f4784c6
cce9f7f
a9b1875
cdb71d1
dfa3e67
c2ab39d
26ca348
370dd5d
e4cf441
5000b2d
a68f96c
3f2656a
e289419
1eb8a7b
581c142
7dfe830
b5cee25
e000b41
0c52ebf
eaa947d
9db74c7
52cceec
0be9e4c
db58f15
97c9bb6
6577351
216f447
eafccbe
b2be461
b894b25
0780889
aeceb82
8331801
7a81223
dd6d269
280404e
22e2192
29c7306
4ce79e6
b5f9dbf
7fe7f3e
45f53b4
e3b47bf
3727f56
a2d2a00
f19ab98
aa55b10
098b6cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,8 @@ | |
{% for edit_form in edit_forms %} | ||
<fieldset> | ||
{{ edit_form|crispy }} | ||
<!-- TODO: style this correctly --> | ||
<strong>{{ edit_form.non_form_errors }}</strong> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be above the form and probably protected with a block that checks that |
||
</fieldset> | ||
{% endfor %} | ||
</form> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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='...'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing calls this and it's untested. Can you remove it please? |
||
""" | ||
Useful for truncating text strings in the following manner: | ||
|
||
'MyFancyTeam' => 'MyFancyT...' | ||
""" | ||
if len(content) <= k: | ||
return content | ||
else: | ||
return content[:k - len(sfx)] + sfx |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
from go.vumitools.router.definition import RouterDefinitionBase | ||
|
||
|
||
class RouterDefinition(RouterDefinitionBase): | ||
router_type = 'application_multiplexer' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will need to be updated when we update the namespace name. |
||
|
||
def configured_outbound_endpoints(self, config): | ||
return list(set(config.get('endpoints', {}).values())) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
from go.base.tests.helpers import GoDjangoTestCase | ||
from go.routers.tests.view_helpers import RouterViewsHelper | ||
from go.vumitools.api import VumiApiCommand | ||
|
||
|
||
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')) | ||
|
||
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', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be updated with less confusing test data. |
||
}, | ||
}) | ||
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.')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the keyword router does it, but I wouldn't test this validation stuff here -- I would test the forms themselves directly. |
||
|
||
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.")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto for this test. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This attribute is unnecessary and should be removed. |
||
|
||
@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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing calls this. Can it be removed? |
||
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') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick -- small |
||
max_length=29 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's set |
||
) | ||
|
||
|
||
class ApplicationMultiplexerForm(forms.Form): | ||
application_title = forms.CharField( | ||
label="Application Title", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would call this |
||
) | ||
target_endpoint = forms.CharField( | ||
label="Endpoint" | ||
) | ||
|
||
|
||
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." | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just drop this validation as discussed on IRC yesterday? There isn't actually a hard requirement that endpoint names are unique (although it'd be usual if they weren't), or that application names be unique (although that'd be unusual too) and the length of the menu that makes sense is really transport and application dependent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just because the Django docs have this, doesn't mean we have to. :) |
||
|
||
@staticmethod | ||
def initial_from_config(data): | ||
return [{'application_title': k, 'target_endpoint': v} | ||
for k, v in sorted(data.items())] | ||
|
||
def to_config(self): | ||
mappings = {} | ||
for form in self.forms: | ||
if (not form.is_valid()) or form.cleaned_data['DELETE']: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Elsewhere we use |
||
continue | ||
application_title = form.cleaned_data['application_title'] | ||
target_endpoint = form.cleaned_data['target_endpoint'] | ||
mappings[application_title] = target_endpoint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're going to need to maintain the ordering of the applications, so we probably want this to be a list of dicts, not a simple mapping. |
||
return mappings | ||
|
||
|
||
ApplicationMultiplexerFormSet = forms.formsets.formset_factory( | ||
ApplicationMultiplexerForm, | ||
can_delete=True, | ||
extra=1, | ||
can_order=True, | ||
formset=BaseApplicationMultiplexerFormSet) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest putting these forms in a |
||
|
||
|
||
class EditApplicationMultiplexerView(EditRouterView): | ||
edit_forms = ( | ||
('menu_title', ApplicationMultiplexerTitleForm), | ||
('endpoints', ApplicationMultiplexerFormSet), | ||
) | ||
|
||
|
||
class RouterViewDefinition(RouterViewDefinitionBase): | ||
edit_view = EditApplicationMultiplexerView |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably best if the namespace matches the module name, i.e.
app_multiplexer
. We don't do that in a few places but those are all places which we want to get rid of eventually.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
namespace
value needs to change to match the module name.