CLOUDSTACK-9438: Fix for global default

Author: nvazquez

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -535,9 +535,6 @@ public String updateConfiguration(final long userId, final String name, final St
                     throw new InvalidParameterValueException("unable to find image store by id " + resourceId);
                 }
                 ImageStoreDetailVO imgStoreDetail = _imageStoreDetailsDao.findDetail(imgStore.getId(), name);
-                if (imgStoreDetail == null){
-                    throw new InvalidParameterValueException("unable to find image store details by id " + resourceId + " and name " + name);
-                }
                 _imageStoreDetailsDao.addDetail(resourceId, name, value, true);
                 break;
 

server/src/com/cloud/storage/ImageStoreDetailsUtil.java

@@ -20,6 +20,8 @@
 
 import javax.inject.Inject;
 
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
@@ -32,23 +34,40 @@ public class ImageStoreDetailsUtil {
     protected ImageStoreDao imageStoreDao;
     @Inject
     protected ImageStoreDetailsDao imageStoreDetailsDao;
+    @Inject
+    protected ConfigurationDao configurationDao;
 
     /**
-     * Obtain NFS protocol version (if provided) for a store id.<br/>
+     * Retrieve global secondary storage NFS version default value
+     * @return global default value
+     */
+    protected Integer getGlobalDefaultNfsVersion(){
+        ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key());
+        String value = globalNfsVersion.getValue();
+        return (value != null ? Integer.valueOf(value) : null);
+    }
+    /**
+     * Obtain NFS protocol version (if provided) for a store id, if not use default config value<br/>
      * @param storeId image store id
      * @return {@code null} if {@code secstorage.nfs.version} is not found for storeId <br/>
      * {@code X} if {@code secstorage.nfs.version} is found found for storeId
      */
     public Integer getNfsVersion(long storeId) throws NumberFormatException {
-        String nfsVersion = null;
+        Integer nfsVersion;
         if (imageStoreDetailsDao.getDetails(storeId) != null){
             Map<String, String> storeDetails = imageStoreDetailsDao.getDetails(storeId);
-            String nfsVersionKey = CapacityManager.ImageStoreNFSVersion.key();
-            if (storeDetails != null && storeDetails.containsKey(nfsVersionKey)){
-                nfsVersion = storeDetails.get(nfsVersionKey);
+            if (storeDetails.containsKey(CapacityManager.ImageStoreNFSVersion.key())){
+                String val = storeDetails.get(CapacityManager.ImageStoreNFSVersion.key());
+                nfsVersion = (val != null ? Integer.valueOf(val) : null);
+            }
+            else {
+                nfsVersion = getGlobalDefaultNfsVersion();
             }
         }
-        return (nfsVersion != null ? Integer.valueOf(nfsVersion) : null);
+        else {
+            nfsVersion = getGlobalDefaultNfsVersion();
+        }
+        return nfsVersion;
     }
 
     /**
@@ -62,7 +81,7 @@ public Integer getNfsVersionByUuid(String storeUuid){
         if (imageStore != null){
             return getNfsVersion(imageStore.getId());
         }
-        return null;
+        return getGlobalDefaultNfsVersion();
     }
 
 }

server/test/com/cloud/storage/ImageStoreDetailsUtilTest.java

@@ -17,13 +17,14 @@
 package com.cloud.storage;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
@@ -37,11 +38,13 @@ public class ImageStoreDetailsUtilTest {
     private final static long STORE_ID = 1l;
     private final static String STORE_UUID = "aaaa-aaaa-aaaa-aaaa";
     private final static Integer NFS_VERSION = 3;
+    private final static Integer NFS_VERSION_DEFAULT = 2;
 
     ImageStoreDetailsUtil imageStoreDetailsUtil = new ImageStoreDetailsUtil();
 
     ImageStoreDao imgStoreDao = mock(ImageStoreDao.class);
     ImageStoreDetailsDao imgStoreDetailsDao = mock(ImageStoreDetailsDao.class);
+    ConfigurationDao configurationDao = mock(ConfigurationDao.class);
 
     @Before
     public void setup() throws Exception {
@@ -54,8 +57,14 @@ public void setup() throws Exception {
         when(imgStoreVO.getId()).thenReturn(Long.valueOf(STORE_ID));
         when(imgStoreDao.findByUuid(STORE_UUID)).thenReturn(imgStoreVO);
 
+        ConfigurationVO confVO = mock(ConfigurationVO.class);
+        String defaultValue = (NFS_VERSION_DEFAULT == null ? null : String.valueOf(NFS_VERSION_DEFAULT));
+        when(confVO.getValue()).thenReturn(defaultValue);
+        when(configurationDao.findByName(nfsVersionKey)).thenReturn(confVO);
+
         imageStoreDetailsUtil.imageStoreDao = imgStoreDao;
         imageStoreDetailsUtil.imageStoreDetailsDao = imgStoreDetailsDao;
+        imageStoreDetailsUtil.configurationDao = configurationDao;
     }
 
     @Test
@@ -71,7 +80,7 @@ public void testGetNfsVersionNotFound(){
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
         Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
-        assertNull(nfsVersion);
+        assertEquals(NFS_VERSION_DEFAULT, nfsVersion);
     }
 
     @Test
@@ -80,7 +89,7 @@ public void testGetNfsVersionNoDetails(){
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
         Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
-        assertNull(nfsVersion);
+        assertEquals(NFS_VERSION_DEFAULT, nfsVersion);
     }
 
     @Test
@@ -93,6 +102,13 @@ public void testGetNfsVersionByUuid(){
     public void testGetNfsVersionByUuidNoImgStore(){
         when(imgStoreDao.findByUuid(STORE_UUID)).thenReturn(null);
         Integer nfsVersion = imageStoreDetailsUtil.getNfsVersionByUuid(STORE_UUID);
-        assertNull(nfsVersion);
+        assertEquals(NFS_VERSION_DEFAULT, nfsVersion);
     }
-}
+
+    @Test
+    public void testGetGlobalDefaultNfsVersion(){
+        Integer globalDefaultNfsVersion = imageStoreDetailsUtil.getGlobalDefaultNfsVersion();
+        assertEquals(NFS_VERSION_DEFAULT, globalDefaultNfsVersion);
+    }
+
+}