kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

Refactor HasManyThroughAssociation to inherit from HasManyAssociation. Association callbacks and <association>_ids= now work with hm:t. Closes #11516 [rubyruy]

Browse source 
git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9230 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
This commit is contained in:
Pratik Naik 2008-04-06 00:27:12 +00:00
parent 15d88885ee
commit f6b12c11cd
10 changed files with 301 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
activerecord/CHANGELOG
View file

@ -1,5 +1,7 @@
*SVN*
* Refactor HasManyThroughAssociation to inherit from HasManyAssociation. Association callbacks and <association>_ids= now work with hm:t. #11516 [rubyruy]
* Ensure HABTM#create and HABTM#build do not load entire association. [Pratik]
* Improve documentation. [Xavier Noria, Jack Danger Canty, leethal]

27
activerecord/lib/active_record/associations.rb
View file

@ -44,6 +44,11 @@ module ActiveRecord
end
end
class HasManyThroughCantAssociateThroughHasManyReflection < ActiveRecordError #:nodoc:
def initialize(owner, reflection)
super("Cannot modify association '#{owner.class.name}##{reflection.name}' because the source reflection class '#{reflection.source_reflection.class_name}' is associated to '#{reflection.through_reflection.class_name}' via :#{reflection.source_reflection.macro}.")
end
end
class HasManyThroughCantAssociateNewRecords < ActiveRecordError #:nodoc:
def initialize(owner, reflection)
super("Cannot associate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to create the has_many :through record associating them.")
@ -125,27 +130,27 @@ module ActiveRecord
# generated methods | habtm | has_many | :through
# ----------------------------------+-------+----------+----------
# #others | X | X | X
# #others=(other,other,...) | X | X |
# #others=(other,other,...) | X | X | X
# #other_ids | X | X | X
# #other_ids=(id,id,...) | X | X |
# #other_ids=(id,id,...) | X | X | X
# #others<< | X | X | X
# #others.push | X | X | X
# #others.concat | X | X | X
# #others.build(attributes={}) | X | X |
# #others.create(attributes={}) | X | X |
# #others.build(attributes={}) | X | X | X
# #others.create(attributes={}) | X | X | X
# #others.create!(attributes={}) | X | X | X
# #others.size | X | X | X
# #others.length | X | X | X
# #others.count | X | X | X
# #others.sum(args*,&block) | X | X | X
# #others.empty? | X | X | X
# #others.clear | X | X |
# #others.clear | X | X | X
# #others.delete(other,other,...) | X | X | X
# #others.delete_all | X | X |
# #others.destroy_all | X | X | X
# #others.find(*args) | X | X | X
# #others.find_first | X | |
# #others.uniq | X | X |
# #others.uniq | X | X | X
# #others.reset | X | X | X
#
# == Cardinality and associations
@ -650,7 +655,8 @@ module ActiveRecord
# * <tt>:dependent</tt> - if set to <tt>:destroy</tt> all the associated objects are destroyed
# alongside this object by calling their destroy method. If set to <tt>:delete_all</tt> all associated
# objects are deleted *without* calling their destroy method. If set to <tt>:nullify</tt> all associated
# objects' foreign keys are set to +NULL+ *without* calling their save callbacks.
# objects' foreign keys are set to +NULL+ *without* calling their save callbacks. *Warning:* This option is ignored when also using
# the <tt>through</tt> option.
# * <tt>:finder_sql</tt> - specify a complete SQL statement to fetch the association. This is a good way to go for complex
# associations that depend on multiple tables. Note: When this option is used, +find_in_collection+ is _not_ added.
# * <tt>:counter_sql</tt> - specify a complete SQL statement to fetch the size of the association. If <tt>:finder_sql</tt> is
@ -693,11 +699,12 @@ module ActiveRecord
configure_dependency_for_has_many(reflection)
add_multiple_associated_save_callbacks(reflection.name)
add_association_callbacks(reflection.name, reflection.options)
if options[:through]
collection_accessor_methods(reflection, HasManyThroughAssociation, false)
collection_accessor_methods(reflection, HasManyThroughAssociation)
else
add_multiple_associated_save_callbacks(reflection.name)
add_association_callbacks(reflection.name, reflection.options)
collection_accessor_methods(reflection, HasManyAssociation)
end
end

16
activerecord/lib/active_record/associations/association_collection.rb
View file

@ -13,6 +13,14 @@ module ActiveRecord
@loaded = false
end
def build(attributes = {})
if attributes.is_a?(Array)
attributes.collect { |attr| build(attr) }
else
build_record(attributes) { |record| set_belongs_to_association_for(record) }
end
end
# Add +records+ to this association. Returns +self+ so method calls may be chained.
# Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
def <<(*records)
@ -55,7 +63,13 @@ module ActiveRecord
def delete(*records)
records = flatten_deeper(records)
records.each { |record| raise_on_type_mismatch(record) }
records.reject! { |record| @target.delete(record) if record.new_record? }
records.reject! do |record|
if record.new_record?
callback(:before_remove, record)
@target.delete(record)
callback(:after_remove, record)
end
end
return if records.empty?
@owner.transaction do

6
activerecord/lib/active_record/associations/association_proxy.rb
View file

@ -7,10 +7,10 @@ module ActiveRecord
# HasOneAssociation
# BelongsToPolymorphicAssociation
# AssociationCollection
# HasManyAssociation
# HasAndBelongsToManyAssociation
# HasManyThroughAssociation
# HasOneThroughAssociation
# HasManyAssociation
# HasManyThroughAssociation
# HasOneThroughAssociation
#
# Association proxies in Active Record are middlemen between the object that
# holds the association, known as the <tt>@owner</tt>, and the actual associated

4
activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
View file

@ -6,10 +6,6 @@ module ActiveRecord
construct_sql
end
def build(attributes = {})
build_record(attributes)
end
def create(attributes = {})
create_record(attributes) { |record| insert_record(record) }
end

8
activerecord/lib/active_record/associations/has_many_association.rb
View file

@ -6,14 +6,6 @@ module ActiveRecord
construct_sql
end
def build(attributes = {})
if attributes.is_a?(Array)
attributes.collect { |attr| build(attr) }
else
build_record(attributes) { |record| set_belongs_to_association_for(record) }
end
end
# Count the number of associated records. All arguments are optional.
def count(*args)
if @reflection.options[:counter_sql]

93
activerecord/lib/active_record/associations/has_many_through_association.rb
View file

@ -1,13 +1,13 @@
module ActiveRecord
module Associations
class HasManyThroughAssociation < AssociationCollection #:nodoc:
class HasManyThroughAssociation < HasManyAssociation #:nodoc:
def initialize(owner, reflection)
super
reflection.check_validity!
@finder_sql = construct_conditions
construct_sql
end
def find(*args)
options = args.extract_options!
@ -35,65 +35,6 @@ module ActiveRecord
@reflection.klass.find(*args)
end
def reset
@target = []
@loaded = false
end
# Adds records to the association. The source record and its associates
# must have ids in order to create records associating them, so this
# will raise ActiveRecord::HasManyThroughCantAssociateNewRecords if
# either is a new record. Calls create! so you can rescue errors.
#
# The :before_add and :after_add callbacks are not yet supported.
def <<(*records)
return if records.empty?
through = @reflection.through_reflection
raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) if @owner.new_record?
klass = through.klass
klass.transaction do
flatten_deeper(records).each do |associate|
raise_on_type_mismatch(associate)
raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
@owner.send(@reflection.through_reflection.name).proxy_target << klass.send(:with_scope, :create => construct_join_attributes(associate)) { klass.create! }
@target << associate if loaded?
end
end
self
end
[:push, :concat].each { |method| alias_method method, :<< }
# Removes +records+ from this association. Does not destroy +records+.
def delete(*records)
records = flatten_deeper(records)
records.each { |associate| raise_on_type_mismatch(associate) }
through = @reflection.through_reflection
raise ActiveRecord::HasManyThroughCantDissociateNewRecords.new(@owner, through) if @owner.new_record?
load_target
klass = through.klass
klass.transaction do
flatten_deeper(records).each do |associate|
raise_on_type_mismatch(associate)
raise ActiveRecord::HasManyThroughCantDissociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
klass.delete_all(construct_join_attributes(associate))
@target.delete(associate)
end
end
self
end
def build(attrs = nil)
raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, @reflection.through_reflection)
end
alias_method :new, :build
def create!(attrs = nil)
@ -103,6 +44,13 @@ module ActiveRecord
end
end
def create(attrs = nil)
@reflection.klass.transaction do
self << (object = @reflection.klass.send(:with_scope, :create => attrs) { @reflection.klass.create })
object
end
end
# Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been loaded and
# calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero
# and you need to fetch that collection afterwards, it'll take one less SELECT query if you use length.
@ -131,7 +79,28 @@ module ActiveRecord
@reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) }
end
protected
def insert_record(record, force=true)
if record.new_record?
if force
record.save!
else
return false unless record.save
end
end
klass = @reflection.through_reflection.klass
@owner.send(@reflection.through_reflection.name).proxy_target << klass.send(:with_scope, :create => construct_join_attributes(record)) { klass.create! }
end
# TODO - add dependent option support
def delete_records(records)
klass = @reflection.through_reflection.klass
records.each do |associate|
klass.delete_all(construct_join_attributes(associate))
end
end
def method_missing(method, *args)
if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
if block_given?
@ -178,6 +147,8 @@ module ActiveRecord
# Construct attributes for :through pointing to owner and associate.
def construct_join_attributes(associate)
# TODO: revist this to allow it for deletion, supposing dependent option is supported
raise ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection.new(@owner, @reflection) if @reflection.source_reflection.macro == :has_many
join_attributes = construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id)
if @reflection.options[:source_type]
join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name.to_s)

32
activerecord/test/cases/associations/join_model_test.rb
View file

@ -443,11 +443,33 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
assert_nil posts(:thinking).tags.find_by_name("General").attributes["tag_id"]
end
def test_raise_error_when_adding_new_record_to_has_many_through
assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags << tags(:general).clone }
assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).clone.tags << tags(:general) }
assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags.build }
assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags.new }
def test_associating_unsaved_records_with_has_many_through
saved_post = posts(:thinking)
new_tag = Tag.new(:name => "new")
saved_post.tags << new_tag
assert !new_tag.new_record? #consistent with habtm!
assert !saved_post.new_record?
assert saved_post.tags.include?(new_tag)
assert !new_tag.new_record?
assert saved_post.reload.tags(true).include?(new_tag)
new_post = Post.new(:title => "Association replacmenet works!", :body => "You best believe it.")
saved_tag = tags(:general)
new_post.tags << saved_tag
assert new_post.new_record?
assert !saved_tag.new_record?
assert new_post.tags.include?(saved_tag)
new_post.save!
assert !new_post.new_record?
assert new_post.reload.tags(true).include?(saved_tag)
assert posts(:thinking).tags.build.new_record?
assert posts(:thinking).tags.new.new_record?
end
def test_create_associate_when_adding_to_has_many_through

194
activerecord/test/cases/associations_test.rb
View file

@ -550,8 +550,181 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
end
class HasManyThroughAssociationsTest < ActiveRecord::TestCase
fixtures :posts, :readers, :people
def test_associate_existing
assert_queries(2) { posts(:thinking);people(:david) }
assert_queries(1) do
posts(:thinking).people << people(:david)
end
assert_queries(1) do
assert posts(:thinking).people.include?(people(:david))
end
assert posts(:thinking).reload.people(true).include?(people(:david))
end
def test_associating_new
assert_queries(1) { posts(:thinking) }
new_person = nil # so block binding catches it
assert_queries(0) do
new_person = Person.new
end
# Associating new records always saves them
# Thus, 1 query for the new person record, 1 query for the new join table record
assert_queries(2) do
posts(:thinking).people << new_person
end
assert_queries(1) do
assert posts(:thinking).people.include?(new_person)
end
assert posts(:thinking).reload.people(true).include?(new_person)
end
def test_associate_new_by_building
assert_queries(1) { posts(:thinking) }
assert_queries(0) do
posts(:thinking).people.build(:first_name=>"Bob")
posts(:thinking).people.new(:first_name=>"Ted")
end
# Should only need to load the association once
assert_queries(1) do
assert posts(:thinking).people.collect(&:first_name).include?("Bob")
assert posts(:thinking).people.collect(&:first_name).include?("Ted")
end
# 2 queries for each new record (1 to save the record itself, 1 for the join model)
# * 2 new records = 4
# + 1 query to save the actual post = 5
assert_queries(5) do
posts(:thinking).save
end
assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Bob")
assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted")
end
def test_delete_association
assert_queries(2){posts(:welcome);people(:michael); }
assert_queries(1) do
posts(:welcome).people.delete(people(:michael))
end
assert_queries(1) do
assert posts(:welcome).people.empty?
end
assert posts(:welcome).reload.people(true).empty?
end
def test_replace_association
assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)}
# 1 query to delete the existing reader (michael)
# 1 query to associate the new reader (david)
assert_queries(2) do
posts(:welcome).people = [people(:david)]
end
assert_queries(0){
assert posts(:welcome).people.include?(people(:david))
assert !posts(:welcome).people.include?(people(:michael))
}
assert posts(:welcome).reload.people(true).include?(people(:david))
assert !posts(:welcome).reload.people(true).include?(people(:michael))
end
def test_associate_with_create
assert_queries(1) { posts(:thinking) }
# 1 query for the new record, 1 for the join table record
# No need to update the actual collection yet!
assert_queries(2) do
posts(:thinking).people.create(:first_name=>"Jeb")
end
# *Now* we actually need the collection so it's loaded
assert_queries(1) do
assert posts(:thinking).people.collect(&:first_name).include?("Jeb")
end
assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Jeb")
end
def test_clear_associations
assert_queries(2) { posts(:welcome);posts(:welcome).people(true) }
assert_queries(1) do
posts(:welcome).people.clear
end
assert_queries(0) do
assert posts(:welcome).people.empty?
end
assert posts(:welcome).reload.people(true).empty?
end
def test_association_callback_ordering
Post.reset_log
log = Post.log
post = posts(:thinking)
post.people_with_callbacks << people(:michael)
assert_equal [
[:added, :before, "Michael"],
[:added, :after, "Michael"]
], log.last(2)
post.people_with_callbacks.push(people(:david), Person.create!(:first_name => "Bob"), Person.new(:first_name => "Lary"))
assert_equal [
[:added, :before, "David"],
[:added, :after, "David"],
[:added, :before, "Bob"],
[:added, :after, "Bob"],
[:added, :before, "Lary"],
[:added, :after, "Lary"]
],log.last(6)
post.people_with_callbacks.build(:first_name => "Ted")
assert_equal [
[:added, :before, "Ted"],
[:added, :after, "Ted"]
], log.last(2)
post.people_with_callbacks.create(:first_name => "Sam")
assert_equal [
[:added, :before, "Sam"],
[:added, :after, "Sam"]
], log.last(2)
post.people_with_callbacks = [people(:michael),people(:david), Person.new(:first_name => "Julian"), Person.create!(:first_name => "Roger")]
assert_equal (%w(Ted Bob Sam Lary) * 2).sort, log[-12..-5].collect(&:last).sort
assert_equal [
[:added, :before, "Julian"],
[:added, :after, "Julian"],
[:added, :before, "Roger"],
[:added, :after, "Roger"]
], log.last(4)
post.people_with_callbacks.clear
assert_equal (%w(Michael David Julian Roger) * 2).sort, log.last(8).collect(&:last).sort
end
end
class HasManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :companies, :developers, :projects,
fixtures :accounts, :categories, :companies, :developers, :projects,
:developers_projects, :topics, :authors, :comments, :author_addresses,
:people, :posts
@ -1291,8 +1464,23 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_equal [comments(:eager_other_comment1).id], authors(:mary).comment_ids
end
def test_assign_ids_for_through
assert_raise(NoMethodError) { authors(:mary).comment_ids = [123] }
def test_modifying_a_through_a_has_many_should_raise
[
lambda { authors(:mary).comment_ids = [comments(:greetings).id, comments(:more_greetings).id] },
lambda { authors(:mary).comments = [comments(:greetings), comments(:more_greetings)] },
lambda { authors(:mary).comments << Comment.create!(:body => "Yay", :post_id => 424242) },
lambda { authors(:mary).comments.delete(authors(:mary).comments.first) },
].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection, &block) }
end
def test_assign_ids_for_through_a_belongs_to
post = Post.new(:title => "Assigning IDs works!", :body => "You heared it here first, folks!")
post.person_ids = [people(:david).id, people(:michael).id]
post.save
post.reload
assert_equal 2, post.people.length
assert post.people.include?(people(:david))
end
def test_dynamic_find_should_respect_association_order_for_through

14
activerecord/test/models/post.rb
View file

@ -46,6 +46,20 @@ class Post < ActiveRecord::Base
has_many :readers
has_many :people, :through => :readers
has_many :people_with_callbacks, :source=>:person, :through => :readers,
:before_add => lambda {|owner, reader| log(:added, :before, reader.first_name) },
:after_add => lambda {|owner, reader| log(:added, :after, reader.first_name) },
:before_remove => lambda {|owner, reader| log(:removed, :before, reader.first_name) },
:after_remove => lambda {|owner, reader| log(:removed, :after, reader.first_name) }
def self.reset_log
@log = []
end
def self.log(message=nil, side=nil, new_record=nil)
return @log if message.nil?
@log << [message, side, new_record]
end
def self.what_are_you
'a post...'