From 5e8689836d092adba9f7978d0f19eaaf9213da92 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Fri, 8 Mar 2024 21:25:04 -0800 Subject: [PATCH] Made media attribute-related yt-dlp calls return a struct --- lib/pinchflat/media.ex | 12 +++--- lib/pinchflat/tasks/source_tasks.ex | 5 ++- lib/pinchflat/yt_dlp/backend/media.ex | 37 +++++++++++++++---- .../yt_dlp/backend/media_collection.ex | 1 + test/pinchflat/media_test.exs | 16 +++++--- .../yt_dlp/backend/media_collection_test.exs | 2 +- test/pinchflat/yt_dlp/backend/media_test.exs | 2 +- 7 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/pinchflat/media.ex b/lib/pinchflat/media.ex index 04d1379..c968269 100644 --- a/lib/pinchflat/media.ex +++ b/lib/pinchflat/media.ex @@ -188,14 +188,14 @@ defmodule Pinchflat.Media do Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}} """ - def create_media_item_from_backend_attrs(source, media_attrs) do + def create_media_item_from_backend_attrs(source, media_attrs_struct) do 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"], - description: media_attrs["description"] + title: media_attrs_struct.title, + media_id: media_attrs_struct.media_id, + original_url: media_attrs_struct.original_url, + livestream: media_attrs_struct.livestream, + description: media_attrs_struct.description } create_media_item(attrs) diff --git a/lib/pinchflat/tasks/source_tasks.ex b/lib/pinchflat/tasks/source_tasks.ex index fa8f973..8fab791 100644 --- a/lib/pinchflat/tasks/source_tasks.ex +++ b/lib/pinchflat/tasks/source_tasks.ex @@ -20,6 +20,8 @@ defmodule Pinchflat.Tasks.SourceTasks do alias Pinchflat.Workers.MediaCollectionIndexingWorker alias Pinchflat.Utils.FilesystemUtils.FileFollowerServer + alias Pinchflat.YtDlp.Backend.Media, as: YtDlpMedia + @doc """ Starts tasks for indexing a source's media regardless of the source's indexing frequency. It's assumed the caller will check for that. @@ -162,7 +164,8 @@ defmodule Pinchflat.Tasks.SourceTasks do {:ok, media_attrs} -> Logger.debug("FileFollowerServer Handler: Got media attributes: #{inspect(media_attrs)}") - create_media_item_and_enqueue_download(source, media_attrs) + media_struct = YtDlpMedia.response_to_struct(media_attrs) + create_media_item_and_enqueue_download(source, media_struct) err -> Logger.debug("FileFollowerServer Handler: Error decoding JSON: #{inspect(err)}") diff --git a/lib/pinchflat/yt_dlp/backend/media.ex b/lib/pinchflat/yt_dlp/backend/media.ex index ae1dbb0..89ff3f0 100644 --- a/lib/pinchflat/yt_dlp/backend/media.ex +++ b/lib/pinchflat/yt_dlp/backend/media.ex @@ -3,6 +3,17 @@ defmodule Pinchflat.YtDlp.Backend.Media do Contains utilities for working with singular pieces of media """ + defstruct [ + :media_id, + :title, + :description, + :original_url, + :livestream + ] + + alias __MODULE__ + alias Pinchflat.Utils.FunctionUtils + @doc """ Downloads a single piece of media (and possibly its metadata) directly to its final destination. Returns the parsed JSON output from yt-dlp. @@ -23,11 +34,6 @@ defmodule Pinchflat.YtDlp.Backend.Media do @doc """ Returns a map representing the media at the given URL. - IDEA: should I return a struct here? I want these methods to be agnostic - to implementation, but maybe it can specify its own contract by - returning a struct with well-known fields. Would make refactoring - turbo easy if a field needs to exist but its behavior changes slightly. - Returns {:ok, [map()]} | {:error, any, ...}. """ def get_media_attributes(url) do @@ -36,8 +42,14 @@ defmodule Pinchflat.YtDlp.Backend.Media do output_template = indexing_output_template() case runner.run(url, command_opts, output_template) do - {:ok, output} -> Phoenix.json_library().decode(output) - res -> res + {:ok, output} -> + output + |> Phoenix.json_library().decode!() + |> response_to_struct() + |> FunctionUtils.wrap_ok() + + res -> + res end end @@ -48,6 +60,17 @@ defmodule Pinchflat.YtDlp.Backend.Media do "%(.{id,title,was_live,original_url,description})j" end + # TODO: test + def response_to_struct(response) do + %Media{ + media_id: response["id"], + title: response["title"], + description: response["description"], + original_url: response["original_url"], + livestream: response["was_live"] + } + end + defp backend_runner do # This approach lets us mock the command for testing Application.get_env(:pinchflat, :yt_dlp_runner) diff --git a/lib/pinchflat/yt_dlp/backend/media_collection.ex b/lib/pinchflat/yt_dlp/backend/media_collection.ex index 13c0317..ff8e044 100644 --- a/lib/pinchflat/yt_dlp/backend/media_collection.ex +++ b/lib/pinchflat/yt_dlp/backend/media_collection.ex @@ -36,6 +36,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaCollection do output |> String.split("\n", trim: true) |> Enum.map(&Phoenix.json_library().decode!/1) + |> Enum.map(&YtDlpMedia.response_to_struct/1) |> FunctionUtils.wrap_ok() res -> diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index a7a992f..3c66e84 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -11,6 +11,8 @@ defmodule Pinchflat.MediaTest do alias Pinchflat.Media.MediaItem alias Pinchflat.Metadata.MetadataFileHelpers + alias Pinchflat.YtDlp.Backend.Media, as: YtDlpMedia + setup :verify_on_exit! @invalid_attrs %{title: nil, media_id: nil, media_filepath: nil} @@ -388,14 +390,18 @@ defmodule Pinchflat.MediaTest do describe "create_media_item_from_backend_attrs/2" do test "creates a media item for a given source and attributes" do source = source_fixture() - media_attrs = Phoenix.json_library().decode!(media_attributes_return_fixture()) + + media_attrs = + media_attributes_return_fixture() + |> Phoenix.json_library().decode!() + |> YtDlpMedia.response_to_struct() assert {:ok, %MediaItem{} = media_item} = Media.create_media_item_from_backend_attrs(source, media_attrs) assert media_item.source_id == source.id - assert media_item.title == media_attrs["title"] - assert media_item.media_id == media_attrs["id"] - assert media_item.original_url == media_attrs["original_url"] - assert media_item.description == media_attrs["description"] + assert media_item.title == media_attrs.title + assert media_item.media_id == media_attrs.media_id + assert media_item.original_url == media_attrs.original_url + assert media_item.description == media_attrs.description end end diff --git a/test/pinchflat/yt_dlp/backend/media_collection_test.exs b/test/pinchflat/yt_dlp/backend/media_collection_test.exs index 01c1ac0..1e5805d 100644 --- a/test/pinchflat/yt_dlp/backend/media_collection_test.exs +++ b/test/pinchflat/yt_dlp/backend/media_collection_test.exs @@ -16,7 +16,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaCollectionTest do {:ok, source_attributes_return_fixture() <> "\n\n"} end) - assert {:ok, [%{"id" => "video1"}, %{"id" => "video2"}, %{"id" => "video3"}]} = + assert {:ok, [%Media{media_id: "video1"}, %Media{media_id: "video2"}, %Media{media_id: "video3"}]} = MediaCollection.get_media_attributes_for_collection(@channel_url) end diff --git a/test/pinchflat/yt_dlp/backend/media_test.exs b/test/pinchflat/yt_dlp/backend/media_test.exs index f9bf74c..8072a2e 100644 --- a/test/pinchflat/yt_dlp/backend/media_test.exs +++ b/test/pinchflat/yt_dlp/backend/media_test.exs @@ -55,7 +55,7 @@ defmodule Pinchflat.YtDlp.Backend.MediaTest do {:ok, media_attributes_return_fixture()} end) - assert {:ok, %{"description" => _, "id" => _, "original_url" => _, "title" => _, "was_live" => _}} = + assert {:ok, %{description: _, media_id: _, original_url: _, title: _, livestream: _}} = Media.get_media_attributes(@media_url) end