fix bug where updating release cascade deleted all tracks

azuline · Apr 21, 2024 · b64f4d0 · b64f4d0
1 parent cf66f72
commit b64f4d0
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 3 deletions.
diff --git a/rose/cache.py b/rose/cache.py
@@ -1045,6 +1045,7 @@ def _update_cache_for_releases_executor(
                 args.extend([release_id, *utrks])
             conn.execute(query, args)
         if upd_release_args:
+            # The OR REPLACE handles source_path conflicts. The ON CONFLICT handles normal updates.
             conn.execute(
                 f"""
                 INSERT OR REPLACE INTO releases (
@@ -1062,7 +1063,20 @@ def _update_cache_for_releases_executor(
                   , new
                   , metahash
                 ) VALUES {",".join(["(?,?,?,?,?,?,?,?,?,?,?,?,?)"] * len(upd_release_args))}
-                """,
+                ON CONFLICT (id) DO UPDATE SET
+                    source_path      = excluded.source_path
+                  , cover_image_path = excluded.cover_image_path
+                  , added_at         = excluded.added_at
+                  , datafile_mtime   = excluded.datafile_mtime
+                  , title            = excluded.title
+                  , releasetype      = excluded.releasetype
+                  , releaseyear      = excluded.releaseyear
+                  , compositionyear  = excluded.compositionyear
+                  , catalognumber    = excluded.catalognumber
+                  , disctotal        = excluded.disctotal
+                  , new              = excluded.new
+                  , metahash         = excluded.metahash
+               """,
                 _flatten(upd_release_args),
             )
         if upd_release_genre_args:
@@ -1111,6 +1125,7 @@ def _update_cache_for_releases_executor(
                 _flatten(upd_release_artist_args),
             )
         if upd_track_args:
+            # The OR REPLACE handles source_path conflicts. The ON CONFLICT handles normal updates.
             conn.execute(
                 f"""
                 INSERT OR REPLACE INTO tracks (
@@ -1126,6 +1141,16 @@ def _update_cache_for_releases_executor(
                   , metahash
                 )
                 VALUES {",".join(["(?,?,?,?,?,?,?,?,?,?)"]*len(upd_track_args))}
+                ON CONFLICT (id) DO UPDATE SET
+                    source_path                = excluded.source_path
+                  , source_mtime               = excluded.source_mtime
+                  , title                      = excluded.title
+                  , release_id                 = excluded.release_id
+                  , tracknumber                = excluded.tracknumber
+                  , tracktotal                 = excluded.tracktotal
+                  , discnumber                 = excluded.discnumber
+                  , duration_seconds           = excluded.duration_seconds
+                  , metahash                   = excluded.metahash
                 """,
                 _flatten(upd_track_args),
             )
@@ -1402,7 +1427,8 @@ def update_cache_for_collages(
                 logger.info(f"Updating cache for collage {cached_collage.name}")
                 conn.execute(
                     """
-                    INSERT OR REPLACE INTO collages (name, source_mtime) VALUES (?, ?)
+                    INSERT INTO collages (name, source_mtime) VALUES (?, ?)
+                    ON CONFLICT (name) DO UPDATE SET source_mtime = excluded.source_mtime
                     """,
                     (cached_collage.name, cached_collage.source_mtime),
                 )
@@ -1629,7 +1655,10 @@ def update_cache_for_playlists(
                 logger.info(f"Updating cache for playlist {cached_playlist.name}")
                 conn.execute(
                     """
-                    INSERT OR REPLACE INTO playlists (name, source_mtime, cover_path) VALUES (?, ?, ?)
+                    INSERT INTO playlists (name, source_mtime, cover_path) VALUES (?, ?, ?)
+                    ON CONFLICT (name) DO UPDATE SET
+                        source_mtime = excluded.source_mtime
+                      , cover_path = excluded.cover_path
                     """,
                     (
                         cached_playlist.name,

diff --git a/rose/cache_test.py b/rose/cache_test.py
@@ -604,6 +604,31 @@ def test_update_cache_rename_source_files(config: Config) -> None:
         }
 
 
+def test_update_cache_add_cover_art(config: Config) -> None:
+    """
+    Test that adding a cover art (i.e. modifying release w/out modifying tracks) does not affect
+    the tracks.
+    """
+    config = dataclasses.replace(config, rename_source_files=True)
+    shutil.copytree(TEST_RELEASE_1, config.music_source_dir / TEST_RELEASE_1.name)
+    update_cache(config)
+    expected_dir = config.music_source_dir / "BLACKPINK - 1990. I Love Blackpink [NEW]"
+
+    (expected_dir / "cover.jpg").touch()
+    update_cache(config)
+
+    with connect(config) as conn:
+        cursor = conn.execute("SELECT source_path, cover_image_path FROM releases")
+        row = cursor.fetchone()
+        assert Path(row["source_path"]) == expected_dir
+        assert Path(row["cover_image_path"]) == expected_dir / "cover.jpg"
+        cursor = conn.execute("SELECT source_path FROM tracks")
+        assert {Path(r[0]) for r in cursor} == {
+            expected_dir / "01. Track 1.m4a",
+            expected_dir / "02. Track 2.m4a",
+        }
+
+
 def test_update_cache_rename_source_files_nested_file_directories(config: Config) -> None:
     """Test that we properly rename arbitrarily nested files and clean up the empty dirs."""
     config = dataclasses.replace(config, rename_source_files=True)