From 75c5d5974ed3f6a5069f601f4c0d42ad3ae48c76 Mon Sep 17 00:00:00 2001 From: Chayim Date: Tue, 2 Nov 2021 11:55:07 +0200 Subject: Improved JSON accuracy (#1666) --- redis/commands/helpers.py | 2 + redis/commands/json/__init__.py | 8 ++- redis/commands/json/commands.py | 93 ++++++++++++++++++++-------------- redis/commands/json/decoders.py | 12 +++++ redis/commands/json/path.py | 11 ++--- requirements.txt | 1 + setup.py | 3 ++ tests/test_json.py | 107 ++++++++++++++++++++++++++++++---------- tox.ini | 4 +- 9 files changed, 165 insertions(+), 76 deletions(-) create mode 100644 redis/commands/json/decoders.py create mode 100644 requirements.txt 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..fb00e22 100644 --- a/redis/commands/json/commands.py +++ b/redis/commands/json/commands.py @@ -1,5 +1,7 @@ -from .path import Path, str_path +from .path import Path from .helpers import decode_dict_keys +from deprecated import deprecated +from redis.exceptions import DataError class JSONCommands: @@ -9,7 +11,7 @@ class JSONCommands: """Append the objects ``args`` to the array under the ``path` in key ``name``. """ - pieces = [name, str_path(path)] + pieces = [name, str(path)] for o in args: pieces.append(self._encode(o)) return self.execute_command("JSON.ARRAPPEND", *pieces) @@ -23,7 +25,7 @@ class JSONCommands: and exclusive ``stop`` indices. """ return self.execute_command( - "JSON.ARRINDEX", name, str_path(path), self._encode(scalar), + "JSON.ARRINDEX", name, str(path), self._encode(scalar), start, stop ) @@ -31,67 +33,64 @@ class JSONCommands: """Insert the objects ``args`` to the array at index ``index`` under the ``path` in key ``name``. """ - pieces = [name, str_path(path), index] + pieces = [name, str(path), index] for o in args: 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``. """ - return self.execute_command("JSON.ARRLEN", name, str_path(path)) + return self.execute_command("JSON.ARRLEN", name, str(path)) def arrpop(self, name, path=Path.rootPath(), index=-1): """Pop the element at ``index`` in the array JSON value under ``path`` at key ``name``. """ - return self.execute_command("JSON.ARRPOP", name, str_path(path), index) + return self.execute_command("JSON.ARRPOP", name, str(path), index) def arrtrim(self, name, path, start, stop): """Trim the array JSON value under ``path`` at key ``name`` to the inclusive range given by ``start`` and ``stop``. """ - return self.execute_command("JSON.ARRTRIM", name, str_path(path), + return self.execute_command("JSON.ARRTRIM", name, str(path), start, stop) def type(self, name, path=Path.rootPath()): """Get the type of the JSON value under ``path`` from key ``name``.""" - return self.execute_command("JSON.TYPE", name, str_path(path)) + return self.execute_command("JSON.TYPE", name, str(path)) def resp(self, name, path=Path.rootPath()): """Return the JSON value under ``path`` at key ``name``.""" - return self.execute_command("JSON.RESP", name, str_path(path)) + return self.execute_command("JSON.RESP", name, str(path)) def objkeys(self, name, path=Path.rootPath()): """Return the key names in the dictionary JSON value under ``path`` at key ``name``.""" - return self.execute_command("JSON.OBJKEYS", name, str_path(path)) + return self.execute_command("JSON.OBJKEYS", name, str(path)) def objlen(self, name, path=Path.rootPath()): """Return the length of the dictionary JSON value under ``path`` at key ``name``. """ - return self.execute_command("JSON.OBJLEN", name, str_path(path)) + return self.execute_command("JSON.OBJLEN", name, str(path)) def numincrby(self, name, path, number): """Increment the numeric (integer or floating point) JSON value under ``path`` at key ``name`` by the provided ``number``. """ return self.execute_command( - "JSON.NUMINCRBY", name, str_path(path), self._encode(number) + "JSON.NUMINCRBY", name, str(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``. """ return self.execute_command( - "JSON.NUMMULTBY", name, str_path(path), self._encode(number) + "JSON.NUMMULTBY", name, str(path), self._encode(number) ) def clear(self, name, path=Path.rootPath()): @@ -102,11 +101,14 @@ class JSONCommands: Return the count of cleared paths (ignoring non-array and non-objects paths). """ - return self.execute_command("JSON.CLEAR", name, str_path(path)) + return self.execute_command("JSON.CLEAR", name, str(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)) - 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)) + # forget is an alias for delete + forget = delete def get(self, name, *args, no_escape=False): """ @@ -125,7 +127,7 @@ class JSONCommands: else: for p in args: - pieces.append(str_path(p)) + pieces.append(str(p)) # Handle case where key doesn't exist. The JSONDecoder would raise a # TypeError exception since it can't decode None @@ -134,13 +136,14 @@ 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.append(str_path(path)) + pieces += keys + pieces.append(str(path)) return self.execute_command("JSON.MGET", *pieces) def set(self, name, path, obj, nx=False, xx=False, decode_keys=False): @@ -155,7 +158,7 @@ class JSONCommands: if decode_keys: obj = decode_dict_keys(obj) - pieces = [name, str_path(path), self._encode(obj)] + pieces = [name, str(path), self._encode(obj)] # Handle existential modifiers if nx and xx: @@ -169,29 +172,43 @@ class JSONCommands: pieces.append("XX") return self.execute_command("JSON.SET", *pieces) - def strlen(self, name, path=Path.rootPath()): + def strlen(self, name, path=None): """Return the length of the string JSON value under ``path`` at key ``name``. """ - return self.execute_command("JSON.STRLEN", name, str_path(path)) + pieces = [name] + if path is not None: + pieces.append(str(path)) + return self.execute_command("JSON.STRLEN", *pieces) def toggle(self, name, path=Path.rootPath()): """Toggle boolean value under ``path`` at key ``name``. returning the new value. """ - return self.execute_command("JSON.TOGGLE", name, str_path(path)) + return self.execute_command("JSON.TOGGLE", name, str(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, value, path=Path.rootPath()): + """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, str(path), value] 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)) + 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/redis/commands/json/path.py b/redis/commands/json/path.py index dff8648..6d87045 100644 --- a/redis/commands/json/path.py +++ b/redis/commands/json/path.py @@ -1,11 +1,3 @@ -def str_path(p): - """Return the string representation of a path if it is of class Path.""" - if isinstance(p, Path): - return p.strPath - else: - return p - - class Path(object): """This class represents a path in a JSON value.""" @@ -19,3 +11,6 @@ class Path(object): def __init__(self, path): """Make a new path based on the string representation in `path`.""" self.strPath = path + + def __repr__(self): + return self.strPath 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..f62346f 100644 --- a/tests/test_json.py +++ b/tests/test_json.py @@ -74,27 +74,28 @@ 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 @skip_ifmodversion_lt("99.99.99", "ReJSON") # todo: update after the release -def test_clearShouldSucceed(client): +def test_clear(client): client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) assert 1 == client.json().clear("arr", Path.rootPath()) assert [] == client.json().get("arr") @pytest.mark.redismod -def test_typeshouldsucceed(client): +def test_type(client): client.json().set("1", Path.rootPath(), 1) + assert b"integer" == client.json().type("1", Path.rootPath()) assert b"integer" == client.json().type("1") @pytest.mark.redismod -def test_numincrbyshouldsucceed(client): +def test_numincrby(client): client.json().set("num", Path.rootPath(), 1) assert 2 == client.json().numincrby("num", Path.rootPath(), 1) assert 2.5 == client.json().numincrby("num", Path.rootPath(), 0.5) @@ -102,16 +103,18 @@ def test_numincrbyshouldsucceed(client): @pytest.mark.redismod -def test_nummultbyshouldsucceed(client): +def test_nummultby(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 @skip_ifmodversion_lt("99.99.99", "ReJSON") # todo: update after the release -def test_toggleShouldSucceed(client): +def test_toggle(client): client.json().set("bool", Path.rootPath(), False) assert client.json().toggle("bool", Path.rootPath()) assert not client.json().toggle("bool", Path.rootPath()) @@ -122,28 +125,37 @@ 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 "foobar" == client.json().get("str", Path.rootPath()) +def test_strappend(client): + client.json().set("jsonkey", Path.rootPath(), 'foo') + import json + assert 6 == client.json().strappend("jsonkey", json.dumps('bar')) + with pytest.raises(redis.exceptions.ResponseError): + assert 6 == client.json().strappend("jsonkey", 'bar') + assert "foobar" == client.json().get("jsonkey", Path.rootPath()) @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 -def test_strlenshouldsucceed(client): +def test_strlen(client): client.json().set("str", Path.rootPath(), "foo") assert 3 == client.json().strlen("str", Path.rootPath()) - client.json().strappend("str", "bar", Path.rootPath()) + import json + client.json().strappend("str", json.dumps("bar"), Path.rootPath()) assert 6 == client.json().strlen("str", Path.rootPath()) + assert 6 == client.json().strlen("str") @pytest.mark.redismod -def test_arrappendshouldsucceed(client): +def test_arrappend(client): client.json().set("arr", Path.rootPath(), [1]) assert 2 == client.json().arrappend("arr", Path.rootPath(), 2) assert 4 == client.json().arrappend("arr", Path.rootPath(), 3, 4) @@ -151,14 +163,14 @@ def test_arrappendshouldsucceed(client): @pytest.mark.redismod -def testArrIndexShouldSucceed(client): +def test_arrindex(client): client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) assert 1 == client.json().arrindex("arr", Path.rootPath(), 1) assert -1 == client.json().arrindex("arr", Path.rootPath(), 1, 2) @pytest.mark.redismod -def test_arrinsertshouldsucceed(client): +def test_arrinsert(client): client.json().set("arr", Path.rootPath(), [0, 4]) assert 5 - -client.json().arrinsert( "arr", @@ -172,15 +184,22 @@ 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): +def test_arrlen(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 -def test_arrpopshouldsucceed(client): +def test_arrpop(client): client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) assert 4 == client.json().arrpop("arr", Path.rootPath(), 4) assert 3 == client.json().arrpop("arr", Path.rootPath(), -1) @@ -188,25 +207,50 @@ 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): +def test_arrtrim(client): client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) 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 0 == client.json().arrtrim("arr", Path.rootPath(), -1, 3) + + # testing stop > end + client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) + assert 2 == client.json().arrtrim("arr", Path.rootPath(), 3, 99) + + # start > array size and stop + client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) + assert 0 == client.json().arrtrim("arr", Path.rootPath(), 9, 1) + + # all larger + client.json().set("arr", Path.rootPath(), [0, 1, 2, 3, 4]) + assert 0 == client.json().arrtrim("arr", Path.rootPath(), 9, 11) + @pytest.mark.redismod -def test_respshouldsucceed(client): +def test_resp(client): obj = {"foo": "bar", "baz": 1, "qaz": True} client.json().set("obj", Path.rootPath(), obj) 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 -def test_objkeysshouldsucceed(client): +def test_objkeys(client): obj = {"foo": "bar", "baz": "qaz"} client.json().set("obj", Path.rootPath(), obj) keys = client.json().objkeys("obj", Path.rootPath()) @@ -215,13 +259,22 @@ 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): +def test_objlen(client): obj = {"foo": "bar", "baz": "qaz"} 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 -- cgit v1.2.1