From adafefb51d8ffda7d9f30a874d61ee4ce1b998bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 14 Mar 2012 00:00:34 +0100 Subject: [PATCH 1/3] Remove ARes from the list. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 674d41c88f6be..c6f868498d320 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ rvm: - 1.9.3 env: - "GEM=railties" - - "GEM=ap,am,amo,ares,as" + - "GEM=ap,am,amo,as" - "GEM=ar:mysql" - "GEM=ar:mysql2" - "GEM=ar:sqlite3" From a8dd21d8b459ce4d57160a359798c577085a95e9 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Fri, 2 Mar 2012 00:10:06 -0300 Subject: [PATCH 2/3] Remove IdentityMap --- activerecord/CHANGELOG.md | 2 + activerecord/RUNNING_UNIT_TESTS | 7 - activerecord/lib/active_record.rb | 1 - .../active_record/associations/association.rb | 13 +- .../active_record/attribute_methods/dirty.rb | 5 - .../lib/active_record/autosave_association.rb | 6 - .../lib/active_record/counter_cache.rb | 2 - activerecord/lib/active_record/fixtures.rb | 4 +- .../lib/active_record/identity_map.rb | 144 ------ activerecord/lib/active_record/inheritance.rb | 23 +- activerecord/lib/active_record/model.rb | 1 - activerecord/lib/active_record/persistence.rb | 16 +- activerecord/lib/active_record/railtie.rb | 5 - activerecord/lib/active_record/relation.rb | 11 +- .../active_record/relation/finder_methods.rb | 10 - activerecord/lib/active_record/test_case.rb | 10 - .../lib/active_record/transactions.rb | 2 - ...eager_load_includes_full_sti_class_test.rb | 1 - .../test/cases/associations/eager_test.rb | 20 +- .../cases/associations/identity_map_test.rb | 137 ------ .../test/cases/autosave_association_test.rb | 8 +- activerecord/test/cases/base_test.rb | 4 +- activerecord/test/cases/helper.rb | 3 - .../cases/identity_map/middleware_test.rb | 74 --- activerecord/test/cases/identity_map_test.rb | 439 ------------------ .../test/cases/log_subscriber_test.rb | 12 - activerecord/test/cases/query_cache_test.rb | 2 +- activerecord/test/cases/relations_test.rb | 16 +- activerecord/test/support/connection.rb | 2 +- ci/travis.rb | 9 +- railties/guides/source/configuring.textile | 2 - railties/lib/rails/test_help.rb | 4 - railties/test/application/middleware_test.rb | 7 - 33 files changed, 36 insertions(+), 966 deletions(-) delete mode 100644 activerecord/lib/active_record/identity_map.rb delete mode 100644 activerecord/test/cases/associations/identity_map_test.rb delete mode 100644 activerecord/test/cases/identity_map/middleware_test.rb delete mode 100644 activerecord/test/cases/identity_map_test.rb diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6fd86b1f322d6..98fbec80132c6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -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 diff --git a/activerecord/RUNNING_UNIT_TESTS b/activerecord/RUNNING_UNIT_TESTS index a1ed6712c5593..2c310e7ac3f9c 100644 --- a/activerecord/RUNNING_UNIT_TESTS +++ b/activerecord/RUNNING_UNIT_TESTS @@ -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 diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 73c8a06ab7642..0cb0644bcf74f 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -65,7 +65,6 @@ module ActiveRecord autoload :DynamicFinderMatch autoload :DynamicScopeMatch autoload :Explain - autoload :IdentityMap autoload :Inheritance autoload :Integration autoload :Migration diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 512c52338e541..fb0ca15c2319e 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -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 diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 3a737e5b3527e..11c63591e3a99 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -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 # reload 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 eb3ae7014eda0..be0af081d15f7 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -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 diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 8d5388e1f345c..f52979ebd9a57 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -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 diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index b82d5b5621aea..5c42e8f719b3f 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -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 diff --git a/activerecord/lib/active_record/identity_map.rb b/activerecord/lib/active_record/identity_map.rb deleted file mode 100644 index d9777bb2f65ea..0000000000000 --- a/activerecord/lib/active_record/identity_map.rb +++ /dev/null @@ -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 config.active_record.identity_map = true - # in your config/application.rb 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 diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 2c766411a0958..ebe244c6a6917 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -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. diff --git a/activerecord/lib/active_record/model.rb b/activerecord/lib/active_record/model.rb index 86de5ab2fa1ab..105d1e0e2b017 100644 --- a/activerecord/lib/active_record/model.rb +++ b/activerecord/lib/active_record/model.rb @@ -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 diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 09502bb52c553..35c922e979d26 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -115,10 +115,7 @@ def save!(*) # callbacks, Observer methods, or any :dependent association # options, use #destroy. 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 diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 516c450f85715..ee3a6bf8c088d 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -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) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7531e1fe6fbce..6f39708ec3fe5 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -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 before_* 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 diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index adfacf37ee30c..52e67ecc6eacb 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -318,17 +318,7 @@ def find_with_ids(*ids) def find_one(id) id = id.id if ActiveRecord::Base === id - if IdentityMap.enabled? && where_values.blank? && - limit_value.blank? && order_values.blank? && - includes_values.blank? && preload_values.blank? && - readonly_value.nil? && joins_values.blank? && - !@klass.locking_enabled? && - record = IdentityMap.get(@klass, id) - return record - end - column = columns_hash[primary_key] - substitute = connection.substitute_at(column, @bind_values.length) relation = where(table[primary_key].eq(substitute)) relation.bind_values += [[column, id]] diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb index 4d881f0f7d4f0..fcaa4b74a6d45 100644 --- a/activerecord/lib/active_record/test_case.rb +++ b/activerecord/lib/active_record/test_case.rb @@ -7,20 +7,10 @@ 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 teardown SQLCounter.log.clear 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 b492377d182d5..743dfc5a387f3 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -251,7 +251,6 @@ def rollback_active_record_state! remember_transaction_record_state yield rescue Exception - IdentityMap.remove(self) if IdentityMap.enabled? restore_transaction_record_state raise ensure @@ -270,7 +269,6 @@ def committed! #:nodoc: def rolledback!(force_restore_state = false) #:nodoc: run_callbacks :rollback ensure - IdentityMap.remove(self) if IdentityMap.enabled? restore_transaction_record_state(force_restore_state) end 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 7965bb404c7b1..3044a8c312478 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,7 +27,6 @@ def test_class_names 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 b79c69bbb5bdc..b27a93f857d6c 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -197,7 +197,7 @@ def test_finding_with_includes_on_has_one_assocation_with_same_include_includes_ author = authors(:david) post = author.post_about_thinking_with_last_comment last_comment = post.last_comment - 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 + 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 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 @@ -208,7 +208,7 @@ def test_finding_with_includes_on_belongs_to_association_with_same_include_inclu post = posts(:welcome) author = post.author author_address = author.author_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 + 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 assert_no_queries do assert_equal author, post.author_with_address assert_equal author_address, post.author_with_address.author_address @@ -611,9 +611,9 @@ def test_eager_with_has_and_belongs_to_many_and_limit assert posts[1].categories.include?(categories(:general)) end - # This is only really relevant when the identity map is off. Since the preloader for habtm - # gets raw row hashes from the database and then instantiates them, this test ensures that - # it only instantiates one actual object per record from the database. + # Since the preloader for habtm gets raw row hashes from the database and then + # instantiates them, this test ensures that it only instantiates one actual + # object per record from the database. def test_has_and_belongs_to_many_should_not_instantiate_same_records_multiple_times welcome = posts(:welcome) categories = Category.includes(:posts) @@ -1002,18 +1002,18 @@ def test_eager_loading_with_conditions_on_joined_table_preloads assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author} - posts = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + posts = assert_queries(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(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + posts = assert_queries(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(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + posts = assert_queries(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 @@ -1027,7 +1027,7 @@ def test_eager_loading_with_conditions_on_string_joined_table_preloads assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author} - posts = assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + 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 @@ -1116,7 +1116,7 @@ def test_preloading_empty_belongs_to 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(ActiveRecord::IdentityMap.enabled? ? 1 : 2) { Tagging.preload(:taggable).find(t.id) } + tagging = assert_queries(2) { Tagging.preload(:taggable).find(t.id) } assert_no_queries { assert_nil tagging.taggable } end diff --git a/activerecord/test/cases/associations/identity_map_test.rb b/activerecord/test/cases/associations/identity_map_test.rb deleted file mode 100644 index 9b8635774c1ec..0000000000000 --- a/activerecord/test/cases/associations/identity_map_test.rb +++ /dev/null @@ -1,137 +0,0 @@ -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/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 1376810adfa61..67b65c51d59a4 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -978,10 +978,7 @@ def test_should_allow_to_bypass_validations_on_associated_models_at_any_depth values = [@pirate.reload.catchphrase, @pirate.ship.name, *@pirate.ship.parts.map(&:name)] # Oracle saves empty string as NULL if current_adapter?(:OracleAdapter) - expected = ActiveRecord::IdentityMap.enabled? ? - [nil, nil, '', ''] : - [nil, nil, nil, nil] - assert_equal expected, values + assert_equal [nil, nil, nil, nil], values else assert_equal ['', '', '', ''], values end @@ -1077,8 +1074,7 @@ def test_should_still_allow_to_bypass_validations_on_the_associated_model @ship.save(:validate => false) # Oracle saves empty string as NULL if current_adapter?(:OracleAdapter) - expected = ActiveRecord::IdentityMap.enabled? ? [nil, ''] : [nil, nil] - assert_equal expected, [@ship.reload.name, @ship.pirate.catchphrase] + assert_equal [nil, nil], [@ship.reload.name, @ship.pirate.catchphrase] else assert_equal ['', ''], [@ship.reload.name, @ship.pirate.catchphrase] end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 01f647b261f24..76b999efac598 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1139,7 +1139,7 @@ def test_geometric_content assert g.save # Reload and check that we have all the geometric attributes. - h = ActiveRecord::IdentityMap.without { Geometric.find(g.id) } + h = Geometric.find(g.id) assert_equal '(5,6.1)', h.a_point assert_equal '[(2,3),(5.5,7)]', h.a_line_segment @@ -1168,7 +1168,7 @@ def test_geometric_content assert g.save # Reload and check that we have all the geometric attributes. - h = ActiveRecord::IdentityMap.without { Geometric.find(g.id) } + h = Geometric.find(g.id) assert_equal '(5,6.1)', h.a_point assert_equal '[(2,3),(5.5,7)]', h.a_line_segment diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 9f5f012073eed..5c3560a33beb7 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -19,9 +19,6 @@ # Show backtraces for deprecated behavior for quicker cleanup. ActiveSupport::Deprecation.debug = true -# Enable Identity Map only when ENV['IM'] is set to "true" -ActiveRecord::IdentityMap.enabled = (ENV['IM'] == "true") - # Avoid deprecation warning setting dependent_restrict_raises to false. The default is true ActiveRecord::Base.dependent_restrict_raises = false diff --git a/activerecord/test/cases/identity_map/middleware_test.rb b/activerecord/test/cases/identity_map/middleware_test.rb deleted file mode 100644 index 5b32a1c6d2fbb..0000000000000 --- a/activerecord/test/cases/identity_map/middleware_test.rb +++ /dev/null @@ -1,74 +0,0 @@ -require "cases/helper" -require "rack" - -module ActiveRecord - module IdentityMap - class MiddlewareTest < ActiveRecord::TestCase - def setup - super - @enabled = IdentityMap.enabled - IdentityMap.enabled = false - end - - def teardown - super - IdentityMap.enabled = @enabled - IdentityMap.clear - end - - def test_delegates - called = false - mw = Middleware.new lambda { |env| - called = true - [200, {}, nil] - } - mw.call({}) - assert called, 'middleware delegated' - end - - def test_im_enabled_during_delegation - mw = Middleware.new lambda { |env| - assert IdentityMap.enabled?, 'identity map should be enabled' - [200, {}, nil] - } - mw.call({}) - end - - class Enum < Struct.new(:iter) - def each(&b) - iter.call(&b) - end - end - - def test_im_enabled_during_body_each - mw = Middleware.new lambda { |env| - [200, {}, Enum.new(lambda { |&b| - assert IdentityMap.enabled?, 'identity map should be enabled' - b.call "hello" - })] - } - body = mw.call({}).last - body.each { |x| assert_equal 'hello', x } - end - - def test_im_disabled_after_body_close - mw = Middleware.new lambda { |env| [200, {}, []] } - body = mw.call({}).last - assert IdentityMap.enabled?, 'identity map should be enabled' - body.close - assert !IdentityMap.enabled?, 'identity map should be disabled' - end - - def test_im_cleared_after_body_close - mw = Middleware.new lambda { |env| [200, {}, []] } - body = mw.call({}).last - - IdentityMap.repository['hello'] = 'world' - assert !IdentityMap.repository.empty?, 'repo should not be empty' - - body.close - assert IdentityMap.repository.empty?, 'repo should be empty' - end - end - end -end diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb deleted file mode 100644 index 3efc8bf559568..0000000000000 --- a/activerecord/test/cases/identity_map_test.rb +++ /dev/null @@ -1,439 +0,0 @@ -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 - - def test_queries_are_not_executed_when_finding_by_id - Post.find 1 - assert_no_queries do - Post.find 1 - end - 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 if IM is functioning properly on classes with multiple # - # types of inheritance # - ############################################################################## - - def test_inherited_without_type_attribute_without_identity_map - ActiveRecord::IdentityMap.without do - p1 = DestructivePirate.create!(:catchphrase => "I'm not a regular Pirate") - p2 = Pirate.find(p1.id) - assert_not_same(p1, p2) - end - end - - def test_inherited_with_type_attribute_without_identity_map - ActiveRecord::IdentityMap.without do - c = comments(:sub_special_comment) - c1 = SubSpecialComment.find(c.id) - c2 = Comment.find(c.id) - assert_same(c1.class, c2.class) - end - end - - def test_inherited_without_type_attribute - p1 = DestructivePirate.create!(:catchphrase => "I'm not a regular Pirate") - p2 = Pirate.find(p1.id) - assert_not_same(p1, p2) - end - - def test_inherited_with_type_attribute - c = comments(:sub_special_comment) - c1 = SubSpecialComment.find(c.id) - c2 = Comment.find(c.id) - assert_same(c1, c2) - end - - ############################################################################## - # Tests checking dirty attribute behavior 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) - - 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"}) - - assert_equal({"name"=>["Marcin Raczkowski", "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"}) - - assert_equal("Swistak Sreberkowiec", swistak.name) - assert_equal({"name"=>["Marcin Raczkowski", "Swistak Sreberkowiec"]}, 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, _ = 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 behavior with preloaded associations, joins, # - # includes etc. # - ############################################################################## - - def test_find_with_preloaded_associations - assert_queries(2) do - posts = Post.preload(:comments).order('posts.id') - 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).order('posts.id') - assert posts.first.comments.first - end - - assert_queries(2) do - posts = Post.preload(:author).order('posts.id') - 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).order('posts.id') - assert posts.first.author - end - - assert_queries(1) do - posts = Post.preload(:author, :comments).order('posts.id') - 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).order('posts.id') - assert posts.first.comments.first - end - - assert_queries(1) do - posts = Post.scoped.includes(:comments).order('posts.id') - assert posts.first.comments.first - end - - assert_queries(2) do - posts = Post.includes(:author).order('posts.id') - assert posts.first.author - end - - assert_queries(1) do - posts = Post.includes(:author, :comments).order('posts.id') - 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 related 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, frozen, 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/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index d1f0ace184e4f..acd2fcdad424b 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -11,8 +11,6 @@ class LogSubscriberTest < ActiveRecord::TestCase def setup @old_logger = ActiveRecord::Base.logger - @using_identity_map = ActiveRecord::IdentityMap.enabled? - ActiveRecord::IdentityMap.enabled = false Developer.primary_key super ActiveRecord::LogSubscriber.attach_to(:active_record) @@ -22,7 +20,6 @@ def teardown super ActiveRecord::LogSubscriber.log_subscribers.pop ActiveRecord::Base.logger = @old_logger - ActiveRecord::IdentityMap.enabled = @using_identity_map end def set_logger(logger) @@ -103,13 +100,4 @@ def test_cached_queries_doesnt_log_when_level_is_not_debug def test_initializes_runtime Thread.new { assert_equal 0, ActiveRecord::LogSubscriber.runtime }.join end - - def test_log - ActiveRecord::IdentityMap.use do - Post.find 1 - Post.find 1 - end - wait - assert_match(/From Identity Map/, @logger.logged(:debug).last) - end end diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index f36f4237b1879..d93478513b869 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -107,7 +107,7 @@ def test_cache_clear_after_close end def test_find_queries - assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) { Task.find(1); Task.find(1) } + assert_queries(2) { Task.find(1); Task.find(1) } end def test_find_queries_with_cache diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index acec230c7237c..91bc407d42f09 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -323,7 +323,7 @@ def test_find_with_preloaded_associations assert posts.first.comments.first end - assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + assert_queries(2) do posts = Post.preload(:comments).order('posts.id') assert posts.first.comments.first end @@ -333,12 +333,12 @@ def test_find_with_preloaded_associations assert posts.first.author end - assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + assert_queries(2) do posts = Post.preload(:author).order('posts.id') assert posts.first.author end - assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 3) do + assert_queries(3) do posts = Post.preload(:author, :comments).order('posts.id') assert posts.first.author assert posts.first.comments.first @@ -351,7 +351,7 @@ def test_find_with_included_associations assert posts.first.comments.first end - assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do + assert_queries(2) do posts = Post.scoped.includes(:comments).order('posts.id') assert posts.first.comments.first end @@ -361,7 +361,7 @@ def test_find_with_included_associations assert posts.first.author end - assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 3) do + assert_queries(3) do posts = Post.includes(:author, :comments).order('posts.id') assert posts.first.author assert posts.first.comments.first @@ -685,10 +685,8 @@ def test_relation_merging_with_locks end def test_relation_merging_with_preload - 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 + [Post.scoped.merge(Post.preload(:author)), Post.preload(:author).merge(Post.scoped)].each do |posts| + assert_queries(2) { assert posts.first.author } end end diff --git a/activerecord/test/support/connection.rb b/activerecord/test/support/connection.rb index 11154c379740a..c176316a0534a 100644 --- a/activerecord/test/support/connection.rb +++ b/activerecord/test/support/connection.rb @@ -12,7 +12,7 @@ def self.connection_config end def self.connect - puts "Using #{connection_name} with Identity Map #{ActiveRecord::IdentityMap.enabled? ? 'on' : 'off'}" + puts "Using #{connection_name}" ActiveRecord::Model.logger = ActiveSupport::Logger.new("debug.log") ActiveRecord::Model.configurations = connection_config ActiveRecord::Model.establish_connection 'arunit' diff --git a/ci/travis.rb b/ci/travis.rb index 217cda9c82c69..fc120f80baaae 100755 --- a/ci/travis.rb +++ b/ci/travis.rb @@ -34,7 +34,6 @@ def run!(options = {}) self.options.update(options) Dir.chdir(dir) do announce(heading) - ENV['IM'] = identity_map?.inspect rake(*tasks) end end @@ -45,7 +44,7 @@ def announce(heading) def heading heading = [gem] - heading << "with #{adapter} IM #{identity_map? ? 'enabled' : 'disabled'}" if activerecord? + heading << "with #{adapter}" if activerecord? heading << "in isolation" if isolated? heading.join(' ') end @@ -61,7 +60,6 @@ def tasks def key key = [gem] key << adapter if activerecord? - key << 'IM' if identity_map? key << 'isolated' if isolated? key.join(':') end @@ -70,10 +68,6 @@ def activerecord? gem == 'activerecord' end - def identity_map? - options[:identity_map] - end - def isolated? options[:isolated] end @@ -107,7 +101,6 @@ def rake(*tasks) results[build.key] = build.run! if build.activerecord? - build.options[:identity_map] = true results[build.key] = build.run! end end diff --git a/railties/guides/source/configuring.textile b/railties/guides/source/configuring.textile index 49b15bd650efe..79980be5efdf1 100644 --- a/railties/guides/source/configuring.textile +++ b/railties/guides/source/configuring.textile @@ -288,8 +288,6 @@ h4. Configuring Active Record * +config.active_record.whitelist_attributes+ will create an empty whitelist of attributes available for mass-assignment security for all models in your app. -* +config.active_record.identity_map+ controls whether the identity map is enabled, and is false by default. - * +config.active_record.auto_explain_threshold_in_seconds+ configures the threshold for automatic EXPLAINs (+nil+ disables this feature). Queries exceeding the threshold get their query plan logged. Default is 0.5 in development mode. * +config.active_record.dependent_restrict_raises+ will control the behavior when an object with a :dependent => :restrict association is deleted. Setting this to false will prevent +DeleteRestrictionError+ from being raised and instead will add an error on the model object. Defaults to false in the development mode. diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index 11e4353c87b99..46bf3bbe48e83 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -18,10 +18,6 @@ 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/middleware_test.rb b/railties/test/application/middleware_test.rb index 712a555c4ae08..e6f85918f60d0 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -115,7 +115,6 @@ def app 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 @@ -173,12 +172,6 @@ def app assert_equal "Rack::Runtime", middleware.fourth end - test "identity map is inserted" do - add_to_config "config.active_record.identity_map = true" - boot! - assert middleware.include?("ActiveRecord::IdentityMap::Middleware") - end - test "insert middleware before" do add_to_config "config.middleware.insert_before ActionDispatch::Static, Rack::Config" boot! From dde3058c3a7350d6e28d602a1f57fe908075bbe7 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sun, 4 Mar 2012 16:10:48 -0300 Subject: [PATCH 3/3] Expand changelog and upgrading rails guide with IdentityMap info --- activerecord/CHANGELOG.md | 11 ++++++++++- .../guides/source/upgrading_ruby_on_rails.textile | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 98fbec80132c6..5a289a5aac003 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,6 +1,15 @@ ## Rails 4.0.0 (unreleased) ## -* Remove IdentityMap *Carlos Antonio da Silva* +* Remove IdentityMap + + IdentityMap has never graduated to be an "enabled-by-default" feature, due + to some inconsistencies with associations, as described in this commit: + + https://github.com/rails/rails/commit/302c912bf6bcd0fa200d964ec2dc4a44abe328a6 + + Hence the removal from the codebase, until such issues are fixed. + + *Carlos Antonio da Silva* * Added the schema cache dump feature. diff --git a/railties/guides/source/upgrading_ruby_on_rails.textile b/railties/guides/source/upgrading_ruby_on_rails.textile index 731d56c850d68..e63548abc9f99 100644 --- a/railties/guides/source/upgrading_ruby_on_rails.textile +++ b/railties/guides/source/upgrading_ruby_on_rails.textile @@ -34,6 +34,10 @@ h4(#plugins4_0). vendor/plugins Rails 4.0 no longer supports loading plugins from vendor/plugins. You must replace any plugins by extracting them to gems and adding them to your Gemfile. If you choose not to make them gems, you can move them into, say, lib/my_plugin/* and add an appropriate initializer in config/initializers/my_plugin.rb. +h4(#identity_map4_0). IdentityMap + +Rails 4.0 has removed IdentityMap from ActiveRecord, due to "some inconsistencies with associations":https://github.com/rails/rails/commit/302c912bf6bcd0fa200d964ec2dc4a44abe328a6. If you have manually enabled it in your application, you will have to remove the following config that has no effect anymore: config.active_record.identity_map. + h3. Upgrading from Rails 3.1 to Rails 3.2 If your application is currently on any version of Rails older than 3.1.x, you should upgrade to Rails 3.1 before attempting an update to Rails 3.2.