Fix request_path being replaced by a named route helper (supersedes #3)

CHANGELOG

@@ -1,3 +1,12 @@
+= 5.0.1
+  * renamed the internal RequestPathIntrospection#request_path to #rc_request_path: a route like
+    `resources :requests` generates a named route helper called request_path, which is included into
+    controllers after resources_controller's modules, and so replaced resources_controller's own method
+    (breaking enclosing resource detection).  Overriding #request_path to customise the path will no
+    longer work - use the documented :resource_path param, or override #rc_request_path (reported by
+    Robert Thau in #3)
+  * README.rdoc is now Markdown (README.md), with the Travis badge replaced by the GitHub Actions one
+
 = 5.0.0
   * performance optimizations across routing helpers, request introspection, and instance variable access
   * the instance variable read/written by #resource and #resources is now the one named by the resource
@@ -21,7 +30,6 @@
     app and could no longer run
   * gem packaging: added the MIT license and gem metadata, dropped the stale rubygems_version, and
     s.files no longer ships the specs, CI config and other development files
-  * README.rdoc is now Markdown (README.md), with the Travis badge replaced by the GitHub Actions one
   * require Rails >= 8.0 and Ruby >= 3.3, dropping support for older versions
 
 = 4.0.0

lib/resources_controller.rb

@@ -855,7 +855,7 @@ def raise_cant_find_singleton(name, klass) #:nodoc:
     def raise_resource_mismatch(controller) #:nodoc:
       raise ResourceMismatch, <<-end_str
 resources_controller can't match the route to the resource specification
-path:         #{controller.send(:request_path)}
+path:         #{controller.send(:rc_request_path)}
 specification: enclosing: [#{controller.specifications.collect{|s| s.is_a?(Specification) ? ":#{s.segment}" : s}.join(', ')}], resource :#{controller.resource_specification.segment}
 
 the successfully loaded enclosing resources are: #{controller.enclosing_resources.join(', ')}

lib/resources_controller/request_path_introspection.rb

@@ -7,12 +7,17 @@ module ResourcesController
   # these methods are aware of resource specifications specified either by map_enclosing_resource.
   module RequestPathIntrospection
   protected
-    def request_path
-      @request_path ||= params[:resource_path] || request.path
+    # NOTE: this is deliberately not called #request_path.  A route like
+    # `resources :requests` generates a named route helper called request_path, and the
+    # url_helpers module is included into the controller after this one, so it wins the
+    # name lookup - resources_controller would then read the helper's value instead of
+    # the path of the current request.  See spec/lib/request_path_introspection_spec.rb.
+    def rc_request_path
+      @rc_request_path ||= params[:resource_path] || request.path
     end
 
     def nesting_request_path
-      @nesting_request_path ||= remove_namespace(remove_current_segment(request_path))
+      @nesting_request_path ||= remove_namespace(remove_current_segment(rc_request_path))
     end
 
     # returns an array of hashes like {:segment => 'forum', :singleton => false}
@@ -29,7 +34,7 @@ def namespace_segments
         @namespace_segments = if namespace.empty?
           []
         else
-          request_path.start_with?("/#{namespace}") ? namespace.split('/') : []
+          rc_request_path.start_with?("/#{namespace}") ? namespace.split('/') : []
         end
       end
       @namespace_segments

lib/resources_controller/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module ResourcesController
-  VERSION = "5.0.0"
+  VERSION = "5.0.1"
 end

spec/controllers/tags_controller_spec.rb

@@ -8,7 +8,7 @@
       allow(@tag).to receive(:to_param).and_return('2')
       allow(Tag).to receive(:find).and_return(@tag)
 
-      allow(@controller).to receive(:request_path).and_return('/tags/2')
+      allow(@controller).to receive(:rc_request_path).and_return('/tags/2')
       get :show, params: { :id => "2" }
     end
 
@@ -66,7 +66,7 @@
     end
 
     def do_get
-      allow(@controller).to receive(:request_path).and_return('/tags/index')
+      allow(@controller).to receive(:rc_request_path).and_return('/tags/index')
       get :index
     end
 

spec/controllers/tags_controller_via_account_info_spec.rb

@@ -24,7 +24,7 @@ def setup_mocks
       allow(@tag).to receive(:to_param).and_return('2')
       allow(@info_tags).to receive(:find).and_return(@tag)
 
-      allow(@controller).to receive(:request_path).and_return('/account/info/tags/2')
+      allow(@controller).to receive(:rc_request_path).and_return('/account/info/tags/2')
       get :show, params: {  :id => 2 }
     end
 
@@ -67,7 +67,7 @@ def setup_mocks
       @other_tag = Tag.create
 
       @controller.instance_variable_set('@current_user', @account)
-      allow(@controller).to receive(:request_path).and_return('/account/info/tags')
+      allow(@controller).to receive(:rc_request_path).and_return('/account/info/tags')
       get :index
       @resource_service = controller.send :resource_service
     end
@@ -104,7 +104,7 @@ def setup_mocks
     end
 
     def do_get
-      allow(@controller).to receive(:request_path).and_return('/account/info/tags')
+      allow(@controller).to receive(:rc_request_path).and_return('/account/info/tags')
       get :index
     end
 

spec/controllers/tags_controller_via_forum_post_spec.rb

@@ -26,7 +26,7 @@ def setup_mocks
       allow(@tag).to receive(:to_param).and_return('3')
       allow(@post_tags).to receive(:find).and_return(@tag)
 
-      allow(@controller).to receive(:request_path).and_return('/forums/1/posts/1/tags/3')
+      allow(@controller).to receive(:rc_request_path).and_return('/forums/1/posts/1/tags/3')
       get :show, params: { :forum_id => "1", :post_id => "2", :id => "3" }
     end
 
@@ -68,7 +68,7 @@ def setup_mocks
       @other_post  = Post.create :forum_id => @forum.id
       @other_tag   = Tag.create :taggable_id => @other_post.id, :taggable_type => 'Post'
 
-      allow(@controller).to receive(:request_path).and_return("/forums/:id/posts/:id/tags")
+      allow(@controller).to receive(:rc_request_path).and_return("/forums/:id/posts/:id/tags")
       get :index, params: { :forum_id => @forum.id, :post_id => @post.id }
       @resource_service = controller.send :resource_service
     end
@@ -105,7 +105,7 @@ def setup_mocks
     end
 
     def do_get
-      allow(@controller).to receive(:request_path).and_return("/forums/1/posts/2/tags")
+      allow(@controller).to receive(:rc_request_path).and_return("/forums/1/posts/2/tags")
       get :index, params: { :forum_id => '1', :post_id => '2' }
     end
 

spec/lib/load_enclosing_resources_spec.rb

@@ -11,7 +11,7 @@ def setup_tags_controller(options = {})
 
   def setup_common
     @controller = @klass.new
-    allow(@controller).to receive(:request_path).and_return('')
+    allow(@controller).to receive(:rc_request_path).and_return('')
     # stub :load_enclosing_resource_from_specification, increase enclosing_resources by one, and return a mock resource
     allow(@controller).to receive(:load_enclosing_resource_from_specification) do |name, _|
       double("resource: #{name}").tap do |resource|

spec/lib/request_path_introspection_spec.rb

@@ -15,67 +15,87 @@ class User < ActiveRecord::Base; end
       allow(@controller).to receive(:request).and_return(double('request', :path => '/forums'))
     end
 
-    describe "#request_path" do
+    describe "#rc_request_path" do
       it "should default to request.path" do
-        expect(@controller.send(:request_path)).to eq('/forums')
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
       end
 
       it " should be params[:resource_path], when set" do
         @controller.params[:resource_path] = '/foo'
-        expect(@controller.send(:request_path)).to eq('/foo')
+        expect(@controller.send(:rc_request_path)).to eq('/foo')
       end
     end
 
     describe "#nesting_request_path" do
       it "should remove the controller_name segment" do
-        allow(@controller).to receive(:request_path).and_return('/users/1/forums/2')
+        allow(@controller).to receive(:rc_request_path).and_return('/users/1/forums/2')
         expect(@controller.send(:nesting_request_path)).to eq('/users/1')
       end
 
       it "when resource_specification present, would remove that segment" do
         allow(@controller).to receive(:resource_specification).and_return(ResourcesController::Specification.new(:forum, :class => RequestPathIntrospectionSpec::Forum, :segment => 'foromas'))
-        allow(@controller).to receive(:request_path).and_return('/users/1/foromas/2')
+        allow(@controller).to receive(:rc_request_path).and_return('/users/1/foromas/2')
         expect(@controller.send(:nesting_request_path)).to eq('/users/1')
       end
 
       it "should remove only the controller_name segment, when nesting is same name" do
-        allow(@controller).to receive(:request_path).and_return('/forums/1/forums/2')
+        allow(@controller).to receive(:rc_request_path).and_return('/forums/1/forums/2')
         expect(@controller.send(:nesting_request_path)).to eq('/forums/1')
       end
 
       it "should remove the controller_name segment, even when id matches controller name" do
-        allow(@controller).to receive(:request_path).and_return('/forums/1/forums/forums.atom')
+        allow(@controller).to receive(:rc_request_path).and_return('/forums/1/forums/forums.atom')
         expect(@controller.send(:nesting_request_path)).to eq('/forums/1')
       end
 
       it "should remove only the controller_name segment even when nesting is same name" do
         allow(@controller).to receive(:resource_specification).and_return(ResourcesController::Specification.new(:forum, :class => RequestPathIntrospectionSpec::Forum, :singleton => true))
-        allow(@controller).to receive(:request_path).and_return('/users/1/forum/forum.atom')
+        allow(@controller).to receive(:rc_request_path).and_return('/users/1/forum/forum.atom')
         expect(@controller.send(:nesting_request_path)).to eq('/users/1/forum')
       end
 
       it "should match the current segment literally, when it contains regexp characters" do
         allow(@controller).to receive(:resource_specification).and_return(ResourcesController::Specification.new(:forum, :class => RequestPathIntrospectionSpec::Forum, :segment => 'v1.0'))
-        allow(@controller).to receive(:request_path).and_return('/v1X0/2')
+        allow(@controller).to receive(:rc_request_path).and_return('/v1X0/2')
         expect(@controller.send(:nesting_request_path)).to eq('/v1X0/2')
       end
 
       it "should remove any controller namespace" do
         allow(@controller).to receive(:controller_path).and_return('some/name/space/forums')
-        allow(@controller).to receive(:request_path).and_return('/some/name/space/users/1/secret/forums')
+        allow(@controller).to receive(:rc_request_path).and_return('/some/name/space/users/1/secret/forums')
         expect(@controller.send(:nesting_request_path)).to eq('/users/1/secret')
       end
     end
+
+    describe "when a named route helper called request_path is in the controller" do
+      # `resources :requests` defines a public request_path helper, and Rails includes the
+      # url_helpers module into controllers after resources_controller's modules, so the
+      # helper wins the name lookup.  resources_controller must not depend on that name.
+      before do
+        shadow = Module.new do
+          def request_path(*)
+            raise "resources_controller called the route helper instead of reading the request path"
+          end
+        end
+        @klass.send(:include, shadow)
+      end
+
+      it "should still read the path of the request" do
+        allow(@controller.request).to receive(:path).and_return('/users/1/forums')
+        expect(@controller.send(:rc_request_path)).to eq('/users/1/forums')
+        expect(@controller.send(:nesting_request_path)).to eq('/users/1')
+      end
+    end
 
-    it "#namespace_segments should return [] segments if NOT present in request_path" do
+    it "#namespace_segments should return [] segments if NOT present in rc_request_path" do
       allow(@controller).to receive(:controller_path).and_return('some/name/space/forums')
-      allow(@controller).to receive(:request_path).and_return('/SAM/name/space/users/1/secret/forums')
+      allow(@controller).to receive(:rc_request_path).and_return('/SAM/name/space/users/1/secret/forums')
       expect(@controller.send(:namespace_segments)).to eq([])
     end
 
-    it "#namespace_segments should return namespace segments if present in request_path" do
+    it "#namespace_segments should return namespace segments if present in rc_request_path" do
       allow(@controller).to receive(:controller_path).and_return('some/name/space/forums')
-      allow(@controller).to receive(:request_path).and_return('/some/name/space/users/1/secret/forums')
+      allow(@controller).to receive(:rc_request_path).and_return('/some/name/space/users/1/secret/forums')
       expect(@controller.send(:namespace_segments)).to eq(['some', 'name', 'space'])
     end
 

spec/lib/resources_controller_spec.rb

@@ -21,7 +21,7 @@
     @controller = TagsController.new
     info = mock_model(Info, :tags => [])
     allow(@controller).to receive(:current_user).and_return(mock_model(User, :info => info))
-    allow(@controller).to receive(:request_path).and_return('/account/info/tags')
+    allow(@controller).to receive(:rc_request_path).and_return('/account/info/tags')
     allow(@controller).to receive(:params).and_return({})
     @controller.send :load_enclosing_resources
   end