move summaries to car profile, test for references, use boost adaptors

CHANGELOG.md

@@ -1,3 +1,10 @@
+# 5.4.0-rc.6
+  - Changes from 5.4.0-rc.5
+    - Bugfixes
+      - fixed a bug where polyline decoding on a defective polyline could end up in out-of-bound access on a vector
+    - Guidance
+      - Summaries have been improved to consider references as well
+
 # 5.4.0-rc.5
   - Changes from 5.4.0-rc.4
     - Guidance
@@ -17,6 +24,10 @@
     - Bugfixes
       - BREAKING: Fixed a bug where some roads could be falsly identified as sliproadsi This change requires reprocessing datasets with osrm-extract and osrm-contract
       - BREAKING: Fixed a bug that resulted in false names/ref/destination/pronunciation This change requires reprocessing datasets with osrm-extract and osrm-contract
+      - `restrictions` is now used for namespaced restrictions and restriction exceptions (e.g. `restriction:motorcar=` as well as `except=motorcar`)
+      - replaced lhs/rhs profiles by using test defined profiles
+    - Trip Plugin
+      - changed internal behaviour to prefer the smallest lexicographic result over the largest one
 
 # 5.4.0
   - Changes from 5.3.0

features/car/summaries.feature

@@ -0,0 +1,94 @@
+@routing @basic @car
+Feature: Basic Routing
+
+    Background:
+        Given the profile "car"
+        Given a grid size of 500 meters
+
+    @smallest
+    Scenario: Summaries when routing on a simple network
+        Given the node map
+            | b |   |   | f |
+            |   |   |   |   |
+            | c | d |   | g |
+            |   |   |   |   |
+            | a |   | e |   |
+
+        And the ways
+            | nodes | name   |
+            | acb   | road   |
+            | de    | 1 st   |
+            | cd    |        |
+            | dg    | blvd   |
+            | df    | street |
+
+        When I route I should get
+            | waypoints | route                               | summary               |
+            | a,e       | road,,1 st,1 st                     | road,1 st             |
+            | a,d,f     | road,,,street,street                | road;street           |
+            | a,e,f     | road,,1 st,1 st,1 st,street,street  | road,1 st;1 st,street |
+
+     Scenario: Name Empty
+        Given the node map
+            | a |   | b |   |   | c |
+
+        And the ways
+            | nodes | name |
+            | ab    | road |
+            | bc    |      |
+
+        When I route I should get
+            | waypoints | route  | summary |
+            | a,c       | road,  | road    |
+
+     Scenario: Name Empty But Ref
+        Given the node map
+            | a |   | b |   |   | c |
+
+        And the ways
+            | nodes | name | ref |
+            | ab    | road |     |
+            | bc    |      | 101 |
+
+        When I route I should get
+            | waypoints | route | summary  |
+            | a,c       | road, | road,101 |
+
+     Scenario: Only Refs
+        Given the node map
+            | a |   | b |   |   | c |
+
+        And the ways
+            | nodes | name | ref |
+            | ab    |      | 100 |
+            | bc    |      | 101 |
+
+        When I route I should get
+            | waypoints | route  | summary |
+            | a,c       | ,      | 100,101 |
+
+     Scenario: Single Ref
+        Given the node map
+            | a |   | b |   |   | c |
+
+        And the ways
+            | nodes | name | ref |
+            | ab    |      |     |
+            | bc    |      | 101 |
+
+        When I route I should get
+            | waypoints | route | summary |
+            | a,c       | ,,    | 101     |
+
+     Scenario: Nothing
+        Given the node map
+            | a |   | b |   |   | c |
+
+        And the ways
+            | nodes | name |
+            | ab    |      |
+            | bc    |      |
+
+        When I route I should get
+            | waypoints | route | summary |
+            | a,c       | ,     |         |

features/support/route.js

@@ -132,7 +132,7 @@ module.exports = function () {
 
     this.summary = (instructions) => {
         if (instructions) {
-            return instructions.legs.map(l => l.summary).join(',');
+            return instructions.legs.map(l => l.summary).join(';');
         }
     };
 

features/testbot/summary.feature

@@ -18,12 +18,12 @@ Feature: Basic Routing
             | de    |
 
         When I route I should get
-            | from | to | route          | summary  |
-            | a    | e  | ab,bc,cd,de,de | ab, bc   |
-            | e    | a  | de,cd,bc,ab,ab | de, bc   |
-            | a    | b  | ab,ab          | ab       |
-            | b    | d  | bc,cd,cd       | bc, cd   |
-            | 1    | c  | bc,bc          | bc       |
+            | from | to | route          | summary |
+            | a    | e  | ab,bc,cd,de,de | ab,bc   |
+            | e    | a  | de,cd,bc,ab,ab | de,bc   |
+            | a    | b  | ab,ab          | ab      |
+            | b    | d  | bc,cd,cd       | bc,cd   |
+            | 1    | c  | bc,bc          | bc      |
 
     @smallest
     Scenario: Check handling empty values
@@ -40,8 +40,8 @@ Feature: Basic Routing
             | df    | df   |
 
         When I route I should get
-            | from | to | route          | summary  |
-            | e    | a  | de,,bc,ab,ab   | de, bc   |
+            | from | to | route          | summary |
+            | e    | a  | de,,bc,ab,ab   | de,bc   |
 
     @smallest @todo
     Scenario: Summaries when routing on a simple network
@@ -53,9 +53,9 @@ Feature: Basic Routing
             | ab    |
 
         When I route I should get
-            | from | to | route | summary  |
-            | a    | b  | ab,ab | ab       |
-            | b    | a  | ab,ab | ab       |
+            | from | to | route | summary |
+            | a    | b  | ab,ab | ab      |
+            | b    | a  | ab,ab | ab      |
 
     @repeated
     Scenario: Check handling empty values
@@ -74,6 +74,6 @@ Feature: Basic Routing
             | xey   | cross  |we need this because phantom node segments are not considered for the summary |
 
         When I route I should get
-            | from | to | route                     | summary       |
-            | a    | 1  | first,first,second,second | first, second |
+            | from | to | route                     | summary      |
+            | a    | 1  | first,first,second,second | first,second |
 

include/engine/guidance/assemble_leg.hpp

@@ -6,6 +6,11 @@
 #include "engine/guidance/route_leg.hpp"
 #include "engine/guidance/route_step.hpp"
 #include "engine/internal_route_result.hpp"
+#include "util/typedefs.hpp"
+
+#include <boost/algorithm/string/join.hpp>
+#include <boost/range/adaptor/filtered.hpp>
+#include <boost/range/adaptor/transformed.hpp>
 
 #include <cstddef>
 #include <cstdint>
@@ -107,7 +112,7 @@ std::array<std::uint32_t, SegmentNumber> summarizeRoute(const std::vector<PathDa
         });
 
     std::array<std::uint32_t, SegmentNumber> summary;
-    std::fill(summary.begin(), summary.end(), 0);
+    std::fill(summary.begin(), summary.end(), EMPTY_NAMEID);
     std::transform(segments.begin(),
                    segments.end(),
                    summary.begin(),
@@ -172,21 +177,28 @@ inline RouteLeg assembleLeg(const datafacade::BaseDataFacade &facade,
     {
         auto summary_array = detail::summarizeRoute<detail::MAX_USED_SEGMENTS>(
             route_data, target_node, target_traversed_in_reverse);
-        if (route_data.empty())
-            summary_array[0] = source_node.name_id;
 
         BOOST_ASSERT(detail::MAX_USED_SEGMENTS > 0);
         BOOST_ASSERT(summary_array.begin() != summary_array.end());
-        summary = std::accumulate(std::next(summary_array.begin()),
-                                  summary_array.end(),
-                                  facade.GetNameForID(summary_array.front()),
-                                  [&facade](std::string previous, const std::uint32_t name_id) {
-                                      if (name_id != 0)
-                                      {
-                                          previous += ", " + facade.GetNameForID(name_id);
-                                      }
-                                      return previous;
-                                  });
+
+        // transform a name_id into a string containing either the name, or -if the name is empty-
+        // the reference.
+        const auto name_id_to_string = [&](const NameID name_id) {
+            const auto name = facade.GetNameForID(name_id);
+            if (!name.empty())
+                return name;
+            else
+            {
+                const auto ref = facade.GetRefForID(name_id);
+                return ref;
+            }
+        };
+
+        const auto not_empty = [&](const std::string &name) { return !name.empty(); };
+
+        const auto summary_names = summary_array | boost::adaptors::transformed(name_id_to_string) |
+                                   boost::adaptors::filtered(not_empty);
+        summary = boost::algorithm::join(summary_names, ",");
     }
 
     return RouteLeg{duration, distance, summary, {}};

src/engine/polyline_compressor.cpp

@@ -100,7 +100,7 @@ std::vector<util::Coordinate> decodePolyline(const std::string &geometry_string)
             b = geometry_string.at(index++) - 63;
             result |= (b & 0x1f) << shift;
             shift += 5;
-        } while (b >= 0x20);
+        } while (b >= 0x20 && index < len);
         int dlat = ((result & 1) != 0 ? ~(result >> 1) : (result >> 1));
         lat += dlat;
 
@@ -111,7 +111,7 @@ std::vector<util::Coordinate> decodePolyline(const std::string &geometry_string)
             b = geometry_string.at(index++) - 63;
             result |= (b & 0x1f) << shift;
             shift += 5;
-        } while (b >= 0x20);
+        } while (b >= 0x20 && index < len);
         int dlng = ((result & 1) != 0 ? ~(result >> 1) : (result >> 1));
         lng += dlng;