diff --git a/.iex.exs b/.iex.exs index e90bc79..4ff1900 100644 --- a/.iex.exs +++ b/.iex.exs @@ -44,7 +44,7 @@ defmodule IexHelpers do :channel -> channel_url() end - SourceDetails.get_video_ids(source) + SourceDetails.get_media_attributes(source) end end diff --git a/lib/pinchflat/media/media_item.ex b/lib/pinchflat/media/media_item.ex index 1af9284..d46ca83 100644 --- a/lib/pinchflat/media/media_item.ex +++ b/lib/pinchflat/media/media_item.ex @@ -13,17 +13,21 @@ defmodule Pinchflat.Media.MediaItem do @allowed_fields ~w( title media_id + original_url + livestream media_filepath source_id subtitle_filepaths thumbnail_filepath metadata_filepath )a - @required_fields ~w(media_id source_id)a + @required_fields ~w(title original_url livestream media_id source_id)a schema "media_items" do field :title, :string field :media_id, :string + field :original_url, :string + field :livestream, :boolean, default: false field :media_filepath, :string field :thumbnail_filepath, :string field :metadata_filepath, :string diff --git a/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex b/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex index 89cb1af..66e56e1 100644 --- a/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex +++ b/lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex @@ -4,18 +4,26 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollection do videos (aka: a source [ie: channels, playlists]). """ - @doc """ - Returns a list of strings representing the video ids in the collection. + alias Pinchflat.Utils.FunctionUtils - Returns {:ok, [binary()]} | {:error, any, ...}. + @doc """ + Returns a list of maps representing the videos in the collection. + + Returns {:ok, [map()]} | {:error, any, ...}. """ - def get_video_ids(url, command_opts \\ []) do + def get_media_attributes(url, command_opts \\ []) do runner = Application.get_env(:pinchflat, :yt_dlp_runner) opts = command_opts ++ [:simulate, :skip_download] - case runner.run(url, opts, "%(id)s") do - {:ok, output} -> {:ok, String.split(output, "\n", trim: true)} - res -> res + case runner.run(url, opts, "%(.{id,title,was_live,original_url})j") do + {:ok, output} -> + output + |> String.split("\n", trim: true) + |> Enum.map(&Phoenix.json_library().decode!/1) + |> FunctionUtils.wrap_ok() + + res -> + res end end diff --git a/lib/pinchflat/media_client/source_details.ex b/lib/pinchflat/media_client/source_details.ex index dcef2d5..488edda 100644 --- a/lib/pinchflat/media_client/source_details.ex +++ b/lib/pinchflat/media_client/source_details.ex @@ -6,11 +6,8 @@ defmodule Pinchflat.MediaClient.SourceDetails do it open-ish for future expansion (just in case). """ - alias Pinchflat.Repo alias Pinchflat.MediaSource.Source - alias Pinchflat.MediaClient.Backends.YtDlp.VideoCollection, as: YtDlpSource - alias Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder, as: YtDlpIndexOptionBuilder @doc """ Gets a source's ID and name from its URL using the given backend. @@ -22,24 +19,19 @@ defmodule Pinchflat.MediaClient.SourceDetails do end @doc """ - Returns a list of video IDs for the given source URL OR source record using the given backend. + Returns a list of basic video data mapsfor the given source URL OR + source record using the given backend. - If passing a source record, the call to the backend may have custom options applied based on - the `option_builder`. - - Returns {:ok, list(binary())} | {:error, any, ...}. + Returns {:ok, [map()]} | {:error, any, ...}. """ - def get_video_ids(sourceable, backend \\ :yt_dlp) + def get_media_attributes(sourceable, backend \\ :yt_dlp) - def get_video_ids(%Source{} = source, backend) do - media_profile = Repo.preload(source, :media_profile).media_profile - {:ok, options} = option_builder(backend).build(media_profile) - - source_module(backend).get_video_ids(source.collection_id, options) + def get_media_attributes(%Source{} = source, backend) do + source_module(backend).get_media_attributes(source.collection_id) end - def get_video_ids(source_url, backend) when is_binary(source_url) do - source_module(backend).get_video_ids(source_url) + def get_media_attributes(source_url, backend) when is_binary(source_url) do + source_module(backend).get_media_attributes(source_url) end defp source_module(backend) do @@ -47,10 +39,4 @@ defmodule Pinchflat.MediaClient.SourceDetails do :yt_dlp -> YtDlpSource end end - - defp option_builder(backend) do - case backend do - :yt_dlp -> YtDlpIndexOptionBuilder - end - end end diff --git a/lib/pinchflat/media_source.ex b/lib/pinchflat/media_source.ex index 8396b8c..2ef7f3a 100644 --- a/lib/pinchflat/media_source.ex +++ b/lib/pinchflat/media_source.ex @@ -7,7 +7,6 @@ defmodule Pinchflat.MediaSource do alias Pinchflat.Repo alias Pinchflat.Tasks - alias Pinchflat.Media alias Pinchflat.Tasks.SourceTasks alias Pinchflat.MediaSource.Source alias Pinchflat.MediaClient.SourceDetails @@ -39,26 +38,6 @@ defmodule Pinchflat.MediaSource do |> commit_and_start_indexing() end - @doc """ - Given a media source, creates (indexes) the media by creating media_items for each - media ID in the source. - - Returns [%MediaItem{}, ...] | [%Ecto.Changeset{}, ...] - """ - def index_media_items(%Source{} = source) do - {:ok, media_ids} = SourceDetails.get_video_ids(source.original_url) - - media_ids - |> Enum.map(fn media_id -> - attrs = %{source_id: source.id, media_id: media_id} - - case Media.create_media_item(attrs) do - {:ok, media_item} -> media_item - {:error, changeset} -> changeset - end - end) - end - @doc """ Updates a source. May attempt to pull additional source details from the original_url (if changed). May attempt to start indexing the source's @@ -101,6 +80,9 @@ defmodule Pinchflat.MediaSource do This means that it'll go for it even if a changeset is otherwise invalid. This is pretty easy to change, but for MVP I'm not concerned. + NOTE: When operating in the ideal path, this effectively adds an API call + to the source creation/update process. Should be used only when needed. + IDEA: Maybe I could discern `collection_type` based on the original URL? It also seems like it's a channel when the returned yt-dlp channel_id is the same as the playlist_id - maybe could use that? diff --git a/lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex b/lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex deleted file mode 100644 index 2040271..0000000 --- a/lib/pinchflat/profiles/options/yt_dlp/index_option_builder.ex +++ /dev/null @@ -1,52 +0,0 @@ -defmodule Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder do - @moduledoc """ - Builds the options for yt-dlp to index a media source based on the given media profile. - """ - - alias Pinchflat.Profiles.MediaProfile - - @doc """ - Builds the options for yt-dlp to index a media source based on the given media profile. - """ - def build(%MediaProfile{} = media_profile) do - built_options = release_type_options(media_profile) - - {:ok, built_options} - end - - defp release_type_options(media_profile) do - mapped_struct = Map.from_struct(media_profile) - - # Appending multiple match filters treats them as an OR condition, - # so we have to be careful around combining `only` and `exclude` options. - # eg: only shorts + exclude livestreams = "any video that is a short OR is not a livestream" - # which will return all shorts AND normal videos. - Enum.reduce(mapped_struct, [], fn attr, acc -> - case {attr, media_profile} do - {{:shorts_behaviour, :only}, _} -> - acc ++ [match_filter: "original_url*=/shorts/"] - - {{:livestream_behaviour, :only}, _} -> - acc ++ [match_filter: "was_live"] - - # Since match_filter is an OR (see above), `exclude`s must be ignored entirely if the - # other type is set to `only`. There is also special behaviour if they're both excludes, - # hence why these check against `:include` alone. - {{:shorts_behaviour, :exclude}, %{livestream_behaviour: :include}} -> - acc ++ [match_filter: "original_url!*=/shorts/"] - - {{:livestream_behaviour, :exclude}, %{shorts_behaviour: :include}} -> - acc ++ [match_filter: "!was_live"] - - # Again, since it's an OR, there's a special syntax if they're both excluded - # to make it an AND. Note that I'm not checking for the other permutation of - # both excluding since this MUST get hit so adding the other version would double up. - {{:livestream_behaviour, :exclude}, %{shorts_behaviour: :exclude}} -> - acc ++ [match_filter: "!was_live & original_url!*=/shorts/"] - - _ -> - acc - end - end) - end -end diff --git a/lib/pinchflat/tasks/source_tasks.ex b/lib/pinchflat/tasks/source_tasks.ex index 9d2d8a5..6d16523 100644 --- a/lib/pinchflat/tasks/source_tasks.ex +++ b/lib/pinchflat/tasks/source_tasks.ex @@ -6,6 +6,7 @@ defmodule Pinchflat.Tasks.SourceTasks do alias Pinchflat.Media alias Pinchflat.Tasks alias Pinchflat.MediaSource.Source + alias Pinchflat.MediaClient.SourceDetails alias Pinchflat.Workers.MediaIndexingWorker alias Pinchflat.Workers.VideoDownloadWorker @@ -33,6 +34,32 @@ defmodule Pinchflat.Tasks.SourceTasks do end end + @doc """ + Given a media source, creates (indexes) the media by creating media_items for each + media ID in the source. + + Returns [%MediaItem{}, ...] | [%Ecto.Changeset{}, ...] + """ + def index_media_items(%Source{} = source) do + {:ok, media_attributes} = SourceDetails.get_media_attributes(source.original_url) + + media_attributes + |> Enum.map(fn media_attrs -> + attrs = %{ + source_id: source.id, + title: media_attrs["title"], + media_id: media_attrs["id"], + original_url: media_attrs["original_url"], + livestream: media_attrs["was_live"] + } + + case Media.create_media_item(attrs) do + {:ok, media_item} -> media_item + {:error, changeset} -> changeset + end + end) + end + @doc """ Starts tasks for downloading videos for any of a sources _pending_ media items. Jobs are not enqueued if the source is set to not download media. This will return :ok. diff --git a/lib/pinchflat/utils/function_utils.ex b/lib/pinchflat/utils/function_utils.ex new file mode 100644 index 0000000..c06866f --- /dev/null +++ b/lib/pinchflat/utils/function_utils.ex @@ -0,0 +1,15 @@ +defmodule Pinchflat.Utils.FunctionUtils do + @moduledoc """ + Utility functions for working with functions + """ + + @doc """ + Wraps the provided term in an :ok tuple. Useful for fulfilling a contract, but + other usage should be assessed to see if it's the right fit. + + Returns {:ok, term} + """ + def wrap_ok(value) do + {:ok, value} + end +end diff --git a/lib/pinchflat/workers/media_indexing_worker.ex b/lib/pinchflat/workers/media_indexing_worker.ex index f46e427..e8e5b37 100644 --- a/lib/pinchflat/workers/media_indexing_worker.ex +++ b/lib/pinchflat/workers/media_indexing_worker.ex @@ -47,7 +47,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do end defp index_media_and_reschedule(source) do - MediaSource.index_media_items(source) + SourceTasks.index_media_items(source) SourceTasks.enqueue_pending_media_downloads(source) source diff --git a/priv/repo/migrations/20240208194729_add_indexing_attributes_to_media_items.exs b/priv/repo/migrations/20240208194729_add_indexing_attributes_to_media_items.exs new file mode 100644 index 0000000..c9b0247 --- /dev/null +++ b/priv/repo/migrations/20240208194729_add_indexing_attributes_to_media_items.exs @@ -0,0 +1,10 @@ +defmodule Pinchflat.Repo.Migrations.AddIndexingAttributesToMediaItems do + use Ecto.Migration + + def change do + alter table(:media_items) do + add :livestream, :boolean, default: false, null: false + add :original_url, :string, null: false + end + end +end diff --git a/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs b/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs index 709050a..5230f5e 100644 --- a/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs +++ b/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs @@ -1,6 +1,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do use ExUnit.Case, async: true import Mox + import Pinchflat.MediaSourceFixtures alias Pinchflat.MediaClient.Backends.YtDlp.VideoCollection @@ -8,22 +9,23 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do setup :verify_on_exit! - describe "get_video_ids/2" do - test "returns a list of video ids with no blank elements" do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "id1\nid2\n\nid3\n"} end) + describe "get_media_attributes/2" do + test "returns a list of video attributes with no blank elements" do + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture() <> "\n\n"} end) - assert {:ok, ["id1", "id2", "id3"]} = VideoCollection.get_video_ids(@channel_url) + assert {:ok, [%{"id" => "video1"}, %{"id" => "video2"}, %{"id" => "video3"}]} = + VideoCollection.get_media_attributes(@channel_url) end test "it passes the expected default args" do expect(YtDlpRunnerMock, :run, fn _url, opts, ot -> assert opts == [:simulate, :skip_download] - assert ot == "%(id)s" + assert ot == "%(.{id,title,was_live,original_url})j" {:ok, ""} end) - assert {:ok, _} = VideoCollection.get_video_ids(@channel_url) + assert {:ok, _} = VideoCollection.get_media_attributes(@channel_url) end test "it passes the expected custom args" do @@ -33,13 +35,13 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do {:ok, ""} end) - assert {:ok, _} = VideoCollection.get_video_ids(@channel_url, [:custom_arg]) + assert {:ok, _} = VideoCollection.get_media_attributes(@channel_url, [:custom_arg]) end test "returns the error straight through when the command fails" do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "Big issue", 1} end) - assert {:error, "Big issue", 1} = VideoCollection.get_video_ids(@channel_url) + assert {:error, "Big issue", 1} = VideoCollection.get_media_attributes(@channel_url) end end diff --git a/test/pinchflat/media_client/source_details_test.exs b/test/pinchflat/media_client/source_details_test.exs index 58de272..2e1bd61 100644 --- a/test/pinchflat/media_client/source_details_test.exs +++ b/test/pinchflat/media_client/source_details_test.exs @@ -22,7 +22,7 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do assert {:ok, _} = SourceDetails.get_source_details(@channel_url) end - test "it returns a struct composed of the returned data" do + test "it returns a map composed of the returned data" do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> Phoenix.json_library().encode(%{ channel: "TheUselessTrials", @@ -43,42 +43,42 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do end end - describe "get_video_ids/2 when passed a string" do + describe "get_media_attributes/2 when passed a string" do test "it passes the expected arguments to the backend" do expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot -> assert opts == [:simulate, :skip_download] - assert ot == "%(id)s" + assert ot == "%(.{id,title,was_live,original_url})j" {:ok, ""} end) - assert {:ok, _} = SourceDetails.get_video_ids(@channel_url) + assert {:ok, _} = SourceDetails.get_media_attributes(@channel_url) end - test "it returns a list of strings" do + test "it returns a list of maps" do expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> - {:ok, "video1\nvideo2\nvideo3"} + {:ok, source_attributes_return_fixture()} end) - assert {:ok, ["video1", "video2", "video3"]} = SourceDetails.get_video_ids(@channel_url) + assert {:ok, [%{}, %{}, %{}]} = SourceDetails.get_media_attributes(@channel_url) end end - describe "get_video_ids/2 when passed a Source record" do + describe "get_media_attributes/2 when passed a Source record" do test "it calls the backend with the source's collection ID" do source = source_fixture() expect(YtDlpRunnerMock, :run, fn url, _opts, _ot -> assert source.collection_id == url - {:ok, "video1\nvideo2\nvideo3"} + {:ok, source_attributes_return_fixture()} end) - assert {:ok, _} = SourceDetails.get_video_ids(source) + assert {:ok, _} = SourceDetails.get_media_attributes(source) end test "it builds options based on the source's media profile" do expect(YtDlpRunnerMock, :run, fn _url, opts, _ot -> - assert opts == [{:match_filter, "!was_live"}, :simulate, :skip_download] + assert opts == [:simulate, :skip_download] {:ok, ""} end) @@ -89,7 +89,7 @@ defmodule Pinchflat.MediaClient.SourceDetailsTest do ) source = source_fixture(media_profile_id: media_profile.id) - assert {:ok, _} = SourceDetails.get_video_ids(source) + assert {:ok, _} = SourceDetails.get_media_attributes(source) end end end diff --git a/test/pinchflat/media_client/video_downloader_test.exs b/test/pinchflat/media_client/video_downloader_test.exs index 8356eab..b03ef8c 100644 --- a/test/pinchflat/media_client/video_downloader_test.exs +++ b/test/pinchflat/media_client/video_downloader_test.exs @@ -10,7 +10,7 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do setup do media_item = Repo.preload( - media_item_fixture(%{title: nil, media_filepath: nil}), + media_item_fixture(%{title: "Something", media_filepath: nil}), [:metadata, source: :media_profile] ) @@ -58,7 +58,6 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do end test "it extracts the title", %{media_item: media_item} do - assert media_item.title == nil assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item) assert updated_media_item.title == "Trying to Wheelie Without the Rear Brake" end diff --git a/test/pinchflat/media_source_test.exs b/test/pinchflat/media_source_test.exs index 2ba30fb..76d66ca 100644 --- a/test/pinchflat/media_source_test.exs +++ b/test/pinchflat/media_source_test.exs @@ -6,7 +6,6 @@ defmodule Pinchflat.MediaSourceTest do import Pinchflat.MediaSourceFixtures alias Pinchflat.MediaSource - alias Pinchflat.Media.MediaItem alias Pinchflat.MediaSource.Source alias Pinchflat.Workers.MediaIndexingWorker @@ -117,59 +116,6 @@ defmodule Pinchflat.MediaSourceTest do end end - describe "index_media_items/1" do - setup do - stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo2\nvideo3"} end) - - {:ok, [source: source_fixture()]} - end - - test "it creates a media_item record for each media ID returned", %{source: source} do - assert media_items = MediaSource.index_media_items(source) - - assert Enum.count(media_items) == 3 - assert ["video1", "video2", "video3"] == Enum.map(media_items, & &1.media_id) - assert Enum.all?(media_items, fn %MediaItem{} -> true end) - end - - test "it attaches all media_items to the given source", %{source: source} do - source_id = source.id - assert media_items = MediaSource.index_media_items(source) - - assert Enum.count(media_items) == 3 - assert Enum.all?(media_items, fn %MediaItem{source_id: ^source_id} -> true end) - end - - test "it won't duplicate media_items based on media_id and source", %{source: source} do - _first_run = MediaSource.index_media_items(source) - _duplicate_run = MediaSource.index_media_items(source) - - media_items = Repo.preload(source, :media_items).media_items - assert Enum.count(media_items) == 3 - end - - test "it can duplicate media_ids for different sources", %{source: source} do - other_source = source_fixture() - - media_items = MediaSource.index_media_items(source) - media_items_other_source = MediaSource.index_media_items(other_source) - - assert Enum.count(media_items) == 3 - assert Enum.count(media_items_other_source) == 3 - - assert Enum.map(media_items, & &1.media_id) == - Enum.map(media_items_other_source, & &1.media_id) - end - - test "it returns a list of media_items or changesets", %{source: source} do - first_run = MediaSource.index_media_items(source) - duplicate_run = MediaSource.index_media_items(source) - - assert Enum.all?(first_run, fn %MediaItem{} -> true end) - assert Enum.all?(duplicate_run, fn %Ecto.Changeset{} -> true end) - end - end - describe "update_source/2" do test "updates with valid data updates the source" do source = source_fixture() diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index 13d7b4b..234eba4 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -63,7 +63,8 @@ defmodule Pinchflat.MediaTest do media_id: Faker.String.base64(12), title: Faker.Commerce.product_name(), media_filepath: "/video/#{Faker.File.file_name(:video)}", - source_id: source_fixture().id + source_id: source_fixture().id, + original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}" } assert {:ok, %MediaItem{} = media_item} = Media.create_media_item(valid_attrs) diff --git a/test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs b/test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs deleted file mode 100644 index 07c315d..0000000 --- a/test/pinchflat/profiles/options/yt_dlp/index_option_builder_test.exs +++ /dev/null @@ -1,104 +0,0 @@ -defmodule Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilderTest do - use ExUnit.Case, async: true - - alias Pinchflat.Profiles.MediaProfile - alias Pinchflat.Profiles.Options.YtDlp.IndexOptionBuilder - - @media_profile %MediaProfile{ - output_path_template: "{{ title }}.%(ext)s", - shorts_behaviour: :include, - livestream_behaviour: :include - } - - describe "build/1 when testing release type options" do - test "adds correct filter when shorts_behaviour is :only" do - media_profile = %MediaProfile{@media_profile | shorts_behaviour: :only} - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "original_url*=/shorts/"} in res - refute {:match_filter, "original_url!*=/shorts/"} in res - refute {:match_filter, "!was_live"} in res - refute {:match_filter, "was_live"} in res - end - - test "adds correct filter when livestream_behaviour is :only" do - media_profile = %MediaProfile{@media_profile | livestream_behaviour: :only} - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "was_live"} in res - refute {:match_filter, "!was_live"} in res - refute {:match_filter, "!original_url*=/shorts/"} in res - refute {:match_filter, "original_url*=/shorts/"} in res - end - - test "adds correct filter when both livestreams and shorts are :only" do - media_profile = %MediaProfile{ - @media_profile - | shorts_behaviour: :only, - livestream_behaviour: :only - } - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "original_url*=/shorts/"} in res - assert {:match_filter, "was_live"} in res - refute {:match_filter, "original_url!*=/shorts/"} in res - refute {:match_filter, "!was_live"} in res - end - - test "adds correct filter when shorts_behaviour is :exclude" do - media_profile = %MediaProfile{@media_profile | shorts_behaviour: :exclude} - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "original_url!*=/shorts/"} in res - refute {:match_filter, "original_url*=/shorts/"} in res - refute {:match_filter, "was_live"} in res - refute {:match_filter, "!was_live"} in res - end - - test "adds correct filter when livestream_behaviour is :exclude" do - media_profile = %MediaProfile{@media_profile | livestream_behaviour: :exclude} - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "!was_live"} in res - refute {:match_filter, "was_live"} in res - refute {:match_filter, "original_url!*=/shorts/"} in res - refute {:match_filter, "original_url*=/shorts/"} in res - end - - test "adds correct filter when shorts and livestreams are both exclude" do - media_profile = %MediaProfile{ - @media_profile - | shorts_behaviour: :exclude, - livestream_behaviour: :exclude - } - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "!was_live & original_url!*=/shorts/"} in res - refute {:match_filter, "original_url!*=/shorts/"} in res - refute {:match_filter, "!was_live"} in res - refute {:match_filter, "original_url*=/shorts/"} in res - refute {:match_filter, "was_live"} in res - end - - test "does not add exclusion filter if one is excluded and the other is only" do - media_profile = %MediaProfile{ - @media_profile - | shorts_behaviour: :exclude, - livestream_behaviour: :only - } - - assert {:ok, res} = IndexOptionBuilder.build(media_profile) - - assert {:match_filter, "was_live"} in res - refute {:match_filter, "original_url!*=/shorts/"} in res - refute {:match_filter, "original_url*=/shorts/"} in res - refute {:match_filter, "!was_live"} in res - end - end -end diff --git a/test/pinchflat/tasks/source_tasks_test.exs b/test/pinchflat/tasks/source_tasks_test.exs index 3996da9..09f7291 100644 --- a/test/pinchflat/tasks/source_tasks_test.exs +++ b/test/pinchflat/tasks/source_tasks_test.exs @@ -1,6 +1,7 @@ defmodule Pinchflat.Tasks.SourceTasksTest do use Pinchflat.DataCase + import Mox import Pinchflat.TasksFixtures import Pinchflat.MediaFixtures import Pinchflat.MediaSourceFixtures @@ -8,9 +9,12 @@ defmodule Pinchflat.Tasks.SourceTasksTest do alias Pinchflat.Tasks alias Pinchflat.Tasks.Task alias Pinchflat.Tasks.SourceTasks + alias Pinchflat.Media.MediaItem alias Pinchflat.Workers.MediaIndexingWorker alias Pinchflat.Workers.VideoDownloadWorker + setup :verify_on_exit! + describe "kickoff_indexing_task/1" do test "it does not schedule a job if the interval is <= 0" do source = source_fixture(index_frequency_minutes: -1) @@ -46,6 +50,61 @@ defmodule Pinchflat.Tasks.SourceTasksTest do end end + describe "index_media_items/1" do + setup do + stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end) + + {:ok, [source: source_fixture()]} + end + + test "it creates a media_item record for each media ID returned", %{source: source} do + assert media_items = SourceTasks.index_media_items(source) + + assert Enum.count(media_items) == 3 + assert ["video1", "video2", "video3"] == Enum.map(media_items, & &1.media_id) + assert ["Video 1", "Video 2", "Video 3"] == Enum.map(media_items, & &1.title) + assert Enum.all?(media_items, fn mi -> mi.original_url end) + assert Enum.all?(media_items, fn %MediaItem{} -> true end) + end + + test "it attaches all media_items to the given source", %{source: source} do + source_id = source.id + assert media_items = SourceTasks.index_media_items(source) + + assert Enum.count(media_items) == 3 + assert Enum.all?(media_items, fn %MediaItem{source_id: ^source_id} -> true end) + end + + test "it won't duplicate media_items based on media_id and source", %{source: source} do + _first_run = SourceTasks.index_media_items(source) + _duplicate_run = SourceTasks.index_media_items(source) + + media_items = Repo.preload(source, :media_items).media_items + assert Enum.count(media_items) == 3 + end + + test "it can duplicate media_ids for different sources", %{source: source} do + other_source = source_fixture() + + media_items = SourceTasks.index_media_items(source) + media_items_other_source = SourceTasks.index_media_items(other_source) + + assert Enum.count(media_items) == 3 + assert Enum.count(media_items_other_source) == 3 + + assert Enum.map(media_items, & &1.media_id) == + Enum.map(media_items_other_source, & &1.media_id) + end + + test "it returns a list of media_items or changesets", %{source: source} do + first_run = SourceTasks.index_media_items(source) + duplicate_run = SourceTasks.index_media_items(source) + + assert Enum.all?(first_run, fn %MediaItem{} -> true end) + assert Enum.all?(duplicate_run, fn %Ecto.Changeset{} -> true end) + end + end + describe "enqueue_pending_media_downloads/1" do test "it enqueues a job for each pending media item" do source = source_fixture() diff --git a/test/pinchflat/utils/function_utils_test.exs b/test/pinchflat/utils/function_utils_test.exs new file mode 100644 index 0000000..6478599 --- /dev/null +++ b/test/pinchflat/utils/function_utils_test.exs @@ -0,0 +1,11 @@ +defmodule Pinchflat.Utils.FunctionUtilsTest do + use ExUnit.Case, async: true + + alias Pinchflat.Utils.FunctionUtils + + describe "wrap_ok/1" do + test "wraps the provided term in an :ok tuple" do + assert FunctionUtils.wrap_ok("hello") == {:ok, "hello"} + end + end +end diff --git a/test/pinchflat/workers/media_indexing_worker_test.exs b/test/pinchflat/workers/media_indexing_worker_test.exs index 7a9a87b..4659cb6 100644 --- a/test/pinchflat/workers/media_indexing_worker_test.exs +++ b/test/pinchflat/workers/media_indexing_worker_test.exs @@ -38,32 +38,33 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do end test "it kicks off a download job for each pending media item" do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end) source = source_fixture(index_frequency_minutes: 10) perform_job(MediaIndexingWorker, %{id: source.id}) - assert [_] = all_enqueued(worker: VideoDownloadWorker) + assert length(all_enqueued(worker: VideoDownloadWorker)) == 3 end test "it starts a job for any pending media item even if it's from another run" do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end) source = source_fixture(index_frequency_minutes: 10) media_item_fixture(%{source_id: source.id, media_filepath: nil}) perform_job(MediaIndexingWorker, %{id: source.id}) - assert [_, _] = all_enqueued(worker: VideoDownloadWorker) + assert length(all_enqueued(worker: VideoDownloadWorker)) == 4 end test "it does not kick off a job for media items that could not be saved" do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo1"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end) source = source_fixture(index_frequency_minutes: 10) + media_item_fixture(%{source_id: source.id, media_filepath: nil, media_id: "video1"}) perform_job(MediaIndexingWorker, %{id: source.id}) - # Only one job should be enqueued, since the second video is a duplicate - assert [_] = all_enqueued(worker: VideoDownloadWorker) + # Only 3 jobs should be enqueued, since the first video is a duplicate + assert length(all_enqueued(worker: VideoDownloadWorker)) end test "it reschedules the job based on the index frequency" do @@ -91,7 +92,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do end test "it creates the basic media_item records" do - expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo2"} end) + expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, source_attributes_return_fixture()} end) source = source_fixture(index_frequency_minutes: 10) @@ -102,7 +103,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do |> Enum.map(fn media_item -> media_item.media_id end) end - assert_changed([from: [], to: ["video1", "video2"]], media_item_fetcher, fn -> + assert_changed([from: [], to: ["video1", "video2", "video3"]], media_item_fetcher, fn -> perform_job(MediaIndexingWorker, %{id: source.id}) end) end diff --git a/test/support/fixtures/media_fixtures.ex b/test/support/fixtures/media_fixtures.ex index 5f75cc4..66bb1da 100644 --- a/test/support/fixtures/media_fixtures.ex +++ b/test/support/fixtures/media_fixtures.ex @@ -15,6 +15,8 @@ defmodule Pinchflat.MediaFixtures do |> Enum.into(%{ media_id: Faker.String.base64(12), title: Faker.Commerce.product_name(), + original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}", + livestream: false, media_filepath: "/video/#{Faker.File.file_name(:video)}", source_id: MediaSourceFixtures.source_fixture().id }) diff --git a/test/support/fixtures/media_source_fixtures.ex b/test/support/fixtures/media_source_fixtures.ex index 89500ba..958854e 100644 --- a/test/support/fixtures/media_source_fixtures.ex +++ b/test/support/fixtures/media_source_fixtures.ex @@ -29,4 +29,30 @@ defmodule Pinchflat.MediaSourceFixtures do source end + + def source_attributes_return_fixture do + source_attributes = [ + %{ + id: "video1", + title: "Video 1", + original_url: "https://example.com/video1", + was_live: false + }, + %{ + id: "video2", + title: "Video 2", + original_url: "https://example.com/video2", + was_live: true + }, + %{ + id: "video3", + title: "Video 3", + original_url: "https://example.com/video3", + was_live: false + } + ] + + source_attributes + |> Enum.map_join("\n", &Phoenix.json_library().encode!(&1)) + end end