From b838436e58a6e7df89d960c7805edc3fd3ed91a4 Mon Sep 17 00:00:00 2001
From: Laszlo Hornyak <laszlo.hornyak@gmail.com>
Date: Fri, 24 Jan 2014 23:15:26 +0100
Subject: [PATCH] cleanup around seemingly impossible cast

- changed type parameters on details map in CreateSecondaryStagingStoreCmd - <String, String> was misleading since it can not work with a string value and it is never a string
- introducing the type parameters allowed some simplifications in getDetails()
- added unit test

Signed-off-by: Laszlo Hornyak <laszlo.hornyak@gmail.com>
Signed-off-by: Pierre-Luc Dion <pdion891@apache.org>
---
 .../CreateSecondaryStagingStoreCmd.java       | 13 +---
 .../CreateSecondaryStagingStoreCmdTest.java   | 59 +++++++++++++++++++
 2 files changed, 62 insertions(+), 10 deletions(-)
 create mode 100644 api/test/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmdTest.java

diff --git a/api/src/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmd.java b/api/src/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmd.java
index 39ec6098516..cfe4383578d 100644
--- a/api/src/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmd.java
@@ -18,9 +18,7 @@
  */
 package org.apache.cloudstack.api.command.admin.storage;
 
-import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.Map;
 
 import org.apache.log4j.Logger;
@@ -54,7 +52,7 @@ public class CreateSecondaryStagingStoreCmd extends BaseCmd {
     private Long zoneId;
 
     @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "the details for the staging store")
-    private Map<String, String> details;
+    private Map<String, ? extends Map<String, String>> details;
 
     @Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, required = false, description = "the scope of the staging store: zone only for now")
     private String scope;
@@ -78,13 +76,8 @@ public class CreateSecondaryStagingStoreCmd extends BaseCmd {
         Map<String, String> detailsMap = null;
         if (details != null && !details.isEmpty()) {
             detailsMap = new HashMap<String, String>();
-            Collection<?> props = details.values();
-            Iterator<?> iter = props.iterator();
-            while (iter.hasNext()) {
-                HashMap<String, String> detail = (HashMap<String, String>)iter.next();
-                String key = detail.get("key");
-                String value = detail.get("value");
-                detailsMap.put(key, value);
+            for (Map<String, String> detail : details.values()) {
+                detailsMap.put(detail.get("key"), detail.get("value"));
             }
         }
         return detailsMap;
diff --git a/api/test/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmdTest.java b/api/test/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmdTest.java
new file mode 100644
index 00000000000..f84437a6fae
--- /dev/null
+++ b/api/test/org/apache/cloudstack/api/command/admin/storage/CreateSecondaryStagingStoreCmdTest.java
@@ -0,0 +1,59 @@
+package org.apache.cloudstack.api.command.admin.storage;
+
+import java.lang.reflect.Field;
+import java.util.HashMap;
+import java.util.Map;
+
+import junit.framework.Assert;
+
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.junit.Test;
+
+public class CreateSecondaryStagingStoreCmdTest {
+
+    static void set(BaseCmd cmd, String fieldName, Object value)
+            throws IllegalArgumentException, IllegalAccessException {
+        for (Field field : cmd.getClass().getDeclaredFields()) {
+            Parameter parameter = field.getAnnotation(Parameter.class);
+            if (parameter != null && fieldName.equals(parameter.name())) {
+                field.setAccessible(true);
+                field.set(cmd, value);
+            }
+        }
+    }
+
+    @Test
+    public void getDetails() throws IllegalArgumentException,
+            IllegalAccessException {
+        CreateSecondaryStagingStoreCmd cmd = new CreateSecondaryStagingStoreCmd();
+        HashMap<String, Map<String, String>> details = new HashMap<String, Map<String, String>>();
+        HashMap<String, String> kv = new HashMap<String, String>();
+        kv.put("key", "TEST-KEY");
+        kv.put("value", "TEST-VALUE");
+        details.put("does not matter", kv);
+        set(cmd, "details", details);
+        Map<String, String> detailsMap = cmd.getDetails();
+        Assert.assertNotNull(detailsMap);
+        Assert.assertEquals(1, detailsMap.size());
+        Assert.assertTrue(detailsMap.containsKey("TEST-KEY"));
+        Assert.assertEquals("TEST-VALUE", detailsMap.get("TEST-KEY"));
+    }
+
+    @Test
+    public void getDetailsEmpty() throws IllegalArgumentException,
+            IllegalAccessException {
+        CreateSecondaryStagingStoreCmd cmd = new CreateSecondaryStagingStoreCmd();
+        set(cmd, "details", new HashMap<String, Map<String, String>>());
+        Assert.assertNull(cmd.getDetails());
+    }
+
+    @Test
+    public void getDetailsNull() throws IllegalArgumentException,
+            IllegalAccessException {
+        CreateSecondaryStagingStoreCmd cmd = new CreateSecondaryStagingStoreCmd();
+        set(cmd, "details", null);
+        Assert.assertNull(cmd.getDetails());
+    }
+
+}