From 04b14719ee4ee7fa4ab989f2f7728e7be84c0433 Mon Sep 17 00:00:00 2001 From: Kieran Date: Thu, 2 May 2024 11:06:10 -0700 Subject: [PATCH] [Enhancement] Add Media Center support for videos uploaded on the same day (#221) * Added upload date index field to media_items * Added incrementing index for upload dates * Added media item upload date index to download option builder * Added new season_episode_index_from_date to UI; updated parser * Improve support for channels * Hopefully fixed flakey test --- .../downloading/download_option_builder.ex | 31 ++-- .../downloading/output_path_builder.ex | 13 +- lib/pinchflat/media/media_item.ex | 29 ++++ lib/pinchflat/media/media_query.ex | 8 + .../media_profiles/media_profile_html.ex | 8 +- ...5_add_upload_date_index_to_media_items.exs | 11 ++ .../download_option_builder_test.exs | 15 ++ .../downloading/output_path_builder_test.exs | 10 ++ test/pinchflat/media_test.exs | 144 ++++++++++++++++++ .../metadata/metadata_parser_test.exs | 8 +- 10 files changed, 261 insertions(+), 16 deletions(-) create mode 100644 priv/repo/migrations/20240502161955_add_upload_date_index_to_media_items.exs diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index a9eeba5..0627bd6 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -35,12 +35,19 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do Builds the output path for yt-dlp to download media based on the given source's media profile. Uses the source's override output path template if it exists. + Accepts a %MediaItem{} or %Source{} struct. If a %Source{} struct is passed, it + will use a default %MediaItem{} struct with the given source. + Returns binary() """ - def build_output_path_for(%Source{} = source_with_preloads) do - output_path_template = Sources.output_path_template(source_with_preloads) + def build_output_path_for(%MediaItem{} = media_item_with_preloads) do + output_path_template = Sources.output_path_template(media_item_with_preloads.source) - build_output_path(output_path_template, source_with_preloads) + build_output_path(output_path_template, media_item_with_preloads) + end + + def build_output_path_for(%Source{} = source_with_preloads) do + build_output_path_for(%MediaItem{source: source_with_preloads}) end defp default_options do @@ -168,23 +175,29 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do defp output_options(media_item_with_preloads) do [ - output: build_output_path_for(media_item_with_preloads.source) + output: build_output_path_for(media_item_with_preloads) ] end - defp build_output_path(string, source) do - additional_options_map = output_options_map(source) + defp build_output_path(string, media_item_with_preloads) do + additional_options_map = output_options_map(media_item_with_preloads) {:ok, output_path} = OutputPathBuilder.build(string, additional_options_map) Path.join(base_directory(), output_path) end - defp output_options_map(source) do + defp output_options_map(media_item_with_preloads) do + source = media_item_with_preloads.source + %{ "source_custom_name" => source.custom_name, "source_collection_id" => source.collection_id, "source_collection_name" => source.collection_name, - "source_collection_type" => source.collection_type + "source_collection_type" => to_string(source.collection_type), + "media_upload_date_index" => + media_item_with_preloads.upload_date_index + |> to_string() + |> String.pad_leading(2, "0") } end @@ -198,7 +211,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do |> String.split(~r{\.}, include_captures: true) |> List.insert_at(-3, "-thumb") |> Enum.join() - |> build_output_path(media_item_with_preloads.source) + |> build_output_path(media_item_with_preloads) end defp base_directory do diff --git a/lib/pinchflat/downloading/output_path_builder.ex b/lib/pinchflat/downloading/output_path_builder.ex index 89d3f6c..d4f6e27 100644 --- a/lib/pinchflat/downloading/output_path_builder.ex +++ b/lib/pinchflat/downloading/output_path_builder.ex @@ -9,13 +9,23 @@ defmodule Pinchflat.Downloading.OutputPathBuilder do Builds the actual final filepath from a given template. Optionally, you can pass in a map of additional options to be used in the template. + Custom options are recursively expanded _once_ so you can nest custom options + one-deep if needed. + Translates liquid-style templates into yt-dlp-style templates, leaving yt-dlp syntax intact. """ def build(template_string, additional_template_options \\ %{}) do combined_options = Map.merge(custom_yt_dlp_option_map(), additional_template_options) - TemplateParser.parse(template_string, combined_options, &identifier_fn/2) + expanded_options = + Enum.map(combined_options, fn {key, value} -> + {:ok, parse_result} = TemplateParser.parse(value, combined_options, &identifier_fn/2) + + {key, parse_result} + end) + + TemplateParser.parse(template_string, Map.new(expanded_options), &identifier_fn/2) end # The `nil` case simply wraps the identifier in yt-dlp-style syntax. This assumes that @@ -43,6 +53,7 @@ defmodule Pinchflat.Downloading.OutputPathBuilder do "upload_yyyy_mm_dd" => "%(upload_date>%Y-%m-%d)S", "season_from_date" => "%(upload_date>%Y)S", "season_episode_from_date" => "s%(upload_date>%Y)Se%(upload_date>%m%d)S", + "season_episode_index_from_date" => "s%(upload_date>%Y)Se%(upload_date>%m%d)S{{ media_upload_date_index }}", "artist_name" => "%(artist,creator,uploader,uploader_id)S" } end diff --git a/lib/pinchflat/media/media_item.ex b/lib/pinchflat/media/media_item.ex index fbf740f..f0ed063 100644 --- a/lib/pinchflat/media/media_item.ex +++ b/lib/pinchflat/media/media_item.ex @@ -9,8 +9,10 @@ defmodule Pinchflat.Media.MediaItem do alias __MODULE__ alias Pinchflat.Repo + alias Pinchflat.Sources alias Pinchflat.Tasks.Task alias Pinchflat.Sources.Source + alias Pinchflat.Media.MediaQuery alias Pinchflat.Metadata.MediaMetadata alias Pinchflat.Media.MediaItemsSearchIndex @@ -24,6 +26,7 @@ defmodule Pinchflat.Media.MediaItem do :source_id, :short_form_content, :upload_date, + :upload_date_index, :duration_seconds, # these fields are captured only on download :media_downloaded_at, @@ -66,6 +69,7 @@ defmodule Pinchflat.Media.MediaItem do field :media_downloaded_at, :utc_datetime field :media_redownloaded_at, :utc_datetime field :upload_date, :date + field :upload_date_index, :integer, default: 0 field :duration_seconds, :integer field :media_filepath, :string @@ -100,6 +104,7 @@ defmodule Pinchflat.Media.MediaItem do |> cast(attrs, @allowed_fields) |> cast_assoc(:metadata, with: &MediaMetadata.changeset/2, required: false) |> dynamic_default(:uuid, fn _ -> Ecto.UUID.generate() end) + |> update_upload_date_index() |> validate_required(@required_fields) |> unique_constraint([:media_id, :source_id]) end @@ -124,6 +129,30 @@ defmodule Pinchflat.Media.MediaItem do ~w(__meta__ __struct__ metadata tasks media_items_search_index)a end + defp update_upload_date_index(%{changes: changes} = changeset) when is_map_key(changes, :upload_date) do + source_id = get_field(changeset, :source_id) + source = Sources.get_source!(source_id) + # Channels should count down from 99, playlists should count up from 0 + # This reflects the fact that channels prepend new videos to the top of the list + # and playlists append new videos to the bottom of the list. + default_index = if source.collection_type == :channel, do: 99, else: 0 + aggregator = if source.collection_type == :channel, do: :min, else: :max + change_direction = if source.collection_type == :channel, do: -1, else: 1 + + current_max = + MediaQuery.new() + |> MediaQuery.for_source(source_id) + |> MediaQuery.where_uploaded_on_date(changes.upload_date) + |> Repo.aggregate(aggregator, :upload_date_index) + + case current_max do + nil -> put_change(changeset, :upload_date_index, default_index) + max -> put_change(changeset, :upload_date_index, max + change_direction) + end + end + + defp update_upload_date_index(changeset), do: changeset + defimpl Jason.Encoder, for: MediaItem do def encode(value, opts) do value diff --git a/lib/pinchflat/media/media_query.ex b/lib/pinchflat/media/media_query.ex index 024f745..53e2c35 100644 --- a/lib/pinchflat/media/media_query.ex +++ b/lib/pinchflat/media/media_query.ex @@ -25,6 +25,10 @@ defmodule Pinchflat.Media.MediaQuery do MediaItem end + def for_source(query, source_id) when is_integer(source_id) do + where(query, [mi], mi.source_id == ^source_id) + end + def for_source(query, source) do where(query, [mi], mi.source_id == ^source.id) end @@ -99,6 +103,10 @@ defmodule Pinchflat.Media.MediaQuery do |> where([mi, source], is_nil(source.download_cutoff_date) or mi.upload_date >= source.download_cutoff_date) end + def where_uploaded_on_date(query, date) do + where(query, [mi], mi.upload_date == ^date) + end + def where_download_not_prevented(query) do where(query, [mi], mi.prevent_download == false) end diff --git a/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex b/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex index 6395214..aae74ce 100644 --- a/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex +++ b/lib/pinchflat_web/controllers/media_profiles/media_profile_html.ex @@ -63,7 +63,11 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileHTML do source_collection_name: "the YouTube name of the sources that use this profile (often the same as source_custom_name)", source_collection_type: "the collection type of the sources using this profile. Either 'channel' or 'playlist'", - artist_name: "the name of the artist with fallbacks to other uploader fields" + artist_name: "the name of the artist with fallbacks to other uploader fields", + season_from_date: "alias for upload_year", + season_episode_from_date: "the upload date formatted as sYYYYeMMDD", + season_episode_index_from_date: + "the upload date formatted as sYYYYeMMDDII where II is an index to prevent date collisions" } end @@ -94,7 +98,7 @@ defmodule PinchflatWeb.MediaProfiles.MediaProfileHTML do end defp media_center_output_template do - "/shows/{{ source_custom_name }}/Season {{ season_from_date }}/{{ season_episode_from_date }} - {{ title }}.{{ ext }}" + "/shows/{{ source_custom_name }}/Season {{ season_from_date }}/{{ season_episode_index_from_date }} - {{ title }}.{{ ext }}" end defp audio_output_template do diff --git a/priv/repo/migrations/20240502161955_add_upload_date_index_to_media_items.exs b/priv/repo/migrations/20240502161955_add_upload_date_index_to_media_items.exs new file mode 100644 index 0000000..d40aceb --- /dev/null +++ b/priv/repo/migrations/20240502161955_add_upload_date_index_to_media_items.exs @@ -0,0 +1,11 @@ +defmodule Pinchflat.Repo.Migrations.AddUploadDateIndexToMediaItems do + use Ecto.Migration + + def change do + alter table(:media_items) do + add :upload_date_index, :integer, null: false, default: 0 + end + + create index("media_items", [:upload_date]) + end +end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index 8008b7f..9c4fc3f 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -33,6 +33,15 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert {:output, "/tmp/test/media/#{media_item.source.custom_name}.%(ext)s"} in res end + test "respects custom media_item-related output path options", %{media_item: media_item} do + media_item = + update_media_profile_attribute(media_item, %{output_path_template: "{{ media_upload_date_index }}.%(ext)s"}) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + assert {:output, "/tmp/test/media/99.%(ext)s"} in res + end + test "uses source's output override if present", %{media_item: media_item} do source = media_item.source {:ok, _} = Sources.update_source(source, %{output_path_template_override: "override.%(ext)s"}) @@ -386,6 +395,12 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do end describe "build_output_path_for/1" do + test "builds an output path for a media item", %{media_item: media_item} do + path = DownloadOptionBuilder.build_output_path_for(media_item) + + assert path == "/tmp/test/media/%(title)S.%(ext)s" + end + test "builds an output path for a source", %{media_item: media_item} do path = DownloadOptionBuilder.build_output_path_for(media_item.source) diff --git a/test/pinchflat/downloading/output_path_builder_test.exs b/test/pinchflat/downloading/output_path_builder_test.exs index 57051f6..c9a6660 100644 --- a/test/pinchflat/downloading/output_path_builder_test.exs +++ b/test/pinchflat/downloading/output_path_builder_test.exs @@ -27,5 +27,15 @@ defmodule Pinchflat.Downloading.OutputPathBuilderTest do assert res == "/videos/%(title)s.%(ext)s" end + + test "recursively expands variables" do + additional_options = %{ + "media_upload_date_index" => "99" + } + + assert {:ok, res} = OutputPathBuilder.build("{{ season_episode_index_from_date }}.{{ ext }}", additional_options) + + assert res == "s%(upload_date>%Y)Se%(upload_date>%m%d)S99.%(ext)S" + end end end diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index cf238c1..a6468e0 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -37,6 +37,150 @@ defmodule Pinchflat.MediaTest do end end + describe "schema when testing upload_date_index and source is a channel" do + test "upload_date_index is set to 99 if it's the only video uploaded that day" do + upload_date = Date.utc_today() + source = source_fixture(%{collection_type: :channel}) + media_item = media_item_fixture(%{source_id: source.id, upload_date: upload_date}) + + assert media_item.upload_date_index == 99 + end + + test "upload_date_index is set to 98 if it's the second video uploaded that day" do + upload_date = Date.utc_today() + source = source_fixture(%{collection_type: :channel}) + + media_item_one = media_item_fixture(%{source_id: source.id, upload_date: upload_date}) + media_item_two = media_item_fixture(%{source_id: source.id, upload_date: upload_date}) + + assert media_item_one.upload_date_index == 99 + assert media_item_two.upload_date_index == 98 + end + + test "upload_date_index doesn't decrement if the video is uploaded on a different day" do + today = Date.utc_today() + one_day_ago = Date.add(today, -1) + source = source_fixture(%{collection_type: :channel}) + + media_item_new = media_item_fixture(%{source_id: source.id, upload_date: today}) + media_item_old = media_item_fixture(%{source_id: source.id, upload_date: one_day_ago}) + + assert media_item_new.upload_date_index == 99 + assert media_item_old.upload_date_index == 99 + end + + test "recomputes upload_date_index if an upload_date is changed...somehow" do + today = Date.utc_today() + one_day_ago = Date.add(today, -1) + source = source_fixture(%{collection_type: :channel}) + + media_item_new = media_item_fixture(%{source_id: source.id, upload_date: today}) + media_item_old = media_item_fixture(%{source_id: source.id, upload_date: one_day_ago}) + + {:ok, updated_media_item} = Media.update_media_item(media_item_old, %{upload_date: today}) + + assert media_item_new.upload_date_index == 99 + assert updated_media_item.upload_date_index == 98 + end + + test "upload_date_index doesn't decrement if the video is for a different source" do + today = Date.utc_today() + + source_one = source_fixture(%{collection_type: :channel}) + source_two = source_fixture(%{collection_type: :channel}) + + media_item_one = media_item_fixture(%{source_id: source_one.id, upload_date: today}) + media_item_two = media_item_fixture(%{source_id: source_two.id, upload_date: today}) + + assert media_item_one.upload_date_index == 99 + assert media_item_two.upload_date_index == 99 + end + + test "upload_date_index doesn't decrement if the a video's upload_date is updated but doesn't change" do + today = Date.utc_today() + source = source_fixture(%{collection_type: :channel}) + + media_item_one = media_item_fixture(%{source_id: source.id, upload_date: today}) + _media_item_two = media_item_fixture(%{source_id: source.id, upload_date: today}) + + {:ok, updated_media_item} = Media.update_media_item(media_item_one, %{upload_date: today, title: "New title"}) + + assert updated_media_item.upload_date_index == 99 + end + end + + describe "schema when testing upload_date_index and source is a playlist" do + test "upload_date_index is set to 0 if it's the only video uploaded that day" do + upload_date = Date.utc_today() + source = source_fixture(%{collection_type: :playlist}) + media_item = media_item_fixture(%{source_id: source.id, upload_date: upload_date}) + + assert media_item.upload_date_index == 0 + end + + test "upload_date_index is set to 1 if it's the second video uploaded that day" do + upload_date = Date.utc_today() + source = source_fixture(%{collection_type: :playlist}) + + media_item_one = media_item_fixture(%{source_id: source.id, upload_date: upload_date}) + media_item_two = media_item_fixture(%{source_id: source.id, upload_date: upload_date}) + + assert media_item_one.upload_date_index == 0 + assert media_item_two.upload_date_index == 1 + end + + test "upload_date_index doesn't increment if the video is uploaded on a different day" do + today = Date.utc_today() + one_day_ago = Date.add(today, -1) + source = source_fixture(%{collection_type: :playlist}) + + media_item_new = media_item_fixture(%{source_id: source.id, upload_date: today}) + media_item_old = media_item_fixture(%{source_id: source.id, upload_date: one_day_ago}) + + assert media_item_new.upload_date_index == 0 + assert media_item_old.upload_date_index == 0 + end + + test "recomputes upload_date_index if an upload_date is changed...somehow" do + today = Date.utc_today() + one_day_ago = Date.add(today, -1) + source = source_fixture(%{collection_type: :playlist}) + + media_item_new = media_item_fixture(%{source_id: source.id, upload_date: today}) + media_item_old = media_item_fixture(%{source_id: source.id, upload_date: one_day_ago}) + + {:ok, updated_media_item} = Media.update_media_item(media_item_old, %{upload_date: today}) + + assert media_item_new.upload_date_index == 0 + assert updated_media_item.upload_date_index == 1 + end + + test "upload_date_index doesn't increment if the video is for a different source" do + today = Date.utc_today() + + source_one = source_fixture(%{collection_type: :playlist}) + source_two = source_fixture(%{collection_type: :playlist}) + + media_item_one = media_item_fixture(%{source_id: source_one.id, upload_date: today}) + media_item_two = media_item_fixture(%{source_id: source_two.id, upload_date: today}) + + assert media_item_one.upload_date_index == 0 + assert media_item_two.upload_date_index == 0 + end + + test "upload_date_index doesn't increment if the a video's upload_date is updated but doesn't change" do + today = Date.utc_today() + source = source_fixture(%{collection_type: :playlist}) + + media_item_one = media_item_fixture(%{source_id: source.id, upload_date: today}) + _media_item_two = media_item_fixture(%{source_id: source.id, upload_date: today}) + + {:ok, updated_media_item} = Media.update_media_item(media_item_one, %{upload_date: today, title: "New title"}) + + assert updated_media_item.upload_date_index == 0 + end + end + describe "list_media_items/0" do test "it returns all media_items" do media_item = media_item_fixture() diff --git a/test/pinchflat/metadata/metadata_parser_test.exs b/test/pinchflat/metadata/metadata_parser_test.exs index dd55a19..ba7e798 100644 --- a/test/pinchflat/metadata/metadata_parser_test.exs +++ b/test/pinchflat/metadata/metadata_parser_test.exs @@ -106,7 +106,7 @@ defmodule Pinchflat.Metadata.MetadataParserTest do :ok = File.cp(thumbnail_filepath_fixture(), thumbnail_filepath) - on_exit(fn -> File.rm(thumbnail_filepath) end) + on_exit(fn -> File.rm_rf(thumbnail_filepath) end) {:ok, filepath: thumbnail_filepath} end @@ -127,7 +127,7 @@ defmodule Pinchflat.Metadata.MetadataParserTest do end test "doesn't include thumbnail if the file doesn't exist on-disk", %{metadata: metadata, filepath: filepath} do - File.rm(filepath) + File.rm_rf(filepath) result = Parser.parse_for_media_item(metadata) @@ -156,7 +156,7 @@ defmodule Pinchflat.Metadata.MetadataParserTest do infojson_filepath = metadata["infojson_filename"] :ok = File.cp(infojson_filepath_fixture(), infojson_filepath) - on_exit(fn -> File.rm(infojson_filepath) end) + on_exit(fn -> File.rm_rf(infojson_filepath) end) {:ok, filepath: infojson_filepath} end @@ -168,7 +168,7 @@ defmodule Pinchflat.Metadata.MetadataParserTest do end test "doesn't include metadata if the file doesn't exist on-disk", %{metadata: metadata, filepath: filepath} do - File.rm(filepath) + File.rm_rf(filepath) result = Parser.parse_for_media_item(metadata)