Skip to content

Commit 9049f43

Browse files
Fix collection-to-set conversion causing type mismatch in JCasC setters (#2829)
1 parent eaed8d4 commit 9049f43

2 files changed

Lines changed: 76 additions & 7 deletions

File tree

plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@
2929
import java.util.Collections;
3030
import java.util.HashMap;
3131
import java.util.HashSet;
32+
import java.util.LinkedHashSet;
3233
import java.util.List;
3334
import java.util.Map;
3435
import java.util.Set;
36+
import java.util.SortedSet;
37+
import java.util.TreeSet;
3538
import java.util.logging.Level;
3639
import java.util.logging.Logger;
3740
import java.util.stream.Collectors;
@@ -169,14 +172,23 @@ public abstract class BaseConfigurator<T> implements Configurator<T> {
169172
rawAttribute.setter((targetInstance, value) -> {
170173
Object finalValue = value;
171174

172-
if (value instanceof Collection<?> collection && finalType.rawType.isArray()) {
173-
Object array = newInstance(rawAttribute.getType(), collection.size());
174-
int i = 0;
175-
for (Object item : collection) {
176-
set(array, i++, item);
175+
if (value instanceof Collection<?> collection) {
176+
if (finalType.rawType.isArray()) {
177+
Object array = newInstance(rawAttribute.getType(), collection.size());
178+
int i = 0;
179+
for (Object item : collection) {
180+
set(array, i++, item);
181+
}
182+
finalValue = array;
183+
184+
} else if (SortedSet.class.isAssignableFrom(finalType.rawType)) {
185+
finalValue = new TreeSet<>(collection);
186+
187+
} else if (Set.class.isAssignableFrom(finalType.rawType)) {
188+
finalValue = new LinkedHashSet<>(collection);
177189
}
178-
finalValue = array;
179190
}
191+
180192
bestMethod.invoke(targetInstance, finalValue);
181193
});
182194

plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111
import io.jenkins.plugins.casc.model.Mapping;
1212
import java.lang.reflect.Method;
1313
import java.util.Arrays;
14+
import java.util.LinkedHashSet;
1415
import java.util.List;
1516
import java.util.Map;
1617
import java.util.Set;
18+
import java.util.SortedSet;
19+
import java.util.TreeSet;
1720
import java.util.stream.Collectors;
1821
import org.junit.Test;
1922
import org.kohsuke.accmod.Restricted;
@@ -58,6 +61,8 @@ public static class Z_Class {}
5861
public static class DummyTarget {
5962

6063
private String[] items;
64+
private Set<String> stringSet;
65+
private SortedSet<String> sortedStringSet;
6166

6267
public String getStandard() {
6368
return null;
@@ -109,6 +114,22 @@ public void setItems(String[] items) {
109114
this.items = items;
110115
}
111116

117+
public Set<String> getStringSet() {
118+
return stringSet;
119+
}
120+
121+
public void setStringSet(Set<String> stringSet) {
122+
this.stringSet = stringSet;
123+
}
124+
125+
public SortedSet<String> getSortedStringSet() {
126+
return sortedStringSet;
127+
}
128+
129+
public void setSortedStringSet(SortedSet<String> sortedStringSet) {
130+
this.sortedStringSet = sortedStringSet;
131+
}
132+
112133
public List<String> getArrayFallback() {
113134
return null;
114135
}
@@ -269,7 +290,7 @@ public void testDescribeResolvesBestSetters() {
269290
Map<String, Class<?>> resolvedAttributes =
270291
attributes.stream().collect(Collectors.toMap(Attribute::getName, attr -> (Class<?>) attr.getType()));
271292

272-
assertEquals("Should discover exactly 22 configurable properties", 22, resolvedAttributes.size());
293+
assertEquals("Should discover exactly 24 configurable properties", 24, resolvedAttributes.size());
273294

274295
assertEquals("Standard setter should resolve to String", String.class, resolvedAttributes.get("standard"));
275296

@@ -363,6 +384,8 @@ public void testDescribeResolvesBestSetters() {
363384
"ambiguous",
364385
"mismatchedToken",
365386
"items",
387+
"stringSet",
388+
"sortedStringSet",
366389
"arrayFallback",
367390
"shapeSubtype",
368391
"concreteWins",
@@ -405,6 +428,40 @@ public void testCollectionToArrayConversion() throws Exception {
405428
assertArrayEquals(new String[] {"foo", "bar", "baz"}, result);
406429
}
407430

431+
@Test
432+
@SuppressWarnings({"unchecked", "rawtypes"})
433+
public void testCollectionToSetConversions() throws Exception {
434+
DummyConfigurator configurator = new DummyConfigurator();
435+
Set<Attribute<DummyTarget, ?>> attributes = configurator.describe();
436+
437+
Attribute<DummyTarget, ?> setAttr = attributes.stream()
438+
.filter(a -> a.getName().equals("stringSet"))
439+
.findFirst()
440+
.orElseThrow(() -> new AssertionError("stringSet attribute not found"));
441+
442+
Attribute<DummyTarget, ?> sortedSetAttr = attributes.stream()
443+
.filter(a -> a.getName().equals("sortedStringSet"))
444+
.findFirst()
445+
.orElseThrow(() -> new AssertionError("sortedStringSet attribute not found"));
446+
447+
DummyTarget target = new DummyTarget();
448+
449+
List<String> inputCollection = Arrays.asList("foo", "bar", "baz");
450+
451+
((Attribute) setAttr).setValue(target, inputCollection);
452+
((Attribute) sortedSetAttr).setValue(target, inputCollection);
453+
454+
Set<String> setResult = target.getStringSet();
455+
assertNotNull("Set should have been populated", setResult);
456+
assertEquals("Set should be converted to LinkedHashSet", LinkedHashSet.class, setResult.getClass());
457+
assertEquals("Set should have 3 items", 3, setResult.size());
458+
459+
SortedSet<String> sortedSetResult = target.getSortedStringSet();
460+
assertNotNull("SortedSet should have been populated", sortedSetResult);
461+
assertEquals("SortedSet should be converted to TreeSet", TreeSet.class, sortedSetResult.getClass());
462+
assertEquals("SortedSet should have 3 items", 3, sortedSetResult.size());
463+
}
464+
408465
@Test
409466
public void testFindGetterReturnsNullForMissingOrInvalidGetters() {
410467
NoGetterConfigurator configurator = new NoGetterConfigurator();

0 commit comments

Comments
 (0)