fix: false positive warnings for new properties

[winklerm]

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 #2616.

[greyhairredbear]

Nice tests about the logging!

Q: Would it maybe make sense to have that parameterized (with the other new/legacy property names), or do you think this would basically just be duplicating the implementation i.e. the mapping from new name to legacy name?
(IMO, it could make sense in this instance, thus the suggestion)

[winklerm]

Nice tests about the logging!

Q: Would it maybe make sense to have that parameterized (with the other new/legacy property names), or do you think this would basically just be duplicating the implementation i.e. the mapping from new name to legacy name? (IMO, it could make sense in this instance, thus the suggestion)

@greyhairredbear Do you mean having the tests parameterized with all mapped properties in general, or just the logging tests?

[greyhairredbear]

@greyhairredbear Do you mean having the tests parameterized with all mapped properties in general, or just the logging tests?

I just thought about the logging here, but you've got a point, might as well extend it to the other tests 🤔, not really that much of a difference. But then, we might also descope it from this PR.

[winklerm]

@greyhairredbear Do you mean having the tests parameterized with all mapped properties in general, or just the logging tests?

I just thought about the logging here, but you've got a point, might as well extend it to the other tests 🤔, not really that much of a difference. But then, we might also descope it from this PR.

Got it, updated the logging tests. The remaining tests seem to already cover different mapping types, if I have noticed correctly, so we should be good there.

[Copilot]

Pull request overview

This PR adjusts the Quarkus config fallback interceptor so deprecation warnings are only logged when a legacy (deprecated) configuration key actually wins during resolution, eliminating false-positive warnings described in #2616.

Changes:

• Move warning emission to the point where a legacy value is actually selected over the new key’s value.
• Consolidate legacy-key resolution logic into a shared resolver method for exact, prefix, and multi-legacy mappings.
• Add parameterized tests to verify warnings are logged only when legacy keys are used, and not when only the new key is set.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptor.java	Updates legacy-name resolution so WARN is emitted only when a legacy ConfigValue is actually selected.
service/quarkus/runtime/src/test/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptorTest.java	Adds parameterized coverage asserting warning/no-warning behavior across mapping types.

Suppressed comments (1)

service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/TimefoldRenamedPropertiesFallbackInterceptor.java:96

• When multiple legacy keys are defined for the same new key, resolveWithLegacyNames() can emit multiple WARN logs even though only the highest-precedence legacy value ends up being used.

    private static ConfigValue resolveWithLegacyNames(ConfigSourceInterceptorContext context, String name,
            List<String> legacyNames) {
        ConfigValue value = context.proceed(name);
        for (String legacyName : legacyNames) {
            ConfigValue legacyValue = context.proceed(legacyName);
            if (legacyValue != null && (value == null || CONFIG_SOURCE_COMPARATOR.compare(legacyValue, value) > 0)) {
                logWarningForResolvedKey(name, legacyName);
                value = legacyValue.withName(name);
            }
        }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[winklerm]

Tested with FSR locally, looks good.

[greyhairredbear]

Nice work, thanks for cleaning this up!

[triceo]

Approving in Radek's absence. Was validated with models.