From 06eeacea4b64606bc5b136828d3f6a5f894b1541 Mon Sep 17 00:00:00 2001
From: Joao Paulo Magalhaes <dev@jpmag.me>
Date: Sun, 17 Nov 2024 13:09:04 +0000
Subject: [PATCH] read<std::map>() implementation: overwrite existing entries

re #477
---
 changelog/current.md      |  3 +++
 src/c4/yml/std/map.hpp    |  9 ++++++++-
 src/c4/yml/std/vector.hpp |  4 +++-
 test/test_serialize.cpp   | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/changelog/current.md b/changelog/current.md
index e69de29bb..cb94ad98a 100644
--- a/changelog/current.md
+++ b/changelog/current.md
@@ -0,0 +1,3 @@
+## Fixes
+
+- [BREAKING] Fix [#477](https://github.com/biojppm/rapidyaml/issues/477): changed `read<std::map>()` to overwrite existing entries. The provided implementations had an inconsistency between `std::map` (which wasn't overwriting) and `std::vector` (which *was* overwriting).
diff --git a/src/c4/yml/std/map.hpp b/src/c4/yml/std/map.hpp
index fc48dc5e6..6c799cab6 100644
--- a/src/c4/yml/std/map.hpp
+++ b/src/c4/yml/std/map.hpp
@@ -25,6 +25,9 @@ void write(c4::yml::NodeRef *n, std::map<K, V, Less, Alloc> const& m)
     }
 }
 
+/** read the node members, assigning into the existing map. If a key
+ * is already present in the map, then its value will be
+ * move-assigned. */
 template<class K, class V, class Less, class Alloc>
 bool read(c4::yml::ConstNodeRef const& n, std::map<K, V, Less, Alloc> * m)
 {
@@ -34,7 +37,11 @@ bool read(c4::yml::ConstNodeRef const& n, std::map<K, V, Less, Alloc> * m)
     {
         ch >> c4::yml::key(k);
         ch >> v;
-        m->emplace(std::make_pair(std::move(k), std::move(v)));
+        const auto it = m->find(k);
+        if(it == m->end())
+            m->emplace(std::make_pair(std::move(k), std::move(v)));
+        else
+            it->second = std::move(v);
     }
     return true;
 }
diff --git a/src/c4/yml/std/vector.hpp b/src/c4/yml/std/vector.hpp
index 44dedf02a..68aa39a24 100644
--- a/src/c4/yml/std/vector.hpp
+++ b/src/c4/yml/std/vector.hpp
@@ -21,6 +21,7 @@ void write(c4::yml::NodeRef *n, std::vector<V, Alloc> const& vec)
         n->append_child() << v;
 }
 
+/** read the node members, overwriting existing vector entries. */
 template<class V, class Alloc>
 bool read(c4::yml::ConstNodeRef const& n, std::vector<V, Alloc> *vec)
 {
@@ -33,7 +34,8 @@ bool read(c4::yml::ConstNodeRef const& n, std::vector<V, Alloc> *vec)
     return true;
 }
 
-/** specialization: std::vector<bool> uses std::vector<bool>::reference as
+/** read the node members, overwriting existing vector entries.
+ * specialization: std::vector<bool> uses std::vector<bool>::reference as
  * the return value of its operator[]. */
 template<class Alloc>
 bool read(c4::yml::ConstNodeRef const& n, std::vector<bool, Alloc> *vec)
diff --git a/test/test_serialize.cpp b/test/test_serialize.cpp
index 854727bcd..ec2ff5595 100644
--- a/test/test_serialize.cpp
+++ b/test/test_serialize.cpp
@@ -804,6 +804,39 @@ TEST(serialize, issue442_61)
 }
 
 
+TEST(serialize, issue477_vec)
+{
+    const Tree t = parse_in_arena(R"([0, 10, 20, 30, 40])");
+    std::vector<int> vec = {100, 1, 2, 3};
+    ASSERT_EQ(vec.size(), 4);
+    EXPECT_EQ(vec[0], 100);
+    EXPECT_EQ(vec[1], 1);
+    EXPECT_EQ(vec[2], 2);
+    EXPECT_EQ(vec[3], 3);
+    t.rootref() >> vec;
+    ASSERT_EQ(vec.size(), 5);
+    EXPECT_EQ(vec[0], 0);
+    EXPECT_EQ(vec[1], 10);
+    EXPECT_EQ(vec[2], 20);
+    EXPECT_EQ(vec[3], 30);
+    EXPECT_EQ(vec[4], 40);
+}
+
+TEST(serialize, issue477_map)
+{
+    const Tree t = parse_in_arena(R"({0: 10, 2: 30, 4: 50})");
+    std::map<int,int> map = {{0, 1}, {2, 3}};
+    ASSERT_EQ(map.size(), 2);
+    EXPECT_EQ(map[0], 1);
+    EXPECT_EQ(map[2], 3);
+    t.rootref() >> map;
+    ASSERT_EQ(map.size(), 3); // added a new member
+    EXPECT_EQ(map[0], 10); // modified
+    EXPECT_EQ(map[2], 30); // modified
+    EXPECT_EQ(map[4], 50);
+}
+
+
 //-------------------------------------------
 // this is needed to use the test case library
 Case const* get_case(csubstr /*name*/)