hobo/context_processors.py

@@ -58,7 +58,7 @@ class RemoteTemplate:
         if self.source != '404' and item:
             # page_cache is a dict redirect_url -> page content, get the best
             # match.
-            page_cache, expiry_time = item
+            page_cache = item['page_cache']
             selected_cache_page = None
             for page_redirect_url in sorted(page_cache.keys(), key=len):
                 if selected_cache_page is None:
@@ -68,19 +68,21 @@ class RemoteTemplate:
                     continue
                 if len(page_redirect_url) > len(selected_cache_page):
                     selected_cache_page = page_redirect_url
-            item = (page_cache[selected_cache_page], expiry_time)
+            return {**item, 'template_body': page_cache[selected_cache_page], 'page_cache': None, 'count': 0}
         else:
-            item = cache.get(self.cache_key)
-        return item
+            return cache.get(self.cache_key)
 
     def get_page_cache_key(self):
-        return self.PAGE_CACHE_KEY + '-' + self.language_code
+        # added v2 for change of cache item structure
+        return self.PAGE_CACHE_KEY + '-' + self.language_code + 'v2'
 
     @property
     def cache_key(self):
+        # added v2 for change of cache item structure
         return hashlib.md5(
             urllib.parse.urlunparse(urllib.parse.urlparse(self.source)[:3] + ('', '', '')).encode('ascii')
             + self.language_code.encode('ascii')
+            + b'v2'
         ).hexdigest()
 
     def get_template(self):
@@ -100,38 +102,50 @@ class RemoteTemplate:
         else:
             self.theme_skeleton_url = settings.THEME_SKELETON_URL
         if item is None:
-            template_body = self.update_content(in_thread=False)
-            if template_body is None:
-                raise Exception('Failed to retrieve theme')
+            template_body = self.update_content()
         else:
-            template_body, expiry_time = item
-            if expiry_time < datetime.datetime.now():
+            if item['expiry_time'] < datetime.datetime.now():
                 # stale value, put it back into the cache for other consumers and
                 # update the content in a different thread
-                self.cache(template_body)
-                threading.Thread(target=self.update_content).start()
+                self.cache(item['template_body'], old_cache=item)
+                threading.Thread(target=lambda: self.update_content(old_cache=item)).start()
+            template_body = item['template_body']
         return Template(template_body)
 
-    def update_content(self, in_thread=True):
-        r = requests.get(
-            self.theme_skeleton_url,
-            params={'source': self.source},
-            headers={'Accept-Language': self.language_code},
-            timeout=10,
-        )
-        if r.status_code != 200:
-            logger.error('failed to retrieve theme (status code: %s)', r.status_code)
+    def update_content(self, old_cache=None):
+        last_update = old_cache and old_cache['last_update']
+        count = old_cache['count'] if old_cache else 0
+        try:
+            r = requests.get(
+                self.theme_skeleton_url,
+                params={'source': self.source},
+                headers={'Accept-Language': self.language_code},
+                timeout=10,
+            )
+            r.raise_for_status()
+        except requests.RequestException as e:
+            if old_cache is None:
+                raise Exception(f'Failed to retrieve theme: {e}')
+            if datetime.datetime.now() - last_update < datetime.timedelta(hours=1) or count < 3:
+                log_function = logger.warning
+            else:
+                log_function = logger.error
+            log_function('failed to retrieve theme since %s: %s', last_update, e)
             return None
-        self.cache(r.text)
-        if r.headers.get('X-Combo-Skeleton-Pages'):
+
+        template_body = r.text
+
+        self.cache(template_body)
+        if x_combo_skeleton_pages := r.headers.get('X-Combo-Skeleton-Pages'):
             # X-Combo-Skeleton-Pages header is a dict (page_id -> redirect_url),
             # it is use to create page cache.
-            self.combo_skeleton_pages = json.loads(r.headers.get('X-Combo-Skeleton-Pages'))
-            if in_thread:
+            self.combo_skeleton_pages = json.loads(x_combo_skeleton_pages)
+            if last_update is not None:
+                # cache is filled, so we are in a thread
                 self.update_all_pages_cache()
             else:
                 threading.Thread(target=self.update_all_pages_cache).start()
-        return r.text
+        return template_body
 
     def update_all_pages_cache(self):
         for lang_code, _ in settings.LANGUAGES:
@@ -143,25 +157,44 @@ class RemoteTemplate:
 
             page_cache = {}
             for page_redirect_url in self.combo_skeleton_pages.values():
-                r = requests.get(
-                    self.theme_skeleton_url,
-                    params={'source': page_redirect_url},
-                    headers={'Accept-Language': lang_code},
-                    timeout=10,
-                )
-                if r.status_code != 200:
+                try:
+                    r = requests.get(
+                        self.theme_skeleton_url,
+                        params={'source': page_redirect_url},
+                        headers={'Accept-Language': lang_code},
+                        timeout=10,
+                    )
+                    r.raise_for_status()
+                except requests.RequestException:
                     # abort
                     return
                 page_cache[page_redirect_url] = r.text
 
             expiry_time = datetime.datetime.now() + datetime.timedelta(seconds=CACHE_REFRESH_TIMEOUT)
             cache.set(
-                self.get_page_cache_key(), (page_cache, expiry_time), 2592000
+                self.get_page_cache_key(),
+                {
+                    'page_cache': page_cache,
+                    'expiry_time': expiry_time,
+                    'last_update': datetime.datetime.now(),
+                },
+                2592000,
             )  # bypass cache level expiration time
 
-    def cache(self, template_body):
+    def cache(self, template_body, old_cache=None):
+        last_update = old_cache['last_update'] if old_cache else datetime.datetime.now()
         expiry_time = datetime.datetime.now() + datetime.timedelta(seconds=CACHE_REFRESH_TIMEOUT)
-        cache.set(self.cache_key, (template_body, expiry_time), 2592000)  # bypass cache level expiration time
+        count = old_cache['count'] + 1 if old_cache else 0
+        cache.set(
+            self.cache_key,
+            {
+                'template_body': template_body,
+                'expiry_time': expiry_time,
+                'last_update': last_update,
+                'count': count,
+            },
+            2592000,
+        )  # bypass cache level expiration time
 
 
 def theme_base(request):

tests/test_context_processors.py

@@ -1,123 +1,209 @@
 import json
+import re
+import threading
 import time
 from unittest import mock
 
+import pytest
+import requests
+import responses
 from django.core.cache import cache
 from django.test import override_settings
 from django.test.client import RequestFactory
 from django.utils import translation
-from httmock import HTTMock, urlmatch
 
 from hobo.context_processors import theme_base, user_urls
 
+TEMPLATE = 'Feeling lucky, punk?'
+TEMPLATE_PAGE2 = 'Response for page 2'
 
-def test_theme_base(settings, rf):
-    settings.THEME_SKELETON_URL = 'http://combo.example.com/_skeleton_/'
-    seen_urls = []
-    TEMPLATE = 'Feeling lucky, punk?'
-    TEMPLATE_PAGE2 = 'Response for page 2'
 
-    @urlmatch(netloc=r'combo.example.com$')
-    def combo_mock(url, request):
-        seen_urls.append(url.geturl())
-        status_code = 200
-        content = TEMPLATE
-        if 'page2' in url.query:
-            content = TEMPLATE_PAGE2
-        elif 'page1' in url.query:
-            status_code = 500
-            content = 'No template sorry'
-        return {
-            'status_code': status_code,
-            'content': content,
-            'headers': {
-                'X-Combo-Skeleton-Pages': json.dumps(
-                    {'1': 'http://testserver/foo', '2': 'http://testserver/page1/page2/'}
-                )
-            },
+def wait_other_threads():
+    time.sleep(0.1)
+    for thread in threading.enumerate()[1:]:
+        thread.join(timeout=10)
+    assert len(threading.enumerate()) == 1
+
+
+@pytest.fixture
+def portal_mock():
+    def callback(request):
+        headers = {
+            'X-Combo-Skeleton-Pages': json.dumps(
+                {'1': 'http://testserver/foo', '2': 'http://testserver/page1/page2/'}
+            )
         }
+        if 'page2' in request.url:
+            return 200, headers, TEMPLATE_PAGE2
+        else:
+            return 200, headers, TEMPLATE
 
-    cache.clear()
-
-    def check(context, value):
-        assert context['theme_base']().source == value
-
-    with HTTMock(combo_mock), override_settings(INSTALLED_APPS=[]):
-        context = theme_base(rf.get('/'))
-        check(context, TEMPLATE)
-        assert seen_urls[0] == 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2F'
-        for dummy in range(10):
-            # wait for the other requests, made from a thread, to happen
-            time.sleep(0.1)
-            if len(seen_urls) == 4:
-                break
-        assert len(seen_urls) == 4
-        # requested page + root + pages from X-Combo-Skeleton-Pages header
-        seen_urls = []
-        context = theme_base(rf.get('/'))
-        check(context, TEMPLATE)
-        assert len(seen_urls) == 0
-        seen_urls = []
-        context = theme_base(rf.get('/page1/page2/'))
-        check(context, TEMPLATE_PAGE2)
-        assert len(seen_urls) == 0
-        seen_urls = []
-        context = theme_base(rf.get('/page1/'))
-        check(context, TEMPLATE)
-        assert len(seen_urls) == 0
-        context = theme_base(rf.get('/page1/page2/'))
-        check(context, TEMPLATE_PAGE2)
-        assert len(seen_urls) == 0
-
-
-def test_theme_base_language(settings, rf):
-    settings.THEME_SKELETON_URL = 'http://combo.example.com/_skeleton_/'
-    seen_urls = []
-
-    @urlmatch(netloc=r'combo.example.com$')
-    def combo_mock(url, request):
-        language = request.headers['Accept-Language']
-        seen_urls.append((language, url.geturl()))
-        status_code = 200
-        assert language in ('en', 'fr'), 'invalid language'
-        if language == 'en':
-            content = 'Skeleton for English'
-        elif language == 'fr':
-            content = 'Skeleton for French'
-        return {
-            'status_code': status_code,
-            'content': content,
-            'headers': {'X-Combo-Skeleton-Pages': json.dumps({'1': 'http://testserver/foo'})},
-        }
-
-    cache.clear()
-
-    with HTTMock(combo_mock), override_settings(
-        INSTALLED_APPS=[], LANGUAGES=[('en', 'English'), ('fr', 'French')]
+    with responses.RequestsMock() as rsps, override_settings(
+        INSTALLED_APPS=[], THEME_SKELETON_URL='http://combo.example.com/_skeleton_/'
     ):
-        context = theme_base(rf.get('/'))
-        assert context['theme_base']().source == 'Skeleton for English'
-        for dummy in range(10):
-            # wait for the other requests, made from a thread, to happen
-            time.sleep(0.1)
-            if len(seen_urls) == 5:
-                break
-        assert len(seen_urls) == 5
-        assert ('en', 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2F') in seen_urls
-        assert ('fr', 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2F') in seen_urls
-        assert (
-            'en',
-            'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2Ffoo',
-        ) in seen_urls
-        assert (
-            'fr',
-            'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2Ffoo',
-        ) in seen_urls
+        rsps.add_callback(responses.GET, 'http://combo.example.com/_skeleton_/', callback=callback)
+        yield rsps
 
-        assert theme_base(rf.get('/'))['theme_base']().source == 'Skeleton for English'
 
-        with translation.override('fr'):
-            assert theme_base(rf.get('/'))['theme_base']().source == 'Skeleton for French'
+@pytest.fixture
+def theme_base_source(rf):
+    def theme_base_source(path):
+        context = theme_base(rf.get(path))
+        return context['theme_base']().source
+
+    return theme_base_source
+
+
+def test_theme_base(portal_mock, theme_base_source):
+    cache.clear()
+
+    calls = portal_mock.calls
+
+    assert theme_base_source('/') == TEMPLATE
+    assert calls[0].request.url == 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2F'
+    wait_other_threads()
+
+    # requested page + root + pages from X-Combo-Skeleton-Pages header
+    assert len(calls) == 4
+
+    calls.reset()
+
+    assert theme_base_source('/') == TEMPLATE
+    assert not calls
+
+    assert theme_base_source('/page1/page2/') == TEMPLATE_PAGE2
+    assert not calls
+
+    assert theme_base_source('/page1/') == TEMPLATE
+    assert not calls
+
+    assert theme_base_source('/page1/page2/') == TEMPLATE_PAGE2
+    assert not calls
+
+
+@pytest.fixture
+def portal_mock_error():
+    def callback(request):
+        headers = {
+            'X-Combo-Skeleton-Pages': json.dumps(
+                {'1': 'http://testserver/foo', '2': 'http://testserver/page1/page2/'}
+            )
+        }
+        if 'page2' in request.url:
+            raise requests.RequestException('page2 boom!')
+        return 200, headers, TEMPLATE
+
+    with responses.RequestsMock() as rsps, override_settings(
+        INSTALLED_APPS=[], THEME_SKELETON_URL='http://combo.example.com/_skeleton_/'
+    ):
+        rsps.add_callback(responses.GET, 'http://combo.example.com/_skeleton_/', callback=callback)
+        yield rsps
+
+
+def test_theme_base_error(portal_mock_error, theme_base_source, caplog, freezer):
+    calls = portal_mock_error.calls
+    cache.clear()
+    caplog.set_level('WARNING')
+
+    assert not caplog.messages
+    assert theme_base_source('/') == TEMPLATE
+    wait_other_threads()
+    assert len(calls) == 3
+
+    calls.reset()
+    portal_mock_error.replace(
+        responses.GET, 'http://combo.example.com/_skeleton_/', body=requests.RequestException('boom!')
+    )
+
+    assert theme_base_source('/') == TEMPLATE
+    assert not calls
+    assert not caplog.messages
+
+    # move 5 minutes later...
+    freezer.tick(301)
+    assert theme_base_source('/') == TEMPLATE
+    wait_other_threads()
+
+    assert calls
+    assert re.match('WARNING.*failed to retrieve theme.*boom!', caplog.text)
+    caplog.clear()
+    calls.reset()
+
+    # move 1 hours later...
+    freezer.tick(3300)
+    assert theme_base_source('/') == TEMPLATE
+    wait_other_threads()
+    assert len(calls) == 1
+    assert re.match('WARNING.*failed to retrieve theme.*boom!', caplog.text)
+    caplog.clear()
+    calls.reset()
+
+    freezer.tick(301)
+
+    assert theme_base_source('/') == TEMPLATE
+    wait_other_threads()
+    assert len(calls) == 1
+    assert re.match('WARNING.*failed to retrieve theme.*boom!', caplog.text)
+    caplog.clear()
+    calls.reset()
+
+    freezer.tick(301)
+
+    assert theme_base_source('/') == TEMPLATE
+    wait_other_threads()
+    assert len(calls) == 1
+    assert re.match('ERROR.*failed to retrieve theme.*boom!', caplog.text)
+    caplog.clear()
+    calls.reset()
+
+    # after some times cache is evicted and...
+    cache.clear()
+    with pytest.raises(Exception, match=r'Failed to retrieve theme:.*boom'):
+        theme_base_source('/')
+
+
+@pytest.fixture
+def portal_mock_language():
+    def callback(request):
+        language = request.headers['Accept-Language']
+        headers = {'X-Combo-Skeleton-Pages': json.dumps({'1': 'http://testserver/foo'})}
+        if language == 'en':
+            return 200, headers, 'Skeleton for English'
+        elif language == 'fr':
+            return 200, headers, 'Skeleton for French'
+
+    with responses.RequestsMock() as rsps, override_settings(
+        INSTALLED_APPS=[],
+        LANGUAGES=[('en', 'English'), ('fr', 'French')],
+        THEME_SKELETON_URL='http://combo.example.com/_skeleton_/',
+    ):
+        rsps.add_callback(responses.GET, 'http://combo.example.com/_skeleton_/', callback=callback)
+        yield rsps
+
+
+def test_theme_base_language(portal_mock_language, theme_base_source):
+    cache.clear()
+
+    calls = portal_mock_language.calls
+
+    assert theme_base_source(path='/') == 'Skeleton for English'
+
+    wait_other_threads()
+
+    assert len(calls) == 5
+
+    language_and_urls = {(call.request.headers['accept-language'], call.request.url) for call in calls}
+    assert {
+        ('en', 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2F'),
+        ('fr', 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2F'),
+        ('en', 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2Ffoo'),
+        ('fr', 'http://combo.example.com/_skeleton_/?source=http%3A%2F%2Ftestserver%2Ffoo'),
+    } == language_and_urls
+
+    assert theme_base_source(path='/') == 'Skeleton for English'
+
+    with translation.override('fr'):
+        assert theme_base_source(path='/') == 'Skeleton for French'
 
 
 class IdPRequestFactory(RequestFactory):