Improvements to JSON coverageck-fixjson

tests to validate json behaviour

deprecation support

api changes

8 files changed, 120 insertions, 29 deletions

diff --git a/redis/commands/helpers.py b/redis/commands/helpers.py
index a92c025..48ee556 100644
--- a/redis/commands/helpers.py
+++ b/redis/commands/helpers.py
@@ -22,6 +22,8 @@ def nativestr(x):
 
 def delist(x):
     """Given a list of binaries, return the stringified version."""
+    if x is None:
+        return x
     return [nativestr(obj) for obj in x]
 
 
diff --git a/redis/commands/json/__init__.py b/redis/commands/json/__init__.py
index 9783705..3149bb8 100644
--- a/redis/commands/json/__init__.py
+++ b/redis/commands/json/__init__.py
@@ -1,5 +1,9 @@
 from json import JSONDecoder, JSONEncoder
 
+from .decoders import (
+    int_or_list,
+    int_or_none
+)
 from .helpers import bulk_of_jsons
 from ..helpers import nativestr, delist
 from .commands import JSONCommands
@@ -48,13 +52,13 @@ class JSON(JSONCommands):
             "JSON.ARRAPPEND": int,
             "JSON.ARRINDEX": int,
             "JSON.ARRINSERT": int,
-            "JSON.ARRLEN": int,
+            "JSON.ARRLEN": int_or_none,
             "JSON.ARRPOP": self._decode,
             "JSON.ARRTRIM": int,
             "JSON.OBJLEN": int,
             "JSON.OBJKEYS": delist,
             # "JSON.RESP": delist,
-            "JSON.DEBUG": int,
+            "JSON.DEBUG": int_or_list,
         }
 
         self.client = client
diff --git a/redis/commands/json/commands.py b/redis/commands/json/commands.py
index 2f8039f..d0b3d99 100644
--- a/redis/commands/json/commands.py
+++ b/redis/commands/json/commands.py
@@ -1,5 +1,7 @@
 from .path import Path, str_path
 from .helpers import decode_dict_keys
+from deprecated import deprecated
+from redis.exceptions import DataError
 
 
 class JSONCommands:
@@ -36,10 +38,6 @@ class JSONCommands:
             pieces.append(self._encode(o))
         return self.execute_command("JSON.ARRINSERT", *pieces)
 
-    def forget(self, name, path=Path.rootPath()):
-        """Alias for jsondel (delete the JSON value)."""
-        return self.execute_command("JSON.FORGET", name, str_path(path))
-
     def arrlen(self, name, path=Path.rootPath()):
         """Return the length of the array JSON value under ``path``
         at key``name``.
@@ -86,6 +84,7 @@ class JSONCommands:
             "JSON.NUMINCRBY", name, str_path(path), self._encode(number)
         )
 
+    @deprecated(version='4.0.0', reason='deprecated since redisjson 1.0.0')
     def nummultby(self, name, path, number):
         """Multiply the numeric (integer or floating point) JSON value under
         ``path`` at key ``name`` with the provided ``number``.
@@ -104,9 +103,12 @@ class JSONCommands:
         """
         return self.execute_command("JSON.CLEAR", name, str_path(path))
 
-    def delete(self, name, path=Path.rootPath()):
-        """Delete the JSON value stored at key ``name`` under ``path``."""
-        return self.execute_command("JSON.DEL", name, str_path(path))
+    def delete(self, key, path=Path.rootPath()):
+        """Delete the JSON value stored at key ``key`` under ``path``."""
+        return self.execute_command("JSON.DEL", key, str_path(path))
+
+    # forget is an alias for delete
+    forget = delete
 
     def get(self, name, *args, no_escape=False):
         """
@@ -134,12 +136,13 @@ class JSONCommands:
         except TypeError:
             return None
 
-    def mget(self, path, *args):
-        """Get the objects stored as a JSON values under ``path`` from keys
-        ``args``.
+    def mget(self, keys, path):
+        """
+        Get the objects stored as a JSON values under ``path``. ``keys``
+        is a list of one or more keys.
         """
         pieces = []
-        pieces.extend(args)
+        pieces += keys
         pieces.append(str_path(path))
         return self.execute_command("JSON.MGET", *pieces)
 
@@ -181,17 +184,33 @@ class JSONCommands:
         """
         return self.execute_command("JSON.TOGGLE", name, str_path(path))
 
-    def strappend(self, name, string, path=Path.rootPath()):
-        """Append to the string JSON value under ``path`` at key ``name``
-        the provided ``string``.
+    def strappend(self, name, *args):
+        """Append to the string JSON value. If two options are specified after
+        the key name, the path is determined to be the first. If a single
+        option is passed, then the rootpath (i.e Path.rootPath()) is used.
         """
+        pieces = [name]
+        if len(args) == 1:
+            pieces.append(Path.rootPath())
+        for a in args:
+            pieces.append(a)
+
         return self.execute_command(
-            "JSON.STRAPPEND", name, str_path(path), self._encode(string)
+            "JSON.STRAPPEND", *pieces
         )
 
-    def debug(self, name, path=Path.rootPath()):
+    def debug(self, subcommand, key=None, path=Path.rootPath()):
         """Return the memory usage in bytes of a value under ``path`` from
         key ``name``.
         """
-        return self.execute_command("JSON.DEBUG", "MEMORY",
-                                    name, str_path(path))
+        valid_subcommands = ["MEMORY", "HELP"]
+        if subcommand not in valid_subcommands:
+            raise DataError("The only valid subcommands are ",
+                            str(valid_subcommands))
+        pieces = [subcommand]
+        if subcommand == "MEMORY":
+            if key is None:
+                raise DataError("No key specified")
+            pieces.append(key)
+            pieces.append(str_path(path))
+        return self.execute_command("JSON.DEBUG", *pieces)
diff --git a/redis/commands/json/decoders.py b/redis/commands/json/decoders.py
new file mode 100644
index 0000000..0ee102a
--- /dev/null
+++ b/redis/commands/json/decoders.py
@@ -0,0 +1,12 @@
+def int_or_list(b):
+    if isinstance(b, int):
+        return b
+    else:
+        return b
+
+
+def int_or_none(b):
+    if b is None:
+        return None
+    if isinstance(b, int):
+        return b
diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 0000000..9f8d550
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1 @@
+deprecated
diff --git a/setup.py b/setup.py
index d0c81b4..6c712bd 100644
--- a/setup.py
+++ b/setup.py
@@ -23,6 +23,9 @@ setup(
     author="Redis Inc.",
     author_email="oss@redis.com",
     python_requires=">=3.6",
+    install_requires=[
+        'deprecated'
+    ],
     classifiers=[
         "Development Status :: 5 - Production/Stable",
         "Environment :: Console",
diff --git a/tests/test_json.py b/tests/test_json.py
index 83fbf28..a6205da 100644
--- a/tests/test_json.py
+++ b/tests/test_json.py
@@ -74,9 +74,9 @@ def test_jsonsetexistentialmodifiersshouldsucceed(client):
 def test_mgetshouldsucceed(client):
     client.json().set("1", Path.rootPath(), 1)
     client.json().set("2", Path.rootPath(), 2)
-    r = client.json().mget(Path.rootPath(), "1", "2")
-    e = [1, 2]
-    assert e == r
+    assert client.json().mget(["1"], Path.rootPath()) == [1]
+
+    assert client.json().mget([1, 2], Path.rootPath()) == [1, 2]
 
 
 @pytest.mark.redismod
@@ -90,6 +90,7 @@ def test_clearShouldSucceed(client):
 @pytest.mark.redismod
 def test_typeshouldsucceed(client):
     client.json().set("1", Path.rootPath(), 1)
+    assert b"integer" == client.json().type("1", Path.rootPath())
     assert b"integer" == client.json().type("1")
 
 
@@ -104,9 +105,11 @@ def test_numincrbyshouldsucceed(client):
 @pytest.mark.redismod
 def test_nummultbyshouldsucceed(client):
     client.json().set("num", Path.rootPath(), 1)
-    assert 2 == client.json().nummultby("num", Path.rootPath(), 2)
-    assert 5 == client.json().nummultby("num", Path.rootPath(), 2.5)
-    assert 2.5 == client.json().nummultby("num", Path.rootPath(), 0.5)
+
+    with pytest.deprecated_call():
+        assert 2 == client.json().nummultby("num", Path.rootPath(), 2)
+        assert 5 == client.json().nummultby("num", Path.rootPath(), 2.5)
+        assert 2.5 == client.json().nummultby("num", Path.rootPath(), 0.5)
 
 
 @pytest.mark.redismod
@@ -124,14 +127,21 @@ def test_toggleShouldSucceed(client):
 @pytest.mark.redismod
 def test_strappendshouldsucceed(client):
     client.json().set("str", Path.rootPath(), "foo")
-    assert 6 == client.json().strappend("str", "bar", Path.rootPath())
+    assert 6 == client.json().strappend("str", "bar")
     assert "foobar" == client.json().get("str", Path.rootPath())
 
+    client.json().set("str2", Path.rootPath(), "some")
+    assert 9 == client.json().strappend("str2", "value")
+
 
 @pytest.mark.redismod
 def test_debug(client):
     client.json().set("str", Path.rootPath(), "foo")
-    assert 24 == client.json().debug("str", Path.rootPath())
+    assert 24 == client.json().debug("MEMORY", "str", Path.rootPath())
+    assert 24 == client.json().debug("MEMORY", "str")
+
+    # technically help is valid
+    assert isinstance(client.json().debug("HELP"), list)
 
 
 @pytest.mark.redismod
@@ -140,6 +150,7 @@ def test_strlenshouldsucceed(client):
     assert 3 == client.json().strlen("str", Path.rootPath())
     client.json().strappend("str", "bar", Path.rootPath())
     assert 6 == client.json().strlen("str", Path.rootPath())
+    assert 6 == client.json().strlen("str")
 
 
 @pytest.mark.redismod
@@ -172,11 +183,18 @@ def test_arrinsertshouldsucceed(client):
     )
     assert [0, 1, 2, 3, 4] == client.json().get("arr")
 
+    # test prepends
+    client.json().set("val2", Path.rootPath(), [5, 6, 7, 8, 9])
+    client.json().arrinsert("val2", Path.rootPath(), 0, ['some', 'thing'])
+    assert client.json().get("val2") == [["some", "thing"], 5, 6, 7, 8, 9]
+
 
 @pytest.mark.redismod
 def test_arrlenshouldsucceed(client):
     client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4])
     assert 5 == client.json().arrlen("arr", Path.rootPath())
+    assert 5 == client.json().arrlen("arr")
+    assert client.json().arrlen('fakekey') is None
 
 
 @pytest.mark.redismod
@@ -188,6 +206,14 @@ def test_arrpopshouldsucceed(client):
     assert 0 == client.json().arrpop("arr", Path.rootPath(), 0)
     assert [1] == client.json().get("arr")
 
+    # test out of bounds
+    client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4])
+    assert 4 == client.json().arrpop("arr", Path.rootPath(), 99)
+
+    # none test
+    client.json().set("arr", Path.rootPath(), [])
+    assert client.json().arrpop("arr") is None
+
 
 @pytest.mark.redismod
 def test_arrtrimshouldsucceed(client):
@@ -195,6 +221,18 @@ def test_arrtrimshouldsucceed(client):
     assert 3 == client.json().arrtrim("arr", Path.rootPath(), 1, 3)
     assert [1, 2, 3] == client.json().get("arr")
 
+    # <0 test, should be 0 equivalent
+    client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4])
+    assert 4 == client.json().arrtrim("arr", Path.rootPath(), -1, 3)
+
+    # testing stop > end
+    client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4])
+    assert 3 == client.json().arrtrim("arr", Path.rootPath(), 4, 99)
+
+    # start > array size and stop
+    client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4])
+    assert [] == client.json().arrtrim("arr", Path.rootPath(), 9, 1)
+
 
 @pytest.mark.redismod
 def test_respshouldsucceed(client):
@@ -203,6 +241,7 @@ def test_respshouldsucceed(client):
     assert b"bar" == client.json().resp("obj", Path("foo"))
     assert 1 == client.json().resp("obj", Path("baz"))
     assert client.json().resp("obj", Path("qaz"))
+    assert isinstance(client.json().resp("obj"), list)
 
 
 @pytest.mark.redismod
@@ -215,6 +254,12 @@ def test_objkeysshouldsucceed(client):
     exp.sort()
     assert exp == keys
 
+    client.json().set("obj", Path.rootPath(), obj)
+    keys = client.json().objkeys("obj")
+    assert keys == list(obj.keys())
+
+    assert client.json().objkeys("fakekey") is None
+
 
 @pytest.mark.redismod
 def test_objlenshouldsucceed(client):
@@ -222,6 +267,9 @@ def test_objlenshouldsucceed(client):
     client.json().set("obj", Path.rootPath(), obj)
     assert len(obj) == client.json().objlen("obj", Path.rootPath())
 
+    client.json().set("obj", Path.rootPath(), obj)
+    assert len(obj) == client.json().objlen("obj")
+
 
 # @pytest.mark.pipeline
 # @pytest.mark.redismod
diff --git a/tox.ini b/tox.ini
index 211f69e..f09b3a8 100644
--- a/tox.ini
+++ b/tox.ini
@@ -75,7 +75,9 @@ volumes =
     bind:rw:{toxinidir}:/data
 
 [testenv]
-deps = -r {toxinidir}/dev_requirements.txt
+deps =
+    -r {toxinidir}/requirements.txt
+    -r {toxinidir}/dev_requirements.txt
 docker =
     master
     replica