added nested attributes validation support

[schmurfy]

I added support for nested attribute validations to allow this:

params do
  requires 'user.id', type: Integer
  requires 'user.name', type: String
end

This works alongside what I and @adamgotterer did and allow both the symbol syntax as well as this dotted string syntax.

I think we can generate these validations to validate Virtus value objects but there will be some scope issues (a validation should be able to create other validations which create some issues with the current interface).

[adamgotterer]

What would the behavior be if you did the following and the User object described the attributes? Would it be smart enough to extract out the parts?

params do
  params 'user', type: User
end

[schmurfy]

I am pretty sure we can turn this:

params do
  params 'user', type: User
end

into this:

params do
  params 'user', type: User
  requires 'user.id', type: Integer
  requires 'user.name', type: String
end

I have already played enough with Virtus to know it provides every attribute definitions so we can use them directly,
the only problem for me is to find a way to implement this which does not do too much dirty things (since the validation class would need the API Class to add anything to it) but I did not gave it to much thought as I wanted to get the basic model out first.

If you have any idea on a clean way to implement this, basically what we need to know is look at the type provided in the coerce validator and if we have a Virtus value object then extract the informations and add the new validations in the method definitions.

[adamgotterer]

For clarification: I think there are two different possible things you can do here.

1. If you are passing nested objects you shouldn't even need a Virtus object. The ability to coerce nested objects is an awesome feature and would work the same as another other param.

2. Not sure exactly how this would work. But if you specify a custom type it should leverage the information in the Virtus object to map the coercions. I'm not sure how we should handle the requires/optional settings though. But it seems repetitive to have to set the types again in the params block.

[schmurfy]

@dblock any thoughts on this ?:)

[dblock]

I am not a big fan of automatic parsing of dotted notations. I think that "nested" attributes should be truly nested, much like grape routes.

requires :user do
  param :id, type: Integer
end

Furthermore, I think nested parameters are a beginning of a black hole of confusion and misunderstanding. But I've been reading a lot of science-fiction lately, so I might be under influence :)

[adamgotterer]

I liked the original idea of custom object coercion, but the coercion and validation never worked as expected. I agree that this way could become confusing.

[schmurfy]

@dblock nested attributes validation is the reason why I added validations to grape, I am not really sure how it can become confusing.
I used the dot syntax for many reasons:

• little to no chance a dot will be used in an actual attribute name
• we need a label for error reporting and in this case it matches the name you wrote

I can give your syntax a try if it can helps getting this in core, another solution can be to provide what is needed in the core to add this as an external gem.

what do you think ?

[dblock]

I'd like some more people to comment on this, of course, first.

I would love nice nested attribute support in Grape, but I think a nested attribute DSL that matches parameters 1:1 without any parsing would be a much more robust solution. Maybe a simplified version with an explicit group keyword?

group :user do
...
end

Also, how do we do with double-nesting: "user.widget.id"?

[schmurfy]

I never needed more then one level so I never really gave it much thoughts
but my current implementation should work with more than one level.
The next question is whether or not using more then one level is a good idea but I prefer to let the users decide.

[dblock]

I know this has been open for a while and we're not getting any useful comments from other people. If @jch and @mbleigh are reading this, I would really appreciate an opinion one way or another.

My opinion is that a nested DSL is what we would want, and that I feel pretty strongly about not having parameter parsing where "x.y" is a special format. I see this PR as a step back, which means I don't want to merge it. @schmurfy, no hard feelings.

[jonhyman]

I agree with @dblock about not using dotted parameters. It feels nonstandard to something for Ruby (whereas Mongo using dot notation seems fine). If you're defining an object, you should be able to group it together in one block. E.g., don't allow people to break up the definition like this:

requires 'user.id'
requires 'foo'
requires 'user.name'

Though, do you even need to validate down at a nested level? I could see that being a maintenance burden when modifying your models. That is, if you're validating at a nested level, you're probably doing something like validating that "user" contains "id" and "name", but then you can't gracefully add "email" without updating your validations.

[schmurfy]

Though, do you even need to validate down at a nested level? I could see that being a maintenance burden when modifying your models.
In my case the json is not mapped 1:1 to a model and I prefer having the validations inside my API than adding a dummy model to do the validation, it just feels more logic.

Whether or not I want to have nested attributes is out of my hands, I just need to provide such an API for an existing application and it will no change its API anytime soon and this is not such an exotic need, here is an example coming from not far: http://developer.github.com/v3/repos/#list-organization-repositories :)

I don't really care about the syntax I care more about the feature, as I said there are two ways for me I can get this:

• we can agree on a form which could be merged
• or the grape gem could provide some sort of extensions api to add more validations (and at the same time allow deeply nested attributes validation)

There is a last way which is to maintain my fork but I really don't want that, that is what I already have with goliath since they removed a large chunk of what I use (which I based my work on but did not added, namely the routing part).
(The service I want to port to grape is currently powered by Goliath)

I am fine with something like this and can try to implement it if you prefer:

requires :operation, type: String

group :user do
  requires :id, type: Integer
  requires :login, type: String
end

What do you think ?

[dblock]

@schmurfy I would definitely merge something that implements the group syntax.

[mbleigh]

I definitely like a nested DSL better than dot notation. Agree with @dblock

On Tue, Aug 21, 2012 at 5:34 PM, Daniel Doubrovkine (dB.) <
notifications@github.com> wrote:

@schmurfy https://github.com/schmurfy I would definitely merge
something that implements the group syntax.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/222#issuecomment-7918906.

[schmurfy]

ok, i will give the group syntax a try.

[dblock]

@schmurfy if you're happy with #236, you can close this here

[schmurfy]

#236 looks nice indeed :)