diff options
38 files changed, 360 insertions, 169 deletions
diff --git a/actionview/test/template/compiled_templates_test.rb b/actionview/test/template/compiled_templates_test.rb index 2336321f3e..b84aca6746 100644 --- a/actionview/test/template/compiled_templates_test.rb +++ b/actionview/test/template/compiled_templates_test.rb @@ -1,15 +1,8 @@ require 'abstract_unit' class CompiledTemplatesTest < ActiveSupport::TestCase - def setup - # Clean up any details key cached to expose failures - # that otherwise would appear just on isolated tests + teardown do ActionView::LookupContext::DetailsKey.clear - - @compiled_templates = ActionView::CompiledTemplates - @compiled_templates.instance_methods.each do |m| - @compiled_templates.send(:remove_method, m) if m =~ /^_render_template_/ - end end def test_template_gets_recompiled_when_using_different_keys_in_local_assigns diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index fc726d6519..823597fc92 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Detect in-place modifications of PG array types + + *Sean Griffin* + * Add `bin/rake db:purge` task to empty the current database. *Yves Senn* diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 9d1310b576..ca71834641 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -40,7 +40,7 @@ module ActiveRecord def initialize_dup(other) # :nodoc: super - init_changed_attributes + calculate_changes_from_defaults end def changed? @@ -71,17 +71,9 @@ module ActiveRecord private - def initialize_internals_callback - super - init_changed_attributes - end - - def init_changed_attributes + def calculate_changes_from_defaults @changed_attributes = nil - # Intentionally avoid using #column_defaults since overridden defaults (as is done in - # 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 + self.class.column_defaults.each do |attr, orig_value| changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 0b788ea1f9..662c99269e 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -308,10 +308,10 @@ module ActiveRecord #:nodoc: include Integration include Validations include CounterCache - include Locking::Optimistic - include Locking::Pessimistic include Attributes include AttributeDecorators + include Locking::Optimistic + include Locking::Pessimistic include AttributeMethods include Callbacks include Timestamp diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index 5a0efe49c7..9bd0401e40 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -23,7 +23,8 @@ module ActiveRecord spec[:precision] = column.precision.inspect if column.precision spec[:scale] = column.scale.inspect if column.scale spec[:null] = 'false' unless column.null - spec[:default] = column.type_cast_for_schema(column.default) if column.has_default? + spec[:default] = schema_default(column) if column.has_default? + spec.delete(:default) if spec[:default].nil? spec end @@ -31,6 +32,15 @@ module ActiveRecord def migration_keys [:name, :limit, :precision, :scale, :default, :null] end + + private + + def schema_default(column) + default = column.type_cast_from_database(column.default) + unless default.nil? + column.type_cast_for_schema(default) + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index a9b97d5919..3ef8878ad1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -62,15 +62,14 @@ module ActiveRecord @extra = extra super(name, default, cast_type, sql_type, null) assert_valid_default(default) + extract_default end - def default - @default ||= if blob_or_text_column? - null || strict ? nil : '' - elsif missing_default_forged_as_empty_string?(@original_default) - nil - else - super + def extract_default + if blob_or_text_column? + @default = null || strict ? nil : '' + elsif missing_default_forged_as_empty_string?(@default) + @default = nil end end diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index d629fca911..8be4678ace 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -13,7 +13,7 @@ module ActiveRecord ISO_DATETIME = /\A(\d{4})-(\d\d)-(\d\d) (\d\d):(\d\d):(\d\d)(\.\d+)?\z/ end - attr_reader :name, :cast_type, :null, :sql_type, :default_function + attr_reader :name, :cast_type, :null, :sql_type, :default, :default_function delegate :type, :precision, :scale, :limit, :klass, :accessor, :text?, :number?, :binary?, :changed?, @@ -35,7 +35,7 @@ module ActiveRecord @cast_type = cast_type @sql_type = sql_type @null = null - @original_default = default + @default = default @default_function = nil end @@ -51,13 +51,8 @@ module ActiveRecord Base.human_attribute_name(@name) end - def default - @default ||= type_cast_from_database(@original_default) - end - def with_type(type) dup.tap do |clone| - clone.instance_variable_set('@default', nil) clone.instance_variable_set('@cast_type', type) end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb index bb54de05c8..a865c5c310 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb @@ -3,12 +3,16 @@ module ActiveRecord module PostgreSQL module Cast # :nodoc: def point_to_string(point) # :nodoc: - "(#{point[0]},#{point[1]})" + "(#{number_for_point(point[0])},#{number_for_point(point[1])})" + end + + def number_for_point(number) + number.to_s.gsub(/\.0$/, '') end def hstore_to_string(object, array_member = false) # :nodoc: if Hash === object - string = object.map { |k, v| "#{escape_hstore(k)}=>#{escape_hstore(v)}" }.join(',') + string = object.map { |k, v| "#{escape_hstore(k)}=>#{escape_hstore(v)}" }.join(', ') string = escape_hstore(string) if array_member string else @@ -38,21 +42,6 @@ module ActiveRecord end end - def array_to_string(value, column, adapter) # :nodoc: - casted_values = value.map do |val| - if String === val - if val == "NULL" - "\"#{val}\"" - else - quote_and_escape(adapter.type_cast(val, column, true)) - end - else - adapter.type_cast(val, column, true) - end - end - "{#{casted_values.join(',')}}" - end - def range_to_string(object) # :nodoc: from = object.begin.respond_to?(:infinite?) && object.begin.infinite? ? '' : object.begin to = object.end.respond_to?(:infinite?) && object.end.infinite? ? '' : object.end @@ -86,19 +75,6 @@ module ActiveRecord end end end - - ARRAY_ESCAPE = "\\" * 2 * 2 # escape the backslash twice for PG arrays - - def quote_and_escape(value) - case value - when "NULL", Numeric - value - else - value = value.gsub(/\\/, ARRAY_ESCAPE) - value.gsub!(/"/,"\\\"") - "\"#{value}\"" - end - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb index 4e7d472d97..d322c56acc 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb @@ -3,11 +3,26 @@ module ActiveRecord module PostgreSQL module OID # :nodoc: class Array < Type::Value - attr_reader :subtype + include Type::Mutable + + # Loads pg_array_parser if available. String parsing can be + # performed quicker by a native extension, which will not create + # a large amount of Ruby objects that will need to be garbage + # collected. pg_array_parser has a C and Java extension + begin + require 'pg_array_parser' + include PgArrayParser + rescue LoadError + require 'active_record/connection_adapters/postgresql/array_parser' + include PostgreSQL::ArrayParser + end + + attr_reader :subtype, :delimiter delegate :type, to: :subtype - def initialize(subtype) + def initialize(subtype, delimiter = ',') @subtype = subtype + @delimiter = delimiter end def type_cast_from_database(value) @@ -22,16 +37,12 @@ module ActiveRecord type_cast_array(value, :type_cast_from_user) end - # Loads pg_array_parser if available. String parsing can be - # performed quicker by a native extension, which will not create - # a large amount of Ruby objects that will need to be garbage - # collected. pg_array_parser has a C and Java extension - begin - require 'pg_array_parser' - include PgArrayParser - rescue LoadError - require 'active_record/connection_adapters/postgresql/array_parser' - include PostgreSQL::ArrayParser + def type_cast_for_database(value) + if value.is_a?(::Array) + cast_value_for_database(value) + else + super + end end private @@ -43,6 +54,41 @@ module ActiveRecord @subtype.public_send(method, value) end end + + def cast_value_for_database(value) + if value.is_a?(::Array) + casted_values = value.map { |item| cast_value_for_database(item) } + "{#{casted_values.join(delimiter)}}" + else + quote_and_escape(subtype.type_cast_for_database(value)) + end + end + + ARRAY_ESCAPE = "\\" * 2 * 2 # escape the backslash twice for PG arrays + + def quote_and_escape(value) + case value + when ::String + if string_requires_quoting?(value) + value = value.gsub(/\\/, ARRAY_ESCAPE) + value.gsub!(/"/,"\\\"") + %("#{value}") + else + value + end + when nil then "NULL" + else value + end + end + + # See http://www.postgresql.org/docs/9.2/static/arrays.html#ARRAYS-IO + # for a list of all cases in which strings will be quoted. + def string_requires_quoting?(string) + string.empty? || + string == "NULL" || + string =~ /[\{\}"\\\s]/ || + string.include?(delimiter) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb index 9007bfb178..86277c5542 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb @@ -3,20 +3,33 @@ module ActiveRecord module PostgreSQL module OID # :nodoc: class Point < Type::Value + include Type::Mutable + def type :point end def type_cast(value) - if ::String === value + case value + when ::String if value[0] == '(' && value[-1] == ')' value = value[1...-1] end - value.split(',').map{ |v| Float(v) } + type_cast(value.split(',')) + when ::Array + value.map { |v| Float(v) } else value end end + + def type_cast_for_database(value) + if value.is_a?(::Array) + PostgreSQLColumn.point_to_string(value) + else + super + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb index 28f7a4eafb..e396ff4a1e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb @@ -40,7 +40,7 @@ module ActiveRecord def register_array_type(row) if subtype = @store.lookup(row['typelem'].to_i) - register row['oid'], OID::Array.new(subtype) + register row['oid'], OID::Array.new(subtype, row['typdelim']) end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 3cf40e6cd4..17fabe5af6 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -32,11 +32,7 @@ module ActiveRecord when 'point' then super(PostgreSQLColumn.point_to_string(value)) when 'json' then super(PostgreSQLColumn.json_to_string(value)) else - if column.array - "'#{PostgreSQLColumn.array_to_string(value, column, self).gsub(/'/, "''")}'" - else - super - end + super(value, array_column(column)) end when Hash case sql_type @@ -98,11 +94,7 @@ module ActiveRecord when 'point' then PostgreSQLColumn.point_to_string(value) when 'json' then PostgreSQLColumn.json_to_string(value) else - if column.array - PostgreSQLColumn.array_to_string(value, column, self) - else - super(value, column) - end + super(value, array_column(column)) end when Hash case column.sql_type @@ -185,6 +177,26 @@ module ActiveRecord super end end + + def array_column(column) + if column.array && !column.respond_to?(:type_cast_for_database) + OID::Array.new(AdapterProxyType.new(column, self)) + else + column + end + end + + class AdapterProxyType < SimpleDelegator + def initialize(column, adapter) + @column = column + @adapter = adapter + super(column) + end + + def type_cast_for_database(value) + @adapter.type_cast(value, @column) + end + end end end end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 4528d8783c..0a764fb7ad 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -53,6 +53,11 @@ module ActiveRecord included do class_attribute :lock_optimistically, instance_writer: false self.lock_optimistically = true + + is_lock_column = ->(name, _) { lock_optimistically && name == locking_column } + decorate_matching_attribute_types(is_lock_column, :_optimistic_locking) do |type| + LockingType.new(type) + end end def locking_enabled? #:nodoc: @@ -141,7 +146,7 @@ module ActiveRecord # Set the column to use for optimistic locking. Defaults to +lock_version+. def locking_column=(value) - @column_defaults = nil + clear_caches_calculated_from_columns @locking_column = value.to_s end @@ -162,18 +167,26 @@ module ActiveRecord counters = counters.merge(locking_column => 1) if locking_enabled? super end + end + end - def column_defaults - @column_defaults ||= begin - defaults = super + class LockingType < SimpleDelegator + def type_cast_from_database(value) + # `nil` *should* be changed to 0 + super.to_i + end - if defaults.key?(locking_column) && lock_optimistically - defaults[locking_column] ||= 0 - end + def changed?(old_value, *) + # Ensure we save if the default was `nil` + super || old_value == 0 + end - defaults - end - end + def init_with(coder) + __setobj__(coder['subtype']) + end + + def encode_with(coder) + coder['subtype'] = __getobj__ end end end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index b9558b0752..fad7eae461 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -230,14 +230,16 @@ module ActiveRecord # Returns a hash where the keys are column names and the values are # default values when instantiating the AR object for this table. def column_defaults - @column_defaults ||= Hash[columns.map { |c| [c.name, c.default] }] + @column_defaults ||= Hash[columns_hash.map { |name, column| + [name, column.type_cast_from_database(column.default)] + }] end # Returns a hash where the keys are the column names and the values # are the default values suitable for use in `@raw_attriubtes` def raw_column_defaults # :nodoc: - @raw_column_defauts ||= Hash[column_defaults.map { |name, default| - [name, columns_hash[name].type_cast_for_database(default)] + @raw_column_defaults ||= Hash[columns_hash.map { |name, column| + [name, column.default] }] end @@ -285,7 +287,7 @@ module ActiveRecord @arel_engine = nil @column_defaults = nil - @raw_column_defauts = nil + @raw_column_defaults = nil @column_names = nil @column_types = nil @content_columns = nil diff --git a/activerecord/lib/active_record/type/date_time.rb b/activerecord/lib/active_record/type/date_time.rb index 560d63c101..5f19608a33 100644 --- a/activerecord/lib/active_record/type/date_time.rb +++ b/activerecord/lib/active_record/type/date_time.rb @@ -7,6 +7,16 @@ module ActiveRecord :datetime end + def type_cast_for_database(value) + zone_conversion_method = ActiveRecord::Base.default_timezone == :utc ? :getutc : :getlocal + + if value.acts_like?(:time) + value.send(zone_conversion_method) + else + super + end + end + private def cast_value(string) diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 0b1e3295cc..a51d5e9d31 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -3,6 +3,7 @@ require "cases/helper" class PostgresqlArrayTest < ActiveRecord::TestCase include InTimeZone + OID = ActiveRecord::ConnectionAdapters::PostgreSQL::OID class PgArray < ActiveRecord::Base self.table_name = 'pg_arrays' @@ -10,11 +11,20 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def setup @connection = ActiveRecord::Base.connection + + unless @connection.extension_enabled?('hstore') + @connection.enable_extension 'hstore' + @connection.commit_db_transaction + end + + @connection.reconnect! + @connection.transaction do @connection.create_table('pg_arrays') do |t| t.string 'tags', array: true t.integer 'ratings', array: true t.datetime :datetimes, array: true + t.hstore :hstores, array: true end end @column = PgArray.columns_hash['tags'] @@ -41,9 +51,8 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_default @connection.add_column 'pg_arrays', 'score', :integer, array: true, default: [4, 4, 2] PgArray.reset_column_information - column = PgArray.columns_hash["score"] - assert_equal([4, 4, 2], column.default) + assert_equal([4, 4, 2], PgArray.column_defaults['score']) assert_equal([4, 4, 2], PgArray.new.score) ensure PgArray.reset_column_information @@ -52,9 +61,8 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_default_strings @connection.add_column 'pg_arrays', 'names', :string, array: true, default: ["foo", "bar"] PgArray.reset_column_information - column = PgArray.columns_hash["names"] - assert_equal(["foo", "bar"], column.default) + assert_equal(["foo", "bar"], PgArray.column_defaults['names']) assert_equal(["foo", "bar"], PgArray.new.names) ensure PgArray.reset_column_information @@ -68,7 +76,7 @@ class PostgresqlArrayTest < ActiveRecord::TestCase column = PgArray.columns_hash['snippets'] assert_equal :text, column.type - assert_equal [], column.default + assert_equal [], PgArray.column_defaults['snippets'] assert column.array end @@ -85,8 +93,7 @@ class PostgresqlArrayTest < ActiveRecord::TestCase @connection.change_column_default :pg_arrays, :tags, [] PgArray.reset_column_information - column = PgArray.columns_hash['tags'] - assert_equal [], column.default + assert_equal [], PgArray.column_defaults['tags'] end def test_type_cast_array @@ -203,6 +210,45 @@ class PostgresqlArrayTest < ActiveRecord::TestCase assert_equal tags, ar.tags end + def test_string_quoting_rules_match_pg_behavior + tags = ["", "one{", "two}", %(three"), "four\\", "five ", "six\t", "seven\n", "eight,", "nine", "ten\r", "NULL"] + x = PgArray.create!(tags: tags) + x.reload + + assert_equal x.tags_before_type_cast, PgArray.columns_hash['tags'].type_cast_for_database(tags) + end + + def test_quoting_non_standard_delimiters + strings = ["hello,", "world;"] + comma_delim = OID::Array.new(ActiveRecord::Type::String.new, ',') + semicolon_delim = OID::Array.new(ActiveRecord::Type::String.new, ';') + + assert_equal %({"hello,",world;}), comma_delim.type_cast_for_database(strings) + assert_equal %({hello,;"world;"}), semicolon_delim.type_cast_for_database(strings) + end + + def test_mutate_array + x = PgArray.create!(tags: %w(one two)) + + x.tags << "three" + x.save! + x.reload + + assert_equal %w(one two three), x.tags + assert_not x.changed? + end + + def test_mutate_value_in_array + x = PgArray.create!(hstores: [{ a: 'a' }, { b: 'b' }]) + + x.hstores.first['a'] = 'c' + x.save! + x.reload + + assert_equal [{ 'a' => 'c' }, { 'b' => 'b' }], x.hstores + assert_not x.changed? + end + def test_datetime_with_timezone_awareness tz = "Pacific Time (US & Canada)" diff --git a/activerecord/test/cases/adapters/postgresql/bit_string_test.rb b/activerecord/test/cases/adapters/postgresql/bit_string_test.rb index 3a9397bc26..9ee3610afd 100644 --- a/activerecord/test/cases/adapters/postgresql/bit_string_test.rb +++ b/activerecord/test/cases/adapters/postgresql/bit_string_test.rb @@ -43,12 +43,10 @@ class PostgresqlBitStringTest < ActiveRecord::TestCase end def test_default - column = PostgresqlBitString.columns_hash["a_bit"] - assert_equal "00000011", column.default + assert_equal "00000011", PostgresqlBitString.column_defaults['a_bit'] assert_equal "00000011", PostgresqlBitString.new.a_bit - column = PostgresqlBitString.columns_hash["a_bit_varying"] - assert_equal "0011", column.default + assert_equal "0011", PostgresqlBitString.column_defaults['a_bit_varying'] assert_equal "0011", PostgresqlBitString.new.a_bit_varying end diff --git a/activerecord/test/cases/adapters/postgresql/enum_test.rb b/activerecord/test/cases/adapters/postgresql/enum_test.rb index b809f1a79c..0e97f37a6c 100644 --- a/activerecord/test/cases/adapters/postgresql/enum_test.rb +++ b/activerecord/test/cases/adapters/postgresql/enum_test.rb @@ -42,9 +42,8 @@ class PostgresqlEnumTest < ActiveRecord::TestCase def test_enum_defaults @connection.add_column 'postgresql_enums', 'good_mood', :mood, default: 'happy' PostgresqlEnum.reset_column_information - column = PostgresqlEnum.columns_hash["good_mood"] - assert_equal "happy", column.default + assert_equal "happy", PostgresqlEnum.column_defaults['good_mood'] assert_equal "happy", PostgresqlEnum.new.good_mood ensure PostgresqlEnum.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/geometric_test.rb b/activerecord/test/cases/adapters/postgresql/geometric_test.rb index 2f106ee664..faf195783d 100644 --- a/activerecord/test/cases/adapters/postgresql/geometric_test.rb +++ b/activerecord/test/cases/adapters/postgresql/geometric_test.rb @@ -35,12 +35,10 @@ class PostgresqlPointTest < ActiveRecord::TestCase end def test_default - column = PostgresqlPoint.columns_hash["y"] - assert_equal [12.2, 13.3], column.default + assert_equal [12.2, 13.3], PostgresqlPoint.column_defaults['y'] assert_equal [12.2, 13.3], PostgresqlPoint.new.y - column = PostgresqlPoint.columns_hash["z"] - assert_equal [14.4, 15.5], column.default + assert_equal [14.4, 15.5], PostgresqlPoint.column_defaults['z'] assert_equal [14.4, 15.5], PostgresqlPoint.new.z end @@ -61,4 +59,15 @@ class PostgresqlPointTest < ActiveRecord::TestCase assert record.reload assert_equal [1.1, 2.2], record.x end + + def test_mutation + p = PostgresqlPoint.create! x: [10, 20] + + p.x[1] = 25 + p.save! + p.reload + + assert_equal [10.0, 25.0], p.x + assert_not p.changed? + end end diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index 83b495d600..06788df4e1 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -64,9 +64,8 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase def test_default @connection.add_column 'hstores', 'permissions', :hstore, default: '"users"=>"read", "articles"=>"write"' Hstore.reset_column_information - column = Hstore.columns_hash["permissions"] - assert_equal({"users"=>"read", "articles"=>"write"}, column.default) + assert_equal({"users"=>"read", "articles"=>"write"}, Hstore.column_defaults['permissions']) assert_equal({"users"=>"read", "articles"=>"write"}, Hstore.new.permissions) ensure Hstore.reset_column_information @@ -170,6 +169,7 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase hstore.reload assert_equal 'four', hstore.settings['three'] + assert_not hstore.changed? end def test_gen1 diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index a3400a5a19..4cdb4a4893 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -43,9 +43,8 @@ class PostgresqlJSONTest < ActiveRecord::TestCase def test_default @connection.add_column 'json_data_type', 'permissions', :json, default: '{"users": "read", "posts": ["read", "write"]}' JsonDataType.reset_column_information - column = JsonDataType.columns_hash["permissions"] - assert_equal({"users"=>"read", "posts"=>["read", "write"]}, column.default) + assert_equal({"users"=>"read", "posts"=>["read", "write"]}, JsonDataType.column_defaults['permissions']) assert_equal({"users"=>"read", "posts"=>["read", "write"]}, JsonDataType.new.permissions) ensure JsonDataType.reset_column_information @@ -183,6 +182,7 @@ class PostgresqlJSONTest < ActiveRecord::TestCase json.save! json.reload - assert json.payload['three'] = 'four' + assert_equal({ 'one' => 'two', 'three' => 'four' }, json.payload) + assert_not json.changed? end end diff --git a/activerecord/test/cases/adapters/postgresql/money_test.rb b/activerecord/test/cases/adapters/postgresql/money_test.rb index bdfeedafab..cf2a4ab6ea 100644 --- a/activerecord/test/cases/adapters/postgresql/money_test.rb +++ b/activerecord/test/cases/adapters/postgresql/money_test.rb @@ -32,8 +32,7 @@ class PostgresqlMoneyTest < ActiveRecord::TestCase end def test_default - column = PostgresqlMoney.columns_hash["depth"] - assert_equal BigDecimal.new("150.55"), column.default + assert_equal BigDecimal.new("150.55"), PostgresqlMoney.column_defaults['depth'] assert_equal BigDecimal.new("150.55"), PostgresqlMoney.new.depth end diff --git a/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb new file mode 100644 index 0000000000..23817198b1 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb @@ -0,0 +1,15 @@ +require 'cases/helper' + +class PostgresqlTypeLookupTest < ActiveRecord::TestCase + setup do + @connection = ActiveRecord::Base.connection + end + + test "array delimiters are looked up correctly" do + box_array = @connection.type_map.lookup(1020) + int_array = @connection.type_map.lookup(1007) + + assert_equal ';', box_array.delimiter + assert_equal ',', int_array.delimiter + end +end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index e55525177f..b89caa3d55 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -339,7 +339,7 @@ module ActiveRecord column = @conn.columns('ex').find { |x| x.name == 'number' } - assert_equal 10, column.default + assert_equal '10', column.default end end diff --git a/activerecord/test/cases/attribute_decorators_test.rb b/activerecord/test/cases/attribute_decorators_test.rb index bc3e9a8cf5..b352d1a6c2 100644 --- a/activerecord/test/cases/attribute_decorators_test.rb +++ b/activerecord/test/cases/attribute_decorators_test.rb @@ -98,17 +98,7 @@ module ActiveRecord assert_equal 'Hello! decorated!', model.a_string assert_equal 'whatever', model.another_string assert_equal 'Hello! decorated! decorated!', child.a_string - # We are round tripping the default, and we don't undo our decoration - assert_equal 'whatever decorated! decorated!', child.another_string - end - - test "defaults are decorated on the column" do - Model.attribute :a_string, Type::String.new, default: 'whatever' - Model.decorate_attribute_type(:a_string, :test) { |t| StringDecorator.new(t) } - - column = Model.columns_hash['a_string'] - - assert_equal 'whatever decorated!', column.default + assert_equal 'whatever decorated!', child.another_string end class Multiplier < SimpleDelegator diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index 92144bc802..c089e63128 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -154,7 +154,7 @@ if current_adapter?(:MysqlAdapter, :Mysql2Adapter) t.column :omit, :integer, :null => false end - assert_equal 0, klass.columns_hash['zero'].default + assert_equal '0', klass.columns_hash['zero'].default assert !klass.columns_hash['zero'].null # 0 in MySQL 4, nil in 5. assert [0, nil].include?(klass.columns_hash['omit'].default) diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index c221430757..0c9dff2c25 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -272,6 +272,13 @@ 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_yaml_dumping_with_lock_column + t1 = LockWithoutDefault.new + t2 = YAML.load(YAML.dump(t1)) + + assert_equal t1.attributes, t2.attributes + end end class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index 9b26c30d14..c66eaf1ee1 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -68,9 +68,9 @@ module ActiveRecord five = columns.detect { |c| c.name == "five" } unless mysql assert_equal "hello", one.default - assert_equal true, two.default - assert_equal false, three.default - assert_equal 1, four.default + assert_equal true, two.type_cast_from_database(two.default) + assert_equal false, three.type_cast_from_database(three.default) + assert_equal '1', four.default assert_equal "hello", five.default unless mysql end @@ -275,7 +275,7 @@ module ActiveRecord person_klass.connection.add_column "testings", "wealth", :integer, :null => false, :default => 99 person_klass.reset_column_information - assert_equal 99, person_klass.columns_hash["wealth"].default + assert_equal 99, person_klass.column_defaults["wealth"] assert_equal false, person_klass.columns_hash["wealth"].null # Oracle needs primary key value from sequence if current_adapter?(:OracleAdapter) @@ -287,20 +287,20 @@ module ActiveRecord # change column default to see that column doesn't lose its not null definition person_klass.connection.change_column_default "testings", "wealth", 100 person_klass.reset_column_information - assert_equal 100, person_klass.columns_hash["wealth"].default + assert_equal 100, person_klass.column_defaults["wealth"] assert_equal false, person_klass.columns_hash["wealth"].null # rename column to see that column doesn't lose its not null and/or default definition person_klass.connection.rename_column "testings", "wealth", "money" person_klass.reset_column_information assert_nil person_klass.columns_hash["wealth"] - assert_equal 100, person_klass.columns_hash["money"].default + assert_equal 100, person_klass.column_defaults["money"] assert_equal false, person_klass.columns_hash["money"].null # change column person_klass.connection.change_column "testings", "money", :integer, :null => false, :default => 1000 person_klass.reset_column_information - assert_equal 1000, person_klass.columns_hash["money"].default + assert_equal 1000, person_klass.column_defaults["money"] assert_equal false, person_klass.columns_hash["money"].null # change column, make it nullable and clear default diff --git a/activerecord/test/cases/migration/columns_test.rb b/activerecord/test/cases/migration/columns_test.rb index a7c287515d..4e6d7963aa 100644 --- a/activerecord/test/cases/migration/columns_test.rb +++ b/activerecord/test/cases/migration/columns_test.rb @@ -53,13 +53,13 @@ module ActiveRecord add_column 'test_models', 'salary', :integer, :default => 70000 default_before = connection.columns("test_models").find { |c| c.name == "salary" }.default - assert_equal 70000, default_before + assert_equal '70000', default_before rename_column "test_models", "salary", "annual_salary" assert TestModel.column_names.include?("annual_salary") default_after = connection.columns("test_models").find { |c| c.name == "annual_salary" }.default - assert_equal 70000, default_after + assert_equal '70000', default_after end if current_adapter?(:MysqlAdapter, :Mysql2Adapter) @@ -193,14 +193,21 @@ module ActiveRecord old_columns = connection.columns(TestModel.table_name) assert old_columns.find { |c| - c.name == 'approved' && c.type == :boolean && c.default == true + default = c.type_cast_from_database(c.default) + c.name == 'approved' && c.type == :boolean && default == true } change_column :test_models, :approved, :boolean, :default => false new_columns = connection.columns(TestModel.table_name) - assert_not new_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == true } - assert new_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == false } + assert_not new_columns.find { |c| + default = c.type_cast_from_database(c.default) + c.name == 'approved' and c.type == :boolean and default == true + } + assert new_columns.find { |c| + default = c.type_cast_from_database(c.default) + c.name == 'approved' and c.type == :boolean and default == false + } change_column :test_models, :approved, :boolean, :default => true end diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index a925cf4c05..1c0134843b 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -157,6 +157,23 @@ module ActiveRecord assert_equal [:remove_column, [:table, :column, :type, {}], nil], remove end + def test_invert_change_column + assert_raises(ActiveRecord::IrreversibleMigration) do + @recorder.inverse_of :change_column, [:table, :column, :type, {}] + end + end + + def test_invert_change_column_default + assert_raises(ActiveRecord::IrreversibleMigration) do + @recorder.inverse_of :change_column_default, [:table, :column, 'default_value'] + end + end + + def test_invert_change_column_null + add = @recorder.inverse_of :change_column_null, [:table, :column, true] + assert_equal [:change_column_null, [:table, :column, false]], add + end + def test_invert_remove_column add = @recorder.inverse_of :remove_column, [:table, :column, :type, {}] assert_equal [:add_column, [:table, :column, :type, {}], nil], add diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 9855835e27..6b840e16bb 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -567,7 +567,7 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert_equal 8, columns.size [:name, :qualification, :experience].each {|s| assert_equal :string, column(s).type } - assert_equal 0, column(:age).default + assert_equal '0', column(:age).default end def test_removing_columns diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb index 5934c578ea..8657f34be2 100644 --- a/activesupport/lib/active_support/core_ext/hash/keys.rb +++ b/activesupport/lib/active_support/core_ext/hash/keys.rb @@ -57,9 +57,11 @@ class Hash end alias_method :to_options!, :symbolize_keys! - # Validate all keys in a hash match <tt>*valid_keys</tt>, raising ArgumentError - # on a mismatch. Note that keys are NOT treated indifferently, meaning if you - # use strings for keys but assert symbols as keys, this will fail. + # Validate all keys in a hash match <tt>*valid_keys</tt>, raising + # ArgumentError on a mismatch. + # + # Note that keys are treated differently than HashWithIndifferentAccess, + # meaning that string and symbol keys will not match. # # { name: 'Rob', years: '28' }.assert_valid_keys(:name, :age) # => raises "ArgumentError: Unknown key: :years. Valid keys are: :name, :age" # { name: 'Rob', age: '28' }.assert_valid_keys('name', 'age') # => raises "ArgumentError: Unknown key: :name. Valid keys are: 'name', 'age'" diff --git a/activesupport/test/core_ext/kernel/concern_test.rb b/activesupport/test/core_ext/kernel/concern_test.rb index 9b1fdda3b0..478a00d2d2 100644 --- a/activesupport/test/core_ext/kernel/concern_test.rb +++ b/activesupport/test/core_ext/kernel/concern_test.rb @@ -6,7 +6,8 @@ class KernelConcernTest < ActiveSupport::TestCase mod = ::TOPLEVEL_BINDING.eval 'concern(:ToplevelConcern) { }' assert_equal mod, ::ToplevelConcern assert_kind_of ActiveSupport::Concern, ::ToplevelConcern - assert !Object.ancestors.include?(::ToplevelConcern), mod.ancestors.inspect + assert_not Object.ancestors.include?(::ToplevelConcern), mod.ancestors.inspect + ensure Object.send :remove_const, :ToplevelConcern end end diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 3350b1a4b2..c33a4ed192 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,17 @@ +* Deprecate `Rails::Rack::LogTailer` with not replacement. + + *Rafael Mendonça França* + +* Add a generic --skip-gems options to generator + + This option is useful if users want to remove some gems like jbuilder, + turbolinks, coffee-rails, etc that don't have specific options on the + generator. + + rails new my_app --skip-gems turbolinks coffee-rails + + *Rafael Mendonça França* + * Invalid `bin/rails generate` commands will now show spelling suggestions. *Richard Schneeman* diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 569afe8104..76f8a1b816 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -41,6 +41,9 @@ module Rails class_option :skip_active_record, type: :boolean, aliases: '-O', default: false, desc: 'Skip Active Record files' + class_option :skip_gems, type: :array, default: [], + desc: 'Skip the provided gems files' + class_option :skip_action_view, type: :boolean, aliases: '-V', default: false, desc: 'Skip Action View files' @@ -79,7 +82,7 @@ module Rails end def initialize(*args) - @gem_filter = lambda { |gem| true } + @gem_filter = lambda { |gem| !options[:skip_gems].include?(gem.name) } @extra_entries = [] super convert_database_option_for_jruby @@ -104,14 +107,14 @@ module Rails end def gemfile_entries - [ rails_gemfile_entry, - database_gemfile_entry, - assets_gemfile_entry, - javascript_gemfile_entry, - jbuilder_gemfile_entry, - sdoc_gemfile_entry, - spring_gemfile_entry, - @extra_entries].flatten.find_all(&@gem_filter) + [rails_gemfile_entry, + database_gemfile_entry, + assets_gemfile_entry, + javascript_gemfile_entry, + jbuilder_gemfile_entry, + sdoc_gemfile_entry, + spring_gemfile_entry, + @extra_entries].flatten.find_all(&@gem_filter) end def add_gem_entry_filter diff --git a/railties/lib/rails/rack/log_tailer.rb b/railties/lib/rails/rack/log_tailer.rb index 50d0eb96fc..bc26421a9e 100644 --- a/railties/lib/rails/rack/log_tailer.rb +++ b/railties/lib/rails/rack/log_tailer.rb @@ -1,7 +1,11 @@ +require 'active_support/deprecation' + module Rails module Rack class LogTailer def initialize(app, log = nil) + ActiveSupport::Deprecation.warn "LogTailer is deprecated and will be removed on Rails 5" + @app = app path = Pathname.new(log || "#{::File.expand_path(Rails.root)}/log/#{Rails.env}.log").cleanpath diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 74cff08676..2ac5410960 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -448,6 +448,21 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_generator_if_skip_gems_is_given + run_generator [destination_root, "--skip-gems", "turbolinks", "coffee-rails"] + + assert_file "Gemfile" do |content| + assert_no_match(/turbolinks/, content) + assert_no_match(/coffee-rails/, content) + end + assert_file "app/views/layouts/application.html.erb" do |content| + assert_no_match(/data-turbolinks-track/, content) + end + assert_file "app/assets/javascripts/application.js" do |content| + assert_no_match(/turbolinks/, content) + end + end + def test_gitignore_when_sqlite3 run_generator diff --git a/railties/test/railties/railtie_test.rb b/railties/test/railties/railtie_test.rb index a458240d2f..5042d628cf 100644 --- a/railties/test/railties/railtie_test.rb +++ b/railties/test/railties/railtie_test.rb @@ -73,7 +73,7 @@ module RailtiesTest end test "railtie have access to application in before_configuration callbacks" do - $after_initialize = false + $before_configuration = false class Foo < Rails::Railtie ; config.before_configuration { $before_configuration = Rails.root.to_path } ; end assert_not $before_configuration require "#{app_path}/config/environment" |