Async barcode scan batch import for 2+ new records
When barcode scan results contain at least 2 new records, enqueue individual Oban jobs instead of importing synchronously. Wishlisted/collected/not_found results still process synchronously.
This commit is contained in:
@@ -5,6 +5,7 @@ defmodule MusicLibrary.BarcodeScan do
|
|||||||
|
|
||||||
alias MusicLibrary.BarcodeScan.Result
|
alias MusicLibrary.BarcodeScan.Result
|
||||||
alias MusicLibrary.Records
|
alias MusicLibrary.Records
|
||||||
|
alias MusicLibrary.Worker.ImportFromMusicbrainzRelease
|
||||||
|
|
||||||
@spec scan(String.t()) :: {:ok, Result.t()} | {:error, term()}
|
@spec scan(String.t()) :: {:ok, Result.t()} | {:error, term()}
|
||||||
def scan(number) do
|
def scan(number) do
|
||||||
@@ -31,6 +32,32 @@ defmodule MusicLibrary.BarcodeScan do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@spec should_import_async?([Result.t()]) :: boolean()
|
||||||
|
def should_import_async?(scan_results) do
|
||||||
|
Enum.count(scan_results, &(&1.status == :new)) >= 2
|
||||||
|
end
|
||||||
|
|
||||||
|
@spec import_results_async([Result.t()], DateTime.t()) ::
|
||||||
|
{:ok, sync_errors :: [{String.t(), term()}], async_count :: non_neg_integer()}
|
||||||
|
def import_results_async(scan_results, current_time) do
|
||||||
|
{new_results, other_results} = Enum.split_with(scan_results, &(&1.status == :new))
|
||||||
|
|
||||||
|
sync_errors = import_results(other_results, current_time)
|
||||||
|
|
||||||
|
Enum.each(new_results, fn scan_result ->
|
||||||
|
%{
|
||||||
|
"release_id" => scan_result.release.id,
|
||||||
|
"format" => MusicBrainz.ReleaseSearchResult.format(scan_result.release),
|
||||||
|
"purchased_at" => DateTime.to_iso8601(current_time),
|
||||||
|
"selected_release_id" => scan_result.release.id
|
||||||
|
}
|
||||||
|
|> ImportFromMusicbrainzRelease.new()
|
||||||
|
|> Oban.insert!()
|
||||||
|
end)
|
||||||
|
|
||||||
|
{:ok, sync_errors, length(new_results)}
|
||||||
|
end
|
||||||
|
|
||||||
@spec import_results([Result.t()], DateTime.t()) :: [{String.t(), term()}]
|
@spec import_results([Result.t()], DateTime.t()) :: [{String.t(), term()}]
|
||||||
def import_results(scan_results, current_time) do
|
def import_results(scan_results, current_time) do
|
||||||
Enum.reduce(scan_results, [], fn scan_result, errors ->
|
Enum.reduce(scan_results, [], fn scan_result, errors ->
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
defmodule MusicLibrary.Worker.ImportFromMusicbrainzRelease do
|
||||||
|
@moduledoc """
|
||||||
|
Imports a record from a MusicBrainz release in the background.
|
||||||
|
|
||||||
|
Used by barcode scan batch imports when there are multiple new records to import.
|
||||||
|
"""
|
||||||
|
|
||||||
|
use Oban.Worker, queue: :music_brainz, max_attempts: 3
|
||||||
|
|
||||||
|
@impl Oban.Worker
|
||||||
|
def perform(%Oban.Job{args: %{"release_id" => release_id} = args}) do
|
||||||
|
opts = [
|
||||||
|
format: args["format"],
|
||||||
|
purchased_at: parse_datetime(args["purchased_at"]),
|
||||||
|
selected_release_id: args["selected_release_id"]
|
||||||
|
]
|
||||||
|
|
||||||
|
case MusicLibrary.Records.import_from_musicbrainz_release(release_id, opts) do
|
||||||
|
{:ok, _record} -> :ok
|
||||||
|
{:error, reason} -> {:error, reason}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp parse_datetime(nil), do: nil
|
||||||
|
|
||||||
|
defp parse_datetime(str) do
|
||||||
|
{:ok, datetime, _offset} = DateTime.from_iso8601(str)
|
||||||
|
datetime
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -402,23 +402,32 @@ defmodule MusicLibraryWeb.Components.BarcodeScanner do
|
|||||||
|
|
||||||
def handle_event("import_releases", _params, socket) do
|
def handle_event("import_releases", _params, socket) do
|
||||||
current_time = DateTime.utc_now()
|
current_time = DateTime.utc_now()
|
||||||
|
scan_results = socket.assigns.scan_results
|
||||||
|
|
||||||
socket =
|
socket =
|
||||||
case BarcodeScan.import_results(socket.assigns.scan_results, current_time) do
|
if BarcodeScan.should_import_async?(scan_results) do
|
||||||
[] ->
|
{:ok, sync_errors, async_count} =
|
||||||
put_toast(socket, :info, gettext("Records imported successfully"))
|
BarcodeScan.import_results_async(scan_results, current_time)
|
||||||
|
|
||||||
errors ->
|
socket
|
||||||
errors_summary =
|
|> maybe_toast_errors(sync_errors)
|
||||||
Enum.map_join(errors, "\n", fn {number, reason} ->
|
|> put_toast(
|
||||||
"#{number}: #{ErrorMessages.friendly_message(reason)}"
|
:info,
|
||||||
end)
|
ngettext(
|
||||||
|
"Importing %{count} record in the background...",
|
||||||
put_toast(
|
"Importing %{count} records in the background...",
|
||||||
socket,
|
async_count,
|
||||||
:error,
|
count: async_count
|
||||||
gettext("Some records could not be imported: %{summary}", summary: errors_summary)
|
|
||||||
)
|
)
|
||||||
|
)
|
||||||
|
else
|
||||||
|
case BarcodeScan.import_results(scan_results, current_time) do
|
||||||
|
[] ->
|
||||||
|
put_toast(socket, :info, gettext("Records imported successfully"))
|
||||||
|
|
||||||
|
errors ->
|
||||||
|
maybe_toast_errors(socket, errors)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
qs = %{order: :purchase}
|
qs = %{order: :purchase}
|
||||||
@@ -429,6 +438,21 @@ defmodule MusicLibraryWeb.Components.BarcodeScanner do
|
|||||||
|> push_patch(to: ~p"/collection?#{qs}")}
|
|> push_patch(to: ~p"/collection?#{qs}")}
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp maybe_toast_errors(socket, []), do: socket
|
||||||
|
|
||||||
|
defp maybe_toast_errors(socket, errors) do
|
||||||
|
errors_summary =
|
||||||
|
Enum.map_join(errors, "\n", fn {number, reason} ->
|
||||||
|
"#{number}: #{ErrorMessages.friendly_message(reason)}"
|
||||||
|
end)
|
||||||
|
|
||||||
|
put_toast(
|
||||||
|
socket,
|
||||||
|
:error,
|
||||||
|
gettext("Some records could not be imported: %{summary}", summary: errors_summary)
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
defp release_format_label(release) do
|
defp release_format_label(release) do
|
||||||
release
|
release
|
||||||
|> MusicBrainz.ReleaseSearchResult.format()
|
|> MusicBrainz.ReleaseSearchResult.format()
|
||||||
|
|||||||
@@ -2395,3 +2395,10 @@ msgstr ""
|
|||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Record has a selected release"
|
msgid "Record has a selected release"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/music_library_web/components/barcode_scanner.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "Importing %{count} record in the background..."
|
||||||
|
msgid_plural "Importing %{count} records in the background..."
|
||||||
|
msgstr[0] ""
|
||||||
|
msgstr[1] ""
|
||||||
|
|||||||
@@ -2395,3 +2395,10 @@ msgstr ""
|
|||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Record has a selected release"
|
msgid "Record has a selected release"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/music_library_web/components/barcode_scanner.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "Importing %{count} record in the background..."
|
||||||
|
msgid_plural "Importing %{count} records in the background..."
|
||||||
|
msgstr[0] ""
|
||||||
|
msgstr[1] ""
|
||||||
|
|||||||
@@ -254,4 +254,218 @@ defmodule MusicLibrary.BarcodeScanTest do
|
|||||||
assert {"2222222222", :not_found} in errors
|
assert {"2222222222", :not_found} in errors
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "should_import_async?/1" do
|
||||||
|
test "returns false with zero new results" do
|
||||||
|
refute BarcodeScan.should_import_async?([])
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns false with one new result" do
|
||||||
|
results = [Result.new("111", %{})]
|
||||||
|
refute BarcodeScan.should_import_async?(results)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns true with two new results" do
|
||||||
|
results = [Result.new("111", %{}), Result.new("222", %{})]
|
||||||
|
assert BarcodeScan.should_import_async?(results)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "only counts :new results" do
|
||||||
|
results = [
|
||||||
|
Result.new("111", %{}),
|
||||||
|
Result.wishlisted("222", "some-id", %{}),
|
||||||
|
Result.collected("333", "some-id", %{}),
|
||||||
|
Result.not_found("444")
|
||||||
|
]
|
||||||
|
|
||||||
|
refute BarcodeScan.should_import_async?(results)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns true with mixed statuses including two new" do
|
||||||
|
results = [
|
||||||
|
Result.new("111", %{}),
|
||||||
|
Result.wishlisted("222", "some-id", %{}),
|
||||||
|
Result.new("333", %{})
|
||||||
|
]
|
||||||
|
|
||||||
|
assert BarcodeScan.should_import_async?(results)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "import_results_async/2" do
|
||||||
|
test "enqueues new results as Oban jobs" do
|
||||||
|
current_time = DateTime.utc_now()
|
||||||
|
|
||||||
|
new_result_1 = %Result{
|
||||||
|
status: :new,
|
||||||
|
number: "111",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "release-1",
|
||||||
|
title: "Album 1",
|
||||||
|
release_group: %{id: "rg-1", type: :album, title: "Album 1"},
|
||||||
|
artists: "Artist 1",
|
||||||
|
date: "2024",
|
||||||
|
barcode: "111",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 10}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
new_result_2 = %Result{
|
||||||
|
status: :new,
|
||||||
|
number: "222",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "release-2",
|
||||||
|
title: "Album 2",
|
||||||
|
release_group: %{id: "rg-2", type: :album, title: "Album 2"},
|
||||||
|
artists: "Artist 2",
|
||||||
|
date: "2024",
|
||||||
|
barcode: "222",
|
||||||
|
media: [%{format: "12\" Vinyl", disc_count: 1, track_count: 8}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
assert {:ok, [], 2} =
|
||||||
|
BarcodeScan.import_results_async([new_result_1, new_result_2], current_time)
|
||||||
|
|
||||||
|
assert_enqueued(
|
||||||
|
worker: MusicLibrary.Worker.ImportFromMusicbrainzRelease,
|
||||||
|
args: %{
|
||||||
|
"release_id" => "release-1",
|
||||||
|
"format" => "cd",
|
||||||
|
"purchased_at" => DateTime.to_iso8601(current_time),
|
||||||
|
"selected_release_id" => "release-1"
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_enqueued(
|
||||||
|
worker: MusicLibrary.Worker.ImportFromMusicbrainzRelease,
|
||||||
|
args: %{
|
||||||
|
"release_id" => "release-2",
|
||||||
|
"format" => "vinyl",
|
||||||
|
"purchased_at" => DateTime.to_iso8601(current_time),
|
||||||
|
"selected_release_id" => "release-2"
|
||||||
|
}
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "processes wishlisted results synchronously" do
|
||||||
|
current_time = DateTime.utc_now()
|
||||||
|
wishlisted_record = record(%{purchased_at: nil})
|
||||||
|
|
||||||
|
wishlisted_result = %Result{
|
||||||
|
status: :wishlisted,
|
||||||
|
number: "333",
|
||||||
|
record_id: wishlisted_record.id,
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "some-release-id",
|
||||||
|
title: "Test",
|
||||||
|
release_group: nil,
|
||||||
|
artists: "Test Artist",
|
||||||
|
date: "2021",
|
||||||
|
barcode: "333",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 10}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
new_result_1 = %Result{
|
||||||
|
status: :new,
|
||||||
|
number: "111",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "release-1",
|
||||||
|
title: "Album 1",
|
||||||
|
release_group: %{id: "rg-1", type: :album, title: "Album 1"},
|
||||||
|
artists: "Artist 1",
|
||||||
|
date: "2024",
|
||||||
|
barcode: "111",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 10}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
new_result_2 = %Result{
|
||||||
|
status: :new,
|
||||||
|
number: "222",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "release-2",
|
||||||
|
title: "Album 2",
|
||||||
|
release_group: %{id: "rg-2", type: :album, title: "Album 2"},
|
||||||
|
artists: "Artist 2",
|
||||||
|
date: "2024",
|
||||||
|
barcode: "222",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 10}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
assert {:ok, [], 2} =
|
||||||
|
BarcodeScan.import_results_async(
|
||||||
|
[wishlisted_result, new_result_1, new_result_2],
|
||||||
|
current_time
|
||||||
|
)
|
||||||
|
|
||||||
|
updated_record = Records.get_record!(wishlisted_record.id)
|
||||||
|
assert updated_record.purchased_at == DateTime.truncate(current_time, :second)
|
||||||
|
|
||||||
|
assert_enqueued(
|
||||||
|
worker: MusicLibrary.Worker.ImportFromMusicbrainzRelease,
|
||||||
|
args: %{"release_id" => "release-1"}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_enqueued(
|
||||||
|
worker: MusicLibrary.Worker.ImportFromMusicbrainzRelease,
|
||||||
|
args: %{"release_id" => "release-2"}
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns sync errors from non-new results" do
|
||||||
|
current_time = DateTime.utc_now()
|
||||||
|
|
||||||
|
collected_result = %Result{
|
||||||
|
status: :collected,
|
||||||
|
number: "333",
|
||||||
|
record_id: "some-id",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "r1",
|
||||||
|
title: "T",
|
||||||
|
release_group: nil,
|
||||||
|
artists: "A",
|
||||||
|
date: "2021",
|
||||||
|
barcode: "333",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 1}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
new_result_1 = %Result{
|
||||||
|
status: :new,
|
||||||
|
number: "111",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "release-1",
|
||||||
|
title: "Album 1",
|
||||||
|
release_group: %{id: "rg-1", type: :album, title: "Album 1"},
|
||||||
|
artists: "Artist 1",
|
||||||
|
date: "2024",
|
||||||
|
barcode: "111",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 10}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
new_result_2 = %Result{
|
||||||
|
status: :new,
|
||||||
|
number: "222",
|
||||||
|
release: %MusicBrainz.ReleaseSearchResult{
|
||||||
|
id: "release-2",
|
||||||
|
title: "Album 2",
|
||||||
|
release_group: %{id: "rg-2", type: :album, title: "Album 2"},
|
||||||
|
artists: "Artist 2",
|
||||||
|
date: "2024",
|
||||||
|
barcode: "222",
|
||||||
|
media: [%{format: "CD", disc_count: 1, track_count: 10}]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
assert {:ok, [{"333", :already_collected}], 2} =
|
||||||
|
BarcodeScan.import_results_async(
|
||||||
|
[collected_result, new_result_1, new_result_2],
|
||||||
|
current_time
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
defmodule MusicLibrary.Worker.ImportFromMusicbrainzReleaseTest do
|
||||||
|
use MusicLibrary.DataCase
|
||||||
|
|
||||||
|
import MusicBrainz.Fixtures.Release
|
||||||
|
import MusicBrainz.Fixtures.ReleaseGroup
|
||||||
|
import MusicLibrary.Fixtures.Records
|
||||||
|
|
||||||
|
alias MusicLibrary.Records.Record
|
||||||
|
alias MusicLibrary.Worker.ImportFromMusicbrainzRelease
|
||||||
|
|
||||||
|
describe "perform/1" do
|
||||||
|
test "imports a record from a MusicBrainz release" do
|
||||||
|
release_data = release(:marbles)
|
||||||
|
release_id = release_id(:marbles)
|
||||||
|
|
||||||
|
release_group_data = release_group(:marbles)
|
||||||
|
release_group_id = release_group_id(:marbles)
|
||||||
|
release_group_releases_data = release_group_releases(:marbles)
|
||||||
|
|
||||||
|
cover_data = marbles_cover_data()
|
||||||
|
|
||||||
|
Req.Test.stub(MusicBrainz.API, fn conn ->
|
||||||
|
case conn.path_info do
|
||||||
|
[_ws, _version, "release-group", ^release_group_id] ->
|
||||||
|
Req.Test.json(conn, release_group_data)
|
||||||
|
|
||||||
|
[_ws, _version, "release", ^release_id] ->
|
||||||
|
Req.Test.json(conn, release_data)
|
||||||
|
|
||||||
|
[_ws, _version, "release"] ->
|
||||||
|
Req.Test.json(conn, release_group_releases_data)
|
||||||
|
|
||||||
|
[_release_group, ^release_group_id, "front"] ->
|
||||||
|
Plug.Conn.send_resp(conn, 200, cover_data)
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
|
||||||
|
purchased_at = DateTime.utc_now()
|
||||||
|
|
||||||
|
assert :ok =
|
||||||
|
perform_job(ImportFromMusicbrainzRelease, %{
|
||||||
|
"release_id" => release_id,
|
||||||
|
"format" => "cd",
|
||||||
|
"purchased_at" => DateTime.to_iso8601(purchased_at),
|
||||||
|
"selected_release_id" => release_id
|
||||||
|
})
|
||||||
|
|
||||||
|
imported_record = Repo.get_by!(Record, musicbrainz_id: release_group_id)
|
||||||
|
assert imported_record.title == "Marbles"
|
||||||
|
assert imported_record.purchased_at == DateTime.truncate(purchased_at, :second)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns error on transport failure" do
|
||||||
|
Req.Test.stub(MusicBrainz.API, fn conn ->
|
||||||
|
Req.Test.transport_error(conn, :timeout)
|
||||||
|
end)
|
||||||
|
|
||||||
|
assert {:error, %Req.TransportError{reason: :timeout}} =
|
||||||
|
perform_job(ImportFromMusicbrainzRelease, %{
|
||||||
|
"release_id" => "nonexistent-release-id",
|
||||||
|
"format" => "cd",
|
||||||
|
"purchased_at" => DateTime.to_iso8601(DateTime.utc_now()),
|
||||||
|
"selected_release_id" => "nonexistent-release-id"
|
||||||
|
})
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Reference in New Issue
Block a user