Skip to content

Commit 4616561

Browse files
EricHayterEricHayter
authored
feat: Implement GEORADIUS_RO command (#5678)
* feat: Implement GEORADIUS_RO command Signed-off-by: Eric <hayter.eric@gmail.com>
1 parent 4323aa3 commit 4616561

File tree

3 files changed

+62
-16
lines changed

3 files changed

+62
-16
lines changed

‎src/server/geo_family.cc

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ void GeoFamily::GeoRadiusByMember(CmdArgList args, const CommandContext& cmd_cnt
773773
GeoSearchStoreGeneric(cmd_cntx.tx, builder, shape, key, member, geo_ops);
774774
}
775775

776-
void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
776+
void GeoFamily::GeoRadiusGeneric(CmdArgList args, const CommandContext& cmd_cntx, bool read_only) {
777777
GeoShape shape = {};
778778
GeoSearchOpts geo_ops;
779779

@@ -789,12 +789,21 @@ void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
789789
shape.type = CIRCULAR_TYPE;
790790

791791
while (parser.HasNext()) {
792+
// try and parse for only RO options first
792793
auto type =
793-
parser.MapNext("STORE", Type::STORE, "STOREDIST", Type::STOREDIST, "ASC", Type::ASC, "DESC",
794-
Type::DESC, "COUNT", Type::COUNT, "WITHCOORD", Type::WITHCOORD, "WITHDIST",
795-
Type::WITHDIST, "WITHHASH", Type::WITHHASH);
794+
parser.TryMapNext("ASC", Type::ASC, "DESC", Type::DESC, "COUNT", Type::COUNT, "WITHCOORD",
795+
Type::WITHCOORD, "WITHDIST", Type::WITHDIST, "WITHHASH", Type::WITHHASH);
796+
// if writing variant and there there was a mapping failure test for write variant arguments
797+
if (!type && !read_only) {
798+
type = parser.MapNext("STORE", Type::STORE, "STOREDIST", Type::STOREDIST);
799+
}
796800

797-
switch (type) {
801+
// could not map the argument to an argument for RO or write GEORADIUS
802+
if (!type) {
803+
return builder->SendError("syntax error", kSyntaxErrType);
804+
}
805+
806+
switch (*type) {
798807
case Type::STORE:
799808
geo_ops.store_key = parser.Next();
800809
geo_ops.store = geo_ops.store == GeoStoreType::kNoStore ? GeoStoreType::kStoreHash
@@ -867,6 +876,14 @@ void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
867876
GeoSearchStoreGeneric(cmd_cntx.tx, builder, shape, key, "", geo_ops);
868877
}
869878

879+
void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
880+
GeoRadiusGeneric(args, cmd_cntx, false);
881+
}
882+
883+
void GeoFamily::GeoRadiusRO(CmdArgList args, const CommandContext& cmd_cntx) {
884+
GeoRadiusGeneric(args, cmd_cntx, true);
885+
}
886+
870887
#define HFUNC(x) SetHandler(&GeoFamily::x)
871888

872889
namespace acl {
@@ -877,21 +894,21 @@ constexpr uint32_t kGeoDist = READ | GEO | SLOW;
877894
constexpr uint32_t kGeoSearch = READ | GEO | SLOW;
878895
constexpr uint32_t kGeoRadiusByMember = WRITE | GEO | SLOW;
879896
constexpr uint32_t kGeoRadius = WRITE | GEO | SLOW;
897+
constexpr uint32_t kGeoRadiusRO = READ | GEO | SLOW;
880898
} // namespace acl
881899

882900
void GeoFamily::Register(CommandRegistry* registry) {
883901
registry->StartFamily();
884-
*registry << CI{"GEOADD", CO::FAST | CO::WRITE | CO::DENYOOM, -5, 1, 1, acl::kGeoAdd}.HFUNC(
885-
GeoAdd)
886-
<< CI{"GEOHASH", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoHash}.HFUNC(GeoHash)
887-
<< CI{"GEOPOS", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoPos}.HFUNC(GeoPos)
888-
<< CI{"GEODIST", CO::READONLY, -4, 1, 1, acl::kGeoDist}.HFUNC(GeoDist)
889-
<< CI{"GEOSEARCH", CO::READONLY, -7, 1, 1, acl::kGeoSearch}.HFUNC(GeoSearch)
890-
<< CI{"GEORADIUSBYMEMBER", CO::WRITE | CO::STORE_LAST_KEY, -5, 1, 1,
891-
acl::kGeoRadiusByMember}
892-
.HFUNC(GeoRadiusByMember)
893-
<< CI{"GEORADIUS", CO::WRITE | CO::STORE_LAST_KEY, -6, 1, 1, acl::kGeoRadius}.HFUNC(
894-
GeoRadius);
902+
*registry
903+
<< CI{"GEOADD", CO::FAST | CO::WRITE | CO::DENYOOM, -5, 1, 1, acl::kGeoAdd}.HFUNC(GeoAdd)
904+
<< CI{"GEOHASH", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoHash}.HFUNC(GeoHash)
905+
<< CI{"GEOPOS", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoPos}.HFUNC(GeoPos)
906+
<< CI{"GEODIST", CO::READONLY, -4, 1, 1, acl::kGeoDist}.HFUNC(GeoDist)
907+
<< CI{"GEOSEARCH", CO::READONLY, -7, 1, 1, acl::kGeoSearch}.HFUNC(GeoSearch)
908+
<< CI{"GEORADIUSBYMEMBER", CO::WRITE | CO::STORE_LAST_KEY, -5, 1, 1, acl::kGeoRadiusByMember}
909+
.HFUNC(GeoRadiusByMember)
910+
<< CI{"GEORADIUS", CO::WRITE | CO::STORE_LAST_KEY, -6, 1, 1, acl::kGeoRadius}.HFUNC(GeoRadius)
911+
<< CI{"GEORADIUS_RO", CO::READONLY, -6, 1, 1, acl::kGeoRadiusRO}.HFUNC(GeoRadiusRO);
895912
}
896913

897914
} // namespace dfly

‎src/server/geo_family.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ class GeoFamily {
2222
static void GeoDist(CmdArgList args, const CommandContext& cmd_cntx);
2323
static void GeoSearch(CmdArgList args, const CommandContext& cmd_cntx);
2424
static void GeoRadiusByMember(CmdArgList args, const CommandContext& cmd_cntx);
25+
static void GeoRadiusGeneric(CmdArgList args, const CommandContext& cmd_cntx, bool read_only);
2526
static void GeoRadius(CmdArgList args, const CommandContext& cmd_cntx);
27+
static void GeoRadiusRO(CmdArgList args, const CommandContext& cmd_cntx);
2628
};
2729

2830
} // namespace dfly

‎src/server/geo_family_test.cc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,33 @@ TEST_F(GeoFamilyTest, GeoRadius) {
317317
EXPECT_THAT(resp, ErrArg(err));
318318
}
319319

320+
TEST_F(GeoFamilyTest, GeoRadiusRO) {
321+
EXPECT_EQ(10, CheckedInt({"geoadd", "Europe", "13.4050", "52.5200", "Berlin", "3.7038",
322+
"40.4168", "Madrid", "9.1427", "38.7369", "Lisbon", "2.3522",
323+
"48.8566", "Paris", "16.3738", "48.2082", "Vienna", "4.8952",
324+
"52.3702", "Amsterdam", "10.7522", "59.9139", "Oslo", "23.7275",
325+
"37.9838", "Athens", "19.0402", "47.4979", "Budapest", "6.2603",
326+
"53.3498", "Dublin"}));
327+
328+
// GEORADIUS_RO should not accept arguments for storing (writing data)
329+
auto resp =
330+
Run({"GEORADIUS_RO", "Europe", "13.4050", "52.5200", "900", "KM", "STORE_DIST", "store_key"});
331+
EXPECT_THAT(resp, ErrArg("syntax error"));
332+
333+
resp = Run({"GEORADIUS_RO", "Europe", "13.4050", "52.5200", "900", "KM", "STORE", "store_key"});
334+
EXPECT_THAT(resp, ErrArg("syntax error"));
335+
336+
resp = Run({"GEORADIUS_RO", "Europe", "13.4050", "52.5200", "500", "KM", "COUNT", "3",
337+
"WITHCOORD", "WITHDIST"});
338+
EXPECT_THAT(
339+
resp,
340+
RespArray(ElementsAre(
341+
RespArray(ElementsAre("Berlin", DoubleArg(0.00017343178521311378),
342+
RespArray(ElementsAre(DoubleArg(13.4050), DoubleArg(52.5200))))),
343+
RespArray(ElementsAre("Dublin", DoubleArg(487.5619030644293),
344+
RespArray(ElementsAre(DoubleArg(6.2603), DoubleArg(53.3498))))))));
345+
}
346+
320347
TEST_F(GeoFamilyTest, GeoRadiusByMemberUb) {
321348
Run({"GEOADD", "geo", "-118.2437", "34.0522", "972"});
322349
Run({"GEOADD", "geo", "-73.935242", "40.730610", "973"});

0 commit comments

Comments
 (0)