To get artist info, try mbid and then artist name
This commit is contained in:
@@ -2,7 +2,9 @@ defmodule LastFm.APIBehaviour do
|
||||
alias LastFm.{Artist, Config, Track}
|
||||
|
||||
@type musicbrainz_id :: String.t()
|
||||
@type name :: String.t()
|
||||
@type config :: Config.t()
|
||||
@callback get_recent_tracks(config) :: {:ok, [Track.t()]} | {:error, String.t()}
|
||||
@callback get_artist_info(musicbrainz_id, config) :: {:ok, Artist.t()} | {:error, String.t()}
|
||||
@callback get_artist_info({:musicbrainz_id, musicbrainz_id} | {:name, name}, config) ::
|
||||
{:ok, Artist.t()} | {:error, String.t()}
|
||||
end
|
||||
|
||||
+17
-8
@@ -47,16 +47,25 @@ defmodule LastFm.APIImpl do
|
||||
end
|
||||
|
||||
@impl true
|
||||
def get_artist_info(artist_mbid, config) do
|
||||
options = [
|
||||
def get_artist_info({:musicbrainz_id, artist_mbid}, config) do
|
||||
do_get_artist_info([mbid: artist_mbid], config)
|
||||
end
|
||||
|
||||
def get_artist_info({:name, artist_name}, config) do
|
||||
do_get_artist_info([artist: artist_name], config)
|
||||
end
|
||||
|
||||
def do_get_artist_info(options, config) do
|
||||
base_options = [
|
||||
method: "artist.getInfo",
|
||||
api_key: config.api_key,
|
||||
user: config.user,
|
||||
mbid: artist_mbid,
|
||||
format: "json",
|
||||
limit: 50
|
||||
]
|
||||
|
||||
options = Keyword.merge(base_options, options)
|
||||
|
||||
url = @base_url <> "?" <> URI.encode_query(options)
|
||||
|
||||
Logger.debug("Fetching data from #{sanitize_url(url, config.api_key)}")
|
||||
@@ -68,12 +77,12 @@ defmodule LastFm.APIImpl do
|
||||
|> Map.get("artist")
|
||||
|> Artist.from_api_response()}
|
||||
|
||||
other ->
|
||||
error ->
|
||||
msg =
|
||||
"Failed to fetch data from #{sanitize_url(url, config.api_key)}, reason: #{inspect(other)}"
|
||||
"Failed to fetch data from #{sanitize_url(url, config.api_key)}, reason: #{inspect(error)}"
|
||||
|
||||
Logger.error(msg)
|
||||
{:error, msg}
|
||||
error
|
||||
end
|
||||
end
|
||||
|
||||
@@ -97,8 +106,8 @@ defmodule LastFm.APIImpl do
|
||||
String.replace(url, api_key, "<redacted_api_key>")
|
||||
end
|
||||
|
||||
defp identify_body({:ok, %{"error" => error_number, "message" => message}}) do
|
||||
{:error, "Error #{error_number}: #{message}"}
|
||||
defp identify_body({:ok, error = %{"error" => _error_number, "message" => _message}}) do
|
||||
{:error, error}
|
||||
end
|
||||
|
||||
defp identify_body(other), do: other
|
||||
|
||||
@@ -110,9 +110,24 @@ defmodule MusicLibrary.Records do
|
||||
Repo.all(q)
|
||||
end
|
||||
|
||||
def get_artist_info(musicbrainz_id) do
|
||||
def get_artist_info(artist) do
|
||||
last_fm_config = last_fm_config()
|
||||
last_fm_config.api.get_artist_info(musicbrainz_id, last_fm_config)
|
||||
# Sometimes the artist info cannot be identified with the MusicBrainz ID,
|
||||
# because Last.fm doesn't have that information. In that case, we try again with the artist name.
|
||||
case last_fm_config.api.get_artist_info(
|
||||
{:musicbrainz_id, artist.musicbrainz_id},
|
||||
last_fm_config
|
||||
) do
|
||||
{:ok, info} ->
|
||||
{:ok, info}
|
||||
|
||||
# TODO: remap error codes
|
||||
{:error, %{"error" => 6}} ->
|
||||
last_fm_config.api.get_artist_info({:name, artist.name}, last_fm_config)
|
||||
|
||||
error ->
|
||||
error
|
||||
end
|
||||
end
|
||||
|
||||
def get_cover(id) do
|
||||
|
||||
@@ -23,7 +23,7 @@ defmodule MusicLibraryWeb.ArtistLive.Show do
|
||||
|> assign(:artist, artist)
|
||||
|> assign(:artist_records, grouped_artist_records)
|
||||
|> assign_async(:artist_info, fn ->
|
||||
with {:ok, artist_info} <- Records.get_artist_info(musicbrainz_id) do
|
||||
with {:ok, artist_info} <- Records.get_artist_info(artist) do
|
||||
{:ok, %{artist_info: artist_info}}
|
||||
end
|
||||
end)
|
||||
|
||||
@@ -23,7 +23,8 @@ defmodule MusicLibraryWeb.ArtistLive.ShowTest do
|
||||
[artist] = collection_record.artists
|
||||
artist_musicbrainz_id = artist.musicbrainz_id
|
||||
|
||||
expect(APIBehaviourMock, :get_artist_info, fn ^artist_musicbrainz_id, _config ->
|
||||
expect(APIBehaviourMock, :get_artist_info, fn {:musicbrainz_id, ^artist_musicbrainz_id},
|
||||
_config ->
|
||||
{:ok,
|
||||
artist_get_info()
|
||||
|> Map.get("artist")
|
||||
|
||||
Reference in New Issue
Block a user