search: apply aliases after parsing searches.

Make PostQueryBuilder apply aliases earlier, immediately after parsing
the search.

On the post index page there are multiple places where we need to apply
aliases:

* When running the search with PostQueryBuilder#build.
* When calculating the search count with PostQueryBuilder#fast_count.
* When calculating the related tags for the sidebar.
* When tracking missed searches and popular searches for Reportbooru.
* When looking up wiki excerpts.

Applying aliases after parsing ensures we only have to apply aliases
once for all of these things.

We also normalize the order of tags in searches and strip repeated tags.
This is so that we have consistent cache keys for fast_count.

* Fixes searches for aliased tags being counted as missed searches (fixes #4433).
* Fixes wiki excerpts not showing up when searching for aliased tags.
This commit is contained in:
evazion
2020-05-07 13:30:04 -05:00
parent f38c38f26e
commit 67aab0236d
13 changed files with 72 additions and 56 deletions

View File

@@ -13,14 +13,12 @@ module PostsHelper
params[:tags] =~ /order:rank/ || params[:action] =~ /searches|viewed/ params[:tags] =~ /order:rank/ || params[:action] =~ /searches|viewed/
end end
def missed_post_search_count_js(post_set) def missed_post_search_count_js(tags)
tags = post_set.query.normalize_query
sig = generate_reportbooru_signature(tags) sig = generate_reportbooru_signature(tags)
render "posts/partials/index/missed_search_count", sig: sig render "posts/partials/index/missed_search_count", sig: sig
end end
def post_search_count_js(post_set) def post_search_count_js(tags)
tags = post_set.query.normalize_query
sig = generate_reportbooru_signature("ps-#{tags}") sig = generate_reportbooru_signature("ps-#{tags}")
render "posts/partials/index/search_count", sig: sig render "posts/partials/index/search_count", sig: sig
end end

View File

@@ -6,8 +6,8 @@ class TagBatchChangeJob < ApplicationJob
def perform(antecedent, consequent, updater, updater_ip_addr) def perform(antecedent, consequent, updater, updater_ip_addr)
raise Error.new("antecedent is missing") if antecedent.blank? raise Error.new("antecedent is missing") if antecedent.blank?
normalized_antecedent = TagAlias.to_aliased(PostQueryBuilder.new(antecedent.mb_chars.downcase).split_query) normalized_antecedent = PostQueryBuilder.new(antecedent).split_query
normalized_consequent = TagAlias.to_aliased(PostQueryBuilder.new(consequent.mb_chars.downcase).parse_tag_edit) normalized_consequent = PostQueryBuilder.new(consequent).parse_tag_edit
CurrentUser.scoped(updater, updater_ip_addr) do CurrentUser.scoped(updater, updater_ip_addr) do
migrate_posts(normalized_antecedent, normalized_consequent) migrate_posts(normalized_antecedent, normalized_consequent)

View File

@@ -53,15 +53,18 @@ class PostQueryBuilder
COUNT_METATAG_SYNONYMS.flat_map { |str| [str, "#{str}_asc"] } + COUNT_METATAG_SYNONYMS.flat_map { |str| [str, "#{str}_asc"] } +
CATEGORY_COUNT_METATAGS.flat_map { |str| [str, "#{str}_asc"] } CATEGORY_COUNT_METATAGS.flat_map { |str| [str, "#{str}_asc"] }
attr_reader :query_string, :current_user, :safe_mode, :hide_deleted_posts attr_reader :query_string, :terms, :current_user, :safe_mode, :hide_deleted_posts
alias_method :safe_mode?, :safe_mode alias_method :safe_mode?, :safe_mode
alias_method :hide_deleted_posts?, :hide_deleted_posts alias_method :hide_deleted_posts?, :hide_deleted_posts
def initialize(query_string, current_user = User.anonymous, safe_mode: false, hide_deleted_posts: false) def initialize(query_string, current_user = User.anonymous, normalize_aliases: true, normalize_order: true, safe_mode: false, hide_deleted_posts: false)
@query_string = query_string @query_string = query_string
@current_user = current_user @current_user = current_user
@safe_mode = safe_mode @safe_mode = safe_mode
@hide_deleted_posts = hide_deleted_posts @hide_deleted_posts = hide_deleted_posts
@terms = scan_query
normalize_aliases! if normalize_aliases
normalize_order! if normalize_order
end end
def tags_match(tags, relation) def tags_match(tags, relation)
@@ -71,9 +74,9 @@ class PostQueryBuilder
optional_wildcard_tags, optional_tags = tags.select(&:optional).partition(&:wildcard) optional_wildcard_tags, optional_tags = tags.select(&:optional).partition(&:wildcard)
required_wildcard_tags, required_tags = tags.reject(&:negated).reject(&:optional).partition(&:wildcard) required_wildcard_tags, required_tags = tags.reject(&:negated).reject(&:optional).partition(&:wildcard)
negated_tags = TagAlias.to_aliased(negated_tags.map(&:name)) negated_tags = negated_tags.map(&:name)
optional_tags = TagAlias.to_aliased(optional_tags.map(&:name)) optional_tags = optional_tags.map(&:name)
required_tags = TagAlias.to_aliased(required_tags.map(&:name)) required_tags = required_tags.map(&:name)
negated_tags += negated_wildcard_tags.flat_map { |tag| Tag.wildcard_matches(tag.name) } negated_tags += negated_wildcard_tags.flat_map { |tag| Tag.wildcard_matches(tag.name) }
optional_tags += optional_wildcard_tags.flat_map { |tag| Tag.wildcard_matches(tag.name) } optional_tags += optional_wildcard_tags.flat_map { |tag| Tag.wildcard_matches(tag.name) }
@@ -671,7 +674,7 @@ class PostQueryBuilder
end end
def split_query def split_query
scan_query.map do |term| terms.map do |term|
if term.type == :metatag && !term.negated && !term.quoted if term.type == :metatag && !term.negated && !term.quoted
"#{term.name}:#{term.value}" "#{term.name}:#{term.value}"
elsif term.type == :metatag && !term.negated && term.quoted elsif term.type == :metatag && !term.negated && term.quoted
@@ -690,13 +693,18 @@ class PostQueryBuilder
end end
end end
def normalize_query(normalize_aliases: false, sort: true) def normalize_aliases!
tags = split_query tag_names = tags.map(&:name)
tags = tags.map { |t| Tag.normalize_name(t) } tag_aliases = tag_names.zip(TagAlias.to_aliased(tag_names)).to_h
tags = TagAlias.to_aliased(tags) if normalize_aliases
tags = tags.sort if sort terms.map! do |term|
tags = tags.uniq term.name = tag_aliases[term.name] if term.type == :tag
tags.join(" ") term
end
end
def normalize_order!
terms.sort_by!(&:to_s).uniq!
end end
def parse_tag_edit def parse_tag_edit
@@ -816,7 +824,7 @@ class PostQueryBuilder
concerning :CountMethods do concerning :CountMethods do
def fast_count(timeout: 1_000, raise_on_timeout: false, skip_cache: false) def fast_count(timeout: 1_000, raise_on_timeout: false, skip_cache: false)
tags = normalize_query(normalize_aliases: true) tags = to_s
tags += " rating:s" if safe_mode? tags += " rating:s" if safe_mode?
tags += " -status:deleted" if hide_deleted? tags += " -status:deleted" if hide_deleted?
tags = tags.strip tags = tags.strip
@@ -876,7 +884,7 @@ class PostQueryBuilder
end end
def get_count_from_cache(tags) def get_count_from_cache(tags)
if PostQueryBuilder.new(tags).is_simple_tag? if PostQueryBuilder.new(tags, normalize_aliases: false).is_simple_tag?
count = Tag.find_by(name: tags).try(:post_count) count = Tag.find_by(name: tags).try(:post_count)
else else
# this will only have a value for multi-tag searches or single metatag searches # this will only have a value for multi-tag searches or single metatag searches
@@ -898,16 +906,16 @@ class PostQueryBuilder
end end
concerning :UtilityMethods do concerning :UtilityMethods do
def terms def to_s
scan_query split_query.join(" ")
end end
def tags def tags
scan_query.select { |term| term.type == :tag } terms.select { |term| term.type == :tag }
end end
def metatags def metatags
scan_query.select { |term| term.type == :metatag } terms.select { |term| term.type == :metatag }
end end
def select_metatags(*names) def select_metatags(*names)
@@ -935,11 +943,11 @@ class PostQueryBuilder
end end
def is_empty_search? def is_empty_search?
scan_query.size == 0 terms.size == 0
end end
def is_single_term? def is_single_term?
scan_query.size == 1 terms.size == 1
end end
def is_single_tag? def is_single_tag?
@@ -956,5 +964,5 @@ class PostQueryBuilder
end end
end end
memoize :scan_query, :split_query, :normalize_query memoize :split_query
end end

View File

@@ -103,7 +103,7 @@ module PostSets
if is_random? if is_random?
temp = get_random_posts temp = get_random_posts
else else
temp = ::Post.user_tag_match(tag_string).where("true /* PostSets::Post#posts:2 */").paginate(page, :count => post_count, :limit => per_page) temp = query.build.paginate(page, count: post_count, limit: per_page)
end end
end end
end end

View File

@@ -56,7 +56,7 @@ class RelatedTagQuery
versions = PostVersion.where(updater_id: user.id).where("updated_at > ?", since).order(id: :desc).limit(max_edits) versions = PostVersion.where(updater_id: user.id).where("updated_at > ?", since).order(id: :desc).limit(max_edits)
tags = versions.flat_map(&:added_tags) tags = versions.flat_map(&:added_tags)
tags = tags.select { |tag| PostQueryBuilder.new(tag).is_simple_tag? } tags = tags.select { |tag| PostQueryBuilder.new(tag, normalize_aliases: false).is_simple_tag? }
tags = tags.group_by(&:itself).transform_values(&:size).sort_by { |tag, count| [-count, tag] }.map(&:first) tags = tags.group_by(&:itself).transform_values(&:size).sort_by { |tag, count| [-count, tag] }.map(&:first)
tags.take(max_tags) tags.take(max_tags)
end end

View File

@@ -604,7 +604,7 @@ class Post < ApplicationRecord
# If someone else committed changes to this post before we did, # If someone else committed changes to this post before we did,
# then try to merge the tag changes together. # then try to merge the tag changes together.
current_tags = tag_string_was.split current_tags = tag_string_was.split
new_tags = PostQueryBuilder.new(tag_string).split_query new_tags = PostQueryBuilder.new(tag_string, normalize_aliases: false).parse_tag_edit
old_tags = old_tag_string.split old_tags = old_tag_string.split
kept_tags = current_tags & new_tags kept_tags = current_tags & new_tags
@@ -642,7 +642,7 @@ class Post < ApplicationRecord
end end
def normalize_tags def normalize_tags
normalized_tags = PostQueryBuilder.new(tag_string).split_query normalized_tags = PostQueryBuilder.new(tag_string, normalize_aliases: false).parse_tag_edit
normalized_tags = apply_casesensitive_metatags(normalized_tags) normalized_tags = apply_casesensitive_metatags(normalized_tags)
normalized_tags = normalized_tags.map(&:downcase) normalized_tags = normalized_tags.map(&:downcase)
normalized_tags = filter_metatags(normalized_tags) normalized_tags = filter_metatags(normalized_tags)

View File

@@ -134,17 +134,17 @@ class SavedSearch < ApplicationRecord
def queries_for(user_id, label: nil, options: {}) def queries_for(user_id, label: nil, options: {})
searches = SavedSearch.where(user_id: user_id) searches = SavedSearch.where(user_id: user_id)
searches = searches.labeled(label) if label.present? searches = searches.labeled(label) if label.present?
queries = searches.pluck(:query).map { |query| PostQueryBuilder.new(query).normalize_query(normalize_aliases: true, sort: true) } queries = searches.pluck(:query).map { |query| PostQueryBuilder.new(query).to_s }
queries.sort.uniq queries.sort.uniq
end end
end end
def normalized_query def normalized_query
PostQueryBuilder.new(query).normalize_query(sort: true) PostQueryBuilder.new(query, normalize_aliases: false).to_s
end end
def normalize_query def normalize_query
self.query = PostQueryBuilder.new(query).normalize_query(normalize_aliases: true, sort: false) self.query = PostQueryBuilder.new(query, normalize_order: false).to_s
end end
end end

View File

@@ -237,7 +237,7 @@ class Upload < ApplicationRecord
include SourceMethods include SourceMethods
def assign_rating_from_tags def assign_rating_from_tags
if rating = PostQueryBuilder.new(tag_string).find_metatag(:rating) if rating = PostQueryBuilder.new(tag_string, normalize_aliases: false).find_metatag(:rating)
self.rating = rating.downcase.first self.rating = rating.downcase.first
end end
end end

View File

@@ -163,7 +163,7 @@ class PostPresenter
end end
def has_sequential_navigation?(params) def has_sequential_navigation?(params)
return false if PostQueryBuilder.new(params[:q]).has_metatag?(:order, :ordfav, :ordpool) return false if PostQueryBuilder.new(params[:q], normalize_aliases: false).has_metatag?(:order, :ordfav, :ordpool)
return false if params[:pool_id].present? || params[:favgroup_id].present? return false if params[:pool_id].present? || params[:favgroup_id].present?
return CurrentUser.user.enable_sequential_post_navigation return CurrentUser.user.enable_sequential_post_navigation
end end

View File

@@ -51,9 +51,9 @@
<% if post_search_counts_enabled? && @post_set.query.is_simple_tag? && @post_set.current_page == 1 %> <% if post_search_counts_enabled? && @post_set.query.is_simple_tag? && @post_set.current_page == 1 %>
<% if @post_set.post_count == 0 %> <% if @post_set.post_count == 0 %>
<%= missed_post_search_count_js(@post_set) %> <%= missed_post_search_count_js(@post_set.query.to_s) %>
<% else %> <% else %>
<%= post_search_count_js(@post_set) %> <%= post_search_count_js(@post_set.query.to_s) %>
<% end %> <% end %>
<% end %> <% end %>

View File

@@ -20,7 +20,7 @@
<% end %> <% end %>
<% if CurrentUser.user.is_member? && post_set.tag.present? && post_set.current_page == 1 %> <% if CurrentUser.user.is_member? && post_set.tag.present? && post_set.current_page == 1 %>
<% cache("tag-change-notice:#{post_set.query.normalize_query}", expires_in: 4.hours) do %> <% cache("tag-change-notice:#{post_set.tag.name}", expires_in: 4.hours) do %>
<% if post_set.pending_bulk_update_requests.present? %> <% if post_set.pending_bulk_update_requests.present? %>
<div class="fineprint tag-change-notice"> <div class="fineprint tag-change-notice">
<p> <p>

View File

@@ -68,6 +68,17 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
assert_select "#show-excerpt-link", count: 0 assert_select "#show-excerpt-link", count: 0
end end
should "render for an aliased tag" do
create(:tag_alias, antecedent_name: "/lav", consequent_name: "looking_at_viewer")
as(@user) { create(:wiki_page, title: "looking_at_viewer") }
@post = create(:post, tag_string: "looking_at_viewer", rating: "s")
get posts_path, params: { tags: "/lav" }
assert_response :success
assert_select "#post_#{@post.id}", count: 1
assert_select "#excerpt .wiki-link[href='/wiki_pages/looking_at_viewer']", count: 1
end
should "render for a wildcard tag search" do should "render for a wildcard tag search" do
create(:post, tag_string: "1girl solo") create(:post, tag_string: "1girl solo")
get posts_path(tags: "*girl*") get posts_path(tags: "*girl*")

View File

@@ -1018,7 +1018,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
context "Parsing:" do context "Parsing:" do
should "split a query" do should "split a query" do
assert_equal(%w(aaa bbb), PostQueryBuilder.new("aaa bbb").split_query) assert_equal(%w(aaa bbb), PostQueryBuilder.new("aaa bbb").split_query)
assert_equal(%w(~aaa -bbb* -bbb*), PostQueryBuilder.new("~AAa -BBB* -bbb*").split_query) assert_equal(%w(~aaa -bbb*), PostQueryBuilder.new("~AAa -BBB* -bbb*").split_query)
end end
should "not strip out valid characters when scanning" do should "not strip out valid characters when scanning" do
@@ -1054,19 +1054,18 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
should "work" do should "work" do
create(:tag_alias, antecedent_name: "gray", consequent_name: "grey") create(:tag_alias, antecedent_name: "gray", consequent_name: "grey")
assert_equal("foo", PostQueryBuilder.new("foo").normalize_query) assert_equal("foo", PostQueryBuilder.new("foo").to_s)
assert_equal("foo", PostQueryBuilder.new(" foo ").normalize_query) assert_equal("foo", PostQueryBuilder.new(" foo ").to_s)
assert_equal("foo", PostQueryBuilder.new("FOO").normalize_query) assert_equal("foo", PostQueryBuilder.new("FOO").to_s)
assert_equal("foo", PostQueryBuilder.new("foo foo").normalize_query) assert_equal("foo", PostQueryBuilder.new("foo foo").to_s)
assert_equal("gray", PostQueryBuilder.new("gray").normalize_query) assert_equal("grey", PostQueryBuilder.new("gray").to_s)
assert_equal("grey", PostQueryBuilder.new("gray").normalize_query(normalize_aliases: true)) assert_equal("aaa bbb", PostQueryBuilder.new("bbb aaa").to_s)
assert_equal("aaa bbb", PostQueryBuilder.new("bbb aaa").normalize_query) assert_equal("-aaa bbb", PostQueryBuilder.new("bbb -aaa").to_s)
assert_equal("-aaa bbb", PostQueryBuilder.new("bbb -aaa").normalize_query) assert_equal("~aaa ~bbb", PostQueryBuilder.new("~bbb ~aaa").to_s)
assert_equal("~aaa ~bbb", PostQueryBuilder.new("~bbb ~aaa").normalize_query) assert_equal("commentary:true bbb", PostQueryBuilder.new("bbb commentary:true").to_s)
assert_equal("bbb commentary:true", PostQueryBuilder.new("bbb commentary:true").normalize_query) assert_equal('commentary:"true" bbb', PostQueryBuilder.new("bbb commentary:'true'").to_s)
assert_equal('bbb commentary:"true"', PostQueryBuilder.new("bbb commentary:'true'").normalize_query) assert_equal('-commentary:true bbb', PostQueryBuilder.new("bbb -commentary:true").to_s)
assert_equal('-commentary:true bbb', PostQueryBuilder.new("bbb -commentary:true").normalize_query) assert_equal('-commentary:"true" bbb', PostQueryBuilder.new("bbb -commentary:'true'").to_s)
assert_equal('-commentary:"true" bbb', PostQueryBuilder.new("bbb -commentary:'true'").normalize_query)
end end
end end
@@ -1107,7 +1106,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
context "for a multi-tag search" do context "for a multi-tag search" do
should "return the cached count, if it exists" do should "return the cached count, if it exists" do
PostQueryBuilder.new(nil).set_count_in_cache("aaa score:42", 100) PostQueryBuilder.new(nil).set_count_in_cache("score:42 aaa", 100)
assert_fast_count(100, "aaa score:42") assert_fast_count(100, "aaa score:42")
end end
@@ -1116,7 +1115,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
end end
should "set the expiration time" do should "set the expiration time" do
Cache.expects(:put).with(PostQueryBuilder.new(nil).count_cache_key("aaa score:42"), 1, 180) Cache.expects(:put).with(PostQueryBuilder.new(nil).count_cache_key("score:42 aaa"), 1, 180)
PostQueryBuilder.new("aaa score:42").fast_count PostQueryBuilder.new("aaa score:42").fast_count
end end