From 813cfbf33616054d25500c31e69c2ab4dba6891a Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Tue, 24 May 2022 15:47:31 +0300 Subject: [PATCH] Fix cache panel miss counting The cache panel was not counting misses for the get_many() cache method, because it assumed that all keys would be present in the returned dict (with a value of None if not present in the cache) while in reality only keys present in the cache are present in the returned dict. Correct the miss counting logic, and add a test for tracking hits and misses. --- debug_toolbar/panels/cache.py | 11 ++++++----- docs/changes.rst | 2 ++ tests/panels/test_cache.py | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/debug_toolbar/panels/cache.py b/debug_toolbar/panels/cache.py index 7877ef0ab..8294d7734 100644 --- a/debug_toolbar/panels/cache.py +++ b/debug_toolbar/panels/cache.py @@ -101,11 +101,12 @@ def _store_call_info( else: self.hits += 1 elif name == "get_many": - for key, value in return_value.items(): - if value is None: - self.misses += 1 - else: - self.hits += 1 + if "keys" in kwargs: + keys = kwargs["keys"] + else: + keys = args[0] + self.hits += len(return_value) + self.misses += len(keys) - len(return_value) time_taken *= 1000 self.total_time += time_taken diff --git a/docs/changes.rst b/docs/changes.rst index 9b50631ba..019a6eac3 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -8,6 +8,8 @@ Change log instances isn't a valid use of Django but, again, django-debug-toolbar shouldn't crash. * Added pyflame (for flame graphs) to the list of third-party panels. +* Fixed the cache panel to correctly count cache misses from the get_many() + cache method. 3.4.0 (2022-05-03) ------------------ diff --git a/tests/panels/test_cache.py b/tests/panels/test_cache.py index d45eabb26..aacf521cb 100644 --- a/tests/panels/test_cache.py +++ b/tests/panels/test_cache.py @@ -26,6 +26,23 @@ def test_recording_caches(self): second_cache.get("foo") self.assertEqual(len(self.panel.calls), 2) + def test_hits_and_misses(self): + cache.cache.clear() + cache.cache.get("foo") + self.assertEqual(self.panel.hits, 0) + self.assertEqual(self.panel.misses, 1) + cache.cache.set("foo", 1) + cache.cache.get("foo") + self.assertEqual(self.panel.hits, 1) + self.assertEqual(self.panel.misses, 1) + cache.cache.get_many(["foo", "bar"]) + self.assertEqual(self.panel.hits, 2) + self.assertEqual(self.panel.misses, 2) + cache.cache.set("bar", 2) + cache.cache.get_many(keys=["foo", "bar"]) + self.assertEqual(self.panel.hits, 4) + self.assertEqual(self.panel.misses, 2) + def test_get_or_set_value(self): cache.cache.get_or_set("baz", "val") self.assertEqual(cache.cache.get("baz"), "val")