Change Tags to preserve order of insertion

src/tags.cc

@@ -0,0 +1,55 @@
+#include "tags.h"
+#include <algorithm>
+
+using namespace opustags;
+
+Tags::Tags() : max_index(0)
+{
+}
+
+const std::vector<std::tuple<std::string, std::string>> Tags::get_all() const
+{
+    std::vector<std::string> keys;
+    for (const auto &kv : key_to_value)
+        keys.push_back(kv.first);
+
+    std::sort(
+        keys.begin(),
+        keys.end(),
+        [&](const std::string &a, const std::string &b)
+        {
+            return key_to_index.at(a) < key_to_index.at(b);
+        });
+
+    std::vector<std::tuple<std::string, std::string>> result;
+    for (const auto &key : keys) {
+        result.push_back(
+            std::make_tuple(
+                key, key_to_value.at(key)));
+    }
+
+    return result;
+}
+
+std::string Tags::get(const std::string &key) const
+{
+    return key_to_value.at(key);
+}
+
+void Tags::set(const std::string &key, const std::string &value)
+{
+    key_to_value[key] = value;
+    key_to_index[key] = max_index;
+    max_index++;
+}
+
+void Tags::remove(const std::string &key)
+{
+    key_to_value.erase(key);
+    key_to_index.erase(key);
+}
+
+bool Tags::contains(const std::string &key) const
+{
+    return key_to_value.find(key) != key_to_value.end();
+}

src/tags.h

@@ -1,9 +1,28 @@
 #pragma once
 
 #include <map>
+#include <vector>
+#include <tuple>
 
 namespace opustags {
 
-    using Tags = std::map<std::string, std::string>;
+    // A std::map adapter that keeps the order of insertion.
+    class Tags final
+    {
+    public:
+        Tags();
+
+        const std::vector<std::tuple<std::string, std::string>> get_all() const;
+
+        std::string get(const std::string &key) const;
+        void set(const std::string &key, const std::string &value);
+        void remove(const std::string &key);
+        bool contains(const std::string &key) const;
+
+    private:
+        std::map<std::string, std::string> key_to_value;
+        std::map<std::string, size_t> key_to_index;
+        size_t max_index;
+    };
 
 }

src/tags_handlers/insertion_tags_handler.cc

@@ -13,9 +13,9 @@ InsertionTagsHandler::InsertionTagsHandler(
 
 bool InsertionTagsHandler::edit_impl(Tags &tags)
 {
-    if (tags.find(tag_key) != tags.end())
+    if (tags.contains(tag_key))
         throw TagAlreadyExistsError(tag_key);
 
-    tags[tag_key] = tag_value;
+    tags.set(tag_key, tag_value);
     return true;
 }

src/tags_handlers/modification_tags_handler.cc

@@ -12,6 +12,6 @@ ModificationTagsHandler::ModificationTagsHandler(
 
 bool ModificationTagsHandler::edit_impl(Tags &tags)
 {
-    tags[tag_key] = tag_value;
+    tags.set(tag_key, tag_value);
     return true;
 }

src/tags_handlers/removal_tags_handler.cc

@@ -11,9 +11,9 @@ RemovalTagsHandler::RemovalTagsHandler(
 
 bool RemovalTagsHandler::edit_impl(Tags &tags)
 {
-    if (tags.find(tag_key) == tags.end())
+    if (!tags.contains(tag_key))
         throw TagDoesNotExistError(tag_key);
 
-    tags.erase(tag_key);
+    tags.remove(tag_key);
     return true;
 }

tests/tags_handlers/insertion_tags_handler_test.cc

@@ -13,7 +13,7 @@ TEST_CASE("Insertion tags handler test")
         streamno, expected_tag_key, expected_tag_value);
 
     REQUIRE(handler.edit(streamno, tags));
-    REQUIRE(tags.size() == 1);
-    REQUIRE(tags[expected_tag_key] == expected_tag_value);
+    REQUIRE(tags.get_all().size() == 1);
+    REQUIRE(tags.get(expected_tag_key) == expected_tag_value);
     REQUIRE_THROWS(handler.edit(streamno, tags));
 }

tests/tags_handlers/modification_tags_handler_test.cc

@@ -11,18 +11,20 @@ TEST_CASE("Modification tags handler test")
     const auto dummy_value = "dummy";
     const auto new_value = "dummy 2";
 
-    Tags tags = {{first_tag_key, dummy_value}};
-    REQUIRE(tags.size() == 1);
+    Tags tags;
+    tags.set(first_tag_key, dummy_value);
+
+    REQUIRE(tags.get_all().size() == 1);
 
     // setting nonexistent keys adds them
     ModificationTagsHandler handler1(streamno, other_tag_key, dummy_value);
     REQUIRE(handler1.edit(streamno, tags));
-    REQUIRE(tags.size() == 2);
-    REQUIRE(tags[other_tag_key] == dummy_value);
+    REQUIRE(tags.get_all().size() == 2);
+    REQUIRE(tags.get(other_tag_key) == dummy_value);
 
     // setting existing keys overrides their values
     ModificationTagsHandler handler2(streamno, other_tag_key, new_value);
     REQUIRE(handler2.edit(streamno, tags));
-    REQUIRE(tags.size() == 2);
-    REQUIRE(tags[other_tag_key] == new_value);
+    REQUIRE(tags.get_all().size() == 2);
+    REQUIRE(tags.get(other_tag_key) == new_value);
 }

tests/tags_handlers/removal_tags_handler_test.cc

@@ -11,10 +11,13 @@ TEST_CASE("Removal tags handler test")
     const auto dummy_value = "dummy";
     RemovalTagsHandler handler(streamno, expected_tag_key);
 
-    Tags tags = {{expected_tag_key, dummy_value}, {other_tag_key, dummy_value}};
-    REQUIRE(tags.size() == 2);
+    Tags tags;
+    tags.set(expected_tag_key, dummy_value);
+    tags.set(other_tag_key, dummy_value);
+
+    REQUIRE(tags.get_all().size() == 2);
     REQUIRE(handler.edit(streamno, tags));
-    REQUIRE(tags.size() == 1);
-    REQUIRE(tags.find(other_tag_key) != tags.end());
+    REQUIRE(tags.get_all().size() == 1);
+    REQUIRE(tags.contains(other_tag_key));
     REQUIRE_THROWS(handler.edit(streamno, tags));
 }

tests/tags_test.cc

@@ -0,0 +1,42 @@
+#include "tags.h"
+#include "catch.h"
+
+using namespace opustags;
+
+TEST_CASE("Tag manipulation test")
+{
+    SECTION("Basic operations") {
+        Tags tags;
+        REQUIRE(!tags.contains("a"));
+        tags.set("a", "1");
+        REQUIRE(tags.get("a") == "1");
+        REQUIRE(tags.contains("a"));
+        tags.remove("a");
+        REQUIRE(!tags.contains("a"));
+        REQUIRE_THROWS(tags.get("a"));
+    }
+
+    SECTION("Maintaing order of insertions") {
+        Tags tags;
+        tags.set("z", "1");
+        tags.set("y", "2");
+        tags.set("x", "3");
+        tags.set("y", "4");
+
+        REQUIRE(tags.get_all().size() == 3);
+        REQUIRE(std::get<0>(tags.get_all()[0]) == "z");
+        REQUIRE(std::get<0>(tags.get_all()[1]) == "x");
+        REQUIRE(std::get<0>(tags.get_all()[2]) == "y");
+
+        tags.remove("z");
+        REQUIRE(tags.get_all().size() == 2);
+        REQUIRE(std::get<0>(tags.get_all()[0]) == "x");
+        REQUIRE(std::get<0>(tags.get_all()[1]) == "y");
+
+        tags.set("gamma", "5");
+        REQUIRE(tags.get_all().size() == 3);
+        REQUIRE(std::get<0>(tags.get_all()[0]) == "x");
+        REQUIRE(std::get<0>(tags.get_all()[1]) == "y");
+        REQUIRE(std::get<0>(tags.get_all()[2]) == "gamma");
+    }
+}