diff options
35 files changed, 929 insertions, 49 deletions
diff --git a/.gitignore b/.gitignore index a18fba3780..81403942c9 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ railties/test/initializer/root/log railties/doc railties/guides/output railties/tmp +nbproject diff --git a/activerecord/examples/performance.rb b/activerecord/examples/performance.rb index c4ce361b5d..c97ef5b41f 100644 --- a/activerecord/examples/performance.rb +++ b/activerecord/examples/performance.rb @@ -92,10 +92,17 @@ else `#{mysqldump_bin} -u #{conn[:username]} #{"-p#{conn[:password]}" unless conn[:password].blank?} #{conn[:database]} exhibits users > #{sqlfile}` end +ActiveRecord::IdentityMap.enabled = true unless ENV['IM'] == "disabled" + +def clear_identity_map! + ActiveRecord::IdentityMap.clear +end + RBench.run(TIMES) do column :times column :ar + report 'Model#id', (TIMES * 100).ceil do ar_obj = Exhibit.find(1) @@ -112,18 +119,22 @@ RBench.run(TIMES) do end report 'Model.first' do + clear_identity_map! ar { Exhibit.first.look } end report 'Model.all limit(100)', (TIMES / 10).ceil do + clear_identity_map! ar { Exhibit.look Exhibit.limit(100) } end report 'Model.all limit(100) with relationship', (TIMES / 10).ceil do + clear_identity_map! ar { Exhibit.feel Exhibit.limit(100).includes(:user) } end report 'Model.all limit(10,000)', (TIMES / 1000).ceil do + clear_identity_map! ar { Exhibit.look Exhibit.limit(10000) } end @@ -134,41 +145,50 @@ RBench.run(TIMES) do } report 'Model.create' do + clear_identity_map! ar { Exhibit.create(exhibit) } end report 'Resource#attributes=' do + clear_identity_map! attrs_first = { :name => 'sam' } attrs_second = { :name => 'tom' } ar { exhibit = Exhibit.new(attrs_first); exhibit.attributes = attrs_second } end report 'Resource#update' do + clear_identity_map! ar { Exhibit.first.update_attributes(:name => 'bob') } end report 'Resource#destroy' do + clear_identity_map! ar { Exhibit.first.destroy } end report 'Model.transaction' do + clear_identity_map! ar { Exhibit.transaction { Exhibit.new } } end report 'Model.find(id)' do + clear_identity_map! id = Exhibit.first.id ar { Exhibit.find(id) } end report 'Model.find_by_sql' do + clear_identity_map! ar { Exhibit.find_by_sql("SELECT * FROM exhibits WHERE id = #{(rand * 1000 + 1).to_i}").first } end report 'Model.log', (TIMES * 10) do + clear_identity_map! ar { Exhibit.connection.send(:log, "hello", "world") {} } end report 'AR.execute(query)', (TIMES / 2) do + clear_identity_map! ar { ActiveRecord::Base.connection.execute("Select * from exhibits where id = #{(rand * 1000 + 1).to_i}") } end diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index c80bce2849..83e20af3d0 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 AttributeMethods diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index f2094283a2..373532704f 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -126,7 +126,7 @@ module ActiveRecord parent_records.each do |parent_record| association_proxy = parent_record.send(reflection_name) association_proxy.loaded - association_proxy.target.push(*Array.wrap(associated_record)) + association_proxy.target = association_proxy.target | [*Array.wrap(associated_record)] association_proxy.__send__(:set_inverse_instance, associated_record, parent_record) end @@ -199,8 +199,10 @@ module ActiveRecord select("#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id"). order(options[:order]) - all_associated_records = associated_records(ids) do |some_ids| - associated_records_proxy.where([conditions, ids]).to_a + all_associated_records = ActiveRecord::IdentityMap.without do + associated_records(ids) do |some_ids| + associated_records_proxy.where([conditions, ids]).to_a + end end set_association_collection_records(id_to_record_map, reflection.name, all_associated_records, 'the_parent_record_id') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index cdc8f25119..371dad9104 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -119,6 +119,12 @@ module ActiveRecord # Clears out the association cache. def clear_association_cache #:nodoc: self.class.reflect_on_all_associations.to_a.each do |assoc| + if IdentityMap.enabled? && instance_variable_defined?("@#{assoc.name}") + Array(instance_variable_get("@#{assoc.name}")).each do |t| + next unless t.respond_to?(:target) + IdentityMap.remove t.target unless t.target.nil? + end + end instance_variable_set "@#{assoc.name}", nil end if self.persisted? end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 252ff7e7ea..53ec5a0da6 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -110,6 +110,7 @@ module ActiveRecord # Resets the \loaded flag to +false+ and sets the \target to +nil+. def reset @loaded = false + IdentityMap.remove(@target) if defined?(@target) && @target && IdentityMap.enabled? @target = nil end @@ -253,7 +254,10 @@ module ActiveRecord return nil unless defined?(@loaded) if !loaded? && (!@owner.new_record? || foreign_key_present) - @target = find_target + if IdentityMap.enabled? && association_class + @target = IdentityMap.get(association_class, @owner[@reflection.association_foreign_key]) + end + @target ||= find_target end @loaded = true @@ -309,6 +313,10 @@ module ActiveRecord def we_can_set_the_inverse_on_this?(record) false end + + def association_class + @reflection.klass + end end end end 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/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 4a18719551..863f64eb3a 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 @@ -303,22 +320,25 @@ module ActiveRecord autosave = reflection.options[:autosave] if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) - records.each do |record| - next if record.destroyed? - - if autosave && record.marked_for_destruction? - association.destroy(record) - elsif autosave != false && (@new_record_before_save || record.new_record?) - if autosave - saved = association.send(:insert_record, record, false, false) - else - association.send(:insert_record, record) + begin + records.each do |record| + next if record.destroyed? + + if autosave && record.marked_for_destruction? + association.destroy(record) + elsif autosave != false && (@new_record_before_save || record.new_record?) + if autosave + saved = association.send(:insert_record, record, false, false) + else + association.send(:insert_record, record) + end + + raise ActiveRecord::Rollback if saved == false end - elsif autosave - saved = record.save(:validate => false) end - - raise ActiveRecord::Rollback if saved == false + rescue + records.each {|x| IdentityMap.remove(x) } if IdentityMap.enabled? + raise end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 858ccebbfa..24c87662b8 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -790,6 +790,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. @@ -881,9 +885,24 @@ module ActiveRecord #:nodoc: # single-table inheritance model that makes it possible to create # objects of different types from the same table. def instantiate(record) - model = find_sti_class(record[inheritance_column]).allocate - model.init_with('attributes' => record) - model + 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 def find_sti_class(type_name) @@ -1399,6 +1418,8 @@ MSG @new_record = false _run_find_callbacks _run_initialize_callbacks + + self end # Returns a String, which Action Pack uses for constructing an URL to this @@ -1851,6 +1872,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/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 6fb723f2f5..b46310f8e0 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -892,7 +892,9 @@ module ActiveRecord @fixture_cache[table_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 + ActiveRecord::IdentityMap.without do + @fixture_cache[table_name][fixture] ||= @loaded_fixtures[table_name][fixture.to_s].find + end else raise StandardError, "No fixture with name '#{fixture}' found for table '#{table_name}'" 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..438287d3ea --- /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 + + module InstanceMethods + # 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]} + + _run_find_callbacks + + self + end + 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/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 050b521b6a..0a073ad6c8 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -403,11 +403,17 @@ 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 } - association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? && !call_reject_if(association_name, attributes) - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) - + 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 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 9ac8fcb176..78e84c73ee 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 @@ -272,6 +281,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 2accf0a48f..a634153d6f 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/relation.rb b/activerecord/lib/active_record/relation.rb index 7ecba1c43a..c6943444a4 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -73,7 +73,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/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 443f318067..de496698f1 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/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_test.rb b/activerecord/test/cases/associations/eager_test.rb index d5262b1ee4..c064731859 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -184,7 +184,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(2) { 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 @@ -195,7 +195,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(2) { 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 @@ -803,18 +803,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(1) 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(1) 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(1) 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 @@ -828,7 +828,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(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 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..3d3ae0594b --- /dev/null +++ b/activerecord/test/cases/associations/identity_map_test.rb @@ -0,0 +1,135 @@ +require "cases/helper" +require 'models/author' +require 'models/post' + +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.target + 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.replace([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 diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 0a57c7883f..d6e772e989 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -94,7 +94,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 diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 27aee400f9..0fd1e62134 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -1207,6 +1207,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 @@ -1215,7 +1216,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/helper.rb b/activerecord/test/cases/helper.rb index f9bbc5299b..942a5c048e 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 = true + def current_adapter?(*types) types.any? do |type| ActiveRecord::ConnectionAdapters.const_defined?(type) && diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb new file mode 100644 index 0000000000..74d4cb0bfb --- /dev/null +++ b/activerecord/test/cases/identity_map_test.rb @@ -0,0 +1,400 @@ +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" + +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.target + end + + def test_associated_object_are_assigned_from_identity_map + post = Post.find(1) + + post.comments.each do |comment| + assert_same post, comment.post.target + assert_equal post.object_id, comment.post.target.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.send(:load_target).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.target, 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.target, 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.target, 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.target, 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 diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 20bfafbc5e..b3d4305f7c 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -279,7 +279,7 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.comments.first end - assert_queries(2) do + assert_queries(1) do posts = Post.preload(:comments).to_a assert posts.first.comments.first end @@ -289,12 +289,12 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.author end - assert_queries(2) do + assert_queries(1) do posts = Post.preload(:author).to_a assert posts.first.author end - assert_queries(3) do + assert_queries(1) do posts = Post.preload(:author, :comments).to_a assert posts.first.author assert posts.first.comments.first @@ -307,7 +307,7 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.comments.first end - assert_queries(2) do + assert_queries(1) do posts = Post.scoped.includes(:comments) assert posts.first.comments.first end @@ -317,7 +317,7 @@ class RelationTest < ActiveRecord::TestCase assert posts.first.author end - assert_queries(3) do + assert_queries(1) do posts = Post.includes(:author, :comments).to_a assert posts.first.author assert posts.first.comments.first @@ -563,9 +563,11 @@ class RelationTest < ActiveRecord::TestCase end def test_relation_merging_with_preload + ActiveRecord::IdentityMap.without do [Post.scoped & Post.preload(:author), Post.preload(:author) & Post.scoped].each do |posts| assert_queries(2) { assert posts.first.author } end + end end def test_relation_merging_with_joins 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/activesupport/lib/active_support/weak_hash.rb b/activesupport/lib/active_support/weak_hash.rb new file mode 100644 index 0000000000..d6096e606e --- /dev/null +++ b/activesupport/lib/active_support/weak_hash.rb @@ -0,0 +1,41 @@ +module ActiveSupport + if defined?(RUBY_ENGINE) && RUBY_ENGINE == 'jruby' + WeakHash = ::Weakling::WeakHash + else + class WeakHash + def initialize(cache = Hash.new) + @cache = cache + @key_map = {} + @rev_cache = Hash.new{|h,k| h[k] = {}} + @reclaim_value = lambda do |value_id| + if value = @rev_cache.delete(value_id) + value.each_key{|key| @cache.delete key} + end + end + end + + def [](key) + value_id = @cache[key] + value_id && ObjectSpace._id2ref(value_id) + rescue RangeError + nil + end + + def []=(key, value) + @rev_cache[value.object_id][key] = true + @cache[key] = value.object_id + @key_map[key.object_id] = key + + ObjectSpace.define_finalizer(value, @reclaim_value) + end + + def clear + @cache.clear + end + + def delete(key) + @cache.delete(key) + end + end + end +end diff --git a/activesupport/test/weak_hash_test.rb b/activesupport/test/weak_hash_test.rb new file mode 100644 index 0000000000..7d1620dc34 --- /dev/null +++ b/activesupport/test/weak_hash_test.rb @@ -0,0 +1,33 @@ +require 'abstract_unit' +require 'active_support/weak_hash' + +class WeakHashTest < ActiveSupport::TestCase + + def setup + @weak_hash = ActiveSupport::WeakHash.new + @str = "A"; + @obj = Object.new + end + + test "allows us to assign value, and return assigned value" do + a = @str; b = @obj + assert_equal @weak_hash[a] = b, b + end + + test "should allow us to assign and read value" do + a = @str; b = @obj + assert_equal @weak_hash[a] = b, b + assert_equal @weak_hash[a], b + end + + test "should use object_id to identify objects" do + a = Object.new + @weak_hash[a] = "b" + assert_nil @weak_hash[a.dup] + end + + test "should find objects that have same hash" do + @weak_hash["a"] = "b" + assert_equal "b", @weak_hash["a"] + end +end diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb b/railties/lib/rails/generators/rails/app/templates/config/application.rb index 6e515756fe..b7f64af339 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb @@ -57,5 +57,10 @@ module <%= app_const_base %> # Configure sensitive parameters which will be filtered from the log file. config.filter_parameters += [:password] + +<% unless options[:skip_active_record] -%> + # Enable IdentityMap for Active Record, to disable set to false or remove the line below. + config.active_record.identity_map = true +<% end -%> end end diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index f81002328f..00029e627e 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -19,6 +19,10 @@ if defined?(ActiveRecord) class ActiveSupport::TestCase include ActiveRecord::TestFixtures self.fixture_path = "#{Rails.root}/test/fixtures/" + + setup do + ActiveRecord::IdentityMap.clear + end end ActionDispatch::IntegrationTest.fixture_path = ActiveSupport::TestCase.fixture_path diff --git a/railties/test/application/initializers/frameworks_test.rb b/railties/test/application/initializers/frameworks_test.rb index 475091f789..19311a7fa0 100644 --- a/railties/test/application/initializers/frameworks_test.rb +++ b/railties/test/application/initializers/frameworks_test.rb @@ -1,7 +1,7 @@ require "isolation/abstract_unit" module ApplicationTests - class FrameworlsTest < Test::Unit::TestCase + class FrameworksTest < Test::Unit::TestCase include ActiveSupport::Testing::Isolation def setup @@ -166,7 +166,7 @@ module ApplicationTests require "#{app_path}/config/environment" - expects = [ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActiveRecord::SessionStore] + expects = [ActiveRecord::IdentityMap::Middleware, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActiveRecord::SessionStore] middleware = Rails.application.config.middleware.map { |m| m.klass } assert_equal expects, middleware & expects end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index a2217888e4..d88bd05a74 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -29,6 +29,7 @@ module ApplicationTests "Rack::Sendfile", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", + "ActiveRecord::IdentityMap::Middleware", "ActiveRecord::ConnectionAdapters::ConnectionManagement", "ActiveRecord::QueryCache", "ActionDispatch::Cookies", @@ -56,6 +57,7 @@ module ApplicationTests boot! assert !middleware.include?("ActiveRecord::ConnectionAdapters::ConnectionManagement") assert !middleware.include?("ActiveRecord::QueryCache") + assert !middleware.include?("ActiveRecord::IdentityMap::Middleware") end test "removes lock if allow concurrency is set" do @@ -112,6 +114,11 @@ module ApplicationTests assert_equal "Rack::Runtime", middleware.fourth end + test "identity map is inserted" do + boot! + assert_equal "ActiveRecord::IdentityMap::Middleware", middleware[9] + end + test "insert middleware before" do add_to_config "config.middleware.insert_before ActionDispatch::Static, Rack::Config" boot! diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 3b03e4eb3d..c5b1cb9a80 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -215,6 +215,13 @@ module TestHelpers end end + def remove_from_config(str) + file = "#{app_path}/config/application.rb" + contents = File.read(file) + contents.sub!(/#{str}/, "") + File.open(file, "w+") { |f| f.puts contents } + end + def app_file(path, contents) FileUtils.mkdir_p File.dirname("#{app_path}/#{path}") File.open("#{app_path}/#{path}", 'w') do |f| @@ -231,6 +238,7 @@ module TestHelpers :activemodel, :activerecord, :activeresource] - arr + remove_from_config "config.active_record.identity_map = true" if to_remove.include? :activerecord $:.reject! {|path| path =~ %r'/(#{to_remove.join('|')})/' } end |