Fix json mget when dict is returned

[Pichlerdom]

Fix AssertionError in _run_command when mget returns a dict within the list of results.

Can be reproduced by running this script. I would expect the output to be [[5, 8, {"b": "nested3"}]].

r = fakeredis.FakeStrictRedis(version=7)
r.json().set(
    "doc3", "$", {
        "a": 5,
        "b": 5,
        "nested": {
            "a": 8
        },
        "c": None,
        "nested2": {
            "a": {
                "b": "nested3"
            }
        }
    })

print(r.json().mget(["doc3"], "$..a"))

This is my first PR so please don't be to harsh. :)

[cunla]

Hi @Pichlerdom, thanks for this PR! This is a good catch.

One thing I would want to change in this PR:
instead of calling JSON.encode for every element in the list, call it once before returning. I am certain it will have the same result with better performance (calling method once instead of N times).

So it would look like:

...
result = [self._get_single(key, path_str, empty_list_as_none=True) for key in keys]
return JSONObject.encode(result)

[Pichlerdom]

Hi @cunla.
Thanks you for the feedback.
I will try calling JSONObject.encode() only once on return. Should have thought of that myself...

I have two question on how to proceed with this PR now.
Should i commit the changes on top of the commit in this PR or should i amend the commit?
Can i reopen the PR or is it better to submit a new one?

Thanks for your help in advance and awesome that you responded in such a nice and clear way... It's realy good for beginners like me trying to learn.

[cunla]

I have two question on how to proceed with this PR now. Should i commit the changes on top of the commit in this PR or should i amend the commit?

When merging we can use Squash and merge, so it does not really matter if you prefer to commit on top or amend. I personally prefer to commit on top (in my opinion it is easier).

Can i reopen the PR or is it better to submit a new one?

No need to reopen/submit a new one. When you push a new commit to your branch (or amend an existing commit) it will be added automatically to the PR.

See some of the closed PRs in this repo, some of them have 20 commits :) (example)

Thanks for your help in advance and awesome that you responded in such a nice and clear way... It's realy good for beginners like me trying to learn.

No problem! Thank you for contributing!

[Pichlerdom]

I have tried the change you suggested but i don't think it will work.

I get this error msg:

_________________________ test_jsonmget[FakeStrictRedis] __________________________

r = FakeStrictRedis<ConnectionPool<FakeConnection<server=<fakeredis._server.FakeServer object at 0x7f2a545aeb30>,db=0>>>

    def test_jsonmget(r: redis.Redis):
        # Test mget with multi paths
        r.json().set("doc1", "$", {"a": 1, "b": 2, "nested": {"a": 3}, "c": None, "nested2": {"a": None}})
        r.json().set("doc2", "$", {"a": 4, "b": 5, "nested": {"a": 6}, "c": None, "nested2": {"a": [None]}})
        r.json().set("doc3", "$", {"a": 5, "b": 5, "nested": {"a": 8}, "c": None, "nested2": {"a": {"b": "nested3"}}})
        # Compare also to single JSON.GET
        assert r.json().get("doc1", Path("$..a")) == [1, 3, None]
        assert r.json().get("doc2", "$..a") == [4, 6, [None]]
        assert r.json().get("doc3", "$..a") == [5, 8, {"b": "nested3"}]
    
        # Test mget with single path
>       assert r.json().mget(["doc1"], "$..a") == [[1, 3, None]]

test/test_json/test_json.py:146: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/commands/json/commands.py:211: in mget
    return self.execute_command("JSON.MGET", *pieces)
../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/client.py:1258: in execute_command
    return conn.retry.call_with_retry(
../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/retry.py:46: in call_with_retry
    return do()
../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/client.py:1259: in <lambda>
    lambda: self._send_command_parse_response(
../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/client.py:1235: in _send_command_parse_response
    return self.parse_response(conn, command_name, **options)
../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/client.py:1285: in parse_response
    return self.response_callbacks[command_name](response, **options)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

b = b'[[1, 3, null]]'

    def _f(b):
        for index, item in enumerate(b):
            if item is not None:
>               b[index] = d(item)
E               TypeError: 'bytes' object does not support item assignment

../../.virtualenvs/fakeredis-test-py310/lib/python3.10/site-packages/redis/commands/json/decoders.py:15: TypeError

Which suggests to me that redis expects a list of byte encoded json objects in "redis/commands/json/decoders.py" bulk_of_jsons.
But i am not to sure about that.

[cunla]

Interesting... Ok, in that case, let's merge it as you originally proposed.

[Pichlerdom]

Nice.
Thanks for the help :D