Centralize release id extraction

[snejus]

Description

Refactor: Centralize release ID extraction

This change introduces a new utility function extract_release_id in beets.util.id_extractors to handle the parsing of release IDs (or URLs containing IDs) for various metadata sources (Spotify, Deezer, Beatport, Discogs, MusicBrainz, Bandcamp, Tidal).

Key changes:

• Added extract_release_id function and PATTERN_BY_SOURCE regex dictionary.
• Converted MetadataSourcePlugin._get_id static method to an instance method which uses the data_source property to pick the correct id extractor.
• Removed the id_regex property and updated _get_id calls in all modules.
• Replaced old tests related to ID parsing in individual plugin test files with test/util/test_id_extractors.py that tests the extract_release_id function.

This refactoring simplifies the codebase, reduces redundancy, and makes it easier to manage and extend ID extraction logic for different sources.

[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.

[Copilot]

Pull Request Overview

This PR centralizes the extraction of release IDs by introducing a new utility function extract_release_id and replacing source‐specific regex handling with a common implementation. Key changes include:

• Adding and integrating extract_release_id into various plugins and test files.
• Removing redundant code and tests that previously handled ID extraction separately.
• Updating plugin methods to use the new extraction logic.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/util/test_id_extractors.py	Introduces tests for the centralized extract_release_id function.
test/test_plugins.py	Removes outdated tests related to individual ID regex extraction.
test/plugins/test_musicbrainz.py	Removes tests for the old _parse_id functionality.
test/plugins/test_discogs.py	Deletes legacy Discogs ID parsing tests in favor of the new method.
beetsplug/spotify.py	Updates album_for_id and track_for_id methods to use extract_release_id.
beetsplug/musicbrainz.py	Switches from _parse_id to extract_release_id; updates album/track lookups.
beetsplug/discogs.py	Replaces extract_discogs_id_regex with extract_release_id.
beetsplug/deezer.py	Refines ID extraction using the new utility with walrus operator.
beetsplug/beatport.py	Updates release ID extraction handling with the centralized approach.
beets/util/id_extractors.py	Defines extract_release_id and a PATTERN_BY_SOURCE mapping.
beets/plugins.py	Refactors _get_id to delegate ID extraction to extract_release_id.

[semohr]

Thanks for already implementing this!

Are we set on the functional approach here? I would prefer if this would be part of the plugins themself (e.g. defined as an required property or method in the abc). Is in my opinion closer to the actual logic and would required anyone who creates a new source plugin to properly register their ids.

I know not all plugins are SourcePlugins yet, but we might want to implement the shared parent class first before continuing with this. In my opinion such shared behavior (across all source plugins) is exactly a use case where abstract base classes shine bright.

[wisp3rwind]

Are we set on the functional approach here? I would prefer if this would be part of the plugins themself (e.g. defined as an required property or method in the abc). Is in my opinion closer to the actual logic and would required anyone who creates a new source plugin to properly register their ids.

👍 I only had a very superficial look at the PR, but I'd agree that this looks like plugins should register their ID patterns. Right now, beetsplug and beets are unexpectedly intertwined.

[snejus]

In an ideal world, I would also imagine that metadata source plugins have something like what @semohr suggested:

I would prefer if this would be part of the plugins themself (e.g. defined as an required property or method in the abc)

And they indeed originally had been part of the plugins, however they have been moved out #4633 in order to support saving external IDs by the musicbrainz plugin (see the relevant changes in beetsplug/musicbrainz.py in this PR).

Arguably, we may be able to remove this ID parsing from musicbrainz and instead just save the URLs? The code that will eventually make use of these IDs can parse them from URLs. Currently, we have no functionality that makes use of them.

On the other hand, what do we do about bandcamp and tidal id regexes? These plugins are not present in the codebase, so how would we parse them if we'd expect the plugins to define them?

[snejus]

I see. See my question from above:

On the other hand, what do we do about bandcamp and tidal id regexes? These plugins are not present in the codebase, so how would we parse them if we'd expect the plugins to define them?

[semohr]

These additional regexes are only used in the musicbrainz plugin, right? If this is the case, it wouldn't be too big of an issue to just have them as part of the mb plugin.

I'm not super set on this approach, but I generally think the extraction should be closer to the plugins.

[semohr]

I had a try at this myself and implemented the extraction on plugin level in an earlier commit of #5764.

To be honest my approach gets convoluted and incomprehensive quickly, even if working in theory. I would be happy to go with your inital approach for id extraction as it is more understandable and maintainable.

Could we move the extractors utils into the beetsplug folder tho? That way they are close to where the are used. I think that should be a decent compromise.

[snejus]

Could we move the extractors utils into the beetsplug folder tho? That way they are close to where the are used. I think that should be a decent compromise.

The extraction function is called from the generic MetadataSourcePlugin, i.e., it's imported in beets core beets.plugins. I think the consensus is that beets core does not import from beetsplug package since it's for plugins only.

[semohr]

The extraction function is called from the generic MetadataSourcePlugin, i.e., it's imported in beets core beets.plugins. I think the consensus is that beets core does not import from beetsplug package since it's for plugins only.

The _get_id method is not required by the MetadataSourcePlugin abstract base class—for example, the Chroma plugin does not use ID extraction at all. Furthermore, only concrete implementations of MetadataSourcePlugin make use of _get_id, which is somewhat of an antipattern in abstract class design.

• If this functionality is not needed for the general MetadataSourcePlugin interface (e.g. from the Beets core side), I would suggest omitting it from the base class entirely.
• I view this base class primarily as a contract that plugins must fulfill in order to be recognized as MetadataSourcePlugins.

Also, once we merge #5764 into this PR, the _get_id method should already be removed.

Apologies for the back and forth—this really feels more like a design decision we should get right to ensure it's future-proof than a simple refactor.

[snejus]

Apologies for the back and forth—this really feels more like a design decision we should get right to ensure it's future-proof than a simple refactor.

No need to apologize! All good 😆

The _get_id method is not required by the MetadataSourcePlugin abstract base class

_get_id is an internal plugin util rather than a public method which must exist in order to fulfil plugin's designed purpose (like candidates etc.). Each metadata source plugin requires it (musicbrainz and discogs use extract_release_id function directly since they don't inherit MetadataSourcePlugin). One of the reasons why I want to make these two inherit MetadataSourcePlugin is so that they use the unified _get_id implementation.

the Chroma plugin does not use ID extraction at all

Indeed - another reason why _get_id shouldn't be abstract.

I don't think we need to overthink this too much - this method simply follows DRY design and defines shared functionality within the base class.

[semohr]

I do not think thought _get_id should be abstract. I think it is probably not necessary at all if we define the patterns together in one place 😅

We should really think about moving the extractor utils into the beetsplug folder tho. It is kinda confusing that the regex patterns are defined in beets, because they are not directly used in beets but only by beetsplug i.e. inside specific plugin implementations.

Nonetheless Im still pretty happy with these enhancments as they are a mayor improvment over the previous behaviour. We can still move/remove it in favor of direct imports later.

[snejus]

Just need your approval @semohr if this looks good

[semohr]

Looks good to me 👍 Once we are done with this, It would be awesome to continue with #5787, since it partially depends on this PR.