[core] Add support for $id key to filters

https://github.com/mapbox/mapbox-gl-style-spec/issues/391

9 files changed, 116 insertions, 85 deletions

diff --git a/benchmark/parse/filter.cpp b/benchmark/parse/filter.cpp
index 6b430a9b11..5214f682d6 100644
--- a/benchmark/parse/filter.cpp
+++ b/benchmark/parse/filter.cpp
@@ -28,7 +28,7 @@ static void Parse_EvaluateFilter(benchmark::State& state) {
     const PropertyMap properties = { { "foo", std::string("bar") } };
 
     while (state.KeepRunning()) {
-        filter(FeatureType::Unknown, [&] (const std::string& key) -> optional<Value> {
+        filter(FeatureType::Unknown, {}, [&] (const std::string& key) -> optional<Value> {
             auto it = properties.find(key);
             if (it == properties.end())
                 return {};
diff --git a/include/mbgl/style/filter.hpp b/include/mbgl/style/filter.hpp
index 6a5afb7b47..dd2b20cd0d 100644
--- a/include/mbgl/style/filter.hpp
+++ b/include/mbgl/style/filter.hpp
@@ -107,8 +107,10 @@ class Filter : public FilterBase {
 public:
     using FilterBase::FilterBase;
 
+    bool operator()(const Feature&) const;
+
     template <class PropertyAccessor>
-    bool operator()(FeatureType type, PropertyAccessor accessor) const;
+    bool operator()(FeatureType type, optional<FeatureIdentifier> id, PropertyAccessor accessor) const;
 };
 
 } // namespace style
diff --git a/include/mbgl/style/filter_evaluator.hpp b/include/mbgl/style/filter_evaluator.hpp
index cf91fdab1f..793abe6da0 100644
--- a/include/mbgl/style/filter_evaluator.hpp
+++ b/include/mbgl/style/filter_evaluator.hpp
@@ -26,6 +26,7 @@ template <class PropertyAccessor>
 class FilterEvaluator {
 public:
     const FeatureType featureType;
+    const optional<FeatureIdentifier> featureIdentifier;
     const PropertyAccessor propertyAccessor;
 
     bool operator()(const NullFilter&) const {
@@ -123,9 +124,19 @@ public:
 
 private:
     optional<Value> getValue(const std::string& key_) const {
-        return key_ == "$type"
-            ? optional<Value>(uint64_t(featureType))
-            : propertyAccessor(key_);
+        if (key_ == "$type") {
+            return optional<Value>(uint64_t(featureType));
+        } else if (key_ == "$id") {
+            if (featureIdentifier) {
+                return FeatureIdentifier::visit(*featureIdentifier, [] (auto id) {
+                    return Value(std::move(id));
+                });
+            } else {
+                return optional<Value>();
+            }
+        } else {
+            return propertyAccessor(key_);
+        }
     }
 
     template <class Op>
@@ -183,9 +194,18 @@ private:
     }
 };
 
+inline bool Filter::operator()(const Feature& feature) const {
+    return operator()(apply_visitor(ToFeatureType(), feature.geometry), feature.id, [&] (const std::string& key) -> optional<Value> {
+        auto it = feature.properties.find(key);
+        if (it == feature.properties.end())
+            return {};
+        return it->second;
+    });
+}
+
 template <class PropertyAccessor>
-bool Filter::operator()(FeatureType type, PropertyAccessor accessor) const {
-    return FilterBase::visit(*this, FilterEvaluator<PropertyAccessor> { type, accessor });
+bool Filter::operator()(FeatureType type, optional<FeatureIdentifier> id, PropertyAccessor accessor) const {
+    return FilterBase::visit(*this, FilterEvaluator<PropertyAccessor> { type, id, accessor });
 }
 
 } // namespace style
diff --git a/include/mbgl/util/feature.hpp b/include/mbgl/util/feature.hpp
index 7c5c8d7625..b72aa15ddd 100644
--- a/include/mbgl/util/feature.hpp
+++ b/include/mbgl/util/feature.hpp
@@ -10,10 +10,6 @@ using Value = mapbox::geometry::value;
 using NullValue = mapbox::geometry::null_value_t;
 using PropertyMap = mapbox::geometry::property_map;
 using FeatureIdentifier = mapbox::geometry::identifier;
-class Feature : public mapbox::geometry::feature<double> {
-public:
-    Feature(geometry_type&& geometry_)
-        : mapbox::geometry::feature<double> { std::move(geometry_) } {}
-};
+using Feature = mapbox::geometry::feature<double>;
 
 } // namespace mbgl
diff --git a/platform/darwin/test/MGLFeatureTests.mm b/platform/darwin/test/MGLFeatureTests.mm
index 13ad8759b0..1b1722f172 100644
--- a/platform/darwin/test/MGLFeatureTests.mm
+++ b/platform/darwin/test/MGLFeatureTests.mm
@@ -1,6 +1,7 @@
 #import <Mapbox/Mapbox.h>
 #import <XCTest/XCTest.h>
 
+#import <mbgl/util/geometry.hpp>
 #import "../../darwin/src/MGLFeature_Private.h"
 
 @interface MGLFeatureTests : XCTestCase
@@ -12,16 +13,16 @@
 - (void)testGeometryConversion {
     std::vector<mbgl::Feature> features;
     
-    mapbox::geometry::point<double> point = { -90.066667, 29.95 };
-    features.emplace_back(point);
+    mbgl::Point<double> point = { -90.066667, 29.95 };
+    features.push_back(mbgl::Feature { point });
     
-    mapbox::geometry::line_string<double> lineString = {
+    mbgl::LineString<double> lineString = {
         { -84.516667, 39.1 },
         { -90.066667, 29.95 },
     };
-    features.emplace_back(lineString);
+    features.push_back(mbgl::Feature { lineString });
     
-    mapbox::geometry::polygon<double> polygon = {
+    mbgl::Polygon<double> polygon = {
         {
             { 1, 1 },
             { 4, 1 },
@@ -35,7 +36,7 @@
             { 2, 3 },
         },
     };
-    features.emplace_back(polygon);
+    features.push_back(mbgl::Feature { polygon });
     
     NS_ARRAY_OF(MGLShape <MGLFeature> *) *shapes = MGLFeaturesFromMBGLFeatures(features);
     XCTAssertEqual(shapes.count, 3, @"All features should be converted into shapes");
@@ -87,8 +88,8 @@
 - (void)testPropertyConversion {
     std::vector<mbgl::Feature> features;
     
-    mapbox::geometry::point<double> point = { -90.066667, 29.95 };
-    mbgl::Feature pointFeature(point);
+    mbgl::Point<double> point = { -90.066667, 29.95 };
+    mbgl::Feature pointFeature { point };
     pointFeature.id = { UINT64_MAX };
     pointFeature.properties["null"] = nullptr;
     pointFeature.properties["bool"] = true;
diff --git a/src/mbgl/annotation/shape_annotation_impl.cpp b/src/mbgl/annotation/shape_annotation_impl.cpp
index 620b1acc76..2236f179b9 100644
--- a/src/mbgl/annotation/shape_annotation_impl.cpp
+++ b/src/mbgl/annotation/shape_annotation_impl.cpp
@@ -24,7 +24,7 @@ void ShapeAnnotationImpl::updateTileData(const CanonicalTileID& tileID, Annotati
     if (!shapeTiler) {
         mapbox::geometry::feature_collection<double> features;
         features.emplace_back(ShapeAnnotationGeometry::visit(geometry(), [] (auto&& geom) {
-            return Feature(std::move(geom));
+            return Feature { std::move(geom) };
         }));
         mapbox::geojsonvt::Options options;
         options.maxZoom = util::clamp<uint8_t>(maxZoom, 0, 18);
diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp
index f48a98eaac..008372b2bf 100644
--- a/src/mbgl/renderer/symbol_bucket.cpp
+++ b/src/mbgl/renderer/symbol_bucket.cpp
@@ -119,7 +119,7 @@ void SymbolBucket::parseFeatures(const GeometryTileLayer& layer, const Filter& f
     const GLsizei featureCount = static_cast<GLsizei>(layer.featureCount());
     for (GLsizei i = 0; i < featureCount; i++) {
         auto feature = layer.getFeature(i);
-        if (!filter(feature->getType(), [&] (const auto& key) { return feature->getValue(key); }))
+        if (!filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); }))
             continue;
 
         SymbolFeature ft;
diff --git a/src/mbgl/style/bucket_parameters.cpp b/src/mbgl/style/bucket_parameters.cpp
index 5bb08af873..64f53babcd 100644
--- a/src/mbgl/style/bucket_parameters.cpp
+++ b/src/mbgl/style/bucket_parameters.cpp
@@ -10,7 +10,7 @@ void BucketParameters::eachFilteredFeature(const Filter& filter,
     auto name = layer.getName();
     for (std::size_t i = 0; !cancelled() && i < layer.featureCount(); i++) {
         auto feature = layer.getFeature(i);
-        if (!filter(feature->getType(), [&] (const auto& key) { return feature->getValue(key); }))
+        if (!filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); }))
             continue;
         function(*feature, i, name);
     }
diff --git a/test/style/filter.cpp b/test/style/filter.cpp
index 53504d84ae..b49f424a39 100644
--- a/test/style/filter.cpp
+++ b/test/style/filter.cpp
@@ -1,4 +1,6 @@
 #include <mbgl/test/util.hpp>
+#include <mbgl/util/feature.hpp>
+#include <mbgl/util/geometry.hpp>
 
 #include <mbgl/style/filter.hpp>
 #include <mbgl/style/filter_evaluator.hpp>
@@ -17,97 +19,107 @@ Filter parse(const char * expression) {
     return *conversion::convert<Filter>(doc);
 }
 
-bool evaluate(const Filter& filter, const PropertyMap& properties, FeatureType type = FeatureType::Unknown) {
-    return filter(type, [&] (const std::string& key) -> optional<Value> {
-        auto it = properties.find(key);
-        if (it == properties.end())
-            return {};
-        return it->second;
-    });
+Feature feature(const PropertyMap& properties, const Geometry<double>& geometry = Point<double>()) {
+    Feature result { geometry };
+    result.properties = properties;
+    return result;
 }
 
 TEST(Filter, EqualsString) {
     Filter f = parse("[\"==\", \"foo\", \"bar\"]");
-    ASSERT_TRUE(evaluate(f, {{ "foo", std::string("bar") }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", std::string("baz") }}));
+    ASSERT_TRUE(f(feature({{ "foo", std::string("bar") }})));
+    ASSERT_FALSE(f(feature({{ "foo", std::string("baz") }})));
 }
 
 TEST(Filter, EqualsNumber) {
     Filter f = parse("[\"==\", \"foo\", 0]");
-    ASSERT_TRUE(evaluate(f, {{ "foo", int64_t(0) }}));
-    ASSERT_TRUE(evaluate(f, {{ "foo", uint64_t(0) }}));
-    ASSERT_TRUE(evaluate(f, {{ "foo", double(0) }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", int64_t(1) }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", uint64_t(1) }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", double(1) }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", std::string("0") }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", false }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", true }}));
-    ASSERT_FALSE(evaluate(f, {{ "foo", nullptr }}));
-    ASSERT_FALSE(evaluate(f, {{}}));
+    ASSERT_TRUE(f(feature({{ "foo", int64_t(0) }})));
+    ASSERT_TRUE(f(feature({{ "foo", uint64_t(0) }})));
+    ASSERT_TRUE(f(feature({{ "foo", double(0) }})));
+    ASSERT_FALSE(f(feature({{ "foo", int64_t(1) }})));
+    ASSERT_FALSE(f(feature({{ "foo", uint64_t(1) }})));
+    ASSERT_FALSE(f(feature({{ "foo", double(1) }})));
+    ASSERT_FALSE(f(feature({{ "foo", std::string("0") }})));
+    ASSERT_FALSE(f(feature({{ "foo", false }})));
+    ASSERT_FALSE(f(feature({{ "foo", true }})));
+    ASSERT_FALSE(f(feature({{ "foo", nullptr }})));
+    ASSERT_FALSE(f(feature({{}})));
 }
 
 TEST(Filter, EqualsType) {
     Filter f = parse("[\"==\", \"$type\", \"LineString\"]");
-    ASSERT_FALSE(evaluate(f, {{}}, FeatureType::Point));
-    ASSERT_TRUE(evaluate(f, {{}}, FeatureType::LineString));
+    ASSERT_FALSE(f(feature({{}}, Point<double>())));
+    ASSERT_TRUE(f(feature({{}}, LineString<double>())));
 }
 
 TEST(Filter, InType) {
     Filter f = parse("[\"in\", \"$type\", \"LineString\", \"Polygon\"]");
-    ASSERT_FALSE(evaluate(f, {{}}, FeatureType::Point));
-    ASSERT_TRUE(evaluate(f, {{}}, FeatureType::LineString));
-    ASSERT_TRUE(evaluate(f, {{}}, FeatureType::Polygon));
+    ASSERT_FALSE(f(feature({{}}, Point<double>())));
+    ASSERT_TRUE(f(feature({{}}, LineString<double>())));
+    ASSERT_TRUE(f(feature({{}}, Polygon<double>())));
 }
 
 TEST(Filter, Any) {
-    ASSERT_FALSE(evaluate(parse("[\"any\"]"), {{}}));
-    ASSERT_TRUE(evaluate(parse("[\"any\", [\"==\", \"foo\", 1]]"),
-                         {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_FALSE(evaluate(parse("[\"any\", [\"==\", \"foo\", 0]]"),
-                          {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_TRUE(evaluate(parse("[\"any\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]"),
-                         {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_FALSE(parse("[\"any\"]")(feature({{}})));
+    ASSERT_TRUE(parse("[\"any\", [\"==\", \"foo\", 1]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_FALSE(parse("[\"any\", [\"==\", \"foo\", 0]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_TRUE(parse("[\"any\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
 }
 
 TEST(Filter, All) {
-    ASSERT_TRUE(evaluate(parse("[\"all\"]"), {{}}));
-    ASSERT_TRUE(evaluate(parse("[\"all\", [\"==\", \"foo\", 1]]"),
-                         {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_FALSE(evaluate(parse("[\"all\", [\"==\", \"foo\", 0]]"),
-                          {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_FALSE(evaluate(parse("[\"all\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]"),
-                          {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_TRUE(parse("[\"all\"]")(feature({{}})));
+    ASSERT_TRUE(parse("[\"all\", [\"==\", \"foo\", 1]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_FALSE(parse("[\"all\", [\"==\", \"foo\", 0]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_FALSE(parse("[\"all\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
 }
 
 TEST(Filter, None) {
-    ASSERT_TRUE(evaluate(parse("[\"none\"]"), {{}}));
-    ASSERT_FALSE(evaluate(parse("[\"none\", [\"==\", \"foo\", 1]]"),
-                          {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_TRUE(evaluate(parse("[\"none\", [\"==\", \"foo\", 0]]"),
-                         {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_FALSE(evaluate(parse("[\"none\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]"),
-                          {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_TRUE(parse("[\"none\"]")(feature({{}})));
+    ASSERT_FALSE(parse("[\"none\", [\"==\", \"foo\", 1]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_TRUE(parse("[\"none\", [\"==\", \"foo\", 0]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_FALSE(parse("[\"none\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]")(
+                         feature({{ std::string("foo"), int64_t(1) }})));
 }
 
 TEST(Filter, Has) {
-    ASSERT_TRUE(evaluate(parse("[\"has\", \"foo\"]"),
-                          {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_TRUE(evaluate(parse("[\"has\", \"foo\"]"),
-                          {{ std::string("foo"), int64_t(0) }}));
-    ASSERT_TRUE(evaluate(parse("[\"has\", \"foo\"]"),
-                          {{ std::string("foo"), false }}));
-    ASSERT_FALSE(evaluate(parse("[\"has\", \"foo\"]"),
-                          {{}}));
+    ASSERT_TRUE(parse("[\"has\", \"foo\"]")(
+                          feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_TRUE(parse("[\"has\", \"foo\"]")(
+                          feature({{ std::string("foo"), int64_t(0) }})));
+    ASSERT_TRUE(parse("[\"has\", \"foo\"]")(
+                          feature({{ std::string("foo"), false }})));
+    ASSERT_FALSE(parse("[\"has\", \"foo\"]")(
+                          feature({{}})));
 }
 
 TEST(Filter, NotHas) {
-    ASSERT_FALSE(evaluate(parse("[\"!has\", \"foo\"]"),
-                          {{ std::string("foo"), int64_t(1) }}));
-    ASSERT_FALSE(evaluate(parse("[\"!has\", \"foo\"]"),
-                          {{ std::string("foo"), int64_t(0) }}));
-    ASSERT_FALSE(evaluate(parse("[\"!has\", \"foo\"]"),
-                          {{ std::string("foo"), false }}));
-    ASSERT_TRUE(evaluate(parse("[\"!has\", \"foo\"]"),
-                          {{}}));
+    ASSERT_FALSE(parse("[\"!has\", \"foo\"]")(
+                          feature({{ std::string("foo"), int64_t(1) }})));
+    ASSERT_FALSE(parse("[\"!has\", \"foo\"]")(
+                          feature({{ std::string("foo"), int64_t(0) }})));
+    ASSERT_FALSE(parse("[\"!has\", \"foo\"]")(
+                          feature({{ std::string("foo"), false }})));
+    ASSERT_TRUE(parse("[\"!has\", \"foo\"]")(
+                          feature({{}})));
+}
+
+TEST(Filter, ID) {
+    Feature feature1 { Point<double>() };
+    feature1.id = { 1234 };
+
+    ASSERT_TRUE(parse("[\"==\", \"$id\", 1234]")(feature1));
+    ASSERT_FALSE(parse("[\"==\", \"$id\", \"1234\"]")(feature1));
+
+    Feature feature2 { Point<double>() };
+    feature2.properties["id"] = { 1234 };
+
+    ASSERT_FALSE(parse("[\"==\", \"$id\", 1234]")(feature2));
 }