Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Commit

Permalink
Merge pull request #1162 from mozilla-services/bug/1161
Browse files Browse the repository at this point in the history
bug: Remove expiry checks for routing to prevent dropped mobile UAID
  • Loading branch information
pjenvey authored Mar 20, 2018
2 parents e723de3 + 8a6cd61 commit 6720bbf
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 27 deletions.
19 changes: 5 additions & 14 deletions autopush/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down
25 changes: 13 additions & 12 deletions autopush/tests/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion autopush/web/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 6720bbf

Please sign in to comment.