diff --git a/autopush/db.py b/autopush/db.py index a11987cd..f8df639f 100644 --- a/autopush/db.py +++ b/autopush/db.py @@ -281,17 +281,9 @@ def create_router_table(tablename="router", read_throughput=5, ) table.meta.client.get_waiter('table_exists').wait( TableName=tablename) - try: - table.meta.client.update_time_to_live( - TableName=tablename, - TimeToLiveSpecification={ - 'Enabled': True, - 'AttributeName': 'expiry' - } - ) - except ClientError as ex: # pragma nocover - if ex.response["Error"]["Code"] != "UnknownOperationException": - raise + # Mobile devices (particularly older ones) do not have expiry and + # do not check in regularly. We don't know when they expire other than + # the bridge server failing the UID from their side. return table @@ -776,9 +768,8 @@ def get_uaid(self, uaid): # Incomplete record, drop it. self.drop_user(uaid) raise ItemNotFound("uaid not found") - if item.get("expiry") < _expiry(0): - self.drop_user(uaid) - raise ItemNotFound("uaid expired") + # Mobile users do not check in after initial registration. + # DO NOT EXPIRE THEM. return item except Boto3Error: # pragma: nocover # We trap JSONResponseError because Moto returns text instead of diff --git a/autopush/tests/test_db.py b/autopush/tests/test_db.py index 96da9e08..4dbace48 100644 --- a/autopush/tests/test_db.py +++ b/autopush/tests/test_db.py @@ -437,6 +437,19 @@ def test_drop_old_users(self): results = self.router.drop_old_users(months_ago=0) assert list(results) == [25, 25, 3] + def test_old_mobile_user(self): + # Old mobile users (ones that use a bridge) don't regularly check + # in, or update their expiry record. It's important that we don't + # drop them because reconnecting requires a re-installation. + old_mobile = self._create_minimal_record() + old_mobile["expiry"] = None + m_user = old_mobile['uaid'] + self.router.register_user(old_mobile) + # verify that fetching a user without a expiry still works. + # old mobile users don't have, and may never get, and expiry + user = self.router.get_uaid(m_user) + assert user["uaid"] == m_user + def test_custom_tablename(self): db_name = "router_%s" % uuid.uuid4() assert not table_exists(db_name, boto_resource=self.resource) @@ -524,18 +537,6 @@ def raise_error(*args, **kwargs): router_type="webpush")) assert res == (False, {}) - def test_register_user_expired(self): - from time import time - - router = Router(self.table_conf, SinkMetrics(), resource=self.resource) - expired = self._create_minimal_record() - uaid = expired["uaid"] - expired["expiry"] = int(time()) - 100 - self.router.register_user(expired) - with pytest.raises(ItemNotFound) as ex: - router.get_uaid(uaid) - assert ex.value.message == "uaid expired" - def test_clear_node_provision_failed(self): router = Router(self.table_conf, SinkMetrics(), resource=self.resource) diff --git a/autopush/web/registration.py b/autopush/web/registration.py index 18145c1e..22b0163f 100644 --- a/autopush/web/registration.py +++ b/autopush/web/registration.py @@ -293,13 +293,19 @@ def _register_channel(self, uaid, chid, app_server_key): def _register_user(self, uaid, router_type, router_data): # type: (uuid.UUID, str, JSONDict) -> None - """Save a new user record""" + """Save a new user record + + We set the expiry to 0 here because mobile users never check back, + so the record may be incorrectly expired. (Expiry records older than + 5 years are not automatically deleted.) + """ self.db.router.register_user(dict( uaid=uaid.hex, router_type=router_type, router_data=router_data, connected_at=ms_time(), last_connect=generate_last_connect(), + expiry=0 )) def _write_endpoint(self, endpoint, uaid, chid, router_type, router_data,