From: Tom Pantelis <tpanteli@brocade.com>
Date: Wed, 28 Oct 2015 19:37:32 +0000 (-0400)
Subject: Fix NPE in EntityOwnerSelectionStrategyConfigReader
X-Git-Tag: release/lithium-sr3~8
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=b3cf2ee574aa20886e0e2fc3273ceb77d3893123;p=controller.git

Fix NPE in EntityOwnerSelectionStrategyConfigReader

Configuration#getProperties can return null if the config doesn't exist
- need to check for that. Also added more checks and unit tests.

Change-Id: If468fb8c3df7ecba664bb00a8f01bdfec7b4ceeb
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
---

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReader.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReader.java
index f9d1aea0a1..b6092b2e00 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReader.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReader.java
@@ -12,6 +12,7 @@ import com.google.common.base.Preconditions;
 import java.io.IOException;
 import java.util.Dictionary;
 import java.util.Enumeration;
+import javax.annotation.Nullable;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.cm.Configuration;
@@ -39,35 +40,48 @@ public class EntityOwnerSelectionStrategyConfigReader {
     }
 
     private EntityOwnerSelectionStrategyConfig readConfiguration(ServiceReference<ConfigurationAdmin> configAdminServiceReference) {
-        EntityOwnerSelectionStrategyConfig strategyConfig;
         EntityOwnerSelectionStrategyConfig.Builder builder = EntityOwnerSelectionStrategyConfig.newBuilder();
+        ConfigurationAdmin configAdmin = null;
         try {
-            ConfigurationAdmin configAdmin = bundleContext.getService(configAdminServiceReference);
-            Configuration config = configAdmin.getConfiguration(CONFIG_ID);
-            Dictionary<String, Object> properties = config.getProperties();
-            Enumeration<String> keys = properties.keys();
-            while (keys.hasMoreElements()) {
-                String key = keys.nextElement();
-                String strategyProps = (String) properties.get(key);
-                String[] strategyClassAndDelay = strategyProps.split(",");
-                if(strategyClassAndDelay.length >= 1) {
-                    Class<? extends EntityOwnerSelectionStrategy> aClass
-                            = (Class<? extends EntityOwnerSelectionStrategy>) getClass().getClassLoader().loadClass(strategyClassAndDelay[0]);
-                    long delay = 0;
-                    if(strategyClassAndDelay.length > 1){
-                        delay = Long.parseLong(strategyClassAndDelay[1]);
+            configAdmin = bundleContext.getService(configAdminServiceReference);
+            Dictionary<String, Object> properties = getProperties(configAdmin);
+            if(properties != null) {
+                Enumeration<String> keys = properties.keys();
+                while (keys.hasMoreElements()) {
+                    String key = keys.nextElement();
+                    String strategyProps = (String) properties.get(key);
+                    String[] strategyClassAndDelay = strategyProps.split(",");
+                    if(strategyClassAndDelay.length >= 1) {
+                        @SuppressWarnings("unchecked")
+                        Class<? extends EntityOwnerSelectionStrategy> aClass
+                        = (Class<? extends EntityOwnerSelectionStrategy>) getClass().getClassLoader().loadClass(strategyClassAndDelay[0]);
+                        long delay = 0;
+                        if(strategyClassAndDelay.length > 1){
+                            delay = Long.parseLong(strategyClassAndDelay[1]);
+                        }
+                        builder.addStrategy(key, aClass, delay);
                     }
-                    builder.addStrategy(key, aClass, delay);
                 }
             }
-            strategyConfig = builder.build();
-        } catch(IOException | ClassNotFoundException | NumberFormatException e){
+        } catch(Exception e){
             LOG.warn("Failed to read selection strategy configuration file. All configuration will be ignored.", e);
-            strategyConfig = EntityOwnerSelectionStrategyConfig.newBuilder().build();
         } finally {
-            bundleContext.ungetService(configAdminServiceReference);
+            if(configAdmin != null) {
+                try {
+                    bundleContext.ungetService(configAdminServiceReference);
+                } catch (Exception e) {
+                    LOG.debug("Error from ungetService", e);
+                }
+            }
         }
-        return strategyConfig;
+
+        return builder.build();
+    }
+
+    @Nullable
+    private static Dictionary<String, Object> getProperties(ConfigurationAdmin configAdmin) throws IOException {
+        Configuration config = configAdmin.getConfiguration(CONFIG_ID);
+        return config != null ? config.getProperties() : null;
     }
 
     public EntityOwnerSelectionStrategyConfig getConfig() {
diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReaderTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReaderTest.java
index feced489a2..5302283a15 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReaderTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReaderTest.java
@@ -37,9 +37,6 @@ public class EntityOwnerSelectionStrategyConfigReaderTest {
         doReturn(mockConfigAdmin).when(mockBundleContext).getService(mockConfigAdminServiceRef);
 
         doReturn(mockConfig).when(mockConfigAdmin).getConfiguration(EntityOwnerSelectionStrategyConfigReader.CONFIG_ID);
-
-
-
     }
 
     @Test
@@ -59,7 +56,7 @@ public class EntityOwnerSelectionStrategyConfigReaderTest {
     }
 
     @Test
-    public void testReadStrategiesForNonExistentFile() throws IOException {
+    public void testReadStrategiesWithIOException() throws IOException {
         doThrow(IOException.class).when(mockConfigAdmin).getConfiguration(EntityOwnerSelectionStrategyConfigReader.CONFIG_ID);
 
         EntityOwnerSelectionStrategyConfig config = new EntityOwnerSelectionStrategyConfigReader(mockBundleContext).getConfig();
@@ -67,6 +64,24 @@ public class EntityOwnerSelectionStrategyConfigReaderTest {
         assertFalse(config.isStrategyConfigured("test"));
     }
 
+    @Test
+    public void testReadStrategiesWithNullConfiguration() throws IOException {
+        doReturn(null).when(mockConfigAdmin).getConfiguration(EntityOwnerSelectionStrategyConfigReader.CONFIG_ID);
+
+        EntityOwnerSelectionStrategyConfig config = new EntityOwnerSelectionStrategyConfigReader(mockBundleContext).getConfig();
+
+        assertFalse(config.isStrategyConfigured("test"));
+    }
+
+    @Test
+    public void testReadStrategiesWithNullConfigurationProperties() throws IOException {
+        doReturn(null).when(mockConfig).getProperties();
+
+        EntityOwnerSelectionStrategyConfig config = new EntityOwnerSelectionStrategyConfigReader(mockBundleContext).getConfig();
+
+        assertFalse(config.isStrategyConfigured("test"));
+    }
+
     @Test
     public void testReadStrategiesInvalidDelay(){
         Hashtable<String, Object> props = new Hashtable<>();