CAMEL-24704: camel-yaml-dsl - beans keyed by the bean name fail with the list form to write

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<BeanFactoryDefinition<?>> delayedRegistrations,

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<BeanFactoryDefinition<RouteTemplateDefinition>> items
                         = (List) asFlatList(node, BeanFactoryDefinition.class);
                 target.setTemplateBeans(items);

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<BeanFactoryDefinition<TemplatedRouteDefinition>> items
                         = (List) asFlatList(node, BeanFactoryDefinition.class);
                 target.setBeans(items);

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,

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<Error> 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<Error> errors = validator.validate("""

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()) {