From c05ecbdb8a3849f803fd3bfc9fbc5166e0a216e6 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Mon, 1 Aug 2016 17:04:07 -0700 Subject: [PATCH] fix: Normalize endpoint and web _router_completed calls and tests closes #549 --- autopush/endpoint.py | 18 +++++++++++++++--- autopush/tests/test_endpoint.py | 12 ++++++++---- autopush/tests/test_web_webpush.py | 3 ++- autopush/web/webpush.py | 14 ++++++++++++-- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/autopush/endpoint.py b/autopush/endpoint.py index f3e6a412..d3297bfc 100644 --- a/autopush/endpoint.py +++ b/autopush/endpoint.py @@ -584,11 +584,22 @@ def _route_notification(self, version, result, data, ttl=None): def _router_completed(self, response, uaid_data, warning=""): """Called after router has completed successfully""" # TODO: Add some custom wake logic here - # Were we told to update the router data? - if response.router_data: - uaid_data["router_data"] = response.router_data + if response.router_data is not None: + if not response.router_data: + # An empty router_data object indicates that the record should + # be deleted. There is no longer valid route information for + # this record. + self.ap_settings.router.drop_user(self.uaid) + return self._router_response(response) + else: + # The router_data needs to be updated to include any changes + # requested by the bridge system. + uaid_data["router_data"] = response.router_data + # set the AWS mandatory data. uaid_data["connected_at"] = ms_time() + uaid_data["router_type"] = uaid_data.get("router_type", + self.router_key) d = deferToThread(self.ap_settings.router.register_user, uaid_data) response.router_data = None @@ -597,6 +608,7 @@ def _router_completed(self, response, uaid_data, warning=""): warning)) return d else: + # No changes are requested by the bridge system, proceed as normal if response.status_code == 200 or response.logged_status == 200: self.log.info(format="Successful delivery", **self._client_info) diff --git a/autopush/tests/test_endpoint.py b/autopush/tests/test_endpoint.py index 6317f5d4..6df1a0a1 100644 --- a/autopush/tests/test_endpoint.py +++ b/autopush/tests/test_endpoint.py @@ -618,13 +618,16 @@ def test_put_router_needs_change(self): ) self.sp_router_mock.route_notification.return_value = RouterResponse( status_code=500, - router_data={}, + router_data=dict(token="new_connect"), ) def handle_finish(result): self.assertTrue(result) self.endpoint.set_status.assert_called_with(500, None) - ok_(not self.router_mock.register_user.called) + ru = self.router_mock.register_user + ok_(ru.called) + eq_('simplepush', ru.call_args[0][0].get('router_type')) + self.finish_deferred.addCallback(handle_finish) self.endpoint.put(None, dummy_uaid) @@ -638,13 +641,14 @@ def test_put_router_needs_update(self): ) self.sp_router_mock.route_notification.return_value = RouterResponse( status_code=503, - router_data=dict(token="new_connect"), + router_data=dict(), ) def handle_finish(result): self.assertTrue(result) self.endpoint.set_status.assert_called_with(503, None) - assert(self.router_mock.register_user.called) + self.router_mock.drop_user.assert_called() + self.finish_deferred.addCallback(handle_finish) self.endpoint.put(None, dummy_uaid) diff --git a/autopush/tests/test_web_webpush.py b/autopush/tests/test_web_webpush.py index b71288d8..ff253942 100644 --- a/autopush/tests/test_web_webpush.py +++ b/autopush/tests/test_web_webpush.py @@ -108,7 +108,8 @@ def test_router_returns_data_without_detail(self): def handle_finish(result): eq_(result, True) self.wp.set_status.assert_called_with(503) - assert(self.router_mock.register_user.called) + assert(self.router_mock.drop_user.called) + self.finish_deferred.addCallback(handle_finish) self.wp.post("v1", dummy_token) diff --git a/autopush/web/webpush.py b/autopush/web/webpush.py index f95a8cbf..b1922d77 100644 --- a/autopush/web/webpush.py +++ b/autopush/web/webpush.py @@ -56,13 +56,22 @@ def post(self, api_ver="v1", token=None): def _router_completed(self, response, uaid_data, warning=""): """Called after router has completed successfully""" # Were we told to update the router data? - # GCM/APNS bridges can result in data updates if response.router_data is not None: if not response.router_data: - del uaid_data["router_data"] + # An empty router_data object indicates that the record should + # be deleted. There is no longer valid route information for + # this record. + self.ap_settings.router.drop_user(self.uaid) + return self._router_response(response) else: + # The router data needs to be updated to include any changes + # requested by the bridge system uaid_data["router_data"] = response.router_data + # set the AWS mandatory data uaid_data["connected_at"] = ms_time() + rt = self.valid_input.get( + 'subscription', {}).get('user_data', {}).get('router_type') + uaid_data["router_type"] = uaid_data.get("router_type", rt) d = deferToThread(self.ap_settings.router.register_user, uaid_data) response.router_data = None @@ -71,6 +80,7 @@ def _router_completed(self, response, uaid_data, warning=""): warning)) return d else: + # No changes are requested by the bridge system, proceed as normal if response.status_code == 200 or response.logged_status == 200: self.log.info(format="Successful delivery", **self._client_info)