From 78576941cdc012e2e6badf38b07998475661e918 Mon Sep 17 00:00:00 2001 From: krunal Date: Mon, 28 Aug 2023 13:03:19 +0530 Subject: [PATCH 1/3] refactor --- include/core_api.h | 2 +- src/core_api.cpp | 12 ++++++------ src/main/typesense_server.cpp | 2 +- src/stopwords_manager.cpp | 14 +++++++------- test/stopwords_manager_test.cpp | 26 +++++++------------------- 5 files changed, 22 insertions(+), 34 deletions(-) diff --git a/include/core_api.h b/include/core_api.h index 6d4fe6a9..2b813f5c 100644 --- a/include/core_api.h +++ b/include/core_api.h @@ -69,7 +69,7 @@ bool get_stopwords(const std::shared_ptr& req, const std::shared_ptr& req, const std::shared_ptr& res); -bool put_upsert_stopword(const std::shared_ptr& req, const std::shared_ptr& res); +bool upsert_stopword(const std::shared_ptr& req, const std::shared_ptr& res); bool del_stopword(const std::shared_ptr& req, const std::shared_ptr& res); diff --git a/src/core_api.cpp b/src/core_api.cpp index fbd1579d..ac12d340 100644 --- a/src/core_api.cpp +++ b/src/core_api.cpp @@ -1899,9 +1899,9 @@ bool get_stopwords(const std::shared_ptr& req, const std::shared_ptr& req, const std::shared_ptr& req, const std::shared_ptr& res) { - const std::string & stopword_name = req->params["stopword_name"]; + const std::string & stopword_name = req->params["name"]; StopwordsManager& stopwordManager = StopwordsManager::get_instance(); spp::sparse_hash_set stopwords; @@ -1932,7 +1932,7 @@ bool get_stopword(const std::shared_ptr& req, const std::shared_ptr& req, const std::shared_ptr& res) { +bool upsert_stopword(const std::shared_ptr& req, const std::shared_ptr& res) { nlohmann::json req_json; try { @@ -1977,9 +1977,9 @@ bool del_stopword(const std::shared_ptr& req, const std::shared_ptrset_200(res_json.dump()); diff --git a/src/main/typesense_server.cpp b/src/main/typesense_server.cpp index 735b70e8..addaa02c 100644 --- a/src/main/typesense_server.cpp +++ b/src/main/typesense_server.cpp @@ -70,7 +70,7 @@ void master_server_routes() { server->get("/stopwords", get_stopwords); server->get("/stopwords/:name", get_stopword); - server->put("/stopwords/:name", put_upsert_stopword); + server->put("/stopwords/:name", upsert_stopword); server->del("/stopwords/:name", del_stopword); // analytics diff --git a/src/stopwords_manager.cpp b/src/stopwords_manager.cpp index f24cb8ab..3d608830 100644 --- a/src/stopwords_manager.cpp +++ b/src/stopwords_manager.cpp @@ -28,6 +28,7 @@ Option StopwordsManager::upsert_stopword(const std::string& stopword_name, const char* STOPWORD_VALUES = "stopwords"; const char* STOPWORD_LOCALE = "locale"; + bool locale_exist = false; if(stopwords_json.count(STOPWORD_VALUES) == 0){ return Option(400, (std::string("Parameter `") + STOPWORD_VALUES + "` is required")); @@ -37,12 +38,11 @@ Option StopwordsManager::upsert_stopword(const std::string& stopword_name, return Option(400, (std::string("Parameter `") + STOPWORD_VALUES + "` is required as string array value")); } - if(stopwords_json.count(STOPWORD_LOCALE) == 0) { - return Option(400, (std::string("Parameter `") + STOPWORD_LOCALE + "` is required")); - } - - if(!stopwords_json[STOPWORD_LOCALE].is_string()) { - return Option(400, (std::string("Parameter `") + STOPWORD_LOCALE + "` is required as string value")); + if(stopwords_json.count(STOPWORD_LOCALE) != 0) { + locale_exist = true; + if (!stopwords_json[STOPWORD_LOCALE].is_string()) { + return Option(400, (std::string("Parameter `") + STOPWORD_LOCALE + "` is required as string value")); + } } if(write_to_store) { @@ -55,7 +55,7 @@ Option StopwordsManager::upsert_stopword(const std::string& stopword_name, std::vector tokens; spp::sparse_hash_set stopwords_set; const auto& stopwords = stopwords_json[STOPWORD_VALUES]; - const auto& locale = stopwords_json[STOPWORD_LOCALE]; + const auto& locale = locale_exist? stopwords_json[STOPWORD_LOCALE] : ""; for (const auto &stopword: stopwords.items()) { const auto& val = stopword.value().get(); diff --git a/test/stopwords_manager_test.cpp b/test/stopwords_manager_test.cpp index 8653af19..ce02fbbe 100644 --- a/test/stopwords_manager_test.cpp +++ b/test/stopwords_manager_test.cpp @@ -211,7 +211,7 @@ TEST_F(StopwordsManagerTest, StopwordsBasics) { req->params["name"] = "articles"; req->body = stopword_value.dump(); - auto result = put_upsert_stopword(req, res); + auto result = upsert_stopword(req, res); if(!result) { LOG(ERROR) << res->body; FAIL(); @@ -247,7 +247,7 @@ TEST_F(StopwordsManagerTest, StopwordsBasics) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = put_upsert_stopword(req, res); + result = upsert_stopword(req, res); if(!result) { LOG(ERROR) << res->body; FAIL(); @@ -328,20 +328,8 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { std::shared_ptr req = std::make_shared(); std::shared_ptr res = std::make_shared(nullptr); - auto stopword_value = R"( - {"stopwords": ["america", "europe"]} - )"_json; - - req->params["collection"] = "coll1"; - req->params["name"] = "continents"; - req->body = stopword_value.dump(); - - auto result = put_upsert_stopword(req, res); - ASSERT_EQ(400, res->status_code); - ASSERT_EQ("{\"message\": \"Parameter `locale` is required\"}", res->body); - //with a typo - stopword_value = R"( + auto stopword_value = R"( {"stopword": ["america", "europe"], "locale": "en"} )"_json; @@ -349,7 +337,7 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = put_upsert_stopword(req, res); + auto result = upsert_stopword(req, res); ASSERT_EQ(400, res->status_code); ASSERT_STREQ("{\"message\": \"Parameter `stopwords` is required\"}", res->body.c_str()); @@ -362,7 +350,7 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = put_upsert_stopword(req, res); + result = upsert_stopword(req, res); ASSERT_EQ(400, res->status_code); ASSERT_STREQ("{\"message\": \"Parameter `locale` is required as string value\"}", res->body.c_str()); @@ -374,7 +362,7 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = put_upsert_stopword(req, res); + result = upsert_stopword(req, res); ASSERT_EQ(400, res->status_code); ASSERT_STREQ("{\"message\": \"Parameter `stopwords` is required as string array value\"}", res->body.c_str()); @@ -404,7 +392,7 @@ TEST_F(StopwordsManagerTest, ReloadStopwordsOnRestart) { req->params["name"] = "genre"; req->body = stopword_value.dump(); - auto result = put_upsert_stopword(req, res); + auto result = upsert_stopword(req, res); if(!result) { LOG(ERROR) << res->body; FAIL(); From d58250840460bde21d518f67a3798efd4f1d25a4 Mon Sep 17 00:00:00 2001 From: krunal Date: Mon, 28 Aug 2023 18:36:33 +0530 Subject: [PATCH 2/3] updating changes --- include/core_api.h | 2 +- src/core_api.cpp | 2 +- src/main/typesense_server.cpp | 2 +- src/stopwords_manager.cpp | 5 ++--- test/stopwords_manager_test.cpp | 12 ++++++------ 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/include/core_api.h b/include/core_api.h index 2b813f5c..6d4fe6a9 100644 --- a/include/core_api.h +++ b/include/core_api.h @@ -69,7 +69,7 @@ bool get_stopwords(const std::shared_ptr& req, const std::shared_ptr& req, const std::shared_ptr& res); -bool upsert_stopword(const std::shared_ptr& req, const std::shared_ptr& res); +bool put_upsert_stopword(const std::shared_ptr& req, const std::shared_ptr& res); bool del_stopword(const std::shared_ptr& req, const std::shared_ptr& res); diff --git a/src/core_api.cpp b/src/core_api.cpp index ac12d340..5b3b8bea 100644 --- a/src/core_api.cpp +++ b/src/core_api.cpp @@ -1932,7 +1932,7 @@ bool get_stopword(const std::shared_ptr& req, const std::shared_ptr& req, const std::shared_ptr& res) { +bool put_upsert_stopword(const std::shared_ptr& req, const std::shared_ptr& res) { nlohmann::json req_json; try { diff --git a/src/main/typesense_server.cpp b/src/main/typesense_server.cpp index addaa02c..735b70e8 100644 --- a/src/main/typesense_server.cpp +++ b/src/main/typesense_server.cpp @@ -70,7 +70,7 @@ void master_server_routes() { server->get("/stopwords", get_stopwords); server->get("/stopwords/:name", get_stopword); - server->put("/stopwords/:name", upsert_stopword); + server->put("/stopwords/:name", put_upsert_stopword); server->del("/stopwords/:name", del_stopword); // analytics diff --git a/src/stopwords_manager.cpp b/src/stopwords_manager.cpp index 3d608830..af72c39f 100644 --- a/src/stopwords_manager.cpp +++ b/src/stopwords_manager.cpp @@ -28,7 +28,7 @@ Option StopwordsManager::upsert_stopword(const std::string& stopword_name, const char* STOPWORD_VALUES = "stopwords"; const char* STOPWORD_LOCALE = "locale"; - bool locale_exist = false; + std::string locale = ""; if(stopwords_json.count(STOPWORD_VALUES) == 0){ return Option(400, (std::string("Parameter `") + STOPWORD_VALUES + "` is required")); @@ -39,10 +39,10 @@ Option StopwordsManager::upsert_stopword(const std::string& stopword_name, } if(stopwords_json.count(STOPWORD_LOCALE) != 0) { - locale_exist = true; if (!stopwords_json[STOPWORD_LOCALE].is_string()) { return Option(400, (std::string("Parameter `") + STOPWORD_LOCALE + "` is required as string value")); } + locale = stopwords_json[STOPWORD_LOCALE]; } if(write_to_store) { @@ -55,7 +55,6 @@ Option StopwordsManager::upsert_stopword(const std::string& stopword_name, std::vector tokens; spp::sparse_hash_set stopwords_set; const auto& stopwords = stopwords_json[STOPWORD_VALUES]; - const auto& locale = locale_exist? stopwords_json[STOPWORD_LOCALE] : ""; for (const auto &stopword: stopwords.items()) { const auto& val = stopword.value().get(); diff --git a/test/stopwords_manager_test.cpp b/test/stopwords_manager_test.cpp index ce02fbbe..bb06eb10 100644 --- a/test/stopwords_manager_test.cpp +++ b/test/stopwords_manager_test.cpp @@ -211,7 +211,7 @@ TEST_F(StopwordsManagerTest, StopwordsBasics) { req->params["name"] = "articles"; req->body = stopword_value.dump(); - auto result = upsert_stopword(req, res); + auto result = put_upsert_stopword(req, res); if(!result) { LOG(ERROR) << res->body; FAIL(); @@ -247,7 +247,7 @@ TEST_F(StopwordsManagerTest, StopwordsBasics) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = upsert_stopword(req, res); + result = put_upsert_stopword(req, res); if(!result) { LOG(ERROR) << res->body; FAIL(); @@ -337,7 +337,7 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - auto result = upsert_stopword(req, res); + auto result = put_upsert_stopword(req, res); ASSERT_EQ(400, res->status_code); ASSERT_STREQ("{\"message\": \"Parameter `stopwords` is required\"}", res->body.c_str()); @@ -350,7 +350,7 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = upsert_stopword(req, res); + result = put_upsert_stopword(req, res); ASSERT_EQ(400, res->status_code); ASSERT_STREQ("{\"message\": \"Parameter `locale` is required as string value\"}", res->body.c_str()); @@ -362,7 +362,7 @@ TEST_F(StopwordsManagerTest, StopwordsValidation) { req->params["name"] = "continents"; req->body = stopword_value.dump(); - result = upsert_stopword(req, res); + result = put_upsert_stopword(req, res); ASSERT_EQ(400, res->status_code); ASSERT_STREQ("{\"message\": \"Parameter `stopwords` is required as string array value\"}", res->body.c_str()); @@ -392,7 +392,7 @@ TEST_F(StopwordsManagerTest, ReloadStopwordsOnRestart) { req->params["name"] = "genre"; req->body = stopword_value.dump(); - auto result = upsert_stopword(req, res); + auto result = put_upsert_stopword(req, res); if(!result) { LOG(ERROR) << res->body; FAIL(); From a3d976b535d367b3ac6935a0400ae73f1ca90d61 Mon Sep 17 00:00:00 2001 From: Harpreet Sangar Date: Tue, 29 Aug 2023 10:21:39 +0530 Subject: [PATCH 3/3] Fix `_eval` parsing. #1089 --- src/collection_manager.cpp | 39 ++++++++++++++++++++++++++++++++ test/collection_sorting_test.cpp | 16 +++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/collection_manager.cpp b/src/collection_manager.cpp index 481e69b5..e0ede7dd 100644 --- a/src/collection_manager.cpp +++ b/src/collection_manager.cpp @@ -623,6 +623,45 @@ bool CollectionManager::parse_sort_by_str(std::string sort_by_str, std::vector 0) { + if (sort_by_str[i] == '(') { + paren_count++; + } else if (sort_by_str[i] == ')') { + paren_count--; + } + sort_field_expr += sort_by_str[i]; + } + if (paren_count != 0 || i >= sort_by_str.size()) { + return false; + } + + while (sort_by_str[i] != ':' && ++i < sort_by_str.size()); + if (i >= sort_by_str.size()) { + return false; + } + + std::string order_str; + while (++i < sort_by_str.size() && sort_by_str[i] != ',') { + order_str += sort_by_str[i]; + } + StringUtils::trim(order_str); + StringUtils::toupper(order_str); + + sort_fields.emplace_back(sort_field_expr, order_str); + sort_field_expr = ""; + continue; + } + if(i == sort_by_str.size()-1 || (sort_by_str[i] == ',' && !isdigit(prev_non_space_char))) { if(i == sort_by_str.size()-1) { sort_field_expr += sort_by_str[i]; diff --git a/test/collection_sorting_test.cpp b/test/collection_sorting_test.cpp index e7b354b9..4ae097d0 100644 --- a/test/collection_sorting_test.cpp +++ b/test/collection_sorting_test.cpp @@ -2135,6 +2135,22 @@ TEST_F(CollectionSortingTest, OptionalFilteringViaSortingSearch) { results = coll1->search("title", {"title"}, "", {}, sort_fields, {2}, 10, 1, FREQUENCY, {true}, 10).get(); ASSERT_EQ(5, results["hits"].size()); + std::map req_params = { + {"collection", "coll1"}, + {"q", "title"}, + {"query_by", "title"}, + {"sort_by", "_eval(brand:[nike, adidas] && points:0):desc, points:DESC"} + }; + nlohmann::json embedded_params; + std::string json_res; + auto now_ts = std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch()).count(); + + auto search_op = collectionManager.do_search(req_params, embedded_params, json_res, now_ts); + ASSERT_TRUE(search_op.ok()); + results = nlohmann::json::parse(json_res); + ASSERT_EQ(5, results["hits"].size()); + expected_ids = {"0", "4", "3", "2", "1"}; for(size_t i = 0; i < expected_ids.size(); i++) { ASSERT_EQ(expected_ids[i], results["hits"][i]["document"]["id"].get());