From 3900f4007ee6463b8936af23c04017a900673866 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 2 May 2009 14:23:44 -0500 Subject: Deprecate assert_redirect_to's partial hash matching --- actionpack/lib/action_controller/base/base.rb | 1 - actionpack/lib/action_controller/base/redirect.rb | 4 ---- actionpack/lib/action_controller/testing/process.rb | 18 +++++++++--------- actionpack/lib/action_dispatch/http/response.rb | 2 -- .../lib/action_dispatch/testing/assertions/response.rb | 9 ++------- .../lib/action_dispatch/testing/test_response.rb | 6 ++++++ .../test/controller/action_pack_assertions_test.rb | 8 +++++--- actionpack/test/controller/redirect_test.rb | 6 ++++-- 8 files changed, 26 insertions(+), 28 deletions(-) diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb index 99b5963891..ef97986c0e 100644 --- a/actionpack/lib/action_controller/base/base.rb +++ b/actionpack/lib/action_controller/base/base.rb @@ -820,7 +820,6 @@ module ActionController #:nodoc: @template = ActionView::Base.new(self.class.view_paths, {}, self, formats) response.template = @template if response.respond_to?(:template=) @template.helpers.send :include, self.class.master_helper_module - response.redirected_to = nil @performed_render = @performed_redirect = false end diff --git a/actionpack/lib/action_controller/base/redirect.rb b/actionpack/lib/action_controller/base/redirect.rb index 2e92117e7c..4849caac0a 100644 --- a/actionpack/lib/action_controller/base/redirect.rb +++ b/actionpack/lib/action_controller/base/redirect.rb @@ -48,8 +48,6 @@ module ActionController status = 302 end - response.redirected_to = options - case options # The scheme name consist of a letter followed by any combination of # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") @@ -82,8 +80,6 @@ module ActionController # The response body is not reset here, see +erase_render_results+ def erase_redirect_results #:nodoc: @performed_redirect = false - response.redirected_to = nil - response.redirected_to_method_params = nil response.status = DEFAULT_RENDER_STATUS_CODE response.headers.delete('Location') end diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index 8315a160ad..f62d18a1f9 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -45,17 +45,16 @@ module ActionController #:nodoc: end end - # Integration test methods such as ActionController::Integration::Session#get - # and ActionController::Integration::Session#post return objects of class - # TestResponse, which represent the HTTP response results of the requested - # controller actions. - # - # See Response for more information on controller response objects. class TestResponse < ActionDispatch::TestResponse def recycle! - body_parts.clear - headers.delete('ETag') - headers.delete('Last-Modified') + @status = 200 + @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS) + @writer = lambda { |x| @body << x } + @block = nil + @length = 0 + @body = [] + + @request = @template = nil end end @@ -132,6 +131,7 @@ module ActionController #:nodoc: build_request_uri(action, parameters) Base.class_eval { include ProcessWithTest } unless Base < ProcessWithTest + @controller.process(@request, @response) end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 2b969323ca..2618e284fe 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -34,8 +34,6 @@ module ActionDispatch # :nodoc: DEFAULT_HEADERS = { "Cache-Control" => "no-cache" } attr_accessor :request - attr_accessor :redirected_to, :redirected_to_method_params - attr_writer :header alias_method :headers=, :header= diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index a72ce9084f..92e3da580e 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -60,14 +60,9 @@ module ActionDispatch validate_request! assert_response(:redirect, message) - return true if options == @response.redirected_to + return true if options == @response.location - # Support partial arguments for hash redirections - if options.is_a?(Hash) && @response.redirected_to.is_a?(Hash) - return true if options.all? {|(key, value)| @response.redirected_to[key] == value} - end - - redirected_to_after_normalisation = normalize_argument_to_redirection(@response.redirected_to) + redirected_to_after_normalisation = normalize_argument_to_redirection(@response.location) options_after_normalisation = normalize_argument_to_redirection(options) if redirected_to_after_normalisation != options_after_normalisation diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 50c6d85828..c5464f1559 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -1,4 +1,10 @@ module ActionDispatch + # Integration test methods such as ActionController::Integration::Session#get + # and ActionController::Integration::Session#post return objects of class + # TestResponse, which represent the HTTP response results of the requested + # controller actions. + # + # See Response for more information on controller response objects. class TestResponse < Response def self.from_response(response) new.tap do |resp| diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 711640f9a9..60957a2f0e 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -514,9 +514,11 @@ class ActionPackHeaderTest < ActionController::TestCase end def test_rendering_xml_respects_content_type - @response.headers['type'] = 'application/pdf' - process :hello_xml_world - assert_equal('application/pdf; charset=utf-8', @response.headers['Content-Type']) + pending do + @response.headers['type'] = 'application/pdf' + process :hello_xml_world + assert_equal('application/pdf; charset=utf-8', @response.headers['Content-Type']) + end end def test_render_text_with_custom_content_type diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 91e21db854..9ad63a1c60 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -234,8 +234,10 @@ class RedirectTest < ActionController::TestCase end def test_redirect_with_partial_params - get :module_redirect - assert_redirected_to :action => 'hello_world' + pending do + get :module_redirect + assert_redirected_to :action => 'hello_world' + end end def test_redirect_to_nil -- cgit v1.2.3 From a8b75c480fc9774252f5976dcf1a614079822e56 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 2 May 2009 14:57:40 -0500 Subject: Functional test runner finalizes response just like the integration test runner. In both runners, the @response object will now behave the same. Some functional tests will need to be updated if they are relying on preprocessed data on the response. --- .../lib/action_controller/testing/process.rb | 5 ++- actionpack/lib/action_dispatch/http/response.rb | 2 +- .../lib/action_dispatch/testing/test_response.rb | 18 ++++------ actionpack/test/controller/cookie_test.rb | 17 +++++---- actionpack/test/controller/render_test.rb | 22 ++++++------ actionpack/test/controller/send_file_test.rb | 8 ++--- actionpack/test/controller/test_test.rb | 4 ++- actionpack/test/template/body_parts_test.rb | 6 +++- actionpack/test/template/output_buffer_test.rb | 42 ++++++++++++---------- 9 files changed, 66 insertions(+), 58 deletions(-) diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index f62d18a1f9..b44ec2f94b 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -132,7 +132,10 @@ module ActionController #:nodoc: Base.class_eval { include ProcessWithTest } unless Base < ProcessWithTest - @controller.process(@request, @response) + response = Rack::MockResponse.new(*@controller.process(@request, ActionDispatch::Response.new).to_a) + @response.request, @response.template = @request, @controller.template + @response.status, @response.headers, @response.body = response.status, response.headers, response.body + @response end def xml_http_request(request_method, action, parameters = nil, session = nil, flash = nil) diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 2618e284fe..133b4f7335 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -185,7 +185,7 @@ module ActionDispatch # :nodoc: @writer = lambda { |x| callback.call(x) } @body.call(self, self) else - @body.each(&callback) + @body.each { |part| callback.call(part.to_s) } end @writer = callback diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index c5464f1559..c35982e075 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -93,6 +93,12 @@ module ActionDispatch ActiveSupport::Deprecation.warn("response.has_template_object? has been deprecated. Use tempate.assigns[name].nil? instead", caller) !template_objects[name].nil? end + + # Returns binary content (downloadable file), converted to a String + def binary_content + ActiveSupport::Deprecation.warn("response.binary_content has been deprecated. Use response.body instead", caller) + body + end end include DeprecatedHelpers @@ -121,17 +127,5 @@ module ActionDispatch def client_error? (400..499).include?(response_code) end - - # Returns binary content (downloadable file), converted to a String - def binary_content - raise "Response body is not a Proc: #{body_parts.inspect}" unless body_parts.kind_of?(Proc) - require 'stringio' - - sio = StringIO.new - body_parts.call(self, sio) - - sio.rewind - sio.read - end end end diff --git a/actionpack/test/controller/cookie_test.rb b/actionpack/test/controller/cookie_test.rb index c861982698..0f22714071 100644 --- a/actionpack/test/controller/cookie_test.rb +++ b/actionpack/test/controller/cookie_test.rb @@ -54,39 +54,38 @@ class CookieTest < ActionController::TestCase def test_setting_cookie get :authenticate - assert_equal ["user_name=david; path=/"], @response.headers["Set-Cookie"] + assert_equal "user_name=david; path=/", @response.headers["Set-Cookie"] assert_equal({"user_name" => "david"}, @response.cookies) end def test_setting_with_escapable_characters get :set_with_with_escapable_characters - assert_equal ["that+%26+guy=foo+%26+bar+%3D%3E+baz; path=/"], @response.headers["Set-Cookie"] + assert_equal "that+%26+guy=foo+%26+bar+%3D%3E+baz; path=/", @response.headers["Set-Cookie"] assert_equal({"that & guy" => "foo & bar => baz"}, @response.cookies) end def test_setting_cookie_for_fourteen_days get :authenticate_for_fourteen_days - assert_equal ["user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal "user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT", @response.headers["Set-Cookie"] assert_equal({"user_name" => "david"}, @response.cookies) end def test_setting_cookie_for_fourteen_days_with_symbols get :authenticate_for_fourteen_days_with_symbols - assert_equal ["user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal "user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT", @response.headers["Set-Cookie"] assert_equal({"user_name" => "david"}, @response.cookies) end def test_setting_cookie_with_http_only get :authenticate_with_http_only - assert_equal ["user_name=david; path=/; HttpOnly"], @response.headers["Set-Cookie"] + assert_equal "user_name=david; path=/; HttpOnly", @response.headers["Set-Cookie"] assert_equal({"user_name" => "david"}, @response.cookies) end def test_multiple_cookies get :set_multiple_cookies assert_equal 2, @response.cookies.size - assert_equal "user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT", @response.headers["Set-Cookie"][0] - assert_equal "login=XJ-122; path=/", @response.headers["Set-Cookie"][1] + assert_equal "user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT\nlogin=XJ-122; path=/", @response.headers["Set-Cookie"] assert_equal({"login" => "XJ-122", "user_name" => "david"}, @response.cookies) end @@ -96,7 +95,7 @@ class CookieTest < ActionController::TestCase def test_expiring_cookie get :logout - assert_equal ["user_name=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal "user_name=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", @response.headers["Set-Cookie"] assert_equal({"user_name" => nil}, @response.cookies) end @@ -117,6 +116,6 @@ class CookieTest < ActionController::TestCase def test_delete_cookie_with_path get :delete_cookie_with_path - assert_equal ["user_name=; path=/beaten; expires=Thu, 01-Jan-1970 00:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal "user_name=; path=/beaten; expires=Thu, 01-Jan-1970 00:00:00 GMT", @response.headers["Set-Cookie"] end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 023bf0eeaa..dd2dd7a502 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1293,15 +1293,15 @@ class RenderTest < ActionController::TestCase def test_head_with_symbolic_status get :head_with_symbolic_status, :status => "ok" - assert_equal "200 OK", @response.status + assert_equal 200, @response.status assert_response :ok get :head_with_symbolic_status, :status => "not_found" - assert_equal "404 Not Found", @response.status + assert_equal 404, @response.status assert_response :not_found get :head_with_symbolic_status, :status => "no_content" - assert_equal "204 No Content", @response.status + assert_equal 204, @response.status assert !@response.headers.include?('Content-Length') assert_response :no_content @@ -1322,7 +1322,7 @@ class RenderTest < ActionController::TestCase def test_head_with_string_status get :head_with_string_status, :status => "404 Eat Dirt" assert_equal 404, @response.response_code - assert_equal "Eat Dirt", @response.message + assert_equal "Not Found", @response.message assert_response :not_found end @@ -1590,7 +1590,7 @@ class EtagRenderTest < ActionController::TestCase def test_render_against_etag_request_should_304_when_match @request.if_none_match = etag_for("hello david") get :render_hello_world_from_variable - assert_equal "304 Not Modified", @response.status + assert_equal 304, @response.status assert @response.body.empty? end @@ -1603,13 +1603,13 @@ class EtagRenderTest < ActionController::TestCase def test_render_against_etag_request_should_200_when_no_match @request.if_none_match = etag_for("hello somewhere else") get :render_hello_world_from_variable - assert_equal "200 OK", @response.status + assert_equal 200, @response.status assert !@response.body.empty? end def test_render_should_not_set_etag_when_last_modified_has_been_specified get :render_hello_world_with_last_modified_set - assert_equal "200 OK", @response.status + assert_equal 200, @response.status assert_not_nil @response.last_modified assert_nil @response.etag assert @response.body.present? @@ -1623,11 +1623,11 @@ class EtagRenderTest < ActionController::TestCase @request.if_none_match = expected_etag get :render_hello_world_from_variable - assert_equal "304 Not Modified", @response.status + assert_equal 304, @response.status @request.if_none_match = "\"diftag\"" get :render_hello_world_from_variable - assert_equal "200 OK", @response.status + assert_equal 200, @response.status end def render_with_404_shouldnt_have_etag @@ -1695,7 +1695,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified @request.if_modified_since = @last_modified get :conditional_hello - assert_equal "304 Not Modified", @response.status + assert_equal 304, @response.status assert @response.body.blank?, @response.body assert_equal @last_modified, @response.headers['Last-Modified'] end @@ -1710,7 +1710,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_modified @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' get :conditional_hello - assert_equal "200 OK", @response.status + assert_equal 200, @response.status assert !@response.body.blank? assert_equal @last_modified, @response.headers['Last-Modified'] end diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 2e14a0a32c..6007ebef7a 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -44,12 +44,12 @@ class SendFileTest < ActionController::TestCase response = nil assert_nothing_raised { response = process('file') } assert_not_nil response - assert_kind_of Proc, response.body_parts + assert_kind_of Array, response.body_parts require 'stringio' output = StringIO.new output.binmode - assert_nothing_raised { response.body_parts.call(response, output) } + assert_nothing_raised { response.body_parts.each { |part| output << part.to_s } } assert_equal file_data, output.string end @@ -149,13 +149,13 @@ class SendFileTest < ActionController::TestCase define_method "test_send_#{method}_status" do @controller.options = { :stream => false, :status => 500 } assert_nothing_raised { assert_not_nil process(method) } - assert_equal '500 Internal Server Error', @response.status + assert_equal 500, @response.status end define_method "test_default_send_#{method}_status" do @controller.options = { :stream => false } assert_nothing_raised { assert_not_nil process(method) } - assert_equal ActionController::DEFAULT_RENDER_STATUS_CODE, @response.status + assert_equal 200, @response.status end end end diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 919f9815ec..9e88188b9a 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -614,7 +614,9 @@ XML def test_binary_content_works_with_send_file get :test_send_file - assert_nothing_raised(NoMethodError) { @response.binary_content } + assert_deprecated do + assert_nothing_raised(NoMethodError) { @response.binary_content } + end end protected diff --git a/actionpack/test/template/body_parts_test.rb b/actionpack/test/template/body_parts_test.rb index 5be8533293..e17092a452 100644 --- a/actionpack/test/template/body_parts_test.rb +++ b/actionpack/test/template/body_parts_test.rb @@ -16,7 +16,11 @@ class BodyPartsTest < ActionController::TestCase def test_body_parts get :index - assert_equal RENDERINGS, @response.body_parts + pending do + # TestProcess buffers body_parts into body + # TODO: Rewrite test w/o going through process + assert_equal RENDERINGS, @response.body_parts + end assert_equal RENDERINGS.join, @response.body end end diff --git a/actionpack/test/template/output_buffer_test.rb b/actionpack/test/template/output_buffer_test.rb index bc17f36783..3faea64db3 100644 --- a/actionpack/test/template/output_buffer_test.rb +++ b/actionpack/test/template/output_buffer_test.rb @@ -10,26 +10,32 @@ class OutputBufferTest < ActionController::TestCase tests TestController def test_flush_output_buffer - # Start with the default body parts - get :index - assert_equal ['foo'], @response.body_parts - assert_nil @controller.template.output_buffer + pending do + # TODO: This tests needs to be rewritten due + # The @response is not the same response object assigned + # to the @controller.template - # Nil output buffer is skipped - @controller.template.flush_output_buffer - assert_nil @controller.template.output_buffer - assert_equal ['foo'], @response.body_parts + # Start with the default body parts + get :index + assert_equal ['foo'], @response.body_parts + assert_nil @controller.template.output_buffer - # Empty output buffer is skipped - @controller.template.output_buffer = '' - @controller.template.flush_output_buffer - assert_equal '', @controller.template.output_buffer - assert_equal ['foo'], @response.body_parts + # Nil output buffer is skipped + @controller.template.flush_output_buffer + assert_nil @controller.template.output_buffer + assert_equal ['foo'], @response.body_parts - # Flushing appends the output buffer to the body parts - @controller.template.output_buffer = 'bar' - @controller.template.flush_output_buffer - assert_equal '', @controller.template.output_buffer - assert_equal ['foo', 'bar'], @response.body_parts + # Empty output buffer is skipped + @controller.template.output_buffer = '' + @controller.template.flush_output_buffer + assert_equal '', @controller.template.output_buffer + assert_equal ['foo'], @response.body_parts + + # Flushing appends the output buffer to the body parts + @controller.template.output_buffer = 'bar' + @controller.template.flush_output_buffer + assert_equal '', @controller.template.output_buffer + assert_equal ['foo', 'bar'], @response.body_parts + end end end -- cgit v1.2.3 From f32cf44870549c6cc5b6e6c84cffc1facf6ec38e Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 2 May 2009 15:29:18 -0500 Subject: Switch functional tests to run through the rack interface instead of process --- actionpack/lib/action_controller/base/base.rb | 12 ++++++++---- actionpack/lib/action_controller/dispatch/dispatcher.rb | 2 +- actionpack/lib/action_controller/dispatch/rescue.rb | 2 +- actionpack/lib/action_controller/routing/route_set.rb | 2 +- actionpack/lib/action_controller/testing/process.rb | 13 +++++++++++-- actionpack/lib/action_dispatch/testing/test_request.rb | 5 +++++ actionpack/test/controller/caching_test.rb | 2 +- 7 files changed, 28 insertions(+), 10 deletions(-) diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb index ef97986c0e..eb65f59ddd 100644 --- a/actionpack/lib/action_controller/base/base.rb +++ b/actionpack/lib/action_controller/base/base.rb @@ -371,10 +371,7 @@ module ActionController #:nodoc: class << self def call(env) - # HACK: For global rescue to have access to the original request and response - request = env["action_controller.rescue.request"] ||= ActionDispatch::Request.new(env) - response = env["action_controller.rescue.response"] ||= ActionDispatch::Response.new - process(request, response) + new.call(env) end # Factory for the standard create, process loop where the controller is discarded after processing. @@ -511,6 +508,13 @@ module ActionController #:nodoc: end public + def call(env) + # HACK: For global rescue to have access to the original request and response + request = env["action_dispatch.rescue.request"] ||= ActionDispatch::Request.new(env) + response = env["action_dispatch.rescue.response"] ||= ActionDispatch::Response.new + process(request, response).to_a + end + # Extracts the action_name from the request parameters and performs that action. def process(request, response, method = :perform_action, *arguments) #:nodoc: response.request = request diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb index bb9d8bd063..25844bf2a2 100644 --- a/actionpack/lib/action_controller/dispatch/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatch/dispatcher.rb @@ -80,7 +80,7 @@ module ActionController Routing::Routes.call(env) rescue Exception => exception if controller ||= (::ApplicationController rescue Base) - controller.call_with_exception(env, exception).to_a + controller.call_with_exception(env, exception) else raise exception end diff --git a/actionpack/lib/action_controller/dispatch/rescue.rb b/actionpack/lib/action_controller/dispatch/rescue.rb index df80ac0909..2f3b40c231 100644 --- a/actionpack/lib/action_controller/dispatch/rescue.rb +++ b/actionpack/lib/action_controller/dispatch/rescue.rb @@ -62,7 +62,7 @@ module ActionController #:nodoc: def call_with_exception(env, exception) #:nodoc: request = env["action_controller.rescue.request"] ||= ActionDispatch::Request.new(env) response = env["action_controller.rescue.response"] ||= ActionDispatch::Response.new - new.process(request, response, :rescue_action, exception) + new.process(request, response, :rescue_action, exception).to_a end end diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb index 70cd1f642d..172b867bf0 100644 --- a/actionpack/lib/action_controller/routing/route_set.rb +++ b/actionpack/lib/action_controller/routing/route_set.rb @@ -430,7 +430,7 @@ module ActionController def call(env) request = ActionDispatch::Request.new(env) app = Routing::Routes.recognize(request) - app.call(env).to_a + app.call(env) end def recognize(request) diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index b44ec2f94b..49e8322491 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -35,12 +35,13 @@ module ActionController #:nodoc: end data = params.to_query - @env['CONTENT_LENGTH'] = data.length + @env['CONTENT_LENGTH'] = data.length.to_s @env['rack.input'] = StringIO.new(data) end def recycle! @env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ } + @env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ } @env['action_dispatch.request.query_parameters'] = {} end end @@ -132,7 +133,15 @@ module ActionController #:nodoc: Base.class_eval { include ProcessWithTest } unless Base < ProcessWithTest - response = Rack::MockResponse.new(*@controller.process(@request, ActionDispatch::Response.new).to_a) + env = @request.env + app = @controller + + # TODO: Enable Lint + # app = Rack::Lint.new(app) + + status, headers, body = app.call(env) + response = Rack::MockResponse.new(status, headers, body) + @response.request, @response.template = @request, @controller.template @response.status, @response.headers, @response.body = response.status, response.headers, response.body @response diff --git a/actionpack/lib/action_dispatch/testing/test_request.rb b/actionpack/lib/action_dispatch/testing/test_request.rb index 5d8cd7e619..20288aa7a5 100644 --- a/actionpack/lib/action_dispatch/testing/test_request.rb +++ b/actionpack/lib/action_dispatch/testing/test_request.rb @@ -16,6 +16,7 @@ module ActionDispatch def env write_cookies! + delete_nil_values! super end @@ -74,5 +75,9 @@ module ActionDispatch @env['HTTP_COOKIE'] = @cookies.map { |name, value| "#{name}=#{value};" }.join(' ') end end + + def delete_nil_values! + @env.delete_if { |k, v| v.nil? } + end end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 10b904481d..6b9d1056a3 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -111,7 +111,7 @@ class PageCachingTest < ActionController::TestCase end def test_should_cache_ok_at_custom_path - @request.stubs(:path).returns("/index.html") + @request.request_uri = "/index.html" get :ok assert_response :ok assert File.exist?("#{FILE_STORE_PATH}/index.html") -- cgit v1.2.3 From 24affdc88c4a4af03ce1ec5b23c3def18b90effe Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 2 May 2009 15:37:29 -0500 Subject: Deprecate Controller.process interface --- actionpack/lib/action_controller/base/base.rb | 1 + actionpack/test/controller/filters_test.rb | 2 +- actionpack/test/controller/helper_test.rb | 20 +++++++++++--------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb index eb65f59ddd..60567454b1 100644 --- a/actionpack/lib/action_controller/base/base.rb +++ b/actionpack/lib/action_controller/base/base.rb @@ -376,6 +376,7 @@ module ActionController #:nodoc: # Factory for the standard create, process loop where the controller is discarded after processing. def process(request, response) #:nodoc: + ActiveSupport::Deprecation.warn("Controller.process has been deprecated. Use Controller.call instead", caller) new.process(request, response) end diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 9e74538310..5876f55420 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -603,7 +603,7 @@ class FilterTest < Test::Unit::TestCase %w(foo bar baz).each do |action| request = ActionController::TestRequest.new request.query_parameters[:choose] = action - response = DynamicDispatchController.process(request, ActionController::TestResponse.new) + response = Rack::MockResponse.new(*DynamicDispatchController.call(request.env)) assert_equal action, response.body end end diff --git a/actionpack/test/controller/helper_test.rb b/actionpack/test/controller/helper_test.rb index 58addc123d..e72bce1791 100644 --- a/actionpack/test/controller/helper_test.rb +++ b/actionpack/test/controller/helper_test.rb @@ -102,19 +102,19 @@ class HelperTest < Test::Unit::TestCase end def test_helper_for_nested_controller - request = ActionController::TestRequest.new - response = ActionController::TestResponse.new + request = ActionController::TestRequest.new request.action = 'render_hello_world' - assert_equal 'hello: Iz guuut!', Fun::GamesController.process(request, response).body + response = Rack::MockResponse.new(*Fun::GamesController.call(request.env)) + assert_equal 'hello: Iz guuut!', response.body end def test_helper_for_acronym_controller - request = ActionController::TestRequest.new - response = ActionController::TestResponse.new + request = ActionController::TestRequest.new request.action = 'test' - assert_equal 'test: baz', Fun::PdfController.process(request, response).body + response = Rack::MockResponse.new(*Fun::PdfController.call(request.env)) + assert_equal 'test: baz', response.body end def test_all_helpers @@ -211,14 +211,16 @@ class IsolatedHelpersTest < Test::Unit::TestCase end def test_helper_in_a - assert_raise(ActionView::TemplateError) { A.process(@request, @response) } + assert_raise(ActionView::TemplateError) { A.call(@request.env) } end def test_helper_in_b - assert_equal 'B', B.process(@request, @response).body + response = Rack::MockResponse.new(*B.call(@request.env)) + assert_equal 'B', response.body end def test_helper_in_c - assert_equal 'C', C.process(@request, @response).body + response = Rack::MockResponse.new(*C.call(@request.env)) + assert_equal 'C', response.body end end -- cgit v1.2.3 From 945bf9c254b5bfb56df874c1a3f7f0f1e89dc8b8 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 2 May 2009 15:16:12 -0700 Subject: Check for sibling Active Support first --- activerecord/lib/active_record.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 500a90d6cb..1982ffc791 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -21,15 +21,10 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #++ -begin - require 'active_support' -rescue LoadError - activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" - if File.directory?(activesupport_path) - $:.unshift activesupport_path - require 'active_support' - end +"#{File.dirname(__FILE__)}/../../activesupport/lib".tap do |path| + $:.unshift(path) if File.directory?(path) end +require 'active_support' require 'active_support/core/all' module ActiveRecord -- cgit v1.2.3 From 1c6fcbfd2d67ce2b2faa29d956566b48e9a61188 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 2 May 2009 15:30:00 -0700 Subject: Fix implicit ordering expectation --- actionpack/test/dispatch/test_request_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/test/dispatch/test_request_test.rb b/actionpack/test/dispatch/test_request_test.rb index 19b5e2988b..5da02b2ea6 100644 --- a/actionpack/test/dispatch/test_request_test.rb +++ b/actionpack/test/dispatch/test_request_test.rb @@ -40,6 +40,6 @@ class TestRequestTest < ActiveSupport::TestCase req.cookies["login"] = "XJ-122" assert_equal({"user_name" => "david", "login" => "XJ-122"}, req.cookies) - assert_equal "login=XJ-122; user_name=david;", req.env["HTTP_COOKIE"] + assert_equal %w(login=XJ-122 user_name=david), req.env["HTTP_COOKIE"].split(/; ?/).sort end end -- cgit v1.2.3 From 11af089cee0a0e744e267d32becfe2c66a586d31 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 2 May 2009 23:02:22 -0500 Subject: Extract ActionController rescue templates into Rescue and ShowExceptions middleware. This commit breaks all exception catching plugins like ExceptionNotifier. These plugins should be rewritten as middleware instead overriding Controller#rescue_action_in_public. --- actionpack/lib/action_controller.rb | 2 +- actionpack/lib/action_controller/base/base.rb | 9 +- actionpack/lib/action_controller/base/rescue.rb | 50 +++ .../lib/action_controller/dispatch/dispatcher.rb | 16 +- .../lib/action_controller/dispatch/middlewares.rb | 5 + .../lib/action_controller/dispatch/rescue.rb | 185 ----------- .../templates/rescues/_request_and_response.erb | 24 -- .../dispatch/templates/rescues/_trace.erb | 26 -- .../dispatch/templates/rescues/diagnostics.erb | 10 - .../dispatch/templates/rescues/layout.erb | 29 -- .../templates/rescues/missing_template.erb | 2 - .../dispatch/templates/rescues/routing_error.erb | 10 - .../dispatch/templates/rescues/template_error.erb | 21 -- .../dispatch/templates/rescues/unknown_action.erb | 2 - actionpack/lib/action_dispatch.rb | 2 + .../lib/action_dispatch/middleware/rescue.rb | 14 + .../action_dispatch/middleware/show_exceptions.rb | 142 +++++++++ .../templates/rescues/_request_and_response.erb | 24 ++ .../middleware/templates/rescues/_trace.erb | 26 ++ .../middleware/templates/rescues/diagnostics.erb | 10 + .../middleware/templates/rescues/layout.erb | 29 ++ .../templates/rescues/missing_template.erb | 2 + .../middleware/templates/rescues/routing_error.erb | 10 + .../templates/rescues/template_error.erb | 21 ++ .../templates/rescues/unknown_action.erb | 2 + .../action_dispatch/testing/assertions/response.rb | 8 +- .../test/controller/action_pack_assertions_test.rb | 2 +- actionpack/test/controller/base_test.rb | 9 +- actionpack/test/controller/caching_test.rb | 3 + .../deprecation/deprecated_base_methods_test.rb | 6 - actionpack/test/controller/dispatcher_test.rb | 13 - actionpack/test/controller/filters_test.rb | 5 + actionpack/test/controller/layout_test.rb | 3 +- actionpack/test/controller/rescue_test.rb | 346 +++------------------ actionpack/test/dispatch/show_exceptions_test.rb | 103 ++++++ 35 files changed, 509 insertions(+), 662 deletions(-) create mode 100644 actionpack/lib/action_controller/base/rescue.rb delete mode 100644 actionpack/lib/action_controller/dispatch/rescue.rb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb delete mode 100644 actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb create mode 100644 actionpack/lib/action_dispatch/middleware/rescue.rb create mode 100644 actionpack/lib/action_dispatch/middleware/show_exceptions.rb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb create mode 100644 actionpack/test/dispatch/show_exceptions_test.rb diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index aee6b9dc9f..ab78bc9222 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -60,7 +60,7 @@ module ActionController autoload :Redirector, 'action_controller/base/redirect' autoload :Renderer, 'action_controller/base/render' autoload :RequestForgeryProtection, 'action_controller/base/request_forgery_protection' - autoload :Rescue, 'action_controller/dispatch/rescue' + autoload :Rescue, 'action_controller/base/rescue' autoload :Resources, 'action_controller/routing/resources' autoload :Responder, 'action_controller/base/responder' autoload :Routing, 'action_controller/routing' diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb index 60567454b1..7bbde519cc 100644 --- a/actionpack/lib/action_controller/base/base.rb +++ b/actionpack/lib/action_controller/base/base.rb @@ -30,10 +30,6 @@ module ActionController #:nodoc: def allowed_methods_header allowed_methods.map { |method_symbol| method_symbol.to_s.upcase } * ', ' end - - def handle_response!(response) - response.headers['Allow'] ||= allowed_methods_header - end end class NotImplemented < MethodNotAllowed #:nodoc: @@ -510,9 +506,8 @@ module ActionController #:nodoc: public def call(env) - # HACK: For global rescue to have access to the original request and response - request = env["action_dispatch.rescue.request"] ||= ActionDispatch::Request.new(env) - response = env["action_dispatch.rescue.response"] ||= ActionDispatch::Response.new + request = ActionDispatch::Request.new(env) + response = ActionDispatch::Response.new process(request, response).to_a end diff --git a/actionpack/lib/action_controller/base/rescue.rb b/actionpack/lib/action_controller/base/rescue.rb new file mode 100644 index 0000000000..2717a06a37 --- /dev/null +++ b/actionpack/lib/action_controller/base/rescue.rb @@ -0,0 +1,50 @@ +module ActionController #:nodoc: + # Actions that fail to perform as expected throw exceptions. These + # exceptions can either be rescued for the public view (with a nice + # user-friendly explanation) or for the developers view (with tons of + # debugging information). The developers view is already implemented by + # the Action Controller, but the public view should be tailored to your + # specific application. + # + # The default behavior for public exceptions is to render a static html + # file with the name of the error code thrown. If no such file exists, an + # empty response is sent with the correct status code. + # + # You can override what constitutes a local request by overriding the + # local_request? method in your own controller. Custom rescue + # behavior is achieved by overriding the rescue_action_in_public + # and rescue_action_locally methods. + module Rescue + def self.included(base) #:nodoc: + base.send :include, ActiveSupport::Rescuable + base.extend(ClassMethods) + + base.class_eval do + alias_method_chain :perform_action, :rescue + end + end + + module ClassMethods + def rescue_action(env) + exception = env.delete('action_dispatch.rescue.exception') + request = ActionDispatch::Request.new(env) + response = ActionDispatch::Response.new + new.process(request, response, :rescue_action, exception).to_a + end + end + + protected + # Exception handler called when the performance of an action raises + # an exception. + def rescue_action(exception) + rescue_with_handler(exception) || raise(exception) + end + + private + def perform_action_with_rescue + perform_action_without_rescue + rescue Exception => exception + rescue_action(exception) + end + end +end diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb index 25844bf2a2..c0eb359547 100644 --- a/actionpack/lib/action_controller/dispatch/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatch/dispatcher.rb @@ -75,18 +75,10 @@ module ActionController end def _call(env) - begin - run_callbacks :before_dispatch - Routing::Routes.call(env) - rescue Exception => exception - if controller ||= (::ApplicationController rescue Base) - controller.call_with_exception(env, exception) - else - raise exception - end - ensure - run_callbacks :after_dispatch, :enumerator => :reverse_each - end + run_callbacks :before_dispatch + Routing::Routes.call(env) + ensure + run_callbacks :after_dispatch, :enumerator => :reverse_each end def flush_logger diff --git a/actionpack/lib/action_controller/dispatch/middlewares.rb b/actionpack/lib/action_controller/dispatch/middlewares.rb index b5adbae746..31a7b00d28 100644 --- a/actionpack/lib/action_controller/dispatch/middlewares.rb +++ b/actionpack/lib/action_controller/dispatch/middlewares.rb @@ -3,6 +3,11 @@ use "Rack::Lock", :if => lambda { } use "ActionDispatch::Failsafe" +use "ActionDispatch::ShowExceptions", lambda { ActionController::Base.consider_all_requests_local } +use "ActionDispatch::Rescue", lambda { + controller = (::ApplicationController rescue ActionController::Base) + controller.method(:rescue_action) +} use lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options } diff --git a/actionpack/lib/action_controller/dispatch/rescue.rb b/actionpack/lib/action_controller/dispatch/rescue.rb deleted file mode 100644 index 2f3b40c231..0000000000 --- a/actionpack/lib/action_controller/dispatch/rescue.rb +++ /dev/null @@ -1,185 +0,0 @@ -module ActionController #:nodoc: - # Actions that fail to perform as expected throw exceptions. These - # exceptions can either be rescued for the public view (with a nice - # user-friendly explanation) or for the developers view (with tons of - # debugging information). The developers view is already implemented by - # the Action Controller, but the public view should be tailored to your - # specific application. - # - # The default behavior for public exceptions is to render a static html - # file with the name of the error code thrown. If no such file exists, an - # empty response is sent with the correct status code. - # - # You can override what constitutes a local request by overriding the - # local_request? method in your own controller. Custom rescue - # behavior is achieved by overriding the rescue_action_in_public - # and rescue_action_locally methods. - module Rescue - LOCALHOST = '127.0.0.1'.freeze - - DEFAULT_RESCUE_RESPONSE = :internal_server_error - DEFAULT_RESCUE_RESPONSES = { - 'ActionController::RoutingError' => :not_found, - 'ActionController::UnknownAction' => :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 - } - - DEFAULT_RESCUE_TEMPLATE = 'diagnostics' - DEFAULT_RESCUE_TEMPLATES = { - 'ActionView::MissingTemplate' => 'missing_template', - 'ActionController::RoutingError' => 'routing_error', - 'ActionController::UnknownAction' => 'unknown_action', - 'ActionView::TemplateError' => 'template_error' - } - - RESCUES_TEMPLATE_PATH = ActionView::Template::FileSystemPath.new( - File.join(File.dirname(__FILE__), "templates")) - - def self.included(base) #:nodoc: - base.cattr_accessor :rescue_responses - base.rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE) - base.rescue_responses.update DEFAULT_RESCUE_RESPONSES - - base.cattr_accessor :rescue_templates - base.rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE) - base.rescue_templates.update DEFAULT_RESCUE_TEMPLATES - - base.extend(ClassMethods) - base.send :include, ActiveSupport::Rescuable - - base.class_eval do - alias_method_chain :perform_action, :rescue - end - end - - module ClassMethods - def call_with_exception(env, exception) #:nodoc: - request = env["action_controller.rescue.request"] ||= ActionDispatch::Request.new(env) - response = env["action_controller.rescue.response"] ||= ActionDispatch::Response.new - new.process(request, response, :rescue_action, exception).to_a - end - end - - protected - # Exception handler called when the performance of an action raises - # an exception. - def rescue_action(exception) - rescue_with_handler(exception) || - rescue_action_without_handler(exception) - end - - # Overwrite to implement custom logging of errors. By default - # logs as fatal. - def log_error(exception) #:doc: - ActiveSupport::Deprecation.silence do - if ActionView::TemplateError === exception - logger.fatal(exception.to_s) - else - logger.fatal( - "\n#{exception.class} (#{exception.message}):\n " + - clean_backtrace(exception).join("\n ") + "\n\n" - ) - end - end - end - - # Overwrite to implement public exception handling (for requests - # answering false to local_request?). By default will call - # render_optional_error_file. Override this method to provide more - # user friendly error messages. - def rescue_action_in_public(exception) #:doc: - render_optional_error_file response_code_for_rescue(exception) - end - - # Attempts to render a static error page based on the - # status_code 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 public/500.da.html - # then attempt to render public/500.html. If none of them exist, - # the body of the response will be left empty. - def render_optional_error_file(status_code) - status = interpret_status(status_code) - locale_path = "#{Rails.public_path}/#{status[0,3]}.#{I18n.locale}.html" if I18n.locale - path = "#{Rails.public_path}/#{status[0,3]}.html" - - if locale_path && File.exist?(locale_path) - render :file => locale_path, :status => status, :content_type => Mime::HTML - elsif File.exist?(path) - render :file => path, :status => status, :content_type => Mime::HTML - else - head status - end - end - - # True if the request came from localhost, 127.0.0.1. Override this - # method if you wish to redefine the meaning of a local request to - # include remote IP addresses or other criteria. - def local_request? #:doc: - request.remote_addr == LOCALHOST && request.remote_ip == LOCALHOST - end - - # Render detailed diagnostics for unhandled exceptions rescued from - # a controller action. - def rescue_action_locally(exception) - @template.instance_variable_set("@exception", exception) - @template.instance_variable_set("@rescues_path", RESCUES_TEMPLATE_PATH) - @template.instance_variable_set("@contents", - @template._render_template(template_path_for_local_rescue(exception))) - - response.content_type = Mime::HTML - response.status = interpret_status(response_code_for_rescue(exception)) - - content = @template._render_template(rescues_path("layout")) - render_for_text(content) - end - - def rescue_action_without_handler(exception) - log_error(exception) if logger - erase_results if performed? - - # Let the exception alter the response if it wants. - # For example, MethodNotAllowed sets the Allow header. - if exception.respond_to?(:handle_response!) - exception.handle_response!(response) - end - - if consider_all_requests_local || local_request? - rescue_action_locally(exception) - else - rescue_action_in_public(exception) - end - end - - private - def perform_action_with_rescue #:nodoc: - perform_action_without_rescue - rescue Exception => exception - rescue_action(exception) - end - - def rescues_path(template_name) - RESCUES_TEMPLATE_PATH.find_by_parts("rescues/#{template_name}.erb") - end - - def template_path_for_local_rescue(exception) - rescues_path(rescue_templates[exception.class.name]) - end - - def response_code_for_rescue(exception) - rescue_responses[exception.class.name] - end - - def clean_backtrace(exception) - defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ? - Rails.backtrace_cleaner.clean(exception.backtrace) : - exception.backtrace - end - end -end diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb deleted file mode 100644 index 64b34650b1..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb +++ /dev/null @@ -1,24 +0,0 @@ -<% unless @exception.blamed_files.blank? %> - <% if (hide = @exception.blamed_files.length > 8) %> - Show blamed files - <% end %> - 
><%=h @exception.describe_blame %>
-<% end %> - -<% - clean_params = request.parameters.clone - clean_params.delete("action") - clean_params.delete("controller") - - request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") -%> - -

Request

-

Parameters: 

<%=h request_dump %>

- -

Show session dump

- - - -

Response

-

Headers: 

<%=h response ? response.headers.inspect.gsub(',', ",\n") : 'None' %>

diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb deleted file mode 100644 index bb2d8375bd..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb +++ /dev/null @@ -1,26 +0,0 @@ -<% - traces = [ - ["Application Trace", @exception.application_backtrace], - ["Framework Trace", @exception.framework_backtrace], - ["Full Trace", @exception.clean_backtrace] - ] - names = traces.collect {|name, trace| name} -%> - -

RAILS_ROOT: <%= defined?(RAILS_ROOT) ? RAILS_ROOT : "unset" %>

- -
- <% names.each do |name| %> - <% - show = "document.getElementById('#{name.gsub /\s/, '-'}').style.display='block';" - hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub /\s/, '-'}').style.display='none';"} - %> - <%= name %> <%= '|' unless names.last == name %> - <% end %> - - <% traces.each do |name, trace| %> -
;"> - 
<%= trace.join "\n" %>
-
- <% end %> -
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb deleted file mode 100644 index e5c647c826..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb +++ /dev/null @@ -1,10 +0,0 @@ -

- <%=h @exception.class.to_s %> - <% if request.parameters['controller'] %> - in <%=h request.parameters['controller'].humanize %>Controller<% if request.parameters['action'] %>#<%=h request.parameters['action'] %><% end %> - <% end %> -

-
<%=h @exception.clean_message %>
- -<%= @template._render_template(@rescues_path.find_by_parts("rescues/_trace.erb")) %> -<%= @template._render_template(@rescues_path.find_by_parts("rescues/_request_and_response.erb")) %> \ No newline at end of file diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb deleted file mode 100644 index 4a04742e40..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb +++ /dev/null @@ -1,29 +0,0 @@ - - - Action Controller: Exception caught - - - - -<%= @contents %> - - - \ No newline at end of file diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb deleted file mode 100644 index dbfdf76947..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb +++ /dev/null @@ -1,2 +0,0 @@ -

Template is missing

-

<%=h @exception.message %>

diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb deleted file mode 100644 index ccfa858cce..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb +++ /dev/null @@ -1,10 +0,0 @@ -

Routing Error

-

<%=h @exception.message %>

-<% unless @exception.failures.empty? %>

-

Failure reasons:

-
    - <% @exception.failures.each do |route, reason| %> -
  1. <%=h route.inspect.gsub('\\', '') %> failed because <%=h reason.downcase %>
    2. - <% end %> -
-

<% end %> diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb deleted file mode 100644 index 2e34e03bd5..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb +++ /dev/null @@ -1,21 +0,0 @@ -

- <%=h @exception.original_exception.class.to_s %> in - <%=h request.parameters["controller"].capitalize if request.parameters["controller"]%>#<%=h request.parameters["action"] %> -

- -

- Showing <%=h @exception.file_name %> where line #<%=h @exception.line_number %> raised: - 

<%=h @exception.message %>
-

- -

Extracted source (around line #<%=h @exception.line_number %>): -

<%=h @exception.source_extract %>

- -

<%=h @exception.sub_template_message %>

- -<% @real_exception = @exception - @exception = @exception.original_exception || @exception %> -<%= render :file => @rescues_path["rescues/_trace.erb"] %> -<% @exception = @real_exception %> - -<%= render :file => @rescues_path["rescues/_request_and_response.erb"] %> diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb deleted file mode 100644 index 683379da10..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb +++ /dev/null @@ -1,2 +0,0 @@ -

Unknown action

-

<%=h @exception.message %>

diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 5f6202d153..5a082fa7e3 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -48,6 +48,8 @@ module ActionDispatch autoload :Failsafe, 'action_dispatch/middleware/failsafe' autoload :ParamsParser, 'action_dispatch/middleware/params_parser' autoload :Reloader, 'action_dispatch/middleware/reloader' + autoload :Rescue, 'action_dispatch/middleware/rescue' + autoload :ShowExceptions, 'action_dispatch/middleware/show_exceptions' autoload :MiddlewareStack, 'action_dispatch/middleware/stack' autoload :Assertions, 'action_dispatch/testing/assertions' diff --git a/actionpack/lib/action_dispatch/middleware/rescue.rb b/actionpack/lib/action_dispatch/middleware/rescue.rb new file mode 100644 index 0000000000..1456825526 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/rescue.rb @@ -0,0 +1,14 @@ +module ActionDispatch + class Rescue + def initialize(app, rescuer) + @app, @rescuer = app, rescuer + end + + def call(env) + @app.call(env) + rescue Exception => exception + env['action_dispatch.rescue.exception'] = exception + @rescuer.call(env) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb new file mode 100644 index 0000000000..791254cdf2 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -0,0 +1,142 @@ +module ActionDispatch + class ShowExceptions + include StatusCodes + + LOCALHOST = '127.0.0.1'.freeze + + DEFAULT_RESCUE_RESPONSE = :internal_server_error + DEFAULT_RESCUE_RESPONSES = { + 'ActionController::RoutingError' => :not_found, + 'ActionController::UnknownAction' => :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 + } + + DEFAULT_RESCUE_TEMPLATE = 'diagnostics' + DEFAULT_RESCUE_TEMPLATES = { + 'ActionView::MissingTemplate' => 'missing_template', + 'ActionController::RoutingError' => 'routing_error', + 'ActionController::UnknownAction' => 'unknown_action', + 'ActionView::TemplateError' => 'template_error' + } + + RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') + + cattr_accessor :rescue_responses + @@rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE) + @@rescue_responses.update DEFAULT_RESCUE_RESPONSES + + cattr_accessor :rescue_templates + @@rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE) + @@rescue_templates.update DEFAULT_RESCUE_TEMPLATES + + def initialize(app, consider_all_requests_local = false) + @app = app + @consider_all_requests_local = consider_all_requests_local + end + + def call(env) + @app.call(env) + rescue Exception => exception + log_error(exception) if logger + + request = Request.new(env) + if @consider_all_requests_local || local_request?(request) + rescue_action_locally(request, exception) + else + rescue_action_in_public(exception) + end + end + + private + # Render detailed diagnostics for unhandled exceptions rescued from + # a controller action. + def rescue_action_locally(request, exception) + template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], + :template => template, + :request => request, + :exception => exception + ) + file = "rescues/#{@@rescue_templates[exception.class.name]}.erb" + body = template.render(:file => file, :layout => 'rescues/layout.erb') + + headers = {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s} + status = status_code(exception) + + [status, headers, body] + end + + # Attempts to render a static error page based on the + # status_code 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 public/500.da.html + # then attempt to render public/500.html. If none of them exist, + # the body of the response will be left empty. + def rescue_action_in_public(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_public_file(status, locale_path) + elsif File.exist?(path) + render_public_file(status, path) + else + [status, {'Content-Type' => 'text/html', 'Content-Length' => '0'}, []] + end + end + + # True if the request came from localhost, 127.0.0.1. + def local_request?(request) + request.remote_addr == LOCALHOST && request.remote_ip == LOCALHOST + end + + def render_public_file(status, path) + body = File.read(path) + [status, {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}, body] + end + + def status_code(exception) + interpret_status(@@rescue_responses[exception.class.name]).to_i + end + + def public_path + if defined?(Rails) + Rails.public_path + else + "public" + end + end + + def log_error(exception) #:doc: + ActiveSupport::Deprecation.silence do + if ActionView::TemplateError === exception + logger.fatal(exception.to_s) + else + logger.fatal( + "\n#{exception.class} (#{exception.message}):\n " + + clean_backtrace(exception).join("\n ") + "\n\n" + ) + end + end + end + + def clean_backtrace(exception) + defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ? + Rails.backtrace_cleaner.clean(exception.backtrace) : + exception.backtrace + end + + def logger + if defined?(Rails.logger) + Rails.logger + end + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb new file mode 100644 index 0000000000..5224403dab --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -0,0 +1,24 @@ +<% unless @exception.blamed_files.blank? %> + <% if (hide = @exception.blamed_files.length > 8) %> + Show blamed files + <% end %> + 
><%=h @exception.describe_blame %>
+<% end %> + +<% + clean_params = @request.parameters.clone + clean_params.delete("action") + clean_params.delete("controller") + + request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") +%> + +

Request

+

Parameters: 

<%=h request_dump %>

+ +

Show session dump

+ + + +

Response

+

Headers: 

<%=h @response ? @response.headers.inspect.gsub(',', ",\n") : 'None' %>

diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb new file mode 100644 index 0000000000..bb2d8375bd --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb @@ -0,0 +1,26 @@ +<% + traces = [ + ["Application Trace", @exception.application_backtrace], + ["Framework Trace", @exception.framework_backtrace], + ["Full Trace", @exception.clean_backtrace] + ] + names = traces.collect {|name, trace| name} +%> + +

RAILS_ROOT: <%= defined?(RAILS_ROOT) ? RAILS_ROOT : "unset" %>

+ +
+ <% names.each do |name| %> + <% + show = "document.getElementById('#{name.gsub /\s/, '-'}').style.display='block';" + hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub /\s/, '-'}').style.display='none';"} + %> + <%= name %> <%= '|' unless names.last == name %> + <% end %> + + <% traces.each do |name, trace| %> +
;"> + 
<%= trace.join "\n" %>
+
+ <% end %> +
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb new file mode 100644 index 0000000000..693e56270a --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -0,0 +1,10 @@ +

+ <%=h @exception.class.to_s %> + <% if @request.parameters['controller'] %> + in <%=h @request.parameters['controller'].humanize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> + <% end %> +

+
<%=h @exception.clean_message %>
+ +<%= render :file => "rescues/_trace.erb" %> +<%= render :file => "rescues/_request_and_response.erb" %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb new file mode 100644 index 0000000000..6c32fb17b8 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -0,0 +1,29 @@ + + + Action Controller: Exception caught + + + + +<%= yield %> + + + diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb new file mode 100644 index 0000000000..dbfdf76947 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb @@ -0,0 +1,2 @@ +

Template is missing

+

<%=h @exception.message %>

diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb new file mode 100644 index 0000000000..ccfa858cce --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb @@ -0,0 +1,10 @@ +

Routing Error

+

<%=h @exception.message %>

+<% unless @exception.failures.empty? %>

+

Failure reasons:

+
    + <% @exception.failures.each do |route, reason| %> +
  1. <%=h route.inspect.gsub('\\', '') %> failed because <%=h reason.downcase %>
    2. + <% end %> +
+

<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb new file mode 100644 index 0000000000..02fa18211d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb @@ -0,0 +1,21 @@ +

+ <%=h @exception.original_exception.class.to_s %> in + <%=h @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%=h @request.parameters["action"] %> +

+ +

+ Showing <%=h @exception.file_name %> where line #<%=h @exception.line_number %> raised: + 

<%=h @exception.message %>
+

+ +

Extracted source (around line #<%=h @exception.line_number %>): +

<%=h @exception.source_extract %>

+ +

<%=h @exception.sub_template_message %>

+ +<% @real_exception = @exception + @exception = @exception.original_exception || @exception %> +<%= render :file => "rescues/_trace.erb" %> +<% @exception = @real_exception %> + +<%= render :file => "rescues/_request_and_response.erb" %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb new file mode 100644 index 0000000000..683379da10 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb @@ -0,0 +1,2 @@ +

Unknown action

+

<%=h @exception.message %>

diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 92e3da580e..501a7c4dfb 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -31,13 +31,7 @@ module ActionDispatch elsif type.is_a?(Symbol) && @response.response_code == ActionDispatch::StatusCodes::SYMBOL_TO_STATUS_CODE[type] assert_block("") { true } # to count the assertion else - if @controller && @response.error? - exception = @controller.template.instance_variable_get(:@exception) - exception_message = exception && exception.message - assert_block(build_message(message, "Expected response to be a , but was \n", type, @response.response_code, exception_message.to_s)) { false } - else - assert_block(build_message(message, "Expected response to be a , but was ", type, @response.response_code)) { false } - end + assert_block(build_message(message, "Expected response to be a , but was ", type, @response.response_code)) { false } end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 60957a2f0e..484d3c5ce7 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -489,7 +489,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase get :index assert_response :success flunk 'Expected non-success response' - rescue ActiveSupport::TestCase::Assertion => e + rescue RuntimeError => e assert e.message.include?('FAIL') end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index f4517d06c4..0c54d61426 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -27,6 +27,7 @@ class EmptyController < ActionController::Base end class NonEmptyController < ActionController::Base def public_action + render :nothing => true end hide_action :hidden_action @@ -51,6 +52,7 @@ end class DefaultUrlOptionsController < ActionController::Base def default_url_options_action + render :nothing => true end def default_url_options(options = nil) @@ -151,11 +153,8 @@ class PerformActionTest < ActionController::TestCase def test_get_on_hidden_should_fail use_controller NonEmptyController - get :hidden_action - assert_response 404 - - get :another_hidden_action - assert_response 404 + assert_raise(ActionController::UnknownAction) { get :hidden_action } + assert_raise(ActionController::UnknownAction) { get :another_hidden_action } end def test_namespaced_action_should_log_module_name diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 6b9d1056a3..560c09509b 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -149,6 +149,9 @@ class PageCachingTest < ActionController::TestCase end class ActionCachingTestController < ActionController::Base + rescue_from(Exception) { head 500 } + rescue_from(ActiveRecord::RecordNotFound) { head :not_found } + caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour caches_action :show, :cache_path => 'http://test.host/custom/show' caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" } diff --git a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb index dd69a63020..0c02afea36 100644 --- a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb +++ b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb @@ -15,12 +15,6 @@ class DeprecatedBaseMethodsTest < ActionController::TestCase tests Target - def test_log_error_silences_deprecation_warnings - get :raises_name_error - rescue => e - assert_not_deprecated { @controller.send :log_error, e } - end - if defined? Test::Unit::Error def test_assertion_failed_error_silences_deprecation_warnings get :raises_name_error diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index c3bd113b86..b315232a7b 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -45,19 +45,6 @@ class DispatcherTest < Test::Unit::TestCase def log_failsafe_exception(status, exception); end end - def test_failsafe_response - Dispatcher.any_instance.expects(:_call).raises('b00m') - ActionDispatch::Failsafe.any_instance.expects(:log_failsafe_exception) - - assert_nothing_raised do - assert_equal [ - 500, - {"Content-Type" => "text/html"}, - ["

500 Internal Server Error

"] - ], dispatch - end - end - def test_prepare_callbacks a = b = c = nil Dispatcher.to_prepare { |*args| a = b = c = 1 } diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 5876f55420..5fd3eb0446 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -169,6 +169,7 @@ class FilterTest < Test::Unit::TestCase end def public + render :text => 'ok' end end @@ -177,6 +178,10 @@ class FilterTest < Test::Unit::TestCase before_filter :find_record before_filter :ensure_login + def index + render :text => 'ok' + end + private def find_record @ran_filter ||= [] diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index da3f7b0cb8..5b7d40d16d 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -205,8 +205,7 @@ end class LayoutExceptionRaised < ActionController::TestCase def test_exception_raised_when_layout_file_not_found @controller = SetsNonExistentLayoutFile.new - get :hello - assert_kind_of ActionView::MissingTemplate, @controller.template.instance_eval { @exception } + assert_raise(ActionView::MissingTemplate) { get :hello } end end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 894420a910..f745926b20 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -138,213 +138,88 @@ class RescueController < ActionController::Base end end -class RescueControllerTest < ActionController::TestCase - FIXTURE_PUBLIC = "#{File.dirname(__FILE__)}/../fixtures".freeze - - def setup - super - set_all_requests_local - populate_exception_object - end - - def set_all_requests_local - RescueController.consider_all_requests_local = true - @request.remote_addr = '1.2.3.4' - @request.host = 'example.com' - end - - def populate_exception_object - begin - raise 'foo' - rescue => @exception - end - end - - def test_rescue_exceptions_raised_by_filters - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :before_filter_raises - end - end +class ExceptionInheritanceRescueController < ActionController::Base - assert_response :internal_server_error + class ParentException < StandardError end - def test_rescue_action_locally_if_all_requests_local - @controller.expects(:local_request?).never - @controller.expects(:rescue_action_locally).with(@exception) - @controller.expects(:rescue_action_in_public).never - - with_all_requests_local do - @controller.send :rescue_action, @exception - end + class ChildException < ParentException end - def test_rescue_action_locally_if_remote_addr_is_localhost - @controller.expects(:local_request?).returns(true) - @controller.expects(:rescue_action_locally).with(@exception) - @controller.expects(:rescue_action_in_public).never - - with_all_requests_local false do - @controller.send :rescue_action, @exception - end + class GrandchildException < ChildException end - def test_rescue_action_in_public_otherwise - @controller.expects(:local_request?).returns(false) - @controller.expects(:rescue_action_locally).never - @controller.expects(:rescue_action_in_public).with(@exception) + rescue_from ChildException, :with => lambda { head :ok } + rescue_from ParentException, :with => lambda { head :created } + rescue_from GrandchildException, :with => lambda { head :no_content } - with_all_requests_local false do - @controller.send :rescue_action, @exception - end + def raise_parent_exception + raise ParentException end - def test_rescue_action_in_public_with_localized_error_file - # Change locale - old_locale = I18n.locale - I18n.locale = :da - - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :raises - end - end - - assert_response :internal_server_error - body = File.read("#{FIXTURE_PUBLIC}/public/500.da.html") - assert_equal body, @response.body - ensure - I18n.locale = old_locale + def raise_child_exception + raise ChildException end - def test_rescue_action_in_public_with_error_file - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :raises - end - end - - assert_response :internal_server_error - body = File.read("#{FIXTURE_PUBLIC}/public/500.html") - assert_equal body, @response.body + def raise_grandchild_exception + raise GrandchildException end +end - def test_rescue_action_in_public_without_error_file - with_rails_root '/tmp' do - with_all_requests_local false do - get :raises - end - end - - assert_response :internal_server_error - assert_equal ' ', @response.body +class ExceptionInheritanceRescueControllerTest < ActionController::TestCase + def test_bottom_first + get :raise_grandchild_exception + assert_response :no_content end - def test_rescue_unknown_action_in_public_with_error_file - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :foobar_doesnt_exist - end - end - - assert_response :not_found - body = File.read("#{FIXTURE_PUBLIC}/public/404.html") - assert_equal body, @response.body + def test_inheritance_works + get :raise_child_exception + assert_response :created end +end - def test_rescue_unknown_action_in_public_without_error_file - with_rails_root '/tmp' do - with_all_requests_local false do - get :foobar_doesnt_exist - end - end - - assert_response :not_found - assert_equal ' ', @response.body +class ControllerInheritanceRescueController < ExceptionInheritanceRescueController + class FirstExceptionInChildController < StandardError end - def test_rescue_missing_template_in_public - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local true do - get :missing_template - end - end - - assert_response :internal_server_error - assert @response.body.include?('missing_template'), "Response should include the template name." + class SecondExceptionInChildController < StandardError end - def test_rescue_action_locally - get :raises - assert_response :internal_server_error - assert_template 'diagnostics.erb' - assert @response.body.include?('RescueController#raises'), "Response should include controller and action." - assert @response.body.include?("don't panic"), "Response should include exception message." - end + rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone } - def test_local_request_when_remote_addr_is_localhost - @controller.expects(:request).returns(@request).at_least_once - with_remote_addr '127.0.0.1' do - assert @controller.send(:local_request?) - end + def raise_first_exception_in_child_controller + raise FirstExceptionInChildController end - def test_local_request_when_remote_addr_isnt_locahost - @controller.expects(:request).returns(@request) - with_remote_addr '1.2.3.4' do - assert !@controller.send(:local_request?) - end + def raise_second_exception_in_child_controller + raise SecondExceptionInChildController end +end - def test_rescue_responses - responses = ActionController::Base.rescue_responses - - assert_equal ActionController::Rescue::DEFAULT_RESCUE_RESPONSE, responses.default - assert_equal ActionController::Rescue::DEFAULT_RESCUE_RESPONSE, responses[Exception.new] - - assert_equal :not_found, responses[ActionController::RoutingError.name] - assert_equal :not_found, responses[ActionController::UnknownAction.name] - assert_equal :not_found, responses['ActiveRecord::RecordNotFound'] - assert_equal :conflict, responses['ActiveRecord::StaleObjectError'] - assert_equal :unprocessable_entity, responses['ActiveRecord::RecordInvalid'] - assert_equal :unprocessable_entity, responses['ActiveRecord::RecordNotSaved'] - assert_equal :method_not_allowed, responses['ActionController::MethodNotAllowed'] - assert_equal :not_implemented, responses['ActionController::NotImplemented'] +class ControllerInheritanceRescueControllerTest < ActionController::TestCase + def test_first_exception_in_child_controller + get :raise_first_exception_in_child_controller + assert_response :gone end - def test_rescue_templates - templates = ActionController::Base.rescue_templates - - assert_equal ActionController::Rescue::DEFAULT_RESCUE_TEMPLATE, templates.default - assert_equal ActionController::Rescue::DEFAULT_RESCUE_TEMPLATE, templates[Exception.new] - - assert_equal 'missing_template', templates[ActionView::MissingTemplate.name] - assert_equal 'routing_error', templates[ActionController::RoutingError.name] - assert_equal 'unknown_action', templates[ActionController::UnknownAction.name] - assert_equal 'template_error', templates[ActionView::TemplateError.name] + def test_second_exception_in_child_controller + get :raise_second_exception_in_child_controller + assert_response :gone end - def test_not_implemented - with_all_requests_local false do - with_rails_public_path(".") do - head :not_implemented - end - end - assert_response :not_implemented - assert_equal "GET, PUT", @response.headers['Allow'] + def test_exception_in_parent_controller + get :raise_parent_exception + assert_response :created end +end - def test_method_not_allowed - with_all_requests_local false do - with_rails_public_path(".") do - get :method_not_allowed - end - end - assert_response :method_not_allowed - assert_equal "GET, HEAD, PUT", @response.headers['Allow'] +class ApplicationController < ActionController::Base + rescue_from ActionController::RoutingError do + render :text => 'no way' end +end +class RescueControllerTest < ActionController::TestCase def test_rescue_handler get :not_authorized assert_response :forbidden @@ -399,133 +274,6 @@ class RescueControllerTest < ActionController::TestCase get :resource_unavailable_raise_as_string assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body end - - protected - def with_all_requests_local(local = true) - old_local, ActionController::Base.consider_all_requests_local = - ActionController::Base.consider_all_requests_local, local - yield - ensure - ActionController::Base.consider_all_requests_local = old_local - end - - def with_remote_addr(addr) - old_remote_addr, @request.remote_addr = @request.remote_addr, addr - yield - ensure - @request.remote_addr = old_remote_addr - end - - def with_rails_public_path(rails_root) - old_rails = Object.const_get(:Rails) rescue nil - mod = Object.const_set(:Rails, Module.new) - (class << mod; self; end).instance_eval do - define_method(:public_path) { "#{rails_root}/public" } - end - yield - ensure - Object.module_eval { remove_const(:Rails) } if defined?(Rails) - Object.const_set(:Rails, old_rails) if old_rails - end - - def with_rails_root(path = nil,&block) - old_rails_root = RAILS_ROOT if defined?(RAILS_ROOT) - if path - silence_warnings { Object.const_set(:RAILS_ROOT, path) } - else - Object.remove_const(:RAILS_ROOT) rescue nil - end - - with_rails_public_path(path, &block) - - ensure - if old_rails_root - silence_warnings { Object.const_set(:RAILS_ROOT, old_rails_root) } - else - Object.remove_const(:RAILS_ROOT) rescue nil - end - end -end - -class ExceptionInheritanceRescueController < ActionController::Base - - class ParentException < StandardError - end - - class ChildException < ParentException - end - - class GrandchildException < ChildException - end - - rescue_from ChildException, :with => lambda { head :ok } - rescue_from ParentException, :with => lambda { head :created } - rescue_from GrandchildException, :with => lambda { head :no_content } - - def raise_parent_exception - raise ParentException - end - - def raise_child_exception - raise ChildException - end - - def raise_grandchild_exception - raise GrandchildException - end -end - -class ExceptionInheritanceRescueControllerTest < ActionController::TestCase - def test_bottom_first - get :raise_grandchild_exception - assert_response :no_content - end - - def test_inheritance_works - get :raise_child_exception - assert_response :created - end -end - -class ControllerInheritanceRescueController < ExceptionInheritanceRescueController - class FirstExceptionInChildController < StandardError - end - - class SecondExceptionInChildController < StandardError - end - - rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone } - - def raise_first_exception_in_child_controller - raise FirstExceptionInChildController - end - - def raise_second_exception_in_child_controller - raise SecondExceptionInChildController - end -end - -class ControllerInheritanceRescueControllerTest < ActionController::TestCase - def test_first_exception_in_child_controller - get :raise_first_exception_in_child_controller - assert_response :gone - end - - def test_second_exception_in_child_controller - get :raise_second_exception_in_child_controller - assert_response :gone - end - - def test_exception_in_parent_controller - get :raise_parent_exception - assert_response :created - end -end - -class ApplicationController < ActionController::Base - rescue_from ActionController::RoutingError do - render :text => 'no way' - end end class RescueTest < ActionController::IntegrationTest diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb new file mode 100644 index 0000000000..f8f562e7c1 --- /dev/null +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -0,0 +1,103 @@ +require 'abstract_unit' + +module ActionDispatch + class ShowExceptions + private + def public_path + "#{FIXTURE_LOAD_PATH}/public" + end + end +end + +class ShowExceptionsTest < ActionController::IntegrationTest + Boomer = lambda do |env| + req = ActionDispatch::Request.new(env) + case req.path + when "/not_found" + raise ActionController::UnknownAction + when "/method_not_allowed" + raise ActionController::MethodNotAllowed + when "/not_implemented" + raise ActionController::NotImplemented + when "/unprocessable_entity" + raise ActionController::InvalidAuthenticityToken + else + raise "puke!" + end + end + + ProductionApp = ActionDispatch::ShowExceptions.new(Boomer, false) + DevelopmentApp = ActionDispatch::ShowExceptions.new(Boomer, true) + + test "rescue in public from a remote ip" do + @integration_session = open_session(ProductionApp) + self.remote_addr = '208.77.188.166' + + get "/" + assert_response 500 + assert_equal "500 error fixture\n", body + + get "/not_found" + assert_response 404 + assert_equal "404 error fixture\n", body + + get "/method_not_allowed" + assert_response 405 + assert_equal "", body + end + + test "rescue locally from a local request" do + @integration_session = open_session(ProductionApp) + self.remote_addr = '127.0.0.1' + + get "/" + assert_response 500 + assert_match /puke/, body + + get "/not_found" + assert_response 404 + assert_match /ActionController::UnknownAction/, body + + get "/method_not_allowed" + assert_response 405 + assert_match /ActionController::MethodNotAllowed/, body + end + + test "localize public rescue message" do + # Change locale + old_locale = I18n.locale + I18n.locale = :da + + begin + @integration_session = open_session(ProductionApp) + self.remote_addr = '208.77.188.166' + + get "/" + assert_response 500 + assert_equal "500 localized error fixture\n", body + + get "/not_found" + assert_response 404 + assert_equal "404 error fixture\n", body + ensure + I18n.locale = old_locale + end + end + + test "always rescue locally in development mode" do + @integration_session = open_session(DevelopmentApp) + self.remote_addr = '208.77.188.166' + + get "/" + assert_response 500 + assert_match /puke/, body + + get "/not_found" + assert_response 404 + assert_match /ActionController::UnknownAction/, body + + get "/method_not_allowed" + assert_response 405 + assert_match /ActionController::MethodNotAllowed/, body + end +end -- cgit v1.2.3 From e0660192806fc1ec8b75654e69211f85c9f1256b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 3 May 2009 09:24:32 -0500 Subject: Show lazy middleware args in pretty print --- actionpack/lib/action_dispatch/middleware/stack.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index ee5f28d5cb..ec8a9ca76e 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -61,7 +61,7 @@ module ActionDispatch def inspect str = klass.to_s - args.each { |arg| str += ", #{arg.inspect}" } + args.each { |arg| str += ", #{build_args.inspect}" } str end @@ -74,7 +74,6 @@ module ActionDispatch end private - def build_args Array(args).map { |arg| arg.respond_to?(:call) ? arg.call : arg } end -- cgit v1.2.3 From bcc4537f2a0d37fc02d67e9564fa5c9c5555b3d5 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 3 May 2009 09:41:40 -0500 Subject: Wrap dispatcher callbacks around the whole middleware chain. Kill unnecessary Reloader middleware. --- .../lib/action_controller/dispatch/dispatcher.rb | 45 +++++++++++----------- actionpack/lib/action_dispatch.rb | 1 - .../lib/action_dispatch/middleware/reloader.rb | 14 ------- 3 files changed, 22 insertions(+), 38 deletions(-) delete mode 100644 actionpack/lib/action_dispatch/middleware/reloader.rb diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb index c0eb359547..cce3b6175d 100644 --- a/actionpack/lib/action_controller/dispatch/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatch/dispatcher.rb @@ -5,9 +5,9 @@ module ActionController class << self def define_dispatcher_callbacks(cache_classes) unless cache_classes - unless self.middleware.include?(ActionDispatch::Reloader) - self.middleware.insert_after(ActionDispatch::Failsafe, ActionDispatch::Reloader) - end + # Development mode callbacks + before_dispatch :reload_application + after_dispatch :cleanup_application ActionView::Helpers::AssetTagHelper.cache_asset_timestamps = false end @@ -40,22 +40,11 @@ module ActionController def run_prepare_callbacks new.send :run_callbacks, :prepare_dispatch end - - def reload_application - # Run prepare callbacks before every request in development mode - run_prepare_callbacks - - Routing::Routes.reload - end - - def cleanup_application - # Cleanup the application before processing the current request. - ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) - ActiveSupport::Dependencies.clear - ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord) - end end + cattr_accessor :router + self.router = Routing::Routes + cattr_accessor :middleware self.middleware = ActionDispatch::MiddlewareStack.new do |middleware| middlewares = File.join(File.dirname(__FILE__), "middlewares.rb") @@ -66,21 +55,31 @@ module ActionController define_callbacks :prepare_dispatch, :before_dispatch, :after_dispatch def initialize - @app = @@middleware.build(lambda { |env| self._call(env) }) + @app = @@middleware.build(@@router) freeze end def call(env) - @app.call(env) - end - - def _call(env) run_callbacks :before_dispatch - Routing::Routes.call(env) + @app.call(env) ensure run_callbacks :after_dispatch, :enumerator => :reverse_each end + def reload_application + # Run prepare callbacks before every request in development mode + run_callbacks :prepare_dispatch + + @@router.reload + end + + def cleanup_application + # Cleanup the application before processing the current request. + ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) + ActiveSupport::Dependencies.clear + ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord) + end + def flush_logger Base.logger.flush end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 5a082fa7e3..c882930e64 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -47,7 +47,6 @@ module ActionDispatch autoload :Failsafe, 'action_dispatch/middleware/failsafe' autoload :ParamsParser, 'action_dispatch/middleware/params_parser' - autoload :Reloader, 'action_dispatch/middleware/reloader' autoload :Rescue, 'action_dispatch/middleware/rescue' autoload :ShowExceptions, 'action_dispatch/middleware/show_exceptions' autoload :MiddlewareStack, 'action_dispatch/middleware/stack' diff --git a/actionpack/lib/action_dispatch/middleware/reloader.rb b/actionpack/lib/action_dispatch/middleware/reloader.rb deleted file mode 100644 index 67313e30e4..0000000000 --- a/actionpack/lib/action_dispatch/middleware/reloader.rb +++ /dev/null @@ -1,14 +0,0 @@ -module ActionDispatch - class Reloader - def initialize(app) - @app = app - end - - def call(env) - ActionController::Dispatcher.reload_application - @app.call(env) - ensure - ActionController::Dispatcher.cleanup_application - end - end -end -- cgit v1.2.3 From d3ee8756c8b09b8524a9599926b02ededd27319c Mon Sep 17 00:00:00 2001 From: Chris Kampmeier Date: Sun, 3 May 2009 18:09:06 -0700 Subject: Don't use #tap before Active Support is available, since older versions of ruby don't have native implementations [#2603 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 1982ffc791..06d6c87090 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -21,9 +21,8 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #++ -"#{File.dirname(__FILE__)}/../../activesupport/lib".tap do |path| - $:.unshift(path) if File.directory?(path) -end +activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" +$:.unshift(activesupport_path) if File.directory?(activesupport_path) require 'active_support' require 'active_support/core/all' -- cgit v1.2.3 From eb201e64c0b68aee6d0715d44cf48178204c4133 Mon Sep 17 00:00:00 2001 From: codebrulee Date: Mon, 4 May 2009 12:36:22 -0400 Subject: Fixed Hash#from_xml with keys that are all caps. Signed-off-by: Michael Koziarski --- .../lib/active_support/core_ext/hash/conversions.rb | 2 +- activesupport/test/core_ext/hash_ext_test.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index f9dddec687..fe1f79050c 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -217,7 +217,7 @@ class Hash case params.class.to_s when "Hash" params.inject({}) do |h,(k,v)| - h[k.to_s.underscore.tr("-", "_")] = unrename_keys(v) + h[k.to_s.tr("-", "_")] = unrename_keys(v) h end when "Array" diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index d65a5323bf..ece5466abb 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -646,6 +646,22 @@ class HashToXmlTest < Test::Unit::TestCase assert_equal expected_topic_hash, Hash.from_xml(topic_xml)["rsp"]["photos"]["photo"] end + def test_all_caps_key_from_xml + test_xml = <<-EOT + + Lorem Ipsum + + EOT + + expected_hash = { + "ABC3XYZ" => { + "TEST" => "Lorem Ipsum" + } + } + + assert_equal expected_hash, Hash.from_xml(test_xml) + end + def test_empty_array_from_xml blog_xml = <<-XML -- cgit v1.2.3 From ccea98389abbf150b886c9f964b1def47f00f237 Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Fri, 1 May 2009 16:01:13 +0100 Subject: Providing support for :inverse_of as an option to associations. You can now add an :inverse_of option to has_one, has_many and belongs_to associations. This is best described with an example: class Man < ActiveRecord::Base has_one :face, :inverse_of => :man end class Face < ActiveRecord::Base belongs_to :man, :inverse_of => :face end m = Man.first f = m.face Without :inverse_of m and f.man would be different instances of the same object (f.man being pulled from the database again). With these new :inverse_of options m and f.man are the same in memory instance. Currently :inverse_of supports has_one and has_many (but not the :through variants) associations. It also supplies inverse support for belongs_to associations where the inverse is a has_one and it's not a polymorphic. Signed-off-by: Murray Steele Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 12 +- .../associations/association_collection.rb | 8 +- .../associations/association_proxy.rb | 14 ++ .../associations/belongs_to_association.rb | 12 +- .../associations/has_many_association.rb | 5 + .../associations/has_many_through_association.rb | 10 +- .../associations/has_one_association.rb | 11 +- activerecord/lib/active_record/reflection.rb | 21 ++ .../associations/inverse_associations_test.rb | 252 +++++++++++++++++++++ activerecord/test/fixtures/faces.yml | 7 + activerecord/test/fixtures/interests.yml | 29 +++ activerecord/test/fixtures/men.yml | 5 + activerecord/test/fixtures/zines.yml | 5 + activerecord/test/models/face.rb | 5 + activerecord/test/models/interest.rb | 4 + activerecord/test/models/man.rb | 7 + activerecord/test/models/zine.rb | 3 + activerecord/test/schema/schema.rb | 20 ++ 18 files changed, 418 insertions(+), 12 deletions(-) create mode 100644 activerecord/test/cases/associations/inverse_associations_test.rb create mode 100644 activerecord/test/fixtures/faces.yml create mode 100644 activerecord/test/fixtures/interests.yml create mode 100644 activerecord/test/fixtures/men.yml create mode 100644 activerecord/test/fixtures/zines.yml create mode 100644 activerecord/test/models/face.rb create mode 100644 activerecord/test/models/interest.rb create mode 100644 activerecord/test/models/man.rb create mode 100644 activerecord/test/models/zine.rb diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2115878e32..0952b087d1 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1,4 +1,10 @@ module ActiveRecord + class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc: + def initialize(reflection) + super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{reflection.class_name})") + end + end + class HasManyThroughAssociationNotFoundError < ActiveRecordError #:nodoc: def initialize(owner_class_name, reflection) super("Could not find the association #{reflection.options[:through].inspect} in model #{owner_class_name}") @@ -1488,7 +1494,7 @@ module ActiveRecord :finder_sql, :counter_sql, :before_add, :after_add, :before_remove, :after_remove, :extend, :readonly, - :validate + :validate, :inverse_of ] def create_has_many_reflection(association_id, options, &extension) @@ -1502,7 +1508,7 @@ module ActiveRecord @@valid_keys_for_has_one_association = [ :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :readonly, - :validate, :primary_key + :validate, :primary_key, :inverse_of ] def create_has_one_reflection(association_id, options) @@ -1521,7 +1527,7 @@ module ActiveRecord @@valid_keys_for_belongs_to_association = [ :class_name, :foreign_key, :foreign_type, :remote, :select, :conditions, :include, :dependent, :counter_cache, :extend, :polymorphic, :readonly, - :validate, :touch + :validate, :touch, :inverse_of ] def create_belongs_to_reflection(association_id, options) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 3aef1b21e9..26987dde97 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -399,11 +399,14 @@ module ActiveRecord find(:all) end - @reflection.options[:uniq] ? uniq(records) : records + records = @reflection.options[:uniq] ? uniq(records) : records + records.each do |record| + set_inverse_instance(record, @owner) + end + records end private - def create_record(attrs) attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash) ensure_owner_is_not_new @@ -433,6 +436,7 @@ module ActiveRecord @target ||= [] unless loaded? @target << record unless @reflection.options[:uniq] && @target.include?(record) callback(:after_add, record) + set_inverse_instance(record, @owner) record end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 241b9bfee0..e36b04ea95 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -53,6 +53,7 @@ module ActiveRecord def initialize(owner, reflection) @owner, @reflection = owner, reflection + reflection.check_validity! Array(reflection.options[:extend]).each { |ext| proxy_extend(ext) } reset end @@ -274,6 +275,19 @@ module ActiveRecord def owner_quoted_id @owner.quoted_id end + + def set_inverse_instance(record, instance) + return if record.nil? || !we_can_set_the_inverse_on_this?(record) + inverse_relationship = @reflection.inverse_of + unless inverse_relationship.nil? + record.send(:"set_#{inverse_relationship.name}_target", instance) + end + end + + # Override in subclasses + def we_can_set_the_inverse_on_this?(record) + false + end end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index f05c6be075..c88575048a 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -31,6 +31,8 @@ module ActiveRecord @updated = true end + set_inverse_instance(record, @owner) + loaded record end @@ -41,18 +43,26 @@ module ActiveRecord private def find_target - @reflection.klass.find( + the_target = @reflection.klass.find( @owner[@reflection.primary_key_name], :select => @reflection.options[:select], :conditions => conditions, :include => @reflection.options[:include], :readonly => @reflection.options[:readonly] ) + set_inverse_instance(the_target, @owner) + the_target end def foreign_key_present !@owner[@reflection.primary_key_name].nil? end + + # NOTE - for now, we're only supporting inverse setting from belongs_to back onto + # has_one associations. + def we_can_set_the_inverse_on_this?(record) + @reflection.has_inverse? && @reflection.inverse_of.macro == :has_one + end end end end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index a2cbabfe0c..73dd50dd07 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -116,6 +116,11 @@ module ActiveRecord :create => create_scoping } end + + def we_can_set_the_inverse_on_this?(record) + inverse = @reflection.inverse_of + return !inverse.nil? + end end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 1c091e7d5a..2dca84b911 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -1,11 +1,6 @@ module ActiveRecord module Associations class HasManyThroughAssociation < HasManyAssociation #:nodoc: - def initialize(owner, reflection) - reflection.check_validity! - super - end - alias_method :new, :build def create!(attrs = nil) @@ -251,6 +246,11 @@ module ActiveRecord def cached_counter_attribute_name "#{@reflection.name}_count" end + + # NOTE - not sure that we can actually cope with inverses here + def we_can_set_the_inverse_on_this?(record) + false + end end end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 1464227bb0..4908005d2e 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -74,13 +74,15 @@ module ActiveRecord private def find_target - @reflection.klass.find(:first, + the_target = @reflection.klass.find(:first, :conditions => @finder_sql, :select => @reflection.options[:select], :order => @reflection.options[:order], :include => @reflection.options[:include], :readonly => @reflection.options[:readonly] ) + set_inverse_instance(the_target, @owner) + the_target end def construct_sql @@ -117,8 +119,15 @@ module ActiveRecord self.target = record end + set_inverse_instance(record, @owner) + record end + + def we_can_set_the_inverse_on_this?(record) + inverse = @reflection.inverse_of + return !inverse.nil? + end end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2d4c1d5507..ec0175497d 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -212,6 +212,13 @@ module ActiveRecord end def check_validity! + check_validity_of_inverse! + end + + def check_validity_of_inverse! + if has_inverse? && inverse_of.nil? + raise InverseOfAssociationNotFoundError.new(self) + end end def through_reflection @@ -225,6 +232,18 @@ module ActiveRecord nil end + def has_inverse? + !@options[:inverse_of].nil? + end + + def inverse_of + if has_inverse? + @inverse_of ||= klass.reflect_on_association(options[:inverse_of]) + else + nil + end + end + private def derive_class_name class_name = name.to_s.camelize @@ -300,6 +319,8 @@ module ActiveRecord unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil? raise HasManyThroughSourceAssociationMacroError.new(self) end + + check_validity_of_inverse! end def through_reflection_primary_key diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb new file mode 100644 index 0000000000..616f8dfbbe --- /dev/null +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -0,0 +1,252 @@ +require "cases/helper" +require 'models/man' +require 'models/face' +require 'models/interest' +require 'models/zine' +require 'models/club' +require 'models/sponsor' + +class InverseAssociationTests < ActiveRecord::TestCase + def test_should_allow_for_inverse_of_options_in_associations + assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_many') do + Class.new(ActiveRecord::Base).has_many(:wheels, :inverse_of => :car) + end + + assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_one') do + Class.new(ActiveRecord::Base).has_one(:engine, :inverse_of => :car) + end + + assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on belongs_to') do + Class.new(ActiveRecord::Base).belongs_to(:car, :inverse_of => :driver) + end + end + + def test_should_be_able_to_ask_a_reflection_if_it_has_an_inverse + has_one_with_inverse_ref = Man.reflect_on_association(:face) + assert has_one_with_inverse_ref.respond_to?(:has_inverse?) + assert has_one_with_inverse_ref.has_inverse? + + has_many_with_inverse_ref = Man.reflect_on_association(:interests) + assert has_many_with_inverse_ref.respond_to?(:has_inverse?) + assert has_many_with_inverse_ref.has_inverse? + + belongs_to_with_inverse_ref = Face.reflect_on_association(:man) + assert belongs_to_with_inverse_ref.respond_to?(:has_inverse?) + assert belongs_to_with_inverse_ref.has_inverse? + + has_one_without_inverse_ref = Club.reflect_on_association(:sponsor) + assert has_one_without_inverse_ref.respond_to?(:has_inverse?) + assert !has_one_without_inverse_ref.has_inverse? + + has_many_without_inverse_ref = Club.reflect_on_association(:memberships) + assert has_many_without_inverse_ref.respond_to?(:has_inverse?) + assert !has_many_without_inverse_ref.has_inverse? + + belongs_to_without_inverse_ref = Sponsor.reflect_on_association(:sponsor_club) + assert belongs_to_without_inverse_ref.respond_to?(:has_inverse?) + assert !belongs_to_without_inverse_ref.has_inverse? + end + + def test_should_be_able_to_ask_a_reflection_what_it_is_the_inverse_of + has_one_ref = Man.reflect_on_association(:face) + assert has_one_ref.respond_to?(:inverse_of) + + has_many_ref = Man.reflect_on_association(:interests) + assert has_many_ref.respond_to?(:inverse_of) + + belongs_to_ref = Face.reflect_on_association(:man) + assert belongs_to_ref.respond_to?(:inverse_of) + end + + def test_inverse_of_method_should_supply_the_actual_reflection_instance_it_is_the_inverse_of + has_one_ref = Man.reflect_on_association(:face) + assert_equal Face.reflect_on_association(:man), has_one_ref.inverse_of + + has_many_ref = Man.reflect_on_association(:interests) + assert_equal Interest.reflect_on_association(:man), has_many_ref.inverse_of + + belongs_to_ref = Face.reflect_on_association(:man) + assert_equal Man.reflect_on_association(:face), belongs_to_ref.inverse_of + end + + def test_associations_with_no_inverse_of_should_return_nil + has_one_ref = Club.reflect_on_association(:sponsor) + assert_nil has_one_ref.inverse_of + + has_many_ref = Club.reflect_on_association(:memberships) + assert_nil has_many_ref.inverse_of + + belongs_to_ref = Sponsor.reflect_on_association(:sponsor_club) + assert_nil belongs_to_ref.inverse_of + end +end + +class InverseHasOneTests < ActiveRecord::TestCase + fixtures :men, :faces + + def test_parent_instance_should_be_shared_with_child_on_find + m = Man.find(:first) + f = m.face + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_built_child + m = Man.find(:first) + f = m.build_face(:description => 'haunted') + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to just-built-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_child + m = Man.find(:first) + f = m.create_face(:description => 'haunted') + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).dirty_face } + end +end + +class InverseHasManyTests < ActiveRecord::TestCase + fixtures :men, :interests + + def test_parent_instance_should_be_shared_with_every_child_on_find + m = Man.find(:first) + is = m.interests + is.each do |i| + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance" + end + end + + def test_parent_instance_should_be_shared_with_newly_built_child + m = Man.find(:first) + i = m.interests.build(:topic => 'Industrial Revolution Re-enactment') + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to just-built-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_child + m = Man.find(:first) + i = m.interests.create(:topic => 'Industrial Revolution Re-enactment') + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_poked_in_child + m = Man.find(:first) + i = Interest.create(:topic => 'Industrial Revolution Re-enactment') + m.interests << i + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests } + end +end + +class InverseBelongsToTests < ActiveRecord::TestCase + fixtures :men, :faces, :interests + + def test_child_instance_should_be_shared_with_parent_on_find + f = Face.find(:first) + m = f.man + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance" + end + + def test_child_instance_should_be_shared_with_newly_built_parent + f = Face.find(:first) + m = f.build_man(:name => 'Charles') + assert_not_nil m.face + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to just-built-parent-owned instance" + end + + def test_child_instance_should_be_shared_with_newly_created_parent + f = Face.find(:first) + m = f.create_man(:name => 'Charles') + assert_not_nil m.face + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to newly-created-parent-owned instance" + end + + def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many + i = Interest.find(:first) + m = i.man + assert_not_nil m.interests + iz = m.interests.detect {|iz| iz.id == i.id} + assert_not_nil iz + assert_equal i.topic, iz.topic, "Interest topics should be the same before changes to child" + i.topic = 'Eating cheese with a spoon' + assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to child" + iz.topic = 'Cow tipping' + assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance" + end + + def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man } + end +end + +# NOTE - these tests might not be meaningful, ripped as they were from the parental_control plugin +# which would guess the inverse rather than look for an explicit configuration option. +class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase + fixtures :men, :interests, :zines + + def test_that_we_can_load_associations_that_have_the_same_reciprocal_name_from_different_models + assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do + i = Interest.find(:first) + z = i.zine + m = i.man + end + end + + def test_that_we_can_create_associations_that_have_the_same_reciprocal_name_from_different_models + assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do + i = Interest.find(:first) + i.build_zine(:title => 'Get Some in Winter! 2008') + i.build_man(:name => 'Gordon') + i.save! + end + end +end diff --git a/activerecord/test/fixtures/faces.yml b/activerecord/test/fixtures/faces.yml new file mode 100644 index 0000000000..1dd2907cf7 --- /dev/null +++ b/activerecord/test/fixtures/faces.yml @@ -0,0 +1,7 @@ +trusting: + description: trusting + man: gordon + +weather_beaten: + description: weather beaten + man: steve diff --git a/activerecord/test/fixtures/interests.yml b/activerecord/test/fixtures/interests.yml new file mode 100644 index 0000000000..ec71890ab6 --- /dev/null +++ b/activerecord/test/fixtures/interests.yml @@ -0,0 +1,29 @@ +trainspotting: + topic: Trainspotting + zine: staying_in + man: gordon + +birdwatching: + topic: Birdwatching + zine: staying_in + man: gordon + +stamp_collecting: + topic: Stamp Collecting + zine: staying_in + man: gordon + +hunting: + topic: Hunting + zine: going_out + man: steve + +woodsmanship: + topic: Woodsmanship + zine: going_out + man: steve + +survial: + topic: Survival + zine: going_out + man: steve diff --git a/activerecord/test/fixtures/men.yml b/activerecord/test/fixtures/men.yml new file mode 100644 index 0000000000..c67429f925 --- /dev/null +++ b/activerecord/test/fixtures/men.yml @@ -0,0 +1,5 @@ +gordon: + name: Gordon + +steve: + name: Steve diff --git a/activerecord/test/fixtures/zines.yml b/activerecord/test/fixtures/zines.yml new file mode 100644 index 0000000000..07dce4db7e --- /dev/null +++ b/activerecord/test/fixtures/zines.yml @@ -0,0 +1,5 @@ +staying_in: + title: Staying in '08 + +going_out: + title: Outdoor Pursuits 2k+8 diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb new file mode 100644 index 0000000000..1540dbf741 --- /dev/null +++ b/activerecord/test/models/face.rb @@ -0,0 +1,5 @@ +class Face < ActiveRecord::Base + belongs_to :man, :inverse_of => :face + # This is a "broken" inverse_of for the purposes of testing + belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face +end diff --git a/activerecord/test/models/interest.rb b/activerecord/test/models/interest.rb new file mode 100644 index 0000000000..d8291d00cc --- /dev/null +++ b/activerecord/test/models/interest.rb @@ -0,0 +1,4 @@ +class Interest < ActiveRecord::Base + belongs_to :man, :inverse_of => :interests + belongs_to :zine, :inverse_of => :interests +end diff --git a/activerecord/test/models/man.rb b/activerecord/test/models/man.rb new file mode 100644 index 0000000000..f40bc9d0fc --- /dev/null +++ b/activerecord/test/models/man.rb @@ -0,0 +1,7 @@ +class Man < ActiveRecord::Base + has_one :face, :inverse_of => :man + has_many :interests, :inverse_of => :man + # These are "broken" inverse_of associations for the purposes of testing + has_one :dirty_face, :class_name => 'Face', :inverse_of => :dirty_man + has_many :secret_interests, :class_name => 'Interest', :inverse_of => :secret_man +end diff --git a/activerecord/test/models/zine.rb b/activerecord/test/models/zine.rb new file mode 100644 index 0000000000..c2d0fdaf25 --- /dev/null +++ b/activerecord/test/models/zine.rb @@ -0,0 +1,3 @@ +class Zine < ActiveRecord::Base + has_many :interests, :inverse_of => :zine +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 6918a4fcab..3b0e17c867 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -468,6 +468,26 @@ ActiveRecord::Schema.define do end end + # NOTE - the following 4 tables are used by models that have :inverse_of options on the associations + create_table :men, :force => true do |t| + t.string :name + end + + create_table :faces, :force => true do |t| + t.string :description + t.integer :man_id + end + + create_table :interests, :force => true do |t| + t.string :topic + t.integer :man_id + t.integer :zine_id + end + + create_table :zines, :force => true do |t| + t.string :title + end + except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk create_table :fk_test_has_fk, :force => true do |t| -- cgit v1.2.3 From b9f9dd8fe9d84c22450c1cf24b6c8a41eae8bbfd Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 4 May 2009 20:02:14 -0500 Subject: Fix console --- railties/lib/console_app.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/railties/lib/console_app.rb b/railties/lib/console_app.rb index 5c8302634a..c944d49205 100644 --- a/railties/lib/console_app.rb +++ b/railties/lib/console_app.rb @@ -26,7 +26,8 @@ end #reloads the environment def reload! puts "Reloading..." - Dispatcher.cleanup_application - Dispatcher.reload_application + dispatcher = ActionController::Dispatcher.new + dispatcher.cleanup_application + dispatcher.reload_application true end -- cgit v1.2.3 From e3891601d137a400722cfd0f941b9195b0a8217b Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Mon, 4 May 2009 20:11:24 -0500 Subject: fix problems with requires in metal choking under development reloading [#2579 state:resolved] Signed-off-by: Joshua Peek --- railties/lib/rails/rack/metal.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/rack/metal.rb b/railties/lib/rails/rack/metal.rb index 7a616c7911..b031be29af 100644 --- a/railties/lib/rails/rack/metal.rb +++ b/railties/lib/rails/rack/metal.rb @@ -1,5 +1,6 @@ require 'active_support/ordered_hash' require 'active_support/core_ext/class/attribute_accessors' +require 'active_support/dependencies' module Rails module Rack @@ -27,7 +28,7 @@ module Rails load_list.map do |requested_metal| if metal = all_metals[requested_metal] - require metal + require_dependency metal requested_metal.constantize end end.compact -- cgit v1.2.3 From d4402c5554a0d3202fc22eb38c5c1a8c4b9afd93 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 4 May 2009 20:26:43 -0500 Subject: assert_redirect_to's partial hash matching was deprecated in 2-3 stable 7f1f16c01 --- actionpack/test/controller/redirect_test.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 9ad63a1c60..13247f2d08 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -233,13 +233,6 @@ class RedirectTest < ActionController::TestCase assert_redirected_to Workshop.new(5, true) end - def test_redirect_with_partial_params - pending do - get :module_redirect - assert_redirected_to :action => 'hello_world' - end - end - def test_redirect_to_nil assert_raise(ActionController::ActionControllerError) do get :redirect_to_nil -- cgit v1.2.3 From 2f42433f0a9d777e2a8f5fa05370ba464823933c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 5 May 2009 21:40:53 -0700 Subject: Cherry-pick the inflector only --- activemodel/lib/active_model/core.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/activemodel/lib/active_model/core.rb b/activemodel/lib/active_model/core.rb index b4b020defc..4201bcf158 100644 --- a/activemodel/lib/active_model/core.rb +++ b/activemodel/lib/active_model/core.rb @@ -1,12 +1,3 @@ -begin - require 'active_support' -rescue LoadError - activesupport_path = "#{File.dirname(__FILE__)}/../../../activesupport/lib" - if File.directory?(activesupport_path) - $:.unshift activesupport_path - require 'active_support' - end -end - -# So far, we only need the string inflections and not the rest of ActiveSupport. +activesupport_path = "#{File.dirname(__FILE__)}/../../../activesupport/lib" +$:.unshift(activesupport_path) if File.directory?(activesupport_path) require 'active_support/inflector' -- cgit v1.2.3 From a05cfb6df5336aa0369e6b27294dae3c418a75c6 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 5 May 2009 21:44:02 -0700 Subject: Prefer sibling Active Support --- activeresource/lib/active_resource.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/activeresource/lib/active_resource.rb b/activeresource/lib/active_resource.rb index 2f0c8d1a8e..7a3faf445a 100644 --- a/activeresource/lib/active_resource.rb +++ b/activeresource/lib/active_resource.rb @@ -21,15 +21,9 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #++ -begin - require 'active_support' -rescue LoadError - activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" - if File.directory?(activesupport_path) - $:.unshift activesupport_path - require 'active_support' - end -end +activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" +$:.unshift(activesupport_path) if File.directory?(activesupport_path) +require 'active_support' require 'active_support/core/all' require 'active_resource/formats' -- cgit v1.2.3 From c585e263ab40101eb0fd71a1d24d0d704f4ce026 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 5 May 2009 21:50:53 -0700 Subject: Remove superfluous CGI require --- activeresource/lib/active_resource/base.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index f9a461d02e..590d4f6232 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -1,5 +1,4 @@ require 'active_resource/connection' -require 'cgi' require 'set' module ActiveResource -- cgit v1.2.3 From 6d4a4fabbbb04c20cee51c4e374045cc75e2ec16 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 6 May 2009 00:14:55 -0700 Subject: Cherry pick Active Support dependencies. Autoload. --- activeresource/lib/active_resource.rb | 11 +--- activeresource/lib/active_resource/base.rb | 16 ++++-- activeresource/lib/active_resource/connection.rb | 60 ++-------------------- .../lib/active_resource/custom_methods.rb | 4 ++ activeresource/lib/active_resource/exceptions.rb | 55 ++++++++++++++++++++ activeresource/lib/active_resource/formats.rb | 8 +-- .../lib/active_resource/formats/xml_format.rb | 2 + activeresource/lib/active_resource/http_mock.rb | 7 ++- activeresource/lib/active_resource/validations.rb | 7 +++ activeresource/test/abstract_unit.rb | 9 ++-- activeresource/test/base_test.rb | 2 +- 11 files changed, 100 insertions(+), 81 deletions(-) create mode 100644 activeresource/lib/active_resource/exceptions.rb diff --git a/activeresource/lib/active_resource.rb b/activeresource/lib/active_resource.rb index 7a3faf445a..720abee72e 100644 --- a/activeresource/lib/active_resource.rb +++ b/activeresource/lib/active_resource.rb @@ -24,16 +24,7 @@ activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" $:.unshift(activesupport_path) if File.directory?(activesupport_path) require 'active_support' -require 'active_support/core/all' - -require 'active_resource/formats' -require 'active_resource/base' -require 'active_resource/validations' -require 'active_resource/custom_methods' module ActiveResource - Base.class_eval do - include Validations - include CustomMethods - end + autoload :Base, 'active_resource/base' end diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 590d4f6232..be022f5c44 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -1,7 +1,14 @@ -require 'active_resource/connection' +require 'active_support/core_ext/class/attribute_accessors' +require 'active_support/core_ext/class/inheritable_attributes' +require 'active_support/core_ext/module/attr_accessor_with_default' +require 'active_support/core_ext/module/delegation' +require 'active_support/core_ext/module/aliasing' require 'set' module ActiveResource + autoload :Formats, 'active_resource/formats' + autoload :Connection, 'active_resource/connection' + # ActiveResource::Base is the main class for mapping RESTful resources as models in a Rails application. # # For an outline of what Active Resource is capable of, see link:files/vendor/rails/activeresource/README.html. @@ -297,7 +304,7 @@ module ActiveResource # Returns the current format, default is ActiveResource::Formats::XmlFormat. def format - read_inheritable_attribute(:format) || ActiveResource::Formats[:xml] + read_inheritable_attribute(:format) || ActiveResource::Formats::XmlFormat end # Sets the number of seconds after which requests to the REST API should time out. @@ -894,7 +901,7 @@ module ActiveResource # applicable depend on the configured encoding format. def encode(options={}) case self.class.format - when ActiveResource::Formats[:xml] + when ActiveResource::Formats::XmlFormat self.class.format.encode(attributes, {:root => self.class.element_name}.merge(options)) else self.class.format.encode(attributes, options) @@ -1079,3 +1086,6 @@ module ActiveResource end end end + +require 'active_resource/validations' +require 'active_resource/custom_methods' diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index 80d5c95b68..6661469c5b 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -1,64 +1,12 @@ +require 'active_resource/exceptions' +require 'active_resource/formats' +require 'active_support/core_ext/benchmark' require 'net/https' require 'date' require 'time' require 'uri' -require 'benchmark' module ActiveResource - class ConnectionError < StandardError # :nodoc: - attr_reader :response - - def initialize(response, message = nil) - @response = response - @message = message - end - - def to_s - "Failed with #{response.code} #{response.message if response.respond_to?(:message)}" - end - end - - # Raised when a Timeout::Error occurs. - class TimeoutError < ConnectionError - def initialize(message) - @message = message - end - def to_s; @message ;end - end - - # 3xx Redirection - class Redirection < ConnectionError # :nodoc: - def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end - end - - # 4xx Client Error - class ClientError < ConnectionError; end # :nodoc: - - # 400 Bad Request - class BadRequest < ClientError; end # :nodoc - - # 401 Unauthorized - class UnauthorizedAccess < ClientError; end # :nodoc - - # 403 Forbidden - class ForbiddenAccess < ClientError; end # :nodoc - - # 404 Not Found - class ResourceNotFound < ClientError; end # :nodoc: - - # 409 Conflict - class ResourceConflict < ClientError; end # :nodoc: - - # 5xx Server Error - class ServerError < ConnectionError; end # :nodoc: - - # 405 Method Not Allowed - class MethodNotAllowed < ClientError # :nodoc: - def allowed_methods - @response['Allow'].split(',').map { |verb| verb.strip.downcase.to_sym } - end - end - # Class to handle connections to remote web services. # This class is used by ActiveResource::Base to interface with REST # services. @@ -81,7 +29,7 @@ module ActiveResource # The +site+ parameter is required and will set the +site+ # attribute to the URI for the remote resource service. - def initialize(site, format = ActiveResource::Formats[:xml]) + def initialize(site, format = ActiveResource::Formats::XmlFormat) raise ArgumentError, 'Missing site URI' unless site @user = @password = nil self.site = site diff --git a/activeresource/lib/active_resource/custom_methods.rb b/activeresource/lib/active_resource/custom_methods.rb index 4647e8342c..0d05d06035 100644 --- a/activeresource/lib/active_resource/custom_methods.rb +++ b/activeresource/lib/active_resource/custom_methods.rb @@ -117,4 +117,8 @@ module ActiveResource end end end + + class Base + include CustomMethods + end end diff --git a/activeresource/lib/active_resource/exceptions.rb b/activeresource/lib/active_resource/exceptions.rb new file mode 100644 index 0000000000..5e4b1d4487 --- /dev/null +++ b/activeresource/lib/active_resource/exceptions.rb @@ -0,0 +1,55 @@ +module ActiveResource + class ConnectionError < StandardError # :nodoc: + attr_reader :response + + def initialize(response, message = nil) + @response = response + @message = message + end + + def to_s + "Failed with #{response.code} #{response.message if response.respond_to?(:message)}" + end + end + + # Raised when a Timeout::Error occurs. + class TimeoutError < ConnectionError + def initialize(message) + @message = message + end + def to_s; @message ;end + end + + # 3xx Redirection + class Redirection < ConnectionError # :nodoc: + def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end + end + + # 4xx Client Error + class ClientError < ConnectionError; end # :nodoc: + + # 400 Bad Request + class BadRequest < ClientError; end # :nodoc + + # 401 Unauthorized + class UnauthorizedAccess < ClientError; end # :nodoc + + # 403 Forbidden + class ForbiddenAccess < ClientError; end # :nodoc + + # 404 Not Found + class ResourceNotFound < ClientError; end # :nodoc: + + # 409 Conflict + class ResourceConflict < ClientError; end # :nodoc: + + # 5xx Server Error + class ServerError < ConnectionError; end # :nodoc: + + # 405 Method Not Allowed + class MethodNotAllowed < ClientError # :nodoc: + def allowed_methods + @response['Allow'].split(',').map { |verb| verb.strip.downcase.to_sym } + end + end +end diff --git a/activeresource/lib/active_resource/formats.rb b/activeresource/lib/active_resource/formats.rb index 28864cf588..53b75b34e7 100644 --- a/activeresource/lib/active_resource/formats.rb +++ b/activeresource/lib/active_resource/formats.rb @@ -1,14 +1,14 @@ module ActiveResource module Formats + autoload :XmlFormat, 'active_resource/formats/xml_format' + autoload :JsonFormat, 'active_resource/formats/json_format' + # Lookup the format class from a mime type reference symbol. Example: # # ActiveResource::Formats[:xml] # => ActiveResource::Formats::XmlFormat # ActiveResource::Formats[:json] # => ActiveResource::Formats::JsonFormat def self.[](mime_type_reference) - ActiveResource::Formats.const_get(mime_type_reference.to_s.camelize + "Format") + ActiveResource::Formats.const_get(ActiveSupport::Inflector.camelize(mime_type_reference.to_s) + "Format") end end end - -require 'active_resource/formats/xml_format' -require 'active_resource/formats/json_format' \ No newline at end of file diff --git a/activeresource/lib/active_resource/formats/xml_format.rb b/activeresource/lib/active_resource/formats/xml_format.rb index 86c6cec745..3b2575cfa1 100644 --- a/activeresource/lib/active_resource/formats/xml_format.rb +++ b/activeresource/lib/active_resource/formats/xml_format.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/hash/conversions' + module ActiveResource module Formats module XmlFormat diff --git a/activeresource/lib/active_resource/http_mock.rb b/activeresource/lib/active_resource/http_mock.rb index 7d7e378436..aae2d6508c 100644 --- a/activeresource/lib/active_resource/http_mock.rb +++ b/activeresource/lib/active_resource/http_mock.rb @@ -1,4 +1,5 @@ require 'active_resource/connection' +require 'active_support/core_ext/kernel/reporting' module ActiveResource class InvalidRequestError < StandardError; end #:nodoc: @@ -129,7 +130,11 @@ module ActiveResource def #{method}(path, #{'body, ' if has_body}headers) request = ActiveResource::Request.new(:#{method}, path, #{has_body ? 'body, ' : 'nil, '}headers) self.class.requests << request - self.class.responses.assoc(request).try(:second) || raise(InvalidRequestError.new("No response recorded for \#{request}")) + if response = self.class.responses.assoc(request) + response[1] + else + raise InvalidRequestError.new("No response recorded for \#{request}") + end end EOE end diff --git a/activeresource/lib/active_resource/validations.rb b/activeresource/lib/active_resource/validations.rb index 8d21f8adbb..da6084fe9f 100644 --- a/activeresource/lib/active_resource/validations.rb +++ b/activeresource/lib/active_resource/validations.rb @@ -1,3 +1,6 @@ +require 'active_resource/exceptions' +require 'active_support/core_ext/array/wrap' + module ActiveResource class ResourceInvalid < ClientError #:nodoc: end @@ -272,4 +275,8 @@ module ActiveResource @errors ||= Errors.new(self) end end + + class Base + include Validations + end end diff --git a/activeresource/test/abstract_unit.rb b/activeresource/test/abstract_unit.rb index 0f11ea482a..3398f2dac7 100644 --- a/activeresource/test/abstract_unit.rb +++ b/activeresource/test/abstract_unit.rb @@ -5,19 +5,16 @@ gem 'mocha', '>= 0.9.5' require 'mocha' $:.unshift "#{File.dirname(__FILE__)}/../lib" -$:.unshift "#{File.dirname(__FILE__)}/../../activesupport/lib" require 'active_resource' require 'active_resource/http_mock' $:.unshift "#{File.dirname(__FILE__)}/../test" require 'setter_trap' +require 'logger' ActiveResource::Base.logger = Logger.new("#{File.dirname(__FILE__)}/debug.log") -def uses_gem(gem_name, test_name, version = '> 0') - gem gem_name.to_s, version - require gem_name.to_s - yield +begin + require 'ruby-debug' rescue LoadError - $stderr.puts "Skipping #{test_name} tests. `gem install #{gem_name}` and try again." end diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb index 6ed6f1a406..a6cef6b2ae 100644 --- a/activeresource/test/base_test.rb +++ b/activeresource/test/base_test.rb @@ -853,7 +853,7 @@ class BaseTest < Test::Unit::TestCase def test_to_xml matz = Person.find(1) xml = matz.encode - assert xml.starts_with?('') + assert xml.include?('') assert xml.include?('Matz') assert xml.include?('1') end -- cgit v1.2.3 From bd7659e014482a25b7ab6c6242a2b0926dcfc4d2 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 6 May 2009 01:28:06 -0700 Subject: Fix old reference to ActionController::Failsafe --- railties/lib/initializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 71366a4480..9d27488e8a 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -462,7 +462,7 @@ Run `rake gems:install` to install the missing gems. if RAILS_CACHE.respond_to?(:middleware) # Insert middleware to setup and teardown local cache for each request - configuration.middleware.insert_after(:"ActionController::Failsafe", RAILS_CACHE.middleware) + configuration.middleware.insert_after(:"ActionDispatch::Failsafe", RAILS_CACHE.middleware) end end end -- cgit v1.2.3 From 201d8b17552c8c9eb96e2aca2f871e0fd9b96e15 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 6 May 2009 12:19:30 -0700 Subject: Fix tests on 1.8.6 --- activeresource/lib/active_resource/base.rb | 1 + activeresource/test/base/load_test.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index be022f5c44..8a1236c9a8 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -3,6 +3,7 @@ require 'active_support/core_ext/class/inheritable_attributes' require 'active_support/core_ext/module/attr_accessor_with_default' require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/module/aliasing' +require 'active_support/core_ext/object/misc' require 'set' module ActiveResource diff --git a/activeresource/test/base/load_test.rb b/activeresource/test/base/load_test.rb index a475fab34e..cd2103acb7 100644 --- a/activeresource/test/base/load_test.rb +++ b/activeresource/test/base/load_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require "fixtures/person" require "fixtures/street_address" +require 'active_support/core_ext/symbol' module Highrise class Note < ActiveResource::Base -- cgit v1.2.3 From bfc1609a501fc3ed442685819de5bcdb5fbada1c Mon Sep 17 00:00:00 2001 From: Matt Jones Date: Wed, 6 May 2009 19:35:54 -0500 Subject: Remove stray call to gems:unpack in gems:build:force [#2266 state:committed] Signed-off-by: Jeremy Kemper --- railties/lib/tasks/gems.rake | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/railties/lib/tasks/gems.rake b/railties/lib/tasks/gems.rake index efadb1da3b..7cf7061f38 100644 --- a/railties/lib/tasks/gems.rake +++ b/railties/lib/tasks/gems.rake @@ -27,8 +27,7 @@ namespace :gems do desc "Force the build of all gems" task :force do $gems_build_rake_task = true - Rake::Task['gems:unpack'].invoke - current_gems.each { |gem| gem.build(:force => true) } + frozen_gems.each { |gem| gem.build(:force => true) } end end -- cgit v1.2.3 From 783deae99a4850f597135146b19e7ee4622da94e Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 7 May 2009 10:03:39 -0500 Subject: Add test coverage to module setup extensions --- activesupport/test/core_ext/module/setup_test.rb | 74 ++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 activesupport/test/core_ext/module/setup_test.rb diff --git a/activesupport/test/core_ext/module/setup_test.rb b/activesupport/test/core_ext/module/setup_test.rb new file mode 100644 index 0000000000..132790f2ff --- /dev/null +++ b/activesupport/test/core_ext/module/setup_test.rb @@ -0,0 +1,74 @@ +require 'abstract_unit' +require 'active_support/core/time' +require 'active_support/core_ext/module/setup' + +class SetupTest < Test::Unit::TestCase + module Baz + module ClassMethods + def baz + "baz" + end + + def setup=(value) + @@setup = value + end + + def setup + @@setup + end + end + + setup do + self.setup = true + end + + def baz + "baz" + end + end + + module Bar + depends_on Baz + + def bar + "bar" + end + + def baz + "bar+" + super + end + end + + def setup + @klass = Class.new + end + + def test_module_is_included_normally + @klass.use(Baz) + assert_equal "baz", @klass.new.baz + assert_equal SetupTest::Baz, @klass.included_modules[0] + + @klass.use(Baz) + assert_equal "baz", @klass.new.baz + assert_equal SetupTest::Baz, @klass.included_modules[0] + end + + def test_class_methods_are_extended + @klass.use(Baz) + assert_equal "baz", @klass.baz + assert_equal SetupTest::Baz::ClassMethods, (class << @klass; self.included_modules; end)[0] + end + + def test_setup_block_is_ran + @klass.use(Baz) + assert_equal true, @klass.setup + end + + def test_modules_dependencies_are_met + @klass.use(Bar) + assert_equal "bar", @klass.new.bar + assert_equal "bar+baz", @klass.new.baz + assert_equal "baz", @klass.baz + assert_equal [SetupTest::Bar, SetupTest::Baz], @klass.included_modules[0..1] + end +end -- cgit v1.2.3 From 2854535b02bcee3668bda02c76c3105fc24730d3 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 7 May 2009 10:29:22 -0500 Subject: Make module dependency DSL opt in --- .../lib/action_controller/abstract/helpers.rb | 2 + .../lib/action_controller/abstract/layouts.rb | 5 +- .../lib/action_controller/abstract/logger.rb | 2 + .../lib/action_controller/abstract/renderer.rb | 2 + .../lib/action_controller/new_base/layouts.rb | 2 + .../lib/action_controller/new_base/renderer.rb | 2 + .../test/abstract_controller/callbacks_test.rb | 2 +- .../test/abstract_controller/layouts_test.rb | 6 +- actionpack/test/new_base/test_helper.rb | 14 ++-- activesupport/lib/active_support.rb | 1 + .../lib/active_support/core_ext/module.rb | 1 - .../lib/active_support/core_ext/module/setup.rb | 26 -------- .../lib/active_support/dependency_module.rb | 24 +++++++ activesupport/test/core_ext/module/setup_test.rb | 74 --------------------- activesupport/test/dependency_module_test.rb | 77 ++++++++++++++++++++++ 15 files changed, 126 insertions(+), 114 deletions(-) delete mode 100644 activesupport/lib/active_support/core_ext/module/setup.rb create mode 100644 activesupport/lib/active_support/dependency_module.rb delete mode 100644 activesupport/test/core_ext/module/setup_test.rb create mode 100644 activesupport/test/dependency_module_test.rb diff --git a/actionpack/lib/action_controller/abstract/helpers.rb b/actionpack/lib/action_controller/abstract/helpers.rb index 1f0b38417b..ffcae29ddc 100644 --- a/actionpack/lib/action_controller/abstract/helpers.rb +++ b/actionpack/lib/action_controller/abstract/helpers.rb @@ -1,5 +1,7 @@ module AbstractController module Helpers + extend ActiveSupport::DependencyModule + depends_on Renderer setup do diff --git a/actionpack/lib/action_controller/abstract/layouts.rb b/actionpack/lib/action_controller/abstract/layouts.rb index 0039e67c5a..76130f8dc0 100644 --- a/actionpack/lib/action_controller/abstract/layouts.rb +++ b/actionpack/lib/action_controller/abstract/layouts.rb @@ -1,8 +1,9 @@ module AbstractController module Layouts - + extend ActiveSupport::DependencyModule + depends_on Renderer - + module ClassMethods def layout(layout) unless [String, Symbol, FalseClass, NilClass].include?(layout.class) diff --git a/actionpack/lib/action_controller/abstract/logger.rb b/actionpack/lib/action_controller/abstract/logger.rb index 4117369bd4..bdb8defa0c 100644 --- a/actionpack/lib/action_controller/abstract/logger.rb +++ b/actionpack/lib/action_controller/abstract/logger.rb @@ -1,5 +1,7 @@ module AbstractController module Logger + extend ActiveSupport::DependencyModule + setup do cattr_accessor :logger end diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index e31accbbfc..531eb28dcb 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -2,6 +2,8 @@ require "action_controller/abstract/logger" module AbstractController module Renderer + extend ActiveSupport::DependencyModule + depends_on AbstractController::Logger setup do diff --git a/actionpack/lib/action_controller/new_base/layouts.rb b/actionpack/lib/action_controller/new_base/layouts.rb index a8e0809ac6..89d24fe92d 100644 --- a/actionpack/lib/action_controller/new_base/layouts.rb +++ b/actionpack/lib/action_controller/new_base/layouts.rb @@ -1,5 +1,7 @@ module ActionController module Layouts + extend ActiveSupport::DependencyModule + depends_on ActionController::Renderer depends_on AbstractController::Layouts diff --git a/actionpack/lib/action_controller/new_base/renderer.rb b/actionpack/lib/action_controller/new_base/renderer.rb index ed34c46aed..be4ea54c3b 100644 --- a/actionpack/lib/action_controller/new_base/renderer.rb +++ b/actionpack/lib/action_controller/new_base/renderer.rb @@ -1,5 +1,7 @@ module ActionController module Renderer + extend ActiveSupport::DependencyModule + depends_on AbstractController::Renderer def initialize(*) diff --git a/actionpack/test/abstract_controller/callbacks_test.rb b/actionpack/test/abstract_controller/callbacks_test.rb index 5fce30f478..89243b631e 100644 --- a/actionpack/test/abstract_controller/callbacks_test.rb +++ b/actionpack/test/abstract_controller/callbacks_test.rb @@ -4,7 +4,7 @@ module AbstractController module Testing class ControllerWithCallbacks < AbstractController::Base - use AbstractController::Callbacks + include AbstractController::Callbacks end class Callback1 < ControllerWithCallbacks diff --git a/actionpack/test/abstract_controller/layouts_test.rb b/actionpack/test/abstract_controller/layouts_test.rb index 3d4570bfef..a305a30a54 100644 --- a/actionpack/test/abstract_controller/layouts_test.rb +++ b/actionpack/test/abstract_controller/layouts_test.rb @@ -5,9 +5,9 @@ module AbstractControllerTests # Base controller for these tests class Base < AbstractController::Base - use AbstractController::Renderer - use AbstractController::Layouts - + include AbstractController::Renderer + include AbstractController::Layouts + self.view_paths = [ActionView::FixtureTemplate::FixturePath.new( "layouts/hello.erb" => "With String <%= yield %>", "layouts/hello_override.erb" => "With Override <%= yield %>", diff --git a/actionpack/test/new_base/test_helper.rb b/actionpack/test/new_base/test_helper.rb index d29449ddc1..89c29d9af3 100644 --- a/actionpack/test/new_base/test_helper.rb +++ b/actionpack/test/new_base/test_helper.rb @@ -26,14 +26,14 @@ require 'rack/test' module ActionController class Base2 < AbstractBase - use AbstractController::Callbacks - use AbstractController::Helpers - use AbstractController::Logger + include AbstractController::Callbacks + include AbstractController::Helpers + include AbstractController::Logger - use ActionController::HideActions - use ActionController::UrlFor - use ActionController::Renderer - use ActionController::Layouts + include ActionController::HideActions + include ActionController::UrlFor + include ActionController::Renderer + include ActionController::Layouts def self.inherited(klass) ::ActionController::Base2.subclasses << klass.to_s diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index 0879535487..dab017770d 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -34,6 +34,7 @@ module ActiveSupport autoload :Callbacks, 'active_support/callbacks' autoload :NewCallbacks, 'active_support/new_callbacks' autoload :ConcurrentHash, 'active_support/concurrent_hash' + autoload :DependencyModule, 'active_support/dependency_module' autoload :Deprecation, 'active_support/deprecation' autoload :Gzip, 'active_support/gzip' autoload :Inflector, 'active_support/inflector' diff --git a/activesupport/lib/active_support/core_ext/module.rb b/activesupport/lib/active_support/core_ext/module.rb index fee91534e7..215c47b114 100644 --- a/activesupport/lib/active_support/core_ext/module.rb +++ b/activesupport/lib/active_support/core_ext/module.rb @@ -9,4 +9,3 @@ require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/module/loading' require 'active_support/core_ext/module/model_naming' require 'active_support/core_ext/module/synchronization' -require 'active_support/core_ext/module/setup' diff --git a/activesupport/lib/active_support/core_ext/module/setup.rb b/activesupport/lib/active_support/core_ext/module/setup.rb deleted file mode 100644 index e6dfd0cf56..0000000000 --- a/activesupport/lib/active_support/core_ext/module/setup.rb +++ /dev/null @@ -1,26 +0,0 @@ -class Module - attr_accessor :_setup_block - attr_accessor :_dependencies - - def setup(&blk) - @_setup_block = blk - end - - def use(mod) - return if self < mod - - (mod._dependencies || []).each do |dep| - use dep - end - # raise "Circular dependencies" if self < mod - include mod - extend mod.const_get("ClassMethods") if mod.const_defined?("ClassMethods") - class_eval(&mod._setup_block) if mod._setup_block - end - - def depends_on(mod) - return if self < mod - @_dependencies ||= [] - @_dependencies << mod - end -end \ No newline at end of file diff --git a/activesupport/lib/active_support/dependency_module.rb b/activesupport/lib/active_support/dependency_module.rb new file mode 100644 index 0000000000..0e1cc67b53 --- /dev/null +++ b/activesupport/lib/active_support/dependency_module.rb @@ -0,0 +1,24 @@ +module ActiveSupport + module DependencyModule + def setup(&blk) + @_setup_block = blk + end + + def append_features(base) + return if base < self + (@_dependencies ||= []).each { |dep| base.send(:include, dep) } + super + end + + def included(base) + base.extend const_get("ClassMethods") if const_defined?("ClassMethods") + base.class_eval(&@_setup_block) if instance_variable_defined?("@_setup_block") + end + + def depends_on(mod) + return if self < mod + @_dependencies ||= [] + @_dependencies << mod + end + end +end diff --git a/activesupport/test/core_ext/module/setup_test.rb b/activesupport/test/core_ext/module/setup_test.rb deleted file mode 100644 index 132790f2ff..0000000000 --- a/activesupport/test/core_ext/module/setup_test.rb +++ /dev/null @@ -1,74 +0,0 @@ -require 'abstract_unit' -require 'active_support/core/time' -require 'active_support/core_ext/module/setup' - -class SetupTest < Test::Unit::TestCase - module Baz - module ClassMethods - def baz - "baz" - end - - def setup=(value) - @@setup = value - end - - def setup - @@setup - end - end - - setup do - self.setup = true - end - - def baz - "baz" - end - end - - module Bar - depends_on Baz - - def bar - "bar" - end - - def baz - "bar+" + super - end - end - - def setup - @klass = Class.new - end - - def test_module_is_included_normally - @klass.use(Baz) - assert_equal "baz", @klass.new.baz - assert_equal SetupTest::Baz, @klass.included_modules[0] - - @klass.use(Baz) - assert_equal "baz", @klass.new.baz - assert_equal SetupTest::Baz, @klass.included_modules[0] - end - - def test_class_methods_are_extended - @klass.use(Baz) - assert_equal "baz", @klass.baz - assert_equal SetupTest::Baz::ClassMethods, (class << @klass; self.included_modules; end)[0] - end - - def test_setup_block_is_ran - @klass.use(Baz) - assert_equal true, @klass.setup - end - - def test_modules_dependencies_are_met - @klass.use(Bar) - assert_equal "bar", @klass.new.bar - assert_equal "bar+baz", @klass.new.baz - assert_equal "baz", @klass.baz - assert_equal [SetupTest::Bar, SetupTest::Baz], @klass.included_modules[0..1] - end -end diff --git a/activesupport/test/dependency_module_test.rb b/activesupport/test/dependency_module_test.rb new file mode 100644 index 0000000000..d611b4056c --- /dev/null +++ b/activesupport/test/dependency_module_test.rb @@ -0,0 +1,77 @@ +require 'abstract_unit' +require 'active_support/dependency_module' + +class DependencyModuleTest < Test::Unit::TestCase + module Baz + extend ActiveSupport::DependencyModule + + module ClassMethods + def baz + "baz" + end + + def setup=(value) + @@setup = value + end + + def setup + @@setup + end + end + + setup do + self.setup = true + end + + def baz + "baz" + end + end + + module Bar + extend ActiveSupport::DependencyModule + + depends_on Baz + + def bar + "bar" + end + + def baz + "bar+" + super + end + end + + def setup + @klass = Class.new + end + + def test_module_is_included_normally + @klass.send(:include, Baz) + assert_equal "baz", @klass.new.baz + assert_equal DependencyModuleTest::Baz, @klass.included_modules[0] + + @klass.send(:include, Baz) + assert_equal "baz", @klass.new.baz + assert_equal DependencyModuleTest::Baz, @klass.included_modules[0] + end + + def test_class_methods_are_extended + @klass.send(:include, Baz) + assert_equal "baz", @klass.baz + assert_equal DependencyModuleTest::Baz::ClassMethods, (class << @klass; self.included_modules; end)[0] + end + + def test_setup_block_is_ran + @klass.send(:include, Baz) + assert_equal true, @klass.setup + end + + def test_modules_dependencies_are_met + @klass.send(:include, Bar) + assert_equal "bar", @klass.new.bar + assert_equal "bar+baz", @klass.new.baz + assert_equal "baz", @klass.baz + assert_equal [DependencyModuleTest::Bar, DependencyModuleTest::Baz], @klass.included_modules[0..1] + end +end -- cgit v1.2.3 From af40fa6d036d86895e7be4ef46a615d44eb41ede Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 7 May 2009 10:38:57 -0500 Subject: Prefer "included" language over "setup" --- actionpack/lib/action_controller/abstract/callbacks.rb | 11 +++++++---- actionpack/lib/action_controller/abstract/helpers.rb | 4 ++-- actionpack/lib/action_controller/abstract/logger.rb | 2 +- actionpack/lib/action_controller/abstract/renderer.rb | 10 +++++----- .../lib/action_controller/new_base/hide_actions.rb | 6 +++--- activesupport/lib/active_support/dependency_module.rb | 14 +++++++------- activesupport/test/dependency_module_test.rb | 16 ++++++++-------- 7 files changed, 33 insertions(+), 30 deletions(-) diff --git a/actionpack/lib/action_controller/abstract/callbacks.rb b/actionpack/lib/action_controller/abstract/callbacks.rb index c8b509081c..d7faaf4236 100644 --- a/actionpack/lib/action_controller/abstract/callbacks.rb +++ b/actionpack/lib/action_controller/abstract/callbacks.rb @@ -1,10 +1,13 @@ module AbstractController module Callbacks - setup do - include ActiveSupport::NewCallbacks - define_callbacks :process_action + extend ActiveSupport::DependencyModule + + depends_on ActiveSupport::NewCallbacks + + included do + define_callbacks :process_action end - + def process_action _run_process_action_callbacks(action_name) do super diff --git a/actionpack/lib/action_controller/abstract/helpers.rb b/actionpack/lib/action_controller/abstract/helpers.rb index ffcae29ddc..62caa119e7 100644 --- a/actionpack/lib/action_controller/abstract/helpers.rb +++ b/actionpack/lib/action_controller/abstract/helpers.rb @@ -3,8 +3,8 @@ module AbstractController extend ActiveSupport::DependencyModule depends_on Renderer - - setup do + + included do extlib_inheritable_accessor :master_helper_module self.master_helper_module = Module.new end diff --git a/actionpack/lib/action_controller/abstract/logger.rb b/actionpack/lib/action_controller/abstract/logger.rb index bdb8defa0c..5fb78f1755 100644 --- a/actionpack/lib/action_controller/abstract/logger.rb +++ b/actionpack/lib/action_controller/abstract/logger.rb @@ -2,7 +2,7 @@ module AbstractController module Logger extend ActiveSupport::DependencyModule - setup do + included do cattr_accessor :logger end end diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index 531eb28dcb..beb848f90e 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -5,15 +5,15 @@ module AbstractController extend ActiveSupport::DependencyModule depends_on AbstractController::Logger - - setup do + + included do attr_internal :formats - + extlib_inheritable_accessor :_view_paths - + self._view_paths ||= ActionView::PathSet.new end - + def _action_view @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self) end diff --git a/actionpack/lib/action_controller/new_base/hide_actions.rb b/actionpack/lib/action_controller/new_base/hide_actions.rb index 473a8ea72b..aa420442fb 100644 --- a/actionpack/lib/action_controller/new_base/hide_actions.rb +++ b/actionpack/lib/action_controller/new_base/hide_actions.rb @@ -1,10 +1,10 @@ module ActionController module HideActions - setup do + included do extlib_inheritable_accessor :hidden_actions - self.hidden_actions ||= Set.new + self.hidden_actions ||= Set.new end - + def action_methods() self.class.action_names end def action_names() action_methods end diff --git a/activesupport/lib/active_support/dependency_module.rb b/activesupport/lib/active_support/dependency_module.rb index 0e1cc67b53..c690b49a2b 100644 --- a/activesupport/lib/active_support/dependency_module.rb +++ b/activesupport/lib/active_support/dependency_module.rb @@ -1,18 +1,18 @@ module ActiveSupport module DependencyModule - def setup(&blk) - @_setup_block = blk - end - def append_features(base) return if base < self (@_dependencies ||= []).each { |dep| base.send(:include, dep) } super end - def included(base) - base.extend const_get("ClassMethods") if const_defined?("ClassMethods") - base.class_eval(&@_setup_block) if instance_variable_defined?("@_setup_block") + def included(base = nil, &block) + if base.nil? && block_given? + @_included_block = block + else + base.extend const_get("ClassMethods") if const_defined?("ClassMethods") + base.class_eval(&@_included_block) if instance_variable_defined?("@_included_block") + end end def depends_on(mod) diff --git a/activesupport/test/dependency_module_test.rb b/activesupport/test/dependency_module_test.rb index d611b4056c..07090d15a1 100644 --- a/activesupport/test/dependency_module_test.rb +++ b/activesupport/test/dependency_module_test.rb @@ -10,17 +10,17 @@ class DependencyModuleTest < Test::Unit::TestCase "baz" end - def setup=(value) - @@setup = value + def included_ran=(value) + @@included_ran = value end - def setup - @@setup + def included_ran + @@included_ran end end - setup do - self.setup = true + included do + self.included_ran = true end def baz @@ -62,9 +62,9 @@ class DependencyModuleTest < Test::Unit::TestCase assert_equal DependencyModuleTest::Baz::ClassMethods, (class << @klass; self.included_modules; end)[0] end - def test_setup_block_is_ran + def test_included_block_is_ran @klass.send(:include, Baz) - assert_equal true, @klass.setup + assert_equal true, @klass.included_ran end def test_modules_dependencies_are_met -- cgit v1.2.3 From a747ab5b20b9d543e9d311070e3b720c761ae716 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 7 May 2009 10:45:29 -0500 Subject: Whitespace! --- actionpack/lib/action_controller/abstract.rb | 2 +- actionpack/lib/action_controller/abstract/base.rb | 19 +++++++-------- .../lib/action_controller/abstract/callbacks.rb | 8 +++---- .../lib/action_controller/abstract/exceptions.rb | 2 +- .../lib/action_controller/abstract/helpers.rb | 17 +++++++------- .../lib/action_controller/abstract/layouts.rb | 27 +++++++++++----------- .../lib/action_controller/abstract/logger.rb | 2 +- .../lib/action_controller/abstract/renderer.rb | 19 ++++++++------- actionpack/lib/action_controller/new_base.rb | 2 +- actionpack/lib/action_controller/new_base/base.rb | 27 +++++++++++----------- .../lib/action_controller/new_base/hide_actions.rb | 11 ++++----- .../lib/action_controller/new_base/layouts.rb | 14 +++++------ .../lib/action_controller/new_base/renderer.rb | 23 +++++++++--------- .../lib/action_controller/new_base/url_for.rb | 6 ++--- 14 files changed, 84 insertions(+), 95 deletions(-) diff --git a/actionpack/lib/action_controller/abstract.rb b/actionpack/lib/action_controller/abstract.rb index 3f5c4a185f..9442d4559f 100644 --- a/actionpack/lib/action_controller/abstract.rb +++ b/actionpack/lib/action_controller/abstract.rb @@ -7,4 +7,4 @@ module AbstractController autoload :Renderer, "action_controller/abstract/renderer" # === Exceptions autoload :ActionNotFound, "action_controller/abstract/exceptions" -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/base.rb b/actionpack/lib/action_controller/abstract/base.rb index ade7719cc0..39e9ad0440 100644 --- a/actionpack/lib/action_controller/abstract/base.rb +++ b/actionpack/lib/action_controller/abstract/base.rb @@ -1,41 +1,38 @@ module AbstractController class Base - attr_internal :response_body attr_internal :response_obj attr_internal :action_name - + def self.process(action) new.process(action) end - + def self.inherited(klass) end - + def initialize self.response_obj = {} end - + def process(action_name) unless respond_to_action?(action_name) raise ActionNotFound, "The action '#{action_name}' could not be found" end - + @_action_name = action_name process_action self.response_obj[:body] = self.response_body self end - + private - def process_action respond_to?(action_name) ? send(action_name) : send(:action_missing, action_name) end - + def respond_to_action?(action_name) respond_to?(action_name) || respond_to?(:action_missing, true) end - end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/callbacks.rb b/actionpack/lib/action_controller/abstract/callbacks.rb index d7faaf4236..cb8aade0be 100644 --- a/actionpack/lib/action_controller/abstract/callbacks.rb +++ b/actionpack/lib/action_controller/abstract/callbacks.rb @@ -13,7 +13,7 @@ module AbstractController super end end - + module ClassMethods def _normalize_callback_options(options) if only = options[:only] @@ -21,11 +21,11 @@ module AbstractController options[:per_key] = {:if => only} end if except = options[:except] - except = Array(except).map {|e| "action_name == :#{e}"}.join(" || ") + except = Array(except).map {|e| "action_name == :#{e}"}.join(" || ") options[:per_key] = {:unless => except} end end - + [:before, :after, :around].each do |filter| class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def #{filter}_filter(*names, &blk) @@ -40,4 +40,4 @@ module AbstractController end end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/exceptions.rb b/actionpack/lib/action_controller/abstract/exceptions.rb index ec4680629b..a7d07868bb 100644 --- a/actionpack/lib/action_controller/abstract/exceptions.rb +++ b/actionpack/lib/action_controller/abstract/exceptions.rb @@ -1,3 +1,3 @@ module AbstractController class ActionNotFound < StandardError ; end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/helpers.rb b/actionpack/lib/action_controller/abstract/helpers.rb index 62caa119e7..38e3ce8127 100644 --- a/actionpack/lib/action_controller/abstract/helpers.rb +++ b/actionpack/lib/action_controller/abstract/helpers.rb @@ -8,14 +8,14 @@ module AbstractController extlib_inheritable_accessor :master_helper_module self.master_helper_module = Module.new end - + # def self.included(klass) # klass.class_eval do # extlib_inheritable_accessor :master_helper_module # self.master_helper_module = Module.new # end # end - + def _action_view @_action_view ||= begin av = super @@ -23,19 +23,19 @@ module AbstractController av end end - + module ClassMethods def inherited(klass) klass.master_helper_module = Module.new klass.master_helper_module.__send__ :include, master_helper_module - + super end - + def add_template_helper(mod) master_helper_module.module_eval { include mod } end - + def helper_method(*meths) meths.flatten.each do |meth| master_helper_module.class_eval <<-ruby_eval, __FILE__, __LINE__ + 1 @@ -45,7 +45,7 @@ module AbstractController ruby_eval end end - + def helper(*args, &blk) args.flatten.each do |arg| case arg @@ -56,6 +56,5 @@ module AbstractController master_helper_module.module_eval(&blk) if block_given? end end - end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/layouts.rb b/actionpack/lib/action_controller/abstract/layouts.rb index 76130f8dc0..69fe4efc19 100644 --- a/actionpack/lib/action_controller/abstract/layouts.rb +++ b/actionpack/lib/action_controller/abstract/layouts.rb @@ -9,18 +9,18 @@ module AbstractController unless [String, Symbol, FalseClass, NilClass].include?(layout.class) raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" end - + @_layout = layout || false # Converts nil to false _write_layout_method end - + def _implied_layout_name name.underscore end - + # Takes the specified layout and creates a _layout method to be called # by _default_layout - # + # # If the specified layout is a: # String:: return the string # Symbol:: call the method specified by the symbol @@ -49,35 +49,34 @@ module AbstractController end end end - + def _render_template(template, options) _action_view._render_template_with_layout(template, options[:_layout]) end - + private - def _layout() end # This will be overwritten - + def _layout_for_name(name) unless [String, FalseClass, NilClass].include?(name.class) raise ArgumentError, "String, false, or nil expected; you passed #{name.inspect}" end - + name && view_paths.find_by_parts(name, {:formats => formats}, "layouts") end - + def _default_layout(require_layout = false) if require_layout && !_layout - raise ArgumentError, + raise ArgumentError, "There was no default layout for #{self.class} in #{view_paths.inspect}" end - + begin layout = _layout_for_name(_layout) rescue NameError => e - raise NoMethodError, + raise NoMethodError, "You specified #{@_layout.inspect} as the layout, but no such method was found" end end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/logger.rb b/actionpack/lib/action_controller/abstract/logger.rb index 5fb78f1755..c3bccd7778 100644 --- a/actionpack/lib/action_controller/abstract/logger.rb +++ b/actionpack/lib/action_controller/abstract/logger.rb @@ -6,4 +6,4 @@ module AbstractController cattr_accessor :logger end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index beb848f90e..b5c31a27ee 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -15,22 +15,22 @@ module AbstractController end def _action_view - @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self) + @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self) end - + def render(options = {}) self.response_body = render_to_body(options) end - + # Raw rendering of a template to a Rack-compatible body. # ==== # @option _prefix The template's path prefix # @option _layout The relative path to the layout template to use - # + # # :api: plugin def render_to_body(options = {}) name = options[:_template_name] || action_name - + template = options[:_template] || view_paths.find_by_parts(name.to_s, {:formats => formats}, options[:_prefix]) _render_template(template, options) end @@ -39,7 +39,7 @@ module AbstractController # ==== # @option _prefix The template's path prefix # @option _layout The relative path to the layout template to use - # + # # :api: plugin def render_to_string(options = {}) AbstractController::Renderer.body_to_s(render_to_body(options)) @@ -48,7 +48,7 @@ module AbstractController def _render_template(template, options) _action_view._render_template_with_layout(template) end - + def view_paths() _view_paths end # Return a string representation of a Rack-compatible response body. @@ -64,15 +64,14 @@ module AbstractController end module ClassMethods - def append_view_path(path) self.view_paths << path end - + def view_paths self._view_paths end - + def view_paths=(paths) self._view_paths = paths.is_a?(ActionView::PathSet) ? paths : ActionView::Base.process_view_paths(paths) diff --git a/actionpack/lib/action_controller/new_base.rb b/actionpack/lib/action_controller/new_base.rb index 7c65f1cdc1..29dc3aaddc 100644 --- a/actionpack/lib/action_controller/new_base.rb +++ b/actionpack/lib/action_controller/new_base.rb @@ -4,4 +4,4 @@ module ActionController autoload :Layouts, "action_controller/new_base/layouts" autoload :Renderer, "action_controller/new_base/renderer" autoload :UrlFor, "action_controller/new_base/url_for" -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/new_base/base.rb b/actionpack/lib/action_controller/new_base/base.rb index 08e7a1a0e7..2dd5390c99 100644 --- a/actionpack/lib/action_controller/new_base/base.rb +++ b/actionpack/lib/action_controller/new_base/base.rb @@ -1,6 +1,5 @@ module ActionController class AbstractBase < AbstractController::Base - # :api: public attr_internal :request, :response, :params @@ -12,46 +11,46 @@ module ActionController # :api: public def controller_name() self.class.controller_name end - # :api: public + # :api: public def self.controller_path @controller_path ||= self.name.sub(/Controller$/, '').underscore end - - # :api: public + + # :api: public def controller_path() self.class.controller_path end - - # :api: private + + # :api: private def self.action_methods @action_names ||= Set.new(self.public_instance_methods - self::CORE_METHODS) end - - # :api: private + + # :api: private def self.action_names() action_methods end - - # :api: private + + # :api: private def action_methods() self.class.action_names end # :api: private def action_names() action_methods end - + # :api: plugin def self.call(env) controller = new controller.call(env).to_rack end - + # :api: plugin def response_body=(body) @_response.body = body end - + # :api: private def call(env) @_request = ActionDispatch::Request.new(env) @_response = ActionDispatch::Response.new process(@_request.parameters[:action]) end - + # :api: private def to_rack response.to_a diff --git a/actionpack/lib/action_controller/new_base/hide_actions.rb b/actionpack/lib/action_controller/new_base/hide_actions.rb index aa420442fb..422ea180c4 100644 --- a/actionpack/lib/action_controller/new_base/hide_actions.rb +++ b/actionpack/lib/action_controller/new_base/hide_actions.rb @@ -6,21 +6,20 @@ module ActionController end def action_methods() self.class.action_names end - def action_names() action_methods end - + def action_names() action_methods end + private - def respond_to_action?(action_name) !hidden_actions.include?(action_name) && (super || respond_to?(:method_missing)) end - + module ClassMethods def hide_action(*args) args.each do |arg| self.hidden_actions << arg.to_s end end - + def action_methods @action_names ||= Set.new(super.reject {|name| self.hidden_actions.include?(name.to_s)}) end @@ -28,4 +27,4 @@ module ActionController def self.action_names() action_methods end end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/new_base/layouts.rb b/actionpack/lib/action_controller/new_base/layouts.rb index 89d24fe92d..228162421d 100644 --- a/actionpack/lib/action_controller/new_base/layouts.rb +++ b/actionpack/lib/action_controller/new_base/layouts.rb @@ -4,13 +4,13 @@ module ActionController depends_on ActionController::Renderer depends_on AbstractController::Layouts - + module ClassMethods def _implied_layout_name controller_path end end - + def render_to_body(options) # render :text => ..., :layout => ... # or @@ -18,22 +18,20 @@ module ActionController if !options.key?(:text) || options.key?(:layout) options[:_layout] = options.key?(:layout) ? _layout_for_option(options[:layout]) : _default_layout end - + super end - + private - def _layout_for_option(name) case name when String then _layout_for_name(name) when true then _default_layout(true) when false, nil then nil else - raise ArgumentError, - "String, true, or false, expected for `layout'; you passed #{name.inspect}" + raise ArgumentError, + "String, true, or false, expected for `layout'; you passed #{name.inspect}" end end - end end diff --git a/actionpack/lib/action_controller/new_base/renderer.rb b/actionpack/lib/action_controller/new_base/renderer.rb index be4ea54c3b..d7ea9ec4a5 100644 --- a/actionpack/lib/action_controller/new_base/renderer.rb +++ b/actionpack/lib/action_controller/new_base/renderer.rb @@ -3,22 +3,22 @@ module ActionController extend ActiveSupport::DependencyModule depends_on AbstractController::Renderer - + def initialize(*) self.formats = [:html] super end - + def render(action, options = {}) # TODO: Move this into #render_to_body if action.is_a?(Hash) - options, action = action, nil + options, action = action, nil else options.merge! :action => action end - + _process_options(options) - + self.response_body = render_to_body(options) end @@ -34,18 +34,17 @@ module ActionController options[:_template_name] = options[:template] elsif options.key?(:action) options[:_template_name] = options[:action].to_s - options[:_prefix] = _prefix + options[:_prefix] = _prefix end - + super(options) end - + private - def _prefix controller_path - end - + end + def _text(options) text = options[:text] @@ -54,7 +53,7 @@ module ActionController else text.to_s end end - + def _process_options(options) if status = options[:status] response.status = status.to_i diff --git a/actionpack/lib/action_controller/new_base/url_for.rb b/actionpack/lib/action_controller/new_base/url_for.rb index af5b21012b..3179c8cd5b 100644 --- a/actionpack/lib/action_controller/new_base/url_for.rb +++ b/actionpack/lib/action_controller/new_base/url_for.rb @@ -16,7 +16,7 @@ module ActionController # by this method. def default_url_options(options = nil) end - + def rewrite_options(options) #:nodoc: if defaults = default_url_options(options) defaults.merge(options) @@ -24,7 +24,7 @@ module ActionController options end end - + def url_for(options = {}) options ||= {} case options @@ -37,4 +37,4 @@ module ActionController end end end -end \ No newline at end of file +end -- cgit v1.2.3 From 8ee0c598dbe3f9fd133e85c69c6e43f62056646c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 May 2009 12:10:26 -0700 Subject: Tool for profiling resource usage in each require call. $ ruby -Iactiveresource/lib tools/profile_requires.rb active_resource 91.84 KB 3220 obj 4.0 ms active_resource 83.86 KB 3108 obj 3.3 ms active_support 69.32 KB 2682 obj 2.6 ms active_support/vendor 33.98 KB 651 obj 0.6 ms i18n 94.40 KB 315 obj 4.0 ms 44 KB RSS --- tools/profile_requires.rb | 75 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tools/profile_requires.rb diff --git a/tools/profile_requires.rb b/tools/profile_requires.rb new file mode 100644 index 0000000000..68d0de3d80 --- /dev/null +++ b/tools/profile_requires.rb @@ -0,0 +1,75 @@ +#!/usr/bin/env ruby +# Example: +# ruby -Iactivesupport/lib tools/profile_requires.rb active_support +# ruby -Iactionpack/lib tools/profile_requires.rb action_controller +abort 'Use REE so you can profile memory and object allocation' unless GC.respond_to?(:enable_stats) + +GC.enable_stats +require 'rubygems' +Gem.source_index +require 'benchmark' + +module TrackHeapGrowth + class << self + attr_accessor :indent + attr_accessor :stats + end + self.indent = 0 + self.stats = [] + + def track_growth(file) + TrackHeapGrowth.indent += 1 + heap_before, objects_before = GC.allocated_size, ObjectSpace.allocated_objects + result = nil + elapsed = Benchmark.realtime { result = yield } + heap_after, objects_after = GC.allocated_size, ObjectSpace.allocated_objects + TrackHeapGrowth.indent -= 1 + TrackHeapGrowth.stats << [file, TrackHeapGrowth.indent, elapsed, heap_after - heap_before, objects_after - objects_before] if result + result + end + + def require(file, *args) + track_growth(file) { super } + end + + def load(file, *args) + track_growth(file) { super } + end +end + +Object.instance_eval { include TrackHeapGrowth } + +GC.start +before = GC.allocated_size +before_rss = `ps -o rss= -p #{Process.pid}`.to_i +before_live_objects = ObjectSpace.live_objects + +path = ARGV.shift + +if mode = ARGV.shift + require 'ruby-prof' + RubyProf.measure_mode = RubyProf.const_get(mode.upcase) + RubyProf.start +end + +ENV['NO_RELOAD'] ||= '1' +ENV['RAILS_ENV'] ||= 'development' +elapsed = Benchmark.realtime { require path } +results = RubyProf.stop if mode + +GC.start +after_live_objects = ObjectSpace.live_objects +after_rss = `ps -o rss= -p #{Process.pid}`.to_i +after = GC.allocated_size +usage = (after - before) / 1024.0 + +if mode + File.open("profile_startup.#{mode}.tree", 'w') do |out| + RubyProf::CallTreePrinter.new(results).print(out) + end +end + +TrackHeapGrowth.stats.reverse_each do |file, indent, sec, bytes, objects| + puts "%10.2f KB %10d obj %8.1f ms %s%s" % [bytes / 1024.0, objects, sec * 1000, ' ' * indent, file] +end +puts "%10.2f KB %10d obj %8.1f ms %d KB RSS" % [usage, after_live_objects - before_live_objects, elapsed * 1000, after_rss - before_rss] -- cgit v1.2.3 From 49ed45213607ed6ce7e6c13c319bf55f916a0ac4 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 May 2009 18:27:50 -0700 Subject: Defer rake/contrib/sshpublisher require so basic tasks don't need the full rake gem --- activeresource/Rakefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/activeresource/Rakefile b/activeresource/Rakefile index bf7bbb0201..c3cde26b6c 100644 --- a/activeresource/Rakefile +++ b/activeresource/Rakefile @@ -4,7 +4,6 @@ require 'rake/testtask' require 'rake/rdoctask' require 'rake/packagetask' require 'rake/gempackagetask' -require 'rake/contrib/sshpublisher' require File.join(File.dirname(__FILE__), 'lib', 'active_resource', 'version') @@ -117,12 +116,14 @@ end desc "Publish the beta gem" task :pgem => [:package] do + require 'rake/contrib/sshpublisher' Rake::SshFilePublisher.new("gems.rubyonrails.org", "/u/sites/gems/gems", "pkg", "#{PKG_FILE_NAME}.gem").upload `ssh gems.rubyonrails.org '/u/sites/gems/gemupdate.sh'` end desc "Publish the API documentation" task :pdoc => [:rdoc] do + require 'rake/contrib/sshpublisher' Rake::SshDirPublisher.new("wrath.rubyonrails.org", "public_html/ar", "doc").upload end -- cgit v1.2.3 From 4817bf94d135c44ddfae1a30acb15de989e3c86c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 7 May 2009 18:39:03 -0700 Subject: Check for date/time methods that moved upstream in 1.9 --- activesupport/lib/active_support/core_ext/date/calculations.rb | 4 ++-- activesupport/lib/active_support/core_ext/date_time/conversions.rb | 6 +++--- activesupport/lib/active_support/core_ext/time/conversions.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/date/calculations.rb b/activesupport/lib/active_support/core_ext/date/calculations.rb index 04a32edefd..1fe4ffb8e1 100644 --- a/activesupport/lib/active_support/core_ext/date/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date/calculations.rb @@ -132,7 +132,7 @@ class Date # Short-hand for years_since(1) def next_year years_since(1) - end + end unless method_defined?(:next_year) # Short-hand for months_ago(1) def last_month @@ -142,7 +142,7 @@ class Date # Short-hand for months_since(1) def next_month months_since(1) - end + end unless method_defined?(:next_month) # Returns a new Date/DateTime representing the "start" of this week (i.e, Monday; DateTime objects will have time set to 0:00) def beginning_of_week diff --git a/activesupport/lib/active_support/core_ext/date_time/conversions.rb b/activesupport/lib/active_support/core_ext/date_time/conversions.rb index ddfa8d610d..5f01bc4fd6 100644 --- a/activesupport/lib/active_support/core_ext/date_time/conversions.rb +++ b/activesupport/lib/active_support/core_ext/date_time/conversions.rb @@ -58,7 +58,7 @@ class DateTime # Converts self to a Ruby Date object; time portion is discarded def to_date ::Date.new(year, month, day) - end + end unless method_defined?(:to_date) # Attempts to convert self to a Ruby Time object; returns self if out of range of Ruby Time class # If self has an offset other than 0, self will just be returned unaltered, since there's no clean way to map it to a Time @@ -69,12 +69,12 @@ class DateTime # To be able to keep Times, Dates and DateTimes interchangeable on conversions def to_datetime self - end + end unless method_defined?(:to_datetime) # Converts datetime to an appropriate format for use in XML def xmlschema strftime("%Y-%m-%dT%H:%M:%S%Z") - end if RUBY_VERSION < '1.9' + end unless method_defined?(:xmlschema) # Converts self to a floating-point number of seconds since the Unix epoch def to_f diff --git a/activesupport/lib/active_support/core_ext/time/conversions.rb b/activesupport/lib/active_support/core_ext/time/conversions.rb index 94e01597a9..6d9c080442 100644 --- a/activesupport/lib/active_support/core_ext/time/conversions.rb +++ b/activesupport/lib/active_support/core_ext/time/conversions.rb @@ -69,7 +69,7 @@ class Time # In this case, it simply returns +self+. def to_time self - end + end unless method_defined?(:to_time) # Converts a Time instance to a Ruby DateTime instance, preserving UTC offset. # -- cgit v1.2.3 From ef6f22ab2aadf619c993527150490d6d48a719c6 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Thu, 7 May 2009 01:03:52 +0100 Subject: honour inverse_of when preloading associations Signed-off-by: Michael Koziarski --- .../lib/active_record/association_preload.rb | 4 ++- .../associations/inverse_associations_test.rb | 33 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index e4ab69aa1b..967fff4d6f 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -126,6 +126,7 @@ module ActiveRecord association_proxy = parent_record.send(reflection_name) association_proxy.loaded association_proxy.target.push(*[associated_record].flatten) + association_proxy.__send__(:set_inverse_instance, associated_record, parent_record) end end @@ -152,7 +153,8 @@ module ActiveRecord seen_keys[associated_record[key].to_s] = true mapped_records = id_to_record_map[associated_record[key].to_s] mapped_records.each do |mapped_record| - mapped_record.send("set_#{reflection_name}_target", associated_record) + association_proxy = mapped_record.send("set_#{reflection_name}_target", associated_record) + association_proxy.__send__(:set_inverse_instance, associated_record, mapped_record) end end end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 616f8dfbbe..d123837efd 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -94,6 +94,17 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance" end + + def test_parent_instance_should_be_shared_with_eager_loaded_child_on_find + m = Man.find(:first, :include => :face) + f = m.face + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance" + end + def test_parent_instance_should_be_shared_with_newly_built_child m = Man.find(:first) f = m.build_face(:description => 'haunted') @@ -136,6 +147,18 @@ class InverseHasManyTests < ActiveRecord::TestCase end end + def test_parent_instance_should_be_shared_with_eager_loaded_children + m = Man.find(:first, :include => :interests) + is = m.interests + is.each do |i| + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance" + end + end + def test_parent_instance_should_be_shared_with_newly_built_child m = Man.find(:first) i = m.interests.build(:topic => 'Industrial Revolution Re-enactment') @@ -188,6 +211,16 @@ class InverseBelongsToTests < ActiveRecord::TestCase assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance" end + def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find + f = Face.find(:first, :include => :man) + m = f.man + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance" + end + def test_child_instance_should_be_shared_with_newly_built_parent f = Face.find(:first) m = f.build_man(:name => 'Charles') -- cgit v1.2.3 From 235775de291787bba6b7bc3c58e791216c3b5090 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Thu, 7 May 2009 01:43:15 +0100 Subject: honour :inverse_of for joins based include Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/associations.rb | 10 ++++++-- .../associations/inverse_associations_test.rb | 28 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 0952b087d1..2928b0bf83 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1916,21 +1916,27 @@ module ActiveRecord return nil if record.id.to_s != join.parent.record_id(row).to_s or row[join.aliased_primary_key].nil? association = join.instantiate(row) collection.target.push(association) + collection.__send__(:set_inverse_instance, association, record) when :has_one return if record.id.to_s != join.parent.record_id(row).to_s return if record.instance_variable_defined?("@#{join.reflection.name}") association = join.instantiate(row) unless row[join.aliased_primary_key].nil? - record.send("set_#{join.reflection.name}_target", association) + set_target_and_inverse(join, association, record) when :belongs_to return if record.id.to_s != join.parent.record_id(row).to_s or row[join.aliased_primary_key].nil? association = join.instantiate(row) - record.send("set_#{join.reflection.name}_target", association) + set_target_and_inverse(join, association, record) else raise ConfigurationError, "unknown macro: #{join.reflection.macro}" end return association end + def set_target_and_inverse(join, association, record) + association_proxy = record.send("set_#{join.reflection.name}_target", association) + association_proxy.__send__(:set_inverse_instance, association, record) + end + class JoinBase # :nodoc: attr_reader :active_record, :table_joins delegate :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :to => :active_record diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index d123837efd..47f83db112 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -103,6 +103,14 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" f.man.name = 'Mungo' assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance" + + m = Man.find(:first, :include => :face, :order => 'faces.id') + f = m.face + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance" end def test_parent_instance_should_be_shared_with_newly_built_child @@ -157,6 +165,17 @@ class InverseHasManyTests < ActiveRecord::TestCase i.man.name = 'Mungo' assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance" end + + m = Man.find(:first, :include => :interests, :order => 'interests.id') + is = m.interests + is.each do |i| + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance" + end + end def test_parent_instance_should_be_shared_with_newly_built_child @@ -219,6 +238,15 @@ class InverseBelongsToTests < ActiveRecord::TestCase assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" m.face.description = 'pleasing' assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance" + + + f = Face.find(:first, :include => :man, :order => 'men.id') + m = f.man + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance" end def test_child_instance_should_be_shared_with_newly_built_parent -- cgit v1.2.3 From 9e0cfdb7f951c0446cdfd3b2cc26443712fe657a Mon Sep 17 00:00:00 2001 From: Ken Collins Date: Sat, 9 May 2009 18:35:31 -0400 Subject: ActiveSupport::OrderedHash#to_a method returns an ordered set of arrays. Matches ruby1.9's Hash#to_a. Signed-off-by: Michael Koziarski [#2629 state:committed] --- activesupport/lib/active_support/ordered_hash.rb | 4 ++++ activesupport/test/ordered_hash_test.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index fed8094a24..33fd2a29b9 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -57,6 +57,10 @@ module ActiveSupport self end + def to_a + @keys.map { |key| [ key, self[key] ] } + end + def each_key @keys.each { |key| yield key } end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 7cd8c8a8f4..6fccbbdc41 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -51,6 +51,10 @@ class OrderedHashTest < Test::Unit::TestCase assert_same @ordered_hash, @ordered_hash.to_hash end + def test_to_a + assert_equal @keys.zip(@values), @ordered_hash.to_a + end + def test_has_key assert_equal true, @ordered_hash.has_key?('blue') assert_equal true, @ordered_hash.key?('blue') -- cgit v1.2.3 From 026b78f9076216990bddb1aa5d83d23a647c02a5 Mon Sep 17 00:00:00 2001 From: Anthony Crumley Date: Mon, 4 May 2009 09:49:43 -0500 Subject: Fixed eager load error on find with include => [:table_name] and hash conditions like {:table_name => {:column => 'value'}} Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/associations.rb | 20 ++++++++++++++++---- activerecord/test/cases/associations/eager_test.rb | 12 ++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2928b0bf83..781a0290e8 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1671,17 +1671,29 @@ module ActiveRecord string.scan(/([\.a-zA-Z_]+).?\./).flatten end + def tables_in_hash(hash) + return [] if hash.blank? + tables = hash.map do |key, value| + if value.is_a?(Hash) + key.to_s + else + tables_in_string(key) if key.is_a?(String) + end + end + tables.flatten.compact + end + def conditions_tables(options) # look in both sets of conditions conditions = [scope(:find, :conditions), options[:conditions]].inject([]) do |all, cond| case cond when nil then all - when Array then all << cond.first - when Hash then all << cond.keys - else all << cond + when Array then all << tables_in_string(cond.first) + when Hash then all << tables_in_hash(cond) + else all << tables_in_string(cond) end end - tables_in_string(conditions.join(' ')) + conditions.flatten end def order_tables(options) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 40723814c5..d23f86b700 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -223,6 +223,18 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_eager_association_loading_with_belongs_to_and_conditions_hash + comments = [] + assert_nothing_raised do + comments = Comment.find(:all, :include => :post, :conditions => {:posts => {:id => 4}}, :limit => 3, :order => 'comments.id') + end + assert_equal 3, comments.length + assert_equal [5,6,7], comments.collect { |c| c.id } + assert_no_queries do + comments.first.post + end + end + def test_eager_association_loading_with_belongs_to_and_conditions_string_with_quoted_table_name quoted_posts_id= Comment.connection.quote_table_name('posts') + '.' + Comment.connection.quote_column_name('id') assert_nothing_raised do -- cgit v1.2.3 From 9010ed27559ed5ab89ea71b4b16f4c8e56d03dbb Mon Sep 17 00:00:00 2001 From: Mike Breen Date: Sun, 10 May 2009 15:02:00 +1200 Subject: Allow you to pass :all_blank to :reject_if option to automatically create a Proc that will reject any record with blank attributes. --- activerecord/lib/active_record/nested_attributes.rb | 11 ++++++++++- activerecord/test/cases/nested_attributes_test.rb | 18 +++++++++++++++++- activerecord/test/models/pirate.rb | 2 ++ activerecord/test/schema/schema.rb | 1 + 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index e3122d195a..dfad2901c5 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -180,10 +180,14 @@ module ActiveRecord # and the Proc should return either +true+ or +false+. When no Proc # is specified a record will be built for all attribute hashes that # do not have a _delete that evaluates to true. + # Passing :all_blank instead of a Proc will create a proc + # that will reject a record where all the attributes are blank. # # Examples: # # creates avatar_attributes= # accepts_nested_attributes_for :avatar, :reject_if => proc { |attributes| attributes['name'].blank? } + # # creates avatar_attributes= + # accepts_nested_attributes_for :avatar, :reject_if => :all_blank # # creates avatar_attributes= and posts_attributes= # accepts_nested_attributes_for :avatar, :posts, :allow_destroy => true def accepts_nested_attributes_for(*attr_names) @@ -201,7 +205,12 @@ module ActiveRecord end reflection.options[:autosave] = true - self.reject_new_nested_attributes_procs[association_name.to_sym] = options[:reject_if] + + self.reject_new_nested_attributes_procs[association_name.to_sym] = if options[:reject_if] == :all_blank + proc { |attributes| attributes.all? {|k,v| v.blank?} } + else + options[:reject_if] + end # def pirate_attributes=(attributes) # assign_nested_attributes_for_one_to_one_association(:pirate, attributes, false) diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index cd6277c24b..f1741ed54d 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -31,11 +31,27 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase end def test_should_add_a_proc_to_reject_new_nested_attributes_procs - [:parrots, :birds].each do |name| + [:parrots, :birds, :birds_with_reject_all_blank].each do |name| assert_instance_of Proc, Pirate.reject_new_nested_attributes_procs[name] end end + def test_should_not_build_a_new_record_if_reject_all_blank_returns_false + pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") + pirate.birds_with_reject_all_blank_attributes = [{:name => '', :color => ''}] + pirate.save! + + assert pirate.birds_with_reject_all_blank.empty? + end + + def test_should_build_a_new_record_if_reject_all_blank_does_not_return_false + pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") + pirate.birds_with_reject_all_blank_attributes = [{:name => 'Tweetie', :color => ''}] + pirate.save! + + assert_equal 1, pirate.birds_with_reject_all_blank.count + end + def test_should_raise_an_ArgumentError_for_non_existing_associations assert_raise_with_message ArgumentError, "No association found for name `honesty'. Has it been defined yet?" do Pirate.accepts_nested_attributes_for :honesty diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 238917bf30..acf53fce8b 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -28,11 +28,13 @@ class Pirate < ActiveRecord::Base :after_add => proc {|p,b| p.ship_log << "after_adding_proc_bird_#{b.id || ''}"}, :before_remove => proc {|p,b| p.ship_log << "before_removing_proc_bird_#{b.id}"}, :after_remove => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"} + has_many :birds_with_reject_all_blank, :class_name => "Bird" accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks, :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true + accepts_nested_attributes_for :birds_with_reject_all_blank, :reject_if => :all_blank validates_presence_of :catchphrase diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 3b0e17c867..6fb918c60e 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -57,6 +57,7 @@ ActiveRecord::Schema.define do create_table :birds, :force => true do |t| t.string :name + t.string :color t.integer :pirate_id end -- cgit v1.2.3 From 2e7409f035236c719c5b1567c4bb3e65a5e543bc Mon Sep 17 00:00:00 2001 From: Alexander Dymo Date: Thu, 7 May 2009 03:19:17 +0300 Subject: Change spelling of Kyev timezone to Kyiv [#2613 state:resolved] --- activesupport/CHANGELOG | 2 ++ activesupport/lib/active_support/values/time_zone.rb | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index ca5ab13a46..032f0fb9c1 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Change spelling of Kyev timezone to Kyiv #2613 [Alexander Dymo] + * Add ActiveSupport.parse_json_times to disable time parsing in JSON backends that don't support it or don't need it. [rick] * Add pluggable JSON backends with support for the JSON gem. [rick] diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index e2d759aa50..b37dae1c2a 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -92,7 +92,7 @@ module ActiveSupport "Bucharest" => "Europe/Bucharest", "Cairo" => "Africa/Cairo", "Helsinki" => "Europe/Helsinki", - "Kyev" => "Europe/Kiev", + "Kyiv" => "Europe/Kiev", "Riga" => "Europe/Riga", "Sofia" => "Europe/Sofia", "Tallinn" => "Europe/Tallinn", @@ -336,7 +336,7 @@ module ActiveSupport "Copenhagen", "Madrid", "Paris", "Amsterdam", "Berlin", "Bern", "Rome", "Stockholm", "Vienna", "West Central Africa" ], - [ 7_200, "Bucharest", "Cairo", "Helsinki", "Kyev", "Riga", "Sofia", + [ 7_200, "Bucharest", "Cairo", "Helsinki", "Kyiv", "Riga", "Sofia", "Tallinn", "Vilnius", "Athens", "Istanbul", "Minsk", "Jerusalem", "Harare", "Pretoria" ], [ 10_800, "Moscow", "St. Petersburg", "Volgograd", "Kuwait", "Riyadh", -- cgit v1.2.3 From 4932f7b38f72104819022abca0c952ba6f9888cb Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 11 May 2009 19:08:13 +0200 Subject: Added db/seeds.rb as a default file for storing seed data for the database. Can be loaded with rake db:seed (or created alongside the db with db:setup). (This is also known as the "Stop Putting Gawd Damn Seed Data In Your Migrations" feature) [DHH] --- railties/CHANGELOG | 5 +++++ railties/Rakefile | 3 +++ railties/configs/seeds.rb | 7 +++++++ .../generators/applications/app/app_generator.rb | 5 +++++ railties/lib/tasks/databases.rake | 13 +++++++++++-- 5 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 railties/configs/seeds.rb diff --git a/railties/CHANGELOG b/railties/CHANGELOG index 98e3a861e8..8e7dfb38cc 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,3 +1,8 @@ +*Edge* + +* Added db/seeds.rb as a default file for storing seed data for the database. Can be loaded with rake db:seed (or created alongside the db with db:setup). (This is also known as the "Stop Putting Gawd Damn Seed Data In Your Migrations" feature) [DHH] + + *2.3.2 [Final] (March 15, 2009)* * Remove outdated script/plugin options [Pratik Naik] diff --git a/railties/Rakefile b/railties/Rakefile index 9cd102df0f..4247742664 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -200,11 +200,14 @@ task :copy_configs do cp "configs/locales/en.yml", "#{PKG_DESTINATION}/config/locales/en.yml" + cp "configs/seeds.rb", "#{PKG_DESTINATION}/db/seeds.rb" + cp "environments/boot.rb", "#{PKG_DESTINATION}/config/boot.rb" cp "environments/environment.rb", "#{PKG_DESTINATION}/config/environment.rb" cp "environments/production.rb", "#{PKG_DESTINATION}/config/environments/production.rb" cp "environments/development.rb", "#{PKG_DESTINATION}/config/environments/development.rb" cp "environments/test.rb", "#{PKG_DESTINATION}/config/environments/test.rb" + end task :copy_binfiles do diff --git a/railties/configs/seeds.rb b/railties/configs/seeds.rb new file mode 100644 index 0000000000..3174d0cb8a --- /dev/null +++ b/railties/configs/seeds.rb @@ -0,0 +1,7 @@ +# This file should contain all the record creation needed to seed the database with its default values. +# The data can then be loaded with the rake db:seed (or created alongside the db with db:setup). +# +# Examples: +# +# cities = City.create([{ :name => 'Chicago' }, { :name => 'Copenhagen' }]) +# Major.create(:name => 'Daley', :city => cities.first) diff --git a/railties/lib/rails_generator/generators/applications/app/app_generator.rb b/railties/lib/rails_generator/generators/applications/app/app_generator.rb index 2c31d89538..c8c2239f34 100644 --- a/railties/lib/rails_generator/generators/applications/app/app_generator.rb +++ b/railties/lib/rails_generator/generators/applications/app/app_generator.rb @@ -125,6 +125,7 @@ class AppGenerator < Rails::Generator::Base create_database_configuration_file(m) create_routes_file(m) create_locale_file(m) + create_seeds_file(m) create_initializer_files(m) create_environment_files(m) end @@ -176,6 +177,10 @@ class AppGenerator < Rails::Generator::Base m.file "configs/routes.rb", "config/routes.rb" end + def create_seeds_file(m) + m.file "configs/seeds.rb", "db/seeds.rb" + end + def create_initializer_files(m) %w( backtrace_silencers diff --git a/railties/lib/tasks/databases.rake b/railties/lib/tasks/databases.rake index 9588fabb2d..cdab5d8bb0 100644 --- a/railties/lib/tasks/databases.rake +++ b/railties/lib/tasks/databases.rake @@ -156,8 +156,8 @@ namespace :db do Rake::Task["db:schema:dump"].invoke if ActiveRecord::Base.schema_format == :ruby end - desc 'Drops and recreates the database from db/schema.rb for the current environment.' - task :reset => ['db:drop', 'db:create', 'db:schema:load'] + desc 'Drops and recreates the database from db/schema.rb for the current environment and loads the seeds.' + task :reset => [ 'db:drop', 'db:setup' ] desc "Retrieves the charset for the current environment's database" task :charset => :environment do @@ -206,6 +206,15 @@ namespace :db do end end + desc 'Create the database, load the schema, and initialize with the seed data' + task :setup => [ 'db:create', 'db:schema:load', 'db:seed' ] + + desc 'Load the seed data from db/seeds.rb' + task :seed => :environment do + seed_file = File.join(Rails.root, 'db', 'seeds.rb') + load(seed_file) if File.exist?(seed_file) + end + namespace :fixtures do desc "Load fixtures into the current environment's database. Load specific fixtures using FIXTURES=x,y. Load from subdirectory in test/fixtures using FIXTURES_DIR=z. Specify an alternative path (eg. spec/fixtures) using FIXTURES_PATH=spec/fixtures." task :load => :environment do -- cgit v1.2.3 From e1854e0b199fba352ddcaa58a3af168e8cc70e3a Mon Sep 17 00:00:00 2001 From: Douglas F Shearer Date: Thu, 7 May 2009 23:58:07 +0100 Subject: ActiveSupport::OrderedHash[1,2,3,4] creates an OrderedHash instead of a Hash. [#2615 state:committed] Signed-off-by: Jeremy Kemper --- activesupport/lib/active_support/ordered_hash.rb | 10 ++++++++++ activesupport/test/ordered_hash_test.rb | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 33fd2a29b9..8d1c0f5160 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -10,6 +10,16 @@ module ActiveSupport @keys = [] end + def self.[](*args) + ordered_hash = new + args.each_with_index { |val,ind| + # Only every second value is a key. + next if ind % 2 != 0 + ordered_hash[val] = args[ind + 1] + } + ordered_hash + end + def initialize_copy(other) super # make a deep copy of keys diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 6fccbbdc41..647938dd87 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -162,4 +162,10 @@ class OrderedHashTest < Test::Unit::TestCase def test_inspect assert @ordered_hash.inspect.include?(@hash.inspect) end + + def test_alternate_initialization + alternate = ActiveSupport::OrderedHash[1,2,3,4] + assert_kind_of ActiveSupport::OrderedHash, alternate + assert_equal [1, 3], alternate.keys + end end -- cgit v1.2.3 From ddbeb15a5e7e0c3c5f316ccf65b557bc5311a6c4 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 11 May 2009 12:01:08 -0700 Subject: Revert "Fixed bug with polymorphic has_one :as pointing to an STI record" [#2594 state:open] This reverts commit 99c103be1165da9c8299bc0977188ecf167e06a5. --- .../lib/active_record/associations/has_one_association.rb | 2 +- .../test/cases/associations/has_one_associations_test.rb | 9 +-------- activerecord/test/fixtures/organizations.yml | 4 +--- activerecord/test/fixtures/sponsors.yml | 4 +--- activerecord/test/models/organization.rb | 4 ---- activerecord/test/schema/schema.rb | 3 +-- 6 files changed, 5 insertions(+), 21 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 4908005d2e..b72b84343b 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -90,7 +90,7 @@ module ActiveRecord when @reflection.options[:as] @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " + - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.name.to_s)}" + "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}" else @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}" end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 3984945f9f..1ddb3f49bf 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -2,11 +2,9 @@ require "cases/helper" require 'models/developer' require 'models/project' require 'models/company' -require 'models/sponsor' -require 'models/organization' class HasOneAssociationsTest < ActiveRecord::TestCase - fixtures :accounts, :companies, :developers, :projects, :developers_projects, :organizations, :sponsors + fixtures :accounts, :companies, :developers, :projects, :developers_projects def setup Account.destroyed_account_ids.clear @@ -308,9 +306,4 @@ class HasOneAssociationsTest < ActiveRecord::TestCase Firm.find(@firm.id, :include => :account).save! end end - - def test_polymorphic_sti - assert_equal organizations(:sponsorable), sponsors(:org_sponsor).sponsorable - assert_equal sponsors(:org_sponsor), organizations(:sponsorable).sponsor - end end diff --git a/activerecord/test/fixtures/organizations.yml b/activerecord/test/fixtures/organizations.yml index 05d620fbc6..25295bff87 100644 --- a/activerecord/test/fixtures/organizations.yml +++ b/activerecord/test/fixtures/organizations.yml @@ -2,6 +2,4 @@ nsa: name: No Such Agency discordians: name: Discordians -sponsorable: - name: We Need Money - type: SponsorableOrganization + diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml index 97a7784047..42df8957d1 100644 --- a/activerecord/test/fixtures/sponsors.yml +++ b/activerecord/test/fixtures/sponsors.yml @@ -6,6 +6,4 @@ boring_club_sponsor_for_groucho: sponsorable: some_other_guy (Member) crazy_club_sponsor_for_groucho: sponsor_club: crazy_club - sponsorable: some_other_guy (Member) -org_sponsor: - sponsorable: sponsorable (SponsorableOrganization) \ No newline at end of file + sponsorable: some_other_guy (Member) \ No newline at end of file diff --git a/activerecord/test/models/organization.rb b/activerecord/test/models/organization.rb index 5d1308354d..d79d5037c8 100644 --- a/activerecord/test/models/organization.rb +++ b/activerecord/test/models/organization.rb @@ -1,8 +1,4 @@ class Organization < ActiveRecord::Base has_many :member_details has_many :members, :through => :member_details -end - -class SponsorableOrganization < Organization - has_one :sponsor, :as => :sponsorable end \ No newline at end of file diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 6fb918c60e..a776cd974b 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -284,7 +284,6 @@ ActiveRecord::Schema.define do create_table :organizations, :force => true do |t| t.string :name - t.string :type end create_table :owners, :primary_key => :owner_id ,:force => true do |t| @@ -390,7 +389,7 @@ ActiveRecord::Schema.define do create_table :sponsors, :force => true do |t| t.integer :club_id t.integer :sponsorable_id - t.string :sponsorable_type + t.string :sponsorable_type end create_table :subscribers, :force => true, :id => false do |t| -- cgit v1.2.3 From 0cac68d3bed3e6bf8ec2eb994858e4a179046941 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Mon, 11 May 2009 15:03:24 -0700 Subject: Revert "Whitespace!" This reverts commit a747ab5b20b9d543e9d311070e3b720c761ae716. --- actionpack/lib/action_controller/abstract.rb | 2 +- actionpack/lib/action_controller/abstract/base.rb | 19 ++++++++------- .../lib/action_controller/abstract/callbacks.rb | 8 +++---- .../lib/action_controller/abstract/exceptions.rb | 2 +- .../lib/action_controller/abstract/helpers.rb | 17 +++++++------- .../lib/action_controller/abstract/layouts.rb | 27 +++++++++++----------- .../lib/action_controller/abstract/logger.rb | 2 +- .../lib/action_controller/abstract/renderer.rb | 19 +++++++-------- actionpack/lib/action_controller/new_base.rb | 2 +- actionpack/lib/action_controller/new_base/base.rb | 27 +++++++++++----------- .../lib/action_controller/new_base/hide_actions.rb | 11 +++++---- .../lib/action_controller/new_base/layouts.rb | 14 ++++++----- .../lib/action_controller/new_base/renderer.rb | 23 +++++++++--------- .../lib/action_controller/new_base/url_for.rb | 6 ++--- 14 files changed, 95 insertions(+), 84 deletions(-) diff --git a/actionpack/lib/action_controller/abstract.rb b/actionpack/lib/action_controller/abstract.rb index 9442d4559f..3f5c4a185f 100644 --- a/actionpack/lib/action_controller/abstract.rb +++ b/actionpack/lib/action_controller/abstract.rb @@ -7,4 +7,4 @@ module AbstractController autoload :Renderer, "action_controller/abstract/renderer" # === Exceptions autoload :ActionNotFound, "action_controller/abstract/exceptions" -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/base.rb b/actionpack/lib/action_controller/abstract/base.rb index 39e9ad0440..ade7719cc0 100644 --- a/actionpack/lib/action_controller/abstract/base.rb +++ b/actionpack/lib/action_controller/abstract/base.rb @@ -1,38 +1,41 @@ module AbstractController class Base + attr_internal :response_body attr_internal :response_obj attr_internal :action_name - + def self.process(action) new.process(action) end - + def self.inherited(klass) end - + def initialize self.response_obj = {} end - + def process(action_name) unless respond_to_action?(action_name) raise ActionNotFound, "The action '#{action_name}' could not be found" end - + @_action_name = action_name process_action self.response_obj[:body] = self.response_body self end - + private + def process_action respond_to?(action_name) ? send(action_name) : send(:action_missing, action_name) end - + def respond_to_action?(action_name) respond_to?(action_name) || respond_to?(:action_missing, true) end + end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/callbacks.rb b/actionpack/lib/action_controller/abstract/callbacks.rb index cb8aade0be..d7faaf4236 100644 --- a/actionpack/lib/action_controller/abstract/callbacks.rb +++ b/actionpack/lib/action_controller/abstract/callbacks.rb @@ -13,7 +13,7 @@ module AbstractController super end end - + module ClassMethods def _normalize_callback_options(options) if only = options[:only] @@ -21,11 +21,11 @@ module AbstractController options[:per_key] = {:if => only} end if except = options[:except] - except = Array(except).map {|e| "action_name == :#{e}"}.join(" || ") + except = Array(except).map {|e| "action_name == :#{e}"}.join(" || ") options[:per_key] = {:unless => except} end end - + [:before, :after, :around].each do |filter| class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def #{filter}_filter(*names, &blk) @@ -40,4 +40,4 @@ module AbstractController end end end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/exceptions.rb b/actionpack/lib/action_controller/abstract/exceptions.rb index a7d07868bb..ec4680629b 100644 --- a/actionpack/lib/action_controller/abstract/exceptions.rb +++ b/actionpack/lib/action_controller/abstract/exceptions.rb @@ -1,3 +1,3 @@ module AbstractController class ActionNotFound < StandardError ; end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/helpers.rb b/actionpack/lib/action_controller/abstract/helpers.rb index 38e3ce8127..62caa119e7 100644 --- a/actionpack/lib/action_controller/abstract/helpers.rb +++ b/actionpack/lib/action_controller/abstract/helpers.rb @@ -8,14 +8,14 @@ module AbstractController extlib_inheritable_accessor :master_helper_module self.master_helper_module = Module.new end - + # def self.included(klass) # klass.class_eval do # extlib_inheritable_accessor :master_helper_module # self.master_helper_module = Module.new # end # end - + def _action_view @_action_view ||= begin av = super @@ -23,19 +23,19 @@ module AbstractController av end end - + module ClassMethods def inherited(klass) klass.master_helper_module = Module.new klass.master_helper_module.__send__ :include, master_helper_module - + super end - + def add_template_helper(mod) master_helper_module.module_eval { include mod } end - + def helper_method(*meths) meths.flatten.each do |meth| master_helper_module.class_eval <<-ruby_eval, __FILE__, __LINE__ + 1 @@ -45,7 +45,7 @@ module AbstractController ruby_eval end end - + def helper(*args, &blk) args.flatten.each do |arg| case arg @@ -56,5 +56,6 @@ module AbstractController master_helper_module.module_eval(&blk) if block_given? end end + end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/layouts.rb b/actionpack/lib/action_controller/abstract/layouts.rb index 69fe4efc19..76130f8dc0 100644 --- a/actionpack/lib/action_controller/abstract/layouts.rb +++ b/actionpack/lib/action_controller/abstract/layouts.rb @@ -9,18 +9,18 @@ module AbstractController unless [String, Symbol, FalseClass, NilClass].include?(layout.class) raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" end - + @_layout = layout || false # Converts nil to false _write_layout_method end - + def _implied_layout_name name.underscore end - + # Takes the specified layout and creates a _layout method to be called # by _default_layout - # + # # If the specified layout is a: # String:: return the string # Symbol:: call the method specified by the symbol @@ -49,34 +49,35 @@ module AbstractController end end end - + def _render_template(template, options) _action_view._render_template_with_layout(template, options[:_layout]) end - + private + def _layout() end # This will be overwritten - + def _layout_for_name(name) unless [String, FalseClass, NilClass].include?(name.class) raise ArgumentError, "String, false, or nil expected; you passed #{name.inspect}" end - + name && view_paths.find_by_parts(name, {:formats => formats}, "layouts") end - + def _default_layout(require_layout = false) if require_layout && !_layout - raise ArgumentError, + raise ArgumentError, "There was no default layout for #{self.class} in #{view_paths.inspect}" end - + begin layout = _layout_for_name(_layout) rescue NameError => e - raise NoMethodError, + raise NoMethodError, "You specified #{@_layout.inspect} as the layout, but no such method was found" end end end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/logger.rb b/actionpack/lib/action_controller/abstract/logger.rb index c3bccd7778..5fb78f1755 100644 --- a/actionpack/lib/action_controller/abstract/logger.rb +++ b/actionpack/lib/action_controller/abstract/logger.rb @@ -6,4 +6,4 @@ module AbstractController cattr_accessor :logger end end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index b5c31a27ee..beb848f90e 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -15,22 +15,22 @@ module AbstractController end def _action_view - @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self) + @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self) end - + def render(options = {}) self.response_body = render_to_body(options) end - + # Raw rendering of a template to a Rack-compatible body. # ==== # @option _prefix The template's path prefix # @option _layout The relative path to the layout template to use - # + # # :api: plugin def render_to_body(options = {}) name = options[:_template_name] || action_name - + template = options[:_template] || view_paths.find_by_parts(name.to_s, {:formats => formats}, options[:_prefix]) _render_template(template, options) end @@ -39,7 +39,7 @@ module AbstractController # ==== # @option _prefix The template's path prefix # @option _layout The relative path to the layout template to use - # + # # :api: plugin def render_to_string(options = {}) AbstractController::Renderer.body_to_s(render_to_body(options)) @@ -48,7 +48,7 @@ module AbstractController def _render_template(template, options) _action_view._render_template_with_layout(template) end - + def view_paths() _view_paths end # Return a string representation of a Rack-compatible response body. @@ -64,14 +64,15 @@ module AbstractController end module ClassMethods + def append_view_path(path) self.view_paths << path end - + def view_paths self._view_paths end - + def view_paths=(paths) self._view_paths = paths.is_a?(ActionView::PathSet) ? paths : ActionView::Base.process_view_paths(paths) diff --git a/actionpack/lib/action_controller/new_base.rb b/actionpack/lib/action_controller/new_base.rb index 29dc3aaddc..7c65f1cdc1 100644 --- a/actionpack/lib/action_controller/new_base.rb +++ b/actionpack/lib/action_controller/new_base.rb @@ -4,4 +4,4 @@ module ActionController autoload :Layouts, "action_controller/new_base/layouts" autoload :Renderer, "action_controller/new_base/renderer" autoload :UrlFor, "action_controller/new_base/url_for" -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/new_base/base.rb b/actionpack/lib/action_controller/new_base/base.rb index 2dd5390c99..08e7a1a0e7 100644 --- a/actionpack/lib/action_controller/new_base/base.rb +++ b/actionpack/lib/action_controller/new_base/base.rb @@ -1,5 +1,6 @@ module ActionController class AbstractBase < AbstractController::Base + # :api: public attr_internal :request, :response, :params @@ -11,46 +12,46 @@ module ActionController # :api: public def controller_name() self.class.controller_name end - # :api: public + # :api: public def self.controller_path @controller_path ||= self.name.sub(/Controller$/, '').underscore end - - # :api: public + + # :api: public def controller_path() self.class.controller_path end - - # :api: private + + # :api: private def self.action_methods @action_names ||= Set.new(self.public_instance_methods - self::CORE_METHODS) end - - # :api: private + + # :api: private def self.action_names() action_methods end - - # :api: private + + # :api: private def action_methods() self.class.action_names end # :api: private def action_names() action_methods end - + # :api: plugin def self.call(env) controller = new controller.call(env).to_rack end - + # :api: plugin def response_body=(body) @_response.body = body end - + # :api: private def call(env) @_request = ActionDispatch::Request.new(env) @_response = ActionDispatch::Response.new process(@_request.parameters[:action]) end - + # :api: private def to_rack response.to_a diff --git a/actionpack/lib/action_controller/new_base/hide_actions.rb b/actionpack/lib/action_controller/new_base/hide_actions.rb index 422ea180c4..aa420442fb 100644 --- a/actionpack/lib/action_controller/new_base/hide_actions.rb +++ b/actionpack/lib/action_controller/new_base/hide_actions.rb @@ -6,20 +6,21 @@ module ActionController end def action_methods() self.class.action_names end - def action_names() action_methods end - + def action_names() action_methods end + private + def respond_to_action?(action_name) !hidden_actions.include?(action_name) && (super || respond_to?(:method_missing)) end - + module ClassMethods def hide_action(*args) args.each do |arg| self.hidden_actions << arg.to_s end end - + def action_methods @action_names ||= Set.new(super.reject {|name| self.hidden_actions.include?(name.to_s)}) end @@ -27,4 +28,4 @@ module ActionController def self.action_names() action_methods end end end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/new_base/layouts.rb b/actionpack/lib/action_controller/new_base/layouts.rb index 228162421d..89d24fe92d 100644 --- a/actionpack/lib/action_controller/new_base/layouts.rb +++ b/actionpack/lib/action_controller/new_base/layouts.rb @@ -4,13 +4,13 @@ module ActionController depends_on ActionController::Renderer depends_on AbstractController::Layouts - + module ClassMethods def _implied_layout_name controller_path end end - + def render_to_body(options) # render :text => ..., :layout => ... # or @@ -18,20 +18,22 @@ module ActionController if !options.key?(:text) || options.key?(:layout) options[:_layout] = options.key?(:layout) ? _layout_for_option(options[:layout]) : _default_layout end - + super end - + private + def _layout_for_option(name) case name when String then _layout_for_name(name) when true then _default_layout(true) when false, nil then nil else - raise ArgumentError, - "String, true, or false, expected for `layout'; you passed #{name.inspect}" + raise ArgumentError, + "String, true, or false, expected for `layout'; you passed #{name.inspect}" end end + end end diff --git a/actionpack/lib/action_controller/new_base/renderer.rb b/actionpack/lib/action_controller/new_base/renderer.rb index d7ea9ec4a5..be4ea54c3b 100644 --- a/actionpack/lib/action_controller/new_base/renderer.rb +++ b/actionpack/lib/action_controller/new_base/renderer.rb @@ -3,22 +3,22 @@ module ActionController extend ActiveSupport::DependencyModule depends_on AbstractController::Renderer - + def initialize(*) self.formats = [:html] super end - + def render(action, options = {}) # TODO: Move this into #render_to_body if action.is_a?(Hash) - options, action = action, nil + options, action = action, nil else options.merge! :action => action end - + _process_options(options) - + self.response_body = render_to_body(options) end @@ -34,17 +34,18 @@ module ActionController options[:_template_name] = options[:template] elsif options.key?(:action) options[:_template_name] = options[:action].to_s - options[:_prefix] = _prefix + options[:_prefix] = _prefix end - + super(options) end - + private + def _prefix controller_path - end - + end + def _text(options) text = options[:text] @@ -53,7 +54,7 @@ module ActionController else text.to_s end end - + def _process_options(options) if status = options[:status] response.status = status.to_i diff --git a/actionpack/lib/action_controller/new_base/url_for.rb b/actionpack/lib/action_controller/new_base/url_for.rb index 3179c8cd5b..af5b21012b 100644 --- a/actionpack/lib/action_controller/new_base/url_for.rb +++ b/actionpack/lib/action_controller/new_base/url_for.rb @@ -16,7 +16,7 @@ module ActionController # by this method. def default_url_options(options = nil) end - + def rewrite_options(options) #:nodoc: if defaults = default_url_options(options) defaults.merge(options) @@ -24,7 +24,7 @@ module ActionController options end end - + def url_for(options = {}) options ||= {} case options @@ -37,4 +37,4 @@ module ActionController end end end -end +end \ No newline at end of file -- cgit v1.2.3