Correctly return 404s in /images endpoint

lib/music_library/records.ex

@@ -68,13 +68,13 @@ defmodule MusicLibrary.Records do
     Repo.one!(q)
   end
 
-  def get_cover!(id) do
+  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)
+    Repo.one(q)
   end
 
   def search_release_group(query, opts \\ []) do

lib/music_library_web/controllers/image_controller.ex

@@ -4,9 +4,11 @@ defmodule MusicLibraryWeb.ImageController do
   alias MusicLibrary.Records
 
   def show(conn, %{"record_id" => record_id}) do
-    case Records.get_cover!(record_id) do
+    case Records.get_cover(record_id) do
       nil ->
-        send_resp(conn, 404, "Not found")
+        conn
+        |> put_status(:not_found)
+        |> text("Not found")
 
       %{cover_data: cover_data, cover_hash: etag} ->
         case get_req_header(conn, "if-none-match") do

test/music_library_web/controllers/image_controller_test.exs

@@ -0,0 +1,56 @@
+defmodule MusicLibraryWeb.ImageControllerTest do
+  use MusicLibraryWeb.ConnCase
+
+  import MusicLibrary.RecordsFixtures
+
+  defp create_record(_) do
+    %{record: record_fixture()}
+  end
+
+  describe "GET /images/:record_id" do
+    setup [:create_record]
+
+    test "404s when record doesn't exist", %{conn: conn} do
+      id = Ecto.UUID.generate()
+
+      conn = get(conn, ~p"/images/#{id}")
+      assert text_response(conn, 404) == "Not found"
+    end
+
+    test "serves the image when etag doesn't exist", %{conn: conn, record: record} do
+      conn = get(conn, ~p"/images/#{record.id}")
+
+      assert conn.status == 200
+      assert get_resp_header(conn, "content-type") == ["image/jpeg; charset=utf-8"]
+      assert get_resp_header(conn, "etag") == [record.cover_hash]
+
+      assert conn.resp_body == record.cover_data
+    end
+
+    test "serves the image when etag doesn't match", %{conn: conn, record: record} do
+      conn =
+        conn
+        |> put_req_header("if-none-match", "invalid-etag")
+        |> get(~p"/images/#{record.id}")
+
+      assert conn.status == 200
+      assert get_resp_header(conn, "content-type") == ["image/jpeg; charset=utf-8"]
+      assert get_resp_header(conn, "etag") == [record.cover_hash]
+
+      assert conn.resp_body == record.cover_data
+    end
+
+    test "serves a 304 when etag matches", %{conn: conn, record: record} do
+      conn =
+        conn
+        |> put_req_header("if-none-match", record.cover_hash)
+        |> get(~p"/images/#{record.id}")
+
+      assert conn.status == 304
+      assert get_resp_header(conn, "content-type") == []
+      assert get_resp_header(conn, "etag") == []
+
+      assert conn.resp_body == <<>>
+    end
+  end
+end