diff --git a/lib/philomena/activities.ex b/lib/philomena/activities.ex index 37fb26aeb..50b31ac09 100644 --- a/lib/philomena/activities.ex +++ b/lib/philomena/activities.ex @@ -47,7 +47,11 @@ defmodule Philomena.Activities do defp watched_definition(%Actor{} = actor, scope) do with {:ok, {definition, _tags}} <- - ImageSearch.search_string(actor, scope, "my:watched", + ImageSearch.search_string( + actor, + scope, + ImageSearch.default_sort(), + "my:watched", pagination: %{scope.pagination | page_number: 1} ) do {:ok, definition} @@ -62,8 +66,8 @@ defmodule Philomena.Activities do ImageSearch.query( actor, scope, + ImageSearch.default_sort(), %{range: %{first_seen_at: %{gt: "now-3d"}}}, - sorts: &%{query: &1, sorts: [%{wilson_score: :desc}, %{first_seen_at: :desc}]}, pagination: %{page_number: :rand.uniform(6), page_size: 4} ) @@ -78,11 +82,10 @@ defmodule Philomena.Activities do case watched_definition(actor, scope) do {:ok, watched_definition} -> - {:ok, - {images_definition, top_scoring_definition, comments_definition, watched_definition}} + {images_definition, top_scoring_definition, comments_definition, watched_definition} _error -> - {:ok, {images_definition, top_scoring_definition, comments_definition, nil}} + {images_definition, top_scoring_definition, comments_definition, nil} end end @@ -154,8 +157,8 @@ defmodule Philomena.Activities do show_nsfw_channels? ) when is_boolean(show_nsfw_channels?) do - with :ok <- authorize(actor, :show, FrontPage), - {:ok, definitions} <- search_definitions(actor, scope, filter) do + with :ok <- authorize(actor, :show, FrontPage) do + definitions = search_definitions(actor, scope, filter) {:ok, assemble_front_page(actor, scope, definitions, show_nsfw_channels?)} end end diff --git a/lib/philomena/galleries.ex b/lib/philomena/galleries.ex index f0c8c6459..1c7298010 100644 --- a/lib/philomena/galleries.ex +++ b/lib/philomena/galleries.ex @@ -194,13 +194,13 @@ defmodule Philomena.Galleries do {:ok, nil} end - defp image_sort_direction(%{order_position_asc: true}), do: "asc" - defp image_sort_direction(_gallery), do: "desc" + defp image_sort_direction(%{order_position_asc: true}), do: :asc + defp image_sort_direction(_gallery), do: :desc - defp put_query(list, name, actor, scope, query, pagination) do + defp put_query(list, name, {actor, scope, sort, query}, pagination) do if pagination.page_number > 0 do {:ok, {definition, _tags}} = - ImageSearch.search_string(actor, scope, query, pagination: pagination) + ImageSearch.search_string(actor, scope, sort, query, pagination: pagination) Keyword.put(list, name, {definition, preload(Image, [:sources, tags: :aliases])}) else @@ -210,7 +210,8 @@ defmodule Philomena.Galleries do defp reorder_window(%Actor{} = actor, %Scope{} = scope, %Gallery{} = gallery) do query = "gallery_id:#{gallery.id}" - scope = %{scope | sf: query, sd: image_sort_direction(gallery)} + sort = ImageSearch.gallery_sort(gallery.id, image_sort_direction(gallery)) + params = {actor, scope, sort, query} limit = scope.pagination.page_size offset = (scope.pagination.page_number - 1) * limit @@ -219,9 +220,9 @@ defmodule Philomena.Galleries do # with an empty page is inserted if no search was performed. [] - |> put_query(:images, actor, scope, query, scope.pagination) - |> put_query(:leading, actor, scope, query, %{page_number: offset - 1, page_size: 1}) - |> put_query(:trailing, actor, scope, query, %{page_number: offset + limit, page_size: 1}) + |> put_query(:images, params, scope.pagination) + |> put_query(:leading, params, %{page_number: offset - 1, page_size: 1}) + |> put_query(:trailing, params, %{page_number: offset + limit, page_size: 1}) |> Search.msearch_records_with_hits() |> Map.put_new(:leading, %Scrivener.Page{}) end diff --git a/lib/philomena/images.ex b/lib/philomena/images.ex index ba4170a3a..00c23dbf5 100644 --- a/lib/philomena/images.ex +++ b/lib/philomena/images.ex @@ -100,8 +100,7 @@ defmodule Philomena.Images do ) end - defp custom_ordering?(%{sf: sf}) when sf not in [nil, "id", "first_seen_at"], do: true - defp custom_ordering?(_scope), do: false + defp custom_ordering?(%{sf: sf}), do: sf not in [:id, {:field, :first_seen_at}] defp maybe_jump_to_last_page( %Actor{ @@ -802,8 +801,9 @@ defmodule Philomena.Images do {:ok, %{images: Scrivener.Page.t(), tags: [Tag.t()]}} | {:error, String.t()} def query_images(%Actor{} = actor, scope, opts \\ []) do with :ok <- authorize(actor, :index, Image), + sort = ImageSearch.scope_sort(scope), {:ok, {definition, tags}} <- - ImageSearch.search_string(actor, scope, scope.q) do + ImageSearch.search_string(actor, scope, sort, scope.q) do preload = Keyword.get(opts, :preload, [:sources, tags: :aliases]) hits = Keyword.get(opts, :hits, custom_ordering?(scope)) @@ -851,7 +851,8 @@ defmodule Philomena.Images do {:ok, Scrivener.Page.t()} | {:error, :unauthorized | String.t()} def list_watched_images(%Actor{} = actor, scope) do with :ok <- authorize(actor, :index_watched, Image), - {:ok, {definition, _tags}} <- ImageSearch.search_string(actor, scope, "my:watched") do + sort = ImageSearch.scope_sort(scope), + {:ok, {definition, _tags}} <- ImageSearch.search_string(actor, scope, sort, "my:watched") do {:ok, ImageSearch.execute(definition)} end end @@ -994,7 +995,7 @@ defmodule Philomena.Images do @doc group: "Browsing and discovery" @doc """ Returns the 1-based page number on which the image `image_id` - names appears when all images are listed by descending id, on behalf of + names appears when all images are listed by the default sort, on behalf of `actor`. Loading and authorization follow `find_consecutive_image/3`. @@ -1012,7 +1013,13 @@ defmodule Philomena.Images do pagination = %{scope.pagination | page_number: 1} {definition, _tags} = - ImageSearch.query(actor, scope, %{range: %{id: %{gt: image.id}}}, pagination: pagination) + ImageSearch.query( + actor, + scope, + ImageSearch.default_sort(), + %{range: %{id: %{gt: image.id}}}, + pagination: pagination + ) images = ImageSearch.execute(definition, preload: []) @@ -1079,8 +1086,8 @@ defmodule Philomena.Images do ImageSearch.query( actor, scope, + ImageSearch.relevance_sort(), query, - sorts: &%{query: &1, sorts: [%{_score: :desc}]}, pagination: %{scope.pagination | page_number: 1} ) @@ -1111,9 +1118,9 @@ defmodule Philomena.Images do ImageSearch.search_string( actor, scope, + ImageSearch.random_sort(), scope.q || "*", - pagination: %{page_size: 1}, - sorts: &ImageSearch.parse_sort(%{"sf" => "random"}, &1) + pagination: %{page_size: 1} ) do definition |> ImageSearch.execute(preload: []) diff --git a/lib/philomena/images/search.ex b/lib/philomena/images/search.ex index fcf206e9c..09c80c9d7 100644 --- a/lib/philomena/images/search.ex +++ b/lib/philomena/images/search.ex @@ -24,37 +24,9 @@ defmodule Philomena.Images.Search do import Ecto.Query import Philomena.Authorization, only: [authorize: 3] - @allowed_sort_fields ~W( - id - updated_at - first_seen_at - aspect_ratio - faves - downvotes - upvotes - width - height - score - comment_count - tag_count - wilson_score - pixels - size - duration - hides - ) - - @order_for_dir %{ - "next" => %{"asc" => "asc", "desc" => "desc"}, - "prev" => %{"asc" => "desc", "desc" => "asc"} - } - @type definition :: Search.search_definition() @type query_result :: {definition(), [Tag.t()]} - @type option :: - {:pagination, map()} - | {:sorts, (map() -> %{query: map(), sorts: list()})} - | {:tag_names, [String.t()]} + @type option :: {:pagination, map()} | {:tag_names, [String.t()]} @doc """ Builds the default image listing query for the viewer. @@ -78,7 +50,7 @@ defmodule Philomena.Images.Search do }, else: %{match_all: %{}} - query(actor, scope, body, options) + query(actor, scope, default_sort(), body, options) end @doc """ @@ -87,13 +59,19 @@ defmodule Philomena.Images.Search do Returns `{:ok, {definition, tags}}`, or the compiler's `{:error, msg}` for a malformed query. """ - @spec search_string(Actor.t(), Scope.t(), String.t() | nil, [option()]) :: + @spec search_string(Actor.t(), Scope.t(), term(), String.t() | nil, [option()]) :: {:ok, query_result()} | {:error, String.t()} # sobelow_skip ["SQL.Query"] - def search_string(actor, scope, search_string, options \\ []) do + def search_string( + %Actor{} = actor, + %Scope{} = scope, + sort, + search_string, + options \\ [] + ) do case Query.compile_with_tag_names(search_string, user: actor.user) do {:ok, %{query: tree, tag_names: tag_names}} -> - {:ok, query(actor, scope, tree, Keyword.put(options, :tag_names, tag_names))} + {:ok, query(actor, scope, sort, tree, Keyword.put(options, :tag_names, tag_names))} {:error, _message} = error -> error @@ -101,23 +79,18 @@ defmodule Philomena.Images.Search do end @doc """ - Builds a query definition from an already-compiled query body. + Builds a query definition from an already-compiled query body and sort. - Options: `:pagination` overrides the scope's window; `:sorts` replaces the - parameter-driven sort with a custom `body -> %{query:, sorts:}` function. + The `:pagination` option overrides the scope's window. Returns `{definition, tags}`. """ - @spec query(Actor.t(), Scope.t(), map(), [option()]) :: query_result() - def query(actor, scope, body, options \\ []) do + @spec query(Actor.t(), Scope.t(), term(), map(), [option()]) :: query_result() + def query(%Actor{} = actor, %Scope{} = scope, sort, body, options \\ []) do pagination = Keyword.get(options, :pagination, scope.pagination) - sorts = Keyword.get(options, :sorts, &parse_sort(scope, &1)) - tags = options |> Keyword.get(:tag_names, []) |> load_tags() - filters = create_filters(actor, scope) - - %{query: query, sorts: sort} = sorts.(body) + {query, sort} = compile_sort(sort, body) definition = Search.search_definition( @@ -157,25 +130,43 @@ defmodule Philomena.Images.Search do end @doc """ - Maps the "sf"/"sd" parameters onto a sort order for `query_body`. - - Unlisted or missing fields sort by `first_seen_at`; `random`/`random:seed` - wrap the query in a seeded `function_score`; `gallery_id:n` sorts by the - image's position in that gallery. + Returns the default sort. Used when no sort is explicitly specified. + """ + @spec default_sort() :: term() + def default_sort do + {{:field, :first_seen_at}, :desc} + end - Returns `%{query:, sorts:}`. + @doc """ + Returns a random sort. """ - @spec parse_sort(map(), map()) :: %{query: map(), sorts: list()} - def parse_sort(%Scope{} = scope, query_body) do - sd = parse_sd(%{"sd" => scope.sd}) + @spec random_sort() :: term() + def random_sort do + {{:random, :rand.uniform(4_294_967_296)}, :desc} + end - parse_sf(%{"sf" => scope.sf}, sd, query_body) + @doc """ + Returns the sort for a gallery's images in position order. + """ + @spec gallery_sort(integer(), :asc | :desc) :: term() + def gallery_sort(gallery_id, direction) do + {{:gallery, gallery_id}, direction} end - def parse_sort(params, query_body) when is_map(params) do - sd = parse_sd(params) + @doc """ + Returns a sort in search relevance order. + """ + @spec relevance_sort() :: term() + def relevance_sort() do + {{:field, :_score}, :desc} + end - parse_sf(params, sd, query_body) + @doc """ + Returns the exact sort provided by the search scope. + """ + @spec scope_sort(Scope.t()) :: term() + def scope_sort(%Scope{sf: sf, sd: sd}) do + {sf, sd} end @doc """ @@ -183,47 +174,39 @@ defmodule Philomena.Images.Search do describe, for prev/next navigation. `compiled_query` is the compiled body of the listing's search query; - `scope.rel` selects the direction and `scope.sort` - carries the sort cursor of the current image, when present. + `scope.rel` selects the direction and `scope.sort` carries the sort cursor + of the current image, when present. - Returns the `{image, hit}` pair for the neighbouring image, or `nil` at - the end of the sequence. + Returns the `{image, hit}` pair for the neighboring image, or `nil` + when an error occurred or the end of the sequence was reached. """ @spec find_consecutive(Actor.t(), Scope.t(), Image.t(), map()) :: {Image.t(), map()} | nil - def find_consecutive(actor, scope, image, compiled_query) do - sf = scope.sf || "first_seen_at" - - %{query: compiled_query, sorts: sorts} = parse_sort(scope, compiled_query) - - sorts = - sorts - |> Enum.flat_map(&Enum.to_list/1) - |> Enum.map(&apply_direction(&1, scope.rel)) - - search_after = - scope.sort - |> permit_list() - |> Enum.flat_map(&permit_value/1) - |> default_cursors(sf, image) - - maybe_search_after( - Image, - %{ - query: %{ + def find_consecutive(%Actor{} = actor, %Scope{} = scope, %Image{} = image, compiled_query) do + scope = apply_reverse_navigation(scope) + + consecutive = + with {:ok, cursor} <- cursor_or_default(image, scope.sf, scope.sort) do + query = %{ bool: %{ must: compiled_query, must_not: [%{term: %{id: image.id}} | create_filters(actor, scope)] } - }, - sort: sorts, - search_after: search_after - }, - %{page_size: 1}, - Image, - length(sorts) == length(search_after) - ) - |> Enum.to_list() - |> case do + } + + {query, sorts} = compile_sort(scope_sort(scope), query) + + Image + |> Search.search_definition( + %{query: query, sort: sorts, search_after: cursor}, + %{page_size: 1} + ) + |> Search.search_records_with_hits(Image) + |> Enum.to_list() + else + _ -> [] + end + + case consecutive do [] -> nil [next_image] -> next_image end @@ -307,108 +290,77 @@ defmodule Philomena.Images.Search do |> Tag.display_order() end - defp parse_sd(%{"sd" => sd}) when sd in ~W(asc desc), do: sd - defp parse_sd(_params), do: "desc" + # Navigation - defp parse_sf(%{"sf" => sf}, sd, query) when sf == "id" do - %{query: query, sorts: [%{"id" => sd}]} + defp apply_reverse_navigation(%Scope{} = scope) do + if scope.rel == :prev do + case scope.sd do + :asc -> %{scope | sd: :desc} + :desc -> %{scope | sd: :asc} + end + else + scope + end end - defp parse_sf(%{"sf" => sf}, sd, query) when sf in @allowed_sort_fields do - %{query: query, sorts: [%{sf => sd}, %{"id" => sd}]} + defp cursor_or_default(%Image{} = image, sf, sort) do + if sort do + {:ok, sort} + else + default_cursor(sf, image) + end end - defp parse_sf(%{"sf" => "_score"}, sd, query) do - %{query: query, sorts: [%{"_score" => sd}, %{"id" => sd}]} + defp default_cursor(:id, %Image{id: id}) do + {:ok, [id]} end - defp parse_sf(%{"sf" => "random"}, sd, query) do - random_query(:rand.uniform(4_294_967_296), sd, query) + defp default_cursor({:field, :first_seen_at}, %Image{first_seen_at: first_seen_at, id: id}) do + {:ok, [DateTime.to_unix(first_seen_at, :millisecond), id]} end - defp parse_sf(%{"sf" => <<"random:", seed::binary>>}, sd, query) do - case Integer.parse(seed) do - {seed, _rest} -> - random_query(seed, sd, query) + defp default_cursor(_sort, _image), do: :error - _ -> - random_query(:rand.uniform(4_294_967_296), sd, query) - end - end + # Sorting - defp parse_sf(%{"sf" => <<"gallery_id:", gallery::binary>>}, sd, query) do - case Integer.parse(gallery) do - {gallery, _rest} -> - %{ - query: query, - sorts: [ + defp compile_sort({sf, sd} = _sort, query) do + case sf do + :id -> + {query, [%{id: sd}]} + + {:field, field} -> + {query, [%{field => sd}, %{id: sd}]} + + {:random, seed} -> + { + %{ + function_score: %{ + query: query, + random_score: %{seed: seed, field: :id}, + boost_mode: :replace + } + }, + [%{_score: sd}, %{id: sd}] + } + + {:gallery, gallery_id} -> + { + query, + [ %{ "galleries.position" => %{ order: sd, nested: %{ path: :galleries, filter: %{ - term: %{"galleries.id" => gallery} + term: %{"galleries.id" => gallery_id} } } } }, - %{"id" => "desc"} + %{id: sd} ] } - - _ -> - %{query: query, sorts: []} end end - - defp parse_sf(_params, sd, query) do - %{query: query, sorts: [%{"first_seen_at" => sd}, %{"id" => sd}]} - end - - defp random_query(seed, sd, query) do - %{ - query: %{ - function_score: %{ - query: query, - random_score: %{seed: seed, field: :id}, - boost_mode: :replace - } - }, - sorts: [%{"_score" => sd}, %{"id" => sd}] - } - end - - defp maybe_search_after(module, body, options, queryable, true) do - module - |> Search.search_definition(body, options) - |> Search.search_records_with_hits(queryable) - end - - defp maybe_search_after(_module, _body, _options, _queryable, _false) do - [] - end - - defp default_cursors([], "id", image), do: [image.id] - - defp default_cursors([], "first_seen_at", image), - do: [image.first_seen_at |> DateTime.to_unix(:millisecond), image.id] - - defp default_cursors(list, _sf, _image), do: list - - defp apply_direction({"galleries.position", sort_body}, rel) do - sort_body = update_in(sort_body.order, fn direction -> @order_for_dir[rel][direction] end) - - %{"galleries.position" => sort_body} - end - - defp apply_direction({field, direction}, rel) do - %{field => @order_for_dir[rel][direction]} - end - - defp permit_list(value) when is_list(value), do: value - defp permit_list(_value), do: [] - - defp permit_value(value) when is_binary(value) or is_number(value), do: [value] - defp permit_value(_value), do: [] end diff --git a/lib/philomena/images/search/cursor.ex b/lib/philomena/images/search/cursor.ex new file mode 100644 index 000000000..cc130e58a --- /dev/null +++ b/lib/philomena/images/search/cursor.ex @@ -0,0 +1,106 @@ +defmodule Philomena.Images.Search.Cursor do + @moduledoc false + + import Ecto.Changeset + + @cursor_types %{ + updated_at: :epoch, + first_seen_at: :epoch, + faves: :integer, + downvotes: :integer, + upvotes: :integer, + width: :integer, + height: :integer, + score: :integer, + hides: :integer, + comment_count: :integer, + pixels: :integer, + size: :integer, + aspect_ratio: :float, + wilson_score: :float, + duration: :float, + _score: :float + } + + def cast_cursor(changeset, sort_field, cursor_field) do + with {:ok, cursor} <- fetch_change(changeset, cursor_field) do + sf = get_field(changeset, sort_field) + + case parse_cursor(sf, cursor) do + {:ok, cursor} -> + put_change(changeset, cursor_field, cursor) + + :error -> + add_error(changeset, cursor_field, "is invalid for the given sort field") + end + else + _ -> + changeset + end + end + + defp parse_cursor(:id, [id_value]) do + with {:ok, id_value} <- parse_integer(id_value) do + {:ok, [id_value]} + end + end + + defp parse_cursor({:field, field_name}, [field_value, id_value]) do + field_type = Map.fetch!(@cursor_types, field_name) + + with {:ok, field_value} <- parse_sort_field_value(field_type, field_value), + {:ok, id_value} <- parse_integer(id_value) do + {:ok, [field_value, id_value]} + end + end + + defp parse_cursor({:random, _seed}, [offset_value, id_value]) do + with {:ok, offset_value} <- parse_float(offset_value), + {:ok, id_value} <- parse_integer(id_value) do + {:ok, [offset_value, id_value]} + end + end + + defp parse_cursor({:gallery, _gallery_id}, [position_value, id_value]) do + with {:ok, position_value} <- parse_integer(position_value), + {:ok, id_value} <- parse_integer(id_value) do + {:ok, [position_value, id_value]} + end + end + + defp parse_cursor(_type, _values), + do: :error + + defp parse_sort_field_value(field_type, field_value) do + case field_type do + :epoch -> parse_epoch(field_value) + :integer -> parse_integer(field_value) + :float -> parse_float(field_value) + end + end + + defp parse_epoch(value), + do: parse_integer(value) + + defp parse_integer(value) when is_binary(value) do + with {value, ""} <- Integer.parse(value) do + {:ok, value} + else + _ -> :error + end + end + + defp parse_integer(_value), + do: :error + + defp parse_float(value) when is_binary(value) do + with {value, ""} <- Float.parse(value) do + {:ok, value} + else + _ -> :error + end + end + + defp parse_float(_value), + do: :error +end diff --git a/lib/philomena/images/search/scope.ex b/lib/philomena/images/search/scope.ex index a3eb18340..22d950eb7 100644 --- a/lib/philomena/images/search/scope.ex +++ b/lib/philomena/images/search/scope.ex @@ -14,22 +14,27 @@ defmodule Philomena.Images.Search.Scope do use Ecto.Schema import Ecto.Changeset + alias Philomena.Images.Search.SortField + import Philomena.Images.Search.Cursor + @type t :: %__MODULE__{ filter: map(), pagination: PhilomenaQuery.Search.pagination_params() } + @primary_key false + embedded_schema do field :filter, :map, virtual: true field :pagination, :map, virtual: true field :q, :string - field :sf, :string - field :sd, :string - field :sort, {:array, :string} + field :sf, SortField, default: {:field, :first_seen_at} + field :sd, Ecto.Enum, values: [:asc, :desc], default: :desc + field :sort, {:array, :any} field :del, :string field :hidden, :boolean - field :rel, :string + field :rel, Ecto.Enum, values: [:prev, :next], default: :next end @spec new(map(), PhilomenaQuery.Search.pagination_params(), map()) :: t() @@ -43,6 +48,7 @@ defmodule Philomena.Images.Search.Scope do scope |> cast(attrs, [:q, :sf, :sd, :sort, :del, :hidden, :rel]) + |> cast_cursor(:sf, :sort) |> then(&keep_valid(scope, &1)) |> apply_action!(:create) end diff --git a/lib/philomena/images/search/sort_field.ex b/lib/philomena/images/search/sort_field.ex new file mode 100644 index 000000000..70990ebb8 --- /dev/null +++ b/lib/philomena/images/search/sort_field.ex @@ -0,0 +1,82 @@ +defmodule Philomena.Images.Search.SortField do + @moduledoc false + + use Ecto.Type + + @sort_fields ~W( + updated_at + first_seen_at + aspect_ratio + faves + downvotes + upvotes + width + height + score + comment_count + tag_count + wilson_score + pixels + size + duration + hides + _score + )a + + @sort_fields Map.new(@sort_fields, &{Atom.to_string(&1), &1}) + + def type do + :string + end + + def cast("id") do + {:ok, :id} + end + + def cast("random") do + {:ok, {:random, :rand.uniform(4_294_967_296)}} + end + + def cast(<<"random:", seed::binary>>) do + case Integer.parse(seed) do + {seed, ""} -> + {:ok, {:random, seed}} + + _ -> + {:error, message: "has an invalid random seed"} + end + end + + def cast(<<"gallery_id:", gallery_id::binary>>) do + case Integer.parse(gallery_id) do + {gallery_id, ""} -> + {:ok, {:gallery, gallery_id}} + + _ -> + {:error, message: "has an invalid gallery ID"} + end + end + + def cast(value) do + case Map.fetch(@sort_fields, value) do + {:ok, field_name} -> + {:ok, {:field, field_name}} + + _ -> + {:error, message: "is invalid"} + end + end + + def load(value) do + cast(value) + end + + def dump(value) do + case value do + :id -> {:ok, "id"} + {:field, field_name} -> {:ok, Atom.to_string(field_name)} + {:random, random_seed} -> {:ok, "random:#{random_seed}"} + {:gallery, gallery_id} -> {:ok, "gallery_id:#{gallery_id}"} + end + end +end diff --git a/lib/philomena/profiles.ex b/lib/philomena/profiles.ex index 8ecbda626..bb5163e4a 100644 --- a/lib/philomena/profiles.ex +++ b/lib/philomena/profiles.ex @@ -49,12 +49,20 @@ defmodule Philomena.Profiles do defp assemble_profile_page(actor, scope, current_filter, user) do {:ok, {recent_uploads_def, _tags}} = - ImageSearch.search_string(actor, scope, "uploader_id:#{user.id}", + ImageSearch.search_string( + actor, + scope, + ImageSearch.default_sort(), + "uploader_id:#{user.id}", pagination: %{page_number: 1, page_size: 4} ) {:ok, {recent_faves_def, _tags}} = - ImageSearch.search_string(actor, scope, "faved_by_id:#{user.id}", + ImageSearch.search_string( + actor, + scope, + ImageSearch.default_sort(), + "faved_by_id:#{user.id}", pagination: %{page_number: 1, page_size: 4} ) @@ -154,7 +162,11 @@ defmodule Philomena.Profiles do defp recent_artwork_definition(actor, scope, tags) do {definition, _tags} = - ImageSearch.query(actor, scope, %{terms: %{tag_ids: Enum.map(tags, & &1.id)}}, + ImageSearch.query( + actor, + scope, + ImageSearch.default_sort(), + %{terms: %{tag_ids: Enum.map(tags, & &1.id)}}, pagination: %{page_number: 1, page_size: 4} ) diff --git a/lib/philomena/tags.ex b/lib/philomena/tags.ex index e3d872554..a313c6bcc 100644 --- a/lib/philomena/tags.ex +++ b/lib/philomena/tags.ex @@ -660,7 +660,8 @@ defmodule Philomena.Tags do {:aliased_to, tag} _tag -> - {images, _tags} = ImageSearch.query(actor, scope, %{term: %{"tags" => tag.name}}) + sort = ImageSearch.scope_sort(scope) + {images, _tags} = ImageSearch.query(actor, scope, sort, %{term: %{"tags" => tag.name}}) images = ImageSearch.execute(images) {:ok, diff --git a/test/philomena/images/search_test.exs b/test/philomena/images/search_test.exs index 9f4c8e0c3..230294929 100644 --- a/test/philomena/images/search_test.exs +++ b/test/philomena/images/search_test.exs @@ -59,6 +59,10 @@ defmodule Philomena.Images.SearchTest do ) end + defp sort do + Search.scope_sort(scope()) + end + defp result_ids(definition) do definition |> Search.execute() @@ -130,7 +134,7 @@ defmodule Philomena.Images.SearchTest do image = image_fixture(hidden_from_users: true) SearchHelpers.reindex_all!(Image) - {definition, _tags} = Search.query(actor(), scope(), %{match_all: %{}}) + {definition, _tags} = Search.query(actor(), scope(), sort(), %{match_all: %{}}) refute image.id in result_ids(definition) end @@ -143,7 +147,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(), scope(params: %{"del" => "1"}), %{match_all: %{}}) + Search.query(actor(), scope(params: %{"del" => "1"}), sort(), %{match_all: %{}}) refute image.id in result_ids(definition) end @@ -155,7 +159,7 @@ defmodule Philomena.Images.SearchTest do image = image_fixture(hidden_from_users: true) SearchHelpers.reindex_all!(Image) - {definition, _tags} = Search.query(actor(admin), scope(), %{match_all: %{}}) + {definition, _tags} = Search.query(actor(admin), scope(), sort(), %{match_all: %{}}) refute image.id in result_ids(definition) end @@ -167,7 +171,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(admin), scope(params: %{"del" => "1"}), %{match_all: %{}}) + Search.query(actor(admin), scope(params: %{"del" => "1"}), sort(), %{match_all: %{}}) ids = result_ids(definition) assert hidden.id in ids @@ -181,7 +185,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(admin), scope(params: %{"del" => "only"}), %{match_all: %{}}) + Search.query(actor(admin), scope(params: %{"del" => "only"}), sort(), %{match_all: %{}}) ids = result_ids(definition) assert hidden.id in ids @@ -199,7 +203,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(admin), scope(params: %{"del" => "deleted"}), %{match_all: %{}}) + Search.query(actor(admin), scope(params: %{"del" => "deleted"}), sort(), %{match_all: %{}}) ids = result_ids(definition) assert hidden_non_dupe.id in ids @@ -213,7 +217,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(moderator), scope(params: %{"del" => "1"}), %{match_all: %{}}) + Search.query(actor(moderator), scope(params: %{"del" => "1"}), sort(), %{match_all: %{}}) assert image.id in result_ids(definition) end @@ -226,7 +230,9 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(moderator), scope(params: %{"del" => "only"}), %{match_all: %{}}) + Search.query(actor(moderator), scope(params: %{"del" => "only"}), sort(), %{ + match_all: %{} + }) ids = result_ids(definition) assert hidden.id in ids @@ -239,7 +245,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(admin), scope(params: %{"del" => "1"}), %{match_all: %{}}) + Search.query(actor(admin), scope(params: %{"del" => "1"}), sort(), %{match_all: %{}}) refute image.id in result_ids(definition) end @@ -252,7 +258,7 @@ defmodule Philomena.Images.SearchTest do hides_image!(image, user) SearchHelpers.reindex_all!(Image) - {definition, _tags} = Search.query(actor(user), scope(), %{match_all: %{}}) + {definition, _tags} = Search.query(actor(user), scope(), sort(), %{match_all: %{}}) refute image.id in result_ids(definition) end @@ -264,7 +270,7 @@ defmodule Philomena.Images.SearchTest do SearchHelpers.reindex_all!(Image) {definition, _tags} = - Search.query(actor(user), scope(params: %{"hidden" => "1"}), %{match_all: %{}}) + Search.query(actor(user), scope(params: %{"hidden" => "1"}), sort(), %{match_all: %{}}) assert image.id in result_ids(definition) end @@ -272,7 +278,7 @@ defmodule Philomena.Images.SearchTest do describe "search_string/3" do test "returns {:ok, {definition, tags}} for a valid query naming no tag" do - assert {:ok, {definition, tags}} = Search.search_string(actor(), scope(), "*") + assert {:ok, {definition, tags}} = Search.search_string(actor(), scope(), sort(), "*") assert is_map(definition) assert tags == [] @@ -281,7 +287,7 @@ defmodule Philomena.Images.SearchTest do test "returns the raw Tag record a single-tag query names" do _image = image_fixture(tags: "safe") - assert {:ok, {_definition, tags}} = Search.search_string(actor(), scope(), "safe") + assert {:ok, {_definition, tags}} = Search.search_string(actor(), scope(), sort(), "safe") assert [%Tag{} = tag] = tags assert tag.name == "safe" @@ -290,76 +296,74 @@ defmodule Philomena.Images.SearchTest do end test "returns {:error, msg} for a malformed query" do - assert {:error, msg} = Search.search_string(actor(), scope(), "width.gte:abc") + assert {:error, msg} = Search.search_string(actor(), scope(), sort(), "width.gte:abc") assert is_binary(msg) end end - describe "parse_sort/2" do + describe "scope sort parsing" do @query %{match_all: %{}} + defp query(params) do + scope = scope(params: params) + sort = Search.scope_sort(scope) + {definition, _tags} = Search.query(actor(), scope, sort, @query) + definition.body + end + test "an allowed field sorts by that field and then id" do - assert %{query: @query, sorts: [%{"score" => "desc"}, %{"id" => "desc"}]} = - Search.parse_sort(%{"sf" => "score"}, @query) + assert %{sort: [%{score: :desc}, %{id: :desc}]} = query(%{sf: "score"}) end test "the sd parameter selects the direction" do - assert %{sorts: [%{"score" => "asc"}, %{"id" => "asc"}]} = - Search.parse_sort(%{"sf" => "score", "sd" => "asc"}, @query) + assert %{sort: [%{score: :asc}, %{id: :asc}]} = query(%{sf: "score", sd: "asc"}) end test "the id field sorts by id alone" do - assert %{query: @query, sorts: [%{"id" => "desc"}]} = - Search.parse_sort(%{"sf" => "id"}, @query) + assert %{sort: [%{id: :desc}]} = query(%{sf: "id"}) end test "an unknown field falls back to first_seen_at" do - assert %{sorts: [%{"first_seen_at" => "desc"}, %{"id" => "desc"}]} = - Search.parse_sort(%{"sf" => "bogus"}, @query) + assert %{sort: [%{first_seen_at: :desc}, %{id: :desc}]} = query(%{sf: "bogus"}) end test "a missing field falls back to first_seen_at" do - assert %{sorts: [%{"first_seen_at" => "desc"}, %{"id" => "desc"}]} = - Search.parse_sort(%{}, @query) + assert %{sort: [%{first_seen_at: :desc}, %{id: :desc}]} = query(%{}) end test "random:seed wraps the query in a seeded function_score" do - result = Search.parse_sort(%{"sf" => "random:12345"}, @query) + result = query(%{sf: "random:12345"}) assert %{ function_score: %{ - query: @query, random_score: %{seed: 12_345, field: :id}, boost_mode: :replace } - } = result.query + } = result.query.bool.must - assert result.sorts == [%{"_score" => "desc"}, %{"id" => "desc"}] + assert result.sort == [%{_score: :desc}, %{id: :desc}] end test "random:seed is deterministic for a fixed seed" do - assert Search.parse_sort(%{"sf" => "random:42"}, @query) == - Search.parse_sort(%{"sf" => "random:42"}, @query) + assert query(%{sf: "random:42"}) == query(%{sf: "random:42"}) end test "gallery_id:n produces a nested gallery-position sort" do assert %{ - query: @query, - sorts: [ + sort: [ %{ "galleries.position" => %{ - order: "desc", + order: :desc, nested: %{path: :galleries, filter: %{term: %{"galleries.id" => 7}}} } }, - %{"id" => "desc"} + %{id: :desc} ] - } = Search.parse_sort(%{"sf" => "gallery_id:7"}, @query) + } = query(%{sf: "gallery_id:7"}) end - test "an invalid gallery id yields empty sorts" do - assert %{query: @query, sorts: []} = - Search.parse_sort(%{"sf" => "gallery_id:abc"}, @query) + test "an invalid gallery id falls back to the default sort" do + assert %{sort: [%{first_seen_at: :desc}, %{id: :desc}]} = query(%{sf: "gallery_id:abc"}) end end @@ -400,8 +404,9 @@ defmodule Philomena.Images.SearchTest do test "uses a provided cursor for a non-default sort with tied sort values", %{ compiled: compiled } do - sort_scope = scope(params: %{"sf" => "score", "sd" => "desc"}) - {definition, _tags} = Search.query(actor(), sort_scope, %{match_all: %{}}) + scope = scope(params: %{"sf" => "score", "sd" => "desc"}) + sort = Search.scope_sort(scope) + {definition, _tags} = Search.query(actor(), scope, sort, %{match_all: %{}}) %{entries: [{first, first_hit}, {second, second_hit} | _]} = Search.execute(definition, hits: true) @@ -426,10 +431,48 @@ defmodule Philomena.Images.SearchTest do assert scope.pagination == @pagination end - test "casts the search_after sort cursor as a string array" do + test "casts a valid sort cursor for id" do + scope = Scope.new(%{match_all: %{}}, @pagination, %{"sf" => "id", "sort" => ["123"]}) + + assert scope.sort == [123] + end + + test "rejects an invalid sort cursor for id" do + scope = Scope.new(%{match_all: %{}}, @pagination, %{"sf" => "id", "sort" => ["3.14"]}) + + assert scope.sort == nil + end + + test "casts a valid sort cursor for first_seen_at" do scope = Scope.new(%{match_all: %{}}, @pagination, %{"sort" => ["123", "456"]}) - assert scope.sort == ["123", "456"] + assert scope.sort == [123, 456] + end + + test "rejects an invalid sort cursor for first_seen_at" do + scope = Scope.new(%{match_all: %{}}, @pagination, %{"sort" => ["123"]}) + + assert scope.sort == nil + end + + test "casts a valid sort cursor for wilson_score" do + scope = + Scope.new(%{match_all: %{}}, @pagination, %{ + "sf" => "wilson_score", + "sort" => ["3.14", "159"] + }) + + assert scope.sort == [3.14, 159] + end + + test "rejects an invalid sort cursor for wilson_score" do + scope = + Scope.new(%{match_all: %{}}, @pagination, %{ + "sf" => "wilson_score", + "sort" => ["abcd", "159"] + }) + + assert scope.sort == nil end test "defaults params and pagination" do