Make initialization fail with a specific exception if the dataset isn't compatible with the algorithm being used, rather than crashing when a query occurs.

CHANGELOG.md

@@ -19,6 +19,7 @@
     - Bugfixes
       - Fixed a copy/paste issue assigning wrong directions in similar turns (left over right)
       - #4074: fixed a bug that would announce entering highway ramps as u-turns
+      - #4122: osrm-routed/libosrm should throw exception when a dataset incompatible with the requested algorithm is loaded
 
 # 5.7.1
     - Bugfixes

features/options/routed/invalid.feature

@@ -14,5 +14,5 @@ Feature: osrm-routed command line options: invalid options
     Scenario: osrm-routed - Missing file
         When I try to run "osrm-routed over-the-rainbow.osrm"
         Then stderr should contain "over-the-rainbow.osrm"
-        And stderr should contain "not found"
+        And stderr should contain "Required files are missing"
         And it should exit with an error

include/engine/engine.hpp

@@ -159,12 +159,12 @@ bool Engine<routing_algorithms::ch::Algorithm>::CheckCompability(const EngineCon
     }
     else
     {
-        std::ifstream in(config.storage_config.hsgr_data_path.string().c_str());
+        if (!boost::filesystem::exists(config.storage_config.hsgr_data_path))
-        if (!in)
             return false;
+        storage::io::FileReader in(config.storage_config.hsgr_data_path,
+                                   storage::io::FileReader::VerifyFingerprint);
 
-        in.seekg(0, std::ios::end);
+        auto size = in.GetSize();
-        auto size = in.tellg();
         return size > 0;
     }
 }
@@ -190,14 +190,38 @@ bool Engine<routing_algorithms::corech::Algorithm>::CheckCompability(const Engin
     }
     else
     {
-        std::ifstream in(config.storage_config.core_data_path.string().c_str());
+        if (!boost::filesystem::exists(config.storage_config.core_data_path))
-        if (!in)
             return false;
+        storage::io::FileReader in(config.storage_config.core_data_path,
+                                   storage::io::FileReader::VerifyFingerprint);
 
-        in.seekg(0, std::ios::end);
+        auto size = in.GetSize();
-        std::size_t size = in.tellg();
+        return size > sizeof(std::uint64_t);
-        // An empty core files is only the 8 byte size header plus the 8 byte Fingerprint.
+    }
-        return size > sizeof(std::uint64_t) + sizeof(util::FingerPrint);
+}
+
+template <>
+bool Engine<routing_algorithms::mld::Algorithm>::CheckCompability(const EngineConfig &config)
+{
+    if (config.use_shared_memory)
+    {
+        storage::SharedMonitor<storage::SharedDataTimestamp> barrier;
+        using mutex_type = typename decltype(barrier)::mutex_type;
+        boost::interprocess::scoped_lock<mutex_type> current_region_lock(barrier.get_mutex());
+
+        auto mem = storage::makeSharedMemory(barrier.data().region);
+        auto layout = reinterpret_cast<storage::DataLayout *>(mem->Ptr());
+        return layout->GetBlockSize(storage::DataLayout::MLD_PARTITION) > 0;
+    }
+    else
+    {
+        if (!boost::filesystem::exists(config.storage_config.mld_partition_path))
+            return false;
+        storage::io::FileReader in(config.storage_config.mld_partition_path,
+                                   storage::io::FileReader::VerifyFingerprint);
+
+        auto size = in.GetSize();
+        return size > 0;
     }
 }
 }

src/osrm/osrm.cpp

@@ -22,10 +22,33 @@ OSRM::OSRM(engine::EngineConfig &config)
     using CoreCH = engine::routing_algorithms::corech::Algorithm;
     using MLD = engine::routing_algorithms::mld::Algorithm;
 
+    // First, check that necessary core data is available
+    if (!config.use_shared_memory && !config.storage_config.IsValid())
+    {
+        throw util::exception("Required files are missing, cannot continue.  Have all the "
+                              "pre-processing steps been run?");
+    }
+    else if (config.use_shared_memory)
+    {
+        storage::SharedMonitor<storage::SharedDataTimestamp> barrier;
+        using mutex_type = typename decltype(barrier)::mutex_type;
+        boost::interprocess::scoped_lock<mutex_type> current_region_lock(barrier.get_mutex());
+
+        auto mem = storage::makeSharedMemory(barrier.data().region);
+        auto layout = reinterpret_cast<storage::DataLayout *>(mem->Ptr());
+        if (layout->GetBlockSize(storage::DataLayout::NAME_CHAR_DATA) == 0)
+            throw util::exception(
+                "No name data loaded, cannot continue.  Have you run osrm-datastore to load data?");
+    }
+
+    // Now, check that the algorithm requested can be used with the data
+    // that's available.
+
     if (config.algorithm == EngineConfig::Algorithm::CoreCH ||
         config.algorithm == EngineConfig::Algorithm::CH)
     {
         bool corech_compatible = engine::Engine<CoreCH>::CheckCompability(config);
+        bool ch_compatible = engine::Engine<CH>::CheckCompability(config);
 
         // Activate CoreCH if we can because it is faster
         if (config.algorithm == EngineConfig::Algorithm::CH && corech_compatible)
@@ -33,13 +56,26 @@ OSRM::OSRM(engine::EngineConfig &config)
             config.algorithm = EngineConfig::Algorithm::CoreCH;
         }
 
-        // throw error if dataset is not usable with CoreCH
+        // throw error if dataset is not usable with CoreCH or CH
         if (config.algorithm == EngineConfig::Algorithm::CoreCH && !corech_compatible)
         {
             throw util::RuntimeError("Dataset is not compatible with CoreCH.",
                                      ErrorCode::IncompatibleDataset,
                                      SOURCE_REF);
         }
+        else if (config.algorithm == EngineConfig::Algorithm::CH && !ch_compatible)
+        {
+            throw util::exception("Dataset is not compatible with CH");
+        }
+    }
+    else if (config.algorithm == EngineConfig::Algorithm::MLD)
+    {
+        bool mld_compatible = engine::Engine<MLD>::CheckCompability(config);
+        // throw error if dataset is not usable with MLD
+        if (!mld_compatible)
+        {
+            throw util::exception("Dataset is not compatible with MLD.");
+        }
     }
 
     switch (config.algorithm)

src/storage/storage_config.cpp

@@ -14,9 +14,9 @@ bool CheckFileList(const std::vector<boost::filesystem::path> &files)
     bool success = true;
     for (auto &path : files)
     {
-        if (!boost::filesystem::is_regular_file(path))
+        if (!boost::filesystem::exists(path))
         {
-            util::Log(logWARNING) << "Missing/Broken File: " << path.string();
+            util::Log(logERROR) << "Missing File: " << path.string();
             success = false;
         }
     }
@@ -62,14 +62,6 @@ bool StorageConfig::IsValid() const
         return false;
     }
 
-    // TODO: add algorithm checks
-
-    // CH files
-    CheckFileList({hsgr_data_path, core_data_path});
-
-    // MLD files
-    CheckFileList({mld_partition_path, mld_storage_path, mld_graph_path});
-
     return true;
 }
 }

src/tools/routed.cpp

@@ -223,33 +223,17 @@ int main(int argc, const char *argv[]) try
     {
         config.storage_config = storage::StorageConfig(base_path);
     }
+    if (!config.use_shared_memory && !config.storage_config.IsValid())
+    {
+        util::Log(logERROR) << "Required files are missing, cannot continue";
+        return EXIT_FAILURE;
+    }
     if (!config.IsValid())
     {
         if (base_path.empty() != config.use_shared_memory)
         {
             util::Log(logWARNING) << "Path settings and shared memory conflicts.";
         }
-        else
-        {
-            auto required_files = {config.storage_config.ram_index_path,
-                                   config.storage_config.file_index_path,
-                                   config.storage_config.hsgr_data_path,
-                                   config.storage_config.node_based_nodes_data_path,
-                                   config.storage_config.edge_based_nodes_data_path,
-                                   config.storage_config.edges_data_path,
-                                   config.storage_config.core_data_path,
-                                   config.storage_config.geometries_path,
-                                   config.storage_config.datasource_indexes_path,
-                                   config.storage_config.names_data_path,
-                                   config.storage_config.properties_path};
-            for (auto file : required_files)
-            {
-                if (!boost::filesystem::is_regular_file(file))
-                {
-                    util::Log(logWARNING) << file << " is not found";
-                }
-            }
-        }
         return EXIT_FAILURE;
     }
     config.algorithm = stringToAlgorithm(algorithm);

test/nodejs/index.js

@@ -23,9 +23,12 @@ test('constructor: does not accept more than one parameter', function(assert) {
 });
 
 test('constructor: throws if necessary files do not exist', function(assert) {
-    assert.plan(1);
+    assert.plan(2);
-    assert.throws(function() { new OSRM("missing.osrm"); },
+    assert.throws(function() { new OSRM('missing.osrm'); },
-        /Problem opening file: missing.osrm.names/);
+        /Required files are missing, cannot continue/);
+
+    assert.throws(function() { new OSRM({path: 'missing.osrm', algorithm: 'MLD'}); },
+        /Required files are missing, cannot continue/);
 });
 
 test('constructor: takes a shared memory argument', function(assert) {
@@ -82,6 +85,19 @@ test('constructor: loads CoreCH if given as algorithm', function(assert) {
     assert.ok(osrm);
 });
 
+test('constructor: autoswitches to CoreCH for a CH dataset if capable', function(assert) {
+    assert.plan(1);
+    var osrm = new OSRM({algorithm: 'CH', path: monaco_corech_path});
+    assert.ok(osrm);
+});
+
+test('constructor: throws if data doesn\'t match algorithm', function(assert) {
+    assert.plan(3);
+    assert.throws(function() { new OSRM({algorithm: 'CoreCH', path: monaco_mld_path}); });
+    assert.throws(function() { new OSRM({algorithm: 'CoreCH', path: monaco_path}); });
+    assert.throws(function() { new OSRM({algorithm: 'MLD', path: monaco_path}); });
+});
+
 require('./route.js');
 require('./trip.js');
 require('./match.js');

unit_tests/library/algorithm.cpp

@@ -0,0 +1,34 @@
+#include <boost/test/test_case_template.hpp>
+#include <boost/test/unit_test.hpp>
+
+#include "fixture.hpp"
+
+#include "osrm/exception.hpp"
+
+BOOST_AUTO_TEST_SUITE(table)
+
+BOOST_AUTO_TEST_CASE(test_incompatible_with_mld)
+{
+    // Can't use the MLD algorithm with CH data
+    BOOST_CHECK_THROW(
+        getOSRM(OSRM_TEST_DATA_DIR "/ch/monaco.osrm", osrm::EngineConfig::Algorithm::MLD),
+        osrm::exception);
+}
+
+BOOST_AUTO_TEST_CASE(test_incompatible_with_corech)
+{
+    // Note - CH-only data can't be used with the CoreCH algorithm
+    BOOST_CHECK_THROW(
+        getOSRM(OSRM_TEST_DATA_DIR "/ch/monaco.osrm", osrm::EngineConfig::Algorithm::CoreCH),
+        osrm::exception);
+}
+
+BOOST_AUTO_TEST_CASE(test_incompatible_with_ch)
+{
+    // Can't use the CH algorithm with MLD data
+    BOOST_CHECK_THROW(
+        getOSRM(OSRM_TEST_DATA_DIR "/mld/monaco.osrm", osrm::EngineConfig::Algorithm::CH),
+        osrm::exception);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

unit_tests/library/fixture.hpp

@@ -9,11 +9,14 @@
 // I couldn't get Boost.UnitTest to provide a test suite level fixture with custom
 // arguments per test suite (osrm base path from argv), so this has to suffice.
 
-inline osrm::OSRM getOSRM(const std::string &base_path)
+inline osrm::OSRM
+getOSRM(const std::string &base_path,
+        osrm::EngineConfig::Algorithm algorithm = osrm::EngineConfig::Algorithm::CH)
 {
     osrm::EngineConfig config;
     config.storage_config = {base_path};
     config.use_shared_memory = false;
+    config.algorithm = algorithm;
 
     return osrm::OSRM{config};
 }

unit_tests/library/route.cpp

@@ -7,6 +7,7 @@
 
 #include "osrm/coordinate.hpp"
 #include "osrm/engine_config.hpp"
+#include "osrm/exception.hpp"
 #include "osrm/json_container.hpp"
 #include "osrm/json_container.hpp"
 #include "osrm/osrm.hpp"