Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some fixes for the database migration and matching logic #958

Merged
merged 7 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions music_assistant/server/controllers/media/albums.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,15 +394,15 @@ async def find_prov_match(provider: MusicProvider):
for search_result_item in search_result:
if not search_result_item.available:
continue
if not compare_album(search_result_item, db_album):
if not compare_album(db_album, search_result_item):
continue
# we must fetch the full album version, search results are simplified objects
prov_album = await self.get_provider_item(
search_result_item.item_id,
search_result_item.provider,
fallback=search_result_item,
)
if compare_album(prov_album, db_album):
if compare_album(db_album, prov_album):
# 100% match, we update the db with the additional provider mapping(s)
match_found = True
for provider_mapping in search_result_item.provider_mappings:
Expand Down
4 changes: 3 additions & 1 deletion music_assistant/server/controllers/media/artists.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,9 @@ async def _match(self, db_artist: Artist, provider: MusicProvider) -> bool:
if search_result_item.sort_name != ref_album.sort_name:
continue
# artist must match 100%
if not compare_artist(search_result_item.artists[0], db_artist):
if not compare_artist(
db_artist, search_result_item.artists[0], allow_name_match=True
):
continue
# 100% match
# get full artist details so we have all metadata
Expand Down
4 changes: 2 additions & 2 deletions music_assistant/server/controllers/media/tracks.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,15 +327,15 @@ async def _match(self, db_track: Track) -> None:
if not search_result_item.available:
continue
# do a basic compare first
if not compare_track(search_result_item, db_track, strict=False):
if not compare_track(db_track, search_result_item, strict=False):
continue
# we must fetch the full version, search results are simplified objects
prov_track = await self.get_provider_item(
search_result_item.item_id,
search_result_item.provider,
fallback=search_result_item,
)
if compare_track(prov_track, db_track, strict=True, track_albums=track_albums):
if compare_track(db_track, prov_track, strict=True, track_albums=track_albums):
# 100% match, we update the db with the additional provider mapping(s)
match_found = True
for provider_mapping in search_result_item.provider_mappings:
Expand Down
21 changes: 2 additions & 19 deletions music_assistant/server/controllers/music.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import asyncio
import os
import shutil
import sqlite3
import statistics
from collections.abc import AsyncGenerator
from contextlib import suppress
Expand Down Expand Up @@ -668,6 +667,8 @@ async def _setup_database(self):
"ADD COLUMN external_ids "
"json NOT NULL DEFAULT '[]'"
)
if table in (DB_TABLE_PLAYLISTS, DB_TABLE_RADIOS):
continue
# migrate existing ids into the new external_ids column
async for item in self.database.iter_items(table):
external_ids: set[tuple[str, str]] = set()
Expand Down Expand Up @@ -696,24 +697,6 @@ async def _setup_database(self):
"Database migration to version %s completed",
DB_SCHEMA_VERSION,
)
elif prev_version == 26:
self.logger.info(
"Performing database migration from %s to %s",
prev_version,
DB_SCHEMA_VERSION,
)
# migrate playlists and radios tables which we forgot to migrate in schema 26
for table in (
DB_TABLE_PLAYLISTS,
DB_TABLE_RADIOS,
):
# create new external_ids column
with suppress(sqlite3.OperationalError):
await self.database.execute(
f"ALTER TABLE {table} "
"ADD COLUMN external_ids "
"json NOT NULL DEFAULT '[]'"
)
# handle all other schema versions
else:
# we keep it simple and just recreate the tables
Expand Down
26 changes: 17 additions & 9 deletions music_assistant/server/helpers/compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def compare_album(
if (
hasattr(base_item, "metadata")
and hasattr(compare_item, "metadata")
and not compare_explicit(base_item.metadata, compare_item.metadata)
and compare_explicit(base_item.metadata, compare_item.metadata) is False
):
return False
# compare album artist
Expand Down Expand Up @@ -105,6 +105,7 @@ def compare_track(
external_id_match = compare_external_ids(base_item.external_ids, compare_item.external_ids)
if external_id_match is not None:
return external_id_match

## fallback to comparing on attributes
# compare name
if not compare_strings(base_item.name, compare_item.name, strict=True):
Expand All @@ -120,9 +121,9 @@ def compare_track(
base_item.metadata.explicit = base_item.album.metadata.explicit
if compare_item.metadata.explicit is None and isinstance(compare_item.album, Album):
compare_item.metadata.explicit = compare_item.album.metadata.explicit
if strict and not compare_explicit(base_item.metadata, compare_item.metadata):
if strict and compare_explicit(base_item.metadata, compare_item.metadata) is False:
return False
if not strict and not track_albums:
if not strict and not (base_item.album or track_albums):
# in non-strict mode, the album does not have to match
return abs(base_item.duration - compare_item.duration) <= 3
# exact albumtrack match = 100% match
Expand All @@ -138,14 +139,14 @@ def compare_track(
base_item.album is not None
and compare_item.album is not None
and compare_album(base_item.album, compare_item.album)
and abs(base_item.duration - compare_item.duration) <= 5
and abs(base_item.duration - compare_item.duration) <= 3
):
return True
# fallback: additional compare albums provided for base track
if (
compare_item.album is not None
and track_albums
and abs(base_item.duration - compare_item.duration) <= 5
and abs(base_item.duration - compare_item.duration) <= 3
):
for track_album in track_albums:
if compare_album(track_album, compare_item.album):
Expand All @@ -154,7 +155,7 @@ def compare_track(
if (
base_item.album is None
and compare_item.album is None
and abs(base_item.duration - compare_item.duration) <= 3
and abs(base_item.duration - compare_item.duration) <= 1
):
return True

Expand Down Expand Up @@ -243,10 +244,17 @@ def compare_external_ids(
# handle upc stored as EAN-13 barcode
if external_id_base[0] == ExternalID.BARCODE and len(external_id_base[1]) == 12:
external_id_base[1] = f"0{external_id_base}"
if external_id_compare[0] == ExternalID.BARCODE and len(external_id_compare[1]) == 12:
if external_id_compare[1] == ExternalID.BARCODE and len(external_id_compare[1]) == 12:
external_id_compare[1] = f"0{external_id_compare}"
# external id is exact match. either it is a match or it isn't
return external_id_compare[0] == external_id_base[0]
if external_id_base[0] in (ExternalID.ISRC, ExternalID.BARCODE):
if external_id_compare[1] == external_id_base[1]:
# barcode and isrc can be multiple per media item
# so we only return early on match as there might be
# another entry for this ExternalID type.
return True
continue
# other ExternalID types: external id must be exact match.
return external_id_compare[1] == external_id_base[1]
# return None to define we did not find the same external id type in both sets
return None

Expand Down
19 changes: 17 additions & 2 deletions music_assistant/server/providers/sonos/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ def update_attributes(self):
self.player.elapsed_time_last_updated = self.track_info_updated

# zone topology (syncing/grouping) details
if self.group_info and self.group_info.coordinator.uid == self.player_id:
if (
self.group_info
and self.group_info.coordinator
and self.group_info.coordinator.uid == self.player_id
):
# this player is the sync leader
self.player.synced_to = None
group_members = {x.uid for x in self.group_info.members if x.is_visible}
Expand Down Expand Up @@ -375,6 +379,10 @@ async def cmd_sync(self, player_id: str, target_player: str) -> None:
"""
sonos_player = self.sonosplayers[player_id]
await asyncio.to_thread(sonos_player.soco.join, self.sonosplayers[target_player].soco)
await asyncio.to_thread(
sonos_player.update_info,
update_group_info=True,
)

async def cmd_unsync(self, player_id: str) -> None:
"""Handle UNSYNC command for given player.
Expand All @@ -385,6 +393,10 @@ async def cmd_unsync(self, player_id: str) -> None:
"""
sonos_player = self.sonosplayers[player_id]
await asyncio.to_thread(sonos_player.soco.unjoin)
await asyncio.to_thread(
sonos_player.update_info,
update_group_info=True,
)

async def poll_player(self, player_id: str) -> None:
"""Poll player for state updates.
Expand Down Expand Up @@ -431,7 +443,10 @@ async def _run_discovery(self) -> None:
for device in discovered_devices:
if device.uid not in added_devices:
continue
await self._device_discovered(device)
try:
await self._device_discovered(device)
except Exception as err:
self.logger.exception(str(err), exc_info=err)

finally:
self._discovery_running = False
Expand Down