From 028497f41534fda2679657d89dc245789f984421 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Wed, 13 Mar 2024 21:15:41 -0700 Subject: [PATCH] Appended -thumb to thumbnails when downloading --- .../downloading/download_option_builder.ex | 35 ++++++++++++--- lib/pinchflat/profiles/media_profile.ex | 6 +++ .../download_option_builder_test.exs | 10 ++++- test/pinchflat/profiles_test.exs | 44 +++++++++++++++++-- .../media_profile_controller_test.exs | 4 +- 5 files changed, 85 insertions(+), 14 deletions(-) diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex index dabf407..2c4a77e 100644 --- a/lib/pinchflat/downloading/download_option_builder.ex +++ b/lib/pinchflat/downloading/download_option_builder.ex @@ -18,7 +18,7 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do built_options = default_options() ++ subtitle_options(media_profile) ++ - thumbnail_options(media_profile) ++ + thumbnail_options(media_item_with_preloads) ++ metadata_options(media_profile) ++ quality_options(media_profile) ++ output_options(media_item_with_preloads) @@ -57,13 +57,16 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end) end - defp thumbnail_options(media_profile) do + defp thumbnail_options(media_item_with_preloads) do + media_profile = media_item_with_preloads.source.media_profile mapped_struct = Map.from_struct(media_profile) Enum.reduce(mapped_struct, [], fn attr, acc -> case attr do {:download_thumbnail, true} -> - acc ++ [:write_thumbnail, convert_thumbnail: "jpg"] + thumbnail_save_location = determine_thumbnail_location(media_item_with_preloads) + + acc ++ [:write_thumbnail, convert_thumbnail: "jpg", output: "thumbnail:#{thumbnail_save_location}"] {:embed_thumbnail, true} -> acc ++ [:embed_thumbnail, convert_thumbnail: "jpg"] @@ -102,15 +105,20 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do end defp output_options(media_item_with_preloads) do - media_profile = media_item_with_preloads.source.media_profile - additional_options_map = output_options_map(media_item_with_preloads) - {:ok, output_path} = OutputPathBuilder.build(media_profile.output_path_template, additional_options_map) + output_path_template = media_item_with_preloads.source.media_profile.output_path_template [ - output: Path.join(base_directory(), output_path) + output: build_output_path(output_path_template, media_item_with_preloads) ] end + defp build_output_path(string, media_item_with_preloads) do + additional_options_map = output_options_map(media_item_with_preloads) + {:ok, output_path} = OutputPathBuilder.build(string, additional_options_map) + + Path.join(base_directory(), output_path) + end + defp output_options_map(media_item_with_preloads) do source = media_item_with_preloads.source @@ -120,6 +128,19 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do } end + # I don't love the string manipulation here, but what can ya' do. + # It's dependent on the output_path_template being a string ending `.{{ ext }}` + # (or equivalent), but that's validated by the MediaProfile schema. + defp determine_thumbnail_location(media_item_with_preloads) do + output_path_template = media_item_with_preloads.source.media_profile.output_path_template + + output_path_template + |> String.split(~r{\.}, include_captures: true) + |> List.insert_at(-3, "-thumb") + |> Enum.join() + |> build_output_path(media_item_with_preloads) + end + defp base_directory do Application.get_env(:pinchflat, :media_directory) end diff --git a/lib/pinchflat/profiles/media_profile.ex b/lib/pinchflat/profiles/media_profile.ex index 7ecddb7..ef60b02 100644 --- a/lib/pinchflat/profiles/media_profile.ex +++ b/lib/pinchflat/profiles/media_profile.ex @@ -67,6 +67,12 @@ defmodule Pinchflat.Profiles.MediaProfile do media_profile |> cast(attrs, @allowed_fields) |> validate_required(@required_fields) + # Ensures it ends with `.{{ ext }}` or `.%(ext)s` or similar (with a little wiggle room) + |> validate_format(:output_path_template, ext_regex(), message: "must end with .{{ ext }}") |> unique_constraint(:name) end + + defp ext_regex do + ~r/\.({{ ?ext ?}}|%\( ?ext ?\)[sS])$/ + end end diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs index 22fba78..bd88236 100644 --- a/test/pinchflat/downloading/download_option_builder_test.exs +++ b/test/pinchflat/downloading/download_option_builder_test.exs @@ -125,7 +125,15 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do assert :write_thumbnail in res end - test "convertes thumbnail to jpg when download_thumbnail is true", %{media_item: media_item} do + test "appends -thumb to the thumbnail name when download_thumbnail is true", %{media_item: media_item} do + media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true}) + + assert {:ok, res} = DownloadOptionBuilder.build(media_item) + + assert {:output, "thumbnail:/tmp/test/media/%(title)S-thumb.%(ext)s"} in res + end + + test "converts thumbnail to jpg when download_thumbnail is true", %{media_item: media_item} do media_item = update_media_profile_attribute(media_item, %{download_thumbnail: true}) assert {:ok, res} = DownloadOptionBuilder.build(media_item) diff --git a/test/pinchflat/profiles_test.exs b/test/pinchflat/profiles_test.exs index ea127c6..6bb597e 100644 --- a/test/pinchflat/profiles_test.exs +++ b/test/pinchflat/profiles_test.exs @@ -26,11 +26,11 @@ defmodule Pinchflat.ProfilesTest do describe "create_media_profile/1" do test "creation with valid data creates a media_profile" do - valid_attrs = %{name: "some name", output_path_template: "some output_path_template"} + valid_attrs = %{name: "some name", output_path_template: "output_template.{{ ext }}"} assert {:ok, %MediaProfile{} = media_profile} = Profiles.create_media_profile(valid_attrs) assert media_profile.name == "some name" - assert media_profile.output_path_template == "some output_path_template" + assert media_profile.output_path_template == "output_template.{{ ext }}" end test "creation with invalid data returns error changeset" do @@ -44,14 +44,14 @@ defmodule Pinchflat.ProfilesTest do update_attrs = %{ name: "some updated name", - output_path_template: "some updated output_path_template" + output_path_template: "new_output_template.{{ ext }}" } assert {:ok, %MediaProfile{} = media_profile} = Profiles.update_media_profile(media_profile, update_attrs) assert media_profile.name == "some updated name" - assert media_profile.output_path_template == "some updated output_path_template" + assert media_profile.output_path_template == "new_output_template.{{ ext }}" end test "updating with invalid data returns error changeset" do @@ -132,5 +132,41 @@ defmodule Pinchflat.ProfilesTest do media_profile = media_profile_fixture() assert %Ecto.Changeset{} = Profiles.change_media_profile(media_profile) end + + test "it ensures the media profile's output template ends with an extension" do + valid_templates = [ + "output_template.{{ ext }}", + "output_template.{{ext}}", + "output_template.%(ext)s", + "output_template.%(ext)S", + "output_template.%( ext )s", + "output_template.%( ext )S" + ] + + for template <- valid_templates do + cs = Profiles.change_media_profile(%MediaProfile{}, %{name: "a", output_path_template: template}) + + assert cs.valid? + end + end + + test "it does not allow invalid output templates" do + invalid_templates = [ + "output_template.{{ ext }}.something", + "output_template.{{ ext }}", + "output_template{{ ext }}", + "output_template.%(ext)s.something", + "output_template.txt", + "output_template%(ext)s", + "output_template.%(nope)s", + "output_template" + ] + + for template <- invalid_templates do + cs = Profiles.change_media_profile(%MediaProfile{}, %{name: "a", output_path_template: template}) + + refute cs.valid? + end + end end end diff --git a/test/pinchflat_web/controllers/media_profile_controller_test.exs b/test/pinchflat_web/controllers/media_profile_controller_test.exs index 1a2387b..8a707eb 100644 --- a/test/pinchflat_web/controllers/media_profile_controller_test.exs +++ b/test/pinchflat_web/controllers/media_profile_controller_test.exs @@ -8,10 +8,10 @@ defmodule PinchflatWeb.MediaProfileControllerTest do alias Pinchflat.Repo alias Pinchflat.Settings - @create_attrs %{name: "some name", output_path_template: "some output_path_template"} + @create_attrs %{name: "some name", output_path_template: "output_template.{{ ext }}"} @update_attrs %{ name: "some updated name", - output_path_template: "some updated output_path_template" + output_path_template: "new_output_template.{{ ext }}" } @invalid_attrs %{name: nil, output_path_template: nil}