Merge pull request #148 from avaje/feature/builder-logging

Improve Builder logging for loading resources and files - followup for #141

Author: rob-bygrave

avaje-config/src/main/java/io/avaje/config/CoreConfigurationBuilder.java

@@ -11,42 +11,48 @@
 import java.util.ServiceLoader;
 import java.util.stream.Collectors;
 
+import static java.lang.System.Logger.Level.DEBUG;
+import static java.lang.System.Logger.Level.INFO;
 import static java.util.Objects.requireNonNull;
 
 @NonNullApi
 final class CoreConfigurationBuilder implements Configuration.Builder {
 
+  private ConfigurationLog log = initialiseLog();
   private final Parsers parsers = new Parsers();
   private final CoreEntry.CoreMap sourceMap = CoreEntry.newMap();
   private ResourceLoader resourceLoader = initialiseResourceLoader();
   private ModificationEventRunner eventRunner;
-  private ConfigurationLog configurationLog;
   private boolean includeResourceLoading;
   private InitialLoader initialLoader;
 
+  private static ConfigurationLog initialiseLog() {
+    return ServiceLoader.load(ConfigurationLog.class)
+        .findFirst()
+        .orElseGet(DefaultConfigurationLog::new);
+  }
+
   @Override
   public Configuration.Builder eventRunner(ModificationEventRunner eventRunner) {
-    this.eventRunner = eventRunner;
+    this.eventRunner = requireNonNull(eventRunner);
     return this;
   }
 
   @Override
-  public Configuration.Builder log(ConfigurationLog configurationLog) {
-    this.configurationLog = configurationLog;
+  public Configuration.Builder log(ConfigurationLog log) {
+    this.log = requireNonNull(log);
     return this;
   }
 
   @Override
   public Configuration.Builder resourceLoader(ResourceLoader resourceLoader) {
-    this.resourceLoader = resourceLoader;
+    this.resourceLoader = requireNonNull(resourceLoader);
     return this;
   }
 
   @Override
   public Configuration.Builder put(String key, String value) {
-    requireNonNull(key);
-    requireNonNull(value);
-    sourceMap.put(key, value, "initial");
+    sourceMap.put(requireNonNull(key), requireNonNull(value), "initial");
     return this;
   }
 
@@ -77,9 +83,12 @@ public Configuration.Builder load(String resource) {
     final var configParser = parser(resource);
     try {
       try (var inputStream = resourceLoader.getResourceAsStream(resource)) {
-        if (inputStream != null) {
+        if (inputStream == null) {
+          log.log(INFO, "Configuration resource:{0} not found", resource);
+        } else {
           var source = "resource:" + resource;
           configParser.load(inputStream).forEach((k, v) -> sourceMap.put(k, v, source));
+          log.log(DEBUG, "loaded {0}", source);
         }
         return this;
       }
@@ -91,13 +100,15 @@ public Configuration.Builder load(String resource) {
   @Override
   public Configuration.Builder load(File file) {
     if (!file.exists()) {
+      log.log(INFO, "Configuration file:{0} not found", file);
       return this;
     }
     final var configParser = parser(file.getName());
     try {
       try (var reader = new FileReader(file)) {
         var source = "file:" + file.getName();
         configParser.load(reader).forEach((k, v) -> sourceMap.put(k, v, source));
+        log.log(DEBUG, "loaded {0}", source);
         return this;
       }
     } catch (IOException e) {
@@ -127,7 +138,6 @@ public Configuration.Builder includeResourceLoading() {
   @Override
   public Configuration build() {
     final var runner = initRunner();
-    final var log = initLog();
     final var sources = ServiceLoader.load(ConfigurationSource.class).stream()
       .map(ServiceLoader.Provider::get)
       .collect(Collectors.toList());
@@ -159,15 +169,6 @@ private static ResourceLoader initialiseResourceLoader() {
       .orElseGet(DefaultResourceLoader::new);
   }
 
-  private ConfigurationLog initLog() {
-    if (configurationLog == null) {
-      configurationLog = ServiceLoader.load(ConfigurationLog.class)
-        .findFirst()
-        .orElseGet(DefaultConfigurationLog::new);
-    }
-    return configurationLog;
-  }
-
   private ModificationEventRunner initRunner() {
     if (eventRunner == null) {
       eventRunner = ServiceLoader.load(ModificationEventRunner.class)

avaje-config/src/main/java/io/avaje/config/InitialLoader.java

@@ -7,12 +7,12 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
-import java.lang.System.Logger.Level;
 import java.util.*;
 import java.util.regex.Pattern;
 
 import static io.avaje.config.InitialLoader.Source.FILE;
 import static io.avaje.config.InitialLoader.Source.RESOURCE;
+import static java.lang.System.Logger.Level.WARNING;
 
 /**
  * Loads the configuration from known/expected locations.
@@ -179,7 +179,7 @@ private boolean loadTest() {
     int before = loadContext.size();
     load("application-test", RESOURCE);
     if (loadProperties("test-ebean.properties", RESOURCE)) {
-      log.log(Level.WARNING, "Loading properties from test-ebean.properties is deprecated. Please migrate to application-test.yaml or application-test.properties instead.");
+      log.log(WARNING, "Loading properties from test-ebean.properties is deprecated. Please migrate to application-test.yaml or application-test.properties instead.");
     }
     return loadContext.size() > before;
   }
@@ -235,7 +235,7 @@ String[] splitPaths(String location) {
   private void loadMain(Source source) {
     load("application", source);
     if (loadProperties("ebean.properties", source)) {
-      log.log(Level.WARNING, "Loading properties from ebean.properties is deprecated. Please migrate to use application.yaml or application.properties instead.");
+      log.log(WARNING, "Loading properties from ebean.properties is deprecated. Please migrate to use application.yaml or application.properties instead.");
     }
   }
 
@@ -244,7 +244,7 @@ private void loadViaSystemProperty() {
     if (fileName == null) {
       fileName = System.getProperty("props.file");
       if (fileName != null && !loadWithExtensionCheck(fileName)) {
-        log.log(Level.WARNING, "Unable to find file {0} to load properties", fileName);
+        log.log(WARNING, "Unable to find file {0} to load properties", fileName);
       }
     }
   }
@@ -296,10 +296,10 @@ private boolean loadCustom(String resourcePath, Source source) {
   boolean loadCustomExtension(String resourcePath, ConfigParser parser, Source source) {
     try (InputStream is = resource(resourcePath, source)) {
       if (is != null) {
-        parser.load(is).forEach((k, v) -> loadContext.put(k, v, (source == RESOURCE ? "resource:" : "file:") + resourcePath));
+        var sourceName = (source == RESOURCE ? "resource:" : "file:") + resourcePath;
+        parser.load(is).forEach((k, v) -> loadContext.put(k, v, sourceName));
         return true;
       }
-
     } catch (Exception e) {
       throw new IllegalStateException("Error loading properties - " + resourcePath, e);
     }

avaje-config/src/test/java/io/avaje/config/CoreConfigurationTest.java

@@ -172,6 +172,8 @@ void builder() {
       .put("myExtraOne", "baz")
       .load(fileSource)
       .load("hi.properties")
+      .load("i-dont-exist.properties")
+      .load(new File("i-dont-exist-file.properties"))
       .build();
 
     assertEquals(conf.get("a"), "1");