FISH-788: DirConfigSource should not log severe messages if unused. (#…

…5104)

* fix(mpconfig): make DirConfigSource less disrupting if not configured

During QA after merging #5007, @lprimak stumbled over a SEVERE
log message during deploys, triggered by the non-existing directory
of DirConfigSource. The directory config value has had a default
before, also the docs say otherwise.

The level issue has been fixed by checking only default config value
being used, resulting in an INFO level message. The logic has been
refactored not to use an exception, but Optional API.

- Docs will be fixed to include the formerly hidden default value
- Part of #5006

* style(mpconfig): reducing log level in DirConfigSource.findDir()

As requested by @pdudits, reducing the log level to FINE
if the directory targeted by default value (secrets) is not present.

nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/source/DirConfigSource.java

@@ -60,12 +60,15 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static java.util.Collections.unmodifiableMap;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.SEVERE;
 import static java.util.logging.Level.WARNING;
 import static java.util.stream.Collectors.toMap;
@@ -210,15 +213,19 @@ public final void run() {
     }
 
     private static final Logger logger = Logger.getLogger(DirConfigSource.class.getName());
+    public static final String DEFAULT_DIR = "secrets";
     private Path directory;
     private final ConcurrentHashMap<String, DirProperty> properties = new ConcurrentHashMap<>();
 
     public DirConfigSource() {
         try {
             // get the directory from the app server config
-            this.directory = findDir();
-            // create the watcher for the directory
-            configService.getExecutor().scheduleWithFixedDelay(createWatcher(this.directory), 0, 1, SECONDS);
+            Optional<Path> dir = findDir();
+            if (dir.isPresent()) {
+                this.directory = dir.get();
+                // create the watcher for the directory
+                configService.getExecutor().scheduleWithFixedDelay(createWatcher(this.directory), 0, 1, SECONDS);
+            }
         } catch (IOException e) {
             logger.log(SEVERE, "MPCONFIG DirConfigSource: error during setup.", e);
         }
@@ -231,7 +238,6 @@ public DirConfigSource() {
         this.directory = directory;
     }
 
-    // Used for testing only
     DirPropertyWatcher createWatcher(Path topmostDirectory) throws IOException {
         return new DirPropertyWatcher(topmostDirectory);
     }
@@ -269,22 +275,31 @@ public String getName() {
         return "Directory";
     }
 
-    Path findDir() throws IOException {
+    Optional<Path> findDir() throws IOException {
         String path = configService.getMPConfig().getSecretDir();
-        List<Path> candidates = new ArrayList<>();
+        if (path == null)
+            return Optional.empty();
 
         // adding all pathes where to look for the directory...
+        List<Path> candidates = new ArrayList<>();
         candidates.add(Paths.get(path));
-        // let's try it relative to server environment root
+        // let's try it relative to server environment root (<PAYARA-HOME>/glassfish/domains/<DOMAIN>/)
         if ( ! Paths.get(path).isAbsolute())
             candidates.add(Paths.get(System.getProperty("com.sun.aas.instanceRoot"), path).normalize());
 
         for (Path candidate : candidates) {
             if (isAptDir(candidate)) {
-                return candidate;
+                return Optional.of(candidate);
             }
         }
-        throw new IOException("Given MPCONFIG directory '"+path+"' is no directory, cannot be read or has a leading dot.");
+
+        // Log that the configured directory is not resolving.
+        Level lvl = SEVERE;
+        // Reduce log level to fine if default setting, as the admin might simply not use this functionality.
+        if(path.equals(DEFAULT_DIR))
+            lvl = FINE;
+        logger.log(lvl, "Given MPCONFIG directory '" + path + "' is no directory, cannot be read or has a leading dot.");
+        return Optional.empty();
     }
 
     /**

nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/MicroprofileConfigConfiguration.java

@@ -43,6 +43,7 @@
 
 import javax.validation.constraints.Min;
 
+import fish.payara.nucleus.microprofile.config.source.DirConfigSource;
 import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.spi.ConfigSource;
 import org.glassfish.api.admin.config.ConfigExtension;
@@ -63,7 +64,7 @@
 @Configured(name="microprofile-config")
 public interface MicroprofileConfigConfiguration extends ConfigExtension {
 
-    @Attribute(defaultValue = "secrets", dataType = String.class)
+    @Attribute(defaultValue = DirConfigSource.DEFAULT_DIR, dataType = String.class)
     String getSecretDir();
     void setSecretDir(String directory);
 

nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/source/DirConfigSourceTest.java

@@ -55,13 +55,7 @@
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.FileTime;
 import java.time.Instant;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Random;
+import java.util.*;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -105,16 +99,40 @@ public void cleanProps() {
         source.setProperties(Collections.emptyMap());
     }
 
+    @Test
+    public void testFindDir_NullPath() throws IOException {
+        // given
+        MicroprofileConfigConfiguration config = mock(MicroprofileConfigConfiguration.class);
+        when(configService.getMPConfig()).thenReturn(config);
+        when(config.getSecretDir()).thenReturn(null);
+        // when
+        Optional<Path> sut = source.findDir();
+        // then
+        assertFalse(sut.isPresent());
+    }
+
+    @Test
+    public void testFindDir_NotExistingPath() throws IOException {
+        // given
+        MicroprofileConfigConfiguration config = mock(MicroprofileConfigConfiguration.class);
+        when(configService.getMPConfig()).thenReturn(config);
+        when(config.getSecretDir()).thenReturn(DirConfigSource.DEFAULT_DIR);
+        // when
+        Optional<Path> sut = source.findDir();
+        // then
+        assertFalse(sut.isPresent());
+    }
+
     @Test
     public void testFindDir_AbsolutePath() throws IOException {
         // given
         MicroprofileConfigConfiguration config = mock(MicroprofileConfigConfiguration.class);
         when(configService.getMPConfig()).thenReturn(config);
         when(config.getSecretDir()).thenReturn(testDirectory.toString());
         // when
-        Path sut = source.findDir();
+        Optional<Path> sut = source.findDir();
         // then
-        assertEquals(testDirectory, sut);
+        assertEquals(testDirectory, sut.get());
     }
 
     @Test
@@ -126,10 +144,10 @@ public void testFindDir_RelativePath() throws IOException {
         when(config.getSecretDir()).thenReturn(".");
 
         // when
-        Path sut = source.findDir();
+        Optional<Path> sut = source.findDir();
 
         // then
-        assertEquals(testDirectory, sut);
+        assertEquals(testDirectory, sut.get());
     }
 
     @Test