From 59246fac4d8ee1f6cbe56fc3456ae267620588ac Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 07:31:40 +0000 Subject: [PATCH 1/3] fix(#1): .items()/.values() honour dol wrappers; scan_items/scan_values opt-in fast path Drop the nested ValuesView/ItemsView (they routed through leaf-bound iter_items/iter_values, escaping any key/value codec). iter_items/iter_values are replaced by free functions that accept only an unwrapped reader. Co-Authored-By: Claude Sonnet 5.5 --- dynamodol/__init__.py | 9 +++- dynamodol/base.py | 93 +++++++++++++++++++++++------------- dynamodol/partition_query.py | 13 +---- tests/test_wrapped_views.py | 85 ++++++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 46 deletions(-) create mode 100644 tests/test_wrapped_views.py diff --git a/dynamodol/__init__.py b/dynamodol/__init__.py index 78b3f28..6e3af28 100644 --- a/dynamodol/__init__.py +++ b/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, diff --git a/dynamodol/base.py b/dynamodol/base.py index 352bcd8..af84510 100644 --- a/dynamodol/base.py +++ b/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): @@ -184,28 +181,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,16 +309,29 @@ 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"] + + #: Removed methods -> what replaces them (see `__getattr__`). + _removed_methods = { + "iter_items": "dynamodol.scan_items(store)", + "iter_values": "dynamodol.scan_values(store)", + } + + def __getattr__(self, name): + if name in type(self)._removed_methods: + raise AttributeError( + f"`{name}` was removed: as a method it ran bound to the inner store, so " + f"on a wrapped store (key codec, filt_iter...) it returned out-of-scope " + f"keys and raw values (i2mint/dynamodol#1). `.items()` / `.values()` " + f"are now correct on wrapped stores; for a single-scan fast path on an " + f"unwrapped store use {type(self)._removed_methods[name]}." + ) + raise AttributeError( + f"{type(self).__name__!r} object has no attribute {name!r}" ) - def iter_values(self): - response = self.table.scan(**self._values_expression) - yield from (self.format_get_item(d) for d in response["Items"]) - def __iter__(self): # This is extremely inefficient and should not be used with large tables in production response = self.table.scan(**self._keys_expression) @@ -484,3 +472,40 @@ 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") + for record in reader._scan_records(reader._keys_values_expression): + yield reader.format_get_key(record), reader.format_get_item(record) + + +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") + for record in reader._scan_records(reader._values_expression): + yield reader.format_get_item(record) diff --git a/dynamodol/partition_query.py b/dynamodol/partition_query.py index cc74e31..292a8d4 100644 --- a/dynamodol/partition_query.py +++ b/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) diff --git a/tests/test_wrapped_views.py b/tests/test_wrapped_views.py new file mode 100644 index 0000000..d4c956f --- /dev/null +++ b/tests/test_wrapped_views.py @@ -0,0 +1,85 @@ +"""`.items()` / `.values()` on a wrapped store must honour the wrapper (dynamodol#1, dol#83).""" + +import pytest +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 + with pytest.raises(AttributeError, match="no attribute 'nope'"): + _leaf().nope + + +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")] From 4dad02f31363689a35766504077493aabd906c28 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 07:34:02 +0000 Subject: [PATCH 2/3] fix(#1): review fix -- removed methods are descriptors, not a __getattr__ that masks AttributeErrors Co-Authored-By: Claude Sonnet 5.5 --- dynamodol/base.py | 42 +++++++++++++++++++++---------------- tests/test_wrapped_views.py | 16 ++++++++++++-- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/dynamodol/base.py b/dynamodol/base.py index af84510..aa1b675 100644 --- a/dynamodol/base.py +++ b/dynamodol/base.py @@ -146,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 @@ -313,24 +335,8 @@ def _scan_records(self, projection): """The raw records for a ``projection`` (an ``Expression*`` kwargs dict).""" return self.table.scan(**projection)["Items"] - #: Removed methods -> what replaces them (see `__getattr__`). - _removed_methods = { - "iter_items": "dynamodol.scan_items(store)", - "iter_values": "dynamodol.scan_values(store)", - } - - def __getattr__(self, name): - if name in type(self)._removed_methods: - raise AttributeError( - f"`{name}` was removed: as a method it ran bound to the inner store, so " - f"on a wrapped store (key codec, filt_iter...) it returned out-of-scope " - f"keys and raw values (i2mint/dynamodol#1). `.items()` / `.values()` " - f"are now correct on wrapped stores; for a single-scan fast path on an " - f"unwrapped store use {type(self)._removed_methods[name]}." - ) - raise AttributeError( - f"{type(self).__name__!r} object has no attribute {name!r}" - ) + 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 diff --git a/tests/test_wrapped_views.py b/tests/test_wrapped_views.py index d4c956f..5114b3e 100644 --- a/tests/test_wrapped_views.py +++ b/tests/test_wrapped_views.py @@ -1,6 +1,7 @@ """`.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 @@ -71,8 +72,19 @@ def test_removed_methods_point_to_replacement(): _leaf().iter_items with pytest.raises(AttributeError, match="scan_values"): alice_only(_leaf()).iter_values - with pytest.raises(AttributeError, match="no attribute 'nope'"): - _leaf().nope + 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(): From 21c93a64182e6455c444f50645b53964a7ea26e8 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 07:34:43 +0000 Subject: [PATCH 3/3] fix(#1): scan_items/scan_values validate eagerly Co-Authored-By: Claude Sonnet 5.5 --- dynamodol/base.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/dynamodol/base.py b/dynamodol/base.py index aa1b675..18d782e 100644 --- a/dynamodol/base.py +++ b/dynamodol/base.py @@ -505,13 +505,18 @@ def scan_items(store): ... 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") - for record in reader._scan_records(reader._keys_values_expression): - yield reader.format_get_key(record), reader.format_get_item(record) + 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") - for record in reader._scan_records(reader._values_expression): - yield reader.format_get_item(record) + records = reader._scan_records(reader._values_expression) + return (reader.format_get_item(record) for record in records)