Misc refactors 2024-03-14 (#89)

* Adds method to improve cleanup of empty directories

* resolved bug where source metadata worker could call itself in an infinite loop

* Refactored file deletion for media items

* Removed useless filesystem data worker

* Updated task listing fns to take a record directly

* Refactored the way I call workers

* Improved some tests

test/pinchflat/boot/data_backfill_worker_test.exs

@@ -4,7 +4,7 @@ defmodule Pinchflat.Boot.DataBackfillWorkerTest do
   import Pinchflat.MediaFixtures
 
   alias Pinchflat.Boot.DataBackfillWorker
-  alias Pinchflat.Filesystem.FilesystemDataWorker
+  alias Pinchflat.JobFixtures.TestJobWorker
 
   describe "cancel_pending_backfill_jobs/0" do
     test "cancels all pending backfill jobs" do
@@ -21,14 +21,14 @@ defmodule Pinchflat.Boot.DataBackfillWorkerTest do
 
     test "does not cancel jobs for other workers" do
       %{id: 0}
-      |> FilesystemDataWorker.new()
+      |> TestJobWorker.new()
       |> Repo.insert_unique_job()
 
-      assert_enqueued(worker: FilesystemDataWorker)
+      assert_enqueued(worker: TestJobWorker)
 
       DataBackfillWorker.cancel_pending_backfill_jobs()
 
-      assert_enqueued(worker: FilesystemDataWorker)
+      assert_enqueued(worker: TestJobWorker)
     end
   end
 

test/pinchflat/downloading/download_option_builder_test.exs

@@ -197,12 +197,19 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
   end
 
   describe "build/1 when testing quality options" do
-    test "it includes quality options", %{media_item: media_item} do
-      media_item = update_media_profile_attribute(media_item, %{preferred_resolution: :"1080p"})
+    test "it includes quality options" do
+      resolutions = ["360", "480", "720", "1080", "2160"]
 
-      assert {:ok, res} = DownloadOptionBuilder.build(media_item)
+      Enum.each(resolutions, fn resolution ->
+        resolution_atom = String.to_existing_atom(resolution <> "p")
 
-      assert {:format_sort, "res:1080,+codec:avc:m4a"} in res
+        media_profile = media_profile_fixture(%{preferred_resolution: resolution_atom})
+        source = source_fixture(%{media_profile_id: media_profile.id})
+        media_item = Repo.preload(media_item_fixture(source_id: source.id), source: :media_profile)
+
+        assert {:ok, res} = DownloadOptionBuilder.build(media_item)
+        assert {:format_sort, "res:#{resolution},+codec:avc:m4a"} in res
+      end)
     end
 
     test "it includes quality options for audio only", %{media_item: media_item} do
@@ -218,10 +225,10 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
   defp update_media_profile_attribute(media_item_with_preloads, attrs) do
     media_item_with_preloads.source.media_profile
     |> Profiles.change_media_profile(attrs)
-    |> Repo.update!()
+    |> Repo.update()
 
     media_item_with_preloads
     |> Repo.reload()
-    |> Repo.preload(source: :media_profile)
+    |> Repo.preload([source: :media_profile], force: true)
   end
 end

test/pinchflat/downloading/downloading_helpers_test.exs

@@ -34,11 +34,11 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest 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 [] = Tasks.list_tasks_for(media_item)
 
       assert :ok = DownloadingHelpers.enqueue_pending_download_tasks(source)
 
-      assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [_] = Tasks.list_tasks_for(media_item)
     end
 
     test "it does not create a job if the source is set to not download" do
@@ -54,7 +54,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
       media_item = media_item_fixture(source_id: source.id, media_filepath: nil)
 
       assert :ok = DownloadingHelpers.enqueue_pending_download_tasks(source)
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
     end
   end
 
@@ -69,7 +69,7 @@ defmodule Pinchflat.Downloading.DownloadingHelpersTest do
       assert :ok = DownloadingHelpers.dequeue_pending_download_tasks(source)
 
       refute_enqueued(worker: MediaDownloadWorker)
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
     end
   end
 end

test/pinchflat/downloading/media_download_worker_test.exs

@@ -5,25 +5,37 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
   import Pinchflat.MediaFixtures
 
   alias Pinchflat.Sources
+  alias Pinchflat.Filesystem.FilesystemHelpers
   alias Pinchflat.Downloading.MediaDownloadWorker
-  alias Pinchflat.Filesystem.FilesystemDataWorker
 
   setup :verify_on_exit!
 
   setup do
-    media_item =
-      Repo.preload(
-        media_item_fixture(%{media_filepath: nil}),
-        [:metadata, source: :media_profile]
-      )
-
     stub(HTTPClientMock, :get, fn _url, _headers, _opts ->
       {:ok, ""}
     end)
 
+    media_item =
+      %{media_filepath: nil}
+      |> media_item_fixture()
+      |> Repo.preload([:metadata, source: :media_profile])
+
     {:ok, %{media_item: media_item}}
   end
 
+  describe "kickoff_with_task/2" do
+    test "starts the worker", %{media_item: media_item} do
+      assert [] = all_enqueued(worker: MediaDownloadWorker)
+      assert {:ok, _} = MediaDownloadWorker.kickoff_with_task(media_item)
+      assert [_] = all_enqueued(worker: MediaDownloadWorker)
+    end
+
+    test "attaches a task", %{media_item: media_item} do
+      assert {:ok, task} = MediaDownloadWorker.kickoff_with_task(media_item)
+      assert task.media_item_id == media_item.id
+    end
+  end
+
   describe "perform/1" do
     test "it saves attributes to the media_item", %{media_item: media_item} do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
@@ -70,16 +82,18 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
       perform_job(MediaDownloadWorker, %{id: media_item.id})
     end
 
-    test "it schedules a filesystem data worker", %{media_item: media_item} do
+    test "it saves the file's size to the database", %{media_item: media_item} do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
-        {:ok, render_metadata(:media_metadata)}
+        metadata = render_parsed_metadata(:media_metadata)
+        FilesystemHelpers.write_p!(metadata["filepath"], "test")
+
+        {:ok, Phoenix.json_library().encode!(metadata)}
       end)
 
-      assert [] = all_enqueued(worker: FilesystemDataWorker)
-
       perform_job(MediaDownloadWorker, %{id: media_item.id})
+      media_item = Repo.reload(media_item)
 
-      assert [_] = all_enqueued(worker: FilesystemDataWorker)
+      assert media_item.media_size_bytes > 0
     end
   end
 end

test/pinchflat/fast_indexing/fast_indexing_helpers_test.exs

@@ -2,51 +2,20 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do
   use Pinchflat.DataCase
 
   import Mox
-  import Pinchflat.TasksFixtures
   import Pinchflat.MediaFixtures
   import Pinchflat.SourcesFixtures
   import Pinchflat.ProfilesFixtures
 
   alias Pinchflat.Tasks
-  alias Pinchflat.Tasks.Task
   alias Pinchflat.Media.MediaItem
   alias Pinchflat.Downloading.MediaDownloadWorker
   alias Pinchflat.FastIndexing.MediaIndexingWorker
   alias Pinchflat.FastIndexing.FastIndexingHelpers
-  alias Pinchflat.FastIndexing.FastIndexingWorker
 
   setup :verify_on_exit!
 
   @media_url "https://www.youtube.com/watch?v=test_1"
 
-  describe "kickoff_fast_indexing_task/1" do
-    test "it schedules a job" do
-      source = source_fixture()
-
-      assert {:ok, _} = FastIndexingHelpers.kickoff_fast_indexing_task(source)
-
-      assert_enqueued(worker: FastIndexingWorker, args: %{"id" => source.id})
-    end
-
-    test "it creates and attaches a task" do
-      source = source_fixture()
-
-      assert {:ok, %Task{} = task} = FastIndexingHelpers.kickoff_fast_indexing_task(source)
-
-      assert task.source_id == source.id
-    end
-
-    test "it deletes any fast indexing tasks for the source" do
-      source = source_fixture()
-      {:ok, job} = Oban.insert(FastIndexingWorker.new(%{"id" => source.id}))
-      task = task_fixture(source_id: source.id, job_id: job.id)
-
-      assert {:ok, _} = FastIndexingHelpers.kickoff_fast_indexing_task(source)
-
-      assert_raise Ecto.NoResultsError, fn -> Repo.reload!(task) end
-    end
-  end
-
   describe "kickoff_indexing_tasks_from_youtube_rss_feed/1" do
     setup do
       {:ok, [source: source_fixture()]}
@@ -104,7 +73,7 @@ defmodule Pinchflat.FastIndexing.FastIndexingHelpersTest do
     test "creates a download task record", %{source: source} do
       assert {:ok, media_item} = FastIndexingHelpers.index_and_enqueue_download_for_media_item(source, @media_url)
 
-      assert [_] = Tasks.list_tasks_for(:media_item_id, media_item.id, "MediaDownloadWorker")
+      assert [_] = Tasks.list_tasks_for(media_item, "MediaDownloadWorker")
     end
 
     test "does not enqueue a download job if the source does not allow it" do

test/pinchflat/fast_indexing/fast_indexing_worker_test.exs

@@ -9,6 +9,23 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do
 
   setup :verify_on_exit!
 
+  describe "kickoff_with_task/2" do
+    test "starts the worker" do
+      source = source_fixture(fast_index: true)
+
+      assert [] = all_enqueued(worker: FastIndexingWorker)
+      assert {:ok, _} = FastIndexingWorker.kickoff_with_task(source)
+      assert [_] = all_enqueued(worker: FastIndexingWorker)
+    end
+
+    test "attaches a task" do
+      source = source_fixture(fast_index: true)
+
+      assert {:ok, task} = FastIndexingWorker.kickoff_with_task(source)
+      assert task.source_id == source.id
+    end
+  end
+
   describe "perform/1" do
     test "calls out to Youtube RSS if enabled" do
       expect(HTTPClientMock, :get, fn _url -> {:ok, ""} end)
@@ -29,6 +46,16 @@ defmodule Pinchflat.FastIndexing.FastIndexingWorkerTest do
       )
     end
 
+    test "does not reschedule if that would create a duplicate job" do
+      stub(HTTPClientMock, :get, fn _url -> {:ok, ""} end)
+      source = source_fixture(fast_index: true)
+
+      perform_job(FastIndexingWorker, %{"id" => source.id})
+      perform_job(FastIndexingWorker, %{"id" => source.id})
+
+      assert [_] = all_enqueued(worker: FastIndexingWorker)
+    end
+
     test "does not call out to Youtube RSS if disabled" do
       expect(HTTPClientMock, :get, 0, fn _url -> {:ok, ""} end)
       source = source_fixture(fast_index: false)

test/pinchflat/fast_indexing/media_indexing_worker_test.exs

@@ -6,8 +6,8 @@ defmodule Pinchflat.FastIndexing.MediaIndexingWorkerTest do
   import Pinchflat.SourcesFixtures
 
   alias Pinchflat.Media.MediaItem
-  alias Pinchflat.FastIndexing.MediaIndexingWorker
   alias Pinchflat.Downloading.MediaDownloadWorker
+  alias Pinchflat.FastIndexing.MediaIndexingWorker
 
   @media_url "https://www.youtube.com/watch?v=1234567890"
 
@@ -19,6 +19,19 @@ defmodule Pinchflat.FastIndexing.MediaIndexingWorkerTest do
     {:ok, source: source}
   end
 
+  describe "kickoff_with_task/2" do
+    test "starts the worker", %{source: source} do
+      assert [] = all_enqueued(worker: MediaIndexingWorker)
+      assert {:ok, _} = MediaIndexingWorker.kickoff_with_task(source, @media_url)
+      assert [_] = all_enqueued(worker: MediaIndexingWorker)
+    end
+
+    test "attaches a task", %{source: source} do
+      assert {:ok, task} = MediaIndexingWorker.kickoff_with_task(source, @media_url)
+      assert task.source_id == source.id
+    end
+  end
+
   describe "perform/1" do
     test "indexes the media item and saves it to the database", %{source: source} do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->

test/pinchflat/filesystem/filesystem_data_worker_test.exs

@@ -1,19 +0,0 @@
-defmodule Pinchflat.Filesystem.FilesystemDataWorkerTest do
-  use Pinchflat.DataCase
-
-  import Pinchflat.MediaFixtures
-
-  alias Pinchflat.Filesystem.FilesystemDataWorker
-
-  describe "perform/1" do
-    test "Computes and stores the media file size" do
-      media_item = media_item_with_attachments()
-
-      refute media_item.media_size_bytes
-
-      perform_job(FilesystemDataWorker, %{id: media_item.id})
-
-      assert Repo.reload!(media_item).media_size_bytes
-    end
-  end
-end

test/pinchflat/filesystem/filesystem_helpers_test.exs

@@ -56,4 +56,55 @@ defmodule Pinchflat.Filesystem.FilesystemHelpersTest do
       File.rm!(filepath)
     end
   end
+
+  describe "delete_file_and_remove_empty_directories/1" do
+    test "deletes file at the provided filepath" do
+      filepath = FilesystemHelpers.generate_metadata_tmpfile(:json)
+
+      assert File.exists?(filepath)
+
+      assert :ok = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath)
+
+      refute File.exists?(filepath)
+    end
+
+    test "deletes empty directories" do
+      tmpfile_directory = Application.get_env(:pinchflat, :tmpfile_directory)
+      filepath = Path.join([tmpfile_directory, "foo", "bar", "baz", "qux.json"])
+      FilesystemHelpers.write_p!(filepath, "")
+
+      assert :ok = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath)
+
+      refute File.exists?(filepath)
+      refute File.exists?(Path.join([tmpfile_directory, "foo", "bar", "baz"]))
+      refute File.exists?(Path.join([tmpfile_directory, "foo", "bar"]))
+      refute File.exists?(Path.join([tmpfile_directory, "foo"]))
+    end
+
+    test "does not delete directories with other files in them" do
+      tmpfile_directory = Application.get_env(:pinchflat, :tmpfile_directory)
+      filepath_1 = Path.join([tmpfile_directory, "foo", "bar", "baz", "qux.json"])
+      filepath_2 = Path.join([tmpfile_directory, "foo", "baz.json"])
+      FilesystemHelpers.write_p!(filepath_1, "")
+      FilesystemHelpers.write_p!(filepath_2, "")
+
+      assert :ok = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath_1)
+
+      refute File.exists?(filepath_1)
+      refute File.exists?(Path.join([tmpfile_directory, "foo", "bar", "baz"]))
+      refute File.exists?(Path.join([tmpfile_directory, "foo", "bar"]))
+
+      assert File.exists?(filepath_2)
+      assert File.exists?(Path.join([tmpfile_directory, "foo"]))
+
+      # cleanup
+      FilesystemHelpers.delete_file_and_remove_empty_directories(filepath_2)
+    end
+
+    test "returns an error if file could not be deleted" do
+      filepath = "/nonexistent/file.json"
+
+      assert {:error, _} = FilesystemHelpers.delete_file_and_remove_empty_directories(filepath)
+    end
+  end
 end

test/pinchflat/media_test.exs

@@ -360,57 +360,6 @@ defmodule Pinchflat.MediaTest do
     end
   end
 
-  describe "media_filepaths/1" do
-    test "returns filepaths in a flat list" do
-      filepaths = %{
-        media_filepath: "/video/test.mp4",
-        thumbnail_filepath: "/video/test.jpg",
-        subtitle_filepaths: [["en", "video/test.srt"]]
-      }
-
-      media_item = media_item_fixture(filepaths)
-
-      assert Media.media_filepaths(media_item) == [
-               "/video/test.mp4",
-               "/video/test.jpg",
-               "video/test.srt"
-             ]
-    end
-
-    test "strips out nil values" do
-      filepaths = %{
-        media_filepath: "/video/test.mp4",
-        thumbnail_filepath: nil,
-        subtitle_filepaths: [["en", nil]]
-      }
-
-      media_item = media_item_fixture(filepaths)
-
-      assert Media.media_filepaths(media_item) == ["/video/test.mp4"]
-    end
-  end
-
-  describe "metadata_filepaths" do
-    test "returns filepaths in a flat list" do
-      filepaths = %{
-        metadata_filepath: "/metadata.json.gz",
-        thumbnail_filepath: "/thumbnail.jpg"
-      }
-
-      media_item = media_item_fixture(%{metadata: filepaths})
-
-      assert Media.metadata_filepaths(media_item) == [
-               "/metadata.json.gz",
-               "/thumbnail.jpg"
-             ]
-    end
-
-    test "returns an empty list when there is no metadata" do
-      media_item = media_item_fixture()
-      assert Media.metadata_filepaths(media_item) == []
-    end
-  end
-
   describe "create_media_item/1" do
     test "creating with valid data creates a media_item" do
       valid_attrs = %{
@@ -515,6 +464,26 @@ defmodule Pinchflat.MediaTest do
       assert {:ok, _} = Media.delete_media_item(media_item)
       assert File.exists?(media_item.media_filepath)
     end
+
+    test "does delete the media item's metadata files" do
+      stub(HTTPClientMock, :get, fn _url, _headers, _opts -> {:ok, ""} end)
+      media_item = Repo.preload(media_item_with_attachments(), :metadata)
+
+      update_attrs = %{
+        metadata: %{
+          metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, %{}),
+          thumbnail_filepath:
+            MetadataFileHelpers.download_and_store_thumbnail_for(media_item, %{
+              "thumbnail" => "https://example.com/thumbnail.jpg"
+            })
+        }
+      }
+
+      {:ok, updated_media_item} = Media.update_media_item(media_item, update_attrs)
+
+      assert {:ok, _} = Media.delete_media_item(updated_media_item)
+      refute File.exists?(updated_media_item.metadata.metadata_filepath)
+    end
   end
 
   describe "delete_media_item/2 when testing file deletion" do

test/pinchflat/metadata/source_metadata_storage_worker_test.exs

@@ -51,5 +51,28 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do
 
       assert metadata == %{"title" => "test"}
     end
+
+    test "won't call itself in an infinite loop" do
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end)
+      source = source_fixture()
+
+      perform_job(SourceMetadataStorageWorker, %{id: source.id})
+      perform_job(SourceMetadataStorageWorker, %{id: source.id})
+
+      assert [_] = all_enqueued(worker: SourceMetadataStorageWorker)
+    end
+
+    test "doesn't prevent over source jobs from running" do
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "{}"} end)
+      source_1 = source_fixture()
+      source_2 = source_fixture()
+
+      perform_job(SourceMetadataStorageWorker, %{id: source_1.id})
+      perform_job(SourceMetadataStorageWorker, %{id: source_1.id})
+      perform_job(SourceMetadataStorageWorker, %{id: source_2.id})
+      perform_job(SourceMetadataStorageWorker, %{id: source_2.id})
+
+      assert [_, _] = all_enqueued(worker: SourceMetadataStorageWorker)
+    end
   end
 end

test/pinchflat/slow_indexing/media_collection_indexing_worker_test.exs

@@ -103,7 +103,7 @@ defmodule Pinchflat.SlowIndexing.MediaCollectionIndexingWorkerTest do
       source = source_fixture(index_frequency_minutes: 10)
 
       task_count_fetcher = fn ->
-        Enum.count(Tasks.list_tasks_for(:source_id, source.id, "MediaCollectionIndexingWorker"))
+        Enum.count(Tasks.list_tasks_for(source, "MediaCollectionIndexingWorker"))
       end
 
       assert_changed([from: 0, to: 1], task_count_fetcher, fn ->

test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs

@@ -149,7 +149,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do
 
       SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source)
 
-      assert [] = Tasks.list_tasks_for(:media_item_id, media_item.id)
+      assert [] = Tasks.list_tasks_for(media_item)
     end
 
     test "it doesn't blow up if a media item cannot be coerced into a struct", %{source: source} do
@@ -290,5 +290,22 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do
       assert Repo.aggregate(MediaItem, :count, :id) == 3
       assert [_, _, _] = all_enqueued(worker: MediaDownloadWorker)
     end
+
+    test "does not blow up if the file returns invalid json", %{source: source} do
+      watcher_poll_interval = Application.get_env(:pinchflat, :file_watcher_poll_interval)
+
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot, addl_opts ->
+        filepath = Keyword.get(addl_opts, :output_filepath)
+        File.write(filepath, "INVALID")
+
+        # Need to add a delay to ensure the file watcher has time to read the file
+        :timer.sleep(watcher_poll_interval * 2)
+        # We know we're testing the file watcher since the syncronous call will only
+        # return an empty string (creating no records)
+        {:ok, ""}
+      end)
+
+      assert [] = SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source)
+    end
   end
 end

test/pinchflat/sources_test.exs

@@ -508,70 +508,6 @@ defmodule Pinchflat.SourcesTest do
     end
   end
 
-  describe "maybe_change_source_from_url/2" do
-    test "it returns a changeset" do
-      stub(YtDlpRunnerMock, :run, &channel_mock/3)
-      source = source_fixture()
-
-      assert %Ecto.Changeset{} = Sources.maybe_change_source_from_url(source, %{})
-    end
-
-    test "it does not fetch source details if the original_url isn't in the changeset" do
-      expect(YtDlpRunnerMock, :run, 0, &channel_mock/3)
-
-      changeset = Sources.maybe_change_source_from_url(%Source{}, %{name: "some updated name"})
-
-      assert %Ecto.Changeset{} = changeset
-    end
-
-    test "it fetches source details if the original_url is in the changeset" do
-      expect(YtDlpRunnerMock, :run, &channel_mock/3)
-
-      changeset =
-        Sources.maybe_change_source_from_url(%Source{}, %{
-          original_url: "https://www.youtube.com/channel/abc123"
-        })
-
-      assert %Ecto.Changeset{} = changeset
-    end
-
-    test "it adds source details to the changeset, keeping the orignal details" do
-      expect(YtDlpRunnerMock, :run, &channel_mock/3)
-
-      media_profile = media_profile_fixture()
-      media_profile_id = media_profile.id
-
-      changeset =
-        Sources.maybe_change_source_from_url(%Source{}, %{
-          original_url: "https://www.youtube.com/channel/abc123",
-          media_profile_id: media_profile.id
-        })
-
-      assert %Ecto.Changeset{} = changeset
-      assert String.starts_with?(changeset.changes.collection_id, "some_channel_id_")
-
-      assert %{
-               collection_name: "some channel name",
-               media_profile_id: ^media_profile_id,
-               original_url: "https://www.youtube.com/channel/abc123"
-             } = changeset.changes
-    end
-
-    test "it adds an error to the changeset if the runner fails" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot ->
-        {:error, "some error", 1}
-      end)
-
-      changeset =
-        Sources.maybe_change_source_from_url(%Source{}, %{
-          original_url: "https://www.youtube.com/channel/abc123"
-        })
-
-      assert %Ecto.Changeset{} = changeset
-      assert errors_on(changeset).original_url == ["could not fetch source details from URL"]
-    end
-  end
-
   defp playlist_mock(_url, _opts, _ot) do
     {
       :ok,

test/pinchflat/tasks_test.exs

@@ -36,53 +36,59 @@ defmodule Pinchflat.TasksTest do
     end
   end
 
-  describe "list_tasks_for/4" do
+  describe "list_tasks_for/3" do
     test "it lets you specify which record type/ID to join on" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id) == [task]
+      assert Tasks.list_tasks_for(source, nil, [:available]) == [task]
     end
 
     test "it lets you specify which job states to include" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id, nil, [:available]) == [task]
-      assert Tasks.list_tasks_for(:source_id, task.source_id, nil, [:cancelled]) == []
+      assert Tasks.list_tasks_for(source, nil, [:available]) == [task]
+      assert Tasks.list_tasks_for(source, nil, [:cancelled]) == []
     end
 
     test "it lets you specify which worker to include" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id, "TestJobWorker") == [task]
-      assert Tasks.list_tasks_for(:source_id, task.source_id, "FooBarWorker") == []
+      assert Tasks.list_tasks_for(source, "TestJobWorker") == [task]
+      assert Tasks.list_tasks_for(source, "FooBarWorker") == []
     end
 
     test "it includes all workers if no worker is specified" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_tasks_for(:source_id, task.source_id, nil) == [task]
+      assert Tasks.list_tasks_for(source, nil) == [task]
     end
   end
 
   describe "list_pending_tasks_for/3" do
     test "it lists pending tasks" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id) == [task]
+      assert Tasks.list_pending_tasks_for(source) == [task]
     end
 
     test "it does not list non-pending tasks" do
-      task = Repo.preload(task_fixture(), :job)
+      task = Repo.preload(task_fixture(), [:job, :source])
       :ok = Oban.cancel_job(task.job)
 
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id) == []
+      assert Tasks.list_pending_tasks_for(task.source) == []
     end
 
     test "it lets you specify which worker to include" do
-      task = task_fixture()
+      source = source_fixture()
+      task = task_fixture(source_id: source.id)
 
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id, "TestJobWorker") == [task]
-      assert Tasks.list_pending_tasks_for(:source_id, task.source_id, "FooBarWorker") == []
+      assert Tasks.list_pending_tasks_for(source, "TestJobWorker") == [task]
+      assert Tasks.list_pending_tasks_for(source, "FooBarWorker") == []
     end
   end
 

test/pinchflat/yt_dlp/media_collection_test.exs

@@ -62,6 +62,15 @@ defmodule Pinchflat.YtDlp.MediaCollectionTest do
       assert_receive {:handler, filename}
       assert String.ends_with?(filename, ".json")
     end
+
+    test "gracefully handles partially failed responses" do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl_opts ->
+        {:ok, "INVALID\n\n" <> source_attributes_return_fixture() <> "\nINVALID\n"}
+      end)
+
+      assert {:ok, [%Media{media_id: "video1"}, %Media{media_id: "video2"}, %Media{media_id: "video3"}]} =
+               MediaCollection.get_media_attributes_for_collection(@channel_url)
+    end
   end
 
   describe "get_source_details/1" do