From 3b3790a4351ba7c9d2711089c21f24fcedc11fc0 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 2 Jul 2008 21:38:58 -0500 Subject: Deprecate :use_full_path render option. The supplying the option no longer has an effect. --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/base.rb | 10 +++---- actionpack/lib/action_view/base.rb | 12 ++++---- actionpack/lib/action_view/partial_template.rb | 2 +- actionpack/lib/action_view/template.rb | 39 ++++++++++++++------------ actionpack/lib/action_view/template_file.rb | 10 +++---- actionpack/test/controller/new_render_test.rb | 8 +++--- actionpack/test/template/render_test.rb | 4 +-- 8 files changed, 45 insertions(+), 42 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 5a5895d9b4..9d64b6f231 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Deprecate :use_full_path render option. The supplying the option no longer has an effect [Josh Peek] + * Add :as option to render a collection of partials with a custom local variable name. #509 [Simon Jefford, Pratik Naik] render :partial => 'other_people', :collection => @people, :as => :person diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 209cdfa686..02941bade9 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -858,7 +858,7 @@ module ActionController #:nodoc: else if file = options[:file] - render_for_file(file, options[:status], options[:use_full_path], options[:locals] || {}) + render_for_file(file, options[:status], nil, options[:locals] || {}) elsif template = options[:template] render_for_file(template, options[:status], true, options[:locals] || {}) @@ -870,9 +870,9 @@ module ActionController #:nodoc: elsif action_name = options[:action] template = default_template_name(action_name.to_s) if options[:layout] && !template_exempt_from_layout?(template) - render_with_a_layout(:file => template, :status => options[:status], :use_full_path => true, :layout => true) + render_with_a_layout(:file => template, :status => options[:status], :layout => true) else - render_with_no_layout(:file => template, :status => options[:status], :use_full_path => true) + render_with_no_layout(:file => template, :status => options[:status]) end elsif xml = options[:xml] @@ -1097,10 +1097,10 @@ module ActionController #:nodoc: private - def render_for_file(template_path, status = nil, use_full_path = false, locals = {}) #:nodoc: + def render_for_file(template_path, status = nil, use_full_path = nil, locals = {}) #:nodoc: add_variables_to_assigns logger.info("Rendering #{template_path}" + (status ? " (#{status})" : '')) if logger - render_for_text(@template.render(:file => template_path, :use_full_path => use_full_path, :locals => locals), status) + render_for_text(@template.render(:file => template_path, :locals => locals), status) end def render_for_text(text = nil, status = nil, append_response = false) #:nodoc: diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 40a3b16e9f..a8c6e15ca3 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -232,12 +232,11 @@ module ActionView #:nodoc: # The hash in local_assigns is made available as local variables. def render(options = {}, local_assigns = {}, &block) #:nodoc: if options.is_a?(String) - render_file(options, true, local_assigns) + render_file(options, nil, local_assigns) elsif options == :update update_page(&block) elsif options.is_a?(Hash) - use_full_path = options[:use_full_path] - options = options.reverse_merge(:locals => {}, :use_full_path => true) + options = options.reverse_merge(:locals => {}) if partial_layout = options.delete(:layout) if block_given? @@ -250,7 +249,7 @@ module ActionView #:nodoc: end end elsif options[:file] - render_file(options[:file], use_full_path || false, options[:locals]) + render_file(options[:file], nil, options[:locals]) elsif options[:partial] && options[:collection] render_partial_collection(options[:partial], options[:collection], options[:spacer_template], options[:locals], options[:as]) elsif options[:partial] @@ -298,10 +297,9 @@ module ActionView #:nodoc: end private - # Renders the template present at template_path. If use_full_path is set to true, - # it's relative to the view_paths array, otherwise it's absolute. The hash in local_assigns + # 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 = true, local_assigns = {}) #:nodoc: + def render_file(template_path, use_full_path = nil, local_assigns = {}) #:nodoc: 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. diff --git a/actionpack/lib/action_view/partial_template.rb b/actionpack/lib/action_view/partial_template.rb index a2129952c0..719199f19d 100644 --- a/actionpack/lib/action_view/partial_template.rb +++ b/actionpack/lib/action_view/partial_template.rb @@ -6,7 +6,7 @@ module ActionView #:nodoc: @view_controller = view.controller if view.respond_to?(:controller) @as = as set_path_and_variable_name!(partial_path) - super(view, @path, true, locals) + super(view, @path, nil, locals) add_object_to_local_assigns!(object) # This is needed here in order to compile template with knowledge of 'counter' diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 4c3f252c10..0cba1942f7 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -5,15 +5,19 @@ module ActionView #:nodoc: attr_accessor :locals attr_reader :handler, :path, :extension, :filename, :method - def initialize(view, path, use_full_path, locals = {}) + 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 + @view = view @paths = view.view_paths @original_path = path - @path = TemplateFile.from_path(path, !use_full_path) + @path = TemplateFile.from_path(path) @view.first_render ||= @path.to_s @source = nil # Don't read the source until we know that it is required - set_extension_and_file_name(use_full_path) + set_extension_and_file_name @locals = locals || {} @handler = self.class.handler_class_for_extension(@extension).new(@view) @@ -63,25 +67,24 @@ module ActionView #:nodoc: end private - def set_extension_and_file_name(use_full_path) + def set_extension_and_file_name @extension = @path.extension - if use_full_path - unless @extension - @path = @view.send(:template_file_from_name, @path) - raise_missing_template_exception unless @path - @extension = @path.extension - end - - if @path = @paths.find_template_file_for_path(path) - @filename = @path.full_path - @extension = @path.extension - end - else - @filename = @path.full_path + unless @extension + @path = @view.send(:template_file_from_name, @path) + raise_missing_template_exception unless @path + @extension = @path.extension end - raise_missing_template_exception if @filename.blank? + 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 end def raise_missing_template_exception diff --git a/actionpack/lib/action_view/template_file.rb b/actionpack/lib/action_view/template_file.rb index dd66482b3c..c38e8ed122 100644 --- a/actionpack/lib/action_view/template_file.rb +++ b/actionpack/lib/action_view/template_file.rb @@ -4,8 +4,8 @@ module ActionView #:nodoc: # 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, use_full_path = false) - path.is_a?(self) ? path : new(path, use_full_path) + def self.from_path(path) + path.is_a?(self) ? path : new(path) end def self.from_full_path(load_path, full_path) @@ -17,11 +17,11 @@ module ActionView #:nodoc: attr_accessor :load_path, :base_path, :name, :format, :extension delegate :to_s, :inspect, :to => :path - def initialize(path, use_full_path = false) + def initialize(path) path = path.dup - # Clear the forward slash in the beginning unless using full path - trim_forward_slash!(path) unless use_full_path + # Clear the forward slash in the beginning + trim_forward_slash!(path) @base_path, @name, @format, @extension = split(path) end diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index 5a7da57559..d60bacd52a 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -81,12 +81,12 @@ class NewRenderTestController < ActionController::Base def render_file_not_using_full_path @secret = 'in the sauce' - render :file => 'test/render_file_with_ivar', :use_full_path => true + render :file => 'test/render_file_with_ivar' end def render_file_not_using_full_path_with_dot_in_path @secret = 'in the sauce' - render :file => 'test/dot.directory/render_file_with_ivar', :use_full_path => true + render :file => 'test/dot.directory/render_file_with_ivar' end def render_xml_hello @@ -231,13 +231,13 @@ class NewRenderTestController < ActionController::Base end def render_to_string_with_exception - render_to_string :file => "exception that will not be caught - this will certainly not work", :use_full_path => true + render_to_string :file => "exception that will not be caught - this will certainly not work" end def render_to_string_with_caught_exception @before = "i'm before the render" begin - render_to_string :file => "exception that will be caught- hope my future instance vars still work!", :use_full_path => true + render_to_string :file => "exception that will be caught- hope my future instance vars still work!" rescue end @after = "i'm after the render" diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 5163c35189..726cf37026 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -12,7 +12,7 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_file_not_using_full_path - assert_equal "Hello world!", @view.render(:file => "test/hello_world.erb", :use_full_path => true) + assert_equal "Hello world!", @view.render(:file => "test/hello_world.erb") end def test_render_file_without_specific_extension @@ -21,7 +21,7 @@ class ViewRenderTest < Test::Unit::TestCase def test_render_file_with_full_path template_path = File.join(File.dirname(__FILE__), '../fixtures/test/hello_world.erb') - assert_equal "Hello world!", @view.render(:file => template_path, :use_full_path => false) + assert_equal "Hello world!", @view.render(:file => template_path) end def test_render_file_with_instance_variables -- cgit v1.2.3 From a37d065f85941e1fe746dff4d42f015e1618834f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Thu, 3 Jul 2008 17:49:12 +0300 Subject: Use :namespace instead of :path_prefix for finding controller. [#544 state:resolved] :namespace is supposed to be the module where controller exists. :path_prefix can contain anything, including variables, which makes it unsuitable for determining the module for a controller. Signed-off-by: Pratik Naik --- actionpack/lib/action_controller/routing/builder.rb | 3 +-- actionpack/test/controller/routing_test.rb | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/routing/builder.rb b/actionpack/lib/action_controller/routing/builder.rb index 4740113ed0..b8323847fd 100644 --- a/actionpack/lib/action_controller/routing/builder.rb +++ b/actionpack/lib/action_controller/routing/builder.rb @@ -67,10 +67,9 @@ module ActionController options = options.dup if options[:namespace] - options[:controller] = "#{options[:path_prefix]}/#{options[:controller]}" + options[:controller] = "#{options.delete(:namespace).sub(/\/$/, '')}/#{options[:controller]}" options.delete(:path_prefix) options.delete(:name_prefix) - options.delete(:namespace) end requirements = (options.delete(:requirements) || {}).dup diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 07c13ebbf7..c5ccb71582 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -2039,6 +2039,26 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do Object.send(:remove_const, :Api) end + def test_namespace_with_path_prefix + Object.const_set(:Api, Module.new { |m| m.const_set(:ProductsController, Class.new) }) + + set.draw do |map| + + map.namespace 'api', :path_prefix => 'prefix' do |api| + api.route 'inventory', :controller => "products", :action => 'inventory' + end + + end + + request.path = "/prefix/inventory" + request.method = :get + assert_nothing_raised { set.recognize(request) } + assert_equal("api/products", request.path_parameters[:controller]) + assert_equal("inventory", request.path_parameters[:action]) + ensure + Object.send(:remove_const, :Api) + end + def test_generate_finds_best_fit set.draw do |map| map.connect "/people", :controller => "people", :action => "index" -- cgit v1.2.3 From 5501166dec84e1dd63800ea25eaf23290216cf80 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 30 Jun 2008 18:44:21 +0300 Subject: Remove strange alias for JavaScriptHelper --- actionpack/lib/action_view/helpers/javascript_helper.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index f89b6c2f70..8caca5b23f 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -216,7 +216,5 @@ module ActionView end end end - - JavascriptHelper = JavaScriptHelper unless const_defined? :JavascriptHelper end end -- cgit v1.2.3 From e358b1fce8fdcbac896dde08286be020420e843e Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 30 Jun 2008 18:46:53 +0300 Subject: Remove old method of including javascripts define_javascript_functions. javascript_include_tag and friends do a much better job. --- actionpack/CHANGELOG | 2 ++ .../lib/action_view/helpers/javascript_helper.rb | 26 ---------------------- actionpack/test/template/javascript_helper_test.rb | 8 ------- 3 files changed, 2 insertions(+), 34 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 9d64b6f231..0a5afddf04 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Remove define_javascript_functions, javascript_include_tag and friends are far superior. [Michael Koziarski] + * Deprecate :use_full_path render option. The supplying the option no longer has an effect [Josh Peek] * Add :as option to render a collection of partials with a custom local variable name. #509 [Simon Jefford, Pratik Naik] diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index 8caca5b23f..22bd5cb440 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -114,32 +114,6 @@ module ActionView tag(:input, html_options.merge(:type => 'button', :value => name, :onclick => onclick)) end - # Includes the Action Pack JavaScript libraries inside a single ' - end - - JS_ESCAPE_MAP = { '\\' => '\\\\', ' '<\/', diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb index d6d398d224..b2c4a130c8 100644 --- a/actionpack/test/template/javascript_helper_test.rb +++ b/actionpack/test/template/javascript_helper_test.rb @@ -3,14 +3,6 @@ require 'abstract_unit' class JavaScriptHelperTest < ActionView::TestCase tests ActionView::Helpers::JavaScriptHelper - def test_define_javascript_functions - # check if prototype.js is included first - assert_not_nil define_javascript_functions.split("\n")[1].match(/Prototype JavaScript framework/) - - # check that scriptaculous.js is not in here, only needed if loaded remotely - assert_nil define_javascript_functions.split("\n")[1].match(/var Scriptaculous = \{/) - end - def test_escape_javascript assert_equal '', escape_javascript(nil) assert_equal %(This \\"thing\\" is really\\n netos\\'), escape_javascript(%(This "thing" is really\n netos')) -- cgit v1.2.3 From efd18066a25610e5c00c7e144a9a08023016e576 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 30 Jun 2008 19:10:48 +0300 Subject: Tighten the rescue clause here to prevent hiding strange mock related errors behind the line offset test --- actionpack/test/controller/render_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 65862c6b14..10264dadaa 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -103,7 +103,7 @@ class TestController < ActionController::Base def render_line_offset begin render :inline => '<% raise %>', :locals => {:foo => 'bar'} - rescue => exc + rescue RuntimeError => exc end line = exc.backtrace.first render :text => line -- cgit v1.2.3 From df36a6f7598a7e963fb3d79fb48fd1c073045a43 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 30 Jun 2008 21:39:22 +0300 Subject: Remove unneeded ObjectWrapper class. Was previously needed to work around the semantics of a deprecated (now removed) API to render :partial --- actionpack/lib/action_controller/base.rb | 2 +- actionpack/lib/action_view/base.rb | 5 +---- actionpack/lib/action_view/partial_template.rb | 7 +------ 3 files changed, 3 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 02941bade9..009c9eec99 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -897,7 +897,7 @@ module ActionController #:nodoc: else render_for_text( @template.send!(:render_partial, partial, - ActionView::Base::ObjectWrapper.new(options[:object]), options[:locals]), options[:status] + options[:object], options[:locals]), options[:status] ) end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index a8c6e15ca3..9e255bd324 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -199,9 +199,6 @@ module ActionView #:nodoc: cattr_reader :computed_public_paths @@computed_public_paths = {} - class ObjectWrapper < Struct.new(:value) #:nodoc: - end - def self.helper_modules #:nodoc: helpers = [] Dir.entries(File.expand_path("#{File.dirname(__FILE__)}/helpers")).sort.each do |file| @@ -253,7 +250,7 @@ module ActionView #:nodoc: elsif options[:partial] && options[:collection] render_partial_collection(options[:partial], options[:collection], options[:spacer_template], options[:locals], options[:as]) elsif options[:partial] - render_partial(options[:partial], ActionView::Base::ObjectWrapper.new(options[:object]), options[:locals]) + render_partial(options[:partial], options[:object], options[:locals]) elsif options[:inline] render_inline(options[:inline], options[:locals], options[:type]) end diff --git a/actionpack/lib/action_view/partial_template.rb b/actionpack/lib/action_view/partial_template.rb index 719199f19d..6ebe165a15 100644 --- a/actionpack/lib/action_view/partial_template.rb +++ b/actionpack/lib/action_view/partial_template.rb @@ -42,12 +42,7 @@ module ActionView #:nodoc: private def add_object_to_local_assigns!(object) @locals[:object] ||= - @locals[@variable_name] ||= - if object.is_a?(ActionView::Base::ObjectWrapper) - object.value - else - object - end || @view_controller.instance_variable_get("@#{variable_name}") + @locals[@variable_name] ||= object || @view_controller.instance_variable_get("@#{variable_name}") @locals[as] ||= @locals[:object] if as end -- cgit v1.2.3 From 5dd10d60bbde906449882e39278be76f1f445a41 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 30 Jun 2008 22:15:12 +0300 Subject: Remove nested ternary operators from select_year in favour of conditionals. --- actionpack/lib/action_view/helpers/date_helper.rb | 31 +++++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 1aee9ef0a2..fa3e612fc1 100755 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -547,23 +547,32 @@ module ActionView # select_year(2006, :start_year => 2000, :end_year => 2010) # def select_year(date, options = {}, html_options = {}) - val = date ? (date.kind_of?(Fixnum) ? date : date.year) : '' + if !date || date == 0 + value = '' + middle_year = Date.today.year + elsif date.kind_of?(Fixnum) + value = middle_year = date + else + value = middle_year = date.year + end + if options[:use_hidden] - hidden_html(options[:field_name] || 'year', val, options) + hidden_html(options[:field_name] || 'year', value, options) else - year_options = [] - y = date ? (date.kind_of?(Fixnum) ? (y = (date == 0) ? Date.today.year : date) : date.year) : Date.today.year + year_options = '' + start_year = options[:start_year] || middle_year - 5 + end_year = options[:end_year] || middle_year + 5 + step_val = start_year < end_year ? 1 : -1 - start_year, end_year = (options[:start_year] || y-5), (options[:end_year] || y+5) - step_val = start_year < end_year ? 1 : -1 start_year.step(end_year, step_val) do |year| - year_options << ((val == year) ? - content_tag(:option, year, :value => year, :selected => "selected") : - content_tag(:option, year, :value => year) - ) + if value == year + year_options << content_tag(:option, year, :value => year, :selected => "selected") + else + year_options << content_tag(:option, year, :value => year) + end year_options << "\n" end - select_html(options[:field_name] || 'year', year_options.join, options, html_options) + select_html(options[:field_name] || 'year', year_options, options, html_options) end end -- cgit v1.2.3 From 7098143f07bd03223bbbeea8a9c23506a7b5dffc Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Wed, 2 Jul 2008 19:53:04 +0300 Subject: Remove unused local_binding option to InstanceTag's Constructor --- actionpack/lib/action_view/helpers/date_helper.rb | 6 +++--- actionpack/lib/action_view/helpers/form_helper.rb | 20 ++++++++++---------- .../lib/action_view/helpers/form_options_helper.rb | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index fa3e612fc1..50ae27da61 100755 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -153,7 +153,7 @@ module ActionView # Note: If the day is not included as an option but the month is, the day will be set to the 1st to ensure that all month # choices are valid. def date_select(object_name, method, options = {}, html_options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_date_select_tag(options, html_options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_date_select_tag(options, html_options) end # Returns a set of select tags (one for hour, minute and optionally second) pre-selected for accessing a specified @@ -188,7 +188,7 @@ module ActionView # Note: If the day is not included as an option but the month is, the day will be set to the 1st to ensure that all month # choices are valid. def time_select(object_name, method, options = {}, html_options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_time_select_tag(options, html_options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_time_select_tag(options, html_options) end # Returns a set of select tags (one for year, month, day, hour, and minute) pre-selected for accessing a specified datetime-based @@ -214,7 +214,7 @@ module ActionView # # The selects are prepared for multi-parameter assignment to an Active Record object. def datetime_select(object_name, method, options = {}, html_options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_datetime_select_tag(options, html_options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_datetime_select_tag(options, html_options) end # Returns a set of html select-tags (one for year, month, day, hour, and minute) pre-selected with the +datetime+. diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 63a932320e..fa5a9bfac6 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -333,7 +333,7 @@ module ActionView # # => # def label(object_name, method, text = nil, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_label_tag(text, options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_label_tag(text, options) end # Returns an input tag of the "text" type tailored for accessing a specified attribute (identified by +method+) on an object @@ -355,7 +355,7 @@ module ActionView # # => # def text_field(object_name, method, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_input_field_tag("text", options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("text", options) end # Returns an input tag of the "password" type tailored for accessing a specified attribute (identified by +method+) on an object @@ -377,7 +377,7 @@ module ActionView # # => # def password_field(object_name, method, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_input_field_tag("password", options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("password", options) end # Returns a hidden input tag tailored for accessing a specified attribute (identified by +method+) on an object @@ -395,7 +395,7 @@ module ActionView # hidden_field(:user, :token) # # => def hidden_field(object_name, method, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_input_field_tag("hidden", options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("hidden", options) end # Returns an file upload input tag tailored for accessing a specified attribute (identified by +method+) on an object @@ -414,7 +414,7 @@ module ActionView # # => # def file_field(object_name, method, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_input_field_tag("file", options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("file", options) end # Returns a textarea opening and closing tag set tailored for accessing a specified attribute (identified by +method+) @@ -442,7 +442,7 @@ module ActionView # # #{@entry.body} # # def text_area(object_name, method, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_text_area_tag(options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_text_area_tag(options) end # Returns a checkbox tag tailored for accessing a specified attribute (identified by +method+) on an object @@ -468,7 +468,7 @@ module ActionView # # # def check_box(object_name, method, options = {}, checked_value = "1", unchecked_value = "0") - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_check_box_tag(options, checked_value, unchecked_value) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_check_box_tag(options, checked_value, unchecked_value) end # Returns a radio button tag for accessing a specified attribute (identified by +method+) on an object @@ -488,7 +488,7 @@ module ActionView # # => # # def radio_button(object_name, method, tag_value, options = {}) - InstanceTag.new(object_name, method, self, nil, options.delete(:object)).to_radio_button_tag(tag_value, options) + InstanceTag.new(object_name, method, self, options.delete(:object)).to_radio_button_tag(tag_value, options) end end @@ -501,9 +501,9 @@ module ActionView DEFAULT_RADIO_OPTIONS = { }.freeze unless const_defined?(:DEFAULT_RADIO_OPTIONS) DEFAULT_TEXT_AREA_OPTIONS = { "cols" => 40, "rows" => 20 }.freeze unless const_defined?(:DEFAULT_TEXT_AREA_OPTIONS) - def initialize(object_name, method_name, template_object, local_binding = nil, object = nil) + def initialize(object_name, method_name, template_object, object = nil) @object_name, @method_name = object_name.to_s.dup, method_name.to_s.dup - @template_object, @local_binding = template_object, local_binding + @template_object= template_object @object = object if @object_name.sub!(/\[\]$/,"") if object ||= @template_object.instance_variable_get("@#{Regexp.last_match.pre_match}") and object.respond_to?(:to_param) diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 0ca5cebcba..ab9e174621 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -96,7 +96,7 @@ module ActionView # By default, post.person_id is the selected option. Specify :selected => value to use a different selection # or :selected => nil to leave all options unselected. def select(object, method, choices, options = {}, html_options = {}) - InstanceTag.new(object, method, self, nil, options.delete(:object)).to_select_tag(choices, options, html_options) + InstanceTag.new(object, method, self, options.delete(:object)).to_select_tag(choices, options, html_options) end # Returns def collection_select(object, method, collection, value_method, text_method, options = {}, html_options = {}) - InstanceTag.new(object, method, self, nil, options.delete(:object)).to_collection_select_tag(collection, value_method, text_method, options, html_options) + InstanceTag.new(object, method, self, options.delete(:object)).to_collection_select_tag(collection, value_method, text_method, options, html_options) end # Return select and option tags for the given object and method, using country_options_for_select to generate the list of option tags. def country_select(object, method, priority_countries = nil, options = {}, html_options = {}) - InstanceTag.new(object, method, self, nil, options.delete(:object)).to_country_select_tag(priority_countries, options, html_options) + InstanceTag.new(object, method, self, options.delete(:object)).to_country_select_tag(priority_countries, options, html_options) end # Return select and option tags for the given object and method, using @@ -169,7 +169,7 @@ module ActionView # # time_zone_select( "user", "time_zone", TZInfo::Timezone.all.sort, :model => TZInfo::Timezone) def time_zone_select(object, method, priority_zones = nil, options = {}, html_options = {}) - InstanceTag.new(object, method, self, nil, options.delete(:object)).to_time_zone_select_tag(priority_zones, options, html_options) + InstanceTag.new(object, method, self, options.delete(:object)).to_time_zone_select_tag(priority_zones, options, html_options) end # Accepts a container (hash, array, enumerable, your type) and returns a string of option tags. Given a container -- cgit v1.2.3 From 12cf8f348b591b7bb0dc899345293a8e37ddad7c Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 27 Jun 2008 21:24:21 +0300 Subject: Move template_format logic out to the request so it's alongside the 'regular' request format. Use xhr? instead of the expensive trip through Request#accepts. --- actionpack/lib/action_controller/request.rb | 13 +++++++++++++ actionpack/lib/action_view/base.rb | 12 +----------- actionpack/test/controller/new_render_test.rb | 3 +-- 3 files changed, 15 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 9b02f2c8a1..c91a3387a0 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -116,6 +116,19 @@ module ActionController @format = Mime::Type.lookup_by_extension(parameters[:format]) end + def template_format + parameter_format = parameters[:format] + + case + when parameter_format.blank? && !xhr? + :html + when parameter_format.blank? && xhr? + :js + else + parameter_format.to_sym + end + end + # Returns true if the request's "X-Requested-With" header contains # "XMLHttpRequest". (The Prototype Javascript library sends this header with # every Ajax request.) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 9e255bd324..e8e690abec 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -273,17 +273,7 @@ module ActionView #:nodoc: return @template_format if @template_format if controller && controller.respond_to?(:request) - parameter_format = controller.request.parameters[:format] - accept_format = controller.request.accepts.first - - case - when parameter_format.blank? && accept_format != :js - @template_format = :html - when parameter_format.blank? && accept_format == :js - @template_format = :js - else - @template_format = parameter_format.to_sym - end + @template_format = controller.request.template_format else @template_format = :html end diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index d60bacd52a..b2691d981b 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -605,8 +605,7 @@ EOS end def test_render_with_default_from_accept_header - @request.env["HTTP_ACCEPT"] = "text/javascript" - get :greeting + xhr :get, :greeting assert_equal "$(\"body\").visualEffect(\"highlight\");", @response.body end -- cgit v1.2.3 From d79cde37ea9a14fc6625297f40050296af7f7630 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Tue, 1 Jul 2008 20:37:21 +0300 Subject: Move the file exists checks outside write_asset_file_contents. This lets us avoid the relatively costly trip through compute_*_paths if the file already exists. --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index e5a95a961c..0122de47af 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -243,7 +243,7 @@ module ActionView 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)) + write_asset_file_contents(joined_javascript_path, compute_javascript_paths(sources)) 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") @@ -370,7 +370,7 @@ module ActionView 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)) + write_asset_file_contents(joined_stylesheet_path, compute_stylesheet_paths(sources)) 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") @@ -601,10 +601,8 @@ module ActionView end def write_asset_file_contents(joined_asset_path, asset_paths) - unless file_exist?(joined_asset_path) - FileUtils.mkdir_p(File.dirname(joined_asset_path)) - File.open(joined_asset_path, "w+") { |cache| cache.write(join_asset_file_contents(asset_paths)) } - end + FileUtils.mkdir_p(File.dirname(joined_asset_path)) + File.open(joined_asset_path, "w+") { |cache| cache.write(join_asset_file_contents(asset_paths)) } end end end -- cgit v1.2.3 From 75e04b52956512d554c83e0134a81c980c15b4fa Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Tue, 1 Jul 2008 20:43:57 +0300 Subject: Tighten the rescue clause when dealing with invalid instance variable names in form_helper. --- actionpack/lib/action_view/helpers/form_helper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index fa5a9bfac6..bafc635ad2 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -601,7 +601,11 @@ module ActionView end def object - @object || (@template_object.instance_variable_get("@#{@object_name}") rescue nil) + @object || @template_object.instance_variable_get("@#{@object_name}") + rescue NameError + # As @object_name may contain the nested syntax (item[subobject]) we + # need to fallback to nil. + nil end def value(object) -- cgit v1.2.3 From 42d215a925a228778e43f7040f03ad8f3eb5341c Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 3 Jul 2008 12:48:00 -0500 Subject: Moved TemplateHandlers to Base --- actionpack/lib/action_view/base.rb | 1 + actionpack/lib/action_view/helpers/cache_helper.rb | 2 +- actionpack/lib/action_view/inline_template.rb | 2 +- actionpack/lib/action_view/template.rb | 7 +++++-- actionpack/test/controller/layout_test.rb | 2 +- actionpack/test/template/render_test.rb | 8 ++++---- 6 files changed, 13 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index e8e690abec..169bee1bfc 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -151,6 +151,7 @@ module ActionView #:nodoc: # # See the ActionView::Helpers::PrototypeHelper::GeneratorMethods documentation for more details. class Base + extend TemplateHandlers include ERB::Util attr_accessor :base_path, :assigns, :template_extension, :first_render diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index 930c397785..c2aab5aa72 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -32,7 +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 = Base.handler_class_for_extension(current_render_extension.to_sym) handler.new(@controller).cache_fragment(block, name, options) end end diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index fd0ad48302..3a97f47e32 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -7,7 +7,7 @@ module ActionView #:nodoc: @extension = type @locals = locals || {} - @handler = self.class.handler_class_for_extension(@extension).new(@view) + @handler = Base.handler_class_for_extension(@extension).new(@view) end def method_key diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 0cba1942f7..d4118f0f86 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,6 +1,9 @@ module ActionView #:nodoc: class Template #:nodoc: - extend TemplateHandlers + class << self + # TODO: Deprecate + delegate :register_template_handler, :to => 'ActionView::Base' + end attr_accessor :locals attr_reader :handler, :path, :extension, :filename, :method @@ -20,7 +23,7 @@ module ActionView #:nodoc: set_extension_and_file_name @locals = locals || {} - @handler = self.class.handler_class_for_extension(@extension).new(@view) + @handler = Base.handler_class_for_extension(@extension).new(@view) end def render_template diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 3dc311b78a..52ab72bb99 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -40,7 +40,7 @@ class MabView < ActionView::TemplateHandler end end -ActionView::Template::register_template_handler :mab, MabView +ActionView::Base.register_template_handler :mab, MabView class LayoutAutoDiscoveryTest < Test::Unit::TestCase def setup diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 726cf37026..0dcf88da83 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -101,12 +101,12 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_inline_with_custom_type - ActionView::Template.register_template_handler :foo, CustomHandler + ActionView::Base.register_template_handler :foo, CustomHandler assert_equal '["Hello, World!", {}]', @view.render(:inline => "Hello, World!", :type => :foo) end def test_render_inline_with_locals_and_custom_type - ActionView::Template.register_template_handler :foo, CustomHandler + ActionView::Base.register_template_handler :foo, CustomHandler assert_equal '["Hello, <%= name %>!", {:name=>"Josh"}]', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) end @@ -121,12 +121,12 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_inline_with_compilable_custom_type - ActionView::Template.register_template_handler :foo, CompilableCustomHandler + ActionView::Base.register_template_handler :foo, CompilableCustomHandler assert_equal 'locals: {}, 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 + ActionView::Base.register_template_handler :foo, CompilableCustomHandler assert_equal 'locals: {:name=>"Josh"}, source: "Hello, <%= name %>!"', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) end end -- cgit v1.2.3 From b6f89a8bf59a0e6227c054d0be49baedf7b9f530 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 3 Jul 2008 12:49:54 -0500 Subject: Don't rely on view instance 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 783456ab53..f436ebbe45 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -41,10 +41,10 @@ module ActionView file_name = template.filename || 'compiled-template' ActionView::Base::CompiledTemplates.module_eval(render_source, file_name, -line_offset) rescue Exception => e # errors from template code - if @view.logger - @view.logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" - @view.logger.debug "Function body: #{render_source}" - @view.logger.debug "Backtrace: #{e.backtrace.join("\n")}" + if Base.logger + Base.logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" + Base.logger.debug "Function body: #{render_source}" + Base.logger.debug "Backtrace: #{e.backtrace.join("\n")}" end raise ActionView::TemplateError.new(template, @view.assigns, e) -- cgit v1.2.3 From 7d5c8505f528f195433693d5074a00f7635955b2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 3 Jul 2008 12:50:43 -0500 Subject: Use render on InlineTemplate --- 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 169bee1bfc..d4802d7965 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -305,7 +305,7 @@ module ActionView #:nodoc: end def render_inline(text, local_assigns = {}, type = nil) - InlineTemplate.new(self, text, local_assigns, type).render_template + InlineTemplate.new(self, text, local_assigns, type).render end def wrap_content_for_layout(content) -- cgit v1.2.3 From 8a442e0d57fabedcaf3ded836cfc12f7067ead68 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 3 Jul 2008 13:06:00 -0500 Subject: Extracted Template rendering logic into Renderer module --- actionpack/lib/action_view.rb | 2 ++ actionpack/lib/action_view/inline_template.rb | 9 ++++--- actionpack/lib/action_view/partial_template.rb | 2 +- actionpack/lib/action_view/renderer.rb | 29 ++++++++++++++++++++++ actionpack/lib/action_view/template.rb | 33 +++++--------------------- 5 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 actionpack/lib/action_view/renderer.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 973020a768..49768fe264 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -24,6 +24,8 @@ require 'action_view/template_handlers' require 'action_view/template_file' require 'action_view/view_load_paths' + +require 'action_view/renderer' require 'action_view/template' require 'action_view/partial_template' require 'action_view/inline_template' diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 3a97f47e32..19ab92ce1a 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -1,5 +1,7 @@ module ActionView #:nodoc: - class InlineTemplate < Template #:nodoc: + class InlineTemplate #:nodoc: + include Renderer + def initialize(view, source, locals = {}, type = nil) @view = view @@ -7,11 +9,8 @@ module ActionView #:nodoc: @extension = type @locals = locals || {} + @method_key = @source @handler = Base.handler_class_for_extension(@extension).new(@view) end - - def method_key - @source - end end end diff --git a/actionpack/lib/action_view/partial_template.rb b/actionpack/lib/action_view/partial_template.rb index 6ebe165a15..72f831e937 100644 --- a/actionpack/lib/action_view/partial_template.rb +++ b/actionpack/lib/action_view/partial_template.rb @@ -18,7 +18,7 @@ module ActionView #:nodoc: def render ActionController::Base.benchmark("Rendered #{@path.path_without_format_and_extension}", Logger::DEBUG, false) do - @handler.render(self) + super end end diff --git a/actionpack/lib/action_view/renderer.rb b/actionpack/lib/action_view/renderer.rb new file mode 100644 index 0000000000..e6c64d2749 --- /dev/null +++ b/actionpack/lib/action_view/renderer.rb @@ -0,0 +1,29 @@ +module ActionView + module Renderer + # TODO: Local assigns should not be tied to template instance + attr_accessor :locals + + # TODO: These readers should be private + attr_reader :filename, :source, :handler, :method_key, :method + + def render + prepare! + @handler.render(self) + end + + private + def prepare! + unless @prepared + @view.send(:evaluate_assigns) + @view.current_render_extension = @extension + + if @handler.compilable? + @handler.compile_template(self) # compile the given template, if necessary + @method = @view.method_names[method_key] # Set the method name for this template and run it + end + + @prepared = true + end + end + end +end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index d4118f0f86..8142232c8f 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,12 +1,13 @@ module ActionView #:nodoc: class Template #:nodoc: + include Renderer + class << self # TODO: Deprecate delegate :register_template_handler, :to => 'ActionView::Base' end - attr_accessor :locals - attr_reader :handler, :path, :extension, :filename, :method + attr_reader :path, :extension def initialize(view, path, use_full_path = nil, locals = {}) unless use_full_path == nil @@ -19,9 +20,10 @@ module ActionView #:nodoc: @original_path = path @path = TemplateFile.from_path(path) @view.first_render ||= @path.to_s - @source = nil # Don't read the source until we know that it is required + set_extension_and_file_name + @method_key = @filename @locals = locals || {} @handler = Base.handler_class_for_extension(@extension).new(@view) end @@ -38,37 +40,14 @@ module ActionView #:nodoc: end end - def render - prepare! - @handler.render(self) - end - - def path_without_extension - @path.path_without_extension - end - def source @source ||= File.read(self.filename) end - def method_key - @filename - end - def base_path_for_exception (@paths.find_load_path_for_path(@path) || @paths.first).to_s end - def prepare! - @view.send :evaluate_assigns - @view.current_render_extension = @extension - - if @handler.compilable? - @handler.compile_template(self) # compile the given template, if necessary - @method = @view.method_names[method_key] # Set the method name for this template and run it - end - end - private def set_extension_and_file_name @extension = @path.extension @@ -94,7 +73,7 @@ module ActionView #:nodoc: 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}") + raise MissingTemplate, "Missing #{template_type} #{full_template_path} in view path #{display_paths}" end end end -- cgit v1.2.3 From 1a478923dc909bf7b6aea4f2ad49cbeee6dea259 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 3 Jul 2008 14:01:45 -0500 Subject: Reduce the number of callsites for new TemplateFiles --- actionpack/lib/action_controller/base.rb | 5 +++-- actionpack/lib/action_view/base.rb | 28 ++++++++++++++++----------- actionpack/lib/action_view/view_load_paths.rb | 22 ++++++++++++--------- 3 files changed, 33 insertions(+), 22 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 009c9eec99..a4bade1bd5 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1235,8 +1235,9 @@ module ActionController #:nodoc: end def template_exempt_from_layout?(template_name = default_template_name) - template_name = @template.send(:template_file_from_name, template_name) if @template - @@exempt_from_layout.any? { |ext| template_name.to_s =~ ext } + 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 } end def default_template_name(action_name = self.action_name) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index d4802d7965..5a3fc4182f 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -284,6 +284,21 @@ module ActionView #:nodoc: view_paths.template_exists?(template_file_from_name(template_path)) 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_extension('users/legacy') + # # => "rhtml" + # + def pick_template_extension(template_path) + if template = template_file_from_name(template_path) + template.extension + end + end + private # Renders the template present at template_path. The hash in local_assigns # is made available as local variables. @@ -336,19 +351,10 @@ module ActionView #:nodoc: def template_file_from_name(template_name) template_name = TemplateFile.from_path(template_name) - pick_template_extension(template_name) unless template_name.extension + pick_template(template_name) unless template_name.extension 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_extension('users/legacy') - # # => "rhtml" - # - def pick_template_extension(file) + 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)) diff --git a/actionpack/lib/action_view/view_load_paths.rb b/actionpack/lib/action_view/view_load_paths.rb index f23ac665f1..6e439a009c 100644 --- a/actionpack/lib/action_view/view_load_paths.rb +++ b/actionpack/lib/action_view/view_load_paths.rb @@ -29,12 +29,10 @@ module ActionView #:nodoc: @paths.freeze end - # Tries to find the extension for the template name. - # If it does not it exist, tries again without the format extension - # find_template_file_for_partial_path('users/show') => 'html.erb' - # find_template_file_for_partial_path('users/legacy') => 'rhtml' - def find_template_file_for_partial_path(file) - @paths[file.path] || @paths[file.path_without_extension] || @paths[file.path_without_format_and_extension] + 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 @@ -81,10 +79,10 @@ module ActionView #:nodoc: find { |path| path.paths[file.to_s] } end - def find_template_file_for_path(file) - file = TemplateFile.from_path(file) + 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(file) + if f = path.find_template_file_for_partial_path(template_path_without_extension, template_extension) return f end end @@ -95,5 +93,11 @@ module ActionView #:nodoc: 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 -- cgit v1.2.3 From 570f5aad663fa3113772cf56306862829babc739 Mon Sep 17 00:00:00 2001 From: miloops Date: Tue, 1 Jul 2008 11:08:25 -0300 Subject: Allow date helpers to ignore date hidden field tags. [#503 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_view/helpers/date_helper.rb | 7 +++++-- actionpack/test/template/date_helper_test.rb | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 50ae27da61..d018034ebe 100755 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -159,7 +159,10 @@ module ActionView # Returns a set of select tags (one for hour, minute and optionally second) pre-selected for accessing a specified # time-based attribute (identified by +method+) on an object assigned to the template (identified by +object+). # You can include the seconds with :include_seconds. - # + # + # This method will also generate 3 input hidden tags, for the actual year, month and day unless the option + # :ignore_date is set to +true+. + # # If anything is passed in the html_options hash it will be applied to every select tag in the set. # # ==== Examples @@ -655,7 +658,7 @@ module ActionView order.reverse.each do |param| # Send hidden fields for discarded elements once output has started # This ensures AR can reconstruct valid dates using ParseDate - next if discard[param] && date_or_time_select.empty? + next if discard[param] && (date_or_time_select.empty? || options[:ignore_date]) date_or_time_select.insert(0, self.send("select_#{param}", datetime, options_with_prefix(position[param], options.merge(:use_hidden => discard[param])), html_options)) date_or_time_select.insert(0, diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index 3faa363459..8b4e94c67f 100755 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -1198,6 +1198,21 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, time_select("post", "written_on") end + def test_time_select_without_date_hidden_fields + @post = Post.new + @post.written_on = Time.local(2004, 6, 15, 15, 16, 35) + + expected = %(\n" + expected << " : " + expected << %(\n" + + assert_dom_equal expected, time_select("post", "written_on", :ignore_date => true) + end + def test_time_select_with_seconds @post = Post.new @post.written_on = Time.local(2004, 6, 15, 15, 16, 35) -- cgit v1.2.3 From bad1eac91d1549631dca8e93e7e846911649acf7 Mon Sep 17 00:00:00 2001 From: josevalim Date: Sat, 14 Jun 2008 10:45:32 +0200 Subject: Allow caches_action to accept cache store options. [#416 state:resolved] Signed-off-by: Pratik Naik --- actionpack/CHANGELOG | 4 ++++ actionpack/lib/action_controller/caching/actions.rb | 18 +++++++++++------- actionpack/test/controller/caching_test.rb | 10 +++++++++- 3 files changed, 24 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 0a5afddf04..c42d3825e3 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,9 @@ *Edge* +* Allow caches_action to accept cache store options. #416. [José Valim]. Example: + + caches_action :index, :redirected, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour + * Remove define_javascript_functions, javascript_include_tag and friends are far superior. [Michael Koziarski] * Deprecate :use_full_path render option. The supplying the option no longer has an effect [Josh Peek] diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 65a36f7f98..234ef8ae5b 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -27,13 +27,15 @@ module ActionController #:nodoc: # You can set modify the default action cache path by passing a :cache_path option. This will be passed directly to ActionCachePath.path_for. This is handy # for actions with multiple possible routes that should be cached differently. If a block is given, it is called with the current controller instance. # - # And you can also use :if to pass a Proc that specifies when the action should be cached. + # And you can also use :if (or :unless) to pass a Proc that specifies when the action should be cached. + # + # Finally, if you are using memcached, you can also pass :expires_in. # # class ListsController < ApplicationController # before_filter :authenticate, :except => :public # caches_page :public # caches_action :index, :if => Proc.new { |c| !c.request.format.json? } # cache if is not a JSON request - # caches_action :show, :cache_path => { :project => 1 } + # caches_action :show, :cache_path => { :project => 1 }, :expires_in => 1.hour # caches_action :feed, :cache_path => Proc.new { |controller| # controller.params[:user_id] ? # controller.send(:user_list_url, c.params[:user_id], c.params[:id]) : @@ -56,8 +58,10 @@ module ActionController #:nodoc: def caches_action(*actions) return unless cache_configured? options = actions.extract_options! - cache_filter = ActionCacheFilter.new(:layout => options.delete(:layout), :cache_path => options.delete(:cache_path)) - around_filter(cache_filter, {:only => actions}.merge(options)) + filter_options = { :only => actions, :if => options.delete(:if), :unless => options.delete(:unless) } + + cache_filter = ActionCacheFilter.new(:layout => options.delete(:layout), :cache_path => options.delete(:cache_path), :store_options => options) + around_filter(cache_filter, filter_options) end end @@ -80,8 +84,8 @@ module ActionController #:nodoc: end def before(controller) - cache_path = ActionCachePath.new(controller, path_options_for(controller, @options)) - if cache = controller.read_fragment(cache_path.path) + cache_path = ActionCachePath.new(controller, path_options_for(controller, @options.slice(:cache_path))) + if cache = controller.read_fragment(cache_path.path, @options[:store_options]) controller.rendered_action_cache = true set_content_type!(controller, cache_path.extension) options = { :text => cache } @@ -96,7 +100,7 @@ module ActionController #:nodoc: def after(controller) return if controller.rendered_action_cache || !caching_allowed(controller) action_content = cache_layout? ? content_for_layout(controller) : controller.response.body - controller.write_fragment(controller.action_cache_path.path, action_content) + controller.write_fragment(controller.action_cache_path.path, action_content, @options[:store_options]) end private diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 0140654155..d8a3ccb35b 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -152,7 +152,7 @@ end class ActionCachingTestController < ActionController::Base - caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| !c.request.format.json? } + caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour caches_action :show, :cache_path => 'http://test.host/custom/show' caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" } caches_action :with_layout @@ -188,6 +188,7 @@ class ActionCachingTestController < ActionController::Base expire_action :controller => 'action_caching_test', :action => 'index' render :nothing => true end + def expire_xml expire_action :controller => 'action_caching_test', :action => 'index', :format => 'xml' render :nothing => true @@ -289,6 +290,13 @@ class ActionCacheTest < Test::Unit::TestCase assert !fragment_exist?('hostname.com/action_caching_test') end + def test_action_cache_with_store_options + MockTime.expects(:now).returns(12345).once + @controller.expects(:read_fragment).with('hostname.com/action_caching_test', :expires_in => 1.hour).once + @controller.expects(:write_fragment).with('hostname.com/action_caching_test', '12345.0', :expires_in => 1.hour).once + get :index + end + def test_action_cache_with_custom_cache_path get :show cached_time = content_to_cache -- cgit v1.2.3 From 01637796d712943ebf9e9a76aa5c708edfab4d02 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 3 Jul 2008 21:08:27 -0500 Subject: Revert "Moved TemplateHandlers to Base" This reverts commit 42d215a925a228778e43f7040f03ad8f3eb5341c. Conflicts: actionpack/lib/action_view/inline_template.rb actionpack/lib/action_view/template.rb --- actionpack/lib/action_view/base.rb | 1 - actionpack/lib/action_view/helpers/cache_helper.rb | 2 +- actionpack/lib/action_view/inline_template.rb | 2 +- actionpack/lib/action_view/template.rb | 8 ++------ actionpack/test/controller/layout_test.rb | 2 +- actionpack/test/template/render_test.rb | 8 ++++---- 6 files changed, 9 insertions(+), 14 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 5a3fc4182f..d18c701619 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -151,7 +151,6 @@ module ActionView #:nodoc: # # See the ActionView::Helpers::PrototypeHelper::GeneratorMethods documentation for more details. class Base - extend TemplateHandlers include ERB::Util attr_accessor :base_path, :assigns, :template_extension, :first_render diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index c2aab5aa72..930c397785 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -32,7 +32,7 @@ module ActionView # Topics listed alphabetically # <% end %> def cache(name = {}, options = nil, &block) - handler = Base.handler_class_for_extension(current_render_extension.to_sym) + handler = Template.handler_class_for_extension(current_render_extension.to_sym) handler.new(@controller).cache_fragment(block, name, options) end end diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 19ab92ce1a..1ccb23a3fc 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -10,7 +10,7 @@ module ActionView #:nodoc: @locals = locals || {} @method_key = @source - @handler = Base.handler_class_for_extension(@extension).new(@view) + @handler = Template.handler_class_for_extension(@extension).new(@view) end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 8142232c8f..dbc8ccdee8 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,12 +1,8 @@ module ActionView #:nodoc: class Template #:nodoc: + extend TemplateHandlers include Renderer - class << self - # TODO: Deprecate - delegate :register_template_handler, :to => 'ActionView::Base' - end - attr_reader :path, :extension def initialize(view, path, use_full_path = nil, locals = {}) @@ -25,7 +21,7 @@ module ActionView #:nodoc: @method_key = @filename @locals = locals || {} - @handler = Base.handler_class_for_extension(@extension).new(@view) + @handler = self.class.handler_class_for_extension(@extension).new(@view) end def render_template diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 52ab72bb99..3dc311b78a 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -40,7 +40,7 @@ class MabView < ActionView::TemplateHandler end end -ActionView::Base.register_template_handler :mab, MabView +ActionView::Template::register_template_handler :mab, MabView class LayoutAutoDiscoveryTest < Test::Unit::TestCase def setup diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 0dcf88da83..726cf37026 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -101,12 +101,12 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_inline_with_custom_type - ActionView::Base.register_template_handler :foo, CustomHandler + ActionView::Template.register_template_handler :foo, CustomHandler assert_equal '["Hello, World!", {}]', @view.render(:inline => "Hello, World!", :type => :foo) end def test_render_inline_with_locals_and_custom_type - ActionView::Base.register_template_handler :foo, CustomHandler + ActionView::Template.register_template_handler :foo, CustomHandler assert_equal '["Hello, <%= name %>!", {:name=>"Josh"}]', @view.render(:inline => "Hello, <%= name %>!", :locals => { :name => "Josh" }, :type => :foo) end @@ -121,12 +121,12 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_inline_with_compilable_custom_type - ActionView::Base.register_template_handler :foo, CompilableCustomHandler + ActionView::Template.register_template_handler :foo, CompilableCustomHandler assert_equal 'locals: {}, source: "Hello, World!"', @view.render(:inline => "Hello, World!", :type => :foo) end def test_render_inline_with_locals_and_compilable_custom_type - ActionView::Base.register_template_handler :foo, CompilableCustomHandler + 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) end end -- cgit v1.2.3 From db5839107951b000633fa8405f78e5c315b6656a Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 23 Jun 2008 20:32:26 +0300 Subject: Remove old broken follow_redirect from functional tests. Still works in integration tests. The follow_redirect in functional tests only worked if you used redirect_to :id=>foo, :action=>bar, rather than named routes. --- actionpack/CHANGELOG | 5 +++++ actionpack/lib/action_controller/test_process.rb | 9 --------- .../test/controller/action_pack_assertions_test.rb | 16 ---------------- actionpack/test/controller/test_test.rb | 18 ------------------ 4 files changed, 5 insertions(+), 43 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index c42d3825e3..7f275bef27 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -18,6 +18,11 @@ * Made ActionView::Base#render_file private [Josh Peek] +* Remove follow_redirect from controller functional tests. + + If you want to follow redirects you can use integration tests. The functional test + version was only useful if you were using redirect_to :id=>... + * Fix polymorphic_url with singleton resources. #461 [Tammer Saleh] * Replaced TemplateFinder abstraction with ViewLoadPaths [Josh Peek] diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 0cf143210d..8ae73a66f4 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -404,15 +404,6 @@ module ActionController #:nodoc: end alias xhr :xml_http_request - def follow_redirect - redirected_controller = @response.redirected_to[:controller] - if redirected_controller && redirected_controller != @controller.controller_name - raise "Can't follow redirects outside of current controller (from #{@controller.controller_name} to #{redirected_controller})" - end - - get(@response.redirected_to.delete(:action), @response.redirected_to.stringify_keys) - end - def assigns(key = nil) if key.nil? @response.template.assigns diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 7a90a9408e..eae05aacab 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -419,22 +419,6 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase assert_equal "Mr. David", @response.body end - def test_follow_redirect - process :redirect_to_action - assert_redirected_to :action => "flash_me" - - follow_redirect - assert_equal 1, @request.parameters["id"].to_i - - assert "Inconceivable!", @response.body - end - - def test_follow_redirect_outside_current_action - process :redirect_to_controller - assert_redirected_to :controller => "elsewhere", :action => "flash_me" - - assert_raises(RuntimeError, "Can't follow redirects outside of current controller (elsewhere)") { follow_redirect } - end def test_assert_redirection_fails_with_incorrect_controller process :redirect_to_controller diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 38898a1f75..b624005a57 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -566,24 +566,6 @@ XML assert_raise(RuntimeError) { ActionController::TestUploadedFile.new('non_existent_file') } end - def test_assert_follow_redirect_to_same_controller - with_foo_routing do |set| - get :redirect_to_same_controller - assert_response :redirect - assert_redirected_to :controller => 'test_test/test', :action => 'test_uri', :id => 5 - assert_nothing_raised { follow_redirect } - end - end - - def test_assert_follow_redirect_to_different_controller - with_foo_routing do |set| - get :redirect_to_different_controller - assert_response :redirect - assert_redirected_to :controller => 'fail', :id => 5 - assert_raise(RuntimeError) { follow_redirect } - end - end - def test_redirect_url_only_cares_about_location_header get :create assert_response :created -- cgit v1.2.3 From c3aaba0180f0710094d974b4ba4659bce81446df Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 23 Jun 2008 19:46:15 +0300 Subject: Simplify the implementation of assert_redirected_to to normalise the urls before comparing. Also allows for a simpler implementation of redirect_to without most of the recursion. Also allows for assert_redirected_to @some_record --- actionpack/CHANGELOG | 5 ++ .../assertions/response_assertions.rb | 96 +++++----------------- actionpack/lib/action_controller/base.rb | 32 ++++---- .../test/controller/action_pack_assertions_test.rb | 10 +-- actionpack/test/controller/components_test.rb | 2 +- actionpack/test/controller/redirect_test.rb | 19 +---- 6 files changed, 50 insertions(+), 114 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 7f275bef27..a99811c715 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -18,6 +18,11 @@ * Made ActionView::Base#render_file private [Josh Peek] +* Refactor and simplify the implementation of assert_redirected_to. Arguments are now normalised relative to the controller being tested, not the root of the application. [Michael Koziarski] + + This could cause some erroneous test failures if you were redirecting between controllers + in different namespaces and wrote your assertions relative to the root of the application. + * Remove follow_redirect from controller functional tests. If you want to follow redirects you can use integration tests. The functional test diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index 3deda0b45a..3ecaee641f 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -56,75 +56,18 @@ module ActionController # # assert that the redirection was to the named route login_url # assert_redirected_to login_url # + # # assert that the redirection was to the url for @customer + # assert_redirected_to @customer + # def assert_redirected_to(options = {}, message=nil) clean_backtrace do assert_response(:redirect, message) return true if options == @response.redirected_to - ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? - - begin - url = {} - original = { :expected => options, :actual => @response.redirected_to.is_a?(Symbol) ? @response.redirected_to : @response.redirected_to.dup } - original.each do |key, value| - if value.is_a?(Symbol) - value = @controller.respond_to?(value, true) ? @controller.send(value) : @controller.send("hash_for_#{value}_url") - end - - unless value.is_a?(Hash) - request = case value - when NilClass then nil - when /^\w+:\/\// then recognized_request_for(%r{^(\w+://.*?(/|$|\?))(.*)$} =~ value ? $3 : nil) - else recognized_request_for(value) - end - value = request.path_parameters if request - end - - if value.is_a?(Hash) # stringify 2 levels of hash keys - if name = value.delete(:use_route) - route = ActionController::Routing::Routes.named_routes[name] - value.update(route.parameter_shell) - end - - value.stringify_keys! - value.values.select { |v| v.is_a?(Hash) }.collect { |v| v.stringify_keys! } - if key == :expected && value['controller'] == @controller.controller_name && original[:actual].is_a?(Hash) - original[:actual].stringify_keys! - value.delete('controller') if original[:actual]['controller'].nil? || original[:actual]['controller'] == value['controller'] - end - end - - if value.respond_to?(:[]) && value['controller'] - value['controller'] = value['controller'].to_s - if key == :actual && value['controller'].first != '/' && !value['controller'].include?('/') - new_controller_path = ActionController::Routing.controller_relative_to(value['controller'], @controller.class.controller_path) - value['controller'] = new_controller_path if value['controller'] != new_controller_path && ActionController::Routing.possible_controllers.include?(new_controller_path) && @response.redirected_to.is_a?(Hash) - end - value['controller'] = value['controller'][1..-1] if value['controller'].first == '/' # strip leading hash - end - url[key] = value - end - - @response_diff = url[:actual].diff(url[:expected]) if url[:actual] - msg = build_message(message, "expected a redirect to , found one to , a difference of ", url[:expected], url[:actual], @response_diff) - - assert_block(msg) do - url[:expected].keys.all? do |k| - if k == :controller then url[:expected][k] == ActionController::Routing.controller_relative_to(url[:actual][k], @controller.class.controller_path) - else parameterize(url[:expected][k]) == parameterize(url[:actual][k]) - end - end - end - rescue ActionController::RoutingError # routing failed us, so match the strings only. - msg = build_message(message, "expected a redirect to , found one to ", options, @response.redirect_url) - url_regexp = %r{^(\w+://.*?(/|$|\?))(.*)$} - eurl, epath, url, path = [options, @response.redirect_url].collect do |url| - u, p = (url_regexp =~ url) ? [$1, $3] : [nil, url] - [u, (p.first == '/') ? p : '/' + p] - end.flatten + redirected_to_after_normalisation = normalize_argument_to_redirection(@response.redirected_to) + options_after_normalisation = normalize_argument_to_redirection(options) - assert_equal(eurl, url, msg) if eurl && url - assert_equal(epath, path, msg) if epath && path - end + 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}>" end end @@ -150,23 +93,24 @@ module ActionController end private - # Recognizes the route for a given path. - def recognized_request_for(path, request_method = nil) - path = "/#{path}" unless path.first == '/' - - # Assume given controller - request = ActionController::TestRequest.new({}, {}, nil) - request.env["REQUEST_METHOD"] = request_method.to_s.upcase if request_method - request.path = path - - ActionController::Routing::Routes.recognize(request) - request - end # Proxy to to_param if the object will respond to it. def parameterize(value) value.respond_to?(:to_param) ? value.to_param : value end + + def normalize_argument_to_redirection(fragment) + after_routing = @controller.url_for(fragment) + if after_routing =~ %r{^\w+://.*} + after_routing + else + # FIXME - this should probably get removed. + if after_routing.first != '/' + after_routing = '/' + after_routing + end + @request.protocol + @request.host_with_port + after_routing + end + end end end end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index a4bade1bd5..58eb5cabcd 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1042,29 +1042,31 @@ module ActionController #:nodoc: status = 302 end + response.redirected_to= options + logger.info("Redirected to #{options}") if logger && logger.info? + case options when %r{^\w+://.*} - raise DoubleRenderError if performed? - logger.info("Redirected to #{options}") if logger && logger.info? - response.redirect(options, interpret_status(status)) - response.redirected_to = options - @performed_redirect = true - + redirect_to_full_url(options, status) when String - redirect_to(request.protocol + request.host_with_port + options, :status=>status) - + redirect_to_full_url(request.protocol + request.host_with_port + options, status) when :back - request.env["HTTP_REFERER"] ? redirect_to(request.env["HTTP_REFERER"], :status=>status) : raise(RedirectBackError) - - when Hash - redirect_to(url_for(options), :status=>status) - response.redirected_to = options - + if referer = request.headers["Referer"] + redirect_to(referer, :status=>status) + else + raise RedirectBackError + end else - redirect_to(url_for(options), :status=>status) + redirect_to_full_url(url_for(options), status) end end + def redirect_to_full_url(url, status) + raise DoubleRenderError if performed? + response.redirect(url, interpret_status(status)) + @performed_redirect = true + end + # Sets a HTTP 1.1 Cache-Control header. Defaults to issuing a "private" instruction, so that # intermediate caches shouldn't cache the response. # diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index eae05aacab..f5cda99977 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -232,7 +232,6 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase process :redirect_to_named_route assert_redirected_to 'http://test.host/route_one' assert_redirected_to route_one_url - assert_redirected_to :route_one_url end end @@ -253,9 +252,6 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase assert_raise(Test::Unit::AssertionFailedError) do assert_redirected_to route_two_url end - assert_raise(Test::Unit::AssertionFailedError) do - assert_redirected_to :route_two_url - end end end @@ -432,14 +428,16 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase assert_redirected_to :controller => 'action_pack_assertions', :action => "flash_me", :id => 1, :params => { :panda => 'fun' } end - def test_redirected_to_url_leadling_slash + def test_redirected_to_url_leading_slash process :redirect_to_path assert_redirected_to '/some/path' end + def test_redirected_to_url_no_leadling_slash process :redirect_to_path assert_redirected_to 'some/path' end + def test_redirected_to_url_full_url process :redirect_to_path assert_redirected_to 'http://test.host/some/path' @@ -459,7 +457,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase def test_redirected_to_with_nested_controller @controller = Admin::InnerModuleController.new get :redirect_to_absolute_controller - assert_redirected_to :controller => 'content' + assert_redirected_to :controller => '/content' get :redirect_to_fellow_controller assert_redirected_to :controller => 'admin/user' diff --git a/actionpack/test/controller/components_test.rb b/actionpack/test/controller/components_test.rb index 82c55483dd..71e8a18071 100644 --- a/actionpack/test/controller/components_test.rb +++ b/actionpack/test/controller/components_test.rb @@ -119,7 +119,7 @@ class ComponentsTest < Test::Unit::TestCase def test_component_redirect_redirects get :calling_redirected - assert_redirected_to :action => "being_called" + assert_redirected_to :controller=>"callee", :action => "being_called" end def test_component_multiple_redirect_redirects diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 0e85347bad..8b72426d10 100755 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -168,21 +168,6 @@ class RedirectTest < Test::Unit::TestCase assert_redirected_to :action => "other_host", :only_path => false, :host => 'other.test.host' end - def test_redirect_error_with_pretty_diff - get :host_redirect - assert_response :redirect - begin - assert_redirected_to :action => "other_host", :only_path => true - rescue Test::Unit::AssertionFailedError => err - expected_msg, redirection_msg, diff_msg = err.message.scan(/<\{[^\}]+\}>/).collect { |s| s[2..-3] } - assert_match %r("only_path"=>false), redirection_msg - assert_match %r("host"=>"other.test.host"), redirection_msg - assert_match %r("action"=>"other_host"), redirection_msg - assert_match %r("only_path"=>false), diff_msg - assert_match %r("host"=>"other.test.host"), diff_msg - end - end - def test_module_redirect get :module_redirect assert_response :redirect @@ -235,9 +220,11 @@ class RedirectTest < Test::Unit::TestCase get :redirect_to_existing_record assert_equal "http://test.host/workshops/5", redirect_to_url + assert_redirected_to Workshop.new(5, false) get :redirect_to_new_record assert_equal "http://test.host/workshops", redirect_to_url + assert_redirected_to Workshop.new(5, true) end def test_redirect_to_nil @@ -283,7 +270,7 @@ module ModuleTest def test_module_redirect_using_options get :module_redirect assert_response :redirect - assert_redirected_to :controller => 'redirect', :action => "hello_world" + assert_redirected_to :controller => '/redirect', :action => "hello_world" end end end -- cgit v1.2.3 From 271f5b655fb58b08a89a144aa835f7b70617c399 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 4 Jul 2008 12:51:16 -0700 Subject: Fix rdoc for Filters::ClassMethods --- actionpack/lib/action_controller/filters.rb | 389 ++++++++++++++-------------- 1 file changed, 194 insertions(+), 195 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 60d92d9b98..155d8b480a 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -7,6 +7,200 @@ module ActionController #:nodoc: end end + class FilterChain < ActiveSupport::Callbacks::CallbackChain #:nodoc: + def append_filter_to_chain(filters, filter_type, &block) + pos = find_filter_append_position(filters, filter_type) + update_filter_chain(filters, filter_type, pos, &block) + end + + def prepend_filter_to_chain(filters, filter_type, &block) + pos = find_filter_prepend_position(filters, filter_type) + update_filter_chain(filters, filter_type, pos, &block) + end + + def create_filters(filters, filter_type, &block) + filters, conditions = extract_options(filters, &block) + filters.map! { |filter| find_or_create_filter(filter, filter_type, conditions) } + filters + end + + def skip_filter_in_chain(*filters, &test) + filters, conditions = extract_options(filters) + filters.each do |filter| + if callback = find(filter) then delete(callback) end + end if conditions.empty? + update_filter_in_chain(filters, :skip => conditions, &test) + end + + private + def update_filter_chain(filters, filter_type, pos, &block) + new_filters = create_filters(filters, filter_type, &block) + insert(pos, new_filters).flatten! + end + + def find_filter_append_position(filters, filter_type) + # appending an after filter puts it at the end of the call chain + # before and around filters go before the first after filter in the chain + unless filter_type == :after + each_with_index do |f,i| + return i if f.after? + end + end + return -1 + end + + def find_filter_prepend_position(filters, filter_type) + # prepending a before or around filter puts it at the front of the call chain + # after filters go before the first after filter in the chain + if filter_type == :after + each_with_index do |f,i| + return i if f.after? + end + return -1 + end + return 0 + end + + def find_or_create_filter(filter, filter_type, options = {}) + update_filter_in_chain([filter], options) + + if found_filter = find(filter) { |f| f.type == filter_type } + found_filter + else + filter_kind = case + when filter.respond_to?(:before) && filter_type == :before + :before + when filter.respond_to?(:after) && filter_type == :after + :after + else + :filter + end + + case filter_type + when :before + BeforeFilter.new(filter_kind, filter, options) + when :after + AfterFilter.new(filter_kind, filter, options) + else + AroundFilter.new(filter_kind, filter, options) + end + end + end + + def update_filter_in_chain(filters, options, &test) + filters.map! { |f| block_given? ? find(f, &test) : find(f) } + filters.compact! + + map! do |filter| + if filters.include?(filter) + new_filter = filter.dup + new_filter.options.merge!(options) + new_filter + else + filter + end + end + end + end + + class Filter < ActiveSupport::Callbacks::Callback #:nodoc: + def before? + self.class == BeforeFilter + end + + def after? + self.class == AfterFilter + end + + def around? + self.class == AroundFilter + end + + private + def should_not_skip?(controller) + if options[:skip] + !included_in_action?(controller, options[:skip]) + else + true + end + end + + def included_in_action?(controller, options) + if options[:only] + Array(options[:only]).map(&:to_s).include?(controller.action_name) + elsif options[:except] + !Array(options[:except]).map(&:to_s).include?(controller.action_name) + else + true + end + end + + def should_run_callback?(controller) + should_not_skip?(controller) && included_in_action?(controller, options) && super + end + end + + class AroundFilter < Filter #:nodoc: + def type + :around + end + + def call(controller, &block) + if should_run_callback?(controller) + method = filter_responds_to_before_and_after? ? around_proc : self.method + + # For around_filter do |controller, action| + if method.is_a?(Proc) && method.arity == 2 + evaluate_method(method, controller, block) + else + evaluate_method(method, controller, &block) + end + else + block.call + end + end + + private + def filter_responds_to_before_and_after? + method.respond_to?(:before) && method.respond_to?(:after) + end + + def around_proc + Proc.new do |controller, action| + method.before(controller) + + if controller.send!(:performed?) + controller.send!(:halt_filter_chain, method, :rendered_or_redirected) + else + begin + action.call + ensure + method.after(controller) + end + end + end + end + end + + class BeforeFilter < Filter #:nodoc: + def type + :before + end + + def call(controller, &block) + super + if controller.send!(:performed?) + controller.send!(:halt_filter_chain, method, :rendered_or_redirected) + end + end + end + + class AfterFilter < Filter #:nodoc: + def type + :after + end + end + # Filters enable controllers to run shared pre- and post-processing code for its actions. These filters can be used to do # authentication, caching, or auditing before the intended action is performed. Or to do localization or output # compression after the action has been performed. Filters have access to the request, response, and all the instance @@ -245,201 +439,6 @@ module ActionController #:nodoc: # filter and controller action will not be run. If +before+ renders or redirects, # the second half of +around+ and will still run but +after+ and the # action will not. If +around+ fails to yield, +after+ will not be run. - - class FilterChain < ActiveSupport::Callbacks::CallbackChain #:nodoc: - def append_filter_to_chain(filters, filter_type, &block) - pos = find_filter_append_position(filters, filter_type) - update_filter_chain(filters, filter_type, pos, &block) - end - - def prepend_filter_to_chain(filters, filter_type, &block) - pos = find_filter_prepend_position(filters, filter_type) - update_filter_chain(filters, filter_type, pos, &block) - end - - def create_filters(filters, filter_type, &block) - filters, conditions = extract_options(filters, &block) - filters.map! { |filter| find_or_create_filter(filter, filter_type, conditions) } - filters - end - - def skip_filter_in_chain(*filters, &test) - filters, conditions = extract_options(filters) - filters.each do |filter| - if callback = find(filter) then delete(callback) end - end if conditions.empty? - update_filter_in_chain(filters, :skip => conditions, &test) - end - - private - def update_filter_chain(filters, filter_type, pos, &block) - new_filters = create_filters(filters, filter_type, &block) - insert(pos, new_filters).flatten! - end - - def find_filter_append_position(filters, filter_type) - # appending an after filter puts it at the end of the call chain - # before and around filters go before the first after filter in the chain - unless filter_type == :after - each_with_index do |f,i| - return i if f.after? - end - end - return -1 - end - - def find_filter_prepend_position(filters, filter_type) - # prepending a before or around filter puts it at the front of the call chain - # after filters go before the first after filter in the chain - if filter_type == :after - each_with_index do |f,i| - return i if f.after? - end - return -1 - end - return 0 - end - - def find_or_create_filter(filter, filter_type, options = {}) - update_filter_in_chain([filter], options) - - if found_filter = find(filter) { |f| f.type == filter_type } - found_filter - else - filter_kind = case - when filter.respond_to?(:before) && filter_type == :before - :before - when filter.respond_to?(:after) && filter_type == :after - :after - else - :filter - end - - case filter_type - when :before - BeforeFilter.new(filter_kind, filter, options) - when :after - AfterFilter.new(filter_kind, filter, options) - else - AroundFilter.new(filter_kind, filter, options) - end - end - end - - def update_filter_in_chain(filters, options, &test) - filters.map! { |f| block_given? ? find(f, &test) : find(f) } - filters.compact! - - map! do |filter| - if filters.include?(filter) - new_filter = filter.dup - new_filter.options.merge!(options) - new_filter - else - filter - end - end - end - end - - class Filter < ActiveSupport::Callbacks::Callback #:nodoc: - def before? - self.class == BeforeFilter - end - - def after? - self.class == AfterFilter - end - - def around? - self.class == AroundFilter - end - - private - def should_not_skip?(controller) - if options[:skip] - !included_in_action?(controller, options[:skip]) - else - true - end - end - - def included_in_action?(controller, options) - if options[:only] - Array(options[:only]).map(&:to_s).include?(controller.action_name) - elsif options[:except] - !Array(options[:except]).map(&:to_s).include?(controller.action_name) - else - true - end - end - - def should_run_callback?(controller) - should_not_skip?(controller) && included_in_action?(controller, options) && super - end - end - - class AroundFilter < Filter #:nodoc: - def type - :around - end - - def call(controller, &block) - if should_run_callback?(controller) - method = filter_responds_to_before_and_after? ? around_proc : self.method - - # For around_filter do |controller, action| - if method.is_a?(Proc) && method.arity == 2 - evaluate_method(method, controller, block) - else - evaluate_method(method, controller, &block) - end - else - block.call - end - end - - private - def filter_responds_to_before_and_after? - method.respond_to?(:before) && method.respond_to?(:after) - end - - def around_proc - Proc.new do |controller, action| - method.before(controller) - - if controller.send!(:performed?) - controller.send!(:halt_filter_chain, method, :rendered_or_redirected) - else - begin - action.call - ensure - method.after(controller) - end - end - end - end - end - - class BeforeFilter < Filter #:nodoc: - def type - :before - end - - def call(controller, &block) - super - if controller.send!(:performed?) - controller.send!(:halt_filter_chain, method, :rendered_or_redirected) - end - end - end - - class AfterFilter < Filter #:nodoc: - def type - :after - end - end - module ClassMethods # The passed filters will be appended to the filter_chain and # will execute before the action on this controller is performed. -- cgit v1.2.3 From 1dcc59121b9f0c332f6fe93f90fb028ff3448899 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 12:05:50 -0500 Subject: Renamed Renderer to Renderable --- actionpack/lib/action_view.rb | 2 +- actionpack/lib/action_view/inline_template.rb | 2 +- actionpack/lib/action_view/renderable.rb | 29 +++++++++++++++++++++++++++ actionpack/lib/action_view/renderer.rb | 29 --------------------------- actionpack/lib/action_view/template.rb | 2 +- 5 files changed, 32 insertions(+), 32 deletions(-) create mode 100644 actionpack/lib/action_view/renderable.rb delete mode 100644 actionpack/lib/action_view/renderer.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 49768fe264..8f928d6fdf 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -25,7 +25,7 @@ require 'action_view/template_handlers' require 'action_view/template_file' require 'action_view/view_load_paths' -require 'action_view/renderer' +require 'action_view/renderable' require 'action_view/template' require 'action_view/partial_template' require 'action_view/inline_template' diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 1ccb23a3fc..49147901a1 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -1,6 +1,6 @@ module ActionView #:nodoc: class InlineTemplate #:nodoc: - include Renderer + include Renderable def initialize(view, source, locals = {}, type = nil) @view = view diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb new file mode 100644 index 0000000000..61bd8140fb --- /dev/null +++ b/actionpack/lib/action_view/renderable.rb @@ -0,0 +1,29 @@ +module ActionView + module Renderable + # TODO: Local assigns should not be tied to template instance + attr_accessor :locals + + # TODO: These readers should be private + attr_reader :filename, :source, :handler, :method_key, :method + + def render + prepare! + @handler.render(self) + end + + private + def prepare! + unless @prepared + @view.send(:evaluate_assigns) + @view.current_render_extension = @extension + + if @handler.compilable? + @handler.compile_template(self) # compile the given template, if necessary + @method = @view.method_names[method_key] # Set the method name for this template and run it + end + + @prepared = true + end + end + end +end diff --git a/actionpack/lib/action_view/renderer.rb b/actionpack/lib/action_view/renderer.rb deleted file mode 100644 index e6c64d2749..0000000000 --- a/actionpack/lib/action_view/renderer.rb +++ /dev/null @@ -1,29 +0,0 @@ -module ActionView - module Renderer - # TODO: Local assigns should not be tied to template instance - attr_accessor :locals - - # TODO: These readers should be private - attr_reader :filename, :source, :handler, :method_key, :method - - def render - prepare! - @handler.render(self) - end - - private - def prepare! - unless @prepared - @view.send(:evaluate_assigns) - @view.current_render_extension = @extension - - if @handler.compilable? - @handler.compile_template(self) # compile the given template, if necessary - @method = @view.method_names[method_key] # Set the method name for this template and run it - end - - @prepared = true - end - end - end -end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index dbc8ccdee8..f696ea366d 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,7 +1,7 @@ module ActionView #:nodoc: class Template #:nodoc: extend TemplateHandlers - include Renderer + include Renderable attr_reader :path, :extension -- cgit v1.2.3 From 39ba2da82bcc2f9fad494e6ac0a66a3387ab8ee2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 16:27:43 -0500 Subject: Moved complied method name logic into Renderable --- actionpack/lib/action_view/inline_template.rb | 10 +++++++++ actionpack/lib/action_view/renderable.rb | 4 ++++ actionpack/lib/action_view/template.rb | 9 +++++++- .../action_view/template_handlers/compilable.rb | 24 ++-------------------- 4 files changed, 24 insertions(+), 23 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 49147901a1..fb5e4408db 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -2,6 +2,10 @@ module ActionView #:nodoc: class InlineTemplate #:nodoc: include Renderable + # Count the number of inline templates + cattr_accessor :inline_template_count + @@inline_template_count = 0 + def initialize(view, source, locals = {}, type = nil) @view = view @@ -12,5 +16,11 @@ module ActionView #:nodoc: @method_key = @source @handler = Template.handler_class_for_extension(@extension).new(@view) end + + private + # FIXME: Modifying this shared variable may not thread safe + def method_name_path_segment + "inline_#{@@inline_template_count += 1}" + end end end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 61bd8140fb..d73dc3e2dc 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -11,6 +11,10 @@ module ActionView @handler.render(self) end + def method_name + ['_run', @extension, method_name_path_segment].compact.join('_').to_sym + end + private def prepare! unless @prepared diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index f696ea366d..56d740f478 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -37,7 +37,7 @@ module ActionView #:nodoc: end def source - @source ||= File.read(self.filename) + @source ||= File.read(@filename) end def base_path_for_exception @@ -71,5 +71,12 @@ module ActionView #:nodoc: template_type = (@original_path =~ /layouts/i) ? 'layout' : 'template' raise MissingTemplate, "Missing #{template_type} #{full_template_path} in view path #{display_paths}" end + + def method_name_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 + end end end diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index f436ebbe45..d95a5c8419 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -1,7 +1,6 @@ module ActionView module TemplateHandlers module Compilable - def self.included(base) base.extend ClassMethod @@ -12,10 +11,6 @@ module ActionView # Map method names to the names passed in local assigns so far base.cattr_accessor :template_args base.template_args = {} - - # Count the number of inline templates - base.cattr_accessor :inline_template_count - base.inline_template_count = 0 end module ClassMethod @@ -24,7 +19,7 @@ module ActionView true end end - + def render(template) @view.send :execute, template end @@ -75,22 +70,7 @@ module ActionView end def assign_method_name(template) - @view.method_names[template.method_key] ||= compiled_method_name(template) - end - - def compiled_method_name(template) - ['_run', self.class.to_s.demodulize.underscore, compiled_method_name_file_path_segment(template.filename)].compact.join('_').to_sym - end - - def compiled_method_name_file_path_segment(file_name) - if file_name - s = File.expand_path(file_name) - s.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}/, '') if defined?(RAILS_ROOT) - s.gsub!(/([^a-zA-Z0-9_])/) { $1.ord } - s - else - (self.inline_template_count += 1).to_s - end + @view.method_names[template.method_key] ||= template.method_name end # Method to create the source code for a given template. -- cgit v1.2.3 From cd6fe831526d84ae40e425cadbf22f42a375de2a Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 16:34:51 -0500 Subject: Ensure all complied method names are cleaned up in the error backtrace --- actionpack/lib/action_view/template_error.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template_error.rb b/actionpack/lib/action_view/template_error.rb index 65d80362b5..a1be1a8833 100644 --- a/actionpack/lib/action_view/template_error.rb +++ b/actionpack/lib/action_view/template_error.rb @@ -105,6 +105,6 @@ module ActionView end if defined?(Exception::TraceSubstitutions) - Exception::TraceSubstitutions << [/:in\s+`_run_(html|xml).*'\s*$/, ''] + Exception::TraceSubstitutions << [/:in\s+`_run_.*'\s*$/, ''] Exception::TraceSubstitutions << [%r{^\s*#{Regexp.escape RAILS_ROOT}/}, ''] if defined?(RAILS_ROOT) end -- cgit v1.2.3 From 27f382641c9ec945a1d657f5efba6a06296dca54 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 17:31:57 -0500 Subject: Deprecated TemplateHandler line offset --- actionpack/CHANGELOG | 2 + actionpack/lib/action_view/template_handler.rb | 8 -- .../lib/action_view/template_handlers/builder.rb | 12 +-- .../action_view/template_handlers/compilable.rb | 101 ++++++++++----------- .../lib/action_view/template_handlers/rjs.rb | 8 +- 5 files changed, 57 insertions(+), 74 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index a99811c715..7445ffda98 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Deprecated TemplateHandler line offset [Josh Peek] + * Allow caches_action to accept cache store options. #416. [José Valim]. Example: caches_action :index, :redirected, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour diff --git a/actionpack/lib/action_view/template_handler.rb b/actionpack/lib/action_view/template_handler.rb index 39e578e586..78586d6142 100644 --- a/actionpack/lib/action_view/template_handler.rb +++ b/actionpack/lib/action_view/template_handler.rb @@ -1,9 +1,5 @@ module ActionView class TemplateHandler - def self.line_offset - 0 - end - def self.compilable? false end @@ -22,10 +18,6 @@ module ActionView self.class.compilable? end - def line_offset - self.class.line_offset - end - # Called by CacheHelper#cache def cache_fragment(block, name = {}, options = nil) end diff --git a/actionpack/lib/action_view/template_handlers/builder.rb b/actionpack/lib/action_view/template_handlers/builder.rb index ee02ce1a6f..2d413490bd 100644 --- a/actionpack/lib/action_view/template_handlers/builder.rb +++ b/actionpack/lib/action_view/template_handlers/builder.rb @@ -5,17 +5,11 @@ module ActionView class Builder < TemplateHandler include Compilable - def self.line_offset - 2 - end - def compile(template) - content_type_handler = (@view.send!(:controller).respond_to?(:response) ? "controller.response" : "controller") - - "#{content_type_handler}.content_type ||= Mime::XML\n" + - "xml = ::Builder::XmlMarkup.new(:indent => 2)\n" + + "controller.response.content_type ||= Mime::XML;" + + "xml = ::Builder::XmlMarkup.new(:indent => 2);" + template.source + - "\nxml.target!\n" + ";xml.target!;" end def cache_fragment(block, name = {}, options = nil) diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index d95a5c8419..5278320d1d 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -30,11 +30,10 @@ module ActionView render_symbol = assign_method_name(template) render_source = create_template_source(template, render_symbol) - line_offset = self.template_args[render_symbol].size + self.line_offset begin file_name = template.filename || 'compiled-template' - ActionView::Base::CompiledTemplates.module_eval(render_source, file_name, -line_offset) + ActionView::Base::CompiledTemplates.module_eval(render_source, file_name, 0) rescue Exception => e # errors from template code if Base.logger Base.logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" @@ -50,65 +49,65 @@ module ActionView 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, - # or if the file has changed on disk and checking file mods hasn't been disabled. - def compile_template?(template) - method_key = template.method_key - render_symbol = @view.method_names[method_key] - - compile_time = self.compile_time[render_symbol] - if compile_time && supports_local_assigns?(render_symbol, template.locals) - if template.filename && !@view.cache_template_loading - template_changed_since?(template.filename, compile_time) + # 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, + # or if the file has changed on disk and checking file mods hasn't been disabled. + def compile_template?(template) + method_key = template.method_key + render_symbol = @view.method_names[method_key] + + compile_time = self.compile_time[render_symbol] + if compile_time && supports_local_assigns?(render_symbol, template.locals) + if template.filename && !@view.cache_template_loading + template_changed_since?(template.filename, compile_time) + end + else + true end - else - true end - end - def assign_method_name(template) - @view.method_names[template.method_key] ||= template.method_name - end - - # Method to create the source code for a given template. - def create_template_source(template, render_symbol) - body = compile(template) + def assign_method_name(template) + @view.method_names[template.method_key] ||= template.method_name + end - self.template_args[render_symbol] ||= {} - locals_keys = self.template_args[render_symbol].keys | template.locals.keys - self.template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h } + # Method to create the source code for a given template. + def create_template_source(template, render_symbol) + body = compile(template) - locals_code = "" - locals_keys.each do |key| - locals_code << "#{key} = local_assigns[:#{key}]\n" - end + self.template_args[render_symbol] ||= {} + locals_keys = self.template_args[render_symbol].keys | template.locals.keys + self.template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h } - <<-end_src - def #{render_symbol}(local_assigns) - old_output_buffer = output_buffer;#{locals_code}#{body} - ensure - self.output_buffer = old_output_buffer + locals_code = "" + locals_keys.each do |key| + locals_code << "#{key} = local_assigns[:#{key}];" end - end_src - end - # Return true if the given template was compiled for a superset of the keys in local_assigns - def supports_local_assigns?(render_symbol, local_assigns) - local_assigns.empty? || - ((args = self.template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) }) - end + source = <<-end_src + def #{render_symbol}(local_assigns) + old_output_buffer = output_buffer;#{locals_code};#{body} + ensure + self.output_buffer = old_output_buffer + end + end_src - # Method to handle checking a whether a template has changed since last compile; isolated so that templates - # not stored on the file system can hook and extend appropriately. - def template_changed_since?(file_name, compile_time) - lstat = File.lstat(file_name) - compile_time < lstat.mtime || - (lstat.symlink? && compile_time < File.stat(file_name).mtime) - end + return source + end + # Return true if the given template was compiled for a superset of the keys in local_assigns + def supports_local_assigns?(render_symbol, local_assigns) + local_assigns.empty? || + ((args = self.template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) }) + end + + # Method to handle checking a whether a template has changed since last compile; isolated so that templates + # not stored on the file system can hook and extend appropriately. + def template_changed_since?(file_name, compile_time) + lstat = File.lstat(file_name) + compile_time < lstat.mtime || + (lstat.symlink? && compile_time < File.stat(file_name).mtime) + end end end end diff --git a/actionpack/lib/action_view/template_handlers/rjs.rb b/actionpack/lib/action_view/template_handlers/rjs.rb index 5854e33fed..31ce598c46 100644 --- a/actionpack/lib/action_view/template_handlers/rjs.rb +++ b/actionpack/lib/action_view/template_handlers/rjs.rb @@ -3,13 +3,9 @@ module ActionView class RJS < TemplateHandler include Compilable - def self.line_offset - 2 - end - def compile(template) - "controller.response.content_type ||= Mime::JS\n" + - "update_page do |page|\n#{template.source}\nend" + "controller.response.content_type ||= Mime::JS;" + + "update_page do |page|;#{template.source};end" end def cache_fragment(block, name = {}, options = nil) #:nodoc: -- cgit v1.2.3 From 5a3bc6f12f7ca78d7ce569f6541d691e42fac6f8 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 17:40:39 -0500 Subject: Removed unused template_args variable --- actionpack/lib/action_view/base.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index d18c701619..4fbe6ac674 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -192,8 +192,6 @@ module ActionView #:nodoc: # Maps inline templates to their method names cattr_accessor :method_names @@method_names = {} - # Map method names to the names passed in local assigns so far - @@template_args = {} # Cache public asset paths cattr_reader :computed_public_paths -- cgit v1.2.3 From f22ae15a8e30f7ad475acdbcfcd1120e498cfede Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 17:49:49 -0500 Subject: Use the inline template's hash as a method key instead of relying on a counter --- actionpack/lib/action_view/inline_template.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index fb5e4408db..965df96e3b 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -2,10 +2,6 @@ module ActionView #:nodoc: class InlineTemplate #:nodoc: include Renderable - # Count the number of inline templates - cattr_accessor :inline_template_count - @@inline_template_count = 0 - def initialize(view, source, locals = {}, type = nil) @view = view @@ -13,14 +9,13 @@ module ActionView #:nodoc: @extension = type @locals = locals || {} - @method_key = @source + @method_key = "inline_#{@source.hash.abs}" @handler = Template.handler_class_for_extension(@extension).new(@view) end private - # FIXME: Modifying this shared variable may not thread safe def method_name_path_segment - "inline_#{@@inline_template_count += 1}" + @method_key end end end -- cgit v1.2.3 From ce5d958f8fe878465c0d2142991a2945ca8d3cd1 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 18:35:52 -0500 Subject: 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 --- actionpack/CHANGELOG | 2 + actionpack/lib/action_view/base.rb | 2 +- .../action_view/template_handlers/compilable.rb | 43 ++++++++-------------- 3 files changed, 18 insertions(+), 29 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 7445ffda98..7d2ed7538d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* 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] + * Deprecated TemplateHandler line offset [Josh Peek] * Allow caches_action to accept cache store options. #416. [José Valim]. Example: diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 4fbe6ac674..649bd4dd5b 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -165,7 +165,7 @@ module ActionView #:nodoc: delegate :erb_trim_mode=, :to => 'ActionView::TemplateHandlers::ERB' end - # Specify whether file modification times should be checked to see if a template needs recompilation + # Specify whether templates should be cached. Otherwise the file we be read everytime it is accessed. @@cache_template_loading = false cattr_accessor :cache_template_loading diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index 5278320d1d..887c783537 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -4,10 +4,6 @@ module ActionView def self.included(base) base.extend ClassMethod - # Map method names to their compile time - base.cattr_accessor :compile_time - base.compile_time = {} - # Map method names to the names passed in local assigns so far base.cattr_accessor :template_args base.template_args = {} @@ -26,7 +22,7 @@ module ActionView # Compile and evaluate the template's code def compile_template(template) - return unless compile_template?(template) + return false unless compile_template?(template) render_symbol = assign_method_name(template) render_source = create_template_source(template, render_symbol) @@ -43,28 +39,27 @@ module ActionView raise ActionView::TemplateError.new(template, @view.assigns, e) end - - self.compile_time[render_symbol] = Time.now - # logger.debug "Compiled template #{file_name || template}\n ==> #{render_symbol}" if logger 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, - # or if the file has changed on disk and checking file mods hasn't been disabled. + # if local_assigns has a new key, which isn't supported by the compiled code yet. def compile_template?(template) - method_key = template.method_key - render_symbol = @view.method_names[method_key] + # Unless the template has been complied yet, compile + return true unless render_symbol = @view.method_names[template.method_key] - compile_time = self.compile_time[render_symbol] - if compile_time && supports_local_assigns?(render_symbol, template.locals) - if template.filename && !@view.cache_template_loading - template_changed_since?(template.filename, compile_time) - end - else - true - end + # If template caching is disabled, compile + return true unless Base.cache_template_loading + + # Always recompile inline templates + return true if template.is_a?(InlineTemplate) + + # Unless local assigns support, recompile + return true unless supports_local_assigns?(render_symbol, template.locals) + + # Otherwise, use compiled method + return false end def assign_method_name(template) @@ -100,14 +95,6 @@ module ActionView local_assigns.empty? || ((args = self.template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) }) end - - # Method to handle checking a whether a template has changed since last compile; isolated so that templates - # not stored on the file system can hook and extend appropriately. - def template_changed_since?(file_name, compile_time) - lstat = File.lstat(file_name) - compile_time < lstat.mtime || - (lstat.symlink? && compile_time < File.stat(file_name).mtime) - end end end end -- cgit v1.2.3 From 9828aecd2aa98910f17e4b0f52519f4727d198d8 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 5 Jul 2008 23:54:11 -0500 Subject: Lookup compiled methods in CompiledTemplates instance methods set instead of using a "methods_names" hash --- actionpack/lib/action_view/base.rb | 4 -- actionpack/lib/action_view/inline_template.rb | 7 +-- actionpack/lib/action_view/renderable.rb | 7 ++- actionpack/lib/action_view/template.rb | 4 +- .../action_view/template_handlers/compilable.rb | 58 +++++++++------------- 5 files changed, 30 insertions(+), 50 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 649bd4dd5b..92501133a6 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -189,10 +189,6 @@ module ActionView #:nodoc: end include CompiledTemplates - # Maps inline templates to their method names - cattr_accessor :method_names - @@method_names = {} - # Cache public asset paths cattr_reader :computed_public_paths @@computed_public_paths = {} diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 965df96e3b..3e25f6902d 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -9,13 +9,8 @@ module ActionView #:nodoc: @extension = type @locals = locals || {} - @method_key = "inline_#{@source.hash.abs}" + @method_segment = "inline_#{@source.hash.abs}" @handler = Template.handler_class_for_extension(@extension).new(@view) end - - private - def method_name_path_segment - @method_key - end end end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index d73dc3e2dc..2e2fff44c6 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -4,15 +4,15 @@ module ActionView attr_accessor :locals # TODO: These readers should be private - attr_reader :filename, :source, :handler, :method_key, :method + attr_reader :filename, :source, :handler def render prepare! @handler.render(self) end - def method_name - ['_run', @extension, method_name_path_segment].compact.join('_').to_sym + def method + ['_run', @extension, @method_segment].compact.join('_').to_sym end private @@ -23,7 +23,6 @@ module ActionView if @handler.compilable? @handler.compile_template(self) # compile the given template, if necessary - @method = @view.method_names[method_key] # Set the method name for this template and run it end @prepared = true diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 56d740f478..069d7d57af 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -19,7 +19,7 @@ module ActionView #:nodoc: set_extension_and_file_name - @method_key = @filename + @method_segment = compiled_method_name_file_path_segment @locals = locals || {} @handler = self.class.handler_class_for_extension(@extension).new(@view) end @@ -72,7 +72,7 @@ module ActionView #:nodoc: raise MissingTemplate, "Missing #{template_type} #{full_template_path} in view path #{display_paths}" end - def method_name_path_segment + 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 } diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index 887c783537..ed7b0e71ea 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -4,7 +4,7 @@ module ActionView def self.included(base) base.extend ClassMethod - # Map method names to the names passed in local assigns so far + # Map method names to the compiled local assigns base.cattr_accessor :template_args base.template_args = {} end @@ -17,23 +17,34 @@ module ActionView end def render(template) - @view.send :execute, template + @view.send(:execute, template) end # Compile and evaluate the template's code def compile_template(template) return false unless compile_template?(template) - render_symbol = assign_method_name(template) - render_source = create_template_source(template, render_symbol) + locals_code = "" + locals_keys = cache_template_args(template.method, template.locals) + locals_keys.each do |key| + locals_code << "#{key} = local_assigns[:#{key}];" + end + + 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(render_source, file_name, 0) - rescue Exception => e # errors from template code + ActionView::Base::CompiledTemplates.module_eval(source, file_name, 0) + rescue Exception => e # errors from template code if Base.logger - Base.logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" - Base.logger.debug "Function body: #{render_source}" + Base.logger.debug "ERROR: compiling #{template.method} RAISED #{e}" + Base.logger.debug "Function body: #{source}" Base.logger.debug "Backtrace: #{e.backtrace.join("\n")}" end @@ -47,7 +58,7 @@ module ActionView # if local_assigns has a new key, which isn't supported by the compiled code yet. def compile_template?(template) # Unless the template has been complied yet, compile - return true unless render_symbol = @view.method_names[template.method_key] + 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 @@ -56,38 +67,17 @@ module ActionView return true if template.is_a?(InlineTemplate) # Unless local assigns support, recompile - return true unless supports_local_assigns?(render_symbol, template.locals) + return true unless supports_local_assigns?(template.method, template.locals) # Otherwise, use compiled method return false end - def assign_method_name(template) - @view.method_names[template.method_key] ||= template.method_name - end - - # Method to create the source code for a given template. - def create_template_source(template, render_symbol) - body = compile(template) - + def cache_template_args(render_symbol, local_assigns) self.template_args[render_symbol] ||= {} - locals_keys = self.template_args[render_symbol].keys | template.locals.keys + locals_keys = self.template_args[render_symbol].keys | local_assigns.keys self.template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h } - - locals_code = "" - locals_keys.each do |key| - locals_code << "#{key} = local_assigns[:#{key}];" - end - - source = <<-end_src - def #{render_symbol}(local_assigns) - old_output_buffer = output_buffer;#{locals_code};#{body} - ensure - self.output_buffer = old_output_buffer - end - end_src - - return source + locals_keys end # Return true if the given template was compiled for a superset of the keys in local_assigns -- cgit v1.2.3 From 7b9e8ae2734089555e5bfdb9e9c80d92f43551a2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 6 Jul 2008 00:00:45 -0500 Subject: Synchronize template compiling --- .../action_view/template_handlers/compilable.rb | 65 +++++++++++----------- 1 file changed, 34 insertions(+), 31 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 ed7b0e71ea..d079ef3d15 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -4,9 +4,10 @@ module ActionView def self.included(base) base.extend ClassMethod + @@mutex = Mutex.new + # Map method names to the compiled local assigns - base.cattr_accessor :template_args - base.template_args = {} + @@template_args = {} end module ClassMethod @@ -22,33 +23,35 @@ module ActionView # Compile and evaluate the template's code def compile_template(template) - return false unless compile_template?(template) - - locals_code = "" - locals_keys = cache_template_args(template.method, template.locals) - locals_keys.each do |key| - locals_code << "#{key} = local_assigns[:#{key}];" - end + return false unless recompile_template?(template) - 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 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")}" + @@mutex.synchronize do + locals_code = "" + locals_keys = cache_template_args(template.method, template.locals) + locals_keys.each do |key| + locals_code << "#{key} = local_assigns[:#{key}];" end - raise ActionView::TemplateError.new(template, @view.assigns, e) + 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 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")}" + end + + raise ActionView::TemplateError.new(template, @view.assigns, e) + end end end @@ -56,7 +59,7 @@ module ActionView # 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 compile_template?(template) + def recompile_template?(template) # Unless the template has been complied yet, compile return true unless Base::CompiledTemplates.instance_methods.include?(template.method.to_s) @@ -74,16 +77,16 @@ module ActionView end def cache_template_args(render_symbol, local_assigns) - self.template_args[render_symbol] ||= {} - locals_keys = self.template_args[render_symbol].keys | local_assigns.keys - self.template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h } + @@template_args[render_symbol] ||= {} + locals_keys = @@template_args[render_symbol].keys | local_assigns.keys + @@template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h } locals_keys end # Return true if the given template was compiled for a superset of the keys in local_assigns def supports_local_assigns?(render_symbol, local_assigns) local_assigns.empty? || - ((args = self.template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) }) + ((args = @@template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) }) end end end -- cgit v1.2.3 From 1d8623b42f6249613d39e834a8742b3b143f2aff Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 6 Jul 2008 01:13:15 -0500 Subject: Added local assign keys to compiled method name so two threads evaluating the same template with different locals don't step on top of each other --- actionpack/lib/action_view/renderable.rb | 8 ++++++- actionpack/lib/action_view/template.rb | 2 +- .../action_view/template_handlers/compilable.rb | 25 +--------------------- 3 files changed, 9 insertions(+), 26 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 2e2fff44c6..3a30e603fe 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -12,7 +12,7 @@ module ActionView end def method - ['_run', @extension, @method_segment].compact.join('_').to_sym + ['_run', @extension, @method_segment, local_assigns_keys].compact.join('_').to_sym end private @@ -28,5 +28,11 @@ module ActionView @prepared = true end end + + def local_assigns_keys + if @locals && @locals.any? + "locals_#{@locals.keys.map { |k| k.to_s }.sort.join('_')}" + end + end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 069d7d57af..5e5ea9b9b9 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -20,7 +20,7 @@ module ActionView #:nodoc: set_extension_and_file_name @method_segment = compiled_method_name_file_path_segment - @locals = locals || {} + @locals = (locals && locals.dup) || {} @handler = self.class.handler_class_for_extension(@extension).new(@view) end diff --git a/actionpack/lib/action_view/template_handlers/compilable.rb b/actionpack/lib/action_view/template_handlers/compilable.rb index d079ef3d15..256d8da031 100644 --- a/actionpack/lib/action_view/template_handlers/compilable.rb +++ b/actionpack/lib/action_view/template_handlers/compilable.rb @@ -5,9 +5,6 @@ module ActionView base.extend ClassMethod @@mutex = Mutex.new - - # Map method names to the compiled local assigns - @@template_args = {} end module ClassMethod @@ -26,11 +23,7 @@ module ActionView return false unless recompile_template?(template) @@mutex.synchronize do - locals_code = "" - locals_keys = cache_template_args(template.method, template.locals) - locals_keys.each do |key| - locals_code << "#{key} = local_assigns[:#{key}];" - end + locals_code = template.locals.keys.map { |key| "#{key} = local_assigns[:#{key}];" }.join source = <<-end_src def #{template.method}(local_assigns) @@ -69,25 +62,9 @@ module ActionView # Always recompile inline templates return true if template.is_a?(InlineTemplate) - # Unless local assigns support, recompile - return true unless supports_local_assigns?(template.method, template.locals) - # Otherwise, use compiled method return false end - - def cache_template_args(render_symbol, local_assigns) - @@template_args[render_symbol] ||= {} - locals_keys = @@template_args[render_symbol].keys | local_assigns.keys - @@template_args[render_symbol] = locals_keys.inject({}) { |h, k| h[k] = true; h } - locals_keys - end - - # Return true if the given template was compiled for a superset of the keys in local_assigns - def supports_local_assigns?(render_symbol, local_assigns) - local_assigns.empty? || - ((args = @@template_args[render_symbol]) && local_assigns.all? { |k,_| args.has_key?(k) }) - end end end end -- cgit v1.2.3 From 2f4aaed7b3feb3be787a316fab3144c06bb21a27 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 27 Jun 2008 11:29:04 +0300 Subject: Disable the Accept header by default 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. Alternatively to re-enable it you need to set: config.action_controller.use_accept_header = true A special case remains for ajax requests which will have a javascript format for the base resource (/people/1) if the X-Requested-With header is present. This lets ajax pages still use format.js despite there being no params[:format] --- actionpack/CHANGELOG | 10 +++++++ actionpack/lib/action_controller/base.rb | 10 +++++++ .../lib/action_controller/caching/actions.rb | 5 +--- actionpack/lib/action_controller/mime_responds.rb | 6 +++- actionpack/lib/action_controller/request.rb | 34 ++++++++++++++++------ actionpack/lib/action_view/base.rb | 10 ++----- actionpack/test/controller/caching_test.rb | 10 ++----- actionpack/test/controller/content_type_test.rb | 14 +++++++++ actionpack/test/controller/mime_responds_test.rb | 5 ++++ actionpack/test/controller/request_test.rb | 2 +- 10 files changed, 76 insertions(+), 30 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 7d2ed7538d..f86a3b21fb 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,15 @@ *Edge* +* Disable the Accept header by default [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. + + Alternatively to re-enable it you need to set: + + config.action_controller.use_accept_header = true + * 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] * Deprecated TemplateHandler line offset [Josh Peek] diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 58eb5cabcd..c28e9005cf 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -340,6 +340,16 @@ module ActionController #:nodoc: cattr_accessor :optimise_named_routes self.optimise_named_routes = true + # 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 + # 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 + # Controls whether request forgergy protection is turned on or not. Turned off by default only in test mode. class_inheritable_accessor :allow_forgery_protection self.allow_forgery_protection = true diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 234ef8ae5b..f3535f8330 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -166,10 +166,7 @@ module ActionController #:nodoc: # If there's no extension in the path, check request.format if extension.nil? - extension = request.format.to_sym.to_s - if extension=='all' - extension = nil - end + extension = request.cache_format end extension end diff --git a/actionpack/lib/action_controller/mime_responds.rb b/actionpack/lib/action_controller/mime_responds.rb index 1dbd8b9e6f..29294476f7 100644 --- a/actionpack/lib/action_controller/mime_responds.rb +++ b/actionpack/lib/action_controller/mime_responds.rb @@ -114,7 +114,11 @@ module ActionController #:nodoc: @request = controller.request @response = controller.response - @mime_type_priority = Array(Mime::Type.lookup_by_extension(@request.parameters[:format]) || @request.accepts) + if ActionController::Base.use_accept_header + @mime_type_priority = Array(Mime::Type.lookup_by_extension(@request.parameters[:format]) || @request.accepts) + else + @mime_type_priority = [@request.format] + end @order = [] @responses = {} diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index c91a3387a0..c76a93f7a1 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -89,14 +89,23 @@ module ActionController end end - # Returns the Mime type for the format used in the request. If there is no format available, the first of the - # accept types will be used. Examples: + # Returns the Mime type for the format used in the request. # # GET /posts/5.xml | request.format => Mime::XML # GET /posts/5.xhtml | request.format => Mime::HTML - # GET /posts/5 | request.format => request.accepts.first (usually Mime::HTML for browsers) + # GET /posts/5 | request.format => Mime::HTML or MIME::JS, or request.accepts.first depending on the value of ActionController::Base.use_accept_header def format - @format ||= parameters[:format] ? Mime::Type.lookup_by_extension(parameters[:format]) : accepts.first + @format ||= begin + if parameters[:format] + Mime::Type.lookup_by_extension(parameters[:format]) + elsif ActionController::Base.use_accept_header + accepts.first + elsif xhr? + Mime::Type.lookup_by_extension("js") + else + Mime::Type.lookup_by_extension("html") + end + end end @@ -116,19 +125,26 @@ module ActionController @format = Mime::Type.lookup_by_extension(parameters[:format]) end + # Returns a symbolized version of the :format parameter of the request. + # If no format is given it returns :jsfor AJAX requests and :html + # otherwise. def template_format parameter_format = parameters[:format] - case - when parameter_format.blank? && !xhr? - :html - when parameter_format.blank? && xhr? + if parameter_format + parameter_format.to_sym + elsif xhr? :js else - parameter_format.to_sym + :html end end + def cache_format + parameter_format = parameters[:format] + parameter_format && parameter_format.to_sym + end + # Returns true if the request's "X-Requested-With" header contains # "XMLHttpRequest". (The Prototype Javascript library sends this header with # every Ajax request.) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 92501133a6..66892fdabf 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -256,13 +256,9 @@ module ActionView #:nodoc: template_path.split('/').last[0,1] != '_' end - # Returns a symbolized version of the :format parameter of the request, - # or :html by default. - # - # EXCEPTION: If the :format parameter is not set, the Accept header will be examined for - # whether it contains the JavaScript mime type as its first priority. If that's the case, - # it will be used. This ensures that Ajax applications can use the same URL to support both - # JavaScript and non-JavaScript users. + # The format to be used when choosing between multiple templates with + # the same name but differing formats. See +Request#template_format+ + # for more details. def template_format return @template_format if @template_format diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index d8a3ccb35b..7aa4e0cd93 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -131,8 +131,7 @@ class PageCachingTest < Test::Unit::TestCase end def test_page_caching_conditional_options - @request.env['HTTP_ACCEPT'] = 'application/json' - get :ok + get :ok, :format=>'json' assert_page_not_cached :ok end @@ -219,6 +218,7 @@ class ActionCachingMockController Object.new.instance_eval(<<-EVAL) def path; '#{@mock_path}' end def format; 'all' end + def cache_format; nil end self EVAL end @@ -414,12 +414,6 @@ class ActionCacheTest < Test::Unit::TestCase assert_equal 'application/xml', @response.content_type reset! - @request.env['HTTP_ACCEPT'] = "application/xml" - get :index - assert_equal cached_time, @response.body - assert_equal 'application/xml', @response.content_type - reset! - get :expire_xml reset! diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index 2019b4a2d0..8f55974172 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -114,6 +114,20 @@ class ContentTypeTest < Test::Unit::TestCase assert_equal Mime::HTML, @response.content_type assert_equal "utf-8", @response.charset end +end + +class AcceptBasedContentTypeTest < ActionController::TestCase + + tests ContentTypeController + + def setup + ActionController::Base.use_accept_header = true + end + + def tear_down + ActionController::Base.use_accept_header = false + end + def test_render_default_content_types_for_respond_to @request.env["HTTP_ACCEPT"] = Mime::HTML.to_s diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index fb2519563d..17f5e27232 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -166,6 +166,7 @@ RespondToController.view_paths = [FIXTURE_LOAD_PATH] class MimeControllerTest < Test::Unit::TestCase def setup + ActionController::Base.use_accept_header = true @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new @@ -173,6 +174,10 @@ class MimeControllerTest < Test::Unit::TestCase @request.host = "www.example.com" end + def teardown + ActionController::Base.use_accept_header = false + end + def test_html @request.env["HTTP_ACCEPT"] = "text/html" get :js_or_html diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index 20f3fd4d7b..932c0e21a1 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -386,7 +386,7 @@ class RequestTest < Test::Unit::TestCase def test_nil_format @request.instance_eval { @parameters = { :format => nil } } - @request.env["HTTP_ACCEPT"] = "text/javascript" + @request.env["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" assert_equal Mime::JS, @request.format end -- cgit v1.2.3 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( + " # # # <% form_remote_tag :url => '/posts' do -%> @@ -323,19 +323,19 @@ module ActionView options[:form] = true options[:html] ||= {} - options[:html][:onsubmit] = - (options[:html][:onsubmit] ? options[:html][:onsubmit] + "; " : "") + + options[:html][:onsubmit] = + (options[:html][:onsubmit] ? options[:html][:onsubmit] + "; " : "") + "#{remote_function(options)}; return false;" form_tag(options[:html].delete(:action) || url_for(options[:url]), options[:html], &block) end - # Creates a form that will submit using XMLHttpRequest in the background - # instead of the regular reloading POST arrangement and a scope around a + # Creates a form that will submit using XMLHttpRequest in the background + # instead of the regular reloading POST arrangement and a scope around a # specific resource that is used as a base for questioning about - # values for the fields. + # values for the fields. # - # === Resource + # === Resource # # Example: # <% remote_form_for(@post) do |f| %> @@ -348,7 +348,7 @@ module ActionView # ... # <% end %> # - # === Nested Resource + # === Nested Resource # # Example: # <% remote_form_for([@post, @comment]) do |f| %> @@ -387,23 +387,23 @@ module ActionView concat('') end alias_method :form_remote_for, :remote_form_for - + # Returns a button input tag with the element name of +name+ and a value (i.e., display text) of +value+ # that will submit form using XMLHttpRequest in the background instead of a regular POST request that - # reloads the page. + # reloads the page. # # # Create a button that submits to the create action - # # - # # Generates: # <%= 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 # # - # # Generates: # <%= button_to_remote 'update_btn', 'Update', :url => { :action => 'update' }, # :update => { :success => "succeed", :failure => "fail" } @@ -423,7 +423,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 # update return document using +update_element_function+ calls. @@ -433,11 +433,11 @@ module ActionView # Returns the JavaScript needed for a remote function. # Takes the same arguments as link_to_remote. - # + # # Example: - # # Generates: { :action => :update_options }) %>"> # # @@ -455,7 +455,7 @@ module ActionView update << "'#{options[:update]}'" end - function = update.empty? ? + function = update.empty? ? "new Ajax.Request(" : "new Ajax.Updater(#{update}, " @@ -476,9 +476,9 @@ module ActionView # callback when its contents have changed. The default callback is an # Ajax call. By default the value of the observed field is sent as a # parameter with the Ajax call. - # + # # Example: - # # Generates: new Form.Element.Observer('suggest', 0.25, function(element, value) {new Ajax.Updater('suggest', + # # Generates: new Form.Element.Observer('suggest', 0.25, function(element, value) {new Ajax.Updater('suggest', # # '/testing/find_suggestion', {asynchronous:true, evalScripts:true, parameters:'q=' + value})}) # <%= observe_field :suggest, :url => { :action => :find_suggestion }, # :frequency => 0.25, @@ -500,14 +500,14 @@ module ActionView # new Form.Element.Observer('glass', 1, function(element, value) {alert('Element changed')}) # The element parameter is the DOM element being observed, and the value is its value at the # time the observer is triggered. - # + # # Additional options are: # :frequency:: The frequency (in seconds) at which changes to # this field will be detected. Not setting this # option at all or to a value equal to or less than # zero will use event based observation instead of # time based observation. - # :update:: Specifies the DOM ID of the element whose + # :update:: Specifies the DOM ID of the element whose # innerHTML should be updated with the # XMLHttpRequest response text. # :with:: A JavaScript expression specifying the parameters @@ -518,7 +518,7 @@ module ActionView # variable +value+. # # Examples - # + # # :with => "'my_custom_key=' + value" # :with => "'person[name]=' + prompt('New name')" # :with => "Form.Element.serialize('other-field')" @@ -544,7 +544,7 @@ module ActionView # observe_field 'book_title', # :url => 'http://example.com/books/edit/1', # :with => 'title' - # + # # # Sends params: {:book_title => 'Title of the book'} when the focus leaves # # the input field. # observe_field 'book_title', @@ -558,7 +558,7 @@ module ActionView build_observer('Form.Element.EventObserver', field_id, options) end end - + # Observes the form with the DOM ID specified by +form_id+ and calls a # callback when its contents have changed. The default callback is an # Ajax call. By default all fields of the observed field are sent as @@ -574,16 +574,17 @@ module ActionView build_observer('Form.EventObserver', form_id, options) end end - - # All the methods were moved to GeneratorMethods so that + + # All the methods were moved to GeneratorMethods so that # #include_helpers_from_context has nothing to overwrite. class JavaScriptGenerator #:nodoc: def initialize(context, &block) #:nodoc: @context, @lines = context, [] + @context.output_buffer = @lines if @context include_helpers_from_context @context.instance_exec(self, &block) end - + private def include_helpers_from_context @context.extended_by.each do |mod| @@ -591,17 +592,17 @@ module ActionView end extend GeneratorMethods end - - # JavaScriptGenerator generates blocks of JavaScript code that allow you - # to change the content and presentation of multiple DOM elements. Use + + # JavaScriptGenerator generates blocks of JavaScript code that allow you + # to change the content and presentation of multiple DOM elements. Use # this in your Ajax response bodies, either in a