From f68a385d8f5f1e838d1a9b3fd049a45582d01cce Mon Sep 17 00:00:00 2001 From: Darrell Nash Date: Thu, 23 Aug 2018 04:07:32 -0700 Subject: [PATCH 1/2] Added spec to demonstrate bug with nested requirements. --- .../api/routes_with_requirements_spec.rb | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 spec/grape/api/routes_with_requirements_spec.rb diff --git a/spec/grape/api/routes_with_requirements_spec.rb b/spec/grape/api/routes_with_requirements_spec.rb new file mode 100644 index 0000000000..e66963c38d --- /dev/null +++ b/spec/grape/api/routes_with_requirements_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Grape::Endpoint do + subject { Class.new(Grape::API) } + + def app + subject + end + + context 'get' do + it 'routes to a namespace param with dots' do + subject.namespace ':ns_with_dots', requirements: { ns_with_dots: %r{[^\/]+} } do + get '/' do + params[:ns_with_dots] + end + end + + get '/test.id.with.dots' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq 'test.id.with.dots' + end + + it 'routes to a path with multiple params with dots' do + subject.get ':id_with_dots/:another_id_with_dots', requirements: { id_with_dots: %r{[^\/]+}, + another_id_with_dots: %r{[^\/]+} } do + "#{params[:id_with_dots]}/#{params[:another_id_with_dots]}" + end + + get '/test.id/test2.id' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq 'test.id/test2.id' + end + + it 'routes to namespace and path params with dots, with overridden requirements' do + subject.namespace ':ns_with_dots', requirements: { ns_with_dots: %r{[^\/]+} } do + get ':another_id_with_dots', requirements: { ns_with_dots: %r{[^\/]+}, + another_id_with_dots: %r{[^\/]+} } do + "#{params[:ns_with_dots]}/#{params[:another_id_with_dots]}" + end + end + + get '/test.id/test2.id' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq 'test.id/test2.id' + end + + it 'routes to namespace and path params with dots, with merged requirements' do + subject.namespace ':ns_with_dots', requirements: { ns_with_dots: %r{[^\/]+} } do + get ':another_id_with_dots', requirements: { another_id_with_dots: %r{[^\/]+} } do + "#{params[:ns_with_dots]}/#{params[:another_id_with_dots]}" + end + end + + get '/test.id/test2.id' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq 'test.id/test2.id' + end + end +end From c701d4887defe6ac5ad1736428a850e0e029bfa9 Mon Sep 17 00:00:00 2001 From: darren987469 Date: Thu, 6 Sep 2018 21:16:01 +0800 Subject: [PATCH 2/2] Fix route requirements bug This was a bug introduced by commit 9f4ba67. The commit replaces `options[:route_options].clone.merge(...)` with `options[:route_options].clone.reverse_merge(...)`. That causes disappear of the requirements in namespace. --- CHANGELOG.md | 1 + lib/grape/endpoint.rb | 2 +- spec/grape/api/routes_with_requirements_spec.rb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4910c574f4..f776bf8786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Your contribution here. * [#1776](https://github.com/ruby-grape/grape/pull/1776): Validate response returned by the exception handler - [@darren987469](https://github.com/darren987469). * [#1787](https://github.com/ruby-grape/grape/pull/1787): Add documented but not implemented ability to `.insert` a middleware in the stack - [@michaellennox](https://github.com/michaellennox). +* [#1788](https://github.com/ruby-grape/grape/pull/1788): Fix route requirements bug - [@darren987469](https://github.com/darren987469), [@darrellnash](https://github.com/darrellnash). ### 1.1.0 (8/4/2018) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index aacf0472aa..5100f96541 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -200,7 +200,7 @@ def prepare_version end def merge_route_options(**default) - options[:route_options].clone.reverse_merge(**default) + options[:route_options].clone.merge(**default) end def map_routes diff --git a/spec/grape/api/routes_with_requirements_spec.rb b/spec/grape/api/routes_with_requirements_spec.rb index e66963c38d..c6ce2ccee0 100644 --- a/spec/grape/api/routes_with_requirements_spec.rb +++ b/spec/grape/api/routes_with_requirements_spec.rb @@ -22,7 +22,7 @@ def app it 'routes to a path with multiple params with dots' do subject.get ':id_with_dots/:another_id_with_dots', requirements: { id_with_dots: %r{[^\/]+}, - another_id_with_dots: %r{[^\/]+} } do + another_id_with_dots: %r{[^\/]+} } do "#{params[:id_with_dots]}/#{params[:another_id_with_dots]}" end