Add error handling for avoid not implemented

include/engine/algorithm.hpp

@@ -56,6 +56,9 @@ template <typename AlgorithmT> struct HasManyToManySearch final : std::false_typ
 template <typename AlgorithmT> struct HasGetTileTurns final : std::false_type
 {
 };
+template <typename AlgorithmT> struct HasAvoidFlags final : std::false_type
+{
+};
 
 // Algorithms supported by Contraction Hierarchies
 template <> struct HasAlternativePathSearch<ch::Algorithm> final : std::true_type
@@ -111,6 +114,9 @@ template <> struct HasManyToManySearch<mld::Algorithm> final : std::true_type
 template <> struct HasGetTileTurns<mld::Algorithm> final : std::true_type
 {
 };
+template <> struct HasAvoidFlags<mld::Algorithm> final : std::true_type
+{
+};
 }
 }
 }

include/engine/data_watchdog.hpp

@@ -25,10 +25,12 @@ namespace engine
 // This class monitors the shared memory region that contains the pointers to
 // the data and layout regions that should be used. This region is updated
 // once a new dataset arrives.
-template <typename AlgorithmT> class DataWatchdog final
+template <template<typename A> class FacadeT, typename AlgorithmT> class DataWatchdog;
+
+template <typename AlgorithmT> class DataWatchdog<datafacade::ContiguousInternalMemoryDataFacade,AlgorithmT> final
 {
     using mutex_type = typename storage::SharedMonitor<storage::SharedDataTimestamp>::mutex_type;
-    using FacadeT = datafacade::ContiguousInternalMemoryDataFacade<AlgorithmT>;
+    using Facade = datafacade::ContiguousInternalMemoryDataFacade<AlgorithmT>;
 
   public:
     DataWatchdog() : active(true), timestamp(0)
@@ -37,7 +39,7 @@ template <typename AlgorithmT> class DataWatchdog final
         {
             boost::interprocess::scoped_lock<mutex_type> current_region_lock(barrier.get_mutex());
 
-            facade_factory = DataFacadeFactory<const FacadeT>(
+            facade_factory = DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>(
                 std::make_shared<datafacade::SharedMemoryAllocator>(barrier.data().region));
             timestamp = barrier.data().timestamp;
         }
@@ -52,8 +54,8 @@ template <typename AlgorithmT> class DataWatchdog final
         watcher.join();
     }
 
-    std::shared_ptr<const FacadeT> Get(const api::BaseParameters &params) const { return facade_factory.Get(params); }
+    std::shared_ptr<const Facade> Get(const api::BaseParameters &params) const { return facade_factory.Get(params); }
-    std::shared_ptr<const FacadeT> Get(const api::TileParameters &params) const { return facade_factory.Get(params); }
+    std::shared_ptr<const Facade> Get(const api::TileParameters &params) const { return facade_factory.Get(params); }
 
   private:
     void Run()
@@ -70,7 +72,7 @@ template <typename AlgorithmT> class DataWatchdog final
             if (timestamp != barrier.data().timestamp)
             {
                 auto region = barrier.data().region;
-                facade_factory = DataFacadeFactory<const FacadeT>(
+                facade_factory = DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>(
                     std::make_shared<datafacade::SharedMemoryAllocator>(region));
                 timestamp = barrier.data().timestamp;
                 util::Log() << "updated facade to region " << region << " with timestamp "
@@ -85,7 +87,7 @@ template <typename AlgorithmT> class DataWatchdog final
     std::thread watcher;
     bool active;
     unsigned timestamp;
-    DataFacadeFactory<const FacadeT> facade_factory;
+    DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT> facade_factory;
 };
 }
 }

include/engine/datafacade_factory.hpp

@@ -4,6 +4,7 @@
 #include "extractor/class_data.hpp"
 #include "extractor/profile_properties.hpp"
 
+#include "engine/algorithm.hpp"
 #include "engine/api/base_parameters.hpp"
 #include "engine/api/tile_parameters.hpp"
 
@@ -20,16 +21,33 @@ namespace osrm
 namespace engine
 {
 // This class selects the right facade for
-template <typename FacadeT> class DataFacadeFactory
+template <template <typename A> class FacadeT, typename AlgorithmT> class DataFacadeFactory
 {
+    static constexpr auto has_avoid_flags = routing_algorithms::HasAvoidFlags<AlgorithmT>{};
+
   public:
+    using Facade = FacadeT<AlgorithmT>;
     DataFacadeFactory() = default;
 
-    template <typename AllocatorT> DataFacadeFactory(std::shared_ptr<AllocatorT> allocator)
+    template <typename AllocatorT>
+    DataFacadeFactory(std::shared_ptr<AllocatorT> allocator)
+        : DataFacadeFactory(allocator, has_avoid_flags)
+    {
+    }
+
+    template <typename ParameterT> std::shared_ptr<const Facade> Get(const ParameterT &params) const
+    {
+        return Get(params, has_avoid_flags);
+    }
+
+  private:
+    // Algorithm with avoid flags
+    template <typename AllocatorT>
+    DataFacadeFactory(std::shared_ptr<AllocatorT> allocator, std::true_type)
     {
         for (const auto index : util::irange<std::size_t>(0, facades.size()))
         {
-            facades[index] = std::make_shared<FacadeT>(allocator, index);
+            facades[index] = std::make_shared<const Facade>(allocator, index);
         }
 
         properties = allocator->GetLayout().template GetBlockPtr<extractor::ProfileProperties>(
@@ -45,12 +63,37 @@ template <typename FacadeT> class DataFacadeFactory
         }
     }
 
-    std::shared_ptr<FacadeT> Get(const api::TileParameters &) const
+    // Algorithm without avoid flags
+    template <typename AllocatorT>
+    DataFacadeFactory(std::shared_ptr<AllocatorT> allocator, std::false_type)
+    {
+        facades[0] = std::make_shared<const Facade>(allocator, 0);
+    }
+
+    std::shared_ptr<const Facade> Get(const api::TileParameters &, std::false_type) const
     {
         return facades[0];
     }
 
-    std::shared_ptr<FacadeT> Get(const api::BaseParameters &params) const
+    // Default for non-avoid flags: return only facade
+    std::shared_ptr<const Facade> Get(const api::BaseParameters &params, std::false_type) const
+    {
+        if (!params.avoid.empty())
+        {
+            return {};
+        }
+
+        return facades[0];
+    }
+
+    // TileParameters don't drive from BaseParameters and generally don't have use for avoid flags
+    std::shared_ptr<const Facade> Get(const api::TileParameters &, std::true_type) const
+    {
+        return facades[0];
+    }
+
+    // Selection logic for finding the corresponding datafacade for the given parameters
+    std::shared_ptr<const Facade> Get(const api::BaseParameters &params, std::true_type) const
     {
         if (params.avoid.empty())
             return facades[0];
@@ -73,12 +116,10 @@ template <typename FacadeT> class DataFacadeFactory
             return facades[avoid_index];
         }
 
-        // FIXME We need proper error handling here
         return {};
     }
 
-  private:
+    std::array<std::shared_ptr<const Facade>, extractor::MAX_AVOIDABLE_CLASSES> facades;
-    std::array<std::shared_ptr<FacadeT>, extractor::MAX_AVOIDABLE_CLASSES> facades;
     std::unordered_map<std::string, extractor::ClassData> name_to_class;
     const extractor::ProfileProperties *properties = nullptr;
 };

include/engine/datafacade_provider.hpp

@@ -39,13 +39,13 @@ class ImmutableProvider final : public DataFacadeProvider<AlgorithmT, FacadeT>
     std::shared_ptr<const Facade> Get(const api::BaseParameters &params) const override final { return facade_factory.Get(params); }
 
   private:
-    DataFacadeFactory<const Facade> facade_factory;
+    DataFacadeFactory<FacadeT, AlgorithmT> facade_factory;
 };
 
 template <typename AlgorithmT, template <typename A> class FacadeT>
 class WatchingProvider : public DataFacadeProvider<AlgorithmT, FacadeT>
 {
-    DataWatchdog<AlgorithmT> watchdog;
+    DataWatchdog<FacadeT, AlgorithmT> watchdog;
 
   public:
     using Facade = typename DataFacadeProvider<AlgorithmT, FacadeT>::Facade;

include/engine/engine.hpp

@@ -85,46 +85,45 @@ template <typename Algorithm> class Engine final : public EngineInterface
     Status Route(const api::RouteParameters &params,
                  util::json::Object &result) const override final
     {
-        auto algorithms = RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+        return route_plugin.HandleRequest(GetAlgorithms(params), params, result);
-        return route_plugin.HandleRequest(algorithms, params, result);
     }
 
     Status Table(const api::TableParameters &params,
                  util::json::Object &result) const override final
     {
-        auto algorithms = RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+        return table_plugin.HandleRequest(GetAlgorithms(params), params, result);
-        return table_plugin.HandleRequest(algorithms, params, result);
     }
 
     Status Nearest(const api::NearestParameters &params,
                    util::json::Object &result) const override final
     {
-        auto algorithms = RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+        return nearest_plugin.HandleRequest(GetAlgorithms(params), params, result);
-        return nearest_plugin.HandleRequest(algorithms, params, result);
     }
 
     Status Trip(const api::TripParameters &params, util::json::Object &result) const override final
     {
-        auto algorithms = RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+        return trip_plugin.HandleRequest(GetAlgorithms(params), params, result);
-        return trip_plugin.HandleRequest(algorithms, params, result);
     }
 
     Status Match(const api::MatchParameters &params,
                  util::json::Object &result) const override final
     {
-        auto algorithms = RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+        return match_plugin.HandleRequest(GetAlgorithms(params), params, result);
-        return match_plugin.HandleRequest(algorithms, params, result);
     }
 
     Status Tile(const api::TileParameters &params, std::string &result) const override final
     {
-        auto algorithms = RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+        return tile_plugin.HandleRequest(GetAlgorithms(params), params, result);
-        return tile_plugin.HandleRequest(algorithms, params, result);
     }
 
     static bool CheckCompability(const EngineConfig &config);
 
   private:
+    template<typename ParametersT>
+    auto GetAlgorithms(const ParametersT &params) const
+    {
+        return RoutingAlgorithms<Algorithm>{heaps, facade_provider->Get(params)};
+    }
     std::unique_ptr<DataFacadeProvider<Algorithm>> facade_provider;
     mutable SearchEngineData<Algorithm> heaps;
 

include/engine/plugins/plugin_base.hpp

@@ -4,6 +4,7 @@
 #include "engine/api/base_parameters.hpp"
 #include "engine/datafacade/datafacade_base.hpp"
 #include "engine/phantom_node.hpp"
+#include "engine/routing_algorithms.hpp"
 #include "engine/status.hpp"
 
 #include "util/coordinate.hpp"
@@ -36,6 +37,28 @@ class BasePlugin
             });
     }
 
+    bool CheckAlgorithms(const api::BaseParameters &params, const RoutingAlgorithmsInterface& algorithms, util::json::Object &result) const
+    {
+        if (algorithms.IsValid())
+        {
+            return true;
+        }
+
+        if (!algorithms.HasAvoidFlags() && !params.avoid.empty())
+        {
+            Error("NotImplemented", "This algorithm does not support avoid flags.", result);
+            return false;
+        }
+        if (algorithms.HasAvoidFlags() && !params.avoid.empty())
+        {
+            Error("InvalidValue", "Avoid flag combination is not supported.", result);
+            return false;
+        }
+
+        BOOST_ASSERT_MSG(false, "There are only two reasons why the algorithm interface can be invalid.");
+        return false;
+    }
+
     Status Error(const std::string &code,
                  const std::string &message,
                  util::json::Object &json_result) const

include/engine/routing_algorithms.hpp

@@ -54,6 +54,8 @@ class RoutingAlgorithmsInterface
     virtual bool HasMapMatching() const = 0;
     virtual bool HasManyToManySearch() const = 0;
     virtual bool HasGetTileTurns() const = 0;
+    virtual bool HasAvoidFlags() const = 0;
+    virtual bool IsValid() const = 0;
 };
 
 // Short-lived object passed to each plugin in request to wrap routing algorithms
@@ -127,6 +129,16 @@ template <typename Algorithm> class RoutingAlgorithms final : public RoutingAlgo
         return routing_algorithms::HasGetTileTurns<Algorithm>::value;
     }
 
+    bool HasAvoidFlags() const final override
+    {
+        return routing_algorithms::HasAvoidFlags<Algorithm>::value;
+    }
+
+    bool IsValid() const final override
+    {
+        return static_cast<bool>(facade);
+    }
+
   private:
     SearchEngineData<Algorithm> &heaps;
     std::shared_ptr<const DataFacade<Algorithm>> facade;

src/engine/plugins/match.cpp

@@ -120,6 +120,9 @@ Status MatchPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
                      json_result);
     }
 
+    if (!CheckAlgorithms(parameters, algorithms, json_result))
+        return Status::Error;
+
     const auto &facade = algorithms.GetFacade();
 
     BOOST_ASSERT(parameters.IsValid());

src/engine/plugins/nearest.cpp

@@ -25,6 +25,9 @@ Status NearestPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms
 {
     BOOST_ASSERT(params.IsValid());
 
+    if (!CheckAlgorithms(params, algorithms, json_result))
+        return Status::Error;
+
     const auto &facade = algorithms.GetFacade();
 
     if (max_results > 0 &&

src/engine/plugins/table.cpp

@@ -66,6 +66,9 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
         return Error("TooBig", "Too many table coordinates", result);
     }
 
+    if (!CheckAlgorithms(params, algorithms, result))
+        return Status::Error;
+
     const auto &facade = algorithms.GetFacade();
     auto phantom_nodes = GetPhantomNodes(facade, params);
 

src/engine/plugins/trip.cpp

@@ -191,6 +191,9 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
         return Error("InvalidValue", "Invalid coordinate value.", json_result);
     }
 
+    if (!CheckAlgorithms(parameters, algorithms, json_result))
+        return Status::Error;
+
     const auto &facade = algorithms.GetFacade();
     auto phantom_node_pairs = GetPhantomNodes(facade, parameters);
     if (phantom_node_pairs.size() != number_of_locations)

src/engine/plugins/viaroute.cpp

@@ -73,6 +73,9 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm
         return Error("InvalidValue", "Invalid coordinate value.", json_result);
     }
 
+    if (!CheckAlgorithms(route_parameters, algorithms, json_result))
+        return Status::Error;
+
     const auto &facade = algorithms.GetFacade();
     auto phantom_node_pairs = GetPhantomNodes(facade, route_parameters);
     if (phantom_node_pairs.size() != route_parameters.coordinates.size())