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..18d782e 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): @@ -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) 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..5114b3e --- /dev/null +++ b/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")]