From c859eeee2f7b207c973e0156c938e62931a8a12e Mon Sep 17 00:00:00 2001 From: Hermann Mayer Date: Fri, 19 Feb 2021 20:19:36 +0100 Subject: [PATCH] Corrected a hash modification while iterating issue. Signed-off-by: Hermann Mayer --- CHANGELOG.md | 1 + lib/grape/api.rb | 2 +- spec/grape/api_spec.rb | 77 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b610254e7d..760e2a1233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Your contribution here. * [#2161](https://github.com/ruby-grape/grape/pull/2157): Handle EOFError from Rack when given an empty multipart body - [@bschmeck](https://github.com/bschmeck). +* [#2162](https://github.com/ruby-grape/grape/pull/2162): Corrected a hash modification while iterating issue - [@Jack12816](https://github.com/Jack12816). ### 1.5.2 (2021/02/06) diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 219538292d..11bcb6a447 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -141,7 +141,7 @@ def instance_for_rack # Adds a new stage to the set up require to get a Grape::API up and running def add_setup(method, *args, &block) setup_step = { method: method, args: args, block: block } - @setup << setup_step + @setup += [setup_step] last_response = nil @instances.each do |instance| last_response = replay_step_on(instance, setup_step) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 0d722a4b12..0cb7e1251c 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4056,4 +4056,81 @@ def before expect { get '/const/missing' }.to raise_error(NameError).with_message(/SomeRandomConstant/) end end + + describe 'custom route helpers on nested APIs' do + let(:shared_api_module) do + Module.new do + # rubocop:disable Style/ExplicitBlockArgument because this causes + # the underlying issue in this form + def uniqe_id_route + params do + use :unique_id + end + route_param(:id) do + yield + end + end + # rubocop:enable Style/ExplicitBlockArgument + end + end + let(:shared_api_definitions) do + Module.new do + extend ActiveSupport::Concern + + included do + helpers do + params :unique_id do + requires :id, type: String, + allow_blank: false, + regexp: /\d+-\d+/ + end + end + end + end + end + let(:orders_root) do + shared = shared_api_definitions + find = orders_find_endpoint + Class.new(Grape::API) do + include shared + + namespace(:orders) do + mount find + end + end + end + let(:orders_find_endpoint) do + shared = shared_api_definitions + Class.new(Grape::API) do + include shared + + uniqe_id_route do + desc 'Fetch a single order' do + detail 'While specifying the order id on the route' + end + get { params[:id] } + end + end + end + subject(:grape_api) do + Class.new(Grape::API) do + version 'v1', using: :path + end + end + + before do + Grape::API::Instance.extend(shared_api_module) + subject.mount orders_root + end + + it 'returns an error when the id is bad' do + get '/v1/orders/abc' + expect(last_response.body).to be_eql('id is invalid') + end + + it 'returns the given id when it is valid' do + get '/v1/orders/1-2' + expect(last_response.body).to be_eql('1-2') + end + end end