Fix issue with track linking (#1549)
authorMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 9 Aug 2024 14:09:44 +0000 (16:09 +0200)
committerGitHub <noreply@github.com>
Fri, 9 Aug 2024 14:09:44 +0000 (16:09 +0200)
music_assistant/server/controllers/music.py
music_assistant/server/helpers/compare.py
tests/server/test_compare.py

index d4b9c54f8df7a31c486b0cb4c21693c38d5427b1..069d688229014e6feec256be601680750f97a43a 100644 (file)
@@ -547,7 +547,7 @@ class MusicController(CoreController):
         media_item = await ctrl.get_provider_item(item_id, provider, force_refresh=True)
         # update library item if needed (including refresh of the metadata etc.)
         if is_library_item:
-            library_item = await ctrl.add_item_to_library(media_item)
+            library_item = await ctrl.add_item_to_library(media_item, overwrite_existing=True)
             await self.mass.metadata.update_metadata(library_item, force_refresh=True)
             return library_item
 
index 34a111a12b8edd2abcd84f72d610e30e2b52f786..ba9afaabb149977fcdeaac05d01b6814a0e0f085 100644 (file)
@@ -70,8 +70,8 @@ def compare_artist(
 
 
 def compare_album(
-    base_item: Album | ItemMapping,
-    compare_item: Album | ItemMapping,
+    base_item: Album | ItemMapping | None,
+    compare_item: Album | ItemMapping | None,
     strict: bool = True,
 ) -> bool | None:
     """Compare two album items and return True if they match."""
@@ -117,8 +117,8 @@ def compare_album(
 
 
 def compare_track(
-    base_item: Track,
-    compare_item: Track,
+    base_item: Track | None,
+    compare_item: Track | None,
     strict: bool = True,
     track_albums: list[Album] | None = None,
 ) -> bool:
@@ -144,21 +144,6 @@ def compare_track(
         )
         if external_id_match is not None:
             return external_id_match
-    # return early on exact albumtrack match = 100% match
-    if (
-        base_item.album
-        and compare_item.album
-        and compare_album(base_item.album, compare_item.album, False)
-        and base_item.disc_number
-        and compare_item.disc_number
-        and base_item.track_number
-        and compare_item.track_number
-        and base_item.disc_number == compare_item.disc_number
-        and base_item.track_number == compare_item.track_number
-    ):
-        return True
-
-    ## fallback to comparing on attributes
 
     # compare name
     if not compare_strings(base_item.name, compare_item.name, strict=True):
@@ -177,6 +162,20 @@ def compare_track(
     if strict and compare_explicit(base_item.metadata, compare_item.metadata) is False:
         return False
 
+    # exact albumtrack match = 100% match
+    if (
+        base_item.album
+        and compare_item.album
+        and compare_album(base_item.album, compare_item.album, False)
+        and base_item.disc_number
+        and compare_item.disc_number
+        and base_item.track_number
+        and compare_item.track_number
+        and base_item.disc_number == compare_item.disc_number
+        and base_item.track_number == compare_item.track_number
+    ):
+        return True
+
     # fallback: exact album match and (near-exact) track duration match
     if (
         base_item.album is not None
@@ -216,7 +215,7 @@ def compare_track(
     # Accept last resort (in non strict mode): (near) exact duration,
     # otherwise fail all other cases.
     # Note that as this stage, all other info already matches,
-    # such as title artist etc.
+    # such as title, artist etc.
     return abs(base_item.duration - compare_item.duration) <= 2
 
 
@@ -288,8 +287,6 @@ def compare_artists(
     any_match: bool = True,
 ) -> bool:
     """Compare two lists of artist and return True if both lists match (exactly)."""
-    if not base_items and not compare_items:
-        return True
     if not base_items or not compare_items:
         return False
     # match if first artist matches in both lists
index 8fad014b0af6ba4924d4d7a52ca62cb882718e21..d7af00e94139c979816a85c9ab147b46844ee239 100644 (file)
@@ -80,6 +80,20 @@ def test_compare_album() -> None:
                 item_id="1", provider_domain="test", provider_instance="test1"
             )
         },
+        artists=media_items.UniqueList(
+            [
+                media_items.Artist(
+                    item_id="1",
+                    provider="test1",
+                    name="Artist A",
+                    provider_mappings={
+                        media_items.ProviderMapping(
+                            item_id="1", provider_domain="test", provider_instance="test1"
+                        )
+                    },
+                )
+            ]
+        ),
     )
     album_b = media_items.Album(
         item_id="1",
@@ -90,6 +104,20 @@ def test_compare_album() -> None:
                 item_id="2", provider_domain="test", provider_instance="test2"
             )
         },
+        artists=media_items.UniqueList(
+            [
+                media_items.Artist(
+                    item_id="1",
+                    provider="test1",
+                    name="Artist A",
+                    provider_mappings={
+                        media_items.ProviderMapping(
+                            item_id="1", provider_domain="test", provider_instance="test1"
+                        )
+                    },
+                )
+            ]
+        ),
     )
     # test match on name match
     assert compare.compare_album(album_a, album_b) is True
@@ -174,6 +202,20 @@ def test_compare_track() -> None:  # noqa: PLR0915
                 item_id="1", provider_domain="test", provider_instance="test1"
             )
         },
+        artists=media_items.UniqueList(
+            [
+                media_items.Artist(
+                    item_id="1",
+                    provider="test1",
+                    name="Artist A",
+                    provider_mappings={
+                        media_items.ProviderMapping(
+                            item_id="1", provider_domain="test", provider_instance="test1"
+                        )
+                    },
+                )
+            ]
+        ),
     )
     track_b = media_items.Track(
         item_id="1",
@@ -184,6 +226,20 @@ def test_compare_track() -> None:  # noqa: PLR0915
                 item_id="2", provider_domain="test", provider_instance="test2"
             )
         },
+        artists=media_items.UniqueList(
+            [
+                media_items.Artist(
+                    item_id="1",
+                    provider="test1",
+                    name="Artist A",
+                    provider_mappings={
+                        media_items.ProviderMapping(
+                            item_id="1", provider_domain="test", provider_instance="test1"
+                        )
+                    },
+                )
+            ]
+        ),
     )
     # test match on name match
     assert compare.compare_track(track_a, track_b) is True