[routing] review fixes

generator/feature_segments_checker/CMakeLists.txt

@@ -28,6 +28,7 @@ omim_link_libraries(
   jansson
   minizip
   oauthcpp
+  opening_hours
   protobuf
   stats_client
   succinct

generator/generator_tests/road_access_test.cpp

@@ -63,6 +63,37 @@ void BuildTestMwmWithRoads(LocalCountryFile & country)
   }
 }
 
+size_t GetLinesNumber(string const & text)
+{
+  stringstream ss;
+  ss << text.data();
+  string line;
+  size_t n = 0;
+  while (getline(ss, line))
+    ++n;
+  return n;
+}
+
+bool ExistsConsecutiveLines(string const & text, vector<string> const & lines)
+{
+  stringstream ss;
+  ss << text.data();
+  size_t lineIndex = 0;
+  string lineFromText;
+  while (getline(ss, lineFromText))
+  {
+    if (lineFromText == lines[lineIndex])
+      ++lineIndex;
+    else
+      lineIndex = 0;
+
+    if (lineIndex == lines.size())
+      return true;
+  }
+
+  return false;
+}
+
 void LoadRoadAccess(string const & mwmFilePath, VehicleType vehicleType, RoadAccess & roadAccess)
 {
   FilesContainerR const cont(mwmFilePath);
@@ -280,7 +311,14 @@ UNIT_TEST(RoadAccessCoditionalParse)
         {RoadAccess::Type::Destination, "06:30-08:30 AND agricultural"}}},
 
       {"no @ (Mo-Fr 00:00-08:00,20:00-24:00; Sa-Su 00:00-24:00; PH 00:00-24:00)",
-       {{RoadAccess::Type::No, "Mo-Fr 00:00-08:00,20:00-24:00; Sa-Su 00:00-24:00; PH 00:00-24:00"}}}
+       {{RoadAccess::Type::No, "Mo-Fr 00:00-08:00,20:00-24:00; Sa-Su 00:00-24:00; PH 00:00-24:00"}}},
+
+      // Not valid cases
+      {"trash @ (Mo-Fr 00:00-10:00)", {{RoadAccess::Type::Count, "Mo-Fr 00:00-10:00"}}},
+      {"yes Mo-Fr", {}},
+      {"yes (Mo-Fr)", {}},
+      {"no ; Mo-Fr", {}},
+      {"asdsadasdasd", {}}
   };
 
   vector<string> tags = {
@@ -346,15 +384,25 @@ UNIT_TEST(RoadAccessWriter_ConditionalMerge)
   stringstream buffer;
   buffer << stream.rdbuf();
 
-  string const correctAnswer = "Car 3 2\n"
-                               "Private 12:00-19:00\n"
-                               "No Mo-Su\n"
-                               "Car 2 1\n"
-                               "Private 10:00-20:00\n"
-                               "Car 1 1\n"
-                               "No Mo-Su\n";
+  size_t linesNumber = 0;
+  auto const test = [&linesNumber, &buffer](vector<string> const & lines) {
+    TEST(ExistsConsecutiveLines(buffer.str(), lines), (buffer.str(), lines));
+    linesNumber += lines.size();
+  };
+
+  test({"Car 3 2",
+        "Private 12:00-19:00",
+        "No Mo-Su"});
 
-  TEST_EQUAL(buffer.str(), correctAnswer, ());
+  test({
+      "Car 2 1",
+      "Private 10:00-20:00"});
+
+  test({
+      "Car 1 1",
+      "No Mo-Su"});
+
+  TEST_EQUAL(GetLinesNumber(buffer.str()), linesNumber, ());
 }
 
 UNIT_TEST(RoadAccessWriter_Conditional_WinterRoads)
@@ -386,19 +434,25 @@ UNIT_TEST(RoadAccessWriter_Conditional_WinterRoads)
   stringstream buffer;
   buffer << stream.rdbuf();
 
-  string const correctAnswer = "Pedestrian 2 1\n"
-                               "No Feb - Dec\n"
-                               "Pedestrian 1 1\n"
-                               "No Feb - Dec\n"
-                               "Bicycle 2 1\n"
-                               "No Feb - Dec\n"
-                               "Bicycle 1 1\n"
-                               "No Feb - Dec\n"
-                               "Car 2 1\n"
-                               "No Feb - Dec\n"
-                               "Car 1 1\n"
-                               "No Feb - Dec\n";
+  size_t linesNumber = 0;
+  auto const test = [&linesNumber, &buffer](vector<string> const & lines) {
+    TEST(ExistsConsecutiveLines(buffer.str(), lines), (buffer.str(), lines));
+    linesNumber += lines.size();
+  };
 
-  TEST_EQUAL(buffer.str(), correctAnswer, ());
+  test({"Pedestrian 2 1",
+        "No Mar - Nov"});
+  test({"Pedestrian 1 1",
+        "No Mar - Nov"});
+  test({"Bicycle 2 1",
+        "No Mar - Nov"});
+  test({"Bicycle 1 1",
+        "No Mar - Nov"});
+  test({"Car 2 1",
+        "No Mar - Nov"});
+  test({"Car 1 1",
+        "No Mar - Nov"});
+
+  TEST_EQUAL(GetLinesNumber(buffer.str()), linesNumber, ());
 }
 }  // namespace

generator/road_access_generator.cpp

@@ -47,7 +47,7 @@ using ConditionalTagsList = routing::RoadAccessTagProcessor::ConditionalTagsList
 // Get from: https://taginfo.openstreetmap.org/search?q=%3Aconditional
 // @{
 vector<string> const kCarAccessConditionalTags = {
-    "motorcar:conditional", "vehicle:conditional", "motor_vehicle:conditional"
+    "motor_vehicle:conditional", "motorcar:conditional", "vehicle:conditional",
 };
 
 vector<string> const kDefaultAccessConditionalTags = {
@@ -348,8 +348,10 @@ optional<pair<string, string>> GetTagValueConditionalAccess(
   for (auto const & tags : tagsList)
   {
     for (auto const & tag : tags)
+    {
       if (elem.HasTag(tag))
         return make_pair(tag, elem.GetTag(tag));
+    }
   }
 
   if (tagsList.empty())
@@ -473,7 +475,11 @@ void RoadAccessTagProcessor::ProcessConditional(OsmElement const & elem)
   auto accesses = parser.ParseAccessConditionalTag(tag, value);
 
   auto & toSave = m_wayToAccessConditional[elem.m_id];
-  toSave.insert(toSave.end(), move_iterator(accesses.begin()), move_iterator(accesses.end()));
+  for (auto & access : accesses)
+  {
+    if (access.m_accessType != RoadAccess::Type::Count)
+      toSave.emplace_back(move(access));
+  }
 }
 
 void RoadAccessTagProcessor::WriteWayToAccess(ostream & stream)
@@ -773,13 +779,13 @@ string AccessConditionalTagParser::TrimAndDropAroundParentheses(string input) co
 {
   strings::Trim(input);
 
-  if (input.back() == ';')
-    input.erase(std::prev(input.end()));
+  if (!input.empty() && input.back() == ';')
+    input.pop_back();
 
-  if (input.front() == '(' && input.back() == ')')
+  if (input.size() >= 2 && input.front() == '(' && input.back() == ')')
   {
     input.erase(input.begin());
-    input.erase(std::prev(input.end()));
+    input.pop_back();
   }
 
   return input;

generator/srtm_coverage_checker/CMakeLists.txt

@@ -29,6 +29,7 @@ omim_link_libraries(
   minizip
   oauthcpp
   protobuf
+  opening_hours
   stats_client
   succinct
   ${LIBZ}

routing/CMakeLists.txt

@@ -99,6 +99,7 @@ set(
   restrictions_serialization.hpp
   road_access.cpp
   road_access.hpp
+  road_access_serialization.cpp
   road_access_serialization.hpp
   road_graph.cpp
   road_graph.hpp

routing/road_access.hpp

@@ -108,12 +108,12 @@ class RoadAccess final
     m_pointToAccess = std::forward<PointToAccess>(pointToAccess);
   }
 
-  template <typename WayConditional>
+  template <typename WayConditional, typename PointConditional>
   void SetAccessConditional(WayConditional && wayToAccessConditional,
-                            PointToAccessConditional && pointToAccessConditional)
+                            PointConditional && pointToAccessConditional)
   {
     m_wayToAccessConditional = std::forward<WayConditional>(wayToAccessConditional);
-    m_pointToAccessConditional = std::forward<PointToAccessConditional>(pointToAccessConditional);
+    m_pointToAccessConditional = std::forward<PointConditional>(pointToAccessConditional);
   }
 
   bool operator==(RoadAccess const & rhs) const;

routing/road_access_serialization.cpp

@@ -0,0 +1,17 @@
+#include "routing/road_access_serialization.hpp"
+
+#include <string>
+
+namespace routing
+{
+std::string DebugPrint(RoadAccessSerializer::Header const & header)
+{
+  switch (header)
+  {
+  case RoadAccessSerializer::Header::WithoutAccessConditional: return "WithoutAccessConditional";
+  case RoadAccessSerializer::Header::WithAccessConditional: return "WithAccessConditional";
+  }
+  UNREACHABLE();
+}
+}  // namespace routing
+

routing/road_access_serialization.hpp

@@ -37,12 +37,18 @@ class RoadAccessSerializer final
   using PointToAccessConditional = RoadAccess::PointToAccessConditional;
   using RoadAccessByVehicleType = std::array<RoadAccess, static_cast<size_t>(VehicleType::Count)>;
 
+  enum class Header
+  {
+    WithoutAccessConditional = 1,
+    WithAccessConditional = 2
+  };
+
   RoadAccessSerializer() = delete;
 
   template <class Sink>
   static void Serialize(Sink & sink, RoadAccessByVehicleType const & roadAccessByType)
   {
-    uint32_t const header = kLatestVersion;
+    Header const header = kLatestVersion;
     WriteToSink(sink, header);
     SerializeAccess(sink, roadAccessByType);
     SerializeAccessConditional(sink, roadAccessByType);
@@ -51,16 +57,16 @@ class RoadAccessSerializer final
   template <class Source>
   static void Deserialize(Source & src, VehicleType vehicleType, RoadAccess & roadAccess)
   {
-    uint32_t const header = ReadPrimitiveFromSource<uint32_t>(src);
+    auto const header = static_cast<Header>(ReadPrimitiveFromSource<uint32_t>(src));
     CHECK_LESS_OR_EQUAL(header, kLatestVersion, ());
     switch (header)
     {
-    case 1:
+    case Header::WithoutAccessConditional:
     {
       DeserializeAccess(src, vehicleType, roadAccess);
       break;
     }
-    case 2:
+    case Header::WithAccessConditional:
     {
       DeserializeAccess(src, vehicleType, roadAccess);
       DeserializeAccessConditional(src, vehicleType, roadAccess);
@@ -71,31 +77,19 @@ class RoadAccessSerializer final
   }
 
 private:
-  inline static uint32_t const kLatestVersion = 2;
+  inline static Header const kLatestVersion = Header::WithAccessConditional;
 
   class AccessPosition
   {
   public:
-//    friend void swap(AccessPosition & lhs, AccessPosition & rhs)
-//    {
-//      std::swap(lhs.m_featureId, rhs.m_featureId);
-//      std::swap(lhs.m_pointId, rhs.m_pointId);
-//    }
-
     static AccessPosition MakeWayAccess(uint32_t featureId)
     {
-      AccessPosition accessPosition;
-      accessPosition.m_featureId = featureId;
-      accessPosition.m_pointId = 0;  // wildcard pointId for way access.
-      return accessPosition;
+      return {featureId, 0 /* wildcard pointId for way access */};
     }
 
     static AccessPosition MakePointAccess(uint32_t featureId, uint32_t pointId)
     {
-      AccessPosition accessPosition;
-      accessPosition.m_featureId = featureId;
-      accessPosition.m_pointId = pointId + 1;
-      return accessPosition;
+      return {featureId, pointId + 1};
     }
 
     AccessPosition() = default;
@@ -177,7 +171,6 @@ class RoadAccessSerializer final
       DeserializeOneVehicleType(src, wayToAccess, pointToAccess);
 
       roadAccess.SetAccess(std::move(wayToAccess), std::move(pointToAccess));
-      return;
     }
   }
 
@@ -326,13 +319,12 @@ class RoadAccessSerializer final
 
     for (auto & positionsConditional : positionsByAccessType)
     {
-      std::stable_sort(positionsConditional.begin(), positionsConditional.end(),
-                       [](auto const & lhs, auto const & rhs)
-                       {
-                         auto const & lhsAccessPosition = lhs.first;
-                         auto const & rhsAccessPosition = rhs.first;
-                         return lhsAccessPosition < rhsAccessPosition;
-                       });
+      std::sort(positionsConditional.begin(), positionsConditional.end(),
+                [](auto const & lhs, auto const & rhs) {
+                  auto const & lhsAccessPosition = lhs.first;
+                  auto const & rhsAccessPosition = rhs.first;
+                  return lhsAccessPosition < rhsAccessPosition;
+                });
 
       SerializePositionsAccessConditional(sink, positionsConditional);
     }
@@ -522,4 +514,6 @@ class RoadAccessSerializer final
     return openingHoursSerDes;
   }
 };
+
+std::string DebugPrint(RoadAccessSerializer::Header const & header);
 }  // namespace routing

routing/routes_builder/routes_builder_tool/CMakeLists.txt

@@ -30,6 +30,7 @@ omim_link_libraries(
   icu
   jansson
   oauthcpp
+  opening_hours
   protobuf
   stats_client
   succinct

routing/routing_quality/routing_quality_tests/CMakeLists.txt

@@ -30,6 +30,7 @@ omim_link_libraries(
   coding
   base
   icu
+  opening_hours
   jansson
   oauthcpp
   protobuf

routing/routing_quality/routing_quality_tool/CMakeLists.txt

@@ -37,6 +37,7 @@ omim_link_libraries(
   base
   icu
   jansson
+  opening_hours
   oauthcpp
   protobuf
   stats_client

track_analyzing/track_analyzer/CMakeLists.txt

@@ -40,6 +40,7 @@ omim_link_libraries(
   icu
   jansson
   oauthcpp
+  opening_hours
   protobuf
   stats_client
   succinct

track_generator/CMakeLists.txt

@@ -32,6 +32,7 @@ omim_link_libraries(
   expat
   oauthcpp
   protobuf
+  opening_hours
   stats_client
   succinct
   gflags