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(-) 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 87fbcaa6229e9073095fb8d77c7a536c9466fbce Mon Sep 17 00:00:00 2001 From: David Lowenfels Date: Sat, 28 Jun 2008 17:41:12 -0700 Subject: Add :tokenizer option to validates_length_of. [#507 state:resolved] Signed-off-by: Pratik Naik --- activerecord/CHANGELOG | 5 +++++ activerecord/lib/active_record/validations.rb | 16 ++++++++++------ activerecord/test/cases/validations_test.rb | 12 ++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index d6cc589381..d92b89cfe0 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,10 @@ *Edge* +* Add :tokenizer option to validates_length_of to specify how to split up the attribute string. #507. [David Lowenfels] Example : + + # Ensure essay contains at least 100 words. + validates_length_of :essay, :minimum => 100, :too_short => "Your essay must be at least %d words."), :tokenizer => lambda {|str| str.scan(/\w+/) } + * Allow conditions on multiple tables to be specified using hash. [Pratik Naik]. Example: User.all :joins => :items, :conditions => { :age => 10, :items => { :color => 'black' } } diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index c4e370d017..741649f764 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -479,8 +479,9 @@ module ActiveRecord # validates_length_of :fax, :in => 7..32, :allow_nil => true # validates_length_of :phone, :in => 7..32, :allow_blank => true # validates_length_of :user_name, :within => 6..20, :too_long => "pick a shorter name", :too_short => "pick a longer name" - # validates_length_of :fav_bra_size, :minimum=>1, :too_short=>"please enter at least %d character" - # validates_length_of :smurf_leader, :is=>4, :message=>"papa is spelled with %d characters... don't play me." + # validates_length_of :fav_bra_size, :minimum => 1, :too_short => "please enter at least %d character" + # validates_length_of :smurf_leader, :is => 4, :message => "papa is spelled with %d characters... don't play me." + # validates_length_of :essay, :minimum => 100, :too_short => "Your essay must be at least %d words."), :tokenizer => lambda {|str| str.scan(/\w+/) } # end # # Configuration options: @@ -491,7 +492,6 @@ module ActiveRecord # * :in - A synonym(or alias) for :within. # * :allow_nil - Attribute may be +nil+; skip validation. # * :allow_blank - Attribute may be blank; skip validation. - # # * :too_long - The error message if the attribute goes over the maximum (default is: "is too long (maximum is %d characters)"). # * :too_short - The error message if the attribute goes under the minimum (default is: "is too short (min is %d characters)"). # * :wrong_length - The error message if using the :is method and the attribute is the wrong size (default is: "is the wrong length (should be %d characters)"). @@ -503,12 +503,16 @@ module ActiveRecord # * :unless - Specifies a method, proc or string to call to determine if the validation should # not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The # method, proc or string should return or evaluate to a true or false value. + # * :tokenizer - Specifies how to split up the attribute string. (e.g. :tokenizer => lambda {|str| str.scan(/\w+/)} to + # count words as in above example.) + # Defaults to lambda{ |value| value.split(//) } which counts individual characters. def validates_length_of(*attrs) # Merge given options with defaults. options = { :too_long => ActiveRecord::Errors.default_error_messages[:too_long], :too_short => ActiveRecord::Errors.default_error_messages[:too_short], - :wrong_length => ActiveRecord::Errors.default_error_messages[:wrong_length] + :wrong_length => ActiveRecord::Errors.default_error_messages[:wrong_length], + :tokenizer => lambda {|value| value.split(//)} }.merge(DEFAULT_VALIDATION_OPTIONS) options.update(attrs.extract_options!.symbolize_keys) @@ -535,7 +539,7 @@ module ActiveRecord too_long = options[:too_long] % option_value.end validates_each(attrs, options) do |record, attr, value| - value = value.split(//) if value.kind_of?(String) + value = options[:tokenizer].call(value) if value.kind_of?(String) if value.nil? or value.size < option_value.begin record.errors.add(attr, too_short) elsif value.size > option_value.end @@ -552,7 +556,7 @@ module ActiveRecord message = (options[:message] || options[message_options[option]]) % option_value validates_each(attrs, options) do |record, attr, value| - value = value.split(//) if value.kind_of?(String) + value = options[:tokenizer].call(value) if value.kind_of?(String) record.errors.add(attr, message) unless !value.nil? and value.size.method(validity_checks[option])[option_value] end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 7b71647d25..60566cd064 100755 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -1059,6 +1059,18 @@ class ValidationsTest < ActiveRecord::TestCase end end + def test_validates_length_of_with_block + Topic.validates_length_of :content, :minimum => 5, :too_short=>"Your essay must be at least %d words.", + :tokenizer => lambda {|str| str.scan(/\w+/) } + t = Topic.create!(:content => "this content should be long enough") + assert t.valid? + + t.content = "not long enough" + assert !t.valid? + assert t.errors.on(:content) + assert_equal "Your essay must be at least 5 words.", t.errors[:content] + end + def test_validates_size_of_association_utf8 with_kcode('UTF8') do assert_nothing_raised { Topic.validates_size_of :replies, :minimum => 1 } -- 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(-) 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(-) 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 51e13d51adee3c69feab30aa4c0b08d2278ccc5f Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Sun, 22 Jun 2008 13:44:21 +0300 Subject: Move test related core_ext stuff out of regular core ext to prevent nasty surprises when you require active_support then use Test::Unit directly. --- activesupport/CHANGELOG | 2 + activesupport/lib/active_support/core_ext/test.rb | 1 - .../core_ext/test/unit/assertions.rb | 64 --------------------- activesupport/lib/active_support/test_case.rb | 6 +- .../lib/active_support/testing/core_ext/test.rb | 6 ++ .../testing/core_ext/test/unit/assertions.rb | 65 ++++++++++++++++++++++ 6 files changed, 74 insertions(+), 70 deletions(-) delete mode 100644 activesupport/lib/active_support/core_ext/test.rb delete mode 100644 activesupport/lib/active_support/core_ext/test/unit/assertions.rb create mode 100644 activesupport/lib/active_support/testing/core_ext/test.rb create mode 100644 activesupport/lib/active_support/testing/core_ext/test/unit/assertions.rb diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 73c965b1db..983e7d0dac 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Move the test related core_ext stuff out of core_ext so it's only loaded by the test helpers. [Michael Koziarski] + * Add Inflection rules for String#humanize. #535 [dcmanges] ActiveSupport::Inflector.inflections do |inflect| diff --git a/activesupport/lib/active_support/core_ext/test.rb b/activesupport/lib/active_support/core_ext/test.rb deleted file mode 100644 index c0b19bdc58..0000000000 --- a/activesupport/lib/active_support/core_ext/test.rb +++ /dev/null @@ -1 +0,0 @@ -require 'active_support/core_ext/test/unit/assertions' diff --git a/activesupport/lib/active_support/core_ext/test/unit/assertions.rb b/activesupport/lib/active_support/core_ext/test/unit/assertions.rb deleted file mode 100644 index 77fe325fb4..0000000000 --- a/activesupport/lib/active_support/core_ext/test/unit/assertions.rb +++ /dev/null @@ -1,64 +0,0 @@ -module Test - module Unit - #-- - # FIXME: no Proc#binding in Ruby 2, must change this API - #++ - module Assertions - # Test numeric difference between the return value of an expression as a result of what is evaluated - # in the yielded block. - # - # assert_difference 'Article.count' do - # post :create, :article => {...} - # end - # - # An arbitrary expression is passed in and evaluated. - # - # assert_difference 'assigns(:article).comments(:reload).size' do - # post :create, :comment => {...} - # end - # - # An arbitrary positive or negative difference can be specified. The default is +1. - # - # assert_difference 'Article.count', -1 do - # post :delete, :id => ... - # end - # - # An array of expressions can also be passed in and evaluated. - # - # assert_difference [ 'Article.count', 'Post.count' ], +2 do - # post :create, :article => {...} - # end - # - # A error message can be specified. - # - # assert_difference 'Article.count', -1, "An Article should be destroyed" do - # post :delete, :id => ... - # end - def assert_difference(expressions, difference = 1, message = nil, &block) - expression_evaluations = Array(expressions).collect{ |expression| lambda { eval(expression, block.send!(:binding)) } } - - original_values = expression_evaluations.inject([]) { |memo, expression| memo << expression.call } - yield - expression_evaluations.each_with_index do |expression, i| - assert_equal original_values[i] + difference, expression.call, message - end - end - - # Assertion that the numeric result of evaluating an expression is not changed before and after - # invoking the passed in block. - # - # assert_no_difference 'Article.count' do - # post :create, :article => invalid_attributes - # end - # - # A error message can be specified. - # - # assert_no_difference 'Article.count', "An Article should not be destroyed" do - # post :create, :article => invalid_attributes - # end - def assert_no_difference(expressions, message = nil, &block) - assert_difference expressions, 0, message, &block - end - end - end -end diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index 2fd02d5313..0f531b0c79 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -1,11 +1,7 @@ require 'test/unit/testcase' -require 'active_support/testing/setup_and_teardown' require 'active_support/testing/default' +require 'active_support/testing/core_ext/test' -# TODO: move to core_ext -class Test::Unit::TestCase #:nodoc: - include ActiveSupport::Testing::SetupAndTeardown -end module ActiveSupport class TestCase < Test::Unit::TestCase diff --git a/activesupport/lib/active_support/testing/core_ext/test.rb b/activesupport/lib/active_support/testing/core_ext/test.rb new file mode 100644 index 0000000000..d3f38f0bc7 --- /dev/null +++ b/activesupport/lib/active_support/testing/core_ext/test.rb @@ -0,0 +1,6 @@ +require 'active_support/testing/core_ext/test/unit/assertions' +require 'active_support/testing/setup_and_teardown' + +class Test::Unit::TestCase #:nodoc: + include ActiveSupport::Testing::SetupAndTeardown +end \ No newline at end of file diff --git a/activesupport/lib/active_support/testing/core_ext/test/unit/assertions.rb b/activesupport/lib/active_support/testing/core_ext/test/unit/assertions.rb new file mode 100644 index 0000000000..70a44eab8c --- /dev/null +++ b/activesupport/lib/active_support/testing/core_ext/test/unit/assertions.rb @@ -0,0 +1,65 @@ +require 'test/unit/assertions' +module Test + module Unit + #-- + # FIXME: no Proc#binding in Ruby 2, must change this API + #++ + module Assertions + # Test numeric difference between the return value of an expression as a result of what is evaluated + # in the yielded block. + # + # assert_difference 'Article.count' do + # post :create, :article => {...} + # end + # + # An arbitrary expression is passed in and evaluated. + # + # assert_difference 'assigns(:article).comments(:reload).size' do + # post :create, :comment => {...} + # end + # + # An arbitrary positive or negative difference can be specified. The default is +1. + # + # assert_difference 'Article.count', -1 do + # post :delete, :id => ... + # end + # + # An array of expressions can also be passed in and evaluated. + # + # assert_difference [ 'Article.count', 'Post.count' ], +2 do + # post :create, :article => {...} + # end + # + # A error message can be specified. + # + # assert_difference 'Article.count', -1, "An Article should be destroyed" do + # post :delete, :id => ... + # end + def assert_difference(expressions, difference = 1, message = nil, &block) + expression_evaluations = Array(expressions).collect{ |expression| lambda { eval(expression, block.send!(:binding)) } } + + original_values = expression_evaluations.inject([]) { |memo, expression| memo << expression.call } + yield + expression_evaluations.each_with_index do |expression, i| + assert_equal original_values[i] + difference, expression.call, message + end + end + + # Assertion that the numeric result of evaluating an expression is not changed before and after + # invoking the passed in block. + # + # assert_no_difference 'Article.count' do + # post :create, :article => invalid_attributes + # end + # + # A error message can be specified. + # + # assert_no_difference 'Article.count', "An Article should not be destroyed" do + # post :create, :article => invalid_attributes + # end + def assert_no_difference(expressions, message = nil, &block) + assert_difference expressions, 0, message, &block + end + end + 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(-) 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(-) 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 cb645c8877a52b83296b25bd5e5c5cf319a356fc Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 4 Jul 2008 20:07:00 +0100 Subject: Use ActiveSupport::TimeZone in time:zones rake tasks --- railties/lib/tasks/misc.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/tasks/misc.rake b/railties/lib/tasks/misc.rake index 61042595f9..33bbba1101 100644 --- a/railties/lib/tasks/misc.rake +++ b/railties/lib/tasks/misc.rake @@ -44,7 +44,7 @@ namespace :time do end end previous_offset = nil - TimeZone.__send__(method).each do |zone| + ActiveSupport::TimeZone.__send__(method).each do |zone| if offset.nil? || offset == zone.utc_offset puts "\n* UTC #{zone.formatted_offset} *" unless zone.utc_offset == previous_offset puts zone.name -- 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(-) 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 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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 3351d2997017465047b2c3dc63dc31e2362368af Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Sat, 31 May 2008 23:19:40 -0700 Subject: Add has_many :primary_key option to allow setting the primary key on a has many association Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/associations.rb | 5 +++-- .../lib/active_record/associations/has_many_association.rb | 8 ++++++++ .../test/cases/associations/has_many_associations_test.rb | 4 ++++ activerecord/test/fixtures/companies.yml | 1 + activerecord/test/models/company.rb | 2 ++ activerecord/test/schema/schema.rb | 1 + 6 files changed, 19 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 4b7154043f..c5e8207f14 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -663,6 +663,7 @@ module ActiveRecord # * :foreign_key - Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a +has_many+ association will use "person_id" # as the default :foreign_key. + # * :primary_key - Specify the method that returns the primary key used for the association. By default this is +id+. # * :dependent - If set to :destroy all the associated objects are destroyed # alongside this object by calling their +destroy+ method. If set to :delete_all all associated # objects are deleted *without* calling their +destroy+ method. If set to :nullify all associated @@ -678,7 +679,7 @@ module ActiveRecord # * :limit - An integer determining the limit on the number of rows that should be returned. # * :offset - An integer determining the offset from where the rows should be fetched. So at 5, it would skip the first 4 rows. # * :select - By default, this is * as in SELECT * FROM, but can be changed if you, for example, want to do a join - # but not include the joined columns. Do not forget to include the primary and foreign keys, otherwise it will rise an error. + # but not include the joined columns. Do not forget to include the primary and foreign keys, otherwise it will raise an error. # * :as - Specifies a polymorphic interface (See belongs_to). # * :through - Specifies a Join Model through which to perform the query. Options for :class_name and :foreign_key # are ignored, as the association uses the source reflection. You can only use a :through query through a belongs_to @@ -1347,7 +1348,7 @@ module ActiveRecord def create_has_many_reflection(association_id, options, &extension) options.assert_valid_keys( - :class_name, :table_name, :foreign_key, + :class_name, :table_name, :foreign_key, :primary_key, :dependent, :select, :conditions, :include, :order, :group, :limit, :offset, :as, :through, :source, :source_type, diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 37440aa84d..cb58f0be15 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -19,6 +19,14 @@ module ActiveRecord end protected + def owner_quoted_id + if @reflection.options[:primary_key] + quote_value(@owner.send(@reflection.options[:primary_key])) + else + @owner.quoted_id + end + end + def count_records count = if has_cached_counter? @owner.send(:read_attribute, cached_counter_attribute_name) diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 247726bc61..e90edbb213 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -129,6 +129,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal "Microsoft", Firm.find(:first).clients_like_ms_with_hash_conditions.first.name end + def test_finding_using_primary_key + assert_equal "Summit", Firm.find(:first).clients_using_primary_key.first.name + end + def test_finding_using_sql firm = Firm.find(:first) first_client = firm.clients_using_sql.first diff --git a/activerecord/test/fixtures/companies.yml b/activerecord/test/fixtures/companies.yml index c61128c09b..e7691fde46 100644 --- a/activerecord/test/fixtures/companies.yml +++ b/activerecord/test/fixtures/companies.yml @@ -5,6 +5,7 @@ first_client: client_of: 2 name: Summit ruby_type: Client + firm_name: 37signals first_firm: id: 1 diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 9fa810ac68..c45a21b6a2 100755 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -46,6 +46,8 @@ class Firm < Company has_many :clients_using_finder_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE 1=1' has_many :plain_clients, :class_name => 'Client' has_many :readonly_clients, :class_name => 'Client', :readonly => true + has_many :clients_using_primary_key, :class_name => 'Client', + :primary_key => 'name', :foreign_key => 'firm_name' has_one :account, :foreign_key => "firm_id", :dependent => :destroy, :validate => true has_one :unvalidated_account, :foreign_key => "firm_id", :class_name => 'Account', :validate => false diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 234c43494a..29c91a4464 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -102,6 +102,7 @@ ActiveRecord::Schema.define do t.string :type t.string :ruby_type t.integer :firm_id + t.string :firm_name t.string :name t.integer :client_of t.integer :rating, :default => 1 -- cgit v1.2.3 From afa0c7f728a8896c9ee9d932033e08a4c99dfd50 Mon Sep 17 00:00:00 2001 From: Brad Greenlee Date: Mon, 2 Jun 2008 22:04:46 -0700 Subject: Add support for :primary_key option to has_one as well as has_many so that a key other than the default primary key can be used for the association Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/associations.rb | 3 ++- .../lib/active_record/associations/has_one_association.rb | 11 ++++++++++- .../test/cases/associations/has_one_associations_test.rb | 7 +++++++ activerecord/test/cases/reflection_test.rb | 6 +++--- activerecord/test/models/company.rb | 1 + 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index c5e8207f14..856d872bce 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -759,6 +759,7 @@ module ActiveRecord # * :foreign_key - Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a +has_one+ association will use "person_id" # as the default :foreign_key. + # * :primary_key - Specify the method that returns the primary key used for the association. By default this is +id+. # * :include - Specify second-order associations that should be eager loaded when this object is loaded. # * :as - Specifies a polymorphic interface (See belongs_to). # * :select - By default, this is * as in SELECT * FROM, but can be changed if, for example, you want to do a join @@ -1366,7 +1367,7 @@ module ActiveRecord def create_has_one_reflection(association_id, options) options.assert_valid_keys( - :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :readonly, :validate + :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :readonly, :validate, :primary_key ) create_reflection(:has_one, association_id, options, self) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 25a268e95c..fdc0fa52c9 100755 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -47,7 +47,16 @@ module ActiveRecord return (obj.nil? ? nil : self) end end - + + protected + def owner_quoted_id + if @reflection.options[:primary_key] + quote_value(@owner.send(@reflection.options[:primary_key])) + else + @owner.quoted_id + end + end + private def find_target @reflection.klass.find(:first, diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index d3ca0cae41..99639849a5 100755 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -29,6 +29,13 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal Firm.find(1, :include => :account_with_select).account_with_select.attributes.size, 2 end + def test_finding_using_primary_key + firm = companies(:first_firm) + assert_equal Account.find_by_firm_id(firm.id), firm.account + firm.firm_id = companies(:rails_core).id + assert_equal accounts(:rails_core_account), firm.account_using_primary_key + end + def test_can_marshal_has_one_association_with_nil_target firm = Firm.new assert_nothing_raised do diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 0c57b79401..723062e3b8 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -160,9 +160,9 @@ class ReflectionTest < ActiveRecord::TestCase def test_reflection_of_all_associations # FIXME these assertions bust a lot - assert_equal 22, Firm.reflect_on_all_associations.size - assert_equal 17, Firm.reflect_on_all_associations(:has_many).size - assert_equal 5, Firm.reflect_on_all_associations(:has_one).size + assert_equal 24, Firm.reflect_on_all_associations.size + assert_equal 18, Firm.reflect_on_all_associations(:has_many).size + assert_equal 6, Firm.reflect_on_all_associations(:has_one).size assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index c45a21b6a2..e6aa810146 100755 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -53,6 +53,7 @@ class Firm < Company has_one :unvalidated_account, :foreign_key => "firm_id", :class_name => 'Account', :validate => false has_one :account_with_select, :foreign_key => "firm_id", :select => "id, firm_id", :class_name=>'Account' has_one :readonly_account, :foreign_key => "firm_id", :class_name => "Account", :readonly => true + has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account" end class DependentFirm < Company -- 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(-) 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 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 2b4eb586efa240dd985d8b5fe918084ab17fae2b Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion" Date: Wed, 9 Jul 2008 13:32:40 +0200 Subject: Plugin locator: sort directory listing because we can't assume that the OS will do it for us. This fixes some unit test failures. --- railties/lib/rails/plugin/locator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/plugin/locator.rb b/railties/lib/rails/plugin/locator.rb index 79c07fccd1..678b295dc9 100644 --- a/railties/lib/rails/plugin/locator.rb +++ b/railties/lib/rails/plugin/locator.rb @@ -63,7 +63,7 @@ module Rails # => # def locate_plugins_under(base_path) - Dir.glob(File.join(base_path, '*')).inject([]) do |plugins, path| + Dir.glob(File.join(base_path, '*')).sort.inject([]) do |plugins, path| if plugin = create_plugin(path) plugins << plugin elsif File.directory?(path) -- 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(-) 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 124d1016fa212c008e33853912493fa9ac15d086 Mon Sep 17 00:00:00 2001 From: Chris Cherry Date: Thu, 5 Jun 2008 23:26:35 -0700 Subject: Allow Infinity (1.0/0.0) to pass validates_numericality_of. [#354 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/validations.rb | 2 +- activerecord/test/cases/validations_test.rb | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 741649f764..1035308aa5 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -854,7 +854,7 @@ module ActiveRecord raw_value = raw_value.to_i else begin - raw_value = Kernel.Float(raw_value.to_s) + raw_value = Kernel.Float(raw_value) rescue ArgumentError, TypeError record.errors.add(attr_name, configuration[:message] || ActiveRecord::Errors.default_error_messages[:not_a_number]) next diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 60566cd064..0742e2c632 100755 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -1391,6 +1391,7 @@ class ValidatesNumericalityTest < ActiveRecord::TestCase INTEGERS = [0, 10, -10] + INTEGER_STRINGS BIGDECIMAL = BIGDECIMAL_STRINGS.collect! { |bd| BigDecimal.new(bd) } JUNK = ["not a number", "42 not a number", "0xdeadbeef", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12", "123\nnot a number"] + INFINITY = [1.0/0.0] def setup Topic.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new) @@ -1402,27 +1403,27 @@ class ValidatesNumericalityTest < ActiveRecord::TestCase Topic.validates_numericality_of :approved invalid!(NIL + BLANK + JUNK) - valid!(FLOATS + INTEGERS + BIGDECIMAL) + valid!(FLOATS + INTEGERS + BIGDECIMAL + INFINITY) end def test_validates_numericality_of_with_nil_allowed Topic.validates_numericality_of :approved, :allow_nil => true invalid!(BLANK + JUNK) - valid!(NIL + FLOATS + INTEGERS + BIGDECIMAL) + valid!(NIL + FLOATS + INTEGERS + BIGDECIMAL + INFINITY) end def test_validates_numericality_of_with_integer_only Topic.validates_numericality_of :approved, :only_integer => true - invalid!(NIL + BLANK + JUNK + FLOATS + BIGDECIMAL) + invalid!(NIL + BLANK + JUNK + FLOATS + BIGDECIMAL + INFINITY) valid!(INTEGERS) end def test_validates_numericality_of_with_integer_only_and_nil_allowed Topic.validates_numericality_of :approved, :only_integer => true, :allow_nil => true - invalid!(BLANK + JUNK + FLOATS + BIGDECIMAL) + invalid!(BLANK + JUNK + FLOATS + BIGDECIMAL + INFINITY) valid!(NIL + INTEGERS) end @@ -1443,7 +1444,7 @@ class ValidatesNumericalityTest < ActiveRecord::TestCase def test_validates_numericality_with_equal_to Topic.validates_numericality_of :approved, :equal_to => 10 - invalid!([-10, 11], 'must be equal to 10') + invalid!([-10, 11] + INFINITY, 'must be equal to 10') valid!([10]) end -- cgit v1.2.3 From 84af99e78dbf65b7faa92313acf8457cb0c2b510 Mon Sep 17 00:00:00 2001 From: Daniel Guettler Date: Wed, 9 Jul 2008 14:08:04 +0100 Subject: Ensure NamedScope#build/create/create!/new works as expected when named scope has hash conditions. [Daniel Guettler, Pratik Naik] [#419 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 3 ++- activerecord/test/cases/named_scope_test.rb | 26 ++++++++++++++++++++++++++ activerecord/test/models/topic.rb | 2 ++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index eac61e9e43..080e3d0f5e 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -150,7 +150,8 @@ module ActiveRecord if scopes.include?(method) scopes[method].call(self, *args) else - with_scope :find => proxy_options do + with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {} do + method = :new if method == :build proxy_scope.send(method, *args, &block) end end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 7d73541ee1..0c1eb23428 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -183,4 +183,30 @@ class NamedScopeTest < ActiveRecord::TestCase topics.empty? # use loaded (no query) end end + + def test_should_build_with_proxy_options + topic = Topic.approved.build({}) + assert topic.approved + end + + def test_should_build_new_with_proxy_options + topic = Topic.approved.new + assert topic.approved + end + + def test_should_create_with_proxy_options + topic = Topic.approved.create({}) + assert topic.approved + end + + def test_should_create_with_bang_with_proxy_options + topic = Topic.approved.create!({}) + assert topic.approved + end + + def test_should_build_with_proxy_options_chained + topic = Topic.approved.by_lifo.build({}) + assert topic.approved + assert_equal 'lifo', topic.author_name + end end diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 47b2eec938..39ca1bf42a 100755 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -4,6 +4,8 @@ class Topic < ActiveRecord::Base { :conditions => ['written_on < ?', time] } } named_scope :approved, :conditions => {:approved => true} + named_scope :by_lifo, :conditions => {:author_name => 'lifo'} + named_scope :approved_as_hash_condition, :conditions => {:topics => {:approved => true}} named_scope 'approved_as_string', :conditions => {:approved => true} named_scope :replied, :conditions => ['replies_count > 0'] -- 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(-) 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(-) 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(-) 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 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(-) 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(-) 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(-) 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(-) 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 11252e35b1756b025d8778c151f9f9a24057d1b1 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 8 Jul 2008 17:32:06 -0700 Subject: Boolean type casting creates fewer objects --- .../active_record/connection_adapters/abstract/schema_definitions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 2c03de0f17..d4c8a80448 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -138,7 +138,7 @@ module ActiveRecord if value == true || value == false value else - %w(true t 1).include?(value.to_s.downcase) + !(value.to_s !~ /\A(?:1|t|true)\Z/i) 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 ++++++++++++---- activerecord/lib/active_record/associations.rb | 4 ++-- .../active_record/associations/association_collection.rb | 2 +- .../active_record/associations/has_many_association.rb | 4 ++-- .../lib/active_support/core_ext/module/introspection.rb | 2 +- .../active_support/core_ext/object/instance_variables.rb | 2 +- activesupport/lib/active_support/dependencies.rb | 4 ++-- 8 files changed, 27 insertions(+), 19 deletions(-) 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 diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 856d872bce..6931744058 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1145,7 +1145,7 @@ module ActiveRecord end define_method("#{reflection.name.to_s.singularize}_ids") do - send(reflection.name).map(&:id) + send(reflection.name).map { |record| record.id } end end @@ -1490,7 +1490,7 @@ module ActiveRecord sql << " FROM #{connection.quote_table_name table_name} " if is_distinct - sql << distinct_join_associations.collect(&:association_join).join + sql << distinct_join_associations.collect { |assoc| assoc.association_join }.join add_joins!(sql, options, scope) end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index bbd8af7e76..eb39714909 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -14,7 +14,7 @@ module ActiveRecord # If using a custom finder_sql, scan the entire collection. if @reflection.options[:finder_sql] expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.uniq.map(&:to_i) + ids = args.flatten.compact.uniq.map { |arg| arg.to_i } if ids.size == 1 id = ids.first diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cb58f0be15..e6fa15c173 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -61,9 +61,9 @@ module ActiveRecord def delete_records(records) case @reflection.options[:dependent] when :destroy - records.each(&:destroy) + records.each { |r| r.destroy } when :delete_all - @reflection.klass.delete(records.map(&:id)) + @reflection.klass.delete(records.map { |record| record.id }) else ids = quoted_record_ids(records) @reflection.klass.update_all( diff --git a/activesupport/lib/active_support/core_ext/module/introspection.rb b/activesupport/lib/active_support/core_ext/module/introspection.rb index 40bbebb7c4..bb894ec080 100644 --- a/activesupport/lib/active_support/core_ext/module/introspection.rb +++ b/activesupport/lib/active_support/core_ext/module/introspection.rb @@ -70,6 +70,6 @@ class Module # Returns the names of the constants defined locally rather than the # constants themselves. See local_constants. def local_constant_names - local_constants.map(&:to_s) + local_constants.map { |c| c.to_s } end end diff --git a/activesupport/lib/active_support/core_ext/object/instance_variables.rb b/activesupport/lib/active_support/core_ext/object/instance_variables.rb index 9f1d4ed2aa..4ecaab3bbb 100644 --- a/activesupport/lib/active_support/core_ext/object/instance_variables.rb +++ b/activesupport/lib/active_support/core_ext/object/instance_variables.rb @@ -35,7 +35,7 @@ class Object # C.new(0, 1).instance_variable_names # => ["@y", "@x"] if RUBY_VERSION >= '1.9' def instance_variable_names - instance_variables.map(&:to_s) + instance_variables.map { |var| var.to_s } end else alias_method :instance_variable_names, :instance_variables diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index d3d9ff9de4..2f3fa72bb4 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -387,7 +387,7 @@ module ActiveSupport #:nodoc: ensure # Remove the stack frames that we added. if defined?(watch_frames) && ! watch_frames.blank? - frame_ids = watch_frames.collect(&:object_id) + frame_ids = watch_frames.collect { |frame| frame.object_id } constant_watch_stack.delete_if do |watch_frame| frame_ids.include? watch_frame.object_id end @@ -437,7 +437,7 @@ module ActiveSupport #:nodoc: protected def log_call(*args) if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER && log_activity - arg_str = args.collect(&:inspect) * ', ' + arg_str = args.collect { |arg| arg.inspect } * ', ' /in `([a-z_\?\!]+)'/ =~ caller(1).first selector = $1 || '' log "called #{selector}(#{arg_str})" -- 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(-) 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(-) 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 ++++ railties/configs/routes.rb | 2 ++ 2 files changed, 6 insertions(+) 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, diff --git a/railties/configs/routes.rb b/railties/configs/routes.rb index b579d6c7d1..4f3d9d22dd 100644 --- a/railties/configs/routes.rb +++ b/railties/configs/routes.rb @@ -36,6 +36,8 @@ ActionController::Routing::Routes.draw do |map| # See how all your routes lay out with "rake routes" # Install the default routes as the lowest priority. + # Note: These default routes make all actions in every controller accessible via GET requests. You should + # consider removing the them or commenting them out if you're using named routes and resources. map.connect ':controller/:action/:id' map.connect ':controller/:action/:id.:format' end -- cgit v1.2.3 From 7d5c447d9c4ebebd921a90810ff7547f8e40ea38 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 11:12:53 -0500 Subject: Stubba is included in Mocha already --- actionmailer/test/abstract_unit.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/actionmailer/test/abstract_unit.rb b/actionmailer/test/abstract_unit.rb index 9b7a4661b6..11058a770d 100644 --- a/actionmailer/test/abstract_unit.rb +++ b/actionmailer/test/abstract_unit.rb @@ -32,8 +32,7 @@ end # Wrap tests that use Mocha and skip if unavailable. def uses_mocha(test_name) - gem 'mocha', ">=0.5" - require 'stubba' + gem 'mocha', ">=0.9.0" yield rescue Gem::LoadError $stderr.puts "Skipping #{test_name} tests (Mocha >= 0.5 is required). `gem install mocha` and try again." -- 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(-) 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(-) 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(+) 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(-) 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 f522a89d6447da306778b67353adaf679c26bbd1 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 12:05:02 -0500 Subject: Revert "Fixed generator collisions for nested controller modules." This reverts commit 2d372d704987e05712ccd937e78d8dbd41242efe. --- railties/lib/rails_generator/commands.rb | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/railties/lib/rails_generator/commands.rb b/railties/lib/rails_generator/commands.rb index fb62ba6940..d258aeaa0a 100644 --- a/railties/lib/rails_generator/commands.rb +++ b/railties/lib/rails_generator/commands.rb @@ -154,35 +154,28 @@ HELP # Ruby or Rails. In the future, expand to check other namespaces # such as the rest of the user's app. def class_collisions(*class_names) - - # Initialize some check varibles - last_class = Object - current_class = nil - name = nil - class_names.flatten.each do |class_name| # Convert to string to allow symbol arguments. class_name = class_name.to_s # Skip empty strings. - class_name.strip.empty? ? next : current_class = class_name + next if class_name.strip.empty? # Split the class from its module nesting. nesting = class_name.split('::') name = nesting.pop # Extract the last Module in the nesting. - last = nesting.inject(last_class) { |last, nest| - break unless last_class.const_defined?(nest) - last_class = last_class.const_get(nest) + last = nesting.inject(Object) { |last, nest| + break unless last.const_defined?(nest) + last.const_get(nest) } - end - # If the last Module exists, check whether the given - # class exists and raise a collision if so. - - if last_class and last_class.const_defined?(name.camelize) - raise_class_collision(current_class) + # If the last Module exists, check whether the given + # class exists and raise a collision if so. + if last and last.const_defined?(name.camelize) + raise_class_collision(class_name) + end end end -- cgit v1.2.3 From c00baf496ef40d09962aabcb63d6d319cb8a0584 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 11 Jul 2008 12:09:25 -0500 Subject: Added tests to show that 2d372d7 breaks old generator behavior (#545 state:resolved) --- .../generators/rails_controller_generator_test.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/railties/test/generators/rails_controller_generator_test.rb b/railties/test/generators/rails_controller_generator_test.rb index 0090d21b85..8304fb5a01 100644 --- a/railties/test/generators/rails_controller_generator_test.rb +++ b/railties/test/generators/rails_controller_generator_test.rb @@ -17,4 +17,23 @@ class RailsControllerGeneratorTest < GeneratorTestCase assert_generated_functional_test_for "admin::products" assert_generated_helper_for "admin::products" end + + def test_controller_generates_namespaced_and_not_namespaced_controllers + run_generator('controller', %w(products)) + + # We have to require the generated helper to show the problem because + # the test helpers just check for generated files and contents but + # do not actually load them. But they have to be loaded (as in a real environment) + # to make the second generator run fail + require "#{RAILS_ROOT}/app/helpers/products_helper" + + assert_nothing_raised do + begin + run_generator('controller', %w(admin::products)) + ensure + # cleanup + Object.send(:remove_const, :ProductsHelper) + end + end + end end -- 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(-) 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 292501c7e01993faadfd953fd1b3154c470b65e2 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 11 Jul 2008 22:27:36 +0200 Subject: Use require_dependency 'application' not require in the console bootstraps to avoid requiring application.rb twice --- railties/lib/console_with_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/console_with_helpers.rb b/railties/lib/console_with_helpers.rb index 79018a9f76..be453a6896 100644 --- a/railties/lib/console_with_helpers.rb +++ b/railties/lib/console_with_helpers.rb @@ -16,7 +16,7 @@ def helper(*helper_names) end end -require 'application' +require_dependency 'application' class << helper include_all_modules_from ActionView -- 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(-) 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 5e2e1ed9ffc481a91596d8c3fd9a68d7977e75ca Mon Sep 17 00:00:00 2001 From: Micah Wedemeyer Date: Fri, 11 Jul 2008 23:50:55 +0100 Subject: Ensure MysqlAdapter allows SSL connection when only sslca is supplied. [#253 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/connection_adapters/mysql_adapter.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index c5962764f5..4b13ac8be0 100755 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -69,7 +69,7 @@ module ActiveRecord MysqlCompat.define_all_hashes_method! mysql = Mysql.init - mysql.ssl_set(config[:sslkey], config[:sslcert], config[:sslca], config[:sslcapath], config[:sslcipher]) if config[:sslkey] + mysql.ssl_set(config[:sslkey], config[:sslcert], config[:sslca], config[:sslcapath], config[:sslcipher]) if config[:sslca] || config[:sslkey] ConnectionAdapters::MysqlAdapter.new(mysql, logger, [host, username, password, database, port, socket], config) end @@ -145,6 +145,7 @@ module ActiveRecord # * :password - Defaults to nothing. # * :database - The name of the database. No default, must be provided. # * :encoding - (Optional) Sets the client encoding by executing "SET NAMES " after connection. + # * :sslca - Necessary to use MySQL with an SSL connection. # * :sslkey - Necessary to use MySQL with an SSL connection. # * :sslcert - Necessary to use MySQL with an SSL connection. # * :sslcapath - Necessary to use MySQL with an SSL connection. @@ -507,7 +508,9 @@ module ActiveRecord @connection.options(Mysql::SET_CHARSET_NAME, encoding) rescue nil end - @connection.ssl_set(@config[:sslkey], @config[:sslcert], @config[:sslca], @config[:sslcapath], @config[:sslcipher]) if @config[:sslkey] + if @config[:sslca] || @config[:sslkey] + @connection.ssl_set(@config[:sslkey], @config[:sslcert], @config[:sslca], @config[:sslcapath], @config[:sslcipher]) + end @connection.real_connect(*@connection_options) -- 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(-) 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(-) 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 f90eb81c65d5841b591caf0f5e39ef774d02d06e Mon Sep 17 00:00:00 2001 From: Daniel Guettler Date: Mon, 23 Jun 2008 11:06:13 -0400 Subject: Ensure script/generate finds generators from symlinked plugins. [#449 state:resolved] Signed-off-by: Pratik Naik --- railties/lib/rails_generator/lookup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails_generator/lookup.rb b/railties/lib/rails_generator/lookup.rb index 1f28c39d55..0526d526ad 100644 --- a/railties/lib/rails_generator/lookup.rb +++ b/railties/lib/rails_generator/lookup.rb @@ -108,7 +108,7 @@ module Rails sources << PathSource.new(:vendor, "#{::RAILS_ROOT}/vendor/generators") Rails.configuration.plugin_paths.each do |path| relative_path = Pathname.new(File.expand_path(path)).relative_path_from(Pathname.new(::RAILS_ROOT)) - sources << PathSource.new(:"plugins (#{relative_path})", "#{path}/**/{,rails_}generators") + sources << PathSource.new(:"plugins (#{relative_path})", "#{path}/*/**/{,rails_}generators") end end sources << PathSource.new(:user, "#{Dir.user_home}/.rails/generators") -- 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 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(-) 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(-) 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. --- actionmailer/lib/action_mailer/base.rb | 2 +- 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 + railties/lib/initializer.rb | 2 +- 21 files changed, 356 insertions(+), 467 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 diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 1518e23dfe..5a71935009 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -426,7 +426,7 @@ module ActionMailer #:nodoc: end def template_root=(root) - write_inheritable_attribute(:template_root, ActionView::ViewLoadPaths.new(Array(root))) + write_inheritable_attribute(:template_root, ActionView::PathSet.new(Array(root))) end end 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 diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index d80460d4bc..18bcf69d69 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -137,12 +137,12 @@ module Rails initialize_logger initialize_framework_logging - initialize_framework_views initialize_dependency_mechanism initialize_whiny_nils initialize_temporary_session_directory initialize_time_zone initialize_framework_settings + initialize_framework_views add_support_load_paths -- 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(-) 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