From 53b89dc329026f4a3ff318da9eb5c5c378798fa9 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Wed, 18 Dec 2024 11:12:06 +0000 Subject: [PATCH] Extract Artists context --- lib/music_library.ex | 1 + lib/music_library/artists.ex | 64 +++++++++++++++++++ lib/music_library/records.ex | 58 ----------------- .../live/artist_live/show.ex | 6 +- .../live/stats_live/index.ex | 6 +- test/music_library/artists_test.exs | 17 +++++ test/music_library/records_test.exs | 11 ---- 7 files changed, 88 insertions(+), 75 deletions(-) create mode 100644 lib/music_library/artists.ex create mode 100644 test/music_library/artists_test.exs diff --git a/lib/music_library.ex b/lib/music_library.ex index 0be0e1f3..822ca0a6 100644 --- a/lib/music_library.ex +++ b/lib/music_library.ex @@ -2,6 +2,7 @@ defmodule MusicLibrary do @moduledoc """ Important contexts: + - `MusicLibrary.Artists` contains functions to access artists - `MusicLibrary.Records` contains functions to access and manipulate records _irrespectively_ of being in the collection or wishlist - `MusicLibrary.Collection` contains functions to access and manipulate records in the collection - `MusicLibrary.Wishlist` contains functions to access and manipulate records in the wishlist diff --git a/lib/music_library/artists.ex b/lib/music_library/artists.ex new file mode 100644 index 00000000..09266e0b --- /dev/null +++ b/lib/music_library/artists.ex @@ -0,0 +1,64 @@ +defmodule MusicLibrary.Artists do + import Ecto.Query, warn: false + alias MusicLibrary.Repo + + alias MusicLibrary.Records.ArtistRecord + + def get_artist!(musicbrainz_id) do + q = + from ar in ArtistRecord, + where: ar.musicbrainz_id == ^musicbrainz_id, + limit: 1, + select: ar.artist + + Repo.one!(q) + end + + def get_all_artist_ids do + q = from ar in ArtistRecord, distinct: true, select: ar.musicbrainz_id + + q |> Repo.all() |> MapSet.new() + end + + def get_artist_info(artist) do + last_fm_config = 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_similar_artists(artist) do + last_fm_config = 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_similar_artists( + {: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_similar_artists({:name, artist.name}, last_fm_config) + + error -> + error + end + end + + defp last_fm_config, do: LastFm.Config.resolve(:music_library) +end diff --git a/lib/music_library/records.ex b/lib/music_library/records.ex index bfdacaad..d74624be 100644 --- a/lib/music_library/records.ex +++ b/lib/music_library/records.ex @@ -90,22 +90,6 @@ defmodule MusicLibrary.Records do def get_record!(id), do: Repo.get!(Record, id) - def get_artist!(musicbrainz_id) do - q = - from ar in ArtistRecord, - where: ar.musicbrainz_id == ^musicbrainz_id, - limit: 1, - select: ar.artist - - Repo.one!(q) - end - - def get_all_artist_ids do - q = from ar in ArtistRecord, distinct: true, select: ar.musicbrainz_id - - q |> Repo.all() |> MapSet.new() - end - def get_artist_records(musicbrainz_id) do q = from r in Record, @@ -116,46 +100,6 @@ defmodule MusicLibrary.Records do Repo.all(q) end - def get_artist_info(artist) do - last_fm_config = 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_similar_artists(artist) do - last_fm_config = 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_similar_artists( - {: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_similar_artists({:name, artist.name}, last_fm_config) - - error -> - error - end - end - def get_cover(id) do q = from r in Record, @@ -299,6 +243,4 @@ defmodule MusicLibrary.Records do end defp music_brainz_config, do: MusicBrainz.Config.resolve(:music_library) - - defp last_fm_config, do: LastFm.Config.resolve(:music_library) end diff --git a/lib/music_library_web/live/artist_live/show.ex b/lib/music_library_web/live/artist_live/show.ex index 6f069350..78ea2492 100644 --- a/lib/music_library_web/live/artist_live/show.ex +++ b/lib/music_library_web/live/artist_live/show.ex @@ -1,7 +1,7 @@ defmodule MusicLibraryWeb.ArtistLive.Show do use MusicLibraryWeb, :live_view - alias MusicLibrary.Records + alias MusicLibrary.{Artists, Records} @impl true def mount(_params, _session, socket) do @@ -10,7 +10,7 @@ defmodule MusicLibraryWeb.ArtistLive.Show do @impl true def handle_params(%{"musicbrainz_id" => musicbrainz_id}, _, socket) do - artist = Records.get_artist!(musicbrainz_id) + artist = Artists.get_artist!(musicbrainz_id) grouped_artist_records = musicbrainz_id @@ -24,7 +24,7 @@ defmodule MusicLibraryWeb.ArtistLive.Show do # TODO: make it a stream |> assign(:artist_records, grouped_artist_records) |> assign_async(:artist_info, fn -> - with {:ok, artist_info} <- Records.get_artist_info(artist) do + with {:ok, artist_info} <- Artists.get_artist_info(artist) do {:ok, %{artist_info: artist_info}} end end) diff --git a/lib/music_library_web/live/stats_live/index.ex b/lib/music_library_web/live/stats_live/index.ex index 70937982..ef23e16e 100644 --- a/lib/music_library_web/live/stats_live/index.ex +++ b/lib/music_library_web/live/stats_live/index.ex @@ -3,7 +3,7 @@ defmodule MusicLibraryWeb.StatsLive.Index do import MusicLibraryWeb.StatsLive.DataComponents - alias MusicLibrary.{Collection, Records, Wishlist} + alias MusicLibrary.{Artists, Collection, Records, Wishlist} alias Records.Record def mount(_params, _session, socket) do @@ -25,7 +25,7 @@ defmodule MusicLibraryWeb.StatsLive.Index do collected_release_ids = Collection.collected_release_ids(release_ids) wishlisted_release_ids = Wishlist.wishlisted_release_ids(release_ids) - artist_ids = Records.get_all_artist_ids() + artist_ids = Artists.get_all_artist_ids() if connected?(socket) do LastFm.Feed.subscribe() @@ -88,7 +88,7 @@ defmodule MusicLibraryWeb.StatsLive.Index do collected_release_ids = Collection.collected_release_ids(release_ids) wishlisted_release_ids = Wishlist.wishlisted_release_ids(release_ids) - artist_ids = Records.get_all_artist_ids() + artist_ids = Artists.get_all_artist_ids() {:noreply, socket diff --git a/test/music_library/artists_test.exs b/test/music_library/artists_test.exs new file mode 100644 index 00000000..6faffd4c --- /dev/null +++ b/test/music_library/artists_test.exs @@ -0,0 +1,17 @@ +defmodule MusicLibrary.ArtistsTest do + use MusicLibrary.DataCase + + alias MusicLibrary.Artists + import MusicLibrary.RecordsFixtures + + describe "get_artist/1" do + test "it returns records with essential data" do + record = record_fixture() + [expected] = record.artists + + artist = Artists.get_artist!(expected.musicbrainz_id) + + assert expected == artist + end + end +end diff --git a/test/music_library/records_test.exs b/test/music_library/records_test.exs index 1867bade..57fe5ea8 100644 --- a/test/music_library/records_test.exs +++ b/test/music_library/records_test.exs @@ -168,17 +168,6 @@ defmodule MusicLibrary.RecordsTest do end end - describe "get_artist/1" do - test "it returns records with essential data" do - record = record_fixture() - [expected] = record.artists - - artist = Records.get_artist!(expected.musicbrainz_id) - - assert expected == artist - end - end - describe "get_cover/1" do test "it returns the record cover by id" do # while this test may seem redundant, it implicitely checks that ALL record fields are returned,