4.x: Upgrade to MP Config 3.1 and fix an issue with profile specific …

…properties (#8757) * Correctly handle profile-specific properties * Add test for profile specific properties * Upgrade MP Config to 3.1 * Return a ConfigValue even when expression on rhs does not resolve * Update docs for microprofile config 3.1 * Add ConfigValue test for missing expression
helidon-io · May 29, 2024 · d0d7c64 · d0d7c64
1 parent cf82198
commit d0d7c64
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 21 deletions.
diff --git a/config/config-mp/src/main/java/io/helidon/config/mp/MpConfigImpl.java b/config/config-mp/src/main/java/io/helidon/config/mp/MpConfigImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2023 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.
@@ -107,13 +107,18 @@ class MpConfigImpl implements Config {
 
     @Override
     public ConfigValue getConfigValue(String key) {
+
+        ConfigValue value = findConfigValue(key)
+                .orElse(new ConfigValueImpl(key, null, null, null, 0));
+
         if (configProfile == null) {
-            return findConfigValue(key)
-                    .orElseGet(() -> new ConfigValueImpl(key, null, null, null, 0));
+            return value;
         }
-        return findConfigValue("%" + configProfile + "." + key)
-                .or(() -> findConfigValue(key))
-                .orElseGet(() -> new ConfigValueImpl(key, null, null, null, 0));
+
+        ConfigValue profileValue = findConfigValue("%" + configProfile + "." + key)
+                .orElse(value);
+
+        return value.getSourceOrdinal() > profileValue.getSourceOrdinal() ? value : profileValue;
     }
 
     @Override
@@ -126,12 +131,7 @@ public <T> T getValue(String propertyName, Class<T> propertyType) {
     @SuppressWarnings("unchecked")
     @Override
     public <T> Optional<T> getOptionalValue(String propertyName, Class<T> propertyType) {
-        if (configProfile == null) {
-            return optionalValue(propertyName, propertyType);
-        }
-
-        return optionalValue("%" + configProfile + "." + propertyName, propertyType)
-                .or(() -> optionalValue(propertyName, propertyType));
+        return optionalValue(propertyName, propertyType);
     }
 
     @SuppressWarnings("unchecked")
@@ -187,9 +187,9 @@ private <T> Optional<T> optionalValue(String propertyName, Class<T> propertyType
                 return Optional.empty();
             }
         } else {
-            return findConfigValue(propertyName)
-                    .map(ConfigValue::getValue)
-                    .map(it -> convert(propertyName, propertyType, it));
+            Optional<ConfigValue> value = Optional.of(getConfigValue(propertyName));
+            return value.map(ConfigValue::getValue)
+                 .map(it -> convert(propertyName, propertyType, it));
         }
     }
 
@@ -314,6 +314,7 @@ private <T> T convert(String propertyName, Class<T> type, String value) {
     }
 
     private Optional<ConfigValue> findConfigValue(String propertyName) {
+
         for (ConfigSource source : sources) {
             String value = source.getValue(propertyName);
 
@@ -341,7 +342,7 @@ private Optional<ConfigValue> findConfigValue(String propertyName) {
                         .map(it -> new ConfigValueImpl(propertyName, it, rawValue, source.getName(), source.getOrdinal()));
             } catch (NoSuchElementException e) {
                 // Property expression does not resolve
-                return Optional.empty();
+                return Optional.of(new ConfigValueImpl(propertyName, null, rawValue, source.getName(), source.getOrdinal()));
             }
         }
 

diff --git a/config/config-mp/src/test/java/io/helidon/config/mp/MpConfigReferenceTest.java b/config/config-mp/src/test/java/io/helidon/config/mp/MpConfigReferenceTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2023 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.
@@ -20,13 +20,16 @@
 import java.util.Optional;
 
 import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigValue;
 import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
+import static org.hamcrest.CoreMatchers.endsWith;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.nullValue;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class MpConfigReferenceTest {
@@ -65,6 +68,15 @@ void testMissingRefs() {
         // since Config 2.0, missing references must throw an exception
         assertThrows(NoSuchElementException.class, () -> config.getValue("referencing4-1", String.class));
         assertThrows(NoSuchElementException.class, () -> config.getValue( "referencing4-2", String.class));
+
+        // MP Config 3.1 TCK requires well-formed ConfigValue when missing reference
+        ConfigValue configValue = config.getConfigValue("referencing4-1");
+        assertThat(configValue, notNullValue());
+        assertThat(configValue.getName(), is("referencing4-1"));
+        assertThat(configValue.getValue(), nullValue());
+        assertThat(configValue.getRawValue(), is("${missing}"));
+        assertThat(configValue.getSourceName(), endsWith("microprofile-config.properties"));
+        assertThat(configValue.getSourceOrdinal(), is(100));
     }
 
     @Test

diff --git a/config/config-mp/src/test/java/io/helidon/config/mp/MpConfigSourcesTest.java b/config/config-mp/src/test/java/io/helidon/config/mp/MpConfigSourcesTest.java
@@ -19,6 +19,9 @@
 import java.io.ByteArrayInputStream;
 import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
@@ -60,6 +63,52 @@ void testHelidonMap() {
         assertThat(mpSource.getValue("key.third"), is("<>{}().,:;/|\\~`?!@#$%^&*-=+*"));
     }
 
+    @Test
+    void testProfileSpecificProperty() {
+        Map<String, String> values = Map.of(
+            "%dev.vehicle.name", "car",
+            "vehicle.name", "bike",
+            "%dev.vehicle.color", "blue",
+            "vehicle.color", "red",
+            "%dev.vehicle.size", "large"
+        );
+        org.eclipse.microprofile.config.spi.ConfigSource mapSource = MpConfigSources.create(ConfigSources.create(values).build());
+        assertThat(mapSource.getOrdinal(), is(100));
+        assertThat(mapSource.getValue("vehicle.name"), is("bike"));
+
+        // One data source. The profile specific property should take precedence
+        MpConfigImpl config = new MpConfigImpl(List.of(mapSource), new HashMap<>(), Collections.emptyList(), "dev");
+        assertThat(config.getConfigValue("vehicle.name").getValue(), is("car"));
+        assertThat(config.getOptionalValue("vehicle.name", String.class).orElse("error"), is("car"));
+
+        System.setProperty("vehicle.name", "jet");
+        System.setProperty("%dev.vehicle.make", "tucker");
+        org.eclipse.microprofile.config.spi.ConfigSource propertySource = MpConfigSources.systemProperties();
+        assertThat(propertySource.getOrdinal(), is(400));
+        assertThat(propertySource.getValue("vehicle.name"), is("jet"));
+
+        // Create Config from both data sources with the "dev" profile
+        config = new MpConfigImpl(List.of(propertySource, mapSource), new HashMap<>(), Collections.emptyList(), "dev");
+
+        // The vanilla property in the higher ordinal data source should trump the profile specific property in the
+        // lower ordinal data source
+        assertThat(config.getConfigValue("vehicle.name").getValue(), is("jet"));
+        assertThat(config.getOptionalValue("vehicle.name", String.class).orElse("error"), is("jet"));
+
+        // Within one DataSource the profile specific property takes precedence
+        assertThat(config.getConfigValue("vehicle.color").getValue(), is("blue"));
+        assertThat(config.getOptionalValue("vehicle.color", String.class).orElse("error"), is("blue"));
+
+        // Make sure missing vanilla values do not mess things up
+        assertThat(config.getConfigValue("vehicle.size").getValue(), is("large"));
+        assertThat(config.getOptionalValue("vehicle.size", String.class).orElse("error"), is("large"));
+        assertThat(config.getConfigValue("vehicle.make").getValue(), is("tucker"));
+        assertThat(config.getOptionalValue("vehicle.make", String.class).orElse("error"), is("tucker"));
+
+        System.clearProperty("vehicle.name");
+        System.clearProperty("%dev.vehicle.name");
+    }
+
     @Test
     void testHelidonParsable() {
         ParsableImpl helidonSource = new ParsableImpl();

diff --git a/dependencies/pom.xml b/dependencies/pom.xml
@@ -104,8 +104,7 @@
         <version.lib.micronaut>3.8.7</version.lib.micronaut>
         <version.lib.micronaut.data>3.4.3</version.lib.micronaut.data>
         <version.lib.micronaut.sql>4.8.0</version.lib.micronaut.sql>
-        <!-- FIXME upgrade to 3.1 when it is released in Maven -->
-        <version.lib.microprofile-config>3.0.3</version.lib.microprofile-config>
+        <version.lib.microprofile-config>3.1</version.lib.microprofile-config>
         <!-- FIXME upgrade to 4.1 when it is released in Maven -->
         <version.lib.microprofile-fault-tolerance-api>4.0.2</version.lib.microprofile-fault-tolerance-api>
         <version.lib.microprofile-graphql>2.0</version.lib.microprofile-graphql>

diff --git a/docs/src/main/asciidoc/includes/attributes.adoc b/docs/src/main/asciidoc/includes/attributes.adoc
@@ -43,7 +43,7 @@ endif::[]
 
 // microprofile specifications
 :version-lib-microprofile-lra-api: 2.0
-:version-lib-microprofile-config: 3.0.3
+:version-lib-microprofile-config: 3.1
 :version-lib-microprofile-fault-tolerance-api: 4.0.2
 :version-lib-microprofile-graphql: 2.0
 :version-lib-microprofile-health: 4.0

diff --git a/docs/src/main/asciidoc/mp/config/introduction.adoc b/docs/src/main/asciidoc/mp/config/introduction.adoc
@@ -243,4 +243,4 @@ Step-by-step guide about using {spec-name} in your Helidon MP application.
 == Reference
 
 * link:{microprofile-config-spec-url}[{spec-name} Specifications]
-* link:{microprofile-fault-tolerance-javadoc-url}[{spec-name} Javadocs]
+* link:{microprofile-config-javadoc-url}[{spec-name} Javadocs]