Move unroutable message reply configs to tagpools.

go/vumitools/routing.py

@@ -3,7 +3,7 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 
 from vumi.dispatchers.endpoint_dispatchers import RoutingTableDispatcher
-from vumi.config import ConfigDict, ConfigText, ConfigBool
+from vumi.config import ConfigDict, ConfigText
 from vumi.message import TransportEvent
 from vumi import log
 
@@ -160,17 +160,10 @@ class AccountRoutingTableDispatcherConfig(RoutingTableDispatcher.CONFIG_CLASS,
         static=True, required=False)
     user_account_key = ConfigText(
         "Key of the user account the message is from.")
-    send_unroutable_inbound_session_replies = ConfigBool(
-        "If true, send a closing reply to unroutable inbound messages that"
-        " are part of a session. If the associated tag pool has an"
-        " `unroutable_reply` metadata key, it's value is sent as the reply."
-        " Otherwise the value of `default_unroutable_inbound_session_reply`"
-        " is sent instead.",
-        default=False, static=True, required=False)
-    default_unroutable_inbound_session_reply = ConfigText(
+    default_unroutable_inbound_reply = ConfigText(
         "Default text to send in response to unroutable inbound messages"
-        " that are part of a session if"
-        " `send_unroutable_inbound_session_replies` is set to true.",
+        " if the tagpool specifies `reply_to_unroutable_inbound` but not"
+        " `unroutable_inbound_reply`.",
         default="Vumi Go could not route your message. Please try again soon.",
         static=True, required=False)
 
@@ -564,31 +557,33 @@ def publish_outbound_from_billing(self, config, msg):
         yield self.publish_outbound(msg, dst_connector_name, dst_endpoint)
 
     @inlineCallbacks
-    def publish_unroutable_message_reply(self, msg, connector_name,
-                                         default_response):
+    def handle_unroutable_inbound_message(self, f, msg, connector_name):
+        """Send a reply to the unroutable `msg` if the tagpool ask for one.
+
+        If we can't find a the tagpool or the tagpool isn't configured for
+        replies to unroutable messages, the original exception is reraised.
+        """
         msg_mdh = self.get_metadata_helper(msg)
         if msg_mdh.tag is None:
             # defend against messages without tags (these should not occur
             # but this is an error path)
-            response = default_response
-        else:
-            tagpool_metadata = yield self.vumi_api.tpm.get_metadata(
-                msg_mdh.tag[0])
-            response = tagpool_metadata.get(
-                'unroutable_reply', default_response)
+            f.raiseException()
+
+        tagpool_metadata = yield self.vumi_api.tpm.get_metadata(msg_mdh.tag[0])
+        if not tagpool_metadata.get('reply_to_unroutable_inbound'):
+            f.raiseException()
+
+        config = self.get_static_config()
+        default_response = config.default_unroutable_inbound_reply
+        response = tagpool_metadata.get(
+            'unroutable_inbound_reply', default_response)
         reply = msg.reply(response, continue_session=False)
         self.publish_outbound(
             reply, connector_name, msg.get_routing_endpoint())
 
     def errback_inbound(self, f, msg, connector_name):
-        if (f.check(UnroutableMessageError)
-                and msg['session_event'] is not None):
-            config = self.get_static_config()
-            if config.send_unroutable_inbound_session_replies:
-                return self.publish_unroutable_message_reply(
-                    msg, connector_name,
-                    config.default_unroutable_inbound_session_reply)
-        return f
+        f.trap(UnroutableMessageError)  # Reraise any other exception types.
+        return self.handle_unroutable_inbound_message(f, msg, connector_name)
 
     @inlineCallbacks
     def process_inbound(self, config, msg, connector_name):

go/vumitools/tests/test_routing.py

@@ -252,10 +252,7 @@ def setup_routing_table_dispatcher_test(self):
         self.user_account_key = user_account.key
         self.user_api = self.vumi_api.get_user_api(self.user_account_key)
         tag1, tag2, tag3 = yield self.setup_tagpool(
-            u"pool1", [u"1234", u"5678", u"9012"],
-            metadata={
-                "unroutable_reply": "Message to pool1 could not be routed",
-            })
+            u"pool1", [u"1234", u"5678", u"9012"])
         yield self.user_api.acquire_specific_tag(tag1)
         yield self.user_api.acquire_specific_tag(tag2)
         yield self.user_api.acquire_specific_tag(tag3)
@@ -832,7 +829,7 @@ def get_dispatcher(self, **config_extras):
                 "app1", "router_ri",
             ],
             "send_unroutable_inbound_session_replies": True,
-            "default_unroutable_inbound_session_reply": "Eep!",
+            "default_unroutable_inbound_reply": "Eep!",
         }
         config.update(config_extras)
         dispatcher = yield super(
@@ -859,32 +856,104 @@ def assert_unroutable_reply(self, connector_name, msg,
         self.assert_reply_matches(reply, msg, reply_content, **md)
 
     @inlineCallbacks
-    def test_inbound_message_from_transport_to_unowned_tag(self):
+    def test_unroutable_inbound_from_transport_to_unknown_pool(self):
         yield self.get_dispatcher()
         msg = self.with_md(
             self.msg_helper.make_inbound("foo", session_event='new'),
-            tag=("pool1", "badtag"))
+            tag=("badpool", "tag1"))
         yield self.dispatch_inbound(msg, 'sphex')
-        self.assert_unroutable_reply(
-            'sphex', msg, "Message to pool1 could not be routed",
-            tag=("pool1", "badtag"))
+        self.assert_rkeys_used('sphex.inbound')
+        [failure] = self.flushLoggedErrors()
+        self.assertEqual(
+            'Message received for unowned tag.', failure.value.args[0])
 
     @inlineCallbacks
-    def test_inbound_message_from_transport_to_unknown_pool(self):
+    def test_unroutable_inbound_from_transport_no_config(self):
         yield self.get_dispatcher()
         msg = self.with_md(
             self.msg_helper.make_inbound("foo", session_event='new'),
-            tag=("badpool", "tag1"))
+            tag=("pool1", "tag1"))
+        yield self.dispatch_inbound(msg, 'sphex')
+        self.assert_rkeys_used('sphex.inbound')
+        [failure] = self.flushLoggedErrors()
+        self.assertEqual(
+            'Message received for unowned tag.', failure.value.args[0])
+
+    @inlineCallbacks
+    def test_unroutable_inbound_from_transport_no_reply(self):
+        yield self.vumi_api.tpm.set_metadata(u"pool1", {
+            "reply_to_unroutable_inbound": False,
+        })
+        yield self.get_dispatcher()
+        msg = self.with_md(
+            self.msg_helper.make_inbound("foo", session_event='new'),
+            tag=("pool1", "tag1"))
         yield self.dispatch_inbound(msg, 'sphex')
+        self.assert_rkeys_used('sphex.inbound')
+        [failure] = self.flushLoggedErrors()
+        self.assertEqual(
+            'Message received for unowned tag.', failure.value.args[0])
+
+    @inlineCallbacks
+    def test_unroutable_inbound_from_transport_default_reply(self):
+        yield self.vumi_api.tpm.set_metadata(u"pool1", {
+            "reply_to_unroutable_inbound": True,
+        })
+        yield self.get_dispatcher()
+        msg = self.with_md(
+            self.msg_helper.make_inbound("foo", session_event='new'),
+            tag=("pool1", "tag1"))
+        yield self.dispatch_inbound(msg, 'router_ro')
         self.assert_unroutable_reply(
-            'sphex', msg, "Eep!", tag=("badpool", "tag1"))
+            'router_ro', msg, "Eep!", tag=("pool1", "tag1"))
 
     @inlineCallbacks
-    def test_inbound_message_from_router_to_unroutable(self):
+    def test_unroutable_inbound_from_transport_default_reply_no_session(self):
+        yield self.vumi_api.tpm.set_metadata(u"pool1", {
+            "reply_to_unroutable_inbound": True,
+        })
+        yield self.get_dispatcher()
+        msg = self.with_md(
+            self.msg_helper.make_inbound("foo"), tag=("pool1", "tag1"))
+        yield self.dispatch_inbound(msg, 'router_ro')
+        self.assert_unroutable_reply(
+            'router_ro', msg, "Eep!", tag=("pool1", "tag1"))
+
+    @inlineCallbacks
+    def test_unroutable_inbound_from_transport_custom_reply(self):
+        yield self.vumi_api.tpm.set_metadata(u"pool1", {
+            "reply_to_unroutable_inbound": True,
+            "unroutable_inbound_reply": "Custom Eep!",
+        })
+        yield self.get_dispatcher()
+        msg = self.with_md(
+            self.msg_helper.make_inbound("foo", session_event='new'),
+            tag=("pool1", "tag1"))
+        yield self.dispatch_inbound(msg, 'router_ro')
+        self.assert_unroutable_reply(
+            'router_ro', msg, "Custom Eep!", tag=("pool1", "tag1"))
+
+    @inlineCallbacks
+    def test_unroutable_inbound_from_router_no_config(self):
         yield self.get_dispatcher()
         msg = self.with_md(
             self.msg_helper.make_inbound("foo", session_event='new'),
             router=('router', 'badrouter'))
         yield self.dispatch_inbound(msg, 'router_ro')
+        [failure] = self.flushLoggedErrors()
+        self.assertEqual(
+            "No target found for inbound message from 'router_ro'",
+            failure.value.args[0])
+
+    @inlineCallbacks
+    def test_unroutable_inbound_from_router_default_reply(self):
+        yield self.vumi_api.tpm.set_metadata(u"pool1", {
+            "reply_to_unroutable_inbound": True,
+        })
+        yield self.get_dispatcher()
+        msg = self.with_md(
+            self.msg_helper.make_inbound("foo", session_event='new'),
+            tag=("pool1", "tag1"), router=('router', 'badrouter'))
+        yield self.dispatch_inbound(msg, 'router_ro')
         self.assert_unroutable_reply(
             'router_ro', msg, "Eep!", router=("router", "badrouter"))