fix #600

Instead of calling CompatibleObjectType iterator-range constructor, first convert json::value_type to CompatibleObjectType::value_type
nlohmann · Jun 7, 2017 · cea39df · cea39df
1 parent 85de93b
commit cea39df
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 20 deletions.
diff --git a/src/json.hpp b/src/json.hpp
@@ -867,11 +867,11 @@ void to_json(BasicJsonType& j, T (&arr)[N])
 }
 
 template <typename BasicJsonType, typename CompatibleString, typename T,
-         enable_if_t<std::is_constructible<typename BasicJsonType::string_t,
-                     CompatibleString>::value, int> = 0>
+          enable_if_t<std::is_constructible<typename BasicJsonType::string_t,
+                      CompatibleString>::value, int> = 0>
 void to_json(BasicJsonType& j, std::pair<CompatibleString, T> const& p)
 {
-  j[p.first] = p.second;
+    j[p.first] = p.second;
 }
 
 ///////////////
@@ -1046,10 +1046,24 @@ void from_json(const BasicJsonType& j, CompatibleObjectType& obj)
     auto inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
     using std::begin;
     using std::end;
+    using value_type = typename CompatibleObjectType::value_type;
+    std::vector<value_type> v;
+    v.reserve(j.size());
+    std::transform(
+        inner_object->begin(), inner_object->end(), std::back_inserter(v),
+        [](typename BasicJsonType::object_t::value_type const & p)
+    {
+        return value_type
+        {
+            p.first,
+            p.second
+            .template get<typename CompatibleObjectType::mapped_type>()};
+    });
     // we could avoid the assignment, but this might require a for loop, which
     // might be less efficient than the container constructor for some
     // containers (would it?)
-    obj = CompatibleObjectType(begin(*inner_object), end(*inner_object));
+    obj = CompatibleObjectType(std::make_move_iterator(begin(v)),
+                               std::make_move_iterator(end(v)));
 }
 
 // overload for arithmetic types, not chosen for basic_json template arguments
@@ -1096,8 +1110,8 @@ void from_json(const BasicJsonType& j, ArithmeticType& val)
 }
 
 template <typename BasicJsonType, typename CompatibleString, typename T,
-         enable_if_t<std::is_constructible<typename BasicJsonType::string_t,
-                     CompatibleString>::value, int> = 0>
+          enable_if_t<std::is_constructible<typename BasicJsonType::string_t,
+                      CompatibleString>::value, int> = 0>
 void from_json(const BasicJsonType& j, std::pair<CompatibleString, T>& p)
 {
     if (not j.is_object())
@@ -1112,7 +1126,7 @@ void from_json(const BasicJsonType& j, std::pair<CompatibleString, T>& p)
         JSON_THROW(other_error::create(502, "conversion to std::pair requires the object to have exactly one field, but it has " + std::to_string(size)));
     }
     auto const& obj = *inner_object->begin();
-    // cannot use *inner_object, need to convert both members 
+    // cannot use *inner_object, need to convert both members
     p = std::make_pair(obj.first, obj.second.template get<T>());
 }
 

diff --git a/test/src/unit-constructor1.cpp b/test/src/unit-constructor1.cpp
@@ -158,16 +158,27 @@ TEST_CASE("constructors")
 
         SECTION("std::pair<CompatibleString, T>")
         {
-          std::pair<std::string, std::string> p{"first", "second"};
-          json j(p);
+            std::pair<std::string, std::string> p{"first", "second"};
+            json j(p);
 
-          CHECK((j.get<decltype(p)>() == p));
+            CHECK((j.get<decltype(p)>() == p));
 
-          std::pair<std::string, int> p2{"first", 1};
-          // use char const*
-          json j2(std::make_pair("first", 1));
+            std::pair<std::string, int> p2{"first", 1};
+            // use char const*
+            json j2(std::make_pair("first", 1));
 
-          CHECK((j2.get<decltype(p2)>() == p2));
+            CHECK((j2.get<decltype(p2)>() == p2));
+        }
+
+        SECTION("std::map<std::string, std::string> #600")
+        {
+            std::map<std::string, std::string> m;
+            m["a"] = "b";
+            m["c"] = "d";
+            m["e"] = "f";
+
+            json j(m);
+            CHECK((j.get<decltype(m)>() == m));
         }
 
         SECTION("std::map<const char*, json>")
@@ -971,12 +982,23 @@ TEST_CASE("constructors")
 
             SECTION("std::pair<CompatibleString, T> with error")
             {
-              json j{{"too", "much"}, {"string", "fields"}};
-              CHECK_THROWS_AS((j.get<std::pair<std::string, std::string>>()), json::other_error);
-              CHECK_THROWS_WITH((j.get<std::pair<std::string, std::string>>()),
-                                "[json.exception.other_error.502] conversion "
-                                "to std::pair requires the object to have "
-                                "exactly one field, but it has 2");
+                SECTION("wrong field number")
+                {
+                    json j{{"too", "much"}, {"string", "fields"}};
+                    CHECK_THROWS_AS((j.get<std::pair<std::string, std::string>>()), json::other_error);
+                    CHECK_THROWS_WITH((j.get<std::pair<std::string, std::string>>()),
+                                      "[json.exception.other_error.502] conversion "
+                                      "to std::pair requires the object to have "
+                                      "exactly one field, but it has 2");
+                }
+
+                SECTION("wrong JSON type")
+                {
+                    json j(42);
+                    CHECK_THROWS_AS((j.get<std::pair<std::string, std::string>>()), json::type_error);
+                    CHECK_THROWS_WITH((j.get<std::pair<std::string, std::string>>()),
+                                      "[json.exception.type_error.302] type must be object, but is number");
+                }
             }