From a753ebbea9f7d1b40c1a3701a031816494e7cced Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 6 May 2020 14:47:16 -0500 Subject: [PATCH] posts: move fast_count to PostQueryBuilder. --- app/controllers/counts_controller.rb | 7 +- app/logical/post_query_builder.rb | 83 ++++++++++++++ app/logical/post_sets/post.rb | 2 +- app/logical/related_tag_calculator.rb | 2 +- app/models/post.rb | 100 ++--------------- app/presenters/user_presenter.rb | 4 +- test/functional/counts_controller_test.rb | 2 +- test/unit/post_query_builder_test.rb | 109 ++++++++++++++++++ test/unit/post_test.rb | 131 ---------------------- 9 files changed, 206 insertions(+), 234 deletions(-) diff --git a/app/controllers/counts_controller.rb b/app/controllers/counts_controller.rb index 6c4a758c1..9dc6a2461 100644 --- a/app/controllers/counts_controller.rb +++ b/app/controllers/counts_controller.rb @@ -2,11 +2,6 @@ class CountsController < ApplicationController respond_to :xml, :json def posts - @count = Post.fast_count( - params[:tags], - timeout: CurrentUser.statement_timeout, - raise_on_timeout: true, - skip_cache: params[:skip_cache] - ) + @count = PostQueryBuilder.new(params[:tags]).fast_count(timeout: CurrentUser.statement_timeout, raise_on_timeout: true, skip_cache: params[:skip_cache]) end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 4455e2dc7..bb1bcfe27 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -810,6 +810,89 @@ class PostQueryBuilder end end + concerning :CountMethods do + def fast_count(timeout: 1_000, raise_on_timeout: false, skip_cache: false) + tags = normalize_query(normalize_aliases: true) + tags += " rating:s" if CurrentUser.safe_mode? + tags += " -status:deleted" if CurrentUser.hide_deleted_posts? && !has_metatag?("status") + tags = tags.strip + + # Optimize some cases. these are just estimates but at these + # quantities being off by a few hundred doesn't matter much + if Danbooru.config.estimate_post_counts + if tags == "" + return (Post.maximum(:id).to_i * (2200402.0 / 2232212)).floor + + elsif tags =~ /^rating:s(?:afe)?$/ + return (Post.maximum(:id).to_i * (1648652.0 / 2200402)).floor + + elsif tags =~ /^rating:q(?:uestionable)?$/ + return (Post.maximum(:id).to_i * (350101.0 / 2200402)).floor + + elsif tags =~ /^rating:e(?:xplicit)?$/ + return (Post.maximum(:id).to_i * (201650.0 / 2200402)).floor + + end + end + + count = nil + + unless skip_cache + count = get_count_from_cache(tags) + end + + if count.nil? + count = fast_count_search(tags, timeout: timeout, raise_on_timeout: raise_on_timeout) + end + + count + rescue Post::SearchError + 0 + end + + def fast_count_search(tags, timeout:, raise_on_timeout:) + count = Post.with_timeout(timeout, nil, tags: tags) do + Post.tag_match(tags).count + end + + if count.nil? + # give up + if raise_on_timeout + raise TimeoutError.new("timed out") + end + + count = Danbooru.config.blank_tag_search_fast_count + else + set_count_in_cache(tags, count) + end + + count ? count.to_i : nil + rescue PG::ConnectionBad + return nil + end + + def get_count_from_cache(tags) + if PostQueryBuilder.new(tags).is_simple_tag? + count = Tag.find_by(name: tags).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)) + 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) + end + + def count_cache_key(tags) + "pfc:#{Cache.hash(tags)}" + end + end + concerning :UtilityMethods do def terms scan_query diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index efeac657f..72184e0ed 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -86,7 +86,7 @@ module PostSets # no need to get counts for formats that don't use a paginator return Danbooru.config.blank_tag_search_fast_count else - ::Post.fast_count(tag_string) + query.fast_count end end diff --git a/app/logical/related_tag_calculator.rb b/app/logical/related_tag_calculator.rb index 898a2b308..c66fd3149 100644 --- a/app/logical/related_tag_calculator.rb +++ b/app/logical/related_tag_calculator.rb @@ -1,6 +1,6 @@ module RelatedTagCalculator def self.similar_tags_for_search(tag_query, search_sample_size: 1000, tag_sample_size: 250, category: nil) - search_count = Post.fast_count(tag_query) + search_count = PostQueryBuilder.new(tag_query).fast_count return [] if search_count.nil? search_sample_size = [search_count, search_sample_size].min diff --git a/app/models/post.rb b/app/models/post.rb index 72e5cee8a..e3b0e2d9d 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -588,6 +588,14 @@ class Post < ApplicationRecord end end + def fix_post_counts(post) + post.set_tag_counts(false) + if post.changes_saved? + args = Hash[TagCategory.categories.map {|x| ["tag_count_#{x}", post.send("tag_count_#{x}")]}].update(:tag_count => post.tag_count) + post.update_columns(args) + end + end + def merge_old_changes reset_tag_array_cache @removed_tags = [] @@ -1065,97 +1073,6 @@ class Post < ApplicationRecord end end - module CountMethods - def fast_count(tags = "", timeout: 1_000, raise_on_timeout: false, skip_cache: false) - tags = tags.to_s - tags += " rating:s" if CurrentUser.safe_mode? - tags += " -status:deleted" if CurrentUser.hide_deleted_posts? && !PostQueryBuilder.new(tags).has_metatag?("status") - tags = PostQueryBuilder.new(tags).normalize_query(normalize_aliases: true) - - # Optimize some cases. these are just estimates but at these - # quantities being off by a few hundred doesn't matter much - if Danbooru.config.estimate_post_counts - if tags == "" - return (Post.maximum(:id).to_i * (2200402.0 / 2232212)).floor - - elsif tags =~ /^rating:s(?:afe)?$/ - return (Post.maximum(:id).to_i * (1648652.0 / 2200402)).floor - - elsif tags =~ /^rating:q(?:uestionable)?$/ - return (Post.maximum(:id).to_i * (350101.0 / 2200402)).floor - - elsif tags =~ /^rating:e(?:xplicit)?$/ - return (Post.maximum(:id).to_i * (201650.0 / 2200402)).floor - - end - end - - count = nil - - unless skip_cache - count = get_count_from_cache(tags) - end - - if count.nil? - count = fast_count_search(tags, timeout: timeout, raise_on_timeout: raise_on_timeout) - end - - count - rescue SearchError - 0 - end - - def fast_count_search(tags, timeout:, raise_on_timeout:) - count = Post.with_timeout(timeout, nil, tags: tags) do - Post.tag_match(tags).count - end - - if count.nil? - # give up - if raise_on_timeout - raise TimeoutError.new("timed out") - end - - count = Danbooru.config.blank_tag_search_fast_count - else - set_count_in_cache(tags, count) - end - - count ? count.to_i : nil - rescue PG::ConnectionBad - return nil - end - - def fix_post_counts(post) - post.set_tag_counts(false) - if post.changes_saved? - args = Hash[TagCategory.categories.map {|x| ["tag_count_#{x}", post.send("tag_count_#{x}")]}].update(:tag_count => post.tag_count) - post.update_columns(args) - end - end - - def get_count_from_cache(tags) - if PostQueryBuilder.new(tags).is_simple_tag? - count = Tag.find_by(name: tags).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)) - 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) - end - - def count_cache_key(tags) - "pfc:#{Cache.hash(tags)}" - end - end - module ParentMethods # A parent has many children. A child belongs to a parent. # A parent cannot have a parent. @@ -1715,7 +1632,6 @@ class Post < ApplicationRecord include FavoriteMethods include PoolMethods include VoteMethods - extend CountMethods include ParentMethods include DeletionMethods include VersionMethods diff --git a/app/presenters/user_presenter.rb b/app/presenters/user_presenter.rb index 650aa5287..f9f958c80 100644 --- a/app/presenters/user_presenter.rb +++ b/app/presenters/user_presenter.rb @@ -76,7 +76,7 @@ class UserPresenter end def commented_posts_count(template) - count = CurrentUser.without_safe_mode { Post.fast_count("commenter:#{user.name}") } + count = CurrentUser.without_safe_mode { PostQueryBuilder.new("commenter:#{user.name}").fast_count } count = "?" if count.nil? template.link_to(count, template.posts_path(:tags => "commenter:#{user.name} order:comment_bumped")) end @@ -90,7 +90,7 @@ class UserPresenter end def noted_posts_count(template) - count = CurrentUser.without_safe_mode { Post.fast_count("noteupdater:#{user.name}") } + count = CurrentUser.without_safe_mode { PostQueryBuilder.new("noteupdater:#{user.name}").fast_count } count = "?" if count.nil? template.link_to(count, template.posts_path(:tags => "noteupdater:#{user.name} order:note")) end diff --git a/test/functional/counts_controller_test.rb b/test/functional/counts_controller_test.rb index 323339eb6..5aae7a945 100644 --- a/test/functional/counts_controller_test.rb +++ b/test/functional/counts_controller_test.rb @@ -9,7 +9,7 @@ class CountsControllerTest < ActionDispatch::IntegrationTest end should "render an error during a timeout" do - Post.stubs(:fast_count).raises(Post::TimeoutError.new) + PostQueryBuilder.any_instance.stubs(:fast_count).raises(Post::TimeoutError.new) get posts_counts_path assert_response :error end diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index fe2eeb955..418fb960d 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -5,6 +5,10 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_equal(posts.map(&:id), Post.tag_match(query).pluck(:id)) end + def assert_fast_count(count, query, **options) + assert_equal(count, PostQueryBuilder.new(query).fast_count(**options)) + end + setup do CurrentUser.user = create(:user) CurrentUser.ip_addr = "127.0.0.1" @@ -1062,4 +1066,109 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_equal('-commentary:"true" bbb', PostQueryBuilder.new("bbb -commentary:'true'").normalize_query) end end + + context "#fast_count" do + setup do + Danbooru.config.stubs(:blank_tag_search_fast_count).returns(nil) + Danbooru.config.stubs(:estimate_post_counts).returns(false) + create(:tag, name: "grey_skirt", post_count: 100) + create(:tag_alias, antecedent_name: "gray_skirt", consequent_name: "grey_skirt") + create(:post, tag_string: "aaa", score: 42) + end + + context "for a single basic tag" do + should "return the post_count from the tags table" do + assert_fast_count(100, "grey_skirt") + end + end + + context "for a aliased tag" do + should "return the post count of the consequent tag" do + assert_fast_count(100, "gray_skirt") + end + end + + 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) + 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) + 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("aaa score:42", 100) + assert_fast_count(100, "aaa score:42") + end + + should "return the true count, if not cached" do + assert_fast_count(1, "aaa score:42") + end + + should "set the expiration time" do + Cache.expects(:put).with(PostQueryBuilder.new(nil).count_cache_key("aaa score:42"), 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 + end + end + + context "a blank search" do + should "should execute a search" do + assert_fast_count(1, "") + end + + context "with a primed cache" do + should "fetch the value from the cache" do + PostQueryBuilder.new(nil).set_count_in_cache("", 100) + assert_fast_count(100, "") + end + end + + should "return 0 for a nonexisting tag" do + assert_fast_count(0, "bbb") + end + + context "in safe mode" do + setup do + CurrentUser.stubs(:safe_mode?).returns(true) + create(:post, rating: "s") + end + + should "work for a blank search" do + assert_fast_count(1, "") + end + + should "work for a nil search" do + assert_fast_count(1, nil) + end + + should "not fail for a two tag search by a member" do + post1 = create(:post, tag_string: "aaa bbb rating:s") + post2 = create(:post, tag_string: "aaa bbb rating:e") + + assert_fast_count(1, "aaa bbb") + end + + context "with a primed cache" do + should "fetch the value from the cache" do + PostQueryBuilder.new(nil).set_count_in_cache("rating:s", 100) + assert_fast_count(100, "") + end + end + end + end + end end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index ddcb73e7a..b9e4da568 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1931,137 +1931,6 @@ class PostTest < ActiveSupport::TestCase end end - context "Counting:" do - context "Creating a post" do - setup do - Danbooru.config.stubs(:blank_tag_search_fast_count).returns(nil) - Danbooru.config.stubs(:estimate_post_counts).returns(false) - FactoryBot.create(:tag_alias, :antecedent_name => "alias", :consequent_name => "aaa") - FactoryBot.create(:post, :tag_string => "aaa", "score" => 42) - end - - context "a single basic tag" do - should "return the cached count" do - Tag.find_or_create_by_name("aaa").update_columns(post_count: 100) - assert_equal(100, Post.fast_count("aaa")) - end - end - - context "an aliased tag" do - should "return the count of the consequent tag" do - assert_equal(Post.fast_count("aaa"), Post.fast_count("alias")) - end - end - - context "a single metatag" do - should "return the correct cached count" do - FactoryBot.build(:tag, name: "score:42", post_count: -100).save(validate: false) - Post.set_count_in_cache("score:42", 100) - - assert_equal(100, Post.fast_count("score:42")) - end - - should "return the correct cached count for a pool: search" do - FactoryBot.build(:tag, name: "pool:1234", post_count: -100).save(validate: false) - Post.set_count_in_cache("pool:1234", 100) - - assert_equal(100, Post.fast_count("pool:1234")) - end - end - - context "a multi-tag search" do - should "return the cached count, if it exists" do - Post.set_count_in_cache("aaa score:42", 100) - assert_equal(100, Post.fast_count("aaa score:42")) - end - - should "return the true count, if not cached" do - assert_equal(1, Post.fast_count("aaa score:42")) - end - - should "set the expiration time" do - Cache.expects(:put).with(Post.count_cache_key("aaa score:42"), 1, 180) - Post.fast_count("aaa score:42") - end - - should "work with the hide_deleted_posts option turned on" do - user = create(:user, hide_deleted_posts: true) - as(user) do - assert_equal(1, Post.fast_count("aaa score:42")) - end - end - end - - context "a blank search" do - should "should execute a search" do - Cache.delete(Post.count_cache_key('')) - Post.expects(:fast_count_search).with("", kind_of(Hash)).once.returns(1) - assert_equal(1, Post.fast_count("")) - end - - should "set the value in cache" do - Post.expects(:set_count_in_cache).with("", kind_of(Integer)).once - Post.fast_count("") - end - - context "with a primed cache" do - setup do - Cache.put(Post.count_cache_key(''), "100") - end - - should "fetch the value from the cache" do - assert_equal(100, Post.fast_count("")) - end - end - - should_eventually "translate an alias" do - assert_equal(1, Post.fast_count("alias")) - end - - should "return 0 for a nonexisting tag" do - assert_equal(0, Post.fast_count("bbb")) - end - - context "in safe mode" do - setup do - CurrentUser.stubs(:safe_mode?).returns(true) - FactoryBot.create(:post, "rating" => "s") - end - - should "work for a blank search" do - assert_equal(1, Post.fast_count("")) - end - - should "work for a nil search" do - assert_equal(1, Post.fast_count(nil)) - end - - should "not fail for a two tag search by a member" do - post1 = FactoryBot.create(:post, tag_string: "aaa bbb rating:s") - post2 = FactoryBot.create(:post, tag_string: "aaa bbb rating:e") - - assert_equal(1, Post.fast_count("aaa bbb")) - end - - should "set the value in cache" do - Post.expects(:set_count_in_cache).with("rating:s", kind_of(Integer)).once - Post.fast_count("") - end - - context "with a primed cache" do - setup do - Cache.put(Post.count_cache_key('rating:s'), "100") - end - - should "fetch the value from the cache" do - assert_equal(100, Post.fast_count("")) - end - end - end - end - end - end - context "Reverting: " do context "a post that is rating locked" do setup do