Skip to content

Commit

Permalink
Don't allow a RedisDeque to equal a RedisList... (#576)
Browse files Browse the repository at this point in the history
* Make RedisList.__eq__() more clear

* Make RedisList.__eq__() more clear

* Don't allow a RedisDeque to equal a RedisList...

...even if they're both on the same Redis instance and have the same
key.

Before this PR:

```python
>>> from pottery import RedisDeque, RedisList
>>> RedisDeque(key='videos:dicts') == RedisList(key='videos:dicts')
True
```

As of this PR:

```python
>>> from pottery import RedisDeque, RedisList
>>> RedisDeque(key='videos:dicts') == RedisList(key='videos:dicts')
False
```

* Reorder logic in RedisList.__eq__()

* Bump version number

* Unit test RedisList equality with RedisDeque

* Unit test RedisDeque equality with RedisList

* Name unit tests more accurately
  • Loading branch information
brainix authored Dec 31, 2021
1 parent 856616b commit 97ad704
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 9 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.4'
__version__: Final[str] = '2.3.5'
__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
14 changes: 9 additions & 5 deletions pottery/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
from redis import Redis
from redis import ResponseError
from redis.client import Pipeline
# TODO: When we drop support for Python 3.7, change the following import to:
# from typing import final
from typing_extensions import final

from .annotations import F
from .base import Base
Expand Down Expand Up @@ -258,13 +261,13 @@ def sort(self, *, key: Optional[str] = None, reverse: bool = False) -> None:
self.redis.sort(self.key, desc=reverse, store=self.key)

def __eq__(self, other: Any) -> bool:
if type(other) not in {self.__class__, self._ALLOWED_TO_EQUAL}:
return False
if self is other:
return True
if self._same_redis(other) and self.key == other.key:
return True

if type(other) not in {self.__class__, self._ALLOWED_TO_EQUAL}:
return False
with self._watch(other):
if len(self) != len(other):
return False
Expand All @@ -277,9 +280,9 @@ def __eq__(self, other: Any) -> bool:
InefficientAccessWarning,
)
self_as_list = self.__to_list()
try:
other_as_list = cast('RedisList', other).to_list()
except AttributeError:
if isinstance(other, self.__class__):
other_as_list = other.to_list()
else:
other_as_list = list(other)
return self_as_list == other_as_list

Expand Down Expand Up @@ -352,6 +355,7 @@ def remove(self, value: JSONTypes) -> None:
class_name = self.__class__.__name__
raise ValueError(f'{class_name}.remove(x): x not in {class_name}')

@final
def to_list(self) -> List[JSONTypes]:
'Convert the RedisList to a Python list. O(n)'
warnings.warn(
Expand Down
9 changes: 8 additions & 1 deletion pottery/nextid.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@
'''


# TODO: Remove the following import after deferred evaluation of annotations
# because the default.
# 1. https://docs.python.org/3/whatsnew/3.7.html#whatsnew37-pep563
# 2. https://www.python.org/dev/peps/pep-0563/
# 3. https://www.python.org/dev/peps/pep-0649/
from __future__ import annotations

import concurrent.futures
import contextlib
from typing import ClassVar
Expand Down Expand Up @@ -147,7 +154,7 @@ def __init__(self,
)
self.num_tries = num_tries

def __iter__(self) -> 'NextId':
def __iter__(self) -> NextId:
return self

def __next__(self) -> int:
Expand Down
9 changes: 8 additions & 1 deletion pottery/redlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@
'''


# TODO: Remove the following import after deferred evaluation of annotations
# because the default.
# 1. https://docs.python.org/3/whatsnew/3.7.html#whatsnew37-pep563
# 2. https://www.python.org/dev/peps/pep-0563/
# 3. https://www.python.org/dev/peps/pep-0649/
from __future__ import annotations

import concurrent.futures
import contextlib
import functools
Expand Down Expand Up @@ -584,7 +591,7 @@ def release(self, *, raise_on_redis_errors: Optional[bool] = None) -> None:

__release = release

def __enter__(self) -> 'Redlock':
def __enter__(self) -> Redlock:
'''You can use a Redlock as a context manager.
Usage:
Expand Down
9 changes: 8 additions & 1 deletion pottery/timer.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
'Measure the execution time of small code snippets.'


# TODO: Remove the following import after deferred evaluation of annotations
# because the default.
# 1. https://docs.python.org/3/whatsnew/3.7.html#whatsnew37-pep563
# 2. https://www.python.org/dev/peps/pep-0563/
# 3. https://www.python.org/dev/peps/pep-0649/
from __future__ import annotations

import timeit
from types import TracebackType
from typing import Optional
Expand Down Expand Up @@ -63,7 +70,7 @@ def __init__(self) -> None:
self._started = 0.0
self._stopped = 0.0

def __enter__(self) -> 'ContextTimer':
def __enter__(self) -> ContextTimer:
self.__start()
return self

Expand Down
7 changes: 7 additions & 0 deletions tests/test_deque.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import unittest.mock

from pottery import RedisDeque
from pottery import RedisList
from pottery.base import Base
from tests.base import TestCase

Expand Down Expand Up @@ -198,3 +199,9 @@ def test_repr(self):

d = RedisDeque('ghi', redis=self.redis, maxlen=2)
assert repr(d) == "RedisDeque(['h', 'i'], maxlen=2)"

def test_eq_redislist_same_redis_key(self):
deque = RedisDeque('ghi', redis=self.redis)
list_ = RedisList(redis=self.redis, key=deque.key)
assert not deque == list_
assert deque != list_
7 changes: 7 additions & 0 deletions tests/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import json

from pottery import KeyExistsError
from pottery import RedisDeque
from pottery import RedisList
from tests.base import TestCase

Expand Down Expand Up @@ -167,6 +168,12 @@ def test_sort(self):
with self.assertRaises(NotImplementedError):
squares.sort(key=str)

def test_eq_redisdeque_same_redis_key(self):
list_ = RedisList([1, 4, 9, 16, 25], redis=self.redis, key=self._KEY)
deque = RedisDeque(redis=self.redis, key=self._KEY)
assert not list_ == deque
assert list_ != deque

def test_eq_same_object(self):
squares = RedisList([1, 4, 9, 16, 25], redis=self.redis, key=self._KEY)
assert squares == squares
Expand Down

0 comments on commit 97ad704

Please sign in to comment.