Merge pull request #18627 from tekin/dont-blat-default-format

Preserve default format when generating URLs

actionpack/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Preserve default url options when generating URLs
+
+    Fixes an issue that would cause default_url_options to be lost when
+    generating URLs with fewer positional arguments than parameters in the
+    route definition.
+
+    *Tekin Suleyman*
+
 *   Deprecate *_via_redirect integration test methods.
 
     Use `follow_redirect!` manually after the request call for the same behavior.

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -264,9 +264,10 @@ module ActionDispatch
                 path_params -= controller_options.keys
                 path_params -= result.keys
               end
-              path_params.each { |param|
-                result[param] = inner_options.fetch(param) { args.shift }
-              }
+              path_params -= inner_options.keys
+              path_params.take(args.size).each do |param|
+                result[param] = args.shift
+              end
             end
 
             result.merge!(inner_options)

actionpack/test/controller/base_test.rb

@@ -27,6 +27,16 @@ class DefaultUrlOptionsController < ActionController::Base
   end
 end
 
+class OptionalDefaultUrlOptionsController < ActionController::Base
+  def show
+    render nothing: true
+  end
+
+  def default_url_options
+    { format: 'atom', id: 'default-id' }
+  end
+end
+
 class UrlOptionsController < ActionController::Base
   def from_view
     render :inline => "<%= #{params[:route]} %>"
@@ -234,7 +244,18 @@ class DefaultUrlOptionsTest < ActionController::TestCase
       assert_equal '/en/descriptions/1.xml', @controller.send(:description_path, 1, :format => "xml")
     end
   end
+end
 
+class OptionalDefaultUrlOptionsControllerTest < ActionController::TestCase
+  def test_default_url_options_override_missing_positional_arguments
+    with_routing do |set|
+      set.draw do
+        get "/things/:id(.:format)" => 'things#show', :as => :thing
+      end
+      assert_equal '/things/1.atom', thing_path('1')
+      assert_equal '/things/default-id.atom', thing_path
+    end
+  end
 end
 
 class EmptyUrlOptionsTest < ActionController::TestCase