From 80689517a12cdaa0e59c1750ccf0543633692012 Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Fri, 16 Feb 2024 09:28:59 -0800 Subject: [PATCH] Updated search functions and tests to work with sqlite --- config/config.exs | 1 + lib/pinchflat/media.ex | 41 ++++++++++--------- lib/pinchflat/media/media_item.ex | 2 + .../media/media_items_search_index.ex | 16 ++++++++ test/pinchflat/media_test.exs | 8 ++++ test/support/conn_case.ex | 1 - test/support/data_case.ex | 14 ------- 7 files changed, 48 insertions(+), 35 deletions(-) create mode 100644 lib/pinchflat/media/media_items_search_index.ex diff --git a/config/config.exs b/config/config.exs index 91b3920..1936df3 100644 --- a/config/config.exs +++ b/config/config.exs @@ -29,6 +29,7 @@ config :pinchflat, PinchflatWeb.Endpoint, live_view: [signing_salt: "/t5878kO"] config :pinchflat, Oban, + engine: Oban.Engines.Lite, repo: Pinchflat.Repo, # Keep old jobs for 30 days for display in the UI plugins: [{Oban.Plugins.Pruner, max_age: 30 * 24 * 60 * 60}], diff --git a/lib/pinchflat/media.ex b/lib/pinchflat/media.ex index 14ae494..cdd3fd6 100644 --- a/lib/pinchflat/media.ex +++ b/lib/pinchflat/media.ex @@ -51,31 +51,29 @@ defmodule Pinchflat.Media do Returns a list of media_items that match the search term. Adds a `matching_search_term` virtual field to the result set. + Has explit handling for blank search terms because SQLite doesn't like empty MATCH clauses. + Returns [%MediaItem{}, ...]. """ - def search(search_term, opts \\ []) do + def search(_search_term, _opts \\ []) + def search("", _opts), do: [] + def search(nil, _opts), do: [] + + def search(search_term, opts) do limit = Keyword.get(opts, :limit, 50) from(mi in MediaItem, - where: fragment("searchable @@ websearch_to_tsquery(?)", ^search_term), + join: mi_search_index in assoc(mi, :media_items_search_index), + where: fragment("media_items_search_index MATCH ?", ^search_term), select_merge: %{ matching_search_term: - fragment( - """ - ts_headline( - 'english', - CONCAT(title, ' ', description), - websearch_to_tsquery(?), - 'StartSel=[PF_HIGHLIGHT],StopSel=[/PF_HIGHLIGHT]' - ) - """, - ^search_term - ) - }, - order_by: { - :desc, - fragment("ts_rank_cd(searchable, websearch_to_tsquery(?), 0)", ^search_term) + fragment(""" + snippet(media_items_search_index, 0, '[PF_HIGHLIGHT]', '[/PF_HIGHLIGHT]', '...', 20) || + ' ' || + snippet(media_items_search_index, 1, '[PF_HIGHLIGHT]', '[/PF_HIGHLIGHT]', '...', 20) + """) }, + order_by: [desc: fragment("rank")], limit: ^limit ) |> Repo.all() @@ -174,7 +172,10 @@ defmodule Pinchflat.Media do Enum.reduce(mapped_struct, dynamic(true), fn attr, dynamic -> case {attr, media_profile} do {{:shorts_behaviour, :only}, %{livestream_behaviour: :only}} -> - dynamic([mi], ^dynamic and (mi.livestream == true or fragment("? ILIKE ?", mi.original_url, "%/shorts/%"))) + dynamic( + [mi], + ^dynamic and (mi.livestream == true or fragment("LOWER(?) LIKE LOWER(?)", mi.original_url, "%/shorts/%")) + ) # Technically redundant, but makes the other clauses easier to parse # (redundant because this condition is the same as the condition above, just flipped) @@ -183,7 +184,7 @@ defmodule Pinchflat.Media do {{:shorts_behaviour, :only}, _} -> # return records with /shorts/ in the original_url - dynamic([mi], ^dynamic and fragment("? ILIKE ?", mi.original_url, "%/shorts/%")) + dynamic([mi], ^dynamic and fragment("LOWER(?) LIKE LOWER(?)", mi.original_url, "%/shorts/%")) {{:livestream_behaviour, :only}, _} -> # return records with livestream: true @@ -191,7 +192,7 @@ defmodule Pinchflat.Media do {{:shorts_behaviour, :exclude}, %{livestream_behaviour: lb}} when lb != :only -> # return records without /shorts/ in the original_url - dynamic([mi], ^dynamic and fragment("? NOT ILIKE ?", mi.original_url, "%/shorts/%")) + dynamic([mi], ^dynamic and fragment("LOWER(?) NOT LIKE LOWER(?)", mi.original_url, "%/shorts/%")) {{:livestream_behaviour, :exclude}, %{shorts_behaviour: sb}} when sb != :only -> # return records with livestream: false diff --git a/lib/pinchflat/media/media_item.ex b/lib/pinchflat/media/media_item.ex index b43a1a2..c9c99d6 100644 --- a/lib/pinchflat/media/media_item.ex +++ b/lib/pinchflat/media/media_item.ex @@ -9,6 +9,7 @@ defmodule Pinchflat.Media.MediaItem do alias Pinchflat.Tasks.Task alias Pinchflat.MediaSource.Source alias Pinchflat.Media.MediaMetadata + alias Pinchflat.Media.MediaItemSearchIndex @allowed_fields ~w( title @@ -46,6 +47,7 @@ defmodule Pinchflat.Media.MediaItem do belongs_to :source, Source has_one :metadata, MediaMetadata, on_replace: :update + has_one :media_items_search_index, MediaItemSearchIndex, foreign_key: :id has_many :tasks, Task diff --git a/lib/pinchflat/media/media_items_search_index.ex b/lib/pinchflat/media/media_items_search_index.ex new file mode 100644 index 0000000..c55d787 --- /dev/null +++ b/lib/pinchflat/media/media_items_search_index.ex @@ -0,0 +1,16 @@ +defmodule Pinchflat.Media.MediaItemSearchIndex do + @moduledoc """ + The MediaItem fts5 search index. Not made to be directly interacted with, + but I figured it'd be better to have it in-app so it's not a mystery. + """ + + use Ecto.Schema + + @primary_key {:id, :id, autogenerate: true, source: :rowid} + schema "media_items_search_index" do + field :title, :string + field :description, :string + + field :rank, :float, virtual: true + end +end diff --git a/test/pinchflat/media_test.exs b/test/pinchflat/media_test.exs index 77d267d..d116ddf 100644 --- a/test/pinchflat/media_test.exs +++ b/test/pinchflat/media_test.exs @@ -212,6 +212,14 @@ defmodule Pinchflat.MediaTest do assert [_] = Media.search("dog", limit: 1) end + + test "returns an empty list when the search term is blank" do + assert [] = Media.search("") + end + + test "returns an empty list when the search term is nil" do + assert [] = Media.search(nil) + end end describe "get_media_item!/1" do diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 3aa5fce..ff478a0 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -34,7 +34,6 @@ defmodule PinchflatWeb.ConnCase do setup tags do Pinchflat.DataCase.setup_sandbox(tags) - Pinchflat.DataCase.setup_temp_filepaths() {:ok, conn: Phoenix.ConnTest.build_conn()} end end diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 5bf7f10..d217749 100644 --- a/test/support/data_case.ex +++ b/test/support/data_case.ex @@ -32,20 +32,6 @@ defmodule Pinchflat.DataCase do setup tags do Pinchflat.DataCase.setup_sandbox(tags) - Pinchflat.DataCase.setup_temp_filepaths() - :ok - end - - @doc """ - Sets up the temp filepaths for the media and metadata directories. - """ - def setup_temp_filepaths do - File.rm_rf!(Application.get_env(:pinchflat, :media_directory)) - File.rm_rf!(Application.get_env(:pinchflat, :metadata_directory)) - - File.mkdir_p!(Application.get_env(:pinchflat, :media_directory)) - File.mkdir_p!(Application.get_env(:pinchflat, :metadata_directory)) - :ok end