diff --git a/config/config/src/main/java/io/helidon/config/BuilderImpl.java b/config/config/src/main/java/io/helidon/config/BuilderImpl.java index ceb65a1e040..7e3bcd7910f 100644 --- a/config/config/src/main/java/io/helidon/config/BuilderImpl.java +++ b/config/config/src/main/java/io/helidon/config/BuilderImpl.java @@ -91,7 +91,9 @@ class BuilderImpl implements Config.Builder { */ private boolean cachingEnabled; private boolean keyResolving; + private boolean keyResolvingFailOnMissing; private boolean valueResolving; + private boolean valueResolvingFailOnMissing; private boolean systemPropertiesSourceEnabled; private boolean environmentVariablesSourceEnabled; private boolean envVarAliasGeneratorEnabled; @@ -262,12 +264,24 @@ public Config.Builder disableKeyResolving() { return this; } + @Override + public Config.Builder failOnMissingKeyReference(boolean shouldFail) { + this.keyResolvingFailOnMissing = shouldFail; + return this; + } + @Override public Config.Builder disableValueResolving() { this.valueResolving = false; return this; } + @Override + public Config.Builder failOnMissingValueReference(boolean shouldFail) { + this.valueResolvingFailOnMissing = shouldFail; + return this; + } + @Override public Config.Builder disableEnvironmentVariablesSource() { environmentVariablesSourceEnabled = false; @@ -283,7 +297,7 @@ public Config.Builder disableSystemPropertiesSource() { @Override public AbstractConfigImpl build() { if (valueResolving) { - addFilter(ConfigFilters.valueResolving()); + addFilter(ConfigFilters.valueResolving().failOnMissingReference(valueResolvingFailOnMissing)); } if (null == changesExecutor) { changesExecutor = Executors.newCachedThreadPool(new ConfigThreadFactory("config-changes")); @@ -330,6 +344,7 @@ public AbstractConfigImpl build() { cachingEnabled, changesExecutor, keyResolving, + keyResolvingFailOnMissing, aliasGenerator) .newConfig(); } @@ -440,6 +455,7 @@ ProviderImpl createProvider(ConfigMapperManager configMapperManager, boolean cachingEnabled, Executor changesExecutor, boolean keyResolving, + boolean keyResolvingFailOnMissing, Function> aliasGenerator) { return new ProviderImpl(configMapperManager, targetConfigSource, @@ -448,6 +464,7 @@ ProviderImpl createProvider(ConfigMapperManager configMapperManager, cachingEnabled, changesExecutor, keyResolving, + keyResolvingFailOnMissing, aliasGenerator); } diff --git a/config/config/src/main/java/io/helidon/config/Config.java b/config/config/src/main/java/io/helidon/config/Config.java index 5f23d119f18..59485068bd5 100644 --- a/config/config/src/main/java/io/helidon/config/Config.java +++ b/config/config/src/main/java/io/helidon/config/Config.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2022 Oracle and/or its affiliates. + * Copyright (c) 2017, 2023 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. @@ -1314,6 +1314,16 @@ default Builder sources(Supplier configSource, */ Builder disableKeyResolving(); + /** + * When key resolving is enabled and a reference cannot be resolved, should we fail, or use the key verbatim. + * Defaults to {@code false}, so key resolving does not fail when a reference is missing. + * + * @param shouldFail whether to fail when key reference cannot be resolved + * @return updated builder + * @see #disableKeyResolving() + */ + Builder failOnMissingKeyReference(boolean shouldFail); + /** * Disables an usage of resolving value tokens. *

@@ -1326,6 +1336,16 @@ default Builder sources(Supplier configSource, */ Builder disableValueResolving(); + /** + * When value resolving is enabled and a reference cannot be resolved, should we fail, or use the value verbatim. + * Defaults to {@code false}, so value resolving does not fail when a reference is missing. + * + * @param shouldFail whether to fail when value reference cannot be resolved + * @return updated builder + * @see #disableValueResolving() + */ + Builder failOnMissingValueReference(boolean shouldFail); + /** * Disables use of {@link ConfigSources#environmentVariables() environment variables config source}. * diff --git a/config/config/src/main/java/io/helidon/config/ConfigSources.java b/config/config/src/main/java/io/helidon/config/ConfigSources.java index 055a3da44a9..cd599903180 100644 --- a/config/config/src/main/java/io/helidon/config/ConfigSources.java +++ b/config/config/src/main/java/io/helidon/config/ConfigSources.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2022 Oracle and/or its affiliates. + * Copyright (c) 2017, 2023 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. @@ -70,7 +70,7 @@ public static ConfigSource empty() { * @return {@code ConfigSource} for the same {@code Config} as the original */ public static ConfigSource create(Config config) { - return ConfigSources.create(config.asMap().get()).get(); + return create(ObjectNodeImpl.create(config)); } /** diff --git a/config/config/src/main/java/io/helidon/config/ObjectNodeImpl.java b/config/config/src/main/java/io/helidon/config/ObjectNodeImpl.java index fc2985413df..ba245e01ecd 100644 --- a/config/config/src/main/java/io/helidon/config/ObjectNodeImpl.java +++ b/config/config/src/main/java/io/helidon/config/ObjectNodeImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. + * Copyright (c) 2020, 2023 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,14 @@ public Set> entrySet() { return members.entrySet(); } + static ObjectNode create(Config config) { + ObjectNode.Builder root = ObjectNode.builder(); + + addObjectNode(root, config); + + return root.build(); + } + static void initDescription(ConfigNode node, String description) { switch (node.nodeType()) { case OBJECT: @@ -105,6 +113,56 @@ public MergeableNode merge(MergeableNode node) { } } + private static void addObjectNode(Builder parentBuilder, Config parent) { + parent.asNodeList().ifPresent(it -> { + for (Config child : it) { + switch (child.type()) { + case OBJECT -> { + Builder childBuilder = ObjectNode.builder(); + addObjectNode(childBuilder, child); + parentBuilder.addObject(child.name(), childBuilder.build()); + } + case LIST -> { + ListNode.Builder childBuilder = ListNode.builder(); + addListNode(childBuilder, child); + parentBuilder.addList(child.name(), childBuilder.build()); + } + case VALUE -> { + parentBuilder.addValue(child.name(), child.asString().get()); + } + default -> { + // do nothing + } + } + } + }); + } + + private static void addListNode(ListNode.Builder parentBuilder, Config parent) { + parent.asNodeList().ifPresent(it -> { + for (Config child : it) { + switch (child.type()) { + case OBJECT -> { + Builder childBuilder = ObjectNode.builder(); + addObjectNode(childBuilder, child); + parentBuilder.addObject(childBuilder.build()); + } + case LIST -> { + ListNode.Builder childBuilder = ListNode.builder(); + addListNode(childBuilder, child); + parentBuilder.addList(childBuilder.build()); + } + case VALUE -> { + parentBuilder.addValue(child.asString().get()); + } + default -> { + // do nothing + } + } + } + }); + } + private MergeableNode mergeWithValueNode(ValueNodeImpl node) { ObjectNodeBuilderImpl builder = ObjectNodeBuilderImpl.create(members, resolveTokenFunction); builder.value(node.value()); diff --git a/config/config/src/main/java/io/helidon/config/ProviderImpl.java b/config/config/src/main/java/io/helidon/config/ProviderImpl.java index 3bf20d10f1e..03cae301729 100644 --- a/config/config/src/main/java/io/helidon/config/ProviderImpl.java +++ b/config/config/src/main/java/io/helidon/config/ProviderImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2022 Oracle and/or its affiliates. + * Copyright (c) 2017, 2023 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. @@ -53,6 +53,7 @@ class ProviderImpl implements Config.Context { private final Executor changesExecutor; private final boolean keyResolving; + private final boolean keyResolvingFailOnMissing; private final Function> aliasGenerator; private ConfigDiff lastConfigsDiff; @@ -67,6 +68,7 @@ class ProviderImpl implements Config.Context { boolean cachingEnabled, Executor changesExecutor, boolean keyResolving, + boolean keyResolvingFailOnMissing, Function> aliasGenerator) { this.configMapperManager = configMapperManager; this.configSource = configSource; @@ -79,6 +81,7 @@ class ProviderImpl implements Config.Context { this.lastConfig = (AbstractConfigImpl) Config.empty(); this.keyResolving = keyResolving; + this.keyResolvingFailOnMissing = keyResolvingFailOnMissing; this.aliasGenerator = aliasGenerator; } @@ -152,10 +155,22 @@ private ObjectNode resolveKeys(ObjectNode rootNode) { } Map tokenValueMap = tokenToValueMap(flattenValueNodes); + boolean failOnMissingKeyReference = getBoolean(flattenValueNodes, + "config.key-resolving.fail-on-missing-reference", + keyResolvingFailOnMissing); resolveTokenFunction = (token) -> { if (token.startsWith("$")) { - return tokenValueMap.get(parseTokenReference(token)); + String tokenRef = parseTokenReference(token); + String resolvedValue = tokenValueMap.get(tokenRef); + if (resolvedValue.isEmpty()) { + if (failOnMissingKeyReference) { + throw new ConfigException(String.format("Missing token '%s' to resolve a key reference.", tokenRef)); + } else { + return token; + } + } + return resolvedValue; } return token; }; @@ -163,25 +178,33 @@ private ObjectNode resolveKeys(ObjectNode rootNode) { return ObjectNodeBuilderImpl.create(rootNode, resolveTokenFunction).build(); } + /* + * Returns a map of required replacement tokens to their respective values from the current config tree. + * The values may be empty strings, representing unresolved references. + */ private Map tokenToValueMap(Map flattenValueNodes) { return flattenValueNodes.keySet() .stream() .flatMap(this::tokensFromKey) .distinct() - .collect(Collectors.toMap(Function.identity(), t -> - flattenValueNodes.compute(Config.Key.unescapeName(t), (k, v) -> { - if (v == null) { - throw new ConfigException(String.format("Missing token '%s' to resolve.", t)); - } else if (v.equals("")) { - throw new ConfigException(String.format("Missing value in token '%s' definition.", t)); - } else if (v.startsWith("$")) { - throw new ConfigException(String.format( - "Key token '%s' references to a reference in value. A recursive references is not " - + "allowed.", - t)); - } - return Config.Key.escapeName(v); - }))); + .collect(Collectors.toMap(Function.identity(), t -> { + // t is the reference we need to resolve + // we cannot use compute, as that modifies the map we are currently navigating + String value = flattenValueNodes.get(Config.Key.unescapeName(t)); + if (value == null) { + value = ""; + } else { + if (value.startsWith("$")) { + throw new ConfigException(String.format( + "Key token '%s' references to a reference in value. A recursive" + + " references is not allowed.", + t)); + } + value = Config.Key.escapeName(value); + } + // either null (not found), or escaped value + return value; + })); } private Stream tokensFromKey(String s) { @@ -248,6 +271,14 @@ private void initializeFilters(Config config, ChainConfigFilter chain) { .forEachOrdered(filter -> filter.init(config)); } + private boolean getBoolean(Map valueNodes, String key, boolean defaultValue) { + String value = valueNodes.get(key); + if (value == null) { + return defaultValue; + } + return Boolean.parseBoolean(value); + } + /** * Config filter chain that can combine a collection of {@link ConfigFilter} and wrap them into one config filter. */ diff --git a/config/config/src/test/java/io/helidon/config/BuilderImplTest.java b/config/config/src/test/java/io/helidon/config/BuilderImplTest.java index d74ceafb0fc..5b9ad47b3f8 100644 --- a/config/config/src/test/java/io/helidon/config/BuilderImplTest.java +++ b/config/config/src/test/java/io/helidon/config/BuilderImplTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2022 Oracle and/or its affiliates. + * Copyright (c) 2017, 2023 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. @@ -67,6 +67,7 @@ public void testBuildDefault() { eq(true), //cachingEnabled notNull(), //changesExecutor eq(true), //keyResolving + eq(false), // fail when key resolving cannot find ref isNull() //aliasGenerator ); } @@ -91,6 +92,7 @@ public void testBuildCustomChanges() { eq(true), //cachingEnabled eq(myExecutor), //changesExecutor eq(true), //keyResolving + eq(false), // fail when key resolving cannot find ref isNull() //aliasGenerator ); } @@ -116,6 +118,7 @@ public void testBuildDisableKeyResolving() { eq(true), //cachingEnabled eq(myExecutor), //changesExecutor eq(false), //keyResolving + eq(false), // fail when key resolving cannot find ref isNull() //aliasGenerator ); } diff --git a/config/config/src/test/java/io/helidon/config/ConfigCopyTest.java b/config/config/src/test/java/io/helidon/config/ConfigCopyTest.java new file mode 100644 index 00000000000..6ae20079b28 --- /dev/null +++ b/config/config/src/test/java/io/helidon/config/ConfigCopyTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2023 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; + +import io.helidon.config.spi.ConfigNode; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +/* + * Make sure that when we copy a config that uses list nodes, the list nodes are honored in the copy as well. + */ +class ConfigCopyTest { + @Test + void testCopyWithArray() { + ConfigNode.ListNode listNode = ConfigNode.ListNode.builder() + .addValue("bbb") + .addValue("ccc") + .addValue("ddd") + .build(); + + ConfigNode.ObjectNode otherObject = ConfigNode.ObjectNode.builder() + .addValue("first", "firstValue") + .addValue("second", "secondValue") + .build(); + + ConfigNode.ObjectNode rootNode = ConfigNode.ObjectNode.builder() + .addList("aaa", listNode) + .addValue("value", "some value") + .addObject("object", otherObject) + .build(); + + Config original = Config.builder() + .disableSystemPropertiesSource() + .disableEnvironmentVariablesSource() + .addSource(io.helidon.config.ConfigSources.create(rootNode)) + .build(); + + assertThat("Node type of original aaa", + original.get("aaa").type(), + is(Config.Type.LIST)); + + Config copy = Config.builder() + .disableSystemPropertiesSource() + .disableEnvironmentVariablesSource() + .addSource(ConfigSources.create(original)) + .build(); + + assertThat("Node type of copied aaa", + copy.get("aaa").type(), + is(Config.Type.LIST)); + + // now also assert that all values were correctly copied + assertThat(copy.get("value").asString(), is(ConfigValues.simpleValue("some value"))); + assertThat(copy.get("object.first").asString(), is(ConfigValues.simpleValue("firstValue"))); + assertThat(copy.get("object.second").asString(), is(ConfigValues.simpleValue("secondValue"))); + assertThat(copy.get("aaa.0").asString(), is(ConfigValues.simpleValue("bbb"))); + } +} diff --git a/config/config/src/test/java/io/helidon/config/KeyTokenResolvingTest.java b/config/config/src/test/java/io/helidon/config/KeyTokenResolvingTest.java index f7b13846169..7d2db8431a4 100644 --- a/config/config/src/test/java/io/helidon/config/KeyTokenResolvingTest.java +++ b/config/config/src/test/java/io/helidon/config/KeyTokenResolvingTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2021 Oracle and/or its affiliates. + * Copyright (c) 2017, 2023 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. @@ -182,11 +182,12 @@ public void testResolveChainedTokensConfig() { .build())) .disableSystemPropertiesSource() .disableEnvironmentVariablesSource() + .failOnMissingKeyReference(true) .build(); config.traverse().forEach(System.out::println); }); - assertThat(ex.getMessage(), startsWith("Missing value in token 'region' definition")); + assertThat(ex.getMessage(), startsWith("Missing token 'region' to resolve a key reference")); } @Test @@ -199,6 +200,7 @@ public void testResolveTokenMissingValue() { .build())) .disableSystemPropertiesSource() .disableEnvironmentVariablesSource() + .failOnMissingKeyReference(true) .build(); }); assertThat(ex.getMessage(), startsWith("Missing token 'ad' to resolve")); diff --git a/config/yaml/src/test/java/io/helidon/config/yaml/IgnoreRefsTest.java b/config/yaml/src/test/java/io/helidon/config/yaml/IgnoreRefsTest.java new file mode 100644 index 00000000000..c844e97b474 --- /dev/null +++ b/config/yaml/src/test/java/io/helidon/config/yaml/IgnoreRefsTest.java @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2023 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.yaml; + +import io.helidon.config.Config; +import io.helidon.config.ConfigException; +import io.helidon.config.ConfigSources; +import io.helidon.config.ConfigValues; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/* + * This test could be done in main config module, but the problem was reported for yaml, + * and I wanted to make sure we use the exact YAML from the issue. + */ +class IgnoreRefsTest { + @Test + void testKeyRefsAreIgnored() { + Config config = Config.builder() + .addSource(ConfigSources.classpath("ignore-refs.yaml")) + .disableEnvironmentVariablesSource() + .disableSystemPropertiesSource() + .build(); + + assertThat(config.get("server.port").as(Integer.class), is(ConfigValues.simpleValue(8080))); + assertThat(config.get("openapi.schemas.Pet.content.application/json.schema.$ref").asString(), + is(ConfigValues.simpleValue("#/components/schemas/Pet"))); + } + + @Test + void testKeyRefsAreNotIgnored() { + Config.Builder configBuilder = Config.builder() + .addSource(ConfigSources.classpath("ignore-refs-2.yaml")) + .failOnMissingKeyReference(true) + .disableEnvironmentVariablesSource() + .disableSystemPropertiesSource(); + + ConfigException e = assertThrows(ConfigException.class, configBuilder::build); + assertThat(e.getMessage(), is("Missing token 'ref' to resolve a key reference.")); + } + + @Test + void testKeyRefsAreIgnoredFromBuilder() { + Config config = Config.builder() + .addSource(ConfigSources.classpath("ignore-refs-2.yaml")) + .disableEnvironmentVariablesSource() + .disableSystemPropertiesSource() + .build(); + assertThat(config.get("server.port").as(Integer.class), is(ConfigValues.simpleValue(8080))); + assertThat(config.get("openapi.schemas.Pet.content.application/json.schema.$ref").asString(), + is(ConfigValues.simpleValue("#/components/schemas/Pet"))); + } +} diff --git a/config/yaml/src/test/resources/ignore-refs-2.yaml b/config/yaml/src/test/resources/ignore-refs-2.yaml new file mode 100644 index 00000000000..815105bbc42 --- /dev/null +++ b/config/yaml/src/test/resources/ignore-refs-2.yaml @@ -0,0 +1,30 @@ +# +# Copyright (c) 2023 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. +# + +server: + port: 8080 +openapi: + schemas: + Pet: + content: + application/json: + schema: + "$ref": '#/components/schemas/Pet' + application/xml: + schema: + $ref: '#/components/schemas/Pet' + description: Pet object that needs to be added to the store + required: true \ No newline at end of file diff --git a/config/yaml/src/test/resources/ignore-refs.yaml b/config/yaml/src/test/resources/ignore-refs.yaml new file mode 100644 index 00000000000..4e6e5e11bb8 --- /dev/null +++ b/config/yaml/src/test/resources/ignore-refs.yaml @@ -0,0 +1,33 @@ +# +# Copyright (c) 2023 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. +# + +config: + key-resolving: + fail-on-missing-reference: false +server: + port: 8080 +openapi: + schemas: + Pet: + content: + application/json: + schema: + "$ref": '#/components/schemas/Pet' + application/xml: + schema: + $ref: '#/components/schemas/Pet' + description: Pet object that needs to be added to the store + required: true \ No newline at end of file