Alena, I saw it, at first I thought it would be a problem in a certain cmd
and then I saw it's the same for all of them. Actually Cmd#getCommandName()
should give us what we want here, the command name, right? Why are we
returning the cmdNameResponse instead?

On top of that, if we continue this way (getting it from the annotation as
you propose) then we would end up having N places with the same code smell
(a couple of lines getting the actual cmd name from the annotation), so
instead I should create a new method in the BaseCmd that does this, so that
these clients (like my code) only have to do: cmd.getActualCmdName()

...but then, have you got a suggestion for how to call this method
considering that we already have a method getCommandName?




2014-03-04 1:44 GMT+01:00 Alena Prokharchyk <alena.prokharc...@citrix.com>:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17888/
>
> Antonio, when you log the WARN about incorrect param name, can you please 
> past the actual name of the command? Right now you log the response object 
> name instead:
>
> 2014-03-03 16:41:57,806 WARN  [c.c.a.d.ParamGenericValidationWorker] 
> (1574968208@qtp-585372613-2:ctx-0261a262 ctx-9867c49e) Received unknown 
> parameters for command listregionsresponse. Unknown parameters : listall
> 2014-03-03 16:41:57,843 WARN  [c.c.a.d.ParamGenericValidationWorker] 
> (589128916@qtp-585372613-5:ctx-80f1e7ec ctx-8d796be3) Received unknown 
> parameters for command listprojectsresponse. Unknown parameters : accountid
>
>
> You can get the command name from @APICommand annotation
>
>
> - Alena Prokharchyk
>
> On March 4th, 2014, 12:18 a.m. UTC, Antonio Fornie wrote:
>   Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and
> Hugo Trippaers.
> By Antonio Fornie.
>
> *Updated March 4, 2014, 12:18 a.m.*
>  *Repository: * cloudstack-git
> Description
>
> Dispatcher corrections, refactoring and tests. Corrects problems from 
> previous attempts that were reverted by Alena. Most of the changes are the 
> same, but this one is not creating conflicts of Map types for Aync Commands 
> or for parameters as Lists or Maps.
>
>   Testing
>
> Full build and test plus manually testing many features. Also including 
> CreateTagsCommand that failed in previous commit.
>
> All unit and integration tests.
>
> Test CS Web UI with the browser going through several use cases.
>
> Also use the CS API by sending HTTP requests generated manually including 
> requests for Async Commands with Map parameters and during these tests apart 
> fromtesting correct functionality I also debugged to check that Maps created 
> correctly where they should but also that in the cases where the async 
> command must be persisted and later on retrieved and deserialized by gson 
> everything works ok and does what and where is expected. An example based on 
> the comment by 
> Alena:http://localhost:8096/client/api?command=createTags&resourceids=ids&resourcetype=type&tags[0].key=region&tags[0].value=canada
> Also other examples 
> likehttp://localhost:8096/client/api?command=createSecondaryStagingStore&url=httpbla&details[0].key=region&details[0].value=canada&details[1].key=element&details[1].value=fire
>
>   Diffs
>
>    - api/src/org/apache/cloudstack/api/ApiConstants.java (7b7f9ca)
>    - api/src/org/apache/cloudstack/api/BaseCmd.java (0e83cee)
>    - api/src/org/apache/cloudstack/api/BaseListCmd.java (c1a4b4c)
>    - api/src/org/apache/cloudstack/api/command/admin/user/GetUserCmd.java
>    (b2c6734)
>    - api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java
>    (cf5d355)
>    - 
> api/src/org/apache/cloudstack/api/command/user/autoscale/CreateAutoScaleVmProfileCmd.java
>    (570e018)
>    - 
> server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml
>    (fd2f5fb)
>    - server/src/com/cloud/api/ApiAsyncJobDispatcher.java (f037f2e)
>    - server/src/com/cloud/api/ApiDispatcher.java (ed95c72)
>    - server/src/com/cloud/api/ApiServer.java (25792fb)
>    - server/src/com/cloud/api/ApiServlet.java (46f7eba)
>    - server/src/com/cloud/api/dispatch/CommandCreationWorker.java
>    (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/DispatchChain.java (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/DispatchChainFactory.java
>    (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/DispatchTask.java (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/DispatchWorker.java (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/ParamGenericValidationWorker.java
>    (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/ParamProcessWorker.java
>    (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/ParamSemanticValidationWorker.java
>    (PRE-CREATION)
>    - server/src/com/cloud/api/dispatch/ParamUnpackWorker.java
>    (PRE-CREATION)
>    - server/src/com/cloud/network/as/AutoScaleManagerImpl.java (208b4a4)
>    - 
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
>    (8404cab)
>    - server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java
>    (183a13a)
>    - server/test/com/cloud/api/ApiDispatcherTest.java (7314a57)
>    - server/test/com/cloud/api/dispatch/CommandCreationWorkerTest.java
>    (PRE-CREATION)
>    - server/test/com/cloud/api/dispatch/DispatchChainFactoryTest.java
>    (PRE-CREATION)
>    - server/test/com/cloud/api/dispatch/ParamGenericValidationWorkerTest.java
>    (PRE-CREATION)
>    - server/test/com/cloud/api/dispatch/ParamProcessWorkerTest.java
>    (PRE-CREATION)
>    - server/test/com/cloud/api/dispatch/ParamSemanticValidationWorkerTest.java
>    (PRE-CREATION)
>
> View Diff <https://reviews.apache.org/r/17888/diff/>
>

Reply via email to