diff --git a/.iex.exs b/.iex.exs index dfe1942..e90bc79 100644 --- a/.iex.exs +++ b/.iex.exs @@ -2,6 +2,7 @@ alias Pinchflat.Repo alias Pinchflat.Tasks.Task alias Pinchflat.Media.MediaItem +alias Pinchflat.Tasks.SourceTasks alias Pinchflat.Media.MediaMetadata alias Pinchflat.MediaSource.Source alias Pinchflat.Profiles.MediaProfile diff --git a/lib/pinchflat/media_source.ex b/lib/pinchflat/media_source.ex index 12109d6..b6b4d38 100644 --- a/lib/pinchflat/media_source.ex +++ b/lib/pinchflat/media_source.ex @@ -100,6 +100,8 @@ defmodule Pinchflat.MediaSource do Note that this fetches source details as long as the `original_url` is present. 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. + + IDEA: Maybe I could discern `collection_type` based on the original URL? """ def change_source_from_url(%Source{} = source, attrs) do case change_source(source, attrs) do diff --git a/lib/pinchflat/media_source/source.ex b/lib/pinchflat/media_source/source.ex index 7682d6c..65f48c7 100644 --- a/lib/pinchflat/media_source/source.ex +++ b/lib/pinchflat/media_source/source.ex @@ -9,7 +9,16 @@ defmodule Pinchflat.MediaSource.Source do alias Pinchflat.Media.MediaItem alias Pinchflat.Profiles.MediaProfile - @allowed_fields ~w(collection_name collection_id collection_type index_frequency_minutes original_url media_profile_id)a + @allowed_fields ~w( + collection_name + collection_id + collection_type + friendly_name + index_frequency_minutes + original_url + media_profile_id + )a + @required_fields @allowed_fields -- ~w(index_frequency_minutes friendly_name)a schema "sources" do @@ -17,7 +26,7 @@ defmodule Pinchflat.MediaSource.Source do field :collection_name, :string field :collection_id, :string field :collection_type, Ecto.Enum, values: [:channel, :playlist] - field :index_frequency_minutes, :integer + field :index_frequency_minutes, :integer, default: 60 * 24 # This should only be used for user reference going forward # as the collection_id should be used for all API calls field :original_url, :string diff --git a/lib/pinchflat/tasks/source_tasks.ex b/lib/pinchflat/tasks/source_tasks.ex index 0d5aa08..0ae18f6 100644 --- a/lib/pinchflat/tasks/source_tasks.ex +++ b/lib/pinchflat/tasks/source_tasks.ex @@ -3,9 +3,11 @@ defmodule Pinchflat.Tasks.SourceTasks do This module contains methods for managing tasks (workers) related to sources. """ + alias Pinchflat.Media alias Pinchflat.Tasks alias Pinchflat.MediaSource.Source alias Pinchflat.Workers.MediaIndexingWorker + alias Pinchflat.Workers.VideoDownloadWorker @doc """ Starts tasks for indexing a source's media. @@ -30,4 +32,27 @@ defmodule Pinchflat.Tasks.SourceTasks do end end end + + @doc """ + Starts tasks for downloading videos for any of a sources _pending_ media items. + + NOTE: this starts a download for each media item that is pending, + not just the ones that were indexed in this job run. This should ensure + that any stragglers are caught if, for some reason, they weren't enqueued + or somehow got de-queued. + + I'm not sure of a case where this would happen, but it's cheap insurance. + + Returns :ok + """ + def enqueue_pending_media_downloads(%Source{} = source) do + source + |> Media.list_pending_media_items_for() + |> Enum.each(fn media_item -> + media_item + |> Map.take([:id]) + |> VideoDownloadWorker.new() + |> Tasks.create_job_with_task(media_item) + end) + end end diff --git a/lib/pinchflat/workers/media_indexing_worker.ex b/lib/pinchflat/workers/media_indexing_worker.ex index 166a4bd..f46e427 100644 --- a/lib/pinchflat/workers/media_indexing_worker.ex +++ b/lib/pinchflat/workers/media_indexing_worker.ex @@ -7,10 +7,9 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do tags: ["media_source", "media_indexing"] alias __MODULE__ - alias Pinchflat.Media alias Pinchflat.Tasks alias Pinchflat.MediaSource - alias Pinchflat.Workers.VideoDownloadWorker + alias Pinchflat.Tasks.SourceTasks @impl Oban.Worker @doc """ @@ -49,7 +48,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do defp index_media_and_reschedule(source) do MediaSource.index_media_items(source) - enqueue_video_downloads(source) + SourceTasks.enqueue_pending_media_downloads(source) source |> Map.take([:id]) @@ -60,21 +59,4 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do {:error, :duplicate_job} -> {:ok, :job_exists} end end - - # NOTE: this starts a download for each media item that is pending, - # not just the ones that were indexed in this job run. This should ensure - # that any stragglers are caught if, for some reason, they weren't enqueued - # or somehow got de-queued. - # - # I'm not sure of a case where this would happen, but it's cheap insurance. - defp enqueue_video_downloads(source) do - source - |> Media.list_pending_media_items_for() - |> Enum.each(fn media_item -> - media_item - |> Map.take([:id]) - |> VideoDownloadWorker.new() - |> Tasks.create_job_with_task(media_item) - end) - end end diff --git a/lib/pinchflat_web/controllers/media_sources/source_html.ex b/lib/pinchflat_web/controllers/media_sources/source_html.ex index 9531a59..7f790cd 100644 --- a/lib/pinchflat_web/controllers/media_sources/source_html.ex +++ b/lib/pinchflat_web/controllers/media_sources/source_html.ex @@ -24,4 +24,11 @@ defmodule PinchflatWeb.MediaSources.SourceHTML do {"Monthly", 30 * 24 * 60} ] end + + def friendly_collection_types do + [ + {"Channel", "channel"}, + {"Playlist", "playlist"} + ] + end end diff --git a/lib/pinchflat_web/controllers/media_sources/source_html/source_form.html.heex b/lib/pinchflat_web/controllers/media_sources/source_html/source_form.html.heex index f8a0a87..e7fc1f4 100644 --- a/lib/pinchflat_web/controllers/media_sources/source_html/source_form.html.heex +++ b/lib/pinchflat_web/controllers/media_sources/source_html/source_form.html.heex @@ -10,7 +10,13 @@ label="Media Profile" /> - <.input field={f[:collection_type]} type="text" label="Collection Type" /> + <.input + field={f[:collection_type]} + options={friendly_collection_types()} + type="select" + label="Collection Type" + /> + <.input field={f[:original_url]} type="text" label="Source URL" /> <.input diff --git a/test/pinchflat/tasks/source_tasks_test.exs b/test/pinchflat/tasks/source_tasks_test.exs index 1e9c38f..33cdc03 100644 --- a/test/pinchflat/tasks/source_tasks_test.exs +++ b/test/pinchflat/tasks/source_tasks_test.exs @@ -2,11 +2,14 @@ defmodule Pinchflat.Tasks.SourceTasksTest do use Pinchflat.DataCase import Pinchflat.TasksFixtures + import Pinchflat.MediaFixtures import Pinchflat.MediaSourceFixtures + alias Pinchflat.Tasks alias Pinchflat.Tasks.Task alias Pinchflat.Tasks.SourceTasks alias Pinchflat.Workers.MediaIndexingWorker + alias Pinchflat.Workers.VideoDownloadWorker describe "kickoff_indexing_task/1" do test "it does not schedule a job if the interval is <= 0" do @@ -42,4 +45,35 @@ defmodule Pinchflat.Tasks.SourceTasksTest do assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end end end + + describe "enqueue_pending_media_downloads/1" do + test "it enqueues a job for each pending media item" do + source = source_fixture() + media_item = media_item_fixture(source_id: source.id, media_filepath: nil) + + assert :ok = SourceTasks.enqueue_pending_media_downloads(source) + + assert_enqueued(worker: VideoDownloadWorker, args: %{"id" => media_item.id}) + end + + test "it does not enqueue a job for media items with a filepath" do + source = source_fixture() + _media_item = media_item_fixture(source_id: source.id, media_filepath: "some/filepath.mp4") + + assert :ok = SourceTasks.enqueue_pending_media_downloads(source) + + refute_enqueued(worker: VideoDownloadWorker) + end + + test "it attaches a task to each enqueued job" do + source = source_fixture() + media_item = media_item_fixture(source_id: source.id, media_filepath: nil) + + assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id) + + assert :ok = SourceTasks.enqueue_pending_media_downloads(source) + + assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id) + end + end end