From 5dc418101423bae971e14696651208e1e9818dba Mon Sep 17 00:00:00 2001 From: "byte[]" Date: Sun, 16 Aug 2020 02:48:09 -0400 Subject: [PATCH] tag query names correctly --- lib/philomena/spoiler_executor.ex | 81 ++++++++++++++----- .../controllers/activity_controller.ex | 4 +- lib/philomena_web/views/image_view.ex | 30 +++---- 3 files changed, 75 insertions(+), 40 deletions(-) diff --git a/lib/philomena/spoiler_executor.ex b/lib/philomena/spoiler_executor.ex index 1f3deb6e..df9ec582 100644 --- a/lib/philomena/spoiler_executor.ex +++ b/lib/philomena/spoiler_executor.ex @@ -8,27 +8,42 @@ defmodule Philomena.SpoilerExecutor do import Ecto.Query import Philomena.Search.String + @complex_tag "complex" + @hidden_tag "hidden" + @doc """ Compile a filter's spoiler context for the purpose of executing it as a spoiler. This logic is different from filter execution because it tags query leaves with relevant information about which aspect of the - spoiler they match (tag match, complex filter match). + spoiler they match (tag spoiler match, complex spoiler match). """ @spec compile_spoiler(map(), map()) :: map() def compile_spoiler(user, filter) do spoilered_tags = Enum.map(filter.spoilered_tag_ids, fn id -> - %{term: %{tag_id: id}, _name: Integer.to_string(id)} + %{term: %{tag_ids: %{value: id, _name: Integer.to_string(id)}}} end) - spoilered_complex = - user - |> invalid_filter_guard(filter.spoilered_complex_str) - |> Map.put(:_name, "complex") + spoilered_complex = %{ + bool: %{ + must: invalid_filter_guard(user, filter.spoilered_complex_str), + _name: @complex_tag + } + } + + hides = %{ + bool: %{ + must: [ + invalid_filter_guard(user, filter.hidden_complex_str), + %{terms: %{tag_ids: filter.hidden_tag_ids}} + ], + _name: @hidden_tag + } + } %{ bool: %{ - should: [spoilered_complex | spoilered_tags] + should: [hides, spoilered_complex | spoilered_tags] } } end @@ -39,19 +54,28 @@ defmodule Philomena.SpoilerExecutor do the spoiler matched, or the atom :complex if the complex spoiler matched instead of any tag object. """ - @spec execute_spoiler(map(), list()) :: %{optional(integer()) => [map()] | :complex} + @spec execute_spoiler(map(), list()) :: %{optional(integer()) => [map()] | :complex | :hidden} def execute_spoiler(compiled, images) do - image_ids = %{ + image_ids = extract_ids(images) + + image_terms = %{ terms: %{id: extract_ids(images)} } test_query = %{ - bool: %{ - must: [compiled, image_ids], + query: %{ + bool: %{ + must: [compiled, image_terms] + } } } - results = Elasticsearch.search_results(Image, test_query) + pagination_params = %{ + page_number: 1, + page_size: length(image_ids) + } + + results = Elasticsearch.search_results(Image, test_query, pagination_params) tags = extract_tags(results.entries) @@ -95,7 +119,7 @@ defmodule Philomena.SpoilerExecutor do defp extract_tags(results) do hit_tag_ids = results - |> Enum.flat_map(fn {_id, hit} -> hit["matched_queries"] end) + |> Enum.flat_map(fn {_id, hit} -> filter_special_matched(hit["matched_queries"]) end) |> Enum.uniq() Tag @@ -106,17 +130,23 @@ defmodule Philomena.SpoilerExecutor do # Create a map key for the response of execute_spoiler/2. Determines # the reason an image was in this response. - @spec filter_reason({integer(), map()}, map()) :: {integer(), [map()] | :complex} + @spec filter_reason({integer(), map()}, map()) :: {integer(), [map()] | :complex | :hidden} defp filter_reason({id, hit}, tags) do - tags - |> Map.take(hit["matched_queries"]) - |> Enum.sort(&tag_sort/2) - |> case do - [] -> - {id, :complex} + matched_queries = hit["matched_queries"] - matched_tags -> - {id, matched_tags} + if Enum.member?(matched_queries, @hidden_tag) do + {id, :hidden} + else + tags + |> Map.take(matched_queries) + |> Enum.sort(&tag_sort/2) + |> case do + [] -> + {id, :complex} + + matched_tags -> + {id, matched_tags} + end end end @@ -140,4 +170,11 @@ defmodule Philomena.SpoilerExecutor do true end end + + # The list of matched queries may return things which do not look like + # integer IDs and will cause Postgrex to choke; filter those out here. + @spec filter_special_matched(list()) :: list() + defp filter_special_matched(matched_queries) do + Enum.filter(matched_queries, &String.match?(&1, ~r/\A\d+\z/)) + end end diff --git a/lib/philomena_web/controllers/activity_controller.ex b/lib/philomena_web/controllers/activity_controller.ex index 173629d2..51fde34f 100644 --- a/lib/philomena_web/controllers/activity_controller.ex +++ b/lib/philomena_web/controllers/activity_controller.ex @@ -57,6 +57,8 @@ defmodule PhilomenaWeb.ActivityController do Comment |> preload([:user, image: [:tags]]) ) + comment_images = Enum.map(comments, & &1.image) + watched = if !!user do {:ok, {watched_images, _tags}} = @@ -106,7 +108,7 @@ defmodule PhilomenaWeb.ActivityController do spoilers = SpoilerExecutor.execute_spoiler( conn.assigns.compiled_spoiler, - [images, top_scoring, watched, featured_image] + [images, top_scoring, watched, featured_image, comment_images] ) render( diff --git a/lib/philomena_web/views/image_view.ex b/lib/philomena_web/views/image_view.ex index 0295dbd4..8b536da4 100644 --- a/lib/philomena_web/views/image_view.ex +++ b/lib/philomena_web/views/image_view.ex @@ -236,29 +236,25 @@ defmodule PhilomenaWeb.ImageView do } end + def filter_or_spoiler_value(conn, image) do + spoilered(conn)[image.id] + end + def filter_or_spoiler_hits?(conn, image) do - tag_filter_or_spoiler_hits?(conn, image) or complex_filter_or_spoiler_hits?(conn, image) + Map.has_key?(spoilered(conn), image.id) end - defp tag_filter_or_spoiler_hits?(conn, image) do - filter = conn.assigns.current_filter - filtered_tag_ids = MapSet.new(filter.spoilered_tag_ids ++ filter.hidden_tag_ids) - image_tag_ids = MapSet.new(image.tags, & &1.id) - - MapSet.size(MapSet.intersection(filtered_tag_ids, image_tag_ids)) > 0 + def filter_hits?(conn, image) do + spoilered(conn)[image.id] == :hidden end - defp complex_filter_or_spoiler_hits?(conn, image) do - doc = image_filter_data(image) - complex_filter = conn.assigns.compiled_complex_filter - complex_spoiler = conn.assigns.compiled_complex_spoiler + def spoiler_hits?(conn, image) do + spoilered = spoilered(conn) - query = %{ - bool: %{ - should: [complex_filter, complex_spoiler] - } - } + is_list(spoilered[image.id]) or spoilered[image.id] == :complex + end - Philomena.Search.Evaluator.hits?(doc, query) + defp spoilered(conn) do + Map.get(conn.assigns, :spoilered, %{}) end end