diff --git a/lib/last_fm/api_behaviour.ex b/lib/last_fm/api_behaviour.ex index 46b13b5f..e896db73 100644 --- a/lib/last_fm/api_behaviour.ex +++ b/lib/last_fm/api_behaviour.ex @@ -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 diff --git a/lib/last_fm/api_impl.ex b/lib/last_fm/api_impl.ex index e71c81a3..1e0c2c88 100644 --- a/lib/last_fm/api_impl.ex +++ b/lib/last_fm/api_impl.ex @@ -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, "") 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 diff --git a/lib/music_library/records.ex b/lib/music_library/records.ex index febb6ae4..2b19c6d3 100644 --- a/lib/music_library/records.ex +++ b/lib/music_library/records.ex @@ -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 diff --git a/lib/music_library_web/live/artist_live/show.ex b/lib/music_library_web/live/artist_live/show.ex index 4a8b4e0f..04921a26 100644 --- a/lib/music_library_web/live/artist_live/show.ex +++ b/lib/music_library_web/live/artist_live/show.ex @@ -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) diff --git a/test/music_library_web/live/artist_live/show_test.exs b/test/music_library_web/live/artist_live/show_test.exs index d8a3145e..d58ded89 100644 --- a/test/music_library_web/live/artist_live/show_test.exs +++ b/test/music_library_web/live/artist_live/show_test.exs @@ -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")