From a17027d13a48e1e64b14a28e7d58e341812f8cb4 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 13 Sep 2008 20:28:01 +0100 Subject: Merge docrails --- .../assertions/routing_assertions.rb | 28 +++++++++++----------- .../action_view/helpers/scriptaculous_helper.rb | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/assertions/routing_assertions.rb b/actionpack/lib/action_controller/assertions/routing_assertions.rb index 312b4e228b..8a837c592c 100644 --- a/actionpack/lib/action_controller/assertions/routing_assertions.rb +++ b/actionpack/lib/action_controller/assertions/routing_assertions.rb @@ -10,32 +10,32 @@ module ActionController # and a :method containing the required HTTP verb. # # # assert that POSTing to /items will call the create action on ItemsController - # assert_recognizes({:controller => 'items', :action => 'create'}, {:path => 'items', :method => :post}) + # assert_recognizes {:controller => 'items', :action => 'create'}, {:path => 'items', :method => :post} # # You can also pass in +extras+ with a hash containing URL parameters that would normally be in the query string. This can be used # to assert that values in the query string string will end up in the params hash correctly. To test query strings you must use the # extras argument, appending the query string on the path directly will not work. For example: # # # assert that a path of '/items/list/1?view=print' returns the correct options - # assert_recognizes({:controller => 'items', :action => 'list', :id => '1', :view => 'print'}, 'items/list/1', { :view => "print" }) + # assert_recognizes {:controller => 'items', :action => 'list', :id => '1', :view => 'print'}, 'items/list/1', { :view => "print" } # # The +message+ parameter allows you to pass in an error message that is displayed upon failure. # # ==== Examples # # Check the default route (i.e., the index action) - # assert_recognizes({:controller => 'items', :action => 'index'}, 'items') + # assert_recognizes {:controller => 'items', :action => 'index'}, 'items' # # # Test a specific action - # assert_recognizes({:controller => 'items', :action => 'list'}, 'items/list') + # assert_recognizes {:controller => 'items', :action => 'list'}, 'items/list' # # # Test an action with a parameter - # assert_recognizes({:controller => 'items', :action => 'destroy', :id => '1'}, 'items/destroy/1') + # assert_recognizes {:controller => 'items', :action => 'destroy', :id => '1'}, 'items/destroy/1' # # # Test a custom route - # assert_recognizes({:controller => 'items', :action => 'show', :id => '1'}, 'view/item1') + # assert_recognizes {:controller => 'items', :action => 'show', :id => '1'}, 'view/item1' # # # Check a Simply RESTful generated route - # assert_recognizes(list_items_url, 'items/list') + # assert_recognizes list_items_url, 'items/list' def assert_recognizes(expected_options, path, extras={}, message=nil) if path.is_a? Hash request_method = path[:method] @@ -67,13 +67,13 @@ module ActionController # # ==== Examples # # Asserts that the default action is generated for a route with no action - # assert_generates("/items", :controller => "items", :action => "index") + # assert_generates "/items", :controller => "items", :action => "index" # # # Tests that the list action is properly routed - # assert_generates("/items/list", :controller => "items", :action => "list") + # assert_generates "/items/list", :controller => "items", :action => "list" # # # Tests the generation of a route with a parameter - # assert_generates("/items/list/1", { :controller => "items", :action => "list", :id => "1" }) + # assert_generates "/items/list/1", { :controller => "items", :action => "list", :id => "1" } # # # Asserts that the generated route gives us our custom route # assert_generates "changesets/12", { :controller => 'scm', :action => 'show_diff', :revision => "12" } @@ -104,19 +104,19 @@ module ActionController # # ==== Examples # # Assert a basic route: a controller with the default action (index) - # assert_routing('/home', :controller => 'home', :action => 'index') + # assert_routing '/home', :controller => 'home', :action => 'index' # # # Test a route generated with a specific controller, action, and parameter (id) - # assert_routing('/entries/show/23', :controller => 'entries', :action => 'show', id => 23) + # assert_routing '/entries/show/23', :controller => 'entries', :action => 'show', id => 23 # # # Assert a basic route (controller + default action), with an error message if it fails - # assert_routing('/store', { :controller => 'store', :action => 'index' }, {}, {}, 'Route for store index not generated properly') + # assert_routing '/store', { :controller => 'store', :action => 'index' }, {}, {}, 'Route for store index not generated properly' # # # Tests a route, providing a defaults hash # assert_routing 'controller/action/9', {:id => "9", :item => "square"}, {:controller => "controller", :action => "action"}, {}, {:item => "square"} # # # Tests a route with a HTTP method - # assert_routing({ :method => 'put', :path => '/product/321' }, { :controller => "product", :action => "update", :id => "321" }) + # assert_routing { :method => 'put', :path => '/product/321' }, { :controller => "product", :action => "update", :id => "321" } def assert_routing(path, options, defaults={}, extras={}, message=nil) assert_recognizes(options, path, extras, message) diff --git a/actionpack/lib/action_view/helpers/scriptaculous_helper.rb b/actionpack/lib/action_view/helpers/scriptaculous_helper.rb index b938c1a801..1d01dafd0e 100644 --- a/actionpack/lib/action_view/helpers/scriptaculous_helper.rb +++ b/actionpack/lib/action_view/helpers/scriptaculous_helper.rb @@ -95,7 +95,7 @@ module ActionView # * :containment - Takes an element or array of elements to treat as # potential drop targets (defaults to the original target element). # - # * :only - A CSS class name or arry of class names used to filter + # * :only - A CSS class name or array of class names used to filter # out child elements as candidates. # # * :scroll - Determines whether to scroll the list during drag -- cgit v1.2.3 From 7ecb9689b03335ec28958c506b083390f4212d45 Mon Sep 17 00:00:00 2001 From: Pelle Braendgaard Date: Tue, 16 Sep 2008 09:22:11 -0700 Subject: Added support for http_only cookies in cookie_store Added unit tests for secure and http_only cookies in cookie_store Signed-off-by: Michael Koziarski [#1046 state:committed] --- actionpack/CHANGELOG | 2 + actionpack/lib/action_controller/cgi_process.rb | 3 +- actionpack/lib/action_controller/rack_process.rb | 3 +- .../lib/action_controller/session/cookie_store.rb | 3 +- .../lib/action_controller/session_management.rb | 4 ++ .../test/controller/session/cookie_store_test.rb | 53 +++++++++++++++++++++- 6 files changed, 64 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 3743e3e8fe..54ea93fb72 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Set HttpOnly for the cookie session store's cookie. #1046 + * Added FormTagHelper#image_submit_tag confirm option #784 [Alastair Brunton] * Fixed FormTagHelper#submit_tag with :disable_with option wouldn't submit the button's value when was clicked #633 [Jose Fernandez] diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb index d381af1b84..fabacd9b83 100644 --- a/actionpack/lib/action_controller/cgi_process.rb +++ b/actionpack/lib/action_controller/cgi_process.rb @@ -42,7 +42,8 @@ module ActionController #:nodoc: :prefix => "ruby_sess.", # prefix session file names :session_path => "/", # available to all paths in app :session_key => "_session_id", - :cookie_only => true + :cookie_only => true, + :session_http_only=> true } def initialize(cgi, session_options = {}) diff --git a/actionpack/lib/action_controller/rack_process.rb b/actionpack/lib/action_controller/rack_process.rb index 1ace16da07..e8ea3704a8 100644 --- a/actionpack/lib/action_controller/rack_process.rb +++ b/actionpack/lib/action_controller/rack_process.rb @@ -14,7 +14,8 @@ module ActionController #:nodoc: :prefix => "ruby_sess.", # prefix session file names :session_path => "/", # available to all paths in app :session_key => "_session_id", - :cookie_only => true + :cookie_only => true, + :session_http_only=> true } def initialize(env, session_options = DEFAULT_SESSION_OPTIONS) diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb index 5bf7503f04..f2fb200950 100644 --- a/actionpack/lib/action_controller/session/cookie_store.rb +++ b/actionpack/lib/action_controller/session/cookie_store.rb @@ -70,7 +70,8 @@ class CGI::Session::CookieStore 'path' => options['session_path'], 'domain' => options['session_domain'], 'expires' => options['session_expires'], - 'secure' => options['session_secure'] + 'secure' => options['session_secure'], + 'http_only' => options['session_http_only'] } # Set no_hidden and no_cookies since the session id is unused and we diff --git a/actionpack/lib/action_controller/session_management.rb b/actionpack/lib/action_controller/session_management.rb index f5a1155a46..fd3d94ed97 100644 --- a/actionpack/lib/action_controller/session_management.rb +++ b/actionpack/lib/action_controller/session_management.rb @@ -60,6 +60,10 @@ module ActionController #:nodoc: # # the session will only work over HTTPS, but only for the foo action # session :only => :foo, :session_secure => true # + # # the session by default uses HttpOnly sessions for security reasons. + # # this can be switched off. + # session :only => :foo, :session_http_only => false + # # # the session will only be disabled for 'foo', and only if it is # # requested as a web service # session :off, :only => :foo, diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb index 5adaeaf5c5..010c00fa14 100644 --- a/actionpack/test/controller/session/cookie_store_test.rb +++ b/actionpack/test/controller/session/cookie_store_test.rb @@ -36,7 +36,9 @@ class CookieStoreTest < Test::Unit::TestCase 'session_key' => '_myapp_session', 'secret' => 'Keep it secret; keep it safe.', 'no_cookies' => true, - 'no_hidden' => true } + 'no_hidden' => true, + 'session_http_only' => true + } end def self.cookies @@ -149,6 +151,48 @@ class CookieStoreTest < Test::Unit::TestCase assert_equal 1, session.cgi.output_cookies.size cookie = session.cgi.output_cookies.first assert_cookie cookie, cookie_value(:flashed) + assert_http_only_cookie cookie + assert_secure_cookie cookie, false + end + end + + def test_writes_non_secure_cookie_by_default + set_cookie! cookie_value(:typical) + new_session do |session| + session['flash'] = {} + session.close + cookie = session.cgi.output_cookies.first + assert_secure_cookie cookie,false + end + end + + def test_writes_secure_cookie + set_cookie! cookie_value(:typical) + new_session('session_secure'=>true) do |session| + session['flash'] = {} + session.close + cookie = session.cgi.output_cookies.first + assert_secure_cookie cookie + end + end + + def test_http_only_cookie_by_default + set_cookie! cookie_value(:typical) + new_session do |session| + session['flash'] = {} + session.close + cookie = session.cgi.output_cookies.first + assert_http_only_cookie cookie + end + end + + def test_overides_http_only_cookie + set_cookie! cookie_value(:typical) + new_session('session_http_only'=>false) do |session| + session['flash'] = {} + session.close + cookie = session.cgi.output_cookies.first + assert_http_only_cookie cookie, false end end @@ -195,6 +239,13 @@ class CookieStoreTest < Test::Unit::TestCase assert_equal expires, cookie.expires ? cookie.expires.to_date : cookie.expires, message end + def assert_secure_cookie(cookie,value=true) + assert cookie.secure==value + end + + def assert_http_only_cookie(cookie,value=true) + assert cookie.http_only==value + end def cookies(*which) self.class.cookies.values_at(*which) -- cgit v1.2.3 From 2d27b82d4cf446543539ad20afcbad256d8aeff7 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Thu, 18 Sep 2008 21:27:48 +0200 Subject: Remove the country_select helper. We're in no position to mediate disputes on this matter, and the previous change to use ISO 3166 has offended just as many people as the ad-hoc list did. If you want the old list back you can install the plugin: ruby script/plugin install git://github.com/rails/country_select.git --- actionpack/lib/action_view/helpers.rb | 1 - .../lib/action_view/helpers/form_country_helper.rb | 92 -- .../lib/action_view/helpers/form_options_helper.rb | 3 - .../test/template/form_country_helper_test.rb | 1549 -------------------- 4 files changed, 1645 deletions(-) delete mode 100644 actionpack/lib/action_view/helpers/form_country_helper.rb delete mode 100644 actionpack/test/template/form_country_helper_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers.rb b/actionpack/lib/action_view/helpers.rb index 05e1cf990a..ff97df204c 100644 --- a/actionpack/lib/action_view/helpers.rb +++ b/actionpack/lib/action_view/helpers.rb @@ -21,7 +21,6 @@ module ActionView #:nodoc: include CaptureHelper include DateHelper include DebugHelper - include FormCountryHelper include FormHelper include FormOptionsHelper include FormTagHelper diff --git a/actionpack/lib/action_view/helpers/form_country_helper.rb b/actionpack/lib/action_view/helpers/form_country_helper.rb deleted file mode 100644 index 84e811f61d..0000000000 --- a/actionpack/lib/action_view/helpers/form_country_helper.rb +++ /dev/null @@ -1,92 +0,0 @@ -require 'action_view/helpers/form_options_helper' - -module ActionView - module Helpers - module FormCountryHelper - - # Return select and option tags for the given object and method, using country_options_for_select to generate the list of option tags. - def country_select(object, method, priority_countries = nil, options = {}, html_options = {}) - InstanceTag.new(object, method, self, options.delete(:object)).to_country_select_tag(priority_countries, options, html_options) - end - - # Returns a string of option tags for pretty much any country in the world. Supply a country name as +selected+ to - # have it marked as the selected option tag. You can also supply an array of countries as +priority_countries+, so - # that they will be listed above the rest of the (long) list. - # - # NOTE: Only the option tags are returned, you have to wrap this call in a regular HTML select tag. - def country_options_for_select(selected = nil, priority_countries = nil) - country_options = "" - - if priority_countries - country_options += options_for_select(priority_countries, selected) - country_options += "\n" - end - - return country_options + options_for_select(COUNTRIES, selected) - end - - private - - # All the countries included in the country_options output. - COUNTRIES = ["Afghanistan", "Aland Islands", "Albania", "Algeria", "American Samoa", "Andorra", "Angola", - "Anguilla", "Antarctica", "Antigua And Barbuda", "Argentina", "Armenia", "Aruba", "Australia", "Austria", - "Azerbaijan", "Bahamas", "Bahrain", "Bangladesh", "Barbados", "Belarus", "Belgium", "Belize", "Benin", - "Bermuda", "Bhutan", "Bolivia", "Bosnia and Herzegowina", "Botswana", "Bouvet Island", "Brazil", - "British Indian Ocean Territory", "Brunei Darussalam", "Bulgaria", "Burkina Faso", "Burundi", "Cambodia", - "Cameroon", "Canada", "Cape Verde", "Cayman Islands", "Central African Republic", "Chad", "Chile", "China", - "Christmas Island", "Cocos (Keeling) Islands", "Colombia", "Comoros", "Congo", - "Congo, the Democratic Republic of the", "Cook Islands", "Costa Rica", "Cote d'Ivoire", "Croatia", "Cuba", - "Cyprus", "Czech Republic", "Denmark", "Djibouti", "Dominica", "Dominican Republic", "Ecuador", "Egypt", - "El Salvador", "Equatorial Guinea", "Eritrea", "Estonia", "Ethiopia", "Falkland Islands (Malvinas)", - "Faroe Islands", "Fiji", "Finland", "France", "French Guiana", "French Polynesia", - "French Southern Territories", "Gabon", "Gambia", "Georgia", "Germany", "Ghana", "Gibraltar", "Greece", "Greenland", "Grenada", "Guadeloupe", "Guam", "Guatemala", "Guernsey", "Guinea", - "Guinea-Bissau", "Guyana", "Haiti", "Heard and McDonald Islands", "Holy See (Vatican City State)", - "Honduras", "Hong Kong", "Hungary", "Iceland", "India", "Indonesia", "Iran, Islamic Republic of", "Iraq", - "Ireland", "Isle of Man", "Israel", "Italy", "Jamaica", "Japan", "Jersey", "Jordan", "Kazakhstan", "Kenya", - "Kiribati", "Korea, Democratic People's Republic of", "Korea, Republic of", "Kuwait", "Kyrgyzstan", - "Lao People's Democratic Republic", "Latvia", "Lebanon", "Lesotho", "Liberia", "Libyan Arab Jamahiriya", - "Liechtenstein", "Lithuania", "Luxembourg", "Macao", "Macedonia, The Former Yugoslav Republic Of", - "Madagascar", "Malawi", "Malaysia", "Maldives", "Mali", "Malta", "Marshall Islands", "Martinique", - "Mauritania", "Mauritius", "Mayotte", "Mexico", "Micronesia, Federated States of", "Moldova, Republic of", - "Monaco", "Mongolia", "Montenegro", "Montserrat", "Morocco", "Mozambique", "Myanmar", "Namibia", "Nauru", - "Nepal", "Netherlands", "Netherlands Antilles", "New Caledonia", "New Zealand", "Nicaragua", "Niger", - "Nigeria", "Niue", "Norfolk Island", "Northern Mariana Islands", "Norway", "Oman", "Pakistan", "Palau", - "Palestinian Territory, Occupied", "Panama", "Papua New Guinea", "Paraguay", "Peru", "Philippines", - "Pitcairn", "Poland", "Portugal", "Puerto Rico", "Qatar", "Reunion", "Romania", "Russian Federation", - "Rwanda", "Saint Barthelemy", "Saint Helena", "Saint Kitts and Nevis", "Saint Lucia", - "Saint Pierre and Miquelon", "Saint Vincent and the Grenadines", "Samoa", "San Marino", - "Sao Tome and Principe", "Saudi Arabia", "Senegal", "Serbia", "Seychelles", "Sierra Leone", "Singapore", - "Slovakia", "Slovenia", "Solomon Islands", "Somalia", "South Africa", - "South Georgia and the South Sandwich Islands", "Spain", "Sri Lanka", "Sudan", "Suriname", - "Svalbard and Jan Mayen", "Swaziland", "Sweden", "Switzerland", "Syrian Arab Republic", - "Taiwan, Province of China", "Tajikistan", "Tanzania, United Republic of", "Thailand", "Timor-Leste", - "Togo", "Tokelau", "Tonga", "Trinidad and Tobago", "Tunisia", "Turkey", "Turkmenistan", - "Turks and Caicos Islands", "Tuvalu", "Uganda", "Ukraine", "United Arab Emirates", "United Kingdom", - "United States", "United States Minor Outlying Islands", "Uruguay", "Uzbekistan", "Vanuatu", "Venezuela", - "Viet Nam", "Virgin Islands, British", "Virgin Islands, U.S.", "Wallis and Futuna", "Western Sahara", - "Yemen", "Zambia", "Zimbabwe"] unless const_defined?("COUNTRIES") - end - - class InstanceTag #:nodoc: - include FormCountryHelper - - def to_country_select_tag(priority_countries, options, html_options) - html_options = html_options.stringify_keys - add_default_name_and_id(html_options) - value = value(object) - content_tag("select", - add_options( - country_options_for_select(value, priority_countries), - options, value - ), html_options - ) - end - end - - class FormBuilder - def country_select(method, priority_countries = nil, options = {}, html_options = {}) - @template.country_select(@object_name, method, priority_countries, objectify_options(options), @default_options.merge(html_options)) - end - end - end -end \ No newline at end of file diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 9aae945408..33f8aaf9ed 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -324,9 +324,6 @@ module ActionView value == selected end end - - # All the countries included in the country_options output. - COUNTRIES = ActiveSupport::Deprecation::DeprecatedConstantProxy.new 'COUNTRIES', 'ActionView::Helpers::FormCountryHelper::COUNTRIES' end class InstanceTag #:nodoc: diff --git a/actionpack/test/template/form_country_helper_test.rb b/actionpack/test/template/form_country_helper_test.rb deleted file mode 100644 index 8862e08222..0000000000 --- a/actionpack/test/template/form_country_helper_test.rb +++ /dev/null @@ -1,1549 +0,0 @@ -require 'abstract_unit' - -class FormCountryHelperTest < ActionView::TestCase - tests ActionView::Helpers::FormCountryHelper - - silence_warnings do - Post = Struct.new('Post', :title, :author_name, :body, :secret, :written_on, :category, :origin) - end - - def test_country_select - @post = Post.new - @post.origin = "Denmark" - expected_select = <<-COUNTRIES - -COUNTRIES - assert_dom_equal(expected_select[0..-2], country_select("post", "origin")) - end - - def test_country_select_with_priority_countries - @post = Post.new - @post.origin = "Denmark" - expected_select = <<-COUNTRIES - -COUNTRIES - assert_dom_equal(expected_select[0..-2], country_select("post", "origin", ["New Zealand", "Nicaragua"])) - end - - def test_country_select_with_selected_priority_country - @post = Post.new - @post.origin = "New Zealand" - expected_select = <<-COUNTRIES - -COUNTRIES - assert_dom_equal(expected_select[0..-2], country_select("post", "origin", ["New Zealand", "Nicaragua"])) - end - - def test_country_select_under_fields_for - @post = Post.new - @post.origin = "Australia" - expected_select = <<-COUNTRIES - -COUNTRIES - - fields_for :post, @post do |f| - concat f.country_select("origin") - end - - assert_dom_equal(expected_select[0..-2], output_buffer) - end - - def test_country_select_under_fields_for_with_index - @post = Post.new - @post.origin = "United States" - expected_select = <<-COUNTRIES - -COUNTRIES - - fields_for :post, @post, :index => 325 do |f| - concat f.country_select("origin") - end - - assert_dom_equal(expected_select[0..-2], output_buffer) - end - - def test_country_select_under_fields_for_with_auto_index - @post = Post.new - @post.origin = "Iraq" - def @post.to_param; 325; end - - expected_select = <<-COUNTRIES - -COUNTRIES - - fields_for "post[]", @post do |f| - concat f.country_select("origin") - end - - assert_dom_equal(expected_select[0..-2], output_buffer) - end - -end \ No newline at end of file -- cgit v1.2.3 From a3b7fa78bfdc33e45e39c095b67e02d50a2c7bea Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Mon, 15 Sep 2008 10:26:50 +0200 Subject: I18n: Introduce I18n.load_path in favor of I18n.load_translations and change Simple backend to load translations lazily. [#1048 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_view.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 0ed69f29bf..7cd9b633ac 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -43,7 +43,7 @@ require 'action_view/base' require 'action_view/partials' require 'action_view/template_error' -I18n.load_translations "#{File.dirname(__FILE__)}/action_view/locale/en-US.yml" +I18n.load_path << "#{File.dirname(__FILE__)}/action_view/locale/en-US.yml" require 'action_view/helpers' -- cgit v1.2.3 From 5f83e1844c83c19cf97c6415b943c6ec3cb4bb06 Mon Sep 17 00:00:00 2001 From: Claudio Poli Date: Sat, 20 Sep 2008 22:57:45 -0500 Subject: Fixed missing template paths on exception [#1082 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_view/template.rb | 13 +++++++--- actionpack/lib/action_view/template_error.rb | 30 ++++++++-------------- .../test/fixtures/test/sub_template_raise.html.erb | 1 + actionpack/test/template/render_test.rb | 18 ++++++++++++- 4 files changed, 37 insertions(+), 25 deletions(-) create mode 100644 actionpack/test/fixtures/test/sub_template_raise.html.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 64597b3d39..12808581a3 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -52,15 +52,20 @@ module ActionView #:nodoc: end memoize :path_without_format_and_extension + def relative_path + path = File.expand_path(filename) + path.sub!(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '') if defined?(RAILS_ROOT) + path + end + memoize :relative_path + def source File.read(filename) end memoize :source def 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 } + relative_path.to_s.gsub(/([^a-zA-Z0-9_])/) { $1.ord } end memoize :method_segment @@ -69,7 +74,7 @@ module ActionView #:nodoc: rescue Exception => e raise e unless filename if TemplateError === e - e.sub_template_of(filename) + e.sub_template_of(self) raise e else raise TemplateError.new(self, view.assigns, e) diff --git a/actionpack/lib/action_view/template_error.rb b/actionpack/lib/action_view/template_error.rb index 2368662f31..bcce331773 100644 --- a/actionpack/lib/action_view/template_error.rb +++ b/actionpack/lib/action_view/template_error.rb @@ -7,12 +7,14 @@ module ActionView attr_reader :original_exception def initialize(template, assigns, original_exception) - @base_path = template.base_path.to_s - @assigns, @source, @original_exception = assigns.dup, template.source, original_exception - @file_path = template.filename + @template, @assigns, @original_exception = template, assigns.dup, original_exception @backtrace = compute_backtrace end + def file_name + @template.relative_path + end + def message ActiveSupport::Deprecation.silence { original_exception.message } end @@ -24,7 +26,7 @@ module ActionView def sub_template_message if @sub_templates "Trace of template inclusion: " + - @sub_templates.collect { |template| strip_base_path(template) }.join(", ") + @sub_templates.collect { |template| template.relative_path }.join(", ") else "" end @@ -34,18 +36,18 @@ module ActionView return unless num = line_number num = num.to_i - source_code = IO.readlines(@file_path) + source_code = @template.source.split("\n") start_on_line = [ num - SOURCE_CODE_RADIUS - 1, 0 ].max end_on_line = [ num + SOURCE_CODE_RADIUS - 1, source_code.length].min indent = ' ' * indentation line_counter = start_on_line - return unless source_code = source_code[start_on_line..end_on_line] - + return unless source_code = source_code[start_on_line..end_on_line] + source_code.sum do |line| line_counter += 1 - "#{indent}#{line_counter}: #{line}" + "#{indent}#{line_counter}: #{line}\n" end end @@ -63,12 +65,6 @@ module ActionView end end - def file_name - stripped = strip_base_path(@file_path) - stripped.slice!(0,1) if stripped[0] == ?/ - stripped - end - def to_s "\n\n#{self.class} (#{message}) #{source_location}:\n" + "#{source_extract}\n #{clean_backtrace.join("\n ")}\n\n" @@ -88,12 +84,6 @@ module ActionView ] end - def strip_base_path(path) - stripped_path = File.expand_path(path).gsub(@base_path, "") - stripped_path.gsub!(/^#{Regexp.escape File.expand_path(RAILS_ROOT)}/, '') if defined?(RAILS_ROOT) - stripped_path - end - def source_location if line_number "on line ##{line_number} of " diff --git a/actionpack/test/fixtures/test/sub_template_raise.html.erb b/actionpack/test/fixtures/test/sub_template_raise.html.erb new file mode 100644 index 0000000000..f38c0bda90 --- /dev/null +++ b/actionpack/test/fixtures/test/sub_template_raise.html.erb @@ -0,0 +1 @@ +<%= render :partial => "test/raise" %> \ No newline at end of file diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index f3c8dbcae9..a4ea22ddcb 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -70,7 +70,23 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_partial_with_errors - assert_raise(ActionView::TemplateError) { @view.render(:partial => "test/raise") } + @view.render(:partial => "test/raise") + flunk "Render did not raise TemplateError" + rescue ActionView::TemplateError => e + assert_match "undefined local variable or method `doesnt_exist'", e.message + assert_equal "", e.sub_template_message + assert_equal "1", e.line_number + assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name + end + + def test_render_sub_template_with_errors + @view.render(:file => "test/sub_template_raise") + flunk "Render did not raise TemplateError" + rescue ActionView::TemplateError => e + assert_match "undefined local variable or method `doesnt_exist'", e.message + assert_equal "Trace of template inclusion: #{File.expand_path("#{FIXTURE_LOAD_PATH}/test/sub_template_raise.html.erb")}", e.sub_template_message + assert_equal "1", e.line_number + assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end def test_render_partial_collection -- cgit v1.2.3 From 7329990d868d10e4fbf541097cd5be8f1254e2ce Mon Sep 17 00:00:00 2001 From: Manfred Stienstra Date: Sun, 21 Sep 2008 17:27:25 +0200 Subject: Change all calls to String#chars to String#mb_chars. Remove a exception for Ruby <= 1.9. --- actionpack/lib/action_view/helpers/text_helper.rb | 128 +++++++--------------- 1 file changed, 41 insertions(+), 87 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index ab1fdc80bc..4e371149af 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -42,65 +42,46 @@ module ActionView output_buffer << string end - if RUBY_VERSION < '1.9' - # Truncates a given +text+ after a given :length if +text+ is longer than :length - # (defaults to 30). The last characters will be replaced with the :omission (defaults to "..."). - # - # ==== Examples - # - # truncate("Once upon a time in a world far far away") - # # => Once upon a time in a world f... - # - # truncate("Once upon a time in a world far far away", :length => 14) - # # => Once upon a... - # - # truncate("And they found that many people were sleeping better.", :length => 25, "(clipped)") - # # => And they found that many (clipped) - # - # truncate("And they found that many people were sleeping better.", :omission => "... (continued)", :length => 15) - # # => And they found... (continued) - # - # You can still use truncate with the old API that accepts the - # +length+ as its optional second and the +ellipsis+ as its - # optional third parameter: - # truncate("Once upon a time in a world far far away", 14) - # # => Once upon a time in a world f... - # - # truncate("And they found that many people were sleeping better.", 15, "... (continued)") - # # => And they found... (continued) - def truncate(text, *args) - options = args.extract_options! - unless args.empty? - ActiveSupport::Deprecation.warn('truncate takes an option hash instead of separate ' + - 'length and omission arguments', caller) - - options[:length] = args[0] || 30 - options[:omission] = args[1] || "..." - end - options.reverse_merge!(:length => 30, :omission => "...") + # Truncates a given +text+ after a given :length if +text+ is longer than :length + # (defaults to 30). The last characters will be replaced with the :omission (defaults to "..."). + # + # ==== Examples + # + # truncate("Once upon a time in a world far far away") + # # => Once upon a time in a world f... + # + # truncate("Once upon a time in a world far far away", :length => 14) + # # => Once upon a... + # + # truncate("And they found that many people were sleeping better.", :length => 25, "(clipped)") + # # => And they found that many (clipped) + # + # truncate("And they found that many people were sleeping better.", :omission => "... (continued)", :length => 15) + # # => And they found... (continued) + # + # You can still use truncate with the old API that accepts the + # +length+ as its optional second and the +ellipsis+ as its + # optional third parameter: + # truncate("Once upon a time in a world far far away", 14) + # # => Once upon a time in a world f... + # + # truncate("And they found that many people were sleeping better.", 15, "... (continued)") + # # => And they found... (continued) + def truncate(text, *args) + options = args.extract_options! + unless args.empty? + ActiveSupport::Deprecation.warn('truncate takes an option hash instead of separate ' + + 'length and omission arguments', caller) - if text - l = options[:length] - options[:omission].chars.length - chars = text.chars - (chars.length > options[:length] ? chars[0...l] + options[:omission] : text).to_s - end + options[:length] = args[0] || 30 + options[:omission] = args[1] || "..." end - else - def truncate(text, *args) #:nodoc: - options = args.extract_options! - unless args.empty? - ActiveSupport::Deprecation.warn('truncate takes an option hash instead of separate ' + - 'length and omission arguments', caller) - - options[:length] = args[0] || 30 - options[:omission] = args[1] || "..." - end - options.reverse_merge!(:length => 30, :omission => "...") + options.reverse_merge!(:length => 30, :omission => "...") - if text - l = options[:length].to_i - options[:omission].length - (text.length > options[:length].to_i ? text[0...l] + options[:omission] : text).to_s - end + if text + l = options[:length] - options[:omission].mb_chars.length + chars = text.mb_chars + (chars.length > options[:length] ? chars[0...l] + options[:omission] : text).to_s end end @@ -140,7 +121,6 @@ module ActionView end end - if RUBY_VERSION < '1.9' # Extracts an excerpt from +text+ that matches the first instance of +phrase+. # The :radius option expands the excerpt on each side of the first occurrence of +phrase+ by the number of characters # defined in :radius (which defaults to 100). If the excerpt radius overflows the beginning or end of the +text+, @@ -179,45 +159,19 @@ module ActionView if text && phrase phrase = Regexp.escape(phrase) - if found_pos = text.chars =~ /(#{phrase})/i + if found_pos = text.mb_chars =~ /(#{phrase})/i start_pos = [ found_pos - options[:radius], 0 ].max - end_pos = [ [ found_pos + phrase.chars.length + options[:radius] - 1, 0].max, text.chars.length ].min + end_pos = [ [ found_pos + phrase.mb_chars.length + options[:radius] - 1, 0].max, text.mb_chars.length ].min prefix = start_pos > 0 ? options[:omission] : "" - postfix = end_pos < text.chars.length - 1 ? options[:omission] : "" + postfix = end_pos < text.mb_chars.length - 1 ? options[:omission] : "" - prefix + text.chars[start_pos..end_pos].strip + postfix + prefix + text.mb_chars[start_pos..end_pos].strip + postfix else nil end end end - else - def excerpt(text, phrase, *args) #:nodoc: - options = args.extract_options! - unless args.empty? - options[:radius] = args[0] || 100 - options[:omission] = args[1] || "..." - end - options.reverse_merge!(:radius => 100, :omission => "...") - - if text && phrase - phrase = Regexp.escape(phrase) - - if found_pos = text =~ /(#{phrase})/i - start_pos = [ found_pos - options[:radius], 0 ].max - end_pos = [ [ found_pos + phrase.length + options[:radius] - 1, 0].max, text.length ].min - - prefix = start_pos > 0 ? options[:omission] : "" - postfix = end_pos < text.length - 1 ? options[:omission] : "" - - prefix + text[start_pos..end_pos].strip + postfix - else - nil - end - end - end - end # Attempts to pluralize the +singular+ word unless +count+ is 1. If # +plural+ is supplied, it will use that when count is > 1, otherwise -- cgit v1.2.3 From b8eec5ac33d6f421fe5a2c757794ed2e4965f81d Mon Sep 17 00:00:00 2001 From: Manfred Stienstra Date: Sun, 21 Sep 2008 17:28:46 +0200 Subject: Remove special 1.9 version of excerpt helper. --- actionpack/lib/action_view/helpers/text_helper.rb | 90 +++++++++++------------ 1 file changed, 45 insertions(+), 45 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 4e371149af..3af7440400 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -121,57 +121,57 @@ module ActionView end end - # Extracts an excerpt from +text+ that matches the first instance of +phrase+. - # The :radius option expands the excerpt on each side of the first occurrence of +phrase+ by the number of characters - # defined in :radius (which defaults to 100). If the excerpt radius overflows the beginning or end of the +text+, - # then the :omission option (which defaults to "...") will be prepended/appended accordingly. The resulting string - # will be stripped in any case. If the +phrase+ isn't found, nil is returned. - # - # ==== Examples - # excerpt('This is an example', 'an', :radius => 5) - # # => ...s is an exam... - # - # excerpt('This is an example', 'is', :radius => 5) - # # => This is a... - # - # excerpt('This is an example', 'is') - # # => This is an example - # - # excerpt('This next thing is an example', 'ex', :radius => 2) - # # => ...next... - # - # excerpt('This is also an example', 'an', :radius => 8, :omission => ' ') - # # => is also an example - # - # You can still use excerpt with the old API that accepts the - # +radius+ as its optional third and the +ellipsis+ as its - # optional forth parameter: - # excerpt('This is an example', 'an', 5) # => ...s is an exam... - # excerpt('This is also an example', 'an', 8, ' ') # => is also an example - def excerpt(text, phrase, *args) - options = args.extract_options! - unless args.empty? - options[:radius] = args[0] || 100 - options[:omission] = args[1] || "..." - end - options.reverse_merge!(:radius => 100, :omission => "...") + # Extracts an excerpt from +text+ that matches the first instance of +phrase+. + # The :radius option expands the excerpt on each side of the first occurrence of +phrase+ by the number of characters + # defined in :radius (which defaults to 100). If the excerpt radius overflows the beginning or end of the +text+, + # then the :omission option (which defaults to "...") will be prepended/appended accordingly. The resulting string + # will be stripped in any case. If the +phrase+ isn't found, nil is returned. + # + # ==== Examples + # excerpt('This is an example', 'an', :radius => 5) + # # => ...s is an exam... + # + # excerpt('This is an example', 'is', :radius => 5) + # # => This is a... + # + # excerpt('This is an example', 'is') + # # => This is an example + # + # excerpt('This next thing is an example', 'ex', :radius => 2) + # # => ...next... + # + # excerpt('This is also an example', 'an', :radius => 8, :omission => ' ') + # # => is also an example + # + # You can still use excerpt with the old API that accepts the + # +radius+ as its optional third and the +ellipsis+ as its + # optional forth parameter: + # excerpt('This is an example', 'an', 5) # => ...s is an exam... + # excerpt('This is also an example', 'an', 8, ' ') # => is also an example + def excerpt(text, phrase, *args) + options = args.extract_options! + unless args.empty? + options[:radius] = args[0] || 100 + options[:omission] = args[1] || "..." + end + options.reverse_merge!(:radius => 100, :omission => "...") - if text && phrase - phrase = Regexp.escape(phrase) + if text && phrase + phrase = Regexp.escape(phrase) - if found_pos = text.mb_chars =~ /(#{phrase})/i - start_pos = [ found_pos - options[:radius], 0 ].max - end_pos = [ [ found_pos + phrase.mb_chars.length + options[:radius] - 1, 0].max, text.mb_chars.length ].min + if found_pos = text.mb_chars =~ /(#{phrase})/i + start_pos = [ found_pos - options[:radius], 0 ].max + end_pos = [ [ found_pos + phrase.mb_chars.length + options[:radius] - 1, 0].max, text.mb_chars.length ].min - prefix = start_pos > 0 ? options[:omission] : "" - postfix = end_pos < text.mb_chars.length - 1 ? options[:omission] : "" + prefix = start_pos > 0 ? options[:omission] : "" + postfix = end_pos < text.mb_chars.length - 1 ? options[:omission] : "" - prefix + text.mb_chars[start_pos..end_pos].strip + postfix - else - nil - end + prefix + text.mb_chars[start_pos..end_pos].strip + postfix + else + nil end end + end # Attempts to pluralize the +singular+ word unless +count+ is 1. If # +plural+ is supplied, it will use that when count is > 1, otherwise -- cgit v1.2.3 From 900fd6eca9dd97d2341e89bcb27d7a82d62965bf Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 22 Sep 2008 13:12:32 -0500 Subject: Refactor AssetTagHelper and fix remaining threadsafe issues. --- .../lib/action_view/helpers/asset_tag_helper.rb | 464 ++++++++++++++------- actionpack/test/template/asset_tag_helper_test.rb | 14 +- 2 files changed, 326 insertions(+), 152 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index a926599e25..fb711e7c3f 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -151,7 +151,7 @@ module ActionView # javascript_path "http://www.railsapplication.com/js/xmlhr" # => http://www.railsapplication.com/js/xmlhr.js # javascript_path "http://www.railsapplication.com/js/xmlhr.js" # => http://www.railsapplication.com/js/xmlhr.js def javascript_path(source) - compute_public_path(source, 'javascripts', 'js') + JavaScriptTag.create(self, @controller, source).public_path end alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route @@ -249,15 +249,17 @@ module ActionView joined_javascript_name = (cache == true ? "all" : cache) + ".js" joined_javascript_path = File.join(JAVASCRIPTS_DIR, joined_javascript_name) - write_asset_file_contents(joined_javascript_path, compute_javascript_paths(sources, recursive)) unless File.exists?(joined_javascript_path) + unless File.exists?(joined_javascript_path) + JavaScriptSources.create(self, @controller, sources, recursive).write_asset_file_contents(joined_javascript_path) + end javascript_src_tag(joined_javascript_name, options) else - expand_javascript_sources(sources, recursive).collect { |source| javascript_src_tag(source, options) }.join("\n") + JavaScriptSources.create(self, @controller, sources, recursive).expand_sources.collect { |source| + javascript_src_tag(source, options) + }.join("\n") end end - @@javascript_expansions = { :defaults => JAVASCRIPT_DEFAULT_SOURCES.dup } - # Register one or more javascript files to be included when symbol # is passed to javascript_include_tag. This method is typically intended # to be called from plugin initialization to register javascript files @@ -270,11 +272,9 @@ module ActionView # # def self.register_javascript_expansion(expansions) - @@javascript_expansions.merge!(expansions) + JavaScriptSources.expansions.merge!(expansions) end - @@stylesheet_expansions = {} - # Register one or more stylesheet files to be included when symbol # is passed to stylesheet_link_tag. This method is typically intended # to be called from plugin initialization to register stylesheet files @@ -287,7 +287,7 @@ module ActionView # # def self.register_stylesheet_expansion(expansions) - @@stylesheet_expansions.merge!(expansions) + StylesheetSources.expansions.merge!(expansions) end # Register one or more additional JavaScript files to be included when @@ -295,11 +295,11 @@ module ActionView # typically intended to be called from plugin initialization to register additional # .js files that the plugin installed in public/javascripts. def self.register_javascript_include_default(*sources) - @@javascript_expansions[:defaults].concat(sources) + JavaScriptSources.expansions[:defaults].concat(sources) end def self.reset_javascript_include_default #:nodoc: - @@javascript_expansions[:defaults] = JAVASCRIPT_DEFAULT_SOURCES.dup + JavaScriptSources.expansions[:defaults] = JAVASCRIPT_DEFAULT_SOURCES.dup end # Computes the path to a stylesheet asset in the public stylesheets directory. @@ -314,7 +314,7 @@ module ActionView # stylesheet_path "http://www.railsapplication.com/css/style" # => http://www.railsapplication.com/css/style.css # stylesheet_path "http://www.railsapplication.com/css/style.js" # => http://www.railsapplication.com/css/style.css def stylesheet_path(source) - compute_public_path(source, 'stylesheets', 'css') + StylesheetTag.create(self, @controller, source).public_path end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route @@ -389,10 +389,14 @@ module ActionView joined_stylesheet_name = (cache == true ? "all" : cache) + ".css" joined_stylesheet_path = File.join(STYLESHEETS_DIR, joined_stylesheet_name) - write_asset_file_contents(joined_stylesheet_path, compute_stylesheet_paths(sources, recursive)) unless File.exists?(joined_stylesheet_path) + unless File.exists?(joined_stylesheet_path) + StylesheetSources.create(self, @controller, sources, recursive).write_asset_file_contents(joined_stylesheet_path) + end stylesheet_tag(joined_stylesheet_name, options) else - expand_stylesheet_sources(sources, recursive).collect { |source| stylesheet_tag(source, options) }.join("\n") + StylesheetSources.create(self, @controller, sources, recursive).expand_sources.collect { |source| + stylesheet_tag(source, options) + }.join("\n") end end @@ -407,7 +411,7 @@ module ActionView # image_path("/icons/edit.png") # => /icons/edit.png # image_path("http://www.railsapplication.com/img/edit.png") # => http://www.railsapplication.com/img/edit.png def image_path(source) - compute_public_path(source, 'images') + ImageTag.create(self, @controller, source).public_path end alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route @@ -463,180 +467,344 @@ module ActionView end private - COMPUTED_PUBLIC_PATHS = {} - COMPUTED_PUBLIC_PATHS_GUARD = Mutex.new - - # Add the the extension +ext+ if not present. Return full URLs otherwise untouched. - # Prefix with /dir/ if lacking a leading +/+. Account for relative URL - # roots. Rewrite the asset path for cache-busting asset ids. Include - # asset host, if configured, with the correct request protocol. - def compute_public_path(source, dir, ext = nil, include_host = true) - has_request = @controller.respond_to?(:request) - - cache_key = - if has_request - [ @controller.request.protocol, - ActionController::Base.asset_host.to_s, - ActionController::Base.relative_url_root, - dir, source, ext, include_host ].join - else - [ ActionController::Base.asset_host.to_s, - dir, source, ext, include_host ].join - end + def javascript_src_tag(source, options) + content_tag("script", "", { "type" => Mime::JS, "src" => path_to_javascript(source) }.merge(options)) + end - COMPUTED_PUBLIC_PATHS_GUARD.synchronize do - source = COMPUTED_PUBLIC_PATHS[cache_key] ||= - begin - source += ".#{ext}" if ext && File.extname(source).blank? || File.exist?(File.join(ASSETS_DIR, dir, "#{source}.#{ext}")) + def stylesheet_tag(source, options) + tag("link", { "rel" => "stylesheet", "type" => Mime::CSS, "media" => "screen", "href" => html_escape(path_to_stylesheet(source)) }.merge(options), false, false) + end - if source =~ %r{^[-a-z]+://} - source - else - source = "/#{dir}/#{source}" unless source[0] == ?/ - if has_request - unless source =~ %r{^#{ActionController::Base.relative_url_root}/} - source = "#{ActionController::Base.relative_url_root}#{source}" - end - end + module ImageAsset + DIRECTORY = 'images'.freeze - rewrite_asset_path(source) - end - end + def directory + DIRECTORY end - if include_host && source !~ %r{^[-a-z]+://} - host = compute_asset_host(source) + def extension + nil + end + end - if has_request && !host.blank? && host !~ %r{^[-a-z]+://} - host = "#{@controller.request.protocol}#{host}" - end + module JavaScriptAsset + DIRECTORY = 'javascripts'.freeze + EXTENSION = 'js'.freeze + + def public_directory + JAVASCRIPTS_DIR + end + + def directory + DIRECTORY + end + + def extension + EXTENSION + end + end + + module StylesheetAsset + DIRECTORY = 'stylesheets'.freeze + EXTENSION = 'css'.freeze + + def public_directory + STYLESHEETS_DIR + end + + def directory + DIRECTORY + end - "#{host}#{source}" - else - source + def extension + EXTENSION end end - # Pick an asset host for this source. Returns +nil+ if no host is set, - # the host if no wildcard is set, the host interpolated with the - # numbers 0-3 if it contains %d (the number is the source hash mod 4), - # or the value returned from invoking the proc if it's a proc. - def compute_asset_host(source) - if host = ActionController::Base.asset_host - if host.is_a?(Proc) - case host.arity - when 2 - host.call(source, @controller.request) + class AssetTag + extend ActiveSupport::Memoizable + + Cache = {} + CacheGuard = Mutex.new + + def self.create(template, controller, source, include_host = true) + CacheGuard.synchronize do + key = if controller.respond_to?(:request) + [self, controller.request.protocol, + ActionController::Base.asset_host, + ActionController::Base.relative_url_root, + source, include_host] else - host.call(source) + [self, ActionController::Base.asset_host, source, include_host] end - else - (host =~ /%d/) ? host % (source.hash % 4) : host + Cache[key] ||= new(template, controller, source, include_host).freeze end end - end - # Use the RAILS_ASSET_ID environment variable or the source's - # modification time as its cache-busting asset id. - def rails_asset_id(source) - if asset_id = ENV["RAILS_ASSET_ID"] - asset_id - else - path = File.join(ASSETS_DIR, source) + ProtocolRegexp = %r{^[-a-z]+://}.freeze - if File.exist?(path) - File.mtime(path).to_i.to_s - else - '' - end + def initialize(template, controller, source, include_host = true) + # NOTE: The template arg is temporarily needed for a legacy plugin + # hook that is expected to call rewrite_asset_path on the + # template. This should eventually be removed. + @template = template + @controller = controller + @source = source + @include_host = include_host end - end - # Break out the asset path rewrite in case plugins wish to put the asset id - # someplace other than the query string. - def rewrite_asset_path(source) - asset_id = rails_asset_id(source) - if asset_id.blank? - source - else - source + "?#{asset_id}" + def public_path + compute_public_path(@source) end - end + memoize :public_path - def javascript_src_tag(source, options) - content_tag("script", "", { "type" => Mime::JS, "src" => path_to_javascript(source) }.merge(options)) + def asset_file_path + File.join(ASSETS_DIR, public_path.split('?').first) + end + memoize :asset_file_path + + def contents + File.read(asset_file_path) + end + + def mtime + File.mtime(asset_file_path) + end + + private + def request + @controller.request + end + + def request? + @controller.respond_to?(:request) + end + + # Add the the extension +ext+ if not present. Return full URLs otherwise untouched. + # Prefix with /dir/ if lacking a leading +/+. Account for relative URL + # roots. Rewrite the asset path for cache-busting asset ids. Include + # asset host, if configured, with the correct request protocol. + def compute_public_path(source) + source += ".#{extension}" if missing_extension?(source) + unless source =~ ProtocolRegexp + source = "/#{directory}/#{source}" unless source[0] == ?/ + source = prepend_relative_url_root(source) + source = rewrite_asset_path(source) + end + source = prepend_asset_host(source) + source + end + + def missing_extension?(source) + extension && File.extname(source).blank? || File.exist?(File.join(ASSETS_DIR, directory, "#{source}.#{extension}")) + end + + def prepend_relative_url_root(source) + relative_url_root = ActionController::Base.relative_url_root + if request? && source !~ %r{^#{relative_url_root}/} + "#{relative_url_root}#{source}" + else + source + end + end + + def prepend_asset_host(source) + if @include_host && source !~ ProtocolRegexp + host = compute_asset_host(source) + if request? && !host.blank? && host !~ ProtocolRegexp + host = "#{request.protocol}#{host}" + end + "#{host}#{source}" + else + source + end + end + + # Pick an asset host for this source. Returns +nil+ if no host is set, + # the host if no wildcard is set, the host interpolated with the + # numbers 0-3 if it contains %d (the number is the source hash mod 4), + # or the value returned from invoking the proc if it's a proc. + def compute_asset_host(source) + if host = ActionController::Base.asset_host + if host.is_a?(Proc) + case host.arity + when 2 + host.call(source, request) + else + host.call(source) + end + else + (host =~ /%d/) ? host % (source.hash % 4) : host + end + end + end + + # Use the RAILS_ASSET_ID environment variable or the source's + # modification time as its cache-busting asset id. + def rails_asset_id(source) + if asset_id = ENV["RAILS_ASSET_ID"] + asset_id + else + path = File.join(ASSETS_DIR, source) + + if File.exist?(path) + File.mtime(path).to_i.to_s + else + '' + end + end + end + + # Break out the asset path rewrite in case plugins wish to put the asset id + # someplace other than the query string. + def rewrite_asset_path(source) + if @template.respond_to?(:rewrite_asset_path) + # DEPRECATE: This way to override rewrite_asset_path + @template.send(:rewrite_asset_path, source) + else + asset_id = rails_asset_id(source) + if asset_id.blank? + source + else + "#{source}?#{asset_id}" + end + end + end end - def stylesheet_tag(source, options) - tag("link", { "rel" => "stylesheet", "type" => Mime::CSS, "media" => "screen", "href" => html_escape(path_to_stylesheet(source)) }.merge(options), false, false) + class ImageTag < AssetTag + include ImageAsset end - def compute_javascript_paths(*args) - expand_javascript_sources(*args).collect { |source| compute_public_path(source, 'javascripts', 'js', false) } + class JavaScriptTag < AssetTag + include JavaScriptAsset end - def compute_stylesheet_paths(*args) - expand_stylesheet_sources(*args).collect { |source| compute_public_path(source, 'stylesheets', 'css', false) } + class StylesheetTag < AssetTag + include StylesheetAsset end - def expand_javascript_sources(sources, recursive = false) - if sources.include?(:all) - 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) - end.flatten - expanded_sources << "application" if sources.include?(:defaults) && File.exist?(File.join(JAVASCRIPTS_DIR, "application.js")) - expanded_sources + class AssetCollection + extend ActiveSupport::Memoizable + + Cache = {} + CacheGuard = Mutex.new + + def self.create(template, controller, sources, recursive) + CacheGuard.synchronize do + key = [self, sources, recursive] + Cache[key] ||= new(template, controller, sources, recursive).freeze + end end - end - def expand_stylesheet_sources(sources, recursive) - if sources.first == :all - @@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) - end.flatten + def initialize(template, controller, sources, recursive) + # NOTE: The template arg is temporarily needed for a legacy plugin + # hook. See NOTE under AssetTag#initialize for more details + @template = template + @controller = controller + @sources = sources + @recursive = recursive end - end - def determine_source(source, collection) - case source - when Symbol - collection[source] || raise(ArgumentError, "No expansion found for #{source.inspect}") - else - source + def write_asset_file_contents(joined_asset_path) + FileUtils.mkdir_p(File.dirname(joined_asset_path)) + File.open(joined_asset_path, "w+") { |cache| cache.write(joined_contents) } + mt = latest_mtime + File.utime(mt, mt, joined_asset_path) end - end - def join_asset_file_contents(paths) - paths.collect { |path| File.read(asset_file_path(path)) }.join("\n\n") - end + private + def determine_source(source, collection) + case source + when Symbol + collection[source] || raise(ArgumentError, "No expansion found for #{source.inspect}") + else + source + end + end + + def validate_sources! + @sources.collect { |source| determine_source(source, self.class.expansions) }.flatten + end + + def all_asset_files + path = [public_directory, ('**' if @recursive), "*.#{extension}"].compact + Dir[File.join(*path)].collect { |file| + file[-(file.size - public_directory.size - 1)..-1].sub(/\.\w+$/, '') + }.sort + end + + def tag_sources + expand_sources.collect { |source| tag_class.create(@template, @controller, source, false) } + end - def write_asset_file_contents(joined_asset_path, asset_paths) - FileUtils.mkdir_p(File.dirname(joined_asset_path)) - File.open(joined_asset_path, "w+") { |cache| cache.write(join_asset_file_contents(asset_paths)) } + def joined_contents + tag_sources.collect { |source| source.contents }.join("\n\n") + end - # Set mtime to the latest of the combined files to allow for - # consistent ETag without a shared filesystem. - mt = asset_paths.map { |p| File.mtime(asset_file_path(p)) }.max - File.utime(mt, mt, joined_asset_path) + # Set mtime to the latest of the combined files to allow for + # consistent ETag without a shared filesystem. + def latest_mtime + tag_sources.map { |source| source.mtime }.max + end end - def asset_file_path(path) - File.join(ASSETS_DIR, path.split('?').first) + class JavaScriptSources < AssetCollection + include JavaScriptAsset + + EXPANSIONS = { :defaults => JAVASCRIPT_DEFAULT_SOURCES.dup } + + def self.expansions + EXPANSIONS + end + + APPLICATION_JS = "application".freeze + APPLICATION_FILE = "application.js".freeze + + def expand_sources + if @sources.include?(:all) + assets = all_asset_files + ((defaults.dup & assets) + assets).uniq! + else + expanded_sources = validate_sources! + expanded_sources << APPLICATION_JS if include_application? + expanded_sources + end + end + memoize :expand_sources + + private + def tag_class + JavaScriptTag + end + + def defaults + determine_source(:defaults, self.class.expansions) + end + + def include_application? + @sources.include?(:defaults) && File.exist?(File.join(JAVASCRIPTS_DIR, APPLICATION_FILE)) + end end - def collect_asset_files(*path) - dir = path.first + class StylesheetSources < AssetCollection + include StylesheetAsset + + EXPANSIONS = {} + + def self.expansions + EXPANSIONS + end - Dir[File.join(*path.compact)].collect do |file| - file[-(file.size - dir.size - 1)..-1].sub(/\.\w+$/, '') - end.sort + def expand_sources + @sources.first == :all ? all_asset_files : validate_sources! + end + memoize :expand_sources + + private + def tag_class + StylesheetTag + end end end end diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 7e40a55dc5..95fc044fbb 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -38,7 +38,8 @@ class AssetTagHelperTest < ActionView::TestCase @controller.request = @request ActionView::Helpers::AssetTagHelper::reset_javascript_include_default - COMPUTED_PUBLIC_PATHS.clear + AssetTag::Cache.clear + AssetCollection::Cache.clear end def teardown @@ -155,12 +156,12 @@ class AssetTagHelperTest < ActionView::TestCase PathToJavascriptToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end - def test_javascript_include_tag + def test_javascript_include_tag_with_blank_asset_id ENV["RAILS_ASSET_ID"] = "" JavascriptIncludeToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end - COMPUTED_PUBLIC_PATHS.clear - + def test_javascript_include_tag_with_given_asset_id ENV["RAILS_ASSET_ID"] = "1" assert_dom_equal(%(\n\n\n\n), javascript_include_tag(:defaults)) end @@ -169,6 +170,11 @@ class AssetTagHelperTest < ActionView::TestCase ENV["RAILS_ASSET_ID"] = "" ActionView::Helpers::AssetTagHelper::register_javascript_include_default 'slider' assert_dom_equal %(\n\n\n\n\n), javascript_include_tag(:defaults) + end + + def test_register_javascript_include_default_mixed_defaults + ENV["RAILS_ASSET_ID"] = "" + ActionView::Helpers::AssetTagHelper::register_javascript_include_default 'slider' ActionView::Helpers::AssetTagHelper::register_javascript_include_default 'lib1', '/elsewhere/blub/lib2' assert_dom_equal %(\n\n\n\n\n\n\n), javascript_include_tag(:defaults) end -- cgit v1.2.3 From 10380a22a65d93bee6775a0ffe93071b798bf249 Mon Sep 17 00:00:00 2001 From: Martin Rehfeld Date: Mon, 22 Sep 2008 13:23:23 -0500 Subject: Fixed AssetTag cache with with relative_url_root [#1022 state:resolved] Signed-off-by: Joshua Peek --- .../lib/action_view/helpers/asset_tag_helper.rb | 2 +- actionpack/test/template/asset_tag_helper_test.rb | 50 ++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index fb711e7c3f..63ccde393a 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -601,7 +601,7 @@ module ActionView def prepend_relative_url_root(source) relative_url_root = ActionController::Base.relative_url_root - if request? && source !~ %r{^#{relative_url_root}/} + if request? && @include_host && source !~ %r{^#{relative_url_root}/} "#{relative_url_root}#{source}" else source diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 95fc044fbb..aaf9fe2ebf 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -392,6 +392,31 @@ class AssetTagHelperTest < ActionView::TestCase FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) end + def test_caching_javascript_include_tag_with_relative_url_root + ENV["RAILS_ASSET_ID"] = "" + ActionController::Base.relative_url_root = "/collaboration/hieraki" + ActionController::Base.perform_caching = true + + assert_dom_equal( + %(), + javascript_include_tag(:all, :cache => true) + ) + + assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'all.js')) + + assert_dom_equal( + %(), + javascript_include_tag(:all, :cache => "money") + ) + + assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js')) + + ensure + ActionController::Base.relative_url_root = nil + FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'all.js')) + FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js')) + end + def test_caching_javascript_include_tag_when_caching_off ENV["RAILS_ASSET_ID"] = "" ActionController::Base.perform_caching = false @@ -462,6 +487,31 @@ class AssetTagHelperTest < ActionView::TestCase FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'styles.css')) end + def test_caching_stylesheet_link_tag_with_relative_url_root + ENV["RAILS_ASSET_ID"] = "" + ActionController::Base.relative_url_root = "/collaboration/hieraki" + ActionController::Base.perform_caching = true + + assert_dom_equal( + %(), + stylesheet_link_tag(:all, :cache => true) + ) + + expected = Dir["#{ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR}/*.css"].map { |p| File.mtime(p) }.max + assert_equal expected, File.mtime(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css')) + + assert_dom_equal( + %(), + stylesheet_link_tag(:all, :cache => "money") + ) + + assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css')) + ensure + ActionController::Base.relative_url_root = nil + FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css')) + FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css')) + end + def test_caching_stylesheet_include_tag_when_caching_off ENV["RAILS_ASSET_ID"] = "" ActionController::Base.perform_caching = false -- cgit v1.2.3 From 5f86451a4c5d0beca5a746c4708be48b13f665be Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 22 Sep 2008 17:14:54 +0200 Subject: Bump the Version constants to align with the *next* release rather than the previous release. This allows people tracking non-release gems or git submodules to use the constants. --- actionpack/lib/action_pack/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_pack/version.rb b/actionpack/lib/action_pack/version.rb index c67654d9a8..288b62778e 100644 --- a/actionpack/lib/action_pack/version.rb +++ b/actionpack/lib/action_pack/version.rb @@ -1,7 +1,7 @@ module ActionPack #:nodoc: module VERSION #:nodoc: MAJOR = 2 - MINOR = 1 + MINOR = 2 TINY = 0 STRING = [MAJOR, MINOR, TINY].join('.') -- cgit v1.2.3 From 025736de8eb3a4be1514f10123ce1569fa2d5427 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Wed, 24 Sep 2008 16:24:02 +0200 Subject: Use ActiveSupport::SecureRandom instead of the strange fallback code. --- .../lib/action_controller/cgi_ext/session.rb | 24 ++-------------------- 1 file changed, 2 insertions(+), 22 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/cgi_ext/session.rb b/actionpack/lib/action_controller/cgi_ext/session.rb index a01f17f9ce..d3f85e3705 100644 --- a/actionpack/lib/action_controller/cgi_ext/session.rb +++ b/actionpack/lib/action_controller/cgi_ext/session.rb @@ -6,28 +6,8 @@ class CGI #:nodoc: # * Expose the CGI instance to session stores. # * Don't require 'digest/md5' whenever a new session id is generated. class Session #:nodoc: - begin - require 'securerandom' - - # Generate a 32-character unique id using SecureRandom. - # This is used to generate session ids but may be reused elsewhere. - def self.generate_unique_id(constant = nil) - SecureRandom.hex(16) - end - rescue LoadError - # Generate an 32-character unique id based on a hash of the current time, - # a random number, the process id, and a constant string. This is used - # to generate session ids but may be reused elsewhere. - def self.generate_unique_id(constant = 'foobar') - md5 = Digest::MD5.new - now = Time.now - md5 << now.to_s - md5 << String(now.usec) - md5 << String(rand(0)) - md5 << String($$) - md5 << constant - md5.hexdigest - end + def self.generate_unique_id(constant = nil) + ActiveSupport::SecureRandom.hex(16) end # Make the CGI instance available to session stores. -- cgit v1.2.3