From 0be86db3bfafea6d26af0638d9e5054c38699062 Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <alex@maps.me>
Date: Thu, 26 Nov 2015 22:00:16 -0800
Subject: [PATCH 1/6] Renamed generator metadata test.

---
 generator/generator_tests/generator_tests.pro                   | 2 +-
 .../{metadata_test.cpp => metadata_parser_test.cpp}             | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename generator/generator_tests/{metadata_test.cpp => metadata_parser_test.cpp} (100%)

diff --git a/generator/generator_tests/generator_tests.pro b/generator/generator_tests/generator_tests.pro
index 9a0a9ff059..adda55e8a3 100644
--- a/generator/generator_tests/generator_tests.pro
+++ b/generator/generator_tests/generator_tests.pro
@@ -22,7 +22,7 @@ SOURCES += \
     coasts_test.cpp \
     feature_builder_test.cpp \
     feature_merger_test.cpp \
-    metadata_test.cpp \
+    metadata_parser_test.cpp \
     osm_id_test.cpp \
     osm_o5m_source_test.cpp \
     osm_type_test.cpp \
diff --git a/generator/generator_tests/metadata_test.cpp b/generator/generator_tests/metadata_parser_test.cpp
similarity index 100%
rename from generator/generator_tests/metadata_test.cpp
rename to generator/generator_tests/metadata_parser_test.cpp

From 1edf6fda2099c97d1643d7cb581a68e79d6287e4 Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <alex@maps.me>
Date: Thu, 26 Nov 2015 23:10:46 -0800
Subject: [PATCH 2/6] Moved feature::Metadata class tests into indexer_tests,
 where this class was defined.

---
 .../generator_tests/metadata_parser_test.cpp  | 54 -------------------
 .../indexer_tests/feature_metadata_test.cpp   | 52 ++++++++++++++++++
 indexer/indexer_tests/indexer_tests.pro       |  1 +
 3 files changed, 53 insertions(+), 54 deletions(-)
 create mode 100644 indexer/indexer_tests/feature_metadata_test.cpp

diff --git a/generator/generator_tests/metadata_parser_test.cpp b/generator/generator_tests/metadata_parser_test.cpp
index 1666752024..7176760147 100644
--- a/generator/generator_tests/metadata_parser_test.cpp
+++ b/generator/generator_tests/metadata_parser_test.cpp
@@ -185,57 +185,3 @@ UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
   p(kWikiKey, "http://ru.google.com/wiki/wutlol");
   TEST(params.GetMetadata().Empty(), ("Not a wikipedia site."));
 }
-
-UNIT_TEST(Metadata_ReadWrite_Intermediate)
-{
-  FeatureParams params;
-  FeatureParams params_test;
-  MetadataTagProcessor p(params);
-
-  vector<char> buffer;
-  MemWriter<vector<char> > writer(buffer);
-
-  p("stars", "3");
-  p("phone", "+123456789");
-  p("opening_hours", "24/7");
-  p("cuisine", "regional");
-  p("operator", "Unused");
-  params.GetMetadata().Serialize(writer);
-
-  MemReader reader(buffer.data(), buffer.size());
-  ReaderSource<MemReader> src(reader);
-  params_test.GetMetadata().Deserialize(src);
-
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_STARS), "3", ())
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_PHONE_NUMBER), "+123456789", ())
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_OPEN_HOURS), "24/7", ())
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_CUISINE), "regional", ())
-  TEST(params_test.GetMetadata().Get(feature::Metadata::FMD_OPERATOR).empty(), ())
-}
-
-UNIT_TEST(Metadata_ReadWrite_MWM)
-{
-  FeatureParams params;
-  FeatureParams params_test;
-  MetadataTagProcessor p(params);
-
-  vector<char> buffer;
-  MemWriter<vector<char> > writer(buffer);
-
-  p("stars", "3");
-  p("phone", "+123456789");
-  p("opening_hours", "24/7");
-  p("cuisine", "regional");
-  p("operator", "Unused");
-  params.GetMetadata().SerializeToMWM(writer);
-
-  MemReader reader(buffer.data(), buffer.size());
-  ReaderSource<MemReader> src(reader);
-  params_test.GetMetadata().DeserializeFromMWM(src);
-
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_STARS), "3", ())
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_PHONE_NUMBER), "+123456789", ())
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_OPEN_HOURS), "24/7", ())
-  TEST_EQUAL(params_test.GetMetadata().Get(feature::Metadata::FMD_CUISINE), "regional", ())
-  TEST(params_test.GetMetadata().Get(feature::Metadata::FMD_OPERATOR).empty(), ())
-}
diff --git a/indexer/indexer_tests/feature_metadata_test.cpp b/indexer/indexer_tests/feature_metadata_test.cpp
new file mode 100644
index 0000000000..b61a17df5c
--- /dev/null
+++ b/indexer/indexer_tests/feature_metadata_test.cpp
@@ -0,0 +1,52 @@
+#include "testing/testing.hpp"
+
+#include "indexer/feature_meta.hpp"
+
+#include "coding/reader.hpp"
+#include "coding/writer.hpp"
+
+#include "std/map.hpp"
+
+using feature::Metadata;
+
+UNIT_TEST(Feature_Serialization)
+{
+  Metadata original;
+  for (auto const & value : kPairs)
+    original.Set(value.first, value.second);
+  TEST_EQUAL(original.Size(), kPairs.size(), ());
+
+  {
+    Metadata serialized;
+    vector<char> buffer;
+    MemWriter<vector<char> > writer(buffer);
+    original.Serialize(writer);
+
+    MemReader reader(buffer.data(), buffer.size());
+    ReaderSource<MemReader> src(reader);
+    serialized.Deserialize(src);
+
+    for (auto const & value : kPairs)
+      TEST_EQUAL(serialized.Get(value.first), value.second, ());
+    TEST_EQUAL(serialized.Get(Metadata::FMD_OPERATOR), "", ());
+    TEST_EQUAL(serialized.Size(), kPairs.size(), ());
+  }
+
+  {
+    Metadata serialized;
+    vector<char> buffer;
+    MemWriter<vector<char> > writer(buffer);
+    // Here is the difference.
+    original.SerializeToMWM(writer);
+
+    MemReader reader(buffer.data(), buffer.size());
+    ReaderSource<MemReader> src(reader);
+    // Here is another difference.
+    serialized.DeserializeFromMWM(src);
+
+    for (auto const & value : kPairs)
+      TEST_EQUAL(serialized.Get(value.first), value.second, ());
+    TEST_EQUAL(serialized.Get(Metadata::FMD_OPERATOR), "", ());
+    TEST_EQUAL(serialized.Size(), kPairs.size(), ());
+  }
+}
diff --git a/indexer/indexer_tests/indexer_tests.pro b/indexer/indexer_tests/indexer_tests.pro
index fa2b705834..8ea3baf487 100644
--- a/indexer/indexer_tests/indexer_tests.pro
+++ b/indexer/indexer_tests/indexer_tests.pro
@@ -22,6 +22,7 @@ SOURCES += \
     cell_id_test.cpp \
     checker_test.cpp \
     drules_selector_parser_test.cpp \
+    feature_metadata_test.cpp \
     features_offsets_table_test.cpp \
     geometry_coding_test.cpp \
     geometry_serialization_test.cpp \

From afae5a24f866080b196f53c0d8641fbfde8e023e Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <alex@maps.me>
Date: Thu, 26 Nov 2015 23:14:16 -0800
Subject: [PATCH 3/6] Minor tests formatting to improve readability.

---
 .../generator_tests/metadata_parser_test.cpp  | 126 +++++++++---------
 1 file changed, 65 insertions(+), 61 deletions(-)

diff --git a/generator/generator_tests/metadata_parser_test.cpp b/generator/generator_tests/metadata_parser_test.cpp
index 7176760147..35d1d56026 100644
--- a/generator/generator_tests/metadata_parser_test.cpp
+++ b/generator/generator_tests/metadata_parser_test.cpp
@@ -6,64 +6,67 @@
 #include "coding/writer.hpp"
 #include "coding/reader.hpp"
 
+using feature::Metadata;
+
 UNIT_TEST(Metadata_ValidateAndFormat_stars)
 {
   FeatureParams params;
   MetadataTagProcessor p(params);
+  Metadata & md = params.GetMetadata();
 
   // ignore incorrect values
   p("stars", "0");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
   p("stars", "-1");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
   p("stars", "aasdasdas");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
   p("stars", "8");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
   p("stars", "10");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
   p("stars", "910");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
   p("stars", "100");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
 
   // check correct values
   p("stars", "1");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "1", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "1", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "2");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "2", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "2", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "3");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "3", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "3", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "4");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "4", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "4", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "5");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "5", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "5", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "6");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "6", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "6", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "7");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "7", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "7", ())
+  md.Drop(Metadata::FMD_STARS);
 
   // check almost correct values
   p("stars", "4+");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "4", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "4", ())
+  md.Drop(Metadata::FMD_STARS);
 
   p("stars", "5s");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_STARS), "5", ())
-  params.GetMetadata().Drop(feature::Metadata::FMD_STARS);
+  TEST_EQUAL(md.Get(Metadata::FMD_STARS), "5", ())
+  md.Drop(Metadata::FMD_STARS);
 
 }
 
@@ -76,26 +79,27 @@ UNIT_TEST(Metadata_ValidateAndFormat_operator)
 
   FeatureParams params;
   MetadataTagProcessor p(params);
+  Metadata & md = params.GetMetadata();
 
   // ignore tag 'operator' if feature have inappropriate type
   p("operator", "Some");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
 
   params.SetType(type_atm);
   p("operator", "Some");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_OPERATOR), "Some", ());
-  params.GetMetadata().Drop(feature::Metadata::FMD_OPERATOR);
+  TEST_EQUAL(md.Get(Metadata::FMD_OPERATOR), "Some", ());
+  md.Drop(Metadata::FMD_OPERATOR);
 
   params.SetType(type_fuel);
   p("operator", "Some");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_OPERATOR), "Some", ());
-  params.GetMetadata().Drop(feature::Metadata::FMD_OPERATOR);
+  TEST_EQUAL(md.Get(Metadata::FMD_OPERATOR), "Some", ());
+  md.Drop(Metadata::FMD_OPERATOR);
 
   params.SetType(type_atm);
   params.AddType(type_fuel);
   p("operator", "Some");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_OPERATOR), "Some", ());
-  params.GetMetadata().Drop(feature::Metadata::FMD_OPERATOR);
+  TEST_EQUAL(md.Get(Metadata::FMD_OPERATOR), "Some", ());
+  md.Drop(Metadata::FMD_OPERATOR);
 }
 
 UNIT_TEST(Metadata_ValidateAndFormat_ele)
@@ -106,82 +110,82 @@ UNIT_TEST(Metadata_ValidateAndFormat_ele)
 
   FeatureParams params;
   MetadataTagProcessor p(params);
+  Metadata & md = params.GetMetadata();
 
   // ignore tag 'operator' if feature have inappropriate type
   p("ele", "123");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
 
   params.SetType(type_peak);
   p("ele", "0");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
 
   params.SetType(type_peak);
   p("ele", "0,0000");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
 
   params.SetType(type_peak);
   p("ele", "0.0");
-  TEST(params.GetMetadata().Empty(), ());
+  TEST(md.Empty(), ());
 
   params.SetType(type_peak);
   p("ele", "123");
-  TEST_EQUAL(params.GetMetadata().Get(feature::Metadata::FMD_ELE), "123", ());
-  params.GetMetadata().Drop(feature::Metadata::FMD_ELE);
+  TEST_EQUAL(md.Get(Metadata::FMD_ELE), "123", ());
+  md.Drop(Metadata::FMD_ELE);
 }
 
 UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
 {
-  using feature::Metadata;
-
   char const * kWikiKey = "wikipedia";
 
   FeatureParams params;
   MetadataTagProcessor p(params);
+  Metadata & md = params.GetMetadata();
 
   p(kWikiKey, "en:Bad %20Data");
-  TEST_EQUAL(params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA), "en:Bad %20Data", ());
-  TEST_EQUAL(params.GetMetadata().GetWikiURL(), "https://en.m.wikipedia.org/wiki/Bad_%2520Data", ());
-  params.GetMetadata().Drop(Metadata::FMD_WIKIPEDIA);
+  TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "en:Bad %20Data", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://en.m.wikipedia.org/wiki/Bad_%2520Data", ());
+  md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "ru:Тест_with % sign");
-  TEST_EQUAL(params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA), "ru:Тест with % sign", ());
-  TEST_EQUAL(params.GetMetadata().GetWikiURL(), "https://ru.m.wikipedia.org/wiki/Тест_with_%25_sign", ());
-  params.GetMetadata().Drop(Metadata::FMD_WIKIPEDIA);
+  TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "ru:Тест with % sign", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://ru.m.wikipedia.org/wiki/Тест_with_%25_sign", ());
+  md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "https://be-tarask.wikipedia.org/wiki/Вялікае_Княства_Літоўскае");
-  TEST_EQUAL(params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA), "be-tarask:Вялікае Княства Літоўскае", ());
-  TEST_EQUAL(params.GetMetadata().GetWikiURL(), "https://be-tarask.m.wikipedia.org/wiki/Вялікае_Княства_Літоўскае", ());
-  params.GetMetadata().Drop(Metadata::FMD_WIKIPEDIA);
+  TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "be-tarask:Вялікае Княства Літоўскае", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://be-tarask.m.wikipedia.org/wiki/Вялікае_Княства_Літоўскае", ());
+  md.Drop(Metadata::FMD_WIKIPEDIA);
 
   // Final link points to https and mobile version.
   p(kWikiKey, "http://en.wikipedia.org/wiki/A");
-  TEST_EQUAL(params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA), "en:A", ());
-  TEST_EQUAL(params.GetMetadata().GetWikiURL(), "https://en.m.wikipedia.org/wiki/A", ());
-  params.GetMetadata().Drop(Metadata::FMD_WIKIPEDIA);
+  TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "en:A", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://en.m.wikipedia.org/wiki/A", ());
+  md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "invalid_input_without_language_and_colon");
-  TEST(params.GetMetadata().Empty(), (params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA)));
+  TEST(md.Empty(), (md.Get(Metadata::FMD_WIKIPEDIA)));
 
   p(kWikiKey, "https://en.wikipedia.org/wiki/");
-  TEST(params.GetMetadata().Empty(), (params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA)));
+  TEST(md.Empty(), (md.Get(Metadata::FMD_WIKIPEDIA)));
 
   p(kWikiKey, "http://wikipedia.org/wiki/Article");
-  TEST(params.GetMetadata().Empty(), (params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA)));
+  TEST(md.Empty(), (md.Get(Metadata::FMD_WIKIPEDIA)));
 
   p(kWikiKey, "http://somesite.org");
-  TEST(params.GetMetadata().Empty(), (params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA)));
+  TEST(md.Empty(), (md.Get(Metadata::FMD_WIKIPEDIA)));
 
   p(kWikiKey, "http://www.spamsitewithaslash.com/");
-  TEST(params.GetMetadata().Empty(), (params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA)));
+  TEST(md.Empty(), (md.Get(Metadata::FMD_WIKIPEDIA)));
 
   p(kWikiKey, "http://.wikipedia.org/wiki/Article");
-  TEST(params.GetMetadata().Empty(), (params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA)));
+  TEST(md.Empty(), (md.Get(Metadata::FMD_WIKIPEDIA)));
 
   // Ignore incorrect prefixes.
   p(kWikiKey, "ht.tps://en.wikipedia.org/wiki/Whuh");
-  TEST_EQUAL(params.GetMetadata().Get(Metadata::FMD_WIKIPEDIA), "en:Whuh", ());
-  params.GetMetadata().Drop(Metadata::FMD_WIKIPEDIA);
+  TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "en:Whuh", ());
+  md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "http://ru.google.com/wiki/wutlol");
-  TEST(params.GetMetadata().Empty(), ("Not a wikipedia site."));
+  TEST(md.Empty(), ("Not a wikipedia site."));
 }

From b4b10b3e7d2bd804ad43b63268f5df62fa413c1c Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <alex@maps.me>
Date: Thu, 26 Nov 2015 23:16:26 -0800
Subject: [PATCH 4/6] Fixed Metadata interface and implementation, added
 missing unit tests.

---
 generator/osm2meta.hpp                        | 32 ++++++++---------
 indexer/feature.cpp                           |  2 +-
 indexer/feature_meta.hpp                      | 31 ++++++++++------
 .../indexer_tests/feature_metadata_test.cpp   | 36 +++++++++++++++++++
 4 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/generator/osm2meta.hpp b/generator/osm2meta.hpp
index 79efd40fb9..8874dfb7a7 100644
--- a/generator/osm2meta.hpp
+++ b/generator/osm2meta.hpp
@@ -31,97 +31,97 @@ public:
     {
       string const & value = ValidateAndFormat_cuisine(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_CUISINE, value);
+        md.Set(Metadata::FMD_CUISINE, value);
     }
     else if (k == "phone" || k == "contact:phone")
     {
       string const & value = ValidateAndFormat_phone(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_PHONE_NUMBER, value);
+        md.Set(Metadata::FMD_PHONE_NUMBER, value);
     }
     else if (k == "maxspeed")
     {
       string const & value = ValidateAndFormat_maxspeed(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_MAXSPEED, value);
+        md.Set(Metadata::FMD_MAXSPEED, value);
     }
     else if (k == "stars")
     {
       string const & value = ValidateAndFormat_stars(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_STARS, value);
+        md.Set(Metadata::FMD_STARS, value);
     }
     else if (k == "addr:postcode")
     {
       string const & value = ValidateAndFormat_postcode(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_POSTCODE, value);
+        md.Set(Metadata::FMD_POSTCODE, value);
     }
     else if (k == "url")
     {
       string const & value = ValidateAndFormat_url(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_URL, value);
+        md.Set(Metadata::FMD_URL, value);
     }
     else if (k == "website" || k == "contact:website")
     {
       string const & value = ValidateAndFormat_url(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_WEBSITE, value);
+        md.Set(Metadata::FMD_WEBSITE, value);
     }
     else if (k == "operator")
     {
       string const & value = ValidateAndFormat_operator(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_OPERATOR, value);
+        md.Set(Metadata::FMD_OPERATOR, value);
     }
     else if (k == "opening_hours")
     {
       string const & value = ValidateAndFormat_opening_hours(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_OPEN_HOURS, value);
+        md.Set(Metadata::FMD_OPEN_HOURS, value);
     }
     else if (k == "ele")
     {
       string const & value = ValidateAndFormat_ele(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_ELE, value);
+        md.Set(Metadata::FMD_ELE, value);
     }
     else if (k == "turn:lanes")
     {
       string const & value = ValidateAndFormat_turn_lanes(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_TURN_LANES, value);
+        md.Set(Metadata::FMD_TURN_LANES, value);
     }
     else if (k == "turn:lanes:forward")
     {
       string const & value = ValidateAndFormat_turn_lanes_forward(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_TURN_LANES_FORWARD, value);
+        md.Set(Metadata::FMD_TURN_LANES_FORWARD, value);
     }
     else if (k == "turn:lanes:backward")
     {
       string const & value = ValidateAndFormat_turn_lanes_backward(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_TURN_LANES_BACKWARD, value);
+        md.Set(Metadata::FMD_TURN_LANES_BACKWARD, value);
     }
     else if (k == "email" || k == "contact:email")
     {
       string const & value = ValidateAndFormat_email(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_EMAIL, value);
+        md.Set(Metadata::FMD_EMAIL, value);
     }
     else if (k == "wikipedia")
     {
       string const & value = ValidateAndFormat_wikipedia(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_WIKIPEDIA, value);
+        md.Set(Metadata::FMD_WIKIPEDIA, value);
     }
     else if (k == "addr:flats")
     {
       string const & value = ValidateAndFormat_flats(v);
       if (!value.empty())
-        md.Add(Metadata::FMD_FLATS, value);
+        md.Set(Metadata::FMD_FLATS, value);
     }
     return false;
   }
diff --git a/indexer/feature.cpp b/indexer/feature.cpp
index d46961bdf0..434d6e0611 100644
--- a/indexer/feature.cpp
+++ b/indexer/feature.cpp
@@ -144,7 +144,7 @@ void FeatureType::ParseMetadata() const
   m_pLoader->ParseMetadata();
 
   if (HasInternet())
-    m_metadata.Add(Metadata::FMD_INTERNET, "wlan");
+    m_metadata.Set(Metadata::FMD_INTERNET, "wlan");
 
   m_bMetadataParsed = true;
 }
diff --git a/indexer/feature_meta.hpp b/indexer/feature_meta.hpp
index 363f899ded..3a082f32eb 100644
--- a/indexer/feature_meta.hpp
+++ b/indexer/feature_meta.hpp
@@ -42,14 +42,28 @@ namespace feature
 
     static_assert(FMD_COUNT <= 255, "Meta types count is limited to one byte.");
 
-    bool Add(EType type, string const & s)
+    /// Empty value drops (clears) corresponding type.
+    void Set(EType type, string const & value)
     {
-      string & val = m_metadata[type];
-      if (val.empty())
-        val = s;
+      auto found = m_metadata.find(type);
+      if (found == m_metadata.end())
+      {
+        if (!value.empty())
+          m_metadata[type] = value;
+      }
       else
-        val = val + ", " + s;
-      return true;
+      {
+        if (value.empty())
+          m_metadata.erase(found);
+        else
+          found->second = value;
+      }
+    }
+
+    /// Synonym of Set(type, "").
+    void Drop(EType type)
+    {
+      Set(type, "");
     }
 
     string Get(EType type) const
@@ -69,11 +83,6 @@ namespace feature
       return types;
     }
 
-    void Drop(EType type)
-    {
-      m_metadata.erase(type);
-    }
-
     inline bool Empty() const { return m_metadata.empty(); }
     inline size_t Size() const { return m_metadata.size(); }
 
diff --git a/indexer/indexer_tests/feature_metadata_test.cpp b/indexer/indexer_tests/feature_metadata_test.cpp
index b61a17df5c..38647b56ce 100644
--- a/indexer/indexer_tests/feature_metadata_test.cpp
+++ b/indexer/indexer_tests/feature_metadata_test.cpp
@@ -9,6 +9,42 @@
 
 using feature::Metadata;
 
+UNIT_TEST(Feature_Metadata_GetSet)
+{
+  Metadata m;
+  Metadata::EType const type = Metadata::FMD_ELE;
+  // Absent types should return empty values.
+  TEST_EQUAL(m.Get(type), "", ());
+  m.Set(type, "12345");
+  TEST_EQUAL(m.Get(type), "12345", ());
+  TEST_EQUAL(m.Size(), 1, ());
+  // Same types should replace old metadata values.
+  m.Set(type, "5678");
+  TEST_EQUAL(m.Get(type), "5678", ());
+  // Empty values should drop fields.
+  m.Set(type, "");
+  TEST_EQUAL(m.Get(type), "", ());
+  TEST_EQUAL(m.Size(), 0, ());
+  TEST(m.Empty(), ());
+}
+
+static map<Metadata::EType, string> const kPairs = { {Metadata::FMD_ELE, "12345"},
+                                                     {Metadata::FMD_CUISINE, "greek;mediterranean"},
+                                                     {Metadata::FMD_EMAIL, "cool@email.at"} };
+
+UNIT_TEST(Feature_Metadata_PresentTypes)
+{
+  Metadata m;
+  for (auto const & value : kPairs)
+    m.Set(value.first, value.second);
+  TEST_EQUAL(m.Size(), kPairs.size(), ());
+
+  auto const types = m.GetPresentTypes();
+  TEST_EQUAL(types.size(), m.Size(), ());
+  for (auto const & type : types)
+    TEST_EQUAL(m.Get(type), kPairs.find(type)->second, ());
+}
+
 UNIT_TEST(Feature_Serialization)
 {
   Metadata original;

From ce34db1b7788624efae0243879cc4ad997c72f45 Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <alex@maps.me>
Date: Thu, 26 Nov 2015 23:33:17 -0800
Subject: [PATCH 5/6] Mobile and desktop urls for
 feature::Metadata::GetWikiUrl().

---
 .../generator_tests/metadata_parser_test.cpp     | 16 ++++++++++++----
 indexer/feature_meta.cpp                         | 10 +++++++++-
 indexer/indexer_tests/feature_metadata_test.cpp  | 14 ++++++++++++++
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/generator/generator_tests/metadata_parser_test.cpp b/generator/generator_tests/metadata_parser_test.cpp
index 35d1d56026..101350a184 100644
--- a/generator/generator_tests/metadata_parser_test.cpp
+++ b/generator/generator_tests/metadata_parser_test.cpp
@@ -6,6 +6,8 @@
 #include "coding/writer.hpp"
 #include "coding/reader.hpp"
 
+#include "std/target_os.hpp"
+
 using feature::Metadata;
 
 UNIT_TEST(Metadata_ValidateAndFormat_stars)
@@ -134,6 +136,12 @@ UNIT_TEST(Metadata_ValidateAndFormat_ele)
   md.Drop(Metadata::FMD_ELE);
 }
 
+#ifdef OMIM_OS_MOBILE
+  #define WIKIHOST "m.wikipedia.org"
+#else
+  #define WIKIHOST "wikipedia.org"
+#endif
+
 UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
 {
   char const * kWikiKey = "wikipedia";
@@ -144,23 +152,23 @@ UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
 
   p(kWikiKey, "en:Bad %20Data");
   TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "en:Bad %20Data", ());
-  TEST_EQUAL(md.GetWikiURL(), "https://en.m.wikipedia.org/wiki/Bad_%2520Data", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://en." WIKIHOST "/wiki/Bad_%2520Data", ());
   md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "ru:Тест_with % sign");
   TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "ru:Тест with % sign", ());
-  TEST_EQUAL(md.GetWikiURL(), "https://ru.m.wikipedia.org/wiki/Тест_with_%25_sign", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://ru." WIKIHOST "/wiki/Тест_with_%25_sign", ());
   md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "https://be-tarask.wikipedia.org/wiki/Вялікае_Княства_Літоўскае");
   TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "be-tarask:Вялікае Княства Літоўскае", ());
-  TEST_EQUAL(md.GetWikiURL(), "https://be-tarask.m.wikipedia.org/wiki/Вялікае_Княства_Літоўскае", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://be-tarask." WIKIHOST "/wiki/Вялікае_Княства_Літоўскае", ());
   md.Drop(Metadata::FMD_WIKIPEDIA);
 
   // Final link points to https and mobile version.
   p(kWikiKey, "http://en.wikipedia.org/wiki/A");
   TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "en:A", ());
-  TEST_EQUAL(md.GetWikiURL(), "https://en.m.wikipedia.org/wiki/A", ());
+  TEST_EQUAL(md.GetWikiURL(), "https://en." WIKIHOST "/wiki/A", ());
   md.Drop(Metadata::FMD_WIKIPEDIA);
 
   p(kWikiKey, "invalid_input_without_language_and_colon");
diff --git a/indexer/feature_meta.cpp b/indexer/feature_meta.cpp
index cc41944241..79ef153498 100644
--- a/indexer/feature_meta.cpp
+++ b/indexer/feature_meta.cpp
@@ -1,10 +1,18 @@
 #include "indexer/feature_meta.hpp"
 
 #include "std/algorithm.hpp"
+#include "std/target_os.hpp"
 
 namespace feature
 {
 
+static char constexpr const * kBaseWikiUrl =
+#ifdef OMIM_OS_MOBILE
+    ".m.wikipedia.org/wiki/";
+#else
+    ".wikipedia.org/wiki/";
+#endif
+
 string Metadata::GetWikiURL() const
 {
   string v = this->Get(FMD_WIKIPEDIA);
@@ -27,7 +35,7 @@ string Metadata::GetWikiURL() const
   // Trying to avoid redirects by constructing the right link.
   // TODO: Wikipedia article could be opened it a user's language, but need
   // generator level support to check for available article languages first.
-  return "https://" + v.substr(0, colon) + ".m.wikipedia.org/wiki/" + v.substr(colon + 1);
+  return "https://" + v.substr(0, colon) + kBaseWikiUrl + v.substr(colon + 1);
 }
 
 }  // namespace feature
diff --git a/indexer/indexer_tests/feature_metadata_test.cpp b/indexer/indexer_tests/feature_metadata_test.cpp
index 38647b56ce..cc68052d30 100644
--- a/indexer/indexer_tests/feature_metadata_test.cpp
+++ b/indexer/indexer_tests/feature_metadata_test.cpp
@@ -6,6 +6,7 @@
 #include "coding/writer.hpp"
 
 #include "std/map.hpp"
+#include "std/target_os.hpp"
 
 using feature::Metadata;
 
@@ -86,3 +87,16 @@ UNIT_TEST(Feature_Serialization)
     TEST_EQUAL(serialized.Size(), kPairs.size(), ());
   }
 }
+
+UNIT_TEST(Feature_Metadata_GetWikipedia)
+{
+  Metadata m;
+  Metadata::EType const wikiType = Metadata::FMD_WIKIPEDIA;
+  m.Set(wikiType, "en:Article");
+  TEST_EQUAL(m.Get(wikiType), "en:Article", ());
+#ifdef OMIM_OS_MOBILE
+  TEST_EQUAL(m.GetWikiURL(), "https://en.m.wikipedia.org/wiki/Article", ());
+#else
+  TEST_EQUAL(m.GetWikiURL(), "https://en.wikipedia.org/wiki/Article", ());
+#endif
+}

From d20898af55782772333a804717713063c7dfdd54 Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <alex@maps.me>
Date: Tue, 1 Dec 2015 11:38:14 +0300
Subject: [PATCH 6/6] Code review fixes.

---
 .../generator_tests/metadata_parser_test.cpp  | 31 +++++++++-------
 indexer/feature_meta.cpp                      |  5 ++-
 .../indexer_tests/feature_metadata_test.cpp   | 37 +++++++++++--------
 3 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/generator/generator_tests/metadata_parser_test.cpp b/generator/generator_tests/metadata_parser_test.cpp
index 101350a184..9ba9dda939 100644
--- a/generator/generator_tests/metadata_parser_test.cpp
+++ b/generator/generator_tests/metadata_parser_test.cpp
@@ -1,11 +1,14 @@
 #include "testing/testing.hpp"
-#include "base/logging.hpp"
+
+#include "generator/osm2meta.hpp"
 
 #include "indexer/classificator_loader.hpp"
-#include "generator/osm2meta.hpp"
+
 #include "coding/writer.hpp"
 #include "coding/reader.hpp"
 
+#include "base/logging.hpp"
+
 #include "std/target_os.hpp"
 
 using feature::Metadata;
@@ -16,7 +19,7 @@ UNIT_TEST(Metadata_ValidateAndFormat_stars)
   MetadataTagProcessor p(params);
   Metadata & md = params.GetMetadata();
 
-  // ignore incorrect values
+  // Ignore incorrect values.
   p("stars", "0");
   TEST(md.Empty(), ());
   p("stars", "-1");
@@ -32,7 +35,7 @@ UNIT_TEST(Metadata_ValidateAndFormat_stars)
   p("stars", "100");
   TEST(md.Empty(), ());
 
-  // check correct values
+  // Check correct values.
   p("stars", "1");
   TEST_EQUAL(md.Get(Metadata::FMD_STARS), "1", ())
   md.Drop(Metadata::FMD_STARS);
@@ -61,7 +64,7 @@ UNIT_TEST(Metadata_ValidateAndFormat_stars)
   TEST_EQUAL(md.Get(Metadata::FMD_STARS), "7", ())
   md.Drop(Metadata::FMD_STARS);
 
-  // check almost correct values
+  // Check almost correct values.
   p("stars", "4+");
   TEST_EQUAL(md.Get(Metadata::FMD_STARS), "4", ())
   md.Drop(Metadata::FMD_STARS);
@@ -83,7 +86,7 @@ UNIT_TEST(Metadata_ValidateAndFormat_operator)
   MetadataTagProcessor p(params);
   Metadata & md = params.GetMetadata();
 
-  // ignore tag 'operator' if feature have inappropriate type
+  // Ignore tag 'operator' if feature have inappropriate type.
   p("operator", "Some");
   TEST(md.Empty(), ());
 
@@ -114,7 +117,7 @@ UNIT_TEST(Metadata_ValidateAndFormat_ele)
   MetadataTagProcessor p(params);
   Metadata & md = params.GetMetadata();
 
-  // ignore tag 'operator' if feature have inappropriate type
+  // Ignore tag 'operator' if feature have inappropriate type.
   p("ele", "123");
   TEST(md.Empty(), ());
 
@@ -136,12 +139,6 @@ UNIT_TEST(Metadata_ValidateAndFormat_ele)
   md.Drop(Metadata::FMD_ELE);
 }
 
-#ifdef OMIM_OS_MOBILE
-  #define WIKIHOST "m.wikipedia.org"
-#else
-  #define WIKIHOST "wikipedia.org"
-#endif
-
 UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
 {
   char const * kWikiKey = "wikipedia";
@@ -150,6 +147,12 @@ UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
   MetadataTagProcessor p(params);
   Metadata & md = params.GetMetadata();
 
+#ifdef OMIM_OS_MOBILE
+  #define WIKIHOST "m.wikipedia.org"
+#else
+  #define WIKIHOST "wikipedia.org"
+#endif
+
   p(kWikiKey, "en:Bad %20Data");
   TEST_EQUAL(md.Get(Metadata::FMD_WIKIPEDIA), "en:Bad %20Data", ());
   TEST_EQUAL(md.GetWikiURL(), "https://en." WIKIHOST "/wiki/Bad_%2520Data", ());
@@ -196,4 +199,6 @@ UNIT_TEST(Metadata_ValidateAndFormat_wikipedia)
 
   p(kWikiKey, "http://ru.google.com/wiki/wutlol");
   TEST(md.Empty(), ("Not a wikipedia site."));
+
+#undef WIKIHOST
 }
diff --git a/indexer/feature_meta.cpp b/indexer/feature_meta.cpp
index 79ef153498..a9baec67e5 100644
--- a/indexer/feature_meta.cpp
+++ b/indexer/feature_meta.cpp
@@ -6,12 +6,15 @@
 namespace feature
 {
 
-static char constexpr const * kBaseWikiUrl =
+namespace
+{
+char constexpr const * kBaseWikiUrl =
 #ifdef OMIM_OS_MOBILE
     ".m.wikipedia.org/wiki/";
 #else
     ".wikipedia.org/wiki/";
 #endif
+} // namespace
 
 string Metadata::GetWikiURL() const
 {
diff --git a/indexer/indexer_tests/feature_metadata_test.cpp b/indexer/indexer_tests/feature_metadata_test.cpp
index cc68052d30..9d9cd4de90 100644
--- a/indexer/indexer_tests/feature_metadata_test.cpp
+++ b/indexer/indexer_tests/feature_metadata_test.cpp
@@ -10,6 +10,16 @@
 
 using feature::Metadata;
 
+namespace
+{
+map<Metadata::EType, string> const kKeyValues =
+{
+  {Metadata::FMD_ELE, "12345"},
+  {Metadata::FMD_CUISINE, "greek;mediterranean"},
+  {Metadata::FMD_EMAIL, "cool@email.at"}
+};
+} // namespace
+
 UNIT_TEST(Feature_Metadata_GetSet)
 {
   Metadata m;
@@ -29,50 +39,46 @@ UNIT_TEST(Feature_Metadata_GetSet)
   TEST(m.Empty(), ());
 }
 
-static map<Metadata::EType, string> const kPairs = { {Metadata::FMD_ELE, "12345"},
-                                                     {Metadata::FMD_CUISINE, "greek;mediterranean"},
-                                                     {Metadata::FMD_EMAIL, "cool@email.at"} };
-
 UNIT_TEST(Feature_Metadata_PresentTypes)
 {
   Metadata m;
-  for (auto const & value : kPairs)
+  for (auto const & value : kKeyValues)
     m.Set(value.first, value.second);
-  TEST_EQUAL(m.Size(), kPairs.size(), ());
+  TEST_EQUAL(m.Size(), kKeyValues.size(), ());
 
   auto const types = m.GetPresentTypes();
   TEST_EQUAL(types.size(), m.Size(), ());
   for (auto const & type : types)
-    TEST_EQUAL(m.Get(type), kPairs.find(type)->second, ());
+    TEST_EQUAL(m.Get(type), kKeyValues.find(type)->second, ());
 }
 
 UNIT_TEST(Feature_Serialization)
 {
   Metadata original;
-  for (auto const & value : kPairs)
+  for (auto const & value : kKeyValues)
     original.Set(value.first, value.second);
-  TEST_EQUAL(original.Size(), kPairs.size(), ());
+  TEST_EQUAL(original.Size(), kKeyValues.size(), ());
 
   {
     Metadata serialized;
     vector<char> buffer;
-    MemWriter<vector<char> > writer(buffer);
+    MemWriter<decltype(buffer)> writer(buffer);
     original.Serialize(writer);
 
     MemReader reader(buffer.data(), buffer.size());
     ReaderSource<MemReader> src(reader);
     serialized.Deserialize(src);
 
-    for (auto const & value : kPairs)
+    for (auto const & value : kKeyValues)
       TEST_EQUAL(serialized.Get(value.first), value.second, ());
     TEST_EQUAL(serialized.Get(Metadata::FMD_OPERATOR), "", ());
-    TEST_EQUAL(serialized.Size(), kPairs.size(), ());
+    TEST_EQUAL(serialized.Size(), kKeyValues.size(), ());
   }
 
   {
     Metadata serialized;
     vector<char> buffer;
-    MemWriter<vector<char> > writer(buffer);
+    MemWriter<decltype(buffer)> writer(buffer);
     // Here is the difference.
     original.SerializeToMWM(writer);
 
@@ -81,10 +87,11 @@ UNIT_TEST(Feature_Serialization)
     // Here is another difference.
     serialized.DeserializeFromMWM(src);
 
-    for (auto const & value : kPairs)
+    for (auto const & value : kKeyValues)
       TEST_EQUAL(serialized.Get(value.first), value.second, ());
+
     TEST_EQUAL(serialized.Get(Metadata::FMD_OPERATOR), "", ());
-    TEST_EQUAL(serialized.Size(), kPairs.size(), ());
+    TEST_EQUAL(serialized.Size(), kKeyValues.size(), ());
   }
 }