JAMES-2582 Swift blobstore factory should not be instanciated if not required

            If we don't enable Swift we should not fail when configuration is 
absent.
            The factory eagerly load the configuration right now, so I solved 
the
            issue with "another level of indirection" (tm).


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/623078fc
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/623078fc
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/623078fc

Branch: refs/heads/master
Commit: 623078fc9f595d3406b9319df58ba9faf3b52f0e
Parents: 876987c
Author: Matthieu Baechler <[email protected]>
Authored: Wed Nov 7 15:33:08 2018 +0100
Committer: Antoine Duprat <[email protected]>
Committed: Thu Nov 8 11:57:17 2018 +0100

----------------------------------------------------------------------
 .../objectstore/BlobStoreChoosingModule.java    | 23 ++++++++------------
 .../BlobStoreChoosingModuleTest.java            | 12 +++++-----
 2 files changed, 15 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/623078fc/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/objectstore/BlobStoreChoosingModule.java
----------------------------------------------------------------------
diff --git 
a/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/objectstore/BlobStoreChoosingModule.java
 
b/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/objectstore/BlobStoreChoosingModule.java
index a6467e0..fce77e6 100644
--- 
a/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/objectstore/BlobStoreChoosingModule.java
+++ 
b/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/objectstore/BlobStoreChoosingModule.java
@@ -20,9 +20,9 @@
 package org.apache.james.modules.objectstore;
 
 import java.io.FileNotFoundException;
-import java.util.function.Supplier;
 
 import javax.inject.Inject;
+import javax.inject.Provider;
 import javax.inject.Singleton;
 
 import org.apache.commons.configuration.Configuration;
@@ -49,7 +49,7 @@ import com.google.inject.multibindings.Multibinder;
 
 public class BlobStoreChoosingModule extends AbstractModule {
 
-    interface BlobStoreFactory extends Supplier<BlobStore> {}
+    interface BlobStoreFactory extends Provider<BlobStore> {}
 
     static class CassandraBlobStoreFactory implements BlobStoreFactory {
         private final Session session;
@@ -98,36 +98,31 @@ public class BlobStoreChoosingModule extends AbstractModule 
{
         bind(CassandraBlobStoreFactory.class).in(Scopes.SINGLETON);
         Multibinder<CassandraModule> cassandraDataDefinitions = 
Multibinder.newSetBinder(binder(), CassandraModule.class);
         
cassandraDataDefinitions.addBinding().toInstance(CassandraBlobModule.MODULE);
+
+        
bind(BlobStore.class).toProvider(BlobStoreFactory.class).in(Scopes.SINGLETON);
     }
 
     @VisibleForTesting
     @Provides
     @Singleton
     BlobStoreFactory provideBlobStoreFactory(PropertiesProvider 
propertiesProvider,
-                                             CassandraBlobStoreFactory 
cassandraBlobStoreFactory,
-                                             SwiftBlobStoreFactory 
swiftBlobStoreFactory) throws ConfigurationException {
+                                             
Provider<CassandraBlobStoreFactory> cassandraBlobStoreFactoryProvider,
+                                             Provider<SwiftBlobStoreFactory> 
swiftBlobStoreFactoryProvider) throws ConfigurationException {
         try {
             Configuration configuration = 
propertiesProvider.getConfiguration(BLOBSTORE_CONFIGURATION_NAME);
             BlobStoreChoosingConfiguration choosingConfiguration = 
BlobStoreChoosingConfiguration.from(configuration);
             switch (choosingConfiguration.getImplementation()) {
                 case SWIFT:
-                    return swiftBlobStoreFactory;
+                    return swiftBlobStoreFactoryProvider.get();
                 case CASSANDRA:
-                    return cassandraBlobStoreFactory;
+                    return cassandraBlobStoreFactoryProvider.get();
                 default:
                     throw new RuntimeException(String.format("can not get the 
right blobstore provider with configuration %s",
                         choosingConfiguration.toString()));
             }
         } catch (FileNotFoundException e) {
             LOGGER.warn("Could not find " + BLOBSTORE_CONFIGURATION_NAME + " 
configuration file, using cassandra blobstore as the default");
-            return cassandraBlobStoreFactory;
+            return cassandraBlobStoreFactoryProvider.get();
         }
     }
-
-    @Provides
-    @Singleton
-    private BlobStore provideBlobStore(BlobStoreFactory blobStoreFactory) {
-        return blobStoreFactory.get();
-    }
-
 }

http://git-wip-us.apache.org/repos/asf/james-project/blob/623078fc/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/objectstore/BlobStoreChoosingModuleTest.java
----------------------------------------------------------------------
diff --git 
a/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/objectstore/BlobStoreChoosingModuleTest.java
 
b/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/objectstore/BlobStoreChoosingModuleTest.java
index d6a575d..72d931f 100644
--- 
a/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/objectstore/BlobStoreChoosingModuleTest.java
+++ 
b/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/objectstore/BlobStoreChoosingModuleTest.java
@@ -45,7 +45,7 @@ class BlobStoreChoosingModuleTest {
             .register(BLOBSTORE_CONFIGURATION_NAME, configuration)
             .build();
 
-        assertThatThrownBy(() -> 
module.provideBlobStoreFactory(propertyProvider, CASSANDRA_BLOBSTORE_FACTORY, 
SWIFT_BLOBSTORE_FACTORY))
+        assertThatThrownBy(() -> 
module.provideBlobStoreFactory(propertyProvider, () -> 
CASSANDRA_BLOBSTORE_FACTORY, () -> SWIFT_BLOBSTORE_FACTORY))
             .isInstanceOf(IllegalStateException.class);
     }
 
@@ -58,7 +58,7 @@ class BlobStoreChoosingModuleTest {
             .register(BLOBSTORE_CONFIGURATION_NAME, configuration)
             .build();
 
-        assertThatThrownBy(() -> 
module.provideBlobStoreFactory(propertyProvider, CASSANDRA_BLOBSTORE_FACTORY, 
SWIFT_BLOBSTORE_FACTORY))
+        assertThatThrownBy(() -> 
module.provideBlobStoreFactory(propertyProvider, () -> 
CASSANDRA_BLOBSTORE_FACTORY, () -> SWIFT_BLOBSTORE_FACTORY))
             .isInstanceOf(IllegalStateException.class);
     }
 
@@ -71,7 +71,7 @@ class BlobStoreChoosingModuleTest {
             .register(BLOBSTORE_CONFIGURATION_NAME, configuration)
             .build();
 
-        assertThatThrownBy(() -> 
module.provideBlobStoreFactory(propertyProvider, CASSANDRA_BLOBSTORE_FACTORY, 
SWIFT_BLOBSTORE_FACTORY))
+        assertThatThrownBy(() -> 
module.provideBlobStoreFactory(propertyProvider, () -> 
CASSANDRA_BLOBSTORE_FACTORY, () -> SWIFT_BLOBSTORE_FACTORY))
             .isInstanceOf(IllegalArgumentException.class);
     }
 
@@ -82,7 +82,7 @@ class BlobStoreChoosingModuleTest {
             .register("other_configuration_file", new 
PropertiesConfiguration())
             .build();
 
-        assertThat(module.provideBlobStoreFactory(propertyProvider, 
CASSANDRA_BLOBSTORE_FACTORY, SWIFT_BLOBSTORE_FACTORY))
+        assertThat(module.provideBlobStoreFactory(propertyProvider, () -> 
CASSANDRA_BLOBSTORE_FACTORY, () -> SWIFT_BLOBSTORE_FACTORY))
             .isEqualTo(CASSANDRA_BLOBSTORE_FACTORY);
     }
 
@@ -95,7 +95,7 @@ class BlobStoreChoosingModuleTest {
             .register(BLOBSTORE_CONFIGURATION_NAME, configuration)
             .build();
 
-        assertThat(module.provideBlobStoreFactory(propertyProvider, 
CASSANDRA_BLOBSTORE_FACTORY, SWIFT_BLOBSTORE_FACTORY))
+        assertThat(module.provideBlobStoreFactory(propertyProvider, () -> 
CASSANDRA_BLOBSTORE_FACTORY, () -> SWIFT_BLOBSTORE_FACTORY))
             .isEqualTo(SWIFT_BLOBSTORE_FACTORY);
     }
 
@@ -108,7 +108,7 @@ class BlobStoreChoosingModuleTest {
             .register(BLOBSTORE_CONFIGURATION_NAME, configuration)
             .build();
 
-        assertThat(module.provideBlobStoreFactory(propertyProvider, 
CASSANDRA_BLOBSTORE_FACTORY, SWIFT_BLOBSTORE_FACTORY))
+        assertThat(module.provideBlobStoreFactory(propertyProvider, () -> 
CASSANDRA_BLOBSTORE_FACTORY, () -> SWIFT_BLOBSTORE_FACTORY))
             .isEqualTo(CASSANDRA_BLOBSTORE_FACTORY);
     }
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to