diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d2699618..c9a1d3ccd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/features/options/routed/invalid.feature b/features/options/routed/invalid.feature index 78d28a064..3c82e9437 100644 --- a/features/options/routed/invalid.feature +++ b/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 diff --git a/include/engine/engine.hpp b/include/engine/engine.hpp index 7f32d3e72..29d2d8415 100644 --- a/include/engine/engine.hpp +++ b/include/engine/engine.hpp @@ -159,12 +159,12 @@ bool Engine::CheckCompability(const EngineCon } else { - std::ifstream in(config.storage_config.hsgr_data_path.string().c_str()); - if (!in) + if (!boost::filesystem::exists(config.storage_config.hsgr_data_path)) 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.tellg(); + auto size = in.GetSize(); return size > 0; } } @@ -190,14 +190,38 @@ bool Engine::CheckCompability(const Engin } else { - std::ifstream in(config.storage_config.core_data_path.string().c_str()); - if (!in) + if (!boost::filesystem::exists(config.storage_config.core_data_path)) return false; + storage::io::FileReader in(config.storage_config.core_data_path, + storage::io::FileReader::VerifyFingerprint); - in.seekg(0, std::ios::end); - std::size_t size = in.tellg(); - // 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); + auto size = in.GetSize(); + return size > sizeof(std::uint64_t); + } +} + +template <> +bool Engine::CheckCompability(const EngineConfig &config) +{ + if (config.use_shared_memory) + { + storage::SharedMonitor barrier; + using mutex_type = typename decltype(barrier)::mutex_type; + boost::interprocess::scoped_lock current_region_lock(barrier.get_mutex()); + + auto mem = storage::makeSharedMemory(barrier.data().region); + auto layout = reinterpret_cast(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; } } } diff --git a/src/osrm/osrm.cpp b/src/osrm/osrm.cpp index ba401136d..326e4f321 100644 --- a/src/osrm/osrm.cpp +++ b/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 barrier; + using mutex_type = typename decltype(barrier)::mutex_type; + boost::interprocess::scoped_lock current_region_lock(barrier.get_mutex()); + + auto mem = storage::makeSharedMemory(barrier.data().region); + auto layout = reinterpret_cast(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::CheckCompability(config); + bool ch_compatible = engine::Engine::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::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) diff --git a/src/storage/storage_config.cpp b/src/storage/storage_config.cpp index 968537b07..a3051ca0a 100644 --- a/src/storage/storage_config.cpp +++ b/src/storage/storage_config.cpp @@ -14,9 +14,9 @@ bool CheckFileList(const std::vector &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; } } diff --git a/src/tools/routed.cpp b/src/tools/routed.cpp index 5fea572eb..956a10b56 100644 --- a/src/tools/routed.cpp +++ b/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); diff --git a/test/nodejs/index.js b/test/nodejs/index.js index c21b16b3a..c54f597df 100644 --- a/test/nodejs/index.js +++ b/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.throws(function() { new OSRM("missing.osrm"); }, - /Problem opening file: missing.osrm.names/); + assert.plan(2); + assert.throws(function() { new OSRM('missing.osrm'); }, + /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'); diff --git a/unit_tests/library/algorithm.cpp b/unit_tests/library/algorithm.cpp new file mode 100644 index 000000000..7ad083b3b --- /dev/null +++ b/unit_tests/library/algorithm.cpp @@ -0,0 +1,34 @@ +#include +#include + +#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() diff --git a/unit_tests/library/fixture.hpp b/unit_tests/library/fixture.hpp index 568593dc1..632c72f15 100644 --- a/unit_tests/library/fixture.hpp +++ b/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}; } diff --git a/unit_tests/library/route.cpp b/unit_tests/library/route.cpp index 0a95d1927..e3f8d97ea 100644 --- a/unit_tests/library/route.cpp +++ b/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"