Improve SearchApiMetadataSourcePlugin interface

[amogus07]

Description

I'd like to refactor beets-vocadb to be a SearchApiMetadataSourcePlugin, but it serializes api responses to dataclass-like msgspec structs rather than TypedDicts, making it impossible to follow the class signature. Therefore, I removed the constraint from the type variable to allow plugin developers to use any type of data structure for search search results and instead have them implement custom logic to extract the id from a search result. For example, in beets-vocadb, I'd return result.id instead of result["id"].

To Do

• Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
• Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
• Tests. (Very much encouraged but not strictly required.)

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[codecov]

Codecov Report

❌ Patch coverage is 79.31034% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.44%. Comparing base (fc9c02a) to head (2e0475b).

Files with missing lines	Patch %	Lines
beetsplug/deezer.py	33.33%	2 Missing ⚠️
beetsplug/discogs/__init__.py	33.33%	2 Missing ⚠️
beetsplug/spotify.py	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6637   +/-   ##
=======================================
  Coverage   72.44%   72.44%           
=======================================
  Files         160      160           
  Lines       20690    20700   +10     
  Branches     3272     3272           
=======================================
+ Hits        14989    14997    +8     
- Misses       4976     4978    +2     
  Partials      725      725           

Files with missing lines	Coverage Δ
beets/metadata_plugins.py	85.29% <100.00%> (+0.53%)	⬆️
beetsplug/musicbrainz.py	94.98% <100.00%> (+0.01%)	⬆️
beetsplug/deezer.py	18.30% <33.33%> (-0.13%)	⬇️
beetsplug/discogs/__init__.py	67.17% <33.33%> (-0.21%)	⬇️
beetsplug/spotify.py	62.78% <66.66%> (+0.09%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[snejus]

Would it be possible for you to extend your classes to support __getitem__ method to allow indexing instead?

See beets.autotag.hooks::AttrDict where we do the opposite.

[amogus07]

Would it be possible for you to extend your classes to support __getitem__ method to allow indexing instead?

See beets.autotag.hooks::AttrDict where we do the opposite.

I considered it but it wouldn't work because 1. the TypeVar in SearchApiMetadataSourcePlugin is bound to the IDResponse TypedDict (which I can't inherit from) and 2. I would have to use Any to annotate it which I currently don't use anywhere in the codebase and would like to avoid using.

[amogus07]

I also considered making IDResponse a protocal, but I couldn't get that to work either.

[snejus]

What's the reason for using msgpack.Struct in your case? IDResponse is here for a reason - we expect the results to be returned as dict types. I don't think that we are ready to relax this requirement in the core.

[amogus07]

Update: I could get it working now with the latest stable, except for one patch: importing TypedDict from typing_extensions instead of typing to allow subclasses to use closed=True. It's not very polished as I've mostly copy-pasted from my existing candidates methods, but it does work. Still, please leave this pr open for now as at least that small patch would have to be merged into master, and possibly the interface could be improved in a different way too.

class SongIDResponse(IDResponse, total=True, closed=True):
    data: SongForApiContract

class PluginBase(SearchApiMetadataSourcePlugin[IDResponse | SongIDResponse]):
	...
    @override
    def get_search_query_with_filters(
        self,
        query_type: QueryType,
        items: Sequence[library.Item],
        artist: str,
        name: str,
        va_likely: bool,
    ) -> tuple[str, dict[str, str]]:
        match query_type:
            case "album":
                criteria: dict[str, str] = (
                    {"barcode": barcode}
                    if (item := next(iter(items)))
                    and (barcode := item.get("barcode"))
                    else {}
                )
            case "track":
                item: library.Item
                barcode: str | None
                criteria = {}

        return name, {
            k: _v for k, v in criteria.items() if (_v := v.lower().strip())
        }

    @override
    def get_search_response(
        self, params: SearchParams
    ) -> Sequence[IDResponse | SongIDResponse]:
        match params.query_type:
            case "album":
                self._log.debug(msg=f"Searching for album {params.query}")
                remote_album_find_result: (
                    AlbumForApiContractPartialFindResult | None
                ) = self.album_api.api_albums_get(
                    query=params.query,
                    barcode=params.filters.get("barcode", ""),
                    maxResults=params.limit,
                    nameMatchMode=NameMatchMode.AUTO,
                )
                remote_album_candidates: tuple[AlbumForApiContract, ...] | None
                if not remote_album_find_result or not (
                    remote_album_candidates := remote_album_find_result.items
                ):
                    return []
                else:
                    self._log.debug(
                        msg=f"Found {len(remote_album_candidates)} result(s)"
                        + f" for '{params.query}'"
                    )
                # songFields parameter doesn't exist for album search
                # so we'll get albums by their id
                return [
                    {"id": album_id}
                    for remote_album_candidate in remote_album_candidates
                    if (
                        album_id := self._extract_id(
                            url=remote_album_candidate.id
                        )
                    )
                ]
            case "track":
                self._log.debug(msg=f"Searching for track {params.query}")
                remote_item_find_result: (
                    SongForApiContractPartialFindResult | None
                ) = self.song_api.api_songs_get(
                    query=params.query,
                    fields=SONG_FIELDS,
                    maxResults=params.limit,
                    nameMatchMode=NameMatchMode.AUTO,
                    preferAccurateMatches=True,
                    sort=SongSortRule.SONG_TYPE,
                    lang=self.language_preference,
                )
                remote_item_candidates: tuple[SongForApiContract, ...] | None
                if not remote_item_find_result or not (
                    remote_item_candidates := remote_item_find_result.items
                ):
                    self._log.debug(msg=f"Found 0 results for '{params.query}'")
                    return []
                self._log.debug(
                    msg=f"Found {len(remote_item_candidates)} result(s)"
                    + f" for '{params.query}'"
                )
                return [
                    {
                        "id": str(remote_item_candidate.id),
                        "data": remote_item_candidate,
                    }
                    for remote_item_candidate in remote_item_candidates
                ]

    @override
    def item_candidates(
        self, item: library.Item, artist: str, title: str
    ) -> Iterable[TrackInfo]:
        return filter(
            None,
            map(
                self.mapper.track_info,
                (
                    data
                    if (data := remote_song_candidate.get("data"))
                    and isinstance(data, SongForApiContract)
                    else None
                    for remote_song_candidate in self._get_candidates(
                        query_type="track",
                        items=[item],
                        artist=artist,
                        name=title,
                        va_likely=False,
                    )
                ),
            ),
        )