diff options
Diffstat (limited to 'actionpack')
12 files changed, 162 insertions, 50 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index ee9d30e1fb..fc3410ba6e 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,9 @@ *Rails 3.1.0 (unreleased)* +* ActionDispatch::MiddlewareStack now uses composition over inheritance. It is +no longer an array which means there may be methods missing that were not +tested. + * Add an :authenticity_token option to form_tag for custom handling or to omit the token (pass :authenticity_token => false). [Jakub Kuźma, Igor Wiedler] * HTML5 button_tag helper. [Rizwan Reza] diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index a4bac3caed..a1c582560c 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -103,12 +103,14 @@ module ActionController #:nodoc: end def _save_fragment(name, options) - return unless caching_allowed? - content = response_body content = content.join if content.is_a?(Array) - write_fragment(name, content, options) + if caching_allowed? + write_fragment(name, content, options) + else + content + end end protected diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index b2c8053584..e5db31061b 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -36,7 +36,7 @@ module ActionController action = action.to_s raise "MiddlewareStack#build requires an app" unless app - reverse.inject(app) do |a, middleware| + middlewares.reverse.inject(app) do |a, middleware| middleware.valid?(action) ? middleware.build(a) : a end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index e3cd779756..a4308f528c 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -2,17 +2,26 @@ require "active_support/inflector/methods" require "active_support/dependencies" module ActionDispatch - class MiddlewareStack < Array + class MiddlewareStack class Middleware - attr_reader :args, :block + attr_reader :args, :block, :name, :classcache def initialize(klass_or_name, *args, &block) - @ref = ActiveSupport::Dependencies::Reference.new(klass_or_name) + @klass = nil + + if klass_or_name.respond_to?(:name) + @klass = klass_or_name + @name = @klass.name + else + @name = klass_or_name.to_s + end + + @classcache = ActiveSupport::Dependencies::Reference @args, @block = args, block end def klass - @ref.get + @klass || classcache[@name] end def ==(middleware) @@ -22,7 +31,7 @@ module ActionDispatch when Class klass == middleware else - normalize(@ref.name) == normalize(middleware) + normalize(@name) == normalize(middleware) end end @@ -41,18 +50,39 @@ module ActionDispatch end end - # Use this instead of super to work around a warning. - alias :array_initialize :initialize + include Enumerable + + attr_accessor :middlewares def initialize(*args) - array_initialize(*args) + @middlewares = [] yield(self) if block_given? end + def each + @middlewares.each { |x| yield x } + end + + def size + middlewares.size + end + + def last + middlewares.last + end + + def [](i) + middlewares[i] + end + + def initialize_copy(other) + self.middlewares = other.middlewares.dup + end + def insert(index, *args, &block) index = assert_index(index, :before) middleware = self.class::Middleware.new(*args, &block) - super(index, middleware) + middlewares.insert(index, middleware) end alias_method :insert_before, :insert @@ -67,21 +97,25 @@ module ActionDispatch delete(target) end + def delete(target) + middlewares.delete target + end + def use(*args, &block) middleware = self.class::Middleware.new(*args, &block) - push(middleware) + middlewares.push(middleware) end def build(app = nil, &block) app ||= block raise "MiddlewareStack#build requires an app" unless app - reverse.inject(app) { |a, e| e.build(a) } + middlewares.reverse.inject(app) { |a, e| e.build(a) } end protected def assert_index(index, where) - i = index.is_a?(Integer) ? index : self.index(index) + i = index.is_a?(Integer) ? index : middlewares.index(index) raise "No such middleware to insert #{where}: #{index.inspect}" unless i i end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 913b899e20..c57f694c4d 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -3,10 +3,10 @@ require 'rack/utils' module ActionDispatch class FileHandler def initialize(at, root) - @at, @root = at.chomp('/'), root.chomp('/') - @compiled_at = (Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank?) - @compiled_root = Regexp.compile(/^#{Regexp.escape(root)}/) - @file_server = ::Rack::File.new(@root) + @at, @root = at.chomp('/'), root.chomp('/') + @compiled_at = @at.blank? ? nil : /^#{Regexp.escape(at)}/ + @compiled_root = /^#{Regexp.escape(root)}/ + @file_server = ::Rack::File.new(@root) end def match?(path) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 4b4e9da173..fc86d52a3a 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -50,12 +50,13 @@ module ActionDispatch private def controller_reference(controller_param) + controller_name = "#{controller_param.camelize}Controller" + unless controller = @controllers[controller_param] - controller_name = "#{controller_param.camelize}Controller" controller = @controllers[controller_param] = - ActiveSupport::Dependencies.ref(controller_name) + ActiveSupport::Dependencies.reference(controller_name) end - controller.get + controller.get(controller_name) end def dispatch(controller, action, env) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb index 82bbfcc7d2..b9126af944 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb @@ -69,7 +69,7 @@ module ActionView def register_javascript_expansion(expansions) js_expansions = JavascriptIncludeTag.expansions expansions.each do |key, values| - js_expansions[key] = (js_expansions[key] || []) | Array(values) + js_expansions[key] = (js_expansions[key] || []) | Array(values) if values end end end diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb index a48c87b49a..f3e041de95 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb @@ -46,7 +46,7 @@ module ActionView def register_stylesheet_expansion(expansions) style_expansions = StylesheetIncludeTag.expansions expansions.each do |key, values| - style_expansions[key] = (style_expansions[key] || []) | Array(values) + style_expansions[key] = (style_expansions[key] || []) | Array(values) if values end end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index cc393d3ef4..01f3e8f2b6 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -559,6 +559,11 @@ class ActionCacheTest < ActionController::TestCase assert_response 404 end + def test_four_oh_four_renders_content + get :four_oh_four + assert_equal "404'd!", @response.body + end + def test_simple_runtime_error_returns_500_for_multiple_requests get :simple_runtime_error assert_response 500 diff --git a/actionpack/test/dispatch/middleware_stack/middleware_test.rb b/actionpack/test/dispatch/middleware_stack/middleware_test.rb new file mode 100644 index 0000000000..9607f026db --- /dev/null +++ b/actionpack/test/dispatch/middleware_stack/middleware_test.rb @@ -0,0 +1,77 @@ +require 'abstract_unit' +require 'action_dispatch/middleware/stack' + +module ActionDispatch + class MiddlewareStack + class MiddlewareTest < ActiveSupport::TestCase + class Omg; end + + { + 'concrete' => Omg, + 'anonymous' => Class.new + }.each do |name, klass| + + define_method("test_#{name}_klass") do + mw = Middleware.new klass + assert_equal klass, mw.klass + end + + define_method("test_#{name}_==") do + mw1 = Middleware.new klass + mw2 = Middleware.new klass + assert_equal mw1, mw2 + end + + end + + def test_string_class + mw = Middleware.new Omg.name + assert_equal Omg, mw.klass + end + + def test_double_equal_works_with_classes + k = Class.new + mw = Middleware.new k + assert_operator mw, :==, k + + result = mw != Class.new + assert result, 'middleware should not equal other anon class' + end + + def test_double_equal_works_with_strings + mw = Middleware.new Omg + assert_operator mw, :==, Omg.name + end + + def test_double_equal_normalizes_strings + mw = Middleware.new Omg + assert_operator mw, :==, "::#{Omg.name}" + end + + def test_middleware_loads_classnames_from_cache + mw = Class.new(Middleware) { + attr_accessor :classcache + }.new(Omg.name) + + fake_cache = { mw.name => Omg } + mw.classcache = fake_cache + + assert_equal Omg, mw.klass + + fake_cache[mw.name] = Middleware + assert_equal Middleware, mw.klass + end + + def test_middleware_always_returns_class + mw = Class.new(Middleware) { + attr_accessor :classcache + }.new(Omg) + + fake_cache = { mw.name => Middleware } + mw.classcache = fake_cache + + assert_equal Omg, mw.klass + end + end + end +end diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 1bf748af14..a1a6b5f1d0 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -303,17 +303,8 @@ class AssetTagHelperTest < ActionView::TestCase end def test_custom_javascript_expansions_with_undefined_symbol - assert_raise(ArgumentError) { javascript_include_tag('first', :unknown, 'last') } - end - - def test_custom_javascript_expansions_with_nil_value ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey => nil - assert_dom_equal %(<script src="/javascripts/first.js" type="text/javascript"></script>\n<script src="/javascripts/last.js" type="text/javascript"></script>), javascript_include_tag('first', :monkey, 'last') - end - - def test_custom_javascript_expansions_with_empty_array_value - ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey => [] - assert_dom_equal %(<script src="/javascripts/first.js" type="text/javascript"></script>\n<script src="/javascripts/last.js" type="text/javascript"></script>), javascript_include_tag('first', :monkey, 'last') + assert_raise(ArgumentError) { javascript_include_tag('first', :monkey, 'last') } end def test_custom_javascript_and_stylesheet_expansion_with_same_name @@ -388,18 +379,9 @@ class AssetTagHelperTest < ActionView::TestCase assert_dom_equal %(<link href="/stylesheets/london.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/wellington.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/amsterdam.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag('london', :cities) end - def test_custom_stylesheet_expansions_with_unknown_symbol - assert_raise(ArgumentError) { stylesheet_link_tag('first', :unknown, 'last') } - end - - def test_custom_stylesheet_expansions_with_nil_value + def test_custom_stylesheet_expansions_with_undefined_symbol ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :monkey => nil - assert_dom_equal %(<link href="/stylesheets/first.css" rel="stylesheet" type="text/css" media="screen" />\n<link href="/stylesheets/last.css" rel="stylesheet" type="text/css" media="screen" />), stylesheet_link_tag('first', :monkey, 'last') - end - - def test_custom_stylesheet_expansions_with_empty_array_value - ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :monkey => [] - assert_dom_equal %(<link href="/stylesheets/first.css" rel="stylesheet" type="text/css" media="screen" />\n<link href="/stylesheets/last.css" rel="stylesheet" type="text/css" media="screen" />), stylesheet_link_tag('first', :monkey, 'last') + assert_raise(ArgumentError) { stylesheet_link_tag('first', :monkey, 'last') } end def test_registering_stylesheet_expansions_merges_with_existing_expansions diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index b4eb3f76e1..aca2fef170 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -1882,10 +1882,17 @@ class DateHelperTest < ActionView::TestCase end def test_datetime_select_defaults_to_time_zone_now_when_config_time_zone_is_set - time = stub(:year => 2004, :month => 6, :day => 15, :hour => 16, :min => 35, :sec => 0) - time_zone = mock() - time_zone.expects(:now).returns time - Time.zone = time_zone + # The love zone is UTC+0 + mytz = Class.new(ActiveSupport::TimeZone) { + attr_accessor :now + }.create('tenderlove', 0) + + now = Time.mktime(2004, 6, 15, 16, 35, 0) + mytz.now = now + Time.zone = mytz + + assert_equal mytz, Time.zone + @post = Post.new expected = %{<select id="post_updated_at_1i" name="post[updated_at(1i)]">\n} |