kotovalexarian-likes-github
/
activerecord-hackery--ransack
mirror of https://github.com/activerecord-hackery/ransack.git
1
0
Fork 0
Code Releases Activity

Merge pull request #1004 from gregmolnar/fix-polymorphic-joins 

fix polymorphic joins with rails > 5.2
This commit is contained in:
Greg Molnar 2019-05-11 13:14:34 +02:00 committed by GitHub
parent d44bfe6fe2 0d736a1186
commit 5170f735d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
27 changed files with 104 additions and 370 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -4,3 +4,4 @@ Gemfile.lock
pkg/*
coverage/*
.DS_Store
.byebug_history

5
CHANGELOG.md
View File

@ -2,6 +2,11 @@
## Unreleased
* Fix wrong table alias when using nested join. for ActiveRecord >= 5.2
PR [374](https://github.com/activerecord-hackery/ransack/pull/374)
*hiichan*
## Version 2.1.1 - 2018-12-05
* Add `arabic` translation

12
Gemfile
View File

@ -3,7 +3,7 @@ gemspec
gem 'rake'
rails = ENV['RAILS'] || '5-0-stable'
rails = ENV['RAILS'] || '5-2-stable'
gem 'pry'
@ -22,9 +22,6 @@ when /^v/ # A tagged version
gem 'activerecord', require: false
gem 'actionpack'
end
if rails >= 'v5.2.0'
gem 'mysql2', '~> 0.4.4'
end
else
git 'git://github.com/rails/rails.git', :branch => rails do
gem 'activesupport'
@ -32,13 +29,8 @@ else
gem 'activerecord', require: false
gem 'actionpack'
end
if rails == '3-0-stable'
gem 'mysql2', '< 0.3'
end
if rails == '5-2-stable'
gem 'mysql2', '~> 0.4.4'
end
end
gem 'mysql2', '~> 0.4.4'
group :test do
# TestUnit was removed from Ruby 2.2 but still needed for testing Rails 3.x.

10
lib/ransack/adapters/active_record/context.rb
View File

@ -312,7 +312,7 @@ module Ransack
alias_tracker = ::ActiveRecord::Associations::AliasTracker.create(self.klass.connection, parent.table.name, [])
jd = Polyamorous::JoinDependency.new(
parent.base_klass,
parent.base_klass.arel_table,
parent.table,
Polyamorous::Join.new(name, @join_type, klass),
alias_tracker
)
@ -320,13 +320,12 @@ module Ransack
else
jd = Polyamorous::JoinDependency.new(
parent.base_klass,
parent.base_klass.arel_table,
parent.table,
Polyamorous::Join.new(name, @join_type, klass),
)
found_association = jd.instance_variable_get(:@join_root).children.last
end
@associations_pot[found_association] = parent
# TODO maybe we dont need to push associations here, we could loop
@ -337,14 +336,11 @@ module Ransack
if ::ActiveRecord::VERSION::STRING > Constants::RAILS_5_2_0
@join_dependency.send(:construct_tables!, jd.instance_variable_get(:@join_root))
else
@join_dependency.send(
:construct_tables!, jd.instance_variable_get(:@join_root), found_association
)
@join_dependency.send(:construct_tables!, jd.instance_variable_get(:@join_root), found_association)
end
# Leverage the stashed association functionality in AR
@object = @object.joins(jd)
found_association
end

5
polyamorous/lib/polyamorous.rb
View File

@ -19,6 +19,11 @@ if defined?(::ActiveRecord)
require "polyamorous/activerecord_#{ar_version}_ruby_2/#{file}"
end
if ar_version >= "5.2.0"
require "polyamorous/activerecord_#{ar_version}_ruby_2/reflection.rb"
::ActiveRecord::Reflection::AbstractReflection.send(:prepend, Polyamorous::ReflectionExtensions)
end
Polyamorous::JoinDependency.send(:prepend, Polyamorous::JoinDependencyExtensions)
Polyamorous::JoinDependency.singleton_class.send(:prepend, Polyamorous::JoinDependencyExtensions::ClassMethods)
Polyamorous::JoinAssociation.send(:prepend, Polyamorous::JoinAssociationExtensions)

3
polyamorous/lib/polyamorous/activerecord_5.1_ruby_2/join_association.rb
View File

@ -7,8 +7,7 @@ module Polyamorous
base.class_eval { attr_reader :join_type }
end
def initialize(reflection, children, polymorphic_class = nil,
join_type = Arel::Nodes::InnerJoin)
def initialize(reflection, children, polymorphic_class = nil, join_type = Arel::Nodes::InnerJoin)
@join_type = join_type
if polymorphic_class && ::ActiveRecord::Base > polymorphic_class
swapping_reflection_klass(reflection, polymorphic_class) do |reflection|

5
polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_association.rb
View File

@ -7,8 +7,7 @@ module Polyamorous
base.class_eval { attr_reader :join_type }
end
def initialize(reflection, children, alias_tracker, polymorphic_class = nil,
join_type = Arel::Nodes::InnerJoin)
def initialize(reflection, children, alias_tracker, polymorphic_class = nil, join_type = Arel::Nodes::InnerJoin)
@join_type = join_type
if polymorphic_class && ::ActiveRecord::Base > polymorphic_class
swapping_reflection_klass(reflection, polymorphic_class) do |reflection|
@ -23,7 +22,7 @@ module Polyamorous
def build_constraint(klass, table, key, foreign_table, foreign_key)
if reflection.polymorphic?
super(klass, table, key, foreign_table, foreign_key)
.and(foreign_table[reflection.foreign_type].eq(reflection.klass.name))
.and(foreign_table[reflection.foreign_type].eq(reflection.klass.name))
else
super(klass, table, key, foreign_table, foreign_key)
end

3
polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb
View File

@ -38,7 +38,6 @@ module Polyamorous
# passing an additional argument, `join_type`, to #join_constraints.
#
def join_constraints(outer_joins, join_type)
@alias_tracker = alias_tracker
joins = join_root.children.flat_map { |child|
if join_type == Arel::Nodes::OuterJoin
make_polyamorous_left_outer_joins join_root, child
@ -48,7 +47,7 @@ module Polyamorous
}
joins.concat outer_joins.flat_map { |oj|
if join_root.match? oj.join_root
if join_root.match?(oj.join_root) && join_root.table.name == oj.join_root.table.name
walk(join_root, oj.join_root)
else
oj.join_root.children.flat_map { |child|

12
polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb Normal file
View File

@ -0,0 +1,12 @@
module Polyamorous
module ReflectionExtensions
def build_join_constraint(table, foreign_table)
if polymorphic?
super(table, foreign_table)
.and(foreign_table[foreign_type].eq(klass.name))
else
super(table, foreign_table)
end
end
end
end

9
polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_association.rb
View File

@ -18,14 +18,5 @@ module Polyamorous
super(reflection, children)
end
end
def build_constraint(klass, table, key, foreign_table, foreign_key)
if reflection.polymorphic?
super(klass, table, key, foreign_table, foreign_key)
.and(foreign_table[reflection.foreign_type].eq(reflection.klass.name))
else
super(klass, table, key, foreign_table, foreign_key)
end
end
end
end

26
polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb
View File

@ -3,7 +3,6 @@
module Polyamorous
module JoinDependencyExtensions
# Replaces ActiveRecord::Associations::JoinDependency#build
#
def build(associations, base_klass)
associations.map do |name, right|
if name.is_a? Join
@ -30,6 +29,31 @@ module Polyamorous
end
end
def join_constraints(joins_to_add, join_type, alias_tracker)
@alias_tracker = alias_tracker
construct_tables!(join_root)
joins = make_join_constraints(join_root, join_type)
joins.concat joins_to_add.flat_map { |oj|
construct_tables!(oj.join_root)
if join_root.match?(oj.join_root) && join_root.table.name == oj.join_root.table.name
walk join_root, oj.join_root
else
make_join_constraints(oj.join_root, join_type)
end
}
end
private
def make_constraints(parent, child, join_type = Arel::Nodes::OuterJoin)
foreign_table = parent.table
foreign_klass = parent.base_klass
join_type = child.join_type || join_type if join_type == Arel::Nodes::InnerJoin
joins = child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker)
joins.concat child.children.flat_map { |c| make_constraints(child, c, join_type) }
end
module ClassMethods
# Prepended before ActiveRecord::Associations::JoinDependency#walk_tree
#

2
polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/reflection.rb Normal file
View File

@ -0,0 +1,2 @@
# active_record_5.2.1_ruby_2/reflection.rb
require 'polyamorous/activerecord_5.2.0_ruby_2/reflection'

5
polyamorous/spec/blueprints/articles.rb
View File

@ -1,5 +0,0 @@
Article.blueprint do
person
title
body
end

5
polyamorous/spec/blueprints/comments.rb
View File

@ -1,5 +0,0 @@
Comment.blueprint do
article
person
body
end

3
polyamorous/spec/blueprints/notes.rb
View File

@ -1,3 +0,0 @@
Note.blueprint do
note
end

4
polyamorous/spec/blueprints/people.rb
View File

@ -1,4 +0,0 @@
Person.blueprint do
name
salary
end

3
polyamorous/spec/blueprints/tags.rb
View File

@ -1,3 +0,0 @@
Tag.blueprint do
name { Sham.tag_name }
end

26
polyamorous/spec/helpers/polyamorous_helper.rb
View File

@ -1,26 +0,0 @@
module PolyamorousHelper
if ActiveRecord::VERSION::STRING >= "4.1"
def new_join_association(reflection, children, klass)
Polyamorous::JoinAssociation.new reflection, children, klass
end
else
def new_join_association(reflection, join_dependency, parent, klass)
Polyamorous::JoinAssociation.new reflection, join_dependency, parent, klass
end
end
if ActiveRecord::VERSION::STRING >= "5.2"
def new_join_dependency(klass, associations = {})
alias_tracker = ::ActiveRecord::Associations::AliasTracker.create(klass.connection, klass.table_name, [])
Polyamorous::JoinDependency.new klass, klass.arel_table, associations, alias_tracker
end
else
def new_join_dependency(klass, associations = {})
Polyamorous::JoinDependency.new klass, associations, []
end
end
def new_join(name, type = Polyamorous::InnerJoin, klass = nil)
Polyamorous::Join.new name, type, klass
end
end

29
polyamorous/spec/polyamorous/join_association_spec.rb
View File

@ -1,29 +0,0 @@
require 'spec_helper'
module Polyamorous
describe JoinAssociation do
join_base, join_association_args, polymorphic = [:join_root, 'parent.children', 'reflection.options[:polymorphic]']
let(:join_dependency) { new_join_dependency Note, {} }
let(:reflection) { Note.reflect_on_association(:notable) }
let(:parent) { join_dependency.send(join_base) }
let(:join_association) {
eval("new_join_association(reflection, #{join_association_args}, Article)")
}
it 'leaves the orginal reflection intact for thread safety' do
reflection.instance_variable_set(:@klass, Article)
join_association
.swapping_reflection_klass(reflection, Person) do |new_reflection|
expect(new_reflection.options).not_to equal reflection.options
expect(new_reflection.options).not_to have_key(:polymorphic)
expect(new_reflection.klass).to eq(Person)
expect(reflection.klass).to eq(Article)
end
end
it 'sets the polymorphic option to true after initializing' do
expect(join_association.instance_eval(polymorphic)).to be true
end
end
end

93
polyamorous/spec/polyamorous/join_dependency_spec.rb
View File

@ -1,93 +0,0 @@
require 'spec_helper'
module Polyamorous
describe JoinDependency do
method, join_associations, join_base =
if ActiveRecord::VERSION::STRING >= '4.1'
[:instance_eval, 'join_root.drop(1)', :join_root]
else
[:send, 'join_associations', :join_base]
end
context 'with symbol joins' do
subject { new_join_dependency Person, articles: :comments }
specify { expect(subject.send(method, join_associations).size)
.to eq(2) }
specify { expect(subject.send(method, join_associations).map(&:join_type))
.to be_all { Polyamorous::InnerJoin } }
end
context 'with has_many :through association' do
subject { new_join_dependency Person, :authored_article_comments }
specify { expect(subject.send(method, join_associations).size)
.to eq 1 }
specify { expect(subject.send(method, join_associations).first.table_name)
.to eq 'comments' }
end
context 'with outer join' do
subject { new_join_dependency Person, new_join(:articles, :outer) }
specify { expect(subject.send(method, join_associations).size)
.to eq 1 }
specify { expect(subject.send(method, join_associations).first.join_type)
.to eq Polyamorous::OuterJoin }
end
context 'with nested outer joins' do
subject { new_join_dependency Person,
new_join(:articles, :outer) => new_join(:comments, :outer) }
specify { expect(subject.send(method, join_associations).size)
.to eq 2 }
specify { expect(subject.send(method, join_associations).map(&:join_type))
.to eq [Polyamorous::OuterJoin, Polyamorous::OuterJoin] }
specify { expect(subject.send(method, join_associations).map(&:join_type))
.to be_all { Polyamorous::OuterJoin } }
end
context 'with polymorphic belongs_to join' do
subject { new_join_dependency Note, new_join(:notable, :inner, Person) }
specify { expect(subject.send(method, join_associations).size)
.to eq 1 }
specify { expect(subject.send(method, join_associations).first.join_type)
.to eq Polyamorous::InnerJoin }
specify { expect(subject.send(method, join_associations).first.table_name)
.to eq 'people' }
end
context 'with polymorphic belongs_to join and nested symbol join' do
subject { new_join_dependency Note,
new_join(:notable, :inner, Person) => :comments }
specify { expect(subject.send(method, join_associations).size)
.to eq 2 }
specify { expect(subject.send(method, join_associations).map(&:join_type))
.to be_all { Polyamorous::InnerJoin } }
specify { expect(subject.send(method, join_associations).first.table_name)
.to eq 'people' }
specify { expect(subject.send(method, join_associations)[1].table_name)
.to eq 'comments' }
end
context '#left_outer_join in Rails 5 overrides join type specified',
if: ActiveRecord::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR < 2 do
let(:join_type_class) do
new_join_dependency(
Person,
new_join(:articles)
).join_constraints(
[],
Arel::Nodes::OuterJoin
).first.joins.map(&:class)
end
specify { expect(join_type_class).to eq [Arel::Nodes::OuterJoin] }
end
end
end

19
polyamorous/spec/polyamorous/join_spec.rb
View File

@ -1,19 +0,0 @@
require 'spec_helper'
module Polyamorous
describe Join do
it 'is a tree node' do
join = new_join(:articles, :outer)
expect(join).to be_kind_of(TreeNode)
end
it 'can be added to a tree' do
join = new_join(:articles, :outer)
tree_hash = {}
join.add_to_tree(tree_hash)
expect(tree_hash[join]).to be {}
end
end
end

43
polyamorous/spec/spec_helper.rb
View File

@ -1,43 +0,0 @@
require 'machinist/active_record'
require 'sham'
require 'faker'
require 'polyamorous'
Time.zone = 'Eastern Time (US & Canada)'
Dir[File.expand_path('../{helpers,support,blueprints}/**/*.rb', __FILE__)]
.each do |f|
require f
end
Sham.define do
name { Faker::Name.name }
title { Faker::Lorem.sentence }
body { Faker::Lorem.paragraph }
salary { |index| 30000 + (index * 1000) }
tag_name { Faker::Lorem.words(3).join(' ') }
note { Faker::Lorem.words(7).join(' ') }
end
RSpec.configure do |config|
config.before(:suite) do
message = "Running Polyamorous specs with #{
ActiveRecord::Base.connection.adapter_name
}, Active Record #{::ActiveRecord::VERSION::STRING}, Arel #{Arel::VERSION
} and Ruby #{RUBY_VERSION}"
line = '=' * message.length
puts line, message, line
Schema.create
end
config.before(:all) { Sham.reset(:before_all) }
config.before(:each) { Sham.reset(:before_each) }
config.include PolyamorousHelper
end
RSpec::Matchers.define :be_like do |expected|
match do |actual|
actual.gsub(/^\s+|\s+$/, '').gsub(/\s+/, ' ').strip ==
expected.gsub(/^\s+|\s+$/, '').gsub(/\s+/, ' ').strip
end
end

98
polyamorous/spec/support/schema.rb
View File

@ -1,98 +0,0 @@
require 'active_record'
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
class Person < ActiveRecord::Base
belongs_to :parent, class_name: 'Person', foreign_key: :parent_id
has_many :children, class_name: 'Person', foreign_key: :parent_id
has_many :articles
has_many :comments
has_many :authored_article_comments, through: :articles,
foreign_key: :person_id, source: :comments
has_many :notes, as: :notable
end
class Article < ActiveRecord::Base
belongs_to :person
has_many :comments
has_and_belongs_to_many :tags
has_many :notes, as: :notable
end
class Comment < ActiveRecord::Base
belongs_to :article
belongs_to :person
end
class Tag < ActiveRecord::Base
has_and_belongs_to_many :articles
end
class Note < ActiveRecord::Base
belongs_to :notable, polymorphic: true
end
module Schema
def self.create
ActiveRecord::Migration.verbose = false
ActiveRecord::Schema.define do
create_table :people, force: true do |t|
t.integer :parent_id
t.string :name
t.integer :salary
t.boolean :awesome, default: false
t.timestamps null: false
end
create_table :articles, force: true do |t|
t.integer :person_id
t.string :title
t.text :body
end
create_table :comments, force: true do |t|
t.integer :article_id
t.integer :person_id
t.text :body
end
create_table :tags, force: true do |t|
t.string :name
end
create_table :articles_tags, force: true, id: false do |t|
t.integer :article_id
t.integer :tag_id
end
create_table :notes, force: true do |t|
t.integer :notable_id
t.string :notable_type
t.string :note
end
end
10.times do
person = Person.make
Note.make(notable: person)
3.times do
article = Article.make(person: person)
3.times do
article.tags = [Tag.make, Tag.make, Tag.make]
end
Note.make(notable: article)
10.times do
Comment.make(article: article)
end
end
2.times do
Comment.make(person: person)
end
end
Comment.make(
body: 'First post!', article: Article.make(title: 'Hello, world!')
)
end
end

1
ransack.gemspec
View File

@ -28,6 +28,7 @@ Gem::Specification.new do |s|
s.add_development_dependency 'pg', '~> 0.21'
s.add_development_dependency 'mysql2', '0.3.20'
s.add_development_dependency 'pry', '0.10'
s.add_development_dependency 'byebug'
s.files = `git ls-files`.split("\n")

23
spec/ransack/join_dependency_spec.rb
View File

@ -8,8 +8,8 @@ module Polyamorous
specify { expect(subject.send(:join_root).drop(1).size)
.to eq(2) }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type))
.to be_all { Polyamorous::InnerJoin } }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type).uniq)
.to eq [Polyamorous::InnerJoin] }
end
context 'with has_many :through association' do
@ -38,8 +38,8 @@ module Polyamorous
.to eq 2 }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type))
.to eq [Polyamorous::OuterJoin, Polyamorous::OuterJoin] }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type))
.to be_all { Polyamorous::OuterJoin } }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type).uniq)
.to eq [Polyamorous::OuterJoin] }
end
context 'with polymorphic belongs_to join' do
@ -59,8 +59,19 @@ module Polyamorous
specify { expect(subject.send(:join_root).drop(1).size)
.to eq 2 }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type))
.to be_all { Polyamorous::InnerJoin } }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type).uniq)
.to eq [Polyamorous::InnerJoin] }
specify { expect(subject.send(:join_root).drop(1).first.table_name)
.to eq 'people' }
specify { expect(subject.send(:join_root).drop(1)[1].table_name)
.to eq 'comments' }
end
context 'with polymorphic belongs_to join and nested join' do
subject { new_join_dependency Note,
new_join(:notable, :outer, Person) => :comments }
specify { expect(subject.send(:join_root).drop(1).size).to eq 2 }
specify { expect(subject.send(:join_root).drop(1).map(&:join_type)).to eq [Polyamorous::OuterJoin, Polyamorous::InnerJoin] }
specify { expect(subject.send(:join_root).drop(1).first.table_name)
.to eq 'people' }
specify { expect(subject.send(:join_root).drop(1)[1].table_name)

28
spec/ransack/search_spec.rb
View File

@ -227,13 +227,37 @@ module Ransack
children_people_name_field} = 'Ernie'/
end
it 'use appropriate table alias' do
skip "Make this spec pass for Rails <5.2" if ::ActiveRecord::VERSION::STRING < '5.2.0'
s = Search.new(Person, {
name_eq: "person_name_query",
articles_title_eq: "person_article_title_query",
parent_name_eq: "parent_name_query",
parent_articles_title_eq: 'parents_article_title_query'
}).result
real_query = remove_quotes_and_backticks(s.to_sql)
expect(real_query)
.to include "LEFT OUTER JOIN articles ON articles.person_id = people.id"
expect(real_query)
.to include "LEFT OUTER JOIN articles articles_people ON articles_people.person_id = parents_people.id"
expect(real_query)
.to include "people.name = 'person_name_query'"
expect(real_query)
.to include "articles.title = 'person_article_title_query'"
expect(real_query)
.to include "parents_people.name = 'parent_name_query'"
expect(real_query)
.to include "articles_people.title = 'parents_article_title_query'"
end
# FIXME: Make this spec pass for Rails 4.1 / 4.2 / 5.0 and not just 4.0 by
# commenting out lines 221 and 242 to run the test. Addresses issue #374.
# https://github.com/activerecord-hackery/ransack/issues/374
#
it 'evaluates conditions for multiple `belongs_to` associations to the
same table contextually' do
skip "Make this spec pass for Rails >5.0"
skip "Make this spec pass for Rails <5.2" if ::ActiveRecord::VERSION::STRING < '5.2.0'
s = Search.new(
Recommendation,
person_name_eq: 'Ernie',
@ -248,7 +272,7 @@ module Ransack
ON target_people_recommendations.id = recommendations.target_person_id
LEFT OUTER JOIN people parents_people
ON parents_people.id = target_people_recommendations.parent_id
WHERE ((people.name = 'Ernie' AND parents_people.name = 'Test'))
WHERE (people.name = 'Ernie' AND parents_people.name = 'Test')
SQL
.squish
expect(real_query).to eq expected_query

1
spec/spec_helper.rb
View File

@ -4,6 +4,7 @@ require 'faker'
require 'ransack'
require 'pry'
require 'simplecov'
require 'byebug'
SimpleCov.start
I18n.enforce_available_locales = false