From 580712acd11d5afdacd51c2e8d29313efc93d520 Mon Sep 17 00:00:00 2001
From: cooljqln <cooljqln@noreply.codeberg.org>
Date: Fri, 24 Jan 2025 00:40:48 +0000
Subject: Resolve some issues with dangling index records (#193)

- The tag parser's cache is now cleared between indexing runs, preventing stale data from being used
 - Multi-value tag fields (genres, all artists) are now properly ingested in the tag value cache
 - Cleaning up removed index records now properly handles the case where only a subset of the records for multi-value tags need to be deleted.
 - Synthesizing missing tag values is now done in the tag parser instead of TrackTags, which resolves some issues with multi-value tag callbacks from libtags not being handled properly

These fixes seem to address all the issues with stale index records we were able to repro (including the issues in https://codeberg.org/cool-tech-zone/tangara-fw/issues/191), but if you've got any more cases with consistent repros then lmk!

Co-authored-by: ailurux <ailuruxx@gmail.com>
Reviewed-on: https://codeberg.org/cool-tech-zone/tangara-fw/pulls/193
---
 src/tangara/database/track.cpp | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

(limited to 'src/tangara/database/track.cpp')

diff --git a/src/tangara/database/track.cpp b/src/tangara/database/track.cpp
index ad9db1ba..dc66c9c9 100644
--- a/src/tangara/database/track.cpp
+++ b/src/tangara/database/track.cpp
@@ -83,11 +83,17 @@ auto tagToString(const TagValue& val) -> std::string {
           return std::to_string(arg);
         } else if constexpr (std::is_same_v<
                                  T, std::span<const std::pmr::string>>) {
-          std::ostringstream builder{};
+          std::stringstream stream{};
+          bool first = true;
           for (const auto& str : arg) {
-            builder << std::string{str.data(), str.size()} << ",";
+            if (first) {
+              first = false;
+            } else {
+              stream << ";";
+            }
+            stream << std::string{str.data(), str.size()};
           }
-          return builder.str();
+          return stream.str();
         }
       },
       val);
@@ -101,7 +107,6 @@ auto tagToString(const TagValue& val) -> std::string {
 auto parseDelimitedTags(const std::string_view s,
                         const char* delimiters,
                         std::pmr::vector<std::pmr::string>& out) -> void {
-  out.clear();
   std::string src = {s.data(), s.size()};
   char* token = std::strtok(src.data(), delimiters);
 
@@ -240,7 +245,6 @@ auto TrackTags::artist() const -> const std::optional<std::pmr::string>& {
 
 auto TrackTags::artist(std::string_view s) -> void {
   artist_ = s;
-  maybeSynthesizeAllArtists();
 }
 
 auto TrackTags::allArtists() const -> std::span<const std::pmr::string> {
@@ -249,7 +253,10 @@ auto TrackTags::allArtists() const -> std::span<const std::pmr::string> {
 
 auto TrackTags::allArtists(const std::string_view s) -> void {
   parseDelimitedTags(s, kAllArtistDelimiters, allArtists_);
-  maybeSynthesizeAllArtists();
+}
+
+auto TrackTags::singleAllArtists(const std::string_view s) -> void {
+  allArtists_.emplace_back(s);
 }
 
 auto TrackTags::album() const -> const std::optional<std::pmr::string>& {
@@ -325,17 +332,6 @@ auto TrackTags::Hash() const -> uint64_t {
   return komihash_stream_final(&stream);
 }
 
-/*
- * Adds the current 'artist' tag to 'allArtists' if needed. Many tracks lack a
- * fine-grained 'ARTISTS=' tag (or equivalent), but pushing down this nuance to
- * consumers of TrackTags adds a lot of complexity.
- */
-auto TrackTags::maybeSynthesizeAllArtists() -> void {
-  if (allArtists_.empty() && artist_) {
-    allArtists_.push_back(*artist_);
-  }
-}
-
 auto database::TrackData::clone() const -> std::shared_ptr<TrackData> {
   auto data = std::make_shared<TrackData>();
   data->id = id;
-- 
cgit v1.2.3