From b63254ce572c8153d6323a1a312f2a785a2cf563 Mon Sep 17 00:00:00 2001 From: Marek Winkler Date: Wed, 26 Aug 2026 10:43:55 +0200 Subject: [PATCH] fix: false positive warnings for new properties (#2617) The warning must only be logged when a legacy value is actually chosen over the new one, not merely because the requested name happens to have a known legacy counterpart (which may itself be unset). Closes https://github.com/TimefoldAI/timefold-solver/issues/2616. --- ...dRenamedPropertiesFallbackInterceptor.java | 18 +++-- ...amedPropertiesFallbackInterceptorTest.java | 76 +++++++++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptor.java b/service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptor.java index c430ff7947..896ea23eaa 100644 --- a/service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptor.java +++ b/service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptor.java @@ -60,14 +60,11 @@ public TimefoldRenamedPropertiesFallbackInterceptor() { private static String toLegacyName(String name) { var exactLegacyName = RENAMED_EXACT_PROPERTIES.get(name); if (exactLegacyName != null) { - logWarningForResolvedKey(name, exactLegacyName); return exactLegacyName; } for (Map.Entry prefix : RENAMED_PROPERTY_PREFIXES.entrySet()) { if (name.startsWith(prefix.getKey())) { - var legacyName = prefix.getValue() + name.substring(prefix.getKey().length()); - logWarningForResolvedKey(name, legacyName); - return legacyName; + return prefix.getValue() + name.substring(prefix.getKey().length()); } } return name; @@ -76,10 +73,19 @@ private static String toLegacyName(String name) { @Override public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) { List legacyNames = RENAMED_PROPERTIES_WITH_MULTIPLE_LEGACY_NAMES.get(name); - if (legacyNames == null) { - return super.getValue(context, name); + if (legacyNames != null) { + return resolveWithLegacyNames(context, name, legacyNames); + } + + var legacyName = toLegacyName(name); + if (legacyName.equals(name)) { + return context.proceed(name); } + return resolveWithLegacyNames(context, name, List.of(legacyName)); + } + private static ConfigValue resolveWithLegacyNames(ConfigSourceInterceptorContext context, String name, + List legacyNames) { ConfigValue value = context.proceed(name); for (String legacyName : legacyNames) { ConfigValue legacyValue = context.proceed(legacyName); diff --git a/service/quarkus/runtime/src/test/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptorTest.java b/service/quarkus/runtime/src/test/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptorTest.java index c93d6d59c1..d4875488dd 100644 --- a/service/quarkus/runtime/src/test/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptorTest.java +++ b/service/quarkus/runtime/src/test/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptorTest.java @@ -2,9 +2,19 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.ArrayList; +import java.util.List; import java.util.Map; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import io.smallrye.config.PropertiesConfigSource; import io.smallrye.config.SmallRyeConfig; @@ -19,6 +29,53 @@ private static SmallRyeConfig buildConfig(Map properties) { .build(); } + private static List captureLogs(Runnable action) { + Logger julLogger = Logger.getLogger(TimefoldRenamedPropertiesFallbackInterceptor.class.getName()); + List logRecords = new ArrayList<>(); + Handler handler = new Handler() { + @Override + public void publish(LogRecord logRecord) { + logRecords.add(logRecord); + } + + @Override + public void flush() { + // nothing to flush + } + + @Override + public void close() { + // nothing to close + } + }; + Level originalLevel = julLogger.getLevel(); + julLogger.setLevel(Level.ALL); + julLogger.addHandler(handler); + try { + action.run(); + } finally { + julLogger.removeHandler(handler); + julLogger.setLevel(originalLevel); + } + return logRecords; + } + + /** + * Returns a single pair of new-property-name, deprecated-property-name for every mapping type defined + * in {@link TimefoldRenamedPropertiesFallbackInterceptor}. + * + * @return a stream of arguments for parameterized tests representing each property mapping type + */ + private static Stream renamedProperties() { + return Stream.of( + // RENAMED_EXACT_PROPERTIES + Arguments.of("timefold.model.name", "timefold.application.name"), + // RENAMED_PROPERTY_PREFIXES + Arguments.of("timefold.model.default-config.something", "ai.timefold.model.default-config.something"), + // RENAMED_PROPERTIES_WITH_MULTIPLE_LEGACY_NAMES + Arguments.of("timefold.model.id", "timefold.application.id")); + } + @Test void fallsBackToExactLegacyName() { SmallRyeConfig config = buildConfig(Map.of("timefold.application.name", "my-app")); @@ -119,4 +176,23 @@ void unknownPropertyReturnsNull() { SmallRyeConfig config = buildConfig(Map.of()); assertThat(config.getRawValue("timefold.model.name")).isNull(); } + + @ParameterizedTest + @MethodSource("renamedProperties") + void logsWarningWhenLegacyPropertyIsDefined(String newName, String legacyName) { + SmallRyeConfig config = buildConfig(Map.of(legacyName, "my-value")); + List logRecords = captureLogs(() -> config.getRawValue(newName)); + + assertThat(logRecords) + .anySatisfy(logRecord -> assertThat(logRecord.getMessage()).contains("Deprecated configuration key")); + } + + @ParameterizedTest + @MethodSource("renamedProperties") + void doesNotLogWarningWhenOnlyNewPropertyIsDefined(String newName, String legacyName) { + SmallRyeConfig config = buildConfig(Map.of(newName, "my-value")); + List logRecords = captureLogs(() -> config.getRawValue(newName)); + + assertThat(logRecords).isEmpty(); + } } \ No newline at end of file