From df3b44531e1c2a95edd2a035d3744f34ebb8d0e9 Mon Sep 17 00:00:00 2001 From: Ivo van Dongen Date: Tue, 23 Aug 2016 14:57:55 +0200 Subject: [PATCH] [core] #6071 - extract GeoJSONOptions conversion from GeoJSONSource conversion * [core] geojson_options - retain original error message * [core] tests - initial style conversion stub methods * [core] geojsonoptions conversion - initial unit tests * [core] tests - fix forward reference issue * [core] geojsonoptions conversion - unit tests * [core] geojsonoptions conversion - renamed Holder to Value --- cmake/core-files.cmake | 1 + cmake/test-files.cmake | 4 + .../mbgl/style/conversion/geojson_options.hpp | 78 ++++++++++++ include/mbgl/style/conversion/source.hpp | 60 +--------- test/src/mbgl/test/conversion_stubs.hpp | 111 ++++++++++++++++++ test/style/conversion/geojson_options.cpp | 68 +++++++++++ 6 files changed, 267 insertions(+), 55 deletions(-) create mode 100644 include/mbgl/style/conversion/geojson_options.hpp create mode 100644 test/src/mbgl/test/conversion_stubs.hpp create mode 100644 test/style/conversion/geojson_options.cpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index d817909e3..e2795072b 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -256,6 +256,7 @@ set(MBGL_CORE_FILES include/mbgl/style/conversion/filter.hpp include/mbgl/style/conversion/function.hpp include/mbgl/style/conversion/geojson.hpp + include/mbgl/style/conversion/geojson_options.hpp include/mbgl/style/conversion/layer.hpp include/mbgl/style/conversion/make_property_setters.hpp include/mbgl/style/conversion/property_setter.hpp diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index d6c621529..4736af6c5 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -42,6 +42,7 @@ set(MBGL_TEST_FILES test/src/main.cpp # src/mbgl/test + test/src/mbgl/test/conversion_stubs.hpp test/src/mbgl/test/fixture_log_observer.cpp test/src/mbgl/test/fixture_log_observer.hpp test/src/mbgl/test/stub_file_source.cpp @@ -72,6 +73,9 @@ set(MBGL_TEST_FILES test/style/style_parser.cpp test/style/tile_source.cpp test/style/variant.cpp + + # style conversion + test/style/conversion/geojson_options.cpp # text test/text/quads.cpp diff --git a/include/mbgl/style/conversion/geojson_options.hpp b/include/mbgl/style/conversion/geojson_options.hpp new file mode 100644 index 000000000..880090b40 --- /dev/null +++ b/include/mbgl/style/conversion/geojson_options.hpp @@ -0,0 +1,78 @@ +#pragma once + +#include +#include + +namespace mbgl { +namespace style { +namespace conversion { + +template <> +struct Converter { + + template + Result operator()(const V& value) const { + GeoJSONOptions options; + + const auto maxzoomValue = objectMember(value, "maxzoom"); + if (maxzoomValue) { + if (toNumber(*maxzoomValue)) { + options.maxzoom = static_cast(*toNumber(*maxzoomValue)); + } else { + return Error{ "GeoJSON source maxzoom value must be a number" }; + } + } + + const auto bufferValue = objectMember(value, "buffer"); + if (bufferValue) { + if (toNumber(*bufferValue)) { + options.buffer = static_cast(*toNumber(*bufferValue)); + } else { + return Error{ "GeoJSON source buffer value must be a number" }; + } + } + + const auto toleranceValue = objectMember(value, "tolerance"); + if (toleranceValue) { + if (toNumber(*toleranceValue)) { + options.tolerance = static_cast(*toNumber(*toleranceValue)); + } else { + return Error{ "GeoJSON source tolerance value must be a number" }; + } + } + + const auto clusterValue = objectMember(value, "cluster"); + if (clusterValue) { + if (toBool(*clusterValue)) { + options.cluster = *toBool(*clusterValue); + } else { + return Error{ "GeoJSON source cluster value must be a boolean" }; + } + } + + const auto clusterMaxZoomValue = objectMember(value, "clusterMaxZoom"); + if (clusterMaxZoomValue) { + if (toNumber(*clusterMaxZoomValue)) { + options.clusterMaxZoom = static_cast(*toNumber(*clusterMaxZoomValue)); + } else { + return Error{ "GeoJSON source clusterMaxZoom value must be a number" }; + } + } + + const auto clusterRadiusValue = objectMember(value, "clusterRadius"); + if (clusterRadiusValue) { + if (toNumber(*clusterRadiusValue)) { + options.clusterRadius = static_cast(*toNumber(*clusterRadiusValue)); + } else { + return Error{ "GeoJSON source clusterRadius value must be a number" }; + } + } + + return { options }; + } + +}; + +} // namespace conversion +} // namespace style +} // namespace mbgl diff --git a/include/mbgl/style/conversion/source.hpp b/include/mbgl/style/conversion/source.hpp index c4b2fe303..6e1b4347c 100644 --- a/include/mbgl/style/conversion/source.hpp +++ b/include/mbgl/style/conversion/source.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -103,63 +104,12 @@ private: return Error{ "GeoJSON source must have a data value" }; } - GeoJSONOptions options; - - const auto maxzoomValue = objectMember(value, "maxzoom"); - if (maxzoomValue) { - if (toNumber(*maxzoomValue)) { - options.maxzoom = static_cast(*toNumber(*maxzoomValue)); - } else { - return Error{ "GeoJSON source maxzoom value must be a number" }; - } + Result options = convert(value); + if (!options) { + return options.error(); } - const auto bufferValue = objectMember(value, "buffer"); - if (bufferValue) { - if (toNumber(*bufferValue)) { - options.buffer = static_cast(*toNumber(*bufferValue)); - } else { - return Error{ "GeoJSON source buffer value must be a number" }; - } - } - - const auto toleranceValue = objectMember(value, "tolerance"); - if (toleranceValue) { - if (toNumber(*toleranceValue)) { - options.tolerance = static_cast(*toNumber(*toleranceValue)); - } else { - return Error{ "GeoJSON source tolerance value must be a number" }; - } - } - - const auto clusterValue = objectMember(value, "cluster"); - if (clusterValue) { - if (toBool(*clusterValue)) { - options.cluster = *toBool(*clusterValue); - } else { - return Error{ "GeoJSON source cluster value must be a boolean" }; - } - } - - const auto clusterMaxZoomValue = objectMember(value, "clusterMaxZoom"); - if (clusterMaxZoomValue) { - if (toNumber(*clusterMaxZoomValue)) { - options.clusterMaxZoom = static_cast(*toNumber(*clusterMaxZoomValue)); - } else { - return Error{ "GeoJSON source clusterMaxZoom value must be a number" }; - } - } - - const auto clusterRadiusValue = objectMember(value, "clusterRadius"); - if (clusterRadiusValue) { - if (toNumber(*clusterRadiusValue)) { - options.clusterRadius = static_cast(*toNumber(*clusterRadiusValue)); - } else { - return Error{ "GeoJSON source clusterRadius value must be a number" }; - } - } - - auto result = std::make_unique(id, options); + auto result = std::make_unique(id, *options); if (isObject(*dataValue)) { Result geoJSON = convertGeoJSON(*dataValue); diff --git a/test/src/mbgl/test/conversion_stubs.hpp b/test/src/mbgl/test/conversion_stubs.hpp new file mode 100644 index 000000000..60f110ea0 --- /dev/null +++ b/test/src/mbgl/test/conversion_stubs.hpp @@ -0,0 +1,111 @@ +#pragma once + +#include +#include +#include +#include + +#include +#include + +namespace mbgl { +namespace style { +namespace conversion { + +class Value; +using ValueMap = std::map; +using ValueVector = std::vector; +class Value : public mbgl::variant { + using variant::variant; +}; + +inline bool isUndefined(const Value&) { + //Variant is always intialized + return false; +} + +inline bool isArray(const Value& value) { + return value.is(); +} + +inline std::size_t arrayLength(const Value& value) { + return value.get().size(); +} + +inline Value arrayMember(const Value& value, std::size_t i) { + return value.get()[i]; +} + +inline bool isObject(const Value& value) { + return value.is(); +} + +inline optional objectMember(const Value& value, const char* key) { + auto map = value.get(); + auto iter = map.find(key); + + if (iter != map.end()) { + return iter->second; + } else { + return {}; + } +} + +using EachMemberFn = std::function(const std::string&, const Value&)>; + +optional eachMember(const Value& value, EachMemberFn&& fn) { + auto map = value.get(); + auto iter = map.begin(); + + while (iter != map.end()) { + optional result = fn(iter->first, iter->second); + if (result) { + return result; + } + + ++iter; + } + + return {}; +} + +inline optional toBool(const Value& value) { + if (value.is()) { + return value.get(); + } else { + return {}; + } +} + +inline optional toNumber(const Value& value) { + if (value.is()) { + return value.get(); + } else { + return {}; + } + return {}; +} + +inline optional toString(const Value& value) { + if (value.is()) { + return value.get(); + } else { + return {}; + } +} + +inline optional toValue(const Value& value) { + if (value.is()) { + return { value.get() }; + } else if (value.is()) { + return { value.get() }; + } else if (value.is()) { + return { value.get() }; + } else { + return {}; + } +} + +} // namespace conversion +} // namespace style +} // namespace mbgl diff --git a/test/style/conversion/geojson_options.cpp b/test/style/conversion/geojson_options.cpp new file mode 100644 index 000000000..46a2aa950 --- /dev/null +++ b/test/style/conversion/geojson_options.cpp @@ -0,0 +1,68 @@ +#include + +#include +#include +#include + +#include + +using namespace mbgl::style; +using namespace mbgl::style::conversion; + +TEST(GeoJSONOptions, Basic) { + ValueMap map; + Value raw(map); + Result converted = convert(raw); + ASSERT_TRUE((bool) converted); +} + +TEST(GeoJSONOptions, ErrorHandling) { + ValueMap map {{"maxzoom", "should not be a string"}}; + Value raw(map); + Result converted = convert(raw); + ASSERT_FALSE((bool) converted); +} + +TEST(GeoJSONOptions, RetainsDefaults) { + ValueMap map; + Value raw(map); + GeoJSONOptions converted = *convert(raw); + GeoJSONOptions defaults; + + //GeoJSON-VT + ASSERT_EQ(converted.maxzoom, defaults.maxzoom); + ASSERT_EQ(converted.buffer, defaults.buffer); + ASSERT_EQ(converted.tolerance, defaults.tolerance); + + //Supercluster + ASSERT_EQ(converted.cluster, defaults.cluster); + ASSERT_EQ(converted.clusterRadius, defaults.clusterRadius); + ASSERT_EQ(converted.clusterMaxZoom, defaults.clusterMaxZoom); +} + + +TEST(GeoJSONOptions, FullConversion) { + ValueMap map { + //GeoJSON-VT + {"maxzoom", 1}, + {"buffer", 2}, + {"tolerance", 3}, + + //Supercluster + {"cluster", true}, + {"clusterRadius", 4}, + {"clusterMaxZoom", 5} + }; + Value raw(map); + GeoJSONOptions converted = *convert(raw); + + //GeoJSON-VT + ASSERT_EQ(converted.maxzoom, 1); + ASSERT_EQ(converted.buffer, 2); + ASSERT_EQ(converted.tolerance, 3); + + //Supercluster + ASSERT_EQ(converted.cluster, true); + ASSERT_EQ(converted.clusterRadius, 4); + ASSERT_EQ(converted.clusterMaxZoom, 5); +}