diff --git a/pillar/badge_sync.py b/pillar/badge_sync.py index 7979a947..40db6d7f 100644 --- a/pillar/badge_sync.py +++ b/pillar/badge_sync.py @@ -77,7 +77,7 @@ def find_users_to_sync() -> typing.Iterable[SyncUser]: def fetch_badge_html(session: requests.Session, user: SyncUser, size: str) \ - -> typing.Optional[BadgeHTML]: + -> str: """Fetch a Blender ID badge for this user. :param session: @@ -99,30 +99,24 @@ def fetch_badge_html(session: requests.Session, user: SyncUser, size: str) \ if resp.status_code == 204: my_log.debug('No badges for user %s', user.user_id) - return None + return '' if resp.status_code == 403: my_log.warning('Tried fetching %s for user %s but received a 403: %s', url, user.user_id, resp.text) - return None + return '' if resp.status_code == 400: my_log.warning('Blender ID did not accept our GET request at %s for user %s: %s', url, user.user_id, resp.text) - return None + return '' if resp.status_code == 500: my_log.warning('Blender ID returned an internal server error on %s for user %s, ' 'aborting all badge refreshes: %s', url, user.user_id, resp.text) raise StopRefreshing() if resp.status_code == 404: my_log.warning('Blender ID has no user %s for our user %s', user.bid_user_id, user.user_id) - return None + return '' resp.raise_for_status() - - my_log.debug('Received new badge HTML from %s for user %s', url, user.user_id) - badge_expiry = badge_expiry_config() - return BadgeHTML( - html=resp.text, - expires=utcnow() + badge_expiry, - ) + return resp.text def refresh_all_badges(only_user_id: typing.Optional[bson.ObjectId] = None, *, @@ -164,18 +158,22 @@ def refresh_all_badges(only_user_id: typing.Optional[bson.ObjectId] = None, *, my_log.debug('Skipping user %s', user_info.user_id) continue try: - badge_info = fetch_badge_html(session, user_info, 's') + badge_html = fetch_badge_html(session, user_info, 's') except StopRefreshing: my_log.error('Blender ID has internal problems, stopping badge refreshing at user %s', user_info) break + update = {'badges': { + 'html': badge_html, + 'expires': utcnow() + badge_expiry, + }} num_updates += 1 my_log.info('Updating badges HTML for Blender ID %s, user %s', user_info.bid_user_id, user_info.user_id) if not dry_run: result = users_coll.update_one({'_id': user_info.user_id}, - {'$set': {'badges': badge_info._asdict()}}) + {'$set': update}) if result.matched_count != 1: my_log.warning('Unable to update badges for user %s', user_info.user_id) my_log.info('Updated badges of %d users%s', num_updates, ' (dry-run)' if dry_run else '') diff --git a/tests/test_badge_sync.py b/tests/test_badge_sync.py index d491f429..a43abf04 100644 --- a/tests/test_badge_sync.py +++ b/tests/test_badge_sync.py @@ -50,6 +50,35 @@ class AbstractSyncTest(AbstractPillarTest): self.sync_user1 = badge_sync.SyncUser(self.uid1, 'find-this-token-uid1', '1947') self.sync_user2 = badge_sync.SyncUser(self.uid2, 'find-this-token-uid2', '4488') + def _update_badge_expiry(self, delta_minutes1, delta_minutes2): + """Make badges of userN expire in delta_minutesN minutes.""" + from pillar.api.utils import utcnow, remove_private_keys + now = utcnow() + + # Do the update via Eve so that that flow is covered too. + with self.app.app_context(): + users_coll = self.app.db('users') + db_user1 = users_coll.find_one(self.uid1) + db_user1['badges'] = { + 'html': 'badge for user 1', + 'expires': now + datetime.timedelta(minutes=delta_minutes1) + } + r, _, _, status = self.app.put_internal('users', + remove_private_keys(db_user1), + _id=self.uid1) + self.assertEqual(200, status, r) + + with self.app.app_context(): + db_user2 = users_coll.find_one(self.uid2) + db_user2['badges'] = { + 'html': 'badge for user 2', + 'expires': now + datetime.timedelta(minutes=delta_minutes2) + } + r, _, _, status = self.app.put_internal('users', + remove_private_keys(db_user2), + _id=self.uid2) + self.assertEqual(200, status, r) + class FindUsersToSyncTest(AbstractSyncTest): def test_no_badge_fetched_yet(self): @@ -58,33 +87,6 @@ class FindUsersToSyncTest(AbstractSyncTest): found = set(badge_sync.find_users_to_sync()) self.assertEqual({self.sync_user1, self.sync_user2}, found) - def _update_badge_expiry(self, delta_minutes1, delta_minutes2): - """Make badges of userN expire in delta_minutesN minutes.""" - from pillar.api.utils import utcnow, remove_private_keys - now = utcnow() - - # Do the update via Eve so that that flow is covered too. - users_coll = self.app.db('users') - db_user1 = users_coll.find_one(self.uid1) - db_user1['badges'] = { - 'html': 'badge for user 1', - 'expires': now + datetime.timedelta(minutes=delta_minutes1) - } - r, _, _, status = self.app.put_internal('users', - remove_private_keys(db_user1), - _id=self.uid1) - self.assertEqual(200, status, r) - - db_user2 = users_coll.find_one(self.uid2) - db_user2['badges'] = { - 'html': 'badge for user 2', - 'expires': now + datetime.timedelta(minutes=delta_minutes2) - } - r, _, _, status = self.app.put_internal('users', - remove_private_keys(db_user2), - _id=self.uid2) - self.assertEqual(200, status, r) - def test_badge_fetched_recently(self): from pillar import badge_sync @@ -111,7 +113,6 @@ class FetchHTMLTest(AbstractSyncTest): @httpmock.activate def test_happy(self): from pillar import badge_sync - from pillar.api.utils import utcnow def check_request(request: requests.PreparedRequest): if request.headers['Authorization'] != 'Bearer find-this-token-uid1': @@ -122,12 +123,7 @@ class FetchHTMLTest(AbstractSyncTest): with self.app.app_context(): badge_html = badge_sync.fetch_badge_html(requests.Session(), self.sync_user1, 's') - expected_expire = utcnow() + self.app.config['BLENDER_ID_BADGE_EXPIRY'] - - self.assertEqual('твоја мајка', badge_html.html) - margin = datetime.timedelta(minutes=1) - self.assertLess(expected_expire - margin, badge_html.expires) - self.assertGreater(expected_expire + margin, badge_html.expires) + self.assertEqual('твоја мајка', badge_html) @httpmock.activate def test_internal_server_error(self): @@ -147,7 +143,7 @@ class FetchHTMLTest(AbstractSyncTest): body='', status=204) with self.app.app_context(): badge_html = badge_sync.fetch_badge_html(requests.Session(), self.sync_user1, 's') - self.assertIsNone(badge_html) + self.assertEqual('', badge_html) @httpmock.activate def test_no_such_user(self): @@ -157,7 +153,7 @@ class FetchHTMLTest(AbstractSyncTest): body='Not Found', status=404) with self.app.app_context(): badge_html = badge_sync.fetch_badge_html(requests.Session(), self.sync_user1, 's') - self.assertIsNone(badge_html) + self.assertEqual('', badge_html) @httpmock.activate def test_no_connection_possible(self): @@ -193,3 +189,29 @@ class RefreshAllTest(AbstractSyncTest): # hit the time limit, before doing any call to Blender ID. with self.app.app_context(): badge_sync.refresh_all_badges(timelimit=datetime.timedelta(seconds=-4)) + + @httpmock.activate + def test_remove_badges(self): + from pillar import badge_sync + from pillar.api.utils import utcnow + + # Make sure the user has a badge before getting the 204 No Content from Blender ID. + self._update_badge_expiry(-10, 10) + + httpmock.add('GET', 'http://id.local:8001/api/badges/1947/html/s', + body='', status=204) + + with self.app.app_context(): + badge_sync.refresh_all_badges( + only_user_id=self.uid1, + timelimit=datetime.timedelta(seconds=4)) + expected_expire = utcnow() + self.app.config['BLENDER_ID_BADGE_EXPIRY'] + + # Get directly from MongoDB because JSON doesn't support datetimes. + users_coll = self.app.db('users') + db_user1 = users_coll.find_one(self.uid1) + + self.assertEqual('', db_user1['badges']['html']) + margin = datetime.timedelta(minutes=1) + self.assertLess(expected_expire - margin, db_user1['badges']['expires']) + self.assertGreater(expected_expire + margin, db_user1['badges']['expires'])