Added metadata model and parsing

Adding the metadata model made me realize that, in many cases, yt-dlp
returns undesired input in stdout, breaking parsing. In order to get
the metadata model working, I had to change the way in which the app
interacts with yt-dlp. Now, output is written as a file to disk which
is immediately re-read and returned.

.gitignore

@@ -38,3 +38,4 @@ npm-debug.log
 
 /.elixir_ls
 .env
+.DS_Store

config/test.exs

@@ -3,8 +3,8 @@ import Config
 config :pinchflat,
   # Specifying backend data here makes mocking and local testing SUPER easy
   yt_dlp_executable: Path.join([File.cwd!(), "/test/support/scripts/yt-dlp-mocks/repeater.sh"]),
-  media_directory: Path.join([File.cwd!(), "tmp", "videos"]),
+  media_directory: Path.join([System.tmp_dir!(), "videos"]),
-  metadata_directory: Path.join([File.cwd!(), "tmp", "metadata"])
+  metadata_directory: Path.join([System.tmp_dir!(), "metadata"])
 
 config :pinchflat, Oban, testing: :manual
 

lib/pinchflat/media.ex

@@ -25,7 +25,7 @@ defmodule Pinchflat.Media do
   @doc """
   Creates a media_item. Returns {:ok, %MediaItem{}} | {:error, %Ecto.Changeset{}}.
   """
-  def create_media_item(attrs \\ %{}) do
+  def create_media_item(attrs) do
     %MediaItem{}
     |> MediaItem.changeset(attrs)
     |> Repo.insert()

lib/pinchflat/media/media_item.ex

@@ -7,12 +7,11 @@ defmodule Pinchflat.Media.MediaItem do
   import Ecto.Changeset
 
   alias Pinchflat.MediaSource.Channel
+  alias Pinchflat.Media.MediaMetadata
 
   @required_fields ~w(media_id channel_id)a
   @allowed_fields ~w(title media_id video_filepath channel_id)a
 
-  # IDEA: consider making an attached `metadata` model to store the JSON response from whatever backend is used
-
   schema "media_items" do
     field :title, :string
     field :media_id, :string
@@ -20,6 +19,8 @@ defmodule Pinchflat.Media.MediaItem do
 
     belongs_to :channel, Channel
 
+    has_one :metadata, MediaMetadata, on_replace: :update
+
     timestamps(type: :utc_datetime)
   end
 
@@ -27,6 +28,7 @@ defmodule Pinchflat.Media.MediaItem do
   def changeset(media_item, attrs) do
     media_item
     |> cast(attrs, @allowed_fields)
+    |> cast_assoc(:metadata, with: &MediaMetadata.changeset/2, required: false)
     |> validate_required(@required_fields)
     |> unique_constraint([:media_id, :channel_id])
   end

lib/pinchflat/media/media_metadata.ex

@@ -0,0 +1,28 @@
+defmodule Pinchflat.Media.MediaMetadata do
+  @moduledoc """
+  The MediaMetadata schema.
+
+  Look. Don't @ me about Metadata vs. Metadatum. I'm very sensitive.
+  """
+
+  use Ecto.Schema
+  import Ecto.Changeset
+
+  alias Pinchflat.Media.MediaItem
+
+  schema "media_metadata" do
+    field :client_response, :map
+
+    belongs_to :media_item, MediaItem
+
+    timestamps(type: :utc_datetime)
+  end
+
+  @doc false
+  def changeset(media_metadata, attrs) do
+    media_metadata
+    |> cast(attrs, [:client_response])
+    |> validate_required([:client_response])
+    |> unique_constraint([:media_item_id])
+  end
+end

lib/pinchflat/media_client/backends/backend_command_runner.ex

@@ -3,5 +3,5 @@ defmodule Pinchflat.MediaClient.Backends.BackendCommandRunner do
   A behaviour for running CLI commands against a downloader backend
   """
 
-  @callback run(binary(), keyword()) :: {:ok, binary()} | {:error, binary(), integer()}
+  @callback run(binary(), keyword(), binary()) :: {:ok, binary()} | {:error, binary(), integer()}
 end

lib/pinchflat/media_client/backends/yt_dlp/channel.ex

@@ -16,9 +16,9 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.Channel do
   Returns {:ok, %ChannelDetails{}} | {:error, any, ...}.
   """
   def get_channel_details(channel_url) do
-    opts = [print: "%(.{channel,channel_id})j", playlist_end: 1]
+    opts = [playlist_end: 1]
 
-    with {:ok, output} <- backend_runner().run(channel_url, opts),
+    with {:ok, output} <- backend_runner().run(channel_url, opts, "%(.{channel,channel_id})j"),
          {:ok, parsed_json} <- Phoenix.json_library().decode(output) do
       {:ok, ChannelDetails.new(parsed_json["channel_id"], parsed_json["channel"])}
     else

lib/pinchflat/media_client/backends/yt_dlp/command_runner.ex

@@ -11,7 +11,9 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.CommandRunner do
   @behaviour BackendCommandRunner
 
   @doc """
-  Runs a yt-dlp command and returns the string output
+  Runs a yt-dlp command and returns the string output. Saves the output to
+  a file and then returns its contents because yt-dlp will return warnings
+  to stdout even if the command is successful, but these will break JSON parsing.
 
   Returns {:ok, binary()} | {:error, output, status}.
 
@@ -20,18 +22,38 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.CommandRunner do
   See: https://stackoverflow.com/a/49061086/5665799
   """
   @impl BackendCommandRunner
-  def run(url, command_opts) do
+  def run(url, command_opts, output_template) do
     command = backend_executable()
-    formatted_command_opts = [url] ++ parse_options(command_opts)
+    # These must stay in exactly this order, hence why I'm giving it its own variable.
+    # Also, can't use RAM file since yt-dlp needs a concrete filepath.
+    json_output_path = generate_json_output_path()
+    print_to_file_opts = [{:print_to_file, output_template}, json_output_path]
+    formatted_command_opts = [url] ++ parse_options(command_opts ++ print_to_file_opts)
 
     Logger.debug("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}")
 
     case System.cmd(command, formatted_command_opts, stderr_to_stdout: true) do
-      {output, 0} -> {:ok, output}
+      {_, 0} ->
-      {output, status} -> {:error, output, status}
+        # IDEA: consider deleting the file after reading it
+        # (even on error? especially on error?)
+        File.read(json_output_path)
+
+      {output, status} ->
+        {:error, output, status}
     end
   end
 
+  defp generate_json_output_path do
+    metadata_directory = Application.get_env(:pinchflat, :metadata_directory)
+    filepath = Path.join([metadata_directory, "#{StringUtils.random_string(64)}.json"])
+
+    # Ensure the file can be created and written to BEFORE we run the `yt-dlp` command
+    :ok = File.mkdir_p!(Path.dirname(filepath))
+    :ok = File.write(filepath, "")
+
+    filepath
+  end
+
   # We want to satisfy the following behaviours:
   #
   # 1. If the key is an atom, convert it to a string and convert it to kebab case (for convenience)

lib/pinchflat/media_client/backends/yt_dlp/metadata_parser.ex

@@ -1,7 +1,26 @@
 defmodule Pinchflat.MediaClient.Backends.YtDlp.MetadataParser do
+  @moduledoc """
+  yt-dlp offers a LOT of metadata in its JSON response, some of which
+  needs to be extracted and included in various models.
+
+  For now, also squirrel all of it away in the `media_metadata` table.
+  I might revisit this or pare it down later, but I'd rather need it
+  and not have it, ya know?
+  """
+
+  @doc """
+  Parses the given JSON response from yt-dlp and returns a map of
+  the needful media_item attributes, along with anything needed for
+  its associations.
+
+  Returns map()
+  """
   def parse_for_media_item(metadata) do
     %{
-      video_filepath: metadata["filepath"]
+      video_filepath: metadata["filepath"],
+      metadata: %{
+        client_response: metadata
+      }
     }
   end
 end

lib/pinchflat/media_client/backends/yt_dlp/video.ex

@@ -3,31 +3,17 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.Video do
   Contains utilities for working with singular videos
   """
 
-  alias Pinchflat.Utils.StringUtils
-
   @doc """
-  Downloads a single video (and possible metadata) directly to its
+  Downloads a single video (and possibly its metadata) directly to its
   final destination. Returns the parsed JSON output from yt-dlp.
 
-  It writes to file and then immediately reads from it since printing
-  to stdout also contains any warnings/errors that may have occurred,
-  even if the command is otherwise successful (which creates invalid
-  JSON).
-
   Returns {:ok, map()} | {:error, any, ...}.
-
-  TODO: test changes (writing to then reading from a file)
   """
   def download(url, command_opts \\ []) do
-    json_output_path = Path.join([metadata_directory(), "#{StringUtils.random_string()}.json"])
+    opts = [:no_simulate] ++ command_opts
-    # These must stay in exactly this order, hence why I'm giving it its own variable.
-    # Also, can't use RAM file since yt-dlp needs a concrete filepath.
-    print_to_file_opts = [{:print_to_file, "after_move:%()j"}, json_output_path]
-    opts = [:no_simulate] ++ print_to_file_opts ++ command_opts
 
-    with {:ok, _} <- backend_runner().run(url, opts),
+    with {:ok, output} <- backend_runner().run(url, opts, "after_move:%()j"),
-         {:ok, file_body} <- File.read(json_output_path),
+         {:ok, parsed_json} <- Phoenix.json_library().decode(output) do
-         {:ok, parsed_json} <- Phoenix.json_library().decode(file_body) do
       {:ok, parsed_json}
     else
       err -> err
@@ -37,8 +23,4 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.Video do
   defp backend_runner do
     Application.get_env(:pinchflat, :yt_dlp_runner)
   end
-
-  defp metadata_directory do
-    Application.get_env(:pinchflat, :metadata_directory)
-  end
 end

lib/pinchflat/media_client/backends/yt_dlp/video_collection.ex

@@ -16,9 +16,9 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollection do
       """
       def get_video_ids(url, command_opts \\ []) do
         runner = Application.get_env(:pinchflat, :yt_dlp_runner)
-        opts = command_opts ++ [:simulate, :skip_download, print: :id]
+        opts = command_opts ++ [:simulate, :skip_download]
 
-        case runner.run(url, opts) do
+        case runner.run(url, opts, "%(id)s") do
           {:ok, output} -> {:ok, String.split(output, "\n", trim: true)}
           res -> res
         end

lib/pinchflat/media_client/video_downloader.ex

@@ -18,27 +18,14 @@ defmodule Pinchflat.MediaClient.VideoDownloader do
   alias Pinchflat.MediaClient.Backends.YtDlp.MetadataParser, as: YtDlpMetadataParser
 
   @doc """
-  Downloads a single video based on the settings in the given media profile.
+  Downloads a video for a media item, updating the media item based on the metadata
+  returned by the backend. Also saves the entire metadata response to the associated
+  media_metadata record.
 
-  Returns {:ok, map()} | {:error, any, ...}.
+  Returns {:ok, %MediaItem{}} | {:error, any, ...any}
-  """
-  def download_for_media_profile(url, %MediaProfile{} = media_profile, backend \\ :yt_dlp) do
-    option_builder = option_builder(backend)
-    video_backend = video_backend(backend)
-    {:ok, options} = option_builder.build(media_profile)
-
-    video_backend.download(url, options)
-  end
-
-  @doc """
-  TODO: test
-  TODO: consider removing the above function? I don't know if it's actually useful
-  TODO: save metadata filepath to media item record
-  TODO: consider saving the output JSON to the database instead of the filesystem.
-        reason: would make updating metadata easier (no orphans). Also queryable.
   """
   def download_for_media_item(%MediaItem{} = media_item, backend \\ :yt_dlp) do
-    item_with_preloads = Repo.preload(media_item, channel: :media_profile)
+    item_with_preloads = Repo.preload(media_item, [:metadata, channel: :media_profile])
     media_profile = item_with_preloads.channel.media_profile
 
     case download_for_media_profile(media_item.media_id, media_profile, backend) do
@@ -46,13 +33,23 @@ defmodule Pinchflat.MediaClient.VideoDownloader do
         parser = metadata_parser(backend)
         parsed_attrs = parser.parse_for_media_item(parsed_json)
 
-        Media.update_media_item(media_item, parsed_attrs)
+        # Don't forgor to use preloaded associations or updates to
+        # associations won't work!
+        Media.update_media_item(item_with_preloads, parsed_attrs)
 
       err ->
         err
     end
   end
 
+  defp download_for_media_profile(url, %MediaProfile{} = media_profile, backend) do
+    option_builder = option_builder(backend)
+    video_backend = video_backend(backend)
+    {:ok, options} = option_builder.build(media_profile)
+
+    video_backend.download(url, options)
+  end
+
   defp option_builder(backend) do
     case backend do
       :yt_dlp -> YtDlpOptionBuilder

lib/pinchflat/media_source.ex

@@ -33,7 +33,7 @@ defmodule Pinchflat.MediaSource do
 
   Returns {:ok, %Channel{}} | {:error, %Ecto.Changeset{}}
   """
-  def create_channel(attrs \\ %{}) do
+  def create_channel(attrs) do
     %Channel{}
     |> change_channel_from_url(attrs)
     |> commit_and_start_indexing()
@@ -101,7 +101,7 @@ 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.
   """
-  def change_channel_from_url(%Channel{} = channel, attrs \\ %{}) do
+  def change_channel_from_url(%Channel{} = channel, attrs) do
     case change_channel(channel, attrs) do
       %Ecto.Changeset{changes: %{original_url: _}} = changeset ->
         add_channel_details_to_changeset(channel, changeset)

lib/pinchflat/profiles.ex

@@ -25,7 +25,7 @@ defmodule Pinchflat.Profiles do
   @doc """
   Creates a media_profile. Returns {:ok, %MediaProfile{}} | {:error, %Ecto.Changeset{}}
   """
-  def create_media_profile(attrs \\ %{}) do
+  def create_media_profile(attrs) do
     %MediaProfile{}
     |> MediaProfile.changeset(attrs)
     |> Repo.insert()

lib/pinchflat/tasks.ex

@@ -56,7 +56,7 @@ defmodule Pinchflat.Tasks do
   @doc """
   Creates a task. Returns {:ok, %Task{}} | {:error, %Ecto.Changeset{}}.
   """
-  def create_task(attrs \\ %{}) do
+  def create_task(attrs) do
     %Task{}
     |> Task.changeset(attrs)
     |> Repo.insert()

lib/pinchflat/utils/string_utils.ex

@@ -5,6 +5,8 @@ defmodule Pinchflat.Utils.StringUtils do
 
   @doc """
   Converts a string to kebab-case (ie: `hello world` -> `hello-world`)
+
+  Returns binary()
   """
   def to_kebab_case(string) do
     string
@@ -13,7 +15,9 @@ defmodule Pinchflat.Utils.StringUtils do
   end
 
   @doc """
-  TODO: test
+  Returns a random string of the given length. Base 16 encoded, lower case.
+
+  Returns binary()
   """
   def random_string(length \\ 32) do
     :crypto.strong_rand_bytes(length)

lib/pinchflat/workers/media_indexing_worker.ex

@@ -25,6 +25,11 @@ defmodule Pinchflat.Workers.MediaIndexingWorker do
     actually run every 1 hour and 30 minutes. The tradeoff of not inundating
     the API with requests and also not overlapping jobs is worth it, IMO.
 
+  NOTE: Since indexing can take a LONG time, I should check what happens if an
+  application restart occurs while a job is running. Will the job be lost?
+
+  IDEA: Should I use paging and do indexing in chunks? Is that even faster?
+
   Returns :ok | {:ok, %Task{}}. Not that it matters.
   """
   def perform(%Oban.Job{args: %{"id" => channel_id}}) do

priv/repo/migrations/20240129015810_create_media_metadata.exs

@@ -0,0 +1,15 @@
+defmodule Pinchflat.Repo.Migrations.CreateMediaMetadata do
+  use Ecto.Migration
+
+  def change do
+    create table(:media_metadata) do
+      add :client_response, :jsonb, null: false
+      add :media_item_id, references(:media_items, on_delete: :delete_all), null: false
+
+      timestamps(type: :utc_datetime)
+    end
+
+    create unique_index(:media_metadata, [:media_item_id])
+    create index(:media_metadata, [:client_response], using: :gin)
+  end
+end

test/pinchflat/media_client/backends/yt_dlp/channel_test.exs

@@ -11,7 +11,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.ChannelTest do
 
   describe "get_channel_details/1" do
     test "it returns a %ChannelDetails{} with data on success" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
         {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"}
       end)
 
@@ -20,8 +20,9 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.ChannelTest do
     end
 
     test "it passes the expected args to the backend runner" do
-      expect(YtDlpRunnerMock, :run, fn @channel_url, opts ->
+      expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot ->
-        assert opts == [{:print, "%(.{channel,channel_id})j"}, {:playlist_end, 1}]
+        assert opts == [playlist_end: 1]
+        assert ot == "%(.{channel,channel_id})j"
 
         {:ok, "{}"}
       end)
@@ -30,13 +31,13 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.ChannelTest do
     end
 
     test "it returns an error if the runner returns an error" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:error, "Big issue", 1} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "Big issue", 1} end)
 
       assert {:error, "Big issue", 1} = Channel.get_channel_details(@channel_url)
     end
 
     test "it returns an error if the output is not JSON" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:ok, "Not JSON"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "Not JSON"} end)
 
       assert {:error, %Jason.DecodeError{}} = Channel.get_channel_details(@channel_url)
     end

test/pinchflat/media_client/backends/yt_dlp/command_runner_test.exs

@@ -12,50 +12,49 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.CommandRunnerTest do
 
   describe "run/2" do
     test "it returns the output and status when the command succeeds" do
-      assert {:ok, _output} = Runner.run(@video_url, [])
+      assert {:ok, _output} = Runner.run(@video_url, [], "")
     end
 
     test "it converts symbol k-v arg keys to kebab case" do
-      assert {:ok, output} = Runner.run(@video_url, buffer_size: 1024)
+      assert {:ok, output} = Runner.run(@video_url, [buffer_size: 1024], "")
 
       assert String.contains?(output, "--buffer-size 1024")
     end
 
     test "it keeps string k-v arg keys untouched" do
-      assert {:ok, output} = Runner.run(@video_url, [{"--under_score", 1024}])
+      assert {:ok, output} = Runner.run(@video_url, [{"--under_score", 1024}], "")
 
       assert String.contains?(output, "--under_score 1024")
     end
 
     test "it converts symbol arg keys to kebab case" do
-      assert {:ok, output} = Runner.run(@video_url, [:ignore_errors])
+      assert {:ok, output} = Runner.run(@video_url, [:ignore_errors], "")
 
       assert String.contains?(output, "--ignore-errors")
     end
 
     test "it keeps string arg keys untouched" do
-      assert {:ok, output} = Runner.run(@video_url, ["-v"])
+      assert {:ok, output} = Runner.run(@video_url, ["-v"], "")
 
       assert String.contains?(output, "-v")
       refute String.contains?(output, "--v")
     end
 
-    test "it places arg keys (flags) at the beginning of the command" do
+    test "it includes the video url as the first argument" do
-      assert {:ok, output} =
+      assert {:ok, output} = Runner.run(@video_url, [:ignore_errors], "")
-               Runner.run(@video_url, [{"--under_score", 1024}, :ignore_errors])
 
-      assert String.contains?(output, "--ignore-errors --under_score 1024")
+      assert String.contains?(output, "#{@video_url} --ignore-errors")
     end
 
-    test "it includes the video url as the last argument" do
+    test "it automatically includes the --print-to-file flag" do
-      assert {:ok, output} = Runner.run(@video_url, [:ignore_errors])
+      assert {:ok, output} = Runner.run(@video_url, [], "%(id)s")
 
-      assert String.contains?(output, "--ignore-errors #{@video_url}\n")
+      assert String.contains?(output, "--print-to-file %(id)s /tmp/")
     end
 
     test "it returns the output and status when the command fails" do
       wrap_executable("/bin/false", fn ->
-        assert {:error, "", 1} = Runner.run(@video_url, [])
+        assert {:error, "", 1} = Runner.run(@video_url, [], "")
       end)
     end
   end

test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs

@@ -0,0 +1,39 @@
+defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do
+  use ExUnit.Case, async: true
+
+  alias Pinchflat.MediaClient.Backends.YtDlp.MetadataParser, as: Parser
+
+  setup do
+    json_filepath =
+      Path.join([
+        File.cwd!(),
+        "test",
+        "support",
+        "files",
+        "media_metadata.json"
+      ])
+
+    {:ok, file_body} = File.read(json_filepath)
+    {:ok, parsed_json} = Phoenix.json_library().decode(file_body)
+
+    {:ok,
+     %{
+       metadata: parsed_json
+     }}
+  end
+
+  describe "parse_for_media_item/1" do
+    test "it extracts the video filepath", %{metadata: metadata} do
+      result = Parser.parse_for_media_item(metadata)
+
+      assert String.contains?(result.video_filepath, "bwRHIkYqYJo")
+      assert String.ends_with?(result.video_filepath, ".mkv")
+    end
+
+    test "it returns the metadata as a map", %{metadata: metadata} do
+      result = Parser.parse_for_media_item(metadata)
+
+      assert result.metadata.client_response == metadata
+    end
+  end
+end

test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs

@@ -14,14 +14,15 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do
 
   describe "get_video_ids/2" do
     test "returns a list of video ids with no blank elements" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:ok, "id1\nid2\n\nid3\n"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "id1\nid2\n\nid3\n"} end)
 
       assert {:ok, ["id1", "id2", "id3"]} = VideoCollectionUser.get_video_ids(@channel_url)
     end
 
     test "it passes the expected default args" do
-      expect(YtDlpRunnerMock, :run, fn _url, opts ->
+      expect(YtDlpRunnerMock, :run, fn _url, opts, ot ->
-        assert opts == [:simulate, :skip_download, {:print, :id}]
+        assert opts == [:simulate, :skip_download]
+        assert ot == "%(id)s"
 
         {:ok, ""}
       end)
@@ -30,8 +31,8 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do
     end
 
     test "it passes the expected custom args" do
-      expect(YtDlpRunnerMock, :run, fn _url, opts ->
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot ->
-        assert opts == [:custom_arg, :simulate, :skip_download, {:print, :id}]
+        assert opts == [:custom_arg, :simulate, :skip_download]
 
         {:ok, ""}
       end)
@@ -40,7 +41,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do
     end
 
     test "returns the error straight through when the command fails" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:error, "Big issue", 1} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:error, "Big issue", 1} end)
 
       assert {:error, "Big issue", 1} = VideoCollectionUser.get_video_ids(@channel_url)
     end

test/pinchflat/media_client/backends/yt_dlp/video_test.exs

@@ -1,5 +1,5 @@
 defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoTest do
-  use ExUnit.Case, async: true
+  use Pinchflat.DataCase
   import Mox
 
   alias Pinchflat.MediaClient.Backends.YtDlp.Video
@@ -8,20 +8,27 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoTest do
 
   setup :verify_on_exit!
 
+  # expect(YtDlpRunnerMock, :run, fn _url, [_, _, json_output_path | _] ->
+  # copy_metadata(json_output_path)
+
+  #   {:ok, ""}
+  # end)
+
   describe "download/2" do
     test "it calls the backend runner with the expected arguments" do
-      expect(YtDlpRunnerMock, :run, fn @video_url, opts ->
+      expect(YtDlpRunnerMock, :run, fn @video_url, opts, ot ->
-        assert opts == [:no_simulate, {:print, "%()j"}]
+        assert [:no_simulate] = opts
+        assert "after_move:%()j" = ot
 
-        {:ok, "{}"}
+        {:ok, render_metadata(:media_metadata)}
       end)
 
       assert {:ok, _} = Video.download(@video_url)
     end
 
     test "it passes along additional options" do
-      expect(YtDlpRunnerMock, :run, fn _url, opts ->
+      expect(YtDlpRunnerMock, :run, fn _url, opts, _ot ->
-        assert opts == [:no_simulate, {:print, "%()j"}, :custom_arg]
+        assert [:no_simulate, :custom_arg] = opts
 
         {:ok, "{}"}
       end)
@@ -29,22 +36,21 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoTest do
       assert {:ok, _} = Video.download(@video_url, [:custom_arg])
     end
 
-    test "it parses the output as JSON" do
+    test "it parses and returns the generated file as JSON" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:ok, "{\"title\": \"Test\"}"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+        {:ok, render_metadata(:media_metadata)}
+      end)
 
-      assert {:ok, %{"title" => "Test"}} = Video.download(@video_url)
+      assert {:ok, %{"title" => "Trying to Wheelie Without the Rear Brake"}} =
+               Video.download(@video_url)
     end
 
-    test "it returns an error if the output is not JSON" do
+    test "it returns errors" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:ok, "Not JSON"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opt, _ot ->
+        {:error, "something"}
+      end)
 
-      assert {:error, %Jason.DecodeError{}} = Video.download(@video_url)
+      assert {:error, "something"} = Video.download(@video_url)
-    end
-
-    test "it directly passes along any errors" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:error, "Big issue", 1} end)
-
-      assert {:error, "Big issue", 1} = Video.download(@video_url)
     end
   end
 end

test/pinchflat/media_client/channel_details_test.exs

@@ -17,8 +17,9 @@ defmodule Pinchflat.MediaClient.ChannelDetailsTest do
 
   describe "get_channel_details/2" do
     test "it passes the expected arguments to the backend" do
-      expect(YtDlpRunnerMock, :run, fn @channel_url, opts ->
+      expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot ->
-        assert opts == [{:print, "%(.{channel,channel_id})j"}, {:playlist_end, 1}]
+        assert opts == [playlist_end: 1]
+        assert ot == "%(.{channel,channel_id})j"
 
         {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"}
       end)
@@ -27,7 +28,7 @@ defmodule Pinchflat.MediaClient.ChannelDetailsTest do
     end
 
     test "it returns a struct composed of the returned data" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
         {:ok, "{\"channel\": \"TheUselessTrials\", \"channel_id\": \"UCQH2\"}"}
       end)
 
@@ -38,8 +39,9 @@ defmodule Pinchflat.MediaClient.ChannelDetailsTest do
 
   describe "get_video_ids/2" do
     test "it passes the expected arguments to the backend" do
-      expect(YtDlpRunnerMock, :run, fn @channel_url, opts ->
+      expect(YtDlpRunnerMock, :run, fn @channel_url, opts, ot ->
-        assert opts == [:simulate, :skip_download, {:print, :id}]
+        assert opts == [:simulate, :skip_download]
+        assert ot == "%(id)s"
 
         {:ok, ""}
       end)
@@ -48,7 +50,7 @@ defmodule Pinchflat.MediaClient.ChannelDetailsTest do
     end
 
     test "it returns a list of strings" do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts ->
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
         {:ok, "video1\nvideo2\nvideo3"}
       end)
 

test/pinchflat/media_client/video_downloader_test.exs

@@ -1,35 +1,60 @@
 defmodule Pinchflat.MediaClient.VideoDownloaderTest do
-  use ExUnit.Case, async: true
+  use Pinchflat.DataCase
   import Mox
+  import Pinchflat.MediaFixtures
 
-  alias Pinchflat.Profiles.MediaProfile
   alias Pinchflat.MediaClient.VideoDownloader
 
-  @video_url "https://www.youtube.com/watch?v=TiZPUDkDYbk"
-  @media_profile %MediaProfile{
-    output_path_template: "videos/{{ title }}.%(ext)s"
-  }
-
   setup :verify_on_exit!
 
-  describe "download_for_media_profile/3" do
+  setup do
-    test "it calls the backend runner with the arguments built from the media profile" do
+    media_item =
-      expect(YtDlpRunnerMock, :run, fn @video_url, opts ->
+      Repo.preload(
-        assert :no_simulate in opts
+        media_item_fixture(%{video_filepath: nil}),
-        assert {:print, "%()j"} in opts
+        [:metadata, channel: :media_profile]
-        assert {:output, "/tmp/yt-dlp/videos/%(title)S.%(ext)s"} in opts
+      )
 
-        {:ok, "{}"}
+    {:ok, %{media_item: media_item}}
+  end
+
+  describe "download_for_media_item/3" do
+    test "it calls the backend runner", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, ot ->
+        assert ot == "after_move:%()j"
+
+        {:ok, render_metadata(:media_metadata)}
       end)
 
-      assert {:ok, _} = VideoDownloader.download_for_media_profile(@video_url, @media_profile)
+      assert {:ok, _} = VideoDownloader.download_for_media_item(media_item)
     end
 
-    test "it returns the parsed JSON output" do
+    test "it writes attributes to the media item", %{media_item: media_item} do
-      expect(YtDlpRunnerMock, :run, fn _url, _opts -> {:ok, "{\"title\": \"Test\"}"} end)
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+        {:ok, render_metadata(:media_metadata)}
+      end)
 
-      assert {:ok, %{"title" => "Test"}} =
+      assert is_nil(media_item.video_filepath)
-               VideoDownloader.download_for_media_profile(@video_url, @media_profile)
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert is_binary(updated_media_item.video_filepath)
+    end
+
+    test "it saves the metadata to the database", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+        {:ok, render_metadata(:media_metadata)}
+      end)
+
+      assert is_nil(media_item.metadata)
+      assert {:ok, updated_media_item} = VideoDownloader.download_for_media_item(media_item)
+      assert updated_media_item.metadata
+      assert is_map(updated_media_item.metadata.client_response)
+    end
+
+    test "errors are passed through", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot ->
+        {:error, :some_error}
+      end)
+
+      assert {:error, :some_error} = VideoDownloader.download_for_media_item(media_item)
     end
   end
 end

test/pinchflat/media_source_test.exs

@@ -30,7 +30,7 @@ defmodule Pinchflat.MediaSourceTest do
 
   describe "create_channel/1" do
     test "creates a channel and adds name + ID from runner response" do
-      expect(YtDlpRunnerMock, :run, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, &runner_function_mock/3)
 
       valid_attrs = %{
         media_profile_id: media_profile_fixture().id,
@@ -47,7 +47,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "creation enforces uniqueness of channel_id scoped to the media_profile" do
-      expect(YtDlpRunnerMock, :run, 2, fn _url, _opts ->
+      expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot ->
         {:ok,
          Phoenix.json_library().encode!(%{
            channel: "some name",
@@ -65,7 +65,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "creation lets you duplicate channel_ids as long as the media profile is different" do
-      expect(YtDlpRunnerMock, :run, 2, fn _url, _opts ->
+      expect(YtDlpRunnerMock, :run, 2, fn _url, _opts, _ot ->
         {:ok,
          Phoenix.json_library().encode!(%{
            channel: "some name",
@@ -86,7 +86,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "creation will schedule the indexing task" do
-      expect(YtDlpRunnerMock, :run, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, &runner_function_mock/3)
 
       valid_attrs = %{
         media_profile_id: media_profile_fixture().id,
@@ -101,7 +101,7 @@ defmodule Pinchflat.MediaSourceTest do
 
   describe "index_media_items/1" do
     setup do
-      stub(YtDlpRunnerMock, :run, fn _url, _opts -> {:ok, "video1\nvideo2\nvideo3"} end)
+      stub(YtDlpRunnerMock, :run, fn _url, _opts, _ot -> {:ok, "video1\nvideo2\nvideo3"} end)
 
       {:ok, [channel: channel_fixture()]}
     end
@@ -162,7 +162,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "updating the original_url will re-fetch the channel details" do
-      expect(YtDlpRunnerMock, :run, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, &runner_function_mock/3)
 
       channel = channel_fixture()
       update_attrs = %{original_url: "https://www.youtube.com/channel/abc123"}
@@ -173,7 +173,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "not updating the original_url will not re-fetch the channel details" do
-      expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/3)
 
       channel = channel_fixture()
       update_attrs = %{name: "some updated name"}
@@ -239,14 +239,14 @@ defmodule Pinchflat.MediaSourceTest do
 
   describe "change_channel_from_url/2" do
     test "it returns a changeset" do
-      stub(YtDlpRunnerMock, :run, &runner_function_mock/2)
+      stub(YtDlpRunnerMock, :run, &runner_function_mock/3)
       channel = channel_fixture()
 
-      assert %Ecto.Changeset{} = MediaSource.change_channel_from_url(channel)
+      assert %Ecto.Changeset{} = MediaSource.change_channel_from_url(channel, %{})
     end
 
     test "it does not fetch channel details if the original_url isn't in the changeset" do
-      expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, 0, &runner_function_mock/3)
 
       changeset = MediaSource.change_channel_from_url(%Channel{}, %{name: "some updated name"})
 
@@ -254,7 +254,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "it fetches channel details if the original_url is in the changeset" do
-      expect(YtDlpRunnerMock, :run, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, &runner_function_mock/3)
 
       changeset =
         MediaSource.change_channel_from_url(%Channel{}, %{
@@ -265,7 +265,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "it adds channel details to the changeset, keeping the orignal details" do
-      expect(YtDlpRunnerMock, :run, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, &runner_function_mock/3)
 
       media_profile = media_profile_fixture()
       media_profile_id = media_profile.id
@@ -287,7 +287,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
 
     test "it adds an error to the changeset if the runner fails" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts ->
+      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot ->
         {:error, "some error", 1}
       end)
 
@@ -301,7 +301,7 @@ defmodule Pinchflat.MediaSourceTest do
     end
   end
 
-  defp runner_function_mock(_url, _opts) do
+  defp runner_function_mock(_url, _opts, _ot) do
     {
       :ok,
       Phoenix.json_library().encode!(%{

test/pinchflat/media_test.exs

@@ -9,6 +9,18 @@ defmodule Pinchflat.MediaTest do
 
   @invalid_attrs %{title: nil, media_id: nil, video_filepath: nil}
 
+  describe "schema" do
+    test "media_metadata is deleted when media_item is deleted" do
+      media_item = media_item_fixture(%{metadata: %{client_response: %{foo: "bar"}}})
+      metadata = media_item.metadata
+      assert {:ok, %MediaItem{}} = Media.delete_media_item(media_item)
+
+      assert_raise Ecto.NoResultsError, fn ->
+        Repo.reload!(metadata)
+      end
+    end
+  end
+
   describe "list_media_items/0" do
     test "it returns all media_items" do
       media_item = media_item_fixture()

test/pinchflat/profiles/options/yt_dlp/option_builder_test.exs

@@ -5,14 +5,14 @@ defmodule Pinchflat.Profiles.Options.YtDlp.OptionBuilderTest do
   alias Pinchflat.Profiles.Options.YtDlp.OptionBuilder
 
   @media_profile %MediaProfile{
-    output_path_template: "videos/{{ title }}.%(ext)s"
+    output_path_template: "{{ title }}.%(ext)s"
   }
 
   describe "build/1" do
     test "it generates an expanded output path based on the given template" do
       assert {:ok, res} = OptionBuilder.build(@media_profile)
 
-      assert {:output, "/tmp/yt-dlp/videos/%(title)S.%(ext)s"} in res
+      assert {:output, "/tmp/videos/%(title)S.%(ext)s"} in res
     end
   end
 end

test/pinchflat/tasks_test.exs

@@ -110,6 +110,12 @@ defmodule Pinchflat.TasksTest do
 
       assert task.channel_id == channel.id
     end
+
+    test "it returns an error if the job fails to enqueue" do
+      channel = channel_fixture()
+
+      assert {:error, %Ecto.Changeset{}} = Tasks.create_job_with_task(%Ecto.Changeset{}, channel)
+    end
   end
 
   describe "delete_task/1" do

test/pinchflat/utils/string_utils_test.exs

@@ -12,4 +12,19 @@ defmodule Pinchflat.Utils.StringUtilsTest do
       assert StringUtils.to_kebab_case("hello_world") == "hello-world"
     end
   end
+
+  describe "random_string/1" do
+    test "generates a random string" do
+      assert is_binary(StringUtils.random_string())
+      assert StringUtils.random_string() != StringUtils.random_string()
+    end
+
+    test "has a defined default length" do
+      assert String.length(StringUtils.random_string()) == 32
+    end
+
+    test "can generate a string of a given length" do
+      assert String.length(StringUtils.random_string(64)) == 64
+    end
+  end
 end

test/pinchflat/workers/media_indexing_worker_test.exs

@@ -11,7 +11,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
 
   describe "perform/1" do
     test "it does not do any indexing if the channel shouldn't be indexed" do
-      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot -> {:ok, ""} end)
 
       channel = channel_fixture(index_frequency_minutes: -1)
 
@@ -19,7 +19,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it does not reschedule if the channel shouldn't be indexed" do
-      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 0, fn _url, _opts, _ot -> {:ok, ""} end)
 
       channel = channel_fixture(index_frequency_minutes: -1)
       perform_job(MediaIndexingWorker, %{id: channel.id})
@@ -28,7 +28,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it indexes the channel if it should be indexed" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot -> {:ok, ""} end)
 
       channel = channel_fixture(index_frequency_minutes: 10)
 
@@ -36,7 +36,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it reschedules the job based on the index frequency" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot -> {:ok, ""} end)
 
       channel = channel_fixture(index_frequency_minutes: 10)
       perform_job(MediaIndexingWorker, %{id: channel.id})
@@ -49,7 +49,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it creates a task for the rescheduled job" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts -> {:ok, ""} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot -> {:ok, ""} end)
 
       channel = channel_fixture(index_frequency_minutes: 10)
       task_count_fetcher = fn -> Enum.count(Tasks.list_tasks()) end
@@ -60,7 +60,7 @@ defmodule Pinchflat.Workers.MediaIndexingWorkerTest do
     end
 
     test "it creates the basic media_item records" do
-      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts -> {:ok, "video1\nvideo2"} end)
+      expect(YtDlpRunnerMock, :run, 1, fn _url, _opts, _ot -> {:ok, "video1\nvideo2"} end)
 
       channel = channel_fixture(index_frequency_minutes: 10)
 

test/pinchflat_web/controllers/channel_controller_test.exs

@@ -41,7 +41,7 @@ defmodule PinchflatWeb.ChannelControllerTest do
 
   describe "create channel" do
     test "redirects to show when data is valid", %{conn: conn, create_attrs: create_attrs} do
-      expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/3)
       conn = post(conn, ~p"/media_sources/channels", channel: create_attrs)
 
       assert %{id: id} = redirected_params(conn)
@@ -70,7 +70,7 @@ defmodule PinchflatWeb.ChannelControllerTest do
     setup [:create_channel]
 
     test "redirects when data is valid", %{conn: conn, channel: channel, update_attrs: update_attrs} do
-      expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/2)
+      expect(YtDlpRunnerMock, :run, 1, &runner_function_mock/3)
 
       conn = put(conn, ~p"/media_sources/channels/#{channel}", channel: update_attrs)
       assert redirected_to(conn) == ~p"/media_sources/channels/#{channel}"
@@ -107,7 +107,7 @@ defmodule PinchflatWeb.ChannelControllerTest do
     %{channel: channel}
   end
 
-  defp runner_function_mock(_url, _opts) do
+  defp runner_function_mock(_url, _opts, _ot) do
     {
       :ok,
       Phoenix.json_library().encode!(%{

test/support/fixtures/media_fixtures.ex

@@ -22,4 +22,24 @@ defmodule Pinchflat.MediaFixtures do
 
     media_item
   end
+
+  @doc """
+  Generate a media_item with metadata.
+  """
+  def media_item_with_metadata(attrs \\ %{}) do
+    json_filepath =
+      Path.join([
+        Path.dirname(__ENV__.file),
+        "support",
+        "fixtures",
+        "files",
+        "media_metadata.json"
+      ])
+
+    {:ok, file_body} = File.read(json_filepath)
+    {:ok, parsed_json} = Phoenix.json_library().decode(file_body)
+    merged_attrs = Map.merge(attrs, %{metadata: %{client_respinse: parsed_json}})
+
+    media_item_fixture(merged_attrs)
+  end
 end

test/support/fixtures/profiles_fixtures.ex

@@ -12,7 +12,7 @@ defmodule Pinchflat.ProfilesFixtures do
       attrs
       |> Enum.into(%{
         name: "Media Profile ##{:rand.uniform(1_000_000)}",
-        output_path_template: "/video/{{title}}.{{ext}}"
+        output_path_template: "{{title}}.{{ext}}"
       })
       |> Pinchflat.Profiles.create_media_profile()
 

test/support/scripts/yt-dlp-mocks/repeater.sh

@@ -1,7 +1,16 @@
 #!/bin/bash
 
-if [[ "$@" == *"--dump-json"* ]]; then
+# Args come in the format of "<unknown number of args> --print-to-file <output template> <file location> <unknown number of args>".
-  echo '{ "args": "'$@'"}'
+# I need to extract <file location> and write all args to it.
-else
+
-  echo $@
+# Extract the file location (in an unknown position BUT it's 2 args after --print-to-file).
-fi
+for ((i = 1; i <= $#; i++)); do
+  if [ "${!i}" == "--print-to-file" ]; then
+    # Extract the file location.
+    file_location="${@:i+2:1}"
+    break
+  fi
+done
+
+# Write all args to the file
+echo "$@" >"$file_location"

test/support/testing_helper_methods.ex

@@ -27,4 +27,17 @@ defmodule Pinchflat.TestingHelperMethods do
     assert before_res == from
     assert after_res == to
   end
+
+  def render_metadata(metadata_name) do
+    json_filepath =
+      Path.join([
+        File.cwd!(),
+        "test",
+        "support",
+        "files",
+        "#{metadata_name}.json"
+      ])
+
+    File.read!(json_filepath)
+  end
 end