From d3e4ac7c177486e962cdec328843db9568960f9a Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 7 May 2020 17:05:44 -0500 Subject: [PATCH] search: clean up safe_mode / hide_deleted_posts settings. Change PostQueryBuilder to add rating:s and -status:deleted to the search inside the constructor instead of inside `#build` and `#fast_count`. This lets up clean up `#fast_count` so it doesn't have to reparse the query after adding these tags. This caused aliases to be evaluated more than once on the post index page. --- app/logical/post_query_builder.rb | 40 +++++++++++++--------------- app/logical/post_sets/post.rb | 2 +- app/logical/related_tag_query.rb | 2 +- app/models/saved_search.rb | 4 +-- test/unit/post_query_builder_test.rb | 19 +++++++------ 5 files changed, 32 insertions(+), 35 deletions(-) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 36b8a3eaa..c79766e30 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -62,7 +62,11 @@ class PostQueryBuilder @current_user = current_user @safe_mode = safe_mode @hide_deleted_posts = hide_deleted_posts + @terms = scan_query + @terms << OpenStruct.new(type: :metatag, name: "rating", value: "s") if safe_mode? + @terms << OpenStruct.new(type: :metatag, name: "status", value: "deleted", negated: true) if hide_deleted? + normalize_aliases! if normalize_aliases normalize_order! if normalize_order end @@ -465,8 +469,6 @@ class PostQueryBuilder end relation = Post.all - relation = relation.where(rating: 's') if safe_mode? - relation = relation.undeleted if hide_deleted? relation = add_joins(relation) relation = metatags_match(metatags, relation) relation = tags_match(tags, relation) @@ -825,9 +827,6 @@ class PostQueryBuilder concerning :CountMethods do def fast_count(timeout: 1_000, raise_on_timeout: false, skip_cache: false) tags = to_s - tags += " rating:s" if safe_mode? - tags += " -status:deleted" if hide_deleted? - tags = tags.strip # Optimize some cases. these are just estimates but at these # quantities being off by a few hundred doesn't matter much @@ -850,11 +849,11 @@ class PostQueryBuilder count = nil unless skip_cache - count = get_count_from_cache(tags) + count = get_count_from_cache end if count.nil? - count = fast_count_search(tags, timeout: timeout, raise_on_timeout: raise_on_timeout) + count = fast_count_search(timeout: timeout, raise_on_timeout: raise_on_timeout) end count @@ -862,9 +861,9 @@ class PostQueryBuilder 0 end - def fast_count_search(tags, timeout:, raise_on_timeout:) - count = Post.with_timeout(timeout, nil, tags: tags) do - Post.user_tag_match(tags).count + def fast_count_search(timeout:, raise_on_timeout:) + count = Post.with_timeout(timeout, nil) do + build.count end if count.nil? @@ -875,7 +874,7 @@ class PostQueryBuilder count = Danbooru.config.blank_tag_search_fast_count else - set_count_in_cache(tags, count) + set_count_in_cache(count) end count ? count.to_i : nil @@ -883,25 +882,24 @@ class PostQueryBuilder return nil end - def get_count_from_cache(tags) - if PostQueryBuilder.new(tags, normalize_aliases: false).is_simple_tag? - count = Tag.find_by(name: tags).try(:post_count) + def get_count_from_cache + if is_simple_tag? + count = Tag.find_by(name: tags.first.name).try(:post_count) else # this will only have a value for multi-tag searches or single metatag searches - count = Cache.get(count_cache_key(tags)) + count = Cache.get(count_cache_key) end count.try(:to_i) end - def set_count_in_cache(tags, count, expiry = nil) - expiry ||= count.seconds.clamp(3.minutes, 20.hours).to_i - - Cache.put(count_cache_key(tags), count, expiry) + def set_count_in_cache(count) + expiry = count.seconds.clamp(3.minutes, 20.hours).to_i + Cache.put(count_cache_key, count, expiry) end - def count_cache_key(tags) - "pfc:#{Cache.hash(tags)}" + def count_cache_key + "pfc:#{Cache.hash(to_s)}" end end diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index fc8f0d836..3f25df422 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -4,7 +4,7 @@ module PostSets attr_reader :page, :random, :post_count, :format, :tag_string, :query def initialize(tags, page = 1, per_page = nil, random: false, format: "html") - @query = PostQueryBuilder.new(tags, CurrentUser.user) + @query = PostQueryBuilder.new(tags, CurrentUser.user, safe_mode: CurrentUser.safe_mode?, hide_deleted_posts: CurrentUser.hide_deleted_posts?) @tag_string = tags @page = page @per_page = per_page diff --git a/app/logical/related_tag_query.rb b/app/logical/related_tag_query.rb index de6c4dc01..e14eeece4 100644 --- a/app/logical/related_tag_query.rb +++ b/app/logical/related_tag_query.rb @@ -56,7 +56,7 @@ class RelatedTagQuery versions = PostVersion.where(updater_id: user.id).where("updated_at > ?", since).order(id: :desc).limit(max_edits) tags = versions.flat_map(&:added_tags) - tags = tags.select { |tag| PostQueryBuilder.new(tag, normalize_aliases: false).is_simple_tag? } + tags = tags.reject { |tag| tag.match?(/\A(source:|parent:|rating:)/) } tags = tags.group_by(&:itself).transform_values(&:size).sort_by { |tag, count| [-count, tag] }.map(&:first) tags.take(max_tags) end diff --git a/app/models/saved_search.rb b/app/models/saved_search.rb index 9c37f03d3..6849ba1d9 100644 --- a/app/models/saved_search.rb +++ b/app/models/saved_search.rb @@ -134,13 +134,13 @@ class SavedSearch < ApplicationRecord def queries_for(user_id, label: nil, options: {}) searches = SavedSearch.where(user_id: user_id) searches = searches.labeled(label) if label.present? - queries = searches.pluck(:query).map { |query| PostQueryBuilder.new(query).to_s } + queries = searches.map(&:normalized_query) queries.sort.uniq end end def normalized_query - PostQueryBuilder.new(query, normalize_aliases: false).to_s + PostQueryBuilder.new(query).to_s end def normalize_query diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index 205fde052..73e73f5e3 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -1093,20 +1093,20 @@ class PostQueryBuilderTest < ActiveSupport::TestCase context "for a single metatag" do should "return the correct cached count" do build(:tag, name: "score:42", post_count: -100).save(validate: false) - PostQueryBuilder.new(nil).set_count_in_cache("score:42", 100) + PostQueryBuilder.new("score:42").set_count_in_cache(100) assert_fast_count(100, "score:42") end should "return the correct cached count for a pool: search" do build(:tag, name: "pool:1234", post_count: -100).save(validate: false) - PostQueryBuilder.new(nil).set_count_in_cache("pool:1234", 100) + PostQueryBuilder.new("pool:1234").set_count_in_cache(100) assert_fast_count(100, "pool:1234") end end context "for a multi-tag search" do should "return the cached count, if it exists" do - PostQueryBuilder.new(nil).set_count_in_cache("score:42 aaa", 100) + PostQueryBuilder.new("score:42 aaa").set_count_in_cache(100) assert_fast_count(100, "aaa score:42") end @@ -1115,15 +1115,14 @@ class PostQueryBuilderTest < ActiveSupport::TestCase end should "set the expiration time" do - Cache.expects(:put).with(PostQueryBuilder.new(nil).count_cache_key("score:42 aaa"), 1, 180) + Cache.expects(:put).with(PostQueryBuilder.new("score:42 aaa").count_cache_key, 1, 180) PostQueryBuilder.new("aaa score:42").fast_count end should "work with the hide_deleted_posts option turned on" do - user = create(:user, hide_deleted_posts: true) - as(user) do - assert_fast_count(1, "aaa score:42") - end + create(:post, tag_string: "aaa", score: 42, is_deleted: true) + assert_fast_count(1, "aaa score:42", hide_deleted_posts: true) + assert_fast_count(2, "aaa score:42", hide_deleted_posts: false) end end @@ -1134,7 +1133,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase context "with a primed cache" do should "fetch the value from the cache" do - PostQueryBuilder.new(nil).set_count_in_cache("", 100) + PostQueryBuilder.new("").set_count_in_cache(100) assert_fast_count(100, "") end end @@ -1165,7 +1164,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase context "with a primed cache" do should "fetch the value from the cache" do - PostQueryBuilder.new(nil).set_count_in_cache("rating:s", 100) + PostQueryBuilder.new("rating:s").set_count_in_cache(100) assert_fast_count(100, "", safe_mode: true) end end