Croway commented on PR #2078:
URL:
https://github.com/apache/camel-spring-boot/pull/2078#issuecomment-6737827585
Follow-up after a code review of this PR, a few points for the new ticket:
- **Kafka bridge (4.22.x only).** On this branch
`SpringKafkaPropertiesAutoConfiguration` writes the `spring.kafka.*` values
into `KafkaComponentConfiguration` from a `@PostConstruct`. The new filter
drops them, so `spring.kafka.bootstrap-servers` and the other bridged values no
longer reach the Kafka component. On main the bridge became a
`ComponentCustomizer` in e7850714524, with 115f626f4e7 (CAMEL-22760) fixing its
precedence. Neither is in this PR, which is one more reason not to backport
#1951 as is.
- **platform-http (main).** `SpringBootPlatformWebMvcConfiguration` writes
`spring.mvc.async.request-timeout` into
`PlatformHttpComponentConfiguration.requestTimeout` from code, so it no longer
reaches `PlatformHttpComponent`. Spring MVC still enforces the timeout itself,
so the impact is small. But it's an in-tree case of the same regression and a
good test for A.
- **xslt, as an example of the changed defaults.**
`XsltComponent.xpathTotalOpLimit` is an `int` with `defaultValue = "10000"` in
its metadata, so the field is 0 while the starter initializes 10000. Before
#1951 the starter copied 10000, and `XsltEndpoint` created its own
`TransformerFactory` with that limit. Now the component keeps 0 and neither
happens.
- **Correction to my fallback suggestion.** #1951 removed the branch that
only logged at DEBUG when an option carrying nothing but its default could not
be set. So whenever everything is copied, such an option now fails startup.
That happens today when `BoundConfigurationProperties` is unavailable, and
would happen in A's fallback. That path should only fail on options known to be
configured.
- **Smaller points for the fix on main:**
- `BoundConfigurationProperties.getAll()` is a live view and is iterated
outside the `try`. Take a copy inside it.
- Check `BoundConfigurationProperties.get()` for `null` instead of
catching the NPE.
- Log the options the filter skips at DEBUG.
- Restore coverage in two tests.
`testCopyConfigurationPropertiesIgnoresDefaultThatCannotBeSet` now uses
`configWithoutDrift()`, whose class doesn't have that option.
`testCopyConfigurationPropertiesIgnoresAutoConfigurationOptions` doesn't bind
`enabled` or `customizer`, so the code that strips them isn't exercised.
_Claude Code on behalf of Federico Mariani_
--
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]
