Fix #1: .items()/.values() honour dol wrappers; scan_items/scan_values opt-in

dynamodol/__init__.py

@@ -1,6 +1,13 @@
 """Exports"""
 
-from .base import get_db, set_db_defaults, DynamoDbBasePersister, DynamoDbBaseReader
+from .base import (
+    get_db,
+    set_db_defaults,
+    DynamoDbBasePersister,
+    DynamoDbBaseReader,
+    scan_items,
+    scan_values,
+)
 from .partition_query import (
     DynamoDbPartitionReader,
     DynamoDbPrefixReader,

dynamodol/base.py

@@ -9,10 +9,7 @@
 from typing import Any, Tuple
 from collections.abc import Iterable, Mapping
 
-from dol import KvReader, KvPersister, Store, BaseValuesView, BaseItemsView
-
-
-_not_found = object()  # sentinel: distinguishes "absent" from a stored None
+from dol import KvReader, KvPersister, Store
 
 
 class NoSuchKeyError(KeyError):
@@ -149,6 +146,28 @@ def decimal_to_float(x):
     return x
 
 
+class _RemovedMethod:
+    """Attribute that raises an ``AttributeError`` naming the method's replacement.
+
+    A descriptor (rather than a ``__getattr__``) so it cannot mask unrelated
+    ``AttributeError``s raised inside the reader's own properties.
+    """
+
+    def __init__(self, name, replacement):
+        self.name, self.replacement = name, replacement
+
+    def __get__(self, instance, owner=None):
+        if instance is None:  # class access (dol's class-wrap enumerates dir(cls))
+            return self
+        raise AttributeError(
+            f"`{self.name}` was removed: as a method it ran bound to the inner store, "
+            f"so on a wrapped store (key codec, filt_iter...) it returned out-of-scope "
+            f"keys and raw values (i2mint/dynamodol#1). `.items()` / `.values()` are "
+            f"now correct on wrapped stores; for a single-scan fast path on an "
+            f"unwrapped store use {self.replacement}."
+        )
+
+
 @dataclass
 class DynamoDbBaseReader(KvReader):
     """A basic key-value reader for DynamoDb
@@ -184,28 +203,6 @@ class DynamoDbBaseReader(KvReader):
     data_fields: tuple[str] = field(default=None)
     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 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):
-            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()
-
     def __post_init__(self):
         for k, v in db_defaults.items():
             if getattr(self, k, None) is None:
@@ -334,15 +331,12 @@ def __getitem__(self, k):
         item = get_item_or_raise(self.table, _k, k, **self._values_expression)
         return self.format_get_item(item)
 
-    def iter_items(self):
-        response = self.table.scan(**self._keys_values_expression)
-        yield from (
-            (self.format_get_key(d), self.format_get_item(d)) for d in response["Items"]
-        )
+    def _scan_records(self, projection):
+        """The raw records for a ``projection`` (an ``Expression*`` kwargs dict)."""
+        return self.table.scan(**projection)["Items"]
 
-    def iter_values(self):
-        response = self.table.scan(**self._values_expression)
-        yield from (self.format_get_item(d) for d in response["Items"])
+    iter_items = _RemovedMethod("iter_items", "dynamodol.scan_items(store)")
+    iter_values = _RemovedMethod("iter_values", "dynamodol.scan_values(store)")
 
     def __iter__(self):
         # This is extremely inefficient and should not be used with large tables in production
@@ -484,3 +478,45 @@ def load_sample_data():
 
 
 # TODO class DynamoDbStore(DynamoDbBasePersister, Store): ...
+
+
+def _leaf_reader(store, *, fn):
+    if not isinstance(store, DynamoDbBaseReader):
+        raise TypeError(
+            f"`{fn}` needs an unwrapped dynamodol reader, got "
+            f"{type(store).__name__}. A wrapped store (key codec, filt_iter, value "
+            f"codec...) must be iterated with its own `.items()` / `.values()`, which "
+            f"apply the wrapper's scope and codecs (one `get` per key)."
+        )
+    return store
+
+
+def scan_items(store):
+    """Iterate ``(key, value)`` pairs of an *unwrapped* dynamodol reader in one scan.
+
+    The opt-in fast path that ``store.items()`` deliberately is not: ``items()`` goes
+    through ``__iter__`` and ``__getitem__`` (one get per key) so that it is correct
+    under any dol wrapper. Passing a wrapped store raises ``TypeError``, because a
+    free function cannot apply the wrapper's key mapping, scope or value decoding.
+
+    >>> import dynamodol
+    >>> dynamodol.scan_items(object())
+    Traceback (most recent call last):
+      ...
+    TypeError: `scan_items` needs an unwrapped dynamodol reader, got object. A wrapped store (key codec, filt_iter, value codec...) must be iterated with its own `.items()` / `.values()`, which apply the wrapper's scope and codecs (one `get` per key).
+    """
+    reader = _leaf_reader(
+        store, fn="scan_items"
+    )  # eager: fail at the call, not at first next()
+    records = reader._scan_records(reader._keys_values_expression)
+    return (
+        (reader.format_get_key(record), reader.format_get_item(record))
+        for record in records
+    )
+
+
+def scan_values(store):
+    """Iterate the values of an *unwrapped* dynamodol reader in one scan (see `scan_items`)."""
+    reader = _leaf_reader(store, fn="scan_values")
+    records = reader._scan_records(reader._values_expression)
+    return (reader.format_get_item(record) for record in records)

dynamodol/partition_query.py

@@ -174,17 +174,8 @@ def filter_kwargs(self):
             result["FilterExpression"] = self.attr_query
         return result
 
-    def iter_items(self):
-        response = self.table.query(
-            **self.filter_kwargs, **self._keys_values_expression
-        )
-        yield from (
-            (self.format_get_key(d), self.format_get_item(d)) for d in response["Items"]
-        )
-
-    def iter_values(self):
-        response = self.table.query(**self.filter_kwargs, **self._values_expression)
-        yield from (self.format_get_item(d) for d in response["Items"])
+    def _scan_records(self, projection):
+        return self.table.query(**self.filter_kwargs, **projection)["Items"]
 
     def __iter__(self):
         response = self.table.query(**self.filter_kwargs, **self._keys_expression)

tests/test_wrapped_views.py

@@ -0,0 +1,97 @@
+"""`.items()` / `.values()` on a wrapped store must honour the wrapper (dynamodol#1, dol#83)."""
+
+import pytest
+from botocore.exceptions import ClientError
+from dol import KeyCodecs, Pipe, filt_iter, wrap_kvs
+
+import dynamodol
+from dynamodol import DynamoDbBasePersister, DynamoDbPrefixReader
+
+from test_offline import FakeDb, FakeTable, KEY_FIELDS, DATA_FIELDS, TABLE_NAME
+
+ROWS = (
+    {"pkey": "part1", "skey": "v1/alice/notes.txt", "value": "ALICE-SECRET"},
+    {"pkey": "part1", "skey": "v1/bob/notes.txt", "value": "BOB-SECRET"},
+)
+
+# the standard dol idiom for scoping a store down to one tenant
+alice_only = Pipe(filt_iter.prefixes("alice/"), KeyCodecs.prefixed("alice/"))
+
+
+def _leaf(cls=DynamoDbPrefixReader):
+    return cls(
+        db=FakeDb(FakeTable(ROWS)),
+        table_name=TABLE_NAME,
+        key_fields=("pkey", "skey"),
+        data_fields=DATA_FIELDS,
+        partition="part1",
+        prefix="v1/",
+    )
+
+
+@pytest.mark.parametrize(
+    "mk", [lambda: alice_only(_leaf()), lambda: _leaf(alice_only(DynamoDbPrefixReader))]
+)
+def test_key_scoped_store_items_and_values_are_scoped(mk):
+    s = mk()
+    assert list(s) == ["notes.txt"]
+    assert dict(s) == {"notes.txt": "ALICE-SECRET"}
+    assert dict(s.items()) == dict(s)
+    assert list(s.values()) == ["ALICE-SECRET"]
+    assert ("notes.txt", "ALICE-SECRET") in s.items()
+    assert "BOB-SECRET" not in s.values()
+
+
+def test_value_codec_applies_to_values_and_items():
+    s = wrap_kvs(_leaf(), obj_of_data=lambda v: {"text": v})
+    assert s["alice/notes.txt"] == {"text": "ALICE-SECRET"}
+    assert list(s.values()) == [{"text": "ALICE-SECRET"}, {"text": "BOB-SECRET"}]
+    assert dict(s.items()) == dict(s)
+
+
+def test_unwrapped_items_match_dict():
+    s = _leaf()
+    assert dict(s.items()) == dict(s)
+
+
+def test_scan_fast_path_on_unwrapped_reader():
+    s = _leaf()
+    assert dict(dynamodol.scan_items(s)) == dict(s)
+    assert list(dynamodol.scan_values(s)) == list(s.values())
+
+
+def test_scan_refuses_wrapped_store():
+    with pytest.raises(TypeError, match="unwrapped"):
+        list(dynamodol.scan_items(alice_only(_leaf())))
+    with pytest.raises(TypeError, match="unwrapped"):
+        list(dynamodol.scan_values(alice_only(_leaf())))
+
+
+def test_removed_methods_point_to_replacement():
+    with pytest.raises(AttributeError, match="scan_items"):
+        _leaf().iter_items
+    with pytest.raises(AttributeError, match="scan_values"):
+        alice_only(_leaf()).iter_values
+    assert not hasattr(_leaf(), "iter_items")
+
+
+def test_real_attribute_errors_inside_properties_are_not_masked():
+    class BadDb:  # no `.Table`: e.g. a boto3 *client* passed where a resource is expected
+        def create_table(self, **kwargs):
+            raise ClientError({"Error": {"Code": "InUse"}}, "CreateTable")
+
+    s = DynamoDbPrefixReader(
+        db=BadDb(), table_name="t", key_fields=("p", "s"), partition="x"
+    )
+    with pytest.raises(AttributeError, match="'BadDb' object has no attribute 'Table'"):
+        s["k"]
+
+
+def test_base_persister_scan():
+    s = DynamoDbBasePersister(
+        db=FakeDb(FakeTable(({"key": "k1", "value": "A"},))),
+        table_name=TABLE_NAME,
+        key_fields=KEY_FIELDS,
+        data_fields=DATA_FIELDS,
+    )
+    assert list(dynamodol.scan_items(s)) == [("k1", "A")]