This is an automated email from the ASF dual-hosted git repository. Croway pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-spring-boot.git
commit e59029e01829e1353a7beb575ab5a1ab1504b762 Author: croway <[email protected]> AuthorDate: Wed Sep 2 19:02:05 2026 +0200 CAMEL-24501: Scope the generated converters to Camel's own configuration binding Review follow-up on the previous commit. The generated converters are registered with @ConfigurationPropertiesBinding, so they take part in every @ConfigurationProperties binding in the application, not only in Camel's own. Failing closed unconditionally therefore turned an unrelated application property of a type a starter registers for, such as javax.net.ssl.HostnameVerifier, into a startup failure that talks about camel.component.*. BeanReferenceHelper now resolves the class being bound from TypeDescriptor.getSource(), which Spring Boot's binder fills with the setter's MethodParameter (or the Field for field access), and only applies the strict behaviour when that class is Camel's own: under org.apache.camel, or annotated with @ConfigurationProperties for a camel. prefix. Any other class keeps the behaviour it had before, and an unrecognised source counts as Camel's own so that Camel's own binding is never weakened. This is decided in convert() rather than in ConditionalGenericConverter.matches: GenericConversionService caches the converter it picked per source/target TypeDescriptor pair and TypeDescriptor.equals ignores the source, so matches is consulted once for the first class bound and the answer reused for every other class with a field of the same type. A conditional converter would therefore be order dependent, and in the bad order it would report a missing converter for a valid Camel property. Also from the review: - #type: now checks that the bean found by type is assignable to the option type, instead of leaving a ClassCastException for the binder to hit later. - camel.springboot.lenient-configuration-binding is read from the Spring Environment rather than from Camel's PropertiesComponent, and is declared in additional-spring-configuration-metadata.json so it shows up in IDE completion. - The mojo fails the build if a catalog option is ever named enabled or customizer, since the customizers strip those before binding. No component, data format or language declares one today. - The error message names the target class and points out that the option may be listed in the starter documentation, which is generated from the catalog rather than from that class, and so may never have taken effect. - isExplicitlyConfigured logs at WARN when it cannot decide, as returning false there downgrades a hard error to an ignored option. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../httpstarter/ThirdPartyHttpProperties.java | 39 ++++++++ .../HttpComponentBeanReferenceBindingTest.java | 34 +++++++ .../src/main/docs/spring-boot.json | 6 ++ .../spring/boot/util/BeanReferenceHelper.java | 77 +++++++++++++++- .../spring/boot/util/CamelPropertiesHelper.java | 39 ++++---- .../additional-spring-configuration-metadata.json | 6 ++ .../springboot/CamelPrefixedProperties.java | 38 ++++++++ .../example/springboot/ThirdPartyProperties.java | 38 ++++++++ .../spring/boot/util/BeanReferenceHelperTest.java | 101 ++++++++++++++++++++- .../maven/SpringBootAutoConfigurationMojo.java | 27 ++++++ .../maven/SpringBootAutoConfigurationMojoTest.java | 17 ++++ 11 files changed, 398 insertions(+), 24 deletions(-) diff --git a/components-starter/camel-http-starter/src/test/java/com/example/httpstarter/ThirdPartyHttpProperties.java b/components-starter/camel-http-starter/src/test/java/com/example/httpstarter/ThirdPartyHttpProperties.java new file mode 100644 index 00000000000..2c62a4fcd57 --- /dev/null +++ b/components-starter/camel-http-starter/src/test/java/com/example/httpstarter/ThirdPartyHttpProperties.java @@ -0,0 +1,39 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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 com.example.httpstarter; + +import javax.net.ssl.HostnameVerifier; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Stands in for an application class that happens to have a field of a type HttpComponentConverter registers itself + * for. Deliberately outside the org.apache.camel packages. + */ +@ConfigurationProperties(prefix = "thirdparty.http") +public class ThirdPartyHttpProperties { + + private HostnameVerifier verifier; + + public HostnameVerifier getVerifier() { + return verifier; + } + + public void setVerifier(HostnameVerifier verifier) { + this.verifier = verifier; + } +} diff --git a/components-starter/camel-http-starter/src/test/java/org/apache/camel/component/http/springboot/HttpComponentBeanReferenceBindingTest.java b/components-starter/camel-http-starter/src/test/java/org/apache/camel/component/http/springboot/HttpComponentBeanReferenceBindingTest.java index cc2859adfdd..e351743589b 100644 --- a/components-starter/camel-http-starter/src/test/java/org/apache/camel/component/http/springboot/HttpComponentBeanReferenceBindingTest.java +++ b/components-starter/camel-http-starter/src/test/java/org/apache/camel/component/http/springboot/HttpComponentBeanReferenceBindingTest.java @@ -16,6 +16,7 @@ */ package org.apache.camel.component.http.springboot; +import com.example.httpstarter.ThirdPartyHttpProperties; import org.apache.camel.support.jsse.SSLContextParameters; import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; @@ -25,6 +26,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertNull; /** * A complex (object) typed option such as {@code sslContextParameters} is configured with a reference to a bean in the @@ -79,6 +81,38 @@ class HttpComponentBeanReferenceBindingTest { }); } + @Test + void testThirdPartyPropertiesAreNotIntercepted() { + // HttpComponentConverter is registered with @ConfigurationPropertiesBinding and so takes part in every + // @ConfigurationProperties binding, not only in Camel's own. Adding the starter to the classpath must not + // make an unrelated property of the same type fail to bind. + new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(ThirdPartyConfiguration.class)) + .withUserConfiguration(HttpComponentConverter.class) + .withPropertyValues("thirdparty.http.verifier=someValue") + .run(context -> { + assertThat(context).hasNotFailed(); + assertNull(context.getBean(ThirdPartyHttpProperties.class).getVerifier()); + }); + } + + @Test + void testThirdPartyPropertiesStillResolveHashReferences() { + new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(ThirdPartyConfiguration.class)) + .withUserConfiguration(HttpComponentConverter.class) + .withPropertyValues("thirdparty.http.verifier=#bean:noSuchVerifier") + .run(context -> { + assertThat(context).hasFailed(); + assertThat(context.getStartupFailure()).hasStackTraceContaining("noSuchVerifier"); + }); + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(ThirdPartyHttpProperties.class) + static class ThirdPartyConfiguration { + } + @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(HttpComponentConfiguration.class) static class TestConfiguration { diff --git a/core/camel-spring-boot/src/main/docs/spring-boot.json b/core/camel-spring-boot/src/main/docs/spring-boot.json index 29389f13467..70bab980056 100644 --- a/core/camel-spring-boot/src/main/docs/spring-boot.json +++ b/core/camel-spring-boot/src/main/docs/spring-boot.json @@ -1335,6 +1335,12 @@ "description": "Security policy for plain-text secrets. When set, overrides the global policy for properties that contain sensitive values configured as plain text.", "sourceType": "org.apache.camel.spring.boot.security.CamelSecurityPolicyConfigurationProperties" }, + { + "name": "camel.springboot.lenient-configuration-binding", + "type": "java.lang.Boolean", + "description": "Whether a camel.component, camel.dataformat or camel.language option that cannot be set on the Camel component, data format or language it configures should be tolerated. When false (the default) an option the application configured explicitly and that cannot be set aborts startup, so a configured option can never be silently dropped. When true the option is logged at WARN and ignored.", + "defaultValue": false + }, { "name": "camel.ssl.cert-alias", "type": "java.lang.String", diff --git a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/BeanReferenceHelper.java b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/BeanReferenceHelper.java index 63de31c97e5..8c8aa810982 100644 --- a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/BeanReferenceHelper.java +++ b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/BeanReferenceHelper.java @@ -16,8 +16,12 @@ */ package org.apache.camel.spring.boot.util; +import java.lang.reflect.Field; import org.springframework.beans.BeansException; +import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.ApplicationContext; +import org.springframework.core.MethodParameter; +import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.convert.TypeDescriptor; import org.springframework.util.ClassUtils; @@ -37,6 +41,12 @@ import org.springframework.util.ClassUtils; * </ul> * A configured value that cannot be resolved is reported by throwing an {@link IllegalArgumentException}, so a typo in * a configuration file is not silently turned into a <tt>null</tt> value. + * <p/> + * The generated converters are registered with {@code @ConfigurationPropertiesBinding} and therefore take part in + * every {@code @ConfigurationProperties} binding in the application, not only in Camel's own. A binding whose target + * is not a Camel configuration class keeps the behaviour it had before Camel 4.23 - a value that is not a bean + * reference converts to <tt>null</tt> - so that adding a starter to the classpath cannot make an unrelated + * application property fail to bind. See {@link #isCamelConfigurationTarget(TypeDescriptor)}. */ public final class BeanReferenceHelper { @@ -45,6 +55,9 @@ public final class BeanReferenceHelper { private static final String CLASS_PREFIX = "#class:"; private static final String AUTOWIRED = "#autowired"; + private static final String CAMEL_PACKAGE = "org.apache.camel"; + private static final String CAMEL_PROPERTY_PREFIX = "camel."; + private BeanReferenceHelper() { } @@ -64,7 +77,8 @@ public final class BeanReferenceHelper { * @return the resolved bean, or <tt>null</tt> if no value was configured * * @throws IllegalArgumentException - * if a value was configured but cannot be resolved to a bean of the target type + * if a Camel option was configured with a value that cannot be resolved to a bean + * of the target type */ public static Object resolveBeanReference(ApplicationContext applicationContext, Object source, TypeDescriptor targetType, String propertyPrefix) { @@ -76,6 +90,13 @@ public final class BeanReferenceHelper { return null; } Class<?> type = targetType != null ? targetType.getObjectType() : Object.class; + if (!isCamelConfigurationTarget(targetType)) { + // this binding belongs to somebody else, so only resolve what has always been resolved here and + // leave the rest alone rather than imposing Camel's bean reference syntax on it + if (!value.startsWith("#")) { + return null; + } + } if (applicationContext == null) { throw new IllegalArgumentException( message(value, type, propertyPrefix, "there is no Spring application context available")); @@ -90,7 +111,13 @@ public final class BeanReferenceHelper { } if (value.startsWith(TYPE_PREFIX)) { String fqn = value.substring(TYPE_PREFIX.length()).trim(); - return applicationContext.getBean(ClassUtils.forName(fqn, applicationContext.getClassLoader())); + Object bean = applicationContext.getBean(ClassUtils.forName(fqn, applicationContext.getClassLoader())); + if (!type.isInstance(bean)) { + throw new IllegalArgumentException(message(value, type, propertyPrefix, + "the bean found by type is a [" + bean.getClass().getName() + + "] which is not assignable to the option type")); + } + return bean; } String id = value; if (id.startsWith(BEAN_PREFIX)) { @@ -107,6 +134,52 @@ public final class BeanReferenceHelper { } } + /** + * Whether the binding this conversion takes part in targets a Camel configuration class. + * <p/> + * Spring Boot's binder builds the target {@link TypeDescriptor} from the setter's {@link MethodParameter} (or from + * the {@link Field} for field access), so the class being bound is reachable through + * {@link TypeDescriptor#getSource()}. A class counts as Camel's own when it lives under + * <tt>org.apache.camel</tt>, or when it is annotated with {@link ConfigurationProperties} for a prefix starting + * with <tt>camel.</tt>. + * <p/> + * When the source does not identify a class this returns <tt>true</tt>, so that Camel's own binding is never + * weakened by a shape of the binder this does not recognise. + * <p/> + * Note that this cannot be done in {@code ConditionalGenericConverter.matches}: Spring's + * {@code GenericConversionService} caches the converter it picked per source/target {@code TypeDescriptor} pair, + * and {@code TypeDescriptor.equals} ignores the source, so {@code matches} is consulted once for the first class + * bound and the answer is then reused for every other class with a field of the same type. + */ + static boolean isCamelConfigurationTarget(TypeDescriptor targetType) { + Class<?> owner = boundClass(targetType); + if (owner == null) { + return true; + } + if (owner.getName().startsWith(CAMEL_PACKAGE)) { + return true; + } + ConfigurationProperties annotation + = AnnotatedElementUtils.findMergedAnnotation(owner, ConfigurationProperties.class); + if (annotation != null) { + String prefix = !annotation.prefix().isEmpty() ? annotation.prefix() : annotation.value(); + return prefix.startsWith(CAMEL_PROPERTY_PREFIX); + } + return false; + } + + private static Class<?> boundClass(TypeDescriptor targetType) { + Object source = targetType != null ? targetType.getSource() : null; + if (source instanceof MethodParameter methodParameter) { + Class<?> containing = methodParameter.getContainingClass(); + return containing != null ? containing : methodParameter.getDeclaringClass(); + } + if (source instanceof Field field) { + return field.getDeclaringClass(); + } + return null; + } + private static String message(String value, Class<?> type, String propertyPrefix, String reason) { StringBuilder sb = new StringBuilder(256); sb.append("Cannot resolve value [").append(value).append("] as a bean of type [").append(type.getName()) diff --git a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/CamelPropertiesHelper.java b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/CamelPropertiesHelper.java index 66b1601fd29..3bf909d09d8 100644 --- a/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/CamelPropertiesHelper.java +++ b/core/camel-spring-boot/src/main/java/org/apache/camel/spring/boot/util/CamelPropertiesHelper.java @@ -22,13 +22,11 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.Set; import org.apache.camel.CamelContext; import org.apache.camel.Component; import org.apache.camel.PropertyBindingException; import org.apache.camel.spi.BeanIntrospection; -import org.apache.camel.spi.PropertiesComponent; import org.apache.camel.spi.PropertyConfigurer; import org.apache.camel.support.PluginHelper; import org.apache.camel.support.PropertyBindingSupport; @@ -105,7 +103,7 @@ public final class CamelPropertiesHelper { return; } - boolean lenient = isLenientBinding(camelContext); + boolean lenient = isLenientBinding(applicationContext); List<String> failed = new ArrayList<>(); for (Map.Entry<String, Object> entry : properties.entrySet()) { String name = entry.getKey(); @@ -125,10 +123,12 @@ public final class CamelPropertiesHelper { } if (!failed.isEmpty()) { throw new IllegalArgumentException( - "Cannot configure " + failed + " as the bean class [" + ObjectHelper.classCanonicalName(target) - + "] has no suitable setter method, or it is not possible to lookup a bean with that id in the" - + " Spring Boot registry. Remove or correct the option, or set " - + LENIENT_CONFIGURATION_BINDING + "=true to ignore it."); + "Cannot configure " + failed + " on [" + ObjectHelper.classCanonicalName(target) + + "], which has no suitable setter method for it, and no bean with that id exists in the Spring" + + " Boot registry. The option may be listed in the documentation of the starter, as that is" + + " generated from the Camel catalog rather than from this class, in which case it has never" + + " taken effect. Correct or remove the option, or set " + + LENIENT_CONFIGURATION_BINDING + "=true to keep ignoring it."); } } @@ -156,26 +156,25 @@ public final class CamelPropertiesHelper { } } } catch (Exception e) { - LOG.debug("Cannot determine whether {}.{} was configured due to: {}", propertyPrefix, name, - e.getMessage()); + // returning false downgrades a hard error to an ignored option, so this must not stay quiet + LOG.warn("Cannot determine whether {} was configured due to: {}. Treating it as not configured.", + optionKey(propertyPrefix, name), e.getMessage(), e); } return false; } - private static boolean isLenientBinding(CamelContext camelContext) { + private static boolean isLenientBinding(ApplicationContext applicationContext) { + if (applicationContext == null) { + return false; + } try { - PropertiesComponent pc = camelContext.getPropertiesComponent(); - if (pc != null) { - Optional<String> value = pc.resolveProperty(LENIENT_CONFIGURATION_BINDING); - if (value.isPresent()) { - return "true".equalsIgnoreCase(value.get().trim()); - } - } + return applicationContext.getEnvironment().getProperty(LENIENT_CONFIGURATION_BINDING, Boolean.class, + Boolean.FALSE); } catch (Exception e) { - LOG.debug("Cannot resolve {} due to: {}. Using strict configuration binding.", - LENIENT_CONFIGURATION_BINDING, e.getMessage()); + LOG.warn("Cannot resolve {} due to: {}. Using strict configuration binding.", + LENIENT_CONFIGURATION_BINDING, e.getMessage(), e); + return false; } - return false; } /** diff --git a/core/camel-spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/core/camel-spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 2488ae24608..049f5d23e8f 100644 --- a/core/camel-spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/core/camel-spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -1,5 +1,11 @@ { "properties": [ + { + "name": "camel.springboot.lenient-configuration-binding", + "type": "java.lang.Boolean", + "description": "Whether a camel.component, camel.dataformat or camel.language option that cannot be set on the Camel component, data format or language it configures should be tolerated. When false (the default) an option the application configured explicitly and that cannot be set aborts startup, so a configured option can never be silently dropped. When true the option is logged at WARN and ignored.", + "defaultValue": false + }, { "name": "camel.vault.ignore-resolution-failures", "type": "java.lang.Boolean", diff --git a/core/camel-spring-boot/src/test/java/com/example/springboot/CamelPrefixedProperties.java b/core/camel-spring-boot/src/test/java/com/example/springboot/CamelPrefixedProperties.java new file mode 100644 index 00000000000..2f04bdcbb2e --- /dev/null +++ b/core/camel-spring-boot/src/test/java/com/example/springboot/CamelPrefixedProperties.java @@ -0,0 +1,38 @@ +/* + * 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 com.example.springboot; + +import javax.net.ssl.HostnameVerifier; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * A configuration class that binds camel.* properties without living in the org.apache.camel packages, as a starter + * outside this repository could. + */ +@ConfigurationProperties(prefix = "camel.example.thirdparty") +public class CamelPrefixedProperties { + + private HostnameVerifier verifier; + + public HostnameVerifier getVerifier() { + return verifier; + } + + public void setVerifier(HostnameVerifier verifier) { + this.verifier = verifier; + } +} diff --git a/core/camel-spring-boot/src/test/java/com/example/springboot/ThirdPartyProperties.java b/core/camel-spring-boot/src/test/java/com/example/springboot/ThirdPartyProperties.java new file mode 100644 index 00000000000..f62df2bbb99 --- /dev/null +++ b/core/camel-spring-boot/src/test/java/com/example/springboot/ThirdPartyProperties.java @@ -0,0 +1,38 @@ +/* + * 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 com.example.springboot; + +import javax.net.ssl.HostnameVerifier; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Stands in for an application or third party {@code @ConfigurationProperties} class that happens to have a field of a + * type one of the generated Camel converters registers itself for. Deliberately outside the org.apache.camel packages. + */ +@ConfigurationProperties(prefix = "thirdparty.example") +public class ThirdPartyProperties { + + private HostnameVerifier verifier; + + public HostnameVerifier getVerifier() { + return verifier; + } + + public void setVerifier(HostnameVerifier verifier) { + this.verifier = verifier; + } +} diff --git a/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/util/BeanReferenceHelperTest.java b/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/util/BeanReferenceHelperTest.java index ac8b807c82a..9773db941fe 100644 --- a/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/util/BeanReferenceHelperTest.java +++ b/core/camel-spring-boot/src/test/java/org/apache/camel/spring/boot/util/BeanReferenceHelperTest.java @@ -16,15 +16,21 @@ */ package org.apache.camel.spring.boot.util; +import java.lang.reflect.Method; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLSession; +import com.example.springboot.CamelPrefixedProperties; +import com.example.springboot.ThirdPartyProperties; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.MethodParameter; import org.springframework.core.convert.TypeDescriptor; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -49,6 +55,11 @@ public class BeanReferenceHelperTest { return new MyOption(); } + @Bean(name = "myVerifier") + HostnameVerifier myVerifier() { + return (String hostname, SSLSession session) -> true; + } + @Bean(name = "myOtherOption") MyOtherOption myOtherOption() { return new MyOtherOption(); @@ -154,7 +165,93 @@ public class BeanReferenceHelperTest { IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> BeanReferenceHelper.resolveBeanReference(applicationContext, "nope", TypeDescriptor.valueOf(MyOtherOption.class), PREFIX)); - assertEquals(true, e.getMessage().contains(MyOtherOption.class.getName())); + assertTrue(e.getMessage().contains(MyOtherOption.class.getName()), e.getMessage()); + } + + @Test + public void testTypeOfAnUnrelatedBeanFailsClosed() { + // #type: resolves a single bean of the named type, which need not be assignable to the option + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> resolve("#type:" + MyOtherOption.class.getName())); + assertTrue(e.getMessage().contains("not assignable to the option type"), e.getMessage()); + assertTrue(e.getMessage().contains(MyOtherOption.class.getName()), e.getMessage()); + } + + // ************************************* + // The generated converters are registered with @ConfigurationPropertiesBinding and so take part in every + // @ConfigurationProperties binding in the application, not only in Camel's own. + // ************************************* + + private static TypeDescriptor verifierOf(Class<?> owner) throws Exception { + Method setter = owner.getMethod("setVerifier", HostnameVerifier.class); + return new TypeDescriptor(new MethodParameter(setter, 0)); + } + + @Test + public void testTargetInCamelPackageIsCamelsOwn() throws Exception { + assertTrue(BeanReferenceHelper.isCamelConfigurationTarget(verifierOf(CamelStyleProperties.class))); + } + + @Test + public void testTargetWithCamelPrefixIsCamelsOwn() throws Exception { + assertTrue(BeanReferenceHelper.isCamelConfigurationTarget(verifierOf(CamelPrefixedProperties.class))); + } + + @Test + public void testThirdPartyTargetIsNotCamelsOwn() throws Exception { + assertFalse(BeanReferenceHelper.isCamelConfigurationTarget(verifierOf(ThirdPartyProperties.class))); + } + + @Test + public void testUnknownTargetIsTreatedAsCamelsOwn() { + // a TypeDescriptor that does not carry the bound class must never weaken Camel's own binding + assertTrue(BeanReferenceHelper.isCamelConfigurationTarget(TypeDescriptor.valueOf(HostnameVerifier.class))); + assertTrue(BeanReferenceHelper.isCamelConfigurationTarget(null)); + } + + @Test + public void testThirdPartyBindingIsNotIntercepted() throws Exception { + // this is what a third party class with a field of a type a starter registers for used to get, and + // adding a starter to the classpath must not turn it into a startup failure + assertNull(BeanReferenceHelper.resolveBeanReference(applicationContext, "someValue", + verifierOf(ThirdPartyProperties.class), PREFIX)); + } + + @Test + public void testThirdPartyBindingStillResolvesHashReferences() throws Exception { + assertSame(applicationContext.getBean("myVerifier"), BeanReferenceHelper + .resolveBeanReference(applicationContext, "#bean:myVerifier", verifierOf(ThirdPartyProperties.class), + PREFIX)); + } + + @Test + public void testCamelBindingResolvesHashReferences() throws Exception { + assertSame(applicationContext.getBean("myVerifier"), BeanReferenceHelper + .resolveBeanReference(applicationContext, "#bean:myVerifier", verifierOf(CamelStyleProperties.class), + PREFIX)); + } + + @Test + public void testCamelBindingResolvesPlainBeanIds() throws Exception { + assertSame(applicationContext.getBean("myVerifier"), BeanReferenceHelper + .resolveBeanReference(applicationContext, "myVerifier", verifierOf(CamelStyleProperties.class), + PREFIX)); + } + + /** + * Stands in for a generated {@code *ComponentConfiguration}, which always lives under org.apache.camel. + */ + public static class CamelStyleProperties { + + private HostnameVerifier verifier; + + public HostnameVerifier getVerifier() { + return verifier; + } + + public void setVerifier(HostnameVerifier verifier) { + this.verifier = verifier; + } } } diff --git a/tooling/camel-spring-boot-generator-maven-plugin/src/main/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojo.java b/tooling/camel-spring-boot-generator-maven-plugin/src/main/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojo.java index f9888d27724..6a33b36e52b 100644 --- a/tooling/camel-spring-boot-generator-maven-plugin/src/main/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojo.java +++ b/tooling/camel-spring-boot-generator-maven-plugin/src/main/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojo.java @@ -89,6 +89,12 @@ public class SpringBootAutoConfigurationMojo extends AbstractSpringBootGenerator */ private static final boolean DELETE_FILES_ON_MAIN_ARTIFACTS = false; + /** + * Option names owned by the Spring Boot auto configuration layer, see + * {@code CamelPropertiesHelper.copyConfigurationProperties}. + */ + private static final Set<String> RESERVED_OPTION_NAMES = Set.of("enabled", "customizer"); + private static final Map<String, String> PRIMITIVEMAP; private static final Map<Type, Type> PRIMITIVE_CLASSES; @@ -712,6 +718,8 @@ public class SpringBootAutoConfigurationMojo extends AbstractSpringBootGenerator for (ComponentOptionModel option : model.getComponentOptions()) { + checkReservedOptionName("component", model.getScheme(), option.getName()); + if (skipComponentOption(model, option)) { // some component options should be skipped continue; @@ -1164,6 +1172,7 @@ public class SpringBootAutoConfigurationMojo extends AbstractSpringBootGenerator .setStringValue("prefix", prefix); for (DataFormatOptionModel option : model.getOptions()) { + checkReservedOptionName("data format", model.getName(), option.getName()); // skip option with name id in data format as we do not need that if ("id".equals(option.getName())) { continue; @@ -1268,6 +1277,7 @@ public class SpringBootAutoConfigurationMojo extends AbstractSpringBootGenerator .setStringValue("prefix", prefix); for (LanguageOptionModel option : model.getOptions()) { + checkReservedOptionName("language", model.getName(), option.getName()); // skip option with name id, or expression in language as we do not // need that and skip resultType as they are not global options if ("id".equals(option.getName()) || "expression".equals(option.getName()) @@ -1473,6 +1483,23 @@ public class SpringBootAutoConfigurationMojo extends AbstractSpringBootGenerator writeComponentSpringFactorySource(packageName, name); } + /** + * The generated customizers strip the options owned by the Spring Boot auto configuration layer itself before + * binding the configuration onto the Camel target, because they are not options on that target. No Camel + * component, data format or language declares an option with one of these names today, and this check makes sure + * that a catalog option carrying one of them can never be dropped silently by that stripping. + */ + static void checkReservedOptionName(String kind, String name, String optionName) + throws MojoFailureException { + if (RESERVED_OPTION_NAMES.contains(optionName.toLowerCase(Locale.US))) { + throw new MojoFailureException( + "The " + kind + " " + name + " declares an option named " + optionName + + ", which collides with the Spring Boot auto configuration layer. Such an option is removed" + + " before the configuration is copied onto the Camel target, so it would never take effect." + + " Rename the option, or teach CamelPropertiesHelper to tell the two apart."); + } + } + private static String componentPropertyPrefix(ComponentModel model, String overrideComponentName) { return ("camel.component." + camelCaseToDash(overrideComponentName != null ? overrideComponentName : model.getScheme())) diff --git a/tooling/camel-spring-boot-generator-maven-plugin/src/test/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojoTest.java b/tooling/camel-spring-boot-generator-maven-plugin/src/test/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojoTest.java index e41d3ae1f35..5b963c0ec7b 100644 --- a/tooling/camel-spring-boot-generator-maven-plugin/src/test/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojoTest.java +++ b/tooling/camel-spring-boot-generator-maven-plugin/src/test/java/org/apache/camel/springboot/maven/SpringBootAutoConfigurationMojoTest.java @@ -16,10 +16,13 @@ */ package org.apache.camel.springboot.maven; +import org.apache.maven.plugin.MojoFailureException; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for the code that {@link SpringBootAutoConfigurationMojo} generates into the starters. @@ -73,4 +76,18 @@ class SpringBootAutoConfigurationMojoTest { assertThat(body).doesNotContain("CamelPropertiesHelper.copyProperties("); } + @Test + @DisplayName("A catalog option named enabled or customizer fails the build") + void testReservedOptionNamesAreRejected() { + assertThatThrownBy(() -> SpringBootAutoConfigurationMojo.checkReservedOptionName("component", "foo", "enabled")) + .isInstanceOf(MojoFailureException.class) + .hasMessageContaining("enabled") + .hasMessageContaining("would never take effect"); + assertThatThrownBy( + () -> SpringBootAutoConfigurationMojo.checkReservedOptionName("data format", "foo", "Customizer")) + .isInstanceOf(MojoFailureException.class); + assertThatCode(() -> SpringBootAutoConfigurationMojo.checkReservedOptionName("language", "foo", "trim")) + .doesNotThrowAnyException(); + } + }
