Skip to content

Commit

Permalink
[Bugfix] Respect cookies preference when performing download pre-check (
Browse files Browse the repository at this point in the history 
#517)

* Updated get_downloadable_status to pass yt cookies

* Updated tests
  • Loading branch information
@kieraneglin
kieraneglin authored Dec 17, 2024
1 parent 0be469d commit a2a70fc
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 156 deletions.
5 changes: 3 additions & 2 deletions lib/pinchflat/downloading/media_downloader.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ defmodule Pinchflat.Downloading.MediaDownloader do

defp download_with_options(url, item_with_preloads, output_filepath, override_opts) do
{:ok, options} = DownloadOptionBuilder.build(item_with_preloads, override_opts)
runner_opts = [output_filepath: output_filepath, use_cookies: item_with_preloads.source.use_cookies]
use_cookies = item_with_preloads.source.use_cookies
runner_opts = [output_filepath: output_filepath, use_cookies: use_cookies]

case YtDlpMedia.get_downloadable_status(url) do
case YtDlpMedia.get_downloadable_status(url, use_cookies: use_cookies) do
{:ok, :downloadable} -> YtDlpMedia.download(url, options, runner_opts)
{:ok, :ignorable} -> {:error, :unsuitable_for_download}
err -> err
Expand Down
7 changes: 5 additions & 2 deletions lib/pinchflat/yt_dlp/media.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ defmodule Pinchflat.YtDlp.Media do
Returns {:ok, :downloadable | :ignorable} | {:error, any}
"""
def get_downloadable_status(url) do
case backend_runner().run(url, :get_downloadable_status, [:simulate, :skip_download], "%(.{live_status})j") do
def get_downloadable_status(url, addl_opts \\ []) do
action = :get_downloadable_status
command_opts = [:simulate, :skip_download]

case backend_runner().run(url, action, command_opts, "%(.{live_status})j", addl_opts) do
{:ok, output} ->
output
|> Phoenix.json_library().decode!()
Expand Down
154 changes: 81 additions & 73 deletions test/pinchflat/downloading/media_download_worker_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do

setup do
stub(YtDlpRunnerMock, :run, fn
_url, :get_downloadable_status, _opts, _ot, _addl -> {:ok, "{}"}
_url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""}
_url, :download, _opts, _ot, _addl -> {:ok, ""}
_url, :download, _opts, _ot, _addl -> {:ok, render_metadata(:media_metadata)}
end)

stub(YtDlpRunnerMock, :run, fn _url, :get_downloadable_status, _opts, _ot -> {:ok, "{}"} end)
stub(UserScriptRunnerMock, :run, fn _event_type, _data -> {:ok, "", 0} end)
stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)

Expand Down Expand Up @@ -57,13 +57,17 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

describe "perform/1" do
test "it saves attributes to the media_item", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
{:ok, render_metadata(:media_metadata)}
setup do
stub(YtDlpRunnerMock, :run, fn
_url, :get_downloadable_status, _opts, _ot, _addl -> {:ok, "{}"}
_url, :download, _opts, _ot, _addl -> {:ok, render_metadata(:media_metadata)}
_url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""}
end)

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
:ok
end

test "it saves attributes to the media_item", %{media_item: media_item} do
assert media_item.media_filepath == nil
perform_job(MediaDownloadWorker, %{id: media_item.id})
media_item = Repo.reload(media_item)
Expand All @@ -72,12 +76,6 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "it saves the metadata to the media_item", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
{:ok, render_metadata(:media_metadata)}
end)

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)

assert media_item.metadata == nil
perform_job(MediaDownloadWorker, %{id: media_item.id})
assert Repo.reload(media_item).metadata != nil
Expand All @@ -91,7 +89,10 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "it sets the job to retryable if the download fails", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl -> {:error, "error"} end)
expect(YtDlpRunnerMock, :run, 2, fn
_url, :get_downloadable_status, _opts, _ot, _addl -> {:ok, "{}"}
_url, :download, _opts, _ot, _addl -> {:error, "error"}
end)

Oban.Testing.with_testing_mode(:inline, fn ->
{:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id}))
Expand All @@ -101,8 +102,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "sets the job to retryable if the download failed and was retried", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl ->
{:error, "Unable to communicate with SponsorBlock", 1}
expect(YtDlpRunnerMock, :run, 2, fn
_url, :get_downloadable_status, _opts, _ot, _addl -> {:ok, "{}"}
_url, :download, _opts, _ot, _addl -> {:error, "Unable to communicate with SponsorBlock", 1}
end)

Oban.Testing.with_testing_mode(:inline, fn ->
Expand All @@ -113,8 +115,9 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "does not set the job to retryable if retrying wouldn't fix the issue", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl ->
{:error, "Something something Video unavailable something something", 1}
expect(YtDlpRunnerMock, :run, 2, fn
_url, :get_downloadable_status, _opts, _ot, _addl -> {:ok, "{}"}
_url, :download, _opts, _ot, _addl -> {:error, "Something something Video unavailable something something", 1}
end)

Oban.Testing.with_testing_mode(:inline, fn ->
Expand All @@ -125,7 +128,10 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "it ensures error are returned in a 2-item tuple", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl -> {:error, "error", 1} end)
expect(YtDlpRunnerMock, :run, 2, fn
_url, :get_downloadable_status, _opts, _ot, _addl -> {:ok, "{}"}
_url, :download, _opts, _ot, _addl -> {:error, "error", 1}
end)

assert {:error, :download_failed} = perform_job(MediaDownloadWorker, %{id: media_item.id})
end
Expand All @@ -147,14 +153,19 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "it saves the file's size to the database", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
metadata = render_parsed_metadata(:media_metadata)
FilesystemUtils.write_p!(metadata["filepath"], "test")
expect(YtDlpRunnerMock, :run, 3, fn
_url, :get_downloadable_status, _opts, _ot, _addl ->
{:ok, "{}"}

{:ok, Phoenix.json_library().encode!(metadata)}
end)
_url, :download, _opts, _ot, _addl ->
metadata = render_parsed_metadata(:media_metadata)
FilesystemUtils.write_p!(metadata["filepath"], "test")

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
{:ok, Phoenix.json_library().encode!(metadata)}

_url, :download_thumbnail, _opts, _ot, _addl ->
{:ok, ""}
end)

perform_job(MediaDownloadWorker, %{id: media_item.id})
media_item = Repo.reload(media_item)
Expand All @@ -163,12 +174,6 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "does not set redownloaded_at by default", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
{:ok, render_metadata(:media_metadata)}
end)

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)

perform_job(MediaDownloadWorker, %{id: media_item.id})
media_item = Repo.reload(media_item)

Expand All @@ -180,22 +185,27 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

test "sets the no_force_overwrites runner option", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, opts, _ot, _addl ->
assert :no_force_overwrites in opts
refute :force_overwrites in opts
expect(YtDlpRunnerMock, :run, 3, fn
_url, :get_downloadable_status, _opts, _ot, _addl ->
{:ok, "{}"}

{:ok, render_metadata(:media_metadata)}
end)
_url, :download, opts, _ot, _addl ->
assert :no_force_overwrites in opts
refute :force_overwrites in opts

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
{:ok, render_metadata(:media_metadata)}

_url, :download_thumbnail, _opts, _ot, _addl ->
{:ok, ""}
end)

perform_job(MediaDownloadWorker, %{id: media_item.id})
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, :get_downloadable_status, _opts, _ot ->
stub(YtDlpRunnerMock, :run, fn _url, :get_downloadable_status, _opts, _ot, _addl ->
{:ok, Phoenix.json_library().encode!(%{"live_status" => "is_live"})}
end)

Expand All @@ -209,65 +219,72 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do

describe "perform/1 when testing forced downloads" do
test "ignores 'prevent_download' if forced", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, fn _url, :download, _opts, _ot, _addl -> :ok end)

Sources.update_source(media_item.source, %{download_media: false})
Media.update_media_item(media_item, %{prevent_download: true})

perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
end

test "sets force_overwrites runner option", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, opts, _ot, _addl ->
assert :force_overwrites in opts
refute :no_force_overwrites in opts
expect(YtDlpRunnerMock, :run, 3, fn
_url, :get_downloadable_status, _opts, _ot, _addl ->
{:ok, "{}"}

{:ok, render_metadata(:media_metadata)}
end)
_url, :download, opts, _ot, _addl ->
assert :force_overwrites in opts
refute :no_force_overwrites in opts

{:ok, render_metadata(:media_metadata)}

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
_url, :download_thumbnail, _opts, _ot, _addl ->
{:ok, ""}
end)

perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
end
end

describe "perform/1 when testing re-downloads" do
test "sets redownloaded_at on the media_item", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
{:ok, render_metadata(:media_metadata)}
end)

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)

perform_job(MediaDownloadWorker, %{id: media_item.id, quality_upgrade?: true})
media_item = Repo.reload(media_item)

assert media_item.media_redownloaded_at != nil
end

test "sets force_overwrites runner option", %{media_item: media_item} do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, opts, _ot, _addl ->
assert :force_overwrites in opts
refute :no_force_overwrites in opts
expect(YtDlpRunnerMock, :run, 3, fn
_url, :get_downloadable_status, _opts, _ot, _addl ->
{:ok, "{}"}

{:ok, render_metadata(:media_metadata)}
end)
_url, :download, opts, _ot, _addl ->
assert :force_overwrites in opts
refute :no_force_overwrites in opts

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
{:ok, render_metadata(:media_metadata)}

_url, :download_thumbnail, _opts, _ot, _addl ->
{:ok, ""}
end)

perform_job(MediaDownloadWorker, %{id: media_item.id, force: true})
end

test "deletes old files if the media item has been updated" do
expect(YtDlpRunnerMock, :run, 1, fn _url, :download, _opts, _ot, _addl ->
tmp_media_item = media_item_with_attachments()
metadata = render_parsed_metadata(:media_metadata)
metadata = Map.put(metadata, "filepath", tmp_media_item.media_filepath)
expect(YtDlpRunnerMock, :run, 3, fn
_url, :get_downloadable_status, _opts, _ot, _addl ->
{:ok, "{}"}

{:ok, Phoenix.json_library().encode!(metadata)}
end)
_url, :download, _opts, _ot, _addl ->
tmp_media_item = media_item_with_attachments()
metadata = render_parsed_metadata(:media_metadata)
metadata = Map.put(metadata, "filepath", tmp_media_item.media_filepath)

expect(YtDlpRunnerMock, :run, 1, fn _url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""} end)
{:ok, Phoenix.json_library().encode!(metadata)}

_url, :download_thumbnail, _opts, _ot, _addl ->
{:ok, ""}
end)

old_media_item = media_item_with_attachments()
perform_job(MediaDownloadWorker, %{id: old_media_item.id, force: true})
Expand All @@ -280,15 +297,6 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
end

describe "perform/1 when testing user script callbacks" do
setup do
stub(YtDlpRunnerMock, :run, fn
_url, :download, _opts, _ot, _addl -> {:ok, render_metadata(:media_metadata)}
_url, :download_thumbnail, _opts, _ot, _addl -> {:ok, ""}
end)

:ok
end

test "calls the media_pre_download user script runner", %{media_item: media_item} do
expect(UserScriptRunnerMock, :run, fn :media_pre_download, data ->
assert data.id == media_item.id
Expand Down
Loading

0 comments on commit a2a70fc

Please sign in to comment.