From 554783076711a2ccc89897075b237abfd19ef61c Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Thu, 3 Aug 2023 17:13:35 +0100 Subject: [PATCH] Changed priority of the LoggingInterceptor to avoid logging profiled lookups (#968) --- .../LoggingConfigSourceInterceptor.java | 12 ++--- .../ProfileConfigSourceInterceptor.java | 11 ++--- .../LoggingConfigSourceInterceptorTest.java | 45 ++++++++++++------- 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java b/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java index ad9c6fa80..bfa22cbc0 100644 --- a/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java +++ b/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java @@ -4,7 +4,7 @@ import jakarta.annotation.Priority; -@Priority(Priorities.LIBRARY + 150) +@Priority(Priorities.LIBRARY + 250) public class LoggingConfigSourceInterceptor implements ConfigSourceInterceptor { private static final long serialVersionUID = 367246512037404779L; @@ -13,10 +13,11 @@ public ConfigValue getValue(final ConfigSourceInterceptorContext context, final try { // Unlocked keys will run here. ConfigValue configValue = doLocked(() -> context.proceed(name)); - if (configValue != null) + if (configValue != null) { ConfigLogging.log.lookup(configValue.getName(), configValue.getLocation(), configValue.getValue()); - else + } else { ConfigLogging.log.notFound(name); + } return configValue; } catch (SecurityException e) { // Handled next, to omit the values to log from the secret. @@ -24,10 +25,11 @@ public ConfigValue getValue(final ConfigSourceInterceptorContext context, final // Locked keys here. final ConfigValue secret = context.proceed(name); - if (secret != null) + if (secret != null) { ConfigLogging.log.lookup(secret.getName(), "secret", "secret"); - else + } else { ConfigLogging.log.notFound(name); + } return secret; } } diff --git a/implementation/src/main/java/io/smallrye/config/ProfileConfigSourceInterceptor.java b/implementation/src/main/java/io/smallrye/config/ProfileConfigSourceInterceptor.java index f331fb515..47736808e 100644 --- a/implementation/src/main/java/io/smallrye/config/ProfileConfigSourceInterceptor.java +++ b/implementation/src/main/java/io/smallrye/config/ProfileConfigSourceInterceptor.java @@ -10,7 +10,6 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.NoSuchElementException; import java.util.Set; import jakarta.annotation.Priority; @@ -36,13 +35,9 @@ public ConfigValue getValue(final ConfigSourceInterceptorContext context, final final String normalizeName = normalizeName(name); final ConfigValue profileValue = getProfileValue(context, normalizeName); if (profileValue != null) { - try { - final ConfigValue originalValue = context.proceed(normalizeName); - if (originalValue != null && CONFIG_SOURCE_COMPARATOR.compare(originalValue, profileValue) > 0) { - return originalValue; - } - } catch (final NoSuchElementException e) { - // We couldn't find the main property so we fallback to the profile property because it exists. + final ConfigValue originalValue = context.proceed(normalizeName); + if (originalValue != null && CONFIG_SOURCE_COMPARATOR.compare(originalValue, profileValue) > 0) { + return originalValue; } return profileValue.withName(normalizeName); } diff --git a/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java b/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java index f854d8378..a436613b8 100644 --- a/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java +++ b/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java @@ -3,6 +3,7 @@ import static io.smallrye.config.KeyValuesConfigSource.config; import static java.util.stream.Collectors.toList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -46,37 +47,47 @@ void interceptor() throws Exception { // This should not log the secret value: assertEquals("12345678", SecretKeys.doUnlocked(() -> config.getValue("secret", String.class))); - // 1st element is the SR profile parent lookup - // 2nd element is the MP profile lookup - // 3rd element is the SR profile lookup - // 4th element is the MP Property Expressions List logs = logCapture.records().stream().map(LogRecord::getMessage).collect(toList()); + for (String log : logs) { + System.out.println(log); + } // my.prop lookup - assertTrue(logs.get(4).startsWith("SRCFG01001")); - assertTrue(logs.get(4).contains("The config my.prop was loaded from ConfigValuePropertiesConfigSource")); - assertTrue(logs.get(4).contains(":1 with the value abc")); + assertTrue(logs.stream() + .anyMatch(log -> log.contains("The config my.prop was loaded from ConfigValuePropertiesConfigSource"))); + assertTrue(logs.stream().anyMatch(log -> log.contains(":1 with the value abc"))); // not.found lookup - assertEquals("SRCFG01002: The config not.found was not found", logs.get(5)); + assertTrue(logs.contains("SRCFG01002: The config not.found was not found")); // secret lookup, shows the key but hides the source and value - assertEquals("SRCFG01001: The config secret was loaded from secret with the value secret", logs.get(6)); + assertTrue(logs.contains("SRCFG01001: The config secret was loaded from secret with the value secret")); } @Test void expansion() { - final SmallRyeConfig config = new SmallRyeConfigBuilder() + SmallRyeConfig config = new SmallRyeConfigBuilder() .addDefaultInterceptors() .withInterceptors(new LoggingConfigSourceInterceptor()) .withSources(config("my.prop.expand", "${expand}", "expand", "1234")) .build(); assertEquals("1234", config.getRawValue("my.prop.expand")); - // 1st element is the SR profile parent lookup - // 2nd element is the MP profile lookup - // 3rd element is the SR profile lookup - // 4th element is the MP Property Expressions List logs = logCapture.records().stream().map(LogRecord::getMessage).collect(toList()); - assertEquals("SRCFG01001: The config my.prop.expand was loaded from KeyValuesConfigSource with the value ${expand}", - logs.get(4)); - assertEquals("SRCFG01001: The config expand was loaded from KeyValuesConfigSource with the value 1234", logs.get(5)); + assertTrue(logs.contains( + "SRCFG01001: The config my.prop.expand was loaded from KeyValuesConfigSource with the value ${expand}")); + assertTrue(logs.contains("SRCFG01001: The config expand was loaded from KeyValuesConfigSource with the value 1234")); + } + + @Test + void profiles() { + SmallRyeConfig config = new SmallRyeConfigBuilder() + .addDefaultInterceptors() + .withInterceptors(new LoggingConfigSourceInterceptor()) + .withSources(config("%prod.my.prop", "1234", "my.prop", "5678")) + .withProfile("prod") + .build(); + + assertEquals("1234", config.getRawValue("my.prop")); + List logs = logCapture.records().stream().map(LogRecord::getMessage).collect(toList()); + assertTrue(logs.contains("SRCFG01001: The config my.prop was loaded from KeyValuesConfigSource with the value 1234")); + assertFalse(logs.stream().anyMatch(log -> log.contains("%prod.my.prop"))); } }