beets icon indicating copy to clipboard operation
beets copied to clipboard

Centralise common autotagger search functionality

Open snejus opened this issue 6 months ago • 4 comments

Add a unified search abstraction across metadata source plugins.

Summary:

  • Introduces SearchApiMetadataSourcePlugin with SearchParams, get_search_filters, and get_search_response hooks to standardize album/track searches.
  • Replaces ad‑hoc _search_api and query construction logic in Deezer, Spotify, MusicBrainz, and Discogs plugins with the new shared implementation.
  • Injects search_limit via a common path (added automatically as a filter) and applies optional ASCII normalization once.
  • Refactors Discogs and MusicBrainz plugins to use the new abstraction; factors their criteria building into get_search_filters.
  • Centralizes error handling and logging; Deezer & Spotify now return empty tuples on failures or transparently re-authenticate (Spotify).

Summary by Sourcery

Centralize and standardize autotagger search functionality by introducing a shared SearchApiMetadataSourcePlugin and refactoring existing metadata source plugins to use it

New Features:

  • Introduce SearchApiMetadataSourcePlugin with SearchParams and hooks for unified album/track searches

Enhancements:

  • Refactor Deezer, Spotify, MusicBrainz, and Discogs plugins to use the shared search abstraction
  • Centralize ASCII normalization, search limit injection, error handling, and logging in the unified search API
  • Add Spotify transparent re-authentication on 401 errors and ensure Deezer returns empty results on failures

Tests:

  • Update MusicBrainz plugin tests to adapt to the new search response handling and ID mapping

snejus avatar Sep 01 '25 11:09 snejus

Reviewer's Guide

Adds a shared SearchApiMetadataSourcePlugin abstraction to centralize query building, normalization, limit injection, error handling, and logging, then refactors all existing metadata source plugins and related tests to adopt the new hooks.

Sequence diagram for unified search flow in metadata source plugins

sequenceDiagram
    participant Plugin as SearchApiMetadataSourcePlugin
    participant UserCode as Importer/Autotagger
    participant API as External API (e.g. Spotify, Deezer)
    UserCode->>Plugin: candidates(items, artist, album, va_likely)
    Plugin->>Plugin: get_search_filters(query_type, items, artist, name, va_likely)
    Plugin->>Plugin: _search_api(query_type, query, filters)
    Plugin->>Plugin: get_search_response(SearchParams)
    Plugin->>API: Perform API request
    API-->>Plugin: Return results
    Plugin-->>UserCode: Return AlbumInfo/TrackInfo objects

Class diagram for the new SearchApiMetadataSourcePlugin abstraction and plugin refactoring

classDiagram
    class SearchApiMetadataSourcePlugin {
        +config
        +_log
        +get_search_filters(query_type, items, artist, name, va_likely)
        +get_search_response(params)
        +_search_api(query_type, query, filters)
        +_get_candidates(query_type, ...)
        +candidates(items, artist, album, va_likely)
        +item_candidates(item, artist, title)
    }
    class SearchParams {
        +query_type: QueryType
        +query: str
        +filters: dict[str, str]
    }
    class IDResponse {
        +id: str
    }
    SearchApiMetadataSourcePlugin <|-- SpotifyPlugin
    SearchApiMetadataSourcePlugin <|-- DeezerPlugin
    SearchApiMetadataSourcePlugin <|-- MusicBrainzPlugin
    SearchApiMetadataSourcePlugin <|-- DiscogsPlugin
    SearchParams <.. SearchApiMetadataSourcePlugin
    IDResponse <.. SearchApiMetadataSourcePlugin
    class SpotifyPlugin {
        +get_search_response(params)
    }
    class DeezerPlugin {
        +get_search_response(params)
    }
    class MusicBrainzPlugin {
        +get_search_filters(...)
        +get_search_response(params)
    }
    class DiscogsPlugin {
        +get_search_filters(...)
        +get_search_response(params)
    }

Class diagram for SearchParams and its usage

classDiagram
    class SearchParams {
        +query_type: QueryType
        +query: str
        +filters: dict[str, str]
    }
    SearchParams <.. SearchApiMetadataSourcePlugin: uses

File-Level Changes

Change Details Files
Centralize search logic in metadata_plugins.py by introducing SearchApiMetadataSourcePlugin
  • Define SearchParams NamedTuple and QueryType alias, replacing the old SearchFilter TypedDict
  • Add abstract get_search_filters and get_search_response hooks
  • Implement shared _search_api with ASCII normalization, limit injection, centralized error handling and logging
  • Introduce _get_candidates helper and update type annotations, removing legacy query builders
beets/metadata_plugins.py
Migrate plugins to use the new search abstraction
  • Remove custom _search_api implementations and query construction in Spotify, Deezer, MusicBrainz, and Discogs
  • Implement get_search_filters and get_search_response per plugin to build filters and parse API responses
  • Add Spotify reauthentication on HTTP 401 failures
  • Simplify candidate and album/track lookup methods to delegate to base class logic
beetsplug/spotify.py
beetsplug/deezer.py
beetsplug/musicbrainz.py
beetsplug/discogs.py
Update MusicBrainz plugin tests for new search behavior
  • Adjust RECORDING fixture to use the new response ID format
  • Monkeypatch get_recording_by_id to align test expectations with the new hook
test/plugins/test_musicbrainz.py

Possibly linked issues

  • #1234: The PR centralizes search functionality, making unidecode optional via a config setting, which directly resolves the issue of incorrect non-Latin search results in the Deezer plugin.
  • #123: The PR centralizes search_limit injection for Spotify, addressing the issue's request to improve search_limit handling.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it. You can also reply to a review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in the pull request body to generate a PR summary at any time exactly where you want it. You can also comment @sourcery-ai summary on the pull request to (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the pull request to resolve all Sourcery comments. Useful if you've already addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull request to dismiss all existing Sourcery reviews. Especially useful if you want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Sep 01 '25 11:09 sourcery-ai[bot]

@sourcery-ai review

snejus avatar Sep 01 '25 11:09 snejus

Codecov Report

:x: Patch coverage is 70.83333% with 21 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 67.50%. Comparing base (3a72d85) to head (36a6ee8). :white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
beetsplug/discogs.py 33.33% 8 Missing :warning:
beets/metadata_plugins.py 78.12% 7 Missing :warning:
beetsplug/spotify.py 61.53% 5 Missing :warning:
beetsplug/deezer.py 75.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5982      +/-   ##
==========================================
- Coverage   67.61%   67.50%   -0.12%     
==========================================
  Files         136      136              
  Lines       18521    18502      -19     
  Branches     3129     3129              
==========================================
- Hits        12523    12489      -34     
- Misses       5334     5348      +14     
- Partials      664      665       +1     
Files with missing lines Coverage Δ
beetsplug/musicbrainz.py 78.01% <100.00%> (+0.91%) :arrow_up:
beetsplug/deezer.py 17.96% <75.00%> (+1.30%) :arrow_up:
beetsplug/spotify.py 43.80% <61.53%> (-0.99%) :arrow_down:
beets/metadata_plugins.py 84.16% <78.12%> (+7.64%) :arrow_up:
beetsplug/discogs.py 71.68% <33.33%> (+0.90%) :arrow_up:

... and 1 file with indirect coverage changes

:rocket: New features to boost your workflow:
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Sep 01 '25 11:09 codecov[bot]

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.

github-actions[bot] avatar Nov 11 '25 05:11 github-actions[bot]