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

Add nested validator to validate nested children #189

Merged
merged 7 commits into from
Jan 27, 2014

Conversation

mtparet
Copy link
Contributor

@mtparet mtparet commented Jan 24, 2014

The goal of this validator is to validate each element inside of the array.

It enable to validate this kind of parameters:

{ 
  user_id: 5,
  comments: [
   {
     name: 'name of comment1'
     comment: 'blabla1'
   },
  {
     name: 'name of comment2'
     comment: 'blabla2'
  }
  ]
}

The definition of the parameters will be:

param :user_id, Integer
param :comments, Array do
    param :name, String
    param :comment, String
end

I think this validator could be useful for many people and so should be included by default in the gem.

Any remarks ?

@iNecas
Copy link
Member

iNecas commented Jan 24, 2014

You're on fire! Seems like it might solve the #151 issue, right?

What it it worked this way (as suggested in #151):

param :user_id, Integer
param :comments, Array do
  param :name, String
  param :comment, String
end

Will try to find some time over the weekend to look though the rest of the PRs, with plan of having new version released next week with this changes (and perhaps some others that I manage to review)

@mtparet
Copy link
Contributor Author

mtparet commented Jan 25, 2014

In fact, I'm not sure Array is a better name.
Array should succeed to validate this kind of structure without nested hash:

ids: [4, 10, 6, 7]  
comments: ['string1', 'string2']

so the parameters definition will be like:

param :comments, Array do
  param nil, String # no name for the parameter
end

param :ids, Array do
  param nil, Integer # no name for the parameter
end

which is different from what I implemented which can validate this kind of structure:

  comments: [
   {
     name: 'name of comment1'
     comment: 'blabla1'
   },
  {
     name: 'name of comment2'
     comment: 'blabla2'
  }
  ]

But perhaps we could find a solution where Array could work for both structures (with and without nested hash).

@iNecas
Copy link
Member

iNecas commented Jan 25, 2014

For the example above, it might be something like:

param :ids, array_of(String)

What do you thing about this?

@mtparet
Copy link
Contributor Author

mtparet commented Jan 26, 2014

or why not just using brackets ?

param :ids, [String]

@iNecas
Copy link
Member

iNecas commented Jan 26, 2014

That looks good!

@mtparet
Copy link
Contributor Author

mtparet commented Jan 27, 2014

I committed the changes to use Array instead of :nested. I'm not sure I will have the time to implement the simple Array validator [Type] this week. (But this PR could be merged without it)

end

def self.build(param_description, argument, options, block)
self.new(param_description, block, options[:param_group]) if block.is_a?(Proc) && block.arity <= 0 && argument == Array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicker: could the block.arity be less than 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know I just copied the one used for Hash without thinking about it :(
https://github.com/Pajk/apipie-rails/blob/master/lib/apipie/validator.rb#L204

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm checking this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, arity can be negative, when optional args are given http://www.ruby-doc.org/core-2.1.0/Proc.html#method-i-arity. Anyway, I'm quite sure we want to have enough 0 there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you are right, == 0 should be better.

@iNecas
Copy link
Member

iNecas commented Jan 27, 2014

Nice, I have one nitpicker comment left, but all looks great otherwise

@iNecas
Copy link
Member

iNecas commented Jan 27, 2014

Fixes #151

iNecas added a commit that referenced this pull request Jan 27, 2014
Add nested validator to validate nested children
@iNecas iNecas merged commit c202372 into Apipie:master Jan 27, 2014
@iNecas iNecas mentioned this pull request Jan 27, 2014
@iNecas
Copy link
Member

iNecas commented Jan 27, 2014

Thanks a lot for this patch! Great job

@nlenepveu nlenepveu deleted the nested_validator branch January 29, 2014 17:57
@iNecas
Copy link
Member

iNecas commented Mar 3, 2014

Released in 0.1.0. Thank you for your help!

@ywen ywen mentioned this pull request Mar 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants