John, It's not JAX-RS that's the problem. It's the fact that CloudStack current API was written to fit what the UI needs so therefore it pulls in a lot of items that doesn't belong to REST-ful entities. So just switching to using JAX-RS would mean a big change in the current work when the current work is not designed correctly and mostly throw away.
As I mentioned in a previous mail, most of this work is designed to be reusable by a switch to a completely REST-ful api. We'll be using JAX-RS then. I did a review of JAX-RS 1.1 and 2.0 and felt that it's still somewhat lacking but I think it can be augmented by other annotations to make it work better. --Alex > -----Original Message----- > From: John Burwell [mailto:[email protected]] > Sent: Saturday, December 22, 2012 11:43 AM > To: [email protected] > Subject: Re: Merge request: Merging api_refactoring on master > > Alex, > > Just out of curiosity, what type of backwards compatibility issues > prevented the adoption of a standard mechanism such as JAX-RS? > > Thanks, > -John > > On Dec 22, 2012, at 2:40 PM, Alex Huang <[email protected]> wrote: > > > John, > > > > This refactoring did not use JAX-RS precisely because of the concern that > we needed to keep backwards compatibility. > > > > --Alex > > > >> -----Original Message----- > >> From: John Burwell [mailto:[email protected]] > >> Sent: Saturday, December 22, 2012 10:58 AM > >> To: [email protected] > >> Subject: Re: Merge request: Merging api_refactoring on master > >> > >> Alex, > >> > >> Is the refactoring based on JAX-RS or does it still use the custom > >> REST mechanism? > >> > >> Thanks, > >> -John > >> > >> On Dec 22, 2012, at 1:52 PM, Alex Huang <[email protected]> wrote: > >> > >>> Correct me if I'm wrong here, Rohit. I believe the refactoring work > consists > >> of the following. > >>> > >>> - Moving java packages around for better grouping. These doesn't have > >> much impact on the query API, except for maybe some typos in the > >> commands.properties file. > >>> - Splitting the commands that have optional admin commands into an > >> admin package. The current commands.properties should still be > referencing > >> the admin package as it is backwards compatible with 4.0.0. > >>> - Additions in the processing engine to process the new annotations > added. > >> If the annotation is not there, the processing remains the same as the > 4.0.0. > >>> - Work on the response side to make sure the UUIDs that were being > >> returned are not done through n+1 queries but from a big join. > >>> > >>> The work on uuid etc actually happened in 3.0.0 but it was done in rather > >> horrific fashion, causing problems in upgrade, performance, scalability, > and > >> security. We're really just cleaning up in terms of that. If you're > >> running > 3.0- > >> 4.0, you should be seeing uuids in the responses and using uuids in the > >> incoming query parameters already. If you see specific examples where it > is > >> not, it's a bug in the api. > >>> > >>> I don't think it will break the end user api other than bugs introduced > during > >> coding. In fact, we took great effort to keep the api the same. If we > didn't > >> have that constraint, I would have designed a completely new REST style > api > >> instead of keeping the semi-awkward query language the current api is on. > >> This refactoring is all about keeping the over-the-wire api the same while > >> moving a lot of the hard-coded parameter checking, security checking, etc > >> into adapters to decouple the different aspects of checking to see if an > >> api > >> should be executed. > >>> > >>> --Alex > >>> > >>>> -----Original Message----- > >>>> From: Chip Childers [mailto:[email protected]] > >>>> Sent: Saturday, December 22, 2012 6:26 AM > >>>> To: <[email protected]> > >>>> Subject: Re: Merge request: Merging api_refactoring on master > >>>> > >>>> So it sounds like a ton of good work. > >>>> > >>>> However, the proposed merge also sounds like it breaks public API > >>>> compatibility with 4.0.0 in both the uuid / id changes and in the list > >>>> result changes. > >>>> > >>>> So I guess this is my first question: does the community agree that > >>>> the benefits of these changes outweigh the concerns about moving > >>>> straight from 4.0.0 to 5.0.0? > >>>> > >>>> Rohit, I think we HAVE to have concerns us on that question before > >>>> this merge happens. > >>>> > >>>> - chip > >>>> > >>>> Sent from my iPhone. > >>>> > >>>> On Dec 22, 2012, at 4:38 AM, Rohit Yadav <[email protected]> > wrote: > >>>> > >>>>> Hi everyone, > >>>>> > >>>>> I'm planning to merge api_refactoring branch on to master after 72 > hour > >>>> period which would be Monday EOD. Pl. go through the email, and > >> previous > >>>> threads on api refactoring rework and feel free to share your ideas, > >>>> comments and vote to agree, disagree. If no one objects I would like to > >> ask > >>>> the git Santa to merge it on Christmas 25 Dec :D (after 72 hour window) > >>>>> > >>>>> The reason why I want to merge around the next week is because I > think > >>>> we would have lower frequencies of emails, review requests and > people > >>>> contributing, hence I can move around a lot of code (mostly package > >>>> renames to org.apache.cloudstack in cloud-api) and right now the > merge > >>>> conflicts are really minimum, about 100-200 lines. (A top level issues to > >> track > >>>> sub-issues: https://issues.apache.org/jira/browse/CLOUDSTACK-638) > >>>>> > >>>>> What will be affected: > >>>>> 0. Any class in cloud-api and on api-layer only > >>>>> 1. Any class that imports from/to cloud-api's response and cmd classes > >>>>> > >>>>> Some of the major changes that will be merged on master; > >>>>> 0. Over the wire (OTW) HTTP request to API server would send only > >> UUID > >>>> strings. All requests done via UUIDs (and not CloudStack's internal db's > >> IDs). > >>>>> 1. https://issues.apache.org/jira/browse/CLOUDSTACK-518 Fix > >>>> @Parameter annotation to have annotation field to a Response class > >> which > >>>> would give us entities (interface to VO objects). This would get rid of > >>>> all > >>>> IdentityMapper using which was used earlier to get VO entities from an > >>>> annotated table name. This helps us to translate OTW UUIDs to > >> CloudStack's > >>>> DB's internal IDs. > >>>>> 2. Separation of ACL Role access checker as an adapter, so > organizations > >> can > >>>> implement their own role based access checking. The mechanism > would > >> exist > >>>> in CloudStack's API server but policy checking is moved out of > CloudStack. > >>>> (https://issues.apache.org/jira/browse/CLOUDSTACK-639) This works, > >> but > >>>> was tough to get it right the first time, there is better way which I'll > share > >>>> before the merge. > >>>>> 3. Group APIs to > >>>> org.apache.cloudstack.api.{command,response}.{entity1,entity2 etc.} > >>>> packages. This is mainly done for developers, so when they work on > API > >> layer > >>>> they would know which api has what level of security and as they are > >>>> grouped based on entity type, it will be easier to search. This was > mostly > >> file > >>>> movement to org.apache.cloudstack.api package and helped us track > >> couple > >>>> of classes which are no longer needed. Another aim was to move from > >>>> com.cloud to org.apache.cloudstack (only cloud-api for now). > >>>>> 4. Annotation work as described in 1., also for @ACL etc. > >>>>> 5. DB, ACL validation wip code > >>>>> 6. A lot of list api optimizations and response view work from our > newest > >>>> commiter, Min Chen. The aim is to simply response, right now for. > >> example > >>>> when we listVMs we don't want unnecessary (serialized) response > >> objects > >>>> which could be queried using uuids separately. > >>>>> > >>>>> Pl. ask away any doubts, questions and concerns you may have. It was > >>>> challenging for me as well to understand the functional spec, to know > the > >>>> why/what/how, and if you read the old threads you can tell I did not > get it > >>>> the first time. > >>>>> > >>>>> A lot of annotation work is aimed to be completed over this weekend, > so > >>>> when the branch is finally merged it won't break any functionality. At > >> present > >>>> the branch is quite stable > >>>>> > >>>>> Testing and how or why do it? > >>>>> 0. Prasanna, Meghna? can help us write few basic sets of unit tests > and > >>>> marvin integration tests for OTW requests. We already have few of > their > >>>> patches on rb. > >>>>> 1. We can also have drivers to automate tests (Prasanna can talk more > >> on > >>>> this and on his devcloud based continuous intergration server) > >>>>> 2. If I do it now, there would be a lot more eyes to point out bugs and > I > >>>> want more people to participate in the refactoring work. > >>>>> 3. Right now, it builds and runs fine with minimal breaks and no > >>>> functionality breakage as most of the changes are only restricted to api- > >>>> server (:cloud-api artifact). I'm able to deploy a zone etc. To make the > >> UUID > >>>> thing work, I've put in hardcoded (for. ex. projectId=-1 which should be > a > >>>> string uuid not a long int value -1) stuff that saves the UI from being > >> broken > >>>> which I'll remove after merging on master so UI engineers can help fix > UI > >>>> issues. > >>>>> > >>>>> Lastly, I would like to thank Min for her amazing patches and > >> optimization > >>>> work, Prachi for her work on ACL, Fang, Prasanna, Likitha for their help > >> with > >>>> the refactoring work and for their contributions. Community for asking > >>>> questions, raising issues and thanks to Alex for his guidance, reviews > and > >>>> kickass OOP concepts. > >>>>> > >>>>> Ref; > >>>>> http://www.slideshare.net/buildacloud/cloudstack-collaboration- > >>>> conference-12-refactoring-cloud-stack > >>>>> https://git-wip-us.apache.org/repos/asf?p=incubator- > >>>> cloudstack.git;a=shortlog;h=refs/heads/api_refactoring > >> > https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+API+ > >>>> refactoring > >>>>> > >>>>> Regards. > >>>>> PS. will write a blog on it this weekend so folks can follow what's > >>>>> going > >> on :) > >>>>> PPS. maybe explain in a video
