checked out the code, looks good Rohit. It will give us a handle to keep the API from becoming even more inconsistent, and maybe even improve.
On Thu, Apr 21, 2016 at 1:07 PM, Rohit Yadav <rohit.ya...@shapeblue.com> wrote: > All, > > Whenever we've to write new APIs, we end up putting a lot of validations > such as Strings.isNullOrEmpty or checks on API args such as on id we do > this a lot getId() != null && getId() > 0 and on failing throw server api > exception (as cloudstack entity ids are always > 0, such as ids of vms, > volumes etc). > > While working on recent dynamic-roles PR, I ended up writing several such > validations for all the new APIs and based on suggestions from John I could > rid of all such checks by introducing a new @Parameter annotation field > call validators() which is a list of validators we want to run for an API > arg. This does not break any existing API which might have inconsistent > assumptions (such as few accept id to be -1), and on local env all existing > unit tests and smoke tests have passed. > > Please find an implementation in the dynamic-roles PR where all the new > dynamic roles APIs use these annotations: > https://github.com/apache/cloudstack/pull/1489 > > Ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-9361 > > As soon as the PR is accepted and merged, I'll update the wiki page and we > can get this pattern adopted over time: > > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Annotations+use+in+the+API > > Regards, > Rohit Yadav > > Regards, > > Rohit Yadav > > rohit.ya...@shapeblue.com > www.shapeblue.com > 53 Chandos Place, Covent Garden, London WC2N 4HSUK > @shapeblue > -- Daan
