Antonio, I don¹t have a history about why we return cmdNameResponse instead of returning actual command name. We might change the method name in the future.
And yes, please go ahead and create a new method in BaseCmd. -Alena. On 3/4/14, 1:02 AM, "Antonio Fornié Casarrubios" <antonio.for...@gmail.com> wrote: >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/CreateAutoScaleV >>mProfileCmd.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.ja >>va >> (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/> >>