-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8565/#review14593
-----------------------------------------------------------
Ship it!
Fixed your patch, pl. read the following for future patches:
Why we are doing what we're doing in api_refactoring:
0. For better doc generation. Aggregate apis with src packages around common
entity.
1. Decouple role based api call verification using a plugin/adapter.
2. Decouple tightly coupled database and acl validations from service layer to
api layer using @ACL, @Validator. They can be adapters too.
3. Using CommandType.UUID to annotate fields that are uuids from over the wire
requests and use this annotations to convert them from string to internal ID.
entityType helps us know the Response class whose @Entity helps us know the db
table (impl. in EntityManagerImpl). Useful for @Validator to validate db/range
validations as well.
4. Remove bloat, decouple policy from mechanism (using plugins or adapters),
data from code wherever possible and use OOP styled coding, remove
iterative/primitive C like code wherever possible.
(NOTE: Most of it would be done in all Cmd classes, only @Entity in Response
classes)
0. Remove IdentityMapper and do 1.
1. Fix/add entityType to a Response.class, in @Parameter. Goto 2.
2. For every Response class that was added in the entityType field in
@Parameter in 1., add @Entity(). The idea is that from this @Entity we would
know the interface through which we can know the db table. For. example for
UserResponse, we should annotate with @Entity(User.class) and which is an
interfaces implemented by UserVO.java through which we know it has
@Table(name="user"). This is useful to convert the over-the-wire requests that
have uuids to convert them to internal ids.
3. In @Parameter, if the entity would receive a uuid, change it's type to
CommandType.UUID (recently introduced) and collectionType (if it's collection
of Long, or basically list of uuids for example) too. This annotation will help
us generate docs so users/developers would know what params is actually uuid.
(See 1-3, to better understand why we're doing this)
4. @ACL annotation for all params in a cmd class whose access control for the
caller user has to be checked. If the parameter is a Map (generally would be a
Long that is UUID type), a map or dictionary has a key and a value, we need to
specify which one needs checking. For example:
@ACL(checkValueAccess=true)
@Parameter(bla bla bla, entityType={class1Response.class,class2Response.class})
private Map IpToNetworkList;
5. Add @Validator (defined as @validate in FS) to annotate which field in Cmd
needs to be be validated and using which validators (current we've
DBEntityValidator.class and RandgeValidator.class). This is work in progress,
won't work as of now. You may leave @Validator for now. Example:
@ACL
@Validator(validtors={DBEntityValidator.class})
@Parameter(bla bla bla, type=CommandType.UUID,
entityType={class1Response.class,class2Response.class})
private Long domainId;
Present status, all apis which are queried by passing a uuid (for ex. list apis
with uuid param) are broken, work in progress.
Applied on api_refactoring:
commit cba97b17423a8dda4e8d8b170088fe014754d283
Author: Likitha Shetty <likitha.she...@citrix.com>
Date: Mon Dec 17 09:55:52 2012 -0800
api_refactoring: add parameter annotation for user 'security-group'
- Add the entityType to the parameter annotation
- Annotate SecurityGroupRules response
- Rohit Yadav
On Dec. 17, 2012, 10:35 a.m., Likitha Shetty wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8565/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2012, 10:35 a.m.)
>
>
> Review request for cloudstack, Rohit Yadav and Fang Wang.
>
>
> Description
> -------
>
> Add the entityType to the parameter annotation.
> Annotate SecurityGroupRules response
>
>
> This addresses bug CLOUDSTACK-518.
>
>
> Diffs
> -----
>
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/AuthorizeSecurityGroupEgressCmd.java
> 566e5c0
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/AuthorizeSecurityGroupIngressCmd.java
> b5e6aa2
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/CreateSecurityGroupCmd.java
> b83a972
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/DeleteSecurityGroupCmd.java
> 5ca76ef
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/ListSecurityGroupsCmd.java
> 3a49b26
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/RevokeSecurityGroupEgressCmd.java
> 9783218
>
> api/src/org/apache/cloudstack/api/command/user/securitygroup/RevokeSecurityGroupIngressCmd.java
> 34c0004
> api/src/org/apache/cloudstack/api/response/SecurityGroupRuleResponse.java
> 206e5fb
>
> Diff: https://reviews.apache.org/r/8565/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Likitha Shetty
>
>
