You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Wilder Rodrigues <wr...@schubergphilis.com> on 2014/02/13 13:06:39 UTC
Review Request 18072: Fixing 11 scary and 1 of concern issues found by
findBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils
class.l
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18072/
-----------------------------------------------------------
Review request for cloudstack, daan Hoogland and Hugo Trippaers.
Repository: cloudstack-git
Description
-------
Fixing 11 scary and 1 of concern issues found by FindBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils class.
Diffs
-----
plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java 99d0a12
plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java PRE-CREATION
utils/src/com/cloud/utils/UuidUtils.java 7831bea
utils/test/com/cloud/utils/UuidUtilsTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/18072/diff/
Testing
-------
Build successfully and added 13 unit tests
Thanks,
Wilder Rodrigues
Re: Review Request 18072: Fixing 11 scary and 1 of concern issues found by
findBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils
class.l
Posted by Wilder Rodrigues <wr...@schubergphilis.com>.
> On Feb. 14, 2014, 9:08 a.m., Hugo Trippaers wrote:
> > plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java, line 63
> > <https://reviews.apache.org/r/18072/diff/1/?file=484234#file484234line63>
> >
> > Why recreate the map?
After explained how this method could be used, we agreed to remove the map recreation.
> On Feb. 14, 2014, 9:08 a.m., Hugo Trippaers wrote:
> > utils/src/com/cloud/utils/UuidUtils.java, line 27
> > <https://reviews.apache.org/r/18072/diff/1/?file=484236#file484236line27>
> >
> > UUID.fromString? And use this method in your other patch where you verify UUIDs?
UUID.fromString() does not validate an UUID properly. I tried, for example, informing less digits in the UUID, where 12 were expected,
and the UUID.fromstring() did not thrown the exception as expected. However, if you try UUID.fromString("aaa") it breaks, but if you try
UUID.fromString("3dd4fa6e-2899-4429-b818-d34fe8df5") it doesn't (the last portion should have 12, instead of 9 digits).
- Wilder
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18072/#review34466
-----------------------------------------------------------
On Feb. 13, 2014, 12:06 p.m., Wilder Rodrigues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18072/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2014, 12:06 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixing 11 scary and 1 of concern issues found by FindBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils class.
>
>
> Diffs
> -----
>
> plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java 99d0a12
> plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java PRE-CREATION
> utils/src/com/cloud/utils/UuidUtils.java 7831bea
> utils/test/com/cloud/utils/UuidUtilsTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18072/diff/
>
>
> Testing
> -------
>
> Build successfully and added 13 unit tests
>
>
> Thanks,
>
> Wilder Rodrigues
>
>
Re: Review Request 18072: Fixing 11 scary and 1 of concern issues found by
findBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils
class.l
Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18072/#review34466
-----------------------------------------------------------
plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java
<https://reviews.apache.org/r/18072/#comment64583>
Why recreate the map?
utils/src/com/cloud/utils/UuidUtils.java
<https://reviews.apache.org/r/18072/#comment64582>
UUID.fromString? And use this method in your other patch where you verify UUIDs?
- Hugo Trippaers
On Feb. 13, 2014, 12:06 p.m., Wilder Rodrigues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18072/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2014, 12:06 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixing 11 scary and 1 of concern issues found by FindBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils class.
>
>
> Diffs
> -----
>
> plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java 99d0a12
> plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java PRE-CREATION
> utils/src/com/cloud/utils/UuidUtils.java 7831bea
> utils/test/com/cloud/utils/UuidUtilsTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18072/diff/
>
>
> Testing
> -------
>
> Build successfully and added 13 unit tests
>
>
> Thanks,
>
> Wilder Rodrigues
>
>
Re: Review Request 18072: Fixing 11 scary and 1 of concern issues found by
findBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils
class.l
Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18072/#review34493
-----------------------------------------------------------
Ship it!
commit 643301e031817cc4495fff5d37bd4970ca578c9c
Author: wrodrigues <wr...@schubergphilis.com>
Date: Tue Feb 11 11:15:55 2014 +0100
Fixes on Contrail and Mon InMemory plugins; adding comments about the changes.
Signed-off-by: Hugo Trippaers <ht...@schubergphilis.com>
- Hugo Trippaers
On Feb. 14, 2014, 2:43 p.m., Wilder Rodrigues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18072/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2014, 2:43 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixing 11 scary and 1 of concern issues found by FindBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils class.
>
>
> Diffs
> -----
>
> plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java 99d0a12
> plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java PRE-CREATION
> plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java b25de48
> plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java 1b048ed
> plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java 0ce22ad
> plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java 4d0218c
> plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java b0505b1
> plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java f85beb6
> plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java b1b5ae1
> utils/src/com/cloud/utils/UuidUtils.java 7831bea
> utils/test/com/cloud/utils/UuidUtilsTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18072/diff/
>
>
> Testing
> -------
>
> Build successfully and added 13 unit tests
>
>
> Thanks,
>
> Wilder Rodrigues
>
>
Re: Review Request 18072: Fixing 11 scary and 1 of concern issues found by
findBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils
class.l
Posted by Wilder Rodrigues <wr...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18072/
-----------------------------------------------------------
(Updated Feb. 14, 2014, 2:43 p.m.)
Review request for cloudstack, daan Hoogland and Hugo Trippaers.
Changes
-------
Removed 2 unit tests because the UUID validate method was moved to UuidUtils class.
Repository: cloudstack-git
Description
-------
Fixing 11 scary and 1 of concern issues found by FindBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils class.
Diffs (updated)
-----
plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java 99d0a12
plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java PRE-CREATION
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java b25de48
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java 1b048ed
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java 0ce22ad
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java 4d0218c
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java b0505b1
plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java f85beb6
plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java b1b5ae1
utils/src/com/cloud/utils/UuidUtils.java 7831bea
utils/test/com/cloud/utils/UuidUtilsTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/18072/diff/
Testing
-------
Build successfully and added 13 unit tests
Thanks,
Wilder Rodrigues
Re: Review Request 18072: Fixing 11 scary and 1 of concern issues found by
findBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils
class.l
Posted by Wilder Rodrigues <wr...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18072/
-----------------------------------------------------------
(Updated Feb. 14, 2014, 1:52 p.m.)
Review request for cloudstack, daan Hoogland and Hugo Trippaers.
Changes
-------
Squashing the contrail and UuidUtils changes; adding comments about the changes I did i the contrail plugin and also why the UUID.fromString() does not work properly; updating the unit tests
Repository: cloudstack-git
Description
-------
Fixing 11 scary and 1 of concern issues found by FindBugs. Adding 13 unit tests and also 1 validate method in the UuidUtils class.
Diffs (updated)
-----
plugins/event-bus/inmemory/src/org/apache/cloudstack/mom/inmemory/InMemoryEventBus.java 99d0a12
plugins/event-bus/inmemory/test/org/apache/cloudstack/mom/inmemory/InMemoryEventBusTest.java PRE-CREATION
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/management/ContrailManagerImpl.java b25de48
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ModelObject.java 1b048ed
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/ServiceInstanceModel.java 0ce22ad
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualMachineModel.java 4d0218c
plugins/network-elements/juniper-contrail/src/org/apache/cloudstack/network/contrail/model/VirtualNetworkModel.java b0505b1
plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualMachineModelTest.java f85beb6
plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/model/VirtualNetworkModelTest.java b1b5ae1
utils/src/com/cloud/utils/UuidUtils.java 7831bea
utils/test/com/cloud/utils/UuidUtilsTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/18072/diff/
Testing
-------
Build successfully and added 13 unit tests
Thanks,
Wilder Rodrigues