Fix export of list attributes with custom Stapler converters

plugin/src/main/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfigurator.java

@@ -11,6 +11,7 @@
 import io.jenkins.plugins.casc.impl.attributes.DescribableAttribute;
 import io.jenkins.plugins.casc.model.CNode;
 import io.jenkins.plugins.casc.model.Mapping;
+import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -301,8 +302,52 @@ public CNode describe(T instance, ConfigurationContext context) throws Exception
             if (a != null) {
                 Object value = a.getValue(instance);
                 if (value != null) {
-                    Object converted = Stapler.CONVERT_UTILS.convert(value, a.getType());
-                    if (converted instanceof Collection || p.getType().isArray() || !a.isMultiple()) {
+                    Object converted;
+                    Class<?> targetType = a.getType();
+
+                    boolean targetIsCollectionOrArray =
+                            Collection.class.isAssignableFrom(targetType) || targetType.isArray();
+
+                    if (!targetIsCollectionOrArray
+                            && (value instanceof Collection || value.getClass().isArray())) {
+
+                        Iterable<?> iterable;
+                        if (value instanceof Collection) {
+                            iterable = (Collection<?>) value;
+                        } else {
+                            int length = Array.getLength(value);
+                            List<Object> list = new ArrayList<>(length);
+                            for (int j = 0; j < length; j++) {
+                                list.add(Array.get(value, j));
+                            }
+                            iterable = list;
+                        }
+
+                        List<Object> convertedList = new ArrayList<>();
+                        for (Object item : iterable) {
+                            if (item != null) {
+                                convertedList.add(Stapler.CONVERT_UTILS.convert(item, targetType));
+                            } else {
+                                convertedList.add(null);
+                            }
+                        }
+                        converted = convertedList;
+
+                    } else {
+                        converted = Stapler.CONVERT_UTILS.convert(value, targetType);
+                    }
+                    if (p.getType().isArray() && converted instanceof Collection<?> col) {
+                        Class<?> component = p.getType().getComponentType();
+                        Object array = Array.newInstance(component, col.size());
+                        int idx = 0;
+                        for (Object o : col) {
+                            Array.set(array, idx++, o);
+                        }
+                        args[i] = array;
+
+                    } else if (Set.class.isAssignableFrom(p.getType()) && converted instanceof Collection) {
+                        args[i] = new HashSet<>((Collection<?>) converted);
+                    } else if (converted instanceof Collection || !a.isMultiple()) {
                         args[i] = converted;
                     } else if (Set.class.isAssignableFrom(p.getType())) {
                         args[i] = Collections.singleton(converted);

test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfiguratorTest.java

@@ -30,12 +30,17 @@
 import io.jenkins.plugins.casc.model.Sequence;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.ParametersAreNonnullByDefault;
 import javax.annotation.PostConstruct;
+import org.apache.commons.beanutils.ConversionException;
+import org.apache.commons.beanutils.Converter;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -426,4 +431,225 @@ public ArrayConstructor(Foo[] anArray) {
             this.anArray = anArray;
         }
     }
+
+    @SuppressWarnings("ClassCanBeRecord")
+    public static class CustomItem {
+        private final String value;
+
+        @DataBoundConstructor
+        public CustomItem(String value) {
+            this.value = value;
+        }
+
+        public String getValue() {
+            return value;
+        }
+
+        @SuppressWarnings("unused")
+        public static class StaplerConverterImpl implements Converter {
+            @SuppressWarnings("unchecked")
+            @Override
+            public <T> T convert(Class<T> type, Object value) {
+                if (type == String.class && value instanceof CustomItem) {
+                    return (T) ("converted-" + ((CustomItem) value).getValue());
+                }
+                if (type == CustomItem.class && value instanceof String) {
+                    return (T) new CustomItem(((String) value).replace("converted-", ""));
+                }
+                if (type.isInstance(value)) {
+                    return (T) value;
+                }
+                throw new ConversionException(
+                        "Unsupported conversion from " + value.getClass().getName() + " to " + type.getName());
+            }
+        }
+    }
+
+    @SuppressWarnings("ClassCanBeRecord")
+    public static class CustomItemListHolder {
+        private final List<CustomItem> items;
+
+        @DataBoundConstructor
+        public CustomItemListHolder(List<CustomItem> items) {
+            this.items = items;
+        }
+
+        @SuppressWarnings("unused")
+        public List<CustomItem> getItems() {
+            return items;
+        }
+    }
+
+    @Test
+    @Issue("https://github.com/jenkinsci/configuration-as-code-plugin/issues/2346")
+    void exportWithCustomConverterIteratesOverList() throws Exception {
+        List<CustomItem> list = Arrays.asList(new CustomItem("A"), new CustomItem("B"));
+        CustomItemListHolder holder = new CustomItemListHolder(list);
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        final Configurator<Object> c = registry.lookupOrFail(CustomItemListHolder.class);
+        final ConfigurationContext context = new ConfigurationContext(registry);
+
+        CNode node = c.describe(holder, context);
+
+        assertNotNull(node);
+        assertInstanceOf(Mapping.class, node);
+        Mapping map = (Mapping) node;
+
+        assertEquals(
+                "- value: \"A\"\n- value: \"B\"",
+                Util.toYamlString(map.get("items")).trim());
+    }
+
+    @SuppressWarnings("ClassCanBeRecord")
+    public static class CustomItemSetHolder {
+        private final Set<CustomItem> items;
+
+        @DataBoundConstructor
+        public CustomItemSetHolder(Set<CustomItem> items) {
+            this.items = items;
+        }
+
+        @SuppressWarnings("unused")
+        public Set<CustomItem> getItems() {
+            return items;
+        }
+    }
+
+    @Test
+    void exportWithCustomConverterIteratesOverSet() throws Exception {
+        Set<CustomItem> set = new HashSet<>();
+        set.add(new CustomItem("A"));
+        set.add(new CustomItem("B"));
+
+        CustomItemSetHolder holder = new CustomItemSetHolder(set);
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        Configurator<Object> c = registry.lookupOrFail(CustomItemSetHolder.class);
+
+        CNode node = c.describe(holder, new ConfigurationContext(registry));
+        Mapping map = (Mapping) node;
+
+        String yaml =
+                Util.toYamlString(Objects.requireNonNull(map).get("items")).trim();
+
+        assertTrue(yaml.contains("value: \"A\""));
+        assertTrue(yaml.contains("value: \"B\""));
+    }
+
+    @SuppressWarnings("ClassCanBeRecord")
+    public static class CustomItemArrayHolder {
+        private final CustomItem[] items;
+
+        @DataBoundConstructor
+        public CustomItemArrayHolder(CustomItem[] items) {
+            this.items = items;
+        }
+
+        @SuppressWarnings("unused")
+        public CustomItem[] getItems() {
+            return items;
+        }
+    }
+
+    @Test
+    void exportWithCustomConverterIteratesOverArray() throws Exception {
+        CustomItem[] array = {new CustomItem("A"), new CustomItem("B")};
+
+        CustomItemArrayHolder holder = new CustomItemArrayHolder(array);
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        Configurator<Object> c = registry.lookupOrFail(CustomItemArrayHolder.class);
+
+        CNode node = c.describe(holder, new ConfigurationContext(registry));
+        Mapping map = (Mapping) node;
+
+        assertEquals(
+                "- value: \"A\"\n- value: \"B\"",
+                Util.toYamlString(Objects.requireNonNull(map).get("items")).trim());
+    }
+
+    @Test
+    void exportWithSingleElementList() throws Exception {
+        CustomItemListHolder holder = new CustomItemListHolder(List.of(new CustomItem("A")));
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        Configurator<Object> c = registry.lookupOrFail(CustomItemListHolder.class);
+
+        CNode node = c.describe(holder, new ConfigurationContext(registry));
+        Mapping map = (Mapping) node;
+
+        assertEquals(
+                "- value: \"A\"",
+                Util.toYamlString(Objects.requireNonNull(map).get("items")).trim());
+    }
+
+    @Test
+    void exportWithCustomConverterIteratesOverListWithNull() throws Exception {
+        List<CustomItem> list = Arrays.asList(new CustomItem("A"), null);
+        CustomItemListHolder holder = new CustomItemListHolder(list);
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        Configurator<Object> c = registry.lookupOrFail(CustomItemListHolder.class);
+
+        CNode node = c.describe(holder, new ConfigurationContext(registry));
+
+        assertNotNull(node, "Node should not be null");
+        assertInstanceOf(Mapping.class, node, "Node should be exported as a Mapping");
+
+        String yaml = Util.toYamlString(node);
+        assertTrue(yaml.contains("A"), "The valid item 'A' should be present in the exported YAML");
+    }
+
+    @Test
+    void configureIteratesOverList() {
+        Mapping config = new Mapping();
+        Sequence items = new Sequence();
+
+        Mapping itemA = new Mapping();
+        itemA.put("value", "A");
+        items.add(itemA);
+
+        Mapping itemB = new Mapping();
+        itemB.put("value", "B");
+        items.add(itemB);
+
+        config.put("items", items);
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        CustomItemListHolder configured = (CustomItemListHolder)
+                registry.lookupOrFail(CustomItemListHolder.class).configure(config, new ConfigurationContext(registry));
+
+        assertNotNull(configured);
+        assertEquals(2, configured.getItems().size());
+        assertEquals("A", configured.getItems().get(0).getValue());
+        assertEquals("B", configured.getItems().get(1).getValue());
+    }
+
+    @Test
+    void configureConvertsListToSet() {
+        Mapping config = new Mapping();
+        Sequence items = new Sequence();
+
+        Mapping itemA = new Mapping();
+        itemA.put("value", "A");
+        items.add(itemA);
+
+        Mapping itemB = new Mapping();
+        itemB.put("value", "B");
+        items.add(itemB);
+
+        config.put("items", items);
+
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        CustomItemSetHolder configured = (CustomItemSetHolder)
+                registry.lookupOrFail(CustomItemSetHolder.class).configure(config, new ConfigurationContext(registry));
+
+        assertNotNull(configured);
+        assertEquals(2, configured.getItems().size());
+        assertNotNull(configured.getItems());
+
+        assertTrue(configured.getItems().stream().anyMatch(i -> "A".equals(i.getValue())));
+        assertTrue(configured.getItems().stream().anyMatch(i -> "B".equals(i.getValue())));
+    }
 }