diff --git a/lib/music_library/assets.ex b/lib/music_library/assets.ex index f388303a..79a50d74 100644 --- a/lib/music_library/assets.ex +++ b/lib/music_library/assets.ex @@ -22,6 +22,10 @@ defmodule MusicLibrary.Assets do end def get(hash) do + Repo.get(Asset, hash) + end + + def get!(hash) do Repo.get!(Asset, hash) end end diff --git a/lib/music_library_web/controllers/cover_controller.ex b/lib/music_library_web/controllers/cover_controller.ex index 4be434f9..e0cb761f 100644 --- a/lib/music_library_web/controllers/cover_controller.ex +++ b/lib/music_library_web/controllers/cover_controller.ex @@ -2,20 +2,19 @@ defmodule MusicLibraryWeb.CoverController do use MusicLibraryWeb, :controller alias MusicLibrary.Assets - alias MusicLibrary.Records alias MusicLibrary.Records.Cover # 1 year in seconds @cache_duration 60 * 60 * 24 * 365 - def show(conn, %{"record_id" => record_id, "size" => size}) do - case Records.get_cover(record_id) do + def show(conn, %{"hash" => hash, "size" => size}) do + case Assets.get(hash) do nil -> not_found(conn) - %{cover_data: cover_data} -> + %{content: content} -> # TODO: find a way to cache computation, or pre-compute thumb and store it - {:ok, thumb_data} = Cover.resize(cover_data, String.to_integer(size)) + {:ok, thumb_data} = Cover.resize(content, String.to_integer(size)) hash = Cover.hash(thumb_data) case get_req_header(conn, "if-none-match") do @@ -25,16 +24,14 @@ defmodule MusicLibraryWeb.CoverController do end end - def show(conn, %{"record_id" => record_id}) do - case Records.get_cover(record_id) do + def show(conn, %{"hash" => hash}) do + case Assets.get(hash) do nil -> not_found(conn) - %{cover_hash: etag} -> - asset = Assets.get(etag) - + asset -> case get_req_header(conn, "if-none-match") do - [^etag] -> extend_cache(conn) + [^hash] -> extend_cache(conn) _ -> respond_with_cache(conn, asset) end end diff --git a/lib/music_library_web/router.ex b/lib/music_library_web/router.ex index 81923151..e8b11dac 100644 --- a/lib/music_library_web/router.ex +++ b/lib/music_library_web/router.ex @@ -36,7 +36,7 @@ defmodule MusicLibraryWeb.Router do get "/backup", ArchiveController, :backup - get "/covers/:record_id", CoverController, :show + get "/covers/:hash", CoverController, :show get "/artists/:musicbrainz_id/image", ArtistController, :image live_session :default, @@ -79,7 +79,7 @@ defmodule MusicLibraryWeb.Router do get "/collection/latest", CollectionController, :latest get "/collection/random", CollectionController, :random get "/collection", CollectionController, :index - get "/covers/:record_id", CoverController, :show + get "/covers/:hash", CoverController, :show get "/backup", ArchiveController, :backup end diff --git a/test/music_library_web/controllers/cover_controller_test.exs b/test/music_library_web/controllers/cover_controller_test.exs index e017dc8a..58b5082f 100644 --- a/test/music_library_web/controllers/cover_controller_test.exs +++ b/test/music_library_web/controllers/cover_controller_test.exs @@ -6,56 +6,52 @@ defmodule MusicLibraryWeb.CoverControllerTest do alias MusicLibrary.Assets alias MusicLibrary.Records.Cover - defp create_record(_) do - %{record: record()} - end - - defp create_asset(%{record: record}) do - {:ok, asset} = Assets.store(%{content: record.cover_data, format: "image/jpeg"}) + defp create_asset(_config) do + {:ok, asset} = Assets.store(%{content: marbles_cover_data(), format: "image/jpeg"}) %{asset: asset} end - describe "GET /covers/:record_id" do - setup [:create_record, :create_asset] + describe "GET /covers/:hash" do + setup [:create_asset] - test "404s when record doesn't exist", %{conn: conn} do + test "404s when asset doesn't exist", %{conn: conn} do id = Ecto.UUID.generate() conn = get(conn, ~p"/covers/#{id}") assert text_response(conn, 404) == "Not found" end - test "serves the cover without etag", %{conn: conn, record: record} do - conn = get(conn, ~p"/covers/#{record.id}") + test "serves the cover without etag", %{conn: conn, asset: asset} do + conn = get(conn, ~p"/covers/#{asset.hash}") assert conn.status == 200 assert get_resp_header(conn, "content-type") == ["image/jpeg; charset=utf-8"] assert get_resp_header(conn, "cache-control") == ["public, max-age=31536000"] - assert get_resp_header(conn, "etag") == [record.cover_hash] + assert get_resp_header(conn, "etag") == [asset.hash] - assert conn.resp_body == record.cover_data + assert conn.resp_body == asset.content end - test "serves the cover when etag doesn't match", %{conn: conn, record: record} do + test "serves the cover when etag doesn't match", %{conn: conn, asset: asset} do conn = conn |> put_req_header("if-none-match", "invalid-etag") - |> get(~p"/covers/#{record.id}") + |> get(~p"/covers/#{asset.hash}") assert conn.status == 200 assert get_resp_header(conn, "content-type") == ["image/jpeg; charset=utf-8"] assert get_resp_header(conn, "cache-control") == ["public, max-age=31536000"] - assert get_resp_header(conn, "etag") == [record.cover_hash] + assert get_resp_header(conn, "etag") == [asset.hash] - assert conn.resp_body == record.cover_data + assert conn.resp_body == asset.content end - test "serves a 304 when etag matches", %{conn: conn, record: record} do + test "serves a 304 when etag matches", %{conn: conn, asset: asset} do conn = conn - |> put_req_header("if-none-match", record.cover_hash) - |> get(~p"/covers/#{record.id}") + |> put_req_header("if-none-match", asset.hash) + |> get(~p"/covers/#{asset.hash}") assert conn.status == 304 assert get_resp_header(conn, "content-type") == [] @@ -65,8 +61,8 @@ defmodule MusicLibraryWeb.CoverControllerTest do assert conn.resp_body == <<>> end - test "accepts a size attribute for resizing", %{conn: conn, record: record} do - conn = get(conn, ~p"/covers/#{record.id}?size=480") + test "accepts a size attribute for resizing", %{conn: conn, asset: asset} do + conn = get(conn, ~p"/covers/#{asset.hash}?size=480") thumb = marbles_thumb_data() hash = Cover.hash(thumb)