jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

searchable: fix bug with searching multiple association attributes.

Browse Source 
Fix a bug with searches like the following not working correctly:

* https://danbooru.donmai.us/comments.json?search[creator][level]=20&search[creator_id]=1234
* https://danbooru.donmai.us/comments.json?search[creator][level]=20&search[creator_name]=abcd
* https://danbooru.donmai.us/comments.json?search[post][rating]=s&search[post_tags_match]=touhou

It wasn't possible to search for both `creator` and `creator_id` at the
same time (or `post` and `post_tags_match`, etc). Only the `creator_id`
param would be recognized.

Also refactor some internals:

* `search_includes` was renamed to `search_associated_attribute`.
* `search_attribute` was split up into `search_basic_attribute` and
  `search_associated_attribute`.
This commit is contained in:
evazion
2021-01-07 15:59:56 -06:00
parent e356fd0fd5
commit da3e8e4726
2 changed files with 121 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
app/logical/concerns/searchable.rb
View File

@@ -10,10 +10,6 @@ module Searchable
1 + params.values.map { |v| parameter_hash?(v) ? parameter_depth(v) : 1 }.max
end
def prefix_matches(prefix, params)
params.keys.any? { |x| x.starts_with?(prefix) }
end
def negate_relation
unscoped.where(all.where_clause.invert.ast)
end
@@ -172,8 +168,18 @@ module Searchable
end
def search_attribute(name, params, current_user)
if has_attribute?(name)
search_basic_attribute(name, params, current_user)
elsif reflections.has_key?(name.to_s)
search_association_attribute(name, params, current_user)
else
raise ArgumentError, "#{name} is not an attribute or association"
end
end
def search_basic_attribute(name, params, current_user)
column = column_for_attribute(name)
type = column.type || reflect_on_association(name)&.class_name
type = column.type
if column.try(:array?)
subtype = type
@@ -183,12 +189,6 @@ module Searchable
end
case type
when "User"
search_user_attribute(name, params, current_user)
when "Post"
search_post_attribute(name, params, current_user)
when "Model"
search_polymorphic_attribute(name, params, current_user)
when :string, :text
search_text_attribute(name, params)
when :boolean
@@ -202,8 +202,7 @@ module Searchable
when :array
search_array_attribute(name, subtype, params)
else
raise NotImplementedError, "unhandled attribute type: #{name}" if type.blank?
search_includes(name, params, type, current_user)
raise NotImplementedError, "unhandled attribute type: #{name}"
end
end
@@ -265,33 +264,36 @@ module Searchable
end
end
def search_user_attribute(attr, params, current_user)
if params["#{attr}_name"].present?
where(attr => User.search(name_matches: params["#{attr}_name"]).reorder(nil))
else
search_includes(attr, params, "User", current_user)
end
end
def search_association_attribute(attr, params, current_user)
association = reflect_on_association(attr)
relation = all
def search_post_attribute(attr, params, current_user)
if params["#{attr}_tags_match"]
where(attr => Post.user_tag_match(params["#{attr}_tags_match"], current_user).reorder(nil))
else
search_includes(attr, params, "Post", current_user)
if association.polymorphic?
return search_polymorphic_attribute(attr, params, current_user)
end
end
def search_includes(attr, params, type, current_user)
model = Kernel.const_get(type)
if prefix_matches("#{attr}_id", params)
search_attribute("#{attr}_id", params, current_user)
elsif params["has_#{attr}"].to_s.truthy? || params["has_#{attr}"].to_s.falsy?
search_has_include(attr, params["has_#{attr}"].to_s.truthy?, model)
elsif parameter_hash?(params[attr])
where(attr => model.visible(current_user).search(params[attr]).reorder(nil))
else
all
if association.belongs_to?
relation = relation.search_attribute(association.foreign_key, params, current_user)
end
model = association.klass
if model == User && params["#{attr}_name"].present?
relation = relation.where(attr => User.search(name_matches: params["#{attr}_name"]).reorder(nil))
end
if model == Post && params["#{attr}_tags_match"].present?
relation = relation.where(attr => Post.user_tag_match(params["#{attr}_tags_match"], current_user).reorder(nil))
end
if params["has_#{attr}"].to_s.truthy? || params["has_#{attr}"].to_s.falsy?
relation = relation.search_has_include(attr, params["has_#{attr}"].to_s.truthy?, model)
end
if parameter_hash?(params[attr])
relation = relation.where(attr => model.visible(current_user).search(params[attr]).reorder(nil))
end
relation
end
def search_polymorphic_attribute(attr, params, current_user)

85
test/unit/concerns/searchable.rb
View File

@@ -1,8 +1,10 @@
require 'test_helper'
class SearchableTest < ActiveSupport::TestCase
def assert_search_equals(results, **params)
assert_equal(Array(results).map(&:id), subject.search(**params).ids)
def assert_search_equals(results, current_user: User.anonymous, **params)
as(current_user) do
assert_equal(Array(results).map(&:id), subject.search(**params).ids)
end
end
context "#search method" do
@@ -14,6 +16,14 @@ class SearchableTest < ActiveSupport::TestCase
@p3 = create(:post, source: "c3", score: 3, is_deleted: false)
end
context "for a nonexistent attribute" do
should "raise an error" do
assert_raises(ArgumentError) do
Post.search_attribute(:answer, 42, User.anonymous)
end
end
end
context "for a numeric attribute" do
should "support basic operators" do
assert_search_equals(@p1, score_eq: 1)
@@ -120,5 +130,76 @@ class SearchableTest < ActiveSupport::TestCase
assert_search_equals(@wp, other_name_count: 2)
end
end
context "for a belongs_to association" do
context "for a user association" do
should "work" do
assert_search_equals(@p1, uploader_id: @p1.uploader_id)
assert_search_equals(@p1, uploader_name: @p1.uploader.name)
assert_search_equals(@p1, uploader: { id: @p1.uploader_id })
assert_search_equals(@p1, uploader: { name: @p1.uploader.name })
assert_search_equals(@p1, uploader: { name: @p1.uploader.name }, uploader_id: @p1.uploader.id)
assert_search_equals([], uploader: { name: @p1.uploader.name }, uploader_id: @p2.uploader.id)
end
end
context "for a post association" do
should "work" do
@p1.update!(parent: @p2)
assert_search_equals(@p1, parent_id: @p2.id)
assert_search_equals(@p1, parent: { id: @p2.id })
assert_search_equals(@p1, parent_tags_match: "id:#{@p2.id}")
assert_search_equals([], parent_tags_match: "id:0")
assert_search_equals(@p2, children_tags_match: "id:#{@p1.id}")
assert_search_equals([], children_tags_match: "id:0")
assert_search_equals(@p1, has_parent: true)
assert_search_equals([@p3, @p2], has_parent: false)
end
end
context "for a polymorphic association" do
subject { ModerationReport }
should "work" do
as(create(:user)) do
@mr1 = create(:moderation_report, model: create(:comment))
@mr2 = create(:moderation_report, model: create(:forum_post))
@mr3 = create(:moderation_report, model: create(:dmail))
end
assert_search_equals(@mr1, model_type: "Comment")
assert_search_equals(@mr2, model_type: "ForumPost")
assert_search_equals(@mr3, model_type: "Dmail")
assert_search_equals(@mr1, model_type: "Comment", model_id: @mr1.model.id)
assert_search_equals(@mr2, model_type: "ForumPost", model_id: @mr2.model.id)
assert_search_equals(@mr3, model_type: "Dmail", model_id: @mr3.model.id)
assert_search_equals(@mr1, Comment: { body: @mr1.model.body })
assert_search_equals(@mr2, ForumPost: { body: @mr2.model.body })
assert_search_equals([], Dmail: { body: @mr3.model.body }, current_user: User.anonymous)
assert_search_equals(@mr3, Dmail: { body: @mr3.model.body }, current_user: @mr3.model.owner)
end
end
end
context "for a has_many association" do
should "work" do
as(@p1.uploader) { create(:comment, post: @p1) }
assert_search_equals(@p1, has_comments: true)
assert_search_equals([@p3, @p2], has_comments: false)
assert_search_equals(@p1, comments: { id: @p1.comments.first.id })
assert_search_equals(@p1, has_comments: true, comments: { id: @p1.comments.first.id })
end
end
end
end