diff --git a/src/geo.c b/src/geo.c index 893f78a7e..c3f6c922c 100644 --- a/src/geo.c +++ b/src/geo.c @@ -509,7 +509,7 @@ void geoaddCommand(client *c) { * [COUNT count [ANY]] [STORE key] [STOREDIST key] * GEORADIUSBYMEMBER key member radius unit ... options ... * GEOSEARCH key [FROMMEMBER member] [FROMLONLAT long lat] [BYRADIUS radius unit] - * [BYBOX width height unit] [WITHCORD] [WITHDIST] [WITHASH] [COUNT count [ANY]] [ASC|DESC] + * [BYBOX width height unit] [WITHCOORD] [WITHDIST] [WITHASH] [COUNT count [ANY]] [ASC|DESC] * GEOSEARCHSTORE dest_key src_key [FROMMEMBER member] [FROMLONLAT long lat] [BYRADIUS radius unit] * [BYBOX width height unit] [WITHCORD] [WITHDIST] [WITHASH] [COUNT count [ANY]] [ASC|DESC] [STOREDIST] * */ @@ -518,21 +518,24 @@ void georadiusGeneric(client *c, int srcKeyIndex, int flags) { int storedist = 0; /* 0 for STORE, 1 for STOREDIST. */ /* Look up the requested zset */ - robj *zobj = NULL; - if ((zobj = lookupKeyReadOrReply(c, c->argv[srcKeyIndex], shared.emptyarray)) == NULL || - checkType(c, zobj, OBJ_ZSET)) { - return; - } + robj *zobj = lookupKeyRead(c->db, c->argv[srcKeyIndex]); + if (checkType(c, zobj, OBJ_ZSET)) return; /* Find long/lat to use for radius or box search based on inquiry type */ int base_args; GeoShape shape = {0}; if (flags & RADIUS_COORDS) { + /* GEORADIUS or GEORADIUS_RO */ base_args = 6; shape.type = CIRCULAR_TYPE; if (extractLongLatOrReply(c, c->argv + 2, shape.xy) == C_ERR) return; if (extractDistanceOrReply(c, c->argv+base_args-2, &shape.conversion, &shape.t.radius) != C_OK) return; + } else if ((flags & RADIUS_MEMBER) && !zobj) { + /* We don't have a source key, but we need to proceed with argument + * parsing, so we know which reply to use depending on the STORE flag. */ + base_args = 5; } else if (flags & RADIUS_MEMBER) { + /* GEORADIUSBYMEMBER or GEORADIUSBYMEMBER_RO */ base_args = 5; shape.type = CIRCULAR_TYPE; robj *member = c->argv[2]; @@ -542,6 +545,7 @@ void georadiusGeneric(client *c, int srcKeyIndex, int flags) { } if (extractDistanceOrReply(c, c->argv+base_args-2, &shape.conversion, &shape.t.radius) != C_OK) return; } else if (flags & GEOSEARCH) { + /* GEOSEARCH or GEOSEARCHSTORE */ base_args = 2; if (flags & GEOSEARCHSTORE) { base_args = 3; @@ -608,6 +612,13 @@ void georadiusGeneric(client *c, int srcKeyIndex, int flags) { flags & GEOSEARCH && !fromloc) { + /* No source key, proceed with argument parsing and return an error when done. */ + if (zobj == NULL) { + frommember = 1; + i++; + continue; + } + if (longLatFromMember(zobj, c->argv[base_args+i+1], shape.xy) == C_ERR) { addReplyError(c, "could not decode requested zset member"); return; @@ -676,6 +687,23 @@ void georadiusGeneric(client *c, int srcKeyIndex, int flags) { return; } + /* Return ASAP when src key does not exist. */ + if (zobj == NULL) { + if (storekey) { + /* store key is not NULL, try to delete it and return 0. */ + if (dbDelete(c->db, storekey)) { + signalModifiedKey(c, c->db, storekey); + notifyKeyspaceEvent(NOTIFY_GENERIC, "del", storekey, c->db->id); + server.dirty++; + } + addReply(c, shared.czero); + } else { + /* Otherwise we return an empty array. */ + addReply(c, shared.emptyarray); + } + return; + } + /* COUNT without ordering does not make much sense (we need to * sort in order to return the closest N entries), * force ASC ordering if COUNT was specified but no sorting was diff --git a/tests/unit/geo.tcl b/tests/unit/geo.tcl index a51d1dc5c..3aa8f4d56 100644 --- a/tests/unit/geo.tcl +++ b/tests/unit/geo.tcl @@ -71,6 +71,34 @@ proc pointInRectangle {width_km height_km lon lat search_lon search_lat error} { return true } +proc verify_geo_edge_response_bylonlat {expected_response expected_store_response} { + catch {r georadius src{t} 1 1 1 km} response + assert_match $expected_response $response + + catch {r georadius src{t} 1 1 1 km store dest{t}} response + assert_match $expected_store_response $response + + catch {r geosearch src{t} fromlonlat 0 0 byradius 1 km} response + assert_match $expected_response $response + + catch {r geosearchstore dest{t} src{t} fromlonlat 0 0 byradius 1 km} response + assert_match $expected_store_response $response +} + +proc verify_geo_edge_response_bymember {expected_response expected_store_response} { + catch {r georadiusbymember src{t} member 1 km} response + assert_match $expected_response $response + + catch {r georadiusbymember src{t} member 1 km store dest{t}} response + assert_match $expected_store_response $response + + catch {r geosearch src{t} frommember member bybox 1 1 km} response + assert_match $expected_response $response + + catch {r geosearchstore dest{t} src{t} frommember member bybox 1 1 m} response + assert_match $expected_store_response $response +} + # The following list represents sets of random seed, search position # and radius that caused bugs in the past. It is used by the randomized # test later as a starting point. When the regression vectors are scanned @@ -95,6 +123,34 @@ set regression_vectors { set rv_idx 0 start_server {tags {"geo"}} { + test {GEO with wrong type src key} { + r set src{t} wrong_type + + verify_geo_edge_response_bylonlat "WRONGTYPE*" "WRONGTYPE*" + verify_geo_edge_response_bymember "WRONGTYPE*" "WRONGTYPE*" + } + + test {GEO with non existing src key} { + r del src{t} + + verify_geo_edge_response_bylonlat {} 0 + verify_geo_edge_response_bymember {} 0 + } + + test {GEO BYLONLAT with empty search} { + r del src{t} + r geoadd src{t} 13.361389 38.115556 "Palermo" 15.087269 37.502669 "Catania" + + verify_geo_edge_response_bylonlat {} 0 + } + + test {GEO BYMEMBER with non existing member} { + r del src{t} + r geoadd src{t} 13.361389 38.115556 "Palermo" 15.087269 37.502669 "Catania" + + verify_geo_edge_response_bymember "ERR*" "ERR*" + } + test {GEOADD create} { r geoadd nyc -73.9454966 40.747533 "lic market" } {1} @@ -357,6 +413,21 @@ start_server {tags {"geo"}} { assert_equal [r zrange points 0 -1] [r zrange points2 0 -1] } + test {GEORADIUSBYMEMBER STORE/STOREDIST option: plain usage} { + r del points{t} + r geoadd points{t} 13.361389 38.115556 "Palermo" 15.087269 37.502669 "Catania" + + r georadiusbymember points{t} Palermo 500 km store points2{t} + assert_equal {Palermo Catania} [r zrange points2{t} 0 -1] + + r georadiusbymember points{t} Catania 500 km storedist points2{t} + assert_equal {Catania Palermo} [r zrange points2{t} 0 -1] + + set res [r zrange points2{t} 0 -1 withscores] + assert {[lindex $res 1] < 1} + assert {[lindex $res 3] > 166} + } + test {GEOSEARCHSTORE STORE option: plain usage} { r geosearchstore points2 points fromlonlat 13.361389 38.115556 byradius 500 km assert_equal [r zrange points 0 -1] [r zrange points2 0 -1]