From dc264897d6da6e648a26c495c8a5473a0a8c6a18 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Sat, 24 Feb 2024 11:56:23 -0800 Subject: [PATCH] Updated app to store compressed metadata; automatically download thumbnails --- .iex.exs | 1 + lib/pinchflat/http/http_behaviour.ex | 8 ++ lib/pinchflat/http/http_client.ex | 35 ++++++++ .../backends/yt_dlp/command_runner.ex | 4 +- .../backends/yt_dlp/metadata_file_helpers.ex | 74 ++++++++++++++++ .../backends/yt_dlp/metadata_parser.ex | 9 +- .../media_client/video_downloader.ex | 15 +++- .../yt_dlp/metadata_file_helpers_test.exs | 87 +++++++++++++++++++ .../backends/yt_dlp/metadata_parser_test.exs | 8 +- .../yt_dlp/output_path_builder_test.exs | 2 +- .../backends/yt_dlp/video_collection_test.exs | 2 +- .../media_client/video_downloader_test.exs | 11 ++- test/pinchflat/media_test.exs | 4 +- .../workers/video_download_worker_test.exs | 4 + test/support/fixtures/media_fixtures.ex | 19 ++-- 15 files changed, 244 insertions(+), 39 deletions(-) create mode 100644 lib/pinchflat/http/http_behaviour.ex create mode 100644 lib/pinchflat/http/http_client.ex create mode 100644 lib/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers.ex create mode 100644 test/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers_test.exs diff --git a/.iex.exs b/.iex.exs index f9b879b..b15931d 100644 --- a/.iex.exs +++ b/.iex.exs @@ -13,6 +13,7 @@ alias Pinchflat.Profiles alias Pinchflat.Sources alias Pinchflat.MediaClient.{SourceDetails, VideoDownloader} +alias Pinchflat.Metadata.{Zipper, ThumbnailFetcher} defmodule IexHelpers do def playlist_url do diff --git a/lib/pinchflat/http/http_behaviour.ex b/lib/pinchflat/http/http_behaviour.ex new file mode 100644 index 0000000..2fe639e --- /dev/null +++ b/lib/pinchflat/http/http_behaviour.ex @@ -0,0 +1,8 @@ +defmodule Pinchflat.HTTP.HTTPBehaviour do + @moduledoc """ + This module defines the behaviour for HTTP clients. Literally just + so I can use Mox to create an HTTP mock + """ + + @callback get(String.t(), Keyword.t(), Keyword.t()) :: {:ok, String.t()} | {:error, String.t()} +end diff --git a/lib/pinchflat/http/http_client.ex b/lib/pinchflat/http/http_client.ex new file mode 100644 index 0000000..45abda4 --- /dev/null +++ b/lib/pinchflat/http/http_client.ex @@ -0,0 +1,35 @@ +defmodule Pinchflat.HTTP.HTTPClient do + @moduledoc """ + This module provides a simple interface for making HTTP requests. + + Made to be easily swappable with other HTTP clients. If you need more complexity + or security, check out HTTPoison or Mint. + """ + + alias Pinchflat.HTTP.HTTPBehaviour + + @behaviour HTTPBehaviour + + @doc """ + Makes a GET request to the given URL and returns the response. + + NOTE: I can't really test this with Mox and I can't think of a way to test this + that isn't ultimately redundant. I'm just going to leave it untested for now and + focus more on testing the consumers of this module. + + Returns {:ok, String.t()} | {:error, String.t()} + """ + @impl HTTPBehaviour + def get(url, headers \\ [], opts \\ []) do + case :httpc.request(:get, {url, headers}, [], opts) do + {:ok, {{_version, 200, _reason_phrase}, _headers, body}} -> + {:ok, body} + + {:ok, {{_version, status_code, reason_phrase}, _headers, _body}} -> + {:error, "HTTP request failed with status code #{status_code}: #{reason_phrase}"} + + {:error, reason} -> + {:error, "HTTP request failed: #{reason}"} + end + end +end diff --git a/lib/pinchflat/media_client/backends/yt_dlp/command_runner.ex b/lib/pinchflat/media_client/backends/yt_dlp/command_runner.ex index bb98b20..9abe6cb 100644 --- a/lib/pinchflat/media_client/backends/yt_dlp/command_runner.ex +++ b/lib/pinchflat/media_client/backends/yt_dlp/command_runner.ex @@ -44,8 +44,8 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.CommandRunner do 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"]) + tmpfile_directory = Application.get_env(:pinchflat, :tmpfile_directory) + filepath = Path.join([tmpfile_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)) diff --git a/lib/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers.ex b/lib/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers.ex new file mode 100644 index 0000000..1863df0 --- /dev/null +++ b/lib/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers.ex @@ -0,0 +1,74 @@ +defmodule Pinchflat.MediaClient.Backends.YtDlp.MetadataFileHelpers do + @moduledoc """ + Provides methods for creating/downloading/storing related metadata + out-of-band of the normal yt-dlp backend process. + + The idea is that I don't want to craft a complicated yt-dlp command, + instead focusing on downloading the video as the user wants it then + I can use the result of that here to grab the additional information + needed + """ + + # TODO: ensure media metadata is deleted when the media item is deleted + + @doc """ + Compresses and stores metadata for a media item, returning the filepath. + + Returns binary() + """ + def compress_and_store_metadata_for(database_record, metadata_map) do + filepath = generate_filepath_for(database_record, "metadata.json.gz") + {:ok, json} = Phoenix.json_library().encode(metadata_map) + + File.mkdir_p!(Path.dirname(filepath)) + :ok = File.write(filepath, json, [:compressed]) + + filepath + end + + @doc """ + Reads and decodes compressed metadata from a filepath. + + Returns {:ok, map()} | {:error, any} + """ + def read_compressed_metadata(filepath) do + {:ok, json} = File.open(filepath, [:read, :compressed], &IO.read(&1, :all)) + + Phoenix.json_library().decode(json) + end + + @doc """ + Downloads and stores a thumbnail for a media item, returning the filepath. + + Returns binary() + """ + def download_and_store_thumbnail_for(database_record, metadata_map) do + thumbnail_url = metadata_map["thumbnail"] + filepath = generate_filepath_for(database_record, Path.basename(thumbnail_url)) + thumbnail_blob = fetch_thumbnail_from_url(thumbnail_url) + + File.mkdir_p!(Path.dirname(filepath)) + :ok = File.write(filepath, thumbnail_blob) + + filepath + end + + defp fetch_thumbnail_from_url(url) do + http_client = Application.get_env(:pinchflat, :http_client, Pinchflat.HTTP.HTTPClient) + {:ok, body} = http_client.get(url, [], body_format: :binary) + + body + end + + defp generate_filepath_for(database_record, filename) do + metadata_directory = Application.get_env(:pinchflat, :metadata_directory) + record_table_name = database_record.__meta__.source + + Path.join([ + metadata_directory, + record_table_name, + to_string(database_record.id), + filename + ]) + end +end diff --git a/lib/pinchflat/media_client/backends/yt_dlp/metadata_parser.ex b/lib/pinchflat/media_client/backends/yt_dlp/metadata_parser.ex index 757829b..a32ea73 100644 --- a/lib/pinchflat/media_client/backends/yt_dlp/metadata_parser.ex +++ b/lib/pinchflat/media_client/backends/yt_dlp/metadata_parser.ex @@ -16,13 +16,7 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MetadataParser do Returns map() """ def parse_for_media_item(metadata) do - metadata_attrs = %{ - metadata: %{ - client_response: metadata - } - } - - metadata_attrs + Map.new() |> Map.merge(parse_media_metadata(metadata)) |> Map.merge(parse_subtitle_metadata(metadata)) |> Map.merge(parse_thumbnail_metadata(metadata)) @@ -38,7 +32,6 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MetadataParser do end defp parse_subtitle_metadata(metadata) do - # IDEA: if needed, consider filtering out subtitles that don't exist on-disk subtitle_filepaths = (metadata["requested_subtitles"] || %{}) |> Enum.map(fn {lang, attrs} -> [lang, attrs["filepath"]] end) diff --git a/lib/pinchflat/media_client/video_downloader.ex b/lib/pinchflat/media_client/video_downloader.ex index 9643ef1..4ab205f 100644 --- a/lib/pinchflat/media_client/video_downloader.ex +++ b/lib/pinchflat/media_client/video_downloader.ex @@ -16,6 +16,7 @@ defmodule Pinchflat.MediaClient.VideoDownloader do alias Pinchflat.MediaClient.Backends.YtDlp.Video, as: YtDlpVideo alias Pinchflat.Profiles.Options.YtDlp.DownloadOptionBuilder, as: YtDlpDownloadOptionBuilder alias Pinchflat.MediaClient.Backends.YtDlp.MetadataParser, as: YtDlpMetadataParser + alias Pinchflat.MediaClient.Backends.YtDlp.MetadataFileHelpers, as: YtDlpMetadataHelpers @doc """ Downloads a video for a media item, updating the media item based on the metadata @@ -34,12 +35,18 @@ defmodule Pinchflat.MediaClient.VideoDownloader do case download_for_media_profile(media_item.original_url, media_profile, backend) do {:ok, parsed_json} -> - parser = metadata_parser(backend) + {parser, helpers} = metadata_parsers(backend) parsed_attrs = parsed_json |> parser.parse_for_media_item() - |> Map.merge(%{media_downloaded_at: DateTime.utc_now()}) + |> Map.merge(%{ + media_downloaded_at: DateTime.utc_now(), + metadata: %{ + metadata_filepath: helpers.compress_and_store_metadata_for(media_item, parsed_json), + thumbnail_filepath: helpers.download_and_store_thumbnail_for(media_item, parsed_json) + } + }) # Don't forgor to use preloaded associations or updates to # associations won't work! @@ -70,9 +77,9 @@ defmodule Pinchflat.MediaClient.VideoDownloader do end end - defp metadata_parser(backend) do + defp metadata_parsers(backend) do case backend do - :yt_dlp -> YtDlpMetadataParser + :yt_dlp -> {YtDlpMetadataParser, YtDlpMetadataHelpers} end end end diff --git a/test/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers_test.exs b/test/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers_test.exs new file mode 100644 index 0000000..ffbfff9 --- /dev/null +++ b/test/pinchflat/media_client/backends/yt_dlp/metadata_file_helpers_test.exs @@ -0,0 +1,87 @@ +defmodule Pinchflat.MediaClient.Backends.YtDlp.MetadataFileHelpersTest do + use Pinchflat.DataCase + import Mox + import Pinchflat.MediaFixtures + + alias Pinchflat.MediaClient.Backends.YtDlp.MetadataFileHelpers, as: Helpers + + setup do + media_item = media_item_fixture() + + {:ok, %{media_item: media_item}} + end + + setup :verify_on_exit! + + describe "compress_and_store_metadata_for/2" do + test "returns the filepath", %{media_item: media_item} do + metadata_map = %{"foo" => "bar"} + + filepath = Helpers.compress_and_store_metadata_for(media_item, metadata_map) + + assert filepath =~ ~r{/media_items/#{media_item.id}/metadata.json.gz} + end + + test "creates folder structure based on passed record", %{media_item: media_item} do + metadata_map = %{"foo" => "bar"} + + filepath = Helpers.compress_and_store_metadata_for(media_item, metadata_map) + + assert File.exists?(Path.dirname(filepath)) + end + + test "stores it as compressed JSON", %{media_item: media_item} do + metadata_map = %{"foo" => "bar"} + + filepath = Helpers.compress_and_store_metadata_for(media_item, metadata_map) + {:ok, json} = File.open(filepath, [:read, :compressed], &IO.read(&1, :all)) + + assert json == Phoenix.json_library().encode!(metadata_map) + end + end + + describe "read_compressed_metadata/1" do + test "returns the compressed and decoded metadata", %{media_item: media_item} do + metadata_map = %{"foo" => "bar"} + + filepath = Helpers.compress_and_store_metadata_for(media_item, metadata_map) + {:ok, decoded_json} = Helpers.read_compressed_metadata(filepath) + + assert decoded_json == metadata_map + end + end + + describe "download_and_store_thumbnail_for/2" do + setup do + # This tests that the HTTP endpoint is being called with every test + expect(HTTPClientMock, :get, fn url, _headers, _opts -> + assert url =~ "example.com" + + {:ok, "thumbnail data"} + end) + + metadata = %{"thumbnail" => "example.com/thumbnail.jpg"} + + {:ok, %{metadata: metadata}} + end + + test "returns the filepath", %{media_item: media_item, metadata: metadata} do + filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata) + + assert filepath =~ ~r{/media_items/#{media_item.id}/thumbnail.jpg} + end + + test "creates folder structure based on passed record", %{media_item: media_item, metadata: metadata} do + filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata) + + assert File.exists?(Path.dirname(filepath)) + end + + test "the filename and extension is based on the URL", %{media_item: media_item} do + metadata = %{"thumbnail" => "example.com/maxres.webp"} + filepath = Helpers.download_and_store_thumbnail_for(media_item, metadata) + + assert Path.basename(filepath) == "maxres.webp" + end + end +end diff --git a/test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs b/test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs index c29675d..b40fc70 100644 --- a/test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs +++ b/test/pinchflat/media_client/backends/yt_dlp/metadata_parser_test.exs @@ -1,5 +1,5 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do - use ExUnit.Case, async: true + use Pinchflat.DataCase alias Pinchflat.MediaClient.Backends.YtDlp.MetadataParser, as: Parser @@ -41,12 +41,6 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.MediaParserTest do assert is_binary(result.description) 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 describe "parse_for_media_item/1 when testing subtitle metadata" do diff --git a/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs b/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs index 4c67a44..8915639 100644 --- a/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs +++ b/test/pinchflat/media_client/backends/yt_dlp/output_path_builder_test.exs @@ -1,5 +1,5 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.OutputPathBuilderTest do - use ExUnit.Case, async: true + use Pinchflat.DataCase alias Pinchflat.Profiles.Options.YtDlp.OutputPathBuilder diff --git a/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs b/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs index b23bfcf..1eae92a 100644 --- a/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs +++ b/test/pinchflat/media_client/backends/yt_dlp/video_collection_test.exs @@ -1,5 +1,5 @@ defmodule Pinchflat.MediaClient.Backends.YtDlp.VideoCollectionTest do - use ExUnit.Case, async: true + use Pinchflat.DataCase import Mox import Pinchflat.SourcesFixtures diff --git a/test/pinchflat/media_client/video_downloader_test.exs b/test/pinchflat/media_client/video_downloader_test.exs index 65631bf..27013d4 100644 --- a/test/pinchflat/media_client/video_downloader_test.exs +++ b/test/pinchflat/media_client/video_downloader_test.exs @@ -14,6 +14,10 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do [:metadata, source: :media_profile] ) + stub(HTTPClientMock, :get, fn _url, _headers, _opts -> + {:ok, ""} + end) + {:ok, %{media_item: media_item}} end @@ -29,15 +33,16 @@ defmodule Pinchflat.MediaClient.VideoDownloaderTest do assert {:ok, _} = VideoDownloader.download_for_media_item(media_item) end - test "it saves the metadata to the database", %{media_item: media_item} do + test "it saves the metadata filepatha 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) + + assert updated_media_item.metadata.metadata_filepath =~ "media_items/#{media_item.id}/metadata.json.gz" + assert updated_media_item.metadata.thumbnail_filepath =~ "media_items/#{media_item.id}/maxresdefault.jpg" end test "errors are passed through", %{media_item: media_item} do diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index cb9ab77..fe92375 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -13,7 +13,9 @@ defmodule Pinchflat.MediaTest do describe "schema" do test "media_metadata is deleted when media_item is deleted" do - media_item = media_item_fixture(%{metadata: %{client_response: %{foo: "bar"}}}) + media_item = + media_item_fixture(%{metadata: %{metadata_filepath: "/metadata.json.gz", thumbnail_filepath: "/thumbnail.jpg"}}) + metadata = media_item.metadata assert {:ok, %MediaItem{}} = Media.delete_media_item(media_item) diff --git a/test/pinchflat/workers/video_download_worker_test.exs b/test/pinchflat/workers/video_download_worker_test.exs index 376b14b..0e043f1 100644 --- a/test/pinchflat/workers/video_download_worker_test.exs +++ b/test/pinchflat/workers/video_download_worker_test.exs @@ -16,6 +16,10 @@ defmodule Pinchflat.Workers.VideoDownloadWorkerTest do [:metadata, source: :media_profile] ) + stub(HTTPClientMock, :get, fn _url, _headers, _opts -> + {:ok, ""} + end) + {:ok, %{media_item: media_item}} end diff --git a/test/support/fixtures/media_fixtures.ex b/test/support/fixtures/media_fixtures.ex index 29cbec0..c327c46 100644 --- a/test/support/fixtures/media_fixtures.ex +++ b/test/support/fixtures/media_fixtures.ex @@ -31,18 +31,13 @@ defmodule Pinchflat.MediaFixtures do Generate a media_item with metadata. """ def media_item_with_metadata(attrs \\ %{}) 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) - merged_attrs = Map.merge(attrs, %{metadata: %{client_respinse: parsed_json}}) + merged_attrs = + Map.merge(attrs, %{ + metadata: %{ + metadata_filepath: Application.get_env(:pinchflat, :metadata_directory) <> "/metadata.json.gz", + thumbnail_filepath: Application.get_env(:pinchflat, :metadata_directory) <> "/thumbnail.jpg" + } + }) media_item_fixture(merged_attrs) end