From b6e03b205807407e2ca777ce08ffe51bc0f6069c Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Mon, 18 Mar 2024 11:17:38 -0700 Subject: [PATCH] Hooked up series directory finding to source metadata runner --- .../downloading/download_option_builder.ex | 27 +++-- .../metadata/metadata_file_helpers.ex | 37 ++++++ .../source_metadata_storage_worker.ex | 33 ++++- lib/pinchflat/sources/source.ex | 7 ++ lib/pinchflat/sources/sources.ex | 4 +- lib/pinchflat/yt_dlp/media_collection.ex | 14 ++- ...20240318161405_add_nfo_path_to_sources.exs | 11 ++ .../download_option_builder_test.exs | 8 ++ .../metadata/metadata_file_helpers_test.exs | 48 ++++++++ .../source_metadata_storage_worker_test.exs | 113 +++++++++++++----- .../yt_dlp/media_collection_test.exs | 2 +- test/support/fixtures/sources_fixtures.ex | 14 +++ 12 files changed, 267 insertions(+), 51 deletions(-) create mode 100644 priv/repo/migrations/20240318161405_add_nfo_path_to_sources.exs diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index 49a688c..b9233d1 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -3,6 +3,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do Builds the options for yt-dlp to download media based on the given media profile. """ + alias Pinchflat.Sources.Source alias Pinchflat.Media.MediaItem alias Pinchflat.Downloading.OutputPathBuilder @@ -26,6 +27,18 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do {:ok, built_options} end + @doc """ + Builds the output path for yt-dlp to download media based on the given source's + media profile. + + Returns binary() + """ + def build_output_path_for(%Source{} = source_with_preloads) do + output_path_template = source_with_preloads.media_profile.output_path_template + + build_output_path(output_path_template, source_with_preloads) + end + defp default_options do [:no_progress, :windows_filenames] end @@ -104,23 +117,19 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end defp output_options(media_item_with_preloads) do - output_path_template = media_item_with_preloads.source.media_profile.output_path_template - [ - output: build_output_path(output_path_template, media_item_with_preloads) + output: build_output_path_for(media_item_with_preloads.source) ] end - defp build_output_path(string, media_item_with_preloads) do - additional_options_map = output_options_map(media_item_with_preloads) + defp build_output_path(string, source) do + additional_options_map = output_options_map(source) {:ok, output_path} = OutputPathBuilder.build(string, additional_options_map) Path.join(base_directory(), output_path) end - defp output_options_map(media_item_with_preloads) do - source = media_item_with_preloads.source - + defp output_options_map(source) do %{ "source_custom_name" => source.custom_name, "source_collection_type" => source.collection_type @@ -137,7 +146,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) + |> build_output_path(media_item_with_preloads.source) end defp base_directory do diff --git a/lib/pinchflat/metadata/metadata_file_helpers.ex b/lib/pinchflat/metadata/metadata_file_helpers.ex index f1e768d..dc9d95d 100644 --- a/lib/pinchflat/metadata/metadata_file_helpers.ex +++ b/lib/pinchflat/metadata/metadata_file_helpers.ex @@ -63,6 +63,43 @@ defmodule Pinchflat.Metadata.MetadataFileHelpers do Date.from_iso8601!("#{year}-#{month}-#{day}") end + @doc """ + Attempts to determine the series directory from a media filepath. + The series directory is the "root" directory for a given source + which should contain all the season-level folders of that source. + + Used for determining where to store things like NFO data and banners + for media center apps. Not useful without a media center app. + + Returns {:ok, binary()} | {:error, :indeterminable} + """ + def series_directory_from_media_filepath(media_filepath) do + # Matches "s" or "season" (case-insensitive) + # followed by an optional non-word character (. or _ or , etc) + # followed by at least one digit + # followed immediately by the end of the string + # Example matches: s1, s.1, s01 season 1, Season.01, Season_1, Season 1, Season1 + # Example non-matches: s01e01, season, series 1, + season_regex = ~r/^s(eason)?(\W|_)?\d{1,}$/i + + {series_directory, found_series_directory} = + media_filepath + |> Path.split() + |> Enum.reduce_while({[], false}, fn part, {directory_acc, _} -> + if String.match?(part, season_regex) do + {:halt, {directory_acc, true}} + else + {:cont, {directory_acc ++ [part], false}} + end + end) + + if found_series_directory do + {:ok, Path.join(series_directory)} + else + {:error, :indeterminable} + end + end + defp fetch_thumbnail_from_url(url) do http_client = Application.get_env(:pinchflat, :http_client, Pinchflat.HTTP.HTTPClient) {:ok, body} = http_client.get(url, [], body_format: :binary) diff --git a/lib/pinchflat/metadata/source_metadata_storage_worker.ex b/lib/pinchflat/metadata/source_metadata_storage_worker.ex index 7d298c2..c430213 100644 --- a/lib/pinchflat/metadata/source_metadata_storage_worker.ex +++ b/lib/pinchflat/metadata/source_metadata_storage_worker.ex @@ -6,8 +6,8 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorker do tags: ["media_source", "source_metadata", "remote_metadata"], max_attempts: 1, # This is the only thing stopping this job from calling itself - # in an infinite loop. - unique: [period: 600] + # in an infinite loop. Time is in seconds + unique: [period: 120] require Logger @@ -17,6 +17,7 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorker do alias Pinchflat.Sources alias Pinchflat.YtDlp.MediaCollection alias Pinchflat.Metadata.MetadataFileHelpers + alias Pinchflat.Downloading.DownloadOptionBuilder @doc """ Starts the source metadata storage worker and creates a task for the source. @@ -30,21 +31,25 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorker do end @doc """ - Fetches and stores metadata for a source in the secret metadata location. + Fetches and stores various forms of metadata for a source: + - JSON metadata for internal use + - The series directory for the source + - The NFO file for the source (if specified) Returns :ok """ @impl Oban.Worker def perform(%Oban.Job{args: %{"id" => source_id}}) do - source = Repo.preload(Sources.get_source!(source_id), :metadata) - {:ok, metadata} = MediaCollection.get_source_metadata(source.original_url) + source = Repo.preload(Sources.get_source!(source_id), [:metadata, :media_profile]) + series_directory = determine_series_directory(source) # Since updating a source kicks this job off again, we enforce job uniqueness (above) # to once, per source, per x minutes. This is to prevent a job from calling itself # in an infinite loop. Sources.update_source(source, %{ + series_directory: series_directory, metadata: %{ - metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(source, metadata) + metadata_filepath: store_source_metadata(source) } }) @@ -53,4 +58,20 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorker do Ecto.NoResultsError -> Logger.info("#{__MODULE__} discarded: source #{source_id} not found") Ecto.StaleEntryError -> Logger.info("#{__MODULE__} discarded: source #{source_id} stale") end + + defp store_source_metadata(source) do + {:ok, metadata} = MediaCollection.get_source_metadata(source.original_url) + + MetadataFileHelpers.compress_and_store_metadata_for(source, metadata) + end + + defp determine_series_directory(source) do + output_path = DownloadOptionBuilder.build_output_path_for(source) + {:ok, %{filepath: filepath}} = MediaCollection.get_source_details(source.original_url, output: output_path) + + case MetadataFileHelpers.series_directory_from_media_filepath(filepath) do + {:ok, series_directory} -> series_directory + {:error, _} -> nil + end + end end diff --git a/lib/pinchflat/sources/source.ex b/lib/pinchflat/sources/source.ex index 6a39e5e..1d393c2 100644 --- a/lib/pinchflat/sources/source.ex +++ b/lib/pinchflat/sources/source.ex @@ -17,6 +17,9 @@ defmodule Pinchflat.Sources.Source do collection_id collection_type custom_name + download_nfo + nfo_filepath + series_directory index_frequency_minutes fast_index download_media @@ -37,6 +40,7 @@ defmodule Pinchflat.Sources.Source do download_media original_url media_profile_id + download_nfo )a @pre_insert_required_fields @initially_required_fields ++ @@ -52,6 +56,9 @@ defmodule Pinchflat.Sources.Source do field :collection_name, :string field :collection_id, :string field :collection_type, Ecto.Enum, values: [:channel, :playlist] + field :download_nfo, :boolean, default: false + field :nfo_filepath, :string + field :series_directory, :string field :index_frequency_minutes, :integer, default: 60 * 24 field :fast_index, :boolean, default: false field :download_media, :boolean, default: true diff --git a/lib/pinchflat/sources/sources.ex b/lib/pinchflat/sources/sources.ex index d7e387a..64f1e61 100644 --- a/lib/pinchflat/sources/sources.ex +++ b/lib/pinchflat/sources/sources.ex @@ -147,9 +147,7 @@ defmodule Pinchflat.Sources do end defp add_source_details_to_changeset(source, changeset) do - %Ecto.Changeset{changes: changes} = changeset - - case MediaCollection.get_source_details(changes.original_url) do + case MediaCollection.get_source_details(changeset.changes.original_url) do {:ok, source_details} -> add_source_details_by_collection_type(source, changeset, source_details) diff --git a/lib/pinchflat/yt_dlp/media_collection.ex b/lib/pinchflat/yt_dlp/media_collection.ex index 5a0912d..da86600 100644 --- a/lib/pinchflat/yt_dlp/media_collection.ex +++ b/lib/pinchflat/yt_dlp/media_collection.ex @@ -64,14 +64,14 @@ defmodule Pinchflat.YtDlp.MediaCollection do Returns {:ok, map()} | {:error, any, ...}. """ - def get_source_details(source_url) do + def get_source_details(source_url, addl_opts \\ []) do # `ignore_no_formats_error` is necessary because yt-dlp will error out if # the first video has not released yet (ie: is a premier). We don't care about # available formats since we're just getting the source details - opts = [:simulate, :skip_download, :ignore_no_formats_error, playlist_end: 1] - output_template = "%(.{channel,channel_id,playlist_id,playlist_title})j" + command_opts = [:simulate, :skip_download, :ignore_no_formats_error, playlist_end: 1] ++ addl_opts + output_template = "%(.{channel,channel_id,playlist_id,playlist_title,filename})j" - with {:ok, output} <- backend_runner().run(source_url, opts, output_template), + with {:ok, output} <- backend_runner().run(source_url, command_opts, output_template), {:ok, parsed_json} <- Phoenix.json_library().decode(output) do {:ok, format_source_details(parsed_json)} else @@ -112,7 +112,11 @@ defmodule Pinchflat.YtDlp.MediaCollection do channel_id: response["channel_id"], channel_name: response["channel"], playlist_id: response["playlist_id"], - playlist_name: response["playlist_title"] + playlist_name: response["playlist_title"], + # It's not a name, it's a path dammit! + # This actually isn't used for the inital response - it's + # used later to update a source's metadata + filepath: response["filename"] } end diff --git a/priv/repo/migrations/20240318161405_add_nfo_path_to_sources.exs b/priv/repo/migrations/20240318161405_add_nfo_path_to_sources.exs new file mode 100644 index 0000000..dd336c3 --- /dev/null +++ b/priv/repo/migrations/20240318161405_add_nfo_path_to_sources.exs @@ -0,0 +1,11 @@ +defmodule Pinchflat.Repo.Migrations.AddNfoPathToSources do + use Ecto.Migration + + def change do + alter table(:sources) do + add :download_nfo, :boolean, default: false, null: false + add :nfo_filepath, :string + add :series_directory, :string + end + end +end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index 3f759b3..02103d6 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -222,6 +222,14 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do end end + describe "build_output_path_for/1" do + test "builds an output path for a source", %{media_item: media_item} do + path = DownloadOptionBuilder.build_output_path_for(media_item.source) + + assert path == "/tmp/test/media/%(title)S.%(ext)s" + end + end + defp update_media_profile_attribute(media_item_with_preloads, attrs) do media_item_with_preloads.source.media_profile |> Profiles.change_media_profile(attrs) diff --git a/test/pinchflat/metadata/metadata_file_helpers_test.exs b/test/pinchflat/metadata/metadata_file_helpers_test.exs index b28e497..511ec89 100644 --- a/test/pinchflat/metadata/metadata_file_helpers_test.exs +++ b/test/pinchflat/metadata/metadata_file_helpers_test.exs @@ -92,4 +92,52 @@ defmodule Pinchflat.Metadata.MetadataFileHelpersTest do assert Helpers.parse_upload_date(upload_date) == ~D[2021-01-01] end end + + describe "series_directory_from_media_filepath/1" do + test "returns base series directory if filepaths are setup as expected" do + good_filepaths = [ + "/media/season1/episode.mp4", + "/media/season 1/episode.mp4", + "/media/season.1/episode.mp4", + "/media/season_1/episode.mp4", + "/media/season-1/episode.mp4", + "/media/SEASON 1/episode.mp4", + "/media/SEASON.1/episode.mp4", + "/media/s1/episode.mp4", + "/media/s.1/episode.mp4", + "/media/s_1/episode.mp4", + "/media/s-1/episode.mp4", + "/media/s 1/episode.mp4", + "/media/S1/episode.mp4", + "/media/S.1/episode.mp4" + ] + + for filepath <- good_filepaths do + assert {:ok, "/media"} = Helpers.series_directory_from_media_filepath(filepath) + end + end + + test "returns an error if the season filepath can't be determined" do + bad_filepaths = [ + "/media/1/episode.mp4", + "/media/(s1)/episode.mp4", + "/media/episode.mp4", + "/media/s1e1/episode.mp4", + "/media/s1 e1/episode.mp4", + "/media/s1 (something else)/episode.mp4", + "/media/season1e1/episode.mp4", + "/media/season1 e1/episode.mp4", + "/media/seasoning1/episode.mp4", + "/media/season/episode.mp4", + "/media/series1/episode.mp4", + "/media/s/episode.mp4", + "/media/foo", + "/media/bar/" + ] + + for filepath <- bad_filepaths do + assert {:error, :indeterminable} = Helpers.series_directory_from_media_filepath(filepath) + end + end + end end diff --git a/test/pinchflat/metadata/source_metadata_storage_worker_test.exs b/test/pinchflat/metadata/source_metadata_storage_worker_test.exs index 41b191e..0a24e49 100644 --- a/test/pinchflat/metadata/source_metadata_storage_worker_test.exs +++ b/test/pinchflat/metadata/source_metadata_storage_worker_test.exs @@ -6,6 +6,9 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do alias Pinchflat.Metadata.MetadataFileHelpers alias Pinchflat.Metadata.SourceMetadataStorageWorker + @source_details_ot "%(.{channel,channel_id,playlist_id,playlist_title,filename})j" + @metadata_ot "playlist:%()j" + setup :verify_on_exit! describe "kickoff_with_task/1" do @@ -27,33 +30,12 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do end describe "perform/1" do - test "sets metadata location for source" do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end) - source = Repo.preload(source_fixture(), :metadata) - - refute source.metadata - perform_job(SourceMetadataStorageWorker, %{id: source.id}) - source = Repo.preload(Repo.reload(source), :metadata) - - assert source.metadata.metadata_filepath - - File.rm!(source.metadata.metadata_filepath) - end - - test "fetches and stores returned metadata for source" do - source = source_fixture() - file_contents = Phoenix.json_library().encode!(%{"title" => "test"}) - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, file_contents} end) - - perform_job(SourceMetadataStorageWorker, %{id: source.id}) - source = Repo.preload(Repo.reload(source), :metadata) - {:ok, metadata} = MetadataFileHelpers.read_compressed_metadata(source.metadata.metadata_filepath) - - assert metadata == %{"title" => "test"} - end - test "won't call itself in an infinite loop" do - stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end) + stub(YtDlpRunnerMock, :run, fn + _url, _opts, ot when ot == @source_details_ot -> {:ok, source_details_return_fixture()} + _url, _opts, ot when ot == @metadata_ot -> {:ok, "{}"} + end) + source = source_fixture() perform_job(SourceMetadataStorageWorker, %{id: source.id}) @@ -63,7 +45,11 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do end test "doesn't prevent over source jobs from running" do - stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end) + stub(YtDlpRunnerMock, :run, fn + _url, _opts, ot when ot == @source_details_ot -> {:ok, source_details_return_fixture()} + _url, _opts, ot when ot == @metadata_ot -> {:ok, "{}"} + end) + source_1 = source_fixture() source_2 = source_fixture() @@ -79,4 +65,77 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do assert :ok = perform_job(SourceMetadataStorageWorker, %{id: 0}) end end + + describe "perform/1 when testing metadata storage" do + test "sets metadata location for source" do + stub(YtDlpRunnerMock, :run, fn + _url, _opts, ot when ot == @source_details_ot -> {:ok, source_details_return_fixture()} + _url, _opts, ot when ot == @metadata_ot -> {:ok, "{}"} + end) + + source = Repo.preload(source_fixture(), :metadata) + + refute source.metadata + perform_job(SourceMetadataStorageWorker, %{id: source.id}) + source = Repo.preload(Repo.reload(source), :metadata) + + assert source.metadata.metadata_filepath + + File.rm!(source.metadata.metadata_filepath) + end + + test "fetches and stores returned metadata for source" do + source = source_fixture() + file_contents = Phoenix.json_library().encode!(%{"title" => "test"}) + + stub(YtDlpRunnerMock, :run, fn + _url, _opts, ot when ot == @source_details_ot -> {:ok, source_details_return_fixture()} + _url, _opts, ot when ot == @metadata_ot -> {:ok, file_contents} + end) + + perform_job(SourceMetadataStorageWorker, %{id: source.id}) + source = Repo.preload(Repo.reload(source), :metadata) + {:ok, metadata} = MetadataFileHelpers.read_compressed_metadata(source.metadata.metadata_filepath) + + assert metadata == %{"title" => "test"} + end + end + + describe "perform/1 when determining the series_directory" do + test "sets the series directory based on the returned media filepath" do + stub(YtDlpRunnerMock, :run, fn + _url, _opts, ot when ot == @source_details_ot -> + filename = Path.join([Application.get_env(:pinchflat, :media_directory), "Season 1", "bar.mp4"]) + + {:ok, source_details_return_fixture(%{filename: filename})} + + _url, _opts, ot when ot == @metadata_ot -> + {:ok, "{}"} + end) + + source = source_fixture(%{series_directory: nil}) + perform_job(SourceMetadataStorageWorker, %{id: source.id}) + source = Repo.reload(source) + + assert source.series_directory + end + + test "does not set the series directory if it cannot be determined" do + stub(YtDlpRunnerMock, :run, fn + _url, _opts, ot when ot == @source_details_ot -> + filename = Path.join([Application.get_env(:pinchflat, :media_directory), "foo", "bar.mp4"]) + + {:ok, source_details_return_fixture(%{filename: filename})} + + _url, _opts, ot when ot == @metadata_ot -> + {:ok, "{}"} + end) + + source = source_fixture(%{series_directory: nil}) + perform_job(SourceMetadataStorageWorker, %{id: source.id}) + source = Repo.reload(source) + + refute source.series_directory + end + end end diff --git a/test/pinchflat/yt_dlp/media_collection_test.exs b/test/pinchflat/yt_dlp/media_collection_test.exs index 1f88c53..8fbf6fc 100644 --- a/test/pinchflat/yt_dlp/media_collection_test.exs +++ b/test/pinchflat/yt_dlp/media_collection_test.exs @@ -97,7 +97,7 @@ defmodule Pinchflat.YtDlp.MediaCollectionTest do test "it passes the expected args to the backend runner" do expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot -> assert opts == [:simulate, :skip_download, :ignore_no_formats_error, playlist_end: 1] - assert ot == "%(.{channel,channel_id,playlist_id,playlist_title})j" + assert ot == "%(.{channel,channel_id,playlist_id,playlist_title,filename})j" {:ok, "{}"} end) diff --git a/test/support/fixtures/sources_fixtures.ex b/test/support/fixtures/sources_fixtures.ex index 7966edd..0db2e78 100644 --- a/test/support/fixtures/sources_fixtures.ex +++ b/test/support/fixtures/sources_fixtures.ex @@ -85,4 +85,18 @@ defmodule Pinchflat.SourcesFixtures do source_attributes |> Enum.map_join("\n", &Phoenix.json_library().encode!(&1)) end + + def source_details_return_fixture(attrs \\ %{}) do + channel_id = Faker.String.base64(12) + + %{ + channel_id: channel_id, + channel: "Channel Name", + playlist_id: channel_id, + playlist_title: "Channel Name", + filename: Path.join([Application.get_env(:pinchflat, :media_directory), "foo", "bar.mp4"]) + } + |> Map.merge(attrs) + |> Phoenix.json_library().encode!() + end end