cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rohit Yadav" <rohit.ya...@citrix.com>
Subject Re: Review Request: API_refactoring: add parameter annotation for user 'security-group' group
Date Mon, 17 Dec 2012 18:34:07 GMT

-----------------------------------------------------------
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.shetty@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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message