Refactor getIntegerProperty and isPropertyEnabled methods into Tools

- Add getIntegerProperty and isPropertyEnabled methods which take default value as third parameter - Remove all duplicated code from RefactiveForwarding, GroupManager, FlowRuleManager, CoreManager, HostLocationProvider and ProxyArp Change-Id: Ifc93aa813acfdd4cbac0166497d7b526b08b2090

Refactor getIntegerProperty and isPropertyEnabled methods into Tools
- Add getIntegerProperty and isPropertyEnabled methods which take default value as third parameter - Remove all duplicated code from RefactiveForwarding, GroupManager, FlowRuleManager, CoreManager, HostLocationProvider and ProxyArp Change-Id: Ifc93aa813acfdd4cbac0166497d7b526b08b2090
Jian Li · Gerrit Code Review
Commit d9b5f55457f8a1cfe77ba467ef290d860c69b472 d9b5f554 1 parent 24431192
Showing 7 changed files with 98 additions and 139 deletions
apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java
apps/proxyarp/src/main/java/org/onosproject/proxyarp/ProxyArp.java
core/net/src/main/java/org/onosproject/core/impl/CoreManager.java
core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java
utils/misc/src/main/java/org/onlab/util/Tools.java
--- a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java
View file @d9b5f55
+++ b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java
View file @d9b5f55
--- a/apps/proxyarp/src/main/java/org/onosproject/proxyarp/ProxyArp.java
View file @d9b5f55
+++ b/apps/proxyarp/src/main/java/org/onosproject/proxyarp/ProxyArp.java
View file @d9b5f55
@@ -25,6 +25,7 @@ import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.ICMP6;
 import org.onlab.packet.IPv6;
+import org.onlab.util.Tools;
 import org.onosproject.cfg.ComponentConfigService;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
@@ -40,7 +41,6 @@ import org.slf4j.Logger;
 import java.util.Dictionary;
-import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.onlab.packet.Ethernet.TYPE_ARP;
 import static org.onlab.packet.Ethernet.TYPE_IPV6;
 import static org.onlab.packet.ICMP6.NEIGHBOR_ADVERTISEMENT;
@@ -173,7 +173,7 @@ public class ProxyArp {
         Dictionary<?, ?> properties = context.getProperties();
         Boolean flag;
-        flag = isPropertyEnabled(properties, "ipv6NeighborDiscovery");
+        flag = Tools.isPropertyEnabled(properties, "ipv6NeighborDiscovery");
         if (flag == null) {
             log.info("IPv6 Neighbor Discovery is not configured, " +
                              "using current value of {}", ipv6NeighborDiscovery);
@@ -185,26 +185,6 @@ public class ProxyArp {
     }
     /**
-     * Check property name is defined and set to true.
-     *
-     * @param properties   properties to be looked up
-     * @param propertyName the name of the property to look up
-     * @return value when the propertyName is defined or return null
-     */
-    private static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
-                                             String propertyName) {
-        Boolean value = null;
-        try {
-            String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? null : s.trim().equals("true");
-        } catch (ClassCastException e) {
-            // No propertyName defined.
-            value = null;
-        }
-        return value;
-    }
-
-    /**
      * Packet processor responsible for forwarding packets along their paths.
      */
     private class ProxyArpProcessor implements PacketProcessor {
--- a/core/net/src/main/java/org/onosproject/core/impl/CoreManager.java
View file @d9b5f55
+++ b/core/net/src/main/java/org/onosproject/core/impl/CoreManager.java
View file @d9b5f55
@@ -25,6 +25,7 @@ import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.metrics.MetricsService;
 import org.onlab.util.SharedExecutors;
+import org.onlab.util.Tools;
 import org.onosproject.app.ApplicationService;
 import org.onosproject.cfg.ComponentConfigService;
 import org.onosproject.core.ApplicationId;
@@ -48,9 +49,9 @@ import java.util.List;
 import java.util.Set;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.onosproject.security.AppGuard.checkPermission;
-import static org.onosproject.security.AppPermission.Type.*;
+import static org.onosproject.security.AppPermission.Type.APP_READ;
+import static org.onosproject.security.AppPermission.Type.APP_WRITE;
 /**
@@ -172,7 +173,7 @@ public class CoreManager implements CoreService {
     @Modified
     public void modified(ComponentContext context) {
         Dictionary<?, ?> properties = context.getProperties();
-        Integer poolSize = getIntegerProperty(properties, "sharedThreadPoolSize");
+        Integer poolSize = Tools.getIntegerProperty(properties, "sharedThreadPoolSize");
         if (poolSize != null && poolSize > 1) {
             sharedThreadPoolSize = poolSize;
@@ -181,7 +182,7 @@ public class CoreManager implements CoreService {
             log.warn("sharedThreadPoolSize must be greater than 1");
         }
-        Integer timeLimit = getIntegerProperty(properties, "maxEventTimeLimit");
+        Integer timeLimit = Tools.getIntegerProperty(properties, "maxEventTimeLimit");
         if (timeLimit != null && timeLimit > 1) {
             maxEventTimeLimit = timeLimit;
             eventDeliveryService.setDispatchTimeLimit(maxEventTimeLimit);
@@ -189,7 +190,7 @@ public class CoreManager implements CoreService {
             log.warn("maxEventTimeLimit must be greater than 1");
         }
-        Boolean performanceCheck = isPropertyEnabled(properties, "sharedThreadPerformanceCheck");
+        Boolean performanceCheck = Tools.isPropertyEnabled(properties, "sharedThreadPerformanceCheck");
         if (performanceCheck != null) {
             calculatePoolPerformance = performanceCheck;
             SharedExecutors.setCalculatePoolPerformance(calculatePoolPerformance, metricsService);
@@ -198,48 +199,4 @@ public class CoreManager implements CoreService {
         log.info("Settings: sharedThreadPoolSize={}, maxEventTimeLimit={}, calculatePoolPerformance={}",
                  sharedThreadPoolSize, maxEventTimeLimit, calculatePoolPerformance);
     }
-
-
-    /**
-     * Get Integer property from the propertyName
-     * Return null if propertyName is not found.
-     *
-     * @param properties   properties to be looked up
-     * @param propertyName the name of the property to look up
-     * @return value when the propertyName is defined or return null
-     */
-    private static Integer getIntegerProperty(Dictionary<?, ?> properties,
-                                              String propertyName) {
-        Integer value;
-        try {
-            String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? null : Integer.parseInt(s.trim());
-        } catch (NumberFormatException | ClassCastException e) {
-            value = null;
-        }
-        return value;
-    }
-
-    /**
-     * Check property name is defined and set to true.
-     *
-     * @param properties   properties to be looked up
-     * @param propertyName the name of the property to look up
-     * @return value when the propertyName is defined or return null
-     */
-    private static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
-                                             String propertyName) {
-        Boolean value = null;
-        try {
-            String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? null : s.trim().equals("true");
-        } catch (ClassCastException e) {
-            // No propertyName defined.
-            value = null;
-        }
-        return value;
-    }
-
-
-
 }
--- a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
View file @d9b5f55
+++ b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java
View file @d9b5f55
@@ -29,6 +29,7 @@ import org.apache.felix.scr.annotations.Property;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
+import org.onlab.util.Tools;
 import org.onosproject.cfg.ComponentConfigService;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
@@ -190,7 +191,7 @@ public class FlowRuleManager
         Dictionary<?, ?> properties = context.getProperties();
         Boolean flag;
-        flag = isPropertyEnabled(properties, "allowExtraneousRules");
+        flag = Tools.isPropertyEnabled(properties, "allowExtraneousRules");
         if (flag == null) {
             log.info("AllowExtraneousRules is not configured, " +
                     "using current value of {}", allowExtraneousRules);
@@ -200,7 +201,7 @@ public class FlowRuleManager
                     allowExtraneousRules ? "enabled" : "disabled");
         }
-        flag = isPropertyEnabled(properties, "purgeOnDisconnection");
+        flag = Tools.isPropertyEnabled(properties, "purgeOnDisconnection");
         if (flag == null) {
             log.info("PurgeOnDisconnection is not configured, " +
                     "using current value of {}", purgeOnDisconnection);
@@ -218,24 +219,6 @@ public class FlowRuleManager
         }
     }
-    /**
-     * Check property name is defined and set to true.
-     *
-     * @param properties   properties to be looked up
-     * @param propertyName the name of the property to look up
-     * @return value when the propertyName is defined or return null
-     */
-    private static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
-            String propertyName) {
-        try {
-            String s = (String) properties.get(propertyName);
-            return  isNullOrEmpty(s) ? null : s.trim().equals("true");
-        } catch (ClassCastException e) {
-            // No propertyName defined.
-            return null;
-        }
-    }
-
     @Override
     public int getFlowRuleCount() {
         checkPermission(FLOWRULE_READ);
--- a/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
View file @d9b5f55
+++ b/core/net/src/main/java/org/onosproject/net/group/impl/GroupManager.java
View file @d9b5f55
@@ -23,8 +23,8 @@ import org.apache.felix.scr.annotations.Property;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
+import org.onlab.util.Tools;
 import org.onosproject.cfg.ComponentConfigService;
-import org.onosproject.net.provider.AbstractListenerProviderRegistry;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.device.DeviceEvent;
@@ -45,6 +45,7 @@ import org.onosproject.net.group.GroupService;
 import org.onosproject.net.group.GroupStore;
 import org.onosproject.net.group.GroupStore.UpdateType;
 import org.onosproject.net.group.GroupStoreDelegate;
+import org.onosproject.net.provider.AbstractListenerProviderRegistry;
 import org.onosproject.net.provider.AbstractProviderService;
 import org.osgi.service.component.ComponentContext;
 import org.slf4j.Logger;
@@ -53,10 +54,10 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Dictionary;
-import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.GROUP_READ;
+import static org.onosproject.security.AppPermission.Type.GROUP_WRITE;
 import static org.slf4j.LoggerFactory.getLogger;
-import static org.onosproject.security.AppPermission.Type.*;
@@ -129,7 +130,7 @@ public class GroupManager
         Dictionary<?, ?> properties = context.getProperties();
         Boolean flag;
-        flag = isPropertyEnabled(properties, "purgeOnDisconnection");
+        flag = Tools.isPropertyEnabled(properties, "purgeOnDisconnection");
         if (flag == null) {
             log.info("PurgeOnDisconnection is not configured, " +
                     "using current value of {}", purgeOnDisconnection);
@@ -141,26 +142,6 @@ public class GroupManager
     }
     /**
-     * Check property name is defined and set to true.
-     *
-     * @param properties   properties to be looked up
-     * @param propertyName the name of the property to look up
-     * @return value when the propertyName is defined or return null
-     */
-    private static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
-            String propertyName) {
-        Boolean value = null;
-        try {
-            String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? null : s.trim().equals("true");
-        } catch (ClassCastException e) {
-            // No propertyName defined.
-            value = null;
-        }
-        return value;
-    }
-
-    /**
      * Create a group in the specified device with the provided parameters.
      *
      * @param groupDesc group creation parameters
--- a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java
View file @d9b5f55
+++ b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java
View file @d9b5f55
@@ -35,6 +35,7 @@ import org.onlab.packet.ndp.NeighborAdvertisement;
 import org.onlab.packet.ndp.NeighborSolicitation;
 import org.onlab.packet.ndp.RouterAdvertisement;
 import org.onlab.packet.ndp.RouterSolicitation;
+import org.onlab.util.Tools;
 import org.onosproject.cfg.ComponentConfigService;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
@@ -69,7 +70,6 @@ import java.util.Dictionary;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
-import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
 import static org.onlab.util.Tools.groupedThreads;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -228,7 +228,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
         Dictionary<?, ?> properties = context.getProperties();
         Boolean flag;
-        flag = isPropertyEnabled(properties, "hostRemovalEnabled");
+        flag = Tools.isPropertyEnabled(properties, "hostRemovalEnabled");
         if (flag == null) {
             log.info("Host removal on port/device down events is not configured, " +
                              "using current value of {}", hostRemovalEnabled);
@@ -238,7 +238,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
                      hostRemovalEnabled ? "enabled" : "disabled");
         }
-        flag = isPropertyEnabled(properties, "ipv6NeighborDiscovery");
+        flag = Tools.isPropertyEnabled(properties, "ipv6NeighborDiscovery");
         if (flag == null) {
             log.info("Using IPv6 Neighbor Discovery is not configured, " +
                              "using current value of {}", ipv6NeighborDiscovery);
@@ -248,7 +248,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
                      ipv6NeighborDiscovery ? "enabled" : "disabled");
         }
-        flag = isPropertyEnabled(properties, "requestInterceptsEnabled");
+        flag = Tools.isPropertyEnabled(properties, "requestInterceptsEnabled");
         if (flag == null) {
             log.info("Request intercepts is not configured, " +
                     "using current value of {}", requestInterceptsEnabled);
@@ -259,26 +259,6 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
         }
     }
-    /**
-     * Check property name is defined and set to true.
-     *
-     * @param properties   properties to be looked up
-     * @param propertyName the name of the property to look up
-     * @return value when the propertyName is defined or return null
-     */
-    private static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
-                                             String propertyName) {
-        Boolean value = null;
-        try {
-            String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? null : s.trim().equals("true");
-        } catch (ClassCastException e) {
-            // No propertyName defined.
-            value = null;
-        }
-        return value;
-    }
-
     @Override
     public void triggerProbe(Host host) {
         log.info("Triggering probe on device {}", host);
--- a/utils/misc/src/main/java/org/onlab/util/Tools.java
View file @d9b5f55
+++ b/utils/misc/src/main/java/org/onlab/util/Tools.java
View file @d9b5f55
@@ -275,6 +275,84 @@ public abstract class Tools {
     }
     /**
+     * Get Integer property from the propertyName
+     * Return null if propertyName is not found.
+     *
+     * @param properties   properties to be looked up
+     * @param propertyName the name of the property to look up
+     * @return value when the propertyName is defined or return null
+     */
+    public static Integer getIntegerProperty(Dictionary<?, ?> properties,
+                                             String propertyName) {
+        Integer value;
+        try {
+            String s = get(properties, propertyName);
+            value = Strings.isNullOrEmpty(s) ? null : Integer.valueOf(s);
+        } catch (NumberFormatException | ClassCastException e) {
+            value = null;
+        }
+        return value;
+    }
+
+    /**
+     * Get Integer property from the propertyName
+     * Return default value if propertyName is not found.
+     *
+     * @param properties   properties to be looked up
+     * @param propertyName the name of the property to look up
+     * @param defaultValue the default value that to be assigned
+     * @return value when the propertyName is defined or return default value
+     */
+    public static int getIntegerProperty(Dictionary<?, ?> properties,
+                                         String propertyName,
+                                         int defaultValue) {
+        try {
+            String s = get(properties, propertyName);
+            return Strings.isNullOrEmpty(s) ? defaultValue : Integer.valueOf(s);
+        } catch (NumberFormatException | ClassCastException e) {
+            return defaultValue;
+        }
+    }
+
+    /**
+     * Check property name is defined and set to true.
+     *
+     * @param properties   properties to be looked up
+     * @param propertyName the name of the property to look up
+     * @return value when the propertyName is defined or return null
+     */
+    public static Boolean isPropertyEnabled(Dictionary<?, ?> properties,
+                                             String propertyName) {
+        Boolean value;
+        try {
+            String s = get(properties, propertyName);
+            value = Strings.isNullOrEmpty(s) ? null : Boolean.valueOf(s);
+        } catch (ClassCastException e) {
+            value = null;
+        }
+        return value;
+    }
+
+    /**
+     * Check property name is defined as set to true.
+     *
+     * @param properties   properties to be looked up
+     * @param propertyName the name of the property to look up
+     * @param defaultValue the default value that to be assigned
+     * @return value when the propertyName is defined or return the default value
+     */
+    public static boolean isPropertyEnabled(Dictionary<?, ?> properties,
+                                            String propertyName,
+                                            boolean defaultValue) {
+        try {
+            String s = get(properties, propertyName);
+            return Strings.isNullOrEmpty(s) ? defaultValue : Boolean.valueOf(s);
+        } catch (ClassCastException e) {
+            return defaultValue;
+        }
+    }
+
+    /**
      * Suspends the current thread for a specified number of millis.
      *
      * @param ms number of millis