diff options
166 files changed, 1536 insertions, 849 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 6ae3940e6d..0bf60a2c24 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -409,7 +409,7 @@ module ActionMailer #:nodoc: protected def set_payload_for_mail(payload, mail) #:nodoc: - payload[:mailer] = self.name + payload[:mailer] = name payload[:message_id] = mail.message_id payload[:subject] = mail.subject payload[:to] = mail.to @@ -421,11 +421,8 @@ module ActionMailer #:nodoc: end def method_missing(method, *args) #:nodoc: - if action_methods.include?(method.to_s) - new(method, *args).message - else - super - end + return super unless respond_to?(method) + new(method, *args).message end end diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index 889ae34407..86136bdafd 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -18,6 +18,10 @@ module ActionMailer options.javascripts_dir ||= paths.public.javascripts.to_a.first options.stylesheets_dir ||= paths.public.stylesheets.to_a.first + # make sure readers methods get compiled + options.asset_path ||= nil + options.asset_host ||= nil + ActiveSupport.on_load(:action_mailer) do include AbstractController::UrlFor extend ::AbstractController::Railties::RoutesHelpers.with(app.routes) @@ -25,5 +29,11 @@ module ActionMailer options.each { |k,v| send("#{k}=", v) } end end + + initializer "action_mailer.compile_config_methods" do + ActiveSupport.on_load(:action_mailer) do + config.compile_methods! if config.respond_to?(:compile_methods!) + end + end end end diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 521ee6913a..a6ce08113f 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -18,7 +18,7 @@ Rake::TestTask.new(:test_action_pack) do |t| # this will not happen automatically and the tests (as a whole) will error t.test_files = Dir.glob('test/{abstract,controller,dispatch,template}/**/*_test.rb').sort - #t.warning = true + t.warning = true t.verbose = true end diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 85270d84d8..f9f6eb945e 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -61,13 +61,13 @@ module AbstractController def action_methods @action_methods ||= begin # All public instance methods of this class, including ancestors - methods = public_instance_methods(true).map { |m| m.to_s }.to_set - + methods = (public_instance_methods(true) - # Except for public instance methods of Base and its ancestors - internal_methods.map { |m| m.to_s } + + internal_methods + # Be sure to include shadowed public instance methods of this class - public_instance_methods(false).map { |m| m.to_s } - + public_instance_methods(false)).uniq.map { |x| x.to_s } - # And always exclude explicitly hidden actions - hidden_actions + hidden_actions.to_a # Clear out AS callback method pollution methods.reject { |method| method =~ /_one_time_conditions/ } diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 958e7f7ec8..b68c7d9216 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -347,8 +347,7 @@ module AbstractController begin layout_name = _layout if action_has_layout? rescue NameError => e - raise NoMethodError, - "You specified #{@_layout.inspect} as the layout, but no such method was found" + raise e, "Could not render layout: #{e.message}" end if require_layout && action_has_layout? && !layout_name diff --git a/actionpack/lib/abstract_controller/url_for.rb b/actionpack/lib/abstract_controller/url_for.rb index 2e9de22ecd..e5d5bef6b4 100644 --- a/actionpack/lib/abstract_controller/url_for.rb +++ b/actionpack/lib/abstract_controller/url_for.rb @@ -1,7 +1,6 @@ module AbstractController module UrlFor extend ActiveSupport::Concern - include ActionDispatch::Routing::UrlFor def _routes diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index e02578eafd..5b81cd39f4 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -72,4 +72,5 @@ require 'active_support/core_ext/load_error' require 'active_support/core_ext/module/attr_internal' require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/name_error' +require 'active_support/core_ext/uri' require 'active_support/inflector' diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index a3591eafbe..d69d96b974 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -161,7 +161,7 @@ module ActionController #:nodoc: def normalize!(path) path << 'index' if path[-1] == ?/ path << ".#{extension}" if extension and !path.ends_with?(extension) - URI.unescape(path) + URI.parser.unescape(path) end end end diff --git a/actionpack/lib/action_controller/caching/pages.rb b/actionpack/lib/action_controller/caching/pages.rb index 4f7a5d3f55..df4d500069 100644 --- a/actionpack/lib/action_controller/caching/pages.rb +++ b/actionpack/lib/action_controller/caching/pages.rb @@ -1,5 +1,4 @@ require 'fileutils' -require 'uri' require 'active_support/core_ext/class/attribute_accessors' module ActionController #:nodoc: @@ -99,7 +98,7 @@ module ActionController #:nodoc: private def page_cache_file(path) - name = (path.empty? || path == "/") ? "/index" : URI.unescape(path.chomp('/')) + name = (path.empty? || path == "/") ? "/index" : URI.parser.unescape(path.chomp('/')) name << page_cache_extension unless (name.split('/').last || name).include? '.' return name end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 96cb5977d5..ace1aabe03 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -85,6 +85,9 @@ module ActionController def initialize(*) @_headers = {"Content-Type" => "text/html"} @_status = 200 + @_request = nil + @_response = nil + @_routes = nil super end @@ -99,7 +102,7 @@ module ActionController # Basic implementations for content_type=, location=, and headers are # provided to reduce the dependency on the RackDelegation module # in Renderer and Redirector. - + def content_type=(type) headers["Content-Type"] = type.to_s end diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 4d5d534c75..d14831b763 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -96,9 +96,9 @@ module ActionController def all_helpers_from_path(path) helpers = [] - Array.wrap(path).each do |path| - extract = /^#{Regexp.quote(path.to_s)}\/?(.*)_helper.rb$/ - helpers += Dir["#{path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') } + Array.wrap(path).each do |_path| + extract = /^#{Regexp.quote(_path.to_s)}\/?(.*)_helper.rb$/ + helpers += Dir["#{_path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') } end helpers.sort! helpers.uniq! diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 6a7e170306..547cec7081 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -214,7 +214,7 @@ module ActionController 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(', ') + "Digest " + credentials.sort_by {|x| x[0].to_s }.map {|v| "#{v[0]}='#{v[1]}'" }.join(', ') end def decode_credentials_header(request) @@ -423,14 +423,13 @@ module ActionController # Returns nil if no token is found. def token_and_options(request) if header = request.authorization.to_s[/^Token (.*)/] - values = $1.split(','). - inject({}) do |memo, value| - value.strip! # remove any spaces between commas and values - key, value = value.split(/\=\"?/) # split key=value pairs - value.chomp!('"') # chomp trailing " in value - value.gsub!(/\\\"/, '"') # unescape remaining quotes - memo.update(key => value) - end + values = Hash[$1.split(',').map do |value| + value.strip! # remove any spaces between commas and values + key, value = value.split(/\=\"?/) # split key=value pairs + value.chomp!('"') # chomp trailing " in value + value.gsub!(/\\\"/, '"') # unescape remaining quotes + [key, value] + end] [values.delete("token"), values.with_indifferent_access] end end @@ -442,9 +441,8 @@ module ActionController # # Returns String. def encode_credentials(token, options = {}) - values = ["token=#{token.to_s.inspect}"] - options.each do |key, value| - values << "#{key}=#{value.to_s.inspect}" + values = ["token=#{token.to_s.inspect}"] + options.map do |key, value| + "#{key}=#{value.to_s.inspect}" end "Token #{values * ", "}" end diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 0be07cd1fc..f9b226b7c9 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -71,7 +71,7 @@ module ActionController end add :json do |json, options| - json = ActiveSupport::JSON.encode(json, options) unless json.respond_to?(:to_str) + json = json.to_json(options) unless json.respond_to?(:to_str) json = "#{options[:callback]}(#{json})" unless options[:callback].blank? self.content_type ||= Mime::JSON self.response_body = json diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index 85c6b0a9b5..333eeaeffb 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -5,16 +5,20 @@ module ActionController include AbstractController::UrlFor def url_options - options = {} - if _routes.equal?(env["action_dispatch.routes"]) - options[:script_name] = request.script_name.dup - end - - super.merge(options).reverse_merge( + @_url_options ||= super.reverse_merge( :host => request.host_with_port, :protocol => request.protocol, :_path_segments => request.symbolized_path_parameters - ) + ).freeze + + if _routes.equal?(env["action_dispatch.routes"]) + @_url_options.dup.tap do |options| + options[:script_name] = request.script_name.dup + options.freeze + end + else + @_url_options + end end end end diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index aea28d9265..0ade42ba2d 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -26,6 +26,10 @@ module ActionController options.stylesheets_dir ||= paths.public.stylesheets.to_a.first options.page_cache_directory ||= paths.public.to_a.first + # make sure readers methods get compiled + options.asset_path ||= nil + options.asset_host ||= nil + ActiveSupport.on_load(:action_controller) do include app.routes.mounted_helpers extend ::AbstractController::Railties::RoutesHelpers.with(app.routes) @@ -33,5 +37,11 @@ module ActionController options.each { |k,v| send("#{k}=", v) } end end + + initializer "action_controller.compile_config_methods" do + ActiveSupport.on_load(:action_controller) do + config.compile_methods! if config.respond_to?(:compile_methods!) + end + end end end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index f5ae1c3fff..1af75fc2d7 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -127,7 +127,7 @@ module ActionController class Result < ::Array #:nodoc: def to_s() join '/' end def self.new_escaped(strings) - new strings.collect {|str| URI.unescape str} + new strings.collect {|str| uri_parser.unescape str} end end @@ -394,7 +394,7 @@ module ActionController parameters ||= {} @request.assign_parameters(@routes, @controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters) - @request.session = ActionController::TestSession.new(session) unless session.nil? + @request.session = ActionController::TestSession.new(session) if session @request.session["flash"] = @request.flash.update(flash || {}) @request.session["flash"].sweep @@ -417,7 +417,7 @@ module ActionController @request.env.delete('PATH_INFO') - if @controller + if defined?(@controller) && @controller @controller.request = @request @controller.params = {} end @@ -462,9 +462,11 @@ module ActionController # The exception is stored in the exception accessor for further inspection. module RaiseActionExceptions def self.included(base) - base.class_eval do - attr_accessor :exception - protected :exception, :exception= + unless base.method_defined?(:exception) && base.method_defined?(:exception=) + base.class_eval do + attr_accessor :exception + protected :exception, :exception= + end end end diff --git a/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb b/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb index 85250721e7..22b3243104 100644 --- a/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb +++ b/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb @@ -18,14 +18,14 @@ module HTML #:nodoc: hash[k] = Conditions.new(v) when :children hash[k] = v = keys_to_symbols(v) - v.each do |k,v2| - case k + v.each do |key,value| + case key when :count, :greater_than, :less_than # keys are valid, and require no further processing when :only - v[k] = Conditions.new(v2) + v[key] = Conditions.new(value) else - raise "illegal key #{k.inspect} => #{v2.inspect}" + raise "illegal key #{key.inspect} => #{value.inspect}" end end else diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 7a28228817..09d6ba8223 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -54,11 +54,7 @@ module ActionDispatch # the application should use), this \method returns the overridden # value, not the original. def request_method - @request_method ||= begin - method = env["REQUEST_METHOD"] - HTTP_METHOD_LOOKUP[method] || raise(ActionController::UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}") - method - end + @request_method ||= check_method(env["REQUEST_METHOD"]) end # Returns a symbol form of the #request_method @@ -70,11 +66,7 @@ module ActionDispatch # even if it was overridden by middleware. See #request_method for # more information. def method - @method ||= begin - method = env["rack.methodoverride.original_method"] || env['REQUEST_METHOD'] - HTTP_METHOD_LOOKUP[method] || raise(ActionController::UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}") - method - end + @method ||= check_method(env["rack.methodoverride.original_method"] || env['REQUEST_METHOD']) end # Returns a symbol form of the #method @@ -246,5 +238,12 @@ module ActionDispatch def local? LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip } end + + private + + def check_method(name) + HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}") + name + end end end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index ff5e96fdf7..151c90167b 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -140,7 +140,6 @@ module ActionDispatch # :nodoc: assign_default_content_type_and_charset! handle_conditional_get! self["Set-Cookie"] = self["Set-Cookie"].join("\n") if self["Set-Cookie"].respond_to?(:join) - self["ETag"] = @_etag if @_etag super end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 2e39d0dbc2..3e5cd6a2f9 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -15,12 +15,12 @@ module ActionDispatch # Returns 'https://' if this is an SSL request and 'http://' otherwise. def protocol - ssl? ? 'https://' : 'http://' + @protocol ||= ssl? ? 'https://' : 'http://' end # Is this an SSL request? def ssl? - @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' + @ssl ||= @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' end # Returns the \host for this request, such as "example.com". diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb index e4ae480bfb..0bb950d1cc 100644 --- a/actionpack/lib/action_dispatch/middleware/callbacks.rb +++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb @@ -19,9 +19,11 @@ module ActionDispatch # replace the existing callback. Passing an identifier is a suggested # practice if the code adding a preparation block may be reloaded. def self.to_prepare(*args, &block) - if args.first.is_a?(Symbol) && block_given? - define_method :"__#{args.first}", &block - set_callback(:prepare, :"__#{args.first}") + first_arg = args.first + if first_arg.is_a?(Symbol) && block_given? + remove_method :"__#{first_arg}" if method_defined?(:"__#{first_arg}") + define_method :"__#{first_arg}", &block + set_callback(:prepare, :"__#{first_arg}") else set_callback(:prepare, *args, &block) end diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 348a2d1eb2..db0187c015 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -102,8 +102,8 @@ module ActionDispatch def destroy clear - @by.send(:destroy, @env) if @by - @env[ENV_SESSION_OPTIONS_KEY][:id] = nil if @env && @env[ENV_SESSION_OPTIONS_KEY] + @by.send(:destroy, @env) if defined?(@by) && @by + @env[ENV_SESSION_OPTIONS_KEY][:id] = nil if defined?(@env) && @env && @env[ENV_SESSION_OPTIONS_KEY] @loaded = false end @@ -165,7 +165,7 @@ module ActionDispatch return response unless value cookie = { :value => value } - unless options[:expire_after].nil? + if options[:expire_after] cookie[:expires] = Time.now + options.delete(:expire_after) end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index db7f342bc5..e3cd779756 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -41,9 +41,12 @@ module ActionDispatch end end - def initialize(*args, &block) - super(*args) - block.call(self) if block_given? + # Use this instead of super to work around a warning. + alias :array_initialize :initialize + + def initialize(*args) + array_initialize(*args) + yield(self) if block_given? end def insert(index, *args, &block) diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 581cadbeb4..cf13938331 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -4,7 +4,7 @@ module ActionDispatch class FileHandler def initialize(at, root) @at, @root = at.chomp('/'), root.chomp('/') - @compiled_at = Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank? + @compiled_at = (Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank?) @compiled_root = Regexp.compile(/^#{Regexp.escape(root)}/) @file_server = ::Rack::File.new(root) end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index e963b04524..97f7cf0bbe 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -14,7 +14,7 @@ def debug_hash(hash) hash.sort_by { |k, v| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") - end + end unless self.class.method_defined?(:debug_hash) %> <h2 style="margin-top: 30px">Request</h2> @@ -28,4 +28,4 @@ <h2 style="margin-top: 30px">Response</h2> -<p><b>Headers</b>: <pre><%=h @response ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p> +<p><b>Headers</b>: <pre><%=h defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb index d18b162a93..8771b5fd6d 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb @@ -12,14 +12,14 @@ <div id="traces"> <% names.each do |name| %> <% - show = "document.getElementById('#{name.gsub /\s/, '-'}').style.display='block';" - hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub /\s/, '-'}').style.display='none';"} + show = "document.getElementById('#{name.gsub(/\s/, '-')}').style.display='block';" + hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub(/\s/, '-')}').style.display='none';"} %> <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %> <% end %> <% traces.each do |name, trace| %> - <div id="<%= name.gsub /\s/, '-' %>" style="display: <%= name == "Application Trace" ? 'block' : 'none' %>;"> + <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == "Application Trace") ? 'block' : 'none' %>;"> <pre><code><%=h trace.join "\n" %></code></pre> </div> <% end %> diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index fe85acb94e..47aed0273c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -62,7 +62,6 @@ module ActionDispatch if using_match_shorthand?(path_without_format, @options) to_shorthand = @options[:to].blank? @options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1') - @options[:as] ||= Mapper.normalize_name(path_without_format) end @options.merge!(default_controller_and_action(to_shorthand)) @@ -346,11 +345,11 @@ module ActionDispatch # Redirect any path to another path: # # match "/stories" => redirect("/posts") - def redirect(*args, &block) + def redirect(*args) options = args.last.is_a?(Hash) ? args.pop : {} - path = args.shift || block - path_proc = path.is_a?(Proc) ? path : proc { |params| path % params } + path = args.shift || Proc.new + path_proc = path.is_a?(Proc) ? path : proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } status = options[:status] || 301 lambda do |env| @@ -395,10 +394,10 @@ module ActionDispatch # namespace "admin" do # resources :posts, :comments # end - # + # # This will create a number of routes for each of the posts and comments # controller. For Admin::PostsController, Rails will create: - # + # # GET /admin/photos # GET /admin/photos/new # POST /admin/photos @@ -406,33 +405,33 @@ module ActionDispatch # GET /admin/photos/1/edit # PUT /admin/photos/1 # DELETE /admin/photos/1 - # + # # If you want to route /photos (without the prefix /admin) to # Admin::PostsController, you could use - # + # # scope :module => "admin" do # resources :posts, :comments # end # # or, for a single case - # + # # resources :posts, :module => "admin" - # + # # If you want to route /admin/photos to PostsController # (without the Admin:: module prefix), you could use - # + # # scope "/admin" do # resources :posts, :comments # end # # or, for a single case - # + # # resources :posts, :path => "/admin" # # In each of these cases, the named routes remain the same as if you did # not use scope. In the last case, the following paths map to # PostsController: - # + # # GET /admin/photos # GET /admin/photos/new # POST /admin/photos @@ -676,6 +675,7 @@ module ActionDispatch DEFAULT_ACTIONS = [:show, :create, :update, :destroy, :new, :edit] def initialize(entities, options) + @as = nil @name = entities.to_s @path = (options.delete(:path) || @name).to_s @controller = (options.delete(:controller) || plural).to_s @@ -923,9 +923,14 @@ module ActionDispatch if action.to_s =~ /^[\w\/]+$/ options[:action] ||= action unless action.to_s.include?("/") - options[:as] = name_for_action(action, options[:as]) else - options[:as] = name_for_action(options[:as]) + action = nil + end + + if options.key?(:as) && !options[:as] + options.delete(:as) + else + options[:as] = name_for_action(options[:as], action) end super(path, options) @@ -1091,18 +1096,16 @@ module ActionDispatch path || @scope[:path_names][name.to_sym] || name.to_s end - def prefix_name_for_action(action, as) - if as.present? + def prefix_name_for_action(as, action) + if as as.to_s - elsif as - nil elsif !canonical_action?(action, @scope[:scope_level]) action.to_s end end - def name_for_action(action, as=nil) - prefix = prefix_name_for_action(action, as) + def name_for_action(as, action) + prefix = prefix_name_for_action(as, action) prefix = Mapper.normalize_name(prefix) if prefix name_prefix = @scope[:as] @@ -1126,7 +1129,8 @@ module ActionDispatch [name_prefix, member_name, prefix] end - name.select(&:present?).join("_").presence + candidate = name.select(&:present?).join("_").presence + candidate unless as.nil? && @set.routes.find { |r| r.name == candidate } end end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 02ba5236ee..49e237f8db 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -105,7 +105,7 @@ module ActionDispatch else [ record_or_hash_or_array ] end - inflection = if options[:action].to_s == "new" + inflection = if options[:action] && options[:action].to_s == "new" args.pop :singular elsif (record.respond_to?(:persisted?) && !record.persisted?) @@ -168,10 +168,7 @@ module ActionDispatch end def build_named_route_call(records, inflection, options = {}) - unless records.is_a?(Array) - record = extract_record(records) - route = [] - else + if records.is_a?(Array) record = records.pop route = records.map do |parent| if parent.is_a?(Symbol) || parent.is_a?(String) @@ -180,6 +177,9 @@ module ActionDispatch ActiveModel::Naming.route_key(parent).singularize end end + else + record = extract_record(records) + route = [] end if record.is_a?(Symbol) || record.is_a?(String) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 1a5f21bd09..32f41934f1 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,6 +1,7 @@ require 'rack/mount' require 'forwardable' require 'active_support/core_ext/object/to_query' +require 'active_support/core_ext/hash/slice' module ActionDispatch module Routing @@ -66,7 +67,7 @@ module ActionDispatch end def split_glob_param!(params) - params[@glob_param] = params[@glob_param].split('/').map { |v| URI.unescape(v) } + params[@glob_param] = params[@glob_param].split('/').map { |v| URI.parser.unescape(v) } end end @@ -157,6 +158,7 @@ module ActionDispatch # We use module_eval to avoid leaks @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1 + remove_method :#{selector} if method_defined?(:#{selector}) def #{selector}(*args) options = args.extract_options! @@ -190,6 +192,7 @@ module ActionDispatch hash_access_method = hash_access_name(name, kind) @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1 + remove_method :#{selector} if method_defined?(:#{selector}) def #{selector}(*args) url_for(#{hash_access_method}(*args)) end @@ -300,9 +303,9 @@ module ActionDispatch extend ActiveSupport::Concern include UrlFor - @routes = routes + @_routes = routes class << self - delegate :url_for, :to => '@routes' + delegate :url_for, :to => '@_routes' end extend routes.named_routes.module @@ -311,7 +314,7 @@ module ActionDispatch # Yes plz - JP included do routes.install_helpers(self) - singleton_class.send(:define_method, :_routes) { routes } + singleton_class.send(:redefine_method, :_routes) { routes } end define_method(:_routes) { @_routes || routes } @@ -334,6 +337,19 @@ module ActionDispatch end class Generator #:nodoc: + PARAMETERIZE = { + :parameterize => lambda do |name, value| + if name == :controller + value + elsif value.is_a?(Array) + value.map { |v| Rack::Mount::Utils.escape_uri(v.to_param) }.join('/') + else + return nil unless param = value.to_param + param.split('/').map { |v| Rack::Mount::Utils.escape_uri(v) }.join("/") + end + end + } + attr_reader :options, :recall, :set, :named_route def initialize(options, recall, set, extras = false) @@ -422,7 +438,7 @@ module ActionDispatch end def generate - path, params = @set.set.generate(:path_info, named_route, options, recall, opts) + path, params = @set.set.generate(:path_info, named_route, options, recall, PARAMETERIZE) raise_routing_error unless path @@ -430,26 +446,12 @@ module ActionDispatch return [path, params.keys] if @extras - path << "?#{params.to_query}" if params.any? + path << "?#{params.to_query}" unless params.empty? path rescue Rack::Mount::RoutingError raise_routing_error end - def opts - parameterize = lambda do |name, value| - if name == :controller - value - elsif value.is_a?(Array) - value.map { |v| Rack::Mount::Utils.escape_uri(v.to_param) }.join('/') - else - return nil unless param = value.to_param - param.split('/').map { |v| Rack::Mount::Utils.escape_uri(v) }.join("/") - end - end - {:parameterize => parameterize} - end - def raise_routing_error raise ActionController::RoutingError.new("No route matches #{options.inspect}") end @@ -510,7 +512,7 @@ module ActionDispatch end script_name = options.delete(:script_name) - path = (script_name.blank? ? _generate_prefix(options) : script_name).to_s + path = (script_name.blank? ? _generate_prefix(options) : script_name.chomp('/')).to_s path_options = options.except(*RESERVED_OPTIONS) path_options = yield(path_options) if block_given? @@ -543,7 +545,7 @@ module ActionDispatch params.each do |key, value| if value.is_a?(String) value = value.dup.force_encoding(Encoding::BINARY) if value.encoding_aware? - params[key] = URI.unescape(value) + params[key] = URI.parser.unescape(value) end end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index e836cf7c8e..bfdea41f60 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -98,6 +98,11 @@ module ActionDispatch end end + def initialize(*) + @_routes = nil + super + end + def url_options default_url_options end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 10b122557a..1558c3ae05 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -81,14 +81,10 @@ module ActionDispatch def normalize_argument_to_redirection(fragment) case fragment - when %r{^\w[\w\d+.-]*:.*} + when %r{^\w[A-Za-z\d+.-]*:.*} fragment when String - if fragment =~ %r{^\w[\w\d+.-]*:.*} - fragment - else - @request.protocol + @request.host_with_port + fragment - end + @request.protocol + @request.host_with_port + fragment when :back raise RedirectBackError unless refer = @request.headers["Referer"] refer diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index c5fed1fc8f..1390b74a95 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -146,16 +146,16 @@ module ActionDispatch # def with_routing old_routes, @routes = @routes, ActionDispatch::Routing::RouteSet.new - old_controller, @controller = @controller, @controller.clone if @controller - _routes = @routes - - # Unfortunately, there is currently an abstraction leak between AC::Base - # and AV::Base which requires having the URL helpers in both AC and AV. - # To do this safely at runtime for tests, we need to bump up the helper serial - # to that the old AV subclass isn't cached. - # - # TODO: Make this unnecessary - if @controller + if defined?(@controller) && @controller + old_controller, @controller = @controller, @controller.clone + _routes = @routes + + # Unfortunately, there is currently an abstraction leak between AC::Base + # and AV::Base which requires having the URL helpers in both AC and AV. + # To do this safely at runtime for tests, we need to bump up the helper serial + # to that the old AV subclass isn't cached. + # + # TODO: Make this unnecessary @controller.singleton_class.send(:include, _routes.url_helpers) @controller.view_context_class = Class.new(@controller.view_context_class) do include _routes.url_helpers @@ -164,14 +164,14 @@ module ActionDispatch yield @routes ensure @routes = old_routes - if @controller + if defined?(@controller) && @controller @controller = old_controller end end # ROUTES TODO: These assertions should really work in an integration context def method_missing(selector, *args, &block) - if @controller && @routes && @routes.named_routes.helpers.include?(selector) + if defined?(@controller) && @controller && @routes && @routes.named_routes.helpers.include?(selector) @controller.send(selector, *args, &block) else super diff --git a/actionpack/lib/action_dispatch/testing/assertions/selector.rb b/actionpack/lib/action_dispatch/testing/assertions/selector.rb index e1015c62cd..2b862fb7d6 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/selector.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/selector.rb @@ -67,7 +67,7 @@ module ActionDispatch arg = args.shift elsif arg == nil raise ArgumentError, "First argument is either selector or element to select, but nil found. Perhaps you called assert_select with an element that does not exist?" - elsif @selected + elsif defined?(@selected) && @selected matches = [] @selected.each do |selected| @@ -187,6 +187,7 @@ module ActionDispatch def assert_select(*args, &block) # Start with optional element followed by mandatory selector. arg = args.shift + @selected ||= nil if arg.is_a?(HTML::Node) # First argument is a node (tag or text, but also HTML root), @@ -442,6 +443,7 @@ module ActionDispatch assert_block("") { true } # to count the assertion if block_given? && !([:remove, :show, :hide, :toggle].include? rjs_type) begin + @selected ||= nil in_scope, @selected = @selected, matches yield matches ensure @@ -513,8 +515,8 @@ module ActionDispatch node.content.gsub(/<!\[CDATA\[(.*)(\]\]>)?/m) { Rack::Utils.escapeHTML($1) } end - selected = elements.map do |element| - text = element.children.select{ |c| not c.tag? }.map{ |c| fix_content[c] }.join + selected = elements.map do |_element| + text = _element.children.select{ |c| not c.tag? }.map{ |c| fix_content[c] }.join root = HTML::Document.new(CGI.unescapeHTML("<encoded>#{text}</encoded>")).root css_select(root, "encoded:root", &block)[0] end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 84039c06ab..fee8cad9f5 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -171,6 +171,7 @@ module ActionDispatch # Create and initialize a new Session instance. def initialize(app) + super() @app = app # If the app is a Rails app, make url_helpers available on the session @@ -182,6 +183,7 @@ module ActionDispatch reset! end + remove_method :default_url_options def default_url_options { :host => host, :protocol => https? ? "https" : "http" } end @@ -257,19 +259,19 @@ module ActionDispatch end end - port = host.split(':')[1] + hostname, port = host.split(':') env = { :method => method, :params => parameters, - "SERVER_NAME" => host.split(':')[0], - "SERVER_PORT" => (port ? port : (https? ? "443" : "80")), + "SERVER_NAME" => hostname, + "SERVER_PORT" => port || (https? ? "443" : "80"), "HTTPS" => https? ? "on" : "off", "rack.url_scheme" => https? ? "https" : "http", "REQUEST_URI" => path, - "HTTP_HOST" => [host, port].compact.join(':'), + "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, "CONTENT_TYPE" => "application/x-www-form-urlencoded", "HTTP_ACCEPT" => accept @@ -307,7 +309,7 @@ module ActionDispatch include ActionDispatch::Assertions def app - @app + @app ||= nil end # Reset the current session. This is useful for testing multiple sessions @@ -319,10 +321,10 @@ module ActionDispatch %w(get post put head delete cookies assigns xml_http_request xhr get_via_redirect post_via_redirect).each do |method| define_method(method) do |*args| - reset! unless @integration_session + reset! unless integration_session # reset the html_document variable, but only for new get/post calls @html_document = nil unless %w(cookies assigns).include?(method) - @integration_session.__send__(method, *args).tap do + integration_session.__send__(method, *args).tap do copy_session_variables! end end @@ -347,7 +349,7 @@ module ActionDispatch # Copy the instance variables from the current session instance into the # test instance. def copy_session_variables! #:nodoc: - return unless @integration_session + return unless integration_session %w(controller response request).each do |var| instance_variable_set("@#{var}", @integration_session.__send__(var)) end @@ -357,21 +359,26 @@ module ActionDispatch include ActionDispatch::Routing::UrlFor def url_options - reset! unless @integration_session - @integration_session.url_options + reset! unless integration_session + integration_session.url_options end # Delegate unhandled messages to the current session instance. def method_missing(sym, *args, &block) - reset! unless @integration_session - if @integration_session.respond_to?(sym) - @integration_session.__send__(sym, *args, &block).tap do + reset! unless integration_session + if integration_session.respond_to?(sym) + integration_session.__send__(sym, *args, &block).tap do copy_session_variables! end else super end end + + private + def integration_session + @integration_session ||= nil + end end end diff --git a/actionpack/lib/action_dispatch/testing/test_request.rb b/actionpack/lib/action_dispatch/testing/test_request.rb index c587a36930..cf440a1fad 100644 --- a/actionpack/lib/action_dispatch/testing/test_request.rb +++ b/actionpack/lib/action_dispatch/testing/test_request.rb @@ -13,6 +13,7 @@ module ActionDispatch env = Rails.application.env_config.merge(env) if defined?(Rails.application) super(DEFAULT_ENV.merge(env)) + @cookies = nil self.host = 'test.host' self.remote_addr = '0.0.0.0' self.user_agent = 'Rails Testing' @@ -66,7 +67,7 @@ module ActionDispatch def accept=(mime_types) @env.delete('action_dispatch.request.accepts') - @env['HTTP_ACCEPT'] = Array(mime_types).collect { |mime_types| mime_types.to_s }.join(",") + @env['HTTP_ACCEPT'] = Array(mime_types).collect { |mime_type| mime_type.to_s }.join(",") end def cookies diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 3fa46d0f43..0bef3e3a08 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -209,8 +209,7 @@ module ActionView #:nodoc: @_request = controller.request if controller.respond_to?(:request) end - config = controller && controller.respond_to?(:config) ? controller.config : {} - @_config = ActiveSupport::InheritableOptions.new(config) + @_config = controller && controller.respond_to?(:config) ? controller.config.inheritable_copy : {} @_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } @_virtual_path = nil diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 687cb83d75..c1dfbe5dc3 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -705,19 +705,29 @@ module ActionView private - def rewrite_extension?(source, dir, ext) - source_ext = File.extname(source)[1..-1] - ext && (source_ext.blank? || (ext != source_ext && File.exist?(File.join(config.assets_dir, dir, "#{source}.#{ext}")))) + def rewrite_extension(source, dir, ext) + source_ext = File.extname(source) + + if source_ext.empty? + "#{source}.#{ext}" + elsif ext != source_ext[1..-1] + with_ext = "#{source}.#{ext}" + with_ext if File.exist?(File.join(config.assets_dir, dir, with_ext)) + end || source end def rewrite_host_and_protocol(source, has_request) host = compute_asset_host(source) - if has_request && host.present? && !is_uri?(host) + if has_request && host && !is_uri?(host) host = "#{controller.request.protocol}#{host}" end "#{host}#{source}" end + def rewrite_relative_url_root(source, relative_url_root) + relative_url_root && !source.starts_with?("#{relative_url_root}/") ? "#{relative_url_root}#{source}" : source + end + # Add the the extension +ext+ if not present. Return full URLs otherwise untouched. # Prefix with <tt>/dir/</tt> if lacking a leading +/+. Account for relative URL # roots. Rewrite the asset path for cache-busting asset ids. Include @@ -725,17 +735,15 @@ module ActionView def compute_public_path(source, dir, ext = nil, include_host = true) return source if is_uri?(source) - source += ".#{ext}" if rewrite_extension?(source, dir, ext) - source = "/#{dir}/#{source}" unless source[0] == ?/ + source = rewrite_extension(source, dir, ext) if ext + source = "/#{dir}/#{source}" unless source[0] == ?/ if controller.respond_to?(:env) && controller.env["action_dispatch.asset_path"] source = rewrite_asset_path(source, controller.env["action_dispatch.asset_path"]) end source = rewrite_asset_path(source, config.asset_path) has_request = controller.respond_to?(:request) - if has_request && include_host && source !~ %r{^#{controller.config.relative_url_root}/} - source = "#{controller.config.relative_url_root}#{source}" - end + source = rewrite_relative_url_root(source, controller.config.relative_url_root) if has_request && include_host source = rewrite_host_and_protocol(source, has_request) if include_host source @@ -802,10 +810,10 @@ module ActionView end asset_id = rails_asset_id(source) - if asset_id.blank? + if asset_id.empty? source else - source + "?#{asset_id}" + "#{source}?#{asset_id}" end end diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 9891478606..3aee4fb773 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -923,6 +923,7 @@ module ActionView private def datetime_selector(options, html_options) datetime = value(object) || default_datetime(options) + @auto_index ||= nil options = options.dup options[:field_name] = @method_name diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 3ebc6601e5..1836baaf12 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -850,7 +850,7 @@ module ActionView extend ActiveSupport::Concern include Helpers::CaptureHelper, Context, Helpers::TagHelper, Helpers::FormTagHelper - attr_reader :method_name, :object_name + attr_reader :object, :method_name, :object_name DEFAULT_FIELD_OPTIONS = { "size" => 30 } DEFAULT_RADIO_OPTIONS = { } @@ -859,14 +859,9 @@ module ActionView def initialize(object_name, method_name, template_object, object = nil) @object_name, @method_name = object_name.to_s.dup, method_name.to_s.dup @template_object = template_object - @object = object - if @object_name.sub!(/\[\]$/,"") || @object_name.sub!(/\[\]\]$/,"]") - if (object ||= @template_object.instance_variable_get("@#{Regexp.last_match.pre_match}")) && object.respond_to?(:to_param) - @auto_index = object.to_param - else - raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}" - end - end + @object_name.sub!(/\[\]$/,"") || @object_name.sub!(/\[\]\]$/,"]") + @object = retrieve_object(object) + @auto_index = retrieve_autoindex(Regexp.last_match.pre_match) if Regexp.last_match end def to_label_tag(text = nil, options = {}, &block) @@ -990,14 +985,26 @@ module ActionView content_tag(tag_name, value(object), options) end - def object - @object || @template_object.instance_variable_get("@#{@object_name}") + def retrieve_object(object) + if object + object + elsif @template_object.instance_variable_defined?("@#{@object_name}") + @template_object.instance_variable_get("@#{@object_name}") + end rescue NameError - # As @object_name may contain the nested syntax (item[subobject]) we - # need to fallback to nil. + # As @object_name may contain the nested syntax (item[subobject]) we need to fallback to nil. nil end + def retrieve_autoindex(pre_match) + object = self.object || @template_object.instance_variable_get("@#{pre_match}") + if object && object.respond_to?(:to_param) + object.to_param + else + raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}" + end + end + def value(object) self.class.value(object, @method_name) end @@ -1008,7 +1015,7 @@ module ActionView module ClassMethods def value(object, method_name) - object.send method_name unless object.nil? + object.send method_name if object end def value_before_type_cast(object, method_name) @@ -1237,7 +1244,7 @@ module ActionView end def emitted_hidden_id? - @emitted_hidden_id + @emitted_hidden_id ||= nil end private diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 43cbba8a0a..7d6aca0470 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -297,7 +297,6 @@ module ActionView def options_for_select(container, selected = nil) return container if String === container - container = container.to_a if Hash === container selected, disabled = extract_selected_and_disabled(selected).map do | r | Array.wrap(r).map(&:to_s) end @@ -395,12 +394,12 @@ module ActionView # <b>Note:</b> Only the <tt><optgroup></tt> and <tt><option></tt> tags are returned, so you still have to # wrap the output in an appropriate <tt><select></tt> tag. def option_groups_from_collection_for_select(collection, group_method, group_label_method, option_key_method, option_value_method, selected_key = nil) - collection.inject("") do |options_for_select, group| + collection.map do |group| group_label_string = eval("group.#{group_label_method}") - options_for_select += "<optgroup label=\"#{html_escape(group_label_string)}\">" - options_for_select += options_from_collection_for_select(eval("group.#{group_method}"), option_key_method, option_value_method, selected_key) - options_for_select += '</optgroup>' - end.html_safe + "<optgroup label=\"#{html_escape(group_label_string)}\">" + + options_from_collection_for_select(eval("group.#{group_method}"), option_key_method, option_value_method, selected_key) + + '</optgroup>' + end.join.html_safe end # Returns a string of <tt><option></tt> tags, like <tt>options_for_select</tt>, but diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 3bc5afc2c4..94348cf9fa 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -365,7 +365,7 @@ module ActionView # <% end %> def current_cycle(name = "default") cycle = get_cycle(name) - cycle.current_value unless cycle.nil? + cycle.current_value if cycle end # Resets a cycle so that it starts from the first element the next time diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index f8147840ed..1c3ca78d28 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -95,7 +95,7 @@ module ActionView # # => javascript:history.back() def url_for(options = {}) options ||= {} - url = case options + case options when String options when Hash @@ -106,8 +106,6 @@ module ActionView else polymorphic_path(options) end - - url end # Creates a link tag of the given +name+ using a URL created by the set @@ -586,20 +584,24 @@ module ActionView private def convert_options_to_data_attributes(options, html_options) - html_options = {} if html_options.nil? - html_options = html_options.stringify_keys + if html_options.nil? + link_to_remote_options?(options) ? {'data-remote' => 'true'} : {} + else + html_options = html_options.stringify_keys + html_options['data-remote'] = 'true' if link_to_remote_options?(options) || link_to_remote_options?(html_options) - if (options.is_a?(Hash) && options.key?('remote') && options.delete('remote')) || (html_options.is_a?(Hash) && html_options.key?('remote') && html_options.delete('remote')) - html_options['data-remote'] = 'true' - end + confirm = html_options.delete('confirm') + method = html_options.delete('method') - confirm = html_options.delete("confirm") - method, href = html_options.delete("method"), html_options['href'] + add_confirm_to_attributes!(html_options, confirm) if confirm + add_method_to_attributes!(html_options, method) if method - add_confirm_to_attributes!(html_options, confirm) if confirm - add_method_to_attributes!(html_options, method) if method + html_options + end + end - html_options + def link_to_remote_options?(options) + options.is_a?(Hash) && options.key?('remote') && options.delete('remote') end def add_confirm_to_attributes!(html_options, confirm) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index a999a0b7d7..164d177dcc 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -101,6 +101,8 @@ module ActionView attr_reader :source, :identifier, :handler, :virtual_path, :formats, :original_encoding + # This finalizer is needed (and exactly with a proc inside another proc) + # otherwise templates leak in development. Finalizer = proc do |method_name, mod| proc do mod.module_eval do diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index b1839b65e5..423e1e0bf5 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -52,6 +52,7 @@ module ActionView def initialize(template, assigns, original_exception) @template, @assigns, @original_exception = template, assigns.dup, original_exception + @sub_templates = nil @backtrace = original_exception.backtrace end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index c9e20ca14e..a261e08dbc 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -6,6 +6,7 @@ module ActionView # = Action View Resolver class Resolver def initialize + @path = nil @cached = Hash.new { |h1,k1| h1[k1] = Hash.new { |h2,k2| h2[k2] = Hash.new { |h3, k3| h3[k3] = {} } } } end diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index ff35fb7df4..915c2f90d7 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -20,12 +20,12 @@ module ActionView end def initialize + super self.class.controller_path = "" @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new @request.env.delete('PATH_INFO') - @params = {} end end @@ -127,6 +127,7 @@ module ActionView def say_no_to_protect_against_forgery! _helpers.module_eval do + remove_method :protect_against_forgery? if method_defined?(:protect_against_forgery?) def protect_against_forgery? false end @@ -136,8 +137,10 @@ module ActionView def make_test_case_available_to_view! test_case_instance = self _helpers.module_eval do - define_method(:_test_case) { test_case_instance } - private :_test_case + unless private_method_defined?(:_test_case) + define_method(:_test_case) { test_case_instance } + private :_test_case + end end end @@ -153,15 +156,15 @@ module ActionView # The instance of ActionView::Base that is used by +render+. def view @view ||= begin - view = ActionView::Base.new(ActionController::Base.view_paths, {}, @controller) - view.singleton_class.send :include, _helpers - view.singleton_class.send :include, @controller._routes.url_helpers - view.singleton_class.send :delegate, :alert, :notice, :to => "request.flash" - view.extend(Locals) - view.locals = self.locals - view.output_buffer = self.output_buffer - view - end + view = ActionView::Base.new(ActionController::Base.view_paths, {}, @controller) + view.singleton_class.send :include, _helpers + view.singleton_class.send :include, @controller._routes.url_helpers + view.singleton_class.send :delegate, :alert, :notice, :to => "request.flash" + view.extend(Locals) + view.locals = self.locals + view.output_buffer = self.output_buffer + view + end end alias_method :_view, :view @@ -198,7 +201,7 @@ module ActionView def method_missing(selector, *args) if @controller.respond_to?(:_routes) && - @controller._routes.named_routes.helpers.include?(selector) + @controller._routes.named_routes.helpers.include?(selector) @controller.__send__(selector, *args) else super diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index 97de2471cf..b2b62528a9 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/actionpack/lib/action_view/testing/resolvers.rb @@ -22,10 +22,10 @@ module ActionView #:nodoc: end templates = [] - @hash.select { |k,v| k =~ /^#{query}$/ }.each do |path, source| - handler, format = extract_handler_and_format(path, formats) - templates << Template.new(source, path, handler, - :virtual_path => path, :format => format) + @hash.select { |k,v| k =~ /^#{query}$/ }.each do |_path, source| + handler, format = extract_handler_and_format(_path, formats) + templates << Template.new(source, _path, handler, + :virtual_path => _path, :format => format) end templates.sort_by {|t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size } diff --git a/actionpack/test/abstract/callbacks_test.rb b/actionpack/test/abstract/callbacks_test.rb index b2d4d5f79a..2d02078020 100644 --- a/actionpack/test/abstract/callbacks_test.rb +++ b/actionpack/test/abstract/callbacks_test.rb @@ -47,6 +47,7 @@ module AbstractController end def index + @text ||= nil self.response_body = @text.to_s end end @@ -152,7 +153,7 @@ module AbstractController test "when :except is specified, an after filter is not triggered on that action" do result = @controller.process(:index) - assert_nil @controller.instance_variable_get("@authenticated") + assert !@controller.instance_variable_defined?("@authenticated") end end @@ -196,7 +197,7 @@ module AbstractController test "when :except is specified with an array, an after filter is not triggered on that action" do result = @controller.process(:index) - assert_nil @controller.instance_variable_get("@authenticated") + assert !@controller.instance_variable_defined?("@authenticated") end end @@ -204,6 +205,7 @@ module AbstractController before_filter :first, :only => :index def not_index + @text ||= nil self.response_body = @text.to_s end end diff --git a/actionpack/test/abstract/layouts_test.rb b/actionpack/test/abstract/layouts_test.rb index f580ad40f7..5ed6aa68b5 100644 --- a/actionpack/test/abstract/layouts_test.rb +++ b/actionpack/test/abstract/layouts_test.rb @@ -225,7 +225,7 @@ module AbstractControllerTests end test "when the layout is specified as a symbol and the method doesn't exist, raise an exception" do - assert_raises(NoMethodError) { WithSymbolAndNoMethod.new.process(:index) } + assert_raises(NameError) { WithSymbolAndNoMethod.new.process(:index) } end test "when the layout is specified as a symbol and the method returns something besides a string/false/nil, raise an exception" do diff --git a/actionpack/test/activerecord/controller_runtime_test.rb b/actionpack/test/activerecord/controller_runtime_test.rb index cfd86d704d..16fc901760 100644 --- a/actionpack/test/activerecord/controller_runtime_test.rb +++ b/actionpack/test/activerecord/controller_runtime_test.rb @@ -37,6 +37,6 @@ class ControllerRuntimeLogSubscriberTest < ActionController::TestCase wait assert_equal 2, @logger.logged(:info).size - assert_match /\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[1] + assert_match(/\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[1]) end -end
\ No newline at end of file +end diff --git a/actionpack/test/activerecord/render_partial_with_record_identification_test.rb b/actionpack/test/activerecord/render_partial_with_record_identification_test.rb index df50c3dc6f..43c534c111 100644 --- a/actionpack/test/activerecord/render_partial_with_record_identification_test.rb +++ b/actionpack/test/activerecord/render_partial_with_record_identification_test.rb @@ -93,38 +93,6 @@ class RenderPartialWithRecordIdentificationTest < ActiveRecordTestCase end end -class RenderPartialWithRecordIdentificationController < ActionController::Base - def render_with_has_many_and_belongs_to_association - @developer = Developer.find(1) - render :partial => @developer.projects - end - - def render_with_has_many_association - @topic = Topic.find(1) - render :partial => @topic.replies - end - - def render_with_has_many_through_association - @developer = Developer.find(:first) - render :partial => @developer.topics - end - - def render_with_belongs_to_association - @reply = Reply.find(1) - render :partial => @reply.topic - end - - def render_with_record - @developer = Developer.find(:first) - render :partial => @developer - end - - def render_with_record_collection - @developers = Developer.find(:all) - render :partial => @developers - end -end - class Game < Struct.new(:name, :id) extend ActiveModel::Naming include ActiveModel::Conversion diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 443191d4fa..5a8b763717 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -32,6 +32,8 @@ class ActionPackAssertionsController < ActionController::Base def redirect_to_path() redirect_to '/some/path' end + def redirect_invalid_external_route() redirect_to 'ht_tp://www.rubyonrails.org' end + def redirect_to_named_route() redirect_to route_one_url end def redirect_external() redirect_to "http://www.rubyonrails.org"; end @@ -234,13 +236,13 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_template_objects_exist process :assign_this - assert !@controller.instance_variable_get(:"@hi") + assert !@controller.instance_variable_defined?(:"@hi") assert @controller.instance_variable_get(:"@howdy") end def test_template_objects_missing process :nothing - assert_nil @controller.instance_variable_get(:@howdy) + assert !@controller.instance_variable_defined?(:@howdy) end def test_empty_flash @@ -314,7 +316,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_redirect_url_match process :redirect_external assert @response.redirect? - assert_match /rubyonrails/, @response.redirect_url + assert_match(/rubyonrails/, @response.redirect_url) assert !/perloffrails/.match(@response.redirect_url) end @@ -368,6 +370,11 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end end + def test_redirect_invalid_external_route + process :redirect_invalid_external_route + assert_redirected_to "http://test.hostht_tp://www.rubyonrails.org" + end + def test_redirected_to_url_full_url process :redirect_to_path assert_redirected_to 'http://test.host/some/path' diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index a83f5155f8..914ae56032 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -185,6 +185,7 @@ class ActionCachingTestController < CachingController def with_layout @cache_this = MockTime.now.to_f.to_s + @title = nil render :text => @cache_this, :layout => true end @@ -728,7 +729,7 @@ CACHED get :html_fragment_cached_with_partial assert_response :success assert_match(/Old fragment caching in a partial/, @response.body) - assert_match "Old fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial') + assert_match("Old fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial')) end def test_render_inline_before_fragment_caching @@ -736,14 +737,14 @@ CACHED assert_response :success assert_match(/Some inline content/, @response.body) assert_match(/Some cached content/, @response.body) - assert_match "Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached') + assert_match("Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached')) end def test_fragment_caching_in_rjs_partials xhr :get, :js_fragment_cached_with_partial assert_response :success assert_match(/Old fragment caching in a partial/, @response.body) - assert_match "Old fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial') + assert_match("Old fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial')) end def test_html_formatted_fragment_caching diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index 47253f22b8..d78acb8ce8 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -6,21 +6,29 @@ class CaptureController < ActionController::Base def self.controller_path; "test"; end def content_for + @title = nil render :layout => "talk_from_action" end def content_for_with_parameter + @title = nil render :layout => "talk_from_action" end def content_for_concatenated + @title = nil render :layout => "talk_from_action" end def non_erb_block_content_for + @title = nil render :layout => "talk_from_action" end + def proper_block_detection + @todo = "some todo" + end + def rescue_action(e) raise end end @@ -62,8 +70,8 @@ class CaptureTest < ActionController::TestCase end def test_proper_block_detection - @todo = "some todo" get :proper_block_detection + assert_equal "some todo", @response.body end private diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index cf4a461622..dfc90943e1 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -314,6 +314,7 @@ class FilterTest < ActionController::TestCase def initialize @@execution_log = "" + super() end before_filter { |c| c.class.execution_log << " before procfilter " } @@ -669,7 +670,7 @@ class FilterTest < ActionController::TestCase assert_equal %w( ensure_login find_user ), assigns["ran_filter"] test_process(ConditionalSkippingController, "login") - assert_nil @controller.instance_variable_get("@ran_after_filter") + assert !@controller.instance_variable_defined?("@ran_after_filter") test_process(ConditionalSkippingController, "change_password") assert_equal %w( clean_up ), @controller.instance_variable_get("@ran_after_filter") end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 7f7246d15c..10873708fe 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -23,7 +23,7 @@ module Another def with_fragment_cache render :inline => "<%= cache('foo'){ 'bar' } %>" end - + def with_fragment_cache_and_percent_in_key render :inline => "<%= cache('foo%bar'){ 'Contains % sign in key' } %>" end @@ -72,8 +72,8 @@ class ACLogSubscriberTest < ActionController::TestCase get :show wait assert_equal 2, logs.size - assert_match /Completed/, logs.last - assert_match /200 OK/, logs.last + assert_match(/Completed/, logs.last) + assert_match(/200 OK/, logs.last) end def test_process_action_without_parameters @@ -93,7 +93,7 @@ class ACLogSubscriberTest < ActionController::TestCase def test_process_action_with_view_runtime get :show wait - assert_match /\(Views: [\d\.]+ms\)/, logs[1] + assert_match(/\(Views: [\d\.]+ms\)/, logs[1]) end def test_process_action_with_filter_parameters @@ -103,9 +103,9 @@ class ACLogSubscriberTest < ActionController::TestCase wait params = logs[1] - assert_match /"amount"=>"\[FILTERED\]"/, params - assert_match /"lifo"=>"\[FILTERED\]"/, params - assert_match /"step"=>"1"/, params + assert_match(/"amount"=>"\[FILTERED\]"/, params) + assert_match(/"lifo"=>"\[FILTERED\]"/, params) + assert_match(/"step"=>"1"/, params) end def test_redirect_to @@ -121,7 +121,7 @@ class ACLogSubscriberTest < ActionController::TestCase wait assert_equal 3, logs.size - assert_match /Sent data file\.txt/, logs[1] + assert_match(/Sent data file\.txt/, logs[1]) end def test_send_file @@ -129,8 +129,8 @@ class ACLogSubscriberTest < ActionController::TestCase wait assert_equal 3, logs.size - assert_match /Sent file/, logs[1] - assert_match /test\/fixtures\/company\.rb/, logs[1] + assert_match(/Sent file/, logs[1]) + assert_match(/test\/fixtures\/company\.rb/, logs[1]) end def test_with_fragment_cache @@ -139,8 +139,8 @@ class ACLogSubscriberTest < ActionController::TestCase wait assert_equal 4, logs.size - assert_match /Exist fragment\? views\/foo/, logs[1] - assert_match /Write fragment views\/foo/, logs[2] + assert_match(/Exist fragment\? views\/foo/, logs[1]) + assert_match(/Write fragment views\/foo/, logs[2]) ensure @controller.config.perform_caching = true end @@ -163,8 +163,8 @@ class ACLogSubscriberTest < ActionController::TestCase wait assert_equal 3, logs.size - assert_match /Write page/, logs[1] - assert_match /\/index\.html/, logs[1] + assert_match(/Write page/, logs[1]) + assert_match(/\/index\.html/, logs[1]) ensure @controller.config.perform_caching = true end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 8c0af0dc30..adccfa028f 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -784,8 +784,8 @@ class RespondWithControllerTest < ActionController::TestCase get :using_resource_with_collection assert_equal "application/xml", @response.content_type assert_equal 200, @response.status - assert_match /<name>david<\/name>/, @response.body - assert_match /<name>jamis<\/name>/, @response.body + assert_match(/<name>david<\/name>/, @response.body) + assert_match(/<name>jamis<\/name>/, @response.body) end def test_using_resource_with_action diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index 5958b18d80..6dd2a9f23d 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -9,6 +9,10 @@ class RenderJsonTest < ActionController::TestCase hash.except!(*options[:except]) if options[:except] hash end + + def to_json(options = {}) + super :except => [:c, :e] + end end class TestController < ActionController::Base @@ -49,6 +53,10 @@ class RenderJsonTest < ActionController::TestCase def render_json_with_extra_options render :json => JsonRenderable.new, :except => [:c, :e] end + + def render_json_without_options + render :json => JsonRenderable.new + end end tests TestController @@ -109,4 +117,9 @@ class RenderJsonTest < ActionController::TestCase assert_equal '{"a":"b"}', @response.body assert_equal 'application/json', @response.content_type end + + def test_render_json_calls_to_json_from_object + get :render_json_without_options + assert_equal '{"a":"b"}', @response.body + end end diff --git a/actionpack/test/controller/render_other_test.rb b/actionpack/test/controller/render_other_test.rb index dfc4f2db8c..eda777e7a7 100644 --- a/actionpack/test/controller/render_other_test.rb +++ b/actionpack/test/controller/render_other_test.rb @@ -120,6 +120,7 @@ class RenderOtherTest < ActionController::TestCase private def default_render + @alternate_default_render ||= nil if @alternate_default_render @alternate_default_render.call else @@ -224,15 +225,15 @@ class RenderOtherTest < ActionController::TestCase get :update_page_with_instance_variables assert_template nil assert_equal 'text/javascript; charset=utf-8', @response.headers["Content-Type"] - assert_match /balance/, @response.body - assert_match /\$37/, @response.body + assert_match(/balance/, @response.body) + assert_match(/\$37/, @response.body) end def test_update_page_with_view_method get :update_page_with_view_method assert_template nil assert_equal 'text/javascript; charset=utf-8', @response.headers["Content-Type"] - assert_match /2 people/, @response.body + assert_match(/2 people/, @response.body) end def test_should_render_html_formatted_partial_with_rjs diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 42723c834e..7ca784c467 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -276,6 +276,7 @@ class TestController < ActionController::Base # :ported: def builder_layout_test + @name = nil render :action => "hello", :layout => "layouts/builder" end @@ -327,6 +328,7 @@ class TestController < ActionController::Base end def default_render + @alternate_default_render ||= nil if @alternate_default_render @alternate_default_render.call else @@ -339,14 +341,17 @@ class TestController < ActionController::Base end def layout_test_with_different_layout + @variable_for_layout = nil render :action => "hello_world", :layout => "standard" end def layout_test_with_different_layout_and_string_action + @variable_for_layout = nil render "hello_world", :layout => "standard" end def layout_test_with_different_layout_and_symbol_action + @variable_for_layout = nil render :hello_world, :layout => "standard" end @@ -355,6 +360,7 @@ class TestController < ActionController::Base end def layout_overriding_layout + @variable_for_layout = nil render :action => "hello_world", :layout => "standard" end @@ -643,6 +649,7 @@ class TestController < ActionController::Base private def determine_layout + @variable_for_layout ||= nil case action_name when "hello_world", "layout_test", "rendering_without_layout", "rendering_nothing_on_layout", "render_text_hello_world", @@ -1020,7 +1027,7 @@ class RenderTest < ActionController::TestCase assert_equal " ", @response.body end - def test_render_to_string + def test_render_to_string_not_deprecated assert_not_deprecated { get :hello_in_a_string } assert_equal "How's there? goodbyeHello: davidHello: marygoodbye\n", @response.body end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 5af25a0894..2c9aa6187b 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -55,26 +55,25 @@ module RequestForgeryProtectionTests ActionController::Base.request_forgery_protection_token = nil end - def test_should_render_form_with_token_tag - get :index - assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token - end - - def test_should_render_button_to_with_token_tag - get :show_button - assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token - end - - def test_should_allow_get - get :index - assert_response :success - end - - def test_should_allow_post_without_token_on_unsafe_action - post :unsafe - assert_response :success - end + get :index + assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token + end + + def test_should_render_button_to_with_token_tag + get :show_button + assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token + end + + def test_should_allow_get + get :index + assert_response :success + end + + def test_should_allow_post_without_token_on_unsafe_action + post :unsafe + assert_response :success + end def test_should_not_allow_html_post_without_token @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index cefbc548c0..a2418bb7c0 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -3,10 +3,12 @@ require 'abstract_unit' module ActionDispatch class ShowExceptions private + remove_method :public_path def public_path "#{FIXTURE_LOAD_PATH}/public" end + remove_method :logger # Silence logger def logger nil diff --git a/actionpack/test/controller/selector_test.rb b/actionpack/test/controller/selector_test.rb index 23ccbf6987..8ce9e43402 100644 --- a/actionpack/test/controller/selector_test.rb +++ b/actionpack/test/controller/selector_test.rb @@ -471,7 +471,7 @@ class SelectorTest < Test::Unit::TestCase end - def test_first_and_last + def test_only_child_and_only_type_first_and_last # Only child. parse(%Q{<table><tr></tr></table>}) select("table:only-child") diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index e90fc49542..edda0d0a30 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -591,7 +591,7 @@ XML assert false, "expected RuntimeError, got nothing" rescue RuntimeError => error assert true - assert_match %r{@#{variable} is nil}, error.message + assert_match(%r{@#{variable} is nil}, error.message) rescue => error assert false, "expected RuntimeError, got #{error.class}" end diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 2d0c019128..4c07ca4cc3 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -219,7 +219,7 @@ module AbstractController def test_hash_recursive_and_array_parameters url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :id => 101, :query => {:person => {:name => 'Bob', :position => ['prof', 'art director']}, :hobby => 'piercing'}) - assert_match %r(^/c/a/101), url + assert_match(%r(^/c/a/101), url) params = extract_params(url) assert_equal params[0], { 'query[hobby]' => 'piercing' }.to_query assert_equal params[1], { 'query[person][name]' => 'Bob' }.to_query diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 5564775a48..6ba4c6c48d 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -24,6 +24,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def setup @controller = TestController.new + @integration_session = nil end def test_check_parameters diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 360fb351df..efdc1f5d93 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -47,7 +47,7 @@ class CookiesTest < ActionController::TestCase cookies["user_name"] = { :value => "david", :httponly => true } head :ok end - + def authenticate_with_secure cookies["user_name"] = { :value => "david", :secure => true } head :ok @@ -133,7 +133,7 @@ class CookiesTest < ActionController::TestCase assert_cookie_header "user_name=david; path=/; HttpOnly" assert_equal({"user_name" => "david"}, @response.cookies) end - + def test_setting_cookie_with_secure get :authenticate_with_secure assert_cookie_header "user_name=david; path=/; secure" @@ -169,8 +169,8 @@ class CookiesTest < ActionController::TestCase def test_permanent_cookie get :set_permanent_cookie - assert_match /Jamie/, @response.headers["Set-Cookie"] - assert_match %r(#{20.years.from_now.utc.year}), @response.headers["Set-Cookie"] + assert_match(/Jamie/, @response.headers["Set-Cookie"]) + assert_match(%r(#{20.years.from_now.utc.year}), @response.headers["Set-Cookie"]) end def test_signed_cookie @@ -185,7 +185,7 @@ class CookiesTest < ActionController::TestCase def test_permanent_signed_cookie get :set_permanent_signed_cookie - assert_match %r(#{20.years.from_now.utc.year}), @response.headers["Set-Cookie"] + assert_match(%r(#{20.years.from_now.utc.year}), @response.headers["Set-Cookie"]) assert_equal 100, @controller.send(:cookies).signed[:remember_me] end diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 369212e2d0..4c2b95550c 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -41,7 +41,6 @@ class MimeTypeTest < ActiveSupport::TestCase begin Mime::Type.register("image/gif", :gif) assert_nothing_raised do - Mime::GIF assert_equal Mime::GIF, Mime::SET.last end ensure diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 26d76557dd..18f28deee4 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -1,8 +1,23 @@ require 'abstract_unit' +require 'rack/test' module TestGenerationPrefix + class Post + extend ActiveModel::Naming + + def to_param + "1" + end + + def self.model_name + klass = "Post" + def klass.name; self end + + ActiveModel::Name.new(klass) + end + end + class WithMountedEngine < ActionDispatch::IntegrationTest - require 'rack/test' include Rack::Test::Methods class BlogEngine @@ -55,21 +70,6 @@ module TestGenerationPrefix # force draw RailsApplication.routes - class Post - extend ActiveModel::Naming - - def to_param - "1" - end - - def self.model_name - klass = "Post" - def klass.name; self end - - ActiveModel::Name.new(klass) - end - end - class ::InsideEngineGeneratingController < ActionController::Base include BlogEngine.routes.url_helpers include RailsApplication.routes.mounted_helpers @@ -253,4 +253,65 @@ module TestGenerationPrefix assert_equal "http://www.example.com/awesome/blog/posts/1", path end end + + class EngineMountedAtRoot < ActionDispatch::IntegrationTest + include Rack::Test::Methods + + class BlogEngine + def self.routes + @routes ||= begin + routes = ActionDispatch::Routing::RouteSet.new + routes.draw do + match "/posts/:id", :to => "posts#show", :as => :post + end + + routes + end + end + + def self.call(env) + env['action_dispatch.routes'] = routes + routes.call(env) + end + end + + class RailsApplication + def self.routes + @routes ||= begin + routes = ActionDispatch::Routing::RouteSet.new + routes.draw do + mount BlogEngine => "/" + end + + routes + end + end + + def self.call(env) + env['action_dispatch.routes'] = routes + routes.call(env) + end + end + + # force draw + RailsApplication.routes + + class ::PostsController < ActionController::Base + include BlogEngine.routes.url_helpers + include RailsApplication.routes.mounted_helpers + + def show + render :text => post_path(:id => params[:id]) + end + end + + def app + RailsApplication + end + + test "generating path inside engine" do + get "/posts/1" + assert_equal "/posts/1", last_response.body + end + end end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 1ae514ebf7..073dd3ddad 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -68,7 +68,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type - assert ('a' * 20480) == file.read + assert_equal(('a' * 20480), file.read) end test "parses binary file" do diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb index 16669614d6..ad9de02eb4 100644 --- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb @@ -18,6 +18,7 @@ class XmlParamsParsingTest < ActionDispatch::IntegrationTest test "parses a strict rack.input" do class Linted + undef call if method_defined?(:call) def call(env) bar = env['action_dispatch.request.request_parameters']['foo'] result = "<ok>#{bar}</ok>" diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index a8b8f9377b..3efed8bef6 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -45,9 +45,9 @@ class RequestTest < ActiveSupport::TestCase e = assert_raise(ActionDispatch::RemoteIp::IpSpoofAttackError) { request.remote_ip } - assert_match /IP spoofing attack/, e.message - assert_match /HTTP_X_FORWARDED_FOR="1.1.1.1"/, e.message - assert_match /HTTP_CLIENT_IP="2.2.2.2"/, e.message + assert_match(/IP spoofing attack/, e.message) + assert_match(/HTTP_X_FORWARDED_FOR="1.1.1.1"/, e.message) + assert_match(/HTTP_CLIENT_IP="2.2.2.2"/, e.message) # turn IP Spoofing detection off. # This is useful for sites that are aimed at non-IP clients. The typical @@ -474,6 +474,7 @@ protected def stub_request(env = {}) ip_spoofing_check = env.key?(:ip_spoofing_check) ? env.delete(:ip_spoofing_check) : true + @trusted_proxies ||= nil ip_app = ActionDispatch::RemoteIp.new(Proc.new { }, ip_spoofing_check, @trusted_proxies) tld_length = env.key?(:tld_length) ? env.delete(:tld_length) : 1 ip_app.call(env) diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index c20fa10f63..cd0418c338 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -120,10 +120,10 @@ class ResponseTest < ActiveSupport::TestCase end test "read cache control" do - resp = ActionDispatch::Response.new.tap { |resp| - resp.cache_control[:public] = true - resp.etag = '123' - resp.body = 'Hello' + resp = ActionDispatch::Response.new.tap { |response| + response.cache_control[:public] = true + response.etag = '123' + response.body = 'Hello' } resp.to_a @@ -135,10 +135,10 @@ class ResponseTest < ActiveSupport::TestCase end test "read charset and content type" do - resp = ActionDispatch::Response.new.tap { |resp| - resp.charset = 'utf-16' - resp.content_type = Mime::XML - resp.body = 'Hello' + resp = ActionDispatch::Response.new.tap { |response| + response.charset = 'utf-16' + response.content_type = Mime::XML + response.body = 'Hello' } resp.to_a diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index bdef04efc6..5c188a60c7 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -442,6 +442,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get :preview, :on => :member end + scope :as => "routes" do + get "/c/:id", :as => :collision, :to => "collision#show" + get "/collision", :to => "collision#show" + get "/no_collision", :to => "collision#show", :as => nil + + get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" + get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" + end + match '/purchases/:token/:filename', :to => 'purchases#fetch', :token => /[[:alnum:]]{10}/, @@ -1216,14 +1225,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_index - with_test_routes do - assert_equal '/info', info_path - get '/info' - assert_equal 'projects#info', @response.body - end - end - def test_match_shorthand_with_no_scope with_test_routes do assert_equal '/account/overview', account_overview_path @@ -2128,6 +2129,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_raises(ActionController::RoutingError){ list_todo_path(:list_id => '2', :id => '1') } end + def test_named_routes_collision_is_avoided_unless_explicitly_given_as + assert_equal "/c/1", routes_collision_path(1) + assert_equal "/forced_collision", routes_forced_collision_path + end + + def test_explicitly_avoiding_the_named_route + assert !respond_to?(:routes_no_collision_path) + end + def test_controller_name_with_leading_slash_raise_error assert_raise(ArgumentError) do self.class.stub_controllers do |routes| diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index b436fb5518..3489f628ed 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -279,7 +279,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_session_store_with_explicit_domain with_test_route_set(:domain => "example.es") do get '/set_session_value' - assert_match /domain=example\.es/, headers['Set-Cookie'] + assert_match(/domain=example\.es/, headers['Set-Cookie']) headers['Set-Cookie'] end end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 5b181600e8..4ede1ab47c 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -58,15 +58,15 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 - assert_match /puke/, body + assert_match(/puke/, body) get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 - assert_match /#{ActionController::UnknownAction.name}/, body + assert_match(/#{ActionController::UnknownAction.name}/, body) get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 - assert_match /ActionController::MethodNotAllowed/, body + assert_match(/ActionController::MethodNotAllowed/, body) end end @@ -96,15 +96,15 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 - assert_match /puke/, body + assert_match(/puke/, body) get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 - assert_match /#{ActionController::UnknownAction.name}/, body + assert_match(/#{ActionController::UnknownAction.name}/, body) get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 - assert_match /ActionController::MethodNotAllowed/, body + assert_match(/ActionController::MethodNotAllowed/, body) end test "does not show filtered parameters" do @@ -113,6 +113,6 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest get "/", {"foo"=>"bar"}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.parameter_filter' => [:foo]} assert_response 500 - assert_match ""foo"=>"[FILTERED]"", body + assert_match(""foo"=>"[FILTERED]"", body) end end diff --git a/actionpack/test/fixtures/alternate_helpers/foo_helper.rb b/actionpack/test/fixtures/alternate_helpers/foo_helper.rb index a956fce6fa..2528584473 100644 --- a/actionpack/test/fixtures/alternate_helpers/foo_helper.rb +++ b/actionpack/test/fixtures/alternate_helpers/foo_helper.rb @@ -1,3 +1,3 @@ module FooHelper - def baz() end + redefine_method(:baz) {} end diff --git a/actionpack/test/fixtures/test/hello_world_from_rxml.builder b/actionpack/test/fixtures/test/hello_world_from_rxml.builder index 8455b11edc..619a97ba96 100644 --- a/actionpack/test/fixtures/test/hello_world_from_rxml.builder +++ b/actionpack/test/fixtures/test/hello_world_from_rxml.builder @@ -1,4 +1,3 @@ xml.html do xml.p "Hello" end -"String return value" diff --git a/actionpack/test/fixtures/test/proper_block_detection.erb b/actionpack/test/fixtures/test/proper_block_detection.erb index 23564dbcee..b55efbb25d 100644 --- a/actionpack/test/fixtures/test/proper_block_detection.erb +++ b/actionpack/test/fixtures/test/proper_block_detection.erb @@ -1 +1 @@ -<%= @todo %> +<%= @todo %>
\ No newline at end of file diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index ec28d4442c..3abcdfbc1e 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -975,7 +975,7 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase def test_should_wildcard_asset_host_between_zero_and_four @controller.config.asset_host = 'http://a%d.example.com' - assert_match %r(http://a[0123].example.com/collaboration/hieraki/images/xml.png), image_path('xml.png') + assert_match(%r(http://a[0123].example.com/collaboration/hieraki/images/xml.png), image_path('xml.png')) end def test_asset_host_without_protocol_should_use_request_protocol diff --git a/actionpack/test/template/atom_feed_helper_test.rb b/actionpack/test/template/atom_feed_helper_test.rb index 9f0a975255..36102bbc4f 100644 --- a/actionpack/test/template/atom_feed_helper_test.rb +++ b/actionpack/test/template/atom_feed_helper_test.rb @@ -203,7 +203,7 @@ class AtomFeedTest < ActionController::TestCase def test_feed_should_use_default_language_if_none_is_given with_restful_routing(:scrolls) do get :index, :id => "defaults" - assert_match %r{xml:lang="en-US"}, @response.body + assert_match(%r{xml:lang="en-US"}, @response.body) end end diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index ea7c096b97..0cf7885772 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -1497,26 +1497,6 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, date_select("post", "written_on", :order=>[:year, :month], :include_blank=>true) end - def test_date_select_with_nil_and_blank_and_order - @post = Post.new - - start_year = Time.now.year-5 - end_year = Time.now.year+5 - - expected = '<input name="post[written_on(3i)]" type="hidden" id="post_written_on_3i"/>' + "\n" - expected << %{<select id="post_written_on_1i" name="post[written_on(1i)]">\n} - expected << "<option value=\"\"></option>\n" - start_year.upto(end_year) { |i| expected << %(<option value="#{i}">#{i}</option>\n) } - expected << "</select>\n" - - expected << %{<select id="post_written_on_2i" name="post[written_on(2i)]">\n} - expected << "<option value=\"\"></option>\n" - 1.upto(12) { |i| expected << %(<option value="#{i}">#{Date::MONTHNAMES[i]}</option>\n) } - expected << "</select>\n" - - assert_dom_equal expected, date_select("post", "written_on", :order=>[:year, :month], :include_blank=>true) - end - def test_date_select_cant_override_discard_hour @post = Post.new @post.written_on = Date.new(2004, 6, 15) diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index b3fae4d564..abc98ebe69 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -106,7 +106,6 @@ class FormHelperTest < ActionView::TestCase if object.is_a?(Hash) && object[:use_route].blank? && object[:controller].blank? object.merge!(:controller => "main", :action => "index") end - object super end @@ -269,7 +268,7 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, hidden_field("post", "title", :value => nil) end - def test_text_field_with_options + def test_hidden_field_with_options assert_dom_equal '<input id="post_title" name="post[title]" type="hidden" value="Something Else" />', hidden_field("post", "title", :value => "Something Else") end diff --git a/actionpack/test/template/html-scanner/tag_node_test.rb b/actionpack/test/template/html-scanner/tag_node_test.rb index 9c8fcdc8fc..0d87f1bd42 100644 --- a/actionpack/test/template/html-scanner/tag_node_test.rb +++ b/actionpack/test/template/html-scanner/tag_node_test.rb @@ -55,7 +55,7 @@ class TagNodeTest < Test::Unit::TestCase def test_to_s node = tag("<a b=c d='f' g=\"h 'i'\" />") - assert_equal %(<a b='c' d='f' g='h \\'i\\'' />), node.to_s + assert_equal %(<a b="c" d="f" g="h 'i'" />), node.to_s end def test_tag @@ -221,7 +221,7 @@ class TagNodeTest < Test::Unit::TestCase assert !m.match(:after => {:tag => "span", :attributes => {:k => true}}) end - def test_to_s + def test_tag_to_s t = tag("<b x='foo'>") tag("hello", t) tag("<hr />", t) diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb index a8ca19931b..2e7484afaf 100644 --- a/actionpack/test/template/javascript_helper_test.rb +++ b/actionpack/test/template/javascript_helper_test.rb @@ -22,8 +22,6 @@ class JavaScriptHelperTest < ActionView::TestCase ActiveSupport.escape_html_entities_in_json = false end - def _evaluate_assigns_and_ivars() end - def test_escape_javascript assert_equal '', escape_javascript(nil) assert_equal %(This \\"thing\\" is really\\n netos\\'), escape_javascript(%(This "thing" is really\n netos')) diff --git a/actionpack/test/template/log_subscriber_test.rb b/actionpack/test/template/log_subscriber_test.rb index eb1e548672..6fb8d39818 100644 --- a/actionpack/test/template/log_subscriber_test.rb +++ b/actionpack/test/template/log_subscriber_test.rb @@ -29,7 +29,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered test\/hello_world\.erb/, @logger.logged(:info).last + assert_match(/Rendered test\/hello_world\.erb/, @logger.logged(:info).last) end def test_render_text_template @@ -37,7 +37,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered text template/, @logger.logged(:info).last + assert_match(/Rendered text template/, @logger.logged(:info).last) end def test_render_inline_template @@ -45,7 +45,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered inline template/, @logger.logged(:info).last + assert_match(/Rendered inline template/, @logger.logged(:info).last) end def test_render_partial_template @@ -53,7 +53,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered test\/_customer.erb/, @logger.logged(:info).last + assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) end def test_render_partial_with_implicit_path @@ -62,7 +62,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last + assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) end def test_render_collection_template @@ -70,7 +70,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered test\/_customer.erb/, @logger.logged(:info).last + assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) end def test_render_collection_with_implicit_path @@ -79,7 +79,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last + assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) end def test_render_collection_template_without_path @@ -88,6 +88,6 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match /Rendered collection/, @logger.logged(:info).last + assert_match(/Rendered collection/, @logger.logged(:info).last) end -end
\ No newline at end of file +end diff --git a/actionpack/test/template/number_helper_i18n_test.rb b/actionpack/test/template/number_helper_i18n_test.rb index 8561019461..c82ead663f 100644 --- a/actionpack/test/template/number_helper_i18n_test.rb +++ b/actionpack/test/template/number_helper_i18n_test.rb @@ -41,7 +41,7 @@ class NumberHelperTest < ActionView::TestCase :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"} end - def test_number_to_currency + def test_number_to_i18n_currency assert_equal("&$ - 10.00", number_to_currency(10, :locale => 'ts')) end @@ -51,7 +51,7 @@ class NumberHelperTest < ActionView::TestCase end end - def test_number_with_precision + def test_number_with_i18n_precision #Delimiter was set to "" assert_equal("10000", number_with_precision(10000, :locale => 'ts')) @@ -60,12 +60,12 @@ class NumberHelperTest < ActionView::TestCase end - def test_number_with_delimiter + def test_number_with_i18n_delimiter #Delimiter "," and separator "." assert_equal("1,000,000.234", number_with_delimiter(1000000.234, :locale => 'ts')) end - def test_number_to_percentage + def test_number_to_i18n_percentage # to see if strip_insignificant_zeros is true assert_equal("1%", number_to_percentage(1, :locale => 'ts')) # precision is 2, significant should be inherited @@ -74,7 +74,7 @@ class NumberHelperTest < ActionView::TestCase assert_equal("12434%", number_to_percentage(12434, :locale => 'ts')) end - def test_number_to_human_size + def test_number_to_i18n_human_size #b for bytes and k for kbytes assert_equal("2 k", number_to_human_size(2048, :locale => 'ts')) assert_equal("42 b", number_to_human_size(42, :locale => 'ts')) diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb index 036a44730c..a6aa848a00 100644 --- a/actionpack/test/template/prototype_helper_test.rb +++ b/actionpack/test/template/prototype_helper_test.rb @@ -71,7 +71,7 @@ class PrototypeHelperBaseTest < ActionView::TestCase end def create_generator - block = Proc.new { |*args| yield *args if block_given? } + block = Proc.new { |*args| yield(*args) if block_given? } JavaScriptGenerator.new self, &block end end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index c17bec891b..205fdcf345 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -319,10 +319,11 @@ class LazyViewRenderTest < ActiveSupport::TestCase end def with_external_encoding(encoding) - old, Encoding.default_external = Encoding.default_external, encoding + old = Encoding.default_external + silence_warnings { Encoding.default_external = encoding } yield ensure - Encoding.default_external = old + silence_warnings { Encoding.default_external = old } end end end diff --git a/actionpack/test/template/tag_helper_test.rb b/actionpack/test/template/tag_helper_test.rb index 85ac515660..c742683821 100644 --- a/actionpack/test/template/tag_helper_test.rb +++ b/actionpack/test/template/tag_helper_test.rb @@ -11,8 +11,8 @@ class TagHelperTest < ActionView::TestCase def test_tag_options str = tag("p", "class" => "show", :class => "elsewhere") - assert_match /class="show"/, str - assert_match /class="elsewhere"/, str + assert_match(/class="show"/, str) + assert_match(/class="elsewhere"/, str) end def test_tag_options_rejects_nil_option @@ -103,7 +103,7 @@ class TagHelperTest < ActionView::TestCase def test_skip_invalid_escaped_attributes ['&1;', 'dfa3;', '& #123;'].each do |escaped| - assert_equal %(<a href="#{escaped.gsub /&/, '&'}" />), tag('a', :href => escaped) + assert_equal %(<a href="#{escaped.gsub(/&/, '&')}" />), tag('a', :href => escaped) end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index fbc9350c69..c7c33af670 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -6,6 +6,7 @@ class TestERBTemplate < ActiveSupport::TestCase class Context def initialize @output_buffer = "original" + @_virtual_path = nil end def hello @@ -82,12 +83,11 @@ class TestERBTemplate < ActiveSupport::TestCase # is set to something other than UTF-8, we don't # get any errors and get back a UTF-8 String. def test_default_external_works - Encoding.default_external = "ISO-8859-1" - @template = new_template("hello \xFCmlat") - assert_equal Encoding::UTF_8, render.encoding - assert_equal "hello \u{fc}mlat", render - ensure - Encoding.default_external = "UTF-8" + with_external_encoding "ISO-8859-1" do + @template = new_template("hello \xFCmlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + end end def test_encoding_can_be_specified_with_magic_comment @@ -123,10 +123,11 @@ class TestERBTemplate < ActiveSupport::TestCase end def with_external_encoding(encoding) - old, Encoding.default_external = Encoding.default_external, encoding + old = Encoding.default_external + silence_warnings { Encoding.default_external = encoding } yield ensure - Encoding.default_external = old + silence_warnings { Encoding.default_external = old } end end end diff --git a/actionpack/test/template/test_case_test.rb b/actionpack/test/template/test_case_test.rb index f766c2c0b6..8526db61cc 100644 --- a/actionpack/test/template/test_case_test.rb +++ b/actionpack/test/template/test_case_test.rb @@ -112,7 +112,7 @@ module ActionView @controller.controller_path = 'test' @customers = [stub(:name => 'Eloy'), stub(:name => 'Manfred')] - assert_match /Hello: EloyHello: Manfred/, render(:partial => 'test/from_helper') + assert_match(/Hello: EloyHello: Manfred/, render(:partial => 'test/from_helper')) end end @@ -201,7 +201,7 @@ module ActionView @controller.controller_path = "test" @customers = [stub(:name => 'Eloy'), stub(:name => 'Manfred')] - assert_match /Hello: EloyHello: Manfred/, render(:file => 'test/list') + assert_match(/Hello: EloyHello: Manfred/, render(:file => 'test/list')) end test "is able to render partials from templates and also use instance variables after view has been referenced" do @@ -210,7 +210,7 @@ module ActionView view @customers = [stub(:name => 'Eloy'), stub(:name => 'Manfred')] - assert_match /Hello: EloyHello: Manfred/, render(:file => 'test/list') + assert_match(/Hello: EloyHello: Manfred/, render(:file => 'test/list')) end end diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 88ec6fc740..9e9ed9120d 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -491,7 +491,7 @@ class TextHelperTest < ActionView::TestCase url = "http://api.rubyonrails.com/Foo.html" email = "fantabulous@shiznadel.ic" - assert_equal %(<p><a href="#{url}">#{url[0...7]}...</a><br /><a href="mailto:#{email}">#{email[0...7]}...</a><br /></p>), auto_link("<p>#{url}<br />#{email}<br /></p>") { |url| truncate(url, :length => 10) } + assert_equal %(<p><a href="#{url}">#{url[0...7]}...</a><br /><a href="mailto:#{email}">#{email[0...7]}...</a><br /></p>), auto_link("<p>#{url}<br />#{email}<br /></p>") { |_url| truncate(_url, :length => 10) } end def test_auto_link_with_block_with_html diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index fc2abacb6d..2d580fd325 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -73,8 +73,10 @@ module ActiveModel # Returns an ActiveModel::Name object for module. It can be # used to retrieve all kinds of naming-related information. def model_name - namespace = self.parents.detect { |n| n.respond_to?(:_railtie) } - @_model_name ||= ActiveModel::Name.new(self, namespace) + @_model_name ||= begin + namespace = self.parents.detect { |n| n.respond_to?(:_railtie) } + ActiveModel::Name.new(self, namespace) + end end # Returns the plural class name of a record or class. Examples: diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index 2c8a840124..00df10cef0 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -10,7 +10,9 @@ module ActiveModel end def setup(klass) - klass.send(:attr_accessor, *attributes.map { |attribute| :"#{attribute}_confirmation" }) + klass.send(:attr_accessor, *attributes.map do |attribute| + :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation") + end.compact) end end diff --git a/activemodel/test/cases/callbacks_test.rb b/activemodel/test/cases/callbacks_test.rb index 64dc7b5026..069d907fb2 100644 --- a/activemodel/test/cases/callbacks_test.rb +++ b/activemodel/test/cases/callbacks_test.rb @@ -81,4 +81,34 @@ class CallbacksTest < ActiveModel::TestCase assert !ModelCallbacks.respond_to?(:around_empty) assert !ModelCallbacks.respond_to?(:after_empty) end + + class Violin + attr_reader :history + def initialize + @history = [] + end + extend ActiveModel::Callbacks + define_model_callbacks :create + def callback1; self.history << 'callback1'; end + def callback2; self.history << 'callback2'; end + def create + _run_create_callbacks {} + self + end + end + class Violin1 < Violin + after_create :callback1, :callback2 + end + class Violin2 < Violin + after_create :callback1 + after_create :callback2 + end + + test "after_create callbacks with both callbacks declared in one line" do + assert_equal ["callback1", "callback2"], Violin1.new.create.history + end + test "after_create callbacks with both callbacks declared in differnt lines" do + assert_equal ["callback1", "callback2"], Violin2.new.create.history + end + end diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 1387d6e288..b8a980b1fb 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -23,6 +23,6 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) - s.add_dependency('arel', '~> 1.0.1') + s.add_dependency('arel', '~> 2.0.0') s.add_dependency('tzinfo', '~> 0.3.23') end diff --git a/activerecord/examples/performance.rb b/activerecord/examples/performance.rb index ccd60c6c69..c4ce361b5d 100644 --- a/activerecord/examples/performance.rb +++ b/activerecord/examples/performance.rb @@ -88,7 +88,7 @@ else ) end - mysqldump_bin = %w[mysqldump mysqldump5].select { |bin| `which #{bin}`.length > 0 } + mysqldump_bin = %w[mysqldump mysqldump5].detect { |bin| `which #{bin}`.length > 0 } `#{mysqldump_bin} -u #{conn[:username]} #{"-p#{conn[:password]}" unless conn[:password].blank?} #{conn[:database]} exhibits users > #{sqlfile}` end diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 715c868598..e6b367790b 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -321,14 +321,14 @@ module ActiveRecord klasses_and_ids[reflection.klass.name] = id_map unless id_map.empty? end - klasses_and_ids.each do |klass_name, id_map| + klasses_and_ids.each do |klass_name, _id_map| klass = klass_name.constantize table_name = klass.quoted_table_name primary_key = reflection.options[:primary_key] || klass.primary_key column_type = klass.columns.detect{|c| c.name == primary_key}.type - ids = id_map.keys.map do |id| + ids = _id_map.keys.map do |id| if column_type == :integer id.to_i elsif column_type == :float @@ -343,7 +343,7 @@ module ActiveRecord associated_records = klass.unscoped.where([conditions, ids]).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a - set_association_single_records(id_map, reflection.name, associated_records, primary_key) + set_association_single_records(_id_map, reflection.name, associated_records, primary_key) end end diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 38454ec242..e429806b0c 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -39,7 +39,7 @@ module ActiveRecord def set_inverse_instance(record, instance) return if record.nil? || !we_can_set_the_inverse_on_this?(record) inverse_relationship = @reflection.polymorphic_inverse_of(record.class) - unless inverse_relationship.nil? + if inverse_relationship record.send(:"set_#{inverse_relationship.name}_target", instance) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index f5291b180e..2157a0aded 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1014,8 +1014,9 @@ module ActiveRecord #:nodoc: end def all_attributes_exists?(attribute_names) - attribute_names = expand_attribute_names_for_aggregates(attribute_names) - attribute_names.all? { |name| column_methods_hash.include?(name.to_sym) } + expand_attribute_names_for_aggregates(attribute_names).all? { |name| + column_methods_hash.include?(name.to_sym) + } end protected @@ -1383,10 +1384,7 @@ MSG ensure_proper_type - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| self.send("#{att}=", value) } if create_with - end + populate_with_current_scope_attributes self.attributes = attributes unless attributes.nil? result = yield self if block_given? @@ -1415,10 +1413,7 @@ MSG @new_record = true ensure_proper_type - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| self.send("#{att}=", value) } if create_with - end + populate_with_current_scope_attributes end # Returns a String, which Action Pack uses for constructing an URL to this @@ -1807,6 +1802,13 @@ MSG return string unless string.is_a?(String) && string =~ /^---/ YAML::load(string) rescue string end + + def populate_with_current_scope_attributes + if scope = self.class.send(:current_scoped_methods) + create_with = scope.scope_for_create + create_with.each { |att,value| self.respond_to?(:"#{att}=") && self.send("#{att}=", value) } if create_with + end + end end Base.class_eval do diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 25432e9985..646a78622c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -35,7 +35,7 @@ module ActiveRecord undef_method :select_rows # Executes the SQL statement in the context of this connection. - def execute(sql, name = nil, skip_logging = false) + def execute(sql, name = nil) end undef_method :execute diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 84fc4c03f9..6480aeb171 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -318,21 +318,13 @@ module ActiveRecord @base = base end - #Handles non supported datatypes - e.g. XML - def method_missing(symbol, *args) - if symbol.to_s == 'xml' - xml_column_fallback(args) - else - super - end - end + def xml(*args) + raise NotImplementedError unless %w{ + sqlite mysql mysql2 + }.include? @base.adapter_name.downcase - def xml_column_fallback(*args) - case @base.adapter_name.downcase - when 'sqlite', 'mysql' - options = args.extract_options! - column(args[0], :text, options) - end + options = args.extract_options! + column(args[0], :text, options) end # Appends a primary key definition to the table definition. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 310423bb20..4e770c37da 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -151,10 +151,10 @@ module ActiveRecord # # See also TableDefinition#column for details on how to create columns. def create_table(table_name, options = {}) - table_definition = TableDefinition.new(self) - table_definition.primary_key(options[:primary_key] || Base.get_primary_key(table_name.to_s.singularize)) unless options[:id] == false + td = table_definition + td.primary_key(options[:primary_key] || Base.get_primary_key(table_name.to_s.singularize)) unless options[:id] == false - yield table_definition if block_given? + yield td if block_given? if options[:force] && table_exists?(table_name) drop_table(table_name, options) @@ -162,7 +162,7 @@ module ActiveRecord create_sql = "CREATE#{' TEMPORARY' if options[:temporary]} TABLE " create_sql << "#{quote_table_name(table_name)} (" - create_sql << table_definition.to_sql + create_sql << td.to_sql create_sql << ") #{options[:options]}" execute create_sql end @@ -327,14 +327,12 @@ module ActiveRecord # # Note: SQLite doesn't support index length def add_index(table_name, column_name, options = {}) - options[:name] = options[:name].to_s if options.key?(:name) - column_names = Array.wrap(column_name) index_name = index_name(table_name, :column => column_names) if Hash === options # legacy support, since this param was a string index_type = options[:unique] ? "UNIQUE" : "" - index_name = options[:name] || index_name + index_name = options[:name].to_s if options.key?(:name) else index_type = options end @@ -404,6 +402,7 @@ module ActiveRecord # as there's no way to determine the correct answer in that case. def index_name_exists?(table_name, index_name, default) return default unless respond_to?(:indexes) + index_name = index_name.to_s indexes(table_name).detect { |i| i.name == index_name } end @@ -535,6 +534,11 @@ module ActiveRecord def options_include_default?(options) options.include?(:default) && !(options[:null] == false && options[:default].nil?) end + + private + def table_definition + TableDefinition.new(self) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0a2bacdb84..194842a9a0 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1,20 +1,19 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_support/core_ext/kernel/requires' require 'active_support/core_ext/object/blank' +require 'pg' module ActiveRecord class Base # Establishes a connection to the database that's used by all Active Record objects def self.postgresql_connection(config) # :nodoc: - require 'pg' - config = config.symbolize_keys host = config[:host] port = config[:port] || 5432 username = config[:username].to_s if config[:username] password = config[:password].to_s if config[:password] - if config.has_key?(:database) + if config.key?(:database) database = config[:database] else raise ArgumentError, "No database specified. Missing argument: database." @@ -27,12 +26,6 @@ module ActiveRecord end module ConnectionAdapters - class TableDefinition - def xml(*args) - options = args.extract_options! - column(args[0], 'xml', options) - end - end # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: # Instantiates a new PostgreSQL column definition in a table. @@ -170,9 +163,7 @@ module ActiveRecord end end end - end - module ConnectionAdapters # The PostgreSQL adapter works both with the native C (http://ruby.scripting.ca/postgres/) and the pure # Ruby (available both as gem and from http://rubyforge.org/frs/?group_id=234&release_id=1944) drivers. # @@ -192,10 +183,17 @@ module ActiveRecord # * <tt>:allow_concurrency</tt> - If true, use async query methods so Ruby threads don't deadlock; # otherwise, use blocking query methods. class PostgreSQLAdapter < AbstractAdapter - ADAPTER_NAME = 'PostgreSQL'.freeze + class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition + def xml(*args) + options = args.extract_options! + column(args[0], 'xml', options) + end + end + + ADAPTER_NAME = 'PostgreSQL' NATIVE_DATABASE_TYPES = { - :primary_key => "serial primary key".freeze, + :primary_key => "serial primary key", :string => { :name => "character varying", :limit => 255 }, :text => { :name => "text" }, :integer => { :name => "integer" }, @@ -317,19 +315,22 @@ module ActiveRecord def quote(value, column = nil) #:nodoc: return super unless column - if value.kind_of?(String) && column.type == :binary - "'#{escape_bytea(value)}'" - elsif value.kind_of?(String) && column.sql_type == 'xml' - "xml '#{quote_string(value)}'" - elsif value.kind_of?(Numeric) && column.sql_type == 'money' + case value + when Numeric + return super unless column.sql_type == 'money' # Not truly string input, so doesn't require (or allow) escape string syntax. "'#{value}'" - elsif value.kind_of?(String) && column.sql_type =~ /^bit/ - case value - when /^[01]*$/ - "B'#{value}'" # Bit-string notation - when /^[0-9A-F]*$/i - "X'#{value}'" # Hexadecimal notation + when String + case column.sql_type + when 'bytea' then "'#{escape_bytea(value)}'" + when 'xml' then "xml '#{quote_string(value)}'" + when /^bit/ + case value + when /^[01]*$/ then "B'#{value}'" # Bit-string notation + when /^[0-9A-F]*$/i then "X'#{value}'" # Hexadecimal notation + end + else + super end else super @@ -1024,6 +1025,10 @@ module ActiveRecord [match_data[1], (rest.length > 0 ? rest : nil)] end end + + def table_definition + TableDefinition.new(self) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index e5e92f2b1c..5ca1923d89 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -1,4 +1,5 @@ require 'active_record/connection_adapters/sqlite_adapter' +require 'sqlite3' module ActiveRecord class Base @@ -20,16 +21,12 @@ module ActiveRecord raise ArgumentError, 'adapter name should be "sqlite3"' end - unless self.class.const_defined?(:SQLite3) - require_library_or_gem(config[:adapter]) - end - db = SQLite3::Database.new( config[:database], :results_as_hash => true ) - db.busy_timeout(config[:timeout]) unless config[:timeout].nil? + db.busy_timeout(config[:timeout]) if config[:timeout] ConnectionAdapters::SQLite3Adapter.new(db, logger, config) end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 0571e0cd14..c0cc7ba20d 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -360,8 +360,8 @@ module ActiveRecord end def copy_table_contents(from, to, columns, rename = {}) #:nodoc: - column_mappings = Hash[*columns.map {|name| [name, name]}.flatten] - rename.inject(column_mappings) {|map, a| map[a.last] = a.first; map} + column_mappings = Hash[columns.map {|name| [name, name]}] + rename.each { |a| column_mappings[a.last] = a.first } from_columns = columns(from).collect {|col| col.name} columns = columns.find_all{|col| from_columns.include?(column_mappings[col])} quoted_columns = columns.map { |col| quote_column_name(col) } * ',' diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 533bc331ae..b309df9b1b 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -6,40 +6,43 @@ module ActiveRecord # class DynamicFinderMatch def self.match(method) - df_match = self.new(method) - df_match.finder ? df_match : nil - end - - def initialize(method) - @finder = :first - @bang = false - @instantiator = nil + finder = :first + bang = false + instantiator = nil case method.to_s - when /^find_(all_by|last_by|by)_([_a-zA-Z]\w*)$/ - @finder = :last if $1 == 'last_by' - @finder = :all if $1 == 'all_by' + when /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ + finder = :last if $1 == 'last_' + finder = :all if $1 == 'all_' names = $2 when /^find_by_([_a-zA-Z]\w*)\!$/ - @bang = true + bang = true names = $1 when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ - @instantiator = $1 == 'initialize' ? :new : :create + instantiator = $1 == 'initialize' ? :new : :create names = $2 else - @finder = nil + return nil end - @attribute_names = names && names.split('_and_') + + new(finder, instantiator, bang, names.split('_and_')) + end + + def initialize(finder, instantiator, bang, attribute_names) + @finder = finder + @instantiator = instantiator + @bang = bang + @attribute_names = attribute_names end attr_reader :finder, :attribute_names, :instantiator def finder? - !@finder.nil? && @instantiator.nil? + @finder && !@instantiator end def instantiator? - @finder == :first && !@instantiator.nil? + @finder == :first && @instantiator end def creator? diff --git a/activerecord/lib/active_record/dynamic_scope_match.rb b/activerecord/lib/active_record/dynamic_scope_match.rb index 61c3ea0e7f..c832e927d6 100644 --- a/activerecord/lib/active_record/dynamic_scope_match.rb +++ b/activerecord/lib/active_record/dynamic_scope_match.rb @@ -8,25 +8,16 @@ module ActiveRecord # scope except that it's dynamic. class DynamicScopeMatch def self.match(method) - ds_match = self.new(method) - ds_match.scope ? ds_match : nil + return unless method.to_s =~ /^scoped_by_([_a-zA-Z]\w*)$/ + new(true, $1 && $1.split('_and_')) end - def initialize(method) - @scope = true - case method.to_s - when /^scoped_by_([_a-zA-Z]\w*)$/ - names = $1 - else - @scope = nil - end - @attribute_names = names && names.split('_and_') + def initialize(scope, attribute_names) + @scope = scope + @attribute_names = attribute_names end attr_reader :scope, :attribute_names - - def scope? - !@scope.nil? - end + alias :scope? :scope end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 826031a3e3..6fb723f2f5 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -704,11 +704,9 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) end def read_yaml_fixture_files - yaml_string = "" - Dir["#{@fixture_path}/**/*.yml"].select { |f| test(?f, f) }.each do |subfixture_path| - yaml_string << IO.read(subfixture_path) - end - yaml_string << IO.read(yaml_file_path) + yaml_string = (Dir["#{@fixture_path}/**/*.yml"].select { |f| + File.file?(f) + } + [yaml_file_path]).map { |file_path| IO.read(file_path) }.join if yaml = parse_yaml_string(yaml_string) # If the file is an ordered map, extract its children. diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 7372ab3278..bdd940f3ee 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -377,7 +377,12 @@ module ActiveRecord end if attributes_collection.is_a? Hash - attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes } + keys = attributes_collection.keys + attributes_collection = if keys.include?('id') || keys.include?(:id) + Array.wrap(attributes_collection) + else + attributes_collection.sort_by { |i, _| i.to_i }.map { |_, attributes| attributes } + end end association = send(association_name) diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 94dda4e413..868fd6c3ff 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -13,7 +13,7 @@ module ActiveRecord class Railtie < Rails::Railtie config.active_record = ActiveSupport::OrderedOptions.new - config.generators.orm :active_record, :migration => true, + config.app_generators.orm :active_record, :migration => true, :timestamps => true config.app_middleware.insert_after "::ActionDispatch::Callbacks", diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 12bfe3c738..58c705c8b2 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -108,7 +108,7 @@ namespace :db do end end when 'postgresql' - @encoding = config[:encoding] || ENV['CHARSET'] || 'utf8' + @encoding = config['encoding'] || ENV['CHARSET'] || 'utf8' begin ActiveRecord::Base.establish_connection(config.merge('database' => 'postgres', 'schema_search_path' => 'public')) ActiveRecord::Base.connection.create_database(config['database'], config.merge('encoding' => @encoding)) @@ -390,7 +390,7 @@ namespace :db do db_string = firebird_db_string(abcs[Rails.env]) sh "isql -a #{db_string} > #{Rails.root}/db/#{Rails.env}_structure.sql" else - raise "Task not supported by '#{abcs["test"]["adapter"]}'" + raise "Task not supported by '#{abcs[Rails.env]["adapter"]}'" end if ActiveRecord::Base.connection.supports_migrations? diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 478f1e8ef1..04ba5b291e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -326,7 +326,11 @@ module ActiveRecord def scope_for_create @scope_for_create ||= begin - @create_with_value || where_values_hash + if @create_with_value + @create_with_value.reverse_merge(where_values_hash) + else + where_values_hash + end end end @@ -358,15 +362,6 @@ module ActiveRecord scoping { @klass.send(method, *args, &block) } elsif arel.respond_to?(method) arel.send(method, *args, &block) - elsif match = DynamicFinderMatch.match(method) - attributes = match.attribute_names - super unless @klass.send(:all_attributes_exists?, attributes) - - if match.finder? - find_by_attributes(match, attributes, *args) - elsif match.instantiator? - find_or_instantiator_by_attributes(match, attributes, *args, &block) - end else super end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 12a2c6aec3..03862c78e4 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -191,7 +191,11 @@ module ActiveRecord end # Postgresql doesn't like ORDER BY when there are no GROUP BY - relation = except(:order).select(operation == 'count' ? column.count(distinct) : column.send(operation)) + relation = except(:order) + select_value = operation == 'count' ? column.count(distinct) : column.send(operation) + + relation.select_values = [select_value] + type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation) end @@ -208,21 +212,22 @@ module ActiveRecord aggregate_alias = column_alias_for(operation, column_name) select_statement = if operation == 'count' && column_name == :all - "COUNT(*) AS count_all" + ["COUNT(*) AS count_all"] else - Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias).to_sql + [Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias)] end - select_statement << ", #{group_field} AS #{group_alias}" + select_statement << "#{group_field} AS #{group_alias}" - relation = except(:group).select(select_statement).group(group) + relation = except(:group).group(group) + relation.select_values = select_statement calculated_data = @klass.connection.select_all(relation.to_sql) if association key_ids = calculated_data.collect { |row| row[group_alias] } key_records = association.klass.base_class.find(key_ids) - key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } + key_records = Hash[key_records.map { |r| [r.id, r] }] end ActiveSupport::OrderedHash[calculated_data.map do |row| diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6a33edeb97..2e0a2effc2 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -36,7 +36,7 @@ module ActiveRecord to_a.select {|*block_args| value.call(*block_args) } else relation = clone - relation.select_values += [value] + relation.select_values += Array.wrap(value) relation end end @@ -135,14 +135,13 @@ module ActiveRecord end def reverse_order - order_clause = arel.order_clauses.join(', ') - relation = except(:order) + order_clause = arel.order_clauses - order = order_clause.blank? ? + order = order_clause.empty? ? "#{@klass.table_name}.#{@klass.primary_key} DESC" : - reverse_sql_order(order_clause) + reverse_sql_order(order_clause).join(', ') - relation.order(Arel::SqlLiteral.new(order)) + except(:order).order(Arel::SqlLiteral.new(order)) end def arel @@ -150,7 +149,7 @@ module ActiveRecord end def custom_join_sql(*joins) - arel = table + arel = table.select_manager joins.each do |join| next if join.blank? @@ -158,16 +157,13 @@ module ActiveRecord @implicit_readonly = true case join - when Hash, Array, Symbol - if array_of_strings?(join) - join_string = join.join(' ') - arel = arel.join(Arel::SqlLiteral.new(join_string)) - end + when Array + join = Arel.sql(join.join(' ')) if array_of_strings?(join) when String - arel = arel.join(Arel::SqlLiteral.new(join)) - else - arel = arel.join(join) + join = Arel.sql(join) end + + arel.join(join) end arel.joins(arel) @@ -179,13 +175,8 @@ module ActiveRecord arel = build_joins(arel, @joins_values) unless @joins_values.empty? (@where_values - ['']).uniq.each do |where| - case where - when Arel::SqlLiteral - arel = arel.where(where) - else - sql = where.is_a?(String) ? where : where.to_sql - arel = arel.where(Arel::SqlLiteral.new("(#{sql})")) - end + where = Arel.sql(where) if String === where + arel = arel.where(Arel::Nodes::Grouping.new(where)) end arel = arel.having(*@having_values.uniq.reject{|h| h.blank?}) unless @having_values.empty? @@ -260,16 +251,7 @@ module ActiveRecord def build_select(arel, selects) unless selects.empty? @implicit_readonly = false - # TODO: fix this ugly hack, we should refactor the callers to get an Arel compatible array. - # Before this change we were passing to Arel the last element only, and Arel is capable of handling an array - case select = selects.last - when Arel::Expression, Arel::SqlLiteral - arel.project(select) - when /^COUNT\(/ - arel.project(Arel::SqlLiteral.new(select)) - else - arel.project(*selects) - end + arel.project(*selects) else arel.project(Arel::SqlLiteral.new(@klass.quoted_table_name + '.*')) end @@ -283,15 +265,9 @@ module ActiveRecord end def reverse_sql_order(order_query) - order_query.split(',').each { |s| - if s.match(/\s(asc|ASC)$/) - s.gsub!(/\s(asc|ASC)$/, ' DESC') - elsif s.match(/\s(desc|DESC)$/) - s.gsub!(/\s(desc|DESC)$/, ' ASC') - else - s.concat(' DESC') - end - }.join(',') + order_query.join(', ').split(',').collect do |s| + s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') + end end def array_of_strings?(o) diff --git a/activerecord/lib/active_record/serialization.rb b/activerecord/lib/active_record/serialization.rb index ad3f7afd6f..398eb1534a 100644 --- a/activerecord/lib/active_record/serialization.rb +++ b/activerecord/lib/active_record/serialization.rb @@ -45,7 +45,7 @@ module ActiveRecord #:nodoc: send(association) end - unless records.nil? + if records association_options = include_has_options ? include_associations[association] : base_only_or_except opts = options.merge(association_options) yield(association, records, opts) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 742513230e..cbaa4990f7 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -474,4 +474,9 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase Author.belongs_to :special_author_address, :dependent => :restrict end end + + def test_attributes_are_being_set_when_initialized_from_belongs_to_association_with_where_clause + new_firm = accounts(:signals37).build_firm(:name => 'Apple') + assert_equal new_firm.name, "Apple" + end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 2bdf9d8971..c0be7dfdcc 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 @@ -848,4 +848,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_queries(0) { david.projects.columns; david.projects.columns } end + def test_attributes_are_being_set_when_initialized_from_habm_association_with_where_clause + new_developer = projects(:action_controller).developers.where(:name => "Marcelo").build + assert_equal new_developer.name, "Marcelo" + end + + def test_attributes_are_being_set_when_initialized_from_habm_association_with_multiple_where_clauses + new_developer = projects(:action_controller).developers.where(:name => "Marcelo").where(:salary => 90_000).build + assert_equal new_developer.name, "Marcelo" + assert_equal new_developer.salary, 90_000 + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index efabf74e13..720b7fc386 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1255,4 +1255,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end EOF end + + def test_attributes_are_being_set_when_initialized_from_has_many_association_with_where_clause + new_comment = posts(:welcome).comments.where(:body => "Some content").build + assert_equal new_comment.body, "Some content" + end + + def test_attributes_are_being_set_when_initialized_from_has_many_association_with_multiple_where_clauses + new_comment = posts(:welcome).comments.where(:body => "Some content").where(:type => 'SpecialComment').build + assert_equal new_comment.body, "Some content" + assert_equal new_comment.type, "SpecialComment" + assert_equal new_comment.post_id, posts(:welcome).id + end end 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 45f8bd64eb..0dac633852 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -421,4 +421,18 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_raises(ActiveRecord::RecordNotFound) {company.developer_ids= ids} end + def test_build_a_model_from_hm_through_association_with_where_clause + assert_nothing_raised { books(:awdr).subscribers.where(:nick => "marklazz").build } + end + + def test_attributes_are_being_set_when_initialized_from_hm_through_association_with_where_clause + new_subscriber = books(:awdr).subscribers.where(:nick => "marklazz").build + assert_equal new_subscriber.nick, "marklazz" + end + + def test_attributes_are_being_set_when_initialized_from_hm_through_association_with_multiple_where_clauses + new_subscriber = books(:awdr).subscribers.where(:nick => "marklazz").where(:name => 'Marcelo Giorgi').build + assert_equal new_subscriber.nick, "marklazz" + assert_equal new_subscriber.name, "Marcelo Giorgi" + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index e959ed46cc..b522be3fe0 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -326,4 +326,9 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert !account.new_record? assert_equal 500, account.credit_limit end + + def test_attributes_are_being_set_when_initialized_from_has_one_association_with_where_clause + new_account = companies(:first_firm).build_account(:firm_name => 'Account') + assert_equal new_account.firm_name, "Account" + end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d87f259f4b..16fd9a7465 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -69,6 +69,24 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_use_table_engine_for_quoting_where + relation = Topic.where(Topic.arel_table[:id].eq(1)) + engine = relation.table.engine + + fakepool = Class.new(Struct.new(:spec)) { + def with_connection; yield self; end + def connection_pool; self; end + def quote_table_name(*args); raise "lol quote_table_name"; end + } + + relation.table.engine = fakepool.new(engine.connection_pool.spec) + + error = assert_raises(RuntimeError) { relation.to_a } + assert_match('lol', error.message) + ensure + relation.table.engine = engine + end + def test_preserving_time_objects assert_kind_of( Time, Topic.find(1).bonus_time, @@ -366,6 +384,10 @@ class BasicsTest < ActiveRecord::TestCase assert_equal Topic.find(1), Topic.find(2).topic end + def test_find_by_slug + assert_equal Topic.find('1-meowmeow'), Topic.find(1) + end + def test_equality_of_new_records assert_not_equal Topic.new, Topic.new end diff --git a/activerecord/test/cases/dynamic_finder_match_test.rb b/activerecord/test/cases/dynamic_finder_match_test.rb new file mode 100644 index 0000000000..e576870317 --- /dev/null +++ b/activerecord/test/cases/dynamic_finder_match_test.rb @@ -0,0 +1,98 @@ +require "cases/helper" + +module ActiveRecord + class DynamicFinderMatchTest < ActiveRecord::TestCase + def test_find_or_create_by + match = DynamicFinderMatch.match("find_or_create_by_age_and_sex_and_location") + assert_not_nil match + assert !match.finder? + assert match.instantiator? + assert_equal :first, match.finder + assert_equal :create, match.instantiator + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_or_initialize_by + match = DynamicFinderMatch.match("find_or_initialize_by_age_and_sex_and_location") + assert_not_nil match + assert !match.finder? + assert match.instantiator? + assert_equal :first, match.finder + assert_equal :new, match.instantiator + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_no_match + assert_nil DynamicFinderMatch.match("not_a_finder") + end + + def find_by_bang + match = DynamicFinderMatch.match("find_by_age_and_sex_and_location!") + assert_not_nil match + assert match.finder? + assert match.bang? + assert_equal :first, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_by + match = DynamicFinderMatch.match("find_by_age_and_sex_and_location") + assert_not_nil match + assert match.finder? + assert_equal :first, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_by_with_symbol + m = DynamicFinderMatch.match(:find_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_all_by_with_symbol + m = DynamicFinderMatch.match(:find_all_by_foo) + assert_equal :all, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_all_by + match = DynamicFinderMatch.match("find_all_by_age_and_sex_and_location") + assert_not_nil match + assert match.finder? + assert_equal :all, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_last_by + m = DynamicFinderMatch.match(:find_last_by_foo) + assert_equal :last, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_by! + m = DynamicFinderMatch.match(:find_by_foo!) + assert_equal :first, m.finder + assert m.bang?, 'should be banging' + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_or_create + m = DynamicFinderMatch.match(:find_or_create_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + assert_equal :create, m.instantiator + end + + def test_find_or_initialize + m = DynamicFinderMatch.match(:find_or_initialize_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + assert_equal :new, m.instantiator + end + + def test_garbage + assert !DynamicFinderMatch.match(:fooo), 'should be false' + assert !DynamicFinderMatch.match(:find_by), 'should be false' + end + end +end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 4f3e43d77d..26b5096255 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -11,57 +11,6 @@ require 'models/project' require 'models/developer' require 'models/customer' -class DynamicFinderMatchTest < ActiveRecord::TestCase - def test_find_no_match - assert_nil ActiveRecord::DynamicFinderMatch.match("not_a_finder") - end - - def test_find_by - match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location") - assert_not_nil match - assert match.finder? - assert_equal :first, match.finder - assert_equal %w(age sex location), match.attribute_names - end - - def find_by_bang - match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location!") - assert_not_nil match - assert match.finder? - assert match.bang? - assert_equal :first, match.finder - assert_equal %w(age sex location), match.attribute_names - end - - def test_find_all_by - match = ActiveRecord::DynamicFinderMatch.match("find_all_by_age_and_sex_and_location") - assert_not_nil match - assert match.finder? - assert_equal :all, match.finder - assert_equal %w(age sex location), match.attribute_names - end - - def test_find_or_initialize_by - match = ActiveRecord::DynamicFinderMatch.match("find_or_initialize_by_age_and_sex_and_location") - assert_not_nil match - assert !match.finder? - assert match.instantiator? - assert_equal :first, match.finder - assert_equal :new, match.instantiator - assert_equal %w(age sex location), match.attribute_names - end - - def test_find_or_create_by - match = ActiveRecord::DynamicFinderMatch.match("find_or_create_by_age_and_sex_and_location") - assert_not_nil match - assert !match.finder? - assert match.instantiator? - assert_equal :first, match.finder - assert_equal :create, match.instantiator - assert_equal %w(age sex location), match.attribute_names - end -end - class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index ffe16ffdfa..f3d3d62830 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -219,7 +219,7 @@ class MethodScopingTest < ActiveRecord::TestCase new_comment = nil VerySpecialComment.send(:with_scope, :create => { :post_id => 1 }) do - assert_equal({:post_id => 1}, VerySpecialComment.scoped.send(:scope_for_create)) + assert_equal({:post_id => 1, :type => 'VerySpecialComment' }, VerySpecialComment.scoped.send(:scope_for_create)) new_comment = VerySpecialComment.create :body => "Wonderful world" end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index bcae46c7e8..6e8ee95613 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -91,7 +91,7 @@ if ActiveRecord::Base.connection.supports_migrations? # Oracle adapter is shortening index name when just column list is given unless current_adapter?(:OracleAdapter) assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) } - assert_nothing_raised { Person.connection.remove_index("people", :name => "index_people_on_last_name_and_first_name") } + assert_nothing_raised { Person.connection.remove_index("people", :name => :index_people_on_last_name_and_first_name) } assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) } assert_nothing_raised { Person.connection.remove_index("people", "last_name_and_first_name") } end @@ -124,6 +124,13 @@ if ActiveRecord::Base.connection.supports_migrations? end end + def test_index_symbol_names + assert_nothing_raised { Person.connection.add_index :people, :primary_contact_id, :name => :symbol_index_name } + assert Person.connection.index_exists?(:people, :primary_contact_id, :name => :symbol_index_name) + assert_nothing_raised { Person.connection.remove_index :people, :name => :symbol_index_name } + assert !Person.connection.index_exists?(:people, :primary_contact_id, :name => :symbol_index_name) + end + def test_add_index_length_limit good_index_name = 'x' * Person.connection.index_name_length too_long_index_name = good_index_name + 'x' @@ -1581,13 +1588,23 @@ if ActiveRecord::Base.connection.supports_migrations? end end - if current_adapter?(:PostgreSQLAdapter) + if current_adapter?(:PostgreSQLAdapter) || current_adapter?(:SQLiteAdapter) || current_adapter?(:MysqlAdapter) || current_adapter?(:Mysql2Adapter) def test_xml_creates_xml_column + type = current_adapter?(:PostgreSQLAdapter) ? 'xml' : :text + with_new_table do |t| - t.expects(:column).with(:data, 'xml', {}) + t.expects(:column).with(:data, type, {}) t.xml :data end end + else + def test_xml_creates_xml_column + with_new_table do |t| + assert_raises(NotImplementedError) do + t.xml :data + end + end + end end protected diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 62e073ba8c..8382ca048b 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -115,7 +115,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_difference('Ship.count') { pirate.save! } end - def test_reject_if_with_a_proc_which_returns_true_always + def test_reject_if_with_a_proc_which_returns_true_always_for_has_one Pirate.accepts_nested_attributes_for :ship, :reject_if => proc {|attributes| true } pirate = Pirate.new(:catchphrase => "Stop wastin' me time") ship = pirate.create_ship(:name => 's1') @@ -123,6 +123,22 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_equal 's1', ship.reload.name end + def test_reject_if_with_a_proc_which_returns_true_always_for_has_many + Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true } + man = Man.create(:name => "John") + interest = man.interests.create(:topic => 'photography') + man.update_attributes({:interests_attributes => { :topic => 'gardening', :id => interest.id } }) + assert_equal 'photography', interest.reload.topic + end + + def test_has_many_association_updating_a_single_record + Man.accepts_nested_attributes_for(:interests) + man = Man.create(:name => 'John') + interest = man.interests.create(:topic => 'photography') + man.update_attributes({:interests_attributes => {:topic => 'gardening', :id => interest.id}}) + assert_equal 'gardening', interest.reload.topic + end + end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 3bc3671b77..d642aeed8b 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -32,6 +32,11 @@ class RelationTest < ActiveRecord::TestCase assert_equal 5, Post.where(:id => post_authors).size end + def test_dynamic_finder + x = Post.where('author_id = ?', 1) + assert x.klass.respond_to?(:find_by_id), '@klass should handle dynamic finders' + end + def test_multivalue_where posts = Post.where('author_id = ? AND id = ?', 1, 1) assert_equal 1, posts.to_a.size diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index d31db9f0ba..b5b46d7431 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -12,6 +12,7 @@ require 'active_support/core_ext/object/duplicable' require 'set' require 'uri' +require 'active_support/core_ext/uri' require 'active_resource/exceptions' require 'active_resource/connection' require 'active_resource/formats' @@ -166,6 +167,7 @@ module ActiveResource # # GET http://api.people.com:3000/people/999.xml # ryan = Person.find(999) # 404, raises ActiveResource::ResourceNotFound # + # # <tt>404</tt> is just one of the HTTP error response codes that Active Resource will handle with its own exception. The # following HTTP response codes will also result in these exceptions: # @@ -194,6 +196,16 @@ module ActiveResource # redirect_to :action => 'new' # end # + # When a GET is requested for a nested resource and you don't provide the prefix_param + # an ActiveResource::MissingPrefixParam will be raised. + # + # class Comment < ActiveResource::Base + # self.site = "http://someip.com/posts/:post_id/" + # end + # + # Comment.find(1) + # # => ActiveResource::MissingPrefixParam: post_id prefix_option is missing + # # === Validation errors # # Active Resource supports validations on resources and will return errors if any of these validations fail @@ -403,8 +415,8 @@ module ActiveResource @site = nil else @site = create_site_uri_from(site) - @user = uri_parser.unescape(@site.user) if @site.user - @password = uri_parser.unescape(@site.password) if @site.password + @user = URI.parser.unescape(@site.user) if @site.user + @password = URI.parser.unescape(@site.password) if @site.password end end @@ -577,7 +589,7 @@ module ActiveResource # Default value is <tt>site.path</tt>. def prefix=(value = '/') # Replace :placeholders with '#{embedded options[:lookups]}' - prefix_call = value.gsub(/:\w+/) { |key| "\#{URI.escape options[#{key}].to_s}" } + prefix_call = value.gsub(/:\w+/) { |key| "\#{URI.parser.escape options[#{key}].to_s}" } # Clear prefix parameters in case they have been cached @prefix_parameters = nil @@ -621,8 +633,10 @@ module ActiveResource # # => /posts/5/comments/1.xml?active=1 # def element_path(id, prefix_options = {}, query_options = nil) + check_prefix_options(prefix_options) + prefix_options, query_options = split_options(prefix_options) if query_options.nil? - "#{prefix(prefix_options)}#{collection_name}/#{URI.escape id.to_s}.#{format.extension}#{query_string(query_options)}" + "#{prefix(prefix_options)}#{collection_name}/#{URI.parser.escape id.to_s}.#{format.extension}#{query_string(query_options)}" end # Gets the new element path for REST resources. @@ -663,6 +677,7 @@ module ActiveResource # # => /posts/5/comments.xml?active=1 # def collection_path(prefix_options = {}, query_options = nil) + check_prefix_options(prefix_options) prefix_options, query_options = split_options(prefix_options) if query_options.nil? "#{prefix(prefix_options)}#{collection_name}.#{format.extension}#{query_string(query_options)}" end @@ -842,6 +857,14 @@ module ActiveResource end private + + def check_prefix_options(prefix_options) + p_options = HashWithIndifferentAccess.new(prefix_options) + prefix_parameters.each do |p| + raise(MissingPrefixParam, "#{p} prefix_option is missing") if p_options[p].blank? + end + end + # Find every resource def find_every(options) begin @@ -894,12 +917,12 @@ module ActiveResource # Accepts a URI and creates the site URI from that. def create_site_uri_from(site) - site.is_a?(URI) ? site.dup : uri_parser.parse(site) + site.is_a?(URI) ? site.dup : URI.parser.parse(site) end # Accepts a URI and creates the proxy URI from that. def create_proxy_uri_from(proxy) - proxy.is_a?(URI) ? proxy.dup : uri_parser.parse(proxy) + proxy.is_a?(URI) ? proxy.dup : URI.parser.parse(proxy) end # contains a set of the current prefix parameters. @@ -924,10 +947,6 @@ module ActiveResource [ prefix_options, query_options ] end - - def uri_parser - @uri_parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI - end end attr_accessor :attributes #:nodoc: @@ -1352,8 +1371,9 @@ module ActiveResource namespaces = module_names[0, module_names.size-1].map do |module_name| receiver = receiver.const_get(module_name) end - if namespace = namespaces.reverse.detect { |ns| ns.const_defined?(resource_name) } - return namespace.const_get(resource_name) + const_args = RUBY_VERSION < "1.9" ? [resource_name] : [resource_name, false] + if namespace = namespaces.reverse.detect { |ns| ns.const_defined?(*const_args) } + return namespace.const_get(*const_args) else raise NameError end @@ -1369,8 +1389,9 @@ module ActiveResource self.class.const_get(resource_name) end rescue NameError - if self.class.const_defined?(resource_name) - resource = self.class.const_get(resource_name) + const_args = RUBY_VERSION < "1.9" ? [resource_name] : [resource_name, false] + if self.class.const_defined?(*const_args) + resource = self.class.const_get(*const_args) else resource = self.class.const_set(resource_name, Class.new(ActiveResource::Base)) end diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index c587225e24..480f2fbecb 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/benchmark' +require 'active_support/core_ext/uri' require 'net/https' require 'date' require 'time' @@ -31,21 +32,20 @@ module ActiveResource def initialize(site, format = ActiveResource::Formats::XmlFormat) raise ArgumentError, 'Missing site URI' unless site @user = @password = nil - @uri_parser = URI.const_defined?(:Parser) ? URI::Parser.new : URI self.site = site self.format = format end # Set URI for remote service. def site=(site) - @site = site.is_a?(URI) ? site : @uri_parser.parse(site) - @user = @uri_parser.unescape(@site.user) if @site.user - @password = @uri_parser.unescape(@site.password) if @site.password + @site = site.is_a?(URI) ? site : URI.parser.parse(site) + @user = URI.parser.unescape(@site.user) if @site.user + @password = URI.parser.unescape(@site.password) if @site.password end # Set the proxy for remote service. def proxy=(proxy) - @proxy = proxy.is_a?(URI) ? proxy : @uri_parser.parse(proxy) + @proxy = proxy.is_a?(URI) ? proxy : URI.parser.parse(proxy) end # Sets the user for remote service. diff --git a/activeresource/lib/active_resource/exceptions.rb b/activeresource/lib/active_resource/exceptions.rb index 0f4549fd73..6b953b28ad 100644 --- a/activeresource/lib/active_resource/exceptions.rb +++ b/activeresource/lib/active_resource/exceptions.rb @@ -36,6 +36,9 @@ module ActiveResource def to_s; response['Location'] ? "#{super} => #{response['Location']}" : super; end end + # Raised when ... + class MissingPrefixParam < ArgumentError; end # :nodoc: + # 4xx Client Error class ClientError < ConnectionError; end # :nodoc: diff --git a/activeresource/test/abstract_unit.rb b/activeresource/test/abstract_unit.rb index 129efeb879..195f93f2a6 100644 --- a/activeresource/test/abstract_unit.rb +++ b/activeresource/test/abstract_unit.rb @@ -75,6 +75,10 @@ def setup_response </person> eof + @startup_sound = { + :name => "Mac Startup Sound", :author => { :name => "Jim Reekes" } + }.to_xml(:root => 'sound') + ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/1.xml", {}, @matz mock.get "/people/2.xml", {}, @david @@ -97,6 +101,7 @@ def setup_response mock.put "/people/1/addresses/1.xml", {}, nil, 204 mock.delete "/people/1/addresses/1.xml", {}, nil, 200 mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' + mock.get "/people/1/addresses/99.xml", {}, nil, 404 mock.get "/people//addresses.xml", {}, nil, 404 mock.get "/people//addresses/1.xml", {}, nil, 404 mock.put "/people//addresses/1.xml", {}, nil, 404 @@ -111,6 +116,8 @@ def setup_response mock.head "/people/Greg/addresses/1.xml", {}, nil, 200 # customer mock.get "/customers/1.xml", {}, @luis + # sound + mock.get "/sounds/1.xml", {}, @startup_sound end Person.user = nil diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index 6fabeeebcd..abf4259a54 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -475,6 +475,12 @@ class BaseTest < Test::Unit::TestCase assert_equal '/people/ann%20mary/addresses/ann%20mary.xml', StreetAddress.element_path(:'ann mary', 'person_id' => 'ann mary') end + def test_custom_element_path_without_required_prefix_param + assert_raise ActiveResource::MissingPrefixParam do + StreetAddress.element_path(1) + end + end + def test_module_element_path assert_equal '/sounds/1.xml', Asset::Sound.element_path(1) end @@ -513,6 +519,12 @@ class BaseTest < Test::Unit::TestCase assert_equal '/people/1/addresses/1.xml?type=work', StreetAddress.element_path(1, {:person_id => 1}, {:type => 'work'}) end + def test_custom_collection_path_without_required_prefix_param + assert_raise ActiveResource::MissingPrefixParam do + StreetAddress.collection_path + end + end + def test_custom_collection_path assert_equal '/people/1/addresses.xml', StreetAddress.collection_path(:person_id => 1) assert_equal '/people/1/addresses.xml', StreetAddress.collection_path('person_id' => 1) @@ -560,6 +572,8 @@ class BaseTest < Test::Unit::TestCase assert_equal Set.new([:the_param1]), person_class.prefix_parameters person_class.prefix = "the_prefix/:the_param2" assert_equal Set.new([:the_param2]), person_class.prefix_parameters + person_class.prefix = "the_prefix/:the_param1/other_prefix/:the_param2" + assert_equal Set.new([:the_param2, :the_param1]), person_class.prefix_parameters end end @@ -1083,4 +1097,9 @@ class BaseTest < Test::Unit::TestCase plan.save! assert_equal 10.00, plan.price end + + def test_namespacing + sound = Asset::Sound.find(1) + assert_equal "Asset::Sound::Author", sound.author.class.to_s + end end diff --git a/activeresource/test/cases/finder_test.rb b/activeresource/test/cases/finder_test.rb index fd09ef46d7..ebb783996d 100644 --- a/activeresource/test/cases/finder_test.rb +++ b/activeresource/test/cases/finder_test.rb @@ -84,7 +84,7 @@ class FinderTest < Test::Unit::TestCase def test_find_by_id_not_found assert_raise(ActiveResource::ResourceNotFound) { Person.find(99) } - assert_raise(ActiveResource::ResourceNotFound) { StreetAddress.find(1) } + assert_raise(ActiveResource::ResourceNotFound) { StreetAddress.find(99, :params => {:person_id => 1}) } end def test_find_all_sub_objects diff --git a/activeresource/test/cases/format_test.rb b/activeresource/test/cases/format_test.rb index c3733e13d8..bed95ef524 100644 --- a/activeresource/test/cases/format_test.rb +++ b/activeresource/test/cases/format_test.rb @@ -33,7 +33,7 @@ class FormatTest < Test::Unit::TestCase ActiveResource::HttpMock.respond_to.get "/people.#{format}", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode(@programmers) remote_programmers = Person.find(:all) assert_equal 2, remote_programmers.size - assert remote_programmers.select { |p| p.name == 'David' } + assert remote_programmers.map { |p| p.name }.include? 'David' end end end diff --git a/activeresource/test/fixtures/sound.rb b/activeresource/test/fixtures/sound.rb index 5c0ef5d55c..d9d2b99fcd 100644 --- a/activeresource/test/fixtures/sound.rb +++ b/activeresource/test/fixtures/sound.rb @@ -1,5 +1,9 @@ -module Asset +module Asset class Sound < ActiveResource::Base self.site = "http://37s.sunrise.i:3000" end end + +# to test namespacing in a module +class Author +end
\ No newline at end of file diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index d811c3b2f0..0c1d46c7ec 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -482,7 +482,7 @@ module ActiveSupport chain.delete_if {|c| c.matches?(type, filter) } end - options[:prepend] ? chain.unshift(*mapped) : chain.push(*mapped) + options[:prepend] ? chain.unshift(*(mapped.reverse)) : chain.push(*mapped) end end diff --git a/activesupport/lib/active_support/configurable.rb b/activesupport/lib/active_support/configurable.rb index 5b85f9394a..58ed37b018 100644 --- a/activesupport/lib/active_support/configurable.rb +++ b/activesupport/lib/active_support/configurable.rb @@ -9,9 +9,29 @@ module ActiveSupport module Configurable extend ActiveSupport::Concern + class Configuration < ActiveSupport::InheritableOptions + def compile_methods! + self.class.compile_methods!(keys.reject {|key| respond_to?(key)}) + end + + # compiles reader methods so we don't have to go through method_missing + def self.compile_methods!(keys) + keys.each do |key| + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{key}; _get(#{key.inspect}); end + RUBY + end + end + end + module ClassMethods def config - @_config ||= ActiveSupport::InheritableOptions.new(superclass.respond_to?(:config) ? superclass.config : {}) + @_config ||= if superclass.respond_to?(:config) + superclass.config.inheritable_copy + else + # create a new "anonymous" class that will host the compiled reader methods + Class.new(Configuration).new + end end def configure @@ -48,7 +68,7 @@ module ActiveSupport # user.config.level # => 1 # def config - @_config ||= ActiveSupport::InheritableOptions.new(self.class.config) + @_config ||= self.class.config.inheritable_copy end end end diff --git a/activesupport/lib/active_support/core_ext/class/attribute.rb b/activesupport/lib/active_support/core_ext/class/attribute.rb index 688cba03db..5414b3a18f 100644 --- a/activesupport/lib/active_support/core_ext/class/attribute.rb +++ b/activesupport/lib/active_support/core_ext/class/attribute.rb @@ -72,11 +72,21 @@ class Class remove_possible_method(:#{name}) define_method(:#{name}) { val } end + + if singleton_class? + class_eval do + remove_possible_method(:#{name}) + def #{name} + defined?(@#{name}) ? @#{name} : singleton_class.#{name} + end + end + end val end + remove_method :#{name} if method_defined?(:#{name}) def #{name} - defined?(@#{name}) ? @#{name} : singleton_class.#{name} + defined?(@#{name}) ? @#{name} : self.class.#{name} end def #{name}? @@ -87,4 +97,15 @@ class Class attr_writer name if instance_writer end end + + private + def singleton_class? + # in case somebody is crazy enough to overwrite allocate + allocate = Class.instance_method(:allocate) + # object.class always points to a real (non-singleton) class + allocate.bind(self).call.class != self + rescue TypeError + # MRI/YARV/JRuby all disallow creating new instances of a singleton class + true + end end diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb index 045a6944fa..d8748b1138 100644 --- a/activesupport/lib/active_support/core_ext/hash/keys.rb +++ b/activesupport/lib/active_support/core_ext/hash/keys.rb @@ -35,11 +35,13 @@ class Hash # as keys, this will fail. # # ==== Examples - # { :name => "Rob", :years => "28" }.assert_valid_keys(:name, :age) # => raises "ArgumentError: Unknown key(s): years" - # { :name => "Rob", :age => "28" }.assert_valid_keys("name", "age") # => raises "ArgumentError: Unknown key(s): name, age" + # { :name => "Rob", :years => "28" }.assert_valid_keys(:name, :age) # => raises "ArgumentError: Unknown key: years" + # { :name => "Rob", :age => "28" }.assert_valid_keys("name", "age") # => raises "ArgumentError: Unknown key: name" # { :name => "Rob", :age => "28" }.assert_valid_keys(:name, :age) # => passes, raises nothing def assert_valid_keys(*valid_keys) - unknown_keys = keys - [valid_keys].flatten - raise(ArgumentError, "Unknown key(s): #{unknown_keys.join(", ")}") unless unknown_keys.empty? + valid_keys.flatten! + each_key do |k| + raise(ArgumentError, "Unknown key: #{k}") unless valid_keys.include?(k) + end end end diff --git a/activesupport/lib/active_support/core_ext/module/attr_internal.rb b/activesupport/lib/active_support/core_ext/module/attr_internal.rb index 28bc30ae26..00db75bfec 100644 --- a/activesupport/lib/active_support/core_ext/module/attr_internal.rb +++ b/activesupport/lib/active_support/core_ext/module/attr_internal.rb @@ -1,16 +1,12 @@ class Module # Declares an attribute reader backed by an internally-named instance variable. def attr_internal_reader(*attrs) - attrs.each do |attr| - module_eval "def #{attr}() #{attr_internal_ivar_name(attr)} end", __FILE__, __LINE__ - end + attrs.each {|attr_name| attr_internal_define(attr_name, :reader)} end # Declares an attribute writer backed by an internally-named instance variable. def attr_internal_writer(*attrs) - attrs.each do |attr| - module_eval "def #{attr}=(v) #{attr_internal_ivar_name(attr)} = v end", __FILE__, __LINE__ - end + attrs.each {|attr_name| attr_internal_define(attr_name, :writer)} end # Declares an attribute reader and writer backed by an internally-named instance @@ -29,4 +25,15 @@ class Module def attr_internal_ivar_name(attr) Module.attr_internal_naming_format % attr end + + def attr_internal_define(attr_name, type) + internal_name = attr_internal_ivar_name(attr_name).sub(/\A@/, '') + class_eval do # class_eval is necessary on 1.9 or else the methods a made private + # use native attr_* methods as they are faster on some Ruby implementations + send("attr_#{type}", internal_name) + end + attr_name, internal_name = "#{attr_name}=", "#{internal_name}=" if type == :writer + alias_method attr_name, internal_name + remove_method internal_name + end end diff --git a/activesupport/lib/active_support/core_ext/object/to_param.rb b/activesupport/lib/active_support/core_ext/object/to_param.rb index f2e7c2351e..ecb2bca82c 100644 --- a/activesupport/lib/active_support/core_ext/object/to_param.rb +++ b/activesupport/lib/active_support/core_ext/object/to_param.rb @@ -35,7 +35,8 @@ end class Hash # Converts a hash into a string suitable for use as a URL query string. An optional <tt>namespace</tt> can be - # passed to enclose the param names (see example below). + # passed to enclose the param names (see example below). The string pairs "key=value" that conform the query + # string are sorted lexicographically in ascending order. # # ==== Examples # { :name => 'David', :nationality => 'Danish' }.to_param # => "name=David&nationality=Danish" @@ -44,6 +45,6 @@ class Hash def to_param(namespace = nil) collect do |key, value| value.to_query(namespace ? "#{namespace}[#{key}]" : key) - end * '&' + end.sort * '&' end end diff --git a/activesupport/lib/active_support/core_ext/uri.rb b/activesupport/lib/active_support/core_ext/uri.rb index b7fe0a6209..ee991e3439 100644 --- a/activesupport/lib/active_support/core_ext/uri.rb +++ b/activesupport/lib/active_support/core_ext/uri.rb @@ -20,3 +20,11 @@ if RUBY_VERSION >= '1.9' end end end + +module URI + class << self + def parser + @parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI + end + end +end diff --git a/activesupport/lib/active_support/ordered_options.rb b/activesupport/lib/active_support/ordered_options.rb index 37e357552c..124e1a74f8 100644 --- a/activesupport/lib/active_support/ordered_options.rb +++ b/activesupport/lib/active_support/ordered_options.rb @@ -18,6 +18,9 @@ require 'active_support/ordered_hash' # module ActiveSupport #:nodoc: class OrderedOptions < OrderedHash + alias_method :_get, :[] # preserve the original #[] method + protected :_get # make it protected + def []=(key, value) super(key.to_sym, value) end @@ -36,8 +39,19 @@ module ActiveSupport #:nodoc: end class InheritableOptions < OrderedOptions - def initialize(parent) - super() { |h,k| parent[k] } + def initialize(parent = nil) + if parent.kind_of?(OrderedOptions) + # use the faster _get when dealing with OrderedOptions + super() { |h,k| parent._get(k) } + elsif parent + super() { |h,k| parent[k] } + else + super() + end + end + + def inheritable_copy + self.class.new(self) end end end diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 292383e3d8..51b28b6a43 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -149,6 +149,27 @@ module CallbacksTest end end + class AfterSaveConditionalPerson < Record + after_save Proc.new { |r| r.history << [:after_save, :string1] } + after_save Proc.new { |r| r.history << [:after_save, :string2] } + def save + run_callbacks :save + end + end + + class AfterSaveConditionalPersonCallbackTest < Test::Unit::TestCase + def test_after_save_runs_in_the_reverse_order + person = AfterSaveConditionalPerson.new + person.save + assert_equal [ + [:after_save, :string2], + [:after_save, :string1] + ], person.history + end + end + + + class ConditionalPerson < Record # proc before_save Proc.new { |r| r.history << [:before_save, :proc] }, :if => Proc.new { |r| true } @@ -352,6 +373,8 @@ module CallbacksTest end end + + class ResetCallbackTest < Test::Unit::TestCase def test_save_conditional_person person = CleanPerson.new diff --git a/activesupport/test/configurable_test.rb b/activesupport/test/configurable_test.rb index cef67e3cf9..9c773c1944 100644 --- a/activesupport/test/configurable_test.rb +++ b/activesupport/test/configurable_test.rb @@ -39,4 +39,22 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase assert_equal :baz, instance.config.foo assert_equal :bar, Parent.config.foo end + + test "configuration is crystalizeable" do + parent = Class.new { include ActiveSupport::Configurable } + child = Class.new(parent) + + parent.config.bar = :foo + assert !parent.config.respond_to?(:bar) + assert !child.config.respond_to?(:bar) + assert !child.new.config.respond_to?(:bar) + + parent.config.compile_methods! + assert_equal :foo, parent.config.bar + assert_equal :foo, child.new.config.bar + + assert_respond_to parent.config, :bar + assert_respond_to child.config, :bar + assert_respond_to child.new.config, :bar + end end
\ No newline at end of file diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index fc8d8170a1..0f35eb9e78 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -282,7 +282,7 @@ class HashExtTest < Test::Unit::TestCase { :failure => "stuff", :funny => "business" }.assert_valid_keys(:failure, :funny) end - assert_raise(ArgumentError, "Unknown key(s): failore") do + assert_raise(ArgumentError, "Unknown key: failore") do { :failore => "stuff", :funny => "business" }.assert_valid_keys([ :failure, :funny ]) { :failore => "stuff", :funny => "business" }.assert_valid_keys(:failure, :funny) end @@ -462,20 +462,24 @@ class HashExtToParamTests < Test::Unit::TestCase assert_equal '', {}.to_param assert_equal 'hello=world', { :hello => "world" }.to_param assert_equal 'hello=10', { "hello" => 10 }.to_param - assert_equal 'hello=world&say_bye=true', ActiveSupport::OrderedHash[:hello, "world", "say_bye", true].to_param + assert_equal 'hello=world&say_bye=true', {:hello => "world", "say_bye" => true}.to_param end def test_number_hash - assert_equal '10=20&30=40&50=60', ActiveSupport::OrderedHash[10, 20, 30, 40, 50, 60].to_param + assert_equal '10=20&30=40&50=60', {10 => 20, 30 => 40, 50 => 60}.to_param end def test_to_param_hash - assert_equal 'custom=param-1&custom2=param2-1', ActiveSupport::OrderedHash[ToParam.new('custom'), ToParam.new('param'), ToParam.new('custom2'), ToParam.new('param2')].to_param + assert_equal 'custom2=param2-1&custom=param-1', {ToParam.new('custom') => ToParam.new('param'), ToParam.new('custom2') => ToParam.new('param2')}.to_param end def test_to_param_hash_escapes_its_keys_and_values assert_equal 'param+1=A+string+with+%2F+characters+%26+that+should+be+%3F+escaped', { 'param 1' => 'A string with / characters & that should be ? escaped' }.to_param end + + def test_to_param_orders_by_key_in_ascending_order + assert_equal 'a=2&b=1&c=0', ActiveSupport::OrderedHash[*%w(b 1 c 0 a 2)].to_param + end end class HashToXmlTest < Test::Unit::TestCase diff --git a/railties/guides/source/action_controller_overview.textile b/railties/guides/source/action_controller_overview.textile index 5c77d74df1..c02e9f1912 100644 --- a/railties/guides/source/action_controller_overview.textile +++ b/railties/guides/source/action_controller_overview.textile @@ -98,7 +98,7 @@ The value of +params[:ids]+ will now be +["1", "2", "3"]+. Note that parameter v To send a hash you include the key name inside the brackets: <html> -<form action="/clients" method="post"> +<form accept-charset="UTF-8" action="/clients" method="post"> <input type="text" name="client[name]" value="Acme" /> <input type="text" name="client[phone]" value="12345" /> <input type="text" name="client[address][postcode]" value="12345" /> @@ -115,10 +115,7 @@ h4. Routing Parameters The +params+ hash will always contain the +:controller+ and +:action+ keys, but you should use the methods +controller_name+ and +action_name+ instead to access these values. Any other parameters defined by the routing, such as +:id+ will also be available. As an example, consider a listing of clients where the list can show either active or inactive clients. We can add a route which captures the +:status+ parameter in a "pretty" URL: <ruby> -map.connect "/clients/:status", - :controller => "clients", - :action => "index", - :foo => "bar" +match '/clients/:status' => 'clients#index', :foo => "bar" </ruby> In this case, when a user opens the URL +/clients/active+, +params[:status]+ will be set to "active". When this route is used, +params[:foo]+ will also be set to "bar" just like it was passed in the query string. In the same way +params[:action]+ will contain "index". @@ -214,7 +211,7 @@ class ApplicationController < ActionController::Base # logging out removes it. def current_user @_current_user ||= session[:current_user_id] && - User.find(session[:current_user_id]) + User.find_by_id(session[:current_user_id]) end end </ruby> @@ -424,7 +421,7 @@ Around filters are responsible for running the action, but they can choose not t <ruby> # Example taken from the Rails API filter documentation: -# http://api.rubyonrails.org/classes/ActionController/Filters/ClassMethods.html +# http://ap.rubyonrails.org/classes/ActionController/Filters/ClassMethods.html class ApplicationController < ActionController::Base around_filter :catch_exceptions @@ -474,11 +471,11 @@ end Again, this is not an ideal example for this filter, because it's not run in the scope of the controller but gets the controller passed as an argument. The filter class has a class method +filter+ which gets run before or after the action, depending on if it's a before or after filter. Classes used as around filters can also use the same +filter+ method, which will get run in the same way. The method must +yield+ to execute the action. Alternatively, it can have both a +before+ and an +after+ method that are run before and after the action. -The Rails API documentation has "more information on using filters":http://api.rubyonrails.org/classes/ActionController/Filters/ClassMethods.html. +The Rails API documentation has "more information on using filters":http://ap.rubyonrails.org/classes/ActionController/Filters/ClassMethods.html. h3. Verification -Verifications make sure certain criteria are met in order for a controller or action to run. They can specify that a certain key (or several keys in the form of an array) is present in the +params+, +session+ or +flash+ hashes or that a certain HTTP method was used or that the request was made using +XMLHttpRequest+ (Ajax). The default action taken when these criteria are not met is to render a 400 Bad Request response, but you can customize this by specifying a redirect URL or rendering something else and you can also add flash messages and HTTP headers to the response. It is described in the "API documentation":http://api.rubyonrails.org/classes/ActionController/Verification/ClassMethods.html as "essentially a special kind of before_filter". +Verifications make sure certain criteria are met in order for a controller or action to run. They can specify that a certain key (or several keys in the form of an array) is present in the +params+, +session+ or +flash+ hashes or that a certain HTTP method was used or that the request was made using +XMLHttpRequest+ (Ajax). The default action taken when these criteria are not met is to render a 400 Bad Request response, but you can customize this by specifying a redirect URL or rendering something else and you can also add flash messages and HTTP headers to the response. It is described in the "API documentation":http://ap.rubyonrails.org/classes/ActionController/Verification/ClassMethods.html as "essentially a special kind of before_filter". Here's an example of using verification to make sure the user supplies a username and a password in order to log in: @@ -535,7 +532,7 @@ If you generate a form like this: You will see how the token gets added as a hidden field: <html> -<form action="/users/1" method="post"> +<form accept-charset="UTF-8" action="/users/1" method="post"> <input type="hidden" value="67250ab105eb5ad10851c00a5621854a23af5489" name="authenticity_token"/> @@ -555,7 +552,7 @@ In every controller there are two accessor methods pointing to the request and t h4. The +request+ Object -The request object contains a lot of useful information about the request coming in from the client. To get a full list of the available methods, refer to the "API documentation":http://api.rubyonrails.org/classes/ActionController/AbstractRequest.html. Among the properties that you can access on this object are: +The request object contains a lot of useful information about the request coming in from the client. To get a full list of the available methods, refer to the "API documentation":http://ap.rubyonrails.org/classes/ActionController/AbstractRequest.html. Among the properties that you can access on this object are: |_.Property of +request+|_.Purpose| |host|The hostname used for this request.| diff --git a/railties/guides/source/action_mailer_basics.textile b/railties/guides/source/action_mailer_basics.textile index 8eb48e2751..ac9052786e 100644 --- a/railties/guides/source/action_mailer_basics.textile +++ b/railties/guides/source/action_mailer_basics.textile @@ -242,6 +242,36 @@ end :class => 'photos' %> </erb> +h5. Sending Email To Multiple Recipients + +It is possible to send email to one or more recipients in one email (for e.g. informing all admins of a new signup) by setting the list of emails to the <tt>:to</tt> key. The <tt>to:</tt> key however expects a string so you have join the list of recipients using a comma. + +<ruby> + Class AdminMailer < ActionMailer::Base + default :to => Admin.all.map(&:email).join(", "), + :from => "notification@example.com" + + def new_registration(user) + @user = user + mail(:subject => "New User Signup: #{@user.email}") + end + end +</ruby> + +h5. Sending Email With Name + +Sometimes you wish to show the name of the person instead of just their email address when they receive the email. The trick to doing that is +to format the email address in the format <tt>"Name <email>"</tt>. + +<ruby> + def welcome_email(user) + @user = user + email_with_name = "#{@user.name} <#{@user.email}>" + mail(:to => email_with_name, + :subject => "Welcome to My Awesome Site") + end +</ruby> + h4. Mailer Views Mailer views are located in the +app/views/name_of_mailer_class+ directory. The specific mailer view is known to the class because it's name is the same as the mailer method. So for example, in our example from above, our mailer view for the +welcome_email+ method will be in +app/views/user_mailer/welcome_email.html.erb+ for the HTML version and +welcome_email.text.erb+ for the plain text version. diff --git a/railties/guides/source/active_record_querying.textile b/railties/guides/source/active_record_querying.textile index 1de1808bc7..6837c8b11a 100644 --- a/railties/guides/source/active_record_querying.textile +++ b/railties/guides/source/active_record_querying.textile @@ -769,13 +769,13 @@ Even though Active Record lets you specify conditions on the eager loaded associ h3. Dynamic Finders -For every field (also known as an attribute) you define in your table, Active Record provides a finder method. If you have a field called +first_name+ on your +Client+ model for example, you get +find_by_first_name+ and +find_all_by_first_name+ for free from Active Record. If you have also have a +locked+ field on the +Client+ model, you also get +find_by_locked+ and +find_all_by_locked+. +For every field (also known as an attribute) you define in your table, Active Record provides a finder method. If you have a field called +first_name+ on your +Client+ model for example, you get +find_by_first_name+ and +find_all_by_first_name+ for free from Active Record. If you have a +locked+ field on the +Client+ model, you also get +find_by_locked+ and +find_all_by_locked+ methods. -You can do +find_last_by_*+ methods too which will find the last record matching your argument. +You can also use +find_last_by_*+ methods which will find the last record matching your argument. You can specify an exclamation point (<tt>!</tt>) on the end of the dynamic finders to get them to raise an +ActiveRecord::RecordNotFound+ error if they do not return any records, like +Client.find_by_name!("Ryan")+ -If you want to find both by name and locked, you can chain these finders together by simply typing +and+ between the fields for example +Client.find_by_first_name_and_locked("Ryan", true)+. +If you want to find both by name and locked, you can chain these finders together by simply typing +and+ between the fields. For example, +Client.find_by_first_name_and_locked("Ryan", true)+. There's another set of dynamic finders that let you find or create/initialize objects if they aren't found. These work in a similar fashion to the other finders and can be used like +find_or_create_by_first_name(params[:first_name])+. Using this will first perform a find and then create if the find returns +nil+. The SQL looks like this for +Client.find_or_create_by_first_name("Ryan")+: diff --git a/railties/guides/source/association_basics.textile b/railties/guides/source/association_basics.textile index dbef9463a9..6996dab8c5 100644 --- a/railties/guides/source/association_basics.textile +++ b/railties/guides/source/association_basics.textile @@ -30,7 +30,7 @@ Now, suppose we wanted to add a new order for an existing customer. We'd need to Or consider deleting a customer, and ensuring that all of its orders get deleted as well: <ruby> -@orders = Order.find_all_by_customer_id(@customer.id) +@orders = Order.where(:customer_id => @customer.id) @orders.each do |order| order.destroy end @@ -550,7 +550,7 @@ build_customer create_customer </ruby> -h6. <tt>_association_(force_reload = false)</tt> +h6(#belongs_to-association). <tt>_association_(force_reload = false)</tt> The <tt><em>association</em></tt> method returns the associated object, if any. If no associated object is found, it returns +nil+. @@ -560,7 +560,7 @@ The <tt><em>association</em></tt> method returns the associated object, if any. If the associated object has already been retrieved from the database for this object, the cached version will be returned. To override this behavior (and force a database read), pass +true+ as the +force_reload+ argument. -h6. <tt>_association_=(associate)</tt> +h6(#belongs_to-association_equal). <tt>_association_=(associate)</tt> The <tt><em>association</em>=</tt> method assigns an associated object to this object. Behind the scenes, this means extracting the primary key from the associate object and setting this object's foreign key to the same value. @@ -637,7 +637,7 @@ class Order < ActiveRecord::Base end </ruby> -h6. +:counter_cache+ +h6(#belongs_to-counter_cache). +:counter_cache+ The +:counter_cache+ option can be used to make finding the number of belonging objects more efficient. Consider these models: @@ -733,7 +733,7 @@ end NOTE: There's no need to use +:include+ for immediate associations - that is, if you have +Order belongs_to :customer+, then the customer is eager-loaded automatically when it's needed. -h6. +:polymorphic+ +h6(#belongs_to-polymorphic). +:polymorphic+ Passing +true+ to the +:polymorphic+ option indicates that this is a polymorphic association. Polymorphic associations were discussed in detail <a href="#polymorphic-associations">earlier in this guide</a>. @@ -747,7 +747,7 @@ The +:select+ option lets you override the SQL +SELECT+ clause that is used to r TIP: If you set the +:select+ option on a +belongs_to+ association, you should also set the +foreign_key+ option to guarantee the correct results. -h6. +:touch+ +h6(#belongs_to-touch). +:touch+ If you set the +:touch+ option to +:true+, then the +updated_at+ or +updated_on+ timestamp on the associated object will be set to the current time whenever this object is saved or destroyed: @@ -817,7 +817,7 @@ build_account create_account </ruby> -h6. <tt><em>association</em>(force_reload = false)</tt> +h6(#has_one-association). <tt><em>association</em>(force_reload = false)</tt> The <tt><em>association</em></tt> method returns the associated object, if any. If no associated object is found, it returns +nil+. @@ -827,7 +827,7 @@ The <tt><em>association</em></tt> method returns the associated object, if any. If the associated object has already been retrieved from the database for this object, the cached version will be returned. To override this behavior (and force a database read), pass +true+ as the +force_reload+ argument. -h6. <tt><em>association</em>=(associate)</tt> +h6(#has_one-association_equal). <tt><em>association</em>=(associate)</tt> The <tt><em>association</em>=</tt> method assigns an associated object to this object. Behind the scenes, this means extracting the primary key from this object and setting the associate object's foreign key to the same value. @@ -1029,6 +1029,7 @@ When you declare a +has_many+ association, the declaring class automatically gai * <tt><em>collection</em>.empty?</tt> * <tt><em>collection</em>.size</tt> * <tt><em>collection</em>.find(...)</tt> +* <tt><em>collection</em>.where(...)</tt> * <tt><em>collection</em>.exists?(...)</tt> * <tt><em>collection</em>.build(attributes = {}, ...)</tt> * <tt><em>collection</em>.create(attributes = {})</tt> @@ -1054,6 +1055,7 @@ orders.clear orders.empty? orders.size orders.find(...) +orders.where(...) orders.exists?(...) orders.build(attributes = {}, ...) orders.create(attributes = {}) @@ -1083,10 +1085,10 @@ The <tt><em>collection</em>.delete</tt> method removes one or more objects from @customer.orders.delete(@order1) </ruby> -WARNING: Objects will be in addition destroyed if they're associated with +:dependent => :destroy+, and deleted if they're associated with +:dependent => :delete_all+. +WARNING: Additionally, objects will be destroyed if they're associated with +:dependent => :destroy+, and deleted if they're associated with +:dependent => :delete_all+. -h6(#has_many-collection_equal). <tt><em>collection</em>=objects</tt> +h6(#has_many-collection-equal). <tt><em>collection</em>=objects</tt> The <tt><em>collection</em>=</tt> method makes the collection contain only the supplied objects, by adding and deleting as appropriate. @@ -1102,11 +1104,11 @@ h6(#has_many-collection_singular_ids_ids). <tt><em>collection_singular</em>_ids= The <tt><em>collection_singular</em>_ids=</tt> method makes the collection contain only the objects identified by the supplied primary key values, by adding and deleting as appropriate. -h6(#has_many-collection_clear). <tt><em>collection</em>.clear</tt> +h6(#has_many-collection-clear). <tt><em>collection</em>.clear</tt> The <tt><em>collection</em>.clear</tt> method removes every object from the collection. This destroys the associated objects if they are associated with +:dependent => :destroy+, deletes them directly from the database if +:dependent => :delete_all+, and otherwise sets their foreign keys to +NULL+. -h6. <tt><em>collection</em>.empty?</tt> +h6(#has_many-collection-empty). <tt><em>collection</em>.empty?</tt> The <tt><em>collection</em>.empty?</tt> method returns +true+ if the collection does not contain any associated objects. @@ -1116,7 +1118,7 @@ The <tt><em>collection</em>.empty?</tt> method returns +true+ if the collection <% end %> </ruby> -h6. <tt><em>collection</em>.size</tt> +h6(#has_many-collection-size). <tt><em>collection</em>.size</tt> The <tt><em>collection</em>.size</tt> method returns the number of objects in the collection. @@ -1124,7 +1126,7 @@ The <tt><em>collection</em>.size</tt> method returns the number of objects in th @order_count = @customer.orders.size </ruby> -h6. <tt><em>collection</em>.find(...)</tt> +h6(#has_many-collection-find). <tt><em>collection</em>.find(...)</tt> The <tt><em>collection</em>.find</tt> method finds objects within the collection. It uses the same syntax and options as +ActiveRecord::Base.find+. @@ -1132,11 +1134,22 @@ The <tt><em>collection</em>.find</tt> method finds objects within the collection @open_orders = @customer.orders.find(:all, :conditions => "open = 1") </ruby> -h6. <tt><em>collection</em>.exists?(...)</tt> +NOTE: Starting Rails 3, supplying options to +ActiveRecord::Base.find+ method is discouraged. Use <tt><em>collection</em>.where</tt> instead when you need to pass conditions. + +h6(#has_many-collection-where). <tt><em>collection</em>.where(...)</tt> + +The <tt><em>collection</em>.where</tt> method finds objects within the collection based on the conditions supplied but the objects are loaded lazily meaning that the database is queried only when the object(s) are accessed. + +<ruby> +@open_orders = @customer.orders.where(:open => true) # No query yet +@open_order = @open_orders.first # Now the database will be queried +</ruby> + +h6(#has_many-collection-exists). <tt><em>collection</em>.exists?(...)</tt> The <tt><em>collection</em>.exists?</tt> method checks whether an object meeting the supplied conditions exists in the collection. It uses the same syntax and options as +ActiveRecord::Base.exists?+. -h6(#has_many_collection_build). <tt><em>collection</em>.build(attributes = {}, ...)</tt> +h6(#has_many-collection-build). <tt><em>collection</em>.build(attributes = {}, ...)</tt> The <tt><em>collection</em>.build</tt> method returns one or more new objects of the associated type. These objects will be instantiated from the passed attributes, and the link through their foreign key will be created, but the associated objects will _not_ yet be saved. @@ -1145,7 +1158,7 @@ The <tt><em>collection</em>.build</tt> method returns one or more new objects of :order_number => "A12345") </ruby> -h6. <tt><em>collection</em>.create(attributes = {})</tt> +h6(#has_many-collection-create). <tt><em>collection</em>.create(attributes = {})</tt> The <tt><em>collection</em>.create</tt> method returns a new object of the associated type. This object will be instantiated from the passed attributes, the link through its foreign key will be created, and the associated object _will_ be saved (assuming that it passes any validations). @@ -1193,7 +1206,7 @@ h6(#has_many-as). +:as+ Setting the +:as+ option indicates that this is a polymorphic association, as discussed <a href="#polymorphic-associations">earlier in this guide</a>. -h6. +:autosave+ +h6(#has_many-autosave). +:autosave+ If you set the +:autosave+ option to +true+, Rails will save any loaded members and destroy members that are marked for destruction whenever you save the parent object. @@ -1439,11 +1452,12 @@ When you declare a +has_and_belongs_to_many+ association, the declaring class au * <tt><em>collection</em>.empty?</tt> * <tt><em>collection</em>.size</tt> * <tt><em>collection</em>.find(...)</tt> +* <tt><em>collection</em>.where(...)</tt> * <tt><em>collection</em>.exists?(...)</tt> * <tt><em>collection</em>.build(attributes = {})</tt> * <tt><em>collection</em>.create(attributes = {})</tt> -In all of these methods, <tt><em>collection</em></tt> is replaced with the symbol passed as the first argument to +has_and_belongs_to_many+, and <tt><em>collection_singular</em></tt> is replaced with the singularized version of that symbol.. For example, given the declaration: +In all of these methods, <tt><em>collection</em></tt> is replaced with the symbol passed as the first argument to +has_and_belongs_to_many+, and <tt><em>collection_singular</em></tt> is replaced with the singularized version of that symbol. For example, given the declaration: <ruby> class Part < ActiveRecord::Base @@ -1464,6 +1478,7 @@ assemblies.clear assemblies.empty? assemblies.size assemblies.find(...) +assemblies.where(...) assemblies.exists?(...) assemblies.build(attributes = {}, ...) assemblies.create(attributes = {}) @@ -1476,7 +1491,7 @@ If the join table for a +has_and_belongs_to_many+ association has additional col WARNING: The use of extra attributes on the join table in a +has_and_belongs_to_many+ association is deprecated. If you require this sort of complex behavior on the table that joins two models in a many-to-many relationship, you should use a +has_many :through+ association instead of +has_and_belongs_to_many+. -h6. <tt><em>collection</em>(force_reload = false)</tt> +h6(#has_and_belongs_to_many-collection). <tt><em>collection</em>(force_reload = false)</tt> The <tt><em>collection</em></tt> method returns an array of all of the associated objects. If there are no associated objects, it returns an empty array. @@ -1484,7 +1499,7 @@ The <tt><em>collection</em></tt> method returns an array of all of the associate @assemblies = @part.assemblies </ruby> -h6. <tt><em>collection</em><<(object, ...)</tt> +h6(#has_and_belongs_to_many-collection-lt_lt). <tt><em>collection</em><<(object, ...)</tt> The <tt><em>collection</em><<</tt> method adds one or more objects to the collection by creating records in the join table. @@ -1494,7 +1509,7 @@ The <tt><em>collection</em><<</tt> method adds one or more objects to the collec NOTE: This method is aliased as <tt><em>collection</em>.concat</tt> and <tt><em>collection</em>.push</tt>. -h6. <tt><em>collection</em>.delete(object, ...)</tt> +h6(#has_and_belongs_to_many-collection-delete). <tt><em>collection</em>.delete(object, ...)</tt> The <tt><em>collection</em>.delete</tt> method removes one or more objects from the collection by deleting records in the join table. This does not destroy the objects. @@ -1502,11 +1517,11 @@ The <tt><em>collection</em>.delete</tt> method removes one or more objects from @part.assemblies.delete(@assembly1) </ruby> -h6. <tt><em>collection</em>=objects</tt> +h6(#has_and_belongs_to_many-collection-equal). <tt><em>collection</em>=objects</tt> The <tt><em>collection</em>=</tt> method makes the collection contain only the supplied objects, by adding and deleting as appropriate. -h6. <tt><em>collection_singular</em>_ids</tt> +h6(#has_and_belongs_to_many-collection_singular). <tt><em>collection_singular</em>_ids</tt> The <tt><em>collection_singular</em>_ids</tt> method returns an array of the ids of the objects in the collection. @@ -1514,11 +1529,11 @@ The <tt><em>collection_singular</em>_ids</tt> method returns an array of the ids @assembly_ids = @part.assembly_ids </ruby> -h6. <tt><em>collection_singular</em>_ids=ids</tt> +h6(#has_and_belongs_to_many-collection_singular_ids_ids). <tt><em>collection_singular</em>_ids=ids</tt> The <tt><em>collection_singular</em>_ids=</tt> method makes the collection contain only the objects identified by the supplied primary key values, by adding and deleting as appropriate. -h6. <tt><em>collection</em>.clear</tt> +h6(#has_and_belongs_to_many-collection-clear). <tt><em>collection</em>.clear</tt> The <tt><em>collection</em>.clear</tt> method removes every object from the collection by deleting the rows from the joining table. This does not destroy the associated objects. @@ -1549,6 +1564,16 @@ The <tt><em>collection</em>.find</tt> method finds objects within the collection :conditions => ["created_at > ?", 2.days.ago]) </ruby> +NOTE: Starting Rails 3, supplying options to +ActiveRecord::Base.find+ method is discouraged. Use <tt><em>collection</em>.where</tt> instead when you need to pass conditions. + +h6(#has_and_belongs_to_many-collection-where). <tt><em>collection</em>.where(...)</tt> + +The <tt><em>collection</em>.where</tt> method finds objects within the collection based on the conditions supplied but the objects are loaded lazily meaning that the database is queried only when the object(s) are accessed. It also adds the additional condition that the object must be in the collection. + +<ruby> +@new_assemblies = @part.assemblies.where("created_at > ?", 2.days.ago) +</ruby> + h6(#has_and_belongs_to_many-collection-exists). <tt><em>collection</em>.exists?(...)</tt> The <tt><em>collection</em>.exists?</tt> method checks whether an object meeting the supplied conditions exists in the collection. It uses the same syntax and options as +ActiveRecord::Base.exists?+. @@ -1605,7 +1630,7 @@ The +has_and_belongs_to_many+ association supports these options: * +:uniq+ * +:validate+ -h6. +:association_foreign_key+ +h6(#has_and_belongs_to_many-association_foreign_key). +:association_foreign_key+ By convention, Rails guesses that the column in the join table used to hold the foreign key pointing to the other model is the name of that model with the suffix +_id+ added. The +:association_foreign_key+ option lets you set the name of the foreign key directly: @@ -1661,7 +1686,7 @@ Normally Rails automatically generates the proper SQL to count the association m NOTE: If you specify +:finder_sql+ but not +:counter_sql+, then the counter SQL will be generated by substituting +SELECT COUNT(*) FROM+ for the +SELECT ... FROM+ clause of your +:finder_sql+ statement. -h6. +:delete_sql+ +h6(#has_and_belongs_to_many-delete_sql). +:delete_sql+ Normally Rails automatically generates the proper SQL to remove links between the associated classes. With the +:delete_sql+ option, you can specify a complete SQL statement to delete them yourself. @@ -1699,11 +1724,11 @@ h6(#has_and_belongs_to_many-include). +:include+ You can use the +:include+ option to specify second-order associations that should be eager-loaded when this association is used. -h6. +:insert_sql+ +h6(#has_and_belongs_to_many-insert_sql). +:insert_sql+ Normally Rails automatically generates the proper SQL to create links between the associated classes. With the +:insert_sql+ option, you can specify a complete SQL statement to insert them yourself. -h6. +:join_table+ +h6(#has_and_belongs_to_many-join_table). +:join_table+ If the default name of the join table, based on lexical ordering, is not what you want, you can use the +:join_table+ option to override the default. @@ -1821,7 +1846,7 @@ If you have an extension that should be shared by many associations, you can use <ruby> module FindRecentExtension def find_recent - find(:all, :conditions => ["created_at > ?", 5.days.ago]) + where("created_at > ?", 5.days.ago) end end diff --git a/railties/guides/source/caching_with_rails.textile b/railties/guides/source/caching_with_rails.textile index 09cd796271..3320b610e7 100644 --- a/railties/guides/source/caching_with_rails.textile +++ b/railties/guides/source/caching_with_rails.textile @@ -362,20 +362,9 @@ class ProductsController < ApplicationController end </ruby> -h3. Advanced Caching - -Along with the built-in mechanisms outlined above, a number of excellent plugins exist to help with finer grained control over caching. These include Chris Wanstrath's excellent cache_fu plugin (more info "here":http://errtheblog.com/posts/57-kickin-ass-w-cachefu ) and Evan Weaver's interlock plugin (more info "here":http://blog.evanweaver.com/articles/2007/12/13/better-rails-caching/ ). Both of these plugins play nice with memcached and are a must-see for anyone -seriously considering optimizing their caching needs. - -Also the new "Cache money":http://github.com/ngmoco/cache-money/tree/rails3 plugin is supposed to be mad cool. - -h3. References +h3. Further reading * "Scaling Rails Screencasts":http://railslab.newrelic.com/scaling-rails -* "RailsEnvy, Rails Caching Tutorial, Part 1":http://www.railsenvy.com/2007/2/28/rails-caching-tutorial -* "RailsEnvy, Rails Caching Tutorial, Part 2":http://www.railsenvy.com/2007/3/20/ruby-on-rails-caching-tutorial-part-2 -* "ActiveSupport::Cache documentation":http://api.rubyonrails.org/classes/ActiveSupport/Cache.html -* "Rails 2.1 integrated caching tutorial":http://thewebfellas.com/blog/2008/6/9/rails-2-1-now-with-better-integrated-caching h3. Changelog diff --git a/railties/guides/source/form_helpers.textile b/railties/guides/source/form_helpers.textile index e3b9d745b2..ce0f66b2c1 100644 --- a/railties/guides/source/form_helpers.textile +++ b/railties/guides/source/form_helpers.textile @@ -31,17 +31,18 @@ When called without arguments like this, it creates a form element that has the Sample output from +form_tag+: <html> -<form action="/home/index" method="post"> +<form accept-charset="UTF-8" action="/home/index" method="post"> <div style="margin:0;padding:0"> + <input name="utf8" type="hidden" value="✓" /> <input name="authenticity_token" type="hidden" value="f755bb0ed134b76c432144748a6d4b7a7ddf2b71" /> </div> Form contents </form> </html> -If you carefully observe this output, you can see that the helper generated something you didn't specify: a +div+ element with a hidden input inside. This is a security feature of Rails called *cross-site request forgery protection* and form helpers generate it for every form whose action is not "get" (provided that this security feature is enabled). You can read more about this in the "Ruby On Rails Security Guide":./security.html#_cross_site_reference_forgery_csrf. +If you carefully observe this output, you can see that the helper generated something you didn't specify: a +div+ element with two hidden input elements inside. The first input element with name +utf8+ enforces browsers to properly respect your form's character encoding and is generated for all forms whether action is "get" or "post". Second input element with name +authenticity_token+ is a security feature of Rails called *cross-site request forgery protection* and form helpers generate it for every form whose action is not "get" (provided that this security feature is enabled). You can read more about this in the "Ruby On Rails Security Guide":./security.html#_cross_site_reference_forgery_csrf. -NOTE: Throughout this guide, this +div+ with the hidden input will be stripped away to have clearer code samples. +NOTE: Throughout this guide, this +div+ with the hidden input elements will be stripped away to have clearer code samples. h4. A Generic Search Form @@ -71,7 +72,7 @@ TIP: +search_path+ can be a named route specified in "routes.rb": <br /><tt>map. The above view code will result in the following markup: <html> -<form action="/search" method="get"> +<form accept-charset="UTF-8" action="/search" method="get"> <label for="q">Search for:</label> <input id="q" name="q" type="text" /> <input name="commit" type="submit" value="Search" /> @@ -90,14 +91,14 @@ As with the +link_to+ helper, the path argument doesn't have to be given a strin <ruby> form_tag(:controller => "people", :action => "search", :method => "get", :class => "nifty_form") -# => <form action="/people/search?method=get&class=nifty_form" method="post"> +# => <form accept-charset="UTF-8" action="/people/search?method=get&class=nifty_form" method="post"> </ruby> Here you wanted to pass two hashes, but the Ruby interpreter sees only one hash, so Rails will construct a URL with extraneous parameters. The correct way of passing multiple hashes as arguments is to delimit the first hash (or both hashes) with curly brackets: <ruby> form_tag({:controller => "people", :action => "search"}, :method => "get", :class => "nifty_form") -# => <form action="/people/search" method="get" class="nifty_form"> +# => <form accept-charset="UTF-8" action="/people/search" method="get" class="nifty_form"> </ruby> This is a common pitfall when using form helpers, since many of them accept multiple hashes. So in future, if a helper produces unexpected output, make sure that you have delimited the hash parameters properly. @@ -239,7 +240,7 @@ There are a few things to note here: The resulting HTML is: <html> -<form action="/articles/create" method="post" class="nifty_form"> +<form accept-charset="UTF-8" action="/articles/create" method="post" class="nifty_form"> <input id="article_title" name="article[title]" size="30" type="text" /> <textarea id="article_body" name="article[body]" cols="60" rows="12"></textarea> <input name="commit" type="submit" value="Create" /> @@ -264,7 +265,7 @@ You can create a similar binding without actually creating +<form>+ tags which produces the following output: <html> -<form action="/people/create" class="new_person" id="new_person" method="post"> +<form accept-charset="UTF-8" action="/people/create" class="new_person" id="new_person" method="post"> <input id="person_name" name="person[name]" size="30" type="text" /> <input id="contact_detail_phone_number" name="contact_detail[phone_number]" size="30" type="text" /> </form> @@ -334,9 +335,10 @@ form_tag(search_path, :method => "put") output: <html> -<form action="/search" method="post"> +<form accept-charset="UTF-8" action="/search" method="post"> <div style="margin:0;padding:0"> <input name="_method" type="hidden" value="put" /> + <input name="utf8" type="hidden" value="✓" /> <input name="authenticity_token" type="hidden" value="f755bb0ed134b76c432144748a6d4b7a7ddf2b71" /> </div> ... @@ -633,7 +635,12 @@ action for a Person model, +params[:model]+ would usually be a hash of all the a Fundamentally HTML forms don't know about any sort of structured data, all they generate is name–value pairs, where pairs are just plain strings. The arrays and hashes you see in your application are the result of some parameter naming conventions that Rails uses. -TIP: You may find you can try out examples in this section faster by using the console to directly invoke Rails' parameter parser. For example <tt> ActionController::UrlEncodedPairParser.parse_query_parameters "name=fred&phone=0123456789" # => {"name"=>"fred", "phone"=>"0123456789"} </tt> +TIP: You may find you can try out examples in this section faster by using the console to directly invoke Rails' parameter parser. For example, + +<ruby> +ActionController::UrlEncodedPairParser.parse_query_parameters "name=fred&phone=0123456789" +# => {"name"=>"fred", "phone"=>"0123456789"} +</ruby> h4. Basic Structures @@ -709,7 +716,7 @@ You might want to render a form with a set of edit fields for each of a person's Assuming the person had two addresses, with ids 23 and 45 this would create output similar to this: <html> -<form action="/people/1" class="edit_person" id="edit_person_1" method="post"> +<form accept-charset="UTF-8" action="/people/1" class="edit_person" id="edit_person_1" method="post"> <input id="person_name" name="person[name]" size="30" type="text" /> <input id="person_address_23_city" name="person[address][23][city]" size="30" type="text" /> <input id="person_address_45_city" name="person[address][45][city]" size="30" type="text" /> diff --git a/railties/guides/source/layouts_and_rendering.textile b/railties/guides/source/layouts_and_rendering.textile index 14107eb6f5..50c5986a64 100644 --- a/railties/guides/source/layouts_and_rendering.textile +++ b/railties/guides/source/layouts_and_rendering.textile @@ -27,7 +27,7 @@ I'll cover each of these methods in turn. But first, a few words about the very h4. Rendering by Default: Convention Over Configuration in Action -You've heard that Rails promotes "convention over configuration." Default rendering is an excellent example of this. By default, controllers in Rails automatically render views with names that correspond to valid routes. For example, if you have this code in your +BooksController+ class: +You've heard that Rails promotes "convention over configuration". Default rendering is an excellent example of this. By default, controllers in Rails automatically render views with names that correspond to valid routes. For example, if you have this code in your +BooksController+ class: <ruby> class BooksController < ApplicationController @@ -46,7 +46,7 @@ And you have a view file +app/views/books/index.html.erb+: <h1>Books are coming soon!</h1> </ruby> -Rails will automatically render +app/views/books/index.html.erb+ when you navigate to +/books+ and you will see on your screen that "Books are coming soon!" +Rails will automatically render +app/views/books/index.html.erb+ when you navigate to +/books+ and you will see "Books are coming soon!" on your screen. However a coming soon screen is only minimally useful, so you will soon create your +Book+ model and add the index action to +BooksController+: @@ -58,9 +58,9 @@ class BooksController < ApplicationController end </ruby> -Note that again, we have convention over configuration, in that there is no explicit render at the end of this index action. The rule is that if you do not explicitly render something by the end of the controller action, rails will look for the +action_name.html.erb+ template in the controllers view path and then render that, so in this case, Rails will render the +app/views/books/index.html.erb+ file. +Note that we don't have explicit render at the end of the index action in accordance with "convention over configuration" principle. The rule is that if you do not explicitly render something at the end of a controller action, Rails will automatically look for the +action_name.html.erb+ template in the controller's view path and render it. So in this case, Rails will render the +app/views/books/index.html.erb+ file. -So in our view, we want to display the properties of all the books, we could do this with an ERB template like this: +If we want to display the properties of all the books in our view, we can do so with an ERB template like this: <ruby> <h1>Listing Books</h1> @@ -123,7 +123,7 @@ Cache-Control: no-cache $ </shell> -We see there is an empty response (no data after the +Cache-Control+ line), but that Rails has set the response to 200 OK, so the request was successful. You can set the +:status+ options on render to change this response. Rendering nothing can be useful for AJAX requests where all you want to send back to the browser is an acknowledgement that the request was completed. +We see there is an empty response (no data after the +Cache-Control+ line), but the request was successful because Rails has set the response to 200 OK. You can set the +:status+ options on render to change this response. Rendering nothing can be useful for AJAX requests where all you want to send back to the browser is an acknowledgement that the request was completed. TIP: You should probably be using the +head+ method, discussed later in this guide, instead of +render :nothing+. This provides additional flexibility and makes it explicit that you're only generating HTTP headers. @@ -134,11 +134,10 @@ If you want to render the view that corresponds to a different action within the <ruby> def update @book = Book.find(params[:id]) - if @book.update_attributes(params[:book]) - redirect_to(@book) - else - render "edit" - end + if @book.update_attributes(params[:book]) + redirect_to(@book) + else + render "edit" end end </ruby> @@ -150,11 +149,10 @@ If you prefer, you can use a symbol instead of a string to specify the action to <ruby> def update @book = Book.find(params[:id]) - if @book.update_attributes(params[:book]) - redirect_to(@book) - else - render :edit - end + if @book.update_attributes(params[:book]) + redirect_to(@book) + else + render :edit end end </ruby> @@ -164,11 +162,10 @@ To be explicit, you can use +render+ with the +:action+ option (though this is n <ruby> def update @book = Book.find(params[:id]) - if @book.update_attributes(params[:book]) - redirect_to(@book) - else - render :action => "edit" - end + if @book.update_attributes(params[:book]) + redirect_to(@book) + else + render :action => "edit" end end </ruby> @@ -208,13 +205,13 @@ The +:file+ option takes an absolute file-system path. Of course, you need to ha NOTE: By default, the file is rendered without using the current layout. If you want Rails to put the file into the current layout, you need to add the +:layout => true+ option. -TIP: If you're running on Microsoft Windows, you should use the +:file+ option to render a file, because Windows filenames do not have the same format as Unix filenames. +TIP: If you're running Rails on Microsoft Windows, you should use the +:file+ option to render a file, because Windows filenames do not have the same format as Unix filenames. h5. Wrapping it up -The above three methods of render (rendering another template within the controller, rendering a template within another controller and rendering an arbitrary file on the file system) are actually all variants of the same action. +The above three ways of rendering (rendering another template within the controller, rendering a template within another controller and rendering an arbitrary file on the file system) are actually variants of the same action. -In fact, in the BooksController method, inside of the edit action where we want to render the edit template if the book does not update successfully, all of the following render calls would all render the +edit.html.erb+ template in the +views/books+ directory: +In fact, in the BooksController class, inside of the edit action where we want to render the edit template if the book does not update successfully, all of the following render calls would all render the +edit.html.erb+ template in the +views/books+ directory: <ruby> render :edit @@ -241,12 +238,12 @@ The +render+ method can do without a view completely, if you're willing to use t <ruby> render :inline => - "<% products.each do |p| %><p><%= p.name %><p><% end %>" + "<% products.each do |p| %><p><%= p.name %></p><% end %>" </ruby> WARNING: There is seldom any good reason to use this option. Mixing ERB into your controllers defeats the MVC orientation of Rails and will make it harder for other developers to follow the logic of your project. Use a separate erb view instead. -By default, inline rendering uses ERb. You can force it to use Builder instead with the +:type+ option: +By default, inline rendering uses ERB. You can force it to use Builder instead with the +:type+ option: <ruby> render :inline => @@ -275,7 +272,7 @@ render :text => "OK" TIP: Rendering pure text is most useful when you're responding to AJAX or web service requests that are expecting something other than proper HTML. -NOTE: By default, if you use the +:text+ option the text is rendered without using the current layout. If you want Rails to put the text into the current layout, you need to add the +:layout => true+ option. +NOTE: By default, if you use the +:text+ option, the text is rendered without using the current layout. If you want Rails to put the text into the current layout, you need to add the +:layout => true+ option. h5. Rendering JSON @@ -498,7 +495,7 @@ def show end </ruby> -If +@book.special?+ evaluates to +true+, Rails will start the rendering process to dump the +@book+ variable into the +special_show+ view. But this will _not_ stop the rest of the code in the +show+ action from running, and when Rails hits the end of the action, it will start to render the +show+ view - and throw an error. The solution is simple: make sure that you only have one call to +render+ or +redirect+ in a single code path. One thing that can help is +and return+. Here's a patched version of the method: +If +@book.special?+ evaluates to +true+, Rails will start the rendering process to dump the +@book+ variable into the +special_show+ view. But this will _not_ stop the rest of the code in the +show+ action from running, and when Rails hits the end of the action, it will start to render the +regular_show+ view - and throw an error. The solution is simple: make sure that you have only one call to +render+ or +redirect+ in a single code path. One thing that can help is +and return+. Here's a patched version of the method: <ruby> def show @@ -568,7 +565,7 @@ def show end </ruby> -With the code in this form, there will be likely be a problem if the +@book+ variable is +nil+. Remember, a +render :action+ doesn't run any code in the target action, so nothing will set up the +@books+ variable that the +index+ view is presumably depending on. One way to fix this is to redirect instead of rendering: +With the code in this form, there will likely be a problem if the +@book+ variable is +nil+. Remember, a +render :action+ doesn't run any code in the target action, so nothing will set up the +@books+ variable that the +index+ view is presumably depending on. One way to fix this is to redirect instead of rendering: <ruby> def index @@ -585,7 +582,7 @@ end With this code, the browser will make a fresh request for the index page, the code in the +index+ method will run, and all will be well. -The only downside to this code, is that it requires a round trip to the browser, the browser requested the show action with +/books/1+ and the controller finds that there are no books, so the controller sends out a 301 redirect response to the browser telling it to go to +/books/+, the browser complies and sends a new request back to the controller asking now for the +index+ action, the controller then gets all the books in the database and renders the index template, sending it back down to the browser which then shows it on your screen. +The only downside to this code, is that it requires a round trip to the browser, the browser requested the show action with +/books/1+ and the controller finds that there are no books, so the controller sends out a 302 redirect response to the browser telling it to go to +/books/+, the browser complies and sends a new request back to the controller asking now for the +index+ action, the controller then gets all the books in the database and renders the index template, sending it back down to the browser which then shows it on your screen. While in a small app, this added latency might not be a problem, it is something to think about when speed of response is of the essence. One way to handle this double request (though a contrived example) could be: @@ -603,7 +600,7 @@ def show end </ruby> -Which would detect that there are no books populate the +@books+ instance variable with all the books in the database and then directly render the +index.html.erb+ template returning it to the browser with a flash alert message telling the user what happened. +Which would detect that there are no books, populate the +@books+ instance variable with all the books in the database and then directly render the +index.html.erb+ template returning it to the browser with a flash alert message telling the user what happened. h4. Using +head+ To Build Header-Only Responses @@ -806,7 +803,9 @@ You can even use dynamic paths such as +cache/#{current_site}/main/display+. h5. Linking to Images with +image_tag+ -The +image_tag+ helper builds an HTML +<img />+ tag to the specified file. By default, files are loaded from +public/images+, note, you must specify the extension, previous versions of Rails would allow you to just call the image name and would append +.png+ if no extension was given, Rails 3.0 does not. +The +image_tag+ helper builds an HTML +<img />+ tag to the specified file. By default, files are loaded from +public/images+. + +WARNING: Note that you must specify the extension of the image. Previous versions of Rails would allow you to just use the image name and would append +.png+ if no extension was given but Rails 3.0 does not. <erb> <%= image_tag "header.png" %> @@ -843,7 +842,7 @@ You can also specify a special size tag, in the format "{width}x{height}": <%= image_tag "home.gif", :size => "50x20" %> </erb> -In addition to the above special tags, you can supply a final hash of standard HTML options, such as +:class+ or +:id+ or +:name+: +In addition to the above special tags, you can supply a final hash of standard HTML options, such as +:class+, +:id+ or +:name+: <erb> <%= image_tag "home.gif", :alt => "Go Home", @@ -1087,7 +1086,7 @@ Partials are very useful in rendering collections. When you pass a collection to When a partial is called with a pluralized collection, then the individual instances of the partial have access to the member of the collection being rendered via a variable named after the partial. In this case, the partial is +_product+, and within the +_product+ partial, you can refer to +product+ to get the instance that is being rendered. -In Rails 3.0 there is also a shorthand for this, assuming +@products+ is a collection of +product+ instances, you can simply do in the +index.html.erb+: +In Rails 3.0, there is also a shorthand for this. Assuming +@products+ is a collection of +product+ instances, you can simply write this in the +index.html.erb+: <erb> <h1>Products</h1> diff --git a/railties/guides/source/routing.textile b/railties/guides/source/routing.textile index cfba58d8da..7d4fb69d3b 100644 --- a/railties/guides/source/routing.textile +++ b/railties/guides/source/routing.textile @@ -161,15 +161,15 @@ end This will create a number of routes for each of the +posts+ and +comments+ controller. For +Admin::PostsController+, Rails will create: |_. Verb |_.Path |_.action |_. helper | -|GET |/admin/photos |index | admin_photos_path | -|GET |/admin/photos/new |new | new_admin_photos_path | -|POST |/admin/photos |create | admin_photos_path | -|GET |/admin/photos/1 |show | admin_photo_path(id) | -|GET |/admin/photos/1/edit |edit | edit_admin_photo_path(id) | -|PUT |/admin/photos/1 |update | admin_photo_path(id) | -|DELETE |/admin/photos/1 |destroy | admin_photo_path(id) | +|GET |/admin/posts |index | admin_posts_path | +|GET |/admin/posts/new |new | new_admin_posts_path | +|POST |/admin/posts |create | admin_posts_path | +|GET |/admin/posts/1 |show | admin_post_path(id) | +|GET |/admin/posts/1/edit |edit | edit_admin_post_path(id) | +|PUT |/admin/posts/1 |update | admin_post_path(id) | +|DELETE |/admin/posts/1 |destroy | admin_post_path(id) | -If you want to route +/photos+ (without the prefix +/admin+) to +Admin::PostsController+, you could use +If you want to route +/posts+ (without the prefix +/admin+) to +Admin::PostsController+, you could use <ruby> scope :module => "admin" do @@ -183,7 +183,7 @@ or, for a single case resources :posts, :module => "admin" </ruby> -If you want to route +/admin/photos+ to +PostsController+ (without the +Admin::+ module prefix), you could use +If you want to route +/admin/posts+ to +PostsController+ (without the +Admin::+ module prefix), you could use <ruby> scope "/admin" do @@ -200,13 +200,13 @@ resources :posts, :path => "/admin" In each of these cases, the named routes remain the same as if you did not use +scope+. In the last case, the following paths map to +PostsController+: |_. Verb |_.Path |_.action |_. helper | -|GET |/admin/photos |index | photos_path | -|GET |/admin/photos/new |new | photos_path | -|POST |/admin/photos |create | photos_path | -|GET |/admin/photos/1 |show | photo_path(id) | -|GET |/admin/photos/1/edit |edit | edit_photo_path(id) | -|PUT |/admin/photos/1 |update | photo_path(id) | -|DELETE |/admin/photos/1 |destroy | photo_path(id) | +|GET |/admin/posts |index | posts_path | +|GET |/admin/posts/new |new | posts_path | +|POST |/admin/posts |create | posts_path | +|GET |/admin/posts/1 |show | post_path(id) | +|GET |/admin/posts/1/edit |edit | edit_post_path(id) | +|PUT |/admin/posts/1 |update | post_path(id) | +|DELETE |/admin/posts/1 |destroy | post_path(id) | h4. Nested Resources @@ -589,7 +589,7 @@ The +:controller+ option lets you explicitly specify a controller to use for the resources :photos, :controller => "images" </ruby> -will recognize incoming paths beginning with +/photo+ but route to the +Images+ controller: +will recognize incoming paths beginning with +/photos+ but route to the +Images+ controller: |_. Verb |_.Path |_.action | |GET |/photos |index | @@ -723,7 +723,7 @@ scope(:path_names => { :new => "neu", :edit => "bearbeiten" }) do end </ruby> -Rails now creates routes to the +CategoriesControlleR+. +Rails now creates routes to the +CategoriesController+. |_.HTTP verb|_.Path |_.action | |GET |/kategorien |index | @@ -788,7 +788,7 @@ TIP: You'll find that the output from +rake routes+ is much more readable if you h4. Testing Routes -Routes should be included in your testing strategy (just like the rest of your application). Rails offers three "built-in assertions":http://api.rubyonrails.org/classes/ActionController/Assertions/RoutingAssertions.html designed to make testing routes simpler: +Routes should be included in your testing strategy (just like the rest of your application). Rails offers three "built-in assertions":http://api.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html designed to make testing routes simpler: * +assert_generates+ * +assert_recognizes+ diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 0e85e6d1d5..aafbbc29ee 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -39,6 +39,7 @@ module Rails autoload :Configuration, 'rails/application/configuration' autoload :Finisher, 'rails/application/finisher' autoload :Railties, 'rails/application/railties' + autoload :RoutesReloader, 'rails/application/routes_reloader' class << self def inherited(base) @@ -81,17 +82,7 @@ module Rails end def routes_reloader - @routes_reloader ||= ActiveSupport::FileUpdateChecker.new([]){ reload_routes! } - end - - def reload_routes! - _routes = self.routes - _routes.disable_clear_and_finalize = true - _routes.clear! - routes_reloader.paths.each { |path| load(path) } - ActiveSupport.on_load(:action_controller) { _routes.finalize! } - ensure - _routes.disable_clear_and_finalize = false + @routes_reloader ||= RoutesReloader.new end def initialize! diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index a0ecbc0fc8..f902c3ded2 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -24,6 +24,7 @@ module Rails @time_zone = "UTC" @middleware = app_middleware @asset_path = '/' + @generators = app_generators end def asset_path=(value) diff --git a/railties/lib/rails/application/routes_reloader.rb b/railties/lib/rails/application/routes_reloader.rb new file mode 100644 index 0000000000..23b72a0ec6 --- /dev/null +++ b/railties/lib/rails/application/routes_reloader.rb @@ -0,0 +1,55 @@ +module Rails + class Application + class RoutesReloader < ::ActiveSupport::FileUpdateChecker + def initialize + super([]) { reload! } + end + + def blocks + @blocks ||= {} + end + private + def reload! + clear! + load_blocks + load_paths + finalize! + ensure + revert + end + + def clear! + routers.each do |routes| + routes.disable_clear_and_finalize = true + routes.clear! + end + end + + def load_blocks + blocks.each do |routes, block| + routes.draw(&block) if block + end + end + + def load_paths + paths.each { |path| load(path) } + end + + def finalize! + routers.each do |routes| + ActiveSupport.on_load(:action_controller) { routes.finalize! } + end + end + + def revert + routers.each do |routes| + routes.disable_clear_and_finalize = false + end + end + + def routers + blocks.keys + end + end + end +end diff --git a/railties/lib/rails/commands.rb b/railties/lib/rails/commands.rb index 60a93c9848..e3aa6d7c3e 100644 --- a/railties/lib/rails/commands.rb +++ b/railties/lib/rails/commands.rb @@ -11,7 +11,17 @@ command = ARGV.shift command = aliases[command] || command case command -when 'generate', 'destroy', 'plugin', 'benchmarker', 'profiler' +when 'generate', 'destroy', 'plugin' + require APP_PATH + Rails.application.require_environment! + + if defined?(ENGINE_PATH) + engine = Rails.application.railties.engines.find { |r| r.root.to_s == ENGINE_PATH } + Rails.application = engine + end + require "rails/commands/#{command}" + +when 'benchmarker', 'profiler' require APP_PATH Rails.application.require_environment! require "rails/commands/#{command}" @@ -23,8 +33,15 @@ when 'console' Rails::Console.start(Rails.application) when 'server' + # Change to the application's path if there is no config.ru file in current dir. + # This allows us to run script/rails server from other directories, but still get + # the main config.ru and properly set the tmp directory. + Dir.chdir(File.expand_path('../../', APP_PATH)) unless File.exists?(File.expand_path("config.ru")) + require 'rails/commands/server' Rails::Server.new.tap { |server| + # We need to require application after the server sets environment, + # otherwise the --environment option given to the server won't propagate. require APP_PATH Dir.chdir(Rails.application.root) server.start diff --git a/railties/lib/rails/commands/plugin.rb b/railties/lib/rails/commands/plugin.rb index 7bb4a1c054..8b2cd1bdba 100644 --- a/railties/lib/rails/commands/plugin.rb +++ b/railties/lib/rails/commands/plugin.rb @@ -58,7 +58,7 @@ class RailsEnvironment else plugin = name_uri_or_plugin end - unless plugin.nil? + if plugin plugin.install else puts "Plugin not found: #{name_uri_or_plugin}" diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 0620b8608e..9ae235b818 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -333,21 +333,22 @@ module Rails def namespace(mod) engine_name(generate_railtie_name(mod)) - _railtie = self name = engine_name - mod.singleton_class.instance_eval do - define_method(:_railtie) do - _railtie - end - - define_method(:table_name_prefix) do - "#{name}_" - end - end - self.routes.default_scope = {:module => name} - self.namespaced = true + + unless mod.respond_to?(:_railtie) + _railtie = self + mod.singleton_class.instance_eval do + define_method(:_railtie) do + _railtie + end + + define_method(:table_name_prefix) do + "#{name}_" + end + end + end end def namespaced? @@ -398,8 +399,10 @@ module Rails } end - def routes + def routes(&block) @routes ||= ActionDispatch::Routing::RouteSet.new + self.routes_draw_block = block if block_given? + @routes end def initializers @@ -446,6 +449,7 @@ module Rails end initializer :add_routing_paths do |app| + app.routes_reloader.blocks[routes] = routes_draw_block paths.config.routes.to_a.each do |route| app.routes_reloader.paths.unshift(route) if File.exists?(route) end @@ -498,6 +502,8 @@ module Rails end protected + attr_accessor :routes_draw_block + def find_root_with_flag(flag, default=nil) root_path = self.class.called_from diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index d553c09484..1dbf27d978 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -11,6 +11,7 @@ gem 'rails', '<%= Rails::VERSION::STRING %>' # Bundle edge Rails instead: # gem 'rails', :git => 'git://github.com/rails/rails.git' +# gem 'arel', :git => 'git://github.com/rails/arel.git' <%- end -%> <% unless options[:skip_active_record] -%> diff --git a/railties/lib/rails/railtie/configuration.rb b/railties/lib/rails/railtie/configuration.rb index f09e3940cc..e0e4324a4a 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -17,6 +17,19 @@ module Rails @@app_middleware ||= Rails::Configuration::MiddlewareStackProxy.new end + # This allows you to modify application's generators from Railties. + # + # Values set on app_generators will become defaults for applicaiton, unless + # application overwrites them. + def app_generators + @@app_generators ||= Rails::Configuration::Generators.new + if block_given? + yield @@app_generators + else + @@app_generators + end + end + # Holds generators configuration: # # config.generators do |g| @@ -30,11 +43,11 @@ module Rails # config.generators.colorize_logging = false # def generators - @@generators ||= Rails::Configuration::Generators.new + @generators ||= Rails::Configuration::Generators.new if block_given? - yield @@generators + yield @generators else - @@generators + @generators end end diff --git a/railties/lib/rails/test_unit/railtie.rb b/railties/lib/rails/test_unit/railtie.rb index e3fafc4b9d..2b6170ebfb 100644 --- a/railties/lib/rails/test_unit/railtie.rb +++ b/railties/lib/rails/test_unit/railtie.rb @@ -1,6 +1,6 @@ module Rails class TestUnitRailtie < Rails::Railtie - config.generators do |c| + config.app_generators do |c| c.test_framework :test_unit, :fixture => true, :fixture_replacement => nil diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index a9dd7d4c1b..c75639d740 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -643,5 +643,104 @@ module RailtiesTest Bukkits::Engine.load_seed assert Bukkits::Engine.config.bukkits_seeds_loaded end + + test "using namespace more than once on one module should not overwrite _railtie method" do + @plugin.write "lib/bukkits.rb", <<-RUBY + module AppTemplate + class Engine < ::Rails::Engine + namespace(AppTemplate) + end + end + RUBY + + add_to_config "namespace AppTemplate" + + app_file "config/routes.rb", <<-RUBY + AppTemplate::Application.routes.draw do end + RUBY + + boot_rails + + assert_equal AppTemplate._railtie, AppTemplate::Engine + end + + test "properly reload routes" do + # when routes are inside application class definition + # they should not be reloaded when engine's routes + # file has changed + add_to_config <<-RUBY + routes do + mount lambda{|env| [200, {}, ["foo"]]} => "/foo" + mount Bukkits::Engine => "/bukkits" + end + RUBY + + FileUtils.rm(File.join(app_path, "config/routes.rb")) + + @plugin.write "config/routes.rb", <<-RUBY + Bukkits::Engine.routes.draw do + mount lambda{|env| [200, {}, ["bar"]]} => "/bar" + end + RUBY + + @plugin.write "lib/bukkits.rb", <<-RUBY + module Bukkits + class Engine < ::Rails::Engine + namespace(Bukkits) + end + end + RUBY + + require 'rack/test' + extend Rack::Test::Methods + + boot_rails + + require "#{rails_root}/config/environment" + get "/foo" + assert_equal "foo", last_response.body + + get "/bukkits/bar" + assert_equal "bar", last_response.body + end + + test "setting generators for engine and overriding app generator's" do + @plugin.write "lib/bukkits.rb", <<-RUBY + module Bukkits + class Engine < ::Rails::Engine + config.generators do |g| + g.orm :datamapper + g.template_engine :haml + g.test_framework :rspec + end + + config.app_generators do |g| + g.orm :mongoid + g.template_engine :liquid + g.test_framework :shoulda + end + end + end + RUBY + + add_to_config <<-RUBY + config.generators do |g| + g.test_framework :test_unit + end + RUBY + + boot_rails + require "#{rails_root}/config/environment" + + app_generators = Rails.application.config.generators.options[:rails] + assert_equal :mongoid , app_generators[:orm] + assert_equal :liquid , app_generators[:template_engine] + assert_equal :test_unit, app_generators[:test_framework] + + generators = Bukkits::Engine.config.generators.options[:rails] + assert_equal :datamapper, generators[:orm] + assert_equal :haml , generators[:template_engine] + assert_equal :rspec , generators[:test_framework] + end end end |