Permalink
Browse files

Remove IdentityMap

  • Loading branch information...
1 parent c1f397f commit a8dd21d8b459ce4d57160a359798c577085a95e9 @carlosantoniodasilva carlosantoniodasilva committed Mar 2, 2012
Showing with 36 additions and 966 deletions.
  1. +2 −0 activerecord/CHANGELOG.md
  2. +0 −7 activerecord/RUNNING_UNIT_TESTS
  3. +0 −1 activerecord/lib/active_record.rb
  4. +1 −12 activerecord/lib/active_record/associations/association.rb
  5. +0 −5 activerecord/lib/active_record/attribute_methods/dirty.rb
  6. +0 −6 activerecord/lib/active_record/autosave_association.rb
  7. +0 −2 activerecord/lib/active_record/counter_cache.rb
  8. +1 −3 activerecord/lib/active_record/fixtures.rb
  9. +0 −144 activerecord/lib/active_record/identity_map.rb
  10. +3 −20 activerecord/lib/active_record/inheritance.rb
  11. +0 −1 activerecord/lib/active_record/model.rb
  12. +4 −12 activerecord/lib/active_record/persistence.rb
  13. +0 −5 activerecord/lib/active_record/railtie.rb
  14. +1 −10 activerecord/lib/active_record/relation.rb
  15. +0 −10 activerecord/lib/active_record/relation/finder_methods.rb
  16. +0 −10 activerecord/lib/active_record/test_case.rb
  17. +0 −2 activerecord/lib/active_record/transactions.rb
  18. +0 −1 activerecord/test/cases/associations/eager_load_includes_full_sti_class_test.rb
  19. +10 −10 activerecord/test/cases/associations/eager_test.rb
  20. +0 −137 activerecord/test/cases/associations/identity_map_test.rb
  21. +2 −6 activerecord/test/cases/autosave_association_test.rb
  22. +2 −2 activerecord/test/cases/base_test.rb
  23. +0 −3 activerecord/test/cases/helper.rb
  24. +0 −74 activerecord/test/cases/identity_map/middleware_test.rb
  25. +0 −439 activerecord/test/cases/identity_map_test.rb
  26. +0 −12 activerecord/test/cases/log_subscriber_test.rb
  27. +1 −1 activerecord/test/cases/query_cache_test.rb
  28. +7 −9 activerecord/test/cases/relations_test.rb
  29. +1 −1 activerecord/test/support/connection.rb
  30. +1 −8 ci/travis.rb
  31. +0 −2 railties/guides/source/configuring.textile
  32. +0 −4 railties/lib/rails/test_help.rb
  33. +0 −7 railties/test/application/middleware_test.rb
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ##
+* Remove IdentityMap *Carlos Antonio da Silva*
+
* Added the schema cache dump feature.
`Schema cache dump` feature was implemetend. This feature can dump/load internal state of `SchemaCache` instance
@@ -26,13 +26,6 @@ You can run all the tests for a given database via rake:
The 'rake test' task will run all the tests for mysql, mysql2, sqlite3 and postgresql.
-== Identity Map
-
-By default the tests run with the Identity Map turned off. But all tests should pass whether or
-not the identity map is on or off. You can turn it on using the IM env variable:
-
- $ IM=true ruby -Itest test/case/base_test.rb
-
== Config file
By default, the config file is expected to be at the path test/config.yml. You can specify a
@@ -65,7 +65,6 @@ module ActiveRecord
autoload :DynamicFinderMatch
autoload :DynamicScopeMatch
autoload :Explain
- autoload :IdentityMap
autoload :Inheritance
autoload :Integration
autoload :Migration
@@ -45,7 +45,6 @@ def aliased_table_name
# 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
@@ -135,17 +134,7 @@ def target_scope
# 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
+ @target ||= find_target if find_target?
loaded! unless loaded?
target
rescue ActiveRecord::RecordNotFound
@@ -22,8 +22,6 @@ def save(*) #:nodoc:
if status = super
@previously_changed = changes
@changed_attributes.clear
- elsif IdentityMap.enabled?
- IdentityMap.remove(self)
end
status
end
@@ -34,9 +32,6 @@ def save!(*) #:nodoc:
@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.
@@ -328,7 +328,6 @@ def save_collection_association(reflection)
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?
@@ -348,11 +347,6 @@ def save_collection_association(reflection)
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
@@ -69,8 +69,6 @@ def update_counters(id, counters)
"#{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
@@ -796,9 +796,7 @@ def setup_fixture_accessors(fixture_names = nil)
@fixture_cache[fixture_name].delete(fixture) if force_reload
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
+ @fixture_cache[fixture_name][fixture] ||= @loaded_fixtures[fixture_name][fixture.to_s].find
else
raise StandardError, "No entry named '#{fixture}' found for fixture collection '#{fixture_name}'"
end
@@ -1,144 +0,0 @@
-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 and still in development (i.e. use it with care).
- #
- # == Associations
- #
- # Active Record Identity Map does not track associations yet. For example:
- #
- # comment = @post.comments.first
- # comment.post = nil
- # @post.comments.include?(comment) #=> true
- #
- # Ideally, the example above would return false, removing the comment object from the
- # post association when the association is nullified. This may cause side effects, as
- # in the situation below, if Identity Map is enabled:
- #
- # Post.has_many :comments, :dependent => :destroy
- #
- # comment = @post.comments.first
- # comment.post = nil
- # comment.save
- # Post.destroy(@post.id)
- #
- # Without using Identity Map, the code above will destroy the @post object leaving
- # the comment object intact. However, once we enable Identity Map, the post loaded
- # by Post.destroy is exactly the same object as the object @post. As the object @post
- # still has the comment object in @post.comments, once Identity Map is enabled, the
- # comment object will be accidently removed.
- #
- # This inconsistency is meant to be fixed in future Rails releases.
- #
- module IdentityMap
-
- 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)
- record = repository[klass.symbolized_sti_name][primary_key]
-
- if record.is_a?(klass)
- ActiveSupport::Notifications.instrument("identity.active_record",
- :line => "From Identity Map (id: #{primary_key})",
- :name => "#{klass} Loaded",
- :connection_id => object_id)
-
- record
- else
- nil
- end
- end
-
- def add(record)
- repository[record.class.symbolized_sti_name][record.id] = record
- end
-
- def remove(record)
- repository[record.class.symbolized_sti_name].delete(record.id)
- end
-
- def remove_by_id(symbolized_sti_name, id)
- repository[symbolized_sti_name].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 = self.class.initialize_attributes(coder['attributes'].except(*dirty))
- @attributes.update(attributes)
- @changed_attributes.update(coder['attributes'].slice(*dirty))
- @changed_attributes.delete_if{|k,v| v.eql? @attributes[k]}
-
- run_callbacks :find
-
- self
- end
-
- class Middleware
- def initialize(app)
- @app = app
- end
-
- def call(env)
- enabled = IdentityMap.enabled
- IdentityMap.enabled = true
-
- response = @app.call(env)
- response[2] = Rack::BodyProxy.new(response[2]) do
- IdentityMap.enabled = enabled
- IdentityMap.clear
- end
-
- response
- end
- end
- end
-end
@@ -63,26 +63,9 @@ def sti_name
# single-table inheritance model that makes it possible to create
# objects of different types from the same table.
def instantiate(record, column_types = {})
- 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
- column_types = sti_class.decorate_columns(column_types)
- instance = sti_class.allocate.init_with('attributes' => record,
- 'column_types' => column_types)
- end
-
- instance
+ sti_class = find_sti_class(record[inheritance_column])
+ column_types = sti_class.decorate_columns(column_types)
+ sti_class.allocate.init_with('attributes' => record, 'column_types' => column_types)
end
# For internal use.
@@ -60,7 +60,6 @@ def self.extend(*modules)
include AttributeMethods
include Callbacks, ActiveModel::Observing, Timestamp
include Associations
- include IdentityMap
include ActiveModel::SecurePassword
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Serialization, Store
@@ -115,10 +115,7 @@ def save!(*)
# callbacks, Observer methods, or any <tt>:dependent</tt> association
# options, use <tt>#destroy</tt>.
def delete
- if persisted?
- self.class.delete(id)
- IdentityMap.remove(self) if IdentityMap.enabled?
- end
+ self.class.delete(id) if persisted?
@destroyed = true
freeze
end
@@ -129,7 +126,6 @@ def destroy
destroy_associations
if persisted?
- IdentityMap.remove(self) if IdentityMap.enabled?
pk = self.class.primary_key
column = self.class.columns_hash[pk]
substitute = connection.substitute_at(column, 0)
@@ -284,11 +280,9 @@ def reload(options = nil)
clear_aggregation_cache
clear_association_cache
- IdentityMap.without do
- fresh_object = self.class.unscoped { self.class.find(id, options) }
- @attributes.update(fresh_object.instance_variable_get('@attributes'))
- @columns_hash = fresh_object.instance_variable_get('@columns_hash')
- end
+ fresh_object = self.class.unscoped { self.class.find(id, options) }
+ @attributes.update(fresh_object.instance_variable_get('@attributes'))
+ @columns_hash = fresh_object.instance_variable_get('@columns_hash')
@attributes_cache = {}
self
@@ -363,10 +357,8 @@ def create
attributes_values = arel_attributes_with_values_for_create(!id.nil?)
new_id = self.class.unscoped.insert attributes_values
-
self.id ||= new_id if self.class.primary_key
- IdentityMap.add(self) if IdentityMap.enabled?
@new_record = false
id
end
@@ -53,11 +53,6 @@ class Railtie < Rails::Railtie
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
if app.config.active_record.delete(:whitelist_attributes)
@@ -168,13 +168,7 @@ def exec_queries
default_scoped = with_default_scope
if default_scoped.equal?(self)
- @records = if @readonly_value.nil? && !@klass.locking_enabled?
- eager_loading? ? find_with_associations : @klass.find_by_sql(arel, @bind_values)
- else
- IdentityMap.without do
- eager_loading? ? find_with_associations : @klass.find_by_sql(arel, @bind_values)
- end
- end
+ @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, @bind_values)
preload = @preload_values
preload += @includes_values unless eager_loading?
@@ -274,7 +268,6 @@ def scoping
# # The same idea applies to limit and order
# Book.where('title LIKE ?', '%Rails%').order(:created_at).limit(5).update_all(:author => 'David')
def update_all(updates, conditions = nil, options = {})
- IdentityMap.repository[symbolized_base_class].clear if IdentityMap.enabled?
if conditions || options.present?
where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates)
else
@@ -404,7 +397,6 @@ def destroy(id)
# If you need to destroy dependent associations or call your <tt>before_*</tt> or
# +after_destroy+ callbacks, use the +destroy_all+ method instead.
def delete_all(conditions = nil)
- IdentityMap.repository[symbolized_base_class] = {} if IdentityMap.enabled?
if conditions
where(conditions).delete_all
else
@@ -437,7 +429,6 @@ def delete_all(conditions = nil)
# # Delete multiple rows
# Todo.delete([2,3,4])
def delete(id_or_array)
- IdentityMap.remove_by_id(self.symbolized_base_class, id_or_array) if IdentityMap.enabled?
where(primary_key => id_or_array).delete_all
end
Oops, something went wrong.

3 comments on commit a8dd21d

@zimbatm

Hi, is it possible to comment a bit on why the IdentityMap got removed from Rails ? I just got bit by that change today and was surprised that it wasn't explained more in the release notes and in the commit.

@dmathieu
Contributor

Because of some serious inconsistencies with associations. See #474

@zimbatm
Please sign in to comment.