From 013db1ca1c2d904d78ecca9adb243eb5462d69e2 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Fri, 31 Mar 2017 14:29:16 -0700 Subject: [PATCH] refactor: various cleanup - params -> router_data, kill unused vapid_info/body - kill bogus/unused timing vars - app_id isn't optional issue #695 --- autopush/db.py | 7 ++++--- autopush/router/fcm.py | 2 +- autopush/router/interface.py | 18 +++++++--------- autopush/tests/test_web_validation.py | 6 ++---- autopush/web/registration.py | 30 ++++++++++++--------------- autopush/websocket.py | 2 +- 6 files changed, 29 insertions(+), 36 deletions(-) diff --git a/autopush/db.py b/autopush/db.py index 027b6261..f31b7d22 100644 --- a/autopush/db.py +++ b/autopush/db.py @@ -70,6 +70,7 @@ # Typing T = TypeVar('T') # noqa +ItemLike = Union[Item, Dict[str, Any]] key_hash = "" TRACK_DB_CALLS = False @@ -105,7 +106,7 @@ def hasher(uaid): def dump_uaid(uaid_data): - # type: (Union[Dict[str, Any], Item]) -> str + # type: (ItemLike) -> str """Return a dict for a uaid. This is utilized instead of repr since some db methods return a @@ -303,7 +304,7 @@ def wrapper(self, *args, **kwargs): def has_connected_this_month(item): - # type: (Dict[str, Any]) -> bool + # type: (ItemLike) -> bool """Whether or not a router item has connected this month""" last_connect = item.get("last_connect") if not last_connect: @@ -678,7 +679,7 @@ def get_uaid(self, uaid): @track_provisioned def register_user(self, data): - # type: (Dict[str, Any]) -> Tuple[bool, Dict[str, Any], Dict[str, Any]] + # type: (ItemLike) -> Tuple[bool, Dict[str, Any], Dict[str, Any]] """Register this user If a record exists with a newer ``connected_at``, then the user will diff --git a/autopush/router/fcm.py b/autopush/router/fcm.py index e47a8cbc..c8b98c55 100644 --- a/autopush/router/fcm.py +++ b/autopush/router/fcm.py @@ -122,7 +122,7 @@ def amend_msg(self, msg, data=None): msg["senderid"] = data.get('creds', {}).get('senderID') return msg - def register(self, uaid, router_data, app_id=None, *args, **kwargs): + def register(self, uaid, router_data, app_id, *args, **kwargs): """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. diff --git a/autopush/router/interface.py b/autopush/router/interface.py index a40b239a..5cfbac8d 100644 --- a/autopush/router/interface.py +++ b/autopush/router/interface.py @@ -1,4 +1,5 @@ """Router interface""" +from typing import Any, Optional # noqa class RouterResponse(object): @@ -26,20 +27,16 @@ def __init__(self, settings, router_conf): the given settings and router conf.""" raise NotImplementedError("__init__ must be implemented") - def register(self, uaid, routing_data, app_id, *args, **kwargs): - """Register the uaid with the connect dict however is preferred and - return a dict that will be stored as routing_data for this user in the - future. + def register(self, uaid, router_data, app_id, *args, **kwargs): + # type: (str, dict, str, *Any, **Any) -> dict + """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. :param uaid: User Agent Identifier - :type uaid: str - :param routing_data: Route specific configuration info - :type routing_data: dict + :param router_data: Route specific configuration info :param app_id: Application identifier from URI - :type app_id: str - :returns: A response object - :rtype: :class:`RouterResponse` :raises: :exc:`RouterException` if data supplied is invalid. @@ -47,6 +44,7 @@ def register(self, uaid, routing_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 :param msg: A dict of the response data to be sent to the client diff --git a/autopush/tests/test_web_validation.py b/autopush/tests/test_web_validation.py index 99b09ee6..a7905608 100644 --- a/autopush/tests/test_web_validation.py +++ b/autopush/tests/test_web_validation.py @@ -98,19 +98,17 @@ def test_validate_invalid_schema(self): eq_(d, {}) def test_call_func_no_error(self): - start_time = time.time() mock_func = Mock() tv, rh = self._make_full() result = tv._validate_request(rh) - tv._call_func(result, mock_func, rh, start_time) + tv._call_func(result, mock_func, rh) mock_func.assert_called() def test_call_func_error(self): - start_time = time.time() mock_func = Mock() tv, rh = self._make_full(schema=InvalidSchema) result = tv._validate_request(rh) - tv._call_func(result, mock_func, rh, start_time) + tv._call_func(result, mock_func, rh) self._mock_errors.assert_called() eq_(len(mock_func.mock_calls), 0) diff --git a/autopush/web/registration.py b/autopush/web/registration.py index 9680738a..e41816a2 100644 --- a/autopush/web/registration.py +++ b/autopush/web/registration.py @@ -1,6 +1,5 @@ import json import re -import time import uuid from boto.dynamodb2.exceptions import ItemNotFound @@ -28,19 +27,17 @@ class RegistrationSchema(Schema): uaid = fields.UUID(allow_none=True) chid = fields.Str(allow_none=True) - body = fields.Dict(allow_none=True) router_type = fields.Str() router_token = fields.Str() - params = fields.Dict() + router_data = fields.Dict() auth = fields.Str(allow_none=True) - vapid_info = fields.Dict(allow_none=True) @pre_load def extract_data(self, req): - params = {} + router_data = {} if req['body']: try: - params = json.loads(req['body']) + router_data = json.loads(req['body']) except ValueError: raise InvalidRequest("Invalid Request body", status_code=401, @@ -48,7 +45,7 @@ def extract_data(self, req): # UAID and CHID may be empty. This can trigger different behaviors # in the handlers, so we can't set default values here. uaid = req['path_kwargs'].get('uaid') - chid = req['path_kwargs'].get('chid', params.get("channelID")) + chid = req['path_kwargs'].get('chid', router_data.get("channelID")) if uaid: try: u_uuid = uuid.UUID(uaid) @@ -74,7 +71,7 @@ def extract_data(self, req): return dict( auth=req.get('headers', {}).get("Authorization"), - params=params, + router_data=router_data, router_type=req['path_kwargs'].get('router_type'), router_token=req['path_kwargs'].get('router_token'), uaid=uaid, @@ -145,15 +142,14 @@ def post(self, *args, **kwargs): """ - self.start_time = time.time() self.add_header("Content-Type", "application/json") - params = self.valid_input['params'] + router_data = self.valid_input['router_data'] # If the client didn't provide a CHID, make one up. # Note, valid_input may explicitly set "chid" to None # THIS VALUE MUST MATCH WHAT'S SPECIFIED IN THE BRIDGE CONNECTIONS. # currently hex formatted. - self.chid = params["channelID"] = (self.valid_input["chid"] or - uuid.uuid4().hex) + self.chid = router_data["channelID"] = (self.valid_input["chid"] or + uuid.uuid4().hex) self.ap_settings.metrics.increment("updates.client.register", tags=self.base_tags()) # If there's a UAID, ensure its valid, otherwise we ensure the hash @@ -167,10 +163,11 @@ def post(self, *args, **kwargs): self.valid_input['uaid'] = uuid.uuid4() new_uaid = True self.uaid = self.valid_input['uaid'] - self.app_server_key = params.get("key") + self.app_server_key = router_data.get("key") if new_uaid: d = Deferred() - d.addCallback(router.register, router_data=params, + d.addCallback(router.register, + router_data=router_data, app_id=self.valid_input.get("router_token"), uri=self.request.uri) d.addCallback(self._save_router_data, @@ -192,13 +189,12 @@ def put(self, *args, **kwargs): Update router type/data for a UAID. """ - self.start_time = time.time() - self.uaid = self.valid_input['uaid'] router = self.valid_input['router'] self.add_header("Content-Type", "application/json") d = Deferred() - d.addCallback(router.register, router_data=self.valid_input['params'], + d.addCallback(router.register, + router_data=self.valid_input['router_data'], app_id=self.valid_input['router_token'], uri=self.request.uri) d.addCallback(self._save_router_data, self.valid_input['router_type']) diff --git a/autopush/websocket.py b/autopush/websocket.py index 49e1bcbf..81d0da3f 100644 --- a/autopush/websocket.py +++ b/autopush/websocket.py @@ -584,7 +584,7 @@ def _lookup_node(self, results): extra="Failed to get UAID for redeliver") def _trap_uaid_not_found(self, fail): - # type: (Failure) -> None + # type: (failure.Failure) -> None """Traps UAID not found error""" fail.trap(ItemNotFound) self.ps.metrics.increment("client.lookup_uaid_failure",