Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing optional Array params default to the Hash instead of Array #892

Closed
adamcooper opened this issue Jan 14, 2015 · 8 comments
Closed

Comments

@adamcooper
Copy link

If if declare a param as a type: Array with nested params

params do
  optional :comments, type: Array do
    optional :description, type: String
  end
end
post do
  { params: params.slice(:comments), declared: declared(params) }
end

and it is missing from the incoming params (or has a default of nil) then it defaults to a Hash

curl -X POST http://localhost:9292/api/entities  -d '{}' -H "Content-Type: application/json"

{"params":{},"declared":{"comments":{"description":null}}}

This only occurs on the optional as the group and requires both throw validation errors.

There is a workaround by setting a default: [] on the Array param.

@dblock
Copy link
Member

dblock commented Jan 14, 2015

This is an interesting one. I think it's a legit bug. Would love at least a spec that reproduces it.

@adamcooper
Copy link
Author

@dblock - I opened a PR w/ two specs for Arrays and Hashes.

I wonder if the required params inside optional groups issue (#723) is related.

@dm1try
Copy link
Member

dm1try commented Feb 15, 2015

There are two different issues here.

The first,

def slice(*keys) # If the given keys don't exist, returns an empty hash.

so { params: params.slice(:missing_param) } =>"{"params":{}}" is expected behaviour

the second one, declared has include_missing option(by default is true)
so your specs from #895 are passed if include_missing: false is used.

@adamcooper take a look :)

@adamcooper
Copy link
Author

@dm1try - Thanks for the response.

I just included the first slice to prove that nothing was actually in the params.

I wrote the specs to possibly simplify the implementation.

The issue is the default value (with include_missing = true) for nested arrays comes back as a hash instead of an array. That seems like a bug to me.

I updated the specs in this commit to help show the issue.

@dm1try
Copy link
Member

dm1try commented Feb 18, 2015

@adamcooper,

I just included the first slice to prove that nothing was actually in the params.

why just not params[:comments]?)

The issue is the default value (with include_missing = true) for nested arrays comes back as a hash instead of an array. That seems like a bug to me.

Anyway, it's definitely a bug. I see now.

I updated the specs in this commit to help show the issue.

Thanks, now it is better than it was and makes sense!

@adamcooper
Copy link
Author

@dm1try - I can't remember exactly why I used slice instead of the simpler params[:comments]. I think I was trying different combinations of multiple parameters so slice(:array, :hash) was simpler in one line rather than array: params[:array], hash: params[:hash]. hehe.

Thanks! Please let me know if I can help out. I spent some time reading through the source when I was using Grape and Grape Swagger - trying to understand how to use them together really. I'm not super familiar but would like to help out if I can.

I originally ran into something like this issue #928. I also was trying to use nested require(s/d) validations inside an optional array validation.

@u2
Copy link
Contributor

u2 commented Apr 20, 2015

See #994.It can be closed, I think.

@dblock
Copy link
Member

dblock commented Apr 20, 2015

Closing, @adamcooper please confirm.

@dblock dblock closed this as completed Apr 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants