You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Antonio Fornie <an...@gmail.com> on 2014/03/03 10:01:41 UTC

Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

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

(Updated March 3, 2014, 9:01 a.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Added cmdeventtype ctxuserid ctxstarteventid ctxaccountid as expected parameters, and added Unit Test to check that none of the expected parameters detected so far is considered unknown.


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 (updated)
-----

  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 ff2b2ea 
  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/#review36049
-----------------------------------------------------------


Dario, the patch fails to apply on the latest master, again some issue with AutoScaleManagerImpl. Should be a quick fix from your side?

Applying: Dispatcher corrections, refactoring and tests.
error: patch failed: server/src/com/cloud/network/as/AutoScaleManagerImpl.java:448
error: server/src/com/cloud/network/as/AutoScaleManagerImpl.java: patch does not apply
Patch failed at 0001 Dispatcher corrections, refactoring and tests.

- Alena Prokharchyk


On March 3, 2014, 9:01 a.m., Antonio Fornie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17888/
> -----------------------------------------------------------
> 
> (Updated March 3, 2014, 9:01 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 ff2b2ea 
>   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
> 
>


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornié Casarrubios <an...@gmail.com>.
Fixed. Applied and mvn build works completely


2014-03-04 0:37 GMT+01:00 Alena Prokharchyk <al...@citrix.com>:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17888/
>
> Getting compilation error after applying the latest:
>
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile (default-compile) on project cloud-server: Compilation failure
> [ERROR] /Users/alena/repos/dr/cloudstack/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:[4202,33] error: cannot find symbol
> [ERROR] -> [Help 1]
>
>
> - Alena Prokharchyk
>
> On March 3rd, 2014, 11:26 p.m. UTC, Antonio Fornie wrote:
>   Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and
> Hugo Trippaers.
> By Antonio Fornie.
>
> *Updated March 3, 2014, 11:26 p.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/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/>
>

Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/#review36052
-----------------------------------------------------------


Getting compilation error after applying the latest:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile (default-compile) on project cloud-server: Compilation failure
[ERROR] /Users/alena/repos/dr/cloudstack/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:[4202,33] error: cannot find symbol
[ERROR] -> [Help 1]

- Alena Prokharchyk


On March 3, 2014, 11:26 p.m., Antonio Fornie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17888/
> -----------------------------------------------------------
> 
> (Updated March 3, 2014, 11:26 p.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/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
> 
>


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/#review36300
-----------------------------------------------------------


Fails to apply on the latest master :(

alena@Alenas-MacBook-Air: [master]~/repos/dr/cloudstack$ git am --signoff < ~/Desktop/0001-Dispatcher-corrections-refactoring-and-tests.patch
Applying: Dispatcher corrections, refactoring and tests
error: patch failed: server/src/com/cloud/api/ApiServer.java:881
error: server/src/com/cloud/api/ApiServer.java: patch does not apply
Patch failed at 0001 Dispatcher corrections, refactoring and tests
The copy of the patch that failed is found in:
   /Users/alena/repos/dr/cloudstack/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

- Alena Prokharchyk


On March 5, 2014, 10:35 a.m., Antonio Fornie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17888/
> -----------------------------------------------------------
> 
> (Updated March 5, 2014, 10:35 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 e869ddf 
>   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 
>   api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
>   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 5bdefe7 
>   server/src/com/cloud/api/ApiServer.java 3df599e 
>   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 37171f5 
>   server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
>   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
> 
>


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 7, 2014, 3:59 p.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Correct header for CommandCreationWorker


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 14df653 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  api/src/org/apache/cloudstack/api/BaseListCmd.java c1a4b4c 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml fd2f5fb 
  server/src/com/cloud/api/ApiDispatcher.java 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 7e29324 
  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/ParamUnpackWorker.java PRE-CREATION 
  server/src/com/cloud/api/dispatch/SpecificCmdValidationWorker.java PRE-CREATION 
  server/src/com/cloud/network/as/AutoScaleManagerImpl.java 208b4a4 
  server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 8486f06 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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/SpecificCmdValidationWorkerTest.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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 7, 2014, 3:45 p.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Problems updating previous patch


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 14df653 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  api/src/org/apache/cloudstack/api/BaseListCmd.java c1a4b4c 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml fd2f5fb 
  server/src/com/cloud/api/ApiDispatcher.java 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 7e29324 
  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/ParamUnpackWorker.java PRE-CREATION 
  server/src/com/cloud/api/dispatch/SpecificCmdValidationWorker.java PRE-CREATION 
  server/src/com/cloud/network/as/AutoScaleManagerImpl.java 208b4a4 
  server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 8486f06 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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/SpecificCmdValidationWorkerTest.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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 7, 2014, 3:41 p.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Resolve conflicts with master.


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 14df653 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  api/src/org/apache/cloudstack/api/BaseListCmd.java c1a4b4c 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml fd2f5fb 
  server/src/com/cloud/api/ApiDispatcher.java 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 7e29324 
  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/ParamUnpackWorker.java PRE-CREATION 
  server/src/com/cloud/api/dispatch/SpecificCmdValidationWorker.java PRE-CREATION 
  server/src/com/cloud/network/as/AutoScaleManagerImpl.java 208b4a4 
  server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 37171f5 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 7, 2014, 3:38 p.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Fix conflicts with last master


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 14df653 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  api/src/org/apache/cloudstack/api/BaseListCmd.java c1a4b4c 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml fd2f5fb 
  server/src/com/cloud/api/ApiDispatcher.java 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 7e29324 
  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/ParamUnpackWorker.java PRE-CREATION 
  server/src/com/cloud/api/dispatch/SpecificCmdValidationWorker.java PRE-CREATION 
  server/src/com/cloud/network/as/AutoScaleManagerImpl.java 208b4a4 
  server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 37171f5 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 6, 2014, 4:36 p.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Fix conflicts due to last master changes. Plus improving a couple of names.


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 14df653 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  api/src/org/apache/cloudstack/api/BaseListCmd.java c1a4b4c 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml fd2f5fb 
  server/src/com/cloud/api/ApiDispatcher.java 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 7e29324 
  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/ParamUnpackWorker.java PRE-CREATION 
  server/src/com/cloud/api/dispatch/SpecificCmdValidationWorker.java PRE-CREATION 
  server/src/com/cloud/network/as/AutoScaleManagerImpl.java 208b4a4 
  server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 37171f5 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 5, 2014, 10:35 a.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

CallContext.current().setEventDisplayEnabled(cmd.isDisplayResourceEnabled());
should be called after the dispatch chain


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 7b7f9ca 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  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 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  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 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 3df599e 
  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 37171f5 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 5, 2014, 10:29 a.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

New BaseCmd#getActualCommandName plus resolving new conflict with recent commits


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 7b7f9ca 
  api/src/org/apache/cloudstack/api/BaseCmd.java e869ddf 
  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 
  api/test/org/apache/cloudstack/api/BaseCmdTest.java PRE-CREATION 
  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 5bdefe7 
  server/src/com/cloud/api/ApiServer.java 3df599e 
  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 37171f5 
  server/src/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java a85c052 
  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Alena Prokharchyk <Al...@citrix.com>.
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"
<an...@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
><al...@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/>
>>


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornié Casarrubios <an...@gmail.com>.
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 <al...@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/>
>

Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
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.


Changes
-------

Fix compilation problem


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 (updated)
-----

  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


Re: Review Request 17888: Dispatcher corrections, refactoring and tests. Corrects problems from previous attempt

Posted by Antonio Fornie <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17888/
-----------------------------------------------------------

(Updated March 3, 2014, 11:26 p.m.)


Review request for cloudstack, Alena Prokharchyk, daan Hoogland, and Hugo Trippaers.


Changes
-------

Resolved conflicts with latest master


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 (updated)
-----

  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/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