diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/BeansDeserializer.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/BeansDeserializer.java index 92b4ef071ea1b..46c58236ec002 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/BeansDeserializer.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/BeansDeserializer.java @@ -27,6 +27,7 @@ import org.apache.camel.dsl.yaml.common.YamlDeserializationContext; import org.apache.camel.dsl.yaml.common.YamlDeserializerResolver; import org.apache.camel.dsl.yaml.common.YamlDeserializerSupport; +import org.apache.camel.dsl.yaml.common.exception.YamlDeserializationException; import org.apache.camel.model.BeanFactoryDefinition; import org.apache.camel.model.BeanModelHelper; import org.apache.camel.model.Model; @@ -39,7 +40,10 @@ import org.apache.camel.support.PojoBeanHelper; import org.apache.camel.util.ObjectHelper; import org.snakeyaml.engine.v2.api.ConstructNode; +import org.snakeyaml.engine.v2.nodes.MappingNode; import org.snakeyaml.engine.v2.nodes.Node; +import org.snakeyaml.engine.v2.nodes.NodeTuple; +import org.snakeyaml.engine.v2.nodes.ScalarNode; import org.snakeyaml.engine.v2.nodes.SequenceNode; @YamlIn @@ -57,6 +61,7 @@ public class BeansDeserializer extends YamlDeserializerSupport implements Constr @Override public Object construct(Node node) { final BeansCustomizer answer = new BeansCustomizer(); + checkBeansIsAList(node); final SequenceNode sn = asSequenceNode(node); final YamlDeserializationContext dc = getDeserializationContext(node); @@ -100,6 +105,42 @@ public void clearCache() { beanCache.clear(); } + /** + * Fails with what to write when beans: is written keyed by the bean name, as a map (beans: {myBean: {type: ...}}) + * or as list items (- myBean: {type: ...}), which is what people and AI models tend to write first (CAMEL-24704). + * The list with name: is the only form of beans: (also in routeTemplate and templatedRoute). + */ + static void checkBeansIsAList(Node node) { + if (node instanceof MappingNode mn) { + String name = "myBean"; + if (!mn.getValue().isEmpty()) { + NodeTuple first = mn.getValue().get(0); + String key = first.getKeyNode() instanceof ScalarNode sk ? sk.getValue() : name; + // beans:\n name: myBean\n type: ...: the "- " of the list item was forgotten + if ("name".equals(key) && first.getValueNode() instanceof ScalarNode sv) { + key = sv.getValue(); + } + name = key; + } + throw new YamlDeserializationException( + node, "beans is a list: each bean starts with \"- \": - name: " + name + + " followed by type: \"#class:com.example.MyBean\" (indented under the -)"); + } + if (node instanceof SequenceNode sn) { + for (Node item : sn.getValue()) { + // - myBean: {type: ...}: a single key holding the bean, which is not one of its map properties + if (item instanceof MappingNode mn && mn.getValue().size() == 1 + && mn.getValue().get(0).getKeyNode() instanceof ScalarNode sk + && mn.getValue().get(0).getValueNode() instanceof MappingNode + && !Set.of("properties", "constructors").contains(sk.getValue())) { + throw new YamlDeserializationException( + item, "a bean item is written as - name: " + sk.getValue() + + " followed by type: \"#class:com.example.MyBean\" (the name is a property, not the key)"); + } + } + } + } + protected void registerBean( CamelContext camelContext, List> delayedRegistrations, diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/RouteTemplateDefinitionDeserializer.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/RouteTemplateDefinitionDeserializer.java index 2322b47349f3e..2303041846933 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/RouteTemplateDefinitionDeserializer.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/RouteTemplateDefinitionDeserializer.java @@ -100,6 +100,7 @@ protected boolean setProperty( break; } case "beans": { + BeansDeserializer.checkBeansIsAList(node); List> items = (List) asFlatList(node, BeanFactoryDefinition.class); target.setTemplateBeans(items); diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/TemplatedRouteDefinitionDeserializer.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/TemplatedRouteDefinitionDeserializer.java index 93f4f62a4cbcf..8341ca666a7b8 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/TemplatedRouteDefinitionDeserializer.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/main/java/org/apache/camel/dsl/yaml/deserializers/TemplatedRouteDefinitionDeserializer.java @@ -88,6 +88,7 @@ protected boolean setProperty( break; } case "beans": { + BeansDeserializer.checkBeansIsAList(node); List> items = (List) asFlatList(node, BeanFactoryDefinition.class); target.setBeans(items); diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java index d8ad480acf0a6..f73aaf52c2e9c 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java @@ -470,11 +470,11 @@ static String expressionKeyHint(String value) { m -> "steps: is a property of from:, next to uri:; a steps: written at the route level must be" + " indented under from:"), // - id: myBean / class: ... : the bean properties are name and type - unknownProperty("/\\d+/beans/\\d+", m -> Set.of("id", "ref", "class").contains(m.unknown()), + unknownProperty(".*/beans/\\d+", m -> Set.of("id", "ref", "class").contains(m.unknown()), m -> "a bean is - name: myBean followed by type: \"#class:com.example.MyBean\" (name instead of " + m.unknown() + (m.unknown().equals("class") ? ", type instead of class" : "") + ")"), // - myBean: {type: ...} instead of - name: myBean / type: ... - unknownProperty("/\\d+/beans/\\d+", ANY, + unknownProperty(".*/beans/\\d+", ANY, m -> "a bean item is written as - name: " + m.unknown() + " followed by type: \"#class:com.example.MyBean\" (the name is a property, not the key)"), unknownProperty(null, diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java index 7def87e4ed955..dbeb8e2d6a8ad 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java @@ -428,6 +428,45 @@ public void testBeanItemKeyedByNameSaysHowToWriteIt() throws Exception { assertThat(errors).anyMatch(e -> e.getMessage().contains("a bean item is written as - name: myBean")); } + @Test + public void testTemplateBeansKeyedByNameSayHowToWriteThem() throws Exception { + // CAMEL-24704: the beans of a routeTemplate and a templatedRoute are the same list as the top-level beans + List errors = validator.validate(""" + - routeTemplate: + id: myTemplate + beans: + myBean: + type: "#class:com.example.MyBean" + from: + uri: "direct:{{name}}" + steps: + - to: "mock:result" + """); + assertThat(errors).anyMatch(e -> e.getMessage().contains("beans is a list: - name: myBean")); + + errors = validator.validate(""" + - templatedRoute: + routeTemplateRef: myTemplate + beans: + - myBean: + type: "#class:com.example.MyBean" + """); + assertThat(errors).anyMatch(e -> e.getMessage().contains("a bean item is written as - name: myBean")); + + errors = validator.validate(""" + - routeTemplate: + id: myTemplate + beans: + - id: myBean + type: "#class:com.example.MyBean" + from: + uri: "direct:{{name}}" + steps: + - to: "mock:result" + """); + assertThat(errors).anyMatch(e -> e.getMessage().contains("(name instead of id)")); + } + @Test public void testJbangDepsLineIsNamed() throws Exception { List errors = validator.validate(""" diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/java/org/apache/camel/dsl/yaml/BeansTest.java b/dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/java/org/apache/camel/dsl/yaml/BeansTest.java index 207f756a1b80f..d99692b595d08 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/java/org/apache/camel/dsl/yaml/BeansTest.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl/src/test/java/org/apache/camel/dsl/yaml/BeansTest.java @@ -406,6 +406,89 @@ void beansClassNotFoundThatIsNotABuiltInBeanKeepsTheGenericHint() { assertThat(msg.contains("did you mean")).isFalse(); } + // CAMEL-24704: beans keyed by the bean name, as most other formats write them, fail with the list form to write + // instead of "Node type map is invalid, expected array" / "Unsupported field: myProcessor" + + @Test + void beansAsMapSaysBeansIsAList() { + Exception e = assertThrows(Exception.class, () -> loadRoutesNoValidate(""" + - beans: + myProcessor: + type: %s + """.formatted(MyBean.class.getName()))); + + assertThat(messages(e)).contains("beans is a list: each bean starts with \"- \": - name: myProcessor"); + } + + @Test + void beansListWithoutDashSaysBeansIsAList() { + // the "- " was forgotten: the bean reads as a map whose first key is name, so the hint uses its value + Exception e = assertThrows(Exception.class, () -> loadRoutesNoValidate(""" + - beans: + name: myProcessor + type: %s + """.formatted(MyBean.class.getName()))); + + assertThat(messages(e)).contains("beans is a list: each bean starts with \"- \": - name: myProcessor"); + } + + @Test + void beanItemKeyedByNameSaysNameIsAProperty() { + Exception e = assertThrows(Exception.class, () -> loadRoutesNoValidate(""" + - beans: + - name: first + type: %s + - myProcessor: + type: %s + """.formatted(MyBean.class.getName(), MyBean.class.getName()))); + + assertThat(messages(e)) + .contains("a bean item is written as - name: myProcessor") + .contains("(the name is a property, not the key)"); + } + + @Test + void beanItemWithOnlyPropertiesIsNotReadAsKeyedByName() { + // properties: is a map property of the bean, not its name: the error is the missing name + Exception e = assertThrows(Exception.class, () -> loadRoutesNoValidate(""" + - beans: + - properties: + field1: f1 + """)); + + assertThat(messages(e)).contains("The bean name must be set").doesNotContain("a bean item is written as"); + } + + @Test + void routeTemplateBeansAsMapSaysBeansIsAList() { + Exception e = assertThrows(Exception.class, () -> loadRoutesNoValidate(""" + - routeTemplate: + id: myTemplate + beans: + myProcessor: + type: %s + from: + uri: "direct:{{directName}}" + steps: + - to: "mock:result" + """.formatted(MyBean.class.getName()))); + + assertThat(messages(e)).contains("beans is a list: each bean starts with \"- \": - name: myProcessor"); + } + + @Test + void templatedRouteBeanItemKeyedByNameSaysNameIsAProperty() { + Exception e = assertThrows(Exception.class, () -> loadRoutesNoValidate(""" + - templatedRoute: + routeTemplateRef: myTemplate + beans: + - myProcessor: + type: %s + """.formatted(MyBean.class.getName()))); + + assertThat(messages(e)).contains("a bean item is written as - name: myProcessor"); + } + private static String messages(Throwable e) { StringBuilder sb = new StringBuilder(); for (Throwable t = e; t != null; t = t.getCause()) {