cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohit Yadav <rohityada...@gmail.com>
Subject Re: [PROPOSAL][QUESTION] Map parameters in API Commands
Date Wed, 12 Feb 2014 22:32:33 GMT
On Wed, Feb 12, 2014 at 9:52 PM, Antonio ForniƩ Casarrubios
<antonio.fornie@gmail.com> wrote:
> Hi Rohit,
>
> I didn't mean changing the format of the HTTP request, but only changing the
> intermediate format in which we keep it in the property of the Command
> class. I mentioned the format in the request just to explain what I meant.
>
> My proposal is to leave the request format as it is, but then when the
> method "apiDispatcher#setFieldValue" parses the map and assign it to the
> property, do it in a normal way: which is a Map<String, String> instead of a
> Map<String, Map<String, Object>> as it is now. And then, our getter methods
> (like CreateTagsCommand#GetTag) will be just a normal getter that doesn't
> need to transform the structure on the fly.

Cool, let's request the present API layer maintainer(s) and other
folks in the community to comment.

Regards.

>
> Thanks, cheers
> antonio
>
>
> 2014-02-11 17:38 GMT+01:00 Rohit Yadav <rohityadav89@gmail.com>:
>
>> Hi Antonio,
>>
>> On Tue, Feb 11, 2014 at 9:57 PM, Antonio ForniƩ Casarrubios
>> <antonio.fornie@gmail.com> wrote:
>> > Hi all,
>> >
>> > When invoking a CS API command that has parameters of type Map, the
>> > request
>> > will be something like this:
>> >
>> > URL/api?command=createTags&tags[0].key=region&tags[0].value=canada&tags[1].key=name&tags[1].value=bob
>> >
>> > in order to send a Map with the pairs:
>> >
>> > tags{
>> >    region : "canada",
>> >    name : "bob"
>> > }
>> >
>> > Then in the server side the parameters go through several stages (IMHO
>> > too
>> > many), and have different formats. At some point
>> >
>> > apiDispatcher#setFieldValue
>> >
>> > will assign the value to the command property (CreateTagsCmd#tag in the
>> > example) in a VERY strange way:
>> >
>> > CreateTagsCmd#tag = {
>> >    0 : {
>> >       "key" : "region",
>> >       "value" : "canada"
>> >    },
>> >    1 : {
>> >       "key" : "name",
>> >       "value" : "bob"
>> >    }
>> > }
>> >
>> > This is true for several Cmd classes. And the funny thing is they
>> > usually
>> > provide a public getter method to get the Map in an already "normalized"
>> > structure. The problem is we have this method again a again in each of
>> > these commands, only with different name depending on what property the
>> > get, and the body is almost copy and pasted. so my next refactoring
>> > would
>> > be to have a generic method only once in BaseCmd so that all subclasses
>> > can
>> > reuse it for their Map getters. Pretty obvious, but...
>>
>> This is a well know issue and is such a pain, both for the users of
>> the API to create this API and the programmer who have to put hack at
>> the backend to extract the map.
>>
>> > Is it really necessary to have this strange format? Wouldn't it be much
>> > better to just store it in a more normal way from the beginning, and
>> > have
>> > the getters just standard getters? Does it have any use to have those
>> > Maps
>> > of Maps?
>>
>> Changing the API will break many client so no one attempted it for
>> keeping backward-compatibility I think.
>>
>> The HTTP RFC states that if same keys are sent in param they must be
>> received as an array. For example, /api?q=1&q=2&q=3 should received q
>> = [1,2,3] which is what we're not doing.
>>
>> We should do that and this way we can capture maps using keys and
>> values in order, so for example,
>> /api?q.key1=value1&q.key2=value2&q.key1=value3&q.key2=value4 should be
>> received as as array of maps: [{key1: value1, key2: value2},
>> {key3:value3, key4: value4}] etc.
>>
>> I think it does not have to be maps of maps, but since our API is
>> query based these ugly hacks were invented. We should definitely get
>> rid of them, and perhaps work on the RESTful API layer, cloud-engine
>> and other good stuff we were talking about more than a year ago and
>> deprecate the present query API over next few years. Thoughts, flames?
>>
>> Regards.
>>
>> >
>> > Thanks. Cheers
>> > Antonio Fornie
>> > Schuberg Philis - MCE
>
>

Mime
View raw message