Croway commented on PR #1935: URL: https://github.com/apache/camel-spring-boot/pull/1935#issuecomment-5513308774
Thanks for the review — all nine items are addressed in `6d5652a` (pushed on top of the existing two commits; no history rewritten). The PR description now carries a "Review follow-up" section with the details. **1 (blocking), converters are global.** I spiked it first, and the spike changed the fix. `targetType.getSource()` does carry the bound class — it is a `org.springframework.core.MethodParameter` whose `getContainingClass()`/`getDeclaringClass()` is the `@ConfigurationProperties` class on Spring Boot 4.1.1. But `ConditionalGenericConverter.matches` turns out to be the wrong place for the check: `GenericConversionService` caches the converter it picked per source/target `TypeDescriptor` pair, and `TypeDescriptor.equals` ignores the source. With one `camel.*` class and one third-party class each holding a `HostnameVerifier` field, `matches` was called **once** and `convert` **twice**, so the decision taken for whichever class binds first would be reused for the other — order dependent, and in the bad order a valid Camel property gets a `ConverterNotFoundException`. So the decision is taken in `convert()`, which the spike shows is invoked per binding with the right `targetType`. `BeanReferenceHelper.isCamelConfigurationTarget(TypeDescriptor)` resolves the bound class from `getSource()` (`MethodParameter` or `Field`) and returns true when it is under `org.apache.camel` or annotated with `@ConfigurationProperties` for a `camel.` prefix, and true when the source does not identify a class so Camel's own binding is never weakened by a binder shape it does not recognise. Every other class keeps exactly the pre-4.23 behaviour. That is the substance of option (a) rather than the weaker option (b): it keys on the class being bound rather than on the target type's package, so `camel.component.http.x509-hostname-verifier = myVerifier` still resolves even though `javax.net.ssl.HostnameVerifier` is not a Camel type. The generated body stays one line. Covered by `BeanReferenceHelperTest` (both rules, the unknown-source fallback, a third-party binding that is not intercepted) and end-to-end by two new cases in `HttpComponentBeanReferenceBindingTest`, one of which is exactly your scenario: a `@ConfigurationProperties` class in `com.example` with a `HostnameVerifier` field bound from a plain string, asserting the context starts and the field is simply left null. **2.** `#type:` now checks `type.isInstance(bean)` and throws the same `IllegalArgumentException` naming the type actually found. Test added. **3.** The flag is read from the Spring `Environment` through the `ApplicationContext` the helper already has, declared in `additional-spring-configuration-metadata.json` next to `camel.vault.ignore-resolution-failures`, and `spring-boot.json` is regenerated. **4.** Confirmed across all 479 component, data format and language JSON files in `catalog/camel-catalog-provider-springboot`: no option is named `enabled` or `customizer`. Stated in the PR description, and the mojo now fails the build if one ever is — the full regeneration run exercised that check against every catalog entry. **5.** The message names the target class and says the option may be listed in the starter documentation, which is generated from the catalog rather than from that class, in which case it never took effect. **6.** The `WARN` on the legacy `copyProperties` / `setCamelProperties(..., false)` path stays, and the upgrade note now says explicitly that this is public API used by hand-written customizers and that those will see new `WARN` lines for options that were previously dropped in silence. **7.** Acknowledged: no starter generates a language converter, so that template is covered only by the mojo string assertion, not by any running starter. **8.** Done, `assertTrue`. **9.** `isExplicitlyConfigured` logs at `WARN` with the exception now. Re-running the full regeneration after these changes produces a byte-identical tree to the existing `CAMEL-24501: Regen` commit, so there is no second regeneration commit. Green: the generator plugin, `core/camel-spring-boot` full suite, `camel-core-starter`, `camel-http-starter`, `camel-netty-http-starter`, `camel-jackson-starter`. _Claude Code (Opus 5) 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]
