[Bugfix] Ensure livestreams aren't downloaded until they're finished processing (#485)

* Added logic to ignore downloads that aren't in the right live state

* Added tests for get_downloadable_status method

* Added tests for media downloader module

* Added tests to download worker modeule
This commit is contained in:
Kieran 2024-11-26 11:56:33 -08:00 committed by GitHub
parent d9c48370df
commit bfb27427ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 156 additions and 3 deletions

View file

@ -94,6 +94,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
{:recovered, _} ->
{:error, :retry}
{:error, :unsuitable_for_download} ->
{:ok, :non_retry}
{:error, message} ->
action_on_error(message)
end

View file

@ -37,6 +37,13 @@ defmodule Pinchflat.Downloading.MediaDownloader do
{:ok, parsed_json} ->
update_media_item_from_parsed_json(media_with_preloads, parsed_json)
{:error, :unsuitable_for_download} ->
Logger.warning(
"Media item ##{media_with_preloads.id} isn't suitable for download yet. May be an active or processing live stream"
)
{:error, :unsuitable_for_download}
{:error, message, _exit_code} ->
Logger.error("yt-dlp download error for media item ##{media_with_preloads.id}: #{inspect(message)}")
@ -108,7 +115,11 @@ defmodule Pinchflat.Downloading.MediaDownloader do
{:ok, options} = DownloadOptionBuilder.build(item_with_preloads, override_opts)
runner_opts = [output_filepath: output_filepath, use_cookies: item_with_preloads.source.use_cookies]
YtDlpMedia.download(url, options, runner_opts)
case YtDlpMedia.get_downloadable_status(url) do
{:ok, :downloadable} -> YtDlpMedia.download(url, options, runner_opts)
{:ok, :ignorable} -> {:error, :unsuitable_for_download}
err -> err
end
end
defp recoverable_errors do

View file

@ -49,6 +49,24 @@ defmodule Pinchflat.YtDlp.Media do
end
end
@doc """
Determines if the media at the given URL is ready to be downloaded.
Common examples of non-downloadable media are upcoming or in-progress live streams.
Returns {:ok, :downloadable | :ignorable} | {:error, any}
"""
def get_downloadable_status(url) do
case backend_runner().run(url, [:simulate, :skip_download], "%(.{live_status})j") do
{:ok, output} ->
output
|> Phoenix.json_library().decode!()
|> parse_downloadable_status()
err ->
err
end
end
@doc """
Downloads a thumbnail for a single piece of media. Usually used for
downloading thumbnails for internal use
@ -71,11 +89,10 @@ defmodule Pinchflat.YtDlp.Media do
Returns {:ok, %Media{}} | {:error, any, ...}.
"""
def get_media_attributes(url, command_opts \\ [], addl_opts \\ []) do
runner = Application.get_env(:pinchflat, :yt_dlp_runner)
all_command_opts = [:simulate, :skip_download] ++ command_opts
output_template = indexing_output_template()
case runner.run(url, all_command_opts, output_template, addl_opts) do
case backend_runner().run(url, all_command_opts, output_template, addl_opts) do
{:ok, output} ->
output
|> Phoenix.json_library().decode!()
@ -147,6 +164,16 @@ defmodule Pinchflat.YtDlp.Media do
defp parse_uploaded_at(%{"upload_date" => nil}), do: nil
defp parse_uploaded_at(response), do: MetadataFileHelpers.parse_upload_date(response["upload_date"])
defp parse_downloadable_status(response) do
case response["live_status"] do
status when status in ["is_live", "is_upcoming", "post_live"] -> {:ok, :ignorable}
status when status in ["was_live", "not_live"] -> {:ok, :downloadable}
# This preserves my tenuous support for non-youtube sources.
nil -> {:ok, :downloadable}
_ -> {:error, "Unknown live status: #{response["live_status"]}"}
end
end
defp backend_runner do
# This approach lets us mock the command for testing
Application.get_env(:pinchflat, :yt_dlp_runner)

View file

@ -9,6 +9,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
alias Pinchflat.Downloading.MediaDownloadWorker
setup do
stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end)
stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end)
stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
@ -186,6 +187,20 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end
end
describe "perform/1 when testing non-downloadable media" do
test "does not retry the job if the media is currently not downloadable", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)
Oban.Testing.with_testing_mode(:inline, fn ->
{:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id}))
assert job.state == "completed"
end)
end
end
describe "perform/1 when testing forced downloads" do
test "ignores 'prevent_download' if forced", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> :ok end)

View file

@ -16,6 +16,7 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
)
stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end)
stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_args -> {:ok, ""} end)
{:ok, %{media_item: media_item}}
@ -49,6 +50,14 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
assert updated_media_item.metadata.thumbnail_filepath =~ "media_items/#{media_item.id}/thumbnail.jpg"
end
test "errors for non-downloadable media are passed through", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)
assert {:error, :unsuitable_for_download} = MediaDownloader.download_for_media_item(media_item)
end
test "non-recoverable errors are passed through", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
{:error, :some_error, 1}
@ -67,6 +76,36 @@ defmodule Pinchflat.Downloading.MediaDownloaderTest do
end
end
describe "download_for_media_item/3 when testing non-downloadable media" do
test "calls the download runner if the media is currently downloadable", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "was_live"})}
end)
expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot, _addl ->
{:ok, render_metadata(:media_metadata)}
end)
assert {:ok, _} = MediaDownloader.download_for_media_item(media_item)
end
test "does not call the download runner if the media is not downloadable", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)
expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot, _addl -> {:ok, ""} end)
assert {:error, :unsuitable_for_download} = MediaDownloader.download_for_media_item(media_item)
end
test "returns unexpected errors from the download status determination method", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, :what_tha} end)
assert {:error, "Unknown error: {:error, :what_tha}"} = MediaDownloader.download_for_media_item(media_item)
end
end
describe "download_for_media_item/3 when testing override options" do
test "includes override opts if specified", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, opts, _ot, _addl ->

View file

@ -58,6 +58,64 @@ defmodule Pinchflat.YtDlp.MediaTest do
end
end
describe "get_downloadable_status/1" do
test "returns :downloadable if the media was never live" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "not_live"})}
end)
assert {:ok, :downloadable} = Media.get_downloadable_status(@media_url)
end
test "returns :downloadable if the media was live and has been processed" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "was_live"})}
end)
assert {:ok, :downloadable} = Media.get_downloadable_status(@media_url)
end
test "returns :downloadable if the media's live_status is nil" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => nil})}
end)
assert {:ok, :downloadable} = Media.get_downloadable_status(@media_url)
end
test "returns :ignorable if the media is currently live" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)
assert {:ok, :ignorable} = Media.get_downloadable_status(@media_url)
end
test "returns :ignorable if the media is scheduled to be live" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_upcoming"})}
end)
assert {:ok, :ignorable} = Media.get_downloadable_status(@media_url)
end
test "returns :ignorable if the media was live but hasn't been processed" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "post_live"})}
end)
assert {:ok, :ignorable} = Media.get_downloadable_status(@media_url)
end
test "returns an error if the downloadable status can't be determined" do
expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "what_tha"})}
end)
assert {:error, "Unknown live status: what_tha"} = Media.get_downloadable_status(@media_url)
end
end
describe "download_thumbnail/2" do
test "calls the backend runner with the expected arguments" do
expect(YtDlpRunnerMock, :run, fn @media_url, opts, ot, _addl ->