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

Refactor/cleanup the get/add logic for mediaitems + metadata retrieval #1480

Merged
merged 1 commit into from
Jul 8, 2024
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
6 changes: 0 additions & 6 deletions music_assistant/client/music.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,6 @@ async def get_item(
media_type: MediaType,
item_id: str,
provider_instance_id_or_domain: str,
force_refresh: bool = False,
lazy: bool = True,
add_to_library: bool = False,
) -> MediaItemType | ItemMapping:
"""Get single music item by id and media type."""
return media_from_dict(
Expand All @@ -485,9 +482,6 @@ async def get_item(
media_type=media_type,
item_id=item_id,
provider_instance_id_or_domain=provider_instance_id_or_domain,
force_refresh=force_refresh,
lazy=lazy,
add_to_library=add_to_library,
)
)

Expand Down
45 changes: 7 additions & 38 deletions music_assistant/server/controllers/media/albums.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from random import choice, random
from typing import TYPE_CHECKING

from music_assistant.common.helpers.global_cache import get_global_cache_value
from music_assistant.common.helpers.json import serialize_to_json
from music_assistant.common.models.enums import ProviderFeature
from music_assistant.common.models.errors import (
Expand Down Expand Up @@ -69,21 +68,17 @@ async def get(
self,
item_id: str,
provider_instance_id_or_domain: str,
force_refresh: bool = False,
lazy: bool = True,
details: Album | ItemMapping = None,
add_to_library: bool = False,
recursive: bool = True,
) -> Album:
"""Return (full) details for a single media item."""
album = await super().get(
item_id,
provider_instance_id_or_domain,
force_refresh=force_refresh,
lazy=lazy,
details=details,
add_to_library=add_to_library,
)
# append artist details to full track item (resolve ItemMappings)
if not recursive:
return album

# append artist details to full album item (resolve ItemMappings)
album_artists = UniqueList()
for artist in album.artists:
if not isinstance(artist, ItemMapping):
Expand All @@ -94,36 +89,9 @@ async def get(
await self.mass.music.artists.get(
artist.item_id,
artist.provider,
lazy=lazy,
details=artist,
add_to_library=False,
)
)
album.artists = album_artists
if not force_refresh:
return album
# if force refresh, we need to ensure that we also refresh all album tracks
# in case of a filebased (non streaming) provider to ensure we catch changes the user
# made on track level and then pressed the refresh button on album level.
file_provs = get_global_cache_value("non_streaming_providers", [])
for album_provider_mapping in album.provider_mappings:
if album_provider_mapping.provider_instance not in file_provs:
continue
for prov_album_track in await self._get_provider_album_tracks(
album_provider_mapping.item_id, album_provider_mapping.provider_instance
):
if prov_album_track.provider != "library":
continue
for track_prov_map in prov_album_track.provider_mappings:
if track_prov_map.provider_instance != album_provider_mapping.provider_instance:
continue
prov_track = await self.mass.music.tracks.get_provider_item(
track_prov_map.item_id, track_prov_map.provider_instance, force_refresh=True
)
await self.mass.music.tracks._update_library_item(
prov_album_track.item_id, prov_track, True
)
break
return album

async def remove_item_from_library(self, item_id: str | int) -> None:
Expand Down Expand Up @@ -411,7 +379,7 @@ async def _set_album_artist(
)
return ItemMapping.from_item(db_artist)

async def _match(self, db_album: Album) -> None:
async def match_providers(self, db_album: Album) -> None:
"""Try to find match on all (streaming) providers for the provided (database) album.

This is used to link objects of different providers/qualities together.
Expand Down Expand Up @@ -451,6 +419,7 @@ async def find_prov_match(provider: MusicProvider):
match_found = True
for provider_mapping in search_result_item.provider_mappings:
await self.add_provider_mapping(db_album.item_id, provider_mapping)
db_album.provider_mappings.add(provider_mapping)
return match_found

# try to find match on all providers
Expand Down
3 changes: 2 additions & 1 deletion music_assistant/server/controllers/media/artists.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ async def _get_dynamic_tracks(
msg = "No Music Provider found that supports requesting similar tracks."
raise UnsupportedFeaturedException(msg)

async def _match(self, db_artist: Artist) -> None:
async def match_providers(self, db_artist: Artist) -> None:
"""Try to find matching artists on all providers for the provided (database) item_id.

This is used to link objects of different providers together.
Expand Down Expand Up @@ -485,6 +485,7 @@ async def _match_provider(self, db_artist: Artist, provider: MusicProvider) -> b
# 100% match, we update the db with the additional provider mapping(s)
for provider_mapping in prov_artist.provider_mappings:
await self.add_provider_mapping(db_artist.item_id, provider_mapping)
db_artist.provider_mappings.add(provider_mapping)
return True
# try to get a match with some reference albums of this artist
ref_albums = await self.mass.music.artists.albums(db_artist.item_id, db_artist.provider)
Expand Down
83 changes: 11 additions & 72 deletions music_assistant/server/controllers/media/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from abc import ABCMeta, abstractmethod
from collections.abc import Iterable
from contextlib import suppress
from time import time
from typing import TYPE_CHECKING, Any, Generic, TypeVar

from music_assistant.common.helpers.json import json_loads, serialize_to_json
Expand Down Expand Up @@ -36,7 +35,6 @@

ItemCls = TypeVar("ItemCls", bound="MediaItemType")

REFRESH_INTERVAL = 60 * 60 * 24 * 30
JSON_KEYS = ("artists", "album", "metadata", "provider_mappings", "external_ids")

SORT_KEYS = {
Expand Down Expand Up @@ -91,25 +89,24 @@ def __init__(self, mass: MusicAssistant) -> None:
self._db_add_lock = asyncio.Lock()

async def add_item_to_library(
self, item: ItemCls, metadata_lookup: bool = True, overwrite_existing: bool = False
self,
item: ItemCls,
metadata_lookup: bool = True,
overwrite_existing: bool = False,
) -> ItemCls:
"""Add item to library and return the new (or updated) database item."""
new_item = False
# grab additional metadata
if metadata_lookup:
await self.mass.metadata.get_metadata(item)
# check for existing item first
library_id = await self._get_library_item_by_match(item, overwrite_existing)

if library_id is None:
# actually add a new item in the library db
async with self._db_add_lock:
library_id = await self._add_library_item(item)
new_item = True
# also fetch same track on all providers (will also get other quality versions)
# grab additional metadata
if metadata_lookup:
library_item = await self.get_library_item(library_id)
await self._match(library_item)
await self.mass.metadata.update_metadata(library_item)
# return final library_item after all match/metadata actions
library_item = await self.get_library_item(library_id)
self.mass.signal_event(
Expand Down Expand Up @@ -149,7 +146,7 @@ async def _get_library_item_by_match(
async def update_item_in_library(
self, item_id: str | int, update: ItemCls, overwrite: bool = False
) -> ItemCls:
"""Update existing library record in the database."""
"""Update existing library record in the library database."""
await self._update_library_item(item_id, update, overwrite=overwrite)
# return the updated object
library_item = await self.get_library_item(item_id)
Expand Down Expand Up @@ -265,77 +262,19 @@ async def get(
self,
item_id: str,
provider_instance_id_or_domain: str,
force_refresh: bool = False,
lazy: bool = True,
details: ItemCls = None,
add_to_library: bool = False,
) -> ItemCls:
"""Return (full) details for a single media item."""
metadata_lookup = False
# always prefer the full library item if we have it
library_item = await self.get_library_item_by_prov_id(
if library_item := await self.get_library_item_by_prov_id(
item_id,
provider_instance_id_or_domain,
)
if library_item and (time() - (library_item.metadata.last_refresh or 0)) > REFRESH_INTERVAL:
# it's been too long since the full metadata was last retrieved (or never at all)
# NOTE: do not attempt metadata refresh on unavailable items as it has side effects
metadata_lookup = library_item.available

if library_item and not (force_refresh or metadata_lookup or add_to_library):
# we have a library item and no refreshing is needed, return the results!
):
return library_item

if force_refresh:
# get (first) provider item id belonging to this library item
add_to_library = True
metadata_lookup = True
if library_item:
# resolve library item into a provider item to get the source details
provider_instance_id_or_domain, item_id = await self.get_provider_mapping(
library_item
)

# grab full details from the provider
details = await self.get_provider_item(
return await self.get_provider_item(
item_id,
provider_instance_id_or_domain,
force_refresh=force_refresh,
fallback=details,
)
if not details and library_item:
# something went wrong while trying to fetch/refresh this item
# return the existing (unavailable) library item and leave this for another day
return library_item

if not details:
# we couldn't get a match from any of the providers, raise error
msg = f"Item not found: {provider_instance_id_or_domain}/{item_id}"
raise MediaNotFoundError(msg)

if not (add_to_library or metadata_lookup):
# return the provider item as-is
return details

# create task to add the item to the library,
# including matching metadata etc. takes some time
# in 99% of the cases we just return lazy because we want the details as fast as possible
# only if we really need to wait for the result (e.g. to prevent race conditions),
# we can set lazy to false and we await the job to complete.
overwrite_existing = force_refresh and library_item is not None
task_id = f"add_{self.media_type.value}.{details.provider}.{details.item_id}"
add_task = self.mass.create_task(
self.add_item_to_library,
item=details,
metadata_lookup=metadata_lookup,
overwrite_existing=overwrite_existing,
task_id=task_id,
)
if not lazy:
await add_task
return add_task.result()

return library_item or details

async def search(
self,
Expand Down Expand Up @@ -728,7 +667,7 @@ async def _update_library_item(
) -> None:
"""Update existing library record in the database."""

async def _match(self, db_item: ItemCls) -> None:
async def match_providers(self, db_item: ItemCls) -> None:
"""
Try to find match on all (streaming) providers for the provided (database) item.

Expand Down
39 changes: 23 additions & 16 deletions music_assistant/server/controllers/media/playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import random
import time
from typing import Any

from music_assistant.common.helpers.json import serialize_to_json
Expand Down Expand Up @@ -55,17 +56,19 @@ async def tracks(
playlist = await self.get(
item_id,
provider_instance_id_or_domain,
force_refresh=force_refresh,
lazy=not force_refresh,
)
# a playlist can only have one provider so simply pick the first one
prov_map = next(x for x in playlist.provider_mappings)
cache_checksum = playlist.cache_checksum
# playlist tracks ar enot stored in the db,
# we always fetched them (cached) from the provider
tracks = await self._get_provider_playlist_tracks(
prov_map.item_id,
prov_map.provider_instance,
cache_checksum=cache_checksum,
offset=offset,
limit=limit,
force_refresh=force_refresh,
)
if prefer_library_items:
final_tracks = []
Expand Down Expand Up @@ -183,9 +186,15 @@ async def add_playlist_tracks(self, db_playlist_id: str | int, uris: list[str])
continue

# ensure we have a full library track
db_track = await self.mass.music.tracks.get(
item_id, provider_instance_id_or_domain, lazy=False, add_to_library=True
full_track = await self.mass.music.tracks.get(
item_id,
provider_instance_id_or_domain,
recursive=provider_instance_id_or_domain != "library",
)
if full_track.provider == "library":
db_track = full_track
else:
db_track = await self.mass.music.tracks.add_item_to_library(full_track)
# a track can contain multiple versions on the same provider
# simply sort by quality and just add the first available version
for track_version in sorted(
Expand Down Expand Up @@ -241,11 +250,8 @@ async def add_playlist_tracks(self, db_playlist_id: str | int, uris: list[str])
# actually add the tracks to the playlist on the provider
await playlist_prov.add_playlist_tracks(playlist_prov_map.item_id, list(ids_to_add))
# invalidate cache so tracks get refreshed
await self.get(
playlist.item_id,
playlist.provider,
force_refresh=True,
)
playlist.cache_checksum = str(time.time())
await self.update_item_in_library(db_playlist_id, playlist)

async def add_playlist_track(self, db_playlist_id: str | int, track_uri: str) -> None:
"""Add (single) track to playlist."""
Expand Down Expand Up @@ -273,11 +279,8 @@ async def remove_playlist_tracks(
continue
await provider.remove_playlist_tracks(prov_mapping.item_id, positions_to_remove)
# invalidate cache so tracks get refreshed
await self.get(
playlist.item_id,
playlist.provider,
force_refresh=True,
)
playlist.cache_checksum = str(time.time())
await self.update_item_in_library(db_playlist_id, playlist)

async def get_all_playlist_tracks(
self, playlist: Playlist, prefer_library_items: bool = False
Expand Down Expand Up @@ -375,6 +378,7 @@ async def _get_provider_playlist_tracks(
cache_checksum: Any = None,
offset: int = 0,
limit: int = 50,
force_refresh: bool = False,
) -> list[PlaylistTrack]:
"""Return playlist tracks for the given provider playlist id."""
assert provider_instance_id_or_domain != "library"
Expand All @@ -383,9 +387,12 @@ async def _get_provider_playlist_tracks(
return []
# prefer cache items (if any)
cache_key = f"{provider.lookup_key}.playlist.{item_id}.tracks.{offset}.{limit}"
if (cache := await self.mass.cache.get(cache_key, checksum=cache_checksum)) is not None:
if (
not force_refresh
and (cache := await self.mass.cache.get(cache_key, checksum=cache_checksum)) is not None
):
return [PlaylistTrack.from_dict(x) for x in cache]
# no items in cache - get listing from provider
# no items in cache (or force_refresh) - get listing from provider
result: list[Track] = []
for item in await provider.get_playlist_tracks(item_id, offset=offset, limit=limit):
# double check if position set
Expand Down
Loading