diff options
63 files changed, 1341 insertions, 579 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index b091d12d2c..0545d36a0a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,6 +1,12 @@ ## Rails 3.2.0 (unreleased) ## -* Log "Filter chain halted as CALLBACKNAME rendered or redirected" every time a before callback halts. *José Valim* +* Allow rescue responses to be configured through a railtie as in `config.action_dispatch.rescue_responses`. Please look at ActiveRecord::Railtie for an example *José Valim* + +* Allow fresh_when/stale? to take a record instead of an options hash *DHH* + +* Assets should use the request protocol by default or default to relative if no request is available *Jonathan del Strother* + +* Log "Filter chain halted as CALLBACKNAME rendered or redirected" every time a before callback halts *José Valim* * You can provide a namespace for your form to ensure uniqueness of id attributes on form elements. The namespace attribute will be prefixed with underscore on the generate HTML id. *Vasiliy Ermolovich* diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 8666ecc9c8..de250053d3 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -20,7 +20,7 @@ module ActionController status = payload[:status] if status.nil? && payload[:exception].present? - status = Rack::Utils.status_code(ActionDispatch::ShowExceptions.rescue_responses[payload[:exception].first]) rescue nil + status = Rack::Utils.status_code(ActionDispatch::ExceptionWrapper.new({}, payload[:exception]).status_code) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in %.0fms" % event.duration message << " (#{additions.join(" | ")})" unless additions.blank? diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index a5e37172c9..1645400693 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -23,8 +23,27 @@ module ActionController # This will render the show template if the request isn't sending a matching etag or # If-Modified-Since header and just a <tt>304 Not Modified</tt> response if there's a match. # - def fresh_when(options) - options.assert_valid_keys(:etag, :last_modified, :public) + # You can also just pass a record where last_modified will be set by calling updated_at and the etag by passing the object itself. Example: + # + # def show + # @article = Article.find(params[:id]) + # fresh_when(@article) + # end + # + # When passing a record, you can still set whether the public header: + # + # def show + # @article = Article.find(params[:id]) + # fresh_when(@article, :public => true) + # end + def fresh_when(record_or_options, additional_options = {}) + if record_or_options.is_a? Hash + options = record_or_options + options.assert_valid_keys(:etag, :last_modified, :public) + else + record = record_or_options + options = { :etag => record, :last_modified => record.try(:updated_at) }.merge(additional_options) + end response.etag = options[:etag] if options[:etag] response.last_modified = options[:last_modified] if options[:last_modified] @@ -55,8 +74,34 @@ module ActionController # end # end # end - def stale?(options) - fresh_when(options) + # + # You can also just pass a record where last_modified will be set by calling updated_at and the etag by passing the object itself. Example: + # + # def show + # @article = Article.find(params[:id]) + # + # if stale?(@article) + # @statistics = @article.really_expensive_call + # respond_to do |format| + # # all the supported formats + # end + # end + # end + # + # When passing a record, you can still set whether the public header: + # + # def show + # @article = Article.find(params[:id]) + # + # if stale?(@article, :public => true) + # @statistics = @article.really_expensive_call + # respond_to do |format| + # # all the supported formats + # end + # end + # end + def stale?(record_or_options, additional_options = {}) + fresh_when(record_or_options, additional_options) !request.fresh?(response) end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 1e92d14542..89f515ee3d 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -51,6 +51,8 @@ module ActionDispatch autoload :BestStandardsSupport autoload :Callbacks autoload :Cookies + autoload :DebugExceptions + autoload :ExceptionWrapper autoload :Flash autoload :Head autoload :ParamsParser diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb new file mode 100644 index 0000000000..5da21ddd3d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -0,0 +1,77 @@ +require 'action_dispatch/http/request' +require 'action_dispatch/middleware/exception_wrapper' + +module ActionDispatch + # This middleware is responsible for logging exceptions and + # showing a debugging page in case the request is local. + class DebugExceptions + RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') + + def initialize(app) + @app = app + end + + def call(env) + begin + response = @app.call(env) + + if response[1]['X-Cascade'] == 'pass' + raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" + end + rescue Exception => exception + raise exception if env['action_dispatch.show_exceptions'] == false + end + + response ? response : render_exception(env, exception) + end + + private + + def render_exception(env, exception) + wrapper = ExceptionWrapper.new(env, exception) + log_error(env, wrapper) + + if env['action_dispatch.show_detailed_exceptions'] + template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], + :request => Request.new(env), + :exception => wrapper.exception, + :application_trace => wrapper.application_trace, + :framework_trace => wrapper.framework_trace, + :full_trace => wrapper.full_trace + ) + + file = "rescues/#{wrapper.rescue_template}" + body = template.render(:template => file, :layout => 'rescues/layout') + render(wrapper.status_code, body) + else + raise exception + end + end + + def render(status, body) + [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] + end + + def log_error(env, wrapper) + logger = logger(env) + return unless logger + + exception = wrapper.exception + + ActiveSupport::Deprecation.silence do + message = "\n#{exception.class} (#{exception.message}):\n" + message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) + message << " " << wrapper.application_trace.join("\n ") + logger.fatal("#{message}\n\n") + end + end + + def logger(env) + env['action_dispatch.logger'] || stderr_logger + end + + def stderr_logger + @stderr_logger ||= Logger.new($stderr) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb new file mode 100644 index 0000000000..c0532c80c4 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -0,0 +1,78 @@ +require 'action_controller/metal/exceptions' +require 'active_support/core_ext/exception' + +module ActionDispatch + class ExceptionWrapper + cattr_accessor :rescue_responses + @@rescue_responses = Hash.new(:internal_server_error) + @@rescue_responses.merge!( + 'ActionController::RoutingError' => :not_found, + 'AbstractController::ActionNotFound' => :not_found, + 'ActionController::MethodNotAllowed' => :method_not_allowed, + 'ActionController::NotImplemented' => :not_implemented, + 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity + ) + + cattr_accessor :rescue_templates + @@rescue_templates = Hash.new('diagnostics') + @@rescue_templates.merge!( + 'ActionView::MissingTemplate' => 'missing_template', + 'ActionController::RoutingError' => 'routing_error', + 'AbstractController::ActionNotFound' => 'unknown_action', + 'ActionView::Template::Error' => 'template_error' + ) + + attr_reader :env, :exception + + def initialize(env, exception) + @env = env + @exception = original_exception(exception) + end + + def rescue_template + @@rescue_templates[@exception.class.name] + end + + def status_code + Rack::Utils.status_code(@@rescue_responses[@exception.class.name]) + end + + def application_trace + clean_backtrace(:silent) + end + + def framework_trace + clean_backtrace(:noise) + end + + def full_trace + clean_backtrace(:all) + end + + private + + def original_exception(exception) + if registered_original_exception?(exception) + exception.original_exception + else + exception + end + end + + def registered_original_exception?(exception) + exception.respond_to?(:original_exception) && @@rescue_responses.has_key?(exception.original_exception.class.name) + end + + def clean_backtrace(*args) + if backtrace_cleaner + backtrace_cleaner.clean(@exception.backtrace, *args) + else + @exception.backtrace + end + end + + def backtrace_cleaner + @backtrace_cleaner ||= @env['action_dispatch.backtrace_cleaner'] + end + end +end
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index c850e25507..9efe90a670 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -1,44 +1,33 @@ -require 'active_support/core_ext/exception' -require 'action_controller/metal/exceptions' -require 'active_support/notifications' require 'action_dispatch/http/request' +require 'action_dispatch/middleware/exception_wrapper' require 'active_support/deprecation' module ActionDispatch - # This middleware rescues any exception returned by the application and renders - # nice exception pages if it's being rescued locally. + # This middleware rescues any exception returned by the application + # and wraps them in a format for the end user. class ShowExceptions RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') - cattr_accessor :rescue_responses - @@rescue_responses = Hash.new(:internal_server_error) - @@rescue_responses.update({ - 'ActionController::RoutingError' => :not_found, - 'AbstractController::ActionNotFound' => :not_found, - 'ActiveRecord::RecordNotFound' => :not_found, - 'ActiveRecord::StaleObjectError' => :conflict, - 'ActiveRecord::RecordInvalid' => :unprocessable_entity, - 'ActiveRecord::RecordNotSaved' => :unprocessable_entity, - 'ActionController::MethodNotAllowed' => :method_not_allowed, - 'ActionController::NotImplemented' => :not_implemented, - 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity - }) - - cattr_accessor :rescue_templates - @@rescue_templates = Hash.new('diagnostics') - @@rescue_templates.update({ - 'ActionView::MissingTemplate' => 'missing_template', - 'ActionController::RoutingError' => 'routing_error', - 'AbstractController::ActionNotFound' => 'unknown_action', - 'ActionView::Template::Error' => 'template_error' - }) - FAILSAFE_RESPONSE = [500, {'Content-Type' => 'text/html'}, ["<html><body><h1>500 Internal Server Error</h1>" << "If you are the administrator of this website, then please read this web " << "application's log file and/or the web server's log file to find out what " << "went wrong.</body></html>"]] + class << self + def rescue_responses + ActiveSupport::Deprecation.warn "ActionDispatch::ShowExceptions.rescue_responses is deprecated. " \ + "Please configure your exceptions using a railtie or in your application config instead." + ExceptionWrapper.rescue_responses + end + + def rescue_templates + ActiveSupport::Deprecation.warn "ActionDispatch::ShowExceptions.rescue_templates is deprecated. " \ + "Please configure your exceptions using a railtie or in your application config instead." + ExceptionWrapper.rescue_templates + end + end + def initialize(app, consider_all_requests_local = nil) ActiveSupport::Deprecation.warn "Passing consider_all_requests_local option to ActionDispatch::ShowExceptions middleware no longer works" unless consider_all_requests_local.nil? @app = app @@ -46,131 +35,52 @@ module ActionDispatch def call(env) begin - status, headers, body = @app.call(env) - exception = nil - - # Only this middleware cares about RoutingError. So, let's just raise - # it here. - if headers['X-Cascade'] == 'pass' - raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" - end + response = @app.call(env) rescue Exception => exception raise exception if env['action_dispatch.show_exceptions'] == false end - exception ? render_exception(env, exception) : [status, headers, body] + response ? response : render_exception_with_failsafe(env, exception) end private - def render_exception(env, exception) - log_error(env, exception) - exception = original_exception(exception) - - if env['action_dispatch.show_detailed_exceptions'] == true - rescue_action_diagnostics(env, exception) - else - rescue_action_error_page(exception) - end - rescue Exception => failsafe_error - $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" - FAILSAFE_RESPONSE - end - - # Render detailed diagnostics for unhandled exceptions rescued from - # a controller action. - def rescue_action_diagnostics(env, exception) - template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], - :request => Request.new(env), - :exception => exception, - :application_trace => application_trace(env, exception), - :framework_trace => framework_trace(env, exception), - :full_trace => full_trace(env, exception) - ) - file = "rescues/#{@@rescue_templates[exception.class.name]}" - body = template.render(:template => file, :layout => 'rescues/layout') - render(status_code(exception), body) - end - - # Attempts to render a static error page based on the - # <tt>status_code</tt> thrown, or just return headers if no such file - # exists. At first, it will try to render a localized static page. - # For example, if a 500 error is being handled Rails and locale is :da, - # it will first attempt to render the file at <tt>public/500.da.html</tt> - # then attempt to render <tt>public/500.html</tt>. If none of them exist, - # the body of the response will be left empty. - def rescue_action_error_page(exception) - status = status_code(exception) - locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale - path = "#{public_path}/#{status}.html" - - if locale_path && File.exist?(locale_path) - render(status, File.read(locale_path)) - elsif File.exist?(path) - render(status, File.read(path)) - else - render(status, '') - end - end - - def status_code(exception) - Rack::Utils.status_code(@@rescue_responses[exception.class.name]) - end - - def render(status, body) - [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] - end - - def public_path - defined?(Rails.public_path) ? Rails.public_path : 'public_path' - end - - def log_error(env, exception) - return unless logger(env) - - ActiveSupport::Deprecation.silence do - message = "\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << application_trace(env, exception).join("\n ") - logger(env).fatal("#{message}\n\n") - end - end - - def application_trace(env, exception) - clean_backtrace(env, exception, :silent) - end - - def framework_trace(env, exception) - clean_backtrace(env, exception, :noise) - end - def full_trace(env, exception) - clean_backtrace(env, exception, :all) - end + # Define this method because some plugins were monkey patching it. + # Remove this after 3.2 is out with the other deprecations in this class. + def status_code(*) + end - def clean_backtrace(env, exception, *args) - env['action_dispatch.backtrace_cleaner'] ? - env['action_dispatch.backtrace_cleaner'].clean(exception.backtrace, *args) : - exception.backtrace - end + def render_exception_with_failsafe(env, exception) + render_exception(env, exception) + rescue Exception => failsafe_error + $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" + FAILSAFE_RESPONSE + end - def logger(env) - env['action_dispatch.logger'] || stderr_logger - end + def render_exception(env, exception) + wrapper = ExceptionWrapper.new(env, exception) - def stderr_logger - Logger.new($stderr) - end + status = wrapper.status_code + locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale + path = "#{public_path}/#{status}.html" - def original_exception(exception) - if registered_original_exception?(exception) - exception.original_exception + if locale_path && File.exist?(locale_path) + render(status, File.read(locale_path)) + elsif File.exist?(path) + render(status, File.read(path)) else - exception + render(status, '') end end - def registered_original_exception?(exception) - exception.respond_to?(:original_exception) && @@rescue_responses.has_key?(exception.original_exception.class.name) + def render(status, body) + [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] + end + + # TODO: Make this a middleware initialization parameter once + # we removed the second option (which is deprecated) + def public_path + defined?(Rails.public_path) ? Rails.public_path : 'public_path' end end end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index f18ebabf29..2ba3198ebd 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -9,11 +9,22 @@ module ActionDispatch config.action_dispatch.best_standards_support = true config.action_dispatch.tld_length = 1 config.action_dispatch.ignore_accept_header = false - config.action_dispatch.rack_cache = {:metastore => "rails:/", :entitystore => "rails:/", :verbose => true} + config.action_dispatch.rescue_templates = { } + config.action_dispatch.rescue_responses = { } + + config.action_dispatch.rack_cache = { + :metastore => "rails:/", + :entitystore => "rails:/", + :verbose => true + } + initializer "action_dispatch.configure" do |app| ActionDispatch::Http::URL.tld_length = app.config.action_dispatch.tld_length ActionDispatch::Request.ignore_accept_header = app.config.action_dispatch.ignore_accept_header + ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses) + ActionDispatch::ExceptionWrapper.rescue_templates.merge!(config.action_dispatch.rescue_templates) + config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil? ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie end diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 6c97d75922..e674e12509 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -160,7 +160,7 @@ module ActionView # here a named route directly as well. Defaults to the current action. # * <tt>:namespace</tt> - A namespace for your form to ensure uniqueness of # id attributes on form elements. The namespace attribute will be prefixed - # with underscore on the generate HTML id. + # with underscore on the generated HTML id. # * <tt>:html</tt> - Optional HTML attributes for the form tag. # # Also note that +form_for+ doesn't create an exclusive scope. It's still diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index 587e37a84f..fe27e54037 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -89,10 +89,14 @@ module ActionView line_counter = start_on_line return unless source_code = source_code[start_on_line..end_on_line] - source_code.sum do |line| + extract = source_code.sum do |line| line_counter += 1 "#{indent}#{line_counter}: #{line}\n" end + + extract.encode! if extract.respond_to?(:encode!) + + extract end def sub_template_of(template_path) diff --git a/actionpack/lib/sprockets/railtie.rb b/actionpack/lib/sprockets/railtie.rb index 3d330bd91a..a7eb03acaf 100644 --- a/actionpack/lib/sprockets/railtie.rb +++ b/actionpack/lib/sprockets/railtie.rb @@ -10,8 +10,6 @@ module Sprockets # TODO: Get rid of config.assets.enabled class Railtie < ::Rails::Railtie - config.action_controller.default_asset_host_protocol = :relative - rake_tasks do load "sprockets/assets.rake" end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index d191a203dd..c95b8221a1 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -175,6 +175,7 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| middleware.use "ActionDispatch::ShowExceptions" + middleware.use "ActionDispatch::DebugExceptions" middleware.use "ActionDispatch::Callbacks" middleware.use "ActionDispatch::ParamsParser" middleware.use "ActionDispatch::Cookies" @@ -338,16 +339,19 @@ end module ActionDispatch class ShowExceptions private - remove_method :public_path - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end + remove_method :public_path + def public_path + "#{FIXTURE_LOAD_PATH}/public" + end + end - remove_method :stderr_logger - # Silence logger - def stderr_logger - nil - end + class DebugExceptions + private + remove_method :stderr_logger + # Silence logger + def stderr_logger + nil + end end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index aea603b014..243bad8749 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -50,12 +50,28 @@ class TestController < ActionController::Base end end + def conditional_hello_with_record + record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123") + + if stale?(record) + render :action => 'hello_world' + end + end + def conditional_hello_with_public_header if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true) render :action => 'hello_world' end end + def conditional_hello_with_public_header_with_record + record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123") + + if stale?(record, :public => true) + render :action => 'hello_world' + end + end + def conditional_hello_with_public_header_and_expires_at expires_in 1.minute if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true) @@ -1440,6 +1456,36 @@ class LastModifiedRenderTest < ActionController::TestCase assert_equal @last_modified, @response.headers['Last-Modified'] end + + def test_responds_with_last_modified_with_record + get :conditional_hello_with_record + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_not_modified_with_record + @request.if_modified_since = @last_modified + get :conditional_hello_with_record + assert_equal 304, @response.status.to_i + assert_blank @response.body + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_not_modified_but_etag_differs_with_record + @request.if_modified_since = @last_modified + @request.if_none_match = "234" + get :conditional_hello_with_record + assert_response :success + end + + def test_request_modified_with_record + @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' + get :conditional_hello_with_record + assert_equal 200, @response.status.to_i + assert_present @response.body + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_with_bang_gets_last_modified get :conditional_hello_with_bangs assert_equal @last_modified, @response.headers['Last-Modified'] diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 74067cb895..5eff1eb09d 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -3,6 +3,7 @@ require 'abstract_unit' module ShowExceptions class ShowExceptionsController < ActionController::Base use ActionDispatch::ShowExceptions + use ActionDispatch::DebugExceptions def boom raise 'boom!' diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb new file mode 100644 index 0000000000..e6c0a06878 --- /dev/null +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -0,0 +1,116 @@ +require 'abstract_unit' + +class DebugExceptionsTest < ActionDispatch::IntegrationTest + + class Boomer + def initialize(detailed = false) + @detailed = detailed + end + + def call(env) + env['action_dispatch.show_detailed_exceptions'] = @detailed + req = ActionDispatch::Request.new(env) + case req.path + when "/not_found" + raise ActionController::UnknownAction + when "/runtime_error" + raise RuntimeError + when "/method_not_allowed" + raise ActionController::MethodNotAllowed + when "/not_implemented" + raise ActionController::NotImplemented + when "/unprocessable_entity" + raise ActionController::InvalidAuthenticityToken + when "/not_found_original_exception" + raise ActionView::Template::Error.new('template', {}, AbstractController::ActionNotFound.new) + else + raise "puke!" + end + end + end + + ProductionApp = ActionDispatch::DebugExceptions.new((Boomer.new(false))) + DevelopmentApp = ActionDispatch::DebugExceptions.new((Boomer.new(true))) + + test 'skip diagnosis if not showing detailed exceptions' do + @app = ProductionApp + assert_raise RuntimeError do + get "/", {}, {'action_dispatch.show_exceptions' => true} + end + end + + test 'skip diagnosis if not showing exceptions' do + @app = DevelopmentApp + assert_raise RuntimeError do + get "/", {}, {'action_dispatch.show_exceptions' => false} + end + end + + test "rescue with diagnostics message" do + @app = DevelopmentApp + + get "/", {}, {'action_dispatch.show_exceptions' => true} + assert_response 500 + assert_match(/puke/, body) + + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} + assert_response 404 + assert_match(/#{ActionController::UnknownAction.name}/, body) + + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} + assert_response 405 + assert_match(/ActionController::MethodNotAllowed/, body) + end + + test "does not show filtered parameters" do + @app = DevelopmentApp + + get "/", {"foo"=>"bar"}, {'action_dispatch.show_exceptions' => true, + 'action_dispatch.parameter_filter' => [:foo]} + assert_response 500 + assert_match(""foo"=>"[FILTERED]"", body) + end + + test "show registered original exception for wrapped exceptions" do + @app = DevelopmentApp + + get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + assert_response 404 + assert_match(/AbstractController::ActionNotFound/, body) + end + + test "show the controller name in the diagnostics template when controller name is present" do + @app = DevelopmentApp + get("/runtime_error", {}, { + 'action_dispatch.show_exceptions' => true, + 'action_dispatch.request.parameters' => { + 'action' => 'show', + 'id' => 'unknown', + 'controller' => 'featured_tile' + } + }) + assert_response 500 + assert_match(/RuntimeError\n in FeaturedTileController/, body) + end + + test "sets the HTTP charset parameter" do + @app = DevelopmentApp + + get "/", {}, {'action_dispatch.show_exceptions' => true} + assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] + end + + test 'uses logger from env' do + @app = DevelopmentApp + output = StringIO.new + get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} + assert_match(/puke/, output.rewind && output.read) + end + + test 'uses backtrace cleaner from env' do + @app = DevelopmentApp + cleaner = stub(:clean => ['passed backtrace cleaner']) + get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner} + assert_match(/passed backtrace cleaner/, body) + end +end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 90f13a3bb9..020cc80f3f 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -3,24 +3,13 @@ require 'abstract_unit' class ShowExceptionsTest < ActionDispatch::IntegrationTest class Boomer - def initialize(detailed = false) - @detailed = detailed - end - def call(env) - env['action_dispatch.show_detailed_exceptions'] = @detailed req = ActionDispatch::Request.new(env) case req.path when "/not_found" raise ActionController::UnknownAction - when "/runtime_error" - raise RuntimeError when "/method_not_allowed" raise ActionController::MethodNotAllowed - when "/not_implemented" - raise ActionController::NotImplemented - when "/unprocessable_entity" - raise ActionController::InvalidAuthenticityToken when "/not_found_original_exception" raise ActionView::Template::Error.new('template', {}, AbstractController::ActionNotFound.new) else @@ -29,10 +18,16 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end end - ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new(false)) - DevelopmentApp = ActionDispatch::ShowExceptions.new(Boomer.new(true)) + ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new) + + test 'skip diagnosis if not showing exceptions' do + @app = ProductionApp + assert_raise RuntimeError do + get "/", {}, {'action_dispatch.show_exceptions' => false} + end + end - test "rescue with error page when show_exceptions is false" do + test "rescue with error page" do @app = ProductionApp get "/", {}, {'action_dispatch.show_exceptions' => true} @@ -48,24 +43,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest assert_equal "", body end - test "rescue with diagnostics message when show_exceptions is true" do - @app = DevelopmentApp - - get "/", {}, {'action_dispatch.show_exceptions' => true} - assert_response 500 - assert_match(/puke/, body) - - get "/not_found", {}, {'action_dispatch.show_exceptions' => true} - assert_response 404 - assert_match(/#{ActionController::UnknownAction.name}/, body) - - get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} - assert_response 405 - assert_match(/ActionController::MethodNotAllowed/, body) - end - test "localize rescue error page" do - # Change locale old_locale, I18n.locale = I18n.locale, :da begin @@ -83,63 +61,18 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end end - test "does not show filtered parameters" do - @app = DevelopmentApp - - get "/", {"foo"=>"bar"}, {'action_dispatch.show_exceptions' => true, - 'action_dispatch.parameter_filter' => [:foo]} - assert_response 500 - assert_match(""foo"=>"[FILTERED]"", body) - end - - test "show registered original exception for wrapped exceptions when show_exceptions is false" do - @app = ProductionApp - - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} - assert_response 404 - assert_match(/404 error/, body) - end - - test "show registered original exception for wrapped exceptions when show_exceptions is true" do - @app = DevelopmentApp - - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} - assert_response 404 - assert_match(/AbstractController::ActionNotFound/, body) - end - - test "show the controller name in the diagnostics template when controller name is present" do - @app = DevelopmentApp - get("/runtime_error", {}, { - 'action_dispatch.show_exceptions' => true, - 'action_dispatch.request.parameters' => { - 'action' => 'show', - 'id' => 'unknown', - 'controller' => 'featured_tile' - } - }) - assert_response 500 - assert_match(/RuntimeError\n in FeaturedTileController/, body) - end - test "sets the HTTP charset parameter" do - @app = DevelopmentApp + @app = ProductionApp get "/", {}, {'action_dispatch.show_exceptions' => true} assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end - test 'uses logger from env' do + test "show registered original exception for wrapped exceptions" do @app = ProductionApp - output = StringIO.new - get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} - assert_match(/puke/, output.rewind && output.read) - end - test 'uses backtrace cleaner from env' do - @app = DevelopmentApp - cleaner = stub(:clean => ['passed backtrace cleaner']) - get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner} - assert_match(/passed backtrace cleaner/, body) + get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + assert_response 404 + assert_match(/404 error/, body) end end diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index eb7aac709d..6bc928bab7 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -23,7 +23,7 @@ module ActiveModel keys = CHECKS.keys & options.keys if keys.empty? - raise ArgumentError, 'Range unspecified. Specify the :within, :maximum, :minimum, or :is option.' + raise ArgumentError, 'Range unspecified. Specify the :in, :within, :maximum, :minimum, or :is option.' end keys.each do |key| diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f798b03ea1..9ffd085925 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,16 @@ ## Rails 3.2.0 (unreleased) ## +* Implements automatic EXPLAIN logging for slow queries. + + A new configuration parameter `config.active_record.auto_explain_threshold_in_seconds` + determines what's to be considered a slow query. Setting that to `nil` disables + this feature. Defaults are 0.5 in development mode, and `nil` in test and production + modes. + + As of this writing there's support for SQLite, MySQL (mysql2 adapter), and + PostgreSQL. + + *fxn* * Implemented ActiveRecord::Relation#pluck method diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 3572c640eb..b1377c24dd 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -50,6 +50,7 @@ module ActiveRecord autoload :PredicateBuilder autoload :SpawnMethods autoload :Batches + autoload :Explain end autoload :Base @@ -75,6 +76,7 @@ module ActiveRecord autoload :Transactions autoload :Validations autoload :IdentityMap + autoload :Explain end module Coders @@ -92,6 +94,8 @@ module ActiveRecord autoload :Read autoload :TimeZoneConversion autoload :Write + autoload :Serialization + autoload :DeprecatedUnderscoreRead end end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 60bbc325df..98efdb90c8 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -213,7 +213,7 @@ module ActiveRecord # # If your model class is <tt>Project</tt>, the module is # named <tt>Project::GeneratedFeatureMethods</tt>. The GeneratedFeatureMethods module is - # is included in the model class immediately after the (anonymous) generated attributes methods + # included in the model class immediately after the (anonymous) generated attributes methods # module, meaning an association will override the methods for an attribute with the same name. # # === A word of warning diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index d7bfaa5655..3c9fafb1ca 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -29,12 +29,16 @@ module ActiveRecord end end - def undefine_attribute_methods(*args) + def generated_attribute_methods + @generated_attribute_methods ||= (base_class == self ? super : base_class.generated_attribute_methods) + end + + def undefine_attribute_methods if base_class == self super @attribute_methods_generated = false else - base_class.undefine_attribute_methods(*args) + base_class.undefine_attribute_methods end end diff --git a/activerecord/lib/active_record/attribute_methods/deprecated_underscore_read.rb b/activerecord/lib/active_record/attribute_methods/deprecated_underscore_read.rb new file mode 100644 index 0000000000..0eb0db65b1 --- /dev/null +++ b/activerecord/lib/active_record/attribute_methods/deprecated_underscore_read.rb @@ -0,0 +1,32 @@ +require 'active_support/concern' +require 'active_support/deprecation' + +module ActiveRecord + module AttributeMethods + module DeprecatedUnderscoreRead + extend ActiveSupport::Concern + + included do + attribute_method_prefix "_" + end + + module ClassMethods + protected + + def define_method__attribute(attr_name) + # Do nothing, let it hit method missing instead. + end + end + + protected + + def _attribute(attr_name) + ActiveSupport::Deprecation.warn( + "You have called '_#{attr_name}'. This is deprecated. Please use " \ + "either '#{attr_name}' or read_attribute('#{attr_name}')." + ) + read_attribute(attr_name) + end + end + end +end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 3eff3d54e3..f61e016f46 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -34,9 +34,9 @@ module ActiveRecord @previously_changed = changes @changed_attributes.clear end - rescue - IdentityMap.remove(self) if IdentityMap.enabled? - raise + rescue + IdentityMap.remove(self) if IdentityMap.enabled? + raise end # <tt>reload</tt> the record and clears changed attributes. diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 93dae3ff86..c756924186 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -5,15 +5,49 @@ module ActiveRecord # Returns this record's primary key value wrapped in an Array if one is available def to_key - key = send(self.class.primary_key) + key = self.id [key] if key end + # Returns the primary key value + def id + read_attribute(self.class.primary_key) + end + + # Sets the primary key value + def id=(value) + write_attribute(self.class.primary_key, value) + end + + # Queries the primary key value + def id? + query_attribute(self.class.primary_key) + end + module ClassMethods + def define_method_attribute(attr_name) + super + + if attr_name == primary_key && attr_name != 'id' + generated_attribute_methods.send(:alias_method, :id, primary_key) + generated_attribute_methods.module_eval <<-CODE, __FILE__, __LINE__ + def self.attribute_id(v, attributes, attributes_cache, attr_name) + attr_name = '#{primary_key}' + send(:'attribute_#{attr_name}', attributes[attr_name], attributes, attributes_cache, attr_name) + end + CODE + end + end + + def dangerous_attribute_method?(method_name) + super && !['id', 'id=', 'id?'].include?(method_name) + end + # Defines the primary key field -- can be overridden in subclasses. Overwriting will negate any effect of the # primary_key_prefix_type setting, though. def primary_key - @primary_key ||= reset_primary_key + @primary_key = reset_primary_key unless defined? @primary_key + @primary_key end # Returns a quoted version of the primary key name, used to construct SQL statements. diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 4a5afcd585..8942fab500 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -8,9 +8,6 @@ module ActiveRecord included do cattr_accessor :attribute_types_cached_by_default, :instance_writer => false self.attribute_types_cached_by_default = ATTRIBUTE_TYPES_CACHED_BY_DEFAULT - - # Undefine id so it can be used as an attribute name - undef_method(:id) if method_defined?(:id) end module ClassMethods @@ -32,109 +29,106 @@ module ActiveRecord cached_attributes.include?(attr_name) end + def undefine_attribute_methods + if base_class == self + generated_attribute_methods.module_eval do + public_methods(false).each do |m| + singleton_class.send(:undef_method, m) if m.to_s =~ /^attribute_/ + end + end + end + + super + end + protected + # We want to generate the methods via module_eval rather than define_method, + # because define_method is slower on dispatch and uses more memory (because it + # creates a closure). + # + # But sometimes the database might return columns with characters that are not + # allowed in normal method names (like 'my_column(omg)'. So to work around this + # we first define with the __temp__ identifier, and then use alias method to + # rename it to what we want. def define_method_attribute(attr_name) - if serialized_attributes.include?(attr_name) - define_read_method_for_serialized_attribute(attr_name) - else - define_read_method(attr_name, attr_name, columns_hash[attr_name]) - end + cast_code = attribute_cast_code(attr_name) + internal = internal_attribute_access_code(attr_name, cast_code) + external = external_attribute_access_code(attr_name, cast_code) - if attr_name == primary_key && attr_name != "id" - define_read_method('id', attr_name, columns_hash[attr_name]) - end + generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + def __temp__ + #{internal} + end + alias_method '#{attr_name}', :__temp__ + undef_method :__temp__ + STR + + generated_attribute_methods.singleton_class.module_eval <<-STR, __FILE__, __LINE__ + def __temp__(v, attributes, attributes_cache, attr_name) + #{external} + end + alias_method 'attribute_#{attr_name}', :__temp__ + undef_method :__temp__ + STR end private def cacheable_column?(column) - serialized_attributes.include?(column.name) || attribute_types_cached_by_default.include?(column.type) - end - - # Define read method for serialized attribute. - def define_read_method_for_serialized_attribute(attr_name) - access_code = "@attributes_cache['#{attr_name}'] ||= @attributes['#{attr_name}']" - generated_attribute_methods.module_eval("def _#{attr_name}; #{access_code}; end; alias #{attr_name} _#{attr_name}", __FILE__, __LINE__) + attribute_types_cached_by_default.include?(column.type) end - # Define an attribute reader method. Cope with nil column. - # method_name is the same as attr_name except when a non-standard primary key is used, - # we still define #id as an accessor for the key - def define_read_method(method_name, attr_name, column) - cast_code = column.type_cast_code('v') - access_code = "(v=@attributes['#{attr_name}']) && #{cast_code}" + def internal_attribute_access_code(attr_name, cast_code) + access_code = "(v=@attributes[attr_name]) && #{cast_code}" - unless attr_name.to_s == self.primary_key.to_s - access_code.insert(0, "missing_attribute('#{attr_name}', caller) unless @attributes.has_key?('#{attr_name}'); ") + unless attr_name == primary_key + access_code.insert(0, "missing_attribute(attr_name, caller) unless @attributes.has_key?(attr_name); ") end if cache_attribute?(attr_name) - access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" + access_code = "@attributes_cache[attr_name] ||= (#{access_code})" end - # Where possible, generate the method by evalling a string, as this will result in - # faster accesses because it avoids the block eval and then string eval incurred - # by the second branch. - # - # The second, slower, branch is necessary to support instances where the database - # returns columns with extra stuff in (like 'my_column(omg)'). - if method_name =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP - generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ - def _#{method_name} - #{access_code} - end - - alias #{method_name} _#{method_name} - STR - else - generated_attribute_methods.module_eval do - define_method("_#{method_name}") { eval(access_code) } - alias_method(method_name, "_#{method_name}") - end + "attr_name = '#{attr_name}'; #{access_code}" + end + + def external_attribute_access_code(attr_name, cast_code) + access_code = "v && #{cast_code}" + + if cache_attribute?(attr_name) + access_code = "attributes_cache[attr_name] ||= (#{access_code})" end + + access_code + end + + def attribute_cast_code(attr_name) + columns_hash[attr_name].type_cast_code('v') end end # Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example, # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)). def read_attribute(attr_name) - method = "_#{attr_name}" - if respond_to? method - send method if @attributes.has_key?(attr_name.to_s) - else - _read_attribute attr_name - end - end - - def _read_attribute(attr_name) attr_name = attr_name.to_s - attr_name = self.class.primary_key if attr_name == 'id' - value = @attributes[attr_name] - unless value.nil? - if column = column_for_attribute(attr_name) - if unserializable_attribute?(attr_name, column) - unserialize_attribute(attr_name) - else - column.type_cast(value) - end - else - value + accessor = "attribute_#{attr_name}" + methods = self.class.generated_attribute_methods + + if methods.respond_to?(accessor) + if @attributes.has_key?(attr_name) || attr_name == 'id' + methods.send(accessor, @attributes[attr_name], @attributes, @attributes_cache, attr_name) end + elsif !self.class.attribute_methods_generated? + # If we haven't generated the caster methods yet, do that and + # then try again + self.class.define_attribute_methods + read_attribute(attr_name) + else + # If we get here, the attribute has no associated DB column, so + # just return it verbatim. + @attributes[attr_name] end end - # Returns true if the attribute is of a text column and marked for serialization. - def unserializable_attribute?(attr_name, column) - column.text? && self.class.serialized_attributes.include?(attr_name) - end - - # Returns the unserialized object of the attribute. - def unserialize_attribute(attr_name) - coder = self.class.serialized_attributes[attr_name] - unserialized_object = coder.load(@attributes[attr_name]) - - @attributes.frozen? ? unserialized_object : @attributes[attr_name] = unserialized_object - end - private def attribute(attribute_name) read_attribute(attribute_name) diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb new file mode 100644 index 0000000000..0a4432506f --- /dev/null +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -0,0 +1,89 @@ +module ActiveRecord + module AttributeMethods + module Serialization + extend ActiveSupport::Concern + + included do + # Returns a hash of all the attributes that have been specified for serialization as + # keys and their class restriction as values. + class_attribute :serialized_attributes + self.serialized_attributes = {} + end + + class Attribute < Struct.new(:coder, :value, :state) + def unserialized_value + state == :serialized ? unserialize : value + end + + def serialized_value + state == :unserialized ? serialize : value + end + + def unserialize + self.state = :unserialized + self.value = coder.load(value) + end + + def serialize + self.state = :serialized + self.value = coder.dump(value) + end + end + + module ClassMethods + # If you have an attribute that needs to be saved to the database as an object, and retrieved as the same object, + # then specify the name of that attribute using this method and it will be handled automatically. + # The serialization is done through YAML. If +class_name+ is specified, the serialized object must be of that + # class on retrieval or SerializationTypeMismatch will be raised. + # + # ==== Parameters + # + # * +attr_name+ - The field name that should be serialized. + # * +class_name+ - Optional, class name that the object type should be equal to. + # + # ==== Example + # # Serialize a preferences attribute + # class User < ActiveRecord::Base + # serialize :preferences + # end + def serialize(attr_name, class_name = Object) + coder = if [:load, :dump].all? { |x| class_name.respond_to?(x) } + class_name + else + Coders::YAMLColumn.new(class_name) + end + + # merge new serialized attribute and create new hash to ensure that each class in inheritance hierarchy + # has its own hash of own serialized attributes + self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder) + end + + private + + def attribute_cast_code(attr_name) + if serialized_attributes.include?(attr_name) + "v.unserialized_value" + else + super + end + end + end + + def set_serialized_attributes + self.class.serialized_attributes.each do |key, coder| + if @attributes.key?(key) + @attributes[key] = Attribute.new(coder, @attributes[key], :serialized) + end + end + end + + def type_cast_attribute_for_write(column, value) + if column && coder = self.class.serialized_attributes[column.name] + Attribute.new(coder, value, :unserialized) + else + super + end + end + end + end +end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 62a3cfa9a5..236681096b 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -19,18 +19,19 @@ module ActiveRecord # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. # This enhanced read method automatically converts the UTC time stored in the database to the time # zone stored in Time.zone. - def define_method_attribute(attr_name) + def internal_attribute_access_code(attr_name, cast_code) + column = columns_hash[attr_name] + + if create_time_zone_conversion_attribute?(attr_name, column) + super(attr_name, "(v=#{column.type_cast_code('v')}) && #{cast_code}") + else + super + end + end + + def attribute_cast_code(attr_name) if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) - method_body, line = <<-EOV, __LINE__ + 1 - def _#{attr_name} - cached = @attributes_cache['#{attr_name}'] - return cached if cached - time = _read_attribute('#{attr_name}') - @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time - end - alias #{attr_name} _#{attr_name} - EOV - generated_attribute_methods.module_eval(method_body, __FILE__, line) + "(v.acts_like?(:time) ? v.in_time_zone : v)" else super end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index eb585ee906..07499db9f0 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -17,10 +17,6 @@ module ActiveRecord write_attribute(attr_name, new_value) end end - - if attr_name == primary_key && attr_name != "id" - generated_attribute_methods.module_eval("alias :id= :'#{primary_key}='") - end end end @@ -32,10 +28,8 @@ module ActiveRecord @attributes_cache.delete(attr_name) column = column_for_attribute(attr_name) - if column && column.number? - @attributes[attr_name] = convert_number_column_value(value) - elsif column || @attributes.has_key?(attr_name) - @attributes[attr_name] = value + if column || @attributes.has_key?(attr_name) + @attributes[attr_name] = type_cast_attribute_for_write(column, value) else raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'" end @@ -47,6 +41,14 @@ module ActiveRecord def attribute=(attribute_name, value) write_attribute(attribute_name, value) end + + def type_cast_attribute_for_write(column, value) + if column && column.number? + convert_number_column_value(value) + else + value + end + end end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2a02380591..3d55729318 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -433,10 +433,10 @@ module ActiveRecord #:nodoc: class_attribute :default_scopes, :instance_writer => false self.default_scopes = [] - # Returns a hash of all the attributes that have been specified for serialization as - # keys and their class restriction as values. - class_attribute :serialized_attributes - self.serialized_attributes = {} + # If a query takes longer than these many seconds we log its query plan + # automatically. nil disables this feature. + class_attribute :auto_explain_threshold_in_seconds, :instance_writer => false + self.auto_explain_threshold_in_seconds = nil class_attribute :_attr_readonly, :instance_writer => false self._attr_readonly = [] @@ -489,7 +489,9 @@ module ActiveRecord #:nodoc: # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # > [#<Post:0x36bff9c @attributes={"title"=>"The Cheap Man Buys Twice"}>, ...] def find_by_sql(sql, binds = []) - connection.select_all(sanitize_sql(sql), "#{name} Load", binds).collect! { |record| instantiate(record) } + logging_query_plan do + connection.select_all(sanitize_sql(sql), "#{name} Load", binds).collect! { |record| instantiate(record) } + end end # Creates an object (or multiple objects) and saves it to the database, if validations pass. @@ -560,33 +562,6 @@ module ActiveRecord #:nodoc: self._attr_readonly end - # If you have an attribute that needs to be saved to the database as an object, and retrieved as the same object, - # then specify the name of that attribute using this method and it will be handled automatically. - # The serialization is done through YAML. If +class_name+ is specified, the serialized object must be of that - # class on retrieval or SerializationTypeMismatch will be raised. - # - # ==== Parameters - # - # * +attr_name+ - The field name that should be serialized. - # * +class_name+ - Optional, class name that the object type should be equal to. - # - # ==== Example - # # Serialize a preferences attribute - # class User < ActiveRecord::Base - # serialize :preferences - # end - def serialize(attr_name, class_name = Object) - coder = if [:load, :dump].all? { |x| class_name.respond_to?(x) } - class_name - else - Coders::YAMLColumn.new(class_name) - end - - # merge new serialized attribute and create new hash to ensure that each class in inheritance hierarchy - # has its own hash of own serialized attributes - self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder) - end - def deprecated_property_setter(property, value, block) #:nodoc: if block ActiveSupport::Deprecation.warn( @@ -1853,7 +1828,7 @@ MSG # Returns true if the specified +attribute+ has been set by the user or by a database load and is neither # nil nor empty? (the latter only applies to objects that respond to empty?, most notably Strings). def attribute_present?(attribute) - value = _read_attribute(attribute) + value = read_attribute(attribute) !value.nil? || (value.respond_to?(:empty?) && !value.empty?) end @@ -1967,6 +1942,26 @@ MSG "#<#{self.class} #{inspection}>" end + # Hackery to accomodate Syck. Remove for 4.0. + def to_yaml(opts = {}) #:nodoc: + if YAML.const_defined?(:ENGINE) && !YAML::ENGINE.syck? + super + else + coder = {} + encode_with(coder) + YAML.quick_emit(self, opts) do |out| + out.map(taguri, to_yaml_style) do |map| + coder.each { |k, v| map.add(k, v) } + end + end + end + end + + # Hackery to accomodate Syck. Remove for 4.0. + def yaml_initialize(tag, coder) #:nodoc: + init_with(coder) + end + protected def clone_attributes(reader_method = :read_attribute, attributes = {}) attribute_names.each do |name| @@ -2004,14 +1999,6 @@ MSG nil end - def set_serialized_attributes - sattrs = self.class.serialized_attributes - - sattrs.each do |key, coder| - @attributes[key] = coder.load @attributes[key] if @attributes.key?(key) - end - end - # Sets the attribute used for single table inheritance to this class name if this is not the # ActiveRecord::Base descendant. # Considering the hierarchy Reply < Message < ActiveRecord::Base, this makes it possible to @@ -2043,8 +2030,8 @@ MSG if include_readonly_attributes || (!include_readonly_attributes && !self.class.readonly_attributes.include?(name)) - value = if coder = klass.serialized_attributes[name] - coder.dump @attributes[name] + value = if klass.serialized_attributes.include?(name) + @attributes[name].serialized_value else # FIXME: we need @attributes to be used consistently. # If the values stored in @attributes were already type @@ -2219,11 +2206,14 @@ MSG include AttributeMethods::PrimaryKey include AttributeMethods::TimeZoneConversion include AttributeMethods::Dirty + include AttributeMethods::Serialization + include AttributeMethods::DeprecatedUnderscoreRead include ActiveModel::MassAssignmentSecurity include Callbacks, ActiveModel::Observing, Timestamp include Associations, NamedScope include IdentityMap include ActiveModel::SecurePassword + extend Explain # AutosaveAssociation needs to be included before Transactions, because we want # #save_with_autosave_associations to be wrapped inside a transaction. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 1a4cc93d2d..5a2493f69d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -142,6 +142,12 @@ module ActiveRecord false end + # Does this adapter support explain? As of this writing sqlite3, + # mysql2, and postgresql are the only ones that do. + def supports_explain? + false + end + # QUOTING ================================================== # Override to return the quoted table name. Defaults to column quoting. 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 f143fd348e..7f5ddf77d6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -225,80 +225,6 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== - def explain(arel) - sql = "EXPLAIN #{to_sql(arel)}" - start = Time.now - result = exec_query(sql, 'EXPLAIN') - elapsed = Time.now - start - - ExplainPrettyPrinter.new.pp(result, elapsed) - end - - class ExplainPrettyPrinter # :nodoc: - # Pretty prints the result of a EXPLAIN in a way that resembles the output of the - # MySQL shell: - # - # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ - # | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | - # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ - # | 1 | SIMPLE | users | const | PRIMARY | PRIMARY | 4 | const | 1 | | - # | 1 | SIMPLE | posts | ALL | NULL | NULL | NULL | NULL | 1 | Using where | - # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ - # 2 rows in set (0.00 sec) - # - # This is an exercise in Ruby hyperrealism :). - def pp(result, elapsed) - widths = compute_column_widths(result) - separator = build_separator(widths) - - pp = [] - - pp << separator - pp << build_cells(result.columns, widths) - pp << separator - - result.rows.each do |row| - pp << build_cells(row, widths) - end - - pp << separator - pp << build_footer(result.rows.length, elapsed) - - pp.join("\n") + "\n" - end - - private - - def compute_column_widths(result) - [].tap do |widths| - result.columns.each_with_index do |column, i| - cells_in_column = [column] + result.rows.map {|r| r[i].nil? ? 'NULL' : r[i].to_s} - widths << cells_in_column.map(&:length).max - end - end - end - - def build_separator(widths) - padding = 1 - '+' + widths.map {|w| '-' * (w + (padding*2))}.join('+') + '+' - end - - def build_cells(items, widths) - cells = [] - items.each_with_index do |item, i| - item = 'NULL' if item.nil? - justifier = item.is_a?(Numeric) ? 'rjust' : 'ljust' - cells << item.to_s.send(justifier, widths[i]) - end - '| ' + cells.join(' | ') + ' |' - end - - def build_footer(nrows, elapsed) - rows_label = nrows == 1 ? 'row' : 'rows' - "#{nrows} #{rows_label} in set (%.2f sec)" % elapsed - end - end - # Executes the SQL statement in the context of this connection. def execute(sql, name = nil) if name == :skip_logging diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 95f254ddd2..626571a948 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -35,6 +35,10 @@ module ActiveRecord configure_connection end + def supports_explain? + true + end + # HELPER METHODS =========================================== def each_hash(result) # :nodoc: @@ -93,6 +97,80 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== + def explain(arel, binds = []) + sql = "EXPLAIN #{to_sql(arel)}" + start = Time.now + result = exec_query(sql, 'EXPLAIN', binds) + elapsed = Time.now - start + + ExplainPrettyPrinter.new.pp(result, elapsed) + end + + class ExplainPrettyPrinter # :nodoc: + # Pretty prints the result of a EXPLAIN in a way that resembles the output of the + # MySQL shell: + # + # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ + # | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | + # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ + # | 1 | SIMPLE | users | const | PRIMARY | PRIMARY | 4 | const | 1 | | + # | 1 | SIMPLE | posts | ALL | NULL | NULL | NULL | NULL | 1 | Using where | + # +----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+ + # 2 rows in set (0.00 sec) + # + # This is an exercise in Ruby hyperrealism :). + def pp(result, elapsed) + widths = compute_column_widths(result) + separator = build_separator(widths) + + pp = [] + + pp << separator + pp << build_cells(result.columns, widths) + pp << separator + + result.rows.each do |row| + pp << build_cells(row, widths) + end + + pp << separator + pp << build_footer(result.rows.length, elapsed) + + pp.join("\n") + "\n" + end + + private + + def compute_column_widths(result) + [].tap do |widths| + result.columns.each_with_index do |column, i| + cells_in_column = [column] + result.rows.map {|r| r[i].nil? ? 'NULL' : r[i].to_s} + widths << cells_in_column.map(&:length).max + end + end + end + + def build_separator(widths) + padding = 1 + '+' + widths.map {|w| '-' * (w + (padding*2))}.join('+') + '+' + end + + def build_cells(items, widths) + cells = [] + items.each_with_index do |item, i| + item = 'NULL' if item.nil? + justifier = item.is_a?(Numeric) ? 'rjust' : 'ljust' + cells << item.to_s.send(justifier, widths[i]) + end + '| ' + cells.join(' | ') + ' |' + end + + def build_footer(nrows, elapsed) + rows_label = nrows == 1 ? 'row' : 'rows' + "#{nrows} #{rows_label} in set (%.2f sec)" % elapsed + end + end + # FIXME: re-enable the following once a "better" query_cache solution is in core # # The overrides below perform much better than the originals in AbstractAdapter diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 2f01fbb829..6b742ed858 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -390,6 +390,11 @@ module ActiveRecord true end + # Returns true. + def supports_explain? + true + end + # Returns the configured supported identifier length supported by PostgreSQL def table_alias_length @table_alias_length ||= query('SHOW max_identifier_length')[0][0].to_i @@ -514,9 +519,9 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== - def explain(arel) + def explain(arel, binds = []) sql = "EXPLAIN #{to_sql(arel)}" - ExplainPrettyPrinter.new.pp(exec_query(sql)) + ExplainPrettyPrinter.new.pp(exec_query(sql, 'EXPLAIN', binds)) end class ExplainPrettyPrinter # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index c11f82a33f..b8e91a2aea 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -122,6 +122,11 @@ module ActiveRecord true end + # Returns true. + def supports_explain? + true + end + def requires_reloading? true end @@ -219,9 +224,9 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== - def explain(arel) + def explain(arel, binds = []) sql = "EXPLAIN QUERY PLAN #{to_sql(arel)}" - ExplainPrettyPrinter.new.pp(exec_query(sql, 'EXPLAIN')) + ExplainPrettyPrinter.new.pp(exec_query(sql, 'EXPLAIN', binds)) end class ExplainPrettyPrinter diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb new file mode 100644 index 0000000000..4d013f0ef4 --- /dev/null +++ b/activerecord/lib/active_record/explain.rb @@ -0,0 +1,76 @@ +module ActiveRecord + module Explain # :nodoc: + # logging_query_plan calls could appear nested in the call stack. In + # particular this happens when a relation fetches its records, since + # that results in find_by_sql calls downwards. + # + # This flag allows nested calls to detect this situation and bypass + # it, thus preventing repeated EXPLAINs. + LOGGING_QUERY_PLAN = :logging_query_plan + + # If auto explain is enabled, this method triggers EXPLAIN logging for the + # queries triggered by the block if it takes more than the threshold as a + # whole. That is, the threshold is not checked against each individual + # query, but against the duration of the entire block. This approach is + # convenient for relations. + def logging_query_plan(&block) + if (t = auto_explain_threshold_in_seconds) && !Thread.current[LOGGING_QUERY_PLAN] + begin + Thread.current[LOGGING_QUERY_PLAN] = true + start = Time.now + result, sqls, binds = collecting_sqls_for_explain(&block) + logger.warn(exec_explain(sqls, binds)) if Time.now - start > t + result + ensure + Thread.current[LOGGING_QUERY_PLAN] = false + end + else + block.call + end + end + + # SCHEMA queries cannot be EXPLAINed, also we do not want to run EXPLAIN on + # our own EXPLAINs now matter how loopingly beautiful that would be. + SKIP_EXPLAIN_FOR = %w(SCHEMA EXPLAIN) + def ignore_explain_notification?(payload) + payload[:exception] || SKIP_EXPLAIN_FOR.include?(payload[:name]) + end + + # Collects all queries executed while the passed block runs. Returns an + # array with three elements, the result of the block, the strings with the + # queries, and their respective bindings. + def collecting_sqls_for_explain(&block) + sqls = [] + binds = [] + callback = lambda do |*args| + payload = args.last + unless ignore_explain_notification?(payload) + sqls << payload[:sql] + binds << payload[:binds] + end + end + + result = nil + ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do + result = block.call + end + + [result, sqls, binds] + end + + # Makes the adapter execute EXPLAIN for the given queries and bindings. + # Returns a formatted string ready to be logged. + def exec_explain(sqls, binds) + sqls.zip(binds).map do |sql, bind| + [].tap do |msg| + msg << "EXPLAIN for: #{sql}" + unless bind.empty? + bind_msg = bind.map {|col, val| [col.name, val]}.inspect + msg.last << " #{bind_msg}" + end + msg << connection.explain(sql, bind) + end.join("\n") + end.join("\n") + end + end +end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 47133e77e8..c2e31579a4 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -22,6 +22,13 @@ module ActiveRecord config.app_middleware.insert_after "::ActionDispatch::Callbacks", "ActiveRecord::ConnectionAdapters::ConnectionManagement" + config.action_dispatch.rescue_responses.merge!( + 'ActiveRecord::RecordNotFound' => :not_found, + 'ActiveRecord::StaleObjectError' => :conflict, + 'ActiveRecord::RecordInvalid' => :unprocessable_entity, + 'ActiveRecord::RecordNotSaved' => :unprocessable_entity + ) + rake_tasks do load "active_record/railties/databases.rake" end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 0c32ad5139..30cd8679fe 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -10,11 +10,12 @@ module ActiveRecord MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :bind] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reorder, :reverse_order, :uniq] - include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches + include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain # These are explicitly delegated to improve performance (avoids method_missing) delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to => :to_a - delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :column_hash,:to => :klass + delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, + :connection, :column_hash, :auto_explain_threshold_in_seconds, :to => :klass attr_reader :table, :klass, :loaded attr_accessor :extensions, :default_scoped @@ -144,23 +145,35 @@ module ActiveRecord super end + # Runs EXPLAIN on the query or queries triggered by this relation and + # returns the result as a string. The string is formatted imitating the + # ones printed by the database shell. + # + # Note that this method actually runs the queries, since the results of some + # are needed by the next ones when eager loading is going on. + # + # Please see further details in the + # {Active Record Query Interface guide}[http://edgeguides.rubyonrails.org/active_record_querying.html#running-explain]. def explain - queries = [] - callback = lambda do |*args| - payload = args.last - queries << payload[:sql] unless payload[:exception] || %w(SCHEMA EXPLAIN).include?(payload[:name]) - end + _, sqls, binds = collecting_sqls_for_explain { exec_query } + exec_explain(sqls, binds) + end - ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do - to_a + def to_a + # We monitor here the entire execution rather than individual SELECTs + # because from the point of view of the user fetching the records of a + # relation is a single unit of work. You want to know if this call takes + # too long, not if the individual queries take too long. + # + # It could be the case that none of the queries involved surpass the + # threshold, and at the same time the sum of them all does. The user + # should get a query plan logged in that case. + logging_query_plan do + exec_query end - - queries.map do |sql| - "EXPLAIN for: #{sql}\n#{@klass.connection.explain(sql)}" - end.join("\n") end - def to_a + def exec_query return @records if loaded? default_scoped = with_default_scope @@ -191,6 +204,7 @@ module ActiveRecord @loaded = true @records end + private :exec_query def as_json(options = nil) #:nodoc: to_a.as_json(options) @@ -543,6 +557,5 @@ module ActiveRecord # ignore raw_sql_ that is used by Oracle adapter as alias for limit/offset subqueries string.scan(/([a-zA-Z_][.\w]+).?\./).flatten.map{ |s| s.downcase }.uniq - ['raw_sql_'] end - end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 9300c57819..a7cad329e8 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -53,6 +53,12 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert Boolean.find(b4.id).attribute_present?(:value) end + def test_caching_nil_primary_key + klass = Class.new(Minimalistic) + klass.expects(:reset_primary_key).returns(nil).once + 2.times { klass.primary_key } + end + def test_attribute_keys_on_new_instance t = Topic.new assert_equal nil, t.title, "The topics table has a title column, so it should be nil" @@ -108,6 +114,11 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert !topic.respond_to?(:nothingness) end + def test_deprecated_underscore_method + topic = Topic.find(1) + assert_equal topic.title, assert_deprecated { topic._title } + end + def test_respond_to_with_custom_primary_key keyboard = Keyboard.create assert_not_nil keyboard.key_number @@ -675,7 +686,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase topic = Topic.new(:id => 5) topic.id = 5 - topic.method(:id).owner.send(:remove_method, :id) + topic.method(:id).owner.send(:undef_method, :id) assert_deprecated do assert_equal 5, topic.id @@ -686,17 +697,13 @@ class AttributeMethodsTest < ActiveRecord::TestCase private def cached_columns - @cached_columns ||= (time_related_columns_on_topic + serialized_columns_on_topic).map(&:name) + @cached_columns ||= time_related_columns_on_topic.map(&:name) end def time_related_columns_on_topic Topic.columns.select { |c| c.type.in?([:time, :date, :datetime, :timestamp]) } end - def serialized_columns_on_topic - Topic.columns.select { |c| Topic.serialized_attributes.include?(c.name) } - end - def in_time_zone(zone) old_zone = Time.zone old_tz = ActiveRecord::Base.time_zone_aware_attributes diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b1a429c869..d846eb03aa 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -570,10 +570,12 @@ class BasicsTest < ActiveRecord::TestCase weird = Weird.create('a$b' => 'value') weird.reload assert_equal 'value', weird.send('a$b') + assert_equal 'value', weird.read_attribute('a$b') weird.update_column('a$b', 'value2') weird.reload assert_equal 'value2', weird.send('a$b') + assert_equal 'value2', weird.read_attribute('a$b') end def test_multiparameter_attributes_on_date diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb new file mode 100644 index 0000000000..d3eb9c2cb2 --- /dev/null +++ b/activerecord/test/cases/explain_test.rb @@ -0,0 +1,91 @@ +require 'cases/helper' +require 'models/car' +require 'active_support/core_ext/string/strip' + +if ActiveRecord::Base.connection.supports_explain? + class ExplainTest < ActiveRecord::TestCase + fixtures :cars + + def base + ActiveRecord::Base + end + + def connection + base.connection + end + + def test_logging_query_plan + base.logger.expects(:warn).with do |value| + value.starts_with?('EXPLAIN for:') + end + + with_threshold(0) do + Car.where(:name => 'honda').all + end + end + + def test_collecting_sqls_for_explain + base.auto_explain_threshold_in_seconds = nil + honda = cars(:honda) + + expected_sqls = [] + expected_binds = [] + callback = lambda do |*args| + payload = args.last + unless base.ignore_explain_notification?(payload) + expected_sqls << payload[:sql] + expected_binds << payload[:binds] + end + end + + result = sqls = binds = nil + ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do + with_threshold(0) do + result, sqls, binds = base.collecting_sqls_for_explain { + Car.where(:name => 'honda').all + } + end + end + + assert_equal result, [honda] + assert_equal expected_sqls, sqls + assert_equal expected_binds, binds + end + + def test_exec_explain_with_no_binds + sqls = %w(foo bar) + binds = [[], []] + + connection.stubs(:explain).returns('query plan foo', 'query plan bar') + expected = sqls.map {|sql| "EXPLAIN for: #{sql}\nquery plan #{sql}"}.join("\n") + assert_equal expected, base.exec_explain(sqls, binds) + end + + def test_exec_explain_with_binds + cols = [Object.new, Object.new] + cols[0].expects(:name).returns('wadus') + cols[1].expects(:name).returns('chaflan') + + sqls = %w(foo bar) + binds = [[[cols[0], 1]], [[cols[1], 2]]] + + connection.stubs(:explain).returns("query plan foo\n", "query plan bar\n") + expected = <<-SQL.strip_heredoc + EXPLAIN for: #{sqls[0]} [["wadus", 1]] + query plan foo + + EXPLAIN for: #{sqls[1]} [["chaflan", 2]] + query plan bar + SQL + assert_equal expected, base.exec_explain(sqls, binds) + end + + def with_threshold(threshold) + current_threshold = base.auto_explain_threshold_in_seconds + base.auto_explain_threshold_in_seconds = threshold + yield + ensure + base.auto_explain_threshold_in_seconds = current_threshold + end + end +end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 3d6db91f81..be7edb858f 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -320,6 +320,8 @@ class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase assert_equal true, p1.frozen? assert_raises(ActiveRecord::RecordNotFound) { Person.find(p1.id) } assert_raises(ActiveRecord::RecordNotFound) { LegacyThing.find(t.id) } + ensure + remove_counter_column_from(Person, 'legacy_things_count') end private @@ -331,8 +333,8 @@ class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase model.update_all(col => 0) if current_adapter?(:OpenBaseAdapter) end - def remove_counter_column_from(model) - model.connection.remove_column model.table_name, :test_count + def remove_counter_column_from(model, col = :test_count) + model.connection.remove_column model.table_name, col model.reset_column_information end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index cc8ffb5f27..8d248c3f9f 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -23,6 +23,11 @@ class PrimaryKeysTest < ActiveRecord::TestCase assert_equal keyboard.to_key, [keyboard.id] end + def test_read_attribute_with_custom_primary_key + keyboard = Keyboard.create! + assert_equal keyboard.key_number, keyboard.read_attribute(:id) + end + def test_to_key_with_primary_key_after_destroy topic = Topic.find(1) topic.destroy diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index 074fd39e65..5a3f9a9711 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -24,9 +24,9 @@ class StoreTest < ActiveRecord::TestCase @john.settings[:icecream] = 'graeters' @john.save - assert 'graeters', @john.reload.settings[:icecream] + assert_equal 'graeters', @john.reload.settings[:icecream] end - + test "updating the store will mark it as changed" do @john.color = 'red' assert @john.settings_changed? diff --git a/activerecord/test/cases/yaml_serialization_test.rb b/activerecord/test/cases/yaml_serialization_test.rb index 0b54c309d1..5a38f2c6ee 100644 --- a/activerecord/test/cases/yaml_serialization_test.rb +++ b/activerecord/test/cases/yaml_serialization_test.rb @@ -18,6 +18,11 @@ class YamlSerializationTest < ActiveRecord::TestCase assert_equal topic, t end + def test_roundtrip_serialized_column + topic = Topic.new(:content => {:omg=>:lol}) + assert_equal({:omg=>:lol}, YAML.load(YAML.dump(topic)).content) + end + def test_encode_with_coder topic = Topic.first coder = {} diff --git a/activeresource/lib/active_resource/exceptions.rb b/activeresource/lib/active_resource/exceptions.rb index 6b953b28ad..51bede3bd0 100644 --- a/activeresource/lib/active_resource/exceptions.rb +++ b/activeresource/lib/active_resource/exceptions.rb @@ -33,35 +33,45 @@ module ActiveResource # 3xx Redirection class Redirection < ConnectionError # :nodoc: - def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end + def to_s + response['Location'] ? "#{super} => #{response['Location']}" : super + end end - # Raised when ... - class MissingPrefixParam < ArgumentError; end # :nodoc: + class MissingPrefixParam < ArgumentError # :nodoc: + end # 4xx Client Error - class ClientError < ConnectionError; end # :nodoc: + class ClientError < ConnectionError # :nodoc: + end # 400 Bad Request - class BadRequest < ClientError; end # :nodoc + class BadRequest < ClientError # :nodoc: + end # 401 Unauthorized - class UnauthorizedAccess < ClientError; end # :nodoc + class UnauthorizedAccess < ClientError # :nodoc: + end # 403 Forbidden - class ForbiddenAccess < ClientError; end # :nodoc + class ForbiddenAccess < ClientError # :nodoc: + end # 404 Not Found - class ResourceNotFound < ClientError; end # :nodoc: + class ResourceNotFound < ClientError # :nodoc: + end # 409 Conflict - class ResourceConflict < ClientError; end # :nodoc: + class ResourceConflict < ClientError # :nodoc: + end # 410 Gone - class ResourceGone < ClientError; end # :nodoc: + class ResourceGone < ClientError # :nodoc: + end # 5xx Server Error - class ServerError < ConnectionError; end # :nodoc: + class ServerError < ConnectionError # :nodoc: + end # 405 Method Not Allowed class MethodNotAllowed < ClientError # :nodoc: diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index be829222ff..7f1103a6d7 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,7 +1,9 @@ ## Rails 3.2.0 (unreleased) ## -* Module#synchronize is deprecated with no replacement. Please use `monitor` - from ruby's standard library. +* Added Enumerable#pluck to wrap the common pattern of collect(&:method) *DHH* + +* Module#synchronize is deprecated with no replacement. Please use `monitor` + from ruby's standard library. * (Date|DateTime|Time)#beginning_of_week accept an optional argument to be able to set the day at which weeks are assumed to start. diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 9343bb7106..d9856f2e84 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -63,6 +63,13 @@ module Enumerable end end + # Plucks the value of the passed method for each element and returns the result as an array. Example: + # + # people.pluck(:name) # => [ "David Heinemeier Hansson", "Jamie Heinemeier Hansson" ] + def pluck(method) + collect { |element| element.send(method) } + end + # Iterates over a collection, passing the current element *and* the # +memo+ to the block. Handy for building up hashes or # reducing collections down to one object. Examples: diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 1372e71a61..100b48312a 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -71,14 +71,20 @@ module ActiveSupport #:nodoc: # # This is handled by walking back up the watch stack and adding the constants # found by child.rb to the list of original constants in parent.rb - class WatchStack < Hash + class WatchStack + include Enumerable + # @watching is a stack of lists of constants being watched. For instance, # if parent.rb is autoloaded, the stack will look like [[Object]]. If parent.rb # then requires namespace/child.rb, the stack will look like [[Object], [Namespace]]. def initialize @watching = [] - super { |h,k| h[k] = [] } + @stack = Hash.new { |h,k| h[k] = [] } + end + + def each(&block) + @stack.each(&block) end # return a list of new constants found since the last call to watch_namespaces @@ -89,7 +95,7 @@ module ActiveSupport #:nodoc: @watching.last.each do |namespace| # Retrieve the constants that were present under the namespace when watch_namespaces # was originally called - original_constants = self[namespace].last + original_constants = @stack[namespace].last mod = Inflector.constantize(namespace) if Dependencies.qualified_const_defined?(namespace) next unless mod.is_a?(Module) @@ -102,7 +108,7 @@ module ActiveSupport #:nodoc: # element of self[Object] will be an Array of the constants that were present # before parent.rb was required. The second element will be an Array of the # constants that were present before child.rb was required. - self[namespace].each do |namespace_constants| + @stack[namespace].each do |namespace_constants| namespace_constants.concat(new_constants) end @@ -126,13 +132,14 @@ module ActiveSupport #:nodoc: Inflector.constantize(module_name).local_constant_names : [] watching << module_name - self[module_name] << original_constants + @stack[module_name] << original_constants end @watching << watching end + private def pop_modules(modules) - modules.each { |mod| self[mod].pop } + modules.each { |mod| @stack[mod].pop } end end diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index cdfa991a34..7ce7c4d52d 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -126,4 +126,11 @@ class EnumerableTests < Test::Unit::TestCase assert_equal true, GenericEnumerable.new([ 1 ]).exclude?(2) assert_equal false, GenericEnumerable.new([ 1 ]).exclude?(1) end -end + + def test_pluck_single_method + person = Struct.new(:name) + people = [ person.new("David"), person.new("Jamie") ] + + assert_equal [ "David", "Jamie" ], people.pluck(:name) + end +end
\ No newline at end of file diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 6b0be4c096..64abff6cb3 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,13 +1,20 @@ ## Rails 3.2.0 (unreleased) ## -* Display mounted engine's routes in `rake routes`. *Piotr Sarnacki* +* New applications get a flag + `config.active_record.auto_explain_threshold_in_seconds` in the evironments + configuration files. With a value of 0.5 in development.rb, and commented + out in production.rb. No mention in test.rb. *fxn* + +* Add DebugExceptions middleware which contains features extracted from ShowExceptions middleware *José Valim* + +* Display mounted engine's routes in `rake routes` *Piotr Sarnacki* * Allow to change the loading order of railties with `config.railties_order=` *Piotr Sarnacki* Example: config.railties_order = [Blog::Engine, :main_app, :all] -* Scaffold returns 204 No Content for API requests without content. This makes scaffold work with jQuery out of the box. *José Valim* +* Scaffold returns 204 No Content for API requests without content. This makes scaffold work with jQuery out of the box *José Valim* * Update Rails::Rack::Logger middleware to apply any tags set in config.log_tags to the newly ActiveSupport::TaggedLogging Rails.logger. This makes it easy to tag log lines with debug information like subdomain and request id -- both very helpful in debugging multi-user production applications *DHH* diff --git a/railties/guides/code/getting_started/config/environments/development.rb b/railties/guides/code/getting_started/config/environments/development.rb index 89932bf19b..d60a10568b 100644 --- a/railties/guides/code/getting_started/config/environments/development.rb +++ b/railties/guides/code/getting_started/config/environments/development.rb @@ -22,6 +22,13 @@ Blog::Application.configure do # Only use best-standards-support built into browsers config.action_dispatch.best_standards_support = :builtin + # Raise exception on mass assignment protection for ActiveRecord models + config.active_record.mass_assignment_sanitizer = :strict + + # Log the query plan for queries taking more than this (works + # with SQLite, MySQL, and PostgreSQL) + config.active_record.auto_explain_threshold_in_seconds = 0.5 + # Do not compress assets config.assets.compress = false diff --git a/railties/guides/code/getting_started/config/environments/production.rb b/railties/guides/code/getting_started/config/environments/production.rb index dee8acfdfe..c9b2f41c39 100644 --- a/railties/guides/code/getting_started/config/environments/production.rb +++ b/railties/guides/code/getting_started/config/environments/production.rb @@ -60,4 +60,8 @@ Blog::Application.configure do # Send deprecation notices to registered listeners config.active_support.deprecation = :notify + + # Log the query plan for queries taking more than this (works + # with SQLite, MySQL, and PostgreSQL) + # config.active_record.auto_explain_threshold_in_seconds = 0.5 end diff --git a/railties/guides/source/active_record_querying.textile b/railties/guides/source/active_record_querying.textile index 073f7c143d..352f23dc01 100644 --- a/railties/guides/source/active_record_querying.textile +++ b/railties/guides/source/active_record_querying.textile @@ -1148,11 +1148,26 @@ Client.connection.select_all("SELECT * FROM clients WHERE id = '1'") h3. +pluck+ -<tt>pluck</tt> can be used to query single column from table under model. It accepts column name as argument and returns Array of values of the specified column with corresponding data type. +<tt>pluck</tt> can be used to query a single column from the underlying table of a model. It accepts a column name as argument and returns an array of values of the specified column with the corresponding data type. <ruby> -Client.where(:active => true).pluck(:id) # SELECT id FROM clients WHERE clients.active -Client.uniq.pluck(:role) # SELECT DISTINCT role FROM clients +Client.where(:active => true).pluck(:id) +# SELECT id FROM clients WHERE active = 1 + +Client.uniq.pluck(:role) +# SELECT DISTINCT role FROM clients +</ruby> + ++pluck+ makes it possible to replace code like + +<ruby> +Client.select(:id).map { |c| c.id } +</ruby> + +with + +<ruby> +Client.pluck(:id) </ruby> h3. Existence of Objects diff --git a/railties/guides/source/configuring.textile b/railties/guides/source/configuring.textile index d91011c370..8e65dbccbb 100644 --- a/railties/guides/source/configuring.textile +++ b/railties/guides/source/configuring.textile @@ -268,6 +268,8 @@ h4. Configuring Active Record * +config.active_record.identity_map+ controls whether the identity map is enabled, and is false by default. +* +config.active_record.auto_explain_threshold_in_seconds+ configures the threshold for automatic EXPLAINs (+nil+ disables this feature). Queries exceeding the threshold get their query plan logged. Default is 0.5 in development mode. + The MySQL adapter adds one additional configuration option: * +ActiveRecord::ConnectionAdapters::MysqlAdapter.emulate_booleans+ controls whether Active Record will consider all +tinyint(1)+ columns in a MySQL database to be booleans and is true by default. diff --git a/railties/guides/source/form_helpers.textile b/railties/guides/source/form_helpers.textile index 821bb305f6..64eb2d8f36 100644 --- a/railties/guides/source/form_helpers.textile +++ b/railties/guides/source/form_helpers.textile @@ -229,7 +229,7 @@ The corresponding view +app/views/articles/new.html.erb+ using +form_for+ looks There are a few things to note here: # +@article+ is the actual object being edited. -# There is a single hash of options. Routing options are passed in the +:url+ hash, HTML options are passed in the +:html+ hash. +# There is a single hash of options. Routing options are passed in the +:url+ hash, HTML options are passed in the +:html+ hash. Also you can provide a +:namespace+ option for your form to ensure uniqueness of id attributes on form elements. The namespace attribute will be prefixed with underscore on the generated HTML id. # The +form_for+ method yields a *form builder* object (the +f+ variable). # Methods to create form controls are called *on* the form builder object +f+ diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 2a62446a04..f5f47acfbc 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -195,10 +195,13 @@ module Rails middleware.use ::ActionDispatch::RequestId middleware.use ::Rails::Rack::Logger, config.log_tags # must come after Rack::MethodOverride to properly log overridden methods middleware.use ::ActionDispatch::ShowExceptions + middleware.use ::ActionDispatch::DebugExceptions middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies + if config.action_dispatch.x_sendfile_header.present? middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header end + middleware.use ::ActionDispatch::Reloader unless config.cache_classes middleware.use ::ActionDispatch::Callbacks middleware.use ::ActionDispatch::Cookies diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt index 47078e3af9..beaf941282 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt @@ -25,6 +25,10 @@ <%- unless options.skip_active_record? -%> # Raise exception on mass assignment protection for ActiveRecord models config.active_record.mass_assignment_sanitizer = :strict + + # Log the query plan for queries taking more than this (works + # with SQLite, MySQL, and PostgreSQL) + config.active_record.auto_explain_threshold_in_seconds = 0.5 <%- end -%> # Do not compress assets diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 50f2df3d35..40ec3fc644 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -60,4 +60,10 @@ # Send deprecation notices to registered listeners config.active_support.deprecation = :notify + + <%- unless options.skip_active_record? -%> + # Log the query plan for queries taking more than this (works + # with SQLite, MySQL, and PostgreSQL) + # config.active_record.auto_explain_threshold_in_seconds = 0.5 + <%- end -%> end diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index d4ffbe3d66..a22013f81c 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -451,6 +451,28 @@ module ApplicationTests assert_equal 0, files.length, "Expected application.js asset to be removed, but still exists" end + test "asset urls should use the request's protocol by default" do + app_with_assets_in_view + add_to_config "config.asset_host = 'example.com'" + require "#{app_path}/config/environment" + class ::PostsController < ActionController::Base; end + + get '/posts', {}, {'HTTPS'=>'off'} + assert_match('src="http://example.com/assets/application.js', last_response.body) + get '/posts', {}, {'HTTPS'=>'on'} + assert_match('src="https://example.com/assets/application.js', last_response.body) + end + + test "asset urls should be protocol-relative if no request is in scope" do + app_file "app/assets/javascripts/image_loader.js.erb", 'var src="<%= image_path("rails.png") %>";' + add_to_config "config.assets.precompile = %w{image_loader.js}" + add_to_config "config.asset_host = 'example.com'" + precompile! + + assert_match 'src="//example.com/assets/rails.png"', File.read("#{app_path}/public/assets/image_loader.js") + end + + private def app_with_assets_in_view diff --git a/railties/test/application/middleware/exceptions_test.rb b/railties/test/application/middleware/exceptions_test.rb new file mode 100644 index 0000000000..0174352900 --- /dev/null +++ b/railties/test/application/middleware/exceptions_test.rb @@ -0,0 +1,89 @@ +# encoding: utf-8 +require 'isolation/abstract_unit' +require 'rack/test' + +module ApplicationTests + class MiddlewareExceptionsTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + include Rack::Test::Methods + + def setup + build_app + boot_rails + end + + def teardown + teardown_app + end + + test "show exceptions middleware filter backtrace before logging" do + my_middleware = Struct.new(:app) do + def call(env) + raise "Failure" + end + end + + app.config.middleware.use my_middleware + + stringio = StringIO.new + Rails.logger = Logger.new(stringio) + + get "/" + assert_no_match(/action_dispatch/, stringio.string) + end + + test "renders active record exceptions as 404" do + my_middleware = Struct.new(:app) do + def call(env) + raise ActiveRecord::RecordNotFound + end + end + + app.config.middleware.use my_middleware + + get "/" + assert_equal 404, last_response.status + end + + test "unspecified route when set action_dispatch.show_exceptions to false" do + app.config.action_dispatch.show_exceptions = false + + assert_raise(ActionController::RoutingError) do + get '/foo' + end + end + + test "unspecified route when set action_dispatch.show_exceptions to true" do + app.config.action_dispatch.show_exceptions = true + + assert_nothing_raised(ActionController::RoutingError) do + get '/foo' + end + end + + test "displays diagnostics message when exception raised in template that contains UTF-8" do + app.config.action_dispatch.show_exceptions = true + + controller :foo, <<-RUBY + class FooController < ActionController::Base + def index + end + end + RUBY + + app_file 'app/views/foo/index.html.erb', <<-ERB + <% raise 'boooom' %> + ✓ + ERB + + app_file 'config/routes.rb', <<-RUBY + AppTemplate::Application.routes.draw do + match ':controller(/:action)' + end + RUBY + + post '/foo', :utf8 => '✓' + assert_match(/boooom/, last_response.body) + end + end +end diff --git a/railties/test/application/middleware/show_exceptions_test.rb b/railties/test/application/middleware/show_exceptions_test.rb deleted file mode 100644 index e3f27f63c3..0000000000 --- a/railties/test/application/middleware/show_exceptions_test.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'isolation/abstract_unit' - -module ApplicationTests - class ShowExceptionsTest < Test::Unit::TestCase - include ActiveSupport::Testing::Isolation - - def setup - build_app - boot_rails - FileUtils.rm_rf "#{app_path}/config/environments" - end - - def teardown - teardown_app - end - - def app - @app ||= Rails.application - end - - test "unspecified route when set action_dispatch.show_exceptions to false" do - make_basic_app do |app| - app.config.action_dispatch.show_exceptions = false - end - - assert_raise(ActionController::RoutingError) do - get '/foo' - end - end - - test "unspecified route when set action_dispatch.show_exceptions to true" do - make_basic_app do |app| - app.config.action_dispatch.show_exceptions = true - end - - assert_nothing_raised(ActionController::RoutingError) do - get '/foo' - end - end - end -end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 4703a59326..578370cfca 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -33,6 +33,7 @@ module ApplicationTests "ActionDispatch::RequestId", "Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods "ActionDispatch::ShowExceptions", + "ActionDispatch::DebugExceptions", "ActionDispatch::RemoteIp", "Rack::Sendfile", "ActionDispatch::Reloader", @@ -104,10 +105,11 @@ module ApplicationTests assert !middleware.include?("ActionDispatch::Static") end - test "includes show exceptions even action_dispatch.show_exceptions is disabled" do + test "includes exceptions middlewares even if action_dispatch.show_exceptions is disabled" do add_to_config "config.action_dispatch.show_exceptions = false" boot! assert middleware.include?("ActionDispatch::ShowExceptions") + assert middleware.include?("ActionDispatch::DebugExceptions") end test "removes ActionDispatch::Reloader if cache_classes is true" do @@ -191,26 +193,6 @@ module ApplicationTests assert_equal nil, last_response.headers["Etag"] end - # Show exceptions middleware - test "show exceptions middleware filter backtrace before logging" do - my_middleware = Struct.new(:app) do - def call(env) - raise "Failure" - end - end - - make_basic_app do |app| - app.config.middleware.use my_middleware - end - - stringio = StringIO.new - Rails.logger = Logger.new(stringio) - - env = Rack::MockRequest.env_for("/") - Rails.application.call(env) - assert_no_match(/action_dispatch/, stringio.string) - end - private def boot! diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 235c08e38e..41bb4d8b20 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -336,6 +336,15 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_generated_environments_file_for_auto_explain + run_generator [destination_root, "--skip-active-record"] + %w(development test production).each do |env| + assert_file "config/environments/#{env}.rb" do |file| + assert_no_match %r(auto_explain_threshold_in_seconds), file + end + end + end + protected def action(*args, &block) |