Allow for strings in 'only' and 'except' options #136
Conversation
|
|
||
| @only_fields ||= options[:only].each_with_object({}) do |attribute, allowed_fields| | ||
| if attribute.is_a?(Hash) | ||
| attribute.each do |attr, nested_attrs| | ||
| attr = attr.to_sym |
There was a problem hiding this comment.
I think you should convert those to strings. Until Ruby 2.2 symbols are not garbage collected so you're creating an opportunity for some DDoS if this is used with user-supplied data.
|
See my comment around symbols vs. strings to be converted/used internally. Otherwise this makes total sense. |
|
@dblock what are your thoughts on this latest approach (and trailing-method-call-on-an- Rather than coercing the keys to strings inside the loop, we can use ActiveSupport's |
|
I think this is fine. I'd like to have explicit tests for this rather than updating existing tests with mixed keys. Write one that says something like "it works for strings", another that "it works for symbols" and another that "it works for mixed strings and symbols". |
|
Absolutely. Just added explicit tests for the symbol and string code paths of |
|
Good. Could you please squash these commits? I'll merge. |
With the addition of the `only` and `except` options, it opens up the possiblities to the [partial response](http://bit.ly/1fxWuXJ) design pattern where the consumer of the API can specifically request only the fields that it desires. A common pattern to request these fields would be via `params`, which will arrive as strings. Currently only an array of symbols (or hash containing symbols) is supported to define the desired fields. This commit updates Grape::Entity to support an array of symbols _or_ strings to be passed as the `only` or `except` options of `represent`.
|
Much thanks @dblock! Done. |
Allow for strings in 'only' and 'except' options
|
Merged. |
With the addition of the
onlyandexceptoptions, possibilities such as the partial response design pattern (where the consumer of the API can specifically request only the fields that it desires) become available. A common pattern to request these fields would be viaparams, which will arrive as strings. Currently only an array of symbols (or hash containing symbols) is supported to define the desired fields.As discussed here, it certainly should be the responsibility of another gem to do parameter validation in a partial response scenario, but such a gem would also have to do a [perhaps unnecessary] recursive symbol coercion as well when it could be handled by grape-entity, as proposed by this pull request.
This commit updates Grape::Entity to support an array of symbols or strings to be passed as the
onlyorexceptoptions ofrepresent.