diff options
Diffstat (limited to 'activerecord')
35 files changed, 344 insertions, 135 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index fd571c4ca4..8ebc87145c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,8 @@ *Rails 3.1.0 (unreleased)* +* Removed support for interpolated SQL conditions. Please use scoping +along with attribute conditionals as a replacement. + * Added ActiveRecord::Base#has_secure_password (via ActiveModel::SecurePassword) to encapsulate dead-simple password usage with BCrypt encryption and salting [DHH]. Example: # Schema: User(name:string, password_digest:string, password_salt:string) diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index f2094283a2..6d905fe6fe 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -185,6 +185,9 @@ module ActiveRecord end def preload_has_and_belongs_to_many_association(records, reflection, preload_options={}) + + left = reflection.klass.arel_table + table_name = reflection.klass.quoted_table_name id_to_record_map, ids = construct_id_map(records) records.each {|record| record.send(reflection.name).loaded} @@ -193,14 +196,28 @@ module ActiveRecord conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}" conditions << append_conditions(reflection, preload_options) + right = Arel::Table.new(options[:join_table]).alias('t0') + condition = left[reflection.klass.primary_key].eq( + right[reflection.association_foreign_key]) + + join = left.create_join(right, left.create_on(condition)) + select = [ + # FIXME: options[:select] is always nil in the tests. Do we really + # need it? + options[:select] || left[Arel.star], + right[reflection.primary_key_name].as( + Arel.sql('the_parent_record_id')) + ] + associated_records_proxy = reflection.klass.unscoped. includes(options[:include]). - joins("INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}"). - select("#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id"). order(options[:order]) + associated_records_proxy.joins_values = [join] + associated_records_proxy.select_values = select + all_associated_records = associated_records(ids) do |some_ids| - associated_records_proxy.where([conditions, ids]).to_a + associated_records_proxy.where([conditions, some_ids]).to_a end set_association_collection_records(id_to_record_map, reflection.name, all_associated_records, 'the_parent_record_id') @@ -373,14 +390,9 @@ module ActiveRecord end end - - def interpolate_sql_for_preload(sql) - instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__) - end - def append_conditions(reflection, preload_options) sql = "" - sql << " AND (#{interpolate_sql_for_preload(reflection.sanitized_conditions)})" if reflection.sanitized_conditions + sql << " AND (#{reflection.sanitized_conditions})" if reflection.sanitized_conditions sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions] sql end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index cdc8f25119..1056c51a3d 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -20,18 +20,30 @@ module ActiveRecord end end - class HasManyThroughAssociationPolymorphicError < ActiveRecordError #:nodoc: + class HasManyThroughAssociationPolymorphicSourceError < ActiveRecordError #:nodoc: def initialize(owner_class_name, reflection, source_reflection) super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' on the polymorphic object '#{source_reflection.class_name}##{source_reflection.name}'.") end end + class HasManyThroughAssociationPolymorphicThroughError < ActiveRecordError #:nodoc: + def initialize(owner_class_name, reflection) + super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' which goes through the polymorphic association '#{owner_class_name}##{reflection.through_reflection.name}'.") + end + end + class HasManyThroughAssociationPointlessSourceTypeError < ActiveRecordError #:nodoc: def initialize(owner_class_name, reflection, source_reflection) super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' with a :source_type option if the '#{reflection.through_reflection.class_name}##{source_reflection.name}' is not polymorphic. Try removing :source_type on your association.") end end + class HasOneThroughCantAssociateThroughCollection < ActiveRecordError #:nodoc: + def initialize(owner_class_name, reflection, through_reflection) + super("Cannot have a has_one :through association '#{owner_class_name}##{reflection.name}' where the :through association '#{owner_class_name}##{through_reflection.name}' is a collection. Specify a has_one or belongs_to association in the :through option instead.") + end + end + class HasManyThroughSourceAssociationNotFoundError < ActiveRecordError #:nodoc: def initialize(reflection) through_reflection = reflection.through_reflection @@ -1223,14 +1235,12 @@ module ActiveRecord if reflection.options[:polymorphic] association_accessor_methods(reflection, BelongsToPolymorphicAssociation) - association_foreign_type_setter_method(reflection) else association_accessor_methods(reflection, BelongsToAssociation) association_constructor_method(:build, reflection, BelongsToAssociation) association_constructor_method(:create, reflection, BelongsToAssociation) end - association_foreign_key_setter_method(reflection) add_counter_cache_callbacks(reflection) if options[:counter_cache] add_touch_callbacks(reflection, options[:touch]) if options[:touch] @@ -1450,7 +1460,7 @@ module ActiveRecord force_reload = params.first unless params.empty? association = association_instance_get(reflection.name) - if association.nil? || force_reload + if association.nil? || force_reload || association.stale_target? association = association_proxy_class.new(self, reflection) retval = force_reload ? reflection.klass.uncached { association.reload } : association.reload if retval.nil? and association_proxy_class == BelongsToAssociation @@ -1497,7 +1507,11 @@ module ActiveRecord association_instance_set(reflection.name, association) end - reflection.klass.uncached { association.reload } if force_reload + if force_reload + reflection.klass.uncached { association.reload } + elsif association.stale_target? + association.reload + end association end @@ -1506,16 +1520,9 @@ module ActiveRecord if send(reflection.name).loaded? || reflection.options[:finder_sql] send(reflection.name).map { |r| r.id } else - if reflection.through_reflection && reflection.source_reflection.belongs_to? - through = reflection.through_reflection - primary_key = reflection.source_reflection.primary_key_name - send(through.name).select("DISTINCT #{through.quoted_table_name}.#{primary_key}").map! { |r| r.send(primary_key) } - else - send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id } - end + send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id } end end - end def collection_accessor_methods(reflection, association_proxy_class, writer = true) @@ -1557,45 +1564,6 @@ module ActiveRecord end end - def association_foreign_key_setter_method(reflection) - setters = reflect_on_all_associations(:belongs_to).map do |belongs_to_reflection| - if belongs_to_reflection.primary_key_name == reflection.primary_key_name - "association_instance_set(:#{belongs_to_reflection.name}, nil);" - end - end.compact.join - - if method_defined?(:"#{reflection.primary_key_name}=") - undef_method :"#{reflection.primary_key_name}=" - end - - class_eval <<-FILE, __FILE__, __LINE__ + 1 - def #{reflection.primary_key_name}=(new_id) - write_attribute :#{reflection.primary_key_name}, new_id - if #{reflection.primary_key_name}_changed? - #{ setters } - end - end - FILE - end - - def association_foreign_type_setter_method(reflection) - setters = reflect_on_all_associations(:belongs_to).map do |belongs_to_reflection| - if belongs_to_reflection.options[:foreign_type] == reflection.options[:foreign_type] - "association_instance_set(:#{belongs_to_reflection.name}, nil);" - end - end.compact.join - - field = reflection.options[:foreign_type] - class_eval <<-FILE, __FILE__, __LINE__ + 1 - def #{field}=(new_id) - write_attribute :#{field}, new_id - if #{field}_changed? - #{ setters } - end - end - FILE - end - def add_counter_cache_callbacks(reflection) cache_column = reflection.counter_cache_column diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index c513e8ab08..7964f4fa2b 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -462,10 +462,10 @@ module ActiveRecord callback(:before_add, record) yield(record) if block_given? @target ||= [] unless loaded? - if index = @target.index(record) + if @reflection.options[:uniq] && index = @target.index(record) @target[index] = record else - @target << record + @target << record end callback(:after_add, record) set_inverse_instance(record, @owner) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 252ff7e7ea..f4eceeed8c 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -130,6 +130,16 @@ module ActiveRecord @loaded = true end + # The target is stale if the target no longer points to the record(s) that the + # relevant foreign_key(s) refers to. If stale, the association accessor method + # on the owner will reload the target. It's up to subclasses to implement this + # method if relevant. + # + # Note that if the target has not been loaded, it is not considered stale. + def stale_target? + false + end + # Returns the target of this proxy, same as +proxy_target+. def target @target diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index b438620c8f..bbfe18f9fb 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -42,6 +42,17 @@ module ActiveRecord @updated end + def stale_target? + if @target && @target.persisted? + target_id = @target.send(@reflection.association_primary_key).to_s + foreign_key = @owner.send(@reflection.primary_key_name).to_s + + target_id != foreign_key + else + false + end + end + private def find_target find_method = if @reflection.options[:primary_key] diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index a0df860623..c580de7fbe 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -23,6 +23,19 @@ module ActiveRecord @updated end + def stale_target? + if @target && @target.persisted? + target_id = @target.send(@reflection.association_primary_key).to_s + foreign_key = @owner.send(@reflection.primary_key_name).to_s + target_type = @target.class.base_class.name + foreign_type = @owner.send(@reflection.options[:foreign_type]).to_s + + target_id != foreign_key || target_type != foreign_type + else + false + end + end + private # NOTE - for now, we're only supporting inverse setting from belongs_to back onto 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 f0bc6aedf2..5f4667b4d8 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -59,6 +59,7 @@ module ActiveRecord def find_target return [] unless target_reflection_has_associated_record? + update_stale_state scoped.all end diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index e8cf73976b..eb17935d6a 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -33,6 +33,7 @@ module ActiveRecord private def find_target + update_stale_state scoped.first end end diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb index 5dc5b0c048..e57de84f66 100644 --- a/activerecord/lib/active_record/associations/through_association_scope.rb +++ b/activerecord/lib/active_record/associations/through_association_scope.rb @@ -10,6 +10,17 @@ module ActiveRecord end end + def stale_target? + if @target && @reflection.through_reflection.macro == :belongs_to && defined?(@through_foreign_key) + previous_key = @through_foreign_key.to_s + current_key = @owner.send(@reflection.through_reflection.primary_key_name).to_s + + previous_key != current_key + else + false + end + end + protected def construct_find_scope @@ -63,8 +74,8 @@ module ActiveRecord end def construct_select(custom_select = nil) - distinct = "DISTINCT " if @reflection.options[:uniq] - custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*" + distinct = "DISTINCT #{@reflection.quoted_table_name}.*" if @reflection.options[:uniq] + custom_select || @reflection.options[:select] || distinct end def construct_joins @@ -106,7 +117,12 @@ module ActiveRecord # TODO: revisit this to allow it for deletion, supposing dependent option is supported raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro) - join_attributes = construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id) + join_attributes = construct_owner_attributes(@reflection.through_reflection) + + join_attributes.merge!( + @reflection.source_reflection.primary_key_name => + associate.send(@reflection.source_reflection.association_primary_key) + ) if @reflection.options[:source_type] join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name) @@ -160,6 +176,14 @@ module ActiveRecord end alias_method :sql_conditions, :conditions + + def update_stale_state + construct_scope if stale_target? + + if @reflection.through_reflection.macro == :belongs_to + @through_foreign_key = @owner.send(@reflection.through_reflection.primary_key_name) + end + end end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 858ccebbfa..6aac25ba9b 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -913,7 +913,7 @@ module ActiveRecord #:nodoc: end def type_condition - sti_column = arel_table[inheritance_column] + sti_column = arel_table[inheritance_column.to_sym] condition = sti_column.eq(sti_name) descendants.each { |subclass| condition = condition.or(sti_column.eq(subclass.sti_name)) } diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 050b521b6a..16023defe3 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -405,7 +405,18 @@ module ActiveRecord end elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s } - association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? && !call_reject_if(association_name, attributes) + unless association.loaded? || call_reject_if(association_name, attributes) + # Make sure we are operating on the actual object which is in the association's + # proxy_target array (either by finding it, or adding it if not found) + target_record = association.proxy_target.detect { |record| record == existing_record } + + if target_record + existing_record = target_record + else + association.send(:add_record_to_target_with_callbacks, existing_record) + end + end + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) else diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 70165c600e..0310e7050d 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -209,7 +209,10 @@ module ActiveRecord end def association_primary_key - @association_primary_key ||= options[:primary_key] || klass.primary_key + @association_primary_key ||= + options[:primary_key] || + !options[:polymorphic] && klass.primary_key || + 'id' end def active_record_primary_key @@ -372,6 +375,10 @@ module ActiveRecord raise HasManyThroughAssociationNotFoundError.new(active_record.name, self) end + if through_reflection.options[:polymorphic] + raise HasManyThroughAssociationPolymorphicThroughError.new(active_record.name, self) + end + if source_reflection.nil? raise HasManyThroughSourceAssociationNotFoundError.new(self) end @@ -381,13 +388,17 @@ module ActiveRecord end if source_reflection.options[:polymorphic] && options[:source_type].nil? - raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection) + raise HasManyThroughAssociationPolymorphicSourceError.new(active_record.name, self, source_reflection) end unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil? raise HasManyThroughSourceAssociationMacroError.new(self) end + if macro == :has_one && through_reflection.collection? + raise HasOneThroughCantAssociateThroughCollection.new(active_record.name, self, through_reflection) + end + check_validity_of_inverse! end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7ecba1c43a..d4c5c85594 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -31,7 +31,7 @@ module ActiveRecord im = arel.compile_insert values im.into @table primary_key_name = @klass.primary_key - primary_key_value = Hash === values ? values[primary_key_name] : nil + primary_key_value = primary_key_name && Hash === values ? values[primary_key] : nil @klass.connection.insert( im.to_sql, diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 70d84619a1..b3f1b9e36a 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -15,7 +15,7 @@ module ActiveRecord table = Arel::Table.new(table_name, :engine => engine) end - attribute = table[column] || Arel::Attribute.new(table, column) + attribute = table[column.to_sym] case value when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::Relation @@ -26,7 +26,7 @@ module ActiveRecord when Range, Arel::Relation attribute.in(value) when ActiveRecord::Base - attribute.eq(Arel.sql(value.quoted_id)) + attribute.eq(value.id) when Class # FIXME: I think we need to deprecate this behavior attribute.eq(value.name) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 1820f95261..cdde9a80d3 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -17,7 +17,7 @@ require 'models/essay' class BelongsToAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :topics, :developers_projects, :computers, :authors, :author_addresses, - :posts, :tags, :taggings, :comments + :posts, :tags, :taggings, :comments, :sponsors, :members def test_belongs_to Client.find(3).firm.name @@ -488,39 +488,53 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_reassigning_the_parent_id_updates_the_object - original_parent = Firm.create! :name => "original" - updated_parent = Firm.create! :name => "updated" + client = companies(:second_client) - client = Client.new("client_of" => original_parent.id) - assert_equal original_parent, client.firm - assert_equal original_parent, client.firm_with_condition - assert_equal original_parent, client.firm_with_other_name + client.firm + client.firm_with_condition + firm_proxy = client.send(:association_instance_get, :firm) + firm_with_condition_proxy = client.send(:association_instance_get, :firm_with_condition) - client.client_of = updated_parent.id - assert_equal updated_parent, client.firm - assert_equal updated_parent, client.firm_with_condition - assert_equal updated_parent, client.firm_with_other_name + assert !firm_proxy.stale_target? + assert !firm_with_condition_proxy.stale_target? + assert_equal companies(:first_firm), client.firm + assert_equal companies(:first_firm), client.firm_with_condition + + client.client_of = companies(:another_firm).id + + assert firm_proxy.stale_target? + assert firm_with_condition_proxy.stale_target? + assert_equal companies(:another_firm), client.firm + assert_equal companies(:another_firm), client.firm_with_condition end def test_polymorphic_reassignment_of_associated_id_updates_the_object - member1 = Member.create! - member2 = Member.create! + sponsor = sponsors(:moustache_club_sponsor_for_groucho) + + sponsor.sponsorable + proxy = sponsor.send(:association_instance_get, :sponsorable) + + assert !proxy.stale_target? + assert_equal members(:groucho), sponsor.sponsorable - sponsor = Sponsor.new("sponsorable_type" => "Member", "sponsorable_id" => member1.id) - assert_equal member1, sponsor.sponsorable + sponsor.sponsorable_id = members(:some_other_guy).id - sponsor.sponsorable_id = member2.id - assert_equal member2, sponsor.sponsorable + assert proxy.stale_target? + assert_equal members(:some_other_guy), sponsor.sponsorable end def test_polymorphic_reassignment_of_associated_type_updates_the_object - member1 = Member.create! + sponsor = sponsors(:moustache_club_sponsor_for_groucho) - sponsor = Sponsor.new("sponsorable_type" => "Member", "sponsorable_id" => member1.id) - assert_equal member1, sponsor.sponsorable + sponsor.sponsorable + proxy = sponsor.send(:association_instance_get, :sponsorable) - sponsor.sponsorable_type = "Firm" - assert_not_equal member1, sponsor.sponsorable - end + assert !proxy.stale_target? + assert_equal members(:groucho), sponsor.sponsorable + + sponsor.sponsorable_type = 'Firm' + assert proxy.stale_target? + assert_equal companies(:first_firm), sponsor.sponsorable + end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index d5262b1ee4..6efb0d57e4 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -658,10 +658,6 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal people(:david, :susan), Person.find(:all, :include => [:readers, :primary_contact, :number1_fan], :conditions => "number1_fans_people.first_name like 'M%'", :order => 'people.id', :limit => 2, :offset => 0) end - def test_preload_with_interpolation - assert_equal [comments(:greetings)], Post.find(posts(:welcome).id, :include => :comments_with_interpolated_conditions).comments_with_interpolated_conditions - end - def test_polymorphic_type_condition post = Post.find(posts(:thinking).id, :include => :taggings) assert post.taggings.include?(taggings(:thinking_general)) 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 cf0eedbd13..e98d178ff5 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -19,9 +19,10 @@ require 'models/book' require 'models/subscription' require 'models/categorization' require 'models/category' +require 'models/essay' class HasManyThroughAssociationsTest < ActiveRecord::TestCase - fixtures :posts, :readers, :people, :comments, :authors, + fixtures :posts, :readers, :people, :comments, :authors, :categories, :owners, :pets, :toys, :jobs, :references, :companies, :subscribers, :books, :subscriptions, :developers, :categorizations @@ -45,6 +46,16 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:thinking).reload.people(true).include?(people(:david)) end + def test_associate_existing_record_twice_should_add_to_target_twice + post = posts(:thinking) + person = people(:david) + + assert_difference 'post.people.to_a.count', 2 do + post.people << person + post.people << person + end + end + def test_associating_new assert_queries(1) { posts(:thinking) } new_person = nil # so block binding catches it @@ -324,12 +335,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal 2, people(:michael).jobs.size end - def test_get_ids_for_belongs_to_source - assert_sql(/DISTINCT/) { assert_equal [posts(:welcome).id, posts(:authorless).id].sort, people(:michael).post_ids.sort } - end - - def test_get_ids_for_has_many_source - assert_equal [comments(:eager_other_comment1).id], authors(:mary).comment_ids + def test_get_ids + assert_equal [posts(:welcome).id, posts(:authorless).id].sort, people(:michael).post_ids.sort end def test_get_ids_for_loaded_associations @@ -397,6 +404,34 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal people(:susan).agents.map(&:agents).flatten, people(:susan).agents_of_agents end + def test_associate_existing_with_nonstandard_primary_key_on_belongs_to + Categorization.create(:author => authors(:mary), :named_category_name => categories(:general).name) + assert_equal categories(:general), authors(:mary).named_categories.first + end + + def test_collection_build_with_nonstandard_primary_key_on_belongs_to + author = authors(:mary) + category = author.named_categories.build(:name => "Primary") + author.save + assert Categorization.exists?(:author_id => author.id, :named_category_name => category.name) + assert author.named_categories(true).include?(category) + end + + def test_collection_create_with_nonstandard_primary_key_on_belongs_to + author = authors(:mary) + category = author.named_categories.create(:name => "Primary") + assert Categorization.exists?(:author_id => author.id, :named_category_name => category.name) + assert author.named_categories(true).include?(category) + end + + def test_collection_delete_with_nonstandard_primary_key_on_belongs_to + author = authors(:mary) + category = author.named_categories.create(:name => "Primary") + author.named_categories.delete(category) + assert !Categorization.exists?(:author_id => author.id, :named_category_name => category.name) + assert author.named_categories(true).empty? + end + def test_collection_singular_ids_getter_with_string_primary_keys book = books(:awdr) assert_equal 2, book.subscriber_ids.size @@ -466,7 +501,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_with_default_scope_on_join_model - assert_equal posts(:welcome).comments, authors(:david).comments_on_first_posts + assert_equal posts(:welcome).comments.order('id').all, authors(:david).comments_on_first_posts end def test_create_has_many_through_with_default_scope_on_join_model @@ -486,4 +521,33 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal [posts(:eager_other)], posts end + + def test_select_chosen_fields_only + author = authors(:david) + assert_equal ['body'], author.comments.select('comments.body').first.attributes.keys + end + + def test_get_has_many_through_belongs_to_ids_with_conditions + assert_equal [categories(:general).id], authors(:mary).categories_like_general_ids + end + + def test_count_has_many_through_with_named_scope + assert_equal 2, authors(:mary).categories.count + assert_equal 1, authors(:mary).categories.general.count + end + + def test_has_many_through_belongs_to_should_update_when_the_through_foreign_key_changes + post = posts(:eager_other) + + post.author_categorizations + proxy = post.send(:association_instance_get, :author_categorizations) + + assert !proxy.stale_target? + assert_equal authors(:mary).categorizations.sort_by(&:id), post.author_categorizations.sort_by(&:id) + + post.author_id = authors(:david).id + + assert proxy.stale_target? + assert_equal authors(:david).categorizations.sort_by(&:id), post.author_categorizations.sort_by(&:id) + end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 93a4f498d0..7d55d909a7 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -25,10 +25,6 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_equal clubs(:boring_club), @member.club end - def test_has_one_through_with_has_many - assert_equal clubs(:moustache_club), @member.favourite_club - end - def test_creating_association_creates_through_record new_member = Member.create(:name => "Chris") new_member.club = Club.create(:name => "LRUG") @@ -88,6 +84,18 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_not_nil assert_no_queries {members[0].sponsor_club} end + def test_has_one_through_with_conditions_eager_loading + # conditions on the through table + assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :favourite_club).favourite_club + memberships(:membership_of_favourite_club).update_attribute(:favourite, false) + assert_equal nil, Member.find(@member.id, :include => :favourite_club).favourite_club + + # conditions on the source table + assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :hairy_club).hairy_club + clubs(:moustache_club).update_attribute(:name, "Association of Clean-Shaven Persons") + assert_equal nil, Member.find(@member.id, :include => :hairy_club).hairy_club + end + def test_has_one_through_polymorphic_with_source_type assert_equal members(:groucho), clubs(:moustache_club).sponsored_member end @@ -235,6 +243,27 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end def test_has_one_through_with_default_scope_on_join_model - assert_equal posts(:welcome).comments.first, authors(:david).comment_on_first_posts + assert_equal posts(:welcome).comments.order('id').first, authors(:david).comment_on_first_post + end + + def test_has_one_through_many_raises_exception + assert_raise(ActiveRecord::HasOneThroughCantAssociateThroughCollection) do + members(:groucho).club_through_many + end + end + + def test_has_one_through_belongs_to_should_update_when_the_through_foreign_key_changes + minivan = minivans(:cool_first) + + minivan.dashboard + proxy = minivan.send(:association_instance_get, :dashboard) + + assert !proxy.stale_target? + assert_equal dashboards(:cool_first), minivan.dashboard + + minivan.speedometer_id = speedometers(:second).id + + assert proxy.stale_target? + assert_equal dashboards(:second), minivan.dashboard end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 58542bc939..263c90097f 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -340,11 +340,16 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_polymorphic - assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicError do - assert_equal posts(:welcome, :thinking), tags(:general).taggables + assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicSourceError do + tags(:general).taggables end + + assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicThroughError do + taggings(:welcome_general).things + end + assert_raise ActiveRecord::EagerLoadPolymorphicError do - assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable, :conditions => 'bogus_table.column = 1') + tags(:general).taggings.find(:all, :include => :taggable, :conditions => 'bogus_table.column = 1') end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index ffcc7a081a..7d9b1104cd 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -298,7 +298,7 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true @ship.delete - + @pirate.reload.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' }) assert_not_nil @pirate.ship @@ -411,6 +411,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id @pirate.stubs(:id).returns('ABC1X') + @ship.stubs(:pirate_id).returns(@pirate.id) # prevents pirate from being reloaded due to non-matching foreign key @ship.pirate_attributes = { :id => @pirate.id, :catchphrase => 'Arr' } assert_equal 'Arr', @ship.pirate.catchphrase @@ -451,7 +452,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_not_destroy_the_associated_model_until_the_parent_is_saved pirate = @ship.pirate - + @ship.attributes = { :pirate_attributes => { :id => pirate.id, '_destroy' => true } } assert_nothing_raised(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) } @ship.save diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb index 98011f40a4..a1eb96ef09 100644 --- a/activerecord/test/cases/readonly_test.rb +++ b/activerecord/test/cases/readonly_test.rb @@ -12,7 +12,7 @@ def Project.foo() find :first end class ReadOnlyTest < ActiveRecord::TestCase - fixtures :posts, :comments, :developers, :projects, :developers_projects + fixtures :posts, :comments, :developers, :projects, :developers_projects, :people, :readers def test_cant_save_readonly_record dev = Developer.find(1) @@ -71,6 +71,18 @@ class ReadOnlyTest < ActiveRecord::TestCase assert !people.any?(&:readonly?) end + def test_has_many_with_through_is_not_implicitly_marked_readonly_while_finding_by_id + assert !posts(:welcome).people.find(1).readonly? + end + + def test_has_many_with_through_is_not_implicitly_marked_readonly_while_finding_first + assert !posts(:welcome).people.first.readonly? + end + + def test_has_many_with_through_is_not_implicitly_marked_readonly_while_finding_last + assert !posts(:welcome).people.last.readonly? + end + def test_readonly_scoping Post.send(:with_scope, :find => { :conditions => '1=1' }) do assert !Post.find(1).readonly? diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 1e205714a7..901c12b26c 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -7,6 +7,7 @@ require 'models/subscriber' require 'models/ship' require 'models/pirate' require 'models/price_estimate' +require 'models/tagging' class ReflectionTest < ActiveRecord::TestCase include ActiveRecord::Reflection @@ -194,6 +195,7 @@ class ReflectionTest < ActiveRecord::TestCase def test_association_primary_key assert_equal "id", Author.reflect_on_association(:posts).association_primary_key.to_s assert_equal "name", Author.reflect_on_association(:essay).association_primary_key.to_s + assert_equal "id", Tagging.reflect_on_association(:taggable).association_primary_key.to_s end def test_active_record_primary_key diff --git a/activerecord/test/fixtures/dashboards.yml b/activerecord/test/fixtures/dashboards.yml index e75bf46e6c..a4c7e0d309 100644 --- a/activerecord/test/fixtures/dashboards.yml +++ b/activerecord/test/fixtures/dashboards.yml @@ -1,3 +1,6 @@ cool_first: dashboard_id: d1 - name: my_dashboard
\ No newline at end of file + name: my_dashboard +second: + dashboard_id: d2 + name: second diff --git a/activerecord/test/fixtures/members.yml b/activerecord/test/fixtures/members.yml index 6db945e61d..824840b7e5 100644 --- a/activerecord/test/fixtures/members.yml +++ b/activerecord/test/fixtures/members.yml @@ -1,6 +1,8 @@ groucho: + id: 1 name: Groucho Marx member_type_id: 1 some_other_guy: + id: 2 name: Englebert Humperdink member_type_id: 2 diff --git a/activerecord/test/fixtures/memberships.yml b/activerecord/test/fixtures/memberships.yml index b9722dbc8a..eed8b22af8 100644 --- a/activerecord/test/fixtures/memberships.yml +++ b/activerecord/test/fixtures/memberships.yml @@ -1,20 +1,20 @@ membership_of_boring_club: joined_on: <%= 3.weeks.ago.to_s(:db) %> club: boring_club - member: groucho + member_id: 1 favourite: false type: CurrentMembership membership_of_favourite_club: joined_on: <%= 3.weeks.ago.to_s(:db) %> club: moustache_club - member: groucho + member_id: 1 favourite: true type: Membership other_guys_membership: joined_on: <%= 4.weeks.ago.to_s(:db) %> club: boring_club - member: some_other_guy + member_id: 2 favourite: false type: CurrentMembership diff --git a/activerecord/test/fixtures/speedometers.yml b/activerecord/test/fixtures/speedometers.yml index 6a471aba0a..e12398f0c4 100644 --- a/activerecord/test/fixtures/speedometers.yml +++ b/activerecord/test/fixtures/speedometers.yml @@ -1,4 +1,8 @@ cool_first: speedometer_id: s1 name: my_speedometer - dashboard_id: d1
\ No newline at end of file + dashboard_id: d1 +second: + speedometer_id: s2 + name: second + dashboard_id: d2 diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml index 42df8957d1..bfc6b238b1 100644 --- a/activerecord/test/fixtures/sponsors.yml +++ b/activerecord/test/fixtures/sponsors.yml @@ -1,9 +1,12 @@ moustache_club_sponsor_for_groucho: sponsor_club: moustache_club - sponsorable: groucho (Member) + sponsorable_id: 1 + sponsorable_type: Member boring_club_sponsor_for_groucho: sponsor_club: boring_club - sponsorable: some_other_guy (Member) + sponsorable_id: 2 + sponsorable_type: Member crazy_club_sponsor_for_groucho: sponsor_club: crazy_club - sponsorable: some_other_guy (Member)
\ No newline at end of file + sponsorable_id: 2 + sponsorable_type: Member diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index fd6d2b384a..83a6f5d926 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -28,7 +28,9 @@ class Author < ActiveRecord::Base has_many :first_posts has_many :comments_on_first_posts, :through => :first_posts, :source => :comments, :order => 'posts.id desc, comments.id asc' - has_one :comment_on_first_posts, :through => :first_posts, :source => :comments, :order => 'posts.id desc, comments.id asc' + + has_one :first_post + has_one :comment_on_first_post, :through => :first_post, :source => :comments, :order => 'posts.id desc, comments.id asc' has_many :thinking_posts, :class_name => 'Post', :conditions => { :title => 'So I was thinking' }, :dependent => :delete_all has_many :welcome_posts, :class_name => 'Post', :conditions => { :title => 'Welcome to the weblog' } @@ -76,6 +78,7 @@ class Author < ActiveRecord::Base has_many :categorizations has_many :categories, :through => :categorizations + has_many :named_categories, :through => :categorizations has_many :special_categorizations has_many :special_categories, :through => :special_categorizations, :source => :category diff --git a/activerecord/test/models/categorization.rb b/activerecord/test/models/categorization.rb index fdb0a11540..45f50e4af3 100644 --- a/activerecord/test/models/categorization.rb +++ b/activerecord/test/models/categorization.rb @@ -1,6 +1,7 @@ class Categorization < ActiveRecord::Base belongs_to :post belongs_to :category + belongs_to :named_category, :class_name => 'Category', :foreign_key => :named_category_name, :primary_key => :name belongs_to :author belongs_to :author_using_custom_pk, :class_name => 'Author', :foreign_key => :author_id, :primary_key => :author_address_extra_id diff --git a/activerecord/test/models/category.rb b/activerecord/test/models/category.rb index 48415846dd..06908ea85e 100644 --- a/activerecord/test/models/category.rb +++ b/activerecord/test/models/category.rb @@ -23,6 +23,8 @@ class Category < ActiveRecord::Base has_many :categorizations has_many :authors, :through => :categorizations, :select => 'authors.*, categorizations.post_id' + + scope :general, :conditions => { :name => 'General' } end class SpecialCategory < Category diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index 255fb569d7..15ad6aedd3 100644 --- a/activerecord/test/models/member.rb +++ b/activerecord/test/models/member.rb @@ -1,12 +1,16 @@ class Member < ActiveRecord::Base has_one :current_membership - has_many :memberships + has_one :membership has_many :fellow_members, :through => :club, :source => :members has_one :club, :through => :current_membership - has_one :favourite_club, :through => :memberships, :conditions => ["memberships.favourite = ?", true], :source => :club + has_one :favourite_club, :through => :membership, :conditions => ["memberships.favourite = ?", true], :source => :club + has_one :hairy_club, :through => :membership, :conditions => {:clubs => {:name => "Moustache and Eyebrow Fancier Club"}}, :source => :club has_one :sponsor, :as => :sponsorable has_one :sponsor_club, :through => :sponsor has_one :member_detail has_one :organization, :through => :member_detail belongs_to :member_type -end
\ No newline at end of file + + has_many :current_memberships + has_one :club_through_many, :through => :current_memberships, :source => :club +end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 0083560ebe..b51f7ff48f 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -40,9 +40,6 @@ class Post < ActiveRecord::Base has_many :author_favorites, :through => :author has_many :author_categorizations, :through => :author, :source => :categorizations - has_many :comments_with_interpolated_conditions, :class_name => 'Comment', - :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome'] - has_one :very_special_comment has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post has_many :special_comments diff --git a/activerecord/test/models/tagging.rb b/activerecord/test/models/tagging.rb index a1fa1a9750..33ffc623d7 100644 --- a/activerecord/test/models/tagging.rb +++ b/activerecord/test/models/tagging.rb @@ -7,4 +7,5 @@ class Tagging < ActiveRecord::Base belongs_to :super_tag, :class_name => 'Tag', :foreign_key => 'super_tag_id' belongs_to :invalid_tag, :class_name => 'Tag', :foreign_key => 'tag_id' belongs_to :taggable, :polymorphic => true, :counter_cache => true -end
\ No newline at end of file + has_many :things, :through => :taggable +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 99761a4160..3dea7e1492 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -110,6 +110,7 @@ ActiveRecord::Schema.define do create_table :categorizations, :force => true do |t| t.column :category_id, :integer + t.string :named_category_name t.column :post_id, :integer t.column :author_id, :integer t.column :special, :boolean |