From 4913534bcdb1f4f5bd521f84c30ac5ce6772ca46 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 18 Sep 2020 13:08:49 +1200 Subject: [PATCH 1/2] Move Grape::Endpoint#declared specs into own spec file --- spec/grape/endpoint/declared_spec.rb | 545 +++++++++++++++++++++++++++ spec/grape/endpoint_spec.rb | 534 -------------------------- 2 files changed, 545 insertions(+), 534 deletions(-) create mode 100644 spec/grape/endpoint/declared_spec.rb diff --git a/spec/grape/endpoint/declared_spec.rb b/spec/grape/endpoint/declared_spec.rb new file mode 100644 index 0000000000..73d145a8eb --- /dev/null +++ b/spec/grape/endpoint/declared_spec.rb @@ -0,0 +1,545 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Grape::Endpoint do + subject { Class.new(Grape::API) } + + def app + subject + end + + describe '#declared' do + before do + subject.format :json + subject.params do + requires :first + optional :second + optional :third, default: 'third-default' + optional :nested, type: Hash do + optional :fourth + optional :fifth + optional :nested_two, type: Hash do + optional :sixth + optional :nested_three, type: Hash do + optional :seventh + end + end + optional :nested_arr, type: Array do + optional :eighth + end + end + optional :arr, type: Array do + optional :nineth + end + end + end + + context 'when params are not built with default class' do + it 'returns an object that corresponds with the params class - hash with indifferent access' do + subject.params do + build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder + end + subject.get '/declared' do + d = declared(params, include_missing: true) + { declared_class: d.class.to_s } + end + + get '/declared?first=present' + expect(JSON.parse(last_response.body)['declared_class']).to eq('ActiveSupport::HashWithIndifferentAccess') + end + + it 'returns an object that corresponds with the params class - hashie mash' do + subject.params do + build_with Grape::Extensions::Hashie::Mash::ParamBuilder + end + subject.get '/declared' do + d = declared(params, include_missing: true) + { declared_class: d.class.to_s } + end + + get '/declared?first=present' + expect(JSON.parse(last_response.body)['declared_class']).to eq('Hashie::Mash') + end + + it 'returns an object that corresponds with the params class - hash' do + subject.params do + build_with Grape::Extensions::Hash::ParamBuilder + end + subject.get '/declared' do + d = declared(params, include_missing: true) + { declared_class: d.class.to_s } + end + + get '/declared?first=present' + expect(JSON.parse(last_response.body)['declared_class']).to eq('Hash') + end + end + + it 'should show nil for nested params if include_missing is true' do + subject.get '/declared' do + declared(params, include_missing: true) + end + + get '/declared?first=present' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['nested']['fourth']).to be_nil + end + + it 'does not work in a before filter' do + subject.before do + declared(params) + end + subject.get('/declared') { declared(params) } + + expect { get('/declared') }.to raise_error( + Grape::DSL::InsideRoute::MethodNotYetAvailable + ) + end + + it 'has as many keys as there are declared params' do + subject.get '/declared' do + declared(params) + end + get '/declared?first=present' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body).keys.size).to eq(5) + end + + it 'has a optional param with default value all the time' do + subject.get '/declared' do + declared(params) + end + get '/declared?first=one' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['third']).to eql('third-default') + end + + it 'builds nested params' do + subject.get '/declared' do + declared(params) + end + + get '/declared?first=present&nested[fourth]=1' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 4 + end + + it 'builds nested params when given array' do + subject.get '/dummy' do + end + subject.params do + requires :first + optional :second + optional :third, default: 'third-default' + optional :nested, type: Array do + optional :fourth + end + end + subject.get '/declared' do + declared(params) + end + + get '/declared?first=present&nested[][fourth]=1&nested[][fourth]=2' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['nested'].size).to eq 2 + end + + context 'sets nested objects when the param is missing' do + it 'to be a hash when include_missing is true' do + subject.get '/declared' do + declared(params, include_missing: true) + end + + get '/declared?first=present' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['nested']).to eq({}) + end + + it 'to be an array when include_missing is true' do + subject.get '/declared' do + declared(params, include_missing: true) + end + + get '/declared?first=present' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['arr']).to be_a(Array) + end + + it 'to be an array when nested and include_missing is true' do + subject.get '/declared' do + declared(params, include_missing: true) + end + + get '/declared?first=present&nested[fourth]=1' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['nested']['nested_arr']).to be_a(Array) + end + + it 'to be nil when include_missing is false' do + subject.get '/declared' do + declared(params, include_missing: false) + end + + get '/declared?first=present' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['nested']).to be_nil + end + end + + it 'filters out any additional params that are given' do + subject.get '/declared' do + declared(params) + end + get '/declared?first=one&other=two' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body).key?(:other)).to eq false + end + + it 'stringifies if that option is passed' do + subject.get '/declared' do + declared(params, stringify: true) + end + + get '/declared?first=one&other=two' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)['first']).to eq 'one' + end + + it 'does not include missing attributes if that option is passed' do + subject.get '/declared' do + error! 'expected nil', 400 if declared(params, include_missing: false).key?(:second) + '' + end + + get '/declared?first=one&other=two' + expect(last_response.status).to eq(200) + end + + it 'does not include renamed missing attributes if that option is passed' do + subject.params do + optional :renamed_original, as: :renamed + end + subject.get '/declared' do + error! 'expected nil', 400 if declared(params, include_missing: false).key?(:renamed) + '' + end + + get '/declared?first=one&other=two' + expect(last_response.status).to eq(200) + end + + it 'includes attributes with value that evaluates to false' do + subject.params do + requires :first + optional :boolean + end + + subject.post '/declared' do + error!('expected false', 400) if declared(params, include_missing: false)[:boolean] != false + '' + end + + post '/declared', ::Grape::Json.dump(first: 'one', boolean: false), 'CONTENT_TYPE' => 'application/json' + expect(last_response.status).to eq(201) + end + + it 'includes attributes with value that evaluates to nil' do + subject.params do + requires :first + optional :second + end + + subject.post '/declared' do + error!('expected nil', 400) unless declared(params, include_missing: false)[:second].nil? + '' + end + + post '/declared', ::Grape::Json.dump(first: 'one', second: nil), 'CONTENT_TYPE' => 'application/json' + expect(last_response.status).to eq(201) + end + + it 'includes missing attributes with defaults when there are nested hashes' do + subject.get '/dummy' do + end + + subject.params do + requires :first + optional :second + optional :third, default: nil + optional :nested, type: Hash do + optional :fourth, default: nil + optional :fifth, default: nil + requires :nested_nested, type: Hash do + optional :sixth, default: 'sixth-default' + optional :seven, default: nil + end + end + end + + subject.get '/declared' do + declared(params, include_missing: false) + end + + get '/declared?first=present&nested[fourth]=&nested[nested_nested][sixth]=sixth' + json = JSON.parse(last_response.body) + expect(last_response.status).to eq(200) + expect(json['first']).to eq 'present' + expect(json['nested'].keys).to eq %w[fourth fifth nested_nested] + expect(json['nested']['fourth']).to eq '' + expect(json['nested']['nested_nested'].keys).to eq %w[sixth seven] + expect(json['nested']['nested_nested']['sixth']).to eq 'sixth' + end + + it 'does not include missing attributes when there are nested hashes' do + subject.get '/dummy' do + end + + subject.params do + requires :first + optional :second + optional :third + optional :nested, type: Hash do + optional :fourth + optional :fifth + end + end + + subject.get '/declared' do + declared(params, include_missing: false) + end + + get '/declared?first=present&nested[fourth]=4' + json = JSON.parse(last_response.body) + expect(last_response.status).to eq(200) + expect(json['first']).to eq 'present' + expect(json['nested'].keys).to eq %w[fourth] + expect(json['nested']['fourth']).to eq '4' + end + end + + describe '#declared; call from child namespace' do + before do + subject.format :json + subject.namespace :parent do + params do + requires :parent_name, type: String + end + + namespace ':parent_name' do + params do + requires :child_name, type: String + requires :child_age, type: Integer + end + + namespace ':child_name' do + params do + requires :grandchild_name, type: String + end + + get ':grandchild_name' do + { + 'params' => params, + 'without_parent_namespaces' => declared(params, include_parent_namespaces: false), + 'with_parent_namespaces' => declared(params, include_parent_namespaces: true) + } + end + end + end + end + + get '/parent/foo/bar/baz', child_age: 5, extra: 'hello' + end + + let(:parsed_response) { JSON.parse(last_response.body, symbolize_names: true) } + + it { expect(last_response.status).to eq 200 } + + context 'with include_parent_namespaces: false' do + it 'returns declared parameters only from current namespace' do + expect(parsed_response[:without_parent_namespaces]).to eq( + grandchild_name: 'baz' + ) + end + end + + context 'with include_parent_namespaces: true' do + it 'returns declared parameters from every parent namespace' do + expect(parsed_response[:with_parent_namespaces]).to eq( + parent_name: 'foo', + child_name: 'bar', + grandchild_name: 'baz', + child_age: 5 + ) + end + end + + context 'without declaration' do + it 'returns all requested parameters' do + expect(parsed_response[:params]).to eq( + parent_name: 'foo', + child_name: 'bar', + grandchild_name: 'baz', + child_age: 5, + extra: 'hello' + ) + end + end + end + + describe '#declared; from a nested mounted endpoint' do + before do + doubly_mounted = Class.new(Grape::API) + doubly_mounted.namespace :more do + params do + requires :y, type: Integer + end + route_param :y do + get do + { + params: params, + declared_params: declared(params) + } + end + end + end + + mounted = Class.new(Grape::API) + mounted.namespace :another do + params do + requires :mount_space, type: Integer + end + route_param :mount_space do + mount doubly_mounted + end + end + + subject.format :json + subject.namespace :something do + params do + requires :id, type: Integer + end + resource ':id' do + mount mounted + end + end + end + + it 'can access parent attributes' do + get '/something/123/another/456/more/789' + expect(last_response.status).to eq 200 + json = JSON.parse(last_response.body, symbolize_names: true) + + # test all three levels of params + expect(json[:declared_params][:y]).to eq 789 + expect(json[:declared_params][:mount_space]).to eq 456 + expect(json[:declared_params][:id]).to eq 123 + end + end + + describe '#declared; mixed nesting' do + before do + subject.format :json + subject.resource :users do + route_param :id, type: Integer, desc: 'ID desc' do + # Adding this causes route_setting(:declared_params) to be nil for the + # get block in namespace 'foo' below + get do + end + + namespace 'foo' do + get do + { + params: params, + declared_params: declared(params), + declared_params_no_parent: declared(params, include_parent_namespaces: false) + } + end + end + end + end + end + + it 'can access parent route_param' do + get '/users/123/foo', bar: 'bar' + expect(last_response.status).to eq 200 + json = JSON.parse(last_response.body, symbolize_names: true) + + expect(json[:declared_params][:id]).to eq 123 + expect(json[:declared_params_no_parent][:id]).to eq nil + end + end + + describe '#declared; with multiple route_param' do + before do + mounted = Class.new(Grape::API) + mounted.namespace :albums do + get do + declared(params) + end + end + + subject.format :json + subject.namespace :artists do + route_param :id, type: Integer do + get do + declared(params) + end + + params do + requires :filter, type: String + end + get :some_route do + declared(params) + end + end + + route_param :artist_id, type: Integer do + namespace :compositions do + get do + declared(params) + end + end + end + + route_param :compositor_id, type: Integer do + mount mounted + end + end + end + + it 'return only :id without :artist_id' do + get '/artists/1' + json = JSON.parse(last_response.body, symbolize_names: true) + + expect(json.key?(:id)).to be_truthy + expect(json.key?(:artist_id)).not_to be_truthy + end + + it 'return only :artist_id without :id' do + get '/artists/1/compositions' + json = JSON.parse(last_response.body, symbolize_names: true) + + expect(json.key?(:artist_id)).to be_truthy + expect(json.key?(:id)).not_to be_truthy + end + + it 'return :filter and :id parameters in declared for second enpoint inside route_param' do + get '/artists/1/some_route', filter: 'some_filter' + json = JSON.parse(last_response.body, symbolize_names: true) + + expect(json.key?(:filter)).to be_truthy + expect(json.key?(:id)).to be_truthy + expect(json.key?(:artist_id)).not_to be_truthy + end + + it 'return :compositor_id for mounter in route_param' do + get '/artists/1/albums' + json = JSON.parse(last_response.body, symbolize_names: true) + + expect(json.key?(:compositor_id)).to be_truthy + expect(json.key?(:id)).not_to be_truthy + expect(json.key?(:artist_id)).not_to be_truthy + end + end +end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index a45abe36d3..4bbeb070c3 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -280,540 +280,6 @@ def app end end - describe '#declared' do - before do - subject.format :json - subject.params do - requires :first - optional :second - optional :third, default: 'third-default' - optional :nested, type: Hash do - optional :fourth - optional :fifth - optional :nested_two, type: Hash do - optional :sixth - optional :nested_three, type: Hash do - optional :seventh - end - end - optional :nested_arr, type: Array do - optional :eighth - end - end - optional :arr, type: Array do - optional :nineth - end - end - end - - context 'when params are not built with default class' do - it 'returns an object that corresponds with the params class - hash with indifferent access' do - subject.params do - build_with Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder - end - subject.get '/declared' do - d = declared(params, include_missing: true) - { declared_class: d.class.to_s } - end - - get '/declared?first=present' - expect(JSON.parse(last_response.body)['declared_class']).to eq('ActiveSupport::HashWithIndifferentAccess') - end - - it 'returns an object that corresponds with the params class - hashie mash' do - subject.params do - build_with Grape::Extensions::Hashie::Mash::ParamBuilder - end - subject.get '/declared' do - d = declared(params, include_missing: true) - { declared_class: d.class.to_s } - end - - get '/declared?first=present' - expect(JSON.parse(last_response.body)['declared_class']).to eq('Hashie::Mash') - end - - it 'returns an object that corresponds with the params class - hash' do - subject.params do - build_with Grape::Extensions::Hash::ParamBuilder - end - subject.get '/declared' do - d = declared(params, include_missing: true) - { declared_class: d.class.to_s } - end - - get '/declared?first=present' - expect(JSON.parse(last_response.body)['declared_class']).to eq('Hash') - end - end - - it 'should show nil for nested params if include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end - - get '/declared?first=present' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']['fourth']).to be_nil - end - - it 'does not work in a before filter' do - subject.before do - declared(params) - end - subject.get('/declared') { declared(params) } - - expect { get('/declared') }.to raise_error( - Grape::DSL::InsideRoute::MethodNotYetAvailable - ) - end - - it 'has as many keys as there are declared params' do - subject.get '/declared' do - declared(params) - end - get '/declared?first=present' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body).keys.size).to eq(5) - end - - it 'has a optional param with default value all the time' do - subject.get '/declared' do - declared(params) - end - get '/declared?first=one' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['third']).to eql('third-default') - end - - it 'builds nested params' do - subject.get '/declared' do - declared(params) - end - - get '/declared?first=present&nested[fourth]=1' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 4 - end - - it 'builds nested params when given array' do - subject.get '/dummy' do - end - subject.params do - requires :first - optional :second - optional :third, default: 'third-default' - optional :nested, type: Array do - optional :fourth - end - end - subject.get '/declared' do - declared(params) - end - - get '/declared?first=present&nested[][fourth]=1&nested[][fourth]=2' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested'].size).to eq 2 - end - - context 'sets nested objects when the param is missing' do - it 'to be a hash when include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end - - get '/declared?first=present' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']).to eq({}) - end - - it 'to be an array when include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end - - get '/declared?first=present' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['arr']).to be_a(Array) - end - - it 'to be an array when nested and include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end - - get '/declared?first=present&nested[fourth]=1' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']['nested_arr']).to be_a(Array) - end - - it 'to be nil when include_missing is false' do - subject.get '/declared' do - declared(params, include_missing: false) - end - - get '/declared?first=present' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']).to be_nil - end - end - - it 'filters out any additional params that are given' do - subject.get '/declared' do - declared(params) - end - get '/declared?first=one&other=two' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body).key?(:other)).to eq false - end - - it 'stringifies if that option is passed' do - subject.get '/declared' do - declared(params, stringify: true) - end - - get '/declared?first=one&other=two' - expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['first']).to eq 'one' - end - - it 'does not include missing attributes if that option is passed' do - subject.get '/declared' do - error! 'expected nil', 400 if declared(params, include_missing: false).key?(:second) - '' - end - - get '/declared?first=one&other=two' - expect(last_response.status).to eq(200) - end - - it 'does not include renamed missing attributes if that option is passed' do - subject.params do - optional :renamed_original, as: :renamed - end - subject.get '/declared' do - error! 'expected nil', 400 if declared(params, include_missing: false).key?(:renamed) - '' - end - - get '/declared?first=one&other=two' - expect(last_response.status).to eq(200) - end - - it 'includes attributes with value that evaluates to false' do - subject.params do - requires :first - optional :boolean - end - - subject.post '/declared' do - error!('expected false', 400) if declared(params, include_missing: false)[:boolean] != false - '' - end - - post '/declared', ::Grape::Json.dump(first: 'one', boolean: false), 'CONTENT_TYPE' => 'application/json' - expect(last_response.status).to eq(201) - end - - it 'includes attributes with value that evaluates to nil' do - subject.params do - requires :first - optional :second - end - - subject.post '/declared' do - error!('expected nil', 400) unless declared(params, include_missing: false)[:second].nil? - '' - end - - post '/declared', ::Grape::Json.dump(first: 'one', second: nil), 'CONTENT_TYPE' => 'application/json' - expect(last_response.status).to eq(201) - end - - it 'includes missing attributes with defaults when there are nested hashes' do - subject.get '/dummy' do - end - - subject.params do - requires :first - optional :second - optional :third, default: nil - optional :nested, type: Hash do - optional :fourth, default: nil - optional :fifth, default: nil - requires :nested_nested, type: Hash do - optional :sixth, default: 'sixth-default' - optional :seven, default: nil - end - end - end - - subject.get '/declared' do - declared(params, include_missing: false) - end - - get '/declared?first=present&nested[fourth]=&nested[nested_nested][sixth]=sixth' - json = JSON.parse(last_response.body) - expect(last_response.status).to eq(200) - expect(json['first']).to eq 'present' - expect(json['nested'].keys).to eq %w[fourth fifth nested_nested] - expect(json['nested']['fourth']).to eq '' - expect(json['nested']['nested_nested'].keys).to eq %w[sixth seven] - expect(json['nested']['nested_nested']['sixth']).to eq 'sixth' - end - - it 'does not include missing attributes when there are nested hashes' do - subject.get '/dummy' do - end - - subject.params do - requires :first - optional :second - optional :third - optional :nested, type: Hash do - optional :fourth - optional :fifth - end - end - - subject.get '/declared' do - declared(params, include_missing: false) - end - - get '/declared?first=present&nested[fourth]=4' - json = JSON.parse(last_response.body) - expect(last_response.status).to eq(200) - expect(json['first']).to eq 'present' - expect(json['nested'].keys).to eq %w[fourth] - expect(json['nested']['fourth']).to eq '4' - end - end - - describe '#declared; call from child namespace' do - before do - subject.format :json - subject.namespace :parent do - params do - requires :parent_name, type: String - end - - namespace ':parent_name' do - params do - requires :child_name, type: String - requires :child_age, type: Integer - end - - namespace ':child_name' do - params do - requires :grandchild_name, type: String - end - - get ':grandchild_name' do - { - 'params' => params, - 'without_parent_namespaces' => declared(params, include_parent_namespaces: false), - 'with_parent_namespaces' => declared(params, include_parent_namespaces: true) - } - end - end - end - end - - get '/parent/foo/bar/baz', child_age: 5, extra: 'hello' - end - - let(:parsed_response) { JSON.parse(last_response.body, symbolize_names: true) } - - it { expect(last_response.status).to eq 200 } - - context 'with include_parent_namespaces: false' do - it 'returns declared parameters only from current namespace' do - expect(parsed_response[:without_parent_namespaces]).to eq( - grandchild_name: 'baz' - ) - end - end - - context 'with include_parent_namespaces: true' do - it 'returns declared parameters from every parent namespace' do - expect(parsed_response[:with_parent_namespaces]).to eq( - parent_name: 'foo', - child_name: 'bar', - grandchild_name: 'baz', - child_age: 5 - ) - end - end - - context 'without declaration' do - it 'returns all requested parameters' do - expect(parsed_response[:params]).to eq( - parent_name: 'foo', - child_name: 'bar', - grandchild_name: 'baz', - child_age: 5, - extra: 'hello' - ) - end - end - end - - describe '#declared; from a nested mounted endpoint' do - before do - doubly_mounted = Class.new(Grape::API) - doubly_mounted.namespace :more do - params do - requires :y, type: Integer - end - route_param :y do - get do - { - params: params, - declared_params: declared(params) - } - end - end - end - - mounted = Class.new(Grape::API) - mounted.namespace :another do - params do - requires :mount_space, type: Integer - end - route_param :mount_space do - mount doubly_mounted - end - end - - subject.format :json - subject.namespace :something do - params do - requires :id, type: Integer - end - resource ':id' do - mount mounted - end - end - end - - it 'can access parent attributes' do - get '/something/123/another/456/more/789' - expect(last_response.status).to eq 200 - json = JSON.parse(last_response.body, symbolize_names: true) - - # test all three levels of params - expect(json[:declared_params][:y]).to eq 789 - expect(json[:declared_params][:mount_space]).to eq 456 - expect(json[:declared_params][:id]).to eq 123 - end - end - - describe '#declared; mixed nesting' do - before do - subject.format :json - subject.resource :users do - route_param :id, type: Integer, desc: 'ID desc' do - # Adding this causes route_setting(:declared_params) to be nil for the - # get block in namespace 'foo' below - get do - end - - namespace 'foo' do - get do - { - params: params, - declared_params: declared(params), - declared_params_no_parent: declared(params, include_parent_namespaces: false) - } - end - end - end - end - end - - it 'can access parent route_param' do - get '/users/123/foo', bar: 'bar' - expect(last_response.status).to eq 200 - json = JSON.parse(last_response.body, symbolize_names: true) - - expect(json[:declared_params][:id]).to eq 123 - expect(json[:declared_params_no_parent][:id]).to eq nil - end - end - - describe '#declared; with multiple route_param' do - before do - mounted = Class.new(Grape::API) - mounted.namespace :albums do - get do - declared(params) - end - end - - subject.format :json - subject.namespace :artists do - route_param :id, type: Integer do - get do - declared(params) - end - - params do - requires :filter, type: String - end - get :some_route do - declared(params) - end - end - - route_param :artist_id, type: Integer do - namespace :compositions do - get do - declared(params) - end - end - end - - route_param :compositor_id, type: Integer do - mount mounted - end - end - end - - it 'return only :id without :artist_id' do - get '/artists/1' - json = JSON.parse(last_response.body, symbolize_names: true) - - expect(json.key?(:id)).to be_truthy - expect(json.key?(:artist_id)).not_to be_truthy - end - - it 'return only :artist_id without :id' do - get '/artists/1/compositions' - json = JSON.parse(last_response.body, symbolize_names: true) - - expect(json.key?(:artist_id)).to be_truthy - expect(json.key?(:id)).not_to be_truthy - end - - it 'return :filter and :id parameters in declared for second enpoint inside route_param' do - get '/artists/1/some_route', filter: 'some_filter' - json = JSON.parse(last_response.body, symbolize_names: true) - - expect(json.key?(:filter)).to be_truthy - expect(json.key?(:id)).to be_truthy - expect(json.key?(:artist_id)).not_to be_truthy - end - - it 'return :compositor_id for mounter in route_param' do - get '/artists/1/albums' - json = JSON.parse(last_response.body, symbolize_names: true) - - expect(json.key?(:compositor_id)).to be_truthy - expect(json.key?(:id)).not_to be_truthy - expect(json.key?(:artist_id)).not_to be_truthy - end - end - describe '#params' do it 'is available to the caller' do subject.get('/hey') do From 678cd1366462c1a679a65833d9fb6f1fc0c87c72 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 18 Sep 2020 15:21:46 +1200 Subject: [PATCH 2/2] Ensure complete declared params structure is present --- CHANGELOG.md | 3 +- README.md | 55 ++++++++++++++++--- UPGRADING.md | 47 ++++++++++++++-- lib/grape/dsl/inside_route.rb | 59 ++++++++------------ lib/grape/version.rb | 2 +- spec/grape/endpoint/declared_spec.rb | 81 +++++++++++++++++++--------- 6 files changed, 174 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08995a1a47..eaf05a116c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -### 1.4.1 (Next) +### 1.5.0 (Next) #### Features @@ -7,6 +7,7 @@ #### Fixes * Your contribution here. +* [#2103](https://github.com/ruby-grape/grape/pull/2103): Ensure complete declared params structure is present - [@tlconnor](https://github.com/tlconnor). * [#2099](https://github.com/ruby-grape/grape/pull/2099): Added truffleruby to Travis-CI - [@gogainda](https://github.com/gogainda). * [#2089](https://github.com/ruby-grape/grape/pull/2089): Specify order of mounting Grape with Rack::Cascade in README - [@jonmchan](https://github.com/jonmchan). * [#2083](https://github.com/ruby-grape/grape/pull/2083): Set `Cache-Control` header only for streamed responses - [@stanhu](https://github.com/stanhu). diff --git a/README.md b/README.md index c3a5ce73b7..43f35cbf09 100644 --- a/README.md +++ b/README.md @@ -156,7 +156,7 @@ content negotiation, versioning and much more. ## Stable Release -You're reading the documentation for the next release of Grape, which should be **1.4.1**. +You're reading the documentation for the next release of Grape, which should be **1.5.0**. Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version. The current stable release is [1.4.0](https://github.com/ruby-grape/grape/blob/v1.4.0/README.md). @@ -353,7 +353,7 @@ use Rack::Session::Cookie run Rack::Cascade.new [Web, API] ``` -Note that order of loading apps using `Rack::Cascade` matters. The grape application must be last if you want to raise custom 404 errors from grape (such as `error!('Not Found',404)`). If the grape application is not last and returns 404 or 405 response, [cascade utilizes that as a signal to try the next app](https://www.rubydoc.info/gems/rack/Rack/Cascade). This may lead to undesirable behavior showing the [wrong 404 page from the wrong app](https://github.com/ruby-grape/grape/issues/1515). +Note that order of loading apps using `Rack::Cascade` matters. The grape application must be last if you want to raise custom 404 errors from grape (such as `error!('Not Found',404)`). If the grape application is not last and returns 404 or 405 response, [cascade utilizes that as a signal to try the next app](https://www.rubydoc.info/gems/rack/Rack/Cascade). This may lead to undesirable behavior showing the [wrong 404 page from the wrong app](https://github.com/ruby-grape/grape/issues/1515). ### Rails @@ -787,7 +787,12 @@ Available parameter builders are `Grape::Extensions::Hash::ParamBuilder`, `Grape ### Declared -Grape allows you to access only the parameters that have been declared by your `params` block. It filters out the params that have been passed, but are not allowed. Consider the following API endpoint: +Grape allows you to access only the parameters that have been declared by your `params` block. It will: + + * Filter out the params that have been passed, but are not allowed. + * Include any optional params that are declared but not passed. + +Consider the following API endpoint: ````ruby format :json @@ -820,9 +825,9 @@ Once we add parameters requirements, grape will start returning only the declare format :json params do - requires :user, type: Hash do - requires :first_name, type: String - requires :last_name, type: String + optional :user, type: Hash do + optional :first_name, type: String + optional :last_name, type: String end end @@ -850,6 +855,44 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d } ```` +Missing params that are declared as type `Hash` or `Array` will be included. + +````ruby +format :json + +params do + optional :user, type: Hash do + optional :first_name, type: String + optional :last_name, type: String + end + optional :widgets, type: Array +end + +post 'users/signup' do + { 'declared_params' => declared(params) } +end +```` + +**Request** + +````bash +curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d '{}' +```` + +**Response** + +````json +{ + "declared_params": { + "user": { + "first_name": null, + "last_name": null + }, + "widgets": [] + } +} +```` + The returned hash is an `ActiveSupport::HashWithIndifferentAccess`. The `#declared` method is not available to `before` filters, as those are evaluated prior to parameter coercion. diff --git a/UPGRADING.md b/UPGRADING.md index 591fa51add..9d63392b96 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,45 @@ Upgrading Grape =============== +### Upgrading to >= 1.5.0 + +Prior to 1.3.3, the `declared` helper would always return the complete params structure if `include_missing=true` was set. In 1.3.3 a regression was introduced such that a missing Hash with or without nested parameters would always resolve to `{}`. + +In 1.5.0 this behavior is reverted, so the whole params structure will always be available via `declared`, regardless of whether any params are passed. + +The following rules now apply to the `declared` helper when params are missing and `include_missing=true`: + +* Hash params with children will resolve to a Hash with keys for each declared child. +* Hash params with no children will resolve to `{}`. +* Set params will resolve to `Set.new`. +* Array params will resolve to `[]`. +* All other params will resolve to `nil`. + +#### Example + +```ruby +class Api < Grape::API + params do + optional :outer, type: Hash do + optional :inner, type: Hash do + optional :value, type: String + end + end + end + get 'example' do + declared(params, include_missing: true) + end +end +``` + +``` +get '/example' +# 1.3.3 = {} +# 1.5.0 = {outer: {inner: {value:null}}} +``` + +For more information see [#2103](https://github.com/ruby-grape/grape/pull/2103). + ### Upgrading to >= 1.4.0 #### Reworking stream and file and un-deprecating stream like-objects @@ -28,17 +67,17 @@ class API < Grape::API end ``` -Or use `stream` to stream other kinds of content. In the following example a streamer class +Or use `stream` to stream other kinds of content. In the following example a streamer class streams paginated data from a database. ```ruby -class MyObject +class MyObject attr_accessor :result def initialize(query) @result = query end - + def each yield '[' # Do paginated DB fetches and return each page formatted @@ -47,7 +86,7 @@ class MyObject yield process_records(records, first) first = false end - yield ']' + yield ']' end def process_records(records, first) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index dc8f9f05cd..1eb0c3084e 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -58,7 +58,7 @@ def declared_hash(passed_params, options, declared_params, params_nested_path) passed_children_params = passed_params[declared_parent_param] || passed_params.class.new memo_key = optioned_param_key(declared_parent_param, options) - memo[memo_key] = handle_passed_param(passed_children_params, params_nested_path_dup) do + memo[memo_key] = handle_passed_param(params_nested_path_dup, passed_children_params.any?) do declared(passed_children_params, options, declared_children_params, params_nested_path_dup) end end @@ -70,57 +70,44 @@ def declared_hash(passed_params, options, declared_params, params_nested_path) next unless options[:include_missing] || passed_params.key?(declared_param) || (param_renaming && passed_params.key?(param_renaming)) - if param_renaming - memo[optioned_param_key(param_renaming, options)] = passed_params[param_renaming] - else - memo[optioned_param_key(declared_param, options)] = passed_params[declared_param] + memo_key = optioned_param_key(param_renaming || declared_param, options) + passed_param = passed_params[param_renaming || declared_param] + + params_nested_path_dup = params_nested_path.dup + params_nested_path_dup << declared_param.to_s + + memo[memo_key] = handle_passed_param(params_nested_path_dup) do + passed_param end end end end - def handle_passed_param(passed_children_params, params_nested_path, &_block) - if should_be_empty_hash?(passed_children_params, params_nested_path) + def handle_passed_param(params_nested_path, has_passed_children = false, &_block) + return yield if has_passed_children + + key = params_nested_path[0] + key += '[' + params_nested_path[1..-1].join('][') + ']' if params_nested_path.size > 1 + + route_options_params = options[:route_options][:params] || {} + type = route_options_params.dig(key, :type) + has_children = route_options_params.keys.any? { |k| k != key && k.start_with?(key) } + + if type == 'Hash' && !has_children {} - elsif should_be_empty_array?(passed_children_params, params_nested_path) + elsif type == 'Array' || type&.start_with?('[') [] + elsif type == 'Set' || type&.start_with?('# 1 - key - end - def optioned_declared_params(**options) declared_params = if options[:include_parent_namespaces] # Declared params including parent namespaces diff --git a/lib/grape/version.rb b/lib/grape/version.rb index 615c251ab9..05aefeb275 100644 --- a/lib/grape/version.rb +++ b/lib/grape/version.rb @@ -2,5 +2,5 @@ module Grape # The current version of Grape. - VERSION = '1.4.1' + VERSION = '1.5.0' end diff --git a/spec/grape/endpoint/declared_spec.rb b/spec/grape/endpoint/declared_spec.rb index 73d145a8eb..cf332933bd 100644 --- a/spec/grape/endpoint/declared_spec.rb +++ b/spec/grape/endpoint/declared_spec.rb @@ -28,10 +28,20 @@ def app optional :nested_arr, type: Array do optional :eighth end + optional :empty_arr, type: Array + optional :empty_typed_arr, type: Array[String] + optional :empty_hash, type: Hash + optional :empty_set, type: Set + optional :empty_typed_set, type: Set[String] end optional :arr, type: Array do optional :nineth end + optional :empty_arr, type: Array + optional :empty_typed_arr, type: Array[String] + optional :empty_hash, type: Hash + optional :empty_set, type: Set + optional :empty_typed_set, type: Set[String] end end @@ -103,7 +113,7 @@ def app end get '/declared?first=present' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body).keys.size).to eq(5) + expect(JSON.parse(last_response.body).keys.size).to eq(10) end it 'has a optional param with default value all the time' do @@ -122,7 +132,7 @@ def app get '/declared?first=present&nested[fourth]=1' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 4 + expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 9 end it 'builds nested params when given array' do @@ -145,45 +155,66 @@ def app expect(JSON.parse(last_response.body)['nested'].size).to eq 2 end - context 'sets nested objects when the param is missing' do - it 'to be a hash when include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end + context 'when the param is missing and include_missing=false' do + before do + subject.get('/declared') { declared(params, include_missing: false) } + end + it 'sets nested objects to be nil' do get '/declared?first=present' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']).to eq({}) + expect(JSON.parse(last_response.body)['nested']).to be_nil end + end - it 'to be an array when include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end + context 'when the param is missing and include_missing=true' do + before do + subject.get('/declared') { declared(params, include_missing: true) } + end + it 'sets objects with type=Hash to be a hash' do get '/declared?first=present' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['arr']).to be_a(Array) - end - it 'to be an array when nested and include_missing is true' do - subject.get '/declared' do - declared(params, include_missing: true) - end + body = JSON.parse(last_response.body) + expect(body['empty_hash']).to eq({}) + expect(body['nested']).to be_a(Hash) + expect(body['nested']['empty_hash']).to eq({}) + expect(body['nested']['nested_two']).to be_a(Hash) + end - get '/declared?first=present&nested[fourth]=1' + it 'sets objects with type=Set to be a set' do + get '/declared?first=present' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']['nested_arr']).to be_a(Array) + + body = JSON.parse(last_response.body) + expect(['#', []]).to include(body['empty_set']) + expect(['#', []]).to include(body['empty_typed_set']) + expect(['#', []]).to include(body['nested']['empty_set']) + expect(['#', []]).to include(body['nested']['empty_typed_set']) end - it 'to be nil when include_missing is false' do - subject.get '/declared' do - declared(params, include_missing: false) - end + it 'sets objects with type=Array to be an array' do + get '/declared?first=present' + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body['empty_arr']).to eq([]) + expect(body['empty_typed_arr']).to eq([]) + expect(body['arr']).to eq([]) + expect(body['nested']['empty_arr']).to eq([]) + expect(body['nested']['empty_typed_arr']).to eq([]) + expect(body['nested']['nested_arr']).to eq([]) + end + it 'includes all declared children when type=Hash' do get '/declared?first=present' expect(last_response.status).to eq(200) - expect(JSON.parse(last_response.body)['nested']).to be_nil + + body = JSON.parse(last_response.body) + expect(body['nested'].keys).to eq(%w[fourth fifth nested_two nested_arr empty_arr empty_typed_arr empty_hash empty_set empty_typed_set]) + expect(body['nested']['nested_two'].keys).to eq(%w[sixth nested_three]) + expect(body['nested']['nested_two']['nested_three'].keys).to eq(%w[seventh]) end end