Make RedisList/RedisDeque .insert() use pipelines (#484)

* Make RedisList/RedisDeque .insert() use pipelines

This avoids race conditions.

* Fix inserting into RedisList w/ repeated elements

* Bump version number

* Increase unit test coverage

* Use pipeline to get len in RedisDeque.__extend()

* Refactor RedisList.insert() and RedisDeque.insert()

Don't repeat myself.

* Delete redundant unit tests

pottery/__init__.py

@@ -30,7 +30,7 @@
 
 
 __title__ = 'pottery'
-__version__ = '1.4.6'
+__version__ = '1.4.7'
 __description__ = __doc__.split(sep='\n\n', maxsplit=1)[0]
 __url__ = 'https://github.com/brainix/pottery'
 __author__ = 'Rajiv Bakulesh Shah'

pottery/deque.py

@@ -21,6 +21,7 @@
 from typing import Optional
 from typing import Tuple
 from typing import Union
+from typing import cast
 
 from redis import Redis
 from redis.client import Pipeline
@@ -59,6 +60,8 @@ def _populate(self,
             if self.maxlen:
                 iterable = tuple(iterable)[-self.maxlen:]
             else:
+                # self.maxlen == 0.  Populate the RedisDeque with an empty
+                # iterable.
                 iterable = tuple()
         super()._populate(pipeline, iterable)
 
@@ -75,11 +78,13 @@ def maxlen(self, value: int) -> None:
 
     def insert(self, index: int, value: JSONTypes) -> None:
         'Insert an element into the RedisDeque before the given index.  O(n)'
-        if self.maxlen is not None and len(self) >= self.maxlen:
-            raise IndexError(
-                f'{self.__class__.__name__} already at its maximum size'
-            )
-        return super()._insert(index, value)
+        with self._watch() as pipeline:
+            current_length = cast(int, pipeline.llen(self.key))
+            if self.maxlen is not None and current_length >= self.maxlen:
+                raise IndexError(
+                    f'{self.__class__.__name__} already at its maximum size'
+                )
+            super()._insert(index, value, pipeline=pipeline)
 
     def append(self, value: JSONTypes) -> None:
         'Add an element to the right side of the RedisDeque.  O(1)'
@@ -113,7 +118,7 @@ def __extend(self,
         with self._watch(values) as pipeline:
             push_method = 'rpush' if right else 'lpush'
             encoded_values = [self._encode(value) for value in values]
-            len_ = len(self) + len(encoded_values)
+            len_ = cast(int, pipeline.llen(self.key)) + len(encoded_values)
             trim_indices: Union[Tuple[int, int], Tuple] = tuple()
             if self.maxlen is not None and len_ >= self.maxlen:
                 trim_indices = (len_-self.maxlen, len_) if right else (0, self.maxlen-1)

pottery/list.py

@@ -19,6 +19,7 @@
 import collections.abc
 import functools
 import itertools
+import uuid
 from typing import Any
 from typing import Callable
 from typing import Iterable
@@ -144,6 +145,9 @@ def __delitem__(self, index: Union[slice, int]) -> None:  # type: ignore
         with self._watch() as pipeline:
             self.__delete(pipeline, index)
 
+    # Preserve the Open-Closed Principle with name mangling.
+    #   https://youtu.be/miGolgp9xq8?t=2086
+    #   https://stackoverflow.com/a/38534939
     def __delete(self, pipeline: Pipeline, index: Union[slice, int]) -> None:
         # Python's list API requires us to delete an element by *index.*  Redis
         # supports only deleting an element by *value.*  So our workaround is
@@ -162,33 +166,39 @@ def __len__(self) -> int:
         return self.redis.llen(self.key)
 
     def insert(self, index: int, value: JSONTypes) -> None:
-        'Insert an element into the RedisList before the given index.  O(n)'
-        self.__insert(index, value)
-
-    def _insert(self, index: int, value: JSONTypes) -> None:
+        'Insert an element into the RedisDeque before the given index.  O(n)'
+        with self._watch() as pipeline:
+            self.__insert(index, value, pipeline=pipeline)
+
+    def _insert(self,
+                index: int,
+                value: JSONTypes,
+                *,
+                pipeline: Pipeline,
+                ) -> None:
         encoded_value = self._encode(value)
+        current_length = cast(int, pipeline.llen(self.key))
         if index <= 0:
-            self.redis.lpush(self.key, encoded_value)
-        elif index < len(self):
+            pipeline.multi()
+            pipeline.lpush(self.key, encoded_value)
+        elif index < current_length:
             # Python's list API requires us to insert an element before the
             # given *index.*  Redis supports only inserting an element before a
             # given (pivot) *value.*  So our workaround is to set the pivot
-            # value to 0, then to insert the desired value before the value 0,
-            # then to set the value 0 back to the original pivot value.  More
-            # info:
+            # value to a UUID4, then to insert the desired value before the
+            # UUID4, then to set the value UUID4 back to the original pivot
+            # pivot value.  More info:
             #   http://redis.io/commands/linsert
-            with self._watch() as pipeline:
-                pivot = self._encode(self[index])
-                pipeline.multi()
-                pipeline.lset(self.key, index, 0)
-                pipeline.linsert(self.key, 'BEFORE', 0, encoded_value)
-                pipeline.lset(self.key, index+1, pivot)
+            pivot = cast(bytes, pipeline.lindex(self.key, index))
+            pipeline.multi()
+            uuid4 = str(uuid.uuid4())
+            pipeline.lset(self.key, index, uuid4)
+            pipeline.linsert(self.key, 'BEFORE', uuid4, encoded_value)
+            pipeline.lset(self.key, index+1, pivot)
         else:
-            self.redis.rpush(self.key, encoded_value)
+            pipeline.multi()
+            pipeline.rpush(self.key, encoded_value)
 
-    # Preserve the Open-Closed Principle with name mangling.
-    #   https://youtu.be/miGolgp9xq8?t=2086
-    #   https://stackoverflow.com/a/38534939
     __insert = _insert
 
     # Methods required for Raj's sanity:

tests/test_list.py

@@ -140,6 +140,17 @@ def test_insert_left(self):
         squares.insert(0, 1)
         assert squares == [1, 4, 9, 16, 25]
 
+    def test_insert_middle(self):
+        nums = RedisList([0, 0, 0, 0], redis=self.redis)
+        nums.insert(2, 2)
+        assert nums == [0, 0, 2, 0, 0]
+
+    def test_insert_right(self):
+        squares = RedisList([1, 4, 9], redis=self.redis)
+        squares.insert(100, 16)
+        squares.insert(100, 25)
+        assert squares == [1, 4, 9, 16, 25]
+
     def test_extend(self):
         squares = RedisList([1, 4, 9], redis=self.redis)
         squares.extend([16, 25])