diff options
Diffstat (limited to 'activerecord')
90 files changed, 2916 insertions, 1404 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index d1124801df..1f343f690c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,68 @@ *Rails 3.1.0 (unreleased)* +* ActiveRecord::Associations::AssociationProxy has been split. There is now an Association class + (and subclasses) which are responsible for operating on associations, and then a separate, + thin wrapper called CollectionProxy, which proxies collection associations. + + This prevents namespace pollution, separates concerns, and will allow further refactorings. + + Singular associations (has_one, belongs_to) no longer have a proxy at all. They simply return + the associated record or nil. This means that you should not use undocumented methods such + as bob.mother.create - use bob.create_mother instead. + + [Jon Leighton] + +* Make has_many :through associations work correctly when you build a record and then save it. This + requires you to set the :inverse_of option on the source reflection on the join model, like so: + + class Post < ActiveRecord::Base + has_many :taggings + has_many :tags, :through => :taggings + end + + class Tagging < ActiveRecord::Base + belongs_to :post + belongs_to :tag, :inverse_of => :tagging # :inverse_of must be set! + end + + class Tag < ActiveRecord::Base + has_many :taggings + has_many :posts, :through => :taggings + end + + post = Post.first + tag = post.tags.build :name => "ruby" + tag.save # will save a Taggable linking to the post + + [Jon Leighton] + +* Support the :dependent option on has_many :through associations. For historical and practical + reasons, :delete_all is the default deletion strategy employed by association.delete(*records), + despite the fact that the default strategy is :nullify for regular has_many. Also, this only + works at all if the source reflection is a belongs_to. For other situations, you should directly + modify the through association. + + [Jon Leighton] + +* Changed the behaviour of association.destroy for has_and_belongs_to_many and has_many :through. + From now on, 'destroy' or 'delete' on an association will be taken to mean 'get rid of the link', + not (necessarily) 'get rid of the associated records'. + + Previously, has_and_belongs_to_many.destroy(*records) would destroy the records themselves. It + would not delete any records in the join table. Now, it deletes the records in the join table. + + Previously, has_many_through.destroy(*records) would destroy the records themselves, and the + records in the join table. [Note: This has not always been the case; previous version of Rails + only deleted the records themselves.] Now, it destroys only the records in the join table. + + Note that this change is backwards-incompatible to an extent, but there is unfortunately no + way to 'deprecate' it before changing it. The change is being made in order to have + consistency as to the meaning of 'destroy' or 'delete' across the different types of associations. + + If you wish to destroy the records themselves, you can do records.association.each(&:destroy) + + [Jon Leighton] + * Add :bulk => true option to change_table to make all the schema changes defined in change_table block using a single ALTER statement. [Pratik Naik] Example: @@ -23,8 +86,27 @@ (for example, add_name_to_users) use the reversible migration's `change` method instead of the ordinary `up` and `down` methods. [Prem Sichanugrist] -* Removed support for interpolated SQL conditions. Please use scoping -along with attribute conditionals as a replacement. +* Removed support for interpolating string SQL conditions on associations. Instead, you should + use a proc, like so: + + Before: + + has_many :things, :conditions => 'foo = #{bar}' + + After: + + has_many :things, :conditions => proc { "foo = #{bar}" } + + Inside the proc, 'self' is the object which is the owner of the association, unless you are + eager loading the association, in which case 'self' is the class which the association is within. + + You can have any "normal" conditions inside the proc, so the following will work too: + + has_many :things, :conditions => proc { ["foo = ?", bar] } + + Previously :insert_sql and :delete_sql on has_and_belongs_to_many association allowed you to call + 'record' to get the record being inserted or deleted. This is now passed as an argument to + the proc. * Added ActiveRecord::Base#has_secure_password (via ActiveModel::SecurePassword) to encapsulate dead-simple password usage with BCrypt encryption and salting [DHH]. Example: diff --git a/activerecord/examples/performance.rb b/activerecord/examples/performance.rb index c4ce361b5d..63822731d5 100644 --- a/activerecord/examples/performance.rb +++ b/activerecord/examples/performance.rb @@ -1,31 +1,9 @@ -#!/usr/bin/env ruby -KU - TIMES = (ENV['N'] || 10000).to_i -require 'rubygems' - -gem 'addressable', '~>2.0' -gem 'faker', '~>0.3.1' -gem 'rbench', '~>0.2.3' -require 'addressable/uri' -require 'faker' -require 'rbench' - -require File.expand_path("../../../load_paths", __FILE__) +require 'rubygems' require "active_record" -conn = { :adapter => 'mysql', - :database => 'activerecord_unittest', - :username => 'rails', :password => '', - :encoding => 'utf8' } - -conn[:socket] = Pathname.glob(%w[ - /opt/local/var/run/mysql5/mysqld.sock - /tmp/mysqld.sock - /tmp/mysql.sock - /var/mysql/mysql.sock - /var/run/mysqld/mysqld.sock -]).find { |path| path.socket? }.to_s +conn = { :adapter => 'sqlite3', :database => ':memory:' } ActiveRecord::Base.establish_connection(conn) @@ -55,125 +33,126 @@ class Exhibit < ActiveRecord::Base def self.feel(exhibits) exhibits.each { |e| e.feel } end end -sqlfile = File.expand_path("../performance.sql", __FILE__) - -if File.exists?(sqlfile) - mysql_bin = %w[mysql mysql5].detect { |bin| `which #{bin}`.length > 0 } - `#{mysql_bin} -u #{conn[:username]} #{"-p#{conn[:password]}" unless conn[:password].blank?} #{conn[:database]} < #{sqlfile}` -else - puts 'Generating data...' - - # pre-compute the insert statements and fake data compilation, - # so the benchmarks below show the actual runtime for the execute - # method, minus the setup steps - - # Using the same paragraph for all exhibits because it is very slow - # to generate unique paragraphs for all exhibits. - notes = Faker::Lorem.paragraphs.join($/) - today = Date.today - - puts 'Inserting 10,000 users and exhibits...' - 10_000.times do - user = User.create( - :created_at => today, - :name => Faker::Name.name, - :email => Faker::Internet.email - ) - - Exhibit.create( - :created_at => today, - :name => Faker::Company.name, - :user => user, - :notes => notes - ) - end - - mysqldump_bin = %w[mysqldump mysqldump5].detect { |bin| `which #{bin}`.length > 0 } - `#{mysqldump_bin} -u #{conn[:username]} #{"-p#{conn[:password]}" unless conn[:password].blank?} #{conn[:database]} exhibits users > #{sqlfile}` +puts 'Generating data...' + +module ActiveRecord + class Faker + LOREM = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse non aliquet diam. Curabitur vel urna metus, quis malesuada elit. Integer consequat tincidunt felis. Etiam non erat dolor. Vivamus imperdiet nibh sit amet diam eleifend id posuere diam malesuada. Mauris at accumsan sem. Donec id lorem neque. Fusce erat lorem, ornare eu congue vitae, malesuada quis neque. Maecenas vel urna a velit pretium fermentum. Donec tortor enim, tempor venenatis egestas a, tempor sed ipsum. Ut arcu justo, faucibus non imperdiet ac, interdum at diam. Pellentesque ipsum enim, venenatis ut iaculis vitae, varius vitae sem. Sed rutrum quam ac elit euismod bibendum. Donec ultricies ultricies magna, at lacinia libero mollis aliquam. Sed ac arcu in tortor elementum tincidunt vel interdum sem. Curabitur eget erat arcu. Praesent eget eros leo. Nam magna enim, sollicitudin vehicula scelerisque in, vulputate ut libero. Praesent varius tincidunt commodo".split + def self.name + LOREM.grep(/^\w*$/).sort_by { rand }.first(2).join ' ' + end + + def self.email + LOREM.grep(/^\w*$/).sort_by { rand }.first(2).join('@') + ".com" + end + end +end + +# pre-compute the insert statements and fake data compilation, +# so the benchmarks below show the actual runtime for the execute +# method, minus the setup steps + +# Using the same paragraph for all exhibits because it is very slow +# to generate unique paragraphs for all exhibits. +notes = ActiveRecord::Faker::LOREM.join ' ' +today = Date.today + +puts 'Inserting 10,000 users and exhibits...' +10_000.times do + user = User.create( + :created_at => today, + :name => ActiveRecord::Faker.name, + :email => ActiveRecord::Faker.email + ) + + Exhibit.create( + :created_at => today, + :name => ActiveRecord::Faker.name, + :user => user, + :notes => notes + ) end -RBench.run(TIMES) do - column :times - column :ar +require 'benchmark' - report 'Model#id', (TIMES * 100).ceil do - ar_obj = Exhibit.find(1) +Benchmark.bm(46) do |x| + ar_obj = Exhibit.find(1) + attrs = { :name => 'sam' } + attrs_first = { :name => 'sam' } + attrs_second = { :name => 'tom' } + exhibit = { + :name => ActiveRecord::Faker.name, + :notes => notes, + :created_at => Date.today + } - ar { ar_obj.id } + x.report("Model#id (x#{(TIMES * 100).ceil})") do + (TIMES * 100).ceil.times { ar_obj.id } end - report 'Model.new (instantiation)' do - ar { Exhibit.new } + x.report 'Model.new (instantiation)' do + TIMES.times { Exhibit.new } end - report 'Model.new (setting attributes)' do - attrs = { :name => 'sam' } - ar { Exhibit.new(attrs) } + x.report 'Model.new (setting attributes)' do + TIMES.times { Exhibit.new(attrs) } end - report 'Model.first' do - ar { Exhibit.first.look } + x.report 'Model.first' do + TIMES.times { Exhibit.first.look } end - report 'Model.all limit(100)', (TIMES / 10).ceil do - ar { Exhibit.look Exhibit.limit(100) } + x.report("Model.all limit(100) (x#{(TIMES / 10).ceil})") do + (TIMES / 10).ceil.times { Exhibit.look Exhibit.limit(100) } end - report 'Model.all limit(100) with relationship', (TIMES / 10).ceil do - ar { Exhibit.feel Exhibit.limit(100).includes(:user) } + x.report "Model.all limit(100) with relationship (x#{(TIMES / 10).ceil})" do + (TIMES / 10).ceil.times { Exhibit.feel Exhibit.limit(100).includes(:user) } end - report 'Model.all limit(10,000)', (TIMES / 1000).ceil do - ar { Exhibit.look Exhibit.limit(10000) } + x.report "Model.all limit(10,000) x(#{(TIMES / 1000).ceil})" do + (TIMES / 1000).ceil.times { Exhibit.look Exhibit.limit(10000) } end - exhibit = { - :name => Faker::Company.name, - :notes => Faker::Lorem.paragraphs.join($/), - :created_at => Date.today - } - - report 'Model.create' do - ar { Exhibit.create(exhibit) } + x.report 'Model.create' do + TIMES.times { Exhibit.create(exhibit) } end - report 'Resource#attributes=' do - attrs_first = { :name => 'sam' } - attrs_second = { :name => 'tom' } - ar { exhibit = Exhibit.new(attrs_first); exhibit.attributes = attrs_second } + x.report 'Resource#attributes=' do + TIMES.times { + exhibit = Exhibit.new(attrs_first) + exhibit.attributes = attrs_second + } end - report 'Resource#update' do - ar { Exhibit.first.update_attributes(:name => 'bob') } + x.report 'Resource#update' do + TIMES.times { Exhibit.first.update_attributes(:name => 'bob') } end - report 'Resource#destroy' do - ar { Exhibit.first.destroy } + x.report 'Resource#destroy' do + TIMES.times { Exhibit.first.destroy } end - report 'Model.transaction' do - ar { Exhibit.transaction { Exhibit.new } } + x.report 'Model.transaction' do + TIMES.times { Exhibit.transaction { Exhibit.new } } end - report 'Model.find(id)' do + x.report 'Model.find(id)' do id = Exhibit.first.id - ar { Exhibit.find(id) } + TIMES.times { Exhibit.find(id) } end - report 'Model.find_by_sql' do - ar { Exhibit.find_by_sql("SELECT * FROM exhibits WHERE id = #{(rand * 1000 + 1).to_i}").first } + x.report 'Model.find_by_sql' do + TIMES.times { + Exhibit.find_by_sql("SELECT * FROM exhibits WHERE id = #{(rand * 1000 + 1).to_i}").first + } end - report 'Model.log', (TIMES * 10) do - ar { Exhibit.connection.send(:log, "hello", "world") {} } + x.report "Model.log x(#{TIMES * 10})" do + (TIMES * 10).times { Exhibit.connection.send(:log, "hello", "world") {} } end - report 'AR.execute(query)', (TIMES / 2) do - ar { ActiveRecord::Base.connection.execute("Select * from exhibits where id = #{(rand * 1000 + 1).to_i}") } + x.report "AR.execute(query) (#{TIMES / 2})" do + (TIMES / 2).times { ActiveRecord::Base.connection.execute("Select * from exhibits where id = #{(rand * 1000 + 1).to_i}") } end - - summary 'Total' end - -ActiveRecord::Migration.drop_table "exhibits" -ActiveRecord::Migration.drop_table "users" diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 5afb97803e..8379f6a66f 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -79,6 +79,7 @@ module ActiveRecord autoload :Timestamp autoload :Transactions autoload :Validations + autoload :IdentityMap end module Coders diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index b83c00e9f8..a34a73cf5d 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -124,16 +124,16 @@ module ActiveRecord def add_preloaded_records_to_collection(parent_records, reflection_name, associated_record) parent_records.each do |parent_record| - association_proxy = parent_record.send(reflection_name) - association_proxy.loaded! - association_proxy.target.concat(Array.wrap(associated_record)) - association_proxy.send(:set_inverse_instance, associated_record) + association = parent_record.association(reflection_name) + association.loaded! + association.target.concat(Array.wrap(associated_record)) + association.set_inverse_instance(associated_record) end end def add_preloaded_record_to_collection(parent_records, reflection_name, associated_record) parent_records.each do |parent_record| - parent_record.send(:association_proxy, reflection_name).target = associated_record + parent_record.association(reflection_name).target = associated_record end end @@ -158,7 +158,7 @@ module ActiveRecord seen_keys[seen_key] = true mapped_records = id_to_record_map[seen_key] mapped_records.each do |mapped_record| - association_proxy = mapped_record.send(:association_proxy, reflection_name) + association_proxy = mapped_record.association(reflection_name) association_proxy.target = associated_record association_proxy.send(:set_inverse_instance, associated_record) end @@ -187,7 +187,7 @@ module ActiveRecord id_to_record_map = construct_id_map(records) - records.each { |record| record.send(reflection.name).loaded! } + records.each { |record| record.association(reflection.name).loaded! } options = reflection.options right = Arel::Table.new(options[:join_table]).alias('t0') @@ -233,7 +233,7 @@ module ActiveRecord end def preload_has_one_association(records, reflection, preload_options={}) - return if records.first.send(:association_proxy, reflection.name).loaded? + return if records.first.association(reflection.name).loaded? id_to_record_map = construct_id_map(records, reflection.options[:primary_key]) options = reflection.options @@ -268,7 +268,7 @@ module ActiveRecord foreign_key = reflection.through_reflection_foreign_key id_to_record_map = construct_id_map(records, foreign_key || reflection.options[:primary_key]) - records.each { |record| record.send(reflection.name).loaded! } + records.each { |record| record.association(reflection.name).loaded! } if options[:through] through_records = preload_through_records(records, reflection, options[:through]) @@ -298,7 +298,7 @@ module ActiveRecord # Dont cache the association - we would only be caching a subset records.map { |record| - proxy = record.send(through_association) + proxy = record.association(through_association) if proxy.respond_to?(:target) Array.wrap(proxy.target).tap { proxy.reset } @@ -320,7 +320,7 @@ module ActiveRecord end def preload_belongs_to_association(records, reflection, preload_options={}) - return if records.first.send(:association_proxy, reflection.name).loaded? + return if records.first.association(reflection.name).loaded? options = reflection.options klasses_and_ids = {} @@ -399,10 +399,18 @@ module ActiveRecord end end + def process_conditions(conditions, klass = self) + if conditions.respond_to?(:to_proc) + conditions = instance_eval(&conditions) + end + + klass.send(:sanitize_sql, conditions) + end + def append_conditions(reflection, preload_options) [ - ("(#{reflection.sanitized_conditions})" if reflection.sanitized_conditions), - ("(#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]), + ('(' + process_conditions(reflection.options[:conditions], reflection.klass) + ')' if reflection.options[:conditions]), + ('(' + process_conditions(preload_options[:conditions]) + ')' if preload_options[:conditions]), ].compact.map { |x| Arel.sql x } end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b39f6a49ae..364a7248d2 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -118,17 +118,19 @@ module ActiveRecord # These classes will be loaded when associations are created. # So there is no need to eager load them. - autoload :AssociationCollection, 'active_record/associations/association_collection' - autoload :SingularAssociation, 'active_record/associations/singular_association' - autoload :AssociationProxy, 'active_record/associations/association_proxy' - autoload :ThroughAssociation, 'active_record/associations/through_association' - autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' + autoload :Association, 'active_record/associations/association' + autoload :SingularAssociation, 'active_record/associations/singular_association' + autoload :CollectionAssociation, 'active_record/associations/collection_association' + autoload :CollectionProxy, 'active_record/associations/collection_proxy' + + autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' autoload :BelongsToPolymorphicAssociation, 'active_record/associations/belongs_to_polymorphic_association' - autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' - autoload :HasManyAssociation, 'active_record/associations/has_many_association' - autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association' - autoload :HasOneAssociation, 'active_record/associations/has_one_association' - autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association' + autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' + autoload :HasManyAssociation, 'active_record/associations/has_many_association' + autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association' + autoload :HasOneAssociation, 'active_record/associations/has_one_association' + autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association' + autoload :ThroughAssociation, 'active_record/associations/through_association' # Clears out the association cache. def clear_association_cache #:nodoc: @@ -138,29 +140,23 @@ module ActiveRecord # :nodoc: attr_reader :association_cache - protected - - # Returns the proxy for the given association name, instantiating it if it doesn't - # already exist - def association_proxy(name) - association = association_instance_get(name) + # Returns the association instance for the given name, instantiating it if it doesn't already exist + def association(name) #:nodoc: + association = association_instance_get(name) - if association.nil? - reflection = self.class.reflect_on_association(name) - association = reflection.proxy_class.new(self, reflection) - association_instance_set(name, association) - end - - association + if association.nil? + reflection = self.class.reflect_on_association(name) + association = reflection.association_class.new(self, reflection) + association_instance_set(name, association) end + association + end + private # Returns the specified association instance if it responds to :loaded?, nil otherwise. def association_instance_get(name) - if @association_cache.key? name - association = @association_cache[name] - association if association.respond_to?(:loaded?) - end + @association_cache[name] end # Set the specified association instance. @@ -232,10 +228,9 @@ module ActiveRecord # others.empty? | X | X | X # others.clear | X | X | X # others.delete(other,other,...) | X | X | X - # others.delete_all | X | X | + # others.delete_all | X | X | X # others.destroy_all | X | X | X # others.find(*args) | X | X | X - # others.find_first | X | | # others.exists? | X | X | X # others.uniq | X | X | X # others.reset | X | X | X @@ -524,6 +519,22 @@ module ActiveRecord # @group.avatars << Avatar.new # this would work if User belonged_to Avatar rather than the other way around # @group.avatars.delete(@group.avatars.last) # so would this # + # If you are using a +belongs_to+ on the join model, it is a good idea to set the + # <tt>:inverse_of</tt> option on the +belongs_to+, which will mean that the following example + # works correctly (where <tt>tags</tt> is a +has_many+ <tt>:through</tt> association): + # + # @post = Post.first + # @tag = @post.tags.build :name => "ruby" + # @tag.save + # + # The last line ought to save the through record (a <tt>Taggable</tt>). This will only work if the + # <tt>:inverse_of</tt> is set: + # + # class Taggable < ActiveRecord::Base + # belongs_to :post + # belongs_to :tag, :inverse_of => :taggings + # end + # # === Polymorphic Associations # # Polymorphic associations on models are not restricted on what types of models they @@ -833,6 +844,73 @@ module ActiveRecord # * does not work with <tt>:polymorphic</tt> associations. # * for +belongs_to+ associations +has_many+ inverse associations are ignored. # + # == Deleting from associations + # + # === Dependent associations + # + # +has_many+, +has_one+ and +belongs_to+ associations support the <tt>:dependent</tt> option. + # This allows you to specify that associated records should be deleted when the owner is + # deleted. + # + # For example: + # + # class Author + # has_many :posts, :dependent => :destroy + # end + # Author.find(1).destroy # => Will destroy all of the author's posts, too + # + # The <tt>:dependent</tt> option can have different values which specify how the deletion + # is done. For more information, see the documentation for this option on the different + # specific association types. + # + # === Delete or destroy? + # + # +has_many+ and +has_and_belongs_to_many+ associations have the methods <tt>destroy</tt>, + # <tt>delete</tt>, <tt>destroy_all</tt> and <tt>delete_all</tt>. + # + # For +has_and_belongs_to_many+, <tt>delete</tt> and <tt>destroy</tt> are the same: they + # cause the records in the join table to be removed. + # + # For +has_many+, <tt>destroy</tt> will always call the <tt>destroy</tt> method of the + # record(s) being removed so that callbacks are run. However <tt>delete</tt> will either + # do the deletion according to the strategy specified by the <tt>:dependent</tt> option, or + # if no <tt>:dependent</tt> option is given, then it will follow the default strategy. + # The default strategy is <tt>:nullify</tt> (set the foreign keys to <tt>nil</tt>), except for + # +has_many+ <tt>:through</tt>, where the default strategy is <tt>delete_all</tt> (delete + # the join records, without running their callbacks). + # + # There is also a <tt>clear</tt> method which is the same as <tt>delete_all</tt>, except that + # it returns the association rather than the records which have been deleted. + # + # === What gets deleted? + # + # There is a potential pitfall here: +has_and_belongs_to_many+ and +has_many+ <tt>:through</tt> + # associations have records in join tables, as well as the associated records. So when we + # call one of these deletion methods, what exactly should be deleted? + # + # The answer is that it is assumed that deletion on an association is about removing the + # <i>link</i> between the owner and the associated object(s), rather than necessarily the + # associated objects themselves. So with +has_and_belongs_to_many+ and +has_many+ + # <tt>:through</tt>, the join records will be deleted, but the associated records won't. + # + # This makes sense if you think about it: if you were to call <tt>post.tags.delete(Tag.find_by_name('food'))</tt> + # you would want the 'food' tag to be unlinked from the post, rather than for the tag itself + # to be removed from the database. + # + # However, there are examples where this strategy doesn't make sense. For example, suppose + # a person has many projects, and each project has many tasks. If we deleted one of a person's + # tasks, we would probably not want the project to be deleted. In this scenario, the delete method + # won't actually work: it can only be used if the association on the join model is a + # +belongs_to+. In other situations you are expected to perform operations directly on + # either the associated records or the <tt>:through</tt> association. + # + # With a regular +has_many+ there is no distinction between the "associated records" + # and the "link", so there is only one choice for what gets deleted. + # + # With +has_and_belongs_to_many+ and +has_many+ <tt>:through</tt>, if you want to delete the + # associated records themselves, you can always do something along the lines of + # <tt>person.tasks.each(&:destroy)</tt>. + # # == Type safety with <tt>ActiveRecord::AssociationTypeMismatch</tt> # # If you attempt to assign an object to an association that doesn't match the inferred @@ -857,6 +935,10 @@ module ActiveRecord # Removes one or more objects from the collection by setting their foreign keys to +NULL+. # Objects will be in addition destroyed if they're associated with <tt>:dependent => :destroy</tt>, # and deleted if they're associated with <tt>:dependent => :delete_all</tt>. + # + # If the <tt>:through</tt> option is used, then the join records are deleted (rather than + # nullified) by default, but you can specify <tt>:dependent => :destroy</tt> or + # <tt>:dependent => :nullify</tt> to override this. # [collection=objects] # Replaces the collections content by deleting and adding objects as appropriate. If the <tt>:through</tt> # option is true callbacks in the join models are triggered except destroy callbacks, since deletion is @@ -940,7 +1022,9 @@ module ActiveRecord # objects' foreign keys are set to +NULL+ *without* calling their +save+ callbacks. If set to # <tt>:restrict</tt> this object cannot be deleted if it has any associated object. # - # *Warning:* This option is ignored when used with <tt>:through</tt> option. + # If using with the <tt>:through</tt> option, the association on the join model must be + # a +belongs_to+, and the records which get deleted are the join records, rather than + # the associated records. # # [:finder_sql] # Specify a complete SQL statement to fetch the association. This is a good way to go for complex @@ -971,13 +1055,21 @@ module ActiveRecord # [:as] # Specifies a polymorphic interface (See <tt>belongs_to</tt>). # [:through] - # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt> - # and <tt>:foreign_key</tt> are ignored, as the association uses the source reflection. You - # can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>, <tt>has_one</tt> - # or <tt>has_many</tt> association on the join model. The collection of join models - # can be managed via the collection API. For example, new join models are created for - # newly associated objects, and if some are gone their rows are deleted (directly, - # no destroy callbacks are triggered). + # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt>, + # <tt>:primary_key</tt> and <tt>:foreign_key</tt> are ignored, as the association uses the + # source reflection. You can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>, + # <tt>has_one</tt> or <tt>has_many</tt> association on the join model. + # + # If the association on the join model is a +belongs_to+, the collection can be modified + # and the records on the <tt>:through</tt> model will be automatically created and removed + # as appropriate. Otherwise, the collection is read-only, so you should manipulate the + # <tt>:through</tt> association directly. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option on the source association on the + # join model. This allows associated records to be built which will automatically create + # the appropriate join model records when they are saved. (See the 'Association Join Models' + # section above.) # [:source] # Specifies the source association name used by <tt>has_many :through</tt> queries. # Only use it if the name cannot be inferred from the association. @@ -1478,7 +1570,7 @@ module ActiveRecord def association_accessor_methods(reflection) redefine_method(reflection.name) do |*params| force_reload = params.first unless params.empty? - association = association_proxy(reflection.name) + association = association(reflection.name) if force_reload reflection.klass.uncached { association.reload } @@ -1486,18 +1578,18 @@ module ActiveRecord association.reload end - association.target.nil? ? nil : association + association.target end redefine_method("#{reflection.name}=") do |record| - association_proxy(reflection.name).replace(record) + association(reflection.name).replace(record) end end def collection_reader_method(reflection) redefine_method(reflection.name) do |*params| force_reload = params.first unless params.empty? - association = association_proxy(reflection.name) + association = association(reflection.name) if force_reload reflection.klass.uncached { association.reload } @@ -1505,14 +1597,17 @@ module ActiveRecord association.reload end - association + association.proxy end redefine_method("#{reflection.name.to_s.singularize}_ids") do if send(reflection.name).loaded? || reflection.options[:finder_sql] - send(reflection.name).map { |r| r.id } + records = send(reflection.name) + records.map { |r| r.send(reflection.association_primary_key) } else - send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id } + column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}" + records = send(reflection.name).select(column).except(:includes) + records.map! { |r| r.send(reflection.association_primary_key) } end end end @@ -1522,7 +1617,7 @@ module ActiveRecord if writer redefine_method("#{reflection.name}=") do |new_value| - association_proxy(reflection.name).replace(new_value) + association(reflection.name).replace(new_value) end redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value| @@ -1544,7 +1639,7 @@ module ActiveRecord constructors.each do |name, proxy_name| redefine_method(name) do |*params| attributes = params.first unless params.empty? - association_proxy(reflection.name).send(proxy_name, attributes) + association(reflection.name).send(proxy_name, attributes) end end end @@ -1606,12 +1701,11 @@ module ActiveRecord send(reflection.name).each do |o| # No point in executing the counter update since we're going to destroy the parent anyway counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym - if(o.respond_to? counter_method) then + if o.respond_to?(counter_method) class << o self end.send(:define_method, counter_method, Proc.new {}) end - o.destroy end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb new file mode 100644 index 0000000000..2eb431dfec --- /dev/null +++ b/activerecord/lib/active_record/associations/association.rb @@ -0,0 +1,262 @@ +require 'active_support/core_ext/array/wrap' + +module ActiveRecord + module Associations + # = Active Record Associations + # + # This is the root class of all associations ('+ Foo' signifies an included module Foo): + # + # Association + # SingularAssociaton + # HasOneAssociation + # HasOneThroughAssociation + ThroughAssociation + # BelongsToAssociation + # BelongsToPolymorphicAssociation + # CollectionAssociation + # HasAndBelongsToManyAssociation + # HasManyAssociation + # HasManyThroughAssociation + ThroughAssociation + class Association #:nodoc: + attr_reader :owner, :target, :reflection + + delegate :options, :klass, :to => :reflection + + def initialize(owner, reflection) + reflection.check_validity! + + @target = nil + @owner, @reflection = owner, reflection + @updated = false + + reset + construct_scope + end + + # Returns the name of the table of the related class: + # + # post.comments.aliased_table_name # => "comments" + # + def aliased_table_name + @reflection.klass.table_name + end + + # Resets the \loaded flag to +false+ and sets the \target to +nil+. + def reset + @loaded = false + IdentityMap.remove(@target) if IdentityMap.enabled? && @target + @target = nil + end + + # Reloads the \target and returns +self+ on success. + def reload + reset + construct_scope + load_target + self unless @target.nil? + end + + # Has the \target been already \loaded? + def loaded? + @loaded + end + + # Asserts the \target has been loaded setting the \loaded flag to +true+. + def loaded! + @loaded = true + @stale_state = stale_state + 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 the + # state_state method if relevant. + # + # Note that if the target has not been loaded, it is not considered stale. + def stale_target? + loaded? && @stale_state != stale_state + end + + # Sets the target of this association to <tt>\target</tt>, and the \loaded flag to +true+. + def target=(target) + @target = target + loaded! + end + + def scoped + target_scope.merge(@association_scope) + end + + # Construct the scope for this association. + # + # Note that the association_scope is merged into the targed_scope only when the + # scoped method is called. This is because at that point the call may be surrounded + # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which + # actually gets built. + def construct_scope + @association_scope = association_scope if target_klass + end + + def association_scope + scope = target_klass.unscoped + scope = scope.create_with(creation_attributes) + scope = scope.apply_finder_options(@reflection.options.slice(:readonly, :include)) + scope = scope.where(interpolate(@reflection.options[:conditions])) + if select = select_value + scope = scope.select(select) + end + scope = scope.extending(*Array.wrap(@reflection.options[:extend])) + scope.where(construct_owner_conditions) + end + + def aliased_table + target_klass.arel_table + end + + # Set the inverse association, if possible + def set_inverse_instance(record) + if record && invertible_for?(record) + inverse = record.association(inverse_reflection_for(record).name) + inverse.target = @owner + end + end + + # This class of the target. belongs_to polymorphic overrides this to look at the + # polymorphic_type field on the owner. + def target_klass + @reflection.klass + end + + # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the + # through association's scope) + def target_scope + target_klass.scoped + end + + # Loads the \target if needed and returns it. + # + # This method is abstract in the sense that it relies on +find_target+, + # which is expected to be provided by descendants. + # + # If the \target is already \loaded it is just returned. Thus, you can call + # +load_target+ unconditionally to get the \target. + # + # ActiveRecord::RecordNotFound is rescued within the method, and it is + # not reraised. The proxy is \reset and +nil+ is the return value. + def load_target + if find_target? + begin + if IdentityMap.enabled? && association_class && association_class.respond_to?(:base_class) + @target = IdentityMap.get(association_class, @owner[@reflection.foreign_key]) + end + rescue NameError + nil + ensure + @target ||= find_target + end + end + loaded! + target + rescue ActiveRecord::RecordNotFound + reset + end + + private + + def find_target? + !loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass + end + + def interpolate(sql, record = nil) + if sql.respond_to?(:to_proc) + @owner.send(:instance_exec, record, &sql) + else + sql + end + end + + def select_value + @reflection.options[:select] + end + + # Implemented by (some) subclasses + def creation_attributes + { } + end + + # Returns a hash linking the owner to the association represented by the reflection + def construct_owner_attributes(reflection = @reflection) + attributes = {} + if reflection.macro == :belongs_to + attributes[reflection.association_primary_key] = @owner[reflection.foreign_key] + else + attributes[reflection.foreign_key] = @owner[reflection.active_record_primary_key] + + if reflection.options[:as] + attributes["#{reflection.options[:as]}_type"] = @owner.class.base_class.name + end + end + attributes + end + + # Builds an array of arel nodes from the owner attributes hash + def construct_owner_conditions(table = aliased_table, reflection = @reflection) + conditions = construct_owner_attributes(reflection).map do |attr, value| + table[attr].eq(value) + end + table.create_and(conditions) + end + + # Sets the owner attributes on the given record + def set_owner_attributes(record) + if @owner.persisted? + construct_owner_attributes.each { |key, value| record[key] = value } + end + end + + # Should be true if there is a foreign key present on the @owner which + # references the target. This is used to determine whether we can load + # the target if the @owner is currently a new record (and therefore + # without a key). + # + # Currently implemented by belongs_to (vanilla and polymorphic) and + # has_one/has_many :through associations which go through a belongs_to + def foreign_key_present? + false + end + + # Raises ActiveRecord::AssociationTypeMismatch unless +record+ is of + # the kind of the class of the associated objects. Meant to be used as + # a sanity check when you are about to assign an associated record. + def raise_on_type_mismatch(record) + unless record.is_a?(@reflection.klass) || record.is_a?(@reflection.class_name.constantize) + message = "#{@reflection.class_name}(##{@reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" + raise ActiveRecord::AssociationTypeMismatch, message + end + end + + # Can be redefined by subclasses, notably polymorphic belongs_to + # The record parameter is necessary to support polymorphic inverses as we must check for + # the association in the specific class of the record. + def inverse_reflection_for(record) + @reflection.inverse_of + end + + # Is this association invertible? Can be redefined by subclasses. + def invertible_for?(record) + inverse_reflection_for(record) + end + + # This should be implemented to return the values of the relevant key(s) on the owner, + # so that when state_state is different from the value stored on the last find_target, + # the target is stale. + # + # This is only relevant to certain associations, which is why it returns nil by default. + def stale_state + end + + def association_class + @reflection.klass + end + end + end +end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb deleted file mode 100644 index 07fff7f7d7..0000000000 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ /dev/null @@ -1,328 +0,0 @@ -require 'active_support/core_ext/array/wrap' - -module ActiveRecord - module Associations - # = Active Record Associations - # - # This is the root class of all association proxies ('+ Foo' signifies an included module Foo): - # - # AssociationProxy - # SingularAssociaton - # HasOneAssociation - # HasOneThroughAssociation + ThroughAssociation - # BelongsToAssociation - # BelongsToPolymorphicAssociation - # AssociationCollection - # HasAndBelongsToManyAssociation - # HasManyAssociation - # HasManyThroughAssociation + ThroughAssociation - # - # Association proxies in Active Record are middlemen between the object that - # holds the association, known as the <tt>@owner</tt>, and the actual associated - # object, known as the <tt>@target</tt>. The kind of association any proxy is - # about is available in <tt>@reflection</tt>. That's an instance of the class - # ActiveRecord::Reflection::AssociationReflection. - # - # For example, given - # - # class Blog < ActiveRecord::Base - # has_many :posts - # end - # - # blog = Blog.find(:first) - # - # the association proxy in <tt>blog.posts</tt> has the object in +blog+ as - # <tt>@owner</tt>, the collection of its posts as <tt>@target</tt>, and - # the <tt>@reflection</tt> object represents a <tt>:has_many</tt> macro. - # - # This class has most of the basic instance methods removed, and delegates - # unknown methods to <tt>@target</tt> via <tt>method_missing</tt>. As a - # corner case, it even removes the +class+ method and that's why you get - # - # blog.posts.class # => Array - # - # though the object behind <tt>blog.posts</tt> is not an Array, but an - # ActiveRecord::Associations::HasManyAssociation. - # - # The <tt>@target</tt> object is not \loaded until needed. For example, - # - # blog.posts.count - # - # is computed directly through SQL and does not trigger by itself the - # instantiation of the actual post records. - class AssociationProxy #:nodoc: - alias_method :proxy_extend, :extend - - instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id|to_a)$|^__|^respond_to|proxy_/ } - - def initialize(owner, reflection) - @owner, @reflection = owner, reflection - @updated = false - reflection.check_validity! - Array.wrap(reflection.options[:extend]).each { |ext| proxy_extend(ext) } - reset - construct_scope - end - - def to_param - proxy_target.to_param - end - - # Returns the owner of the proxy. - def proxy_owner - @owner - end - - # Returns the reflection object that represents the association handled - # by the proxy. - def proxy_reflection - @reflection - end - - # Does the proxy or its \target respond to +symbol+? - def respond_to?(*args) - super || (load_target && @target.respond_to?(*args)) - end - - # Forwards any missing method call to the \target. - def method_missing(method, *args, &block) - if load_target - return super unless @target.respond_to?(method) - @target.send(method, *args, &block) - end - rescue NoMethodError => e - raise e, e.message.sub(/ for #<.*$/, " via proxy for #{@target}") - end - - # Forwards <tt>===</tt> explicitly to the \target because the instance method - # removal above doesn't catch it. Loads the \target if needed. - def ===(other) - other === load_target - end - - # Returns the name of the table of the related class: - # - # post.comments.aliased_table_name # => "comments" - # - def aliased_table_name - @reflection.klass.table_name - end - - # Resets the \loaded flag to +false+ and sets the \target to +nil+. - def reset - @loaded = false - @target = nil - end - - # Reloads the \target and returns +self+ on success. - def reload - reset - construct_scope - load_target - self unless @target.nil? - end - - # Has the \target been already \loaded? - def loaded? - @loaded - end - - # Asserts the \target has been loaded setting the \loaded flag to +true+. - def loaded! - @loaded = true - @stale_state = stale_state - 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 the - # state_state method if relevant. - # - # Note that if the target has not been loaded, it is not considered stale. - def stale_target? - loaded? && @stale_state != stale_state - end - - # Returns the target of this proxy, same as +proxy_target+. - attr_reader :target - - # Returns the \target of the proxy, same as +target+. - alias :proxy_target :target - - # Sets the target of this proxy to <tt>\target</tt>, and the \loaded flag to +true+. - def target=(target) - @target = target - loaded! - end - - # Forwards the call to the target. Loads the \target if needed. - def inspect - load_target.inspect - end - - def send(method, *args) - return super if respond_to?(method) - load_target.send(method, *args) - end - - def scoped - target_scope & @association_scope - end - - protected - - # Construct the scope for this association. - # - # Note that the association_scope is merged into the targed_scope only when the - # scoped method is called. This is because at that point the call may be surrounded - # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which - # actually gets built. - def construct_scope - @association_scope = association_scope if target_klass - end - - def association_scope - scope = target_klass.unscoped - scope = scope.create_with(creation_attributes) - scope = scope.apply_finder_options(@reflection.options.slice(:conditions, :readonly, :include)) - if select = select_value - scope = scope.select(select) - end - scope = scope.extending(*Array.wrap(@reflection.options[:extend])) - scope.where(construct_owner_conditions) - end - - def aliased_table - target_klass.arel_table - end - - # Set the inverse association, if possible - def set_inverse_instance(record) - if record && invertible_for?(record) - inverse = record.send(:association_proxy, inverse_reflection_for(record).name) - inverse.target = @owner - end - end - - # This class of the target. belongs_to polymorphic overrides this to look at the - # polymorphic_type field on the owner. - def target_klass - @reflection.klass - end - - # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the - # through association's scope) - def target_scope - target_klass.scoped - end - - # Loads the \target if needed and returns it. - # - # This method is abstract in the sense that it relies on +find_target+, - # which is expected to be provided by descendants. - # - # If the \target is already \loaded it is just returned. Thus, you can call - # +load_target+ unconditionally to get the \target. - # - # ActiveRecord::RecordNotFound is rescued within the method, and it is - # not reraised. The proxy is \reset and +nil+ is the return value. - def load_target - @target = find_target if find_target? - loaded! - target - rescue ActiveRecord::RecordNotFound - reset - end - - private - - def find_target? - !loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass - end - - def interpolate_sql(sql, record = nil) - @owner.send(:interpolate_sql, sql, record) - end - - def select_value - @reflection.options[:select] - end - - # Implemented by (some) subclasses - def creation_attributes - { } - end - - # Returns a hash linking the owner to the association represented by the reflection - def construct_owner_attributes(reflection = @reflection) - attributes = {} - if reflection.macro == :belongs_to - attributes[reflection.association_primary_key] = @owner[reflection.foreign_key] - else - attributes[reflection.foreign_key] = @owner[reflection.active_record_primary_key] - - if reflection.options[:as] - attributes["#{reflection.options[:as]}_type"] = @owner.class.base_class.name - end - end - attributes - end - - # Builds an array of arel nodes from the owner attributes hash - def construct_owner_conditions(table = aliased_table, reflection = @reflection) - conditions = construct_owner_attributes(reflection).map do |attr, value| - table[attr].eq(value) - end - table.create_and(conditions) - end - - # Sets the owner attributes on the given record - def set_owner_attributes(record) - if @owner.persisted? - construct_owner_attributes.each { |key, value| record[key] = value } - end - end - - # Should be true if there is a foreign key present on the @owner which - # references the target. This is used to determine whether we can load - # the target if the @owner is currently a new record (and therefore - # without a key). - # - # Currently implemented by belongs_to (vanilla and polymorphic) and - # has_one/has_many :through associations which go through a belongs_to - def foreign_key_present? - false - end - - # Raises ActiveRecord::AssociationTypeMismatch unless +record+ is of - # the kind of the class of the associated objects. Meant to be used as - # a sanity check when you are about to assign an associated record. - def raise_on_type_mismatch(record) - unless record.is_a?(@reflection.klass) || record.is_a?(@reflection.class_name.constantize) - message = "#{@reflection.class_name}(##{@reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" - raise ActiveRecord::AssociationTypeMismatch, message - end - end - - # Can be redefined by subclasses, notably polymorphic belongs_to - # The record parameter is necessary to support polymorphic inverses as we must check for - # the association in the specific class of the record. - def inverse_reflection_for(record) - @reflection.inverse_of - end - - # Is this association invertible? Can be redefined by subclasses. - def invertible_for?(record) - inverse_reflection_for(record) - end - - # This should be implemented to return the values of the relevant key(s) on the owner, - # so that when state_state is different from the value stored on the last find_target, - # the target is stale. - # - # This is only relevant to certain associations, which is why it returns nil by default. - def stale_state - end - end - end -end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb index fdd4fe8946..b711ff35ca 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb @@ -187,8 +187,8 @@ module ActiveRecord construct(parent, association, join_parts, row) end when Hash - associations.sort_by { |k,_| k.to_s }.each do |name, assoc| - association = construct(parent, name, join_parts, row) + associations.sort_by { |k,_| k.to_s }.each do |association_name, assoc| + association = construct(parent, association_name, join_parts, row) construct(association, assoc, join_parts, row) if association end else @@ -209,10 +209,10 @@ module ActiveRecord association = join_part.instantiate(row) case macro when :has_many, :has_and_belongs_to_many - collection = record.send(join_part.reflection.name) - collection.loaded! - collection.target.push(association) - collection.send(:set_inverse_instance, association) + other = record.association(join_part.reflection.name) + other.loaded! + other.target.push(association) + other.set_inverse_instance(association) when :belongs_to set_target_and_inverse(join_part, association, record) else @@ -223,9 +223,9 @@ module ActiveRecord end def set_target_and_inverse(join_part, association, record) - association_proxy = record.send(:association_proxy, join_part.reflection.name) - association_proxy.target = association - association_proxy.send(:set_inverse_instance, association) + other = record.association(join_part.reflection.name) + other.target = association + other.set_inverse_instance(association) end end end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb index 3fea24ebf8..aaa475109e 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb @@ -82,12 +82,12 @@ module ActiveRecord connection = active_record.connection name = connection.table_alias_for "#{pluralize(reflection.name)}_#{parent_table_name}#{suffix}" - table_index = aliases[name] + 1 - name = name[0, connection.table_alias_length-3] + "_#{table_index}" if table_index > 1 + aliases[name] += 1 + name = name[0, connection.table_alias_length-3] + "_#{aliases[name]}" if aliases[name] > 1 + else + aliases[name] += 1 end - aliases[name] += 1 - name end @@ -108,6 +108,10 @@ module ActiveRecord end def process_conditions(conditions, table_name) + if conditions.respond_to?(:to_proc) + conditions = instance_eval(&conditions) + end + Arel.sql(sanitize_sql(conditions, table_name)) end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/collection_association.rb index 2811f53424..68631681e4 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -17,8 +17,26 @@ module ActiveRecord # # If you need to work on all current children, new and existing records, # +load_target+ and the +loaded+ flag are your friends. - class AssociationCollection < AssociationProxy #:nodoc: - delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped + class CollectionAssociation < Association #:nodoc: + attr_reader :proxy + + def initialize(owner, reflection) + # When scopes are created via method_missing on the proxy, they are stored so that + # any records fetched from the database are kept around for future use. + @scopes_cache = Hash.new do |hash, method| + hash[method] = { } + end + + super + + @proxy = CollectionProxy.new(self) + end + + def reset + @loaded = false + @target = [] + @scopes_cache.clear + end def select(select = nil) if block_given? @@ -44,49 +62,42 @@ module ActiveRecord first_or_last(:last, *args) end - def to_ary - load_target.dup + def build(attributes = {}, &block) + build_or_create(attributes, :build, &block) end - alias_method :to_a, :to_ary - def reset - @_scopes_cache = {} - @loaded = false - @target = [] + def create(attributes = {}, &block) + unless @owner.persisted? + raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" + end + + build_or_create(attributes, :create, &block) end - def build(attributes = {}, &block) - if attributes.is_a?(Array) - attributes.collect { |attr| build(attr, &block) } - else - build_record(attributes) do |record| - block.call(record) if block_given? - set_owner_attributes(record) - end - end + def create!(attrs = {}, &block) + record = create(attrs, &block) + Array.wrap(record).each(&:save!) + record 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) + def concat(*records) result = true load_target if @owner.new_record? transaction do records.flatten.each do |record| raise_on_type_mismatch(record) - add_record_to_target_with_callbacks(record) do |r| + add_to_target(record) do |r| result &&= insert_record(record) unless @owner.new_record? end end end - result && self + result && records end - alias_method :push, :<< - alias_method :concat, :<< - # Starts a transaction in the association class's database connection. # # class Author < ActiveRecord::Base @@ -112,13 +123,6 @@ module ActiveRecord end end - # Identical to delete_all, except that the return value is the association (for chaining) - # rather than the records which have been removed. - def clear - delete_all - self - end - # Destroy all the records from this association. # # See destroy for more info. @@ -178,10 +182,7 @@ module ActiveRecord # are actually removed from the database, that depends precisely on # +delete_records+. They are in any case removed from the collection. def delete(*records) - remove_records(records) do |_records, old_records| - delete_records(old_records) if old_records.any? - _records.each { |record| @target.delete(record) } - end + delete_or_destroy(records, @reflection.options[:dependent]) end # Destroy +records+ and remove them from this association calling @@ -190,30 +191,8 @@ module ActiveRecord # Note that this method will _always_ remove records from the database # ignoring the +:dependent+ option. def destroy(*records) - records = find(records) if records.any? {|record| record.kind_of?(Fixnum) || record.kind_of?(String)} - remove_records(records) do |_records, old_records| - old_records.each { |record| record.destroy } - end - - load_target - end - - def create(attrs = {}) - if attrs.is_a?(Array) - attrs.collect { |attr| create(attr) } - else - create_record(attrs) do |record| - yield(record) if block_given? - insert_record(record, false) - end - end - end - - def create!(attrs = {}) - create_record(attrs) do |record| - yield(record) if block_given? - insert_record(record, true) - end + records = find(records) if records.any? { |record| record.kind_of?(Fixnum) || record.kind_of?(String) } + delete_or_destroy(records, :destroy) end # Returns the size of the collection by executing a SELECT COUNT(*) @@ -272,7 +251,7 @@ module ActiveRecord end end - def uniq(collection = self) + def uniq(collection = load_target) seen = {} collection.find_all do |record| seen[record.id] = true unless seen.key?(record.id) @@ -290,7 +269,7 @@ module ActiveRecord unless concat(other_array - @target) @target = original_target - raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the " + raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the " \ "new records could not be saved." end end @@ -309,66 +288,50 @@ module ActiveRecord end end - def respond_to?(method, include_private = false) - super || @reflection.klass.respond_to?(method, include_private) + def cached_scope(method, args) + @scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args) end - def method_missing(method, *args, &block) - match = DynamicFinderMatch.match(method) - if match && match.creator? - attributes = match.attribute_names - return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)]) - end - - if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method)) - super - elsif @reflection.klass.scopes[method] - @_scopes_cache ||= {} - @_scopes_cache[method] ||= {} - @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args) - else - scoped.readonly(nil).send(method, *args, &block) - end + def association_scope + options = @reflection.options.slice(:order, :limit, :joins, :group, :having, :offset) + super.apply_finder_options(options) end - protected - - def association_scope - options = @reflection.options.slice(:order, :limit, :joins, :group, :having, :offset) - super.apply_finder_options(options) - end - - def load_target - if find_target? - targets = [] - - begin - targets = find_target - rescue ActiveRecord::RecordNotFound - reset - end + def load_target + if find_target? + targets = [] - @target = merge_target_lists(targets, @target) + begin + targets = find_target + rescue ActiveRecord::RecordNotFound + reset end - loaded! - target + @target = merge_target_lists(targets, @target) end - def add_record_to_target_with_callbacks(record) + loaded! + target + end + + def add_to_target(record) + transaction do callback(:before_add, record) yield(record) if block_given? - @target ||= [] unless loaded? + if @reflection.options[:uniq] && index = @target.index(record) @target[index] = record else @target << record end + callback(:after_add, record) set_inverse_instance(record) - record end + record + end + private def select_value @@ -381,17 +344,15 @@ module ActiveRecord def custom_counter_sql if @reflection.options[:counter_sql] - counter_sql = @reflection.options[:counter_sql] + interpolate(@reflection.options[:counter_sql]) else # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ - counter_sql = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } + interpolate(@reflection.options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } end - - interpolate_sql(counter_sql) end def custom_finder_sql - interpolate_sql(@reflection.options[:finder_sql]) + interpolate(@reflection.options[:finder_sql]) end def find_target @@ -428,40 +389,49 @@ module ActiveRecord end + existing end - # Do the relevant stuff to insert the given record into the association collection. The - # force param specifies whether or not an exception should be raised on failure. The - # validate param specifies whether validation should be performed (if force is false). - def insert_record(record, force = true, validate = true) - raise NotImplementedError - end + def build_or_create(attributes, method) + records = Array.wrap(attributes).map do |attrs| + record = build_record(attrs) + + add_to_target(record) do + yield(record) if block_given? + insert_record(record) if method == :create + end + end - def save_record(record, force, validate) - force ? record.save! : record.save(:validate => validate) + attributes.is_a?(Array) ? records : records.first end - def create_record(attributes, &block) - ensure_owner_is_persisted! - transaction { build_record(attributes, &block) } + # Do the relevant stuff to insert the given record into the association collection. + def insert_record(record, validate = true) + raise NotImplementedError end - def build_record(attributes, &block) - attributes = scoped.scope_for_create.merge(attributes) - record = @reflection.build_association(attributes) - add_record_to_target_with_callbacks(record, &block) + def build_record(attributes) + @reflection.build_association(scoped.scope_for_create.merge(attributes)) end - def remove_records(*records) + def delete_or_destroy(records, method) records = records.flatten records.each { |record| raise_on_type_mismatch(record) } + existing_records = records.reject { |r| r.new_record? } transaction do records.each { |record| callback(:before_remove, record) } - old_records = records.reject { |r| r.new_record? } - yield(records, old_records) + + delete_records(existing_records, method) if existing_records.any? + records.each { |record| @target.delete(record) } + records.each { |record| callback(:after_remove, record) } end end + # Delete the given records from the association, using one of the methods :destroy, + # :delete_all or :nullify (or nil, in which case a default is used). + def delete_records(records, method) + raise NotImplementedError + end + def callback(method, record) callbacks_for(method).each do |callback| case callback @@ -480,12 +450,6 @@ module ActiveRecord @owner.class.send(full_callback_name.to_sym) || [] end - def ensure_owner_is_persisted! - unless @owner.persisted? - raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" - end - end - # Should we deal with assoc.first or assoc.last by issuing an independent query to # the database, or by getting the target, and then taking the first/last item from that? # @@ -511,8 +475,8 @@ module ActiveRecord def include_in_memory?(record) if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) - @owner.send(proxy_reflection.through_reflection.name).any? { |source| - target = source.send(proxy_reflection.source_reflection.name) + @owner.send(@reflection.through_reflection.name).any? { |source| + target = source.send(@reflection.source_reflection.name) target.respond_to?(:include?) ? target.include?(record) : target == record } || @target.include?(record) else diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb new file mode 100644 index 0000000000..cf77d770c9 --- /dev/null +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -0,0 +1,127 @@ +module ActiveRecord + module Associations + # Association proxies in Active Record are middlemen between the object that + # holds the association, known as the <tt>@owner</tt>, and the actual associated + # object, known as the <tt>@target</tt>. The kind of association any proxy is + # about is available in <tt>@reflection</tt>. That's an instance of the class + # ActiveRecord::Reflection::AssociationReflection. + # + # For example, given + # + # class Blog < ActiveRecord::Base + # has_many :posts + # end + # + # blog = Blog.find(:first) + # + # the association proxy in <tt>blog.posts</tt> has the object in +blog+ as + # <tt>@owner</tt>, the collection of its posts as <tt>@target</tt>, and + # the <tt>@reflection</tt> object represents a <tt>:has_many</tt> macro. + # + # This class has most of the basic instance methods removed, and delegates + # unknown methods to <tt>@target</tt> via <tt>method_missing</tt>. As a + # corner case, it even removes the +class+ method and that's why you get + # + # blog.posts.class # => Array + # + # though the object behind <tt>blog.posts</tt> is not an Array, but an + # ActiveRecord::Associations::HasManyAssociation. + # + # The <tt>@target</tt> object is not \loaded until needed. For example, + # + # blog.posts.count + # + # is computed directly through SQL and does not trigger by itself the + # instantiation of the actual post records. + class CollectionProxy # :nodoc: + alias :proxy_extend :extend + + instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id|to_a)$|^__|^respond_to|proxy_/ } + + delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, + :lock, :readonly, :having, :to => :scoped + + delegate :target, :load_target, :loaded?, :scoped, + :to => :@association + + delegate :select, :find, :first, :last, + :build, :create, :create!, + :concat, :delete_all, :destroy_all, :delete, :destroy, :uniq, + :sum, :count, :size, :length, :empty?, + :any?, :many?, :include?, + :to => :@association + + def initialize(association) + @association = association + Array.wrap(association.options[:extend]).each { |ext| proxy_extend(ext) } + end + + def respond_to?(*args) + super || + (load_target && target.respond_to?(*args)) || + @association.klass.respond_to?(*args) + end + + def method_missing(method, *args, &block) + match = DynamicFinderMatch.match(method) + if match && match.creator? + attributes = match.attribute_names + return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)]) + end + + if target.respond_to?(method) || (!@association.klass.respond_to?(method) && Class.respond_to?(method)) + if load_target + if target.respond_to?(method) + target.send(method, *args, &block) + else + begin + super + rescue NoMethodError => e + raise e, e.message.sub(/ for #<.*$/, " via proxy for #{target}") + end + end + end + + elsif @association.klass.scopes[method] + @association.cached_scope(method, args) + else + scoped.readonly(nil).send(method, *args, &block) + end + end + + # Forwards <tt>===</tt> explicitly to the \target because the instance method + # removal above doesn't catch it. Loads the \target if needed. + def ===(other) + other === load_target + end + + def to_ary + load_target.dup + end + alias_method :to_a, :to_ary + + def <<(*records) + @association.concat(records) && self + end + alias_method :push, :<< + + def clear + delete_all + self + end + + def reload + @association.reload + self + end + + def new(*args, &block) + if @association.is_a?(HasManyThroughAssociation) + @association.build(*args, &block) + else + method_missing(:new, *args, &block) + end + end + end + end +end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 3329a4af8e..bcaea5ded4 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -1,7 +1,7 @@ module ActiveRecord # = Active Record Has And Belongs To Many Association module Associations - class HasAndBelongsToManyAssociation < AssociationCollection #:nodoc: + class HasAndBelongsToManyAssociation < CollectionAssociation #:nodoc: attr_reader :join_table def initialize(owner, reflection) @@ -9,30 +9,26 @@ module ActiveRecord super end - protected + def insert_record(record, validate = true) + return if record.new_record? && !record.save(:validate => validate) - def insert_record(record, force = true, validate = true) - if record.new_record? - return false unless save_record(record, force, validate) - end - - if @reflection.options[:insert_sql] - @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record)) - else - stmt = join_table.compile_insert( - join_table[@reflection.foreign_key] => @owner.id, - join_table[@reflection.association_foreign_key] => record.id - ) - - @owner.connection.insert stmt.to_sql - end + if @reflection.options[:insert_sql] + @owner.connection.insert(interpolate(@reflection.options[:insert_sql], record)) + else + stmt = join_table.compile_insert( + join_table[@reflection.foreign_key] => @owner.id, + join_table[@reflection.association_foreign_key] => record.id + ) - true + @owner.connection.insert stmt.to_sql end - def association_scope - super.joins(construct_joins) - end + record + end + + def association_scope + super.joins(construct_joins) + end private @@ -40,9 +36,9 @@ module ActiveRecord load_target.size end - def delete_records(records) + def delete_records(records, method) if sql = @reflection.options[:delete_sql] - records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) } + records.each { |record| @owner.connection.delete(interpolate(sql, record)) } else relation = join_table stmt = relation.where(relation[@reflection.foreign_key].eq(@owner.id). diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index caefd14ee3..91565b247a 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -5,13 +5,11 @@ module ActiveRecord # # If the association has a <tt>:through</tt> option further specialization # is provided by its child HasManyThroughAssociation. - class HasManyAssociation < AssociationCollection #:nodoc: - protected - - def insert_record(record, force = false, validate = true) - set_owner_attributes(record) - save_record(record, force, validate) - end + class HasManyAssociation < CollectionAssociation #:nodoc: + def insert_record(record, validate = true) + set_owner_attributes(record) + record.save(:validate => validate) + end private @@ -45,30 +43,54 @@ module ActiveRecord [@reflection.options[:limit], count].compact.min end - def has_cached_counter? - @owner.attribute_present?(cached_counter_attribute_name) + def has_cached_counter?(reflection = @reflection) + @owner.attribute_present?(cached_counter_attribute_name(reflection)) end - def cached_counter_attribute_name - "#{@reflection.name}_count" + def cached_counter_attribute_name(reflection = @reflection) + "#{reflection.name}_count" end - # Deletes the records according to the <tt>:dependent</tt> option. - def delete_records(records) - case @reflection.options[:dependent] - when :destroy - records.each { |r| r.destroy } - when :delete_all - @reflection.klass.delete(records.map { |r| r.id }) - else - updates = { @reflection.foreign_key => nil } - conditions = { @reflection.association_primary_key => records.map { |r| r.id } } - - scoped.where(conditions).update_all(updates) + def update_counter(difference, reflection = @reflection) + if has_cached_counter?(reflection) + counter = cached_counter_attribute_name(reflection) + @owner.class.update_counters(@owner.id, counter => difference) + @owner[counter] += difference + @owner.changed_attributes.delete(counter) # eww end + end + + # This shit is nasty. We need to avoid the following situation: + # + # * An associated record is deleted via record.destroy + # * Hence the callbacks run, and they find a belongs_to on the record with a + # :counter_cache options which points back at our @owner. So they update the + # counter cache. + # * In which case, we must make sure to *not* update the counter cache, or else + # it will be decremented twice. + # + # Hence this method. + def inverse_updates_counter_cache?(reflection = @reflection) + counter_name = cached_counter_attribute_name(reflection) + reflection.klass.reflect_on_all_associations(:belongs_to).any? { |inverse_reflection| + inverse_reflection.counter_cache_column == counter_name + } + end - if has_cached_counter? && @reflection.options[:dependent] != :destroy - @owner.class.update_counters(@owner.id, cached_counter_attribute_name => -records.size) + # Deletes the records according to the <tt>:dependent</tt> option. + def delete_records(records, method) + if method == :destroy + records.each { |r| r.destroy } + update_counter(-records.length) unless inverse_updates_counter_cache? + else + keys = records.map { |r| r[@reflection.association_primary_key] } + scope = scoped.where(@reflection.association_primary_key => keys) + + if method == :delete_all + update_counter(-scope.delete_all) + else + update_counter(-scope.update_all(@reflection.foreign_key => nil)) + end end end 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 d5b901beff..664c284d45 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -8,13 +8,6 @@ module ActiveRecord alias_method :new, :build - def destroy(*records) - transaction do - delete_records(records.flatten) - super - 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 fewer @@ -29,18 +22,56 @@ module ActiveRecord end end - protected + def concat(*records) + unless @owner.new_record? + records.flatten.each do |record| + raise_on_type_mismatch(record) + record.save! if record.new_record? + end + end + + super + end + + def insert_record(record, validate = true) + return if record.new_record? && !record.save(:validate => validate) + through_record(record).save! + update_counter(1) + record + end - def insert_record(record, force = true, validate = true) - if record.new_record? - return false unless save_record(record, force, validate) + private + + def through_record(record) + through_association = @owner.association(@reflection.through_reflection.name) + attributes = construct_join_attributes(record) + + through_record = Array.wrap(through_association.target).find { |candidate| + candidate.attributes.slice(*attributes.keys) == attributes + } + + unless through_record + through_record = through_association.build(attributes) + through_record.send("#{@reflection.source_reflection.name}=", record) end - through_association = @owner.send(@reflection.through_reflection.name) - through_association.create!(construct_join_attributes(record)) + through_record end - private + def build_record(attributes) + record = super(attributes) + + inverse = @reflection.source_reflection.inverse_of + if inverse + if inverse.macro == :has_many + record.send(inverse.name) << through_record(record) + elsif inverse.macro == :has_one + record.send("#{inverse.name}=", through_record(record)) + end + end + + record + end def target_reflection_has_associated_record? if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank? @@ -50,11 +81,48 @@ module ActiveRecord end end - # TODO - add dependent option support - def delete_records(records) - through_association = @owner.send(@reflection.through_reflection.name) - records.each do |associate| - through_association.where(construct_join_attributes(associate)).delete_all + def update_through_counter?(method) + case method + when :destroy + !inverse_updates_counter_cache?(@reflection.through_reflection) + when :nullify + false + else + true + end + end + + def delete_records(records, method) + through = @owner.association(@reflection.through_reflection.name) + scope = through.scoped.where(construct_join_attributes(*records)) + + case method + when :destroy + count = scope.destroy_all.length + when :nullify + count = scope.update_all(@reflection.source_reflection.foreign_key => nil) + else + count = scope.delete_all + end + + delete_through_records(through, records) + + if @reflection.through_reflection.macro == :has_many && update_through_counter?(method) + update_counter(-count, @reflection.through_reflection) + end + + update_counter(-count) + end + + def delete_through_records(through, records) + if @reflection.through_reflection.macro == :has_many + records.each do |record| + through.target.delete(through_record(record)) + end + else + records.each do |record| + through.target = nil if through.target == through_record(record) + end end 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 69771afe50..112b773ec4 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -1,7 +1,7 @@ module ActiveRecord # = Active Record Has One Through Association module Associations - class HasOneThroughAssociation < HasOneAssociation + class HasOneThroughAssociation < HasOneAssociation #:nodoc: include ThroughAssociation def replace(record) @@ -12,7 +12,7 @@ module ActiveRecord private def create_through_record(record) - through_proxy = @owner.send(:association_proxy, @reflection.through_reflection.name) + through_proxy = @owner.association(@reflection.through_reflection.name) through_record = through_proxy.send(:load_target) if through_record && !record diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 7f92d9712a..0aa647c63d 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -1,6 +1,6 @@ module ActiveRecord module Associations - class SingularAssociation < AssociationProxy #:nodoc: + class SingularAssociation < Association #:nodoc: def create(attributes = {}) new_record(:create, attributes) end @@ -29,7 +29,7 @@ module ActiveRecord end def check_record(record) - record = record.target if AssociationProxy === record + record = record.target if Association === record raise_on_type_mismatch(record) if record record end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index c840a16160..8db8068295 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -1,12 +1,12 @@ module ActiveRecord # = Active Record Through Association module Associations - module ThroughAssociation + module ThroughAssociation #:nodoc: protected def target_scope - super & @reflection.through_reflection.klass.scoped + super.merge(@reflection.through_reflection.klass.scoped) end def association_scope @@ -74,21 +74,40 @@ module ActiveRecord right.create_on(right.create_and(conditions))) end - # Construct attributes for :through pointing to owner and associate. - def construct_join_attributes(associate) - # 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) + # Construct attributes for :through pointing to owner and associate. This is used by the + # methods which create and delete records on the association. + # + # We only support indirectly modifying through associations which has a belongs_to source. + # This is the "has_many :tags, :through => :taggings" situation, where the join model + # typically has a belongs_to on both side. In other words, associations which could also + # be represented as has_and_belongs_to_many associations. + # + # We do not support creating/deleting records on the association where the source has + # some other type, because this opens up a whole can of worms, and in basically any + # 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 @reflection.source_reflection.macro != :belongs_to + raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) + end join_attributes = { @reflection.source_reflection.foreign_key => - associate.send(@reflection.source_reflection.association_primary_key) + records.map { |record| + record.send(@reflection.source_reflection.association_primary_key) + } } if @reflection.options[:source_type] - join_attributes.merge!(@reflection.source_reflection.foreign_type => associate.class.base_class.name) + join_attributes[@reflection.source_reflection.foreign_type] = + records.map { |record| record.class.base_class.name } end - join_attributes + if records.count == 1 + Hash[join_attributes.map { |k, v| [k, v.first] }] + else + join_attributes + end end # The reason that we are operating directly on the scope here (rather than passing @@ -100,14 +119,14 @@ module ActiveRecord scope = scope.where(@reflection.through_reflection.klass.send(:type_condition)) end - scope = scope.where(@reflection.source_reflection.options[:conditions]) + scope = scope.where(interpolate(@reflection.source_reflection.options[:conditions])) scope.where(through_conditions) end # If there is a hash of conditions then we make sure the keys are scoped to the # through table name if left ambiguous. def through_conditions - conditions = @reflection.through_reflection.options[:conditions] + conditions = interpolate(@reflection.through_reflection.options[:conditions]) if conditions.is_a?(Hash) Hash[conditions.map { |key, value| diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 4f4a0a5fee..2c5db51f7f 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -10,7 +10,7 @@ module ActiveRecord # Generates all the attribute related methods for columns in the database # accessors, mutators and query methods. def define_attribute_methods - super(columns_hash.keys) + super(column_names) end # Checks whether the method is defined in the model or any of its subclasses diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index c19a33faa8..3eff3d54e3 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -22,6 +22,8 @@ module ActiveRecord if status = super @previously_changed = changes @changed_attributes.clear + elsif IdentityMap.enabled? + IdentityMap.remove(self) end status end @@ -32,6 +34,9 @@ module ActiveRecord @previously_changed = changes @changed_attributes.clear end + rescue + IdentityMap.remove(self) if IdentityMap.enabled? + raise end # <tt>reload</tt> the record and clears changed attributes. diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 978cd7fbe3..fcdd31ddea 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -56,6 +56,7 @@ module ActiveRecord @primary_key ||= '' self.original_primary_key = @primary_key value &&= value.to_s + connection_pool.primary_keys[table_name] = value self.primary_key = block_given? ? instance_eval(&block) : value end end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index a72eecb50e..76218d2a73 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -41,7 +41,7 @@ module ActiveRecord if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body, line = <<-EOV, __LINE__ + 1 def #{attr_name}=(original_time) - time = original_time.dup + time = original_time.dup unless original_time.nil? unless time.acts_like?(:time) time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 9c7bb67479..476598bf88 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -140,6 +140,23 @@ module ActiveRecord CODE end + def define_non_cyclic_method(name, reflection, &block) + define_method(name) do |*args| + result = true; @_already_called ||= {} + # Loop prevention for validation of associations + unless @_already_called[[name, reflection.name]] + begin + @_already_called[[name, reflection.name]]=true + result = instance_eval(&block) + ensure + @_already_called[[name, reflection.name]]=false + end + end + + result + end + end + # Adds validation and save callbacks for the association as specified by # the +reflection+. # @@ -160,7 +177,7 @@ module ActiveRecord if collection before_save :before_save_collection_association - define_method(save_method) { save_collection_association(reflection) } + define_non_cyclic_method(save_method, reflection) { save_collection_association(reflection) } # Doesn't use after_save as that would save associations added in after_create/after_update twice after_create save_method after_update save_method @@ -178,7 +195,7 @@ module ActiveRecord after_create save_method after_update save_method else - define_method(save_method) { save_belongs_to_association(reflection) } + define_non_cyclic_method(save_method, reflection) { save_belongs_to_association(reflection) } before_save save_method end end @@ -186,7 +203,7 @@ module ActiveRecord if reflection.validate? && !method_defined?(validation_method) method = (collection ? :validate_collection_association : :validate_single_association) - define_method(validation_method) { send(method, reflection) } + define_non_cyclic_method(validation_method, reflection) { send(method, reflection) } validate validation_method end end @@ -227,7 +244,7 @@ module ActiveRecord # unless the parent is/was a new record itself. def associated_records_to_validate_or_save(association, new_record, autosave) if new_record - association + association && association.target elsif autosave association.target.find_all { |record| record.changed_for_autosave? } else @@ -247,9 +264,9 @@ module ActiveRecord # Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is # turned on for the association. def validate_single_association(reflection) - if (association = association_instance_get(reflection.name)) && !association.target.nil? - association_valid?(reflection, association) - end + association = association_instance_get(reflection.name) + record = association && association.target + association_valid?(reflection, record) if record end # Validate the associated records if <tt>:validate</tt> or @@ -266,12 +283,12 @@ module ActiveRecord # Returns whether or not the association is valid and applies any errors to # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt> # enabled records if they're marked_for_destruction? or destroyed. - def association_valid?(reflection, association) - return true if association.destroyed? || association.marked_for_destruction? + def association_valid?(reflection, record) + return true if record.destroyed? || record.marked_for_destruction? - unless valid = association.valid? + unless valid = record.valid? if reflection.options[:autosave] - association.errors.each do |attribute, message| + record.errors.each do |attribute, message| attribute = "#{reflection.name}.#{attribute}" errors[attribute] << message errors[attribute].uniq! @@ -303,23 +320,31 @@ module ActiveRecord autosave = reflection.options[:autosave] if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) + begin records.each do |record| next if record.destroyed? + saved = true + if autosave && record.marked_for_destruction? - association.destroy(record) + association.proxy.destroy(record) elsif autosave != false && (@new_record_before_save || record.new_record?) if autosave - saved = association.send(:insert_record, record, false, false) + saved = association.insert_record(record, false) else - association.send(:insert_record, record) + association.insert_record(record) end elsif autosave saved = record.save(:validate => false) end - raise ActiveRecord::Rollback if saved == false + raise ActiveRecord::Rollback unless saved end + rescue + records.each {|x| IdentityMap.remove(x) } if IdentityMap.enabled? + raise + end + end # reconstruct the scope now that we know the owner's id @@ -336,16 +361,18 @@ module ActiveRecord # This all happens inside a transaction, _if_ the Transactions module is included into # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. def save_has_one_association(reflection) - if (association = association_instance_get(reflection.name)) && !association.target.nil? && !association.destroyed? + association = association_instance_get(reflection.name) + record = association && association.load_target + if record && !record.destroyed? autosave = reflection.options[:autosave] - if autosave && association.marked_for_destruction? - association.destroy + if autosave && record.marked_for_destruction? + record.destroy else key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id - if autosave != false && (new_record? || association.new_record? || association[reflection.foreign_key] != key || autosave) - association[reflection.foreign_key] = key - saved = association.save(:validate => !autosave) + if autosave != false && (new_record? || record.new_record? || record[reflection.foreign_key] != key || autosave) + record[reflection.foreign_key] = key + saved = record.save(:validate => !autosave) raise ActiveRecord::Rollback if !saved && autosave saved end @@ -357,16 +384,18 @@ module ActiveRecord # # In addition, it will destroy the association if it was marked for destruction. def save_belongs_to_association(reflection) - if (association = association_instance_get(reflection.name)) && !association.destroyed? + association = association_instance_get(reflection.name) + record = association && association.load_target + if record && !record.destroyed? autosave = reflection.options[:autosave] - if autosave && association.marked_for_destruction? - association.destroy + if autosave && record.marked_for_destruction? + record.destroy elsif autosave != false - saved = association.save(:validate => !autosave) if association.new_record? || (autosave && association.changed_for_autosave?) + saved = record.save(:validate => !autosave) if record.new_record? || (autosave && record.changed_for_autosave?) if association.updated? - association_id = association.send(reflection.options[:primary_key] || :id) + association_id = record.send(reflection.options[:primary_key] || :id) self[reflection.foreign_key] = association_id association.loaded! end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index c48ba3114e..eca4be10d6 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -550,7 +550,9 @@ module ActiveRecord #:nodoc: Coders::YAMLColumn.new(class_name) end - serialized_attributes[attr_name.to_s] = coder + # merge new serialized attribute and create new hash to ensure that each class in inheritance hierarchy + # has its own hash of own serialized attributes + self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder) end # Guesses the table name (in forced lower-case) based on the name of the class in the @@ -633,6 +635,9 @@ module ActiveRecord #:nodoc: def set_table_name(value = nil, &block) @quoted_table_name = nil define_attr_method :table_name, value, &block + + @arel_table = Arel::Table.new(table_name, :engine => arel_engine) + @relation = Relation.new(self, arel_table) end alias :table_name= :set_table_name @@ -811,6 +816,10 @@ module ActiveRecord #:nodoc: object.is_a?(self) end + def symbolized_base_class + @symbolized_base_class ||= base_class.to_s.to_sym + end + # Returns the base AR subclass that this class descends from. If A # extends AR::Base, A.base_class will return A. If B descends from A # through some arbitrarily deep hierarchy, B.base_class will return A. @@ -905,10 +914,25 @@ module ActiveRecord #:nodoc: # Finder methods must instantiate through this method to work with the # single-table inheritance model that makes it possible to create # objects of different types from the same table. - def instantiate(record) # :nodoc: - model = find_sti_class(record[inheritance_column]).allocate - model.init_with('attributes' => record) - model + def instantiate(record) + sti_class = find_sti_class(record[inheritance_column]) + record_id = sti_class.primary_key && record[sti_class.primary_key] + + if ActiveRecord::IdentityMap.enabled? && record_id + if (column = sti_class.columns_hash[sti_class.primary_key]) && column.number? + record_id = record_id.to_i + end + if instance = IdentityMap.get(sti_class, record_id) + instance.reinit_with('attributes' => record) + else + instance = sti_class.allocate.init_with('attributes' => record) + IdentityMap.add(instance) + end + else + instance = sti_class.allocate.init_with('attributes' => record) + end + + instance end private @@ -1459,6 +1483,8 @@ MSG @new_record = false run_callbacks :find run_callbacks :initialize + + self end # Specifies how the record is dumped by +Marshal+. @@ -1782,12 +1808,6 @@ MSG self.class.connection.quote(value, column) end - # Interpolate custom SQL string in instance context. - # Optional record argument is meant for custom insert_sql. - def interpolate_sql(sql, record = nil) - instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__) - end - # Instantiates objects for all attribute classes that needs more than one constructor parameter. This is done # by calling new on the column type or aggregation type (through composed_of) object with these parameters. # So having the pairs written_on(1) = "2004", written_on(2) = "6", written_on(3) = "24", will instantiate @@ -1931,6 +1951,7 @@ MSG include ActiveModel::MassAssignmentSecurity include Callbacks, ActiveModel::Observing, Timestamp include Associations, AssociationPreload, NamedScope + include IdentityMap include ActiveModel::SecurePassword # AutosaveAssociation needs to be included before Transactions, because we want diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 4475019e0e..4297c26413 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -59,7 +59,7 @@ module ActiveRecord class ConnectionPool attr_accessor :automatic_reconnect attr_reader :spec, :connections - attr_reader :columns, :columns_hash, :primary_keys + attr_reader :columns, :columns_hash, :primary_keys, :tables # Creates a new ConnectionPool object. +spec+ is a ConnectionSpecification # object which describes database connection information (e.g. adapter, @@ -84,12 +84,7 @@ module ActiveRecord @connections = [] @checked_out = [] @automatic_reconnect = true - - @tables = Hash.new do |h, table_name| - with_connection do |conn| - h[table_name] = conn.table_exists?(table_name) - end - end + @tables = {} @columns = Hash.new do |h, table_name| h[table_name] = with_connection do |conn| @@ -113,21 +108,30 @@ module ActiveRecord @primary_keys = Hash.new do |h, table_name| h[table_name] = with_connection do |conn| - @tables[table_name] ? conn.primary_key(table_name) : 'id' + table_exists?(table_name) ? conn.primary_key(table_name) : 'id' end end end + # A cached lookup for table existence + def table_exists?(name) + return true if @tables.key? name + + with_connection do |conn| + conn.tables.each { |table| @tables[table] = true } + end + + @tables.key? name + end + # Clears out internal caches: # # * columns # * columns_hash - # * primary_keys # * tables def clear_cache! @columns.clear @columns_hash.clear - @primary_keys.clear @tables.clear end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 01e53b46c8..5c1ce173c8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -261,7 +261,15 @@ module ActiveRecord # Inserts the given fixture into the table. Overridden in adapters that require # something beyond a simple insert (eg. Oracle). def insert_fixture(fixture, table_name) - execute "INSERT INTO #{quote_table_name(table_name)} (#{fixture.key_list}) VALUES (#{fixture.value_list})", 'Fixture Insert' + columns = Hash[columns(table_name).map { |c| [c.name, c] }] + + key_list = [] + value_list = fixture.map do |name, value| + key_list << quote_column_name(name) + quote(value, columns[name]) + end + + execute "INSERT INTO #{quote_table_name(table_name)} (#{key_list.join(', ')}) VALUES (#{value_list.join(', ')})", 'Fixture Insert' end def empty_insert_statement_value @@ -276,6 +284,25 @@ module ActiveRecord "WHERE #{quoted_primary_key} IN (SELECT #{quoted_primary_key} FROM #{quoted_table_name} #{where_sql})" end + # Sanitizes the given LIMIT parameter in order to prevent SQL injection. + # + # The +limit+ may be anything that can evaluate to a string via #to_s. It + # should look like an integer, or a comma-delimited list of integers, or + # an Arel SQL literal. + # + # Returns Integer and Arel::Nodes::SqlLiteral limits as is. + # Returns the sanitized limit parameter, either as an integer, or as a + # string which contains a comma-delimited list of integers. + def sanitize_limit(limit) + if limit.is_a?(Integer) || limit.is_a?(Arel::Nodes::SqlLiteral) + limit + elsif limit.to_s =~ /,/ + Arel.sql limit.to_s.split(',').map{ |i| Integer(i) }.join(',') + else + Integer(limit) + end + end + protected # Returns an array of record hashes with the column names as keys and # column values as values. @@ -299,21 +326,6 @@ module ActiveRecord update_sql(sql, name) end - # Sanitizes the given LIMIT parameter in order to prevent SQL injection. - # - # +limit+ may be anything that can evaluate to a string via #to_s. It - # should look like an integer, or a comma-delimited list of integers. - # - # Returns the sanitized limit parameter, either as an integer, or as a - # string which contains a comma-delimited list of integers. - def sanitize_limit(limit) - if limit.to_s =~ /,/ - limit.to_s.split(',').map{ |i| i.to_i }.join(',') - else - limit.to_i - end - end - # Send a rollback message to all records after they have been rolled back. If rollback # is false, only rollback records since the last save point. def rollback_transaction_records(rollback) #:nodoc diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 3a3a73fc42..0f44baa2fe 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -209,11 +209,13 @@ module ActiveRecord protected - def log(sql, name = "SQL") - @instrumenter.instrument("sql.active_record", - :sql => sql, :name => name, :connection_id => object_id) do - yield - end + def log(sql, name = "SQL", binds = []) + @instrumenter.instrument( + "sql.active_record", + :sql => sql, + :name => name, + :connection_id => object_id, + :binds => binds) { yield } rescue Exception => e message = "#{e.class.name}: #{e.message}: #{sql}" @logger.debug message if @logger diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index acf1832938..7bad511c64 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -197,10 +197,7 @@ module ActiveRecord def active? return false unless @connection - @connection.query 'select 1' - true - rescue Mysql2::Error - false + @connection.ping end def reconnect! @@ -418,7 +415,7 @@ module ActiveRecord def columns(table_name, name = nil) sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}" columns = [] - result = execute(sql, :skip_logging) + result = execute(sql) result.each(:symbolize_keys => true, :as => :hash) { |field| columns << Mysql2Column.new(field[:Field], field[:Default], field[:Type], field[:Null] == "YES") } diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index cdf1ebfee4..368c5b2023 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -331,7 +331,7 @@ module ActiveRecord end def exec_query(sql, name = 'SQL', binds = []) - log(sql, name) do + log(sql, name, binds) do result = nil cache = {} diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 46c0f3fafe..576450bc3a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -532,7 +532,7 @@ module ActiveRecord def exec_query(sql, name = 'SQL', binds = []) return exec_no_cache(sql, name) if binds.empty? - log(sql, name) do + log(sql, name, binds) do unless @statements.key? sql nextkey = "a#{@statements.length + 1}" @connection.prepare nextkey, sql @@ -845,14 +845,18 @@ module ActiveRecord # Adds a new column to the named table. # See TableDefinition#column for details of the options you can use. def add_column(table_name, column_name, type, options = {}) - default = options[:default] - notnull = options[:null] == false + add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" + add_column_options!(add_column_sql, options) - # Add the column. - execute("ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}") + begin + execute add_column_sql + rescue ActiveRecord::StatementInvalid => e + raise e if postgresql_version > 80000 - change_column_default(table_name, column_name, default) if options_include_default?(options) - change_column_null(table_name, column_name, false, default) if notnull + execute("ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}") + change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) + change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + end end # Changes the column of a table. diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index f650a1bc74..9ee6b88ab6 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -148,7 +148,7 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== def exec_query(sql, name = nil, binds = []) - log(sql, name) do + log(sql, name, binds) do # Don't cache statements without bind values if binds.empty? diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 8180bf0987..7839f03848 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -74,6 +74,8 @@ module ActiveRecord "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{value.abs}" end + IdentityMap.remove_by_id(symbolized_base_class, id) if IdentityMap.enabled? + update_all(updates.join(', '), primary_key => id ) end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 216c691833..d523c643ba 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -12,15 +12,7 @@ require 'active_support/dependencies' require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/logger' - -if RUBY_VERSION < '1.9' - module YAML #:nodoc: - class Omap #:nodoc: - def keys; map { |k, v| k } end - def values; map { |k, v| v } end - end - end -end +require 'active_support/ordered_hash' if defined? ActiveRecord class FixtureClassNotFound < ActiveRecord::ActiveRecordError #:nodoc: @@ -452,20 +444,23 @@ class FixturesFileNotFound < StandardError; end # # Any fixture labeled "DEFAULTS" is safely ignored. -class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) +class Fixtures MAX_ID = 2 ** 30 - 1 - DEFAULT_FILTER_RE = /\.ya?ml$/ - @@all_cached_fixtures = {} + @@all_cached_fixtures = Hash.new { |h,k| h[k] = {} } - def self.reset_cache(connection = nil) - connection ||= ActiveRecord::Base.connection - @@all_cached_fixtures[connection.object_id] = {} + def self.find_table_name(table_name) # :nodoc: + ActiveRecord::Base.pluralize_table_names ? + table_name.to_s.singularize.camelize : + table_name.to_s.camelize + end + + def self.reset_cache + @@all_cached_fixtures.clear end def self.cache_for_connection(connection) - @@all_cached_fixtures[connection.object_id] ||= {} - @@all_cached_fixtures[connection.object_id] + @@all_cached_fixtures[connection] end def self.fixture_is_cached?(connection, table_name) @@ -474,27 +469,23 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) def self.cached_fixtures(connection, keys_to_fetch = nil) if keys_to_fetch - fixtures = cache_for_connection(connection).values_at(*keys_to_fetch) + cache_for_connection(connection).values_at(*keys_to_fetch) else - fixtures = cache_for_connection(connection).values + cache_for_connection(connection).values end - fixtures.size > 1 ? fixtures : fixtures.first end def self.cache_fixtures(connection, fixtures_map) cache_for_connection(connection).update(fixtures_map) end - def self.instantiate_fixtures(object, table_name, fixtures, load_instances = true) - object.instance_variable_set "@#{table_name.to_s.gsub('.','_')}", fixtures + def self.instantiate_fixtures(object, fixture_name, fixtures, load_instances = true) if load_instances - ActiveRecord::Base.silence do - fixtures.each do |name, fixture| - begin - object.instance_variable_set "@#{name}", fixture.find - rescue FixtureClassNotFound - nil - end + fixtures.each do |name, fixture| + begin + object.instance_variable_set "@#{name}", fixture.find + rescue FixtureClassNotFound + nil end end end @@ -511,36 +502,56 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) def self.create_fixtures(fixtures_directory, table_names, class_names = {}) table_names = [table_names].flatten.map { |n| n.to_s } - table_names.each { |n| class_names[n.tr('/', '_').to_sym] = n.classify if n.include?('/') } - connection = block_given? ? yield : ActiveRecord::Base.connection + table_names.each { |n| + class_names[n.tr('/', '_').to_sym] = n.classify if n.include?('/') + } - table_names_to_fetch = table_names.reject { |table_name| fixture_is_cached?(connection, table_name) } + # FIXME: Apparently JK uses this. + connection = block_given? ? yield : ActiveRecord::Base.connection - unless table_names_to_fetch.empty? - ActiveRecord::Base.silence do - connection.disable_referential_integrity do - fixtures_map = {} + files_to_read = table_names.reject { |table_name| fixture_is_cached?(connection, table_name) } - fixtures = table_names_to_fetch.map do |table_name| - fixtures_map[table_name] = Fixtures.new(connection, table_name.tr('/', '_'), class_names[table_name.tr('/', '_').to_sym], File.join(fixtures_directory, table_name)) - end + unless files_to_read.empty? + connection.disable_referential_integrity do + fixtures_map = {} + + fixture_files = files_to_read.map do |path| + table_name = path.tr '/', '_' + + fixtures_map[path] = Fixtures.new( + connection, + table_name, + class_names[table_name.to_sym], + File.join(fixtures_directory, path)) + end - all_loaded_fixtures.update(fixtures_map) + all_loaded_fixtures.update(fixtures_map) - connection.transaction(:requires_new => true) do - fixtures.reverse.each { |fixture| fixture.delete_existing_fixtures } - fixtures.each { |fixture| fixture.insert_fixtures } + connection.transaction(:requires_new => true) do + fixture_files.each do |ff| + conn = ff.model_class.respond_to?(:connection) ? ff.model_class.connection : connection + table_rows = ff.table_rows - # Cap primary key sequences to max(pk). - if connection.respond_to?(:reset_pk_sequence!) - table_names.each do |table_name| - connection.reset_pk_sequence!(table_name.tr('/', '_')) + table_rows.keys.each do |table| + conn.delete "DELETE FROM #{conn.quote_table_name(table)}", 'Fixture Delete' + end + + table_rows.each do |table_name,rows| + rows.each do |row| + conn.insert_fixture(row, table_name) end end end - cache_fixtures(connection, fixtures_map) + # Cap primary key sequences to max(pk). + if connection.respond_to?(:reset_pk_sequence!) + table_names.each do |table_name| + connection.reset_pk_sequence!(table_name.tr('/', '_')) + end + end end + + cache_fixtures(connection, fixtures_map) end end cached_fixtures(connection, table_names) @@ -552,40 +563,59 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) Zlib.crc32(label.to_s) % MAX_ID end - attr_reader :table_name, :name + attr_reader :table_name, :name, :fixtures, :model_class + + def initialize(connection, table_name, class_name, fixture_path) + @connection = connection + @table_name = table_name + @fixture_path = fixture_path + @name = table_name # preserve fixture base name + @class_name = class_name + + @fixtures = ActiveSupport::OrderedHash.new + @table_name = "#{ActiveRecord::Base.table_name_prefix}#{@table_name}#{ActiveRecord::Base.table_name_suffix}" + + # Should be an AR::Base type class + if class_name.is_a?(Class) + @table_name = class_name.table_name + @connection = class_name.connection + @model_class = class_name + else + @model_class = class_name.constantize rescue nil + end - def initialize(connection, table_name, class_name, fixture_path, file_filter = DEFAULT_FILTER_RE) - @connection, @table_name, @fixture_path, @file_filter = connection, table_name, fixture_path, file_filter - @name = table_name # preserve fixture base name - @class_name = class_name || - (ActiveRecord::Base.pluralize_table_names ? @table_name.singularize.camelize : @table_name.camelize) - @table_name = "#{ActiveRecord::Base.table_name_prefix}#{@table_name}#{ActiveRecord::Base.table_name_suffix}" - @table_name = class_name.table_name if class_name.respond_to?(:table_name) - @connection = class_name.connection if class_name.respond_to?(:connection) read_fixture_files end - def delete_existing_fixtures - @connection.delete "DELETE FROM #{@connection.quote_table_name(table_name)}", 'Fixture Delete' + def [](x) + fixtures[x] + end + + def []=(k,v) + fixtures[k] = v + end + + def each(&block) + fixtures.each(&block) + end + + def size + fixtures.size end - def insert_fixtures + # Return a hash of rows to be inserted. The key is the table, the value is + # a list of rows to insert to that table. + def table_rows now = ActiveRecord::Base.default_timezone == :utc ? Time.now.utc : Time.now now = now.to_s(:db) # allow a standard key to be used for doing defaults in YAML - if is_a?(Hash) - delete('DEFAULTS') - else - delete(assoc('DEFAULTS')) - end + fixtures.delete('DEFAULTS') # track any join tables we need to insert later - habtm_fixtures = Hash.new do |h, habtm| - h[habtm] = HabtmFixtures.new(@connection, habtm.options[:join_table], nil, nil) - end + rows = Hash.new { |h,table| h[table] = [] } - each do |label, fixture| + rows[table_name] = fixtures.map do |label, fixture| row = fixture.to_hash if model_class && model_class < ActiveRecord::Base @@ -631,47 +661,22 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) when :has_and_belongs_to_many if (targets = row.delete(association.name.to_s)) targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/) - join_fixtures = habtm_fixtures[association] - - targets.each do |target| - join_fixtures["#{label}_#{target}"] = Fixture.new( - { association.foreign_key => row[primary_key_name], - association.association_foreign_key => Fixtures.identify(target) }, - nil, @connection) - end + table_name = association.options[:join_table] + rows[table_name].concat targets.map { |target| + { association.foreign_key => row[primary_key_name], + association.association_foreign_key => Fixtures.identify(target) } + } end end end end - @connection.insert_fixture(fixture, @table_name) - end - - # insert any HABTM join tables we discovered - habtm_fixtures.values.each do |fixture| - fixture.delete_existing_fixtures - fixture.insert_fixtures + row end + rows end private - class HabtmFixtures < ::Fixtures #:nodoc: - def read_fixture_files; end - end - - def model_class - unless defined?(@model_class) - @model_class = - if @class_name.nil? || @class_name.is_a?(Class) - @class_name - else - @class_name.constantize rescue nil - end - end - - @model_class - end - def primary_key_name @primary_key_name ||= model_class && model_class.primary_key end @@ -725,7 +730,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) raise Fixture::FormatError, "Bad data for #{@class_name} fixture named #{name} (nil)" end - self[name] = Fixture.new(data, model_class, @connection) + fixtures[name] = Fixture.new(data, model_class) end end end @@ -738,7 +743,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) reader.each do |row| data = {} row.each_with_index { |cell, j| data[header[j].to_s.strip] = cell.to_s.strip } - self["#{@class_name.to_s.underscore}_#{i+=1}"] = Fixture.new(data, model_class, @connection) + fixtures["#{@class_name.to_s.underscore}_#{i+=1}"] = Fixture.new(data, model_class) end end @@ -774,44 +779,30 @@ class Fixture #:nodoc: class FormatError < FixtureError #:nodoc: end - attr_reader :model_class + attr_reader :model_class, :fixture - def initialize(fixture, model_class, connection = ActiveRecord::Base.connection) - @connection = connection - @fixture = fixture - @model_class = model_class.is_a?(Class) ? model_class : model_class.constantize rescue nil + def initialize(fixture, model_class) + @fixture = fixture + @model_class = model_class end def class_name - @model_class.name if @model_class + model_class.name if model_class end def each - @fixture.each { |item| yield item } + fixture.each { |item| yield item } end def [](key) - @fixture[key] - end - - def to_hash - @fixture + fixture[key] end - def key_list - @fixture.keys.map { |column_name| @connection.quote_column_name(column_name) }.join(', ') - end - - def value_list - cols = (model_class && model_class < ActiveRecord::Base) ? model_class.columns_hash : {} - @fixture.map do |key, value| - @connection.quote(value, cols[key]).gsub('[^\]\\n', "\n").gsub('[^\]\\r', "\r") - end.join(', ') - end + alias :to_hash :fixture def find if model_class - model_class.find(self[model_class.primary_key]) + model_class.find(fixture[model_class.primary_key]) else raise FixtureClassNotFound, "No class attached to find." end @@ -838,7 +829,9 @@ module ActiveRecord self.use_instantiated_fixtures = false self.pre_loaded_fixtures = false - self.fixture_class_names = {} + self.fixture_class_names = Hash.new do |h, table_name| + h[table_name] = Fixtures.find_table_name(table_name) + end end module ClassMethods @@ -846,17 +839,17 @@ module ActiveRecord self.fixture_class_names = self.fixture_class_names.merge(class_names) end - def fixtures(*table_names) - if table_names.first == :all - table_names = Dir["#{fixture_path}/**/*.{yml,csv}"] - table_names.map! { |f| f[(fixture_path.size + 1)..-5] } + def fixtures(*fixture_names) + if fixture_names.first == :all + fixture_names = Dir["#{fixture_path}/**/*.{yml,csv}"] + fixture_names.map! { |f| f[(fixture_path.size + 1)..-5] } else - table_names = table_names.flatten.map { |n| n.to_s } + fixture_names = fixture_names.flatten.map { |n| n.to_s } end - self.fixture_table_names |= table_names - require_fixture_classes(table_names) - setup_fixture_accessors(table_names) + self.fixture_table_names |= fixture_names + require_fixture_classes(fixture_names) + setup_fixture_accessors(fixture_names) end def try_to_load_dependency(file_name) @@ -871,38 +864,43 @@ module ActiveRecord end end - def require_fixture_classes(table_names = nil) - (table_names || fixture_table_names).each do |table_name| - file_name = table_name.to_s + def require_fixture_classes(fixture_names = nil) + (fixture_names || fixture_table_names).each do |fixture_name| + file_name = fixture_name.to_s file_name = file_name.singularize if ActiveRecord::Base.pluralize_table_names try_to_load_dependency(file_name) end end - def setup_fixture_accessors(table_names = nil) - table_names = Array.wrap(table_names || fixture_table_names) - table_names.each do |table_name| - table_name = table_name.to_s.tr('./', '_') + def setup_fixture_accessors(fixture_names = nil) + fixture_names = Array.wrap(fixture_names || fixture_table_names) + methods = Module.new do + fixture_names.each do |fixture_name| + fixture_name = fixture_name.to_s.tr('./', '_') - redefine_method(table_name) do |*fixtures| - force_reload = fixtures.pop if fixtures.last == true || fixtures.last == :reload + define_method(fixture_name) do |*fixtures| + force_reload = fixtures.pop if fixtures.last == true || fixtures.last == :reload - @fixture_cache[table_name] ||= {} + @fixture_cache[fixture_name] ||= {} - instances = fixtures.map do |fixture| - @fixture_cache[table_name].delete(fixture) if force_reload + instances = fixtures.map do |fixture| + @fixture_cache[fixture_name].delete(fixture) if force_reload - if @loaded_fixtures[table_name][fixture.to_s] - @fixture_cache[table_name][fixture] ||= @loaded_fixtures[table_name][fixture.to_s].find - else - raise StandardError, "No fixture with name '#{fixture}' found for table '#{table_name}'" + if @loaded_fixtures[fixture_name][fixture.to_s] + ActiveRecord::IdentityMap.without do + @fixture_cache[fixture_name][fixture] ||= @loaded_fixtures[fixture_name][fixture.to_s].find + end + else + raise StandardError, "No fixture with name '#{fixture}' found for table '#{fixture_name}'" + end end - end - instances.size == 1 ? instances.first : instances + instances.size == 1 ? instances.first : instances + end + private fixture_name end - private table_name end + include methods end def uses_transaction(*methods) @@ -922,7 +920,7 @@ module ActiveRecord end def setup_fixtures - return unless defined?(ActiveRecord) && !ActiveRecord::Base.configurations.blank? + return unless !ActiveRecord::Base.configurations.blank? if pre_loaded_fixtures && !use_transactional_fixtures raise RuntimeError, 'pre_loaded_fixtures requires use_transactional_fixtures' @@ -936,7 +934,7 @@ module ActiveRecord if @@already_loaded_fixtures[self.class] @loaded_fixtures = @@already_loaded_fixtures[self.class] else - load_fixtures + @loaded_fixtures = load_fixtures @@already_loaded_fixtures[self.class] = @loaded_fixtures end ActiveRecord::Base.connection.increment_open_transactions @@ -946,7 +944,7 @@ module ActiveRecord else Fixtures.reset_cache @@already_loaded_fixtures[self.class] = nil - load_fixtures + @loaded_fixtures = load_fixtures end # Instantiate fixtures for every test if requested. @@ -970,15 +968,8 @@ module ActiveRecord private def load_fixtures - @loaded_fixtures = {} fixtures = Fixtures.create_fixtures(fixture_path, fixture_table_names, fixture_class_names) - unless fixtures.nil? - if fixtures.instance_of?(Fixtures) - @loaded_fixtures[fixtures.name] = fixtures - else - fixtures.each { |f| @loaded_fixtures[f.name] = f } - end - end + Hash[fixtures.map { |f| [f.name, f] }] end # for pre_loaded_fixtures, only require the classes once. huge speed improvement @@ -994,8 +985,8 @@ module ActiveRecord Fixtures.instantiate_all_loaded_fixtures(self, load_instances?) else raise RuntimeError, 'Load fixtures before instantiating them.' if @loaded_fixtures.nil? - @loaded_fixtures.each do |table_name, fixtures| - Fixtures.instantiate_fixtures(self, table_name, fixtures, load_instances?) + @loaded_fixtures.each do |fixture_name, fixtures| + Fixtures.instantiate_fixtures(self, fixture_name, fixtures, load_instances?) end end end diff --git a/activerecord/lib/active_record/identity_map.rb b/activerecord/lib/active_record/identity_map.rb new file mode 100644 index 0000000000..d18b2b0a54 --- /dev/null +++ b/activerecord/lib/active_record/identity_map.rb @@ -0,0 +1,102 @@ +module ActiveRecord + # = Active Record Identity Map + # + # Ensures that each object gets loaded only once by keeping every loaded + # object in a map. Looks up objects using the map when referring to them. + # + # More information on Identity Map pattern: + # http://www.martinfowler.com/eaaCatalog/identityMap.html + # + # == Configuration + # + # In order to enable IdentityMap, set <tt>config.active_record.identity_map = true</tt> + # in your <tt>config/application.rb</tt> file. + # + # IdentityMap is disabled by default. + # + module IdentityMap + extend ActiveSupport::Concern + + class << self + def enabled=(flag) + Thread.current[:identity_map_enabled] = flag + end + + def enabled + Thread.current[:identity_map_enabled] + end + alias enabled? enabled + + def repository + Thread.current[:identity_map] ||= Hash.new { |h,k| h[k] = {} } + end + + def use + old, self.enabled = enabled, true + + yield if block_given? + ensure + self.enabled = old + clear + end + + def without + old, self.enabled = enabled, false + + yield if block_given? + ensure + self.enabled = old + end + + def get(klass, primary_key) + obj = repository[klass.symbolized_base_class][primary_key] + obj.is_a?(klass) ? obj : nil + end + + def add(record) + repository[record.class.symbolized_base_class][record.id] = record + end + + def remove(record) + repository[record.class.symbolized_base_class].delete(record.id) + end + + def remove_by_id(symbolized_base_class, id) + repository[symbolized_base_class].delete(id) + end + + def clear + repository.clear + end + end + + # Reinitialize an Identity Map model object from +coder+. + # +coder+ must contain the attributes necessary for initializing an empty + # model object. + def reinit_with(coder) + @attributes_cache = {} + dirty = @changed_attributes.keys + @attributes.update(coder['attributes'].except(*dirty)) + @changed_attributes.update(coder['attributes'].slice(*dirty)) + @changed_attributes.delete_if{|k,v| v.eql? @attributes[k]} + + set_serialized_attributes + + run_callbacks :find + + self + end + + class Middleware + def initialize(app) + @app = app + end + + def call(env) + ActiveRecord::IdentityMap.use do + @app.call(env) + end + end + end + end +end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index e5065de7fb..6b2b1ebafe 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -58,6 +58,12 @@ module ActiveRecord end private + def increment_lock + lock_col = self.class.locking_column + previous_lock_value = send(lock_col).to_i + send(lock_col + '=', previous_lock_value + 1) + end + def attributes_from_column_definition result = super @@ -78,8 +84,8 @@ module ActiveRecord return 0 if attribute_names.empty? lock_col = self.class.locking_column - previous_value = send(lock_col).to_i - send(lock_col + '=', previous_value + 1) + previous_lock_value = send(lock_col).to_i + increment_lock attribute_names += [lock_col] attribute_names.uniq! @@ -89,7 +95,7 @@ module ActiveRecord stmt = relation.where( relation.table[self.class.primary_key].eq(quoted_id).and( - relation.table[lock_col].eq(quote_value(previous_value)) + relation.table[lock_col].eq(quote_value(previous_lock_value)) ) ).arel.compile_update(arel_attributes_values(false, false, attribute_names)) @@ -103,7 +109,7 @@ module ActiveRecord # If something went wrong, revert the version. rescue Exception - send(lock_col + '=', previous_value) + send(lock_col + '=', previous_lock_value) raise end end diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb index d900831e13..557b277d6b 100644 --- a/activerecord/lib/active_record/locking/pessimistic.rb +++ b/activerecord/lib/active_record/locking/pessimistic.rb @@ -40,7 +40,7 @@ module ActiveRecord # # Database-specific information on row locking: # MySQL: http://dev.mysql.com/doc/refman/5.1/en/innodb-locking-reads.html - # PostgreSQL: http://www.postgresql.org/docs/8.1/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE + # PostgreSQL: http://www.postgresql.org/docs/current/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE module Pessimistic # Obtain a row lock on this record. Reloads the record to obtain the requested # lock. Pass an SQL locking clause to append the end of the SELECT statement diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index c7ae12977a..afadbf03ef 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -22,8 +22,16 @@ module ActiveRecord self.class.runtime += event.duration return unless logger.debug? - name = '%s (%.1fms)' % [event.payload[:name], event.duration] - sql = event.payload[:sql].squeeze(' ') + payload = event.payload + name = '%s (%.1fms)' % [payload[:name], event.duration] + sql = payload[:sql].squeeze(' ') + binds = nil + + unless (payload[:binds] || []).empty? + binds = " " + payload[:binds].map { |col,v| + [col.name, v] + }.inspect + end if odd? name = color(name, CYAN, true) @@ -32,7 +40,7 @@ module ActiveRecord name = color(name, MAGENTA, true) end - debug " #{name} #{sql}" + debug " #{name} #{sql}#{binds}" end def odd? @@ -45,4 +53,4 @@ module ActiveRecord end end -ActiveRecord::LogSubscriber.attach_to :active_record
\ No newline at end of file +ActiveRecord::LogSubscriber.attach_to :active_record diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 16023defe3..522c0cfc9f 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -387,13 +387,13 @@ module ActiveRecord end end - association = send(association_name) + association = association(association_name) existing_records = if association.loaded? - association.to_a + association.target else attribute_ids = attributes_collection.map {|a| a['id'] || a[:id] }.compact - attribute_ids.empty? ? [] : association.all(:conditions => {association.primary_key => attribute_ids}) + attribute_ids.empty? ? [] : association.scoped.where(association.klass.primary_key => attribute_ids) end attributes_collection.each do |attributes| @@ -403,22 +403,29 @@ module ActiveRecord unless reject_new_record?(association_name, attributes) association.build(attributes.except(*UNASSIGNABLE_KEYS)) end - + elsif existing_records.count == 0 #Existing record but not yet associated + existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id']) + if !call_reject_if(association_name, attributes) + association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) + end elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s } 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 } + target_record = association.target.detect { |record| record == existing_record } if target_record existing_record = target_record else - association.send(:add_record_to_target_with_callbacks, existing_record) + association.add_to_target(existing_record) end - end - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) + end + if !call_reject_if(association_name, attributes) + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) + end else raise_nested_attributes_record_not_found(association_name, attributes['id']) end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index b05957981d..df7b22080c 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -64,7 +64,10 @@ module ActiveRecord # callbacks, Observer methods, or any <tt>:dependent</tt> association # options, use <tt>#destroy</tt>. def delete - self.class.delete(id) if persisted? + if persisted? + self.class.delete(id) + IdentityMap.remove(self) if IdentityMap.enabled? + end @destroyed = true freeze end @@ -73,6 +76,7 @@ module ActiveRecord # that no changes should be made (since they can't be persisted). def destroy if persisted? + IdentityMap.remove(self) if IdentityMap.enabled? self.class.unscoped.where(self.class.arel_table[self.class.primary_key].eq(id)).delete_all end @@ -196,7 +200,12 @@ module ActiveRecord def reload(options = nil) clear_aggregation_cache clear_association_cache - @attributes.update(self.class.unscoped { self.class.find(self.id, options) }.instance_variable_get('@attributes')) + + IdentityMap.without do + fresh_object = self.class.unscoped { self.class.find(self.id, options) } + @attributes.update(fresh_object.instance_variable_get('@attributes')) + end + @attributes_cache = {} self end @@ -224,6 +233,7 @@ module ActiveRecord def touch(name = nil) attributes = timestamp_attributes_for_update_in_model attributes << name if name + unless attributes.empty? current_time = current_time_from_proper_timezone changes = {} @@ -232,6 +242,8 @@ module ActiveRecord changes[column.to_s] = write_attribute(column.to_s, current_time) end + changes[self.class.locking_column] = increment_lock if locking_enabled? + @changed_attributes.except!(*changes.keys) primary_key = self.class.primary_key self.class.update_all(changes, { primary_key => self[primary_key] }) == 1 @@ -272,6 +284,7 @@ module ActiveRecord self.id ||= new_id + IdentityMap.add(self) if IdentityMap.enabled? @new_record = false id end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 72687c9ca3..cace6f0cc0 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -43,6 +43,11 @@ module ActiveRecord ActiveSupport.on_load(:active_record) { self.logger ||= ::Rails.logger } end + initializer "active_record.identity_map" do |app| + config.app_middleware.insert_after "::ActionDispatch::Callbacks", + "ActiveRecord::IdentityMap::Middleware" if config.active_record.delete(:identity_map) + end + initializer "active_record.set_configs" do |app| ActiveSupport.on_load(:active_record) do app.config.active_record.each do |k,v| diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 49d4b8f76b..ff36814684 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -296,8 +296,8 @@ db_namespace = namespace :db do base_dir = ENV['FIXTURES_PATH'] ? File.join(Rails.root, ENV['FIXTURES_PATH']) : File.join(Rails.root, 'test', 'fixtures') fixtures_dir = ENV['FIXTURES_DIR'] ? File.join(base_dir, ENV['FIXTURES_DIR']) : base_dir - (ENV['FIXTURES'] ? ENV['FIXTURES'].split(/,/).map {|f| File.join(fixtures_dir, f) } : Dir["#{fixtures_dir}/**/*.{yml,csv}"]).each do |fixture_file| - Fixtures.create_fixtures(fixtures_dir, fixture_file[(fixtures_dir.size + 1)..-5]) + (ENV['FIXTURES'] ? ENV['FIXTURES'].split(/,/) : Dir["#{fixtures_dir}/**/*.{yml,csv}"].map {|f| f[(fixtures_dir.size + 1)..-5] }).each do |fixture_file| + Fixtures.create_fixtures(fixtures_dir, fixture_file) end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index ceeb0ec39d..4093a1a209 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -313,7 +313,7 @@ module ActiveRecord macro == :belongs_to end - def proxy_class + def association_class case macro when :belongs_to if options[:polymorphic] @@ -394,6 +394,10 @@ module ActiveRecord @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym } end + def association_primary_key + source_reflection.association_primary_key + end + def check_validity! if through_reflection.nil? raise HasManyThroughAssociationNotFoundError.new(active_record.name, self) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 1441e9750e..cb684c1109 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -32,7 +32,14 @@ module ActiveRecord def insert(values) im = arel.compile_insert values im.into @table - primary_key_value = primary_key && Hash === values ? values[table[primary_key]] : nil + + primary_key_value = nil + + if primary_key && Hash === values + primary_key_value = values[values.keys.find { |k| + k.name == primary_key + }] + end @klass.connection.insert( im.to_sql, @@ -74,7 +81,13 @@ module ActiveRecord def to_a return @records if loaded? - @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql, @bind_values) + @records = if @readonly_value.nil? && !@klass.locking_enabled? + eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql, @bind_values) + else + IdentityMap.without do + eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql, @bind_values) + end + end preload = @preload_values preload += @includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 61d9974570..9633fd3d82 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -18,7 +18,10 @@ module ActiveRecord attribute = table[column.to_sym] case value - when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::Relation + when ActiveRecord::Relation + value.select_values = [value.klass.arel_table['id']] if value.select_values.empty? + attribute.in(value.arel.ast) + when Array, ActiveRecord::Associations::CollectionProxy values = value.to_a.map { |x| x.is_a?(ActiveRecord::Base) ? x.id : x } diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 2cbb103eb9..f76681e880 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -171,7 +171,7 @@ module ActiveRecord arel.having(*@having_values.uniq.reject{|h| h.blank?}) unless @having_values.empty? - arel.take(@limit_value) if @limit_value + arel.take(connection.sanitize_limit(@limit_value)) if @limit_value arel.skip(@offset_value) if @offset_value arel.group(*@group_values.uniq.reject{|g| g.blank?}) unless @group_values.empty? @@ -209,16 +209,7 @@ module ActiveRecord def collapse_wheres(arel, wheres) equalities = wheres.grep(Arel::Nodes::Equality) - groups = equalities.group_by do |equality| - equality.left - end - - groups.each do |_, eqls| - test = eqls.inject(eqls.shift) do |memo, expr| - memo.or(expr) - end - arel.where(test) - end + arel.where(Arel::Nodes::And.new(equalities)) unless equalities.empty? (wheres - equalities).each do |where| where = Arel.sql(where) if String === where diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 69a7642ec5..4150e36a9a 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -61,8 +61,6 @@ module ActiveRecord merged_relation end - alias :& :merge - # Removes from the query the condition(s) specified in +skips+. # # Example: diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb index 014a900c71..4e711c4884 100644 --- a/activerecord/lib/active_record/test_case.rb +++ b/activerecord/lib/active_record/test_case.rb @@ -3,6 +3,16 @@ module ActiveRecord # # Defines some test assertions to test against SQL queries. class TestCase < ActiveSupport::TestCase #:nodoc: + setup :cleanup_identity_map + + def setup + cleanup_identity_map + end + + def cleanup_identity_map + ActiveRecord::IdentityMap.clear + end + def assert_date_from_db(expected, actual, message = nil) # SybaseAdapter doesn't have a separate column type just for dates, # so the time is in the string and incorrectly formatted diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 45a4425944..60d4c256c4 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -251,6 +251,7 @@ module ActiveRecord remember_transaction_record_state yield rescue Exception + IdentityMap.remove(self) if IdentityMap.enabled? restore_transaction_record_state raise ensure diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb index 62ffde558f..eb3f8143e7 100644 --- a/activerecord/test/cases/adapters/mysql/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql/connection_test.rb @@ -102,7 +102,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase end # Test that MySQL allows multiple results for stored procedures - if Mysql.const_defined?(:CLIENT_MULTI_RESULTS) + if defined?(Mysql) && Mysql.const_defined?(:CLIENT_MULTI_RESULTS) def test_multi_results rows = ActiveRecord::Base.connection.select_rows('CALL ten();') assert_equal 10, rows[0][0].to_i, "ten() did not return 10 as expected: #{rows.inspect}" diff --git a/activerecord/test/cases/adapters/mysql/reserved_word_test.rb b/activerecord/test/cases/adapters/mysql/reserved_word_test.rb index b5c938b14a..43015098c9 100644 --- a/activerecord/test/cases/adapters/mysql/reserved_word_test.rb +++ b/activerecord/test/cases/adapters/mysql/reserved_word_test.rb @@ -78,24 +78,6 @@ class MysqlReservedWordTest < ActiveRecord::TestCase self.use_instantiated_fixtures = true self.use_transactional_fixtures = false - #fixtures :group - - def test_fixtures - f = create_test_fixtures :select, :distinct, :group, :values, :distincts_selects - - assert_nothing_raised { - f.each do |x| - x.delete_existing_fixtures - end - } - - assert_nothing_raised { - f.each do |x| - x.insert_fixtures - end - } - end - #activerecord model class with reserved-word table name def test_activerecord_model create_test_fixtures :select, :distinct, :group, :values, :distincts_selects diff --git a/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb b/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb index 90d8b0d923..1efa7deaeb 100644 --- a/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb +++ b/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb @@ -78,24 +78,6 @@ class MysqlReservedWordTest < ActiveRecord::TestCase self.use_instantiated_fixtures = true self.use_transactional_fixtures = false - #fixtures :group - - def test_fixtures - f = create_test_fixtures :select, :distinct, :group, :values, :distincts_selects - - assert_nothing_raised { - f.each do |x| - x.delete_existing_fixtures - end - } - - assert_nothing_raised { - f.each do |x| - x.insert_fixtures - end - } - end - #activerecord model class with reserved-word table name def test_activerecord_model create_test_fixtures :select, :distinct, :group, :values, :distincts_selects diff --git a/activerecord/test/cases/associations/association_proxy_test.rb b/activerecord/test/cases/associations/association_proxy_test.rb deleted file mode 100644 index 55d8da4c4e..0000000000 --- a/activerecord/test/cases/associations/association_proxy_test.rb +++ /dev/null @@ -1,52 +0,0 @@ -require "cases/helper" - -module ActiveRecord - module Associations - class AsssociationProxyTest < ActiveRecord::TestCase - class FakeOwner - attr_accessor :new_record - alias :new_record? :new_record - - def initialize - @new_record = false - end - end - - class FakeReflection < Struct.new(:options, :klass) - def initialize options = {}, klass = nil - super - end - - def check_validity! - true - end - end - - class FakeTarget - end - - class FakeTargetProxy < AssociationProxy - def association_scope - true - end - - def find_target - FakeTarget.new - end - end - - def test_method_missing_error - reflection = FakeReflection.new({}, Object.new) - owner = FakeOwner.new - proxy = FakeTargetProxy.new(owner, reflection) - - exception = assert_raises(NoMethodError) do - proxy.omg - end - - assert_match('omg', exception.message) - assert_match(FakeTarget.name, exception.message) - end - end - end -end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 01073bca3d..9006914508 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -50,11 +50,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { account.firm = account.firm } end - def test_triple_equality - assert Client.find(3).firm === Firm - assert Firm === Client.find(3).firm - end - def test_type_mismatch assert_raise(ActiveRecord::AssociationTypeMismatch) { Account.find(1).firm = 1 } assert_raise(ActiveRecord::AssociationTypeMismatch) { Account.find(1).firm = Project.find(1) } @@ -569,13 +564,15 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase def test_reloading_association_with_key_change client = companies(:second_client) - firm = client.firm # note this is a proxy object + firm = client.association(:firm) client.firm = companies(:another_firm) - assert_equal companies(:another_firm), firm.reload + firm.reload + assert_equal companies(:another_firm), firm.target client.client_of = companies(:first_firm).id - assert_equal companies(:first_firm), firm.reload + firm.reload + assert_equal companies(:first_firm), firm.target end def test_polymorphic_counter_cache diff --git a/activerecord/test/cases/associations/eager_load_includes_full_sti_class_test.rb b/activerecord/test/cases/associations/eager_load_includes_full_sti_class_test.rb index fb59f63f91..d75791cab9 100644 --- a/activerecord/test/cases/associations/eager_load_includes_full_sti_class_test.rb +++ b/activerecord/test/cases/associations/eager_load_includes_full_sti_class_test.rb @@ -27,6 +27,7 @@ class EagerLoadIncludeFullStiClassNamesTest < ActiveRecord::TestCase post = Namespaced::Post.find_by_title( 'Great stuff', :include => :tagging ) assert_nil post.tagging + ActiveRecord::IdentityMap.clear ActiveRecord::Base.store_full_sti_class = true post = Namespaced::Post.find_by_title( 'Great stuff', :include => :tagging ) assert_instance_of Tagging, post.tagging 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 8957586189..2cf9f89c3c 100644 --- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb +++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb @@ -1,5 +1,6 @@ require 'cases/helper' require 'models/post' +require 'models/tag' require 'models/author' require 'models/comment' require 'models/category' diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index e11f1009dc..ca71cd8ed3 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -185,7 +185,7 @@ class EagerAssociationTest < ActiveRecord::TestCase author = authors(:david) post = author.post_about_thinking_with_last_comment last_comment = post.last_comment - author = assert_queries(3) { Author.find(author.id, :include => {:post_about_thinking_with_last_comment => :last_comment})} # find the author, then find the posts, then find the comments + author = assert_queries(ActiveRecord::IdentityMap.enabled? ? 2 : 3) { Author.find(author.id, :include => {:post_about_thinking_with_last_comment => :last_comment})} # find the author, then find the posts, then find the comments assert_no_queries do assert_equal post, author.post_about_thinking_with_last_comment assert_equal last_comment, author.post_about_thinking_with_last_comment.last_comment @@ -196,7 +196,7 @@ class EagerAssociationTest < ActiveRecord::TestCase post = posts(:welcome) author = post.author author_address = author.author_address - post = assert_queries(3) { Post.find(post.id, :include => {:author_with_address => :author_address}) } # find the post, then find the author, then find the address + post = assert_queries(ActiveRecord::IdentityMap.enabled? ? 2 : 3) { Post.find(post.id, :include => {:author_with_address => :author_address}) } # find the post, then find the author, then find the address assert_no_queries do assert_equal author, post.author_with_address assert_equal author_address, post.author_with_address.author_address @@ -668,6 +668,14 @@ 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 + post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id) + assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions + + post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id) + assert_equal [comments(:greetings)], post.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)) @@ -809,18 +817,18 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author} - posts = assert_queries(2) do + posts = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => [:comments], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') end assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author} - posts = assert_queries(2) do + posts = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do Post.find(:all, :include => :author, :joins => {:taggings => :tag}, :conditions => "tags.name = 'General'", :order => 'posts.id') end assert_equal posts(:welcome, :thinking), posts - posts = assert_queries(2) do + posts = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do Post.find(:all, :include => :author, :joins => {:taggings => {:tag => :taggings}}, :conditions => "taggings_tags.super_tag_id=2", :order => 'posts.id') end assert_equal posts(:welcome, :thinking), posts @@ -834,7 +842,7 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author} - posts = assert_queries(2) do + posts = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => ["INNER JOIN comments on comments.post_id = posts.id"], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') end assert_equal [posts(:welcome)], posts @@ -851,6 +859,8 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_loading_with_conditions_on_join_model_preloads + Author.columns + authors = assert_queries(2) do Author.find(:all, :include => :author_address, :joins => :comments, :conditions => "posts.title like 'Welcome%'") end @@ -921,7 +931,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_preloading_empty_belongs_to_polymorphic t = Tagging.create!(:taggable_type => 'Post', :taggable_id => Post.maximum(:id) + 1, :tag => tags(:general)) - tagging = assert_queries(2) { Tagging.preload(:taggable).find(t.id) } + tagging = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) { Tagging.preload(:taggable).find(t.id) } assert_no_queries { assert_nil tagging.taggable } end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 126b767d06..dc382c3007 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -72,7 +72,7 @@ class DeveloperWithCounterSQL < ActiveRecord::Base :join_table => "developers_projects", :association_foreign_key => "project_id", :foreign_key => "developer_id", - :counter_sql => 'SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}' + :counter_sql => proc { "SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}" } end class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase @@ -372,27 +372,34 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_destroying david = Developer.find(1) - active_record = Project.find(1) + project = Project.find(1) david.projects.reload assert_equal 2, david.projects.size - assert_equal 3, active_record.developers.size + assert_equal 3, project.developers.size - assert_difference "Project.count", -1 do - david.projects.destroy(active_record) + assert_no_difference "Project.count" do + david.projects.destroy(project) end + join_records = Developer.connection.select_all("SELECT * FROM developers_projects WHERE developer_id = #{david.id} AND project_id = #{project.id}") + assert join_records.empty? + assert_equal 1, david.reload.projects.size assert_equal 1, david.projects(true).size end - def test_destroying_array + def test_destroying_many david = Developer.find(1) david.projects.reload + projects = Project.all - assert_difference "Project.count", -Project.count do - david.projects.destroy(Project.find(:all)) + assert_no_difference "Project.count" do + david.projects.destroy(*projects) end + join_records = Developer.connection.select_all("SELECT * FROM developers_projects WHERE developer_id = #{david.id}") + assert join_records.empty? + assert_equal 0, david.reload.projects.size assert_equal 0, david.projects(true).size end @@ -401,7 +408,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase david = Developer.find(1) david.projects.reload assert !david.projects.empty? - david.projects.destroy_all + + assert_no_difference "Project.count" do + david.projects.destroy_all + end + + join_records = Developer.connection.select_all("SELECT * FROM developers_projects WHERE developer_id = #{david.id}") + assert join_records.empty? + assert david.projects.empty? assert david.projects(true).empty? end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index e36124a055..ad774eb9ce 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -279,7 +279,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_counting_using_finder_sql assert_equal 2, Firm.find(4).clients_using_sql.count - assert_equal 2, Firm.find(4).clients_using_multiline_sql.count end def test_belongs_to_sanity @@ -630,7 +629,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal topic.replies.to_a.size, topic.replies_count end - def test_deleting_updates_counter_cache_without_dependent_destroy + def test_deleting_updates_counter_cache_without_dependent_option post = posts(:welcome) assert_difference "post.reload.taggings_count", -1 do @@ -640,16 +639,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_delete_all post = posts(:welcome) - - # Manually update the count as the tagging will have been added to the taggings association, - # rather than to the taggings_with_delete_all one (which is just a 'shadow' of the former) - post.update_attribute(:taggings_with_delete_all_count, post.taggings_with_delete_all.to_a.count) + post.update_attribute(:taggings_with_delete_all_count, post.taggings_count) assert_difference "post.reload.taggings_with_delete_all_count", -1 do post.taggings_with_delete_all.delete(post.taggings_with_delete_all.first) end end + def test_deleting_updates_counter_cache_with_dependent_destroy + post = posts(:welcome) + post.update_attribute(:taggings_with_destroy_count, post.taggings_count) + + assert_difference "post.reload.taggings_with_destroy_count", -1 do + post.taggings_with_destroy.delete(post.taggings_with_destroy.first) + end + end + def test_deleting_a_collection force_signal37_to_load_all_clients_of_firm companies(:first_firm).clients_of_firm.create("name" => "Another Client") @@ -701,9 +706,9 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_clearing_updates_counter_cache topic = Topic.first - topic.replies.clear - topic.reload - assert_equal 0, topic.replies_count + assert_difference 'topic.reload.replies_count', -1 do + topic.replies.clear + end end def test_clearing_a_dependent_association_collection 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 96f4597726..efdecd4b09 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -25,8 +25,8 @@ require 'models/membership' require 'models/club' class HasManyThroughAssociationsTest < ActiveRecord::TestCase - fixtures :posts, :readers, :people, :comments, :authors, :categories, - :owners, :pets, :toys, :jobs, :references, :companies, :members, + fixtures :posts, :readers, :people, :comments, :authors, :categories, :taggings, :tags, + :owners, :pets, :toys, :jobs, :references, :companies, :members, :author_addresses, :subscribers, :books, :subscriptions, :developers, :categorizations # Dummies to force column loads so query counts are clean. @@ -113,6 +113,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted") end + def test_build_then_save_with_has_many_inverse + post = posts(:thinking) + person = post.people.build(:first_name => "Bob") + person.save + post.reload + + assert post.people.include?(person) + end + + def test_build_then_save_with_has_one_inverse + post = posts(:thinking) + person = post.single_people.build(:first_name => "Bob") + person.save + post.reload + + assert post.single_people.include?(person) + end + def test_delete_association assert_queries(2){posts(:welcome);people(:michael); } @@ -128,8 +146,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_destroy_association - assert_difference ["Person.count", "Reader.count"], -1 do - posts(:welcome).people.destroy(people(:michael)) + assert_no_difference "Person.count" do + assert_difference "Reader.count", -1 do + posts(:welcome).people.destroy(people(:michael)) + end end assert posts(:welcome).reload.people.empty? @@ -137,8 +157,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_destroy_all - assert_difference ["Person.count", "Reader.count"], -1 do - posts(:welcome).people.destroy_all + assert_no_difference "Person.count" do + assert_difference "Reader.count", -1 do + posts(:welcome).people.destroy_all + end end assert posts(:welcome).reload.people.empty? @@ -151,6 +173,137 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end end + def test_delete_through_belongs_to_with_dependent_nullify + Reference.make_comments = true + + person = people(:michael) + job = jobs(:magician) + reference = Reference.where(:job_id => job.id, :person_id => person.id).first + + assert_no_difference ['Job.count', 'Reference.count'] do + assert_difference 'person.jobs.count', -1 do + person.jobs_with_dependent_nullify.delete(job) + end + end + + assert_equal nil, reference.reload.job_id + ensure + Reference.make_comments = false + end + + def test_delete_through_belongs_to_with_dependent_delete_all + Reference.make_comments = true + + person = people(:michael) + job = jobs(:magician) + + # Make sure we're not deleting everything + assert person.jobs.count >= 2 + + assert_no_difference 'Job.count' do + assert_difference ['person.jobs.count', 'Reference.count'], -1 do + person.jobs_with_dependent_delete_all.delete(job) + end + end + + # Check that the destroy callback on Reference did not run + assert_equal nil, person.reload.comments + ensure + Reference.make_comments = false + end + + def test_delete_through_belongs_to_with_dependent_destroy + Reference.make_comments = true + + person = people(:michael) + job = jobs(:magician) + + # Make sure we're not deleting everything + assert person.jobs.count >= 2 + + assert_no_difference 'Job.count' do + assert_difference ['person.jobs.count', 'Reference.count'], -1 do + person.jobs_with_dependent_destroy.delete(job) + end + end + + # Check that the destroy callback on Reference ran + assert_equal "Reference destroyed", person.reload.comments + ensure + Reference.make_comments = false + end + + def test_belongs_to_with_dependent_destroy + person = PersonWithDependentDestroyJobs.find(1) + + # Create a reference which is not linked to a job. This should not be destroyed. + person.references.create! + + assert_no_difference 'Job.count' do + assert_difference 'Reference.count', -person.jobs.count do + person.destroy + end + end + end + + def test_belongs_to_with_dependent_delete_all + person = PersonWithDependentDeleteAllJobs.find(1) + + # Create a reference which is not linked to a job. This should not be destroyed. + person.references.create! + + assert_no_difference 'Job.count' do + assert_difference 'Reference.count', -person.jobs.count do + person.destroy + end + end + end + + def test_belongs_to_with_dependent_nullify + person = PersonWithDependentNullifyJobs.find(1) + + references = person.references.to_a + + assert_no_difference ['Reference.count', 'Job.count'] do + person.destroy + end + + references.each do |reference| + assert_equal nil, reference.reload.job_id + end + end + + def test_update_counter_caches_on_delete + post = posts(:welcome) + tag = post.tags.create!(:name => 'doomed') + + assert_difference ['post.reload.taggings_count', 'post.reload.tags_count'], -1 do + posts(:welcome).tags.delete(tag) + end + end + + def test_update_counter_caches_on_delete_with_dependent_destroy + post = posts(:welcome) + tag = post.tags.create!(:name => 'doomed') + post.update_attribute(:tags_with_destroy_count, post.tags.count) + + assert_difference ['post.reload.taggings_count', 'post.reload.tags_with_destroy_count'], -1 do + posts(:welcome).tags_with_destroy.delete(tag) + end + end + + def test_update_counter_caches_on_delete_with_dependent_nullify + post = posts(:welcome) + tag = post.tags.create!(:name => 'doomed') + post.update_attribute(:tags_with_nullify_count, post.tags.count) + + assert_no_difference 'post.reload.taggings_count' do + assert_difference 'post.reload.tags_with_nullify_count', -1 do + posts(:welcome).tags_with_nullify.delete(tag) + end + end + end + def test_replace_association assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} @@ -567,4 +720,30 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal true, club.reload.membership.favourite end + + def test_deleting_from_has_many_through_a_belongs_to_should_not_try_to_update_counter + post = posts(:welcome) + address = author_addresses(:david_address) + + assert post.author_addresses.include?(address) + post.author_addresses.delete(address) + assert post[:author_count].nil? + end + + def test_interpolated_conditions + post = posts(:welcome) + assert !post.tags.empty? + assert_equal post.tags, post.interpolated_tags + assert_equal post.tags, post.interpolated_tags_2 + end + + def test_primary_key_option_on_source + post = posts(:welcome) + category = categories(:general) + categorization = Categorization.create!(:post_id => post.id, :named_category_name => category.name) + + assert_equal [category], post.named_categories + assert_equal [category.name], post.named_category_ids # checks when target loaded + assert_equal [category.name], post.reload.named_category_ids # checks when target no loaded + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index dbf6dfe20d..c1dad5e246 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -66,11 +66,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { company.account = company.account } end - def test_triple_equality - assert Account === companies(:first_firm).account - assert companies(:first_firm).account === Account - end - def test_type_mismatch assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = 1 } assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = Project.find(1) } @@ -243,6 +238,14 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm.destroy end + def test_finding_with_interpolated_condition + firm = Firm.find(:first) + superior = firm.clients.create(:name => 'SuperiorCo') + superior.rating = 10 + superior.save + assert_equal 10, firm.clients_with_interpolated_conditions.first.rating + end + def test_assignment_before_child_saved firm = Firm.find(1) firm.account = a = Account.new("credit_limit" => 1000) @@ -312,7 +315,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_creation_failure_without_dependent_option pirate = pirates(:blackbeard) - orig_ship = pirate.ship.target + orig_ship = pirate.ship assert_equal ships(:black_pearl), orig_ship new_ship = pirate.create_ship @@ -325,7 +328,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_creation_failure_with_dependent_option pirate = pirates(:blackbeard).becomes(DestructivePirate) - orig_ship = pirate.dependent_ship.target + orig_ship = pirate.dependent_ship new_ship = pirate.create_dependent_ship assert new_ship.new_record? 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 91d3025468..bfc5ddc747 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -88,12 +88,12 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase # 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 + assert_equal nil, Member.find(@member.id, :include => :favourite_club).reload.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 + assert_equal nil, Member.find(@member.id, :include => :hairy_club).reload.hairy_club end def test_has_one_through_polymorphic_with_source_type @@ -139,7 +139,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_assigning_association_correctly_assigns_target new_member = Member.create(:name => "Chris") new_member.club = new_club = Club.create(:name => "LRUG") - assert_equal new_club, new_member.club.target + assert_equal new_club, new_member.association(:club).target end def test_has_one_through_proxy_should_not_respond_to_private_methods @@ -197,7 +197,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase MemberDetail.find(:all, :include => :member_type) end @new_detail = @member_details[0] - assert @new_detail.send(:association_proxy, :member_type).loaded? + assert @new_detail.send(:association, :member_type).loaded? assert_not_nil assert_no_queries { @new_detail.member_type } end diff --git a/activerecord/test/cases/associations/identity_map_test.rb b/activerecord/test/cases/associations/identity_map_test.rb new file mode 100644 index 0000000000..9b8635774c --- /dev/null +++ b/activerecord/test/cases/associations/identity_map_test.rb @@ -0,0 +1,137 @@ +require "cases/helper" +require 'models/author' +require 'models/post' + +if ActiveRecord::IdentityMap.enabled? +class InverseHasManyIdentityMapTest < ActiveRecord::TestCase + fixtures :authors, :posts + + def test_parent_instance_should_be_shared_with_every_child_on_find + m = Author.first + is = m.posts + is.each do |i| + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to child-owned instance" + end + end + + def test_parent_instance_should_be_shared_with_eager_loaded_children + m = Author.find(:first, :include => :posts) + is = m.posts + is.each do |i| + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to child-owned instance" + end + + m = Author.find(:first, :include => :posts, :order => 'posts.id') + is = m.posts + is.each do |i| + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to child-owned instance" + end + end + + def test_parent_instance_should_be_shared_with_newly_built_child + m = Author.first + i = m.posts.build(:title => 'Industrial Revolution Re-enactment', :body => 'Lorem ipsum') + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to just-built-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_block_style_built_child + m = Author.first + i = m.posts.build {|ii| ii.title = 'Industrial Revolution Re-enactment'; ii.body = 'Lorem ipsum'} + assert_not_nil i.title, "Child attributes supplied to build via blocks should be populated" + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to just-built-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_child + m = Author.first + i = m.posts.create(:title => 'Industrial Revolution Re-enactment', :body => 'Lorem ipsum') + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_via_bang_method_child + m = Author.first + i = m.posts.create!(:title => 'Industrial Revolution Re-enactment', :body => 'Lorem ipsum') + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_block_style_created_child + m = Author.first + i = m.posts.create {|ii| ii.title = 'Industrial Revolution Re-enactment'; ii.body = 'Lorem ipsum'} + assert_not_nil i.title, "Child attributes supplied to create via blocks should be populated" + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_poked_in_child + m = Author.first + i = Post.create(:title => 'Industrial Revolution Re-enactment', :body => 'Lorem ipsum') + m.posts << i + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_replaced_via_accessor_children + m = Author.first + i = Post.new(:title => 'Industrial Revolution Re-enactment', :body => 'Lorem ipsum') + m.posts = [i] + assert_same m, i.author + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to replaced-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_replaced_via_method_children + m = Author.first + i = Post.new(:title => 'Industrial Revolution Re-enactment', :body => 'Lorem ipsum') + m.posts = [i] + assert_not_nil i.author + assert_equal m.name, i.author.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to parent instance" + i.author.name = 'Mungo' + assert_equal m.name, i.author.name, "Name of man should be the same after changes to replaced-child-owned instance" + end +end +end diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index da2a81e98a..e2228228a3 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -4,6 +4,7 @@ require 'models/comment' require 'models/author' require 'models/category' require 'models/categorization' +require 'models/person' require 'models/tagging' require 'models/tag' @@ -16,6 +17,13 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase assert_equal authors(:david), result.first end + def test_construct_finder_sql_does_not_table_name_collide_on_duplicate_associations + assert_nothing_raised do + sql = Person.joins(:agents => {:agents => :agents}).joins(:agents => {:agents => {:primary_contact => :agents}}).to_sql + assert_match(/agents_people_4/i, sql) + end + end + def test_construct_finder_sql_ignores_empty_joins_hash sql = Author.joins({}).to_sql assert_no_match(/JOIN/i, sql) diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index e9a57a00a0..76282213d8 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -137,7 +137,7 @@ class InverseHasOneTests < ActiveRecord::TestCase def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method m = Man.find(:first) - f = m.face.create!(:description => 'haunted') + f = m.create_face!(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -158,18 +158,6 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" end - def test_parent_instance_should_be_shared_with_replaced_via_method_child - m = Man.find(:first) - f = Face.new(:description => 'haunted') - m.face.replace(f) - assert_not_nil f.man - assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" - m.name = 'Bongo' - assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" - f.man.name = 'Mungo' - assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" - end - def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).dirty_face } end @@ -271,18 +259,6 @@ class InverseHasManyTests < ActiveRecord::TestCase assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance" end - def test_parent_instance_should_be_shared_with_replaced_via_method_children - m = Man.find(:first) - i = Interest.new(:topic => 'Industrial Revolution Re-enactment') - m.interests.replace([i]) - assert_not_nil i.man - assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" - m.name = 'Bongo' - assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" - i.man.name = 'Mungo' - assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance" - end - def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests } end @@ -366,19 +342,6 @@ class InverseBelongsToTests < ActiveRecord::TestCase assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance" end - def test_child_instance_should_be_shared_with_replaced_via_method_parent - f = faces(:trusting) - assert_not_nil f.man - m = Man.new(:name => 'Charles') - f.man.replace(m) - assert_not_nil m.face - assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" - f.description = 'gormless' - assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" - m.face.description = 'pleasing' - assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance" - end - def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man } end @@ -434,7 +397,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase new_man = Man.new assert_not_nil face.polymorphic_man - face.polymorphic_man.replace(new_man) + face.polymorphic_man = new_man assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance" face.description = 'Bongo' diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index c50fcd3f33..6d7f905dc5 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -88,7 +88,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_polymorphic_has_many_going_through_join_model_with_custom_select_and_joins assert_equal tags(:general), tag = posts(:welcome).tags.add_joins_and_select.first - tag.author_id + assert_nothing_raised(NoMethodError) { tag.author_id } end def test_polymorphic_has_many_going_through_join_model_with_custom_foreign_key @@ -153,7 +153,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_create_polymorphic_has_one_with_scope old_count = Tagging.count - tagging = posts(:welcome).tagging.create(:tag => tags(:misc)) + tagging = posts(:welcome).create_tagging(:tag => tags(:misc)) assert_equal "Post", tagging.taggable_type assert_equal old_count+1, Tagging.count end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 83c605d2bb..47b8e48582 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -133,25 +133,6 @@ end class AssociationProxyTest < ActiveRecord::TestCase fixtures :authors, :posts, :categorizations, :categories, :developers, :projects, :developers_projects - def test_proxy_accessors - welcome = posts(:welcome) - assert_equal welcome, welcome.author.proxy_owner - assert_equal welcome.class.reflect_on_association(:author), welcome.author.proxy_reflection - welcome.author.class # force load target - assert_equal welcome.author, welcome.author.proxy_target - - david = authors(:david) - assert_equal david, david.posts.proxy_owner - assert_equal david.class.reflect_on_association(:posts), david.posts.proxy_reflection - david.posts.class # force load target - assert_equal david.posts, david.posts.proxy_target - - assert_equal david, david.posts_with_extension.testing_proxy_owner - assert_equal david.class.reflect_on_association(:posts_with_extension), david.posts_with_extension.testing_proxy_reflection - david.posts_with_extension.class # force load target - assert_equal david.posts_with_extension, david.posts_with_extension.testing_proxy_target - end - def test_push_does_not_load_target david = authors(:david) @@ -216,37 +197,12 @@ class AssociationProxyTest < ActiveRecord::TestCase assert_equal post.body, "More cool stuff!" end - def test_failed_reload_returns_nil - p = setup_dangling_association - assert_nil p.author.reload - end - - def test_failed_reset_returns_nil - p = setup_dangling_association - assert_nil p.author.reset - end - def test_reload_returns_assocition david = developers(:david) assert_nothing_raised do assert_equal david.projects, david.projects.reload.reload end end - - if RUBY_VERSION < '1.9' - def test_splat_does_not_invoke_to_a_on_singular_targets - author = posts(:welcome).author - author.reload.target.expects(:to_a).never - [*author] - end - end - - def setup_dangling_association - josh = Author.create(:name => "Josh") - p = Post.create(:title => "New on Edge", :body => "More cool stuff!", :author => josh) - josh.destroy - p - end end class OverridingAssociationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 7e3e204626..dfacf58da8 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -8,6 +8,7 @@ require 'models/topic' require 'models/company' require 'models/category' require 'models/reply' +require 'models/contact' class AttributeMethodsTest < ActiveRecord::TestCase fixtures :topics, :developers, :companies, :computers @@ -131,7 +132,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_equal developer.created_at, nil developer.created_at = "2010-03-21 21:23:32" - assert_equal developer.created_at_before_type_cast.to_s, "2010-03-21 21:23:32" + assert_equal developer.created_at_before_type_cast, "2010-03-21 21:23:32" assert_equal developer.created_at, Time.parse("2010-03-21 21:23:32") end @@ -460,6 +461,14 @@ class AttributeMethodsTest < ActiveRecord::TestCase end end + def test_write_nil_to_time_attributes + in_time_zone "Pacific Time (US & Canada)" do + record = @target.new + record.written_on = nil + assert_nil record.written_on + end + end + def test_time_attributes_are_retrieved_in_current_time_zone in_time_zone "Pacific Time (US & Canada)" do utc_time = Time.utc(2008, 1, 1) @@ -601,6 +610,10 @@ class AttributeMethodsTest < ActiveRecord::TestCase Object.send(:undef_method, :title) # remove test method from object end + def test_list_of_serialized_attributes + assert_equal %w(content), Topic.serialized_attributes.keys + assert_equal %w(preferences), Contact.serialized_attributes.keys + end private def cached_columns diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 8688ebc617..0e93b468c1 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -112,7 +112,7 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas def test_build_before_child_saved firm = Firm.find(1) - account = firm.account.build("credit_limit" => 1000) + account = firm.build_account("credit_limit" => 1000) assert_equal account, firm.account assert !account.persisted? assert firm.save @@ -585,7 +585,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase @pirate.ship.mark_for_destruction assert !@pirate.reload.marked_for_destruction? - assert !@pirate.ship.marked_for_destruction? + assert !@pirate.ship.reload.marked_for_destruction? end # has_one @@ -689,64 +689,127 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase assert_equal 'NewName', @parrot.reload.name end - # has_many & has_and_belongs_to - %w{ parrots birds }.each do |association_name| - define_method("test_should_destroy_#{association_name}_as_part_of_the_save_transaction_if_they_were_marked_for_destroyal") do - 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } + def test_should_destroy_has_many_as_part_of_the_save_transaction_if_they_were_marked_for_destruction + 2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") } - assert !@pirate.send(association_name).any? { |child| child.marked_for_destruction? } + assert !@pirate.birds.any? { |child| child.marked_for_destruction? } - @pirate.send(association_name).each { |child| child.mark_for_destruction } - klass = @pirate.send(association_name).first.class - ids = @pirate.send(association_name).map(&:id) + @pirate.birds.each { |child| child.mark_for_destruction } + klass = @pirate.birds.first.class + ids = @pirate.birds.map(&:id) - assert @pirate.send(association_name).all? { |child| child.marked_for_destruction? } - ids.each { |id| assert klass.find_by_id(id) } + assert @pirate.birds.all? { |child| child.marked_for_destruction? } + ids.each { |id| assert klass.find_by_id(id) } - @pirate.save - assert @pirate.reload.send(association_name).empty? - ids.each { |id| assert_nil klass.find_by_id(id) } + @pirate.save + assert @pirate.reload.birds.empty? + ids.each { |id| assert_nil klass.find_by_id(id) } + end + + def test_should_skip_validation_on_has_many_if_marked_for_destruction + 2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") } + + @pirate.birds.each { |bird| bird.name = '' } + assert !@pirate.valid? + + @pirate.birds.each do |bird| + bird.mark_for_destruction + bird.expects(:valid?).never end + assert_difference("Bird.count", -2) { @pirate.save! } + end + + def test_should_skip_validation_on_has_many_if_destroyed + @pirate.birds.create!(:name => "birds_1") + + @pirate.birds.each { |bird| bird.name = '' } + assert !@pirate.valid? + + @pirate.birds.each { |bird| bird.destroy } + assert @pirate.valid? + end + + def test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_has_many + @pirate.birds.create!(:name => "birds_1") + + @pirate.birds.each { |bird| bird.mark_for_destruction } + assert @pirate.save - define_method("test_should_skip_validation_on_the_#{association_name}_association_if_marked_for_destruction") do - 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } - children = @pirate.send(association_name) + @pirate.birds.each { |bird| bird.expects(:destroy).never } + assert @pirate.save + end - children.each { |child| child.name = '' } - assert !@pirate.valid? + def test_should_rollback_destructions_if_an_exception_occurred_while_saving_has_many + 2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") } + before = @pirate.birds.map { |c| c.mark_for_destruction ; c } - children.each do |child| - child.mark_for_destruction - child.expects(:valid?).never + # Stub the destroy method of the the second child to raise an exception + class << before.last + def destroy(*args) + super + raise 'Oh noes!' end - assert_difference("#{association_name.classify}.count", -2) { @pirate.save! } end - define_method("test_should_skip_validation_on_the_#{association_name}_association_if_destroyed") do - @pirate.send(association_name).create!(:name => "#{association_name}_1") - children = @pirate.send(association_name) + assert_raise(RuntimeError) { assert !@pirate.save } + assert_equal before, @pirate.reload.birds + end + + # Add and remove callbacks tests for association collections. + %w{ method proc }.each do |callback_type| + define_method("test_should_run_add_callback_#{callback_type}s_for_has_many") do + association_name_with_callbacks = "birds_with_#{callback_type}_callbacks" - children.each { |child| child.name = '' } - assert !@pirate.valid? + pirate = Pirate.new(:catchphrase => "Arr") + pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed") - children.each { |child| child.destroy } - assert @pirate.valid? + expected = [ + "before_adding_#{callback_type}_bird_<new>", + "after_adding_#{callback_type}_bird_<new>" + ] + + assert_equal expected, pirate.ship_log end - define_method("test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_#{association_name}") do - @pirate.send(association_name).create!(:name => "#{association_name}_1") - children = @pirate.send(association_name) + define_method("test_should_run_remove_callback_#{callback_type}s_for_has_many") do + association_name_with_callbacks = "birds_with_#{callback_type}_callbacks" - children.each { |child| child.mark_for_destruction } - assert @pirate.save - children.each { |child| child.expects(:destroy).never } - assert @pirate.save + @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed") + @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction } + child_id = @pirate.send(association_name_with_callbacks).first.id + + @pirate.ship_log.clear + @pirate.save + + expected = [ + "before_removing_#{callback_type}_bird_#{child_id}", + "after_removing_#{callback_type}_bird_#{child_id}" + ] + + assert_equal expected, @pirate.ship_log end + end + + def test_should_destroy_habtm_as_part_of_the_save_transaction_if_they_were_marked_for_destruction + 2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") } - define_method("test_should_rollback_destructions_if_an_exception_occurred_while_saving_#{association_name}") do - 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } - before = @pirate.send(association_name).map { |c| c.mark_for_destruction ; c } +<<<<<<< HEAD + assert !@pirate.parrots.any? { |parrot| parrot.marked_for_destruction? } + @pirate.parrots.each { |parrot| parrot.mark_for_destruction } + assert_no_difference "Parrot.count" do + @pirate.save + end + + assert @pirate.reload.parrots.empty? + + join_records = Pirate.connection.select_all("SELECT * FROM parrots_pirates WHERE pirate_id = #{@pirate.id}") + assert join_records.empty? + end + + def test_should_skip_validation_on_habtm_if_marked_for_destruction + 2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") } +======= # Stub the destroy method of the second child to raise an exception class << before.last def destroy(*args) @@ -754,45 +817,89 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase raise 'Oh noes!' end end +>>>>>>> 220cb107b672d65fdc0488d4ff310ab04b62b463 + + @pirate.parrots.each { |parrot| parrot.name = '' } + assert !@pirate.valid? - assert_raise(RuntimeError) { assert !@pirate.save } - assert_equal before, @pirate.reload.send(association_name) + @pirate.parrots.each do |parrot| + parrot.mark_for_destruction + parrot.expects(:valid?).never end - # Add and remove callbacks tests for association collections. - %w{ method proc }.each do |callback_type| - define_method("test_should_run_add_callback_#{callback_type}s_for_#{association_name}") do - association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks" + @pirate.save! + assert @pirate.reload.parrots.empty? + end - pirate = Pirate.new(:catchphrase => "Arr") - pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed") + def test_should_skip_validation_on_habtm_if_destroyed + @pirate.parrots.create!(:name => "parrots_1") - expected = [ - "before_adding_#{callback_type}_#{association_name.singularize}_<new>", - "after_adding_#{callback_type}_#{association_name.singularize}_<new>" - ] + @pirate.parrots.each { |parrot| parrot.name = '' } + assert !@pirate.valid? - assert_equal expected, pirate.ship_log - end + @pirate.parrots.each { |parrot| parrot.destroy } + assert @pirate.valid? + end - define_method("test_should_run_remove_callback_#{callback_type}s_for_#{association_name}") do - association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks" + def test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_habtm + @pirate.parrots.create!(:name => "parrots_1") - @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed") - @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction } - child_id = @pirate.send(association_name_with_callbacks).first.id + @pirate.parrots.each { |parrot| parrot.mark_for_destruction } + assert @pirate.save - @pirate.ship_log.clear - @pirate.save + assert_no_queries do + assert @pirate.save + end + end - expected = [ - "before_removing_#{callback_type}_#{association_name.singularize}_#{child_id}", - "after_removing_#{callback_type}_#{association_name.singularize}_#{child_id}" - ] + def test_should_rollback_destructions_if_an_exception_occurred_while_saving_habtm + 2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") } + before = @pirate.parrots.map { |c| c.mark_for_destruction ; c } - assert_equal expected, @pirate.ship_log + class << @pirate.parrots + def destroy(*args) + super + raise 'Oh noes!' end end + + assert_raise(RuntimeError) { assert !@pirate.save } + assert_equal before, @pirate.reload.parrots + end + + # Add and remove callbacks tests for association collections. + %w{ method proc }.each do |callback_type| + define_method("test_should_run_add_callback_#{callback_type}s_for_habtm") do + association_name_with_callbacks = "parrots_with_#{callback_type}_callbacks" + + pirate = Pirate.new(:catchphrase => "Arr") + pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed") + + expected = [ + "before_adding_#{callback_type}_parrot_<new>", + "after_adding_#{callback_type}_parrot_<new>" + ] + + assert_equal expected, pirate.ship_log + end + + define_method("test_should_run_remove_callback_#{callback_type}s_for_habtm") do + association_name_with_callbacks = "parrots_with_#{callback_type}_callbacks" + + @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed") + @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction } + child_id = @pirate.send(association_name_with_callbacks).first.id + + @pirate.ship_log.clear + @pirate.save + + expected = [ + "before_removing_#{callback_type}_parrot_#{child_id}", + "after_removing_#{callback_type}_parrot_#{child_id}" + ] + + assert_equal expected, @pirate.ship_log + end end end @@ -1214,6 +1321,7 @@ class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::Tes def setup @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?") @pirate.create_ship(:name => 'titanic') + super end test "should automatically validate associations with :validate => true" do @@ -1222,7 +1330,7 @@ class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::Tes assert !@pirate.valid? end - test "should not automatically validate associations without :validate => true" do + test "should not automatically asd validate associations without :validate => true" do assert @pirate.valid? @pirate.non_validated_ship.name = '' assert @pirate.valid? diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 68adeff882..0ad20bb9bc 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -20,6 +20,7 @@ require 'models/warehouse_thing' require 'models/parrot' require 'models/loose_person' require 'models/edge' +require 'models/joke' require 'rexml/document' require 'active_support/core_ext/exception' @@ -49,10 +50,57 @@ class Boolean < ActiveRecord::Base; end class BasicsTest < ActiveRecord::TestCase fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts + def test_columns_should_obey_set_primary_key + pk = Subscriber.columns.find { |x| x.name == 'nick' } + assert pk.primary, 'nick should be primary key' + end + def test_primary_key_with_no_id assert_nil Edge.primary_key end + unless current_adapter?(:PostgreSQLAdapter,:OracleAdapter,:SQLServerAdapter) + def test_limit_with_comma + assert_nothing_raised do + Topic.limit("1,2").all + end + end + end + + def test_limit_without_comma + assert_nothing_raised do + assert_equal 1, Topic.limit("1").all.length + end + + assert_nothing_raised do + assert_equal 1, Topic.limit(1).all.length + end + end + + def test_invalid_limit + assert_raises(ArgumentError) do + Topic.limit("asdfadf").all + end + end + + def test_limit_should_sanitize_sql_injection_for_limit_without_comas + assert_raises(ArgumentError) do + Topic.limit("1 select * from schema").all + end + end + + def test_limit_should_sanitize_sql_injection_for_limit_with_comas + assert_raises(ArgumentError) do + Topic.limit("1, 7 procedure help()").all + end + end + + unless current_adapter?(:MysqlAdapter) || current_adapter?(:Mysql2Adapter) + def test_limit_should_allow_sql_literal + assert_equal 1, Topic.limit(Arel.sql('2-1')).all.length + end + end + def test_select_symbol topic_ids = Topic.select(:id).map(&:id).sort assert_equal Topic.find(:all).map(&:id).sort, topic_ids @@ -1156,6 +1204,16 @@ class BasicsTest < ActiveRecord::TestCase assert_equal "bar", k.table_name end + def test_switching_between_table_name + assert_difference("GoodJoke.count") do + Joke.set_table_name "cold_jokes" + Joke.create + + Joke.set_table_name "funny_jokes" + Joke.create + end + end + def test_quoted_table_name_after_set_table_name klass = Class.new(ActiveRecord::Base) @@ -1237,12 +1295,6 @@ class BasicsTest < ActiveRecord::TestCase assert_equal res6, res7 end - def test_interpolate_sql - assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo@bar') } - assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar) baz') } - assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar} baz') } - end - def test_scoped_find_conditions scoped_developers = Developer.send(:with_scope, :find => { :conditions => 'salary > 90000' }) do Developer.find(:all, :conditions => 'id < 5') diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb new file mode 100644 index 0000000000..19383bb06b --- /dev/null +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -0,0 +1,90 @@ +require 'cases/helper' +require 'models/topic' + +module ActiveRecord + class BindParameterTest < ActiveRecord::TestCase + fixtures :topics + + class LogListener + attr_accessor :calls + + def initialize + @calls = [] + end + + def call(*args) + calls << args + end + end + + def setup + super + @connection = ActiveRecord::Base.connection + @listener = LogListener.new + @pk = Topic.columns.find { |c| c.primary } + ActiveSupport::Notifications.subscribe('sql.active_record', @listener) + end + + def teardown + ActiveSupport::Notifications.unsubscribe(@listener) + end + + def test_binds_are_logged + # FIXME: use skip with minitest + return unless @connection.supports_statement_cache? + + sub = @connection.substitute_for(@pk, []) + binds = [[@pk, 1]] + sql = "select * from topics where id = #{sub}" + + @connection.exec_query(sql, 'SQL', binds) + + message = @listener.calls.find { |args| args[4][:sql] == sql } + assert_equal binds, message[4][:binds] + end + + def test_find_one_uses_binds + # FIXME: use skip with minitest + return unless @connection.supports_statement_cache? + + Topic.find(1) + binds = [[@pk, 1]] + message = @listener.calls.find { |args| args[4][:binds] == binds } + assert message, 'expected a message with binds' + end + + def test_logs_bind_vars + # FIXME: use skip with minitest + return unless @connection.supports_statement_cache? + + pk = Topic.columns.find { |x| x.primary } + + payload = { + :name => 'SQL', + :sql => 'select * from topics where id = ?', + :binds => [[pk, 10]] + } + event = ActiveSupport::Notifications::Event.new( + 'foo', + Time.now, + Time.now, + 123, + payload) + + logger = Class.new(ActiveRecord::LogSubscriber) { + attr_reader :debugs + def initialize + super + @debugs = [] + end + + def debug str + @debugs << str + end + }.new + + logger.sql event + assert_match([[pk.name, 10]].inspect, logger.debugs.first) + end + end +end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 55ac1bc406..7ac14fa8d6 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -6,6 +6,16 @@ module ActiveRecord def setup # Keep a duplicate pool so we do not bother others @pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec + + if in_memory_db? + # Separate connections to an in-memory database create an entirely new database, + # with an empty schema etc, so we just stub out this schema on the fly. + @pool.with_connection do |connection| + connection.create_table :posts do |t| + t.integer :cololumn + end + end + end end def test_pool_caches_columns @@ -18,16 +28,14 @@ module ActiveRecord assert_equal columns_hash, @pool.columns_hash['posts'] end - def test_clearing_cache + def test_clearing_column_cache @pool.columns['posts'] @pool.columns_hash['posts'] - @pool.primary_keys['posts'] @pool.clear_cache! assert_equal 0, @pool.columns.size assert_equal 0, @pool.columns_hash.size - assert_equal 0, @pool.primary_keys.size end def test_primary_key diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 864a7a2acc..fa40fad56d 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -35,7 +35,7 @@ class FixturesTest < ActiveRecord::TestCase def test_clean_fixtures FIXTURES.each do |name| fixtures = nil - assert_nothing_raised { fixtures = create_fixtures(name) } + assert_nothing_raised { fixtures = create_fixtures(name).first } assert_kind_of(Fixtures, fixtures) fixtures.each { |_name, fixture| fixture.each { |key, value| @@ -53,7 +53,7 @@ class FixturesTest < ActiveRecord::TestCase end def test_attributes - topics = create_fixtures("topics") + topics = create_fixtures("topics").first assert_equal("The First Topic", topics["first"]["title"]) assert_nil(topics["second"]["author_email_address"]) end @@ -127,12 +127,11 @@ class FixturesTest < ActiveRecord::TestCase end def test_instantiation - topics = create_fixtures("topics") + topics = create_fixtures("topics").first assert_kind_of Topic, topics["first"].find end def test_complete_instantiation - assert_equal 4, @topics.size assert_equal "The First Topic", @first.title end @@ -142,7 +141,6 @@ class FixturesTest < ActiveRecord::TestCase end def test_erb_in_fixtures - assert_equal 11, @developers.size assert_equal "fixture_5", @dev_5.name end @@ -199,7 +197,6 @@ class FixturesTest < ActiveRecord::TestCase end def test_binary_in_fixtures - assert_equal 1, @binaries.size data = File.open(ASSETS_ROOT + "/flowers.jpg", 'rb') { |f| f.read } data.force_encoding('ASCII-8BIT') if data.respond_to?(:force_encoding) data.freeze @@ -245,7 +242,7 @@ if Account.connection.respond_to?(:reset_pk_sequence!) def test_create_fixtures_resets_sequences_when_not_cached @instances.each do |instance| - max_id = create_fixtures(instance.class.table_name).inject(0) do |_max_id, (_, fixture)| + max_id = create_fixtures(instance.class.table_name).first.fixtures.inject(0) do |_max_id, (_, fixture)| fixture_id = fixture['id'].to_i fixture_id > _max_id ? fixture_id : _max_id end @@ -304,9 +301,6 @@ class FixturesWithoutInstanceInstantiationTest < ActiveRecord::TestCase def test_without_instance_instantiation assert !defined?(@first), "@first is not defined" - assert_not_nil @topics - assert_not_nil @developers - assert_not_nil @accounts end end @@ -384,6 +378,21 @@ class ForeignKeyFixturesTest < ActiveRecord::TestCase end end +class OverRideFixtureMethodTest < ActiveRecord::TestCase + fixtures :topics + + def topics(name) + topic = super + topic.title = 'omg' + topic + end + + def test_fixture_methods_can_be_overridden + x = topics :first + assert_equal 'omg', x.title + end +end + class CheckSetTableNameFixturesTest < ActiveRecord::TestCase set_fixture_class :funny_jokes => 'Joke' fixtures :funny_jokes @@ -509,7 +518,7 @@ class FasterFixturesTest < ActiveRecord::TestCase fixtures :categories, :authors def load_extra_fixture(name) - fixture = create_fixtures(name) + fixture = create_fixtures(name).first assert fixture.is_a?(Fixtures) @loaded_fixtures[fixture.table_name] = fixture end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 97bb631d2d..fd20f1b120 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -11,7 +11,14 @@ require 'mocha' require 'active_record' require 'active_support/dependencies' -require 'connection' +begin + require 'connection' +rescue LoadError + # If we cannot load connection we assume that driver was not loaded for this test case, so we load sqlite3 as default one. + # This allows for running separate test cases by simply running test file. + connection_type = defined?(JRUBY_VERSION) ? 'jdbc' : 'native' + require "test/connections/#{connection_type}_sqlite3/connection" +end # Show backtraces for deprecated behavior for quicker cleanup. ActiveSupport::Deprecation.debug = true @@ -19,6 +26,9 @@ ActiveSupport::Deprecation.debug = true # Quote "type" if it's a reserved word for the current connection. QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name('type') +# Enable Identity Map for testing +ActiveRecord::IdentityMap.enabled = (ENV['IM'] == "false" ? false : true) + def current_adapter?(*types) types.any? do |type| ActiveRecord::ConnectionAdapters.const_defined?(type) && @@ -49,42 +59,31 @@ ensure ActiveRecord::Base.default_timezone = old_zone end -ActiveRecord::Base.connection.class.class_eval do - IGNORED_SQL = [/^PRAGMA (?!(table_info))/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/] +module ActiveRecord + class SQLCounter + IGNORED_SQL = [/^PRAGMA (?!(table_info))/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /^SHOW max_identifier_length/] - # FIXME: this needs to be refactored so specific database can add their own - # ignored SQL. This ignored SQL is for Oracle. - IGNORED_SQL.concat [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from ((all|user)_tab_columns|(all|user)_triggers|(all|user)_constraints)/im] + # FIXME: this needs to be refactored so specific database can add their own + # ignored SQL. This ignored SQL is for Oracle. + IGNORED_SQL.concat [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im] - def execute_with_query_record(sql, name = nil, &block) - $queries_executed ||= [] - $queries_executed << sql unless IGNORED_SQL.any? { |r| sql =~ r } - execute_without_query_record(sql, name, &block) - end + def initialize + $queries_executed = [] + end - alias_method_chain :execute, :query_record + def call(name, start, finish, message_id, values) + sql = values[:sql] - def exec_query_with_query_record(sql, name = nil, binds = [], &block) - $queries_executed ||= [] - $queries_executed << sql unless IGNORED_SQL.any? { |r| sql =~ r } - exec_query_without_query_record(sql, name, binds, &block) + # FIXME: this seems bad. we should probably have a better way to indicate + # the query was cached + unless 'CACHE' == values[:name] + $queries_executed << sql unless IGNORED_SQL.any? { |r| sql =~ r } + end + end end - - alias_method_chain :exec_query, :query_record + ActiveSupport::Notifications.subscribe('sql.active_record', SQLCounter.new) end -ActiveRecord::Base.connection.class.class_eval { - attr_accessor :column_calls - - def columns_with_calls(*args) - @column_calls ||= 0 - @column_calls += 1 - columns_without_calls(*args) - end - - alias_method_chain :columns, :calls -} - unless ENV['FIXTURE_DEBUG'] module ActiveRecord::TestFixtures::ClassMethods def try_to_load_dependency_with_silence(*args) @@ -105,7 +104,7 @@ class ActiveSupport::TestCase self.use_transactional_fixtures = true def create_fixtures(*table_names, &block) - Fixtures.create_fixtures(ActiveSupport::TestCase.fixture_path, table_names, {}, &block) + Fixtures.create_fixtures(ActiveSupport::TestCase.fixture_path, table_names, fixture_class_names, &block) end end diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb new file mode 100644 index 0000000000..d98638ab73 --- /dev/null +++ b/activerecord/test/cases/identity_map_test.rb @@ -0,0 +1,402 @@ +require "cases/helper" +require 'models/developer' +require 'models/project' +require 'models/company' +require 'models/topic' +require 'models/reply' +require 'models/computer' +require 'models/customer' +require 'models/order' +require 'models/post' +require 'models/author' +require 'models/tag' +require 'models/tagging' +require 'models/comment' +require 'models/sponsor' +require 'models/member' +require 'models/essay' +require 'models/subscriber' +require "models/pirate" +require "models/bird" +require "models/parrot" + +if ActiveRecord::IdentityMap.enabled? +class IdentityMapTest < ActiveRecord::TestCase + fixtures :accounts, :companies, :developers, :projects, :topics, + :developers_projects, :computers, :authors, :author_addresses, + :posts, :tags, :taggings, :comments, :subscribers + + ############################################################################## + # Basic tests checking if IM is functioning properly on basic find operations# + ############################################################################## + + def test_find_id + assert_same(Client.find(3), Client.find(3)) + end + + def test_find_id_without_identity_map + ActiveRecord::IdentityMap.without do + assert_not_same(Client.find(3), Client.find(3)) + end + end + + def test_find_id_use_identity_map + ActiveRecord::IdentityMap.enabled = false + ActiveRecord::IdentityMap.use do + assert_same(Client.find(3), Client.find(3)) + end + ActiveRecord::IdentityMap.enabled = true + end + + def test_find_pkey + assert_same( + Subscriber.find('swistak'), + Subscriber.find('swistak') + ) + end + + def test_find_by_id + assert_same( + Client.find_by_id(3), + Client.find_by_id(3) + ) + end + + def test_find_by_string_and_numeric_id + assert_same( + Client.find_by_id("3"), + Client.find_by_id(3) + ) + end + + def test_find_by_pkey + assert_same( + Subscriber.find_by_nick('swistak'), + Subscriber.find_by_nick('swistak') + ) + end + + def test_find_first_id + assert_same( + Client.find(:first, :conditions => {:id => 1}), + Client.find(:first, :conditions => {:id => 1}) + ) + end + + def test_find_first_pkey + assert_same( + Subscriber.find(:first, :conditions => {:nick => 'swistak'}), + Subscriber.find(:first, :conditions => {:nick => 'swistak'}) + ) + end + + ############################################################################## + # Tests checking if IM is functioning properly on more advanced finds # + # and associations # + ############################################################################## + + def test_owner_object_is_associated_from_identity_map + post = Post.find(1) + comment = post.comments.first + + assert_no_queries do + comment.post + end + assert_same post, comment.post + end + + def test_associated_object_are_assigned_from_identity_map + post = Post.find(1) + + post.comments.each do |comment| + assert_same post, comment.post + assert_equal post.object_id, comment.post.object_id + end + end + + def test_creation + t1 = Topic.create("title" => "t1") + t2 = Topic.find(t1.id) + assert_same(t1, t2) + end + + ############################################################################## + # Tests checking dirty attribute behaviour with IM # + ############################################################################## + + def test_loading_new_instance_should_not_update_dirty_attributes + swistak = Subscriber.find(:first, :conditions => {:nick => 'swistak'}) + swistak.name = "Swistak Sreberkowiec" + assert_equal(["name"], swistak.changed) + assert_equal({"name" => ["Marcin Raczkowski", "Swistak Sreberkowiec"]}, swistak.changes) + + s = Subscriber.find('swistak') + + assert swistak.name_changed? + assert_equal("Swistak Sreberkowiec", swistak.name) + end + + def test_loading_new_instance_should_change_dirty_attribute_original_value + swistak = Subscriber.find(:first, :conditions => {:nick => 'swistak'}) + swistak.name = "Swistak Sreberkowiec" + + Subscriber.update_all({:name => "Raczkowski Marcin"}, {:name => "Marcin Raczkowski"}) + + s = Subscriber.find('swistak') + + assert_equal({'name' => ["Raczkowski Marcin", "Swistak Sreberkowiec"]}, swistak.changes) + assert_equal("Swistak Sreberkowiec", swistak.name) + end + + def test_loading_new_instance_should_remove_dirt + swistak = Subscriber.find(:first, :conditions => {:nick => 'swistak'}) + swistak.name = "Swistak Sreberkowiec" + + assert_equal({"name" => ["Marcin Raczkowski", "Swistak Sreberkowiec"]}, swistak.changes) + + Subscriber.update_all({:name => "Swistak Sreberkowiec"}, {:name => "Marcin Raczkowski"}) + + s = Subscriber.find('swistak') + + assert_equal("Swistak Sreberkowiec", swistak.name) + assert_equal({}, swistak.changes) + assert !swistak.name_changed? + end + + def test_has_many_associations + pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") + pirate.birds.create!(:name => 'Posideons Killer') + pirate.birds.create!(:name => 'Killer bandita Dionne') + + posideons, killer = pirate.birds + + pirate.reload + + pirate.birds_attributes = [{ :id => posideons.id, :name => 'Grace OMalley' }] + assert_equal 'Grace OMalley', pirate.birds.to_a.find { |r| r.id == posideons.id }.name + end + + def test_changing_associations + post1 = Post.create("title" => "One post", "body" => "Posting...") + post2 = Post.create("title" => "Another post", "body" => "Posting... Again...") + comment = Comment.new("body" => "comment") + + comment.post = post1 + assert comment.save + + assert_same(post1.comments.first, comment) + + comment.post = post2 + assert comment.save + + assert_same(post2.comments.first, comment) + assert_equal(0, post1.comments.size) + end + + def test_im_with_polymorphic_has_many_going_through_join_model_with_custom_select_and_joins + tag = posts(:welcome).tags.first + tag_with_joins_and_select = posts(:welcome).tags.add_joins_and_select.first + assert_same(tag, tag_with_joins_and_select) + assert_nothing_raised(NoMethodError, "Joins/select was not loaded") { tag.author_id } + end + + ############################################################################## + # Tests checking Identity Map behaviour with preloaded associations, joins, # + # includes etc. # + ############################################################################## + + def test_find_with_preloaded_associations + assert_queries(2) do + posts = Post.preload(:comments) + assert posts.first.comments.first + end + + # With IM we'll retrieve post object from previous query, it'll have comments + # already preloaded from first call + assert_queries(1) do + posts = Post.preload(:comments).to_a + assert posts.first.comments.first + end + + assert_queries(2) do + posts = Post.preload(:author) + assert posts.first.author + end + + # With IM we'll retrieve post object from previous query, it'll have comments + # already preloaded from first call + assert_queries(1) do + posts = Post.preload(:author).to_a + assert posts.first.author + end + + assert_queries(1) do + posts = Post.preload(:author, :comments).to_a + assert posts.first.author + assert posts.first.comments.first + end + end + + def test_find_with_included_associations + assert_queries(2) do + posts = Post.includes(:comments) + assert posts.first.comments.first + end + + assert_queries(1) do + posts = Post.scoped.includes(:comments) + assert posts.first.comments.first + end + + assert_queries(2) do + posts = Post.includes(:author) + assert posts.first.author + end + + assert_queries(1) do + posts = Post.includes(:author, :comments).to_a + assert posts.first.author + assert posts.first.comments.first + end + end + + def test_eager_loading_with_conditions_on_joined_table_preloads + posts = Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => [:comments], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') + assert_equal [posts(:welcome)], posts + assert_equal authors(:david), assert_no_queries { posts[0].author} + assert_same posts.first.author, Author.first + + posts = Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => [:comments], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') + assert_equal [posts(:welcome)], posts + assert_equal authors(:david), assert_no_queries { posts[0].author} + assert_same posts.first.author, Author.first + + posts = Post.find(:all, :include => :author, :joins => {:taggings => :tag}, :conditions => "tags.name = 'General'", :order => 'posts.id') + assert_equal posts(:welcome, :thinking), posts + assert_same posts.first.author, Author.first + + posts = Post.find(:all, :include => :author, :joins => {:taggings => {:tag => :taggings}}, :conditions => "taggings_tags.super_tag_id=2", :order => 'posts.id') + assert_equal posts(:welcome, :thinking), posts + assert_same posts.first.author, Author.first + end + + def test_eager_loading_with_conditions_on_string_joined_table_preloads + posts = assert_queries(2) do + Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => "INNER JOIN comments on comments.post_id = posts.id", :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') + end + assert_equal [posts(:welcome)], posts + assert_equal authors(:david), assert_no_queries { posts[0].author} + + posts = assert_queries(1) do + Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => ["INNER JOIN comments on comments.post_id = posts.id"], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') + end + assert_equal [posts(:welcome)], posts + assert_equal authors(:david), assert_no_queries { posts[0].author} + end + + ############################################################################## + # Behaviour releated to saving failures + ############################################################################## + + def test_reload_object_if_save_failed + developer = Developer.first + developer.salary = 0 + + assert !developer.save + + same_developer = Developer.first + + assert_not_same developer, same_developer + assert_not_equal 0, same_developer.salary + assert_not_equal developer.salary, same_developer.salary + end + + def test_reload_object_if_forced_save_failed + developer = Developer.first + developer.salary = 0 + + assert_raise(ActiveRecord::RecordInvalid) { developer.save! } + + same_developer = Developer.first + + assert_not_same developer, same_developer + assert_not_equal 0, same_developer.salary + assert_not_equal developer.salary, same_developer.salary + end + + def test_reload_object_if_update_attributes_fails + developer = Developer.first + developer.salary = 0 + + assert !developer.update_attributes(:salary => 0) + + same_developer = Developer.first + + assert_not_same developer, same_developer + assert_not_equal 0, same_developer.salary + assert_not_equal developer.salary, same_developer.salary + end + + ############################################################################## + # Behaviour of readonly, forzen, destroyed + ############################################################################## + + def test_find_using_identity_map_respects_readonly_when_loading_associated_object_first + author = Author.first + readonly_comment = author.readonly_comments.first + + comment = Comment.first + assert !comment.readonly? + + assert readonly_comment.readonly? + + assert_raise(ActiveRecord::ReadOnlyRecord) {readonly_comment.save} + assert comment.save + end + + def test_find_using_identity_map_respects_readonly + comment = Comment.first + assert !comment.readonly? + + author = Author.first + readonly_comment = author.readonly_comments.first + + assert readonly_comment.readonly? + + assert_raise(ActiveRecord::ReadOnlyRecord) {readonly_comment.save} + assert comment.save + end + + def test_find_using_select_and_identity_map + author_id, author = Author.select('id').first, Author.first + + assert_equal author_id, author + assert_same author_id, author + assert_not_nil author.name + + post, post_id = Post.first, Post.select('id').first + + assert_equal post_id, post + assert_same post_id, post + assert_not_nil post.title + end + +# Currently AR is not allowing changing primary key (see Persistence#update) +# So we ignore it. If this changes, this test needs to be uncommented. +# def test_updating_of_pkey +# assert client = Client.find(3), +# client.update_attribute(:id, 666) +# +# assert Client.find(666) +# assert_same(client, Client.find(666)) +# +# s = Subscriber.find_by_nick('swistak') +# assert s.update_attribute(:nick, 'swistakTheJester') +# assert_equal('swistakTheJester', s.nick) +# +# assert stj = Subscriber.find_by_nick('swistakTheJester') +# assert_same(s, stj) +# end + +end +end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 2a72838d06..636a709924 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -1,6 +1,7 @@ require 'thread' require "cases/helper" require 'models/person' +require 'models/job' require 'models/reader' require 'models/legacy_thing' require 'models/reference' @@ -98,6 +99,14 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 1, p1.lock_version end + def test_touch_existing_lock + p1 = Person.find(1) + assert_equal 0, p1.lock_version + + p1.touch + assert_equal 1, p1.lock_version + end + def test_lock_column_name_existing t1 = LegacyThing.find(1) diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index e3ba65b4fa..7e8383da9e 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -227,7 +227,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_create_with_join_and_merge - (Comment.where(:body => "but Who's Buying?").joins(:post) & Post.where(:body => 'Peace Sells...')).with_scope do + Comment.where(:body => "but Who's Buying?").joins(:post).merge(Post.where(:body => 'Peace Sells...')).with_scope do assert_equal({:body => "but Who's Buying?"}, Comment.scoped.scope_for_create) end end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index ed5e1e0cba..d05b0ff947 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -448,7 +448,7 @@ class NamedScopeTest < ActiveRecord::TestCase [:destroy_all, :reset, :delete_all].each do |method| before = post.comments.containing_the_letter_e - post.comments.send(method) + post.association(:comments).send(method) assert before.object_id != post.comments.containing_the_letter_e.object_id, "AssociationCollection##{method} should reset the named scopes cache" end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index d1afe7376a..c57ab7ed28 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -155,7 +155,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase man = Man.find man.id man.interests_attributes = [{:id => interest.id, :topic => 'gardening'}] assert_equal man.interests.first.topic, man.interests[0].topic - end + end end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase @@ -918,16 +918,16 @@ class TestHasManyAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveR test "if association is not loaded and association record is saved and then in memory record attributes should be saved" do @ship.parts_attributes=[{:id => @part.id,:name =>'Deck'}] - assert_equal 1, @ship.parts.proxy_target.size + assert_equal 1, @ship.association(:parts).target.size assert_equal 'Deck', @ship.parts[0].name end test "if association is not loaded and child doesn't change and I am saving a grandchild then in memory record should be used" do @ship.parts_attributes=[{:id => @part.id,:trinkets_attributes =>[{:id => @trinket.id, :name => 'Ruby'}]}] - assert_equal 1, @ship.parts.proxy_target.size + assert_equal 1, @ship.association(:parts).target.size assert_equal 'Mast', @ship.parts[0].name - assert_no_difference("@ship.parts[0].trinkets.proxy_target.size") do - @ship.parts[0].trinkets.proxy_target.size + assert_no_difference("@ship.parts[0].association(:trinkets).target.size") do + @ship.parts[0].association(:trinkets).target.size end assert_equal 'Ruby', @ship.parts[0].trinkets[0].name @ship.save diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index 1bdf3136d4..cda2850b02 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -488,8 +488,8 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_create_with_merge - aaron = (PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20) & - PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new + aaron = PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20).merge( + PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new assert_equal 20, aaron.salary assert_equal 'Aaron', aaron.name diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 5018b16b67..37bbb17e74 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -285,7 +285,7 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.comments.first end - assert_queries(2) do + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do posts = Post.preload(:comments).to_a assert posts.first.comments.first end @@ -295,12 +295,12 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.author end - assert_queries(2) do + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do posts = Post.preload(:author).to_a assert posts.first.author end - assert_queries(3) do + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 3) do posts = Post.preload(:author, :comments).to_a assert posts.first.author assert posts.first.comments.first @@ -313,7 +313,7 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.comments.first end - assert_queries(2) do + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do posts = Post.scoped.includes(:comments) assert posts.first.comments.first end @@ -323,7 +323,7 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.author end - assert_queries(3) do + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 3) do posts = Post.includes(:author, :comments).to_a assert posts.first.author assert posts.first.comments.first @@ -474,10 +474,17 @@ class RelationTest < ActiveRecord::TestCase relation = relation.where(:name => david.name) relation = relation.where(:name => 'Santiago') relation = relation.where(:id => david.id) - assert_equal [david], relation.all + assert_equal [], relation.all end - def test_find_all_with_multiple_ors + def test_multi_where_ands_queries + relation = Author.unscoped + david = authors(:david) + sql = relation.where(:name => david.name).where(:name => 'Santiago').to_sql + assert_match('AND', sql) + end + + def test_find_all_with_multiple_should_use_and david = authors(:david) relation = [ { :name => david.name }, @@ -486,7 +493,34 @@ class RelationTest < ActiveRecord::TestCase ].inject(Author.unscoped) do |memo, param| memo.where(param) end - assert_equal [david], relation.all + assert_equal [], relation.all + end + + def test_find_all_using_where_with_relation + david = authors(:david) + # switching the lines below would succeed in current rails + # assert_queries(2) { + assert_queries(1) { + relation = Author.where(:id => Author.where(:id => david.id)) + assert_equal [david], relation.all + } + end + + def test_find_all_using_where_with_relation_with_joins + david = authors(:david) + assert_queries(1) { + relation = Author.where(:id => Author.joins(:posts).where(:id => david.id)) + assert_equal [david], relation.all + } + end + + + def test_find_all_using_where_with_relation_with_select_to_build_subquery + david = authors(:david) + assert_queries(1) { + relation = Author.where(:name => Author.where(:id => david.id).select(:name)) + assert_equal [david], relation.all + } end def test_exists @@ -545,17 +579,17 @@ class RelationTest < ActiveRecord::TestCase end def test_relation_merging - devs = Developer.where("salary >= 80000") & Developer.limit(2) & Developer.order('id ASC').where("id < 3") + devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order('id ASC').where("id < 3")) assert_equal [developers(:david), developers(:jamis)], devs.to_a - dev_with_count = Developer.limit(1) & Developer.order('id DESC') & Developer.select('developers.*') + dev_with_count = Developer.limit(1).merge(Developer.order('id DESC')).merge(Developer.select('developers.*')) assert_equal [developers(:poor_jamis)], dev_with_count.to_a end def test_relation_merging_with_eager_load relations = [] - relations << (Post.order('comments.id DESC') & Post.eager_load(:last_comment) & Post.scoped) - relations << (Post.eager_load(:last_comment) & Post.order('comments.id DESC') & Post.scoped) + relations << Post.order('comments.id DESC').merge(Post.eager_load(:last_comment)).merge(Post.scoped) + relations << Post.eager_load(:last_comment).merge(Post.order('comments.id DESC')).merge(Post.scoped) relations.each do |posts| post = posts.find { |p| p.id == 1 } @@ -564,18 +598,20 @@ class RelationTest < ActiveRecord::TestCase end def test_relation_merging_with_locks - devs = Developer.lock.where("salary >= 80000").order("id DESC") & Developer.limit(2) + devs = Developer.lock.where("salary >= 80000").order("id DESC").merge(Developer.limit(2)) assert_present devs.locked end def test_relation_merging_with_preload - [Post.scoped & Post.preload(:author), Post.preload(:author) & Post.scoped].each do |posts| - assert_queries(2) { assert posts.first.author } + ActiveRecord::IdentityMap.without do + [Post.scoped.merge(Post.preload(:author)), Post.preload(:author).merge(Post.scoped)].each do |posts| + assert_queries(2) { assert posts.first.author } + end end end def test_relation_merging_with_joins - comments = Comment.joins(:post).where(:body => 'Thank you for the welcome') & Post.where(:body => 'Such a lovely day') + comments = Comment.joins(:post).where(:body => 'Thank you for the welcome').merge(Post.where(:body => 'Such a lovely day')) assert_equal 1, comments.count end diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index f817493190..07069a064f 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -5,6 +5,7 @@ welcome: body: Such a lovely day comments_count: 2 taggings_count: 1 + tags_count: 1 type: Post thinking: @@ -14,6 +15,7 @@ thinking: body: Like I hopefully always am comments_count: 1 taggings_count: 1 + tags_count: 1 type: SpecialPost authorless: diff --git a/activerecord/test/fixtures/subscribers.yml b/activerecord/test/fixtures/subscribers.yml index 9ffb4a156f..c6a8c2fa24 100644 --- a/activerecord/test/fixtures/subscribers.yml +++ b/activerecord/test/fixtures/subscribers.yml @@ -5,3 +5,7 @@ first: second: nick: webster132 name: David Heinemeier Hansson + +thrid: + nick: swistak + name: Marcin Raczkowski
\ No newline at end of file diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 3e219fbe4a..e0b30efd51 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -48,19 +48,16 @@ class Firm < Company has_many :dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :destroy has_many :exclusively_dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all has_many :limited_clients, :class_name => "Client", :limit => 1 + has_many :clients_with_interpolated_conditions, :class_name => "Client", :conditions => proc { "rating > #{rating}" } has_many :clients_like_ms, :conditions => "name = 'Microsoft'", :class_name => "Client", :order => "id" has_many :clients_like_ms_with_hash_conditions, :conditions => { :name => 'Microsoft' }, :class_name => "Client", :order => "id" - has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' - has_many :clients_using_multiline_sql, :class_name => "Client", :finder_sql => ' - SELECT - companies.* - FROM companies WHERE companies.client_of = #{id}' + has_many :clients_using_sql, :class_name => "Client", :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" } has_many :clients_using_counter_sql, :class_name => "Client", - :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}', - :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = #{id}' + :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id} " }, + :counter_sql => proc { "SELECT COUNT(*) FROM companies WHERE client_of = #{id}" } has_many :clients_using_zero_counter_sql, :class_name => "Client", - :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}', - :counter_sql => 'SELECT 0 FROM companies WHERE client_of = #{id}' + :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" }, + :counter_sql => proc { "SELECT 0 FROM companies WHERE client_of = #{id}" } has_many :no_clients_using_counter_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = 1000', :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000' diff --git a/activerecord/test/models/joke.rb b/activerecord/test/models/joke.rb index 3978abc2ba..d7f01e59e6 100644 --- a/activerecord/test/models/joke.rb +++ b/activerecord/test/models/joke.rb @@ -1,3 +1,7 @@ class Joke < ActiveRecord::Base set_table_name 'funny_jokes' end + +class GoodJoke < ActiveRecord::Base + set_table_name 'funny_jokes' +end diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index bee89de042..cc3a4f5f9d 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -1,15 +1,21 @@ class Person < ActiveRecord::Base has_many :readers + has_one :reader + has_many :posts, :through => :readers has_many :posts_with_no_comments, :through => :readers, :source => :post, :include => :comments, :conditions => 'comments.id is null' has_many :references has_many :bad_references has_many :fixed_bad_references, :conditions => { :favourite => true }, :class_name => 'BadReference' - has_many :jobs, :through => :references has_one :favourite_reference, :class_name => 'Reference', :conditions => ['favourite=?', true] has_many :posts_with_comments_sorted_by_comment_id, :through => :readers, :source => :post, :include => :comments, :order => 'comments.id' + has_many :jobs, :through => :references + has_many :jobs_with_dependent_destroy, :source => :job, :through => :references, :dependent => :destroy + has_many :jobs_with_dependent_delete_all, :source => :job, :through => :references, :dependent => :delete_all + has_many :jobs_with_dependent_nullify, :source => :job, :through => :references, :dependent => :nullify + belongs_to :primary_contact, :class_name => 'Person' has_many :agents, :class_name => 'Person', :foreign_key => 'primary_contact_id' has_many :agents_of_agents, :through => :agents, :source => :agents @@ -18,3 +24,24 @@ class Person < ActiveRecord::Base scope :males, :conditions => { :gender => 'M' } scope :females, :conditions => { :gender => 'F' } end + +class PersonWithDependentDestroyJobs < ActiveRecord::Base + self.table_name = 'people' + + has_many :references, :foreign_key => :person_id + has_many :jobs, :source => :job, :through => :references, :dependent => :destroy +end + +class PersonWithDependentDeleteAllJobs < ActiveRecord::Base + self.table_name = 'people' + + has_many :references, :foreign_key => :person_id + has_many :jobs, :source => :job, :through => :references, :dependent => :delete_all +end + +class PersonWithDependentNullifyJobs < ActiveRecord::Base + self.table_name = 'people' + + has_many :references, :foreign_key => :person_id + has_many :jobs, :source => :job, :through => :references, :dependent => :nullify +end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 1c95d30d6b..a342aaf60b 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -39,6 +39,10 @@ class Post < ActiveRecord::Base has_many :author_favorites, :through => :author has_many :author_categorizations, :through => :author, :source => :categorizations + has_many :author_addresses, :through => :author + + has_many :comments_with_interpolated_conditions, :class_name => 'Comment', + :conditions => proc { ["#{"#{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 @@ -56,7 +60,15 @@ class Post < ActiveRecord::Base end end + has_many :interpolated_taggings, :class_name => 'Tagging', :as => :taggable, :conditions => proc { "1 = #{1}" } + has_many :interpolated_tags, :through => :taggings + has_many :interpolated_tags_2, :through => :interpolated_taggings, :source => :tag + has_many :taggings_with_delete_all, :class_name => 'Tagging', :as => :taggable, :dependent => :delete_all + has_many :taggings_with_destroy, :class_name => 'Tagging', :as => :taggable, :dependent => :destroy + + has_many :tags_with_destroy, :through => :taggings, :source => :tag, :dependent => :destroy + has_many :tags_with_nullify, :through => :taggings, :source => :tag, :dependent => :nullify has_many :misc_tags, :through => :taggings, :source => :tag, :conditions => "tags.name = 'Misc'" has_many :funky_tags, :through => :taggings, :source => :tag @@ -78,10 +90,12 @@ class Post < ActiveRecord::Base has_many :standard_categorizations, :class_name => 'Categorization', :foreign_key => :post_id has_many :author_using_custom_pk, :through => :standard_categorizations has_many :authors_using_custom_pk, :through => :standard_categorizations + has_many :named_categories, :through => :standard_categorizations has_many :readers has_many :readers_with_person, :include => :person, :class_name => "Reader" has_many :people, :through => :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) }, :after_add => lambda {|owner, reader| log(:added, :after, reader.first_name) }, diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb index 8a53a8f803..efe1ce67da 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -7,14 +7,15 @@ class Project < ActiveRecord::Base has_and_belongs_to_many :developers_named_david, :class_name => "Developer", :conditions => "name = 'David'", :uniq => true has_and_belongs_to_many :developers_named_david_with_hash_conditions, :class_name => "Developer", :conditions => { :name => 'David' }, :uniq => true has_and_belongs_to_many :salaried_developers, :class_name => "Developer", :conditions => "salary > 0" - has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => 'SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id' - has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => ' - SELECT - t.*, j.* - FROM - developers_projects j, - developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id' - has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => "DELETE FROM developers_projects WHERE project_id = \#{id} AND developer_id = \#{record.id}" + has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => proc { "SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" } + has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => proc { + "SELECT + t.*, j.* + FROM + developers_projects j, + developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" + } + has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => proc { |record| "DELETE FROM developers_projects WHERE project_id = #{id} AND developer_id = #{record.id}" } has_and_belongs_to_many :developers_with_callbacks, :class_name => "Developer", :before_add => Proc.new {|o, r| o.developers_log << "before_adding#{r.id || '<new>'}"}, :after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || '<new>'}"}, :before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"}, diff --git a/activerecord/test/models/reader.rb b/activerecord/test/models/reader.rb index 27527bf566..0207a2bd92 100644 --- a/activerecord/test/models/reader.rb +++ b/activerecord/test/models/reader.rb @@ -1,4 +1,5 @@ class Reader < ActiveRecord::Base belongs_to :post - belongs_to :person + belongs_to :person, :inverse_of => :readers + belongs_to :single_person, :class_name => 'Person', :foreign_key => :person_id, :inverse_of => :reader end diff --git a/activerecord/test/models/reference.rb b/activerecord/test/models/reference.rb index 4a17c936f5..06c4f79ef3 100644 --- a/activerecord/test/models/reference.rb +++ b/activerecord/test/models/reference.rb @@ -1,6 +1,18 @@ class Reference < ActiveRecord::Base belongs_to :person belongs_to :job + + class << self + attr_accessor :make_comments + end + + before_destroy :make_comments + + def make_comments + if self.class.make_comments + person.update_attributes :comments => "Reference destroyed" + end + end end class BadReference < ActiveRecord::Base diff --git a/activerecord/test/models/tagging.rb b/activerecord/test/models/tagging.rb index 33ffc623d7..231d2b5890 100644 --- a/activerecord/test/models/tagging.rb +++ b/activerecord/test/models/tagging.rb @@ -6,6 +6,7 @@ class Tagging < ActiveRecord::Base belongs_to :tag, :include => :tagging 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 :interpolated_tag, :class_name => 'Tag', :foreign_key => :tag_id, :conditions => proc { "1 = #{1}" } belongs_to :taggable, :polymorphic => true, :counter_cache => true has_many :things, :through => :taggable end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 326c336317..0b3865fc78 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -229,6 +229,10 @@ ActiveRecord::Schema.define do t.string :name end + create_table :cold_jokes, :force => true do |t| + t.string :name + end + create_table :goofy_string_id, :force => true, :id => false do |t| t.string :id, :null => false t.string :info @@ -425,6 +429,8 @@ ActiveRecord::Schema.define do t.string :gender, :limit => 1 t.references :number1_fan t.integer :lock_version, :null => false, :default => 0 + t.string :comments + t.timestamps end create_table :pets, :primary_key => :pet_id ,:force => true do |t| @@ -454,6 +460,10 @@ ActiveRecord::Schema.define do t.integer :comments_count, :default => 0 t.integer :taggings_count, :default => 0 t.integer :taggings_with_delete_all_count, :default => 0 + t.integer :taggings_with_destroy_count, :default => 0 + t.integer :tags_count, :default => 0 + t.integer :tags_with_destroy_count, :default => 0 + t.integer :tags_with_nullify_count, :default => 0 end create_table :price_estimates, :force => true do |t| |