diff options
73 files changed, 1146 insertions, 193 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index db4589ee8f..b77409b64b 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -479,7 +479,7 @@ module ActionMailer #:nodoc: ) end unless @parts.empty? - @content_type = "multipart/alternative" + @content_type = "multipart/alternative" if @content_type !~ /^multipart/ @parts = sort_parts(@parts, @implicit_parts_order) end end diff --git a/actionmailer/test/mail_layout_test.rb b/actionmailer/test/mail_layout_test.rb index c185bd5acd..50901f52ec 100644 --- a/actionmailer/test/mail_layout_test.rb +++ b/actionmailer/test/mail_layout_test.rb @@ -21,10 +21,12 @@ class AutoLayoutMailer < ActionMailer::Base body render(:inline => "Hello, <%= @world %>", :layout => false, :body => { :world => "Earth" }) end - def multipart(recipient) + def multipart(recipient, type = nil) recipients recipient subject "You have a mail" from "tester@example.com" + + content_type(type) if type end end @@ -64,6 +66,19 @@ class LayoutMailerTest < Test::Unit::TestCase def test_should_pickup_multipart_layout mail = AutoLayoutMailer.create_multipart(@recipient) + assert_equal "multipart/alternative", mail.content_type + assert_equal 2, mail.parts.size + + assert_equal 'text/plain', mail.parts.first.content_type + assert_equal "text/plain layout - text/plain multipart", mail.parts.first.body + + assert_equal 'text/html', mail.parts.last.content_type + assert_equal "Hello from layout text/html multipart", mail.parts.last.body + end + + def test_should_pickup_multipartmixed_layout + mail = AutoLayoutMailer.create_multipart(@recipient, "multipart/mixed") + assert_equal "multipart/mixed", mail.content_type assert_equal 2, mail.parts.size assert_equal 'text/plain', mail.parts.first.content_type @@ -73,6 +88,19 @@ class LayoutMailerTest < Test::Unit::TestCase assert_equal "Hello from layout text/html multipart", mail.parts.last.body end + def test_should_fix_multipart_layout + mail = AutoLayoutMailer.create_multipart(@recipient, "text/plain") + assert_equal "multipart/alternative", mail.content_type + assert_equal 2, mail.parts.size + + assert_equal 'text/plain', mail.parts.first.content_type + assert_equal "text/plain layout - text/plain multipart", mail.parts.first.body + + assert_equal 'text/html', mail.parts.last.content_type + assert_equal "Hello from layout text/html multipart", mail.parts.last.body + end + + def test_should_pickup_layout_given_to_render mail = AutoLayoutMailer.create_spam(@recipient) assert_equal "Spammer layout Hello, Earth", mail.body.strip diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index c4e4211c42..90232d8c2d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -6,6 +6,8 @@ * Fixed that passing a custom form builder would be forwarded to nested fields_for calls #2023 [Eloy Duran/Nate Wiger] +* Form option helpers now support disabled option tags and the use of lambdas for selecting/disabling option tags from collections #837 [Tekin] + * Added partial scoping to TranslationHelper#translate, so if you call translate(".foo") from the people/index.html.erb template, you'll actually be calling I18n.translate("people.index.foo") [DHH] * Fix a syntax error in current_page?() that was prevent matches against URL's with multiple query parameters #1385, #1868 [chris finne/Andrew White] diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 0d673c617d..2c0c28b755 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -81,7 +81,6 @@ spec = Gem::Specification.new do |s| s.requirements << 'none' s.add_dependency('activesupport', '= 2.3.1' + PKG_BUILD) - s.add_dependency('rack', '>= 0.9.0') s.require_path = 'lib' s.autorequire = 'action_controller' diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index ca826e7bfc..d03f4cb231 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -31,7 +31,12 @@ rescue LoadError end end -require 'action_controller/vendor/rack-1.0/rack' +begin + gem 'rack', '~> 1.0.0' + require 'rack' +rescue Gem::LoadError + require 'action_controller/vendor/rack-1.0/rack' +end module ActionController # TODO: Review explicit to see if they will automatically be handled by diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 34e1c3527f..87b5029e57 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -129,24 +129,23 @@ module ActionController #:nodoc: attr_reader :path, :extension class << self - def path_for(controller, options, infer_extension=true) + def path_for(controller, options, infer_extension = true) new(controller, options, infer_extension).path end end # When true, infer_extension will look up the cache path extension from the request's path & format. - # This is desirable when reading and writing the cache, but not when expiring the cache - expire_action should expire the same files regardless of the request format. - def initialize(controller, options = {}, infer_extension=true) - if infer_extension and options.is_a? Hash - request_extension = extract_extension(controller.request) - options = options.reverse_merge(:format => request_extension) + # This is desirable when reading and writing the cache, but not when expiring the cache - + # expire_action should expire the same files regardless of the request format. + def initialize(controller, options = {}, infer_extension = true) + if infer_extension + extract_extension(controller.request) + options = options.reverse_merge(:format => @extension) if options.is_a?(Hash) end + path = controller.url_for(options).split('://').last normalize!(path) - if infer_extension - @extension = request_extension - add_extension!(path, @extension) - end + add_extension!(path, @extension) @path = URI.unescape(path) end @@ -162,13 +161,7 @@ module ActionController #:nodoc: def extract_extension(request) # Don't want just what comes after the last '.' to accommodate multi part extensions # such as tar.gz. - extension = request.path[/^[^.]+\.(.+)$/, 1] - - # If there's no extension in the path, check request.format - if extension.nil? - extension = request.cache_format - end - extension + @extension = request.path[/^[^.]+\.(.+)$/, 1] || request.cache_format end end end diff --git a/actionpack/lib/action_controller/http_authentication.rb b/actionpack/lib/action_controller/http_authentication.rb index 2ccbc22420..b6b5267c66 100644 --- a/actionpack/lib/action_controller/http_authentication.rb +++ b/actionpack/lib/action_controller/http_authentication.rb @@ -68,8 +68,11 @@ module ActionController # # Simple Digest example: # + # require 'digest/md5' # class PostsController < ApplicationController - # USERS = {"dhh" => "secret"} + # REALM = "SuperSecret" + # USERS = {"dhh" => "secret", #plain text password + # "dap" => Digest:MD5::hexdigest(["dap",REALM,"secret"].join(":")) #ha1 digest password # # before_filter :authenticate, :except => [:index] # @@ -83,14 +86,18 @@ module ActionController # # private # def authenticate - # authenticate_or_request_with_http_digest(realm) do |username| + # authenticate_or_request_with_http_digest(REALM) do |username| # USERS[username] # end # end # end # - # NOTE: The +authenticate_or_request_with_http_digest+ 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. + # NOTE: The +authenticate_or_request_with_http_digest+ block must return the user's password or the ha1 digest hash so the framework can appropriately + # hash to check the user's credentials. Returning +nil+ will cause authentication to fail. + # Storing the ha1 hash: MD5(username:realm:password), is better than storing a plain password. If + # the password file or database is compromised, the attacker would be able to use the ha1 hash to + # authenticate as the user at this +realm+, but would not have the user's password to try using at + # other sites. # # On shared hosts, Apache sometimes doesn't pass authentication headers to # FCGI instances. If your environment matches this description and you cannot @@ -177,26 +184,37 @@ module ActionController end # Raises error unless the request credentials response value matches the expected value. + # First try the password as a ha1 digest password. If this fails, then try it as a plain + # text password. def validate_digest_response(request, realm, &password_procedure) credentials = decode_credentials_header(request) valid_nonce = validate_nonce(request, credentials[:nonce]) - if valid_nonce && realm == credentials[:realm] && opaque(request.session.session_id) == credentials[:opaque] + if valid_nonce && realm == credentials[:realm] && opaque == credentials[:opaque] password = password_procedure.call(credentials[:username]) - expected = expected_response(request.env['REQUEST_METHOD'], credentials[:uri], credentials, password) - expected == credentials[:response] + + [true, false].any? do |password_is_ha1| + expected = expected_response(request.env['REQUEST_METHOD'], request.env['REQUEST_URI'], credentials, password, password_is_ha1) + expected == credentials[:response] + end end 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(':')) + # Optional parameter +password_is_ha1+ is set to +true+ by default, since best practice is to store ha1 digest instead + # of a plain-text password. + def expected_response(http_method, uri, credentials, password, password_is_ha1=true) + ha1 = password_is_ha1 ? password : ha1(credentials, password) 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) + def ha1(credentials, password) + ::Digest::MD5.hexdigest([credentials[:username], credentials[:realm], password].join(':')) + end + + def encode_credentials(http_method, credentials, password, password_is_ha1) + credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password, password_is_ha1) "Digest " + credentials.sort_by {|x| x[0].to_s }.inject([]) {|a, v| a << "#{v[0]}='#{v[1]}'" }.join(', ') end @@ -213,8 +231,7 @@ module ActionController end def authentication_header(controller, realm) - session_id = controller.request.session.session_id - controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce(session_id)}", opaque="#{opaque(session_id)}") + controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce}", opaque="#{opaque}") end def authentication_request(controller, realm, message = nil) @@ -252,23 +269,36 @@ module ActionController # 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(session_id, time = Time.now) + # The nonce is opaque to the client. Composed of Time, and hash of Time with secret + # key from the Rails session secret generated upon creation of project. Ensures + # the time cannot be modifed by client. + def nonce(time = Time.now) t = time.to_i - hashed = [t, session_id] + hashed = [t, secret_key] digest = ::Digest::MD5.hexdigest(hashed.join(":")) Base64.encode64("#{t}:#{digest}").gsub("\n", '') end - def validate_nonce(request, value) + # Might want a shorter timeout depending on whether the request + # is a PUT or POST, and if client is browser or web service. + # Can be much shorter if the Stale directive is implemented. This would + # allow a user to use new nonce without prompting user again for their + # username and password. + def validate_nonce(request, value, seconds_to_timeout=5*60) t = Base64.decode64(value).split(":").first.to_i - nonce(request.session.session_id, t) == value && (t - Time.now.to_i).abs <= 10 * 60 + nonce(t) == value && (t - Time.now.to_i).abs <= seconds_to_timeout end - # Opaque based on digest of session_id - def opaque(session_id) - Base64.encode64(::Digest::MD5::hexdigest(session_id)).gsub("\n", '') + # Opaque based on random generation - but changing each request? + def opaque() + ::Digest::MD5.hexdigest(secret_key) end + + # Set in /initializers/session_store.rb, and loaded even if sessions are not in use. + def secret_key + ActionController::Base.session_options[:secret] + end + end end end diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 2cabab9ec8..ef223f157c 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -442,6 +442,7 @@ EOM end def reset_session + @env['rack.session.options'].delete(:id) @env['rack.session'] = {} end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb index 6c03b55552..e3e20ed2a3 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack.rb @@ -31,6 +31,7 @@ module Rack autoload :CommonLogger, "rack/commonlogger" autoload :ConditionalGet, "rack/conditionalget" autoload :ContentLength, "rack/content_length" + autoload :ContentType, "rack/content_type" autoload :File, "rack/file" autoload :Deflater, "rack/deflater" autoload :Directory, "rack/directory" diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb index 8489c9b9c4..214df6299e 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/abstract/handler.rb @@ -8,8 +8,8 @@ module Rack attr_accessor :realm - def initialize(app, &authenticator) - @app, @authenticator = app, authenticator + def initialize(app, realm=nil, &authenticator) + @app, @realm, @authenticator = app, realm, authenticator end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb index 6d2bd29c2e..e579dc9632 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/md5.rb @@ -21,7 +21,7 @@ module Rack attr_writer :passwords_hashed - def initialize(app) + def initialize(*args) super @passwords_hashed = nil end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb index a40f57b7f1..a8aa3bf996 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/auth/digest/request.rb @@ -8,7 +8,7 @@ module Rack class Request < Auth::AbstractRequest def method - @env['REQUEST_METHOD'] + @env['rack.methodoverride.original_method'] || @env['REQUEST_METHOD'] end def digest? diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb index 25994d5a44..295235e56a 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/builder.rb @@ -34,11 +34,7 @@ module Rack end def use(middleware, *args, &block) - @ins << if block_given? - lambda { |app| middleware.new(app, *args, &block) } - else - lambda { |app| middleware.new(app, *args) } - end + @ins << lambda { |app| middleware.new(app, *args, &block) } end def run(app) diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb index bce22a32c5..1e56d43853 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_length.rb @@ -3,21 +3,23 @@ require 'rack/utils' module Rack # Sets the Content-Length header on responses with fixed-length bodies. class ContentLength + include Rack::Utils + def initialize(app) @app = app end def call(env) status, headers, body = @app.call(env) - headers = Utils::HeaderHash.new(headers) + headers = HeaderHash.new(headers) - if !Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status) && + if !STATUS_WITH_NO_ENTITY_BODY.include?(status) && !headers['Content-Length'] && !headers['Transfer-Encoding'] && (body.respond_to?(:to_ary) || body.respond_to?(:to_str)) body = [body] if body.respond_to?(:to_str) # rack 0.4 compat - length = body.to_ary.inject(0) { |len, part| len + part.length } + length = body.to_ary.inject(0) { |len, part| len + bytesize(part) } headers['Content-Length'] = length.to_s end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb new file mode 100644 index 0000000000..0c1e1ca3e1 --- /dev/null +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/content_type.rb @@ -0,0 +1,23 @@ +require 'rack/utils' + +module Rack + + # Sets the Content-Type header on responses which don't have one. + # + # Builder Usage: + # use Rack::ContentType, "text/plain" + # + # When no content type argument is provided, "text/html" is assumed. + class ContentType + def initialize(app, content_type = "text/html") + @app, @content_type = app, content_type + end + + def call(env) + status, headers, body = @app.call(env) + headers = Utils::HeaderHash.new(headers) + headers['Content-Type'] ||= @content_type + [status, headers.to_hash, body] + end + end +end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb index 3e66680092..a42b7477ae 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/deflater.rb @@ -36,12 +36,12 @@ module Rack mtime = headers.key?("Last-Modified") ? Time.httpdate(headers["Last-Modified"]) : Time.now body = self.class.gzip(body, mtime) - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) headers = headers.merge("Content-Encoding" => "gzip", "Content-Length" => size.to_s) [status, headers, [body]] when "deflate" body = self.class.deflate(body) - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) headers = headers.merge("Content-Encoding" => "deflate", "Content-Length" => size.to_s) [status, headers, [body]] when "identity" diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb index 56ee5e7b60..acdd3029d3 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/directory.rb @@ -70,7 +70,7 @@ table { width:100%%; } return unless @path_info.include? ".." body = "Forbidden\n" - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) return [403, {"Content-Type" => "text/plain","Content-Length" => size.to_s}, [body]] end @@ -89,6 +89,8 @@ table { width:100%%; } type = stat.directory? ? 'directory' : Mime.mime_type(ext) size = stat.directory? ? '-' : filesize_format(size) mtime = stat.mtime.httpdate + url << '/' if stat.directory? + basename << '/' if stat.directory? @files << [ url, basename, size, type, mtime ] end @@ -120,7 +122,7 @@ table { width:100%%; } def entity_not_found body = "Entity not found: #{@path_info}\n" - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) return [404, {"Content-Type" => "text/plain", "Content-Length" => size.to_s}, [body]] end diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb index 1f850fc77b..dfc79c204b 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/lsws.rb @@ -13,7 +13,7 @@ module Rack env.delete "HTTP_CONTENT_LENGTH" env["SCRIPT_NAME"] = "" if env["SCRIPT_NAME"] == "/" env.update({"rack.version" => [0,1], - "rack.input" => $stdin, + "rack.input" => StringIO.new($stdin.read.to_s), "rack.errors" => $stderr, "rack.multithread" => false, "rack.multiprocess" => true, diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb index 40be79de13..138aae0ee9 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/handler/webrick.rb @@ -35,7 +35,12 @@ module Rack env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"] env["QUERY_STRING"] ||= "" env["REQUEST_PATH"] ||= "/" - env.delete "PATH_INFO" if env["PATH_INFO"] == "" + if env["PATH_INFO"] == "" + env.delete "PATH_INFO" + else + path, n = req.request_uri.path, env["SCRIPT_NAME"].length + env["PATH_INFO"] = path[n, path.length-n] + end status, headers, body = @app.call(env) begin diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb index 7eb05437f0..ec4dac96f8 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/lint.rb @@ -88,7 +88,9 @@ module Rack ## within the application. This may be an ## empty string, if the request URL targets ## the application root and does not have a - ## trailing slash. + ## trailing slash. This information should be + ## decoded by the server if it comes from a + ## URL. ## <tt>QUERY_STRING</tt>:: The portion of the request URL that ## follows the <tt>?</tt>, if any. May be @@ -401,7 +403,7 @@ module Rack break end - bytes += (part.respond_to?(:bytesize) ? part.bytesize : part.size) + bytes += Rack::Utils.bytesize(part) } if env["REQUEST_METHOD"] == "HEAD" diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb index a593110139..caf60d5b19 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/response.rb @@ -16,6 +16,8 @@ module Rack # Your application's +call+ should end returning Response#finish. class Response + attr_accessor :length + def initialize(body=[], status=200, header={}, &block) @status = status @header = Utils::HeaderHash.new({"Content-Type" => "text/html"}. diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb index 5f13404dce..28258c7c89 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/showstatus.rb @@ -27,7 +27,7 @@ module Rack message = Rack::Utils::HTTP_STATUS_CODES[status.to_i] || status.to_s detail = env["rack.showstatus.detail"] || message body = @template.result(binding) - size = body.respond_to?(:bytesize) ? body.bytesize : body.size + size = Rack::Utils.bytesize(body) [status, headers.merge("Content-Type" => "text/html", "Content-Length" => size.to_s), [body]] else [status, headers, body] diff --git a/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb b/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb index f352cb6783..e86d4ccdcd 100644 --- a/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb +++ b/actionpack/lib/action_controller/vendor/rack-1.0/rack/utils.rb @@ -144,6 +144,19 @@ module Rack end module_function :select_best_encoding + # Return the bytesize of String; uses String#length under Ruby 1.8 and + # String#bytesize under 1.9. + if ''.respond_to?(:bytesize) + def bytesize(string) + string.bytesize + end + else + def bytesize(string) + string.size + end + end + module_function :bytesize + # Context allows the use of a compatible middleware at different points # in a request handling stack. A compatible middleware must define # #context which should take the arguments env and app. The first of which diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index feeddba78d..48bf4717ad 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -536,9 +536,10 @@ module ActionView text.gsub(AUTO_LINK_RE) do href = $& punctuation = '' - # detect already linked URLs - if $` =~ /<a\s[^>]*href="$/ - # do not change string; URL is already linked + left, right = $`, $' + # detect already linked URLs and URLs in the middle of a tag + if left =~ /<[^>]+$/ && right =~ /^[^>]*>/ + # do not change string; URL is alreay linked href else # don't include trailing punctuation character as part of the URL diff --git a/actionpack/test/activerecord/active_record_store_test.rb b/actionpack/test/activerecord/active_record_store_test.rb index 7998f9c22f..c98892edc1 100644 --- a/actionpack/test/activerecord/active_record_store_test.rb +++ b/actionpack/test/activerecord/active_record_store_test.rb @@ -21,8 +21,15 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest render :text => "foo: #{session[:foo].inspect}" end + def get_session_id + session[:foo] + render :text => "#{request.session_options[:id]}" + end + def call_reset_session + session[:bar] reset_session + session[:bar] = "baz" head :ok end @@ -71,6 +78,7 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest get '/set_session_value' assert_response :success assert cookies['_session_id'] + session_id = cookies['_session_id'] get '/call_reset_session' assert_response :success @@ -79,6 +87,23 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest get '/get_session_value' assert_response :success assert_equal 'foo: nil', response.body + + get '/get_session_id' + assert_response :success + assert_not_equal session_id, response.body + end + end + + def test_getting_session_id + with_test_route_set do + get '/set_session_value' + assert_response :success + assert cookies['_session_id'] + session_id = cookies['_session_id'] + + get '/get_session_id' + assert_response :success + assert_equal session_id, response.body end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 9af1ccc740..86dafd9221 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -428,6 +428,20 @@ class ActionCacheTest < ActionController::TestCase assert_equal 'application/xml', @response.content_type end + def test_correct_content_type_is_returned_for_cache_hit_on_action_with_string_key + # run it twice to cache it the first time + get :show, :format => 'xml' + get :show, :format => 'xml' + assert_equal 'application/xml', @response.content_type + end + + def test_correct_content_type_is_returned_for_cache_hit_on_action_with_string_key_from_proc + # run it twice to cache it the first time + get :edit, :id => 1, :format => 'xml' + get :edit, :id => 1, :format => 'xml' + assert_equal 'application/xml', @response.content_type + end + def test_empty_path_is_normalized @mock_controller.mock_url_for = 'http://example.org/' @mock_controller.mock_path = '/' diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 4913e7633b..00789eea38 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -5,7 +5,8 @@ class HttpDigestAuthenticationTest < ActionController::TestCase before_filter :authenticate, :only => :index before_filter :authenticate_with_request, :only => :display - USERS = { 'lifo' => 'world', 'pretty' => 'please' } + USERS = { 'lifo' => 'world', 'pretty' => 'please', + 'dhh' => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":"))} def index render :text => "Hello Secret" @@ -107,8 +108,42 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert_equal 'Definitely Maybe', @response.body end - test "authentication request with relative URI" do - @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "/", :username => 'pretty', :password => 'please') + test "authentication request with valid credential and nil session" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + + # session_id = "" in functional test, but is +nil+ in real life + @request.session.session_id = nil + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end + + test "authentication request with request-uri that doesn't match credentials digest-uri" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + @request.env['REQUEST_URI'] = "/http_digest_authentication_test/dummy_digest/altered/uri" + get :display + + assert_response :unauthorized + assert_equal "Authentication Failed", @response.body + end + + test "authentication request with absolute uri" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "http://test.host/http_digest_authentication_test/dummy_digest/display", + :username => 'pretty', :password => 'please') + @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest/display" + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end + + test "authentication request with password stored as ha1 digest hash" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'dhh', + :password => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":")), + :password_is_ha1 => true) get :display assert_response :success @@ -119,18 +154,22 @@ class HttpDigestAuthenticationTest < ActionController::TestCase private def encode_credentials(options) - options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b") + options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b", :password_is_ha1 => false) password = options.delete(:password) - # Perform unautheticated get to retrieve digest parameters to use on subsequent request + # Set in /initializers/session_store.rb. Used as secret in generating nonce + # to prevent tampering of timestamp + ActionController::Base.session_options[:secret] = "session_options_secret" + + # Perform unauthenticated GET to retrieve digest parameters to use on subsequent request get :index assert_response :unauthorized credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) - credentials.reverse_merge!(:uri => "http://#{@request.host}#{@request.env['REQUEST_URI']}") - ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password) + credentials.reverse_merge!(:uri => "#{@request.env['REQUEST_URI']}") + ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password, options[:password_is_ha1]) end def decode_credentials(header) diff --git a/actionpack/test/controller/session/mem_cache_store_test.rb b/actionpack/test/controller/session/mem_cache_store_test.rb index c3a6c8ce45..2f80a3c7c2 100644 --- a/actionpack/test/controller/session/mem_cache_store_test.rb +++ b/actionpack/test/controller/session/mem_cache_store_test.rb @@ -17,11 +17,14 @@ class MemCacheStoreTest < ActionController::IntegrationTest end def get_session_id - render :text => "foo: #{session[:foo].inspect}; id: #{request.session_options[:id]}" + session[:foo] + render :text => "#{request.session_options[:id]}" end def call_reset_session + session[:bar] reset_session + session[:bar] = "baz" head :ok end @@ -58,47 +61,52 @@ class MemCacheStoreTest < ActionController::IntegrationTest end end - def test_getting_session_id + def test_setting_session_value_after_session_reset with_test_route_set do get '/set_session_value' assert_response :success assert cookies['_session_id'] session_id = cookies['_session_id'] - get '/get_session_id' + get '/call_reset_session' assert_response :success - assert_equal "foo: \"bar\"; id: #{session_id}", response.body - end - end + assert_not_equal [], headers['Set-Cookie'] - def test_prevents_session_fixation - with_test_route_set do get '/get_session_value' assert_response :success assert_equal 'foo: nil', response.body - session_id = cookies['_session_id'] - - reset! - get '/set_session_value', :_session_id => session_id + get '/get_session_id' assert_response :success - assert_equal nil, cookies['_session_id'] + assert_not_equal session_id, response.body end end - def test_setting_session_value_after_session_reset + def test_getting_session_id with_test_route_set do get '/set_session_value' assert_response :success assert cookies['_session_id'] + session_id = cookies['_session_id'] - get '/call_reset_session' + get '/get_session_id' assert_response :success - assert_not_equal [], headers['Set-Cookie'] + assert_equal session_id, response.body + end + end + def test_prevents_session_fixation + with_test_route_set do get '/get_session_value' assert_response :success assert_equal 'foo: nil', response.body + session_id = cookies['_session_id'] + + reset! + + get '/set_session_value', :_session_id => session_id + assert_response :success + assert_equal nil, cookies['_session_id'] end end rescue LoadError, RuntimeError diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 564845779f..a370f1458f 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -375,6 +375,12 @@ class TextHelperTest < ActionView::TestCase assert_equal "{link: #{link3_result}}", auto_link("{link: #{link3_raw}}") end + def test_auto_link_in_tags + link_raw = 'http://www.rubyonrails.org/images/rails.png' + link_result = %Q(<img src="#{link_raw}" />) + assert_equal link_result, auto_link(link_result) + end + def test_auto_link_at_eol url1 = "http://api.rubyonrails.com/Foo.html" url2 = "http://www.ruby-doc.org/core/Bar.html" diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 404481ea38..6d2f597004 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,10 +1,9 @@ *2.3.1 [RC2] (March 5, 2009)* -* Added ActiveRecord::Base.each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] +* Added ActiveRecord::Base.find_each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] * Added that ActiveRecord::Base.exists? can be called with no arguments #1817 [Scott Taylor] - *2.3.0 [RC1] (February 1st, 2009)* * Add Support for updating deeply nested models from a single form. #1202 [Eloy Duran] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 301b3a3b58..6d25b36aea 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1856,9 +1856,10 @@ module ActiveRecord def construct(parent, associations, joins, row) case associations when Symbol, String - while (join = joins.shift).reflection.name.to_s != associations.to_s - raise ConfigurationError, "Not Enough Associations" if joins.empty? - end + join = joins.detect{|j| j.reflection.name.to_s == associations.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + joins.delete(join) construct_association(parent, join, row) when Array associations.each do |association| @@ -1866,7 +1867,11 @@ module ActiveRecord end when Hash associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| - association = construct_association(parent, joins.shift, row) + join = joins.detect{|j| j.reflection.name.to_s == name.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + association = construct_association(parent, join, row) + joins.delete(join) construct(association, associations[name], joins, row) if association end else diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0fefec1216..ad375be184 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -60,7 +60,7 @@ module ActiveRecord @reflection.klass.find(*args) end end - + # Fetches the first one using SQL if possible. def first(*args) if fetch_first_or_last_using_find?(args) @@ -186,7 +186,6 @@ module ActiveRecord end end - # Removes +records+ from this association calling +before_remove+ and # +after_remove+ callbacks. # @@ -195,20 +194,23 @@ module ActiveRecord # are actually removed from the database, that depends precisely on # +delete_records+. They are in any case removed from the collection. def delete(*records) - records = flatten_deeper(records) - records.each { |record| raise_on_type_mismatch(record) } - - transaction do - records.each { |record| callback(:before_remove, record) } - - old_records = records.reject {|r| r.new_record? } + remove_records(records) do |records, old_records| delete_records(old_records) if old_records.any? - - records.each do |record| - @target.delete(record) - callback(:after_remove, record) - end + records.each { |record| @target.delete(record) } + end + end + + # Destroy +records+ and remove from this association calling +before_remove+ + # and +after_remove+ callbacks. + # + # Note this method will always remove records from database ignoring the + # +:dependent+ option. + def destroy(*records) + remove_records(records) do |records, old_records| + old_records.each { |record| record.destroy } end + + load_target end # Removes all records from this association. Returns +self+ so method calls may be chained. @@ -223,15 +225,14 @@ module ActiveRecord self end - - def destroy_all - transaction do - each { |record| record.destroy } - end + # Destory all the records from this association + def destroy_all + load_target + destroy(@target) reset_target! end - + def create(attrs = {}) if attrs.is_a?(Array) attrs.collect { |attr| create(attr) } @@ -431,6 +432,18 @@ module ActiveRecord record end + def remove_records(*records) + records = flatten_deeper(records) + records.each { |record| raise_on_type_mismatch(record) } + + transaction do + records.each { |record| callback(:before_remove, record) } + old_records = records.reject { |r| r.new_record? } + yield(records, old_records) + records.each { |record| callback(:after_remove, record) } + end + end + def callback(method, record) callbacks_for(method).each do |callback| ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 1c3d0567c1..741aa2acbe 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -283,7 +283,7 @@ module ActiveRecord if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) records.each do |record| if autosave && record.marked_for_destruction? - record.destroy + association.destroy(record) elsif @new_record_before_save || record.new_record? if autosave association.send(:insert_record, record, false, false) @@ -310,7 +310,7 @@ module ActiveRecord # This all happens inside a transaction, _if_ the Transactions module is included into # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. def save_has_one_association(reflection) - if association = association_instance_get(reflection.name) + if (association = association_instance_get(reflection.name)) && !association.target.nil? if reflection.options[:autosave] && association.marked_for_destruction? association.destroy elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || reflection.options[:autosave] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 75f84ef2ed..a5459d09da 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -416,7 +416,7 @@ module ActiveRecord #:nodoc: end @@subclasses = {} - + ## # :singleton-method: # Contains the database configuration - as is typically stored in config/database.yml - @@ -661,7 +661,6 @@ module ActiveRecord #:nodoc: connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } end - # Returns true if a record exists in the table that matches the +id+ or # conditions given, or false otherwise. The argument can take five forms: # @@ -1003,7 +1002,6 @@ module ActiveRecord #:nodoc: update_counters(id, counter_name => -1) end - # Attributes named in this macro are protected from mass-assignment, # such as <tt>new(attributes)</tt>, # <tt>update_attributes(attributes)</tt>, or @@ -1104,7 +1102,6 @@ module ActiveRecord #:nodoc: read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {}) end - # Guesses the table name (in forced lower-case) based on the name of the class in the inheritance hierarchy descending # directly from ActiveRecord::Base. So if the hierarchy looks like: Reply < Message < ActiveRecord::Base, then Message is used # to guess the table name even when called on Reply. The rules used to do the guess are handled by the Inflector class @@ -1417,7 +1414,6 @@ module ActiveRecord #:nodoc: end end - def quote_value(value, column = nil) #:nodoc: connection.quote(value,column) end @@ -1486,7 +1482,7 @@ module ActiveRecord #:nodoc: elsif match = DynamicScopeMatch.match(method_id) return true if all_attributes_exists?(match.attribute_names) end - + super end @@ -1745,7 +1741,9 @@ module ActiveRecord #:nodoc: scoped_order = scope[:order] if scope if order sql << " ORDER BY #{order}" - sql << ", #{scoped_order}" if scoped_order + if scoped_order && scoped_order != order + sql << ", #{scoped_order}" + end else sql << " ORDER BY #{scoped_order}" if scoped_order end @@ -2014,7 +2012,6 @@ module ActiveRecord #:nodoc: end end - # Defines an "attribute" method (like +inheritance_column+ or # +table_name+). A new (class) method will be created with the # given name. If a value is specified, the new method will @@ -2111,7 +2108,7 @@ module ActiveRecord #:nodoc: end # Merge scopings - if action == :merge && current_scoped_methods + if [:merge, :reverse_merge].include?(action) && current_scoped_methods method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)| case hash[method] when Hash @@ -2133,7 +2130,11 @@ module ActiveRecord #:nodoc: end end else - hash[method] = hash[method].merge(params) + if action == :reverse_merge + hash[method] = hash[method].merge(params) + else + hash[method] = params.merge(hash[method]) + end end else hash[method] = params @@ -2143,7 +2144,6 @@ module ActiveRecord #:nodoc: end self.scoped_methods << method_scoping - begin yield ensure @@ -2749,7 +2749,6 @@ module ActiveRecord #:nodoc: assign_multiparameter_attributes(multi_parameter_attributes) end - # Returns a hash of all the attributes with their names as keys and the values of the attributes as values. def attributes self.attribute_names.inject({}) do |attrs, name| diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb index 9e9c8fbbf4..03bd4f9f93 100644 --- a/activerecord/lib/active_record/batches.rb +++ b/activerecord/lib/active_record/batches.rb @@ -11,14 +11,14 @@ module ActiveRecord # # Example: # - # Person.each(:conditions => "age > 21") do |person| + # Person.find_each(:conditions => "age > 21") do |person| # person.party_all_night! # end # # Note: This method is only intended to use for batch processing of large amounts of records that wouldn't fit in # memory all at once. If you just need to loop over less than 1000 records, it's probably better just to use the # regular find methods. - def each(options = {}) + def find_each(options = {}) find_in_batches(options) do |records| records.each { |record| yield record } end @@ -49,12 +49,15 @@ module ActiveRecord raise "You can't specify a limit, it's forced to be the batch_size" if options[:limit] start = options.delete(:start).to_i + batch_size = options.delete(:batch_size) || 1000 - with_scope(:find => options.merge(:order => batch_order, :limit => options.delete(:batch_size) || 1000)) do + with_scope(:find => options.merge(:order => batch_order, :limit => batch_size)) do records = find(:all, :conditions => [ "#{table_name}.#{primary_key} >= ?", start ]) while records.any? yield records + + break if records.size < batch_size records = find(:all, :conditions => [ "#{table_name}.#{primary_key} > ?", records.last.id ]) end end diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 43411dfb55..1f3ef300f2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -104,7 +104,7 @@ module ActiveRecord end end end - + class Scope attr_reader :proxy_scope, :proxy_options, :current_scoped_methods_when_defined NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set @@ -117,6 +117,7 @@ module ActiveRecord delegate :scopes, :with_scope, :to => :proxy_scope def initialize(proxy_scope, options, &block) + options ||= {} [options[:extend]].flatten.each { |extension| extend extension } if options[:extend] extend Module.new(&block) if block_given? unless Scope === proxy_scope @@ -175,7 +176,7 @@ module ActiveRecord if scopes.include?(method) scopes[method].call(self, *args) else - with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {} do + with_scope({:find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {}}, :reverse_merge) do method = :new if method == :build if current_scoped_methods_when_defined with_scope current_scoped_methods_when_defined do diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index 4749823b94..fa75874603 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -231,16 +231,22 @@ module ActiveRecord #:nodoc: def add_associations(association, records, opts) if records.is_a?(Enumerable) tag = reformat_name(association.to_s) + type = options[:skip_types] ? {} : {:type => "array"} + if records.empty? - builder.tag!(tag, :type => :array) + builder.tag!(tag, type) else - builder.tag!(tag, :type => :array) do + builder.tag!(tag, type) do association_name = association.to_s.singularize records.each do |record| - record.to_xml opts.merge( - :root => association_name, - :type => (record.class.to_s.underscore == association_name ? nil : record.class.name) - ) + if options[:skip_types] + record_type = {} + else + record_class = (record.class.to_s.underscore == association_name) ? nil : record.class.name + record_type = {:type => record_class} + end + + record.to_xml opts.merge(:root => association_name).merge(record_type) end end end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index ff3e54712e..13a78a1890 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -326,4 +326,20 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase companies(:first_firm).send(:private_method) companies(:second_client).firm.send(:private_method) end + + def test_save_of_record_with_loaded_belongs_to + @account = companies(:first_firm).account + + assert_nothing_raised do + Account.find(@account.id).save! + Account.find(@account.id, :include => :firm).save! + end + + @account.firm.delete + + assert_nothing_raised do + Account.find(@account.id).save! + Account.find(@account.id, :include => :firm).save! + end + end end diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb index 12dec5ccd1..1b2e0fc11e 100644 --- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb +++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb @@ -1,4 +1,9 @@ require 'cases/helper' +require 'models/author' +require 'models/post' +require 'models/comment' +require 'models/category' +require 'models/categorization' module Remembered def self.included(base) @@ -99,3 +104,27 @@ class EagerLoadPolyAssocsTest < ActiveRecord::TestCase end end end + +class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase + def setup + @davey_mcdave = Author.create(:name => 'Davey McDave') + @first_post = @davey_mcdave.posts.create(:title => 'Davey Speaks', :body => 'Expressive wordage') + @first_comment = @first_post.comments.create(:body => 'Inflamatory doublespeak') + @first_categorization = @davey_mcdave.categorizations.create(:category => Category.first, :post => @first_post) + end + + def teardown + @davey_mcdave.destroy + @first_post.destroy + @first_comment.destroy + @first_categorization.destroy + end + + def test_missing_data_in_a_nested_include_should_not_cause_errors_when_constructing_objects + assert_nothing_raised do + # @davey_mcdave doesn't have any author_favorites + includes = {:posts => :comments, :categorizations => :category, :author_favorites => :favorite_author } + Author.all :include => includes, :conditions => {:authors => {:name => @davey_mcdave.name}}, :order => 'categories.name' + end + end +end
\ No newline at end of file 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 ca1772d1ca..5e8b2cadfc 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 @@ -381,6 +381,33 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_date_from_db Date.new(2004, 10, 10), Developer.find(1).projects.first.joined_on.to_date end + def test_destroying + david = Developer.find(1) + active_record = Project.find(1) + david.projects.reload + assert_equal 2, david.projects.size + assert_equal 3, active_record.developers.size + + assert_difference "Project.count", -1 do + david.projects.destroy(active_record) + end + + assert_equal 1, david.reload.projects.size + assert_equal 1, david.projects(true).size + end + + def test_destroying_array + david = Developer.find(1) + david.projects.reload + + assert_difference "Project.count", -Project.count do + david.projects.destroy(Project.find(:all)) + end + + assert_equal 0, david.reload.projects.size + assert_equal 0, david.projects(true).size + end + def test_destroy_all david = Developer.find(1) david.projects.reload diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index b7fa9d9d7c..30edf79a26 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -219,6 +219,45 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, firm.clients.find(:all, :conditions => "name = 'Summit'").length end + def test_find_each + firm = companies(:first_firm) + + assert ! firm.clients.loaded? + + assert_queries(3) do + firm.clients.find_each(:batch_size => 1) {|c| assert_equal firm.id, c.firm_id } + end + + assert ! firm.clients.loaded? + end + + def test_find_each_with_conditions + firm = companies(:first_firm) + + assert_queries(2) do + firm.clients.find_each(:batch_size => 1, :conditions => {:name => "Microsoft"}) do |c| + assert_equal firm.id, c.firm_id + assert_equal "Microsoft", c.name + end + end + + assert ! firm.clients.loaded? + end + + def test_find_in_batches + firm = companies(:first_firm) + + assert ! firm.clients.loaded? + + assert_queries(2) do + firm.clients.find_in_batches(:batch_size => 2) do |clients| + clients.each {|c| assert_equal firm.id, c.firm_id } + end + end + + assert ! firm.clients.loaded? + end + def test_find_all_sanitized firm = Firm.find(:first) summit = firm.clients.find(:all, :conditions => "name = 'Summit'") @@ -641,6 +680,30 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(Project.find(1).developers) } end + def test_destroying + force_signal37_to_load_all_clients_of_firm + + assert_difference "Client.count", -1 do + companies(:first_firm).clients_of_firm.destroy(companies(:first_firm).clients_of_firm.first) + end + + assert_equal 0, companies(:first_firm).reload.clients_of_firm.size + assert_equal 0, companies(:first_firm).clients_of_firm(true).size + end + + def test_destroying_a_collection + force_signal37_to_load_all_clients_of_firm + companies(:first_firm).clients_of_firm.create("name" => "Another Client") + assert_equal 2, companies(:first_firm).clients_of_firm.size + + assert_difference "Client.count", -2 do + companies(:first_firm).clients_of_firm.destroy([companies(:first_firm).clients_of_firm[0], companies(:first_firm).clients_of_firm[1]]) + end + + assert_equal 0, companies(:first_firm).reload.clients_of_firm.size + assert_equal 0, companies(:first_firm).clients_of_firm(true).size + end + def test_destroy_all force_signal37_to_load_all_clients_of_firm assert !companies(:first_firm).clients_of_firm.empty?, "37signals has clients after load" diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index c3ad0ee6de..97efca7891 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -92,6 +92,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).reload.people(true).empty? end + def test_destroy_association + assert_difference "Person.count", -1 do + posts(:welcome).people.destroy(people(:michael)) + end + + assert posts(:welcome).reload.people.empty? + assert posts(:welcome).people(true).empty? + end + + def test_destroy_all + assert_difference "Person.count", -1 do + posts(:welcome).people.destroy_all + end + + assert posts(:welcome).reload.people.empty? + assert posts(:welcome).people(true).empty? + end + def test_replace_association assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 4947f1543c..1ddb3f49bf 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -290,4 +290,20 @@ class HasOneAssociationsTest < ActiveRecord::TestCase companies(:first_firm).account.send(:private_method) end + def test_save_of_record_with_loaded_has_one + @firm = companies(:first_firm) + assert_not_nil @firm.account + + assert_nothing_raised do + Firm.find(@firm.id).save! + Firm.find(@firm.id, :include => :account).save! + end + + @firm.account.destroy + + assert_nothing_raised do + Firm.find(@firm.id).save! + Firm.find(@firm.id, :include => :account).save! + end + end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index f96b55513e..12c598751b 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -173,4 +173,20 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_not_nil assert_no_queries { @new_detail.member_type } end + def test_save_of_record_with_loaded_has_one_through + @club = @member.club + assert_not_nil @club.sponsored_member + + assert_nothing_raised do + Club.find(@club.id).save! + Club.find(@club.id, :include => :sponsored_member).save! + end + + @club.sponsor.destroy + + assert_nothing_raised do + Club.find(@club.id).save! + Club.find(@club.id, :include => :sponsored_member).save! + end + end end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index b179bd827a..436f50d395 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -556,6 +556,41 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase assert_raise(RuntimeError) { assert !@pirate.save } assert_equal before, @pirate.reload.send(association_name) end + + # Add and remove callbacks tests for association collections. + %w{ method proc }.each do |callback_type| + define_method("test_should_run_add_callback_#{callback_type}s_for_#{association_name}") do + association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks" + + pirate = Pirate.new(:catchphrase => "Arr") + pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed") + + expected = [ + "before_adding_#{callback_type}_#{association_name.singularize}_<new>", + "after_adding_#{callback_type}_#{association_name.singularize}_<new>" + ] + + assert_equal expected, pirate.ship_log + end + + define_method("test_should_run_remove_callback_#{callback_type}s_for_#{association_name}") do + association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks" + + @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed") + @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction } + child_id = @pirate.send(association_name_with_callbacks).first.id + + @pirate.ship_log.clear + @pirate.save + + expected = [ + "before_removing_#{callback_type}_#{association_name.singularize}_#{child_id}", + "after_removing_#{callback_type}_#{association_name.singularize}_#{child_id}" + ] + + assert_equal expected, @pirate.ship_log + end + end end end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 108d679108..5009a90846 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -11,7 +11,7 @@ class EachTest < ActiveRecord::TestCase def test_each_should_excecute_one_query_per_batch assert_queries(Post.count + 1) do - Post.each(:batch_size => 1) do |post| + Post.find_each(:batch_size => 1) do |post| assert_kind_of Post, post end end @@ -19,13 +19,13 @@ class EachTest < ActiveRecord::TestCase def test_each_should_raise_if_the_order_is_set assert_raise(RuntimeError) do - Post.each(:order => "title") { |post| post } + Post.find_each(:order => "title") { |post| post } end end def test_each_should_raise_if_the_limit_is_set assert_raise(RuntimeError) do - Post.each(:limit => 1) { |post| post } + Post.find_each(:limit => 1) { |post| post } end end @@ -46,4 +46,16 @@ class EachTest < ActiveRecord::TestCase end end end + + def test_find_in_batches_shouldnt_excute_query_unless_needed + post_count = Post.count + + assert_queries(2) do + Post.find_in_batches(:batch_size => post_count) {|batch| assert_kind_of Array, batch } + end + + assert_queries(1) do + Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch } + end + end end
\ No newline at end of file diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index c676c1c72b..3c34cdeade 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -262,6 +262,15 @@ class NestedScopingTest < ActiveRecord::TestCase end end + def test_merge_inner_scope_has_priority + Developer.with_scope(:find => { :limit => 5 }) do + Developer.with_scope(:find => { :limit => 10 }) do + merged_option = Developer.instance_eval('current_scoped_methods')[:find] + assert_equal({ :limit => 10 }, merged_option) + end + end + end + def test_replace_options Developer.with_scope(:find => { :conditions => "name = 'David'" }) do Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do @@ -369,8 +378,10 @@ class NestedScopingTest < ActiveRecord::TestCase def test_merged_scoped_find poor_jamis = developers(:poor_jamis) Developer.with_scope(:find => { :conditions => "salary < 100000" }) do - Developer.with_scope(:find => { :offset => 1 }) do - assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc')) + Developer.with_scope(:find => { :offset => 1, :order => 'id asc' }) do + assert_sql /ORDER BY id asc / do + assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc')) + end end end end @@ -400,6 +411,29 @@ class NestedScopingTest < ActiveRecord::TestCase end end + def test_nested_scoped_create + comment = nil + Comment.with_scope(:create => { :post_id => 1}) do + Comment.with_scope(:create => { :post_id => 2}) do + assert_equal({ :post_id => 2 }, Comment.send(:current_scoped_methods)[:create]) + comment = Comment.create :body => "Hey guys, nested scopes are broken. Please fix!" + end + end + assert_equal 2, comment.post_id + end + + def test_nested_exclusive_scope_for_create + comment = nil + Comment.with_scope(:create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do + Comment.with_exclusive_scope(:create => { :post_id => 1 }) do + assert_equal({ :post_id => 1 }, Comment.send(:current_scoped_methods)[:create]) + comment = Comment.create :body => "Hey guys" + end + end + assert_equal 1, comment.post_id + assert_equal 'Hey guys', comment.body + end + def test_merged_scoped_find_on_blank_conditions [nil, " ", [], {}].each do |blank| Developer.with_scope(:find => {:conditions => blank}) do @@ -523,7 +557,6 @@ class HasManyScopingTest< ActiveRecord::TestCase end end - class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase fixtures :posts, :categories, :categories_posts @@ -549,7 +582,6 @@ class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase end end - class DefaultScopingTest < ActiveRecord::TestCase fixtures :developers @@ -577,7 +609,7 @@ class DefaultScopingTest < ActiveRecord::TestCase # Scopes added on children should append to parent scope expected_klass_scope = [{ :create => {}, :find => { :order => 'salary DESC' }}, { :create => {}, :find => {} }] assert_equal expected_klass_scope, klass.send(:scoped_methods) - + # Parent should still have the original scope assert_equal scope, DeveloperOrderedBySalary.send(:scoped_methods) end @@ -620,7 +652,6 @@ end =begin # We disabled the scoping for has_one and belongs_to as we can't think of a proper use case - class BelongsToScopingTest< ActiveRecord::TestCase fixtures :comments, :posts @@ -640,7 +671,6 @@ class BelongsToScopingTest< ActiveRecord::TestCase end - class HasOneScopingTest< ActiveRecord::TestCase fixtures :comments, :posts diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 9f3a3848e2..ae6a54a5bd 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -15,7 +15,7 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal Topic.find(:all), Topic.base assert_equal Topic.find(:all), Topic.base.to_a assert_equal Topic.find(:first), Topic.base.first - assert_equal Topic.find(:all), Topic.base.each { |i| i } + assert_equal Topic.find(:all), Topic.base.map { |i| i } end def test_found_items_are_cached @@ -99,6 +99,12 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal topics_written_before_the_second, Topic.written_before(topics(:second).written_on) end + def test_procedural_scopes_returning_nil + all_topics = Topic.find(:all) + + assert_equal all_topics, Topic.written_before(nil) + end + def test_scopes_with_joins address = author_addresses(:david_address) posts_with_authors_at_address = Post.find( @@ -247,7 +253,7 @@ class NamedScopeTest < ActiveRecord::TestCase topic = Topic.approved.create!({}) assert topic.approved end - + def test_should_build_with_proxy_options_chained topic = Topic.approved.by_lifo.build({}) assert topic.approved @@ -287,15 +293,21 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal post.comments.size, Post.scoped(:joins => join).scoped(:joins => join, :conditions => "posts.id = #{post.id}").size end - def test_chanining_should_use_latest_conditions_when_creating - post1 = Topic.rejected.approved.new - assert post1.approved? + def test_chaining_should_use_latest_conditions_when_creating + post = Topic.rejected.new + assert !post.approved? - post2 = Topic.approved.rejected.new - assert ! post2.approved? + post = Topic.rejected.approved.new + assert post.approved? + + post = Topic.approved.rejected.new + assert !post.approved? + + post = Topic.approved.rejected.approved.new + assert post.approved? end - def test_chanining_should_use_latest_conditions_when_searching + def test_chaining_should_use_latest_conditions_when_searching # Normal hash conditions assert_equal Topic.all(:conditions => {:approved => true}), Topic.rejected.approved.all assert_equal Topic.all(:conditions => {:approved => false}), Topic.approved.rejected.all @@ -306,10 +318,24 @@ class NamedScopeTest < ActiveRecord::TestCase # Nested hash conditions with different keys assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).all.uniq end - + def test_methods_invoked_within_scopes_should_respect_scope assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id] end + + def test_named_scopes_batch_finders + assert_equal 3, Topic.approved.count + + assert_queries(4) do + Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? } + end + + assert_queries(2) do + Topic.approved.find_in_batches(:batch_size => 2) do |group| + group.each {|t| assert t.approved? } + end + end + end end class DynamicScopeMatchTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb index 39c6ea820d..b49997669e 100644 --- a/activerecord/test/cases/xml_serialization_test.rb +++ b/activerecord/test/cases/xml_serialization_test.rb @@ -38,11 +38,15 @@ class XmlSerializationTest < ActiveRecord::TestCase assert_match %r{<CreatedAt}, @xml end + def test_should_allow_skipped_types + @xml = Contact.new(:age => 25).to_xml :skip_types => true + assert %r{<age>25</age>}.match(@xml) + end + def test_should_include_yielded_additions @xml = Contact.new.to_xml do |xml| xml.creator "David" end - assert_match %r{<creator>David</creator>}, @xml end end @@ -145,6 +149,13 @@ class DatabaseConnectedXmlSerializationTest < ActiveRecord::TestCase assert_match %r{<hello-post type="StiPost">}, xml end + def test_included_associations_should_skip_types + xml = authors(:david).to_xml :include=>:hello_posts, :indent => 0, :skip_types => true + assert_match %r{<hello-posts>}, xml + assert_match %r{<hello-post>}, xml + assert_match %r{<hello-post>}, xml + end + def test_methods_are_called_on_object xml = authors(:david).to_xml :methods => :label, :indent => 0 assert_match %r{<label>.*</label>}, xml diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 7bc50e0e7b..238917bf30 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -1,16 +1,63 @@ class Pirate < ActiveRecord::Base belongs_to :parrot has_and_belongs_to_many :parrots - has_many :treasures, :as => :looter + has_and_belongs_to_many :parrots_with_method_callbacks, :class_name => "Parrot", + :before_add => :log_before_add, + :after_add => :log_after_add, + :before_remove => :log_before_remove, + :after_remove => :log_after_remove + has_and_belongs_to_many :parrots_with_proc_callbacks, :class_name => "Parrot", + :before_add => proc {|p,pa| p.ship_log << "before_adding_proc_parrot_#{pa.id || '<new>'}"}, + :after_add => proc {|p,pa| p.ship_log << "after_adding_proc_parrot_#{pa.id || '<new>'}"}, + :before_remove => proc {|p,pa| p.ship_log << "before_removing_proc_parrot_#{pa.id}"}, + :after_remove => proc {|p,pa| p.ship_log << "after_removing_proc_parrot_#{pa.id}"} + has_many :treasures, :as => :looter has_many :treasure_estimates, :through => :treasures, :source => :price_estimates # These both have :autosave enabled because accepts_nested_attributes_for is used on them. has_one :ship has_many :birds + has_many :birds_with_method_callbacks, :class_name => "Bird", + :before_add => :log_before_add, + :after_add => :log_after_add, + :before_remove => :log_before_remove, + :after_remove => :log_after_remove + has_many :birds_with_proc_callbacks, :class_name => "Bird", + :before_add => proc {|p,b| p.ship_log << "before_adding_proc_bird_#{b.id || '<new>'}"}, + :after_add => proc {|p,b| p.ship_log << "after_adding_proc_bird_#{b.id || '<new>'}"}, + :before_remove => proc {|p,b| p.ship_log << "before_removing_proc_bird_#{b.id}"}, + :after_remove => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"} accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks, + :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true validates_presence_of :catchphrase + + def ship_log + @ship_log ||= [] + end + + private + def log_before_add(record) + log(record, "before_adding_method") + end + + def log_after_add(record) + log(record, "after_adding_method") + end + + def log_before_remove(record) + log(record, "before_removing_method") + end + + def log_after_remove(record) + log(record, "after_removing_method") + end + + def log(record, callback) + ship_log << "#{callback}_#{record.class.name.downcase}_#{record.id || '<new>'}" + end end diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index f1b7bbae3b..51012d22ed 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -1,7 +1,9 @@ class Topic < ActiveRecord::Base named_scope :base named_scope :written_before, lambda { |time| - { :conditions => ['written_on < ?', time] } + if time + { :conditions => ['written_on < ?', time] } + end } named_scope :approved, :conditions => {:approved => true} named_scope :rejected, :conditions => {:approved => false} diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb index 69bb324813..6ed6f1a406 100644 --- a/activeresource/test/base_test.rb +++ b/activeresource/test/base_test.rb @@ -47,7 +47,7 @@ class BaseTest < Test::Unit::TestCase {:name => 'Milena', :children => []}]}]}.to_xml(:root => 'customer') # - resource with yaml array of strings; for ActiveRecords using serialize :bar, Array - @marty = <<-eof + @marty = <<-eof.strip <?xml version=\"1.0\" encoding=\"UTF-8\"?> <person> <id type=\"integer\">5</id> diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 1d42000867..8c6e724606 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,6 +1,7 @@ *Edge* -* XmlMini supports libxml if available. #2084 [Bart ten Brinke] +* XmlMini supports LibXML and Nokogiri backends. #2084, #2190 [Bart ten Brinke, Aaron Patterson] + Example: XmlMini.backend = 'Nokogiri' *2.3.1 [RC2] (March 5, 2009)* diff --git a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb index c96c5160b3..34ba8a005d 100644 --- a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb +++ b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb @@ -91,6 +91,12 @@ class HashWithIndifferentAccess < Hash self.dup.update(hash) end + # Performs the opposite of merge, with the keys and values from the first hash taking precedence over the second. + # This overloaded definition prevents returning a regular hash, if reverse_merge is called on a HashWithDifferentAccess. + def reverse_merge(other_hash) + super other_hash.with_indifferent_access + end + # Removes a specified key from the hash. def delete(key) super(convert_key(key)) diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 2e2a63c3e7..3ed30bdf56 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -257,20 +257,22 @@ module ActiveSupport # <%= link_to(@person.name, person_path(@person)) %> # # => <a href="/person/1-donald-e-knuth">Donald E. Knuth</a> def parameterize(string, sep = '-') - re_sep = Regexp.escape(sep) - # Replace accented chars with their ASCII equivalents. + # replace accented chars with ther ascii equivalents parameterized_string = transliterate(string) - # Turn unwanted chars into the separator. + # Turn unwanted chars into the seperator parameterized_string.gsub!(/[^a-z0-9\-_\+]+/i, sep) - # No more than one of the separator in a row. - parameterized_string.squeeze!(sep) - # Remove leading/trailing separator. - parameterized_string.gsub!(/^#{re_sep}|#{re_sep}$/i, '') + unless sep.blank? + re_sep = Regexp.escape(sep) + # No more than one of the separator in a row. + parameterized_string.gsub!(/#{re_sep}{2,}/, sep) + # Remove leading/trailing separator. + parameterized_string.gsub!(/^#{re_sep}|#{re_sep}$/i, '') + end parameterized_string.downcase end - # Replaces accented characters with their ASCII equivalents. + # Replaces accented characters with their ascii equivalents. def transliterate(string) Iconv.iconv('ascii//ignore//translit', 'utf-8', string).to_s end diff --git a/activesupport/lib/active_support/json/decoding.rb b/activesupport/lib/active_support/json/decoding.rb index 5eb8c0fd7d..0e079341ff 100644 --- a/activesupport/lib/active_support/json/decoding.rb +++ b/activesupport/lib/active_support/json/decoding.rb @@ -43,14 +43,32 @@ module ActiveSupport end if marks.empty? - json.gsub(/\\\//, '/') + json.gsub(/\\([\\\/]|u[[:xdigit:]]{4})/) do + ustr = $1 + if ustr.starts_with?('u') + [ustr[1..-1].to_i(16)].pack("U") + elsif ustr == '\\' + '\\\\' + else + ustr + end + end else left_pos = [-1].push(*marks) right_pos = marks << scanner.pos + scanner.rest_size output = [] left_pos.each_with_index do |left, i| scanner.pos = left.succ - output << scanner.peek(right_pos[i] - scanner.pos + 1) + output << scanner.peek(right_pos[i] - scanner.pos + 1).gsub(/\\([\\\/]|u[[:xdigit:]]{4})/) do + ustr = $1 + if ustr.starts_with?('u') + [ustr[1..-1].to_i(16)].pack("U") + elsif ustr == '\\' + '\\\\' + else + ustr + end + end end output = output * " " diff --git a/activesupport/lib/active_support/testing/setup_and_teardown.rb b/activesupport/lib/active_support/testing/setup_and_teardown.rb index 7edf6fdb32..aaf9f8f42c 100644 --- a/activesupport/lib/active_support/testing/setup_and_teardown.rb +++ b/activesupport/lib/active_support/testing/setup_and_teardown.rb @@ -45,7 +45,12 @@ module ActiveSupport return if @method_name.to_s == "default_test" if using_mocha = respond_to?(:mocha_verify) - assertion_counter = Mocha::TestCaseAdapter::AssertionCounter.new(result) + assertion_counter_klass = if defined?(Mocha::TestCaseAdapter::AssertionCounter) + Mocha::TestCaseAdapter::AssertionCounter + else + Mocha::Integration::TestUnit::AssertionCounter + end + assertion_counter = assertion_counter_klass.new(result) end yield(Test::Unit::TestCase::STARTED, name) diff --git a/activesupport/lib/active_support/xml_mini.rb b/activesupport/lib/active_support/xml_mini.rb index 99158e4ff7..ccd1349491 100644 --- a/activesupport/lib/active_support/xml_mini.rb +++ b/activesupport/lib/active_support/xml_mini.rb @@ -6,11 +6,24 @@ module ActiveSupport # XmlMini.backend = 'LibXML' module XmlMini extend self - delegate :parse, :to => :@backend + + attr_reader :backend + delegate :parse, :to => :backend def backend=(name) - require "active_support/xml_mini/#{name.to_s.downcase}.rb" - @backend = ActiveSupport.const_get("XmlMini_#{name}") + if name.is_a?(Module) + @backend = name + else + require "active_support/xml_mini/#{name.to_s.downcase}.rb" + @backend = ActiveSupport.const_get("XmlMini_#{name}") + end + end + + def with_backend(name) + old_backend, self.backend = backend, name + yield + ensure + self.backend = old_backend end end diff --git a/activesupport/lib/active_support/xml_mini/libxml.rb b/activesupport/lib/active_support/xml_mini/libxml.rb index e1549d8c58..3586b24a6b 100644 --- a/activesupport/lib/active_support/xml_mini/libxml.rb +++ b/activesupport/lib/active_support/xml_mini/libxml.rb @@ -1,4 +1,6 @@ -# = XML Mini Libxml implementation +require 'libxml' + +# = XmlMini LibXML implementation module ActiveSupport module XmlMini_LibXML #:nodoc: extend self @@ -7,19 +9,19 @@ module ActiveSupport # string:: # XML Document string to parse def parse(string) - XML.default_keep_blanks = false + LibXML::XML.default_keep_blanks = false if string.blank? {} else - XML::Parser.string(string.strip).parse.to_hash + LibXML::XML::Parser.string(string.strip).parse.to_hash end end end end -module XML +module LibXML module Conversions module Document def to_hash @@ -37,7 +39,7 @@ module XML # Hash to merge the converted element into. def to_hash(hash={}) if text? - raise RuntimeError if content.length >= LIB_XML_LIMIT + raise LibXML::XML::Error if content.length >= LIB_XML_LIMIT hash[CONTENT_ROOT] = content else sub_hash = insert_name_into_hash(hash, name) @@ -127,5 +129,5 @@ module XML end end -XML::Document.send(:include, XML::Conversions::Document) -XML::Node.send(:include, XML::Conversions::Node) +LibXML::XML::Document.send(:include, LibXML::Conversions::Document) +LibXML::XML::Node.send(:include, LibXML::Conversions::Node) diff --git a/activesupport/lib/active_support/xml_mini/nokogiri.rb b/activesupport/lib/active_support/xml_mini/nokogiri.rb new file mode 100644 index 0000000000..10281584fc --- /dev/null +++ b/activesupport/lib/active_support/xml_mini/nokogiri.rb @@ -0,0 +1,77 @@ +require 'nokogiri' + +# = XmlMini Nokogiri implementation +module ActiveSupport + module XmlMini_Nokogiri #:nodoc: + extend self + + # Parse an XML Document string into a simple hash using libxml / nokogiri. + # string:: + # XML Document string to parse + def parse(string) + if string.blank? + {} + else + doc = Nokogiri::XML(string) + raise doc.errors.first if doc.errors.length > 0 + doc.to_hash + end + end + + module Conversions + module Document + def to_hash + root.to_hash + end + end + + module Node + CONTENT_ROOT = '__content__' + + # Convert XML document to hash + # + # hash:: + # Hash to merge the converted element into. + def to_hash(hash = {}) + hash[name] ||= attributes_as_hash + + walker = lambda { |memo, parent, child, callback| + next if child.blank? && 'file' != parent['type'] + + if child.text? + (memo[CONTENT_ROOT] ||= '') << child.content + next + end + + name = child.name + + child_hash = child.attributes_as_hash + if memo[name] + memo[name] = [memo[name]].flatten + memo[name] << child_hash + else + memo[name] = child_hash + end + + # Recusively walk children + child.children.each { |c| + callback.call(child_hash, child, c, callback) + } + } + + children.each { |c| walker.call(hash[name], self, c, walker) } + hash + end + + def attributes_as_hash + Hash[*(attribute_nodes.map { |node| + [node.node_name, node.value] + }.flatten)] + end + end + end + + Nokogiri::XML::Document.send(:include, Conversions::Document) + Nokogiri::XML::Node.send(:include, Conversions::Node) + end +end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 2285d5a724..482ae57830 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -174,6 +174,13 @@ class HashExtTest < Test::Unit::TestCase assert_equal 2, hash['b'] end + def test_indifferent_reverse_merging + hash = HashWithIndifferentAccess.new('some' => 'value', 'other' => 'value') + hash.reverse_merge!(:some => 'noclobber', :another => 'clobber') + assert_equal 'value', hash[:some] + assert_equal 'clobber', hash[:another] + end + def test_indifferent_deleting get_hash = proc{ { :a => 'foo' }.with_indifferent_access } hash = get_hash.call @@ -884,7 +891,12 @@ class QueryTest < Test::Unit::TestCase end def test_expansion_count_is_limited - expected = defined?(LibXML) ? LibXML::XML::Error : RuntimeError + expected = { + 'ActiveSupport::XmlMini_REXML' => 'RuntimeError', + 'ActiveSupport::XmlMini_Nokogiri' => 'Nokogiri::XML::SyntaxError', + 'ActiveSupport::XmlMini_LibXML' => 'LibXML::XML::Error', + }[ActiveSupport::XmlMini.backend.name].constantize + assert_raise expected do attack_xml = <<-EOT <?xml version="1.0" encoding="UTF-8"?> diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index 948b6d9bb0..6b9fbd3156 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -116,6 +116,12 @@ class InflectorTest < Test::Unit::TestCase end end + def test_parameterize_with_multi_character_separator + StringToParameterized.each do |some_string, parameterized_string| + assert_equal(parameterized_string.gsub('-', '__sep__'), ActiveSupport::Inflector.parameterize(some_string, '__sep__')) + end + end + def test_classify ClassNameToTableName.each do |class_name, table_name| assert_equal(class_name, ActiveSupport::Inflector.classify(table_name)) diff --git a/activesupport/test/json/decoding_test.rb b/activesupport/test/json/decoding_test.rb index b88a00e584..8fe40557d6 100644 --- a/activesupport/test/json/decoding_test.rb +++ b/activesupport/test/json/decoding_test.rb @@ -28,7 +28,11 @@ class TestJSONDecoding < Test::Unit::TestCase %(null) => nil, %(true) => true, %(false) => false, - %q("http:\/\/test.host\/posts\/1") => "http://test.host/posts/1" + %q("http:\/\/test.host\/posts\/1") => "http://test.host/posts/1", + %q("\u003cunicode\u0020escape\u003e") => "<unicode escape>", + %q("\\\\u0020skip double backslashes") => "\\u0020skip double backslashes", + %q({a: "\u003cbr /\u003e"}) => {'a' => "<br />"}, + %q({b:["\u003ci\u003e","\u003cb\u003e","\u003cu\u003e"]}) => {'b' => ["<i>","<b>","<u>"]} } TESTS.each do |json, expected| diff --git a/activesupport/test/xml_mini/nokogiri_engine_test.rb b/activesupport/test/xml_mini/nokogiri_engine_test.rb new file mode 100644 index 0000000000..e5174a0b57 --- /dev/null +++ b/activesupport/test/xml_mini/nokogiri_engine_test.rb @@ -0,0 +1,157 @@ +require 'abstract_unit' +require 'active_support/xml_mini' + +begin + gem 'nokogiri', '>= 1.1.1' +rescue Gem::LoadError + # Skip nokogiri tests +else + +require 'nokogiri' + +class NokogiriEngineTest < Test::Unit::TestCase + include ActiveSupport + + def setup + @default_backend = XmlMini.backend + XmlMini.backend = 'Nokogiri' + end + + def teardown + XmlMini.backend = @default_backend + end + + def test_file_from_xml + hash = Hash.from_xml(<<-eoxml) + <blog> + <logo type="file" name="logo.png" content_type="image/png"> + </logo> + </blog> + eoxml + assert hash.has_key?('blog') + assert hash['blog'].has_key?('logo') + + file = hash['blog']['logo'] + assert_equal 'logo.png', file.original_filename + assert_equal 'image/png', file.content_type + end + + def test_exception_thrown_on_expansion_attack + assert_raise Nokogiri::XML::SyntaxError do + attack_xml = <<-EOT + <?xml version="1.0" encoding="UTF-8"?> + <!DOCTYPE member [ + <!ENTITY a "&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;"> + <!ENTITY b "&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;"> + <!ENTITY c "&d;&d;&d;&d;&d;&d;&d;&d;&d;&d;"> + <!ENTITY d "&e;&e;&e;&e;&e;&e;&e;&e;&e;&e;"> + <!ENTITY e "&f;&f;&f;&f;&f;&f;&f;&f;&f;&f;"> + <!ENTITY f "&g;&g;&g;&g;&g;&g;&g;&g;&g;&g;"> + <!ENTITY g "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"> + ]> + <member> + &a; + </member> + EOT + Hash.from_xml(attack_xml) + end + end + + def test_setting_nokogiri_as_backend + XmlMini.backend = 'Nokogiri' + assert_equal XmlMini_Nokogiri, XmlMini.backend + end + + def test_blank_returns_empty_hash + assert_equal({}, XmlMini.parse(nil)) + assert_equal({}, XmlMini.parse('')) + end + + def test_array_type_makes_an_array + assert_equal_rexml(<<-eoxml) + <blog> + <posts type="array"> + <post>a post</post> + <post>another post</post> + </posts> + </blog> + eoxml + end + + def test_one_node_document_as_hash + assert_equal_rexml(<<-eoxml) + <products/> + eoxml + end + + def test_one_node_with_attributes_document_as_hash + assert_equal_rexml(<<-eoxml) + <products foo="bar"/> + eoxml + end + + def test_products_node_with_book_node_as_hash + assert_equal_rexml(<<-eoxml) + <products> + <book name="awesome" id="12345" /> + </products> + eoxml + end + + def test_products_node_with_two_book_nodes_as_hash + assert_equal_rexml(<<-eoxml) + <products> + <book name="awesome" id="12345" /> + <book name="america" id="67890" /> + </products> + eoxml + end + + def test_single_node_with_content_as_hash + assert_equal_rexml(<<-eoxml) + <products> + hello world + </products> + eoxml + end + + def test_children_with_children + assert_equal_rexml(<<-eoxml) + <root> + <products> + <book name="america" id="67890" /> + </products> + </root> + eoxml + end + + def test_children_with_text + assert_equal_rexml(<<-eoxml) + <root> + <products> + hello everyone + </products> + </root> + eoxml + end + + def test_children_with_non_adjacent_text + assert_equal_rexml(<<-eoxml) + <root> + good + <products> + hello everyone + </products> + morning + </root> + eoxml + end + + private + def assert_equal_rexml(xml) + hash = XmlMini.with_backend('REXML') { XmlMini.parse(xml) } + assert_equal(hash, XmlMini.parse(xml)) + end +end + +end diff --git a/activesupport/test/xml_mini/rexml_engine_test.rb b/activesupport/test/xml_mini/rexml_engine_test.rb new file mode 100644 index 0000000000..a412d8ca05 --- /dev/null +++ b/activesupport/test/xml_mini/rexml_engine_test.rb @@ -0,0 +1,15 @@ +require 'abstract_unit' +require 'active_support/xml_mini' + +class REXMLEngineTest < Test::Unit::TestCase + include ActiveSupport + + def test_default_is_rexml + assert_equal XmlMini_REXML, XmlMini.backend + end + + def test_set_rexml_as_backend + XmlMini.backend = 'REXML' + assert_equal XmlMini_REXML, XmlMini.backend + end +end diff --git a/railties/lib/commands/plugin.rb b/railties/lib/commands/plugin.rb index a67c2ab447..8589b1698d 100644 --- a/railties/lib/commands/plugin.rb +++ b/railties/lib/commands/plugin.rb @@ -134,7 +134,8 @@ class RailsEnvironment def externals return [] unless use_externals? ext = `svn propget svn:externals "#{root}/vendor/plugins"` - ext.reject{ |line| line.strip == '' }.map do |line| + lines = ext.respond_to?(:lines) ? ext.lines : ext + lines.reject{ |line| line.strip == '' }.map do |line| line.strip.split(/\s+/, 2) end end diff --git a/railties/lib/rails/rack/static.rb b/railties/lib/rails/rack/static.rb index ef4e2642e2..f07c6beb5e 100644 --- a/railties/lib/rails/rack/static.rb +++ b/railties/lib/rails/rack/static.rb @@ -13,14 +13,18 @@ module Rails def call(env) path = env['PATH_INFO'].chomp('/') method = env['REQUEST_METHOD'] - cached_path = (path.empty? ? 'index' : path) + ::ActionController::Base.page_cache_extension if FILE_METHODS.include?(method) if file_exist?(path) return @file_server.call(env) - elsif file_exist?(cached_path) - env['PATH_INFO'] = cached_path - return @file_server.call(env) + else + cached_path = directory_exist?(path) ? "#{path}/index" : path + cached_path += ::ActionController::Base.page_cache_extension + + if file_exist?(cached_path) + env['PATH_INFO'] = cached_path + return @file_server.call(env) + end end end @@ -32,6 +36,11 @@ module Rails full_path = File.join(@file_server.root, ::Rack::Utils.unescape(path)) File.file?(full_path) && File.readable?(full_path) end + + def directory_exist?(path) + full_path = File.join(@file_server.root, ::Rack::Utils.unescape(path)) + File.directory?(full_path) && File.readable?(full_path) + end end end end diff --git a/railties/lib/rails_generator/generators/applications/app/template_runner.rb b/railties/lib/rails_generator/generators/applications/app/template_runner.rb index 73ab57d4f0..3b49b1fa92 100644 --- a/railties/lib/rails_generator/generators/applications/app/template_runner.rb +++ b/railties/lib/rails_generator/generators/applications/app/template_runner.rb @@ -90,7 +90,7 @@ module Rails gems_code = "config.gem '#{name}'" if options.any? - opts = options.inject([]) {|result, h| result << [":#{h[0]} => '#{h[1]}'"] }.sort.join(", ") + opts = options.inject([]) {|result, h| result << [":#{h[0]} => #{h[1].inspect.gsub('"',"'")}"] }.sort.join(", ") gems_code << ", #{opts}" end diff --git a/railties/test/fixtures/public/foo/bar.html b/railties/test/fixtures/public/foo/bar.html new file mode 100644 index 0000000000..9a35646205 --- /dev/null +++ b/railties/test/fixtures/public/foo/bar.html @@ -0,0 +1 @@ +/foo/bar.html
\ No newline at end of file diff --git a/railties/test/fixtures/public/foo/index.html b/railties/test/fixtures/public/foo/index.html new file mode 100644 index 0000000000..497a2e898f --- /dev/null +++ b/railties/test/fixtures/public/foo/index.html @@ -0,0 +1 @@ +/foo/index.html
\ No newline at end of file diff --git a/railties/test/fixtures/public/index.html b/railties/test/fixtures/public/index.html new file mode 100644 index 0000000000..525950ba6b --- /dev/null +++ b/railties/test/fixtures/public/index.html @@ -0,0 +1 @@ +/index.html
\ No newline at end of file diff --git a/railties/test/generators/rails_template_runner_test.rb b/railties/test/generators/rails_template_runner_test.rb index 4e1937e0c6..2da6bd59b5 100644 --- a/railties/test/generators/rails_template_runner_test.rb +++ b/railties/test/generators/rails_template_runner_test.rb @@ -93,6 +93,11 @@ class RailsTemplateRunnerTest < GeneratorTestCase assert_generated_file_with_data('config/environments/test.rb', "config.gem 'quietbacktrace'") end + def test_gem_with_lib_option_set_to_false_should_put_gem_dependency_in_enviroment_correctly + run_template_method(:gem, 'mislav-will-paginate', :lib => false, :source => 'http://gems.github.com') + assert_rails_initializer_includes("config.gem 'mislav-will-paginate', :lib => false, :source => 'http://gems.github.com'") + end + def test_environment_should_include_data_in_environment_initializer_block load_paths = 'config.load_paths += %w["#{RAILS_ROOT}/app/extras"]' run_template_method(:environment, load_paths) diff --git a/railties/test/rack_static_test.rb b/railties/test/rack_static_test.rb new file mode 100644 index 0000000000..ad673f6f19 --- /dev/null +++ b/railties/test/rack_static_test.rb @@ -0,0 +1,46 @@ +require 'abstract_unit' + +require 'action_controller' +require 'rails/rack' + +class RackStaticTest < ActiveSupport::TestCase + def setup + FileUtils.cp_r "#{RAILS_ROOT}/fixtures/public", "#{RAILS_ROOT}/public" + end + + def teardown + FileUtils.rm_rf "#{RAILS_ROOT}/public" + end + + DummyApp = lambda { |env| + [200, {"Content-Type" => "text/plain"}, ["Hello, World!"]] + } + App = Rails::Rack::Static.new(DummyApp) + + test "serves dynamic content" do + assert_equal "Hello, World!", get("/nofile") + end + + test "serves static index at root" do + assert_equal "/index.html", get("/index.html") + assert_equal "/index.html", get("/index") + assert_equal "/index.html", get("/") + end + + test "serves static file in directory" do + assert_equal "/foo/bar.html", get("/foo/bar.html") + assert_equal "/foo/bar.html", get("/foo/bar/") + assert_equal "/foo/bar.html", get("/foo/bar") + end + + test "serves static index file in directory" do + assert_equal "/foo/index.html", get("/foo/index.html") + assert_equal "/foo/index.html", get("/foo/") + assert_equal "/foo/index.html", get("/foo") + end + + private + def get(path) + Rack::MockRequest.new(App).request("GET", path).body + end +end |