From 0b28e9dc2cfef1b7edff4999689f6ae080ec33ff Mon Sep 17 00:00:00 2001 From: Kishore Nallan Date: Fri, 2 Jul 2021 10:35:31 +0530 Subject: [PATCH] Remove deprecated default_resolution config for geo fields. --- include/field.h | 42 +++-------------------------- src/collection.cpp | 4 --- src/collection_manager.cpp | 8 +----- test/collection_all_fields_test.cpp | 24 ----------------- test/collection_locale_test.cpp | 20 +++++++------- test/collection_manager_test.cpp | 8 +++--- 6 files changed, 19 insertions(+), 87 deletions(-) diff --git a/include/field.h b/include/field.h index ae637dca..8d9cbf83 100644 --- a/include/field.h +++ b/include/field.h @@ -34,13 +34,9 @@ namespace fields { static const std::string facet = "facet"; static const std::string optional = "optional"; static const std::string index = "index"; - static const std::string geo_resolution = "geo_resolution"; static const std::string locale = "locale"; } -static const uint8_t DEFAULT_GEO_RESOLUTION = 7; -static const uint8_t FINEST_GEO_RESOLUTION = 15; - struct field { std::string name; std::string type; @@ -48,15 +44,11 @@ struct field { bool optional; bool index; - uint8_t geo_resolution; - std::string locale; field(const std::string &name, const std::string &type, const bool facet, const bool optional = false, - bool index = true, const uint8_t geo_resolution = DEFAULT_GEO_RESOLUTION, - std::string locale = "") : - name(name), type(type), facet(facet), optional(optional), index(index), - geo_resolution(geo_resolution), locale(locale) { + bool index = true, std::string locale = "") : + name(name), type(type), facet(facet), optional(optional), index(index), locale(locale) { } @@ -205,9 +197,6 @@ struct field { field_val[fields::type] = field.type; field_val[fields::facet] = field.facet; field_val[fields::optional] = field.optional; - if(field.is_geopoint()) { - field_val[fields::geo_resolution] = field.geo_resolution; - } field_val[fields::locale] = field.locale; @@ -306,19 +295,6 @@ struct field { } } - if(field_json.count(fields::geo_resolution) != 0) { - if(!field_json.at(fields::geo_resolution).is_number_integer()) { - return Option(400, std::string("The `geo_resolution` property of the field `") + - field_json[fields::name].get() + std::string("` should be an integer.")); - } - - int field_geo_res = field_json.at(fields::geo_resolution).get(); - if(field_geo_res < 0 || field_geo_res > 15) { - return Option(400, std::string("The `geo_resolution` property of the field `") + - field_json[fields::name].get() + std::string("` should be between 0 and 15.")); - } - } - if(field_json.count(fields::locale) != 0){ if(!field_json.at(fields::locale).is_string()) { return Option(400, std::string("The `locale` property of the field `") + @@ -350,10 +326,6 @@ struct field { field_json[fields::locale] = ""; } - if(field_json.count(fields::geo_resolution) != 0) { - return Option(400, "Field `.*` cannot contain a geo resolution."); - } - if(field_json[fields::optional] == false) { return Option(400, "Field `.*` must be an optional field."); } @@ -367,8 +339,7 @@ struct field { } field fallback_field(field_json["name"], field_json["type"], field_json["facet"], - field_json["optional"], field_json[fields::index], - DEFAULT_GEO_RESOLUTION, field_json[fields::locale]); + field_json["optional"], field_json[fields::index], field_json[fields::locale]); if(fallback_field.has_valid_type()) { fallback_field_type = fallback_field.type; @@ -399,14 +370,9 @@ struct field { field_json[fields::optional] = is_dynamic; } - if(field_json.count(fields::geo_resolution) == 0) { - field_json[fields::geo_resolution] = DEFAULT_GEO_RESOLUTION; - } - fields.emplace_back( field(field_json[fields::name], field_json[fields::type], field_json[fields::facet], - field_json[fields::optional], field_json[fields::index], - field_json[fields::geo_resolution], field_json[fields::locale]) + field_json[fields::optional], field_json[fields::index], field_json[fields::locale]) ); } diff --git a/src/collection.cpp b/src/collection.cpp index c80ef82c..4ea1b113 100644 --- a/src/collection.cpp +++ b/src/collection.cpp @@ -161,10 +161,6 @@ nlohmann::json Collection::get_summary_json() const { field_json[fields::optional] = coll_field.optional; field_json[fields::index] = coll_field.index; - if(coll_field.is_geopoint()) { - field_json[fields::geo_resolution] = size_t(coll_field.geo_resolution); - } - fields_arr.push_back(field_json); } diff --git a/src/collection_manager.cpp b/src/collection_manager.cpp index d50e856d..dc81f1ad 100644 --- a/src/collection_manager.cpp +++ b/src/collection_manager.cpp @@ -31,18 +31,12 @@ Collection* CollectionManager::init_collection(const nlohmann::json & collection field_obj[fields::index] = true; } - // handle older records indexed before geo_resolution field introduction - if(field_obj.count(fields::geo_resolution) == 0) { - field_obj[fields::geo_resolution] = size_t(DEFAULT_GEO_RESOLUTION); - } - if(field_obj.count(fields::locale) == 0) { field_obj[fields::locale] = ""; } fields.push_back({field_obj[fields::name], field_obj[fields::type], field_obj[fields::facet], - field_obj[fields::optional], field_obj[fields::index], - field_obj[fields::geo_resolution], field_obj[fields::locale]}); + field_obj[fields::optional], field_obj[fields::index], field_obj[fields::locale]}); } std::string default_sorting_field = collection_meta[Collection::COLLECTION_DEFAULT_SORTING_FIELD_KEY].get(); diff --git a/test/collection_all_fields_test.cpp b/test/collection_all_fields_test.cpp index 9f77ac27..8deb28fe 100644 --- a/test/collection_all_fields_test.cpp +++ b/test/collection_all_fields_test.cpp @@ -583,14 +583,6 @@ TEST_F(CollectionAllFieldsTest, JsonFieldsToFieldsConversion) { fields_json = nlohmann::json::array(); fields_json.emplace_back(all_field); - // reject when you try to set geo property on * field - fields_json[0][fields::geo_resolution] = 10; - parse_op = field::json_fields_to_fields(fields_json, fallback_field_type, fields); - - ASSERT_FALSE(parse_op.ok()); - ASSERT_EQ("Field `.*` cannot contain a geo resolution.", parse_op.error()); - fields_json[0].erase(fields::geo_resolution); - // reject when you try to set optional to false or facet to true fields_json[0][fields::optional] = false; parse_op = field::json_fields_to_fields(fields_json, fallback_field_type, fields); @@ -648,22 +640,6 @@ TEST_F(CollectionAllFieldsTest, JsonFieldsToFieldsConversion) { parse_op = field::json_fields_to_fields(fields_json, fallback_field_type, fields); ASSERT_TRUE(parse_op.ok()); ASSERT_EQ("ko", fields[0].locale); - - fields_json.clear(); - all_field[fields::name] = "loc"; - all_field[fields::type] = "geopoint"; - all_field[fields::geo_resolution] = "blah"; - fields_json.emplace_back(all_field); - parse_op = field::json_fields_to_fields(fields_json, fallback_field_type, fields); - ASSERT_FALSE(parse_op.ok()); - ASSERT_EQ("The `geo_resolution` property of the field `loc` should be an integer.", parse_op.error()); - - fields_json.clear(); - all_field[fields::geo_resolution] = 24; - fields_json.emplace_back(all_field); - parse_op = field::json_fields_to_fields(fields_json, fallback_field_type, fields); - ASSERT_FALSE(parse_op.ok()); - ASSERT_EQ("The `geo_resolution` property of the field `loc` should be between 0 and 15.", parse_op.error()); } TEST_F(CollectionAllFieldsTest, WildcardFacetFieldsOnAutoSchema) { diff --git a/test/collection_locale_test.cpp b/test/collection_locale_test.cpp index e9b0270a..c6f5b63a 100644 --- a/test/collection_locale_test.cpp +++ b/test/collection_locale_test.cpp @@ -33,7 +33,7 @@ protected: TEST_F(CollectionLocaleTest, SearchAgainstJapaneseText) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "ja"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "ja"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -70,7 +70,7 @@ TEST_F(CollectionLocaleTest, SearchAgainstJapaneseText) { TEST_F(CollectionLocaleTest, SearchAgainstChineseText) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "zh"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "zh"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -134,7 +134,7 @@ TEST_F(CollectionLocaleTest, SearchAgainstChineseText) { TEST_F(CollectionLocaleTest, SearchAgainstThaiText) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "th"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "th"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -189,7 +189,7 @@ TEST_F(CollectionLocaleTest, SearchAgainstThaiText) { TEST_F(CollectionLocaleTest, SearchThaiTextPreSegmentedQuery) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "th"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "th"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -219,7 +219,7 @@ TEST_F(CollectionLocaleTest, SearchThaiTextPreSegmentedQuery) { {"title"}, "", {}, {}, {0}, 10, 1, FREQUENCY, {true}, 10, spp::sparse_hash_set(), spp::sparse_hash_set(), 10, "", 30, 4, "", 40, {}, {}, {}, 0, - "", "", {1}, 1000, true, {true}).get(); + "", "", {1}, 1000, true, true).get(); ASSERT_EQ(1, results["found"].get()); ASSERT_EQ("0", results["hits"][0]["document"]["id"].get()); @@ -228,7 +228,7 @@ TEST_F(CollectionLocaleTest, SearchThaiTextPreSegmentedQuery) { TEST_F(CollectionLocaleTest, SearchAgainstThaiTextExactMatch) { Collection* coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "th"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "th"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -271,7 +271,7 @@ TEST_F(CollectionLocaleTest, SearchAgainstThaiTextExactMatch) { TEST_F(CollectionLocaleTest, SearchAgainstKoreanText) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "ko"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "ko"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -316,7 +316,7 @@ TEST_F(CollectionLocaleTest, SearchAgainstKoreanText) { TEST_F(CollectionLocaleTest, KoreanTextPrefixConsonant) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "ko"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "ko"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -375,7 +375,7 @@ TEST_F(CollectionLocaleTest, KoreanTextPrefixConsonant) { TEST_F(CollectionLocaleTest, KoreanTextPrefixVowel) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "ko"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "ko"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; @@ -417,7 +417,7 @@ TEST_F(CollectionLocaleTest, KoreanTextPrefixVowel) { TEST_F(CollectionLocaleTest, SearchAgainstKoreanTextContainingEnglishChars) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "th"), + std::vector fields = {field("title", field_types::STRING, false, false, true, "th"), field("artist", field_types::STRING, false), field("points", field_types::INT32, false),}; diff --git a/test/collection_manager_test.cpp b/test/collection_manager_test.cpp index 5ee19b87..8b3f507a 100644 --- a/test/collection_manager_test.cpp +++ b/test/collection_manager_test.cpp @@ -24,11 +24,11 @@ protected: collectionManager.load(8, 1000); search_fields = { - field("title", field_types::STRING, false, false, true, DEFAULT_GEO_RESOLUTION, "en"), + field("title", field_types::STRING, false, false, true, "en"), field("starring", field_types::STRING, false), field("cast", field_types::STRING_ARRAY, true, true), field(".*_year", field_types::INT32, true, true), - field("location", field_types::GEOPOINT, false, true, true, 14), + field("location", field_types::GEOPOINT, false, true, true), field("points", field_types::INT32, false) }; @@ -92,7 +92,7 @@ TEST_F(CollectionManagerTest, CollectionCreation) { "{\"facet\":false,\"locale\":\"\",\"name\":\"starring\",\"optional\":false,\"type\":\"string\"}," "{\"facet\":true,\"locale\":\"\",\"name\":\"cast\",\"optional\":true,\"type\":\"string[]\"}," "{\"facet\":true,\"locale\":\"\",\"name\":\".*_year\",\"optional\":true,\"type\":\"int32\"}," - "{\"facet\":false,\"geo_resolution\":14,\"locale\":\"\",\"name\":\"location\",\"optional\":true,\"type\":\"geopoint\"}," + "{\"facet\":false,\"locale\":\"\",\"name\":\"location\",\"optional\":true,\"type\":\"geopoint\"}," "{\"facet\":false,\"locale\":\"\",\"name\":\"points\",\"optional\":false,\"type\":\"int32\"}],\"id\":0," "\"name\":\"collection1\",\"num_memory_shards\":4}", collection_meta_json); @@ -545,7 +545,7 @@ TEST_F(CollectionManagerTest, LoadMultipleCollections) { field("starring", field_types::STRING, false), field("cast", field_types::STRING_ARRAY, true, true), field(".*_year", field_types::INT32, true, true), - field("location", field_types::GEOPOINT, false, true, true, 14), + field("location", field_types::GEOPOINT, false, true, true), field("points", field_types::INT32, false) };