enable clusterProperty render test

12 files changed, 98 insertions, 42 deletions

diff --git a/include/mbgl/style/expression/dsl.hpp b/include/mbgl/style/expression/dsl.hpp
index 9d1c87e37e..9c117d4d9b 100644
--- a/include/mbgl/style/expression/dsl.hpp
+++ b/include/mbgl/style/expression/dsl.hpp
@@ -1,21 +1,21 @@
 #pragma once
 
-#include <mbgl/style/expression/value.hpp>
 #include <mbgl/style/expression/expression.hpp>
 #include <mbgl/style/expression/interpolator.hpp>
+#include <mbgl/style/expression/value.hpp>
 #include <mbgl/util/ignore.hpp>
 
+#include <initializer_list>
 #include <memory>
 #include <string>
-#include <initializer_list>
 
 namespace mbgl {
 namespace style {
 namespace expression {
 namespace dsl {
 
-// This convenience API does little to no expression validation or type-checking, and is intended for
-// use only by test and other non-production code.
+// This convenience API does little to no expression validation or type-checking, and is intended
+// for use only by test and other non-production code.
 
 template <class... Args>
 std::vector<std::unique_ptr<Expression>> vec(Args... args) {
@@ -24,15 +24,16 @@ std::vector<std::unique_ptr<Expression>> vec(Args... args) {
     return result;
 }
 std::unique_ptr<Expression> createExpression(const char* expr);
+std::unique_ptr<Expression> createExpression(const mbgl::style::conversion::Convertible& expr);
 std::unique_ptr<Expression> error(std::string);
 
 std::unique_ptr<Expression> literal(const char* value);
 std::unique_ptr<Expression> literal(Value value);
 std::unique_ptr<Expression> literal(std::initializer_list<double> value);
-std::unique_ptr<Expression> literal(std::initializer_list<const char *> value);
+std::unique_ptr<Expression> literal(std::initializer_list<const char*> value);
 
-std::unique_ptr<Expression> assertion(type::Type, std::unique_ptr<Expression>,
-                                      std::unique_ptr<Expression> def = nullptr);
+std::unique_ptr<Expression>
+assertion(type::Type, std::unique_ptr<Expression>, std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> number(std::unique_ptr<Expression>,
                                    std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> string(std::unique_ptr<Expression>,
@@ -46,21 +47,17 @@ std::unique_ptr<Expression> toString(std::unique_ptr<Expression>,
                                      std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> toFormatted(std::unique_ptr<Expression>,
                                         std::unique_ptr<Expression> def = nullptr);
-    
+
 std::unique_ptr<Expression> get(const char* value);
 std::unique_ptr<Expression> get(std::unique_ptr<Expression>);
 
 std::unique_ptr<Expression> id();
 std::unique_ptr<Expression> zoom();
 
-std::unique_ptr<Expression> eq(std::unique_ptr<Expression>,
-                               std::unique_ptr<Expression>);
-std::unique_ptr<Expression> ne(std::unique_ptr<Expression>,
-                               std::unique_ptr<Expression>);
-std::unique_ptr<Expression> gt(std::unique_ptr<Expression>,
-                               std::unique_ptr<Expression>);
-std::unique_ptr<Expression> lt(std::unique_ptr<Expression>,
-                               std::unique_ptr<Expression>);
+std::unique_ptr<Expression> eq(std::unique_ptr<Expression>, std::unique_ptr<Expression>);
+std::unique_ptr<Expression> ne(std::unique_ptr<Expression>, std::unique_ptr<Expression>);
+std::unique_ptr<Expression> gt(std::unique_ptr<Expression>, std::unique_ptr<Expression>);
+std::unique_ptr<Expression> lt(std::unique_ptr<Expression>, std::unique_ptr<Expression>);
 
 std::unique_ptr<Expression> step(std::unique_ptr<Expression> input,
                                  std::unique_ptr<Expression> output0,
@@ -86,7 +83,7 @@ std::unique_ptr<Expression> interpolate(Interpolator interpolator,
                                         double input3, std::unique_ptr<Expression> output3);
 
 std::unique_ptr<Expression> concat(std::vector<std::unique_ptr<Expression>> inputs);
-    
+
 std::unique_ptr<Expression> format(const char* value);
 std::unique_ptr<Expression> format(std::unique_ptr<Expression>);
 
diff --git a/include/mbgl/style/expression/expression.hpp b/include/mbgl/style/expression/expression.hpp
index 9ba5c72bce..7168128b91 100644
--- a/include/mbgl/style/expression/expression.hpp
+++ b/include/mbgl/style/expression/expression.hpp
@@ -6,6 +6,7 @@
 #include <mbgl/style/expression/type.hpp>
 #include <mbgl/style/expression/value.hpp>
 #include <mbgl/style/expression/parsing_context.hpp>
+#include <mbgl/util/feature.hpp>
 
 #include <array>
 #include <vector>
@@ -32,8 +33,8 @@ public:
     EvaluationContext(float zoom_, GeometryTileFeature const * feature_) :
         zoom(zoom_), feature(feature_)
     {}
-    EvaluationContext(optional<double> accumulated_, GeometryTileFeature const * feature_) :
-    accumulated(std::move(accumulated_)), feature(feature_)
+    EvaluationContext(optional<mbgl::Value> accumulated_, GeometryTileFeature const * feature_) :
+        accumulated(std::move(accumulated_)), feature(feature_)
     {}
     EvaluationContext(optional<float> zoom_, GeometryTileFeature const * feature_, optional<double> colorRampParameter_) :
         zoom(std::move(zoom_)), feature(feature_), colorRampParameter(std::move(colorRampParameter_))
@@ -45,7 +46,7 @@ public:
     };
 
     optional<float> zoom;
-    optional<double> accumulated;
+    optional<mbgl::Value> accumulated;
     GeometryTileFeature const * feature = nullptr;
     optional<double> colorRampParameter;
 
@@ -168,7 +169,7 @@ public:
     type::Type getType() const { return type; };
     
     EvaluationResult evaluate(optional<float> zoom, const Feature& feature, optional<double> colorRampParameter) const;
-    EvaluationResult evaluate(optional<double> accumulated, const Feature& feature) const;
+    EvaluationResult evaluate(optional<mbgl::Value> accumulated, const Feature& feature) const;
     /**
      * Statically analyze the expression, attempting to enumerate possible outputs. Returns
      * an array of values plus the sentinel null optional value, used to indicate that the
diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp
index 7141765d3c..447fd1f3b4 100644
--- a/include/mbgl/style/sources/geojson_source.hpp
+++ b/include/mbgl/style/sources/geojson_source.hpp
@@ -38,7 +38,7 @@ struct GeoJSONOptions {
 
 class GeoJSONSource : public Source {
 public:
-    GeoJSONSource(const std::string& id, const GeoJSONOptions& = {});
+    GeoJSONSource(const std::string& id, optional<GeoJSONOptions> = nullopt);
     ~GeoJSONSource() final;
 
     void setURL(const std::string& url);
diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json
index 4e4f537a23..6d7612164c 100644
--- a/platform/node/test/ignores.json
+++ b/platform/node/test/ignores.json
@@ -92,7 +92,6 @@
   "render-tests/text-variable-anchor/pitched-rotated-debug": "https://github.com/mapbox/mapbox-gl-native/issues/14211",
   "render-tests/text-variable-anchor/rotated-offset": "https://github.com/mapbox/mapbox-gl-native/issues/14211",
   "render-tests/text-variable-anchor/remember-last-placement": "skip - fails on gl-native, as symbol index is not functional at static map mode - needs issue",
-  "render-tests/geojson/clustered-properties": "https://github.com/mapbox/mapbox-gl-native/issues/14043",
   "render-tests/remove-feature-state/composite-expression": "https://github.com/mapbox/mapbox-gl-native/issues/12413",
   "render-tests/remove-feature-state/data-expression": "https://github.com/mapbox/mapbox-gl-native/issues/12413",
   "render-tests/remove-feature-state/vector-source": "https://github.com/mapbox/mapbox-gl-native/issues/12413",
diff --git a/src/mbgl/style/conversion/geojson_options.cpp b/src/mbgl/style/conversion/geojson_options.cpp
index 0a654e406e..6bcc325da2 100644
--- a/src/mbgl/style/conversion/geojson_options.cpp
+++ b/src/mbgl/style/conversion/geojson_options.cpp
@@ -1,11 +1,15 @@
 #include <mbgl/style/conversion/geojson_options.hpp>
 #include <mbgl/style/conversion_impl.hpp>
+#include <mbgl/style/expression/dsl.hpp>
+#include <sstream>
+#include <utility>
 
 namespace mbgl {
 namespace style {
 namespace conversion {
 
-optional<GeoJSONOptions> Converter<GeoJSONOptions>::operator()(const Convertible& value, Error& error) const {
+optional<GeoJSONOptions> Converter<GeoJSONOptions>::operator()(const Convertible& value,
+                                                               Error& error) const {
     GeoJSONOptions options;
 
     const auto minzoomValue = objectMember(value, "minzoom");
@@ -83,11 +87,51 @@ optional<GeoJSONOptions> Converter<GeoJSONOptions>::operator()(const Convertible
         if (toBool(*lineMetricsValue)) {
             options.lineMetrics = *toBool(*lineMetricsValue);
         } else {
-            error = { "GeoJSON source lineMetrics value must be a boolean" };
+            error.message = "GeoJSON source lineMetrics value must be a boolean";
             return nullopt;
         }
     }
 
+    const auto clusterProperties = objectMember(value, "clusterProperties");
+    if (clusterProperties) {
+        if (!isObject(*clusterProperties)) {
+            error.message = "GeoJSON source clusterProperties value must be an object";
+            return nullopt;
+        }
+        GeoJSONOptions::ClusterProperties result;
+        eachMember(
+            *clusterProperties,
+            [&](const std::string& k,
+                const mbgl::style::conversion::Convertible& v) -> optional<conversion::Error> {
+                // "key" : [operator, [mapExpression]]
+                if (!isArray(v) || arrayLength(v) != 2) {
+                    error.message =
+                        "GeoJSON source clusterProperties member must be an array with length of 2";
+                    return {};
+                }
+                auto reduceOp = toString(arrayMember(v, 0));
+                if (!reduceOp) {
+                    error.message =
+                        "GeoJSON source clusterProperties member must contain a valid operator";
+                    return {};
+                }
+                auto map = expression::dsl::createExpression(arrayMember(v, 1));
+                std::stringstream ss;
+                // [operator, ['accumulated'], ['get', key]]
+                ss << std::string(R"([")") << *reduceOp
+                   << std::string(R"(", ["accumulated"], ["get", ")") << k << std::string(R"("]])");
+                auto reduce = expression::dsl::createExpression(ss.str().c_str());
+                if (map && reduce) {
+                    result.emplace(k, std::make_pair(std::move(map), std::move(reduce)));
+                }
+                return {};
+            });
+        if (!error.message.empty()) {
+            return nullopt;
+        }
+        options.clusterProperties = std::move(result);
+    }
+
     return { std::move(options) };
 }
 
diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp
index 6ca6eec850..825307432b 100644
--- a/src/mbgl/style/expression/compound_expression.cpp
+++ b/src/mbgl/style/expression/compound_expression.cpp
@@ -369,13 +369,13 @@ const auto& lineProgressCompoundExpression() {
 }
 
 const auto& accumulatedCompoundExpression() {
-    static auto signature = detail::makeSignature("accumulated", [](const EvaluationContext& params) -> Result<double> {
+    static auto signature = detail::makeSignature("accumulated", [](const EvaluationContext& params) -> Result<Value> {
         if (!params.accumulated) {
             return EvaluationError {
                 "The 'accumulated' expression is unavailable in the current evaluation context."
             };
         }
-        return *(params.accumulated);
+        return Value(toExpressionValue(*params.accumulated));
     });
     return signature;
 }
diff --git a/src/mbgl/style/expression/dsl.cpp b/src/mbgl/style/expression/dsl.cpp
index 87494344fe..b26bce3f6c 100644
--- a/src/mbgl/style/expression/dsl.cpp
+++ b/src/mbgl/style/expression/dsl.cpp
@@ -30,15 +30,20 @@ std::unique_ptr<Expression> createExpression(const char* expr) {
     using JSValue = rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>;
     rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> document;
     document.Parse<0>(expr);
-    assert(!document.HasParseError());
+    if (document.HasParseError()) return nullptr;
     
-    //        optional<expression::TypeAnnotationOption> typeAnnotationOption;
+    //optional<expression::TypeAnnotationOption> typeAnnotationOption;
     const JSValue* expression = &document;
     expression::ParsingContext ctx;
     expression::ParseResult parsed =
     ctx.parseExpression(mbgl::style::conversion::Convertible(expression));
-    assert(parsed);
-    return std::move(*parsed);
+    return parsed ? std::move(*parsed) : nullptr;
+}
+
+std::unique_ptr<Expression> createExpression(const mbgl::style::conversion::Convertible& expr) {
+    expression::ParsingContext ctx;
+    expression::ParseResult parsed = ctx.parseExpression(expr);
+    return parsed ? std::move(*parsed) : nullptr;
 }
 
 std::unique_ptr<Expression> error(std::string message) {
diff --git a/src/mbgl/style/expression/expression.cpp b/src/mbgl/style/expression/expression.cpp
index 4a7c10fa74..b3641b9448 100644
--- a/src/mbgl/style/expression/expression.cpp
+++ b/src/mbgl/style/expression/expression.cpp
@@ -33,7 +33,7 @@ EvaluationResult Expression::evaluate(optional<float> zoom, const Feature& featu
     return this->evaluate(EvaluationContext(zoom,  &f, colorRampParameter));
 }
 
-EvaluationResult Expression::evaluate(optional<double> accumulated, const Feature& feature) const{
+    EvaluationResult Expression::evaluate(optional<mbgl::Value> accumulated, const Feature& feature) const{
     GeoJSONFeature f(feature);
     return this->evaluate(EvaluationContext(accumulated,&f));
 }
diff --git a/src/mbgl/style/sources/geojson_source.cpp b/src/mbgl/style/sources/geojson_source.cpp
index 4e3478322d..72a51e212f 100644
--- a/src/mbgl/style/sources/geojson_source.cpp
+++ b/src/mbgl/style/sources/geojson_source.cpp
@@ -9,7 +9,7 @@
 namespace mbgl {
 namespace style {
 
-GeoJSONSource::GeoJSONSource(const std::string& id, const GeoJSONOptions& options)
+GeoJSONSource::GeoJSONSource(const std::string& id, optional<GeoJSONOptions> options)
     : Source(makeMutable<Impl>(std::move(id), options)) {
 }
 
diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp
index fcef35730b..ba26115957 100644
--- a/src/mbgl/style/sources/geojson_source_impl.cpp
+++ b/src/mbgl/style/sources/geojson_source_impl.cpp
@@ -71,7 +71,7 @@ private:
 
 template <class T>
 T EvaluateFeature(optional<T> accumulated,
-                  mapbox::feature::feature<T>& f,
+                  mapbox::feature::feature<double>& f,
                   const std::shared_ptr<expression::Expression> expression,
                   T finalDefaultValue = T()) {
     const expression::EvaluationResult result = expression->evaluate(accumulated, f);
@@ -82,9 +82,9 @@ T EvaluateFeature(optional<T> accumulated,
     return finalDefaultValue;
 }
 
-GeoJSONSource::Impl::Impl(std::string id_, GeoJSONOptions options_)
+GeoJSONSource::Impl::Impl(std::string id_, optional<GeoJSONOptions> options_)
     : Source::Impl(SourceType::GeoJSON, std::move(id_)) {
-    options = std::move(options_);
+        options = options_ ? std::move(*options_) : GeoJSONOptions{};
 }
 
 GeoJSONSource::Impl::Impl(const Impl& other, const GeoJSON& geoJSON) : Source::Impl(other) {
@@ -103,7 +103,7 @@ GeoJSONSource::Impl::Impl(const Impl& other, const GeoJSON& geoJSON) : Source::I
             for (const auto& p : options.clusterProperties) {
                 auto feature = mapbox::feature::feature<double>();
                 feature.properties = properties;
-                ret[p.first] = EvaluateFeature<double>(nullopt, feature, p.second.first);
+                ret[p.first] = EvaluateFeature<mapbox::feature::value>(nullopt, feature, p.second.first);
             }
             return ret;
         };
@@ -112,9 +112,9 @@ GeoJSONSource::Impl::Impl(const Impl& other, const GeoJSON& geoJSON) : Source::I
             for (const auto& p : options.clusterProperties) {
                 auto feature = mapbox::feature::feature<double>();
                 feature.properties = toFill;
-                optional<double> accumulated(toReturn[p.first].get<double>());
-                toReturn[p.first] = EvaluateFeature<double>(accumulated, feature, p.second.second);
-                ;
+                optional<mapbox::feature::value> accumulated(toReturn[p.first]);
+                toReturn[p.first] = EvaluateFeature<mapbox::feature::value>(accumulated, feature, p.second.second);
+                
             }
         };
         data = std::make_shared<SuperclusterData>(
diff --git a/src/mbgl/style/sources/geojson_source_impl.hpp b/src/mbgl/style/sources/geojson_source_impl.hpp
index 8d40b58100..50528a01d1 100644
--- a/src/mbgl/style/sources/geojson_source_impl.hpp
+++ b/src/mbgl/style/sources/geojson_source_impl.hpp
@@ -26,7 +26,7 @@ public:
 
 class GeoJSONSource::Impl : public Source::Impl {
 public:
-    Impl(std::string id, GeoJSONOptions);
+    Impl(std::string id, optional<GeoJSONOptions>);
     Impl(const GeoJSONSource::Impl&, const GeoJSON&);
     ~Impl() final;
  
diff --git a/test/style/conversion/geojson_options.test.cpp b/test/style/conversion/geojson_options.test.cpp
index 181189775b..b50707bbe6 100644
--- a/test/style/conversion/geojson_options.test.cpp
+++ b/test/style/conversion/geojson_options.test.cpp
@@ -38,6 +38,7 @@ TEST(GeoJSONOptions, RetainsDefaults) {
     ASSERT_EQ(converted.cluster, defaults.cluster);
     ASSERT_EQ(converted.clusterRadius, defaults.clusterRadius);
     ASSERT_EQ(converted.clusterMaxZoom, defaults.clusterMaxZoom);
+    ASSERT_TRUE(converted.clusterProperties.empty());
 }
 
 TEST(GeoJSONOptions, FullConversion) {
@@ -49,7 +50,12 @@ TEST(GeoJSONOptions, FullConversion) {
         "cluster": true,
         "clusterRadius": 4,
         "clusterMaxZoom": 5,
-        "lineMetrics": true
+        "lineMetrics": true,
+        "clusterProperties": {
+            "max": ["max", ["get", "scalerank"]],
+            "sum": ["+", ["get", "scalerank"]],
+            "has_island": ["any", ["==", ["get", "featureclass"], "island"]]
+        }
     })JSON", error);
 
     // GeoJSON-VT
@@ -63,4 +69,8 @@ TEST(GeoJSONOptions, FullConversion) {
     ASSERT_EQ(converted.cluster, true);
     ASSERT_EQ(converted.clusterRadius, 4);
     ASSERT_EQ(converted.clusterMaxZoom, 5);
+    ASSERT_EQ(converted.clusterProperties.size(), 3);
+    ASSERT_EQ(converted.clusterProperties.count("max"), 1);
+    ASSERT_EQ(converted.clusterProperties.count("sum"), 1);
+    ASSERT_EQ(converted.clusterProperties.count("has_island"), 1);
 }