Merge pull request #13 from metabrainz/BU10-expire

BU-10: Correctly expire cache keys

brainzutils/cache.py

@@ -97,12 +97,10 @@ def set(key, val, time=0, namespace=None, encode=True):
     """Set a key to a given value.
 
     Args:
-        key: Key of the item.
+        key (str): Key of the item.
         val: Item's value.
-        time: The time after which this value should expire, either as a delta
-            number of seconds, or an absolute unix time-since-the-epoch value.
-            If set to 0, value will be stored "forever".
-        namespace: Optional namespace in which key needs to be defined.
+        time (int): The time after which this value should expire, in seconds.
+        namespace (str): Optional namespace in which key needs to be defined.
         encode: True if the value should be encoded with msgpack, False otherwise
 
     Returns:
@@ -154,7 +152,7 @@ def set_many(mapping, time=None, namespace=None, encode=True):
 
     Args:
         mapping (dict): A dict of key/value pairs to set.
-        time (int): Time to store the keys (in milliseconds).
+        time (int): The time after which this value should expire, in seconds.
         namespace (str): Namespace for the keys.
         encode: True if the values should be encoded with msgpack, False otherwise
 
@@ -165,7 +163,7 @@ def set_many(mapping, time=None, namespace=None, encode=True):
     result = _r.mset(_prep_dict(mapping, namespace, encode))
     if time:
         for key in _prep_keys_list(list(mapping.keys()), namespace):
-            _r.pexpire(_prep_key(key, namespace), time)
+            _r.pexpire(key, time * 1000)
 
     return result
 

brainzutils/test/test_cache.py

@@ -3,9 +3,11 @@
 
 import datetime
 import os
-import redis
 import unittest
 
+import mock as mock
+import redis
+
 from brainzutils import cache
 
 
@@ -146,3 +148,42 @@ def test_increment_invalid_value(self):
         cache.set("a", "not a number")
         with self.assertRaises(redis.exceptions.ResponseError):
             cache.increment("a")
+
+
+class CacheKeyTestCase(unittest.TestCase):
+    namespace = "NS_TEST"
+
+    @mock.patch('brainzutils.cache.redis.StrictRedis', autospec=True)
+    def test_set_key(self, mock_redis):
+        """Test setting a bytes value"""
+        cache.init(host='host', port=2, namespace=self.namespace)
+        cache.set('key', u'value'.encode('utf-8'))
+
+        # Keys are encoded into bytes always
+        expected_key = 'NS_TEST:a62f2225bf70bfaccbc7f1ef2a397836717377de'.encode('utf-8')
+        # msgpack encoded value
+        expected_value = b'\xc4\x05value'
+        mock_redis.return_value.mset.assert_called_with({expected_key: expected_value})
+        mock_redis.return_value.pexpire.assert_not_called()
+
+    @mock.patch('brainzutils.cache.redis.StrictRedis', autospec=True)
+    def test_set_key_unicode(self, mock_redis):
+        """Test setting a unicode value"""
+        cache.init(host='host', port=2, namespace=self.namespace)
+        cache.set('key', u'value')
+
+        expected_key = 'NS_TEST:a62f2225bf70bfaccbc7f1ef2a397836717377de'.encode('utf-8')
+        # msgpack encoded value
+        expected_value = b'\xa5value'
+        mock_redis.return_value.mset.assert_called_with({expected_key: expected_value})
+        mock_redis.return_value.pexpire.assert_not_called()
+
+    @mock.patch('brainzutils.cache.redis.StrictRedis', autospec=True)
+    def test_key_expire(self, mock_redis):
+        cache.init(host='host', port=2, namespace=self.namespace)
+        cache.set('key', u'value'.encode('utf-8'), time=30)
+        expected_key = 'NS_TEST:a62f2225bf70bfaccbc7f1ef2a397836717377de'.encode('utf-8')
+        # msgpack encoded value
+        expected_value = b'\xc4\x05value'
+        mock_redis.return_value.mset.assert_called_with({expected_key: expected_value})
+        mock_redis.return_value.pexpire.assert_called_with(expected_key, 30000)

requirements_dev.txt

@@ -1,3 +1,4 @@
 pytest==3.1.2
 pytest-cov==2.5.1
 pylint==1.7.2
+mock==2.0.0