From 1f695b8b18476fa14a87651cf34e7907d1a723df Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 31 Jan 2024 15:08:51 +0100 Subject: [PATCH 1/2] test(mpconfig): add failing unit test for #6549 --- .../spi/ConfigExpressionResolverTest.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java b/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java index a7759ce5e74..4b3bcd377ea 100644 --- a/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java +++ b/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java @@ -44,7 +44,9 @@ import org.junit.Before; import org.junit.Test; +import java.util.Collections; import java.util.HashMap; +import java.util.Set; import static fish.payara.nucleus.microprofile.config.spi.ConfigTestUtils.createSource; import static java.util.Collections.singleton; @@ -55,7 +57,8 @@ public class ConfigExpressionResolverTest { private final ConfigSource source = createSource("S1", 100, new HashMap<>()); - private final ConfigExpressionResolver resolver = new ConfigExpressionResolver(singleton(source), "test"); + private final ConfigSource source2 = createSource("S2", 110, new HashMap<>()); + private final ConfigExpressionResolver resolver = new ConfigExpressionResolver(Set.of(source, source2), "test"); @Before public void configureConfigProperties() { @@ -74,6 +77,10 @@ public void configureConfigProperties() { source.getProperties().put("default.key.reference", "${${not.existing:key}:not.found}"); source.getProperties().put("%test.fish.payara.badger", "mushroom"); source.getProperties().put("fish.payara.badger", "badger"); + source.getProperties().put("%test.fish.payara.rod", "bites"); + + source2.getProperties().put("fish.payara.rod", "nobites"); + source2.getProperties().put("%test.fish.payara.profile-only", "gotcha"); } @Test @@ -185,4 +192,18 @@ public void testProfiles() { assertEquals("mushroom", result.getRawValue()); } + @Test + public void testProfilesOverrideProfiledValueFromSourceWithHigherOrdinal() { + ConfigValue result = resolver.resolve("fish.payara.rod"); + assertEquals("nobites", result.getValue()); + assertEquals("nobites", result.getRawValue()); + } + + @Test + public void testProfilesDoNotFailToLookupWhenOnlyProfiledValueExists() { + ConfigValue result = resolver.resolve("fish.payara.profile-only"); + assertEquals("gotcha", result.getValue()); + assertEquals("gotcha", result.getRawValue()); + } + } From 9baaeb8dc9793f90b56bf1f1e26fbeefac532441 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 31 Jan 2024 15:45:34 +0100 Subject: [PATCH 2/2] fix(mpconfig): do not throw NPE when profiled setting only #6954 Also simplifies the check in case of a same ordinal value source is around. Left comments and extended unit test to make sure the desired behaviour is tested. --- .../config/spi/ConfigExpressionResolver.java | 8 ++++++-- .../config/spi/ConfigExpressionResolverTest.java | 16 +++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolver.java b/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolver.java index 7fd03f9c2bf..a7ad231a3ac 100644 --- a/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolver.java +++ b/nucleus/payara-modules/nucleus-microprofile/config-service/src/main/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolver.java @@ -109,8 +109,12 @@ private ConfigValueImpl resolve(String propertyName, String propertyDefault, boo if(profile != null && result != null) { ConfigValueImpl resultWithoutProfile = getValue(resolveExpression(propertyName)); - result = resultWithoutProfile.getSourceOrdinal() == result.getSourceOrdinal() ? result : - (resultWithoutProfile.getSourceOrdinal() > result.getSourceOrdinal()) ? resultWithoutProfile : result; + // Note: In case there is a non-profiled value from a source with the same ordinal value, it will be ignored. + // All spec versions including v3.1 do not include a definition for this edge case - + // all sources are supposed to have a unique ordinal value. + if (resultWithoutProfile != null && resultWithoutProfile.getSourceOrdinal() > result.getSourceOrdinal()) { + result = resultWithoutProfile; + } } diff --git a/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java b/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java index 4b3bcd377ea..b396016996e 100644 --- a/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java +++ b/nucleus/payara-modules/nucleus-microprofile/config-service/src/test/java/fish/payara/nucleus/microprofile/config/spi/ConfigExpressionResolverTest.java @@ -44,7 +44,6 @@ import org.junit.Before; import org.junit.Test; -import java.util.Collections; import java.util.HashMap; import java.util.Set; @@ -57,8 +56,10 @@ public class ConfigExpressionResolverTest { private final ConfigSource source = createSource("S1", 100, new HashMap<>()); - private final ConfigSource source2 = createSource("S2", 110, new HashMap<>()); - private final ConfigExpressionResolver resolver = new ConfigExpressionResolver(Set.of(source, source2), "test"); + private final ConfigSource sourceHigherOrdinal = createSource("SHO", 110, new HashMap<>()); + private final ConfigSource sourceEqualOrdinal = createSource("SEO", 100, new HashMap<>()); + private final ConfigSource sourceLowerOrdinal = createSource("SEO", 90, new HashMap<>()); + private final ConfigExpressionResolver resolver = new ConfigExpressionResolver(Set.of(source, sourceHigherOrdinal, sourceEqualOrdinal, sourceLowerOrdinal), "test"); @Before public void configureConfigProperties() { @@ -79,8 +80,11 @@ public void configureConfigProperties() { source.getProperties().put("fish.payara.badger", "badger"); source.getProperties().put("%test.fish.payara.rod", "bites"); - source2.getProperties().put("fish.payara.rod", "nobites"); - source2.getProperties().put("%test.fish.payara.profile-only", "gotcha"); + sourceHigherOrdinal.getProperties().put("fish.payara.rod", "nobites"); + sourceHigherOrdinal.getProperties().put("%test.fish.payara.profile-only", "gotcha"); + + sourceEqualOrdinal.getProperties().put("fish.payara.badger", "i-shall-be-ignored"); + sourceLowerOrdinal.getProperties().put("fish.payara.badger", "i-shall-be-ignored"); } @Test @@ -187,6 +191,8 @@ public void testExpressionExpansionDisabled() { @Test public void testProfiles() { + // This test case does not only test if the profiled value is used from the same source, but also if the + // defined equal and lower ordinal sources are ignored ConfigValue result = resolver.resolve("fish.payara.badger"); assertEquals("mushroom", result.getValue()); assertEquals("mushroom", result.getRawValue());