diff options
40 files changed, 289 insertions, 192 deletions
diff --git a/actionmailer/lib/action_mailer/test_case.rb b/actionmailer/lib/action_mailer/test_case.rb index d3eded547c..1d09a4ee96 100644 --- a/actionmailer/lib/action_mailer/test_case.rb +++ b/actionmailer/lib/action_mailer/test_case.rb @@ -41,6 +41,7 @@ module ActionMailer setup :initialize_test_deliveries setup :set_expected_mail teardown :restore_test_deliveries + ActiveSupport.run_load_hooks(:action_mailer_test_case, self) end module ClassMethods diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a66a1e8af3..999ac82d42 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,10 +1,27 @@ +* Fix nested multiple roots + + The PR #20940 enabled the use of multiple roots with different constraints + at the top level but unfortunately didn't work when those roots were inside + a namespace and also broke the use of root inside a namespace after a top + level root was defined because the check for the existence of the named route + used the global :root name and not the namespaced name. + + This is fixed by using the name_for_action method to expand the :root name to + the full namespaced name. We can pass nil for the second argument as we're not + dealing with resource definitions so don't need to handle the cases for edit + and new routes. + + Fixes #26148. + + *Ryo Hashimoto*, *Andrew White* + * Include the content of the flash in the auto-generated etag. This solves the following problem: 1. POST /messages 2. redirect_to messages_url, notice: 'Message was created' 3. GET /messages/1 4. GET /messages - + Step 4 would before still include the flash message, even though it's no longer relevant, because the etag cache was recorded with the flash in place and didn't change when it was gone. diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index b598dd4d77..6a1acb64f7 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -70,6 +70,7 @@ module ActionController #:nodoc: send_file_headers! options self.status = options[:status] || 200 + self.content_type = options[:type] if options.key?(:type) self.content_type = options[:content_type] if options.key?(:content_type) response.send_file path end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index c414527d63..4137db99ea 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -48,7 +48,7 @@ module ActionController "NOTE! For XHR/Ajax or API requests, this action would normally " \ "respond with 204 No Content: an empty white screen. Since you're " \ "loading it in a web browser, we assume that you expected to " \ - "actually render a template, not… nothing, so we're showing an " \ + "actually render a template, not nothing, so we're showing an " \ "error to be extra-clear. If you expect 204 No Content, carry on. " \ "That's what you'll get from an XHR or API request. Give it a shot." diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 11a6cf8e15..d84320b713 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -627,6 +627,7 @@ module ActionController include ActionDispatch::Assertions class_attribute :_controller_class setup :setup_controller_request_and_response + ActiveSupport.run_load_hooks(:action_controller_test_case, self) end private diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index d5eef2987d..0528a6ad08 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -86,7 +86,7 @@ module ActionDispatch @req.fetch_header(env_name(key)) do return default unless default == DEFAULT return yield if block_given? - raise NameError, key + raise KeyError, key end end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 9788431943..e8173e2a99 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -224,8 +224,10 @@ module ActionDispatch # :nodoc: # Sets the HTTP content type. def content_type=(content_type) - header_info = parse_content_type - set_content_type content_type.to_s, header_info.charset || self.class.default_charset + return unless content_type + new_header_info = parse_content_type(content_type.to_s) + prev_header_info = parsed_content_type_header + set_content_type new_header_info.mime_type, new_header_info.charset || prev_header_info.charset || self.class.default_charset end # Sets the HTTP response's content MIME type. For example, in the controller @@ -238,7 +240,7 @@ module ActionDispatch # :nodoc: # information. def content_type - parse_content_type.mime_type + parsed_content_type_header.mime_type end def sending_file=(v) @@ -253,7 +255,7 @@ module ActionDispatch # :nodoc: # response.charset = 'utf-16' # => 'utf-16' # response.charset = nil # => 'utf-8' def charset=(charset) - header_info = parse_content_type + header_info = parsed_content_type_header if false == charset set_header CONTENT_TYPE, header_info.mime_type else @@ -265,7 +267,7 @@ module ActionDispatch # :nodoc: # The charset of the response. HTML wants to know the encoding of the # content you're giving them, so we need to send that along. def charset - header_info = parse_content_type + header_info = parsed_content_type_header header_info.charset || self.class.default_charset end @@ -403,8 +405,7 @@ module ActionDispatch # :nodoc: ContentTypeHeader = Struct.new :mime_type, :charset NullContentTypeHeader = ContentTypeHeader.new nil, nil - def parse_content_type - content_type = get_header CONTENT_TYPE + def parse_content_type(content_type) if content_type type, charset = content_type.split(/;\s*charset=/) type = nil if type.empty? @@ -414,6 +415,12 @@ module ActionDispatch # :nodoc: end end + # Small internal convenience method to get the parsed version of the current + # content type header. + def parsed_content_type_header + parse_content_type(get_header(CONTENT_TYPE)) + end + def set_content_type(content_type, charset) type = (content_type || "").dup type << "; charset=#{charset}" if charset @@ -450,7 +457,7 @@ module ActionDispatch # :nodoc: def assign_default_content_type_and_charset! return if content_type - ct = parse_content_type + ct = parsed_content_type_header set_content_type(ct.mime_type || Mime[:html].to_s, ct.charset || self.class.default_charset) end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index b3acac42fc..67b9463a6a 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1927,7 +1927,7 @@ to this: end def match_root_route(options) - name = has_named_route?(:root) ? nil : :root + name = has_named_route?(name_for_action(:root, nil)) ? nil : :root args = ["/", { as: name, via: :get }.merge!(options)] match(*args) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 13f7fc6fa6..f5ec28a9c2 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -420,9 +420,13 @@ module ActionDispatch attr_reader :app + def initialize(*args, &blk) + super(*args, &blk) + @integration_session = nil + end + def before_setup # :nodoc: @app = nil - @integration_session = nil super end diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 8d5e3ce2fe..2b3859aa57 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -229,14 +229,12 @@ class ForceSSLFlashTest < ActionController::TestCase assert_response 302 assert_equal "http://test.host/force_ssl_flash/cheeseburger", redirect_to_url - # FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists @request.env.delete("PATH_INFO") get :cheeseburger assert_response 301 assert_equal "https://test.host/force_ssl_flash/cheeseburger", redirect_to_url - # FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists @request.env.delete("PATH_INFO") get :use_flash diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 25420ead3b..8dc565ac8d 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -233,4 +233,18 @@ class SendFileTest < ActionController::TestCase response = process("file") assert_equal 200, response.status end + + def test_send_file_charset_with_type_options_key + @controller = SendFileWithActionControllerLive.new + @controller.options = { type: "text/calendar; charset=utf-8" } + response = process("file") + assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"] + end + + def test_send_file_charset_with_content_type_options_key + @controller = SendFileWithActionControllerLive.new + @controller.options = { content_type: "text/calendar" } + response = process("file") + assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"] + end end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 7a155d70cb..374e618b42 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -158,4 +158,10 @@ class HeaderTest < ActiveSupport::TestCase assert_equal({ "HTTP_REFERER"=>"http://example.com/", "CONTENT_TYPE"=>"text/plain" }, env) end + + test "fetch exception" do + assert_raises KeyError do + @headers.fetch(:some_key_that_doesnt_exist) + end + end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index ee5c30ef0e..0938460632 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3669,6 +3669,48 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_multiple_roots + draw do + namespace :foo do + root "pages#index", constraints: { host: 'www.example.com' } + root "admin/pages#index", constraints: { host: 'admin.example.com' } + end + + root "pages#index", constraints: { host: 'www.example.com' } + root "admin/pages#index", constraints: { host: 'admin.example.com' } + end + + get "http://www.example.com/foo" + assert_equal "foo/pages#index", @response.body + + get "http://admin.example.com/foo" + assert_equal "foo/admin/pages#index", @response.body + + get "http://www.example.com/" + assert_equal "pages#index", @response.body + + get "http://admin.example.com/" + assert_equal "admin/pages#index", @response.body + end + + def test_namespaced_roots + draw do + namespace :foo do + root "test#index" + end + + root "test#index" + + namespace :bar do + root "test#index" + end + end + + assert_equal "/foo", foo_root_path + assert_equal "/", root_path + assert_equal "/bar", bar_root_path + end + private def draw(&block) diff --git a/actionpack/test/dispatch/runner_test.rb b/actionpack/test/dispatch/runner_test.rb new file mode 100644 index 0000000000..969933c9ed --- /dev/null +++ b/actionpack/test/dispatch/runner_test.rb @@ -0,0 +1,18 @@ +require "abstract_unit" + +class RunnerTest < ActiveSupport::TestCase + test "runner preserves the setting of integration_session" do + runner = Class.new do + def before_setup + + end + end.new + + runner.extend(ActionDispatch::Integration::Runner) + runner.integration_session.host! "lvh.me" + + runner.before_setup + + assert_equal "lvh.me", runner.integration_session.host + end +end diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index b598469d01..5258a01144 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -211,6 +211,8 @@ module ActionView end end + attr_reader :cache_hit # :nodoc: + private def fragment_name_with_digest(name, virtual_path) #:nodoc: @@ -224,13 +226,12 @@ module ActionView end end - # TODO: Create an object that has caching read/write on it def fragment_for(name = {}, options = nil, &block) #:nodoc: if content = read_fragment_for(name, options) - @log_payload_for_partial_render[:cache_hit] = true if defined?(@log_payload_for_partial_render) + @cache_hit = true content else - @log_payload_for_partial_render[:cache_hit] = false if defined?(@log_payload_for_partial_render) + @cache_hit = false write_fragment_for(name, options, &block) end end diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index 4314e1ff71..4b6aecd187 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -333,8 +333,6 @@ module ActionView view, locals, block = @view, @locals, @block object, as = @object, @variable - view.instance_variable_set(:@log_payload_for_partial_render, payload) - if !block && (layout = @options[:layout]) layout = find_template(layout.to_s, @template_keys) end @@ -347,6 +345,7 @@ module ActionView end content = layout.render(view, locals) { content } if layout + payload[:cache_hit] = view.cache_hit content end end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 5480501988..513935cef0 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -152,7 +152,7 @@ module ActionView # we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. def render(view, locals, buffer=nil, &block) - instrument("!render_template".freeze) do + instrument_render_template do compile!(view) view.send(method_name, locals, buffer, &block) end @@ -341,13 +341,17 @@ module ActionView end def instrument(action, &block) - payload = { virtual_path: @virtual_path, identifier: @identifier } - case action - when "!render_template".freeze - ActiveSupport::Notifications.instrument("!render_template.action_view".freeze, payload, &block) - else - ActiveSupport::Notifications.instrument("#{action}.action_view".freeze, payload, &block) - end + ActiveSupport::Notifications.instrument("#{action}.action_view".freeze, instrument_payload, &block) + end + + private + + def instrument_render_template(&block) + ActiveSupport::Notifications.instrument("!render_template.action_view".freeze, instrument_payload, &block) + end + + def instrument_payload + { virtual_path: @virtual_path, identifier: @identifier } end end end diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 2805cfe612..3eb1ac0826 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -152,6 +152,7 @@ module ActionView included do setup :setup_with_controller + ActiveSupport.run_load_hooks(:action_view_test_case, self) end private diff --git a/activejob/lib/active_job/test_case.rb b/activejob/lib/active_job/test_case.rb index 0d97e00bfa..a5ec45e4a7 100644 --- a/activejob/lib/active_job/test_case.rb +++ b/activejob/lib/active_job/test_case.rb @@ -3,5 +3,7 @@ require "active_support/test_case" module ActiveJob class TestCase < ActiveSupport::TestCase include ActiveJob::TestHelper + + ActiveSupport.run_load_hooks(:active_job_test_case, self) end end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 45ef14013a..191039f598 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -372,7 +372,7 @@ module ActiveModel To achieve the same use: - errors.add(attribute, :empty, options) if value.blank? + errors.add(attribute, :blank, options) if value.blank? MESSAGE Array(attributes).each do |attribute| diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index af27955554..48e0e5c9f6 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -26,8 +26,6 @@ module ActiveModel raw_value = value end - return if options[:allow_nil] && raw_value.nil? - unless is_number?(raw_value) record.errors.add(attr_name, :not_a_number, filtered_options(raw_value)) return diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index 84ed430d2a..fefab29f15 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -35,6 +35,13 @@ class NumericalityValidationTest < ActiveModel::TestCase valid!(NIL + FLOATS + INTEGERS + BIGDECIMAL + INFINITY) end + def test_validates_numericality_of_with_blank_allowed + Topic.validates_numericality_of :approved, allow_blank: true + + invalid!(JUNK) + valid!(NIL + BLANK + FLOATS + INTEGERS + BIGDECIMAL + INFINITY) + end + def test_validates_numericality_of_with_integer_only Topic.validates_numericality_of :approved, only_integer: true diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cb11cdefd9..aef4761be4 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -541,15 +541,15 @@ module ActiveRecord private - # Under Ruby 1.9, Array#flatten will call #to_ary (recursively) on each of the elements - # of the array, and then rescues from the possible NoMethodError. If those elements are - # ActiveRecord::Base's, then this triggers the various method_missing's that we have, - # which significantly impacts upon performance. - # - # So we can avoid the method_missing hit by explicitly defining #to_ary as nil here. - # - # See also http://tenderlovemaking.com/2011/06/28/til-its-ok-to-return-nil-from-to_ary.html - def to_ary # :nodoc: + # +Array#flatten+ will call +#to_ary+ (recursively) on each of the elements of + # the array, and then rescues from the possible +NoMethodError+. If those elements are + # +ActiveRecord::Base+'s, then this triggers the various +method_missing+'s that we have, + # which significantly impacts upon performance. + # + # So we can avoid the +method_missing+ hit by explicitly defining +#to_ary+ as +nil+ here. + # + # See also http://tenderlovemaking.com/2011/06/28/til-its-ok-to-return-nil-from-to_ary.html + def to_ary nil end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3983065d7a..d7de1032b6 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -29,9 +29,7 @@ module ActiveRecord end def initialize_copy(other) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - @values = Hash[@values] + @values = @values.dup reset end @@ -504,15 +502,10 @@ module ActiveRecord # Post.limit(100).delete_all # # => ActiveRecord::ActiveRecordError: delete_all doesn't support limit def delete_all(conditions = nil) - invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select { |method| - if MULTI_VALUE_METHODS.include?(method) - send("#{method}_values").any? - elsif SINGLE_VALUE_METHODS.include?(method) - send("#{method}_value") - elsif CLAUSE_METHODS.include?(method) - send("#{method}_clause").any? - end - } + invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select do |method| + value = get_value(method) + SINGLE_VALUE_METHODS.include?(method) ? value : value.any? + end if invalid_methods.any? raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}") end @@ -666,7 +659,7 @@ module ActiveRecord end def values - Hash[@values] + @values.dup end def inspect diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index df43b5c64b..5dac00724a 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -151,15 +151,11 @@ module ActiveRecord end end - CLAUSE_METHOD_NAMES = CLAUSE_METHODS.map do |name| - ["#{name}_clause", "#{name}_clause="] - end - def merge_clauses - CLAUSE_METHOD_NAMES.each do |(reader, writer)| - clause = relation.send(reader) - other_clause = other.send(reader) - relation.send(writer, clause.merge(other_clause)) + CLAUSE_METHODS.each do |method| + clause = relation.get_value(method) + other_clause = other.get_value(method) + relation.set_value(method, clause.merge(other_clause)) end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 9a1ac207f2..bd41653df0 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -56,49 +56,25 @@ module ActiveRecord end FROZEN_EMPTY_ARRAY = [].freeze - Relation::MULTI_VALUE_METHODS.each do |name| - class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_values - @values[:#{name}] || FROZEN_EMPTY_ARRAY - end - - def #{name}_values=(values) - assert_mutability! - @values[:#{name}] = values - end - CODE - end + FROZEN_EMPTY_HASH = {}.freeze - (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |name| + Relation::VALUE_METHODS.each do |name| + method_name = case name + when *Relation::MULTI_VALUE_METHODS then "#{name}_values" + when *Relation::SINGLE_VALUE_METHODS then "#{name}_value" + when *Relation::CLAUSE_METHODS then "#{name}_clause" + end class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_value # def readonly_value - @values[:#{name}] # @values[:readonly] + def #{method_name} # def includes_values + get_value(#{name.inspect}) # get_value(:includes) end # end - CODE - end - Relation::SINGLE_VALUE_METHODS.each do |name| - class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_value=(value) # def readonly_value=(value) - assert_mutability! # assert_mutability! - @values[:#{name}] = value # @values[:readonly] = value + def #{method_name}=(value) # def includes_values=(value) + set_value(#{name.inspect}, value) # set_value(:includes, value) end # end CODE end - Relation::CLAUSE_METHODS.each do |name| - class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_clause # def where_clause - @values[:#{name}] || new_#{name}_clause # @values[:where] || new_where_clause - end # end - # - def #{name}_clause=(value) # def where_clause=(value) - assert_mutability! # assert_mutability! - @values[:#{name}] = value # @values[:where] = value - end # end - CODE - end - def bound_attributes if limit_value && !string_containing_comma?(limit_value) limit_bind = Attribute.with_cast_value( @@ -124,11 +100,6 @@ module ActiveRecord ) end - FROZEN_EMPTY_HASH = {}.freeze - def create_with_value # :nodoc: - @values[:create_with] || FROZEN_EMPTY_HASH - end - alias extensions extending_values # Specify relationships to be included in the result set. For @@ -418,7 +389,10 @@ module ActiveRecord args.each do |scope| case scope when Symbol - symbol_unscoping(scope) + if !VALID_UNSCOPING_VALUES.include?(scope) + raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." + end + set_value(scope, nil) when Hash scope.each do |key, target_value| if key != :where @@ -950,6 +924,17 @@ module ActiveRecord @arel ||= build_arel end + # Returns a relation value with a given name + def get_value(name) # :nodoc: + @values[name] || default_value_for(name) + end + + # Sets the relation value with the given name + def set_value(name, value) # :nodoc: + assert_mutability! + @values[name] = value + end + private def assert_mutability! @@ -986,29 +971,6 @@ module ActiveRecord arel end - def symbol_unscoping(scope) - if !VALID_UNSCOPING_VALUES.include?(scope) - raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." - end - - clause_method = Relation::CLAUSE_METHODS.include?(scope) - multi_val_method = Relation::MULTI_VALUE_METHODS.include?(scope) - if clause_method - unscope_code = "#{scope}_clause=" - else - unscope_code = "#{scope}_value#{'s' if multi_val_method}=" - end - - case scope - when :order - result = [] - else - result = [] if multi_val_method - end - - send(unscope_code, result) - end - def build_from opts = from_clause.value name = from_clause.name @@ -1210,28 +1172,39 @@ module ActiveRecord end end + STRUCTURAL_OR_METHODS = Relation::VALUE_METHODS - [:extending, :where, :having] 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 - Relation::WhereClause.empty + STRUCTURAL_OR_METHODS.reject do |method| + get_value(method) == other.get_value(method) + end end - alias new_having_clause new_where_clause def where_clause_factory @where_clause_factory ||= Relation::WhereClauseFactory.new(klass, predicate_builder) end alias having_clause_factory where_clause_factory - def new_from_clause - Relation::FromClause.empty - end - def string_containing_comma?(value) ::String === value && value.include?(",") end + + def default_value_for(name) + case name + when :create_with + FROZEN_EMPTY_HASH + when :readonly + false + when :where, :having + Relation::WhereClause.empty + when :from + Relation::FromClause.empty + when *Relation::MULTI_VALUE_METHODS + FROZEN_EMPTY_ARRAY + when *Relation::SINGLE_VALUE_METHODS + nil + else + raise ArgumentError, "unknown relation value #{name.inspect}" + end + end end end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index b65d5b56f1..ab2d64e903 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -145,22 +145,9 @@ HEADER # find all migration keys used in this table keys = @connection.migration_keys - # figure out the lengths for each column based on above keys - lengths = [0] * keys.length - - # the string we're going to sprintf our values against, with standardized column widths - format_string = ["%s"] * keys.length - - # add column type definition to our format string - format_string.unshift " t.%s " - - format_string *= "" - column_specs.each do |colspec| - values = keys.zip(lengths).map { |key, len| colspec.key?(key) ? colspec[key] + ", " : " " * len } - values.unshift colspec[:type] - tbl.print((format_string % values).gsub(/,\s*$/, "")) - tbl.puts + values = keys.map { |key| colspec[key] }.compact + tbl.puts " t.#{colspec[:type]} #{values.join(", ")}" end indexes_in_create(table, tbl) diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb index 0ca880e635..a2cb3ea1be 100644 --- a/activerecord/lib/active_record/table_metadata.rb +++ b/activerecord/lib/active_record/table_metadata.rb @@ -10,9 +10,7 @@ module ActiveRecord end def resolve_column_aliases(hash) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - new_hash = Hash[hash] + new_hash = hash.dup hash.each do |key, _| if (key.is_a?(Symbol)) && klass.attribute_alias?(key) new_hash[klass.attribute_alias(key)] = new_hash.delete(key) diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index d99e8a1dc6..7f7faca70d 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -212,6 +212,26 @@ class QueryCacheTest < ActiveRecord::TestCase end end + def test_query_cached_even_when_types_are_reset + Task.cache do + # Warm the cache + Task.find(1) + + Task.connection.type_map.clear + + # Preload the type cache again (so we don't have those queries issued during our assertions) + Task.connection.send(:initialize_type_map, Task.connection.type_map) + + # Clear places where type information is cached + Task.reset_column_information + Task.initialize_find_by_cache + + assert_queries(0) do + Task.find(1) + end + end + end + private def middleware(&app) executor = Class.new(ActiveSupport::Executor) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 103075d09c..32b8781a6b 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -40,9 +40,10 @@ module ActiveRecord def test_initialize_single_values relation = Relation.new(FakeKlass, :b, nil) - (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| + (Relation::SINGLE_VALUE_METHODS - [:create_with, :readonly]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end + assert_equal false, relation.readonly_value value = relation.create_with_value assert_equal({}, value) assert_predicate value, :frozen? diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index f01ff23bcf..f5f4ba61b7 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -256,7 +256,7 @@ class Module # end # end # - # The target can be anything callable withing the object. E.g. instance + # The target can be anything callable within the object. E.g. instance # variables, methods, constants ant the likes. def delegate_missing_to(target) target = target.to_s diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index a8e98b704a..1c599b8851 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -65,16 +65,5 @@ module ActiveSupport alias :assert_not_predicate :refute_predicate alias :assert_not_respond_to :refute_respond_to alias :assert_not_same :refute_same - - # Assertion that the block should not raise an exception. - # - # Passes if evaluated code in the yielded block raises no exception. - # - # assert_nothing_raised do - # perform_service(param: 'no_exception') - # end - def assert_nothing_raised - yield - end end end diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index 7770aa8006..8c9ea2c0e8 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -19,6 +19,17 @@ module ActiveSupport assert !object, message end + # Assertion that the block should not raise an exception. + # + # Passes if evaluated code in the yielded block raises no exception. + # + # assert_nothing_raised do + # perform_service(param: 'no_exception') + # end + def assert_nothing_raised + yield + end + # Test numeric difference between the return value of an expression as a # result of what is evaluated in the yielded block. # @@ -136,7 +147,7 @@ module ActiveSupport retval = yield unless from == UNTRACKED - error = "#{expression.inspect} isn't #{from}" + error = "#{expression.inspect} isn't #{from.inspect}" error = "#{message}.\n#{error}" if message assert from === before, error end diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index d3d2ed8d7a..dfbb8aa41d 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -138,6 +138,15 @@ class AssertDifferenceTest < ActiveSupport::TestCase end end + def test_assert_changes_with_from_option_with_nil + error = assert_raises Minitest::Assertion do + assert_changes "@object.num", from: nil do + @object.increment + end + end + assert_equal "\"@object.num\" isn't nil", error.message + end + def test_assert_changes_with_to_option assert_changes "@object.num", to: 1 do @object.increment diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index 45f396bbd6..9c5ffb1d94 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -125,7 +125,7 @@ store_listing.price_in_cents # => 10 StoreListing.new.my_string # => "new default" StoreListing.new.my_default_proc # => 2015-05-30 11:04:48 -0600 model = StoreListing.new(field_without_db_column: ["1", "2", "3"]) -model.attributes #=> {field_without_db_column: [1, 2, 3]} +model.attributes # => {field_without_db_column: [1, 2, 3]} ``` **Creating Custom Types:** @@ -171,7 +171,7 @@ instead of waiting for the suite to complete. - Complete exception backtrace output using `-b` option. - Integration with `Minitest` to allow options like `-s` for test seed data, `-n` for running specific test by name, `-v` for better verbose output and so forth. -- Colored test output +- Colored test output. Railties -------- @@ -212,7 +212,7 @@ Please refer to the [Changelog][railties] for detailed changes. ([Pull Request](https://github.com/rails/rails/pull/22173)) * Deprecated the tasks in the `rails` task namespace in favor of the `app` namespace. - (e.g. `rails:update` and `rails:template` tasks is renamed to `app:update` and `app:template`.) + (e.g. `rails:update` and `rails:template` tasks are renamed to `app:update` and `app:template`.) ([Pull Request](https://github.com/rails/rails/pull/23439)) ### Notable changes @@ -432,11 +432,6 @@ Please refer to the [Changelog][action-pack] for detailed changes. * Added request encoding and response parsing to integration tests. ([Pull Request](https://github.com/rails/rails/pull/21671)) -* Update default rendering policies when the controller action did - not explicitly indicate a response. - ([Pull Request](https://github.com/rails/rails/pull/23827)) - - * Add `ActionController#helpers` to get access to the view context at the controller level. ([Pull Request](https://github.com/rails/rails/pull/24866)) @@ -703,9 +698,6 @@ Please refer to the [Changelog][active-record] for detailed changes. operator to combine WHERE or HAVING clauses. ([commit](https://github.com/rails/rails/commit/b0b37942d729b6bdcd2e3178eda7fa1de203b3d0)) -* Added `:time` option added for `#touch`. - ([Pull Request](https://github.com/rails/rails/pull/18956)) - * Added `ActiveRecord::Base.suppress` to prevent the receiver from being saved during the given block. ([Pull Request](https://github.com/rails/rails/pull/18910)) @@ -962,8 +954,10 @@ Please refer to the [Changelog][active-support] for detailed changes. `ActiveSupport::Cache::MemCachedStore#escape_key`, and `ActiveSupport::Cache::FileStore#key_file_path`. Use `normalize_key` instead. + ([Pull Request](https://github.com/rails/rails/pull/22215), + [commit](https://github.com/rails/rails/commit/a8f773b0)) - Deprecated `ActiveSupport::Cache::LocaleCache#set_cache_value` in favor of `write_cache_value`. +* Deprecated `ActiveSupport::Cache::LocaleCache#set_cache_value` in favor of `write_cache_value`. ([Pull Request](https://github.com/rails/rails/pull/22215)) * Deprecated passing arguments to `assert_nothing_raised`. diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 7239105b29..c938edd8f7 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1217,21 +1217,25 @@ 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. It works like this: +You can configure your own code through the Rails configuration object with +custom configuration under either the `config.x` namespace, or `config` directly. +The key difference between these two is that you should be using `config.x` if you +are defining _nested_ configuration (ex: `config.x.nested.nested.hi`), and just +`config` for _single level_ configuration (ex: `config.hello`). ```ruby - config.payment_processing.schedule = :daily - config.payment_processing.retries = 3 + config.x.payment_processing.schedule = :daily + config.x.payment_processing.retries = 3 config.super_debugger = true ``` These configuration points are then available through the configuration object: ```ruby - Rails.configuration.payment_processing.schedule # => :daily - Rails.configuration.payment_processing.retries # => 3 - Rails.configuration.super_debugger # => true - Rails.configuration.super_debugger.not_set # => nil + Rails.configuration.x.payment_processing.schedule # => :daily + Rails.configuration.x.payment_processing.retries # => 3 + Rails.configuration.x.payment_processing.not_set # => nil + Rails.configuration.super_debugger # => true ``` You can also use `Rails::Application.config_for` to load whole configuration files: diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index ba8d085f79..332d8fc852 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -677,7 +677,7 @@ $ git format-patch master --stdout > ~/my_changes.patch Switch over to the target branch and apply your changes: ```bash -$ git checkout -b my_backport_branch 3-2-stable +$ git checkout -b my_backport_branch 4-2-stable $ git apply ~/my_changes.patch ``` diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 627e23422d..2925fb4b58 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -100,9 +100,9 @@ url: configuring.html description: This guide covers the basic configuration settings for a Rails application. - - name: Rails Command Line Tools and Rake Tasks + name: The Rails Command Line url: command_line.html - description: This guide covers the command line tools and rake tasks provided by Rails. + description: This guide covers the command line tools provided by Rails. - name: Asset Pipeline url: asset_pipeline.html diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 54421a328c..c4a8eacc57 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -182,6 +182,7 @@ of the files and folders that Rails created by default: |test/|Unit tests, fixtures, and other test apparatus. These are covered in [Testing Rails Applications](testing.html).| |tmp/|Temporary files (like cache and pid files).| |vendor/|A place for all third-party code. In a typical Rails application this includes vendored gems.| +|.gitignore|This file tells git which files (or patterns) it should ignore. See [Github - Ignoring files](https://help.github.com/articles/ignoring-files) for more info about ignoring files. Hello, Rails! ------------- diff --git a/guides/source/testing.md b/guides/source/testing.md index 4ca3236ec1..4de32d9d77 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -312,7 +312,6 @@ specify to make your test failure messages clearer. | `assert_not_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.| | `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.| | `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.| -| `assert_nothing_raised { block }` | Ensures that the given block doesn't raise any exceptions.| | `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.| | `assert_not_instance_of( class, obj, [msg] )` | Ensures that `obj` is not an instance of `class`.| | `assert_kind_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class` or is descending from it.| @@ -343,6 +342,7 @@ Rails adds some custom assertions of its own to the `minitest` framework: | --------------------------------------------------------------------------------- | ------- | | [`assert_difference(expressions, difference = 1, message = nil) {...}`](http://api.rubyonrails.org/classes/ActiveSupport/Testing/Assertions.html#method-i-assert_difference) | Test numeric difference between the return value of an expression as a result of what is evaluated in the yielded block.| | [`assert_no_difference(expressions, message = nil, &block)`](http://api.rubyonrails.org/classes/ActiveSupport/Testing/Assertions.html#method-i-assert_no_difference) | Asserts that the numeric result of evaluating an expression is not changed before and after invoking the passed in block.| +| [`assert_nothing_raised { block }`](http://api.rubyonrails.org/classes/ActiveSupport/TestCase.html#method-i-assert_nothing_raised) | Ensures that the given block doesn't raise any exceptions.| | [`assert_recognizes(expected_options, path, extras={}, message=nil)`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html#method-i-assert_recognizes) | Asserts that the routing of the given path was handled correctly and that the parsed options (given in the expected_options hash) match path. Basically, it asserts that Rails recognizes the route given by expected_options.| | [`assert_generates(expected_path, options, defaults={}, extras = {}, message=nil)`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html#method-i-assert_generates) | Asserts that the provided options can be used to generate the provided path. This is the inverse of assert_recognizes. The extras parameter is used to tell the request the names and values of additional request parameters that would be in a query string. The message parameter allows you to specify a custom error message for assertion failures.| | [`assert_response(type, message = nil)`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/ResponseAssertions.html#method-i-assert_response) | Asserts that the response comes with a specific status code. You can specify `:success` to indicate 200-299, `:redirect` to indicate 300-399, `:missing` to indicate 404, or `:error` to match the 500-599 range. You can also pass an explicit status number or its symbolic equivalent. For more information, see [full list of status codes](http://rubydoc.info/github/rack/rack/master/Rack/Utils#HTTP_STATUS_CODES-constant) and how their [mapping](http://rubydoc.info/github/rack/rack/master/Rack/Utils#SYMBOL_TO_STATUS_CODE-constant) works.| |