From 32afdafc52b56ccdd1d9d453ebf8bd7485ab56cc Mon Sep 17 00:00:00 2001 From: Maxim Pimenov Date: Tue, 2 Jul 2019 02:31:25 +0300 Subject: [PATCH] [generator] Tests for the Cities Ids section. --- generator/cities_boundaries_builder.cpp | 18 --- generator/cities_boundaries_builder.hpp | 1 + generator/cities_ids_builder.cpp | 66 +++++++++-- generator/cities_ids_builder.hpp | 7 ++ generator/generator_tests/CMakeLists.txt | 1 + .../generator_tests/cities_ids_tests.cpp | 104 ++++++++++++++++++ .../test_mwm_builder.cpp | 8 +- generator/generator_tool/generator_tool.cpp | 10 +- generator/utils.cpp | 21 ++++ generator/utils.hpp | 2 + 10 files changed, 202 insertions(+), 36 deletions(-) create mode 100644 generator/generator_tests/cities_ids_tests.cpp diff --git a/generator/cities_boundaries_builder.cpp b/generator/cities_boundaries_builder.cpp index d38436b1dc..8f39dce8f5 100644 --- a/generator/cities_boundaries_builder.cpp +++ b/generator/cities_boundaries_builder.cpp @@ -39,24 +39,6 @@ namespace generator { namespace { -bool ParseFeatureIdToTestIdMapping(string const & path, unordered_map & mapping) -{ - bool success = true; - feature::ForEachFromDat(path, [&](FeatureType & feature, uint32_t fid) { - auto const & metatada = feature.GetMetadata(); - auto const sid = metatada.Get(feature::Metadata::FMD_TEST_ID); - uint64_t tid; - if (!strings::to_uint64(sid, tid)) - { - LOG(LERROR, ("Can't parse test id from:", sid, "for a feature", fid)); - success = false; - return; - } - mapping.emplace(fid, tid); - }); - return success; -} - template bool BuildCitiesBoundaries(string const & dataPath, BoundariesTable & table, MappingReader && reader) diff --git a/generator/cities_boundaries_builder.hpp b/generator/cities_boundaries_builder.hpp index eb1fdf0507..192ffdd37b 100644 --- a/generator/cities_boundaries_builder.hpp +++ b/generator/cities_boundaries_builder.hpp @@ -10,6 +10,7 @@ namespace generator { +// todo(@m) Make test ids a new source in base::GeoObjectId? using OsmIdToBoundariesTable = base::ClusteringMap; using TestIdToBoundariesTable = base::ClusteringMap; diff --git a/generator/cities_ids_builder.cpp b/generator/cities_ids_builder.cpp index d17a5241a7..c3f7cbd363 100644 --- a/generator/cities_ids_builder.cpp +++ b/generator/cities_ids_builder.cpp @@ -3,6 +3,7 @@ #include "generator/utils.hpp" #include "indexer/classificator_loader.hpp" +#include "indexer/data_header.hpp" #include "indexer/feature_to_osm.hpp" #include "search/categories_cache.hpp" @@ -23,23 +24,26 @@ #include "defines.hpp" -namespace generator +namespace { -// todo(@m) This should be built only for World.mwm. -bool BuildCitiesIds(std::string const & dataPath, std::string const & osmToFeaturePath) +bool IsWorldMwm(std::string const & dataPath) { - classificator::Load(); - - std::unordered_map mapping; - if (!ParseFeatureIdToOsmIdMapping(osmToFeaturePath, mapping)) + try + { + feature::DataHeader const header(dataPath); + return header.GetType() == feature::DataHeader::world; + } + catch (Reader::OpenException const & e) { - LOG(LERROR, ("Can't parse feature id to osm id mapping.")); return false; } +} +void WriteCitiesIdsSectionToFile(std::string const & dataPath, + std::unordered_map const & mapping) +{ indexer::FeatureIdToGeoObjectIdBimapMem map; - - auto const localities = GetLocalities(dataPath); + auto const localities = generator::GetLocalities(dataPath); localities.ForEach([&](uint64_t fid64) { auto const fid = base::checked_cast(fid64); auto const it = mapping.find(fid); @@ -55,7 +59,7 @@ bool BuildCitiesIds(std::string const & dataPath, std::string const & osmToFeatu auto const hasOldFid = map.GetKey(osmId, oldFid); LOG(LWARNING, - ("Could not add the pair (", fid, osmId, + ("Could not add the pair (", fid, ",", osmId, ") to the cities ids section; old fid:", (hasOldFid ? DebugPrint(oldFid) : "none"), "old osmId:", (hasOldOsmId ? DebugPrint(oldOsmId) : "none"))); } @@ -69,7 +73,47 @@ bool BuildCitiesIds(std::string const & dataPath, std::string const & osmToFeatu LOG(LINFO, ("Serialized cities ids. Number of entries:", map.Size(), "Size in bytes:", pos1 - pos0)); +} +} // namespace +namespace generator +{ +bool BuildCitiesIds(std::string const & dataPath, std::string const & osmToFeaturePath) +{ + if (!IsWorldMwm(dataPath)) + { + LOG(LINFO, ("Skipping generation of cities ids for the non-world mwm file at", dataPath)); + return false; + } + + classificator::Load(); + + std::unordered_map mapping; + if (!ParseFeatureIdToOsmIdMapping(osmToFeaturePath, mapping)) + { + LOG(LERROR, ("Can't parse feature id to osm id mapping.")); + return false; + } + + WriteCitiesIdsSectionToFile(dataPath, mapping); + return true; +} + +bool BuildCitiesIdsForTesting(std::string const & dataPath) +{ + CHECK(IsWorldMwm(dataPath), ()); + + std::unordered_map mapping; + if (!ParseFeatureIdToTestIdMapping(dataPath, mapping)) + return false; + + std::unordered_map mappingToGeoObjects; + for (auto const & entry : mapping) + { + // todo(@m) Make test ids a new source in base::GeoObjectId? + mappingToGeoObjects.emplace(entry.first, base::MakeOsmNode(entry.second)); + } + WriteCitiesIdsSectionToFile(dataPath, mappingToGeoObjects); return true; } } // namespace generator diff --git a/generator/cities_ids_builder.hpp b/generator/cities_ids_builder.hpp index aa635b3fb3..bb683059aa 100644 --- a/generator/cities_ids_builder.hpp +++ b/generator/cities_ids_builder.hpp @@ -7,5 +7,12 @@ namespace generator // Takes the FeatureID <-> base::GeoObjectId bidirectional map for features // corresponding to cities from |osmToFeaturePath| and serializes it // as a section in the mwm file at |dataPath|. +// Only works for World.mwm and always returns false otherwise. bool BuildCitiesIds(std::string const & dataPath, std::string const & osmToFeaturePath); + +// Takes the FeatureID <-> base::GeoObjectId bidirectional map for features +// corresponding to cities from the (test) features metadata and serializes the map +// as a section in the mwm file at |dataPath|. +// Only works for World.mwm and always returns false otherwise. +bool BuildCitiesIdsForTesting(std::string const & dataPath); } // namespace generator diff --git a/generator/generator_tests/CMakeLists.txt b/generator/generator_tests/CMakeLists.txt index 6ba1e4d23d..4b1f0b24e3 100644 --- a/generator/generator_tests/CMakeLists.txt +++ b/generator/generator_tests/CMakeLists.txt @@ -7,6 +7,7 @@ set( camera_collector_tests.cpp check_mwms.cpp cities_boundaries_checker_tests.cpp + cities_ids_tests.cpp city_roads_tests.cpp coasts_test.cpp common.cpp diff --git a/generator/generator_tests/cities_ids_tests.cpp b/generator/generator_tests/cities_ids_tests.cpp new file mode 100644 index 0000000000..05f6a8c5ec --- /dev/null +++ b/generator/generator_tests/cities_ids_tests.cpp @@ -0,0 +1,104 @@ +#include "testing/testing.hpp" + +#include "generator/cities_ids_builder.hpp" +#include "generator/feature_builder.hpp" +#include "generator/generator_tests_support/test_feature.hpp" +#include "generator/generator_tests_support/test_with_custom_mwms.hpp" + +#include "indexer/classificator.hpp" +#include "indexer/data_source.hpp" +#include "indexer/feature.hpp" +#include "indexer/feature_processor.hpp" +#include "indexer/feature_to_osm.hpp" +#include "indexer/ftypes_matcher.hpp" + +#include "platform/country_defines.hpp" +#include "platform/local_country_file.hpp" + +#include "geometry/point2d.hpp" +#include "geometry/rect2d.hpp" + +#include +#include +#include + +using namespace generator; +using namespace generator::tests_support; + +namespace +{ +class CitiesIdsTest : public TestWithCustomMwms +{ +public: + DataSource const & GetDataSource() const { return m_dataSource; } +}; + +// A feature that is big enough for World.mwm but is not a locality. +class TestSea : public TestFeature +{ +public: + TestSea(m2::PointD const & center, std::string const & name, std::string const & lang) + : TestFeature(center, name, lang), m_name(name) + { + } + + // TestFeature overrides: + void Serialize(feature::FeatureBuilder & fb) const + { + TestFeature::Serialize(fb); + fb.AddType(classif().GetTypeByPath({"place", "sea"})); + } + + std::string ToDebugString() const + { + std::ostringstream oss; + oss << "TestSea [" << m_name << "]"; + return oss.str(); + } + +private: + std::string m_name; +}; +} // namespace + +UNIT_CLASS_TEST(CitiesIdsTest, BuildCitiesIds) +{ + TestCity paris(m2::PointD(0, 0), "Paris", "fr", 100 /* rank */); + TestCity wien(m2::PointD(1, 1), "Wien", "de", 100 /* rank */); + TestSea marCaribe(m2::PointD(2, 2), "Mar Caribe", "es"); + + auto testWorldId = BuildWorld([&](TestMwmBuilder & builder) { + builder.Add(paris); + builder.Add(wien); + builder.Add(marCaribe); + }); + + indexer::FeatureIdToGeoObjectIdBimap bimap(GetDataSource()); + TEST(bimap.Load(), ()); + + { + size_t numFeatures = 0; + size_t numLocalities = 0; + auto const test = [&](FeatureType & ft, uint32_t index) { + ++numFeatures; + FeatureID const fid(testWorldId, index); + base::GeoObjectId gid; + FeatureID receivedFid; + + bool const mustExist = ftypes::IsLocalityChecker::Instance()(ft); + + TEST_EQUAL(bimap.GetGeoObjectId(fid, gid), mustExist, (index, ft.GetNames())); + if (!mustExist) + return; + + ++numLocalities; + TEST_EQUAL(bimap.GetFeatureID(gid, receivedFid), mustExist, (index)); + TEST_EQUAL(receivedFid, fid, ()); + }; + + feature::ForEachFromDat(testWorldId.GetInfo()->GetLocalFile().GetPath(MapOptions::Map), test); + + TEST_EQUAL(numFeatures, 3, ()); + TEST_EQUAL(numLocalities, 2, ()); + } +} diff --git a/generator/generator_tests_support/test_mwm_builder.cpp b/generator/generator_tests_support/test_mwm_builder.cpp index 5268acdb69..7fc5497204 100644 --- a/generator/generator_tests_support/test_mwm_builder.cpp +++ b/generator/generator_tests_support/test_mwm_builder.cpp @@ -1,6 +1,7 @@ #include "generator/generator_tests_support/test_mwm_builder.hpp" #include "generator/centers_table_builder.hpp" +#include "generator/cities_ids_builder.hpp" #include "generator/feature_builder.hpp" #include "generator/feature_generator.hpp" #include "generator/feature_sorter.hpp" @@ -116,9 +117,9 @@ void TestMwmBuilder::SetMwmLanguages(vector const & languages) void TestMwmBuilder::Finish() { - string const tmpFilePath = m_collector->GetFilePath(); - CHECK(m_collector, ("Finish() already was called.")); + + string const tmpFilePath = m_collector->GetFilePath(); m_collector.reset(); GenerateInfo info; @@ -141,7 +142,10 @@ void TestMwmBuilder::Finish() ("Can't build search index.")); if (m_type == DataHeader::world) + { CHECK(generator::BuildCitiesBoundariesForTesting(path, m_boundariesTable), ()); + CHECK(generator::BuildCitiesIdsForTesting(path), ()); + } CHECK(indexer::BuildCentersTableFromDataFile(path, true /* forceRebuild */), ("Can't build centers table.")); diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index f729e13a6e..8d49547230 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -132,10 +132,10 @@ DEFINE_bool(generate_regions_kv, false, "Generate regions key-value for server-side reverse geocoder."); DEFINE_bool(dump_cities_boundaries, false, "Dump cities boundaries to a file"); -DEFINE_bool(generate_cities_boundaries, false, "Generate cities boundaries section"); +DEFINE_bool(generate_cities_boundaries, false, "Generate the cities boundaries section"); DEFINE_string(cities_boundaries_data, "", "File with cities boundaries"); -DEFINE_bool(generate_cities_fid_bimap, false, "Generate cities fid bimap, todo(@m)"); +DEFINE_bool(generate_cities_ids, false, "Generate the cities ids section"); DEFINE_bool(generate_world, false, "Generate separate world file."); DEFINE_bool(split_by_polygons, false, @@ -592,11 +592,11 @@ int GeneratorToolMain(int argc, char ** argv) LOG(LCRITICAL, ("Error generating cities boundaries.")); } - if (FLAGS_generate_cities_fid_bimap) + if (FLAGS_generate_cities_ids) { - LOG(LINFO, ("Generating cities fid bimap for", datFile)); + LOG(LINFO, ("Generating cities ids for", datFile)); if (!generator::BuildCitiesIds(datFile, osmToFeatureFilename)) - LOG(LCRITICAL, ("Error generating cities fid bimap.")); + LOG(LCRITICAL, ("Error generating cities ids.")); } if (!FLAGS_srtm_path.empty()) diff --git a/generator/utils.cpp b/generator/utils.cpp index 40c7edacaf..2151c782a9 100644 --- a/generator/utils.cpp +++ b/generator/utils.cpp @@ -4,6 +4,8 @@ #include "search/localities_source.hpp" #include "search/mwm_context.hpp" +#include "indexer/feature_processor.hpp" + #include "platform/local_country_file.hpp" #include "platform/local_country_file_utils.hpp" #include "platform/platform.hpp" @@ -60,6 +62,25 @@ bool ParseFeatureIdToOsmIdMapping(std::string const & path, }); } +bool ParseFeatureIdToTestIdMapping(std::string const & path, + std::unordered_map & mapping) +{ + bool success = true; + feature::ForEachFromDat(path, [&](FeatureType & feature, uint32_t fid) { + auto const & metatada = feature.GetMetadata(); + auto const sid = metatada.Get(feature::Metadata::FMD_TEST_ID); + uint64_t tid; + if (!strings::to_uint64(sid, tid)) + { + LOG(LERROR, ("Can't parse test id from:", sid, "for the feature", fid)); + success = false; + return; + } + mapping.emplace(fid, tid); + }); + return success; +} + search::CBV GetLocalities(std::string const & dataPath) { FrozenDataSource dataSource; diff --git a/generator/utils.hpp b/generator/utils.hpp index c87c18baff..9cc6b5d520 100644 --- a/generator/utils.hpp +++ b/generator/utils.hpp @@ -66,6 +66,8 @@ bool ForEachOsmId2FeatureId(std::string const & path, ToDo && toDo) bool ParseFeatureIdToOsmIdMapping(std::string const & path, std::unordered_map & mapping); +bool ParseFeatureIdToTestIdMapping(std::string const & path, + std::unordered_map & mapping); search::CBV GetLocalities(std::string const & dataPath); } // namespace generator