# HG changeset patch
# User Laurent Wouters <lwouters@cenotelie.fr>
# Date 1524561737 -7200
# Node ID 198cb7d7b4ac46e8dfa7cd3841f8cc67c5388343
# Parent  0d474f888f4a1e2f4fec060113c8ab1286a26dab
[utils] Fixed issue in clearing QueryCache

When trying to add an item in a full QueryCache, the cache tries to make room.
It tries to list and remove non-permanent items with a transient counter (number
 of times it has been requested). However, there is a pathological case where
items could be non-permanent but still not have a transient counter because they
were added, but never requested. In some cases, the full cache could be flushed,
 including the permanent items.
This changeset attempts to fix this issue by only dropping the non-permanent
items that did not hav a transient counter.

diff -r 0d474f888f4a -r 198cb7d7b4ac cubicweb/test/unittest_utils.py
--- a/cubicweb/test/unittest_utils.py	Mon Nov 13 16:08:58 2017 +0100
+++ b/cubicweb/test/unittest_utils.py	Tue Apr 24 11:22:17 2018 +0200
@@ -109,6 +109,26 @@
                           'itemcount': 10,
                           'permanentcount': 5})
 
+    def test_clear_on_overflow(self):
+        """Tests that only non-permanent items in the cache are wiped-out on ceiling overflow
+        """
+        c = QueryCache(ceiling=10)
+        # set 10 values
+        for x in range(10):
+            c[x] = x
+        # arrange for the first 5 to be permanent
+        for x in range(5):
+            for r in range(QueryCache._maxlevel + 2):
+                v = c[x]
+                self.assertEqual(v, x)
+        # Add the 11-th
+        c[10] = 10
+        self.assertEqual(c._usage_report(),
+                         {'transientcount': 0,
+                          'itemcount': 6,
+                          'permanentcount': 5})
+
+
 class UStringIOTC(TestCase):
     def test_boolean_value(self):
         self.assertTrue(UStringIO())
diff -r 0d474f888f4a -r 198cb7d7b4ac cubicweb/utils.py
--- a/cubicweb/utils.py	Mon Nov 13 16:08:58 2017 +0100
+++ b/cubicweb/utils.py	Tue Apr 24 11:22:17 2018 +0200
@@ -684,10 +684,20 @@
                     break
                 level = v
         else:
-            # we removed cruft but everything is permanent
+            # we removed cruft
             if len(self._data) >= self._max:
-                logger.warning('Cache %s is full.' % id(self))
-                self._clear()
+                if len(self._permanent) >= self._max:
+                    # we really are full with permanents => clear
+                    logger.warning('Cache %s is full.' % id(self))
+                    self._clear()
+                else:
+                    # pathological case where _transient was probably empty ...
+                    # drop all non-permanents
+                    to_drop = set(self._data.keys()).difference(self._permanent)
+                    for k in to_drop:
+                        # should not be in _transient
+                        assert k not in self._transient
+                        self._data.pop(k, None)
 
     def _usage_report(self):
         with self._lock: