From b15288e0ea525948eef8fa7d3dc7a18bda84b340 Mon Sep 17 00:00:00 2001 From: Michael Krasnyk Date: Wed, 30 Aug 2017 22:11:26 +0200 Subject: [PATCH] Add location_dependent_data unit tests --- include/extractor/location_dependent_data.hpp | 10 +- src/extractor/location_dependent_data.cpp | 53 ++++---- src/extractor/scripting_environment_lua.cpp | 36 +++++- .../location_dependent_data_tests.cpp | 122 ++++++++++++++++++ unit_tests/util/serialization.cpp | 2 - 5 files changed, 186 insertions(+), 37 deletions(-) create mode 100644 unit_tests/extractor/location_dependent_data_tests.cpp diff --git a/include/extractor/location_dependent_data.hpp b/include/extractor/location_dependent_data.hpp index a92562acf..b1638b093 100644 --- a/include/extractor/location_dependent_data.hpp +++ b/include/extractor/location_dependent_data.hpp @@ -7,8 +7,6 @@ #include -#include - #include #include @@ -30,9 +28,15 @@ struct LocationDependentData using property_t = boost::variant; using properties_t = std::unordered_map; + LocationDependentData(const boost::filesystem::path &path); + LocationDependentData(const std::vector &file_paths); - sol::table operator()(sol::state &state, const osmium::Way &way) const; + bool empty() const { return rtree.empty(); } + + properties_t operator()(const point_t &point) const; + + properties_t operator()(const osmium::Way &way) const; private: void loadLocationDependentData(const boost::filesystem::path &file_path); diff --git a/src/extractor/location_dependent_data.cpp b/src/extractor/location_dependent_data.cpp index 664ea8495..c4355d8e4 100644 --- a/src/extractor/location_dependent_data.cpp +++ b/src/extractor/location_dependent_data.cpp @@ -18,6 +18,11 @@ namespace osrm namespace extractor { +LocationDependentData::LocationDependentData(const boost::filesystem::path &path) +{ + loadLocationDependentData(path); +} + LocationDependentData::LocationDependentData(const std::vector &file_paths) { for (const auto &path : file_paths) @@ -139,37 +144,13 @@ void LocationDependentData::loadLocationDependentData(const boost::filesystem::p << polygons.size() << " GeoJSON polygons"; } -namespace +LocationDependentData::properties_t LocationDependentData::operator()(const point_t &point) const { -struct table_setter : public boost::static_visitor<> -{ - table_setter(sol::table &table, const std::string &key) : table(table), key(key) {} - template void operator()(const T &value) const { table.set(key, value); } - void operator()(const boost::blank &) const { /* ignore */} + properties_t result; - sol::table &table; - const std::string &key; -}; -} - -sol::table LocationDependentData::operator()(sol::state &state, const osmium::Way &way) const -{ - if (rtree.empty()) - return sol::make_object(state, sol::nil); - - // HEURISTIC: use a single node (last) of the way to localize the way - // For more complicated scenarios a proper merging of multiple tags - // at one or many locations must be provided - const auto &nodes = way.nodes(); - const auto &location = nodes.back().location(); - const point_t point(location.lon(), location.lat()); - - auto table = sol::table(state, sol::create); - auto merger = [this, &table](const rtree_t::value_type &rtree_entry) { - for (const auto &key_value : properties[polygons[rtree_entry.second].second]) - { - boost::apply_visitor(table_setter(table, key_value.first), key_value.second); - } + auto merger = [this, &result](const rtree_t::value_type &rtree_entry) { + const auto &polygon_properties = properties[polygons[rtree_entry.second].second]; + result.insert(polygon_properties.begin(), polygon_properties.end()); }; // Search the R-tree and collect a Lua table of tags that correspond to the location @@ -179,7 +160,19 @@ sol::table LocationDependentData::operator()(sol::state &state, const osmium::Wa }), boost::make_function_output_iterator(std::ref(merger))); - return table; + return result; +} + +LocationDependentData::properties_t LocationDependentData::operator()(const osmium::Way &way) const +{ + // HEURISTIC: use a single node (last) of the way to localize the way + // For more complicated scenarios a proper merging of multiple tags + // at one or many locations must be provided + const auto &nodes = way.nodes(); + const auto &location = nodes.back().location(); + const point_t point(location.lon(), location.lat()); + + return operator()(point); } } } diff --git a/src/extractor/scripting_environment_lua.cpp b/src/extractor/scripting_environment_lua.cpp index f63b22b22..ce38b9e9d 100644 --- a/src/extractor/scripting_environment_lua.cpp +++ b/src/extractor/scripting_environment_lua.cpp @@ -978,6 +978,31 @@ void LuaScriptingContext::ProcessNode(const osmium::Node &node, } } + +namespace +{ +// boost::variant visitor that inserts a key-value pair in a Lua table +struct table_setter : public boost::static_visitor<> +{ + table_setter(sol::table &table, const std::string &key) : table(table), key(key) {} + template void operator()(const T &value) const { table.set(key, value); } + void operator()(const boost::blank &) const { /* ignore */} + + sol::table &table; + const std::string &key; +}; + +// Converts a properties map into a Lua table +sol::table toLua(sol::state &state, const LocationDependentData::properties_t properties) +{ + auto table = sol::table(state, sol::create); + std::for_each(properties.begin(), properties.end(), [&table](const auto &property) { + boost::apply_visitor(table_setter(table, property.first), property.second); + }); + return table; +} +} + void LuaScriptingContext::ProcessWay(const osmium::Way &way, ExtractionWay &result, const ExtractionRelationContainer::RelationList &relations) @@ -987,10 +1012,17 @@ void LuaScriptingContext::ProcessWay(const osmium::Way &way, switch (api_version) { case 3: - way_function(profile_table, way, result, relations); + if (location_dependent_data.empty()) + { + way_function(profile_table, way, result, relations); + } + else + { + way_function(profile_table, way, result, relations, toLua(state, location_dependent_data(way))); + } break; case 2: - way_function(profile_table, way, result, location_dependent_data(state, way)); + way_function(profile_table, way, result); break; case 1: case 0: diff --git a/unit_tests/extractor/location_dependent_data_tests.cpp b/unit_tests/extractor/location_dependent_data_tests.cpp new file mode 100644 index 000000000..32972c508 --- /dev/null +++ b/unit_tests/extractor/location_dependent_data_tests.cpp @@ -0,0 +1,122 @@ +#include "extractor/location_dependent_data.hpp" + +#include +#include +#include + +#include +#include + +BOOST_AUTO_TEST_SUITE(location_dependent_data_tests) + +using namespace osrm; +using namespace osrm::extractor; +using point_t = LocationDependentData::point_t; + +struct LocationDataFixture +{ + LocationDataFixture(const std::string &json) : temporary_file(boost::filesystem::unique_path()) + { + std::ofstream file(temporary_file.string()); + file << json; + } + ~LocationDataFixture() { remove(temporary_file); } + + boost::filesystem::path temporary_file; +}; + +BOOST_AUTO_TEST_CASE(polygon_tests) +{ + LocationDataFixture fixture(R"json({ +"type": "FeatureCollection", +"features": [ +{ + "type": "Feature", + "properties": { + "answer": 42 + }, + "geometry": { "type": "Polygon", "coordinates": [ + [ [3, 0], [1, 1], [0, 3], [-1, 1], [-3, 0], [-1, -1], [0, -3], [1, -1], [3, 0] ], + [ [1, 0], [0, 1], [-1, 0], [0, -1], [1, 0] ] + ] } +}, +{ + "type": "Feature", + "properties": { + "answer": true + }, + "geometry": { "type": "Polygon", "coordinates": [ [ [0, 10], [3, 5], [1, 5], [10, 0], [-1, 5], [-3, 5], [0, 10] ] ] } +} +]})json"); + + LocationDependentData data(fixture.temporary_file); + BOOST_CHECK(data(point_t(0, 0)).empty()); + BOOST_CHECK(data(point_t(1, 1)).empty()); + BOOST_CHECK(data(point_t(0, 1)).empty()); + BOOST_CHECK(data(point_t(0.5, -0.5)).empty()); + BOOST_CHECK(data(point_t(0, -3)).empty()); + BOOST_CHECK(!data(point_t(-0.75, 0.75)).empty()); + BOOST_CHECK(!data(point_t(-2, 6)).empty()); + BOOST_CHECK_EQUAL(boost::get(data(point_t(2, 0))["answer"]), 42.); + BOOST_CHECK_EQUAL(boost::get(data(point_t(1, 7))["answer"]), true); +} + +BOOST_AUTO_TEST_CASE(multy_polygon_tests) +{ + LocationDataFixture fixture(R"json({ +"type": "FeatureCollection", +"features": [ +{ + "type": "Feature", + "properties": { + "answer": 42 + }, + "geometry": { "type": "MultiPolygon", "coordinates": [ + [ [ [1, 0], [0, 1], [-1, 0], [0, -1], [1, 0] ] ], + [ [ [6, 0], [5, 1], [4, 0], [5, -1], [6, 0] ] ], + [ [ [-4, 0], [-5, 1], [-6, 0], [-5, -1], [-4, 0] ] ] + ] } +} +]})json"); + + LocationDependentData data(fixture.temporary_file); + BOOST_CHECK(data(point_t(0, 2)).empty()); + BOOST_CHECK(data(point_t(0, -3)).empty()); + BOOST_CHECK_EQUAL(boost::get(data(point_t(0, 0))["answer"]), 42.); + BOOST_CHECK_EQUAL(boost::get(data(point_t(5, 0))["answer"]), 42.); + BOOST_CHECK_EQUAL(boost::get(data(point_t(-5, 0))["answer"]), 42.); +} + +BOOST_AUTO_TEST_CASE(polygon_merging_tests) +{ + LocationDataFixture fixture(R"json({ +"type": "FeatureCollection", +"features": [ +{ + "type": "Feature", + "properties": { "answer": "a" }, + "geometry": { "type": "Polygon", "coordinates": [ [ [3, 3], [-3, 3], [-3, -3], [3, -3], [3, 3] ] ] } +}, +{ + "type": "Feature", + "properties": { "answer": "b" }, + "geometry": { "type": "Polygon", "coordinates": [ [ [7, 3], [1, 3], [1, -3], [7, -3], [7, 3] ] ] } +}, +{ + "type": "Feature", + "properties": { "answer": "c" }, + "geometry": { "type": "Polygon", "coordinates": [ [ [8, 3], [2, 3], [2, -3], [8, -3], [8, 3] ] ] } +} +]})json"); + + LocationDependentData data(fixture.temporary_file); + BOOST_CHECK_EQUAL(boost::get(data(point_t(0, 0))["answer"]), "a"); + BOOST_CHECK_EQUAL(boost::get(data(point_t(1, 0))["answer"]), "a"); + BOOST_CHECK_EQUAL(boost::get(data(point_t(2, 0))["answer"]), "a"); + BOOST_CHECK_EQUAL(boost::get(data(point_t(3, 0))["answer"]), "b"); + BOOST_CHECK_EQUAL(boost::get(data(point_t(4, 0))["answer"]), "b"); + BOOST_CHECK_EQUAL(boost::get(data(point_t(6, 0))["answer"]), "b"); + BOOST_CHECK_EQUAL(boost::get(data(point_t(7, 0))["answer"]), "c"); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/unit_tests/util/serialization.cpp b/unit_tests/util/serialization.cpp index 0db1a92fe..7998572f6 100644 --- a/unit_tests/util/serialization.cpp +++ b/unit_tests/util/serialization.cpp @@ -1,11 +1,9 @@ #include "storage/serialization.hpp" #include -#include #include #include -#include #include BOOST_AUTO_TEST_SUITE(serialization_test)