diff options
| author | Pratik Naik <pratiknaik@gmail.com> | 2009-01-18 17:45:39 +0000 |
|---|---|---|
| committer | Pratik Naik <pratiknaik@gmail.com> | 2009-01-18 17:45:39 +0000 |
| commit | aeafccf0ca2b7596a0b08c275d2c2154992fe573 (patch) | |
| tree | c0ab58cbfe0defc8e1078044e83d687d465d4d14 | |
| parent | e35e5ed2a4c87a10c70aa6b87a411182ce967c21 (diff) | |
| parent | 085991891e610ed0ab616ce434eabf42a9437039 (diff) | |
| download | rails-aeafccf0ca2b7596a0b08c275d2c2154992fe573.tar.gz rails-aeafccf0ca2b7596a0b08c275d2c2154992fe573.tar.bz2 rails-aeafccf0ca2b7596a0b08c275d2c2154992fe573.zip | |
Merge commit 'mainstream/master'
64 files changed, 793 insertions, 681 deletions
diff --git a/actionmailer/MIT-LICENSE b/actionmailer/MIT-LICENSE index 13c90d46e9..e7accc5ea1 100644 --- a/actionmailer/MIT-LICENSE +++ b/actionmailer/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2008 David Heinemeier Hansson +Copyright (c) 2004-2009 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/actionmailer/lib/action_mailer.rb b/actionmailer/lib/action_mailer.rb index 0f173ea4e8..45fcab599c 100644 --- a/actionmailer/lib/action_mailer.rb +++ b/actionmailer/lib/action_mailer.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2008 David Heinemeier Hansson +# Copyright (c) 2004-2009 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index c878a8d205..eda5de4e18 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -213,6 +213,8 @@ module ActionMailer #:nodoc: # * <tt>:password</tt> - If your mail server requires authentication, set the password in this setting. # * <tt>:authentication</tt> - If your mail server requires authentication, you need to specify the authentication type here. # This is a symbol and one of <tt>:plain</tt>, <tt>:login</tt>, <tt>:cram_md5</tt>. + # * <tt>:enable_starttls_auto</tt> - When set to true, detects if STARTTLS is enabled in your SMTP server and starts to use it. + # It works only on Ruby >= 1.8.7 and Ruby >= 1.9. Default is true. # # * <tt>sendmail_settings</tt> - Allows you to override options for the <tt>:sendmail</tt> delivery method. # * <tt>:location</tt> - The location of the sendmail executable. Defaults to <tt>/usr/sbin/sendmail</tt>. @@ -230,10 +232,13 @@ module ActionMailer #:nodoc: # # * <tt>default_charset</tt> - The default charset used for the body and to encode the subject. Defaults to UTF-8. You can also # pick a different charset from inside a method with +charset+. + # # * <tt>default_content_type</tt> - The default content type used for the main part of the message. Defaults to "text/plain". You # can also pick a different content type from inside a method with +content_type+. + # # * <tt>default_mime_version</tt> - The default mime version used for the message. Defaults to <tt>1.0</tt>. You # can also pick a different value from inside a method with +mime_version+. + # # * <tt>default_implicit_parts_order</tt> - When a message is built implicitly (i.e. multiple parts are assembled from templates # which specify the content type in their filenames) this variable controls how the parts are ordered. Defaults to # <tt>["text/html", "text/enriched", "text/plain"]</tt>. Items that appear first in the array have higher priority in the mail client @@ -252,12 +257,13 @@ module ActionMailer #:nodoc: cattr_accessor :logger @@smtp_settings = { - :address => "localhost", - :port => 25, - :domain => 'localhost.localdomain', - :user_name => nil, - :password => nil, - :authentication => nil + :address => "localhost", + :port => 25, + :domain => 'localhost.localdomain', + :user_name => nil, + :password => nil, + :authentication => nil, + :enable_starttls_auto => true, } cattr_accessor :smtp_settings @@ -669,7 +675,7 @@ module ActionMailer #:nodoc: sender = mail['return-path'] || mail.from smtp = Net::SMTP.new(smtp_settings[:address], smtp_settings[:port]) - smtp.enable_starttls_auto if smtp.respond_to?(:enable_starttls_auto) + smtp.enable_starttls_auto if smtp_settings[:enable_starttls_auto] && smtp.respond_to?(:enable_starttls_auto) smtp.start(smtp_settings[:domain], smtp_settings[:user_name], smtp_settings[:password], smtp_settings[:authentication]) do |smtp| smtp.sendmail(mail.encoded, sender, destinations) diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb index 15a40552c9..a886b1143e 100644 --- a/actionmailer/test/mail_service_test.rb +++ b/actionmailer/test/mail_service_test.rb @@ -948,6 +948,7 @@ EOF end def test_starttls_is_enabled_if_supported + ActionMailer::Base.smtp_settings[:enable_starttls_auto] = true MockSMTP.any_instance.expects(:respond_to?).with(:enable_starttls_auto).returns(true) MockSMTP.any_instance.expects(:enable_starttls_auto) ActionMailer::Base.delivery_method = :smtp @@ -955,11 +956,22 @@ EOF end def test_starttls_is_disabled_if_not_supported + ActionMailer::Base.smtp_settings[:enable_starttls_auto] = true MockSMTP.any_instance.expects(:respond_to?).with(:enable_starttls_auto).returns(false) MockSMTP.any_instance.expects(:enable_starttls_auto).never ActionMailer::Base.delivery_method = :smtp TestMailer.deliver_signed_up(@recipient) end + + def test_starttls_is_not_enabled + ActionMailer::Base.smtp_settings[:enable_starttls_auto] = false + MockSMTP.any_instance.expects(:respond_to?).never + MockSMTP.any_instance.expects(:enable_starttls_auto).never + ActionMailer::Base.delivery_method = :smtp + TestMailer.deliver_signed_up(@recipient) + ensure + ActionMailer::Base.smtp_settings[:enable_starttls_auto] = true + end end end # uses_mocha diff --git a/actionmailer/test/quoting_test.rb b/actionmailer/test/quoting_test.rb index 2650efccdb..2fee1379db 100644 --- a/actionmailer/test/quoting_test.rb +++ b/actionmailer/test/quoting_test.rb @@ -48,8 +48,10 @@ class QuotingTest < Test::Unit::TestCase result = execute_in_sandbox(<<-CODE) $:.unshift(File.dirname(__FILE__) + "/../lib/") - $KCODE = 'u' - require 'jcode' + if RUBY_VERSION < '1.9' + $KCODE = 'u' + require 'jcode' + end require 'action_mailer/quoting' include ActionMailer::Quoting quoted_printable(#{original.inspect}, "UTF-8") diff --git a/actionpack/MIT-LICENSE b/actionpack/MIT-LICENSE index 13c90d46e9..e7accc5ea1 100644 --- a/actionpack/MIT-LICENSE +++ b/actionpack/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2008 David Heinemeier Hansson +Copyright (c) 2004-2009 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index f808bdd910..dca07a0afc 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2008 David Heinemeier Hansson +# Copyright (c) 2004-2009 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -56,9 +56,9 @@ module ActionController autoload :Integration, 'action_controller/integration' autoload :IntegrationTest, 'action_controller/integration' autoload :Layout, 'action_controller/layout' - autoload :Lock, 'action_controller/lock' autoload :MiddlewareStack, 'action_controller/middleware_stack' autoload :MimeResponds, 'action_controller/mime_responds' + autoload :ParamsParser, 'action_controller/params_parser' autoload :PolymorphicRoutes, 'action_controller/polymorphic_routes' autoload :RecordIdentifier, 'action_controller/record_identifier' autoload :Request, 'action_controller/request' @@ -75,6 +75,7 @@ module ActionController autoload :TestCase, 'action_controller/test_case' autoload :TestProcess, 'action_controller/test_process' autoload :Translation, 'action_controller/translation' + autoload :UploadedFile, 'action_controller/uploaded_file' autoload :UploadedStringIO, 'action_controller/uploaded_file' autoload :UploadedTempfile, 'action_controller/uploaded_file' autoload :UrlEncodedPairParser, 'action_controller/url_encoded_pair_parser' diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index e22114195c..7a380bd1fb 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -301,10 +301,7 @@ module ActionController #:nodoc: # A YAML parser is also available and can be turned on with: # # ActionController::Base.param_parsers[Mime::YAML] = :yaml - @@param_parsers = { Mime::MULTIPART_FORM => :multipart_form, - Mime::URL_ENCODED_FORM => :url_encoded_form, - Mime::XML => :xml_simple, - Mime::JSON => :json } + @@param_parsers = {} cattr_accessor :param_parsers # Controls the default charset for all renders. diff --git a/actionpack/lib/action_controller/lock.rb b/actionpack/lib/action_controller/lock.rb deleted file mode 100644 index c50762216e..0000000000 --- a/actionpack/lib/action_controller/lock.rb +++ /dev/null @@ -1,16 +0,0 @@ -module ActionController - class Lock - FLAG = 'rack.multithread'.freeze - - def initialize(app, lock = Mutex.new) - @app, @lock = app, lock - end - - def call(env) - old, env[FLAG] = env[FLAG], false - @lock.synchronize { @app.call(env) } - ensure - env[FLAG] = old - end - end -end diff --git a/actionpack/lib/action_controller/middlewares.rb b/actionpack/lib/action_controller/middlewares.rb index 0f038b8856..30dbc26f11 100644 --- a/actionpack/lib/action_controller/middlewares.rb +++ b/actionpack/lib/action_controller/middlewares.rb @@ -1,4 +1,4 @@ -use "ActionController::Lock", :if => lambda { +use "Rack::Lock", :if => lambda { !ActionController::Base.allow_concurrency } @@ -19,4 +19,5 @@ use "ActiveRecord::QueryCache", :if => lambda { defined?(ActiveRecord) } end use ActionController::RewindableInput +use ActionController::ParamsParser use Rack::MethodOverride diff --git a/actionpack/lib/action_controller/params_parser.rb b/actionpack/lib/action_controller/params_parser.rb new file mode 100644 index 0000000000..d269fe07fa --- /dev/null +++ b/actionpack/lib/action_controller/params_parser.rb @@ -0,0 +1,71 @@ +module ActionController + class ParamsParser + ActionController::Base.param_parsers[Mime::XML] = :xml_simple + ActionController::Base.param_parsers[Mime::JSON] = :json + + def initialize(app) + @app = app + end + + def call(env) + if params = parse_formatted_parameters(env) + env["action_controller.request.request_parameters"] = params + end + + @app.call(env) + end + + private + def parse_formatted_parameters(env) + request = Request.new(env) + + return false if request.content_length.zero? + + mime_type = content_type_from_legacy_post_data_format_header(env) || request.content_type + strategy = ActionController::Base.param_parsers[mime_type] + + return false unless strategy + + case strategy + when Proc + strategy.call(request.raw_post) + when :xml_simple, :xml_node + body = request.raw_post + body.blank? ? {} : Hash.from_xml(body).with_indifferent_access + when :yaml + YAML.load(request.raw_post) + when :json + body = request.raw_post + if body.blank? + {} + else + data = ActiveSupport::JSON.decode(body) + data = {:_json => data} unless data.is_a?(Hash) + data.with_indifferent_access + end + else + false + end + rescue Exception => e # YAML, XML or Ruby code block errors + raise + { "body" => request.raw_post, + "content_type" => request.content_type, + "content_length" => request.content_length, + "exception" => "#{e.message} (#{e.class})", + "backtrace" => e.backtrace } + end + + def content_type_from_legacy_post_data_format_header(env) + if x_post_format = env['HTTP_X_POST_DATA_FORMAT'] + case x_post_format.to_s.downcase + when 'yaml' + return Mime::YAML + when 'xml' + return Mime::XML + end + end + + nil + end + end +end diff --git a/actionpack/lib/action_controller/rack_ext.rb b/actionpack/lib/action_controller/rack_ext.rb index 3b142307e9..2ba6654e3d 100644 --- a/actionpack/lib/action_controller/rack_ext.rb +++ b/actionpack/lib/action_controller/rack_ext.rb @@ -1,22 +1,3 @@ -module Rack - module Utils - module Multipart - class << self - def parse_multipart_with_rewind(env) - result = parse_multipart_without_rewind(env) - - begin - env['rack.input'].rewind if env['rack.input'].respond_to?(:rewind) - rescue Errno::ESPIPE - # Handles exceptions raised by input streams that cannot be rewound - # such as when using plain CGI under Apache - end - - result - end - - alias_method_chain :parse_multipart, :rewind - end - end - end -end +require 'action_controller/rack_ext/lock' +require 'action_controller/rack_ext/multipart' +require 'action_controller/rack_ext/parse_query' diff --git a/actionpack/lib/action_controller/rack_ext/lock.rb b/actionpack/lib/action_controller/rack_ext/lock.rb new file mode 100644 index 0000000000..9bf1889065 --- /dev/null +++ b/actionpack/lib/action_controller/rack_ext/lock.rb @@ -0,0 +1,21 @@ +module Rack + # Rack::Lock was commited to Rack core + # http://github.com/rack/rack/commit/7409b0c + # Remove this when Rack 1.0 is released + unless defined? Lock + class Lock + FLAG = 'rack.multithread'.freeze + + def initialize(app, lock = Mutex.new) + @app, @lock = app, lock + end + + def call(env) + old, env[FLAG] = env[FLAG], false + @lock.synchronize { @app.call(env) } + ensure + env[FLAG] = old + end + end + end +end diff --git a/actionpack/lib/action_controller/rack_ext/multipart.rb b/actionpack/lib/action_controller/rack_ext/multipart.rb new file mode 100644 index 0000000000..3b142307e9 --- /dev/null +++ b/actionpack/lib/action_controller/rack_ext/multipart.rb @@ -0,0 +1,22 @@ +module Rack + module Utils + module Multipart + class << self + def parse_multipart_with_rewind(env) + result = parse_multipart_without_rewind(env) + + begin + env['rack.input'].rewind if env['rack.input'].respond_to?(:rewind) + rescue Errno::ESPIPE + # Handles exceptions raised by input streams that cannot be rewound + # such as when using plain CGI under Apache + end + + result + end + + alias_method_chain :parse_multipart, :rewind + end + end + end +end diff --git a/actionpack/lib/action_controller/rack_ext/parse_query.rb b/actionpack/lib/action_controller/rack_ext/parse_query.rb new file mode 100644 index 0000000000..2f21a57770 --- /dev/null +++ b/actionpack/lib/action_controller/rack_ext/parse_query.rb @@ -0,0 +1,18 @@ +# Rack does not automatically cleanup Safari 2 AJAX POST body +# This has not yet been commited to Rack, please +1 this ticket: +# http://rack.lighthouseapp.com/projects/22435/tickets/19 + +module Rack + module Utils + alias_method :parse_query_without_ajax_body_cleanup, :parse_query + module_function :parse_query_without_ajax_body_cleanup + + def parse_query(qs, d = '&;') + qs = qs.dup + qs.chop! if qs[-1] == 0 + qs.gsub!(/&_=$/, '') + parse_query_without_ajax_body_cleanup(qs, d) + end + module_function :parse_query + end +end diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 3d13705f8f..09dcd684e8 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -9,11 +9,6 @@ module ActionController class Request < Rack::Request extend ActiveSupport::Memoizable - def initialize(env) - super - @parser = ActionController::RequestParser.new(env) - end - %w[ AUTH_TYPE GATEWAY_INTERFACE PATH_TRANSLATED REMOTE_HOST REMOTE_IDENT REMOTE_USER REMOTE_ADDR @@ -94,7 +89,11 @@ module ActionController # For backward compatibility, the post \format is extracted from the # X-Post-Data-Format HTTP header if present. def content_type - Mime::Type.lookup(@parser.content_type_without_parameters) + if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/ + Mime::Type.lookup($1.strip.downcase) + else + nil + end end memoize :content_type @@ -382,7 +381,11 @@ EOM # Read the request \body. This is useful for web services that need to # work with raw requests directly. def raw_post - @parser.raw_post + unless @env.include? 'RAW_POST_DATA' + @env['RAW_POST_DATA'] = body.read(@env['CONTENT_LENGTH'].to_i) + body.rewind if body.respond_to?(:rewind) + end + @env['RAW_POST_DATA'] end # Returns both GET and POST \parameters in a single hash. @@ -411,19 +414,30 @@ EOM @env["rack.routing_args"] ||= {} end + # The request body is an IO input stream. If the RAW_POST_DATA environment + # variable is already set, wrap it in a StringIO. def body - @parser.body + if raw_post = @env['RAW_POST_DATA'] + raw_post.force_encoding(Encoding::BINARY) if raw_post.respond_to?(:force_encoding) + StringIO.new(raw_post) + else + @env['rack.input'] + end + end + + def form_data? + FORM_DATA_MEDIA_TYPES.include?(content_type.to_s) end # Override Rack's GET method to support nested query strings def GET - @parser.query_parameters + @env["action_controller.request.query_parameters"] ||= UrlEncodedPairParser.parse_query_parameters(query_string) end alias_method :query_parameters, :GET # Override Rack's POST method to support nested query strings def POST - @parser.request_parameters + @env["action_controller.request.request_parameters"] ||= UrlEncodedPairParser.parse_hash_parameters(super) end alias_method :request_parameters, :POST diff --git a/actionpack/lib/action_controller/request_parser.rb b/actionpack/lib/action_controller/request_parser.rb deleted file mode 100644 index d1739ef4d0..0000000000 --- a/actionpack/lib/action_controller/request_parser.rb +++ /dev/null @@ -1,315 +0,0 @@ -module ActionController - class RequestParser - def initialize(env) - @env = env - freeze - end - - def request_parameters - @env["action_controller.request_parser.request_parameters"] ||= parse_formatted_request_parameters - end - - def query_parameters - @env["action_controller.request_parser.query_parameters"] ||= self.class.parse_query_parameters(query_string) - end - - # Returns the query string, accounting for server idiosyncrasies. - def query_string - @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].split('?', 2)[1] || '') - end - - # The request body is an IO input stream. If the RAW_POST_DATA environment - # variable is already set, wrap it in a StringIO. - def body - if raw_post = @env['RAW_POST_DATA'] - raw_post.force_encoding(Encoding::BINARY) if raw_post.respond_to?(:force_encoding) - StringIO.new(raw_post) - else - @env['rack.input'] - end - end - - # The raw content type string with its parameters stripped off. - def content_type_without_parameters - self.class.extract_content_type_without_parameters(content_type_with_parameters) - end - - def raw_post - unless @env.include? 'RAW_POST_DATA' - @env['RAW_POST_DATA'] = body.read(content_length) - body.rewind if body.respond_to?(:rewind) - end - @env['RAW_POST_DATA'] - end - - private - - def parse_formatted_request_parameters - return {} if content_length.zero? - - content_type, boundary = self.class.extract_multipart_boundary(content_type_with_parameters) - - # Don't parse params for unknown requests. - return {} if content_type.blank? - - mime_type = Mime::Type.lookup(content_type) - strategy = ActionController::Base.param_parsers[mime_type] - - # Only multipart form parsing expects a stream. - body = (strategy && strategy != :multipart_form) ? raw_post : self.body - - case strategy - when Proc - strategy.call(body) - when :url_encoded_form - self.class.clean_up_ajax_request_body! body - self.class.parse_query_parameters(body) - when :multipart_form - self.class.parse_multipart_form_parameters(body, boundary, content_length, @env) - when :xml_simple, :xml_node - body.blank? ? {} : Hash.from_xml(body).with_indifferent_access - when :yaml - YAML.load(body) - when :json - if body.blank? - {} - else - data = ActiveSupport::JSON.decode(body) - data = {:_json => data} unless data.is_a?(Hash) - data.with_indifferent_access - end - else - {} - end - rescue Exception => e # YAML, XML or Ruby code block errors - raise - { "body" => body, - "content_type" => content_type_with_parameters, - "content_length" => content_length, - "exception" => "#{e.message} (#{e.class})", - "backtrace" => e.backtrace } - end - - def content_length - @env['CONTENT_LENGTH'].to_i - end - - # The raw content type string. Use when you need parameters such as - # charset or boundary which aren't included in the content_type MIME type. - # Overridden by the X-POST_DATA_FORMAT header for backward compatibility. - def content_type_with_parameters - content_type_from_legacy_post_data_format_header || @env['CONTENT_TYPE'].to_s - end - - def content_type_from_legacy_post_data_format_header - if x_post_format = @env['HTTP_X_POST_DATA_FORMAT'] - case x_post_format.to_s.downcase - when 'yaml'; 'application/x-yaml' - when 'xml'; 'application/xml' - end - end - end - - class << self - def parse_query_parameters(query_string) - return {} if query_string.blank? - - pairs = query_string.split('&').collect do |chunk| - next if chunk.empty? - key, value = chunk.split('=', 2) - next if key.empty? - value = value.nil? ? nil : CGI.unescape(value) - [ CGI.unescape(key), value ] - end.compact - - UrlEncodedPairParser.new(pairs).result - end - - def parse_request_parameters(params) - parser = UrlEncodedPairParser.new - - params = params.dup - until params.empty? - for key, value in params - if key.blank? - params.delete key - elsif !key.include?('[') - # much faster to test for the most common case first (GET) - # and avoid the call to build_deep_hash - parser.result[key] = get_typed_value(value[0]) - params.delete key - elsif value.is_a?(Array) - parser.parse(key, get_typed_value(value.shift)) - params.delete key if value.empty? - else - raise TypeError, "Expected array, found #{value.inspect}" - end - end - end - - parser.result - end - - def parse_multipart_form_parameters(body, boundary, body_size, env) - parse_request_parameters(read_multipart(body, boundary, body_size, env)) - end - - def extract_multipart_boundary(content_type_with_parameters) - if content_type_with_parameters =~ MULTIPART_BOUNDARY - ['multipart/form-data', $1.dup] - else - extract_content_type_without_parameters(content_type_with_parameters) - end - end - - def extract_content_type_without_parameters(content_type_with_parameters) - $1.strip.downcase if content_type_with_parameters =~ /^([^,\;]*)/ - end - - def clean_up_ajax_request_body!(body) - body.chop! if body[-1] == 0 - body.gsub!(/&_=$/, '') - end - - - private - def get_typed_value(value) - case value - when String - value - when NilClass - '' - when Array - value.map { |v| get_typed_value(v) } - else - if value.respond_to? :original_filename - # Uploaded file - if value.original_filename - value - # Multipart param - else - result = value.read - value.rewind - result - end - # Unknown value, neither string nor multipart. - else - raise "Unknown form value: #{value.inspect}" - end - end - end - - MULTIPART_BOUNDARY = %r|\Amultipart/form-data.*boundary=\"?([^\";,]+)\"?|n - - EOL = "\015\012" - - def read_multipart(body, boundary, body_size, env) - params = Hash.new([]) - boundary = "--" + boundary - quoted_boundary = Regexp.quote(boundary) - buf = "" - bufsize = 10 * 1024 - boundary_end="" - - # start multipart/form-data - body.binmode if defined? body.binmode - case body - when File - body.set_encoding(Encoding::BINARY) if body.respond_to?(:set_encoding) - when StringIO - body.string.force_encoding(Encoding::BINARY) if body.string.respond_to?(:force_encoding) - end - boundary_size = boundary.size + EOL.size - body_size -= boundary_size - status = body.read(boundary_size) - if nil == status - raise EOFError, "no content body" - elsif boundary + EOL != status - raise EOFError, "bad content body" - end - - loop do - head = nil - content = - if 10240 < body_size - UploadedTempfile.new("CGI") - else - UploadedStringIO.new - end - content.binmode if defined? content.binmode - - until head and /#{quoted_boundary}(?:#{EOL}|--)/n.match(buf) - - if (not head) and /#{EOL}#{EOL}/n.match(buf) - buf = buf.sub(/\A((?:.|\n)*?#{EOL})#{EOL}/n) do - head = $1.dup - "" - end - next - end - - if head and ( (EOL + boundary + EOL).size < buf.size ) - content.print buf[0 ... (buf.size - (EOL + boundary + EOL).size)] - buf[0 ... (buf.size - (EOL + boundary + EOL).size)] = "" - end - - c = if bufsize < body_size - body.read(bufsize) - else - body.read(body_size) - end - if c.nil? || c.empty? - raise EOFError, "bad content body" - end - buf.concat(c) - body_size -= c.size - end - - buf = buf.sub(/\A((?:.|\n)*?)(?:[\r\n]{1,2})?#{quoted_boundary}([\r\n]{1,2}|--)/n) do - content.print $1 - if "--" == $2 - body_size = -1 - end - boundary_end = $2.dup - "" - end - - content.rewind - - head =~ /Content-Disposition:.* filename=(?:"((?:\\.|[^\"])*)"|([^;]*))/ni - if filename = $1 || $2 - if /Mac/ni.match(env['HTTP_USER_AGENT']) and - /Mozilla/ni.match(env['HTTP_USER_AGENT']) and - (not /MSIE/ni.match(env['HTTP_USER_AGENT'])) - filename = CGI.unescape(filename) - end - content.original_path = filename.dup - end - - head =~ /Content-Type: ([^\r]*)/ni - content.content_type = $1.dup if $1 - - head =~ /Content-Disposition:.* name="?([^\";]*)"?/ni - name = $1.dup if $1 - - if params.has_key?(name) - params[name].push(content) - else - params[name] = [content] - end - break if body_size == -1 - end - raise EOFError, "bad boundary end of body part" unless boundary_end=~/--/ - - begin - body.rewind if body.respond_to?(:rewind) - rescue Errno::ESPIPE - # Handles exceptions raised by input streams that cannot be rewound - # such as when using plain CGI under Apache - end - - params - end - end # class << self - end -end diff --git a/actionpack/lib/action_controller/request_profiler.rb b/actionpack/lib/action_controller/request_profiler.rb deleted file mode 100644 index 80cd55334f..0000000000 --- a/actionpack/lib/action_controller/request_profiler.rb +++ /dev/null @@ -1,168 +0,0 @@ -require 'optparse' - -module ActionController - class RequestProfiler - # Wrap up the integration session runner. - class Sandbox - include Integration::Runner - - def self.benchmark(n, script) - new(script).benchmark(n) - end - - def initialize(script_path) - @quiet = false - define_run_method(script_path) - reset! - end - - def benchmark(n, profiling = false) - @quiet = true - print ' ' - - ms = Benchmark.ms do - n.times do |i| - run(profiling) - print_progress(i) - end - end - - puts - ms - ensure - @quiet = false - end - - def say(message) - puts " #{message}" unless @quiet - end - - private - def define_run_method(script_path) - script = File.read(script_path) - - source = <<-end_source - def run(profiling = false) - if profiling - RubyProf.resume do - #{script} - end - else - #{script} - end - - old_request_count = request_count - reset! - self.request_count = old_request_count - end - end_source - - instance_eval source, script_path, 1 - end - - def print_progress(i) - print "\n " if i % 60 == 0 - print ' ' if i % 10 == 0 - print '.' - $stdout.flush - end - end - - - attr_reader :options - - def initialize(options = {}) - @options = default_options.merge(options) - end - - - def self.run(args = nil, options = {}) - profiler = new(options) - profiler.parse_options(args) if args - profiler.run - end - - def run - sandbox = Sandbox.new(options[:script]) - - puts 'Warming up once' - - elapsed = warmup(sandbox) - puts '%.0f ms, %d requests, %d req/sec' % [elapsed, sandbox.request_count, 1000 * sandbox.request_count / elapsed] - puts "\n#{options[:benchmark] ? 'Benchmarking' : 'Profiling'} #{options[:n]}x" - - options[:benchmark] ? benchmark(sandbox) : profile(sandbox) - end - - def profile(sandbox) - load_ruby_prof - - benchmark(sandbox, true) - results = RubyProf.stop - - show_profile_results results - results - end - - def benchmark(sandbox, profiling = false) - sandbox.request_count = 0 - elapsed = sandbox.benchmark(options[:n], profiling) - count = sandbox.request_count.to_i - puts '%.0f ms, %d requests, %d req/sec' % [elapsed, count, 1000 * count / elapsed] - end - - def warmup(sandbox) - Benchmark.ms { sandbox.run(false) } - end - - def default_options - { :n => 100, :open => 'open %s &' } - end - - # Parse command-line options - def parse_options(args) - OptionParser.new do |opt| - opt.banner = "USAGE: #{$0} [options] [session script path]" - - opt.on('-n', '--times [100]', 'How many requests to process. Defaults to 100.') { |v| options[:n] = v.to_i if v } - opt.on('-b', '--benchmark', 'Benchmark instead of profiling') { |v| options[:benchmark] = v } - opt.on('-m', '--measure [mode]', 'Which ruby-prof measure mode to use: process_time, wall_time, cpu_time, allocations, or memory. Defaults to process_time.') { |v| options[:measure] = v } - opt.on('--open [CMD]', 'Command to open profile results. Defaults to "open %s &"') { |v| options[:open] = v } - opt.on('-h', '--help', 'Show this help') { puts opt; exit } - - opt.parse args - - if args.empty? - puts opt - exit - end - options[:script] = args.pop - end - end - - protected - def load_ruby_prof - begin - gem 'ruby-prof', '>= 0.6.1' - require 'ruby-prof' - if mode = options[:measure] - RubyProf.measure_mode = RubyProf.const_get(mode.upcase) - end - rescue LoadError - abort '`gem install ruby-prof` to use the profiler' - end - end - - def show_profile_results(results) - File.open "#{RAILS_ROOT}/tmp/profile-graph.html", 'w' do |file| - RubyProf::GraphHtmlPrinter.new(results).print(file) - `#{options[:open] % file.path}` if options[:open] - end - - File.open "#{RAILS_ROOT}/tmp/profile-flat.txt", 'w' do |file| - RubyProf::FlatPrinter.new(results).print(file) - `#{options[:open] % file.path}` if options[:open] - end - end - end -end diff --git a/actionpack/lib/action_controller/rewindable_input.rb b/actionpack/lib/action_controller/rewindable_input.rb index 058453ea68..36f655c51e 100644 --- a/actionpack/lib/action_controller/rewindable_input.rb +++ b/actionpack/lib/action_controller/rewindable_input.rb @@ -3,33 +3,17 @@ module ActionController class RewindableIO < ActiveSupport::BasicObject def initialize(io) @io = io - end - - def read(*args) - read_original_io - @io.read(*args) - end - - def rewind - read_original_io - @io.rewind - end - - def string - @string + @rewindable = io.is_a?(StringIO) end def method_missing(method, *args, &block) - @io.send(method, *args, &block) - end - - private - def read_original_io - unless @string - @string = @io.read - @io = StringIO.new(@string) - end + unless @rewindable + @io = StringIO.new(@io.read) + @rewindable = true end + + @io.__send__(method, *args, &block) + end end def initialize(app) diff --git a/actionpack/lib/action_controller/uploaded_file.rb b/actionpack/lib/action_controller/uploaded_file.rb index ea4845c68f..376ba3621a 100644 --- a/actionpack/lib/action_controller/uploaded_file.rb +++ b/actionpack/lib/action_controller/uploaded_file.rb @@ -7,6 +7,13 @@ module ActionController end end + def self.extended(object) + object.class_eval do + attr_accessor :original_path, :content_type + alias_method :local_path, :path + end + end + # Take the basename of the upload's original filename. # This handles the full Windows paths given by Internet Explorer # (and perhaps other broken user agents) without affecting diff --git a/actionpack/lib/action_controller/url_encoded_pair_parser.rb b/actionpack/lib/action_controller/url_encoded_pair_parser.rb index 9883ad0d85..b63dca987d 100644 --- a/actionpack/lib/action_controller/url_encoded_pair_parser.rb +++ b/actionpack/lib/action_controller/url_encoded_pair_parser.rb @@ -1,5 +1,66 @@ module ActionController class UrlEncodedPairParser < StringScanner #:nodoc: + class << self + def parse_query_parameters(query_string) + return {} if query_string.blank? + + pairs = query_string.split('&').collect do |chunk| + next if chunk.empty? + key, value = chunk.split('=', 2) + next if key.empty? + value = value.nil? ? nil : CGI.unescape(value) + [ CGI.unescape(key), value ] + end.compact + + new(pairs).result + end + + def parse_hash_parameters(params) + parser = new + + params = params.dup + until params.empty? + for key, value in params + if key.blank? + params.delete(key) + elsif value.is_a?(Array) + parser.parse(key, get_typed_value(value.shift)) + params.delete(key) if value.empty? + else + parser.parse(key, get_typed_value(value)) + params.delete(key) + end + end + end + + parser.result + end + + private + def get_typed_value(value) + case value + when String + value + when NilClass + '' + when Array + value.map { |v| get_typed_value(v) } + when Hash + if value.has_key?(:tempfile) + upload = value[:tempfile] + upload.extend(UploadedFile) + upload.original_path = value[:filename] + upload.content_type = value[:type] + upload + else + nil + end + else + raise "Unknown form value: #{value.inspect}" + end + end + end + attr_reader :top, :parent, :result def initialize(pairs = []) diff --git a/actionpack/lib/action_pack.rb b/actionpack/lib/action_pack.rb index c7fd3092e7..b90f89be39 100644 --- a/actionpack/lib/action_pack.rb +++ b/actionpack/lib/action_pack.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2008 David Heinemeier Hansson +# Copyright (c) 2004-2009 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 210a5f1a93..0b710bd8d9 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2008 David Heinemeier Hansson +# Copyright (c) 2004-2009 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 4305617ac8..b4c1adbe76 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -860,7 +860,7 @@ module ActionView # => post[written_on(1i)] def input_name_from_type(type) prefix = @options[:prefix] || ActionView::Helpers::DateTimeSelector::DEFAULT_PREFIX - prefix += "[#{@options[:index]}]" if @options[:index] + prefix += "[#{@options[:index]}]" if @options.has_key?(:index) field_name = @options[:field_name] || type if @options[:include_position] @@ -923,7 +923,7 @@ module ActionView options[:field_name] = @method_name options[:include_position] = true options[:prefix] ||= @object_name - options[:index] ||= @auto_index + options[:index] = @auto_index if @auto_index && !options.has_key?(:index) options[:datetime_separator] ||= ' — ' options[:time_separator] ||= ' : ' @@ -961,15 +961,15 @@ module ActionView class FormBuilder def date_select(method, options = {}, html_options = {}) - @template.date_select(@object_name, method, options.merge(:object => @object), html_options) + @template.date_select(@object_name, method, objectify_options(options), html_options) end def time_select(method, options = {}, html_options = {}) - @template.time_select(@object_name, method, options.merge(:object => @object), html_options) + @template.time_select(@object_name, method, objectify_options(options), html_options) end def datetime_select(method, options = {}, html_options = {}) - @template.datetime_select(@object_name, method, options.merge(:object => @object), html_options) + @template.datetime_select(@object_name, method, objectify_options(options), html_options) end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 88ee07d95b..9d1e0d3ac5 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -204,7 +204,7 @@ module ActionView #:nodoc: private def valid_extension?(extension) - Template.template_handler_extensions.include?(extension) + !Template.registered_template_handler(extension).nil? end def find_full_path(path, load_paths) diff --git a/actionpack/lib/action_view/template_handlers.rb b/actionpack/lib/action_view/template_handlers.rb index d06ddd5fb5..205f8628f0 100644 --- a/actionpack/lib/action_view/template_handlers.rb +++ b/actionpack/lib/action_view/template_handlers.rb @@ -32,13 +32,17 @@ module ActionView #:nodoc: @@template_handlers.keys.map(&:to_s).sort end + def registered_template_handler(extension) + extension && @@template_handlers[extension.to_sym] + end + def register_default_template_handler(extension, klass) register_template_handler(extension, klass) @@default_template_handlers = klass end def handler_class_for_extension(extension) - (extension && @@template_handlers[extension.to_sym]) || @@default_template_handlers + registered_template_handler(extension) || @@default_template_handlers end end end diff --git a/actionpack/test/controller/rack_test.rb b/actionpack/test/controller/rack_test.rb index 8fd004a9e9..8ad42614b4 100644 --- a/actionpack/test/controller/rack_test.rb +++ b/actionpack/test/controller/rack_test.rb @@ -57,7 +57,7 @@ class BaseRackTest < Test::Unit::TestCase @request.env['REQUEST_METHOD'] = 'POST' @request.env['CONTENT_LENGTH'] = data.length @request.env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded; charset=utf-8' - @request.env['RAW_POST_DATA'] = data + @request.env['rack.input'] = StringIO.new(data) end end diff --git a/actionpack/test/controller/request/multipart_params_parsing_test.rb b/actionpack/test/controller/request/multipart_params_parsing_test.rb index 137fdbee54..d7ade40f71 100644 --- a/actionpack/test/controller/request/multipart_params_parsing_test.rb +++ b/actionpack/test/controller/request/multipart_params_parsing_test.rb @@ -36,7 +36,7 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest assert_equal 'bar', params['foo'] file = params['file'] - assert_kind_of StringIO, file + assert_kind_of Tempfile, file assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type assert_equal 'contents', file.read @@ -77,13 +77,13 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest assert_equal 'bar', params['foo'] file = params['file'] - assert_kind_of StringIO, file + assert_kind_of Tempfile, file assert_equal 'file.csv', file.original_filename assert_nil file.content_type assert_equal 'contents', file.read file = params['flowers'] - assert_kind_of StringIO, file + assert_kind_of Tempfile, file assert_equal 'flowers.jpg', file.original_filename assert_equal "image/jpeg", file.content_type assert_equal 19512, file.size diff --git a/actionpack/test/controller/request/url_encoded_params_parsing_test.rb b/actionpack/test/controller/request/url_encoded_params_parsing_test.rb index ee2a239d50..89239687de 100644 --- a/actionpack/test/controller/request/url_encoded_params_parsing_test.rb +++ b/actionpack/test/controller/request/url_encoded_params_parsing_test.rb @@ -150,6 +150,18 @@ class UrlEncodedParamsParsingTest < ActionController::IntegrationTest assert_parses expected, query end + test "parses params with Safari 2 trailing null character" do + query = "selected[]=1&selected[]=2&selected[]=3\0" + expected = { "selected" => [ "1", "2", "3" ] } + assert_parses expected, query + end + + test "parses params with Prototype's hack around Safari 2 trailing null character" do + query = "selected[]=1&selected[]=2&selected[]=3&_=" + expected = { "selected" => [ "1", "2", "3" ] } + assert_parses expected, query + end + test "passes through rack middleware and parses params" do with_muck_middleware do assert_parses({ "a" => { "b" => "c" } }, "a[b]=c") diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index 6ec01b7a8f..92cdce2e45 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -1228,6 +1228,38 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal(expected, output_buffer) end + def test_date_select_within_fields_for_with_index + @post = Post.new + @post.written_on = Date.new(2004, 6, 15) + id = 27 + + fields_for :post, @post, :index => id do |f| + concat f.date_select(:written_on) + end + + expected = "<select id='post_#{id}_written_on_1i' name='post[#{id}][written_on(1i)]'>\n<option value='1999'>1999</option>\n<option value='2000'>2000</option>\n<option value='2001'>2001</option>\n<option value='2002'>2002</option>\n<option value='2003'>2003</option>\n<option selected='selected' value='2004'>2004</option>\n<option value='2005'>2005</option>\n<option value='2006'>2006</option>\n<option value='2007'>2007</option>\n<option value='2008'>2008</option>\n<option value='2009'>2009</option>\n</select>\n" + expected << "<select id='post_#{id}_written_on_2i' name='post[#{id}][written_on(2i)]'>\n<option value='1'>January</option>\n<option value='2'>February</option>\n<option value='3'>March</option>\n<option value='4'>April</option>\n<option value='5'>May</option>\n<option selected='selected' value='6'>June</option>\n<option value='7'>July</option>\n<option value='8'>August</option>\n<option value='9'>September</option>\n<option value='10'>October</option>\n<option value='11'>November</option>\n<option value='12'>December</option>\n</select>\n" + expected << "<select id='post_#{id}_written_on_3i' name='post[#{id}][written_on(3i)]'>\n<option value='1'>1</option>\n<option value='2'>2</option>\n<option value='3'>3</option>\n<option value='4'>4</option>\n<option value='5'>5</option>\n<option value='6'>6</option>\n<option value='7'>7</option>\n<option value='8'>8</option>\n<option value='9'>9</option>\n<option value='10'>10</option>\n<option value='11'>11</option>\n<option value='12'>12</option>\n<option value='13'>13</option>\n<option value='14'>14</option>\n<option selected='selected' value='15'>15</option>\n<option value='16'>16</option>\n<option value='17'>17</option>\n<option value='18'>18</option>\n<option value='19'>19</option>\n<option value='20'>20</option>\n<option value='21'>21</option>\n<option value='22'>22</option>\n<option value='23'>23</option>\n<option value='24'>24</option>\n<option value='25'>25</option>\n<option value='26'>26</option>\n<option value='27'>27</option>\n<option value='28'>28</option>\n<option value='29'>29</option>\n<option value='30'>30</option>\n<option value='31'>31</option>\n</select>\n" + + assert_dom_equal(expected, output_buffer) + end + + def test_date_select_within_fields_for_with_blank_index + @post = Post.new + @post.written_on = Date.new(2004, 6, 15) + id = nil + + fields_for :post, @post, :index => id do |f| + concat f.date_select(:written_on) + end + + expected = "<select id='post_#{id}_written_on_1i' name='post[#{id}][written_on(1i)]'>\n<option value='1999'>1999</option>\n<option value='2000'>2000</option>\n<option value='2001'>2001</option>\n<option value='2002'>2002</option>\n<option value='2003'>2003</option>\n<option selected='selected' value='2004'>2004</option>\n<option value='2005'>2005</option>\n<option value='2006'>2006</option>\n<option value='2007'>2007</option>\n<option value='2008'>2008</option>\n<option value='2009'>2009</option>\n</select>\n" + expected << "<select id='post_#{id}_written_on_2i' name='post[#{id}][written_on(2i)]'>\n<option value='1'>January</option>\n<option value='2'>February</option>\n<option value='3'>March</option>\n<option value='4'>April</option>\n<option value='5'>May</option>\n<option selected='selected' value='6'>June</option>\n<option value='7'>July</option>\n<option value='8'>August</option>\n<option value='9'>September</option>\n<option value='10'>October</option>\n<option value='11'>November</option>\n<option value='12'>December</option>\n</select>\n" + expected << "<select id='post_#{id}_written_on_3i' name='post[#{id}][written_on(3i)]'>\n<option value='1'>1</option>\n<option value='2'>2</option>\n<option value='3'>3</option>\n<option value='4'>4</option>\n<option value='5'>5</option>\n<option value='6'>6</option>\n<option value='7'>7</option>\n<option value='8'>8</option>\n<option value='9'>9</option>\n<option value='10'>10</option>\n<option value='11'>11</option>\n<option value='12'>12</option>\n<option value='13'>13</option>\n<option value='14'>14</option>\n<option selected='selected' value='15'>15</option>\n<option value='16'>16</option>\n<option value='17'>17</option>\n<option value='18'>18</option>\n<option value='19'>19</option>\n<option value='20'>20</option>\n<option value='21'>21</option>\n<option value='22'>22</option>\n<option value='23'>23</option>\n<option value='24'>24</option>\n<option value='25'>25</option>\n<option value='26'>26</option>\n<option value='27'>27</option>\n<option value='28'>28</option>\n<option value='29'>29</option>\n<option value='30'>30</option>\n<option value='31'>31</option>\n</select>\n" + + assert_dom_equal(expected, output_buffer) + end + def test_date_select_with_index @post = Post.new @post.written_on = Date.new(2004, 6, 15) @@ -1243,7 +1275,6 @@ class DateHelperTest < ActionView::TestCase expected << %{<select id="post_456_written_on_3i" name="post[#{id}][written_on(3i)]">\n} expected << %{<option value="1">1</option>\n<option value="2">2</option>\n<option value="3">3</option>\n<option value="4">4</option>\n<option value="5">5</option>\n<option value="6">6</option>\n<option value="7">7</option>\n<option value="8">8</option>\n<option value="9">9</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15" selected="selected">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n} - expected << "</select>\n" assert_dom_equal expected, date_select("post", "written_on", :index => id) @@ -1330,13 +1361,13 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, date_select("post", "written_on", :include_blank => true) end - + def test_date_select_with_nil_and_blank_and_order @post = Post.new start_year = Time.now.year-5 end_year = Time.now.year+5 - + expected = '<input name="post[written_on(3i)]" type="hidden" id="post_written_on_3i"/>' + "\n" expected << %{<select id="post_written_on_1i" name="post[written_on(1i)]">\n} expected << "<option value=\"\"></option>\n" @@ -1966,6 +1997,40 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, datetime_select("post", "updated_at", :index => id) end + def test_datetime_select_within_fields_for_with_options_index + @post = Post.new + @post.updated_at = Time.local(2004, 6, 15, 16, 35) + id = 456 + + fields_for :post, @post, :index => id do |f| + concat f.datetime_select(:updated_at) + end + + expected = %{<select id="post_456_updated_at_1i" name="post[#{id}][updated_at(1i)]">\n} + expected << %{<option value="1999">1999</option>\n<option value="2000">2000</option>\n<option value="2001">2001</option>\n<option value="2002">2002</option>\n<option value="2003">2003</option>\n<option value="2004" selected="selected">2004</option>\n<option value="2005">2005</option>\n<option value="2006">2006</option>\n<option value="2007">2007</option>\n<option value="2008">2008</option>\n<option value="2009">2009</option>\n} + expected << "</select>\n" + + expected << %{<select id="post_456_updated_at_2i" name="post[#{id}][updated_at(2i)]">\n} + expected << %{<option value="1">January</option>\n<option value="2">February</option>\n<option value="3">March</option>\n<option value="4">April</option>\n<option value="5">May</option>\n<option value="6" selected="selected">June</option>\n<option value="7">July</option>\n<option value="8">August</option>\n<option value="9">September</option>\n<option value="10">October</option>\n<option value="11">November</option>\n<option value="12">December</option>\n} + expected << "</select>\n" + + expected << %{<select id="post_456_updated_at_3i" name="post[#{id}][updated_at(3i)]">\n} + expected << %{<option value="1">1</option>\n<option value="2">2</option>\n<option value="3">3</option>\n<option value="4">4</option>\n<option value="5">5</option>\n<option value="6">6</option>\n<option value="7">7</option>\n<option value="8">8</option>\n<option value="9">9</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15" selected="selected">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n} + expected << "</select>\n" + + expected << " — " + + expected << %{<select id="post_456_updated_at_4i" name="post[#{id}][updated_at(4i)]">\n} + expected << %{<option value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16" selected="selected">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n} + expected << "</select>\n" + expected << " : " + expected << %{<select id="post_456_updated_at_5i" name="post[#{id}][updated_at(5i)]">\n} + expected << %{<option value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n<option value="32">32</option>\n<option value="33">33</option>\n<option value="34">34</option>\n<option value="35" selected="selected">35</option>\n<option value="36">36</option>\n<option value="37">37</option>\n<option value="38">38</option>\n<option value="39">39</option>\n<option value="40">40</option>\n<option value="41">41</option>\n<option value="42">42</option>\n<option value="43">43</option>\n<option value="44">44</option>\n<option value="45">45</option>\n<option value="46">46</option>\n<option value="47">47</option>\n<option value="48">48</option>\n<option value="49">49</option>\n<option value="50">50</option>\n<option value="51">51</option>\n<option value="52">52</option>\n<option value="53">53</option>\n<option value="54">54</option>\n<option value="55">55</option>\n<option value="56">56</option>\n<option value="57">57</option>\n<option value="58">58</option>\n<option value="59">59</option>\n} + expected << "</select>\n" + + assert_dom_equal expected, output_buffer + end + def test_datetime_select_with_auto_index @post = Post.new @post.updated_at = Time.local(2004, 6, 15, 16, 35) @@ -2253,7 +2318,7 @@ class DateHelperTest < ActionView::TestCase @post = Post.new @post.updated_at = Time.local(2008, 7, 16, 23, 30) - options = { + options = { :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2265,7 +2330,7 @@ class DateHelperTest < ActionView::TestCase # note: the literal hash is intentional to show that the actual options hash isn't modified # don't change this! - assert_equal({ + assert_equal({ :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2279,7 +2344,7 @@ class DateHelperTest < ActionView::TestCase @post = Post.new @post.updated_at = Time.local(2008, 7, 16, 23, 30) - options = { + options = { :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2291,7 +2356,7 @@ class DateHelperTest < ActionView::TestCase # note: the literal hash is intentional to show that the actual options hash isn't modified # don't change this! - assert_equal({ + assert_equal({ :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2305,7 +2370,7 @@ class DateHelperTest < ActionView::TestCase @post = Post.new @post.updated_at = Time.local(2008, 7, 16, 23, 30) - options = { + options = { :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2317,7 +2382,7 @@ class DateHelperTest < ActionView::TestCase # note: the literal hash is intentional to show that the actual options hash isn't modified # don't change this! - assert_equal({ + assert_equal({ :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2328,7 +2393,7 @@ class DateHelperTest < ActionView::TestCase end def test_select_date_should_not_change_passed_options_hash - options = { + options = { :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2340,7 +2405,7 @@ class DateHelperTest < ActionView::TestCase # note: the literal hash is intentional to show that the actual options hash isn't modified # don't change this! - assert_equal({ + assert_equal({ :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2351,7 +2416,7 @@ class DateHelperTest < ActionView::TestCase end def test_select_datetime_should_not_change_passed_options_hash - options = { + options = { :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2363,7 +2428,7 @@ class DateHelperTest < ActionView::TestCase # note: the literal hash is intentional to show that the actual options hash isn't modified # don't change this! - assert_equal({ + assert_equal({ :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2374,7 +2439,7 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_should_not_change_passed_options_hash - options = { + options = { :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, @@ -2386,7 +2451,7 @@ class DateHelperTest < ActionView::TestCase # note: the literal hash is intentional to show that the actual options hash isn't modified # don't change this! - assert_equal({ + assert_equal({ :order => [ :year, :month, :day ], :default => { :year => 2008, :month => 7, :day => 16, :hour => 23, :minute => 30, :second => 1 }, :discard_type => false, diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index fbc71cddf1..507e37ac3b 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,9 @@ *2.3.0/3.0* +* Make after_save callbacks fire only if the record was successfully saved. #1735 [Michael Lovitt] + + Previously the callbacks would fire if a before_save cancelled saving. + * Support nested transactions using database savepoints. #383 [Jonathan Viney, Hongli Lai] * Added dynamic scopes ala dynamic finders #1648 [Yaroslav Markin] diff --git a/activerecord/MIT-LICENSE b/activerecord/MIT-LICENSE index 93be57f683..e6df48772f 100644 --- a/activerecord/MIT-LICENSE +++ b/activerecord/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2008 David Heinemeier Hansson +Copyright (c) 2004-2009 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 390c005785..e1265b7e1e 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2008 David Heinemeier Hansson +# Copyright (c) 2004-2009 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 86616abf52..8b51a38f48 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1531,14 +1531,14 @@ module ActiveRecord association = send(reflection.name) association.destroy unless association.nil? end - before_destroy method_name + after_destroy method_name when :delete method_name = "belongs_to_dependent_delete_for_#{reflection.name}".to_sym define_method(method_name) do association = send(reflection.name) association.delete unless association.nil? end - before_destroy method_name + after_destroy method_name else raise ArgumentError, "The :dependent option expects either :destroy or :delete (#{reflection.options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 3d689098b5..a5cc3bf091 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -9,6 +9,14 @@ module ActiveRecord create_record(attributes) { |record| insert_record(record, true) } end + def columns + @reflection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns") + end + + def reset_column_information + @reflection.reset_column_information + end + protected def construct_find_options!(options) options[:joins] = @join_sql @@ -32,8 +40,6 @@ module ActiveRecord if @reflection.options[:insert_sql] @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record)) else - columns = @owner.connection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns") - attributes = columns.inject({}) do |attrs, column| case column.name.to_s when @reflection.primary_key_name.to_s @@ -103,7 +109,7 @@ module ActiveRecord # clause has been explicitly defined. Otherwise you can get broken records back, if, for example, the join column also has # an id column. This will then overwrite the id column of the records coming back. def finding_with_ambiguous_select?(select_clause) - !select_clause && @owner.connection.columns(@reflection.options[:join_table], "Join Table Columns").size != 2 + !select_clause && columns.size != 2 end private diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index dc33f8c2c7..88958f4583 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -219,8 +219,9 @@ module ActiveRecord def after_save() end def create_or_update_with_callbacks #:nodoc: return false if callback(:before_save) == false - result = create_or_update_without_callbacks - callback(:after_save) + if result = create_or_update_without_callbacks + callback(:after_save) + end result end private :create_or_update_with_callbacks diff --git a/activerecord/lib/active_record/dirty.rb b/activerecord/lib/active_record/dirty.rb index 4c899f58e5..4a2510aa63 100644 --- a/activerecord/lib/active_record/dirty.rb +++ b/activerecord/lib/active_record/dirty.rb @@ -151,8 +151,8 @@ module ActiveRecord def field_changed?(attr, old, value) if column = column_for_attribute(attr) - if column.type == :integer && column.null && (old.nil? || old == 0) && value.blank? - # For nullable integer columns, NULL gets stored in database for blank (i.e. '') values. + if column.number? && column.null && (old.nil? || old == 0) && value.blank? + # For nullable numeric columns, NULL gets stored in database for blank (i.e. '') values. # Hence we don't record it as a change if the value changes from nil to ''. # If an old value of 0 is set to '' we want this to get changed to nil as otherwise it'll # be typecast back to 0 (''.to_i => 0) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index dbff4f24d6..1937abdc83 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -198,6 +198,14 @@ module ActiveRecord end end + def columns(tbl_name, log_msg) + @columns ||= klass.connection.columns(tbl_name, log_msg) + end + + def reset_column_information + @columns = nil + end + def check_validity! end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 2f08e09d43..1e3b423471 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -775,4 +775,15 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end end end + + def test_caching_of_columns + david = Developer.find(1) + # clear cache possibly created by other tests + david.projects.reset_column_information + assert_queries(0) { david.projects.columns; david.projects.columns } + # and again to verify that reset_column_information clears the cache correctly + david.projects.reset_column_information + assert_queries(0) { david.projects.columns; david.projects.columns } + 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 a5ae5cd8ec..428fb50013 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -698,7 +698,8 @@ class HasManyAssociationsTest < ActiveRecord::TestCase authors(:david).destroy end - assert_equal [author_address.id], AuthorAddress.destroyed_author_address_ids[authors(:david).id] + assert_equal nil, AuthorAddress.find_by_id(authors(:david).author_address_id) + assert_equal nil, AuthorAddress.find_by_id(authors(:david).author_address_extra_id) end def test_invalid_belongs_to_dependent_option_raises_exception diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 11830a2ef6..33b1ea034d 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -121,9 +121,19 @@ end class CallbackCancellationDeveloper < ActiveRecord::Base set_table_name 'developers' - def before_create - false - end + + attr_reader :after_save_called, :after_create_called, :after_update_called, :after_destroy_called + attr_accessor :cancel_before_save, :cancel_before_create, :cancel_before_update, :cancel_before_destroy + + def before_save; !@cancel_before_save; end + def before_create; !@cancel_before_create; end + def before_update; !@cancel_before_update; end + def before_destroy; !@cancel_before_destroy; end + + def after_save; @after_save_called = true; end + def after_update; @after_update_called = true; end + def after_create; @after_create_called = true; end + def after_destroy; @after_destroy_called = true; end end class CallbacksTest < ActiveRecord::TestCase @@ -349,19 +359,47 @@ class CallbacksTest < ActiveRecord::TestCase assert !david.valid? assert !david.save assert_raises(ActiveRecord::RecordInvalid) { david.save! } + + someone = CallbackCancellationDeveloper.find(1) + someone.cancel_before_save = true + assert someone.valid? + assert !someone.save + assert_save_callbacks_not_called(someone) end def test_before_create_returning_false someone = CallbackCancellationDeveloper.new + someone.cancel_before_create = true assert someone.valid? assert !someone.save + assert_save_callbacks_not_called(someone) + end + + def test_before_update_returning_false + someone = CallbackCancellationDeveloper.find(1) + someone.cancel_before_update = true + assert someone.valid? + assert !someone.save + assert_save_callbacks_not_called(someone) end def test_before_destroy_returning_false david = ImmutableDeveloper.find(1) assert !david.destroy assert_not_nil ImmutableDeveloper.find_by_id(1) + + someone = CallbackCancellationDeveloper.find(1) + someone.cancel_before_destroy = true + assert !someone.destroy + assert !someone.after_destroy_called + end + + def assert_save_callbacks_not_called(someone) + assert !someone.after_save_called + assert !someone.after_create_called + assert !someone.after_update_called end + private :assert_save_callbacks_not_called def test_zzz_callback_returning_false # must be run last since we modify CallbackDeveloper david = CallbackDeveloper.find(1) diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 10cdbdc622..1c9e281cc0 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -21,6 +21,10 @@ private end end +class NumericData < ActiveRecord::Base + self.table_name = 'numeric_data' +end + class DirtyTest < ActiveRecord::TestCase def test_attribute_changes # New record - no changes. @@ -58,7 +62,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal parrot.name_change, parrot.title_change end - def test_nullable_integer_not_marked_as_changed_if_new_value_is_blank + def test_nullable_number_not_marked_as_changed_if_new_value_is_blank pirate = Pirate.new ["", nil].each do |value| @@ -68,6 +72,26 @@ class DirtyTest < ActiveRecord::TestCase end end + def test_nullable_decimal_not_marked_as_changed_if_new_value_is_blank + numeric_data = NumericData.new + + ["", nil].each do |value| + numeric_data.bank_balance = value + assert !numeric_data.bank_balance_changed? + assert_nil numeric_data.bank_balance_change + end + end + + def test_nullable_float_not_marked_as_changed_if_new_value_is_blank + numeric_data = NumericData.new + + ["", nil].each do |value| + numeric_data.temperature = value + assert !numeric_data.temperature_changed? + assert_nil numeric_data.temperature_change + end + end + def test_nullable_integer_zero_to_string_zero_not_marked_as_changed pirate = Pirate.new pirate.parrot_id = 0 diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index ccd04b67f6..24ce35e2e2 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -34,7 +34,7 @@ rescue LoadError end ActiveRecord::Base.connection.class.class_eval do - IGNORED_SQL = [/^PRAGMA/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/] + IGNORED_SQL = [/^PRAGMA/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /SHOW FIELDS/] def execute_with_query_record(sql, name = nil, &block) $queries_executed ||= [] diff --git a/activerecord/test/cases/json_serialization_test.rb b/activerecord/test/cases/json_serialization_test.rb index 3446e5e7f0..975acde096 100644 --- a/activerecord/test/cases/json_serialization_test.rb +++ b/activerecord/test/cases/json_serialization_test.rb @@ -200,6 +200,6 @@ class DatabaseConnectedJsonEncodingTest < ActiveRecord::TestCase 2 => @mary } - assert_equal %({1: {"name": "David"}}), authors_hash.to_json(:only => [1, :name]) + assert_equal %({"1": {"name": "David"}}), authors_hash.to_json(:only => [1, :name]) end end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 8199cb8fc7..094932d375 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -252,6 +252,7 @@ ActiveRecord::Schema.define do t.decimal :world_population, :precision => 10, :scale => 0 t.decimal :my_house_population, :precision => 2, :scale => 0 t.decimal :decimal_number_with_default, :precision => 3, :scale => 2, :default => 2.78 + t.float :temperature end create_table :orders, :force => true do |t| diff --git a/activeresource/MIT-LICENSE b/activeresource/MIT-LICENSE index 7f65dd1989..e6df48772f 100644 --- a/activeresource/MIT-LICENSE +++ b/activeresource/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2006 David Heinemeier Hansson +Copyright (c) 2004-2009 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/activesupport/MIT-LICENSE b/activesupport/MIT-LICENSE index 2ba4e17035..d6fdf21596 100644 --- a/activesupport/MIT-LICENSE +++ b/activesupport/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2005-2008 David Heinemeier Hansson +Copyright (c) 2005-2009 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 6a6c861458..83174d3a85 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -10,6 +10,10 @@ module ActiveSupport autoload :MemCacheStore, 'active_support/cache/mem_cache_store' autoload :CompressedMemCacheStore, 'active_support/cache/compressed_mem_cache_store' + module Strategy + autoload :LocalCache, 'active_support/cache/strategy/local_cache' + end + # Creates a new CacheStore object according to the given options. # # If no arguments are passed to this method, then a new diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index f9a7fb1440..4d8e1fdd67 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -13,6 +13,7 @@ module ActiveSupport # server goes down, then MemCacheStore will ignore it until it goes back # online. # - Time-based expiry support. See #write and the +:expires_in+ option. + # - Per-request in memory cache for all communication with the MemCache server(s). class MemCacheStore < Store module Response # :nodoc: STORED = "STORED\r\n" @@ -38,6 +39,8 @@ module ActiveSupport addresses = ["localhost"] if addresses.empty? @addresses = addresses @data = MemCache.new(addresses, options) + + extend Strategy::LocalCache end def read(key, options = nil) # :nodoc: @@ -80,6 +83,7 @@ module ActiveSupport def exist?(key, options = nil) # :nodoc: # Doesn't call super, cause exist? in memcache is in fact a read # But who cares? Reading is very fast anyway + # Local cache is checked first, if it doesn't know then memcache itself is read from !read(key, options).nil? end @@ -94,7 +98,6 @@ module ActiveSupport def decrement(key, amount = 1) # :nodoc: log("decrement", key, amount) - response = @data.decr(key, amount) response == Response::NOT_FOUND ? nil : response rescue MemCache::MemCacheError @@ -102,6 +105,8 @@ module ActiveSupport end def delete_matched(matcher, options = nil) # :nodoc: + # don't do any local caching at present, just pass + # through and let the error happen super raise "Not supported by Memcache" end diff --git a/activesupport/lib/active_support/cache/strategy/local_cache.rb b/activesupport/lib/active_support/cache/strategy/local_cache.rb new file mode 100644 index 0000000000..621358d701 --- /dev/null +++ b/activesupport/lib/active_support/cache/strategy/local_cache.rb @@ -0,0 +1,104 @@ +module ActiveSupport + module Cache + module Strategy + module LocalCache + # this allows caching of the fact that there is nothing in the remote cache + NULL = 'remote_cache_store:null' + + def with_local_cache + Thread.current[thread_local_key] = MemoryStore.new + yield + ensure + Thread.current[thread_local_key] = nil + end + + def middleware + @middleware ||= begin + klass = Class.new + klass.class_eval(<<-EOS, __FILE__, __LINE__) + def initialize(app) + @app = app + end + + def call(env) + Thread.current[:#{thread_local_key}] = MemoryStore.new + @app.call(env) + ensure + Thread.current[:#{thread_local_key}] = nil + end + EOS + klass + end + end + + def read(key, options = nil) + value = local_cache && local_cache.read(key) + if value == NULL + nil + elsif value.nil? + value = super + local_cache.write(key, value || NULL) if local_cache + value + else + # forcing the value to be immutable + value.dup + end + end + + def write(key, value, options = nil) + value = value.to_s if respond_to?(:raw?) && raw?(options) + local_cache.write(key, value || NULL) if local_cache + super + end + + def delete(key, options = nil) + local_cache.write(key, NULL) if local_cache + super + end + + def exist(key, options = nil) + value = local_cache.read(key) if local_cache + if value == NULL + false + elsif value + true + else + super + end + end + + def increment(key, amount = 1) + if value = super + local_cache.write(key, value.to_s) if local_cache + value + else + nil + end + end + + def decrement(key, amount = 1) + if value = super + local_cache.write(key, value.to_s) if local_cache + value + else + nil + end + end + + def clear + local_cache.clear if local_cache + super + end + + private + def thread_local_key + @thread_local_key ||= "#{self.class.name.underscore}_local_cache".gsub("/", "_").to_sym + end + + def local_cache + Thread.current[thread_local_key] + end + end + end + end +end diff --git a/activesupport/lib/active_support/core_ext/object/misc.rb b/activesupport/lib/active_support/core_ext/object/misc.rb index c0a109ecf3..4acdfa3d6c 100644 --- a/activesupport/lib/active_support/core_ext/object/misc.rb +++ b/activesupport/lib/active_support/core_ext/object/misc.rb @@ -87,21 +87,4 @@ class Object respond_to? "acts_like_#{duck}?" end - # Tries to send the method only if object responds to it. Return +nil+ otherwise. - # It will also forward any arguments and/or block like Object#send does. - # - # ==== Example : - # - # # Without try - # @person ? @person.name : nil - # - # With try - # @person.try(:name) - # - # # try also accepts arguments/blocks for the method it is trying - # Person.try(:find, 1) - # @people.try(:map) {|p| p.name} - def try(method, *args, &block) - send(method, *args, &block) unless self.nil? - end end diff --git a/activesupport/lib/active_support/core_ext/try.rb b/activesupport/lib/active_support/core_ext/try.rb new file mode 100644 index 0000000000..0dccd40c55 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/try.rb @@ -0,0 +1,30 @@ +class Object + # Tries to send the method only if object responds to it. Return +nil+ otherwise. + # It will also forward any arguments and/or block like Object#send does. + # + # ==== Examples + # + # Without try + # @person && @person.name + # or + # @person ? @person.name : nil + # + # With try + # @person.try(:name) + # + # Try also accepts arguments/blocks for the method it is trying + # Person.try(:find, 1) + # @people.try(:collect) {|p| p.name} + #-- + # This method def is for rdoc only. The alias_method below overrides it as an optimization. + def try(method, *args, &block) + send(method, *args, &block) + end + alias_method :try, :__send__ +end + +class NilClass + def try(*args) + nil + end +end diff --git a/activesupport/lib/active_support/json/encoders/hash.rb b/activesupport/lib/active_support/json/encoders/hash.rb index b9bdd55fa5..16dc8337f5 100644 --- a/activesupport/lib/active_support/json/encoders/hash.rb +++ b/activesupport/lib/active_support/json/encoders/hash.rb @@ -5,7 +5,7 @@ class Hash # the hash keys. For example: # # { :name => "Konata Izumi", 'age' => 16, 1 => 2 }.to_json - # # => {"name": "Konata Izumi", 1: 2, "age": 16} + # # => {"name": "Konata Izumi", "1": 2, "age": 16} # # The keys in the JSON string are unordered due to the nature of hashes. # @@ -39,7 +39,7 @@ class Hash returning result = '{' do result << hash_keys.map do |key| - "#{ActiveSupport::JSON.encode(key)}: #{ActiveSupport::JSON.encode(self[key], options)}" + "#{ActiveSupport::JSON.encode(key.to_s)}: #{ActiveSupport::JSON.encode(self[key], options)}" end * ', ' result << '}' end diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 3def0be639..25ea505813 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -10,10 +10,14 @@ module ActiveSupport @keys = [] end + def initialize_copy(other) + super + # make a deep copy of keys + @keys = other.keys + end + def []=(key, value) - if !has_key?(key) - @keys << key - end + @keys << key if !has_key?(key) super end @@ -24,6 +28,12 @@ module ActiveSupport end super end + + def delete_if + super + sync_keys! + self + end def reject! super @@ -36,7 +46,7 @@ module ActiveSupport end def keys - @keys + @keys.dup end def values @@ -56,7 +66,7 @@ module ActiveSupport end def each - keys.each {|key| yield [key, self[key]]} + @keys.each {|key| yield [key, self[key]]} end alias_method :each_pair, :each @@ -73,13 +83,16 @@ module ActiveSupport [k, v] end + def merge!(other_hash) + other_hash.each {|k,v| self[k] = v } + self + end + def merge(other_hash) - result = dup - other_hash.each {|k,v| result[k]=v} - result + dup.merge!(other_hash) end - private + private def sync_keys! @keys.delete_if {|k| !has_key?(k)} diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index d8506de986..e8e0b41d4d 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -1,18 +1,18 @@ require 'abstract_unit' -class CacheKeyTest < Test::Unit::TestCase +class CacheKeyTest < ActiveSupport::TestCase def test_expand_cache_key assert_equal 'name/1/2/true', ActiveSupport::Cache.expand_cache_key([1, '2', true], :name) end end -class CacheStoreSettingTest < Test::Unit::TestCase +class CacheStoreSettingTest < ActiveSupport::TestCase def test_file_fragment_cache_store store = ActiveSupport::Cache.lookup_store :file_store, "/path/to/cache/directory" assert_kind_of(ActiveSupport::Cache::FileStore, store) assert_equal "/path/to/cache/directory", store.cache_path end - + def test_drb_fragment_cache_store store = ActiveSupport::Cache.lookup_store :drb_store, "druby://localhost:9192" assert_kind_of(ActiveSupport::Cache::DRbStore, store) @@ -24,13 +24,13 @@ class CacheStoreSettingTest < Test::Unit::TestCase assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) assert_equal %w(localhost), store.addresses end - + def test_mem_cache_fragment_cache_store_with_multiple_servers store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1' assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) assert_equal %w(localhost 192.168.1.1), store.addresses end - + def test_mem_cache_fragment_cache_store_with_options store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1', :namespace => 'foo' assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) @@ -45,7 +45,7 @@ class CacheStoreSettingTest < Test::Unit::TestCase end end -class CacheStoreTest < Test::Unit::TestCase +class CacheStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:memory_store) end @@ -116,9 +116,15 @@ module CacheStoreBehavior assert_equal 1, @cache.decrement('foo') assert_equal 1, @cache.read('foo', :raw => true).to_i end + + def test_exist + @cache.write('foo', 'bar') + assert @cache.exist?('foo') + assert !@cache.exist?('bar') + end end -class FileStoreTest < Test::Unit::TestCase +class FileStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:file_store, Dir.pwd) end @@ -130,7 +136,7 @@ class FileStoreTest < Test::Unit::TestCase include CacheStoreBehavior end -class MemoryStoreTest < Test::Unit::TestCase +class MemoryStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:memory_store) end @@ -145,28 +151,111 @@ class MemoryStoreTest < Test::Unit::TestCase end uses_memcached 'memcached backed store' do - class MemCacheStoreTest < Test::Unit::TestCase + class MemCacheStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:mem_cache_store) + @data = @cache.instance_variable_get(:@data) @cache.clear end include CacheStoreBehavior def test_store_objects_should_be_immutable - @cache.write('foo', 'bar') - @cache.read('foo').gsub!(/.*/, 'baz') - assert_equal 'bar', @cache.read('foo') + @cache.with_local_cache do + @cache.write('foo', 'bar') + @cache.read('foo').gsub!(/.*/, 'baz') + assert_equal 'bar', @cache.read('foo') + end end def test_write_should_return_true_on_success - result = @cache.write('foo', 'bar') - assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written - assert result + @cache.with_local_cache do + result = @cache.write('foo', 'bar') + assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written + assert result + end + end + + def test_local_writes_are_persistent_on_the_remote_cache + @cache.with_local_cache do + @cache.write('foo', 'bar') + end + + assert_equal 'bar', @cache.read('foo') + end + + def test_clear_also_clears_local_cache + @cache.with_local_cache do + @cache.write('foo', 'bar') + @cache.clear + assert_nil @cache.read('foo') + end + end + + def test_local_cache_of_read_and_write + @cache.with_local_cache do + @cache.write('foo', 'bar') + @data.flush_all # Clear remote cache + assert_equal 'bar', @cache.read('foo') + end + end + + def test_local_cache_of_delete + @cache.with_local_cache do + @cache.write('foo', 'bar') + @cache.delete('foo') + @data.flush_all # Clear remote cache + assert_nil @cache.read('foo') + end + end + + def test_local_cache_of_exist + @cache.with_local_cache do + @cache.write('foo', 'bar') + @cache.instance_variable_set(:@data, nil) + @data.flush_all # Clear remote cache + assert @cache.exist?('foo') + end + end + + def test_local_cache_of_increment + @cache.with_local_cache do + @cache.write('foo', 1, :raw => true) + @cache.increment('foo') + @data.flush_all # Clear remote cache + assert_equal 2, @cache.read('foo', :raw => true).to_i + end + end + + def test_local_cache_of_decrement + @cache.with_local_cache do + @cache.write('foo', 1, :raw => true) + @cache.decrement('foo') + @data.flush_all # Clear remote cache + assert_equal 0, @cache.read('foo', :raw => true).to_i + end + end + + def test_exist_with_nulls_cached_locally + @cache.with_local_cache do + @cache.write('foo', 'bar') + @cache.delete('foo') + assert !@cache.exist?('foo') + end + end + + def test_middleware + app = lambda { |env| + result = @cache.write('foo', 'bar') + assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written + assert result + } + app = @cache.middleware.new(app) + app.call({}) end end - class CompressedMemCacheStore < Test::Unit::TestCase + class CompressedMemCacheStore < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:compressed_mem_cache_store) @cache.clear diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 8ed21cc9ad..2c5b4d0378 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -59,7 +59,7 @@ class TestJSONEncoding < Test::Unit::TestCase assert_equal %({\"a\": \"b\"}), { :a => :b }.to_json assert_equal %({\"a\": 1}), { 'a' => 1 }.to_json assert_equal %({\"a\": [1, 2]}), { 'a' => [1,2] }.to_json - assert_equal %({1: 2}), { 1 => 2 }.to_json + assert_equal %({"1": 2}), { 1 => 2 }.to_json sorted_json = '{' + {:a => :b, :c => :d}.to_json[1..-2].split(', ').sort.join(', ') + '}' assert_equal %({\"a\": \"b\", \"c\": \"d\"}), sorted_json @@ -80,7 +80,7 @@ class TestJSONEncoding < Test::Unit::TestCase def test_hash_key_identifiers_are_always_quoted values = {0 => 0, 1 => 1, :_ => :_, "$" => "$", "a" => "a", :A => :A, :A0 => :A0, "A0B" => "A0B"} - assert_equal %w( "$" "A" "A0" "A0B" "_" "a" 0 1 ), object_keys(values.to_json) + assert_equal %w( "$" "A" "A0" "A0B" "_" "a" "0" "1" ).sort, object_keys(values.to_json) end def test_hash_should_allow_key_filtering_with_only diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 0e2aa4543d..fb76ca1ab6 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -98,7 +98,8 @@ class OrderedHashTest < Test::Unit::TestCase end def test_delete_if - (copy = @ordered_hash.dup).delete('pink') + copy = @ordered_hash.dup + copy.delete('pink') assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' } assert !@ordered_hash.keys.include?('pink') end @@ -115,6 +116,7 @@ class OrderedHashTest < Test::Unit::TestCase new_ordered_hash = @ordered_hash.reject { |k, _| k == 'pink' } assert_equal copy, @ordered_hash assert !new_ordered_hash.keys.include?('pink') + assert @ordered_hash.keys.include?('pink') end def test_clear @@ -140,4 +142,10 @@ class OrderedHashTest < Test::Unit::TestCase assert_equal [@keys.first, @values.first], pair assert !@ordered_hash.keys.include?(pair.first) end + + def test_keys + original = @ordered_hash.keys.dup + @ordered_hash.keys.pop + assert_equal original, @ordered_hash.keys + end end
\ No newline at end of file diff --git a/railties/CHANGELOG b/railties/CHANGELOG index b313f082a3..b36f57f75d 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,11 @@ *2.3.0 [Edge]* +* Remove script/performance/profiler in favour of performance integration tests. [Pratik Naik] + + To continue using script/performance/profiler, install the request_profiler plugin : + + script/plugin install git://github.com/rails/request_profiler.git + * Add a rake task to apply a template to an existing application : rake rails:template LOCATION=~/template.rb [Pratik Naik] * Add "-m/--template" option to Rails generator to apply a template to the generated application. [Jeremy McAnally] diff --git a/railties/MIT-LICENSE b/railties/MIT-LICENSE index 93be57f683..e6df48772f 100644 --- a/railties/MIT-LICENSE +++ b/railties/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2008 David Heinemeier Hansson +Copyright (c) 2004-2009 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/railties/bin/performance/request b/railties/bin/performance/request deleted file mode 100755 index ae3f38c74b..0000000000 --- a/railties/bin/performance/request +++ /dev/null @@ -1,3 +0,0 @@ -#!/usr/bin/env ruby -require File.dirname(__FILE__) + '/../../config/boot' -require 'commands/performance/request' diff --git a/railties/lib/commands/performance/request.rb b/railties/lib/commands/performance/request.rb deleted file mode 100755 index 1773886487..0000000000 --- a/railties/lib/commands/performance/request.rb +++ /dev/null @@ -1,6 +0,0 @@ -#!/usr/bin/env ruby -require 'config/environment' -require 'application' -require 'action_controller/request_profiler' - -ActionController::RequestProfiler.run(ARGV) diff --git a/railties/lib/dispatcher.rb b/railties/lib/dispatcher.rb index 0bad45d9d8..9f8b59aa3d 100644 --- a/railties/lib/dispatcher.rb +++ b/railties/lib/dispatcher.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2008 David Heinemeier Hansson +# Copyright (c) 2004-2009 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 824d1d6096..f6b8899d58 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -414,6 +414,11 @@ Run `rake gems:install` to install the missing gems. def initialize_cache unless defined?(RAILS_CACHE) silence_warnings { Object.const_set "RAILS_CACHE", ActiveSupport::Cache.lookup_store(configuration.cache_store) } + + if RAILS_CACHE.respond_to?(:middleware) + # Insert middleware to setup and teardown local cache for each request + configuration.middleware.insert_after(:"ActionController::Failsafe", RAILS_CACHE.middleware) + end end end diff --git a/railties/lib/rails_generator/generators/applications/app/app_generator.rb b/railties/lib/rails_generator/generators/applications/app/app_generator.rb index 795a0d7653..2c31d89538 100644 --- a/railties/lib/rails_generator/generators/applications/app/app_generator.rb +++ b/railties/lib/rails_generator/generators/applications/app/app_generator.rb @@ -151,7 +151,7 @@ class AppGenerator < Rails::Generator::Base def create_script_files(m) %w( about console dbconsole destroy generate runner server plugin - performance/benchmarker performance/profiler performance/request + performance/benchmarker performance/profiler ).each do |file| m.file "bin/#{file}", "script/#{file}", { :chmod => 0755, |