From 1cc8731914bd8122b38ddbc759cb56cd21bbde7b Mon Sep 17 00:00:00 2001 From: Kieran Date: Tue, 12 Mar 2024 20:23:51 -0700 Subject: [PATCH] [Bugfix] Fix reddit bugs v2 (#82) * Ensured thumbnail is converted to jpg before embedding * Ensured media indexing doesn't fail if an upload date can't be parsed --- .../downloading/download_option_builder.ex | 2 +- .../slow_indexing/slow_indexing_helpers.ex | 3 ++- lib/pinchflat/yt_dlp/media.ex | 7 ++++-- .../download_option_builder_test.exs | 8 +++++++ .../slow_indexing_helpers_test.exs | 24 +++++++++++++++++++ test/pinchflat/yt_dlp/media_test.exs | 21 ++++++++++++++++ 6 files changed, 61 insertions(+), 4 deletions(-) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 0bb25e2..dabf407 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -66,7 +66,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do acc ++ [:write_thumbnail, convert_thumbnail: "jpg"] {:embed_thumbnail, true} -> - acc ++ [:embed_thumbnail] + acc ++ [:embed_thumbnail, convert_thumbnail: "jpg"] _ -> acc diff --git a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex index 81e7784..4598d1b 100644 --- a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex +++ b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex @@ -41,6 +41,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do Given a media source, creates (indexes) the media by creating media_items for each media ID in the source. Afterward, kicks off a download task for each pending media item belonging to the source. You can't tell me the method name isn't descriptive! + Returns a list of media items or changesets (if the media item couldn't be created). Indexing is slow and usually returns a list of all media data at once for record creation. To help with this, we use a file follower to watch the file that yt-dlp writes to @@ -56,7 +57,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do Since indexing returns all media data EVERY TIME, we that that opportunity to update indexing metadata for media items that have already been created. - Returns [%MediaItem{}, ...] + Returns [%MediaItem{} | %Ecto.Changeset{}] """ def index_and_enqueue_download_for_media_items(%Source{} = source) do # See the method definition below for more info on how file watchers work diff --git a/lib/pinchflat/yt_dlp/media.ex b/lib/pinchflat/yt_dlp/media.ex index 34c9276..b8bd614 100644 --- a/lib/pinchflat/yt_dlp/media.ex +++ b/lib/pinchflat/yt_dlp/media.ex @@ -85,8 +85,8 @@ defmodule Pinchflat.YtDlp.Media do description: response["description"], original_url: response["webpage_url"], livestream: response["was_live"], - short_form_content: short_form_content?(response), - upload_date: parse_upload_date(response["upload_date"]) + short_form_content: response["webpage_url"] && short_form_content?(response), + upload_date: response["upload_date"] && parse_upload_date(response["upload_date"]) } end @@ -99,6 +99,9 @@ defmodule Pinchflat.YtDlp.Media do # WILL returns false positives, but it's a best-effort approach # that should work for most cases. The aspect_ratio check is # based on a gut feeling and may need to be tweaked. + # + # These don't fail if duration or aspect_ratio are missing + # due to Elixir's comparison semantics response["duration"] <= 60 && response["aspect_ratio"] < 0.8 end end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index 8d2cc46..22fba78 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -141,6 +141,14 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert :embed_thumbnail in res end + test "convertes thumbnail to jpg when embed_thumbnail is true", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{embed_thumbnail: true}) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + assert {:convert_thumbnail, "jpg"} in res + end + test "doesn't include these options when not specified", %{media_item: media_item} do media_item = update_media_profile_attribute(media_item, %{embed_thumbnail: false, download_thumbnail: false}) diff --git a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs index 6d385a5..754f331 100644 --- a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs +++ b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs @@ -151,6 +151,30 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id) end + + test "it doesn't blow up if a media item cannot be coerced into a struct", %{source: source} do + stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts -> + response = + Phoenix.json_library().encode!(%{ + id: "video3", + title: "Video 3", + was_live: false, + description: "desc3", + # Only focusing on these because these are passed to functions that + # could fail if they're not present + webpage_url: nil, + aspect_ratio: nil, + duration: nil, + upload_date: nil + }) + + {:ok, response} + end) + + assert [changeset] = SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + + assert %Ecto.Changeset{} = changeset + end end describe "index_and_enqueue_download_for_media_items/1 when testing file watcher" do diff --git a/test/pinchflat/yt_dlp/media_test.exs b/test/pinchflat/yt_dlp/media_test.exs index 6484e8b..bdc811b 100644 --- a/test/pinchflat/yt_dlp/media_test.exs +++ b/test/pinchflat/yt_dlp/media_test.exs @@ -141,6 +141,16 @@ defmodule Pinchflat.YtDlp.MediaTest do assert %Media{short_form_content: false} = Media.response_to_struct(response) end + test "doesn't blow up if short form content-related fields are missing" do + response = %{ + "webpage_url" => nil, + "aspect_ratio" => nil, + "duration" => nil + } + + assert %Media{short_form_content: nil} = Media.response_to_struct(response) + end + test "parses the upload date" do response = %{ "webpage_url" => "https://www.youtube.com/watch?v=TiZPUDkDYbk", @@ -153,5 +163,16 @@ defmodule Pinchflat.YtDlp.MediaTest do assert %Media{upload_date: ^expected_date} = Media.response_to_struct(response) end + + test "doesn't blow up if upload date is missing" do + response = %{ + "webpage_url" => "https://www.youtube.com/watch?v=TiZPUDkDYbk", + "aspect_ratio" => 1.0, + "duration" => 61, + "upload_date" => nil + } + + assert %Media{upload_date: nil} = Media.response_to_struct(response) + end end end