chore: Revert to original fix

Author: Michael Brewer

Diff for: aws_lambda_powertools/shared/cache_dict.py

@@ -1,32 +1,31 @@
 from collections import OrderedDict
 
 
-class LRUDict:
+class LRUDict(OrderedDict):
     """
     Cache implementation based on ordered dict with a maximum number of items. Last accessed item will be evicted
-    first, unless the item is None which will be evicted first. Currently used only by idempotency utility.
+    first. Currently used by idempotency utility.
     """
 
     def __init__(self, max_items=1024, *args, **kwargs):
         self.max_items = max_items
-        self._cache = OrderedDict(*args, **kwargs)
+        super().__init__(*args, **kwargs)
 
-    def __getitem__(self, key: str):
-        value = self._cache.__getitem__(key)
-        if value:
-            self._cache.move_to_end(key)
+    def __getitem__(self, key):
+        value = super().__getitem__(key)
+        self.move_to_end(key)
         return value
 
-    def __setitem__(self, key: str, value):
-        self._cache.__setitem__(key, value)
-        if key in self._cache:
-            self._cache.move_to_end(key, last=value is not None)
-        if len(self._cache) > self.max_items:
-            oldest = next(iter(self._cache))
-            del self._cache[oldest]
+    def __setitem__(self, key, value):
+        if key in self:
+            self.move_to_end(key)
+        super().__setitem__(key, value)
+        if len(self) > self.max_items:
+            oldest = next(iter(self))
+            del self[oldest]
 
-    def get(self, key: str):
-        value = self._cache.get(key)
-        if value:
-            self._cache.move_to_end(key)
-        return value
+    def get(self, key, *args, **kwargs):
+        item = super(LRUDict, self).get(key, *args, **kwargs)
+        if item:
+            self.move_to_end(key=key)
+        return item

Diff for: aws_lambda_powertools/utilities/idempotency/persistence/base.py

@@ -272,7 +272,8 @@ def _retrieve_from_cache(self, idempotency_key: str):
     def _delete_from_cache(self, idempotency_key: str):
         if not self.use_local_cache:
             return
-        self._cache[idempotency_key] = None
+        if idempotency_key in self._cache:
+            del self._cache[idempotency_key]
 
     def save_success(self, event: Dict[str, Any], result: dict) -> None:
         """

Diff for: tests/unit/test_lru_cache.py

@@ -9,60 +9,72 @@
 
 
 @pytest.fixture
-def populated_cache() -> LRUDict:
+def populated_cache():
     cache_dict = LRUDict(max_items=MAX_CACHE_ITEMS, **{f"key_{i}": f"val_{i}" for i in range(0, PREFILL_CACHE_ITEMS)})
     return cache_dict
 
 
-def test_cache_order_init(populated_cache: LRUDict):
-    first_item = list(populated_cache._cache)[0]
-    last_item = list(populated_cache._cache)[-1]
+def test_cache_order_init(populated_cache):
+    first_item = list(populated_cache)[0]
+    last_item = list(populated_cache)[-1]
 
     assert first_item == "key_0"
     assert last_item == f"key_{MAX_CACHE_ITEMS - 1}"
 
 
-def test_cache_order_getitem(populated_cache: LRUDict):
+def test_cache_order_getitem(populated_cache):
     random_value = random.randrange(0, MAX_CACHE_ITEMS)
     _ = populated_cache[f"key_{random_value}"]
 
-    last_item = list(populated_cache._cache)[-1]
+    last_item = list(populated_cache)[-1]
 
     assert last_item == f"key_{random_value}"
 
 
-def test_cache_order_get(populated_cache: LRUDict):
+def test_cache_order_get(populated_cache):
     random_value = random.randrange(0, MAX_CACHE_ITEMS)
     _ = populated_cache.get(f"key_{random_value}")
 
-    last_item = list(populated_cache._cache)[-1]
+    last_item = list(populated_cache)[-1]
 
     assert last_item == f"key_{random_value}"
 
 
-def test_cache_evict_over_max_items(populated_cache: LRUDict):
-    assert "key_0" in populated_cache._cache
-    assert len(populated_cache._cache) == MAX_CACHE_ITEMS
+def test_cache_evict_over_max_items(populated_cache):
+    assert "key_0" in populated_cache
+    assert len(populated_cache) == MAX_CACHE_ITEMS
     populated_cache["new_item"] = "new_value"
-    assert len(populated_cache._cache) == MAX_CACHE_ITEMS
-    assert "key_0" not in populated_cache._cache
-    assert "key_1" in populated_cache._cache
+    assert len(populated_cache) == MAX_CACHE_ITEMS
+    assert "key_0" not in populated_cache
+    assert "key_1" in populated_cache
 
 
-def test_setitem_moves_to_end(populated_cache: LRUDict):
+def test_setitem_moves_to_end(populated_cache):
     random_value = random.randrange(0, MAX_CACHE_ITEMS)
     populated_cache[f"key_{random_value}"] = f"new_val_{random_value}"
-    last_item = list(populated_cache._cache)[-1]
+    last_item = list(populated_cache)[-1]
 
     assert last_item == f"key_{random_value}"
     assert populated_cache[f"key_{random_value}"] == f"new_val_{random_value}"
 
 
-def test_setitem_none_not_moved(populated_cache: LRUDict):
-    populated_cache["value_is_none"] = None
-
-    first_item = list(populated_cache._cache)[0]
-    last_item = list(populated_cache._cache)[-1]
-
-    assert first_item == "key_0"
-    assert last_item == f"key_{MAX_CACHE_ITEMS - 1}"
+def test_lru_pop_failing():
+    cache = LRUDict()
+    key = "test"
+    cache[key] = "value"
+    try:
+        cache.pop(key, None)
+        pytest.fail("GitHub #300: LRUDict pop bug has been fixed :)")
+    except KeyError as e:
+        assert e.args[0] == key
+
+
+def test_lru_del():
+    cache = LRUDict()
+    key = "test"
+    cache[key] = "value"
+    assert len(cache) == 1
+    if key in cache:
+        del cache[key]
+    assert key not in cache
+    assert len(cache) == 0