diff --git a/src/ogg.cc b/src/ogg.cc index e87acb6..5ee7459 100644 --- a/src/ogg.cc +++ b/src/ogg.cc @@ -121,7 +121,7 @@ void ogg::Stream::parse_opustags(const ogg_packet &op) uint32_t comment_length = le32toh(*reinterpret_cast(data)); if (remaining - 4 < comment_length) throw std::runtime_error("no space for comment contents"); - tags.set(std::string(data + 4, comment_length)); + tags.add(std::string(data + 4, comment_length)); data += 4 + comment_length; remaining -= 4 + comment_length; } @@ -290,12 +290,12 @@ std::string ogg::Encoder::render_opustags(const Tags &tags) length = htole32(assocs.size()); s.sputn(reinterpret_cast(&length), 4); - for (auto assoc : assocs) { - length = htole32(assoc.first.size() + 1 + assoc.second.size()); + for (const auto assoc : assocs) { + length = htole32(assoc.key.size() + 1 + assoc.value.size()); s.sputn(reinterpret_cast(&length), 4); - s.sputn(assoc.first.data(), assoc.first.size()); + s.sputn(assoc.key.data(), assoc.key.size()); s.sputc('='); - s.sputn(assoc.second.data(), assoc.second.size()); + s.sputn(assoc.value.data(), assoc.value.size()); } s.sputn(tags.extra.data(), tags.extra.size()); diff --git a/src/tags.cc b/src/tags.cc index d9f14e9..2fd09f3 100644 --- a/src/tags.cc +++ b/src/tags.cc @@ -3,63 +3,54 @@ using namespace opustags; -Tags::Tags() : max_index(0) +const std::vector Tags::get_all() const { -} - -const std::vector> Tags::get_all() const -{ - std::vector 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> result; - for (const auto &key : keys) { - result.push_back( - std::make_pair( - key, key_to_value.at(key))); - } - - return result; + return tags; } std::string Tags::get(const std::string &key) const { - return key_to_value.at(key); + for (auto &tag : tags) + if (tag.key == key) + return tag.value; + throw std::runtime_error("Tag '" + key + "' not found."); } -void Tags::set(const std::string &key, const std::string &value) +void Tags::add(const std::string &key, const std::string &value) { - key_to_value[key] = value; - key_to_index[key] = max_index; - max_index++; + tags.push_back({key, value}); } -void Tags::set(const std::string &assoc) +void Tags::clear() +{ + tags.clear(); +} + +void Tags::add(const std::string &assoc) { size_t eq = assoc.find_first_of('='); if (eq == std::string::npos) throw std::runtime_error("misconstructed tag"); std::string name = assoc.substr(0, eq); std::string value = assoc.substr(eq + 1); - set(name, value); + add(name, value); } void Tags::remove(const std::string &key) { - key_to_value.erase(key); - key_to_index.erase(key); + std::vector new_tags; + std::copy_if( + tags.begin(), + tags.end(), + std::back_inserter(new_tags), + [&](const Tag &tag) { return tag.key != key; }); + tags = new_tags; } bool Tags::contains(const std::string &key) const { - return key_to_value.find(key) != key_to_value.end(); + return std::count_if( + tags.begin(), + tags.end(), + [&](const Tag &tag) { return tag.key == key; }) > 0; } diff --git a/src/tags.h b/src/tags.h index ed51987..deeff5a 100644 --- a/src/tags.h +++ b/src/tags.h @@ -6,19 +6,24 @@ namespace opustags { + struct Tag final + { + std::string key; + std::string value; + }; + // A std::map adapter that keeps the order of insertion. class Tags final { public: - Tags(); - - const std::vector> get_all() const; + const std::vector get_all() const; std::string get(const std::string &key) const; - void set(const std::string &key, const std::string &value); - void set(const std::string &assoc); // KEY=value + void add(const std::string &key, const std::string &value); + void add(const std::string &assoc); // KEY=value void remove(const std::string &key); bool contains(const std::string &key) const; + void clear(); // Additional fields are required to match the specs: // https://tools.ietf.org/html/draft-ietf-codec-oggopus-14#section-5.2 @@ -27,9 +32,7 @@ namespace opustags { std::string extra; private: - std::map key_to_value; - std::map key_to_index; - size_t max_index; + std::vector tags; }; } diff --git a/src/tags_handlers/insertion_tags_handler.cc b/src/tags_handlers/insertion_tags_handler.cc index e7b1598..74cafe2 100644 --- a/src/tags_handlers/insertion_tags_handler.cc +++ b/src/tags_handlers/insertion_tags_handler.cc @@ -26,6 +26,6 @@ bool InsertionTagsHandler::edit_impl(Tags &tags) if (tags.contains(tag_key)) throw TagAlreadyExistsError(tag_key); - tags.set(tag_key, tag_value); + tags.add(tag_key, tag_value); return true; } diff --git a/src/tags_handlers/listing_tags_handler.cc b/src/tags_handlers/listing_tags_handler.cc index 1013667..f2af7af 100644 --- a/src/tags_handlers/listing_tags_handler.cc +++ b/src/tags_handlers/listing_tags_handler.cc @@ -11,6 +11,6 @@ ListingTagsHandler::ListingTagsHandler( void ListingTagsHandler::list_impl(const Tags &tags) { - for (const auto &kv : tags.get_all()) - output_stream << std::get<0>(kv) << "=" << std::get<1>(kv) << "\n"; + for (const auto &tag : tags.get_all()) + output_stream << tag.key << "=" << tag.value << "\n"; } diff --git a/src/tags_handlers/modification_tags_handler.cc b/src/tags_handlers/modification_tags_handler.cc index 8801d22..13ee67f 100644 --- a/src/tags_handlers/modification_tags_handler.cc +++ b/src/tags_handlers/modification_tags_handler.cc @@ -22,9 +22,13 @@ std::string ModificationTagsHandler::get_tag_value() const bool ModificationTagsHandler::edit_impl(Tags &tags) { - if (tags.contains(tag_key) && tags.get(tag_key) == tag_value) - return false; + if (tags.contains(tag_key)) + { + if (tags.get(tag_key) == tag_value) + return false; + tags.remove(tag_key); + } - tags.set(tag_key, tag_value); + tags.add(tag_key, tag_value); return true; } diff --git a/src/tags_handlers/removal_tags_handler.cc b/src/tags_handlers/removal_tags_handler.cc index 67b16c1..419a21b 100644 --- a/src/tags_handlers/removal_tags_handler.cc +++ b/src/tags_handlers/removal_tags_handler.cc @@ -23,10 +23,9 @@ bool RemovalTagsHandler::edit_impl(Tags &tags) { if (tag_key.empty()) { - const auto all_tags = tags.get_all(); - for (const auto &kv : all_tags) - tags.remove(std::get<0>(kv)); - return !all_tags.empty(); + const auto anything_removed = tags.get_all().size() > 0; + tags.clear(); + return anything_removed; } else { diff --git a/tests/tags_handlers/listing_tags_handler_test.cc b/tests/tags_handlers/listing_tags_handler_test.cc index 65998a6..a7a0ddb 100644 --- a/tests/tags_handlers/listing_tags_handler_test.cc +++ b/tests/tags_handlers/listing_tags_handler_test.cc @@ -9,10 +9,10 @@ TEST_CASE("Listing tags handler test") const auto streamno = 1; Tags tags; - tags.set("z", "value1"); - tags.set("a", "value2"); - tags.set("y", "value3"); - tags.set("c", "value4"); + tags.add("z", "value1"); + tags.add("a", "value2"); + tags.add("y", "value3"); + tags.add("c", "value4"); std::stringstream ss; ListingTagsHandler handler(streamno, ss); diff --git a/tests/tags_handlers/modification_tags_handler_test.cc b/tests/tags_handlers/modification_tags_handler_test.cc index 84340ca..6254a91 100644 --- a/tests/tags_handlers/modification_tags_handler_test.cc +++ b/tests/tags_handlers/modification_tags_handler_test.cc @@ -12,7 +12,7 @@ TEST_CASE("Modification tags handler test") const auto new_value = "dummy 2"; Tags tags; - tags.set(first_tag_key, dummy_value); + tags.add(first_tag_key, dummy_value); REQUIRE(tags.get_all().size() == 1); diff --git a/tests/tags_handlers/removal_tags_handler_test.cc b/tests/tags_handlers/removal_tags_handler_test.cc index 3607b6a..f044a7d 100644 --- a/tests/tags_handlers/removal_tags_handler_test.cc +++ b/tests/tags_handlers/removal_tags_handler_test.cc @@ -15,8 +15,8 @@ TEST_CASE("Removal tags handler test") RemovalTagsHandler handler(streamno, expected_tag_key); Tags tags; - tags.set(expected_tag_key, dummy_value); - tags.set(other_tag_key, dummy_value); + tags.add(expected_tag_key, dummy_value); + tags.add(other_tag_key, dummy_value); REQUIRE(tags.get_all().size() == 2); REQUIRE(handler.edit(streamno, tags)); @@ -30,10 +30,10 @@ TEST_CASE("Removal tags handler test") RemovalTagsHandler handler(streamno); Tags tags; - tags.set("z", "value1"); - tags.set("a", "value2"); - tags.set("y", "value3"); - tags.set("c", "value4"); + tags.add("z", "value1"); + tags.add("a", "value2"); + tags.add("y", "value3"); + tags.add("c", "value4"); REQUIRE(tags.get_all().size() == 4); REQUIRE(handler.edit(streamno, tags)); diff --git a/tests/tags_test.cc b/tests/tags_test.cc index 795e910..a6a7eaf 100644 --- a/tests/tags_test.cc +++ b/tests/tags_test.cc @@ -8,7 +8,7 @@ TEST_CASE("Tag manipulation test", "[tags]") SECTION("Basic operations") { Tags tags; REQUIRE(!tags.contains("a")); - tags.set("a", "1"); + tags.add("a", "1"); REQUIRE(tags.get("a") == "1"); REQUIRE(tags.contains("a")); tags.remove("a"); @@ -16,48 +16,68 @@ TEST_CASE("Tag manipulation test", "[tags]") REQUIRE_THROWS(tags.get("a")); } + SECTION("Clearing") { + Tags tags; + tags.add("a", "1"); + tags.add("b", "2"); + REQUIRE(tags.get_all().size() == 2); + tags.clear(); + REQUIRE(tags.get_all().size() == 0); + } + SECTION("Maintaing order of insertions") { Tags tags; - tags.set("z", "1"); - tags.set("y", "2"); - tags.set("x", "3"); - tags.set("y", "4"); + tags.add("z", "1"); + tags.add("y", "2"); + tags.add("x", "3"); + tags.add("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"); + REQUIRE(tags.get_all().size() == 4); + REQUIRE(tags.get_all()[0].key == "z"); + REQUIRE(tags.get_all()[1].key == "y"); + REQUIRE(tags.get_all()[2].key == "x"); + REQUIRE(tags.get_all()[3].key == "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"); + REQUIRE(tags.get_all()[0].key == "y"); + REQUIRE(tags.get_all()[1].key == "x"); + REQUIRE(tags.get_all()[2].key == "y"); + + tags.add("gamma", "5"); + REQUIRE(tags.get_all().size() == 4); + REQUIRE(tags.get_all()[0].key == "y"); + REQUIRE(tags.get_all()[1].key == "x"); + REQUIRE(tags.get_all()[2].key == "y"); + REQUIRE(tags.get_all()[3].key == "gamma"); } SECTION("Key to multiple values") { // ARTIST is set once per artist. // https://www.xiph.org/vorbis/doc/v-comment.html Tags tags; - tags.set("ARTIST", "You"); - tags.set("ARTIST", "Me"); + tags.add("ARTIST", "You"); + tags.add("ARTIST", "Me"); REQUIRE(tags.get_all().size() == 2); } + SECTION("Removing multivalues should remove all of them") { + Tags tags; + tags.add("ARTIST", "You"); + tags.add("ARTIST", "Me"); + tags.remove("ARTIST"); + REQUIRE(tags.get_all().size() == 0); + } + SECTION("Raw set") { Tags tags; - tags.set("TITLE=Foo=Bar"); + tags.add("TITLE=Foo=Bar"); REQUIRE(tags.get("TITLE") == "Foo=Bar"); } SECTION("Case insensitiveness for keys") { Tags tags; - tags.set("TiTlE=Boop"); + tags.add("TiTlE=Boop"); REQUIRE(tags.get("tiTLE") == "Boop"); } }