diff options
Diffstat (limited to 'activerecord/lib/active_record')
| -rw-r--r-- | activerecord/lib/active_record/association_preload.rb | 8 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations.rb | 6 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/association_proxy.rb | 10 | ||||
| -rw-r--r-- | activerecord/lib/active_record/attribute_methods/dirty.rb | 5 | ||||
| -rw-r--r-- | activerecord/lib/active_record/autosave_association.rb | 54 | ||||
| -rw-r--r-- | activerecord/lib/active_record/base.rb | 28 | ||||
| -rw-r--r-- | activerecord/lib/active_record/counter_cache.rb | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/fixtures.rb | 4 | ||||
| -rw-r--r-- | activerecord/lib/active_record/identity_map.rb | 102 | ||||
| -rw-r--r-- | activerecord/lib/active_record/nested_attributes.rb | 14 | ||||
| -rw-r--r-- | activerecord/lib/active_record/persistence.rb | 14 | ||||
| -rw-r--r-- | activerecord/lib/active_record/railtie.rb | 5 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation.rb | 8 | ||||
| -rw-r--r-- | activerecord/lib/active_record/test_case.rb | 10 | ||||
| -rw-r--r-- | activerecord/lib/active_record/transactions.rb | 1 |
15 files changed, 239 insertions, 32 deletions
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 |