From 4042bb67f9f90194708f7f83213725e521914284 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Wed, 26 Jan 2022 11:11:43 +0300 Subject: [PATCH] [search] Minor types checker code fixes. Signed-off-by: Viktor Govako --- generator/search_index_builder.cpp | 9 +- indexer/ftypes_matcher.cpp | 1 - indexer/ftypes_matcher.hpp | 7 +- search/model.cpp | 16 +-- .../search_integration_tests/smoke_test.cpp | 134 ++++++++++-------- search/types_skipper.cpp | 15 +- search/types_skipper.hpp | 2 +- 7 files changed, 94 insertions(+), 90 deletions(-) diff --git a/generator/search_index_builder.cpp b/generator/search_index_builder.cpp index 15e6819e4a..1bd2a2ef4c 100644 --- a/generator/search_index_builder.cpp +++ b/generator/search_index_builder.cpp @@ -298,15 +298,14 @@ public: if (skipIndex.SkipAlways(types)) return; - auto const isCountryOrState = [](auto types) { - auto const & isLocalityChecker = ftypes::IsLocalityChecker::Instance(); - auto const localityType = isLocalityChecker.GetType(types); + auto const isCountryOrState = [](auto types) + { + auto const localityType = ftypes::IsLocalityChecker::Instance().GetType(types); return localityType == ftypes::LocalityType::Country || localityType == ftypes::LocalityType::State; }; - auto const & streetChecker = ftypes::IsStreetOrSquareChecker::Instance(); - bool const hasStreetType = streetChecker(types); + bool const hasStreetType = ftypes::IsStreetOrSquareChecker::Instance()(types); // Init inserter with serialized value. // Insert synonyms only for countries and states (maybe will add cities in future). diff --git a/indexer/ftypes_matcher.cpp b/indexer/ftypes_matcher.cpp index 708c1e204f..43e740cc44 100644 --- a/indexer/ftypes_matcher.cpp +++ b/indexer/ftypes_matcher.cpp @@ -2,7 +2,6 @@ #include "indexer/classificator.hpp" #include "indexer/feature.hpp" -#include "indexer/feature_data.hpp" #include "base/assert.hpp" #include "base/buffer_vector.hpp" diff --git a/indexer/ftypes_matcher.hpp b/indexer/ftypes_matcher.hpp index 6de036ad6c..23871b0983 100644 --- a/indexer/ftypes_matcher.hpp +++ b/indexer/ftypes_matcher.hpp @@ -5,11 +5,9 @@ #include "base/base.hpp" #include "base/stl_helpers.hpp" -#include #include -#include #include -#include +#include #include #include #include @@ -17,9 +15,6 @@ #include #include -namespace feature { class TypesHolder; } -class FeatureType; - #define DECLARE_CHECKER_INSTANCE(CheckerType) static CheckerType const & Instance() { \ static CheckerType const inst; return inst; } diff --git a/search/model.cpp b/search/model.cpp index 5c4224a895..ea993c3f47 100644 --- a/search/model.cpp +++ b/search/model.cpp @@ -21,6 +21,8 @@ TwoLevelPOIChecker::TwoLevelPOIChecker() : ftypes::BaseChecker(2 /* level */) {"building", "train_station"}, {"emergency", "defibrillator"}, {"emergency", "fire_hydrant"}, + {"emergency", "phone"}, + {"healthcare", "laboratory"}, {"highway", "bus_stop"}, {"highway", "ford"}, {"highway", "raceway"}, @@ -31,15 +33,10 @@ TwoLevelPOIChecker::TwoLevelPOIChecker() : ftypes::BaseChecker(2 /* level */) {"natural", "cave_entrance"}, {"natural", "spring"}, {"natural", "volcano"}, - {"office", "estate_agent"}, - {"office", "government"}, - {"office", "insurance"}, - {"office", "lawyer"}, - {"office", "telecommunication"}, {"waterway", "waterfall"}}; - for (size_t i = 0; i < ARRAY_SIZE(arr); ++i) - m_types.push_back(c.GetTypeByPath(arr[i])); + for (auto const & path : arr) + m_types.push_back(c.GetTypeByPath(path)); } namespace @@ -61,8 +58,6 @@ public: class IsPoiChecker { public: - IsPoiChecker() {} - static IsPoiChecker const & Instance() { static const IsPoiChecker inst; @@ -131,9 +126,6 @@ public: { return !ft.GetHouseNumber().empty() || IsBuildingChecker::Instance()(ft); } - -private: - CustomIsBuildingChecker() {} }; } // namespace diff --git a/search/search_integration_tests/smoke_test.cpp b/search/search_integration_tests/smoke_test.cpp index ea353ace44..89f1bdec11 100644 --- a/search/search_integration_tests/smoke_test.cpp +++ b/search/search_integration_tests/smoke_test.cpp @@ -8,6 +8,8 @@ #include "generator/generator_tests_support/test_feature.hpp" #include "generator/generator_tests_support/test_mwm_builder.hpp" +#include "search/types_skipper.hpp" + #include "indexer/classificator.hpp" #include "indexer/feature_meta.hpp" @@ -25,8 +27,7 @@ using namespace std; namespace search { -namespace -{ + class IncorrectCountry : public TestCountry { public: @@ -145,69 +146,90 @@ UNIT_CLASS_TEST(SmokeTest, DeepCategoryTest) } } +UNIT_CLASS_TEST(SmokeTest, TypesSkipperTest) +{ + TypesSkipper skipper; + + base::StringIL const arr[] = { + {"barrier", "block"}, + {"barrier", "toll_booth"}, + {"entrance"} + }; + + auto const & cl = classif(); + for (auto const & path : arr) + { + feature::TypesHolder types; + types.Add(cl.GetTypeByPath(path)); + + TEST(!skipper.SkipAlways(types), (path)); + skipper.SkipEmptyNameTypes(types); + TEST_EQUAL(types.Size(), 1, ()); + } +} + UNIT_CLASS_TEST(SmokeTest, CategoriesTest) { + auto const & cl = classif(); + // todo(@t.yan): fix some or delete category. - vector> const invisibleAsPointTags = {{"waterway", "canal"}, - {"waterway", "river"}, - {"waterway", "riverbank"}, - {"waterway", "stream"}, - {"landuse", "basin"}, - {"place", "county"}, - {"place", "islet"}, - {"highway", "footway"}, - {"highway", "cycleway"}, - {"highway", "living_street"}, - {"highway", "motorway"}, - {"highway", "motorway_link"}, - {"highway", "path"}, - {"highway", "pedestrian"}, - {"highway", "primary"}, - {"highway", "primary_link"}, - {"highway", "raceway"}, - {"highway", "residential"}, - {"highway", "road"}, - {"highway", "secondary"}, - {"highway", "secondary_link"}, - {"highway", "service"}, - {"highway", "steps"}, - {"area:highway", "steps"}, - {"highway", "tertiary"}, - {"highway", "tertiary_link"}, - {"highway", "track"}, - {"highway", "traffic_signals"}, - {"highway", "trunk"}, - {"highway", "trunk_link"}, - {"highway", "unclassified"}, - {"man_made", "tower"}, - {"man_made", "water_tower"}, - {"man_made", "water_well"}, - {"natural", "glacier"}, - {"natural", "water", "pond"}, - {"natural", "tree"}}; + base::StringIL const invisibleAsPointTags[] = { + {"waterway", "canal"}, + {"waterway", "river"}, + {"waterway", "riverbank"}, + {"waterway", "stream"}, + {"landuse", "basin"}, + {"place", "county"}, + {"place", "islet"}, + {"highway", "footway"}, + {"highway", "cycleway"}, + {"highway", "living_street"}, + {"highway", "motorway"}, + {"highway", "motorway_link"}, + {"highway", "path"}, + {"highway", "pedestrian"}, + {"highway", "primary"}, + {"highway", "primary_link"}, + {"highway", "raceway"}, + {"highway", "residential"}, + {"highway", "road"}, + {"highway", "secondary"}, + {"highway", "secondary_link"}, + {"highway", "service"}, + {"highway", "steps"}, + {"area:highway", "steps"}, + {"highway", "tertiary"}, + {"highway", "tertiary_link"}, + {"highway", "track"}, + {"highway", "traffic_signals"}, + {"highway", "trunk"}, + {"highway", "trunk_link"}, + {"highway", "unclassified"}, + {"man_made", "tower"}, + {"man_made", "water_tower"}, + {"man_made", "water_well"}, + {"natural", "glacier"}, + {"natural", "water", "pond"}, + {"natural", "tree"} + }; set invisibleTypes; for (auto const & tags : invisibleAsPointTags) - invisibleTypes.insert(classif().GetTypeByPath(tags)); + invisibleTypes.insert(cl.GetTypeByPath(tags)); - vector> const notSupportedTags = {// Not visible because excluded by TypesSkipper. - {"barrier", "block"}, - {"barrier", "bollard"}, - {"barrier", "entrance"}, - {"barrier", "gate"}, - {"barrier", "lift_gate"}, - {"barrier", "stile"}, - {"barrier", "toll_booth"}, - {"building", "address"}, - {"entrance"}, - // Not visible for country scale range. - {"place", "continent"}, - {"place", "region"}}; + base::StringIL const notSupportedTags[] = { + // Not visible because excluded by TypesSkipper. + {"building", "address"}, + // Not visible for country scale range. + {"place", "continent"}, + {"place", "region"} + }; set notSupportedTypes; for (auto const & tags : notSupportedTags) - notSupportedTypes.insert(classif().GetTypeByPath(tags)); + notSupportedTypes.insert(cl.GetTypeByPath(tags)); auto const & holder = GetDefaultCategories(); - auto testCategory = [&](uint32_t type, CategoriesHolder::Category const &) { + auto testCategory = [&](uint32_t type, CategoriesHolder::Category const &) + { if (invisibleTypes.find(type) != invisibleTypes.end()) return; @@ -310,5 +332,5 @@ UNIT_CLASS_TEST(SmokeTest, PoiWithAddress) TEST(ResultsMatch("Starbucks Main street 27 ", rules), ()); } } -} // namespace + } // namespace search diff --git a/search/types_skipper.cpp b/search/types_skipper.cpp index 84894a8cfe..6cb11cec26 100644 --- a/search/types_skipper.cpp +++ b/search/types_skipper.cpp @@ -1,7 +1,7 @@ #include "search/types_skipper.hpp" +#include "search/model.hpp" #include "indexer/classificator.hpp" -#include "indexer/feature_data.hpp" #include "indexer/ftypes_matcher.hpp" #include "base/stl_helpers.hpp" @@ -18,10 +18,9 @@ TypesSkipper::TypesSkipper() StringIL const typesLengthOne[] = {{"building"}, {"highway"}, {"landuse"}, {"natural"}, {"office"}, {"waterway"}, {"area:highway"}}; + for (auto const & e : typesLengthOne) - { m_skipIfEmptyName[0].push_back(c.GetTypeByPath(e)); - } StringIL const typesLengthTwo[] = {{"man_made", "chimney"}, {"place", "country"}, @@ -33,10 +32,9 @@ TypesSkipper::TypesSkipper() {"place", "suburb"}, {"place", "neighbourhood"}, {"place", "square"}}; + for (auto const & e : typesLengthTwo) - { m_skipIfEmptyName[1].push_back(c.GetTypeByPath(e)); - } m_skipAlways[0].push_back(c.GetTypeByPath({"isoline"})); } @@ -44,7 +42,8 @@ TypesSkipper::TypesSkipper() void TypesSkipper::SkipEmptyNameTypes(feature::TypesHolder & types) const { static const TwoLevelPOIChecker dontSkip; - auto shouldBeRemoved = [this](uint32_t type) + + types.RemoveIf([this](uint32_t type) { if (dontSkip.IsMatched(type)) return false; @@ -58,9 +57,7 @@ void TypesSkipper::SkipEmptyNameTypes(feature::TypesHolder & types) const return true; return false; - }; - - types.RemoveIf(shouldBeRemoved); + }); } bool TypesSkipper::SkipAlways(feature::TypesHolder const & types) const diff --git a/search/types_skipper.hpp b/search/types_skipper.hpp index 34b1f791d0..cf9b4f4a44 100644 --- a/search/types_skipper.hpp +++ b/search/types_skipper.hpp @@ -1,6 +1,6 @@ #pragma once -#include "search/model.hpp" +#include "indexer/feature_data.hpp" #include "base/buffer_vector.hpp"