diff options
60 files changed, 382 insertions, 234 deletions
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/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index a3591eafbe..e00f4a7b1d 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -161,7 +161,11 @@ 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 + + def uri_parser + @uri_parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI end end end diff --git a/actionpack/lib/action_controller/caching/pages.rb b/actionpack/lib/action_controller/caching/pages.rb index 4f7a5d3f55..b845c6f6f9 100644 --- a/actionpack/lib/action_controller/caching/pages.rb +++ b/actionpack/lib/action_controller/caching/pages.rb @@ -99,7 +99,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 @@ -111,6 +111,10 @@ module ActionController #:nodoc: def instrument_page_cache(name, path) ActiveSupport::Notifications.instrument("#{name}.action_controller", :path => path){ yield } end + + def uri_parser + @uri_parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI + end end # Expires the page that was cached with the +options+ as a key. Example: diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 96cb5977d5..7fcc4e7211 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -85,6 +85,8 @@ module ActionController def initialize(*) @_headers = {"Content-Type" => "text/html"} @_status = 200 + @_request = nil + @_response = nil super end @@ -99,7 +101,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..f10fc66b52 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 |p| + extract = /^#{Regexp.quote(p.to_s)}\/?(.*)_helper.rb$/ + helpers += Dir["#{p}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') } end helpers.sort! helpers.uniq! 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..676828957a 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -127,7 +127,11 @@ 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 + + def uri_parser + @uri_parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI end end @@ -417,7 +421,7 @@ module ActionController @request.env.delete('PATH_INFO') - if @controller + if defined?(@controller) && @controller @controller.request = @request @controller.params = {} 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..c82324dee6 100644 --- a/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb +++ b/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb @@ -18,12 +18,12 @@ 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,v2| + 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(v2) else raise "illegal key #{k.inspect} => #{v2.inspect}" end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index ff5e96fdf7..a5d082c6a2 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -43,6 +43,7 @@ module ActionDispatch # :nodoc: @writer = lambda { |x| @body << x } @block = nil @length = 0 + @_etag = nil @status, @header = status, header self.body = body 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/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index fe85acb94e..5e95d8ed39 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -350,7 +350,7 @@ module ActionDispatch options = args.last.is_a?(Hash) ? args.pop : {} path = args.shift || block - path_proc = path.is_a?(Proc) ? path : proc { |params| path % params } + path_proc = path.is_a?(Proc) ? path : proc { |params| params.empty? ? path : (path % params) } status = options[:status] || 301 lambda do |env| @@ -395,10 +395,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 +406,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 +676,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 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..a203ee5934 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -66,7 +66,11 @@ 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 + + def uri_parser + @uri_parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI end end @@ -157,6 +161,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 +195,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 @@ -334,6 +340,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 +441,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 +449,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 @@ -543,7 +548,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 @@ -560,6 +565,10 @@ module ActionDispatch end private + def uri_parser + @uri_parser ||= URI.const_defined?(:Parser) ? URI::Parser.new : URI + end + def handle_positional_args(options) return unless args = options.delete(:_positional_args) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index e836cf7c8e..045c9ec77d 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -128,6 +128,7 @@ module ActionDispatch when String options when nil, Hash + @_routes ||= nil _routes.url_for((options || {}).reverse_merge!(url_options).symbolize_keys) else polymorphic_url(options) 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..86fba87586 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/selector.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/selector.rb @@ -513,8 +513,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 |ele| + text = ele.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 50bdad1b1c..3204115e9f 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -182,6 +182,7 @@ module ActionDispatch reset! end + remove_method :default_url_options def default_url_options { :host => host, :protocol => https? ? "https" : "http" } end @@ -319,10 +320,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 +348,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 +358,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..8bae6d2796 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -1237,7 +1237,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/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/test_case.rb b/actionpack/lib/action_view/test_case.rb index ff35fb7df4..32fd8c5e21 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -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 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/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/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 443191d4fa..a0092a9f73 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -314,7 +314,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 diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index a83f5155f8..8030499235 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -728,7 +728,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 +736,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/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 7f7246d15c..b5bc0e9e9a 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -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_other_test.rb b/actionpack/test/controller/render_other_test.rb index dfc4f2db8c..3117be6e81 100644 --- a/actionpack/test/controller/render_other_test.rb +++ b/actionpack/test/controller/render_other_test.rb @@ -224,15 +224,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/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/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/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/request_test.rb b/actionpack/test/dispatch/request_test.rb index a8b8f9377b..a53d4f1edd 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 diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index c20fa10f63..1efb664304 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 { |_resp| + _resp.cache_control[:public] = true + _resp.etag = '123' + _resp.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 { |_resp| + _resp.charset = 'utf-16' + _resp.content_type = Mime::XML + _resp.body = 'Hello' } resp.to_a diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index bdef04efc6..53b13501b2 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1216,14 +1216,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 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/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/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/tag_helper_test.rb b/actionpack/test/template/tag_helper_test.rb index 85ac515660..cca161bc80 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 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..43e920f9b5 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>") { |u| truncate(u, :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/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/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6a33edeb97..c97b1a24d2 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -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 @@ -283,15 +282,15 @@ module ActiveRecord end def reverse_sql_order(order_query) - order_query.split(',').each { |s| + order_query.join(', ').split(',').collect { |s| if s.match(/\s(asc|ASC)$/) - s.gsub!(/\s(asc|ASC)$/, ' DESC') + s.gsub(/\s(asc|ASC)$/, ' DESC') elsif s.match(/\s(desc|DESC)$/) - s.gsub!(/\s(desc|DESC)$/, ' ASC') + s.gsub(/\s(desc|DESC)$/, ' ASC') else - s.concat(' DESC') + s + ' DESC' end - }.join(',') + } end def array_of_strings?(o) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d87f259f4b..b434b0faa8 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -366,6 +366,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/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/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/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..e5438745e0 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 |