Override pagination (#1588)

* add limit and offset support for collection

* use req->params, dont restrict limit param when records are less

* add more checks and not sort with pagination

* minor updations

* pagination support for overrides

include/collection.h

@@ -620,10 +620,9 @@ public:
 
     Option<uint32_t> remove_override(const std::string & id);
 
-    std::map<std::string, override_t> get_overrides() {
-        std::shared_lock lock(mutex);
-        return overrides;
-    };
+    Option<std::map<std::string, override_t*>> get_overrides(uint32_t limit=0, uint32_t offset=0);
+
+    Option<override_t> get_override(const std::string& override_id);
 
     // synonym operations
 

src/collection.cpp

@@ -4712,6 +4712,44 @@ void Collection::synonym_reduction(const std::vector<std::string>& tokens,
     return synonym_index->synonym_reduction(tokens, results);
 }
 
+Option<override_t> Collection::get_override(const std::string& override_id) {
+    std::shared_lock lock(mutex);
+
+    if(overrides.count(override_id) == 0) {
+        return Option<override_t>(404, "override " + override_id + " not found.");
+    }
+
+    return Option<override_t>(overrides.at(override_id));
+}
+
+Option<std::map<std::string, override_t*>> Collection::get_overrides(uint32_t limit, uint32_t offset) {
+    std::shared_lock lock(mutex);
+    std::map<std::string, override_t*> overrides_map;
+
+    auto overrides_it = overrides.begin();
+
+    if(offset > 0) {
+        if(offset >= overrides.size()) {
+            return Option<std::map<std::string, override_t*>>(400, "Invalid offset param.");
+        }
+
+        std::advance(overrides_it, offset);
+    }
+
+    auto overrides_end = overrides.end();
+
+    if(limit > 0 && (offset + limit < overrides.size())) {
+        overrides_end = overrides_it;
+        std::advance(overrides_end, limit);
+    }
+
+    for (overrides_it; overrides_it != overrides_end; ++overrides_it) {
+        overrides_map[overrides_it->first] = &overrides_it->second;
+    }
+
+    return Option<std::map<std::string, override_t*>>(overrides_map);
+}
+
 spp::sparse_hash_map<std::string, synonym_t> Collection::get_synonyms() {
     std::shared_lock lock(mutex);
     return synonym_index->get_synonyms();

src/collection_manager.cpp

@@ -2351,9 +2351,9 @@ Option<Collection*> CollectionManager::clone_collection(const string& existing_n
     }
 
     // copy overrides
-    auto overrides = existing_coll->get_overrides();
-    for(const auto& override: overrides) {
-        new_coll->add_override(override.second);
+    auto overrides = existing_coll->get_overrides().get();
+    for(const auto& kv: overrides) {
+        new_coll->add_override(*kv.second);
     }
 
     return Option<Collection*>(new_coll);

src/core_api.cpp

@@ -1697,13 +1697,38 @@ bool get_overrides(const std::shared_ptr<http_req>& req, const std::shared_ptr<h
         return false;
     }
 
+    uint32_t offset = 0, limit = 0;
+    if(req->params.count("offset") != 0) {
+        const auto &offset_str = req->params["offset"];
+        if(!StringUtils::is_uint32_t(offset_str)) {
+            res->set(400, "Offset param should be unsigned integer.");
+            return false;
+        }
+        offset = std::stoi(offset_str);
+    }
+
+    if(req->params.count("limit") != 0) {
+        const auto &limit_str = req->params["limit"];
+        if(!StringUtils::is_uint32_t(limit_str)) {
+            res->set(400, "Limit param should be unsigned integer.");
+            return false;
+        }
+        limit = std::stoi(limit_str);
+    }
+
     nlohmann::json res_json;
     res_json["overrides"] = nlohmann::json::array();
 
-    const std::map<std::string, override_t>& overrides = collection->get_overrides();
-    for(const auto & kv: overrides) {
-        nlohmann::json override = kv.second.to_json();
-        res_json["overrides"].push_back(override);
+    auto overrides_op = collection->get_overrides(limit, offset);
+    if(!overrides_op.ok()) {
+        res->set(overrides_op.code(), overrides_op.error());
+        return false;
+    }
+
+    const auto overrides = overrides_op.get();
+
+    for(const auto &kv: overrides) {
+        res_json["overrides"].push_back(kv.second->to_json());
     }
 
     res->set_200(res_json.dump());
@@ -1721,16 +1746,15 @@ bool get_override(const std::shared_ptr<http_req>& req, const std::shared_ptr<ht
 
     std::string override_id = req->params["id"];
 
-    const std::map<std::string, override_t>& overrides = collection->get_overrides();
+    auto overrides_op = collection->get_override(override_id);
 
-    if(overrides.count(override_id) != 0) {
-        nlohmann::json override = overrides.at(override_id).to_json();
-        res->set_200(override.dump());
-        return true;
+    if(!overrides_op.ok()) {
+        res->set(overrides_op.code(), overrides_op.error());
+        return false;
     }
 
-    res->set_404();
-    return false;
+    res->set_200(overrides_op.get().to_json());
+    return true;
 }
 
 bool put_override(const std::shared_ptr<http_req>& req, const std::shared_ptr<http_res>& res) {

test/collection_manager_test.cpp

@@ -521,9 +521,9 @@ TEST_F(CollectionManagerTest, RestoreRecordsOnRestart) {
 
     ASSERT_TRUE(collection1->get_enable_nested_fields());
 
-    ASSERT_EQ(2, collection1->get_overrides().size());
-    ASSERT_STREQ("exclude-rule", collection1->get_overrides()["exclude-rule"].id.c_str());
-    ASSERT_STREQ("include-rule", collection1->get_overrides()["include-rule"].id.c_str());
+    ASSERT_EQ(2, collection1->get_overrides().get().size());
+    ASSERT_STREQ("exclude-rule", collection1->get_overrides().get()["exclude-rule"]->id.c_str());
+    ASSERT_STREQ("include-rule", collection1->get_overrides().get()["include-rule"]->id.c_str());
 
     const auto& synonyms = collection1->get_synonyms();
     ASSERT_EQ(2, synonyms.size());
@@ -1409,7 +1409,7 @@ TEST_F(CollectionManagerTest, CloneCollection) {
     ASSERT_EQ("coll2", coll2->get_name());
     ASSERT_EQ(1, coll2->get_fields().size());
     ASSERT_EQ(1, coll2->get_synonyms().size());
-    ASSERT_EQ(1, coll2->get_overrides().size());
+    ASSERT_EQ(1, coll2->get_overrides().get().size());
     ASSERT_EQ("", coll2->get_fallback_field_type());
 
     ASSERT_EQ(1, coll2->get_symbols_to_index().size());
@@ -2023,4 +2023,4 @@ TEST_F(CollectionManagerTest, CollectionPagination) {
     collection_op = collectionManager.get_collections(limit, offset);
     ASSERT_FALSE(collection_op.ok());
     ASSERT_EQ("Invalid offset param.", collection_op.error());
-}
+}

test/collection_override_test.cpp

@@ -331,9 +331,9 @@ TEST_F(CollectionOverrideTest, IncludeHitsFilterOverrides) {
     override_t::parse(override_json_include, "", override_include);
     coll_mul_fields->add_override(override_include);
 
-    std::map<std::string, override_t> overrides = coll_mul_fields->get_overrides();
+    std::map<std::string, override_t*> overrides = coll_mul_fields->get_overrides().get();
     ASSERT_EQ(1, overrides.size());
-    auto override_json = overrides["include-rule"].to_json();
+    auto override_json = overrides.at("include-rule")->to_json();
     ASSERT_TRUE(override_json.contains("filter_curated_hits"));
     ASSERT_TRUE(override_json["filter_curated_hits"].get<bool>());
 
@@ -440,9 +440,9 @@ TEST_F(CollectionOverrideTest, ExcludeIncludeFacetFilterQuery) {
 
     coll_mul_fields->add_override(override_include);
 
-    std::map<std::string, override_t> overrides = coll_mul_fields->get_overrides();
+    std::map<std::string, override_t*> overrides = coll_mul_fields->get_overrides().get();
     ASSERT_EQ(1, overrides.size());
-    auto override_json = overrides["include-rule"].to_json();
+    auto override_json = overrides.at("include-rule")->to_json();
     ASSERT_FALSE(override_json.contains("filter_by"));
     ASSERT_TRUE(override_json.contains("remove_matched_tokens"));
     ASSERT_TRUE(override_json.contains("filter_curated_hits"));
@@ -520,7 +520,7 @@ TEST_F(CollectionOverrideTest, ExcludeIncludeFacetFilterQuery) {
     // should be able to replace existing override
     override_include.rule.query = "found";
     coll_mul_fields->add_override(override_include);
-    ASSERT_STREQ("found", coll_mul_fields->get_overrides()["include-rule"].rule.query.c_str());
+    ASSERT_STREQ("found", coll_mul_fields->get_overrides().get()["include-rule"]->rule.query.c_str());
 
     coll_mul_fields->remove_override("include-rule");
 }
@@ -2450,9 +2450,9 @@ TEST_F(CollectionOverrideTest, DynamicFilteringWithSynonyms) {
     ASSERT_TRUE(op.ok());
     coll1->add_override(override1);
 
-    std::map<std::string, override_t> overrides = coll1->get_overrides();
+    std::map<std::string, override_t*> overrides = coll1->get_overrides().get();
     ASSERT_EQ(1, overrides.size());
-    auto override_json = overrides["dynamic-filters"].to_json();
+    auto override_json = overrides.at("dynamic-filters")->to_json();
     ASSERT_EQ("category: {category}", override_json["filter_by"].get<std::string>());
     ASSERT_EQ(true, override_json["remove_matched_tokens"].get<bool>());  // must be true by default
 
@@ -4023,3 +4023,103 @@ TEST_F(CollectionOverrideTest, WildcardSearchOverride) {
 
     collectionManager.drop_collection("coll1");
 }
+
+TEST_F(CollectionOverrideTest, OverridesPagination) {
+    Collection *coll2;
+
+    std::vector<field> fields = {field("title", field_types::STRING, false),
+                                 field("points", field_types::INT32, false)};
+
+    coll2 = collectionManager.get_collection("coll2").get();
+    if(coll2 == nullptr) {
+        coll2 = collectionManager.create_collection("coll2", 1, fields, "points").get();
+    }
+
+    for(int i = 0; i < 5; ++i) {
+        nlohmann::json override_json = {
+                {"id",       "override"},
+                {
+                 "rule",     {
+                                     {"query", "not-found"},
+                                     {"match", override_t::MATCH_EXACT}
+                             }
+                },
+                {"metadata", {       {"foo",   "bar"}}},
+        };
+
+        override_json["id"] = override_json["id"].get<std::string>() + std::to_string(i + 1);
+        override_t override;
+        override_t::parse(override_json, "", override);
+
+        coll2->add_override(override);
+    }
+
+    uint32_t limit = 0, offset = 0, i = 0;
+
+    //limit collections by 2
+    limit=2;
+    auto override_op = coll2->get_overrides(limit);
+    auto override_map = override_op.get();
+    ASSERT_EQ(2, override_map.size());
+    i=offset;
+    for(const auto &kv : override_map) {
+        ASSERT_EQ("override" + std::to_string(i+1), kv.second->id);
+        ++i;
+    }
+
+    //get 2 collection from offset 3
+    offset=3;
+    override_op = coll2->get_overrides(limit, offset);
+    override_map = override_op.get();
+    ASSERT_EQ(2, override_map.size());
+    i=offset;
+    for(const auto &kv : override_map) {
+        ASSERT_EQ("override" + std::to_string(i+1), kv.second->id);
+        ++i;
+    }
+
+    //get all collection except first
+    offset=1; limit=0;
+    override_op = coll2->get_overrides(limit, offset);
+    override_map = override_op.get();
+    ASSERT_EQ(4, override_map.size());
+    i=offset;
+    for(const auto &kv : override_map) {
+        ASSERT_EQ("override" + std::to_string(i+1), kv.second->id);
+        ++i;
+    }
+
+    //get last collection
+    offset=4, limit=1;
+    override_op = coll2->get_overrides(limit, offset);
+    override_map = override_op.get();
+    ASSERT_EQ(1, override_map.size());
+    ASSERT_EQ("override5", override_map.begin()->second->id);
+
+    //if limit is greater than number of collection then return all from offset
+    offset=0; limit=8;
+    override_op = coll2->get_overrides(limit, offset);
+    override_map = override_op.get();
+    ASSERT_EQ(5, override_map.size());
+    i=offset;
+    for(const auto &kv : override_map) {
+        ASSERT_EQ("override" + std::to_string(i+1), kv.second->id);
+        ++i;
+    }
+
+    offset=3; limit=4;
+    override_op = coll2->get_overrides(limit, offset);
+    override_map = override_op.get();
+    ASSERT_EQ(2, override_map.size());
+    i=offset;
+    for(const auto &kv : override_map) {
+        ASSERT_EQ("override" + std::to_string(i+1), kv.second->id);
+        ++i;
+    }
+
+    //invalid offset
+    offset=6; limit=0;
+    override_op = coll2->get_overrides(limit, offset);
+    ASSERT_FALSE(override_op.ok());
+    ASSERT_EQ("Invalid offset param.", override_op.error());
+}

test/core_api_utils_test.cpp

@@ -1622,6 +1622,78 @@ TEST_F(CoreAPIUtilsTest, CollectionsPagination) {
     ASSERT_EQ(400, resp->status_code);
     ASSERT_EQ("{\"message\": \"Offset param should be unsigned integer.\"}", resp->body);
 
+    //invalid limit string
+    req->params["offset"] = "0";
+    req->params["limit"] = "-1";
+    get_collections(req, resp);
+    ASSERT_EQ(400, resp->status_code);
+    ASSERT_EQ("{\"message\": \"Limit param should be unsigned integer.\"}", resp->body);
+}
+
+TEST_F(CoreAPIUtilsTest, OverridesPagination) {
+    Collection *coll2;
+
+    std::vector<field> fields = {field("title", field_types::STRING, false),
+                                 field("points", field_types::INT32, false)};
+
+    coll2 = collectionManager.get_collection("coll2").get();
+    if(coll2 == nullptr) {
+        coll2 = collectionManager.create_collection("coll2", 1, fields, "points").get();
+    }
+
+    for(int i = 0; i < 5; ++i) {
+        nlohmann::json override_json = {
+                {"id",       "override"},
+                {
+                 "rule",     {
+                                     {"query", "not-found"},
+                                     {"match", override_t::MATCH_EXACT}
+                             }
+                },
+                {"metadata", {       {"foo",   "bar"}}},
+        };
+
+        override_json["id"] = override_json["id"].get<std::string>() + std::to_string(i + 1);
+        override_t override;
+        override_t::parse(override_json, "", override);
+
+        coll2->add_override(override);
+    }
+
+    auto req = std::make_shared<http_req>();
+    auto resp = std::make_shared<http_res>(nullptr);
+
+    req->params["collection"] = "coll2";
+    req->params["offset"] = "0";
+    req->params["limit"] = "1";
+
+    get_overrides(req, resp);
+    nlohmann::json expected_json = R"({
+        "overrides":[
+                    {
+                        "excludes":[],
+                        "filter_curated_hits":false,
+                        "id":"override1",
+                        "includes":[],
+                        "metadata":{"foo":"bar"},
+                        "remove_matched_tokens":false,
+                        "rule":{
+                                "match":"exact",
+                                "query":"not-found"
+                        },
+                        "stop_processing":true
+                    }]
+    })"_json;
+
+    ASSERT_EQ(expected_json.dump(), resp->body);
+
+    //invalid offset string
+    req->params["offset"] = "0a";
+    get_collections(req, resp);
+
+    ASSERT_EQ(400, resp->status_code);
+    ASSERT_EQ("{\"message\": \"Offset param should be unsigned integer.\"}", resp->body);
+
     //invalid limit string
     req->params["offset"] = "0";
     req->params["limit"] = "-1";