From 7ad4551e4c83df30b01f210dd4cb2a4c6b34fe82 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 14 Nov 2016 17:13:14 +0300 Subject: [PATCH 1/8] Using final stage osm ids to feature ids mapping for restriction generation. --- generator/gen_mwm_info.hpp | 12 ++-- .../restriction_collector_test.cpp | 2 +- generator/generator_tool/generator_tool.cpp | 2 +- generator/restriction_collector.cpp | 72 ++++++++++--------- generator/restriction_collector.hpp | 11 +-- generator/restriction_generator.cpp | 8 +-- 6 files changed, 56 insertions(+), 51 deletions(-) diff --git a/generator/gen_mwm_info.hpp b/generator/gen_mwm_info.hpp index bd58e4282e..d9f8ce99af 100644 --- a/generator/gen_mwm_info.hpp +++ b/generator/gen_mwm_info.hpp @@ -4,13 +4,14 @@ #include "base/assert.hpp" -#include "std/utility.hpp" #include "std/algorithm.hpp" - +#include "std/fstream.hpp" +#include "std/iostream.hpp" +#include "std/string.hpp" +#include "std/utility.hpp" namespace gen { - template class Accumulator { protected: @@ -30,6 +31,8 @@ public: { rw::ReadVectorOfPOD(src, m_data); } + + vector const & GetData() const { return m_data; } }; class OsmID2FeatureID : public Accumulator> @@ -63,5 +66,4 @@ public: return 0; } }; - -} +} // namespace gen diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index 233e8f952f..838a9a65e1 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -101,7 +101,7 @@ UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping) RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); Platform const & platform = Platform(); - restrictionCollector.ParseFeatureId2OsmIdsMapping( + restrictionCollector.ParseOsmIdToFeatureIdMapping( my::JoinFoldersToPath(platform.WritableDir(), kFeatureIdToOsmIdsPath)); vector> const expectedOsmIds2FeatureId = { diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index 7e7394cc24..96843e401b 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -244,7 +244,7 @@ int main(int argc, char ** argv) { routing::BuildRoadRestrictions( datFile, genInfo.GetIntermediateFileName(genInfo.m_restrictions, "" /* extention */), - genInfo.GetIntermediateFileName(genInfo.m_featureIdToOsmIds, "" /* extention */)); + genInfo.GetTargetFileName(country) + OSM2FEATURE_FILE_EXTENSION); } } diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index ba0395f25a..3c977fe6e2 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -1,5 +1,10 @@ #include "generator/restriction_collector.hpp" +#include "generator/gen_mwm_info.hpp" + +#include "coding/file_reader.hpp" +#include "coding/reader.hpp" + #include "base/assert.hpp" #include "base/logging.hpp" #include "base/scope_guard.hpp" @@ -31,17 +36,17 @@ bool ParseLineOfNumbers(strings::SimpleTokenizer & iter, vector & numb namespace routing { RestrictionCollector::RestrictionCollector(string const & restrictionPath, - string const & featureIdToOsmIdsPath) + string const & osmIdsToFeatureIdPath) { MY_SCOPE_GUARD(clean, [this](){ m_osmIdToFeatureId.clear(); m_restrictions.clear(); }); - if (!ParseFeatureId2OsmIdsMapping(featureIdToOsmIdsPath)) + if (!ParseOsmIdToFeatureIdMapping(osmIdsToFeatureIdPath)) { LOG(LWARNING, ("An error happened while parsing feature id to osm ids mapping from file:", - featureIdToOsmIdsPath)); + osmIdsToFeatureIdPath)); return; } @@ -65,34 +70,34 @@ bool RestrictionCollector::IsValid() const [](Restriction const & r) { return !r.IsValid(); }) == end(m_restrictions); } -bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureIdToOsmIdsPath) +bool RestrictionCollector::ParseOsmIdToFeatureIdMapping(string const & osmIdsToFeatureIdPath) { - ifstream featureIdToOsmIdsStream(featureIdToOsmIdsPath); - if (featureIdToOsmIdsStream.fail()) - return false; + LOG(LINFO, ("osmIdsToFeatureIdPath =", osmIdsToFeatureIdPath)); - string line; - while (getline(featureIdToOsmIdsStream, line)) + using OsmIdToFeatureId = pair; + gen::Accumulator osmIdsToFeatureIds; + try { - vector osmIds; - strings::SimpleTokenizer iter(line, kDelim); - if (!ParseLineOfNumbers(iter, osmIds)) - { - LOG(LWARNING, ("Cannot parse feature id to osm ids mapping. Line:", line)); - return false; - } - - if (osmIds.size() < 2) - { - LOG(LWARNING, ("Parse result of mapping feature id to osm ids is too small. Line:", - line)); - return false; // Every line should contain feature id and at least one osm id. - } - - uint32_t const featureId = static_cast(osmIds.front()); - osmIds.erase(osmIds.begin()); - AddFeatureId(featureId, osmIds); + FileReader reader(osmIdsToFeatureIdPath); + ReaderSource src(reader); + osmIdsToFeatureIds.Read(src); } + catch (FileReader::Exception const & e) + { + LOG(LERROR, ("Exception while reading file:", osmIdsToFeatureIdPath, ". Msg:", e.Msg())); + return false; + } + + vector const & mapping = osmIdsToFeatureIds.GetData(); + if (mapping.empty()) + { + LOG(LINFO, ("No osm ids to feature ids mapping in file", osmIdsToFeatureIdPath)); + return true; + } + + for (auto const osmIdToFeatureId : mapping) + AddFeatureId(osmIdToFeatureId.second /* feature id */, osmIdToFeatureId.first /* osm id */); + return true; } @@ -150,18 +155,15 @@ bool RestrictionCollector::AddRestriction(Restriction::Type type, vector const & osmIds) +void RestrictionCollector::AddFeatureId(uint32_t featureId, uint64_t osmId) { // Note. One |featureId| could correspond to several osm ids. // But for road feature |featureId| corresponds to exactly one osm id. - for (uint64_t const & osmId : osmIds) + auto const result = m_osmIdToFeatureId.insert(make_pair(osmId, featureId)); + if (result.second == false) { - auto const result = m_osmIdToFeatureId.insert(make_pair(osmId, featureId)); - if (result.second == false) - { - LOG(LERROR, ("Osm id", osmId, "is included in two feature ids: ", featureId, - m_osmIdToFeatureId.find(osmId)->second)); - } + LOG(LERROR, ("Osm id", osmId, "is included in two feature ids: ", featureId, + m_osmIdToFeatureId.find(osmId)->second)); } } diff --git a/generator/restriction_collector.hpp b/generator/restriction_collector.hpp index 19a04a1f95..155e84973c 100644 --- a/generator/restriction_collector.hpp +++ b/generator/restriction_collector.hpp @@ -42,10 +42,10 @@ private: /// 137999, 5170186, /// 138000, 5170209, 5143342, /// 138001, 5170228, - /// \param featureIdToOsmIdsPath path to the text file. + /// \param osmIdsToFeatureIdPath path to the text file. /// \note Most restrictions consist of type and two linear(road) features. /// \note For the time being only line-point-line restrictions are supported. - bool ParseFeatureId2OsmIdsMapping(string const & featureIdToOsmIdsPath); + bool ParseOsmIdToFeatureIdMapping(string const & osmIdsToFeatureIdPath); /// \brief Parses comma separated text file with line in following format: /// , , , and so on @@ -56,9 +56,10 @@ private: /// \param featureIdToOsmIdsPath path to the text file. bool ParseRestrictions(string const & path); - /// \brief Adds feature id and corresponding vector of |osmIds| to |m_osmIdToFeatureId|. - /// \note One feature id (|featureId|) may correspond to several osm ids (|osmIds|). - void AddFeatureId(uint32_t featureId, vector const & osmIds); + /// \brief Adds feature id and corresponding |osmId| to |m_osmIdToFeatureId|. + /// \note In general one feature id (|featureId|) may correspond to several osm ids (|osmIds|). + /// But for road feature one feature id corresponds one osm id. + void AddFeatureId(uint32_t featureId, uint64_t osmId); /// \brief Adds a restriction (vector of osm id). /// \param type is a type of restriction diff --git a/generator/restriction_generator.cpp b/generator/restriction_generator.cpp index 2a5eb1a4bf..1320686461 100644 --- a/generator/restriction_generator.cpp +++ b/generator/restriction_generator.cpp @@ -15,15 +15,15 @@ namespace routing { bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPath, - string const & featureIdToOsmIdsPath) + string const & osmIdsTofeatureIdsPath) { LOG(LINFO, - ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", featureIdToOsmIdsPath, ");")); - RestrictionCollector restrictionCollector(restrictionPath, featureIdToOsmIdsPath); + ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", osmIdsTofeatureIdsPath, ");")); + RestrictionCollector restrictionCollector(restrictionPath, osmIdsTofeatureIdsPath); if (!restrictionCollector.HasRestrictions() || !restrictionCollector.IsValid()) { LOG(LWARNING, ("No valid restrictions for", mwmPath, "It's necessary to check that", - restrictionPath, "and", featureIdToOsmIdsPath, "are available.")); + restrictionPath, "and", osmIdsTofeatureIdsPath, "are available.")); return false; } From e995ce1d3992931a73bf2093e06d2ce4a35cd1b1 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 15 Nov 2016 08:40:56 +0300 Subject: [PATCH 2/8] Fixing restriction_test and making them workable with osm ids to second stage feature ids binary mapping file. --- .../generator_tests/restriction_test.cpp | 90 +++++++++++++------ generator/restriction_collector.cpp | 2 +- 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index 8973866e4d..e16bbe1172 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -3,6 +3,7 @@ #include "generator/generator_tests_support/test_feature.hpp" #include "generator/generator_tests_support/test_mwm_builder.hpp" +#include "generator/gen_mwm_info.hpp" #include "generator/restriction_collector.hpp" #include "generator/restriction_generator.hpp" @@ -12,6 +13,7 @@ #include "indexer/mwm_set.hpp" #include "coding/file_name_utils.hpp" +#include "coding/file_writer.hpp" #include "platform/platform_tests_support/scoped_dir.hpp" #include "platform/platform_tests_support/scoped_file.hpp" @@ -21,7 +23,9 @@ #include "base/logging.hpp" #include "base/scope_guard.hpp" +#include "base/string_utils.hpp" +#include "std/utility.hpp" #include "std/string.hpp" using namespace feature; @@ -37,17 +41,47 @@ string const kTestDir = "restriction_generation_test"; // Temporary mwm name for testing. string const kTestMwm = "test"; string const kRestrictionFileName = "restrictions_in_osm_ids.csv"; -string const featureId2OsmIdsName = "feature_id_to_osm_ids.csv"; +string const kOsmIdsToFeatureIdsName = "osm_ids_to_feature_ids" OSM2FEATURE_FILE_EXTENSION; void BuildEmptyMwm(LocalCountryFile & country) { generator::tests_support::TestMwmBuilder builder(country, feature::DataHeader::country); } +/// \brief Generates a binary file with by a string with mapping from osm ids to feature ids. +/// \param mappingContent a string with lines with mapping from osm id to feature id (one to one). +/// For example +/// 10, 1, +/// 20, 2 +/// 30, 3, +/// 40, 4 +/// \parma outputFilePath full path to an output file where the mapping is saved. +void GenerateOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath) +{ + strings::SimpleTokenizer lineIter(mappingContent, "\n\r" /* string delimiter */); + + gen::Accumulator> osmIdsToFeatureIds; + for (; lineIter; ++lineIter) + { + strings::SimpleTokenizer idIter(*lineIter, ", \t" /* id delimiter */); + uint64_t osmId = 0; + TEST(strings::to_uint64(*idIter, osmId), ("Cannot covert to uint64_t:", *idIter)); + TEST(idIter, ("Wrong feature ids to osm ids mapping.")); + ++idIter; + + uint32_t featureId = 0; + TEST(strings::to_uint(*idIter, featureId), ("Cannot covert to uint:", *idIter)); + osmIdsToFeatureIds.Add(make_pair(osmId, featureId)); + } + + FileWriter osm2ftWriter(outputFilePath); + osmIdsToFeatureIds.Flush(osm2ftWriter); +} + /// \brief Generates a restriction section, adds it to an empty mwm, /// loads the restriction section and test loaded restrictions. /// \param restrictionContent comma separated text with restrictions in osm id terms. -/// \param mappingContent comma separated text with with mapping from feature ids to osm ids. +/// \param mappingContent comma separated text with with mapping from osm ids to feature ids. void TestRestrictionBuilding(string const & restrictionContent, string const & mappingContent) { Platform & platform = GetPlatform(); @@ -60,16 +94,18 @@ void TestRestrictionBuilding(string const & restrictionContent, string const & m ScopedFile const scopedMwm(mwmRelativePath); BuildEmptyMwm(country); - // Creating files with restrictions. + // Creating a file with restrictions. string const restrictionRelativePath = my::JoinFoldersToPath(kTestDir, kRestrictionFileName); ScopedFile const restrictionScopedFile(restrictionRelativePath, restrictionContent); - string const mappingRelativePath = my::JoinFoldersToPath(kTestDir, featureId2OsmIdsName); - ScopedFile const mappingScopedFile(mappingRelativePath, mappingContent); + // Creating osm ids to feature ids mapping. + string const mappingRelativePath = my::JoinFoldersToPath(kTestDir, kOsmIdsToFeatureIdsName); + ScopedFile const mappingScopedFile(mappingRelativePath); + string const mappingFullPath = my::JoinFoldersToPath(writableDir, mappingRelativePath); + GenerateOsmIdsToFeatureIdsMapping(mappingContent, mappingFullPath); // Adding restriction section to mwm. string const restrictionFullPath = my::JoinFoldersToPath(writableDir, restrictionRelativePath); - string const mappingFullPath = my::JoinFoldersToPath(writableDir, mappingRelativePath); string const mwmFullPath = my::JoinFoldersToPath(writableDir, mwmRelativePath); BuildRoadRestrictions(mwmFullPath, restrictionFullPath, mappingFullPath); @@ -89,22 +125,22 @@ void TestRestrictionBuilding(string const & restrictionContent, string const & m UNIT_TEST(RestrictionGenerationTest_NoRestriction) { string const restrictionContent = ""; - string const featureIdToOsmIdsContent = ""; - TestRestrictionBuilding(restrictionContent, featureIdToOsmIdsContent); + string const osmIdsToFeatureIdsContent = ""; + TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } UNIT_TEST(RestrictionGenerationTest_ZeroId) { string const restrictionContent = R"(Only, 0, 0,)"; - string const featureIdToOsmIdsContent = R"(0, 0,)"; - TestRestrictionBuilding(restrictionContent, featureIdToOsmIdsContent); + string const osmIdsToFeatureIdsContent = R"(0, 0,)"; + TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } UNIT_TEST(RestrictionGenerationTest_OneRestriction) { string const restrictionContent = R"(No, 10, 10,)"; - string const featureIdToOsmIdsContent = R"(1, 10,)"; - TestRestrictionBuilding(restrictionContent, featureIdToOsmIdsContent); + string const osmIdsToFeatureIdsContent = R"(10, 1,)"; + TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } UNIT_TEST(RestrictionGenerationTest_ThreeRestrictions) @@ -112,11 +148,11 @@ UNIT_TEST(RestrictionGenerationTest_ThreeRestrictions) string const restrictionContent = R"(No, 10, 10, Only, 10, 20 Only, 30, 40)"; - string const featureIdToOsmIdsContent = R"(1, 10, - 2, 20 - 3, 30, - 4, 40)"; - TestRestrictionBuilding(restrictionContent, featureIdToOsmIdsContent); + string const osmIdsToFeatureIdsContent = R"(10, 1, + 20, 2 + 30, 3, + 40, 4)"; + TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } UNIT_TEST(RestrictionGenerationTest_SevenRestrictions) @@ -128,11 +164,11 @@ UNIT_TEST(RestrictionGenerationTest_SevenRestrictions) No, 30, 30, No, 40, 40, Only, 30, 40,)"; - string const featureIdToOsmIdsContent = R"(1, 10, - 2, 20, - 3, 30, - 4, 40)"; - TestRestrictionBuilding(restrictionContent, featureIdToOsmIdsContent); + string const osmIdsToFeatureIdsContent = R"(10, 1, + 20, 2, + 30, 3, + 40, 4)"; + TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } UNIT_TEST(RestrictionGenerationTest_ThereAndMoreLinkRestrictions) @@ -141,10 +177,10 @@ UNIT_TEST(RestrictionGenerationTest_ThereAndMoreLinkRestrictions) No, 20, 20, Only, 10, 20, 30, 40 Only, 20, 30, 40)"; - string const featureIdToOsmIdsContent = R"(1, 10, - 2, 20, - 3, 30, - 4, 40)"; - TestRestrictionBuilding(restrictionContent, featureIdToOsmIdsContent); + string const osmIdsToFeatureIdsContent = R"(10, 1, + 20, 2, + 30, 3, + 40, 4)"; + TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } } // namespace diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index 3c977fe6e2..58c85e27a6 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -84,7 +84,7 @@ bool RestrictionCollector::ParseOsmIdToFeatureIdMapping(string const & osmIdsToF } catch (FileReader::Exception const & e) { - LOG(LERROR, ("Exception while reading file:", osmIdsToFeatureIdPath, ". Msg:", e.Msg())); + LOG(LWARNING, ("Exception while reading file:", osmIdsToFeatureIdPath, ". Msg:", e.Msg())); return false; } From 649d4f4bedaf509549084860514824909df3c66d Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 15 Nov 2016 10:12:32 +0300 Subject: [PATCH 3/8] Fixing restrictions_collector_test and moving osm ids to feature ids generation to generator_tests_support. --- .../restriction_collector_test.cpp | 76 +++++++------------ .../generator_tests/restriction_test.cpp | 37 +-------- .../generator_tests_support.pro | 2 + .../restrcion_support.cpp | 36 +++++++++ .../restrcion_support.hpp | 16 ++++ generator/restriction_collector.hpp | 6 +- generator/restriction_generator.hpp | 13 +--- 7 files changed, 89 insertions(+), 97 deletions(-) create mode 100644 generator/generator_tests_support/restrcion_support.cpp create mode 100644 generator/generator_tests_support/restrcion_support.hpp diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index 838a9a65e1..852f7193b6 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -1,5 +1,7 @@ #include "testing/testing.hpp" +#include "generator/generator_tests_support/restrcion_support.hpp" + #include "generator/osm_id.hpp" #include "generator/restriction_collector.hpp" @@ -18,6 +20,7 @@ #include "std/utility.hpp" #include "std/vector.hpp" +using namespace generator; using namespace platform; using namespace platform::tests_support; @@ -27,13 +30,13 @@ string const kRestrictionTestDir = "test-restrictions"; UNIT_TEST(RestrictionTest_ValidCase) { - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* osmIdsToFeatureIdsPath */); // Adding feature ids. - restrictionCollector.AddFeatureId(30 /* featureId */, {3} /* osmIds */); - restrictionCollector.AddFeatureId(10 /* featureId */, {1} /* osmIds */); - restrictionCollector.AddFeatureId(50 /* featureId */, {5} /* osmIds */); - restrictionCollector.AddFeatureId(70 /* featureId */, {7} /* osmIds */); - restrictionCollector.AddFeatureId(20 /* featureId */, {2} /* osmIds */); + restrictionCollector.AddFeatureId(30 /* featureId */, 3 /* osmId */); + restrictionCollector.AddFeatureId(10 /* featureId */, 1 /* osmId */); + restrictionCollector.AddFeatureId(50 /* featureId */, 5 /* osmId */); + restrictionCollector.AddFeatureId(70 /* featureId */, 7 /* osmId */); + restrictionCollector.AddFeatureId(20 /* featureId */, 2 /* osmId */); // Adding restrictions. TEST(restrictionCollector.AddRestriction(Restriction::Type::No, {1, 2} /* osmIds */), ()); @@ -52,9 +55,9 @@ UNIT_TEST(RestrictionTest_ValidCase) UNIT_TEST(RestrictionTest_InvalidCase) { - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); - restrictionCollector.AddFeatureId(0 /* featureId */, {0} /* osmIds */); - restrictionCollector.AddFeatureId(20 /* featureId */, {2} /* osmIds */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* osmIdsToFeatureIdsPath */); + restrictionCollector.AddFeatureId(0 /* featureId */, 0 /* osmId */); + restrictionCollector.AddFeatureId(20 /* featureId */, 2 /* osmId */); TEST(!restrictionCollector.AddRestriction(Restriction::Type::No, {0, 1} /* osmIds */), ()); @@ -75,7 +78,7 @@ UNIT_TEST(RestrictionTest_ParseRestrictions) ScopedDir const scopedDir(kRestrictionTestDir); ScopedFile const scopedFile(kRestrictionPath, kRestrictionContent); - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* osmIdsToFeatureIdsPath */); Platform const & platform = Platform(); @@ -85,35 +88,6 @@ UNIT_TEST(RestrictionTest_ParseRestrictions) TEST(!restrictionCollector.HasRestrictions(), ()); } -UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping) -{ - string const kFeatureIdToOsmIdsName = "feature_id_to_osm_ids.csv"; - string const kFeatureIdToOsmIdsPath = - my::JoinFoldersToPath(kRestrictionTestDir, kFeatureIdToOsmIdsName); - string const kFeatureIdToOsmIdsContent = R"(1, 10, - 2, 20 - 779703, 5423239545, - 3, 30)"; - - ScopedDir const scopedDir(kRestrictionTestDir); - ScopedFile const scopedFile(kFeatureIdToOsmIdsPath, kFeatureIdToOsmIdsContent); - - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); - - Platform const & platform = Platform(); - restrictionCollector.ParseOsmIdToFeatureIdMapping( - my::JoinFoldersToPath(platform.WritableDir(), kFeatureIdToOsmIdsPath)); - - vector> const expectedOsmIds2FeatureId = { - {10, 1}, {20, 2}, {30, 3}, {5423239545, 779703}}; - vector> osmIds2FeatureId( - restrictionCollector.m_osmIdToFeatureId.cbegin(), - restrictionCollector.m_osmIdToFeatureId.cend()); - sort(osmIds2FeatureId.begin(), osmIds2FeatureId.end(), - my::LessBy(&pair::first)); - TEST_EQUAL(osmIds2FeatureId, expectedOsmIds2FeatureId, ()); -} - UNIT_TEST(RestrictionTest_RestrictionCollectorWholeClassTest) { string const kRestrictionName = "restrictions_in_osm_ids.csv"; @@ -122,22 +96,24 @@ UNIT_TEST(RestrictionTest_RestrictionCollectorWholeClassTest) Only, 10, 20, Only, 30, 40,)"; - string const kFeatureIdToOsmIdsName = "feature_id_to_osm_ids.csv"; - string const kFeatureIdToOsmIdsPath = - my::JoinFoldersToPath(kRestrictionTestDir, kFeatureIdToOsmIdsName); - string const kFeatureIdToOsmIdsContent = R"(1, 10, - 2, 20, - 3, 30, - 4, 40)"; + string const kOsmIdsToFeatureIdsName = "osm_ids_to_feature_ids" OSM2FEATURE_FILE_EXTENSION; + string const osmIdsToFeatureIdsPath = + my::JoinFoldersToPath(kRestrictionTestDir, kOsmIdsToFeatureIdsName); + string const kOsmIdsToFeatureIdsContent = R"(10, 1, + 20, 2, + 30, 3, + 40, 4)"; + Platform const & platform = Platform(); + string const osmIdsToFeatureIdsFullPath = my::JoinFoldersToPath(platform.WritableDir(), + osmIdsToFeatureIdsPath); + GenerateOsmIdsToFeatureIdsMapping(kOsmIdsToFeatureIdsContent, osmIdsToFeatureIdsFullPath); ScopedDir scopedDir(kRestrictionTestDir); ScopedFile restrictionScopedFile(kRestrictionPath, kRestrictionContent); - ScopedFile mappingScopedFile(kFeatureIdToOsmIdsPath, kFeatureIdToOsmIdsContent); + ScopedFile mappingScopedFile(osmIdsToFeatureIdsPath); - Platform const & platform = Platform(); RestrictionCollector restrictionCollector( - my::JoinFoldersToPath(platform.WritableDir(), kRestrictionPath), - my::JoinFoldersToPath(platform.WritableDir(), kFeatureIdToOsmIdsPath)); + my::JoinFoldersToPath(platform.WritableDir(), kRestrictionPath), osmIdsToFeatureIdsFullPath); TEST(restrictionCollector.IsValid(), ()); RestrictionVec const & restrictions = restrictionCollector.GetRestrictions(); diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index e16bbe1172..efd6e5ff96 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -1,9 +1,9 @@ #include "testing/testing.hpp" +#include "generator/generator_tests_support/restrcion_support.hpp" #include "generator/generator_tests_support/test_feature.hpp" #include "generator/generator_tests_support/test_mwm_builder.hpp" -#include "generator/gen_mwm_info.hpp" #include "generator/restriction_collector.hpp" #include "generator/restriction_generator.hpp" @@ -13,7 +13,6 @@ #include "indexer/mwm_set.hpp" #include "coding/file_name_utils.hpp" -#include "coding/file_writer.hpp" #include "platform/platform_tests_support/scoped_dir.hpp" #include "platform/platform_tests_support/scoped_file.hpp" @@ -23,9 +22,7 @@ #include "base/logging.hpp" #include "base/scope_guard.hpp" -#include "base/string_utils.hpp" -#include "std/utility.hpp" #include "std/string.hpp" using namespace feature; @@ -48,40 +45,10 @@ void BuildEmptyMwm(LocalCountryFile & country) generator::tests_support::TestMwmBuilder builder(country, feature::DataHeader::country); } -/// \brief Generates a binary file with by a string with mapping from osm ids to feature ids. -/// \param mappingContent a string with lines with mapping from osm id to feature id (one to one). -/// For example -/// 10, 1, -/// 20, 2 -/// 30, 3, -/// 40, 4 -/// \parma outputFilePath full path to an output file where the mapping is saved. -void GenerateOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath) -{ - strings::SimpleTokenizer lineIter(mappingContent, "\n\r" /* string delimiter */); - - gen::Accumulator> osmIdsToFeatureIds; - for (; lineIter; ++lineIter) - { - strings::SimpleTokenizer idIter(*lineIter, ", \t" /* id delimiter */); - uint64_t osmId = 0; - TEST(strings::to_uint64(*idIter, osmId), ("Cannot covert to uint64_t:", *idIter)); - TEST(idIter, ("Wrong feature ids to osm ids mapping.")); - ++idIter; - - uint32_t featureId = 0; - TEST(strings::to_uint(*idIter, featureId), ("Cannot covert to uint:", *idIter)); - osmIdsToFeatureIds.Add(make_pair(osmId, featureId)); - } - - FileWriter osm2ftWriter(outputFilePath); - osmIdsToFeatureIds.Flush(osm2ftWriter); -} - /// \brief Generates a restriction section, adds it to an empty mwm, /// loads the restriction section and test loaded restrictions. /// \param restrictionContent comma separated text with restrictions in osm id terms. -/// \param mappingContent comma separated text with with mapping from osm ids to feature ids. +/// \param mappingContent comma separated text with mapping from osm ids to feature ids. void TestRestrictionBuilding(string const & restrictionContent, string const & mappingContent) { Platform & platform = GetPlatform(); diff --git a/generator/generator_tests_support/generator_tests_support.pro b/generator/generator_tests_support/generator_tests_support.pro index 380dc89064..af3ff085e2 100644 --- a/generator/generator_tests_support/generator_tests_support.pro +++ b/generator/generator_tests_support/generator_tests_support.pro @@ -7,9 +7,11 @@ ROOT_DIR = ../.. include($$ROOT_DIR/common.pri) SOURCES += \ + restrcion_support.cpp \ test_feature.cpp \ test_mwm_builder.cpp \ HEADERS += \ + restrcion_support.hpp \ test_feature.hpp \ test_mwm_builder.hpp \ diff --git a/generator/generator_tests_support/restrcion_support.cpp b/generator/generator_tests_support/restrcion_support.cpp new file mode 100644 index 0000000000..678cc37fd4 --- /dev/null +++ b/generator/generator_tests_support/restrcion_support.cpp @@ -0,0 +1,36 @@ +#include "generator/generator_tests_support/restrcion_support.hpp" + +#include "testing/testing.hpp" + +#include "generator/gen_mwm_info.hpp" + +#include "coding/file_writer.hpp" + +#include "base/string_utils.hpp" + +#include "std/utility.hpp" + +namespace generator +{ +void GenerateOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath) +{ + strings::SimpleTokenizer lineIter(mappingContent, "\n\r" /* string delimiter */); + + gen::Accumulator> osmIdsToFeatureIds; + for (; lineIter; ++lineIter) + { + strings::SimpleTokenizer idIter(*lineIter, ", \t" /* id delimiter */); + uint64_t osmId = 0; + TEST(strings::to_uint64(*idIter, osmId), ("Cannot covert to uint64_t:", *idIter)); + TEST(idIter, ("Wrong feature ids to osm ids mapping.")); + ++idIter; + + uint32_t featureId = 0; + TEST(strings::to_uint(*idIter, featureId), ("Cannot covert to uint:", *idIter)); + osmIdsToFeatureIds.Add(make_pair(osmId, featureId)); + } + + FileWriter osm2ftWriter(outputFilePath); + osmIdsToFeatureIds.Flush(osm2ftWriter); +} +} // namespace generator diff --git a/generator/generator_tests_support/restrcion_support.hpp b/generator/generator_tests_support/restrcion_support.hpp new file mode 100644 index 0000000000..2268158c5a --- /dev/null +++ b/generator/generator_tests_support/restrcion_support.hpp @@ -0,0 +1,16 @@ +#pragma once + +#include "std/string.hpp" + +namespace generator +{ +/// \brief Generates a binary file with by a string with mapping from osm ids to feature ids. +/// \param mappingContent a string with lines with mapping from osm id to feature id (one to one). +/// For example +/// 10, 1, +/// 20, 2 +/// 30, 3, +/// 40, 4 +/// \parma outputFilePath full path to an output file where the mapping is saved. +void GenerateOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath); +} // namespace generator diff --git a/generator/restriction_collector.hpp b/generator/restriction_collector.hpp index 155e84973c..432acdc159 100644 --- a/generator/restriction_collector.hpp +++ b/generator/restriction_collector.hpp @@ -17,8 +17,8 @@ class RestrictionCollector { public: /// \param restrictionPath full path to file with road restrictions in osm id terms. - /// \param featureIdToOsmIdsPath full path to file with mapping from feature id to osm id. - RestrictionCollector(string const & restrictionPath, string const & featureIdToOsmIdsPath); + /// \param osmIdsToFeatureIdsPath full path to file with mapping from osm ids to feature ids. + RestrictionCollector(string const & restrictionPath, string const & osmIdsToFeatureIdsPath); bool HasRestrictions() const { return !m_restrictions.empty(); } @@ -53,7 +53,7 @@ private: /// Only, 335049632, 49356687, /// No, 157616940, 157616940, /// No, 157616940, 157617107, - /// \param featureIdToOsmIdsPath path to the text file. + /// \param path path to the text file with restrictions. bool ParseRestrictions(string const & path); /// \brief Adds feature id and corresponding |osmId| to |m_osmIdToFeatureId|. diff --git a/generator/restriction_generator.hpp b/generator/restriction_generator.hpp index 17636f6fc8..d960f8149c 100644 --- a/generator/restriction_generator.hpp +++ b/generator/restriction_generator.hpp @@ -12,14 +12,9 @@ namespace routing /// For example: /// Only, 335049632, 49356687, /// No, 157616940, 157616940, -/// \param featureIdToOsmIdsPath comma separated (csv like) file with mapping from feature id to osm -/// ids -/// in following format: -/// , , , and so -/// on -/// For example: -/// 137999, 5170186, -/// 138000, 5170209, +/// \param osmIdsToFeatureIdsPath a binary file with mapping form osm ids to feature ids. +/// One osm id is mapped to one feature is. The file should be saved with the help of +/// OsmID2FeatureID class or using a similar way. bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPath, - string const & featureIdToOsmIdsPath); + string const & osmIdsToFeatureIdsPath); } // namespace routing From 110926845ea15bb3b2f820bf5bb21a10f8bd2fbc Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 15 Nov 2016 10:38:50 +0300 Subject: [PATCH 4/8] Removing saving intermediate feature ids to csv file. --- generator/generate_info.hpp | 3 --- generator/generator.pro | 2 -- generator/generator_tool/generator_tool.cpp | 2 -- generator/osm_source.cpp | 13 +-------- generator/polygonizer.hpp | 26 ++++++------------ generator/sync_ofsteam.cpp | 30 --------------------- generator/sync_ofsteam.hpp | 23 ---------------- generator/world_map_generator.hpp | 3 +-- 8 files changed, 10 insertions(+), 92 deletions(-) delete mode 100644 generator/sync_ofsteam.cpp delete mode 100644 generator/sync_ofsteam.hpp diff --git a/generator/generate_info.hpp b/generator/generate_info.hpp index ccfe28dd09..e5b52becc1 100644 --- a/generator/generate_info.hpp +++ b/generator/generate_info.hpp @@ -38,9 +38,6 @@ struct GenerateInfo // File name for a file with restrictions in osm id terms. string m_restrictions; - // File name for a file with mapping from feature ids to osm ids. - // Note. One feature id maps to one or more osm ids. - string m_featureIdToOsmIds; NodeStorageType m_nodeStorageType; OsmSourceType m_osmFileType; diff --git a/generator/generator.pro b/generator/generator.pro index 2fd4d2a676..4c416eeed5 100644 --- a/generator/generator.pro +++ b/generator/generator.pro @@ -44,7 +44,6 @@ SOURCES += \ sponsored_scoring.cpp \ srtm_parser.cpp \ statistics.cpp \ - sync_ofsteam.cpp \ tesselator.cpp \ towns_dumper.cpp \ unpack_mwm.cpp \ @@ -87,7 +86,6 @@ HEADERS += \ sponsored_scoring.hpp \ srtm_parser.hpp \ statistics.hpp \ - sync_ofsteam.hpp \ tag_admixer.hpp \ tesselator.hpp \ towns_dumper.hpp \ diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index 96843e401b..817853d436 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -118,7 +118,6 @@ int main(int argc, char ** argv) genInfo.m_osmFileName = FLAGS_osm_file_name; genInfo.m_restrictions = FLAGS_restriction_name; - genInfo.m_featureIdToOsmIds = FLAGS_feature_ids_to_osm_ids_name; genInfo.m_failOnCoasts = FLAGS_fail_on_coasts; genInfo.m_preloadCache = FLAGS_preload_cache; genInfo.m_bookingDatafileName = FLAGS_booking_data; @@ -163,7 +162,6 @@ int main(int argc, char ** argv) LOG(LINFO, ("Generating final data ...")); genInfo.m_restrictions = FLAGS_restriction_name; - genInfo.m_featureIdToOsmIds = FLAGS_feature_ids_to_osm_ids_name; genInfo.m_splitByPolygons = FLAGS_split_by_polygons; genInfo.m_createWorld = FLAGS_generate_world; genInfo.m_makeCoasts = FLAGS_make_coasts; diff --git a/generator/osm_source.cpp b/generator/osm_source.cpp index 4e6695540e..c39fc229e8 100644 --- a/generator/osm_source.cpp +++ b/generator/osm_source.cpp @@ -8,7 +8,6 @@ #include "generator/osm_translator.hpp" #include "generator/osm_xml_source.hpp" #include "generator/polygonizer.hpp" -#include "generator/sync_ofsteam.hpp" #include "generator/tag_admixer.hpp" #include "generator/towns_dumper.hpp" #include "generator/world_map_generator.hpp" @@ -269,7 +268,6 @@ class MainFeaturesEmitter : public EmitterBase using TWorldGenerator = WorldMapGenerator; using TCountriesGenerator = CountryMapGenerator>; - generator::SyncOfstream m_featureIdToOsmIds; unique_ptr m_countries; unique_ptr m_world; @@ -337,17 +335,8 @@ public: m_coastsHolder.reset( new feature::FeaturesCollector(info.GetTmpFileName(WORLD_COASTS_FILE_NAME))); - string const featureIdToOsmIdsFile = info.GetIntermediateFileName(info.m_featureIdToOsmIds, ""); - LOG(LINFO, ("Saving mapping from feature ids to osm ids to", featureIdToOsmIdsFile)); - m_featureIdToOsmIds.Open(featureIdToOsmIdsFile); - if (!m_featureIdToOsmIds.IsOpened()) - { - LOG(LWARNING, - ("Cannot open", featureIdToOsmIdsFile, ". Feature ids to osm ids to map won't be saved.")); - } - if (info.m_splitByPolygons || !info.m_fileName.empty()) - m_countries = make_unique(info, m_featureIdToOsmIds); + m_countries = make_unique(info); if (info.m_createWorld) m_world.reset(new TWorldGenerator(info)); diff --git a/generator/polygonizer.hpp b/generator/polygonizer.hpp index 5b6f0ec5ba..f796a7e6aa 100644 --- a/generator/polygonizer.hpp +++ b/generator/polygonizer.hpp @@ -4,7 +4,6 @@ #include "generator/feature_builder.hpp" #include "generator/generate_info.hpp" #include "generator/osm_source.hpp" -#include "generator/sync_ofsteam.hpp" #include "indexer/feature_visibility.hpp" #include "indexer/cell_id.hpp" @@ -42,8 +41,6 @@ namespace feature vector m_Names; borders::CountriesContainerT m_countries; - generator::SyncOfstream & m_featureIdToOsmIds; - #if PARALLEL_POLYGONIZER QThreadPool m_ThreadPool; QSemaphore m_ThreadPoolSemaphore; @@ -51,8 +48,7 @@ namespace feature #endif public: - Polygonizer(feature::GenerateInfo const & info, generator::SyncOfstream & featureIdToOsmIds) - : m_info(info), m_featureIdToOsmIds(featureIdToOsmIds) + Polygonizer(feature::GenerateInfo const & info) : m_info(info) #if PARALLEL_POLYGONIZER , m_ThreadPoolSemaphore(m_ThreadPool.maxThreadCount() * 8) #endif @@ -122,14 +118,14 @@ namespace feature { case 0: break; - case 1: EmitFeature(vec[0], fb, m_featureIdToOsmIds); break; + case 1: EmitFeature(vec[0], fb); break; default: { #if PARALLEL_POLYGONIZER m_ThreadPoolSemaphore.acquire(); - m_ThreadPool.start(new PolygonizerTask(this, vec, fb, m_featureIdToOsmIds)); + m_ThreadPool.start(new PolygonizerTask(this, vec, fb)); #else - PolygonizerTask task(this, vec, fb, m_featureIdToOsmIds); + PolygonizerTask task(this, vec, fb); task.RunBase(); #endif } @@ -150,8 +146,7 @@ namespace feature #endif } - void EmitFeature(borders::CountryPolygons const * country, FeatureBuilder1 const & fb, - generator::SyncOfstream & featureIdToOsmIds) + void EmitFeature(borders::CountryPolygons const * country, FeatureBuilder1 const & fb) { #if PARALLEL_POLYGONIZER QMutexLocker mutexLocker(&m_EmitFeatureMutex); @@ -169,10 +164,7 @@ namespace feature m_currentNames += country->m_name; auto & bucket = *(m_Buckets[country->m_index]); - uint32_t const featureId = bucket(fb); - - if (fb.IsLine()) - featureIdToOsmIds.Write(featureId /* feature id of |fb| */, fb.GetOsmIds()); + bucket(fb); } vector const & Names() const @@ -191,11 +183,10 @@ namespace feature public: PolygonizerTask(Polygonizer * pPolygonizer, buffer_vector const & countries, - FeatureBuilder1 const & fb, generator::SyncOfstream & featureIdToOsmIds) + FeatureBuilder1 const & fb) : m_pPolygonizer(pPolygonizer) , m_Countries(countries) , m_fb(fb) - , m_featureIdToOsmIds(featureIdToOsmIds) { } @@ -207,7 +198,7 @@ namespace feature m_fb.ForEachGeometryPoint(doCheck); if (doCheck.m_belongs) - m_pPolygonizer->EmitFeature(m_Countries[i], m_fb, m_featureIdToOsmIds); + m_pPolygonizer->EmitFeature(m_Countries[i], m_fb); } } @@ -224,7 +215,6 @@ namespace feature Polygonizer * m_pPolygonizer; buffer_vector m_Countries; FeatureBuilder1 m_fb; - generator::SyncOfstream & m_featureIdToOsmIds; }; }; } diff --git a/generator/sync_ofsteam.cpp b/generator/sync_ofsteam.cpp deleted file mode 100644 index 885a737428..0000000000 --- a/generator/sync_ofsteam.cpp +++ /dev/null @@ -1,30 +0,0 @@ -#include "sync_ofsteam.hpp" - -#include "std/iostream.hpp" - -namespace generator -{ -void SyncOfstream::Open(string const & fullPath) -{ - lock_guard guard(m_mutex); - m_stream.open(fullPath, std::ofstream::out); -} - -bool SyncOfstream::IsOpened() -{ - lock_guard guard(m_mutex); - return m_stream.is_open() && !m_stream.fail(); -} - -void SyncOfstream::Write(uint32_t featureId, vector const & osmIds) -{ - if (!IsOpened()) - return; - - lock_guard guard(m_mutex); - m_stream << featureId; - for (osm::Id const & osmId : osmIds) - m_stream << "," << osmId.OsmId(); - m_stream << endl; -} -} // namespace generator diff --git a/generator/sync_ofsteam.hpp b/generator/sync_ofsteam.hpp deleted file mode 100644 index 856cdac6c1..0000000000 --- a/generator/sync_ofsteam.hpp +++ /dev/null @@ -1,23 +0,0 @@ -#pragma once - -#include "generator/osm_id.hpp" - -#include "std/fstream.hpp" -#include "std/mutex.hpp" -#include "std/string.hpp" -#include "std/vector.hpp" - -namespace generator -{ -class SyncOfstream -{ -public: - void Open(string const & fullPath); - bool IsOpened(); - void Write(uint32_t featureId, vector const & osmIds); - -private: - ofstream m_stream; - mutex m_mutex; -}; -} // namespace generator diff --git a/generator/world_map_generator.hpp b/generator/world_map_generator.hpp index 5cc9322ab6..cb2d4a73ff 100644 --- a/generator/world_map_generator.hpp +++ b/generator/world_map_generator.hpp @@ -312,8 +312,7 @@ class CountryMapGenerator FeatureOutT m_bucket; public: - CountryMapGenerator(feature::GenerateInfo const & info, generator::SyncOfstream & featureIdToOsmIds) - : m_bucket(info, featureIdToOsmIds) {} + CountryMapGenerator(feature::GenerateInfo const & info) : m_bucket(info) {} void operator()(FeatureBuilder1 fb) { From 71f64bdbbf1dacb1aed4b1d9a546b400fb4e2f81 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 16 Nov 2016 09:13:09 +0300 Subject: [PATCH 5/8] Fixing test RestrictionTest_RestrictionCollectorWholeClassTest and review fixes. --- generator/gen_mwm_info.hpp | 12 ++++++------ .../generator_tests/restriction_collector_test.cpp | 10 +++++----- generator/generator_tests/restriction_test.cpp | 4 ++-- .../generator_tests_support.pro | 4 ++-- ...restrcion_support.cpp => restriction_helpers.cpp} | 10 ++++++---- ...restrcion_support.hpp => restriction_helpers.hpp} | 4 ++-- generator/restriction_collector.cpp | 7 ++----- 7 files changed, 25 insertions(+), 26 deletions(-) rename generator/generator_tests_support/{restrcion_support.cpp => restriction_helpers.cpp} (76%) rename generator/generator_tests_support/{restrcion_support.hpp => restriction_helpers.hpp} (69%) diff --git a/generator/gen_mwm_info.hpp b/generator/gen_mwm_info.hpp index d9f8ce99af..b1f875b0f9 100644 --- a/generator/gen_mwm_info.hpp +++ b/generator/gen_mwm_info.hpp @@ -5,10 +5,8 @@ #include "base/assert.hpp" #include "std/algorithm.hpp" -#include "std/fstream.hpp" -#include "std/iostream.hpp" -#include "std/string.hpp" #include "std/utility.hpp" +#include "std/vector.hpp" namespace gen { @@ -31,11 +29,11 @@ public: { rw::ReadVectorOfPOD(src, m_data); } - - vector const & GetData() const { return m_data; } }; -class OsmID2FeatureID : public Accumulator> +using OsmIdAndFeatureId = pair; + +class OsmID2FeatureID : public Accumulator { typedef Accumulator BaseT; @@ -65,5 +63,7 @@ public: else return 0; } + + vector const & GetData() const { return m_data; } }; } // namespace gen diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index 852f7193b6..94ea9c0ad9 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -1,6 +1,6 @@ #include "testing/testing.hpp" -#include "generator/generator_tests_support/restrcion_support.hpp" +#include "generator/generator_tests_support/restriction_helpers.hpp" #include "generator/osm_id.hpp" #include "generator/restriction_collector.hpp" @@ -90,11 +90,14 @@ UNIT_TEST(RestrictionTest_ParseRestrictions) UNIT_TEST(RestrictionTest_RestrictionCollectorWholeClassTest) { + ScopedDir scopedDir(kRestrictionTestDir); + string const kRestrictionName = "restrictions_in_osm_ids.csv"; string const kRestrictionPath = my::JoinFoldersToPath(kRestrictionTestDir, kRestrictionName); string const kRestrictionContent = R"(No, 10, 10, Only, 10, 20, Only, 30, 40,)"; + ScopedFile restrictionScopedFile(kRestrictionPath, kRestrictionContent); string const kOsmIdsToFeatureIdsName = "osm_ids_to_feature_ids" OSM2FEATURE_FILE_EXTENSION; string const osmIdsToFeatureIdsPath = @@ -106,10 +109,7 @@ UNIT_TEST(RestrictionTest_RestrictionCollectorWholeClassTest) Platform const & platform = Platform(); string const osmIdsToFeatureIdsFullPath = my::JoinFoldersToPath(platform.WritableDir(), osmIdsToFeatureIdsPath); - GenerateOsmIdsToFeatureIdsMapping(kOsmIdsToFeatureIdsContent, osmIdsToFeatureIdsFullPath); - - ScopedDir scopedDir(kRestrictionTestDir); - ScopedFile restrictionScopedFile(kRestrictionPath, kRestrictionContent); + ReEncodeOsmIdsToFeatureIdsMapping(kOsmIdsToFeatureIdsContent, osmIdsToFeatureIdsFullPath); ScopedFile mappingScopedFile(osmIdsToFeatureIdsPath); RestrictionCollector restrictionCollector( diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index efd6e5ff96..71b5aef962 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -1,6 +1,6 @@ #include "testing/testing.hpp" -#include "generator/generator_tests_support/restrcion_support.hpp" +#include "generator/generator_tests_support/restriction_helpers.hpp" #include "generator/generator_tests_support/test_feature.hpp" #include "generator/generator_tests_support/test_mwm_builder.hpp" @@ -69,7 +69,7 @@ void TestRestrictionBuilding(string const & restrictionContent, string const & m string const mappingRelativePath = my::JoinFoldersToPath(kTestDir, kOsmIdsToFeatureIdsName); ScopedFile const mappingScopedFile(mappingRelativePath); string const mappingFullPath = my::JoinFoldersToPath(writableDir, mappingRelativePath); - GenerateOsmIdsToFeatureIdsMapping(mappingContent, mappingFullPath); + ReEncodeOsmIdsToFeatureIdsMapping(mappingContent, mappingFullPath); // Adding restriction section to mwm. string const restrictionFullPath = my::JoinFoldersToPath(writableDir, restrictionRelativePath); diff --git a/generator/generator_tests_support/generator_tests_support.pro b/generator/generator_tests_support/generator_tests_support.pro index af3ff085e2..46b0560b79 100644 --- a/generator/generator_tests_support/generator_tests_support.pro +++ b/generator/generator_tests_support/generator_tests_support.pro @@ -7,11 +7,11 @@ ROOT_DIR = ../.. include($$ROOT_DIR/common.pri) SOURCES += \ - restrcion_support.cpp \ + restriction_helpers.cpp \ test_feature.cpp \ test_mwm_builder.cpp \ HEADERS += \ - restrcion_support.hpp \ + restriction_helpers.hpp \ test_feature.hpp \ test_mwm_builder.hpp \ diff --git a/generator/generator_tests_support/restrcion_support.cpp b/generator/generator_tests_support/restriction_helpers.cpp similarity index 76% rename from generator/generator_tests_support/restrcion_support.cpp rename to generator/generator_tests_support/restriction_helpers.cpp index 678cc37fd4..3ac0aacba3 100644 --- a/generator/generator_tests_support/restrcion_support.cpp +++ b/generator/generator_tests_support/restriction_helpers.cpp @@ -1,4 +1,4 @@ -#include "generator/generator_tests_support/restrcion_support.hpp" +#include "generator/generator_tests_support/restriction_helpers.hpp" #include "testing/testing.hpp" @@ -12,20 +12,22 @@ namespace generator { -void GenerateOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath) +void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath) { - strings::SimpleTokenizer lineIter(mappingContent, "\n\r" /* string delimiter */); + strings::SimpleTokenizer lineIter(mappingContent, "\n\r" /* line delimiters */); gen::Accumulator> osmIdsToFeatureIds; for (; lineIter; ++lineIter) { - strings::SimpleTokenizer idIter(*lineIter, ", \t" /* id delimiter */); + strings::SimpleTokenizer idIter(*lineIter, ", \t" /* id delimiters */); uint64_t osmId = 0; + TEST(idIter, ()); TEST(strings::to_uint64(*idIter, osmId), ("Cannot covert to uint64_t:", *idIter)); TEST(idIter, ("Wrong feature ids to osm ids mapping.")); ++idIter; uint32_t featureId = 0; + TEST(idIter, ()); TEST(strings::to_uint(*idIter, featureId), ("Cannot covert to uint:", *idIter)); osmIdsToFeatureIds.Add(make_pair(osmId, featureId)); } diff --git a/generator/generator_tests_support/restrcion_support.hpp b/generator/generator_tests_support/restriction_helpers.hpp similarity index 69% rename from generator/generator_tests_support/restrcion_support.hpp rename to generator/generator_tests_support/restriction_helpers.hpp index 2268158c5a..46a85a1ebf 100644 --- a/generator/generator_tests_support/restrcion_support.hpp +++ b/generator/generator_tests_support/restriction_helpers.hpp @@ -4,7 +4,7 @@ namespace generator { -/// \brief Generates a binary file with by a string with mapping from osm ids to feature ids. +/// \brief Generates a binary file by |mappingContent| with mapping from osm ids to feature ids. /// \param mappingContent a string with lines with mapping from osm id to feature id (one to one). /// For example /// 10, 1, @@ -12,5 +12,5 @@ namespace generator /// 30, 3, /// 40, 4 /// \parma outputFilePath full path to an output file where the mapping is saved. -void GenerateOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath); +void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath); } // namespace generator diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index 58c85e27a6..7e6ff19828 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -72,10 +72,7 @@ bool RestrictionCollector::IsValid() const bool RestrictionCollector::ParseOsmIdToFeatureIdMapping(string const & osmIdsToFeatureIdPath) { - LOG(LINFO, ("osmIdsToFeatureIdPath =", osmIdsToFeatureIdPath)); - - using OsmIdToFeatureId = pair; - gen::Accumulator osmIdsToFeatureIds; + gen::OsmID2FeatureID osmIdsToFeatureIds; try { FileReader reader(osmIdsToFeatureIdPath); @@ -88,7 +85,7 @@ bool RestrictionCollector::ParseOsmIdToFeatureIdMapping(string const & osmIdsToF return false; } - vector const & mapping = osmIdsToFeatureIds.GetData(); + vector const & mapping = osmIdsToFeatureIds.GetData(); if (mapping.empty()) { LOG(LINFO, ("No osm ids to feature ids mapping in file", osmIdsToFeatureIdPath)); From ba4656b2ee9e35744669ad8e2e29afe443214253 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 16 Nov 2016 14:19:32 +0300 Subject: [PATCH 6/8] Review fixes. --- generator/gen_mwm_info.hpp | 11 +++++++---- generator/generator_tests/restriction_test.cpp | 12 ++++++------ .../generator_tests_support/restriction_helpers.cpp | 2 ++ generator/restriction_collector.cpp | 12 +++--------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/generator/gen_mwm_info.hpp b/generator/gen_mwm_info.hpp index b1f875b0f9..6b18742a21 100644 --- a/generator/gen_mwm_info.hpp +++ b/generator/gen_mwm_info.hpp @@ -31,9 +31,7 @@ public: } }; -using OsmIdAndFeatureId = pair; - -class OsmID2FeatureID : public Accumulator +class OsmID2FeatureID : public Accumulator> { typedef Accumulator BaseT; @@ -64,6 +62,11 @@ public: return 0; } - vector const & GetData() const { return m_data; } + template + void ForEach(Fn && fn) const + { + for (ValueT const & v : m_data) + fn(v); + } }; } // namespace gen diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index 71b5aef962..d0e4d2d03a 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -116,9 +116,9 @@ UNIT_TEST(RestrictionGenerationTest_ThreeRestrictions) Only, 10, 20 Only, 30, 40)"; string const osmIdsToFeatureIdsContent = R"(10, 1, - 20, 2 - 30, 3, - 40, 4)"; + 20, 2 + 30, 3, + 40, 4)"; TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } @@ -132,9 +132,9 @@ UNIT_TEST(RestrictionGenerationTest_SevenRestrictions) No, 40, 40, Only, 30, 40,)"; string const osmIdsToFeatureIdsContent = R"(10, 1, - 20, 2, - 30, 3, - 40, 4)"; + 20, 2, + 30, 3, + 40, 4)"; TestRestrictionBuilding(restrictionContent, osmIdsToFeatureIdsContent); } diff --git a/generator/generator_tests_support/restriction_helpers.cpp b/generator/generator_tests_support/restriction_helpers.cpp index 3ac0aacba3..c65a1f2f64 100644 --- a/generator/generator_tests_support/restriction_helpers.cpp +++ b/generator/generator_tests_support/restriction_helpers.cpp @@ -30,6 +30,8 @@ void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, string con TEST(idIter, ()); TEST(strings::to_uint(*idIter, featureId), ("Cannot covert to uint:", *idIter)); osmIdsToFeatureIds.Add(make_pair(osmId, featureId)); + ++idIter; + TEST(!idIter, ()); } FileWriter osm2ftWriter(outputFilePath); diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index 7e6ff19828..6f655a6157 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -85,15 +85,9 @@ bool RestrictionCollector::ParseOsmIdToFeatureIdMapping(string const & osmIdsToF return false; } - vector const & mapping = osmIdsToFeatureIds.GetData(); - if (mapping.empty()) - { - LOG(LINFO, ("No osm ids to feature ids mapping in file", osmIdsToFeatureIdPath)); - return true; - } - - for (auto const osmIdToFeatureId : mapping) - AddFeatureId(osmIdToFeatureId.second /* feature id */, osmIdToFeatureId.first /* osm id */); + osmIdsToFeatureIds.ForEach([this](gen::OsmID2FeatureID::ValueT const & p){ + AddFeatureId(p.second /* feature id */, p.first /* osm id */); + }); return true; } From 672a8001c2d366e0aabbd3ad33eec7c7c8cbea02 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 16 Nov 2016 14:22:18 +0300 Subject: [PATCH 7/8] git-clang-format --- .../generator_tests/restriction_collector_test.cpp | 13 ++++++++----- .../generator_tests_support/restriction_helpers.hpp | 3 ++- generator/polygonizer.hpp | 9 ++++----- generator/restriction_collector.cpp | 2 +- generator/restriction_generator.cpp | 4 ++-- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index 94ea9c0ad9..be561e4867 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -30,7 +30,8 @@ string const kRestrictionTestDir = "test-restrictions"; UNIT_TEST(RestrictionTest_ValidCase) { - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* osmIdsToFeatureIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, + "" /* osmIdsToFeatureIdsPath */); // Adding feature ids. restrictionCollector.AddFeatureId(30 /* featureId */, 3 /* osmId */); restrictionCollector.AddFeatureId(10 /* featureId */, 1 /* osmId */); @@ -55,7 +56,8 @@ UNIT_TEST(RestrictionTest_ValidCase) UNIT_TEST(RestrictionTest_InvalidCase) { - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* osmIdsToFeatureIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, + "" /* osmIdsToFeatureIdsPath */); restrictionCollector.AddFeatureId(0 /* featureId */, 0 /* osmId */); restrictionCollector.AddFeatureId(20 /* featureId */, 2 /* osmId */); @@ -78,7 +80,8 @@ UNIT_TEST(RestrictionTest_ParseRestrictions) ScopedDir const scopedDir(kRestrictionTestDir); ScopedFile const scopedFile(kRestrictionPath, kRestrictionContent); - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* osmIdsToFeatureIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, + "" /* osmIdsToFeatureIdsPath */); Platform const & platform = Platform(); @@ -107,8 +110,8 @@ UNIT_TEST(RestrictionTest_RestrictionCollectorWholeClassTest) 30, 3, 40, 4)"; Platform const & platform = Platform(); - string const osmIdsToFeatureIdsFullPath = my::JoinFoldersToPath(platform.WritableDir(), - osmIdsToFeatureIdsPath); + string const osmIdsToFeatureIdsFullPath = + my::JoinFoldersToPath(platform.WritableDir(), osmIdsToFeatureIdsPath); ReEncodeOsmIdsToFeatureIdsMapping(kOsmIdsToFeatureIdsContent, osmIdsToFeatureIdsFullPath); ScopedFile mappingScopedFile(osmIdsToFeatureIdsPath); diff --git a/generator/generator_tests_support/restriction_helpers.hpp b/generator/generator_tests_support/restriction_helpers.hpp index 46a85a1ebf..1b2b4cc9a8 100644 --- a/generator/generator_tests_support/restriction_helpers.hpp +++ b/generator/generator_tests_support/restriction_helpers.hpp @@ -12,5 +12,6 @@ namespace generator /// 30, 3, /// 40, 4 /// \parma outputFilePath full path to an output file where the mapping is saved. -void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath); +void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, + string const & outputFilePath); } // namespace generator diff --git a/generator/polygonizer.hpp b/generator/polygonizer.hpp index f796a7e6aa..03b79b3539 100644 --- a/generator/polygonizer.hpp +++ b/generator/polygonizer.hpp @@ -48,9 +48,10 @@ namespace feature #endif public: - Polygonizer(feature::GenerateInfo const & info) : m_info(info) + Polygonizer(feature::GenerateInfo const & info) + : m_info(info) #if PARALLEL_POLYGONIZER - , m_ThreadPoolSemaphore(m_ThreadPool.maxThreadCount() * 8) + , m_ThreadPoolSemaphore(m_ThreadPool.maxThreadCount() * 8) #endif { #if PARALLEL_POLYGONIZER @@ -184,9 +185,7 @@ namespace feature PolygonizerTask(Polygonizer * pPolygonizer, buffer_vector const & countries, FeatureBuilder1 const & fb) - : m_pPolygonizer(pPolygonizer) - , m_Countries(countries) - , m_fb(fb) + : m_pPolygonizer(pPolygonizer), m_Countries(countries), m_fb(fb) { } diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index 6f655a6157..b2faddb556 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -85,7 +85,7 @@ bool RestrictionCollector::ParseOsmIdToFeatureIdMapping(string const & osmIdsToF return false; } - osmIdsToFeatureIds.ForEach([this](gen::OsmID2FeatureID::ValueT const & p){ + osmIdsToFeatureIds.ForEach([this](gen::OsmID2FeatureID::ValueT const & p) { AddFeatureId(p.second /* feature id */, p.first /* osm id */); }); diff --git a/generator/restriction_generator.cpp b/generator/restriction_generator.cpp index 1320686461..7ee0a7e3a3 100644 --- a/generator/restriction_generator.cpp +++ b/generator/restriction_generator.cpp @@ -17,8 +17,8 @@ namespace routing bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPath, string const & osmIdsTofeatureIdsPath) { - LOG(LINFO, - ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", osmIdsTofeatureIdsPath, ");")); + LOG(LINFO, ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", + osmIdsTofeatureIdsPath, ");")); RestrictionCollector restrictionCollector(restrictionPath, osmIdsTofeatureIdsPath); if (!restrictionCollector.HasRestrictions() || !restrictionCollector.IsValid()) { From 0ffae52152c4735a38406fe8b9bae1f2809d71f4 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 16 Nov 2016 14:59:48 +0300 Subject: [PATCH 8/8] Minor fixes. --- generator/gen_mwm_info.hpp | 2 +- generator/generator_tests_support/restriction_helpers.cpp | 4 ++-- generator/generator_tests_support/restriction_helpers.hpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/generator/gen_mwm_info.hpp b/generator/gen_mwm_info.hpp index 6b18742a21..9a36814a7b 100644 --- a/generator/gen_mwm_info.hpp +++ b/generator/gen_mwm_info.hpp @@ -65,7 +65,7 @@ public: template void ForEach(Fn && fn) const { - for (ValueT const & v : m_data) + for (auto const & v : m_data) fn(v); } }; diff --git a/generator/generator_tests_support/restriction_helpers.cpp b/generator/generator_tests_support/restriction_helpers.cpp index c65a1f2f64..d4ff6cba77 100644 --- a/generator/generator_tests_support/restriction_helpers.cpp +++ b/generator/generator_tests_support/restriction_helpers.cpp @@ -22,13 +22,13 @@ void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, string con strings::SimpleTokenizer idIter(*lineIter, ", \t" /* id delimiters */); uint64_t osmId = 0; TEST(idIter, ()); - TEST(strings::to_uint64(*idIter, osmId), ("Cannot covert to uint64_t:", *idIter)); + TEST(strings::to_uint64(*idIter, osmId), ("Cannot convert to uint64_t:", *idIter)); TEST(idIter, ("Wrong feature ids to osm ids mapping.")); ++idIter; uint32_t featureId = 0; TEST(idIter, ()); - TEST(strings::to_uint(*idIter, featureId), ("Cannot covert to uint:", *idIter)); + TEST(strings::to_uint(*idIter, featureId), ("Cannot convert to uint:", *idIter)); osmIdsToFeatureIds.Add(make_pair(osmId, featureId)); ++idIter; TEST(!idIter, ()); diff --git a/generator/generator_tests_support/restriction_helpers.hpp b/generator/generator_tests_support/restriction_helpers.hpp index 1b2b4cc9a8..83f1a174f2 100644 --- a/generator/generator_tests_support/restriction_helpers.hpp +++ b/generator/generator_tests_support/restriction_helpers.hpp @@ -11,7 +11,7 @@ namespace generator /// 20, 2 /// 30, 3, /// 40, 4 -/// \parma outputFilePath full path to an output file where the mapping is saved. +/// \param outputFilePath full path to an output file where the mapping is saved. void ReEncodeOsmIdsToFeatureIdsMapping(string const & mappingContent, string const & outputFilePath); } // namespace generator