From b7e7992f4d280529de8cc4b336fdd9bef142c571 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Tue, 26 Mar 2024 12:43:35 -0700 Subject: [PATCH] Many tests --- lib/pinchflat/boot/pre_job_startup_tasks.ex | 2 - lib/pinchflat/podcasts/podcast_helpers.ex | 50 +++++-- lib/pinchflat/podcasts/rss_feed_builder.ex | 51 ++++--- lib/pinchflat/utils/datetime_utils.ex | 17 +++ .../podcasts/podcast_controller.ex | 24 ++-- .../source_metadata_storage_worker_test.exs | 2 +- .../podcasts/podcast_helpers_test.exs | 64 +++++++++ .../podcasts/rss_feed_builder_test.exs | 132 ++++++++++++++++++ test/pinchflat/utils/datetime_utils_test.exs | 14 ++ .../controllers/podcast_controller_test.exs | 38 +++++ test/support/fixtures/media_fixtures.ex | 37 ++++- test/support/fixtures/sources_fixtures.ex | 29 +++- 12 files changed, 410 insertions(+), 50 deletions(-) create mode 100644 lib/pinchflat/utils/datetime_utils.ex create mode 100644 test/pinchflat/podcasts/podcast_helpers_test.exs create mode 100644 test/pinchflat/podcasts/rss_feed_builder_test.exs create mode 100644 test/pinchflat/utils/datetime_utils_test.exs create mode 100644 test/pinchflat_web/controllers/podcast_controller_test.exs diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index 83573c5..0e28d94 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -14,8 +14,6 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do alias Pinchflat.Repo alias Pinchflat.Settings - alias Pinchflat.Sources.Source - alias Pinchflat.Media.MediaItem alias Pinchflat.Filesystem.FilesystemHelpers def start_link(opts \\ []) do diff --git a/lib/pinchflat/podcasts/podcast_helpers.ex b/lib/pinchflat/podcasts/podcast_helpers.ex index a999dd1..33e328e 100644 --- a/lib/pinchflat/podcasts/podcast_helpers.ex +++ b/lib/pinchflat/podcasts/podcast_helpers.ex @@ -1,8 +1,27 @@ defmodule Pinchflat.Podcasts.PodcastHelpers do + @moduledoc """ + Methods for fetching postcast-related data from a source + or its media items + """ + alias Pinchflat.Repo alias Pinchflat.Media + alias Pinchflat.Metadata.MediaMetadata + alias Pinchflat.Metadata.SourceMetadata - # TODO: test + @doc """ + Returns a list of media items that have been downloaded to disk + and have been proven to still exist there. + + Useful for podcasts since we don't want to serve media that + has been deleted or moved, but it's also fairly generally useful + so I could see this being moved in the future. + + Options: + - limit: integer - the maximum number of media items to return + + Returns: [%MediaItem{}] + """ def persisted_media_items_for(source, opts \\ []) do limit = Keyword.get(opts, :limit, 500) @@ -11,8 +30,19 @@ defmodule Pinchflat.Podcasts.PodcastHelpers do |> Enum.filter(fn media_item -> File.exists?(media_item.media_filepath) end) end - # TODO: test - # Returns string or nil + @doc """ + Selects a cover image for a source based on the source's metadata + and the metadata of the media items associated with the source. Also + ensures images exist on disk. + + Only one media item should need to be returned since this is using the + internal metadata which, so long as the media_item was _downloaded_, should + be guaranteed to exist. + + Prefers the source's poster, then fanart, then the media item's thumbnail. + + Returns: {:ok, filepath} | {:error, :no_suitable_image} + """ def select_cover_image(source, media_items) do source_with_preloads = Repo.preload(source, :metadata) @@ -20,20 +50,24 @@ defmodule Pinchflat.Podcasts.PodcastHelpers do |> get_images_by_preference(media_items) |> Enum.reject(&is_nil(&1)) |> Enum.find(&File.exists?/1) + |> case do + nil -> {:error, :no_suitable_image} + filepath -> {:ok, filepath} + end end - def get_images_by_preference(source_with_preloads, []) do - source_metadata = source_with_preloads.metadata + defp get_images_by_preference(source_with_preloads, []) do + source_metadata = source_with_preloads.metadata || %SourceMetadata{} [ source_metadata.poster_filepath, - source_metadata.banner_filepath + source_metadata.fanart_filepath ] end - def get_images_by_preference(source_with_preloads, [media_item | _]) do + defp get_images_by_preference(source_with_preloads, [media_item | _]) do media_item_with_preloads = Repo.preload(media_item, :metadata) - media_item_metadata = media_item_with_preloads.metadata + media_item_metadata = media_item_with_preloads.metadata || %MediaMetadata{} source_images = get_images_by_preference(source_with_preloads, []) source_images ++ [media_item_metadata.thumbnail_filepath] diff --git a/lib/pinchflat/podcasts/rss_feed_builder.ex b/lib/pinchflat/podcasts/rss_feed_builder.ex index 0733ca1..72973f5 100644 --- a/lib/pinchflat/podcasts/rss_feed_builder.ex +++ b/lib/pinchflat/podcasts/rss_feed_builder.ex @@ -1,17 +1,34 @@ defmodule Pinchflat.Podcasts.RssFeedBuilder do + @moduledoc """ + Methods for building an RSS feed for a source and its media items. + """ + @datetime_format "%a, %d %b %Y %H:%M:%S %z" + alias Pinchflat.Utils.DatetimeUtils alias Pinchflat.Podcasts.PodcastHelpers alias PinchflatWeb.Router.Helpers, as: Routes - # TODO: test - # TODO: only MIs that are confirmed to exist on-disk should be provided - def build(source, media_items) do + @doc """ + Builds an RSS feed for a given source and its media items. + Only MediaItems that have been persisted will be included in the feed. + + ## Options: + - `:limit` - The maximum number of media items to include in the feed. Defaults to 300. + + Returns an XML document as a string. + """ + def build(source, opts \\ []) do + limit = Keyword.get(opts, :limit, 300) + + media_items = PodcastHelpers.persisted_media_items_for(source, limit: limit) build_source_xml(source, media_items) end defp build_source_xml(source, media_items) do media_item_xml = Enum.map(media_items, &build_media_item_xml(source, &1)) + # "caching" the image path since it requires some DB calls and is used twice + feed_image_path = feed_image_path(source, media_items) # Useful: resources: # - https://validator.w3.org/feed/#validate_by_input @@ -30,20 +47,20 @@ defmodule Pinchflat.Podcasts.RssFeedBuilder do TV & Film Generated by Pinchflat en-us - #{Calendar.strftime(DateTime.utc_now(), @datetime_format)} + #{Calendar.strftime(source.updated_at, @datetime_format)} #{Calendar.strftime(source.inserted_at, @datetime_format)} yes #{source.uuid} - #{feed_image_path(source, media_items)} + #{feed_image_path} #{source.custom_name} #{source.original_url} #{source.custom_name} #{source.custom_name} yes - + false @@ -65,8 +82,8 @@ defmodule Pinchflat.Podcasts.RssFeedBuilder do - + type="#{MIME.from_path(media_item.media_filepath)}" + /> #{source.custom_name} #{media_item.title} @@ -86,30 +103,22 @@ defmodule Pinchflat.Podcasts.RssFeedBuilder do end defp feed_image_path(source, media_items) do - image_path_on_disk = PodcastHelpers.select_cover_image(source, media_items) - - case image_path_on_disk do - nil -> + case PodcastHelpers.select_cover_image(source, media_items) do + {:error, _} -> "" - _ -> - extension = Path.extname(image_path_on_disk) + {:ok, filepath} -> + extension = Path.extname(filepath) Path.join(url_base(), "#{podcast_route(:feed_image, source.uuid)}#{extension}") end end defp generate_upload_date(media_item) do media_item.upload_date - |> Date.to_gregorian_days() - |> Kernel.*(86400) - |> DateTime.from_gregorian_seconds() + |> DatetimeUtils.date_to_datetime() |> Calendar.strftime(@datetime_format) end - defp determine_content_type(media_item) do - MIME.from_path(media_item.media_filepath) - end - defp podcast_route(action, params) do Routes.podcast_path(PinchflatWeb.Endpoint, action, params) end diff --git a/lib/pinchflat/utils/datetime_utils.ex b/lib/pinchflat/utils/datetime_utils.ex new file mode 100644 index 0000000..7fce7c6 --- /dev/null +++ b/lib/pinchflat/utils/datetime_utils.ex @@ -0,0 +1,17 @@ +defmodule Pinchflat.Utils.DatetimeUtils do + @moduledoc """ + Utility methods for working with dates and datetimes + """ + + @doc """ + Converts a Date to a DateTime + + Returns %DateTime{} + """ + def date_to_datetime(date) do + date + |> Date.to_gregorian_days() + |> Kernel.*(86_400) + |> DateTime.from_gregorian_seconds() + end +end diff --git a/lib/pinchflat_web/controllers/podcasts/podcast_controller.ex b/lib/pinchflat_web/controllers/podcasts/podcast_controller.ex index ac4760b..284851e 100644 --- a/lib/pinchflat_web/controllers/podcasts/podcast_controller.ex +++ b/lib/pinchflat_web/controllers/podcasts/podcast_controller.ex @@ -3,15 +3,13 @@ defmodule PinchflatWeb.Podcasts.PodcastController do alias Pinchflat.Repo alias Pinchflat.Media + alias Pinchflat.Sources.Source alias Pinchflat.Podcasts.RssFeedBuilder alias Pinchflat.Podcasts.PodcastHelpers - # TODO: test def rss_feed(conn, %{"uuid" => uuid}) do - # TODO: change this to UUID - source = Repo.get_by!(Source, id: uuid) - media_items = PodcastHelpers.persisted_media_items_for(source) - xml = RssFeedBuilder.build(source, media_items) + source = Repo.get_by!(Source, uuid: uuid) + xml = RssFeedBuilder.build(source, limit: 300) conn |> put_resp_content_type("application/rss+xml") @@ -19,21 +17,21 @@ defmodule PinchflatWeb.Podcasts.PodcastController do |> send_resp(200, xml) end - # TODO: test def feed_image(conn, %{"uuid" => uuid}) do source = Repo.get_by!(Source, uuid: uuid) # This provides a fallback image if the source has none. # We only need one since we're using the internal metadata image which # we know exists. media_items = Media.list_downloaded_media_items_for(source, limit: 1) - filepath = PodcastHelpers.select_cover_image(source, media_items) - if filepath && File.exists?(filepath) do - conn - |> put_resp_content_type(MIME.from_path(filepath)) - |> send_file(200, filepath) - else - send_resp(conn, 404, "File not found") + case PodcastHelpers.select_cover_image(source, media_items) do + {:error, _} -> + send_resp(conn, 404, "Image not found") + + {:ok, filepath} -> + conn + |> put_resp_content_type(MIME.from_path(filepath)) + |> send_file(200, filepath) end end end diff --git a/test/pinchflat/metadata/source_metadata_storage_worker_test.exs b/test/pinchflat/metadata/source_metadata_storage_worker_test.exs index 878f8fd..06e6959 100644 --- a/test/pinchflat/metadata/source_metadata_storage_worker_test.exs +++ b/test/pinchflat/metadata/source_metadata_storage_worker_test.exs @@ -57,7 +57,7 @@ defmodule Pinchflat.Metadata.SourceMetadataStorageWorkerTest do _url, _opts, ot when ot == @metadata_ot -> {:ok, render_metadata(:channel_source_metadata)} end) - source = source_fixture() + source = source_fixture(%{description: nil}) refute source.description perform_job(SourceMetadataStorageWorker, %{id: source.id}) diff --git a/test/pinchflat/podcasts/podcast_helpers_test.exs b/test/pinchflat/podcasts/podcast_helpers_test.exs new file mode 100644 index 0000000..1d21d05 --- /dev/null +++ b/test/pinchflat/podcasts/podcast_helpers_test.exs @@ -0,0 +1,64 @@ +defmodule Pinchflat.Podcasts.PodcastHelpersTest do + use Pinchflat.DataCase + + import Pinchflat.SourcesFixtures + import Pinchflat.MediaFixtures + + alias Pinchflat.Podcasts.PodcastHelpers + + describe "persisted_media_items_for/2" do + test "returns media items with files that exist on-disk" do + source = source_fixture() + good_media = media_item_with_attachments(%{source_id: source.id}) + _bad_media = media_item_fixture(%{source_id: source.id, media_filepath: "/tmp/existing_file.mp3"}) + + assert [persisted_media] = PodcastHelpers.persisted_media_items_for(source) + assert persisted_media.id == good_media.id + end + + test "lets you specify a limit" do + source = source_fixture() + _good_media = media_item_with_attachments(%{source_id: source.id}) + + assert [] = PodcastHelpers.persisted_media_items_for(source, limit: 0) + end + end + + describe "select_cover_image/2" do + test "returns a source's poster, if present" do + source = source_with_metadata_attachments() + + {:ok, res} = PodcastHelpers.select_cover_image(source, []) + + assert res == source.metadata.poster_filepath + end + + test "falls back to a source's fanart, if present" do + source = source_with_metadata_attachments() + + File.rm(source.metadata.poster_filepath) + + {:ok, res} = PodcastHelpers.select_cover_image(source, []) + + assert res == source.metadata.fanart_filepath + end + + test "falls back to a media item's thumbnail, if present" do + source = source_with_metadata_attachments() + media_item = media_item_with_metadata_attachments(%{source_id: source.id}) + + File.rm(source.metadata.poster_filepath) + File.rm(source.metadata.fanart_filepath) + + {:ok, res} = PodcastHelpers.select_cover_image(source, [media_item]) + + assert res == media_item.metadata.thumbnail_filepath + end + + test "returns error if no artwork can be found" do + source = source_fixture() + + assert PodcastHelpers.select_cover_image(source, []) == {:error, :no_suitable_image} + end + end +end diff --git a/test/pinchflat/podcasts/rss_feed_builder_test.exs b/test/pinchflat/podcasts/rss_feed_builder_test.exs new file mode 100644 index 0000000..6e24c42 --- /dev/null +++ b/test/pinchflat/podcasts/rss_feed_builder_test.exs @@ -0,0 +1,132 @@ +defmodule Pinchflat.Podcasts.RssFeedBuilderTest do + use Pinchflat.DataCase + + import Pinchflat.MediaFixtures + import Pinchflat.SourcesFixtures + + alias Pinchflat.Podcasts.RssFeedBuilder + + @datetime_format "%a, %d %b %Y %H:%M:%S %z" + + setup do + source = source_fixture() + + {:ok, source: source} + end + + describe "build/2" do + test "returns an XML document", %{source: source} do + res = RssFeedBuilder.build(source) + + assert String.contains?(res, ~s()) + end + + test "can optionally apply a limit to media items", %{source: source} do + good_media = media_item_with_attachments(%{source_id: source.id}) + + res = RssFeedBuilder.build(source, limit: 0) + + refute String.contains?(res, ~s(#{good_media.title})) + end + end + + describe "build/2 when testing source XML" do + test "returns XML for static source attributes", %{source: source} do + res = RssFeedBuilder.build(source) + + assert String.contains?(res, ~s(#{source.custom_name})) + assert String.contains?(res, ~s(#{source.original_url})) + assert String.contains?(res, ~s(#{source.description})) + assert String.contains?(res, ~s(#{source.custom_name})) + assert String.contains?(res, ~s(#{source.custom_name})) + assert String.contains?(res, ~s(#{source.description})) + assert String.contains?(res, ~s(#{source.uuid})) + end + + test "returns the lastBuildDate and pubDate based off the source's timestamps", %{source: source} do + res = RssFeedBuilder.build(source) + + assert String.contains?(res, ~s(#{format_date(source.updated_at)})) + assert String.contains?(res, ~s(#{format_date(source.inserted_at)})) + end + + test "returns a self-link", %{source: source} do + res = RssFeedBuilder.build(source) + + assert String.contains?( + res, + ~s() + ) + end + + test "returns a link to the feed image" do + source = source_with_metadata_attachments() + + res = RssFeedBuilder.build(source) + [_before, image_block, _after] = String.split(res, ~r()) + + assert String.contains?(image_block, ~s(http://localhost:4008/sources/#{source.uuid}/feed_image.jpg)) + assert String.contains?(image_block, ~s(#{source.custom_name})) + assert String.contains?(image_block, ~s(#{source.original_url})) + + assert String.contains?( + res, + ~s() + ) + end + end + + describe "build/2 when testing media XML" do + test "only includes media persisted to disk", %{source: source} do + good_media = media_item_with_attachments(%{source_id: source.id}) + bad_media = media_item_fixture(%{source_id: source.id, media_filepath: "/tmp/existing_file.mp3"}) + pending_media = media_item_fixture(%{source_id: source.id, media_filepath: nil}) + + res = RssFeedBuilder.build(source) + + assert String.contains?(res, ~s(#{good_media.title})) + refute String.contains?(res, ~s(#{bad_media.title})) + refute String.contains?(res, ~s(#{pending_media.title})) + end + + test "returns XML for static media attributes", %{source: source} do + media_item = media_item_with_attachments(%{source_id: source.id}) + + res = RssFeedBuilder.build(source) + [_before, item_xml, _after] = String.split(res, ~r()) + + assert String.contains?(item_xml, ~s(#{media_item.uuid})) + assert String.contains?(item_xml, ~s(#{media_item.title})) + assert String.contains?(item_xml, ~s(#{media_item.original_url})) + assert String.contains?(item_xml, ~s(#{media_item.description})) + assert String.contains?(item_xml, ~s(#{source.custom_name})) + assert String.contains?(item_xml, ~s(#{media_item.title})) + assert String.contains?(item_xml, ~s()) + end + + test "returns pubDate based off the media's upload_date", %{source: source} do + media_item_with_attachments(%{source_id: source.id, upload_date: ~D[2020-01-01]}) + + res = RssFeedBuilder.build(source) + [_before, item_xml, _after] = String.split(res, ~r()) + + assert String.contains?(item_xml, ~s(Wed, 01 Jan 2020 00:00:00 +0000)) + end + + test "returns an enclosure tag with the media's stream URL", %{source: source} do + media_item = media_item_with_attachments(%{source_id: source.id, media_size_bytes: 1234}) + + res = RssFeedBuilder.build(source) + [_before, item_xml, _after] = String.split(res, ~r()) + + assert String.contains?(item_xml, ~s( ".xml") + + assert conn.status == 200 + assert {"content-type", "application/rss+xml; charset=utf-8"} in conn.resp_headers + assert {"content-disposition", "inline"} in conn.resp_headers + end + end + + describe "feed_image" do + test "returns a feed image if one can be found", %{conn: conn} do + source = source_with_metadata_attachments() + + conn = get(conn, ~p"/sources/#{source.uuid}/feed_image" <> ".jpg") + + assert conn.status == 200 + assert {"content-type", "image/jpeg; charset=utf-8"} in conn.resp_headers + assert conn.resp_body == File.read!(source.metadata.poster_filepath) + end + + test "returns 404 if an image cannot be found", %{conn: conn} do + source = source_fixture() + + conn = get(conn, ~p"/sources/#{source.uuid}/feed_image" <> ".jpg") + + assert conn.status == 404 + assert conn.resp_body == "Image not found" + end + end +end diff --git a/test/support/fixtures/media_fixtures.ex b/test/support/fixtures/media_fixtures.ex index d9c208c..f724039 100644 --- a/test/support/fixtures/media_fixtures.ex +++ b/test/support/fixtures/media_fixtures.ex @@ -5,6 +5,7 @@ defmodule Pinchflat.MediaFixtures do """ alias Pinchflat.SourcesFixtures + alias Pinchflat.Filesystem.FilesystemHelpers @doc """ Generate a media_item. @@ -44,6 +45,27 @@ defmodule Pinchflat.MediaFixtures do media_item_fixture(merged_attrs) end + def media_item_with_metadata_attachments(attrs \\ %{}) do + metadata_dir = + Path.join(Application.get_env(:pinchflat, :metadata_directory), "#{:rand.uniform(1_000_000)}") + + json_gz_filepath = Path.join(metadata_dir, "metadata.json.gz") + thumbnail_filepath = Path.join(metadata_dir, "thumbnail.jpg") + + FilesystemHelpers.cp_p!(media_metadata_filepath_fixture(), json_gz_filepath) + FilesystemHelpers.cp_p!(thumbnail_filepath_fixture(), thumbnail_filepath) + + merged_attrs = + Map.merge(attrs, %{ + metadata: %{ + metadata_filepath: json_gz_filepath, + thumbnail_filepath: thumbnail_filepath + } + }) + + media_item_with_attachments(merged_attrs) + end + def media_item_with_attachments(attrs \\ %{}) do stored_media_filepath = Path.join([ @@ -52,10 +74,7 @@ defmodule Pinchflat.MediaFixtures do "#{:rand.uniform(1_000_000)}_media.mp4" ]) - fixture_media_filepath = media_filepath_fixture() - - :ok = File.mkdir_p(Path.dirname(stored_media_filepath)) - :ok = File.cp(fixture_media_filepath, stored_media_filepath) + FilesystemHelpers.cp_p!(media_filepath_fixture(), stored_media_filepath) merged_attrs = Map.merge(attrs, %{media_filepath: stored_media_filepath}) media_item_fixture(merged_attrs) @@ -105,4 +124,14 @@ defmodule Pinchflat.MediaFixtures do "example.info.json" ]) end + + def media_metadata_filepath_fixture do + Path.join([ + File.cwd!(), + "test", + "support", + "files", + "media_metadata.json" + ]) + end end diff --git a/test/support/fixtures/sources_fixtures.ex b/test/support/fixtures/sources_fixtures.ex index 0db2e78..55375e5 100644 --- a/test/support/fixtures/sources_fixtures.ex +++ b/test/support/fixtures/sources_fixtures.ex @@ -5,8 +5,10 @@ defmodule Pinchflat.SourcesFixtures do """ alias Pinchflat.Repo - alias Pinchflat.ProfilesFixtures + alias Pinchflat.MediaFixtures alias Pinchflat.Sources.Source + alias Pinchflat.ProfilesFixtures + alias Pinchflat.Filesystem.FilesystemHelpers @doc """ Generate a source. @@ -22,6 +24,7 @@ defmodule Pinchflat.SourcesFixtures do collection_id: Base.encode16(:crypto.hash(:md5, "#{:rand.uniform(1_000_000)}")), collection_type: "channel", custom_name: "Cool and good internal name!", + description: "This is a description", original_url: "https://www.youtube.com/channel/#{Faker.String.base64(12)}", media_profile_id: ProfilesFixtures.media_profile_fixture().id, index_frequency_minutes: 60 @@ -48,6 +51,30 @@ defmodule Pinchflat.SourcesFixtures do source_fixture(merged_attrs) end + def source_with_metadata_attachments(attrs \\ %{}) do + metadata_dir = + Path.join(Application.get_env(:pinchflat, :metadata_directory), "#{:rand.uniform(1_000_000)}") + + json_gz_filepath = Path.join(metadata_dir, "metadata.json.gz") + poster_filepath = Path.join(metadata_dir, "poster.jpg") + fanart_filepath = Path.join(metadata_dir, "fanart.jpg") + + FilesystemHelpers.cp_p!(MediaFixtures.media_metadata_filepath_fixture(), json_gz_filepath) + FilesystemHelpers.cp_p!(MediaFixtures.thumbnail_filepath_fixture(), poster_filepath) + FilesystemHelpers.cp_p!(MediaFixtures.thumbnail_filepath_fixture(), fanart_filepath) + + merged_attrs = + Map.merge(attrs, %{ + metadata: %{ + metadata_filepath: json_gz_filepath, + poster_filepath: poster_filepath, + fanart_filepath: fanart_filepath + } + }) + + source_fixture(merged_attrs) + end + def source_attributes_return_fixture do source_attributes = [ %{