Badge sync: also support removal of all badges

Removal is stored as '' for the HTML. This way there is still the expiry
date, which means we won't repeatedly check for changes.

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()
-
+    return resp.text
-    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,
-    )
 
 
 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 '')

tests/test_badge_sync.py

@@ -50,20 +50,13 @@ 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')
 
-
-class FindUsersToSyncTest(AbstractSyncTest):
-    def test_no_badge_fetched_yet(self):
-        from pillar import badge_sync
-        with self.app.app_context():
-            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.
+        with self.app.app_context():
             users_coll = self.app.db('users')
             db_user1 = users_coll.find_one(self.uid1)
             db_user1['badges'] = {
@@ -75,6 +68,7 @@ class FindUsersToSyncTest(AbstractSyncTest):
                                                     _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',
@@ -85,6 +79,14 @@ class FindUsersToSyncTest(AbstractSyncTest):
                                                     _id=self.uid2)
         self.assertEqual(200, status, r)
 
+
+class FindUsersToSyncTest(AbstractSyncTest):
+    def test_no_badge_fetched_yet(self):
+        from pillar import badge_sync
+        with self.app.app_context():
+            found = set(badge_sync.find_users_to_sync())
+        self.assertEqual({self.sync_user1, self.sync_user2}, found)
+
     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)
-
-        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)
 
     @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'])