diff options
185 files changed, 2271 insertions, 1283 deletions
diff --git a/.travis.yml b/.travis.yml index 2237ca85d9..605d1ff247 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,14 +23,10 @@ env: rvm: - 2.2.2 - ruby-head - - rbx-2 - - jruby-head matrix: allow_failures: - env: "GEM=ar:mysql" - rvm: ruby-head - - rvm: rbx-2 - - rvm: jruby-head fast_finish: true notifications: email: false @@ -5,6 +5,9 @@ gemspec # We need a newish Rake since Active Job sets its test tasks' descriptions. gem 'rake', '>= 10.3' +# Active Job depends on the URI::GID::MissingModelIDError, which isn't released yet. +gem 'globalid', github: 'rails/globalid' + # This needs to be with require false as it is # loaded after loading the test library to # ensure correct loading order diff --git a/Gemfile.lock b/Gemfile.lock index 2140881170..897bdeff83 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,6 +26,13 @@ GIT arel (7.0.0.alpha) GIT + remote: git://github.com/rails/globalid.git + revision: 4df66fb9e9f0c832d29119aa8bc30be55a614b71 + specs: + globalid (0.3.5) + activesupport (>= 4.1.0) + +GIT remote: git://github.com/rails/jquery-rails.git revision: 272abdd319bb3381b23182b928b25320590096b0 branch: master @@ -78,6 +85,7 @@ PATH activesupport (= 5.0.0.alpha) arel (= 7.0.0.alpha) activesupport (5.0.0.alpha) + concurrent-ruby (~> 0.9.0) i18n (~> 0.7) json (~> 1.7, >= 1.7.7) minitest (~> 5.1) @@ -128,6 +136,7 @@ GEM execjs coffee-script-source (1.9.0) columnize (0.9.0) + concurrent-ruby (0.9.0) connection_pool (2.1.1) dalli (2.7.2) dante (0.1.5) @@ -138,8 +147,6 @@ GEM delayed_job (>= 3.0, < 4.1) erubis (2.7.0) execjs (2.3.0) - globalid (0.3.3) - activesupport (>= 4.1.0) hitimes (1.2.2) hitimes (1.2.2-x86-mingw32) i18n (0.7.0) @@ -272,6 +279,7 @@ DEPENDENCIES dalli (>= 2.2.1) delayed_job delayed_job_active_record + globalid! jquery-rails! json kindlerb (= 0.1.1) @@ -308,4 +316,4 @@ DEPENDENCIES w3c_validators BUNDLED WITH - 1.10.4 + 1.10.5 diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index cb5e7516fb..4b081591c0 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,32 @@ +* `ActionController::Parameters` no longer inherits from + `HashWithIndifferentAccess` + + Inheriting from `HashWithIndifferentAccess` allowed users to call any + enumerable methods on `Parameters` object, resulting in a risk of losing the + `permitted?` status or even getting back a pure `Hash` object instead of + a `Parameters` object with proper sanitization. + + By not inheriting from `HashWithIndifferentAccess`, we are able to make + sure that all methods that are defined in `Parameters` object will return + a proper `Parameters` object with a correct `permitted?` flag. + + *Prem Sichanugrist* + +* Replaced `ActiveSupport::Concurrency::Latch` with `Concurrent::CountDownLatch` + from the concurrent-ruby gem. + + *Jerry D'Antonio* + +* Add ability to filter parameters based on parent keys. + + # matches {credit_card: {code: "xxxx"}} + # doesn't match {file: { code: "xxxx"}} + config.filter_parameters += [ "credit_card.code" ] + + See #13897. + + *Guillaume Malette* + * Deprecate passing first parameter as `Hash` and default status code for `head` method. *Mehmet Emin İNAÇ* @@ -154,7 +183,8 @@ *arthurnn* * `ActionController#translate` supports symbols as shortcuts. - When shortcut is given it also lookups without action name. + When a shortcut is given it also performs the lookup without the action + name. *Max Melentiev* diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index de85e0c1a7..a4e4992cfe 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -8,7 +8,7 @@ module ActionController # # You can read more about each approach by clicking the modules below. # - # Note: To turn off all caching, set + # Note: To turn off all caching provided by Action Controller, set # config.action_controller.perform_caching = false # # == \Caching stores diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 1abd8d3a33..e6d7f958bb 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -85,6 +85,10 @@ module ActionController #:nodoc: @to_path = path end + def body + File.binread(to_path) + end + # Stream the file's contents if Rack::Sendfile isn't present. def each File.open(to_path, 'rb') do |file| @@ -126,7 +130,7 @@ module ActionController #:nodoc: # See +send_file+ for more information on HTTP Content-* headers and caching. def send_data(data, options = {}) #:doc: send_file_headers! options - render options.slice(:status, :content_type).merge(:text => data) + render options.slice(:status, :content_type).merge(body: data) end private diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 39bed955a4..032275ac64 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -502,7 +502,7 @@ module ActionController def authentication_request(controller, realm, message = nil) message ||= "HTTP Token: Access denied.\n" controller.headers["WWW-Authenticate"] = %(Token realm="#{realm.tr('"'.freeze, "".freeze)}") - controller.__send__ :render, :text => message, :status => :unauthorized + controller.__send__ :render, plain: message, status: :unauthorized end end end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index fab1be3459..1db68db20f 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -156,16 +156,16 @@ module ActionController #:nodoc: # It works for both inline: # # respond_to do |format| - # format.html.any { render text: "any" } - # format.html.phone { render text: "phone" } + # format.html.any { render html: "any" } + # format.html.phone { render html: "phone" } # end # # and block syntax: # # respond_to do |format| # format.html do |variant| - # variant.any(:tablet, :phablet){ render text: "any" } - # variant.phone { render text: "phone" } + # variant.any(:tablet, :phablet){ render html: "any" } + # variant.phone { render html: "phone" } # end # end # diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 45d3962494..cb74c4f0d4 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -94,7 +94,7 @@ module ActionController # This method is the opposite of add method. # - # Usage: + # To remove a csv renderer: # # ActionController::Renderers.remove(:csv) def self.remove(key) diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index b9ae8dd5ea..a3b0645dc0 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -1,3 +1,6 @@ +require 'active_support/deprecation' +require 'active_support/core_ext/string/filters' + module ActionController module Rendering extend ActiveSupport::Concern @@ -74,6 +77,17 @@ module ActionController def _normalize_options(options) #:nodoc: _normalize_text(options) + if options[:text] + ActiveSupport::Deprecation.warn <<-WARNING.squish + `render :text` is deprecated because it does not actually render a + `text/plain` response. Switch to `render plain: 'plain text'` to + render as `text/plain`, `render html: '<strong>HTML</strong>'` to + render as `text/html`, or `render body: 'raw'` to match the deprecated + behavior and render with the default Content-Type, which is + `text/plain`. + WARNING + end + if options[:html] options[:html] = ERB::Util.html_escape(options[:html]) end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 09867e2407..06d625e4d5 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -104,10 +104,12 @@ module ActionController # params = ActionController::Parameters.new(key: 'value') # params[:key] # => "value" # params["key"] # => "value" - class Parameters < ActiveSupport::HashWithIndifferentAccess + class Parameters cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false + delegate :keys, :key?, :has_key?, :empty?, :inspect, to: :@parameters + # By default, never raise an UnpermittedParameters exception if these # params are present. The default includes both 'controller' and 'action' # because they are added by Rails and should be of no concern. One way @@ -144,11 +146,22 @@ module ActionController # params = ActionController::Parameters.new(name: 'Francesco') # params.permitted? # => true # Person.new(params) # => #<Person id: nil, name: "Francesco"> - def initialize(attributes = nil) - super(attributes) + def initialize(parameters = {}) + @parameters = parameters.with_indifferent_access @permitted = self.class.permit_all_parameters end + # Returns true if another +Parameters+ object contains the same content and + # permitted flag, or other Hash-like object contains the same content. This + # override is in place so you can perform a comparison with `Hash`. + def ==(other_hash) + if other_hash.respond_to?(:permitted?) + super + else + @parameters == other_hash + end + end + # Returns a safe +Hash+ representation of this parameter with all # unpermitted keys removed. # @@ -162,7 +175,7 @@ module ActionController # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} def to_h if permitted? - to_hash + @parameters.to_h else slice(*self.class.always_permitted_parameters).permit!.to_h end @@ -170,20 +183,17 @@ module ActionController # Returns an unsafe, unfiltered +Hash+ representation of this parameter. def to_unsafe_h - to_hash + @parameters.to_h end alias_method :to_unsafe_hash, :to_unsafe_h # Convert all hashes in values into parameters, then yield each pair like # the same way as <tt>Hash#each_pair</tt> def each_pair(&block) - super do |key, value| - convert_hashes_to_parameters(key, value) + @parameters.each_pair do |key, value| + yield key, convert_hashes_to_parameters(key, value) end - - super end - alias_method :each, :each_pair # Attribute that keeps track of converted arrays, if any, to avoid double @@ -347,7 +357,13 @@ module ActionController # params[:person] # => {"name"=>"Francesco"} # params[:none] # => nil def [](key) - convert_hashes_to_parameters(key, super) + convert_hashes_to_parameters(key, @parameters[key]) + end + + # Assigns a value to a given +key+. The given key may still get filtered out + # when +permit+ is called. + def []=(key, value) + @parameters[key] = value end # Returns a parameter for the given +key+. If the +key+ @@ -361,8 +377,12 @@ 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) - convert_hashes_to_parameters(key, super, false) + def fetch(key, *args, &block) + convert_hashes_to_parameters( + key, + @parameters.fetch(key, *args, &block), + false + ) rescue KeyError raise ActionController::ParameterMissing.new(key) end @@ -375,7 +395,24 @@ module ActionController # params.slice(:a, :b) # => {"a"=>1, "b"=>2} # params.slice(:d) # => {} def slice(*keys) - new_instance_with_inherited_permitted_status(super) + new_instance_with_inherited_permitted_status(@parameters.slice(*keys)) + end + + # Returns current <tt>ActionController::Parameters</tt> instance which + # contains only the given +keys+. + def slice!(*keys) + @parameters.slice!(*keys) + self + end + + # Returns a new <tt>ActionController::Parameters</tt> instance that + # filters out the given +keys+. + # + # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) + # params.except(:a, :b) # => {"c"=>3} + # params.except(:d) # => {"a"=>1,"b"=>2,"c"=>3} + def except(*keys) + new_instance_with_inherited_permitted_status(@parameters.except(*keys)) end # Removes and returns the key/value pairs matching the given keys. @@ -384,7 +421,7 @@ module ActionController # params.extract!(:a, :b) # => {"a"=>1, "b"=>2} # params # => {"c"=>3} def extract!(*keys) - new_instance_with_inherited_permitted_status(super) + new_instance_with_inherited_permitted_status(@parameters.extract!(*keys)) end # Returns a new <tt>ActionController::Parameters</tt> with the results of @@ -393,36 +430,80 @@ module ActionController # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) # params.transform_values { |x| x * 2 } # # => {"a"=>2, "b"=>4, "c"=>6} - def transform_values - if block_given? - new_instance_with_inherited_permitted_status(super) + def transform_values(&block) + if block + new_instance_with_inherited_permitted_status( + @parameters.transform_values(&block) + ) else - super + @parameters.transform_values end end - # This method is here only to make sure that the returned object has the - # correct +permitted+ status. It should not matter since the parent of - # this object is +HashWithIndifferentAccess+ - def transform_keys # :nodoc: - if block_given? - new_instance_with_inherited_permitted_status(super) + # Performs values transformation and returns the altered + # <tt>ActionController::Parameters</tt> instance. + def transform_values!(&block) + @parameters.transform_values!(&block) + self + end + + # Returns a new <tt>ActionController::Parameters</tt> instance with the + # results of running +block+ once for every key. The values are unchanged. + def transform_keys(&block) + if block + new_instance_with_inherited_permitted_status( + @parameters.transform_keys(&block) + ) else - super + @parameters.transform_keys end end + # Performs keys transfomration and returns the altered + # <tt>ActionController::Parameters</tt> instance. + def transform_keys!(&block) + @parameters.transform_keys!(&block) + self + end + # Deletes and returns a key-value pair from +Parameters+ whose key is equal # 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) - convert_hashes_to_parameters(key, super, false) + convert_hashes_to_parameters(key, @parameters.delete(key), false) + end + + # Returns a new instance of <tt>ActionController::Parameters</tt> with only + # items that the block evaluates to true. + def select(&block) + new_instance_with_inherited_permitted_status(@parameters.select(&block)) end # Equivalent to Hash#keep_if, but returns nil if no changes were made. def select!(&block) - convert_value_to_parameters(super) + @parameters.select!(&block) + self + end + alias_method :keep_if, :select! + + # Returns a new instance of <tt>ActionController::Parameters</tt> with items + # that the block evaluates to true removed. + def reject(&block) + new_instance_with_inherited_permitted_status(@parameters.reject(&block)) + end + + # Removes items that the block evaluates to true and returns self. + def reject!(&block) + @parameters.reject!(&block) + self + end + alias_method :delete_if, :reject! + + # Return values that were assigned to the given +keys+. Note that all the + # +Hash+ objects will be converted to <tt>ActionController::Parameters</tt>. + def values_at(*keys) + convert_value_to_parameters(@parameters.values_at(*keys)) end # Returns an exact copy of the <tt>ActionController::Parameters</tt> @@ -439,11 +520,30 @@ module ActionController end end + # Returns a new <tt>ActionController::Parameters</tt> with all keys from + # +other_hash+ merges into current hash. + def merge(other_hash) + new_instance_with_inherited_permitted_status( + @parameters.merge(other_hash) + ) + end + + # This is required by ActiveModel attribute assignment, so that user can + # pass +Parameters+ to a mass assignment methods in a model. It should not + # matter as we are using +HashWithIndifferentAccess+ internally. + def stringify_keys # :nodoc: + dup + end + protected def permitted=(new_permitted) @permitted = new_permitted end + def fields_for_style? + @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } + end + private def new_instance_with_inherited_permitted_status(hash) self.class.new(hash).tap do |new_instance| @@ -453,7 +553,7 @@ module ActionController def convert_hashes_to_parameters(key, value, assign_if_converted=true) converted = convert_value_to_parameters(value) - self[key] = converted if assign_if_converted && !converted.equal?(value) + @parameters[key] = converted if assign_if_converted && !converted.equal?(value) converted end @@ -470,21 +570,20 @@ module ActionController end def each_element(object) - if object.is_a?(Array) - object.map { |el| yield el }.compact - elsif fields_for_style?(object) - hash = object.class.new - object.each { |k,v| hash[k] = yield v } - hash - else - yield object + case object + when Array + object.grep(Parameters).map { |el| yield el }.compact + when Parameters + if object.fields_for_style? + hash = object.class.new + object.each { |k,v| hash[k] = yield v } + hash + else + yield object + end end end - def fields_for_style?(object) - object.is_a?(Hash) && object.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } - end - def unpermitted_parameters!(params) unpermitted_keys = unpermitted_keys(params) if unpermitted_keys.any? @@ -546,14 +645,8 @@ module ActionController end def array_of_permitted_scalars?(value) - if value.is_a?(Array) - value.all? {|element| permitted_scalar?(element)} - end - end - - def array_of_permitted_scalars_filter(params, key) - if has_key?(key) && array_of_permitted_scalars?(self[key]) - params[key] = self[key] + if value.is_a?(Array) && value.all? {|element| permitted_scalar?(element)} + yield value end end @@ -564,17 +657,17 @@ module ActionController # Slicing filters out non-declared keys. slice(*filter.keys).each do |key, value| next unless value + next unless has_key? key if filter[key] == EMPTY_ARRAY # Declaration { comment_ids: [] }. - array_of_permitted_scalars_filter(params, key) + array_of_permitted_scalars?(self[key]) do |val| + params[key] = val + end else # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| - if element.is_a?(Hash) - element = self.class.new(element) unless element.respond_to?(:permit) - element.permit(*Array.wrap(filter[key])) - end + element.permit(*Array.wrap(filter[key])) end end end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 96f161fb09..f922e134f7 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -10,27 +10,45 @@ module ActionController DEFAULT_ENV = ActionDispatch::TestRequest::DEFAULT_ENV.dup DEFAULT_ENV.delete 'PATH_INFO' - def initialize(env = {}) - super + def self.new_session + TestSession.new + end + + # Create a new test request with default `env` values + def self.create + env = {} + env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application + env["rack.request.cookie_hash"] = {}.with_indifferent_access + new(default_env.merge(env), new_session) + end + + def self.default_env + DEFAULT_ENV + end + private_class_method :default_env + + def initialize(env, session) + super(env) - self.session = TestSession.new + self.session = session self.session_options = TestSession::DEFAULT_OPTIONS end + def query_string=(string) + @env[Rack::QUERY_STRING] = string + end + + def request_parameters=(params) + @env["action_dispatch.request.request_parameters"] = params + end + def assign_parameters(routes, controller_path, action, parameters = {}) parameters = parameters.symbolize_keys - extra_keys = routes.extra_keys(parameters.merge(:controller => controller_path, :action => action)) - non_path_parameters = get? ? query_parameters : request_parameters + generated_path, extra_keys = routes.generate_extras(parameters.merge(:controller => controller_path, :action => action)) + non_path_parameters = {} + path_parameters = {} parameters.each do |key, value| - if value.is_a?(Array) && (value.frozen? || value.any?(&:frozen?)) - value = value.map{ |v| v.duplicable? ? v.dup : v } - elsif value.is_a?(Hash) && (value.frozen? || value.any?{ |k,v| v.frozen? }) - value = Hash[value.map{ |k,v| [k, v.duplicable? ? v.dup : v] }] - elsif value.frozen? && value.duplicable? - value = value.dup - end - if extra_keys.include?(key) || key == :action || key == :controller non_path_parameters[key] = value else @@ -44,69 +62,83 @@ module ActionController end end - path_parameters[:controller] = controller_path - path_parameters[:action] = action + if get? + if self.query_string.blank? + self.query_string = non_path_parameters.to_query + end + else + if ENCODER.should_multipart?(non_path_parameters) + @env['CONTENT_TYPE'] = ENCODER.content_type + data = ENCODER.build_multipart non_path_parameters + else + @env['CONTENT_TYPE'] ||= 'application/x-www-form-urlencoded' + + # FIXME: setting `request_parametes` is normally handled by the + # params parser middleware, and we should remove this roundtripping + # when we switch to caling `call` on the controller + + case content_mime_type.ref + when :json + data = ActiveSupport::JSON.encode(non_path_parameters) + params = ActiveSupport::JSON.decode(data).with_indifferent_access + self.request_parameters = params + when :xml + data = non_path_parameters.to_xml + params = Hash.from_xml(data)['hash'] + self.request_parameters = params + when :url_encoded_form + data = non_path_parameters.to_query + else + raise "Unknown Content-Type: #{content_type}" + end + end - # Clear the combined params hash in case it was already referenced. - @env.delete("action_dispatch.request.parameters") + @env['CONTENT_LENGTH'] = data.length.to_s + @env['rack.input'] = StringIO.new(data) + end - # Clear the filter cache variables so they're not stale - @filtered_parameters = @filtered_env = @filtered_path = nil + @env["PATH_INFO"] ||= generated_path + path_parameters[:controller] = controller_path + path_parameters[:action] = action - data = request_parameters.to_query - @env['CONTENT_LENGTH'] = data.length.to_s - @env['rack.input'] = StringIO.new(data) + self.path_parameters = path_parameters end - def recycle! - @formats = nil - @env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ } - @env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ } - @method = @request_method = nil - @fullpath = @ip = @remote_ip = @protocol = nil - @env['action_dispatch.request.query_parameters'] = {} - @set_cookies ||= {} - @set_cookies.update(Hash[cookie_jar.instance_variable_get("@set_cookies").map{ |k,o| [k,o[:value]] }]) - deleted_cookies = cookie_jar.instance_variable_get("@delete_cookies") - @set_cookies.reject!{ |k,v| deleted_cookies.include?(k) } - cookie_jar.update(rack_cookies) - cookie_jar.update(cookies) - cookie_jar.update(@set_cookies) - cookie_jar.recycle! - end + ENCODER = Class.new do + include Rack::Test::Utils + + def should_multipart?(params) + # FIXME: lifted from Rack-Test. We should push this separation upstream + multipart = false + query = lambda { |value| + case value + when Array + value.each(&query) + when Hash + value.values.each(&query) + when Rack::Test::UploadedFile + multipart = true + end + } + params.values.each(&query) + multipart + end - private + public :build_multipart - def default_env - DEFAULT_ENV - end - end - - class TestResponse < ActionDispatch::TestResponse - def recycle! - initialize - end + def content_type + "multipart/form-data; boundary=#{Rack::Test::MULTIPART_BOUNDARY}" + end + end.new end class LiveTestResponse < Live::Response - def recycle! - @body = nil - initialize - end - - def body - @body ||= super - end - # Was the response successful? alias_method :success?, :successful? # Was the URL not found? alias_method :missing?, :not_found? - # Were we redirected? - alias_method :redirect?, :redirection? - # Was there a server-side error? alias_method :error?, :server_error? end @@ -195,7 +227,7 @@ module ActionController # request. You can modify this object before sending the HTTP request. For example, # you might want to set some session properties before sending a GET request. # <b>@response</b>:: - # An ActionController::TestResponse object, representing the response + # An ActionDispatch::TestResponse object, representing the response # of the last HTTP response. In the above example, <tt>@response</tt> becomes valid # after calling +post+. If the various assert methods are not sufficient, then you # may use this object to inspect the HTTP response in detail. @@ -317,7 +349,9 @@ module ActionController # Note that the request method is not verified. The different methods are # available to make the tests more expressive. def get(action, *args) - process_with_kwargs("GET", action, *args) + res = process_with_kwargs("GET", action, *args) + cookies.update res.cookies + res end # Simulate a POST request with the given parameters and set/volley the response. @@ -365,19 +399,6 @@ module ActionController end alias xhr :xml_http_request - def paramify_values(hash_or_array_or_value) - case hash_or_array_or_value - when Hash - Hash[hash_or_array_or_value.map{|key, value| [key, paramify_values(value)] }] - when Array - hash_or_array_or_value.map {|i| paramify_values(i)} - when Rack::Test::UploadedFile, ActionDispatch::Http::UploadedFile - hash_or_array_or_value - else - hash_or_array_or_value.to_param - end - end - # Simulate a HTTP request to +action+ by specifying request method, # parameters and set/volley the response. # @@ -437,10 +458,6 @@ module ActionController parameters ||= {} - # Ensure that numbers and symbols passed as params are converted to - # proper params, as is the case when engaging rack. - parameters = paramify_values(parameters) if html_format?(parameters) - if format.present? parameters[:format] = format end @@ -451,8 +468,13 @@ module ActionController @controller.extend(Testing::Functional) end - @request.recycle! - @response.recycle! + self.cookies.update @request.cookies + @request.env['HTTP_COOKIE'] = cookies.to_header + @request.env['action_dispatch.cookies'] = nil + + @request = TestRequest.new scrub_env!(@request.env), @request.session + @response = build_response @response_klass + @response.request = @request @controller.recycle! @request.env['REQUEST_METHOD'] = http_method @@ -474,14 +496,17 @@ module ActionController @controller.request = @request @controller.response = @response - build_request_uri(controller_class_name, action, parameters) + @request.env["SCRIPT_NAME"] ||= @controller.config.relative_url_root @controller.recycle! @controller.process(action) + @request.env.delete 'HTTP_COOKIE' + if cookies = @request.env['action_dispatch.cookies'] unless @response.committed? cookies.write(@response) + self.cookies.update(cookies.instance_variable_get(:@cookies)) end end @response.prepare! @@ -496,6 +521,7 @@ module ActionController @request.env.delete 'HTTP_X_REQUESTED_WITH' @request.env.delete 'HTTP_ACCEPT' end + @request.query_string = '' @response end @@ -503,11 +529,11 @@ module ActionController def setup_controller_request_and_response @controller = nil unless defined? @controller - response_klass = TestResponse + @response_klass = ActionDispatch::TestResponse if klass = self.class.controller_class if klass < ActionController::Live - response_klass = LiveTestResponse + @response_klass = LiveTestResponse end unless @controller begin @@ -518,8 +544,8 @@ module ActionController end end - @request = build_request - @response = build_response response_klass + @request = TestRequest.create + @response = build_response @response_klass @response.request = @request if @controller @@ -528,10 +554,6 @@ module ActionController end end - def build_request - TestRequest.new - end - def build_response(klass) klass.new end @@ -545,6 +567,14 @@ module ActionController private + def scrub_env!(env) + env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ } + env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ } + env.delete 'action_dispatch.request.query_parameters' + env.delete 'action_dispatch.request.request_parameters' + env + end + def process_with_kwargs(http_method, action, *args) if kwarg_request?(args) args.first.merge!(method: http_method) @@ -591,23 +621,6 @@ module ActionController end end - def build_request_uri(controller_class_name, action, parameters) - unless @request.env["PATH_INFO"] - options = @controller.respond_to?(:url_options) ? @controller.__send__(:url_options).merge(parameters) : parameters - options.update( - :controller => controller_class_name, - :action => action, - :relative_url_root => nil, - :_recall => @request.path_parameters) - - url, query_string = @routes.path_for(options).split("?", 2) - - @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root - @request.env["PATH_INFO"] = url - @request.env["QUERY_STRING"] = query_string || "" - end - end - def html_format?(parameters) return true unless parameters.key?(:format) Mime.fetch(parameters[:format]) { Mime['html'] }.html? diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index dcd3ee0644..f6336c8c7a 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -52,6 +52,7 @@ module ActionDispatch autoload :DebugExceptions autoload :ExceptionWrapper autoload :Flash + autoload :LoadInterlock autoload :ParamsParser autoload :PublicExceptions autoload :Reloader diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 7e585aa244..a639f8a8f8 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -45,7 +45,7 @@ module Mime # # respond_to do |format| # format.html - # format.ics { render text: @post.to_ics, mime_type: Mime::Type.lookup("text/calendar") } + # format.ics { render body: @post.to_ics, mime_type: Mime::Type.lookup("text/calendar") } # format.xml { render xml: @post } # end # end @@ -211,7 +211,7 @@ module Mime # This method is opposite of register method. # - # Usage: + # To unregister a MIME type: # # Mime::Type.unregister(:mobile) def unregister(symbol) diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index df4b073a17..6e058b829e 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -30,36 +30,46 @@ module ActionDispatch when Regexp regexps << item else - strings << item.to_s + strings << Regexp.escape(item.to_s) end end + deep_regexps, regexps = regexps.partition { |r| r.to_s.include?("\\.") } + deep_strings, strings = strings.partition { |s| s.include?("\\.") } + regexps << Regexp.new(strings.join('|'), true) unless strings.empty? - new regexps, blocks + deep_regexps << Regexp.new(deep_strings.join('|'), true) unless deep_strings.empty? + + new regexps, deep_regexps, blocks end - attr_reader :regexps, :blocks + attr_reader :regexps, :deep_regexps, :blocks - def initialize(regexps, blocks) + def initialize(regexps, deep_regexps, blocks) @regexps = regexps + @deep_regexps = deep_regexps.any? ? deep_regexps : nil @blocks = blocks end - def call(original_params) + def call(original_params, parents = []) filtered_params = {} original_params.each do |key, value| + parents.push(key) if deep_regexps if regexps.any? { |r| key =~ r } value = FILTERED + elsif deep_regexps && (joined = parents.join('.')) && deep_regexps.any? { |r| joined =~ r } + value = FILTERED elsif value.is_a?(Hash) - value = call(value) + value = call(value, parents) elsif value.is_a?(Array) - value = value.map { |v| v.is_a?(Hash) ? call(v) : v } + value = value.map { |v| v.is_a?(Hash) ? call(v, parents) : v } elsif blocks.any? key = key.dup if key.duplicable? value = value.dup if value.duplicable? blocks.each { |b| b.call(key, value) } end + parents.pop if deep_regexps filtered_params[key] = value end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index c5939adb9f..eab7d0ab57 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -80,11 +80,21 @@ module ActionDispatch # :nodoc: @response = response @buf = buf @closed = false + @str_body = nil + end + + def body + @str_body ||= begin + buf = '' + each { |chunk| buf << chunk } + buf + end end def write(string) raise IOError, "closed stream" if closed? + @str_body = nil @response.commit! @buf.push string end @@ -222,9 +232,7 @@ module ActionDispatch # :nodoc: # Returns the content of the response as a string. This contains the contents # of any calls to <tt>render</tt>. def body - strings = [] - each { |part| strings << part.to_s } - strings.join + @stream.body end EMPTY = " " diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index dd1f140051..07d97bd6bd 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -8,7 +8,7 @@ require 'active_support/json' module ActionDispatch class Request < Rack::Request def cookie_jar - env['action_dispatch.cookies'] ||= Cookies::CookieJar.build(self) + env['action_dispatch.cookies'] ||= Cookies::CookieJar.build(env, host, ssl?, cookies) end end @@ -228,16 +228,12 @@ module ActionDispatch } end - def self.build(request) - env = request.env + def self.build(env, host, secure, cookies) key_generator = env[GENERATOR_KEY] options = options_for_env env - host = request.host - secure = request.ssl? - new(key_generator, host, secure, options).tap do |hash| - hash.update(request.cookies) + hash.update(cookies) end end @@ -283,6 +279,10 @@ module ActionDispatch self end + def to_header + @cookies.map { |k,v| "#{k}=#{v}" }.join ';' + end + def handle_options(options) #:nodoc: options[:path] ||= "/" diff --git a/actionpack/lib/action_dispatch/middleware/load_interlock.rb b/actionpack/lib/action_dispatch/middleware/load_interlock.rb new file mode 100644 index 0000000000..07f498319c --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/load_interlock.rb @@ -0,0 +1,21 @@ +require 'active_support/dependencies' +require 'rack/body_proxy' + +module ActionDispatch + class LoadInterlock + def initialize(app) + @app = app + end + + def call(env) + interlock = ActiveSupport::Dependencies.interlock + interlock.start_running + response = @app.call(env) + body = Rack::BodyProxy.new(response[2]) { interlock.done_running } + response[2] = body + response + ensure + interlock.done_running unless body + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 29d43faeed..e2b3b06fd8 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -13,40 +13,35 @@ module ActionDispatch end end - DEFAULT_PARSERS = { Mime::JSON => :json } + DEFAULT_PARSERS = { + Mime::JSON => lambda { |raw_post| + data = ActiveSupport::JSON.decode(raw_post) + data = {:_json => data} unless data.is_a?(Hash) + Request::Utils.deep_munge(data).with_indifferent_access + } + } def initialize(app, parsers = {}) @app, @parsers = app, DEFAULT_PARSERS.merge(parsers) end def call(env) - if params = parse_formatted_parameters(env) - env["action_dispatch.request.request_parameters"] = params - end + default = env["action_dispatch.request.request_parameters"] + env["action_dispatch.request.request_parameters"] = parse_formatted_parameters(env, @parsers, default) @app.call(env) end private - def parse_formatted_parameters(env) + def parse_formatted_parameters(env, parsers, default) request = Request.new(env) - return false if request.content_length.zero? + return default if request.content_length.zero? - strategy = @parsers[request.content_mime_type] + strategy = parsers.fetch(request.content_mime_type) { return default } - return false unless strategy + strategy.call(request.raw_post) - case strategy - when Proc - strategy.call(request.raw_post) - when :json - data = ActiveSupport::JSON.decode(request.raw_post) - data = {:_json => data} unless data.is_a?(Hash) - Request::Utils.deep_munge(data).with_indifferent_access - else - false - end rescue => e # JSON or Ruby code block errors logger(env).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 8379d089df..967bbd62f8 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -171,6 +171,10 @@ module ActionDispatch route_name = options.delete :use_route _routes.url_for(options.symbolize_keys.reverse_merge!(url_options), route_name) + when ActionController::Parameters + route_name = options.delete :use_route + _routes.url_for(options.to_unsafe_h.symbolize_keys. + reverse_merge!(url_options), route_name) when String options when Symbol diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 13a72220b3..b6e21b0d28 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -3,6 +3,13 @@ module ActionDispatch module Assertions # A small suite of assertions that test responses from \Rails applications. module ResponseAssertions + RESPONSE_PREDICATES = { # :nodoc: + success: :successful?, + missing: :not_found?, + redirect: :redirection?, + error: :server_error?, + } + # Asserts that the response is one of the following types: # # * <tt>:success</tt> - Status code was in the 200-299 range @@ -20,11 +27,9 @@ module ActionDispatch # # assert that the response code was status code 401 (unauthorized) # assert_response 401 def assert_response(type, message = nil) - message ||= "Expected response to be a <#{type}>, but was <#{@response.response_code}>" - if Symbol === type if [:success, :missing, :redirect, :error].include?(type) - assert @response.send("#{type}?"), message + assert_predicate @response, RESPONSE_PREDICATES[type], message else code = Rack::Utils::SYMBOL_TO_STATUS_CODE[type] if code.nil? diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index c94eea9134..d0e3ea818e 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -165,7 +165,7 @@ module ActionDispatch # ROUTES TODO: These assertions should really work in an integration context def method_missing(selector, *args, &block) - if defined?(@controller) && @controller && @routes && @routes.named_routes.route_defined?(selector) + if defined?(@controller) && @controller && defined?(@routes) && @routes && @routes.named_routes.route_defined?(selector) @controller.send(selector, *args, &block) else super @@ -183,7 +183,7 @@ module ActionDispatch end # Assume given controller - request = ActionController::TestRequest.new + request = ActionController::TestRequest.create if path =~ %r{://} fail_on(URI::InvalidURIError, msg) do diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index 415ef80cd2..494644cd46 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -19,7 +19,7 @@ module ActionDispatch end def cookies - @request.cookie_jar + @cookie_jar ||= Cookies::CookieJar.build(@request.env, @request.host, @request.ssl?, @request.cookies) end def redirect_to_url diff --git a/actionpack/lib/action_dispatch/testing/test_request.rb b/actionpack/lib/action_dispatch/testing/test_request.rb index 4b9a088265..ad1a7f7109 100644 --- a/actionpack/lib/action_dispatch/testing/test_request.rb +++ b/actionpack/lib/action_dispatch/testing/test_request.rb @@ -4,19 +4,22 @@ require 'rack/utils' module ActionDispatch class TestRequest < Request DEFAULT_ENV = Rack::MockRequest.env_for('/', - 'HTTP_HOST' => 'test.host', - 'REMOTE_ADDR' => '0.0.0.0', - 'HTTP_USER_AGENT' => 'Rails Testing' + 'HTTP_HOST' => 'test.host', + 'REMOTE_ADDR' => '0.0.0.0', + 'HTTP_USER_AGENT' => 'Rails Testing', ) - def self.new(env = {}) - super + # Create a new test request with default `env` values + def self.create(env = {}) + env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application + env["rack.request.cookie_hash"] ||= {}.with_indifferent_access + new(default_env.merge(env)) end - def initialize(env = {}) - env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application - super(default_env.merge(env)) + def self.default_env + DEFAULT_ENV end + private_class_method :default_env def request_method=(method) @env['REQUEST_METHOD'] = method.to_s.upcase @@ -62,17 +65,5 @@ module ActionDispatch @env.delete('action_dispatch.request.accepts') @env['HTTP_ACCEPT'] = Array(mime_types).collect(&:to_s).join(",") end - - alias :rack_cookies :cookies - - def cookies - @cookies ||= {}.with_indifferent_access - end - - private - - def default_env - DEFAULT_ENV - end end end diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index a9b88ac5fd..6a31d6243f 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -16,9 +16,6 @@ module ActionDispatch # Was the URL not found? alias_method :missing?, :not_found? - # Were we redirected? - alias_method :redirect?, :redirection? - # Was there a server-side error? alias_method :error?, :server_error? end diff --git a/actionpack/test/assertions/response_assertions_test.rb b/actionpack/test/assertions/response_assertions_test.rb index 5e64cae7e2..82c747680d 100644 --- a/actionpack/test/assertions/response_assertions_test.rb +++ b/actionpack/test/assertions/response_assertions_test.rb @@ -7,7 +7,7 @@ module ActionDispatch include ResponseAssertions FakeResponse = Struct.new(:response_code) do - [:success, :missing, :redirect, :error].each do |sym| + [:successful, :not_found, :redirection, :server_error].each do |sym| define_method("#{sym}?") do sym == response_code end @@ -16,7 +16,7 @@ module ActionDispatch def test_assert_response_predicate_methods [:success, :missing, :redirect, :error].each do |sym| - @response = FakeResponse.new sym + @response = FakeResponse.new RESPONSE_PREDICATES[sym].to_s.sub(/\?/, '').to_sym assert_response sym assert_raises(Minitest::Assertion) { diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 5b85e83045..beeafc2e53 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -43,12 +43,12 @@ class ActionPackAssertionsController < ActionController::Base def flash_me flash['hello'] = 'my name is inigo montoya...' - render :text => "Inconceivable!" + render plain: "Inconceivable!" end def flash_me_naked flash.clear - render :text => "wow!" + render plain: "wow!" end def assign_this @@ -57,30 +57,30 @@ class ActionPackAssertionsController < ActionController::Base end def render_based_on_parameters - render :text => "Mr. #{params[:name]}" + render plain: "Mr. #{params[:name]}" end def render_url - render :text => "<div>#{url_for(:action => 'flash_me', :only_path => true)}</div>" + render html: "<div>#{url_for(action: 'flash_me', only_path: true)}</div>" end def render_text_with_custom_content_type - render :text => "Hello!", :content_type => Mime::RSS + render body: "Hello!", content_type: Mime::RSS end def session_stuffing session['xmas'] = 'turkey' - render :text => "ho ho ho" + render plain: "ho ho ho" end def raise_exception_on_get raise "get" if request.get? - render :text => "request method: #{request.env['REQUEST_METHOD']}" + render plain: "request method: #{request.env['REQUEST_METHOD']}" end def raise_exception_on_post raise "post" if request.post? - render :text => "request method: #{request.env['REQUEST_METHOD']}" + render plain: "request method: #{request.env['REQUEST_METHOD']}" end def render_file_absolute_path @@ -101,7 +101,7 @@ class AssertResponseWithUnexpectedErrorController < ActionController::Base end def show - render :text => "Boom", :status => 500 + render plain: "Boom", status: 500 end end @@ -318,7 +318,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_missing_response_code process :response404 - assert @response.missing? + assert @response.not_found? end def test_client_error_response_code @@ -346,12 +346,12 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_successful_response_code process :nothing - assert @response.success? + assert @response.successful? end def test_response_object process :nothing - assert_kind_of ActionController::TestResponse, @response + assert_kind_of ActionDispatch::TestResponse, @response end def test_render_based_on_parameters diff --git a/actionpack/test/controller/api/conditional_get_test.rb b/actionpack/test/controller/api/conditional_get_test.rb index d1eb27bf24..b4f1673be0 100644 --- a/actionpack/test/controller/api/conditional_get_test.rb +++ b/actionpack/test/controller/api/conditional_get_test.rb @@ -7,12 +7,12 @@ class ConditionalGetApiController < ActionController::API def one if stale?(last_modified: Time.now.utc.beginning_of_day, etag: [:foo, 123]) - render text: "Hi!" + render plain: "Hi!" end end def two - render text: "Hi!" + render plain: "Hi!" end private diff --git a/actionpack/test/controller/api/params_wrapper_test.rb b/actionpack/test/controller/api/params_wrapper_test.rb index e40a39d829..53b3a0c3cc 100644 --- a/actionpack/test/controller/api/params_wrapper_test.rb +++ b/actionpack/test/controller/api/params_wrapper_test.rb @@ -7,7 +7,7 @@ class ParamsWrapperForApiTest < ActionController::TestCase wrap_parameters :person, format: [:json] def test - self.last_parameters = params.except(:controller, :action) + self.last_parameters = params.except(:controller, :action).to_unsafe_h head :ok end end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 3240185414..d9374ce9c3 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -53,7 +53,7 @@ end class ActionMissingController < ActionController::Base def action_missing(action) - render :text => "Response for #{action}" + render plain: "Response for #{action}" end end @@ -127,8 +127,6 @@ class PerformActionTest < ActionController::TestCase # a more accurate simulation of what happens in "real life". @controller.logger = ActiveSupport::Logger.new(nil) - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @request.host = "www.nextangle.com" end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index f21dd87400..5698159eba 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -22,8 +22,6 @@ class FragmentCachingMetalTest < ActionController::TestCase @controller.perform_caching = true @controller.cache_store = @store @params = { controller: 'posts', action: 'index' } - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @controller.params = @params @controller.request = @request @controller.response = @response @@ -52,8 +50,6 @@ class FragmentCachingTest < ActionController::TestCase @controller.perform_caching = true @controller.cache_store = @store @params = {:controller => 'posts', :action => 'index'} - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @controller.params = @params @controller.request = @request @controller.response = @response @@ -166,6 +162,8 @@ class FunctionalCachingController < CachingController end def formatted_fragment_cached_with_variant + request.variant = :phone if params[:v] == "phone" + respond_to do |format| format.html.phone format.html @@ -183,8 +181,6 @@ class FunctionalFragmentCachingTest < ActionController::TestCase @controller = FunctionalCachingController.new @controller.perform_caching = true @controller.cache_store = @store - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new end def test_fragment_caching @@ -268,9 +264,7 @@ CACHED def test_fragment_caching_with_variant - @request.variant = :phone - - get :formatted_fragment_cached_with_variant, format: "html" + get :formatted_fragment_cached_with_variant, format: "html", params: { v: :phone } assert_response :success expected_body = "<body>\n<p>PHONE</p>\n</body>\n" @@ -381,6 +375,7 @@ class AutomaticCollectionCacheTest < ActionController::TestCase @controller.perform_caching = true @controller.partial_rendered_times = 0 @controller.cache_store = ActiveSupport::Cache::MemoryStore.new + ActionView::PartialRenderer.collection_cache = @controller.cache_store end def test_collection_fetches_cached_views diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index 89667df3a4..c5bbc479c9 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -4,29 +4,29 @@ class OldContentTypeController < ActionController::Base # :ported: def render_content_type_from_body response.content_type = Mime::RSS - render :text => "hello world!" + render body: "hello world!" end # :ported: def render_defaults - render :text => "hello world!" + render body: "hello world!" end # :ported: def render_content_type_from_render - render :text => "hello world!", :content_type => Mime::RSS + render body: "hello world!", :content_type => Mime::RSS end # :ported: def render_charset_from_body response.charset = "utf-16" - render :text => "hello world!" + render body: "hello world!" end # :ported: def render_nil_charset_from_body response.charset = nil - render :text => "hello world!" + render body: "hello world!" end def render_default_for_erb @@ -42,10 +42,10 @@ class OldContentTypeController < ActionController::Base def render_default_content_types_for_respond_to respond_to do |format| - format.html { render :text => "hello world!" } - format.xml { render :action => "render_default_content_types_for_respond_to" } - format.js { render :text => "hello world!" } - format.rss { render :text => "hello world!", :content_type => Mime::XML } + format.html { render body: "hello world!" } + format.xml { render action: "render_default_content_types_for_respond_to" } + format.js { render body: "hello world!" } + format.rss { render body: "hello world!", content_type: Mime::XML } end end end @@ -64,14 +64,14 @@ class ContentTypeTest < ActionController::TestCase def test_render_defaults get :render_defaults assert_equal "utf-8", @response.charset - assert_equal Mime::HTML, @response.content_type + assert_equal Mime::TEXT, @response.content_type end def test_render_changed_charset_default with_default_charset "utf-16" do get :render_defaults assert_equal "utf-16", @response.charset - assert_equal Mime::HTML, @response.content_type + assert_equal Mime::TEXT, @response.content_type end end @@ -92,14 +92,14 @@ class ContentTypeTest < ActionController::TestCase # :ported: def test_charset_from_body get :render_charset_from_body - assert_equal Mime::HTML, @response.content_type + assert_equal Mime::TEXT, @response.content_type assert_equal "utf-16", @response.charset end # :ported: def test_nil_charset_from_body get :render_nil_charset_from_body - assert_equal Mime::HTML, @response.content_type + assert_equal Mime::TEXT, @response.content_type assert_equal "utf-8", @response.charset, @response.headers.inspect end diff --git a/actionpack/test/controller/default_url_options_with_before_action_test.rb b/actionpack/test/controller/default_url_options_with_before_action_test.rb index 230f40d7ad..12fbe0424e 100644 --- a/actionpack/test/controller/default_url_options_with_before_action_test.rb +++ b/actionpack/test/controller/default_url_options_with_before_action_test.rb @@ -6,7 +6,7 @@ class ControllerWithBeforeActionAndDefaultUrlOptions < ActionController::Base after_action { I18n.locale = "en" } def target - render :text => "final response" + render plain: "final response" end def redirect diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 9b0487841f..08271012e9 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -40,7 +40,7 @@ class FilterTest < ActionController::TestCase before_action :ensure_login, :except => [:go_wild] def go_wild - render :text => "gobble" + render plain: "gobble" end end @@ -51,7 +51,7 @@ class FilterTest < ActionController::TestCase (1..3).each do |i| define_method "fail_#{i}" do - render :text => i.to_s + render plain: i.to_s end end @@ -222,7 +222,7 @@ class FilterTest < ActionController::TestCase skip_before_action :clean_up_tmp, only: :login, if: -> { true } def login - render text: 'ok' + render plain: 'ok' end end @@ -234,7 +234,7 @@ class FilterTest < ActionController::TestCase skip_before_action :clean_up_tmp, if: -> { true }, except: :login def login - render text: 'ok' + render plain: 'ok' end end @@ -258,11 +258,11 @@ class FilterTest < ActionController::TestCase before_action :ensure_login, :only => :index def index - render :text => 'ok' + render plain: 'ok' end def public - render :text => 'ok' + render plain: 'ok' end end @@ -272,7 +272,7 @@ class FilterTest < ActionController::TestCase before_action :ensure_login def index - render :text => 'ok' + render plain: 'ok' end private @@ -383,7 +383,7 @@ class FilterTest < ActionController::TestCase before_action(AuditFilter) def show - render :text => "hello" + render plain: "hello" end end @@ -421,11 +421,11 @@ class FilterTest < ActionController::TestCase before_action :second, :only => :foo def foo - render :text => 'foo' + render plain: 'foo' end def bar - render :text => 'bar' + render plain: 'bar' end protected @@ -442,7 +442,7 @@ class FilterTest < ActionController::TestCase before_action :choose %w(foo bar baz).each do |action| - define_method(action) { render :text => action } + define_method(action) { render plain: action } end private @@ -471,7 +471,7 @@ class FilterTest < ActionController::TestCase @ran_filter << 'between_before_all_and_after_all' end def show - render :text => 'hello' + render plain: 'hello' end end @@ -481,7 +481,7 @@ class FilterTest < ActionController::TestCase def around(controller) yield rescue ErrorToRescue => ex - controller.__send__ :render, :text => "I rescued this: #{ex.inspect}" + controller.__send__ :render, plain: "I rescued this: #{ex.inspect}" end end @@ -757,9 +757,8 @@ class FilterTest < ActionController::TestCase def test_dynamic_dispatch %w(foo bar baz).each do |action| - request = ActionController::TestRequest.new - request.query_parameters[:choose] = action - response = DynamicDispatchController.action(action).call(request.env).last + @request.query_parameters[:choose] = action + response = DynamicDispatchController.action(action).call(@request.env).last assert_equal action, response.body end end @@ -820,7 +819,7 @@ class FilterTest < ActionController::TestCase response = test_process(RescuedController) end - assert response.success? + assert response.successful? assert_equal("I rescued this: #<FilterTest::ErrorToRescue: Something made the bad noise.>", response.body) end @@ -839,8 +838,6 @@ class FilterTest < ActionController::TestCase private def test_process(controller, action = "show") @controller = controller.is_a?(Class) ? controller.new : controller - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new process(action) end diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 72ae30eb39..22f1cc7c22 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -2,11 +2,11 @@ require 'abstract_unit' class ForceSSLController < ActionController::Base def banana - render :text => "monkey" + render plain: "monkey" end def cheeseburger - render :text => "sikachu" + render plain: "sikachu" end end @@ -26,7 +26,7 @@ class ForceSSLCustomOptions < ForceSSLController force_ssl :notice => 'Foo, Bar!', :only => :redirect_notice def force_ssl_action - render :text => action_name + render plain: action_name end alias_method :redirect_host, :force_ssl_action @@ -40,15 +40,15 @@ class ForceSSLCustomOptions < ForceSSLController alias_method :redirect_notice, :force_ssl_action def use_flash - render :text => flash[:message] + render plain: flash[:message] end def use_alert - render :text => flash[:alert] + render plain: flash[:alert] end def use_notice - render :text => flash[:notice] + render plain: flash[:notice] end end @@ -85,10 +85,10 @@ end class RedirectToSSL < ForceSSLController def banana - force_ssl_redirect || render(:text => 'monkey') + force_ssl_redirect || render(plain: 'monkey') end def cheeseburger - force_ssl_redirect('secure.cheeseburger.host') || render(:text => 'ihaz') + force_ssl_redirect('secure.cheeseburger.host') || render(plain: 'ihaz') end end diff --git a/actionpack/test/controller/helper_test.rb b/actionpack/test/controller/helper_test.rb index e263ed341f..3ecfedefd1 100644 --- a/actionpack/test/controller/helper_test.rb +++ b/actionpack/test/controller/helper_test.rb @@ -73,14 +73,8 @@ module LocalAbcHelper end class HelperPathsTest < ActiveSupport::TestCase - def setup - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new - end - def test_helpers_paths_priority - request = ActionController::TestRequest.new - responses = HelpersPathsController.action(:index).call(request.env) + responses = HelpersPathsController.action(:index).call(ActionController::TestRequest::DEFAULT_ENV.dup) # helpers1_pack was given as a second path, so pack1_helper should be # included as the second one @@ -141,8 +135,7 @@ class HelperTest < ActiveSupport::TestCase end def call_controller(klass, action) - request = ActionController::TestRequest.new - klass.action(action).call(request.env) + klass.action(action).call(ActionController::TestRequest::DEFAULT_ENV.dup) end def test_helper_for_nested_controller @@ -158,7 +151,7 @@ class HelperTest < ActiveSupport::TestCase assert_equal "test: baz", call_controller(Fun::PdfController, "test").last.body # # request = ActionController::TestRequest.new - # response = ActionController::TestResponse.new + # response = ActionDispatch::TestResponse.new # request.action = 'test' # # assert_equal 'test: baz', Fun::PdfController.process(request, response).body @@ -249,7 +242,7 @@ class HelperTest < ActiveSupport::TestCase end -class IsolatedHelpersTest < ActiveSupport::TestCase +class IsolatedHelpersTest < ActionController::TestCase class A < ActionController::Base def index render :inline => '<%= shout %>' @@ -273,13 +266,11 @@ class IsolatedHelpersTest < ActiveSupport::TestCase end def call_controller(klass, action) - request = ActionController::TestRequest.new - klass.action(action).call(request.env) + klass.action(action).call(@request.env) end def setup - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new + super @request.action = 'index' end diff --git a/actionpack/test/controller/http_basic_authentication_test.rb b/actionpack/test/controller/http_basic_authentication_test.rb index adaf19c2dc..ed3632007d 100644 --- a/actionpack/test/controller/http_basic_authentication_test.rb +++ b/actionpack/test/controller/http_basic_authentication_test.rb @@ -9,19 +9,19 @@ class HttpBasicAuthenticationTest < ActionController::TestCase http_basic_authenticate_with :name => "David", :password => "Goliath", :only => :search def index - render :text => "Hello Secret" + render plain: "Hello Secret" end def display - render :text => 'Definitely Maybe' if @logged_in + render plain: 'Definitely Maybe' if @logged_in end def show - render :text => 'Only for loooooong credentials' + render plain: 'Only for loooooong credentials' end def search - render :text => 'All inline' + render plain: 'All inline' end private diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 57964769a7..f06912bd5a 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -10,11 +10,11 @@ class HttpDigestAuthenticationTest < ActionController::TestCase 'dhh' => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":"))} def index - render :text => "Hello Secret" + render plain: "Hello Secret" end def display - render :text => 'Definitely Maybe' if @logged_in + render plain: 'Definitely Maybe' if @logged_in end private diff --git a/actionpack/test/controller/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb index 802c17b6bf..9c5a01c318 100644 --- a/actionpack/test/controller/http_token_authentication_test.rb +++ b/actionpack/test/controller/http_token_authentication_test.rb @@ -7,15 +7,15 @@ class HttpTokenAuthenticationTest < ActionController::TestCase before_action :authenticate_long_credentials, only: :show def index - render :text => "Hello Secret" + render plain: "Hello Secret" end def display - render :text => 'Definitely Maybe' + render plain: 'Definitely Maybe' end def show - render :text => 'Only for loooooong credentials' + render plain: 'Only for loooooong credentials' end private diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index a6460168cb..0b9be8d671 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -359,28 +359,28 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest class IntegrationController < ActionController::Base def get respond_to do |format| - format.html { render :text => "OK", :status => 200 } - format.js { render :text => "JS OK", :status => 200 } + format.html { render plain: "OK", status: 200 } + format.js { render plain: "JS OK", status: 200 } format.xml { render :xml => "<root></root>", :status => 200 } end end def get_with_params - render :text => "foo: #{params[:foo]}", :status => 200 + render plain: "foo: #{params[:foo]}", status: 200 end def post - render :text => "Created", :status => 201 + render plain: "Created", status: 201 end def method - render :text => "method: #{request.method.downcase}" + render plain: "method: #{request.method.downcase}" end def cookie_monster cookies["cookie_1"] = nil cookies["cookie_3"] = "chocolate" - render :text => "Gone", :status => 410 + render plain: "Gone", status: 410 end def set_cookie @@ -389,7 +389,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end def get_cookie - render :text => cookies["foo"] + render plain: cookies["foo"] end def redirect @@ -760,7 +760,7 @@ end class ApplicationIntegrationTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base def index - render :text => "index" + render plain: "index" end end @@ -847,7 +847,7 @@ end class EnvironmentFilterIntegrationTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base def post - render :text => "Created", :status => 201 + render plain: "Created", status: 201 end end @@ -880,15 +880,15 @@ end class UrlOptionsIntegrationTest < ActionDispatch::IntegrationTest class FooController < ActionController::Base def index - render :text => "foo#index" + render plain: "foo#index" end def show - render :text => "foo#show" + render plain: "foo#show" end def edit - render :text => "foo#show" + render plain: "foo#show" end end @@ -898,7 +898,7 @@ class UrlOptionsIntegrationTest < ActionDispatch::IntegrationTest end def index - render :text => "foo#index" + render plain: "foo#index" end end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 0c65270ec1..6ba361f2f7 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -1,5 +1,5 @@ require 'abstract_unit' -require 'active_support/concurrency/latch' +require 'concurrent/atomics' Thread.abort_on_exception = true module ActionController @@ -125,7 +125,7 @@ module ActionController end def render_text - render :text => 'zomg' + render plain: 'zomg' end def default_header @@ -145,7 +145,7 @@ module ActionController response.headers['Content-Type'] = 'text/event-stream' %w{ hello world }.each do |word| response.stream.write word - latch.await + latch.wait end response.stream.close end @@ -162,7 +162,7 @@ module ActionController end def with_stale - render text: 'stale' if stale?(etag: "123", template: false) + render plain: 'stale' if stale?(etag: "123", template: false) end def exception_in_view @@ -212,7 +212,7 @@ module ActionController # .. plus one more, because the #each frees up a slot: response.stream.write '.' - latch.release + latch.count_down # This write will block, and eventually raise response.stream.write 'x' @@ -233,7 +233,7 @@ module ActionController end logger.info 'Work complete' - latch.release + latch.count_down end end @@ -278,7 +278,7 @@ module ActionController def test_async_stream rubinius_skip "https://github.com/rubinius/rubinius/issues/2934" - @controller.latch = ActiveSupport::Concurrency::Latch.new + @controller.latch = Concurrent::CountDownLatch.new parts = ['hello', 'world'] @controller.request = @request @@ -289,8 +289,8 @@ module ActionController resp.stream.each do |part| assert_equal parts.shift, part ol = @controller.latch - @controller.latch = ActiveSupport::Concurrency::Latch.new - ol.release + @controller.latch = Concurrent::CountDownLatch.new + ol.count_down end } @@ -300,23 +300,23 @@ module ActionController end def test_abort_with_full_buffer - @controller.latch = ActiveSupport::Concurrency::Latch.new + @controller.latch = Concurrent::CountDownLatch.new @request.parameters[:format] = 'plain' @controller.request = @request @controller.response = @response - got_error = ActiveSupport::Concurrency::Latch.new + got_error = Concurrent::CountDownLatch.new @response.stream.on_error do ActionController::Base.logger.warn 'Error while streaming' - got_error.release + got_error.count_down end t = Thread.new(@response) { |resp| resp.await_commit _, _, body = resp.to_a body.each do |part| - @controller.latch.await + @controller.latch.wait body.close break end @@ -325,13 +325,13 @@ module ActionController capture_log_output do |output| @controller.process :overfill_buffer_and_die t.join - got_error.await + got_error.wait assert_match 'Error while streaming', output.rewind && output.read end end def test_ignore_client_disconnect - @controller.latch = ActiveSupport::Concurrency::Latch.new + @controller.latch = Concurrent::CountDownLatch.new @controller.request = @request @controller.response = @response @@ -349,7 +349,7 @@ module ActionController @controller.process :ignore_client_disconnect t.join Timeout.timeout(3) do - @controller.latch.await + @controller.latch.wait end assert_match 'Work complete', output.rewind && output.read end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 8591bdb4d9..64eb33f78f 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -4,43 +4,50 @@ require "active_support/log_subscriber/test_helper" class RespondToController < ActionController::Base layout :set_layout + before_action { + case params[:v] + when String then request.variant = params[:v].to_sym + when Array then request.variant = params[:v].map(&:to_sym) + end + } + def html_xml_or_rss respond_to do |type| - type.html { render :text => "HTML" } - type.xml { render :text => "XML" } - type.rss { render :text => "RSS" } - type.all { render :text => "Nothing" } + type.html { render body: "HTML" } + type.xml { render body: "XML" } + type.rss { render body: "RSS" } + type.all { render body: "Nothing" } end end def js_or_html respond_to do |type| - type.html { render :text => "HTML" } - type.js { render :text => "JS" } - type.all { render :text => "Nothing" } + type.html { render body: "HTML" } + type.js { render body: "JS" } + type.all { render body: "Nothing" } end end def json_or_yaml respond_to do |type| - type.json { render :text => "JSON" } - type.yaml { render :text => "YAML" } + type.json { render body: "JSON" } + type.yaml { render body: "YAML" } end end def html_or_xml respond_to do |type| - type.html { render :text => "HTML" } - type.xml { render :text => "XML" } - type.all { render :text => "Nothing" } + type.html { render body: "HTML" } + type.xml { render body: "XML" } + type.all { render body: "Nothing" } end end def json_xml_or_html respond_to do |type| - type.json { render :text => 'JSON' } + type.json { render body: 'JSON' } type.xml { render :xml => 'XML' } - type.html { render :text => 'HTML' } + type.html { render body: 'HTML' } end end @@ -49,14 +56,14 @@ class RespondToController < ActionController::Base request.format = :xml respond_to do |type| - type.html { render :text => "HTML" } - type.xml { render :text => "XML" } + type.html { render body: "HTML" } + type.xml { render body: "XML" } end end def just_xml respond_to do |type| - type.xml { render :text => "XML" } + type.xml { render body: "XML" } end end @@ -74,52 +81,52 @@ class RespondToController < ActionController::Base def using_defaults_with_all respond_to do |type| type.html - type.all{ render text: "ALL" } + type.all{ render body: "ALL" } end end def made_for_content_type respond_to do |type| - type.rss { render :text => "RSS" } - type.atom { render :text => "ATOM" } - type.all { render :text => "Nothing" } + type.rss { render body: "RSS" } + type.atom { render body: "ATOM" } + type.all { render body: "Nothing" } end end def custom_type_handling respond_to do |type| - type.html { render :text => "HTML" } - type.custom("application/crazy-xml") { render :text => "Crazy XML" } - type.all { render :text => "Nothing" } + type.html { render body: "HTML" } + type.custom("application/crazy-xml") { render body: "Crazy XML" } + type.all { render body: "Nothing" } end end def custom_constant_handling respond_to do |type| - type.html { render :text => "HTML" } - type.mobile { render :text => "Mobile" } + type.html { render body: "HTML" } + type.mobile { render body: "Mobile" } end end def custom_constant_handling_without_block respond_to do |type| - type.html { render :text => "HTML" } + type.html { render body: "HTML" } type.mobile end end def handle_any respond_to do |type| - type.html { render :text => "HTML" } - type.any(:js, :xml) { render :text => "Either JS or XML" } + type.html { render body: "HTML" } + type.any(:js, :xml) { render body: "Either JS or XML" } end end def handle_any_any respond_to do |type| - type.html { render :text => 'HTML' } - type.any { render :text => 'Whatever you ask for, I got it' } + type.html { render body: 'HTML' } + type.any { render body: 'Whatever you ask for, I got it' } end end @@ -160,15 +167,15 @@ class RespondToController < ActionController::Base request.variant = :mobile respond_to do |type| - type.html { render text: "mobile" } + type.html { render body: "mobile" } end end def multiple_variants_for_format respond_to do |type| type.html do |html| - html.tablet { render text: "tablet" } - html.phone { render text: "phone" } + html.tablet { render body: "tablet" } + html.phone { render body: "phone" } end end end @@ -176,7 +183,7 @@ class RespondToController < ActionController::Base def variant_plus_none_for_format respond_to do |format| format.html do |variant| - variant.phone { render text: "phone" } + variant.phone { render body: "phone" } variant.none end end @@ -184,9 +191,9 @@ class RespondToController < ActionController::Base def variant_inline_syntax respond_to do |format| - format.js { render text: "js" } - format.html.none { render text: "none" } - format.html.phone { render text: "phone" } + format.js { render body: "js" } + format.html.none { render body: "none" } + format.html.phone { render body: "phone" } end end @@ -201,8 +208,8 @@ class RespondToController < ActionController::Base def variant_any respond_to do |format| format.html do |variant| - variant.any(:tablet, :phablet){ render text: "any" } - variant.phone { render text: "phone" } + variant.any(:tablet, :phablet){ render body: "any" } + variant.phone { render body: "phone" } end end end @@ -210,23 +217,23 @@ class RespondToController < ActionController::Base def variant_any_any respond_to do |format| format.html do |variant| - variant.any { render text: "any" } - variant.phone { render text: "phone" } + variant.any { render body: "any" } + variant.phone { render body: "phone" } end end end def variant_inline_any respond_to do |format| - format.html.any(:tablet, :phablet){ render text: "any" } - format.html.phone { render text: "phone" } + format.html.any(:tablet, :phablet){ render body: "any" } + format.html.phone { render body: "phone" } end end def variant_inline_any_any respond_to do |format| - format.html.phone { render text: "phone" } - format.html.any { render text: "any" } + format.html.phone { render body: "phone" } + format.html.any { render body: "any" } end end @@ -239,16 +246,16 @@ class RespondToController < ActionController::Base def variant_any_with_none respond_to do |format| - format.html.any(:none, :phone){ render text: "none or phone" } + format.html.any(:none, :phone){ render body: "none or phone" } end end def format_any_variant_any respond_to do |format| - format.html { render text: "HTML" } + format.html { render body: "HTML" } format.any(:js, :xml) do |variant| - variant.phone{ render text: "phone" } - variant.any(:tablet, :phablet){ render text: "tablet" } + variant.phone{ render body: "phone" } + variant.any(:tablet, :phablet){ render body: "tablet" } end end end @@ -612,8 +619,7 @@ class RespondToControllerTest < ActionController::TestCase logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new old_logger, ActionController::Base.logger = ActionController::Base.logger, logger - @request.variant = :invalid - get :variant_with_implicit_rendering + get :variant_with_implicit_rendering, params: { v: :invalid } assert_response :no_content assert_equal 1, logger.logged(:info).select{ |s| s =~ /No template found/ }.size, "Implicit head :no_content not logged" ensure @@ -626,28 +632,24 @@ class RespondToControllerTest < ActionController::TestCase end def test_variant_with_implicit_rendering - @request.variant = :implicit - get :variant_with_implicit_rendering + get :variant_with_implicit_rendering, params: { v: :implicit } assert_response :no_content end def test_variant_with_implicit_template_rendering - @request.variant = :mobile - get :variant_with_implicit_rendering + get :variant_with_implicit_rendering, params: { v: :mobile } assert_equal "text/html", @response.content_type assert_equal "mobile", @response.body end def test_variant_with_format_and_custom_render - @request.variant = :phone - get :variant_with_format_and_custom_render + get :variant_with_format_and_custom_render, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "mobile", @response.body end def test_multiple_variants_for_format - @request.variant = :tablet - get :multiple_variants_for_format + get :multiple_variants_for_format, params: { v: :tablet } assert_equal "text/html", @response.content_type assert_equal "tablet", @response.body end @@ -667,32 +669,27 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "text/html", @response.content_type assert_equal "none", @response.body - @request.variant = :phone - get :variant_inline_syntax + get :variant_inline_syntax, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body end def test_variant_inline_syntax_without_block - @request.variant = :phone - get :variant_inline_syntax_without_block + get :variant_inline_syntax_without_block, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body end def test_variant_any - @request.variant = :phone - get :variant_any + get :variant_any, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body - @request.variant = :tablet - get :variant_any + get :variant_any, params: { v: :tablet } assert_equal "text/html", @response.content_type assert_equal "any", @response.body - @request.variant = :phablet - get :variant_any + get :variant_any, params: { v: :phablet } assert_equal "text/html", @response.content_type assert_equal "any", @response.body end @@ -702,54 +699,45 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "text/html", @response.content_type assert_equal "any", @response.body - @request.variant = :phone - get :variant_any_any + get :variant_any_any, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body - @request.variant = :yolo - get :variant_any_any + get :variant_any_any, params: { v: :yolo } assert_equal "text/html", @response.content_type assert_equal "any", @response.body end def test_variant_inline_any - @request.variant = :phone - get :variant_any + get :variant_any, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body - @request.variant = :tablet - get :variant_inline_any + get :variant_inline_any, params: { v: :tablet } assert_equal "text/html", @response.content_type assert_equal "any", @response.body - @request.variant = :phablet - get :variant_inline_any + get :variant_inline_any, params: { v: :phablet } assert_equal "text/html", @response.content_type assert_equal "any", @response.body end def test_variant_inline_any_any - @request.variant = :phone - get :variant_inline_any_any + get :variant_inline_any_any, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body - @request.variant = :yolo - get :variant_inline_any_any + get :variant_inline_any_any, params: { v: :yolo } assert_equal "text/html", @response.content_type assert_equal "any", @response.body end def test_variant_any_implicit_render - @request.variant = :tablet - get :variant_any_implicit_render + get :variant_any_implicit_render, params: { v: :tablet } assert_equal "text/html", @response.content_type assert_equal "tablet", @response.body - @request.variant = :phablet - get :variant_any_implicit_render + get :variant_any_implicit_render, params: { v: :phablet } assert_equal "text/html", @response.content_type assert_equal "phablet", @response.body end @@ -759,36 +747,31 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "text/html", @response.content_type assert_equal "none or phone", @response.body - @request.variant = :phone - get :variant_any_with_none + get :variant_any_with_none, params: { v: :phone } assert_equal "text/html", @response.content_type assert_equal "none or phone", @response.body end def test_format_any_variant_any - @request.variant = :tablet - get :format_any_variant_any, format: :js + get :format_any_variant_any, format: :js, params: { v: :tablet } assert_equal "text/javascript", @response.content_type assert_equal "tablet", @response.body end def test_variant_negotiation_inline_syntax - @request.variant = [:tablet, :phone] - get :variant_inline_syntax_without_block + get :variant_inline_syntax_without_block, params: { v: [:tablet, :phone] } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body end def test_variant_negotiation_block_syntax - @request.variant = [:tablet, :phone] - get :variant_plus_none_for_format + get :variant_plus_none_for_format, params: { v: [:tablet, :phone] } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body end def test_variant_negotiation_without_block - @request.variant = [:tablet, :phone] - get :variant_inline_syntax_without_block + get :variant_inline_syntax_without_block, params: { v: [:tablet, :phone] } assert_equal "text/html", @response.content_type assert_equal "phone", @response.body end @@ -797,7 +780,7 @@ end class RespondToWithBlockOnDefaultRenderController < ActionController::Base def show default_render do - render text: 'default_render yielded' + render body: 'default_render yielded' end end end @@ -811,6 +794,6 @@ class RespondToWithBlockOnDefaultRenderControllerTest < ActionController::TestCa def test_default_render_uses_block_when_no_template_exists get :show assert_equal "default_render yielded", @response.body - assert_equal "text/html", @response.content_type + assert_equal "text/plain", @response.content_type end end diff --git a/actionpack/test/controller/new_base/base_test.rb b/actionpack/test/controller/new_base/base_test.rb index 964f22eb03..0755dafe93 100644 --- a/actionpack/test/controller/new_base/base_test.rb +++ b/actionpack/test/controller/new_base/base_test.rb @@ -6,7 +6,7 @@ module Dispatching before_action :authenticate def index - render :text => "success" + render body: "success" end def modify_response_body @@ -22,7 +22,7 @@ module Dispatching end def show_actions - render :text => "actions: #{action_methods.to_a.sort.join(', ')}" + render body: "actions: #{action_methods.to_a.sort.join(', ')}" end protected @@ -51,7 +51,7 @@ module Dispatching assert_body "success" assert_status 200 - assert_content_type "text/html; charset=utf-8" + assert_content_type "text/plain; charset=utf-8" end # :api: plugin diff --git a/actionpack/test/controller/new_base/content_negotiation_test.rb b/actionpack/test/controller/new_base/content_negotiation_test.rb index c8166280fc..c0e92b3b05 100644 --- a/actionpack/test/controller/new_base/content_negotiation_test.rb +++ b/actionpack/test/controller/new_base/content_negotiation_test.rb @@ -9,7 +9,7 @@ module ContentNegotiation )] def all - render :text => self.formats.inspect + render plain: self.formats.inspect end end diff --git a/actionpack/test/controller/new_base/content_type_test.rb b/actionpack/test/controller/new_base/content_type_test.rb index 88453988dd..0445a837ca 100644 --- a/actionpack/test/controller/new_base/content_type_test.rb +++ b/actionpack/test/controller/new_base/content_type_test.rb @@ -3,16 +3,16 @@ require 'abstract_unit' module ContentType class BaseController < ActionController::Base def index - render :text => "Hello world!" + render body: "Hello world!" end def set_on_response_obj response.content_type = Mime::RSS - render :text => "Hello world!" + render body: "Hello world!" end def set_on_render - render :text => "Hello world!", :content_type => Mime::RSS + render body: "Hello world!", content_type: Mime::RSS end end @@ -30,17 +30,17 @@ module ContentType class CharsetController < ActionController::Base def set_on_response_obj response.charset = "utf-16" - render :text => "Hello world!" + render body: "Hello world!" end def set_as_nil_on_response_obj response.charset = nil - render :text => "Hello world!" + render body: "Hello world!" end end class ExplicitContentTypeTest < Rack::TestCase - test "default response is HTML and UTF8" do + test "default response is text/plain and UTF8" do with_routing do |set| set.draw do get ':controller', :action => 'index' @@ -49,7 +49,7 @@ module ContentType get "/content_type/base" assert_body "Hello world!" - assert_header "Content-Type", "text/html; charset=utf-8" + assert_header "Content-Type", "text/plain; charset=utf-8" end end @@ -99,14 +99,14 @@ module ContentType get "/content_type/charset/set_on_response_obj" assert_body "Hello world!" - assert_header "Content-Type", "text/html; charset=utf-16" + assert_header "Content-Type", "text/plain; charset=utf-16" end test "setting the charset of the response as nil directly on the response object" do get "/content_type/charset/set_as_nil_on_response_obj" assert_body "Hello world!" - assert_header "Content-Type", "text/html; charset=utf-8" + assert_header "Content-Type", "text/plain; charset=utf-8" end end end diff --git a/actionpack/test/controller/new_base/render_test.rb b/actionpack/test/controller/new_base/render_test.rb index 11a19ab783..963f2c2f5c 100644 --- a/actionpack/test/controller/new_base/render_test.rb +++ b/actionpack/test/controller/new_base/render_test.rb @@ -37,14 +37,14 @@ module Render private def secretz - render :text => "FAIL WHALE!" + render plain: "FAIL WHALE!" end end class DoubleRenderController < ActionController::Base def index - render :text => "hello" - render :text => "world" + render plain: "hello" + render plain: "world" end end diff --git a/actionpack/test/controller/new_base/render_text_test.rb b/actionpack/test/controller/new_base/render_text_test.rb index 10bad57cd6..435bb18dce 100644 --- a/actionpack/test/controller/new_base/render_text_test.rb +++ b/actionpack/test/controller/new_base/render_text_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'active_support/deprecation' module RenderText class MinimalController < ActionController::Metal @@ -73,7 +74,10 @@ module RenderText class RenderTextTest < Rack::TestCase test "rendering text from a minimal controller" do - get "/render_text/minimal/index" + ActiveSupport::Deprecation.silence do + get "/render_text/minimal/index" + end + assert_body "Hello World!" assert_status 200 end @@ -82,7 +86,10 @@ module RenderText with_routing do |set| set.draw { get ':controller', action: 'index' } - get "/render_text/simple" + ActiveSupport::Deprecation.silence do + get "/render_text/simple" + end + assert_body "hello david" assert_status 200 end @@ -92,7 +99,9 @@ module RenderText with_routing do |set| set.draw { get ':controller', action: 'index' } - get "/render_text/with_layout" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout" + end assert_body "hello david" assert_status 200 @@ -100,59 +109,81 @@ module RenderText end test "rendering text, while also providing a custom status code" do - get "/render_text/with_layout/custom_code" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/custom_code" + end assert_body "hello world" assert_status 404 end test "rendering text with nil returns an empty body" do - get "/render_text/with_layout/with_nil" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_nil" + end assert_body "" assert_status 200 end test "Rendering text with nil and custom status code returns an empty body and the status" do - get "/render_text/with_layout/with_nil_and_status" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_nil_and_status" + end assert_body "" assert_status 403 end test "rendering text with false returns the string 'false'" do - get "/render_text/with_layout/with_false" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_false" + end assert_body "false" assert_status 200 end test "rendering text with layout: true" do - get "/render_text/with_layout/with_layout_true" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_layout_true" + end assert_body "hello world, I'm here!" assert_status 200 end test "rendering text with layout: 'greetings'" do - get "/render_text/with_layout/with_custom_layout" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_custom_layout" + end assert_body "hello world, I wish thee well." assert_status 200 end test "rendering text with layout: false" do - get "/render_text/with_layout/with_layout_false" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_layout_false" + end assert_body "hello world" assert_status 200 end test "rendering text with layout: nil" do - get "/render_text/with_layout/with_layout_nil" + ActiveSupport::Deprecation.silence do + get "/render_text/with_layout/with_layout_nil" + end assert_body "hello world" assert_status 200 end + + test "rendering text displays deprecation warning" do + assert_deprecated do + get "/render_text/with_layout/with_layout_nil" + end + end end end diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index 744d8664be..6c57c4caeb 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -62,11 +62,15 @@ class ParametersMutatorsTest < ActiveSupport::TestCase end test "select! retains permitted status" do + jruby_skip "https://github.com/jruby/jruby/issues/3137" + @params.permit! assert @params.select! { |k| k != "person" }.permitted? end test "select! retains unpermitted status" do + jruby_skip "https://github.com/jruby/jruby/issues/3137" + assert_not @params.select! { |k| k != "person" }.permitted? end diff --git a/actionpack/test/controller/parameters/nested_parameters_test.rb b/actionpack/test/controller/parameters/nested_parameters_test.rb index 3b1257e8d5..7151a8567c 100644 --- a/actionpack/test/controller/parameters/nested_parameters_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_test.rb @@ -136,7 +136,7 @@ class NestedParametersTest < ActiveSupport::TestCase authors_attributes: { :'0' => { name: 'William Shakespeare', age_of_death: '52' }, :'1' => { name: 'Unattributed Assistant' }, - :'2' => { name: %w(injected names)} + :'2' => { name: %w(injected names) } } } }) diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 2ed486516d..05532ec21b 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -253,7 +253,6 @@ class ParametersPermitTest < ActiveSupport::TestCase assert @params.to_h.is_a? Hash assert_not @params.to_h.is_a? ActionController::Parameters - assert_equal @params.to_hash, @params.to_h end test "to_h returns converted hash when .permit_all_parameters is set" do @@ -284,6 +283,5 @@ class ParametersPermitTest < ActiveSupport::TestCase test "to_unsafe_h returns unfiltered params" do assert @params.to_h.is_a? Hash assert_not @params.to_h.is_a? ActionController::Parameters - assert_equal @params.to_hash, @params.to_unsafe_h end end diff --git a/actionpack/test/controller/permitted_params_test.rb b/actionpack/test/controller/permitted_params_test.rb index 7136fafae5..7c753a45a5 100644 --- a/actionpack/test/controller/permitted_params_test.rb +++ b/actionpack/test/controller/permitted_params_test.rb @@ -2,11 +2,11 @@ require 'abstract_unit' class PeopleController < ActionController::Base def create - render text: params[:person].permitted? ? "permitted" : "forbidden" + render plain: params[:person].permitted? ? "permitted" : "forbidden" end def create_with_permit - render text: params[:person].permit(:name).permitted? ? "permitted" : "forbidden" + render plain: params[:person].permit(:name).permitted? ? "permitted" : "forbidden" end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 4f5ca46b04..91b30ede6a 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -3,8 +3,8 @@ require 'abstract_unit' class RedirectController < ActionController::Base # empty method not used anywhere to ensure methods like # `status` and `location` aren't called on `redirect_to` calls - def status; render :text => 'called status'; end - def location; render :text => 'called location'; end + def status; render plain: 'called status'; end + def location; render plain: 'called location'; end def simple_redirect redirect_to :action => "hello_world" diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index b1ad16bc55..3773900cc4 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -28,7 +28,7 @@ class RenderJsonTest < ActionController::TestCase end def render_json_render_to_string - render :text => render_to_string(:json => '[]') + render plain: render_to_string(json: '[]') end def render_json_hello_world diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 43e992d432..9acdc29aeb 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -10,16 +10,16 @@ class TestControllerWithExtraEtags < ActionController::Base etag { nil } def fresh - render text: "stale" if stale?(etag: '123', template: false) + render plain: "stale" if stale?(etag: '123', template: false) end def array - render text: "stale" if stale?(etag: %w(1 2 3), template: false) + render plain: "stale" if stale?(etag: %w(1 2 3), template: false) end def with_template if stale? template: 'test/hello_world' - render text: 'stale' + render plain: 'stale' end end end @@ -622,7 +622,7 @@ class HttpCacheForeverTest < ActionController::TestCase class HttpCacheForeverController < ActionController::Base def cache_me_forever http_cache_forever(public: params[:public], version: params[:version] || 'v1') do - render text: 'hello' + render plain: 'hello' end end end diff --git a/actionpack/test/controller/request/test_request_test.rb b/actionpack/test/controller/request/test_request_test.rb index 77a2f68b1c..e5d698d5c2 100644 --- a/actionpack/test/controller/request/test_request_test.rb +++ b/actionpack/test/controller/request/test_request_test.rb @@ -1,11 +1,7 @@ require 'abstract_unit' require 'stringio' -class ActionController::TestRequestTest < ActiveSupport::TestCase - - def setup - @request = ActionController::TestRequest.new - end +class ActionController::TestRequestTest < ActionController::TestCase def test_test_request_has_session_options_initialized assert @request.session_options diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 82c808754c..868520a219 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -13,7 +13,7 @@ module RequestForgeryProtectionActions end def unsafe - render :text => 'pwn' + render plain: 'pwn' end def meta @@ -484,11 +484,10 @@ end class FreeCookieControllerTest < ActionController::TestCase def setup @controller = FreeCookieController.new - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @token = "cf50faa3fe97702ca1ae" SecureRandom.stubs(:base64).returns(@token) + super end def test_should_not_render_form_with_token_tag diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 2cfd0b8023..e767323773 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -43,8 +43,8 @@ class RescueController < ActionController::Base rescue_from NotAllowed, :with => proc { head :forbidden } rescue_from 'RescueController::NotAllowedToRescueAsString', :with => proc { head :forbidden } - rescue_from InvalidRequest, :with => proc { |exception| render :text => exception.message } - rescue_from 'InvalidRequestToRescueAsString', :with => proc { |exception| render :text => exception.message } + rescue_from InvalidRequest, with: proc { |exception| render plain: exception.message } + rescue_from 'InvalidRequestToRescueAsString', with: proc { |exception| render plain: exception.message } rescue_from BadGateway do head 502 @@ -54,18 +54,18 @@ class RescueController < ActionController::Base end rescue_from ResourceUnavailable do |exception| - render :text => exception.message + render plain: exception.message end rescue_from 'ResourceUnavailableToRescueAsString' do |exception| - render :text => exception.message + render plain: exception.message end rescue_from ActionView::TemplateError do - render :text => 'action_view templater error' + render plain: 'action_view templater error' end rescue_from IOError do - render :text => 'io error' + render plain: 'io error' end before_action(only: :before_action_raises) { raise 'umm nice' } @@ -74,7 +74,7 @@ class RescueController < ActionController::Base end def raises - render :text => 'already rendered' + render plain: 'already rendered' raise "don't panic!" end @@ -302,7 +302,7 @@ class RescueTest < ActionDispatch::IntegrationTest rescue_from RecordInvalid, :with => :show_errors def foo - render :text => "foo" + render plain: "foo" end def invalid @@ -315,7 +315,7 @@ class RescueTest < ActionDispatch::IntegrationTest protected def show_errors(exception) - render :text => exception.message + render plain: exception.message end end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index f3da2df3ef..5a279639cc 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -1207,8 +1207,6 @@ class ResourcesTest < ActionController::TestCase @controller = "#{options[:options][:controller].camelize}Controller".constantize.new @controller.singleton_class.include(@routes.url_helpers) - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new get :index, params: options[:options] options[:options].delete :action @@ -1277,8 +1275,6 @@ class ResourcesTest < ActionController::TestCase (options[:options] ||= {})[:controller] ||= singleton_name.to_s.pluralize @controller = "#{options[:options][:controller].camelize}Controller".constantize.new @controller.singleton_class.include(@routes.url_helpers) - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new get :show, params: options[:options] options[:options].delete :action diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 36c57ec9b2..c0ddcf7f50 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -34,8 +34,6 @@ class SendFileTest < ActionController::TestCase def setup @controller = SendFileController.new - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new end def test_file_nostream diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 86ffb898ad..1c5de983d8 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -6,78 +6,78 @@ require 'rails/engine' class TestCaseTest < ActionController::TestCase class TestController < ActionController::Base def no_op - render text: 'dummy' + render plain: 'dummy' end def set_flash flash["test"] = ">#{flash["test"]}<" - render text: 'ignore me' + render plain: 'ignore me' end def delete_flash flash.delete("test") - render :text => 'ignore me' + render plain: 'ignore me' end def set_flash_now flash.now["test_now"] = ">#{flash["test_now"]}<" - render text: 'ignore me' + render plain: 'ignore me' end def set_session session['string'] = 'A wonder' session[:symbol] = 'it works' - render text: 'Success' + render plain: 'Success' end def reset_the_session reset_session - render text: 'ignore me' + render plain: 'ignore me' end def render_raw_post raise ActiveSupport::TestCase::Assertion, "#raw_post is blank" if request.raw_post.blank? - render text: request.raw_post + render plain: request.raw_post end def render_body - render text: request.body.read + render plain: request.body.read end def test_params - render text: params.inspect + render plain: ::JSON.dump(params.to_unsafe_h) end def test_query_parameters - render text: request.query_parameters.inspect + render plain: ::JSON.dump(request.query_parameters) end def test_request_parameters - render text: request.request_parameters.inspect + render plain: request.request_parameters.inspect end def test_uri - render text: request.fullpath + render plain: request.fullpath end def test_format - render text: request.format + render plain: request.format end def test_query_string - render text: request.query_string + render plain: request.query_string end def test_protocol - render text: request.protocol + render plain: request.protocol end def test_headers - render text: request.headers.env.to_json + render plain: request.headers.env.to_json end def test_html_output - render text: <<HTML + render plain: <<HTML <html> <body> <a href="/"><img src="/images/button.png" /></a> @@ -99,7 +99,7 @@ HTML def test_xml_output response.content_type = "application/xml" - render text: <<XML + render plain: <<XML <?xml version="1.0" encoding="UTF-8"?> <root> <area>area is an empty tag in HTML, raising an error if not in xml mode</area> @@ -108,15 +108,15 @@ XML end def test_only_one_param - render text: (params[:left] && params[:right]) ? "EEP, Both here!" : "OK" + render plain: (params[:left] && params[:right]) ? "EEP, Both here!" : "OK" end def test_remote_addr - render text: (request.remote_addr || "not specified") + render plain: (request.remote_addr || "not specified") end def test_file_upload - render text: params[:file].size + render plain: params[:file].size end def test_send_file @@ -158,8 +158,6 @@ XML def setup super @controller = TestController.new - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @request.env['PATH_INFO'] = nil @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do @@ -172,7 +170,7 @@ XML before_action { @dynamic_opt = 'opt' } def test_url_options_reset - render text: url_for(params) + render plain: url_for(params) end def default_url_options @@ -229,7 +227,7 @@ XML def test_document_body_and_params_with_post post :test_params, params: { id: 1 } - assert_equal(%({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}), @response.body) + assert_equal({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}, ::JSON.parse(@response.body)) end def test_document_body_with_post @@ -485,7 +483,7 @@ XML assert_deprecated { get :test_params, page: { name: "Page name", month: '4', year: '2004', day: '6' } } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( { 'controller' => 'test_case_test/test', 'action' => 'test_params', @@ -504,7 +502,7 @@ XML day: '6' } } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( { 'controller' => 'test_case_test/test', 'action' => 'test_params', @@ -516,8 +514,8 @@ XML def test_query_param_named_action get :test_query_parameters, params: {action: 'foobar'} - parsed_params = eval(@response.body) - assert_equal({action: 'foobar'}, parsed_params) + parsed_params = JSON.parse(@response.body) + assert_equal({'action' => 'foobar'}, parsed_params) end def test_request_param_named_action @@ -536,7 +534,7 @@ XML } }, session: { 'foo' => 'bar' }, flash: { notice: 'created' } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', 'page' => {'name' => "Page name", 'month' => '4', 'year' => '2004', 'day' => '6'}}, @@ -551,7 +549,7 @@ XML get :test_params, params: { page: { name: "Page name", month: 4, year: 2004, day: 6 } } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', 'page' => {'name' => "Page name", 'month' => '4', 'year' => '2004', 'day' => '6'}}, @@ -561,17 +559,17 @@ XML def test_params_passing_with_fixnums_when_not_html_request get :test_params, params: { format: 'json', count: 999 } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', - 'format' => 'json', 'count' => 999 }, + 'format' => 'json', 'count' => '999' }, parsed_params ) end def test_params_passing_path_parameter_is_string_when_not_html_request get :test_params, params: { format: 'json', id: 1 } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', 'format' => 'json', 'id' => '1' }, @@ -581,7 +579,7 @@ XML def test_deprecated_params_passing_path_parameter_is_string_when_not_html_request assert_deprecated { get :test_params, format: 'json', id: 1 } - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', 'format' => 'json', 'id' => '1' }, @@ -595,7 +593,7 @@ XML frozen: 'icy'.freeze, frozens: ['icy'.freeze].freeze, deepfreeze: { frozen: 'icy'.freeze }.freeze } end - parsed_params = eval(@response.body) + parsed_params = ::JSON.parse(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', 'frozen' => 'icy', 'frozens' => ['icy'], 'deepfreeze' => { 'frozen' => 'icy' }}, @@ -693,13 +691,13 @@ XML def test_deprecated_xhr_with_params assert_deprecated { xhr :get, :test_params, params: { id: 1 } } - assert_equal(%({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}), @response.body) + assert_equal({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}, ::JSON.parse(@response.body)) end def test_xhr_with_params get :test_params, params: { id: 1 }, xhr: true - assert_equal(%({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}), @response.body) + assert_equal({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}, ::JSON.parse(@response.body)) end def test_xhr_with_session @@ -720,12 +718,6 @@ XML assert_equal 'it works', session[:symbol], "Test session hash should allow indifferent access" end - def test_header_properly_reset_after_get_request - get :test_params - @request.recycle! - assert_nil @request.instance_variable_get("@request_method") - end - def test_deprecated_params_reset_between_post_requests assert_deprecated { post :no_op, foo: "bar" } assert_equal "bar", @request.params[:foo] @@ -916,7 +908,7 @@ XML filename = 'mona_lisa.jpg' path = "#{FILES_DIR}/#{filename}" assert_deprecated { - post :test_file_upload, file: ActionDispatch::Http::UploadedFile.new(filename: path, type: "image/jpg", tempfile: File.open(path)) + post :test_file_upload, file: Rack::Test::UploadedFile.new(path, "image/jpg", true) } assert_equal '159528', @response.body end @@ -925,7 +917,7 @@ XML filename = 'mona_lisa.jpg' path = "#{FILES_DIR}/#{filename}" post :test_file_upload, params: { - file: ActionDispatch::Http::UploadedFile.new(filename: path, type: "image/jpg", tempfile: File.open(path)) + file: Rack::Test::UploadedFile.new(path, "image/jpg", true) } assert_equal '159528', @response.body end @@ -957,10 +949,11 @@ class ResponseDefaultHeadersTest < ActionController::TestCase end end - setup do + def before_setup @original = ActionDispatch::Response.default_headers @defaults = { 'A' => '1', 'B' => '2' } ActionDispatch::Response.default_headers = @defaults + super end teardown do @@ -970,8 +963,6 @@ class ResponseDefaultHeadersTest < ActionController::TestCase def setup super @controller = TestController.new - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new @request.env['PATH_INFO'] = nil @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do @@ -1006,7 +997,7 @@ module EngineControllerTests class BarController < ActionController::Base def index - render text: 'bar' + render plain: 'bar' end end @@ -1092,7 +1083,7 @@ class AnonymousControllerTest < ActionController::TestCase def setup @controller = Class.new(ActionController::Base) do def index - render text: params[:controller] + render plain: params[:controller] end end.new @@ -1113,11 +1104,11 @@ class RoutingDefaultsTest < ActionController::TestCase def setup @controller = Class.new(ActionController::Base) do def post - render text: request.fullpath + render plain: request.fullpath end def project - render text: request.fullpath + render plain: request.fullpath end end.new diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index d9a1ae7d4f..5f2abc9606 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -1,7 +1,7 @@ require 'abstract_unit' require 'controller/fake_controllers' -class UrlRewriterTests < ActiveSupport::TestCase +class UrlRewriterTests < ActionController::TestCase class Rewriter def initialize(request) @options = { @@ -16,7 +16,6 @@ class UrlRewriterTests < ActiveSupport::TestCase end def setup - @request = ActionController::TestRequest.new @params = {} @rewriter = Rewriter.new(@request) #.new(@request, @params) @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 21fa670bb6..b26f037c36 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -5,16 +5,22 @@ class WebServiceTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base def assign_parameters if params[:full] - render :text => dump_params_keys + render plain: dump_params_keys else - render :text => (params.keys - ['controller', 'action']).sort.join(", ") + render plain: (params.keys - ['controller', 'action']).sort.join(", ") end end def dump_params_keys(hash = params) hash.keys.sort.inject("") do |s, k| value = hash[k] - value = Hash === value ? "(#{dump_params_keys(value)})" : "" + + if value.is_a?(Hash) || value.is_a?(ActionController::Parameters) + value = "(#{dump_params_keys(value)})" + else + value = "" + end + s << ", " unless s.empty? s << "#{k}#{value}" end diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index 512f3a8a7a..5cfa5f7b3b 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -1,5 +1,5 @@ require 'abstract_unit' -require 'active_support/concurrency/latch' +require 'concurrent/atomics' module ActionController module Live @@ -27,18 +27,18 @@ module ActionController end def test_parallel - latch = ActiveSupport::Concurrency::Latch.new + latch = Concurrent::CountDownLatch.new t = Thread.new { @response.stream.write 'foo' - latch.await + latch.wait @response.stream.close } @response.await_commit @response.each do |part| assert_equal 'foo', part - latch.release + latch.count_down end assert t.join end @@ -62,15 +62,15 @@ module ActionController def test_headers_cannot_be_written_after_webserver_reads @response.stream.write 'omg' - latch = ActiveSupport::Concurrency::Latch.new + latch = Concurrent::CountDownLatch.new t = Thread.new { @response.stream.each do |chunk| - latch.release + latch.count_down end } - latch.await + latch.wait assert @response.headers.frozen? e = assert_raises(ActionDispatch::IllegalStateError) do @response.headers['Content-Length'] = "zomg" diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index f90d5499d7..d75e31db62 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -73,26 +73,26 @@ module TestGenerationPrefix include RailsApplication.routes.mounted_helpers def index - render :text => posts_path + render plain: posts_path end def show - render :text => post_path(:id => params[:id]) + render plain: post_path(id: params[:id]) end def url_to_application path = main_app.url_for(:controller => "outside_engine_generating", :action => "index", :only_path => true) - render :text => path + render plain: path end def polymorphic_path_for_engine - render :text => polymorphic_path(Post.new) + render plain: polymorphic_path(Post.new) end def conflicting - render :text => "engine" + render plain: "engine" end end @@ -101,28 +101,28 @@ module TestGenerationPrefix include RailsApplication.routes.url_helpers def index - render :text => blog_engine.post_path(:id => 1) + render plain: blog_engine.post_path(id: 1) end def polymorphic_path_for_engine - render :text => blog_engine.polymorphic_path(Post.new) + render plain: blog_engine.polymorphic_path(Post.new) end def polymorphic_path_for_app - render :text => polymorphic_path(Post.new) + render plain: polymorphic_path(Post.new) end def polymorphic_with_url_for - render :text => blog_engine.url_for(Post.new) + render plain: blog_engine.url_for(Post.new) end def conflicting - render :text => "application" + render plain: "application" end def ivar_usage @blog_engine = "Not the engine route helper" - render :text => blog_engine.post_path(:id => 1) + render plain: blog_engine.post_path(id: 1) end end @@ -378,7 +378,7 @@ module TestGenerationPrefix include RailsApplication.routes.mounted_helpers def show - render :text => post_path(:id => params[:id]) + render plain: post_path(id: params[:id]) end end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index d77341bc64..c2300a0142 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -113,7 +113,7 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest def parse self.class.last_request_parameters = request.request_parameters - self.class.last_parameters = params + self.class.last_parameters = params.to_unsafe_h head :ok end end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 50f69c53cb..939a771c65 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -17,7 +17,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest end def read - render :text => "File: #{params[:uploaded_data].read}" + render plain: "File: #{params[:uploaded_data].read}" end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 27ee8603e4..ff63c10e8d 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -989,6 +989,7 @@ class RequestParameterFilter < BaseRequestTest [{'foo'=>'bar', 'baz'=>'foo'},{'foo'=>'[FILTERED]', 'baz'=>'[FILTERED]'},%w'foo baz'], [{'bar'=>{'foo'=>'bar','bar'=>'foo'}},{'bar'=>{'foo'=>'[FILTERED]','bar'=>'foo'}},%w'fo'], [{'foo'=>{'foo'=>'bar','bar'=>'foo'}},{'foo'=>'[FILTERED]'},%w'f banana'], + [{'deep'=>{'cc'=>{'code'=>'bar','bar'=>'foo'},'ss'=>{'code'=>'bar'}}},{'deep'=>{'cc'=>{'code'=>'[FILTERED]','bar'=>'foo'},'ss'=>{'code'=>'bar'}}},%w'deep.cc.code'], [{'baz'=>[{'foo'=>'baz'}, "1"]}, {'baz'=>[{'foo'=>'[FILTERED]'}, "1"]}, [/foo/]]] test_hashes.each do |before_filter, after_filter, filter_words| diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 7aca251066..5aa0215e8f 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -171,6 +171,8 @@ class ResponseTest < ActiveSupport::TestCase end test "read content type without charset" do + jruby_skip "https://github.com/jruby/jruby/issues/3138" + original = ActionDispatch::Response.default_charset begin ActionDispatch::Response.default_charset = 'utf-16' diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 7ef513b0c8..361ceca677 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -109,6 +109,8 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest end def test_concerns_executes_block_in_context_of_current_mapper + jruby_skip "https://github.com/jruby/jruby/issues/3143" + mapper = ActionDispatch::Routing::Mapper.new(ActionDispatch::Routing::RouteSet.new) mapper.concern :test_concern do resources :things diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 26b8636c2f..b18a9ab647 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3729,7 +3729,7 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest module ::Admin class StorageFilesController < ActionController::Base def index - render :text => "admin/storage_files#index" + render plain: "admin/storage_files#index" end end end @@ -3824,7 +3824,7 @@ class TestDefaultScope < ActionDispatch::IntegrationTest module ::Blog class PostsController < ActionController::Base def index - render :text => "blog/posts#index" + render plain: "blog/posts#index" end end end @@ -4164,13 +4164,13 @@ end class TestNamedRouteUrlHelpers < ActionDispatch::IntegrationTest class CategoriesController < ActionController::Base def show - render :text => "categories#show" + render plain: "categories#show" end end class ProductsController < ActionController::Base def show - render :text => "products#show" + render plain: "products#show" end end @@ -4265,7 +4265,7 @@ end class TestInvalidUrls < ActionDispatch::IntegrationTest class FooController < ActionController::Base def show - render :text => "foo#show" + render plain: "foo#show" end end @@ -4568,7 +4568,7 @@ end class TestDefaultUrlOptions < ActionDispatch::IntegrationTest class PostsController < ActionController::Base def archive - render :text => "posts#archive" + render plain: "posts#archive" end end diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb index 22a46b0930..e6a70358c8 100644 --- a/actionpack/test/dispatch/session/cache_store_test.rb +++ b/actionpack/test/dispatch/session/cache_store_test.rb @@ -18,11 +18,11 @@ class CacheStoreTest < ActionDispatch::IntegrationTest end def get_session_value - render :text => "foo: #{session[:foo].inspect}" + render plain: "foo: #{session[:foo].inspect}" end def get_session_id - render :text => "#{request.session.id}" + render plain: "#{request.session.id}" end def call_reset_session diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index e7f4235de8..715eb90566 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -16,25 +16,25 @@ class CookieStoreTest < ActionDispatch::IntegrationTest end def persistent_session_id - render :text => session[:session_id] + render plain: session[:session_id] end def set_session_value session[:foo] = "bar" - render :text => Rack::Utils.escape(Verifier.generate(session.to_hash)) + render plain: Rack::Utils.escape(Verifier.generate(session.to_hash)) end def get_session_value - render :text => "foo: #{session[:foo].inspect}" + render plain: "foo: #{session[:foo].inspect}" end def get_session_id - render :text => "id: #{request.session.id}" + render plain: "id: #{request.session.id}" end def get_class_after_reset_session reset_session - render :text => "class: #{session.class}" + render plain: "class: #{session.class}" end def call_session_clear diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index 9a5d5131c0..6e9107ecbf 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -19,11 +19,11 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest end def get_session_value - render :text => "foo: #{session[:foo].inspect}" + render plain: "foo: #{session[:foo].inspect}" end def get_session_id - render :text => "#{request.session.id}" + render plain: "#{request.session.id}" end def call_reset_session diff --git a/actionpack/test/dispatch/test_request_test.rb b/actionpack/test/dispatch/test_request_test.rb index cc35d4594e..ede1cec4e6 100644 --- a/actionpack/test/dispatch/test_request_test.rb +++ b/actionpack/test/dispatch/test_request_test.rb @@ -2,7 +2,7 @@ require 'abstract_unit' class TestRequestTest < ActiveSupport::TestCase test "sane defaults" do - env = ActionDispatch::TestRequest.new.env + env = ActionDispatch::TestRequest.create.env assert_equal "GET", env.delete("REQUEST_METHOD") assert_equal "off", env.delete("HTTPS") @@ -24,12 +24,10 @@ class TestRequestTest < ActiveSupport::TestCase assert_equal true, env.delete("rack.multithread") assert_equal true, env.delete("rack.multiprocess") assert_equal false, env.delete("rack.run_once") - - assert env.empty?, env.inspect end test "cookie jar" do - req = ActionDispatch::TestRequest.new + req = ActionDispatch::TestRequest.create({}) assert_equal({}, req.cookies) assert_equal nil, req.env["HTTP_COOKIE"] @@ -57,38 +55,38 @@ class TestRequestTest < ActiveSupport::TestCase test "does not complain when Rails.application is nil" do Rails.stubs(:application).returns(nil) - req = ActionDispatch::TestRequest.new + req = ActionDispatch::TestRequest.create({}) assert_equal false, req.env.empty? end test "default remote address is 0.0.0.0" do - req = ActionDispatch::TestRequest.new + req = ActionDispatch::TestRequest.create({}) assert_equal '0.0.0.0', req.remote_addr end test "allows remote address to be overridden" do - req = ActionDispatch::TestRequest.new('REMOTE_ADDR' => '127.0.0.1') + req = ActionDispatch::TestRequest.create('REMOTE_ADDR' => '127.0.0.1') assert_equal '127.0.0.1', req.remote_addr end test "default host is test.host" do - req = ActionDispatch::TestRequest.new + req = ActionDispatch::TestRequest.create({}) assert_equal 'test.host', req.host end test "allows host to be overridden" do - req = ActionDispatch::TestRequest.new('HTTP_HOST' => 'www.example.com') + req = ActionDispatch::TestRequest.create('HTTP_HOST' => 'www.example.com') assert_equal 'www.example.com', req.host end test "default user agent is 'Rails Testing'" do - req = ActionDispatch::TestRequest.new + req = ActionDispatch::TestRequest.create({}) assert_equal 'Rails Testing', req.user_agent end test "allows user agent to be overridden" do - req = ActionDispatch::TestRequest.new('HTTP_USER_AGENT' => 'GoogleBot') + req = ActionDispatch::TestRequest.create('HTTP_USER_AGENT' => 'GoogleBot') assert_equal 'GoogleBot', req.user_agent end diff --git a/actionpack/test/dispatch/test_response_test.rb b/actionpack/test/dispatch/test_response_test.rb index dc17668def..a4f9d56a6a 100644 --- a/actionpack/test/dispatch/test_response_test.rb +++ b/actionpack/test/dispatch/test_response_test.rb @@ -11,10 +11,9 @@ class TestResponseTest < ActiveSupport::TestCase end test "helpers" do - assert_response_code_range 200..299, :success? - assert_response_code_range [404], :missing? - assert_response_code_range 300..399, :redirect? - assert_response_code_range 500..599, :error? + assert_response_code_range 200..299, :successful? + assert_response_code_range [404], :not_found? + assert_response_code_range 300..399, :redirection? assert_response_code_range 500..599, :server_error? assert_response_code_range 400..499, :client_error? end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index ce1e1d0a6a..fd4ede4d1b 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -8,7 +8,7 @@ module TestUrlGeneration class ::MyRouteGeneratingController < ActionController::Base include Routes.url_helpers def index - render :text => foo_path + render plain: foo_path end end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 069d181674..1f6bb31cd4 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,8 @@ +* Allow defining explicit collection caching using a `# Template Collection: ...` + directive inside templates. + + *Dov Murik* + * Asset helpers raise `ArgumentError` when `nil` is passed as a source. *Anton Kolomiychuk* diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 8945575860..797d029317 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -137,6 +137,21 @@ module ActionView # The automatic cache multi read can be turned off like so: # # <%= render @notifications, cache: false %> + # + # === Explicit Collection Caching + # + # If the partial template doesn't start with a clean cache call as + # mentioned above, you can still benefit from collection caching by + # adding a special comment format anywhere in the template, like: + # + # <%# Template Collection: notification %> + # <% my_helper_that_calls_cache(some_arg, notification) do %> + # <%= notification.name %> + # <% end %> + # + # The pattern used to match these is <tt>/# Template Collection: (\S+)/</tt>, + # so it's important that you type it out just so. + # You can only declare one collection in a partial template file. def cache(name = {}, options = {}, &block) if controller.respond_to?(:perform_caching) && controller.perform_caching safe_concat(fragment_for(cache_fragment_name(name, options), options, &block)) diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 9c8edc69a9..fbd7261477 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -70,7 +70,7 @@ module ActionView # distance_of_time_in_words(Time.now, Time.now) # => less than a minute # # With the <tt>scope</tt> option, you can define a custom scope for Rails - # to lookup the translation. + # to look up the translation. # # For example you can define the following in your locale (e.g. en.yml). # diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 1b7b188d65..8e729b3c39 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -35,8 +35,8 @@ module ActionView # <select name="post[person_id]" id="post_person_id"> # <option value="">None</option> # <option value="1">David</option> - # <option value="2" selected="selected">Sam</option> - # <option value="3">Tobias</option> + # <option value="2" selected="selected">Eileen</option> + # <option value="3">Rafael</option> # </select> # # * <tt>:prompt</tt> - set to true or a prompt string. When the select element doesn't have a value yet, this prepends an option with a generic prompt -- "Please select" -- or the given prompt string. @@ -48,8 +48,8 @@ module ActionView # <select name="post[person_id]" id="post_person_id"> # <option value="">Select Person</option> # <option value="1">David</option> - # <option value="2">Sam</option> - # <option value="3">Tobias</option> + # <option value="2">Eileen</option> + # <option value="3">Rafael</option> # </select> # # * <tt>:index</tt> - like the other form helpers, +select+ can accept an <tt>:index</tt> option to manually set the ID used in the resulting output. Unlike other helpers, +select+ expects this @@ -112,8 +112,8 @@ module ActionView # <select name="post[person_id]" id="post_person_id"> # <option value=""></option> # <option value="1" selected="selected">David</option> - # <option value="2">Sam</option> - # <option value="3">Tobias</option> + # <option value="2">Eileen</option> + # <option value="3">Rafael</option> # </select> # # assuming the associated person has ID 1. diff --git a/actionview/lib/action_view/helpers/rendering_helper.rb b/actionview/lib/action_view/helpers/rendering_helper.rb index 827932d8e2..c98f2d74a8 100644 --- a/actionview/lib/action_view/helpers/rendering_helper.rb +++ b/actionview/lib/action_view/helpers/rendering_helper.rb @@ -18,7 +18,7 @@ module ActionView # performs HTML escape on the string first. Setting the content type as # <tt>text/html</tt>. # * <tt>:body</tt> - Renders the text passed in, and inherits the content - # type of <tt>text/html</tt> from <tt>ActionDispatch::Response</tt> + # type of <tt>text/plain</tt> from <tt>ActionDispatch::Response</tt> # object. # # If no options hash is passed or :update specified, the default is to render a partial and use the second parameter diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index c216d4401f..6a3d01667d 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -206,6 +206,11 @@ module ActionView # +plural+ is supplied, it will use that when count is > 1, otherwise # it will use the Inflector to determine the plural form. # + # 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 + # # pluralize(1, 'person') # # => 1 person # @@ -217,11 +222,14 @@ module ActionView # # pluralize(0, 'person') # # => 0 people - def pluralize(count, singular, plural = nil) + # + # pluralize(2, 'Person', locale: :de) + # # => 2 Personen + def pluralize(count, singular, plural = nil, locale: nil) word = if (count == 1 || count =~ /^1(\.0+)?$/) singular else - plural || singular.pluralize + plural || singular.pluralize(locale) end "#{count || 0} #{word}" diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 817b30dd2f..1c8359005b 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -6,10 +6,11 @@ require 'action_view/template/resolver' module ActionView # = Action View Lookup Context # - # <tt>LookupContext</tt> is the object responsible to hold all information required to lookup - # templates, i.e. view paths and details. The LookupContext is also responsible to - # generate a key, given to view paths, used in the resolver cache lookup. Since - # this key is generated just once during the request, it speeds up all cache accesses. + # <tt>LookupContext</tt> is the object responsible for holding all information + # required for looking up templates, i.e. view paths and details. + # <tt>LookupContext</tt> is also responsible for generating a key, given to + # view paths, used in the resolver cache lookup. Since this key is generated + # only once during the request, it speeds up all cache accesses. class LookupContext #:nodoc: attr_accessor :prefixes, :rendered_format diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 91b7e845d6..86a80a5421 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -103,7 +103,7 @@ module ActionView view_renderer.render(context, options) end - # Assign the rendered format to lookup context. + # Assign the rendered format to look up context. def _process_format(format, options = {}) #:nodoc: super lookup_context.formats = [format.to_sym] diff --git a/actionview/lib/action_view/routing_url_for.rb b/actionview/lib/action_view/routing_url_for.rb index 0371db07dc..20d6b9a64c 100644 --- a/actionview/lib/action_view/routing_url_for.rb +++ b/actionview/lib/action_view/routing_url_for.rb @@ -92,6 +92,16 @@ module ActionView end super(options) + when ActionController::Parameters + unless options.key?(:only_path) + if options[:host].nil? + options[:only_path] = _generate_paths_by_default + else + options[:only_path] = false + end + end + + super(options) when :back _back_url when Array diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 377ceb534a..d8585514d5 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -130,7 +130,7 @@ module ActionView @source = source @identifier = identifier @handler = handler - @cache_name = extract_resource_cache_call_name + @cache_name = extract_resource_cache_name @compiled = false @original_encoding = nil @locals = details[:locals] || [] @@ -351,9 +351,18 @@ module ActionView ActiveSupport::Notifications.instrument("#{action}.action_view", payload, &block) end - def extract_resource_cache_call_name - $1 if @handler.respond_to?(:resource_cache_call_pattern) && - @source =~ @handler.resource_cache_call_pattern + EXPLICIT_COLLECTION = /# Template Collection: (?<resource_name>\w+)/ + + def extract_resource_cache_name + if match = @source.match(EXPLICIT_COLLECTION) || resource_cache_call_match + match[:resource_name] + end + end + + def resource_cache_call_match + if @handler.respond_to?(:resource_cache_call_pattern) + @source.match(@handler.resource_cache_call_pattern) + end end def inferred_cache_name diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb index da96347e4d..1f8459c24b 100644 --- a/actionview/lib/action_view/template/handlers/erb.rb +++ b/actionview/lib/action_view/template/handlers/erb.rb @@ -125,7 +125,7 @@ module ActionView # Returns Regexp to extract a cached resource's name from a cache call at the # first line of a template. - # The extracted cache name is expected in $1. + # The extracted cache name is captured as :resource_name. # # <% cache notification do %> # => notification # @@ -138,7 +138,14 @@ module ActionView # # <% cache notification.event do %> # => nil def resource_cache_call_pattern - /\A(?:<%#.*%>)*\s*<%\s*cache\(?\s*(\w+)[\s\)]/m + /\A + (?:<%\#.*%>)* # optional initial comment + \s* # followed by optional spaces or newlines + <%\s*cache[\(\s] # followed by an ERB call to cache + \s* # followed by optional spaces or newlines + (?<resource_name>\w+) # capture the cache call argument as :resource_name + [\s\)] # followed by a space or close paren + /xm end private diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 06810ad14d..c4bc26ca8a 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -24,8 +24,8 @@ module ActionView def initialize super self.class.controller_path = "" - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new + @request = ActionController::TestRequest.create + @response = ActionDispatch::TestResponse.new @request.env.delete('PATH_INFO') @params = {} diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index ebca598337..37722013ce 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -36,9 +36,9 @@ module ActionView self.class._prefixes end - # <tt>LookupContext</tt> is the object responsible to hold all information required to lookup - # templates, i.e. view paths and details. Check <tt>ActionView::LookupContext</tt> for more - # information. + # <tt>LookupContext</tt> is the object responsible for holding all + # information required for looking up templates, i.e. view paths and + # details. Check <tt>ActionView::LookupContext</tt> for more information. def lookup_context @_lookup_context ||= ActionView::LookupContext.new(self.class._view_paths, details_for_lookup, _prefixes) diff --git a/actionview/test/actionpack/abstract/layouts_test.rb b/actionview/test/actionpack/abstract/layouts_test.rb index a6786d9b6b..80bc665b0a 100644 --- a/actionview/test/actionpack/abstract/layouts_test.rb +++ b/actionview/test/actionpack/abstract/layouts_test.rb @@ -52,7 +52,7 @@ module AbstractControllerTests end def overwrite_skip - render :text => "Hello text!" + render plain: "Hello text!" end end @@ -371,7 +371,7 @@ module AbstractControllerTests test "layout for anonymous controller" do klass = Class.new(WithString) do def index - render :text => 'index', :layout => true + render plain: 'index', layout: true end end diff --git a/actionview/test/actionpack/abstract/render_test.rb b/actionview/test/actionpack/abstract/render_test.rb index d09f91c1e2..e5721d6416 100644 --- a/actionview/test/actionpack/abstract/render_test.rb +++ b/actionview/test/actionpack/abstract/render_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'active_support/deprecation' module AbstractController module Testing @@ -33,7 +34,7 @@ module AbstractController end def text - render :text => "With Text" + render plain: "With Text" end def default diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index ebaf39a12d..8d048ddbcb 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -121,7 +121,7 @@ class TestController < ApplicationController # :ported: def render_hello_world_from_variable @person = "david" - render :text => "hello #{@person}" + render plain: "hello #{@person}" end # :ported: @@ -143,13 +143,13 @@ class TestController < ApplicationController # :ported: def render_text_hello_world - render :text => "hello world" + render plain: "hello world" end # :ported: def render_text_hello_world_with_layout @variable_for_layout = ", I am here!" - render :text => "hello world", :layout => true + render plain: "hello world", :layout => true end def hello_world_with_layout_false @@ -212,26 +212,26 @@ class TestController < ApplicationController # :ported: def render_custom_code - render :text => "hello world", :status => 404 + render plain: "hello world", :status => 404 end # :ported: def render_text_with_nil - render :text => nil + render plain: nil end # :ported: def render_text_with_false - render :text => false + render plain: false end def render_text_with_resource - render :text => Customer.new("David") + render plain: Customer.new("David") end # :ported: def render_nothing_with_appendix - render :text => "appended" + render plain: "appended" end # This test is testing 3 things: @@ -262,7 +262,7 @@ class TestController < ApplicationController # :ported: def blank_response - render :text => ' ' + render plain: ' ' end # :ported: @@ -294,7 +294,7 @@ class TestController < ApplicationController def hello_in_a_string @customers = [ Customer.new("david"), Customer.new("mary") ] - render :text => "How's there? " + render_to_string(:template => "test/list") + render plain: "How's there? " + render_to_string(:template => "test/list") end def accessing_params_in_template @@ -362,7 +362,7 @@ class TestController < ApplicationController def render_to_string_with_assigns @before = "i'm before the render" - render_to_string :text => "foo" + render_to_string plain: "foo" @after = "i'm after the render" render :template => "test/hello_world" end @@ -409,8 +409,8 @@ class TestController < ApplicationController # :ported: def double_render - render :text => "hello" - render :text => "world" + render plain: "hello" + render plain: "world" end def double_redirect @@ -419,13 +419,13 @@ class TestController < ApplicationController end def render_and_redirect - render :text => "hello" + render plain: "hello" redirect_to :action => "double_render" end def render_to_string_and_render - @stuff = render_to_string :text => "here is some cached stuff" - render :text => "Hi web users! #{@stuff}" + @stuff = render_to_string plain: "here is some cached stuff" + render plain: "Hi web users! #{@stuff}" end def render_to_string_with_inline_and_render @@ -454,7 +454,7 @@ class TestController < ApplicationController # :addressed: def render_text_with_assigns @hello = "world" - render :text => "foo" + render plain: "foo" end def render_with_assigns_option @@ -467,7 +467,7 @@ class TestController < ApplicationController def render_content_type_from_body response.content_type = Mime::RSS - render :text => "hello world!" + render body: "hello world!" end def render_using_layout_around_block @@ -770,7 +770,7 @@ class RenderTest < ActionController::TestCase # :ported: def test_do_with_render_text_and_layout get :render_text_hello_world_with_layout - assert_equal "<html>hello world, I am here!</html>", @response.body + assert_equal "{{hello world, I am here!}}\n", @response.body end # :ported: diff --git a/actionview/test/actionpack/controller/view_paths_test.rb b/actionview/test/actionpack/controller/view_paths_test.rb index 7fba9ff8ff..e99659c802 100644 --- a/actionview/test/actionpack/controller/view_paths_test.rb +++ b/actionview/test/actionpack/controller/view_paths_test.rb @@ -23,8 +23,8 @@ class ViewLoadPathsTest < ActionController::TestCase end def setup - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new + @request = ActionController::TestRequest.create + @response = ActionDispatch::TestResponse.new @controller = TestController.new @paths = TestController.view_paths end diff --git a/actionview/test/fixtures/actionpack/layouts/standard.text.erb b/actionview/test/fixtures/actionpack/layouts/standard.text.erb new file mode 100644 index 0000000000..a58afb1aa2 --- /dev/null +++ b/actionview/test/fixtures/actionpack/layouts/standard.text.erb @@ -0,0 +1 @@ +{{<%= yield %><%= @variable_for_layout %>}} diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index d17034e88f..d3b51cd629 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -194,14 +194,15 @@ class TestERBTemplate < ActiveSupport::TestCase [ "<%= 'Hello' %>", "<% cache_customer = 42 %>", - "<% cache customer.name do %><% end %>" + "<% cache customer.name do %><% end %>", + "<% my_cache customer do %><% end %>" ].each do |body| template = new_template(body, virtual_path: "test/foo/_customer") assert_not template.eligible_for_collection_caching?, "Template #{body.inspect} should not be eligible for collection caching" end end - def test_eligible_for_collection_caching_with_cache_call + def test_eligible_for_collection_caching_with_cache_call_or_explicit [ "<% cache customer do %><% end %>", "<% cache(customer) do %><% end %>", @@ -213,7 +214,8 @@ class TestERBTemplate < ActiveSupport::TestCase "<%# comment %><% cache customer do %><% end %>", "<%# comment %>\n<% cache customer do %><% end %>", "<%# comment\n line 2\n line 3 %>\n<% cache customer do %><% end %>", - "<%# comment 1 %>\n<%# comment 2 %>\n<% cache customer do %><% end %>" + "<%# comment 1 %>\n<%# comment 2 %>\n<% cache customer do %><% end %>", + "<%# comment 1 %>\n<%# Template Collection: customer %>\n<% my_cache customer do %><% end %>" ].each do |body| template = new_template(body, virtual_path: "test/foo/_customer") assert template.eligible_for_collection_caching?, "Template #{body.inspect} should be eligible for collection caching" diff --git a/actionview/test/template/text_helper_test.rb b/actionview/test/template/text_helper_test.rb index f1b84c4786..5791f33069 100644 --- a/actionview/test/template/text_helper_test.rb +++ b/actionview/test/template/text_helper_test.rb @@ -383,6 +383,18 @@ class TextHelperTest < ActionView::TestCase 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 + + assert_equal("2 People", pluralize(2, "Person", locale: :en)) + assert_equal("2 Personen", pluralize(2, "Person", locale: :de)) + + ActiveSupport::Inflector.inflections(:de).clear + end + def test_cycle_class value = Cycle.new("one", 2, "3") assert_equal("one", value.to_s) diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index cdb37879b6..8e462bfe5d 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -16,13 +16,13 @@ module ActiveJob end end - # Raised when an unsupported argument type is being set as job argument. We + # Raised when an unsupported argument type is set as a job argument. We # currently support NilClass, Fixnum, Float, String, TrueClass, FalseClass, - # Bignum and object that can be represented as GlobalIDs (ex: Active Record). - # Also raised if you set the key for a Hash something else than a string or - # a symbol. - class SerializationError < ArgumentError - end + # Bignum and objects that can be represented as GlobalIDs (ex: Active Record). + # Raised if you set the key for a Hash something else than a string or + # a symbol. Also raised when trying to serialize an object which can't be + # identified with a Global ID - such as an unpersisted Active Record model. + class SerializationError < ArgumentError; end module Arguments extend self @@ -59,7 +59,7 @@ module ActiveJob when *TYPE_WHITELIST argument when GlobalID::Identification - { GLOBALID_KEY => argument.to_global_id.to_s } + convert_to_global_id_hash(argument) when Array argument.map { |arg| serialize_argument(arg) } when ActiveSupport::HashWithIndifferentAccess @@ -147,5 +147,12 @@ module ActiveJob end end end + + def convert_to_global_id_hash(argument) + { GLOBALID_KEY => argument.to_global_id.to_s } + rescue URI::GID::MissingModelIdError + raise SerializationError, "Unable to serialize #{argument.class} " \ + "without an id. (Maybe you forgot to call save?)" + end end end diff --git a/activejob/test/cases/argument_serialization_test.rb b/activejob/test/cases/argument_serialization_test.rb index 8b9b62190f..933972a52b 100644 --- a/activejob/test/cases/argument_serialization_test.rb +++ b/activejob/test/cases/argument_serialization_test.rb @@ -88,6 +88,13 @@ class ArgumentSerializationTest < ActiveSupport::TestCase assert_equal "Job with argument: 2", JobBuffer.last_value end + test 'raises a friendly SerializationError for records without ids' do + err = assert_raises ActiveJob::SerializationError do + ActiveJob::Arguments.serialize [Person.new(nil)] + end + assert_match 'Unable to serialize Person without an id.', err.message + end + private def assert_arguments_unchanged(*args) assert_arguments_roundtrip args diff --git a/activemodel/README.rdoc b/activemodel/README.rdoc index 5c36b1277e..d954467387 100644 --- a/activemodel/README.rdoc +++ b/activemodel/README.rdoc @@ -154,7 +154,7 @@ behavior out of the box: * Making objects serializable - ActiveModel::Serialization provides a standard interface for your object + <tt>ActiveModel::Serialization</tt> provides a standard interface for your object to provide +to_json+ or +to_xml+ serialization. class SerialPerson diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index c1019169e1..5f1dde4aa3 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -87,8 +87,7 @@ module ActiveModel validates_with BlockValidator, _merge_attributes(attr_names), &block end - # :nodoc: - VALID_OPTIONS_FOR_VALIDATE = [:on, :if, :unless, :prepend].freeze + VALID_OPTIONS_FOR_VALIDATE = [:on, :if, :unless, :prepend].freeze # :nodoc: # Adds a validation method or block to the class. This is useful when # overriding the +validate+ instance method becomes too unwieldy and @@ -130,6 +129,9 @@ module ActiveModel # end # end # + # Note that the return value of validation methods is not relevant. + # It's not possible to halt the validate callback chain. + # # Options: # * <tt>:on</tt> - Specifies the contexts where this validation is active. # Runs in all validation contexts by default (nil). You can pass a symbol @@ -402,7 +404,7 @@ module ActiveModel protected def run_validations! #:nodoc: - run_callbacks :validate + _run_validate_callbacks errors.empty? end diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index b4301c23e4..4b58ef66e3 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -109,7 +109,7 @@ module ActiveModel # Overwrite run validations to include callbacks. def run_validations! #:nodoc: - run_callbacks(:validation) { super } + _run_validation_callbacks { super } end end end diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index 3fa63917d0..f6d171bec6 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -27,14 +27,6 @@ class ErrorsTest < ActiveModel::TestCase end end - def setup - @mock_generator = MiniTest::Mock.new - end - - def teardown - @mock_generator.verify - end - def test_delete errors = ActiveModel::Errors.new(self) errors[:foo] << 'omg' @@ -307,8 +299,7 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_empty generates message" do person = Person.new - @mock_generator.expect(:call, nil, [:name, :empty, {}]) - person.errors.stub(:generate_message, @mock_generator) do + assert_called_with(person.errors, :generate_message, [:name, :empty, {}]) do assert_deprecated do person.errors.add_on_empty :name end @@ -317,9 +308,8 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_empty generates message for multiple attributes" do person = Person.new - @mock_generator.expect(:call, nil, [:name, :empty, {}]) - @mock_generator.expect(:call, nil, [:age, :empty, {}]) - person.errors.stub(:generate_message, @mock_generator) do + expected_calls = [ [:name, :empty, {}], [:age, :empty, {}] ] + assert_called_with(person.errors, :generate_message, expected_calls) do assert_deprecated do person.errors.add_on_empty [:name, :age] end @@ -328,8 +318,7 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_empty generates message with custom default message" do person = Person.new - @mock_generator.expect(:call, nil, [:name, :empty, { message: 'custom' }]) - person.errors.stub(:generate_message, @mock_generator) do + assert_called_with(person.errors, :generate_message, [:name, :empty, { message: 'custom' }]) do assert_deprecated do person.errors.add_on_empty :name, message: 'custom' end @@ -339,8 +328,7 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_empty generates message with empty string value" do person = Person.new person.name = '' - @mock_generator.expect(:call, nil, [:name, :empty, {}]) - person.errors.stub(:generate_message, @mock_generator) do + assert_called_with(person.errors, :generate_message, [:name, :empty, {}]) do assert_deprecated do person.errors.add_on_empty :name end @@ -349,8 +337,7 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_blank generates message" do person = Person.new - @mock_generator.expect(:call, nil, [:name, :blank, {}]) - person.errors.stub(:generate_message, @mock_generator) do + assert_called_with(person.errors, :generate_message, [:name, :blank, {}]) do assert_deprecated do person.errors.add_on_blank :name end @@ -359,9 +346,8 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_blank generates message for multiple attributes" do person = Person.new - @mock_generator.expect(:call, nil, [:name, :blank, {}]) - @mock_generator.expect(:call, nil, [:age, :blank, {}]) - person.errors.stub(:generate_message, @mock_generator) do + expected_calls = [ [:name, :blank, {}], [:age, :blank, {}] ] + assert_called_with(person.errors, :generate_message, expected_calls) do assert_deprecated do person.errors.add_on_blank [:name, :age] end @@ -370,8 +356,7 @@ class ErrorsTest < ActiveModel::TestCase test "add_on_blank generates message with custom default message" do person = Person.new - @mock_generator.expect(:call, nil, [:name, :blank, { message: 'custom' }]) - person.errors.stub(:generate_message, @mock_generator) do + assert_called_with(person.errors, :generate_message, [:name, :blank, { message: 'custom' }]) do assert_deprecated do person.errors.add_on_blank :name, message: 'custom' end diff --git a/activemodel/test/cases/helper.rb b/activemodel/test/cases/helper.rb index 0d179ea9ad..c100646837 100644 --- a/activemodel/test/cases/helper.rb +++ b/activemodel/test/cases/helper.rb @@ -11,6 +11,7 @@ ActiveSupport::Deprecation.debug = true I18n.enforce_available_locales = false require 'active_support/testing/autorun' +require 'active_support/testing/method_call_assertions' require 'minitest/mock' @@ -22,3 +23,7 @@ end def jruby_skip(message = '') skip message if defined?(JRUBY_VERSION) end + +class ActiveModel::TestCase + include ActiveSupport::Testing::MethodCallAssertions +end diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index 70b93a202b..ce9a782f73 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -11,7 +11,6 @@ class I18nValidationTest < ActiveModel::TestCase I18n.load_path.clear I18n.backend = I18n::Backend::Simple.new I18n.backend.store_translations('en', errors: { messages: { custom: nil } }) - @mock_generator = MiniTest::Mock.new end def teardown @@ -19,7 +18,6 @@ class I18nValidationTest < ActiveModel::TestCase I18n.load_path.replace @old_load_path I18n.backend = @old_backend I18n.backend.reload! - @mock_generator.verify end def test_full_message_encoding @@ -32,8 +30,7 @@ class I18nValidationTest < ActiveModel::TestCase def test_errors_full_messages_translates_human_attribute_name_for_model_attributes @person.errors.add(:name, 'not found') - @mock_generator.expect(:call, "Person's name", [:name, default: 'Name']) - Person.stub(:human_attribute_name, @mock_generator) do + assert_called_with(Person, :human_attribute_name, [:name, default: 'Name'], returns: "Person's name") do assert_equal ["Person's name not found"], @person.errors.full_messages end end @@ -58,206 +55,175 @@ class I18nValidationTest < ActiveModel::TestCase [ "given option that is not reserved", { format: "jpg" }, { format: "jpg" }] ] - # validates_confirmation_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_confirmation_of on generated message #{name}" do Person.validates_confirmation_of :title, validation_options @person.title_confirmation = 'foo' - @mock_generator.expect(:call, nil, [:title_confirmation, :confirmation, generate_message_options.merge(attribute: 'Title')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title_confirmation, :confirmation, generate_message_options.merge(attribute: 'Title')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_acceptance_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_acceptance_of on generated message #{name}" do Person.validates_acceptance_of :title, validation_options.merge(allow_nil: false) - @mock_generator.expect(:call, nil, [:title, :accepted, generate_message_options]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :accepted, generate_message_options] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_presence_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_presence_of on generated message #{name}" do Person.validates_presence_of :title, validation_options - @mock_generator.expect(:call, nil, [:title, :blank, generate_message_options]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :blank, generate_message_options] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_length_of :within too short w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_length_of for :within on generated message when too short #{name}" do Person.validates_length_of :title, validation_options.merge(within: 3..5) - @mock_generator.expect(:call, nil, [:title, :too_short, generate_message_options.merge(count: 3)]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :too_short, generate_message_options.merge(count: 3)] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_length_of :within too long w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_length_of for :too_long generated message #{name}" do Person.validates_length_of :title, validation_options.merge(within: 3..5) @person.title = 'this title is too long' - @mock_generator.expect(:call, nil, [:title, :too_long, generate_message_options.merge(count: 5)]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :too_long, generate_message_options.merge(count: 5)] + assert_called_with(@person.errors, :generate_message, ) do @person.valid? end end end - # validates_length_of :is w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_length_of for :is on generated message #{name}" do Person.validates_length_of :title, validation_options.merge(is: 5) - @mock_generator.expect(:call, nil, [:title, :wrong_length, generate_message_options.merge(count: 5)]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :wrong_length, generate_message_options.merge(count: 5)] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_format_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_format_of on generated message #{name}" do Person.validates_format_of :title, validation_options.merge(with: /\A[1-9][0-9]*\z/) @person.title = '72x' - @mock_generator.expect(:call, nil, [:title, :invalid, generate_message_options.merge(value: '72x')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :invalid, generate_message_options.merge(value: '72x')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_inclusion_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_inclusion_of on generated message #{name}" do Person.validates_inclusion_of :title, validation_options.merge(in: %w(a b c)) @person.title = 'z' - @mock_generator.expect(:call, nil, [:title, :inclusion, generate_message_options.merge(value: 'z')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :inclusion, generate_message_options.merge(value: 'z')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_inclusion_of using :within w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_inclusion_of using :within on generated message #{name}" do Person.validates_inclusion_of :title, validation_options.merge(within: %w(a b c)) @person.title = 'z' - @mock_generator.expect(:call, nil, [:title, :inclusion, generate_message_options.merge(value: 'z')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :inclusion, generate_message_options.merge(value: 'z')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_exclusion_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_exclusion_of generated message #{name}" do Person.validates_exclusion_of :title, validation_options.merge(in: %w(a b c)) @person.title = 'a' - @mock_generator.expect(:call, nil, [:title, :exclusion, generate_message_options.merge(value: 'a')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :exclusion, generate_message_options.merge(value: 'a')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_exclusion_of using :within w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_exclusion_of using :within generated message #{name}" do Person.validates_exclusion_of :title, validation_options.merge(within: %w(a b c)) @person.title = 'a' - @mock_generator.expect(:call, nil, [:title, :exclusion, generate_message_options.merge(value: 'a')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :exclusion, generate_message_options.merge(value: 'a')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_numericality_of without :only_integer w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of generated message #{name}" do Person.validates_numericality_of :title, validation_options @person.title = 'a' - @mock_generator.expect(:call, nil, [:title, :not_a_number, generate_message_options.merge(value: 'a')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :not_a_number, generate_message_options.merge(value: 'a')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_numericality_of with :only_integer w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of for :only_integer on generated message #{name}" do Person.validates_numericality_of :title, validation_options.merge(only_integer: true) @person.title = '0.0' - @mock_generator.expect(:call, nil, [:title, :not_an_integer, generate_message_options.merge(value: '0.0')]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :not_an_integer, generate_message_options.merge(value: '0.0')] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_numericality_of :odd w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of for :odd on generated message #{name}" do Person.validates_numericality_of :title, validation_options.merge(only_integer: true, odd: true) @person.title = 0 - @mock_generator.expect(:call, nil, [:title, :odd, generate_message_options.merge(value: 0)]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :odd, generate_message_options.merge(value: 0)] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - # validates_numericality_of :less_than w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of for :less_than on generated message #{name}" do Person.validates_numericality_of :title, validation_options.merge(only_integer: true, less_than: 0) @person.title = 1 - @mock_generator.expect(:call, nil, [:title, :less_than, generate_message_options.merge(value: 1, count: 0)]) - @person.errors.stub(:generate_message, @mock_generator) do + call = [:title, :less_than, generate_message_options.merge(value: 1, count: 0)] + assert_called_with(@person.errors, :generate_message, call) do @person.valid? end end end - - # To make things DRY this macro is defined to define 3 tests for every validation case. + # To make things DRY this macro is created to define 3 tests for every validation case. def self.set_expectations_for_validation(validation, error_type, &block_that_sets_validation) if error_type == :confirmation attribute = :title_confirmation else attribute = :title end - # test "validates_confirmation_of finds custom model key translation when blank" + test "#{validation} finds custom model key translation when #{error_type}" do I18n.backend.store_translations 'en', activemodel: { errors: { models: { person: { attributes: { attribute => { error_type => 'custom message' } } } } } } I18n.backend.store_translations 'en', errors: { messages: { error_type => 'global message'}} @@ -267,7 +233,6 @@ class I18nValidationTest < ActiveModel::TestCase assert_equal ['custom message'], @person.errors[attribute] end - # test "validates_confirmation_of finds custom model key translation with interpolation when blank" test "#{validation} finds custom model key translation with interpolation when #{error_type}" do I18n.backend.store_translations 'en', activemodel: { errors: { models: { person: { attributes: { attribute => { error_type => 'custom message with %{extra}' } } } } } } I18n.backend.store_translations 'en', errors: { messages: {error_type => 'global message'} } @@ -277,7 +242,6 @@ class I18nValidationTest < ActiveModel::TestCase assert_equal ['custom message with extra information'], @person.errors[attribute] end - # test "validates_confirmation_of finds global default key translation when blank" test "#{validation} finds global default key translation when #{error_type}" do I18n.backend.store_translations 'en', errors: { messages: {error_type => 'global message'} } @@ -287,27 +251,19 @@ class I18nValidationTest < ActiveModel::TestCase end end - # validates_confirmation_of w/o mocha - set_expectations_for_validation "validates_confirmation_of", :confirmation do |person, options_to_merge| Person.validates_confirmation_of :title, options_to_merge person.title_confirmation = 'foo' end - # validates_acceptance_of w/o mocha - set_expectations_for_validation "validates_acceptance_of", :accepted do |person, options_to_merge| Person.validates_acceptance_of :title, options_to_merge.merge(allow_nil: false) end - # validates_presence_of w/o mocha - set_expectations_for_validation "validates_presence_of", :blank do |person, options_to_merge| Person.validates_presence_of :title, options_to_merge end - # validates_length_of :within w/o mocha - set_expectations_for_validation "validates_length_of", :too_short do |person, options_to_merge| Person.validates_length_of :title, options_to_merge.merge(within: 3..5) end @@ -317,61 +273,43 @@ class I18nValidationTest < ActiveModel::TestCase person.title = "too long" end - # validates_length_of :is w/o mocha - set_expectations_for_validation "validates_length_of", :wrong_length do |person, options_to_merge| Person.validates_length_of :title, options_to_merge.merge(is: 5) end - # validates_format_of w/o mocha - set_expectations_for_validation "validates_format_of", :invalid do |person, options_to_merge| Person.validates_format_of :title, options_to_merge.merge(with: /\A[1-9][0-9]*\z/) end - # validates_inclusion_of w/o mocha - set_expectations_for_validation "validates_inclusion_of", :inclusion do |person, options_to_merge| Person.validates_inclusion_of :title, options_to_merge.merge(in: %w(a b c)) end - # validates_exclusion_of w/o mocha - set_expectations_for_validation "validates_exclusion_of", :exclusion do |person, options_to_merge| Person.validates_exclusion_of :title, options_to_merge.merge(in: %w(a b c)) person.title = 'a' end - # validates_numericality_of without :only_integer w/o mocha - set_expectations_for_validation "validates_numericality_of", :not_a_number do |person, options_to_merge| Person.validates_numericality_of :title, options_to_merge person.title = 'a' end - # validates_numericality_of with :only_integer w/o mocha - set_expectations_for_validation "validates_numericality_of", :not_an_integer do |person, options_to_merge| Person.validates_numericality_of :title, options_to_merge.merge(only_integer: true) person.title = '1.0' end - # validates_numericality_of :odd w/o mocha - set_expectations_for_validation "validates_numericality_of", :odd do |person, options_to_merge| Person.validates_numericality_of :title, options_to_merge.merge(only_integer: true, odd: true) person.title = 0 end - # validates_numericality_of :less_than w/o mocha - set_expectations_for_validation "validates_numericality_of", :less_than do |person, options_to_merge| Person.validates_numericality_of :title, options_to_merge.merge(only_integer: true, less_than: 0) person.title = 1 end - # test with validates_with - def test_validations_with_message_symbol_must_translate I18n.backend.store_translations 'en', errors: { messages: { custom_error: "I am a custom error" } } Person.validates_presence_of :title, message: :custom_error diff --git a/activemodel/test/cases/validations/with_validation_test.rb b/activemodel/test/cases/validations/with_validation_test.rb index 9ee8b79da9..03c7943308 100644 --- a/activemodel/test/cases/validations/with_validation_test.rb +++ b/activemodel/test/cases/validations/with_validation_test.rb @@ -97,7 +97,7 @@ class ValidatesWithTest < ActiveModel::TestCase test "passes all configuration options to the validator class" do topic = Topic.new - validator = MiniTest::Mock.new + validator = Minitest::Mock.new validator.expect(:new, validator, [{foo: :bar, if: "1 == 1", class: Topic}]) validator.expect(:validate, nil, [topic]) validator.expect(:is_a?, false, [Symbol]) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6054b08f33..47364325ea 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,33 @@ +* Ensure that `ActionController::Parameters` can still be passed to nested + attributes. + + Fixes #20922. + + *Sean Griffin* + +* Deprecate force association reload by passing a truthy argument to + association method. + + For collection association, you can call `#reload` on association proxy to + force a reload: + + @user.posts.reload # Instead of @user.posts(true) + + For singular association, you can call `#reload` on the parent object to + clear its association cache then call the association method: + + @user.reload.profile # Instead of @user.profile(true) + + Passing a truthy argument to force association to reload will be removed in + Rails 5.1. + + *Prem Sichanugrist* + +* Replaced `ActiveSupport::Concurrency::Latch` with `Concurrent::CountDownLatch` + from the concurrent-ruby gem. + + *Jerry D'Antonio* + * Fix through associations using scopes having the scope merged multiple times. @@ -21,7 +51,7 @@ Example: change_column_default :posts, :status, from: nil, to: "draft" - change_column_default :users, authorized, from: true, to: false + change_column_default :users, :authorized, from: true, to: false *Prem Sichanugrist* diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 6caadb4ce8..87576abd92 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -1,3 +1,5 @@ +require "active_support/deprecation" + module ActiveRecord module Associations # = Active Record Association Collection @@ -28,6 +30,12 @@ module ActiveRecord # Implements the reader method, e.g. foo.items for Foo.has_many :items def reader(force_reload = false) if force_reload + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing an argument to force an association to reload is now + deprecated and will be removed in Rails 5.1. Please call `reload` + on the result collection proxy instead. + MSG + klass.uncached { reload } elsif stale_target? reload diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index cd79266952..1aa6a2ca74 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -133,7 +133,7 @@ module ActiveRecord if scope.klass.primary_key count = scope.destroy_all.length else - scope.each { |record| record.run_callbacks :destroy } + scope.each(&:_run_destroy_callbacks) arel = scope.arel diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index bec9505bd2..30c5d72482 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -1,9 +1,17 @@ +require "active_support/deprecation" + module ActiveRecord module Associations class SingularAssociation < Association #:nodoc: # Implements the reader method, e.g. foo.bar for Foo.has_one :bar def reader(force_reload = false) if force_reload && klass + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing an argument to force an association to reload is now + deprecated and will be removed in Rails 5.1. Please call `reload` + on the parent object instead. + MSG + klass.uncached { reload } elsif !loaded? || stale_target? reload diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 3027ce928e..19f0dca5a6 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -289,24 +289,25 @@ module ActiveRecord end def destroy #:nodoc: - run_callbacks(:destroy) { super } + _run_destroy_callbacks { super } end def touch(*) #:nodoc: - run_callbacks(:touch) { super } + _run_touch_callbacks { super } end private + def create_or_update(*) #:nodoc: - run_callbacks(:save) { super } + _run_save_callbacks { super } end def _create_record #:nodoc: - run_callbacks(:create) { super } + _run_create_callbacks { super } end def _update_record(*) #:nodoc: - run_callbacks(:update) { super } + _run_update_callbacks { super } end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 6535121075..282af220fb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -508,7 +508,7 @@ module ActiveRecord synchronize do remove_connection_from_thread_cache conn - conn.run_callbacks :checkin do + conn._run_checkin_callbacks do conn.expire end @@ -764,7 +764,7 @@ module ActiveRecord end def checkout_and_verify(c) - c.run_callbacks :checkout do + c._run_checkout_callbacks do c.verify! end c diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 8a014e682e..b82488a59c 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -303,7 +303,7 @@ module ActiveRecord assign_attributes(attributes) if attributes yield self if block_given? - run_callbacks :initialize + _run_initialize_callbacks end # Initialize an empty model object from +coder+. +coder+ should be @@ -330,8 +330,8 @@ module ActiveRecord self.class.define_attribute_methods - run_callbacks :find - run_callbacks :initialize + _run_find_callbacks + _run_initialize_callbacks self end @@ -367,7 +367,7 @@ module ActiveRecord @attributes = @attributes.dup @attributes.reset(self.class.primary_key) - run_callbacks(:initialize) + _run_initialize_callbacks @new_record = true @destroyed = false diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index c942d0e265..c337e1d18f 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -386,6 +386,9 @@ module ActiveRecord # then the existing record will be marked for destruction. def assign_nested_attributes_for_one_to_one_association(association_name, attributes) options = self.nested_attributes_options[association_name] + if attributes.respond_to?(:permitted?) + attributes = attributes.to_h + end attributes = attributes.with_indifferent_access existing_record = send(association_name) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index df72ba7e9c..0f6015fa93 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -139,7 +139,7 @@ module ActiveRecord # # SELECT people.id, people.name FROM people # # => [[1, 'David'], [2, 'Jeremy'], [3, 'Jose']] # - # Person.uniq.pluck(:role) + # Person.distinct.pluck(:role) # # SELECT DISTINCT role FROM people # # => ['admin', 'member', 'guest'] # @@ -195,7 +195,8 @@ module ActiveRecord def perform_calculation(operation, column_name) operation = operation.to_s.downcase - # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count) + # If #count is used with #distinct (i.e. `relation.distinct.count`) it is + # considered distinct. distinct = self.distinct_value if operation == "count" diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index dd8f0aa298..0b38666ce9 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -87,8 +87,8 @@ module ActiveRecord return if other.preload_values.empty? && other.includes_values.empty? if other.klass == relation.klass - relation.preload! other.preload_values unless other.preload_values.empty? - relation.includes! other.includes_values unless other.includes_values.empty? + relation.preload!(*other.preload_values) unless other.preload_values.empty? + relation.includes!(other.includes_values) unless other.includes_values.empty? else reflection = relation.klass.reflect_on_all_associations.find do |r| r.class_name == other.klass.name diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 6f2def0df1..3131723828 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -319,8 +319,8 @@ module ActiveRecord end def before_committed! # :nodoc: - run_callbacks :before_commit_without_transaction_enrollment - run_callbacks :before_commit + _run_before_commit_without_transaction_enrollment_callbacks + _run_before_commit_callbacks end # Call the +after_commit+ callbacks. @@ -329,8 +329,8 @@ module ActiveRecord # but call it after the commit of a destroyed object. def committed!(should_run_callbacks: true) #:nodoc: if should_run_callbacks && destroyed? || persisted? - run_callbacks :commit_without_transaction_enrollment - run_callbacks :commit + _run_commit_without_transaction_enrollment_callbacks + _run_commit_callbacks end ensure force_clear_transaction_record_state @@ -340,8 +340,8 @@ module ActiveRecord # state should be rolled back to the beginning or just to the last savepoint. def rolledback!(force_restore_state: false, should_run_callbacks: true) #:nodoc: if should_run_callbacks - run_callbacks :rollback - run_callbacks :rollback_without_transaction_enrollment + _run_rollback_callbacks + _run_rollback_without_transaction_enrollment_callbacks end ensure restore_transaction_record_state(force_restore_state) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index ebe08c618f..73cde05504 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -293,7 +293,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase client = Client.find(3) client.firm = nil client.save - assert_nil client.firm(true) + client.association(:firm).reload + assert_nil client.firm assert_nil client.client_of end @@ -301,7 +302,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase client = Client.create(:name => "Primary key client", :firm_name => companies(:first_firm).name) client.firm_with_primary_key = nil client.save - assert_nil client.firm_with_primary_key(true) + client.association(:firm_with_primary_key).reload + assert_nil client.firm_with_primary_key assert_nil client.client_of end @@ -318,11 +320,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase def test_polymorphic_association_class sponsor = Sponsor.new assert_nil sponsor.association(:sponsorable).send(:klass) - assert_nil sponsor.sponsorable(force_reload: true) + sponsor.association(:sponsorable).reload + assert_nil sponsor.sponsorable sponsor.sponsorable_type = '' # the column doesn't have to be declared NOT NULL assert_nil sponsor.association(:sponsorable).send(:klass) - assert_nil sponsor.sponsorable(force_reload: true) + sponsor.association(:sponsorable).reload + assert_nil sponsor.sponsorable sponsor.sponsorable = Member.new :name => "Bert" assert_equal Member, sponsor.association(:sponsorable).send(:klass) @@ -557,7 +561,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert final_cut.persisted? assert firm.persisted? assert_equal firm, final_cut.firm - assert_equal firm, final_cut.firm(true) + final_cut.association(:firm).reload + assert_equal firm, final_cut.firm end def test_assignment_before_child_saved_with_primary_key @@ -569,7 +574,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert final_cut.persisted? assert firm.persisted? assert_equal firm, final_cut.firm_with_primary_key - assert_equal firm, final_cut.firm_with_primary_key(true) + final_cut.association(:firm_with_primary_key).reload + assert_equal firm, final_cut.firm_with_primary_key end def test_new_record_with_foreign_key_but_no_object @@ -1064,6 +1070,12 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase Column.create! record: record assert_equal 1, Column.count end + + def test_association_force_reload_with_only_true_is_deprecated + client = Client.find(3) + + assert_deprecated { client.firm(true) } + end end class BelongsToWithForeignKeyTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index e584c94ad8..abbe37ab38 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -157,8 +157,8 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase jamis.projects << action_controller assert_equal 2, jamis.projects.size - assert_equal 2, jamis.projects(true).size - assert_equal 2, action_controller.developers(true).size + assert_equal 2, jamis.projects.reload.size + assert_equal 2, action_controller.developers.reload.size end def test_adding_type_mismatch @@ -176,9 +176,9 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase action_controller.developers << jamis - assert_equal 2, jamis.projects(true).size + assert_equal 2, jamis.projects.reload.size assert_equal 2, action_controller.developers.size - assert_equal 2, action_controller.developers(true).size + assert_equal 2, action_controller.developers.reload.size end def test_adding_from_the_project_fixed_timestamp @@ -192,9 +192,9 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase action_controller.developers << jamis assert_equal updated_at, jamis.updated_at - assert_equal 2, jamis.projects(true).size + assert_equal 2, jamis.projects.reload.size assert_equal 2, action_controller.developers.size - assert_equal 2, action_controller.developers(true).size + assert_equal 2, action_controller.developers.reload.size end def test_adding_multiple @@ -203,7 +203,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase aredridel.projects.reload aredridel.projects.push(Project.find(1), Project.find(2)) assert_equal 2, aredridel.projects.size - assert_equal 2, aredridel.projects(true).size + assert_equal 2, aredridel.projects.reload.size end def test_adding_a_collection @@ -212,7 +212,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase aredridel.projects.reload aredridel.projects.concat([Project.find(1), Project.find(2)]) assert_equal 2, aredridel.projects.size - assert_equal 2, aredridel.projects(true).size + assert_equal 2, aredridel.projects.reload.size end def test_habtm_adding_before_save @@ -227,7 +227,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal no_of_devels+1, Developer.count assert_equal no_of_projects+1, Project.count assert_equal 2, aredridel.projects.size - assert_equal 2, aredridel.projects(true).size + assert_equal 2, aredridel.projects.reload.size end def test_habtm_saving_multiple_relationships @@ -391,8 +391,8 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase david.projects.delete(active_record) assert_equal 1, david.projects.size - assert_equal 1, david.projects(true).size - assert_equal 2, active_record.developers(true).size + assert_equal 1, david.projects.reload.size + assert_equal 2, active_record.developers.reload.size end def test_deleting_array @@ -400,7 +400,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase david.projects.reload david.projects.delete(Project.all.to_a) assert_equal 0, david.projects.size - assert_equal 0, david.projects(true).size + assert_equal 0, david.projects.reload.size end def test_deleting_all @@ -408,7 +408,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase david.projects.reload david.projects.clear assert_equal 0, david.projects.size - assert_equal 0, david.projects(true).size + assert_equal 0, david.projects.reload.size end def test_removing_associations_on_destroy @@ -434,7 +434,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert join_records.empty? assert_equal 1, david.reload.projects.size - assert_equal 1, david.projects(true).size + assert_equal 1, david.projects.reload.size end def test_destroying_many @@ -450,7 +450,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert join_records.empty? assert_equal 0, david.reload.projects.size - assert_equal 0, david.projects(true).size + assert_equal 0, david.projects.reload.size end def test_destroy_all @@ -466,7 +466,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert join_records.empty? assert david.projects.empty? - assert david.projects(true).empty? + assert david.projects.reload.empty? end def test_destroy_associations_destroys_multiple_associations @@ -482,11 +482,11 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase join_records = Parrot.connection.select_all("SELECT * FROM parrots_pirates WHERE parrot_id = #{george.id}") assert join_records.empty? - assert george.pirates(true).empty? + assert george.pirates.reload.empty? join_records = Parrot.connection.select_all("SELECT * FROM parrots_treasures WHERE parrot_id = #{george.id}") assert join_records.empty? - assert george.treasures(true).empty? + assert george.treasures.reload.empty? end def test_associations_with_conditions @@ -654,7 +654,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_habtm_respects_select - categories(:technology).select_testing_posts(true).each do |o| + categories(:technology).select_testing_posts.reload.each do |o| assert_respond_to o, :correctness_marker end assert_respond_to categories(:technology).select_testing_posts.first, :correctness_marker @@ -726,7 +726,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_get_ids_for_loaded_associations developer = developers(:david) - developer.projects(true) + developer.projects.reload assert_queries(0) do developer.project_ids developer.project_ids @@ -917,4 +917,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase DeveloperWithSymbolClassName.new end end + + def test_association_force_reload_with_only_true_is_deprecated + developer = Developer.find(1) + + assert_deprecated { developer.projects(true) } + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 2c4e2a875c..f4bacbea2e 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -704,7 +704,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase natural = Client.new("name" => "Natural Company") companies(:first_firm).clients_of_firm << natural assert_equal 3, companies(:first_firm).clients_of_firm.size # checking via the collection - assert_equal 3, companies(:first_firm).clients_of_firm(true).size # checking using the db + assert_equal 3, companies(:first_firm).clients_of_firm.reload.size # checking using the db assert_equal natural, companies(:first_firm).clients_of_firm.last end @@ -759,7 +759,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase force_signal37_to_load_all_clients_of_firm companies(:first_firm).clients_of_firm.concat([Client.new("name" => "Natural Company"), Client.new("name" => "Apple")]) assert_equal 4, companies(:first_firm).clients_of_firm.size - assert_equal 4, companies(:first_firm).clients_of_firm(true).size + assert_equal 4, companies(:first_firm).clients_of_firm.reload.size end def test_transactions_when_adding_to_persisted @@ -771,7 +771,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase rescue Client::RaisedOnSave end - assert !companies(:first_firm).clients_of_firm(true).include?(good) + assert !companies(:first_firm).clients_of_firm.reload.include?(good) end def test_transactions_when_adding_to_new_record @@ -903,12 +903,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase new_client = companies(:first_firm).clients_of_firm.create("name" => "Another Client") assert new_client.persisted? assert_equal new_client, companies(:first_firm).clients_of_firm.last - assert_equal new_client, companies(:first_firm).clients_of_firm(true).last + assert_equal new_client, companies(:first_firm).clients_of_firm.reload.last end def test_create_many companies(:first_firm).clients_of_firm.create([{"name" => "Another Client"}, {"name" => "Another Client II"}]) - assert_equal 4, companies(:first_firm).clients_of_firm(true).size + assert_equal 4, companies(:first_firm).clients_of_firm.reload.size end def test_create_followed_by_save_does_not_load_target @@ -921,7 +921,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase force_signal37_to_load_all_clients_of_firm companies(:first_firm).clients_of_firm.delete(companies(:first_firm).clients_of_firm.first) assert_equal 1, companies(:first_firm).clients_of_firm.size - assert_equal 1, companies(:first_firm).clients_of_firm(true).size + assert_equal 1, companies(:first_firm).clients_of_firm.reload.size end def test_deleting_before_save @@ -1058,7 +1058,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, companies(:first_firm).clients_of_firm.size companies(:first_firm).clients_of_firm.delete([companies(:first_firm).clients_of_firm[0], companies(:first_firm).clients_of_firm[1], companies(:first_firm).clients_of_firm[2]]) assert_equal 0, companies(:first_firm).clients_of_firm.size - assert_equal 0, companies(:first_firm).clients_of_firm(true).size + assert_equal 0, companies(:first_firm).clients_of_firm.reload.size end def test_delete_all @@ -1079,7 +1079,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase companies(:first_firm).clients_of_firm.reset companies(:first_firm).clients_of_firm.delete_all assert_equal 0, companies(:first_firm).clients_of_firm.size - assert_equal 0, companies(:first_firm).clients_of_firm(true).size + assert_equal 0, companies(:first_firm).clients_of_firm.reload.size end def test_transaction_when_deleting_persisted @@ -1093,7 +1093,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase rescue Client::RaisedOnDestroy end - assert_equal [good, bad], companies(:first_firm).clients_of_firm(true) + assert_equal [good, bad], companies(:first_firm).clients_of_firm.reload end def test_transaction_when_deleting_new_record @@ -1113,7 +1113,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm.clients_of_firm.clear assert_equal 0, firm.clients_of_firm.size - assert_equal 0, firm.clients_of_firm(true).size + assert_equal 0, firm.clients_of_firm.reload.size assert_equal [], Client.destroyed_client_ids[firm.id] # Should not be destroyed since the association is not dependent. @@ -1149,7 +1149,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm.dependent_clients_of_firm.clear assert_equal 0, firm.dependent_clients_of_firm.size - assert_equal 0, firm.dependent_clients_of_firm(true).size + assert_equal 0, firm.dependent_clients_of_firm.reload.size assert_equal [], Client.destroyed_client_ids[firm.id] # Should be destroyed since the association is dependent. @@ -1182,7 +1182,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm.exclusively_dependent_clients_of_firm.clear assert_equal 0, firm.exclusively_dependent_clients_of_firm.size - assert_equal 0, firm.exclusively_dependent_clients_of_firm(true).size + assert_equal 0, firm.exclusively_dependent_clients_of_firm.reload.size # no destroy-filters should have been called assert_equal [], Client.destroyed_client_ids[firm.id] @@ -1231,7 +1231,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase # break the vanilla firm_id foreign key assert_equal 3, firm.clients.count firm.clients.first.update_columns(firm_id: nil) - assert_equal 2, firm.clients(true).count + assert_equal 2, firm.clients.reload.count assert_equal 2, firm.clients_using_primary_key_with_delete_all.count old_record = firm.clients_using_primary_key_with_delete_all.first firm = Firm.first @@ -1257,7 +1257,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm.clients_of_firm.clear assert_equal 0, firm.clients_of_firm.size - assert_equal 0, firm.clients_of_firm(true).size + assert_equal 0, firm.clients_of_firm.reload.size end def test_deleting_a_item_which_is_not_in_the_collection @@ -1265,7 +1265,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase summit = Client.find_by_name('Summit') companies(:first_firm).clients_of_firm.delete(summit) assert_equal 2, companies(:first_firm).clients_of_firm.size - assert_equal 2, companies(:first_firm).clients_of_firm(true).size + assert_equal 2, companies(:first_firm).clients_of_firm.reload.size assert_equal 2, summit.client_of end @@ -1303,7 +1303,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end assert_equal 1, companies(:first_firm).reload.clients_of_firm.size - assert_equal 1, companies(:first_firm).clients_of_firm(true).size + assert_equal 1, companies(:first_firm).clients_of_firm.reload.size end def test_destroying_by_fixnum_id @@ -1314,7 +1314,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end assert_equal 1, companies(:first_firm).reload.clients_of_firm.size - assert_equal 1, companies(:first_firm).clients_of_firm(true).size + assert_equal 1, companies(:first_firm).clients_of_firm.reload.size end def test_destroying_by_string_id @@ -1325,7 +1325,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end assert_equal 1, companies(:first_firm).reload.clients_of_firm.size - assert_equal 1, companies(:first_firm).clients_of_firm(true).size + assert_equal 1, companies(:first_firm).clients_of_firm.reload.size end def test_destroying_a_collection @@ -1338,7 +1338,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end assert_equal 1, companies(:first_firm).reload.clients_of_firm.size - assert_equal 1, companies(:first_firm).clients_of_firm(true).size + assert_equal 1, companies(:first_firm).clients_of_firm.reload.size end def test_destroy_all @@ -1349,7 +1349,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal clients.sort_by(&:id), destroyed.sort_by(&:id) assert destroyed.all?(&:frozen?), "destroyed clients should be frozen" assert companies(:first_firm).clients_of_firm.empty?, "37signals has no clients after destroy all" - assert companies(:first_firm).clients_of_firm(true).empty?, "37signals has no clients after destroy all and refresh" + assert companies(:first_firm).clients_of_firm.reload.empty?, "37signals has no clients after destroy all and refresh" end def test_dependence @@ -1518,7 +1518,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase rescue Client::RaisedOnSave end - assert_equal [good], companies(:first_firm).clients_of_firm(true) + assert_equal [good], companies(:first_firm).clients_of_firm.reload end def test_transactions_when_replacing_on_new_record @@ -1534,7 +1534,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_get_ids_for_loaded_associations company = companies(:first_firm) - company.clients(true) + company.clients.reload assert_queries(0) do company.client_ids company.client_ids @@ -1588,7 +1588,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm.client_ids = [companies(:first_client).id, nil, companies(:second_client).id, ''] firm.save! - assert_equal 2, firm.clients(true).size + assert_equal 2, firm.clients.reload.size assert_equal true, firm.clients.include?(companies(:second_client)) end @@ -2252,4 +2252,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [first_bulb, second_bulb], car.bulbs end + + def test_association_force_reload_with_only_true_is_deprecated + company = Company.find(1) + + assert_deprecated { company.clients_of_firm(true) } + end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index c34387f06d..ce2557339e 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -188,7 +188,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert post.people.include?(person) end - assert post.reload.people(true).include?(person) + assert post.reload.people.reload.include?(person) end def test_delete_all_for_with_dependent_option_destroy @@ -229,7 +229,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase post = posts(:thinking) post.people.concat [person] assert_equal 1, post.people.size - assert_equal 1, post.people(true).size + assert_equal 1, post.people.reload.size end def test_associate_existing_record_twice_should_add_to_target_twice @@ -285,7 +285,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:thinking).people.include?(new_person) end - assert posts(:thinking).reload.people(true).include?(new_person) + assert posts(:thinking).reload.people.reload.include?(new_person) end def test_associate_new_by_building @@ -310,8 +310,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase posts(:thinking).save end - assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Bob") - assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted") + assert posts(:thinking).reload.people.reload.collect(&:first_name).include?("Bob") + assert posts(:thinking).reload.people.reload.collect(&:first_name).include?("Ted") end def test_build_then_save_with_has_many_inverse @@ -356,7 +356,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).people.empty? end - assert posts(:welcome).reload.people(true).empty? + assert posts(:welcome).reload.people.reload.empty? end def test_destroy_association @@ -367,7 +367,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end assert posts(:welcome).reload.people.empty? - assert posts(:welcome).people(true).empty? + assert posts(:welcome).people.reload.empty? end def test_destroy_all @@ -378,7 +378,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end assert posts(:welcome).reload.people.empty? - assert posts(:welcome).people(true).empty? + assert posts(:welcome).people.reload.empty? end def test_should_raise_exception_for_destroying_mismatching_records @@ -539,7 +539,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_replace_association - assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} + assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people.reload} # 1 query to delete the existing reader (michael) # 1 query to associate the new reader (david) @@ -552,8 +552,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert !posts(:welcome).people.include?(people(:michael)) } - assert posts(:welcome).reload.people(true).include?(people(:david)) - assert !posts(:welcome).reload.people(true).include?(people(:michael)) + assert posts(:welcome).reload.people.reload.include?(people(:david)) + assert !posts(:welcome).reload.people.reload.include?(people(:michael)) end def test_replace_order_is_preserved @@ -592,7 +592,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:thinking).people.collect(&:first_name).include?("Jeb") end - assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Jeb") + assert posts(:thinking).reload.people.reload.collect(&:first_name).include?("Jeb") end def test_through_record_is_built_when_created_with_where @@ -668,7 +668,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_clear_associations - assert_queries(2) { posts(:welcome);posts(:welcome).people(true) } + assert_queries(2) { posts(:welcome);posts(:welcome).people.reload } assert_queries(1) do posts(:welcome).people.clear @@ -678,7 +678,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).people.empty? end - assert posts(:welcome).reload.people(true).empty? + assert posts(:welcome).reload.people.reload.empty? end def test_association_callback_ordering @@ -750,7 +750,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_get_ids_for_loaded_associations person = people(:michael) - person.posts(true) + person.posts.reload assert_queries(0) do person.post_ids person.post_ids @@ -828,14 +828,14 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase category = author.named_categories.build(:name => "Primary") author.save assert Categorization.exists?(:author_id => author.id, :named_category_name => category.name) - assert author.named_categories(true).include?(category) + assert author.named_categories.reload.include?(category) end def test_collection_create_with_nonstandard_primary_key_on_belongs_to author = authors(:mary) category = author.named_categories.create(:name => "Primary") assert Categorization.exists?(:author_id => author.id, :named_category_name => category.name) - assert author.named_categories(true).include?(category) + assert author.named_categories.reload.include?(category) end def test_collection_exists @@ -850,7 +850,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase category = author.named_categories.create(:name => "Primary") author.named_categories.delete(category) assert !Categorization.exists?(:author_id => author.id, :named_category_name => category.name) - assert author.named_categories(true).empty? + assert author.named_categories.reload.empty? end def test_collection_singular_ids_getter_with_string_primary_keys @@ -871,10 +871,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_nothing_raised do book = books(:awdr) book.subscriber_ids = [subscribers(:second).nick] - assert_equal [subscribers(:second)], book.subscribers(true) + assert_equal [subscribers(:second)], book.subscribers.reload book.subscriber_ids = [] - assert_equal [], book.subscribers(true) + assert_equal [], book.subscribers.reload end end @@ -1165,4 +1165,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_nil Club.new.special_favourites.distinct_value end + + def test_association_force_reload_with_only_true_is_deprecated + post = Post.find(1) + + assert_deprecated { post.people(true) } + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 5c2e5e7b43..b7cb5a064f 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -332,7 +332,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert a.persisted? assert_equal a, firm.account assert_equal a, firm.account - assert_equal a, firm.account(true) + firm.association(:account).reload + assert_equal a, firm.account end def test_save_still_works_after_accessing_nil_has_one @@ -607,4 +608,10 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end end end + + def test_association_force_reload_with_only_true_is_deprecated + firm = Firm.find(1) + + assert_deprecated { firm.account(true) } + end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index f8772547a2..97fd458994 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -245,12 +245,14 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_not_nil @member_detail.member_type @member_detail.destroy assert_queries(1) do - assert_not_nil @member_detail.member_type(true) + @member_detail.association(:member_type).reload + assert_not_nil @member_detail.member_type end @member_detail.member.destroy assert_queries(1) do - assert_nil @member_detail.member_type(true) + @member_detail.association(:member_type).reload + assert_nil @member_detail.member_type end end @@ -344,4 +346,10 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end end end + + def test_association_force_reload_with_only_true_is_deprecated + member = Member.find(1) + + assert_deprecated { member.club(true) } + end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 5575419c35..f6dddaf5b4 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -213,7 +213,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase old_count = Tagging.count post.destroy assert_equal old_count-1, Tagging.count - assert_nil posts(:welcome).tagging(true) + posts(:welcome).association(:tagging).reload + assert_nil posts(:welcome).tagging end def test_delete_polymorphic_has_one_with_nullify @@ -224,7 +225,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase old_count = Tagging.count post.destroy assert_equal old_count, Tagging.count - assert_nil posts(:welcome).tagging(true) + posts(:welcome).association(:tagging).reload + assert_nil posts(:welcome).tagging end def test_has_many_with_piggyback @@ -461,7 +463,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert saved_post.tags.include?(new_tag) assert new_tag.persisted? - assert saved_post.reload.tags(true).include?(new_tag) + assert saved_post.reload.tags.reload.include?(new_tag) new_post = Post.new(:title => "Association replacement works!", :body => "You best believe it.") @@ -474,7 +476,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase new_post.save! assert new_post.persisted? - assert new_post.reload.tags(true).include?(saved_tag) + assert new_post.reload.tags.reload.include?(saved_tag) assert !posts(:thinking).tags.build.persisted? assert !posts(:thinking).tags.new.persisted? @@ -490,7 +492,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_nil( wrong = post_thinking.taggings.detect { |t| t.class != Tagging }, message = "Expected a Tagging in taggings collection, got #{wrong.class}.") assert_equal(count + 1, post_thinking.reload.tags.size) - assert_equal(count + 1, post_thinking.tags(true).size) + assert_equal(count + 1, post_thinking.tags.reload.size) assert_kind_of Tag, post_thinking.tags.create!(:name => 'foo') assert_nil( wrong = post_thinking.tags.detect { |t| t.class != Tag }, @@ -498,7 +500,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_nil( wrong = post_thinking.taggings.detect { |t| t.class != Tagging }, message = "Expected a Tagging in taggings collection, got #{wrong.class}.") assert_equal(count + 2, post_thinking.reload.tags.size) - assert_equal(count + 2, post_thinking.tags(true).size) + assert_equal(count + 2, post_thinking.tags.reload.size) assert_nothing_raised { post_thinking.tags.concat(Tag.create!(:name => 'abc'), Tag.create!(:name => 'def')) } assert_nil( wrong = post_thinking.tags.detect { |t| t.class != Tag }, @@ -506,7 +508,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_nil( wrong = post_thinking.taggings.detect { |t| t.class != Tagging }, message = "Expected a Tagging in taggings collection, got #{wrong.class}.") assert_equal(count + 4, post_thinking.reload.tags.size) - assert_equal(count + 4, post_thinking.tags(true).size) + assert_equal(count + 4, post_thinking.tags.reload.size) # Raises if the wrong reflection name is used to set the Edge belongs_to assert_nothing_raised { vertices(:vertex_1).sinks << vertices(:vertex_5) } @@ -544,11 +546,11 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase book = Book.create!(:name => 'Getting Real') book_awdr = books(:awdr) book_awdr.references << book - assert_equal(count + 1, book_awdr.references(true).size) + assert_equal(count + 1, book_awdr.references.reload.size) assert_nothing_raised { book_awdr.references.delete(book) } assert_equal(count, book_awdr.references.size) - assert_equal(count, book_awdr.references(true).size) + assert_equal(count, book_awdr.references.reload.size) assert_equal(references_before.sort, book_awdr.references.sort) end @@ -558,14 +560,14 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase tag = Tag.create!(:name => 'doomed') post_thinking = posts(:thinking) post_thinking.tags << tag - assert_equal(count + 1, post_thinking.taggings(true).size) - assert_equal(count + 1, post_thinking.reload.tags(true).size) + assert_equal(count + 1, post_thinking.taggings.reload.size) + assert_equal(count + 1, post_thinking.reload.tags.reload.size) assert_not_equal(tags_before, post_thinking.tags.sort) assert_nothing_raised { post_thinking.tags.delete(tag) } assert_equal(count, post_thinking.tags.size) - assert_equal(count, post_thinking.tags(true).size) - assert_equal(count, post_thinking.taggings(true).size) + assert_equal(count, post_thinking.tags.reload.size) + assert_equal(count, post_thinking.taggings.reload.size) assert_equal(tags_before, post_thinking.tags.sort) end @@ -577,11 +579,11 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase quaked = Tag.create!(:name => 'quaked') post_thinking = posts(:thinking) post_thinking.tags << doomed << doomed2 - assert_equal(count + 2, post_thinking.reload.tags(true).size) + assert_equal(count + 2, post_thinking.reload.tags.reload.size) assert_nothing_raised { post_thinking.tags.delete(doomed, doomed2, quaked) } assert_equal(count, post_thinking.tags.size) - assert_equal(count, post_thinking.tags(true).size) + assert_equal(count, post_thinking.tags.reload.size) assert_equal(tags_before, post_thinking.tags.sort) end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 3d202a5527..08f790e21b 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -93,8 +93,10 @@ class AssociationsTest < ActiveRecord::TestCase assert firm.clients.empty?, "New firm should have cached no client objects" assert_equal 0, firm.clients.size, "New firm should have cached 0 clients count" - assert !firm.clients(true).empty?, "New firm should have reloaded client objects" - assert_equal 1, firm.clients(true).size, "New firm should have reloaded clients count" + ActiveSupport::Deprecation.silence do + assert !firm.clients(true).empty?, "New firm should have reloaded client objects" + assert_equal 1, firm.clients(true).size, "New firm should have reloaded clients count" + end end def test_using_limitable_reflections_helper @@ -110,10 +112,13 @@ class AssociationsTest < ActiveRecord::TestCase def test_force_reload_is_uncached firm = Firm.create!("name" => "A New Firm, Inc") Client.create!("name" => "TheClient.com", :firm => firm) - ActiveRecord::Base.cache do - firm.clients.each {} - assert_queries(0) { assert_not_nil firm.clients.each {} } - assert_queries(1) { assert_not_nil firm.clients(true).each {} } + + ActiveSupport::Deprecation.silence do + ActiveRecord::Base.cache do + firm.clients.each {} + assert_queries(0) { assert_not_nil firm.clients.each {} } + assert_queries(1) { assert_not_nil firm.clients(true).each {} } + end end end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 80b5a0004d..676b29ed7c 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -149,7 +149,8 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas assert_equal a, firm.account assert firm.save assert_equal a, firm.account - assert_equal a, firm.account(true) + firm.association(:account).reload + assert_equal a, firm.account end def test_assignment_before_either_saved @@ -162,7 +163,8 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas assert firm.persisted? assert a.persisted? assert_equal a, firm.account - assert_equal a, firm.account(true) + firm.association(:account).reload + assert_equal a, firm.account end def test_not_resaved_when_unchanged @@ -248,7 +250,8 @@ class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::Test assert apple.save assert apple.persisted? assert_equal apple, client.firm - assert_equal apple, client.firm(true) + client.association(:firm).reload + assert_equal apple, client.firm end def test_assignment_before_either_saved @@ -261,7 +264,8 @@ class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::Test assert final_cut.persisted? assert apple.persisted? assert_equal apple, final_cut.firm - assert_equal apple, final_cut.firm(true) + final_cut.association(:firm).reload + assert_equal apple, final_cut.firm end def test_store_two_association_with_one_save @@ -456,7 +460,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa assert_equal new_client, companies(:first_firm).clients_of_firm.last assert !companies(:first_firm).save assert !new_client.persisted? - assert_equal 2, companies(:first_firm).clients_of_firm(true).size + assert_equal 2, companies(:first_firm).clients_of_firm.reload.size end def test_adding_before_save @@ -481,7 +485,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa assert_equal no_of_clients + 2, Client.count # Clients were saved to database. assert_equal 2, new_firm.clients_of_firm.size - assert_equal 2, new_firm.clients_of_firm(true).size + assert_equal 2, new_firm.clients_of_firm.reload.size end def test_assign_ids @@ -510,7 +514,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa company.name += '-changed' assert_queries(2) { assert company.save } assert new_client.persisted? - assert_equal 3, company.clients_of_firm(true).size + assert_equal 3, company.clients_of_firm.reload.size end def test_build_many_before_save @@ -519,7 +523,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa company.name += '-changed' assert_queries(3) { assert company.save } - assert_equal 4, company.clients_of_firm(true).size + assert_equal 4, company.clients_of_firm.reload.size end def test_build_via_block_before_save @@ -530,7 +534,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa company.name += '-changed' assert_queries(2) { assert company.save } assert new_client.persisted? - assert_equal 3, company.clients_of_firm(true).size + assert_equal 3, company.clients_of_firm.reload.size end def test_build_many_via_block_before_save @@ -543,7 +547,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa company.name += '-changed' assert_queries(3) { assert company.save } - assert_equal 4, company.clients_of_firm(true).size + assert_equal 4, company.clients_of_firm.reload.size end def test_replace_on_new_object diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 31c31e4329..382adbbdc7 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- require "cases/helper" -require 'active_support/concurrency/latch' require 'models/post' require 'models/author' require 'models/topic' @@ -29,6 +28,7 @@ require 'models/bird' require 'models/car' require 'models/bulb' require 'rexml/document' +require 'concurrent/atomics' class FirstAbstractClass < ActiveRecord::Base self.abstract_class = true @@ -1506,20 +1506,20 @@ class BasicsTest < ActiveRecord::TestCase orig_handler = klass.connection_handler new_handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new after_handler = nil - latch1 = ActiveSupport::Concurrency::Latch.new - latch2 = ActiveSupport::Concurrency::Latch.new + latch1 = Concurrent::CountDownLatch.new + latch2 = Concurrent::CountDownLatch.new t = Thread.new do klass.connection_handler = new_handler - latch1.release - latch2.await + latch1.count_down + latch2.wait after_handler = klass.connection_handler end - latch1.await + latch1.wait klass.connection_handler = orig_handler - latch2.release + latch2.count_down t.join assert_equal after_handler, new_handler diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index c905772193..7ef5c93a48 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -1,5 +1,5 @@ require "cases/helper" -require 'active_support/concurrency/latch' +require 'concurrent/atomics' module ActiveRecord module ConnectionAdapters @@ -133,15 +133,15 @@ module ActiveRecord end def test_reap_inactive - ready = ActiveSupport::Concurrency::Latch.new + ready = Concurrent::CountDownLatch.new @pool.checkout child = Thread.new do @pool.checkout @pool.checkout - ready.release + ready.count_down Thread.stop end - ready.await + ready.wait assert_equal 3, active_connections(@pool).size @@ -360,13 +360,13 @@ module ActiveRecord def test_concurrent_connection_establishment assert_operator @pool.connections.size, :<=, 1 - all_threads_in_new_connection = ActiveSupport::Concurrency::Latch.new(@pool.size - @pool.connections.size) - all_go = ActiveSupport::Concurrency::Latch.new + all_threads_in_new_connection = Concurrent::CountDownLatch.new(@pool.size - @pool.connections.size) + all_go = Concurrent::CountDownLatch.new @pool.singleton_class.class_eval do define_method(:new_connection) do - all_threads_in_new_connection.release - all_go.await + all_threads_in_new_connection.count_down + all_go.wait super() end end @@ -381,14 +381,14 @@ module ActiveRecord # the kernel of the whole test is here, everything else is just scaffolding, # this latch will not be released unless conn. pool allows for concurrent # connection creation - all_threads_in_new_connection.await + all_threads_in_new_connection.wait end rescue Timeout::Error flunk 'pool unable to establish connections concurrently or implementation has ' << 'changed, this test then needs to patch a different :new_connection method' ensure # clean up the threads - all_go.release + all_go.count_down connecting_threads.map(&:join) end end @@ -441,11 +441,11 @@ module ActiveRecord with_single_connection_pool do |pool| [:disconnect, :disconnect!, :clear_reloadable_connections, :clear_reloadable_connections!].each do |group_action_method| conn = pool.connection # drain the only available connection - second_thread_done = ActiveSupport::Concurrency::Latch.new + second_thread_done = Concurrent::CountDownLatch.new # create a first_thread and let it get into the FIFO queue first first_thread = Thread.new do - pool.with_connection { second_thread_done.await } + pool.with_connection { second_thread_done.wait } end # wait for first_thread to get in queue @@ -456,7 +456,7 @@ module ActiveRecord # first_thread when a connection is made available second_thread = Thread.new do pool.send(group_action_method) - second_thread_done.release + second_thread_done.count_down end # wait for second_thread to get in queue @@ -471,7 +471,7 @@ module ActiveRecord failed = true unless second_thread.join(2) #--- post test clean up start - second_thread_done.release if failed + second_thread_done.count_down if failed # after `pool.disconnect()` the first thread will be left stuck in queue, no need to wait for # it to timeout with ConnectionTimeoutError diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 6b4addd52f..933dfac806 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -658,6 +658,16 @@ module NestedAttributesOnACollectionAssociationTests assert_equal "Couldn't find #{@child_1.class.name} with ID=1234567890 for Pirate with ID=#{@pirate.id}", exception.message end + def test_should_raise_RecordNotFound_if_an_id_belonging_to_a_different_record_is_given + other_pirate = Pirate.create! catchphrase: 'Ahoy!' + other_child = other_pirate.send(@association_name).create! name: 'Buccaneers Servant' + + exception = assert_raise ActiveRecord::RecordNotFound do + @pirate.attributes = { association_getter => [{ id: other_child.id }] } + end + assert_equal "Couldn't find #{@child_1.class.name} with ID=#{other_child.id} for Pirate with ID=#{@pirate.id}", exception.message + end + def test_should_automatically_build_new_associated_models_for_each_entry_in_a_hash_where_the_id_is_missing @pirate.send(@association_name).destroy_all @pirate.reload.attributes = { @@ -1054,4 +1064,27 @@ class TestHasManyAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveR assert_not part.valid? assert_equal ["Ship name can't be blank"], part.errors.full_messages end + + class ProtectedParameters + def initialize(hash) + @hash = hash + end + + def permitted? + true + end + + def to_h + @hash + end + end + + test "strong params style objects can be assigned" do + params = { name: "Stern", ship_attributes: + ProtectedParameters.new(name: "The Black Rock") } + part = ShipPart.new(params) + + assert_equal "Stern", part.name + assert_equal "The Black Rock", part.ship.name + end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index acbf85d398..5f48c2b40f 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -647,6 +647,25 @@ class RelationTest < ActiveRecord::TestCase end end + def test_preloading_with_associations_default_scopes_and_merges + post = Post.create! title: 'Uhuu', body: 'body' + reader = Reader.create! post_id: post.id, person_id: 1 + + post_rel = PostWithPreloadDefaultScope.preload(:readers).joins(:readers).where(title: 'Uhuu') + result_post = PostWithPreloadDefaultScope.all.merge(post_rel).to_a.first + + assert_no_queries do + assert_equal [reader], result_post.readers.to_a + end + + post_rel = PostWithIncludesDefaultScope.includes(:readers).where(title: 'Uhuu') + result_post = PostWithIncludesDefaultScope.all.merge(post_rel).to_a.first + + assert_no_queries do + assert_equal [reader], result_post.readers.to_a + end + end + def test_loading_with_one_association posts = Post.preload(:comments) post = posts.find { |p| p.id == 1 } diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 2468a91969..4e163ca4a6 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -181,7 +181,7 @@ class TransactionTest < ActiveRecord::TestCase assert posts_count > 0 status = author.update(name: nil, post_ids: []) assert !status - assert_equal posts_count, author.posts(true).size + assert_equal posts_count, author.posts.reload.size end def test_update_should_rollback_on_failure! @@ -191,7 +191,7 @@ class TransactionTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordInvalid) do author.update!(name: nil, post_ids: []) end - assert_equal posts_count, author.posts(true).size + assert_equal posts_count, author.posts.reload.size end def test_cancellation_from_returning_false_in_before_filter diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 052b1c9690..10f13b67da 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -208,6 +208,22 @@ class PostWithDefaultScope < ActiveRecord::Base default_scope { order(:title) } end +class PostWithPreloadDefaultScope < ActiveRecord::Base + self.table_name = 'posts' + + has_many :readers, foreign_key: 'post_id' + + default_scope { preload(:readers) } +end + +class PostWithIncludesDefaultScope < ActiveRecord::Base + self.table_name = 'posts' + + has_many :readers, foreign_key: 'post_id' + + default_scope { includes(:readers) } +end + class SpecialPostWithDefaultScope < ActiveRecord::Base self.table_name = 'posts' default_scope { where(:id => [1, 5,6]) } diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 6ebbdbc3db..1c69e0eb12 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,24 @@ +* ActiveSupport::HashWithIndifferentAccess `select` and `reject` will now return + enumerator if called without block. + + Fixes #20095 + + *Bernard Potocki* + +* Removed `ActiveSupport::Concurrency::Latch`, superseded by `Concurrent::CountDownLatch` + from the concurrent-ruby gem. + + *Jerry D'Antonio* + +* Fix not calling `#default` on `HashWithIndifferentAccess#to_hash` when only + `default_proc` is set, which could raise. + + *Simon Eskildsen* + +* Fix setting `default_proc` on `HashWithIndifferentAccess#dup` + + *Simon Eskildsen* + * Fix a range of values for parameters of the Time#change *Nikolay Kondratyev* diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index 12c6a4d449..c18c1c87c9 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -25,4 +25,5 @@ Gem::Specification.new do |s| s.add_dependency 'tzinfo', '~> 1.1' s.add_dependency 'minitest', '~> 5.1' s.add_dependency 'thread_safe','~> 0.3', '>= 0.3.4' + s.add_dependency 'concurrent-ruby', '~> 0.9.0' end diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index e8ab3a7db5..f35e1f5098 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -80,8 +80,12 @@ module ActiveSupport # save # end def run_callbacks(kind, &block) - callbacks = send("_#{kind}_callbacks") + send "_run_#{kind}_callbacks", &block + end + + private + def __run_callbacks__(callbacks, &block) if callbacks.empty? yield if block_given? else @@ -91,8 +95,6 @@ module ActiveSupport end end - private - # A hook invoked every time a before callback is halted. # This can be overridden in AS::Callback implementors in order # to provide better debugging/logging. @@ -806,6 +808,12 @@ module ActiveSupport names.each do |name| class_attribute "_#{name}_callbacks" set_callbacks name, CallbackChain.new(name, options) + + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def _run_#{name}_callbacks(&block) + __run_callbacks__(_#{name}_callbacks, &block) + end + RUBY end end diff --git a/activesupport/lib/active_support/concurrency/latch.rb b/activesupport/lib/active_support/concurrency/latch.rb index 1507de433e..7b8df0df04 100644 --- a/activesupport/lib/active_support/concurrency/latch.rb +++ b/activesupport/lib/active_support/concurrency/latch.rb @@ -1,26 +1,18 @@ -require 'thread' -require 'monitor' +require 'concurrent/atomics' module ActiveSupport module Concurrency - class Latch - def initialize(count = 1) - @count = count - @lock = Monitor.new - @cv = @lock.new_cond - end + class Latch < Concurrent::CountDownLatch - def release - @lock.synchronize do - @count -= 1 if @count > 0 - @cv.broadcast if @count.zero? - end + def initialize(count = 1) + ActiveSupport::Deprecation.warn("ActiveSupport::Concurrency::Latch is deprecated. Please use Concurrent::CountDownLatch instead.") + super(count) end + + alias_method :release, :count_down def await - @lock.synchronize do - @cv.wait_while { @count > 0 } - end + wait(nil) end end end diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb new file mode 100644 index 0000000000..f1c6230084 --- /dev/null +++ b/activesupport/lib/active_support/concurrency/share_lock.rb @@ -0,0 +1,138 @@ +require 'thread' +require 'monitor' + +module ActiveSupport + module Concurrency + # A share/exclusive lock, otherwise known as a read/write lock. + # + # https://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock + #-- + # Note that a pending Exclusive lock attempt does not block incoming + # Share requests (i.e., we are "read-preferring"). That seems + # consistent with the behavior of +loose_upgrades+, but may be the + # wrong choice otherwise: it nominally reduces the possibility of + # deadlock by risking starvation instead. + class ShareLock + include MonitorMixin + + # We track Thread objects, instead of just using counters, because + # we need exclusive locks to be reentrant, and we need to be able + # to upgrade share locks to exclusive. + + + # If +loose_upgrades+ is false (the default), then a thread that + # is waiting on an Exclusive lock will continue to hold any Share + # lock that it has already established. This is safer, but can + # lead to deadlock. + # + # If +loose_upgrades+ is true, a thread waiting on an Exclusive + # lock will temporarily relinquish its Share lock. Being less + # strict, this behavior prevents some classes of deadlocks. For + # many resources, loose upgrades are sufficient: if a thread is + # awaiting a lock, it is not running any other code. + attr_reader :loose_upgrades + + def initialize(loose_upgrades = false) + @loose_upgrades = loose_upgrades + + super() + + @cv = new_cond + + @sharing = Hash.new(0) + @exclusive_thread = nil + @exclusive_depth = 0 + end + + # Returns false if +no_wait+ is specified and the lock is not + # immediately available. Otherwise, returns true after the lock + # has been acquired. + def start_exclusive(no_wait=false) + synchronize do + unless @exclusive_thread == Thread.current + return false if no_wait && busy? + + loose_shares = nil + if @loose_upgrades + loose_shares = @sharing.delete(Thread.current) + end + + @cv.wait_while { busy? } if busy? + + @exclusive_thread = Thread.current + @sharing[Thread.current] = loose_shares if loose_shares + end + @exclusive_depth += 1 + + true + end + end + + # Relinquish the exclusive lock. Must only be called by the thread + # that called start_exclusive (and currently holds the lock). + def stop_exclusive + synchronize do + raise "invalid unlock" if @exclusive_thread != Thread.current + + @exclusive_depth -= 1 + if @exclusive_depth == 0 + @exclusive_thread = nil + @cv.broadcast + end + end + end + + def start_sharing + synchronize do + if @exclusive_thread && @exclusive_thread != Thread.current + @cv.wait_while { @exclusive_thread } + end + @sharing[Thread.current] += 1 + end + end + + def stop_sharing + synchronize do + if @sharing[Thread.current] > 1 + @sharing[Thread.current] -= 1 + else + @sharing.delete Thread.current + @cv.broadcast + end + end + end + + # Execute the supplied block while holding the Exclusive lock. If + # +no_wait+ is set and the lock is not immediately available, + # returns +nil+ without yielding. Otherwise, returns the result of + # the block. + def exclusive(no_wait=false) + if start_exclusive(no_wait) + begin + yield + ensure + stop_exclusive + end + end + end + + # Execute the supplied block while holding the Share lock. + def sharing + start_sharing + begin + yield + ensure + stop_sharing + end + end + + private + + # Must be called within synchronize + def busy? + (@exclusive_thread && @exclusive_thread != Thread.current) || + @sharing.size > (@sharing[Thread.current] > 0 ? 1 : 0) + end + end + end +end diff --git a/activesupport/lib/active_support/core_ext/module/concerning.rb b/activesupport/lib/active_support/core_ext/module/concerning.rb index e26b594fc4..65b88b9bbd 100644 --- a/activesupport/lib/active_support/core_ext/module/concerning.rb +++ b/activesupport/lib/active_support/core_ext/module/concerning.rb @@ -99,7 +99,7 @@ class Module # end # # Todo.ancestors - # # => Todo, Todo::EventTracking, Object + # # => [Todo, Todo::EventTracking, Object] # # This small step has some wonderful ripple effects. We can # * grok the behavior of our class in one glance, diff --git a/activesupport/lib/active_support/core_ext/object/duplicable.rb b/activesupport/lib/active_support/core_ext/object/duplicable.rb index 620f7b6561..6fc0cb53f0 100644 --- a/activesupport/lib/active_support/core_ext/object/duplicable.rb +++ b/activesupport/lib/active_support/core_ext/object/duplicable.rb @@ -19,7 +19,7 @@ class Object # Can you safely dup this object? # - # False for +nil+, +false+, +true+, symbol, number objects; + # False for +nil+, +false+, +true+, symbol, number, method objects; # true otherwise. def duplicable? true @@ -78,6 +78,10 @@ end require 'bigdecimal' class BigDecimal + # BigDecimals are duplicable: + # + # BigDecimal.new("1.2").duplicable? # => true + # BigDecimal.new("1.2").dup # => #<BigDecimal:7f9d698eee10,'0.12E1',18(18)> def duplicable? true end diff --git a/activesupport/lib/active_support/core_ext/time/zones.rb b/activesupport/lib/active_support/core_ext/time/zones.rb index d683e7c777..133d3938eb 100644 --- a/activesupport/lib/active_support/core_ext/time/zones.rb +++ b/activesupport/lib/active_support/core_ext/time/zones.rb @@ -65,7 +65,8 @@ class Time if !time_zone || time_zone.is_a?(ActiveSupport::TimeZone) time_zone else - # lookup timezone based on identifier (unless we've been passed a TZInfo::Timezone) + # Look up the timezone based on the identifier (unless we've been + # passed a TZInfo::Timezone) unless time_zone.respond_to?(:period_for_local) time_zone = ActiveSupport::TimeZone[time_zone] || TZInfo::Timezone.get(time_zone) end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 664cc15a29..770c845435 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -12,12 +12,33 @@ require 'active_support/core_ext/kernel/reporting' require 'active_support/core_ext/load_error' require 'active_support/core_ext/name_error' require 'active_support/core_ext/string/starts_ends_with' +require "active_support/dependencies/interlock" require 'active_support/inflector' module ActiveSupport #:nodoc: module Dependencies #:nodoc: extend self + mattr_accessor :interlock + self.interlock = Interlock.new + + # :doc: + + # Execute the supplied block without interference from any + # concurrent loads + def self.run_interlock + Dependencies.interlock.running { yield } + end + + # Execute the supplied block while holding an exclusive lock, + # preventing any other thread from being inside a #run_interlock + # block at the same time + def self.load_interlock + Dependencies.interlock.loading { yield } + end + + # :nodoc: + # Should we turn on Ruby warnings on the first load of dependent files? mattr_accessor :warnings_on_first_load self.warnings_on_first_load = false @@ -234,10 +255,12 @@ module ActiveSupport #:nodoc: end def load_dependency(file) - if Dependencies.load? && ActiveSupport::Dependencies.constant_watch_stack.watching? - Dependencies.new_constants_in(Object) { yield } - else - yield + Dependencies.load_interlock do + if Dependencies.load? && ActiveSupport::Dependencies.constant_watch_stack.watching? + Dependencies.new_constants_in(Object) { yield } + else + yield + end end rescue Exception => exception # errors from loading file exception.blame_file! file if exception.respond_to? :blame_file! @@ -325,9 +348,11 @@ module ActiveSupport #:nodoc: def clear log_call - loaded.clear - loading.clear - remove_unloadable_constants! + Dependencies.load_interlock do + loaded.clear + loading.clear + remove_unloadable_constants! + end end def require_or_load(file_name, const_path = nil) @@ -336,39 +361,44 @@ module ActiveSupport #:nodoc: expanded = File.expand_path(file_name) return if loaded.include?(expanded) - # Record that we've seen this file *before* loading it to avoid an - # infinite loop with mutual dependencies. - loaded << expanded - loading << expanded + Dependencies.load_interlock do + # Maybe it got loaded while we were waiting for our lock: + return if loaded.include?(expanded) - begin - if load? - log "loading #{file_name}" - - # Enable warnings if this file has not been loaded before and - # warnings_on_first_load is set. - load_args = ["#{file_name}.rb"] - load_args << const_path unless const_path.nil? + # Record that we've seen this file *before* loading it to avoid an + # infinite loop with mutual dependencies. + loaded << expanded + loading << expanded - if !warnings_on_first_load or history.include?(expanded) - result = load_file(*load_args) + begin + if load? + log "loading #{file_name}" + + # Enable warnings if this file has not been loaded before and + # warnings_on_first_load is set. + load_args = ["#{file_name}.rb"] + load_args << const_path unless const_path.nil? + + if !warnings_on_first_load or history.include?(expanded) + result = load_file(*load_args) + else + enable_warnings { result = load_file(*load_args) } + end else - enable_warnings { result = load_file(*load_args) } + log "requiring #{file_name}" + result = require file_name end - else - log "requiring #{file_name}" - result = require file_name + rescue Exception + loaded.delete expanded + raise + ensure + loading.pop end - rescue Exception - loaded.delete expanded - raise - ensure - loading.pop - end - # Record history *after* loading so first load gets warnings. - history << expanded - result + # Record history *after* loading so first load gets warnings. + history << expanded + result + end end # Is the provided constant path defined? diff --git a/activesupport/lib/active_support/dependencies/interlock.rb b/activesupport/lib/active_support/dependencies/interlock.rb new file mode 100644 index 0000000000..148212c951 --- /dev/null +++ b/activesupport/lib/active_support/dependencies/interlock.rb @@ -0,0 +1,41 @@ +require 'active_support/concurrency/share_lock' + +module ActiveSupport #:nodoc: + module Dependencies #:nodoc: + class Interlock + def initialize # :nodoc: + @lock = ActiveSupport::Concurrency::ShareLock.new(true) + end + + def loading + @lock.exclusive do + yield + end + end + + # Attempt to obtain a "loading" (exclusive) lock. If possible, + # execute the supplied block while holding the lock. If there is + # concurrent activity, return immediately (without executing the + # block) instead of waiting. + def attempt_loading + @lock.exclusive(true) do + yield + end + end + + def start_running + @lock.start_sharing + end + + def done_running + @lock.stop_sharing + end + + def running + @lock.sharing do + yield + end + end + end + end +end diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 4f71f13971..c5d35b84f0 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -188,7 +188,7 @@ module ActiveSupport # dup[:a][:c] # => "c" def dup self.class.new(self).tap do |new_hash| - new_hash.default = default + set_defaults(new_hash) end end @@ -238,16 +238,20 @@ module ActiveSupport def to_options!; self end def select(*args, &block) + return to_enum(:select) unless block_given? dup.tap { |hash| hash.select!(*args, &block) } end def reject(*args, &block) + return to_enum(:reject) unless block_given? dup.tap { |hash| hash.reject!(*args, &block) } end # Convert to a regular hash with string keys. def to_hash - _new_hash = Hash.new(default) + _new_hash = Hash.new + set_defaults(_new_hash) + each do |key, value| _new_hash[key] = convert_value(value, for: :to_hash) end @@ -275,6 +279,14 @@ module ActiveSupport value end end + + def set_defaults(target) + if default_proc + target.default_proc = default_proc.dup + else + target.default = default + end + end end end diff --git a/activesupport/lib/active_support/ordered_options.rb b/activesupport/lib/active_support/ordered_options.rb index bc0326473d..45864990ce 100644 --- a/activesupport/lib/active_support/ordered_options.rb +++ b/activesupport/lib/active_support/ordered_options.rb @@ -6,6 +6,7 @@ module ActiveSupport # h[:girl] = 'Mary' # h[:boy] # => 'John' # h[:girl] # => 'Mary' + # h[:dog] # => nil # # Using +OrderedOptions+, the above code could be reduced to: # @@ -14,6 +15,13 @@ module ActiveSupport # h.girl = 'Mary' # h.boy # => 'John' # h.girl # => 'Mary' + # h.dog # => nil + # + # To raise an exception when the value is blank, append a + # bang to the key name, like: + # + # h.dog! # => raises KeyError + # class OrderedOptions < Hash alias_method :_get, :[] # preserve the original #[] method protected :_get # make it protected diff --git a/activesupport/lib/active_support/testing/method_call_assertions.rb b/activesupport/lib/active_support/testing/method_call_assertions.rb new file mode 100644 index 0000000000..e3cbe40308 --- /dev/null +++ b/activesupport/lib/active_support/testing/method_call_assertions.rb @@ -0,0 +1,35 @@ +module ActiveSupport + module Testing + module MethodCallAssertions # :nodoc: + private + def assert_called(object, method_name, message = nil, times: 1) + times_called = 0 + + object.stub(method_name, -> { times_called += 1 }) { yield } + + error = "Expected #{method_name} to be called #{times} times, " \ + "but was called #{times_called} times" + error = "#{message}.\n#{error}" if message + assert_equal times, times_called, error + end + + def assert_called_with(object, method_name, args = [], returns: nil) + mock = Minitest::Mock.new + + if args.all? { |arg| arg.is_a?(Array) } + args.each { |arg| mock.expect(:call, returns, arg) } + else + mock.expect(:call, returns, args) + end + + object.stub(method_name, mock) { yield } + + mock.verify + end + + def assert_not_called(object, method_name, message = nil, &block) + assert_called(object, method_name, message, times: 0, &block) + end + end + end +end
\ No newline at end of file diff --git a/activesupport/test/abstract_unit.rb b/activesupport/test/abstract_unit.rb index 9b88a2dcc1..65a8edbabb 100644 --- a/activesupport/test/abstract_unit.rb +++ b/activesupport/test/abstract_unit.rb @@ -15,6 +15,7 @@ silence_warnings do end require 'active_support/testing/autorun' +require 'active_support/testing/method_call_assertions' ENV['NO_RELOAD'] = '1' require 'active_support' @@ -38,4 +39,7 @@ def jruby_skip(message = '') end require 'minitest/mock' -require 'mocha/setup' # FIXME: stop using mocha + +class ActiveSupport::TestCase + include ActiveSupport::Testing::MethodCallAssertions +end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 69aea6213f..74ceff44f9 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -133,37 +133,42 @@ class CacheStoreSettingTest < ActiveSupport::TestCase end def test_mem_cache_fragment_cache_store - Dalli::Client.expects(:new).with(%w[localhost], {}) - store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost" - assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + assert_called_with(Dalli::Client, :new, [%w[localhost], {}]) do + store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost" + assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + end end def test_mem_cache_fragment_cache_store_with_given_mem_cache mem_cache = Dalli::Client.new - Dalli::Client.expects(:new).never - store = ActiveSupport::Cache.lookup_store :mem_cache_store, mem_cache - assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + assert_not_called(Dalli::Client, :new) do + store = ActiveSupport::Cache.lookup_store :mem_cache_store, mem_cache + assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + end end def test_mem_cache_fragment_cache_store_with_not_dalli_client - Dalli::Client.expects(:new).never - memcache = Object.new - assert_raises(ArgumentError) do - ActiveSupport::Cache.lookup_store :mem_cache_store, memcache + assert_not_called(Dalli::Client, :new) do + memcache = Object.new + assert_raises(ArgumentError) do + ActiveSupport::Cache.lookup_store :mem_cache_store, memcache + end end end def test_mem_cache_fragment_cache_store_with_multiple_servers - Dalli::Client.expects(:new).with(%w[localhost 192.168.1.1], {}) - store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1' - assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + assert_called_with(Dalli::Client, :new, [%w[localhost 192.168.1.1], {}]) do + store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1' + assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + end end def test_mem_cache_fragment_cache_store_with_options - Dalli::Client.expects(:new).with(%w[localhost 192.168.1.1], { :timeout => 10 }) - store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1', :namespace => 'foo', :timeout => 10 - assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) - assert_equal 'foo', store.options[:namespace] + assert_called_with(Dalli::Client, :new, [%w[localhost 192.168.1.1], { :timeout => 10 }]) do + store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1', :namespace => 'foo', :timeout => 10 + assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) + assert_equal 'foo', store.options[:namespace] + end end def test_object_assigned_fragment_cache_store @@ -224,13 +229,15 @@ module CacheStoreBehavior def test_fetch_without_cache_miss @cache.write('foo', 'bar') - @cache.expects(:write).never - assert_equal 'bar', @cache.fetch('foo') { 'baz' } + assert_not_called(@cache, :write) do + assert_equal 'bar', @cache.fetch('foo') { 'baz' } + end end def test_fetch_with_cache_miss - @cache.expects(:write).with('foo', 'baz', @cache.options) - assert_equal 'baz', @cache.fetch('foo') { 'baz' } + assert_called_with(@cache, :write, ['foo', 'baz', @cache.options]) do + assert_equal 'baz', @cache.fetch('foo') { 'baz' } + end end def test_fetch_with_cache_miss_passes_key_to_block @@ -245,15 +252,18 @@ module CacheStoreBehavior def test_fetch_with_forced_cache_miss @cache.write('foo', 'bar') - @cache.expects(:read).never - @cache.expects(:write).with('foo', 'bar', @cache.options.merge(:force => true)) - @cache.fetch('foo', :force => true) { 'bar' } + assert_not_called(@cache, :read) do + assert_called_with(@cache, :write, ['foo', 'bar', @cache.options.merge(:force => true)]) do + @cache.fetch('foo', :force => true) { 'bar' } + end + end end def test_fetch_with_cached_nil @cache.write('foo', nil) - @cache.expects(:write).never - assert_nil @cache.fetch('foo') { 'baz' } + assert_not_called(@cache, :write) do + assert_nil @cache.fetch('foo') { 'baz' } + end end def test_should_read_and_write_hash @@ -304,8 +314,9 @@ module CacheStoreBehavior end def test_multi_with_objects - foo = stub(:title => 'FOO!', :cache_key => 'foo') - bar = stub(:cache_key => 'bar') + cache_struct = Struct.new(:cache_key, :title) + foo = cache_struct.new('foo', 'FOO!') + bar = cache_struct.new('bar') @cache.write('bar', 'BAM!') @@ -774,9 +785,10 @@ class FileStoreTest < ActiveSupport::TestCase end def test_log_exception_when_cache_read_fails - File.expects(:exist?).raises(StandardError, "failed") - @cache.send(:read_entry, "winston", {}) - assert @buffer.string.present? + File.stub(:exist?, -> { raise StandardError.new("failed") }) do + @cache.send(:read_entry, "winston", {}) + assert @buffer.string.present? + end end def test_cleanup_removes_all_expired_entries diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index e10bee5e00..4624338df1 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -524,6 +524,10 @@ class HashExtTest < ActiveSupport::TestCase end def test_indifferent_reverse_merging + hash = HashWithIndifferentAccess.new key: :old_value + hash.reverse_merge! key: :new_value + assert_equal :old_value, hash[:key] + hash = HashWithIndifferentAccess.new('some' => 'value', 'other' => 'value') hash.reverse_merge!(:some => 'noclobber', :another => 'clobber') assert_equal 'value', hash[:some] @@ -547,6 +551,11 @@ class HashExtTest < ActiveSupport::TestCase assert_instance_of ActiveSupport::HashWithIndifferentAccess, hash end + def test_indifferent_select_returns_enumerator + enum = ActiveSupport::HashWithIndifferentAccess.new(@strings).select + assert_instance_of Enumerator, enum + end + def test_indifferent_select_returns_a_hash_when_unchanged hash = ActiveSupport::HashWithIndifferentAccess.new(@strings).select {|k,v| true} @@ -568,6 +577,11 @@ class HashExtTest < ActiveSupport::TestCase assert_instance_of ActiveSupport::HashWithIndifferentAccess, hash end + def test_indifferent_reject_returns_enumerator + enum = ActiveSupport::HashWithIndifferentAccess.new(@strings).reject + assert_instance_of Enumerator, enum + end + def test_indifferent_reject_bang indifferent_strings = ActiveSupport::HashWithIndifferentAccess.new(@strings) indifferent_strings.reject! {|k,v| v != 1} @@ -961,10 +975,11 @@ class HashExtTest < ActiveSupport::TestCase assert_raise(RuntimeError) { original.except!(:a) } end - def test_except_with_mocha_expectation_on_original + def test_except_does_not_delete_values_in_original original = { :a => 'x', :b => 'y' } - original.expects(:delete).never - original.except(:a) + assert_not_called(original, :delete) do + original.except(:a) + end end def test_compact @@ -998,6 +1013,37 @@ class HashExtTest < ActiveSupport::TestCase assert hash.key?('a') assert_equal 1, hash[:a] end + + def test_dup_with_default_proc + hash = HashWithIndifferentAccess.new + hash.default_proc = proc { |h, v| raise "walrus" } + assert_nothing_raised { hash.dup } + end + + def test_dup_with_default_proc_sets_proc + hash = HashWithIndifferentAccess.new + hash.default_proc = proc { |h, k| k + 1 } + new_hash = hash.dup + + assert_equal 3, new_hash[2] + + new_hash.default = 2 + assert_equal 2, new_hash[:non_existant] + end + + def test_to_hash_with_raising_default_proc + hash = HashWithIndifferentAccess.new + hash.default_proc = proc { |h, k| raise "walrus" } + + assert_nothing_raised { hash.to_hash } + end + + def test_new_from_hash_copying_default_should_not_raise_when_default_proc_does + hash = Hash.new + hash.default_proc = proc { |h, k| raise "walrus" } + + assert_nothing_raised { HashWithIndifferentAccess.new_from_hash_copying_default(hash) } + end end class IWriteMyOwnXML diff --git a/activesupport/test/core_ext/object/json_gem_encoding_test.rb b/activesupport/test/core_ext/object/json_gem_encoding_test.rb new file mode 100644 index 0000000000..02ab17fb64 --- /dev/null +++ b/activesupport/test/core_ext/object/json_gem_encoding_test.rb @@ -0,0 +1,66 @@ +require 'abstract_unit' +require 'json' +require 'json/encoding_test_cases' + +# These test cases were added to test that we do not interfere with json gem's +# output when the AS encoder is loaded, primarily for problems reported in +# #20775. They need to be executed in isolation to reproduce the scenario +# correctly, because other test cases might have already loaded additional +# dependencies. + +# The AS::JSON encoder requires the BigDecimal core_ext, which, unfortunately, +# changes the BigDecimal#to_s output, and consequently the JSON gem output. So +# we need to require this unfront to ensure we don't get a false failure, but +# ideally we should just fix the BigDecimal core_ext to not change to_s without +# arguments. +require 'active_support/core_ext/big_decimal' + +class JsonGemEncodingTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + JSONTest::EncodingTestCases.constants.each_with_index do |name| + JSONTest::EncodingTestCases.const_get(name).each_with_index do |(subject, _), i| + test("#{name[0..-6].underscore} #{i}") do + assert_same_with_or_without_active_support(subject) + end + end + end + + class CustomToJson + def to_json(*) + '"custom"' + end + end + + test "custom to_json" do + assert_same_with_or_without_active_support(CustomToJson.new) + end + + private + def require_or_skip(file) + require(file) || skip("'#{file}' was already loaded") + end + + def assert_same_with_or_without_active_support(subject) + begin + expected = JSON.generate(subject, quirks_mode: true) + rescue JSON::GeneratorError => e + exception = e + end + + require_or_skip 'active_support/core_ext/object/json' + + if exception + assert_raises_with_message JSON::GeneratorError, e.message do + JSON.generate(subject, quirks_mode: true) + end + else + assert_equal expected, JSON.generate(subject, quirks_mode: true) + end + end + + def assert_raises_with_message(exception_class, message, &block) + err = assert_raises(exception_class) { block.call } + assert_match message, err.message + end +end diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index 5b48bf328c..477a42114b 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -2,6 +2,7 @@ require 'abstract_unit' require 'active_support/time' require 'time_zone_test_helpers' require 'active_support/core_ext/string/strip' +require 'yaml' class TimeWithZoneTest < ActiveSupport::TestCase include TimeZoneTestHelpers @@ -482,22 +483,24 @@ class TimeWithZoneTest < ActiveSupport::TestCase end def test_method_missing_with_non_time_return_value - @twz.time.expects(:foo).returns('bar') + time = @twz.time + def time.foo; 'bar'; end assert_equal 'bar', @twz.foo end def test_date_part_value_methods twz = ActiveSupport::TimeWithZone.new(Time.utc(1999,12,31,19,18,17,500), @time_zone) - twz.expects(:method_missing).never - assert_equal 1999, twz.year - assert_equal 12, twz.month - assert_equal 31, twz.day - assert_equal 14, twz.hour - assert_equal 18, twz.min - assert_equal 17, twz.sec - assert_equal 500, twz.usec - assert_equal 5, twz.wday - assert_equal 365, twz.yday + assert_not_called(twz, :method_missing) do + assert_equal 1999, twz.year + assert_equal 12, twz.month + assert_equal 31, twz.day + assert_equal 14, twz.hour + assert_equal 18, twz.min + assert_equal 17, twz.sec + assert_equal 500, twz.usec + assert_equal 5, twz.wday + assert_equal 365, twz.yday + end end def test_usec_returns_0_when_datetime_is_wrapped diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 6dce7560dd..757e600646 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -721,12 +721,13 @@ class DependenciesTest < ActiveSupport::TestCase end def test_unloadable_constants_should_receive_callback - Object.const_set :C, Class.new + Object.const_set :C, Class.new { def self.before_remove_const; end } C.unloadable - C.expects(:before_remove_const).once - assert C.respond_to?(:before_remove_const) - ActiveSupport::Dependencies.clear - assert !defined?(C) + assert_called(C, :before_remove_const, times: 1) do + assert C.respond_to?(:before_remove_const) + ActiveSupport::Dependencies.clear + assert !defined?(C) + end ensure remove_constants(:C) end diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index 20bd8ee5dd..7e8844b301 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -256,17 +256,17 @@ class DeprecationTest < ActiveSupport::TestCase end def test_deprecate_with_custom_deprecator - custom_deprecator = mock('Deprecator') do - expects(:deprecation_warning) - end + custom_deprecator = Struct.new(:deprecation_warning).new - klass = Class.new do - def method + assert_called_with(custom_deprecator, :deprecation_warning, [:method, nil]) do + klass = Class.new do + def method + end + deprecate :method, deprecator: custom_deprecator end - deprecate :method, deprecator: custom_deprecator - end - klass.new.method + klass.new.method + end end def test_deprecated_constant_with_deprecator_given diff --git a/activesupport/test/hash_with_indifferent_access_test.rb b/activesupport/test/hash_with_indifferent_access_test.rb deleted file mode 100644 index 1facd691fa..0000000000 --- a/activesupport/test/hash_with_indifferent_access_test.rb +++ /dev/null @@ -1,11 +0,0 @@ -require 'abstract_unit' -require 'active_support/hash_with_indifferent_access' - -class HashWithIndifferentAccessTest < ActiveSupport::TestCase - def test_reverse_merge - hash = HashWithIndifferentAccess.new key: :old_value - hash.reverse_merge! key: :new_value - assert_equal :old_value, hash[:key] - end - -end diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index ee47b97a8a..9f4b62fd8b 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -4,122 +4,24 @@ require 'active_support/core_ext/string/inflections' require 'active_support/json' require 'active_support/time' require 'time_zone_test_helpers' +require 'json/encoding_test_cases' class TestJSONEncoding < ActiveSupport::TestCase include TimeZoneTestHelpers - class Foo - def initialize(a, b) - @a, @b = a, b - end - end - - class Hashlike - def to_hash - { :foo => "hello", :bar => "world" } - end - end - - class Custom - def initialize(serialized) - @serialized = serialized - end - - def as_json(options = nil) - @serialized - end - end - - class CustomWithOptions - attr_accessor :foo, :bar - - def as_json(options={}) - options[:only] = %w(foo bar) - super(options) - end - end - - class OptionsTest - def as_json(options = :default) - options - end - end - - class HashWithAsJson < Hash - attr_accessor :as_json_called - - def initialize(*) - super - end - - def as_json(options={}) - @as_json_called = true - super - end - end - - TrueTests = [[ true, %(true) ]] - FalseTests = [[ false, %(false) ]] - NilTests = [[ nil, %(null) ]] - NumericTests = [[ 1, %(1) ], - [ 2.5, %(2.5) ], - [ 0.0/0.0, %(null) ], - [ 1.0/0.0, %(null) ], - [ -1.0/0.0, %(null) ], - [ BigDecimal('0.0')/BigDecimal('0.0'), %(null) ], - [ BigDecimal('2.5'), %("#{BigDecimal('2.5')}") ]] - - StringTests = [[ 'this is the <string>', %("this is the \\u003cstring\\u003e")], - [ 'a "string" with quotes & an ampersand', %("a \\"string\\" with quotes \\u0026 an ampersand") ], - [ 'http://test.host/posts/1', %("http://test.host/posts/1")], - [ "Control characters: \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\u2028\u2029", - %("Control characters: \\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\b\\t\\n\\u000b\\f\\r\\u000e\\u000f\\u0010\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017\\u0018\\u0019\\u001a\\u001b\\u001c\\u001d\\u001e\\u001f\\u2028\\u2029") ]] - - ArrayTests = [[ ['a', 'b', 'c'], %([\"a\",\"b\",\"c\"]) ], - [ [1, 'a', :b, nil, false], %([1,\"a\",\"b\",null,false]) ]] - - RangeTests = [[ 1..2, %("1..2")], - [ 1...2, %("1...2")], - [ 1.5..2.5, %("1.5..2.5")]] - - SymbolTests = [[ :a, %("a") ], - [ :this, %("this") ], - [ :"a b", %("a b") ]] - - ObjectTests = [[ Foo.new(1, 2), %({\"a\":1,\"b\":2}) ]] - HashlikeTests = [[ Hashlike.new, %({\"bar\":\"world\",\"foo\":\"hello\"}) ]] - CustomTests = [[ Custom.new("custom"), '"custom"' ], - [ Custom.new(nil), 'null' ], - [ Custom.new(:a), '"a"' ], - [ Custom.new([ :foo, "bar" ]), '["foo","bar"]' ], - [ Custom.new({ :foo => "hello", :bar => "world" }), '{"bar":"world","foo":"hello"}' ], - [ Custom.new(Hashlike.new), '{"bar":"world","foo":"hello"}' ], - [ Custom.new(Custom.new(Custom.new(:a))), '"a"' ]] - - RegexpTests = [[ /^a/, '"(?-mix:^a)"' ], [/^\w{1,2}[a-z]+/ix, '"(?ix-m:^\\\\w{1,2}[a-z]+)"']] - - DateTests = [[ Date.new(2005,2,1), %("2005/02/01") ]] - TimeTests = [[ Time.utc(2005,2,1,15,15,10), %("2005/02/01 15:15:10 +0000") ]] - DateTimeTests = [[ DateTime.civil(2005,2,1,15,15,10), %("2005/02/01 15:15:10 +0000") ]] - - StandardDateTests = [[ Date.new(2005,2,1), %("2005-02-01") ]] - StandardTimeTests = [[ Time.utc(2005,2,1,15,15,10), %("2005-02-01T15:15:10.000Z") ]] - StandardDateTimeTests = [[ DateTime.civil(2005,2,1,15,15,10), %("2005-02-01T15:15:10.000+00:00") ]] - StandardStringTests = [[ 'this is the <string>', %("this is the <string>")]] - def sorted_json(json) return json unless json =~ /^\{.*\}$/ '{' + json[1..-2].split(',').sort.join(',') + '}' end - constants.grep(/Tests$/).each do |class_tests| + JSONTest::EncodingTestCases.constants.each do |class_tests| define_method("test_#{class_tests[0..-6].underscore}") do begin prev = ActiveSupport.use_standard_json_time_format ActiveSupport.escape_html_entities_in_json = class_tests !~ /^Standard/ ActiveSupport.use_standard_json_time_format = class_tests =~ /^Standard/ - self.class.const_get(class_tests).each do |pair| + JSONTest::EncodingTestCases.const_get(class_tests).each do |pair| assert_equal pair.last, sorted_json(ActiveSupport::JSON.encode(pair.first)) end ensure @@ -224,7 +126,7 @@ class TestJSONEncoding < ActiveSupport::TestCase end def test_hash_like_with_options - h = Hashlike.new + h = JSONTest::Hashlike.new json = h.to_json :only => [:foo] assert_equal({"foo"=>"hello"}, JSON.parse(json)) @@ -345,6 +247,15 @@ class TestJSONEncoding < ActiveSupport::TestCase assert_equal(%([{"address":{"city":"London"}},{"address":{"city":"Paris"}}]), json) end + class CustomWithOptions + attr_accessor :foo, :bar + + def as_json(options={}) + options[:only] = %w(foo bar) + super(options) + end + end + def test_hash_to_json_should_not_keep_options_around f = CustomWithOptions.new f.foo = "hello" @@ -365,6 +276,12 @@ class TestJSONEncoding < ActiveSupport::TestCase {"foo"=>"other_foo","test"=>"other_test"}], ActiveSupport::JSON.decode(array.to_json)) end + class OptionsTest + def as_json(options = :default) + options + end + end + def test_hash_as_json_without_options json = { foo: OptionsTest.new }.as_json assert_equal({"foo" => :default}, json) @@ -412,6 +329,19 @@ class TestJSONEncoding < ActiveSupport::TestCase assert_equal false, false.as_json end + class HashWithAsJson < Hash + attr_accessor :as_json_called + + def initialize(*) + super + end + + def as_json(options={}) + @as_json_called = true + super + end + end + def test_json_gem_dump_by_passing_active_support_encoder h = HashWithAsJson.new h[:foo] = "hello" diff --git a/activesupport/test/json/encoding_test_cases.rb b/activesupport/test/json/encoding_test_cases.rb new file mode 100644 index 0000000000..0159ba8606 --- /dev/null +++ b/activesupport/test/json/encoding_test_cases.rb @@ -0,0 +1,88 @@ +require 'bigdecimal' + +module JSONTest + class Foo + def initialize(a, b) + @a, @b = a, b + end + end + + class Hashlike + def to_hash + { :foo => "hello", :bar => "world" } + end + end + + class Custom + def initialize(serialized) + @serialized = serialized + end + + def as_json(options = nil) + @serialized + end + end + + class MyStruct < Struct.new(:name, :value) + def initialize(*) + @unused = "unused instance variable" + super + end + end + + module EncodingTestCases + TrueTests = [[ true, %(true) ]] + FalseTests = [[ false, %(false) ]] + NilTests = [[ nil, %(null) ]] + NumericTests = [[ 1, %(1) ], + [ 2.5, %(2.5) ], + [ 0.0/0.0, %(null) ], + [ 1.0/0.0, %(null) ], + [ -1.0/0.0, %(null) ], + [ BigDecimal('0.0')/BigDecimal('0.0'), %(null) ], + [ BigDecimal('2.5'), %("#{BigDecimal('2.5')}") ]] + + StringTests = [[ 'this is the <string>', %("this is the \\u003cstring\\u003e")], + [ 'a "string" with quotes & an ampersand', %("a \\"string\\" with quotes \\u0026 an ampersand") ], + [ 'http://test.host/posts/1', %("http://test.host/posts/1")], + [ "Control characters: \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\u2028\u2029", + %("Control characters: \\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\b\\t\\n\\u000b\\f\\r\\u000e\\u000f\\u0010\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017\\u0018\\u0019\\u001a\\u001b\\u001c\\u001d\\u001e\\u001f\\u2028\\u2029") ]] + + ArrayTests = [[ ['a', 'b', 'c'], %([\"a\",\"b\",\"c\"]) ], + [ [1, 'a', :b, nil, false], %([1,\"a\",\"b\",null,false]) ]] + + HashTests = [[ {foo: "bar"}, %({\"foo\":\"bar\"}) ], + [ {1 => 1, 2 => 'a', 3 => :b, 4 => nil, 5 => false}, %({\"1\":1,\"2\":\"a\",\"3\":\"b\",\"4\":null,\"5\":false}) ]] + + RangeTests = [[ 1..2, %("1..2")], + [ 1...2, %("1...2")], + [ 1.5..2.5, %("1.5..2.5")]] + + SymbolTests = [[ :a, %("a") ], + [ :this, %("this") ], + [ :"a b", %("a b") ]] + + ObjectTests = [[ Foo.new(1, 2), %({\"a\":1,\"b\":2}) ]] + HashlikeTests = [[ Hashlike.new, %({\"bar\":\"world\",\"foo\":\"hello\"}) ]] + StructTests = [[ MyStruct.new(:foo, "bar"), %({\"name\":\"foo\",\"value\":\"bar\"}) ], + [ MyStruct.new(nil, nil), %({\"name\":null,\"value\":null}) ]] + CustomTests = [[ Custom.new("custom"), '"custom"' ], + [ Custom.new(nil), 'null' ], + [ Custom.new(:a), '"a"' ], + [ Custom.new([ :foo, "bar" ]), '["foo","bar"]' ], + [ Custom.new({ :foo => "hello", :bar => "world" }), '{"bar":"world","foo":"hello"}' ], + [ Custom.new(Hashlike.new), '{"bar":"world","foo":"hello"}' ], + [ Custom.new(Custom.new(Custom.new(:a))), '"a"' ]] + + RegexpTests = [[ /^a/, '"(?-mix:^a)"' ], [/^\w{1,2}[a-z]+/ix, '"(?ix-m:^\\\\w{1,2}[a-z]+)"']] + + DateTests = [[ Date.new(2005,2,1), %("2005/02/01") ]] + TimeTests = [[ Time.utc(2005,2,1,15,15,10), %("2005/02/01 15:15:10 +0000") ]] + DateTimeTests = [[ DateTime.civil(2005,2,1,15,15,10), %("2005/02/01 15:15:10 +0000") ]] + + StandardDateTests = [[ Date.new(2005,2,1), %("2005-02-01") ]] + StandardTimeTests = [[ Time.utc(2005,2,1,15,15,10), %("2005-02-01T15:15:10.000Z") ]] + StandardDateTimeTests = [[ DateTime.civil(2005,2,1,15,15,10), %("2005-02-01T15:15:10.000+00:00") ]] + StandardStringTests = [[ 'this is the <string>', %("this is the <string>")]] + end +end diff --git a/activesupport/test/log_subscriber_test.rb b/activesupport/test/log_subscriber_test.rb index 2a0e8d20ed..998a6887c5 100644 --- a/activesupport/test/log_subscriber_test.rb +++ b/activesupport/test/log_subscriber_test.rb @@ -82,10 +82,11 @@ class SyncLogSubscriberTest < ActiveSupport::TestCase def test_does_not_send_the_event_if_logger_is_nil ActiveSupport::LogSubscriber.logger = nil - @log_subscriber.expects(:some_event).never - ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber - instrument "some_event.my_log_subscriber" - wait + assert_not_called(@log_subscriber, :some_event) do + ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber + instrument "some_event.my_log_subscriber" + wait + end end def test_does_not_fail_with_non_namespaced_events diff --git a/activesupport/test/multibyte_unicode_database_test.rb b/activesupport/test/multibyte_unicode_database_test.rb index 52ae266f01..dd33641ec2 100644 --- a/activesupport/test/multibyte_unicode_database_test.rb +++ b/activesupport/test/multibyte_unicode_database_test.rb @@ -11,8 +11,9 @@ class MultibyteUnicodeDatabaseTest < ActiveSupport::TestCase UnicodeDatabase::ATTRIBUTES.each do |attribute| define_method "test_lazy_loading_on_attribute_access_of_#{attribute}" do - @ucd.expects(:load) - @ucd.send(attribute) + assert_called(@ucd, :load) do + @ucd.send(attribute) + end end end diff --git a/activesupport/test/testing/method_call_assertions_test.rb b/activesupport/test/testing/method_call_assertions_test.rb new file mode 100644 index 0000000000..a9908aea0d --- /dev/null +++ b/activesupport/test/testing/method_call_assertions_test.rb @@ -0,0 +1,98 @@ +require 'abstract_unit' +require 'active_support/testing/method_call_assertions' + +class MethodCallAssertionsTest < ActiveSupport::TestCase + include ActiveSupport::Testing::MethodCallAssertions + + class Level + def increment; 1; end + def decrement; end + def <<(arg); end + end + + setup do + @object = Level.new + end + + def test_assert_called_with_defaults_to_expect_once + assert_called @object, :increment do + @object.increment + end + end + + def test_assert_called_more_than_once + assert_called(@object, :increment, times: 2) do + @object.increment + @object.increment + end + end + + def test_assert_called_failure + error = assert_raises(Minitest::Assertion) do + assert_called(@object, :increment) do + # Call nothing... + end + end + + assert_equal "Expected increment to be called 1 times, but was called 0 times.\nExpected: 1\n Actual: 0", error.message + end + + def test_assert_called_with_message + error = assert_raises(Minitest::Assertion) do + assert_called(@object, :increment, 'dang it') do + # Call nothing... + end + end + + assert_match(/dang it.\nExpected increment/, error.message) + end + + def test_assert_called_with + assert_called_with(@object, :increment) do + @object.increment + end + end + + def test_assert_called_with_arguments + assert_called_with(@object, :<<, [ 2 ]) do + @object << 2 + end + end + + def test_assert_called_with_failure + assert_raises(MockExpectationError) do + assert_called_with(@object, :<<, [ 4567 ]) do + @object << 2 + end + end + end + + def test_assert_called_with_returns + assert_called_with(@object, :increment, returns: 1) do + @object.increment + end + end + + def test_assert_called_with_multiple_expected_arguments + assert_called_with(@object, :<<, [ [ 1 ], [ 2 ] ]) do + @object << 1 + @object << 2 + end + end + + def test_assert_not_called + assert_not_called(@object, :decrement) do + @object.increment + end + end + + def test_assert_not_called_failure + error = assert_raises(Minitest::Assertion) do + assert_not_called(@object, :increment) do + @object.increment + end + end + + assert_equal "Expected increment to be called 0 times, but was called 1 times.\nExpected: 0\n Actual: 1", error.message + end +end diff --git a/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb index 34bb0e2995..00d40c4497 100644 --- a/activesupport/test/time_zone_test.rb +++ b/activesupport/test/time_zone_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'active_support/time' require 'time_zone_test_helpers' +require 'yaml' class TimeZoneTest < ActiveSupport::TestCase include TimeZoneTestHelpers diff --git a/activesupport/test/xml_mini_test.rb b/activesupport/test/xml_mini_test.rb index 0e4e7427d2..55e8181b54 100644 --- a/activesupport/test/xml_mini_test.rb +++ b/activesupport/test/xml_mini_test.rb @@ -3,6 +3,7 @@ require 'active_support/xml_mini' require 'active_support/builder' require 'active_support/core_ext/hash' require 'active_support/core_ext/big_decimal' +require 'yaml' module XmlMiniTest class RenameKeyTest < ActiveSupport::TestCase diff --git a/guides/bug_report_templates/action_controller_gem.rb b/guides/bug_report_templates/action_controller_gem.rb index 11561c55f9..58ba708a39 100644 --- a/guides/bug_report_templates/action_controller_gem.rb +++ b/guides/bug_report_templates/action_controller_gem.rb @@ -32,7 +32,7 @@ class TestController < ActionController::Base include Rails.application.routes.url_helpers def index - render text: 'Home' + render plain: 'Home' end end diff --git a/guides/bug_report_templates/action_controller_master.rb b/guides/bug_report_templates/action_controller_master.rb index 66887398b9..1a4b736348 100644 --- a/guides/bug_report_templates/action_controller_master.rb +++ b/guides/bug_report_templates/action_controller_master.rb @@ -31,7 +31,7 @@ class TestController < ActionController::Base include Rails.application.routes.url_helpers def index - render text: 'Home' + render plain: 'Home' end end diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index 6f159b2fc4..c31b50fcfc 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -533,6 +533,24 @@ url helper. NOTE: non-`GET` links require [jQuery UJS](https://github.com/rails/jquery-ujs) and won't work in mailer templates. They will result in normal `GET` requests. +### Adding images in Action Mailer Views + +Unlike controllers, the mailer instance doesn't have any context about the +incoming request so you'll need to provide the `:asset_host` parameter yourself. + +As the `:asset_host` usually is consistent across the application you can +configure it globally in config/application.rb: + +```ruby +config.action_mailer.asset_host = 'http://example.com' +``` + +Now you can display an image inside your email. + +```ruby +<%= image_tag 'image.jpg' %> +``` + ### Sending Multipart Emails Action Mailer will automatically send multipart emails if you have different diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index 3c1c3c7873..98c6cbd540 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -1059,14 +1059,6 @@ If `@article.author_ids` is [1], this would return: <input name="article[author_ids][]" type="hidden" value="" /> ``` -#### country_options_for_select - -Returns a string of option tags for pretty much any country in the world. - -#### country_select - -Returns select and option tags for the given object and method, using country_options_for_select to generate the list of option tags. - #### option_groups_from_collection_for_select Returns a string of `option` tags, like `options_from_collection_for_select`, but groups them by `optgroup` tags based on the object relationships of the arguments. @@ -1153,8 +1145,8 @@ If `@article.person_id` is 1, this would become: <select name="article[person_id]"> <option value=""></option> <option value="1" selected="selected">David</option> - <option value="2">Sam</option> - <option value="3">Tobias</option> + <option value="2">Eileen</option> + <option value="3">Rafael</option> </select> ``` diff --git a/guides/source/active_job_basics.md b/guides/source/active_job_basics.md index 22f3c0146a..dd545b56f5 100644 --- a/guides/source/active_job_basics.md +++ b/guides/source/active_job_basics.md @@ -330,6 +330,13 @@ class GuestsCleanupJob < ActiveJob::Base end ``` +### Deserialization + +GlobalID allows serializing full Active Record objects passed to `#perform`. + +If a passed record is deleted after the job is enqueued but before the `#perform` +method is called Active Job will raise an `ActiveJob::DeserializationError` +exception. Job Testing -------------- diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 7932853c11..71ca7a0f66 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -965,8 +965,9 @@ own custom validators. You can also create methods that verify the state of your models and add messages to the `errors` collection when they are invalid. You must then -register these methods by using the `validate` class method, passing in the -symbols for the validation methods' names. +register these methods by using the `validate` +([API](http://api.rubyonrails.org/classes/ActiveModel/Validations/ClassMethods.html#method-i-validate)) +class method, passing in the symbols for the validation methods' names. You can pass more than one symbol for each class method and the respective validations will be run in the same order as they were registered. diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index b0103c9af4..20f11c2bc2 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -31,6 +31,11 @@ the relevant `config/environments/*.rb` file: config.action_controller.perform_caching = true ``` +NOTE: Changing the value of `config.action_controller.perform_caching` will +only have an effect on the caching provided by the Action Controller component. +For instance, it will not impact low-level caching, that we address +[below](#low-level-caching). + ### Page Caching Page caching is a Rails mechanism which allows the request for a generated page @@ -122,7 +127,7 @@ For example, take the following view: Which in turn renders this view: ```erb -<% cache game %> +<% cache game do %> <%= render game %> <% end %> ``` diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 79a80de3cc..9c80e73c73 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -326,7 +326,7 @@ The schema dumper adds one additional configuration option: * `config.action_controller.asset_host` sets the host for the assets. Useful when CDNs are used for hosting assets rather than the application server itself. -* `config.action_controller.perform_caching` configures whether the application should perform caching or not. Set to false in development mode, true in production. +* `config.action_controller.perform_caching` configures whether the application should perform the caching features provided by the Action Controller component or not. Set to false in development mode, true in production. * `config.action_controller.default_static_extension` configures the extension used for cached pages. Defaults to `.html`. @@ -1084,22 +1084,22 @@ development: timeout: 5000 ``` -Since the connection pooling is handled inside of Active Record by default, all application servers (Thin, mongrel, Unicorn etc.) should behave the same. Initially, the database connection pool is empty and it will create additional connections as the demand for them increases, until it reaches the connection pool limit. +Since the connection pooling is handled inside of Active Record by default, all application servers (Thin, mongrel, Unicorn etc.) should behave the same. The database connection pool is initially empty. As demand for connections increases it will create them until it reaches the connection pool limit. -Any one request will check out a connection the first time it requires access to the database, after which it will check the connection back in, at the end of the request, meaning that the additional connection slot will be available again for the next request in the queue. +Any one request will check out a connection the first time it requires access to the database. At the end of the request it will check the connection back in. This means that the additional connection slot will be available again for the next request in the queue. If you try to use more connections than are available, Active Record will block -and wait for a connection from the pool. When it cannot get connection, a timeout -error similar to given below will be thrown. +you and wait for a connection from the pool. If it cannot get a connection, a +timeout error similar to that given below will be thrown. ```ruby ActiveRecord::ConnectionTimeoutError - could not obtain a database connection within 5 seconds. The max pool size is currently 5; consider increasing it: ``` -If you get the above error, you might want to increase the size of connection -pool by incrementing the `pool` option in `database.yml` +If you get the above error, you might want to increase the size of the +connection pool by incrementing the `pool` option in `database.yml` -NOTE. If you are running in a multi-threaded environment, there could be a chance that several threads may be accessing multiple connections simultaneously. So depending on your current request load, you could very well have multiple threads contending for a limited amount of connections. +NOTE. If you are running in a multi-threaded environment, there could be a chance that several threads may be accessing multiple connections simultaneously. So depending on your current request load, you could very well have multiple threads contending for a limited number of connections. Custom configuration diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index c19813c8a5..3b944f1274 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -61,7 +61,7 @@ can expect it to be marked "invalid" as soon as it's reviewed. Sometimes, the line between 'bug' and 'feature' is a hard one to draw. Generally, a feature is anything that adds new behavior, while a bug is -anything that fixes already existing behavior that is misbehaving. Sometimes, +anything that causes incorrect behavior. Sometimes, the core team will have to make a judgement call. That said, the distinction generally just affects which release your patch will get in to; we love feature submissions! They just won't get backported to maintenance branches. @@ -295,7 +295,7 @@ You can run a single test through ruby. For instance: ```bash $ cd actionmailer -$ ruby -w -Itest test/mail_layout_test.rb -n test_explicit_class_layout +$ bundle exec ruby -w -Itest test/mail_layout_test.rb -n test_explicit_class_layout ``` The `-n` option allows you to run a single method instead of the whole @@ -334,7 +334,7 @@ will now run the four of them in turn. You can also run any single test separately: ```bash -$ ARCONN=sqlite3 ruby -Itest test/cases/associations/has_many_associations_test.rb +$ ARCONN=sqlite3 bundle exec ruby -Itest test/cases/associations/has_many_associations_test.rb ``` To run a single test against all adapters, use: diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 11051f71c2..400383cfb5 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -621,7 +621,7 @@ def create end ``` -The `render` method here is taking a very simple hash with a key of `plain` and +The `render` method here is taking a very simple hash with a key of `:plain` and value of `params[:article].inspect`. The `params` method is the object which represents the parameters (or fields) coming in from the form. The `params` method returns an `ActiveSupport::HashWithIndifferentAccess` object, which diff --git a/guides/source/security.md b/guides/source/security.md index 93580d4d4e..485b108d12 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -1026,6 +1026,29 @@ Environmental Security It is beyond the scope of this guide to inform you on how to secure your application code and environments. However, please secure your database configuration, e.g. `config/database.yml`, and your server-side secret, e.g. stored in `config/secrets.yml`. You may want to further restrict access, using environment-specific versions of these files and any others that may contain sensitive information. +### Custom secrets + +Rails generates a `config/secrets.yml`. By default, this file contains the +application's `secret_key_base`, but it could also be used to store other +secrets such as access keys for external APIs. + +The secrets added to this file are accessible via `Rails.application.secrets`. +For example, with the following `config/secrets.yml`: + + development: + secret_key_base: 3b7cd727ee24e8444053437c36cc66c3 + some_api_key: SOMEKEY + +`Rails.application.secrets.some_api_key` returns `SOMEKEY` in the development +environment. + +If you want an exception to be raised when some key is blank, use the bang +version: + +```ruby +Rails.application.secrets.some_api_key! # => raises KeyError +``` + Additional Resources -------------------- diff --git a/guides/source/testing.md b/guides/source/testing.md index 2fd54a48fc..d146df3dc6 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -178,6 +178,14 @@ create test/fixtures/articles.yml ... ``` +You can also generate the test stub for a model using the following command: + +```bash +$ bin/rails generate test_unit:model article title:string body:text +create test/models/article_test.rb +create test/fixtures/articles.yml +``` + The default test stub in `test/models/article_test.rb` looks like this: ```ruby @@ -509,6 +517,14 @@ You should test for things such as: Now that we have used Rails scaffold generator for our `Article` resource, it has already created the controller code and tests. You can take look at the file `articles_controller_test.rb` in the `test/controllers` directory. +The following command will generate a controller test case with a filled up +test for each of the seven default actions. + +```bash +$ bin/rails generate test_unit:scaffold article +create test/controllers/articles_controller_test.rb +``` + Let me take you through one such test, `test_should_get_index` from the file `articles_controller_test.rb`. ```ruby diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 6f9ccec137..88eade5c5a 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -26,7 +26,27 @@ module Rails middleware.use ::Rack::Cache, rack_cache end - middleware.use ::Rack::Lock unless allow_concurrency? + if config.allow_concurrency == false + # User has explicitly opted out of concurrent request + # handling: presumably their code is not threadsafe + + middleware.use ::Rack::Lock + + elsif config.allow_concurrency == :unsafe + # Do nothing, even if we know this is dangerous. This is the + # historical behaviour for true. + + else + # Default concurrency setting: enabled, but safe + + unless config.cache_classes && config.eager_load + # Without cache_classes + eager_load, the load interlock + # is required for proper operation + + middleware.use ::ActionDispatch::LoadInterlock + end + end + middleware.use ::Rack::Runtime middleware.use ::Rack::MethodOverride unless config.api_only middleware.use ::ActionDispatch::RequestId @@ -65,14 +85,6 @@ module Rails config.reload_classes_only_on_change != true || app.reloaders.map(&:updated?).any? end - def allow_concurrency? - if config.allow_concurrency.nil? - config.cache_classes && config.eager_load - else - config.allow_concurrency - end - end - def load_rack_cache rack_cache = config.action_dispatch.rack_cache return unless rack_cache diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 0599e988d9..f8f92792a7 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -86,8 +86,10 @@ module Rails # added in the hook are taken into account. initializer :set_clear_dependencies_hook, group: :all do callback = lambda do - ActiveSupport::DescendantsTracker.clear - ActiveSupport::Dependencies.clear + ActiveSupport::Dependencies.interlock.attempt_loading do + ActiveSupport::DescendantsTracker.clear + ActiveSupport::Dependencies.clear + end end if config.reload_classes_only_on_change diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index 6289d4cc46..d1e445ac70 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -77,17 +77,7 @@ module Rails end def middleware - middlewares = [] - - # FIXME: add Rack::Lock in the case people are using webrick. - # This is to remain backwards compatible for those who are - # running webrick in production. We should consider removing this - # in development. - if server.name == 'Rack::Handler::WEBrick' - middlewares << [::Rack::Lock] - end - - Hash.new(middlewares) + Hash.new([]) end def default_options diff --git a/railties/lib/rails/commands/test.rb b/railties/lib/rails/commands/test.rb index fe5307788a..dd069f081f 100644 --- a/railties/lib/rails/commands/test.rb +++ b/railties/lib/rails/commands/test.rb @@ -1,5 +1,9 @@ require "rails/test_unit/minitest_plugin" -$: << File.expand_path("../../test", APP_PATH) +if defined?(ENGINE_ROOT) + $: << File.expand_path('test', ENGINE_ROOT) +else + $: << File.expand_path('../../test', APP_PATH) +end exit Minitest.run(ARGV) diff --git a/railties/lib/rails/engine/commands.rb b/railties/lib/rails/engine/commands.rb index f39f926109..a6d87b78e4 100644 --- a/railties/lib/rails/engine/commands.rb +++ b/railties/lib/rails/engine/commands.rb @@ -2,7 +2,8 @@ ARGV << '--help' if ARGV.empty? aliases = { "g" => "generate", - "d" => "destroy" + "d" => "destroy", + "t" => "test" } command = ARGV.shift @@ -12,7 +13,7 @@ require ENGINE_PATH engine = ::Rails::Engine.find(ENGINE_ROOT) case command -when 'generate', 'destroy' +when 'generate', 'destroy', 'test' require 'rails/generators' Rails::Generators.namespace = engine.railtie_namespace engine.load_generators @@ -30,6 +31,7 @@ Usage: rails COMMAND [ARGS] The common Rails commands available for engines are: generate Generate new code (short-cut alias: "g") destroy Undo code generated with "generate" (short-cut alias: "d") + test Run tests (short-cut alias: "t") All commands can be run with -h for more information. diff --git a/railties/lib/rails/generators/base.rb b/railties/lib/rails/generators/base.rb index 813b8b629e..6fa413f8b0 100644 --- a/railties/lib/rails/generators/base.rb +++ b/railties/lib/rails/generators/base.rb @@ -103,12 +103,12 @@ module Rails # hook_for :test_framework, as: :controller # end # - # And now it will lookup at: + # And now it will look up at: # # "test_unit:controller", "test_unit" # - # Similarly, if you want it to also lookup in the rails namespace, you just - # need to provide the :in value: + # Similarly, if you want it to also look up in the rails namespace, you + # just need to provide the :in value: # # class AwesomeGenerator < Rails::Generators::Base # hook_for :test_framework, in: :rails, as: :controller diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 4b73313388..b813b083f4 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -356,7 +356,9 @@ module Rails if app_const =~ /^\d/ raise Error, "Invalid application name #{app_name}. Please give a name which does not start with numbers." elsif RESERVED_NAMES.include?(app_name) - raise Error, "Invalid application name #{app_name}. Please give a name which does not match one of the reserved rails words: #{RESERVED_NAMES}" + raise Error, "Invalid application name #{app_name}. Please give a " \ + "name which does not match one of the reserved rails " \ + "words: #{RESERVED_NAMES.join(", ")}" elsif Object.const_defined?(app_const_base) raise Error, "Invalid application name #{app_name}, constant #{app_const_base} is already in use. Please choose another application name." end diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 7f5bf0c8b8..66111004aa 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -364,7 +364,9 @@ task default: :test elsif camelized =~ /^\d/ raise Error, "Invalid plugin name #{original_name}. Please give a name which does not start with numbers." elsif RESERVED_NAMES.include?(name) - raise Error, "Invalid plugin name #{original_name}. Please give a name which does not match one of the reserved rails words: #{RESERVED_NAMES}" + raise Error, "Invalid plugin name #{original_name}. Please give a " \ + "name which does not match one of the reserved rails " \ + "words: #{RESERVED_NAMES.join(", ")}" elsif Object.const_defined?(camelized) raise Error, "Invalid plugin name #{original_name}, constant #{camelized} is already in use. Please choose another plugin name." end diff --git a/railties/lib/rails/generators/rails/plugin/templates/test/integration/navigation_test.rb b/railties/lib/rails/generators/rails/plugin/templates/test/integration/navigation_test.rb index 824caecb24..f5d1ec2046 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/test/integration/navigation_test.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/test/integration/navigation_test.rb @@ -1,10 +1,6 @@ require 'test_helper' class NavigationTest < ActionDispatch::IntegrationTest -<% unless options[:skip_active_record] -%> - fixtures :all -<% end -%> - # test "the truth" do # assert true # end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index ce92ebbf66..d298e8d632 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -26,7 +26,7 @@ module ApplicationTests assert_equal [ "Rack::Sendfile", "ActionDispatch::Static", - "Rack::Lock", + "ActionDispatch::LoadInterlock", "ActiveSupport::Cache::Strategy::LocalCache", "Rack::Runtime", "Rack::MethodOverride", @@ -58,7 +58,7 @@ module ApplicationTests assert_equal [ "Rack::Sendfile", "ActionDispatch::Static", - "Rack::Lock", + "ActionDispatch::LoadInterlock", "ActiveSupport::Cache::Strategy::LocalCache", "Rack::Runtime", "ActionDispatch::RequestId", @@ -121,23 +121,40 @@ module ApplicationTests assert !middleware.include?("ActiveRecord::Migration::CheckPending") end - test "includes lock if cache_classes is set but eager_load is not" do + test "includes interlock if cache_classes is set but eager_load is not" do add_to_config "config.cache_classes = true" boot! - assert middleware.include?("Rack::Lock") + assert_not_includes middleware, "Rack::Lock" + assert_includes middleware, "ActionDispatch::LoadInterlock" + end + + test "includes interlock if cache_classes is off" do + add_to_config "config.cache_classes = false" + boot! + assert_not_includes middleware, "Rack::Lock" + assert_includes middleware, "ActionDispatch::LoadInterlock" end test "does not include lock if cache_classes is set and so is eager_load" do add_to_config "config.cache_classes = true" add_to_config "config.eager_load = true" boot! - assert !middleware.include?("Rack::Lock") + assert_not_includes middleware, "Rack::Lock" + assert_not_includes middleware, "ActionDispatch::LoadInterlock" + end + + test "does not include lock if allow_concurrency is set to :unsafe" do + add_to_config "config.allow_concurrency = :unsafe" + boot! + assert_not_includes middleware, "Rack::Lock" + assert_not_includes middleware, "ActionDispatch::LoadInterlock" end - test "does not include lock if allow_concurrency is set" do - add_to_config "config.allow_concurrency = true" + test "includes lock if allow_concurrency is disabled" do + add_to_config "config.allow_concurrency = false" boot! - assert !middleware.include?("Rack::Lock") + assert_includes middleware, "Rack::Lock" + assert_not_includes middleware, "ActionDispatch::LoadInterlock" end test "removes static asset server if serve_static_files is disabled" do diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index d6e5f4bd89..73e68863f4 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -38,7 +38,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_equal "Invalid plugin name 43things. Please give a name which does not start with numbers.\n", content content = capture(:stderr){ run_generator [File.join(destination_root, "plugin")] } - assert_equal "Invalid plugin name plugin. Please give a name which does not match one of the reserved rails words: [\"application\", \"destroy\", \"plugin\", \"runner\", \"test\"]\n", content + assert_equal "Invalid plugin name plugin. Please give a name which does not match one of the reserved rails words: application, destroy, plugin, runner, test\n", content content = capture(:stderr){ run_generator [File.join(destination_root, "Digest")] } assert_equal "Invalid plugin name Digest, constant Digest is already in use. Please choose another plugin name.\n", content diff --git a/railties/test/generators/scaffold_controller_generator_test.rb b/railties/test/generators/scaffold_controller_generator_test.rb index 648b8d9325..95ef853a11 100644 --- a/railties/test/generators/scaffold_controller_generator_test.rb +++ b/railties/test/generators/scaffold_controller_generator_test.rb @@ -182,7 +182,7 @@ class ScaffoldControllerGeneratorTest < Rails::Generators::TestCase Dir.chdir(engine_path) do quietly { `bin/rails g controller dashboard foo` } - assert_match(/2 runs, 2 assertions, 0 failures, 0 errors/, `bundle exec rake test 2>&1`) + assert_match(/2 runs, 2 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end @@ -193,7 +193,7 @@ class ScaffoldControllerGeneratorTest < Rails::Generators::TestCase Dir.chdir(engine_path) do quietly { `bin/rails g controller dashboard foo` } - assert_match(/2 runs, 2 assertions, 0 failures, 0 errors/, `bundle exec rake test 2>&1`) + assert_match(/2 runs, 2 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index debf1140ab..a853bfbe21 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -488,7 +488,7 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase `bin/rails g scaffold User name:string age:integer; bundle exec rake db:migrate` end - assert_match(/8 runs, 13 assertions, 0 failures, 0 errors/, `bundle exec rake test 2>&1`) + assert_match(/8 runs, 13 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end @@ -502,7 +502,7 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase `bin/rails g scaffold User name:string age:integer; bundle exec rake db:migrate` end - assert_match(/8 runs, 13 assertions, 0 failures, 0 errors/, `bundle exec rake test 2>&1`) + assert_match(/8 runs, 13 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index f983b45d2b..77372fb514 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -56,7 +56,7 @@ module SharedGeneratorTests reserved_words = %w[application destroy plugin runner test] reserved_words.each do |reserved| content = capture(:stderr){ run_generator [File.join(destination_root, reserved)] } - assert_match(/Invalid \w+ name #{reserved}. Please give a name which does not match one of the reserved rails words: \["application", "destroy", "plugin", "runner", "test"\]\n/, content) + assert_match(/Invalid \w+ name #{reserved}. Please give a name which does not match one of the reserved rails words: application, destroy, plugin, runner, test\n/, content) end end |