Skip to content

Commit

Permalink
Dramatically simplify RedisList.__eq__() (#573)
Browse files Browse the repository at this point in the history
* Dramatically simplify RedisList.__eq__()

This makes the code easier to reason about, and prevents potential bugs.

* Warn when doing expensive equality comparisons

* Bump version number

* Make RedisList.__eq__() more performant

* Don't allow RedisDeques to equal RedisLists...

...even if they contain the same elements.

```python
>>> import collections
>>> collections.deque([1, 2, 3]) == [1, 2, 3]
False
```
  • Loading branch information
brainix authored Dec 31, 2021
1 parent d045ca8 commit ccfe435
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 45 deletions.
2 changes: 1 addition & 1 deletion pottery/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@


__title__: Final[str] = 'pottery'
__version__: Final[str] = '2.3.2'
__version__: Final[str] = '2.3.3'
__description__: Final[str] = __doc__.split(sep='\n\n', maxsplit=1)[0]
__url__: Final[str] = 'https://github.com/brainix/pottery'
__author__: Final[str] = 'Rajiv Bakulesh Shah'
Expand Down
7 changes: 6 additions & 1 deletion pottery/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import logging
import os
import uuid
import warnings
from typing import Any
from typing import AnyStr
from typing import ClassVar
Expand All @@ -48,6 +49,7 @@

from . import monkey
from .annotations import JSONTypes
from .exceptions import InefficientAccessWarning
from .exceptions import QuorumIsImpossible
from .exceptions import RandomKeyError

Expand Down Expand Up @@ -272,10 +274,13 @@ def _same_redis(self, *others: Any) -> bool:
def __eq__(self, other: Any) -> bool:
if self is other:
return True

if self._same_redis(other) and self.key == other.key:
return True

warnings.warn(
cast(str, InefficientAccessWarning.__doc__),
InefficientAccessWarning,
)
with self._watch(other):
equals = super().__eq__(other)
if equals is NotImplemented:
Expand Down
60 changes: 17 additions & 43 deletions pottery/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,54 +258,28 @@ def sort(self, *, key: Optional[str] = None, reverse: bool = False) -> None:
def __eq__(self, other: Any) -> bool:
if self is other:
return True

if self._same_redis(other) and self.key == other.key:
# self and other are both RedisLists on the same Redis database and
# with the same key. No need to compare element by element.
return True

with self._watch(other) as pipeline:
len_xs = cast(int, pipeline.llen(self.key))
if isinstance(other, RedisList) and self._same_redis(other):
len_ys = cast(int, pipeline.llen(other.key))
else:
try:
len_ys = len(other)
except TypeError:
# TypeError: other has no len()
return False
if len_xs != len_ys:
# self and other are different lengths.
if type(other) not in {list, self.__class__}:
return False
with self._watch(other):
if len(self) != len(other):
return False

if isinstance(other, collections.abc.MutableSequence):
# self and other are the same length, and other is a mutable
# sequence too. Compare self's and other's elements, pair by
# pair.
warnings.warn(
cast(str, InefficientAccessWarning.__doc__),
InefficientAccessWarning,
)
encoded_xs = cast(
List[bytes],
pipeline.lrange(self.key, 0, -1),
)
decoded_xs = (self._decode(x) for x in encoded_xs)
if isinstance(other, RedisList) and self._same_redis(other):
encoded_ys = cast(
List[bytes],
pipeline.lrange(other.key, 0, -1),
)
decoded_ys: collections.abc.Iterable[Any] = ( # pragma: no cover
self._decode(y) for y in encoded_ys
)
else:
decoded_ys = other
return all(x == y for x, y in zip(decoded_xs, decoded_ys))

# self and other are the same length, but other is an unordered
# collection.
return False
# other is a mutable sequence too, and self and other are the same
# length. Make Python lists out of self and other, and compare
# those lists.
warnings.warn(
cast(str, InefficientAccessWarning.__doc__),
InefficientAccessWarning,
)
self_as_list = self.__to_list()
try:
other_as_list = cast('RedisList', other).to_list()
except AttributeError:
other_as_list = list(other)
return self_as_list == other_as_list

def __add__(self, other: List[JSONTypes]) -> RedisList:
'Append the items in other to the RedisList. O(n)'
Expand Down

0 comments on commit ccfe435

Please sign in to comment.