[4.x] LazyConfigSource is now queried when an unknown node is request…

…ed (#8707)

* LazyConfigSource is now queried when an unknown node is requested by the user, instead of being ignored.
* Removed unneeded synchronized and use locks instead of ConcurrentHashMap, as computeIfAbsent should
not be used for possibly blocking operations (which we need).

config/config/src/main/java/io/helidon/config/ConfigFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2017, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,11 +17,11 @@
 package io.helidon.config;
 
 import java.time.Instant;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Function;
 
 import io.helidon.config.spi.ConfigFilter;
@@ -41,7 +41,8 @@ final class ConfigFactory {
     private final ConfigFilter filter;
     private final ProviderImpl provider;
     private final Function<String, List<String>> aliasGenerator;
-    private final ConcurrentMap<PrefixedKey, AbstractConfigImpl> configCache;
+    private final Map<PrefixedKey, AbstractConfigImpl> configCache;
+    private final ReentrantReadWriteLock configCacheLock = new ReentrantReadWriteLock();
     private final Instant timestamp;
 
     /**
@@ -70,8 +71,9 @@ final class ConfigFactory {
         this.provider = provider;
         this.aliasGenerator = aliasGenerator;
 
-        configCache = new ConcurrentHashMap<>();
-        timestamp = Instant.now();
+        // all access must be guarded by configCacheLock
+        this.configCache = new HashMap<>();
+        this.timestamp = Instant.now();
     }
 
     Instant timestamp() {
@@ -97,7 +99,31 @@ AbstractConfigImpl config() {
     AbstractConfigImpl config(ConfigKeyImpl prefix, ConfigKeyImpl key) {
         PrefixedKey prefixedKey = new PrefixedKey(prefix, key);
 
-        return configCache.computeIfAbsent(prefixedKey, it -> createConfig(prefix, key));
+        try {
+            configCacheLock.readLock().lock();
+            AbstractConfigImpl config = configCache.get(prefixedKey);
+            if (config != null) {
+                return config;
+            }
+        } finally {
+            configCacheLock.readLock().unlock();
+        }
+
+        // use write lock, re-check, and create if still missing (we want to have a guarantee each key is only created once)
+        try {
+            configCacheLock.writeLock().lock();
+            AbstractConfigImpl config = configCache.get(prefixedKey);
+            if (config != null) {
+                return config;
+            }
+            // we use locks, as this may be a blocking operation
+            // such as when using lazy config source that accesses remote servers
+            config = createConfig(prefix, key);
+            configCache.put(prefixedKey, config);
+            return config;
+        } finally {
+            configCacheLock.writeLock().unlock();
+        }
     }
 
     /**
@@ -126,7 +152,8 @@ private AbstractConfigImpl createConfig(ConfigKeyImpl prefix, ConfigKeyImpl key)
     }
 
     private ConfigNode findNode(ConfigKeyImpl prefix, ConfigKeyImpl key) {
-        ConfigNode node = fullKeyToNodeMap.get(prefix.child(key));
+        ConfigKeyImpl realKey = prefix.child(key);
+        ConfigNode node = fullKeyToNodeMap.get(realKey);
         if (node == null && aliasGenerator != null) {
             final String fullKey = key.toString();
             for (final String keyAlias : aliasGenerator.apply(fullKey)) {
@@ -136,6 +163,11 @@ private ConfigNode findNode(ConfigKeyImpl prefix, ConfigKeyImpl key) {
                 }
             }
         }
+        if (node == null) {
+            // check if any lazy source supports this node
+            return provider.lazyValue(realKey.toString())
+                    .orElse(null);
+        }
         return node;
     }
 

config/config/src/main/java/io/helidon/config/ConfigSourcesRuntime.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -75,6 +75,19 @@ public String toString() {
         return allSources.toString();
     }
 
+    Optional<ConfigNode> lazyValue(String key) {
+        // list of sources in `allSources` is final, there is no need to synchronize
+        for (ConfigSourceRuntimeImpl source : allSources) {
+            if (source.isLazy()) {
+                Optional<ConfigNode> node = source.node(key);
+                if (node.isPresent()) {
+                    return node;
+                }
+            }
+        }
+        return Optional.empty();
+    }
+
     void changeListener(Consumer<Optional<ObjectNode>> changeListener) {
         this.changeListener = changeListener;
     }

config/config/src/main/java/io/helidon/config/ProviderImpl.java

@@ -34,6 +34,7 @@
 import java.util.stream.Stream;
 
 import io.helidon.config.spi.ConfigFilter;
+import io.helidon.config.spi.ConfigNode;
 import io.helidon.config.spi.ConfigNode.ObjectNode;
 
 /**
@@ -116,6 +117,10 @@ public synchronized Config last() {
         return lastConfig;
     }
 
+    Optional<ConfigNode> lazyValue(String string) {
+        return configSource.lazyValue(string);
+    }
+
     void onChange(Consumer<ConfigDiff> listener) {
         this.listeners.add(listener);
     }

config/tests/pom.xml

@@ -63,5 +63,6 @@
         <module>service-registry</module>
         <module>config-metadata-meta-api</module>
         <module>config-metadata-builder-api</module>
+        <module>test-lazy-source</module>
     </modules>
 </project>

config/tests/test-lazy-source/pom.xml

@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright (c) 2024 Oracle and/or its affiliates.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+        http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>io.helidon.config.tests</groupId>
+        <artifactId>helidon-config-tests-project</artifactId>
+        <version>4.0.0-SNAPSHOT</version>
+    </parent>
+    <artifactId>helidon-config-tests-test-lazy-source</artifactId>
+    <name>Helidon Config Tests Lazy Source</name>
+
+    <description>
+        Integration tests of lazy config source.
+    </description>
+
+    <dependencies>
+        <dependency>
+            <groupId>io.helidon.config</groupId>
+            <artifactId>helidon-config</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+</project>

config/tests/test-lazy-source/src/test/java/io/helidon/config/tests/lazy/source/LazySourceTest.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.config.tests.lazy.source;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import io.helidon.config.Config;
+import io.helidon.config.ConfigSources;
+import io.helidon.config.spi.ConfigNode;
+import io.helidon.config.spi.ConfigSource;
+import io.helidon.config.spi.LazyConfigSource;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.is;
+
+public class LazySourceTest {
+    @Test
+    void testLazySource() {
+        Map<String, String> map = new HashMap<>();
+        map.put("tree.node2", "value2");
+
+        TestLazySource testLazySource = new TestLazySource(map);
+
+        Config config = Config.builder()
+                .addSource(testLazySource)
+                .addSource(ConfigSources.create(Map.of("tree.node1", "value1")))
+                .disableEnvironmentVariablesSource()
+                .disableSystemPropertiesSource()
+                .build();
+
+        assertThat(config.get("tree.node1").as(String.class).get(), is("value1"));
+
+        // when using lazy config source, we defer the loading of the key until it is actually requested
+        assertThat("tree.node2 should exist", config.get("tree.node2").exists(), is(true));
+        assertThat(config.get("tree.node2").as(String.class).get(), is("value2"));
+        assertThat("tree.node3 should not exist", config.get("tree.node3").exists(), is(false));
+
+        // config tree is immutable once created - so we ignore values that appear in the source later in time,
+        // as we have already resolved that the node is not present
+        map.put("tree.node3", "value3");
+        assertThat("tree.node3 should not exist, as it was already cached as not existing",
+                   config.get("tree.node3").exists(),
+                   is(false));
+
+        // each node should have been requested from the config source, starting from root
+        assertThat(testLazySource.requestedNodes, containsInAnyOrder("", "tree", "tree.node1", "tree.node2", "tree.node3"));
+    }
+
+    private static class TestLazySource implements LazyConfigSource, ConfigSource {
+        private final List<String> requestedNodes = new ArrayList<>();
+        private final Map<String, String> values;
+
+        private TestLazySource(Map<String, String> values) {
+            this.values = values;
+        }
+
+        @Override
+        public Optional<ConfigNode> node(String key) {
+            requestedNodes.add(key);
+            return Optional.ofNullable(values.get(key))
+                    .map(ConfigNode.ValueNode::create);
+        }
+    }
+}