smongiar commented on PR #2079: URL: https://github.com/apache/camel-spring-boot/pull/2079#issuecomment-6743154449
Thanks @gnodet. I was working on the same ticket in parallel, so rather than open a second PR here are the points from my branch that cover what the reviews above ask for. Feel free to take any of them. **Helper (`CamelPropertiesHelper`)** - Compute the defaults once per configuration class (`ConcurrentHashMap<Class<?>, Map<String, Object>>`) instead of constructing an instance on every call. - Fallback: when no instance can be created, `defaultProperties` returns an empty map, so every non-null option counts as assigned. Since #1951 only options the application configured should fail startup, so for an option that is neither bound nor listed as configured and that the target cannot set, log at DEBUG and continue instead of adding it to `failed`. I tracked the bound ones in a `Set<String> configured` for this. - `boundProperties()`: copy `BoundConfigurationProperties.getAll().keySet()` into a new set inside the `try`, and check `BoundConfigurationProperties.get()` for `null`. - Log skipped and copied-by-difference options at DEBUG. - Update the class-level Javadoc of `copyConfigurationProperties` and the comment above the filter, which still say only bound options are copied. **Docs (`starter-configuration.adoc`)**: add after the paragraph #1951 added that a value assigned in Java to the starter's own configuration class is copied when it differs from the default that class starts with, and the known gap: a value equal to the default (e.g. `setSkipRequestHeaders(false)` onto a bean holding `true`) is not copied. **Tests** - `testCopyConfigurationPropertiesDoesNotCopyProgrammaticValueMatchingDefault` passes without the change, as @Croway noted, because `configWithoutDrift()` already sets `secure` to `false`. A configuration subclass whose `anotherOptionOnlyCarryingItsDefault` stays at its default and has no setter on the target exercises the new comparison: it must be dropped without failing. Using that class in `testCopyConfigurationPropertiesIgnoresDefaultThatCannotBeSet` restores its coverage. - `testCopyConfigurationPropertiesIgnoresAutoConfigurationOptions` should bind `enabled` (`camel.test.my-config.enabled = true`) so it covers what its name says. - ai-tool starter: the four MCP hints are `null` on `AiToolComponent` by default, and `camel.component.ai-tool.read-only-hint=true` / `destructive-hint=false` are still applied (an explicit `false` is kept). Two tests in `camel-ai-tool-starter`; they fail on `main` before the fix. **platform-http**: I agree with @Croway that the helper cannot fix `requestTimeout` because of the ordering with the eager `platformHttpEngineRequestMapping` (CAMEL-24351). I did not verify it by running a test. The value has to be set on the component itself, e.g. a `ComponentCustomizer` that applies `spring.mvc.async.request-timeout` only when `camel.component.platform-http.request-timeout` is not set. That could be a separate change; I'm happy to do it, tell me if you prefer it in this PR. The commit is local only; I can push it to my fork if a diff is easier to read. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
