From f125a34501e21b1e0da2b80d149df7a739482804 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Fri, 6 Nov 2009 23:53:33 +0100 Subject: Define autosave association callbacks when using accepts_nested_attributes_for. This way we don't define all the validation methods for all associations by default, but only when needed. [#3355 state:resolved] --- activerecord/lib/active_record/nested_attributes.rb | 2 ++ activerecord/test/models/pirate.rb | 2 +- activerecord/test/models/ship.rb | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index edcf547e01..ca3110a374 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -250,6 +250,8 @@ module ActiveRecord assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes) end }, __FILE__, __LINE__ + + add_autosave_association_callbacks(reflection) else raise ArgumentError, "No association found for name `#{association_name}'. Has it been defined yet?" end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 05c5b666ae..f2c05dd48f 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -18,7 +18,7 @@ class Pirate < ActiveRecord::Base has_many :treasure_estimates, :through => :treasures, :source => :price_estimates # These both have :autosave enabled because accepts_nested_attributes_for is used on them. - has_one :ship, :validate => true + has_one :ship has_one :non_validated_ship, :class_name => 'Ship' has_many :birds has_many :birds_with_method_callbacks, :class_name => "Bird", diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index d0df951622..06759d64b8 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -1,7 +1,7 @@ class Ship < ActiveRecord::Base self.record_timestamps = false - belongs_to :pirate, :validate => true + belongs_to :pirate has_many :parts, :class_name => 'ShipPart', :autosave => true accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } -- cgit v1.2.3 From 1979e9c8553f4d7905822fdcc99e52d179e78c3c Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 9 Nov 2009 21:11:26 +0100 Subject: Symbol#to_proc is not needed for Ruby >= 1.8.7 --- activerecord/lib/active_record/base.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 4e6090458a..056f29f029 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -12,7 +12,6 @@ require 'active_support/core_ext/hash/deep_merge' require 'active_support/core_ext/hash/indifferent_access' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/string/behavior' -require 'active_support/core_ext/symbol' require 'active_support/core_ext/object/metaclass' module ActiveRecord #:nodoc: -- cgit v1.2.3 From f8e713f488bba264ba73251b56ad56385b8ed824 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 9 Nov 2009 21:23:19 +0100 Subject: Object#tap is not needed for Ruby >= 1.8.7 --- activerecord/lib/active_record/attribute_methods/dirty.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 4df0f1af69..4a3ab9ea82 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,5 +1,3 @@ -require 'active_support/core_ext/object/tap' - module ActiveRecord module AttributeMethods module Dirty -- cgit v1.2.3 From 329e7f44417c00a8d6fee337c288a463e9d64346 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 9 Nov 2009 21:46:16 +0100 Subject: Integer#even? and Integer#odd? are not needed for Ruby >= 1.8.7 --- activerecord/lib/active_record/validations.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 0365cb592f..e8a2a72735 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -1,5 +1,3 @@ -require 'active_support/core_ext/integer/even_odd' - module ActiveRecord # Raised by save! and create! when the record is invalid. Use the # +record+ method to retrieve the record which did not validate. -- cgit v1.2.3 From 703d31c20a7b531053d5c009972a89e857b07376 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 9 Nov 2009 17:01:57 -0800 Subject: Clarify failed assertion --- activerecord/test/cases/relations_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 4833d04aff..1a2c8030fb 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -78,7 +78,7 @@ class RelationTest < ActiveRecord::TestCase relation = Topic.all ["map", "uniq", "sort", "insert", "delete", "update"].each do |method| - assert relation.respond_to?(method) + assert relation.respond_to?(method), "Topic.all should respond to #{method.inspect}" end end -- cgit v1.2.3 From 11e798ae0f2f46498811282756c9d21df3d4b523 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 9 Nov 2009 23:28:36 -0600 Subject: Avoid adding component lib/ to load path multiple times --- activerecord/test/cases/helper.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 871cfa6468..25613da912 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -3,9 +3,11 @@ begin require "#{root}/vendor/gems/environment" rescue LoadError $:.unshift("#{root}/activesupport/lib") - $:.unshift("#{root}/activerecord/lib") end +lib = File.expand_path("#{File.dirname(__FILE__)}/../../lib") +$:.unshift(lib) unless $:.include?('lib') || $:.include?(lib) + require 'config' require 'rubygems' -- cgit v1.2.3 From d625312fe11bbf53e285a0efd570249f0bf91fd1 Mon Sep 17 00:00:00 2001 From: Matt Jones Date: Sat, 7 Nov 2009 14:00:54 -0500 Subject: delete correct records for a has_many with :primary_key and :dependent => :delete_all Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/associations.rb | 2 +- .../test/cases/associations/has_many_associations_test.rb | 12 ++++++++++++ activerecord/test/cases/reflection_test.rb | 4 ++-- activerecord/test/models/company.rb | 2 ++ 4 files changed, 17 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 03c8d4b3ed..3a5f3ed030 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1480,7 +1480,7 @@ module ActiveRecord if reflection.options.include?(:dependent) # Add polymorphic type if the :as option is present dependent_conditions = [] - dependent_conditions << "#{reflection.primary_key_name} = \#{record.quoted_id}" + dependent_conditions << "#{reflection.primary_key_name} = \#{record.#{reflection.name}.send(:owner_quoted_id)}" dependent_conditions << "#{reflection.options[:as]}_type = '#{base_class.name}'" if reflection.options[:as] dependent_conditions << sanitize_sql(reflection.options[:conditions], reflection.quoted_table_name) if reflection.options[:conditions] dependent_conditions << extra_conditions if extra_conditions diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index b193f8d8ba..86d14c9c81 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -659,6 +659,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, Client.find_all_by_client_of(firm.id).size end + def test_delete_all_association_with_primary_key_deletes_correct_records + firm = Firm.find(:first) + # break the vanilla firm_id foreign key + assert_equal 2, firm.clients.count + firm.clients.first.update_attribute(:firm_id, nil) + assert_equal 1, firm.clients(true).count + assert_equal 1, firm.clients_using_primary_key_with_delete_all.count + old_record = firm.clients_using_primary_key_with_delete_all.first + firm = Firm.find(:first) + firm.destroy + assert Client.find_by_id(old_record.id).nil? + end def test_creation_respects_hash_condition ms_client = companies(:first_firm).clients_like_ms_with_hash_conditions.build diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 99e248743a..acd214eb5a 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -176,8 +176,8 @@ class ReflectionTest < ActiveRecord::TestCase def test_reflection_of_all_associations # FIXME these assertions bust a lot - assert_equal 36, Firm.reflect_on_all_associations.size - assert_equal 26, Firm.reflect_on_all_associations(:has_many).size + assert_equal 37, Firm.reflect_on_all_associations.size + assert_equal 27, Firm.reflect_on_all_associations(:has_many).size assert_equal 10, Firm.reflect_on_all_associations(:has_one).size assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 469f5399ae..7e93fda1eb 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -68,6 +68,8 @@ class Firm < Company has_many :readonly_clients, :class_name => 'Client', :readonly => true has_many :clients_using_primary_key, :class_name => 'Client', :primary_key => 'name', :foreign_key => 'firm_name' + has_many :clients_using_primary_key_with_delete_all, :class_name => 'Client', + :primary_key => 'name', :foreign_key => 'firm_name', :dependent => :delete_all has_many :clients_grouped_by_firm_id, :class_name => "Client", :group => "firm_id", :select => "firm_id" has_many :clients_grouped_by_name, :class_name => "Client", :group => "name", :select => "name" -- cgit v1.2.3 From 77478f21af980c8879762fc65def8cacba5a7eb7 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 10 Nov 2009 11:00:29 -0800 Subject: Resolve deadlock in pooled connections test --- activerecord/test/cases/pooled_connections_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/pooled_connections_test.rb b/activerecord/test/cases/pooled_connections_test.rb index f9eea3d118..2529a33dab 100644 --- a/activerecord/test/cases/pooled_connections_test.rb +++ b/activerecord/test/cases/pooled_connections_test.rb @@ -105,7 +105,7 @@ class PooledConnectionsTest < ActiveRecord::TestCase Thread.new do ActiveRecord::Base.connection.rollback_db_transaction ActiveRecord::Base.connection_pool.release_connection - end.join rescue nil + end add_record('three') end -- cgit v1.2.3 From 5fa497abf5b885187130e58aefcb1c3228295e3c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 10 Nov 2009 11:00:50 -0800 Subject: Ruby 1.9: fix Relation respond_to? and method_missing --- activerecord/lib/active_record/relation.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 6bc56ecf15..5f0eec754f 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -110,19 +110,17 @@ module ActiveRecord end def respond_to?(method) - if @relation.respond_to?(method) || Array.instance_methods.include?(method.to_s) - true - else - super - end + @relation.respond_to?(method) || Array.method_defined?(method) || super end private def method_missing(method, *args, &block) if @relation.respond_to?(method) @relation.send(method, *args, &block) - elsif Array.instance_methods.include?(method.to_s) + elsif Array.method_defined?(method) to_a.send(method, *args, &block) + else + super end end end -- cgit v1.2.3 From a4aa95ba0ca9c0a6282ed3d70d63be91acf7ab6d Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 10 Nov 2009 15:13:38 -0800 Subject: Bump AR.gemspec Arel dep too --- activerecord/activerecord.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 204cddde47..c84a3ac5a9 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', '= 3.0.pre') s.add_dependency('activemodel', '= 3.0.pre') - s.add_dependency('arel', '~> 0.1.1') + s.add_dependency('arel', '= 0.2.pre') s.require_path = 'lib' s.autorequire = 'active_record' -- cgit v1.2.3 From bbb3e5a858b2d078b2af7516a583fa12f3edb565 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 10 Nov 2009 16:50:15 -0800 Subject: Unify test:isolated across components and run by default at toplevel --- activerecord/Rakefile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/Rakefile b/activerecord/Rakefile index f7585f789b..2511f13fed 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -49,11 +49,13 @@ task :test do run_without_aborting(*tasks) end -task :isolated_test do - tasks = defined?(JRUBY_VERSION) ? - %w(isolated_test_jdbcmysql isolated_test_jdbcsqlite3 isolated_test_jdbcpostgresql) : - %w(isolated_test_mysql isolated_test_sqlite3 isolated_test_postgresql) - run_without_aborting(*tasks) +namespace :test do + task :isolated do + tasks = defined?(JRUBY_VERSION) ? + %w(isolated_test_jdbcmysql isolated_test_jdbcsqlite3 isolated_test_jdbcpostgresql) : + %w(isolated_test_mysql isolated_test_sqlite3 isolated_test_postgresql) + run_without_aborting(*tasks) + end end %w( mysql postgresql sqlite3 firebird db2 oracle sybase openbase frontbase jdbcmysql jdbcpostgresql jdbcsqlite3 jdbcderby jdbch2 jdbchsqldb ).each do |adapter| -- cgit v1.2.3 From fca32eb6c5b41e4f19a25b7b246c4a8a3d763667 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 11 Nov 2009 01:43:58 -0800 Subject: Update AR logger subscriber for Notifications subscriber args change --- activerecord/lib/active_record/notifications.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/notifications.rb b/activerecord/lib/active_record/notifications.rb index a5ce7ac524..562a5b91f4 100644 --- a/activerecord/lib/active_record/notifications.rb +++ b/activerecord/lib/active_record/notifications.rb @@ -1,5 +1,5 @@ require 'active_support/notifications' -ActiveSupport::Notifications.subscribe("sql") do |event| - ActiveRecord::Base.connection.log_info(event.payload[:sql], event.payload[:name], event.duration) +ActiveSupport::Notifications.subscribe("sql") do |name, before, after, result, instrumenter_id, payload| + ActiveRecord::Base.connection.log_info(payload[:sql], name, after - before) end -- cgit v1.2.3 From fbbf0086ca8f13f76d2969f655bb1bfc2f4eb4d6 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 12 Nov 2009 17:21:41 -0800 Subject: Ruby 1.9.2: avoid #flatten --- .../lib/active_record/association_preload.rb | 20 ++++++++++++-------- activerecord/lib/active_record/associations.rb | 16 ++++++++++------ .../active_record/associations/association_proxy.rb | 20 ++++++++++++++++---- 3 files changed, 38 insertions(+), 18 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index e41fda7a4b..9f7b2a60b2 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -1,3 +1,6 @@ +require 'active_support/core_ext/array/wrap' +require 'active_support/core_ext/enumerable' + module ActiveRecord # See ActiveRecord::AssociationPreload::ClassMethods for documentation. module AssociationPreload #:nodoc: @@ -82,7 +85,7 @@ module ActiveRecord # only one level deep in the +associations+ argument, i.e. it's not passed # to the child associations when +associations+ is a Hash. def preload_associations(records, associations, preload_options={}) - records = [records].flatten.compact.uniq + records = Array.wrap(records).compact.uniq return if records.empty? case associations when Array then associations.each {|association| preload_associations(records, association, preload_options)} @@ -92,7 +95,7 @@ module ActiveRecord raise "parent must be an association name" unless parent.is_a?(String) || parent.is_a?(Symbol) preload_associations(records, parent, preload_options) reflection = reflections[parent] - parents = records.map {|record| record.send(reflection.name)}.flatten.compact + parents = records.sum { |record| Array.wrap(record.send(reflection.name)) } unless parents.empty? parents.first.class.preload_associations(parents, child) end @@ -123,7 +126,8 @@ module ActiveRecord parent_records.each do |parent_record| association_proxy = parent_record.send(reflection_name) association_proxy.loaded - association_proxy.target.push(*[associated_record].flatten) + association_proxy.target.push *Array.wrap(associated_record) + association_proxy.__send__(:set_inverse_instance, associated_record, parent_record) end end @@ -254,6 +258,7 @@ module ActiveRecord through_reflection = reflections[through_association] through_primary_key = through_reflection.primary_key_name + through_records = [] if reflection.options[:source_type] interface = reflection.source_reflection.options[:foreign_type] preload_options = {:conditions => ["#{connection.quote_column_name interface} = ?", reflection.options[:source_type]]} @@ -262,23 +267,22 @@ module ActiveRecord records.first.class.preload_associations(records, through_association, preload_options) # Dont cache the association - we would only be caching a subset - through_records = [] records.each do |record| proxy = record.send(through_association) if proxy.respond_to?(:target) - through_records << proxy.target + through_records.concat Array.wrap(proxy.target) proxy.reset else # this is a has_one :through reflection through_records << proxy if proxy end end - through_records.flatten! else records.first.class.preload_associations(records, through_association) - through_records = records.map {|record| record.send(through_association)}.flatten + records.each do |record| + through_records.concat Array.wrap(record.send(through_association)) + end end - through_records.compact! through_records end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3a5f3ed030..6c5e25010f 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1922,12 +1922,16 @@ module ActiveRecord reflection = base.reflections[name] is_collection = [:has_many, :has_and_belongs_to_many].include?(reflection.macro) - parent_records = records.map do |record| - descendant = record.send(reflection.name) - next unless descendant - descendant.target.uniq! if is_collection - descendant - end.flatten.compact + parent_records = [] + records.each do |record| + if descendant = record.send(reflection.name) + if is_collection + parent_records.concat descendant.target.uniq + else + parent_records << descendant + end + end + end remove_duplicate_results!(reflection.klass, parent_records, associations[name]) unless parent_records.empty? end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 75218c01d2..9b96ec0cf4 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -256,10 +256,22 @@ module ActiveRecord end end - # Array#flatten has problems with recursive arrays. Going one level - # deeper solves the majority of the problems. - def flatten_deeper(array) - array.collect { |element| (element.respond_to?(:flatten) && !element.is_a?(Hash)) ? element.flatten : element }.flatten + if RUBY_VERSION < '1.9.2' + # Array#flatten has problems with recursive arrays before Ruby 1.9.2. + # Going one level deeper solves the majority of the problems. + def flatten_deeper(array) + array.collect { |element| (element.respond_to?(:flatten) && !element.is_a?(Hash)) ? element.flatten : element }.flatten + end + else + def flatten_deeper(array) + array.sum [] do |elem| + if elem.respond_to?(:each) + flatten_deeper(elem) + else + Array.wrap(elem) + end + end + end end # Returns the ID of the owner, quoted if needed. -- cgit v1.2.3 From 0da71980cd61aa6297749d0ea8520785c9a5280f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 13 Nov 2009 10:38:23 -0800 Subject: Missing customers fixture --- activerecord/test/cases/copy_table_test_sqlite.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/copy_table_test_sqlite.rb b/activerecord/test/cases/copy_table_test_sqlite.rb index de8af30997..575b4806c1 100644 --- a/activerecord/test/cases/copy_table_test_sqlite.rb +++ b/activerecord/test/cases/copy_table_test_sqlite.rb @@ -1,7 +1,7 @@ require "cases/helper" class CopyTableTest < ActiveRecord::TestCase - fixtures :companies, :comments + fixtures :customers, :companies, :comments def setup @connection = ActiveRecord::Base.connection @@ -27,8 +27,8 @@ class CopyTableTest < ActiveRecord::TestCase test_copy_table('customers', 'customers2', :rename => {'name' => 'person_name'}) do |from, to, options| expected = column_values(from, 'name') - assert expected.any?, 'only nils in resultset; real values are needed' assert_equal expected, column_values(to, 'person_name') + assert expected.any?, "No values in table: #{expected.inspect}" end end -- cgit v1.2.3 From bd51790895fc75a3b4e19e8dd7aa6dc389d77068 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 13 Nov 2009 10:44:34 -0800 Subject: Split arel_table into method to get a relation and another to memoize the default relation. --- activerecord/lib/active_record/associations.rb | 4 ++-- .../associations/has_and_belongs_to_many_association.rb | 4 ++-- .../lib/active_record/associations/has_many_association.rb | 2 +- activerecord/lib/active_record/base.rb | 12 ++++++------ activerecord/lib/active_record/calculations.rb | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6c5e25010f..fc18b53b9d 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1698,7 +1698,7 @@ module ActiveRecord def construct_finder_arel_with_included_associations(options, join_dependency) scope = scope(:find) - relation = arel_table((scope && scope[:from]) || options[:from]) + relation = arel_table_for((scope && scope[:from]) || options[:from]) for association in join_dependency.join_associations relation = association.join_relation(relation) @@ -1741,7 +1741,7 @@ module ActiveRecord def construct_finder_sql_for_association_limiting(options, join_dependency) scope = scope(:find) - relation = arel_table(options[:from]) + relation = arel_table_for(options[:from]) for association in join_dependency.join_associations relation = association.join_relation(relation) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index c646fe488b..ce4e96637b 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -56,7 +56,7 @@ module ActiveRecord if @reflection.options[:insert_sql] @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record)) else - relation = arel_table(@reflection.options[:join_table]) + relation = arel_table_for(@reflection.options[:join_table]) attributes = columns.inject({}) do |attrs, column| case column.name.to_s when @reflection.primary_key_name.to_s @@ -82,7 +82,7 @@ module ActiveRecord if sql = @reflection.options[:delete_sql] records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) } else - relation = arel_table(@reflection.options[:join_table]) + relation = arel_table_for(@reflection.options[:join_table]) relation.conditions(relation[@reflection.primary_key_name].eq(@owner.id). and(Arel::Predicates::In.new(relation[@reflection.association_foreign_key], records.map(&:id))) ).delete diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cd31b0e211..36a668c284 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -69,7 +69,7 @@ module ActiveRecord when :delete_all @reflection.klass.delete(records.map { |record| record.id }) else - relation = arel_table(@reflection.table_name) + relation = arel_table_for(@reflection.table_name) relation.conditions(relation[@reflection.primary_key_name].eq(@owner.id). and(Arel::Predicates::In.new(relation[@reflection.klass.primary_key], records.map(&:id))) ).update(relation[@reflection.primary_key_name] => nil) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 056f29f029..778c36361d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1493,11 +1493,11 @@ module ActiveRecord #:nodoc: def arel_table(table = nil) - table = table_name if table.blank? - if @arel_table.nil? || @arel_table.name != table - @arel_table = Relation.new(self, Arel::Table.new(table)) - end - @arel_table + @arel_table ||= arel_table_for(table_name) + end + + def arel_table_for(table_name) + Relation.new(self, Arel::Table.new(table_name)) end private @@ -1666,7 +1666,7 @@ module ActiveRecord #:nodoc: def construct_finder_arel(options = {}, scope = scope(:find)) # TODO add lock to Arel - relation = arel_table(options[:from]). + relation = arel_table_for(options[:from]). joins(construct_join(options[:joins], scope)). conditions(construct_conditions(options[:conditions], scope)). select(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))). diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 40242333e5..46545d96a3 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -146,7 +146,7 @@ module ActiveRecord join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, construct_join(options[:joins], scope)) construct_finder_arel_with_included_associations(options, join_dependency) else - relation = arel_table(options[:from]). + relation = arel_table_for(options[:from]). joins(construct_join(options[:joins], scope)). conditions(construct_conditions(options[:conditions], scope)). order(options[:order]). @@ -164,7 +164,7 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, options, relation) #:nodoc: column = if column_names.include?(column_name.to_s) - Arel::Attribute.new(arel_table(options[:from] || table_name), + Arel::Attribute.new(arel_table_for(options[:from] || table_name), options[:select] || column_name) else Arel::SqlLiteral.new(options[:select] || -- cgit v1.2.3 From 7b3d85db4c58e2f719981efd6ef5a6b870f6ab49 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 13 Nov 2009 15:30:51 -0800 Subject: Revert "Split arel_table into method to get a relation and another to memoize the default relation." This reverts commit bd51790895fc75a3b4e19e8dd7aa6dc389d77068. --- activerecord/lib/active_record/associations.rb | 4 ++-- .../associations/has_and_belongs_to_many_association.rb | 4 ++-- .../lib/active_record/associations/has_many_association.rb | 2 +- activerecord/lib/active_record/base.rb | 12 ++++++------ activerecord/lib/active_record/calculations.rb | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index fc18b53b9d..6c5e25010f 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1698,7 +1698,7 @@ module ActiveRecord def construct_finder_arel_with_included_associations(options, join_dependency) scope = scope(:find) - relation = arel_table_for((scope && scope[:from]) || options[:from]) + relation = arel_table((scope && scope[:from]) || options[:from]) for association in join_dependency.join_associations relation = association.join_relation(relation) @@ -1741,7 +1741,7 @@ module ActiveRecord def construct_finder_sql_for_association_limiting(options, join_dependency) scope = scope(:find) - relation = arel_table_for(options[:from]) + relation = arel_table(options[:from]) for association in join_dependency.join_associations relation = association.join_relation(relation) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index ce4e96637b..c646fe488b 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -56,7 +56,7 @@ module ActiveRecord if @reflection.options[:insert_sql] @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record)) else - relation = arel_table_for(@reflection.options[:join_table]) + relation = arel_table(@reflection.options[:join_table]) attributes = columns.inject({}) do |attrs, column| case column.name.to_s when @reflection.primary_key_name.to_s @@ -82,7 +82,7 @@ module ActiveRecord if sql = @reflection.options[:delete_sql] records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) } else - relation = arel_table_for(@reflection.options[:join_table]) + relation = arel_table(@reflection.options[:join_table]) relation.conditions(relation[@reflection.primary_key_name].eq(@owner.id). and(Arel::Predicates::In.new(relation[@reflection.association_foreign_key], records.map(&:id))) ).delete diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 36a668c284..cd31b0e211 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -69,7 +69,7 @@ module ActiveRecord when :delete_all @reflection.klass.delete(records.map { |record| record.id }) else - relation = arel_table_for(@reflection.table_name) + relation = arel_table(@reflection.table_name) relation.conditions(relation[@reflection.primary_key_name].eq(@owner.id). and(Arel::Predicates::In.new(relation[@reflection.klass.primary_key], records.map(&:id))) ).update(relation[@reflection.primary_key_name] => nil) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 778c36361d..056f29f029 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1493,11 +1493,11 @@ module ActiveRecord #:nodoc: def arel_table(table = nil) - @arel_table ||= arel_table_for(table_name) - end - - def arel_table_for(table_name) - Relation.new(self, Arel::Table.new(table_name)) + table = table_name if table.blank? + if @arel_table.nil? || @arel_table.name != table + @arel_table = Relation.new(self, Arel::Table.new(table)) + end + @arel_table end private @@ -1666,7 +1666,7 @@ module ActiveRecord #:nodoc: def construct_finder_arel(options = {}, scope = scope(:find)) # TODO add lock to Arel - relation = arel_table_for(options[:from]). + relation = arel_table(options[:from]). joins(construct_join(options[:joins], scope)). conditions(construct_conditions(options[:conditions], scope)). select(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))). diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 46545d96a3..40242333e5 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -146,7 +146,7 @@ module ActiveRecord join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, construct_join(options[:joins], scope)) construct_finder_arel_with_included_associations(options, join_dependency) else - relation = arel_table_for(options[:from]). + relation = arel_table(options[:from]). joins(construct_join(options[:joins], scope)). conditions(construct_conditions(options[:conditions], scope)). order(options[:order]). @@ -164,7 +164,7 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, options, relation) #:nodoc: column = if column_names.include?(column_name.to_s) - Arel::Attribute.new(arel_table_for(options[:from] || table_name), + Arel::Attribute.new(arel_table(options[:from] || table_name), options[:select] || column_name) else Arel::SqlLiteral.new(options[:select] || -- cgit v1.2.3 From 92253829dee9a1fb545f71f71894dee1df604f74 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 14 Nov 2009 01:12:49 -0800 Subject: Ruby 1.9.2: fix flatten_deeper to preserve nils --- activerecord/lib/active_record/associations/association_proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 9b96ec0cf4..3cd1a1d922 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -268,7 +268,7 @@ module ActiveRecord if elem.respond_to?(:each) flatten_deeper(elem) else - Array.wrap(elem) + [elem] end end end -- cgit v1.2.3 From f0f4dffd3b469c0f4ba743c960110e8663955436 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 14 Nov 2009 01:22:36 -0800 Subject: Skip pg locking test due to connection checkout deadlock detection --- activerecord/test/cases/locking_test.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index e177235591..7e51526370 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -282,11 +282,14 @@ unless current_adapter?(:SybaseAdapter, :OpenBaseAdapter) assert first.end > second.end end - def test_second_lock_waits - assert [0.2, 1, 5].any? { |zzz| - first, second = duel(zzz) { Person.find 1, :lock => true } - second.end > first.end - } + # Hit by ruby deadlock detection since connection checkout is mutexed. + if RUBY_VERSION < '1.9.2' + def test_second_lock_waits + assert [0.2, 1, 5].any? { |zzz| + first, second = duel(zzz) { Person.find 1, :lock => true } + second.end > first.end + } + end end protected -- cgit v1.2.3 From 6ebb061b18cd5af087453879b3eac0f719ea4ec4 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 14 Nov 2009 01:50:47 -0800 Subject: Ruby 1.9.2: use recursive flatten --- activerecord/lib/active_record/associations/association_proxy.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 3cd1a1d922..7d8f4670fa 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -264,13 +264,7 @@ module ActiveRecord end else def flatten_deeper(array) - array.sum [] do |elem| - if elem.respond_to?(:each) - flatten_deeper(elem) - else - [elem] - end - end + array.flatten end end -- cgit v1.2.3 From 364a8f390216c4c5695ded89dacab0978fa6ae34 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 14 Nov 2009 01:51:52 -0800 Subject: No need to check for generated method, just redispatch --- activerecord/lib/active_record/attribute_methods.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index ab7ad34b9e..3a9a67e3a2 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -31,11 +31,10 @@ module ActiveRecord self.class.define_attribute_methods method_name = method_id.to_s guard_private_attribute_method!(method_name, args) - if self.class.generated_attribute_methods.instance_methods.include?(method_name) - return self.send(method_id, *args, &block) - end + send(method_id, *args, &block) + else + super end - super end def respond_to?(*args) -- cgit v1.2.3 From 7601d482bd187bdd06d3aa7ac1e8c7bb805f40a3 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 15 Nov 2009 10:28:48 -0800 Subject: Ruby 1.9: skip pg locking test for 1.9.1 also --- activerecord/test/cases/locking_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 7e51526370..f946e8699e 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -283,7 +283,7 @@ unless current_adapter?(:SybaseAdapter, :OpenBaseAdapter) end # Hit by ruby deadlock detection since connection checkout is mutexed. - if RUBY_VERSION < '1.9.2' + if RUBY_VERSION < '1.9.0' def test_second_lock_waits assert [0.2, 1, 5].any? { |zzz| first, second = duel(zzz) { Person.find 1, :lock => true } -- cgit v1.2.3 From fb61fbd35229154f4ced124568697878822336cb Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 17 Nov 2009 15:35:35 -0800 Subject: Revert "Ensure Model#destroy respects optimistic locking" [#1966 state:open] This reverts commit 0d922885fb54c19f04680482f024452859218910. Conflicts: activerecord/lib/active_record/locking/optimistic.rb --- .../lib/active_record/locking/optimistic.rb | 34 ---------------------- activerecord/test/cases/locking_test.rb | 18 ------------ 2 files changed, 52 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index c8cd79a2b0..986bc7009b 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -23,16 +23,6 @@ module ActiveRecord # p2.first_name = "should fail" # p2.save # Raises a ActiveRecord::StaleObjectError # - # Optimistic locking will also check for stale data when objects are destroyed. Example: - # - # p1 = Person.find(1) - # p2 = Person.find(1) - # - # p1.first_name = "Michael" - # p1.save - # - # p2.destroy # Raises a ActiveRecord::StaleObjectError - # # You're then responsible for dealing with the conflict by rescuing the exception and either rolling back, merging, # or otherwise apply the business logic needed to resolve the conflict. # @@ -49,7 +39,6 @@ module ActiveRecord self.lock_optimistically = true alias_method_chain :update, :lock - alias_method_chain :destroy, :lock alias_method_chain :attributes_from_column_definition, :lock class << self @@ -111,29 +100,6 @@ module ActiveRecord end end - def destroy_with_lock #:nodoc: - return destroy_without_lock unless locking_enabled? - - unless new_record? - lock_col = self.class.locking_column - previous_value = send(lock_col).to_i - - arel_table = self.class.arel_table(self.class.table_name) - - affected_rows = arel_table.where( - arel_table[self.class.primary_key].eq(quoted_id).and( - arel_table[self.class.locking_column].eq(quote_value(previous_value)) - ) - ).delete - - unless affected_rows == 1 - raise ActiveRecord::StaleObjectError, "Attempted to delete a stale object" - end - end - - freeze - end - module ClassMethods DEFAULT_LOCKING_COLUMN = 'lock_version' diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index f946e8699e..a64c01292f 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -38,24 +38,6 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_raise(ActiveRecord::StaleObjectError) { p2.save! } end - def test_lock_destroy - p1 = Person.find(1) - p2 = Person.find(1) - assert_equal 0, p1.lock_version - assert_equal 0, p2.lock_version - - p1.first_name = 'stu' - p1.save! - assert_equal 1, p1.lock_version - assert_equal 0, p2.lock_version - - assert_raises(ActiveRecord::StaleObjectError) { p2.destroy } - - assert p1.destroy - assert_equal true, p1.frozen? - assert_raises(ActiveRecord::RecordNotFound) { Person.find(1) } - end - def test_lock_repeating p1 = Person.find(1) p2 = Person.find(1) -- cgit v1.2.3 From ea290e77e6c50b13a0c9000eceaa5412de6918bc Mon Sep 17 00:00:00 2001 From: Gabe da Silveira Date: Tue, 17 Nov 2009 09:36:23 -0800 Subject: Insert generated association members in the same order they are specified when assigning to a has_many :through using the generated *_ids method [#3491 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 5 +++-- .../has_many_through_associations_test.rb | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6c5e25010f..fc6f15206a 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/module/delegation' +require 'active_support/core_ext/enumerable' module ActiveRecord class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc: @@ -1396,8 +1397,8 @@ module ActiveRecord end define_method("#{reflection.name.to_s.singularize}_ids=") do |new_value| - ids = (new_value || []).reject { |nid| nid.blank? } - send("#{reflection.name}=", reflection.klass.find(ids)) + ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i) + send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids)) end end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 5f13b66d11..fe68d03de2 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -137,6 +137,28 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert !posts(:welcome).reload.people(true).include?(people(:michael)) end + def test_replace_order_is_preserved + posts(:welcome).people.clear + posts(:welcome).people = [people(:david), people(:michael)] + assert_equal [people(:david).id, people(:michael).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id) + + # Test the inverse order in case the first success was a coincidence + posts(:welcome).people.clear + posts(:welcome).people = [people(:michael), people(:david)] + assert_equal [people(:michael).id, people(:david).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id) + end + + def test_replace_by_id_order_is_preserved + posts(:welcome).people.clear + posts(:welcome).person_ids = [people(:david).id, people(:michael).id] + assert_equal [people(:david).id, people(:michael).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id) + + # Test the inverse order in case the first success was a coincidence + posts(:welcome).people.clear + posts(:welcome).person_ids = [people(:michael).id, people(:david).id] + assert_equal [people(:michael).id, people(:david).id], posts(:welcome).readers.all(:order => 'id').map(&:person_id) + end + def test_associate_with_create assert_queries(1) { posts(:thinking) } -- cgit v1.2.3