refactor: cleanup some deferred arg bookkeeping

- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695

autopush/db.py

@@ -679,7 +679,7 @@ def get_uaid(self, uaid):
 
     @track_provisioned
     def register_user(self, data):
-        # type: (ItemLike) -> Tuple[bool, Dict[str, Any], Dict[str, Any]]
+        # type: (ItemLike) -> Tuple[bool, Dict[str, Any]]
         """Register this user
 
         If a record exists with a newer ``connected_at``, then the user will
@@ -728,9 +728,9 @@ def register_user(self, data):
                         # this not work
                         r[key] = value
                 result = r
-            return (True, result, data)
+            return (True, result)
         except ConditionalCheckFailedException:
-            return (False, {}, data)
+            return (False, {})
 
     @track_provisioned
     def drop_user(self, uaid):

autopush/router/apnsrouter.py

@@ -1,5 +1,6 @@
 """APNS Router"""
 import uuid
+from typing import Any, Dict  # noqa
 
 from hyper.http20.exceptions import ConnectionError, HTTP20Error
 from twisted.internet.threads import deferToThread
@@ -67,21 +68,15 @@ def __init__(self, ap_settings, router_conf, load_connections=True):
         self.log.debug("Starting APNS router...")
 
     def register(self, uaid, router_data, app_id, *args, **kwargs):
+        # type: (str, Dict[str, Any], str, *Any, **Any) -> None
         """Register an endpoint for APNS, on the `app_id` release channel.
 
         This will validate that an APNs instance token is in the
         `router_data`,
 
         :param uaid: User Agent Identifier
-        :type uaid: str
         :param router_data: Dict containing router specific configuration info
-        :type router_data: dict
         :param app_id: The release channel identifier for cert info lookup
-        :type app_id: str
-
-        :returns: a modified router_data for the user agent record.
-        :rtype: dict
-
 
         """
         if app_id not in self.apns:
@@ -92,11 +87,9 @@ def register(self, uaid, router_data, app_id, *args, **kwargs):
             raise RouterException("No token registered", status_code=400,
                                   response_body="No token registered")
         router_data["rel_channel"] = app_id
-        return router_data
 
-    def amend_msg(self, msg, router_data=None):
+    def amend_endpoint_response(self, response, router_data):
         """This function is stubbed out for this router"""
-        return msg
 
     def route_notification(self, notification, uaid_data):
         """Start the APNS notification routing, returns a deferred

autopush/router/fcm.py

@@ -1,4 +1,5 @@
 """FCM Router"""
+from typing import Any, Dict  # noqa
 
 import pyfcm
 from requests.exceptions import ConnectionError
@@ -117,12 +118,11 @@ def __init__(self, ap_settings, router_conf):
         self.log.debug("Starting FCM router...")
         self.ap_settings = ap_settings
 
-    def amend_msg(self, msg, data=None):
-        if data is not None:
-            msg["senderid"] = data.get('creds', {}).get('senderID')
-        return msg
+    def amend_endpoint_response(self, response, router_data):
+        response["senderid"] = router_data.get('creds', {}).get('senderID')
 
     def register(self, uaid, router_data, app_id, *args, **kwargs):
+        # type: (str, Dict[str, Any], str, *Any, **Any) -> None
         """Validate that the FCM Instance Token is in the ``router_data``"""
         senderid = app_id
         # "token" is the GCM registration id token generated by the client.
@@ -142,7 +142,6 @@ def register(self, uaid, router_data, app_id, *args, **kwargs):
         # Assign a senderid
         router_data["creds"] = {"senderID": self.senderID,
                                 "auth": self.auth}
-        return router_data
 
     def route_notification(self, notification, uaid_data):
         """Start the FCM notification routing, returns a deferred"""

autopush/router/gcm.py

@@ -1,4 +1,5 @@
 """GCM Router"""
+from typing import Any, Dict  # noqa
 
 import gcmclient
 from requests.exceptions import ConnectionError
@@ -40,12 +41,11 @@ def __init__(self, ap_settings, router_conf):
         self.log.debug("Starting GCM router...")
         self.ap_settings = ap_settings
 
-    def amend_msg(self, msg, data=None):
-        if data is not None:
-            msg["senderid"] = data.get('creds', {}).get('senderID')
-        return msg
+    def amend_endpoint_response(self, response, router_data):
+        response["senderid"] = router_data.get('creds', {}).get('senderID')
 
     def register(self, uaid, router_data, app_id, *args, **kwargs):
+        # type: (str, Dict[str, Any], str, *Any, **Any) -> None
         """Validate that the GCM Instance Token is in the ``router_data``"""
         # "token" is the GCM registration id token generated by the client.
         if "token" not in router_data:
@@ -65,7 +65,6 @@ def register(self, uaid, router_data, app_id, *args, **kwargs):
         # Assign a senderid
         router_data["creds"] = {"senderID": senderid,
                                 "auth": self.senderIDs[senderid]}
-        return router_data
 
     def route_notification(self, notification, uaid_data):
         """Start the GCM notification routing, returns a deferred"""

autopush/router/interface.py

@@ -1,5 +1,9 @@
 """Router interface"""
-from typing import Any, Optional  # noqa
+from typing import (  # noqa
+    Any,
+    Dict,
+    Optional
+)
 
 
 class RouterResponse(object):
@@ -28,10 +32,9 @@ def __init__(self, settings, router_conf):
         raise NotImplementedError("__init__ must be implemented")
 
     def register(self, uaid, router_data, app_id, *args, **kwargs):
-        # type: (str, dict, str, *Any, **Any) -> dict
+        # type: (str, Dict[str, Any], str, *Any, **Any) -> None
         """Register the uaid with the router_data dict however is preferred
-        and return a dict that will be stored as router_data for this
-        user in the future.
+        prior to storing the router_data for this user in the future.
 
         :param uaid: User Agent Identifier
         :param router_data: Route specific configuration info
@@ -43,18 +46,20 @@ def register(self, uaid, router_data, app_id, *args, **kwargs):
         """
         raise NotImplementedError("register must be implemented")
 
-    def amend_msg(self, msg, router_data=None):
-        # type: (dict, Optional[dict]) -> dict
-        """Modify an outbound response message to include router info
+    def amend_endpoint_response(self, response, router_data):
+        # type: (Dict[str, Any], Dict[str, Any]) -> None
+        """Modify an outbound Endpoint registration response message to
+        include router info.
 
-        :param msg: A dict of the response data to be sent to the client
-        :param router_data: a dictionary of router data
-        :returns: A potentially modified dict to return to the client
+        Some routers require additional info to be returned to
+        clients.
 
-        Some routers may require additional info to be returned to clients.
+        :param response: The response data to be sent to the client
+        :param router_data: Route specific configuration info
 
         """
-        raise NotImplementedError("amend_msg must be implemented")
+        raise NotImplementedError(
+            "amend_endpoint_response must be implemented")
 
     def route_notification(self, notification, uaid_data):
         """Route a notification

autopush/router/simple.py

@@ -8,6 +8,7 @@
 import json
 from urllib import urlencode
 from StringIO import StringIO
+from typing import Any  # noqa
 
 import requests
 from boto.dynamodb2.exceptions import (
@@ -47,12 +48,12 @@ def __init__(self, ap_settings, router_conf):
         self.conf = router_conf
         self.waker = None
 
-    def register(self, uaid, *args, **kwargs):
-        """Return no additional routing data"""
-        return {}
+    def register(self, *args, **kwargs):
+        # type: (*Any, **Any) -> None
+        """No additional routing data"""
 
-    def amend_msg(self, msg, router_data=None):
-        return msg
+    def amend_endpoint_response(self, response, router_data):
+        """This function is stubbed out for this router"""
 
     def stored_response(self, notification):
         return RouterResponse(202, "Notification Stored")

autopush/router/webpush.py

@@ -92,6 +92,3 @@ def _save_notification(self, uaid_data, notification):
             self.ap_settings.message_tables[month_table].store_message,
             notification=notification,
         )
-
-    def amend_msg(self, msg, router_data=None):
-        return msg

autopush/tests/test_db.py

@@ -501,7 +501,7 @@ def raise_condition(*args, **kwargs):
         router_data = dict(uaid=dummy_uaid, node_id="asdf", connected_at=1234,
                            router_type="simplepush")
         result = router.register_user(router_data)
-        eq_(result, (False, {}, router_data))
+        eq_(result, (False, {}))
 
     def test_node_clear(self):
         r = get_router_table()

autopush/tests/test_endpoint.py

@@ -23,6 +23,7 @@
     create_rotating_message_table,
     has_connected_this_month,
 )
+from autopush.exceptions import RouterException
 from autopush.settings import AutopushSettings
 from autopush.router.interface import IRouter
 from autopush.tests.test_db import make_webpush_notification
@@ -434,6 +435,28 @@ def restore(*args, **kwargs):
         self.reg.post(self._make_req())
         return self.finish_deferred
 
+    def test_post_bad_router_register(self, *args):
+        frouter = Mock(spec=IRouter)
+        self.reg.ap_settings.routers["simplepush"] = frouter
+        rexc = RouterException("invalid", status_code=402, errno=107)
+        frouter.register = Mock(side_effect=rexc)
+
+        self.reg.request.body = json.dumps(dict(
+            type="simplepush",
+            channelID=str(dummy_chid),
+            data={},
+        ))
+        self.reg.request.uri = "/v1/xxx/yyy/register"
+        self.reg.request.headers["Authorization"] = self.auth
+
+        def handle_finish(value):
+            self._check_error(rexc.status_code, rexc.errno, "")
+
+        self.finish_deferred.addBoth(handle_finish)
+        self.reg.post(self._make_req("simplepush", "",
+                                     body=self.reg.request.body))
+        return self.finish_deferred
+
     def test_post_existing_uaid(self, *args):
         self.reg.request.body = json.dumps(dict(
             channelID=str(dummy_chid),
@@ -703,8 +726,8 @@ def handle_finish(value):
             self.reg.write.assert_called_with({})
             frouter.register.assert_called_with(
                 dummy_uaid.hex,
-                router_data=data,
-                app_id='',
+                data,
+                '',
                 uri=self.reg.request.uri
             )
 
@@ -757,6 +780,19 @@ def restore(*args, **kwargs):
         self.reg.put(self._make_req(uaid=dummy_uaid.hex))
         return self.finish_deferred
 
+    def test_put_bad_router_register(self):
+        frouter = self.reg.ap_settings.routers["test"]
+        rexc = RouterException("invalid", status_code=402, errno=107)
+        frouter.register = Mock(side_effect=rexc)
+
+        def handle_finish(value):
+            self._check_error(rexc.status_code, rexc.errno, "")
+
+        self.finish_deferred.addCallback(handle_finish)
+        self.reg.request.headers["Authorization"] = self.auth
+        self.reg.put(self._make_req(router_type='test', uaid=dummy_uaid.hex))
+        return self.finish_deferred
+
     def test_delete_bad_chid_value(self):
         notif = make_webpush_notification(dummy_uaid.hex, str(dummy_chid))
         messages = self.reg.ap_settings.message
@@ -865,6 +901,8 @@ def test_get(self):
         chids = [str(dummy_chid), str(dummy_uaid)]
 
         def handle_finish(value):
+            self.settings.message.all_channels.assert_called_with(
+                str(dummy_uaid))
             call_args = json.loads(
                 self.reg.write.call_args[0][0]
             )