From f6a78ccb89f3100ac502aee2f037d18d4e18fe25 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Thu, 14 Mar 2024 15:41:31 -0700 Subject: [PATCH] Removed useless filesystem data worker --- lib/pinchflat/boot/pre_job_startup_tasks.ex | 1 - .../downloading/media_download_worker.ex | 23 +++++++------- lib/pinchflat/downloading/media_downloader.ex | 2 ++ .../filesystem/filesystem_data_worker.ex | 31 ------------------- .../boot/data_backfill_worker_test.exs | 8 ++--- .../media_download_worker_test.exs | 25 ++++++++------- .../filesystem_data_worker_test.exs | 19 ------------ 7 files changed, 30 insertions(+), 79 deletions(-) delete mode 100644 lib/pinchflat/filesystem/filesystem_data_worker.ex delete mode 100644 test/pinchflat/filesystem/filesystem_data_worker_test.exs diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index 202a746..c3e5e9b 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -69,7 +69,6 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do rename_map = [ ["Pinchflat.Workers.MediaIndexingWorker", "Pinchflat.FastIndexing.MediaIndexingWorker"], ["Pinchflat.Workers.MediaDownloadWorker", "Pinchflat.Downloading.MediaDownloadWorker"], - ["Pinchflat.Workers.FilesystemDataWorker", "Pinchflat.Filesystem.FilesystemDataWorker"], ["Pinchflat.Workers.FastIndexingWorker", "Pinchflat.FastIndexing.FastIndexingWorker"], ["Pinchflat.Workers.MediaCollectionIndexingWorker", "Pinchflat.SlowIndexing.MediaCollectionIndexingWorker"], ["Pinchflat.Workers.DataBackfillWorker", "Pinchflat.Boot.DataBackfillWorker"] diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex index d41a8b3..9ede5ae 100644 --- a/lib/pinchflat/downloading/media_download_worker.ex +++ b/lib/pinchflat/downloading/media_download_worker.ex @@ -8,9 +8,7 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do alias Pinchflat.Repo alias Pinchflat.Media - alias Pinchflat.Tasks alias Pinchflat.Downloading.MediaDownloader - alias Pinchflat.Filesystem.FilesystemDataWorker @impl Oban.Worker @doc """ @@ -35,22 +33,23 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do defp download_media_and_schedule_jobs(media_item) do case MediaDownloader.download_for_media_item(media_item) do - {:ok, _} -> - schedule_filesystem_data_worker(media_item) - {:ok, media_item} + {:ok, updated_media_item} -> + compute_and_save_media_filesize(updated_media_item) + + {:ok, updated_media_item} err -> err end end - defp schedule_filesystem_data_worker(media_item) do - %{id: media_item.id} - |> FilesystemDataWorker.new() - |> Tasks.create_job_with_task(media_item) - |> case do - {:ok, task} -> {:ok, task} - {:error, :duplicate_job} -> {:ok, :job_exists} + defp compute_and_save_media_filesize(media_item) do + case File.stat(media_item.media_filepath) do + {:ok, %{size: size}} -> + Media.update_media_item(media_item, %{media_size_bytes: size}) + + _ -> + :ok end end end diff --git a/lib/pinchflat/downloading/media_downloader.ex b/lib/pinchflat/downloading/media_downloader.ex index a552f87..cf87514 100644 --- a/lib/pinchflat/downloading/media_downloader.ex +++ b/lib/pinchflat/downloading/media_downloader.ex @@ -38,6 +38,8 @@ defmodule Pinchflat.Downloading.MediaDownloader do media_downloaded_at: DateTime.utc_now(), nfo_filepath: determine_nfo_filepath(item_with_preloads, parsed_json), metadata: %{ + # IDEA: might be worth kicking off a job for this since thumbnail fetching + # could fail and I want to handle that in isolation metadata_filepath: MetadataFileHelpers.compress_and_store_metadata_for(media_item, parsed_json), thumbnail_filepath: MetadataFileHelpers.download_and_store_thumbnail_for(media_item, parsed_json) } diff --git a/lib/pinchflat/filesystem/filesystem_data_worker.ex b/lib/pinchflat/filesystem/filesystem_data_worker.ex deleted file mode 100644 index 2b36d37..0000000 --- a/lib/pinchflat/filesystem/filesystem_data_worker.ex +++ /dev/null @@ -1,31 +0,0 @@ -defmodule Pinchflat.Filesystem.FilesystemDataWorker do - @moduledoc false - - use Oban.Worker, - queue: :local_metadata, - tags: ["media_item", "media_metadata", "local_metadata"], - max_attempts: 1 - - alias Pinchflat.Media - alias Pinchflat.Filesystem.FilesystemHelpers - - @impl Oban.Worker - @doc """ - For a given media item, compute and save metadata about the file on-disk. - - IDEA: does this have to be a standalone job? I originally split it out - so a failure here wouldn't cause a downloader job retry, but I can match - for failures so it doesn't retry. - - Returns :ok - """ - def perform(%Oban.Job{args: %{"id" => media_item_id}}) do - media_item = Media.get_media_item!(media_item_id) - - FilesystemHelpers.compute_and_save_media_filesize(media_item) - - # Don't retry on failure - if it didn't work immediately there's no - # reason to believe it will work later. - :ok - end -end diff --git a/test/pinchflat/boot/data_backfill_worker_test.exs b/test/pinchflat/boot/data_backfill_worker_test.exs index f08872c..773cf99 100644 --- a/test/pinchflat/boot/data_backfill_worker_test.exs +++ b/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 diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs index 0023419..fdebac6 100644 --- a/test/pinchflat/downloading/media_download_worker_test.exs +++ b/test/pinchflat/downloading/media_download_worker_test.exs @@ -5,22 +5,21 @@ 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 @@ -70,16 +69,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 diff --git a/test/pinchflat/filesystem/filesystem_data_worker_test.exs b/test/pinchflat/filesystem/filesystem_data_worker_test.exs deleted file mode 100644 index 7a50508..0000000 --- a/test/pinchflat/filesystem/filesystem_data_worker_test.exs +++ /dev/null @@ -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