Some recent updates: - Make over the wire apis params validation/translation as before thereby making changes status quo with master, only enforce uuid param validation for apis introduce since 3.x - Annotate name field in @APICommand for all cmd classes, now we can get rid of apiname, cmd class pkg name mappings from *command.properties.in file - Fixed apidocs, build
Regards. ________________________________________ From: Rohit Yadav Sent: Friday, December 28, 2012 1:18 AM To: cloudstack-dev@incubator.apache.org Subject: Re: API Updates: Tracking progress, changes On 27-Dec-2012, at 7:32 AM, David Nalley <da...@gnsa.us> wrote: > On Mon, Dec 24, 2012 at 10:09 PM, Rohit Yadav <rohit.ya...@citrix.com> wrote: >> Hi Alex, >> >> We got rid of IdentityMapper and enforced the translation of over the wire >> UUID string to internal long int ID, so we know if internal IDs are used in >> any case starting 4.1 it will break any client, script that are based off >> internal ID and not UUID strings. Few folks on the ML have expressed >> concerns, but we need this change and to enforce that we won't have any >> backward compatibility for over the wire api params to accept internal IDs. >> > > > I read this, and am left more confused than before. > There have been assurances regarding the refactor that no API changes > have occurred, but the above seems to contradict that. > I'll apologize for being daft (and for not going in and looking at the > changes) but can you tell me: > 1. what the changes to the API are? Changes to the API: None, for any over the wire request no params have been added or removed, nor any api cmd/response was added or removed. Enforcement on the APIs: All id params should be UUID strings. This should have been done since 3.x, but the changes listed below enforce that. Changes in API Server, OTW param processing etc: 0. Moving classes to org.apache.cloudstack.* package name, this does not change anything just imports in depending classes. 1. In all cmd classes: Annotate @Parameter to have correct type, entityType, collectionType, remove @IdentityMapper annotation. This changes interfaces, processing of those annotations and fix any caller. Therefore this change is refactoring [1]. Any @Parameter annotation with CommandType UUID is considered a case where OTW param string needs to be translated to an InternalId. In CloudStack we use internal db ids and not uuids, implemented a getUuid() in EntityManagerImpl. 2. List apis and response view optimizations by Min, optimizations in this case are considered refactoring as well [1] [2]. 3. Deduplication of code by moving them into methods, this does not change any functionality either, just refactoring. 4. ACL adapter and adapter interface definitions, work by Prachi. Separate out ACL and DB/Range validation code, define interfaces for the adapter, move implementations as plugin. Again, refactoring. 5. Rename @Implementation annotation to @APICommand, introduce a new name field. This name field can tightly couple API name to the cmd class, right now this info is insecurely coupled in commands.properties file where the mapping of apiname=<pkg name>.cmdclass:mask is defined. In future we would only have apiname and role masks which will constitute all whitelisted apis i.e. if api is not declared in commands.properties file, it's blacklisted. This would also enforce uniqueness of api command name. > 2. Why 'we need this change'? Why: 0. Refactoring was needed, as explained above. 1. There was a ticket on JIRA (cannot recall the id) to move classes under org.apache.cloudstack pkg name. 2. Move out policy from CloudStack as plugin so orgs can impl. their own role based and ACL etc. 3. Remove bloat, optimize OTW commands, esp. the responses. 4. Make it easier for contributor to write plugin, apis. 5. Remove code by refactoring acl, db validation code in one place using annotations which is spread across api layer (cloud-api, parts of cloud-server) and service layer (all the managers and managerimpls). > > Breaking backward compatibility is a huge deal IMO. Even changes that > technically didn't break compatibility but were different than folks > expectations (when we introduced UUID) earned us much wailing and > gnashing of teeth. Breaking other people's external tools that > interact with CloudStack is something that shouldn't be done without > much deliberation. :( I've to be the bad guy here, but we need to enforce that params are queried by uuids. Okay I'll ask community for a vote on it, if we want to keep things ambiguous and expose internal db ids to outside world the fix would be a simple if-else block to allow both kinds of ids. Regards. [1] http://martinfowler.com/bliki/refactoring.html [2] http://refactoring.com
