From 2e053aec9bafa8735d70886f36dea06ea10dc4ce Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 22 Dec 2008 14:48:32 -0800 Subject: Don't construct object deprecation proxy if unneeded --- actionpack/lib/action_view/renderable_partial.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_view/renderable_partial.rb b/actionpack/lib/action_view/renderable_partial.rb index d92ff1b8d3..3ea836fa25 100644 --- a/actionpack/lib/action_view/renderable_partial.rb +++ b/actionpack/lib/action_view/renderable_partial.rb @@ -25,12 +25,11 @@ module ActionView end def render_partial(view, object = nil, local_assigns = {}, as = nil) - object ||= local_assigns[:object] || - local_assigns[variable_name] + object ||= local_assigns[:object] || local_assigns[variable_name] - if view.respond_to?(:controller) + if object.nil? && view.respond_to?(:controller) ivar = :"@#{variable_name}" - object ||= + object = if view.controller.instance_variable_defined?(ivar) ActiveSupport::Deprecation::DeprecatedObjectProxy.new( view.controller.instance_variable_get(ivar), -- cgit v1.2.3 From 7db1704068b86fb2212388b14b4963526bacfa5d Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Fri, 26 Dec 2008 22:53:07 +0000 Subject: Fix :include of has_many associations with :primary_key option --- activerecord/lib/active_record/association_preload.rb | 2 +- activerecord/lib/active_record/associations.rb | 2 +- activerecord/test/cases/associations/eager_test.rb | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 9d0bf3a308..195d2fb4f8 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -229,7 +229,7 @@ module ActiveRecord options = reflection.options primary_key_name = reflection.through_reflection_primary_key_name - id_to_record_map, ids = construct_id_map(records, primary_key_name) + id_to_record_map, ids = construct_id_map(records, primary_key_name || reflection.options[:primary_key]) records.each {|record| record.send(reflection.name).loaded} if options[:through] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 5a60b13fd8..237e5c0e9d 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2171,7 +2171,7 @@ module ActiveRecord aliased_table_name, foreign_key, parent.aliased_table_name, - parent.primary_key + reflection.options[:primary_key] || parent.primary_key ] end when :belongs_to diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index afbd9fddf9..ff3d6ece05 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -786,4 +786,21 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal Person.find(person.id).agents, person.agents end end + + def test_preload_has_many_using_primary_key + expected = Firm.find(:first).clients_using_primary_key.to_a + firm = Firm.find :first, :include => :clients_using_primary_key + assert_no_queries do + assert_equal expected, firm.clients_using_primary_key + end + end + + def test_include_has_many_using_primary_key + expected = Firm.find(1).clients_using_primary_key.sort_by &:name + firm = Firm.find 1, :include => :clients_using_primary_key, :order => 'clients_using_primary_keys_companies.name' + assert_no_queries do + assert_equal expected, firm.clients_using_primary_key + end + end + end -- cgit v1.2.3 From f9cab0e503a4721c9d0369f89bb85c6e658f778c Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Fri, 26 Dec 2008 23:26:37 +0000 Subject: Fix :include of has_one with :primary_key option --- activerecord/lib/active_record/association_preload.rb | 2 +- activerecord/test/cases/associations/eager_test.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 195d2fb4f8..e4ab69aa1b 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -195,7 +195,7 @@ module ActiveRecord def preload_has_one_association(records, reflection, preload_options={}) return if records.first.send("loaded_#{reflection.name}?") - id_to_record_map, ids = construct_id_map(records) + id_to_record_map, ids = construct_id_map(records, reflection.options[:primary_key]) options = reflection.options records.each {|record| record.send("set_#{reflection.name}_target", nil)} if options[:through] diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index ff3d6ece05..14099d4176 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -803,4 +803,20 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_preload_has_one_using_primary_key + expected = Firm.find(:first).account_using_primary_key + firm = Firm.find :first, :include => :account_using_primary_key + assert_no_queries do + assert_equal expected, firm.account_using_primary_key + end + end + + def test_include_has_one_using_primary_key + expected = Firm.find(1).account_using_primary_key + firm = Firm.find(:all, :include => :account_using_primary_key, :order => 'accounts.id').detect {|f| f.id == 1} + assert_no_queries do + assert_equal expected, firm.account_using_primary_key + end + end + end -- cgit v1.2.3 From 21efba464afa2ae6e5dfd938ac8a3ce446faf7e7 Mon Sep 17 00:00:00 2001 From: Roman Shterenzon Date: Sat, 27 Dec 2008 01:10:29 +0000 Subject: Fix HasManyAssociation#create ignoring the :primary_key option [#1633 state:resolved] Signed-off-by: Frederick Cheung --- activerecord/lib/active_record/associations/association_proxy.rb | 5 ++++- activerecord/test/cases/associations/has_many_associations_test.rb | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 59f1d3b867..676c4ace61 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -180,7 +180,10 @@ module ActiveRecord record["#{@reflection.options[:as]}_id"] = @owner.id unless @owner.new_record? record["#{@reflection.options[:as]}_type"] = @owner.class.base_class.name.to_s else - record[@reflection.primary_key_name] = @owner.id unless @owner.new_record? + unless @owner.new_record? + primary_key = @reflection.options[:primary_key] || :id + record[@reflection.primary_key_name] = @owner.send(primary_key) + end end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 20b9acda44..a5ae5cd8ec 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1115,5 +1115,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !client_association.respond_to?(:private_method) assert client_association.respond_to?(:private_method, true) end + + def test_creating_using_primary_key + firm = Firm.find(:first) + client = firm.clients_using_primary_key.create!(:name => 'test') + assert_equal firm.name, client.firm_name + end end -- cgit v1.2.3 From afdec83ed543e904b495d3225b6401101ea7ba6c Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Sat, 27 Dec 2008 14:16:17 +0000 Subject: Fix to_sentence being used with options removed by 273c77 --- activerecord/lib/active_record/associations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 237e5c0e9d..eba10b505e 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -22,7 +22,7 @@ module ActiveRecord through_reflection = reflection.through_reflection source_reflection_names = reflection.source_reflection_names source_associations = reflection.through_reflection.klass.reflect_on_all_associations.collect { |a| a.name.inspect } - super("Could not find the source association(s) #{source_reflection_names.collect(&:inspect).to_sentence :connector => 'or'} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => '. Is it one of #{source_associations.to_sentence :connector => 'or'}?") + super("Could not find the source association(s) #{source_reflection_names.collect(&:inspect).to_sentence :two_words_connector => ' or ', :last_word_connector => ', or '} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => '. Is it one of #{source_associations.to_sentence :two_words_connector => ' or ', :last_word_connector => ', or '}?") end end -- cgit v1.2.3 From 5138f755ff31a8f317d649a6f256c74bc371db70 Mon Sep 17 00:00:00 2001 From: Mark Reginald James Date: Sun, 28 Dec 2008 01:15:48 +0000 Subject: Fixed incorrect parsing of query parameters with mixed-depth nesting inside an array [#1622 state:resolved] Signed-off-by: Frederick Cheung --- actionpack/lib/action_controller/url_encoded_pair_parser.rb | 9 ++++----- actionpack/test/controller/request_test.rb | 1 + 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_controller/url_encoded_pair_parser.rb b/actionpack/lib/action_controller/url_encoded_pair_parser.rb index bea96c711d..9883ad0d85 100644 --- a/actionpack/lib/action_controller/url_encoded_pair_parser.rb +++ b/actionpack/lib/action_controller/url_encoded_pair_parser.rb @@ -70,11 +70,12 @@ module ActionController top[-1][key] = value else top << {key => value}.with_indifferent_access - push top.last - value = top[key] end + push top.last + return top[key] else top << value + return value end elsif top.is_a? Hash key = CGI.unescape(key) @@ -84,12 +85,10 @@ module ActionController else raise ArgumentError, "Don't know what to do: top is #{top.inspect}" end - - return value end def type_conflict!(klass, value) raise TypeError, "Conflicting types for parameter containers. Expected an instance of #{klass} but found an instance of #{value.class}. This can be caused by colliding Array and Hash parameters like qs[]=value&qs[key]=value. (The parameters received were #{value.inspect}.)" end end -end \ No newline at end of file +end diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index 349cea268f..c93d3152b8 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -441,6 +441,7 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase def test_deep_query_string_with_array_of_hash assert_equal({'x' => {'y' => [{'z' => '10'}]}}, ActionController::RequestParser.parse_query_parameters('x[y][][z]=10')) assert_equal({'x' => {'y' => [{'z' => '10', 'w' => '10'}]}}, ActionController::RequestParser.parse_query_parameters('x[y][][z]=10&x[y][][w]=10')) + assert_equal({'x' => {'y' => [{'z' => '10', 'v' => {'w' => '10'}}]}}, ActionController::RequestParser.parse_query_parameters('x[y][][z]=10&x[y][][v][w]=10')) end def test_deep_query_string_with_array_of_hashes_with_one_pair -- cgit v1.2.3 From 1e45818a622405e720a4529795f8be2f11660361 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:07:17 -0600 Subject: Allow multiple conditions for callbacks [#1627 state:resolved] --- activesupport/lib/active_support/callbacks.rb | 9 ++---- activesupport/test/callbacks_test.rb | 42 ++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 992827f7f4..86e66e0588 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -192,13 +192,8 @@ module ActiveSupport end def should_run_callback?(*args) - if options[:if] - evaluate_method(options[:if], *args) - elsif options[:unless] - !evaluate_method(options[:unless], *args) - else - true - end + [options[:if]].flatten.compact.all? { |a| evaluate_method(a, *args) } && + ![options[:unless]].flatten.compact.any? { |a| evaluate_method(a, *args) } end end diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 25b8eecef5..2bc2e1eaf0 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -53,10 +53,41 @@ class Person < Record end class ConditionalPerson < Record + # proc before_save Proc.new { |r| r.history << [:before_save, :proc] }, :if => Proc.new { |r| true } before_save Proc.new { |r| r.history << "b00m" }, :if => Proc.new { |r| false } before_save Proc.new { |r| r.history << [:before_save, :proc] }, :unless => Proc.new { |r| false } before_save Proc.new { |r| r.history << "b00m" }, :unless => Proc.new { |r| true } + # symbol + before_save Proc.new { |r| r.history << [:before_save, :symbol] }, :if => :yes + before_save Proc.new { |r| r.history << "b00m" }, :if => :no + before_save Proc.new { |r| r.history << [:before_save, :symbol] }, :unless => :no + before_save Proc.new { |r| r.history << "b00m" }, :unless => :yes + # string + before_save Proc.new { |r| r.history << [:before_save, :string] }, :if => 'yes' + before_save Proc.new { |r| r.history << "b00m" }, :if => 'no' + before_save Proc.new { |r| r.history << [:before_save, :string] }, :unless => 'no' + before_save Proc.new { |r| r.history << "b00m" }, :unless => 'yes' + # Array with conditions + before_save Proc.new { |r| r.history << [:before_save, :symbol_array] }, :if => [:yes, :other_yes] + before_save Proc.new { |r| r.history << "b00m" }, :if => [:yes, :no] + before_save Proc.new { |r| r.history << [:before_save, :symbol_array] }, :unless => [:no, :other_no] + before_save Proc.new { |r| r.history << "b00m" }, :unless => [:yes, :no] + # Combined if and unless + before_save Proc.new { |r| r.history << [:before_save, :combined_symbol] }, :if => :yes, :unless => :no + before_save Proc.new { |r| r.history << "b00m" }, :if => :yes, :unless => :yes + # Array with different types of conditions + before_save Proc.new { |r| r.history << [:before_save, :symbol_proc_string_array] }, :if => [:yes, Proc.new { |r| true }, 'yes'] + before_save Proc.new { |r| r.history << "b00m" }, :if => [:yes, Proc.new { |r| true }, 'no'] + # Array with different types of conditions comibned if and unless + before_save Proc.new { |r| r.history << [:before_save, :combined_symbol_proc_string_array] }, + :if => [:yes, Proc.new { |r| true }, 'yes'], :unless => [:no, 'no'] + before_save Proc.new { |r| r.history << "b00m" }, :if => [:yes, Proc.new { |r| true }, 'no'], :unless => [:no, 'no'] + + def yes; true; end + def other_yes; true; end + def no; false; end + def other_no; false; end def save run_callbacks(:before_save) @@ -90,7 +121,16 @@ class ConditionalCallbackTest < Test::Unit::TestCase person.save assert_equal [ [:before_save, :proc], - [:before_save, :proc] + [:before_save, :proc], + [:before_save, :symbol], + [:before_save, :symbol], + [:before_save, :string], + [:before_save, :string], + [:before_save, :symbol_array], + [:before_save, :symbol_array], + [:before_save, :combined_symbol], + [:before_save, :symbol_proc_string_array], + [:before_save, :combined_symbol_proc_string_array] ], person.history end end -- cgit v1.2.3 From 1f0aecd931a9292b52402143be979ab4c06f06cd Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:10:41 -0600 Subject: Allow custom rails generators to pass in their own binding to Create command so that the corresponding erb templates get rendered with the proper binding [#1493 state:resolved] --- railties/lib/rails_generator/commands.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails_generator/commands.rb b/railties/lib/rails_generator/commands.rb index cacb3807d6..299044c3d7 100644 --- a/railties/lib/rails_generator/commands.rb +++ b/railties/lib/rails_generator/commands.rb @@ -294,7 +294,7 @@ HELP file(relative_source, relative_destination, template_options) do |file| # Evaluate any assignments in a temporary, throwaway binding. vars = template_options[:assigns] || {} - b = binding + b = template_options[:binding] || binding vars.each { |k,v| eval "#{k} = vars[:#{k}] || vars['#{k}']", b } # Render the source file with the temporary binding. -- cgit v1.2.3 From 45dee3842d68359a189fe7c0729359bd5a905ea4 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:13:16 -0600 Subject: HTTP Digest authentication [#1230 state:resolved] --- .../lib/action_controller/http_authentication.rb | 191 ++++++++++++++++++++- actionpack/lib/action_controller/integration.rb | 82 +++++++++ .../controller/http_digest_authentication_test.rb | 73 ++++++++ actionpack/test/controller/integration_test.rb | 88 ++++++++++ 4 files changed, 432 insertions(+), 2 deletions(-) create mode 100644 actionpack/test/controller/http_digest_authentication_test.rb diff --git a/actionpack/lib/action_controller/http_authentication.rb b/actionpack/lib/action_controller/http_authentication.rb index 2ed810db7d..3cb5829eca 100644 --- a/actionpack/lib/action_controller/http_authentication.rb +++ b/actionpack/lib/action_controller/http_authentication.rb @@ -55,7 +55,31 @@ module ActionController # end # end # - # + # Simple Digest example. Note the block must return the user's password so the framework + # can appropriately hash it to check the user's credentials. Returning nil will cause authentication to fail. + # + # class PostsController < ApplicationController + # Users = {"dhh" => "secret"} + # + # before_filter :authenticate, :except => [ :index ] + # + # def index + # render :text => "Everyone can see me!" + # end + # + # def edit + # render :text => "I'm only accessible if you know the password" + # end + # + # private + # def authenticate + # authenticate_or_request_with_http_digest(realm) do |user_name| + # Users[user_name] + # end + # end + # end + # + # # In your integration tests, you can do something like this: # # def test_access_granted_from_xml @@ -108,7 +132,10 @@ module ActionController end def decode_credentials(request) - ActiveSupport::Base64.decode64(authorization(request).split.last || '') + # Properly decode credentials spanning a new-line + auth = authorization(request) + auth.slice!('Basic ') + ActiveSupport::Base64.decode64(auth || '') end def encode_credentials(user_name, password) @@ -120,5 +147,165 @@ module ActionController controller.__send__ :render, :text => "HTTP Basic: Access denied.\n", :status => :unauthorized end end + + module Digest + extend self + + module ControllerMethods + def authenticate_or_request_with_http_digest(realm = "Application", &password_procedure) + begin + authenticate_with_http_digest!(realm, &password_procedure) + rescue ActionController::HttpAuthentication::Error => e + msg = e.message + msg = "#{msg} expected '#{e.expected}' was '#{e.was}'" unless e.expected.nil? + raise msg if e.fatal? + request_http_digest_authentication(realm, msg) + end + end + + # Authenticate using HTTP Digest, throwing ActionController::HttpAuthentication::Error on failure. + # This allows more detailed analysis of authentication failures + # to be relayed to the client. + def authenticate_with_http_digest!(realm = "Application", &login_procedure) + HttpAuthentication::Digest.authenticate(self, realm, &login_procedure) + end + + # Authenticate with HTTP Digest, returns true or false + def authenticate_with_http_digest(realm = "Application", &login_procedure) + HttpAuthentication::Digest.authenticate(self, realm, &login_procedure) rescue false + end + + # Render output including the HTTP Digest authentication header + def request_http_digest_authentication(realm = "Application", message = nil) + HttpAuthentication::Digest.authentication_request(self, realm, message) + end + + # Add HTTP Digest authentication header to result headers + def http_digest_authentication_header(realm = "Application") + HttpAuthentication::Digest.authentication_header(self, realm) + end + end + + # Raises error unless authentictaion succeeds, returns true otherwise + def authenticate(controller, realm, &password_procedure) + raise Error.new(false), "No authorization header found" unless authorization(controller.request) + validate_digest_response(controller, realm, &password_procedure) + true + end + + def authorization(request) + request.env['HTTP_AUTHORIZATION'] || + request.env['X-HTTP_AUTHORIZATION'] || + request.env['X_HTTP_AUTHORIZATION'] || + request.env['REDIRECT_X_HTTP_AUTHORIZATION'] + end + + # Raises error unless the request credentials response value matches the expected value. + def validate_digest_response(controller, realm, &password_procedure) + credentials = decode_credentials(controller.request) + + # Check the nonce, opaque and realm. + # Ignore nc, as we have no way to validate the number of times this nonce has been used + validate_nonce(controller.request, credentials[:nonce]) + raise Error.new(false, realm, credentials[:realm]), "Realm doesn't match" unless realm == credentials[:realm] + raise Error.new(true, opaque(controller.request), credentials[:opaque]),"Opaque doesn't match" unless opaque(controller.request) == credentials[:opaque] + + password = password_procedure.call(credentials[:username]) + raise Error.new(false), "No password" if password.nil? + expected = expected_response(controller.request.env['REQUEST_METHOD'], controller.request.url, credentials, password) + raise Error.new(false, expected, credentials[:response]), "Invalid response" unless expected == credentials[:response] + end + + # Returns the expected response for a request of +http_method+ to +uri+ with the decoded +credentials+ and the expected +password+ + def expected_response(http_method, uri, credentials, password) + ha1 = ::Digest::MD5.hexdigest([credentials[:username], credentials[:realm], password].join(':')) + ha2 = ::Digest::MD5.hexdigest([http_method.to_s.upcase,uri].join(':')) + ::Digest::MD5.hexdigest([ha1,credentials[:nonce], credentials[:nc], credentials[:cnonce],credentials[:qop],ha2].join(':')) + end + + def encode_credentials(http_method, credentials, password) + credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password) + "Digest " + credentials.sort_by {|x| x[0].to_s }.inject([]) {|a, v| a << "#{v[0]}='#{v[1]}'" }.join(', ') + end + + def decode_credentials(request) + authorization(request).to_s.gsub(/^Digest\s+/,'').split(',').inject({}) do |hash, pair| + key, value = pair.split('=', 2) + hash[key.strip.to_sym] = value.to_s.gsub(/^"|"$/,'').gsub(/'/, '') + hash + end + end + + def authentication_header(controller, realm) + controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce(controller.request)}", opaque="#{opaque(controller.request)}") + end + + def authentication_request(controller, realm, message = "HTTP Digest: Access denied") + authentication_header(controller, realm) + controller.send! :render, :text => message, :status => :unauthorized + end + + # Uses an MD5 digest based on time to generate a value to be used only once. + # + # A server-specified data string which should be uniquely generated each time a 401 response is made. + # It is recommended that this string be base64 or hexadecimal data. + # Specifically, since the string is passed in the header lines as a quoted string, the double-quote character is not allowed. + # + # The contents of the nonce are implementation dependent. + # The quality of the implementation depends on a good choice. + # A nonce might, for example, be constructed as the base 64 encoding of + # + # => time-stamp H(time-stamp ":" ETag ":" private-key) + # + # where time-stamp is a server-generated time or other non-repeating value, + # ETag is the value of the HTTP ETag header associated with the requested entity, + # and private-key is data known only to the server. + # With a nonce of this form a server would recalculate the hash portion after receiving the client authentication header and + # reject the request if it did not match the nonce from that header or + # if the time-stamp value is not recent enough. In this way the server can limit the time of the nonce's validity. + # The inclusion of the ETag prevents a replay request for an updated version of the resource. + # (Note: including the IP address of the client in the nonce would appear to offer the server the ability + # to limit the reuse of the nonce to the same client that originally got it. + # However, that would break proxy farms, where requests from a single user often go through different proxies in the farm. + # Also, IP address spoofing is not that hard.) + # + # An implementation might choose not to accept a previously used nonce or a previously used digest, in order to + # protect against a replay attack. Or, an implementation might choose to use one-time nonces or digests for + # POST or PUT requests and a time-stamp for GET requests. For more details on the issues involved see Section 4 + # of this document. + # + # The nonce is opaque to the client. + def nonce(request, time = Time.now) + session_id = request.is_a?(String) ? request : request.session.session_id + t = time.to_i + hashed = [t, session_id] + digest = ::Digest::MD5.hexdigest(hashed.join(":")) + Base64.encode64("#{t}:#{digest}").gsub("\n", '') + end + + def validate_nonce(request, value) + t = Base64.decode64(value).split(":").first.to_i + raise Error.new(true), "Stale Nonce" if (t - Time.now.to_i).abs > 10 * 60 + n = nonce(request, t) + raise Error.new(true, value, n), "Bad Nonce" unless n == value + end + + # Opaque based on digest of session_id + def opaque(request) + session_id = request.is_a?(String) ? request : request.session.session_id + @opaque ||= Base64.encode64(::Digest::MD5::hexdigest(session_id)).gsub("\n", '') + end + end + + class Error < RuntimeError + attr_accessor :expected, :was + def initialize(fatal = false, expected = nil, was = nil) + @fatal = fatal + @expected = expected + @was = was + end + + def fatal?; @fatal; end + end end end diff --git a/actionpack/lib/action_controller/integration.rb b/actionpack/lib/action_controller/integration.rb index 71e2524e81..a8e54c2fc7 100644 --- a/actionpack/lib/action_controller/integration.rb +++ b/actionpack/lib/action_controller/integration.rb @@ -68,6 +68,15 @@ module ActionController # A running counter of the number of requests processed. attr_accessor :request_count + # Nonce value for Digest Authentication, implicitly set on response with WWW-Authentication + attr_accessor :nonce + + # Opaque value for Digest Authentication, implicitly set on response with WWW-Authentication + attr_accessor :opaque + + # Opaque value for Authentication, implicitly set on response with WWW-Authentication + attr_accessor :realm + class MultiPartNeededException < Exception end @@ -243,6 +252,53 @@ module ActionController end alias xhr :xml_http_request + def request_with_noauth(http_method, uri, parameters, headers) + process_with_auth http_method, uri, parameters, headers + end + + # Performs a request with the given http_method and parameters, including HTTP Basic authorization headers. + # See get() for more details on paramters and headers. + # + # You can perform GET, POST, PUT, DELETE, and HEAD requests with #get_with_basic, #post_with_basic, + # #put_with_basic, #delete_with_basic, and #head_with_basic. + def request_with_basic(http_method, uri, parameters, headers, user_name, password) + process_with_auth http_method, uri, parameters, headers.merge(:authorization => ActionController::HttpAuthentication::Basic.encode_credentials(user_name, password)) + end + + # Performs a request with the given http_method and parameters, including HTTP Digest authorization headers. + # See get() for more details on paramters and headers. + # + # You can perform GET, POST, PUT, DELETE, and HEAD requests with #get_with_digest, #post_with_digest, + # #put_with_digest, #delete_with_digest, and #head_with_digest. + def request_with_digest(http_method, uri, parameters, headers, user_name, password) + # Realm, Nonce, and Opaque taken from previoius 401 response + + credentials = { + :username => user_name, + :realm => @realm, + :nonce => @nonce, + :qop => "auth", + :nc => "00000001", + :cnonce => "0a4f113b", + :opaque => @opaque, + :uri => uri + } + + raise "Digest request without previous 401 response" if @opaque.nil? + + process_with_auth http_method, uri, parameters, headers.merge(:authorization => ActionController::HttpAuthentication::Digest.encode_credentials(http_method, credentials, password)) + end + + # def get_with_basic, def post_with_basic, def put_with_basic, def delete_with_basic, def head_with_basic + # def get_with_digest, def post_with_digest, def put_with_digest, def delete_with_digest, def head_with_digest + [:get, :post, :put, :delete, :head].each do |method| + [:noauth, :basic, :digest].each do |auth_type| + define_method("#{method}_with_#{auth_type}") do |uri, parameters, headers, *auth| + send("request_with_#{auth_type}", method, uri, parameters, headers, *auth) + end + end + end + # Returns the URL for the given options, according to the rules specified # in the application's routes. def url_for(options) @@ -364,6 +420,32 @@ module ActionController return status end + # Same as process, but handles authentication returns to perform + # Basic or Digest authentication + def process_with_auth(method, path, parameters = nil, headers = nil) + status = process(method, path, parameters, headers) + + if status == 401 + # Extract authentication information from response + auth_data = @response.headers['WWW-Authenticate'] + if /^Basic /.match(auth_data) + # extract realm, to be used in subsequent request + @realm = auth_header.split(' ')[1] + elsif /^Digest/.match(auth_data) + creds = auth_data.to_s.gsub(/^Digest\s+/,'').split(',').inject({}) do |hash, pair| + key, value = pair.split('=', 2) + hash[key.strip.to_sym] = value.to_s.gsub(/^"|"$/,'').gsub(/'/, '') + hash + end + @realm = creds[:realm] + @nonce = creds[:nonce] + @opaque = creds[:opaque] + end + end + + return status + end + # Encode the cookies hash in a format suitable for passing to a # request. def encode_cookies diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb new file mode 100644 index 0000000000..d5c8636a9e --- /dev/null +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -0,0 +1,73 @@ +require 'abstract_unit' + +class HttpDigestAuthenticationTest < Test::Unit::TestCase + include ActionController::HttpAuthentication::Digest + + class DummyController + attr_accessor :headers, :renders, :request, :response + + def initialize + @headers, @renders = {}, [] + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + request.session.session_id = "test_session" + end + + def render(options) + self.renderers << options + end + end + + def setup + @controller = DummyController.new + @credentials = { + :username => "dhh", + :realm => "testrealm@host.com", + :nonce => ActionController::HttpAuthentication::Digest.nonce(@controller.request), + :qop => "auth", + :nc => "00000001", + :cnonce => "0a4f113b", + :opaque => ActionController::HttpAuthentication::Digest.opaque(@controller.request), + :uri => "http://test.host/" + } + @encoded_credentials = ActionController::HttpAuthentication::Digest.encode_credentials("GET", @credentials, "secret") + end + + def test_decode_credentials + set_headers + assert_equal @credentials, decode_credentials(@controller.request) + end + + def test_nonce_format + assert_nothing_thrown do + validate_nonce(@controller.request, nonce(@controller.request)) + end + end + + def test_authenticate_should_raise_for_nil_password + set_headers ActionController::HttpAuthentication::Digest.encode_credentials(:get, @credentials, nil) + assert_raise ActionController::HttpAuthentication::Error do + authenticate(@controller, @credentials[:realm]) { |user| user == "dhh" && "secret" } + end + end + + def test_authenticate_should_raise_for_incorrect_password + set_headers + assert_raise ActionController::HttpAuthentication::Error do + authenticate(@controller, @credentials[:realm]) { |user| user == "dhh" && "bad password" } + end + end + + def test_authenticate_should_not_raise_for_correct_password + set_headers + assert_nothing_thrown do + authenticate(@controller, @credentials[:realm]) { |user| user == "dhh" && "secret" } + end + end + + private + def set_headers(value = @encoded_credentials, name = 'HTTP_AUTHORIZATION', method = "GET") + @controller.request.env[name] = value + @controller.request.env["REQUEST_METHOD"] = method + end +end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index c28050fe0d..53cebf768e 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -8,7 +8,25 @@ class SessionTest < Test::Unit::TestCase } def setup + @credentials = { + :username => "username", + :realm => "MyApp", + :nonce => ActionController::HttpAuthentication::Digest.nonce("session_id"), + :qop => "auth", + :nc => "00000001", + :cnonce => "0a4f113b", + :opaque => ActionController::HttpAuthentication::Digest.opaque("session_id"), + :uri => "/index" + } + @session = ActionController::Integration::Session.new(StubApp) + @session.nonce = @credentials[:nonce] + @session.opaque = @credentials[:opaque] + @session.realm = @credentials[:realm] + end + + def encoded_credentials(method) + ActionController::HttpAuthentication::Digest.encode_credentials(method, @credentials, "password") end def test_https_bang_works_and_sets_truth_by_default @@ -132,6 +150,76 @@ class SessionTest < Test::Unit::TestCase @session.head(path,params,headers) end + def test_get_with_basic + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => "Basic dXNlcm5hbWU6cGFzc3dvcmQ=\n") + @session.expects(:process).with(:get,path,params,expected_headers) + @session.get_with_basic(path,params,headers,'username','password') + end + + def test_post_with_basic + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => "Basic dXNlcm5hbWU6cGFzc3dvcmQ=\n") + @session.expects(:process).with(:post,path,params,expected_headers) + @session.post_with_basic(path,params,headers,'username','password') + end + + def test_put_with_basic + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => "Basic dXNlcm5hbWU6cGFzc3dvcmQ=\n") + @session.expects(:process).with(:put,path,params,expected_headers) + @session.put_with_basic(path,params,headers,'username','password') + end + + def test_delete_with_basic + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => "Basic dXNlcm5hbWU6cGFzc3dvcmQ=\n") + @session.expects(:process).with(:delete,path,params,expected_headers) + @session.delete_with_basic(path,params,headers,'username','password') + end + + def test_head_with_basic + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => "Basic dXNlcm5hbWU6cGFzc3dvcmQ=\n") + @session.expects(:process).with(:head,path,params,expected_headers) + @session.head_with_basic(path,params,headers,'username','password') + end + + def test_get_with_digest + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => encoded_credentials(:get)) + @session.expects(:process).with(:get,path,params,expected_headers) + @session.get_with_digest(path,params,headers,'username','password') + end + + def test_post_with_digest + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => encoded_credentials(:post)) + @session.expects(:process).with(:post,path,params,expected_headers) + @session.post_with_digest(path,params,headers,'username','password') + end + + def test_put_with_digest + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => encoded_credentials(:put)) + @session.expects(:process).with(:put,path,params,expected_headers) + @session.put_with_digest(path,params,headers,'username','password') + end + + def test_delete_with_digest + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => encoded_credentials(:delete)) + @session.expects(:process).with(:delete,path,params,expected_headers) + @session.delete_with_digest(path,params,headers,'username','password') + end + + def test_head_with_digest + path = "/index"; params = "blah"; headers = {:location => 'blah'} + expected_headers = headers.merge(:authorization => encoded_credentials(:head)) + @session.expects(:process).with(:head,path,params,expected_headers) + @session.head_with_digest(path,params,headers,'username','password') + end + def test_xml_http_request_get path = "/index"; params = "blah"; headers = {:location => 'blah'} headers_after_xhr = headers.merge( -- cgit v1.2.3 From 5d89605c11cc54acadfdd76ccd226d38989ec600 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:31:03 -0600 Subject: Make router and controller classes better rack citizens --- actionpack/lib/action_controller/base.rb | 7 ++ actionpack/lib/action_controller/dispatcher.rb | 11 +- actionpack/lib/action_controller/request.rb | 4 +- actionpack/lib/action_controller/rescue.rb | 4 +- .../lib/action_controller/routing/route_set.rb | 6 + actionpack/test/controller/dispatcher_test.rb | 4 +- actionpack/test/controller/rescue_test.rb | 6 +- actionpack/test/controller/routing_test.rb | 133 ++++++++++----------- 8 files changed, 88 insertions(+), 87 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 5b83494eb4..da3d1f46ee 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -382,6 +382,13 @@ module ActionController #:nodoc: attr_accessor :action_name class << self + def call(env) + # HACK: For global rescue to have access to the original request and response + request = env["actioncontroller.rescue.request"] ||= Request.new(env) + response = env["actioncontroller.rescue.response"] ||= Response.new + process(request, response) + end + # Factory for the standard create, process loop where the controller is discarded after processing. def process(request, response) #:nodoc: new.process(request, response) diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 4dc76e1b49..c4e7357b81 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -60,11 +60,10 @@ module ActionController def dispatch begin run_callbacks :before_dispatch - controller = Routing::Routes.recognize(@request) - controller.process(@request, @response).to_a + Routing::Routes.call(@env) rescue Exception => exception if controller ||= (::ApplicationController rescue Base) - controller.process_with_exception(@request, @response, exception).to_a + controller.call_with_exception(@env, exception).to_a else raise exception end @@ -83,8 +82,7 @@ module ActionController end def _call(env) - @request = Request.new(env) - @response = Response.new + @env = env dispatch end @@ -110,8 +108,7 @@ module ActionController def checkin_connections # Don't return connection (and peform implicit rollback) if this request is a part of integration test - # TODO: This callback should have direct access to env - return if @request.key?("rack.test") + return if @env.key?("rack.test") ActiveRecord::Base.clear_active_connections! end end diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 3390324162..ba27c0d294 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -398,7 +398,7 @@ EOM end def path_parameters=(parameters) #:nodoc: - @path_parameters = parameters + @env["routing_args"] = parameters @symbolized_path_parameters = @parameters = nil end @@ -414,7 +414,7 @@ EOM # # See symbolized_path_parameters for symbolized keys. def path_parameters - @path_parameters ||= {} + @env["routing_args"] ||= {} end def body diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 5ef79a36ce..3a5e5071bb 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -59,7 +59,9 @@ module ActionController #:nodoc: end module ClassMethods - def process_with_exception(request, response, exception) #:nodoc: + def call_with_exception(env, exception) #:nodoc: + request = env["actioncontroller.rescue.request"] + response = env["actioncontroller.rescue.response"] new.process(request, response, :rescue_action, exception) end end diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb index 5975977365..06aef6e169 100644 --- a/actionpack/lib/action_controller/routing/route_set.rb +++ b/actionpack/lib/action_controller/routing/route_set.rb @@ -427,6 +427,12 @@ module ActionController end end + def call(env) + request = Request.new(env) + app = Routing::Routes.recognize(request) + app.call(env).to_a + end + def recognize(request) params = recognize_path(request.path, extract_request_environment(request)) request.path_parameters = params.with_indifferent_access diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index fd06b4ea99..da87d26146 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -96,9 +96,7 @@ class DispatcherTest < Test::Unit::TestCase private def dispatch(cache_classes = true) - controller = mock() - controller.stubs(:process).returns([200, {}, 'response']) - ActionController::Routing::Routes.stubs(:recognize).returns(controller) + ActionController::Routing::RouteSet.any_instance.stubs(:call).returns([200, {}, 'response']) Dispatcher.define_dispatcher_callbacks(cache_classes) @dispatcher.call({}) end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 63f9827f4a..49aca3a6ee 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -367,7 +367,11 @@ class RescueControllerTest < ActionController::TestCase end def test_rescue_dispatcher_exceptions - RescueController.process_with_exception(@request, @response, ActionController::RoutingError.new("Route not found")) + env = @request.env + env["actioncontroller.rescue.request"] = @request + env["actioncontroller.rescue.response"] = @response + + RescueController.call_with_exception(env, ActionController::RoutingError.new("Route not found")) assert_equal "no way", @response.body end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index d5b6bd6b2a..b981119e1e 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -706,7 +706,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do port_string = port == 80 ? '' : ":#{port}" protocol = options.delete(:protocol) || "http" - host = options.delete(:host) || "named.route.test" + host = options.delete(:host) || "test.host" anchor = "##{options.delete(:anchor)}" if options.key?(:anchor) path = routes.generate(options) @@ -715,27 +715,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end def request - @request ||= MockRequest.new(:host => "named.route.test", :method => :get) - end - end - - class MockRequest - attr_accessor :path, :path_parameters, :host, :subdomains, :domain, :method - - def initialize(values={}) - values.each { |key, value| send("#{key}=", value) } - if values[:host] - subdomain, self.domain = values[:host].split(/\./, 2) - self.subdomains = [subdomain] - end - end - - def protocol - "http://" - end - - def host_with_port - (subdomains * '.') + '.' + domain + @request ||= ActionController::TestRequest.new end end @@ -900,7 +880,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_basic_named_route rs.add_named_route :home, '', :controller => 'content', :action => 'list' x = setup_for_named_route - assert_equal("http://named.route.test/", + assert_equal("http://test.host/", x.send(:home_url)) end @@ -908,7 +888,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do rs.add_named_route :home, '', :controller => 'content', :action => 'list' x = setup_for_named_route ActionController::Base.relative_url_root = "/foo" - assert_equal("http://named.route.test/foo/", + assert_equal("http://test.host/foo/", x.send(:home_url)) assert_equal "/foo/", x.send(:home_path) ActionController::Base.relative_url_root = nil @@ -917,14 +897,14 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_with_option rs.add_named_route :page, 'page/:title', :controller => 'content', :action => 'show_page' x = setup_for_named_route - assert_equal("http://named.route.test/page/new%20stuff", + assert_equal("http://test.host/page/new%20stuff", x.send(:page_url, :title => 'new stuff')) end def test_named_route_with_default rs.add_named_route :page, 'page/:title', :controller => 'content', :action => 'show_page', :title => 'AboutPage' x = setup_for_named_route - assert_equal("http://named.route.test/page/AboutRails", + assert_equal("http://test.host/page/AboutRails", x.send(:page_url, :title => "AboutRails")) end @@ -932,21 +912,21 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_with_name_prefix rs.add_named_route :page, 'page', :controller => 'content', :action => 'show_page', :name_prefix => 'my_' x = setup_for_named_route - assert_equal("http://named.route.test/page", + assert_equal("http://test.host/page", x.send(:my_page_url)) end def test_named_route_with_path_prefix rs.add_named_route :page, 'page', :controller => 'content', :action => 'show_page', :path_prefix => 'my' x = setup_for_named_route - assert_equal("http://named.route.test/my/page", + assert_equal("http://test.host/my/page", x.send(:page_url)) end def test_named_route_with_nested_controller rs.add_named_route :users, 'admin/user', :controller => 'admin/user', :action => 'index' x = setup_for_named_route - assert_equal("http://named.route.test/admin/user", + assert_equal("http://test.host/admin/user", x.send(:users_url)) end @@ -985,7 +965,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do map.root :controller => "hello" end x = setup_for_named_route - assert_equal("http://named.route.test/", x.send(:root_url)) + assert_equal("http://test.host/", x.send(:root_url)) assert_equal("/", x.send(:root_path)) end @@ -1001,7 +981,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do # x.send(:article_url, :title => 'hi') # ) assert_equal( - "http://named.route.test/page/2005/6/10/hi", + "http://test.host/page/2005/6/10/hi", x.send(:article_url, :title => 'hi', :day => 10, :year => 2005, :month => 6) ) end @@ -1202,7 +1182,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do assert_equal '/test', rs.generate(:controller => 'post', :action => 'show', :year => nil) x = setup_for_named_route - assert_equal("http://named.route.test/test", + assert_equal("http://test.host/test", x.send(:blog_url)) end @@ -1249,7 +1229,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do assert_equal '/', rs.generate(:controller => 'content') x = setup_for_named_route - assert_equal("http://named.route.test/", + assert_equal("http://test.host/", x.send(:home_url)) end @@ -1591,7 +1571,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end def request - @request ||= MockRequest.new(:host => "named.routes.test", :method => :get) + @request ||= ActionController::TestRequest.new end def test_generate_extras @@ -1692,13 +1672,13 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_url_method controller = setup_named_route_test - assert_equal "http://named.route.test/people/5", controller.send(:show_url, :id => 5) + assert_equal "http://test.host/people/5", controller.send(:show_url, :id => 5) assert_equal "/people/5", controller.send(:show_path, :id => 5) - assert_equal "http://named.route.test/people", controller.send(:index_url) + assert_equal "http://test.host/people", controller.send(:index_url) assert_equal "/people", controller.send(:index_path) - assert_equal "http://named.route.test/admin/users", controller.send(:users_url) + assert_equal "http://test.host/admin/users", controller.send(:users_url) assert_equal '/admin/users', controller.send(:users_path) assert_equal '/admin/users', set.generate(controller.send(:hash_for_users_url), {:controller => 'users', :action => 'index'}) end @@ -1706,28 +1686,28 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_url_method_with_anchor controller = setup_named_route_test - assert_equal "http://named.route.test/people/5#location", controller.send(:show_url, :id => 5, :anchor => 'location') + assert_equal "http://test.host/people/5#location", controller.send(:show_url, :id => 5, :anchor => 'location') assert_equal "/people/5#location", controller.send(:show_path, :id => 5, :anchor => 'location') - assert_equal "http://named.route.test/people#location", controller.send(:index_url, :anchor => 'location') + assert_equal "http://test.host/people#location", controller.send(:index_url, :anchor => 'location') assert_equal "/people#location", controller.send(:index_path, :anchor => 'location') - assert_equal "http://named.route.test/admin/users#location", controller.send(:users_url, :anchor => 'location') + assert_equal "http://test.host/admin/users#location", controller.send(:users_url, :anchor => 'location') assert_equal '/admin/users#location', controller.send(:users_path, :anchor => 'location') - assert_equal "http://named.route.test/people/go/7/hello/joe/5#location", + assert_equal "http://test.host/people/go/7/hello/joe/5#location", controller.send(:multi_url, 7, "hello", 5, :anchor => 'location') - assert_equal "http://named.route.test/people/go/7/hello/joe/5?baz=bar#location", + assert_equal "http://test.host/people/go/7/hello/joe/5?baz=bar#location", controller.send(:multi_url, 7, "hello", 5, :baz => "bar", :anchor => 'location') - assert_equal "http://named.route.test/people?baz=bar#location", + assert_equal "http://test.host/people?baz=bar#location", controller.send(:index_url, :baz => "bar", :anchor => 'location') end def test_named_route_url_method_with_port controller = setup_named_route_test - assert_equal "http://named.route.test:8080/people/5", controller.send(:show_url, 5, :port=>8080) + assert_equal "http://test.host:8080/people/5", controller.send(:show_url, 5, :port=>8080) end def test_named_route_url_method_with_host @@ -1737,30 +1717,30 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_url_method_with_protocol controller = setup_named_route_test - assert_equal "https://named.route.test/people/5", controller.send(:show_url, 5, :protocol => "https") + assert_equal "https://test.host/people/5", controller.send(:show_url, 5, :protocol => "https") end def test_named_route_url_method_with_ordered_parameters controller = setup_named_route_test - assert_equal "http://named.route.test/people/go/7/hello/joe/5", + assert_equal "http://test.host/people/go/7/hello/joe/5", controller.send(:multi_url, 7, "hello", 5) end def test_named_route_url_method_with_ordered_parameters_and_hash controller = setup_named_route_test - assert_equal "http://named.route.test/people/go/7/hello/joe/5?baz=bar", + assert_equal "http://test.host/people/go/7/hello/joe/5?baz=bar", controller.send(:multi_url, 7, "hello", 5, :baz => "bar") end def test_named_route_url_method_with_ordered_parameters_and_empty_hash controller = setup_named_route_test - assert_equal "http://named.route.test/people/go/7/hello/joe/5", + assert_equal "http://test.host/people/go/7/hello/joe/5", controller.send(:multi_url, 7, "hello", 5, {}) end def test_named_route_url_method_with_no_positional_arguments controller = setup_named_route_test - assert_equal "http://named.route.test/people?baz=bar", + assert_equal "http://test.host/people?baz=bar", controller.send(:index_url, :baz => "bar") end @@ -1896,49 +1876,54 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/people" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("index", request.path_parameters[:action]) + request.recycle! - request.method = :post + request.env["REQUEST_METHOD"] = "POST" assert_nothing_raised { set.recognize(request) } assert_equal("create", request.path_parameters[:action]) + request.recycle! - request.method = :put + request.env["REQUEST_METHOD"] = "PUT" assert_nothing_raised { set.recognize(request) } assert_equal("update", request.path_parameters[:action]) + request.recycle! - begin - request.method = :bacon + assert_raises(ActionController::UnknownHttpMethod) { + request.env["REQUEST_METHOD"] = "BACON" set.recognize(request) - flunk 'Should have raised NotImplemented' - rescue ActionController::NotImplemented => e - assert_equal [:get, :post, :put, :delete], e.allowed_methods - end + } + request.recycle! request.path = "/people/5" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("show", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! - request.method = :put + request.env["REQUEST_METHOD"] = "PUT" assert_nothing_raised { set.recognize(request) } assert_equal("update", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! - request.method = :delete + request.env["REQUEST_METHOD"] = "DELETE" assert_nothing_raised { set.recognize(request) } assert_equal("destroy", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! begin - request.method = :post + request.env["REQUEST_METHOD"] = "POST" set.recognize(request) flunk 'Should have raised MethodNotAllowed' rescue ActionController::MethodNotAllowed => e assert_equal [:get, :put, :delete], e.allowed_methods end + request.recycle! ensure Object.send(:remove_const, :PeopleController) @@ -1954,13 +1939,13 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/people" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("people", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) request.path = "/" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("people", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) @@ -1978,7 +1963,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/articles/2005/11/05/a-very-interesting-article" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("permalink", request.path_parameters[:action]) assert_equal("2005", request.path_parameters[:year]) @@ -2015,17 +2000,19 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/people/5" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("show", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! - request.method = :put + request.env["REQUEST_METHOD"] = "PUT" assert_nothing_raised { set.recognize(request) } assert_equal("update", request.path_parameters[:action]) + request.recycle! request.path = "/people/5.png" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("show", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) @@ -2050,7 +2037,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do set.draw { |map| map.root :controller => "people" } request.path = "" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("people", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) @@ -2070,7 +2057,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/api/inventory" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("api/products", request.path_parameters[:controller]) assert_equal("inventory", request.path_parameters[:action]) @@ -2090,7 +2077,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/api" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("api/products", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) @@ -2110,7 +2097,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/prefix/inventory" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("api/products", request.path_parameters[:controller]) assert_equal("inventory", request.path_parameters[:action]) @@ -2246,7 +2233,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/projects/1/milestones" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("milestones", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) -- cgit v1.2.3 From c20c72e3d9321f8c00587aab479d962e80b02c35 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:34:59 -0600 Subject: Use rack namespace for routing args --- actionpack/lib/action_controller/request.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index ba27c0d294..822955d1db 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -398,7 +398,7 @@ EOM end def path_parameters=(parameters) #:nodoc: - @env["routing_args"] = parameters + @env["rack.routing_args"] = parameters @symbolized_path_parameters = @parameters = nil end @@ -414,7 +414,7 @@ EOM # # See symbolized_path_parameters for symbolized keys. def path_parameters - @env["routing_args"] ||= {} + @env["rack.routing_args"] ||= {} end def body -- cgit v1.2.3 From 36af857c435cbbdb43f5a7bed200ddaa5e10ef80 Mon Sep 17 00:00:00 2001 From: Mike Gunderloy Date: Sun, 28 Dec 2008 20:31:33 -0600 Subject: Fix FCGI dispatching tests Signed-off-by: Pratik Naik --- railties/test/fcgi_dispatcher_test.rb | 49 +++++------------------------------ 1 file changed, 7 insertions(+), 42 deletions(-) diff --git a/railties/test/fcgi_dispatcher_test.rb b/railties/test/fcgi_dispatcher_test.rb index cc054c24aa..c469c5dd01 100644 --- a/railties/test/fcgi_dispatcher_test.rb +++ b/railties/test/fcgi_dispatcher_test.rb @@ -1,10 +1,9 @@ require 'abstract_unit' begin +require 'action_controller' require 'fcgi_handler' -module ActionController; module Routing; module Routes; end end end - class RailsFCGIHandlerTest < Test::Unit::TestCase def setup @log = StringIO.new @@ -131,19 +130,11 @@ class RailsFCGIHandlerSignalsTest < Test::Unit::TestCase end end - class ::Dispatcher - class << self - attr_accessor :signal - alias_method :old_dispatch, :dispatch - def dispatch(cgi) - signal ? Process.kill(signal, $$) : old_dispatch - end - end - end - def setup @log = StringIO.new @handler = RailsFCGIHandler.new(@log) + @dispatcher = mock + Dispatcher.stubs(:new).returns(@dispatcher) end def test_interrupted_via_HUP_when_not_in_request @@ -159,19 +150,6 @@ class RailsFCGIHandlerSignalsTest < Test::Unit::TestCase assert_equal :reload, @handler.when_ready end - def test_interrupted_via_HUP_when_in_request - cgi = mock - FCGI.expects(:each_cgi).once.yields(cgi) - Dispatcher.expects(:signal).times(2).returns('HUP') - - @handler.expects(:reload!).once - @handler.expects(:close_connection).never - @handler.expects(:exit).never - - @handler.process! - assert_equal :reload, @handler.when_ready - end - def test_interrupted_via_USR1_when_not_in_request cgi = mock FCGI.expects(:each_cgi).once.yields(cgi) @@ -186,19 +164,6 @@ class RailsFCGIHandlerSignalsTest < Test::Unit::TestCase assert_nil @handler.when_ready end - def test_interrupted_via_USR1_when_in_request - cgi = mock - FCGI.expects(:each_cgi).once.yields(cgi) - Dispatcher.expects(:signal).times(2).returns('USR1') - - @handler.expects(:reload!).never - @handler.expects(:close_connection).with(cgi).once - @handler.expects(:exit).never - - @handler.process! - assert_equal :exit, @handler.when_ready - end - def test_restart_via_USR2_when_in_request cgi = mock FCGI.expects(:each_cgi).once.yields(cgi) @@ -217,7 +182,7 @@ class RailsFCGIHandlerSignalsTest < Test::Unit::TestCase def test_interrupted_via_TERM cgi = mock FCGI.expects(:each_cgi).once.yields(cgi) - Dispatcher.expects(:signal).times(2).returns('TERM') + ::Rack::Handler::FastCGI.expects(:serve).once.returns('TERM') @handler.expects(:reload!).never @handler.expects(:close_connection).never @@ -238,7 +203,7 @@ class RailsFCGIHandlerSignalsTest < Test::Unit::TestCase cgi = mock error = RuntimeError.new('foo') FCGI.expects(:each_cgi).once.yields(cgi) - Dispatcher.expects(:dispatch).once.with(cgi).raises(error) + ::Rack::Handler::FastCGI.expects(:serve).once.raises(error) @handler.expects(:dispatcher_error).with(error, regexp_matches(/^unhandled/)) @handler.process! end @@ -254,7 +219,7 @@ class RailsFCGIHandlerSignalsTest < Test::Unit::TestCase cgi = mock error = SignalException.new('USR2') FCGI.expects(:each_cgi).once.yields(cgi) - Dispatcher.expects(:dispatch).once.with(cgi).raises(error) + ::Rack::Handler::FastCGI.expects(:serve).once.raises(error) @handler.expects(:dispatcher_error).with(error, regexp_matches(/^stopping/)) @handler.process! end @@ -284,7 +249,7 @@ class RailsFCGIHandlerPeriodicGCTest < Test::Unit::TestCase cgi = mock FCGI.expects(:each_cgi).times(10).yields(cgi) - Dispatcher.expects(:dispatch).times(10).with(cgi) + Dispatcher.expects(:new).times(10) @handler.expects(:run_gc!).never 9.times { @handler.process! } -- cgit v1.2.3 From 490c26c8433a6d278bc61118782da360e8889646 Mon Sep 17 00:00:00 2001 From: Mike Gunderloy Date: Mon, 29 Dec 2008 08:00:30 -0600 Subject: Fix failing gem dependency tests [#1659 state:resolved] Signed-off-by: Pratik Naik --- railties/test/gem_dependency_test.rb | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/railties/test/gem_dependency_test.rb b/railties/test/gem_dependency_test.rb index 30fd899fea..6c1f0961a1 100644 --- a/railties/test/gem_dependency_test.rb +++ b/railties/test/gem_dependency_test.rb @@ -9,33 +9,33 @@ Rails::VendorGemSourceIndex.silence_spec_warnings = true uses_mocha "Plugin Tests" do class GemDependencyTest < Test::Unit::TestCase def setup - @gem = Rails::GemDependency.new "hpricot" - @gem_with_source = Rails::GemDependency.new "hpricot", :source => "http://code.whytheluckystiff.net" - @gem_with_version = Rails::GemDependency.new "hpricot", :version => "= 0.6" - @gem_with_lib = Rails::GemDependency.new "aws-s3", :lib => "aws/s3" - @gem_without_load = Rails::GemDependency.new "hpricot", :lib => false + @gem = Rails::GemDependency.new "xhpricotx" + @gem_with_source = Rails::GemDependency.new "xhpricotx", :source => "http://code.whytheluckystiff.net" + @gem_with_version = Rails::GemDependency.new "xhpricotx", :version => "= 0.6" + @gem_with_lib = Rails::GemDependency.new "xaws-s3x", :lib => "aws/s3" + @gem_without_load = Rails::GemDependency.new "xhpricotx", :lib => false end def test_configuration_adds_gem_dependency config = Rails::Configuration.new - config.gem "aws-s3", :lib => "aws/s3", :version => "0.4.0" - assert_equal [["install", "aws-s3", "--version", '"= 0.4.0"']], config.gems.collect(&:install_command) + config.gem "xaws-s3x", :lib => "aws/s3", :version => "0.4.0" + assert_equal [["install", "xaws-s3x", "--version", '"= 0.4.0"']], config.gems.collect(&:install_command) end def test_gem_creates_install_command - assert_equal %w(install hpricot), @gem.install_command + assert_equal %w(install xhpricotx), @gem.install_command end def test_gem_with_source_creates_install_command - assert_equal %w(install hpricot --source http://code.whytheluckystiff.net), @gem_with_source.install_command + assert_equal %w(install xhpricotx --source http://code.whytheluckystiff.net), @gem_with_source.install_command end def test_gem_with_version_creates_install_command - assert_equal ["install", "hpricot", "--version", '"= 0.6"'], @gem_with_version.install_command + assert_equal ["install", "xhpricotx", "--version", '"= 0.6"'], @gem_with_version.install_command end def test_gem_creates_unpack_command - assert_equal %w(unpack hpricot), @gem.unpack_command + assert_equal %w(unpack xhpricotx), @gem.unpack_command end def test_gem_with_version_unpack_install_command @@ -43,7 +43,7 @@ uses_mocha "Plugin Tests" do mock_spec = mock() mock_spec.stubs(:version).returns('0.6') @gem_with_version.stubs(:specification).returns(mock_spec) - assert_equal ["unpack", "hpricot", "--version", '= 0.6'], @gem_with_version.unpack_command + assert_equal ["unpack", "xhpricotx", "--version", '= 0.6'], @gem_with_version.unpack_command end def test_gem_adds_load_paths -- cgit v1.2.3 From 558ab327b733717f4a8de3ed62b8dcd62e9ff9c3 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 29 Dec 2008 19:27:19 -0600 Subject: Clean up view path cruft and split path implementations into Template::Path and Template::EagerPath --- actionmailer/test/abstract_unit.rb | 1 - actionpack/lib/action_controller/base.rb | 12 +-- actionpack/lib/action_controller/rescue.rb | 4 +- actionpack/lib/action_view/base.rb | 2 +- actionpack/lib/action_view/inline_template.rb | 2 +- actionpack/lib/action_view/paths.rb | 107 +-------------------- actionpack/lib/action_view/renderable.rb | 9 +- actionpack/lib/action_view/template.rb | 85 +++++++++++++++- actionpack/test/abstract_unit.rb | 1 - .../test/template/compiled_templates_test.rb | 9 +- actionpack/test/template/render_test.rb | 7 +- railties/lib/initializer.rb | 7 +- 12 files changed, 108 insertions(+), 138 deletions(-) diff --git a/actionmailer/test/abstract_unit.rb b/actionmailer/test/abstract_unit.rb index 4900f6fb35..a6126d6f7f 100644 --- a/actionmailer/test/abstract_unit.rb +++ b/actionmailer/test/abstract_unit.rb @@ -17,7 +17,6 @@ $:.unshift "#{File.dirname(__FILE__)}/fixtures/helpers" FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') ActionMailer::Base.template_root = FIXTURE_LOAD_PATH -ActionMailer::Base.template_root.load class MockSMTP def self.deliveries diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index da3d1f46ee..bc18dfaec8 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -869,7 +869,7 @@ module ActionController #:nodoc: validate_render_arguments(options, extra_options, block_given?) if options.nil? - options = { :template => default_template.filename, :layout => true } + options = { :template => default_template, :layout => true } elsif options == :update options = extra_options.merge({ :update => true }) elsif options.is_a?(String) || options.is_a?(Symbol) @@ -1125,7 +1125,7 @@ module ActionController #:nodoc: end # Sets the etag, last_modified, or both on the response and renders a - # "304 Not Modified" response if the request is already fresh. + # "304 Not Modified" response if the request is already fresh. # # Example: # @@ -1133,8 +1133,8 @@ module ActionController #:nodoc: # @article = Article.find(params[:id]) # fresh_when(:etag => @article, :last_modified => @article.created_at.utc) # end - # - # This will render the show template if the request isn't sending a matching etag or + # + # This will render the show template if the request isn't sending a matching etag or # If-Modified-Since header and just a "304 Not Modified" response if there's a match. def fresh_when(options) options.assert_valid_keys(:etag, :last_modified) @@ -1239,7 +1239,7 @@ module ActionController #:nodoc: log_processing_for_parameters end end - + def log_processing_for_request_id request_id = "\n\nProcessing #{self.class.name}\##{action_name} " request_id << "to #{params[:format]} " if params[:format] @@ -1251,7 +1251,7 @@ module ActionController #:nodoc: def log_processing_for_parameters parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup parameters = parameters.except!(:controller, :action, :format, :_method) - + logger.info " Parameters: #{parameters.inspect}" unless parameters.empty? end diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 3a5e5071bb..de35b53872 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -38,8 +38,8 @@ module ActionController #:nodoc: 'ActionView::TemplateError' => 'template_error' } - RESCUES_TEMPLATE_PATH = ActionView::PathSet::Path.new( - File.join(File.dirname(__FILE__), "templates"), true) + RESCUES_TEMPLATE_PATH = ActionView::Template::EagerPath.new( + File.join(File.dirname(__FILE__), "templates")) def self.included(base) #:nodoc: base.cattr_accessor :rescue_responses diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 8958e61e9d..70a0ba91a7 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -225,7 +225,7 @@ module ActionView #:nodoc: end # Returns the result of a render that's dictated by the options hash. The primary options are: - # + # # * :partial - See ActionView::Partials. # * :update - Calls update_page with the block given. # * :file - Renders an explicit template file (this used to be the old default), add :locals to pass in those. diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 5e00cef13f..54efa543c8 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -12,7 +12,7 @@ module ActionView #:nodoc: private # Always recompile inline templates - def recompile?(local_assigns) + def recompile? true end end diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index b030156889..19207e7262 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -2,7 +2,7 @@ module ActionView #:nodoc: class PathSet < Array #:nodoc: def self.type_cast(obj) if obj.is_a?(String) - Path.new(obj) + Template::EagerPath.new(obj) else obj end @@ -32,111 +32,6 @@ module ActionView #:nodoc: super(*objs.map { |obj| self.class.type_cast(obj) }) end - class Path #:nodoc: - attr_reader :path, :paths - delegate :hash, :inspect, :to => :path - - def initialize(path, load = false) - raise ArgumentError, "path already is a Path class" if path.is_a?(Path) - @path = path.freeze - reload! if load - end - - def to_s - if defined?(RAILS_ROOT) - path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '') - else - path.to_s - end - end - - def to_str - path.to_str - end - - def ==(path) - to_str == path.to_str - end - - def eql?(path) - to_str == path.to_str - end - - # Returns a ActionView::Template object for the given path string. The - # input path should be relative to the view path directory, - # +hello/index.html.erb+. This method also has a special exception to - # match partial file names without a handler extension. So - # +hello/index.html+ will match the first template it finds with a - # known template extension, +hello/index.html.erb+. Template extensions - # should not be confused with format extensions +html+, +js+, +xml+, - # etc. A format must be supplied to match a formated file. +hello/index+ - # will never match +hello/index.html.erb+. - # - # This method also has two different implementations, one that is "lazy" - # and makes file system calls every time and the other is cached, - # "eager" which looks up the template in an in memory index. The "lazy" - # version is designed for development where you want to automatically - # find new templates between requests. The "eager" version is designed - # for production mode and it is much faster but requires more time - # upfront to build the file index. - def [](path) - if loaded? - @paths[path] - else - Dir.glob("#{@path}/#{path}*").each do |file| - template = create_template(file) - if template.accessible_paths.include?(path) - return template - end - end - nil - end - end - - def loaded? - @loaded ? true : false - end - - def load - reload! unless loaded? - self - end - - # Rebuild load path directory cache - def reload! - @paths = {} - - templates_in_path do |template| - template.load! - template.accessible_paths.each do |path| - @paths[path] = template - end - end - - @paths.freeze - @loaded = true - end - - private - def templates_in_path - (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file| - yield create_template(file) unless File.directory?(file) - end - end - - def create_template(file) - Template.new(file.split("#{self}/").last, self) - end - end - - def load - each { |path| path.load } - end - - def reload! - each { |path| path.reload! } - end - def find_template(original_template_path, format = nil) return original_template_path if original_template_path.respond_to?(:render) template_path = original_template_path.sub(/^\//, '') diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index d8e72f1179..153e14f68b 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -60,7 +60,7 @@ module ActionView def compile(local_assigns) render_symbol = method_name(local_assigns) - if recompile?(render_symbol) + if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile? compile!(render_symbol, local_assigns) end end @@ -89,11 +89,8 @@ module ActionView end end - # 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) - !Base::CompiledTemplates.method_defined?(symbol) || !loaded? + def recompile? + false end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 5b384d0e4d..88ee07d95b 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,5 +1,83 @@ module ActionView #:nodoc: class Template + class Path + attr_reader :path, :paths + delegate :hash, :inspect, :to => :path + + def initialize(path) + raise ArgumentError, "path already is a Path class" if path.is_a?(Path) + @path = path.freeze + end + + def to_s + if defined?(RAILS_ROOT) + path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '') + else + path.to_s + end + end + + def to_str + path.to_str + end + + def ==(path) + to_str == path.to_str + end + + def eql?(path) + to_str == path.to_str + end + + # Returns a ActionView::Template object for the given path string. The + # input path should be relative to the view path directory, + # +hello/index.html.erb+. This method also has a special exception to + # match partial file names without a handler extension. So + # +hello/index.html+ will match the first template it finds with a + # known template extension, +hello/index.html.erb+. Template extensions + # should not be confused with format extensions +html+, +js+, +xml+, + # etc. A format must be supplied to match a formated file. +hello/index+ + # will never match +hello/index.html.erb+. + def [](path) + templates_in_path do |template| + if template.accessible_paths.include?(path) + return template + end + end + nil + end + + private + def templates_in_path + (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file| + yield create_template(file) unless File.directory?(file) + end + end + + def create_template(file) + Template.new(file.split("#{self}/").last, self) + end + end + + class EagerPath < Path + def initialize(path) + super + + @paths = {} + templates_in_path do |template| + template.load! + template.accessible_paths.each do |path| + @paths[path] = template + end + end + @paths.freeze + end + + def [](path) + @paths[path] + end + end + extend TemplateHandlers extend ActiveSupport::Memoizable include Renderable @@ -115,13 +193,12 @@ module ActionView #:nodoc: File.mtime(filename) > mtime end - def loaded? - @loaded + def recompile? + !@cached end def load! - @loaded = true - compile({}) + @cached = true freeze end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 2ba056e60f..30e2d863d0 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -34,7 +34,6 @@ ActionController::Base.session_store = nil FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') ActionController::Base.view_paths = FIXTURE_LOAD_PATH -ActionController::Base.view_paths.load def uses_mocha(test_name) yield diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb index a68b09bb45..caea1bd643 100644 --- a/actionpack/test/template/compiled_templates_test.rb +++ b/actionpack/test/template/compiled_templates_test.rb @@ -31,7 +31,7 @@ uses_mocha 'TestTemplateRecompilation' do end def test_compiled_template_will_always_be_recompiled_when_template_is_not_cached - ActionView::Template.any_instance.expects(:loaded?).times(3).returns(false) + ActionView::Template.any_instance.expects(:recompile?).times(3).returns(true) assert_equal 0, @compiled_templates.instance_methods.size assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") ActionView::Template.any_instance.expects(:compile!).times(3) @@ -62,13 +62,14 @@ uses_mocha 'TestTemplateRecompilation' do def render_with_cache(*args) view_paths = ActionController::Base.view_paths - assert view_paths.first.loaded? + assert_equal ActionView::Template::EagerPath, view_paths.first.class ActionView::Base.new(view_paths, {}).render(*args) end def render_without_cache(*args) - view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH) - assert !view_paths.first.loaded? + path = ActionView::Template::Path.new(FIXTURE_LOAD_PATH) + view_paths = ActionView::Base.process_view_paths(path) + assert_equal ActionView::Template::Path, view_paths.first.class ActionView::Base.new(view_paths, {}).render(*args) end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 0387a11de2..4bd897efeb 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -197,7 +197,7 @@ class CachedViewRenderTest < Test::Unit::TestCase # Ensure view path cache is primed def setup view_paths = ActionController::Base.view_paths - assert view_paths.first.loaded? + assert_equal ActionView::Template::EagerPath, view_paths.first.class setup_view(view_paths) end end @@ -208,8 +208,9 @@ class LazyViewRenderTest < Test::Unit::TestCase # Test the same thing as above, but make sure the view path # is not eager loaded def setup - view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH) - assert !view_paths.first.loaded? + path = ActionView::Template::Path.new(FIXTURE_LOAD_PATH) + view_paths = ActionView::Base.process_view_paths(path) + assert_equal ActionView::Template::Path, view_paths.first.class setup_view(view_paths) end end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 637fe74313..10c2490624 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -370,8 +370,9 @@ Run `rake gems:install` to install the missing gems. def load_view_paths if configuration.frameworks.include?(:action_view) if configuration.cache_classes - ActionController::Base.view_paths.load if configuration.frameworks.include?(:action_controller) - ActionMailer::Base.template_root.load if configuration.frameworks.include?(:action_mailer) + view_path = ActionView::Template::EagerPath.new(configuration.view_path) + ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller) + ActionMailer::Base.template_root = view_path if configuration.frameworks.include?(:action_mailer) end end end @@ -473,7 +474,7 @@ Run `rake gems:install` to install the missing gems. # set to use Configuration#view_path. def initialize_framework_views if configuration.frameworks.include?(:action_view) - view_path = ActionView::PathSet::Path.new(configuration.view_path, false) + view_path = ActionView::Template::Path.new(configuration.view_path) ActionMailer::Base.template_root ||= view_path if configuration.frameworks.include?(:action_mailer) ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller) && ActionController::Base.view_paths.empty? end -- cgit v1.2.3 From 220dff4c3b58b7becb587ee6f2434b2ca720f7c3 Mon Sep 17 00:00:00 2001 From: Mike Gunderloy Date: Mon, 29 Dec 2008 20:46:20 -0600 Subject: Add transaction check to SQLite2 adapter to fix test_sqlite_add_column_in_transaction_raises_statement_invalid [#1669 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 84f8c0284e..9387cf8827 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -402,6 +402,10 @@ module ActiveRecord end def add_column(table_name, column_name, type, options = {}) #:nodoc: + if @connection.respond_to?(:transaction_active?) && @connection.transaction_active? + raise StatementInvalid, 'Cannot add columns to a SQLite database while inside a transaction' + end + alter_table(table_name) do |definition| definition.column(column_name, type, options) end -- cgit v1.2.3 From a29369ae4ac705fbbd4ac0c0325468e50e4eeca0 Mon Sep 17 00:00:00 2001 From: Mike Gunderloy Date: Mon, 29 Dec 2008 19:14:30 -0600 Subject: Fix named scope tests for sqlite3 [#1667 state:resolved] Signed-off-by: Pratik Naik --- activerecord/test/cases/named_scope_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index b152f95a15..bab842cf66 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -254,7 +254,7 @@ class NamedScopeTest < ActiveRecord::TestCase end def test_should_use_where_in_query_for_named_scope - assert_equal Developer.find_all_by_name('Jamis'), Developer.find_all_by_id(Developer.jamises) + assert_equal Developer.find_all_by_name('Jamis').to_set, Developer.find_all_by_id(Developer.jamises).to_set end def test_size_should_use_count_when_results_are_not_loaded -- cgit v1.2.3 From 2f9edde142a15452eb088b8b4b2a38272dde2ba5 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 30 Dec 2008 12:44:31 -0800 Subject: Clean trailing / after rails root from backtraces --- railties/lib/rails/backtrace_cleaner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/backtrace_cleaner.rb b/railties/lib/rails/backtrace_cleaner.rb index 94d34cda39..ee67255289 100644 --- a/railties/lib/rails/backtrace_cleaner.rb +++ b/railties/lib/rails/backtrace_cleaner.rb @@ -15,7 +15,7 @@ module Rails def initialize super - add_filter { |line| line.sub(RAILS_ROOT, '') } + add_filter { |line| line.sub("#{RAILS_ROOT}/", '') } add_filter { |line| line.sub(ERB_METHOD_SIG, '') } add_filter { |line| line.sub('./', '/') } # for tests add_filter { |line| line.sub(/(#{GEMS_DIR})\/gems\/([a-z]+)-([0-9.]+)\/(.*)/, '\2 (\3) \4')} # http://gist.github.com/30430 -- cgit v1.2.3 From c69d8c043f8d0a587708fb2247b585ca93df822d Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 30 Dec 2008 15:16:51 -0800 Subject: Fix formatted_* deprecation message --- actionpack/lib/action_controller/routing/route_set.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb index 06aef6e169..044ace7de1 100644 --- a/actionpack/lib/action_controller/routing/route_set.rb +++ b/actionpack/lib/action_controller/routing/route_set.rb @@ -195,8 +195,8 @@ module ActionController def formatted_#{selector}(*args) # def formatted_users_url(*args) ActiveSupport::Deprecation.warn( # ActiveSupport::Deprecation.warn( "formatted_#{selector}() has been deprecated. " + # "formatted_users_url() has been deprecated. " + - "please pass format to the standard" + # "please pass format to the standard" + - "#{selector}() method instead.", caller) # "users_url() method instead.", caller) + "Please pass format to the standard " + # "Please pass format to the standard " + + "#{selector} method instead.", caller) # "users_url method instead.", caller) #{selector}(*args) # users_url(*args) end # end protected :#{selector} # protected :users_url -- cgit v1.2.3 From 2e1132fad8fa2ab58476b9ecc30523ed02a43181 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 30 Dec 2008 18:06:56 -0800 Subject: Test that exceptions raised in filters are properly rescued --- actionpack/lib/action_controller/base.rb | 11 +++++++---- actionpack/test/controller/rescue_test.rb | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index bc18dfaec8..aa604395a9 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1350,9 +1350,12 @@ module ActionController #:nodoc: end Base.class_eval do - include Flash, Filters, Layout, Benchmarking, Rescue, MimeResponds, Helpers - include Cookies, Caching, Verification, Streaming - include SessionManagement, HttpAuthentication::Basic::ControllerMethods - include RecordIdentifier, RequestForgeryProtection, Translation + [ Flash, Filters, Layout, Benchmarking, Rescue, MimeResponds, Helpers, + Cookies, Caching, Verification, Streaming, SessionManagement, + HttpAuthentication::Basic::ControllerMethods, RecordIdentifier, + RequestForgeryProtection, Translation + ].each do |mod| + include mod + end end end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 49aca3a6ee..d45ba3c3a1 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -67,6 +67,11 @@ class RescueController < ActionController::Base render :text => 'no way' end + before_filter(:only => :before_filter_raises) { raise 'umm nice' } + + def before_filter_raises + end + def raises render :text => 'already rendered' raise "don't panic!" @@ -154,6 +159,16 @@ class RescueControllerTest < ActionController::TestCase end end + def test_rescue_exceptions_raised_by_filters + with_rails_root FIXTURE_PUBLIC do + with_all_requests_local false do + get :before_filter_raises + end + end + + assert_response :internal_server_error + end + def test_rescue_action_locally_if_all_requests_local @controller.expects(:local_request?).never @controller.expects(:rescue_action_locally).with(@exception) -- cgit v1.2.3 From a5004573d8d132fe079242fc082ab4661b0976e9 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 30 Dec 2008 18:25:44 -0800 Subject: Only silence backtrace from plugin lib dirs --- railties/lib/rails/backtrace_cleaner.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/backtrace_cleaner.rb b/railties/lib/rails/backtrace_cleaner.rb index ee67255289..e1b422716d 100644 --- a/railties/lib/rails/backtrace_cleaner.rb +++ b/railties/lib/rails/backtrace_cleaner.rb @@ -2,7 +2,7 @@ module Rails class BacktraceCleaner < ActiveSupport::BacktraceCleaner ERB_METHOD_SIG = /:in `_run_erb_.*/ - VENDOR_DIRS = %w( vendor/plugins vendor/gems vendor/rails ) + VENDOR_DIRS = %w( vendor/gems vendor/rails ) SERVER_DIRS = %w( lib/mongrel bin/mongrel lib/passenger bin/passenger-spawn-server lib/rack ) @@ -20,6 +20,7 @@ module Rails add_filter { |line| line.sub('./', '/') } # for tests add_filter { |line| line.sub(/(#{GEMS_DIR})\/gems\/([a-z]+)-([0-9.]+)\/(.*)/, '\2 (\3) \4')} # http://gist.github.com/30430 add_silencer { |line| ALL_NOISE.any? { |dir| line.include?(dir) } } + add_silencer { |line| line =~ %r(vendor/plugins/[^\/]+/lib) } end end -- cgit v1.2.3