Croway commented on PR #1995:
URL:
https://github.com/apache/camel-spring-boot/pull/1995#issuecomment-5812165669
Following up on my last comment, here's a narrower fix I prototyped. I'd
suggest switching the PR to this approach instead of the generator change.
**Idea.** Spring Boot has a supported extension point for this:
`ConfigurationPropertiesBindHandlerAdvisor`. Several advisors can be
registered, and they wrap the `BindHandler` used for every
`@ConfigurationProperties` bind. The prototype adds
`CamelConfigurationPropertiesBindHandlerAdvisor` to `core/camel-spring-boot`,
registered as a `static` `@Bean` in `CamelAutoConfiguration`. Its `onStart`
skips binding a nested target only when **all** of the following hold:
- the name is under `camel.*`;
- the target type is a third-party bean type: not `org.apache.camel.*`,
`java.*`, a primitive, an enum, an array, a `Map` or a `Collection` (e.g.
Jackson's `ObjectMapper`);
- no property source has a value for that exact name, so
`object-mapper=#bean:myMapper` still binds;
- no property source reports a nested key as definitely `PRESENT`.
In practice, with property sources that can list their keys, binding is
exactly as today. The handler only cuts the speculative walk that a
non-enumerable source causes. No generator change is needed, the generated
field types stay as they are, and there are no public API changes.
**Results** (salesforce starter, Spring Boot 4.1.1):
| Case | `main` | With the advisor |
|---|---|---|
| Non-enumerable source, no salesforce properties | fails
(`JacksonFeatureSet` / `InstantiationException`) | starts |
| Non-enumerable source *serving* `config.api-version=62.0` and
`config.object-mapper=#bean:myMapper` | fails | starts, both values applied |
| `config.api-version=62.0` from a normal source | applied | applied |
| `object-mapper=#bean:myMapper` / `config.object-mapper=#bean:myMapper` |
bean injected | bean injected |
| Explicit keys *inside* the `ObjectMapper` (e.g.
`config.object-mapper.property-naming-strategy=x`) | fail | still fail (user
error, reported as before) |
It also passes the existing `core/camel-spring-boot` tests and the http,
salesforce, reactive-streams and jackson starter tests, plus a new unit test
with 4 cases. That test includes a negative control showing the failure without
the advisor, and it checks values served only by a non-enumerable source.
One related detail: Camel's own `FilePropertySource`, registered when
`camel.main.file-configurations` is set, extends the raw `PropertySource`, so
it is non-enumerable too. Making it an `EnumerablePropertySource` could be a
small follow-up hardening.
Would you be OK with reworking the PR along these lines? That would mean
reverting the generator change and the regenerated http/salesforce files,
applying the patch below, and replacing `SalesforceObjectMapperBindingTest`
with the salesforce reproducers from my previous comments or similar, if you
want starter-level coverage too. Happy to help with it.
<details><summary>Patch (against current <code>main</code>)</summary>
```diff
diff --git
a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelAutoConfiguration.java
b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelAutoConfiguration.java
index a3429047a20..723a0197c72 100644
---
a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelAutoConfiguration.java
+++
b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelAutoConfiguration.java
@@ -92,6 +92,15 @@ public class CamelAutoConfiguration {
private static final Logger LOG =
LoggerFactory.getLogger(CamelAutoConfiguration.class);
+ /**
+ * Stops Spring Boot from binding into the internals of third party
types held by the options of the Camel
+ * configuration classes, unless the application configured them.
+ */
+ @Bean
+ static CamelConfigurationPropertiesBindHandlerAdvisor
camelConfigurationPropertiesBindHandlerAdvisor() {
+ return new CamelConfigurationPropertiesBindHandlerAdvisor();
+ }
+
/**
* Spring-aware Camel context for the application. Auto-detects and
loads all routes available in the Spring
* context.
diff --git
a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelConfigurationPropertiesBindHandlerAdvisor.java
b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelConfigurationPropertiesBindHandlerAdvisor.java
new file mode 100644
index 00000000000..8b508c5f20c
--- /dev/null
+++
b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/CamelConfigurationPropertiesBindHandlerAdvisor.java
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.spring.boot;
+
+import java.util.Collection;
+import java.util.Map;
+
+import
org.springframework.boot.context.properties.ConfigurationPropertiesBindHandlerAdvisor;
+import org.springframework.boot.context.properties.bind.AbstractBindHandler;
+import org.springframework.boot.context.properties.bind.BindContext;
+import org.springframework.boot.context.properties.bind.BindHandler;
+import org.springframework.boot.context.properties.bind.Bindable;
+import
org.springframework.boot.context.properties.source.ConfigurationPropertyName;
+import
org.springframework.boot.context.properties.source.ConfigurationPropertySource;
+import
org.springframework.boot.context.properties.source.ConfigurationPropertyState;
+
+/**
+ * Stops Spring Boot from binding into the internals of third party types,
such as a Jackson <tt>ObjectMapper</tt>,
+ * held by the options of the Camel configuration classes, unless the
application configured a property below them.
+ * <p/>
+ * Spring Boot binds a nested JavaBean only when a property source has a
property below it. A property source that
+ * cannot list its property names, such as one that looks properties up in
a remote store, cannot tell, so Spring Boot
+ * binds into every nested JavaBean in case it has a property below it. For
an option of a third party type that walks
+ * into the internals of that type, which may fail, and then the
application fails to start although it configured
+ * nothing there.
+ * <p/>
+ * The options of the Camel configuration classes are still bound when the
application configures them, whether as a
+ * value, such as a <tt>#bean:myObjectMapper</tt> reference, or as
properties below them in a property source that can
+ * list its property names. The options of Camel's own types, such as the
nested configuration of a component, are
+ * bound as before.
+ */
+public class CamelConfigurationPropertiesBindHandlerAdvisor implements
ConfigurationPropertiesBindHandlerAdvisor {
+
+ private static final ConfigurationPropertyName CAMEL =
ConfigurationPropertyName.of("camel");
+
+ @Override
+ public BindHandler apply(BindHandler bindHandler) {
+ return new AbstractBindHandler(bindHandler) {
+ @Override
+ public <T> Bindable<T> onStart(ConfigurationPropertyName name,
Bindable<T> target, BindContext context) {
+ if (context.getDepth() > 0 && CAMEL.isAncestorOf(name) &&
isThirdPartyBean(target)
+ && !isConfigured(name, context)) {
+ return null;
+ }
+ return super.onStart(name, target, context);
+ }
+ };
+ }
+
+ private static boolean isThirdPartyBean(Bindable<?> target) {
+ Class<?> type = target.getType().resolve(Object.class);
+ if (type.isPrimitive() || type.isArray() || type.isEnum() ||
Map.class.isAssignableFrom(type)
+ || Collection.class.isAssignableFrom(type)) {
+ return false;
+ }
+ String name = type.getName();
+ return !name.startsWith("java.") &&
!name.startsWith("org.apache.camel.");
+ }
+
+ /**
+ * Whether a property source has a value for the option, or a property
below it for certain.
+ */
+ private static boolean isConfigured(ConfigurationPropertyName name,
BindContext context) {
+ for (ConfigurationPropertySource source : context.getSources()) {
+ if (source.getConfigurationProperty(name) != null
+ || source.containsDescendantOf(name) ==
ConfigurationPropertyState.PRESENT) {
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git
a/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/CamelConfigurationPropertiesBindHandlerAdvisorTest.java
b/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/CamelConfigurationPropertiesBindHandlerAdvisorTest.java
new file mode 100644
index 00000000000..d1a8d8ae74b
--- /dev/null
+++
b/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/CamelConfigurationPropertiesBindHandlerAdvisorTest.java
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.spring.boot;
+
+import java.util.Map;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import
org.springframework.boot.context.properties.ConfigurationPropertiesBinding;
+import
org.springframework.boot.context.properties.EnableConfigurationProperties;
+import
org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.convert.converter.Converter;
+import org.springframework.core.env.PropertySource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+public class CamelConfigurationPropertiesBindHandlerAdvisorTest {
+
+ private static final ObjectMapper MY_MAPPER = new ObjectMapper();
+
+ private final ApplicationContextRunner runner = new
ApplicationContextRunner()
+ .withUserConfiguration(DummyConfiguration.class);
+
+ @Test
+ public void nonEnumerableSourceFailsWithoutAdvisor() {
+ runner.withInitializer(ctx ->
ctx.getEnvironment().getPropertySources().addLast(nonEnumerable(Map.of())))
+ .run(ctx -> {
+ // Spring Boot binds into the internals of the
ObjectMapper, in case the source has a property there
+ assertNotNull(ctx.getStartupFailure());
+ });
+ }
+
+ @Test
+ public void nonEnumerableSourceDoesNotBindIntoThirdPartyTypes() {
+ runner.withUserConfiguration(AdvisorConfiguration.class)
+ .withInitializer(ctx ->
ctx.getEnvironment().getPropertySources().addLast(nonEnumerable(Map.of())))
+ .run(ctx -> {
+ assertNull(ctx.getStartupFailure());
+ DummyComponentConfiguration config =
ctx.getBean(DummyComponentConfiguration.class);
+ assertNull(config.getObjectMapper());
+ assertNull(config.getConfig().getObjectMapper());
+ });
+ }
+
+ @Test
+ public void nonEnumerableSourceStillBindsConfiguredOptions() {
+ Map<String, Object> values = Map.of(
+ "camel.component.dummy.object-mapper", "myMapper",
+ "camel.component.dummy.config.name", "foo",
+ "camel.component.dummy.config.object-mapper", "myMapper");
+ runner.withUserConfiguration(AdvisorConfiguration.class)
+ .withInitializer(ctx ->
ctx.getEnvironment().getPropertySources().addLast(nonEnumerable(values)))
+ .run(ctx -> {
+ assertNull(ctx.getStartupFailure());
+ DummyComponentConfiguration config =
ctx.getBean(DummyComponentConfiguration.class);
+ assertSame(MY_MAPPER, config.getObjectMapper());
+ assertEquals("foo", config.getConfig().getName());
+ assertSame(MY_MAPPER,
config.getConfig().getObjectMapper());
+ });
+ }
+
+ @Test
+ public void enumerableSourceBindsNestedOptions() {
+ runner.withUserConfiguration(AdvisorConfiguration.class)
+ .withPropertyValues("camel.component.dummy.config.name=foo",
+
"camel.component.dummy.config.object-mapper=myMapper")
+ .run(ctx -> {
+ assertNull(ctx.getStartupFailure());
+ DummyComponentConfiguration config =
ctx.getBean(DummyComponentConfiguration.class);
+ assertEquals("foo", config.getConfig().getName());
+ assertSame(MY_MAPPER,
config.getConfig().getObjectMapper());
+ });
+ }
+
+ private static PropertySource<Object> nonEnumerable(Map<String, Object>
values) {
+ return new PropertySource<>("nonEnumerable") {
+ @Override
+ public Object getProperty(String name) {
+ return values.get(name);
+ }
+ };
+ }
+
+ @Configuration
+ @EnableConfigurationProperties(DummyComponentConfiguration.class)
+ static class DummyConfiguration {
+ @Bean
+ @ConfigurationPropertiesBinding
+ static Converter<String, ObjectMapper> objectMapperConverter() {
+ return source -> "myMapper".equals(source) ? MY_MAPPER : null;
+ }
+ }
+
+ @Configuration
+ static class AdvisorConfiguration {
+ @Bean
+ static CamelConfigurationPropertiesBindHandlerAdvisor advisor() {
+ return new CamelConfigurationPropertiesBindHandlerAdvisor();
+ }
+ }
+
+ @ConfigurationProperties(prefix = "camel.component.dummy")
+ public static class DummyComponentConfiguration {
+ private ObjectMapper objectMapper;
+ private DummyEndpointConfig config = new DummyEndpointConfig();
+
+ public ObjectMapper getObjectMapper() {
+ return objectMapper;
+ }
+
+ public void setObjectMapper(ObjectMapper objectMapper) {
+ this.objectMapper = objectMapper;
+ }
+
+ public DummyEndpointConfig getConfig() {
+ return config;
+ }
+
+ public void setConfig(DummyEndpointConfig config) {
+ this.config = config;
+ }
+ }
+
+ public static class DummyEndpointConfig {
+ private String name;
+ private ObjectMapper objectMapper;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ public ObjectMapper getObjectMapper() {
+ return objectMapper;
+ }
+
+ public void setObjectMapper(ObjectMapper objectMapper) {
+ this.objectMapper = objectMapper;
+ }
+ }
+}
```
</details>
_Claude Code on behalf of Croway_
--
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]
