Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Content-Type: multipart/alternative;
 boundary="===============7200562249159896055=="
MIME-Version: 1.0
Subject: Re: Review Request 17888: Dispatcher corrections,
 refactoring and tests. Corrects problems from previous attempt
From: "Alena Prokharchyk" <alena.prokharchyk@citrix.com>
To: "daan Hoogland" <daan.hoogland@gmail.com>,
 "Hugo Trippaers" <htrippaers@schubergphilis.com>,
 "Alena Prokharchyk" <alena.prokharchyk@citrix.com>
Cc: "Antonio Fornie" <antonio.fornie@gmail.com>,
 "cloudstack" <dev@cloudstack.apache.org>
Date: Tue, 04 Mar 2014 00:44:14 -0000
Message-ID: <20140304004414.10089.91741@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Alena Prokharchyk" <noreply@reviews.apache.org>
References: <20140304001825.10116.14329@reviews.apache.org>
In-Reply-To: <20140304001825.10116.14329@reviews.apache.org>
Reply-To: "Alena Prokharchyk" <alena.prokharchyk@citrix.com>

--===============7200562249159896055==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/#review36068
-----------------------------------------------------------


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 4, 2014, 12:18 a.m., Antonio Fornie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17888/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 12:18 a.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.
> 
> 
> 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.
> 
> 
> 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 
> 
> Diff: https://reviews.apache.org/r/17888/diff/
> 
> 
> 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 like
> http://localhost:8096/client/api?command=createSecondaryStagingStore&url=httpbla&details[0].key=region&details[0].value=canada&details[1].key=element&details[1].value=fire
> 
> 
> Thanks,
> 
> Antonio Fornie
> 
>


--===============7200562249159896055==--