diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfigurator.java index dae7fa9d72..ead17eaff8 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfigurator.java +++ b/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,23 @@ 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()) { + Class targetType = a.getType(); + Object 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 + || (converted != null && converted.getClass().isArray()) + || !a.isMultiple()) { args[i] = converted; } else if (Set.class.isAssignableFrom(p.getType())) { args[i] = Collections.singleton(converted); diff --git a/test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfiguratorTest.java b/test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfiguratorTest.java index 06fdedeba2..fe83a3ab61 100644 --- a/test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfiguratorTest.java +++ b/test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfiguratorTest.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.kohsuke.stapler.Stapler.CONVERT_UTILS; import hudson.util.Secret; import io.jenkins.plugins.casc.ConfigurationAsCode; @@ -30,12 +31,18 @@ import io.jenkins.plugins.casc.model.Sequence; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; +import java.util.List; +import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.ParametersAreNonnullByDefault; import javax.annotation.PostConstruct; +import org.apache.commons.beanutils.Converter; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -45,6 +52,7 @@ import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.Stapler; /** * @author Nicolas De Loof @@ -426,4 +434,484 @@ 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("ClassCanBeRecord") + public static class CustomItemListHolder { + private final List items; + + @DataBoundConstructor + public CustomItemListHolder(List items) { + this.items = items; + } + + public List getItems() { + return items; + } + } + + @Test + @Issue("https://github.com/jenkinsci/configuration-as-code-plugin/issues/2346") + void exportWithCustomConverterIteratesOverList() throws Exception { + List list = Arrays.asList(new CustomItem("A"), new CustomItem("B")); + CustomItemListHolder holder = new CustomItemListHolder(list); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + final Configurator 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 items; + + @DataBoundConstructor + public CustomItemSetHolder(Set items) { + this.items = items; + } + + public Set getItems() { + return items; + } + } + + @Test + void exportWithCustomConverterIteratesOverSet() throws Exception { + Set set = new HashSet<>(); + set.add(new CustomItem("A")); + set.add(new CustomItem("B")); + + CustomItemSetHolder holder = new CustomItemSetHolder(set); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + Configurator 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\"")); + } + + @Test + void exportWithSingleElementList() throws Exception { + CustomItemListHolder holder = new CustomItemListHolder(List.of(new CustomItem("A"))); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + Configurator 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 list = Arrays.asList(new CustomItem("A"), null); + CustomItemListHolder holder = new CustomItemListHolder(list); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + Configurator 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()))); + } + + @SuppressWarnings("ClassCanBeRecord") + public static class StaplerOnlyItem { + private final String value; + + public StaplerOnlyItem(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + @SuppressWarnings("ClassCanBeRecord") + public static class StaplerOnlyItemListHolder { + private final List items; + + @DataBoundConstructor + public StaplerOnlyItemListHolder(List items) { + this.items = items; + } + + public List getItems() { + return items; + } + } + + public static class StaplerOnlyItemConverter implements Converter { + @Override + public T convert(Class type, Object value) { + assertFalse(value instanceof List, "Converter must be called per item, not per list"); + if (value == null) { + return null; + } + + return type.cast(new StaplerOnlyItem("converted-by-stapler-" + value)); + } + } + + @Test + void shouldConvertListItemsUsingStaplerConverter() { + CONVERT_UTILS.register(new StaplerOnlyItemConverter(), StaplerOnlyItem.class); + + try { + Mapping config = new Mapping(); + Sequence items = new Sequence(); + + items.add(new Scalar("ItemA")); + items.add(new Scalar("ItemB")); + config.put("items", items); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + StaplerOnlyItemListHolder configured = + (StaplerOnlyItemListHolder) registry.lookupOrFail(StaplerOnlyItemListHolder.class) + .configure(config, new ConfigurationContext(registry)); + + assertNotNull(configured); + assertEquals(2, configured.getItems().size()); + assertEquals( + "converted-by-stapler-ItemA", configured.getItems().get(0).getValue()); + assertEquals( + "converted-by-stapler-ItemB", configured.getItems().get(1).getValue()); + + } finally { + CONVERT_UTILS.deregister(StaplerOnlyItem.class); + } + } + + public static class ListGetterSetCtor { + + @DataBoundConstructor + @SuppressWarnings("unused") + public ListGetterSetCtor(Set items) { + // intentionally unused: required to exercise Set to Collection conversion + } + + @SuppressWarnings("unused") + public List getItems() { + return List.of("A", "B"); + } + } + + @Test + void describe_convertsCollectionToSetForConstructor() throws Exception { + ListGetterSetCtor obj = new ListGetterSetCtor(Set.of("A", "B")); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + CNode node = registry.lookupOrFail(ListGetterSetCtor.class).describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node); + assertInstanceOf(Mapping.class, node); + + Mapping mapping = (Mapping) node; + assertEquals(2, mapping.get("items").asSequence().size()); + } + + @SuppressWarnings("ClassCanBeRecord") + public static class CollectionCtorArrayGetter { + private final List items; + + @DataBoundConstructor + public CollectionCtorArrayGetter(List items) { + this.items = items; + } + + @SuppressWarnings("unused") + public StaplerOnlyItem[] getItems() { + return items.toArray(new StaplerOnlyItem[0]); + } + } + + @Test + void describe_convertsArrayValueWhenCtorIsCollection() throws Exception { + Stapler.CONVERT_UTILS.register(new StaplerOnlyItemConverter(), StaplerOnlyItem.class); + try { + CollectionCtorArrayGetter obj = new CollectionCtorArrayGetter(List.of(new StaplerOnlyItem("A"))); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + CNode node = registry.lookupOrFail(CollectionCtorArrayGetter.class) + .describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node); + + Mapping mapping = (Mapping) node; + assertEquals(1, mapping.get("items").asSequence().size()); + } finally { + Stapler.CONVERT_UTILS.deregister(StaplerOnlyItem.class); + } + } + + @SuppressWarnings("ClassCanBeRecord") + public static class SetCtorListGetter { + @SuppressWarnings({"unused", "FieldCanBeLocal"}) + private final Set items; + + @DataBoundConstructor + public SetCtorListGetter(Set items) { + this.items = items; + } + + @SuppressWarnings("unused") + public Collection getItems() { + return Arrays.asList("A", "B"); + } + } + + @Test + void describe_convertsCollectionToHashSetWhenCtorIsSet() throws Exception { + SetCtorListGetter obj = new SetCtorListGetter(Set.of("ignored")); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + CNode node = registry.lookupOrFail(SetCtorListGetter.class).describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node); + assertInstanceOf(Mapping.class, node); + + Mapping mapping = (Mapping) node; + assertEquals(2, mapping.get("items").asSequence().size()); + } + + @SuppressWarnings({"ClassCanBeRecord", "unused", "FieldCanBeLocal"}) + public static class SetCtorListGetterMismatch { + private final Set items; + + @DataBoundConstructor + public SetCtorListGetterMismatch(Set items) { + this.items = items; + } + + @SuppressWarnings("unused") + public List getItems() { + return List.of("A", "B"); + } + } + + @Test + void describe_hits_set_rewrap_branch() throws Exception { + SetCtorListGetterMismatch obj = new SetCtorListGetterMismatch(Set.of("ignored")); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + CNode node = registry.lookupOrFail(SetCtorListGetterMismatch.class) + .describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node); + assertInstanceOf(Mapping.class, node); + + Mapping mapping = (Mapping) node; + assertEquals(2, mapping.get("items").asSequence().size()); + } + + @Test + void describe_iteratesArrayAndConvertsEachItem() throws Exception { + Stapler.CONVERT_UTILS.register(new StaplerOnlyItemConverter(), StaplerOnlyItem.class); + try { + CollectionCtorArrayGetter obj = + new CollectionCtorArrayGetter(List.of(new StaplerOnlyItem("A"), new StaplerOnlyItem("B"))); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + CNode node = registry.lookupOrFail(CollectionCtorArrayGetter.class) + .describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node); + Mapping mapping = (Mapping) node; + assertEquals(2, mapping.get("items").asSequence().size()); + } finally { + Stapler.CONVERT_UTILS.deregister(StaplerOnlyItem.class); + } + } + + @SuppressWarnings({"ClassCanBeRecord", "unused", "FieldCanBeLocal"}) + public static class SetCtorStaplerOnlyItemHolder { + private final Set items; + + @DataBoundConstructor + public SetCtorStaplerOnlyItemHolder(Set items) { + this.items = items; + } + + public Set getItems() { + return items; + } + } + + @Test + void describe_converts_collection_to_set_via_stapler_converter() throws Exception { + Stapler.CONVERT_UTILS.register(new StaplerOnlyItemConverter(), StaplerOnlyItem.class); + try { + SetCtorStaplerOnlyItemHolder obj = + new SetCtorStaplerOnlyItemHolder(Set.of(new StaplerOnlyItem("A"), new StaplerOnlyItem("B"))); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + + CNode node = registry.lookupOrFail(SetCtorStaplerOnlyItemHolder.class) + .describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node, "configured instance should not be null"); + assertInstanceOf(Mapping.class, node); + + Mapping mapping = (Mapping) node; + assertEquals(2, mapping.get("items").asSequence().size(), "items set should contain 2 elements"); + + } finally { + Stapler.CONVERT_UTILS.deregister(StaplerOnlyItem.class); + } + } + + @SuppressWarnings({"ClassCanBeRecord", "unused", "FieldCanBeLocal"}) + public static class SetCtorFromStrings { + private final Set items; + + @DataBoundConstructor + public SetCtorFromStrings(Set items) { + this.items = items; + } + + public Collection getItems() { + return List.of("A", "B"); + } + } + + @Test + void configure_converts_sequence_to_set_via_stapler_converter_for_constructor() throws Exception { + final AtomicInteger convertCount = new AtomicInteger(0); + + Converter converter = new Converter() { + @Override + public T convert(Class type, Object value) { + convertCount.incrementAndGet(); + if (value == null) { + return null; + } + return type.cast(new StaplerOnlyItem("converted-by-stapler-" + value)); + } + }; + + Stapler.CONVERT_UTILS.register(converter, StaplerOnlyItem.class); + try { + Mapping config = new Mapping(); + Sequence seq = new Sequence(); + seq.add(new Scalar("A")); + seq.add(new Scalar("B")); + config.put("items", seq); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + + SetCtorFromStrings configured = (SetCtorFromStrings) registry.lookupOrFail(SetCtorFromStrings.class) + .configure(config, new ConfigurationContext(registry)); + + assertNotNull(configured); + assertNotNull(configured.getItems()); + assertEquals(2, configured.getItems().size()); + assertTrue(convertCount.get() >= 2, "Stapler converter should have been called once per sequence element"); + + } finally { + Stapler.CONVERT_UTILS.deregister(StaplerOnlyItem.class); + } + } + + @Test + void describe_hits_set_wrap_branch_when_stapler_returns_collection() throws Exception { + SetCtorStaplerOnlyItemHolder obj = + new SetCtorStaplerOnlyItemHolder(Set.of(new StaplerOnlyItem("A"), new StaplerOnlyItem("B"))); + + ConfiguratorRegistry registry = ConfiguratorRegistry.get(); + + CNode node = registry.lookupOrFail(SetCtorStaplerOnlyItemHolder.class) + .describe(obj, new ConfigurationContext(registry)); + + assertNotNull(node); + assertInstanceOf(Mapping.class, node); + } }