static nat capabilities parsing cleanup and testing

Daan Hoogland · Daan Hoogland · commit 6f970c6ff91c · 2013-09-26T15:18:52.000+02:00
diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -3924,42 +3924,27 @@ void validateSourceNatServiceCapablities(Map<Capability, String> sourceNatServic
 
     void validateStaticNatServiceCapablities(Map<Capability, String> staticNatServiceCapabilityMap) {
         if (staticNatServiceCapabilityMap != null && !staticNatServiceCapabilityMap.isEmpty()) {
-            if (staticNatServiceCapabilityMap.keySet().size() > 2) {
-                throw new InvalidParameterValueException("Only " + Capability.ElasticIp.getName() + " and "
-                        + Capability.AssociatePublicIP.getName()
-                        + " capabilitiy can be sepcified for static nat service");
-            }
-
             boolean eipEnabled = false;
-            boolean eipDisabled = false;
             boolean associatePublicIP = true;
             for (Capability capability : staticNatServiceCapabilityMap.keySet()) {
-                String value = staticNatServiceCapabilityMap.get(capability);
+                String value = staticNatServiceCapabilityMap.get(capability).toLowerCase();
+                if (! (value.contains("true") ^ value.contains("false"))) {
+                    throw new InvalidParameterValueException("Unknown specified value (" + value + ") for "
+                            + capability);
+                }
                 if (capability == Capability.ElasticIp) {
                     eipEnabled = value.contains("true");
-                    eipDisabled = value.contains("false");
-                    if (!eipEnabled && !eipDisabled) {
-                        throw new InvalidParameterValueException("Unknown specified value for "
-                                + Capability.ElasticIp.getName());
-                    }
                 } else if (capability == Capability.AssociatePublicIP) {
-                    if (value.contains("true")) {
-                        associatePublicIP = true;
-                    } else if (value.contains("false")) {
-                        associatePublicIP = false;
-                    } else {
-                        throw new InvalidParameterValueException("Unknown specified value for "
-                                + Capability.AssociatePublicIP.getName());
-                    }
+                    associatePublicIP = value.contains("true");
                 } else {
                     throw new InvalidParameterValueException("Only " + Capability.ElasticIp.getName() + " and "
                             + Capability.AssociatePublicIP.getName()
                             + " capabilitiy can be sepcified for static nat service");
                 }
-                if (eipDisabled && associatePublicIP) {
-                    throw new InvalidParameterValueException("Capability " + Capability.AssociatePublicIP.getName()
-                            + " can only be set when capability " + Capability.ElasticIp.getName() + " is true");
-                }
+            }
+            if ((! eipEnabled) && associatePublicIP) {
+                throw new InvalidParameterValueException("Capability " + Capability.AssociatePublicIP.getName()
+                        + " can only be set when capability " + Capability.ElasticIp.getName() + " is true");
             }
         }
     }
diff --git a/server/test/com/cloud/configuration/ConfigurationManagerTest.java b/server/test/com/cloud/configuration/ConfigurationManagerTest.java
@@ -27,7 +27,9 @@
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.UUID;
 
 import junit.framework.Assert;
@@ -53,7 +55,9 @@
 import com.cloud.dc.dao.AccountVlanMapDao;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.dc.dao.VlanDao;
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.network.IpAddressManager;
+import com.cloud.network.Network.Capability;
 import com.cloud.network.dao.FirewallRulesDao;
 import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
@@ -415,6 +419,70 @@ void runReleaseNonDedicatedPublicIpRange() throws Exception {
         }
     }
 
+    @Test
+    public void validateEmptyStaticNatServiceCapablitiesTest() {
+        Map<Capability, String> staticNatServiceCapabilityMap = new HashMap<Capability, String>();
+
+        configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
+    }
+
+    @Test
+    public void validateInvalidStaticNatServiceCapablitiesTest() {
+        Map<Capability, String> staticNatServiceCapabilityMap = new HashMap<Capability, String>();
+        staticNatServiceCapabilityMap.put(Capability.AssociatePublicIP, "Frue and Talse");
+
+        boolean caught = false;
+        try {
+            configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
+        }
+        catch (InvalidParameterValueException e) {
+            Assert.assertTrue(e.getMessage(),e.getMessage().contains("(frue and talse)"));
+            caught = true;
+        }
+        Assert.assertTrue("should not be accepted",caught);
+    }
+
+    @Test
+    public void validateTTStaticNatServiceCapablitiesTest() {
+        Map<Capability, String> staticNatServiceCapabilityMap = new HashMap<Capability, String>();
+        staticNatServiceCapabilityMap.put(Capability.AssociatePublicIP, "true and Talse");
+        staticNatServiceCapabilityMap.put(Capability.ElasticIp, "True");
+
+        configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
+    }
+    @Test
+    public void validateFTStaticNatServiceCapablitiesTest() {
+        Map<Capability, String> staticNatServiceCapabilityMap = new HashMap<Capability, String>();
+        staticNatServiceCapabilityMap.put(Capability.AssociatePublicIP, "false");
+        staticNatServiceCapabilityMap.put(Capability.ElasticIp, "True");
+
+        configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
+    }
+    @Test
+    public void validateTFStaticNatServiceCapablitiesTest() {
+        Map<Capability, String> staticNatServiceCapabilityMap = new HashMap<Capability, String>();
+        staticNatServiceCapabilityMap.put(Capability.AssociatePublicIP, "true and Talse");
+        staticNatServiceCapabilityMap.put(Capability.ElasticIp, "false");
+
+        boolean caught = false;
+        try {
+            configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
+        }
+        catch (InvalidParameterValueException e) {
+            Assert.assertTrue(e.getMessage(),e.getMessage().contains("Capability " + Capability.AssociatePublicIP.getName()
+                        + " can only be set when capability " + Capability.ElasticIp.getName() + " is true"));
+            caught = true;
+        }
+        Assert.assertTrue("should not be accepted",caught);
+    }
+    @Test
+    public void validateFFStaticNatServiceCapablitiesTest() {
+        Map<Capability, String> staticNatServiceCapabilityMap = new HashMap<Capability, String>();
+        staticNatServiceCapabilityMap.put(Capability.AssociatePublicIP, "false");
+        staticNatServiceCapabilityMap.put(Capability.ElasticIp, "False");
+
+        configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
+    }
 
     public class DedicatePublicIpRangeCmdExtn extends DedicatePublicIpRangeCmd {
         @Override
