Allon Mureinik has uploaded a new change for review.

Change subject: core: sorted getJsonDiskDescription()
......................................................................

core: sorted getJsonDiskDescription()

ImagesHandler.getJsonDiskDescription() used a HashMap to serialize a
Disk's Alias and Description. Since the order of iteration of a HashMap
cannot be trusted, it made the method's behavior unpredictable in
different environments, specifically breaking the unit tests on the
default JDK8 supplied with Fedora 21.

This patch replaces the internal implementation from using a HashMap to
use a TreeMap, ensuring the keys are iterated by order, and thus
producing a reliable output:
{"DiskAlias":"some_alias","DiskDescription":"some_description}. The unit
tests have been fixed accordingly, making them agnostic to JDK upgrades.

Change-Id: Ic8496858a2cb5715ab9f84b8ae77f62b3e6097d4
Bug-Url: https://bugzilla.redhat.com/1180887
Signed-off-by: Allon Mureinik <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
2 files changed, 5 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/36762/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
index 15cd687..e74df35 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
@@ -11,6 +11,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
 
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.bll.context.CompensationContext;
@@ -850,7 +851,7 @@
      * UpdateVmDiskCommand should be changed accordingly.
      */
     public static String getJsonDiskDescription(String diskAlias, String 
diskDescription) throws IOException {
-        Map<String, Object> description = new HashMap<>();
+        Map<String, Object> description = new TreeMap<>();
         description.put(ImagesHandler.DISK_ALIAS, diskAlias);
         description.put(ImagesHandler.DISK_DESCRIPTION, diskDescription != 
null ? diskDescription : "");
         return JsonHelper.mapToJson(description, false);
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
index 6ae6d14..00983b2 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
@@ -125,20 +125,20 @@
         String jsonDescription = null;
         jsonDescription = ImagesHandler.getJsonDiskDescription("DiskAlias", 
"DiskDescription");
         assertTrue("Should be map of disk alias and disk description",
-                
jsonDescription.equals("{\"DiskDescription\":\"DiskDescription\",\"DiskAlias\":\"DiskAlias\"}"));
+                
jsonDescription.equals("{\"DiskAlias\":\"DiskAlias\",\"DiskDescription\":\"DiskDescription\"}"));
     }
 
     @Test
     public void testJsonNullDiskDescription() throws IOException {
         String jsonDescription = 
ImagesHandler.getJsonDiskDescription("DiskAlias", null);
         assertTrue("Should be map of disk alias and disk description",
-                
jsonDescription.equals("{\"DiskDescription\":\"\",\"DiskAlias\":\"DiskAlias\"}"));
+                
jsonDescription.equals("{\"DiskAlias\":\"DiskAlias\",\"DiskDescription\":\"\"}"));
     }
 
     @Test
     public void testJsonEmptyDiskDescription() throws IOException {
         String jsonDescription = 
ImagesHandler.getJsonDiskDescription("DiskAlias", "");
         assertTrue("Should be map of disk alias and disk description",
-                
jsonDescription.equals("{\"DiskDescription\":\"\",\"DiskAlias\":\"DiskAlias\"}"));
+                
jsonDescription.equals("{\"DiskAlias\":\"DiskAlias\",\"DiskDescription\":\"\"}"));
     }
 }


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

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

Reply via email to