diff options
85 files changed, 817 insertions, 371 deletions
diff --git a/.travis.yml b/.travis.yml index 5c2699371b..ae38617b99 100644 --- a/.travis.yml +++ b/.travis.yml @@ -49,4 +49,4 @@ services: - redis - rabbitmq addons: - postgresql: "9.3" + postgresql: "9.4" diff --git a/actioncable/lib/action_cable/connection/identification.rb b/actioncable/lib/action_cable/connection/identification.rb index 2d75ff8d6d..885ff3f102 100644 --- a/actioncable/lib/action_cable/connection/identification.rb +++ b/actioncable/lib/action_cable/connection/identification.rb @@ -11,7 +11,7 @@ module ActionCable end class_methods do - # Mark a key as being a connection identifier index that can then used to find the specific connection again later. + # Mark a key as being a connection identifier index that can then be used to find the specific connection again later. # Common identifiers are current_user and current_account, but could be anything really. # # Note that anything marked as an identifier will automatically create a delegate by the same name on any diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index b93ae8f8ff..b47e73377c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,8 +1,12 @@ +* Provide the name of HTTP Status code in assertions. + + *Sean Collins* + * More explicit error message when running `rake routes`. `CONTROLLER` argument can now be supplied in different ways: - `Rails::WelcomeController`, `Rails::Welcome`, `rails/welcome` + `Rails::WelcomeController`, `Rails::Welcome`, `rails/welcome`. - Fixes #22918 + Fixes #22918. *Edouard Chin* @@ -10,7 +14,7 @@ helper methods. An `ArgumentError` will be raised if the passed parameters are not secure. - Fixes #22832 + Fixes #22832. *Prathamesh Sonpatki* @@ -26,7 +30,7 @@ or unfiltered values based on from where it is called, `to_h` or `to_unsafe_h` respectively. - Fixes #22841 + Fixes #22841. *Prathamesh Sonpatki* diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 4cd67a85cc..5cbf4157a4 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -109,7 +109,8 @@ module ActionController cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false - delegate :keys, :key?, :has_key?, :empty?, :include?, :inspect, to: :@parameters + delegate :keys, :key?, :has_key?, :empty?, :include?, :inspect, + :as_json, to: :@parameters # By default, never raise an UnpermittedParameters exception if these # params are present. The default includes both 'controller' and 'action' @@ -419,7 +420,7 @@ module ActionController # params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none # params.fetch(:none, 'Francesco') # => "Francesco" # params.fetch(:none) { 'Francesco' } # => "Francesco" - def fetch(key, *args, &block) + def fetch(key, *args) convert_value_to_parameters( @parameters.fetch(key) { if block_given? @@ -514,7 +515,7 @@ module ActionController # to key. If the key is not found, returns the default value. If the # optional code block is given and the key is not found, pass in the key # and return the result of block. - def delete(key, &block) + def delete(key) convert_value_to_parameters(@parameters.delete(key)) end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index c55720859e..b43bb9dc17 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -8,6 +8,10 @@ require 'rails-dom-testing' module ActionController # :stopdoc: + class Metal + include Testing::Functional + end + # ActionController::TestCase will be deprecated and moved to a gem in Rails 5.1. # Please use ActionDispatch::IntegrationTest going forward. class TestRequest < ActionDispatch::TestRequest #:nodoc: @@ -455,16 +459,16 @@ module ActionController parameters = nil end - if parameters.present? || session.present? || flash.present? + if parameters || session || flash non_kwarg_request_warning end end - if body.present? + if body @request.set_header 'RAW_POST_DATA', body end - if http_method.present? + if http_method http_method = http_method.to_s.upcase else http_method = "GET" @@ -472,16 +476,12 @@ module ActionController parameters ||= {} - if format.present? + if format parameters[:format] = format end @html_document = nil - unless @controller.respond_to?(:recycle!) - @controller.extend(Testing::Functional) - end - self.cookies.update @request.cookies self.cookies.update_cookies_from_jar @request.set_header 'HTTP_COOKIE', cookies.to_header diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 7bc6575ccd..1e4df07d6e 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -95,6 +95,7 @@ module ActionDispatch autoload :TestProcess autoload :TestRequest autoload :TestResponse + autoload :AssertionResponse end end diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 30ade14c26..842dfa5827 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -91,7 +91,7 @@ module ActionDispatch DATE = 'Date'.freeze LAST_MODIFIED = "Last-Modified".freeze - SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public must-revalidate]) + SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate]) def cache_control_segments if cache_control = _cache_control diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 29cf821090..5427425ef7 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -259,7 +259,7 @@ module ActionDispatch end # Returns the IP address of client as a +String+, - # usually set by the RemoteIp middleware. + # usually set by the RemoteIp middleware. def remote_ip @remote_ip ||= (get_header("action_dispatch.remote_ip") || ip).to_s end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 9b11111a67..14f86c7c07 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -232,7 +232,7 @@ module ActionDispatch # :nodoc: end # Sets the HTTP character set. In case of nil parameter - # it sets the charset to utf-8. + # it sets the charset to utf-8. # # response.charset = 'utf-16' # => 'utf-16' # response.charset = nil # => 'utf-8' @@ -412,6 +412,13 @@ module ActionDispatch # :nodoc: end def before_sending + # Normally we've already committed by now, but it's possible + # (e.g., if the controller action tries to read back its own + # response) to get here before that. In that case, we must force + # an "early" commit: we're about to freeze the headers, so this is + # our last chance. + commit! unless committed? + headers.freeze request.commit_cookie_jar! unless committed? end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index ea9ab3821d..41c220236a 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -27,7 +27,7 @@ module ActionDispatch # in the server's `public/` directory (see Static#call). def match?(path) path = ::Rack::Utils.unescape_path path - return false unless path.valid_encoding? + return false unless valid_path?(path) path = Rack::Utils.clean_path_info path paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] @@ -94,6 +94,10 @@ module ActionDispatch false end end + + def valid_path?(path) + path.valid_encoding? && !path.include?("\0") + end end # This middleware will attempt to return the contents of a file's body from diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 9e7fcbd849..42890225fa 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -109,7 +109,7 @@ module ActionDispatch @delegate.values end - # Writes given value to given key of the session. + # Writes given value to given key of the session. def []=(key, value) load_for_write! @delegate[key.to_s] = value @@ -148,8 +148,8 @@ module ActionDispatch @delegate.delete key.to_s end - # Returns value of given key from the session, or raises +KeyError+ - # if can't find given key in case of not setted dafault value. + # Returns value of the given key from the session, or raises +KeyError+ + # if can't find the given key and no default value is set. # Returns default value if specified. # # session.fetch(:foo) diff --git a/actionpack/lib/action_dispatch/testing/assertion_response.rb b/actionpack/lib/action_dispatch/testing/assertion_response.rb new file mode 100644 index 0000000000..3fb81ff083 --- /dev/null +++ b/actionpack/lib/action_dispatch/testing/assertion_response.rb @@ -0,0 +1,49 @@ +module ActionDispatch + # This is a class that abstracts away an asserted response. + # It purposely does not inherit from Response, because it doesn't need it. + # That means it does not have headers or a body. + # + # As an input to the initializer, we take a Fixnum, a String, or a Symbol. + # If it's a Fixnum or String, we figure out what its symbolized name. + # If it's a Symbol, we figure out what its corresponding code is. + # The resulting code will be a Fixnum, for real HTTP codes, and it will + # be a String for the pseudo-HTTP codes, such as: + # :success, :missing, :redirect and :error + class AssertionResponse + attr_reader :code, :name + + GENERIC_RESPONSE_CODES = { # :nodoc: + success: "2XX", + missing: "404", + redirect: "3XX", + error: "5XX" + } + + def initialize(code_or_name) + if code_or_name.is_a?(Symbol) + @name = code_or_name + @code = code_from_name(code_or_name) + else + @name = name_from_code(code_or_name) + @code = code_or_name + end + + raise ArgumentError, "Invalid response name: #{name}" if @code.nil? + raise ArgumentError, "Invalid response code: #{code}" if @name.nil? + end + + def code_and_name + "#{code}: #{name}" + end + + private + + def code_from_name(name) + GENERIC_RESPONSE_CODES[name] || Rack::Utils::SYMBOL_TO_STATUS_CODE[name] + end + + def name_from_code(code) + GENERIC_RESPONSE_CODES.invert[code] || Rack::Utils::HTTP_STATUS_CODES[code] + end + end +end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index c138660a21..cd55b7d975 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -1,4 +1,3 @@ - module ActionDispatch module Assertions # A small suite of assertions that test responses from \Rails applications. @@ -29,18 +28,10 @@ module ActionDispatch def assert_response(type, message = nil) message ||= generate_response_message(type) - if Symbol === type - if [:success, :missing, :redirect, :error].include?(type) - assert @response.send(RESPONSE_PREDICATES[type]), message - else - code = Rack::Utils::SYMBOL_TO_STATUS_CODE[type] - if code.nil? - raise ArgumentError, "Invalid response type :#{type}" - end - assert_equal code, @response.response_code, message - end + if RESPONSE_PREDICATES.keys.include?(type) + assert @response.send(RESPONSE_PREDICATES[type]), message else - assert_equal type, @response.response_code, message + assert_equal AssertionResponse.new(type).code, @response.response_code, message end end @@ -85,8 +76,9 @@ module ActionDispatch end end - def generate_response_message(type, code = @response.response_code) - "Expected response to be a <#{type}>, but was a <#{code}>" + def generate_response_message(expected, actual = @response.response_code) + "Expected response to be a <#{code_with_name(expected)}>,"\ + " but was a <#{code_with_name(actual)}>" .concat location_if_redirected end @@ -95,6 +87,14 @@ module ActionDispatch location = normalize_argument_to_redirection(@response.location) " redirect to <#{location}>" end + + def code_with_name(code_or_name) + if RESPONSE_PREDICATES.values.include?("#{code_or_name}?".to_sym) + code_or_name = RESPONSE_PREDICATES.invert["#{code_or_name}?".to_sym] + end + + AssertionResponse.new(code_or_name).code_and_name + end end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 604ba267d6..1ef10ff956 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -20,7 +20,11 @@ rescue LoadError puts "'drb/unix' is not available" end -PROCESS_COUNT = (ENV['N'] || 4).to_i +if ENV['TRAVIS'] + PROCESS_COUNT = 0 +else + PROCESS_COUNT = (ENV['N'] || 4).to_i +end require 'active_support/testing/autorun' require 'abstract_controller' diff --git a/actionpack/test/assertions/response_assertions_test.rb b/actionpack/test/assertions/response_assertions_test.rb index 841fa6aaad..579ce0ed29 100644 --- a/actionpack/test/assertions/response_assertions_test.rb +++ b/actionpack/test/assertions/response_assertions_test.rb @@ -74,7 +74,18 @@ module ActionDispatch @response.status = 404 error = assert_raises(Minitest::Assertion) { assert_response :success } - expected = "Expected response to be a <success>, but was a <404>" + expected = "Expected response to be a <2XX: success>,"\ + " but was a <404: Not Found>" + assert_match expected, error.message + end + + def test_error_message_shows_404_when_asserted_for_200 + @response = ActionDispatch::Response.new + @response.status = 404 + + error = assert_raises(Minitest::Assertion) { assert_response 200 } + expected = "Expected response to be a <200: OK>,"\ + " but was a <404: Not Found>" assert_match expected, error.message end @@ -84,7 +95,8 @@ module ActionDispatch @response.location = 'http://test.host/posts/redirect/1' error = assert_raises(Minitest::Assertion) { assert_response :success } - expected = "Expected response to be a <success>, but was a <302>" \ + expected = "Expected response to be a <2XX: success>,"\ + " but was a <302: Found>" \ " redirect to <http://test.host/posts/redirect/1>" assert_match expected, error.message end @@ -95,7 +107,8 @@ module ActionDispatch @response.location = 'http://test.host/posts/redirect/2' error = assert_raises(Minitest::Assertion) { assert_response 301 } - expected = "Expected response to be a <301>, but was a <302>" \ + expected = "Expected response to be a <301: Moved Permanently>,"\ + " but was a <302: Found>" \ " redirect to <http://test.host/posts/redirect/2>" assert_match expected, error.message end diff --git a/actionpack/test/controller/http_basic_authentication_test.rb b/actionpack/test/controller/http_basic_authentication_test.rb index 194f5b3790..adcf259317 100644 --- a/actionpack/test/controller/http_basic_authentication_test.rb +++ b/actionpack/test/controller/http_basic_authentication_test.rb @@ -5,6 +5,7 @@ class HttpBasicAuthenticationTest < ActionController::TestCase before_action :authenticate, only: :index before_action :authenticate_with_request, only: :display before_action :authenticate_long_credentials, only: :show + before_action :auth_with_special_chars, only: :special_creds http_basic_authenticate_with :name => "David", :password => "Goliath", :only => :search @@ -20,6 +21,10 @@ class HttpBasicAuthenticationTest < ActionController::TestCase render plain: 'Only for loooooong credentials' end + def special_creds + render plain: 'Only for special credentials' + end + def search render plain: 'All inline' end @@ -40,6 +45,12 @@ class HttpBasicAuthenticationTest < ActionController::TestCase end end + def auth_with_special_chars + authenticate_or_request_with_http_basic do |username, password| + username == 'login!@#$%^&*()_+{}[];"\',./<>?`~ \n\r\t' && password == 'pwd:!@#$%^&*()_+{}[];"\',./<>?`~ \n\r\t' + end + end + def authenticate_long_credentials authenticate_or_request_with_http_basic do |username, password| username == '1234567890123456789012345678901234567890' && password == '1234567890123456789012345678901234567890' @@ -133,6 +144,14 @@ class HttpBasicAuthenticationTest < ActionController::TestCase assert_equal 'Definitely Maybe', @response.body end + test "authentication request with valid credential special chars" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials('login!@#$%^&*()_+{}[];"\',./<>?`~ \n\r\t', 'pwd:!@#$%^&*()_+{}[];"\',./<>?`~ \n\r\t') + get :special_creds + + assert_response :success + assert_equal 'Only for special credentials', @response.body + end + test "authenticate with class method" do @request.env['HTTP_AUTHORIZATION'] = encode_credentials('David', 'Goliath') get :search diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 97875c3cbb..a8f4d877a6 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -27,6 +27,12 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params[:person][:name].permitted? end + test "as_json returns the JSON representation of the parameters hash" do + assert_not @params.as_json.key? "parameters" + assert_not @params.as_json.key? "permitted" + assert @params.as_json.key? "person" + end + test "each carries permitted status" do @params.permit! @params.each { |key, value| assert(value.permitted?) if key == "person" } diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index c37679bc5f..43ee2efd9f 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -37,6 +37,27 @@ class ResponseTest < ActiveSupport::TestCase assert_equal "closed stream", e.message end + def test_read_body_during_action + @response.body = "Hello, World!" + + # even though there's no explicitly set content-type, + assert_equal nil, @response.content_type + + # after the action reads back @response.body, + assert_equal "Hello, World!", @response.body + + # the response can be built. + status, headers, body = @response.to_a + assert_equal 200, status + assert_equal({ + "Content-Type" => "text/html; charset=utf-8" + }, headers) + + parts = [] + body.each { |part| parts << part } + assert_equal ["Hello, World!"], parts + end + def test_response_body_encoding body = ["hello".encode(Encoding::UTF_8)] response = ActionDispatch::Response.new 200, {}, body diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 1da57ab50b..ea8b5e904e 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -40,6 +40,10 @@ module StaticTests assert_equal "Hello, World!", get("/doorkeeper%E3E4".force_encoding('ASCII-8BIT')).body end + def test_handles_urls_with_null_byte + assert_equal "Hello, World!", get("/doorkeeper%00").body + end + def test_sets_cache_control app = assert_deprecated do ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60") diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index dbdd4378d0..e31ce85610 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -2,7 +2,7 @@ before the actual input radio tags to make the real value override the hidden when passed. - Fixes #22773 + Fixes #22773. *Santiago Pastorino* diff --git a/actionview/lib/action_view/helpers/atom_feed_helper.rb b/actionview/lib/action_view/helpers/atom_feed_helper.rb index bb1cdd0f8d..dba70e284e 100644 --- a/actionview/lib/action_view/helpers/atom_feed_helper.rb +++ b/actionview/lib/action_view/helpers/atom_feed_helper.rb @@ -51,7 +51,7 @@ module ActionView # * <tt>:language</tt>: Defaults to "en-US". # * <tt>:root_url</tt>: The HTML alternative that this feed is doubling for. Defaults to / on the current host. # * <tt>:url</tt>: The URL for this feed. Defaults to the current URL. - # * <tt>:id</tt>: The id for this feed. Defaults to "tag:localhost,2005:/posts", in this case. + # * <tt>:id</tt>: The id for this feed. Defaults to "tag:localhost,2005:/posts", in this case. # * <tt>:schema_date</tt>: The date at which the tag scheme for the feed was first used. A good default is the year you # created the feed. See http://feedvalidator.org/docs/error/InvalidTAG.html for more information. If not specified, # 2005 is used (as an "I don't care" value). diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index 432693bc23..58ce042f12 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -204,12 +204,12 @@ module ActionView # Attempts to pluralize the +singular+ word unless +count+ is 1. If # +plural+ is supplied, it will use that when count is > 1, otherwise - # it will use the Inflector to determine the plural form. + # it will use the Inflector to determine the plural form for the given locale, + # which defaults to I18n.locale # - # If passed an optional +locale:+ parameter, the word will be pluralized - # using rules defined for that language (you must define your own - # inflection rules for languages other than English). See - # ActiveSupport::Inflector.pluralize + # The word will be pluralized using rules defined for the locale + # (you must define your own inflection rules for languages other than English). + # See ActiveSupport::Inflector.pluralize # # pluralize(1, 'person') # # => 1 person @@ -217,7 +217,7 @@ module ActionView # pluralize(2, 'person') # # => 2 people # - # pluralize(3, 'person', 'users') + # pluralize(3, 'person', plural: 'users') # # => 3 users # # pluralize(0, 'person') @@ -225,7 +225,14 @@ module ActionView # # pluralize(2, 'Person', locale: :de) # # => 2 Personen - def pluralize(count, singular, plural = nil, locale: nil) + def pluralize(count, singular, deprecated_plural = nil, plural: nil, locale: I18n.locale) + if deprecated_plural + ActiveSupport::Deprecation.warn("Passing plural as a positional argument " \ + "is deprecated and will be removed in Rails 5.1. Use e.g. " \ + "pluralize(1, 'person', plural: 'people') instead.") + plural ||= deprecated_plural + end + word = if (count == 1 || count =~ /^1(\.0+)?$/) singular else diff --git a/actionview/test/template/text_helper_test.rb b/actionview/test/template/text_helper_test.rb index fae1965ffa..fb98ac6330 100644 --- a/actionview/test/template/text_helper_test.rb +++ b/actionview/test/template/text_helper_test.rb @@ -379,24 +379,36 @@ class TextHelperTest < ActionView::TestCase assert_equal("1.25 counts", pluralize('1.25', "count")) assert_equal("1.0 count", pluralize('1.0', "count")) assert_equal("1.00 count", pluralize('1.00', "count")) - assert_equal("2 counters", pluralize(2, "count", "counters")) - assert_equal("0 counters", pluralize(nil, "count", "counters")) + assert_equal("2 counters", pluralize(2, "count", plural: "counters")) + assert_equal("0 counters", pluralize(nil, "count", plural: "counters")) assert_equal("2 people", pluralize(2, "person")) assert_equal("10 buffaloes", pluralize(10, "buffalo")) assert_equal("1 berry", pluralize(1, "berry")) assert_equal("12 berries", pluralize(12, "berry")) end - def test_pluralization_with_locale - ActiveSupport::Inflector.inflections(:de) do |inflect| - inflect.plural(/(person)$/i, '\1en') - inflect.singular(/(person)en$/i, '\1') - end + def test_localized_pluralization + old_locale = I18n.locale + + begin + I18n.locale = :de + + ActiveSupport::Inflector.inflections(:de) do |inflect| + inflect.irregular 'region', 'regionen' + end - assert_equal("2 People", pluralize(2, "Person", locale: :en)) - assert_equal("2 Personen", pluralize(2, "Person", locale: :de)) + assert_equal("1 region", pluralize(1, "region")) + assert_equal("2 regionen", pluralize(2, "region")) + assert_equal("2 regions", pluralize(2, "region", locale: :en)) + ensure + I18n.locale = old_locale + end + end - ActiveSupport::Inflector.inflections(:de).clear + def test_deprecated_plural_as_positional_argument + assert_deprecated do + pluralize(2, 'count', 'counters') + end end def test_cycle_class diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 5790528e97..eaeb808144 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,17 @@ +* Fix regression when loading fixture files with symbol keys. + + Fixes #22584. + + *Yves Senn* + +* Use `version` column as primary key for schema_migrations table because + `schema_migrations` versions are guaranteed to be unique. + + This makes it possible to use `update_attributes` on models that do + not have a primary key. + + *Richard Schneeman* + * Add short-hand methods for text and blob types in MySQL. In Pg and Sqlite3, `:text` and `:binary` have variable unlimited length. @@ -77,6 +91,14 @@ defaults without breaking existing migrations, or forcing them to be rewritten through a deprecation cycle. + New migrations specify the Rails version they were written for: + + class AddStatusToOrders < ActiveRecord::Migration[5.0] + def change + # ... + end + end + *Matthew Draper*, *Ravil Bayramgalin* * Use bind params for `limit` and `offset`. This will generate significantly diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 462b3066ab..f6d8e8a342 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1164,6 +1164,7 @@ module ActiveRecord # Adds one or more objects to the collection by setting their foreign keys to the collection's primary key. # Note that this operation instantly fires update SQL without waiting for the save or update call on the # parent object, unless the parent object is a new record. + # This will also run validations and callbacks of associated object(s). # [collection.delete(object, ...)] # Removes one or more objects from the collection by setting their foreign keys to +NULL+. # Objects will be in addition destroyed if they're associated with <tt>dependent: :destroy</tt>, diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index a6ad09a38a..be65cf318c 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -74,9 +74,8 @@ module ActiveRecord value = foreign_klass.base_class.name column = klass.columns_hash[reflection.type.to_s] - substitute = klass.connection.substitute_at(column) binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name)) - constraint = constraint.and table[reflection.type].eq substitute + constraint = constraint.and table[reflection.type].eq(Arel::Nodes::BindParam.new) end joins << table.create_join(table, table.create_on(constraint), join_type) 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 d9b9271fb0..061628725d 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -31,6 +31,8 @@ module ActiveRecord if value.acts_like?(:time) value.in_time_zone + elsif value.is_a?(::Float) + value else map(value) { |v| convert_time_to_time_zone(v) } end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 0ac5e80119..7a2a1a0e33 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -106,7 +106,7 @@ module ActiveRecord exec_query(sql, name, binds) end - # Returns the last auto-generated ID from the affected table. + # Executes an INSERT query and returns the new record's ID # # +id_value+ will be returned unless the value is nil, in # which case the database will attempt to calculate the last inserted @@ -115,9 +115,12 @@ module ActiveRecord # If the next id was calculated in advance (as in Oracle), it should be # passed in as +id_value+. def insert(arel, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) - insert_sql(to_sql(arel, binds), name, pk, id_value, sequence_name, binds) + sql, binds, pk, sequence_name = sql_for_insert(to_sql(arel, binds), pk, id_value, sequence_name, binds) + value = exec_insert(sql, name, binds, pk, sequence_name) + id_value || last_inserted_id(value) end alias create insert + alias insert_sql insert # Executes the update statement and returns the number of rows affected. def update(arel, name = nil, binds = []) @@ -297,14 +300,15 @@ module ActiveRecord # Inserts the given fixture into the table. Overridden in adapters that require # something beyond a simple insert (eg. Oracle). def insert_fixture(fixture, table_name) - columns = schema_cache.columns_hash(table_name) + fixture = fixture.stringify_keys + columns = schema_cache.columns_hash(table_name) binds = fixture.map do |name, value| if column = columns[name] type = lookup_cast_type_from_column(column) Relation::QueryAttribute.new(name, value, type) else - raise Fixture::FixtureError, %(table "#{table_name}" has no column named "#{name}".) + raise Fixture::FixtureError, %(table "#{table_name}" has no column named #{name.inspect}.) end end key_list = fixture.keys.map { |name| quote_column_name(name) } @@ -352,13 +356,6 @@ module ActiveRecord end alias join_to_delete join_to_update - # Executes an INSERT query and returns the new record's ID - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) - sql, binds = sql_for_insert(sql, pk, id_value, sequence_name, binds) - value = exec_insert(sql, name, binds, pk, sequence_name) - id_value || last_inserted_id(value) - end - protected # Returns a subquery for the given key using the join information. @@ -378,7 +375,7 @@ module ActiveRecord end def sql_for_insert(sql, pk, id_value, sequence_name, binds) - [sql, binds] + [sql, binds, pk, sequence_name] end def last_inserted_id(result) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index dce34a208f..d9b42d4283 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -311,12 +311,6 @@ module ActiveRecord {} end - # Returns a bind substitution value given a bind +column+ - # NOTE: The column param is currently being used by the sqlserver-adapter - def substitute_at(column, _unused = 0) - Arel::Nodes::BindParam.new - end - # REFERENTIAL INTEGRITY ==================================== # Override to turn off referential integrity while executing <tt>&block</tt>. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 828e46637d..d435d91102 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -1,7 +1,9 @@ require 'active_record/connection_adapters/abstract_adapter' +require 'active_record/connection_adapters/mysql/column' require 'active_record/connection_adapters/mysql/schema_creation' require 'active_record/connection_adapters/mysql/schema_definitions' require 'active_record/connection_adapters/mysql/schema_dumper' +require 'active_record/connection_adapters/mysql/type_metadata' require 'active_support/core_ext/string/strip' @@ -19,78 +21,6 @@ module ActiveRecord MySQL::SchemaCreation.new(self) end - class Column < ConnectionAdapters::Column # :nodoc: - delegate :strict, :extra, to: :sql_type_metadata, allow_nil: true - - def initialize(*) - super - assert_valid_default(default) - extract_default - end - - def extract_default - if blob_or_text_column? - @default = null || strict ? nil : '' - end - end - - def has_default? - return false if blob_or_text_column? # MySQL forbids defaults on blob and text columns - super - end - - def blob_or_text_column? - /\A(?:tiny|medium|long)?blob\b/ === sql_type || type == :text - end - - def unsigned? - /\bunsigned\z/ === sql_type - end - - def case_sensitive? - collation && !collation.match(/_ci$/) - end - - def auto_increment? - extra == 'auto_increment' - end - - private - - def assert_valid_default(default) - if blob_or_text_column? && default.present? - raise ArgumentError, "#{type} columns cannot have a default value: #{default.inspect}" - end - end - end - - class MysqlTypeMetadata < DelegateClass(SqlTypeMetadata) # :nodoc: - attr_reader :extra, :strict - - def initialize(type_metadata, extra: "", strict: false) - super(type_metadata) - @type_metadata = type_metadata - @extra = extra - @strict = strict - end - - def ==(other) - other.is_a?(MysqlTypeMetadata) && - attributes_for_hash == other.attributes_for_hash - end - alias eql? == - - def hash - attributes_for_hash.hash - end - - protected - - def attributes_for_hash - [self.class, @type_metadata, extra, strict] - end - end - ## # :singleton-method: # By default, the Mysql2Adapter will consider all columns of type <tt>tinyint(1)</tt> @@ -234,7 +164,7 @@ module ActiveRecord end def new_column(field, default, sql_type_metadata = nil, null = true, default_function = nil, collation = nil) # :nodoc: - Column.new(field, default, sql_type_metadata, null, default_function, collation) + MySQL::Column.new(field, default, sql_type_metadata, null, default_function, collation) end # Must return the MySQL error number from the exception, if the exception has an @@ -848,7 +778,7 @@ module ActiveRecord end def fetch_type_metadata(sql_type, extra = "") - MysqlTypeMetadata.new(super(sql_type), extra: extra, strict: strict_mode?) + MySQL::TypeMetadata.new(super(sql_type), extra: extra, strict: strict_mode?) end def add_index_length(option_strings, column_names, options = {}) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/column.rb b/activerecord/lib/active_record/connection_adapters/mysql/column.rb new file mode 100644 index 0000000000..9c45fdd44a --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/mysql/column.rb @@ -0,0 +1,50 @@ +module ActiveRecord + module ConnectionAdapters + module MySQL + class Column < ConnectionAdapters::Column # :nodoc: + delegate :strict, :extra, to: :sql_type_metadata, allow_nil: true + + def initialize(*) + super + assert_valid_default + extract_default + end + + def has_default? + return false if blob_or_text_column? # MySQL forbids defaults on blob and text columns + super + end + + def blob_or_text_column? + /\A(?:tiny|medium|long)?blob\b/ === sql_type || type == :text + end + + def unsigned? + /\bunsigned\z/ === sql_type + end + + def case_sensitive? + collation && collation !~ /_ci\z/ + end + + def auto_increment? + extra == 'auto_increment' + end + + private + + def extract_default + if blob_or_text_column? + @default = null || strict ? nil : '' + end + end + + def assert_valid_default + if blob_or_text_column? && default.present? + raise ArgumentError, "#{type} columns cannot have a default value: #{default.inspect}" + end + end + end + end + end +end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb new file mode 100644 index 0000000000..e1e3f7b472 --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb @@ -0,0 +1,32 @@ +module ActiveRecord + module ConnectionAdapters + module MySQL + class TypeMetadata < DelegateClass(SqlTypeMetadata) # :nodoc: + attr_reader :extra, :strict + + def initialize(type_metadata, extra: "", strict: false) + super(type_metadata) + @type_metadata = type_metadata + @extra = extra + @strict = strict + end + + def ==(other) + other.is_a?(MySQL::TypeMetadata) && + attributes_for_hash == other.attributes_for_hash + end + alias eql? == + + def hash + attributes_for_hash.hash + end + + protected + + def attributes_for_hash + [self.class, @type_metadata, extra, strict] + end + end + end + end +end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 11a151edd5..6c15facf3b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -72,16 +72,6 @@ module ActiveRecord end end - # Executes an INSERT query and returns the new record's ID - def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) # :nodoc: - unless pk - # Extract the table from the insert sql. Yuck. - table_ref = extract_table_ref_from_insert_sql(sql) - pk = primary_key(table_ref) if table_ref - end - super - end - # The internal PostgreSQL identifier of the money data type. MONEY_COLUMN_TYPE_OID = 790 #:nodoc: # The internal PostgreSQL identifier of the BYTEA data type. @@ -162,12 +152,18 @@ module ActiveRecord end alias :exec_update :exec_delete - def sql_for_insert(sql, pk, id_value, sequence_name, binds) + def sql_for_insert(sql, pk, id_value, sequence_name, binds) # :nodoc: + unless pk + # Extract the table from the insert sql. Yuck. + table_ref = extract_table_ref_from_insert_sql(sql) + pk = primary_key(table_ref) if table_ref + end + if pk && use_insert_returning? sql = "#{sql} RETURNING #{quote_column_name(pk)}" end - [sql, binds] + super end def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index e313a34c3a..1c20e4700f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -690,15 +690,7 @@ module ActiveRecord end # Returns the current ID of a table's sequence. - def last_insert_id(sequence_name) #:nodoc: - Integer(last_insert_id_value(sequence_name)) - end - - def last_insert_id_value(sequence_name) - last_insert_id_result(sequence_name).rows.first.first - end - - def last_insert_id_result(sequence_name) #:nodoc: + def last_insert_id_result(sequence_name) # :nodoc: exec_query("SELECT currval('#{sequence_name}')", 'SQL') end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 1250f8a3c3..475a298467 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -275,7 +275,7 @@ module ActiveRecord def relation # :nodoc: relation = Relation.create(self, arel_table, predicate_builder) - if finder_needs_type_condition? + if finder_needs_type_condition? && !ignore_default_scope? relation.where(type_condition).create_with(inheritance_column.to_sym => sti_name) else relation diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 04519f4dc3..8655f68308 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -187,7 +187,7 @@ module ActiveRecord # scope :active, -> { where status: 0 } klass.send(:detect_enum_conflict!, name, value_method_name, true) - klass.scope value_method_name, -> { klass.where name => value } + klass.scope value_method_name, -> { where(name => value) } end end defined_enums[name.to_s] = enum_values diff --git a/activerecord/lib/active_record/internal_metadata.rb b/activerecord/lib/active_record/internal_metadata.rb index 7bd66028b6..e5c6e5c885 100644 --- a/activerecord/lib/active_record/internal_metadata.rb +++ b/activerecord/lib/active_record/internal_metadata.rb @@ -30,16 +30,15 @@ module ActiveRecord ActiveSupport::Deprecation.silence { connection.table_exists?(table_name) } end - # Creates a internal metadata table with columns +key+ and +value+ + # Creates an internal metadata table with columns +key+ and +value+ def create_table unless table_exists? - connection.create_table(table_name, primary_key: :key, id: false ) do |t| + connection.create_table(table_name, id: false) do |t| t.column :key, :string t.column :value, :string t.timestamps + t.index :key, unique: true, name: index_name end - - connection.add_index table_name, :key, unique: true, name: index_name end end end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index f699e83ab3..f5b29c7f2e 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -145,7 +145,7 @@ module ActiveRecord class NoEnvironmentInSchemaError < MigrationError #:nodoc: def initialize - msg = "Environment data not found in the schema. To resolve this issue, run: \n\n\tbin/rake db:migrate" + msg = "Environment data not found in the schema. To resolve this issue, run: \n\n\tbin/rake db:environment:set" if defined?(Rails.env) super("#{msg} RAILS_ENV=#{::Rails.env}") else @@ -157,7 +157,7 @@ module ActiveRecord class ProtectedEnvironmentError < ActiveRecordError #:nodoc: def initialize(env = "production") msg = "You are attempting to run a destructive action against your '#{env}' database\n" - msg << "if you are sure you want to continue, run the same command with the environment variable\n" + msg << "If you are sure you want to continue, run the same command with the environment variable\n" msg << "DISABLE_DATABASE_ENVIRONMENT_CHECK=1" super(msg) end @@ -165,10 +165,15 @@ module ActiveRecord class EnvironmentMismatchError < ActiveRecordError def initialize(current: nil, stored: nil) - msg = "You are attempting to modify a database that was last run in #{ stored } environment.\n" - msg << "You are running in #{ current } environment." - msg << "if you are sure you want to continue, run the same command with the environment variable\n" - msg << "DISABLE_DATABASE_ENVIRONMENT_CHECK=1" + msg = "You are attempting to modify a database that was last run in `#{ stored }` environment.\n" + msg << "You are running in `#{ current }` environment." + msg << "If you are sure you want to continue, first set the environment using:\n\n" + msg << "\tbin/rake db:environment:set" + if defined?(Rails.env) + super("#{msg} RAILS_ENV=#{::Rails.env}") + else + super(msg) + end end end @@ -1165,45 +1170,58 @@ module ActiveRecord private + # Used for running a specific migration. def run_without_lock migration = migrations.detect { |m| m.version == @target_version } raise UnknownMigrationVersionError.new(@target_version) if migration.nil? - unless (up? && migrated.include?(migration.version.to_i)) || (down? && !migrated.include?(migration.version.to_i)) - begin - execute_migration_in_transaction(migration, @direction) - rescue => e - canceled_msg = use_transaction?(migration) ? ", this migration was canceled" : "" - raise StandardError, "An error has occurred#{canceled_msg}:\n\n#{e}", e.backtrace - end - end + execute_migration_in_transaction(migration, @direction) + + record_environment end + # Used for running multiple migrations up to or down to a certain value. def migrate_without_lock - if !target && @target_version && @target_version > 0 + if invalid_target? raise UnknownMigrationVersionError.new(@target_version) end runnable.each do |migration| - Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger - - begin - execute_migration_in_transaction(migration, @direction) - rescue => e - canceled_msg = use_transaction?(migration) ? "this and " : "" - raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace - end + execute_migration_in_transaction(migration, @direction) end + + record_environment + end + + # Stores the current environment in the database. + def record_environment + return if down? + ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment end def ran?(migration) migrated.include?(migration.version.to_i) end + # Return true if a valid version is not provided. + def invalid_target? + !target && @target_version && @target_version > 0 + end + def execute_migration_in_transaction(migration, direction) + return if down? && !migrated.include?(migration.version.to_i) + return if up? && migrated.include?(migration.version.to_i) + + Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger + ddl_transaction(migration) do migration.migrate(direction) record_version_state_after_migrating(migration.version) end + rescue => e + msg = "An error has occurred, " + msg << "this and " if use_transaction?(migration) + msg << "all later migrations canceled:\n\n#{e}" + raise StandardError, msg, e.backtrace end def target @@ -1233,7 +1251,6 @@ module ActiveRecord else migrated << version ActiveRecord::SchemaMigration.create!(version: version.to_s) - ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index ead9407391..d81d6b54b3 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -1,6 +1,12 @@ require 'active_record' db_namespace = namespace :db do + desc "Set the environment value for the database" + task "environment:set" => [:environment, :load_config] do + ActiveRecord::InternalMetadata.create_table + ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment + end + task :check_protected_environments => [:environment, :load_config] do ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index a549b28f16..37e18626b5 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -545,7 +545,7 @@ module ActiveRecord end end - # returns either nil or the inverse association name that it finds. + # returns either false or the inverse association name that it finds. def automatic_inverse_of if can_find_inverse_of_automatically?(self) inverse_name = ActiveSupport::Inflector.underscore(options[:as] || active_record.name.demodulize).to_sym diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 6aa490908b..032b8d4c5d 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -99,7 +99,7 @@ module ActiveRecord end substitutes = values.map do |(arel_attr, _)| - [arel_attr, connection.substitute_at(klass.columns_hash[arel_attr.name])] + [arel_attr, Arel::Nodes::BindParam.new] end [substitutes, binds] diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index cb971eb255..396638d74d 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -141,6 +141,9 @@ module ActiveRecord end def merge_single_values + if relation.from_clause.empty? + relation.from_clause = other.from_clause + end relation.lock_value ||= other.lock_value unless other.create_with_value.blank? diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 5635b4215b..716b1e8505 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -659,8 +659,10 @@ module ActiveRecord end def or!(other) # :nodoc: - unless structurally_compatible_for_or?(other) - raise ArgumentError, 'Relation passed to #or must be structurally compatible' + incompatible_values = structurally_incompatible_values_for_or(other) + + unless incompatible_values.empty? + raise ArgumentError, "Relation passed to #or must be structurally compatible. Incompatible values: #{incompatible_values}" end self.where_clause = self.where_clause.or(other.where_clause) @@ -1189,10 +1191,10 @@ module ActiveRecord end end - def structurally_compatible_for_or?(other) - Relation::SINGLE_VALUE_METHODS.all? { |m| send("#{m}_value") == other.send("#{m}_value") } && - (Relation::MULTI_VALUE_METHODS - [:extending]).all? { |m| send("#{m}_values") == other.send("#{m}_values") } && - (Relation::CLAUSE_METHODS - [:having, :where]).all? { |m| send("#{m}_clause") == other.send("#{m}_clause") } + def structurally_incompatible_values_for_or(other) + Relation::SINGLE_VALUE_METHODS.reject { |m| send("#{m}_value") == other.send("#{m}_value") } + + (Relation::MULTI_VALUE_METHODS - [:extending]).reject { |m| send("#{m}_values") == other.send("#{m}_values") } + + (Relation::CLAUSE_METHODS - [:having, :where]).reject { |m| send("#{m}_clause") == other.send("#{m}_clause") } end def new_where_clause diff --git a/activerecord/lib/active_record/schema_migration.rb b/activerecord/lib/active_record/schema_migration.rb index 51b9b17395..ee4c71f304 100644 --- a/activerecord/lib/active_record/schema_migration.rb +++ b/activerecord/lib/active_record/schema_migration.rb @@ -9,7 +9,7 @@ module ActiveRecord class SchemaMigration < ActiveRecord::Base # :nodoc: class << self def primary_key - nil + "version" end def table_name @@ -31,8 +31,8 @@ module ActiveRecord connection.create_table(table_name, id: false) do |t| t.column :version, :string, version_options + t.index :version, unique: true, name: index_name end - connection.add_index table_name, :version, unique: true, name: index_name end end diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb index e395970dc6..7794af8ca4 100644 --- a/activerecord/lib/active_record/scoping.rb +++ b/activerecord/lib/active_record/scoping.rb @@ -11,11 +11,11 @@ module ActiveRecord module ClassMethods def current_scope #:nodoc: - ScopeRegistry.value_for(:current_scope, self.to_s) + ScopeRegistry.value_for(:current_scope, self) end def current_scope=(scope) #:nodoc: - ScopeRegistry.set_value_for(:current_scope, self.to_s, scope) + ScopeRegistry.set_value_for(:current_scope, self, scope) end # Collects attributes from scopes that should be applied when creating @@ -53,18 +53,18 @@ module ActiveRecord # following code: # # registry = ActiveRecord::Scoping::ScopeRegistry - # registry.set_value_for(:current_scope, "Board", some_new_scope) + # registry.set_value_for(:current_scope, Board, some_new_scope) # # Now when you run: # - # registry.value_for(:current_scope, "Board") + # registry.value_for(:current_scope, Board) # # You will obtain whatever was defined in +some_new_scope+. The #value_for # and #set_value_for methods are delegated to the current ScopeRegistry # object, so the above example code can also be called as: # # ActiveRecord::Scoping::ScopeRegistry.set_value_for(:current_scope, - # "Board", some_new_scope) + # Board, some_new_scope) class ScopeRegistry # :nodoc: extend ActiveSupport::PerThreadRegistry @@ -74,16 +74,22 @@ module ActiveRecord @registry = Hash.new { |hash, key| hash[key] = {} } end - # Obtains the value for a given +scope_name+ and +variable_name+. - def value_for(scope_type, variable_name) + # Obtains the value for a given +scope_type+ and +model+. + def value_for(scope_type, model) raise_invalid_scope_type!(scope_type) - @registry[scope_type][variable_name] + klass = model + base = model.base_class + while klass <= base + value = @registry[scope_type][klass.name] + return value if value + klass = klass.superclass + end end - # Sets the +value+ for a given +scope_type+ and +variable_name+. - def set_value_for(scope_type, variable_name, value) + # Sets the +value+ for a given +scope_type+ and +model+. + def set_value_for(scope_type, model, value) raise_invalid_scope_type!(scope_type) - @registry[scope_type][variable_name] = value + @registry[scope_type][model.name] = value end private diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index cdcb73382f..8baf3b8044 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -122,11 +122,11 @@ module ActiveRecord end def ignore_default_scope? # :nodoc: - ScopeRegistry.value_for(:ignore_default_scope, self) + ScopeRegistry.value_for(:ignore_default_scope, base_class) end def ignore_default_scope=(ignore) # :nodoc: - ScopeRegistry.set_value_for(:ignore_default_scope, self, ignore) + ScopeRegistry.set_value_for(:ignore_default_scope, base_class, ignore) end # The ignore_default_scope flag is used to prevent an infinite recursion diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 6a9af5d1b4..8f52e9068a 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -43,7 +43,7 @@ module ActiveRecord LOCAL_HOSTS = ['127.0.0.1', 'localhost'] def check_protected_environments! - unless ENV['DISABLE_DATABASE_internal_metadata'] + unless ENV['DISABLE_DATABASE_ENVIRONMENT_CHECK'] current = ActiveRecord::Migrator.current_environment stored = ActiveRecord::Migrator.last_stored_environment diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index e361521155..f1995b243a 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -139,8 +139,8 @@ module ActiveRecord def test_sql_for_insert_with_returning_disabled connection = connection_without_insert_returning - result = connection.sql_for_insert('sql', nil, nil, nil, 'binds') - assert_equal ['sql', 'binds'], result + sql, binds = connection.sql_for_insert('sql', nil, nil, nil, 'binds') + assert_equal ['sql', 'binds'], [sql, binds] end def test_serial_sequence @@ -322,11 +322,6 @@ module ActiveRecord end end - def test_substitute_at - bind = @connection.substitute_at(nil) - assert_equal Arel.sql('$1'), bind.to_sql - end - def test_partial_index with_example_table do @connection.add_index 'ex', %w{ id number }, :name => 'partial', :where => "number > 100" diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index a2fd1177a6..038d9e2d0f 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -130,11 +130,6 @@ module ActiveRecord assert_equal 'UTF-8', @conn.encoding end - def test_bind_value_substitute - bind_param = @conn.substitute_at('foo') - assert_equal Arel.sql('?'), bind_param.to_sql - end - def test_exec_no_binds with_example_table 'id int, data string' do result = @conn.exec_query('SELECT id, data FROM ex') diff --git a/activerecord/test/cases/ar_schema_test.rb b/activerecord/test/cases/ar_schema_test.rb index 9d5327bf35..1f32c48b95 100644 --- a/activerecord/test/cases/ar_schema_test.rb +++ b/activerecord/test/cases/ar_schema_test.rb @@ -21,10 +21,10 @@ if ActiveRecord::Base.connection.supports_migrations? ActiveRecord::Migration.verbose = @original_verbose end - def test_has_no_primary_key + def test_has_has_primary_key old_primary_key_prefix_type = ActiveRecord::Base.primary_key_prefix_type ActiveRecord::Base.primary_key_prefix_type = :table_name_with_underscore - assert_nil ActiveRecord::SchemaMigration.primary_key + assert_equal "version", ActiveRecord::SchemaMigration.primary_key ActiveRecord::SchemaMigration.create_table assert_difference "ActiveRecord::SchemaMigration.count", 1 do diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index ba3e16bdb2..ecdf508e3e 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1275,9 +1275,10 @@ class BasicsTest < ActiveRecord::TestCase UnloadablePost.send(:current_scope=, UnloadablePost.all) UnloadablePost.unloadable - assert_not_nil ActiveRecord::Scoping::ScopeRegistry.value_for(:current_scope, "UnloadablePost") + klass = UnloadablePost + assert_not_nil ActiveRecord::Scoping::ScopeRegistry.value_for(:current_scope, klass) ActiveSupport::Dependencies.remove_unloadable_constants! - assert_nil ActiveRecord::Scoping::ScopeRegistry.value_for(:current_scope, "UnloadablePost") + assert_nil ActiveRecord::Scoping::ScopeRegistry.value_for(:current_scope, klass) ensure Object.class_eval{ remove_const :UnloadablePost } if defined?(UnloadablePost) end diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 1e38b97c4a..cd9c76f1f0 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -39,7 +39,7 @@ module ActiveRecord end def test_binds_are_logged - sub = @connection.substitute_at(@pk) + sub = Arel::Nodes::BindParam.new binds = [Relation::QueryAttribute.new("id", 1, Type::Value.new)] sql = "select * from topics where id = #{sub.to_sql}" diff --git a/activerecord/test/cases/column_definition_test.rb b/activerecord/test/cases/column_definition_test.rb index 783a374116..81162b7e98 100644 --- a/activerecord/test/cases/column_definition_test.rb +++ b/activerecord/test/cases/column_definition_test.rb @@ -41,49 +41,49 @@ module ActiveRecord if current_adapter?(:Mysql2Adapter) def test_should_set_default_for_mysql_binary_data_types type = SqlTypeMetadata.new(type: :binary, sql_type: "binary(1)") - binary_column = AbstractMysqlAdapter::Column.new("title", "a", type) + binary_column = MySQL::Column.new("title", "a", type) assert_equal "a", binary_column.default type = SqlTypeMetadata.new(type: :binary, sql_type: "varbinary") - varbinary_column = AbstractMysqlAdapter::Column.new("title", "a", type) + varbinary_column = MySQL::Column.new("title", "a", type) assert_equal "a", varbinary_column.default end def test_should_be_empty_string_default_for_mysql_binary_data_types type = SqlTypeMetadata.new(type: :binary, sql_type: "binary(1)") - binary_column = AbstractMysqlAdapter::Column.new("title", "", type, false) + binary_column = MySQL::Column.new("title", "", type, false) assert_equal "", binary_column.default type = SqlTypeMetadata.new(type: :binary, sql_type: "varbinary") - varbinary_column = AbstractMysqlAdapter::Column.new("title", "", type, false) + varbinary_column = MySQL::Column.new("title", "", type, false) assert_equal "", varbinary_column.default end def test_should_not_set_default_for_blob_and_text_data_types assert_raise ArgumentError do - AbstractMysqlAdapter::Column.new("title", "a", SqlTypeMetadata.new(sql_type: "blob")) + MySQL::Column.new("title", "a", SqlTypeMetadata.new(sql_type: "blob")) end - text_type = AbstractMysqlAdapter::MysqlTypeMetadata.new( + text_type = MySQL::TypeMetadata.new( SqlTypeMetadata.new(type: :text)) assert_raise ArgumentError do - AbstractMysqlAdapter::Column.new("title", "Hello", text_type) + MySQL::Column.new("title", "Hello", text_type) end - text_column = AbstractMysqlAdapter::Column.new("title", nil, text_type) + text_column = MySQL::Column.new("title", nil, text_type) assert_equal nil, text_column.default - not_null_text_column = AbstractMysqlAdapter::Column.new("title", nil, text_type, false) + not_null_text_column = MySQL::Column.new("title", nil, text_type, false) assert_equal "", not_null_text_column.default end def test_has_default_should_return_false_for_blob_and_text_data_types binary_type = SqlTypeMetadata.new(sql_type: "blob") - blob_column = AbstractMysqlAdapter::Column.new("title", nil, binary_type) + blob_column = MySQL::Column.new("title", nil, binary_type) assert !blob_column.has_default? text_type = SqlTypeMetadata.new(type: :text) - text_column = AbstractMysqlAdapter::Column.new("title", nil, text_type) + text_column = MySQL::Column.new("title", nil, text_type) assert !text_column.has_default? end end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index c73958900b..da934ab8fe 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -223,6 +223,10 @@ class FixturesTest < ActiveRecord::TestCase assert_equal(%(table "parrots" has no column named "arrr".), e.message) end + def test_yaml_file_with_symbol_columns + ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT + "/naked/yml", "trees") + end + def test_omap_fixtures assert_nothing_raised do fixtures = ActiveRecord::FixtureSet.new(Account.connection, 'categories', Category, FIXTURES_ROOT + "/categories_ordered") diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index cfa223f93e..f51e366b1d 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -301,7 +301,7 @@ class MigrationTest < ActiveRecord::TestCase e = assert_raise(StandardError) { migrator.run } - assert_equal "An error has occurred, this migration was canceled:\n\nSomething broke", e.message + assert_equal "An error has occurred, this and all later migrations canceled:\n\nSomething broke", e.message assert_no_column Person, :last_name, "On error, the Migrator should revert schema changes but it did not." @@ -380,6 +380,33 @@ class MigrationTest < ActiveRecord::TestCase old_path = ActiveRecord::Migrator.migrations_paths ActiveRecord::Migrator.migrations_paths = migrations_path + ActiveRecord::Migrator.up(migrations_path) + assert_equal current_env, ActiveRecord::InternalMetadata[:environment] + + original_rails_env = ENV["RAILS_ENV"] + original_rack_env = ENV["RACK_ENV"] + ENV["RAILS_ENV"] = ENV["RACK_ENV"] = "foofoo" + new_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + + refute_equal current_env, new_env + + sleep 1 # mysql by default does not store fractional seconds in the database + ActiveRecord::Migrator.up(migrations_path) + assert_equal new_env, ActiveRecord::InternalMetadata[:environment] + ensure + ActiveRecord::Migrator.migrations_paths = old_path + ENV["RAILS_ENV"] = original_rails_env + ENV["RACK_ENV"] = original_rack_env + end + + + def test_migration_sets_internal_metadata_even_when_fully_migrated + current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + migrations_path = MIGRATIONS_ROOT + "/valid" + old_path = ActiveRecord::Migrator.migrations_paths + ActiveRecord::Migrator.migrations_paths = migrations_path + + ActiveRecord::Migrator.up(migrations_path) assert_equal current_env, ActiveRecord::InternalMetadata[:environment] original_rails_env = ENV["RAILS_ENV"] @@ -390,6 +417,7 @@ class MigrationTest < ActiveRecord::TestCase refute_equal current_env, new_env sleep 1 # mysql by default does not store fractional seconds in the database + ActiveRecord::Migrator.up(migrations_path) assert_equal new_env, ActiveRecord::InternalMetadata[:environment] ensure diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 0a2e874e4f..60a806c05a 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -104,6 +104,13 @@ class RelationMergingTest < ActiveRecord::TestCase post = PostThatLoadsCommentsInAnAfterSaveHook.create!(title: "First Post", body: "Blah blah blah.") assert_equal "First comment!", post.comments.where(body: "First comment!").first_or_create.body end + + def test_merging_with_from_clause + relation = Post.all + assert relation.from_clause.empty? + relation = relation.merge(Post.from("posts")) + refute relation.from_clause.empty? + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb index 2006fc9611..28a0862f91 100644 --- a/activerecord/test/cases/relation/or_test.rb +++ b/activerecord/test/cases/relation/or_test.rb @@ -52,9 +52,11 @@ module ActiveRecord end def test_or_with_incompatible_relations - assert_raises ArgumentError do + error = assert_raises ArgumentError do Post.order('body asc').where('id = 1').or(Post.order('id desc').where(:id => [2, 3])).to_a end + + assert_equal "Relation passed to #or must be structurally compatible. Incompatible values: [:order]", error.message end def test_or_when_grouping diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 86316ab476..ad5ca70f36 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -459,4 +459,18 @@ class DefaultScopingTest < ActiveRecord::TestCase scope = Bus.all assert_equal scope.where_clause.ast.children.length, 1 end + + def test_sti_conditions_are_not_carried_in_default_scope + ConditionalStiPost.create! body: '' + SubConditionalStiPost.create! body: '' + SubConditionalStiPost.create! title: 'Hello world', body: '' + + assert_equal 2, ConditionalStiPost.count + assert_equal 2, ConditionalStiPost.all.to_a.size + assert_equal 3, ConditionalStiPost.unscope(where: :title).to_a.size + + assert_equal 1, SubConditionalStiPost.count + assert_equal 1, SubConditionalStiPost.all.to_a.size + assert_equal 2, SubConditionalStiPost.unscope(where: :title).to_a.size + end end diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index 4bfffbe9c6..c15d57460b 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -209,9 +209,23 @@ class RelationScopingTest < ActiveRecord::TestCase assert_not_equal [], Developer.all end - def test_current_scope_does_not_pollute_other_subclasses - Post.none.scoping do - assert StiPost.all.any? + def test_current_scope_does_not_pollute_sibling_subclasses + Comment.none.scoping do + assert_not SpecialComment.all.any? + assert_not VerySpecialComment.all.any? + assert_not SubSpecialComment.all.any? + end + + SpecialComment.none.scoping do + assert Comment.all.any? + assert VerySpecialComment.all.any? + assert_not SubSpecialComment.all.any? + end + + SubSpecialComment.none.scoping do + assert Comment.all.any? + assert VerySpecialComment.all.any? + assert SpecialComment.all.any? end end end diff --git a/activerecord/test/fixtures/naked/yml/trees.yml b/activerecord/test/fixtures/naked/yml/trees.yml new file mode 100644 index 0000000000..d163b98f21 --- /dev/null +++ b/activerecord/test/fixtures/naked/yml/trees.yml @@ -0,0 +1,3 @@ +root: + :id: 1 + :name: The Root diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 23cebe2602..bf3079a1df 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -263,3 +263,10 @@ end class SerializedPost < ActiveRecord::Base serialize :title end + +class ConditionalStiPost < Post + default_scope { where(title: 'Untitled') } +end + +class SubConditionalStiPost < ConditionalStiPost +end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index cebe19be89..21c79949ca 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,10 +1,12 @@ -* Match `HashWithIndifferentAccess#default`'s behaviour with `Hash#default` +* Match `HashWithIndifferentAccess#default`'s behaviour with `Hash#default`. *David Cornu* -* Adds `:exception_object` key to `ActiveSupport::Notifications::Instrumenter` payload when an exception is raised. +* Adds `:exception_object` key to `ActiveSupport::Notifications::Instrumenter` + payload when an exception is raised. - Adds new key/value pair to payload when an exception is raised: e.g. `:exception_object => #<RuntimeError: FAIL>`. + Adds new key/value pair to payload when an exception is raised: + e.g. `:exception_object => #<RuntimeError: FAIL>`. *Ryan T. Hosford* diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index dff2443bc8..99c55b1aa4 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -17,6 +17,7 @@ module ActiveSupport FILENAME_MAX_SIZE = 228 # max filename size on file system is 255, minus room for timestamp and random characters appended by Tempfile (used by atomic write) FILEPATH_MAX_SIZE = 900 # max is 1024, plus some room EXCLUDED_DIRS = ['.', '..'].freeze + GITKEEP_FILES = ['.gitkeep', '.keep'].freeze def initialize(cache_path, options = nil) super(options) @@ -24,10 +25,10 @@ module ActiveSupport end # Deletes all items from the cache. In this case it deletes all the entries in the specified - # file store directory except for .gitkeep. Be careful which directory is specified in your + # file store directory except for .keep or .gitkeep. Be careful which directory is specified in your # config file when using +FileStore+ because everything in that directory will be deleted. def clear(options = nil) - root_dirs = Dir.entries(cache_path).reject {|f| (EXCLUDED_DIRS + [".gitkeep"]).include?(f)} + root_dirs = exclude_from(cache_path, EXCLUDED_DIRS + GITKEEP_FILES) FileUtils.rm_r(root_dirs.collect{|f| File.join(cache_path, f)}) rescue Errno::ENOENT end @@ -154,7 +155,7 @@ module ActiveSupport # Delete empty directories in the cache. def delete_empty_directories(dir) return if File.realpath(dir) == File.realpath(cache_path) - if Dir.entries(dir).reject {|f| EXCLUDED_DIRS.include?(f)}.empty? + if exclude_from(dir, EXCLUDED_DIRS).empty? Dir.delete(dir) rescue nil delete_empty_directories(File.dirname(dir)) end @@ -193,6 +194,11 @@ module ActiveSupport end end end + + # Exclude entries from source directory + def exclude_from(source, excludes) + Dir.entries(source).reject { |f| excludes.include?(f) } + end end end end diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb index 32fe8025fe..f5ea6669ce 100644 --- a/activesupport/lib/active_support/deprecation/method_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb @@ -21,15 +21,15 @@ module ActiveSupport # # => [:aaa, :bbb, :ccc] # # Fred.aaa - # # DEPRECATION WARNING: aaa is deprecated and will be removed from Rails 5.0. (called from irb_binding at (irb):10) + # # DEPRECATION WARNING: aaa is deprecated and will be removed from Rails 5.1. (called from irb_binding at (irb):10) # # => nil # # Fred.bbb - # # DEPRECATION WARNING: bbb is deprecated and will be removed from Rails 5.0 (use zzz instead). (called from irb_binding at (irb):11) + # # DEPRECATION WARNING: bbb is deprecated and will be removed from Rails 5.1 (use zzz instead). (called from irb_binding at (irb):11) # # => nil # # Fred.ccc - # # DEPRECATION WARNING: ccc is deprecated and will be removed from Rails 5.0 (use Bar#ccc instead). (called from irb_binding at (irb):12) + # # DEPRECATION WARNING: ccc is deprecated and will be removed from Rails 5.1 (use Bar#ccc instead). (called from irb_binding at (irb):12) # # => nil # # Passing in a custom deprecator: diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index ae6f00b861..d9a668c0ea 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -9,7 +9,6 @@ require 'active_support/testing/isolation' require 'active_support/testing/constant_lookup' require 'active_support/testing/time_helpers' require 'active_support/testing/file_fixtures' -require 'active_support/testing/composite_filter' require 'active_support/core_ext/kernel/reporting' module ActiveSupport @@ -39,15 +38,6 @@ module ActiveSupport def test_order ActiveSupport.test_order ||= :random end - - def run(reporter, options = {}) - if options[:patterns] && options[:patterns].any? { |p| p =~ /:\d+/ } - options[:filter] = \ - Testing::CompositeFilter.new(self, options[:filter], options[:patterns]) - end - - super - end end alias_method :method_name, :name diff --git a/activesupport/lib/active_support/testing/composite_filter.rb b/activesupport/lib/active_support/testing/composite_filter.rb deleted file mode 100644 index bde723e30b..0000000000 --- a/activesupport/lib/active_support/testing/composite_filter.rb +++ /dev/null @@ -1,54 +0,0 @@ -require 'method_source' - -module ActiveSupport - module Testing - class CompositeFilter # :nodoc: - def initialize(runnable, filter, patterns) - @runnable = runnable - @filters = [ derive_regexp(filter), *derive_line_filters(patterns) ].compact - end - - def ===(method) - @filters.any? { |filter| filter === method } - end - - private - def derive_regexp(filter) - filter =~ %r%/(.*)/% ? Regexp.new($1) : filter - end - - def derive_line_filters(patterns) - patterns.map do |file_and_line| - file, line = file_and_line.split(':') - Filter.new(@runnable, file, line) if file - end - end - - class Filter # :nodoc: - def initialize(runnable, file, line) - @runnable, @file = runnable, File.expand_path(file) - @line = line.to_i if line - end - - def ===(method) - return unless @runnable.method_defined?(method) - - if @line - test_file, test_range = definition_for(@runnable.instance_method(method)) - test_file == @file && test_range.include?(@line) - else - @runnable.instance_method(method).source_location.first == @file - end - end - - private - def definition_for(method) - file, start_line = method.source_location - end_line = method.source.count("\n") + start_line - 1 - - return file, start_line..end_line - end - end - end - end -end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 2701dc2fe9..4a299429f3 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -780,10 +780,12 @@ class FileStoreTest < ActiveSupport::TestCase include AutoloadingCacheBehavior def test_clear - filepath = File.join(cache_dir, ".gitkeep") - FileUtils.touch(filepath) + gitkeep = File.join(cache_dir, ".gitkeep") + keep = File.join(cache_dir, ".keep") + FileUtils.touch([gitkeep, keep]) @cache.clear - assert File.exist?(filepath) + assert File.exist?(gitkeep) + assert File.exist?(keep) end def test_clear_without_cache_dir diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index d2e2d27737..f68f2d1faf 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -1118,7 +1118,7 @@ Rails default exception handling displays a "500 Server Error" message for all e ### The Default 500 and 404 Templates -By default a production application will render either a 404 or a 500 error message. These messages are contained in static HTML files in the `public` folder, in `404.html` and `500.html` respectively. You can customize these files to add some extra information and style, but remember that they are static HTML; i.e. you can't use ERB, SCSS, CoffeeScript, or layouts for them. +By default a production application will render either a 404 or a 500 error message, in the development environment all unhandled exceptions are raised. These messages are contained in static HTML files in the `public` folder, in `404.html` and `500.html` respectively. You can customize these files to add some extra information and style, but remember that they are static HTML; i.e. you can't use ERB, SCSS, CoffeeScript, or layouts for them. ### `rescue_from` @@ -1174,7 +1174,11 @@ end WARNING: You shouldn't do `rescue_from Exception` or `rescue_from StandardError` unless you have a particular reason as it will cause serious side-effects (e.g. you won't be able to see exception details and tracebacks during development). -NOTE: Certain exceptions are only rescuable from the `ApplicationController` class, as they are raised before the controller gets initialized and the action gets executed. +NOTE: When running in the production environment, all +`ActiveRecord::RecordNotFound` errors render the 404 error page. Unless you need +a custom behavior you don't need to handle this. + +NOTE: Certain exceptions are only rescuable from the `ApplicationController` class, as they are raised before the controller gets initialized and the action gets executed. Force HTTPS protocol -------------------- diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 4606ac4683..fd5399baec 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -59,7 +59,7 @@ To retrieve objects from the database, Active Record provides several finder met The methods are: -* `bind` +* `find` * `create_with` * `distinct` * `eager_load` diff --git a/guides/source/command_line.md b/guides/source/command_line.md index eb9bf3fa18..5240b5f343 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -392,7 +392,7 @@ rake assets:clobber # Remove compiled assets rake assets:precompile # Compile all the assets named in config.assets.precompile rake db:create # Create the database from config/database.yml for the current Rails.env ... -rake log:clear # Truncates all *.log files in log/ to zero bytes (specify which logs with LOGS=test,development) +rake log:clear # Truncates all/specified *.log files in log/ to zero bytes (specify which logs with LOGS=test,development) rake middleware # Prints out your Rack middleware stack ... rake tmp:clear # Clear cache and socket files from tmp/ (narrow w/ tmp:cache:clear, tmp:sockets:clear) diff --git a/guides/source/configuring.md b/guides/source/configuring.md index e9261a3dab..0b24d4d06a 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1119,21 +1119,48 @@ NOTE. If you are running in a multi-threaded environment, there could be a chanc Custom configuration -------------------- -You can configure your own code through the Rails configuration object with custom configuration under the `config.x` property. It works like this: +You can configure your own code through the Rails configuration object with custom configuration. It works like this: ```ruby - config.x.payment_processing.schedule = :daily - config.x.payment_processing.retries = 3 - config.x.super_debugger = true + config.payment_processing.schedule = :daily + config.payment_processing.retries = 3 + config.super_debugger = true ``` These configuration points are then available through the configuration object: ```ruby - Rails.configuration.x.payment_processing.schedule # => :daily - Rails.configuration.x.payment_processing.retries # => 3 - Rails.configuration.x.super_debugger # => true - Rails.configuration.x.super_debugger.not_set # => nil + Rails.configuration.payment_processing.schedule # => :daily + Rails.configuration.payment_processing.retries # => 3 + Rails.configuration.super_debugger # => true + Rails.configuration.super_debugger.not_set # => nil + ``` + +You can also use `Rails::Application.config_for` to load whole configuration files: + + ```ruby + # config/payment.yml: + production: + environment: production + merchant_id: production_merchant_id + public_key: production_public_key + private_key: production_private_key + development: + environment: sandbox + merchant_id: development_merchant_id + public_key: development_public_key + private_key: development_private_key + + # config/application.rb + module MyApp + class Application < Rails::Application + config.payment = config_for(:payment) + end + end + ``` + + ```ruby + Rails.configuration.payment['merchant_id'] # => production_merchant_id or development_merchant_id ``` Search Engines Indexing diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index ed88ecf6ac..cbc304c87f 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -357,7 +357,6 @@ $ bundle exec rake test:sqlite3 You can now run the tests as you did for `sqlite3`. The tasks are respectively: ```bash -test:mysql test:mysql2 test:postgresql ``` diff --git a/guides/source/development_dependencies_install.md b/guides/source/development_dependencies_install.md index 4322f03d05..7beb8f72a9 100644 --- a/guides/source/development_dependencies_install.md +++ b/guides/source/development_dependencies_install.md @@ -165,7 +165,7 @@ $ bundle exec ruby -Itest path/to/test.rb -n test_name ### Active Record Setup -The test suite of Active Record attempts to run four times: once for SQLite3, once for each of the two MySQL gems (`mysql` and `mysql2`), and once for PostgreSQL. We are going to see now how to set up the environment for them. +Active Record's test suite runs three times: once for SQLite3, once for MySQL, and once for PostgreSQL. We are going to see now how to set up the environment for them. WARNING: If you're working with Active Record code, you _must_ ensure that the tests pass for at least MySQL, PostgreSQL, and SQLite3. Subtle differences between the various adapters have been behind the rejection of many patches that looked OK when tested only against MySQL. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 501c7d2ce5..2c363c55da 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,17 @@ +* Bring back `TEST=` env for `rake test` task. + + *Yves Senn* + +* Specify log file names or all logs to clear `rake log:clear` + + Specify which logs to clear when using the `rake log:clear` task, e.g. `rake log:clear LOGS=test,staging` + + Clear all logs from log/*.log e.g. `rake log:clear LOGS=all` + + By default `rake log:clear` clears standard environment log files i.e. 'development,test,production' + + *Pramod Shinde* + * Fix using `add_source` with a block after using `gem` in a custom generator. *Will Fisher* diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb index 78f4530514..f613b40f80 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb @@ -1,5 +1,6 @@ # Be sure to restart your server when you modify this file. -# Require `belongs_to` associations by default. This is a new Rails 5.0 default, -# so introduced as a config to ensure apps made with earlier versions of Rails aren't affected when upgrading. +# Require `belongs_to` associations by default. This is a new Rails 5.0 +# default, so it is introduced as a configuration option to ensure that apps +# made on earlier versions of Rails are not affected when upgrading. Rails.application.config.active_record.belongs_to_required_by_default = true diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb index 0b718aa1c6..649e82280e 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb @@ -1,5 +1,6 @@ # Be sure to restart your server when you modify this file. -# Do not halt callback chains when a callback returns false. This is a new Rails 5.0 default, -# so introduced as a config to ensure apps made with earlier versions of Rails aren't affected when upgrading. +# Do not halt callback chains when a callback returns false. This is a new +# Rails 5.0 default, so it is introduced as a configuration option to ensure +# that apps made with earlier versions of Rails are not affected when upgrading. ActiveSupport.halt_callback_chains_on_return_false = false diff --git a/railties/lib/rails/tasks/log.rake b/railties/lib/rails/tasks/log.rake index 877f175ef3..073f235ec5 100644 --- a/railties/lib/rails/tasks/log.rake +++ b/railties/lib/rails/tasks/log.rake @@ -1,5 +1,12 @@ namespace :log do - desc "Truncates all *.log files in log/ to zero bytes (specify which logs with LOGS=test,development)" + + ## + # Truncates all/specified log files + # ENV['LOGS'] + # - defaults to standard environment log files i.e. 'development,test,production' + # - ENV['LOGS']=all truncates all files i.e. log/*.log + # - ENV['LOGS']='test,development' truncates only specified files + desc "Truncates all/specified *.log files in log/ to zero bytes (specify which logs with LOGS=test,development)" task :clear do log_files.each do |file| clear_log_file(file) @@ -7,15 +14,21 @@ namespace :log do end def log_files - if ENV['LOGS'] - ENV['LOGS'].split(',') - .map { |file| "log/#{file.strip}.log" } - .select { |file| File.exist?(file) } - else + if ENV['LOGS'] == 'all' FileList["log/*.log"] + elsif ENV['LOGS'] + log_files_to_truncate(ENV['LOGS']) + else + log_files_to_truncate("development,test,production") end end + def log_files_to_truncate(envs) + envs.split(',') + .map { |file| "log/#{file.strip}.log" } + .select { |file| File.exist?(file) } + end + def clear_log_file(file) f = File.open(file, "w") f.close diff --git a/railties/lib/rails/test_unit/line_filtering.rb b/railties/lib/rails/test_unit/line_filtering.rb new file mode 100644 index 0000000000..dab4d3631d --- /dev/null +++ b/railties/lib/rails/test_unit/line_filtering.rb @@ -0,0 +1,70 @@ +require 'method_source' + +module Rails + module LineFiltering # :nodoc: + def run(reporter, options = {}) + if options[:patterns] && options[:patterns].any? { |p| p =~ /:\d+/ } + options[:filter] = \ + CompositeFilter.new(self, options[:filter], options[:patterns]) + end + + super + end + end + + class CompositeFilter # :nodoc: + def initialize(runnable, filter, patterns) + @runnable = runnable + @filters = [ derive_regexp(filter), *derive_line_filters(patterns) ].compact + end + + # Minitest uses === to find matching filters. + def ===(method) + @filters.any? { |filter| filter === method } + end + + private + def derive_regexp(filter) + # Regexp filtering copied from Minitest. + filter =~ %r%/(.*)/% ? Regexp.new($1) : filter + end + + def derive_line_filters(patterns) + patterns.flat_map do |file_and_line| + file, *lines = file_and_line.split(':') + + if lines.empty? + Filter.new(@runnable, file, nil) if file + else + lines.map { |line| Filter.new(@runnable, file, line) } + end + end + end + end + + class Filter # :nodoc: + def initialize(runnable, file, line) + @runnable, @file = runnable, File.expand_path(file) + @line = line.to_i if line + end + + def ===(method) + return unless @runnable.method_defined?(method) + + if @line + test_file, test_range = definition_for(@runnable.instance_method(method)) + test_file == @file && test_range.include?(@line) + else + @runnable.instance_method(method).source_location.first == @file + end + end + + private + def definition_for(method) + file, start_line = method.source_location + end_line = method.source.count("\n") + start_line - 1 + + return file, start_line..end_line + end + end +end diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index d4ab2ada66..29a3d991b8 100644 --- a/railties/lib/rails/test_unit/minitest_plugin.rb +++ b/railties/lib/rails/test_unit/minitest_plugin.rb @@ -80,7 +80,8 @@ module Minitest Minitest.backtrace_filter = ::Rails.backtrace_cleaner if ::Rails.respond_to?(:backtrace_cleaner) end - self.reporter.reporters.clear # Replace progress reporter for colors. + # Replace progress reporter for colors. + self.reporter.reporters.delete_if { |reporter| reporter.kind_of?(SummaryReporter) || reporter.kind_of?(ProgressReporter) } self.reporter << SuppressedSummaryReporter.new(options[:io], options) self.reporter << ::Rails::TestUnitReporter.new(options[:io], options) end diff --git a/railties/lib/rails/test_unit/railtie.rb b/railties/lib/rails/test_unit/railtie.rb index 75180ff978..511cee33bd 100644 --- a/railties/lib/rails/test_unit/railtie.rb +++ b/railties/lib/rails/test_unit/railtie.rb @@ -1,3 +1,5 @@ +require 'rails/test_unit/line_filtering' + if defined?(Rake.application) && Rake.application.top_level_tasks.grep(/^(default$|test(:|$))/).any? ENV['RAILS_ENV'] ||= 'test' end @@ -11,6 +13,10 @@ module Rails c.integration_tool :test_unit end + initializer "test_unit.line_filtering" do + ActiveSupport::TestCase.extend Rails::LineFiltering + end + rake_tasks do load "rails/test_unit/testing.rake" end diff --git a/railties/lib/rails/test_unit/test_requirer.rb b/railties/lib/rails/test_unit/test_requirer.rb index 83d2c55ffd..8b9933bed4 100644 --- a/railties/lib/rails/test_unit/test_requirer.rb +++ b/railties/lib/rails/test_unit/test_requirer.rb @@ -15,7 +15,7 @@ module Rails private def expand_patterns(patterns) patterns.map do |arg| - arg = arg.gsub(/:(\d+)?$/, '') + arg = arg.gsub(/(:\d*)+?$/, '') if Dir.exist?(arg) "#{arg}/**/*_test.rb" else diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake index 6676c6a079..41921e43f3 100644 --- a/railties/lib/rails/test_unit/testing.rake +++ b/railties/lib/rails/test_unit/testing.rake @@ -7,7 +7,12 @@ task default: :test desc "Runs all tests in test folder" task :test do $: << "test" - Minitest.rake_run(["test"]) + pattern = if ENV.key?('TEST') + ENV['TEST'] + else + "test" + end + Minitest.rake_run([pattern]) end namespace :test do diff --git a/railties/test/application/bin_setup_test.rb b/railties/test/application/bin_setup_test.rb index d279f198f2..8c3ab65c51 100644 --- a/railties/test/application/bin_setup_test.rb +++ b/railties/test/application/bin_setup_test.rb @@ -21,13 +21,13 @@ module ApplicationTests RUBY list_tables = lambda { `bin/rails runner 'p ActiveRecord::Base.connection.tables'`.strip } - File.write("log/my.log", "zomg!") + File.write("log/test.log", "zomg!") assert_equal '[]', list_tables.call - assert_equal 5, File.size("log/my.log") + assert_equal 5, File.size("log/test.log") assert_not File.exist?("tmp/restart.txt") `bin/setup 2>&1` - assert_equal 0, File.size("log/my.log") + assert_equal 0, File.size("log/test.log") assert_equal '["articles", "schema_migrations", "active_record_internal_metadatas"]', list_tables.call assert File.exist?("tmp/restart.txt") end diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 5ea4b28acb..639875dd6e 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -30,7 +30,7 @@ module ApplicationTests env RAILS_ENV=production bin/rake db:create db:migrate; env RAILS_ENV=production bin/rake db:test:prepare test 2>&1` - assert_match /ActiveRecord::ProtectedEnvironmentError/, output + assert_match(/ActiveRecord::ProtectedEnvironmentError/, output) end end @@ -40,7 +40,7 @@ module ApplicationTests env RAILS_ENV=test bin/rake db:create db:migrate; env RAILS_ENV=test bin/rake db:test:prepare test 2>&1` - refute_match /ActiveRecord::ProtectedEnvironmentError/, output + refute_match(/ActiveRecord::ProtectedEnvironmentError/, output) end end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 868153762d..bb6c6574c5 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -294,6 +294,84 @@ module ApplicationTests end end + def test_more_than_one_line_filter + app_file 'test/models/post_test.rb', <<-RUBY + require 'test_helper' + + class PostTest < ActiveSupport::TestCase + test "first filter" do + puts 'PostTest:FirstFilter' + assert true + end + + test "second filter" do + puts 'PostTest:SecondFilter' + assert true + end + + test "test line filter does not run this" do + assert true + end + end + RUBY + + run_test_command('test/models/post_test.rb:4:9').tap do |output| + assert_match 'PostTest:FirstFilter', output + assert_match 'PostTest:SecondFilter', output + assert_match '2 runs, 2 assertions', output + end + end + + def test_more_than_one_line_filter_with_multiple_files + app_file 'test/models/account_test.rb', <<-RUBY + require 'test_helper' + + class AccountTest < ActiveSupport::TestCase + test "first filter" do + puts 'AccountTest:FirstFilter' + assert true + end + + test "second filter" do + puts 'AccountTest:SecondFilter' + assert true + end + + test "line filter does not run this" do + assert true + end + end + RUBY + + app_file 'test/models/post_test.rb', <<-RUBY + require 'test_helper' + + class PostTest < ActiveSupport::TestCase + test "first filter" do + puts 'PostTest:FirstFilter' + assert true + end + + test "second filter" do + puts 'PostTest:SecondFilter' + assert true + end + + test "line filter does not run this" do + assert true + end + end + RUBY + + run_test_command('test/models/account_test.rb:4:9 test/models/post_test:4:9').tap do |output| + assert_match 'AccountTest:FirstFilter', output + assert_match 'AccountTest:SecondFilter', output + assert_match 'PostTest:FirstFilter', output + assert_match 'PostTest:SecondFilter', output + assert_match '4 runs, 4 assertions', output + end + end + def test_multiple_line_filters create_test_file :models, 'account' create_test_file :models, 'post' @@ -372,6 +450,17 @@ module ApplicationTests assert_match(%r{cannot load such file.+test/not_exists\.rb}, error) end + def test_pass_TEST_env_on_rake_test + create_test_file :models, 'account' + create_test_file :models, 'post', pass: false + + output = Dir.chdir(app_path) { `bin/rake test TEST=test/models/post_test.rb` } + + assert_match "PostTest", output, "passing TEST= should run selected test" + assert_no_match "AccountTest", output, "passing TEST= should only run selected test" + assert_match '1 runs, 1 assertions', output + end + private def run_test_command(arguments = 'test/unit/test_test.rb') Dir.chdir(app_path) { `bin/rails t #{arguments}` } |