From 91cb8a7ad0f422690c52a203a2ccdaadbbd8e68c Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:26:16 +0000 Subject: [PATCH 1/3] fix: every RedisList operation on a wrong-type key is a TypeError; bytes-like elements (#15) Co-Authored-By: Claude Sonnet 5.5 --- redisdol/base.py | 98 ++++++++++++++++++++++--- tests/test_wrapping_fakeredis.py | 118 ++++++++++++++++++++++++++++++- 2 files changed, 205 insertions(+), 11 deletions(-) diff --git a/redisdol/base.py b/redisdol/base.py index b2b8ba4..4c19f00 100644 --- a/redisdol/base.py +++ b/redisdol/base.py @@ -46,10 +46,21 @@ def from_sourced_object(cls, obj): def _wrapped_class_of(cls): - """``(wrapped class, wrapper class)`` of a ``dol`` class-wrapper (``wrap_kvs``, + """``(wrapped class, delegator class)`` of a ``dol`` class-wrapper (``wrap_kvs``, ``KeyCodecs...``, ``filt_iter``), read from the closure of the ``__init__`` dol generates for it (``None`` if it has none, or delegates through another attribute - than ``store``).""" + than ``store``). Reads the ``_wrapped_class`` / ``_wrapper_class`` / + ``_delegation_attr`` attributes dol sets on the wrapper (dol#131); falls back to the + closure of its generated ``__init__`` for older dol versions.""" + attrs = tuple( + getattr(cls, a, None) + for a in ("_wrapped_class", "_wrapper_class", "_delegation_attr") + ) + if all(a is not None for a in attrs): + wrapped, wrapper, attr = attrs + if isinstance(wrapped, type) and isinstance(wrapper, type) and attr == "store": + return wrapped, wrapper + return None init = getattr(cls, "__init__", None) code, cells = getattr(init, "__code__", None), getattr(init, "__closure__", None) if code is None or not cells: @@ -65,7 +76,7 @@ def _wrapped_class_of(cls): return None if not (isinstance(wrapped, type) and isinstance(wrapper, type) and attr == "store"): return None - return wrapped, wrapper + return wrapped, wrapper.__bases__[0] # the class dol's ``Wrap`` subclasses def _new_with_source(cls, source): @@ -79,11 +90,11 @@ def _new_with_source(cls, source): "Store whose wrapped class can't be determined. Build the store over " "the connection first and wrap that instance instead." ) - wrapped, wrapper = found + wrapped, delegator = found self = object.__new__(cls) - # what dol's generated __init__ does after building the delegate: the wrapper + # what dol's generated __init__ does after building the delegate: the delegator # class's own __init__ (which may set state of its own), not Store's directly - super(wrapper, self).__init__(_new_with_source(wrapped, source)) + delegator.__init__(self, _new_with_source(wrapped, source)) return self self = object.__new__(cls) self._source = source @@ -274,6 +285,49 @@ def _translate_list_response_error(e: ResponseError, what: str): return None +def _wrongtype_as_type_error(what): + """Decorator factory: a ``WRONGTYPE`` ``ResponseError`` raised by the method becomes the + ``TypeError`` that :func:`_translate_list_response_error` maps it to, whichever + operation met a key that holds something other than a list. Any other error passes + through untouched.""" + + def decorator(method): + @wraps(method) + def wrapper(self, *args, **kwargs): + try: + return method(self, *args, **kwargs) + except ResponseError as e: + translated = _translate_list_response_error(e, what) + if isinstance(translated, TypeError): + raise translated from e + raise + + return wrapper + + return decorator + + +def _normalising_bytes_likes(encode): + """``encode`` (or the identity) followed by ``bytes(...)`` for a bytes-like result. + + redis-py would send a ``memoryview`` cast to ``'I'`` by its *length* rather than its + bytes; a bytes-like element is a redis string, as a top-level value is. + """ + encode = encode or _identity + + def encode_element(v): + v = encode(v) + return bytes(v) if isinstance(v, (bytearray, memoryview)) else v + + return encode_element + + +def _type_name(source, name): + """The redis type of ``name`` as bytes (``b'none'`` when it doesn't exist).""" + t = source.type(name) + return t.encode() if isinstance(t, str) else t + + # TODO: Redis list type is more of a linked-list than a list. Better subclass deque? class RedisList(MutableSequence): """ @@ -299,12 +353,14 @@ class RedisList(MutableSequence): def __init__(self, source, name, *, encode=None, decode=None): self._source = source self._name = name - self._encode = encode or _identity + self._encode = _normalising_bytes_likes(encode) self._decode = decode or _identity + @_wrongtype_as_type_error("RedisList length") def __len__(self): return self._source.llen(self._name) + @_wrongtype_as_type_error("RedisList access") def __getitem__(self, i): if isinstance(i, slice): return self._get_slice(i) @@ -329,7 +385,13 @@ def __iter__(self): n = max(1, int(self.iter_batch_size)) start = 0 while True: - batch = self._source.lrange(self._name, start, start + n - 1) + try: + batch = self._source.lrange(self._name, start, start + n - 1) + except ResponseError as e: # a generator: the decorator can't wrap it + translated = _translate_list_response_error(e, "RedisList iteration") + if isinstance(translated, TypeError): + raise translated from e + raise for v in batch: yield self._decode(v) if len(batch) < n: @@ -370,19 +432,35 @@ def __delitem__(self, i): raise raise translated from e + @_wrongtype_as_type_error("RedisList append") def append(self, v): # rpush (not rpushx): appending to a not-yet-existing list creates it, # consistently with `extend`. self._source.rpush(self._name, self._encode(v)) + @_wrongtype_as_type_error("RedisList extend") def extend(self, iterable): values = [self._encode(v) for v in iterable] if values: # rpush with no values is a redis error self._source.rpush(self._name, *values) def clear(self): - self._source.delete(self._name) + """Delete the list. Like the other operations, refuses (``TypeError``) a key that + holds something else, which this handle must not destroy; the check and the + deletion are one WATCH/MULTI transaction.""" + name = self._name + + def _clear(pipe): + if _type_name(pipe, name) not in (RedisType.list, b"none"): + raise TypeError( + "RedisList clear: the key holds a Redis value that is not a list" + ) + pipe.multi() + pipe.delete(name) + + self._source.transaction(_clear, name) + @_wrongtype_as_type_error("RedisList insert") def insert(self, i, v): """Insert ``v`` before position ``i`` (``list.insert`` semantics: ``i`` is clamped). @@ -459,7 +537,7 @@ def __setitem__(self, k, v): if isinstance(v, (bytearray, memoryview)): v = bytes(v) # bytes-like: a redis string, not a list of ints if isinstance(v, Iterable) and not isinstance(v, (str, bytes)): - encode = self._list_codec("list_encode") or _identity + encode = _normalising_bytes_likes(self._list_codec("list_encode")) values = [encode(x) for x in v] # also consumes one-shot iterables if not values: # Redis has no empty list (a list key with no elements doesn't exist). diff --git a/tests/test_wrapping_fakeredis.py b/tests/test_wrapping_fakeredis.py index 3cebd5b..591365c 100644 --- a/tests/test_wrapping_fakeredis.py +++ b/tests/test_wrapping_fakeredis.py @@ -2,7 +2,7 @@ import pytest from fakeredis import FakeRedis -from dol import KeyCodecs, wrap_kvs +from dol import KeyCodecs, Store, wrap_kvs from redisdol import ( RedisBytesPersister, @@ -453,3 +453,119 @@ def execute(s): with pytest.raises(ResponseError, match="READONLY") as info: del RedisList(ReadOnlyPipe(), "no such key")[0] assert info.value.__cause__ is None # not its own cause + + +# ---- every list operation on a wrong-type key; bytes-like elements (i2mint/redisdol#15) -- + + +def _wrong_type_lists(): + r = FakeRedis() + r.set("s", "text") + r.hset("h", "f", "v") + return r, [RedisList(r, "s"), RedisList(r, "h")] + + +_READ_OPS = { + "getitem": lambda l: l[0], + "negative": lambda l: l[-1], + "slice": lambda l: l[0:2], + "stepped slice": lambda l: l[::2], + "len": len, + "iter": list, + "contains": lambda l: "x" in l, + "index": lambda l: l.index("x"), + "count": lambda l: l.count("x"), + "reversed": lambda l: list(reversed(l)), + "append": lambda l: l.append("x"), + "extend": lambda l: l.extend(["x", "y"]), + "insert": lambda l: l.insert(0, "x"), + "pop": lambda l: l.pop(), + "pop0": lambda l: l.pop(0), + "remove": lambda l: l.remove("x"), + "clear": lambda l: l.clear(), + "iadd": lambda l: l.__iadd__(["x"]), + "setitem": lambda l: l.__setitem__(0, "x"), + "delitem": lambda l: l.__delitem__(0), +} + + +@pytest.mark.parametrize("op", list(_READ_OPS.values()), ids=list(_READ_OPS)) +def test_every_list_operation_on_a_wrong_type_key_is_a_type_error(op): + r, lists = _wrong_type_lists() + for lst in lists: + with pytest.raises(TypeError, match="not a list") as info: + op(lst) + assert type(info.value.__cause__).__name__ == "ResponseError" or ( + info.value.__cause__ is None # raised by our own type check (clear) + ) + assert r.get("s") == b"text" # nothing was destroyed + assert r.hget("h", "f") == b"v" + + +def test_clear_still_deletes_lists_and_tolerates_a_missing_key(): + r = FakeRedis() + r.rpush("l", "a", "b") + RedisList(r, "l").clear() + assert not r.exists("l") + RedisList(r, "nope").clear() # like list.clear on an empty list: no error + + +def test_decode_responses_connection_still_gets_type_errors(): + r = FakeRedis(decode_responses=True) + r.set("s", "text") + with pytest.raises(TypeError, match="not a list"): + RedisList(r, "s").clear() + assert r.get("s") == "text" + + +def test_missing_key_reads_are_unchanged(): + lst = RedisList(FakeRedis(), "nope") + assert len(lst) == 0 and list(lst) == [] and lst[0:3] == [] + with pytest.raises(IndexError): + lst[0] + + +@pytest.mark.parametrize("make", [bytearray, memoryview]) +def test_bytes_like_elements_are_stored_as_their_bytes(make): + r = FakeRedis() + lst = RedisList(r, "l") + lst.append(make(b"ab")) + lst.extend([make(b"cd")]) + lst.insert(0, make(b"zz")) + assert r.lrange("l", 0, -1) == [b"zz", b"ab", b"cd"] + lst[0] = make(b"yy") + assert lst[0] == b"yy" + + +def test_cast_memoryview_element_is_sent_by_its_bytes(): + import array + + mv = memoryview(array.array("I", [1, 2])) + assert len(mv) == 2 and mv.nbytes == 8 # redis-py would send len(mv) + r = FakeRedis() + RedisList(r, "l").append(mv) + assert r.lindex("l", 0) == bytes(mv) + s = _store() + s["k"] = [mv, b"x"] # via the store's list path as well + assert s._source.lindex("k", 0) == bytes(mv) + + +def test_element_codec_output_that_is_bytes_like_is_normalised(): + r = FakeRedis() + lst = RedisList(r, "l", encode=lambda s: bytearray(s.encode())) + lst.append("hi") + assert r.lindex("l", 0) == b"hi" + + +def test_wrapped_class_of_falls_back_to_the_init_closure(monkeypatch): + """Older dol versions don't set ``_wrapped_class`` etc.: the closure is read instead.""" + from redisdol.base import _wrapped_class_of + + W = KeyCodecs.prefixed("p:")(RedisPersister) + assert _wrapped_class_of(W)[0] is RedisPersister + for attr in ("_wrapped_class", "_wrapper_class", "_delegation_attr"): + monkeypatch.setattr(W, attr, None) + assert _wrapped_class_of(W) == (RedisPersister, Store) + s = store_from_source(FakeRedis(), W) + s["a"] = "1" + assert s["a"] == b"1" From 95a611332e6c20841cf986cfaba70c012bff539f Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:30:02 +0000 Subject: [PATCH 2/3] fix: _wrapped_class_of keeps refusing subclasses with their own __init__ and honours mixins in the MRO (review of #15) Co-Authored-By: Claude Sonnet 5.5 --- redisdol/base.py | 43 +++++++++++++++++++------------- tests/test_wrapping_fakeredis.py | 27 ++++++++++++++++++-- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/redisdol/base.py b/redisdol/base.py index 4c19f00..a526935 100644 --- a/redisdol/base.py +++ b/redisdol/base.py @@ -46,22 +46,29 @@ def from_sourced_object(cls, obj): def _wrapped_class_of(cls): - """``(wrapped class, delegator class)`` of a ``dol`` class-wrapper (``wrap_kvs``, - ``KeyCodecs...``, ``filt_iter``), read from the closure of the ``__init__`` dol - generates for it (``None`` if it has none, or delegates through another attribute - than ``store``). Reads the ``_wrapped_class`` / ``_wrapper_class`` / - ``_delegation_attr`` attributes dol sets on the wrapper (dol#131); falls back to the - closure of its generated ``__init__`` for older dol versions.""" - attrs = tuple( - getattr(cls, a, None) - for a in ("_wrapped_class", "_wrapper_class", "_delegation_attr") + """``(wrapped class, dol's generated wrapper class)`` of a ``dol`` class-wrapper + (``wrap_kvs``, ``KeyCodecs...``, ``filt_iter``), or ``None`` if ``cls`` is not one, + delegates through another attribute than ``store``, or has an ``__init__`` of its + own (a subclass of a wrapper: it must be built through that ``__init__``). + + Reads the ``_wrapped_class`` / ``_delegation_attr`` attributes dol sets on its + generated wrapper class (dol#131), falling back, for older dol versions, to the + closure of the ``__init__`` it generates. + """ + init = getattr(cls, "__init__", None) + # the generated wrapper class: the one that defines ``__init__`` and, in new dol, + # carries the attributes in its own namespace (a subclass only inherits them) + wrapper = next( + (c for c in cls.__mro__ if "_wrapped_class" in vars(c) and "__init__" in vars(c)), + None, ) - if all(a is not None for a in attrs): - wrapped, wrapper, attr = attrs - if isinstance(wrapped, type) and isinstance(wrapper, type) and attr == "store": + if wrapper is not None: + if vars(wrapper)["__init__"] is not init: + return None + wrapped, attr = wrapper._wrapped_class, wrapper._delegation_attr + if isinstance(wrapped, type) and attr == "store": return wrapped, wrapper return None - init = getattr(cls, "__init__", None) code, cells = getattr(init, "__code__", None), getattr(init, "__closure__", None) if code is None or not cells: return None @@ -76,7 +83,7 @@ def _wrapped_class_of(cls): return None if not (isinstance(wrapped, type) and isinstance(wrapper, type) and attr == "store"): return None - return wrapped, wrapper.__bases__[0] # the class dol's ``Wrap`` subclasses + return wrapped, wrapper def _new_with_source(cls, source): @@ -90,11 +97,11 @@ def _new_with_source(cls, source): "Store whose wrapped class can't be determined. Build the store over " "the connection first and wrap that instance instead." ) - wrapped, delegator = found + wrapped, wrapper = found self = object.__new__(cls) - # what dol's generated __init__ does after building the delegate: the delegator - # class's own __init__ (which may set state of its own), not Store's directly - delegator.__init__(self, _new_with_source(wrapped, source)) + # what dol's generated __init__ does after building the delegate: the next + # __init__ in the instance's MRO (which may set state of its own), not Store's + super(wrapper, self).__init__(_new_with_source(wrapped, source)) return self self = object.__new__(cls) self._source = source diff --git a/tests/test_wrapping_fakeredis.py b/tests/test_wrapping_fakeredis.py index 591365c..4ac2ea9 100644 --- a/tests/test_wrapping_fakeredis.py +++ b/tests/test_wrapping_fakeredis.py @@ -563,9 +563,32 @@ def test_wrapped_class_of_falls_back_to_the_init_closure(monkeypatch): W = KeyCodecs.prefixed("p:")(RedisPersister) assert _wrapped_class_of(W)[0] is RedisPersister + wrap = next(c for c in W.__mro__ if "_wrapped_class" in vars(c)) for attr in ("_wrapped_class", "_wrapper_class", "_delegation_attr"): - monkeypatch.setattr(W, attr, None) - assert _wrapped_class_of(W) == (RedisPersister, Store) + monkeypatch.delattr(wrap, attr) + assert _wrapped_class_of(W)[0] is RedisPersister s = store_from_source(FakeRedis(), W) s["a"] = "1" assert s["a"] == b"1" + + +def test_subclass_with_its_own_init_is_refused_and_a_mixin_in_the_mro_still_runs(): + W = KeyCodecs.prefixed("p:")(RedisPersister) + + class Y(W): # built through its own __init__, which we can't call without a Redis(**kw) + def __init__(self, *a, **k): + super().__init__(*a, **k) + self.extra = 1 + + with pytest.raises(TypeError, match="can't be determined"): + store_from_source(FakeRedis(), Y) + + class Mixin(Store): + def __init__(self, store): + super().__init__(store) + self.mixin_ran = True + + class X(W, Mixin): + pass + + assert store_from_source(FakeRedis(), X).mixin_ran is True From b670de31aebe5a5d5f5e9253c49c79ae860e8c60 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:31:22 +0000 Subject: [PATCH 3/3] test: fallback test works with dol versions that do not set the wrapper attributes (#15) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_wrapping_fakeredis.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_wrapping_fakeredis.py b/tests/test_wrapping_fakeredis.py index 4ac2ea9..a96e858 100644 --- a/tests/test_wrapping_fakeredis.py +++ b/tests/test_wrapping_fakeredis.py @@ -563,9 +563,10 @@ def test_wrapped_class_of_falls_back_to_the_init_closure(monkeypatch): W = KeyCodecs.prefixed("p:")(RedisPersister) assert _wrapped_class_of(W)[0] is RedisPersister - wrap = next(c for c in W.__mro__ if "_wrapped_class" in vars(c)) + wrap = next((c for c in W.__mro__ if "_wrapped_class" in vars(c)), None) for attr in ("_wrapped_class", "_wrapper_class", "_delegation_attr"): - monkeypatch.delattr(wrap, attr) + if wrap is not None: # newer dol sets them; with older dol this is already the fallback + monkeypatch.delattr(wrap, attr) assert _wrapped_class_of(W)[0] is RedisPersister s = store_from_source(FakeRedis(), W) s["a"] = "1"