Spotify provider

[phw]

This implements a Spotify provider (closes #16).

Similar to Tidal this requires API credentials, which can be setup on https://developer.spotify.com/dashboard/ . The API key and secret must be provided by the environment variables HARMONY_SPOTIFY_CLIENT_ID and HARMONY_SPOTIFY_CLIENT_SECRET.

I addressed all the points I brought up in #16. About the linked track thing see my comment #16 (comment)

Loading a release with more than 50 tracks requires additional requests. This is a large one with 100 tracks across 7 media: https://open.spotify.com/intl-de/album/2ey9jImi467qEu67fvW1kP

The copyright info is returned differently:

• For https://open.spotify.com/intl-de/album/3qW1oPucRFfnpYGmYtDQnZ there is both a P and C copyright, but both are entered as "2023 LORD FORGIVE MY SINS" without a symbol
• For e.g. https://open.spotify.com/intl-de/album/4aawyAB9vmqN3uQ7FjRGTy there is only a P copyright with the text "(P) 2012 RCA Records, a division of Sony Music Entertainment", containing the ASCII version of the symbol.
• For https://open.spotify.com/intl-de/album/5H7ixXZfsNMGbIE5OBSpcb both P and C are with the symbol as "© 2024 Taylor Swift" and "℗ 2024 Taylor Swift"

The provider normalizes all cases to start with the © or ℗ symbol, depending on the type.

A few things we might consider for future refinement, but which I think should be put into separate PRs:

• For now the available regions are hardcoded as they are for other providers. But for Spotify we could load them dynamically via the https://developer.spotify.com/documentation/web-api/reference/get-available-markets endpoint. But this probably requires some more extensive refactoring.
• The authentication with OAuth2 client credentials flow is very similar between Tidal and Spotify. I partially applied some refactoring to have at least the caching of the access token centralized, so not every implementation invents its own solution. But I think there could be more generalization.

[phw]

We can factor out the copyright normalization logic and reuse it for other providers, e.g. as suggested for Tidal in https://community.metabrainz.org/t/harmony-music-metadata-aggregator-and-musicbrainz-importer/698641/15

But I'd do this after merging this PR. It also needs some further research. I know Tidal includes the copyright text both with and without the © symbol. What I'm unsure is whether this strictly contains copyright © info, or whether it also can sometimes contain phonographic copyright ℗ info. Spotify has those separated, which makes it easier.

[kellnerd]

Thank you very much for leading the development of new providers!
Code looks good, I will do some testing later today and I have a few questions as usual.

[kellnerd]

Regarding the maximum resolution, there are good news: qsniyg/maxurl#1329
But I guess we will keep the 640 pixels here until IMU officially supports larger resolutions.

[kellnerd]

I have done a bit more testing, especially related to the barcodes.
Unfortunately Spotify's API always seems to do a string comparison for these, so we won't get away with always padding the GTIN to 14 digits before doing a (first and only) search. Maybe we even have to add the 13 digit case back, we will see. In any case padding to 13 digits should be the last attempt as it is the rarest case.

I also wanted to note that the provider is currently not rate limited, probably because the Spotify docs don't specify a specific quota, only the length (30s) of the rate limit interval.
We should probably handle 429 errors and make use their header to implement a retry strategy, but this is rather a general feature which other providers also could make use of.

[phw]

Maybe we even have to add the 13 digit case back, we will see. In any case padding to 13 digits should be the last attempt as it is the rarest case.

I added it back in 75fe532, but refactored the code to be more readable and also try 13 character padding last. This should handle the eventual case of 13 characters without complicating the code or requiring unnecessary requests.

[kellnerd]

That was a long review phase, but I am very happy with the outcome, especially that we have solved one longstanding issue of a-tisket. One last issue and this is good to go.

I think the current strategy to iterate over regions before GTIN barcodes makes perfect sense. We probably still do less requests than a-tisket on average and in the long run we could limit the number of fallback regions which are considered.
For now the users can specify as many as they want, but they will be filtered by available regions of releases whose providers have already been looked up by URL:

harmony/lookup.ts

Lines 246 to 259 in 85414bd

	// Remove all preferred regions where the release is unlikely to be available.
	if (this.options.regions?.size) {
	const availablePreferredRegions = new Set(this.options.regions);
	for (const region of availablePreferredRegions) {
	if (!availableRegions.has(region)) {
	availablePreferredRegions.delete(region);
	}
	}
	this.options.regions = availablePreferredRegions;
	}
	// Use available regions if no (available) preferred regions remain.
	if (!this.options.regions?.size) {
	this.options.regions = availableRegions;
	}

[kellnerd]

The market should only be set for GTIN lookups, otherwise Harmony will use the region which it extracted from the intl-xx part of the Spotify URL for URL/ID lookups:

Suggested change

	if (this.lookup.region) {
	query.set('market', this.lookup.region);
	}
	if (this.lookup.method === 'gtin') {
	lookupUrl = new URL(`search`, this.provider.apiBaseUrl);
	query.set('type', 'album');
	query.set('q', `upc:${this.lookup.value}`);
	if (this.lookup.method === 'gtin') {
	lookupUrl = new URL(`search`, this.provider.apiBaseUrl);
	query.set('type', 'album');
	query.set('q', `upc:${this.lookup.value}`);
	if (this.lookup.region) {
	query.set('market', this.lookup.region);
	}

With this change it is also no longer necessary to reset this.lookup.region to undefined after the initial GTIN search.

P.S. Is the intl-xx part actually a country code or a language code?
I made this mistake for the Deezer provider where I only noticed it when iTunes requests for the region EN were failing 😂
For Spotify it is a bit unclear, MBS-13052 assumes it is a language, but for all cases which I have tried where language code and country code would be different, Spotify always seems to redirect to the URL without the intl part (en vs us or gb, sv vs se), so I can't tell for sure!?
If it is a language, the URLPattern should not use :region but :language or something else, but as it stands it is currently not necessary to change something because all intl codes which I have seen so far are valid region codes.

[kellnerd]

After doing a few more tests (documented in the comments of the MBS ticket) and despite lots of indications making me think the opposite, I am now confident this is a language code (intl-ar is for Arabic and not for Argentina).

[phw]

See 72bbf23

Regarding the code: I honestly don't know. I always interpreted it as the market code. But actually it is hard to test. It nearly always redirects to as you say. The redirection seems to be controlled by the Accept-Language header, so that's an argument it is a language and not a market. Do you know any other intl-* paths being used?

curl -v -H "Accept-Language: fi" https://open.spotify.com/intl-de/album/2Usbb1Ao4tpzDY0IbIQHqB

< HTTP/2 302 
< location: /album/2Usbb1Ao4tpzDY0IbIQHqB

curl -v -H "Accept-Language: de" https://open.spotify.com/album/2Usbb1Ao4tpzDY0IbIQHqB

< HTTP/2 302 
< location: /intl-de/album/2Usbb1Ao4tpzDY0IbIQHqB

For curl -v -H "Accept-Language: de" https://open.spotify.com/intl-de/album/2Usbb1Ao4tpzDY0IbIQHqB there is the no redirect.

I tested the above with a server located in Finland, just to be sure it behaves the same independent of German location.

I changed it to be :language for now. As you say it shouldn't matter, but at least it avoids any issue if this being the region turns out to be wrong.

[kellnerd]

I have also tested the behavior if no credentials are specified and did a similar improvement for the Tidal provider to show the underlying error instead of the generic one: