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

Follow up for #147 #141 to add appropriate logging for resources loaded and
also when not found and not loaded.

Also adds some requireNonNull on builder parameters being set

Note that the InitialLoader (default configuration loading)
already logs the sources that were loaded so just tidy only
changes in InitialLoader

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");