This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 9239669ee691d03edbd8fbd729660ba47224fe0c Author: Benoit Tellier <[email protected]> AuthorDate: Mon Apr 8 15:36:50 2019 +0700 JAMES-2708 Export mechanism choice should not fail when not given In order to avoid breaking changes, let's use localFile option by default --- .../apache/james/modules/BlobExportImplChoice.java | 14 +++++------- .../james/modules/BlobExportMechanismModule.java | 6 ++++- .../james/modules/BlobExportImplChoiceTest.java | 26 ++++++++++------------ .../modules/BlobExportMechanismModuleTest.java | 9 ++++---- 4 files changed, 27 insertions(+), 28 deletions(-) diff --git a/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportImplChoice.java b/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportImplChoice.java index 90f6c35..4551929 100644 --- a/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportImplChoice.java +++ b/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportImplChoice.java @@ -24,7 +24,6 @@ import java.util.Optional; import java.util.stream.Stream; import org.apache.commons.configuration.Configuration; -import org.apache.commons.configuration.ConfigurationException; import com.github.steveash.guavate.Guavate; import com.google.common.base.Joiner; @@ -65,16 +64,15 @@ class BlobExportImplChoice { return new BlobExportImplChoice(BlobExportImplName.LOCAL_FILE); } - static BlobExportImplChoice from(Configuration configuration) throws ConfigurationException { + static Optional<BlobExportImplChoice> from(Configuration configuration) { String blobExportImpl = configuration.getString(BLOB_EXPORT_MECHANISM_IMPL); - String sanitizedImplName = Optional.ofNullable(blobExportImpl) - .map(String::trim) - .orElseThrow(() -> new ConfigurationException(BLOB_EXPORT_MECHANISM_IMPL + " property is mandatory")); + Optional<String> sanitizedImplName = Optional.ofNullable(blobExportImpl) + .map(String::trim); - return BlobExportImplName.from(sanitizedImplName) - .map(BlobExportImplChoice::new) - .orElseThrow(() -> new ConfigurationException(unknownBlobExportErrorMessage(blobExportImpl))); + return sanitizedImplName.map(name -> BlobExportImplName.from(name) + .map(BlobExportImplChoice::new) + .orElseThrow(() -> new IllegalArgumentException(unknownBlobExportErrorMessage(name)))); } private static String unknownBlobExportErrorMessage(String blobExportImpl) { diff --git a/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportMechanismModule.java b/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportMechanismModule.java index 5c9fba5..6fa7862 100644 --- a/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportMechanismModule.java +++ b/server/container/guice/blob-export-guice/src/main/java/org/apache/james/modules/BlobExportMechanismModule.java @@ -52,7 +52,11 @@ public class BlobExportMechanismModule extends AbstractModule { BlobExportImplChoice provideChoice(PropertiesProvider propertiesProvider) throws ConfigurationException { try { Configuration configuration = propertiesProvider.getConfiguration(ConfigurationComponent.NAME); - return BlobExportImplChoice.from(configuration); + return BlobExportImplChoice.from(configuration) + .orElseGet(() -> { + LOGGER.warn("No blob export mechanism defined. Defaulting to " + BlobExportImplChoice.BlobExportImplName.LOCAL_FILE.getImplName()); + return BlobExportImplChoice.localFile(); + }); } catch (FileNotFoundException e) { LOGGER.warn("Could not find " + ConfigurationComponent.NAME + " configuration file, using localFile blob exporting as the default"); return BlobExportImplChoice.localFile(); diff --git a/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportImplChoiceTest.java b/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportImplChoiceTest.java index bb0ed2a..9affa58 100644 --- a/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportImplChoiceTest.java +++ b/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportImplChoiceTest.java @@ -22,9 +22,7 @@ package org.apache.james.modules; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; -import org.apache.james.modules.BlobExportImplChoice.BlobExportImplName; import org.junit.jupiter.api.Test; import nl.jqno.equalsverifier.EqualsVerifier; @@ -38,11 +36,11 @@ class BlobExportImplChoiceTest { } @Test - void fromShouldThrowWhenImplIsNull() { + void fromShouldReturnDefaultWhenImplIsNull() { PropertiesConfiguration configuration = new PropertiesConfiguration(); - assertThatThrownBy(() -> BlobExportImplChoice.from(configuration)) - .isInstanceOf(ConfigurationException.class); + assertThat(BlobExportImplChoice.from(configuration)) + .isEmpty(); } @Test @@ -51,33 +49,33 @@ class BlobExportImplChoiceTest { configuration.addProperty("blob.export.implementation", "unknown"); assertThatThrownBy(() -> BlobExportImplChoice.from(configuration)) - .isInstanceOf(ConfigurationException.class); + .isInstanceOf(IllegalArgumentException.class); } @Test - void fromShouldReturnLocalFileImplWhenPassingLocalFileImplConfiguration() throws Exception { + void fromShouldReturnLocalFileImplWhenPassingLocalFileImplConfiguration() { PropertiesConfiguration configuration = new PropertiesConfiguration(); configuration.addProperty("blob.export.implementation", "localFile"); - assertThat(BlobExportImplChoice.from(configuration).getImpl()) - .isEqualTo(BlobExportImplName.LOCAL_FILE); + assertThat(BlobExportImplChoice.from(configuration)) + .contains(BlobExportImplChoice.localFile()); } @Test - void fromShouldThrowWhenCaseInSensitive() throws Exception { + void fromShouldThrowWhenCaseInSensitive() { PropertiesConfiguration configuration = new PropertiesConfiguration(); configuration.addProperty("blob.export.implementation", "localFILE"); assertThatThrownBy(() -> BlobExportImplChoice.from(configuration)) - .isInstanceOf(ConfigurationException.class); + .isInstanceOf(IllegalArgumentException.class); } @Test - void fromShouldIgnoreBlankSpacesBeforeAndAfter() throws Exception { + void fromShouldIgnoreBlankSpacesBeforeAndAfter() { PropertiesConfiguration configuration = new PropertiesConfiguration(); configuration.addProperty("blob.export.implementation", " localFile "); - assertThat(BlobExportImplChoice.from(configuration).getImpl()) - .isEqualTo(BlobExportImplName.LOCAL_FILE); + assertThat(BlobExportImplChoice.from(configuration)) + .contains(BlobExportImplChoice.localFile()); } } \ No newline at end of file diff --git a/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportMechanismModuleTest.java b/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportMechanismModuleTest.java index e69e608..e9dbdd2 100644 --- a/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportMechanismModuleTest.java +++ b/server/container/guice/blob-export-guice/src/test/java/org/apache/james/modules/BlobExportMechanismModuleTest.java @@ -24,7 +24,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; -import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; import org.apache.james.FakePropertiesProvider; import org.apache.james.blob.export.file.LocalFileBlobExportMechanism; @@ -67,7 +66,7 @@ class BlobExportMechanismModuleTest { } @Test - void provideChoiceShouldThrowWhenConfigurationIsUnknown() throws Exception { + void provideChoiceShouldThrowWhenConfigurationIsUnknown() { PropertiesConfiguration configuration = new PropertiesConfiguration(); configuration.addProperty("blob.export.implementation", "unknown"); @@ -76,7 +75,7 @@ class BlobExportMechanismModuleTest { .build(); assertThatThrownBy(() -> module.provideChoice(noConfigurationFile)) - .isInstanceOf(ConfigurationException.class); + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -86,8 +85,8 @@ class BlobExportMechanismModuleTest { .register(NAME, configuration) .build(); - assertThatThrownBy(() -> module.provideChoice(noConfigurationFile)) - .isInstanceOf(ConfigurationException.class); + assertThat(module.provideChoice(noConfigurationFile)) + .isEqualTo(BlobExportImplChoice.localFile()); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
