diff options
Diffstat (limited to 'activerecord')
18 files changed, 195 insertions, 61 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 474b11f537..309cd633c3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,37 @@ +* `ActiveRecord::ConnectionAdapters.string_to_time` respects + string with timezone (e.g. Wed, 04 Sep 2013 20:30:00 JST). + + Fixes: #12278 + + *kennyj* + +* Calling `update_attributes` will now throw an `ArgumentError` whenever it + gets a `nil` argument. More specifically, it will throw an error if the + argument that it gets passed does not respond to to `stringify_keys`. + + Example: + + @my_comment.update_attributes(nil) # => raises ArgumentError + + *John Wang* + +* Deprecate `quoted_locking_column` method, which isn't used anywhere. + + *kennyj* + +* Migration dump UUID default functions to schema.rb. + + Fixes #10751. + + *kennyj* + +* Fixed a bug in `ActiveRecord::Associations::CollectionAssociation#find_by_scan` + when using `has_many` association with `:inverse_of` option and UUID primary key. + + Fixes #10450. + + *kennyj* + * ActiveRecord::Base#<=> has been removed. Primary keys may not be in order, or even be numbers, so sorting by id doesn't make sense. Please use `sort_by` and specify the attribute you wish to sort with. For example, change: @@ -8,6 +42,8 @@ Post.all.to_a.sort_by(&:id) + *Aaron Patterson* + * Fix: joins association, with defined in the scope block constraints by using several where constraints and at least of them is not `Arel::Nodes::Equality`, generates invalid SQL expression. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index e32c0b0377..98d573a3a2 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -554,14 +554,14 @@ module ActiveRecord # specified, then #find scans the entire collection. def find_by_scan(*args) expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.map{ |arg| arg.to_i }.uniq + ids = args.flatten.compact.map{ |arg| arg.to_s }.uniq if ids.size == 1 id = ids.first - record = load_target.detect { |r| id == r.id } + record = load_target.detect { |r| id == r.id.to_s } expects_array ? [ record ] : record else - load_target.select { |r| ids.include?(r.id) } + load_target.select { |r| ids.include?(r.id.to_s) } end end diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 4f06955406..30fa2c8ba5 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -12,6 +12,9 @@ module ActiveRecord # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> # exception is raised. def assign_attributes(new_attributes) + if !new_attributes.respond_to?(:stringify_keys) + raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." + end return if new_attributes.blank? attributes = new_attributes.stringify_keys diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index dc2399643c..19e81abba5 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -19,8 +19,7 @@ module ActiveRecord # Attempts to +save+ the record and clears changed attributes if successful. def save(*) if status = super - @previously_changed = changes - @changed_attributes.clear + changes_applied end status end @@ -28,16 +27,14 @@ module ActiveRecord # Attempts to <tt>save!</tt> the record and clears changed attributes if successful. def save!(*) super.tap do - @previously_changed = changes - @changed_attributes.clear + changes_applied end end # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - @previously_changed.clear - @changed_attributes.clear + reset_changes end end @@ -48,11 +45,11 @@ module ActiveRecord # The attribute already has an unsaved change. if attribute_changed?(attr) - old = @changed_attributes[attr] - @changed_attributes.delete(attr) unless _field_changed?(attr, old, value) + old = changed_attributes[attr] + changed_attributes.delete(attr) unless _field_changed?(attr, old, value) else old = clone_attribute_value(:read_attribute, attr) - @changed_attributes[attr] = old if _field_changed?(attr, old, value) + changed_attributes[attr] = old if _field_changed?(attr, old, value) end # Carry on. diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index fb53090edc..2596c221bc 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -203,11 +203,19 @@ module ActiveRecord end end - def new_time(year, mon, mday, hour, min, sec, microsec) + def new_time(year, mon, mday, hour, min, sec, microsec, offset = nil) # Treat 0000-00-00 00:00:00 as nil. return nil if year.nil? || (year == 0 && mon == 0 && mday == 0) - Time.send(Base.default_timezone, year, mon, mday, hour, min, sec, microsec) rescue nil + if offset + time = Time.utc(year, mon, mday, hour, min, sec, microsec) rescue nil + return nil unless time + + time -= offset + Base.default_timezone == :utc ? time : time.getlocal + else + Time.public_send(Base.default_timezone, year, mon, mday, hour, min, sec, microsec) rescue nil + end end def fast_string_to_date(string) @@ -232,7 +240,7 @@ module ActiveRecord time_hash = Date._parse(string) time_hash[:sec_fraction] = microseconds(time_hash) - new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction)) + new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction, :offset)) end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 13978fd113..af240fab95 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -45,16 +45,18 @@ module ActiveRecord module ConnectionAdapters # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: - attr_accessor :array + attr_accessor :array, :default_function # Instantiates a new PostgreSQL column definition in a table. def initialize(name, default, oid_type, sql_type = nil, null = true) @oid_type = oid_type + default_value = self.class.extract_value_from_default(default) + @default_function = default if !default_value && default && default =~ /.+\(.*\)/ if sql_type =~ /\[\]$/ @array = true - super(name, self.class.extract_value_from_default(default), sql_type[0..sql_type.length - 3], null) + super(name, default_value, sql_type[0..sql_type.length - 3], null) else @array = false - super(name, self.class.extract_value_from_default(default), sql_type, null) + super(name, default_value, sql_type, null) end end @@ -439,6 +441,7 @@ module ActiveRecord def prepare_column_options(column, types) spec = super spec[:array] = 'true' if column.respond_to?(:array) && column.array + spec[:default] = "\"#{column.default_function}\"" if column.respond_to?(:default_function) && column.default_function spec end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 5e6fee52f7..366ebde418 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -163,7 +163,7 @@ module ActiveRecord # ==== Example: # # Instantiates a single new object # User.new(first_name: 'Jamie') - def initialize(attributes = nil) + def initialize(attributes = nil, options = {}) defaults = self.class.column_defaults.dup defaults.each { |k, v| defaults[k] = v.dup if v.duplicable? } @@ -176,7 +176,9 @@ module ActiveRecord ensure_proper_type populate_with_current_scope_attributes - assign_attributes(attributes) if attributes + # +options+ argument is only needed to make protected_attributes gem easier to hook. + # Remove it when we drop support to this gem. + init_attributes(attributes, options) if attributes yield self if block_given? run_callbacks :initialize unless _initialize_callbacks.empty? @@ -411,8 +413,6 @@ module ActiveRecord @aggregation_cache = {} @association_cache = {} @attributes_cache = {} - @previously_changed = {} - @changed_attributes = {} @readonly = false @destroyed = false @marked_for_destruction = false @@ -429,8 +429,14 @@ module ActiveRecord # optimistic locking) won't get written unless they get marked as changed self.class.columns.each do |c| attr, orig_value = c.name, c.default - @changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) + changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) end end + + # This method is needed to make protected_attributes gem easier to hook. + # Remove it when we drop support to this gem. + def init_attributes(attributes, options) + assign_attributes(attributes) + end end end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 626fe40103..55776a91c0 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -150,6 +150,7 @@ module ActiveRecord # Quote the column name used for optimistic locking. def quoted_locking_column + ActiveSupport::Deprecation.warn "ActiveRecord::Base.quoted_locking_column is deprecated and will be removed in Rails 4.2 or later." connection.quote_column_name(locking_column) end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index d630f31f5f..bdd00ee259 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -434,7 +434,7 @@ module ActiveRecord changes[self.class.locking_column] = increment_lock if locking_enabled? - @changed_attributes.except!(*changes.keys) + changed_attributes.except!(*changes.keys) primary_key = self.class.primary_key self.class.unscoped.where(primary_key => self[primary_key]).update_all(changes) == 1 end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 1181cc739e..8986d255cd 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -17,9 +17,19 @@ module ActiveRecord cattr_accessor :ignore_tables @@ignore_tables = [] - def self.dump(connection=ActiveRecord::Base.connection, stream=STDOUT) - new(connection).dump(stream) - stream + class << self + def dump(connection=ActiveRecord::Base.connection, stream=STDOUT, config = ActiveRecord::Base) + new(connection, generate_options(config)).dump(stream) + stream + end + + private + def generate_options(config) + { + table_name_prefix: config.table_name_prefix, + table_name_suffix: config.table_name_suffix + } + end end def dump(stream) @@ -32,10 +42,11 @@ module ActiveRecord private - def initialize(connection) + def initialize(connection, options = {}) @connection = connection @types = @connection.native_database_types @version = Migrator::current_version rescue nil + @options = options end def header(stream) @@ -201,7 +212,7 @@ HEADER end def remove_prefix_and_suffix(table) - table.gsub(/^(#{ActiveRecord::Base.table_name_prefix})(.+)(#{ActiveRecord::Base.table_name_suffix})$/, "\\2") + table.gsub(/^(#{@options[:table_name_prefix]})(.+)(#{@options[:table_name_suffix]})$/, "\\2") end end end diff --git a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb index f1c4b85126..c5fd40accc 100644 --- a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb +++ b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb @@ -1,38 +1,40 @@ require 'cases/helper' -module ActiveRecord::ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter - class InactivePGconn - def query(*args) - raise PGError - end +module ActiveRecord + module ConnectionAdapters + class PostgreSQLAdapter < AbstractAdapter + class InactivePGconn + def query(*args) + raise PGError + end - def status - PGconn::CONNECTION_BAD + def status + PGconn::CONNECTION_BAD + end end - end - class StatementPoolTest < ActiveRecord::TestCase - def test_cache_is_per_pid - return skip('must support fork') unless Process.respond_to?(:fork) + class StatementPoolTest < ActiveRecord::TestCase + def test_cache_is_per_pid + return skip('must support fork') unless Process.respond_to?(:fork) - cache = StatementPool.new nil, 10 - cache['foo'] = 'bar' - assert_equal 'bar', cache['foo'] + cache = StatementPool.new nil, 10 + cache['foo'] = 'bar' + assert_equal 'bar', cache['foo'] - pid = fork { - lookup = cache['foo']; - exit!(!lookup) - } + pid = fork { + lookup = cache['foo']; + exit!(!lookup) + } - Process.waitpid pid - assert $?.success?, 'process should exit successfully' - end + 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 } + 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/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index b573d48807..0cd5b420fc 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -61,6 +61,7 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase schema = StringIO.new ActiveRecord::SchemaDumper.dump(@connection, schema) assert_match(/\bcreate_table "pg_uuids", id: :uuid\b/, schema.string) + assert_match(/t\.uuid "other_uuid", default: "uuid_generate_v4\(\)"/, schema.string) end end @@ -93,3 +94,43 @@ class PostgresqlUUIDTestNilDefault < ActiveRecord::TestCase assert_nil col_desc["default"] end end + +class PostgresqlUUIDTestInverseOf < ActiveRecord::TestCase + class UuidPost < ActiveRecord::Base + self.table_name = 'pg_uuid_posts' + has_many :uuid_comments, inverse_of: :uuid_post + end + + class UuidComment < ActiveRecord::Base + self.table_name = 'pg_uuid_comments' + belongs_to :uuid_post + end + + def setup + @connection = ActiveRecord::Base.connection + @connection.reconnect! + + @connection.transaction do + @connection.create_table('pg_uuid_posts', id: :uuid) do |t| + t.string 'title' + end + @connection.create_table('pg_uuid_comments', id: :uuid) do |t| + t.uuid :uuid_post_id, default: 'uuid_generate_v4()' + t.string 'content' + end + end + end + + def teardown + @connection.transaction do + @connection.execute 'drop table if exists pg_uuid_comments' + @connection.execute 'drop table if exists pg_uuid_posts' + end + end + + def test_collection_association_with_uuid + post = UuidPost.create! + comment = post.uuid_comments.create! + assert post.uuid_comments.find(comment.id) + end +end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index baba55ea43..9c66ed354e 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -92,7 +92,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_set_attributes_without_hash topic = Topic.new - assert_nothing_raised { topic.attributes = '' } + assert_raise(ArgumentError) { topic.attributes = '' } end def test_integers_as_nil diff --git a/activerecord/test/cases/column_test.rb b/activerecord/test/cases/column_test.rb index 3a4f414ae8..5ab2f18e9d 100644 --- a/activerecord/test/cases/column_test.rb +++ b/activerecord/test/cases/column_test.rb @@ -110,6 +110,16 @@ module ActiveRecord assert_equal 1800, column.type_cast(30.minutes) assert_equal 7200, column.type_cast(2.hours) end + + def test_string_to_time_with_timezone + old = ActiveRecord::Base.default_timezone + [:utc, :local].each do |zone| + ActiveRecord::Base.default_timezone = zone + assert_equal Time.utc(2013, 9, 4, 0, 0, 0), Column.string_to_time("Wed, 04 Sep 2013 03:00:00 EAT") + end + rescue + ActiveRecord::Base.default_timezone = old + end end end end diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 490b599fb6..981a75faf6 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -61,4 +61,9 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase assert_equal 'Guille', person.first_name assert_equal 'm', person.gender end + + def test_blank_attributes_should_not_raise + person = Person.new + assert_nil person.assign_attributes(ProtectedParams.new({})) + end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index dfa12cb97c..a16ed963fe 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -272,6 +272,10 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert p.treasures.empty? assert RichPerson.connection.select_all("SELECT * FROM peoples_treasures WHERE rich_person_id = 1").empty? end + + def test_quoted_locking_column_is_deprecated + assert_deprecated { ActiveRecord::Base.quoted_locking_column } + end end class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 30dc2a34c6..6cd3e2154e 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -419,10 +419,6 @@ class PersistenceTest < ActiveRecord::TestCase assert !Topic.find(1).approved? end - def test_update_attribute_does_not_choke_on_nil - assert Topic.find(1).update(nil) - end - def test_update_attribute_for_readonly_attribute minivan = Minivan.find('m1') assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } @@ -701,6 +697,17 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal topic.title, Topic.find(1234).title end + def test_update_attributes_parameters + topic = Topic.find(1) + assert_nothing_raised do + topic.update_attributes({}) + end + + assert_raises(ArgumentError) do + topic.update_attributes(nil) + end + end + def test_update! Reply.validates_presence_of(:title) reply = Reply.find(2) diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index a48ae1036f..32f86f9c88 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -299,7 +299,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dump_includes_uuid_shorthand_definition output = standard_dump - if %r{create_table "poistgresql_uuids"} =~ output + if %r{create_table "postgresql_uuids"} =~ output assert_match %r{t.uuid "guid"}, output end end |