Allon Mureinik has uploaded a new change for review.

Change subject: core: Don't clone Guids in VDS.clone()
......................................................................

core: Don't clone Guids in VDS.clone()

Since Guids are immutable, there is no need for VDS.clone() to create
new Guid instances, especially not by the expensive string-parsing
method.

This patch changes the new instance created by VDS.clone() to simply
hold references to the old one's Guid, thus improving performance.
As a nice side bonus, this will allow cloning of VDS instances that have
null VDS group or storage pool IDs.

This patch also contains a test to assert this functionality.

Change-Id: I0f7130f76525dda601ac0bc7322fbf96c961c5a6
Signed-off-by: Allon Mureinik <[email protected]>
---
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
A 
backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/VDSTest.java
2 files changed, 39 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/33/16433/1

diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
index ca87adf..7e4d3b4 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
@@ -191,10 +191,10 @@
 
     public VDS clone() {
         VDS vds =
-                new 
VDS(Guid.createGuidFromStringDefaultEmpty(getVdsGroupId().toString()),
+                new VDS(getVdsGroupId(),
                         getVdsGroupName(),
                         getVdsGroupDescription(),
-                        
Guid.createGuidFromStringDefaultEmpty(getId().toString()),
+                        getId(),
                         getName(),
                         getManagementIp(),
                         getHostName(),
diff --git 
a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/VDSTest.java
 
b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/VDSTest.java
new file mode 100644
index 0000000..84bebae
--- /dev/null
+++ 
b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/VDSTest.java
@@ -0,0 +1,37 @@
+package org.ovirt.engine.core.common.businessentities;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.experimental.theories.DataPoints;
+import org.junit.experimental.theories.Theories;
+import org.junit.experimental.theories.Theory;
+import org.junit.runner.RunWith;
+import org.ovirt.engine.core.compat.Guid;
+
+/**
+ * A test case for the {@link VDS} class.
+ */
+@RunWith(Theories.class)
+public class VDSTest {
+
+    @DataPoints
+    public static VDS[] data() {
+        VDS vds1 = new VDS();
+
+        VDS vds2 = new VDS();
+        vds2.setId(Guid.newGuid());
+
+        VDS vds3 = new VDS();
+        vds3.setId(Guid.newGuid());
+        vds3.setVdsGroupId(Guid.newGuid());
+
+        return new VDS[] { vds1, vds2, vds3 };
+    }
+
+    @Theory
+    public void testClone(VDS vds) {
+        VDS cloned = vds.clone();
+        assertEquals("clones not equal", vds, cloned);
+        assertEquals("clones do not have equal hashCodes", vds.hashCode(), 
cloned.hashCode());
+    }
+}


-- 
To view, visit http://gerrit.ovirt.org/16433
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0f7130f76525dda601ac0bc7322fbf96c961c5a6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to