diff options
Diffstat (limited to 'activerecord')
9 files changed, 135 insertions, 17 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 0f6a31d679..46076dac61 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -29,6 +29,11 @@ [Andrés Mejía] +* Fix nested attributes bug where _destroy parameter is taken into account + during :reject_if => :all_blank (fixes #2937) + + [Aaron Christy] + *Rails 3.1.1 (October 7, 2011)* * Add deprecation for the preload_associations method. Fixes #3022. diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index 81ddbba51e..5a8addc4e4 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -172,8 +172,8 @@ module ActiveRecord # with this option. # * <tt>:mapping</tt> - Specifies the mapping of entity attributes to attributes of the value # object. Each mapping is represented as an array where the first item is the name of the - # entity attribute and the second item is the name the attribute in the value object. The - # order in which mappings are defined determine the order in which attributes are sent to the + # entity attribute and the second item is the name of the attribute in the value object. The + # order in which mappings are defined determines the order in which attributes are sent to the # value class constructor. # * <tt>:allow_nil</tt> - Specifies that the value object will not be instantiated when all mapped # attributes are +nil+. Setting the value object to +nil+ has the effect of writing +nil+ to all @@ -191,7 +191,8 @@ module ActiveRecord # # Option examples: # composed_of :temperature, :mapping => %w(reading celsius) - # composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } + # composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), + # :converter => Proc.new { |balance| balance.to_money } # composed_of :address, :mapping => [ %w(address_street street), %w(address_city city) ] # composed_of :gps_location # composed_of :gps_location, :allow_nil => true diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index d859843260..3d084bb178 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -278,13 +278,24 @@ module ActiveRecord cache.clear end + def delete(sql_key) + dealloc cache[sql_key] + cache.delete sql_key + end + private def cache @cache[$$] end def dealloc(key) - @connection.query "DEALLOCATE #{key}" + @connection.query "DEALLOCATE #{key}" if connection_active? + end + + def connection_active? + @connection.status == PGconn::CONNECTION_OK + rescue PGError + false end end @@ -1030,27 +1041,54 @@ module ActiveRecord end private + FEATURE_NOT_SUPPORTED = "0A000" # :nodoc: + def exec_no_cache(sql, binds) @connection.async_exec(sql) end def exec_cache(sql, binds) - sql_key = "#{schema_search_path}-#{sql}" + begin + stmt_key = prepare_statement sql + + # Clear the queue + @connection.get_last_result + @connection.send_query_prepared(stmt_key, binds.map { |col, val| + type_cast(val, col) + }) + @connection.block + @connection.get_last_result + rescue PGError => e + # Get the PG code for the failure. Annoyingly, the code for + # prepared statements whose return value may have changed is + # FEATURE_NOT_SUPPORTED. Check here for more details: + # http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/cache/plancache.c#l573 + code = e.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) + if FEATURE_NOT_SUPPORTED == code + @statements.delete sql_key(sql) + retry + else + raise e + end + end + end + + # Returns the statement identifier for the client side cache + # of statements + def sql_key(sql) + "#{schema_search_path}-#{sql}" + end + + # Prepare the statement if it hasn't been prepared, return + # the statement key. + def prepare_statement(sql) + sql_key = sql_key(sql) unless @statements.key? sql_key nextkey = @statements.next_key @connection.prepare nextkey, sql @statements[sql_key] = nextkey end - - key = @statements[sql_key] - - # Clear the queue - @connection.get_last_result - @connection.send_query_prepared(key, binds.map { |col, val| - type_cast(val, col) - }) - @connection.block - @connection.get_last_result + @statements[sql_key] end # The internal PostgreSQL identifier of the money data type. diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 1932a849ee..e0e957a12c 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -413,6 +413,8 @@ module ActiveRecord self.limit = options[:limit] if options.include?(:limit) self.default = options[:default] if include_default self.null = options[:null] if options.include?(:null) + self.precision = options[:precision] if options.include?(:precision) + self.scale = options[:scale] if options.include?(:scale) end end end @@ -467,6 +469,7 @@ module ActiveRecord @definition.column(column_name, column.type, :limit => column.limit, :default => column.default, + :precision => column.precision, :scale => column.scale, :null => column.null) end @definition.primary_key(primary_key(from)) if primary_key(from) diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 2dbebfcaf8..d2065d701f 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -220,7 +220,7 @@ module ActiveRecord # validates_presence_of :member # end module ClassMethods - REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |_, value| value.blank? } } + REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |key, value| key == '_destroy' || value.blank? } } # Defines an attributes writer for the specified association(s). If you # are using <tt>attr_protected</tt> or <tt>attr_accessible</tt>, then you @@ -239,7 +239,8 @@ module ActiveRecord # is specified, a record will be built for all attribute hashes that # do not have a <tt>_destroy</tt> value that evaluates to true. # Passing <tt>:all_blank</tt> instead of a Proc will create a proc - # that will reject a record where all the attributes are blank. + # that will reject a record where all the attributes are blank excluding + # any value for _destroy. # [:limit] # Allows you to specify the maximum number of the associated records that # can be processed with the nested attributes. If the size of the diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index b01eabc840..c8f8714f66 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -62,6 +62,14 @@ class SchemaTest < ActiveRecord::TestCase @connection.execute "DROP SCHEMA #{SCHEMA_NAME} CASCADE" end + def test_schema_change_with_prepared_stmt + @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]] + @connection.exec_query "alter table developers add column zomg int", 'sql', [] + @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]] + ensure + @connection.exec_query "alter table developers drop column if exists zomg", 'sql', [] + end + def test_table_exists? [Thing1, Thing2, Thing3, Thing4].each do |klass| name = klass.table_name diff --git a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb index a82c6f67d6..f1c4b85126 100644 --- a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb +++ b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb @@ -2,6 +2,16 @@ require 'cases/helper' module ActiveRecord::ConnectionAdapters class PostgreSQLAdapter < AbstractAdapter + class InactivePGconn + def query(*args) + raise PGError + end + + def status + PGconn::CONNECTION_BAD + end + end + class StatementPoolTest < ActiveRecord::TestCase def test_cache_is_per_pid return skip('must support fork') unless Process.respond_to?(:fork) @@ -18,6 +28,12 @@ module ActiveRecord::ConnectionAdapters Process.waitpid pid assert $?.success?, 'process should exit successfully' end + + def test_dealloc_does_not_raise_on_inactive_connection + cache = StatementPool.new InactivePGconn.new, 10 + cache['foo'] = 'bar' + assert_nothing_raised { cache.clear } + end end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 4b5dd18be8..49944eced9 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -474,6 +474,8 @@ if ActiveRecord::Base.connection.supports_migrations? Person.connection.execute "insert into people (id, wealth, created_at, updated_at) values (people_seq.nextval, 12345678901234567890.0123456789, 0, 0)" elsif current_adapter?(:OpenBaseAdapter) || (current_adapter?(:MysqlAdapter) && Mysql.client_version < 50003) #before mysql 5.0.3 decimals stored as strings Person.connection.execute "insert into people (wealth, created_at, updated_at) values ('12345678901234567890.0123456789', 0, 0)" + elsif current_adapter?(:PostgreSQLAdapter) + Person.connection.execute "insert into people (wealth, created_at, updated_at) values (12345678901234567890.0123456789, now(), now())" else Person.connection.execute "insert into people (wealth, created_at, updated_at) values (12345678901234567890.0123456789, 0, 0)" end @@ -516,6 +518,42 @@ if ActiveRecord::Base.connection.supports_migrations? assert_equal 7, wealth_column.scale end + # Test SQLite adapter specifically for decimal types with precision and scale + # attributes, since these need to be maintained in schema but aren't actually + # used in SQLite itself + if current_adapter?(:SQLite3Adapter) + def test_change_column_with_new_precision_and_scale + Person.delete_all + Person.connection.add_column 'people', 'wealth', :decimal, :precision => 9, :scale => 7 + Person.reset_column_information + + Person.connection.change_column 'people', 'wealth', :decimal, :precision => 12, :scale => 8 + Person.reset_column_information + + wealth_column = Person.columns_hash['wealth'] + assert_equal 12, wealth_column.precision + assert_equal 8, wealth_column.scale + end + + def test_change_column_preserve_other_column_precision_and_scale + Person.delete_all + Person.connection.add_column 'people', 'last_name', :string + Person.connection.add_column 'people', 'wealth', :decimal, :precision => 9, :scale => 7 + Person.reset_column_information + + wealth_column = Person.columns_hash['wealth'] + assert_equal 9, wealth_column.precision + assert_equal 7, wealth_column.scale + + Person.connection.change_column 'people', 'last_name', :string, :null => false + Person.reset_column_information + + wealth_column = Person.columns_hash['wealth'] + assert_equal 9, wealth_column.precision + assert_equal 7, wealth_column.scale + end + end + def test_native_types Person.delete_all Person.connection.add_column "people", "last_name", :string diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 67a9ed6cd8..2ae9cb4888 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -45,6 +45,14 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase end end + def test_should_not_build_a_new_record_using_reject_all_even_if_destroy_is_given + pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") + pirate.birds_with_reject_all_blank_attributes = [{:name => '', :color => '', :_destroy => '0'}] + pirate.save! + + assert pirate.birds_with_reject_all_blank.empty? + end + def test_should_not_build_a_new_record_if_reject_all_blank_returns_false pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") pirate.birds_with_reject_all_blank_attributes = [{:name => '', :color => ''}] |