From 72fe7adcb5846ac2539e4dde95de2cf1ddbe1725 Mon Sep 17 00:00:00 2001 From: AvitalFineRedis Date: Wed, 13 Oct 2021 15:13:31 +0200 Subject: [PATCH 1/6] Add support to NX XX and CH to GEOADD --- redis/commands.py | 25 +++++++++++++++++++++++-- tests/test_commands.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/redis/commands.py b/redis/commands.py index eb7cea54f6..55754ef710 100644 --- a/redis/commands.py +++ b/redis/commands.py @@ -2865,17 +2865,38 @@ def register_script(self, script): return Script(self, script) # GEO COMMANDS - def geoadd(self, name, *values): + def geoadd(self, name, *values, nx=False, xx=False, ch=False): """ Add the specified geospatial items to the specified key identified by the ``name`` argument. The Geospatial items are given as ordered members of the ``values`` argument, each item or place is formed by the triad longitude, latitude and name. + + ``nx`` forces ZADD to only create new elements and not to update + scores for elements that already exist. + + ``xx`` forces ZADD to only update scores of elements that already + exist. New elements will not be added. + + ``ch`` modifies the return value to be the numbers of elements changed. + Changed elements include new elements that were added and elements + whose scores changed. """ + if nx and xx: + raise DataError("GEOADD allows either 'nx' or 'xx', not both") if len(values) % 3 != 0: raise DataError("GEOADD requires places with lon, lat and name" " values") - return self.execute_command('GEOADD', name, *values) + + pieces = [name] + if nx: + pieces.append(b'NX') + if xx: + pieces.append(b'XX') + if ch: + pieces.append(b'CH') + pieces.extend(values) + return self.execute_command('GEOADD', *pieces) def geodist(self, name, place1, place2, unit=None): """ diff --git a/tests/test_commands.py b/tests/test_commands.py index 2be8923e0e..bb02da2781 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2382,6 +2382,35 @@ def test_geoadd(self, r): assert r.geoadd('barcelona', *values) == 2 assert r.zcard('barcelona') == 2 + @skip_if_server_version_lt('6.2.0') + def test_geoadd_nx(self, r): + values = (2.1909389952632, 41.433791470673, 'place1') + \ + (2.1873744593677, 41.406342043777, 'place2') + assert r.geoadd('a', *values) == 2 + values = (2.1909389952632, 41.433791470673, 'place1') + \ + (2.1873744593677, 41.406342043777, 'place2') + \ + (2.1804738294738, 41.405647879212, 'place3') + assert r.geoadd('a', *values, nx=True) == 1 + assert r.zrange('a', 0, -1) == [b'place3', b'place2', b'place1'] + + def test_geoadd_xx(self, r): + values = (2.1909389952632, 41.433791470673, 'place1') + assert r.geoadd('a', *values) == 1 + values = (2.1909389952632, 41.433791470673, 'place1') + \ + (2.1873744593677, 41.406342043777, 'place2') + assert r.geoadd('a', *values, xx=True) == 0 + assert r.zrange('a', 0, -1) == \ + [b'place1'] + + def test_geoadd_ch(self, r): + values = (2.1909389952632, 41.433791470673, 'place1') + assert r.geoadd('a', *values) == 1 + values = (2.1909389952632, 31.433791470673, 'place1') + \ + (2.1873744593677, 41.406342043777, 'place2') + assert r.geoadd('a', *values, ch=True) == 2 + assert r.zrange('a', 0, -1) == \ + [b'place1', b'place2'] + @skip_if_server_version_lt('3.2.0') def test_geoadd_invalid_params(self, r): with pytest.raises(exceptions.RedisError): From 3b0992d9ebc144a55429a3c5d3df01028d0cc6ca Mon Sep 17 00:00:00 2001 From: AvitalFineRedis Date: Wed, 13 Oct 2021 15:15:14 +0200 Subject: [PATCH 2/6] skip_if_server_version_lt('6.2.0') --- tests/test_commands.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_commands.py b/tests/test_commands.py index bb02da2781..eb6b98ece9 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2393,6 +2393,7 @@ def test_geoadd_nx(self, r): assert r.geoadd('a', *values, nx=True) == 1 assert r.zrange('a', 0, -1) == [b'place3', b'place2', b'place1'] + @skip_if_server_version_lt('6.2.0') def test_geoadd_xx(self, r): values = (2.1909389952632, 41.433791470673, 'place1') assert r.geoadd('a', *values) == 1 @@ -2402,6 +2403,7 @@ def test_geoadd_xx(self, r): assert r.zrange('a', 0, -1) == \ [b'place1'] + @skip_if_server_version_lt('6.2.0') def test_geoadd_ch(self, r): values = (2.1909389952632, 41.433791470673, 'place1') assert r.geoadd('a', *values) == 1 From 93f29a16b7d94def3f933cb3d7db5797e702c768 Mon Sep 17 00:00:00 2001 From: AvitalFineRedis Date: Thu, 14 Oct 2021 08:24:22 +0200 Subject: [PATCH 3/6] stam --- redis/commands.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/redis/commands.py b/redis/commands.py index 55754ef710..62e4c686da 100644 --- a/redis/commands.py +++ b/redis/commands.py @@ -2887,14 +2887,13 @@ def geoadd(self, name, *values, nx=False, xx=False, ch=False): if len(values) % 3 != 0: raise DataError("GEOADD requires places with lon, lat and name" " values") - pieces = [name] if nx: - pieces.append(b'NX') + pieces.append('NX') if xx: - pieces.append(b'XX') + pieces.append('XX') if ch: - pieces.append(b'CH') + pieces.append('CH') pieces.extend(values) return self.execute_command('GEOADD', *pieces) From af453e5b6bc09995324e100778b5bb9966f3006d Mon Sep 17 00:00:00 2001 From: AvitalFineRedis Date: Thu, 14 Oct 2021 16:27:54 +0200 Subject: [PATCH 4/6] align to redis command --- redis/commands.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/redis/commands.py b/redis/commands.py index 62e4c686da..699a763032 100644 --- a/redis/commands.py +++ b/redis/commands.py @@ -2865,13 +2865,15 @@ def register_script(self, script): return Script(self, script) # GEO COMMANDS - def geoadd(self, name, *values, nx=False, xx=False, ch=False): + def geoadd(self, name, nx=False, xx=False, ch=False, *values): """ Add the specified geospatial items to the specified key identified by the ``name`` argument. The Geospatial items are given as ordered members of the ``values`` argument, each item or place is formed by the triad longitude, latitude and name. + Note: You can use ZREM to remove elements. + ``nx`` forces ZADD to only create new elements and not to update scores for elements that already exist. From 448bc5fa439e9a91799221a3ec67d68aa8c4a75f Mon Sep 17 00:00:00 2001 From: AvitalFineRedis Date: Thu, 14 Oct 2021 17:15:10 +0200 Subject: [PATCH 5/6] align to redis command --- redis/commands.py | 2 +- tests/test_commands.py | 59 +++++++++++++++++++----------------------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/redis/commands.py b/redis/commands.py index 699a763032..81fa8ea618 100644 --- a/redis/commands.py +++ b/redis/commands.py @@ -2865,7 +2865,7 @@ def register_script(self, script): return Script(self, script) # GEO COMMANDS - def geoadd(self, name, nx=False, xx=False, ch=False, *values): + def geoadd(self, name, values, nx=False, xx=False, ch=False): """ Add the specified geospatial items to the specified key identified by the ``name`` argument. The Geospatial items are given as ordered diff --git a/tests/test_commands.py b/tests/test_commands.py index eb6b98ece9..2b63a82411 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2378,38 +2378,37 @@ def test_readonly(self, mock_cluster_resp_ok): def test_geoadd(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - - assert r.geoadd('barcelona', *values) == 2 + assert r.geoadd('barcelona', values) == 2 assert r.zcard('barcelona') == 2 @skip_if_server_version_lt('6.2.0') def test_geoadd_nx(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - assert r.geoadd('a', *values) == 2 + assert r.geoadd('a', values) == 2 values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') + \ (2.1804738294738, 41.405647879212, 'place3') - assert r.geoadd('a', *values, nx=True) == 1 + assert r.geoadd('a', values, nx=True) == 1 assert r.zrange('a', 0, -1) == [b'place3', b'place2', b'place1'] @skip_if_server_version_lt('6.2.0') def test_geoadd_xx(self, r): values = (2.1909389952632, 41.433791470673, 'place1') - assert r.geoadd('a', *values) == 1 + assert r.geoadd('a', values) == 1 values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - assert r.geoadd('a', *values, xx=True) == 0 + assert r.geoadd('a', values, xx=True) == 0 assert r.zrange('a', 0, -1) == \ [b'place1'] @skip_if_server_version_lt('6.2.0') def test_geoadd_ch(self, r): values = (2.1909389952632, 41.433791470673, 'place1') - assert r.geoadd('a', *values) == 1 + assert r.geoadd('a', values) == 1 values = (2.1909389952632, 31.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - assert r.geoadd('a', *values, ch=True) == 2 + assert r.geoadd('a', values, ch=True) == 2 assert r.zrange('a', 0, -1) == \ [b'place1', b'place2'] @@ -2422,22 +2421,20 @@ def test_geoadd_invalid_params(self, r): def test_geodist(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - - assert r.geoadd('barcelona', *values) == 2 + assert r.geoadd('barcelona', values) == 2 assert r.geodist('barcelona', 'place1', 'place2') == 3067.4157 @skip_if_server_version_lt('3.2.0') def test_geodist_units(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.geodist('barcelona', 'place1', 'place2', 'km') == 3.0674 @skip_if_server_version_lt('3.2.0') def test_geodist_missing_one_member(self, r): values = (2.1909389952632, 41.433791470673, 'place1') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.geodist('barcelona', 'place1', 'missing_member', 'km') is None @skip_if_server_version_lt('3.2.0') @@ -2449,8 +2446,7 @@ def test_geodist_invalid_units(self, r): def test_geohash(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.geohash('barcelona', 'place1', 'place2', 'place3') == \ ['sp3e9yg3kd0', 'sp3e9cbc3t0', None] @@ -2459,8 +2455,7 @@ def test_geohash(self, r): def test_geopos(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) # redis uses 52 bits precision, hereby small errors may be introduced. assert r.geopos('barcelona', 'place1', 'place2') == \ [(2.19093829393386841, 41.43379028184083523), @@ -2480,7 +2475,7 @@ def test_geosearch(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, b'\x80place2') + \ (2.583333, 41.316667, 'place3') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.geosearch('barcelona', longitude=2.191, latitude=41.433, radius=1000) == [b'place1'] assert r.geosearch('barcelona', longitude=2.187, @@ -2502,7 +2497,7 @@ def test_geosearch_member(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, b'\x80place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.geosearch('barcelona', member='place1', radius=4000) == \ [b'\x80place2', b'place1'] assert r.geosearch('barcelona', member='place1', radius=10) == \ @@ -2521,7 +2516,7 @@ def test_geosearch_member(self, r): def test_geosearch_sort(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.geosearch('barcelona', longitude=2.191, latitude=41.433, radius=3000, sort='ASC') == \ [b'place1', b'place2'] @@ -2534,7 +2529,7 @@ def test_geosearch_sort(self, r): def test_geosearch_with(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) # test a bunch of combinations to test the parse response # function. @@ -2605,7 +2600,7 @@ def test_geosearchstore(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) r.geosearchstore('places_barcelona', 'barcelona', longitude=2.191, latitude=41.433, radius=1000) assert r.zrange('places_barcelona', 0, -1) == [b'place1'] @@ -2616,7 +2611,7 @@ def test_geosearchstore_dist(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) r.geosearchstore('places_barcelona', 'barcelona', longitude=2.191, latitude=41.433, radius=1000, storedist=True) @@ -2628,7 +2623,7 @@ def test_georadius(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, b'\x80place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.georadius('barcelona', 2.191, 41.433, 1000) == [b'place1'] assert r.georadius('barcelona', 2.187, 41.406, 1000) == [b'\x80place2'] @@ -2637,7 +2632,7 @@ def test_georadius_no_values(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.georadius('barcelona', 1, 2, 1000) == [] @skip_if_server_version_lt('3.2.0') @@ -2645,7 +2640,7 @@ def test_georadius_units(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.georadius('barcelona', 2.191, 41.433, 1, unit='km') == \ [b'place1'] @@ -2655,7 +2650,7 @@ def test_georadius_with(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) # test a bunch of combinations to test the parse response # function. @@ -2683,7 +2678,7 @@ def test_georadius_count(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.georadius('barcelona', 2.191, 41.433, 3000, count=1) == \ [b'place1'] @@ -2692,7 +2687,7 @@ def test_georadius_sort(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.georadius('barcelona', 2.191, 41.433, 3000, sort='ASC') == \ [b'place1', b'place2'] assert r.georadius('barcelona', 2.191, 41.433, 3000, sort='DESC') == \ @@ -2703,7 +2698,7 @@ def test_georadius_store(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) r.georadius('barcelona', 2.191, 41.433, 1000, store='places_barcelona') assert r.zrange('places_barcelona', 0, -1) == [b'place1'] @@ -2713,7 +2708,7 @@ def test_georadius_store_dist(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, 'place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) r.georadius('barcelona', 2.191, 41.433, 1000, store_dist='places_barcelona') # instead of save the geo score, the distance is saved. @@ -2725,7 +2720,7 @@ def test_georadiusmember(self, r): values = (2.1909389952632, 41.433791470673, 'place1') + \ (2.1873744593677, 41.406342043777, b'\x80place2') - r.geoadd('barcelona', *values) + r.geoadd('barcelona', values) assert r.georadiusbymember('barcelona', 'place1', 4000) == \ [b'\x80place2', b'place1'] assert r.georadiusbymember('barcelona', 'place1', 10) == [b'place1'] From 8d4053acfe2d34d4d3077bf6b47364383c254764 Mon Sep 17 00:00:00 2001 From: AvitalFineRedis Date: Thu, 14 Oct 2021 17:33:32 +0200 Subject: [PATCH 6/6] fix --- tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index 2b63a82411..e8b5847e08 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2415,7 +2415,7 @@ def test_geoadd_ch(self, r): @skip_if_server_version_lt('3.2.0') def test_geoadd_invalid_params(self, r): with pytest.raises(exceptions.RedisError): - r.geoadd('barcelona', *(1, 2)) + r.geoadd('barcelona', (1, 2)) @skip_if_server_version_lt('3.2.0') def test_geodist(self, r):