bug: fix up AWS conditional binding and assoc. conditions

AWS throws an exception if conditional args are not defined, this
impacted tests by failing, but throwing an exception and causing fun
havoc. register_user now fails if all conditionals are not met
(preserving previous behaviors). Added tests for older records that may
be incomplete.

Some formatting fixes included as well

autopush/db.py

@@ -18,6 +18,7 @@
 from boto.dynamodb2.table import Table
 from boto.dynamodb2.types import NUMBER
 
+from autopush.exceptions import AutopushException
 from autopush.utils import generate_hash
 
 key_hash = ""
@@ -582,6 +583,10 @@ def register_user(self, data):
         conn = self.table.connection
         db_key = self.encode({"uaid": hasher(data["uaid"])})
         del data["uaid"]
+        if "router_type" not in data or "connected_at" not in data:
+            # Not specifying these values will generate an exception in AWS.
+            raise AutopushException("data is missing router_type"
+                                    "or connected_at")
         # Generate our update expression
         expr = "SET " + ", ".join(["%s=:%s" % (x, x) for x in data.keys()])
         expr_values = self.encode({":%s" % k: v for k, v in data.items()})

autopush/endpoint.py

@@ -569,12 +569,8 @@ def _router_completed(self, response, uaid_data, warning=""):
         # TODO: Add some custom wake logic here
 
         # Were we told to update the router data?
-        if response.router_data is not None:
-            if not response.router_data:
-                del uaid_data["router_data"]
-                del uaid_data["router_type"]
-            else:
-                uaid_data["router_data"] = response.router_data
+        if response.router_data:
+            uaid_data["router_data"] = response.router_data
             uaid_data["connected_at"] = int(time.time() * 1000)
             d = deferToThread(self.ap_settings.router.register_user,
                               uaid_data)

autopush/router/gcm.py

@@ -94,9 +94,9 @@ def _route(self, notification, router_data):
         except KeyError:
             raise self._error("Server error, missing bridge credentials " +
                               "for %s" % creds.get("senderID"), 500)
-        except gcmclient.GCMAuthenticationError, e:
+        except gcmclient.GCMAuthenticationError as e:
             raise self._error("Authentication Error: %s" % e, 500)
-        except Exception, e:
+        except Exception as e:
             raise self._error("Unhandled exception in GCM Routing: %s" % e,
                               500)
         self.metrics.increment("updates.client.bridge.gcm.attempted",

autopush/tests/test_db.py

@@ -459,7 +459,8 @@ def raise_error(*args, **kwargs):
         router.table.connection.update_item.side_effect = raise_error
         with self.assertRaises(ProvisionedThroughputExceededException):
             router.register_user(dict(uaid=dummy_uaid, node_id="me",
-                                 connected_at=1234))
+                                      connected_at=1234,
+                                      router_type="simplepush"))
 
     def test_clear_node_provision_failed(self):
         r = get_router_table()
@@ -473,19 +474,24 @@ def raise_error(*args, **kwargs):
         with self.assertRaises(ProvisionedThroughputExceededException):
             router.clear_node(Item(r, dict(uaid=dummy_uaid,
                                            connected_at="1234",
-                                           node_id="asdf")))
+                                           node_id="asdf",
+                                           router_type="simplepush")))
 
     def test_incomplete_uaid(self):
+        # Older records may be incomplete. We can't inject them using normal
+        # methods.
         uaid = str(uuid.uuid4())
         r = get_router_table()
         router = Router(r, SinkMetrics())
+        router.table.get_item = Mock()
+        router.drop_user = Mock()
+        router.table.get_item.return_value = {"uaid": uuid.uuid4().hex}
         try:
             router.register_user(dict(uaid=uaid))
         except:
             pass
         self.assertRaises(ItemNotFound, router.get_uaid, uaid)
-        self.assertRaises(ItemNotFound, router.table.get_item,
-                          consistent=True, uaid=uaid)
+        assert(router.drop_user.called)
 
     def test_save_new(self):
         r = get_router_table()
@@ -495,6 +501,7 @@ def test_save_new(self):
         router.table.connection = Mock()
         router.table.connection.update_item.return_value = {}
         result = router.register_user(dict(uaid="", node_id="me",
+                                           router_type="simplepush",
                                            connected_at=1234))
         eq_(result[0], True)
 
@@ -507,7 +514,8 @@ def raise_condition(*args, **kwargs):
 
         router.table.connection = Mock()
         router.table.connection.update_item.side_effect = raise_condition
-        router_data = dict(uaid=dummy_uaid, node_id="asdf", connected_at=1234)
+        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))
 
@@ -519,7 +527,6 @@ def test_node_clear(self):
         router.register_user(dict(uaid=dummy_uaid, node_id="asdf",
                                   connected_at=1234,
                                   router_type="webpush"))
-
         # Verify
         user = router.get_uaid(dummy_uaid)
         eq_(user["node_id"], "asdf")
@@ -553,8 +560,9 @@ def test_drop_user(self):
         r = get_router_table()
         router = Router(r, SinkMetrics())
         # Register a node user
-        router.table.put_item(data=dict(uaid=uaid, node_id="asdf",
-                                        connected_at=1234))
+        router.register_user(dict(uaid=uaid, node_id="asdf",
+                                  router_type="simplepush",
+                                  connected_at=1234))
         result = router.drop_user(uaid)
         eq_(result, True)
         # Deleting already deleted record should return false.

autopush/tests/test_endpoint.py

@@ -624,7 +624,7 @@ def test_put_router_needs_change(self):
         def handle_finish(result):
             self.assertTrue(result)
             self.endpoint.set_status.assert_called_with(500, None)
-            assert(self.router_mock.register_user.called)
+            assert(not self.router_mock.register_user.called)
         self.finish_deferred.addCallback(handle_finish)
 
         self.endpoint.put(None, dummy_uaid)

autopush/tests/test_websocket.py

@@ -435,6 +435,7 @@ def test_hello_old(self):
             uaid=orig_uaid,
             connected_at=ms_time(),
             current_month=msg_date,
+            router_type="webpush"
         ))
 
         def fake_msg(data):
@@ -1866,6 +1867,19 @@ def after_hello(result):
         self.proto.ps._register.addErrback(lambda x: d.errback(x))
         return d
 
+    def test_incomplete_uaid(self):
+        mm = self.proto.ap_settings.router = Mock()
+        fr = self.proto.force_retry = Mock()
+        uaid = uuid.uuid4().hex
+        mm.get_uaid.return_value = {
+            'uaid': uaid
+        }
+        self.proto.ps.uaid = uaid
+        reply = self.proto._verify_user_record()
+        eq_(reply, None)
+        assert(fr.called)
+        eq_(fr.call_args[0], (mm.drop_user, uaid))
+
 
 class RouterHandlerTestCase(unittest.TestCase):
     def setUp(self):