diff options
Diffstat (limited to 'actionpack')
21 files changed, 254 insertions, 65 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 91274f35ab..1d38e33874 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,85 @@ ## Rails 3.2.9 (unreleased) ## +* Accept :remote as symbolic option for `link_to` helper. *Riley Lynch* + +* Warn when the `:locals` option is passed to `assert_template` outside of a view test case + Fix #3415 + + *Yves Senn* + +* Rename internal variables on ActionController::TemplateAssertions to prevent + naming collisions. @partials, @templates and @layouts are now prefixed with an underscore. + Fix #7459 + + *Yves Senn* + +* `resource` and `resources` don't modify the passed options hash + Fix #7777 + + *Yves Senn* + +* Precompiled assets include aliases from foo.js to foo/index.js and vice versa. + + # Precompiles phone-<digest>.css and aliases phone/index.css to phone.css. + config.assets.precompile = [ 'phone.css' ] + + # Precompiles phone/index-<digest>.css and aliases phone.css to phone/index.css. + config.assets.precompile = [ 'phone/index.css' ] + + # Both of these work with either precompile thanks to their aliases. + <%= stylesheet_link_tag 'phone', media: 'all' %> + <%= stylesheet_link_tag 'phone/index', media: 'all' %> + + *Jeremy Kemper* + +* `assert_template` is no more passing with what ever string that matches + with the template name. + + Before when we have a template `/layout/hello.html.erb`, `assert_template` + was passing with any string that matches. This behavior allowed false + positive like: + + assert_template "layout" + assert_template "out/hello" + + Now it only passes with: + + assert_template "layout/hello" + assert_template "hello" + + Fixes #3849. + + *Hugolnx* + +* Handle `ActionDispatch::Http::UploadedFile` like `Rack::Test::UploadedFile`, don't call to_param on it. Since + `Rack::Test::UploadedFile` isn't API compatible this is needed to test file uploads that rely on `tempfile` + being available. + + *Tim Vandecasteele* + +* Fixed a bug with shorthand routes scoped with the `:module` option not + adding the module to the controller as described in issue #6497. + This should now work properly: + + scope :module => "engine" do + get "api/version" # routes to engine/api#version + end + + *Luiz Felipe Garcia Pereira* + +* Respect `config.digest = false` for `asset_path` + + Previously, the `asset_path` internals only respected the `:digest` + option, but ignored the global config setting. This meant that + `config.digest = false` could not be used in conjunction with + `config.compile = false` this corrects the behavior. + + *Peter Wagenet* + +* Fix #7646, the log now displays the correct status code when an exception is raised. + + *Yves Senn* + * Fix handling of date selects when using both disabled and discard options. Fixes #7431. diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 3d67541557..002351696d 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |s| s.add_dependency('rack', '~> 1.4.0') s.add_dependency('rack-test', '~> 0.6.1') s.add_dependency('journey', '~> 1.0.4') - s.add_dependency('sprockets', '~> 2.1') + s.add_dependency('sprockets', '~> 2.2') s.add_dependency('erubis', '~> 2.7.0') s.add_development_dependency('tzinfo', '~> 0.3.29') diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index c482062592..3b07e4cdba 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -309,7 +309,7 @@ module AbstractController # still does a dynamic lookup. In next Rails release, we should @_layout # to be inheritable so we can skip the child lookup if the parent explicitly # set the layout. - parent = self.superclass.instance_variable_get(:@_layout) + parent = self.superclass.instance_eval { @_layout if defined?(@_layout) } @_layout = nil inspect = parent.is_a?(Proc) ? parent.inspect : parent diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 4c76f4c43b..194f26aefc 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -20,7 +20,8 @@ module ActionController status = payload[:status] if status.nil? && payload[:exception].present? - status = Rack::Utils.status_code(ActionDispatch::ExceptionWrapper.new({}, payload[:exception]).status_code) + exception_class_name = payload[:exception].first + status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in %.0fms" % event.duration message << " (#{additions.join(" | ")})" unless additions.blank? diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 05e3cd40b5..d486f32c01 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -14,13 +14,13 @@ module ActionController end def setup_subscriptions - @partials = Hash.new(0) - @templates = Hash.new(0) - @layouts = Hash.new(0) + @_partials = Hash.new(0) + @_templates = Hash.new(0) + @_layouts = Hash.new(0) ActiveSupport::Notifications.subscribe("render_template.action_view") do |name, start, finish, id, payload| path = payload[:layout] - @layouts[path] += 1 + @_layouts[path] += 1 end ActiveSupport::Notifications.subscribe("!render_template.action_view") do |name, start, finish, id, payload| @@ -28,11 +28,11 @@ module ActionController next unless path partial = path =~ /^.*\/_[^\/]*$/ if partial - @partials[path] += 1 - @partials[path.split("/").last] += 1 - @templates[path] += 1 + @_partials[path] += 1 + @_partials[path.split("/").last] += 1 + @_templates[path] += 1 else - @templates[path] += 1 + @_templates[path] += 1 end end end @@ -43,9 +43,9 @@ module ActionController end def process(*args) - @partials = Hash.new(0) - @templates = Hash.new(0) - @layouts = Hash.new(0) + @_partials = Hash.new(0) + @_templates = Hash.new(0) + @_layouts = Hash.new(0) super end @@ -72,43 +72,54 @@ module ActionController validate_request! case options - when NilClass, String, Symbol + when NilClass, Regexp, String, Symbol options = options.to_s if Symbol === options - rendered = @templates + rendered = @_templates msg = build_message(message, "expecting <?> but rendering with <?>", options, rendered.keys.join(', ')) - assert_block(msg) do - if options + matches_template = + case options + when String + rendered.any? do |t, num| + options_splited = options.split(File::SEPARATOR) + t_splited = t.split(File::SEPARATOR) + t_splited.last(options_splited.size) == options_splited + end + when Regexp rendered.any? { |t,num| t.match(options) } - else - @templates.blank? + when NilClass + rendered.blank? end - end + assert matches_template, msg when Hash if expected_layout = options[:layout] msg = build_message(message, "expecting layout <?> but action rendered <?>", - expected_layout, @layouts.keys) + expected_layout, @_layouts.keys) case expected_layout when String - assert(@layouts.keys.include?(expected_layout), msg) + assert(@_layouts.keys.include?(expected_layout), msg) when Regexp - assert(@layouts.keys.any? {|l| l =~ expected_layout }, msg) + assert(@_layouts.keys.any? {|l| l =~ expected_layout }, msg) when nil - assert(@layouts.empty?, msg) + assert(@_layouts.empty?, msg) end end if expected_partial = options[:partial] if expected_locals = options[:locals] - actual_locals = @locals[expected_partial.to_s.sub(/^_/,'')] - expected_locals.each_pair do |k,v| - assert_equal(v, actual_locals[k]) + if defined?(@locals) + actual_locals = @locals[expected_partial.to_s.sub(/^_/,'')] + expected_locals.each_pair do |k,v| + assert_equal(v, actual_locals[k]) + end + else + warn "the :locals option to #assert_template is only supported in a ActionView::TestCase" end elsif expected_count = options[:count] - actual_count = @partials[expected_partial] + actual_count = @_partials[expected_partial] msg = build_message(message, "expecting ? to be rendered ? time(s) but rendered ? time(s)", expected_partial, expected_count, actual_count) @@ -116,11 +127,11 @@ module ActionController else msg = build_message(message, "expecting partial <?> but action rendered <?>", - options[:partial], @partials.keys) - assert(@partials.include?(expected_partial), msg) + options[:partial], @_partials.keys) + assert(@_partials.include?(expected_partial), msg) end else - assert @partials.empty?, + assert @_partials.empty?, "Expected no partials to be rendered" end end @@ -422,7 +433,7 @@ module ActionController Hash[hash_or_array_or_value.map{|key, value| [key, paramify_values(value)] }] when Array hash_or_array_or_value.map {|i| paramify_values(i)} - when Rack::Test::UploadedFile + when Rack::Test::UploadedFile, ActionDispatch::Http::UploadedFile hash_or_array_or_value else hash_or_array_or_value.to_param diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 94fa747a79..eb95f79e0b 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -19,7 +19,7 @@ module ActionDispatch [:open, :path, :rewind, :size].each do |method| class_eval "def #{method}; @tempfile.#{method}; end" end - + private def encode_filename(filename) # Encode the filename in the utf8 encoding, unless it is nil or we're in 1.8 diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index c0532c80c4..0f49dc65bf 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -34,7 +34,7 @@ module ActionDispatch end def status_code - Rack::Utils.status_code(@@rescue_responses[@exception.class.name]) + self.class.status_code_for_exception(@exception.class.name) end def application_trace @@ -49,6 +49,10 @@ module ActionDispatch clean_backtrace(:all) end + def self.status_code_for_exception(class_name) + Rack::Utils.status_code(@@rescue_responses[class_name]) + end + private def original_exception(exception) @@ -75,4 +79,4 @@ module ActionDispatch @backtrace_cleaner ||= @env['action_dispatch.backtrace_cleaner'] end end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 40ff69693b..cd2f464506 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -166,7 +166,7 @@ module ActionDispatch controller ||= default_controller action ||= default_action - unless controller.is_a?(Regexp) || to_shorthand + unless controller.is_a?(Regexp) controller = [@scope[:module], controller].compact.join("/").presence end @@ -982,7 +982,7 @@ module ActionDispatch # === Options # Takes same options as +resources+. def resource(*resources, &block) - options = resources.extract_options! + options = resources.extract_options!.dup if apply_common_behavior_for(:resource, resources, options, &block) return self @@ -1120,7 +1120,7 @@ module ActionDispatch # # resource actions are at /admin/posts. # resources :posts, :path => "admin/posts" def resources(*resources, &block) - options = resources.extract_options! + options = resources.extract_options!.dup if apply_common_behavior_for(:resources, resources, options, &block) return self diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 1f1cd3cee3..812bb4de9e 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -633,7 +633,9 @@ module ActionView end def link_to_remote_options?(options) - options.is_a?(Hash) && options.key?('remote') && options.delete('remote') + if options.is_a?(Hash) + options.delete('remote') || options.delete(:remote) + end end def add_method_to_attributes!(html_options, method) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 658a503f7f..dfaa1d88e4 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -193,16 +193,16 @@ module ActionView @_result @_routes @controller - @layouts + @_layouts @locals @method_name @output_buffer - @partials + @_partials @passed @rendered @request @routes - @templates + @_templates @options @test_passed @view diff --git a/actionpack/lib/sprockets/helpers/rails_helper.rb b/actionpack/lib/sprockets/helpers/rails_helper.rb index 8f0e0f8ee1..690c71b472 100644 --- a/actionpack/lib/sprockets/helpers/rails_helper.rb +++ b/actionpack/lib/sprockets/helpers/rails_helper.rb @@ -147,7 +147,9 @@ module Sprockets if source[0] == ?/ source else - source = digest_for(source) unless options[:digest] == false + if digest_assets && options[:digest] != false + source = digest_for(source) + end source = File.join(dir, source) source = "/#{source}" unless source =~ /^\// source diff --git a/actionpack/lib/sprockets/static_compiler.rb b/actionpack/lib/sprockets/static_compiler.rb index 2e2db4b760..4341a27d5d 100644 --- a/actionpack/lib/sprockets/static_compiler.rb +++ b/actionpack/lib/sprockets/static_compiler.rb @@ -15,13 +15,11 @@ module Sprockets def compile manifest = {} - env.each_logical_path do |logical_path| - if File.basename(logical_path)[/[^\.]+/, 0] == 'index' - logical_path.sub!(/\/index\./, '.') - end - next unless compile_path?(logical_path) + env.each_logical_path(paths) do |logical_path| if asset = env.find_asset(logical_path) - manifest[logical_path] = write_asset(asset) + digest_path = write_asset(asset) + manifest[asset.logical_path] = digest_path + manifest[aliased_path_for(asset.logical_path)] = digest_path end end write_manifest(manifest) if @manifest @@ -43,22 +41,16 @@ module Sprockets end end - def compile_path?(logical_path) - paths.each do |path| - case path - when Regexp - return true if path.match(logical_path) - when Proc - return true if path.call(logical_path) - else - return true if File.fnmatch(path.to_s, logical_path) - end - end - false - end - def path_for(asset) @digest ? asset.digest_path : asset.logical_path end + + def aliased_path_for(logical_path) + if File.basename(logical_path).start_with?('index') + logical_path.sub(/\/index([^\/]+)$/, '\1') + else + logical_path.sub(/\.([^\/]+)$/, '/index.\1') + end + end end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 5252e43c25..81cf096952 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -7,6 +7,7 @@ class ActionPackAssertionsController < ActionController::Base def nothing() head :ok end def hello_world() render :template => "test/hello_world"; end + def hello_repeating_in_path() render :template => "test/hello/hello"; end def hello_xml_world() render :template => "test/hello_xml_world"; end @@ -469,6 +470,20 @@ class AssertTemplateTest < ActionController::TestCase end end + def test_fails_with_incorrect_string_that_matches + get :hello_world + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template 'est/he' + end + end + + def test_fails_with_repeated_name_in_path + get :hello_repeating_in_path + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template 'test/hello' + end + end + def test_fails_with_incorrect_symbol get :hello_world assert_raise(ActiveSupport::TestCase::Assertion) do @@ -476,6 +491,13 @@ class AssertTemplateTest < ActionController::TestCase end end + def test_fails_with_incorrect_symbol_that_matches + get :hello_world + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template :"est/he" + end + end + def test_fails_with_wrong_layout get :render_with_layout assert_raise(ActiveSupport::TestCase::Assertion) do diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 700fd788fa..a72b6dde1a 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -54,6 +54,10 @@ module Another def with_rescued_exception raise SpecialException end + + def with_action_not_found + raise AbstractController::ActionNotFound + end end end @@ -225,6 +229,17 @@ class ACLogSubscriberTest < ActionController::TestCase assert_match(/Completed 406/, logs.last) end + def test_process_action_with_with_action_not_found_logs_404 + begin + get :with_action_not_found + wait + rescue AbstractController::ActionNotFound + end + + assert_equal 2, logs.size + assert_match(/Completed 404/, logs.last) + end + def logs @logs ||= @logger.logged(:info) end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index afaf9e5ea9..8c631e218f 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1405,6 +1405,17 @@ class RenderTest < ActionController::TestCase assert_equal "Bonjour: davidBonjour: mary", @response.body end + def test_locals_option_to_assert_template_is_not_supported + warning_buffer = StringIO.new + $stderr = warning_buffer + + get :partial_collection_with_locals + assert_template :partial => 'customer_greeting', :locals => { :greeting => 'Bonjour' } + assert_equal "the :locals option to #assert_template is only supported in a ActionView::TestCase\n", warning_buffer.string + ensure + $stderr = STDERR + end + def test_partial_collection_with_spacer get :partial_collection_with_spacer assert_equal "Hello: davidonly partialHello: mary", @response.body diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 6a5843f9d7..1da5298900 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -766,6 +766,12 @@ XML assert_equal '159528', @response.body end + def test_action_dispatch_uploaded_file_upload + tf = Class.new { def size; 159528 end } + post :test_file_upload, :file => ActionDispatch::Http::UploadedFile.new(:tempfile => tf.new) + assert_equal '159528', @response.body + end + def test_test_uploaded_file_exception_when_file_doesnt_exist assert_raise(RuntimeError) { Rack::Test::UploadedFile.new('non_existent_file') } end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 00c71dc8be..f32b1b9c71 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -362,6 +362,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :errors, :shallow => true do resources :notices end + get 'api/version' end scope :path => 'api' do @@ -1185,6 +1186,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_resource_does_not_modify_passed_options + options = {:id => /.+?/, :format => /json|xml/} + self.class.stub_controllers do |routes| + routes.draw do + resource :user, options + end + end + assert_equal({:id => /.+?/, :format => /json|xml/}, options) + end + + def test_resources_does_not_modify_passed_options + options = {:id => /.+?/, :format => /json|xml/} + self.class.stub_controllers do |routes| + routes.draw do + resources :users, options + end + end + assert_equal({:id => /.+?/, :format => /json|xml/}, options) + end + def test_path_names with_test_routes do get '/pt/projetos' @@ -1376,6 +1397,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_match_shorthand_with_module + assert_equal '/api/version', api_version_path + get '/api/version' + assert_equal 'api/api#version', @response.body + end + def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper with_test_routes do assert_equal '/replies', replies_path diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 7e4a1519fb..fae39a403e 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -12,7 +12,7 @@ module ActionDispatch uf = Http::UploadedFile.new(:filename => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.original_filename end - + if "ruby".encoding_aware? def test_filename_should_be_in_utf_8 uf = Http::UploadedFile.new(:filename => 'foo', :tempfile => Object.new) diff --git a/actionpack/test/fixtures/test/hello/hello.erb b/actionpack/test/fixtures/test/hello/hello.erb new file mode 100644 index 0000000000..6769dd60bd --- /dev/null +++ b/actionpack/test/fixtures/test/hello/hello.erb @@ -0,0 +1 @@ +Hello world!
\ No newline at end of file diff --git a/actionpack/test/template/sprockets_helper_test.rb b/actionpack/test/template/sprockets_helper_test.rb index b908b6777c..72d03e43e9 100644 --- a/actionpack/test/template/sprockets_helper_test.rb +++ b/actionpack/test/template/sprockets_helper_test.rb @@ -360,4 +360,12 @@ class SprocketsHelperTest < ActionView::TestCase assert_equal '/assets/logo.png', asset_path("logo.png") end + + test "`config.digest = false` works with `config.compile = false`" do + @config.assets.digest = false + @config.assets.compile = false + + assert_equal '/assets/logo.png', + asset_path("logo.png") + end end diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index d55b1fd2bd..38f77203e0 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -227,6 +227,13 @@ class UrlHelperTest < ActiveSupport::TestCase ) end + def test_link_to_with_symbolic_remote_in_non_html_options + assert_dom_equal( + "<a href=\"/\" data-remote=\"true\">Hello</a>", + link_to("Hello", hash_for([:remote, true]), {}) + ) + end + def test_link_tag_using_post_javascript assert_dom_equal( "<a href='http://www.example.com' data-method=\"post\" rel=\"nofollow\">Hello</a>", |