diff options
21 files changed, 393 insertions, 23 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f4d108a478..0e624ddb4d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,44 @@ +* Restore 4.0 behavior for using serialize attributes with `JSON` as coder. + + With 4.1.x, `serialize` started returning a string when `JSON` was passed as + the second attribute. It will now return a hash as per previous versions. + + Example: + + class Post < ActiveRecord::Base + serialize :comment, JSON + end + + class Comment + include ActiveModel::Model + attr_accessor :category, :text + end + + post = Post.create! + post.comment = Comment.new(category: "Animals", text: "This is a comment about squirrels.") + post.save! + + # 4.0 + post.comment # => {"category"=>"Animals", "text"=>"This is a comment about squirrels."} + + # 4.1 before + post.comment # => "#<Comment:0x007f80ab48ff98>" + + # 4.1 after + post.comment # => {"category"=>"Animals", "text"=>"This is a comment about squirrels."} + + When using `JSON` as the coder in `serialize`, Active Record will use the + new `ActiveRecord::Coders::JSON` coder which delegates its `dump/load` to + `ActiveSupport::JSON.encode/decode`. This ensures special objects are dumped + correctly using the `#as_json` hook. + + To keep the previous behaviour, supply a custom coder instead + ([example](https://gist.github.com/jenncoop/8c4142bbe59da77daa63)). + + Fixes #15594. + + *Jenn Cooper* + * Do not use `RENAME INDEX` syntax for MariaDB 10.0. Fixes #15931. diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 17b00bbaea..9028970a3d 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -97,6 +97,7 @@ module ActiveRecord module Coders autoload :YAMLColumn, 'active_record/coders/yaml_column' + autoload :JSON, 'active_record/coders/json' end module AttributeMethods diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index 734d94865a..264ce2bdfa 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -37,7 +37,12 @@ module ActiveRecord # serialize :preferences, Hash # end def serialize(attr_name, class_name_or_coder = Object) - coder = if [:load, :dump].all? { |x| class_name_or_coder.respond_to?(x) } + # When ::JSON is used, force it to go through the Active Support JSON encoder + # to ensure special objects (e.g. Active Record models) are dumped correctly + # using the #as_json hook. + coder = if class_name_or_coder == ::JSON + Coders::JSON + elsif [:load, :dump].all? { |x| class_name_or_coder.respond_to?(x) } class_name_or_coder else Coders::YAMLColumn.new(class_name_or_coder) diff --git a/activerecord/lib/active_record/coders/json.rb b/activerecord/lib/active_record/coders/json.rb new file mode 100644 index 0000000000..75d3bfe625 --- /dev/null +++ b/activerecord/lib/active_record/coders/json.rb @@ -0,0 +1,13 @@ +module ActiveRecord + module Coders # :nodoc: + class JSON # :nodoc: + def self.dump(obj) + ActiveSupport::JSON.encode(obj) + end + + def self.load(json) + ActiveSupport::JSON.decode(json) unless json.nil? + end + end + end +end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 6b840e16bb..84720585f2 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -895,4 +895,14 @@ class CopyMigrationsTest < ActiveRecord::TestCase ensure ActiveRecord::Base.logger = old end + + private + + def quietly + silence_stream(STDOUT) do + silence_stream(STDERR) do + yield + end + end + end end diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index 186a1a2ade..f8d87a3661 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -3,10 +3,11 @@ require 'models/topic' require 'models/reply' require 'models/person' require 'models/traffic_light' +require 'models/post' require 'bcrypt' class SerializedAttributeTest < ActiveRecord::TestCase - fixtures :topics + fixtures :topics, :posts MyObject = Struct.new :attribute1, :attribute2 @@ -67,6 +68,40 @@ class SerializedAttributeTest < ActiveRecord::TestCase assert_equal(orig.content, clone.content) end + def test_serialized_json_attribute_returns_unserialized_value + Topic.serialize :content, JSON + my_post = posts(:welcome) + + t = Topic.new(content: my_post) + t.save! + t.reload + + assert_instance_of(Hash, t.content) + assert_equal(my_post.id, t.content["id"]) + assert_equal(my_post.title, t.content["title"]) + end + + def test_json_read_legacy_null + Topic.serialize :content, JSON + + # Force a row to have a JSON "null" instead of a database NULL (this is how + # null values are saved on 4.1 and before) + id = Topic.connection.insert "INSERT INTO topics (content) VALUES('null')" + t = Topic.find(id) + + assert_nil t.content + end + + def test_json_read_db_null + Topic.serialize :content, JSON + + # Force a row to have a database NULL instead of a JSON "null" + id = Topic.connection.insert "INSERT INTO topics (content) VALUES(NULL)" + t = Topic.find(id) + + assert_nil t.content + end + def test_serialized_attribute_declared_in_subclass hash = { 'important1' => 'value1', 'important2' => 'value2' } important_topic = ImportantTopic.create("important" => hash) diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 3e3a2828f3..572ad2ed18 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -288,6 +288,25 @@ module ActiveRecord @configuration.merge('port' => 10000), filename) end + + private + + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end end class MySQLStructureLoadTest < ActiveRecord::TestCase diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 005bcffa26..a667332afc 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,10 @@ +* Deprecate `capture` and `quietly`. + + These methods are not thread safe and may cause issues when used in threaded environments. + To avoid problems we are deprecating them. + + *Tom Meier* + * `DateTime#to_f` now preserves the fractional seconds instead of always rounding to `.0`. diff --git a/activesupport/lib/active_support/core_ext/kernel/reporting.rb b/activesupport/lib/active_support/core_ext/kernel/reporting.rb index f3f8416905..80c531b694 100644 --- a/activesupport/lib/active_support/core_ext/kernel/reporting.rb +++ b/activesupport/lib/active_support/core_ext/kernel/reporting.rb @@ -31,9 +31,13 @@ module Kernel # For compatibility def silence_stderr #:nodoc: + ActiveSupport::Deprecation.warn( + "#silence_stderr is deprecated and will be removed in the next release" + ) #not thread-safe silence_stream(STDERR) { yield } end + # Deprecated : this method is not thread safe # Silences any stream for the duration of the block. # # silence_stream(STDOUT) do @@ -82,6 +86,9 @@ module Kernel # stream = capture(:stderr) { system('echo error 1>&2') } # stream # => "error\n" def capture(stream) + ActiveSupport::Deprecation.warn( + "#capture(stream) is deprecated and will be removed in the next release" + ) #not thread-safe stream = stream.to_s captured_stream = Tempfile.new(stream) stream_io = eval("$#{stream}") @@ -105,6 +112,9 @@ module Kernel # # This method is not thread-safe. def quietly + ActiveSupport::Deprecation.warn( + "#quietly is deprecated and will be removed in the next release" + ) #not thread-safe silence_stream(STDOUT) do silence_stream(STDERR) do yield diff --git a/activesupport/test/core_ext/kernel_test.rb b/activesupport/test/core_ext/kernel_test.rb index d8bf81d02b..a87af0007c 100644 --- a/activesupport/test/core_ext/kernel_test.rb +++ b/activesupport/test/core_ext/kernel_test.rb @@ -30,14 +30,6 @@ class KernelTest < ActiveSupport::TestCase end - def test_silence_stderr - old_stderr_position = STDERR.tell - silence_stderr { STDERR.puts 'hello world' } - assert_equal old_stderr_position, STDERR.tell - rescue Errno::ESPIPE - # Skip if we can't STDERR.tell - end - def test_silence_stream old_stream_position = STDOUT.tell silence_stream(STDOUT) { STDOUT.puts 'hello world' } @@ -56,9 +48,11 @@ class KernelTest < ActiveSupport::TestCase def test_quietly old_stdout_position, old_stderr_position = STDOUT.tell, STDERR.tell - quietly do - puts 'see me, feel me' - STDERR.puts 'touch me, heal me' + assert_deprecated do + quietly do + puts 'see me, feel me' + STDERR.puts 'touch me, heal me' + end end assert_equal old_stdout_position, STDOUT.tell assert_equal old_stderr_position, STDERR.tell @@ -66,10 +60,6 @@ class KernelTest < ActiveSupport::TestCase # Skip if we can't STDERR.tell end - def test_silence_stderr_with_return_value - assert_equal 1, silence_stderr { 1 } - end - def test_class_eval o = Object.new class << o; @x = 1; end @@ -77,10 +67,18 @@ class KernelTest < ActiveSupport::TestCase end def test_capture - assert_equal 'STDERR', capture(:stderr) { $stderr.print 'STDERR' } - assert_equal 'STDOUT', capture(:stdout) { print 'STDOUT' } - assert_equal "STDERR\n", capture(:stderr) { system('echo STDERR 1>&2') } - assert_equal "STDOUT\n", capture(:stdout) { system('echo STDOUT') } + assert_deprecated do + assert_equal 'STDERR', capture(:stderr) { $stderr.print 'STDERR' } + end + assert_deprecated do + assert_equal 'STDOUT', capture(:stdout) { print 'STDOUT' } + end + assert_deprecated do + assert_equal "STDERR\n", capture(:stderr) { system('echo STDERR 1>&2') } + end + assert_deprecated do + assert_equal "STDOUT\n", capture(:stdout) { system('echo STDOUT') } + end end end diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index ee1c69502e..7aff56cbad 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -355,4 +355,21 @@ class DeprecationTest < ActiveSupport::TestCase end deprecator end + + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end end diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 36cd505977..b3e4505fc0 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -52,6 +52,11 @@ Upgrading from Rails 4.1 to Rails 4.2 NOTE: This section is a work in progress. +### Serialized attributes + +When assigning `nil` to a serialized attribute, it will be saved to the database +as `NULL` instead of passing the `nil` value through the coder (e.g. `"null"` +when using the `JSON` coder). Upgrading from Rails 4.0 to Rails 4.1 ------------------------------------- diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 3b71f2c2a3..e9abfac7a0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,21 @@ +* Add `Rails::Application.config_for` to load a configuration for the current + environment. + + # config/exception_notification.yml: + production: + url: http://127.0.0.1:8080 + namespace: my_app_production + development: + url: http://localhost:3001 + namespace: my_app_development + + # config/production.rb + MyApp::Application.configure do + config.middleware.use ExceptionNotifier, config_for(:exception_notification) + end + + *Rafael Mendonça França*, *DHH* + * Deprecate `Rails::Rack::LogTailer` without replacement. *Rafael Mendonça França* diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 362713eb75..c5fd08e743 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -187,6 +187,38 @@ module Rails end end + # Convenience for loading config/foo.yml for the current Rails env. + # + # Example: + # + # # config/exception_notification.yml: + # production: + # url: http://127.0.0.1:8080 + # namespace: my_app_production + # development: + # url: http://localhost:3001 + # namespace: my_app_development + # + # # config/production.rb + # MyApp::Application.configure do + # config.middleware.use ExceptionNotifier, config_for(:exception_notification) + # end + def config_for(name) + yaml = Pathname.new("#{paths["config"].existent.first}/#{name}.yml") + + if yaml.exist? + require "yaml" + require "erb" + (YAML.load(ERB.new(yaml.read).result) || {})[Rails.env] || {} + else + raise "Could not load configuration. No such file - #{yaml}" + end + rescue Psych::SyntaxError => e + raise "YAML syntax error occurred while parsing #{yaml}. " \ + "Please note that YAML must be consistently indented using spaces. Tabs are not allowed. " \ + "Error: #{e.message}" + end + # Stores some of the Rails initial environment parameters which # will be used by middlewares and engines to configure themselves. def env_config diff --git a/railties/lib/rails/generators/testing/behaviour.rb b/railties/lib/rails/generators/testing/behaviour.rb index 7576eba6e0..e0600d0b59 100644 --- a/railties/lib/rails/generators/testing/behaviour.rb +++ b/railties/lib/rails/generators/testing/behaviour.rb @@ -100,6 +100,23 @@ module Rails dirname, file_name = File.dirname(absolute), File.basename(absolute).sub(/\.rb$/, '') Dir.glob("#{dirname}/[0-9]*_*.rb").grep(/\d+_#{file_name}.rb$/).first end + + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end end end end diff --git a/railties/test/abstract_unit.rb b/railties/test/abstract_unit.rb index 9ccc286b4e..b6533a5fb2 100644 --- a/railties/test/abstract_unit.rb +++ b/railties/test/abstract_unit.rb @@ -26,3 +26,26 @@ end def jruby_skip(message = '') skip message if defined?(JRUBY_VERSION) end + +class ActiveSupport::TestCase + private + + unless defined?(:capture) + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end + end +end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 2e056be561..e6e2c67826 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -989,5 +989,89 @@ module ApplicationTests assert_equal Rails.application.config.action_mailer.show_previews, true end + + test "config_for loads custom configuration from yaml files" do + app_file 'config/custom.yml', <<-RUBY + development: + key: 'custom key' + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + + require "#{app_path}/config/environment" + + assert_equal 'custom key', Rails.application.config.my_custom_config['key'] + end + + test "config_for raises an exception if the file does not exist" do + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + + exception = assert_raises(RuntimeError) do + require "#{app_path}/config/environment" + end + + assert_equal "Could not load configuration. No such file - #{app_path}/config/custom.yml", exception.message + end + + test "config_for without the environment configured returns an empty hash" do + app_file 'config/custom.yml', <<-RUBY + test: + key: 'custom key' + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + require "#{app_path}/config/environment" + + assert_equal({}, Rails.application.config.my_custom_config) + end + + test "config_for with empty file returns an empty hash" do + app_file 'config/custom.yml', <<-RUBY + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + require "#{app_path}/config/environment" + + assert_equal({}, Rails.application.config.my_custom_config) + end + + test "config_for containing ERB tags should evaluate" do + app_file 'config/custom.yml', <<-RUBY + development: + key: <%= 'custom key' %> + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + require "#{app_path}/config/environment" + + assert_equal 'custom key', Rails.application.config.my_custom_config['key'] + end + + test "config_for with syntax error show a more descritive exception" do + app_file 'config/custom.yml', <<-RUBY + development: + key: foo: + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + + exception = assert_raises(RuntimeError) do + require "#{app_path}/config/environment" + end + + assert_match 'YAML syntax error occurred while parsing', exception.message + end end end diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 6d6de0fb52..172d724643 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -242,7 +242,7 @@ class ActionsTest < Rails::Generators::TestCase protected def action(*args, &block) - silence(:stdout){ generator.send(*args, &block) } + capture(:stdout){ generator.send(*args, &block) } end end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 2ac5410960..da634decd8 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -490,7 +490,7 @@ class AppGeneratorTest < Rails::Generators::TestCase protected def action(*args, &block) - silence(:stdout) { generator.send(*args, &block) } + capture(:stdout) { generator.send(*args, &block) } end def assert_gem(gem) diff --git a/railties/test/generators/generators_test_helper.rb b/railties/test/generators/generators_test_helper.rb index 77ec2f1c0c..de1e56e7b3 100644 --- a/railties/test/generators/generators_test_helper.rb +++ b/railties/test/generators/generators_test_helper.rb @@ -41,4 +41,12 @@ module GeneratorsTestHelper FileUtils.mkdir_p(destination) FileUtils.cp routes, destination end + + def quietly + silence_stream(STDOUT) do + silence_stream(STDERR) do + yield + end + end + end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 6c50911666..92d6a1729c 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -291,6 +291,33 @@ class ActiveSupport::TestCase include TestHelpers::Paths include TestHelpers::Rack include TestHelpers::Generation + + private + + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end + + def quietly + silence_stream(STDOUT) do + silence_stream(STDERR) do + yield + end + end + end end # Create a scope and build a fixture rails app |