From 88f11896af2c87776ba09933be22ce70c7e62e61 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 10 Oct 2026 09:39:07 +0200 Subject: [PATCH 1/2] Fix request_path being replaced by a named route helper A route like `resources :requests` generates a public `request_path` helper, and Rails includes the url_helpers module into controllers after resources_controller's modules (url_helpers sits at ancestor index 7, RequestPathIntrospection at 12, and the lower index wins). resources_controller's own request_path was therefore replaced by the route helper, so it read the helper's value instead of the path of the current request, and could not work out the enclosing resources: with a top level `resources :requests` route in the spec app, 6 examples in spec/lib/request_path_introspection_spec.rb fail. The internal reader is renamed to rc_request_path, so that no plausible route name can collide with it, and the specs are updated to match. Based on PR #3 by Robert Thau, which diagnosed this and proposed the same rename (as path_of_request) but is written against the pre-rename lib/ardes/ layout and RSpec 2 syntax, so it no longer applies. Adds a regression spec that includes a module defining a public request_path into the controller under test - the position url_helpers occupies - and asserts the path of the request is still read. Co-authored-by: DeepSeek --- CHANGELOG | 6 +++ lib/resources_controller.rb | 2 +- .../request_path_introspection.rb | 13 +++-- spec/controllers/tags_controller_spec.rb | 4 +- .../tags_controller_via_account_info_spec.rb | 6 +-- .../tags_controller_via_forum_post_spec.rb | 6 +-- spec/lib/load_enclosing_resources_spec.rb | 2 +- spec/lib/request_path_introspection_spec.rb | 48 +++++++++++++------ spec/lib/resources_controller_spec.rb | 2 +- 9 files changed, 60 insertions(+), 29 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5eba28b..aa88b4c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,11 @@ = 5.0.0 * performance optimizations across routing helpers, request introspection, and instance variable access + * 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) * the instance variable read/written by #resource and #resources is now the one named by the resource specification. This means overriding #resource_name / #resources_name no longer changes which instance variable those accessors use diff --git a/lib/resources_controller.rb b/lib/resources_controller.rb index fb76d1c..c5516e6 100644 --- a/lib/resources_controller.rb +++ b/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(', ')} diff --git a/lib/resources_controller/request_path_introspection.rb b/lib/resources_controller/request_path_introspection.rb index 58c31fc..359bb7c 100644 --- a/lib/resources_controller/request_path_introspection.rb +++ b/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 diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb index 99ee097..123e223 100644 --- a/spec/controllers/tags_controller_spec.rb +++ b/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 diff --git a/spec/controllers/tags_controller_via_account_info_spec.rb b/spec/controllers/tags_controller_via_account_info_spec.rb index 8a043a3..b5b316d 100644 --- a/spec/controllers/tags_controller_via_account_info_spec.rb +++ b/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 diff --git a/spec/controllers/tags_controller_via_forum_post_spec.rb b/spec/controllers/tags_controller_via_forum_post_spec.rb index 22052d1..4b81f6e 100644 --- a/spec/controllers/tags_controller_via_forum_post_spec.rb +++ b/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 diff --git a/spec/lib/load_enclosing_resources_spec.rb b/spec/lib/load_enclosing_resources_spec.rb index c031688..2f2b2a3 100644 --- a/spec/lib/load_enclosing_resources_spec.rb +++ b/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| diff --git a/spec/lib/request_path_introspection_spec.rb b/spec/lib/request_path_introspection_spec.rb index e72b4bb..5222e70 100644 --- a/spec/lib/request_path_introspection_spec.rb +++ b/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 diff --git a/spec/lib/resources_controller_spec.rb b/spec/lib/resources_controller_spec.rb index 9c32e11..891be3c 100644 --- a/spec/lib/resources_controller_spec.rb +++ b/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 From d507dbe6b52bb6b32716f17ce8d30735043e74c7 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 10 Oct 2026 09:52:32 +0200 Subject: [PATCH 2/2] Release the request_path fix as 5.0.1 v5.0.0 is tagged at the PR #18 merge, so the two changes that landed on master after the tag get their own section: this request_path fix, and the README.rdoc to README.md conversion (which was pushed in 1db30bb, after the tag). Bumps VERSION to 5.0.1 so that version.rb, the gemspec and the CHANGELOG agree. Co-authored-by: DeepSeek --- CHANGELOG | 8 +++++--- lib/resources_controller/version.rb | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index aa88b4c..605b707 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,11 +1,14 @@ -= 5.0.0 - * performance optimizations across routing helpers, request introspection, and instance variable access += 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 specification. This means overriding #resource_name / #resources_name no longer changes which instance variable those accessors use @@ -27,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 diff --git a/lib/resources_controller/version.rb b/lib/resources_controller/version.rb index f88fefd..c1e7a70 100644 --- a/lib/resources_controller/version.rb +++ b/lib/resources_controller/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module ResourcesController - VERSION = "5.0.0" + VERSION = "5.0.1" end