Canonicalizes Spaces in Semicolon Stringlists, fixes #3086

Project-OSRM · Oct 19, 2016 · ab1a927 · ab1a927
1 parent 18bc02f
commit ab1a927
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 17 deletions.
diff --git a/features/guidance/destination-signs.feature b/features/guidance/destination-signs.feature
@@ -31,13 +31,13 @@ Feature: Destination Signs
           | qr    | QR   |     |                | A1;A2           | yes    |                                      |
 
         When I route I should get
-          | from | to | route                                                     | destinations                                    | ref   | #                         |
-          | a    | b  | AB,AB                                                     | ,                                               | E1,E1 |                           |
-          | c    | d  | CD,CD                                                     | Berlin,Berlin                                   | ,     |                           |
-          | e    | f  | EF,EF                                                     | A1: Berlin,A1: Berlin                           | ,     |                           |
-          | g    | h  | ,                                                         | A1: Berlin,A1: Berlin                           | ,     |                           |
-          | i    | j  | ,                                                         | Berlin,Berlin                                   | ,     |                           |
-          | k    | l  | KL,KL                                                     | A1: Berlin,A1: Berlin                           | E1,E1 |                           |
-          | m    | n  | MN,MN                                                     | A1, A2: Berlin, Hamburg,A1, A2: Berlin, Hamburg | ,     |                           |
-          | o    | p  | OP,OP                                                     | ,                                               | ,     | guard against mis-tagging |
-          | q    | r  | QR,QR                                                     | A1, A2,A1, A2                                   | ,     |                           |
+          | from | to | route | destinations                                    | ref   | #                         |
+          | a    | b  | AB,AB | ,                                               | E1,E1 |                           |
+          | c    | d  | CD,CD | Berlin,Berlin                                   | ,     |                           |
+          | e    | f  | EF,EF | A1: Berlin,A1: Berlin                           | ,     |                           |
+          | g    | h  | ,     | A1: Berlin,A1: Berlin                           | ,     |                           |
+          | i    | j  | ,     | Berlin,Berlin                                   | ,     |                           |
+          | k    | l  | KL,KL | A1: Berlin,A1: Berlin                           | E1,E1 |                           |
+          | m    | n  | MN,MN | A1, A2: Berlin, Hamburg,A1, A2: Berlin, Hamburg | ,     |                           |
+          | o    | p  | OP,OP | ,                                               | ,     | guard against mis-tagging |
+          | q    | r  | QR,QR | A1, A2,A1, A2                                   | ,     |                           |
diff --git a/features/guidance/new-name.feature b/features/guidance/new-name.feature
@@ -381,3 +381,33 @@ Feature: New-Name Instructions
         When I route I should get
             | waypoints | route | turns         |
             | a,c       | ,     | depart,arrive |
+
+    Scenario: Spaces in refs for containment check, #3086
+        Given the node map
+            """
+            a       b       c
+            """
+
+        And the ways
+            | nodes | name     | ref                        | highway  |
+            | ab    | Keystone | US 64;US 412;OK 151 Detour | motorway |
+            | bc    | Keystone | US 64; US 412              | motorway |
+
+        When I route I should get
+            | waypoints | route             | turns         |
+            | a,c       | Keystone,Keystone | depart,arrive |
+
+    Scenario: More spaces in refs for containment check, #3086
+        Given the node map
+            """
+            a       b       c
+            """
+
+        And the ways
+            | nodes | name      | ref                              | highway  |
+            | ab    | Keystone  | US 64; US 412  ;   OK 151 Detour | motorway |
+            | bc    |  Keystone |  US 64  ;  US 412                | motorway |
+
+        When I route I should get
+            | waypoints | route             | turns         |
+            | a,c       | Keystone,Keystone | depart,arrive |
diff --git a/include/extractor/extraction_helper_functions.hpp b/include/extractor/extraction_helper_functions.hpp
@@ -4,6 +4,11 @@
 #include <boost/spirit/include/phoenix.hpp>
 #include <boost/spirit/include/qi.hpp>
 
+#include <boost/algorithm/string/replace.hpp>
+
+#include <algorithm>
+#include <cctype>
+#include <iterator>
 #include <limits>
 #include <string>
 
@@ -112,7 +117,24 @@ inline std::string applyAccessTokens(const std::string &lane_string,
 {
     return extractor::guidance::applyAccessTokens(lane_string, access_tokens);
 }
+
+// Takes a string representing a list separated by delim and canonicalizes containing spaces.
+// Example: "aaa;bbb; ccc;  d;dd" => "aaa; bbb; ccc; d; dd"
+inline std::string canonicalizeStringList(std::string strlist, const std::string &delim)
+{
+    // expand space after delimiter: ";X" => "; X"
+    boost::replace_all(strlist, delim, delim + " ");
+
+    // collapse spaces; this is needed in case we expand "; X" => ";  X" above
+    // but also makes sense to do irregardless of the fact - canonicalizing strings.
+    const auto spaces = [](auto lhs, auto rhs) { return ::isspace(lhs) && ::isspace(rhs); };
+    auto it = std::unique(begin(strlist), end(strlist), spaces);
+    strlist.erase(it, end(strlist));
+
+    return strlist;
 }
-}
+
+} // extractor
+} // osrm
 
 #endif // EXTRACTION_HELPER_FUNCTIONS_HPP
diff --git a/profiles/car.lua b/profiles/car.lua
@@ -446,7 +446,7 @@ function way_function (way, result)
   end
 
   if has_ref then
-    result.ref = ref
+    result.ref = canonicalizeStringList(ref, ";")
   end
 
   if has_pronunciation then
@@ -498,19 +498,23 @@ function way_function (way, result)
   -- Set direction according to tags on way
   if obey_oneway then
     if oneway == "-1" then
-      local is_forward = false
       result.forward_mode = mode.inaccessible
-      result.destinations = get_destination(way, is_forward)
+
+      local is_forward = false
+      local destination = get_destination(way, is_forward)
+      result.destinations = canonicalizeStringList(destination, ",")
     elseif oneway == "yes" or
     oneway == "1" or
     oneway == "true" or
     junction == "roundabout" or
     (highway == "motorway" and oneway ~= "no") then
-      local is_forward = true
+
       result.backward_mode = mode.inaccessible
-      result.destinations = get_destination(way, is_forward)
+
+      local is_forward = true
+      local destination = get_destination(way, is_forward)
+      result.destinations = canonicalizeStringList(destination, ",")
     end
-
   end
 
   -- Override speed settings if explicit forward/backward maxspeeds are given

diff --git a/src/extractor/scripting_environment_lua.cpp b/src/extractor/scripting_environment_lua.cpp
@@ -83,6 +83,7 @@ void LuaScriptingEnvironment::InitContext(LuaScriptingContext &context)
          luabind::def("parseDuration", parseDuration),
          luabind::def("trimLaneString", trimLaneString),
          luabind::def("applyAccessTokens", applyAccessTokens),
+         luabind::def("canonicalizeStringList", canonicalizeStringList),
          luabind::class_<TravelMode>("mode").enum_(
              "enums")[luabind::value("inaccessible", TRAVEL_MODE_INACCESSIBLE),
                       luabind::value("driving", TRAVEL_MODE_DRIVING),