From f38c38f26ecbe3143520347317645d3b2a88dd14 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 6 May 2020 22:00:47 -0500 Subject: [PATCH] search: split tag_match into user_tag_match / system_tag_match. When doing a tag search, we have to be careful about which user we're running the search as because the results depend on the current user. Specifically, things like private favorites, private favorite groups, post votes, saved searches, and flagger names depend on the user's permissions, and whether non-safe or deleted posts are filtered out depend on whether the user has safe mode on or the hide deleted posts setting enabled. * Refactor internal searches to explicitly state whether they're running as the system user (DanbooruBot) or as the current user. * Explicitly pass in the current user to PostQueryBuilder instead of implicitly relying on the CurrentUser global. * Get rid of CurrentUser.admin_mode? (used to ignore the hide deleted post setting) and CurrentUser.without_safe_mode (used to ignore safe mode). * Change the /counts/posts.json endpoint to ignore safe mode and the hide deleted posts settings when counting posts. * Fix searches not correctly overriding the hide deleted posts setting when multiple status: metatags were used (e.g. `status:banned status:active`) * Fix fast_count not respecting the hide deleted posts setting when the status:banned metatag was used. --- app/controllers/comments_controller.rb | 2 +- app/controllers/counts_controller.rb | 2 +- app/controllers/explore/posts_controller.rb | 4 +- app/controllers/posts_controller.rb | 2 +- app/jobs/tag_batch_change_job.rb | 14 +++---- app/logical/concerns/searchable.rb | 2 +- app/logical/current_user.rb | 14 ------- app/logical/post_query_builder.rb | 46 +++++++++++---------- app/logical/post_search_context.rb | 4 +- app/logical/post_sets/post.rb | 6 +-- app/logical/recommender_service.rb | 2 +- app/logical/related_tag_calculator.rb | 16 ++++--- app/logical/related_tag_query.rb | 2 +- app/logical/sources/strategies/base.rb | 2 +- app/logical/upload_service/preprocessor.rb | 6 +-- app/models/artist.rb | 44 +++++++++----------- app/models/pool.rb | 2 +- app/models/post.rb | 18 ++++---- app/models/saved_search.rb | 18 ++++---- app/models/tag.rb | 2 +- app/presenters/post_set_presenters/post.rb | 2 +- app/presenters/user_presenter.rb | 10 ++--- test/unit/post_query_builder_test.rb | 18 ++++---- test/unit/related_tag_calculator_test.rb | 29 +++++-------- 24 files changed, 120 insertions(+), 147 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 5bd8ca612..d1b368514 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -82,7 +82,7 @@ class CommentsController < ApplicationController end def index_by_post - @posts = Post.where("last_comment_bumped_at IS NOT NULL").tag_match(params[:tags]).reorder("last_comment_bumped_at DESC NULLS LAST").paginate(params[:page], :limit => 5, :search_count => params[:search]) + @posts = Post.where("last_comment_bumped_at IS NOT NULL").user_tag_match(params[:tags]).reorder("last_comment_bumped_at DESC NULLS LAST").paginate(params[:page], :limit => 5, :search_count => params[:search]) if request.format.html? @posts = @posts.includes(comments: [:creator]) diff --git a/app/controllers/counts_controller.rb b/app/controllers/counts_controller.rb index 9dc6a2461..3a2801ca0 100644 --- a/app/controllers/counts_controller.rb +++ b/app/controllers/counts_controller.rb @@ -2,6 +2,6 @@ class CountsController < ApplicationController respond_to :xml, :json def posts - @count = PostQueryBuilder.new(params[:tags]).fast_count(timeout: CurrentUser.statement_timeout, raise_on_timeout: true, skip_cache: params[:skip_cache]) + @count = PostQueryBuilder.new(params[:tags], CurrentUser.user).fast_count(timeout: CurrentUser.statement_timeout, raise_on_timeout: true, skip_cache: params[:skip_cache]) end end diff --git a/app/controllers/explore/posts_controller.rb b/app/controllers/explore/posts_controller.rb index a0939acb1..c2d7e25c3 100644 --- a/app/controllers/explore/posts_controller.rb +++ b/app/controllers/explore/posts_controller.rb @@ -47,11 +47,11 @@ module Explore end def popular_posts(min_date, max_date) - Post.where(created_at: min_date..max_date).tag_match("order:score") + Post.where(created_at: min_date..max_date).user_tag_match("order:score") end def curated_posts(min_date, max_date) - Post.where(created_at: min_date..max_date).tag_match("order:curated") + Post.where(created_at: min_date..max_date).user_tag_match("order:curated") end end end diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 9d8224b7c..9829c954f 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -80,7 +80,7 @@ class PostsController < ApplicationController end def random - @post = Post.tag_match(params[:tags]).random + @post = Post.user_tag_match(params[:tags]).random raise ActiveRecord::RecordNotFound if @post.nil? authorize @post respond_with(@post) do |format| diff --git a/app/jobs/tag_batch_change_job.rb b/app/jobs/tag_batch_change_job.rb index fb558d250..00d9c5d7b 100644 --- a/app/jobs/tag_batch_change_job.rb +++ b/app/jobs/tag_batch_change_job.rb @@ -9,19 +9,17 @@ class TagBatchChangeJob < ApplicationJob normalized_antecedent = TagAlias.to_aliased(PostQueryBuilder.new(antecedent.mb_chars.downcase).split_query) normalized_consequent = TagAlias.to_aliased(PostQueryBuilder.new(consequent.mb_chars.downcase).parse_tag_edit) - CurrentUser.without_safe_mode do - CurrentUser.scoped(updater, updater_ip_addr) do - migrate_posts(normalized_antecedent, normalized_consequent) - migrate_saved_searches(normalized_antecedent, normalized_consequent) - migrate_blacklists(normalized_antecedent, normalized_consequent) + CurrentUser.scoped(updater, updater_ip_addr) do + migrate_posts(normalized_antecedent, normalized_consequent) + migrate_saved_searches(normalized_antecedent, normalized_consequent) + migrate_blacklists(normalized_antecedent, normalized_consequent) - ModAction.log("processed mass update: #{antecedent} -> #{consequent}", :mass_update) - end + ModAction.log("processed mass update: #{antecedent} -> #{consequent}", :mass_update) end end def migrate_posts(normalized_antecedent, normalized_consequent) - ::Post.tag_match(normalized_antecedent.join(" ")).find_each do |post| + ::Post.system_tag_match(normalized_antecedent.join(" ")).find_each do |post| post.with_lock do tags = (post.tag_array - normalized_antecedent + normalized_consequent).join(" ") post.update(tag_string: tags) diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb index 6cd151723..3b2996b54 100644 --- a/app/logical/concerns/searchable.rb +++ b/app/logical/concerns/searchable.rb @@ -226,7 +226,7 @@ module Searchable end if params[:post_tags_match].present? - relation = relation.where(post_id: Post.tag_match(params[:post_tags_match]).reorder(nil)) + relation = relation.where(post_id: Post.user_tag_match(params[:post_tags_match]).reorder(nil)) end relation diff --git a/app/logical/current_user.rb b/app/logical/current_user.rb index 6069a2bc3..292bf4a72 100644 --- a/app/logical/current_user.rb +++ b/app/logical/current_user.rb @@ -73,20 +73,6 @@ class CurrentUser RequestStore[:safe_mode] end - def self.admin_mode? - RequestStore[:admin_mode] - end - - def self.without_safe_mode - prev = RequestStore[:safe_mode] - RequestStore[:safe_mode] = false - RequestStore[:admin_mode] = true - yield - ensure - RequestStore[:safe_mode] = prev - RequestStore[:admin_mode] = false - end - def self.safe_mode=(safe_mode) RequestStore[:safe_mode] = safe_mode end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index bb1bcfe27..0136a0e8c 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -53,10 +53,15 @@ class PostQueryBuilder COUNT_METATAG_SYNONYMS.flat_map { |str| [str, "#{str}_asc"] } + CATEGORY_COUNT_METATAGS.flat_map { |str| [str, "#{str}_asc"] } - attr_accessor :query_string + attr_reader :query_string, :current_user, :safe_mode, :hide_deleted_posts + alias_method :safe_mode?, :safe_mode + alias_method :hide_deleted_posts?, :hide_deleted_posts - def initialize(query_string) + def initialize(query_string, current_user = User.anonymous, safe_mode: false, hide_deleted_posts: false) @query_string = query_string + @current_user = current_user + @safe_mode = safe_mode + @hide_deleted_posts = hide_deleted_posts end def tags_match(tags, relation) @@ -177,9 +182,9 @@ class PostQueryBuilder when "noteupdater" user_subquery_matches(NoteVersion.unscoped, value, field: :updater) when "upvoter", "upvote" - user_subquery_matches(PostVote.positive.visible(CurrentUser.user), value, field: :user) + user_subquery_matches(PostVote.positive.visible(current_user), value, field: :user) when "downvoter", "downvote" - user_subquery_matches(PostVote.negative.visible(CurrentUser.user), value, field: :user) + user_subquery_matches(PostVote.negative.visible(current_user), value, field: :user) when *CATEGORY_COUNT_METATAGS short_category = name.delete_suffix("tags") category = TagCategory.short_name_mapping[short_category] @@ -248,16 +253,16 @@ class PostQueryBuilder user_subquery_matches(flags, username) do |username| flagger = User.find_by_name(username) - PostFlag.unscoped.creator_matches(flagger, CurrentUser.user) + PostFlag.unscoped.creator_matches(flagger, current_user) end end def saved_search_matches(label) case label.downcase when "all" - Post.where(id: SavedSearch.post_ids_for(CurrentUser.id)) + Post.where(id: SavedSearch.post_ids_for(current_user.id)) else - Post.where(id: SavedSearch.post_ids_for(CurrentUser.id, label: label)) + Post.where(id: SavedSearch.post_ids_for(current_user.id, label: label)) end end @@ -287,8 +292,8 @@ class PostQueryBuilder def disapproved_matches(query) if query.downcase.in?(PostDisapproval::REASONS) Post.where(disapprovals: PostDisapproval.where(reason: query.downcase)) - elsif User.normalize_name(query) == CurrentUser.user.name - Post.where(disapprovals: PostDisapproval.where(user: CurrentUser.user)) + elsif User.normalize_name(query) == current_user.name + Post.where(disapprovals: PostDisapproval.where(user: current_user)) else Post.none end @@ -362,20 +367,20 @@ class PostQueryBuilder def ordfavgroup_matches(query) # XXX unify with FavoriteGroup#posts - favgroup = FavoriteGroup.visible(CurrentUser.user).name_or_id_matches(query, CurrentUser.user) + favgroup = FavoriteGroup.visible(current_user).name_or_id_matches(query, current_user) favgroup_posts = favgroup.joins("CROSS JOIN unnest(favorite_groups.post_ids) WITH ORDINALITY AS row(post_id, favgroup_index)").select(:post_id, :favgroup_index) Post.joins("JOIN (#{favgroup_posts.to_sql}) favgroup_posts ON favgroup_posts.post_id = posts.id").order("favgroup_posts.favgroup_index ASC") end def favgroup_matches(query) - favgroup = FavoriteGroup.visible(CurrentUser.user).name_or_id_matches(query, CurrentUser.user) + favgroup = FavoriteGroup.visible(current_user).name_or_id_matches(query, current_user) Post.where(id: favgroup.select("unnest(post_ids)")) end def favorites_include(username) favuser = User.find_by_name(username) - if favuser.present? && Pundit.policy!([CurrentUser.user, nil], favuser).can_see_favorites? + if favuser.present? && Pundit.policy!([current_user, nil], favuser).can_see_favorites? tags_include("fav:#{favuser.id}") else Post.none @@ -445,10 +450,9 @@ class PostQueryBuilder relation end - def hide_deleted_posts? - return false if CurrentUser.admin_mode? - return false if find_metatag(:status).to_s.downcase.in?(%w[deleted active any all]) - return CurrentUser.user.hide_deleted_posts? + def hide_deleted? + has_status_metatag = select_metatags(:status).any? { |metatag| metatag.value.downcase.in?(%w[deleted active any all]) } + hide_deleted_posts? && !has_status_metatag end def build @@ -458,8 +462,8 @@ class PostQueryBuilder end relation = Post.all - relation = relation.where(rating: 's') if CurrentUser.safe_mode? - relation = relation.undeleted if hide_deleted_posts? + 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) @@ -813,8 +817,8 @@ class PostQueryBuilder 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 += " rating:s" if safe_mode? + tags += " -status:deleted" if hide_deleted? tags = tags.strip # Optimize some cases. these are just estimates but at these @@ -852,7 +856,7 @@ class PostQueryBuilder def fast_count_search(tags, timeout:, raise_on_timeout:) count = Post.with_timeout(timeout, nil, tags: tags) do - Post.tag_match(tags).count + Post.user_tag_match(tags).count end if count.nil? diff --git a/app/logical/post_search_context.rb b/app/logical/post_search_context.rb index 9399bcf63..44b57646c 100644 --- a/app/logical/post_search_context.rb +++ b/app/logical/post_search_context.rb @@ -10,9 +10,9 @@ class PostSearchContext def post_id if seq == "prev" - Post.tag_match(tags).where("posts.id > ?", id).reorder("posts.id asc").first.try(:id) + Post.user_tag_match(tags).where("posts.id > ?", id).reorder("posts.id asc").first.try(:id) else - Post.tag_match(tags).where("posts.id < ?", id).reorder("posts.id desc").first.try(:id) + Post.user_tag_match(tags).where("posts.id < ?", id).reorder("posts.id desc").first.try(:id) end end diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index 72184e0ed..8065699ef 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) + @query = PostQueryBuilder.new(tags, CurrentUser.user) @tag_string = tags @page = page @per_page = per_page @@ -92,7 +92,7 @@ module PostSets def get_random_posts per_page.times.inject([]) do |all, x| - all << ::Post.tag_match(tag_string).random + all << ::Post.user_tag_match(tag_string).random end.compact.uniq end @@ -103,7 +103,7 @@ module PostSets if is_random? temp = get_random_posts else - temp = ::Post.tag_match(tag_string).where("true /* PostSets::Post#posts:2 */").paginate(page, :count => post_count, :limit => per_page) + temp = ::Post.user_tag_match(tag_string).where("true /* PostSets::Post#posts:2 */").paginate(page, :count => post_count, :limit => per_page) end end end diff --git a/app/logical/recommender_service.rb b/app/logical/recommender_service.rb index 6d84ec1ce..315292997 100644 --- a/app/logical/recommender_service.rb +++ b/app/logical/recommender_service.rb @@ -36,7 +36,7 @@ module RecommenderService posts = posts.where.not(id: post.id) if post posts = posts.where.not(uploader_id: uploader.id) if uploader posts = posts.where.not(id: favoriter.favorites.select(:post_id)) if favoriter - posts = posts.where(id: Post.tag_match(tags).reorder(nil).select(:id)) if tags.present? + posts = posts.where(id: Post.user_tag_match(tags).reorder(nil).select(:id)) if tags.present? id_to_score = recs.to_h recs = posts.map { |post| { score: id_to_score[post.id], post: post } } diff --git a/app/logical/related_tag_calculator.rb b/app/logical/related_tag_calculator.rb index c66fd3149..ec56d567b 100644 --- a/app/logical/related_tag_calculator.rb +++ b/app/logical/related_tag_calculator.rb @@ -1,12 +1,12 @@ module RelatedTagCalculator - def self.similar_tags_for_search(tag_query, search_sample_size: 1000, tag_sample_size: 250, category: nil) - search_count = PostQueryBuilder.new(tag_query).fast_count + def self.similar_tags_for_search(tag_query, current_user, search_sample_size: 1000, tag_sample_size: 250, category: nil) + search_count = PostQueryBuilder.new(tag_query, current_user).fast_count return [] if search_count.nil? search_sample_size = [search_count, search_sample_size].min return [] if search_sample_size <= 0 - tags = frequent_tags_for_search(tag_query, search_sample_size: search_sample_size, category: category).limit(tag_sample_size) + tags = frequent_tags_for_search(tag_query, current_user, search_sample_size: search_sample_size, category: category).limit(tag_sample_size) tags = tags.sort_by do |tag| # cosine distance(tag1, tag2) = 1 - {{tag1 tag2}} / sqrt({{tag1}} * {{tag2}}) 1 - tag.overlap_count / Math.sqrt(tag.post_count * search_count.to_f) @@ -15,8 +15,8 @@ module RelatedTagCalculator tags end - def self.frequent_tags_for_search(tag_query, search_sample_size: 1000, category: nil) - sample_posts = Post.tag_match(tag_query).reorder(:md5).limit(search_sample_size) + def self.frequent_tags_for_search(tag_query, current_user, search_sample_size: 1000, category: nil) + sample_posts = Post.user_tag_match(tag_query, current_user).reorder(:md5).limit(search_sample_size) frequent_tags_for_post_relation(sample_posts, category: category) end @@ -36,12 +36,10 @@ module RelatedTagCalculator tags_with_counts.sort_by { |tag_name, count| [-count, tag_name] }.map(&:first) end - def self.cached_similar_tags_for_search(tag_query, max_tags, search_timeout: 2000, cache_timeout: 8.hours) + def self.cached_similar_tags_for_search(tag_query, max_tags, current_user, search_timeout: 2000, cache_timeout: 8.hours) Cache.get("similar_tags:#{tag_query}", cache_timeout, race_condition_ttl: 60.seconds) do ApplicationRecord.with_timeout(search_timeout, []) do - CurrentUser.without_safe_mode do - RelatedTagCalculator.similar_tags_for_search(tag_query).take(max_tags).pluck(:name) - end + RelatedTagCalculator.similar_tags_for_search(tag_query, current_user).take(max_tags).pluck(:name) end end end diff --git a/app/logical/related_tag_query.rb b/app/logical/related_tag_query.rb index b50dce768..f3ac9beb0 100644 --- a/app/logical/related_tag_query.rb +++ b/app/logical/related_tag_query.rb @@ -47,7 +47,7 @@ class RelatedTagQuery end def similar_tags - @similar_tags ||= RelatedTagCalculator.similar_tags_for_search(query, category: category_of).take(limit) + @similar_tags ||= RelatedTagCalculator.similar_tags_for_search(query, user, category: category_of).take(limit) end # Returns the top 20 most frequently added tags within the last 20 edits made by the user in the last hour. diff --git a/app/logical/sources/strategies/base.rb b/app/logical/sources/strategies/base.rb index d21e2dc33..fea60b880 100644 --- a/app/logical/sources/strategies/base.rb +++ b/app/logical/sources/strategies/base.rb @@ -248,7 +248,7 @@ module Sources end def related_posts(limit = 5) - CurrentUser.as_system { Post.tag_match(related_posts_search_query).paginate(1, limit: limit) } + Post.system_tag_match(related_posts_search_query).paginate(1, limit: limit) end memoize :related_posts diff --git a/app/logical/upload_service/preprocessor.rb b/app/logical/upload_service/preprocessor.rb index 6c422bff3..a0b411818 100644 --- a/app/logical/upload_service/preprocessor.rb +++ b/app/logical/upload_service/preprocessor.rb @@ -68,10 +68,8 @@ class UploadService def start! if Utils.is_downloadable?(source) - CurrentUser.as_system do - if Post.tag_match("source:#{canonical_source}").where.not(id: original_post_id).exists? - raise ActiveRecord::RecordNotUnique.new("A post with source #{canonical_source} already exists") - end + if Post.system_tag_match("source:#{canonical_source}").where.not(id: original_post_id).exists? + raise ActiveRecord::RecordNotUnique.new("A post with source #{canonical_source} already exists") end if Upload.where(source: source, status: "completed").exists? diff --git a/app/models/artist.rb b/app/models/artist.rb index 692ff39fe..8e8608d0d 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -134,10 +134,8 @@ class Artist < ApplicationRecord source = params.delete(:source) if source.blank? && params[:name].present? - CurrentUser.without_safe_mode do - post = Post.tag_match("source:http* #{params[:name]}").first - source = post.try(:source) - end + post = Post.system_tag_match("source:http* #{params[:name]}").first + source = post.try(:source) end if source.present? @@ -168,36 +166,32 @@ class Artist < ApplicationRecord module BanMethods def unban! Post.transaction do - CurrentUser.without_safe_mode do - ti = TagImplication.find_by(antecedent_name: name, consequent_name: "banned_artist") - ti&.destroy + ti = TagImplication.find_by(antecedent_name: name, consequent_name: "banned_artist") + ti&.destroy - Post.tag_match(name).find_each do |post| - post.unban! - fixed_tags = post.tag_string.sub(/(?:\A| )banned_artist(?:\Z| )/, " ").strip - post.update(tag_string: fixed_tags) - end - - update!(is_banned: false) - ModAction.log("unbanned artist ##{id}", :artist_unban) + Post.raw_tag_match(name).find_each do |post| + post.unban! + fixed_tags = post.tag_string.sub(/(?:\A| )banned_artist(?:\Z| )/, " ").strip + post.update(tag_string: fixed_tags) end + + update!(is_banned: false) + ModAction.log("unbanned artist ##{id}", :artist_unban) end end def ban!(banner: CurrentUser.user) Post.transaction do - CurrentUser.without_safe_mode do - Post.tag_match(name).each(&:ban!) + Post.raw_tag_match(name).each(&:ban!) - # potential race condition but unlikely - unless TagImplication.where(:antecedent_name => name, :consequent_name => "banned_artist").exists? - tag_implication = TagImplication.create!(antecedent_name: name, consequent_name: "banned_artist", skip_secondary_validations: true, creator: banner) - tag_implication.approve!(approver: banner) - end - - update!(is_banned: true) - ModAction.log("banned artist ##{id}", :artist_ban) + # potential race condition but unlikely + unless TagImplication.where(:antecedent_name => name, :consequent_name => "banned_artist").exists? + tag_implication = TagImplication.create!(antecedent_name: name, consequent_name: "banned_artist", skip_secondary_validations: true, creator: banner) + tag_implication.approve!(approver: banner) end + + update!(is_banned: true) + ModAction.log("banned artist ##{id}", :artist_ban) end end end diff --git a/app/models/pool.rb b/app/models/pool.rb index c42c82738..0d1a579ab 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -27,7 +27,7 @@ class Pool < ApplicationRecord end def post_tags_match(query) - posts = Post.tag_match(query).select(:id).reorder(nil) + posts = Post.user_tag_match(query).select(:id).reorder(nil) pools = Pool.joins("CROSS JOIN unnest(post_ids) AS post_id").group(:id).where("post_id IN (?)", posts) where(id: pools) end diff --git a/app/models/post.rb b/app/models/post.rb index e3b0e2d9d..dc9d9eb23 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -604,7 +604,7 @@ class Post < ApplicationRecord # If someone else committed changes to this post before we did, # then try to merge the tag changes together. current_tags = tag_string_was.split - new_tags = PostQueryBuilder.new(tag_string).parse_tag_edit + new_tags = PostQueryBuilder.new(tag_string).split_query old_tags = old_tag_string.split kept_tags = current_tags & new_tags @@ -642,7 +642,7 @@ class Post < ApplicationRecord end def normalize_tags - normalized_tags = PostQueryBuilder.new(tag_string).parse_tag_edit + normalized_tags = PostQueryBuilder.new(tag_string).split_query normalized_tags = apply_casesensitive_metatags(normalized_tags) normalized_tags = normalized_tags.map(&:downcase) normalized_tags = filter_metatags(normalized_tags) @@ -1372,9 +1372,7 @@ class Post < ApplicationRecord end def sample(query, sample_size) - CurrentUser.without_safe_mode do - tag_match(query).reorder(:md5).limit(sample_size) - end + user_tag_match(query, safe_mode: false, hide_deleted_posts: false).reorder(:md5).limit(sample_size) end # unflattens the tag_string into one tag per row. @@ -1472,8 +1470,12 @@ class Post < ApplicationRecord where("posts.tag_index @@ to_tsquery('danbooru', E?)", tag.to_escaped_for_tsquery) end - def tag_match(query) - PostQueryBuilder.new(query).build + def system_tag_match(query) + user_tag_match(query, User.system, safe_mode: false, hide_deleted_posts: false) + end + + def user_tag_match(query, user = CurrentUser.user, safe_mode: CurrentUser.safe_mode?, hide_deleted_posts: user.hide_deleted_posts?) + PostQueryBuilder.new(query, user, safe_mode: safe_mode, hide_deleted_posts: hide_deleted_posts).build end def search(params) @@ -1488,7 +1490,7 @@ class Post < ApplicationRecord ) if params[:tags].present? - q = q.tag_match(params[:tags]) + q = q.user_tag_match(params[:tags]) end if params[:order].present? diff --git a/app/models/saved_search.rb b/app/models/saved_search.rb index 0413ce814..c194d5807 100644 --- a/app/models/saved_search.rb +++ b/app/models/saved_search.rb @@ -114,18 +114,16 @@ class SavedSearch < ApplicationRecord end def populate(query, timeout: 10_000) - CurrentUser.as_system do - redis_key = "search:#{query}" - return if redis.exists(redis_key) + redis_key = "search:#{query}" + return if redis.exists(redis_key) - post_ids = Post.with_timeout(timeout, [], query: query) do - Post.tag_match(query).limit(QUERY_LIMIT).pluck(:id) - end + post_ids = Post.with_timeout(timeout, [], query: query) do + Post.system_tag_match(query).limit(QUERY_LIMIT).pluck(:id) + end - if post_ids.present? - redis.sadd(redis_key, post_ids) - redis.expire(redis_key, REDIS_EXPIRY.to_i) - end + if post_ids.present? + redis.sadd(redis_key, post_ids) + redis.expire(redis_key, REDIS_EXPIRY.to_i) end end end diff --git a/app/models/tag.rb b/app/models/tag.rb index d925123b3..e575ff840 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -349,7 +349,7 @@ class Tag < ApplicationRecord end def posts - Post.tag_match(name) + Post.system_tag_match(name) end def self.available_includes diff --git a/app/presenters/post_set_presenters/post.rb b/app/presenters/post_set_presenters/post.rb index a60b64ecf..7aa8ca50e 100644 --- a/app/presenters/post_set_presenters/post.rb +++ b/app/presenters/post_set_presenters/post.rb @@ -40,7 +40,7 @@ module PostSetPresenters end def similar_tags - RelatedTagCalculator.cached_similar_tags_for_search(post_set.tag_string, MAX_TAGS) + RelatedTagCalculator.cached_similar_tags_for_search(post_set.tag_string, MAX_TAGS, CurrentUser.user) end def frequent_tags diff --git a/app/presenters/user_presenter.rb b/app/presenters/user_presenter.rb index f9f958c80..10a876249 100644 --- a/app/presenters/user_presenter.rb +++ b/app/presenters/user_presenter.rb @@ -36,11 +36,11 @@ class UserPresenter end def posts_for_saved_search_category(category) - Post.tag_match("search:#{category}").limit(10) + Post.user_tag_match("search:#{category}").limit(10) end def uploads - Post.tag_match("user:#{user.name}").limit(6) + Post.user_tag_match("user:#{user.name}").limit(6) end def has_uploads? @@ -48,7 +48,7 @@ class UserPresenter end def favorites - Post.tag_match("ordfav:#{user.name}").limit(6) + Post.user_tag_match("ordfav:#{user.name}").limit(6) end def has_favorites? @@ -76,7 +76,7 @@ class UserPresenter end def commented_posts_count(template) - count = CurrentUser.without_safe_mode { PostQueryBuilder.new("commenter:#{user.name}").fast_count } + count = PostQueryBuilder.new("commenter:#{user.name}", User.system).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 { PostQueryBuilder.new("noteupdater:#{user.name}").fast_count } + count = PostQueryBuilder.new("noteupdater:#{user.name}", User.system).fast_count count = "?" if count.nil? template.link_to(count, template.posts_path(:tags => "noteupdater:#{user.name} order:note")) end diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index 418fb960d..64d01e46c 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -2,11 +2,11 @@ require 'test_helper' class PostQueryBuilderTest < ActiveSupport::TestCase def assert_tag_match(posts, query) - assert_equal(posts.map(&:id), Post.tag_match(query).pluck(:id)) + assert_equal(posts.map(&:id), Post.user_tag_match(query).pluck(:id)) end def assert_fast_count(count, query, **options) - assert_equal(count, PostQueryBuilder.new(query).fast_count(**options)) + assert_equal(count, PostQueryBuilder.new(query, **options).fast_count) end setup do @@ -616,6 +616,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([deleted], "status:deleted") assert_tag_match([undeleted, deleted], "status:any") assert_tag_match([undeleted, deleted], "status:all") + assert_tag_match([deleted], "status:banned status:deleted") assert_tag_match([], "-status:banned") assert_tag_match([deleted], "-status:active") @@ -629,6 +630,8 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([deleted], "status:deleted") assert_tag_match([undeleted, deleted], "status:any") assert_tag_match([undeleted, deleted], "status:all") + + assert_fast_count(2, "status:banned") end should "return posts for the filetype: metatag" do @@ -988,7 +991,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase post1 = create(:post, rating: "s") assert_raise(::Post::SearchError) do - Post.tag_match("a b c rating:s width:10 height:10 user:bob") + Post.user_tag_match("a b c rating:s width:10 height:10 user:bob") end end @@ -1143,29 +1146,28 @@ class PostQueryBuilderTest < ActiveSupport::TestCase 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, "") + assert_fast_count(1, "", safe_mode: true) end should "work for a nil search" do - assert_fast_count(1, nil) + assert_fast_count(1, nil, safe_mode: true) 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") + assert_fast_count(1, "aaa bbb", safe_mode: true) 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, "") + assert_fast_count(100, "", safe_mode: true) end end end diff --git a/test/unit/related_tag_calculator_test.rb b/test/unit/related_tag_calculator_test.rb index ec9d59b03..66f89b498 100644 --- a/test/unit/related_tag_calculator_test.rb +++ b/test/unit/related_tag_calculator_test.rb @@ -2,14 +2,7 @@ require 'test_helper' class RelatedTagCalculatorTest < ActiveSupport::TestCase setup do - user = FactoryBot.create(:user) - CurrentUser.user = user - CurrentUser.ip_addr = "127.0.0.1" - end - - teardown do - CurrentUser.user = nil - CurrentUser.ip_addr = nil + @user = create(:user) end context "RelatedTagCalculator" do @@ -18,7 +11,7 @@ class RelatedTagCalculatorTest < ActiveSupport::TestCase create(:post, tag_string: "aaa bbb ccc ddd") create(:post, tag_string: "aaa bbb ccc") create(:post, tag_string: "aaa bbb") - posts = Post.tag_match("aaa") + posts = Post.user_tag_match("aaa", @user) assert_equal(%w[aaa bbb ccc ddd], RelatedTagCalculator.frequent_tags_for_post_array(posts)) end @@ -30,7 +23,7 @@ class RelatedTagCalculatorTest < ActiveSupport::TestCase create(:post, tag_string: "aaa bbb ccc") create(:post, tag_string: "aaa bbb") - assert_equal(%w[aaa bbb ccc ddd], RelatedTagCalculator.frequent_tags_for_search("aaa").pluck(:name)) + assert_equal(%w[aaa bbb ccc ddd], RelatedTagCalculator.frequent_tags_for_search("aaa", @user).pluck(:name)) end should "calculate the most frequent tags for a multiple tag search" do @@ -38,16 +31,16 @@ class RelatedTagCalculatorTest < ActiveSupport::TestCase create(:post, tag_string: "aaa bbb ccc ddd") create(:post, tag_string: "aaa eee fff") - assert_equal(%w[aaa bbb ccc ddd], RelatedTagCalculator.frequent_tags_for_search("aaa bbb").pluck(:name)) + assert_equal(%w[aaa bbb ccc ddd], RelatedTagCalculator.frequent_tags_for_search("aaa bbb", @user).pluck(:name)) end should "calculate the most frequent tags with a category constraint" do create(:post, tag_string: "aaa bbb art:ccc copy:ddd") - create(:post, tag_string: "aaa bbb art:ccc") + create(:post, tag_string: "aaa bbb ccc") create(:post, tag_string: "aaa bbb") - assert_equal(%w[aaa bbb], RelatedTagCalculator.frequent_tags_for_search("aaa", category: Tag.categories.general).pluck(:name)) - assert_equal(%w[ccc], RelatedTagCalculator.frequent_tags_for_search("aaa", category: Tag.categories.artist).pluck(:name)) + assert_equal(%w[aaa bbb], RelatedTagCalculator.frequent_tags_for_search("aaa", @user, category: Tag.categories.general).pluck(:name)) + assert_equal(%w[ccc], RelatedTagCalculator.frequent_tags_for_search("aaa", @user, category: Tag.categories.artist).pluck(:name)) end end @@ -57,9 +50,9 @@ class RelatedTagCalculatorTest < ActiveSupport::TestCase create(:post, tag_string: "1girl solo", rating: "q") create(:post, tag_string: "1girl 1boy", rating: "q") - assert_equal(%w[1girl solo 1boy], RelatedTagCalculator.similar_tags_for_search("1girl").pluck(:name)) - assert_equal(%w[1girl 1boy solo], RelatedTagCalculator.similar_tags_for_search("rating:q").pluck(:name)) - assert_equal(%w[solo 1girl], RelatedTagCalculator.similar_tags_for_search("solo").pluck(:name)) + assert_equal(%w[1girl solo 1boy], RelatedTagCalculator.similar_tags_for_search("1girl", @user).pluck(:name)) + assert_equal(%w[1girl 1boy solo], RelatedTagCalculator.similar_tags_for_search("rating:q", @user).pluck(:name)) + assert_equal(%w[solo 1girl], RelatedTagCalculator.similar_tags_for_search("solo", @user).pluck(:name)) end should "calculate the similar tags for an aliased tag" do @@ -67,7 +60,7 @@ class RelatedTagCalculatorTest < ActiveSupport::TestCase create(:post, tag_string: "bunny dog") create(:post, tag_string: "bunny cat") - assert_equal(%w[bunny cat dog], RelatedTagCalculator.similar_tags_for_search("rabbit").pluck(:name)) + assert_equal(%w[bunny cat dog], RelatedTagCalculator.similar_tags_for_search("rabbit", @user).pluck(:name)) end end end