dhis2-devs team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 20267
committer: Halvdan Hoem Grelland <halvdanhg@xxxxxxxxx>
branch nick: dhis2
timestamp: Mon 2015-09-21 16:23:35 +0200
message:
  Refactored and split up init method for readability. Improved logging.
modified:
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceContentStore.java


--
lp:dhis2
https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk

Your team DHIS 2 developers is subscribed to branch lp:dhis2.
To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceContentStore.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceContentStore.java	2015-09-21 13:11:42 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceContentStore.java	2015-09-21 14:23:35 +0000
@@ -52,6 +52,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 /**
@@ -62,6 +63,8 @@
 {
     private static final Log log = LogFactory.getLog( DefaultFileResourceContentStore.class );
 
+    private static final Pattern CONTAINER_NAME_PATTERN = Pattern.compile( "^((?!-)[a-zA-Z0-9-]{1,63}(?<!-))+$" );
+
     private BlobStore blobStore;
     private BlobStoreContext blobStoreContext;
     private String container;
@@ -121,40 +124,35 @@
     // Life cycle management
     // -------------------------------------------------------------------------
 
-    // TODO Untangle and split up
     public void init()
     {
         // -------------------------------------------------------------------------
         // Parse properties
         // -------------------------------------------------------------------------
 
-        Properties properties = configurationProvider.getConfiguration().getProperties();
-
-        Map<String, String> fileStoreConfiguration = properties
-            .entrySet().stream().filter( p -> ((String) p.getKey()).startsWith( FILE_STORE_CONFIG_NAMESPACE ) )
-            .collect( Collectors.toMap(
-                p -> StringUtils.strip( (String) p.getKey() ),
-                p -> StringUtils.strip( (String) p.getValue() )
-            ) );
+        Map<String, String> fileStoreConfiguration = getFileStorePropertiesMap();
 
         String provider = fileStoreConfiguration.getOrDefault( KEY_FILE_STORE_PROVIDER, JCLOUDS_PROVIDER_KEY_FILESYSTEM );
-
-        if ( !SUPPORTED_PROVIDERS.contains( provider ) )
-        {
-            log.warn( "Ignored unsupported file store provider '" + provider + "', falling back to file system." );
-            provider = JCLOUDS_PROVIDER_KEY_FILESYSTEM;
-        }
-
-        if ( provider.equals( JCLOUDS_PROVIDER_KEY_FILESYSTEM ) && !locationManager.externalDirectorySet() )
-        {
-            log.warn( "File system store provider configured but external directory is not set. Falling back to transient file store." );
-            provider = JCLOUDS_PROVIDER_KEY_TRANSIENT;
-        }
-
-        container = fileStoreConfiguration.getOrDefault( KEY_FILE_STORE_CONTAINER, DEFAULT_CONTAINER );
-
-        String location = fileStoreConfiguration.getOrDefault( KEY_FILE_STORE_LOCATION, null );
+        provider = validateAndSelectProvider( provider );
+
+        container = fileStoreConfiguration.get( KEY_FILE_STORE_CONTAINER );
+
+        if ( !isValidContainerName( container ) )
+        {
+            if ( container != null )
+            {
+                log.warn( "Container name '" + container + "' is illegal." +
+                    "Standard domain name naming conventions apply (and underscores are not allowed). " +
+                    "Using default container name." );
+            }
+
+            container = DEFAULT_CONTAINER;
+        }
+
+        String location = fileStoreConfiguration.get( KEY_FILE_STORE_LOCATION );
+
         Properties overrides = new Properties();
+
         Credentials credentials = new Credentials( "Unused", "Unused" );
 
         // -------------------------------------------------------------------------
@@ -164,24 +162,20 @@
         if ( provider.equals( JCLOUDS_PROVIDER_KEY_FILESYSTEM ) && locationManager.externalDirectorySet() )
         {
             overrides.setProperty( FilesystemConstants.PROPERTY_BASEDIR, locationManager.getExternalDirectoryPath() );
-
-            log.info( "File system store provider configured" );
         }
         else if ( provider.equals( JCLOUDS_PROVIDER_KEY_AWS_S3 ) )
         {
             credentials = new Credentials( fileStoreConfiguration.getOrDefault(
                 KEY_FILE_STORE_IDENTITY, "" ), fileStoreConfiguration.getOrDefault( KEY_FILE_STORE_SECRET, "" ) );
 
-            log.info( "AWS S3 filestore provider configured." );
-
             if ( credentials.identity.isEmpty() || credentials.credential.isEmpty() )
             {
-                log.info( "AWS S3 store configured with empty credentials, authentication not possible" );
+                log.warn( "AWS S3 store configured with empty credentials, authentication not possible" );
             }
         }
 
         // -------------------------------------------------------------------------
-        // Set up BlobStore
+        // Set up JClouds context
         // -------------------------------------------------------------------------
 
         blobStoreContext = ContextBuilder.newBuilder( provider )
@@ -194,6 +188,9 @@
             .stream().filter( l -> l.getId().equals( location ) ).findFirst();
 
         blobStore.createContainerInLocation( configuredLocation.isPresent() ? configuredLocation.get() : null, container );
+
+        log.info( "File store configured with provider '" + provider + "' and container '" + container + "'. " +
+            ( configuredLocation.isPresent() ? "Provider location: " + configuredLocation.get().getId() : "" ) );
     }
 
     public void cleanUp()
@@ -290,4 +287,36 @@
             .contentMD5( HashCode.fromString( contentMd5 ) )
             .build();
     }
+
+    private Map<String, String> getFileStorePropertiesMap()
+    {
+        return  configurationProvider.getConfiguration().getProperties().entrySet().stream()
+            .filter( p -> ((String) p.getKey()).startsWith( FILE_STORE_CONFIG_NAMESPACE ) )
+            .collect( Collectors.toMap(
+                p -> StringUtils.strip( (String) p.getKey() ),
+                p -> StringUtils.strip( (String) p.getValue() )
+            ) );
+    }
+
+    private String validateAndSelectProvider( String provider )
+    {
+        if ( !SUPPORTED_PROVIDERS.contains( provider ) )
+        {
+            log.warn( "Ignored unsupported file store provider '" + provider + "', using file system provider." );
+            provider = JCLOUDS_PROVIDER_KEY_FILESYSTEM;
+        }
+
+        if ( provider.equals( JCLOUDS_PROVIDER_KEY_FILESYSTEM ) && !locationManager.externalDirectorySet() )
+        {
+            log.warn( "File system file store provider could not be configured; external directory is not set. " +
+                "Falling back to in-memory provider." );
+            provider = JCLOUDS_PROVIDER_KEY_TRANSIENT;
+        }
+
+        return provider;
+    }
+
+    private boolean isValidContainerName( String containerName ) {
+        return containerName != null && CONTAINER_NAME_PATTERN.matcher( containerName ).matches();
+    }
 }