diff options
21 files changed, 173 insertions, 39 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 539b2eec01..57b8b5dfc9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -2,7 +2,7 @@ * Added `Mime::NullType` class. This allows to use html?, xml?, json?..etc when the `format` of `request` is unknown, without raise an exception. - + *Angelo Capilleri* * Integrate the Journey gem into Action Dispatch so that the global namespace diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 1dc51d62e0..6705e531cb 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -16,10 +16,9 @@ module ActionDispatch def call(env) begin - response = @app.call(env) + response = (_, headers, body = @app.call(env)) - if response[1]['X-Cascade'] == 'pass' - body = response[2] + if headers['X-Cascade'] == 'pass' body.close if body.respond_to?(:close) raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" end diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb index ae84c376b9..ab8c8359fc 100644 --- a/activemodel/lib/active_model/validations/presence.rb +++ b/activemodel/lib/active_model/validations/presence.rb @@ -3,8 +3,8 @@ module ActiveModel module Validations class PresenceValidator < EachValidator # :nodoc: - def validate(record) - record.errors.add_on_blank(attributes, options) + def validate_each(record, attr_name, value) + record.errors.add(attr_name, :blank, options) if value.blank? end end diff --git a/activemodel/test/cases/validations/presence_validation_test.rb b/activemodel/test/cases/validations/presence_validation_test.rb index 510c13a7c3..f77d4da29f 100644 --- a/activemodel/test/cases/validations/presence_validation_test.rb +++ b/activemodel/test/cases/validations/presence_validation_test.rb @@ -41,7 +41,7 @@ class PresenceValidationTest < ActiveModel::TestCase end def test_validates_acceptance_of_with_custom_error_using_quotes - Person.validates_presence_of :karma, :message => "This string contains 'single' and \"double\" quotes" + Person.validates_presence_of :karma, message: "This string contains 'single' and \"double\" quotes" p = Person.new assert p.invalid? assert_equal "This string contains 'single' and \"double\" quotes", p.errors[:karma].last @@ -70,4 +70,38 @@ class PresenceValidationTest < ActiveModel::TestCase p[:karma] = "Cold" assert p.valid? end + + def test_allow_nil + Topic.validates_presence_of(:title, allow_nil: true) + + t = Topic.new(title: "something") + assert t.valid?, t.errors.full_messages + + t.title = "" + assert t.invalid? + assert_equal ["can't be blank"], t.errors[:title] + + t.title = " " + assert t.invalid?, t.errors.full_messages + assert_equal ["can't be blank"], t.errors[:title] + + t.title = nil + assert t.valid?, t.errors.full_messages + end + + def test_allow_blank + Topic.validates_presence_of(:title, allow_blank: true) + + t = Topic.new(title: "something") + assert t.valid?, t.errors.full_messages + + t.title = "" + assert t.valid?, t.errors.full_messages + + t.title = " " + assert t.valid?, t.errors.full_messages + + t.title = nil + assert t.valid?, t.errors.full_messages + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 272cef6fcf..756bad5507 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* `after_commit` and `after_rollback` now validate the `:on` option and raise an `ArgumentError` + if it is not one of `:create`, `:destroy` or ``:update` + + *Pascal Friederich* + * Improve ways to write `change` migrations, making the old `up` & `down` methods no longer necessary. * The methods `drop_table` and `remove_column` are now reversible, as long as the necessary information is given. diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index ce6998530f..4a608e4f7b 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -4,6 +4,7 @@ module ActiveRecord # See ActiveRecord::Transactions::ClassMethods for documentation. module Transactions extend ActiveSupport::Concern + ACTIONS = [:create, :destroy, :update] class TransactionError < ActiveRecordError # :nodoc: end @@ -224,11 +225,7 @@ module ActiveRecord # Note that transactional fixtures do not play well with this feature. Please # use the +test_after_commit+ gem to have these hooks fired in tests. def after_commit(*args, &block) - options = args.last - if options.is_a?(Hash) && options[:on] - options[:if] = Array(options[:if]) - options[:if] << "transaction_include_action?(:#{options[:on]})" - end + set_options_for_callbacks!(args) set_callback(:commit, :after, *args, &block) end @@ -236,12 +233,25 @@ module ActiveRecord # # Please check the documentation of +after_commit+ for options. def after_rollback(*args, &block) + set_options_for_callbacks!(args) + set_callback(:rollback, :after, *args, &block) + end + + private + + def set_options_for_callbacks!(args) options = args.last if options.is_a?(Hash) && options[:on] + assert_valid_transaction_action(options[:on]) options[:if] = Array(options[:if]) options[:if] << "transaction_include_action?(:#{options[:on]})" end - set_callback(:rollback, :after, *args, &block) + end + + def assert_valid_transaction_action(action) + unless ACTIONS.include?(action.to_sym) + raise ArgumentError, ":on conditions for after_commit and after_rollback callbacks have to be one of #{ACTIONS.join(",")}" + end end end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 2ddc449c12..869892e33f 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -244,6 +244,14 @@ class TransactionCallbacksTest < ActiveRecord::TestCase assert_equal :rollback, @first.last_after_transaction_error assert_equal [:after_rollback], @second.history end + + def test_after_rollback_callbacks_should_validate_on_condition + assert_raise(ArgumentError) { Topic.send(:after_rollback, :on => :save) } + end + + def test_after_commit_callbacks_should_validate_on_condition + assert_raise(ArgumentError) { Topic.send(:after_commit, :on => :save) } + end end diff --git a/activesupport/lib/active_support/buffered_logger.rb b/activesupport/lib/active_support/buffered_logger.rb index 0595446189..1cd0c2f790 100644 --- a/activesupport/lib/active_support/buffered_logger.rb +++ b/activesupport/lib/active_support/buffered_logger.rb @@ -2,6 +2,20 @@ require 'active_support/deprecation' require 'active_support/logger' module ActiveSupport - BufferedLogger = ActiveSupport::Deprecation::DeprecatedConstantProxy.new( - 'BufferedLogger', '::ActiveSupport::Logger') + class BufferedLogger < Logger + + def initialize(*args) + self.class._deprecation_warning + super + end + + def self.inherited(*) + _deprecation_warning + super + end + + def self._deprecation_warning + ::ActiveSupport::Deprecation.warn 'ActiveSupport::BufferedLogger is deprecated! Use ActiveSupport::Logger instead.' + end + end end diff --git a/activesupport/lib/active_support/core_ext/thread.rb b/activesupport/lib/active_support/core_ext/thread.rb index 6ad0b2d69c..5481766f10 100644 --- a/activesupport/lib/active_support/core_ext/thread.rb +++ b/activesupport/lib/active_support/core_ext/thread.rb @@ -65,6 +65,10 @@ class Thread private def locals - @locals || LOCK.synchronize { @locals ||= {} } + if defined?(@locals) + @locals + else + LOCK.synchronize { @locals ||= {} } + end end end unless Thread.instance_methods.include?(:thread_variable_set) diff --git a/activesupport/test/configurable_test.rb b/activesupport/test/configurable_test.rb index 215a6e06b0..d00273a028 100644 --- a/activesupport/test/configurable_test.rb +++ b/activesupport/test/configurable_test.rb @@ -38,7 +38,7 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase assert_equal :bar, Parent.config.foo end - test "configuration accessors is not available on instance" do + test "configuration accessors are not available on instance" do instance = Parent.new assert !instance.respond_to?(:bar) diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 51a7e4b2fe..9d913c27b0 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -923,10 +923,8 @@ class DependenciesTest < ActiveSupport::TestCase def test_remove_constant_does_not_autoload_already_removed_parents_as_a_side_effect with_autoloading_fixtures do - silence_warnings do - ::A - ::A::B - end + _ = ::A # assignment to silence parse-time warning "possibly useless use of :: in void context" + _ = ::A::B # assignment to silence parse-time warning "possibly useless use of :: in void context" ActiveSupport::Dependencies.remove_constant('A') ActiveSupport::Dependencies.remove_constant('A::B') assert !defined?(A) @@ -936,9 +934,7 @@ class DependenciesTest < ActiveSupport::TestCase def test_load_once_constants_should_not_be_unloaded with_autoloading_fixtures do ActiveSupport::Dependencies.autoload_once_paths = ActiveSupport::Dependencies.autoload_paths - silence_warnings do - ::A - end + _ = ::A # assignment to silence parse-time warning "possibly useless use of :: in void context" assert defined?(A) ActiveSupport::Dependencies.clear assert defined?(A) diff --git a/activesupport/test/deprecation/buffered_logger_test.rb b/activesupport/test/deprecation/buffered_logger_test.rb new file mode 100644 index 0000000000..bf11a4732c --- /dev/null +++ b/activesupport/test/deprecation/buffered_logger_test.rb @@ -0,0 +1,22 @@ +require 'abstract_unit' +require 'active_support/buffered_logger' + +class BufferedLoggerTest < ActiveSupport::TestCase + + def test_can_be_subclassed + warn = 'ActiveSupport::BufferedLogger is deprecated! Use ActiveSupport::Logger instead.' + + ActiveSupport::Deprecation.expects(:warn).with(warn).once + + Class.new(ActiveSupport::BufferedLogger) + end + + def test_issues_deprecation_when_instantiated + warn = 'ActiveSupport::BufferedLogger is deprecated! Use ActiveSupport::Logger instead.' + + ActiveSupport::Deprecation.expects(:warn).with(warn).once + + ActiveSupport::BufferedLogger.new(STDOUT) + end + +end diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 6dba76c7b2..6e93932d49 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -105,7 +105,7 @@ These configuration methods are to be called on a `Rails::Railtie` object, such * `config.log_tags` accepts a list of methods that respond to `request` object. This makes it easy to tag log lines with debug information like subdomain and request id — both very helpful in debugging multi-user production applications. -* `config.logger` accepts a logger conforming to the interface of Log4r or the default Ruby `Logger` class. Defaults to an instance of `ActiveSupport::BufferedLogger`, with auto flushing off in production mode. +* `config.logger` accepts a logger conforming to the interface of Log4r or the default Ruby `Logger` class. Defaults to an instance of `ActiveSupport::Logger`, with auto flushing off in production mode. * `config.middleware` allows you to configure the application's middleware. This is covered in depth in the [Configuring Middleware](#configuring-middleware) section below. @@ -437,7 +437,7 @@ There are a few configuration options available in Active Support: * `config.active_support.use_standard_json_time_format` enables or disables serializing dates to ISO 8601 format. Defaults to `true`. -* `ActiveSupport::BufferedLogger.silencer` is set to `false` to disable the ability to silence logging in a block. The default is `true`. +* `ActiveSupport::Logger.silencer` is set to `false` to disable the ability to silence logging in a block. The default is `true`. * `ActiveSupport::Cache::Store.logger` specifies the logger to use within cache store operations. @@ -637,7 +637,7 @@ Below is a comprehensive list of all the initializers found in Rails in the orde * `load_active_support` Requires `active_support/dependencies` which sets up the basis for Active Support. Optionally requires `active_support/all` if `config.active_support.bare` is un-truthful, which is the default. -* `initialize_logger` Initializes the logger (an `ActiveSupport::BufferedLogger` object) for the application and makes it accessible at `Rails.logger`, provided that no initializer inserted before this point has defined `Rails.logger`. +* `initialize_logger` Initializes the logger (an `ActiveSupport::Logger` object) for the application and makes it accessible at `Rails.logger`, provided that no initializer inserted before this point has defined `Rails.logger`. * `initialize_cache` If `Rails.cache` isn't set yet, initializes the cache by referencing the value in `config.cache_store` and stores the outcome as `Rails.cache`. If this object responds to the `middleware` method, its middleware is inserted before `Rack::Runtime` in the middleware stack. diff --git a/guides/source/debugging_rails_applications.md b/guides/source/debugging_rails_applications.md index addc3a63a8..2e90e8728c 100644 --- a/guides/source/debugging_rails_applications.md +++ b/guides/source/debugging_rails_applications.md @@ -107,7 +107,7 @@ It can also be useful to save information to log files at runtime. Rails maintai ### What is the Logger? -Rails makes use of the `ActiveSupport::BufferedLogger` class to write log information. You can also substitute another logger such as `Log4r` if you wish. +Rails makes use of the `ActiveSupport::Logger` class to write log information. You can also substitute another logger such as `Log4r` if you wish. You can specify an alternative logger in your `environment.rb` or any environment file: diff --git a/guides/source/migrations.md b/guides/source/migrations.md index 62b70b5571..f2aa72492f 100644 --- a/guides/source/migrations.md +++ b/guides/source/migrations.md @@ -108,7 +108,9 @@ of the migration. The name of the migration class (CamelCased version) should match the latter part of the file name. For example `20080906120000_create_products.rb` should define class `CreateProducts` and `20080906120001_add_details_to_products.rb` should define -`AddDetailsToProducts`. +`AddDetailsToProducts`. Rails uses this timestamp to determine which migration +should be run and in what order, so if you're copying a migration from another +application or generate a file yourself, be aware of its position in the order. Of course, calculating timestamps is no fun, so Active Record provides a generator to handle making it for you: diff --git a/guides/source/performance_testing.md b/guides/source/performance_testing.md index 182f7eb0fd..ee0059623c 100644 --- a/guides/source/performance_testing.md +++ b/guides/source/performance_testing.md @@ -415,7 +415,7 @@ tests will set the following configuration parameters: ```bash ActionController::Base.perform_caching = true ActiveSupport::Dependencies.mechanism = :require -Rails.logger.level = ActiveSupport::BufferedLogger::INFO +Rails.logger.level = ActiveSupport::Logger::INFO ``` As `ActionController::Base.perform_caching` is set to `true`, performance tests diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 88e08a29a6..953133d8ef 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 4.0.0 (unreleased) ## +* Quote column names in generates fixture files. This prevents + conflicts with reserved YAML keywords such as 'yes' and 'no' + Fix #8612 + + *Yves Senn* + * Explicit options have precedence over `~/.railsrc` on the `rails new` command. *Rafael Mendonça França* diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore index 090799d5a0..52abb32479 100644 --- a/railties/lib/rails/generators/rails/app/templates/gitignore +++ b/railties/lib/rails/generators/rails/app/templates/gitignore @@ -4,14 +4,13 @@ # or operating system, you probably want to add a global ignore instead: # git config --global core.excludesfile '~/.gitignore_global' -# Ignore bundler related files +# Ignore bundler config /.bundle -/bin -# Ignore the default SQLite database +# Ignore the default SQLite database. /db/*.sqlite3 /db/*.sqlite3-journal -# Ignore all logfiles and tempfiles +# Ignore all logfiles and tempfiles. /log/*.log /tmp diff --git a/railties/lib/rails/generators/test_unit/model/model_generator.rb b/railties/lib/rails/generators/test_unit/model/model_generator.rb index 2801749ffe..2826a3ffa1 100644 --- a/railties/lib/rails/generators/test_unit/model/model_generator.rb +++ b/railties/lib/rails/generators/test_unit/model/model_generator.rb @@ -3,6 +3,9 @@ require 'rails/generators/test_unit' module TestUnit # :nodoc: module Generators # :nodoc: class ModelGenerator < Base # :nodoc: + + RESERVED_YAML_KEYWORDS = %w(y yes n no true false on off null) + argument :attributes, type: :array, default: [], banner: "field:type field:type" class_option :fixture, type: :boolean @@ -19,6 +22,15 @@ module TestUnit # :nodoc: template 'fixtures.yml', File.join('test/fixtures', class_path, "#{plural_file_name}.yml") end end + + private + def yaml_key_value(key, value) + if RESERVED_YAML_KEYWORDS.include?(key.downcase) + "'#{key}': #{value}" + else + "#{key}: #{value}" + end + end end end end diff --git a/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml b/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml index 7625ff975c..c9d505c84a 100644 --- a/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml +++ b/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml @@ -3,17 +3,17 @@ <% unless attributes.empty? -%> one: <% attributes.each do |attribute| -%> - <%= attribute.column_name %>: <%= attribute.default %> + <%= yaml_key_value(attribute.column_name, attribute.default) %> <%- if attribute.polymorphic? -%> - <%= "#{attribute.name}_type: #{attribute.human_name}" %> + <%= yaml_key_value("#{attribute.name}_type", attribute.human_name) %> <%- end -%> <% end -%> two: <% attributes.each do |attribute| -%> - <%= attribute.column_name %>: <%= attribute.default %> + <%= yaml_key_value(attribute.column_name, attribute.default) %> <%- if attribute.polymorphic? -%> - <%= "#{attribute.name}_type: #{attribute.human_name}" %> + <%= yaml_key_value("#{attribute.name}_type", attribute.human_name) %> <%- end -%> <% end -%> <% else -%> diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index 70e080a8ab..01ab77ee20 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -264,23 +264,40 @@ class ModelGeneratorTest < Rails::Generators::TestCase error = capture(:stderr) { run_generator ["Account", "--force"] } assert_no_match(/Another migration is already named create_accounts/, error) assert_no_file old_migration - assert_migration 'db/migrate/create_accounts.rb' + assert_migration "db/migrate/create_accounts.rb" end def test_invokes_default_test_framework run_generator assert_file "test/models/account_test.rb", /class AccountTest < ActiveSupport::TestCase/ + assert_file "test/fixtures/accounts.yml", /name: MyString/, /age: 1/ + assert_generated_fixture("test/fixtures/accounts.yml", + {"one"=>{"name"=>"MyString", "age"=>1}, "two"=>{"name"=>"MyString", "age"=>1}}) end def test_fixtures_use_the_references_ids run_generator ["LineItem", "product:references", "cart:belongs_to"] + assert_file "test/fixtures/line_items.yml", /product_id: \n cart_id: / + assert_generated_fixture("test/fixtures/line_items.yml", + {"one"=>{"product_id"=>nil, "cart_id"=>nil}, "two"=>{"product_id"=>nil, "cart_id"=>nil}}) end def test_fixtures_use_the_references_ids_and_type run_generator ["LineItem", "product:references{polymorphic}", "cart:belongs_to"] + assert_file "test/fixtures/line_items.yml", /product_id: \n product_type: Product\n cart_id: / + assert_generated_fixture("test/fixtures/line_items.yml", + {"one"=>{"product_id"=>nil, "product_type"=>"Product", "cart_id"=>nil}, + "two"=>{"product_id"=>nil, "product_type"=>"Product", "cart_id"=>nil}}) + end + + def test_fixtures_respect_reserved_yml_keywords + run_generator ["LineItem", "no:integer", "Off:boolean", "ON:boolean"] + + assert_generated_fixture("test/fixtures/line_items.yml", + {"one"=>{"no"=>1, "Off"=>false, "ON"=>false}, "two"=>{"no"=>1, "Off"=>false, "ON"=>false}}) end def test_fixture_is_skipped @@ -338,4 +355,10 @@ class ModelGeneratorTest < Rails::Generators::TestCase end end end + + private + def assert_generated_fixture(path, parsed_contents) + fixture_file = File.new File.expand_path(path, destination_root) + assert_equal(parsed_contents, YAML.load(fixture_file)) + end end |