Merge branch 'master' of gitproxy:rails/rails

actionpack/lib/action_controller.rb

@@ -46,7 +46,6 @@ module ActionController
 
   eager_autoload do
     autoload :RecordIdentifier
-    autoload :UrlRewriter
 
     # TODO: Don't autoload exceptions, setup explicit
     # requires for files that need them

actionpack/lib/action_controller/caching/pages.rb

@@ -121,10 +121,10 @@ module ActionController #:nodoc:
         if options.is_a?(Hash)
           if options[:action].is_a?(Array)
             options[:action].dup.each do |action|
-              self.class.expire_page(url_for(options.merge(:only_path => true, :skip_relative_url_root => true, :action => action)))
+              self.class.expire_page(url_for(options.merge(:only_path => true, :action => action)))
             end
           else
-            self.class.expire_page(url_for(options.merge(:only_path => true, :skip_relative_url_root => true)))
+            self.class.expire_page(url_for(options.merge(:only_path => true)))
           end
         else
           self.class.expire_page(options)
@@ -139,7 +139,7 @@ module ActionController #:nodoc:
 
         path = case options
           when Hash
-            url_for(options.merge(:only_path => true, :skip_relative_url_root => true, :format => params[:format]))
+            url_for(options.merge(:only_path => true, :format => params[:format]))
           when String
             options
           else

actionpack/lib/action_controller/test_case.rb

@@ -345,8 +345,7 @@ module ActionController
             :relative_url_root => nil,
             :_path_segments => @request.symbolized_path_parameters)
 
-        rewriter = ActionController::UrlRewriter
+          url, query_string = @router.url_for(options).split("?", 2)
-        url, query_string = rewriter.rewrite(@router, options).split("?", 2)
 
           @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root
           @request.env["PATH_INFO"] = url

actionpack/lib/action_controller/url_rewriter.rb

@@ -1,64 +0,0 @@
-require 'active_support/core_ext/hash/except'
-
-module ActionController
-  # Rewrites URLs for Base.redirect_to and Base.url_for in the controller.
-  module UrlRewriter #:nodoc:
-    RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :skip_relative_url_root]
-
-    # ROUTES TODO: Class method code smell
-    def self.rewrite(router, options)
-      handle_positional_args(options)
-
-      rewritten_url = ""
-
-      path_segments = options.delete(:_path_segments)
-
-      unless options[:only_path]
-        rewritten_url << (options[:protocol] || "http")
-        rewritten_url << "://" unless rewritten_url.match("://")
-        rewritten_url << rewrite_authentication(options)
-
-        raise "Missing host to link to! Please provide :host parameter or set default_url_options[:host]" unless options[:host]
-
-        rewritten_url << options[:host]
-        rewritten_url << ":#{options.delete(:port)}" if options.key?(:port)
-      end
-
-      path_options = options.except(*RESERVED_OPTIONS)
-      path_options = yield(path_options) if block_given?
-      path = router.generate(path_options, path_segments || {})
-
-      # ROUTES TODO: This can be called directly, so script_name should probably be set in the router
-      rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
-      rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
-
-      rewritten_url
-    end
-
-  protected
-
-    def self.handle_positional_args(options)
-      return unless args = options.delete(:_positional_args)
-
-      keys = options.delete(:_positional_keys)
-      keys -= options.keys if args.size < keys.size - 1 # take format into account
-
-      args = args.zip(keys).inject({}) do |h, (v, k)|
-        h[k] = v
-        h
-      end
-
-      # Tell url_for to skip default_url_options
-      options.merge!(args)
-    end
-
-    def self.rewrite_authentication(options)
-      if options[:user] && options[:password]
-        "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@"
-      else
-        ""
-      end
-    end
-
-  end
-end

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -180,6 +180,11 @@ module ActionDispatch
           @set.add_route(*mapping)
           self
         end
+
+        def default_url_options=(options)
+          @set.default_url_options = options
+        end
+        alias_method :default_url_options, :default_url_options=
       end
 
       module HttpHelpers

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -186,6 +186,7 @@ module ActionDispatch
 
       attr_accessor :routes, :named_routes
       attr_accessor :disable_clear_and_finalize, :resources_path_names
+      attr_accessor :default_url_options
 
       def self.default_resources_path_names
         { :new => 'new', :edit => 'edit' }
@@ -196,6 +197,7 @@ module ActionDispatch
         self.named_routes = NamedRouteCollection.new
         self.resources_path_names = self.class.default_resources_path_names.dup
         self.controller_namespaces = Set.new
+        self.default_url_options = {}
 
         @disable_clear_and_finalize = false
         clear!
@@ -235,31 +237,22 @@ module ActionDispatch
         named_routes.install(destinations, regenerate_code)
       end
 
-      def url_for
-        @url_for ||= begin
-          router = self
-          Module.new do
-            extend ActiveSupport::Concern
-            include UrlFor
-
-            define_method(:_router) { router }
-          end
-        end
-      end
-
       def url_helpers
         @url_helpers ||= begin
           router = self
 
           Module.new do
             extend ActiveSupport::Concern
-            include router.url_for
+            include UrlFor
 
             # ROUTES TODO: install_helpers isn't great... can we make a module with the stuff that
             # we can include?
+            # Yes plz - JP
             included do
               router.install_helpers(self)
             end
+
+            define_method(:_router) { router }
           end
         end
       end
@@ -410,6 +403,39 @@ module ActionDispatch
         Generator.new(options, recall, @set, extras).generate
       end
 
+      RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash]
+
+      def url_for(options)
+        options = default_url_options.merge(options || {})
+
+        handle_positional_args(options)
+
+        rewritten_url = ""
+
+        path_segments = options.delete(:_path_segments)
+
+        unless options[:only_path]
+          rewritten_url << (options[:protocol] || "http")
+          rewritten_url << "://" unless rewritten_url.match("://")
+          rewritten_url << rewrite_authentication(options)
+
+          raise "Missing host to link to! Please provide :host parameter or set default_url_options[:host]" unless options[:host]
+
+          rewritten_url << options[:host]
+          rewritten_url << ":#{options.delete(:port)}" if options.key?(:port)
+        end
+
+        path_options = options.except(*RESERVED_OPTIONS)
+        path_options = yield(path_options) if block_given?
+        path = generate(path_options, path_segments || {})
+
+        # ROUTES TODO: This can be called directly, so script_name should probably be set in the router
+        rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
+        rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
+
+        rewritten_url
+      end
+
       def call(env)
         @set.call(env)
       end
@@ -435,6 +461,30 @@ module ActionDispatch
 
         raise ActionController::RoutingError, "No route matches #{path.inspect}"
       end
+
+      private
+        def handle_positional_args(options)
+          return unless args = options.delete(:_positional_args)
+
+          keys = options.delete(:_positional_keys)
+          keys -= options.keys if args.size < keys.size - 1 # take format into account
+
+          args = args.zip(keys).inject({}) do |h, (v, k)|
+            h[k] = v
+            h
+          end
+
+          # Tell url_for to skip default_url_options
+          options.merge!(args)
+        end
+
+        def rewrite_authentication(options)
+          if options[:user] && options[:password]
+            "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@"
+          else
+            ""
+          end
+        end
     end
   end
 end

actionpack/lib/action_dispatch/routing/url_for.rb

@@ -113,8 +113,6 @@ module ActionDispatch
       #   provided either explicitly, or via +default_url_options+.
       # * <tt>:port</tt> - Optionally specify the port to connect to.
       # * <tt>:anchor</tt> - An anchor name to be appended to the path.
-      # * <tt>:skip_relative_url_root</tt> - If true, the url is not constructed using the
-      #   +relative_url_root+ set in ActionController::Base.relative_url_root.
       # * <tt>:trailing_slash</tt> - If true, adds a trailing slash, as in "/archive/2009/"
       #
       # Any other key (<tt>:controller</tt>, <tt>:action</tt>, etc.) given to
@@ -131,7 +129,7 @@ module ActionDispatch
         when String
           options
         when nil, Hash
-          ActionController::UrlRewriter.rewrite(_router, url_options.merge(options || {}))
+          _router.url_for(url_options.merge(options || {}))
         else
           polymorphic_url(options)
         end

actionpack/test/controller/caching_test.rb

@@ -63,8 +63,7 @@ class PageCachingTest < ActionController::TestCase
     @controller = PageCachingTestController.new
     @controller.cache_store = :file_store, FILE_STORE_PATH
 
-    @params = {:controller => 'posts', :action => 'index', :only_path => true, :skip_relative_url_root => true}
+    @params = {:controller => 'posts', :action => 'index', :only_path => true}
-    @rewriter = ActionController::UrlRewriter
 
     FileUtils.rm_rf(File.dirname(FILE_STORE_PATH))
     FileUtils.mkdir_p(FILE_STORE_PATH)
@@ -82,9 +81,9 @@ class PageCachingTest < ActionController::TestCase
         match '/', :to => 'posts#index', :as => :main
       end
       @params[:format] = 'rss'
-      assert_equal '/posts.rss', @rewriter.rewrite(@router, @params)
+      assert_equal '/posts.rss', @router.url_for(@params)
       @params[:format] = nil
-      assert_equal '/', @rewriter.rewrite(@router, @params)
+      assert_equal '/', @router.url_for(@params)
     end
   end
 

actionpack/test/controller/url_rewriter_test.rb

@@ -1,8 +1,6 @@
 require 'abstract_unit'
 require 'controller/fake_controllers'
 
-ActionController::UrlRewriter
-
 class UrlRewriterTests < ActionController::TestCase
   class Rewriter
     def initialize(request)
@@ -13,8 +11,7 @@ class UrlRewriterTests < ActionController::TestCase
     end
 
     def rewrite(router, options)
-      options = @options.merge(options)
+      router.url_for(@options.merge(options))
-      ActionController::UrlRewriter.rewrite(router, options)
     end
   end
 

actionpack/test/dispatch/routing_test.rb

@@ -15,6 +15,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
   stub_controllers do |routes|
     Routes = routes
     Routes.draw do
+      default_url_options :host => "rubyonrails.org"
+
       controller :sessions do
         get  'login' => :new, :as => :login
         post 'login' => :create
@@ -189,6 +191,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
 
       assert_equal '/login', url_for(:controller => 'sessions', :action => 'create', :only_path => true)
       assert_equal '/login', url_for(:controller => 'sessions', :action => 'new', :only_path => true)
+
+      assert_equal 'http://rubyonrails.org/login', Routes.url_for(:controller => 'sessions', :action => 'create')
     end
   end
 

activerecord/lib/active_record/relation/predicate_builder.rb

@@ -20,7 +20,9 @@ module ActiveRecord
             table = Arel::Table.new(table_name, :engine => @engine)
           end
 
-          attribute = table[column] || Arel::Attribute.new(table, column.to_sym)
+          # TODO : Arel::Table#[] should fallback to using Arel::Attribute if the table/column doesn't exist
+          # attribute = table[column]
+          attribute = Arel::Attribute.new(table, column.to_sym)
 
           case value
           when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope