fix: unmask delete errors, make values/items membership work, drop debug prints

dynamodol/base.py

@@ -12,10 +12,42 @@
 from dol import KvReader, KvPersister, Store, BaseValuesView, BaseItemsView
 
 
+_not_found = object()  # sentinel: distinguishes "absent" from a stored None
+
+
 class NoSuchKeyError(KeyError):
     pass
 
 
+#: Backend error codes that mean "the requested key does not exist".
+NO_SUCH_KEY_ERROR_CODES = frozenset({"NoSuchKey"})
+
+
+def is_no_such_key_error(error: BaseException) -> bool:
+    """Tell whether ``error`` reports a missing key.
+
+    Boto/botocore carry the backend's error code in
+    ``error.response['Error']['Code']``. Note that exception *instances* never
+    have a ``__name__`` (that lives on the class), so testing ``error.__name__``
+    only raises ``AttributeError`` and hides the error being inspected.
+
+    >>> is_no_such_key_error(ValueError("nope"))
+    False
+    >>> from botocore.exceptions import ClientError
+    >>> is_no_such_key_error(
+    ...     ClientError({"Error": {"Code": "NoSuchKey"}}, "DeleteItem")
+    ... )
+    True
+    """
+    response = getattr(error, "response", None)
+    if isinstance(response, Mapping):
+        error_info = response.get("Error") or {}
+        if isinstance(error_info, Mapping):
+            if error_info.get("Code") in NO_SUCH_KEY_ERROR_CODES:
+                return True
+    return type(error).__name__ in NO_SUCH_KEY_ERROR_CODES
+
+
 DFLT_TABLE_NAME = "dynamodol"
 DFLT_KEY_FIELDS = ("key",)
 DFLT_DATA_FIELDS = ("value",)
@@ -47,7 +79,6 @@ def get_db(
 
 
 def decimal_to_float(x):
-    print(f"x: {x}")
     if isinstance(x, str):
         return x
     if isinstance(x, Mapping):
@@ -95,15 +126,23 @@ class DynamoDbBaseReader(KvReader):
     exclude_keys_on_read: bool = field(default=True)
 
     class ValuesView(BaseValuesView):
+        """Values view backed by a single table scan (see ``iter_values``)."""
+
         def __contains__(self, v):
-            return self._mapping.contains_value(v)
+            return any(value is v or value == v for value in self)
 
         def __iter__(self):
             return self._mapping.iter_values()
 
     class ItemsView(BaseItemsView):
+        """Items view backed by a single table scan (see ``iter_items``)."""
+
         def __contains__(self, item):
-            return self._mapping.contains_item(item)
+            k, v = item
+            value = self._mapping.get(k, _not_found)
+            if value is _not_found:
+                return False
+            return value is v or value == v
 
         def __iter__(self):
             return self._mapping.iter_items()
@@ -175,7 +214,6 @@ def extract_obj_from_data(self, item):
     def format_get_item(self, item):
         """TODO: replace with _id_of_key, etc."""
         obj = self.extract_obj_from_data(item)
-        print(f"obj: {obj}")
         return decimal_to_float(obj)
 
     def format_get_key(self, item):
@@ -339,9 +377,8 @@ def __delitem__(self, k):
             key = {att: key for att, key in zip(self.key_fields, k)}
             self.table.delete_item(Key=key)
         except Exception as e:
-            if hasattr(e, "__name__"):
-                if e.__name__ == "NoSuchKey":
-                    raise NoSuchKeyError(f"Key not found: {k}")
+            if is_no_such_key_error(e):
+                raise NoSuchKeyError(f"Key not found: {k}") from e
             raise
 
 

dynamodol/partition_query.py

@@ -4,7 +4,12 @@
 from dataclasses import dataclass, field
 from typing import Any
 
-from dynamodol.base import DynamoDbBaseReader, DynamoDbBasePersister, db_defaults
+from dynamodol.base import (
+    DynamoDbBaseReader,
+    DynamoDbBasePersister,
+    db_defaults,
+    is_no_such_key_error,
+)
 
 
 class NoSuchKeyError(KeyError):
@@ -224,7 +229,6 @@ def format_get_key(self, item):
         return item[self.sort_key]
 
     def __getitem__(self, k):
-        print(f"getitem: {k}")
         try:
             key = {self.partition_key: self.partition, self.sort_key: k}
             response = self.table.get_item(Key=key)
@@ -287,6 +291,6 @@ def __delitem__(self, k):
         try:
             self.table.delete_item(Key=key)
         except Exception as e:
-            if getattr(e, "__name__") == "NoSuchKey":
-                raise NoSuchKeyError(f"Key not found: {k}")
+            if is_no_such_key_error(e):
+                raise NoSuchKeyError(f"Key not found: {k}") from e
             raise

tests/test_offline.py

@@ -0,0 +1,180 @@
+"""Offline tests for dynamodol, using an in-memory stand-in for a boto3 table.
+
+These tests exercise the store protocol (reads, membership views, deletes) with
+no AWS credentials and no DynamoDB Local: ``FakeDb`` plays the role of the boto3
+``dynamodb`` resource and ``FakeTable`` the role of a ``Table``.
+"""
+
+import pytest
+from botocore.exceptions import ClientError
+
+from dynamodol import DynamoDbBasePersister, DynamoDbPartitionPersister
+
+TABLE_NAME = "test_table"
+KEY_FIELDS = ("key",)
+DATA_FIELDS = ("value",)
+
+PARTITION_KEY_FIELDS = ("pkey", "skey")
+PARTITION = "part1"
+
+ONE_ITEM = ({"key": "k1", "value": "A"},)
+ONE_PARTITIONED_ITEM = ({"pkey": PARTITION, "skey": "s1", "value": "A"},)
+
+
+def _client_error(code, operation):
+    """Make a botocore ``ClientError`` shaped the way boto3 raises one."""
+    return ClientError(
+        {"Error": {"Code": code, "Message": f"simulated {code}"}}, operation
+    )
+
+
+class FakeTable:
+    """The subset of the boto3 ``Table`` API that dynamodol actually calls.
+
+    Items are held in a list. ``delete_error``, when given, is raised by
+    ``delete_item`` so backend failures can be exercised without a live table.
+    """
+
+    def __init__(self, items=(), *, delete_error=None):
+        self.items = [dict(item) for item in items]
+        self.delete_error = delete_error
+
+    @staticmethod
+    def _matches(item, key):
+        return all(item.get(k) == v for k, v in key.items())
+
+    def get_item(self, Key, **kwargs):
+        for item in self.items:
+            if self._matches(item, Key):
+                return {"Item": dict(item)}
+        return {}
+
+    def scan(self, **kwargs):
+        if kwargs.get("Select") == "COUNT":
+            return {"Count": len(self.items)}
+        return {"Items": [dict(item) for item in self.items]}
+
+    def query(self, **kwargs):
+        return self.scan(**kwargs)
+
+    def put_item(self, Item):
+        self.items = [item for item in self.items if not self._matches(item, Item)]
+        self.items.append(dict(Item))
+
+    def delete_item(self, Key):
+        if self.delete_error is not None:
+            raise self.delete_error
+        self.items = [item for item in self.items if not self._matches(item, Key)]
+
+
+class FakeDb:
+    """A boto3 ``dynamodb`` resource stand-in serving a single ``FakeTable``.
+
+    ``create_table`` always raises, which is the path dynamodol takes when the
+    table already exists: it falls back to ``Table(table_name)``.
+    """
+
+    def __init__(self, table):
+        self._table = table
+
+    def create_table(self, **kwargs):
+        raise _client_error("ResourceInUseException", "CreateTable")
+
+    def Table(self, table_name):
+        return self._table
+
+
+def _mk_persister(items=(), *, delete_error=None):
+    table = FakeTable(items, delete_error=delete_error)
+    store = DynamoDbBasePersister(
+        db=FakeDb(table),
+        table_name=TABLE_NAME,
+        key_fields=KEY_FIELDS,
+        data_fields=DATA_FIELDS,
+    )
+    return store, table
+
+
+def _mk_partition_persister(items=(), *, delete_error=None):
+    table = FakeTable(items, delete_error=delete_error)
+    store = DynamoDbPartitionPersister(
+        db=FakeDb(table),
+        table_name=TABLE_NAME,
+        key_fields=PARTITION_KEY_FIELDS,
+        data_fields=DATA_FIELDS,
+        partition=PARTITION,
+    )
+    return store, table
+
+
+# --- the library must not print --------------------------------------------
+
+
+def test_getitem_prints_nothing(capsys):
+    store, _ = _mk_persister(ONE_ITEM)
+    assert store["k1"] == "A"
+    assert capsys.readouterr().out == ""
+
+
+def test_partition_getitem_prints_nothing(capsys):
+    store, _ = _mk_partition_persister(ONE_PARTITIONED_ITEM)
+    assert store["s1"] == "A"
+    assert capsys.readouterr().out == ""
+
+
+def test_iteration_prints_nothing(capsys):
+    store, _ = _mk_persister(ONE_ITEM)
+    assert list(store.items()) == [("k1", "A")]
+    assert capsys.readouterr().out == ""
+
+
+# --- membership on the values and items views -------------------------------
+
+
+def test_values_view_contains():
+    store, _ = _mk_persister(ONE_ITEM)
+    assert "A" in store.values()
+    assert "ZZ" not in store.values()
+
+
+def test_items_view_contains():
+    store, _ = _mk_persister(ONE_ITEM)
+    assert ("k1", "A") in store.items()
+    assert ("k1", "ZZ") not in store.items()
+    assert ("no_such_key", "A") not in store.items()
+
+
+def test_partition_views_contains():
+    store, _ = _mk_partition_persister(ONE_PARTITIONED_ITEM)
+    assert "A" in store.values()
+    assert "ZZ" not in store.values()
+    assert ("s1", "A") in store.items()
+    assert ("s1", "ZZ") not in store.items()
+
+
+# --- __delitem__ must not mask the backend error ----------------------------
+
+
+def test_base_delitem_propagates_backend_error():
+    error = _client_error("ProvisionedThroughputExceededException", "DeleteItem")
+    store, _ = _mk_persister(ONE_ITEM, delete_error=error)
+    with pytest.raises(ClientError):
+        del store["k1"]
+
+
+def test_partition_delitem_propagates_backend_error():
+    error = _client_error("ProvisionedThroughputExceededException", "DeleteItem")
+    store, _ = _mk_partition_persister(ONE_PARTITIONED_ITEM, delete_error=error)
+    with pytest.raises(ClientError):
+        del store["s1"]
+
+
+@pytest.mark.parametrize(
+    "mk_store,key",
+    [(_mk_persister, "k1"), (_mk_partition_persister, "s1")],
+)
+def test_delitem_removes_the_item(mk_store, key):
+    items = ONE_ITEM if mk_store is _mk_persister else ONE_PARTITIONED_ITEM
+    store, table = mk_store(items)
+    del store[key]
+    assert table.items == []