Bug 1132517 - Move bitly shortening to use official API library.

This also adds year long caching for the result.

.gitmodules

@@ -145,3 +145,6 @@
 [submodule "vendor/src/django-honeypot"]
 	path = vendor/src/django-honeypot
 	url = git://github.com/sunlightlabs/django-honeypot.git
+[submodule "vendor/src/bitly-api-python"]
+	path = vendor/src/bitly-api-python
+	url = https://github.com/bitly/bitly-api-python.git

kuma/demos/helpers.py

@@ -4,9 +4,11 @@
 import random
 
 from babel import localedata
+import bitly_api
 import jinja2
 
 from django.conf import settings
+from django.utils.encoding import smart_str
 from django.utils.tzinfo import LocalTimezone
 
 import jingo
@@ -26,6 +28,10 @@
 threadedcommentstags.reverse = reverse
 
 
+bitly = bitly_api.Connection(login=getattr(settings, 'BITLY_USERNAME', ''),
+                             api_key=getattr(settings, 'BITLY_API_KEY', ''))
+
+
 TEMPLATE_INCLUDE_CACHE_EXPIRES = getattr(settings,
                                          'TEMPLATE_INCLUDE_CACHE_EXPIRES', 300)
 
@@ -162,30 +168,20 @@ def search_form(context):
     return new_context(**locals())
 
 
-bitly_api = None
-
-
-def _get_bitly_api():
-    """Get an instance of the bit.ly API class"""
-    global bitly_api
-    if bitly_api is None:
-        import bitly
-        login = getattr(settings, 'BITLY_USERNAME', '')
-        apikey = getattr(settings, 'BITLY_API_KEY', '')
-        bitly_api = bitly.Api(login, apikey)
-    return bitly_api
-
-
 @register.filter
 def bitly_shorten(url):
     """Attempt to shorten a given URL through bit.ly / mzl.la"""
-    try:
-        # TODO:caching
-        return _get_bitly_api().shorten(url)
-    except:
-        # Just in case the bit.ly service fails or the API key isn't
-        # configured, fall back to using the original URL.
-        return url
+    cache_key = 'bitly:%s' % hashlib.md5(smart_str(url)).hexdigest()
+    short_url = memcache.get(cache_key)
+    if short_url is None:
+        try:
+            short_url = bitly.shorten(url)['url']
+            memcache.set(cache_key, short_url, 60 * 60 * 24 * 30 * 12)
+        except (bitly_api.BitlyError, KeyError):
+            # Just in case the bit.ly service fails or the API key isn't
+            # configured, fall back to using the original URL.
+            return url
+    return short_url
 
 
 @register.function

kuma/demos/tests/test_helpers.py

@@ -1,20 +1,49 @@
-import test_utils
-from nose.tools import eq_
-
+import bitly_api
 from django.conf import settings
+import mock
+from nose.tools import eq_, ok_
+import test_utils
 
-from ..helpers import tag_description
+from kuma.core.cache import memcache
+from ..helpers import tag_description, bitly_shorten, bitly
 
 
 class HelperTestCase(test_utils.TestCase):
 
     def test_tag_description_no_description(self):
-        settings.TAG_DESCRIPTIONS = {"tag_name": "test_tag",
-                 "title": "Testing tag without description"}
+        settings.TAG_DESCRIPTIONS = {
+            "tag_name": "test_tag",
+            "title": "Testing tag without description",
+        }
         description = tag_description("test_tag")
         eq_("test_tag", description)
 
     def test_tag_description_challenge_none(self):
         tag = 'challenge:none'
         description = tag_description(tag)
         eq_('Removed from Derby', description)
+
+    @mock.patch.object(memcache, 'set')  # prevent caching
+    @mock.patch.object(bitly, 'shorten')
+    def test_bitly_shorten(self, shorten, cache_set):
+        long_url = 'http://example.com/long-url'
+        short_url = 'http://bit.ly/short-url'
+
+        # the usual case of returning a dict with a URL
+        def short_mock(*args, **kwargs):
+            return {'url': short_url}
+        shorten.side_effect = short_mock
+
+        eq_(bitly_shorten(long_url), short_url)
+        shorten.assert_called_with(long_url)
+
+        # in case of a key error
+        def short_mock(*args, **kwargs):
+            return {}
+        shorten.side_effect = short_mock
+        eq_(bitly_shorten(long_url), long_url)
+        shorten.assert_called_with(long_url)
+
+        # in case of an upstream error
+        shorten.side_effect = bitly_api.BitlyError('500', 'fail fail fail')
+        eq_(bitly_shorten(long_url), long_url)

requirements/prod.txt

@@ -84,6 +84,7 @@ hash_ring==1.3.1
 django-recaptcha==0.0.9
 
 django-honeypot==0.4.0
+bitly_api==0.3
 
 ## Formerly vendor/src - arbitrary commits not matched to versions
 # 2011-12-29

vendor/kuma.pth

@@ -97,3 +97,4 @@ src/py-amqp
 src/django-celery
 src/django-banish
 src/django-honeypot
+src/bitly-api-python