fix cache bubble up issue

Author: mredolatti

splitio/storage/adapters/cache_trait.py

@@ -109,41 +109,49 @@ def _bubble_up(self, node):
         if node is None:
             return None
 
+        # First item, just set lru & mru
+        if not self._data:
+            self._lru = node
+            self._mru = node
+            return node
+
+        # MRU, just return it
+        if node is self._mru:
+            return node
+
+        # LRU, update pointer and end-of-list
+        if node is self._lru:
+            self._lru = node.next
+            self._lru.previous = None
+
         if node.previous is not None:
             node.previous.next = node.next
-
         if node.next is not None:
             node.next.previous = node.previous
 
-        if self._lru == node:
-            if node.next is not None: #only update lru pointer if there are more than 1 elements.
-                self._lru = node.next
-
-        if not self._data:
-            # if there are no items, set the LRU to this node
-            self._lru = node
-        else:
-            # if there is at least one item, update the MRU chain
-            self._mru.next = node
-
-        node.next = None
         node.previous = self._mru
+        node.previous.next = node
+        node.next = None
         self._mru = node
+
         return node
 
     def _rollover(self):
         """Check we're within the size limit. Otherwise drop the LRU."""
         if len(self._data) > self._max_size:
             next_item = self._lru.next
+            if next_item is None:
+                print self
             del self._data[self._lru.key]
             self._lru = next_item
+            self._lru.previous = None
 
     def __str__(self):
         """User friendly representation of cache."""
         nodes = []
         node = self._mru
         while node is not None:
-            nodes.append('<%s: %s>  -->' % (node.key, node.value))
+            nodes.append('\t<%s: %s>  -->' % (node.key, node.value))
             node = node.previous
         return '<MRU>\n' + '\n'.join(nodes) + '\n<LRU>'
 

tests/storage/adapters/test_cache_trait.py

@@ -1,6 +1,7 @@
 """Cache testing module."""
 #pylint: disable=protected-access,no-self-use,line-too-long
 import time
+from random import choice
 
 import pytest
 
@@ -55,6 +56,9 @@ def test_lru_behaviour(self, mocker):
         assert 'some' not in cache._data
         assert len(cache._data) == 5
         assert cache._lru.key == 'some_other'
+        for node in cache._data.values():
+            if node != cache._mru:
+                assert node.next is not None
 
         # `some_other` should be the next key to be evicted.
         # if we issue a get call for it, it should be marked as the MRU,
@@ -64,6 +68,21 @@ def test_lru_behaviour(self, mocker):
         assert cache._mru.key == 'some_other'
         assert cache._lru.key == 'another'
 
+    def test_intensive_usage_behavior(self, mocker):
+        """Test fetches with random repeated strings."""
+        user_func = mocker.Mock()
+        user_func.side_effect = lambda *p, **kw: len(p[0])
+        key_func = mocker.Mock()
+        key_func.side_effect = lambda *p, **kw: p[0]
+        cache = cache_trait.LocalMemoryCache(key_func, user_func, 1, 5)
+
+        strings = ['a', 'bb', 'ccc', 'dddd', 'eeeee', 'ffffff', 'ggggggg', 'hhhhhhhh',
+                   'jjjjjjjjj', 'kkkkkkkkkk']
+        for _ in range(0, 100000):
+            chosen = choice(strings)
+            assert cache.get(chosen) == len(chosen)
+            assert cache._lru is not None
+
     def test_expiration_behaviour(self, mocker):
         """Test time expiration works as expected."""
         user_func = mocker.Mock()