From 7cd4063162e47e28715cafc021241f14cf8a278d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 5 Mar 2012 00:45:17 -0300 Subject: Add test case to has_many through association when mass_assignment_sanitizer is :strict Conflicts: activerecord/test/models/person.rb --- .../has_many_through_associations_test.rb | 25 ++++++++++++++++++---- activerecord/test/models/person.rb | 2 ++ activerecord/test/models/post.rb | 2 ++ activerecord/test/models/secure_reader.rb | 9 ++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 activerecord/test/models/secure_reader.rb diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 6836a2b4d0..beefbffe8a 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -4,6 +4,7 @@ require 'models/person' require 'models/reference' require 'models/job' require 'models/reader' +require 'models/secure_reader' require 'models/comment' require 'models/tag' require 'models/tagging' @@ -44,17 +45,33 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_associate_existing - posts(:thinking); people(:david) # Warm cache + post = posts(:thinking) + person = people(:david) assert_queries(1) do - posts(:thinking).people << people(:david) + post.people << person end assert_queries(1) do - assert posts(:thinking).people.include?(people(:david)) + assert post.people.include?(person) end - assert posts(:thinking).reload.people(true).include?(people(:david)) + assert post.reload.people(true).include?(person) + end + + def test_associate_existing_with_strict_mass_assignment_sanitizer + ActiveRecord::Base.mass_assignment_sanitizer = :strict + + SecureReader.new + + post = posts(:thinking) + person = people(:david) + + assert_queries(1) do + post.secure_people << person + end + ensure + ActiveRecord::Base.mass_assignment_sanitizer = :logger end def test_associate_existing_record_twice_should_add_to_target_twice diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 36eb08d02f..dcfea3170c 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -1,8 +1,10 @@ class Person < ActiveRecord::Base has_many :readers + has_many :secure_readers has_one :reader has_many :posts, :through => :readers + has_many :secure_posts, :through => :secure_readers has_many :posts_with_no_comments, :through => :readers, :source => :post, :include => :comments, :conditions => 'comments.id is null' has_many :references diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 1cab78d8c7..0fc22ac6a3 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -115,8 +115,10 @@ class Post < ActiveRecord::Base has_many :named_categories, :through => :standard_categorizations has_many :readers + has_many :secure_readers has_many :readers_with_person, :include => :person, :class_name => "Reader" has_many :people, :through => :readers + has_many :secure_people, :through => :secure_readers has_many :single_people, :through => :readers has_many :people_with_callbacks, :source=>:person, :through => :readers, :before_add => lambda {|owner, reader| log(:added, :before, reader.first_name) }, diff --git a/activerecord/test/models/secure_reader.rb b/activerecord/test/models/secure_reader.rb new file mode 100644 index 0000000000..3a2a8496fd --- /dev/null +++ b/activerecord/test/models/secure_reader.rb @@ -0,0 +1,9 @@ +class SecureReader < ActiveRecord::Base + self.table_name = "readers" + + belongs_to :secure_post, :class_name => "Post", :foreign_key => "post_id" + belongs_to :secure_person, :inverse_of => :secure_readers, :class_name => "Person", :foreign_key => "person_id" + + + attr_accessible nil +end -- cgit v1.2.3 From f18c0547b0a7ff97fec78cb1f0c95c2531290a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 5 Mar 2012 17:00:48 -0300 Subject: Not need to pass join attributes to association build --- .../active_record/associations/has_many_through_association.rb | 4 +++- .../lib/active_record/associations/through_association.rb | 10 +++++++--- .../cases/associations/has_many_through_associations_test.rb | 5 ++--- activerecord/test/models/reader.rb | 9 +++++++++ activerecord/test/models/secure_reader.rb | 9 --------- 5 files changed, 21 insertions(+), 16 deletions(-) delete mode 100644 activerecord/test/models/secure_reader.rb diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 9657cb081d..53d49fef2e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -73,7 +73,9 @@ module ActiveRecord # association def build_through_record(record) @through_records[record.object_id] ||= begin - through_record = through_association.build(construct_join_attributes(record)) + ensure_mutable + + through_record = through_association.build through_record.send("#{source_reflection.name}=", record) through_record end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index f95e5337c2..fd0e90aaf0 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -37,9 +37,7 @@ module ActiveRecord # situation it is more natural for the user to just create or modify their join records # directly as required. def construct_join_attributes(*records) - if source_reflection.macro != :belongs_to - raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) - end + ensure_mutable join_attributes = { source_reflection.foreign_key => @@ -73,6 +71,12 @@ module ActiveRecord !owner[through_reflection.foreign_key].nil? end + def ensure_mutable + if source_reflection.macro != :belongs_to + raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) + end + end + def ensure_not_nested if reflection.nested? raise HasManyThroughNestedAssociationsAreReadonly.new(owner, reflection) diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index beefbffe8a..4489c3e638 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -4,7 +4,6 @@ require 'models/person' require 'models/reference' require 'models/job' require 'models/reader' -require 'models/secure_reader' require 'models/comment' require 'models/tag' require 'models/tagging' @@ -60,7 +59,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_associate_existing_with_strict_mass_assignment_sanitizer - ActiveRecord::Base.mass_assignment_sanitizer = :strict + SecureReader.mass_assignment_sanitizer = :strict SecureReader.new @@ -71,7 +70,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase post.secure_people << person end ensure - ActiveRecord::Base.mass_assignment_sanitizer = :logger + SecureReader.mass_assignment_sanitizer = :logger end def test_associate_existing_record_twice_should_add_to_target_twice diff --git a/activerecord/test/models/reader.rb b/activerecord/test/models/reader.rb index 0207a2bd92..59005ac604 100644 --- a/activerecord/test/models/reader.rb +++ b/activerecord/test/models/reader.rb @@ -3,3 +3,12 @@ class Reader < ActiveRecord::Base belongs_to :person, :inverse_of => :readers belongs_to :single_person, :class_name => 'Person', :foreign_key => :person_id, :inverse_of => :reader end + +class SecureReader < ActiveRecord::Base + self.table_name = "readers" + + belongs_to :secure_post, :class_name => "Post", :foreign_key => "post_id" + belongs_to :secure_person, :inverse_of => :secure_readers, :class_name => "Person", :foreign_key => "person_id" + + attr_accessible nil +end diff --git a/activerecord/test/models/secure_reader.rb b/activerecord/test/models/secure_reader.rb deleted file mode 100644 index 3a2a8496fd..0000000000 --- a/activerecord/test/models/secure_reader.rb +++ /dev/null @@ -1,9 +0,0 @@ -class SecureReader < ActiveRecord::Base - self.table_name = "readers" - - belongs_to :secure_post, :class_name => "Post", :foreign_key => "post_id" - belongs_to :secure_person, :inverse_of => :secure_readers, :class_name => "Person", :foreign_key => "person_id" - - - attr_accessible nil -end -- cgit v1.2.3