diff options
190 files changed, 2011 insertions, 1029 deletions
@@ -21,8 +21,8 @@ gem 'turbolinks', github: 'rails/turbolinks', branch: 'master' gem 'arel', github: 'rails/arel', branch: 'master' gem 'mail', github: 'mikel/mail', branch: 'master' -gem 'sprockets', github: 'rails/sprockets', branch: 'master' -gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'master' +gem 'sprockets', '~> 4.0', github: 'rails/sprockets', branch: 'master' +gem 'sprockets-rails', '~> 3.0.0.beta3', github: 'rails/sprockets-rails', branch: 'master' gem 'sass-rails', github: 'rails/sass-rails', branch: 'master' # require: false so bcrypt is loaded only when has_secure_password is used. @@ -33,7 +33,9 @@ gem 'bcrypt', '~> 3.1.10', require: false # This needs to be with require false to avoid # it being automatically loaded by sprockets gem 'uglifier', '>= 1.3.0', require: false -gem 'sass', '>= 3.3', require: false + +# Track stable branch of sass because it doesn't have circular require warnings +gem 'sass', github: 'sass/sass', branch: 'stable', require: false group :doc do gem 'sdoc', '~> 0.4.0' @@ -120,7 +122,7 @@ end # gems that are necessary for ActiveRecord tests with Oracle database if ENV['ORACLE_ENHANCED'] platforms :ruby do - gem 'ruby-oci8', '~> 2.1' + gem 'ruby-oci8', '~> 2.2' end gem 'activerecord-oracle_enhanced-adapter', github: 'rsim/oracle-enhanced', branch: 'master' end diff --git a/Gemfile.lock b/Gemfile.lock index 563a9f56be..1797355194 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -29,7 +29,7 @@ GIT GIT remote: git://github.com/rack/rack.git - revision: c28f271d0c91f45e13bfa8f07bed445ef91f41de + revision: e836fad64ba3a79f8169ef44aa97b7004fca5e86 branch: master specs: rack (2.0.0.alpha) @@ -73,10 +73,10 @@ GIT GIT remote: git://github.com/rails/sprockets-rails.git - revision: 600f981fe79ff2d4179baf84bd18fac5acb58b5e + revision: 77098c5acd9f27613875097ce6587aff9d871d7f branch: master specs: - sprockets-rails (3.0.0.beta2) + sprockets-rails (3.0.0.beta3) actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) @@ -97,6 +97,13 @@ GIT turbolinks (3.0.0) coffee-rails +GIT + remote: git://github.com/sass/sass.git + revision: 4ef8e3167985ace91b2105916756bd93c5d7bba6 + branch: stable + specs: + sass (3.4.18) + PATH remote: . specs: @@ -130,7 +137,7 @@ PATH activesupport (= 5.0.0.alpha) arel (= 7.0.0.alpha) activesupport (5.0.0.alpha) - concurrent-ruby (~> 1.0.0.pre2, < 2.0.0) + concurrent-ruby (~> 1.0.0.pre3, < 2.0.0) i18n (~> 0.7) json (~> 1.7, >= 1.7.7) method_source @@ -179,7 +186,7 @@ GEM coffee-script-source execjs coffee-script-source (1.9.1.1) - concurrent-ruby (1.0.0.pre2) + concurrent-ruby (1.0.0.pre3) connection_pool (2.2.0) dalli (2.7.4) dante (0.2.0) @@ -251,7 +258,6 @@ GEM resque (~> 1.25) rufus-scheduler (~> 3.0) rufus-scheduler (3.1.4) - sass (3.4.18) sdoc (0.4.1) json (~> 1.7, >= 1.7.7) rdoc (~> 4.0) @@ -336,14 +342,14 @@ DEPENDENCIES redcarpet (~> 3.2.3) resque resque-scheduler - sass (>= 3.3) + sass! sass-rails! sdoc (~> 0.4.0) sequel sidekiq sneakers - sprockets! - sprockets-rails! + sprockets (~> 4.0)! + sprockets-rails (~> 3.0.0.beta3)! sqlite3 (~> 1.3.6) stackprof sucker_punch diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 6f49e130d8..ad971b71c9 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -414,7 +414,7 @@ module ActionMailer # * <tt>deliveries</tt> - Keeps an array of all the emails sent out through the Action Mailer with # <tt>delivery_method :test</tt>. Most useful for unit and functional testing. # - # * <tt>deliver_later_queue_name</tt> - The name of the queue used with <tt>deliver_later</tt> + # * <tt>deliver_later_queue_name</tt> - The name of the queue used with <tt>deliver_later</tt>. class Base < AbstractController::Base include DeliveryMethods include Previews diff --git a/actionmailer/lib/action_mailer/gem_version.rb b/actionmailer/lib/action_mailer/gem_version.rb index ac79788cf0..b35d2ed965 100644 --- a/actionmailer/lib/action_mailer/gem_version.rb +++ b/actionmailer/lib/action_mailer/gem_version.rb @@ -1,5 +1,5 @@ module ActionMailer - # Returns the version of the currently loaded Action Mailer as a <tt>Gem::Version</tt> + # Returns the version of the currently loaded Action Mailer as a <tt>Gem::Version</tt>. def self.gem_version Gem::Version.new VERSION::STRING end diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb index c2f671fdac..7e9d916b66 100644 --- a/actionmailer/lib/action_mailer/log_subscriber.rb +++ b/actionmailer/lib/action_mailer/log_subscriber.rb @@ -29,7 +29,7 @@ module ActionMailer end end - # Use the logger configured for ActionMailer::Base + # Use the logger configured for ActionMailer::Base. def logger ActionMailer::Base.logger end diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb index ff2cb0fd01..622d481113 100644 --- a/actionmailer/lib/action_mailer/message_delivery.rb +++ b/actionmailer/lib/action_mailer/message_delivery.rb @@ -60,9 +60,9 @@ module ActionMailer # # Options: # - # * <tt>:wait</tt> - Enqueue the email to be delivered with a delay - # * <tt>:wait_until</tt> - Enqueue the email to be delivered at (after) a specific date / time - # * <tt>:queue</tt> - Enqueue the email on the specified queue + # * <tt>:wait</tt> - Enqueue the email to be delivered with a delay. + # * <tt>:wait_until</tt> - Enqueue the email to be delivered at (after) a specific date / time. + # * <tt>:queue</tt> - Enqueue the email on the specified queue. def deliver_later(options={}) enqueue_delivery :deliver_now, options end diff --git a/actionmailer/lib/action_mailer/preview.rb b/actionmailer/lib/action_mailer/preview.rb index 25ad7ee721..aab92fe8db 100644 --- a/actionmailer/lib/action_mailer/preview.rb +++ b/actionmailer/lib/action_mailer/preview.rb @@ -52,7 +52,7 @@ module ActionMailer extend ActiveSupport::DescendantsTracker class << self - # Returns all mailer preview classes + # Returns all mailer preview classes. def all load_previews if descendants.empty? descendants @@ -68,27 +68,27 @@ module ActionMailer message end - # Returns all of the available email previews + # Returns all of the available email previews. def emails public_instance_methods(false).map(&:to_s).sort end - # Returns true if the email exists + # Returns true if the email exists. def email_exists?(email) emails.include?(email) end - # Returns true if the preview exists + # Returns true if the preview exists. def exists?(preview) all.any?{ |p| p.preview_name == preview } end - # Find a mailer preview by its underscored class name + # Find a mailer preview by its underscored class name. def find(preview) all.find{ |p| p.preview_name == preview } end - # Returns the underscored name of the mailer preview without the suffix + # Returns the underscored name of the mailer preview without the suffix. def preview_name name.sub(/Preview$/, '').underscore end diff --git a/actionmailer/lib/action_mailer/test_case.rb b/actionmailer/lib/action_mailer/test_case.rb index 766215ce96..0aa15e31ba 100644 --- a/actionmailer/lib/action_mailer/test_case.rb +++ b/actionmailer/lib/action_mailer/test_case.rb @@ -57,28 +57,28 @@ module ActionMailer protected - def initialize_test_deliveries + def initialize_test_deliveries # :nodoc: set_delivery_method :test @old_perform_deliveries = ActionMailer::Base.perform_deliveries ActionMailer::Base.perform_deliveries = true end - def restore_test_deliveries + def restore_test_deliveries # :nodoc: restore_delivery_method ActionMailer::Base.perform_deliveries = @old_perform_deliveries ActionMailer::Base.deliveries.clear end - def set_delivery_method(method) + def set_delivery_method(method) # :nodoc: @old_delivery_method = ActionMailer::Base.delivery_method ActionMailer::Base.delivery_method = method end - def restore_delivery_method + def restore_delivery_method # :nodoc: ActionMailer::Base.delivery_method = @old_delivery_method end - def set_expected_mail + def set_expected_mail # :nodoc: @expected = Mail.new @expected.content_type ["text", "plain", { "charset" => charset }] @expected.mime_version = '1.0' diff --git a/actionmailer/lib/action_mailer/test_helper.rb b/actionmailer/lib/action_mailer/test_helper.rb index 4d03a616d2..45cfe16899 100644 --- a/actionmailer/lib/action_mailer/test_helper.rb +++ b/actionmailer/lib/action_mailer/test_helper.rb @@ -2,7 +2,7 @@ require 'active_job' module ActionMailer # Provides helper methods for testing Action Mailer, including #assert_emails - # and #assert_no_emails + # and #assert_no_emails. module TestHelper include ActiveJob::TestHelper diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index bb15edee63..42b4f1d18b 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,19 @@ +* Allow multiple `root` routes in same scope level. Example: + + ```ruby + root 'blog#show', constraints: ->(req) { Hostname.blog_site?(req.host) } + root 'landing#show' + ``` + *Rafael Sales* + +* Fix regression in mounted engine named routes generation for app deployed to + a subdirectory. `relative_url_root` was prepended to the path twice (e.g. + "/subdir/subdir/engine_path" instead of "/subdir/engine_path") + + Fixes #20920. Fixes #21459. + + *Matthew Erhard* + * ActionDispatch::Response#new no longer applies default headers. If you want default headers applied to the response object, then call `ActionDispatch::Response.create`. This change only impacts people who are @@ -10,12 +26,16 @@ To this: - Mime::Type[:HTML] + Mime[:html] This change is so that Rails will not manage a list of constants, and fixes an issue where if a type isn't registered you could possibly get the wrong object. + `Mime[:html]` is available in older versions of Rails, too, so you can + safely change libraries and plugins and maintain compatibility with + multiple versions of Rails. + * `url_for` does not modify its arguments when generating polymorphic URLs. *Bernerd Schaefer* diff --git a/actionpack/lib/abstract_controller/collector.rb b/actionpack/lib/abstract_controller/collector.rb index 3b5128cda5..55654be224 100644 --- a/actionpack/lib/abstract_controller/collector.rb +++ b/actionpack/lib/abstract_controller/collector.rb @@ -4,11 +4,10 @@ module AbstractController module Collector def self.generate_method_for_mime(mime) sym = mime.is_a?(Symbol) ? mime : mime.to_sym - const = sym.upcase class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{sym}(*args, &block) # def html(*args, &block) - custom(Mime::Type[:#{const}], *args, &block) # custom(Mime::Type[:HTML], *args, &block) - end # end + def #{sym}(*args, &block) + custom(Mime[:#{sym}], *args, &block) + end RUBY end @@ -23,9 +22,7 @@ module AbstractController protected def method_missing(symbol, &block) - const_name = symbol.upcase - - unless Mime::Type.registered?(const_name) + unless mime_constant = Mime[symbol] raise NoMethodError, "To respond to a custom format, register it as a MIME type first: " \ "http://guides.rubyonrails.org/action_controller_overview.html#restful-downloads. " \ "If you meant to respond to a variant like :tablet or :phone, not a custom format, " \ @@ -33,8 +30,6 @@ module AbstractController "format.html { |html| html.tablet { ... } }" end - mime_constant = Mime::Type[const_name] - if Mime::SET.include?(mime_constant) AbstractController::Collector.generate_method_for_mime(mime_constant) send(symbol, &block) diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 78b43f2fbe..a73f188623 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -22,13 +22,13 @@ module AbstractController # :api: public def render(*args, &block) options = _normalize_render(*args, &block) - self.response_body = render_to_body(options) + rendered_body = render_to_body(options) if options[:html] _set_html_content_type else _set_rendered_content_type rendered_format end - self.response_body + self.response_body = rendered_body end # Raw rendering of a template to a string. @@ -55,7 +55,7 @@ module AbstractController # Returns Content-Type of rendered content # :api: public def rendered_format - Mime::Type[:TEXT] + Mime[:text] end DEFAULT_PROTECTED_INSTANCE_VARIABLES = Set.new %i( diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index beeaae9d0c..94ec62ec6f 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -174,7 +174,11 @@ module ActionController def response_body=(body) body = [body] unless body.nil? || body.respond_to?(:each) - response.body = body + response.reset_body! + body.each { |part| + next if part.empty? + response.write part + } super end diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index e6d7f958bb..957e7a3019 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -72,31 +72,7 @@ module ActionController #:nodoc: self.status = options[:status] || 200 self.content_type = options[:content_type] if options.key?(:content_type) - self.response_body = FileBody.new(path) - end - - # Avoid having to pass an open file handle as the response body. - # Rack::Sendfile will usually intercept the response and uses - # the path directly, so there is no reason to open the file. - class FileBody #:nodoc: - attr_reader :to_path - - def initialize(path) - @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| - while chunk = file.read(16384) - yield chunk - end - end - end + response.send_file path end # Sends the given binary data to the browser. This method is similar to diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index fe470552b0..0a36fecd27 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -34,7 +34,7 @@ module ActionController # # def authenticate # case request.format - # when Mime::Type[:XML], Mime::Type[:ATOM] + # when Mime[:xml], Mime[:atom] # if user = authenticate_with_http_basic { |u, p| @account.users.authenticate(u, p) } # @current_user = user # else @@ -361,7 +361,7 @@ module ActionController # # def authenticate # case request.format - # when Mime::Type[:XML], Mime::Type[:ATOM] + # when Mime[:xml], Mime[:atom] # if user = authenticate_with_http_token { |t, o| @account.users.authenticate(t, o) } # @current_user = user # else diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 5de5b02b32..7db8d13e24 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -233,10 +233,6 @@ module ActionController body.each { |part| buf.write part } buf end - - def handle_conditional_get! - super unless committed? - end end def process(name) diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index fc42fe5c07..58df5c539e 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -229,14 +229,14 @@ module ActionController #:nodoc: @responses = {} @variant = variant - mimes.each { |mime| @responses[Mime::Type[mime.upcase.to_sym]] = nil } + mimes.each { |mime| @responses[Mime[mime]] = nil } end def any(*args, &block) if args.any? args.each { |type| send(type, &block) } else - custom(Mime::Type[:ALL], &block) + custom(Mime::ALL, &block) end end alias :all :any @@ -251,7 +251,7 @@ module ActionController #:nodoc: end def response - response = @responses.fetch(format, @responses[Mime::Type[:ALL]]) + response = @responses.fetch(format, @responses[Mime::ALL]) if response.is_a?(VariantCollector) # `format.html.phone` - variant inline syntax response.variant elsif response.nil? || response.arity == 0 # `format.html` - just a format, call its block diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index d867c97b46..22e0bb5955 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -68,11 +68,11 @@ module ActionController # ActionController::Renderers.add :csv do |obj, options| # filename = options[:filename] || 'data' # str = obj.respond_to?(:to_csv) ? obj.to_csv : obj.to_s - # send_data str, type: Mime::Type[:CSV], + # send_data str, type: Mime[:csv], # disposition: "attachment; filename=#{filename}.csv" # end # - # Note that we used Mime::Type[:CSV] for the csv mime type as it comes with Rails. + # Note that we used Mime[:csv] for the csv mime type as it comes with Rails. # For a custom renderer, you'll need to register a mime type with # <tt>Mime::Type.register</tt>. # @@ -116,24 +116,24 @@ module ActionController json = json.to_json(options) unless json.kind_of?(String) if options[:callback].present? - if content_type.nil? || content_type == Mime::Type[:JSON] - self.content_type = Mime::Type[:JS] + if content_type.nil? || content_type == Mime[:json] + self.content_type = Mime[:js] end "/**/#{options[:callback]}(#{json})" else - self.content_type ||= Mime::Type[:JSON] + self.content_type ||= Mime[:json] json end end add :js do |js, options| - self.content_type ||= Mime::Type[:JS] + self.content_type ||= Mime[:js] js.respond_to?(:to_js) ? js.to_js(options) : js end add :xml do |xml, options| - self.content_type ||= Mime::Type[:XML] + self.content_type ||= Mime[:xml] xml.respond_to?(:to_xml) ? xml.to_xml(options) : xml end end diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 1ecccf9864..172fbdf954 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -64,7 +64,7 @@ module ActionController end def _set_html_content_type - self.content_type = Mime::Type[:HTML].to_s + self.content_type = Mime[:html].to_s end def _set_rendered_content_type(format) diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 5674eef67b..64f6f7cf51 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -90,8 +90,10 @@ module ActionController #:nodoc: # # class FooController < ApplicationController # protect_from_forgery except: :index + # end # # You can disable forgery protection on controller by skipping the verification before_action: + # # skip_before_action :verify_authenticity_token # # Valid Options: diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 903dba3eb4..130ba61786 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -540,7 +540,7 @@ module ActionController end alias_method :delete_if, :reject! - # Return values that were assigned to the given +keys+. Note that all the + # Returns 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)) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index cf78688126..380e9d29b4 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -34,7 +34,7 @@ module ActionController self.session = session self.session_options = TestSession::DEFAULT_OPTIONS @custom_param_parsers = { - Mime::Type[:XML] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } + Mime[:xml] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } } end @@ -402,7 +402,7 @@ module ActionController MSG @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - @request.env['HTTP_ACCEPT'] ||= [Mime::Type[:JS], Mime::Type[:HTML], Mime::Type[:XML], 'text/xml', Mime::Type[:ALL]].join(', ') + @request.env['HTTP_ACCEPT'] ||= [Mime[:js], Mime[:html], Mime[:xml], 'text/xml', '*/*'].join(', ') __send__(*args).tap do @request.env.delete 'HTTP_X_REQUESTED_WITH' @request.env.delete 'HTTP_ACCEPT' @@ -505,7 +505,7 @@ module ActionController if xhr @request.set_header 'HTTP_X_REQUESTED_WITH', 'XMLHttpRequest' @request.fetch_header('HTTP_ACCEPT') do |k| - @request.set_header k, [Mime::Type[:JS], Mime::Type[:HTML], Mime::Type[:XML], 'text/xml', Mime::Type[:ALL]].join(', ') + @request.set_header k, [Mime[:js], Mime[:html], Mime[:xml], 'text/xml', '*/*'].join(', ') end end diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 1d0a6b6eb3..30ade14c26 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -59,7 +59,7 @@ module ActionDispatch end def last_modified? - have_header? LAST_MODIFIED + has_header? LAST_MODIFIED end def last_modified=(utc_time) @@ -73,7 +73,7 @@ module ActionDispatch end def date? - have_header? DATE + has_header? DATE end def date=(utc_time) @@ -82,24 +82,19 @@ module ActionDispatch def etag=(etag) key = ActiveSupport::Cache.expand_cache_key(etag) - set_header ETAG, %("#{Digest::MD5.hexdigest(key)}") + super %("#{Digest::MD5.hexdigest(key)}") end - def etag - get_header ETAG - end - alias :etag? :etag + def etag?; etag; end private DATE = 'Date'.freeze LAST_MODIFIED = "Last-Modified".freeze - ETAG = "ETag".freeze - CACHE_CONTROL = "Cache-Control".freeze SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public must-revalidate]) def cache_control_segments - if cache_control = get_header(CACHE_CONTROL) + if cache_control = _cache_control cache_control.delete(' ').split(',') else [] @@ -153,11 +148,11 @@ module ActionDispatch control.merge! cache_control if control.empty? - set_header CACHE_CONTROL, DEFAULT_CACHE_CONTROL + self._cache_control = DEFAULT_CACHE_CONTROL elsif control[:no_cache] - set_header CACHE_CONTROL, NO_CACHE + self._cache_control = NO_CACHE if control[:extras] - set_header(CACHE_CONTROL, get_header(CACHE_CONTROL) + ", #{control[:extras].join(', ')}") + self._cache_control = _cache_control + ", #{control[:extras].join(', ')}" end else extras = control[:extras] @@ -169,7 +164,7 @@ module ActionDispatch options << MUST_REVALIDATE if control[:must_revalidate] options.concat(extras) if extras - set_header CACHE_CONTROL, options.join(", ") + self._cache_control = options.join(", ") end end end diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 9c0f39f2e7..9dcab79c3a 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -30,12 +30,12 @@ module ActionDispatch @filtered_path = nil end - # Return a hash of parameters with all sensitive data replaced. + # Returns a hash of parameters with all sensitive data replaced. def filtered_parameters @filtered_parameters ||= parameter_filter.filter(parameters) end - # Return a hash of request.env with all sensitive data replaced. + # Returns a hash of request.env with all sensitive data replaced. def filtered_env @filtered_env ||= env_filter.filter(@env) end diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 9a3aaca3f0..12f81dc1a5 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -49,6 +49,11 @@ module ActionDispatch @req.set_header env_name(key), value end + # Add a value to a multivalued header like Vary or Accept-Encoding. + def add(key, value) + @req.add_header env_name(key), value + end + def key?(key) @req.has_header? env_name(key) end diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index a966c5e452..7acf91902d 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -10,7 +10,7 @@ module ActionDispatch self.ignore_accept_header = false end - # The MIME type of the HTTP request, such as Mime::Type[:XML]. + # The MIME type of the HTTP request, such as Mime[:xml]. # # For backward compatibility, the post \format is extracted from the # X-Post-Data-Format HTTP header if present. @@ -49,9 +49,9 @@ module ActionDispatch # Returns the MIME type for the \format used in the request. # - # GET /posts/5.xml | request.format => Mime::Type[:XML] - # GET /posts/5.xhtml | request.format => Mime::Type[:HTML] - # GET /posts/5 | request.format => Mime::Type[:HTML] or Mime::Type[:JS], or request.accepts.first + # GET /posts/5.xml | request.format => Mime[:xml] + # GET /posts/5.xhtml | request.format => Mime[:html] + # GET /posts/5 | request.format => Mime[:html] or Mime[:js], or request.accepts.first # def format(view_path = []) formats.first || Mime::NullType.instance @@ -70,9 +70,9 @@ module ActionDispatch elsif use_accept_header && valid_accept_header accepts elsif xhr? - [Mime::Type[:JS]] + [Mime[:js]] else - [Mime::Type[:HTML]] + [Mime[:html]] end set_header k, v end @@ -138,14 +138,14 @@ module ActionDispatch # def negotiate_mime(order) formats.each do |priority| - if priority == Mime::Type[:ALL] + if priority == Mime::ALL return order.first elsif order.include?(priority) return priority end end - order.include?(Mime::Type[:ALL]) ? format : nil + order.include?(Mime::ALL) ? format : nil end protected diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 36e90e5855..95094c25c0 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -46,7 +46,8 @@ module Mime end def const_missing(sym) - if Mime::Type.registered?(sym) + ext = sym.downcase + if Mime[ext] ActiveSupport::Deprecation.warn <<-eow Accessing mime types via constants is deprecated. Please change: @@ -54,16 +55,17 @@ Accessing mime types via constants is deprecated. Please change: to: - `Mime::Type[:#{sym}]` + `Mime[:#{ext}]` eow - Mime::Type[sym] + Mime[ext] else super end end def const_defined?(sym, inherit = true) - if Mime::Type.registered?(sym) + ext = sym.downcase + if Mime[ext] ActiveSupport::Deprecation.warn <<-eow Accessing mime types via constants is deprecated. Please change: @@ -71,7 +73,7 @@ Accessing mime types via constants is deprecated. Please change: to: - `Mime::Type.registered?(:#{sym})` + `Mime[:#{ext}]` eow true else @@ -106,7 +108,7 @@ to: def initialize(index, name, q = nil) @index = index @name = name - q ||= 0.0 if @name == Mime::Type[:ALL].to_s # default wildcard match to end of list + q ||= 0.0 if @name == '*/*'.freeze # default wildcard match to end of list @q = ((q || 1.0).to_f * 100).to_i end @@ -131,7 +133,7 @@ to: exchange_xml_items if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - text_xml.name = Mime::Type[:XML].to_s + text_xml.name = Mime[:xml].to_s end # Look for more specific XML-based types and sort them ahead of app/xml @@ -160,7 +162,7 @@ to: end def app_xml_idx - @app_xml_idx ||= index(Mime::Type[:XML].to_s) + @app_xml_idx ||= index(Mime[:xml].to_s) end def text_xml @@ -177,8 +179,6 @@ to: end end - TYPES = {} - class << self TRAILING_STAR_REGEXP = /(text|application)\/\*/ PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/ @@ -187,18 +187,6 @@ to: @register_callbacks << block end - def registered?(symbol) - TYPES.key? symbol - end - - def [](symbol) - TYPES[symbol] - end - - def add_type(symbol, type) - TYPES[symbol] = type - end - def lookup(string) LOOKUP[string] end @@ -215,7 +203,6 @@ to: def register(string, symbol, mime_type_synonyms = [], extension_synonyms = [], skip_lookup = false) new_mime = Type.new(string, symbol, mime_type_synonyms) - add_type symbol.upcase, new_mime SET << new_mime @@ -255,13 +242,13 @@ to: parse_data_with_trailing_star($1) if accept_header =~ TRAILING_STAR_REGEXP end - # For an input of <tt>'text'</tt>, returns <tt>[Mime::Type[:JSON], Mime::Type[:XML], Mime::Type[:ICS], - # Mime::Type[:HTML], Mime::Type[:CSS], Mime::Type[:CSV], Mime::Type[:JS], Mime::Type[:YAML], Mime::Type[:TEXT]</tt>. + # For an input of <tt>'text'</tt>, returns <tt>[Mime[:json], Mime[:xml], Mime[:ics], + # Mime[:html], Mime[:css], Mime[:csv], Mime[:js], Mime[:yaml], Mime[:text]</tt>. # - # For an input of <tt>'application'</tt>, returns <tt>[Mime::Type[:HTML], Mime::Type[:JS], - # Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:ATOM], Mime::Type[:JSON], Mime::Type[:RSS], Mime::Type[:URL_ENCODED_FORM]</tt>. - def parse_data_with_trailing_star(input) - Mime::SET.select { |m| m =~ input } + # For an input of <tt>'application'</tt>, returns <tt>[Mime[:html], Mime[:js], + # Mime[:xml], Mime[:yaml], Mime[:atom], Mime[:json], Mime[:rss], Mime[:url_encoded_form]</tt>. + def parse_data_with_trailing_star(type) + Mime::SET.select { |m| m =~ type } end # This method is opposite of register method. @@ -270,12 +257,12 @@ to: # # Mime::Type.unregister(:mobile) def unregister(symbol) - symbol = symbol.upcase - mime = TYPES.delete symbol - - SET.delete_if { |v| v.eql?(mime) } - LOOKUP.delete_if { |_,v| v.eql?(mime) } - EXTENSION_LOOKUP.delete_if { |_,v| v.eql?(mime) } + symbol = symbol.downcase + if mime = Mime[symbol] + SET.delete_if { |v| v.eql?(mime) } + LOOKUP.delete_if { |_, v| v.eql?(mime) } + EXTENSION_LOOKUP.delete_if { |_, v| v.eql?(mime) } + end end end @@ -343,13 +330,24 @@ to: def respond_to_missing?(method, include_private = false) #:nodoc: method.to_s.ends_with? '?' end + end + + class AllType < Type + include Singleton - class All < Type - def all?; true; end - def html?; true; end + def initialize + super '*/*', :all end + + def all?; true; end + def html?; true; end end + # ALL isn't a real MIME type, so we don't register it for lookup with the + # other concrete types. It's a wildcard match that we use for `respond_to` + # negotiation internals. + ALL = AllType.instance + class NullType include Singleton diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 04828f7c87..87715205d9 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -31,6 +31,3 @@ Mime::Type.register "application/json", :json, %w( text/x-json application/jsonr Mime::Type.register "application/pdf", :pdf, [], %w(pdf) Mime::Type.register "application/zip", :zip, [], %w(zip) - -# Create Mime::Type[:ALL] but do not add it to the SET. -Mime::Type.add_type :ALL, Mime::Type::All.new("*/*", :all, []) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index e3c4392760..248ecfd676 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -4,7 +4,7 @@ module ActionDispatch PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { - Mime::Type[:JSON] => lambda { |raw_post| + Mime[:json] => lambda { |raw_post| data = ActiveSupport::JSON.decode(raw_post) data.is_a?(Hash) ? data : {:_json => data} } diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 85d9c3be00..c54efb6541 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -39,7 +39,7 @@ module ActionDispatch # :nodoc: end def []=(k,v) - if @response.committed? + if @response.sending? || @response.sent? raise ActionDispatch::IllegalStateError, 'header already sent' end @@ -78,6 +78,10 @@ module ActionDispatch # :nodoc: cattr_accessor(:default_headers) include Rack::Response::Helpers + # Aliasing these off because AD::Http::Cache::Response defines them + alias :_cache_control :cache_control + alias :_cache_control= :cache_control= + include ActionDispatch::Http::FilterRedirect include ActionDispatch::Http::Cache::Response include MonitorMixin @@ -156,7 +160,7 @@ module ActionDispatch # :nodoc: yield self if block_given? end - def have_header?(key); headers.key? key; end + def has_header?(key); headers.key? key; end def get_header(key); headers[key]; end def set_header(key, v); headers[key] = v; end def delete_header(key); headers.delete key; end @@ -279,6 +283,10 @@ module ActionDispatch # :nodoc: @stream.body end + def write(string) + @stream.write string + end + EMPTY = " " # Allows you to manually set or override the response body. @@ -294,6 +302,40 @@ module ActionDispatch # :nodoc: end end + # Avoid having to pass an open file handle as the response body. + # Rack::Sendfile will usually intercept the response and uses + # the path directly, so there is no reason to open the file. + class FileBody #:nodoc: + attr_reader :to_path + + def initialize(path) + @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| + while chunk = file.read(16384) + yield chunk + end + end + end + end + + # Send the file stored at +path+ as the response body. + def send_file(path) + commit! + @stream = FileBody.new(path) + end + + def reset_body! + @stream = build_buffer(self, []) + end + def body_parts parts = [] @stream.each { |x| parts << x } @@ -389,7 +431,7 @@ module ActionDispatch # :nodoc: return if content_type ct = parse_content_type - set_content_type(ct.mime_type || Mime::Type[:HTML].to_s, + set_content_type(ct.mime_type || Mime[:html].to_s, ct.charset || self.class.default_charset) end diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index d069bf0205..2793c5668d 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -42,6 +42,7 @@ module ActionDispatch def terminal?; false; end def star?; false; end def cat?; false; end + def group?; false; end end class Terminal < Node # :nodoc: @@ -95,6 +96,7 @@ module ActionDispatch class Group < Unary # :nodoc: def type; :GROUP; end + def group?; true; end end class Star < Unary # :nodoc: diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index e93970046c..5ee8810066 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -46,7 +46,7 @@ module ActionDispatch end def names - @names ||= spec.grep(Nodes::Symbol).map(&:name) + @names ||= spec.find_all(&:symbol?).map(&:name) end def required_names @@ -54,8 +54,8 @@ module ActionDispatch end def optional_names - @optional_names ||= spec.grep(Nodes::Group).flat_map { |group| - group.grep(Nodes::Symbol) + @optional_names ||= spec.find_all(&:group?).flat_map { |group| + group.find_all(&:symbol?) }.map(&:name).uniq end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 7ed77352ae..2889acaeb8 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -396,7 +396,9 @@ module ActionDispatch end def write(headers) - headers[HTTP_HEADER] = make_set_cookie_header headers[HTTP_HEADER] + if header = make_set_cookie_header(headers[HTTP_HEADER]) + headers[HTTP_HEADER] = header + end end mattr_accessor :always_write_cookie diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index f3c6be864f..59c3f9248f 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -53,7 +53,7 @@ module ActionDispatch # resources :posts, :comments # end # - # Alternately, you can add prefixes to your path without using a separate + # Alternatively, you can add prefixes to your path without using a separate # directory by using +scope+. +scope+ takes additional options which # apply to all enclosed routes. # diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 87b826f7d0..921cda91ee 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -402,7 +402,8 @@ module ActionDispatch # because this means it will be matched first. As this is the most popular route # of most Rails applications, this is beneficial. def root(options = {}) - match '/', { :as => :root, :via => :get }.merge!(options) + name = has_named_route?(:root) ? nil : :root + match '/', { as: name, via: :get }.merge!(options) end # Matches a url pattern to one or more routes. @@ -482,7 +483,7 @@ module ActionDispatch # resources :user, param: :name # # You can override <tt>ActiveRecord::Base#to_param</tt> of a related - # model to construct an URL: + # model to construct a URL: # # class User < ActiveRecord::Base # def to_param @@ -665,6 +666,7 @@ module ActionDispatch super(options) else prefix_options = options.slice(*_route.segment_keys) + prefix_options[:relative_url_root] = ''.freeze # we must actually delete prefix segment keys to avoid passing them to next url_for _route.segment_keys.each { |k| options.delete(k) } _routes.url_helpers.send("#{name}_path", prefix_options) @@ -1866,7 +1868,7 @@ to this: # and return nil in case it isn't. Otherwise, we pass the invalid name # forward so the underlying router engine treats it and raises an exception. if as.nil? - candidate unless candidate !~ /\A[_a-z]/i || @set.named_routes.key?(candidate) + candidate unless candidate !~ /\A[_a-z]/i || has_named_route?(candidate) else candidate end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index e4b8d5993e..339e2b7c4a 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -656,14 +656,18 @@ module ActionDispatch RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length, :trailing_slash, :anchor, :params, :only_path, :script_name, - :original_script_name] + :original_script_name, :relative_url_root] def optimize_routes_generation? default_url_options.empty? end def find_script_name(options) - options.delete(:script_name) || relative_url_root || '' + options.delete(:script_name) || find_relative_url_root(options) || '' + end + + def find_relative_url_root(options) + options.delete(:relative_url_root) || relative_url_root end def path_for(options, route_name = nil) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 883cd9c2c3..b6c031dcf4 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -1,7 +1,7 @@ module ActionDispatch module Routing # In <tt>config/routes.rb</tt> you define URL-to-controller mappings, but the reverse - # is also possible: an URL can be generated from one of your routing definitions. + # is also possible: a URL can be generated from one of your routing definitions. # URL generation functionality is centralized in this module. # # See ActionDispatch::Routing for general information about routing and routes.rb. diff --git a/actionpack/lib/action_dispatch/testing/assertions.rb b/actionpack/lib/action_dispatch/testing/assertions.rb index 81fa10a613..8dd0bd63ad 100644 --- a/actionpack/lib/action_dispatch/testing/assertions.rb +++ b/actionpack/lib/action_dispatch/testing/assertions.rb @@ -12,7 +12,7 @@ module ActionDispatch include Rails::Dom::Testing::Assertions def html_document - @html_document ||= if @response.content_type === Mime::Type[:XML] + @html_document ||= if @response.content_type === Mime[:xml] Nokogiri::XML::Document.parse(@response.body) else Nokogiri::HTML::Document.parse(@response.body) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 753cd2073b..7e59bb68cf 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -354,7 +354,7 @@ module ActionDispatch if xhr headers ||= {} headers['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - headers['HTTP_ACCEPT'] ||= [Mime::Type[:JS], Mime::Type[:HTML], Mime::Type[:XML], 'text/xml', Mime::Type[:ALL]].join(', ') + headers['HTTP_ACCEPT'] ||= [Mime[:js], Mime[:html], Mime[:xml], 'text/xml', '*/*'].join(', ') end # this modifies the passed request_env directly diff --git a/actionpack/test/abstract/collector_test.rb b/actionpack/test/abstract/collector_test.rb index 3b36e43c0b..edbb84d462 100644 --- a/actionpack/test/abstract/collector_test.rb +++ b/actionpack/test/abstract/collector_test.rb @@ -53,9 +53,9 @@ module AbstractController collector.html collector.text(:foo) collector.js(:bar) { :baz } - assert_equal [Mime::Type[:HTML], [], nil], collector.responses[0] - assert_equal [Mime::Type[:TEXT], [:foo], nil], collector.responses[1] - assert_equal [Mime::Type[:JS], [:bar]], collector.responses[2][0,2] + assert_equal [Mime[:html], [], nil], collector.responses[0] + assert_equal [Mime[:text], [:foo], nil], collector.responses[1] + assert_equal [Mime[:js], [:bar]], collector.responses[2][0,2] assert_equal :baz, collector.responses[2][2].call end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 7dfeadceb0..899d92f815 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -65,7 +65,7 @@ class ActionPackAssertionsController < ActionController::Base end def render_text_with_custom_content_type - render body: "Hello!", content_type: Mime::Type[:RSS] + render body: "Hello!", content_type: Mime[:rss] end def session_stuffing diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index 4a86f1bad3..c02607b55e 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -3,7 +3,7 @@ require 'abstract_unit' class OldContentTypeController < ActionController::Base # :ported: def render_content_type_from_body - response.content_type = Mime::Type[:RSS] + response.content_type = Mime[:rss] render body: "hello world!" end @@ -14,7 +14,7 @@ class OldContentTypeController < ActionController::Base # :ported: def render_content_type_from_render - render body: "hello world!", :content_type => Mime::Type[:RSS] + render body: "hello world!", :content_type => Mime[:rss] end # :ported: @@ -36,7 +36,7 @@ class OldContentTypeController < ActionController::Base end def render_change_for_builder - response.content_type = Mime::Type[:HTML] + response.content_type = Mime[:html] render :action => "render_default_for_builder" end @@ -45,7 +45,7 @@ class OldContentTypeController < ActionController::Base 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::Type[:XML] } + format.rss { render body: "hello world!", content_type: Mime[:xml] } end end end @@ -64,68 +64,68 @@ class ContentTypeTest < ActionController::TestCase def test_render_defaults get :render_defaults assert_equal "utf-8", @response.charset - assert_equal Mime::Type[:TEXT], @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::Type[:TEXT], @response.content_type + assert_equal Mime[:text], @response.content_type end end # :ported: def test_content_type_from_body get :render_content_type_from_body - assert_equal Mime::Type[:RSS], @response.content_type + assert_equal Mime[:rss], @response.content_type assert_equal "utf-8", @response.charset end # :ported: def test_content_type_from_render get :render_content_type_from_render - assert_equal Mime::Type[:RSS], @response.content_type + assert_equal Mime[:rss], @response.content_type assert_equal "utf-8", @response.charset end # :ported: def test_charset_from_body get :render_charset_from_body - assert_equal Mime::Type[:TEXT], @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::Type[:TEXT], @response.content_type + assert_equal Mime[:text], @response.content_type assert_equal "utf-8", @response.charset, @response.headers.inspect end def test_nil_default_for_erb with_default_charset nil do get :render_default_for_erb - assert_equal Mime::Type[:HTML], @response.content_type + assert_equal Mime[:html], @response.content_type assert_nil @response.charset, @response.headers.inspect end end def test_default_for_erb get :render_default_for_erb - assert_equal Mime::Type[:HTML], @response.content_type + assert_equal Mime[:html], @response.content_type assert_equal "utf-8", @response.charset end def test_default_for_builder get :render_default_for_builder - assert_equal Mime::Type[:XML], @response.content_type + assert_equal Mime[:xml], @response.content_type assert_equal "utf-8", @response.charset end def test_change_for_builder get :render_change_for_builder - assert_equal Mime::Type[:HTML], @response.content_type + assert_equal Mime[:html], @response.content_type assert_equal "utf-8", @response.charset end @@ -144,24 +144,24 @@ class AcceptBasedContentTypeTest < ActionController::TestCase tests OldContentTypeController def test_render_default_content_types_for_respond_to - @request.accept = Mime::Type[:HTML].to_s + @request.accept = Mime[:html].to_s get :render_default_content_types_for_respond_to - assert_equal Mime::Type[:HTML], @response.content_type + assert_equal Mime[:html], @response.content_type - @request.accept = Mime::Type[:JS].to_s + @request.accept = Mime[:js].to_s get :render_default_content_types_for_respond_to - assert_equal Mime::Type[:JS], @response.content_type + assert_equal Mime[:js], @response.content_type end def test_render_default_content_types_for_respond_to_with_template - @request.accept = Mime::Type[:XML].to_s + @request.accept = Mime[:xml].to_s get :render_default_content_types_for_respond_to - assert_equal Mime::Type[:XML], @response.content_type + assert_equal Mime[:xml], @response.content_type end def test_render_default_content_types_for_respond_to_with_overwrite - @request.accept = Mime::Type[:RSS].to_s + @request.accept = Mime[:rss].to_s get :render_default_content_types_for_respond_to - assert_equal Mime::Type[:XML], @response.content_type + assert_equal Mime[:xml], @response.content_type end end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 64eb33f78f..c025c7fa00 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -81,7 +81,7 @@ class RespondToController < ActionController::Base def using_defaults_with_all respond_to do |type| type.html - type.all{ render body: "ALL" } + type.all { render body: "ALL" } 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 d9899fe01f..a9dcdde4b8 100644 --- a/actionpack/test/controller/new_base/content_type_test.rb +++ b/actionpack/test/controller/new_base/content_type_test.rb @@ -7,12 +7,12 @@ module ContentType end def set_on_response_obj - response.content_type = Mime::Type[:RSS] + response.content_type = Mime[:rss] render body: "Hello world!" end def set_on_render - render body: "Hello world!", content_type: Mime::Type[:RSS] + render body: "Hello world!", content_type: Mime[:rss] end end diff --git a/actionpack/test/controller/render_other_test.rb b/actionpack/test/controller/render_other_test.rb index 8891f6177f..1f5215ac55 100644 --- a/actionpack/test/controller/render_other_test.rb +++ b/actionpack/test/controller/render_other_test.rb @@ -12,7 +12,7 @@ class RenderOtherTest < ActionController::TestCase def test_using_custom_render_option ActionController.add_renderer :simon do |says, options| - self.content_type = Mime::Type[:TEXT] + self.content_type = Mime[:text] self.response_body = "Simon says: #{says}" end diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 094d3ea1d2..f0fd7ddc5e 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -92,6 +92,6 @@ class RenderXmlTest < ActionController::TestCase def test_should_use_implicit_content_type get :implicit_content_type, format: 'atom' - assert_equal Mime::Type[:ATOM], @response.content_type + assert_equal Mime[:atom], @response.content_type end end diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index c712c75c88..2820425c31 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -22,7 +22,7 @@ class SendFileController < ActionController::Base def test_send_file_headers_bang options = { - :type => Mime::Type[:PNG], + :type => Mime[:png], :disposition => 'disposition', :filename => 'filename' } @@ -32,7 +32,7 @@ class SendFileController < ActionController::Base def test_send_file_headers_with_disposition_as_a_symbol options = { - :type => Mime::Type[:PNG], + :type => Mime[:png], :disposition => :disposition, :filename => 'filename' } @@ -195,7 +195,7 @@ class SendFileTest < ActionController::TestCase %w(file data).each do |method| define_method "test_send_#{method}_status" do @controller.options = { :stream => false, :status => 500 } - assert_nothing_raised { assert_not_nil process(method) } + assert_not_nil process(method) assert_equal 500, @response.status end diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 2aee914a24..6d377c4691 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -65,7 +65,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_register_and_use_json_simple with_test_route_set do - with_params_parsers Mime::Type[:JSON] => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do + with_params_parsers Mime[:json] => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do post "/", params: '{"request":{"summary":"content...","title":"JSON"}}', headers: { 'CONTENT_TYPE' => 'application/json' } @@ -99,7 +99,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_parsing_json_doesnot_rescue_exception req = Class.new(ActionDispatch::Request) do def params_parsers - { Mime::Type[:JSON] => Proc.new { |data| raise Interrupt } } + { Mime[:json] => Proc.new { |data| raise Interrupt } } end def content_length; get_header('rack.input').length; end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index e9b2fe3214..84c244c72a 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -3,6 +3,75 @@ require 'openssl' require 'active_support/key_generator' require 'active_support/message_verifier' +class CookieJarTest < ActiveSupport::TestCase + attr_reader :request + + def setup + @request = ActionDispatch::Request.new({}) + end + + def test_fetch + x = Object.new + assert_not request.cookie_jar.key?('zzzzzz') + assert_equal x, request.cookie_jar.fetch('zzzzzz', x) + assert_not request.cookie_jar.key?('zzzzzz') + end + + def test_fetch_exists + x = Object.new + request.cookie_jar['foo'] = 'bar' + assert_equal 'bar', request.cookie_jar.fetch('foo', x) + end + + def test_fetch_block + x = Object.new + assert_not request.cookie_jar.key?('zzzzzz') + assert_equal x, request.cookie_jar.fetch('zzzzzz') { x } + end + + def test_key_is_to_s + request.cookie_jar['foo'] = 'bar' + assert_equal 'bar', request.cookie_jar.fetch(:foo) + end + + def test_fetch_type_error + assert_raises(KeyError) do + request.cookie_jar.fetch(:omglolwut) + end + end + + def test_each + request.cookie_jar['foo'] = :bar + list = [] + request.cookie_jar.each do |k,v| + list << [k, v] + end + + assert_equal [['foo', :bar]], list + end + + def test_enumerable + request.cookie_jar['foo'] = :bar + actual = request.cookie_jar.map { |k,v| [k.to_s, v.to_s] } + assert_equal [['foo', 'bar']], actual + end + + def test_key_methods + assert !request.cookie_jar.key?(:foo) + assert !request.cookie_jar.has_key?("foo") + + request.cookie_jar[:foo] = :bar + assert request.cookie_jar.key?(:foo) + assert request.cookie_jar.has_key?("foo") + end + + def test_write_doesnt_set_a_nil_header + headers = {} + request.cookie_jar.write(headers) + assert !headers.include?('Set-Cookie') + end +end + class CookiesTest < ActionController::TestCase class CustomSerializer def self.load(value) @@ -14,16 +83,6 @@ class CookiesTest < ActionController::TestCase end end - class JSONWrapper - def initialize(obj) - @obj = obj - end - - def as_json(options = nil) - "wrapped: #{@obj.as_json(options)}" - end - end - class TestController < ActionController::Base def authenticate cookies["user_name"] = "david" @@ -88,11 +147,6 @@ class CookiesTest < ActionController::TestCase head :ok end - def set_wrapped_signed_cookie - cookies.signed[:user_id] = JSONWrapper.new(45) - head :ok - end - def get_signed_cookie cookies.signed[:user_id] head :ok @@ -103,6 +157,21 @@ class CookiesTest < ActionController::TestCase head :ok end + class JSONWrapper + def initialize(obj) + @obj = obj + end + + def as_json(options = nil) + "wrapped: #{@obj.as_json(options)}" + end + end + + def set_wrapped_signed_cookie + cookies.signed[:user_id] = JSONWrapper.new(45) + head :ok + end + def set_wrapped_encrypted_cookie cookies.encrypted[:foo] = JSONWrapper.new('bar') head :ok @@ -207,68 +276,18 @@ class CookiesTest < ActionController::TestCase tests TestController + SALT = 'b3c631c314c0bbca50c1b2843150fe33' + def setup super - @request.env["action_dispatch.key_generator"] = ActiveSupport::KeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33", iterations: 2) - @request.env["action_dispatch.signed_cookie_salt"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.encrypted_cookie_salt"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.encrypted_signed_cookie_salt"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.host = "www.nextangle.com" - end - - def test_fetch - x = Object.new - assert_not request.cookie_jar.key?('zzzzzz') - assert_equal x, request.cookie_jar.fetch('zzzzzz', x) - assert_not request.cookie_jar.key?('zzzzzz') - end - - def test_fetch_exists - x = Object.new - request.cookie_jar['foo'] = 'bar' - assert_equal 'bar', request.cookie_jar.fetch('foo', x) - end - - def test_fetch_block - x = Object.new - assert_not request.cookie_jar.key?('zzzzzz') - assert_equal x, request.cookie_jar.fetch('zzzzzz') { x } - end - - def test_key_is_to_s - request.cookie_jar['foo'] = 'bar' - assert_equal 'bar', request.cookie_jar.fetch(:foo) - end - - def test_fetch_type_error - assert_raises(KeyError) do - request.cookie_jar.fetch(:omglolwut) - end - end - - def test_each - request.cookie_jar['foo'] = :bar - list = [] - request.cookie_jar.each do |k,v| - list << [k, v] - end - assert_equal [['foo', :bar]], list - end + @request.env["action_dispatch.key_generator"] = ActiveSupport::KeyGenerator.new(SALT, iterations: 2) - def test_enumerable - request.cookie_jar['foo'] = :bar - actual = request.cookie_jar.map { |k,v| [k.to_s, v.to_s] } - assert_equal [['foo', 'bar']], actual - end + @request.env["action_dispatch.signed_cookie_salt"] = + @request.env["action_dispatch.encrypted_cookie_salt"] = + @request.env["action_dispatch.encrypted_signed_cookie_salt"] = SALT - def test_key_methods - assert !request.cookie_jar.key?(:foo) - assert !request.cookie_jar.has_key?("foo") - - request.cookie_jar[:foo] = :bar - assert request.cookie_jar.key?(:foo) - assert request.cookie_jar.has_key?("foo") + @request.host = "www.nextangle.com" end def test_setting_cookie @@ -1083,11 +1102,11 @@ class CookiesTest < ActionController::TestCase assert_equal "david", cookies[:user_name] get :noop - assert_nil @response.headers["Set-Cookie"] + assert !@response.headers.include?("Set-Cookie") assert_equal "david", cookies[:user_name] get :noop - assert_nil @response.headers["Set-Cookie"] + assert !@response.headers.include?("Set-Cookie") assert_equal "david", cookies[:user_name] end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 79600b654b..7f1ef121b7 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -42,6 +42,24 @@ class HeaderTest < ActiveSupport::TestCase assert_equal "127.0.0.1", @headers["HTTP_HOST"] end + test "add to multivalued headers" do + # Sets header when not present + @headers.add 'Foo', '1' + assert_equal '1', @headers['Foo'] + + # Ignores nil values + @headers.add 'Foo', nil + assert_equal '1', @headers['Foo'] + + # Converts value to string + @headers.add 'Foo', 1 + assert_equal '1,1', @headers['Foo'] + + # Case-insensitive + @headers.add 'fOo', 2 + assert_equal '1,1,2', @headers['foO'] + end + test "headers can contain numbers" do @headers["Content-MD5"] = "Q2hlY2sgSW50ZWdyaXR5IQ==" diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index 1c128365c7..55becc1c91 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -83,6 +83,8 @@ module ActionController def test_headers_cannot_be_written_after_close @response.stream.close + # we can add data until it's actually written, which happens on `each` + @response.each { |x| } e = assert_raises(ActionDispatch::IllegalStateError) do @response.headers['Content-Length'] = "zomg" diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 91f6f66fe3..149e37bf3d 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -1,7 +1,6 @@ require 'abstract_unit' class MimeTypeTest < ActiveSupport::TestCase - test "parse single" do Mime::LOOKUP.each_key do |mime_type| unless mime_type == 'image/*' @@ -11,95 +10,95 @@ class MimeTypeTest < ActiveSupport::TestCase end test "unregister" do + assert_nil Mime[:mobile] + begin - Mime::Type.register("text/x-mobile", :mobile) - assert Mime::Type.registered?(:MOBILE) - assert_equal Mime::Type[:MOBILE], Mime::LOOKUP['text/x-mobile'] - assert_equal Mime::Type[:MOBILE], Mime::EXTENSION_LOOKUP['mobile'] + mime = Mime::Type.register("text/x-mobile", :mobile) + assert_equal mime, Mime[:mobile] + assert_equal mime, Mime::Type.lookup('text/x-mobile') + assert_equal mime, Mime::Type.lookup_by_extension(:mobile) Mime::Type.unregister(:mobile) - assert !Mime.const_defined?(:MOBILE), "Mime::MOBILE should not be defined" - assert !Mime::LOOKUP.has_key?('text/x-mobile'), "Mime::LOOKUP should not have key ['text/x-mobile]" - assert !Mime::EXTENSION_LOOKUP.has_key?('mobile'), "Mime::EXTENSION_LOOKUP should not have key ['mobile]" + assert_nil Mime[:mobile], "Mime[:mobile] should be nil after unregistering :mobile" + assert_nil Mime::Type.lookup_by_extension(:mobile), "Should be missing MIME extension lookup for :mobile" ensure - Mime::LOOKUP.reject!{|key,_| key == 'text/x-mobile'} + Mime::Type.unregister :mobile end end test "parse text with trailing star at the beginning" do accept = "text/*, text/html, application/json, multipart/form-data" - expect = [Mime::Type[:HTML], Mime::Type[:TEXT], Mime::Type[:JS], Mime::Type[:CSS], Mime::Type[:ICS], Mime::Type[:CSV], Mime::Type[:VCF], Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:JSON], Mime::Type[:MULTIPART_FORM]] + expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml], Mime[:json], Mime[:multipart_form]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed end test "parse text with trailing star in the end" do accept = "text/html, application/json, multipart/form-data, text/*" - expect = [Mime::Type[:HTML], Mime::Type[:JSON], Mime::Type[:MULTIPART_FORM], Mime::Type[:TEXT], Mime::Type[:JS], Mime::Type[:CSS], Mime::Type[:ICS], Mime::Type[:CSV], Mime::Type[:VCF], Mime::Type[:XML], Mime::Type[:YAML]] + expect = [Mime[:html], Mime[:json], Mime[:multipart_form], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed end test "parse text with trailing star" do accept = "text/*" - expect = [Mime::Type[:HTML], Mime::Type[:TEXT], Mime::Type[:JS], Mime::Type[:CSS], Mime::Type[:ICS], Mime::Type[:CSV], Mime::Type[:VCF], Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:JSON]] + expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml], Mime[:json]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed end test "parse application with trailing star" do accept = "application/*" - expect = [Mime::Type[:HTML], Mime::Type[:JS], Mime::Type[:XML], Mime::Type[:RSS], Mime::Type[:ATOM], Mime::Type[:YAML], Mime::Type[:URL_ENCODED_FORM], Mime::Type[:JSON], Mime::Type[:PDF], Mime::Type[:ZIP]] + expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed end test "parse without q" do accept = "text/xml,application/xhtml+xml,text/yaml,application/xml,text/html,image/png,text/plain,application/pdf,*/*" - expect = [Mime::Type[:HTML], Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:PNG], Mime::Type[:TEXT], Mime::Type[:PDF], Mime::Type[:ALL]] - assert_equal expect, Mime::Type.parse(accept) + expect = [Mime[:html], Mime[:xml], Mime[:yaml], Mime[:png], Mime[:text], Mime[:pdf], '*/*'] + assert_equal expect.map(&:to_s), Mime::Type.parse(accept).map(&:to_s) end test "parse with q" do accept = "text/xml,application/xhtml+xml,text/yaml; q=0.3,application/xml,text/html; q=0.8,image/png,text/plain; q=0.5,application/pdf,*/*; q=0.2" - expect = [Mime::Type[:HTML], Mime::Type[:XML], Mime::Type[:PNG], Mime::Type[:PDF], Mime::Type[:TEXT], Mime::Type[:YAML], Mime::Type[:ALL]] - assert_equal expect, Mime::Type.parse(accept) + expect = [Mime[:html], Mime[:xml], Mime[:png], Mime[:pdf], Mime[:text], Mime[:yaml], '*/*'] + assert_equal expect.map(&:to_s), Mime::Type.parse(accept).map(&:to_s) end test "parse single media range with q" do accept = "text/html;q=0.9" - expect = [Mime::Type[:HTML]] + expect = [Mime[:html]] assert_equal expect, Mime::Type.parse(accept) end test "parse arbitrary media type parameters" do accept = 'multipart/form-data; boundary="simple boundary"' - expect = [Mime::Type[:MULTIPART_FORM]] + expect = [Mime[:multipart_form]] assert_equal expect, Mime::Type.parse(accept) end # Accept header send with user HTTP_USER_AGENT: Sunrise/0.42j (Windows XP) test "parse broken acceptlines" do accept = "text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/*,,*/*;q=0.5" - expect = [Mime::Type[:HTML], Mime::Type[:XML], "image/*", Mime::Type[:TEXT], Mime::Type[:ALL]] - assert_equal expect, Mime::Type.parse(accept).collect(&:to_s) + expect = [Mime[:html], Mime[:xml], "image/*", Mime[:text], '*/*'] + assert_equal expect.map(&:to_s), Mime::Type.parse(accept).map(&:to_s) end # Accept header send with user HTTP_USER_AGENT: Mozilla/4.0 # (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; InfoPath.1) test "parse other broken acceptlines" do accept = "image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, application/x-shockwave-flash, application/vnd.ms-excel, application/vnd.ms-powerpoint, application/msword, , pronto/1.00.00, sslvpn/1.00.00.00, */*" - expect = ['image/gif', 'image/x-xbitmap', 'image/jpeg','image/pjpeg', 'application/x-shockwave-flash', 'application/vnd.ms-excel', 'application/vnd.ms-powerpoint', 'application/msword', 'pronto/1.00.00', 'sslvpn/1.00.00.00', Mime::Type[:ALL]] - assert_equal expect, Mime::Type.parse(accept).collect(&:to_s) + expect = ['image/gif', 'image/x-xbitmap', 'image/jpeg','image/pjpeg', 'application/x-shockwave-flash', 'application/vnd.ms-excel', 'application/vnd.ms-powerpoint', 'application/msword', 'pronto/1.00.00', 'sslvpn/1.00.00.00', '*/*'] + assert_equal expect.map(&:to_s), Mime::Type.parse(accept).map(&:to_s) end test "custom type" do begin type = Mime::Type.register("image/foo", :foo) - assert_equal Mime::Type[:FOO], type - assert Mime::Type.registered?(:FOO) + assert_equal type, Mime[:foo] ensure - Mime::Type.unregister(:FOO) + Mime::Type.unregister(:foo) end end @@ -107,10 +106,10 @@ class MimeTypeTest < ActiveSupport::TestCase begin Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"] %w[text/foobar text/foo text/bar].each do |type| - assert_equal Mime::Type[:FOOBAR], type + assert_equal Mime[:foobar], type end ensure - Mime::Type.unregister(:FOOBAR) + Mime::Type.unregister(:foobar) end end @@ -121,10 +120,10 @@ class MimeTypeTest < ActiveSupport::TestCase registered_mimes << mime end - Mime::Type.register("text/foo", :foo) - assert_equal [Mime::Type[:FOO]], registered_mimes + mime = Mime::Type.register("text/foo", :foo) + assert_equal [mime], registered_mimes ensure - Mime::Type.unregister(:FOO) + Mime::Type.unregister(:foo) end end @@ -132,36 +131,32 @@ class MimeTypeTest < ActiveSupport::TestCase begin Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"] %w[foobar foo bar].each do |extension| - assert_equal Mime::Type[:FOOBAR], Mime::EXTENSION_LOOKUP[extension] + assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension] end ensure - Mime::Type.unregister(:FOOBAR) + Mime::Type.unregister(:foobar) end end test "register alias" do begin Mime::Type.register_alias "application/xhtml+xml", :foobar - assert_equal Mime::Type[:HTML], Mime::EXTENSION_LOOKUP['foobar'] + assert_equal Mime[:html], Mime::EXTENSION_LOOKUP['foobar'] ensure - Mime::Type.unregister(:FOOBAR) + Mime::Type.unregister(:foobar) end end test "type should be equal to symbol" do - assert_equal Mime::Type[:HTML], 'application/xhtml+xml' - assert_equal Mime::Type[:HTML], :html + assert_equal Mime[:html], 'application/xhtml+xml' + assert_equal Mime[:html], :html end test "type convenience methods" do - # Don't test Mime::Type[:ALL], since it Mime::Type[:ALL].html? == true - types = Mime::SET.symbols.uniq - [:all, :iphone] - - # Remove custom Mime::Type instances set in other tests, like Mime::Type[:GIF] and Mime::Type[:IPHONE] - types.delete_if { |type| !Mime::Type.registered?(type.upcase) } + types = Mime::SET.symbols.uniq - [:iphone] types.each do |type| - mime = Mime::Type[type.upcase] + mime = Mime[type] assert mime.respond_to?("#{type}?"), "#{mime.inspect} does not respond to #{type}?" assert_equal type, mime.symbol, "#{mime.inspect} is not #{type}?" invalid_types = types - [type] @@ -172,37 +167,31 @@ class MimeTypeTest < ActiveSupport::TestCase end end - test "mime all is html" do - assert Mime::Type[:ALL].all?, "Mime::ALL is not all?" - assert Mime::Type[:ALL].html?, "Mime::ALL is not html?" - end - test "deprecated lookup" do assert_deprecated do - assert Mime::ALL.all?, "Mime::ALL is not all?" + Mime::HTML end end - test "verifiable mime types" do - all_types = Mime::SET.symbols - all_types.uniq! - # Remove custom Mime::Type instances set in other tests, like Mime::Type[:GIF] and Mime::Type[:IPHONE] - all_types.delete_if { |type| !Mime::Type.registered?(type.upcase) } + test "deprecated const_defined?" do + assert_deprecated do + Mime.const_defined? :HTML + end end test "references gives preference to symbols before strings" do - assert_equal :html, Mime::Type[:HTML].ref + assert_equal :html, Mime[:html].ref another = Mime::Type.lookup("foo/bar") assert_nil another.to_sym assert_equal "foo/bar", another.ref end test "regexp matcher" do - assert Mime::Type[:JS] =~ "text/javascript" - assert Mime::Type[:JS] =~ "application/javascript" - assert Mime::Type[:JS] !~ "text/html" - assert !(Mime::Type[:JS] !~ "text/javascript") - assert !(Mime::Type[:JS] !~ "application/javascript") - assert Mime::Type[:HTML] =~ 'application/xhtml+xml' + assert Mime[:js] =~ "text/javascript" + assert Mime[:js] =~ "application/javascript" + assert Mime[:js] !~ "text/html" + assert !(Mime[:js] !~ "text/javascript") + assert !(Mime[:js] !~ "application/javascript") + assert Mime[:html] =~ 'application/xhtml+xml' end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 40866595ed..af2ed24f43 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -750,33 +750,33 @@ class RequestFormat < BaseRequestTest test "xml format" do request = stub_request assert_called(request, :parameters, times: 2, returns: {format: :xml}) do - assert_equal Mime::Type[:XML], request.format + assert_equal Mime[:xml], request.format end end test "xhtml format" do request = stub_request assert_called(request, :parameters, times: 2, returns: {format: :xhtml}) do - assert_equal Mime::Type[:HTML], request.format + assert_equal Mime[:html], request.format end end test "txt format" do request = stub_request assert_called(request, :parameters, times: 2, returns: {format: :txt}) do - assert_equal Mime::Type[:TEXT], request.format + assert_equal Mime[:text], request.format end end test "XMLHttpRequest" do request = stub_request( 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest', - 'HTTP_ACCEPT' => [Mime::Type[:JS], Mime::Type[:HTML], Mime::Type[:XML], "text/xml", Mime::Type[:ALL]].join(",") + 'HTTP_ACCEPT' => [Mime[:js], Mime[:html], Mime[:xml], "text/xml", "*/*"].join(",") ) assert_called(request, :parameters, times: 1, returns: {}) do assert request.xhr? - assert_equal Mime::Type[:JS], request.format + assert_equal Mime[:js], request.format end end @@ -796,29 +796,29 @@ class RequestFormat < BaseRequestTest test "formats text/html with accept header" do request = stub_request 'HTTP_ACCEPT' => 'text/html' - assert_equal [Mime::Type[:HTML]], request.formats + assert_equal [Mime[:html]], request.formats end test "formats blank with accept header" do request = stub_request 'HTTP_ACCEPT' => '' - assert_equal [Mime::Type[:HTML]], request.formats + assert_equal [Mime[:html]], request.formats end test "formats XMLHttpRequest with accept header" do request = stub_request 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" - assert_equal [Mime::Type[:JS]], request.formats + assert_equal [Mime[:js]], request.formats end test "formats application/xml with accept header" do request = stub_request('CONTENT_TYPE' => 'application/xml; charset=UTF-8', 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest") - assert_equal [Mime::Type[:XML]], request.formats + assert_equal [Mime[:xml]], request.formats end test "formats format:text with accept header" do request = stub_request assert_called(request, :parameters, times: 2, returns: {format: :txt}) do - assert_equal [Mime::Type[:TEXT]], request.formats + assert_equal [Mime[:text]], request.formats end end @@ -848,7 +848,7 @@ class RequestFormat < BaseRequestTest test "formats with xhr request" do request = stub_request 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [Mime::Type[:JS]], request.formats + assert_equal [Mime[:js]], request.formats end end @@ -859,35 +859,35 @@ class RequestFormat < BaseRequestTest begin request = stub_request 'HTTP_ACCEPT' => 'application/xml' assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime::Type[:HTML] ], request.formats + assert_equal [ Mime[:html] ], request.formats end request = stub_request 'HTTP_ACCEPT' => 'koz-asked/something-crazy' assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime::Type[:HTML] ], request.formats + assert_equal [ Mime[:html] ], request.formats end request = stub_request 'HTTP_ACCEPT' => '*/*;q=0.1' assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime::Type[:HTML] ], request.formats + assert_equal [ Mime[:html] ], request.formats end request = stub_request 'HTTP_ACCEPT' => 'application/jxw' assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime::Type[:HTML] ], request.formats + assert_equal [ Mime[:html] ], request.formats end request = stub_request 'HTTP_ACCEPT' => 'application/xml', 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime::Type[:JS] ], request.formats + assert_equal [ Mime[:js] ], request.formats end request = stub_request 'HTTP_ACCEPT' => 'application/xml', 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" assert_called(request, :parameters, times: 2, returns: {format: :json}) do - assert_equal [ Mime::Type[:JSON] ], request.formats + assert_equal [ Mime[:json] ], request.formats end ensure ActionDispatch::Request.ignore_accept_header = old_ignore_accept_header @@ -897,7 +897,7 @@ end class RequestMimeType < BaseRequestTest test "content type" do - assert_equal Mime::Type[:HTML], stub_request('CONTENT_TYPE' => 'text/html').content_mime_type + assert_equal Mime[:html], stub_request('CONTENT_TYPE' => 'text/html').content_mime_type end test "no content type" do @@ -905,11 +905,11 @@ class RequestMimeType < BaseRequestTest end test "content type is XML" do - assert_equal Mime::Type[:XML], stub_request('CONTENT_TYPE' => 'application/xml').content_mime_type + assert_equal Mime[:xml], stub_request('CONTENT_TYPE' => 'application/xml').content_mime_type end test "content type with charset" do - assert_equal Mime::Type[:XML], stub_request('CONTENT_TYPE' => 'application/xml; charset=UTF-8').content_mime_type + assert_equal Mime[:xml], stub_request('CONTENT_TYPE' => 'application/xml; charset=UTF-8').content_mime_type end test "user agent" do @@ -922,9 +922,9 @@ class RequestMimeType < BaseRequestTest 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" ) - assert_equal nil, request.negotiate_mime([Mime::Type[:XML], Mime::Type[:JSON]]) - assert_equal Mime::Type[:HTML], request.negotiate_mime([Mime::Type[:XML], Mime::Type[:HTML]]) - assert_equal Mime::Type[:HTML], request.negotiate_mime([Mime::Type[:XML], Mime::Type[:ALL]]) + assert_equal nil, request.negotiate_mime([Mime[:xml], Mime[:json]]) + assert_equal Mime[:html], request.negotiate_mime([Mime[:xml], Mime[:html]]) + assert_equal Mime[:html], request.negotiate_mime([Mime[:xml], Mime::ALL]) end test "negotiate_mime with content_type" do @@ -933,7 +933,7 @@ class RequestMimeType < BaseRequestTest 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" ) - assert_equal Mime::Type[:XML], request.negotiate_mime([Mime::Type[:XML], Mime::Type[:CSV]]) + assert_equal Mime[:xml], request.negotiate_mime([Mime[:xml], Mime[:csv]]) end end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 85cdcda655..126379a23c 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -146,7 +146,7 @@ class ResponseTest < ActiveSupport::TestCase test "cookies" do @response.set_cookie("user_name", :value => "david", :path => "/") - status, headers, body = @response.to_a + _status, headers, _body = @response.to_a assert_equal "user_name=david; path=/", headers["Set-Cookie"] assert_equal({"user_name" => "david"}, @response.cookies) end @@ -154,7 +154,7 @@ class ResponseTest < ActiveSupport::TestCase test "multiple cookies" do @response.set_cookie("user_name", :value => "david", :path => "/") @response.set_cookie("login", :value => "foo&bar", :path => "/", :expires => Time.utc(2005, 10, 10,5)) - status, headers, body = @response.to_a + _status, headers, _body = @response.to_a assert_equal "user_name=david; path=/\nlogin=foo%26bar; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000", headers["Set-Cookie"] assert_equal({"login" => "foo&bar", "user_name" => "david"}, @response.cookies) end @@ -163,7 +163,6 @@ class ResponseTest < ActiveSupport::TestCase @response.set_cookie("user_name", :value => "david", :path => "/") @response.set_cookie("login", :value => "foo&bar", :path => "/", :expires => Time.utc(2005, 10, 10,5)) @response.delete_cookie("login") - status, headers, body = @response.to_a assert_equal({"user_name" => "david", "login" => nil}, @response.cookies) end @@ -185,18 +184,28 @@ class ResponseTest < ActiveSupport::TestCase test "read charset and content type" do resp = ActionDispatch::Response.new.tap { |response| response.charset = 'utf-16' - response.content_type = Mime::Type[:XML] + response.content_type = Mime[:xml] response.body = 'Hello' } resp.to_a assert_equal('utf-16', resp.charset) - assert_equal(Mime::Type[:XML], resp.content_type) + assert_equal(Mime[:xml], resp.content_type) assert_equal('application/xml; charset=utf-16', resp.headers['Content-Type']) end - test "read content type without charset" do + test "read content type with default charset utf-8" do + original = ActionDispatch::Response.default_charset + begin + resp = ActionDispatch::Response.new(200, { "Content-Type" => "text/xml" }) + assert_equal('utf-8', resp.charset) + ensure + ActionDispatch::Response.default_charset = original + end + end + + test "read content type with charset utf-16" do jruby_skip "https://github.com/jruby/jruby/issues/3138" original = ActionDispatch::Response.default_charset @@ -283,6 +292,65 @@ class ResponseTest < ActiveSupport::TestCase end end +class ResponseHeadersTest < ActiveSupport::TestCase + def setup + @response = ActionDispatch::Response.create + @response.set_header 'Foo', '1' + end + + test 'has_header?' do + assert @response.has_header? 'Foo' + assert_not @response.has_header? 'foo' + assert_not @response.has_header? nil + end + + test 'get_header' do + assert_equal '1', @response.get_header('Foo') + assert_nil @response.get_header('foo') + assert_nil @response.get_header(nil) + end + + test 'set_header' do + assert_equal '2', @response.set_header('Foo', '2') + assert @response.has_header?('Foo') + assert_equal '2', @response.get_header('Foo') + + assert_nil @response.set_header('Foo', nil) + assert @response.has_header?('Foo') + assert_nil @response.get_header('Foo') + end + + test 'delete_header' do + assert_nil @response.delete_header(nil) + + assert_nil @response.delete_header('foo') + assert @response.has_header?('Foo') + + assert_equal '1', @response.delete_header('Foo') + assert_not @response.has_header?('Foo') + end + + test 'add_header' do + # Add a value to an existing header + assert_equal '1,2', @response.add_header('Foo', '2') + assert_equal '1,2', @response.get_header('Foo') + + # Add nil to an existing header + assert_equal '1,2', @response.add_header('Foo', nil) + assert_equal '1,2', @response.get_header('Foo') + + # Add nil to a nonexistent header + assert_nil @response.add_header('Bar', nil) + assert_not @response.has_header?('Bar') + assert_nil @response.get_header('Bar') + + # Add a value to a nonexistent header + assert_equal '1', @response.add_header('Bar', '1') + assert @response.has_header?('Bar') + assert_equal '1', @response.get_header('Bar') + end +end + class ResponseIntegrationTest < ActionDispatch::IntegrationTest test "response cache control from railsish app" do @app = lambda { |env| @@ -324,7 +392,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest @app = lambda { |env| ActionDispatch::Response.new.tap { |resp| resp.charset = 'utf-16' - resp.content_type = Mime::Type[:XML] + resp.content_type = Mime[:xml] resp.body = 'Hello' }.to_a } @@ -333,7 +401,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_response :success assert_equal('utf-16', @response.charset) - assert_equal(Mime::Type[:XML], @response.content_type) + assert_equal(Mime[:xml], @response.content_type) assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end @@ -349,7 +417,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_response :success assert_equal('utf-16', @response.charset) - assert_equal(Mime::Type[:XML], @response.content_type) + assert_equal(Mime[:xml], @response.content_type) assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 6a3af32946..15d51e5d6c 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -497,17 +497,6 @@ module ActionDispatch private - def add_routes router, paths, anchor = true - paths.each do |path| - if String === path - path = Path::Pattern.from_string path - else - path - end - add_route @app, path, {}, [], {}, {} - end - end - def rails_env env, klass = ActionDispatch::Request klass.new(rack_env(env)) end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 82a88952c3..b27c0a29c6 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -189,7 +189,7 @@ *Nikolay Shebanov* -* Add a `hidden_field` on the `file_field` to avoid raise a error when the only +* Add a `hidden_field` on the `file_field` to avoid raising an error when the only input on the form is the `file_field`. *Mauro George* diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 72ca6f7ec6..ad1cb1a4be 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -75,7 +75,7 @@ module ActionView #:nodoc: # # Headline: <%= local_assigns[:headline] %> # - # This is useful in cases where you aren't sure if the local variable has been assigned. Alternately, you could also use + # This is useful in cases where you aren't sure if the local variable has been assigned. Alternatively, you could also use # <tt>defined? headline</tt> to first check if the variable has been assigned before using it. # # === Template caching diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index e506c782d6..fa46a22500 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -136,7 +136,7 @@ module ActionView tag( "link", "rel" => tag_options[:rel] || "alternate", - "type" => tag_options[:type] || Mime::Type.lookup_by_extension(type.to_s).to_s, + "type" => tag_options[:type] || Mime[type].to_s, "title" => tag_options[:title] || type.to_s.upcase, "href" => url_options.is_a?(Hash) ? url_for(url_options.merge(:only_path => false)) : url_options ) diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb index 610f5c0b4a..2a367b85af 100644 --- a/actionview/lib/action_view/helpers/form_helper.rb +++ b/actionview/lib/action_view/helpers/form_helper.rb @@ -1038,7 +1038,7 @@ module ActionView # date_field("user", "born_on") # # => <input id="user_born_on" name="user[born_on]" type="date" /> # - # The default value is generated by trying to call "to_date" + # The default value is generated by trying to call +strftime+ with "%Y-%m-%d" # on the object's value, which makes it behave as expected for instances # of DateTime and ActiveSupport::TimeWithZone. You can still override that # by passing the "value" option explicitly, e.g. diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index f6dd179bb7..430051379d 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -548,7 +548,7 @@ module ActionView end # Returns a string of option tags for pretty much any time zone in the - # world. Supply a ActiveSupport::TimeZone name as +selected+ to have it + # world. Supply an ActiveSupport::TimeZone name as +selected+ to have it # marked as the selected option tag. You can also supply an array of # ActiveSupport::TimeZone objects as +priority_zones+, so that they will # be listed above the rest of the (long) list. (You can use @@ -556,7 +556,7 @@ module ActionView # of the US time zones, or a Regexp to select the zones of your choice) # # The +selected+ parameter must be either +nil+, or a string that names - # a ActiveSupport::TimeZone. + # an ActiveSupport::TimeZone. # # By default, +model+ is the ActiveSupport::TimeZone constant (which can # be obtained in Active Record as a value object). The only requirement @@ -655,7 +655,7 @@ module ActionView # # params.require(:user).permit(...) # - # will raise a error since no +{user: ...}+ will be present. + # will raise an error since no +{user: ...}+ will be present. # # To prevent this the helper generates an auxiliary hidden field before # every collection of radio buttons. The hidden field has the same name as collection radio button and blank value. diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index af684e05c6..0191064326 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -20,7 +20,7 @@ module ActionView mattr_accessor :embed_authenticity_token_in_remote_forms self.embed_authenticity_token_in_remote_forms = false - # Starts a form tag that points the action to an url configured with <tt>url_for_options</tt> just like + # Starts a form tag that points the action to a url configured with <tt>url_for_options</tt> just like # ActionController::Base#url_for. The method for the form defaults to POST. # # ==== Options @@ -450,9 +450,9 @@ module ActionView disable_with_text ||= value.clone tag_options.deep_merge!("data" => { "disable_with" => disable_with_text }) else - tag_options.delete("data-disable-with") tag_options["data"].delete(:disable_with) if tag_options["data"] end + tag_options.delete("data-disable-with") end tag :input, tag_options diff --git a/actionview/lib/action_view/layouts.rb b/actionview/lib/action_view/layouts.rb index cabba967c9..a74a5e05f3 100644 --- a/actionview/lib/action_view/layouts.rb +++ b/actionview/lib/action_view/layouts.rb @@ -401,7 +401,7 @@ module ActionView # ==== Parameters # * <tt>formats</tt> - The formats accepted to this layout # * <tt>require_layout</tt> - If set to true and layout is not found, - # an ArgumentError exception is raised (defaults to false) + # an +ArgumentError+ exception is raised (defaults to false) # # ==== Returns # * <tt>template</tt> - The template object for the default layout (or nil) diff --git a/actionview/lib/action_view/routing_url_for.rb b/actionview/lib/action_view/routing_url_for.rb index b4cbc80bd5..45e78d1ad9 100644 --- a/actionview/lib/action_view/routing_url_for.rb +++ b/actionview/lib/action_view/routing_url_for.rb @@ -32,7 +32,7 @@ module ActionView # # ==== Examples # <%= url_for(action: 'index') %> - # # => /blog/ + # # => /blogs/ # # <%= url_for(action: 'find', controller: 'books') %> # # => /books/find @@ -84,21 +84,13 @@ module ActionView when Hash options = options.symbolize_keys unless options.key?(:only_path) - if options[:host].nil? - options[:only_path] = _generate_paths_by_default - else - options[:only_path] = false - end + options[:only_path] = only_path?(options[:host]) 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 + options[:only_path] = only_path?(options[:host]) end super(options) @@ -147,5 +139,9 @@ module ActionView def _generate_paths_by_default true end + + def only_path?(host) + _generate_paths_by_default unless host + end end end diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 27150c7d5f..bdb9e0397b 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -466,7 +466,7 @@ class TestController < ApplicationController end def render_content_type_from_body - response.content_type = Mime::RSS + response.content_type = Mime[:rss] render body: "hello world!" end @@ -728,7 +728,7 @@ class RenderTest < ActionController::TestCase def test_render_process get :render_action_hello_world_as_string - assert_equal ["Hello world!"], @controller.process(:render_action_hello_world_as_string) + assert_equal "Hello world!", @controller.process(:render_action_hello_world_as_string) end # :ported: diff --git a/actionview/test/template/form_tag_helper_test.rb b/actionview/test/template/form_tag_helper_test.rb index a9d9562580..de1eb89dc5 100644 --- a/actionview/test/template/form_tag_helper_test.rb +++ b/actionview/test/template/form_tag_helper_test.rb @@ -64,6 +64,18 @@ class FormTagHelperTest < ActionView::TestCase assert_dom_equal expected, actual end + def test_check_box_tag_disabled + actual = check_box_tag "admin","1", false, disabled: true + expected = %(<input id="admin" disabled="disabled" name="admin" type="checkbox" value="1" />) + assert_dom_equal expected, actual + end + + def test_check_box_tag_default_checked + actual = check_box_tag "admin","1", true + expected = %(<input id="admin" checked="checked" name="admin" type="checkbox" value="1" />) + assert_dom_equal expected, actual + end + def test_check_box_tag_id_sanitized label_elem = root_elem(check_box_tag("project[2][admin]")) assert_match VALID_HTML_ID, label_elem['id'] @@ -351,12 +363,18 @@ class FormTagHelperTest < ActionView::TestCase assert_dom_equal expected, actual end - def test_text_field_disabled + def test_text_field_tag_disabled actual = text_field_tag "title", "Hello!", disabled: true expected = %(<input id="title" name="title" disabled="disabled" type="text" value="Hello!" />) assert_dom_equal expected, actual end + def test_text_field_tag_with_placeholder_option + actual = text_field_tag "title", "Hello!", placeholder: 'Enter search term...' + expected = %(<input id="title" name="title" placeholder="Enter search term..." type="text" value="Hello!" />) + assert_dom_equal expected, actual + end + def test_text_field_tag_with_multiple_options actual = text_field_tag "title", "Hello!", :size => 70, :maxlength => 80 expected = %(<input id="title" name="title" size="70" maxlength="80" type="text" value="Hello!" />) @@ -450,21 +468,21 @@ class FormTagHelperTest < ActionView::TestCase ActionView::Base.automatically_disable_submit_tag = true end - def test_data_disable_with_string + def test_submit_tag_having_data_disable_with_string assert_dom_equal( %(<input data-disable-with="Processing..." data-confirm="Are you sure?" name='commit' type="submit" value="Save" />), submit_tag("Save", { "data-disable-with" => "Processing...", "data-confirm" => "Are you sure?" }) ) end - def test_data_disable_with_boolean + def test_submit_tag_having_data_disable_with_boolean assert_dom_equal( %(<input data-confirm="Are you sure?" name='commit' type="submit" value="Save" />), submit_tag("Save", { "data-disable-with" => false, "data-confirm" => "Are you sure?" }) ) end - def test_data_hash_disable_with_boolean + def test_submit_tag_having_data_hash_disable_with_boolean assert_dom_equal( %(<input data-confirm="Are you sure?" name='commit' type="submit" value="Save" />), submit_tag("Save", { :data => { :confirm => "Are you sure?", :disable_with => false } }) @@ -485,6 +503,14 @@ class FormTagHelperTest < ActionView::TestCase ) end + def test_submit_tag_doesnt_have_data_disable_with_twice + assert_equal( + %(<input type="submit" name="commit" value="Save" data-confirm="Are you sure?" data-disable-with="Processing..." />), + submit_tag("Save", { "data-disable-with" => "Processing...", "data-confirm" => "Are you sure?" }) + ) + end + + def test_button_tag assert_dom_equal( %(<button name="button" type="submit">Button</button>), diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 1184cf7da8..2e3a3f9bae 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -52,7 +52,7 @@ class LookupContextTest < ActiveSupport::TestCase end test "handles explicitly defined */* formats fallback to :js" do - @lookup_context.formats = [:js, Mime::Type[:ALL]] + @lookup_context.formats = [:js, Mime::ALL] assert_equal [:js, *Mime::SET.symbols], @lookup_context.formats end diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index b5742191eb..79235019fe 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,12 @@ +* Fixed serializing `:at` option for `assert_enqueued_with` + and `assert_performed_with`. + + *Wojciech Wnętrzak* + +* Support passing array to `assert_enqueued_jobs` in `:only` option. + + *Wojciech Wnętrzak* + * Add job priorities to Active Job. *wvengen* diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index d72e1bdfce..605057d1e8 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -26,7 +26,7 @@ module ActiveJob end end - before_enqueue do |job| + after_enqueue do |job| if job.scheduled_at ActiveSupport::Notifications.instrument "enqueue_at.active_job", adapter: job.class.queue_adapter, job: job diff --git a/activejob/lib/active_job/queue_adapters/sneakers_adapter.rb b/activejob/lib/active_job/queue_adapters/sneakers_adapter.rb index f102c6567e..d78bdecdcb 100644 --- a/activejob/lib/active_job/queue_adapters/sneakers_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/sneakers_adapter.rb @@ -1,5 +1,5 @@ require 'sneakers' -require 'thread' +require 'monitor' module ActiveJob module QueueAdapters diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 200d82838e..44ddfa5f69 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -226,20 +226,22 @@ module ActiveJob # assert_enqueued_with(job: MyJob, args: [1,2,3], queue: 'low') do # MyJob.perform_later(1,2,3) # end + # + # assert_enqueued_with(job: MyJob, at: Date.tomorrow.noon) do + # MyJob.set(wait_until: Date.tomorrow.noon).perform_later + # end # end - def assert_enqueued_with(args = {}, &_block) - original_enqueued_jobs = enqueued_jobs.dup - clear_enqueued_jobs + def assert_enqueued_with(args = {}) + original_enqueued_jobs_count = enqueued_jobs.count args.assert_valid_keys(:job, :args, :at, :queue) serialized_args = serialize_args_for_assertion(args) yield - matching_job = enqueued_jobs.find do |job| + in_block_jobs = enqueued_jobs.drop(original_enqueued_jobs_count) + matching_job = in_block_jobs.find do |job| serialized_args.all? { |key, value| value == job[key] } end assert matching_job, "No enqueued job found with #{args}" instantiate_job(matching_job) - ensure - queue_adapter.enqueued_jobs = original_enqueued_jobs + enqueued_jobs end # Asserts that the job passed in the block has been performed with the given arguments. @@ -248,20 +250,22 @@ module ActiveJob # assert_performed_with(job: MyJob, args: [1,2,3], queue: 'high') do # MyJob.perform_later(1,2,3) # end + # + # assert_performed_with(job: MyJob, at: Date.tomorrow.noon) do + # MyJob.set(wait_until: Date.tomorrow.noon).perform_later + # end # end - def assert_performed_with(args = {}, &_block) - original_performed_jobs = performed_jobs.dup - clear_performed_jobs + def assert_performed_with(args = {}) + original_performed_jobs_count = performed_jobs.count args.assert_valid_keys(:job, :args, :at, :queue) serialized_args = serialize_args_for_assertion(args) perform_enqueued_jobs { yield } - matching_job = performed_jobs.find do |job| + in_block_jobs = performed_jobs.drop(original_performed_jobs_count) + matching_job = in_block_jobs.find do |job| serialized_args.all? { |key, value| value == job[key] } end assert matching_job, "No performed job found with #{args}" instantiate_job(matching_job) - ensure - queue_adapter.performed_jobs = original_performed_jobs + performed_jobs end def perform_enqueued_jobs(only: nil) @@ -290,31 +294,30 @@ module ActiveJob to: :queue_adapter private - def clear_enqueued_jobs + def clear_enqueued_jobs # :nodoc: enqueued_jobs.clear end - def clear_performed_jobs + def clear_performed_jobs # :nodoc: performed_jobs.clear end - def enqueued_jobs_size(only: nil) + def enqueued_jobs_size(only: nil) # :nodoc: if only - enqueued_jobs.select { |job| job.fetch(:job) == only }.size + enqueued_jobs.count { |job| Array(only).include?(job.fetch(:job)) } else - enqueued_jobs.size + enqueued_jobs.count end end - def serialize_args_for_assertion(args) - serialized_args = args.dup - if job_args = serialized_args.delete(:args) - serialized_args[:args] = ActiveJob::Arguments.serialize(job_args) + def serialize_args_for_assertion(args) # :nodoc: + args.dup.tap do |serialized_args| + serialized_args[:args] = ActiveJob::Arguments.serialize(serialized_args[:args]) if serialized_args[:args] + serialized_args[:at] = serialized_args[:at].to_f if serialized_args[:at] end - serialized_args end - def instantiate_job(payload) + def instantiate_job(payload) # :nodoc: job = payload[:job].new(*payload[:args]) job.scheduled_at = Time.at(payload[:at]) if payload.key?(:at) job.queue_name = payload[:queue] diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 8c60f037d2..f7ee763e8a 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -140,6 +140,16 @@ class EnqueuedJobsTest < ActiveJob::TestCase assert_match(/1 .* but 2/, error.message) end + def test_assert_enqueued_jobs_with_only_option_as_array + assert_nothing_raised do + assert_enqueued_jobs 2, only: [HelloJob, LoggingJob] do + HelloJob.perform_later('jeremy') + LoggingJob.perform_later('stewie') + RescueJob.perform_later('david') + end + end + end + def test_assert_no_enqueued_jobs_with_only_option assert_nothing_raised do assert_no_enqueued_jobs only: HelloJob do @@ -159,6 +169,14 @@ class EnqueuedJobsTest < ActiveJob::TestCase assert_match(/0 .* but 1/, error.message) end + def test_assert_no_enqueued_jobs_with_only_option_as_array + assert_nothing_raised do + assert_no_enqueued_jobs only: [HelloJob, RescueJob] do + LoggingJob.perform_later + end + end + end + def test_assert_enqueued_job assert_enqueued_with(job: LoggingJob, queue: 'default') do LoggingJob.set(wait_until: Date.tomorrow.noon).perform_later @@ -200,6 +218,12 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end + def test_assert_enqueued_job_with_at_option + assert_enqueued_with(job: HelloJob, at: Date.tomorrow.noon) do + HelloJob.set(wait_until: Date.tomorrow.noon).perform_later + end + end + def test_assert_enqueued_job_with_global_id_args ricardo = Person.new(9) assert_enqueued_with(job: HelloJob, args: [ricardo]) do @@ -218,6 +242,15 @@ class EnqueuedJobsTest < ActiveJob::TestCase assert_equal "No enqueued job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message end + + def test_assert_enqueued_job_does_not_change_jobs_count + HelloJob.perform_later + assert_enqueued_with(job: HelloJob) do + HelloJob.perform_later + end + + assert_equal 2, ActiveJob::Base.queue_adapter.enqueued_jobs.count + end end class PerformedJobsTest < ActiveJob::TestCase @@ -421,14 +454,26 @@ class PerformedJobsTest < ActiveJob::TestCase def test_assert_performed_job_failure assert_raise ActiveSupport::TestCase::Assertion do - assert_performed_with(job: LoggingJob, at: Date.tomorrow.noon, queue: 'default') do - NestedJob.set(wait_until: Date.tomorrow.noon).perform_later + assert_performed_with(job: LoggingJob) do + HelloJob.perform_later end end assert_raise ActiveSupport::TestCase::Assertion do - assert_performed_with(job: NestedJob, at: Date.tomorrow.noon, queue: 'low') do - NestedJob.set(queue: 'low', wait_until: Date.tomorrow.noon).perform_later + assert_performed_with(job: HelloJob, queue: 'low') do + HelloJob.set(queue: 'important').perform_later + end + end + end + + def test_assert_performed_job_with_at_option + assert_performed_with(job: HelloJob, at: Date.tomorrow.noon) do + HelloJob.set(wait_until: Date.tomorrow.noon).perform_later + end + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, at: Date.today.noon) do + HelloJob.set(wait_until: Date.tomorrow.noon).perform_later end end end @@ -451,4 +496,16 @@ class PerformedJobsTest < ActiveJob::TestCase assert_equal "No performed job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message end + + def test_assert_performed_job_does_not_change_jobs_count + assert_performed_with(job: HelloJob) do + HelloJob.perform_later + end + + assert_performed_with(job: HelloJob) do + HelloJob.perform_later + end + + assert_equal 2, ActiveJob::Base.queue_adapter.performed_jobs.count + end end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index bf0120122d..a3368cd197 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -119,10 +119,10 @@ The preferred method to halt a callback chain from now on is to explicitly `throw(:abort)`. - In the past, returning `false` in an ActiveModel or ActiveModel::Validations - `before_` callback had the side effect of halting the callback chain. + In the past, returning `false` in an Active Model `before_` callback had + the side effect of halting the callback chain. This is not recommended anymore and, depending on the value of the - `config.active_support.halt_callback_chains_on_return_false` option, will + `ActiveSupport.halt_callback_chains_on_return_false` option, will either not work at all or display a deprecation warning. diff --git a/activemodel/lib/active_model/forbidden_attributes_protection.rb b/activemodel/lib/active_model/forbidden_attributes_protection.rb index b4fa378601..d2c6a89cc2 100644 --- a/activemodel/lib/active_model/forbidden_attributes_protection.rb +++ b/activemodel/lib/active_model/forbidden_attributes_protection.rb @@ -17,8 +17,9 @@ module ActiveModel module ForbiddenAttributesProtection # :nodoc: protected def sanitize_for_mass_assignment(attributes) - if attributes.respond_to?(:permitted?) && !attributes.permitted? - raise ActiveModel::ForbiddenAttributesError + if attributes.respond_to?(:permitted?) + raise ActiveModel::ForbiddenAttributesError if !attributes.permitted? + attributes.to_h else attributes end diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index de5fb27467..d86ef6224e 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -226,7 +226,7 @@ module ActiveModel # (See ActiveModel::Name for more information). # # class Person - # include ActiveModel::Model + # extend ActiveModel::Naming # end # # Person.model_name.name # => "Person" diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb index 341651d00d..70e10fa06d 100644 --- a/activemodel/lib/active_model/serialization.rb +++ b/activemodel/lib/active_model/serialization.rb @@ -31,9 +31,9 @@ module ActiveModel # of the attributes hash's keys. In order to override this behavior, take a look # at the private method +read_attribute_for_serialization+. # - # The JSON serialization is provided by default when you include the - # <tt>ActiveModel::Serialization</tt> module, so there is no need to explicitly - # include it. + # ActiveModel::Serializers::JSON module automatically includes + # the <tt>ActiveModel::Serialization</tt> module, so there is no need to + # explicitly include <tt>ActiveModel::Serialization</tt>. # # A minimal implementation including JSON would be: # diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6683b58c36..ad65efc7bc 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,7 +1,31 @@ -* Make AR::Base #increment! and decrement! concurrency safe +* Concurrent `AR::Base#increment!` and `#decrement!` on the same record + are all reflected in the database rather than overwriting each other. *Bogdan Gusiev* +* Avoid leaking the first relation we call `first` on, per model. + + Fixes #21921. + + *Matthew Draper*, *Jean Boussier* + +* Remove unused `pk_and_sequence_for` in AbstractMysqlAdapter. + + *Ryuta Kamizono* + +* Allow fixtures files to set the model class in the YAML file itself. + + To load the fixtures file `accounts.yml` as the `User` model, use: + + _fixture: + model_class: User + david: + name: David + + Fixes #9516. + + *Roque Pinel* + * Don't require a database connection to load a class which uses acceptance validations. @@ -1108,7 +1132,7 @@ In the past, returning `false` in an Active Record `before_` callback had the side effect of halting the callback chain. This is not recommended anymore and, depending on the value of the - `config.active_support.halt_callback_chains_on_return_false` option, will + `ActiveSupport.halt_callback_chains_on_return_false` option, will either not work at all or display a deprecation warning. *claudiob* diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index 2b7e4f28c5..021bc32237 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -2,8 +2,7 @@ require 'active_support/core_ext/string/conversions' module ActiveRecord module Associations - # Keeps track of table aliases for ActiveRecord::Associations::ClassMethods::JoinDependency and - # ActiveRecord::Associations::ThroughAssociationScope + # Keeps track of table aliases for ActiveRecord::Associations::JoinDependency class AliasTracker # :nodoc: attr_reader :aliases diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 79b6b80d84..9994b72158 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -127,7 +127,7 @@ module ActiveRecord # # ] # # person.pets.find(1) # => #<Pet id: 1, name: "Fancy-Fancy", person_id: 1> - # person.pets.find(4) # => ActiveRecord::RecordNotFound: Couldn't find Pet with id=4 + # person.pets.find(4) # => ActiveRecord::RecordNotFound: Couldn't find Pet with 'id'=4 # # person.pets.find(2) { |pet| pet.name.downcase! } # # => #<Pet id: 2, name: "fancy-fancy", person_id: 1> @@ -443,7 +443,7 @@ module ActiveRecord # person.pets.delete_all # # Pet.find(1, 2, 3) - # # => ActiveRecord::RecordNotFound + # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) # # If it is set to <tt>:delete_all</tt>, all the objects are deleted # *without* calling their +destroy+ method. @@ -463,7 +463,7 @@ module ActiveRecord # person.pets.delete_all # # Pet.find(1, 2, 3) - # # => ActiveRecord::RecordNotFound + # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) def delete_all(dependent = nil) @association.delete_all(dependent) end @@ -557,7 +557,7 @@ module ActiveRecord # # => [#<Pet id: 2, name: "Spook", person_id: 1>] # # Pet.find(1, 3) - # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with IDs (1, 3) + # # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 3) # # If it is set to <tt>:delete_all</tt>, all the +records+ are deleted # *without* calling their +destroy+ method. @@ -585,7 +585,7 @@ module ActiveRecord # # ] # # Pet.find(1) - # # => ActiveRecord::RecordNotFound: Couldn't find Pet with id=1 + # # => ActiveRecord::RecordNotFound: Couldn't find Pet with 'id'=1 # # You can pass +Fixnum+ or +String+ values, it finds the records # responding to the +id+ and executes delete on them. @@ -649,7 +649,7 @@ module ActiveRecord # person.pets.size # => 0 # person.pets # => [] # - # Pet.find(1, 2, 3) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with IDs (1, 2, 3) + # Pet.find(1, 2, 3) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (1, 2, 3) # # You can pass +Fixnum+ or +String+ values, it finds the records # responding to the +id+ and then deletes them from the database. @@ -681,7 +681,7 @@ module ActiveRecord # person.pets.size # => 0 # person.pets # => [] # - # Pet.find(4, 5, 6) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with IDs (4, 5, 6) + # Pet.find(4, 5, 6) # => ActiveRecord::RecordNotFound: Couldn't find all Pets with 'id': (4, 5, 6) def destroy(*records) @association.destroy(*records) end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 92792a7a15..7a5a8f8ae6 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -137,7 +137,9 @@ module ActiveRecord scope.where_clause = reflection_scope.where_clause + preload_scope.where_clause scope.references_values = Array(values[:references]) + Array(preload_values[:references]) - scope._select! preload_values[:select] || values[:select] || table[Arel.star] + if preload_values[:select] || values[:select] + scope._select!(preload_values[:select] || values[:select]) + end scope.includes! preload_values[:includes] || values[:includes] if preload_scope.joins_values.any? scope.joins!(preload_scope.joins_values) diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 21fe032a9c..3c4c8f10ec 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -5,8 +5,8 @@ module ActiveRecord FromDatabase.new(name, value, type) end - def from_user(name, value, type) - FromUser.new(name, value, type) + def from_user(name, value, type, original_attribute = nil) + FromUser.new(name, value, type, original_attribute) end def with_cast_value(name, value, type) @@ -26,37 +26,46 @@ module ActiveRecord # This method should not be called directly. # Use #from_database or #from_user - def initialize(name, value_before_type_cast, type) + def initialize(name, value_before_type_cast, type, original_attribute = nil) @name = name @value_before_type_cast = value_before_type_cast @type = type + @original_attribute = original_attribute end def value # `defined?` is cheaper than `||=` when we get back falsy values - @value = original_value unless defined?(@value) + @value = type_cast(value_before_type_cast) unless defined?(@value) @value end def original_value - type_cast(value_before_type_cast) + if assigned? + original_attribute.original_value + else + type_cast(value_before_type_cast) + end end def value_for_database type.serialize(value) end - def changed_from?(old_value) - type.changed?(old_value, value, value_before_type_cast) + def changed? + changed_from_assignment? || changed_in_place? + end + + def changed_in_place? + has_been_read? && type.changed_in_place?(original_value_for_database, value) end - def changed_in_place_from?(old_value) - has_been_read? && type.changed_in_place?(old_value, value) + def forgetting_assignment + with_value_from_database(value_for_database) end def with_value_from_user(value) type.assert_valid_value(value) - self.class.from_user(name, value, type) + self.class.from_user(name, value, type, self) end def with_value_from_database(value) @@ -68,7 +77,7 @@ module ActiveRecord end def with_type(type) - self.class.new(name, value_before_type_cast, type) + self.class.new(name, value_before_type_cast, type, original_attribute) end def type_cast(*) @@ -101,16 +110,39 @@ module ActiveRecord protected + attr_reader :original_attribute + alias_method :assigned?, :original_attribute + def initialize_dup(other) if defined?(@value) && @value.duplicable? @value = @value.dup end end + def changed_from_assignment? + assigned? && type.changed?(original_value, value, value_before_type_cast) + end + + def original_value_for_database + if assigned? + original_attribute.original_value_for_database + else + _original_value_for_database + end + end + + def _original_value_for_database + value_for_database + end + class FromDatabase < Attribute # :nodoc: def type_cast(value) type.deserialize(value) end + + def _original_value_for_database + value_before_type_cast + end end class FromUser < Attribute # :nodoc: diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activerecord/lib/active_record/attribute/user_provided_default.rb index 501590cf0e..fb8ad9163e 100644 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ b/activerecord/lib/active_record/attribute/user_provided_default.rb @@ -4,8 +4,7 @@ module ActiveRecord class Attribute # :nodoc: class UserProvidedDefault < FromUser def initialize(name, value, type, database_default) - super(name, value, type) - @database_default = database_default + super(name, value, type, database_default) end def type_cast(value) @@ -16,17 +15,9 @@ module ActiveRecord end end - def changed_from?(old_value) - super || changed_from?(database_default.value) - end - def with_type(type) - self.class.new(name, value_before_type_cast, type, database_default) + self.class.new(name, value_before_type_cast, type, original_attribute) end - - protected - - attr_reader :database_default end end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 84b942d559..0bcfa5f00d 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -35,23 +35,22 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - clear_changes_information + @mutation_tracker = nil + @previous_mutation_tracker = nil + @changed_attributes = HashWithIndifferentAccess.new end end - def init_internals - super - @mutation_tracker = AttributeMutationTracker.new(@attributes, @attributes.dup) - end - def initialize_dup(other) # :nodoc: super - @mutation_tracker = AttributeMutationTracker.new(@attributes, - self.class._default_attributes.dup) + @attributes = self.class._default_attributes.map do |attr| + attr.with_value_from_user(@attributes.fetch_value(attr.name)) + end + @mutation_tracker = nil end def changes_applied - @previous_mutation_tracker = @mutation_tracker + @previous_mutation_tracker = mutation_tracker @changed_attributes = HashWithIndifferentAccess.new store_original_attributes end @@ -81,7 +80,7 @@ module ActiveRecord if defined?(@cached_changed_attributes) @cached_changed_attributes else - super.reverse_merge(@mutation_tracker.changed_values).freeze + super.reverse_merge(mutation_tracker.changed_values).freeze end end @@ -96,17 +95,24 @@ module ActiveRecord end def attribute_changed_in_place?(attr_name) - @mutation_tracker.changed_in_place?(attr_name) + mutation_tracker.changed_in_place?(attr_name) end private + def mutation_tracker + unless defined?(@mutation_tracker) + @mutation_tracker = nil + end + @mutation_tracker ||= AttributeMutationTracker.new(@attributes) + end + def changes_include?(attr_name) - super || @mutation_tracker.changed?(attr_name) + super || mutation_tracker.changed?(attr_name) end def clear_attribute_change(attr_name) - @mutation_tracker.forget_change(attr_name) + mutation_tracker.forget_change(attr_name) end def _update_record(*) @@ -122,11 +128,12 @@ module ActiveRecord end def store_original_attributes - @mutation_tracker = @mutation_tracker.now_tracking(@attributes) + @attributes = @attributes.map(&:forgetting_assignment) + @mutation_tracker = nil end def previous_mutation_tracker - @previous_mutation_tracker ||= NullMutationTracker.new + @previous_mutation_tracker ||= NullMutationTracker.instance end def cache_changed_attributes diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb index 08eb8ba7ac..0133b4d0be 100644 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ b/activerecord/lib/active_record/attribute_mutation_tracker.rb @@ -1,14 +1,13 @@ module ActiveRecord class AttributeMutationTracker # :nodoc: - def initialize(attributes, original_attributes) + def initialize(attributes) @attributes = attributes - @original_attributes = original_attributes end def changed_values attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| if changed?(attr_name) - result[attr_name] = original_attributes.fetch_value(attr_name) + result[attr_name] = attributes[attr_name].original_value end end end @@ -16,52 +15,39 @@ module ActiveRecord def changes attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| if changed?(attr_name) - result[attr_name] = [original_attributes.fetch_value(attr_name), attributes.fetch_value(attr_name)] + result[attr_name] = [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] end end end def changed?(attr_name) attr_name = attr_name.to_s - modified?(attr_name) || changed_in_place?(attr_name) + attributes[attr_name].changed? end def changed_in_place?(attr_name) - original_database_value = original_attributes[attr_name].value_before_type_cast - attributes[attr_name].changed_in_place_from?(original_database_value) + attributes[attr_name].changed_in_place? end def forget_change(attr_name) attr_name = attr_name.to_s - original_attributes[attr_name] = attributes[attr_name].dup - end - - def now_tracking(new_attributes) - AttributeMutationTracker.new(new_attributes, clean_copy_of(new_attributes)) + attributes[attr_name] = attributes[attr_name].forgetting_assignment end protected - attr_reader :attributes, :original_attributes + attr_reader :attributes private def attr_names attributes.keys end - - def modified?(attr_name) - attributes[attr_name].changed_from?(original_attributes.fetch_value(attr_name)) - end - - def clean_copy_of(attributes) - attributes.map do |attr| - attr.with_value_from_database(attr.value_for_database) - end - end end class NullMutationTracker # :nodoc: + include Singleton + def changed_values {} end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index ee278388a4..be581ac2a9 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -60,8 +60,14 @@ module ActiveRecord super end + def deep_dup + dup.tap do |copy| + copy.instance_variable_set(:@attributes, attributes.deep_dup) + end + end + def initialize_dup(_) - @attributes = attributes.deep_dup + @attributes = attributes.dup super end @@ -85,6 +91,10 @@ module ActiveRecord AttributeSet.new(new_attributes) end + def ==(other) + attributes == other.attributes + end + protected attr_reader :attributes diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 0c730d313f..3bd7c7997b 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -47,8 +47,14 @@ module ActiveRecord delegate_hash[key] = value end + def deep_dup + dup.tap do |copy| + copy.instance_variable_set(:@delegate_hash, delegate_hash.transform_values(&:dup)) + end + end + def initialize_dup(_) - @delegate_hash = delegate_hash.transform_values(&:dup) + @delegate_hash = Hash[delegate_hash] super end @@ -62,10 +68,29 @@ module ActiveRecord end end + def ==(other) + if other.is_a?(LazyAttributeHash) + materialize == other.materialize + else + materialize == other + end + end + protected attr_reader :types, :values, :additional_types, :delegate_hash + def materialize + unless @materialized + values.each_key { |key| self[key] } + types.each_key { |key| self[key] } + unless frozen? + @materialized = true + end + end + delegate_hash + end + private def assign_default_value(name) @@ -79,16 +104,5 @@ module ActiveRecord delegate_hash[name] = Attribute.uninitialized(name, type) end end - - def materialize - unless @materialized - values.each_key { |key| self[key] } - types.each_key { |key| self[key] } - unless frozen? - @materialized = true - end - end - delegate_hash - end end end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 8b2c4c7170..a573f76a8e 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -5,9 +5,6 @@ module ActiveRecord module Attributes extend ActiveSupport::Concern - # :nodoc: - Type = ActiveRecord::Type - included do class_attribute :attributes_to_define_after_schema_loads, instance_accessor: false # :internal: self.attributes_to_define_after_schema_loads = {} diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index b944a8631c..a2f58364a4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -18,7 +18,7 @@ module ActiveRecord spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type].include?(key) }) end - # This can be overridden on a Adapter level basis to support other + # This can be overridden on an Adapter level basis to support other # extended datatypes (Example: Adding an array option in the # PostgreSQLAdapter) def prepare_column_options(column) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 285b4065b1..ccff853987 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -507,7 +507,7 @@ module ActiveRecord raise NotImplementedError, "change_column_default is not implemented" end - # Sets or removes a +NOT NULL+ constraint on a column. The +null+ flag + # Sets or removes a <tt>NOT NULL</tt> constraint on a column. The +null+ flag # indicates whether the value can be +NULL+. For example # # change_column_null(:users, :nickname, false) @@ -519,7 +519,7 @@ module ActiveRecord # allows them to be +NULL+ (drops the constraint). # # The method accepts an optional fourth argument to replace existing - # +NULL+s with some other value. Use that one when enabling the + # <tt>NULL</tt>s with some other value. Use that one when enabling the # constraint if needed, since otherwise those rows would not be valid. # # Please note the fourth argument does not set a column's default. @@ -702,9 +702,11 @@ module ActiveRecord # [<tt>:index</tt>] # Add an appropriate index. Defaults to false. # [<tt>:foreign_key</tt>] - # Add an appropriate foreign key. Defaults to false. + # Add an appropriate foreign key constraint. Defaults to false. # [<tt>:polymorphic</tt>] # Whether an additional +_type+ column should be added. Defaults to false. + # [<tt>:null</tt>] + # Whether the column allows nulls. Defaults to true. # # ====== Create a user_id integer column # diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 79a24d1996..24408d6bb6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -458,7 +458,7 @@ module ActiveRecord end class ExplainPrettyPrinter # :nodoc: - # Pretty prints the result of a EXPLAIN in a way that resembles the output of the + # Pretty prints the result of an EXPLAIN in a way that resembles the output of the # MySQL shell: # # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ @@ -850,13 +850,6 @@ module ActiveRecord nil end - # Returns a table's primary key and belonging sequence. - def pk_and_sequence_for(table) - if pk = primary_key(table) - [ pk, nil ] - end - end - def primary_keys(table_name) # :nodoc: raise ArgumentError unless table_name.present? diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 11d3f5301a..43e18ebb2b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -8,7 +8,7 @@ module ActiveRecord end class ExplainPrettyPrinter # :nodoc: - # Pretty prints the result of a EXPLAIN in a way that resembles the output of the + # Pretty prints the result of an EXPLAIN in a way that resembles the output of the # PostgreSQL shell: # # QUERY PLAN diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index f6a6e3eb4d..505a71720f 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -218,7 +218,7 @@ module ActiveRecord end class ExplainPrettyPrinter - # Pretty prints the result of a EXPLAIN QUERY PLAN in a way that resembles + # Pretty prints the result of an EXPLAIN QUERY PLAN in a way that resembles # the output of the SQLite shell: # # 0|0|0|SEARCH TABLE users USING INTEGER PRIMARY KEY (rowid=?) (~1 rows) diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index d6b661ff76..2fc5e410f9 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -42,7 +42,7 @@ module ActiveRecord # # ActiveRecord::Base.establish_connection(:production) # - # The exceptions AdapterNotSpecified, AdapterNotFound and ArgumentError + # The exceptions +AdapterNotSpecified+, +AdapterNotFound+ and +ArgumentError+ # may be returned on an error. def establish_connection(spec = nil) spec ||= DEFAULT_ENV.call.to_sym diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index ffce2173ec..894d18b79e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -296,7 +296,7 @@ module ActiveRecord # # Instantiates a single new object # User.new(first_name: 'Jamie') def initialize(attributes = nil) - @attributes = self.class._default_attributes.dup + @attributes = self.class._default_attributes.deep_dup self.class.define_attribute_methods init_internals @@ -366,7 +366,7 @@ module ActiveRecord ## def initialize_dup(other) # :nodoc: - @attributes = @attributes.dup + @attributes = @attributes.deep_dup @attributes.reset(self.class.primary_key) _run_initialize_callbacks diff --git a/activerecord/lib/active_record/fixture_set/file.rb b/activerecord/lib/active_record/fixture_set/file.rb index 8132310c91..f969556c50 100644 --- a/activerecord/lib/active_record/fixture_set/file.rb +++ b/activerecord/lib/active_record/fixture_set/file.rb @@ -17,24 +17,39 @@ module ActiveRecord def initialize(file) @file = file - @rows = nil end def each(&block) rows.each(&block) end + def model_class + config_row['model_class'] + end private def rows - return @rows if @rows + @rows ||= raw_rows.reject { |fixture_name, _| fixture_name == '_fixture' } + end + + def config_row + @config_row ||= begin + row = raw_rows.find { |fixture_name, _| fixture_name == '_fixture' } + if row + row.last + else + {'model_class': nil} + end + end + end - begin + def raw_rows + @raw_rows ||= begin data = YAML.load(render(IO.read(@file))) + data ? validate(data).to_a : [] rescue ArgumentError, Psych::SyntaxError => error raise Fixture::FormatError, "a YAML error occurred parsing #{@file}. Please note that YAML must be consistently indented using spaces. Tabs are not allowed. Please have a look at http://www.yaml.org/faq.html\nThe exact error was:\n #{error.class}: #{error}", error.backtrace end - @rows = data ? validate(data).to_a : [] end def render(content) diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index f1dc56df63..59df3c78f3 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -395,6 +395,20 @@ module ActiveRecord # <<: *DEFAULTS # # Any fixture labeled "DEFAULTS" is safely ignored. + # + # == Configure the fixture model class + # + # It's possible to set the fixture's model class directly in the YAML file. + # This is helpful when fixtures are loaded outside tests and + # +set_fixture_class+ is not available (e.g. + # when running <tt>rake db:fixtures:load</tt>). + # + # _fixture: + # model_class: User + # david: + # name: David + # + # Any fixtures labeled "_fixture" are safely ignored. class FixtureSet #-- # An instance of FixtureSet is normally stored in a single YAML file and @@ -578,21 +592,16 @@ module ActiveRecord @name = name @path = path @config = config - @model_class = nil - if class_name.is_a?(Class) # TODO: Should be an AR::Base type class, or any? - @model_class = class_name - else - @model_class = class_name.safe_constantize if class_name - end + self.model_class = class_name + + @fixtures = read_fixture_files(path) @connection = connection @table_name = ( model_class.respond_to?(:table_name) ? model_class.table_name : self.class.default_fixture_table_name(name, config) ) - - @fixtures = read_fixture_files path, @model_class end def [](x) @@ -761,13 +770,25 @@ module ActiveRecord @column_names ||= @connection.columns(@table_name).collect(&:name) end - def read_fixture_files(path, model_class) + def model_class=(class_name) + if class_name.is_a?(Class) # TODO: Should be an AR::Base type class, or any? + @model_class = class_name + else + @model_class = class_name.safe_constantize if class_name + end + end + + # Loads the fixtures from the YAML file at +path+. + # If the file sets the +model_class+ and current instance value is not set, + # it uses the file value. + def read_fixture_files(path) yaml_files = Dir["#{path}/{**,*}/*.yml"].select { |f| ::File.file?(f) } + [yaml_file_path(path)] yaml_files.each_with_object({}) do |file, fixtures| FixtureSet::File.open(file) do |fh| + self.model_class ||= fh.model_class if fh.model_class fh.each do |fixture_name, row| fixtures[fixture_name] = ActiveRecord::Fixture.new(row, model_class) end diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 370c1562c9..19d2589db3 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -15,7 +15,7 @@ module ActiveRecord self.cache_timestamp_format = :nsec end - # Returns a String, which Action Pack uses for constructing an URL to this + # Returns a String, which Action Pack uses for constructing a URL to this # object. The default implementation returns this record's id as a String, # or nil if this record's unsaved. # diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 5bdb7213cd..90b8a29869 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -231,7 +231,7 @@ module ActiveRecord # * <tt>remove_index(table_name, name: index_name)</tt>: Removes the index # specified by +index_name+. # * <tt>add_reference(:table_name, :reference_name)</tt>: Adds a new column - # +reference_name_id+ by default a integer. See + # +reference_name_id+ by default an integer. See # ActiveRecord::ConnectionAdapters::SchemaStatements#add_reference for details. # # == Irreversible transformations @@ -947,7 +947,7 @@ module ActiveRecord def migrations_paths @migrations_paths ||= ['db/migrate'] - # just to not break things if someone uses: migration_path = some_string + # just to not break things if someone uses: migrations_path = some_string Array(@migrations_paths) end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 4c4afb4dbd..0fa665c7e0 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -9,6 +9,7 @@ module ActiveRecord # * add_index # * add_reference # * add_timestamps + # * change_column # * change_column_default (must supply a :from and :to option) # * change_column_null # * create_join_table @@ -18,6 +19,7 @@ module ActiveRecord # * drop_table (must supply a block) # * enable_extension # * remove_column (must supply a type) + # * remove_columns (must specify at least one column name or more) # * remove_foreign_key (must supply a second table) # * remove_index # * remove_reference diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 39e6057b9c..f659d0b10f 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -211,7 +211,7 @@ module ActiveRecord def becomes(klass) became = klass.new became.instance_variable_set("@attributes", @attributes) - became.instance_variable_set("@mutation_tracker", @mutation_tracker) + became.instance_variable_set("@mutation_tracker", @mutation_tracker) if defined?(@mutation_tracker) became.instance_variable_set("@changed_attributes", attributes_changed_by_setter) became.instance_variable_set("@new_record", new_record?) became.instance_variable_set("@destroyed", destroyed?) diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 6dd54f9262..2744673c12 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -121,7 +121,7 @@ module ActiveRecord # This sets the database configuration from Configuration#database_configuration # and then establishes the connection. - initializer "active_record.initialize_database" do |app| + initializer "active_record.initialize_database" do ActiveSupport.on_load(:active_record) do self.configurations = Rails.application.config.database_configuration diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 65ec356735..5b9d45d871 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -62,7 +62,7 @@ module ActiveRecord aggregate_reflections[aggregation.to_s] end - # Returns a Hash of name of the reflection as the key and a AssociationReflection as the value. + # Returns a Hash of name of the reflection as the key and an AssociationReflection as the value. # # Account.reflections # => {"balance" => AggregateReflection} # diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index e596df8742..36cdeed489 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -400,11 +400,11 @@ module ActiveRecord # people = Person.where(group: 'expert') # people.update(group: 'masters') # - # Note: Updating a large number of records will run a - # UPDATE query for each record, which may cause a performance - # issue. So if it is not needed to run callbacks for each update, it is - # preferred to use <tt>update_all</tt> for updating all records using - # a single query. + # Note: Updating a large number of records will run an + # UPDATE query for each record, which may cause a performance + # issue. So if it is not needed to run callbacks for each update, it is + # preferred to use <tt>update_all</tt> for updating all records using + # a single query. def update(id = :all, attributes) if id.is_a?(Array) id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) } diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index eb53a18f0f..7d95fe9a22 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -3,7 +3,6 @@ require "active_record/relation/query_attribute" require "active_record/relation/where_clause" require "active_record/relation/where_clause_factory" require 'active_model/forbidden_attributes_protection' -require 'active_support/core_ext/string/filters' module ActiveRecord module QueryMethods @@ -558,11 +557,8 @@ module ActiveRecord end def where!(opts, *rest) # :nodoc: - if Hash === opts - opts = sanitize_forbidden_attributes(opts) - references!(PredicateBuilder.references(opts)) - end - + opts = sanitize_forbidden_attributes(opts) + references!(PredicateBuilder.references(opts)) if Hash === opts self.where_clause += where_clause_factory.build(opts, rest) self end @@ -619,6 +615,7 @@ module ActiveRecord end def having!(opts, *rest) # :nodoc: + opts = sanitize_forbidden_attributes(opts) references!(PredicateBuilder.references(opts)) if Hash === opts self.having_clause += having_clause_factory.build(opts, rest) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 32f4229977..7f6664ea50 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -4,7 +4,7 @@ module ActiveRecord module ClassMethods # Used to sanitize objects before they're used in an SQL SELECT statement. Delegates to <tt>connection.quote</tt>. - def sanitize(object) #:nodoc: + def sanitize(object) # :nodoc: connection.quote(object) end alias_method :quote_value, :sanitize @@ -13,8 +13,15 @@ module ActiveRecord # Accepts an array or string of SQL conditions and sanitizes # them into a valid SQL fragment for a WHERE clause. - # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" - # "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'" + # + # sanitize_sql_for_conditions(["name=? and group_id=?", "foo'bar", 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_for_conditions(["name='%s' and group_id='%s'", "foo'bar", 4]) + # # => "name='foo''bar' and group_id='4'" + # + # sanitize_sql_for_conditions("name='foo''bar' and group_id='4'") + # # => "name='foo''bar' and group_id='4'" def sanitize_sql_for_conditions(condition) return nil if condition.blank? @@ -28,7 +35,15 @@ module ActiveRecord # Accepts an array, hash, or string of SQL conditions and sanitizes # them into a valid SQL fragment for a SET clause. - # { name: nil, group_id: 4 } returns "name = NULL , group_id='4'" + # + # sanitize_sql_for_assignment(["name=? and group_id=?", nil, 4]) + # # => "name=NULL and group_id=4" + # + # Post.send(:sanitize_sql_for_assignment, { name: nil, group_id: 4 }) + # # => "`posts`.`name` = NULL, `posts`.`group_id` = 4" + # + # sanitize_sql_for_assignment("name=NULL and group_id='4'") + # # => "name=NULL and group_id='4'" def sanitize_sql_for_assignment(assignments, default_table_name = self.table_name) case assignments when Array; sanitize_sql_array(assignments) @@ -40,14 +55,18 @@ module ActiveRecord # Accepts a hash of SQL conditions and replaces those attributes # that correspond to a +composed_of+ relationship with their expanded # aggregate attribute values. + # # Given: - # class Person < ActiveRecord::Base - # composed_of :address, class_name: "Address", - # mapping: [%w(address_street street), %w(address_city city)] - # end + # + # class Person < ActiveRecord::Base + # composed_of :address, class_name: "Address", + # mapping: [%w(address_street street), %w(address_city city)] + # end + # # Then: - # { address: Address.new("813 abc st.", "chicago") } - # # => { address_street: "813 abc st.", address_city: "chicago" } + # + # { address: Address.new("813 abc st.", "chicago") } + # # => { address_street: "813 abc st.", address_city: "chicago" } def expand_hash_conditions_for_aggregates(attrs) expanded_attrs = {} attrs.each do |attr, value| @@ -68,8 +87,9 @@ module ActiveRecord end # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause. - # { status: nil, group_id: 1 } - # # => "status = NULL , group_id = 1" + # + # sanitize_sql_hash_for_assignment({ status: nil, group_id: 1 }, "posts") + # # => "`posts`.`status` = NULL, `posts`.`group_id` = 1" def sanitize_sql_hash_for_assignment(attrs, table) c = connection attrs.map do |attr, value| @@ -79,7 +99,19 @@ module ActiveRecord end # Sanitizes a +string+ so that it is safe to use within an SQL - # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%" + # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%". + # + # sanitize_sql_like("100%") + # # => "100\\%" + # + # sanitize_sql_like("snake_cased_string") + # # => "snake\\_cased\\_string" + # + # sanitize_sql_like("100%", "!") + # # => "100!%" + # + # sanitize_sql_like("snake_cased_string", "!") + # # => "snake!_cased!_string" def sanitize_sql_like(string, escape_character = "\\") pattern = Regexp.union(escape_character, "%", "_") string.gsub(pattern) { |x| [escape_character, x].join } @@ -87,7 +119,12 @@ module ActiveRecord # Accepts an array of conditions. The array has each value # sanitized and interpolated into the SQL statement. - # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" + # + # sanitize_sql_array(["name=? and group_id=?", "foo'bar", 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_array(["name='%s' and group_id='%s'", "foo'bar", 4]) + # # => "name='foo''bar' and group_id='4'" def sanitize_sql_array(ary) statement, *values = ary if values.first.is_a?(Hash) && statement =~ /:\w+/ @@ -101,7 +138,7 @@ module ActiveRecord end end - def replace_bind_variables(statement, values) #:nodoc: + def replace_bind_variables(statement, values) # :nodoc: raise_if_bind_arity_mismatch(statement, statement.count('?'), values.size) bound = values.dup c = connection @@ -110,7 +147,7 @@ module ActiveRecord end end - def replace_bind_variable(value, c = connection) #:nodoc: + def replace_bind_variable(value, c = connection) # :nodoc: if ActiveRecord::Relation === value value.to_sql else @@ -118,7 +155,7 @@ module ActiveRecord end end - def replace_named_bind_variables(statement, bind_vars) #:nodoc: + def replace_named_bind_variables(statement, bind_vars) # :nodoc: statement.gsub(/(:?):([a-zA-Z]\w*)/) do |match| if $1 == ':' # skip postgresql casts match # return the whole match @@ -130,7 +167,7 @@ module ActiveRecord end end - def quote_bound_value(value, c = connection) #:nodoc: + def quote_bound_value(value, c = connection) # :nodoc: if value.respond_to?(:map) && !value.acts_like?(:string) if value.respond_to?(:empty?) && value.empty? c.quote(nil) @@ -142,7 +179,7 @@ module ActiveRecord end end - def raise_if_bind_arity_mismatch(statement, expected, provided) #:nodoc: + def raise_if_bind_arity_mismatch(statement, expected, provided) # :nodoc: unless expected == provided raise PreparedStatementInvalid, "wrong number of bind variables (#{provided} for #{expected}) in: #{statement}" end diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 7b62626896..b2f25752d3 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -39,8 +39,12 @@ module ActiveRecord end end - # Adds a class method for retrieving and querying objects. A \scope - # represents a narrowing of a database query, such as + # Adds a class method for retrieving and querying objects. + # The method is intended to return an <tt>ActiveRecord::Relation</tt> + # object, which is composable with other scopes. + # If it returns +nil+ or +false+, an <tt>all</tt> scope is returned instead. + # + # A \scope represents a narrowing of a database query, such as # <tt>where(color: :red).select('shirts.*').includes(:washing_instructions)</tt>. # # class Shirt < ActiveRecord::Base @@ -66,7 +70,8 @@ module ActiveRecord # end # # Unlike <tt>Shirt.find(...)</tt>, however, the object returned by - # <tt>Shirt.red</tt> is not an Array; it resembles the association object + # <tt>Shirt.red</tt> is not an Array but an <tt>ActiveRecord::Relation</tt>, + # which is composable with other scopes; it resembles the association object # constructed by a +has_many+ declaration. For instance, you can invoke # <tt>Shirt.red.first</tt>, <tt>Shirt.red.count</tt>, # <tt>Shirt.red.where(size: 'small')</tt>. Also, just as with the diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index 29573d8e0d..d2ce48fc00 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -65,30 +65,6 @@ module ActiveRecord end end - def test_pk_and_sequence_for - with_example_table do - pk, seq = @conn.pk_and_sequence_for('ex') - assert_equal 'id', pk - assert_equal @conn.default_sequence_name('ex', 'id'), seq - end - end - - def test_pk_and_sequence_for_with_non_standard_primary_key - with_example_table '`code` INT auto_increment, PRIMARY KEY (`code`)' do - pk, seq = @conn.pk_and_sequence_for('ex') - assert_equal 'code', pk - assert_equal @conn.default_sequence_name('ex', 'code'), seq - end - end - - def test_pk_and_sequence_for_with_custom_index_type_pk - with_example_table '`id` INT auto_increment, PRIMARY KEY USING BTREE (`id`)' do - pk, seq = @conn.pk_and_sequence_for('ex') - assert_equal 'id', pk - assert_equal @conn.default_sequence_name('ex', 'id'), seq - end - end - def test_composite_primary_key with_example_table '`id` INT, `number` INT, foo INT, PRIMARY KEY (`id`, `number`)' do assert_nil @conn.primary_key('ex') diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index b67201d94d..52d197718e 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -177,7 +177,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase if current_adapter?(:MysqlAdapter, :Mysql2Adapter) def test_read_attributes_before_type_cast_on_boolean - bool = Boolean.create({ "value" => false }) + bool = Boolean.create!({ "value" => false }) if RUBY_PLATFORM =~ /java/ # JRuby will return the value before typecast as string assert_equal "0", bool.reload.attributes_before_type_cast["value"] @@ -542,9 +542,6 @@ class AttributeMethodsTest < ActiveRecord::TestCase developer.save! - assert_equal "50000", developer.salary_before_type_cast - assert_equal 1337, developer.name_before_type_cast - assert_equal 50000, developer.salary assert_equal "1337", developer.name end diff --git a/activerecord/test/cases/attribute_set_test.rb b/activerecord/test/cases/attribute_set_test.rb index 7524243270..7a24b85a36 100644 --- a/activerecord/test/cases/attribute_set_test.rb +++ b/activerecord/test/cases/attribute_set_test.rb @@ -29,7 +29,7 @@ module ActiveRecord assert_equal :bar, attributes[:bar].name end - test "duping creates a new hash and dups each attribute" do + test "duping creates a new hash, but does not dup the attributes" do builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::String.new) attributes = builder.build_from_database(foo: 1, bar: 'foo') @@ -43,6 +43,24 @@ module ActiveRecord assert_equal 1, attributes[:foo].value assert_equal 2, duped[:foo].value + assert_equal 'foobar', attributes[:bar].value + assert_equal 'foobar', duped[:bar].value + end + + test "deep_duping creates a new hash and dups each attribute" do + builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::String.new) + attributes = builder.build_from_database(foo: 1, bar: 'foo') + + # Ensure the type cast value is cached + attributes[:foo].value + attributes[:bar].value + + duped = attributes.deep_dup + duped.write_from_database(:foo, 2) + duped[:bar].value << 'bar' + + assert_equal 1, attributes[:foo].value + assert_equal 2, duped[:foo].value assert_equal 'foo', attributes[:bar].value assert_equal 'foobar', duped[:bar].value end @@ -221,5 +239,15 @@ module ActiveRecord assert_equal 2, new_attributes.fetch_value(:foo) assert_equal 3, new_attributes.fetch_value(:bar) end + + test "comparison for equality is correctly implemented" do + builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Integer.new) + attributes = builder.build_from_database(foo: "1", bar: "2") + attributes2 = builder.build_from_database(foo: "1", bar: "2") + attributes3 = builder.build_from_database(foo: "2", bar: "2") + + assert_equal attributes, attributes2 + assert_not_equal attributes2, attributes3 + end end end diff --git a/activerecord/test/cases/attribute_test.rb b/activerecord/test/cases/attribute_test.rb index 0f216b7a13..a24a4fc6a4 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activerecord/test/cases/attribute_test.rb @@ -182,12 +182,49 @@ module ActiveRecord assert attribute.has_been_read? end + test "an attribute is not changed if it hasn't been assigned or mutated" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + + refute attribute.changed? + end + + test "an attribute is changed if it's been assigned a new value" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + changed = attribute.with_value_from_user(2) + + assert changed.changed? + end + + test "an attribute is not changed if it's assigned the same value" do + attribute = Attribute.from_database(:foo, 1, Type::Value.new) + unchanged = attribute.with_value_from_user(1) + + refute unchanged.changed? + end + test "an attribute can not be mutated if it has not been read, and skips expensive calculations" do type_which_raises_from_all_methods = Object.new attribute = Attribute.from_database(:foo, "bar", type_which_raises_from_all_methods) - assert_not attribute.changed_in_place_from?("bar") + assert_not attribute.changed_in_place? + end + + test "an attribute is changed if it has been mutated" do + attribute = Attribute.from_database(:foo, "bar", Type::String.new) + attribute.value << "!" + + assert attribute.changed_in_place? + assert attribute.changed? + end + + test "an attribute can forget its changes" do + attribute = Attribute.from_database(:foo, "bar", Type::String.new) + changed = attribute.with_value_from_user("foo") + forgotten = changed.forgetting_assignment + + assert changed.changed? # sanity check + refute forgotten.changed? end test "with_value_from_user validates the value" do diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index aa10817527..d904b802fa 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -681,4 +681,36 @@ class CalculationsTest < ActiveRecord::TestCase end assert block_called end + + def test_having_with_strong_parameters + protected_params = Class.new do + attr_reader :permitted + alias :permitted? :permitted + + def initialize(parameters) + @parameters = parameters + @permitted = false + end + + def to_h + @parameters + end + + def permit! + @permitted = true + self + end + end + + params = protected_params.new(credit_limit: '50') + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Account.group(:id).having(params) + end + + result = Account.group(:id).having(params.permit!) + assert_equal 50, result[0].credit_limit + assert_equal 50, result[1].credit_limit + assert_equal 50, result[2].credit_limit + end end diff --git a/activerecord/test/cases/fixture_set/file_test.rb b/activerecord/test/cases/fixture_set/file_test.rb index 92efa8aca7..242e7a9bec 100644 --- a/activerecord/test/cases/fixture_set/file_test.rb +++ b/activerecord/test/cases/fixture_set/file_test.rb @@ -123,6 +123,18 @@ END end end + def test_removes_fixture_config_row + File.open(::File.join(FIXTURES_ROOT, 'other_posts.yml')) do |fh| + assert_equal(['second_welcome'], fh.each.map { |name, _| name }) + end + end + + def test_extracts_model_class_from_config_row + File.open(::File.join(FIXTURES_ROOT, 'other_posts.yml')) do |fh| + assert_equal 'Post', fh.model_class + end + end + private def tmp_yaml(name, contents) t = Tempfile.new name diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index e3c4bd9f4a..a0eaa66e94 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -7,15 +7,16 @@ require 'models/binary' require 'models/book' require 'models/bulb' require 'models/category' +require 'models/comment' require 'models/company' require 'models/computer' require 'models/course' require 'models/developer' +require 'models/doubloon' require 'models/joke' require 'models/matey' require 'models/parrot' require 'models/pirate' -require 'models/doubloon' require 'models/post' require 'models/randomly_named_c1' require 'models/reply' @@ -516,6 +517,38 @@ class OverRideFixtureMethodTest < ActiveRecord::TestCase end end +class FixtureWithSetModelClassTest < ActiveRecord::TestCase + fixtures :other_posts, :other_comments + + # Set to false to blow away fixtures cache and ensure our fixtures are loaded + # and thus takes into account the +set_model_class+. + self.use_transactional_tests = false + + def test_uses_fixture_class_defined_in_yaml + assert_kind_of Post, other_posts(:second_welcome) + end + + def test_loads_the_associations_to_fixtures_with_set_model_class + post = other_posts(:second_welcome) + comment = other_comments(:second_greetings) + assert_equal [comment], post.comments + assert_equal post, comment.post + end +end + +class SetFixtureClassPrevailsTest < ActiveRecord::TestCase + set_fixture_class bad_posts: Post + fixtures :bad_posts + + # Set to false to blow away fixtures cache and ensure our fixtures are loaded + # and thus takes into account the +set_model_class+. + self.use_transactional_tests = false + + def test_uses_set_fixture_class + assert_kind_of Post, bad_posts(:bad_welcome) + end +end + class CheckSetTableNameFixturesTest < ActiveRecord::TestCase set_fixture_class :funny_jokes => Joke fixtures :funny_jokes diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 8773986882..89bd3f888e 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -47,9 +47,7 @@ def in_memory_db? end def subsecond_precision_supported? - !current_adapter?(:MysqlAdapter, :Mysql2Adapter) || - (ActiveRecord::Base.connection.send(:version) >= '5.6.0' && - ActiveRecord::Base.connection.send(:version) < '5.7.0') + !current_adapter?(:MysqlAdapter, :Mysql2Adapter) || ActiveRecord::Base.connection.send(:version) >= '5.6.4' end def mysql_enforcing_gtid_consistency? diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb index 80d17b8114..9169207b0a 100644 --- a/activerecord/test/cases/integration_test.rb +++ b/activerecord/test/cases/integration_test.rb @@ -96,7 +96,9 @@ class IntegrationTest < ActiveRecord::TestCase owner.update_column :updated_at, Time.current key = owner.cache_key - assert pet.touch + travel(1.second) do + assert pet.touch + end assert_not_equal key, owner.reload.cache_key end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index 6af31017d6..c3a1471205 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -276,5 +276,31 @@ module ActiveRecord assert_equal essays(:david_modest_proposal), essay end + + def test_where_with_strong_parameters + protected_params = Class.new do + attr_reader :permitted + alias :permitted? :permitted + + def initialize(parameters) + @parameters = parameters + @permitted = false + end + + def to_h + @parameters + end + + def permit! + @permitted = true + self + end + end + + author = authors(:david) + params = protected_params.new(name: author.name) + assert_raises(ActiveModel::ForbiddenAttributesError) { Author.where(params) } + assert_equal author, Author.where(params.permit!).first + end end end diff --git a/activerecord/test/fixtures/bad_posts.yml b/activerecord/test/fixtures/bad_posts.yml new file mode 100644 index 0000000000..addee8e3bf --- /dev/null +++ b/activerecord/test/fixtures/bad_posts.yml @@ -0,0 +1,9 @@ +# Please do not use this fixture without `set_fixture_class` as Post + +_fixture: + model_class: BadPostModel + +bad_welcome: + author_id: 1 + title: Welcome to the another weblog + body: It's really nice today diff --git a/activerecord/test/fixtures/other_comments.yml b/activerecord/test/fixtures/other_comments.yml new file mode 100644 index 0000000000..55e8216ec7 --- /dev/null +++ b/activerecord/test/fixtures/other_comments.yml @@ -0,0 +1,6 @@ +_fixture: + model_class: Comment + +second_greetings: + post: second_welcome + body: Thank you for the second welcome diff --git a/activerecord/test/fixtures/other_posts.yml b/activerecord/test/fixtures/other_posts.yml new file mode 100644 index 0000000000..39ff763547 --- /dev/null +++ b/activerecord/test/fixtures/other_posts.yml @@ -0,0 +1,7 @@ +_fixture: + model_class: Post + +second_welcome: + author_id: 1 + title: Welcome to the another weblog + body: It's really nice today diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 8c1f14bd36..0d90cbb110 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -144,9 +144,6 @@ class Author < ActiveRecord::Base has_many :posts_with_signature, ->(record) { where("posts.title LIKE ?", "%by #{record.name.downcase}%") }, class_name: "Post" - scope :relation_include_posts, -> { includes(:posts) } - scope :relation_include_tags, -> { includes(:tags) } - attr_accessor :post_log after_initialize :set_post_log diff --git a/activerecord/test/models/parrot.rb b/activerecord/test/models/parrot.rb index b26035d944..ddc9dcaf29 100644 --- a/activerecord/test/models/parrot.rb +++ b/activerecord/test/models/parrot.rb @@ -21,9 +21,3 @@ end class DeadParrot < Parrot belongs_to :killer, :class_name => 'Pirate', foreign_key: :killer_id end - -class FunkyParrot < Parrot - before_destroy do - raise "before_destroy was called" - end -end diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index ad12f00d42..a4a9c6b0d4 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -37,7 +37,6 @@ class Person < ActiveRecord::Base has_many :essays, primary_key: "first_name", foreign_key: "writer_id" scope :males, -> { where(:gender => 'M') } - scope :females, -> { where(:gender => 'F') } end class PersonWithDependentDestroyJobs < ActiveRecord::Base diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index a39344e464..b05a068dc1 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* `Cache#fetch` instrumentation marks whether it was a `:hit`. + + *Robin Clowers* + * `assert_difference` and `assert_no_difference` now returns the result of the yielded block. @@ -291,18 +295,15 @@ In the past, callbacks could only be halted by explicitly providing a terminator and by having a callback match the conditions of the terminator. -* Add `Callbacks::CallbackChain.halt_and_display_warning_on_return_false` +* Add `ActiveSupport.halt_callback_chains_on_return_false` - Setting `Callbacks::CallbackChain.halt_and_display_warning_on_return_false` + Setting `ActiveSupport.halt_callback_chains_on_return_false` to `true` will let an app support the deprecated way of halting Active Record, - Active Model and Active Model validations callback chains by returning `false`. + and Active Model callback chains by returning `false`. Setting the value to `false` will tell the app to ignore any `false` value returned by those callbacks, and only halt the chain upon `throw(:abort)`. - The value can also be set with the Rails configuration option - `config.active_support.halt_callback_chains_on_return_false`. - When the configuration option is missing, its value is `true`, so older apps ported to Rails 5.0 will not break (but display a deprecation warning). For new Rails 5.0 apps, its value is set to `false` in an initializer, so diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index d236dbd200..93878518d7 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -24,6 +24,6 @@ Gem::Specification.new do |s| s.add_dependency 'json', '~> 1.7', '>= 1.7.7' s.add_dependency 'tzinfo', '~> 1.1' s.add_dependency 'minitest', '~> 5.1' - s.add_dependency 'concurrent-ruby', '~> 1.0.0.pre2', '< 2.0.0' + s.add_dependency 'concurrent-ruby', '~> 1.0.0.pre3', '< 2.0.0' s.add_dependency 'method_source' end diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index 588d6c49f9..63277a65b4 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -75,11 +75,11 @@ module ActiveSupport cattr_accessor :test_order # :nodoc: def self.halt_callback_chains_on_return_false - Callbacks::CallbackChain.halt_and_display_warning_on_return_false + Callbacks.halt_and_display_warning_on_return_false end def self.halt_callback_chains_on_return_false=(value) - Callbacks::CallbackChain.halt_and_display_warning_on_return_false = value + Callbacks.halt_and_display_warning_on_return_false = value end end diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 8253a76383..3996f583c2 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -277,13 +277,18 @@ module ActiveSupport options = merged_options(options) key = namespaced_key(name, options) - cached_entry = find_cached_entry(key, name, options) unless options[:force] - entry = handle_expired_entry(cached_entry, key, options) + instrument(:read, name, options) do |payload| + cached_entry = read_entry(key, options) unless options[:force] + payload[:super_operation] = :fetch if payload + entry = handle_expired_entry(cached_entry, key, options) - if entry - get_entry_value(entry, name, options) - else - save_block_result_to_cache(name, options) { |_name| yield _name } + if entry + payload[:hit] = true if payload + get_entry_value(entry, name, options) + else + payload[:hit] = false if payload + save_block_result_to_cache(name, options) { |_name| yield _name } + end end else read(name, options) @@ -556,13 +561,6 @@ module ActiveSupport logger.debug(yield) end - def find_cached_entry(key, name, options) - instrument(:read, name, options) do |payload| - payload[:super_operation] = :fetch if payload - read_entry(key, options) - end - end - def handle_expired_entry(entry, key, options) if entry && entry.expired? race_ttl = options[:race_condition_ttl].to_i diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 3db9ea2ac0..252374e817 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/kernel/reporting' require 'active_support/core_ext/kernel/singleton_class' +require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/string/filters' require 'active_support/deprecation' require 'thread' @@ -66,6 +67,12 @@ module ActiveSupport CALLBACK_FILTER_TYPES = [:before, :after, :around] + # If true, Active Record and Active Model callbacks returning +false+ will + # halt the entire callback chain and display a deprecation message. + # If false, callback chains will only be halted by calling +throw :abort+. + # Defaults to +true+. + mattr_accessor(:halt_and_display_warning_on_return_false) { true } + # Runs the callbacks for the given event. # # Calls the before and around callbacks in the order they were set, yields @@ -450,12 +457,6 @@ module ActiveSupport attr_reader :name, :config - # If true, any callback returning +false+ will halt the entire callback - # chain and display a deprecation message. If false, callback chains will - # only be halted by calling +throw :abort+. Defaults to +true+. - class_attribute :halt_and_display_warning_on_return_false - self.halt_and_display_warning_on_return_false = true - def initialize(name, config) @name = name @config = { @@ -760,7 +761,7 @@ module ActiveSupport terminate = true catch(:abort) do result = result_lambda.call if result_lambda.is_a?(Proc) - if CallbackChain.halt_and_display_warning_on_return_false && result == false + if Callbacks.halt_and_display_warning_on_return_false && result == false display_deprecation_warning_for_false_terminator else terminate = false diff --git a/activesupport/lib/active_support/core_ext/date/calculations.rb b/activesupport/lib/active_support/core_ext/date/calculations.rb index c60e833441..d589b67bf7 100644 --- a/activesupport/lib/active_support/core_ext/date/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date/calculations.rb @@ -26,7 +26,7 @@ class Date Thread.current[:beginning_of_week] = find_beginning_of_week!(week_start) end - # Returns week start day symbol (e.g. :monday), or raises an ArgumentError for invalid day symbol. + # Returns week start day symbol (e.g. :monday), or raises an +ArgumentError+ for invalid day symbol. def find_beginning_of_week!(week_start) raise ArgumentError, "Invalid beginning of week: #{week_start}" unless ::Date::DAYS_INTO_WEEK.key?(week_start) week_start diff --git a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb index 40811dafc0..e079af594d 100644 --- a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb @@ -135,7 +135,7 @@ module DateAndTime end alias :at_end_of_quarter :end_of_quarter - # Return a new date/time at the beginning of the year. + # Returns a new date/time at the beginning of the year. # # today = Date.today # => Fri, 10 Jul 2015 # today.beginning_of_year # => Thu, 01 Jan 2015 diff --git a/activesupport/lib/active_support/core_ext/date_time/conversions.rb b/activesupport/lib/active_support/core_ext/date_time/conversions.rb index 2a9c09fc29..f59d05b214 100644 --- a/activesupport/lib/active_support/core_ext/date_time/conversions.rb +++ b/activesupport/lib/active_support/core_ext/date_time/conversions.rb @@ -40,6 +40,8 @@ class DateTime alias_method :to_default_s, :to_s if instance_methods(false).include?(:to_s) alias_method :to_s, :to_formatted_s + # Returns a formatted string of the offset from UTC, or an alternative + # string if the time zone is already UTC. # # datetime = DateTime.civil(2000, 1, 1, 0, 0, 0, Rational(-6, 24)) # datetime.formatted_offset # => "-06:00" diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb index c30044b9ff..07a282e8b6 100644 --- a/activesupport/lib/active_support/core_ext/hash/keys.rb +++ b/activesupport/lib/active_support/core_ext/hash/keys.rb @@ -1,10 +1,14 @@ class Hash - # Returns a new hash with all keys converted using the block operation. + # Returns a new hash with all keys converted using the +block+ operation. # # hash = { name: 'Rob', age: '28' } # - # hash.transform_keys{ |key| key.to_s.upcase } - # # => {"NAME"=>"Rob", "AGE"=>"28"} + # hash.transform_keys { |key| key.to_s.upcase } # => {"NAME"=>"Rob", "AGE"=>"28"} + # + # If you do not provide a +block+, it will return an Enumerator + # for chaining with other methods: + # + # hash.transform_keys.with_index { |k, i| [k, i].join } # => {"name0"=>"Rob", "age1"=>"28"} def transform_keys return enum_for(:transform_keys) unless block_given? result = self.class.new @@ -14,8 +18,8 @@ class Hash result end - # Destructively converts all keys using the block operations. - # Same as transform_keys but modifies +self+. + # Destructively converts all keys using the +block+ operations. + # Same as +transform_keys+ but modifies +self+. def transform_keys! return enum_for(:transform_keys!) unless block_given? keys.each do |key| @@ -60,7 +64,7 @@ class Hash alias_method :to_options!, :symbolize_keys! # Validates all keys in a hash match <tt>*valid_keys</tt>, raising - # ArgumentError on a mismatch. + # +ArgumentError+ on a mismatch. # # Note that keys are treated differently than HashWithIndifferentAccess, # meaning that string and symbol keys will not match. diff --git a/activesupport/lib/active_support/core_ext/hash/transform_values.rb b/activesupport/lib/active_support/core_ext/hash/transform_values.rb index e9bcce761f..9ddb838774 100644 --- a/activesupport/lib/active_support/core_ext/hash/transform_values.rb +++ b/activesupport/lib/active_support/core_ext/hash/transform_values.rb @@ -2,10 +2,15 @@ class Hash # Returns a new hash with the results of running +block+ once for every value. # The keys are unchanged. # - # { a: 1, b: 2, c: 3 }.transform_values { |x| x * 2 } - # # => { a: 2, b: 4, c: 6 } + # { a: 1, b: 2, c: 3 }.transform_values { |x| x * 2 } # => { a: 2, b: 4, c: 6 } + # + # If you do not provide a +block+, it will return an Enumerator + # for chaining with other methods: + # + # { a: 1, b: 2 }.transform_values.with_index { |v, i| [v, i].join.to_i } # => { a: 10, b: 21 } def transform_values return enum_for(:transform_values) unless block_given? + return {} if empty? result = self.class.new each do |key, value| result[key] = yield(value) @@ -13,7 +18,8 @@ class Hash result end - # Destructive +transform_values+ + # Destructively converts all values using the +block+ operations. + # Same as +transform_values+ but modifies +self+. def transform_values! return enum_for(:transform_values!) unless block_given? each do |key, value| diff --git a/activesupport/lib/active_support/core_ext/marshal.rb b/activesupport/lib/active_support/core_ext/marshal.rb index 20a0856e71..e333b26133 100644 --- a/activesupport/lib/active_support/core_ext/marshal.rb +++ b/activesupport/lib/active_support/core_ext/marshal.rb @@ -6,7 +6,7 @@ module ActiveSupport if exc.message.match(%r|undefined class/module (.+)|) # try loading the class/module $1.constantize - # if it is a IO we need to go back to read the object + # if it is an IO we need to go back to read the object source.rewind if source.respond_to?(:rewind) retry else diff --git a/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb b/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb index a084177b9f..bf175a8a70 100644 --- a/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb +++ b/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb @@ -53,7 +53,7 @@ class Module def mattr_reader(*syms) options = syms.extract_options! syms.each do |sym| - raise NameError.new("invalid attribute name: #{sym}") unless sym =~ /^[_A-Za-z]\w*$/ + raise NameError.new("invalid attribute name: #{sym}") unless sym =~ /\A[_A-Za-z]\w*\z/ class_eval(<<-EOS, __FILE__, __LINE__ + 1) @@#{sym} = nil unless defined? @@#{sym} @@ -119,7 +119,7 @@ class Module def mattr_writer(*syms) options = syms.extract_options! syms.each do |sym| - raise NameError.new("invalid attribute name: #{sym}") unless sym =~ /^[_A-Za-z]\w*$/ + raise NameError.new("invalid attribute name: #{sym}") unless sym =~ /\A[_A-Za-z]\w*\z/ class_eval(<<-EOS, __FILE__, __LINE__ + 1) @@#{sym} = nil unless defined? @@#{sym} diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb index f13d09f3ad..82e003fc3b 100644 --- a/activesupport/lib/active_support/core_ext/time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/time/calculations.rb @@ -16,9 +16,9 @@ class Time super || (self == Time && other.is_a?(ActiveSupport::TimeWithZone)) end - # Return the number of days in the given month. + # Returns the number of days in the given month. # If no year is specified, it will use the current year. - def days_in_month(month, year = now.year) + def days_in_month(month, year = current.year) if month == 2 && ::Date.gregorian_leap?(year) 29 else @@ -51,9 +51,9 @@ class Time # Returns the number of seconds since 00:00:00. # - # Time.new(2012, 8, 29, 0, 0, 0).seconds_since_midnight # => 0 - # Time.new(2012, 8, 29, 12, 34, 56).seconds_since_midnight # => 45296 - # Time.new(2012, 8, 29, 23, 59, 59).seconds_since_midnight # => 86399 + # Time.new(2012, 8, 29, 0, 0, 0).seconds_since_midnight # => 0.0 + # Time.new(2012, 8, 29, 12, 34, 56).seconds_since_midnight # => 45296.0 + # Time.new(2012, 8, 29, 23, 59, 59).seconds_since_midnight # => 86399.0 def seconds_since_midnight to_i - change(:hour => 0).to_i + (usec / 1.0e+6) end diff --git a/activesupport/lib/active_support/core_ext/time/conversions.rb b/activesupport/lib/active_support/core_ext/time/conversions.rb index dbf1f2f373..eecbac2c20 100644 --- a/activesupport/lib/active_support/core_ext/time/conversions.rb +++ b/activesupport/lib/active_support/core_ext/time/conversions.rb @@ -24,7 +24,7 @@ class Time # # This method is aliased to <tt>to_s</tt>. # - # time = Time.now # => Thu Jan 18 06:10:17 CST 2007 + # time = Time.now # => 2007-01-18 06:10:17 -06:00 # # time.to_formatted_s(:time) # => "06:10" # time.to_s(:time) # => "06:10" @@ -55,7 +55,8 @@ class Time alias_method :to_default_s, :to_s alias_method :to_s, :to_formatted_s - # Returns the UTC offset as an +HH:MM formatted string. + # Returns a formatted string of the offset from UTC, or an alternative + # string if the time zone is already UTC. # # Time.local(2000).formatted_offset # => "-06:00" # Time.local(2000).formatted_offset(false) # => "-0600" diff --git a/activesupport/lib/active_support/core_ext/time/zones.rb b/activesupport/lib/active_support/core_ext/time/zones.rb index 133d3938eb..877dc84ec8 100644 --- a/activesupport/lib/active_support/core_ext/time/zones.rb +++ b/activesupport/lib/active_support/core_ext/time/zones.rb @@ -53,7 +53,7 @@ class Time # Returns a TimeZone instance matching the time zone provided. # Accepts the time zone in any format supported by <tt>Time.zone=</tt>. - # Raises an ArgumentError for invalid time zones. + # Raises an +ArgumentError+ for invalid time zones. # # Time.find_zone! "America/New_York" # => #<ActiveSupport::TimeZone @name="America/New_York" ...> # Time.find_zone! "EST" # => #<ActiveSupport::TimeZone @name="EST" ...> diff --git a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb index 6572ff78a6..6f0ad445fc 100644 --- a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb @@ -111,7 +111,7 @@ module ActiveSupport # # PLANETS = %w(mercury venus earth mars jupiter saturn uranus neptune pluto) # - # (In a later update, the orignal implementation of `PLANETS` has been removed.) + # (In a later update, the original implementation of `PLANETS` has been removed.) # # PLANETS_POST_2006 = %w(mercury venus earth mars jupiter saturn uranus neptune) # PLANETS = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('PLANETS', 'PLANETS_POST_2006') diff --git a/activesupport/lib/active_support/inflector/transliterate.rb b/activesupport/lib/active_support/inflector/transliterate.rb index 7472d4386a..103207fb63 100644 --- a/activesupport/lib/active_support/inflector/transliterate.rb +++ b/activesupport/lib/active_support/inflector/transliterate.rb @@ -69,10 +69,12 @@ module ActiveSupport # parameterize("Donald E. Knuth") # => "donald-e-knuth" # parameterize("^trés|Jolie-- ") # => "tres-jolie" def parameterize(string, sep = '-') - # replace accented chars with their ascii equivalents + # Replace accented chars with their ASCII equivalents. parameterized_string = transliterate(string) - # Turn unwanted chars into the separator + + # Turn unwanted chars into the separator. parameterized_string.gsub!(/[^a-z0-9\-_]+/i, sep) + unless sep.nil? || sep.empty? if sep == "-".freeze re_duplicate_separator = /-{2,}/ @@ -87,6 +89,7 @@ module ActiveSupport # Remove leading/trailing separator. parameterized_string.gsub!(re_leading_trailing_separator, ''.freeze) end + parameterized_string.downcase! parameterized_string end diff --git a/activesupport/lib/active_support/multibyte/unicode.rb b/activesupport/lib/active_support/multibyte/unicode.rb index 5221a6dfb8..586002b03b 100644 --- a/activesupport/lib/active_support/multibyte/unicode.rb +++ b/activesupport/lib/active_support/multibyte/unicode.rb @@ -256,7 +256,7 @@ module ActiveSupport # * <tt>string</tt> - The string to perform normalization on. # * <tt>form</tt> - The form you want to normalize in. Should be one of # the following: <tt>:c</tt>, <tt>:kc</tt>, <tt>:d</tt>, or <tt>:kd</tt>. - # Default is ActiveSupport::Multibyte.default_normalization_form. + # Default is ActiveSupport::Multibyte::Unicode.default_normalization_form. def normalize(string, form=nil) form ||= @default_normalization_form # See http://www.unicode.org/reports/tr15, Table 1 diff --git a/activesupport/lib/active_support/per_thread_registry.rb b/activesupport/lib/active_support/per_thread_registry.rb index ca2e4d5625..506dd950cb 100644 --- a/activesupport/lib/active_support/per_thread_registry.rb +++ b/activesupport/lib/active_support/per_thread_registry.rb @@ -43,9 +43,9 @@ module ActiveSupport protected def method_missing(name, *args, &block) # :nodoc: # Caches the method definition as a singleton method of the receiver. - define_singleton_method(name) do |*a, &b| - instance.public_send(name, *a, &b) - end + # + # By letting #delegate handle it, we avoid an enclosure that'll capture args. + singleton_class.delegate name, to: :instance send(name, *args, &block) end diff --git a/activesupport/lib/active_support/testing/file_fixtures.rb b/activesupport/lib/active_support/testing/file_fixtures.rb index 4c6a0801b8..affb84cda5 100644 --- a/activesupport/lib/active_support/testing/file_fixtures.rb +++ b/activesupport/lib/active_support/testing/file_fixtures.rb @@ -18,7 +18,7 @@ module ActiveSupport # Returns a +Pathname+ to the fixture file named +fixture_name+. # - # Raises ArgumentError if +fixture_name+ can't be found. + # Raises +ArgumentError+ if +fixture_name+ can't be found. def file_fixture(fixture_name) path = Pathname.new(File.join(file_fixture_path, fixture_name)) diff --git a/activesupport/lib/active_support/testing/isolation.rb b/activesupport/lib/active_support/testing/isolation.rb index 1de0a19998..edf8b30a0a 100644 --- a/activesupport/lib/active_support/testing/isolation.rb +++ b/activesupport/lib/active_support/testing/isolation.rb @@ -41,7 +41,23 @@ module ActiveSupport pid = fork do read.close yield - write.puts [Marshal.dump(self.dup)].pack("m") + begin + if error? + failures.map! { |e| + begin + Marshal.dump e + e + rescue TypeError + ex = Exception.new e.message + ex.set_backtrace e.backtrace + Minitest::UnexpectedError.new ex + end + } + end + result = Marshal.dump(self.dup) + end + + write.puts [result].pack("m") exit! end diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index a82a7dfea0..3592dcba39 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -41,6 +41,9 @@ module ActiveSupport 'Time' end + PRECISIONS = Hash.new { |h, n| h[n] = "%FT%T.%#{n}N".freeze } + PRECISIONS[0] = '%FT%T'.freeze + include Comparable attr_reader :time_zone @@ -142,11 +145,7 @@ module ActiveSupport # # Time.zone.now.xmlschema # => "2014-12-04T11:02:37-05:00" def xmlschema(fraction_digits = 0) - fraction = if fraction_digits.to_i > 0 - (".%06i" % time.usec)[0, fraction_digits.to_i + 1] - end - - "#{time.strftime("%Y-%m-%dT%H:%M:%S")}#{fraction}#{formatted_offset(true, 'Z')}" + "#{time.strftime(PRECISIONS[fraction_digits.to_i])}#{formatted_offset(true, 'Z'.freeze)}" end alias_method :iso8601, :xmlschema diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 4502e01b12..681f659100 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -285,8 +285,12 @@ module ActiveSupport end end - # Returns the offset of this time zone as a formatted string, of the - # format "+HH:MM". + # Returns a formatted string of the offset from UTC, or an alternative + # string if the time zone is already UTC. + # + # zone = ActiveSupport::TimeZone['Central Time (US & Canada)'] + # zone.formatted_offset # => "-06:00" + # zone.formatted_offset(false) # => "-0600" def formatted_offset(colon=true, alternate_utc_string = nil) utc_offset == 0 && alternate_utc_string || self.class.seconds_to_utc_offset(utc_offset, colon) end @@ -384,7 +388,7 @@ module ActiveSupport time_now.utc.in_time_zone(self) end - # Return the current date in this time zone. + # Returns the current date in this time zone. def today tzinfo.now.to_date end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 74ceff44f9..c235dee5e1 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -490,6 +490,34 @@ module CacheStoreBehavior assert_equal({key => "bar"}, @cache.read_multi(key)) assert @cache.delete(key) end + + def test_cache_hit_instrumentation + key = "test_key" + subscribe_executed = false + ActiveSupport::Notifications.subscribe "cache_read.active_support" do |name, start, finish, id, payload| + subscribe_executed = true + assert_equal :fetch, payload[:super_operation] + assert payload[:hit] + end + assert @cache.write(key, "1", :raw => true) + assert @cache.fetch(key) {} + assert subscribe_executed + ensure + ActiveSupport::Notifications.unsubscribe "cache_read.active_support" + end + + def test_cache_miss_instrumentation + subscribe_executed = false + ActiveSupport::Notifications.subscribe "cache_read.active_support" do |name, start, finish, id, payload| + subscribe_executed = true + assert_equal :fetch, payload[:super_operation] + assert_not payload[:hit] + end + assert_not @cache.fetch("bad_key") {} + assert subscribe_executed + ensure + ActiveSupport::Notifications.unsubscribe "cache_read.active_support" + end end # https://rails.lighthouseapp.com/projects/8994/tickets/6225-memcachestore-cant-deal-with-umlauts-and-special-characters diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 4f47e0519f..3b00ff87a0 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -776,7 +776,7 @@ module CallbacksTest class CallbackFalseTerminatorWithConfigTrueTest < ActiveSupport::TestCase def setup - ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = true + ActiveSupport::Callbacks.halt_and_display_warning_on_return_false = true end def test_returning_false_does_not_halt_callback_if_config_variable_is_true @@ -789,7 +789,7 @@ module CallbacksTest class CallbackFalseTerminatorWithConfigFalseTest < ActiveSupport::TestCase def setup - ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = false + ActiveSupport::Callbacks.halt_and_display_warning_on_return_false = false end def test_returning_false_does_not_halt_callback_if_config_variable_is_false diff --git a/activesupport/test/core_ext/module/attribute_accessor_test.rb b/activesupport/test/core_ext/module/attribute_accessor_test.rb index 128c5e3d1a..0b0f3a2808 100644 --- a/activesupport/test/core_ext/module/attribute_accessor_test.rb +++ b/activesupport/test/core_ext/module/attribute_accessor_test.rb @@ -69,6 +69,20 @@ class ModuleAttributeAccessorTest < ActiveSupport::TestCase end end assert_equal "invalid attribute name: 1nvalid", exception.message + + exception = assert_raises NameError do + Class.new do + mattr_reader "valid_part\ninvalid_part" + end + end + assert_equal "invalid attribute name: valid_part\ninvalid_part", exception.message + + exception = assert_raises NameError do + Class.new do + mattr_writer "valid_part\ninvalid_part" + end + end + assert_equal "invalid attribute name: valid_part\ninvalid_part", exception.message end def test_should_use_default_value_if_block_passed diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index ccb7f02331..c40f0bacbf 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -110,14 +110,14 @@ class TimeWithZoneTest < ActiveSupport::TestCase @twz += 0.1234560001 # advance the time by a fraction of a second assert_equal "1999-12-31T19:00:00.123-05:00", @twz.xmlschema(3) assert_equal "1999-12-31T19:00:00.123456-05:00", @twz.xmlschema(6) - assert_equal "1999-12-31T19:00:00.123456-05:00", @twz.xmlschema(12) + assert_equal "1999-12-31T19:00:00.123456000100-05:00", @twz.xmlschema(12) end def test_xmlschema_with_fractional_seconds_lower_than_hundred_thousand @twz += 0.001234 # advance the time by a fraction assert_equal "1999-12-31T19:00:00.001-05:00", @twz.xmlschema(3) assert_equal "1999-12-31T19:00:00.001234-05:00", @twz.xmlschema(6) - assert_equal "1999-12-31T19:00:00.001234-05:00", @twz.xmlschema(12) + assert_equal "1999-12-31T19:00:00.001234000000-05:00", @twz.xmlschema(12) end def test_xmlschema_with_nil_fractional_seconds diff --git a/guides/rails_guides/generator.rb b/guides/rails_guides/generator.rb index 43f6f7eecf..b7a94f144c 100644 --- a/guides/rails_guides/generator.rb +++ b/guides/rails_guides/generator.rb @@ -194,7 +194,7 @@ module RailsGuides layout = kindle? ? 'kindle/layout' : 'layout' File.open(output_path, 'w') do |f| - view = ActionView::Base.new(source_dir, :edge => @edge, :version => @version, :mobi => "kindle/#{mobi}") + view = ActionView::Base.new(source_dir, :edge => @edge, :version => @version, :mobi => "kindle/#{mobi}", :lang => @lang) view.extend(Helpers) if guide =~ /\.(\w+)\.erb$/ diff --git a/guides/rails_guides/helpers.rb b/guides/rails_guides/helpers.rb index a78c2e9fca..5bf73da16c 100644 --- a/guides/rails_guides/helpers.rb +++ b/guides/rails_guides/helpers.rb @@ -15,7 +15,7 @@ module RailsGuides end def documents_by_section - @documents_by_section ||= YAML.load_file(File.expand_path('../../source/documents.yaml', __FILE__)) + @documents_by_section ||= YAML.load_file(File.expand_path("../../source/#{@lang ? @lang + '/' : ''}documents.yaml", __FILE__)) end def documents_flat diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index 76454e77c7..4b0e9bff7c 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -1247,7 +1247,7 @@ file_field_tag 'attachment' #### form_tag -Starts a form tag that points the action to an url configured with `url_for_options` just like `ActionController::Base#url_for`. +Starts a form tag that points the action to a url configured with `url_for_options` just like `ActionController::Base#url_for`. ```html+erb <%= form_tag '/articles' do %> diff --git a/guides/source/active_job_basics.md b/guides/source/active_job_basics.md index e3502d7363..17087d187a 100644 --- a/guides/source/active_job_basics.md +++ b/guides/source/active_job_basics.md @@ -296,7 +296,7 @@ emails asynchronously: ```ruby I18n.locale = :eo -UserMailer.welcome(@user).deliver_later # Email will be localized to Esparanto. +UserMailer.welcome(@user).deliver_later # Email will be localized to Esperanto. ``` diff --git a/guides/source/active_record_postgresql.md b/guides/source/active_record_postgresql.md index 9d495dfacb..f71e6ccd57 100644 --- a/guides/source/active_record_postgresql.md +++ b/guides/source/active_record_postgresql.md @@ -220,11 +220,22 @@ normal text columns: ```ruby # db/migrate/20131220144913_create_articles.rb -execute <<-SQL - CREATE TYPE article_status AS ENUM ('draft', 'published'); -SQL -create_table :articles do |t| - t.column :status, :article_status +def up + execute <<-SQL + CREATE TYPE article_status AS ENUM ('draft', 'published'); + SQL + create_table :articles do |t| + t.column :status, :article_status + end +end + +# NOTE: It's important to drop table before dropping enum. +def down + drop_table :articles + + execute <<-SQL + DROP TYPE article_status; + SQL end # app/models/article.rb @@ -240,6 +251,31 @@ article.status = "published" article.save! ``` +To add a new value before/after existing one you should use [ALTER TYPE](http://www.postgresql.org/docs/current/static/sql-altertype.html): +```ruby +# db/migrate/20150720144913_add_new_state_to_articles.rb +# NOTE: ALTER TYPE ... ADD VALUE cannot be executed inside of a transaction block so here we are using disable_ddl_transaction! +disable_ddl_transaction! + +def up + execute <<-SQL + ALTER TYPE article_status ADD VALUE IF NOT EXISTS 'archived' AFTER 'published'; + SQL +end +``` + +NOTE: by now we can't drop ENUM values. You can read why [here](http://www.postgresql.org/message-id/29F36C7C98AB09499B1A209D48EAA615B7653DBC8A@mail2a.alliedtesting.com). + +Hint: to show all the values of the all enums you have, you should call this query in `bin/rails db` or `psql` console: +```sql +SELECT n.nspname AS enum_schema, + t.typname AS enum_name, + e.enumlabel AS enum_value + FROM pg_type t + JOIN pg_enum e ON t.oid = e.enumtypid + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace +``` + ### UUID * [type definition](http://www.postgresql.org/docs/current/static/datatype-uuid.html) diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index 60790b33a4..999c533fb3 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -932,8 +932,11 @@ If you set the `:dependent` option to: * `:destroy`, when the object is destroyed, `destroy` will be called on its associated objects. -* `:delete`, when the object is destroyed, all its associated objects will be +* `:delete_all`, when the object is destroyed, all its associated objects will be deleted directly from the database without calling their `destroy` method. +* `:nullify`, causes the foreign key to be set to `NULL`. Callbacks are not executed. +* `:restrict_with_exception`, causes an exception to be raised if there is an associated record +* `:restrict_with_error`, causes an error to be added to the owner if there is an associated object WARNING: You should not specify this option on a `belongs_to` association that is connected with a `has_many` association on the other class. Doing so can lead to orphaned records in your database. diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 0dd99cf8e9..87114c4ef0 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -244,7 +244,7 @@ config.middleware.swap ActionController::Failsafe, Lifo::Failsafe They can also be removed from the stack completely: ```ruby -config.middleware.delete "Rack::MethodOverride" +config.middleware.delete Rack::MethodOverride ``` ### Configuring i18n @@ -535,7 +535,7 @@ There are a few configuration options available in Active Support: * `config.active_support.time_precision` sets the precision of JSON encoded time values. Defaults to `3`. -* `config.active_support.halt_callback_chains_on_return_false` specifies whether ActiveRecord, ActiveModel and ActiveModel::Validations callback chains can be halted by returning `false` in a 'before' callback. Defaults to `true`. +* `ActiveSupport.halt_callback_chains_on_return_false` specifies whether Active Record and Active Model callback chains can be halted by returning `false` in a 'before' callback. Defaults to `true`. * `ActiveSupport::Logger.silencer` is set to `false` to disable the ability to silence logging in a block. The default is `true`. @@ -641,7 +641,7 @@ TIP: You don't have to update the database configurations manually. If you look ### Connection Preference -Since there are two ways to set your connection, via environment variable it is important to understand how the two can interact. +Since there are two ways to configure your connection (using `config/database.yml` or using an environment variable) it is important to understand how they can interact. If you have an empty `config/database.yml` file but your `ENV['DATABASE_URL']` is present, then Rails will connect to the database via your environment variable: diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 625299c113..6d689804a8 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -128,7 +128,7 @@ Contributing to the Rails Documentation Ruby on Rails has two main sets of documentation: the guides, which help you learn about Ruby on Rails, and the API, which serves as a reference. -You can help improve the Rails guides by making them more coherent, consistent or readable, adding missing information, correcting factual errors, fixing typos, or bringing it up to date with the latest edge Rails. To get involved in the translation of Rails guides, please see [Translating Rails Guides](https://wiki.github.com/rails/docrails/translating-rails-guides). +You can help improve the Rails guides by making them more coherent, consistent or readable, adding missing information, correcting factual errors, fixing typos, or bringing them up to date with the latest edge Rails. To get involved in the translation of Rails guides, please see [Translating Rails Guides](https://wiki.github.com/rails/docrails/translating-rails-guides). You can either open a pull request to [Rails](http://github.com/rails/rails) or ask the [Rails core team](http://rubyonrails.org/core) for commit access on @@ -318,7 +318,7 @@ $ cd activerecord $ bundle exec rake test:sqlite3 ``` -You can now run the tests as you did for `sqlite3`. The tasks are respectively +You can now run the tests as you did for `sqlite3`. The tasks are respectively: ```bash test:mysql diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index 84a8d695cb..0a6e2e5dba 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -40,7 +40,9 @@ When called without arguments like this, it creates a `<form>` tag which, when s </form> ``` -You'll notice that the HTML contains `input` element with type `hidden`. This `input` is important, because the form cannot be successfully submitted without it. The hidden input element has name attribute of `utf8` enforces browsers to properly respect your form's character encoding and is generated for all forms whether their actions are "GET" or "POST". The second input element with name `authenticity_token` is a security feature of Rails called **cross-site request forgery protection**, and form helpers generate it for every non-GET form (provided that this security feature is enabled). You can read more about this in the [Security Guide](security.html#cross-site-request-forgery-csrf). +You'll notice that the HTML contains an `input` element with type `hidden`. This `input` is important, because the form cannot be successfully submitted without it. The hidden input element with the name `utf8` enforces browsers to properly respect your form's character encoding and is generated for all forms whether their action is "GET" or "POST". + +The second input element with the name `authenticity_token` is a security feature of Rails called **cross-site request forgery protection**, and form helpers generate it for every non-GET form (provided that this security feature is enabled). You can read more about this in the [Security Guide](security.html#cross-site-request-forgery-csrf). ### A Generic Search Form @@ -103,9 +105,9 @@ checkboxes, text fields, and radio buttons. These basic helpers, with names ending in `_tag` (such as `text_field_tag` and `check_box_tag`), generate just a single `<input>` element. The first parameter to these is always the name of the input. When the form is submitted, the name will be passed along with the form -data, and will make its way to the `params` hash in the controller with the -value entered by the user for that field. For example, if the form contains `<%= -text_field_tag(:query) %>`, then you would be able to get the value of this +data, and will make its way to the `params` in the controller with the +value entered by the user for that field. For example, if the form contains +`<%= text_field_tag(:query) %>`, then you would be able to get the value of this field in the controller with `params[:query]`. When naming inputs, Rails uses certain conventions that make it possible to submit parameters with non-scalar values such as arrays or hashes, which will also be accessible in `params`. You can read more about them in [chapter 7 of this guide](#understanding-parameter-naming-conventions). For details on the precise usage of these helpers, please refer to the [API documentation](http://api.rubyonrails.org/classes/ActionView/Helpers/FormTagHelper.html). @@ -212,7 +214,7 @@ month, week, URL, email, number and range inputs are HTML5 controls. If you require your app to have a consistent experience in older browsers, you will need an HTML5 polyfill (provided by CSS and/or JavaScript). There is definitely [no shortage of solutions for this](https://github.com/Modernizr/Modernizr/wiki/HTML5-Cross-Browser-Polyfills), although a popular tool at the moment is -[Modernizr](http://www.modernizr.com/), which provides a simple way to add functionality based on the presence of +[Modernizr](https://modernizr.com/), which provides a simple way to add functionality based on the presence of detected HTML5 features. TIP: If you're using password input fields (for any purpose), you might want to configure your application to prevent those parameters from being logged. You can learn about this in the [Security Guide](security.html#logging). @@ -376,7 +378,7 @@ output: </form> ``` -When parsing POSTed data, Rails will take into account the special `_method` parameter and acts as if the HTTP method was the one specified inside it ("PATCH" in this example). +When parsing POSTed data, Rails will take into account the special `_method` parameter and act as if the HTTP method was the one specified inside it ("PATCH" in this example). Making Select Boxes with Ease ----------------------------- @@ -711,13 +713,6 @@ action for a Person model, `params[:person]` would usually be a hash of all the Fundamentally HTML forms don't know about any sort of structured data, all they generate is name-value pairs, where pairs are just plain strings. The arrays and hashes you see in your application are the result of some parameter naming conventions that Rails uses. -TIP: You may find you can try out examples in this section faster by using the console to directly invoke Rack's parameter parser. For example, - -```ruby -Rack::Utils.parse_query "name=fred&phone=0123456789" -# => {"name"=>"fred", "phone"=>"0123456789"} -``` - ### Basic Structures The two basic structures are arrays and hashes. Hashes mirror the syntax used for accessing the value in `params`. For example, if a form contains: diff --git a/guides/source/i18n.md b/guides/source/i18n.md index ea79855919..87d2fafaf3 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -249,7 +249,7 @@ end With this approach you will not get a `Routing Error` when accessing your resources such as `http://localhost:3001/books` without a locale. This is useful for when you want to use the default locale when one is not specified. -Of course, you need to take special care of the root URL (usually "homepage" or "dashboard") of your application. An URL like `http://localhost:3001/nl` will not work automatically, because the `root to: "books#index"` declaration in your `routes.rb` doesn't take locale into account. (And rightly so: there's only one "root" URL.) +Of course, you need to take special care of the root URL (usually "homepage" or "dashboard") of your application. A URL like `http://localhost:3001/nl` will not work automatically, because the `root to: "books#index"` declaration in your `routes.rb` doesn't take locale into account. (And rightly so: there's only one "root" URL.) You would probably need to map URLs like these: diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index b425eb126a..71cc030f6a 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -280,7 +280,7 @@ render body: "raw" ``` TIP: This option should be used only if you don't care about the content type of -the response. Using `:plain` or `:html` might be more appropriate in most of the +the response. Using `:plain` or `:html` might be more appropriate most of the time. NOTE: Unless overridden, your response returned from this render option will be @@ -781,7 +781,7 @@ The `javascript_include_tag` helper returns an HTML `script` tag for each source If you are using Rails with the [Asset Pipeline](asset_pipeline.html) enabled, this helper will generate a link to `/assets/javascripts/` rather than `public/javascripts` which was used in earlier versions of Rails. This link is then served by the asset pipeline. -A JavaScript file within a Rails application or Rails engine goes in one of three locations: `app/assets`, `lib/assets` or `vendor/assets`. These locations are explained in detail in the [Asset Organization section in the Asset Pipeline Guide](asset_pipeline.html#asset-organization) +A JavaScript file within a Rails application or Rails engine goes in one of three locations: `app/assets`, `lib/assets` or `vendor/assets`. These locations are explained in detail in the [Asset Organization section in the Asset Pipeline Guide](asset_pipeline.html#asset-organization). You can specify a full path relative to the document root, or a URL, if you prefer. For example, to link to a JavaScript file that is inside a directory called `javascripts` inside of one of `app/assets`, `lib/assets` or `vendor/assets`, you would do this: diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index 0db90fedb3..87f869aff3 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -171,7 +171,7 @@ Add the following lines to your application configuration: ```ruby # config/application.rb -config.middleware.delete "Rack::Lock" +config.middleware.delete Rack::Lock ``` And now if you inspect the middleware stack, you'll find that `Rack::Lock` is @@ -191,16 +191,16 @@ If you want to remove session related middleware, do the following: ```ruby # config/application.rb -config.middleware.delete "ActionDispatch::Cookies" -config.middleware.delete "ActionDispatch::Session::CookieStore" -config.middleware.delete "ActionDispatch::Flash" +config.middleware.delete ActionDispatch::Cookies +config.middleware.delete ActionDispatch::Session::CookieStore +config.middleware.delete ActionDispatch::Flash ``` And to remove browser related middleware, ```ruby # config/application.rb -config.middleware.delete "Rack::MethodOverride" +config.middleware.delete Rack::MethodOverride ``` ### Internal Middleware Stack diff --git a/guides/source/routing.md b/guides/source/routing.md index 1fd38c0940..3175716a9c 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -1096,7 +1096,7 @@ Video.find_by(identifier: params[:identifier]) ``` You can override `ActiveRecord::Base#to_param` of a related model to construct -an URL: +a URL: ```ruby class Video < ActiveRecord::Base diff --git a/guides/source/security.md b/guides/source/security.md index 5a6ac9446a..fb9ee7b412 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -196,7 +196,7 @@ This attack method works by including malicious code or a link in a page that ac  -In the [session chapter](#sessions) you have learned that most Rails applications use cookie-based sessions. Either they store the session id in the cookie and have a server-side session hash, or the entire session hash is on the client-side. In either case the browser will automatically send along the cookie on every request to a domain, if it can find a cookie for that domain. The controversial point is, that it will also send the cookie, if the request comes from a site of a different domain. Let's start with an example: +In the [session chapter](#sessions) you have learned that most Rails applications use cookie-based sessions. Either they store the session id in the cookie and have a server-side session hash, or the entire session hash is on the client-side. In either case the browser will automatically send along the cookie on every request to a domain, if it can find a cookie for that domain. The controversial point is that if the request comes from a site of a different domain, it will also send the cookie. Let's start with an example: * Bob browses a message board and views a post from a hacker where there is a crafted HTML image element. The element references a command in Bob's project management application, rather than an image file: `<img src="http://www.webapp.com/project/1/destroy">` * Bob's session at `www.webapp.com` is still alive, because he didn't log out a few minutes ago. @@ -224,9 +224,9 @@ The HTTP protocol basically provides two main types of requests - GET and POST ( * The interaction _changes the state_ of the resource in a way that the user would perceive (e.g., a subscription to a service), or * The user is _held accountable for the results_ of the interaction. -If your web application is RESTful, you might be used to additional HTTP verbs, such as PATCH, PUT or DELETE. Most of today's web browsers, however do not support them - only GET and POST. Rails uses a hidden `_method` field to handle this barrier. +If your web application is RESTful, you might be used to additional HTTP verbs, such as PATCH, PUT or DELETE. Most of today's web browsers, however, do not support them - only GET and POST. Rails uses a hidden `_method` field to handle this barrier. -_POST requests can be sent automatically, too_. Here is an example for a link which displays `www.harmless.com` as destination in the browser's status bar. In fact it dynamically creates a new form that sends a POST request. +_POST requests can be sent automatically, too_. In this example, the link www.harmless.com is shown as the destination in the browser's status bar. But it has actually dynamically created a new form that sends a POST request. ```html <a href="http://www.harmless.com/" onclick=" @@ -301,7 +301,7 @@ This will redirect the user to the main action if they tried to access a legacy http://www.example.com/site/legacy?param1=xy¶m2=23&host=www.attacker.com ``` -If it is at the end of the URL it will hardly be noticed and redirects the user to the attacker.com host. A simple countermeasure would be to _include only the expected parameters in a legacy action_ (again a whitelist approach, as opposed to removing unexpected parameters). _And if you redirect to an URL, check it with a whitelist or a regular expression_. +If it is at the end of the URL it will hardly be noticed and redirects the user to the attacker.com host. A simple countermeasure would be to _include only the expected parameters in a legacy action_ (again a whitelist approach, as opposed to removing unexpected parameters). _And if you redirect to a URL, check it with a whitelist or a regular expression_. #### Self-contained XSS @@ -406,7 +406,7 @@ NOTE: _Almost every web application has to deal with authorization and authentic There are a number of authentication plug-ins for Rails available. Good ones, such as the popular [devise](https://github.com/plataformatec/devise) and [authlogic](https://github.com/binarylogic/authlogic), store only encrypted passwords, not plain-text passwords. In Rails 3.1 you can use the built-in `has_secure_password` method which has similar features. -Every new user gets an activation code to activate their account when they get an e-mail with a link in it. After activating the account, the activation_code columns will be set to NULL in the database. If someone requested an URL like these, they would be logged in as the first activated user found in the database (and chances are that this is the administrator): +Every new user gets an activation code to activate their account when they get an e-mail with a link in it. After activating the account, the activation_code columns will be set to NULL in the database. If someone requested a URL like these, they would be logged in as the first activated user found in the database (and chances are that this is the administrator): ``` http://localhost:3006/user/activate @@ -793,15 +793,13 @@ Another proof-of-concept webmail worm is Nduja, a cross-domain worm for four Ita In December 2006, 34,000 actual user names and passwords were stolen in a [MySpace phishing attack](http://news.netcraft.com/archives/2006/10/27/myspace_accounts_compromised_by_phishers.html). The idea of the attack was to create a profile page named "login_home_index_html", so the URL looked very convincing. Specially-crafted HTML and CSS was used to hide the genuine MySpace content from the page and instead display its own login form. -The MySpace Samy worm will be discussed in the CSS Injection section. - ### CSS Injection INFO: _CSS Injection is actually JavaScript injection, because some browsers (IE, some versions of Safari and others) allow JavaScript in CSS. Think twice about allowing custom CSS in your web application._ -CSS Injection is explained best by a well-known worm, the [MySpace Samy worm](http://namb.la/popular/tech.html). This worm automatically sent a friend request to Samy (the attacker) simply by visiting his profile. Within several hours he had over 1 million friend requests, but it creates too much traffic on MySpace, so that the site goes offline. The following is a technical explanation of the worm. +CSS Injection is explained best by the well-known [MySpace Samy worm](http://namb.la/popular/tech.html). This worm automatically sent a friend request to Samy (the attacker) simply by visiting his profile. Within several hours he had over 1 million friend requests, which created so much traffic that MySpace went offline. The following is a technical explanation of that worm. -MySpace blocks many tags, however it allows CSS. So the worm's author put JavaScript into CSS like this: +MySpace blocked many tags, but allowed CSS. So the worm's author put JavaScript into CSS like this: ```html <div style="background:url('javascript:alert(1)')"> @@ -825,7 +823,7 @@ The next problem was MySpace filtering the word "javascript", so the author used <div id="mycode" expr="alert('hah!')" style="background:url('java↵
script:eval(document.all.mycode.expr)')"> ``` -Another problem for the worm's author were CSRF security tokens. Without them he couldn't send a friend request over POST. He got around it by sending a GET to the page right before adding a user and parsing the result for the CSRF token. +Another problem for the worm's author was the [CSRF security tokens](#cross-site-request-forgery-csrf). Without them he couldn't send a friend request over POST. He got around it by sending a GET to the page right before adding a user and parsing the result for the CSRF token. In the end, he got a 4 KB worm, which he injected into his profile page. @@ -1059,4 +1057,3 @@ The security landscape shifts and it is important to keep up to date, because mi * Subscribe to the Rails security [mailing list](http://groups.google.com/group/rubyonrails-security) * [Keep up to date on the other application layers](http://secunia.com/) (they have a weekly newsletter, too) * A [good security blog](https://www.owasp.org) including the [Cross-Site scripting Cheat Sheet](https://www.owasp.org/index.php/DOM_based_XSS_Prevention_Cheat_Sheet) - diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 52464a1c51..490bda3571 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -55,23 +55,26 @@ Upgrading from Rails 4.2 to Rails 5.0 ### Halting callback chains by returning `false` -In Rails 4.2, when a 'before' callback returns `false` in ActiveRecord, -ActiveModel and ActiveModel::Validations, then the entire callback chain -is halted. In other words, successive 'before' callbacks are not executed, -and neither is the action wrapped in callbacks. +In Rails 4.2, when a 'before' callback returns `false` in Active Record +and Active Model, then the entire callback chain is halted. In other words, +successive 'before' callbacks are not executed, and neither is the action wrapped +in callbacks. -In Rails 5.0, returning `false` in a callback will not have this side effect -of halting the callback chain. Instead, callback chains must be explicitly -halted by calling `throw(:abort)`. +In Rails 5.0, returning `false` in an Active Record or Active Model callback +will not have this side effect of halting the callback chain. Instead, callback +chains must be explicitly halted by calling `throw(:abort)`. -When you upgrade from Rails 4.2 to Rails 5.0, returning `false` in a callback -will still halt the callback chain, but you will receive a deprecation warning -about this upcoming change. +When you upgrade from Rails 4.2 to Rails 5.0, returning `false` in those kind of +callbacks will still halt the callback chain, but you will receive a deprecation +warning about this upcoming change. When you are ready, you can opt into the new behavior and remove the deprecation warning by adding the following configuration to your `config/application.rb`: - config.active_support.halt_callback_chains_on_return_false = false + ActiveSupport.halt_callback_chains_on_return_false = false + +Note that this option will not affect Active Support callbacks since they never +halted the chain when any value was returned. See [#17227](https://github.com/rails/rails/pull/17227) for more details. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index f007d11c72..9337dd6407 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,71 @@ +* Added javascript to update the URL on mailer previews with the currently + selected email format. Reloading the page now keeps you on your selected + format rather than going back to the default html version. + + *James Kerr* + +* Add fail fast to `bin/rails test` + + Adding `--fail-fast` or `-f` when running tests will interrupt the run on + the first failure: + + ``` + # Running: + + ................................................S......E + + ArgumentError: Wups! Bet you didn't expect this! + test/models/bunny_test.rb:19:in `block in <class:BunnyTest>' + + bin/rails test test/models/bunny_test.rb:18 + + ....................................F + + This failed + + bin/rails test test/models/bunny_test.rb:14 + + Interrupted. Exiting... + + + Finished in 0.051427s, 1808.3872 runs/s, 1769.4972 assertions/s. + + ``` + + Note that any unexpected errors don't abort the run. + + *Kasper Timm Hansen* + +* Add inline output to `bin/rails test` + + Any failures or errors (and skips if running in verbose mode) are output + during a test run: + + ``` + # Running: + + .....S..........................................F + + This failed + + bin/rails test test/models/bunny_test.rb:14 + + .................................E + + ArgumentError: Wups! Bet you didn't expect this! + test/models/bunny_test.rb:19:in `block in <class:BunnyTest>' + + bin/rails test test/models/bunny_test.rb:18 + + .................... + + Finished in 0.069708s, 1477.6019 runs/s, 1448.9106 assertions/s. + ``` + + Output can be deferred to after a run with the `--defer-output` option. + + *Kasper Timm Hansen* + * Fix displaying mailer previews on non local requests when config `action_mailer.show_previews` is set @@ -234,10 +302,11 @@ Newly generated Rails apps have a new initializer called `callback_terminator.rb` which sets the value of the configuration option - `config.active_support.halt_callback_chains_on_return_false` to `false`. + `ActiveSupport.halt_callback_chains_on_return_false` to `false`. - As a result, new Rails apps do not halt callback chains when a callback - returns `false`; only when they are explicitly halted with `throw(:abort)`. + As a result, new Rails apps do not halt Active Record and Active Model + callback chains when a callback returns `false`; only when they are + explicitly halted with `throw(:abort)`. The terminator is *not* added when running `rake rails:update`, so returning `false` will still work on old apps ported to Rails 5, displaying a diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 8075068b3f..7916e24af1 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -156,21 +156,12 @@ module Rails self end - # Implements call according to the Rack API. It simply - # dispatches the request to the underlying middleware stack. - def call(env) - req = ActionDispatch::Request.new env - env["ORIGINAL_FULLPATH"] = req.fullpath - env["ORIGINAL_SCRIPT_NAME"] = req.script_name - super(env) - end - # Reload application routes regardless if they changed or not. def reload_routes! routes_reloader.reload! end - # Return the application's KeyGenerator + # Returns the application's KeyGenerator def key_generator # number of iterations selected based on consultation with the google security # team. Details at https://github.com/rails/rails/pull/6952#issuecomment-7661220 @@ -514,5 +505,18 @@ module Rails end end end + + private + + def build_request(env) + req = super + env["ORIGINAL_FULLPATH"] = req.fullpath + env["ORIGINAL_SCRIPT_NAME"] = req.script_name + req + end + + def build_middleware + config.app_middleware + super + end end end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 4fc7a1db62..75112f29b6 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -36,7 +36,6 @@ module Rails @time_zone = "UTC" @beginning_of_week = :monday @log_level = nil - @middleware = app_middleware @generators = app_generators @cache_store = [ :file_store, "#{root}/tmp/cache/" ] @railties_order = [:all] diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index d99d27a756..30eafd59f2 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -33,9 +33,9 @@ module Rails # config.middleware.delete ActionDispatch::Flash # class MiddlewareStackProxy - def initialize - @operations = [] - @delete_operations = [] + def initialize(operations = [], delete_operations = []) + @operations = operations + @delete_operations = delete_operations end def insert_before(*args, &block) @@ -71,6 +71,19 @@ module Rails other end + + def +(other) # :nodoc: + MiddlewareStackProxy.new(@operations + other.operations, @delete_operations + other.delete_operations) + end + + protected + def operations + @operations + end + + def delete_operations + @delete_operations + end end class Generators #:nodoc: diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index e90d41cbec..5757d235d2 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -2,6 +2,7 @@ require 'rails/railtie' require 'rails/engine/railties' require 'active_support/core_ext/module/delegation' require 'pathname' +require 'thread' module Rails # <tt>Rails::Engine</tt> allows you to wrap a specific Rails application or subset of @@ -357,12 +358,7 @@ module Rails Rails::Railtie::Configuration.eager_load_namespaces << base base.called_from = begin - call_stack = if Kernel.respond_to?(:caller_locations) - caller_locations.map { |l| l.absolute_path || l.path } - else - # Remove the line number from backtraces making sure we don't leave anything behind - caller.map { |p| p.sub(/:\d+.*/, '') } - end + call_stack = caller_locations.map { |l| l.absolute_path || l.path } File.dirname(call_stack.detect { |p| p !~ %r[railties[\w.-]*/lib/rails|rack[\w.-]*/lib/rack] }) end @@ -434,6 +430,7 @@ module Rails @env_config = nil @helpers = nil @routes = nil + @app_build_lock = Mutex.new super end @@ -504,10 +501,13 @@ module Rails # Returns the underlying rack application for this engine. def app - @app ||= begin - config.middleware = config.middleware.merge_into(default_middleware_stack) - config.middleware.build(endpoint) - end + @app || @app_build_lock.synchronize { + @app ||= begin + stack = default_middleware_stack + config.middleware = build_middleware.merge_into(stack) + config.middleware.build(endpoint) + end + } end # Returns the endpoint for this engine. If none is registered, @@ -518,11 +518,8 @@ module Rails # Define the Rack API for this engine. def call(env) - env.merge!(env_config) - req = ActionDispatch::Request.new env - req.routes = routes - req.engine_script_name = req.script_name - app.call(env) + req = build_request env + app.call req.env end # Defines additional Rack env configuration that is added on each call. @@ -689,5 +686,19 @@ module Rails def _all_load_paths #:nodoc: @_all_load_paths ||= (config.paths.load_paths + _all_autoload_paths).uniq end + + private + + def build_request(env) + env.merge!(env_config) + req = ActionDispatch::Request.new env + req.routes = routes + req.engine_script_name = req.script_name + req + end + + def build_middleware + config.middleware + end end end diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index 62a4139d07..b4ddee3b1b 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -4,17 +4,14 @@ module Rails class Engine class Configuration < ::Rails::Railtie::Configuration attr_reader :root - attr_writer :middleware, :eager_load_paths, :autoload_once_paths, :autoload_paths + attr_accessor :middleware + attr_writer :eager_load_paths, :autoload_once_paths, :autoload_paths def initialize(root=nil) super() @root = root @generators = app_generators.dup - end - - # Returns the middleware stack for the engine. - def middleware - @middleware ||= Rails::Configuration::MiddlewareStackProxy.new + @middleware = Rails::Configuration::MiddlewareStackProxy.new end # Holds generators configuration: diff --git a/railties/lib/rails/generators/base.rb b/railties/lib/rails/generators/base.rb index 6fa413f8b0..c72ec400a0 100644 --- a/railties/lib/rails/generators/base.rb +++ b/railties/lib/rails/generators/base.rb @@ -302,13 +302,13 @@ module Rails default_for_option(Rails::Generators.options, name, options, options[:default]) end - # Return default aliases for the option name given doing a lookup in + # Returns default aliases for the option name given doing a lookup in # Rails::Generators.aliases. def self.default_aliases_for_option(name, options) default_for_option(Rails::Generators.aliases, name, options, options[:aliases]) end - # Return default for the option name given doing a lookup in config. + # Returns default for the option name given doing a lookup in config. def self.default_for_option(config, name, options, default) if generator_name and c = config[generator_name.to_sym] and c.key?(name) c[name] diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/config/manifest.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/config/manifest.js.tt new file mode 100644 index 0000000000..f80631bac6 --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/config/manifest.js.tt @@ -0,0 +1,8 @@ + +<% unless options.api? -%> +//= link_tree ../images +<% end -%> +<% unless options.skip_javascript -%> +//= link_directory ../javascripts .js +<% end -%> +//= link_directory ../stylesheets .css diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/manifest.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/manifest.js.tt deleted file mode 100644 index 3325553f57..0000000000 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/manifest.js.tt +++ /dev/null @@ -1,8 +0,0 @@ - -<% unless options.api? -%> -//= link_tree ./images -<% end -%> -<% unless options.skip_javascript -%> -//= link ./javascripts/application.js -<% end -%> -//= link ./stylesheets/application.css diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/cors.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/cors.rb index 45c44d24f8..9fca213a04 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/cors.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/cors.rb @@ -3,7 +3,7 @@ # Read more: https://github.com/cyu/rack-cors -# Rails.application.config.middleware.insert_before 0, "Rack::Cors" do +# Rails.application.config.middleware.insert_before 0, Rack::Cors do # allow do # origins 'example.com' # diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 8fea30189e..910c4e743e 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -106,7 +106,7 @@ task default: :test def test_dummy_assets template "rails/javascripts.js", "#{dummy_path}/app/assets/javascripts/application.js", force: true template "rails/stylesheets.css", "#{dummy_path}/app/assets/stylesheets/application.css", force: true - template "rails/dummy_manifest.js", "#{dummy_path}/app/assets/manifest.js", force: true + template "rails/dummy_manifest.js", "#{dummy_path}/app/assets/config/manifest.js", force: true end def test_dummy_clean @@ -124,7 +124,7 @@ task default: :test end def assets_manifest - template "rails/engine_manifest.js", "app/assets/#{underscored_name}_manifest.js" + template "rails/engine_manifest.js", "app/assets/config/#{underscored_name}_manifest.js" end def stylesheets diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js b/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js index ace695a811..8d21b2b6fb 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js @@ -1,11 +1,11 @@ <% unless api? -%> -//= link_tree ./images +//= link_tree ../images <% end -%> <% unless options.skip_javascript -%> -//= link ./javascripts/application.js +//= link_directory ../javascripts .js <% end -%> -//= link ./stylesheets/application.css +//= link_directory ../stylesheets .css <% if mountable? && !api? -%> //= link <%= underscored_name %>_manifest.js <% end -%> diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js b/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js index f8ef26982a..2f23844f5e 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js @@ -1,6 +1,6 @@ <% if mountable? -%> -<% unless options.skip_javascript -%> -//= link ./javascripts/<%= namespaced_name %>/application.js +<% if !options.skip_javascript -%> +//= link_directory ../javascripts/<%= namespaced_name %> .js <% end -%> -//= link ./stylesheets/<%= namespaced_name %>/application.css +//= link_directory ../stylesheets/<%= namespaced_name %> .css <% end -%> diff --git a/railties/lib/rails/generators/testing/assertions.rb b/railties/lib/rails/generators/testing/assertions.rb index 17af6eddfa..76758df86d 100644 --- a/railties/lib/rails/generators/testing/assertions.rb +++ b/railties/lib/rails/generators/testing/assertions.rb @@ -1,5 +1,3 @@ -require 'shellwords' - module Rails module Generators module Testing diff --git a/railties/lib/rails/generators/testing/behaviour.rb b/railties/lib/rails/generators/testing/behaviour.rb index c9700e1cd7..94b5e52224 100644 --- a/railties/lib/rails/generators/testing/behaviour.rb +++ b/railties/lib/rails/generators/testing/behaviour.rb @@ -92,7 +92,8 @@ module Rails cd current_path end - def prepare_destination # :nodoc: + # Clears all files and directories in destination. + def prepare_destination rm_rf(destination_root) mkdir_p(destination_root) end diff --git a/railties/lib/rails/mailers_controller.rb b/railties/lib/rails/mailers_controller.rb index 38004fe99a..6143cf2dd9 100644 --- a/railties/lib/rails/mailers_controller.rb +++ b/railties/lib/rails/mailers_controller.rb @@ -31,7 +31,7 @@ class Rails::MailersController < Rails::ApplicationController # :nodoc: raise AbstractController::ActionNotFound, "Email part '#{part_type}' not found in #{@preview.name}##{@email_action}" end else - @part = find_preferred_part(request.format, Mime::Type[:HTML], Mime::Type[:TEXT]) + @part = find_preferred_part(request.format, Mime[:html], Mime[:text]) render action: 'email', layout: false, formats: %w[html] end else diff --git a/railties/lib/rails/templates/rails/mailers/email.html.erb b/railties/lib/rails/templates/rails/mailers/email.html.erb index cf95cd8ad0..fed96fbc85 100644 --- a/railties/lib/rails/templates/rails/mailers/email.html.erb +++ b/railties/lib/rails/templates/rails/mailers/email.html.erb @@ -94,9 +94,9 @@ <% if @email.multipart? %> <dd> - <select onchange="document.getElementsByName('messageBody')[0].src=this.options[this.selectedIndex].value;"> - <option <%= request.format == Mime::Type[:HTML] ? 'selected' : '' %> value="?part=text%2Fhtml">View as HTML email</option> - <option <%= request.format == Mime::Type[:TEXT] ? 'selected' : '' %> value="?part=text%2Fplain">View as plain-text email</option> + <select onchange="formatChanged(this);"> + <option <%= request.format == Mime[:html] ? 'selected' : '' %> value="?part=text%2Fhtml">View as HTML email</option> + <option <%= request.format == Mime[:text] ? 'selected' : '' %> value="?part=text%2Fplain">View as plain-text email</option> </select> </dd> <% end %> @@ -112,5 +112,19 @@ </p> <% end %> +<script> + function formatChanged(form) { + var part_name = form.options[form.selectedIndex].value + var iframe =document.getElementsByName('messageBody')[0]; + iframe.contentWindow.location.replace(part_name); + + if (history.replaceState) { + var url = location.pathname.replace(/\.(txt|html)$/, ''); + var format = /html/.test(part_name) ? '.html' : '.txt'; + window.history.replaceState({}, '', url + format); + } + } +</script> + </body> </html> diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index dacab08ec3..d1ba35a5ec 100644 --- a/railties/lib/rails/test_unit/minitest_plugin.rb +++ b/railties/lib/rails/test_unit/minitest_plugin.rb @@ -3,6 +3,16 @@ require "rails/test_unit/reporter" require "rails/test_unit/test_requirer" module Minitest + mattr_accessor(:hide_aggregated_results) { false } + + module AggregatedResultSuppresion + def aggregated_results + super unless Minitest.hide_aggregated_results + end + end + + SummaryReporter.prepend AggregatedResultSuppresion + def self.plugin_rails_options(opts, options) opts.separator "" opts.separator "Usage: bin/rails test [options] [files or directories]" @@ -14,6 +24,8 @@ module Minitest opts.separator "" opts.separator " bin/rails test test/controllers test/integration/login_test.rb" opts.separator "" + opts.separator "By default test failures and errors are reported inline during a run." + opts.separator "" opts.separator "Rails options:" opts.on("-e", "--environment ENV", @@ -26,6 +38,17 @@ module Minitest options[:full_backtrace] = true end + opts.on("-d", "--defer-output", + "Output test failures and errors after the test run") do + options[:output_inline] = false + end + + opts.on("-f", "--fail-fast", + "Abort test run on first failure") do + options[:fail_fast] = true + end + + options[:output_inline] = true options[:patterns] = opts.order! end @@ -42,7 +65,8 @@ module Minitest ENV["RAILS_ENV"] = options[:environment] || "test" unless run_with_autorun - ::Rails::TestRequirer.require_files @rake_patterns || options[:patterns] + patterns = defined?(@rake_patterns) ? @rake_patterns : options[:patterns] + ::Rails::TestRequirer.require_files(patterns) end unless options[:full_backtrace] || ENV["BACKTRACE"] @@ -50,6 +74,9 @@ module Minitest Minitest.backtrace_filter = ::Rails.backtrace_cleaner if ::Rails.respond_to?(:backtrace_cleaner) end + # Disable the extra failure output after a run, unless output is deferred. + self.hide_aggregated_results = options[:output_inline] + self.reporter << ::Rails::TestUnitReporter.new(options[:io], options) end diff --git a/railties/lib/rails/test_unit/reporter.rb b/railties/lib/rails/test_unit/reporter.rb index 09b8675cf8..e1fe92a11b 100644 --- a/railties/lib/rails/test_unit/reporter.rb +++ b/railties/lib/rails/test_unit/reporter.rb @@ -6,8 +6,25 @@ module Rails class_attribute :executable self.executable = "bin/rails test" + def record(result) + super + + if output_inline? && result.failure && (!result.skipped? || options[:verbose]) + io.puts + io.puts + io.puts result.failures.map(&:message) + io.puts + io.puts format_rerun_snippet(result) + io.puts + end + + if fail_fast? && result.failure && !result.error? && !result.skipped? + raise Interrupt + end + end + def report - return if filtered_results.empty? + return if output_inline? || filtered_results.empty? io.puts io.puts "Failed tests:" io.puts @@ -15,10 +32,7 @@ module Rails end def aggregated_results # :nodoc: - filtered_results.map do |result| - location, line = result.method(result.name).source_location - "#{self.executable} #{relative_path_for(location)}:#{line}" - end.join "\n" + filtered_results.map { |result| format_rerun_snippet(result) }.join "\n" end def filtered_results @@ -32,5 +46,19 @@ module Rails def relative_path_for(file) file.sub(/^#{Rails.root}\/?/, '') end + + private + def output_inline? + options[:output_inline] + end + + def fail_fast? + options[:fail_fast] + end + + def format_rerun_snippet(result) + location, line = result.method(result.name).source_location + "#{self.executable} #{relative_path_for(location)}:#{line}" + end end end diff --git a/railties/test/application/asset_debugging_test.rb b/railties/test/application/asset_debugging_test.rb index fef65adda2..8b83784ed6 100644 --- a/railties/test/application/asset_debugging_test.rb +++ b/railties/test/application/asset_debugging_test.rb @@ -7,7 +7,10 @@ module ApplicationTests include Rack::Test::Methods def setup - build_app(initializers: true) + # FIXME: shush Sass warning spam, not relevant to testing Railties + Kernel.silence_warnings do + build_app(initializers: true) + end app_file "app/assets/javascripts/application.js", "//= require_tree ." app_file "app/assets/javascripts/xmlhr.js", "function f1() { alert(); }" @@ -33,12 +36,19 @@ module ApplicationTests teardown_app end + # FIXME: shush Sass warning spam, not relevant to testing Railties + def get(*) + Kernel.silence_warnings { super } + end + test "assets are concatenated when debug is off and compile is off either if debug_assets param is provided" do # config.assets.debug and config.assets.compile are false for production environment ENV["RAILS_ENV"] = "production" output = Dir.chdir(app_path){ `bin/rake assets:precompile --trace 2>&1` } assert $?.success?, output - require "#{app_path}/config/environment" + + # Load app env + app "production" class ::PostsController < ActionController::Base ; end @@ -51,8 +61,8 @@ module ApplicationTests test "assets are served with sourcemaps when compile is true and debug_assets params is true" do add_to_env_config "production", "config.assets.compile = true" - ENV["RAILS_ENV"] = "production" - require "#{app_path}/config/environment" + # Load app env + app "production" class ::PostsController < ActionController::Base ; end diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index ddcd51f3fc..dca5cf2e5b 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -17,14 +17,23 @@ module ApplicationTests end def precompile!(env = nil) - quietly do - precompile_task = "bin/rake assets:precompile #{env} --trace 2>&1" - output = Dir.chdir(app_path) { %x[ #{precompile_task} ] } - assert $?.success?, output - output + with_env env.to_h do + quietly do + precompile_task = "bin/rake assets:precompile --trace 2>&1" + output = Dir.chdir(app_path) { %x[ #{precompile_task} ] } + assert $?.success?, output + output + end end end + def with_env(env) + env.each { |k, v| ENV[k.to_s] = v } + yield + ensure + env.each_key { |k| ENV.delete k.to_s } + end + def clean_assets! quietly do assert Dir.chdir(app_path) { system('bin/rake assets:clobber') } @@ -32,7 +41,8 @@ module ApplicationTests end def assert_file_exists(filename) - assert Dir[filename].first, "missing #{filename}" + globbed = Dir[filename] + assert globbed.one?, "Found #{globbed.size} files matching #{filename}. All files in the directory: #{Dir.entries(File.dirname(filename)).inspect}" end def assert_no_file_exists(filename) @@ -51,7 +61,10 @@ module ApplicationTests add_to_env_config "development", "config.assets.digest = false" - require "#{app_path}/config/environment" + # FIXME: shush Sass warning spam, not relevant to testing Railties + Kernel.silence_warnings do + require "#{app_path}/config/environment" + end get "/assets/demo.js" assert_equal 'a = "/assets/rails.png";', last_response.body.strip @@ -62,8 +75,8 @@ module ApplicationTests add_to_env_config "production", "config.assets.compile = true" add_to_env_config "production", "config.assets.precompile = []" - ENV["RAILS_ENV"] = "production" - require "#{app_path}/config/environment" + # Load app env + app "production" assert !defined?(Uglifier) get "/assets/demo.js" @@ -72,11 +85,10 @@ module ApplicationTests end test "precompile creates the file, gives it the original asset's content and run in production as default" do - app_file "app/assets/manifest.js", "//= link_tree ./javascripts" + app_file "app/assets/config/manifest.js", "//= link_tree ../javascripts" app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/foo/application.js", "alert();" - ENV["RAILS_ENV"] = nil precompile! files = Dir["#{app_path}/public/assets/application-*.js"] @@ -88,7 +100,7 @@ module ApplicationTests end def test_precompile_does_not_hit_the_database - app_file "app/assets/manifest.js", "//= link_tree ./javascripts" + app_file "app/assets/config/manifest.js", "//= link_tree ../javascripts" app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/foo/application.js", "alert();" app_file "app/controllers/users_controller.rb", <<-eoruby @@ -98,10 +110,9 @@ module ApplicationTests class User < ActiveRecord::Base; raise 'should not be reached'; end eoruby - ENV['RAILS_ENV'] = 'production' - ENV['DATABASE_URL'] = 'postgresql://baduser:badpass@127.0.0.1/dbname' - - precompile! + precompile! \ + RAILS_ENV: 'production', + DATABASE_URL: 'postgresql://baduser:badpass@127.0.0.1/dbname' files = Dir["#{app_path}/public/assets/application-*.js"] files << Dir["#{app_path}/public/assets/foo/application-*.js"].first @@ -109,9 +120,6 @@ module ApplicationTests assert_not_nil file, "Expected application.js asset to be generated, but none found" assert_equal "alert();".strip, File.read(file).strip end - ensure - ENV.delete 'RAILS_ENV' - ENV.delete 'DATABASE_URL' end test "precompile application.js and application.css and all other non JS/CSS files" do @@ -171,10 +179,9 @@ module ApplicationTests test 'precompile use assets defined in app env config' do add_to_env_config 'production', 'config.assets.precompile = [ "something.js" ]' - app_file 'app/assets/javascripts/something.js.erb', 'alert();' - precompile! 'RAILS_ENV=production' + precompile! RAILS_ENV: 'production' assert_file_exists("#{app_path}/public/assets/something-*.js") end @@ -183,15 +190,17 @@ module ApplicationTests add_to_config 'config.assets.precompile = [ "something_manifest.js" ]' add_to_env_config 'production', 'config.assets.precompile += [ "another_manifest.js" ]' - app_file 'app/assets/something_manifest.js', '//= link ./javascripts/something.js' - app_file 'app/assets/another_manifest.js', '//= link ./javascripts/another.js' + app_file 'app/assets/config/something_manifest.js', '//= link something.js' + app_file 'app/assets/config/another_manifest.js', '//= link another.js' app_file 'app/assets/javascripts/something.js.erb', 'alert();' app_file 'app/assets/javascripts/another.js.erb', 'alert();' - precompile! 'RAILS_ENV=production' + precompile! RAILS_ENV: 'production' + assert_file_exists("#{app_path}/public/assets/something_manifest-*.js") assert_file_exists("#{app_path}/public/assets/something-*.js") + assert_file_exists("#{app_path}/public/assets/another_manifest-*.js") assert_file_exists("#{app_path}/public/assets/another-*.js") end @@ -199,8 +208,8 @@ module ApplicationTests add_to_config "config.action_controller.perform_caching = false" add_to_env_config "production", "config.assets.compile = true" - ENV["RAILS_ENV"] = "production" - require "#{app_path}/config/environment" + # Load app env + app "production" assert_equal Sprockets::CachedEnvironment, Rails.application.assets.class end @@ -211,8 +220,8 @@ module ApplicationTests app_file "app/assets/javascripts/application.js", "alert();" precompile! - manifest = Dir["#{app_path}/public/assets/.sprockets-manifest-*.json"].first + manifest = Dir["#{app_path}/public/assets/.sprockets-manifest-*.json"].first assets = ActiveSupport::JSON.decode(File.read(manifest)) assert_match(/application-([0-z]+)\.js/, assets["assets"]["application.js"]) assert_match(/application-([0-z]+)\.css/, assets["assets"]["application.css"]) @@ -233,14 +242,14 @@ module ApplicationTests app_file "app/assets/javascripts/application.js", "alert();" add_to_env_config "production", "config.serve_static_files = true" - ENV["RAILS_ENV"] = "production" - precompile! + precompile! RAILS_ENV: 'production' manifest = Dir["#{app_path}/public/assets/.sprockets-manifest-*.json"].first assets = ActiveSupport::JSON.decode(File.read(manifest)) asset_path = assets["assets"]["application.js"] - require "#{app_path}/config/environment" + # Load app env + app "production" # Checking if Uglifier is defined we can know if Sprockets was reached or not assert !defined?(Uglifier) @@ -249,12 +258,11 @@ module ApplicationTests assert !defined?(Uglifier) end - test "precompile properly refers files referenced with asset_path and runs in the provided RAILS_ENV" do + test "precompile properly refers files referenced with asset_path" do app_file "app/assets/images/rails.png", "notactuallyapng" app_file "app/assets/stylesheets/application.css.erb", "p { background-image: url(<%= asset_path('rails.png') %>) }" - add_to_env_config "test", "config.assets.digest = true" - precompile!('RAILS_ENV=test') + precompile! file = Dir["#{app_path}/public/assets/application-*.css"].first assert_match(/\/assets\/rails-([0-z]+)\.png/, File.read(file)) @@ -265,8 +273,7 @@ module ApplicationTests app_file "app/assets/stylesheets/application.css.erb", "p { background-image: url(<%= asset_path('rails.png') %>) }" - ENV["RAILS_ENV"] = "production" - precompile! + precompile! RAILS_ENV: 'production' manifest = Dir["#{app_path}/public/assets/.sprockets-manifest-*.json"].first assets = ActiveSupport::JSON.decode(File.read(manifest)) @@ -275,8 +282,8 @@ module ApplicationTests app_file "app/assets/images/rails.png", "p { url: change }" precompile! - assets = ActiveSupport::JSON.decode(File.read(manifest)) + assets = ActiveSupport::JSON.decode(File.read(manifest)) assert_not_equal asset_path, assets["assets"]["application.css"] end @@ -284,8 +291,7 @@ module ApplicationTests app_file "app/assets/images/rails.png", "notactuallyapng" app_file "app/assets/stylesheets/application.css.erb", "p { background-image: url(<%= asset_path('rails.png') %>) }" - ENV["RAILS_ENV"] = "production" - precompile! + precompile! RAILS_ENV: 'production' file = Dir["#{app_path}/public/assets/application-*.css"].first assert_match(/\/assets\/rails-([0-z]+)\.png/, File.read(file)) @@ -294,7 +300,7 @@ module ApplicationTests test "precompile should handle utf8 filenames" do filename = "レイルズ.png" app_file "app/assets/images/#{filename}", "not an image really" - app_file "app/assets/manifest.js", "//= link_tree ./images" + app_file "app/assets/config/manifest.js", "//= link_tree ../images" add_to_config "config.assets.precompile = %w(manifest.js)" precompile! @@ -303,7 +309,8 @@ module ApplicationTests assets = ActiveSupport::JSON.decode(File.read(manifest)) assert asset_path = assets["assets"].find { |(k, _)| k && k =~ /.png/ }[1] - require "#{app_path}/config/environment" + # Load app env + app "development" get "/assets/#{URI.parser.escape(asset_path)}" assert_match "not an image really", last_response.body @@ -326,8 +333,8 @@ module ApplicationTests app_file "app/assets/javascripts/demo.js.erb", "<%= :alert %>();" add_to_config "config.assets.compile = false" - ENV["RAILS_ENV"] = "production" - require "#{app_path}/config/environment" + # Load app env + app "production" get "/assets/demo.js" assert_equal 404, last_response.status @@ -344,7 +351,8 @@ module ApplicationTests add_to_env_config "development", "config.assets.digest = false" - require "#{app_path}/config/environment" + # Load app env + app "development" class ::OmgController < ActionController::Base def index @@ -370,7 +378,8 @@ module ApplicationTests add_to_env_config "development", "config.assets.digest = false" - require "#{app_path}/config/environment" + # Load app env + app "development" get "/assets/demo.js" assert_match "alert();", last_response.body @@ -381,10 +390,10 @@ module ApplicationTests app_with_assets_in_view # config.assets.debug and config.assets.compile are false for production environment - ENV["RAILS_ENV"] = "production" - precompile! + precompile! RAILS_ENV: 'production' - require "#{app_path}/config/environment" + # Load app env + app "production" class ::PostsController < ActionController::Base ; end @@ -400,6 +409,7 @@ module ApplicationTests app_file "app/assets/javascripts/xmlhr.js.erb", "<%= Post.name %>" precompile! + assert_equal "Post\n;\n", File.read(Dir["#{app_path}/public/assets/application-*.js"].first) end @@ -441,7 +451,10 @@ module ApplicationTests app_with_assets_in_view add_to_config "config.asset_host = 'example.com'" add_to_env_config "development", "config.assets.digest = false" - require "#{app_path}/config/environment" + + # Load app env + app "development" + class ::PostsController < ActionController::Base; end get '/posts', {}, {'HTTPS'=>'off'} @@ -456,6 +469,7 @@ module ApplicationTests add_to_config "config.assets.precompile = %w{rails.png image_loader.js}" add_to_config "config.asset_host = 'example.com'" add_to_env_config "development", "config.assets.digest = false" + precompile! assert_match "src='//example.com/assets/rails.png'", File.read(Dir["#{app_path}/public/assets/image_loader-*.js"].first) @@ -467,6 +481,7 @@ module ApplicationTests app_file "app/assets/javascripts/app.js.erb", "var src='<%= image_path('rails.png') %>';" add_to_config "config.assets.precompile = %w{rails.png app.js}" add_to_env_config "development", "config.assets.digest = false" + precompile! assert_match "src='/sub/uri/assets/rails.png'", File.read(Dir["#{app_path}/public/assets/app-*.js"].first) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 464fc1688c..2f407cd851 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -34,8 +34,19 @@ module ApplicationTests FileUtils.cp_r(app_path, new_app) end - def app - @app ||= Rails.application + def app(env = 'development') + @app ||= begin + ENV['RAILS_ENV'] = env + + # FIXME: shush Sass warning spam, not relevant to testing Railties + Kernel.silence_warnings do + require "#{app_path}/config/environment" + end + + Rails.application + ensure + ENV.delete 'RAILS_ENV' + end end def setup @@ -78,7 +89,9 @@ module ApplicationTests require 'my_logger' config.logger = MyLogger.new STDOUT RUBY - require "#{app_path}/config/environment" + + app 'development' + assert_equal 'MyLogger', Rails.application.config.logger.class.name end @@ -97,7 +110,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' ActiveRecord::Migrator.migrations_paths = ["#{app_path}/db/migrate"] @@ -128,29 +141,29 @@ module ApplicationTests test "Rails.application is nil until app is initialized" do require 'rails' assert_nil Rails.application - require "#{app_path}/config/environment" + app 'development' assert_equal AppTemplate::Application.instance, Rails.application end test "Rails.application responds to all instance methods" do - require "#{app_path}/config/environment" + app 'development' assert_respond_to Rails.application, :routes_reloader assert_equal Rails.application.routes_reloader, AppTemplate::Application.routes_reloader end test "Rails::Application responds to paths" do - require "#{app_path}/config/environment" + app 'development' assert_respond_to AppTemplate::Application, :paths assert_equal ["#{app_path}/app/views"], AppTemplate::Application.paths["app/views"].expanded end test "the application root is set correctly" do - require "#{app_path}/config/environment" + app 'development' assert_equal Pathname.new(app_path), Rails.application.root end test "the application root can be seen from the application singleton" do - require "#{app_path}/config/environment" + app 'development' assert_equal Pathname.new(app_path), AppTemplate::Application.root end @@ -162,7 +175,8 @@ module ApplicationTests use_frameworks [] - require "#{app_path}/config/environment" + app 'development' + assert_equal Pathname.new(new_app), Rails.application.root end @@ -172,7 +186,7 @@ module ApplicationTests use_frameworks [] Dir.chdir("#{app_path}") do - require "#{app_path}/config/environment" + app 'development' assert_equal Pathname.new("#{app_path}"), Rails.application.root end end @@ -181,7 +195,9 @@ module ApplicationTests add_to_config <<-RUBY config.root = "#{app_path}" RUBY - require "#{app_path}/config/environment" + + app 'development' + assert_instance_of Pathname, Rails.root end @@ -189,7 +205,9 @@ module ApplicationTests add_to_config <<-RUBY config.paths["public"] = "somewhere" RUBY - require "#{app_path}/config/environment" + + app 'development' + assert_instance_of Pathname, Rails.public_path end @@ -199,12 +217,13 @@ module ApplicationTests config.cache_classes = true RUBY - require "#{app_path}/config/application" - assert Rails.application.initialize! + app 'development' + + assert_equal :require, ActiveSupport::Dependencies.mechanism end test "application is always added to eager_load namespaces" do - require "#{app_path}/config/application" + app 'development' assert_includes Rails.application.config.eager_load_namespaces, AppTemplate::Application end @@ -218,7 +237,7 @@ module ApplicationTests use_frameworks [] assert_nothing_raised do - require "#{app_path}/config/application" + app 'development' end end @@ -230,7 +249,7 @@ module ApplicationTests RUBY assert_nothing_raised do - require "#{app_path}/config/application" + app 'development' end end @@ -239,7 +258,7 @@ module ApplicationTests Rails.application.config.filter_parameters += [ :password, :foo, 'bar' ] RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal [:password, :foo, 'bar'], Rails.application.env_config['action_dispatch.parameter_filter'] end @@ -255,7 +274,7 @@ module ApplicationTests assert !$prepared - require "#{app_path}/config/environment" + app 'development' get "/" assert $prepared @@ -267,7 +286,7 @@ module ApplicationTests end test "skipping config.encoding still results in 'utf-8' as the default" do - require "#{app_path}/config/application" + app 'development' assert_utf8 end @@ -276,7 +295,7 @@ module ApplicationTests config.encoding = "utf-8" RUBY - require "#{app_path}/config/application" + app 'development' assert_utf8 end @@ -285,7 +304,7 @@ module ApplicationTests config.paths["public"] = "somewhere" RUBY - require "#{app_path}/config/application" + app 'development' assert_equal Pathname.new(app_path).join("somewhere"), Rails.public_path end @@ -293,7 +312,7 @@ module ApplicationTests restore_default_config with_rails_env "production" do - require "#{app_path}/config/environment" + app 'production' assert_not app.config.serve_static_files end end @@ -303,7 +322,7 @@ module ApplicationTests with_rails_env "production" do switch_env "RAILS_SERVE_STATIC_FILES", "1" do - require "#{app_path}/config/environment" + app 'production' assert app.config.serve_static_files end end @@ -314,7 +333,7 @@ module ApplicationTests with_rails_env "production" do switch_env "RAILS_SERVE_STATIC_FILES", " " do - require "#{app_path}/config/environment" + app 'production' assert_not app.config.serve_static_files end end @@ -363,8 +382,8 @@ module ApplicationTests development: secret_key_base: YAML - require "#{app_path}/config/environment" + app 'development' assert_equal app.env_config['action_dispatch.key_generator'], Rails.application.key_generator assert_equal app.env_config['action_dispatch.key_generator'].class, ActiveSupport::LegacyKeyGenerator @@ -380,7 +399,8 @@ module ApplicationTests development: secret_key_base: YAML - require "#{app_path}/config/environment" + + app 'development' assert_deprecated(/You didn't set `secret_key_base`./) do app.env_config @@ -395,7 +415,8 @@ module ApplicationTests development: secret_token: 3b7cd727ee24e8444053437c36cc66c3 YAML - require "#{app_path}/config/environment" + + app 'development' assert_equal '3b7cd727ee24e8444053437c36cc66c3', app.secrets.secret_token end @@ -426,7 +447,7 @@ module ApplicationTests secret_key_base: 3b7cd727ee24e8444053437c36cc66c3 YAML - require "#{app_path}/config/environment" + app 'development' assert_equal '3b7cd727ee24e8444053437c36cc66c3', app.secrets.secret_key_base end @@ -436,7 +457,7 @@ module ApplicationTests Rails.application.config.secret_key_base = "3b7cd727ee24e8444053437c36cc66c3" RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal '3b7cd727ee24e8444053437c36cc66c3', app.secrets.secret_key_base end @@ -449,7 +470,8 @@ module ApplicationTests secret_key_base: secret_token: YAML - require "#{app_path}/config/environment" + + app 'development' assert_equal 'b3c631c314c0bbca50c1b2843150fe33', app.secrets.secret_token assert_equal 'b3c631c314c0bbca50c1b2843150fe33', app.config.secret_token @@ -463,7 +485,8 @@ module ApplicationTests aws_secret_access_key: myamazonsecretaccesskey YAML - require "#{app_path}/config/environment" + app 'development' + assert_equal 'myamazonaccesskeyid', app.secrets.aws_access_key_id assert_equal 'myamazonsecretaccesskey', app.secrets.aws_secret_access_key end @@ -471,7 +494,8 @@ module ApplicationTests test "blank config/secrets.yml does not crash the loading process" do app_file 'config/secrets.yml', <<-YAML YAML - require "#{app_path}/config/environment" + + app 'development' assert_nil app.secrets.not_defined end @@ -484,7 +508,8 @@ module ApplicationTests development: secret_key_base: YAML - require "#{app_path}/config/environment" + + app 'development' assert_equal "iaminallyoursecretkeybase", app.secrets.secret_key_base end @@ -497,7 +522,8 @@ module ApplicationTests development: secret_key_base: YAML - require "#{app_path}/config/environment" + + app 'development' assert_equal 'b3c631c314c0bbca50c1b2843150fe33', app.config.secret_token assert_equal nil, app.secrets.secret_key_base @@ -512,7 +538,8 @@ module ApplicationTests development: secret_key_base: YAML - require "#{app_path}/config/environment" + + app 'development' assert_equal '', app.config.secret_token assert_equal nil, app.secrets.secret_key_base @@ -535,7 +562,6 @@ module ApplicationTests end test "default form builder specified as a string" do - app_file 'config/initializers/form_builder.rb', <<-RUBY class CustomFormBuilder < ActionView::Helpers::FormBuilder def text_field(attribute, *args) @@ -567,7 +593,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' get "/posts" assert_match(/label/, last_response.body) @@ -606,9 +632,9 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' - params = {authenticity_token: token} + params = { authenticity_token: token } get "/posts/1" assert_match(/patch/, last_response.body) @@ -659,9 +685,9 @@ module ApplicationTests config.action_mailer.interceptors = MyMailInterceptor RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [::MyMailInterceptor], ::Mail.send(:class_variable_get, "@@delivery_interceptors") @@ -672,9 +698,9 @@ module ApplicationTests config.action_mailer.interceptors = [MyMailInterceptor, "MyOtherMailInterceptor"] RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [::MyMailInterceptor, ::MyOtherMailInterceptor], ::Mail.send(:class_variable_get, "@@delivery_interceptors") @@ -685,9 +711,9 @@ module ApplicationTests config.action_mailer.preview_interceptors = MyPreviewMailInterceptor RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [ActionMailer::InlinePreviewInterceptor, ::MyPreviewMailInterceptor], ActionMailer::Base.preview_interceptors @@ -698,9 +724,9 @@ module ApplicationTests config.action_mailer.preview_interceptors = [MyPreviewMailInterceptor, "MyOtherPreviewMailInterceptor"] RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [ActionMailer::InlinePreviewInterceptor, MyPreviewMailInterceptor, MyOtherPreviewMailInterceptor], ActionMailer::Base.preview_interceptors @@ -711,9 +737,9 @@ module ApplicationTests ActionMailer::Base.preview_interceptors.delete(ActionMailer::InlinePreviewInterceptor) RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [], ActionMailer::Base.preview_interceptors @@ -724,9 +750,9 @@ module ApplicationTests config.action_mailer.observers = MyMailObserver RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [::MyMailObserver], ::Mail.send(:class_variable_get, "@@delivery_notification_observers") @@ -737,9 +763,9 @@ module ApplicationTests config.action_mailer.observers = [MyMailObserver, "MyOtherMailObserver"] RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal [::MyMailObserver, ::MyOtherMailObserver], ::Mail.send(:class_variable_get, "@@delivery_notification_observers") @@ -750,9 +776,9 @@ module ApplicationTests config.action_mailer.deliver_later_queue_name = 'test_default' RUBY - require "#{app_path}/config/environment" - require "mail" + app 'development' + require "mail" _ = ActionMailer::Base assert_equal 'test_default', ActionMailer::Base.send(:class_variable_get, "@@deliver_later_queue_name") @@ -764,7 +790,7 @@ module ApplicationTests config.time_zone = "Wellington" RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal "Wellington", Rails.application.config.time_zone end @@ -776,7 +802,7 @@ module ApplicationTests RUBY assert_raise(ArgumentError) do - require "#{app_path}/config/environment" + app 'development' end end @@ -786,7 +812,7 @@ module ApplicationTests config.beginning_of_week = :wednesday RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal :wednesday, Rails.application.config.beginning_of_week end @@ -798,13 +824,14 @@ module ApplicationTests RUBY assert_raise(ArgumentError) do - require "#{app_path}/config/environment" + app 'development' end end test "config.action_view.cache_template_loading with cache_classes default" do add_to_config "config.cache_classes = true" - require "#{app_path}/config/environment" + + app 'development' require 'action_view/base' assert_equal true, ActionView::Resolver.caching? @@ -812,7 +839,8 @@ module ApplicationTests test "config.action_view.cache_template_loading without cache_classes default" do add_to_config "config.cache_classes = false" - require "#{app_path}/config/environment" + + app 'development' require 'action_view/base' assert_equal false, ActionView::Resolver.caching? @@ -823,7 +851,8 @@ module ApplicationTests config.cache_classes = true config.action_view.cache_template_loading = false RUBY - require "#{app_path}/config/environment" + + app 'development' require 'action_view/base' assert_equal false, ActionView::Resolver.caching? @@ -834,7 +863,8 @@ module ApplicationTests config.cache_classes = false config.action_view.cache_template_loading = true RUBY - require "#{app_path}/config/environment" + + app 'development' require 'action_view/base' assert_equal true, ActionView::Resolver.caching? @@ -849,7 +879,7 @@ module ApplicationTests require 'action_view/railtie' require 'action_view/base' - require "#{app_path}/config/environment" + app 'development' assert_equal false, ActionView::Resolver.caching? end @@ -902,7 +932,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' post "/posts.json", '{ "title": "foo", "name": "bar" }', "CONTENT_TYPE" => "application/json" assert_equal '{"title"=>"foo"}', last_response.body @@ -924,7 +954,7 @@ module ApplicationTests config.action_controller.permit_all_parameters = true RUBY - require "#{app_path}/config/environment" + app 'development' post "/posts", {post: {"title" =>"zomg"}} assert_equal 'permitted', last_response.body @@ -946,7 +976,7 @@ module ApplicationTests config.action_controller.action_on_unpermitted_parameters = :raise RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal :raise, ActionController::Parameters.action_on_unpermitted_parameters @@ -955,7 +985,7 @@ module ApplicationTests end test "config.action_controller.always_permitted_parameters are: controller, action by default" do - require "#{app_path}/config/environment" + app 'development' assert_equal %w(controller action), ActionController::Parameters.always_permitted_parameters end @@ -963,7 +993,9 @@ module ApplicationTests add_to_config <<-RUBY config.action_controller.always_permitted_parameters = %w( controller action format ) RUBY - require "#{app_path}/config/environment" + + app 'development' + assert_equal %w( controller action format ), ActionController::Parameters.always_permitted_parameters end @@ -984,7 +1016,7 @@ module ApplicationTests config.action_controller.action_on_unpermitted_parameters = :raise RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal :raise, ActionController::Parameters.action_on_unpermitted_parameters @@ -993,25 +1025,19 @@ module ApplicationTests end test "config.action_controller.action_on_unpermitted_parameters is :log by default on development" do - ENV["RAILS_ENV"] = "development" - - require "#{app_path}/config/environment" + app 'development' assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters end test "config.action_controller.action_on_unpermitted_parameters is :log by default on test" do - ENV["RAILS_ENV"] = "test" - - require "#{app_path}/config/environment" + app 'test' assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters end test "config.action_controller.action_on_unpermitted_parameters is false by default on production" do - ENV["RAILS_ENV"] = "production" - - require "#{app_path}/config/environment" + app 'production' assert_equal false, ActionController::Parameters.action_on_unpermitted_parameters end @@ -1089,17 +1115,14 @@ module ApplicationTests test "config.active_record.dump_schema_after_migration is false on production" do build_app - ENV["RAILS_ENV"] = "production" - require "#{app_path}/config/environment" + app 'production' assert_not ActiveRecord::Base.dump_schema_after_migration end test "config.active_record.dump_schema_after_migration is true by default on development" do - ENV["RAILS_ENV"] = "development" - - require "#{app_path}/config/environment" + app 'development' assert ActiveRecord::Base.dump_schema_after_migration end @@ -1125,7 +1148,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' assert_not Rails.configuration.ran_block require 'rake' @@ -1147,7 +1170,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' assert_not Rails.configuration.ran_block Rails.application.load_generators @@ -1165,7 +1188,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' assert_not Rails.configuration.ran_block Rails.application.load_console @@ -1183,7 +1206,7 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' assert_not Rails.configuration.ran_block Rails.application.load_runner @@ -1199,14 +1222,14 @@ module ApplicationTests end RUBY - require "#{app_path}/config/environment" + app 'development' assert_kind_of Hash, Rails.application.config.database_configuration end test 'raises with proper error message if no database configuration found' do FileUtils.rm("#{app_path}/config/database.yml") - require "#{app_path}/config/environment" + app 'development' err = assert_raises RuntimeError do Rails.application.config.database_configuration end @@ -1214,25 +1237,23 @@ module ApplicationTests end test 'config.action_mailer.show_previews defaults to true in development' do - Rails.env = "development" - require "#{app_path}/config/environment" + app 'development' assert Rails.application.config.action_mailer.show_previews end test 'config.action_mailer.show_previews defaults to false in production' do - Rails.env = "production" - require "#{app_path}/config/environment" + app 'production' assert_equal false, Rails.application.config.action_mailer.show_previews end test 'config.action_mailer.show_previews can be set in the configuration file' do - Rails.env = "production" add_to_config <<-RUBY config.action_mailer.show_previews = true RUBY - require "#{app_path}/config/environment" + + app 'production' assert_equal true, Rails.application.config.action_mailer.show_previews end @@ -1247,7 +1268,7 @@ module ApplicationTests config.my_custom_config = config_for('custom') RUBY - require "#{app_path}/config/environment" + app 'development' assert_equal 'custom key', Rails.application.config.my_custom_config['key'] end @@ -1258,7 +1279,7 @@ module ApplicationTests RUBY exception = assert_raises(RuntimeError) do - require "#{app_path}/config/environment" + app 'development' end assert_equal "Could not load configuration. No such file - #{app_path}/config/custom.yml", exception.message @@ -1273,7 +1294,8 @@ module ApplicationTests add_to_config <<-RUBY config.my_custom_config = config_for('custom') RUBY - require "#{app_path}/config/environment" + + app 'development' assert_equal({}, Rails.application.config.my_custom_config) end @@ -1285,7 +1307,8 @@ module ApplicationTests add_to_config <<-RUBY config.my_custom_config = config_for('custom') RUBY - require "#{app_path}/config/environment" + + app 'development' assert_equal({}, Rails.application.config.my_custom_config) end @@ -1299,7 +1322,8 @@ module ApplicationTests add_to_config <<-RUBY config.my_custom_config = config_for('custom') RUBY - require "#{app_path}/config/environment" + + app 'development' assert_equal 'custom key', Rails.application.config.my_custom_config['key'] end @@ -1315,7 +1339,7 @@ module ApplicationTests RUBY exception = assert_raises(RuntimeError) do - require "#{app_path}/config/environment" + app 'development' end assert_match 'YAML syntax error occurred while parsing', exception.message diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index cbada6be97..0777714d35 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -21,6 +21,12 @@ module ApplicationTests assert_equal 200, last_response.status end + test "rails/info in development" do + app("development") + get "/rails/info" + assert_equal 302, last_response.status + end + test "rails/info/routes in development" do app("development") get "/rails/info/routes" @@ -63,6 +69,12 @@ module ApplicationTests assert_equal 404, last_response.status end + test "rails/info in production" do + app("production") + get "/rails/info" + assert_equal 404, last_response.status + end + test "rails/info/routes in production" do app("production") get "/rails/info/routes" diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 2d47a31826..0aa6ce2252 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -340,6 +340,27 @@ module ApplicationTests assert_match '0 runs, 0 assertions', run_test_command('') end + def test_output_inline_by_default + create_test_file :models, 'post', pass: false + + output = run_test_command('test/models/post_test.rb') + assert_match %r{Running:\n\nPostTest\nF\n\nwups!\n\nbin/rails test test/models/post_test.rb:4}, output + end + + def test_only_inline_failure_output + create_test_file :models, 'post', pass: false + + output = run_test_command('test/models/post_test.rb') + assert_match %r{Finished in.*\n\n1 runs, 1 assertions}, output + end + + def test_fail_fast + create_test_file :models, 'post', pass: false + + assert_match(/Interrupt/, + capture(:stderr) { run_test_command('test/models/post_test.rb --fail-fast') }) + end + def test_raise_error_when_specified_file_does_not_exist error = capture(:stderr) { run_test_command('test/not_exists.rb') } assert_match(%r{cannot load such file.+test/not_exists\.rb}, error) @@ -396,14 +417,14 @@ module ApplicationTests app_file 'db/schema.rb', '' end - def create_test_file(path = :unit, name = 'test') + def create_test_file(path = :unit, name = 'test', pass: true) app_file "test/#{path}/#{name}_test.rb", <<-RUBY require 'test_helper' class #{name.camelize}Test < ActiveSupport::TestCase def test_truth puts "#{name.camelize}Test" - assert true + assert #{pass}, 'wups!' end end RUBY diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 41d36680e0..df3c2ca66d 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -51,7 +51,12 @@ module TestHelpers old_env = ENV["RAILS_ENV"] @app ||= begin ENV["RAILS_ENV"] = env - require "#{app_path}/config/environment" + + # FIXME: shush Sass warning spam, not relevant to testing Railties + Kernel.silence_warnings do + require "#{app_path}/config/environment" + end + Rails.application end ensure @@ -161,18 +166,18 @@ module TestHelpers require "action_view/railtie" require 'action_dispatch/middleware/flash' - app = Class.new(Rails::Application) - app.config.eager_load = false - app.secrets.secret_key_base = "3b7cd727ee24e8444053437c36cc66c4" - app.config.session_store :cookie_store, key: "_myapp_session" - app.config.active_support.deprecation = :log - app.config.active_support.test_order = :random - app.config.log_level = :info + @app = Class.new(Rails::Application) + @app.config.eager_load = false + @app.secrets.secret_key_base = "3b7cd727ee24e8444053437c36cc66c4" + @app.config.session_store :cookie_store, key: "_myapp_session" + @app.config.active_support.deprecation = :log + @app.config.active_support.test_order = :random + @app.config.log_level = :info - yield app if block_given? - app.initialize! + yield @app if block_given? + @app.initialize! - app.routes.draw do + @app.routes.draw do get "/" => "omg#index" end @@ -297,7 +302,10 @@ module TestHelpers end def boot_rails - require File.expand_path('../../../../load_paths', __FILE__) + # FIXME: shush Sass warning spam, not relevant to testing Railties + Kernel.silence_warnings do + require File.expand_path('../../../../load_paths', __FILE__) + end end end end diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 79bd7a8241..2c82f728ee 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -1304,6 +1304,55 @@ YAML assert_equal '/foo/bukkits/bukkit', last_response.body end + test "paths are properly generated when application is mounted at sub-path and relative_url_root is set" do + add_to_config "config.relative_url_root = '/foo'" + + @plugin.write "lib/bukkits.rb", <<-RUBY + module Bukkits + class Engine < ::Rails::Engine + isolate_namespace Bukkits + end + end + RUBY + + app_file "app/controllers/bar_controller.rb", <<-RUBY + class BarController < ApplicationController + def index + render text: bukkits.bukkit_path + end + end + RUBY + + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + get '/bar' => 'bar#index', :as => 'bar' + mount Bukkits::Engine => "/bukkits", :as => "bukkits" + end + RUBY + + @plugin.write "config/routes.rb", <<-RUBY + Bukkits::Engine.routes.draw do + get '/bukkit' => 'bukkit#index' + end + RUBY + + @plugin.write "app/controllers/bukkits/bukkit_controller.rb", <<-RUBY + class Bukkits::BukkitController < ActionController::Base + def index + render text: main_app.bar_path + end + end + RUBY + + boot_rails + + get("/bukkits/bukkit", {}, {'SCRIPT_NAME' => '/foo'}) + assert_equal '/foo/bar', last_response.body + + get("/bar", {}, {'SCRIPT_NAME' => '/foo'}) + assert_equal '/foo/bukkits/bukkit', last_response.body + end + private def app Rails.application diff --git a/railties/test/test_unit/reporter_test.rb b/railties/test/test_unit/reporter_test.rb index 3066ba82d6..fa6bb71c64 100644 --- a/railties/test/test_unit/reporter_test.rb +++ b/railties/test/test_unit/reporter_test.rb @@ -8,7 +8,7 @@ class TestUnitReporterTest < ActiveSupport::TestCase setup do @output = StringIO.new - @reporter = Rails::TestUnitReporter.new @output + @reporter = Rails::TestUnitReporter.new @output, output_inline: true end test "prints rerun snippet to run a single failed test" do @@ -57,6 +57,59 @@ class TestUnitReporterTest < ActiveSupport::TestCase end end + test "outputs failures inline" do + @reporter.record(failed_test) + @reporter.report + + assert_match %r{\A\n\nboo\n\nbin/rails test .*test/test_unit/reporter_test.rb:6\n\n\z}, @output.string + end + + test "outputs errors inline" do + @reporter.record(errored_test) + @reporter.report + + assert_match %r{\A\n\nArgumentError: wups\n No backtrace\n\nbin/rails test .*test/test_unit/reporter_test.rb:6\n\n\z}, @output.string + end + + test "outputs skipped tests inline if verbose" do + verbose = Rails::TestUnitReporter.new @output, verbose: true, output_inline: true + verbose.record(skipped_test) + verbose.report + + assert_match %r{\A\n\nskipchurches, misstemples\n\nbin/rails test .*test/test_unit/reporter_test.rb:6\n\n\z}, @output.string + end + + test "does not output rerun snippets after run" do + @reporter.record(failed_test) + @reporter.report + + assert_no_match 'Failed tests:', @output.string + end + + test "fail fast interrupts run on failure" do + fail_fast = Rails::TestUnitReporter.new @output, fail_fast: true + interrupt_raised = false + + # Minitest passes through Interrupt, catch it manually. + begin + fail_fast.record(failed_test) + rescue Interrupt + interrupt_raised = true + ensure + assert interrupt_raised, 'Expected Interrupt to be raised.' + end + end + + test "fail fast does not interrupt run errors or skips" do + fail_fast = Rails::TestUnitReporter.new @output, fail_fast: true + + fail_fast.record(errored_test) + assert_no_match 'Failed tests:', @output.string + + fail_fast.record(skipped_test) + assert_no_match 'Failed tests:', @output.string + end + private def assert_rerun_snippet_count(snippet_count) assert_equal snippet_count, @output.string.scan(%r{^bin/rails test }).size @@ -72,6 +125,12 @@ class TestUnitReporterTest < ActiveSupport::TestCase ft end + def errored_test + et = ExampleTest.new(:woot) + et.failures << Minitest::UnexpectedError.new(ArgumentError.new("wups")) + et + end + def passing_test ExampleTest.new(:woot) end @@ -79,7 +138,7 @@ class TestUnitReporterTest < ActiveSupport::TestCase def skipped_test st = ExampleTest.new(:woot) st.failures << begin - raise Minitest::Skip + raise Minitest::Skip, "skipchurches, misstemples" rescue Minitest::Assertion => e e end |