[ 
https://issues.apache.org/jira/browse/CAMEL-26782?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18141295#comment-18141295
 ] 

Salvatore Mongiardo commented on CAMEL-26782:
---------------------------------------------

Update after the first PRs on this ticket: the fix for the helper (copy an 
option when it is bound or differs from a fresh instance of the configuration 
class, as in option A above) is in apache/camel-spring-boot#2079.

Correction to the description: platform-http {{requestTimeout}} is not restored 
by that helper change. 
{{SpringBootPlatformHttpAutoConfiguration.platformHttpEngineRequestMapping}} is 
eager (CAMEL-24351) and customizes the {{platform-http}} component before 
{{SpringBootPlatformWebMvcConfiguration.configureAsyncSupport}} writes 
{{requestTimeout}} into the configuration bean. As reported on the PR, the 
configuration then has 30000 but the component keeps 0. The value must be set 
on the component itself, for example by a {{ComponentCustomizer}} like the 
Kafka bridge on main, applying {{spring.mvc.async.request-timeout}} only when 
{{camel.component.platform-http.request-timeout}} is not set. The platform-http 
case in the Tests list therefore needs that change, not only the helper.


> camel-spring-boot: copyConfigurationProperties drops values assigned in Java 
> to a starter configuration bean
> ------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-26782
>                 URL: https://issues.apache.org/jira/browse/CAMEL-26782
>             Project: Camel
>          Issue Type: Bug
>          Components: tooling
>    Affects Versions: 4.23.0
>            Reporter: Salvatore Mongiardo
>            Assignee: Guillaume Nodet
>            Priority: Major
>             Fix For: 4.22.3, 4.24.0
>
>
> h3. Problem
> CAMEL-24501 (apache/camel-spring-boot#1951, commit 72323d48) changed 
> {{CamelPropertiesHelper.copyConfigurationProperties}} to copy only the 
> options that Spring Boot recorded in {{BoundConfigurationProperties}}. That 
> fixes catalog defaults overwriting values on user supplied Camel beans, but 
> {{isConfigured()}} can only recognise values bound from properties. A value 
> assigned in Java to the generated starter configuration bean before the 
> customizer runs is no longer copied to the component. 
> {{camel.springboot.lenient-configuration-binding=true}} does not restore it. 
> The change is released: {{camel-spring-boot-4.23.0}} contains it.
> h3. Reproduction
> Using {{HttpComponentConfiguration}} and {{HttpComponent}}:
> {code:java}
> configuration.setSkipRequestHeaders(true);   // before the customizer runs
> {code}
> {code}
> camel.component.http.log-http-activity=true
> {code}
> Before #1951 both values reach the component. After it, {{logHttpActivity}} 
> is {{true}} but {{skipRequestHeaders}} stays {{false}}.
> h3. In-tree cases of the same regression
> * {{SpringBootPlatformWebMvcConfiguration}} (main) writes 
> {{spring.mvc.async.request-timeout}} into 
> {{PlatformHttpComponentConfiguration.requestTimeout}} from code, so it no 
> longer reaches {{PlatformHttpComponent}} (small impact, Spring MVC still 
> enforces the timeout). Good test for the fix.
> * {{SpringKafkaPropertiesAutoConfiguration}} (camel-spring-boot-4.22.x only) 
> writes the {{spring.kafka.*}} values into {{KafkaComponentConfiguration}} 
> from a {{@PostConstruct}}. The filter drops them. On main the bridge is a 
> {{ComponentCustomizer}} (e7850714524, precedence fixed in 115f626f4e7 / 
> CAMEL-22760), so 4.22.x must not take #1951 as is.
> h3. Related effect: defaults moved from catalog to the component's own
> Any option the application does not configure now keeps the component's Java 
> default instead of the catalog default, where the two differ. Example: 
> {{XsltComponent.xpathTotalOpLimit}} is an {{int}} with {{defaultValue = 
> "10000"}} in its metadata, the field is 0, the starter initializes 10000. 
> Before #1951 the starter copied 10000. Also, 
> {{BaseOptionModel.resolveDefaultValue()}} turns a missing default into 
> {{false}} for {{java.lang.Boolean}}, so many nullable Booleans go from 
> {{false}} to {{null}}. Only 8 of the 607 {{java.lang.Boolean}} options in the 
> 4.22.2 component catalogs have no default.
> h3. Why it was noticed
> Red Hat CSB-13052: with {{camel-ai-tool-starter}} + 
> {{camel-mcp-server-starter}} on 4.22.1, the catalog {{defaultValue: false}} 
> for the nullable MCP hints ({{readOnlyHint}}, {{destructiveHint}}, 
> {{idempotentHint}}, {{openWorldHint}}) is materialised as {{= false}} in 
> {{AiToolComponentConfiguration}}, copied onto {{AiToolComponent}}, and 
> published in MCP {{tools/list}} as explicit {{false}} annotations although 
> the route sets none. #1951 resolves this on main, so the fix for the 
> regression must keep the hints unset.
> h3. Proposed fix
> *main (4.24.0), option A:* in {{copyConfigurationProperties}} copy an option 
> if it was bound or if its value differs from the value on a freshly 
> constructed instance of the same configuration class (computed once per 
> class). A catalog default equals the fresh instance's value, so it is not 
> copied; a Java assignment differs, so it is copied.
> * Collections: the only object valued initializer in the generated classes is 
> {{serverConfig = new HashMap<>()}} (Jolokia), and two empty maps compare 
> equal; collections without initializer are {{null}} on a fresh instance.
> * Fallback when the instance cannot be created (should not happen, generated 
> classes have a no-arg constructor): copy everything, as {{boundProperties()}} 
> does, but fail only on options known to be configured. #1951 removed the 
> DEBUG-only branch for options that carry just their default, so copying 
> everything would otherwise fail startup.
> * Known gap, to document, also in the paragraph #1951 added to 
> {{starter-configuration.adoc}}: a Java assignment equal to the catalog 
> default (e.g. {{setSkipRequestHeaders(false)}} onto a user bean holding 
> {{true}}) is not copied.
> * Smaller points: copy {{BoundConfigurationProperties.getAll()}} (live view) 
> inside the {{try}}; check {{BoundConfigurationProperties.get()}} for {{null}} 
> instead of catching the NPE; log skipped options at DEBUG; restore coverage 
> in {{testCopyConfigurationPropertiesIgnoresDefaultThatCannotBeSet}} (uses 
> {{configWithoutDrift()}}) and 
> {{testCopyConfigurationPropertiesIgnoresAutoConfigurationOptions}} (does not 
> bind {{enabled}}/{{customizer}}).
> *camel-spring-boot-4.22.x (4.22.3), option C:* do not backport #1951 (it 
> changes every starter on a patch branch). Do not initialize the four nullable 
> MCP hint fields of {{camel-ai-tool}} in {{SpringBootAutoConfigurationMojo}}; 
> {{returnDirect}} keeps its declared {{false}} default. Only the ai-tool 
> starter changes.
> h3. Tests
> * {{HttpComponentConfiguration}} scenario above (Java assignment plus a bound 
> value).
> * ai-tool hints stay unset on {{AiToolComponent}} and in the MCP 
> {{tools/list}} output.
> * A bound value equal to the catalog default is still copied onto a user bean.
> * platform-http {{requestTimeout}} from {{spring.mvc.async.request-timeout}}.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

Reply via email to