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.
This commit is contained in:
evazion
2018-08-31 19:23:25 -05:00
parent 736c22c3ce
commit 0eff095a3e
21 changed files with 56 additions and 122 deletions

View File

@@ -5,15 +5,17 @@ class ApplicationRecord < ActiveRecord::Base
concerning :SearchMethods do concerning :SearchMethods do
class_methods do class_methods do
def attribute_matches(attribute, value) def attribute_matches(attribute, value, **options)
return all if value.nil? return all if value.nil?
column = column_for_attribute(attribute) column = column_for_attribute(attribute)
case column.sql_type_metadata.type case column.sql_type_metadata.type
when :boolean when :boolean
boolean_attribute_matches(attribute, value) boolean_attribute_matches(attribute, value, **options)
when :integer, :datetime when :integer, :datetime
numeric_attribute_matches(attribute, value) numeric_attribute_matches(attribute, value, **options)
when :string, :text
text_attribute_matches(attribute, value, **options)
else else
raise ArgumentError, "unhandled attribute type" raise ArgumentError, "unhandled attribute type"
end end
@@ -40,6 +42,19 @@ class ApplicationRecord < ActiveRecord::Base
PostQueryBuilder.new(nil).add_range_relation(parsed_range, qualified_column, self) PostQueryBuilder.new(nil).add_range_relation(parsed_range, qualified_column, self)
end 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) def apply_default_order(params)
if params[:order] == "custom" if params[:order] == "custom"
parse_ids = Tag.parse_helper(params[:id]) parse_ids = Tag.parse_helper(params[:id])

View File

@@ -44,9 +44,7 @@ class Ban < ApplicationRecord
q = q.where("user_id = ?", params[:user_id].to_i) q = q.where("user_id = ?", params[:user_id].to_i)
end end
if params[:reason_matches].present? q = q.attribute_matches(:reason, params[:reason_matches])
q = q.reason_matches(params[:reason_matches])
end
q = q.expired if params[:expired].to_s.truthy? q = q.expired if params[:expired].to_s.truthy?
q = q.unexpired if params[:expired].to_s.falsy? q = q.unexpired if params[:expired].to_s.falsy?

View File

@@ -58,6 +58,9 @@ class BulkUpdateRequest < ApplicationRecord
q = q.where(status: params[:status].split(",")) q = q.where(status: params[:status].split(","))
end end
q = q.attribute_matches(:title, params[:title_matches])
q = q.attribute_matches(:script, params[:script_matches])
params[:order] ||= "status_desc" params[:order] ||= "status_desc"
case params[:order] case params[:order]
when "id_desc" when "id_desc"

View File

@@ -27,14 +27,6 @@ class Comment < ApplicationRecord
reorder("comments.id desc").limit(6) reorder("comments.id desc").limit(6)
end 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) def hidden(user)
if user.is_moderator? if user.is_moderator?
where("(score < ? and is_sticky = false) or is_deleted = true", user.comment_threshold) where("(score < ? and is_sticky = false) or is_deleted = true", user.comment_threshold)
@@ -74,9 +66,7 @@ class Comment < ApplicationRecord
def search(params) def search(params)
q = super q = super
if params[:body_matches].present? q = q.attribute_matches(:body, params[:body_matches], index_column: :body_index)
q = q.body_matches(params[:body_matches])
end
if params[:post_id].present? if params[:post_id].present?
q = q.where("post_id in (?)", params[:post_id].split(",").map(&:to_i)) q = q.where("post_id in (?)", params[:post_id].split(",").map(&:to_i))

View File

@@ -157,20 +157,6 @@ class Dmail < ApplicationRecord
where("is_deleted = ?", true) where("is_deleted = ?", true)
end 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 def read
where(is_read: true) where(is_read: true)
end end
@@ -194,13 +180,8 @@ class Dmail < ApplicationRecord
def search(params) def search(params)
q = super q = super
if params[:title_matches].present? q = q.attribute_matches(:title, params[:title_matches])
q = q.title_matches(params[:title_matches]) q = q.attribute_matches(:body, params[:message_matches], index_column: :message_index)
end
if params[:message_matches].present?
q = q.search_message(params[:message_matches])
end
if params[:to_name].present? if params[:to_name].present?
q = q.to_name_matches(params[:to_name]) q = q.to_name_matches(params[:to_name])

View File

@@ -32,20 +32,8 @@ class ForumPost < ApplicationRecord
) )
module SearchMethods 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) def topic_title_matches(title)
if title =~ /\*/ && CurrentUser.user.is_builder? joins(:topic).merge(ForumTopic.search(title_matches: title))
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
end end
def for_user(user_id) def for_user(user_id)
@@ -80,9 +68,7 @@ class ForumPost < ApplicationRecord
q = q.topic_title_matches(params[:topic_title_matches]) q = q.topic_title_matches(params[:topic_title_matches])
end end
if params[:body_matches].present? q = q.attribute_matches(:body, params[:body_matches], index_column: :text_index)
q = q.body_matches(params[:body_matches])
end
if params[:creator_name].present? if params[:creator_name].present?
q = q.creator_name(params[:creator_name].tr(" ", "_")) q = q.creator_name(params[:creator_name].tr(" ", "_"))

View File

@@ -51,14 +51,6 @@ class ForumTopic < ApplicationRecord
end end
module SearchMethods 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 def active
where("is_deleted = false") where("is_deleted = false")
end end
@@ -83,9 +75,7 @@ class ForumTopic < ApplicationRecord
q = q.where("min_level >= ?", MIN_LEVELS[:Moderator]) q = q.where("min_level >= ?", MIN_LEVELS[:Moderator])
end end
if params[:title_matches].present? q = q.attribute_matches(:title, params[:title_matches], index_column: :text_index)
q = q.title_matches(params[:title_matches])
end
if params[:category_id].present? if params[:category_id].present?
q = q.for_category_id(params[:category_id]) q = q.for_category_id(params[:category_id])

View File

@@ -56,6 +56,8 @@ class ModAction < ApplicationRecord
def self.search(params) def self.search(params)
q = super q = super
q = q.attribute_matches(:description, params[:description_matches])
if params[:creator_id].present? if params[:creator_id].present?
q = q.where("creator_id = ?", params[:creator_id].to_i) q = q.where("creator_id = ?", params[:creator_id].to_i)
end end

View File

@@ -20,14 +20,6 @@ class Note < ApplicationRecord
where("is_active = TRUE") where("is_active = TRUE")
end 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) def post_tags_match(query)
PostQueryBuilder.new(query).build(self.joins(:post)).reorder("") PostQueryBuilder.new(query).build(self.joins(:post)).reorder("")
end end
@@ -43,10 +35,7 @@ class Note < ApplicationRecord
def search(params) def search(params)
q = super q = super
if params[:body_matches].present? q = q.attribute_matches(:body, params[:body_matches], index_column: :body_index)
q = q.body_matches(params[:body_matches])
end
q = q.attribute_matches(:is_active, params[:is_active]) q = q.attribute_matches(:is_active, params[:is_active])
if params[:post_id].present? if params[:post_id].present?

View File

@@ -18,6 +18,7 @@ class NoteVersion < ApplicationRecord
end end
q = q.attribute_matches(:is_active, params[:is_active]) q = q.attribute_matches(:is_active, params[:is_active])
q = q.attribute_matches(:body, params[:body_matches])
q.apply_default_order(params) q.apply_default_order(params)
end end

View File

@@ -58,9 +58,7 @@ class Pool < ApplicationRecord
q = q.name_matches(params[:name_matches]) q = q.name_matches(params[:name_matches])
end end
if params[:description_matches].present? q = q.attribute_matches(:description, params[:description_matches])
q = q.where("lower(pools.description) like ? escape E'\\\\'", "%" + params[:description_matches].mb_chars.downcase.to_escaped_for_sql_like + "%")
end
if params[:creator_name].present? 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) q = q.where("pools.creator_id = (select _.id from users _ where lower(_.name) = ?)", params[:creator_name].tr(" ", "_").mb_chars.downcase)

View File

@@ -10,14 +10,6 @@ class PostAppeal < ApplicationRecord
validates_uniqueness_of :creator_id, :scope => :post_id, :message => "have already appealed this post" validates_uniqueness_of :creator_id, :scope => :post_id, :message => "have already appealed this post"
module SearchMethods 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) def post_tags_match(query)
PostQueryBuilder.new(query).build(self.joins(:post)) PostQueryBuilder.new(query).build(self.joins(:post))
end end
@@ -45,9 +37,7 @@ class PostAppeal < ApplicationRecord
def search(params) def search(params)
q = super q = super
if params[:reason_matches].present? q = q.attribute_matches(:reason, params[:reason_matches])
q = q.reason_matches(params[:reason_matches])
end
if params[:creator_id].present? if params[:creator_id].present?
q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i)) q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i))

View File

@@ -24,14 +24,6 @@ class PostFlag < ApplicationRecord
scope :in_cooldown, -> { by_users.where("created_at >= ?", COOLDOWN_PERIOD.ago) } scope :in_cooldown, -> { by_users.where("created_at >= ?", COOLDOWN_PERIOD.ago) }
module SearchMethods 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 def duplicate
where("to_tsvector('english', post_flags.reason) @@ to_tsquery('dup | duplicate | sample | smaller')") where("to_tsvector('english', post_flags.reason) @@ to_tsquery('dup | duplicate | sample | smaller')")
end end
@@ -67,9 +59,7 @@ class PostFlag < ApplicationRecord
def search(params) def search(params)
q = super q = super
if params[:reason_matches].present? q = q.attribute_matches(:reason, params[:reason_matches])
q = q.reason_matches(params[:reason_matches])
end
if params[:creator_id].present? if params[:creator_id].present?
if CurrentUser.can_view_flagger?(params[:creator_id].to_i) if CurrentUser.can_view_flagger?(params[:creator_id].to_i)

View File

@@ -27,6 +27,13 @@ class PostReplacement < ApplicationRecord
def search(params = {}) def search(params = {})
q = super 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? if params[:creator_id].present?
q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i)) q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i))
end end

View File

@@ -48,6 +48,8 @@ class UserFeedback < ApplicationRecord
def search(params) def search(params)
q = super q = super
q = q.attribute_matches(:body, params[:body_matches])
if params[:user_id].present? if params[:user_id].present?
q = q.for_user(params[:user_id].to_i) q = q.for_user(params[:user_id].to_i)
end end

View File

@@ -33,14 +33,6 @@ class WikiPage < ApplicationRecord
order("updated_at DESC").limit(25) order("updated_at DESC").limit(25)
end 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) def other_names_equal(name)
query_sql = name.unicode_normalize(:nfkc).to_escaped_for_tsquery query_sql = name.unicode_normalize(:nfkc).to_escaped_for_tsquery
where("other_names_index @@ to_tsquery('danbooru', E?)", query_sql) 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]) q = q.where("creator_id = ?", params[:creator_id])
end end
if params[:body_matches].present? q = q.attribute_matches(:body, params[:body_matches], index_column: :body_index, ts_config: "danbooru")
q = q.body_matches(params[:body_matches])
end
if params[:other_names_match].present? if params[:other_names_match].present?
q = q.other_names_match(params[:other_names_match]) q = q.other_names_match(params[:other_names_match])

View File

@@ -20,6 +20,8 @@ class WikiPageVersion < ApplicationRecord
q = q.where("wiki_page_id = ?", params[:wiki_page_id].to_i) q = q.where("wiki_page_id = ?", params[:wiki_page_id].to_i)
end 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_locked, params[:is_locked])
q = q.attribute_matches(:is_deleted, params[:is_deleted]) q = q.attribute_matches(:is_deleted, params[:is_deleted])

View File

@@ -218,7 +218,7 @@ class CommentTest < ActiveSupport::TestCase
c2 = FactoryBot.create(:comment, :body => "aaa ddd") c2 = FactoryBot.create(:comment, :body => "aaa ddd")
c3 = FactoryBot.create(:comment, :body => "eee") c3 = FactoryBot.create(:comment, :body => "eee")
matches = Comment.body_matches("aaa") matches = Comment.search(body_matches: "aaa")
assert_equal(2, matches.count) assert_equal(2, matches.count)
assert_equal(c2.id, matches.all[0].id) assert_equal(c2.id, matches.all[0].id)
assert_equal(c1.id, matches.all[1].id) assert_equal(c1.id, matches.all[1].id)

View File

@@ -103,10 +103,10 @@ class DmailTest < ActiveSupport::TestCase
should "return results based on title contents" do should "return results based on title contents" do
dmail = FactoryBot.create(:dmail, :title => "xxx", :owner => @user) 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)) 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)) assert_equal([dmail.id], matches.map(&:id))
matches = Dmail.search(message_matches: "xxx") matches = Dmail.search(message_matches: "xxx")
@@ -118,9 +118,9 @@ class DmailTest < ActiveSupport::TestCase
should "return results based on body contents" do should "return results based on body contents" do
dmail = FactoryBot.create(:dmail, :body => "xxx", :owner => @user) dmail = FactoryBot.create(:dmail, :body => "xxx", :owner => @user)
matches = Dmail.search_message("xxx") matches = Dmail.search(message_matches: "xxx")
assert(matches.any?) assert(matches.any?)
matches = Dmail.search_message("aaa") matches = Dmail.search(message_matches: "aaa")
assert(matches.empty?) assert(matches.empty?)
end end
end end

View File

@@ -164,8 +164,8 @@ class ForumPostTest < ActiveSupport::TestCase
should "be searchable by body content" do should "be searchable by body content" do
post = FactoryBot.create(:forum_post, :topic_id => @topic.id, :body => "xxx") post = FactoryBot.create(:forum_post, :topic_id => @topic.id, :body => "xxx")
assert_equal(1, ForumPost.body_matches("xxx").count) assert_equal(1, ForumPost.search(body_matches: "xxx").count)
assert_equal(0, ForumPost.body_matches("aaa").count) assert_equal(0, ForumPost.search(body_matches: "aaa").count)
end end
should "initialize its creator" do should "initialize its creator" do

View File

@@ -196,13 +196,13 @@ class NoteTest < ActiveSupport::TestCase
context "where the body contains the string 'aaa'" do context "where the body contains the string 'aaa'" do
should "return a hit" 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
end end
context "where the body contains the string 'bbb'" do context "where the body contains the string 'bbb'" do
should "return no hits" 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 end
end end