From 91320f2a809d3a59efabd9f839fe9b879f4a9b29 Mon Sep 17 00:00:00 2001 From: Damian Janowski Date: Fri, 4 Jul 2008 17:12:30 -0300 Subject: Add :recursive option to javascript_include_tag and stylesheet_link_tag to be used along with :all. [#480 state:resolved] Signed-off-by: Pratik Naik --- actionpack/CHANGELOG | 2 + .../lib/action_view/helpers/asset_tag_helper.rb | 56 ++++++++++++++++------ .../fixtures/public/javascripts/subdir/subdir.js | 1 + .../fixtures/public/stylesheets/subdir/subdir.css | 1 + actionpack/test/template/asset_tag_helper_test.rb | 43 +++++++++++++++++ 5 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 actionpack/test/fixtures/public/javascripts/subdir/subdir.js create mode 100644 actionpack/test/fixtures/public/stylesheets/subdir/subdir.css (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index f86a3b21fb..5499cc56ae 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Add :recursive option to javascript_include_tag and stylesheet_link_tag to be used along with :all. #480 [Damian Janowski] + * Disable the Accept header by default [Michael Koziarski] The accept header is poorly implemented by browsers and causes strange diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 0122de47af..bf13945844 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -209,6 +209,10 @@ module ActionView # Note that the default javascript files will be included first. So Prototype and Scriptaculous are available to # all subsequently included files. # + # If you want Rails to search in all the subdirectories under javascripts, you should explicitly set :recursive: + # + # javascript_include_tag :all, :recursive => true + # # == Caching multiple javascripts into one # # You can also cache multiple javascripts into one file, which requires less HTTP connections to download and can better be @@ -235,18 +239,23 @@ module ActionView # # javascript_include_tag "prototype", "cart", "checkout", :cache => "shop" # when ActionController::Base.perform_caching is true => # + # + # The :recursive option is also available for caching: + # + # javascript_include_tag :all, :cache => true, :recursive => true def javascript_include_tag(*sources) options = sources.extract_options!.stringify_keys cache = options.delete("cache") + recursive = options.delete("recursive") if ActionController::Base.perform_caching && cache joined_javascript_name = (cache == true ? "all" : cache) + ".js" joined_javascript_path = File.join(JAVASCRIPTS_DIR, joined_javascript_name) - write_asset_file_contents(joined_javascript_path, compute_javascript_paths(sources)) unless File.exists?(joined_javascript_path) + write_asset_file_contents(joined_javascript_path, compute_javascript_paths(sources, recursive)) unless File.exists?(joined_javascript_path) javascript_src_tag(joined_javascript_name, options) else - expand_javascript_sources(sources).collect { |source| javascript_src_tag(source, options) }.join("\n") + expand_javascript_sources(sources, recursive).collect { |source| javascript_src_tag(source, options) }.join("\n") end end @@ -332,13 +341,17 @@ module ActionView # # # - # You can also include all styles in the stylesheet directory using :all as the source: + # You can also include all styles in the stylesheets directory using :all as the source: # # stylesheet_link_tag :all # => # # # # + # If you want Rails to search in all the subdirectories under stylesheets, you should explicitly set :recursive: + # + # stylesheet_link_tag :all, :recursive => true + # # == Caching multiple stylesheets into one # # You can also cache multiple stylesheets into one file, which requires less HTTP connections and can better be @@ -362,18 +375,23 @@ module ActionView # # stylesheet_link_tag "shop", "cart", "checkout", :cache => "payment" # when ActionController::Base.perform_caching is true => # + # + # The :recursive option is also available for caching: + # + # stylesheet_link_tag :all, :cache => true, :recursive => true def stylesheet_link_tag(*sources) options = sources.extract_options!.stringify_keys cache = options.delete("cache") + recursive = options.delete("recursive") if ActionController::Base.perform_caching && cache joined_stylesheet_name = (cache == true ? "all" : cache) + ".css" joined_stylesheet_path = File.join(STYLESHEETS_DIR, joined_stylesheet_name) - write_asset_file_contents(joined_stylesheet_path, compute_stylesheet_paths(sources)) unless File.exists?(joined_stylesheet_path) + write_asset_file_contents(joined_stylesheet_path, compute_stylesheet_paths(sources, recursive)) unless File.exists?(joined_stylesheet_path) stylesheet_tag(joined_stylesheet_name, options) else - expand_stylesheet_sources(sources).collect { |source| stylesheet_tag(source, options) }.join("\n") + expand_stylesheet_sources(sources, recursive).collect { |source| stylesheet_tag(source, options) }.join("\n") end end @@ -556,18 +574,19 @@ module ActionView tag("link", { "rel" => "stylesheet", "type" => Mime::CSS, "media" => "screen", "href" => html_escape(path_to_stylesheet(source)) }.merge(options), false, false) end - def compute_javascript_paths(sources) - expand_javascript_sources(sources).collect { |source| compute_public_path(source, 'javascripts', 'js', false) } + def compute_javascript_paths(*args) + expand_javascript_sources(*args).collect { |source| compute_public_path(source, 'javascripts', 'js', false) } end - def compute_stylesheet_paths(sources) - expand_stylesheet_sources(sources).collect { |source| compute_public_path(source, 'stylesheets', 'css', false) } + def compute_stylesheet_paths(*args) + expand_stylesheet_sources(*args).collect { |source| compute_public_path(source, 'stylesheets', 'css', false) } end - def expand_javascript_sources(sources) + def expand_javascript_sources(sources, recursive = false) if sources.include?(:all) - all_javascript_files = Dir[File.join(JAVASCRIPTS_DIR, '*.js')].collect { |file| File.basename(file).gsub(/\.\w+$/, '') }.sort - @@all_javascript_sources ||= ((determine_source(:defaults, @@javascript_expansions).dup & all_javascript_files) + all_javascript_files).uniq + all_javascript_files = collect_asset_files(JAVASCRIPTS_DIR, ('**' if recursive), '*.js') + @@all_javascript_sources ||= {} + @@all_javascript_sources[recursive] ||= ((determine_source(:defaults, @@javascript_expansions).dup & all_javascript_files) + all_javascript_files).uniq else expanded_sources = sources.collect do |source| determine_source(source, @@javascript_expansions) @@ -577,9 +596,10 @@ module ActionView end end - def expand_stylesheet_sources(sources) + def expand_stylesheet_sources(sources, recursive) if sources.first == :all - @@all_stylesheet_sources ||= Dir[File.join(STYLESHEETS_DIR, '*.css')].collect { |file| File.basename(file).gsub(/\.\w+$/, '') }.sort + @@all_stylesheet_sources ||= {} + @@all_stylesheet_sources[recursive] ||= collect_asset_files(STYLESHEETS_DIR, ('**' if recursive), '*.css') else sources.collect do |source| determine_source(source, @@stylesheet_expansions) @@ -604,6 +624,14 @@ module ActionView FileUtils.mkdir_p(File.dirname(joined_asset_path)) File.open(joined_asset_path, "w+") { |cache| cache.write(join_asset_file_contents(asset_paths)) } end + + def collect_asset_files(*path) + dir = path.first + + Dir[File.join(*path.compact)].collect do |file| + file[-(file.size - dir.size - 1)..-1].sub(/\.\w+$/, '') + end.sort + end end end end diff --git a/actionpack/test/fixtures/public/javascripts/subdir/subdir.js b/actionpack/test/fixtures/public/javascripts/subdir/subdir.js new file mode 100644 index 0000000000..9d23a67aa1 --- /dev/null +++ b/actionpack/test/fixtures/public/javascripts/subdir/subdir.js @@ -0,0 +1 @@ +// subdir js diff --git a/actionpack/test/fixtures/public/stylesheets/subdir/subdir.css b/actionpack/test/fixtures/public/stylesheets/subdir/subdir.css new file mode 100644 index 0000000000..241152a905 --- /dev/null +++ b/actionpack/test/fixtures/public/stylesheets/subdir/subdir.css @@ -0,0 +1 @@ +/* subdir.css */ diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 4a8117a88a..020e112fd0 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -83,6 +83,7 @@ class AssetTagHelperTest < ActionView::TestCase %(javascript_include_tag("common.javascript", "/elsewhere/cools")) => %(\n), %(javascript_include_tag(:defaults)) => %(\n\n\n\n), %(javascript_include_tag(:all)) => %(\n\n\n\n\n\n\n), + %(javascript_include_tag(:all, :recursive => true)) => %(\n\n\n\n\n\n\n\n), %(javascript_include_tag(:defaults, "test")) => %(\n\n\n\n\n), %(javascript_include_tag("test", :defaults)) => %(\n\n\n\n\n) } @@ -108,6 +109,7 @@ class AssetTagHelperTest < ActionView::TestCase %(stylesheet_link_tag("dir/file")) => %(), %(stylesheet_link_tag("style", :media => "all")) => %(), %(stylesheet_link_tag(:all)) => %(\n\n), + %(stylesheet_link_tag(:all, :recursive => true)) => %(\n\n\n), %(stylesheet_link_tag(:all, :media => "all")) => %(\n\n), %(stylesheet_link_tag("random.styles", "/css/stylish")) => %(\n), %(stylesheet_link_tag("http://www.example.com/styles/style")) => %() @@ -343,6 +345,27 @@ class AssetTagHelperTest < ActionView::TestCase FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'cache', 'money.js')) end + def test_caching_javascript_include_tag_with_all_and_recursive_puts_defaults_at_the_start_of_the_file + ENV["RAILS_ASSET_ID"] = "" + ActionController::Base.asset_host = 'http://a0.example.com' + ActionController::Base.perform_caching = true + + assert_dom_equal( + %(), + javascript_include_tag(:all, :cache => "combined", :recursive => true) + ) + + assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) + + assert_equal( + %(// prototype js\n\n// effects js\n\n// dragdrop js\n\n// controls js\n\n// application js\n\n// bank js\n\n// robber js\n\n// subdir js\n\n\n// version.1.0 js), + IO.read(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) + ) + + ensure + FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) + end + def test_caching_javascript_include_tag_with_all_puts_defaults_at_the_start_of_the_file ENV["RAILS_ASSET_ID"] = "" ActionController::Base.asset_host = 'http://a0.example.com' @@ -373,6 +396,11 @@ class AssetTagHelperTest < ActionView::TestCase javascript_include_tag(:all, :cache => true) ) + assert_dom_equal( + %(\n\n\n\n\n\n\n\n), + javascript_include_tag(:all, :cache => true, :recursive => true) + ) + assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'all.js')) assert_dom_equal( @@ -380,6 +408,11 @@ class AssetTagHelperTest < ActionView::TestCase javascript_include_tag(:all, :cache => "money") ) + assert_dom_equal( + %(\n\n\n\n\n\n\n\n), + javascript_include_tag(:all, :cache => "money", :recursive => true) + ) + assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js')) end @@ -432,6 +465,11 @@ class AssetTagHelperTest < ActionView::TestCase stylesheet_link_tag(:all, :cache => true) ) + assert_dom_equal( + %(\n\n\n), + stylesheet_link_tag(:all, :cache => true, :recursive => true) + ) + assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css')) assert_dom_equal( @@ -439,6 +477,11 @@ class AssetTagHelperTest < ActionView::TestCase stylesheet_link_tag(:all, :cache => "money") ) + assert_dom_equal( + %(\n\n\n), + stylesheet_link_tag(:all, :cache => "money", :recursive => true) + ) + assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css')) end end -- cgit v1.2.3 From 96708af6a58a48c2324a3bf8d34232bc29b398c9 Mon Sep 17 00:00:00 2001 From: Cheah Chu Yeow Date: Mon, 23 Jun 2008 20:51:38 +0800 Subject: Ensure url_for(nil) falls back to url_for({}). [#472 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_controller/base.rb | 3 ++- actionpack/lib/action_view/helpers/url_helper.rb | 6 ++---- actionpack/test/template/url_helper_test.rb | 11 ++++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index c28e9005cf..f6d5491100 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -613,7 +613,8 @@ module ActionController #:nodoc: # # This takes the current URL as is and only exchanges the action. In contrast, url_for :action => 'print' # would have slashed-off the path components after the changed action. - def url_for(options = {}) #:doc: + def url_for(options = {}) + options ||= {} case options when String options diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index d5b7255642..f6a1f271f0 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -63,17 +63,15 @@ module ActionView # # calls @workshop.to_s # # => /workshops/5 def url_for(options = {}) + options ||= {} case options when Hash - show_path = options[:host].nil? ? true : false - options = { :only_path => show_path }.update(options.symbolize_keys) + options = { :only_path => options[:host].nil? }.update(options.symbolize_keys) escape = options.key?(:escape) ? options.delete(:escape) : true url = @controller.send(:url_for, options) when String escape = true url = options - when NilClass - url = @controller.send(:url_for, nil) else escape = false url = polymorphic_path(options) diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 3d5f7eae11..8e43629522 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -313,6 +313,10 @@ class UrlHelperWithControllerTest < ActionView::TestCase render :inline => "<%= show_named_route_#{params[:kind]} %>" end + def nil_url_for + render :inline => '<%= url_for(nil) %>' + end + def rescue_action(e) raise e end end @@ -329,7 +333,7 @@ class UrlHelperWithControllerTest < ActionView::TestCase assert_equal '/url_helper_with_controller/show_url_for', @response.body end - def test_named_route_shows_host_and_path + def test_named_route_url_shows_host_and_path with_url_helper_routing do get :show_named_route, :kind => 'url' assert_equal 'http://test.host/url_helper_with_controller/show_named_route', @response.body @@ -343,6 +347,11 @@ class UrlHelperWithControllerTest < ActionView::TestCase end end + def test_url_for_nil_returns_current_path + get :nil_url_for + assert_equal '/url_helper_with_controller/nil_url_for', @response.body + end + protected def with_url_helper_routing with_routing do |set| -- cgit v1.2.3 From 6b61e95dc8d717b4500ab623816863c0bb707e2b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 9 Jul 2008 09:13:24 -0500 Subject: Changed PrototypeHelper#submit_to_remote to PrototypeHelper#button_to_remote to stay consistent with link_to_remote (submit_to_remote still works as an alias) (clemens) [#8994 status:closed] --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_view/helpers/prototype_helper.rb | 9 ++++++--- actionpack/test/template/prototype_helper_test.rb | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 5499cc56ae..20e9da6338 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Changed PrototypeHelper#submit_to_remote to PrototypeHelper#button_to_remote to stay consistent with link_to_remote (submit_to_remote still works as an alias) #8994 [clemens] + * Add :recursive option to javascript_include_tag and stylesheet_link_tag to be used along with :all. #480 [Damian Janowski] * Disable the Accept header by default [Michael Koziarski] diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index a7c3b9ddc3..d0c281c803 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -397,7 +397,7 @@ module ActionView # # Generates: - # <%= submit_to_remote 'create_btn', 'Create', :url => { :action => 'create' } %> + # <%= button_to_remote 'create_btn', 'Create', :url => { :action => 'create' } %> # # # Submit to the remote action update and update the DIV succeed or fail based # # on the success or failure of the request @@ -405,11 +405,13 @@ module ActionView # # Generates: - # <%= submit_to_remote 'update_btn', 'Update', :url => { :action => 'update' }, + # <%= button_to_remote 'update_btn', 'Update', :url => { :action => 'update' }, # :update => { :success => "succeed", :failure => "fail" } # # options argument is the same as in form_remote_tag. - def submit_to_remote(name, value, options = {}) + # + # Note: This method used to be called submit_to_remote, but that's now just an alias for button_to_remote + def button_to_remote(name, value, options = {}) options[:with] ||= 'Form.serialize(this.form)' options[:html] ||= {} @@ -420,6 +422,7 @@ module ActionView tag("input", options[:html], false) end + alias_method :submit_to_remote, :button_to_remote # Returns 'eval(request.responseText)' which is the JavaScript function # that +form_remote_tag+ can call in :complete to evaluate a multiple diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb index 60b83b476d..1d9bc5eb9b 100644 --- a/actionpack/test/template/prototype_helper_test.rb +++ b/actionpack/test/template/prototype_helper_test.rb @@ -201,9 +201,9 @@ class PrototypeHelperTest < PrototypeHelperBaseTest end - def test_submit_to_remote + def test_button_to_remote assert_dom_equal %(), - submit_to_remote("More beer!", 1_000_000, :update => "empty_bottle") + button_to_remote("More beer!", 1_000_000, :update => "empty_bottle") end def test_observe_field -- cgit v1.2.3 From 4ce9931f4f30045b2975328e7d42a02188e35079 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Wed, 9 Jul 2008 18:35:35 +0200 Subject: Reenable the use of the Accept header to give people a chance to update their applications and provide feedback. --- actionpack/CHANGELOG | 9 +++++---- actionpack/lib/action_controller/base.rb | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 20e9da6338..89cd7c64af 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -4,15 +4,16 @@ * Add :recursive option to javascript_include_tag and stylesheet_link_tag to be used along with :all. #480 [Damian Janowski] -* Disable the Accept header by default [Michael Koziarski] +* Allow users to disable the use of the Accept header [Michael Koziarski] The accept header is poorly implemented by browsers and causes strange errors when used on public sites where crawlers make requests too. You - should use formatted urls (e.g. /people/1.xml) to support API clients. + can use formatted urls (e.g. /people/1.xml) to support API clients in a + much simpler way. - Alternatively to re-enable it you need to set: + To disable the header you need to set: - config.action_controller.use_accept_header = true + config.action_controller.use_accept_header = false * Do not stat template files in production mode before rendering. You will no longer be able to modify templates in production mode without restarting the server [Josh Peek] diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index f6d5491100..dd50f17c1c 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -343,12 +343,12 @@ module ActionController #:nodoc: # Indicates whether the response format should be determined by examining the Accept HTTP header, # or by using the simpler params + ajax rules. # - # If this is set to +true+ then +respond_to+ and +Request#format+ will take the Accept header into - # account. If it is set to false (the default) then the request format will be determined solely + # If this is set to +true+ (the default) then +respond_to+ and +Request#format+ will take the Accept + # header into account. If it is set to false then the request format will be determined solely # by examining params[:format]. If params format is missing, the format will be either HTML or # Javascript depending on whether the request is an AJAX request. cattr_accessor :use_accept_header - self.use_accept_header = false + self.use_accept_header = true # Controls whether request forgergy protection is turned on or not. Turned off by default only in test mode. class_inheritable_accessor :allow_forgery_protection -- cgit v1.2.3 From 350faf14e80740a440ab16c7130c5262d603d283 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 7 Jul 2008 11:02:15 -0700 Subject: Pass caller to concat deprecation warning --- actionpack/lib/action_view/helpers/text_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index a6c48737e9..3e3452b615 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -27,7 +27,7 @@ module ActionView # %> def concat(string, unused_binding = nil) if unused_binding - ActiveSupport::Deprecation.warn("The binding argument of #concat is no longer needed. Please remove it from your views and helpers.") + ActiveSupport::Deprecation.warn("The binding argument of #concat is no longer needed. Please remove it from your views and helpers.", caller) end output_buffer << string -- cgit v1.2.3 From 4354aa36fb0b94f3750256441054d42db9900bf5 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 7 Jul 2008 11:57:53 -0700 Subject: Rendering default template for missing actions works with non-word characters in action name --- actionpack/lib/action_view/template_file.rb | 2 +- actionpack/test/controller/new_render_test.rb | 5 +++++ actionpack/test/fixtures/test/hyphen-ated.erb | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 actionpack/test/fixtures/test/hyphen-ated.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template_file.rb b/actionpack/lib/action_view/template_file.rb index c38e8ed122..0aa16b5e70 100644 --- a/actionpack/lib/action_view/template_file.rb +++ b/actionpack/lib/action_view/template_file.rb @@ -74,7 +74,7 @@ module ActionView #:nodoc: # Returns file split into an array # [base_path, name, format, extension] def split(file) - if m = file.match(/^(.*\/)?(\w+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) + if m = file.match(/^(.*\/)?([^\.]+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) if m[5] # Mulipart formats [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]] elsif m[4] # Single format diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index b2691d981b..b4dc2bb4dc 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -489,6 +489,11 @@ class NewRenderTest < Test::Unit::TestCase assert_equal "Hello world!", @response.body end + def test_renders_default_template_for_missing_action + get :'hyphen-ated' + assert_template 'test/hyphen-ated' + end + def test_do_with_render get :render_hello_world assert_template "test/hello_world" diff --git a/actionpack/test/fixtures/test/hyphen-ated.erb b/actionpack/test/fixtures/test/hyphen-ated.erb new file mode 100644 index 0000000000..cd0875583a --- /dev/null +++ b/actionpack/test/fixtures/test/hyphen-ated.erb @@ -0,0 +1 @@ +Hello world! -- cgit v1.2.3 From 7dc10478e53ca4f36951c85aab4885e5de6e11cb Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 7 Jul 2008 12:42:51 -0700 Subject: Use ActionController::Base.logger to report template compilation errors since there is no AV::Base.logger --- actionpack/lib/action_view/template_handlers/compilable.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index 256d8da031..9b9255579c 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -37,10 +37,10 @@ module ActionView file_name = template.filename || 'compiled-template' ActionView::Base::CompiledTemplates.module_eval(source, file_name, 0) rescue Exception => e # errors from template code - if Base.logger - Base.logger.debug "ERROR: compiling #{template.method} RAISED #{e}" - Base.logger.debug "Function body: #{source}" - Base.logger.debug "Backtrace: #{e.backtrace.join("\n")}" + if logger = ActionController::Base.logger + logger.debug "ERROR: compiling #{template.method} RAISED #{e}" + logger.debug "Function body: #{source}" + logger.debug "Backtrace: #{e.backtrace.join("\n")}" end raise ActionView::TemplateError.new(template, @view.assigns, e) -- cgit v1.2.3 From ee6bbcb6ae4488a6ab556236dfa1cf7358378dcd Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 7 Jul 2008 12:43:43 -0700 Subject: Put a newline rather than a semicolon at the end of RJS source to avoid parse errors with embedded heredocs --- actionpack/lib/action_view/template_handlers/rjs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template_handlers/rjs.rb b/actionpack/lib/action_view/template_handlers/rjs.rb index 31ce598c46..3892bf1d6e 100644 --- a/actionpack/lib/action_view/template_handlers/rjs.rb +++ b/actionpack/lib/action_view/template_handlers/rjs.rb @@ -5,7 +5,7 @@ module ActionView def compile(template) "controller.response.content_type ||= Mime::JS;" + - "update_page do |page|;#{template.source};end" + "update_page do |page|;#{template.source}\nend" end def cache_fragment(block, name = {}, options = nil) #:nodoc: -- cgit v1.2.3 From a6d0ae28e3ceef909f78afc27b936bca1e4b1021 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 7 Jul 2008 13:09:56 -0700 Subject: Fix teardown method name typo --- actionpack/test/controller/content_type_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index 8f55974172..33aa4e49ee 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -124,7 +124,7 @@ class AcceptBasedContentTypeTest < ActionController::TestCase ActionController::Base.use_accept_header = true end - def tear_down + def teardown ActionController::Base.use_accept_header = false end -- cgit v1.2.3 From f82bd31cb013cfba5bf3f5cad7356070fcefcc22 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 8 Jul 2008 12:35:03 -0700 Subject: Request#accepts special-cases a single mime type --- actionpack/lib/action_controller/request.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index c76a93f7a1..7791e0696e 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -82,10 +82,16 @@ module ActionController # Returns the accepted MIME type for the request def accepts @accepts ||= - if @env['HTTP_ACCEPT'].to_s.strip.empty? - [ content_type, Mime::ALL ].compact # make sure content_type being nil is not included - else - Mime::Type.parse(@env['HTTP_ACCEPT']) + begin + header = @env['HTTP_ACCEPT'].to_s.strip + + if header.empty? + [content_type, Mime::ALL].compact + elsif header !~ /,/ + [Mime::Type.lookup(header)].compact + else + Mime::Type.parse(header) + end end end -- cgit v1.2.3 From ce4a1bb8538bd7cc5ee3cbf1156dc587482a7839 Mon Sep 17 00:00:00 2001 From: Cheah Chu Yeow Date: Thu, 26 Jun 2008 10:21:53 +0800 Subject: Remove some Symbol#to_proc usage in runtime code. [#484 state:resolved] --- actionpack/lib/action_controller/base.rb | 12 ++++++------ actionpack/lib/action_controller/filters.rb | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index dd50f17c1c..9d9ff527c2 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -412,7 +412,7 @@ module ActionController #:nodoc: # More methods can be hidden using hide_actions. def hidden_actions unless read_inheritable_attribute(:hidden_actions) - write_inheritable_attribute(:hidden_actions, ActionController::Base.public_instance_methods.map(&:to_s)) + write_inheritable_attribute(:hidden_actions, ActionController::Base.public_instance_methods.map { |m| m.to_s }) end read_inheritable_attribute(:hidden_actions) @@ -420,12 +420,12 @@ module ActionController #:nodoc: # Hide each of the given methods from being callable as actions. def hide_action(*names) - write_inheritable_attribute(:hidden_actions, hidden_actions | names.map(&:to_s)) + write_inheritable_attribute(:hidden_actions, hidden_actions | names.map { |name| name.to_s }) end - ## View load paths determine the bases from which template references can be made. So a call to - ## render("test/template") will be looked up in the view load paths array and the closest match will be - ## returned. + # View load paths determine the bases from which template references can be made. So a call to + # render("test/template") will be looked up in the view load paths array and the closest match will be + # returned. def view_paths @view_paths || superclass.view_paths end @@ -1201,7 +1201,7 @@ module ActionController #:nodoc: end def self.action_methods - @action_methods ||= Set.new(public_instance_methods.map(&:to_s)) - hidden_actions + @action_methods ||= Set.new(public_instance_methods.map { |m| m.to_s }) - hidden_actions end def add_variables_to_assigns diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 155d8b480a..c3f1de971e 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -127,9 +127,9 @@ module ActionController #:nodoc: def included_in_action?(controller, options) if options[:only] - Array(options[:only]).map(&:to_s).include?(controller.action_name) + Array(options[:only]).map { |o| o.to_s }.include?(controller.action_name) elsif options[:except] - !Array(options[:except]).map(&:to_s).include?(controller.action_name) + !Array(options[:except]).map { |o| o.to_s }.include?(controller.action_name) else true end @@ -544,13 +544,21 @@ module ActionController #:nodoc: # Returns all the before filters for this class and all its ancestors. # This method returns the actual filter that was assigned in the controller to maintain existing functionality. def before_filters #:nodoc: - filter_chain.select(&:before?).map(&:method) + filters = [] + filter_chain.each do |filter| + filters << filter.method if filter.before? + end + filters end # Returns all the after filters for this class and all its ancestors. # This method returns the actual filter that was assigned in the controller to maintain existing functionality. def after_filters #:nodoc: - filter_chain.select(&:after?).map(&:method) + filters = [] + filter_chain.each do |filter| + filters << filter.method if filter.after? + end + filters end end -- cgit v1.2.3 From 0ce7fe5308337ae0b0394ae4c18b59cf64e33b38 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 8 Jul 2008 23:38:18 -0700 Subject: Don't repeatedly convert only/except options --- actionpack/lib/action_controller/filters.rb | 31 ++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index c3f1de971e..fc63890d13 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -94,7 +94,7 @@ module ActionController #:nodoc: map! do |filter| if filters.include?(filter) new_filter = filter.dup - new_filter.options.merge!(options) + new_filter.update_options!(options) new_filter else filter @@ -104,6 +104,11 @@ module ActionController #:nodoc: end class Filter < ActiveSupport::Callbacks::Callback #:nodoc: + def initialize(kind, method, options = {}) + super + update_options! options + end + def before? self.class == BeforeFilter end @@ -116,6 +121,18 @@ module ActionController #:nodoc: self.class == AroundFilter end + # Make sets of strings from :only/:except options + def update_options!(other) + if other + convert_only_and_except_options_to_sets_of_strings(other) + if other[:skip] + convert_only_and_except_options_to_sets_of_strings(other[:skip]) + end + end + + options.update(other) + end + private def should_not_skip?(controller) if options[:skip] @@ -127,9 +144,9 @@ module ActionController #:nodoc: def included_in_action?(controller, options) if options[:only] - Array(options[:only]).map { |o| o.to_s }.include?(controller.action_name) + options[:only].include?(controller.action_name) elsif options[:except] - !Array(options[:except]).map { |o| o.to_s }.include?(controller.action_name) + !options[:except].include?(controller.action_name) else true end @@ -138,6 +155,14 @@ module ActionController #:nodoc: def should_run_callback?(controller) should_not_skip?(controller) && included_in_action?(controller, options) && super end + + def convert_only_and_except_options_to_sets_of_strings(opts) + [:only, :except].each do |key| + if values = opts[key] + opts[key] = Array(values).map(&:to_s).to_set + end + end + end end class AroundFilter < Filter #:nodoc: -- cgit v1.2.3 From d37e6413366c9a3fafa02c4298a2946dc8327a42 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 9 Jul 2008 11:30:18 -0700 Subject: Move accept header parsing shortcut to Mime::Type.parse --- actionpack/lib/action_controller/mime_type.rb | 82 ++++++++++++++------------- actionpack/lib/action_controller/request.rb | 2 - 2 files changed, 43 insertions(+), 41 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/mime_type.rb b/actionpack/lib/action_controller/mime_type.rb index fa123f7808..a7215e6ea3 100644 --- a/actionpack/lib/action_controller/mime_type.rb +++ b/actionpack/lib/action_controller/mime_type.rb @@ -72,57 +72,61 @@ module Mime end def parse(accept_header) - # keep track of creation order to keep the subsequent sort stable - list = [] - accept_header.split(/,/).each_with_index do |header, index| - params, q = header.split(/;\s*q=/) - if params - params.strip! - list << AcceptItem.new(index, params, q) unless params.empty? + if accept_header !~ /,/ + [Mime::Type.lookup(accept_header)] + else + # keep track of creation order to keep the subsequent sort stable + list = [] + accept_header.split(/,/).each_with_index do |header, index| + params, q = header.split(/;\s*q=/) + if params + params.strip! + list << AcceptItem.new(index, params, q) unless params.empty? + end end - end - list.sort! + list.sort! - # Take care of the broken text/xml entry by renaming or deleting it - text_xml = list.index("text/xml") - app_xml = list.index(Mime::XML.to_s) + # Take care of the broken text/xml entry by renaming or deleting it + text_xml = list.index("text/xml") + app_xml = list.index(Mime::XML.to_s) - if text_xml && app_xml - # set the q value to the max of the two - list[app_xml].q = [list[text_xml].q, list[app_xml].q].max + if text_xml && app_xml + # set the q value to the max of the two + list[app_xml].q = [list[text_xml].q, list[app_xml].q].max - # make sure app_xml is ahead of text_xml in the list - if app_xml > text_xml - list[app_xml], list[text_xml] = list[text_xml], list[app_xml] - app_xml, text_xml = text_xml, app_xml - end + # make sure app_xml is ahead of text_xml in the list + if app_xml > text_xml + list[app_xml], list[text_xml] = list[text_xml], list[app_xml] + app_xml, text_xml = text_xml, app_xml + end - # delete text_xml from the list - list.delete_at(text_xml) + # delete text_xml from the list + list.delete_at(text_xml) - elsif text_xml - list[text_xml].name = Mime::XML.to_s - end + elsif text_xml + list[text_xml].name = Mime::XML.to_s + end - # Look for more specific XML-based types and sort them ahead of app/xml + # Look for more specific XML-based types and sort them ahead of app/xml - if app_xml - idx = app_xml - app_xml_type = list[app_xml] + if app_xml + idx = app_xml + app_xml_type = list[app_xml] - while(idx < list.length) - type = list[idx] - break if type.q < app_xml_type.q - if type.name =~ /\+xml$/ - list[app_xml], list[idx] = list[idx], list[app_xml] - app_xml = idx + while(idx < list.length) + type = list[idx] + break if type.q < app_xml_type.q + if type.name =~ /\+xml$/ + list[app_xml], list[idx] = list[idx], list[app_xml] + app_xml = idx + end + idx += 1 end - idx += 1 end - end - list.map! { |i| Mime::Type.lookup(i.name) }.uniq! - list + list.map! { |i| Mime::Type.lookup(i.name) }.uniq! + list + end end end diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 7791e0696e..2d9f6c3e6f 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -87,8 +87,6 @@ module ActionController if header.empty? [content_type, Mime::ALL].compact - elsif header !~ /,/ - [Mime::Type.lookup(header)].compact else Mime::Type.parse(header) end -- cgit v1.2.3 From feb08984ea5517db5780a88584929feac1cafb59 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Wed, 9 Jul 2008 21:41:03 +0200 Subject: Added notes to Routing documentation and routes.rb regarding defaults routes opening the whole application for GET requests Signed-off-by: Michael Koziarski --- actionpack/lib/action_controller/routing.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 8846dcc504..dfbaa53b7c 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -88,6 +88,10 @@ module ActionController # # map.connect ':controller/:action/:id', :action => 'show', :defaults => { :page => 'Dashboard' } # + # Note: The default routes, as provided by the Rails generator, make all actions in every + # controller accessible via GET requests. You should consider removing them or commenting + # them out if you're using named routes and resources. + # # == Named routes # # Routes can be named with the syntax map.name_of_route options, -- cgit v1.2.3 From 68289693f723f7a11ab13014a784d0a95abfd6ed Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 11:14:59 -0500 Subject: Check for response in builder template since ActionMailer does not have one --- actionpack/lib/action_view/template_handlers/builder.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template_handlers/builder.rb b/actionpack/lib/action_view/template_handlers/builder.rb index 2d413490bd..cbe53e11d8 100644 --- a/actionpack/lib/action_view/template_handlers/builder.rb +++ b/actionpack/lib/action_view/template_handlers/builder.rb @@ -6,7 +6,8 @@ module ActionView include Compilable def compile(template) - "controller.response.content_type ||= Mime::XML;" + + # ActionMailer does not have a response + "controller.respond_to?(:response) && controller.response.content_type ||= Mime::XML;" + "xml = ::Builder::XmlMarkup.new(:indent => 2);" + template.source + ";xml.target!;" -- cgit v1.2.3 From 15b21754268e6e9fd90e866096ebd60ee6fd972b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 11:44:24 -0500 Subject: Fixed teardown method typo (plus whitespace) --- actionpack/test/controller/resources_test.rb | 32 +++++++++++++--------------- 1 file changed, 15 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 0d089d0f23..0f7924649a 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -28,18 +28,16 @@ module Backoffice end class ResourcesTest < Test::Unit::TestCase - - # The assertions in these tests are incompatible with the hash method # optimisation. This could indicate user level problems def setup ActionController::Base.optimise_named_routes = false end - - def tear_down + + def teardown ActionController::Base.optimise_named_routes = true end - + def test_should_arrange_actions resource = ActionController::Resources::Resource.new(:messages, :collection => { :rss => :get, :reorder => :post, :csv => :post }, @@ -159,14 +157,14 @@ class ResourcesTest < Test::Unit::TestCase def test_with_collection_actions_and_name_prefix actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete } - + with_restful_routing :messages, :path_prefix => '/threads/:thread_id', :name_prefix => "thread_", :collection => actions do assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| actions.each do |action, method| assert_recognizes(options.merge(:action => action), :path => "/threads/1/messages/#{action}", :method => method) end end - + assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| actions.keys.each do |action| assert_named_route "/threads/1/messages/#{action}", "#{action}_thread_messages_path", :action => action @@ -177,14 +175,14 @@ class ResourcesTest < Test::Unit::TestCase def test_with_collection_action_and_name_prefix_and_formatted actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete } - + with_restful_routing :messages, :path_prefix => '/threads/:thread_id', :name_prefix => "thread_", :collection => actions do assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| actions.each do |action, method| assert_recognizes(options.merge(:action => action, :format => 'xml'), :path => "/threads/1/messages/#{action}.xml", :method => method) end end - + assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| actions.keys.each do |action| assert_named_route "/threads/1/messages/#{action}.xml", "formatted_#{action}_thread_messages_path", :action => action, :format => 'xml' @@ -279,7 +277,7 @@ class ResourcesTest < Test::Unit::TestCase end end end - + def test_with_new_action_with_name_prefix with_restful_routing :messages, :new => { :preview => :post }, :path_prefix => '/threads/:thread_id', :name_prefix => 'thread_' do preview_options = {:action => 'preview', :thread_id => '1'} @@ -293,7 +291,7 @@ class ResourcesTest < Test::Unit::TestCase end end end - + def test_with_formatted_new_action_with_name_prefix with_restful_routing :messages, :new => { :preview => :post }, :path_prefix => '/threads/:thread_id', :name_prefix => 'thread_' do preview_options = {:action => 'preview', :thread_id => '1', :format => 'xml'} @@ -307,7 +305,7 @@ class ResourcesTest < Test::Unit::TestCase end end end - + def test_override_new_method with_restful_routing :messages do assert_restful_routes_for :messages do |options| @@ -524,9 +522,9 @@ class ResourcesTest < Test::Unit::TestCase map.resources :messages, :collection => {:search => :get}, :new => {:preview => :any}, :name_prefix => 'thread_', :path_prefix => '/threads/:thread_id' map.resource :account, :member => {:login => :get}, :new => {:preview => :any}, :name_prefix => 'admin_', :path_prefix => '/admin' end - + action_separator = ActionController::Base.resource_action_separator - + assert_simply_restful_for :messages, :name_prefix => 'thread_', :path_prefix => 'threads/1/', :options => { :thread_id => '1' } assert_named_route "/threads/1/messages#{action_separator}search", "search_thread_messages_path", {} assert_named_route "/threads/1/messages/new", "new_thread_message_path", {} @@ -623,7 +621,7 @@ class ResourcesTest < Test::Unit::TestCase assert_simply_restful_for :products, :controller => "backoffice/products" end end - + def test_nested_resources_using_namespace with_routing do |set| set.draw do |map| @@ -795,7 +793,7 @@ class ResourcesTest < Test::Unit::TestCase yield options[:options] if block_given? end - + def assert_singleton_routes_for(singleton_name, options = {}) options[:options] ||= {} options[:options][:controller] = options[:controller] || singleton_name.to_s.pluralize @@ -855,7 +853,7 @@ class ResourcesTest < Test::Unit::TestCase actual = @controller.send(route, options) rescue $!.class.name assert_equal expected, actual, "Error on route: #{route}(#{options.inspect})" end - + def assert_resource_methods(expected, resource, action_method, method) assert_equal expected.length, resource.send("#{action_method}_methods")[method].size, "#{resource.send("#{action_method}_methods")[method].inspect}" expected.each do |action| -- cgit v1.2.3 From d106f2d08aad517c0bfa3a11e3eb87e14231e8ce Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 11:49:22 -0500 Subject: Ensure use_accept_header is enabled for test_action_cache_conditional_options --- actionpack/test/controller/caching_test.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 7aa4e0cd93..71b53893b8 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -285,9 +285,11 @@ class ActionCacheTest < Test::Unit::TestCase end def test_action_cache_conditional_options + ActionController::Base.use_accept_header = true @request.env['HTTP_ACCEPT'] = 'application/json' get :index assert !fragment_exist?('hostname.com/action_caching_test') + ActionController::Base.use_accept_header = false end def test_action_cache_with_store_options -- cgit v1.2.3 From 04a87af5b7abd00e74b96c5b1bb789d4484da6d9 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 11:51:35 -0500 Subject: Ensure use_accept_header is enabled for test_action_cache_conditional_options --- actionpack/test/controller/caching_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 71b53893b8..15db70e474 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -285,11 +285,12 @@ class ActionCacheTest < Test::Unit::TestCase end def test_action_cache_conditional_options + old_use_accept_header = ActionController::Base.use_accept_header ActionController::Base.use_accept_header = true @request.env['HTTP_ACCEPT'] = 'application/json' get :index assert !fragment_exist?('hostname.com/action_caching_test') - ActionController::Base.use_accept_header = false + ActionController::Base.use_accept_header = old_use_accept_header end def test_action_cache_with_store_options -- cgit v1.2.3 From 6b9f8adb3eff3be81653bd565be4ff9c63cd775d Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 11 Jul 2008 19:22:43 +0200 Subject: Whitespace --- actionpack/lib/action_view/helpers/capture_helper.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index 990c30b90d..720e2da8cc 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -34,9 +34,8 @@ module ActionView # Return captured buffer in erb. if block_called_from_erb?(block) with_output_buffer { block.call(*args) } - - # Return block result otherwise, but protect buffer also. else + # Return block result otherwise, but protect buffer also. with_output_buffer { return block.call(*args) } end end -- cgit v1.2.3 From 6ebdd0e32b846e99a9885b06fbca2bed58149c7e Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 15:39:22 -0500 Subject: Changed ActionView::TemplateHandler#render API method signature to render(template, local_assigns = {}) --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_view/base.rb | 4 ++-- actionpack/lib/action_view/renderable.rb | 2 +- actionpack/lib/action_view/template_handler.rb | 2 +- actionpack/lib/action_view/template_handlers/compilable.rb | 4 ++-- actionpack/test/controller/layout_test.rb | 4 ++-- actionpack/test/template/render_test.rb | 9 ++++----- 7 files changed, 14 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 89cd7c64af..dc508f701a 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Changed ActionView::TemplateHandler#render API method signature to render(template, local_assigns = {}) [Josh Peek] + * Changed PrototypeHelper#submit_to_remote to PrototypeHelper#button_to_remote to stay consistent with link_to_remote (submit_to_remote still works as an alias) #8994 [clemens] * Add :recursive option to javascript_include_tag and stylesheet_link_tag to be used along with :all. #480 [Damian Janowski] diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 66892fdabf..7ef90ddf5e 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -332,8 +332,8 @@ module ActionView #:nodoc: @assigns.each { |key, value| instance_variable_set("@#{key}", value) } end - def execute(template) - send(template.method, template.locals) do |*names| + def execute(template, local_assigns = {}) + send(template.method, local_assigns) do |*names| instance_variable_get "@content_for_#{names.first || 'layout'}" end end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 3a30e603fe..6a8a0e44fc 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -8,7 +8,7 @@ module ActionView def render prepare! - @handler.render(self) + @handler.render(self, @locals) end def method diff --git a/actionpack/lib/action_view/template_handler.rb b/actionpack/lib/action_view/template_handler.rb index 78586d6142..1afea21f67 100644 --- a/actionpack/lib/action_view/template_handler.rb +++ b/actionpack/lib/action_view/template_handler.rb @@ -8,7 +8,7 @@ module ActionView @view = view end - def render(template) + def render(template, local_assigns = {}) end def compile(template) diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index 9b9255579c..95ae75ca68 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -14,8 +14,8 @@ module ActionView end end - def render(template) - @view.send(:execute, template) + def render(template, local_assigns = {}) + @view.send(:execute, template, local_assigns) end # Compile and evaluate the template's code diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 3dc311b78a..32be7d90ff 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -34,8 +34,8 @@ end class MabView < ActionView::TemplateHandler def initialize(view) end - - def render(template) + + def render(template, local_assigns) template.source end end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 726cf37026..cd004a9f6d 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -95,8 +95,8 @@ class ViewRenderTest < Test::Unit::TestCase end class CustomHandler < ActionView::TemplateHandler - def render(template) - [template.source, template.locals].inspect + def render(template, local_assigns) + [template.source, local_assigns].inspect end end @@ -115,18 +115,17 @@ class ViewRenderTest < Test::Unit::TestCase def compile(template) "@output_buffer = ''\n" + - "@output_buffer << 'locals: #{template.locals.inspect}, '\n" + "@output_buffer << 'source: #{template.source.inspect}'\n" end end def test_render_inline_with_compilable_custom_type ActionView::Template.register_template_handler :foo, CompilableCustomHandler - assert_equal 'locals: {}, source: "Hello, World!"', @view.render(:inline => "Hello, World!", :type => :foo) + assert_equal 'source: "Hello, World!"', @view.render(:inline => "Hello, World!", :type => :foo) end def test_render_inline_with_locals_and_compilable_custom_type ActionView::Template.register_template_handler :foo, CompilableCustomHandler - assert_equal 'locals: {:name=>"Josh"}, source: "Hello, <%= name %>!"', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) + assert_equal 'source: "Hello, <%= name %>!"', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) end end -- cgit v1.2.3 From 50b5c6845ed1645cf25613024ef04187385f8dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20S=C3=B8rensen?= Date: Sat, 12 Jul 2008 00:57:38 +0100 Subject: Ensure mail_to label is obfuscated for javascript encoding. [#294 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_view/helpers/url_helper.rb | 2 +- actionpack/test/template/url_helper_test.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index f6a1f271f0..e5178938fd 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -466,7 +466,7 @@ module ActionView email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.has_key?("replace_dot") if encode == "javascript" - "document.write('#{content_tag("a", name || email_address, html_options.merge({ "href" => "mailto:"+email_address+extras }))}');".each_byte do |c| + "document.write('#{content_tag("a", name || email_address_obfuscated, html_options.merge({ "href" => "mailto:"+email_address+extras }))}');".each_byte do |c| string << sprintf("%%%x", c) end "" diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 8e43629522..4ed5bc372f 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -292,6 +292,7 @@ class UrlHelperTest < ActionView::TestCase assert_dom_equal "My email", mail_to("me@domain.com", "My email", :encode => "hex", :replace_at => "(at)") assert_dom_equal "me(at)domain(dot)com", mail_to("me@domain.com", nil, :encode => "hex", :replace_at => "(at)", :replace_dot => "(dot)") assert_dom_equal "", mail_to("me@domain.com", "My email", :encode => "javascript", :replace_at => "(at)", :replace_dot => "(dot)") + assert_dom_equal "", mail_to("me@domain.com", nil, :encode => "javascript", :replace_at => "(at)", :replace_dot => "(dot)") end def protect_against_forgery? -- cgit v1.2.3 From e53f5fe696d692f1985981c34bb311e898fe3c72 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Sat, 12 Jul 2008 11:42:41 +0200 Subject: Restore support for partial matches in assert_redirected_to If both the actual redirection and the asserted redirection are hashes, succeed if the asserted redirection is a strict subset of the actual redirection. --- .../lib/action_controller/assertions/response_assertions.rb | 11 +++++++++-- actionpack/test/controller/redirect_test.rb | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index 3ecaee641f..64aaf6211f 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -63,11 +63,18 @@ module ActionController clean_backtrace do assert_response(:redirect, message) return true if options == @response.redirected_to + + # 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) options_after_normalisation = normalize_argument_to_redirection(options) - assert_equal redirected_to_after_normalisation, options_after_normalisation, - "Expected response to be a redirect to <#{options_after_normalisation}> but was a redirect to <#{redirected_to_after_normalisation}>" + if redirected_to_after_normalisation != options_after_normalisation + flunk "Expected response to be a redirect to <#{options_after_normalisation}> but was a redirect to <#{redirected_to_after_normalisation}>" + end end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 8b72426d10..28da5c6163 100755 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -227,6 +227,11 @@ class RedirectTest < Test::Unit::TestCase assert_redirected_to Workshop.new(5, true) end + def test_redirect_with_partial_params + get :module_redirect + assert_redirected_to :action => 'hello_world' + end + def test_redirect_to_nil assert_raises(ActionController::ActionControllerError) do get :redirect_to_nil -- cgit v1.2.3 From b603de08825eec05f1b97d0f5d71462f3fa4c222 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 12 Jul 2008 12:15:31 -0500 Subject: Improve test coverage and create fixtures for RenderPartialWithRecordIdentificationTests --- .../render_partial_with_record_identification_test.rb | 9 +++++++++ actionpack/test/fixtures/developers/_developer.erb | 1 + actionpack/test/fixtures/fun/games/_game.erb | 1 + actionpack/test/fixtures/fun/serious/games/_game.erb | 1 + actionpack/test/fixtures/projects/_project.erb | 1 + actionpack/test/fixtures/replies/_reply.erb | 1 + 6 files changed, 14 insertions(+) create mode 100644 actionpack/test/fixtures/developers/_developer.erb create mode 100644 actionpack/test/fixtures/fun/games/_game.erb create mode 100644 actionpack/test/fixtures/fun/serious/games/_game.erb create mode 100644 actionpack/test/fixtures/projects/_project.erb create mode 100644 actionpack/test/fixtures/replies/_reply.erb (limited to 'actionpack') diff --git a/actionpack/test/activerecord/render_partial_with_record_identification_test.rb b/actionpack/test/activerecord/render_partial_with_record_identification_test.rb index af2725a99b..a9d3ff040f 100644 --- a/actionpack/test/activerecord/render_partial_with_record_identification_test.rb +++ b/actionpack/test/activerecord/render_partial_with_record_identification_test.rb @@ -56,26 +56,31 @@ class RenderPartialWithRecordIdentificationTest < ActiveRecordTestCase def test_rendering_partial_with_has_many_and_belongs_to_association get :render_with_has_many_and_belongs_to_association assert_template 'projects/_project' + assert_equal 'Active RecordActive Controller', @response.body end def test_rendering_partial_with_has_many_association get :render_with_has_many_association assert_template 'replies/_reply' + assert_equal 'Birdman is better!', @response.body end def test_rendering_partial_with_named_scope get :render_with_named_scope assert_template 'replies/_reply' + assert_equal 'Birdman is better!Nuh uh!', @response.body end def test_render_with_record get :render_with_record assert_template 'developers/_developer' + assert_equal 'David', @response.body end def test_render_with_record_collection get :render_with_record_collection assert_template 'developers/_developer' + assert_equal 'DavidJamisfixture_3fixture_4fixture_5fixture_6fixture_7fixture_8fixture_9fixture_10Jamis', @response.body end def test_rendering_partial_with_has_one_association @@ -165,11 +170,13 @@ class RenderPartialWithRecordIdentificationAndNestedControllersTest < ActiveReco def test_render_with_record_in_nested_controller get :render_with_record_in_nested_controller assert_template 'fun/games/_game' + assert_equal 'Pong', @response.body end def test_render_with_record_collection_in_nested_controller get :render_with_record_collection_in_nested_controller assert_template 'fun/games/_game' + assert_equal 'PongTank', @response.body end end @@ -184,10 +191,12 @@ class RenderPartialWithRecordIdentificationAndNestedDeeperControllersTest < Acti def test_render_with_record_in_deeper_nested_controller get :render_with_record_in_deeper_nested_controller assert_template 'fun/serious/games/_game' + assert_equal 'Chess', @response.body end def test_render_with_record_collection_in_deeper_nested_controller get :render_with_record_collection_in_deeper_nested_controller assert_template 'fun/serious/games/_game' + assert_equal 'ChessSudokuSolitaire', @response.body end end diff --git a/actionpack/test/fixtures/developers/_developer.erb b/actionpack/test/fixtures/developers/_developer.erb new file mode 100644 index 0000000000..904a3137e7 --- /dev/null +++ b/actionpack/test/fixtures/developers/_developer.erb @@ -0,0 +1 @@ +<%= developer.name %> \ No newline at end of file diff --git a/actionpack/test/fixtures/fun/games/_game.erb b/actionpack/test/fixtures/fun/games/_game.erb new file mode 100644 index 0000000000..d51b7b3ebc --- /dev/null +++ b/actionpack/test/fixtures/fun/games/_game.erb @@ -0,0 +1 @@ +<%= game.name %> \ No newline at end of file diff --git a/actionpack/test/fixtures/fun/serious/games/_game.erb b/actionpack/test/fixtures/fun/serious/games/_game.erb new file mode 100644 index 0000000000..d51b7b3ebc --- /dev/null +++ b/actionpack/test/fixtures/fun/serious/games/_game.erb @@ -0,0 +1 @@ +<%= game.name %> \ No newline at end of file diff --git a/actionpack/test/fixtures/projects/_project.erb b/actionpack/test/fixtures/projects/_project.erb new file mode 100644 index 0000000000..480c4c2af3 --- /dev/null +++ b/actionpack/test/fixtures/projects/_project.erb @@ -0,0 +1 @@ +<%= project.name %> \ No newline at end of file diff --git a/actionpack/test/fixtures/replies/_reply.erb b/actionpack/test/fixtures/replies/_reply.erb new file mode 100644 index 0000000000..68baf548d8 --- /dev/null +++ b/actionpack/test/fixtures/replies/_reply.erb @@ -0,0 +1 @@ +<%= reply.content %> \ No newline at end of file -- cgit v1.2.3 From 65fb2e76f2c4571b04458c7bf6a0c815972232ab Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 12 Jul 2008 12:16:05 -0500 Subject: Removed a few implementation specific view path tests --- actionpack/test/controller/view_paths_test.rb | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/view_paths_test.rb b/actionpack/test/controller/view_paths_test.rb index 9401c87d10..b1c19384aa 100644 --- a/actionpack/test/controller/view_paths_test.rb +++ b/actionpack/test/controller/view_paths_test.rb @@ -146,18 +146,4 @@ class ViewLoadPathsTest < Test::Unit::TestCase assert_nothing_raised { C.view_paths << 'c/path' } assert_equal ['c/path'], C.view_paths end - - def test_find_template_file_for_path - assert_equal "test/hello_world.erb", @controller.view_paths.find_template_file_for_path("test/hello_world.erb").to_s - assert_equal "test/hello.builder", @controller.view_paths.find_template_file_for_path("test/hello.builder").to_s - assert_equal nil, @controller.view_paths.find_template_file_for_path("test/missing.erb") - end - - def test_view_paths_find_template_file_for_path - assert_equal "test/formatted_html_erb.html.erb", @controller.view_paths.find_template_file_for_path("test/formatted_html_erb.html").to_s - assert_equal "test/formatted_xml_erb.xml.erb", @controller.view_paths.find_template_file_for_path("test/formatted_xml_erb.xml").to_s - assert_equal "test/hello_world.erb", @controller.view_paths.find_template_file_for_path("test/hello_world.html").to_s - assert_equal "test/hello_world.erb", @controller.view_paths.find_template_file_for_path("test/hello_world.xml").to_s - assert_equal nil, @controller.view_paths.find_template_file_for_path("test/missing.html") - end end -- cgit v1.2.3 From 30204c4e66cea989c4ee48b52c8827c79e98f14a Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 12 Jul 2008 14:11:51 -0500 Subject: Set global ActionController::Base.view_paths for test cases --- actionpack/test/abstract_unit.rb | 4 +++- .../render_partial_with_record_identification_test.rb | 8 -------- actionpack/test/controller/action_pack_assertions_test.rb | 8 -------- actionpack/test/controller/addresses_render_test.rb | 2 -- actionpack/test/controller/caching_test.rb | 3 --- actionpack/test/controller/capture_test.rb | 2 -- actionpack/test/controller/content_type_test.rb | 2 -- .../test/controller/deprecation/deprecated_base_methods_test.rb | 2 -- actionpack/test/controller/mime_responds_test.rb | 2 -- actionpack/test/controller/new_render_test.rb | 3 --- actionpack/test/controller/render_test.rb | 3 --- actionpack/test/controller/send_file_test.rb | 2 -- actionpack/test/controller/view_paths_test.rb | 2 -- actionpack/test/template/render_test.rb | 2 +- actionpack/test/template/url_helper_test.rb | 8 -------- 15 files changed, 4 insertions(+), 49 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 70f6a28a9c..0d2e0f273a 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -22,7 +22,9 @@ ActiveSupport::Deprecation.debug = true ActionController::Base.logger = nil ActionController::Routing::Routes.reload rescue nil -FIXTURE_LOAD_PATH = ActionView::ViewLoadPaths::LoadPath.new(File.join(File.dirname(__FILE__), 'fixtures')) +ActionView::Base.cache_template_loading = true +FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') +ActionController::Base.view_paths = FIXTURE_LOAD_PATH # Wrap tests that use Mocha and skip if unavailable. def uses_mocha(test_name) diff --git a/actionpack/test/activerecord/render_partial_with_record_identification_test.rb b/actionpack/test/activerecord/render_partial_with_record_identification_test.rb index a9d3ff040f..a82a1a3023 100644 --- a/actionpack/test/activerecord/render_partial_with_record_identification_test.rb +++ b/actionpack/test/activerecord/render_partial_with_record_identification_test.rb @@ -41,8 +41,6 @@ class RenderPartialWithRecordIdentificationController < ActionController::Base end end -RenderPartialWithRecordIdentificationController.view_paths = [FIXTURE_LOAD_PATH] - class RenderPartialWithRecordIdentificationTest < ActiveRecordTestCase fixtures :developers, :projects, :developers_projects, :topics, :replies, :companies, :mascots @@ -123,8 +121,6 @@ class RenderPartialWithRecordIdentificationController < ActionController::Base end end -RenderPartialWithRecordIdentificationController.view_paths = [FIXTURE_LOAD_PATH] - class Game < Struct.new(:name, :id) def to_param id.to_s @@ -142,8 +138,6 @@ module Fun end end - NestedController.view_paths = [FIXTURE_LOAD_PATH] - module Serious class NestedDeeperController < ActionController::Base def render_with_record_in_deeper_nested_controller @@ -154,8 +148,6 @@ module Fun render :partial => [ Game.new("Chess"), Game.new("Sudoku"), Game.new("Solitaire") ] end end - - NestedDeeperController.view_paths = [FIXTURE_LOAD_PATH] end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index f5cda99977..610e196362 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -164,14 +164,6 @@ module Admin end end -# --------------------------------------------------------------------------- - - -# tell the controller where to find its templates but start from parent -# directory of test_request_response to simulate the behaviour of a -# production environment -ActionPackAssertionsController.view_paths = [FIXTURE_LOAD_PATH] - # a test case to exercise the new capabilities TestRequest & TestResponse class ActionPackAssertionsControllerTest < Test::Unit::TestCase # let's get this party started diff --git a/actionpack/test/controller/addresses_render_test.rb b/actionpack/test/controller/addresses_render_test.rb index df87182082..b26cae24fb 100644 --- a/actionpack/test/controller/addresses_render_test.rb +++ b/actionpack/test/controller/addresses_render_test.rb @@ -19,8 +19,6 @@ class AddressesTestController < ActionController::Base def self.controller_path; "addresses"; end end -AddressesTestController.view_paths = [FIXTURE_LOAD_PATH] - class AddressesTest < Test::Unit::TestCase def setup @controller = AddressesTestController.new diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 15db70e474..2e98837a35 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -6,7 +6,6 @@ CACHE_DIR = 'test_cache' FILE_STORE_PATH = File.join(File.dirname(__FILE__), '/../temp/', CACHE_DIR) ActionController::Base.page_cache_directory = FILE_STORE_PATH ActionController::Base.cache_store = :file_store, FILE_STORE_PATH -ActionController::Base.view_paths = [FIXTURE_LOAD_PATH] class PageCachingTestController < ActionController::Base caches_page :ok, :no_content, :if => Proc.new { |c| !c.request.format.json? } @@ -636,8 +635,6 @@ class FunctionalCachingController < ActionController::Base end end -FunctionalCachingController.view_paths = [FIXTURE_LOAD_PATH] - class FunctionalFragmentCachingTest < Test::Unit::TestCase def setup ActionController::Base.perform_caching = true diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index 87f9ce8ab3..5ded6a5d26 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -23,8 +23,6 @@ class CaptureController < ActionController::Base def rescue_action(e) raise end end -CaptureController.view_paths = [FIXTURE_LOAD_PATH] - class CaptureTest < Test::Unit::TestCase def setup @controller = CaptureController.new diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index 33aa4e49ee..d457d13aef 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -45,8 +45,6 @@ class ContentTypeController < ActionController::Base def rescue_action(e) raise end end -ContentTypeController.view_paths = [FIXTURE_LOAD_PATH] - class ContentTypeTest < Test::Unit::TestCase def setup @controller = ContentTypeController.new diff --git a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb index f485500b7f..86555a77df 100644 --- a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb +++ b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb @@ -13,8 +13,6 @@ class DeprecatedBaseMethodsTest < Test::Unit::TestCase def rescue_action(e) raise e end end - Target.view_paths = [FIXTURE_LOAD_PATH] - def setup @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 17f5e27232..1701431858 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -162,8 +162,6 @@ class RespondToController < ActionController::Base end end -RespondToController.view_paths = [FIXTURE_LOAD_PATH] - class MimeControllerTest < Test::Unit::TestCase def setup ActionController::Base.use_accept_header = true diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index b4dc2bb4dc..d2a3a2b0b0 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -465,9 +465,6 @@ class NewRenderTestController < ActionController::Base end end -NewRenderTestController.view_paths = [FIXTURE_LOAD_PATH] -Fun::GamesController.view_paths = [FIXTURE_LOAD_PATH] - class NewRenderTest < Test::Unit::TestCase def setup @controller = NewRenderTestController.new diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 10264dadaa..a857810b78 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -217,9 +217,6 @@ class TestController < ActionController::Base end end -TestController.view_paths = [FIXTURE_LOAD_PATH] -Fun::GamesController.view_paths = [FIXTURE_LOAD_PATH] - class RenderTest < Test::Unit::TestCase def setup @request = ActionController::TestRequest.new diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index ddec51d173..c003abf094 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -19,8 +19,6 @@ class SendFileController < ActionController::Base def rescue_action(e) raise end end -SendFileController.view_paths = [FIXTURE_LOAD_PATH] - class SendFileTest < Test::Unit::TestCase include TestFileUtils diff --git a/actionpack/test/controller/view_paths_test.rb b/actionpack/test/controller/view_paths_test.rb index b1c19384aa..85fa58a45b 100644 --- a/actionpack/test/controller/view_paths_test.rb +++ b/actionpack/test/controller/view_paths_test.rb @@ -1,8 +1,6 @@ require 'abstract_unit' class ViewLoadPathsTest < Test::Unit::TestCase - ActionController::Base.view_paths = [FIXTURE_LOAD_PATH] - class TestController < ActionController::Base def self.controller_path() "test" end def rescue_action(e) raise end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index cd004a9f6d..cc5b4900dc 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -4,7 +4,7 @@ require 'controller/fake_models' class ViewRenderTest < Test::Unit::TestCase def setup @assigns = { :secret => 'in the sauce' } - @view = ActionView::Base.new([FIXTURE_LOAD_PATH], @assigns) + @view = ActionView::Base.new(ActionController::Base.view_paths, @assigns) end def test_render_file diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 4ed5bc372f..91d5c6ffb5 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -302,8 +302,6 @@ end class UrlHelperWithControllerTest < ActionView::TestCase class UrlHelperController < ActionController::Base - self.view_paths = [FIXTURE_LOAD_PATH] - def self.controller_path; 'url_helper_with_controller' end def show_url_for @@ -366,8 +364,6 @@ end class LinkToUnlessCurrentWithControllerTest < ActionView::TestCase class TasksController < ActionController::Base - self.view_paths = [FIXTURE_LOAD_PATH] - def self.controller_path; 'tasks' end def index @@ -458,8 +454,6 @@ end class PolymorphicControllerTest < ActionView::TestCase class WorkshopsController < ActionController::Base - self.view_paths = [FIXTURE_LOAD_PATH] - def self.controller_path; 'workshops' end def index @@ -476,8 +470,6 @@ class PolymorphicControllerTest < ActionView::TestCase end class SessionsController < ActionController::Base - self.view_paths = [FIXTURE_LOAD_PATH] - def self.controller_path; 'sessions' end def index -- cgit v1.2.3 From 73b34e9f75d33dc0709d4ad36c912bdbb8977994 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 12 Jul 2008 14:33:46 -0500 Subject: Refactor template preloading. New abstractions include Renderable mixins and a refactored Template class. --- actionpack/CHANGELOG | 2 + .../assertions/response_assertions.rb | 2 +- actionpack/lib/action_controller/base.rb | 9 +- actionpack/lib/action_controller/layout.rb | 2 +- actionpack/lib/action_controller/test_process.rb | 10 +- actionpack/lib/action_view.rb | 8 +- actionpack/lib/action_view/base.rb | 79 ++++++------ actionpack/lib/action_view/inline_template.rb | 13 +- actionpack/lib/action_view/partial_template.rb | 69 ----------- actionpack/lib/action_view/partials.rb | 58 +++++---- actionpack/lib/action_view/paths.rb | 85 +++++++++++++ actionpack/lib/action_view/renderable.rb | 81 +++++++++--- actionpack/lib/action_view/renderable_partial.rb | 19 +++ actionpack/lib/action_view/template.rb | 138 +++++++++++++-------- actionpack/lib/action_view/template_error.rb | 2 +- actionpack/lib/action_view/template_file.rb | 88 ------------- .../action_view/template_handlers/compilable.rb | 50 -------- actionpack/lib/action_view/view_load_paths.rb | 103 --------------- actionpack/test/controller/layout_test.rb | 1 + 19 files changed, 354 insertions(+), 465 deletions(-) delete mode 100644 actionpack/lib/action_view/partial_template.rb create mode 100644 actionpack/lib/action_view/paths.rb create mode 100644 actionpack/lib/action_view/renderable_partial.rb delete mode 100644 actionpack/lib/action_view/template_file.rb delete mode 100644 actionpack/lib/action_view/view_load_paths.rb (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index dc508f701a..507a2b69bf 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Refactor template preloading. New abstractions include Renderable mixins and a refactored Template class [Josh Peek] + * Changed ActionView::TemplateHandler#render API method signature to render(template, local_assigns = {}) [Josh Peek] * Changed PrototypeHelper#submit_to_remote to PrototypeHelper#button_to_remote to stay consistent with link_to_remote (submit_to_remote still works as an alias) #8994 [clemens] diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index 64aaf6211f..cb36405700 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -93,7 +93,7 @@ module ActionController if expected.nil? !@response.rendered_with_file? else - expected == rendered + rendered.match(expected) end end end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 9d9ff527c2..6926941396 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -431,7 +431,7 @@ module ActionController #:nodoc: end def view_paths=(value) - @view_paths = ActionView::ViewLoadPaths.new(Array(value)) if value + @view_paths = ActionView::PathSet.new(Array(value)) if value end # Adds a view_path to the front of the view_paths array. @@ -652,7 +652,7 @@ module ActionController #:nodoc: end def view_paths=(value) - @template.view_paths = ViewLoadPaths.new(value) + @template.view_paths = PathSet.new(value) end # Adds a view_path to the front of the view_paths array. @@ -1248,9 +1248,8 @@ module ActionController #:nodoc: end def template_exempt_from_layout?(template_name = default_template_name) - extension = @template && @template.pick_template_extension(template_name) - name_with_extension = !template_name.include?('.') && extension ? "#{template_name}.#{extension}" : template_name - @@exempt_from_layout.any? { |ext| name_with_extension =~ ext } + template_name = @template.pick_template(template_name).to_s if @template + @@exempt_from_layout.any? { |ext| template_name =~ ext } end def default_template_name(action_name = self.action_name) diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb index d0c717ff67..8b6febe254 100644 --- a/actionpack/lib/action_controller/layout.rb +++ b/actionpack/lib/action_controller/layout.rb @@ -304,7 +304,7 @@ module ActionController #:nodoc: end def layout_directory?(layout_name) - @template.view_paths.find_template_file_for_path("#{File.join('layouts', layout_name)}.#{@template.template_format}.erb") ? true : false + @template.file_exists?("#{File.join('layouts', layout_name)}.#{@template.template_format}") end end end diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 8ae73a66f4..52884a93f4 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -207,13 +207,9 @@ module ActionController #:nodoc: # Returns the template path of the file which was used to # render this response (or nil) - def rendered_file(with_controller=false) - unless template.first_render.nil? - unless with_controller - template.first_render - else - template.first_render.split('/').last || template.first_render - end + def rendered_file(with_controller = false) + if template.first_render + template.first_render.to_s end end diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 8f928d6fdf..9ab615c7a5 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -21,14 +21,14 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #++ -require 'action_view/template_handlers' -require 'action_view/template_file' -require 'action_view/view_load_paths' +require 'action_view/template_handlers' require 'action_view/renderable' +require 'action_view/renderable_partial' + require 'action_view/template' -require 'action_view/partial_template' require 'action_view/inline_template' +require 'action_view/paths' require 'action_view/base' require 'action_view/partials' diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 7ef90ddf5e..9d4d897dbc 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -3,6 +3,12 @@ module ActionView #:nodoc: end class MissingTemplate < ActionViewError #:nodoc: + def initialize(paths, path, template_format = nil) + full_template_path = path.include?('.') ? path : "#{path}.erb" + display_paths = paths.join(':') + template_type = (path =~ /layouts/i) ? 'layout' : 'template' + super("Missing #{template_type} #{full_template_path} in view path #{display_paths}") + end end # Action View templates can be written in three ways. If the template file has a .erb (or .rhtml) extension then it uses a mixture of ERb @@ -216,12 +222,14 @@ module ActionView #:nodoc: attr_reader :view_paths def view_paths=(paths) - @view_paths = ViewLoadPaths.new(Array(paths)) + @view_paths = PathSet.new(Array(paths)) end # Renders the template present at template_path (relative to the view_paths array). # The hash in local_assigns is made available as local variables. def render(options = {}, local_assigns = {}, &block) #:nodoc: + local_assigns ||= {} + if options.is_a?(String) render_file(options, nil, local_assigns) elsif options == :update @@ -270,21 +278,40 @@ module ActionView #:nodoc: end def file_exists?(template_path) - view_paths.template_exists?(template_file_from_name(template_path)) + pick_template(template_path) ? true : false + rescue MissingTemplate + false end # Gets the extension for an existing template with the given template_path. # Returns the format with the extension if that template exists. # - # pick_template_extension('users/show') - # # => 'html.erb' + # pick_template('users/show') + # # => 'users/show.html.erb' # - # pick_template_extension('users/legacy') - # # => "rhtml" + # pick_template('users/legacy') + # # => 'users/legacy.rhtml' # - def pick_template_extension(template_path) - if template = template_file_from_name(template_path) - template.extension + def pick_template(template_path) + path = template_path.sub(/^\//, '') + if m = path.match(/(.*)\.(\w+)$/) + template_file_name, template_file_extension = m[1], m[2] + else + template_file_name = path + end + + # OPTIMIZE: Checks to lookup template in view path + if template = self.view_paths["#{template_file_name}.#{template_format}"] + template + elsif template = self.view_paths[template_file_name] + template + elsif first_render && template = self.view_paths["#{template_file_name}.#{first_render.extension}"] + template + elsif template_format == :js && template = self.view_paths["#{template_file_name}.html"] + @template_format = :html + template + else + Template.new(template_path, view_paths) end end @@ -292,6 +319,10 @@ module ActionView #:nodoc: # Renders the template present at template_path. The hash in local_assigns # is made available as local variables. def render_file(template_path, use_full_path = nil, local_assigns = {}) #:nodoc: + unless use_full_path == nil + ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) + end + if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && !template_path.include?("/") raise ActionViewError, <<-END_ERROR Due to changes in ActionMailer, you need to provide the mailer_name along with the template name. @@ -305,11 +336,12 @@ module ActionView #:nodoc: END_ERROR end - Template.new(self, template_path, use_full_path, local_assigns).render_template + template = pick_template(template_path) + template.render_template(self, local_assigns) end def render_inline(text, local_assigns = {}, type = nil) - InlineTemplate.new(self, text, local_assigns, type).render + InlineTemplate.new(text, type).render(self, local_assigns) end def wrap_content_for_layout(content) @@ -333,32 +365,9 @@ module ActionView #:nodoc: end def execute(template, local_assigns = {}) - send(template.method, local_assigns) do |*names| + send(template.method(local_assigns), local_assigns) do |*names| instance_variable_get "@content_for_#{names.first || 'layout'}" end end - - def template_file_from_name(template_name) - template_name = TemplateFile.from_path(template_name) - pick_template(template_name) unless template_name.extension - end - - def pick_template(file) - if f = self.view_paths.find_template_file_for_path(file.dup_with_extension(template_format)) || file_from_first_render(file) - f - elsif template_format == :js && f = self.view_paths.find_template_file_for_path(file.dup_with_extension(:html)) - @template_format = :html - f - else - nil - end - end - - # Determine the template extension from the @first_render filename - def file_from_first_render(file) - if extension = File.basename(@first_render.to_s)[/^[^.]+\.(.+)$/, 1] - file.dup_with_extension(extension) - end - end end end diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 3e25f6902d..5e00cef13f 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -2,15 +2,18 @@ module ActionView #:nodoc: class InlineTemplate #:nodoc: include Renderable - def initialize(view, source, locals = {}, type = nil) - @view = view + attr_reader :source, :extension, :method_segment + def initialize(source, type = nil) @source = source @extension = type - @locals = locals || {} - @method_segment = "inline_#{@source.hash.abs}" - @handler = Template.handler_class_for_extension(@extension).new(@view) end + + private + # Always recompile inline templates + def recompile?(local_assigns) + true + end end end diff --git a/actionpack/lib/action_view/partial_template.rb b/actionpack/lib/action_view/partial_template.rb deleted file mode 100644 index 72f831e937..0000000000 --- a/actionpack/lib/action_view/partial_template.rb +++ /dev/null @@ -1,69 +0,0 @@ -module ActionView #:nodoc: - class PartialTemplate < Template #:nodoc: - attr_reader :variable_name, :object, :as - - def initialize(view, partial_path, object = nil, locals = {}, as = nil) - @view_controller = view.controller if view.respond_to?(:controller) - @as = as - set_path_and_variable_name!(partial_path) - super(view, @path, nil, locals) - add_object_to_local_assigns!(object) - - # This is needed here in order to compile template with knowledge of 'counter' - initialize_counter! - - # Prepare early. This is a performance optimization for partial collections - prepare! - end - - def render - ActionController::Base.benchmark("Rendered #{@path.path_without_format_and_extension}", Logger::DEBUG, false) do - super - end - end - - def render_member(object) - @locals[:object] = @locals[@variable_name] = object - @locals[as] = object if as - - template = render_template - @locals[@counter_name] += 1 - @locals.delete(as) - @locals.delete(@variable_name) - @locals.delete(:object) - - template - end - - def counter=(num) - @locals[@counter_name] = num - end - - private - def add_object_to_local_assigns!(object) - @locals[:object] ||= - @locals[@variable_name] ||= object || @view_controller.instance_variable_get("@#{variable_name}") - @locals[as] ||= @locals[:object] if as - end - - def set_path_and_variable_name!(partial_path) - if partial_path.include?('/') - @variable_name = File.basename(partial_path) - @path = "#{File.dirname(partial_path)}/_#{@variable_name}" - elsif @view_controller - @variable_name = partial_path - @path = "#{@view_controller.class.controller_path}/_#{@variable_name}" - else - @variable_name = partial_path - @path = "_#{@variable_name}" - end - - @variable_name = @variable_name.sub(/\..*$/, '').to_sym - end - - def initialize_counter! - @counter_name ||= "#{@variable_name}_counter".to_sym - @locals[@counter_name] = 0 - end - end -end diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 7c6c98d611..116d61e13b 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -104,10 +104,12 @@ module ActionView module Partials private def render_partial(partial_path, object_assigns = nil, local_assigns = {}) #:nodoc: + local_assigns ||= {} + case partial_path when String, Symbol, NilClass - # Render the template - ActionView::PartialTemplate.new(self, partial_path, object_assigns, local_assigns).render_template + variable_name, path = partial_pieces(partial_path) + pick_template(path).render_partial(self, variable_name, object_assigns, local_assigns) when ActionView::Helpers::FormBuilder builder_partial_path = partial_path.class.to_s.demodulize.underscore.sub(/_builder$/, '') render_partial(builder_partial_path, object_assigns, (local_assigns || {}).merge(builder_partial_path.to_sym => partial_path)) @@ -128,31 +130,43 @@ module ActionView local_assigns = local_assigns ? local_assigns.clone : {} spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : '' + _partial_pieces = {} + _templates = {} - if partial_path.nil? - render_partial_collection_with_unknown_partial_path(collection, local_assigns, as) - else - render_partial_collection_with_known_partial_path(collection, partial_path, local_assigns, as) - end.join(spacer) - end + index = 0 + collection.map do |object| + _partial_path ||= partial_path || ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) + variable_name, path = _partial_pieces[_partial_path] ||= partial_pieces(_partial_path) + template = _templates[path] ||= pick_template(path) - def render_partial_collection_with_known_partial_path(collection, partial_path, local_assigns, as) - template = ActionView::PartialTemplate.new(self, partial_path, nil, local_assigns, as) - collection.map do |element| - template.render_member(element) - end + local_assigns["#{variable_name}_counter".to_sym] = index + local_assigns[:object] = local_assigns[variable_name] = object + local_assigns[as] = object if as + + result = template.render_partial(self, variable_name, object, local_assigns) + + local_assigns.delete(as) + local_assigns.delete(variable_name) + local_assigns.delete(:object) + index += 1 + + result + end.join(spacer) end - def render_partial_collection_with_unknown_partial_path(collection, local_assigns, as) - templates = Hash.new - i = 0 - collection.map do |element| - partial_path = ActionController::RecordIdentifier.partial_path(element, controller.class.controller_path) - template = templates[partial_path] ||= ActionView::PartialTemplate.new(self, partial_path, nil, local_assigns, as) - template.counter = i - i += 1 - template.render_member(element) + def partial_pieces(partial_path) + if partial_path.include?('/') + variable_name = File.basename(partial_path) + path = "#{File.dirname(partial_path)}/_#{variable_name}" + elsif respond_to?(:controller) + variable_name = partial_path + path = "#{controller.class.controller_path}/_#{variable_name}" + else + variable_name = partial_path + path = "_#{variable_name}" end + variable_name = variable_name.sub(/\..*$/, '').to_sym + return variable_name, path end end end diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb new file mode 100644 index 0000000000..0fa7d3dad3 --- /dev/null +++ b/actionpack/lib/action_view/paths.rb @@ -0,0 +1,85 @@ +module ActionView #:nodoc: + class PathSet < Array #:nodoc: + def self.type_cast(obj) + obj.is_a?(String) ? Path.new(obj) : obj + end + + class Path #:nodoc: + attr_reader :path, :paths + delegate :to_s, :to_str, :inspect, :to => :path + + def initialize(path) + @path = path.freeze + reload! + end + + def ==(path) + to_str == path.to_str + end + + def [](path) + @paths[path] + end + + # Rebuild load path directory cache + def reload! + @paths = {} + + templates_in_path do |template| + @paths[template.path] = template + @paths[template.path_without_extension] ||= template + end + + @paths.freeze + end + + private + def templates_in_path + (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file| + unless File.directory?(file) + template = Template.new(file.split("#{self}/").last, self) + # Eager load memoized methods and freeze cached template + template.freeze if Base.cache_template_loading + yield template + end + end + end + end + + def initialize(*args) + super(*args).map! { |obj| self.class.type_cast(obj) } + end + + def reload! + each { |path| path.reload! } + end + + def <<(obj) + super(self.class.type_cast(obj)) + end + + def push(*objs) + delete_paths!(objs) + super(*objs.map { |obj| self.class.type_cast(obj) }) + end + + def unshift(*objs) + delete_paths!(objs) + super(*objs.map { |obj| self.class.type_cast(obj) }) + end + + def [](template_path) + each do |path| + if template = path[template_path] + return template + end + end + nil + end + + private + def delete_paths!(paths) + paths.each { |p1| delete_if { |p2| p1.to_s == p2.to_s } } + end + end +end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 6a8a0e44fc..2c4302146f 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -1,37 +1,78 @@ module ActionView module Renderable - # TODO: Local assigns should not be tied to template instance - attr_accessor :locals + # NOTE: The template that this mixin is beening include into is frozen + # So you can not set or modify any instance variables - # TODO: These readers should be private - attr_reader :filename, :source, :handler + def self.included(base) + @@mutex = Mutex.new + end + + # NOTE: Exception to earlier notice. Ensure this is called before freeze + def handler + @handler ||= Template.handler_class_for_extension(extension) + end - def render - prepare! - @handler.render(self, @locals) + # NOTE: Exception to earlier notice. Ensure this is called before freeze + def compiled_source + @compiled_source ||= handler.new(nil).compile(self) if handler.compilable? end - def method - ['_run', @extension, @method_segment, local_assigns_keys].compact.join('_').to_sym + def render(view, local_assigns = {}) + view.first_render ||= self + view.send(:evaluate_assigns) + view.current_render_extension = extension + compile(local_assigns) if handler.compilable? + handler.new(view).render(self, local_assigns) + end + + def method(local_assigns) + if local_assigns && local_assigns.any? + local_assigns_keys = "locals_#{local_assigns.keys.map { |k| k.to_s }.sort.join('_')}" + end + ['_run', extension, method_segment, local_assigns_keys].compact.join('_').to_sym end private - def prepare! - unless @prepared - @view.send(:evaluate_assigns) - @view.current_render_extension = @extension + # Compile and evaluate the template's code + def compile(local_assigns) + render_symbol = method(local_assigns) - if @handler.compilable? - @handler.compile_template(self) # compile the given template, if necessary - end + @@mutex.synchronize do + return false unless recompile?(render_symbol) + + locals_code = local_assigns.keys.map { |key| "#{key} = local_assigns[:#{key}];" }.join + + source = <<-end_src + def #{render_symbol}(local_assigns) + old_output_buffer = output_buffer;#{locals_code};#{compiled_source} + ensure + self.output_buffer = old_output_buffer + end + end_src - @prepared = true + begin + file_name = respond_to?(:filename) ? filename : 'compiled-template' + ActionView::Base::CompiledTemplates.module_eval(source, file_name, 0) + rescue Exception => e # errors from template code + if logger = ActionController::Base.logger + logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" + logger.debug "Function body: #{source}" + logger.debug "Backtrace: #{e.backtrace.join("\n")}" + end + + raise ActionView::TemplateError.new(self, {}, e) + end end end - def local_assigns_keys - if @locals && @locals.any? - "locals_#{@locals.keys.map { |k| k.to_s }.sort.join('_')}" + # Method to check whether template compilation is necessary. + # The template will be compiled if the file has not been compiled yet, or + # if local_assigns has a new key, which isn't supported by the compiled code yet. + def recompile?(symbol) + unless Base::CompiledTemplates.instance_methods.include?(symbol) && Base.cache_template_loading + true + else + false end end end diff --git a/actionpack/lib/action_view/renderable_partial.rb b/actionpack/lib/action_view/renderable_partial.rb new file mode 100644 index 0000000000..6a17b50a14 --- /dev/null +++ b/actionpack/lib/action_view/renderable_partial.rb @@ -0,0 +1,19 @@ +module ActionView + module RenderablePartial + # NOTE: The template that this mixin is beening include into is frozen + # So you can not set or modify any instance variables + + def render(view, local_assigns = {}) + ActionController::Base.benchmark("Rendered #{path_without_format_and_extension}", Logger::DEBUG, false) do + super + end + end + + def render_partial(view, variable_name, object = nil, local_assigns = {}, as = nil) + object ||= view.controller.instance_variable_get("@#{variable_name}") if view.respond_to?(:controller) + local_assigns[:object] ||= local_assigns[variable_name] ||= object + local_assigns[as] ||= local_assigns[:object] if as + render_template(view, local_assigns) + end + end +end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 5e5ea9b9b9..03f9234289 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,82 +1,112 @@ module ActionView #:nodoc: - class Template #:nodoc: + class Template extend TemplateHandlers include Renderable - attr_reader :path, :extension + attr_accessor :filename, :load_path, :base_path, :name, :format, :extension + delegate :to_s, :to => :path - def initialize(view, path, use_full_path = nil, locals = {}) - unless use_full_path == nil - ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) - end + def initialize(template_path, load_paths = []) + template_path = template_path.dup + @base_path, @name, @format, @extension = split(template_path) + @base_path.to_s.gsub!(/\/$/, '') # Push to split method + @load_path, @filename = find_full_path(template_path, load_paths) + + # Extend with partial super powers + extend RenderablePartial if @name =~ /^_/ + end + + def freeze + # Eager load memoized methods + format_and_extension + path + path_without_extension + path_without_format_and_extension + source + method_segment + + # Eager load memoized methods from Renderable + handler + compiled_source + + instance_variables.each { |ivar| ivar.freeze } + + super + end - @view = view - @paths = view.view_paths + def format_and_extension + @format_and_extension ||= (extensions = [format, extension].compact.join(".")).blank? ? nil : extensions + end + + def path + @path ||= [base_path, [name, format, extension].compact.join('.')].compact.join('/') + end - @original_path = path - @path = TemplateFile.from_path(path) - @view.first_render ||= @path.to_s + def path_without_extension + @path_without_extension ||= [base_path, [name, format].compact.join('.')].compact.join('/') + end - set_extension_and_file_name + def path_without_format_and_extension + @path_without_format_and_extension ||= [base_path, name].compact.join('/') + end - @method_segment = compiled_method_name_file_path_segment - @locals = (locals && locals.dup) || {} - @handler = self.class.handler_class_for_extension(@extension).new(@view) + def source + @source ||= File.read(@filename) end - def render_template - render + def method_segment + unless @method_segment + segment = File.expand_path(@filename) + segment.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}/, '') if defined?(RAILS_ROOT) + segment.gsub!(/([^a-zA-Z0-9_])/) { $1.ord } + @method_segment = segment + end + + @method_segment + end + + def render_template(view, local_assigns = {}) + render(view, local_assigns) rescue Exception => e raise e unless filename if TemplateError === e e.sub_template_of(filename) raise e else - raise TemplateError.new(self, @view.assigns, e) + raise TemplateError.new(self, view.assigns, e) end end - def source - @source ||= File.read(@filename) - end - - def base_path_for_exception - (@paths.find_load_path_for_path(@path) || @paths.first).to_s - end - private - def set_extension_and_file_name - @extension = @path.extension - - unless @extension - @path = @view.send(:template_file_from_name, @path) - raise_missing_template_exception unless @path - @extension = @path.extension - end - - if p = @paths.find_template_file_for_path(path) - @path = p - @filename = @path.full_path - @extension = @path.extension - raise_missing_template_exception if @filename.blank? - else - @filename = @original_path - raise_missing_template_exception unless File.exist?(@filename) - end + def valid_extension?(extension) + Template.template_handler_extensions.include?(extension) end - def raise_missing_template_exception - full_template_path = @original_path.include?('.') ? @original_path : "#{@original_path}.#{@view.template_format}.erb" - display_paths = @paths.join(':') - template_type = (@original_path =~ /layouts/i) ? 'layout' : 'template' - raise MissingTemplate, "Missing #{template_type} #{full_template_path} in view path #{display_paths}" + def find_full_path(path, load_paths) + load_paths = Array(load_paths) + [nil] + load_paths.each do |load_path| + file = [load_path, path].compact.join('/') + return load_path, file if File.exist?(file) + end + raise MissingTemplate.new(load_paths, path) end - def compiled_method_name_file_path_segment - s = File.expand_path(@filename) - s.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}/, '') if defined?(RAILS_ROOT) - s.gsub!(/([^a-zA-Z0-9_])/) { $1.ord } - s + # Returns file split into an array + # [base_path, name, format, extension] + def split(file) + if m = file.match(/^(.*\/)?([^\.]+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) + if m[5] # Mulipart formats + [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]] + elsif m[4] # Single format + [m[1], m[2], m[3], m[4]] + else + if valid_extension?(m[3]) # No format + [m[1], m[2], nil, m[3]] + else # No extension + [m[1], m[2], m[3], nil] + end + end + end end end end diff --git a/actionpack/lib/action_view/template_error.rb b/actionpack/lib/action_view/template_error.rb index a1be1a8833..35fc07bdb2 100644 --- a/actionpack/lib/action_view/template_error.rb +++ b/actionpack/lib/action_view/template_error.rb @@ -7,7 +7,7 @@ module ActionView attr_reader :original_exception def initialize(template, assigns, original_exception) - @base_path = template.base_path_for_exception + @base_path = template.base_path @assigns, @source, @original_exception = assigns.dup, template.source, original_exception @file_path = template.filename @backtrace = compute_backtrace diff --git a/actionpack/lib/action_view/template_file.rb b/actionpack/lib/action_view/template_file.rb deleted file mode 100644 index 0aa16b5e70..0000000000 --- a/actionpack/lib/action_view/template_file.rb +++ /dev/null @@ -1,88 +0,0 @@ -module ActionView #:nodoc: - # TemplateFile abstracts the pattern of querying a file path for its - # path with or without its extension. The path is only the partial path - # from the load path root e.g. "hello/index.html.erb" not - # "app/views/hello/index.html.erb" - class TemplateFile - def self.from_path(path) - path.is_a?(self) ? path : new(path) - end - - def self.from_full_path(load_path, full_path) - file = new(full_path.split(load_path).last) - file.load_path = load_path - file.freeze - end - - attr_accessor :load_path, :base_path, :name, :format, :extension - delegate :to_s, :inspect, :to => :path - - def initialize(path) - path = path.dup - - # Clear the forward slash in the beginning - trim_forward_slash!(path) - - @base_path, @name, @format, @extension = split(path) - end - - def freeze - @load_path.freeze - @base_path.freeze - @name.freeze - @format.freeze - @extension.freeze - super - end - - def format_and_extension - extensions = [format, extension].compact.join(".") - extensions.blank? ? nil : extensions - end - - def full_path - if load_path - "#{load_path}/#{path}" - else - path - end - end - - def path - base_path.to_s + [name, format, extension].compact.join(".") - end - - def path_without_extension - base_path.to_s + [name, format].compact.join(".") - end - - def path_without_format_and_extension - "#{base_path}#{name}" - end - - def dup_with_extension(extension) - file = dup - file.extension = extension ? extension.to_s : nil - file - end - - private - def trim_forward_slash!(path) - path.sub!(/^\//, '') - end - - # Returns file split into an array - # [base_path, name, format, extension] - def split(file) - if m = file.match(/^(.*\/)?([^\.]+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) - if m[5] # Mulipart formats - [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]] - elsif m[4] # Single format - [m[1], m[2], m[3], m[4]] - else # No format - [m[1], m[2], nil, m[3]] - end - end - end - end -end diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index 95ae75ca68..a0ebaefeef 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -3,8 +3,6 @@ module ActionView module Compilable def self.included(base) base.extend ClassMethod - - @@mutex = Mutex.new end module ClassMethod @@ -17,54 +15,6 @@ module ActionView def render(template, local_assigns = {}) @view.send(:execute, template, local_assigns) end - - # Compile and evaluate the template's code - def compile_template(template) - return false unless recompile_template?(template) - - @@mutex.synchronize do - locals_code = template.locals.keys.map { |key| "#{key} = local_assigns[:#{key}];" }.join - - source = <<-end_src - def #{template.method}(local_assigns) - old_output_buffer = output_buffer;#{locals_code};#{compile(template)} - ensure - self.output_buffer = old_output_buffer - end - end_src - - begin - file_name = template.filename || 'compiled-template' - ActionView::Base::CompiledTemplates.module_eval(source, file_name, 0) - rescue Exception => e # errors from template code - if logger = ActionController::Base.logger - logger.debug "ERROR: compiling #{template.method} RAISED #{e}" - logger.debug "Function body: #{source}" - logger.debug "Backtrace: #{e.backtrace.join("\n")}" - end - - raise ActionView::TemplateError.new(template, @view.assigns, e) - end - end - end - - private - # Method to check whether template compilation is necessary. - # The template will be compiled if the inline template or file has not been compiled yet, - # if local_assigns has a new key, which isn't supported by the compiled code yet. - def recompile_template?(template) - # Unless the template has been complied yet, compile - return true unless Base::CompiledTemplates.instance_methods.include?(template.method.to_s) - - # If template caching is disabled, compile - return true unless Base.cache_template_loading - - # Always recompile inline templates - return true if template.is_a?(InlineTemplate) - - # Otherwise, use compiled method - return false - end end end end diff --git a/actionpack/lib/action_view/view_load_paths.rb b/actionpack/lib/action_view/view_load_paths.rb deleted file mode 100644 index 6e439a009c..0000000000 --- a/actionpack/lib/action_view/view_load_paths.rb +++ /dev/null @@ -1,103 +0,0 @@ -module ActionView #:nodoc: - class ViewLoadPaths < Array #:nodoc: - def self.type_cast(obj) - obj.is_a?(String) ? LoadPath.new(obj) : obj - end - - class LoadPath #:nodoc: - attr_reader :path, :paths - delegate :to_s, :to_str, :inspect, :to => :path - - def initialize(path) - @path = path.freeze - reload! - end - - def ==(path) - to_str == path.to_str - end - - # Rebuild load path directory cache - def reload! - @paths = {} - - files.each do |file| - @paths[file.path] = file - @paths[file.path_without_extension] ||= file - end - - @paths.freeze - end - - def find_template_file_for_partial_path(template_path, template_format) - @paths["#{template_path}.#{template_format}"] || - @paths[template_path] || - @paths[template_path.gsub(/\..*$/, '')] - end - - private - # Get all the files and directories in the path - def files_in_path - Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**") - end - - # Create an array of all the files within the path - def files - files_in_path.map do |file| - TemplateFile.from_full_path(@path, file) unless File.directory?(file) - end.compact - end - end - - def initialize(*args) - super(*args).map! { |obj| self.class.type_cast(obj) } - end - - def reload! - each { |path| path.reload! } - end - - def <<(obj) - super(self.class.type_cast(obj)) - end - - def push(*objs) - delete_paths!(objs) - super(*objs.map { |obj| self.class.type_cast(obj) }) - end - - def unshift(*objs) - delete_paths!(objs) - super(*objs.map { |obj| self.class.type_cast(obj) }) - end - - def template_exists?(file) - find_load_path_for_path(file) ? true : false - end - - def find_load_path_for_path(file) - find { |path| path.paths[file.to_s] } - end - - def find_template_file_for_path(template_path) - template_path_without_extension, template_extension = path_and_extension(template_path.to_s) - each do |path| - if f = path.find_template_file_for_partial_path(template_path_without_extension, template_extension) - return f - end - end - nil - end - - private - def delete_paths!(paths) - paths.each { |p1| delete_if { |p2| p1.to_s == p2.to_s } } - end - - # Splits the path and extension from the given template_path and returns as an array. - def path_and_extension(template_path) - template_path_without_extension = template_path.sub(/\.(\w+)$/, '') - [template_path_without_extension, $1] - end - end -end diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 32be7d90ff..92b6aa4f2f 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -63,6 +63,7 @@ class LayoutAutoDiscoveryTest < Test::Unit::TestCase end def test_third_party_template_library_auto_discovers_layout + ThirdPartyTemplateLibraryController.view_paths.reload! @controller = ThirdPartyTemplateLibraryController.new get :hello assert_equal 'layouts/third_party_template_library', @controller.active_layout -- cgit v1.2.3 From 99cc85bc099a757cdd44e4f5f1be4972ab124e0d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 12 Jul 2008 15:31:50 -0500 Subject: Set config.action_view.warn_cache_misses = true to receive a warning if you perform an action that results in an expensive disk operation that could be cached --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/base.rb | 4 ++-- actionpack/lib/action_view/base.rb | 22 ++++++++++++++++++++-- actionpack/lib/action_view/paths.rb | 13 ++++++++++++- 4 files changed, 36 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 507a2b69bf..5b7bfe9c30 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Set config.action_view.warn_cache_misses = true to receive a warning if you perform an action that results in an expensive disk operation that could be cached [Josh Peek] + * Refactor template preloading. New abstractions include Renderable mixins and a refactored Template class [Josh Peek] * Changed ActionView::TemplateHandler#render API method signature to render(template, local_assigns = {}) [Josh Peek] diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 6926941396..df94f78f18 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -431,7 +431,7 @@ module ActionController #:nodoc: end def view_paths=(value) - @view_paths = ActionView::PathSet.new(Array(value)) if value + @view_paths = ActionView::Base.process_view_paths(value) if value end # Adds a view_path to the front of the view_paths array. @@ -652,7 +652,7 @@ module ActionController #:nodoc: end def view_paths=(value) - @template.view_paths = PathSet.new(value) + @template.view_paths = ActionView::Base.process_view_paths(value) end # Adds a view_path to the front of the view_paths array. diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 9d4d897dbc..989e92a890 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -185,6 +185,10 @@ module ActionView #:nodoc: @@debug_rjs = false cattr_accessor :debug_rjs + # A warning will be displayed whenever an action results in a cache miss on your view paths. + @@warn_cache_misses = false + cattr_accessor :warn_cache_misses + attr_internal :request delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, @@ -212,6 +216,10 @@ module ActionView #:nodoc: return helpers end + def self.process_view_paths(value) + ActionView::PathSet.new(Array(value)) + end + def initialize(view_paths = [], assigns_for_first_render = {}, controller = nil)#:nodoc: @assigns = assigns_for_first_render @assigns_added = nil @@ -222,7 +230,7 @@ module ActionView #:nodoc: attr_reader :view_paths def view_paths=(paths) - @view_paths = PathSet.new(Array(paths)) + @view_paths = self.class.process_view_paths(paths) end # Renders the template present at template_path (relative to the view_paths array). @@ -311,7 +319,17 @@ module ActionView #:nodoc: @template_format = :html template else - Template.new(template_path, view_paths) + template = Template.new(template_path, view_paths) + + if self.class.warn_cache_misses && logger = ActionController::Base.logger + logger.debug "[PERFORMANCE] Rendering a template that was " + + "not found in view path. Templates outside the view path are " + + "not cached and result in expensive disk operations. Move this " + + "file into #{view_paths.join(':')} or add the folder to your " + + "view path list" + end + + template end end diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index 0fa7d3dad3..b0ab7d0c67 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -1,7 +1,18 @@ module ActionView #:nodoc: class PathSet < Array #:nodoc: def self.type_cast(obj) - obj.is_a?(String) ? Path.new(obj) : obj + if obj.is_a?(String) + if Base.warn_cache_misses && defined?(Rails) && Rails.initialized? + Rails.logger.debug "[PERFORMANCE] Processing view path during a " + + "request. This an expense disk operation that should be done at " + + "boot. You can manually process this view path with " + + "ActionView::Base.process_view_paths(#{obj.inspect}) and set it " + + "as your view path" + end + Path.new(obj) + else + obj + end end class Path #:nodoc: -- cgit v1.2.3 From e0fef66149092dd3d2988fff64f0ce8765735687 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 13 Jul 2008 13:26:48 -0500 Subject: Made ActionView::Base#first_render a little more private. And added _last_render to track the most recent render. Will fix #609 as a side effect. [#609 state:resolved] --- .../action_controller/assertions/response_assertions.rb | 4 ++-- actionpack/lib/action_controller/test_process.rb | 15 ++++----------- actionpack/lib/action_view/base.rb | 6 +++--- actionpack/lib/action_view/helpers/cache_helper.rb | 3 +-- actionpack/lib/action_view/renderable.rb | 4 ++-- actionpack/test/controller/action_pack_assertions_test.rb | 6 +++--- actionpack/test/controller/caching_test.rb | 8 ++++++++ .../functional_caching/inline_fragment_cached.html.erb | 2 ++ 8 files changed, 25 insertions(+), 23 deletions(-) create mode 100644 actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index cb36405700..a98c70d66f 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -87,11 +87,11 @@ module ActionController # def assert_template(expected = nil, message=nil) clean_backtrace do - rendered = expected ? @response.rendered_file(!expected.include?('/')) : @response.rendered_file + rendered = @response.rendered_template.to_s msg = build_message(message, "expecting but rendering with ", expected, rendered) assert_block(msg) do if expected.nil? - !@response.rendered_with_file? + @response.rendered_template ? true : false else rendered.match(expected) end diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 52884a93f4..a6e0c98936 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -205,17 +205,10 @@ module ActionController #:nodoc: p.match(redirect_url) != nil end - # Returns the template path of the file which was used to - # render this response (or nil) - def rendered_file(with_controller = false) - if template.first_render - template.first_render.to_s - end - end - - # Was this template rendered by a file? - def rendered_with_file? - !rendered_file.nil? + # Returns the template of the file which was used to + # render this response (or nil) + def rendered_template + template._first_render end # A shortcut to the flash. Returns an empyt hash if no session flash exists. diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 989e92a890..9f244d7250 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -159,11 +159,11 @@ module ActionView #:nodoc: class Base include ERB::Util - attr_accessor :base_path, :assigns, :template_extension, :first_render + attr_accessor :base_path, :assigns, :template_extension attr_accessor :controller + attr_accessor :_first_render, :_last_render attr_writer :template_format - attr_accessor :current_render_extension attr_accessor :output_buffer @@ -313,7 +313,7 @@ module ActionView #:nodoc: template elsif template = self.view_paths[template_file_name] template - elsif first_render && template = self.view_paths["#{template_file_name}.#{first_render.extension}"] + elsif _first_render && template = self.view_paths["#{template_file_name}.#{_first_render.extension}"] template elsif template_format == :js && template = self.view_paths["#{template_file_name}.html"] @template_format = :html diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index 930c397785..2cdbae6e40 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -32,8 +32,7 @@ module ActionView # Topics listed alphabetically # <% end %> def cache(name = {}, options = nil, &block) - handler = Template.handler_class_for_extension(current_render_extension.to_sym) - handler.new(@controller).cache_fragment(block, name, options) + _last_render.handler.new(@controller).cache_fragment(block, name, options) end end end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 2c4302146f..ebb0f1b674 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -18,9 +18,9 @@ module ActionView end def render(view, local_assigns = {}) - view.first_render ||= self + view._first_render ||= self + view._last_render = self view.send(:evaluate_assigns) - view.current_render_extension = extension compile(local_assigns) if handler.compilable? handler.new(view).render(self, local_assigns) end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 610e196362..56ba36cee5 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -328,11 +328,11 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase # check if we were rendered by a file-based template? def test_rendered_action process :nothing - assert !@response.rendered_with_file? + assert_nil @response.rendered_template process :hello_world - assert @response.rendered_with_file? - assert 'hello_world', @response.rendered_file + assert @response.rendered_template + assert 'hello_world', @response.rendered_template.to_s end # check the redirection location diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 2e98837a35..c30f7be700 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -664,6 +664,14 @@ CACHED assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial') end + def test_render_inline_before_fragment_caching + get :inline_fragment_cached + assert_response :success + assert_match /Some inline content/, @response.body + assert_match /Some cached content/, @response.body + assert_match "Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached') + end + def test_fragment_caching_in_rjs_partials xhr :get, :js_fragment_cached_with_partial assert_response :success diff --git a/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb b/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb new file mode 100644 index 0000000000..87309b8ccb --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/inline_fragment_cached.html.erb @@ -0,0 +1,2 @@ +<%= render :inline => 'Some inline content' %> +<% cache do %>Some cached content<% end %> -- cgit v1.2.3 From 26bc867151a8f302b4c6122e6375c3ea2088a037 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 13 Jul 2008 14:00:40 -0500 Subject: Small tweak to e0fef66 --- actionpack/lib/action_controller/assertions/response_assertions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index a98c70d66f..765225ae24 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -87,13 +87,13 @@ module ActionController # def assert_template(expected = nil, message=nil) clean_backtrace do - rendered = @response.rendered_template.to_s + rendered = @response.rendered_template msg = build_message(message, "expecting but rendering with ", expected, rendered) assert_block(msg) do if expected.nil? - @response.rendered_template ? true : false + @response.rendered_template.nil? else - rendered.match(expected) + rendered.to_s.match(expected) end end end -- cgit v1.2.3 From 68fe898189a27e4e3c4c2fe005c99975d40e1dd7 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 13 Jul 2008 14:05:21 -0500 Subject: Check first render format and extension. Fixes failing ActionMailer test. --- actionpack/lib/action_view/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 9f244d7250..04e8d3a358 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -313,7 +313,7 @@ module ActionView #:nodoc: template elsif template = self.view_paths[template_file_name] template - elsif _first_render && template = self.view_paths["#{template_file_name}.#{_first_render.extension}"] + elsif _first_render && template = self.view_paths["#{template_file_name}.#{_first_render.format_and_extension}"] template elsif template_format == :js && template = self.view_paths["#{template_file_name}.html"] @template_format = :html -- cgit v1.2.3 From 95812d5eafc3b63ce5eeb0748a5d0132f5108b64 Mon Sep 17 00:00:00 2001 From: rsl Date: Mon, 14 Jul 2008 00:55:57 +0100 Subject: Ensure :index works with fields_for select methods. [#518 state:resolved] Signed-off-by: Pratik Naik --- .../lib/action_view/helpers/form_options_helper.rb | 8 +- .../test/template/form_options_helper_test.rb | 929 ++++++++++++++++++++- 2 files changed, 911 insertions(+), 26 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index ab9e174621..87d49397c6 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -445,19 +445,19 @@ module ActionView class FormBuilder def select(method, choices, options = {}, html_options = {}) - @template.select(@object_name, method, choices, options.merge(:object => @object), html_options) + @template.select(@object_name, method, choices, objectify_options(options), @default_options.merge(html_options)) end def collection_select(method, collection, value_method, text_method, options = {}, html_options = {}) - @template.collection_select(@object_name, method, collection, value_method, text_method, options.merge(:object => @object), html_options) + @template.collection_select(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options)) end def country_select(method, priority_countries = nil, options = {}, html_options = {}) - @template.country_select(@object_name, method, priority_countries, options.merge(:object => @object), html_options) + @template.country_select(@object_name, method, priority_countries, objectify_options(options), @default_options.merge(html_options)) end def time_zone_select(method, priority_zones = nil, options = {}, html_options = {}) - @template.time_zone_select(@object_name, method, priority_zones, options.merge(:object => @object), html_options) + @template.time_zone_select(@object_name, method, priority_zones, objectify_options(options), @default_options.merge(html_options)) end end end diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb index 2496931f4b..9dd43d7b4f 100644 --- a/actionpack/test/template/form_options_helper_test.rb +++ b/actionpack/test/template/form_options_helper_test.rb @@ -231,6 +231,35 @@ uses_mocha "FormOptionsHelperTest" do ) end + def test_select_under_fields_for_with_index + @post = Post.new + @post.category = "" + + fields_for :post, @post, :index => 108 do |f| + concat f.select(:category, %w( abe hest)) + end + + assert_dom_equal( + "", + output_buffer + ) + end + + def test_select_under_fields_for_with_auto_index + @post = Post.new + @post.category = "" + def @post.to_param; 108; end + + fields_for "post[]", @post do |f| + concat f.select(:category, %w( abe hest)) + end + + assert_dom_equal( + "", + output_buffer + ) + end + def test_select_with_blank @post = Post.new @post.category = "" @@ -351,6 +380,47 @@ uses_mocha "FormOptionsHelperTest" do ) end + def test_collection_select_under_fields_for_with_index + @posts = [ + Post.new(" went home", "", "To a little house", "shh!"), + Post.new("Babe went home", "Babe", "To a little house", "shh!"), + Post.new("Cabe went home", "Cabe", "To a little house", "shh!") + ] + + @post = Post.new + @post.author_name = "Babe" + + fields_for :post, @post, :index => 815 do |f| + concat f.collection_select(:author_name, @posts, :author_name, :author_name) + end + + assert_dom_equal( + "", + output_buffer + ) + end + + def test_collection_select_under_fields_for_with_auto_index + @posts = [ + Post.new(" went home", "", "To a little house", "shh!"), + Post.new("Babe went home", "Babe", "To a little house", "shh!"), + Post.new("Cabe went home", "Cabe", "To a little house", "shh!") + ] + + @post = Post.new + @post.author_name = "Babe" + def @post.to_param; 815; end + + fields_for "post[]", @post do |f| + concat f.collection_select(:author_name, @posts, :author_name, :author_name) + end + + assert_dom_equal( + "", + output_buffer + ) + end + def test_collection_select_with_blank_and_style @posts = [ Post.new(" went home", "", "To a little house", "shh!"), @@ -1165,28 +1235,843 @@ uses_mocha "FormOptionsHelperTest" do assert_dom_equal(expected_select[0..-2], country_select("post", "origin", ["New Zealand", "Nicaragua"])) end - def test_time_zone_select - @firm = Firm.new("D") - html = time_zone_select( "firm", "time_zone" ) - assert_dom_equal "", - html - end - - def test_time_zone_select_under_fields_for - @firm = Firm.new("D") - - fields_for :firm, @firm do |f| - concat f.time_zone_select(:time_zone) - end - - assert_dom_equal( - " + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + COUNTRIES + + fields_for :post, @post do |f| + concat f.country_select("origin") + end + + assert_dom_equal(expected_select[0..-2], output_buffer) + end + + def test_country_select_under_fields_for_with_index + @post = Post.new + @post.origin = "United States" + expected_select = <<-COUNTRIES + + COUNTRIES + + fields_for :post, @post, :index => 325 do |f| + concat f.country_select("origin") + end + + assert_dom_equal(expected_select[0..-2], output_buffer) + end + + def test_country_select_under_fields_for_with_auto_index + @post = Post.new + @post.origin = "Iraq" + def @post.to_param; 325; end + + expected_select = <<-COUNTRIES + + COUNTRIES + + fields_for "post[]", @post do |f| + concat f.country_select("origin") + end + + assert_dom_equal(expected_select[0..-2], output_buffer) + end + + def test_time_zone_select + @firm = Firm.new("D") + html = time_zone_select( "firm", "time_zone" ) + assert_dom_equal "", + html + end + + def test_time_zone_select_under_fields_for + @firm = Firm.new("D") + + fields_for :firm, @firm do |f| + concat f.time_zone_select(:time_zone) + end + + assert_dom_equal( + "", + output_buffer + ) + end + + def test_time_zone_select_under_fields_for_with_index + @firm = Firm.new("D") + + fields_for :firm, @firm, :index => 305 do |f| + concat f.time_zone_select(:time_zone) + end + + assert_dom_equal( + "", + output_buffer + ) + end + + def test_time_zone_select_under_fields_for_with_auto_index + @firm = Firm.new("D") + def @firm.to_param; 305; end + + fields_for "firm[]", @firm do |f| + concat f.time_zone_select(:time_zone) + end + + assert_dom_equal( + "