diff options
57 files changed, 633 insertions, 160 deletions
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 48f7b0e214..214d63740c 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -11,6 +11,9 @@ If there's anything else that's important and relevant to your pull request, mention that information here. This could include benchmarks, or other information. +If you are updating any of the CHANGELOG files or are asked to update the +CHANGELOG files by reviewers, please add the CHANGELOG entry at the top of the file. + Finally, if your pull request affects documentation or any non-code changes, guidelines for those changes are [available here](http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#contributing-to-the-rails-documentation) diff --git a/.rubocop.yml b/.rubocop.yml index dd8db6af3a..62fb263c9e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,24 +4,40 @@ AllCops: # Two spaces, no tabs (for indentation). Style/IndentationWidth: - enabled: true + Enabled: true # No trailing whitespace. Style/TrailingWhitespace: - enabled: true + Enabled: true # Blank lines should not have any spaces. Style/TrailingBlankLines: - enabled: true + Enabled: true # Use Ruby >= 1.9 syntax for hashes. Prefer { a: :b } over { :a => :b }. Style/HashSyntax: - enabled: true + Enabled: true # Prefer &&/|| over and/or. Style/AndOr: - enabled: true + Enabled: true # Use my_method(my_arg) not my_method( my_arg ) or my_method my_arg. Lint/RequireParentheses: - enabled: true + Enabled: true + +Style/BracesAroundHashParameters: + Enabled: true + +Style/IndentationConsistency: + Enabled: true + EnforcedStyle: rails + +Style/EmptyLinesAroundClassBody: + Enabled: true + +Style/EmptyLinesAroundModuleBody: + Enabled: true + +Lint/EndAlignment: + AlignWith: variable diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1bb37d661a..d50cbaee38 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,13 @@ +* Check `request.path_parameters` encoding at the point they're set. + + Check for any non-UTF8 characters in path parameters at the point they're + set in `env`. Previously they were checked for when used to get a controller + class, but this meant routes that went directly to a Rack app, or skipped + controller instantiation for some other reason, had to defend against + non-UTF8 characters themselves. + + *Grey Baker* + * Don't raise ActionController::UnknownHttpMethod from ActionDispatch::Static Pass `Rack::Request` objects to `ActionDispatch::FileHandler` to avoid it diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index e826551f4b..01f1666b9b 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/duplicable' + module ActionDispatch module Http class ParameterFilter diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index ff5031d7d5..3f0e51790c 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -44,7 +44,14 @@ module ActionDispatch def path_parameters=(parameters) #:nodoc: delete_header('action_dispatch.request.parameters') + + # If any of the path parameters has an invalid encoding then + # raise since it's likely to trigger errors further on. + Request::Utils.check_param_encoding(parameters) + set_header PARAMETERS_KEY, parameters + rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e + raise ActionController::BadRequest.new("Invalid path parameters: #{e.message}") end # Returns a hash with the \parameters used to form the \path of the request. diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index b0ed681623..954dd4f354 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -66,24 +66,12 @@ module ActionDispatch def commit_cookie_jar! # :nodoc: end - def check_path_parameters! - # If any of the path parameters has an invalid encoding then - # raise since it's likely to trigger errors further on. - path_parameters.each do |key, value| - next unless value.respond_to?(:valid_encoding?) - unless value.valid_encoding? - raise ActionController::BadRequest, "Invalid parameter encoding: #{key} => #{value.inspect}" - end - end - end - PASS_NOT_FOUND = Class.new { # :nodoc: def self.action(_); self; end def self.call(_); [404, {'X-Cascade' => 'pass'}, []]; end } def controller_class - check_path_parameters! params = path_parameters if params.key?(:controller) diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index d6987f4d09..3265caa00b 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -22,7 +22,6 @@ module ActionDispatch end def serve(req) - req.check_path_parameters! uri = URI.parse(path(req.path_parameters, req)) unless uri.host diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 1e1d6f5429..c8a45a0851 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -33,7 +33,6 @@ require 'action_view/testing/resolvers' require 'action_dispatch' require 'active_support/dependencies' require 'active_model' -require 'active_record' require 'pp' # require 'pp' early to prevent hidden_methods from not picking up the pretty-print methods until too late diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index d3f2ec6aa1..37a54e7878 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -509,15 +509,6 @@ end class RequestForgeryProtectionControllerUsingResetSessionTest < ActionController::TestCase include RequestForgeryProtectionTests - setup do - @old_request_forgery_protection_token = ActionController::Base.request_forgery_protection_token - ActionController::Base.request_forgery_protection_token = :custom_authenticity_token - end - - teardown do - ActionController::Base.request_forgery_protection_token = @old_request_forgery_protection_token - end - test 'should emit a csrf-param meta tag and a csrf-token meta tag' do @controller.stub :form_authenticity_token, @token + '<=?' do get :meta @@ -677,6 +668,15 @@ class CustomAuthenticityParamControllerTest < ActionController::TestCase end class PerFormTokensControllerTest < ActionController::TestCase + def setup + @old_request_forgery_protection_token = ActionController::Base.request_forgery_protection_token + ActionController::Base.request_forgery_protection_token = :custom_authenticity_token + end + + def teardown + ActionController::Base.request_forgery_protection_token = @old_request_forgery_protection_token + end + def test_per_form_token_is_same_size_as_global_token get :index expected = ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 672b272590..e1d19c3520 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -44,14 +44,14 @@ class MimeTypeTest < ActiveSupport::TestCase accept = "text/*" expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml], Mime[:json]] parsed = Mime::Type.parse(accept) - assert_equal expect, parsed + assert_equal expect.map(&:to_s).sort!, parsed.map(&:to_s).sort! end test "parse application with trailing star" do accept = "application/*" expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip], Mime[:gzip]] parsed = Mime::Type.parse(accept) - assert_equal expect, parsed + assert_equal expect.map(&:to_s).sort!, parsed.map(&:to_s).sort! end test "parse without q" do diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 8a5d85ab84..634f6d80c4 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -1018,17 +1018,13 @@ class RequestParameters < BaseRequestTest end test "path parameters with invalid UTF8 encoding" do - request = stub_request( - "action_dispatch.request.path_parameters" => { foo: "\xBE" } - ) + request = stub_request err = assert_raises(ActionController::BadRequest) do - request.check_path_parameters! + request.path_parameters = { foo: "\xBE" } end - assert_match "Invalid parameter encoding", err.message - assert_match "foo", err.message - assert_match "\\xBE", err.message + assert_equal "Invalid path parameters: Non UTF-8 value: \xBE", err.message end test "parameters not accessible after rack parse error of invalid UTF8 character" do diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index d54cdf7247..5298e63fef 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4331,15 +4331,16 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest test "invalid UTF-8 encoding returns a 400 Bad Request" do with_routing do |set| - ActiveSupport::Deprecation.silence do - set.draw do - get "/bar/:id", :to => redirect("/foo/show/%{id}") - get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show" + set.draw do + get "/bar/:id", :to => redirect("/foo/show/%{id}") + get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show" - ActiveSupport::Deprecation.silence do - get "/foo(/:action(/:id))", :controller => "test_invalid_urls/foo" - get "/:controller(/:action(/:id))" - end + ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] } + get '/foobar/:id', to: ok + + ActiveSupport::Deprecation.silence do + get "/foo(/:action(/:id))", :controller => "test_invalid_urls/foo" + get "/:controller(/:action(/:id))" end end @@ -4354,6 +4355,9 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest get "/bar/%E2%EF%BF%BD%A6" assert_response :bad_request + + get "/foobar/%E2%EF%BF%BD%A6" + assert_response :bad_request end end end @@ -4774,7 +4778,9 @@ class TestPathParameters < ActionDispatch::IntegrationTest end end - get ':controller(/:action/(:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action/(:id))' + end end end diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 9c18ec56ca..cadef22022 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -41,8 +41,7 @@ module ActionView options = {} options[:formats] = [finder.rendered_format] if finder.rendered_format - if finder.disable_cache { finder.exists?(logical_name, [], partial, [], options) } - template = finder.disable_cache { finder.find(logical_name, [], partial, [], options) } + if template = finder.disable_cache { finder.find_all(logical_name, [], partial, [], options).first } finder.rendered_format ||= template.formats.first if node = seen[template.identifier] # handle cycles in the tree diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 06b696f281..0cd3207b12 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -651,12 +651,12 @@ module ActionView # The HTML specification says when nothing is select on a collection of radio buttons # web browsers do not send any value to server. # Unfortunately this introduces a gotcha: - # if a +User+ model has a +category_id+ field, and in the form none category is selected no +category_id+ parameter is sent. So, - # any strong parameters idiom like + # if a +User+ model has a +category_id+ field and in the form no category is selected, no +category_id+ parameter is sent. So, + # any strong parameters idiom like: # # params.require(:user).permit(...) # - # will raise an error since no +{user: ...}+ will be present. + # will raise an error since no <tt>{user: ...}</tt> will be present. # # To prevent this the helper generates an auxiliary hidden field before # every collection of radio buttons. The hidden field has the same name as collection radio button and blank value. diff --git a/actionview/lib/action_view/layouts.rb b/actionview/lib/action_view/layouts.rb index a74a5e05f3..8db1674187 100644 --- a/actionview/lib/action_view/layouts.rb +++ b/actionview/lib/action_view/layouts.rb @@ -248,11 +248,14 @@ module ActionView # # If the specified layout is a: # String:: the String is the template name - # Symbol:: call the method specified by the symbol, which will return the template name + # Symbol:: call the method specified by the symbol + # Proc:: call the passed Proc # false:: There is no layout # true:: raise an ArgumentError # nil:: Force default layout behavior with inheritance # + # Return value of Proc & Symbol arguments should be String, false, true or nil + # with the same meaning as described above. # ==== Parameters # * <tt>layout</tt> - The layout to use. # diff --git a/actionview/test/activerecord/controller_runtime_test.rb b/actionview/test/activerecord/controller_runtime_test.rb index af91348d76..a61181df88 100644 --- a/actionview/test/activerecord/controller_runtime_test.rb +++ b/actionview/test/activerecord/controller_runtime_test.rb @@ -38,8 +38,8 @@ class ControllerRuntimeLogSubscriberTest < ActionController::TestCase tests LogSubscriberController def setup - super @old_logger = ActionController::Base.logger + super ActionController::LogSubscriber.attach_to :action_controller end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 86d07580e8..590fe2f587 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,20 @@ +* Support calling the method `merge` in `scope`'s lambda. + + *Yasuhiro Sugino* + +* Fixes multi-parameter attributes conversion with invalid params. + + *Hiroyuki Ishii* + +* Add newline between each migration in `structure.sql`. + + Keeps schema migration inserts as a single commit, but allows for easier + git diffing. + + Fixes #25504. + + *Grey Baker*, *Norberto Lopes* + * The flag `error_on_ignored_order_or_limit` has been deprecated in favor of the current `error_on_ignored_order`. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3729e22e64..c91b5d3fe3 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -362,7 +362,7 @@ module ActiveRecord # end # end # - # If your model class is <tt>Project</tt>, the module is + # If your model class is <tt>Project</tt>, then the module is # named <tt>Project::GeneratedAssociationMethods</tt>. The +GeneratedAssociationMethods+ module is # included in the model class immediately after the (anonymous) generated attributes methods # module, meaning an association will override the methods for an attribute with the same name. @@ -845,8 +845,8 @@ module ActiveRecord # Post.includes(:author).each do |post| # # This references the name of the #belongs_to association that also used the <tt>:author</tt> - # symbol. After loading the posts, find will collect the +author_id+ from each one and load - # all the referenced authors with one query. Doing so will cut down the number of queries + # symbol. After loading the posts, +find+ will collect the +author_id+ from each one and load + # all of the referenced authors with one query. Doing so will cut down the number of queries # from 201 to 102. # # We can improve upon the situation further by referencing both associations in the finder with: @@ -873,7 +873,7 @@ module ActiveRecord # # Since only one table is loaded at a time, conditions or orders cannot reference tables # other than the main one. If this is the case, Active Record falls back to the previously - # used LEFT OUTER JOIN based strategy. For example: + # used <tt>LEFT OUTER JOIN</tt> based strategy. For example: # # Post.includes([:author, :comments]).where(['comments.approved = ?', true]) # @@ -881,18 +881,18 @@ module ActiveRecord # <tt>LEFT OUTER JOIN comments ON comments.post_id = posts.id</tt> and # <tt>LEFT OUTER JOIN authors ON authors.id = posts.author_id</tt>. Note that using conditions # like this can have unintended consequences. - # In the above example posts with no approved comments are not returned at all, because + # In the above example, posts with no approved comments are not returned at all because # the conditions apply to the SQL statement as a whole and not just to the association. # # You must disambiguate column references for this fallback to happen, for example # <tt>order: "author.name DESC"</tt> will work but <tt>order: "name DESC"</tt> will not. # - # If you want to load all posts (including posts with no approved comments) then write - # your own LEFT OUTER JOIN query using ON + # If you want to load all posts (including posts with no approved comments), then write + # your own <tt>LEFT OUTER JOIN</tt> query using <tt>ON</tt>: # # Post.joins("LEFT OUTER JOIN comments ON comments.post_id = posts.id AND comments.approved = '1'") # - # In this case it is usually more natural to include an association which has conditions defined on it: + # In this case, it is usually more natural to include an association which has conditions defined on it: # # class Post < ActiveRecord::Base # has_many :approved_comments, -> { where(approved: true) }, class_name: 'Comment' @@ -924,7 +924,7 @@ module ActiveRecord # # This will execute one query to load the addresses and load the addressables with one # query per addressable type. - # For example if all the addressables are either of class Person or Company then a total + # For example, if all the addressables are either of class Person or Company, then a total # of 3 queries will be executed. The list of addressable types to load is determined on # the back of the addresses loaded. This is not supported if Active Record has to fallback # to the previous implementation of eager loading and will raise ActiveRecord::EagerLoadPolymorphicError. @@ -1015,7 +1015,7 @@ module ActiveRecord # # == Bi-directional associations # - # When you specify an association there is usually an association on the associated model + # When you specify an association, there is usually an association on the associated model # that specifies the same relationship in reverse. For example, with the following models: # # class Dungeon < ActiveRecord::Base @@ -1032,7 +1032,7 @@ module ActiveRecord # end # # The +traps+ association on +Dungeon+ and the +dungeon+ association on +Trap+ are - # the inverse of each other and the inverse of the +dungeon+ association on +EvilWizard+ + # the inverse of each other, and the inverse of the +dungeon+ association on +EvilWizard+ # is the +evil_wizard+ association on +Dungeon+ (and vice-versa). By default, # Active Record can guess the inverse of the association based on the name # of the class. The result is the following: @@ -1062,7 +1062,7 @@ module ActiveRecord # # * does not work with <tt>:through</tt> associations. # * does not work with <tt>:polymorphic</tt> associations. - # * for #belongs_to associations #has_many inverse associations are ignored. + # * inverse associations for #belongs_to associations #has_many are ignored. # # For more information, see the documentation for the +:inverse_of+ option. # @@ -1070,7 +1070,7 @@ module ActiveRecord # # === Dependent associations # - # #has_many, #has_one and #belongs_to associations support the <tt>:dependent</tt> option. + # #has_many, #has_one, and #belongs_to associations support the <tt>:dependent</tt> option. # This allows you to specify that associated records should be deleted when the owner is # deleted. # diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 5d1e7ffb73..17ccf5a86c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -793,7 +793,7 @@ module ActiveRecord # Returns +true+ if the collection is empty. If the collection has been # loaded it is equivalent # to <tt>collection.size.zero?</tt>. If the collection has not been loaded, - # it is equivalent to <tt>collection.exists?</tt>. If the collection has + # it is equivalent to <tt>!collection.exists?</tt>. If the collection has # not already been loaded and you are going to fetch the records anyway it # is better to check <tt>collection.length.zero?</tt>. # diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index e160460286..cbb4f0cff8 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -39,7 +39,7 @@ module ActiveRecord end def set_time_zone_without_conversion(value) - ::Time.zone.local_to_utc(value).in_time_zone + ::Time.zone.local_to_utc(value).in_time_zone if value end def map_avoiding_infinite_recursion(value) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 9b74c3a10f..dc5b305843 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -849,6 +849,21 @@ module ActiveRecord remove_connection(spec.name) owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec) + + message_bus = ActiveSupport::Notifications.instrumenter + payload = { + connection_id: object_id + } + if spec + payload[:spec_name] = spec.name + payload[:config] = spec.config + end + + message_bus.instrument('!connection.active_record', payload) do + owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec) + end + + owner_to_pool[spec.name] end # Returns true if there are any active connections among the connection @@ -881,7 +896,7 @@ module ActiveRecord # for (not necessarily the current class). def retrieve_connection(spec_name) #:nodoc: pool = retrieve_connection_pool(spec_name) - raise ConnectionNotEstablished, "No connection pool with id '#{spec_name}' found." unless pool + raise ConnectionNotEstablished, "No connection pool with '#{spec_name}' found." unless pool conn = pool.connection raise ConnectionNotEstablished, "No connection for '#{spec_name}' in connection pool" unless conn conn diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 396cb0b07a..9e9ace49db 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -998,8 +998,8 @@ module ActiveRecord sm_table = ActiveRecord::Migrator.schema_migrations_table_name if supports_multi_insert? - sql = "INSERT INTO #{sm_table} (version) VALUES " - sql << versions.map {|v| "('#{v}')" }.join(', ') + sql = "INSERT INTO #{sm_table} (version) VALUES\n" + sql << versions.map {|v| "('#{v}')" }.join(",\n") sql << ";\n\n" sql else diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 5747e4d1ee..4f8490fa2b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -579,14 +579,15 @@ module ActiveRecord exception end - def log(sql, name = "SQL", binds = [], statement_name = nil) + def log(sql, name = "SQL", binds = [], type_casted_binds = [], statement_name = nil) @instrumenter.instrument( "sql.active_record", - :sql => sql, - :name => name, - :connection_id => object_id, - :statement_name => statement_name, - :binds => binds) { yield } + sql: sql, + name: name, + binds: binds, + type_casted_binds: type_casted_binds, + statement_name: statement_name, + connection_id: object_id) { yield } rescue => e raise translate_exception_class(e, sql) end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb index 87f0ff7d85..cc19d95669 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb @@ -79,7 +79,7 @@ module ActiveRecord type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, binds) do + log(sql, name, binds, type_casted_binds) do if cache_stmt cache = @statements[sql] ||= { stmt: @connection.prepare(sql) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 45507e206a..cc606e4828 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -327,12 +327,12 @@ module ActiveRecord end # Returns the sequence name for a table's primary key or some other specified key. - def default_sequence_name(table_name, pk = nil) #:nodoc: - result = serial_sequence(table_name, pk || 'id') + def default_sequence_name(table_name, pk = 'id') #:nodoc: + result = serial_sequence(table_name, pk) return nil unless result Utils.extract_schema_qualified_name(result).to_s rescue ActiveRecord::StatementInvalid - PostgreSQL::Name.new(nil, "#{table_name}_#{pk || 'id'}_seq").to_s + PostgreSQL::Name.new(nil, "#{table_name}_#{pk}_seq").to_s end def serial_sequence(table, column) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index ddfc560747..487165d511 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -598,14 +598,14 @@ module ActiveRecord def exec_no_cache(sql, name, binds) type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, binds) { @connection.async_exec(sql, type_casted_binds) } + log(sql, name, binds, type_casted_binds) { @connection.async_exec(sql, type_casted_binds) } end def exec_cache(sql, name, binds) stmt_key = prepare_statement(sql) type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, binds, stmt_key) do + log(sql, name, binds, type_casted_binds, stmt_key) do @connection.exec_prepared(stmt_key, type_casted_binds) end rescue ActiveRecord::StatementInvalid => e diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 3384012c49..7e23f44ddf 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -190,7 +190,7 @@ module ActiveRecord def exec_query(sql, name = nil, binds = [], prepare: false) type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, binds) do + log(sql, name, binds, type_casted_binds) do # Don't cache statements if they are not prepared unless prepare stmt = @connection.prepare(sql) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 2c94463acd..3e3a7679ac 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -78,7 +78,18 @@ module ActiveRecord mattr_accessor :error_on_ignored_order, instance_writer: false self.error_on_ignored_order = false - mattr_accessor :error_on_ignored_order_or_limit, instance_writer: false + def self.error_on_ignored_order_or_limit + ActiveSupport::Deprecation.warn(<<-MSG.squish) + The flag error_on_ignored_order_or_limit is deprecated. Limits are + now supported. Please use error_on_ignored_order instead. + MSG + self.error_on_ignored_order + end + + def error_on_ignored_order_or_limit + self.class.error_on_ignored_order_or_limit + end + def self.error_on_ignored_order_or_limit=(value) ActiveSupport::Deprecation.warn(<<-MSG.squish) The flag error_on_ignored_order_or_limit is deprecated. Limits are diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 51bf12d0bf..0ec33f7d87 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -968,6 +968,7 @@ module ActiveRecord @fixture_cache = {} @fixture_connections = [] @@already_loaded_fixtures ||= {} + @connection_subscriber = nil # Load fixtures once and begin transaction. if run_in_transaction? @@ -977,10 +978,31 @@ module ActiveRecord @loaded_fixtures = load_fixtures(config) @@already_loaded_fixtures[self.class] = @loaded_fixtures end + + # Begin transactions for connections already established @fixture_connections = enlist_fixture_connections @fixture_connections.each do |connection| connection.begin_transaction joinable: false end + + # When connections are established in the future, begin a transaction too + @connection_subscriber = ActiveSupport::Notifications.subscribe('!connection.active_record') do |_, _, _, _, payload| + spec_name = payload[:spec_name] if payload.key?(:spec_name) + + if spec_name + begin + connection = ActiveRecord::Base.connection_handler.retrieve_connection(spec_name) + rescue ConnectionNotEstablished + connection = nil + end + + if connection && !@fixture_connections.include?(connection) + connection.begin_transaction joinable: false + @fixture_connections << connection + end + end + end + # Load fixtures for every test. else ActiveRecord::FixtureSet.reset_cache @@ -995,6 +1017,7 @@ module ActiveRecord def teardown_fixtures # Rollback changes if a transaction is active. if run_in_transaction? + ActiveSupport::Notifications.unsubscribe(@connection_subscriber) if @connection_subscriber @fixture_connections.each do |connection| connection.rollback_transaction if connection.transaction_open? end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 8e32af1c49..c4998ba686 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -20,18 +20,14 @@ module ActiveRecord @odd = false end - def render_bind(attribute) - value = if attribute.type.binary? && attribute.value - if attribute.value.is_a?(Hash) - "<#{attribute.value_for_database.to_s.bytesize} bytes of binary data>" - else - "<#{attribute.value.bytesize} bytes of binary data>" - end + def render_bind(attr, type_casted_value) + value = if attr.type.binary? && attr.value + "<#{attr.value_for_database.to_s.bytesize} bytes of binary data>" else - attribute.value_for_database + type_casted_value end - [attribute.name, value] + [attr.name, value] end def sql(event) @@ -48,7 +44,9 @@ module ActiveRecord binds = nil unless (payload[:binds] || []).empty? - binds = " " + payload[:binds].map { |attr| render_bind(attr) }.inspect + binds = " " + payload[:binds].zip(payload[:type_casted_binds]).map { |attr, value| + render_bind(attr, value) + }.inspect end name = colorize_payload_name(name, payload[:name]) diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 53ddd95bb0..f8dd35df0f 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -9,7 +9,7 @@ module ActiveRecord delegate :find_each, :find_in_batches, :in_batches, to: :all delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or, :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, - :having, :create_with, :uniq, :distinct, :references, :none, :unscope, to: :all + :having, :create_with, :uniq, :distinct, :references, :none, :unscope, :merge, to: :all delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all delegate :pluck, :ids, to: :all diff --git a/activerecord/lib/active_record/railties/controller_runtime.rb b/activerecord/lib/active_record/railties/controller_runtime.rb index 8727e46cb3..c13238cbe2 100644 --- a/activerecord/lib/active_record/railties/controller_runtime.rb +++ b/activerecord/lib/active_record/railties/controller_runtime.rb @@ -19,7 +19,7 @@ module ActiveRecord end def cleanup_view_runtime - if logger.info? && ActiveRecord::Base.connected? + if logger && logger.info? && ActiveRecord::Base.connected? db_rt_before_render = ActiveRecord::LogSubscriber.reset_runtime self.db_runtime = (db_runtime || 0) + db_rt_before_render runtime = super diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 7ec0dfce7a..61a5c2c3b7 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -244,8 +244,9 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_do_not_call_callbacks_for_delete_all car = Car.create(:name => 'honda') car.funky_bulbs.create! + assert_equal 1, car.funky_bulbs.count assert_nothing_raised { car.reload.funky_bulbs.delete_all } - assert_equal 0, Bulb.count, "bulbs should have been deleted using :delete_all strategy" + assert_equal 0, car.funky_bulbs.count, "bulbs should have been deleted using :delete_all strategy" end def test_delete_all_on_association_is_the_same_as_not_loaded diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 2c6011489e..8a4c1bd615 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -587,7 +587,7 @@ class EachTest < ActiveRecord::TestCase end end - test 'error_on_ignored_order_or_limit= is deprecated' do + test '.error_on_ignored_order_or_limit= is deprecated' do begin prev = ActiveRecord::Base.error_on_ignored_order assert_deprecated 'Please use error_on_ignored_order= instead.' do @@ -598,4 +598,20 @@ class EachTest < ActiveRecord::TestCase ActiveRecord::Base.error_on_ignored_order = prev end end + + test '.error_on_ignored_order_or_limit is deprecated' do + expected = ActiveRecord::Base.error_on_ignored_order + actual = assert_deprecated 'Please use error_on_ignored_order instead.' do + ActiveRecord::Base.error_on_ignored_order_or_limit + end + assert_equal expected, actual + end + + test '#error_on_ignored_order_or_limit is deprecated' do + expected = ActiveRecord::Base.error_on_ignored_order + actual = assert_deprecated 'Please use error_on_ignored_order instead.' do + Post.new.error_on_ignored_order_or_limit + end + assert_equal expected, actual + end end diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index fa924fe4cb..3f01885489 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -57,10 +57,13 @@ module ActiveRecord end def test_logs_bind_vars_after_type_cast + binds = [Relation::QueryAttribute.new("id", "10", Type::Integer.new)] + type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } payload = { :name => 'SQL', :sql => 'select * from topics where id = ?', - :binds => [Relation::QueryAttribute.new("id", "10", Type::Integer.new)] + :binds => binds, + :type_casted_binds => type_casted_binds } event = ActiveSupport::Notifications::Event.new( 'foo', @@ -84,6 +87,12 @@ module ActiveRecord logger.sql event assert_match([[@pk.name, 10]].inspect, logger.debugs.first) end + + private + + def type_cast(value) + ActiveRecord::Base.connection.type_cast(value) + end end end end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 09e7848bda..bbcb42d58e 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -341,6 +341,18 @@ module ActiveRecord end end + def test_connection_notification_is_called + payloads = [] + subscription = ActiveSupport::Notifications.subscribe('!connection.active_record') do |name, started, finished, unique_id, payload| + payloads << payload + end + ActiveRecord::Base.establish_connection :arunit + assert_equal [:config, :connection_id, :spec_name], payloads[0].keys.sort + assert_equal 'primary', payloads[0][:spec_name] + ensure + ActiveSupport::Notifications.unsubscribe(subscription) if subscription + end + def test_pool_sets_connection_schema_cache connection = pool.checkout schema_cache = SchemaCache.new connection diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 9455d4886c..df53bbf950 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -622,6 +622,46 @@ class TransactionalFixturesOnCustomConnectionTest < ActiveRecord::TestCase end end +class TransactionalFixturesOnConnectionNotification < ActiveRecord::TestCase + self.use_transactional_tests = true + self.use_instantiated_fixtures = false + + def test_transaction_created_on_connection_notification + connection = stub(:transaction_open? => false) + connection.expects(:begin_transaction).with(joinable: false) + fire_connection_notification(connection) + end + + def test_notification_established_transactions_are_rolled_back + # Mocha is not thread-safe so define our own stub to test + connection = Class.new do + attr_accessor :rollback_transaction_called + def transaction_open?; true; end + def begin_transaction(*args); end + def rollback_transaction(*args) + @rollback_transaction_called = true + end + end.new + fire_connection_notification(connection) + teardown_fixtures + assert(connection.rollback_transaction_called, "Expected <mock connection>#rollback_transaction to be called but was not") + end + + private + + def fire_connection_notification(connection) + ActiveRecord::Base.connection_handler.stubs(:retrieve_connection).with('book').returns(connection) + message_bus = ActiveSupport::Notifications.instrumenter + payload = { + spec_name: 'book', + config: nil, + connection_id: connection.object_id + } + + message_bus.instrument('!connection.active_record', payload) {} + end +end + class InvalidTableNameFixturesTest < ActiveRecord::TestCase fixtures :funny_jokes # Set to false to blow away fixtures cache and ensure our fixtures are loaded diff --git a/activerecord/test/cases/migration/references_statements_test.rb b/activerecord/test/cases/migration/references_statements_test.rb index 70c64f3e71..5dddb35c4c 100644 --- a/activerecord/test/cases/migration/references_statements_test.rb +++ b/activerecord/test/cases/migration/references_statements_test.rb @@ -35,7 +35,7 @@ module ActiveRecord assert_not index_exists?(table_name, :user_id) end - def test_create_reference_id_index_even_if_index_option_is_passed + def test_create_reference_id_index_even_if_index_option_is_not_passed add_reference table_name, :user assert index_exists?(table_name, :user_id) end diff --git a/activerecord/test/cases/multiparameter_attributes_test.rb b/activerecord/test/cases/multiparameter_attributes_test.rb index d05cb22740..59c340ceb7 100644 --- a/activerecord/test/cases/multiparameter_attributes_test.rb +++ b/activerecord/test/cases/multiparameter_attributes_test.rb @@ -202,6 +202,20 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase Topic.reset_column_information end + def test_multiparameter_attributes_on_time_with_time_zone_aware_attributes_and_invalid_time_params + with_timezone_config aware_attributes: true do + Topic.reset_column_information + attributes = { + "written_on(1i)" => "2004", "written_on(2i)" => "", "written_on(3i)" => "" + } + topic = Topic.find(1) + topic.attributes = attributes + assert_nil topic.written_on + end + ensure + Topic.reset_column_information + end + def test_multiparameter_attributes_on_time_with_time_zone_aware_attributes_false with_timezone_config default: :local, aware_attributes: false, zone: -28800 do attributes = { diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 03ec063671..085636553e 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -6,6 +6,8 @@ require 'models/post' require 'rack' class QueryCacheTest < ActiveRecord::TestCase + self.use_transactional_tests = false + fixtures :tasks, :topics, :categories, :posts, :categories_posts teardown do diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index ffb2da7a26..36cb898010 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -44,8 +44,8 @@ module ActiveRecord end test "#_select!" do - assert relation.public_send("_select!", :foo).equal?(relation) - assert_equal [:foo], relation.public_send("select_values") + assert relation._select!(:foo).equal?(relation) + assert_equal [:foo], relation.select_values end test '#order!' do diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index 0e277ed235..f0dac07acc 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -46,6 +46,13 @@ class NamedScopingTest < ActiveRecord::TestCase assert_equal Topic.average(:replies_count), Topic.base.average(:replies_count) end + def test_calling_merge_at_first_in_scope + Topic.class_eval do + scope :calling_merge_at_first_in_scope, Proc.new { merge(Topic.replied) } + end + assert_equal Topic.calling_merge_at_first_in_scope.to_a, Topic.replied.to_a + end + def test_method_missing_priority_when_delegating klazz = Class.new(ActiveRecord::Base) do self.table_name = "topics" diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index a8d875640e..3749dda9fc 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,42 @@ +* Introduce `assert_changes` and `assert_no_changes`. + + `assert_changes` is a more general `assert_difference` that works with any + value. + + assert_changes 'Error.current', from: nil, to: 'ERR' do + expected_bad_operation + end + + Can be called with strings, to be evaluated in the binding (context) of + the block given to the assertion, or a lambda. + + assert_changes -> { Error.current }, from: nil, to: 'ERR' do + expected_bad_operation + end + + The `from` and `to` arguments are compared with the case operator (`===`). + + assert_changes 'Error.current', from: nil, to: Error do + expected_bad_operation + end + + This is pretty useful, if you need to loosely compare a value. For example, + you need to test a token has been generated and it has that many random + characters. + + user = User.start_registration + assert_changes 'user.token', to: /\w{32}/ do + user.finish_registration + end + + *Genadi Samokovarov* + +* Add `:fallback_string` option to `Array#to_sentence`. If an empty array + calls the function and a fallback string option is set then it returns the + fallback string other than an empty string. + + *Mohamed Osama* + * Fix `ActiveSupport::TimeZone#strptime`. Now raises `ArgumentError` when the given time doesn't match the format. The error is the same as the one given by Ruby's `Date.strptime`. Previously it raised @@ -9,14 +48,14 @@ * `travel/travel_to` travel time helpers, now raise on nested calls, as this can lead to confusing time stubbing. - + Instead of: - + travel_to 2.days.from_now do # 2 days from today travel_to 3.days.from_now do # 5 days from today - end + end end preferred way to achieve above is: @@ -24,13 +63,12 @@ travel 2.days do # 2 days from today end - - travel 5.days do - # 5 days from today - end - + + travel 5.days do + # 5 days from today + end + *Vipul A M* - * Support parsing JSON time in ISO8601 local time strings in `ActiveSupport::JSON.decode` when `parse_json_times` is enabled. diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb index 8718b7e1e5..54fb83581a 100644 --- a/activesupport/lib/active_support/core_ext/array/conversions.rb +++ b/activesupport/lib/active_support/core_ext/array/conversions.rb @@ -40,6 +40,12 @@ class Array # ['one', 'two', 'three'].to_sentence(words_connector: ' or ', last_word_connector: ' or at least ') # # => "one or two or at least three" # + # [].to_sentence(fallback_string: 'none') + # # => "none" + # + # ['one', 'two'].to_sentence(fallback_string: 'none') + # # => "one and two" + # # Using <tt>:locale</tt> option: # # # Given this locale dictionary: @@ -57,7 +63,7 @@ class Array # ['uno', 'dos', 'tres'].to_sentence(locale: :es) # # => "uno o dos o al menos tres" def to_sentence(options = {}) - options.assert_valid_keys(:words_connector, :two_words_connector, :last_word_connector, :locale) + options.assert_valid_keys(:words_connector, :two_words_connector, :last_word_connector, :locale, :fallback_string) default_connectors = { :words_connector => ', ', @@ -72,7 +78,7 @@ class Array case length when 0 - '' + "#{options[:fallback_string] || ''}" when 1 "#{self[0]}" when 2 diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index ad83638572..7770aa8006 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -1,6 +1,8 @@ module ActiveSupport module Testing module Assertions + UNTRACKED = Object.new # :nodoc: + # Asserts that an expression is not truthy. Passes if <tt>object</tt> is # +nil+ or +false+. "Truthy" means "considered true in a conditional" # like <tt>if foo</tt>. @@ -92,6 +94,93 @@ module ActiveSupport def assert_no_difference(expression, message = nil, &block) assert_difference expression, 0, message, &block end + + # Assertion that the result of evaluating an expression is changed before + # and after invoking the passed in block. + # + # assert_changes 'Status.all_good?' do + # post :create, params: { status: { ok: false } } + # end + # + # You can pass the block as a string to be evaluated in the context of + # the block. A lambda can be passed for the block as well. + # + # assert_changes -> { Status.all_good? } do + # post :create, params: { status: { ok: false } } + # end + # + # The assertion is useful to test side effects. The passed block can be + # anything that can be converted to string with #to_s. + # + # assert_changes :@object do + # @object = 42 + # end + # + # The keyword arguments :from and :to can be given to specify the + # expected initial value and the expected value after the block was + # executed. + # + # assert_changes :@object, from: nil, to: :foo do + # @object = :foo + # end + # + # An error message can be specified. + # + # assert_changes -> { Status.all_good? }, 'Expected the status to be bad' do + # post :create, params: { status: { incident: true } } + # end + def assert_changes(expression, message = nil, from: UNTRACKED, to: UNTRACKED, &block) + exp = expression.respond_to?(:call) ? expression : -> { eval(expression.to_s, block.binding) } + + before = exp.call + retval = yield + + unless from == UNTRACKED + error = "#{expression.inspect} isn't #{from}" + error = "#{message}.\n#{error}" if message + assert from === before, error + end + + after = exp.call + + if to == UNTRACKED + error = "#{expression.inspect} didn't changed" + error = "#{message}.\n#{error}" if message + assert_not_equal before, after, error + else + message = "#{expression.inspect} didn't change to #{to}" + error = "#{message}.\n#{error}" if message + assert to === after, error + end + + retval + end + + # Assertion that the result of evaluating an expression is changed before + # and after invoking the passed in block. + # + # assert_no_changes 'Status.all_good?' do + # post :create, params: { status: { ok: true } } + # end + # + # An error message can be specified. + # + # assert_no_changes -> { Status.all_good? }, 'Expected the status to be good' do + # post :create, params: { status: { ok: false } } + # end + def assert_no_changes(expression, message = nil, &block) + exp = expression.respond_to?(:call) ? expression : -> { eval(expression.to_s, block.binding) } + + before = exp.call + retval = yield + after = exp.call + + error = "#{expression.inspect} did change to #{after}" + error = "#{message}.\n#{error}" if message + assert_equal before, after, error + + retval + end end end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 60119ad95e..d4dec81b28 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -25,6 +25,18 @@ module ActiveSupport assert_nil LocalCacheRegistry.cache_for(key) end + def test_local_cache_cleared_and_response_should_be_present_on_invalid_parameters_error + key = "super awesome key" + assert_nil LocalCacheRegistry.cache_for key + middleware = Middleware.new('<3', key).new(->(env) { + assert LocalCacheRegistry.cache_for(key), 'should have a cache' + raise Rack::Utils::InvalidParameterError + }) + response = middleware.call({}) + assert response, 'response should exist' + assert_nil LocalCacheRegistry.cache_for(key) + end + def test_local_cache_cleared_on_exception key = "super awesome key" assert_nil LocalCacheRegistry.cache_for key diff --git a/activesupport/test/core_ext/array/conversions_test.rb b/activesupport/test/core_ext/array/conversions_test.rb index de36e2026d..323b451e02 100644 --- a/activesupport/test/core_ext/array/conversions_test.rb +++ b/activesupport/test/core_ext/array/conversions_test.rb @@ -25,6 +25,11 @@ class ToSentenceTest < ActiveSupport::TestCase assert_equal "one, two and three", ['one', 'two', 'three'].to_sentence(last_word_connector: ' and ') end + def test_to_sentence_with_fallback_string + assert_equal "none", [].to_sentence(fallback_string: 'none') + assert_equal "one, two, and three", ['one', 'two', 'three'].to_sentence(fallback_string: 'none') + end + def test_two_elements assert_equal "one and two", ['one', 'two'].to_sentence assert_equal "one two", ['one', 'two'].to_sentence(two_words_connector: ' ') @@ -58,7 +63,7 @@ class ToSentenceTest < ActiveSupport::TestCase ['one', 'two'].to_sentence(passing: 'invalid option') end - assert_equal exception.message, "Unknown key: :passing. Valid keys are: :words_connector, :two_words_connector, :last_word_connector, :locale" + assert_equal exception.message, "Unknown key: :passing. Valid keys are: :words_connector, :two_words_connector, :last_word_connector, :locale, :fallback_string" end def test_always_returns_string diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index 18228a2ac5..772c3cfca7 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -111,6 +111,112 @@ class AssertDifferenceTest < ActiveSupport::TestCase end end end + + def test_assert_changes_pass + assert_changes '@object.num' do + @object.increment + end + end + + def test_assert_changes_pass_with_lambda + assert_changes -> { @object.num } do + @object.increment + end + end + + def test_assert_changes_with_from_option + assert_changes '@object.num', from: 0 do + @object.increment + end + end + + def test_assert_changes_with_from_option_with_wrong_value + assert_raises Minitest::Assertion do + assert_changes '@object.num', from: -1 do + @object.increment + end + end + end + + def test_assert_changes_with_to_option + assert_changes '@object.num', to: 1 do + @object.increment + end + end + + def test_assert_changes_with_wrong_to_option + assert_raises Minitest::Assertion do + assert_changes '@object.num', to: 2 do + @object.increment + end + end + end + + def test_assert_changes_with_from_option_and_to_option + assert_changes '@object.num', from: 0, to: 1 do + @object.increment + end + end + + def test_assert_changes_with_from_and_to_options_and_wrong_to_value + assert_raises Minitest::Assertion do + assert_changes '@object.num', from: 0, to: 2 do + @object.increment + end + end + end + + def test_assert_changes_works_with_any_object + retval = silence_warnings do + assert_changes :@new_object, from: nil, to: 42 do + @new_object = 42 + end + end + + assert_equal 42, retval + end + + def test_assert_changes_works_with_nil + oldval = @object + + retval = assert_changes :@object, from: oldval, to: nil do + @object = nil + end + + assert_nil retval + end + + def test_assert_changes_with_to_and_case_operator + token = nil + + assert_changes 'token', to: /\w{32}/ do + token = SecureRandom.hex + end + end + + def test_assert_changes_with_to_and_from_and_case_operator + token = SecureRandom.hex + + assert_changes 'token', from: /\w{32}/, to: /\w{32}/ do + token = SecureRandom.hex + end + end + + def test_assert_no_changes_pass + assert_no_changes '@object.num' do + # ... + end + end + + def test_assert_no_changes_with_message + error = assert_raises Minitest::Assertion do + assert_no_changes '@object.num', '@object.num should not change' do + @object.increment + end + end + + assert_equal "@object.num should not change.\n\"@object.num\" did change to 1.\nExpected: 0\n Actual: 1", error.message + end end class AlsoDoingNothingTest < ActiveSupport::TestCase diff --git a/activesupport/test/time_travel_test.rb b/activesupport/test/time_travel_test.rb index e9f87bdf82..e75237e5a4 100644 --- a/activesupport/test/time_travel_test.rb +++ b/activesupport/test/time_travel_test.rb @@ -3,18 +3,18 @@ require 'active_support/core_ext/date_time' require 'active_support/core_ext/numeric/time' class TimeTravelTest < ActiveSupport::TestCase - teardown do - travel_back - end - def test_time_helper_travel Time.stub(:now, Time.now) do - expected_time = Time.now + 1.day - travel 1.day + begin + expected_time = Time.now + 1.day + travel 1.day - assert_equal expected_time.to_s(:db), Time.now.to_s(:db) - assert_equal expected_time.to_date, Date.today - assert_equal expected_time.to_datetime.to_s(:db), DateTime.now.to_s(:db) + assert_equal expected_time.to_s(:db), Time.now.to_s(:db) + assert_equal expected_time.to_date, Date.today + assert_equal expected_time.to_datetime.to_s(:db), DateTime.now.to_s(:db) + ensure + travel_back + end end end @@ -36,12 +36,16 @@ class TimeTravelTest < ActiveSupport::TestCase def test_time_helper_travel_to Time.stub(:now, Time.now) do - expected_time = Time.new(2004, 11, 24, 01, 04, 44) - travel_to expected_time + begin + expected_time = Time.new(2004, 11, 24, 01, 04, 44) + travel_to expected_time - assert_equal expected_time, Time.now - assert_equal Date.new(2004, 11, 24), Date.today - assert_equal expected_time.to_datetime, DateTime.now + assert_equal expected_time, Time.now + assert_equal Date.new(2004, 11, 24), Date.today + assert_equal expected_time.to_datetime, DateTime.now + ensure + travel_back + end end end @@ -63,17 +67,21 @@ class TimeTravelTest < ActiveSupport::TestCase def test_time_helper_travel_back Time.stub(:now, Time.now) do - expected_time = Time.new(2004, 11, 24, 01, 04, 44) + begin + expected_time = Time.new(2004, 11, 24, 01, 04, 44) - travel_to expected_time - assert_equal expected_time, Time.now - assert_equal Date.new(2004, 11, 24), Date.today - assert_equal expected_time.to_datetime, DateTime.now - travel_back + travel_to expected_time + assert_equal expected_time, Time.now + assert_equal Date.new(2004, 11, 24), Date.today + assert_equal expected_time.to_datetime, DateTime.now + travel_back - assert_not_equal expected_time, Time.now - assert_not_equal Date.new(2004, 11, 24), Date.today - assert_not_equal expected_time.to_datetime, DateTime.now + assert_not_equal expected_time, Time.now + assert_not_equal Date.new(2004, 11, 24), Date.today + assert_not_equal expected_time.to_datetime, DateTime.now + ensure + travel_back + end end end @@ -107,14 +115,18 @@ class TimeTravelTest < ActiveSupport::TestCase def test_time_helper_travel_to_with_subsequent_calls Time.stub(:now, Time.now) do - initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44) - subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44) - assert_nothing_raised do - travel_to initial_expected_time - travel_to subsequent_expected_time + begin + initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44) + subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44) + assert_nothing_raised do + travel_to initial_expected_time + travel_to subsequent_expected_time - assert_equal subsequent_expected_time, Time.now + assert_equal subsequent_expected_time, Time.now + travel_back + end + ensure travel_back end end diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index b542005f52..3710247582 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -91,9 +91,9 @@ without having to rely on implementation details or monkey patching. Some things that you can achieve with this: -* The type detected by Active Record can be overridden. -* A default can also be provided. -* Attributes do not need to be backed by a database column. +- The type detected by Active Record can be overridden. +- A default can also be provided. +- Attributes do not need to be backed by a database column. ```ruby @@ -206,7 +206,7 @@ Please refer to the [Changelog][railties] for detailed changes. * Deprecated `config.static_cache_control` in favor of `config.public_file_server.headers`. - ([Pull Request](https://github.com/rails/rails/pull/22173)) + ([Pull Request](https://github.com/rails/rails/pull/19135)) * Deprecated `config.serve_static_files` in favor of `config.public_file_server.enabled`. ([Pull Request](https://github.com/rails/rails/pull/22173)) diff --git a/guides/source/action_cable_overview.md b/guides/source/action_cable_overview.md index 0d00b7f07b..02db86888c 100644 --- a/guides/source/action_cable_overview.md +++ b/guides/source/action_cable_overview.md @@ -333,7 +333,7 @@ class ChatChannel < ApplicationCable::Channel end def receive(data) - ChatChannel.broadcast_to("chat_#{params[:room]}", data) + ActionCable.server.broadcast("chat_#{params[:room]}", data) end end ``` diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index f914122242..a45becf670 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -241,7 +241,7 @@ generates ```ruby class AddUserRefToProducts < ActiveRecord::Migration[5.0] def change - add_reference :products, :user, index: true, foreign_key: true + add_reference :products, :user, foreign_key: true end end ``` @@ -313,7 +313,7 @@ will produce a migration that looks like this class AddDetailsToProducts < ActiveRecord::Migration[5.0] def change add_column :products, :price, :decimal, precision: 5, scale: 2 - add_reference :products, :supplier, polymorphic: true, index: true + add_reference :products, :supplier, polymorphic: true end end ``` diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 34878e5c38..d11edf2bfa 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -163,7 +163,7 @@ pipeline is enabled. It is set to `true` by default. * `config.assets.js_compressor` defines the JavaScript compressor to use. Possible values are `:closure`, `:uglifier` and `:yui` which require the use of the `closure-compiler`, `uglifier` or `yui-compressor` gems respectively. -* `config.assets.gzip` a flag that enables the creation of gzipped version of compiled assets, along with non-gzipped assets. Set to `true` by default. +* `config.assets.gzip` a flag that enables the creation of gzipped version of compiled assets, along with non-gzipped assets. Set to `true` by default. * `config.assets.paths` contains the paths which are used to look for assets. Appending paths to this configuration option will cause those paths to be used in the search for assets. @@ -297,7 +297,7 @@ All these configuration options are delegated to the `I18n` library. * Or you can set different fallbacks for locales individually. For example, if you want to use `:tr` for `:az` and `:de`, `:en` for `:da` as fallbacks, you can do it, like so: ```ruby - config.i18n.fallbacks = { az: :tr, da: [:de, :en] } + config.i18n.fallbacks = { az: :tr, da: [:de, :en] } #or config.i18n.fallbacks.map = { az: :tr, da: [:de, :en] } ``` @@ -324,7 +324,7 @@ All these configuration options are delegated to the `I18n` library. * `config.active_record.schema_format` controls the format for dumping the database schema to a file. The options are `:ruby` (the default) for a database-independent version that depends on migrations, or `:sql` for a set of (potentially database-dependent) SQL statements. -* `config.active_record.error_on_ignored_order_or_limit` specifies if an error should be raised if the order or limit of a query is ignored during a batch query. The options are `true` (raise error) or `false` (warn). Default is `false`. +* `config.active_record.error_on_ignored_order` specifies if an error should be raised if the order of a query is ignored during a batch query. The options are `true` (raise error) or `false` (warn). Default is `false`. * `config.active_record.timestamped_migrations` controls whether migrations are numbered with serial integers or with timestamps. The default is `true`, to use timestamps, which are preferred if there are multiple developers working on the same application. @@ -529,7 +529,7 @@ There are a number of settings available on `config.action_mailer`: * `:authentication` - If your mail server requires authentication, you need to specify the authentication type here. This is a symbol and one of `:plain`, `:login`, `:cram_md5`. * `:enable_starttls_auto` - Detects if STARTTLS is enabled in your SMTP server and starts to use it. It defaults to `true`. * `:openssl_verify_mode` - When using TLS, you can set how OpenSSL checks the certificate. This is useful if you need to validate a self-signed and/or a wildcard certificate. This can be one of the OpenSSL verify constants, `:none` or `:peer` -- or the constant directly `OpenSSL::SSL::VERIFY_NONE` or `OpenSSL::SSL::VERIFY_PEER`, respectively. - * `:ssl/:tls` - Enables the SMTP connection to use SMTP/TLS (SMTPS: SMTP over direct TLS connection). + * `:ssl/:tls` - Enables the SMTP connection to use SMTP/TLS (SMTPS: SMTP over direct TLS connection). * `config.action_mailer.sendmail_settings` allows detailed configuration for the `sendmail` delivery method. It accepts a hash of options, which can include any of these options: * `:location` - The location of the sendmail executable. Defaults to `/usr/sbin/sendmail`. diff --git a/guides/source/i18n.md b/guides/source/i18n.md index f3802a142f..850f0def03 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -1120,7 +1120,7 @@ Contributing to Rails I18n I18n support in Ruby on Rails was introduced in the release 2.2 and is still evolving. The project follows the good Ruby on Rails development tradition of evolving solutions in gems and real applications first, and only then cherry-picking the best-of-breed of most widely useful features for inclusion in the core. -Thus we encourage everybody to experiment with new ideas and features in gems or other libraries and make them available to the community. (Don't forget to announce your work on our [mailing list](http://groups.google.com/group/rails-i18n!)) +Thus we encourage everybody to experiment with new ideas and features in gems or other libraries and make them available to the community. (Don't forget to announce your work on our [mailing list](http://groups.google.com/group/rails-i18n)!) If you find your own locale (language) missing from our [example translations data](https://github.com/svenfuchs/rails-i18n/tree/master/rails/locale) repository for Ruby on Rails, please [_fork_](https://github.com/guides/fork-a-project-and-submit-your-modifications) the repository, add your data and send a [pull request](https://github.com/guides/pull-requests). diff --git a/guides/source/testing.md b/guides/source/testing.md index 440d87bf73..26d50bec0c 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -879,10 +879,10 @@ can be passed as headers: ```ruby # setting an HTTP Header -get articles_url, headers: "Content-Type" => "text/plain" # simulate the request with custom header +get articles_url, headers: { "Content-Type": "text/plain" } # simulate the request with custom header # setting a CGI variable -get articles_url, headers: "HTTP_REFERER" => "http://example.com/home" # simulate the request with custom env variable +get articles_url, headers: { "HTTP_REFERER": "http://example.com/home" } # simulate the request with custom env variable ``` ### Testing `flash` notices diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index f6552a268b..ef7049e6e5 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -8,6 +8,17 @@ *John Meehan* +* Display name of the class defining the initializer along with the initializer + name in the output of `rails initializers`. + + Before: + disable_dependency_loading + + After: + DemoApp::Application.disable_dependency_loading + + *ta1kt0me* + * Do not run `bundle install` when generating a new plugin. Since bundler 1.12.0, the gemspec is validated so the `bundle install` diff --git a/railties/test/application/mailer_previews_test.rb b/railties/test/application/mailer_previews_test.rb index 3bb77ab0f5..3ea74423cf 100644 --- a/railties/test/application/mailer_previews_test.rb +++ b/railties/test/application/mailer_previews_test.rb @@ -27,7 +27,7 @@ module ApplicationTests assert_equal 404, last_response.status end - test "/rails/mailers is accessible with correct configuraiton" do + test "/rails/mailers is accessible with correct configuration" do add_to_config "config.action_mailer.show_previews = true" app("production") get "/rails/mailers", {}, {"REMOTE_ADDR" => "4.2.42.42"} |