Skip to content

Commit

Permalink
[Bugfix] Fix reddit bugs v2 (#82)
Browse files Browse the repository at this point in the history
* Ensured thumbnail is converted to jpg before embedding

* Ensured media indexing doesn't fail if an upload date can't be parsed
  • Loading branch information
kieraneglin authored Mar 12, 2024
1 parent 8f9d18d commit a7af6a9
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lib/pinchflat/downloading/download_option_builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/pinchflat/slow_indexing/slow_indexing_helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions lib/pinchflat/yt_dlp/media.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/pinchflat/downloading/download_option_builder_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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})

Expand Down
24 changes: 24 additions & 0 deletions test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions test/pinchflat/yt_dlp/media_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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

0 comments on commit a7af6a9

Please sign in to comment.