Skip to content

Commit fd0b0d3

Browse files
gerzsegerzse
authored
Handle RESP3 sets as Python lists (#3324)
* Handle RESP3 sets as Python lists Although the RESP3 protocol defines the set data structure, sometimes the responses from the Redis server contain sets with nested maps, which cannot be represented in Python as sets with nested dicts, because dicts are not hashable. Versions of HIREDIS before 3.0.0 would cause segmentation fault when parsing such responses. Starting with version 3.0.0 the problem was fixed, with the compromise that RESP3 sets are represented as Python lists. The embedded RESP3 parser was so far trying to represent RESP3 sets as Python sets, if possible. Only when this was not possible it would switch to the list representation. Arguably this is not the best user experience, not knowing when you will get back a set or a list. Upgrade the required hiredis-py version to be at least 3.0.0, and change the embedded parser to always represent RESP3 sets as lists. This way we get a consistent experience in all cases. This is a breaking change. * Also cover RESP2 sets * Fix failing tests * Fix async RESP3 parser
1 parent 2ffcac3 commit fd0b0d3

18 files changed

+95
-144
lines changed

‎doctests/dt_set.py

+17-17
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@
5858
r.sadd("bikes:racing:usa", "bike:1", "bike:4")
5959
# HIDE_END
6060
res7 = r.sinter("bikes:racing:france", "bikes:racing:usa")
61-
print(res7) # >>> {'bike:1'}
61+
print(res7) # >>> ['bike:1']
6262
# STEP_END
6363

6464
# REMOVE_START
65-
assert res7 == {"bike:1"}
65+
assert res7 == ["bike:1"]
6666
# REMOVE_END
6767

6868
# STEP_START scard
@@ -83,12 +83,12 @@
8383
print(res9) # >>> 3
8484

8585
res10 = r.smembers("bikes:racing:france")
86-
print(res10) # >>> {'bike:1', 'bike:2', 'bike:3'}
86+
print(res10) # >>> ['bike:1', 'bike:2', 'bike:3']
8787
# STEP_END
8888

8989
# REMOVE_START
9090
assert res9 == 3
91-
assert res10 == {"bike:1", "bike:2", "bike:3"}
91+
assert res10 == ['bike:1', 'bike:2', 'bike:3']
9292
# REMOVE_END
9393

9494
# STEP_START smismember
@@ -109,11 +109,11 @@
109109
r.sadd("bikes:racing:usa", "bike:1", "bike:4")
110110

111111
res13 = r.sdiff("bikes:racing:france", "bikes:racing:usa")
112-
print(res13) # >>> {'bike:2', 'bike:3'}
112+
print(res13) # >>> ['bike:2', 'bike:3']
113113
# STEP_END
114114

115115
# REMOVE_START
116-
assert res13 == {"bike:2", "bike:3"}
116+
assert res13 == ['bike:2', 'bike:3']
117117
r.delete("bikes:racing:france")
118118
r.delete("bikes:racing:usa")
119119
# REMOVE_END
@@ -124,27 +124,27 @@
124124
r.sadd("bikes:racing:italy", "bike:1", "bike:2", "bike:3", "bike:4")
125125

126126
res13 = r.sinter("bikes:racing:france", "bikes:racing:usa", "bikes:racing:italy")
127-
print(res13) # >>> {'bike:1'}
127+
print(res13) # >>> ['bike:1']
128128

129129
res14 = r.sunion("bikes:racing:france", "bikes:racing:usa", "bikes:racing:italy")
130-
print(res14) # >>> {'bike:1', 'bike:2', 'bike:3', 'bike:4'}
130+
print(res14) # >>> ['bike:1', 'bike:2', 'bike:3', 'bike:4']
131131

132132
res15 = r.sdiff("bikes:racing:france", "bikes:racing:usa", "bikes:racing:italy")
133-
print(res15) # >>> set()
133+
print(res15) # >>> []
134134

135135
res16 = r.sdiff("bikes:racing:usa", "bikes:racing:france")
136-
print(res16) # >>> {'bike:4'}
136+
print(res16) # >>> ['bike:4']
137137

138138
res17 = r.sdiff("bikes:racing:france", "bikes:racing:usa")
139-
print(res17) # >>> {'bike:2', 'bike:3'}
139+
print(res17) # >>> ['bike:2', 'bike:3']
140140
# STEP_END
141141

142142
# REMOVE_START
143-
assert res13 == {"bike:1"}
144-
assert res14 == {"bike:1", "bike:2", "bike:3", "bike:4"}
145-
assert res15 == set()
146-
assert res16 == {"bike:4"}
147-
assert res17 == {"bike:2", "bike:3"}
143+
assert res13 == ['bike:1']
144+
assert res14 == ['bike:1', 'bike:2', 'bike:3', 'bike:4']
145+
assert res15 == []
146+
assert res16 == ['bike:4']
147+
assert res17 == ['bike:2', 'bike:3']
148148
r.delete("bikes:racing:france")
149149
r.delete("bikes:racing:usa")
150150
r.delete("bikes:racing:italy")
@@ -160,7 +160,7 @@
160160
print(res19) # >>> bike:3
161161

162162
res20 = r.smembers("bikes:racing:france")
163-
print(res20) # >>> {'bike:2', 'bike:4', 'bike:5'}
163+
print(res20) # >>> ['bike:2', 'bike:4', 'bike:5']
164164

165165
res21 = r.srandmember("bikes:racing:france")
166166
print(res21) # >>> bike:4

‎redis/_parsers/helpers.py

-3
Original file line numberDiff line numberDiff line change
@@ -783,9 +783,6 @@ def string_keys_to_dict(key_string, callback):
783783

784784

785785
_RedisCallbacksRESP2 = {
786-
**string_keys_to_dict(
787-
"SDIFF SINTER SMEMBERS SUNION", lambda r: r and set(r) or set()
788-
),
789786
**string_keys_to_dict(
790787
"ZDIFF ZINTER ZPOPMAX ZPOPMIN ZRANGE ZRANGEBYSCORE ZRANK ZREVRANGE "
791788
"ZREVRANGEBYSCORE ZREVRANK ZUNION",

‎redis/_parsers/resp3.py

+2-10
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,11 @@ def _read_response(self, disable_decoding=False, push_request=False):
8888
# set response
8989
elif byte == b"~":
9090
# redis can return unhashable types (like dict) in a set,
91-
# so we need to first convert to a list, and then try to convert it to a set
91+
# so we return sets as list, all the time, for predictability
9292
response = [
9393
self._read_response(disable_decoding=disable_decoding)
9494
for _ in range(int(response))
9595
]
96-
try:
97-
response = set(response)
98-
except TypeError:
99-
pass
10096
# map response
10197
elif byte == b"%":
10298
# We cannot use a dict-comprehension to parse stream.
@@ -233,15 +229,11 @@ async def _read_response(
233229
# set response
234230
elif byte == b"~":
235231
# redis can return unhashable types (like dict) in a set,
236-
# so we need to first convert to a list, and then try to convert it to a set
232+
# so we always convert to a list, to have predictable return types
237233
response = [
238234
(await self._read_response(disable_decoding=disable_decoding))
239235
for _ in range(int(response))
240236
]
241-
try:
242-
response = set(response)
243-
except TypeError:
244-
pass
245237
# map response
246238
elif byte == b"%":
247239
# We cannot use a dict-comprehension to parse stream.

‎redis/commands/bf/commands.py

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from redis.client import NEVER_DECODE
2-
from redis.exceptions import ModuleError
3-
from redis.utils import HIREDIS_AVAILABLE, deprecated_function
2+
from redis.utils import deprecated_function
43

54
BF_RESERVE = "BF.RESERVE"
65
BF_ADD = "BF.ADD"
@@ -139,9 +138,6 @@ def scandump(self, key, iter):
139138
This command will return successive (iter, data) pairs until (0, NULL) to indicate completion.
140139
For more information see `BF.SCANDUMP <https://redis.io/commands/bf.scandump>`_.
141140
""" # noqa
142-
if HIREDIS_AVAILABLE:
143-
raise ModuleError("This command cannot be used when hiredis is available.")
144-
145141
params = [key, iter]
146142
options = {}
147143
options[NEVER_DECODE] = []

‎redis/utils.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
try:
77
import hiredis # noqa
88

9-
# Only support Hiredis >= 1.0:
10-
HIREDIS_AVAILABLE = not hiredis.__version__.startswith("0.")
9+
# Only support Hiredis >= 3.0:
10+
HIREDIS_AVAILABLE = int(hiredis.__version__.split(".")[0]) >= 3
1111
HIREDIS_PACK_AVAILABLE = hasattr(hiredis, "pack_command")
1212
except ImportError:
1313
HIREDIS_AVAILABLE = False

‎setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"Programming Language :: Python :: Implementation :: PyPy",
5757
],
5858
extras_require={
59-
"hiredis": ["hiredis>=1.0.0"],
59+
"hiredis": ["hiredis>=3.0.0"],
6060
"ocsp": ["cryptography>=36.0.1", "pyopenssl==23.2.1", "requests>=2.31.0"],
6161
},
6262
)

‎tests/test_asyncio/test_bloom.py

+1-6
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
import pytest
44
import pytest_asyncio
55
import redis.asyncio as redis
6-
from redis.exceptions import ModuleError, RedisError
7-
from redis.utils import HIREDIS_AVAILABLE
6+
from redis.exceptions import RedisError
87
from tests.conftest import (
98
assert_resp_response,
109
is_resp2_connection,
@@ -105,10 +104,6 @@ async def do_verify():
105104

106105
await do_verify()
107106
cmds = []
108-
if HIREDIS_AVAILABLE:
109-
with pytest.raises(ModuleError):
110-
cur = await decoded_r.bf().scandump("myBloom", 0)
111-
return
112107

113108
cur = await decoded_r.bf().scandump("myBloom", 0)
114109
first = cur[0]

‎tests/test_asyncio/test_cluster.py

+12-12
Original file line numberDiff line numberDiff line change
@@ -1753,49 +1753,49 @@ async def test_cluster_rpoplpush(self, r: RedisCluster) -> None:
17531753

17541754
async def test_cluster_sdiff(self, r: RedisCluster) -> None:
17551755
await r.sadd("{foo}a", "1", "2", "3")
1756-
assert await r.sdiff("{foo}a", "{foo}b") == {b"1", b"2", b"3"}
1756+
assert set(await r.sdiff("{foo}a", "{foo}b")) == {b"1", b"2", b"3"}
17571757
await r.sadd("{foo}b", "2", "3")
1758-
assert await r.sdiff("{foo}a", "{foo}b") == {b"1"}
1758+
assert await r.sdiff("{foo}a", "{foo}b") == [b"1"]
17591759

17601760
async def test_cluster_sdiffstore(self, r: RedisCluster) -> None:
17611761
await r.sadd("{foo}a", "1", "2", "3")
17621762
assert await r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 3
1763-
assert await r.smembers("{foo}c") == {b"1", b"2", b"3"}
1763+
assert set(await r.smembers("{foo}c")) == {b"1", b"2", b"3"}
17641764
await r.sadd("{foo}b", "2", "3")
17651765
assert await r.sdiffstore("{foo}c", "{foo}a", "{foo}b") == 1
1766-
assert await r.smembers("{foo}c") == {b"1"}
1766+
assert await r.smembers("{foo}c") == [b"1"]
17671767

17681768
async def test_cluster_sinter(self, r: RedisCluster) -> None:
17691769
await r.sadd("{foo}a", "1", "2", "3")
1770-
assert await r.sinter("{foo}a", "{foo}b") == set()
1770+
assert await r.sinter("{foo}a", "{foo}b") == []
17711771
await r.sadd("{foo}b", "2", "3")
1772-
assert await r.sinter("{foo}a", "{foo}b") == {b"2", b"3"}
1772+
assert set(await r.sinter("{foo}a", "{foo}b")) == {b"2", b"3"}
17731773

17741774
async def test_cluster_sinterstore(self, r: RedisCluster) -> None:
17751775
await r.sadd("{foo}a", "1", "2", "3")
17761776
assert await r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 0
1777-
assert await r.smembers("{foo}c") == set()
1777+
assert await r.smembers("{foo}c") == []
17781778
await r.sadd("{foo}b", "2", "3")
17791779
assert await r.sinterstore("{foo}c", "{foo}a", "{foo}b") == 2
1780-
assert await r.smembers("{foo}c") == {b"2", b"3"}
1780+
assert set(await r.smembers("{foo}c")) == {b"2", b"3"}
17811781

17821782
async def test_cluster_smove(self, r: RedisCluster) -> None:
17831783
await r.sadd("{foo}a", "a1", "a2")
17841784
await r.sadd("{foo}b", "b1", "b2")
17851785
assert await r.smove("{foo}a", "{foo}b", "a1")
1786-
assert await r.smembers("{foo}a") == {b"a2"}
1787-
assert await r.smembers("{foo}b") == {b"b1", b"b2", b"a1"}
1786+
assert await r.smembers("{foo}a") == [b"a2"]
1787+
assert set(await r.smembers("{foo}b")) == {b"b1", b"b2", b"a1"}
17881788

17891789
async def test_cluster_sunion(self, r: RedisCluster) -> None:
17901790
await r.sadd("{foo}a", "1", "2")
17911791
await r.sadd("{foo}b", "2", "3")
1792-
assert await r.sunion("{foo}a", "{foo}b") == {b"1", b"2", b"3"}
1792+
assert set(await r.sunion("{foo}a", "{foo}b")) == {b"1", b"2", b"3"}
17931793

17941794
async def test_cluster_sunionstore(self, r: RedisCluster) -> None:
17951795
await r.sadd("{foo}a", "1", "2")
17961796
await r.sadd("{foo}b", "2", "3")
17971797
assert await r.sunionstore("{foo}c", "{foo}a", "{foo}b") == 3
1798-
assert await r.smembers("{foo}c") == {b"1", b"2", b"3"}
1798+
assert set(await r.smembers("{foo}c")) == {b"1", b"2", b"3"}
17991799

18001800
@skip_if_server_version_lt("6.2.0")
18011801
async def test_cluster_zdiff(self, r: RedisCluster) -> None:

‎tests/test_asyncio/test_commands.py

+17-19
Original file line numberDiff line numberDiff line change
@@ -1406,7 +1406,7 @@ async def test_zscan_iter(self, r: redis.Redis):
14061406
async def test_sadd(self, r: redis.Redis):
14071407
members = {b"1", b"2", b"3"}
14081408
await r.sadd("a", *members)
1409-
assert await r.smembers("a") == members
1409+
assert set(await r.smembers("a")) == members
14101410

14111411
async def test_scard(self, r: redis.Redis):
14121412
await r.sadd("a", "1", "2", "3")
@@ -1415,34 +1415,34 @@ async def test_scard(self, r: redis.Redis):
14151415
@pytest.mark.onlynoncluster
14161416
async def test_sdiff(self, r: redis.Redis):
14171417
await r.sadd("a", "1", "2", "3")
1418-
assert await r.sdiff("a", "b") == {b"1", b"2", b"3"}
1418+
assert set(await r.sdiff("a", "b")) == {b"1", b"2", b"3"}
14191419
await r.sadd("b", "2", "3")
1420-
assert await r.sdiff("a", "b") == {b"1"}
1420+
assert await r.sdiff("a", "b") == [b"1"]
14211421

14221422
@pytest.mark.onlynoncluster
14231423
async def test_sdiffstore(self, r: redis.Redis):
14241424
await r.sadd("a", "1", "2", "3")
14251425
assert await r.sdiffstore("c", "a", "b") == 3
1426-
assert await r.smembers("c") == {b"1", b"2", b"3"}
1426+
assert set(await r.smembers("c")) == {b"1", b"2", b"3"}
14271427
await r.sadd("b", "2", "3")
14281428
assert await r.sdiffstore("c", "a", "b") == 1
1429-
assert await r.smembers("c") == {b"1"}
1429+
assert await r.smembers("c") == [b"1"]
14301430

14311431
@pytest.mark.onlynoncluster
14321432
async def test_sinter(self, r: redis.Redis):
14331433
await r.sadd("a", "1", "2", "3")
1434-
assert await r.sinter("a", "b") == set()
1434+
assert await r.sinter("a", "b") == []
14351435
await r.sadd("b", "2", "3")
1436-
assert await r.sinter("a", "b") == {b"2", b"3"}
1436+
assert set(await r.sinter("a", "b")) == {b"2", b"3"}
14371437

14381438
@pytest.mark.onlynoncluster
14391439
async def test_sinterstore(self, r: redis.Redis):
14401440
await r.sadd("a", "1", "2", "3")
14411441
assert await r.sinterstore("c", "a", "b") == 0
1442-
assert await r.smembers("c") == set()
1442+
assert await r.smembers("c") == []
14431443
await r.sadd("b", "2", "3")
14441444
assert await r.sinterstore("c", "a", "b") == 2
1445-
assert await r.smembers("c") == {b"2", b"3"}
1445+
assert set(await r.smembers("c")) == {b"2", b"3"}
14461446

14471447
async def test_sismember(self, r: redis.Redis):
14481448
await r.sadd("a", "1", "2", "3")
@@ -1453,22 +1453,22 @@ async def test_sismember(self, r: redis.Redis):
14531453

14541454
async def test_smembers(self, r: redis.Redis):
14551455
await r.sadd("a", "1", "2", "3")
1456-
assert await r.smembers("a") == {b"1", b"2", b"3"}
1456+
assert set(await r.smembers("a")) == {b"1", b"2", b"3"}
14571457

14581458
@pytest.mark.onlynoncluster
14591459
async def test_smove(self, r: redis.Redis):
14601460
await r.sadd("a", "a1", "a2")
14611461
await r.sadd("b", "b1", "b2")
14621462
assert await r.smove("a", "b", "a1")
1463-
assert await r.smembers("a") == {b"a2"}
1464-
assert await r.smembers("b") == {b"b1", b"b2", b"a1"}
1463+
assert await r.smembers("a") == [b"a2"]
1464+
assert set(await r.smembers("b")) == {b"b1", b"b2", b"a1"}
14651465

14661466
async def test_spop(self, r: redis.Redis):
14671467
s = [b"1", b"2", b"3"]
14681468
await r.sadd("a", *s)
14691469
value = await r.spop("a")
14701470
assert value in s
1471-
assert await r.smembers("a") == set(s) - {value}
1471+
assert set(await r.smembers("a")) == set(s) - {value}
14721472

14731473
@skip_if_server_version_lt("3.2.0")
14741474
async def test_spop_multi_value(self, r: redis.Redis):
@@ -1481,9 +1481,7 @@ async def test_spop_multi_value(self, r: redis.Redis):
14811481
assert value in s
14821482

14831483
response = await r.spop("a", 1)
1484-
assert_resp_response(
1485-
r, response, list(set(s) - set(values)), set(s) - set(values)
1486-
)
1484+
assert set(response) == set(s) - set(values)
14871485

14881486
async def test_srandmember(self, r: redis.Redis):
14891487
s = [b"1", b"2", b"3"]
@@ -1502,20 +1500,20 @@ async def test_srem(self, r: redis.Redis):
15021500
await r.sadd("a", "1", "2", "3", "4")
15031501
assert await r.srem("a", "5") == 0
15041502
assert await r.srem("a", "2", "4") == 2
1505-
assert await r.smembers("a") == {b"1", b"3"}
1503+
assert set(await r.smembers("a")) == {b"1", b"3"}
15061504

15071505
@pytest.mark.onlynoncluster
15081506
async def test_sunion(self, r: redis.Redis):
15091507
await r.sadd("a", "1", "2")
15101508
await r.sadd("b", "2", "3")
1511-
assert await r.sunion("a", "b") == {b"1", b"2", b"3"}
1509+
assert set(await r.sunion("a", "b")) == {b"1", b"2", b"3"}
15121510

15131511
@pytest.mark.onlynoncluster
15141512
async def test_sunionstore(self, r: redis.Redis):
15151513
await r.sadd("a", "1", "2")
15161514
await r.sadd("b", "2", "3")
15171515
assert await r.sunionstore("c", "a", "b") == 3
1518-
assert await r.smembers("c") == {b"1", b"2", b"3"}
1516+
assert set(await r.smembers("c")) == {b"1", b"2", b"3"}
15191517

15201518
# SORTED SET COMMANDS
15211519
async def test_zadd(self, r: redis.Redis):

‎tests/test_asyncio/test_search.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
from redis.commands.search.result import Result
2525
from redis.commands.search.suggestion import Suggestion
2626
from tests.conftest import (
27-
assert_resp_response,
2827
is_resp2_connection,
2928
skip_if_redis_enterprise,
3029
skip_if_resp_version,
@@ -862,7 +861,7 @@ async def test_tags(decoded_r: redis.Redis):
862861
assert 1 == res["total_results"]
863862

864863
q2 = await decoded_r.ft().tagvals("tags")
865-
assert set(tags.split(",") + tags2.split(",")) == q2
864+
assert set(tags.split(",") + tags2.split(",")) == set(q2)
866865

867866

868867
@pytest.mark.redismod
@@ -986,7 +985,7 @@ async def test_dict_operations(decoded_r: redis.Redis):
986985

987986
# Dump dict and inspect content
988987
res = await decoded_r.ft().dict_dump("custom_dict")
989-
assert_resp_response(decoded_r, res, ["item1", "item3"], {"item1", "item3"})
988+
assert res == ["item1", "item3"]
990989

991990
# Remove rest of the items before reload
992991
await decoded_r.ft().dict_del("custom_dict", *res)

0 commit comments

Comments
 (0)