From d1c893248d2c13054e38acfd2c4ccd016edc686c Mon Sep 17 00:00:00 2001 From: liamwhite Date: Thu, 13 Aug 2020 11:32:35 -0400 Subject: [PATCH] Search navigation (#14) * return hits from elasticsearch and add in sort param to templates * use returned hits from elasticsearch for navigation * mix format * fix gallery pagination * add missing fields to search help dropdown * unused variable --- lib/philomena/elasticsearch.ex | 13 +- lib/philomena/interactions.ex | 15 +- .../controllers/gallery_controller.ex | 7 +- .../controllers/image/navigate_controller.ex | 18 +- .../controllers/search_controller.ex | 5 +- lib/philomena_web/image_loader.ex | 13 +- lib/philomena_web/image_navigator.ex | 179 ++++-------------- lib/philomena_web/image_scope.ex | 1 + lib/philomena_web/image_sorter.ex | 13 +- lib/philomena_web/param.ex | 11 ++ .../templates/gallery/show.html.slime | 6 +- .../templates/image/index.html.slime | 10 +- .../templates/search/_form.html.slime | 6 +- 13 files changed, 126 insertions(+), 171 deletions(-) create mode 100644 lib/philomena_web/param.ex diff --git a/lib/philomena/elasticsearch.ex b/lib/philomena/elasticsearch.ex index 62b56590..9637cb63 100644 --- a/lib/philomena/elasticsearch.ex +++ b/lib/philomena/elasticsearch.ex @@ -198,7 +198,7 @@ defmodule Philomena.Elasticsearch do results = search(module, elastic_query) time = results["took"] count = results["hits"]["total"]["value"] - entries = Enum.map(results["hits"]["hits"], &String.to_integer(&1["_id"])) + entries = Enum.map(results["hits"]["hits"], &{String.to_integer(&1["_id"]), &1}) Logger.debug("[Elasticsearch] Query took #{time}ms") Logger.debug("[Elasticsearch] #{Jason.encode!(elastic_query)}") @@ -212,9 +212,9 @@ defmodule Philomena.Elasticsearch do } end - def search_records(module, elastic_query, pagination_params, ecto_query) do + def search_records_with_hits(module, elastic_query, pagination_params, ecto_query) do page = search_results(module, elastic_query, pagination_params) - ids = page.entries + {ids, hits} = Enum.unzip(page.entries) records = ecto_query @@ -222,6 +222,13 @@ defmodule Philomena.Elasticsearch do |> Repo.all() |> Enum.sort_by(&Enum.find_index(ids, fn el -> el == &1.id end)) + %{page | entries: Enum.zip(records, hits)} + end + + def search_records(module, elastic_query, pagination_params, ecto_query) do + page = search_records_with_hits(module, elastic_query, pagination_params, ecto_query) + {records, _hits} = Enum.unzip(page.entries) + %{page | entries: records} end end diff --git a/lib/philomena/interactions.ex b/lib/philomena/interactions.ex index 33703e0c..45b4022a 100644 --- a/lib/philomena/interactions.ex +++ b/lib/philomena/interactions.ex @@ -14,11 +14,7 @@ defmodule Philomena.Interactions do def user_interactions(images, user) do ids = images - |> Enum.flat_map(fn - nil -> [] - %{id: id} -> [id] - enum -> Enum.map(enum, & &1.id) - end) + |> flatten_images() |> Enum.uniq() hide_interactions = @@ -140,4 +136,13 @@ defmodule Philomena.Interactions do defp union_all_queries([query | rest]), do: query |> union_all(^union_all_queries(rest)) + + defp flatten_images(images) do + Enum.flat_map(images, fn + nil -> [] + %{id: id} -> [id] + {%{id: id}, _hit} -> [id] + enum -> flatten_images(enum) + end) + end end diff --git a/lib/philomena_web/controllers/gallery_controller.ex b/lib/philomena_web/controllers/gallery_controller.ex index 74c22880..cad4a057 100644 --- a/lib/philomena_web/controllers/gallery_controller.ex +++ b/lib/philomena_web/controllers/gallery_controller.ex @@ -54,7 +54,7 @@ defmodule PhilomenaWeb.GalleryController do conn = %{conn | params: params} - {:ok, {images, _tags}} = ImageLoader.search_string(conn, query) + {:ok, {images, _tags}} = ImageLoader.search_string(conn, query, include_hits: true) {gallery_prev, gallery_next} = prev_next_page_images(conn, query) @@ -66,7 +66,7 @@ defmodule PhilomenaWeb.GalleryController do next_image = if gallery_next, do: [gallery_next], else: [] gallery_images = prev_image ++ Enum.to_list(images) ++ next_image - gallery_json = Jason.encode!(Enum.map(gallery_images, & &1.id)) + gallery_json = Jason.encode!(Enum.map(gallery_images, &elem(&1, 0).id)) Galleries.clear_notification(gallery, user) @@ -162,7 +162,8 @@ defmodule PhilomenaWeb.GalleryController do defp gallery_image(offset, conn, query) do pagination_params = %{page_number: offset + 1, page_size: 1} - {:ok, {image, _tags}} = ImageLoader.search_string(conn, query, pagination: pagination_params) + {:ok, {image, _tags}} = + ImageLoader.search_string(conn, query, pagination: pagination_params, include_hits: true) case Enum.to_list(image) do [image] -> image diff --git a/lib/philomena_web/controllers/image/navigate_controller.ex b/lib/philomena_web/controllers/image/navigate_controller.ex index 360c1632..2e44220f 100644 --- a/lib/philomena_web/controllers/image/navigate_controller.ex +++ b/lib/philomena_web/controllers/image/navigate_controller.ex @@ -10,18 +10,22 @@ defmodule PhilomenaWeb.Image.NavigateController do plug PhilomenaWeb.CanaryMapPlug, index: :show plug :load_and_authorize_resource, model: Image, id_name: "image_id", persisted: true - def index(conn, %{"rel" => rel} = params) when rel in ~W(prev next) do + def index(conn, %{"rel" => rel}) when rel in ~W(prev next) do image = conn.assigns.image filter = conn.assigns.compiled_filter - rel = String.to_existing_atom(rel) - - next_image = - ImageNavigator.find_consecutive(conn, image, rel, params, compile_query(conn), filter) - scope = ImageScope.scope(conn) conn - |> redirect(to: Routes.image_path(conn, :show, next_image, scope)) + |> ImageNavigator.find_consecutive(image, compile_query(conn), filter) + |> case do + {next_image, hit} -> + redirect(conn, + to: Routes.image_path(conn, :show, next_image, Keyword.put(scope, :sort, hit["sort"])) + ) + + nil -> + redirect(conn, to: Routes.image_path(conn, :show, image, scope)) + end end def index(conn, %{"rel" => "find"}) do diff --git a/lib/philomena_web/controllers/search_controller.ex b/lib/philomena_web/controllers/search_controller.ex index 8e3171bf..02a8ba61 100644 --- a/lib/philomena_web/controllers/search_controller.ex +++ b/lib/philomena_web/controllers/search_controller.ex @@ -7,7 +7,7 @@ defmodule PhilomenaWeb.SearchController do def index(conn, params) do user = conn.assigns.current_user - case ImageLoader.search_string(conn, params["q"]) do + case ImageLoader.search_string(conn, params["q"], include_hits: custom_ordering?(conn)) do {:ok, {images, tags}} -> interactions = Interactions.user_interactions(images, user) @@ -30,4 +30,7 @@ defmodule PhilomenaWeb.SearchController do ) end end + + defp custom_ordering?(%{params: %{"sf" => sf}}) when sf != "id", do: true + defp custom_ordering?(_conn), do: false end diff --git a/lib/philomena_web/image_loader.ex b/lib/philomena_web/image_loader.ex index 399f5190..e119e6d2 100644 --- a/lib/philomena_web/image_loader.ex +++ b/lib/philomena_web/image_loader.ex @@ -36,7 +36,7 @@ defmodule PhilomenaWeb.ImageLoader do %{query: query, sorts: sort} = sorts.(body) records = - Elasticsearch.search_records( + search_function(options).( Image, %{ query: %{ @@ -95,6 +95,17 @@ defmodule PhilomenaWeb.ImageLoader do defp maybe_custom_hide(filters, _user, _param), do: filters + # Allow callers to choose if they want inner hit objects returned; + # primarily useful for allowing client navigation through images + + @spec search_function(Keyword.t()) :: function() + defp search_function(options) do + case Keyword.get(options, :include_hits) do + true -> &Elasticsearch.search_records_with_hits/4 + _false -> &Elasticsearch.search_records/4 + end + end + # TODO: the search parser should try to optimize queries defp search_tag_name(%{term: %{"namespaced_tags.name" => tag_name}}), do: [tag_name] defp search_tag_name(_other_query), do: [] diff --git a/lib/philomena_web/image_navigator.ex b/lib/philomena_web/image_navigator.ex index 8686f1d8..c9a58b55 100644 --- a/lib/philomena_web/image_navigator.ex +++ b/lib/philomena_web/image_navigator.ex @@ -1,179 +1,82 @@ defmodule PhilomenaWeb.ImageNavigator do alias PhilomenaWeb.ImageSorter - alias Philomena.Images.{Image, ElasticsearchIndex} + alias Philomena.Images.Image alias Philomena.Elasticsearch - alias Philomena.Repo - import Ecto.Query - # We get consecutive images by finding all images greater than or less than - # the current image, and grabbing the FIRST one - @range_comparison_for_order %{ - "asc" => :gt, - "desc" => :lt - } - - # If we didn't reverse for prev, it would be the LAST image, which would - # make Elasticsearch choke on deep pagination @order_for_dir %{ - next: %{"asc" => "asc", "desc" => "desc"}, - prev: %{"asc" => "desc", "desc" => "asc"} + "next" => %{"asc" => "asc", "desc" => "desc"}, + "prev" => %{"asc" => "desc", "desc" => "asc"} } - @range_map %{ - gt: :gte, - lt: :lte - } + def find_consecutive(conn, image, compiled_query, compiled_filter) do + %{query: compiled_query, sorts: sorts} = ImageSorter.parse_sort(conn.params, compiled_query) - def find_consecutive(conn, image, rel, params, compiled_query, compiled_filter) do - image_index = - Image - |> where(id: ^image.id) - |> preload([:gallery_interactions, tags: :aliases]) - |> Repo.one() - |> Map.merge(empty_fields()) - |> ElasticsearchIndex.as_json() + sorts = + sorts + |> Enum.flat_map(&Enum.to_list/1) + |> Enum.map(&apply_direction(&1, conn.params["rel"])) - %{query: compiled_query, sorts: sort} = ImageSorter.parse_sort(params, compiled_query) + search_after = + conn.params["sort"] + |> permit_list() + |> Enum.flat_map(&permit_value/1) + |> default_value(image.id) - {sorts, filters} = - sort - |> Enum.map(&extract_filters(&1, image_index, rel)) - |> Enum.unzip() - - sorts = sortify(sorts, image_index) - filters = filterify(filters, image_index) - - Elasticsearch.search_records( + maybe_search_after( Image, %{ query: %{ bool: %{ - must: List.flatten([compiled_query, filters]), + must: compiled_query, must_not: [ compiled_filter, %{term: %{hidden_from_users: true}}, + %{term: %{id: image.id}}, hidden_filter(conn.assigns.current_user, conn.params["hidden"]) ] } }, - sort: List.flatten(sorts) + sort: sorts, + search_after: search_after }, %{page_size: 1}, - Image + Image, + length(sorts) == length(search_after) ) |> Enum.to_list() |> case do - [] -> image + [] -> nil [next_image] -> next_image end end - defp extract_filters(%{"galleries.position" => term} = sort, image, rel) do - # Extract gallery ID and current position - gid = term.nested.filter.term["galleries.id"] - pos = Enum.find(image[:galleries], &(&1.id == gid)).position - - # Sort in the other direction if we are going backwards - sd = term.order - order = @order_for_dir[rel][to_string(sd)] - term = %{term | order: order} - sort = %{sort | "galleries.position" => term} - - filter = gallery_range_filter(@range_comparison_for_order[order], pos) - - {[sort], [filter]} + defp maybe_search_after(module, body, options, queryable, true) do + Elasticsearch.search_records_with_hits(module, body, options, queryable) end - defp extract_filters(sort, image, rel) do - [{sf, sd}] = Enum.to_list(sort) - order = @order_for_dir[rel][sd] - sort = %{sort | sf => order} - - field = String.to_existing_atom(sf) - filter = range_filter(sf, @range_comparison_for_order[order], image[field]) - - case sf do - "_score" -> - {[sort], []} - - _ -> - {[sort], [filter]} - end + defp maybe_search_after(_module, _body, _options, _queryable, _false) do + [] end - defp sortify(sorts, _image) do - List.flatten(sorts) + 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 filterify(filters, image) do - filters = List.flatten(filters) - - filters = - filters - |> Enum.with_index() - |> Enum.map(fn - {filter, 0} -> - filter.this - - {filter, i} -> - filters_so_far = - filters - |> Enum.take(i) - |> Enum.map(& &1.for_next) - - %{ - bool: %{ - must: [filter.this | filters_so_far] - } - } - end) - - %{ - bool: %{ - should: filters, - must_not: %{term: %{id: image.id}} - } - } + 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: [] + + defp default_value([], term), do: [term] + defp default_value(list, _term), do: list + defp hidden_filter(%{id: id}, param) when param != "1", do: %{term: %{hidden_by_user_ids: id}} defp hidden_filter(_user, _param), do: %{match_none: %{}} - - defp range_filter(sf, dir, val) do - %{ - this: %{range: %{sf => %{dir => parse_val(val)}}}, - next: %{range: %{sf => %{@range_map[dir] => parse_val(val)}}} - } - end - - defp gallery_range_filter(dir, val) do - %{ - this: %{ - nested: %{ - path: :galleries, - query: %{range: %{"galleries.position" => %{dir => val}}} - } - }, - next: %{ - nested: %{ - path: :galleries, - query: %{range: %{"galleries.position" => %{@range_map[dir] => val}}} - } - } - } - end - - defp empty_fields do - %{ - user: nil, - deleter: nil, - upvoters: [], - downvoters: [], - favers: [], - hiders: [] - } - end - - defp parse_val(%NaiveDateTime{} = value), do: NaiveDateTime.to_iso8601(value) - defp parse_val(value), do: value end diff --git a/lib/philomena_web/image_scope.ex b/lib/philomena_web/image_scope.ex index 3c1c48ed..dae148be 100644 --- a/lib/philomena_web/image_scope.ex +++ b/lib/philomena_web/image_scope.ex @@ -5,6 +5,7 @@ defmodule PhilomenaWeb.ImageScope do |> scope(conn, "sf", :sf) |> scope(conn, "sd", :sd) |> scope(conn, "del", :del) + |> scope(conn, "sort", :sort) |> scope(conn, "hidden", :hidden) end diff --git a/lib/philomena_web/image_sorter.ex b/lib/philomena_web/image_sorter.ex index f727dd72..d2124c77 100644 --- a/lib/philomena_web/image_sorter.ex +++ b/lib/philomena_web/image_sorter.ex @@ -1,11 +1,9 @@ defmodule PhilomenaWeb.ImageSorter do @allowed_fields ~W( - created_at updated_at first_seen_at aspect_ratio faves - id downvotes upvotes width @@ -29,11 +27,11 @@ defmodule PhilomenaWeb.ImageSorter do defp parse_sd(_params), do: "desc" defp parse_sf(%{"sf" => sf}, sd, query) when sf in @allowed_fields do - %{query: query, sorts: [%{sf => sd}]} + %{query: query, sorts: [%{sf => sd}, %{"id" => sd}]} end defp parse_sf(%{"sf" => "_score"}, sd, query) do - %{query: query, sorts: [%{"_score" => sd}]} + %{query: query, sorts: [%{"_score" => sd}, %{"id" => sd}]} end defp parse_sf(%{"sf" => "random"}, sd, query) do @@ -66,7 +64,8 @@ defmodule PhilomenaWeb.ImageSorter do } } } - } + }, + %{"id" => "desc"} ] } @@ -76,7 +75,7 @@ defmodule PhilomenaWeb.ImageSorter do end defp parse_sf(_params, sd, query) do - %{query: query, sorts: [%{"created_at" => sd}]} + %{query: query, sorts: [%{"id" => sd}]} end defp random_query(seed, sd, query) do @@ -88,7 +87,7 @@ defmodule PhilomenaWeb.ImageSorter do boost_mode: :replace } }, - sorts: [%{"_score" => sd}] + sorts: [%{"_score" => sd}, %{"id" => sd}] } end end diff --git a/lib/philomena_web/param.ex b/lib/philomena_web/param.ex new file mode 100644 index 00000000..cac03736 --- /dev/null +++ b/lib/philomena_web/param.ex @@ -0,0 +1,11 @@ +defimpl Phoenix.Param, for: Float do + # Another Phoenix sadness: + # + # "By default, Phoenix implements this protocol for integers, binaries, + # atoms, and structs." + # + @spec to_param(float()) :: binary() + def to_param(term) do + Float.to_string(term) + end +end diff --git a/lib/philomena_web/templates/gallery/show.html.slime b/lib/philomena_web/templates/gallery/show.html.slime index 97abf78d..d38b1520 100644 --- a/lib/philomena_web/templates/gallery/show.html.slime +++ b/lib/philomena_web/templates/gallery/show.html.slime @@ -1,6 +1,6 @@ elixir: scope = scope(@conn) - image_url = fn image -> Routes.image_path(@conn, :show, image, scope) end + image_url = fn image, hit -> Routes.image_path(@conn, :show, image, Keyword.put(scope, :sort, hit["sort"])) end route = fn p -> Routes.gallery_path(@conn, :show, @gallery, p) end pagination = render PhilomenaWeb.PaginationView, "_pagination.html", page: @images, route: route, params: scope info = render PhilomenaWeb.PaginationView, "_pagination_info.html", page: @images @@ -72,8 +72,8 @@ elixir: strong Note that you may have to wait a couple of seconds before the order is applied. .block__content.js-resizable-media-container - = for image <- @gallery_images do - = render PhilomenaWeb.ImageView, "_image_box.html", image: image, link: image_url.(image), size: :thumb, conn: @conn + = for {image, hit} <- @gallery_images do + = render PhilomenaWeb.ImageView, "_image_box.html", image: image, link: image_url.(image, hit), size: :thumb, conn: @conn .block__header.block__header--light.flex = pagination diff --git a/lib/philomena_web/templates/image/index.html.slime b/lib/philomena_web/templates/image/index.html.slime index a463089a..66a941ca 100644 --- a/lib/philomena_web/templates/image/index.html.slime +++ b/lib/philomena_web/templates/image/index.html.slime @@ -5,6 +5,7 @@ elixir: tags = assigns[:tags] || [] route = assigns[:route] || fn p -> Routes.image_path(@conn, :index, p) end image_url = fn image -> Routes.image_path(@conn, :show, image, scope) end + sorted_url = fn image, hit -> Routes.image_path(@conn, :show, image, Keyword.put(scope, :sort, hit["sort"])) end pagination = render PhilomenaWeb.PaginationView, "_pagination.html", page: @images, route: route, params: params info = render PhilomenaWeb.PaginationView, "_pagination_info.html", page: @images @@ -24,8 +25,13 @@ elixir: = info_row @conn, tags .block__content.js-resizable-media-container - = for image <- @images do - = render PhilomenaWeb.ImageView, "_image_box.html", image: image, link: image_url.(image), size: assigns[:size] || :thumb, conn: @conn + = for record <- @images do + = case record do + - {image, hit} -> + = render PhilomenaWeb.ImageView, "_image_box.html", image: image, link: sorted_url.(image, hit), size: assigns[:size] || :thumb, conn: @conn + + - image -> + = render PhilomenaWeb.ImageView, "_image_box.html", image: image, link: image_url.(image), size: assigns[:size] || :thumb, conn: @conn .block__header.block__header--light.flex = pagination diff --git a/lib/philomena_web/templates/search/_form.html.slime b/lib/philomena_web/templates/search/_form.html.slime index 8afc914c..088eae53 100644 --- a/lib/philomena_web/templates/search/_form.html.slime +++ b/lib/philomena_web/templates/search/_form.html.slime @@ -14,11 +14,14 @@ h1 Search .dropdown__content a data-search-add="score.gte:100" data-search-select-last="3" data-search-show-help="numeric" Score a data-search-add="created_at.lte:3 years ago" data-search-select-last="11" data-search-show-help="date" Created at + a data-search-add="id.lte:10" data-search-select-last="2" data-search-show-help="numeric" Numeric ID a data-search-add="faves.gte:100" data-search-select-last="3" data-search-show-help="numeric" Number of faves a data-search-add="upvotes.gte:100" data-search-select-last="3" data-search-show-help="numeric" Number of upvotes a data-search-add="downvotes.gte:100" data-search-select-last="3" data-search-show-help="numeric" Number of downvotes a data-search-add="comment_count.gt:20" data-search-select-last="2" data-search-show-help="numeric" Number of comments a data-search-add="uploader:k_a" data-search-select-last="3" data-search-show-help="literal" Uploader + a data-search-add="original_format:gif" data-search-select-last="3" data-search-show-help="literal" File extension + a data-search-add="mime_type:image/jpeg" data-search-select-last="10" data-search-show-help="literal" MIME type a data-search-add="source_url:*deviantart.com*" data-search-select-last="16" data-search-show-help="literal" Image source URL a data-search-add="width:1920" data-search-select-last="4" data-search-show-help="numeric" Image width a data-search-add="height:1080" data-search-select-last="4" data-search-show-help="numeric" Image height @@ -106,6 +109,7 @@ h1 Search elixir: random_is_selected = to_string(@conn.params["sf"]) =~ ~r/\Arandom(:\d+)?\z/ + random_seed = if random_is_selected do @conn.params["sf"] @@ -114,7 +118,7 @@ h1 Search end sort_fields = [ - "Sort by upload date": :created_at, + "Sort by upload date": :id, "Sort by last modification date": :updated_at, "Sort by initial post date": :first_seen_at, "Sort by aspect ratio": :aspect_ratio,