[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

@@ -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

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

include/mbgl/style/conversion/geojson_options.hpp

@@ -0,0 +1,78 @@
+#pragma once
+
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/sources/geojson_source.hpp>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <>
+struct Converter<GeoJSONOptions> {
+
+    template <class V>
+    Result<GeoJSONOptions> operator()(const V& value) const {
+        GeoJSONOptions options;
+
+        const auto maxzoomValue = objectMember(value, "maxzoom");
+        if (maxzoomValue) {
+            if (toNumber(*maxzoomValue)) {
+                options.maxzoom = static_cast<uint8_t>(*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<uint16_t>(*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<double>(*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<uint8_t>(*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<double>(*toNumber(*clusterRadiusValue));
+            } else {
+                return Error{ "GeoJSON source clusterRadius value must be a number" };
+            }
+        }
+
+        return { options };
+    }
+
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl

include/mbgl/style/conversion/source.hpp

@@ -2,6 +2,7 @@
 
 #include <mbgl/style/conversion.hpp>
 #include <mbgl/style/conversion/geojson.hpp>
+#include <mbgl/style/conversion/geojson_options.hpp>
 #include <mbgl/style/conversion/tileset.hpp>
 #include <mbgl/style/source.hpp>
 #include <mbgl/style/sources/geojson_source.hpp>
@@ -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<uint8_t>(*toNumber(*maxzoomValue));
-            } else {
-                return Error{ "GeoJSON source maxzoom value must be a number" };
-            }
+        Result<GeoJSONOptions> options = convert<GeoJSONOptions>(value);
+        if (!options) {
+            return options.error();
         }
 
-        const auto bufferValue = objectMember(value, "buffer");
-        if (bufferValue) {
-            if (toNumber(*bufferValue)) {
-                options.buffer = static_cast<uint16_t>(*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<double>(*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<uint8_t>(*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<double>(*toNumber(*clusterRadiusValue));
-            } else {
-                return Error{ "GeoJSON source clusterRadius value must be a number" };
-            }
-        }
-
-        auto result = std::make_unique<GeoJSONSource>(id, options);
+        auto result = std::make_unique<GeoJSONSource>(id, *options);
 
         if (isObject(*dataValue)) {
             Result<GeoJSON> geoJSON = convertGeoJSON(*dataValue);

test/src/mbgl/test/conversion_stubs.hpp

@@ -0,0 +1,111 @@
+#pragma once
+
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/util/feature.hpp>
+#include <mbgl/util/optional.hpp>
+#include <mbgl/util/variant.hpp>
+
+#include <string>
+#include <map>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+class Value;
+using ValueMap = std::map<std::string, Value>;
+using ValueVector = std::vector<Value>;
+class Value : public mbgl::variant<std::string, float, bool, ValueMap, ValueVector> {
+     using variant::variant;
+};
+
+inline bool isUndefined(const Value&) {
+    //Variant is always intialized
+    return false;
+}
+
+inline bool isArray(const Value& value) {
+    return value.is<ValueVector>();
+}
+
+inline std::size_t arrayLength(const Value& value) {
+    return value.get<ValueVector>().size();
+}
+
+inline Value arrayMember(const Value& value, std::size_t i) {
+    return value.get<ValueVector>()[i];
+}
+
+inline bool isObject(const Value& value) {
+    return value.is<ValueMap>();
+}
+
+inline optional<Value> objectMember(const Value& value, const char* key) {
+    auto map = value.get<ValueMap>();
+    auto iter = map.find(key);
+
+    if (iter != map.end()) {
+        return iter->second;
+    } else {
+        return {};
+    }
+}
+
+using EachMemberFn = std::function<optional<Error>(const std::string&, const Value&)>;
+
+optional<Error> eachMember(const Value& value, EachMemberFn&& fn) {
+    auto map = value.get<ValueMap>();
+    auto iter = map.begin();
+
+    while (iter != map.end()) {
+        optional<Error> result = fn(iter->first, iter->second);
+        if (result) {
+            return result;
+        }
+
+        ++iter;
+    }
+
+    return {};
+}
+
+inline optional<bool> toBool(const Value& value) {
+    if (value.is<bool>()) {
+        return value.get<bool>();
+    } else {
+        return {};
+    }
+}
+
+inline optional<float> toNumber(const Value& value) {
+    if (value.is<float>()) {
+        return value.get<float>();
+    } else {
+        return {};
+    }
+    return {};
+}
+
+inline optional<std::string> toString(const Value& value) {
+    if (value.is<std::string>()) {
+        return value.get<std::string>();
+    } else {
+        return {};
+    }
+}
+
+inline optional<mbgl::Value> toValue(const Value& value) {
+    if (value.is<bool>()) {
+        return { value.get<bool>() };
+    } else if (value.is<std::string>()) {
+        return { value.get<std::string>() };
+    } else if (value.is<float>()) {
+        return { value.get<float>() };
+    } else {
+        return {};
+    }
+}
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl

test/style/conversion/geojson_options.cpp

@@ -0,0 +1,68 @@
+#include <mbgl/test/util.hpp>
+
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/conversion/geojson_options.hpp>
+#include <mbgl/test/conversion_stubs.hpp>
+
+#include <mbgl/platform/log.hpp>
+
+using namespace mbgl::style;
+using namespace mbgl::style::conversion;
+
+TEST(GeoJSONOptions, Basic) {
+    ValueMap map;
+    Value raw(map);
+    Result<GeoJSONOptions> converted = convert<GeoJSONOptions>(raw);
+    ASSERT_TRUE((bool) converted);
+}
+
+TEST(GeoJSONOptions, ErrorHandling) {
+    ValueMap map {{"maxzoom", "should not be a string"}};
+    Value raw(map);
+    Result<GeoJSONOptions> converted = convert<GeoJSONOptions>(raw);
+    ASSERT_FALSE((bool) converted);
+}
+
+TEST(GeoJSONOptions, RetainsDefaults) {
+    ValueMap map;
+    Value raw(map);
+    GeoJSONOptions converted = *convert<GeoJSONOptions>(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<GeoJSONOptions>(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);
+}