Handle duplicate keys in getValue

include/mapbox/vector_tile.hpp

@@ -40,23 +40,7 @@ class feature {
     feature(protozero::data_view const&, layer const&);
 
     GeomType getType() const { return type; }
-    /**
-     * Retrieve the value associated with a given key from the feature.
-     *
-     * @param key The key used to look up the corresponding value.
-     * @param warning  A pointer to a string that may be used to record any warnings that
- *                     occur during the lookup process.
-     *                 The caller is responsible for managing the memory of this string.
-     * @return The value associated with the specified key, or a null value if the key is not found.
-     *
-     * Note: If the lookup process encounters a duplicate key in the feature, the function will
-     *       return the value in the `values` set to which the associated tag index points to, and 
-     *       will append a message to the `warning` string (if provided) to alert the caller to the
-     *       presence of the duplicate key. 
-     *       The caller should ensure that the `warning` string is properly initialized
-     *       and cleaned up after use.
-     */
-    mapbox::feature::value getValue(std::string const&, std::string* warning = nullptr) const;
+    mapbox::feature::value getValue(std::string const&) const;
     properties_type getProperties() const;
     mapbox::feature::identifier const& getID() const;
     std::uint32_t getExtent() const;
@@ -88,7 +72,7 @@ class layer {
     std::string name;
     std::uint32_t version;
     std::uint32_t extent;
-    std::multimap<std::string, std::uint32_t> keysMap;
+    std::map<std::string, std::uint32_t> keysMap;
     std::vector<std::reference_wrapper<const std::string>> keys;
     std::vector<protozero::data_view> values;
     std::vector<protozero::data_view> features;
@@ -169,13 +153,7 @@ inline feature::feature(protozero::data_view const& feature_view, layer const& l
     }
 }
 
-inline mapbox::feature::value feature::getValue(const std::string& key, std::string* warning ) const {
-    const auto key_range = layer_.keysMap.equal_range(key);
-    const auto key_count = std::distance(key_range.first, key_range.second) ;
-    if (key_count < 1) {
-        return mapbox::feature::null_value;
-    }
-
+inline mapbox::feature::value feature::getValue(const std::string& key) const {
     const auto values_count = layer_.values.size();
     auto start_itr = tags_iter.begin();
     const auto end_itr = tags_iter.end();
@@ -191,19 +169,7 @@ inline mapbox::feature::value feature::getValue(const std::string& key, std::str
             throw std::runtime_error("feature referenced out of range value");
         }
 
-        bool key_found = false;
-        for (auto i = key_range.first; i != key_range.second; ++i) {
-            if (i->second == tag_key) {
-                key_found = true;
-                break;
-            }
-        }
-
-        if (key_found) {
-            // Continue process with case when same keys having multiple tag ids.
-            if (key_count > 1 && warning) {
-                *warning = std::string("duplicate keys with different tag ids are found");
-            }
+        if (layer_.keys.at(tag_key).get() == key) {
             return parseValue(layer_.values[tag_val]);
         }
     }
@@ -427,8 +393,8 @@ inline layer::layer(protozero::data_view const& layer_view) :
             {
                 // We want to keep the keys in the order of the vector tile
                 // https://github.com/mapbox/mapbox-gl-native/pull/5183
-                auto iter = keysMap.emplace(layer_pbf.get_string(), keys.size());
-                keys.emplace_back(std::reference_wrapper<const std::string>(iter->first));
+                auto iter = keysMap.emplace(layer_pbf.get_string(), keysMap.size());
+                keys.emplace_back(std::reference_wrapper<const std::string>(iter.first->first));
             }
             break;
         case LayerType::VALUES:

include/mapbox/vector_tile/version.hpp

@@ -7,10 +7,10 @@
 #define VECTOR_TILE_VERSION_MINOR 0
 
 /// The patch number
-#define VECTOR_TILE_VERSION_PATCH 4
+#define VECTOR_TILE_VERSION_PATCH 5
 
 /// The complete version number
 #define VECTOR_TILE_VERSION_CODE (VECTOR_TILE_VERSION_MAJOR * 10000 + VECTOR_TILE_VERSION_MINOR * 100 + VECTOR_TILE_VERSION_PATCH)
 
 /// Version number as string
-#define VECTOR_TILE_VERSION_STRING "1.0.4"
+#define VECTOR_TILE_VERSION_STRING "1.0.5"

test/test047.mvt

@@ -0,0 +1,4 @@
+7x
+hello



"	2"typetype"
+park"
+lake(�

test/unit/tags.test.cpp

@@ -4,8 +4,8 @@
 #include <catch.hpp>
 
 TEST_CASE( "Version constant" ) {
-    CHECK(std::string(VECTOR_TILE_VERSION_STRING) == std::string("1.0.4"));
-    CHECK(VECTOR_TILE_VERSION_CODE == 10004);
+    CHECK(std::string(VECTOR_TILE_VERSION_STRING) == std::string("1.0.5"));
+    CHECK(VECTOR_TILE_VERSION_CODE == 10005);
 }
 
 TEST_CASE( "Protobuf Tag Constants" ) {

test/unit/vector_tile.test.cpp

@@ -129,28 +129,12 @@ TEST_CASE( "Prevent underflow in case of LineTo with 0 command count" ) {
     }
 }
 
-TEST_CASE( "Allow multiple keys mapping to different tag ids" ) {
-    // duplicate key 'hello' and duplicate value 'world'
-    std::string buffer = open_tile("test/duplicate-keys-values.mvt");
+TEST_CASE( "Handle layer with duplicate keys" ) {
+    std::string buffer = open_tile("test/test047.mvt");
     mapbox::vector_tile::buffer tile(buffer);
-    auto const layer_names = tile.layerNames();
-    REQUIRE(layer_names.size() == 1);
-    REQUIRE(layer_names[0] == "duplicates");
-    auto const layer = tile.getLayer("duplicates");
-    REQUIRE(layer.featureCount() == 1);
-    auto const feature = mapbox::vector_tile::feature(layer.getFeature(0), layer);
-    REQUIRE(feature.getType() == mapbox::vector_tile::GeomType::POLYGON);
-
-    std::string error;
-    auto const val = feature.getValue("hello", &error);
-    REQUIRE(!error.empty());
-    REQUIRE(error == "duplicate keys with different tag ids are found");
-    REQUIRE(val.is<std::string>());
-    REQUIRE(val.get<std::string>() == "world");
-    error.clear();
-    REQUIRE(error.empty());
-    auto const val1 = feature.getValue("unique", &error);
-    REQUIRE(error.empty());
-    REQUIRE(val1.is<std::string>());
-    REQUIRE(val1.get<std::string>() == "single_value");
-}
+    auto const layer = tile.getLayer("hello");
+    auto const feature = mapbox::vector_tile::feature(layer.getFeature(0),layer);
+    auto opt_val = feature.getValue("type"); \
+    REQUIRE(opt_val.is<std::string>()); \
+    REQUIRE(opt_val.get<std::string>() == "lake"); \
+}