From cbfb055f818e3daf72a3ce85f6eadf58d9773262 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Thu, 8 Dec 2016 13:20:23 +0100 Subject: [PATCH] Changes Single Coordinate Geoms from Point to LineString, closes #3425. --- CHANGELOG.md | 1 + docs/http.md | 2 +- include/engine/api/json_factory.hpp | 21 +++++---- unit_tests/CMakeLists.txt | 2 +- unit_tests/library/json.cpp | 71 +++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 unit_tests/library/json.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 31fcc41b7..9660b93c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - fixed a bug that could result in crashes when leaving a ferry directly onto a motorway ramp - fixed a bug in the tile plugin that resulted in discovering invalid edges for connections - improved error messages when missing files during traffic updates (#3114) + - For single coordinate geometries the GeoJSON `Point` encoding was broken. We now always emit `LineString`s even in the one-coordinate-case (backwards compatible) (#3425) - Debug Tiles - Added support for turn penalties - Internals diff --git a/docs/http.md b/docs/http.md index 14dd8cdf6..2b18f1fb1 100644 --- a/docs/http.md +++ b/docs/http.md @@ -519,7 +519,7 @@ step. |------------|--------------------------------------------------------------------| | polyline | [polyline](https://www.npmjs.com/package/polyline) with precision 5 in [latitude,longitude] encoding | | polyline6 | [polyline](https://www.npmjs.com/package/polyline) with precision 6 in [latitude,longitude] encoding | -| geojson | [GeoJSON `LineString`](http://geojson.org/geojson-spec.html#linestring) or [GeoJSON `Point`](http://geojson.org/geojson-spec.html#point) if it is only one coordinate (not wrapped by a GeoJSON feature)| +| geojson | [GeoJSON `LineString`](http://geojson.org/geojson-spec.html#linestring) | - `name`: The name of the way along which travel proceeds. - `ref`: A reference number or code for the way. Optionally included, if ref data is available for the given way. diff --git a/include/engine/api/json_factory.hpp b/include/engine/api/json_factory.hpp index cfb734f2b..b7227d33d 100644 --- a/include/engine/api/json_factory.hpp +++ b/include/engine/api/json_factory.hpp @@ -54,22 +54,25 @@ util::json::Object makeGeoJSONGeometry(ForwardIter begin, ForwardIter end) auto num_coordinates = std::distance(begin, end); BOOST_ASSERT(num_coordinates != 0); util::json::Object geojson; + geojson.values["type"] = "LineString"; + util::json::Array coordinates; if (num_coordinates > 1) { - geojson.values["type"] = "LineString"; - util::json::Array coordinates; coordinates.values.reserve(num_coordinates); - std::transform( - begin, end, std::back_inserter(coordinates.values), &detail::coordinateToLonLat); - geojson.values["coordinates"] = std::move(coordinates); + auto into = std::back_inserter(coordinates.values); + std::transform(begin, end, into, &detail::coordinateToLonLat); } else if (num_coordinates > 0) { - geojson.values["type"] = "Point"; - util::json::Array coordinates; - coordinates.values.push_back(detail::coordinateToLonLat(*begin)); - geojson.values["coordinates"] = std::move(coordinates); + // For a single location we create a [location, location] LineString + // instead of a single Point making the GeoJSON output consistent. + coordinates.values.reserve(2); + auto location = detail::coordinateToLonLat(*begin); + coordinates.values.push_back(location); + coordinates.values.push_back(location); } + geojson.values["coordinates"] = std::move(coordinates); + return geojson; } diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index 3b3144a6e..97c8c57b1 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -57,7 +57,7 @@ target_include_directories(util-tests PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(engine-tests ${ENGINE_LIBRARIES} ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}) target_link_libraries(extractor-tests ${EXTRACTOR_LIBRARIES} ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}) -target_link_libraries(library-tests osrm ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}) +target_link_libraries(library-tests osrm ${ENGINE_LIBRARIES} ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}) target_link_libraries(server-tests osrm ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}) target_link_libraries(util-tests ${UTIL_LIBRARIES} ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}) diff --git a/unit_tests/library/json.cpp b/unit_tests/library/json.cpp new file mode 100644 index 000000000..645acd396 --- /dev/null +++ b/unit_tests/library/json.cpp @@ -0,0 +1,71 @@ +#include +#include + +#include "coordinates.hpp" +#include "equal_json.hpp" + +#include "engine/api/json_factory.hpp" +#include "osrm/coordinate.hpp" + +#include +#include + +using namespace osrm; + +BOOST_AUTO_TEST_SUITE(json) + +BOOST_AUTO_TEST_CASE(test_json_linestring) +{ + const std::vector locations{get_dummy_location(), // + get_dummy_location(), // + get_dummy_location()}; // + + auto geom = engine::api::json::makeGeoJSONGeometry(begin(locations), end(locations)); + + const auto type = geom.values["type"].get().value; + BOOST_CHECK_EQUAL(type, "LineString"); + + const auto coords = geom.values["coordinates"].get().values; + BOOST_CHECK_EQUAL(coords.size(), 3); // array of three location arrays + + for (const auto each : coords) + { + const auto loc = each.get().values; + BOOST_CHECK_EQUAL(loc.size(), 2); + + const auto lon = loc[0].get().value; + const auto lat = loc[1].get().value; + + (void)lon; + (void)lat; + // cast fails if type do not match + } +} + +BOOST_AUTO_TEST_CASE(test_json_single_point) +{ + const std::vector locations{get_dummy_location()}; + + auto geom = engine::api::json::makeGeoJSONGeometry(begin(locations), end(locations)); + + const auto type = geom.values["type"].get().value; + BOOST_CHECK_EQUAL(type, "LineString"); + + const auto coords = geom.values["coordinates"].get().values; + BOOST_CHECK_EQUAL(coords.size(), 2); // array of two location arrays + + for (const auto each : coords) + { + const auto loc = each.get().values; + BOOST_CHECK_EQUAL(loc.size(), 2); + + const auto lon = loc[0].get().value; + const auto lat = loc[1].get().value; + + (void)lon; + (void)lat; + // cast fails if type do not match + } +} + +BOOST_AUTO_TEST_SUITE_END()