Use Transforms to serve covers at the right size
This commit is contained in:
@@ -37,3 +37,9 @@ defmodule MusicLibrary.Assets.Transform do
|
||||
})
|
||||
end
|
||||
end
|
||||
|
||||
defimpl Phoenix.Param, for: MusicLibrary.Assets.Transform do
|
||||
def to_param(transform) do
|
||||
MusicLibrary.Assets.Transform.encode!(transform)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -2,18 +2,26 @@ defmodule MusicLibraryWeb.RecordComponents do
|
||||
use MusicLibraryWeb, :html
|
||||
|
||||
alias MusicBrainz.ReleaseSearchResult
|
||||
alias MusicLibrary.Assets.Transform
|
||||
alias MusicLibrary.Records
|
||||
alias Phoenix.LiveView.JS
|
||||
|
||||
attr :record, :map, required: true
|
||||
attr :class, :string, required: false, default: "rounded-lg"
|
||||
attr :width, :integer, default: nil
|
||||
|
||||
def record_cover(assigns) do
|
||||
payload =
|
||||
%Transform{hash: assigns.record.cover_hash, width: assigns.width}
|
||||
|> Transform.encode!()
|
||||
|
||||
assigns = assign(assigns, :payload, payload)
|
||||
|
||||
~H"""
|
||||
<img
|
||||
class={@class}
|
||||
alt={@record.title}
|
||||
src={~p"/covers/#{@record.cover_hash}"}
|
||||
src={~p"/covers/#{@payload}"}
|
||||
/>
|
||||
"""
|
||||
end
|
||||
@@ -58,7 +66,7 @@ defmodule MusicLibraryWeb.RecordComponents do
|
||||
>
|
||||
<div class="flex min-w-0 gap-x-4 items-center">
|
||||
<div class="relative w-20 flex-none">
|
||||
<.record_cover record={record} />
|
||||
<.record_cover record={record} width={160} />
|
||||
<span
|
||||
:if={Records.Record.included_release_groups_count(record) > 0}
|
||||
class={[
|
||||
@@ -230,6 +238,7 @@ defmodule MusicLibraryWeb.RecordComponents do
|
||||
<.record_cover
|
||||
record={record}
|
||||
class="pointer-events-none aspect-square object-cover group-hover:opacity-75"
|
||||
width={460}
|
||||
/>
|
||||
<span
|
||||
:if={Records.Record.included_release_groups_count(record) > 0}
|
||||
|
||||
@@ -34,6 +34,7 @@ defmodule MusicLibraryWeb.SearchComponents do
|
||||
<.record_cover
|
||||
record={@record}
|
||||
class="w-12 h-12 rounded-md aspect-square object-cover"
|
||||
width={96}
|
||||
/>
|
||||
</div>
|
||||
<div class="min-w-0 flex-1">
|
||||
|
||||
@@ -23,6 +23,7 @@ defmodule MusicLibraryWeb.StatsComponents do
|
||||
<.record_cover
|
||||
record={@record}
|
||||
class="w-16 md:w-20 rounded-md shadow-sm"
|
||||
width={160}
|
||||
/>
|
||||
</div>
|
||||
<div class="ml-4">
|
||||
@@ -167,7 +168,7 @@ defmodule MusicLibraryWeb.StatsComponents do
|
||||
>
|
||||
<div class="flex min-w-0 gap-x-4 items-center">
|
||||
<div class="relative w-12 flex-none">
|
||||
<.record_cover record={record} />
|
||||
<.record_cover record={record} width={96} />
|
||||
<span
|
||||
:if={Records.Record.included_release_groups_count(record) > 0}
|
||||
class={[
|
||||
|
||||
@@ -2,38 +2,32 @@ defmodule MusicLibraryWeb.CoverController do
|
||||
use MusicLibraryWeb, :controller
|
||||
|
||||
alias MusicLibrary.Assets
|
||||
alias MusicLibrary.Assets.Asset
|
||||
alias MusicLibrary.Assets.Transform
|
||||
alias MusicLibrary.Records.Cover
|
||||
|
||||
# 1 year in seconds
|
||||
@cache_duration 60 * 60 * 24 * 365
|
||||
|
||||
def show(conn, %{"hash" => hash, "size" => size}) do
|
||||
case Assets.get(hash) do
|
||||
def show(conn, %{"transform_payload" => payload}) do
|
||||
transform = Transform.decode!(payload)
|
||||
|
||||
case Assets.get(transform.hash) do
|
||||
nil ->
|
||||
not_found(conn)
|
||||
|
||||
%{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 = Asset.hash(thumb_data)
|
||||
image_data =
|
||||
if transform.width do
|
||||
# TODO: find a way to cache computation, or pre-compute thumb and store it
|
||||
{:ok, thumb_data} = Cover.resize(content, transform.width)
|
||||
thumb_data
|
||||
else
|
||||
content
|
||||
end
|
||||
|
||||
case get_req_header(conn, "if-none-match") do
|
||||
[^hash] -> extend_cache(conn)
|
||||
_ -> respond_with_cache(conn, thumb_data, hash)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def show(conn, %{"hash" => hash}) do
|
||||
case Assets.get(hash) do
|
||||
nil ->
|
||||
not_found(conn)
|
||||
|
||||
asset ->
|
||||
case get_req_header(conn, "if-none-match") do
|
||||
[^hash] -> extend_cache(conn)
|
||||
_ -> respond_with_cache(conn, asset)
|
||||
[^payload] -> extend_cache(conn)
|
||||
_ -> respond_with_cache(conn, image_data, payload)
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -50,14 +44,6 @@ defmodule MusicLibraryWeb.CoverController do
|
||||
|> send_resp(304, "")
|
||||
end
|
||||
|
||||
defp respond_with_cache(conn, asset) do
|
||||
conn
|
||||
|> put_resp_content_type(asset.format, "utf-8")
|
||||
|> put_resp_header("cache-control", "public, max-age=#{@cache_duration}")
|
||||
|> put_resp_header("etag", asset.hash)
|
||||
|> send_resp(200, asset.content)
|
||||
end
|
||||
|
||||
defp respond_with_cache(conn, data, etag) do
|
||||
conn
|
||||
|> put_resp_content_type("image/jpeg", "utf-8")
|
||||
|
||||
@@ -36,7 +36,7 @@ defmodule MusicLibraryWeb.Router do
|
||||
|
||||
get "/backup", ArchiveController, :backup
|
||||
|
||||
get "/covers/:hash", CoverController, :show
|
||||
get "/covers/:transform_payload", CoverController, :show
|
||||
get "/artists/:musicbrainz_id/image", ArtistController, :image
|
||||
|
||||
live_session :default,
|
||||
|
||||
@@ -4,6 +4,7 @@ defmodule MusicLibraryWeb.CoverControllerTest do
|
||||
import MusicLibrary.Fixtures.Records
|
||||
|
||||
alias MusicLibrary.Assets
|
||||
alias MusicLibrary.Assets.Transform
|
||||
|
||||
defp create_asset(_config) do
|
||||
{:ok, asset} = Assets.store(%{content: marbles_cover_data(), format: "image/jpeg"})
|
||||
@@ -15,42 +16,51 @@ defmodule MusicLibraryWeb.CoverControllerTest do
|
||||
setup [:create_asset]
|
||||
|
||||
test "404s when asset doesn't exist", %{conn: conn} do
|
||||
id = Ecto.UUID.generate()
|
||||
transform = %Transform{hash: Ecto.UUID.generate()}
|
||||
payload = Transform.encode!(transform)
|
||||
|
||||
conn = get(conn, ~p"/covers/#{id}")
|
||||
conn = get(conn, ~p"/covers/#{payload}")
|
||||
assert text_response(conn, 404) == "Not found"
|
||||
end
|
||||
|
||||
test "serves the cover without etag", %{conn: conn, asset: asset} do
|
||||
conn = get(conn, ~p"/covers/#{asset.hash}")
|
||||
transform = %Transform{hash: asset.hash}
|
||||
payload = Transform.encode!(transform)
|
||||
conn = get(conn, ~p"/covers/#{payload}")
|
||||
|
||||
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") == [asset.hash]
|
||||
assert get_resp_header(conn, "etag") == [payload]
|
||||
|
||||
assert conn.resp_body == asset.content
|
||||
end
|
||||
|
||||
test "serves the cover when etag doesn't match", %{conn: conn, asset: asset} do
|
||||
transform = %Transform{hash: asset.hash}
|
||||
payload = Transform.encode!(transform)
|
||||
|
||||
conn =
|
||||
conn
|
||||
|> put_req_header("if-none-match", "invalid-etag")
|
||||
|> get(~p"/covers/#{asset.hash}")
|
||||
|> get(~p"/covers/#{payload}")
|
||||
|
||||
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") == [asset.hash]
|
||||
assert get_resp_header(conn, "etag") == [payload]
|
||||
|
||||
assert conn.resp_body == asset.content
|
||||
end
|
||||
|
||||
test "serves a 304 when etag matches", %{conn: conn, asset: asset} do
|
||||
transform = %Transform{hash: asset.hash}
|
||||
payload = Transform.encode!(transform)
|
||||
|
||||
conn =
|
||||
conn
|
||||
|> put_req_header("if-none-match", asset.hash)
|
||||
|> get(~p"/covers/#{asset.hash}")
|
||||
|> put_req_header("if-none-match", payload)
|
||||
|> get(~p"/covers/#{payload}")
|
||||
|
||||
assert conn.status == 304
|
||||
assert get_resp_header(conn, "content-type") == []
|
||||
@@ -60,18 +70,18 @@ defmodule MusicLibraryWeb.CoverControllerTest do
|
||||
assert conn.resp_body == <<>>
|
||||
end
|
||||
|
||||
test "accepts a size attribute for resizing", %{conn: conn, asset: asset} do
|
||||
conn = get(conn, ~p"/covers/#{asset.hash}?size=480")
|
||||
test "it handles transforms with width", %{conn: conn, asset: asset} do
|
||||
transform = %Transform{hash: asset.hash, width: 480}
|
||||
payload = Transform.encode!(transform)
|
||||
|
||||
thumb = marbles_thumb_data()
|
||||
hash = Assets.Asset.hash(thumb)
|
||||
conn = get(conn, ~p"/covers/#{payload}")
|
||||
|
||||
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") == [hash]
|
||||
assert get_resp_header(conn, "etag") == [payload]
|
||||
|
||||
assert conn.resp_body == thumb
|
||||
assert conn.resp_body == marbles_thumb_data()
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -8,6 +8,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
|
||||
|
||||
alias MusicBrainz.ReleaseGroupSearchResult
|
||||
alias MusicLibrary.Assets
|
||||
alias MusicLibrary.Assets.Transform
|
||||
alias MusicLibrary.Records.{Cover, Record}
|
||||
|
||||
# make it a multiple of 4 for easier calculations
|
||||
@@ -51,7 +52,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.cover_hash}"
|
||||
cover_url = cover_url(record, 160)
|
||||
|
||||
session
|
||||
|> assert_has("#records-#{record.id}")
|
||||
@@ -140,7 +141,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
|
||||
session =
|
||||
visit(conn, ~p"/collection?#{qs}")
|
||||
|
||||
cover_url = ~p"/covers/#{record.cover_hash}"
|
||||
cover_url = cover_url(record, 160)
|
||||
|
||||
session
|
||||
|> assert_has("#records-#{record.id}")
|
||||
@@ -182,7 +183,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
|
||||
visit(conn, ~p"/collection?#{qs}")
|
||||
|
||||
for record <- present do
|
||||
cover_url = ~p"/covers/#{record.cover_hash}"
|
||||
cover_url = cover_url(record, 160)
|
||||
|
||||
session
|
||||
|> assert_has("#records-#{record.id}")
|
||||
@@ -219,7 +220,7 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
|
||||
|
||||
test "can change the record cover", %{conn: conn} do
|
||||
record = record()
|
||||
cover_url = ~p"/covers/#{record.cover_hash}"
|
||||
cover_url = cover_url(record, nil)
|
||||
|
||||
session =
|
||||
conn
|
||||
@@ -229,25 +230,11 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
|
||||
session =
|
||||
session
|
||||
|> upload("Cover art", raven_cover_fixture())
|
||||
# Shows a range warning
|
||||
#
|
||||
# warning: Range.new/2 and first..last default to a step of -1 when last < first. Use Range.new(first, last, -1) or first..last//-1, or pass 1 if that was your intention
|
||||
# (phoenix_live_view 1.1.10) lib/phoenix_live_view/test/diff.ex:80: Phoenix.LiveViewTest.Diff.deep_merge_diff/2
|
||||
# (stdlib 7.0.2) maps.erl:405: :maps.merge_with_1/3
|
||||
# (phoenix_live_view 1.1.10) lib/phoenix_live_view/test/diff.ex:62: Phoenix.LiveViewTest.Diff.find_component/5
|
||||
# (phoenix_live_view 1.1.10) lib/phoenix_live_view/test/diff.ex:30: anonymous fn/4 in Phoenix.LiveViewTest.Diff.merge_diff/2
|
||||
# (stdlib 7.0.2) maps.erl:894: :maps.fold_1/4
|
||||
# (phoenix_live_view 1.1.10) lib/phoenix_live_view/test/diff.ex:29: Phoenix.LiveViewTest.Diff.merge_diff/2
|
||||
# (phoenix_live_view 1.1.10) lib/phoenix_live_view/test/client_proxy.ex:864: Phoenix.LiveViewTest.ClientProxy.merge_rendered/3
|
||||
# (phoenix_live_view 1.1.10) lib/phoenix_live_view/test/client_proxy.ex:488: Phoenix.LiveViewTest.ClientProxy.handle_info/2
|
||||
# (stdlib 7.0.2) gen_server.erl:2434: :gen_server.try_handle_info/3
|
||||
# (stdlib 7.0.2) gen_server.erl:2420: :gen_server.handle_msg/3
|
||||
# (stdlib 7.0.2) proc_lib.erl:333: :proc_lib.init_p_do_apply/3
|
||||
|> click_button("Save")
|
||||
|> assert_has("p", text: "Record updated successfully")
|
||||
|
||||
updated_record = MusicLibrary.Records.get_record!(record.id)
|
||||
updated_cover_url = ~p"/covers/#{updated_record.cover_hash}"
|
||||
updated_cover_url = cover_url(updated_record, 160)
|
||||
|
||||
assert updated_record.cover_hash !== record.cover_hash
|
||||
assert_has(session, "img[src='#{updated_cover_url}']")
|
||||
@@ -455,4 +442,10 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do
|
||||
musicbrainz_id: "1932f5b6-0b7b-4050-b1df-833ca89e5f44"
|
||||
} = marillion
|
||||
end
|
||||
|
||||
defp cover_url(record, width) do
|
||||
transform = %Transform{hash: record.cover_hash, width: width}
|
||||
payload = Transform.encode!(transform)
|
||||
~p"/covers/#{payload}"
|
||||
end
|
||||
end
|
||||
|
||||
@@ -5,6 +5,7 @@ defmodule MusicLibraryWeb.CollectionLive.ShowTest do
|
||||
import MusicLibraryWeb.RecordComponents, only: [format_label: 1, type_label: 1]
|
||||
|
||||
alias MusicBrainz.Fixtures
|
||||
alias MusicLibrary.Assets.Transform
|
||||
alias MusicLibrary.Records.Record
|
||||
|
||||
describe "Edit record from show page" do
|
||||
@@ -29,7 +30,9 @@ 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.cover_hash}"
|
||||
transform = %Transform{hash: record.cover_hash, width: nil}
|
||||
payload = Transform.encode!(transform)
|
||||
cover_url = ~p"/covers/#{payload}"
|
||||
|
||||
release_response = Fixtures.Release.release(:marbles)
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ defmodule MusicLibraryWeb.WishlistLive.ShowTest do
|
||||
import MusicLibrary.Fixtures.Records
|
||||
import MusicLibraryWeb.RecordComponents, only: [format_label: 1, type_label: 1]
|
||||
|
||||
alias MusicLibrary.Assets.Transform
|
||||
alias MusicLibrary.Records.Record
|
||||
|
||||
describe "Edit record from show page" do
|
||||
@@ -21,7 +22,9 @@ 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.cover_hash}"
|
||||
transform = %Transform{hash: record.cover_hash, width: nil}
|
||||
payload = Transform.encode!(transform)
|
||||
cover_url = ~p"/covers/#{payload}"
|
||||
|
||||
session =
|
||||
conn
|
||||
|
||||
Reference in New Issue
Block a user