gnodet-bot commented on code in PR #26611: URL: https://github.com/apache/camel/pull/26611#discussion_r4052653815
########## core/camel-core/src/test/java/org/apache/camel/model/BeanModelHelperInferredBuilderTest.java: ########## @@ -0,0 +1,348 @@ +/* + * 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.model; + +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; + +import org.apache.camel.ContextTestSupport; +import org.apache.camel.support.PropertyBindingSupport; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CAMEL-24820: a bean whose type has no public no-arg constructor but a builder() or newBuilder() method (Lombok, + * Immutables, LangChain4j, AWS SDK, protobuf, ...) is created via its builder, so it can be declared with only the type + * and properties, without builderClass and builderMethod. + */ +public class BeanModelHelperInferredBuilderTest extends ContextTestSupport { + + @Override + public boolean isUseRouteBuilder() { + return false; + } + + private BeanFactoryDefinition<?> bean(String type, Map<String, Object> properties) { + BeanFactoryDefinition<?> def = new BeanFactoryDefinition<>(); + def.setName("myBean"); + def.setType(type); + def.setProperties(properties); + return def; + } + + private static Map<String, Object> props(Object... kv) { + Map<String, Object> map = new HashMap<>(); + for (int i = 0; i < kv.length; i += 2) { + map.put((String) kv[i], kv[i + 1]); + } + return map; + } + + @Test + public void testBuilderInferred() throws Exception { + // the same shape as dev.langchain4j.model.ollama.OllamaChatModel: builder() and a nested builder with build() + Object out = BeanModelHelper.newInstance( + bean(ChatModel.class.getName(), props("baseUrl", "http://localhost:11434";, "modelName", "qwen2.5", + "temperature", "0.0", "timeout", "120s")), + context); + + ChatModel model = assertInstanceOf(ChatModel.class, out); + assertEquals("http://localhost:11434";, model.baseUrl); + assertEquals("qwen2.5", model.modelName); + assertEquals(0.0, model.temperature); + assertEquals(Duration.ofSeconds(120), model.timeout); + } + + @Test + public void testBuilderInferredWithClassPrefix() throws Exception { + Object out = BeanModelHelper.newInstance( + bean("#class:" + ChatModel.class.getName(), props("modelName", "llama3")), context); + assertEquals("llama3", assertInstanceOf(ChatModel.class, out).modelName); + } + + @Test + public void testBuilderInferredWithoutProperties() throws Exception { + Object out = BeanModelHelper.newInstance(bean(ChatModel.class.getName(), null), context); + ChatModel model = assertInstanceOf(ChatModel.class, out); + assertNull(model.modelName); + assertEquals(Duration.ofSeconds(60), model.timeout, "the builder default"); + } + + @Test + public void testNewBuilderAndNoBuildMethodInferred() throws Exception { + // newBuilder() as the JDK HttpClient and protobuf, and create() as the only method that returns the type + Object out = BeanModelHelper.newInstance( + bean(Channel.class.getName(), props("host", "localhost", "port", "8080")), context); + Channel channel = assertInstanceOf(Channel.class, out); + assertEquals("localhost:8080", channel.address); + } + + @Test + public void testBuilderMethodOverridesInferred() throws Exception { + // the builder is still inferred, only the method that creates the bean is given + BeanFactoryDefinition<?> def = bean(Ambiguous.class.getName(), props("name", "x")); + def.setBuilderMethod("large"); + + Ambiguous out = assertInstanceOf(Ambiguous.class, BeanModelHelper.newInstance(def, context)); + assertEquals("large x", out.size); + } + + @Test + public void testPropertiesTheBuilderDoesNotTakeAreSetOnTheBean() throws Exception { + Object out = BeanModelHelper.newInstance( + bean(ChatModel.class.getName(), props("modelName", "qwen2.5", "label", "support")), context); + ChatModel model = assertInstanceOf(ChatModel.class, out); + assertEquals("qwen2.5", model.modelName); + assertEquals("support", model.label, "label has a setter on the bean, not on the builder"); + } + + @Test + public void testUnknownPropertyFails() { + Exception e = assertThrows(Exception.class, () -> BeanModelHelper.newInstance( + bean(ChatModel.class.getName(), props("modelName", "qwen2.5", "unknown", "x")), context)); + e.printStackTrace(); Review Comment: 🐛 **Debug artifact — remove before merge.** `e.printStackTrace()` is a leftover from debugging. The assertion on the next line already captures the failure message; `printStackTrace` produces noise in the CI log and is never acceptable in committed test code. ```suggestion String msg = e.getMessage(); ``` ########## core/camel-support/src/main/java/org/apache/camel/support/PropertyBindingSupport.java: ########## @@ -1233,6 +1239,150 @@ private static boolean isReferenceParameter(Object obj) { return true; } + /** + * The names of the public static no-arg factory methods that, by convention, return the builder of a class: Lombok + * (@Builder), Immutables, LangChain4j, Spring AI, AWS SDK v2, Jackson, MongoDB and OpenTelemetry use builder(); the + * JDK HttpClient, protobuf, gRPC, Caffeine and Guava use newBuilder(). + */ + private static final String[] BUILDER_FACTORY_METHODS = { "builder", "newBuilder" }; + + /** + * The name of the method that, by convention, creates the bean from its builder. + */ + public static final String DEFAULT_BUILDER_METHOD = "build"; + + /** + * Whether the given type must be created through its builder, because it has no public no-arg constructor but has a + * public static no-arg <tt>builder()</tt> or <tt>newBuilder()</tt> method, as classes built by Lombok, Immutables, + * LangChain4j, the AWS SDK, protobuf and many others have. Camel then creates a bean of the type via the builder + * instead of failing on the missing constructor, so such a class can be declared like any other bean, in YAML and + * XML as <tt>type</tt> and in properties as <tt>#class:</tt>, without naming the builder class. + * <p/> + * A class that has a public no-arg constructor is never regarded as builder-only, so that a class which offers both + * keeps being created with its constructor as before. + * + * @param type the class of the bean to create + * @return <tt>true</tt> if the type is created through an inferred builder, <tt>false</tt> if the type can be + * created with a public no-arg constructor, or has no recognised builder + */ + public static boolean isBuilderOnly(Class<?> type) { + return findBuilderFactoryMethod(type) != null; + } + + /** + * Creates the builder of the given type, when the type is {@link #isBuilderOnly(Class) builder-only}, by invoking + * its public static <tt>builder()</tt> or <tt>newBuilder()</tt> method. + * <p/> + * The properties of the bean are then set on the returned builder, and the bean is created by invoking the + * {@link #findBuilderMethod(Object, Class) builder method}, such as with {@link Builder#build(Class, String)}. Review Comment: 💡 **Broken `@link` — won't resolve at Javadoc build time.** `{@link #findBuilderMethod(Object, Class)}` refers to a non-existent overload: the actual method signature is `findBuilderMethod(Object, Class<?>, String)` (three parameters). Use the fully-qualified form to avoid the broken reference warning. ```suggestion * {@link #findBuilderMethod(Object, Class, String) builder method}, such as with {@link Builder#build(Class, String)}. ``` -- 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]
