From 0eff095a3e6aea610cafdc7639d9b4c38452578e Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Fri, 31 Aug 2018 19:23:25 -0500
Subject: [PATCH] Refactor searching text attributes.

* Allow using ApplicationRecord#attribute_matches to search text attributes,
and standardize models on using this instead of duplicating code.

* Remove restrictions that limited wildcard searches to Builders only in various places.
---
 app/models/application_record.rb  | 21 ++++++++++++++++++---
 app/models/ban.rb                 |  4 +---
 app/models/bulk_update_request.rb |  3 +++
 app/models/comment.rb             | 12 +-----------
 app/models/dmail.rb               | 23 ++---------------------
 app/models/forum_post.rb          | 18 ++----------------
 app/models/forum_topic.rb         | 12 +-----------
 app/models/mod_action.rb          |  2 ++
 app/models/note.rb                | 13 +------------
 app/models/note_version.rb        |  1 +
 app/models/pool.rb                |  4 +---
 app/models/post_appeal.rb         | 12 +-----------
 app/models/post_flag.rb           | 12 +-----------
 app/models/post_replacement.rb    |  7 +++++++
 app/models/user_feedback.rb       |  2 ++
 app/models/wiki_page.rb           | 12 +-----------
 app/models/wiki_page_version.rb   |  2 ++
 test/unit/comment_test.rb         |  2 +-
 test/unit/dmail_test.rb           |  8 ++++----
 test/unit/forum_post_test.rb      |  4 ++--
 test/unit/note_test.rb            |  4 ++--
 21 files changed, 56 insertions(+), 122 deletions(-)

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 9298b4f45..a28754a34 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -5,15 +5,17 @@ class ApplicationRecord < ActiveRecord::Base
 
   concerning :SearchMethods do
     class_methods do
-      def attribute_matches(attribute, value)
+      def attribute_matches(attribute, value, **options)
         return all if value.nil?
 
         column = column_for_attribute(attribute)
         case column.sql_type_metadata.type
         when :boolean
-          boolean_attribute_matches(attribute, value)
+          boolean_attribute_matches(attribute, value, **options)
         when :integer, :datetime
-          numeric_attribute_matches(attribute, value)
+          numeric_attribute_matches(attribute, value, **options)
+        when :string, :text
+          text_attribute_matches(attribute, value, **options)
         else
           raise ArgumentError, "unhandled attribute type"
         end
@@ -40,6 +42,19 @@ class ApplicationRecord < ActiveRecord::Base
         PostQueryBuilder.new(nil).add_range_relation(parsed_range, qualified_column, self)
       end
 
+      def text_attribute_matches(attribute, value, index_column: nil, ts_config: "english")
+        column = column_for_attribute(attribute)
+        qualified_column = "#{table_name}.#{column.name}"
+
+        if value =~ /\*/
+          where("lower(#{qualified_column}) LIKE :value ESCAPE E'\\\\'", value: value.mb_chars.downcase.to_escaped_for_sql_like)
+        elsif index_column.present?
+          where("#{table_name}.#{index_column} @@ plainto_tsquery(:ts_config, :value)", ts_config: ts_config, value: value)
+        else
+          where("to_tsvector(:ts_config, #{qualified_column}) @@ plainto_tsquery(:ts_config, :value)", ts_config: ts_config, value: value)
+        end
+      end
+
       def apply_default_order(params)
         if params[:order] == "custom"
           parse_ids = Tag.parse_helper(params[:id])
diff --git a/app/models/ban.rb b/app/models/ban.rb
index c91e7d666..549af1d96 100644
--- a/app/models/ban.rb
+++ b/app/models/ban.rb
@@ -44,9 +44,7 @@ class Ban < ApplicationRecord
       q = q.where("user_id = ?", params[:user_id].to_i)
     end
 
-    if params[:reason_matches].present?
-      q = q.reason_matches(params[:reason_matches])
-    end
+    q = q.attribute_matches(:reason, params[:reason_matches])
 
     q = q.expired if params[:expired].to_s.truthy?
     q = q.unexpired if params[:expired].to_s.falsy?
diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb
index c38653519..974f6f0b4 100644
--- a/app/models/bulk_update_request.rb
+++ b/app/models/bulk_update_request.rb
@@ -58,6 +58,9 @@ class BulkUpdateRequest < ApplicationRecord
         q = q.where(status: params[:status].split(","))
       end
 
+      q = q.attribute_matches(:title, params[:title_matches])
+      q = q.attribute_matches(:script, params[:script_matches])
+
       params[:order] ||= "status_desc"
       case params[:order]
       when "id_desc"
diff --git a/app/models/comment.rb b/app/models/comment.rb
index 4c3242e03..8926de385 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -27,14 +27,6 @@ class Comment < ApplicationRecord
       reorder("comments.id desc").limit(6)
     end
 
-    def body_matches(query)
-      if query =~ /\*/ && CurrentUser.user.is_builder?
-        where("body ILIKE ? ESCAPE E'\\\\'", query.to_escaped_for_sql_like)
-      else
-        where("body_index @@ plainto_tsquery(?)", query.to_escaped_for_tsquery_split).order("comments.id DESC")
-      end
-    end
-
     def hidden(user)
       if user.is_moderator?
         where("(score < ? and is_sticky = false) or is_deleted = true", user.comment_threshold)
@@ -74,9 +66,7 @@ class Comment < ApplicationRecord
     def search(params)
       q = super
 
-      if params[:body_matches].present?
-        q = q.body_matches(params[:body_matches])
-      end
+      q = q.attribute_matches(:body, params[:body_matches], index_column: :body_index)
 
       if params[:post_id].present?
         q = q.where("post_id in (?)", params[:post_id].split(",").map(&:to_i))
diff --git a/app/models/dmail.rb b/app/models/dmail.rb
index 586e91813..10deeb8bc 100644
--- a/app/models/dmail.rb
+++ b/app/models/dmail.rb
@@ -157,20 +157,6 @@ class Dmail < ApplicationRecord
       where("is_deleted = ?", true)
     end
 
-    def title_matches(query)
-      query = "*#{query}*" unless query =~ /\*/
-      where("lower(dmails.title) LIKE ?", query.mb_chars.downcase.to_escaped_for_sql_like)
-    end
-
-    def search_message(query)
-      if query =~ /\*/ && CurrentUser.user.is_builder?
-        escaped_query = query.to_escaped_for_sql_like
-        where("(title ILIKE ? ESCAPE E'\\\\' OR body ILIKE ? ESCAPE E'\\\\')", escaped_query, escaped_query)
-      else
-        where("message_index @@ plainto_tsquery(?)", query.to_escaped_for_tsquery_split)
-      end
-    end
-
     def read
       where(is_read: true)
     end
@@ -194,13 +180,8 @@ class Dmail < ApplicationRecord
     def search(params)
       q = super
 
-      if params[:title_matches].present?
-        q = q.title_matches(params[:title_matches])
-      end
-
-      if params[:message_matches].present?
-        q = q.search_message(params[:message_matches])
-      end
+      q = q.attribute_matches(:title, params[:title_matches])
+      q = q.attribute_matches(:body, params[:message_matches], index_column: :message_index)
 
       if params[:to_name].present?
         q = q.to_name_matches(params[:to_name])
diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb
index 41b2731ef..b885998ae 100644
--- a/app/models/forum_post.rb
+++ b/app/models/forum_post.rb
@@ -32,20 +32,8 @@ class ForumPost < ApplicationRecord
   )
 
   module SearchMethods
-    def body_matches(body)
-      if body =~ /\*/ && CurrentUser.user.is_builder?
-        where("forum_posts.body ILIKE ? ESCAPE E'\\\\'", body.to_escaped_for_sql_like)
-      else
-        where("forum_posts.text_index @@ plainto_tsquery(E?)", body.to_escaped_for_tsquery)
-      end
-    end
-
     def topic_title_matches(title)
-      if title =~ /\*/ && CurrentUser.user.is_builder?
-        joins(:topic).where("forum_topics.title ILIKE ? ESCAPE E'\\\\'", title.to_escaped_for_sql_like)
-      else
-        joins(:topic).where("forum_topics.text_index @@ plainto_tsquery(E?)", title.to_escaped_for_tsquery_split)
-      end
+      joins(:topic).merge(ForumTopic.search(title_matches: title))
     end
 
     def for_user(user_id)
@@ -80,9 +68,7 @@ class ForumPost < ApplicationRecord
         q = q.topic_title_matches(params[:topic_title_matches])
       end
 
-      if params[:body_matches].present?
-        q = q.body_matches(params[:body_matches])
-      end
+      q = q.attribute_matches(:body, params[:body_matches], index_column: :text_index)
 
       if params[:creator_name].present?
         q = q.creator_name(params[:creator_name].tr(" ", "_"))
diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb
index 4ccff0664..af4c7a29a 100644
--- a/app/models/forum_topic.rb
+++ b/app/models/forum_topic.rb
@@ -51,14 +51,6 @@ class ForumTopic < ApplicationRecord
   end
 
   module SearchMethods
-    def title_matches(title)
-      if title =~ /\*/ && CurrentUser.user.is_builder?
-        where("title ILIKE ? ESCAPE E'\\\\'", title.to_escaped_for_sql_like)
-      else
-        where("text_index @@ plainto_tsquery(E?)", title.to_escaped_for_tsquery_split)
-      end
-    end
-
     def active
       where("is_deleted = false")
     end
@@ -83,9 +75,7 @@ class ForumTopic < ApplicationRecord
         q = q.where("min_level >= ?", MIN_LEVELS[:Moderator])
       end
 
-      if params[:title_matches].present?
-        q = q.title_matches(params[:title_matches])
-      end
+      q = q.attribute_matches(:title, params[:title_matches], index_column: :text_index)
 
       if params[:category_id].present?
         q = q.for_category_id(params[:category_id])
diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb
index 6fb9034bc..d93cd3f2a 100644
--- a/app/models/mod_action.rb
+++ b/app/models/mod_action.rb
@@ -56,6 +56,8 @@ class ModAction < ApplicationRecord
   def self.search(params)
     q = super
 
+    q = q.attribute_matches(:description, params[:description_matches])
+
     if params[:creator_id].present?
       q = q.where("creator_id = ?", params[:creator_id].to_i)
     end
diff --git a/app/models/note.rb b/app/models/note.rb
index e894db891..106795e0d 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -20,14 +20,6 @@ class Note < ApplicationRecord
       where("is_active = TRUE")
     end
 
-    def body_matches(query)
-      if query =~ /\*/ && CurrentUser.user.is_builder?
-        where("body ILIKE ? ESCAPE E'\\\\'", query.to_escaped_for_sql_like)
-      else
-        where("body_index @@ plainto_tsquery(E?)", query.to_escaped_for_tsquery_split)
-      end
-    end
-
     def post_tags_match(query)
       PostQueryBuilder.new(query).build(self.joins(:post)).reorder("")
     end
@@ -43,10 +35,7 @@ class Note < ApplicationRecord
     def search(params)
       q = super
 
-      if params[:body_matches].present?
-        q = q.body_matches(params[:body_matches])
-      end
-
+      q = q.attribute_matches(:body, params[:body_matches], index_column: :body_index)
       q = q.attribute_matches(:is_active, params[:is_active])
 
       if params[:post_id].present?
diff --git a/app/models/note_version.rb b/app/models/note_version.rb
index 45f386744..9b821b267 100644
--- a/app/models/note_version.rb
+++ b/app/models/note_version.rb
@@ -18,6 +18,7 @@ class NoteVersion < ApplicationRecord
     end
 
     q = q.attribute_matches(:is_active, params[:is_active])
+    q = q.attribute_matches(:body, params[:body_matches])
 
     q.apply_default_order(params)
   end
diff --git a/app/models/pool.rb b/app/models/pool.rb
index 22432acde..3d516a55c 100644
--- a/app/models/pool.rb
+++ b/app/models/pool.rb
@@ -58,9 +58,7 @@ class Pool < ApplicationRecord
         q = q.name_matches(params[:name_matches])
       end
 
-      if params[:description_matches].present?
-        q = q.where("lower(pools.description) like ? escape E'\\\\'", "%" + params[:description_matches].mb_chars.downcase.to_escaped_for_sql_like + "%")
-      end
+      q = q.attribute_matches(:description, params[:description_matches])
 
       if params[:creator_name].present?
         q = q.where("pools.creator_id = (select _.id from users _ where lower(_.name) = ?)", params[:creator_name].tr(" ", "_").mb_chars.downcase)
diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb
index 458c32a0b..c04b732e4 100644
--- a/app/models/post_appeal.rb
+++ b/app/models/post_appeal.rb
@@ -10,14 +10,6 @@ class PostAppeal < ApplicationRecord
   validates_uniqueness_of :creator_id, :scope => :post_id, :message => "have already appealed this post"
 
   module SearchMethods
-    def reason_matches(query)
-      if query =~ /\*/
-        where("post_appeals.reason ILIKE ? ESCAPE E'\\\\'", query.to_escaped_for_sql_like)
-      else
-        where("to_tsvector('english', post_appeals.reason) @@ plainto_tsquery(?)", query.to_escaped_for_tsquery)
-      end
-    end
-
     def post_tags_match(query)
       PostQueryBuilder.new(query).build(self.joins(:post))
     end
@@ -45,9 +37,7 @@ class PostAppeal < ApplicationRecord
     def search(params)
       q = super
 
-      if params[:reason_matches].present?
-        q = q.reason_matches(params[:reason_matches])
-      end
+      q = q.attribute_matches(:reason, params[:reason_matches])
 
       if params[:creator_id].present?
         q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i))
diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb
index 72a49b975..528cd57de 100644
--- a/app/models/post_flag.rb
+++ b/app/models/post_flag.rb
@@ -24,14 +24,6 @@ class PostFlag < ApplicationRecord
   scope :in_cooldown, -> { by_users.where("created_at >= ?", COOLDOWN_PERIOD.ago) }
 
   module SearchMethods
-    def reason_matches(query)
-      if query =~ /\*/
-        where("post_flags.reason ILIKE ? ESCAPE E'\\\\'", query.to_escaped_for_sql_like)
-      else
-        where("to_tsvector('english', post_flags.reason) @@ plainto_tsquery(?)", query.to_escaped_for_tsquery)
-      end
-    end
-
     def duplicate
       where("to_tsvector('english', post_flags.reason) @@ to_tsquery('dup | duplicate | sample | smaller')")
     end
@@ -67,9 +59,7 @@ class PostFlag < ApplicationRecord
     def search(params)
       q = super
 
-      if params[:reason_matches].present?
-        q = q.reason_matches(params[:reason_matches])
-      end
+      q = q.attribute_matches(:reason, params[:reason_matches])
 
       if params[:creator_id].present?
         if CurrentUser.can_view_flagger?(params[:creator_id].to_i)
diff --git a/app/models/post_replacement.rb b/app/models/post_replacement.rb
index 840a90f8b..79223d0c5 100644
--- a/app/models/post_replacement.rb
+++ b/app/models/post_replacement.rb
@@ -27,6 +27,13 @@ class PostReplacement < ApplicationRecord
       def search(params = {})
         q = super
 
+        q = q.attribute_matches(:replacement_url, params[:replacement_url])
+        q = q.attribute_matches(:original_url, params[:original_url])
+        q = q.attribute_matches(:file_ext_was, params[:file_ext_was])
+        q = q.attribute_matches(:file_ext, params[:file_ext])
+        q = q.attribute_matches(:md5_was, params[:md5_was])
+        q = q.attribute_matches(:md5, params[:md5])
+
         if params[:creator_id].present?
           q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i))
         end
diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb
index b18bef408..4639fe3f8 100644
--- a/app/models/user_feedback.rb
+++ b/app/models/user_feedback.rb
@@ -48,6 +48,8 @@ class UserFeedback < ApplicationRecord
     def search(params)
       q = super
 
+      q = q.attribute_matches(:body, params[:body_matches])
+
       if params[:user_id].present?
         q = q.for_user(params[:user_id].to_i)
       end
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index b65c3045b..185b33660 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -33,14 +33,6 @@ class WikiPage < ApplicationRecord
       order("updated_at DESC").limit(25)
     end
 
-    def body_matches(query)
-      if query =~ /\*/ && CurrentUser.user.is_builder?
-        where("body ILIKE ? ESCAPE E'\\\\'", query.to_escaped_for_sql_like)
-      else
-        where("body_index @@ plainto_tsquery(?)", query.to_escaped_for_tsquery_split)
-      end
-    end
-
     def other_names_equal(name)
       query_sql = name.unicode_normalize(:nfkc).to_escaped_for_tsquery
       where("other_names_index @@ to_tsquery('danbooru', E?)", query_sql)
@@ -70,9 +62,7 @@ class WikiPage < ApplicationRecord
         q = q.where("creator_id = ?", params[:creator_id])
       end
 
-      if params[:body_matches].present?
-        q = q.body_matches(params[:body_matches])
-      end
+      q = q.attribute_matches(:body, params[:body_matches], index_column: :body_index, ts_config: "danbooru")
 
       if params[:other_names_match].present?
         q = q.other_names_match(params[:other_names_match])
diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb
index e7a07d045..21f0ba4e8 100644
--- a/app/models/wiki_page_version.rb
+++ b/app/models/wiki_page_version.rb
@@ -20,6 +20,8 @@ class WikiPageVersion < ApplicationRecord
         q = q.where("wiki_page_id = ?", params[:wiki_page_id].to_i)
       end
 
+      q = q.attribute_matches(:title, params[:title])
+      q = q.attribute_matches(:body, params[:body])
       q = q.attribute_matches(:is_locked, params[:is_locked])
       q = q.attribute_matches(:is_deleted, params[:is_deleted])
 
diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb
index f95e69879..6cbbc6d5f 100644
--- a/test/unit/comment_test.rb
+++ b/test/unit/comment_test.rb
@@ -218,7 +218,7 @@ class CommentTest < ActiveSupport::TestCase
         c2 = FactoryBot.create(:comment, :body => "aaa ddd")
         c3 = FactoryBot.create(:comment, :body => "eee")
 
-        matches = Comment.body_matches("aaa")
+        matches = Comment.search(body_matches: "aaa")
         assert_equal(2, matches.count)
         assert_equal(c2.id, matches.all[0].id)
         assert_equal(c1.id, matches.all[1].id)
diff --git a/test/unit/dmail_test.rb b/test/unit/dmail_test.rb
index 7c3ad420f..f064317ef 100644
--- a/test/unit/dmail_test.rb
+++ b/test/unit/dmail_test.rb
@@ -103,10 +103,10 @@ class DmailTest < ActiveSupport::TestCase
       should "return results based on title contents" do
         dmail = FactoryBot.create(:dmail, :title => "xxx", :owner => @user)
 
-        matches = Dmail.search(title_matches: "x")
+        matches = Dmail.search(title_matches: "x*")
         assert_equal([dmail.id], matches.map(&:id))
 
-        matches = Dmail.search(title_matches: "X")
+        matches = Dmail.search(title_matches: "X*")
         assert_equal([dmail.id], matches.map(&:id))
 
         matches = Dmail.search(message_matches: "xxx")
@@ -118,9 +118,9 @@ class DmailTest < ActiveSupport::TestCase
 
       should "return results based on body contents" do
         dmail = FactoryBot.create(:dmail, :body => "xxx", :owner => @user)
-        matches = Dmail.search_message("xxx")
+        matches = Dmail.search(message_matches: "xxx")
         assert(matches.any?)
-        matches = Dmail.search_message("aaa")
+        matches = Dmail.search(message_matches: "aaa")
         assert(matches.empty?)
       end
     end
diff --git a/test/unit/forum_post_test.rb b/test/unit/forum_post_test.rb
index 501680866..5589922ab 100644
--- a/test/unit/forum_post_test.rb
+++ b/test/unit/forum_post_test.rb
@@ -164,8 +164,8 @@ class ForumPostTest < ActiveSupport::TestCase
 
     should "be searchable by body content" do
       post = FactoryBot.create(:forum_post, :topic_id => @topic.id, :body => "xxx")
-      assert_equal(1, ForumPost.body_matches("xxx").count)
-      assert_equal(0, ForumPost.body_matches("aaa").count)
+      assert_equal(1, ForumPost.search(body_matches: "xxx").count)
+      assert_equal(0, ForumPost.search(body_matches: "aaa").count)
     end
 
     should "initialize its creator" do
diff --git a/test/unit/note_test.rb b/test/unit/note_test.rb
index d091d3362..8b9e57331 100644
--- a/test/unit/note_test.rb
+++ b/test/unit/note_test.rb
@@ -196,13 +196,13 @@ class NoteTest < ActiveSupport::TestCase
 
       context "where the body contains the string 'aaa'" do
         should "return a hit" do
-          assert_equal(1, Note.body_matches("aaa").count)
+          assert_equal(1, Note.search(body_matches: "aaa").count)
         end
       end
 
       context "where the body contains the string 'bbb'" do
         should "return no hits" do
-          assert_equal(0, Note.body_matches("bbb").count)
+          assert_equal(0, Note.search(body_matches: "bbb").count)
         end
       end
     end