[generator] Tests for the Cities Ids section.

generator/cities_boundaries_builder.cpp

@@ -39,24 +39,6 @@ namespace generator
 {
 namespace
 {
-bool ParseFeatureIdToTestIdMapping(string const & path, unordered_map<uint32_t, uint64_t> & 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 <typename BoundariesTable, typename MappingReader>
 bool BuildCitiesBoundaries(string const & dataPath, BoundariesTable & table,
                            MappingReader && reader)

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<base::GeoObjectId, indexer::CityBoundary>;
 using TestIdToBoundariesTable = base::ClusteringMap<uint64_t, indexer::CityBoundary>;

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<uint32_t, base::GeoObjectId> 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<uint32_t, base::GeoObjectId> 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<uint32_t>(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<uint32_t, base::GeoObjectId> 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<uint32_t, uint64_t> mapping;
+  if (!ParseFeatureIdToTestIdMapping(dataPath, mapping))
+    return false;
+
+  std::unordered_map<uint32_t, base::GeoObjectId> 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

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

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

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 <cstddef>
+#include <sstream>
+#include <string>
+
+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, ());
+  }
+}

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<string> 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."));

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())

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<uint32_t, uint64_t> & 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;

generator/utils.hpp

@@ -66,6 +66,8 @@ bool ForEachOsmId2FeatureId(std::string const & path, ToDo && toDo)
 
 bool ParseFeatureIdToOsmIdMapping(std::string const & path,
                                   std::unordered_map<uint32_t, base::GeoObjectId> & mapping);
+bool ParseFeatureIdToTestIdMapping(std::string const & path,
+                                   std::unordered_map<uint32_t, uint64_t> & mapping);
 
 search::CBV GetLocalities(std::string const & dataPath);
 }  // namespace generator