diff options
author | Jared Armstrong and Neeraj Singh <neerajdotname@gmail.com> | 2013-04-10 08:34:47 -0400 |
---|---|---|
committer | Neeraj Singh <neerajdotname@gmail.com> | 2013-04-10 12:19:47 -0400 |
commit | dc764fcc348d562376add26ff8ef5173946b575b (patch) | |
tree | 3014a67245025f0a5b46a19eeca2fca10a3dee65 /activerecord | |
parent | bb61d2defab7017807c310ac221c9e69d7f99d96 (diff) | |
download | rails-dc764fcc348d562376add26ff8ef5173946b575b.tar.gz rails-dc764fcc348d562376add26ff8ef5173946b575b.tar.bz2 rails-dc764fcc348d562376add26ff8ef5173946b575b.zip |
While merging relations preserve context for joins
Fixes #3002. Also see #5494.
```
class Comment < ActiveRecord::Base
belongs_to :post
end
class Author < ActiveRecord::Base
has_many :posts
end
class Post < ActiveRecord::Base
belongs_to :author
has_many :comments
end
```
`Comment.joins(:post).merge(Post.joins(:author).merge(Author.where(:name => "Joe Blogs"))).all` would
fail with `ActiveRecord::ConfigurationError: Association named 'author' was not found on Comment`.
It is failing because `all` is being called on relation which looks like this after all the merging:
`{:joins=>[:post, :author], :where=>[#<Arel::Nodes::Equality: ....}`. In this relation all the context that
`Post` was joined with `Author` is lost and hence the error that `author` was not found on `Comment`.
Ths solution is to build JoinAssociation when two relations with join information are being merged. And later
while building the arel use the previously built `JoinAssociation` record in `JoinDependency#graft` to
build the right from clause.
Thanks to Jared Armstrong (https://github.com/armstrjare) for most of the work. I ported it to make it
compatible with new code base.
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/CHANGELOG.md | 32 | ||||
-rw-r--r-- | activerecord/lib/active_record/associations/join_dependency/join_association.rb | 7 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation/merger.rb | 32 | ||||
-rw-r--r-- | activerecord/test/cases/relation_test.rb | 11 |
4 files changed, 78 insertions, 4 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 58f6153e96..57a054074b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,37 @@ ## Rails 4.0.0 (unreleased) ## +* Preserve context while merging relations with join information. + + class Comment < ActiveRecord::Base + belongs_to :post + end + + class Author < ActiveRecord::Base + has_many :posts + end + + class Post < ActiveRecord::Base + belongs_to :author + has_many :comments + end + + `Comment.joins(:post).merge(Post.joins(:author).merge(Author.where(:name => "Joe Blogs"))).all` + would fail with + `ActiveRecord::ConfigurationError: Association named 'author' was not found on Comment`. + + It is failing because `all` is being called on relation which looks like this after all + the merging: `{:joins=>[:post, :author], :where=>[#<Arel::Nodes::Equality: ....}`. In this + relation all the context that `Post` was joined with `Author` is lost and hence the error + that `author` was not found on `Comment`. + + Ths solution is to build JoinAssociation when two relations with join information are being + merged. And later while building the arel use the previously built `JoinAssociation` record + in `JoinDependency#graft` to build the right from clause. + + Fixes #3002. + + *Jared Armstrong and Neeraj Singh* + * `default_scopes?` is deprecated. Check for `default_scopes.empty?` instead. *Agis Anastasopoulos* diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 7fa0844a5e..ae1b38ff86 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -55,7 +55,12 @@ module ActiveRecord def find_parent_in(other_join_dependency) other_join_dependency.join_parts.detect do |join_part| - parent == join_part + case parent + when JoinBase + parent.base_klass == join_part.base_klass + else + parent == join_part + end end end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index eb23e92fb8..c7f3b88534 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -39,7 +39,7 @@ module ActiveRecord end class Merger # :nodoc: - attr_reader :relation, :values + attr_reader :relation, :values, :other def initialize(relation, other) if other.default_scoped? && other.klass != relation.klass @@ -48,11 +48,12 @@ module ActiveRecord @relation = relation @values = other.values + @other = other end NORMAL_VALUES = Relation::SINGLE_VALUE_METHODS + Relation::MULTI_VALUE_METHODS - - [:where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc: + [:joins, :where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc: def normal_values NORMAL_VALUES @@ -66,12 +67,39 @@ module ActiveRecord merge_multi_values merge_single_values + merge_joins relation end private + def merge_joins + return if values[:joins].blank? + + if other.klass == relation.klass + relation.joins! *values[:joins] + else + joins_dependency, rest = values[:joins].partition do |join| + case join + when Hash, Symbol, Array + true + else + false + end + end + + join_dependency = ActiveRecord::Associations::JoinDependency.new(other.klass, + joins_dependency, + []) + relation.joins! rest + + join_dependency.join_associations.each do |association| + @relation = association.join_relation(relation) + end + end + end + def merge_multi_values relation.where_values = merged_wheres relation.bind_values = merged_binds diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 9ca980fdf6..1967d8716a 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -1,10 +1,12 @@ require "cases/helper" require 'models/post' require 'models/comment' +require 'models/author' +require 'models/rating' module ActiveRecord class RelationTest < ActiveRecord::TestCase - fixtures :posts, :comments + fixtures :posts, :comments, :authors class FakeKlass < Struct.new(:table_name, :name) end @@ -176,6 +178,13 @@ module ActiveRecord relation.merge!(where: ['foo = ?', 'bar']) assert_equal ['foo = bar'], relation.where_values end + + def test_relation_merging_with_merged_joins + special_comments_with_ratings = SpecialComment.joins(:ratings) + posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings) + assert_equal 3, authors(:david).posts.merge(posts_with_special_comments_with_ratings).to_a.length + end + end class RelationMutationTest < ActiveSupport::TestCase |