From fa8c525d20ff14e4a9d367d2845b446267065bcc Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 22 Apr 2017 16:28:20 +0900 Subject: [PATCH] Fix `automatic_inverse_of` not to be disabled if extension block is given If an association has a scope, `automatic_inverse_of` is to be disabled. But extension block is obviously not a scope. It should not be regarded as a scope. Fixes #28806. --- .../associations/builder/association.rb | 28 +++++++------------ .../builder/collection_association.rb | 12 -------- .../eager_load_nested_include_test.rb | 4 +-- .../has_many_associations_test.rb | 16 +++++------ .../associations/inverse_associations_test.rb | 9 ++++++ activerecord/test/models/post.rb | 4 +++ 6 files changed, 33 insertions(+), 40 deletions(-) diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 7c69cd65ee1..3b4b2431486 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -27,40 +27,32 @@ module ActiveRecord::Associations::Builder # :nodoc: "Please choose a different association name." end - extension = define_extensions model, name, &block - reflection = create_reflection model, name, scope, options, extension + reflection = create_reflection(model, name, scope, options, &block) define_accessors model, reflection define_callbacks model, reflection define_validations model, reflection reflection end - def self.create_reflection(model, name, scope, options, extension = nil) + def self.create_reflection(model, name, scope, options, &block) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) validate_options(options) - scope = build_scope(scope, extension) + extension = define_extensions(model, name, &block) + options[:extend] = [*options[:extend], extension] if extension + + scope = build_scope(scope) ActiveRecord::Reflection.create(macro, name, scope, options, model) end - def self.build_scope(scope, extension) - new_scope = scope - + def self.build_scope(scope) if scope && scope.arity == 0 - new_scope = proc { instance_exec(&scope) } + proc { instance_exec(&scope) } + else + scope end - - if extension - new_scope = wrap_scope new_scope, extension - end - - new_scope - end - - def self.wrap_scope(scope, extension) - scope end def self.macro diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 5848cd9112a..9fccfcce0c1 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -66,17 +66,5 @@ module ActiveRecord::Associations::Builder # :nodoc: end CODE end - - def self.wrap_scope(scope, mod) - if scope - if scope.arity > 0 - proc { |owner| instance_exec(owner, &scope).extending(mod) } - else - proc { instance_exec(&scope).extending(mod) } - end - else - proc { extending(mod) } - end - end end end diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb index 525ad3197a5..849939de753 100644 --- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb +++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb @@ -110,10 +110,10 @@ class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase end teardown do - @davey_mcdave.destroy - @first_post.destroy @first_comment.destroy @first_categorization.destroy + @davey_mcdave.destroy + @first_post.destroy end def test_missing_data_in_a_nested_include_should_not_cause_errors_when_constructing_objects diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 6a7efe21211..32285f269a7 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2578,22 +2578,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase test "association with extend option" do post = posts(:welcome) - assert_equal "lifo", post.comments_with_extend.author - assert_equal "hello", post.comments_with_extend.greeting + assert_equal "lifo", post.comments_with_extend.author + assert_equal "hello :)", post.comments_with_extend.greeting end test "association with extend option with multiple extensions" do post = posts(:welcome) - assert_equal "lifo", post.comments_with_extend_2.author - assert_equal "hullo", post.comments_with_extend_2.greeting + assert_equal "lifo", post.comments_with_extend_2.author + assert_equal "hullo :)", post.comments_with_extend_2.greeting end test "extend option affects per association" do post = posts(:welcome) - assert_equal "lifo", post.comments_with_extend.author - assert_equal "lifo", post.comments_with_extend_2.author - assert_equal "hello", post.comments_with_extend.greeting - assert_equal "hullo", post.comments_with_extend_2.greeting + assert_equal "lifo", post.comments_with_extend.author + assert_equal "lifo", post.comments_with_extend_2.author + assert_equal "hello :)", post.comments_with_extend.greeting + assert_equal "hullo :)", post.comments_with_extend_2.greeting end test "delete record with complex joins" do diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index da3a42e2b5e..669e176dcbb 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -8,6 +8,7 @@ require "models/zine" require "models/club" require "models/sponsor" require "models/rating" +require "models/post" require "models/comment" require "models/car" require "models/bulb" @@ -62,6 +63,14 @@ class AutomaticInverseFindingTests < ActiveRecord::TestCase assert_equal rating_reflection, comment_reflection.inverse_of, "The Comment reflection's inverse should be the Rating reflection" end + def test_has_many_and_belongs_to_should_find_inverse_automatically_for_extension_block + comment_reflection = Comment.reflect_on_association(:post) + post_reflection = Post.reflect_on_association(:comments) + + assert_predicate post_reflection, :has_inverse? + assert_equal comment_reflection, post_reflection.inverse_of + end + def test_has_many_and_belongs_to_should_find_inverse_automatically_for_sti author_reflection = Author.reflect_on_association(:posts) author_child_reflection = Author.reflect_on_association(:special_posts) diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 395b534c630..c34968590fa 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -11,6 +11,10 @@ class Post < ActiveRecord::Base def author "lifo" end + + def greeting + super + " :)" + end end module NamedExtension2