[core] Introduce universal setter for GeoJSONSource

5 files changed, 59 insertions, 16 deletions

diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp
index 34848298bf..f6c0771734 100644
--- a/include/mbgl/style/sources/geojson_source.hpp
+++ b/include/mbgl/style/sources/geojson_source.hpp
@@ -83,6 +83,8 @@ public:
     Value serialize() const override;
 
 protected:
+    optional<conversion::Error> setPropertyInternal(const std::string& name,
+                                                    const conversion::Convertible& value) override;
     Mutable<Source::Impl> createMutable() const noexcept final;
 
 private:
diff --git a/src/mbgl/sourcemanager/geojson_source_factory.cpp b/src/mbgl/sourcemanager/geojson_source_factory.cpp
index 3152fe04ae..0a1ae75351 100644
--- a/src/mbgl/sourcemanager/geojson_source_factory.cpp
+++ b/src/mbgl/sourcemanager/geojson_source_factory.cpp
@@ -27,21 +27,12 @@ std::unique_ptr<style::Source> GeoJSONSourceFactory::createSource(const std::str
     }
 
     auto result = std::make_unique<style::GeoJSONSource>(id, std::move(options));
-
-    if (isObject(*dataValue)) {
-        optional<GeoJSON> geoJSON = style::conversion::convert<GeoJSON>(*dataValue, error);
-        if (!geoJSON) {
-            return nullptr;
-        }
-        result->setGeoJSON(*geoJSON);
-    } else if (toString(*dataValue)) {
-        result->setURL(*toString(*dataValue));
-    } else {
-        error.message = "GeoJSON data must be a URL or an object";
+    if (auto setDataError = result->setProperty("data", *dataValue)) {
+        error = *setDataError;
         return nullptr;
     }
 
-    return {std::move(result)};
+    return result;
 }
 
 std::unique_ptr<RenderSource> GeoJSONSourceFactory::createRenderSource(Immutable<style::Source::Impl> impl) noexcept {
diff --git a/src/mbgl/style/sources/geojson_source.cpp b/src/mbgl/style/sources/geojson_source.cpp
index 5d7605d754..f48e818b12 100644
--- a/src/mbgl/style/sources/geojson_source.cpp
+++ b/src/mbgl/style/sources/geojson_source.cpp
@@ -1,4 +1,5 @@
 #include <mbgl/storage/file_source.hpp>
+#include <mbgl/style/conversion/constant.hpp>
 #include <mbgl/style/conversion/geojson.hpp>
 #include <mbgl/style/conversion/json.hpp>
 #include <mbgl/style/layer.hpp>
@@ -142,6 +143,27 @@ Mutable<Source::Impl> GeoJSONSource::createMutable() const noexcept {
     return staticMutableCast<Source::Impl>(makeMutable<Impl>(impl()));
 }
 
+optional<conversion::Error> GeoJSONSource::setPropertyInternal(const std::string& name,
+                                                               const conversion::Convertible& value) {
+    using namespace conversion;
+    optional<Error> error = Source::setPropertyInternal(name, value);
+    assert(error);
+    if (name == "data") {
+        if (isObject(value)) {
+            if (auto geoJSON = convert<GeoJSON>(value, *error)) {
+                setGeoJSON(*geoJSON);
+                return nullopt;
+            }
+        } else if (auto url_ = convert<std::string>(value, *error)) {
+            setURL(*url_);
+            return nullopt;
+        } else {
+            error = Error{"GeoJSON data must be a URL or an object"};
+        }
+    }
+    return error;
+}
+
 Value GeoJSONSource::serialize() const {
     auto value = Source::serialize();
     assert(value.getObject());
diff --git a/src/mbgl/style/sources/image_source.cpp b/src/mbgl/style/sources/image_source.cpp
index 2e2f8c0148..b30f220520 100644
--- a/src/mbgl/style/sources/image_source.cpp
+++ b/src/mbgl/style/sources/image_source.cpp
@@ -121,7 +121,8 @@ Value ImageSource::serialize() const {
 optional<conversion::Error> ImageSource::setPropertyInternal(const std::string& name,
                                                              const conversion::Convertible& value) {
     using namespace conversion;
-    optional<Error> error;
+    optional<Error> error = Source::setPropertyInternal(name, value);
+    assert(error);
     if (name == "url") {
         if (auto url_ = convert<std::string>(value, *error)) {
             setURL(*url_);
@@ -133,7 +134,7 @@ optional<conversion::Error> ImageSource::setPropertyInternal(const std::string&
             return nullopt;
         }
     }
-    return error ? error : Source::setPropertyInternal(name, value);
+    return error;
 }
 
 Value ImageSource::getPropertyInternal(const std::string& name) const {
diff --git a/test/style/conversion/source.test.cpp b/test/style/conversion/source.test.cpp
index e889e01d1d..fcaff594e7 100644
--- a/test/style/conversion/source.test.cpp
+++ b/test/style/conversion/source.test.cpp
@@ -2,6 +2,7 @@
 
 #include <mbgl/style/conversion/json.hpp>
 #include <mbgl/style/conversion/source.hpp>
+#include <mbgl/style/sources/geojson_source.hpp>
 
 using namespace mbgl;
 using namespace mbgl::style;
@@ -17,12 +18,12 @@ std::unique_ptr<Source> parseSource(const std::string& src, const std::string& s
 
 void checkConstProperty(std::unique_ptr<Source>& source, const std::string& propertyName, const mbgl::Value& expected) {
     Value value = source->getProperty(propertyName);
-    EXPECT_EQ(expected, value) << propertyName;
+    EXPECT_EQ(expected, value) << "get property: " << propertyName;
 }
 
 void checkSetProperty(std::unique_ptr<Source>& source, const std::string& propertyName, const JSValue& value) {
     auto error = source->setProperty(propertyName, Convertible(&value));
-    EXPECT_EQ(nullopt, error) << error->message;
+    EXPECT_EQ(nullopt, error) << "set property: " << propertyName << ", error: " << error->message;
 }
 
 } // namespace
@@ -55,4 +56,30 @@ TEST(StyleConversion, SetSourceGenericProperties) {
     checkConstProperty(source, "max-overscale-factor-for-parent-tiles", NullValue());
     checkSetProperty(source, "max-overscale-factor-for-parent-tiles", JSValue(2));
     checkConstProperty(source, "max-overscale-factor-for-parent-tiles", 2u);
+
+    source = parseSource(R"JSON({
+        "type": "geojson",
+        "data": "http://127.0.0.1:3000/geojson.json"
+    })JSON",
+                         "geojson_source");
+    auto* geojsonSource = static_cast<GeoJSONSource*>(source.get());
+    ASSERT_NE(nullptr, source);
+    auto url = geojsonSource->getURL();
+    ASSERT_TRUE(url);
+    EXPECT_EQ("http://127.0.0.1:3000/geojson.json", *url);
+
+    checkSetProperty(source, "data", JSValue("http://127.0.0.1:3000/geojson1.json"));
+    url = geojsonSource->getURL();
+    ASSERT_TRUE(url);
+    EXPECT_EQ("http://127.0.0.1:3000/geojson1.json", *url);
+
+    EXPECT_FALSE(geojsonSource->getGeoJSONData());
+    JSDocument document;
+    document.Parse<0>(R"({
+        "type": "Point",
+        "coordinates": [0, 0]
+    })");
+    const JSValue* geometry = &document;
+    checkSetProperty(source, "data", *geometry);
+    EXPECT_TRUE(geojsonSource->getGeoJSONData());
 }
\ No newline at end of file