Rename singleAttribute to collapsed and make it work for both Attributes and TextContent (#2585)

* Rename `singleAttribute` to `collapsed` in MCElement and update related usages.

* Refactor handling of `collapsed` elements and update inline parsing logic.

* Extract collapsed element tests to `YAMLParsingCollapsedTest` for better organization.

* coderabbitai suggestions

* comment improvement

* .

Author: christiangoerdes

annot/src/main/java/com/predic8/membrane/annot/MCElement.java

@@ -69,7 +69,14 @@
      * Whether the element has only one attribute.
      * Enables inline yaml object configuration
      * e.g.
+     * <pre><code>
      * allow: foo
+     * </code></pre>
+     * instead of
+     * <pre><code>
+     * allow:
+     *   value: foo
+     *  </code></pre>
      */
-    boolean singleAttribute() default false;
+    boolean collapsed() default false;
 }

annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java

@@ -110,7 +110,7 @@ private void addParserDefinitions(Model m, MainInfo main) {
         }
     }
 
-    private SchemaObject createParser(Model m, MainInfo main, ElementInfo elementInfo) {
+    private AbstractSchema<?> createParser(Model m, MainInfo main, ElementInfo elementInfo) {
         String parserName = elementInfo.getXSDTypeName(m);
 
         if (isComponentsMap(elementInfo)) {
@@ -133,6 +133,14 @@ private SchemaObject createParser(Model m, MainInfo main, ElementInfo elementInf
             return ref(parserName).ref("#/$defs/%sParser".formatted(childName));
         }
 
+        // enforce the inline form for collapsed elements
+        if (elementInfo.getAnnotation().collapsed()) {
+            if (elementInfo.getAnnotation().noEnvelope()) {
+                throw new ProcessingException("@MCElement(collapsed=true) is not compatible with noEnvelope=true.", elementInfo.getElement());
+            }
+            return createCollapsedInlineParser(elementInfo, parserName);
+        }
+
         SchemaObject parser = getParserSchemaObject(elementInfo, parserName);
 
         collectProperties(m, main, elementInfo, parser);
@@ -144,50 +152,53 @@ private SchemaObject createParser(Model m, MainInfo main, ElementInfo elementInf
                     .required(false));
         }
 
-        // allow scalar inline form for single-attribute elements
-        if (elementInfo.getAnnotation().singleAttribute()) {
-            return anyOf(List.of(
-                    parser,
-                    createSingleAttributeInlineVariant(elementInfo)
-            )).name(parserName);
-        }
-
         return parser;
     }
 
-    private AbstractSchema<?> createSingleAttributeInlineVariant(ElementInfo ei) {
-        // Only count attributes that are actually part of the schema.
-        var attrs = ei.getAis().stream()
-                .filter(ai -> !ai.excludedFromJsonSchema())
-                .toList();
+    private AbstractSchema<?> createCollapsedInlineParser(ElementInfo ei, String parserName) {
+        var attrs = ei.getAis().stream().toList();
+
+        boolean hasText = ei.getTci() != null;
+        boolean hasChildren = !ei.getChildElementSpecs().isEmpty();
 
-        if (attrs.size() != 1) {
-            throw new ProcessingException(
-                    "@MCElement(singleAttribute=true) requires exactly one @MCAttribute.",
-                    ei.getElement()
-            );
+        if (hasChildren) {
+            throw new ProcessingException("@MCElement(collapsed=true) must not declare child elements.", ei.getElement());
         }
-        if (ei.getTci() != null || !ei.getChildElementSpecs().isEmpty()) {
-            throw new ProcessingException(
-                    "@MCElement(singleAttribute=true) must not declare text content or child elements.",
-                    ei.getElement()
-            );
+        if (hasText && !attrs.isEmpty()) {
+            throw new ProcessingException("@MCElement(collapsed=true) must not mix @MCTextContent with @MCAttribute.", ei.getElement());
         }
 
-        AttributeInfo ai = attrs.getFirst();
-        String type = ai.getSchemaType(processingEnv.getTypeUtils());
+        // collapsed via single @MCTextContent -> scalar string
+        if (hasText) {
+            return SchemaFactory.from("string")
+                    .name(parserName)
+                    .type("string")
+                    .description(getDescriptionContent(ei));
+        }
 
-        AbstractSchema<?> s = SchemaFactory.from(type)
-                .type(type)
-                .description(getDescriptionContent(ai));
+        // collapsed via a single @MCAttribute-> scalar of that attribute type
+        if (attrs.size() == 1) {
+            AttributeInfo ai = attrs.getFirst();
+            String type = ai.getSchemaType(processingEnv.getTypeUtils());
 
-        if (ai.isEnum(processingEnv.getTypeUtils()) && !"boolean".equals(type)) {
-            s.enumValues(ai.enumsAsLowerCaseList(processingEnv.getTypeUtils()));
+            AbstractSchema<?> s = SchemaFactory.from(type)
+                    .name(parserName)
+                    .type(type)
+                    .description(getDescriptionContent(ai).isEmpty() ? getDescriptionContent(ei) : getDescriptionContent(ai));
+
+            if (ai.isEnum(processingEnv.getTypeUtils()) && !"boolean".equals(type)) {
+                s.enumValues(ai.enumsAsLowerCaseList(processingEnv.getTypeUtils()));
+            }
+            return s;
         }
 
-        return s;
+        throw new ProcessingException(
+                "@MCElement(collapsed=true) requires exactly one @MCAttribute or exactly one @MCTextContent.",
+                ei.getElement()
+        );
     }
 
+
     private SchemaObject getParserSchemaObject(ElementInfo elementInfo, String parserName) {
         return object(parserName)
                 .additionalProperties( elementInfo.isString())

annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java

@@ -25,6 +25,8 @@
 import com.networknt.schema.SchemaRegistry;
 import com.predic8.membrane.annot.Grammar;
 import com.predic8.membrane.annot.MCAttribute;
+import com.predic8.membrane.annot.MCElement;
+import com.predic8.membrane.annot.MCTextContent;
 import com.predic8.membrane.annot.beanregistry.BeanDefinition;
 import com.predic8.membrane.annot.beanregistry.BeanLifecycleManager;
 import com.predic8.membrane.annot.beanregistry.BeanRegistry;
@@ -38,6 +40,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
@@ -51,6 +54,7 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.List.of;
 import static java.util.UUID.randomUUID;
+import static org.springframework.util.ReflectionUtils.doWithMethods;
 
 public class GenericYamlParser {
     private static final Logger log = LoggerFactory.getLogger(GenericYamlParser.class);
@@ -188,16 +192,18 @@ public static <T> T createAndPopulateNode(ParsingContext<?> ctx, Class<T> clazz,
                 return configObj;
             }
 
-            // scalar inline form for @MCElement(singleAttribute=true)
-            if (!node.isObject()) {
-                if (isSingleAttribute(clazz)) {
-                    applySingleAttributeScalar(clazz, node, configObj);
-                    return handlePostConstructAndPreDestroy(ctx, configObj);
+            // scalar inline form for @MCElement(collapsed=true)
+            if (isCollapsed(clazz)) {
+                if (node.isNull()) {
+                    throw new ParsingException("Collapsed element must not be null.", node);
                 }
-                // anything non-object is invalid here
-                ensureMappingStart(node);
+                if (node.isArray() || node.isObject()) {
+                    throw new ParsingException("Element is collapsed; expected an inline scalar value, not " +
+                            (node.isArray() ? "an array" : "an object") + ".", node);
+                }
+                applyCollapsedScalar(clazz, node, configObj);
+                return handlePostConstructAndPreDestroy(ctx, configObj);
             }
-
             ensureMappingStart(node);
             if (isNoEnvelope(clazz))
                 throw new RuntimeException("Class " + clazz.getName() + " is annotated with @MCElement(noEnvelope=true), but the YAML/JSON structure does not contain a list.");
@@ -343,7 +349,7 @@ private static <T> T handlePostConstructAndPreDestroy(ParsingContext<?> ctx, T b
         if (bean instanceof BeanRegistryAware beanRegistryAware) {
             beanRegistryAware.setRegistry(ctx.registry());
         }
-        ReflectionUtils.doWithMethods(bean.getClass(), method -> {
+        doWithMethods(bean.getClass(), method -> {
             if (method.isAnnotationPresent(PostConstruct.class)) {
                 try {
                     method.setAccessible(true);
@@ -362,19 +368,27 @@ private static <T> T handlePostConstructAndPreDestroy(ParsingContext<?> ctx, T b
         return bean;
     }
 
-    private static <T> void applySingleAttributeScalar(Class<T> clazz, JsonNode node, T target) {
+    private static <T> void applyCollapsedScalar(Class<T> clazz, JsonNode node, T target) {
         if (node == null || node.isNull()) {
-            throw new ParsingException("singleAttribute element must not be null.", node);
+            throw new ParsingException("Collapsed element must not be null.", node);
         }
 
-        Method setter = findSingleAttributeSetter(clazz);
+        Method attributeSetter = findSingleSetterOrNullForAnnotation(clazz, MCAttribute.class);
+        Method textSetter = findSingleSetterOrNullForAnnotation(clazz, MCTextContent.class);
+
+        if ((attributeSetter == null) == (textSetter == null)) {
+            // both null or both non-null -> invalid
+            throw new ParsingException("@MCElement(collapsed=true) requires exactly one @MCAttribute setter OR exactly one @MCTextContent setter.", node);
+        }
 
+        Method setter = (attributeSetter != null) ? attributeSetter : textSetter;
         Class<?> paramType = setter.getParameterTypes()[0];
+
         Object value;
         try {
             value = SCALAR_MAPPER.convertValue(node, paramType);
         } catch (IllegalArgumentException e) {
-            throw new ParsingException("Cannot convert scalar value to " + paramType.getSimpleName() + ".", node);
+            throw new ParsingException("Cannot convert inline value to " + paramType.getSimpleName() + ".", node);
         }
 
         try {
@@ -387,20 +401,22 @@ private static <T> void applySingleAttributeScalar(Class<T> clazz, JsonNode node
         }
     }
 
-    private static Method findSingleAttributeSetter(Class<?> clazz) {
+    private static Method findSingleSetterOrNullForAnnotation(Class<?> clazz, Class<? extends java.lang.annotation.Annotation> annotation) {
         List<Method> setters = new ArrayList<>();
-        ReflectionUtils.doWithMethods(clazz, m -> {
-            if (m.isAnnotationPresent(MCAttribute.class) && m.getParameterCount() == 1) {
+        doWithMethods(clazz, m -> {
+            if (m.isAnnotationPresent(annotation) && m.getParameterCount() == 1) {
                 setters.add(m);
             }
         });
 
+        if (setters.isEmpty()) return null;
         if (setters.size() != 1) {
             throw new ParsingException(
-                    "@MCElement(singleAttribute=true) requires exactly one @MCAttribute setter, but found " + setters.size() + ".",
+                    "Multiple @%s setters found for collapsed element.".formatted(annotation.getSimpleName()),
                     JsonNodeFactory.instance.nullNode()
             );
         }
         return setters.getFirst();
     }
+
 }

annot/src/main/java/com/predic8/membrane/annot/yaml/McYamlIntrospector.java

@@ -194,9 +194,9 @@ public static String getElementName(Class<?> type) {
         return type.getSimpleName();
     }
 
-    public static boolean isSingleAttribute(Class<?> clazz) {
+    public static boolean isCollapsed(Class<?> clazz) {
         MCElement el = clazz.getAnnotation(MCElement.class);
-        return el != null && el.singleAttribute();
+        return el != null && el.collapsed();
     }
 
 }