refactored to use a precedence-based approach

This commit is contained in:
Kieran Eglin 2024-04-10 12:21:53 -07:00
parent 07b4ba3c3f
commit 176d4c53b5
No known key found for this signature in database
GPG key ID: 193984967FCF432D
6 changed files with 126 additions and 36 deletions

View file

@ -51,7 +51,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do
end
defp create_blank_yt_dlp_files do
files = ["cookies.txt", "yt-dlp-configs/main.txt"]
files = ["cookies.txt", "yt-dlp-configs/base-config.txt"]
base_dir = Application.get_env(:pinchflat, :extras_directory)
Enum.each(files, fn file ->

View file

@ -10,8 +10,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
@doc """
Builds the options for yt-dlp to download media based on the given media's profile.
IDEA: consider adding the ability to pass in a second argument to override
these options
Returns {:ok, [Keyword.t()]}
"""
def build(%MediaItem{} = media_item_with_preloads) do
media_profile = media_item_with_preloads.source.media_profile
@ -23,7 +22,8 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
metadata_options(media_profile) ++
quality_options(media_profile) ++
sponsorblock_options(media_profile) ++
output_options(media_item_with_preloads)
output_options(media_item_with_preloads) ++
config_file_options(media_item_with_preloads)
{:ok, built_options}
end
@ -128,6 +128,35 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
end
end
# This is put here instead of the CommandRunner module because it should only
# be applied to downloading - if it were in CommandRunner it would apply to
# all yt-dlp commands (like indexing)
defp config_file_options(media_item) do
base_dir = Path.join(Application.get_env(:pinchflat, :extras_directory), "yt-dlp-configs")
# Ordered by priority - the first file has the highest priority
filenames = [
"media-item-#{media_item.id}-config.txt",
"source-#{media_item.source_id}-config.txt",
"media-profile-#{media_item.source.media_profile_id}-config.txt",
"base-config.txt"
]
config_filepaths =
Enum.reduce(filenames, [], fn filename, acc ->
filepath = Path.join(base_dir, filename)
case File.read(filepath) do
{:ok, file_data} ->
if String.trim(file_data) != "", do: [filepath | acc], else: acc
{:error, _} ->
acc
end
end)
Enum.map(config_filepaths, fn filepath -> {:config_locations, filepath} end)
end
defp output_options(media_item_with_preloads) do
[
output: build_output_path_for(media_item_with_preloads.source)

View file

@ -76,11 +76,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do
defp build_external_file_options do
base_dir = Application.get_env(:pinchflat, :extras_directory)
filename_options_map = %{
cookies: "cookies.txt",
config_locations: "yt-dlp-configs/main.txt"
}
filename_options_map = %{cookies: "cookies.txt"}
Enum.reduce(filename_options_map, [], fn {opt_name, filename}, acc ->
filepath = Path.join(base_dir, filename)

View file

@ -42,7 +42,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasksTest do
test "creates a blank yt-dlp config file" do
base_dir = Application.get_env(:pinchflat, :extras_directory)
filepath = Path.join([base_dir, "yt-dlp-configs", "main.txt"])
filepath = Path.join([base_dir, "yt-dlp-configs", "base-config.txt"])
File.rm(filepath)
refute File.exists?(filepath)

View file

@ -5,6 +5,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
import Pinchflat.ProfilesFixtures
alias Pinchflat.Profiles
alias Pinchflat.Utils.FilesystemUtils
alias Pinchflat.Downloading.DownloadOptionBuilder
setup do
@ -261,6 +262,96 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
end
end
describe "build/1 when testing config file options" do
setup do
base_dir = Path.join(Application.get_env(:pinchflat, :extras_directory), "yt-dlp-configs")
{:ok, %{base_dir: base_dir}}
end
test "includes base config file if it's present", %{media_item: media_item, base_dir: base_dir} do
filepath = Path.join(base_dir, "base-config.txt")
FilesystemUtils.write_p!(filepath, "base config")
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
assert {:config_locations, filepath} in res
end
test "includes media profile config file if it's present", %{media_item: media_item, base_dir: base_dir} do
media_profile = media_item.source.media_profile
filepath = Path.join(base_dir, "media-profile-#{media_profile.id}-config.txt")
FilesystemUtils.write_p!(filepath, "profile config")
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
assert {:config_locations, filepath} in res
end
test "includes source config file if it's present", %{media_item: media_item, base_dir: base_dir} do
source = media_item.source
filepath = Path.join(base_dir, "source-#{source.id}-config.txt")
FilesystemUtils.write_p!(filepath, "profile config")
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
assert {:config_locations, filepath} in res
end
test "includes media item config file if it's present", %{media_item: media_item, base_dir: base_dir} do
filepath = Path.join(base_dir, "media-item-#{media_item.id}-config.txt")
FilesystemUtils.write_p!(filepath, "media item config")
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
assert {:config_locations, filepath} in res
end
test "does not include config file options if they are not present", %{media_item: media_item} do
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
refute :config_locations in res
end
test "does not return a config file if it's blank", %{media_item: media_item, base_dir: base_dir} do
filepath = Path.join(base_dir, "base-config.txt")
FilesystemUtils.write_p!(filepath, " \n \n ")
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
refute :config_locations in res
end
test "returns config files in order of precedence", %{media_item: media_item, base_dir: base_dir} do
source = media_item.source
media_profile = source.media_profile
base_filepath = Path.join(base_dir, "base-config.txt")
source_filepath = Path.join(base_dir, "source-#{source.id}-config.txt")
media_item_filepath = Path.join(base_dir, "media-item-#{media_item.id}-config.txt")
media_profile_filepath = Path.join(base_dir, "media-profile-#{media_profile.id}-config.txt")
FilesystemUtils.write_p!(base_filepath, "config")
FilesystemUtils.write_p!(source_filepath, "config")
FilesystemUtils.write_p!(media_item_filepath, "config")
FilesystemUtils.write_p!(media_profile_filepath, "config")
assert {:ok, res} = DownloadOptionBuilder.build(media_item)
expected_order = [
{:config_locations, base_filepath},
{:config_locations, media_profile_filepath},
{:config_locations, source_filepath},
{:config_locations, media_item_filepath}
]
assert Enum.filter(res, fn
{:config_locations, _} -> true
_ -> false
end) == expected_order
end
end
describe "build_output_path_for/1" do
test "builds an output path for a source", %{media_item: media_item} do
path = DownloadOptionBuilder.build_output_path_for(media_item.source)

View file

@ -76,32 +76,6 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do
refute String.contains?(output, "--cookies")
refute String.contains?(output, cookie_file)
end
test "includes yt-dlp options when config file exists", %{yt_dlp_file: yt_dlp_file} do
FilesystemUtils.write_p!(yt_dlp_file, "config data")
assert {:ok, output} = Runner.run(@media_url, [], "")
assert String.contains?(output, "--config-locations #{yt_dlp_file}")
end
test "doesn't include yt-dlp options when config file blank", %{yt_dlp_file: yt_dlp_file} do
FilesystemUtils.write_p!(yt_dlp_file, " \n \n ")
assert {:ok, output} = Runner.run(@media_url, [], "")
refute String.contains?(output, "--config-locations")
refute String.contains?(output, yt_dlp_file)
end
test "ddoesn't include yt-dlp options when config file doesn't exist", %{yt_dlp_file: yt_dlp_file} do
File.rm(yt_dlp_file)
assert {:ok, output} = Runner.run(@media_url, [], "")
refute String.contains?(output, "--config-locations")
refute String.contains?(output, yt_dlp_file)
end
end
describe "version/0" do