Improve error handling for color extraction
Includes a around embedding generation, which was happening twice (the first call was redundant as record embeddings are always regenerated after their parent artist embeddings are regenerated). Closes #144.
This commit is contained in:
@@ -3,6 +3,8 @@ defmodule MusicLibrary.Records do
|
|||||||
Provides function to work with records _irrespectively_ of their status as port of the collection or of the wishlist.
|
Provides function to work with records _irrespectively_ of their status as port of the collection or of the wishlist.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
require Logger
|
||||||
|
|
||||||
import Ecto.Query, warn: false
|
import Ecto.Query, warn: false
|
||||||
|
|
||||||
alias MusicLibrary.Artists
|
alias MusicLibrary.Artists
|
||||||
@@ -303,15 +305,28 @@ defmodule MusicLibrary.Records do
|
|||||||
enqueue_worker(Worker.RefreshCover, %{"id" => record.id}, record_meta(record))
|
enqueue_worker(Worker.RefreshCover, %{"id" => record.id}, record_meta(record))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp best_effort_extract_colors(record) do
|
||||||
|
case maybe_extract_colors(record) do
|
||||||
|
{:ok, record} ->
|
||||||
|
record
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
Logger.warning("Color extraction failed for record #{record.id}: #{inspect(reason)}")
|
||||||
|
record
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
defp maybe_extract_colors(%{dominant_colors: [_ | _]} = record), do: {:ok, record}
|
defp maybe_extract_colors(%{dominant_colors: [_ | _]} = record), do: {:ok, record}
|
||||||
defp maybe_extract_colors(record), do: extract_colors(record)
|
defp maybe_extract_colors(record), do: extract_colors(record)
|
||||||
|
|
||||||
@spec extract_colors(Record.t()) :: {:ok, Record.t()} | {:error, term()}
|
@spec extract_colors(Record.t()) :: {:ok, Record.t()} | {:error, term()}
|
||||||
def extract_colors(record) do
|
def extract_colors(record) do
|
||||||
asset = Assets.get!(record.cover_hash)
|
with asset when not is_nil(asset) <- Assets.get(record.cover_hash),
|
||||||
|
{:ok, colors} <- @color_extractor.extract_dominant_colors(asset.content) do
|
||||||
with {:ok, colors} <- @color_extractor.extract_dominant_colors(asset.content) do
|
|
||||||
update_record(record, %{dominant_colors: colors})
|
update_record(record, %{dominant_colors: colors})
|
||||||
|
else
|
||||||
|
nil -> {:error, :asset_not_found}
|
||||||
|
error -> error
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -392,10 +407,8 @@ defmodule MusicLibrary.Records do
|
|||||||
@spec create_record(map()) :: {:ok, Record.t()} | {:error, Ecto.Changeset.t()}
|
@spec create_record(map()) :: {:ok, Record.t()} | {:error, Ecto.Changeset.t()}
|
||||||
def create_record(attrs \\ %{}) do
|
def create_record(attrs \\ %{}) do
|
||||||
with {:ok, record} <- do_create_record(attrs) do
|
with {:ok, record} <- do_create_record(attrs) do
|
||||||
{:ok, record} = maybe_extract_colors(record)
|
|
||||||
generate_embedding_async(record)
|
|
||||||
|
|
||||||
record
|
record
|
||||||
|
|> best_effort_extract_colors()
|
||||||
|> Record.artist_ids()
|
|> Record.artist_ids()
|
||||||
|> Enum.each(fn artist_id ->
|
|> Enum.each(fn artist_id ->
|
||||||
Artists.refresh_artist_info_async(artist_id)
|
Artists.refresh_artist_info_async(artist_id)
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ defmodule MusicLibraryWeb.ErrorMessages do
|
|||||||
do: gettext("this record is already in your collection")
|
do: gettext("this record is already in your collection")
|
||||||
|
|
||||||
def friendly_message(:not_found), do: gettext("the resource was not found")
|
def friendly_message(:not_found), do: gettext("the resource was not found")
|
||||||
|
def friendly_message(:asset_not_found), do: gettext("the cover image asset was not found")
|
||||||
def friendly_message(:no_discogs_data), do: gettext("no Discogs profile available")
|
def friendly_message(:no_discogs_data), do: gettext("no Discogs profile available")
|
||||||
def friendly_message(:image_not_found), do: gettext("no image could be found")
|
def friendly_message(:image_not_found), do: gettext("no image could be found")
|
||||||
def friendly_message(:invalid_parameters), do: gettext("invalid parameters were provided")
|
def friendly_message(:invalid_parameters), do: gettext("invalid parameters were provided")
|
||||||
|
|||||||
@@ -2375,3 +2375,8 @@ msgstr ""
|
|||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Scan a record"
|
msgid "Scan a record"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/music_library_web/error_messages.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "the cover image asset was not found"
|
||||||
|
msgstr ""
|
||||||
|
|||||||
@@ -2375,3 +2375,8 @@ msgstr ""
|
|||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Scan a record"
|
msgid "Scan a record"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/music_library_web/error_messages.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "the cover image asset was not found"
|
||||||
|
msgstr ""
|
||||||
|
|||||||
@@ -51,6 +51,13 @@ defmodule MusicLibrary.RecordsTest do
|
|||||||
assert Enum.all?(record.dominant_colors, &color_hex?/1)
|
assert Enum.all?(record.dominant_colors, &color_hex?/1)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@tag :capture_log
|
||||||
|
test "succeeds when color extraction fails" do
|
||||||
|
record = record(dominant_colors: [], cover_hash: "nonexistent_hash")
|
||||||
|
|
||||||
|
assert record.dominant_colors == []
|
||||||
|
end
|
||||||
|
|
||||||
test "queues a task to retrieve artist info data" do
|
test "queues a task to retrieve artist info data" do
|
||||||
record =
|
record =
|
||||||
record(musicbrainz_data: release_group(:lockdown_trilogy))
|
record(musicbrainz_data: release_group(:lockdown_trilogy))
|
||||||
|
|||||||
Reference in New Issue
Block a user