From 603b83e1c3fdf55e31721c092f49f9745cf49b76 Mon Sep 17 00:00:00 2001
From: Moritz Kobitzsch <moritz@mapbox.com>
Date: Thu, 28 Jan 2016 15:26:56 +0100
Subject: [PATCH] Fixes Alternative Routing when including loops

---
 features/testbot/alternative_loop.feature     | 29 +++++++++++++++++++
 .../routing_algorithms/alternative_path.hpp   | 19 ++++++++----
 2 files changed, 42 insertions(+), 6 deletions(-)
 create mode 100644 features/testbot/alternative_loop.feature

diff --git a/features/testbot/alternative_loop.feature b/features/testbot/alternative_loop.feature
new file mode 100644
index 000000000..6af2bfe0e
--- /dev/null
+++ b/features/testbot/alternative_loop.feature
@@ -0,0 +1,29 @@
+@routing @testbot @alternative
+Feature: Alternative route
+
+    Background:
+        Given the profile "testbot"
+
+    Scenario: Alternative Loop Paths
+        Given the node map
+            | a | 2 | 1 | b |
+            | 7 |   |   | 4 |
+            | 8 |   |   | 3 |
+            | c | 5 | 6 | d |
+
+        And the ways
+            | nodes | oneway |
+            | ab    | yes    |
+            | bd    | yes    |
+            | dc    | yes    |
+            | ca    | yes    |
+
+        And the query options
+            | alt | true |
+
+        When I route I should get
+            | from | to | route          | alternative |
+            | 1    | 2  | ab,bd,dc,ca,ab |             |
+            | 3    | 4  | bd,dc,ca,ab,bd |             |
+            | 5    | 6  | dc,ca,ab,bd,dc |             |
+            | 7    | 8  | ca,ab,bd,dc,ca |             |
diff --git a/include/engine/routing_algorithms/alternative_path.hpp b/include/engine/routing_algorithms/alternative_path.hpp
index 7d7f64832..024b03611 100644
--- a/include/engine/routing_algorithms/alternative_path.hpp
+++ b/include/engine/routing_algorithms/alternative_path.hpp
@@ -156,8 +156,10 @@ class AlternativeRouting final
         std::vector<NodeID> packed_forward_path;
         std::vector<NodeID> packed_reverse_path;
 
-        if (upper_bound_to_shortest_path_distance !=
-            forward_heap1.GetKey(middle_node) + reverse_heap1.GetKey(middle_node))
+        const bool path_is_a_loop =
+            upper_bound_to_shortest_path_distance !=
+            forward_heap1.GetKey(middle_node) + reverse_heap1.GetKey(middle_node);
+        if (path_is_a_loop)
         {
             // Self Loop
             BOOST_ASSERT(forward_heap1.GetData(middle_node).parent == middle_node &&
@@ -239,6 +241,8 @@ class AlternativeRouting final
         std::vector<NodeID> preselected_node_list;
         for (const NodeID node : via_node_candidate_list)
         {
+            if (node == middle_node)
+                continue;
             const auto fwd_iterator = approximated_forward_sharing.find(node);
             const int fwd_sharing =
                 (fwd_iterator != approximated_forward_sharing.end()) ? fwd_iterator->second : 0;
@@ -265,10 +269,13 @@ class AlternativeRouting final
         }
 
         std::vector<NodeID> &packed_shortest_path = packed_forward_path;
-        std::reverse(packed_shortest_path.begin(), packed_shortest_path.end());
-        packed_shortest_path.emplace_back(middle_node);
-        packed_shortest_path.insert(packed_shortest_path.end(), packed_reverse_path.begin(),
-                                    packed_reverse_path.end());
+        if (!path_is_a_loop)
+        {
+            std::reverse(packed_shortest_path.begin(), packed_shortest_path.end());
+            packed_shortest_path.emplace_back(middle_node);
+            packed_shortest_path.insert(packed_shortest_path.end(), packed_reverse_path.begin(),
+                                        packed_reverse_path.end());
+        }
         std::vector<RankedCandidateNode> ranked_candidates_list;
 
         // prioritizing via nodes for deep inspection