posts/index: fix rating:s being included in page title in safe mode.

Fixes bug described in d3e4ac7c17 (commitcomment-39049351)

When dealing with searches, there are several variables we have to keep
in mind:

* Whether tag aliases should be applied.
* Whether search terms should be sorted.
* Whether the rating:s and -status:deleted metatags should be added by
  safe mode and the hide deleted posts setting.

Which of these things we need to do depends on the context:

* We want to apply aliases when actually doing the search, calculating
  the count, looking up the wiki excerpt, recording missed/popular
  searches in Reportbooru, and calculating related tags for the sidebar,
  but not when displaying the raw search as typed by the user (for
  example, in the page title or in the tag search box).
* We want to sort the search when calculating cache keys for fast_count
  or related tags, and when recording missed/popular searches, but not
  in the page title or when displaying the raw search.
* We want to add rating:s and -status:deleted when performing the
  search, calculating the count, or recording missed/popular searches,
  but not when calculating related tags for the sidebar, or when
  displaying the page title or raw search.

Here we introduce normalized_query and try to use it in contexts where
query normalization is necessary. When to use the normalized query
versus the raw unnormalized query is still subtle and prone to error.

app/controllers/counts_controller.rb

@@ -4,6 +4,6 @@ class CountsController < ApplicationController
   def posts
     estimate_count = params.fetch(:estimate_count, "true").truthy?
     skip_cache = params.fetch(:skip_cache, "false").truthy?
-    @count = PostQueryBuilder.new(params[:tags], CurrentUser.user).fast_count(timeout: CurrentUser.statement_timeout, estimate_count: estimate_count, skip_cache: skip_cache)
+    @count = PostQueryBuilder.new(params[:tags], CurrentUser.user).normalized_query.fast_count(timeout: CurrentUser.statement_timeout, estimate_count: estimate_count, skip_cache: skip_cache)
   end
 end

app/logical/post_query_builder.rb

@@ -53,22 +53,15 @@ class PostQueryBuilder
     COUNT_METATAG_SYNONYMS.flat_map { |str| [str, "#{str}_asc"] } +
     CATEGORY_COUNT_METATAGS.flat_map { |str| [str, "#{str}_asc"] }
 
-  attr_reader :query_string, :terms, :current_user, :safe_mode, :hide_deleted_posts
+  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, current_user = User.anonymous, normalize_aliases: true, normalize_order: true, safe_mode: false, hide_deleted_posts: false)
+  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
-
-    @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
 
   def tags_match(tags, relation)
@@ -457,11 +450,6 @@ class PostQueryBuilder
     relation
   end
 
-  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
     tag_count = terms.count { |term| !Danbooru.config.is_unlimited_tag?(term) }
     if tag_count > Danbooru.config.tag_query_limit
@@ -695,20 +683,6 @@ class PostQueryBuilder
       end
     end
 
-    def normalize_aliases!
-      tag_names = tags.map(&:name)
-      tag_aliases = tag_names.zip(TagAlias.to_aliased(tag_names)).to_h
-
-      terms.map! do |term|
-        term.name = tag_aliases[term.name] if term.type == :tag
-        term
-      end
-    end
-
-    def normalize_order!
-      terms.sort_by!(&:to_s).uniq!
-    end
-
     def parse_tag_edit
       split_query
     end
@@ -884,11 +858,51 @@ class PostQueryBuilder
     end
   end
 
+  concerning :NormalizationMethods do
+    def normalized_query(implicit: true, sort: true)
+      post_query = dup
+      post_query.terms.concat(implicit_metatags) if implicit
+      post_query.normalize_aliases!
+      post_query.normalize_order! if sort
+      post_query
+    end
+
+    def normalize_aliases!
+      tag_names = tags.map(&:name)
+      tag_aliases = tag_names.zip(TagAlias.to_aliased(tag_names)).to_h
+
+      terms.map! do |term|
+        term.name = tag_aliases[term.name] if term.type == :tag
+        term
+      end
+    end
+
+    def normalize_order!
+      terms.sort_by!(&:to_s).uniq!
+    end
+
+    def implicit_metatags
+      metatags = []
+      metatags << OpenStruct.new(type: :metatag, name: "rating", value: "s") if safe_mode?
+      metatags << OpenStruct.new(type: :metatag, name: "status", value: "deleted", negated: true) if hide_deleted?
+      metatags
+    end
+
+    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
+  end
+
   concerning :UtilityMethods do
     def to_s
       split_query.join(" ")
     end
 
+    def terms
+      @terms ||= scan_query
+    end
+
     def tags
       terms.select { |term| term.type == :tag }
     end
@@ -943,5 +957,5 @@ class PostQueryBuilder
     end
   end
 
-  memoize :split_query
+  memoize :split_query, :normalized_query
 end

app/logical/post_sets/post.rb

@@ -4,6 +4,7 @@ module PostSets
     MAX_SIDEBAR_TAGS = 25
 
     attr_reader :page, :random, :post_count, :format, :tag_string, :query
+    delegate :normalized_query, to: :query
 
     def initialize(tags, page = 1, per_page = nil, random: false, format: "html")
       @query = PostQueryBuilder.new(tags, CurrentUser.user, safe_mode: CurrentUser.safe_mode?, hide_deleted_posts: CurrentUser.hide_deleted_posts?)
@@ -29,8 +30,8 @@ module PostSets
     end
 
     def tag
-      return nil unless query.has_single_tag?
+      return nil unless normalized_query.has_single_tag?
-      @tag ||= Tag.find_by(name: query.tags.first.name)
+      @tag ||= Tag.find_by(name: normalized_query.tags.first.name)
     end
 
     def artist
@@ -40,7 +41,7 @@ module PostSets
     end
 
     def pool
-      pool_names = query.select_metatags(:pool, :ordpool).map(&:value)
+      pool_names = normalized_query.select_metatags(:pool, :ordpool).map(&:value)
       name = pool_names.first
       return nil unless pool_names.size == 1
 
@@ -48,7 +49,7 @@ module PostSets
     end
 
     def favgroup
-      favgroup_names = query.select_metatags(:favgroup, :ordfavgroup).map(&:value)
+      favgroup_names = normalized_query.select_metatags(:favgroup, :ordfavgroup).map(&:value)
       name = favgroup_names.first
       return nil unless favgroup_names.size == 1
 
@@ -88,7 +89,7 @@ module PostSets
         # no need to get counts for formats that don't use a paginator
         nil
       else
-        query.fast_count
+        normalized_query.fast_count
       end
     end
 
@@ -105,7 +106,7 @@ module PostSets
         if is_random?
           temp = get_random_posts
         else
-          temp = query.build.paginate(page, count: post_count, search_count: !post_count.nil?, limit: per_page)
+          temp = normalized_query.build.paginate(page, count: post_count, search_count: !post_count.nil?, limit: per_page)
         end
       end
     end
@@ -176,7 +177,7 @@ module PostSets
       end
 
       def similar_tags
-        RelatedTagCalculator.cached_similar_tags_for_search(query, MAX_SIDEBAR_TAGS)
+        RelatedTagCalculator.cached_similar_tags_for_search(normalized_query(implicit: false), MAX_SIDEBAR_TAGS)
       end
 
       def frequent_tags

app/logical/related_tag_query.rb

@@ -6,7 +6,7 @@ class RelatedTagQuery
 
   def initialize(query:, user: User.anonymous, category: nil, type: nil, limit: nil)
     @user = user
-    @post_query = PostQueryBuilder.new(query, user)
+    @post_query = PostQueryBuilder.new(query, user).normalized_query
     @query = @post_query.to_s
     @category = category
     @type = type

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, normalize_aliases: false).parse_tag_edit
+        new_tags = PostQueryBuilder.new(tag_string).parse_tag_edit
         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, normalize_aliases: false).parse_tag_edit
+      normalized_tags = PostQueryBuilder.new(tag_string).parse_tag_edit
       normalized_tags = apply_casesensitive_metatags(normalized_tags)
       normalized_tags = normalized_tags.map(&:downcase)
       normalized_tags = filter_metatags(normalized_tags)
@@ -1475,7 +1475,8 @@ class Post < ApplicationRecord
     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
+      post_query = PostQueryBuilder.new(query, user, safe_mode: safe_mode, hide_deleted_posts: hide_deleted_posts)
+      post_query.normalized_query.build
     end
 
     def search(params)

app/models/saved_search.rb

@@ -140,11 +140,11 @@ class SavedSearch < ApplicationRecord
     end
 
     def normalized_query
-      PostQueryBuilder.new(query).to_s
+      PostQueryBuilder.new(query).normalized_query.to_s
     end
 
     def normalize_query
-      self.query = PostQueryBuilder.new(query, normalize_order: false).to_s
+      self.query = PostQueryBuilder.new(query).normalized_query(sort: false).to_s
     end
   end
 

app/models/upload.rb

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

app/presenters/post_presenter.rb

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

app/presenters/user_presenter.rb

@@ -76,7 +76,7 @@ class UserPresenter
   end
 
   def commented_posts_count(template)
-    count = PostQueryBuilder.new("commenter:#{user.name}", User.system).fast_count
+    count = 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 = PostQueryBuilder.new("noteupdater:#{user.name}", User.system).fast_count
+    count = 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

test/functional/posts_controller_test.rb

@@ -305,6 +305,13 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
           assert_select "#post_#{@post.id}", 1
         end
       end
+
+      context "in safe mode" do
+        should "not include the rating:s tag in the page title" do
+          get posts_path(tags: "1girl", safe_mode: true)
+          assert_select "title", text: "1girl Art | Safebooru"
+        end
+      end
     end
 
     context "show_seq action" do

test/unit/post_query_builder_test.rb

@@ -6,7 +6,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
   end
 
   def assert_fast_count(count, query, query_options = {}, fast_count_options = {})
-    assert_equal(count, PostQueryBuilder.new(query, **query_options).fast_count(**fast_count_options))
+    assert_equal(count, PostQueryBuilder.new(query, **query_options).normalized_query.fast_count(**fast_count_options))
   end
 
   setup do
@@ -1018,7 +1018,6 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
   context "Parsing:" 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* -bbb*").split_query)
     end
 
     should "not strip out valid characters when scanning" do
@@ -1050,22 +1049,22 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
     end
   end
 
-  context "The normalize_query method" do
+  context "The normalized_query method" do
     should "work" do
       create(:tag_alias, antecedent_name: "gray", consequent_name: "grey")
 
-      assert_equal("foo", PostQueryBuilder.new("foo").to_s)
+      assert_equal("foo", PostQueryBuilder.new("foo").normalized_query.to_s)
-      assert_equal("foo", PostQueryBuilder.new(" foo ").to_s)
+      assert_equal("foo", PostQueryBuilder.new(" foo ").normalized_query.to_s)
-      assert_equal("foo", PostQueryBuilder.new("FOO").to_s)
+      assert_equal("foo", PostQueryBuilder.new("FOO").normalized_query.to_s)
-      assert_equal("foo", PostQueryBuilder.new("foo foo").to_s)
+      assert_equal("foo", PostQueryBuilder.new("foo foo").normalized_query.to_s)
-      assert_equal("grey", PostQueryBuilder.new("gray").to_s)
+      assert_equal("grey", PostQueryBuilder.new("gray").normalized_query.to_s)
-      assert_equal("aaa bbb", PostQueryBuilder.new("bbb aaa").to_s)
+      assert_equal("aaa bbb", PostQueryBuilder.new("bbb aaa").normalized_query.to_s)
-      assert_equal("-aaa bbb", PostQueryBuilder.new("bbb -aaa").to_s)
+      assert_equal("-aaa bbb", PostQueryBuilder.new("bbb -aaa").normalized_query.to_s)
-      assert_equal("~aaa ~bbb", PostQueryBuilder.new("~bbb ~aaa").to_s)
+      assert_equal("~aaa ~bbb", PostQueryBuilder.new("~bbb ~aaa").normalized_query.to_s)
-      assert_equal("commentary:true bbb", PostQueryBuilder.new("bbb commentary:true").to_s)
+      assert_equal("commentary:true bbb", PostQueryBuilder.new("bbb commentary:true").normalized_query.to_s)
-      assert_equal('commentary:"true" bbb', PostQueryBuilder.new("bbb commentary:'true'").to_s)
+      assert_equal('commentary:"true" bbb', PostQueryBuilder.new("bbb commentary:'true'").normalized_query.to_s)
-      assert_equal('-commentary:true bbb', PostQueryBuilder.new("bbb -commentary:true").to_s)
+      assert_equal('-commentary:true bbb', PostQueryBuilder.new("bbb -commentary:true").normalized_query.to_s)
-      assert_equal('-commentary:"true" bbb', PostQueryBuilder.new("bbb -commentary:'true'").to_s)
+      assert_equal('-commentary:"true" bbb', PostQueryBuilder.new("bbb -commentary:'true'").normalized_query.to_s)
     end
   end
 
@@ -1114,7 +1113,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
 
       should "set the expiration time" do
         Cache.expects(:put).with(PostQueryBuilder.new("score:42 aaa").count_cache_key, 1, 180)
-        PostQueryBuilder.new("aaa score:42").fast_count
+        assert_fast_count(1, "aaa score:42")
       end
 
       should "work with the hide_deleted_posts option turned on" do
@@ -1126,8 +1125,8 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
 
     context "a blank search" do
       should "should execute a search" do
-        assert_equal(1, PostQueryBuilder.new("").fast_count(estimate_count: false))
+        assert_fast_count(1, "", {}, { estimate_count: false })
-        assert_nothing_raised { PostQueryBuilder.new("").fast_count(estimate_count: true) }
+        assert_nothing_raised { PostQueryBuilder.new("").normalized_query.fast_count(estimate_count: true) }
       end
 
       should "return 0 for a nonexisting tag" do
@@ -1136,13 +1135,13 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
 
       context "in safe mode" do
         should "work for a blank search" do
-          assert_equal(2, PostQueryBuilder.new("").fast_count(estimate_count: false))
+          assert_fast_count(0, "", { safe_mode: true }, { estimate_count: false })
-          assert_nothing_raised { PostQueryBuilder.new("").fast_count(estimate_count: true) }
+          assert_nothing_raised { PostQueryBuilder.new("", safe_mode: true).normalized_query.fast_count(estimate_count: true) }
         end
 
         should "work for a nil search" do
-          assert_equal(2, PostQueryBuilder.new(nil).fast_count(estimate_count: false))
+          assert_fast_count(0, nil, { safe_mode: true }, { estimate_count: false })
-          assert_nothing_raised { PostQueryBuilder.new(nil).fast_count(estimate_count: true) }
+          assert_nothing_raised { PostQueryBuilder.new("", safe_mode: true).normalized_query.fast_count(estimate_count: true) }
         end
 
         should "not fail for a two tag search by a member" do