Migrate from embedded covers to assets

lib/music_library/artists/artist_info.ex

@@ -3,7 +3,7 @@ defmodule MusicLibrary.Artists.ArtistInfo do
 
   import Ecto.Changeset
 
-  alias MusicLibrary.Records.Cover
+  alias MusicLibrary.Assets.Asset
 
   @primary_key {:id, :binary_id, autogenerate: true}
   schema "artist_infos" do
@@ -49,7 +49,7 @@ defmodule MusicLibrary.Artists.ArtistInfo do
   end
 
   def generate_image_hash(%__MODULE__{image_data: image_data} = artist_info) do
-    change(artist_info, image_data_hash: Cover.hash(image_data))
+    change(artist_info, image_data_hash: Asset.hash(image_data))
   end
 
   def generate_image_hash(changeset) do
@@ -58,7 +58,7 @@ defmodule MusicLibrary.Artists.ArtistInfo do
         changeset
 
       image_data ->
-        put_change(changeset, :image_data_hash, Cover.hash(image_data))
+        put_change(changeset, :image_data_hash, Asset.hash(image_data))
     end
   end
 

lib/music_library/assets/asset.ex

@@ -60,7 +60,7 @@ defmodule MusicLibrary.Assets.Asset do
     }
   end
 
-  defp hash(content) do
+  def hash(content) do
     :crypto.hash(:sha256, content) |> Base.encode16()
   end
 end

lib/music_library/records.ex

@@ -148,15 +148,6 @@ defmodule MusicLibrary.Records do
     Repo.all(q)
   end
 
-  def get_cover(id) do
-    q =
-      from r in Record,
-        where: r.id == ^id,
-        select: %{cover_data: r.cover_data, cover_hash: r.cover_hash}
-
-    Repo.one(q)
-  end
-
   def import_from_musicbrainz_release(musicbrainz_id, opts \\ []) do
     case MusicBrainz.get_release(musicbrainz_id) do
       {:ok, release} ->
@@ -175,10 +166,11 @@ defmodule MusicLibrary.Records do
 
     with {:ok, release_group} <- MusicBrainz.get_release_group(musicbrainz_id),
          {:ok, release_group_with_releases} <- merge_releases(musicbrainz_id, release_group),
-         {:ok, cover_data} <- get_cover_art_or_default(musicbrainz_id) do
+         {:ok, cover_data} <- get_cover_art_or_default(musicbrainz_id),
+         {:ok, asset} <- Assets.store_image(%{content: cover_data, format: "image/jpeg"}) do
       release_group_with_releases
       |> build_record_attrs(%{
-        "cover_data" => cover_data,
+        "cover_hash" => asset.hash,
         "format" => format,
         "purchased_at" => purchased_at,
         "selected_release_id" => selected_release_id
@@ -239,11 +231,11 @@ defmodule MusicLibrary.Records do
   end
 
   def refresh_cover(record) do
-    with {:ok, cover_data} <- MusicBrainz.get_cover_art({:url, record.cover_url}) do
+    with {:ok, cover_data} <- MusicBrainz.get_cover_art({:url, record.cover_url}),
-      {:ok, thumb_data} = Cover.resize(cover_data)
+         {:ok, thumb_data} <- Cover.resize(cover_data),
-
+         {:ok, asset} <- Assets.store_image(%{content: thumb_data, format: "image/jpeg"}) do
       record
-      |> Record.add_cover_data(thumb_data)
+      |> Record.set_cover_hash(asset.hash)
       |> Repo.update()
     end
   end
@@ -267,11 +259,12 @@ defmodule MusicLibrary.Records do
   end
 
   def resize_cover(record) do
-    {:ok, thumb_data} = Cover.resize(record.cover_data)
+    with {:ok, thumb_data} <- Cover.resize(record.cover_data),
-
+         {:ok, asset} <- Assets.store_image(%{content: thumb_data, format: "image/jpeg"}) do
-    record
+      record
-    |> Record.add_cover_data(thumb_data)
+      |> Record.set_cover_hash(asset.hash)
-    |> Repo.update()
+      |> Repo.update()
+    end
   end
 
   def refresh_musicbrainz_data(record) do

lib/music_library/records/batch.ex

@@ -2,7 +2,7 @@ defmodule MusicLibrary.Records.Batch do
   import Ecto.Query
 
   alias MusicLibrary.Records
-  alias MusicLibrary.Records.{Cover, Record}
+  alias MusicLibrary.Records.Record
   alias MusicLibrary.Repo
 
   require Logger
@@ -21,18 +21,6 @@ defmodule MusicLibrary.Records.Batch do
     end)
   end
 
-  def refresh_old_artwork do
-    run_on_all_records(&refresh_old_artwork/1)
-  end
-
-  def refresh_old_artwork(record) do
-    if Cover.correct_size?(record.cover_data) do
-      :ok
-    else
-      MusicLibrary.Records.refresh_cover(record)
-    end
-  end
-
   defp run_on_all_records(fun) do
     q = from(r in Record)
     stream = Repo.stream(q, max_rows: 50)

lib/music_library/records/cover.ex

@@ -15,10 +15,6 @@ defmodule MusicLibrary.Records.Cover do
     Image.write_to_buffer(thumb, ".jpg")
   end
 
-  def hash(cover_data) do
-    :crypto.hash(:sha256, cover_data) |> Base.encode16()
-  end
-
   def correct_size?(cover_data) do
     {:ok, image} = Image.new_from_buffer(cover_data)
 

lib/music_library/records/record.ex

@@ -5,8 +5,6 @@ defmodule MusicLibrary.Records.Record do
 
   alias MusicBrainz.{Release, ReleaseGroup}
   alias MusicLibrary.Artists.Artist
-  alias MusicLibrary.Colors.ColorFrequencyExtractor
-  alias MusicLibrary.Records.Cover
 
   @formats [:cd, :backup, :vinyl, :blu_ray, :dvd, :multi]
   @types [:album, :ep, :live, :compilation, :single, :other]
@@ -18,7 +16,6 @@ defmodule MusicLibrary.Records.Record do
     field :format, Ecto.Enum, values: @formats
     field :title, :string
     field :cover_url, :string
-    field :cover_data, :binary
     field :cover_hash, :string
     field :musicbrainz_id, Ecto.UUID
     field :musicbrainz_data, :map, default: %{}
@@ -128,38 +125,24 @@ defmodule MusicLibrary.Records.Record do
       :selected_release_id,
       :included_release_group_ids,
       :cover_url,
-      :cover_data,
+      :cover_hash,
       :dominant_colors,
       :purchased_at
     ])
     |> cast_embed(:artists)
     |> validate_required([:type, :title, :musicbrainz_id, :genres])
     |> unique_constraint(:musicbrainz_id, name: "records_musicbrainz_id_format_index")
-    |> generate_cover_hash()
-    |> maybe_generate_dominant_colors()
     |> update_release_ids()
     |> update_included_release_group_ids()
   end
 
-  defp maybe_generate_dominant_colors(changeset) do
-    case get_change(changeset, :dominant_colors) do
-      nil ->
-        generate_dominant_colors(changeset)
-
-      _dominant_colors ->
-        changeset
-    end
-  end
-
   def add_genres(record, genres) do
     change(record, genres: genres)
   end
 
-  def add_cover_data(record, cover_data) do
+  def set_cover_hash(record, cover_hash) do
     record
-    |> change(cover_data: cover_data)
+    |> change(cover_hash: cover_hash)
-    |> generate_cover_hash()
-    |> generate_dominant_colors()
   end
 
   def add_musicbrainz_data(record, musicbrainz_data) do
@@ -171,38 +154,6 @@ defmodule MusicLibrary.Records.Record do
     |> update_included_release_group_ids()
   end
 
-  def generate_cover_hash(%__MODULE__{cover_data: cover_data} = record) do
-    change(record, cover_hash: Cover.hash(cover_data))
-  end
-
-  def generate_cover_hash(changeset) do
-    case get_change(changeset, :cover_data) do
-      nil ->
-        changeset
-
-      cover_data ->
-        put_change(changeset, :cover_hash, Cover.hash(cover_data))
-    end
-  end
-
-  def generate_dominant_colors(%__MODULE__{cover_data: cover_data} = record) do
-    change(record, dominant_colors: ColorFrequencyExtractor.extract_dominant_colors!(cover_data))
-  end
-
-  def generate_dominant_colors(changeset) do
-    case get_change(changeset, :cover_data) do
-      nil ->
-        changeset
-
-      cover_data ->
-        put_change(
-          changeset,
-          :dominant_colors,
-          ColorFrequencyExtractor.extract_dominant_colors!(cover_data)
-        )
-    end
-  end
-
   def rotate_dominant_colors(%__MODULE__{dominant_colors: dominant_colors} = record) do
     change(record, dominant_colors: rotate(dominant_colors))
   end

lib/music_library/worker/extract_colors.ex

@@ -1,14 +1,15 @@
 defmodule MusicLibrary.Worker.ExtractColors do
   use Oban.Worker, queue: :heavy_writes, max_attempts: 3
 
-  alias MusicLibrary.{Colors, Records}
+  alias MusicLibrary.{Assets, Colors, Records}
 
   @impl Oban.Worker
   def perform(%Oban.Job{args: %{"id" => record_id, "method" => method}}) do
     record = MusicLibrary.Records.get_record!(record_id)
+    asset = Assets.get!(record.cover_hash)
     method = String.to_existing_atom(method)
 
-    with {:ok, colors} <- Colors.extract_colors(record.cover_data, method),
+    with {:ok, colors} <- Colors.extract_colors(asset.content, method),
          {:ok, updated_record} <- Records.update_record(record, %{dominant_colors: colors}) do
       MusicLibrary.Records.notify_update(updated_record)
     end

lib/music_library_web/components/record_components.ex

@@ -14,7 +14,7 @@ defmodule MusicLibraryWeb.RecordComponents do
       class={@class}
       alt={@record.title}
       loading="lazy"
-      src={~p"/covers/#{@record.id}?vsn=#{@record.cover_hash}"}
+      src={~p"/covers/#{@record.cover_hash}"}
     />
     """
   end

lib/music_library_web/components/record_form_component.ex

@@ -199,7 +199,7 @@ defmodule MusicLibraryWeb.RecordFormComponent do
 
         [cover_params] ->
           {:ok, asset} = Assets.store_image(cover_params)
-          Map.put(record_params, "cover_data", asset.content)
+          Map.put(record_params, "cover_hash", asset.hash)
       end
 
     case Records.update_record(socket.assigns.record, params) do

lib/music_library_web/controllers/artist_controller.ex

@@ -31,11 +31,11 @@ defmodule MusicLibraryWeb.ArtistController do
     |> send_resp(304, "")
   end
 
-  defp respond_with_cache(conn, cover_data, etag) do
+  defp respond_with_cache(conn, image_data, etag) do
     conn
     |> put_resp_content_type("image/jpeg", "utf-8")
     |> put_resp_header("cache-control", "public, max-age=#{@cache_duration}")
     |> put_resp_header("etag", etag)
-    |> send_resp(200, cover_data)
+    |> send_resp(200, image_data)
   end
 end

lib/music_library_web/controllers/collection_json.ex

@@ -19,8 +19,8 @@ defmodule MusicLibraryWeb.CollectionJSON do
       id: record.id,
       artists: Enum.map(record.artists, & &1.name),
       title: record.title,
-      cover_url: url(~p"/api/covers/#{record.id}?#{[vsn: record.cover_hash]}"),
+      cover_url: url(~p"/api/covers/#{record.cover_hash}"),
-      thumb_url: url(~p"/api/covers/#{record.id}?#{[vsn: record.cover_hash, size: 480]}")
+      thumb_url: url(~p"/api/covers/#{record.cover_hash}?#{[size: 480]}")
     }
   end
 end

lib/music_library_web/controllers/cover_controller.ex

@@ -2,6 +2,7 @@ defmodule MusicLibraryWeb.CoverController do
   use MusicLibraryWeb, :controller
 
   alias MusicLibrary.Assets
+  alias MusicLibrary.Assets.Asset
   alias MusicLibrary.Records.Cover
 
   # 1 year in seconds
@@ -15,7 +16,7 @@ defmodule MusicLibraryWeb.CoverController do
       %{content: content} ->
         # TODO: find a way to cache computation, or pre-compute thumb and store it
         {:ok, thumb_data} = Cover.resize(content, String.to_integer(size))
-        hash = Cover.hash(thumb_data)
+        hash = Asset.hash(thumb_data)
 
         case get_req_header(conn, "if-none-match") do
           [^hash] -> extend_cache(conn)
@@ -57,11 +58,11 @@ defmodule MusicLibraryWeb.CoverController do
     |> send_resp(200, asset.content)
   end
 
-  defp respond_with_cache(conn, cover_data, etag) do
+  defp respond_with_cache(conn, data, etag) do
     conn
     |> put_resp_content_type("image/jpeg", "utf-8")
     |> put_resp_header("cache-control", "public, max-age=#{@cache_duration}")
     |> put_resp_header("etag", etag)
-    |> send_resp(200, cover_data)
+    |> send_resp(200, data)
   end
 end

test/music_library/records_test.exs

@@ -5,8 +5,9 @@ defmodule MusicLibrary.RecordsTest do
   import MusicBrainz.Fixtures.ReleaseGroup
   import MusicLibrary.Fixtures.Records
 
+  alias MusicLibrary.Assets
   alias MusicLibrary.Records
-  alias MusicLibrary.Records.SearchIndex
+  alias MusicLibrary.Records.{Cover, SearchIndex}
 
   defp create_records(_) do
     records = [
@@ -208,16 +209,6 @@ defmodule MusicLibrary.RecordsTest do
     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,
-      # as opposed to other code paths where we only return essential ones.
-      expected = record()
-
-      assert Map.take(expected, [:cover_hash, :cover_data]) == Records.get_cover(expected.id)
-    end
-  end
-
   describe "import_from_musicbrainz_release_group/2" do
     test "it saves a record with its cover art" do
       current_time = DateTime.utc_now()
@@ -342,10 +333,12 @@ defmodule MusicLibrary.RecordsTest do
 
       assert {:ok, updated_record} = Records.refresh_cover(record)
 
-      assert updated_record.cover_data !== record.cover_data
-
       assert updated_record.cover_hash ==
                "6E0D25D1FD1019D771D7EB3F777E2C7C1B06A73A92E56A584D674D86DD8AF441"
+
+      {:ok, expected_content} = Cover.resize(raven_cover_data())
+
+      assert Assets.get(updated_record.cover_hash).content == expected_content
     end
   end
 end

test/music_library_web/controllers/collection_controller_test.exs

@@ -31,10 +31,8 @@ defmodule MusicLibraryWeb.CollectionControllerTest do
                "id" => record.id,
                "artists" => ["Steven Wilson"],
                "title" => record.title,
-               "cover_url" =>
+               "cover_url" => "http://localhost:4002/api/covers/#{record.cover_hash}",
-                 "http://localhost:4002/api/covers/#{record.id}?vsn=#{record.cover_hash}",
+               "thumb_url" => "http://localhost:4002/api/covers/#{record.cover_hash}?size=480"
-               "thumb_url" =>
-                 "http://localhost:4002/api/covers/#{record.id}?vsn=#{record.cover_hash}&size=480"
              }
     end
   end
@@ -59,10 +57,8 @@ defmodule MusicLibraryWeb.CollectionControllerTest do
                "id" => record.id,
                "artists" => ["Steven Wilson"],
                "title" => record.title,
-               "cover_url" =>
+               "cover_url" => "http://localhost:4002/api/covers/#{record.cover_hash}",
-                 "http://localhost:4002/api/covers/#{record.id}?vsn=#{record.cover_hash}",
+               "thumb_url" => "http://localhost:4002/api/covers/#{record.cover_hash}?size=480"
-               "thumb_url" =>
-                 "http://localhost:4002/api/covers/#{record.id}?vsn=#{record.cover_hash}&size=480"
              }
     end
   end
@@ -91,10 +87,8 @@ defmodule MusicLibraryWeb.CollectionControllerTest do
                    "id" => record.id,
                    "artists" => ["Steven Wilson"],
                    "title" => record.title,
-                   "cover_url" =>
+                   "cover_url" => "http://localhost:4002/api/covers/#{record.cover_hash}",
-                     "http://localhost:4002/api/covers/#{record.id}?vsn=#{record.cover_hash}",
+                   "thumb_url" => "http://localhost:4002/api/covers/#{record.cover_hash}?size=480"
-                   "thumb_url" =>
-                     "http://localhost:4002/api/covers/#{record.id}?vsn=#{record.cover_hash}&size=480"
                  }
                ]
              }

test/music_library_web/controllers/cover_controller_test.exs

@@ -4,7 +4,6 @@ defmodule MusicLibraryWeb.CoverControllerTest do
   import MusicLibrary.Fixtures.Records
 
   alias MusicLibrary.Assets
-  alias MusicLibrary.Records.Cover
 
   defp create_asset(_config) do
     {:ok, asset} = Assets.store(%{content: marbles_cover_data(), format: "image/jpeg"})
@@ -65,7 +64,7 @@ defmodule MusicLibraryWeb.CoverControllerTest do
       conn = get(conn, ~p"/covers/#{asset.hash}?size=480")
 
       thumb = marbles_thumb_data()
-      hash = Cover.hash(thumb)
+      hash = Assets.Asset.hash(thumb)
 
       assert conn.status == 200
       assert get_resp_header(conn, "content-type") == ["image/jpeg; charset=utf-8"]

test/music_library_web/live/collection_live/index_test.exs

@@ -7,6 +7,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
   import MusicLibraryWeb.RecordComponents, only: [format_label: 1, type_label: 1]
 
   alias MusicBrainz.ReleaseGroupSearchResult
+  alias MusicLibrary.Assets
   alias MusicLibrary.Records.{Cover, Record}
 
   # make it a multiple of 4 for easier calculations
@@ -50,7 +51,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
       session = visit(conn, ~p"/collection?order=alphabetical&page_size=#{page_size}")
 
       for record <- expected_present do
-        cover_url = ~p"/covers/#{record.id}?vsn=#{record.cover_hash}"
+        cover_url = ~p"/covers/#{record.cover_hash}"
 
         session
         |> assert_has("#records-#{record.id}")
@@ -139,7 +140,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
       session =
         visit(conn, ~p"/collection?#{qs}")
 
-      cover_url = ~p"/covers/#{record.id}?vsn=#{record.cover_hash}"
+      cover_url = ~p"/covers/#{record.cover_hash}"
 
       session
       |> assert_has("#records-#{record.id}")
@@ -181,7 +182,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
         visit(conn, ~p"/collection?#{qs}")
 
       for record <- present do
-        cover_url = ~p"/covers/#{record.id}?vsn=#{record.cover_hash}"
+        cover_url = ~p"/covers/#{record.cover_hash}"
 
         session
         |> assert_has("#records-#{record.id}")
@@ -217,8 +218,8 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
     end
 
     test "can change the record cover", %{conn: conn} do
-      record = record(cover_data: marbles_cover_data())
+      record = record()
-      cover_url = ~p"/covers/#{record.id}?vsn=#{record.cover_hash}"
+      cover_url = ~p"/covers/#{record.cover_hash}"
 
       session =
         conn
@@ -231,10 +232,10 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
         |> click_button("Save")
         |> assert_has("p", text: "Record updated successfully")
 
-      updated_cover = MusicLibrary.Records.get_cover(record.id)
+      updated_record = MusicLibrary.Records.get_record!(record.id)
-      updated_cover_url = ~p"/covers/#{record.id}?vsn=#{updated_cover.cover_hash}"
+      updated_cover_url = ~p"/covers/#{updated_record.cover_hash}"
 
-      assert updated_cover.cover_hash !== record.cover_hash
+      assert updated_record.cover_hash !== record.cover_hash
       assert_has(session, "img[src='#{updated_cover_url}']")
     end
   end
@@ -317,7 +318,9 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
 
       {:ok, resized_cover_data} = Cover.resize(cover_data)
 
-      assert record.cover_data == resized_cover_data
+      assets = Assets.get(record.cover_hash)
+
+      assert assets.content == resized_cover_data
 
       assert record.inserted_at !== nil
       assert record.updated_at !== nil
@@ -420,9 +423,10 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
     assert record.cover_hash ==
              "E7238C742E5B8711FC5BFF01A4A1F727D9E404A4D1420429A6B37ABFFC0B5960"
 
+    asset = Assets.get(record.cover_hash)
     {:ok, resized_cover_data} = Cover.resize(cover_data)
 
-    assert record.cover_data == resized_cover_data
+    assert asset.content == resized_cover_data
 
     assert record.inserted_at !== nil
     assert record.updated_at !== nil

test/music_library_web/live/collection_live/show_test.exs

@@ -29,7 +29,7 @@ defmodule MusicLibraryWeb.CollectionLive.ShowTest do
   describe "Show record" do
     test "it includes all needed information", %{conn: conn} do
       record = record()
-      cover_url = ~p"/covers/#{record.id}?vsn=#{record.cover_hash}"
+      cover_url = ~p"/covers/#{record.cover_hash}"
 
       release_response = Fixtures.Release.release(:marbles)
 

test/music_library_web/live/wishlist_live/show_test.exs

@@ -21,7 +21,7 @@ defmodule MusicLibraryWeb.WishlistLive.ShowTest do
   describe "Show record" do
     test "it includes all needed information", %{conn: conn} do
       record = record(purchased_at: nil)
-      cover_url = ~p"/covers/#{record.id}?vsn=#{record.cover_hash}"
+      cover_url = ~p"/covers/#{record.cover_hash}"
 
       session =
         conn

test/support/fixtures/music_library/records.ex

@@ -5,6 +5,8 @@ defmodule MusicLibrary.Fixtures.Records do
   """
 
   alias MusicBrainz.Fixtures.ReleaseGroup
+  alias MusicLibrary.Assets
+  alias MusicLibrary.Assets.Asset
   alias MusicLibrary.Records.Record
 
   @genres [
@@ -51,6 +53,10 @@ defmodule MusicLibrary.Fixtures.Records do
   @marbles_thumb_data File.read!(@marbles_thumb_data_path)
   @raven_cover_data File.read!(@raven_cover_data_path)
 
+  @marbles_cover_hash Asset.hash(@marbles_cover_data)
+  @marbles_thumb_hash Asset.hash(@marbles_thumb_data)
+  @raven_cover_hash Asset.hash(@raven_cover_data)
+
   def marbles_cover_fixture, do: @marbles_cover_data_path
   def marbles_thumb_fixture, do: @marbles_thumb_data_path
   def raven_cover_fixture, do: @raven_cover_data_path
@@ -59,17 +65,27 @@ defmodule MusicLibrary.Fixtures.Records do
   def marbles_thumb_data, do: @marbles_thumb_data
   def raven_cover_data, do: @raven_cover_data
 
+  def marbles_cover_hash, do: @marbles_cover_hash
+  def marbles_thumb_hash, do: @marbles_thumb_hash
+  def raven_cover_hash, do: @raven_cover_hash
+
   def record(attrs \\ %{}) do
     record_musicbrainz_id = Ecto.UUID.generate()
     artist_name = Enum.random(@artists)
     current_time = DateTime.utc_now()
 
+    {:ok, asset} =
+      Assets.store(%{
+        content: @marbles_cover_data,
+        format: "image/jpeg"
+      })
+
     {:ok, record} =
       attrs
       |> Enum.into(%{
         genres: Enum.take_random(@genres, :rand.uniform(3)),
         cover_url: "https://coverartarchive.org/release-group/#{record_musicbrainz_id}/front",
-        cover_data: @marbles_cover_data,
+        cover_hash: asset.hash,
         musicbrainz_id: record_musicbrainz_id,
         musicbrainz_data: ReleaseGroup.release_group(:marbles),
         title: Enum.random(@titles),