From 70c2d436bf619a4379c12ead0074a1a7a007160e Mon Sep 17 00:00:00 2001 From: somiljain2006 Date: Sat, 11 Apr 2026 22:55:15 +0530 Subject: [PATCH 1/5] Improve setter resolution logic in BaseConfigurator --- .../plugins/casc/BaseConfigurator.java | 193 +++++++++++++++--- .../plugins/casc/BaseConfiguratorTest.java | 98 +++++++++ 2 files changed, 265 insertions(+), 26 deletions(-) create mode 100644 plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java index 9e46185a84..43e85038fe 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java @@ -22,7 +22,6 @@ import java.lang.reflect.TypeVariable; import java.lang.reflect.WildcardType; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -76,57 +75,119 @@ public abstract class BaseConfigurator implements Configurator { } final Class target = getTarget(); + Map getterCache = new HashMap<>(); + Map> methodsByProperty = new HashMap<>(); // Resolve the methods and merging overrides to more concretized signatures // because the methods can to have been overridden with concretized type - // TODO: Overloaded setters with different types can corrupt this logic for (Method method : target.getMethods()) { final String methodName = method.getName(); - TypePair type; if (method.getParameterCount() == 0 && methodName.startsWith("get") && PersistedList.class.isAssignableFrom(method.getReturnType())) { - type = TypePair.ofReturnType(method); - } else if (method.getParameterCount() != 1 || !methodName.startsWith("set")) { - // Not an accessor, ignore + + String propertySuffix = methodName.substring(3); + final String name = StringUtils.uncapitalize(propertySuffix); + + if (exclusions.contains(name)) { + continue; + } + + TypePair type = TypePair.ofReturnType(method); + @SuppressWarnings("unchecked") + Attribute attribute = (Attribute) createAttribute(name, type); + + if (attribute != null) { + attribute.deprecated(method.getAnnotation(Deprecated.class) != null); + final Restricted r = method.getAnnotation(Restricted.class); + if (r != null) { + attribute.restrictions(r.value()); + } + attributes.putIfAbsent(name, attribute); + } continue; - } else { - type = TypePair.ofParameter(method, 0); } - final String s = methodName.substring(3); - final String name = StringUtils.uncapitalize(s); - if (exclusions.contains(name)) { + if (method.getParameterCount() == 1 && methodName.startsWith("set")) { + String propertySuffix = methodName.substring(3); + final String name = StringUtils.uncapitalize(propertySuffix); + + if (exclusions.contains(name)) { + continue; + } + methodsByProperty + .computeIfAbsent(propertySuffix, k -> new ArrayList<>()) + .add(method); + } + } + + for (Map.Entry> entry : methodsByProperty.entrySet()) { + final String propertySuffix = entry.getKey(); + final String name = StringUtils.uncapitalize(propertySuffix); + final List methods = entry.getValue(); + + TypePair getterType = getterCache.computeIfAbsent(propertySuffix, prop -> { + Method g = findGetter(target, prop); + return g != null ? TypePair.ofReturnType(g) : null; + }); + + if (getterType == null) { + LOGGER.log(Level.FINER, "Ignoring property {0}: no compatible getter found", name); continue; } - if (!hasGetter(target, s)) { - // Looks like a property but no actual getter method we can use to read value + final Class rawType = getterType.rawType; + + List validSetters = methods.stream() + .filter(m -> { + Class paramType = m.getParameterCount() == 0 ? m.getReturnType() : m.getParameterTypes()[0]; + return paramType.equals(rawType) + || rawType.isAssignableFrom(paramType) + || paramType.isAssignableFrom(rawType); + }) + .toList(); + + if (validSetters.isEmpty()) { + LOGGER.log( + Level.FINER, + "Ignoring property {0}: Setters exist, but none are compatible with getter type {1}", + new Object[] {name, getterType.rawType.getName()}); continue; } LOGGER.log(Level.FINER, "Processing {0} property", name); + Method bestMethod = resolveBestSetter(validSetters, getterType.rawType); + + TypePair type = TypePair.ofParameter(bestMethod, 0); + if (Map.class.isAssignableFrom(type.rawType)) { // yaml has support for Maps, but as nobody seem to like them we agreed not to support them LOGGER.log(Level.FINER, "{0} is a Map. We decided not to support Maps.", name); continue; } - Attribute attribute = createAttribute(name, type); + @SuppressWarnings("unchecked") + Attribute attribute = (Attribute) createAttribute(name, type); if (attribute == null) { continue; } - attribute.deprecated(method.getAnnotation(Deprecated.class) != null); - final Restricted r = method.getAnnotation(Restricted.class); + attribute.deprecated(bestMethod.getAnnotation(Deprecated.class) != null); + final Restricted r = bestMethod.getAnnotation(Restricted.class); if (r != null) { attribute.restrictions(r.value()); } - Attribute prevAttribute = attributes.get(name); - // Replace the method if it have more concretized type - if (prevAttribute == null || prevAttribute.type.isAssignableFrom(attribute.type)) { + Attribute prevAttribute = attributes.get(name); + if (prevAttribute == null) { attributes.put(name, attribute); + } else { + Class prevType = prevAttribute.type; + Class currentType = attribute.type; + + if (prevType.isAssignableFrom(currentType)) { + attributes.put(name, attribute); + } } } @@ -136,14 +197,25 @@ public abstract class BaseConfigurator implements Configurator { /** * Check if target class has a Getter method for property s */ - private boolean hasGetter(Class c, String s) { - List candidates = Arrays.asList("get" + s, "is" + s); + private Method findGetter(Class c, String s) { + String getMethod = "get" + s; + String isMethod = "is" + s; + for (Method m : c.getMethods()) { - if (m.getParameterCount() == 0 && candidates.contains(m.getName())) { - return true; + if (m.getParameterCount() == 0) { + if (m.getName().equals(getMethod)) { + return m; + } + + if (m.getName().equals(isMethod)) { + Class returnType = m.getReturnType(); + if (returnType == boolean.class || returnType == Boolean.class) { + return m; + } + } } } - return false; + return null; } /** @@ -439,9 +511,9 @@ public static final class TypePair { /** * Erasure of {@link #type} */ - final Class rawType; + final Class rawType; - public TypePair(Type type, Class rawType) { + public TypePair(Type type, Class rawType) { this.rawType = rawType; this.type = type; } @@ -476,4 +548,73 @@ public boolean equals(Object obj) { public int hashCode() { return getTarget().hashCode(); } + + private Method resolveBestSetter(List methods, Class getterRawType) { + if (methods.size() == 1) { + return methods.get(0); + } + + if (LOGGER.isLoggable(Level.FINE)) { + String methodNames = methods.stream() + .map(m -> m.getParameterCount() > 0 ? m.getParameterTypes()[0].getSimpleName() : "No-Args") + .collect(Collectors.joining(", ")); + LOGGER.log(Level.FINE, "Multiple setters found for getter type {0}. Candidates: [{1}]", new Object[] { + getterRawType.getSimpleName(), methodNames + }); + } + + for (Method m : methods) { + Class currentType = m.getParameterTypes()[0]; + if (currentType.equals(getterRawType)) { + return m; + } + } + + Method best = null; + Class bestType = null; + + for (Method m : methods) { + Class currentType = m.getParameterTypes()[0]; + + if (best == null) { + best = m; + bestType = currentType; + continue; + } + + if (bestType.isAssignableFrom(currentType)) { + best = m; + bestType = currentType; + continue; + } else if (currentType.isAssignableFrom(bestType)) { + continue; + } + + boolean currentCompatible = getterRawType.isAssignableFrom(currentType); + boolean bestCompatible = getterRawType.isAssignableFrom(bestType); + + if (currentCompatible && !bestCompatible) { + best = m; + bestType = currentType; + } else if (currentCompatible == bestCompatible) { + if (!currentType.isInterface() && bestType.isInterface()) { + best = m; + bestType = currentType; + } else if (!currentType.isInterface() || bestType.isInterface()) { + String previousBestName = bestType.getName(); + if (currentType.getName().compareTo(bestType.getName()) < 0) { + best = m; + bestType = currentType; + } + LOGGER.log( + Level.FINER, + "Ambiguous setters for property type {0}: {1} vs {2}. Deterministically chose {3}", + new Object[] { + getterRawType.getName(), previousBestName, currentType.getName(), bestType.getName() + }); + } + } + } + return best; + } } diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java new file mode 100644 index 0000000000..d173aac4f1 --- /dev/null +++ b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java @@ -0,0 +1,98 @@ +package io.jenkins.plugins.casc; + +import static org.junit.Assert.assertEquals; + +import io.jenkins.plugins.casc.model.Mapping; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.Test; + +public class BaseConfiguratorTest { + + public static class Animal {} + + public static class Dog extends Animal {} + + public static class Cat extends Animal {} + + public static class Vehicle {} + + public static class Car extends Vehicle {} + + @SuppressWarnings("unused") + public static class DummyTarget { + + public String getStandard() { + return null; + } + + public void setStandard(String val) {} + + public Car getRide() { + return null; + } + + public void setRide(Vehicle val) {} + + public void setRide(Car val) {} + + public void setRide(Object val) {} + + public void setRide() {} + + public Animal getPet() { + return null; + } + + public void setPet(Object pet) {} + + public void setPet(Animal pet) {} + + public Animal getAmbiguous() { + return null; + } + + public void setAmbiguous(Dog dog) {} + + public void setAmbiguous(Cat cat) {} + } + + public static class DummyConfigurator extends BaseConfigurator { + @Override + public Class getTarget() { + return DummyTarget.class; + } + + @Override + protected DummyTarget instance(Mapping mapping, ConfigurationContext context) { + return new DummyTarget(); + } + } + + @Test + public void testDescribeResolvesBestSetters() { + DummyConfigurator configurator = new DummyConfigurator(); + Set> attributes = configurator.describe(); + + Map> resolvedAttributes = + attributes.stream().collect(Collectors.toMap(Attribute::getName, attr -> (Class) attr.getType())); + + assertEquals("Should discover exactly 4 configurable properties", 4, resolvedAttributes.size()); + + assertEquals("Standard setter should resolve to String", String.class, resolvedAttributes.get("standard")); + + assertEquals( + "Exact match should win over subclasses and superclasses", Car.class, resolvedAttributes.get("ride")); + + assertEquals( + "Most specific compatible type should win over Object", Animal.class, resolvedAttributes.get("pet")); + + assertEquals( + "Alphabetical fallback should deterministically choose Cat over Dog", + Cat.class, + resolvedAttributes.get("ambiguous")); + + assertEquals(Set.of("standard", "ride", "pet", "ambiguous"), resolvedAttributes.keySet()); + } +} From c101b42a533d9a62cdf44d564843cb304f277eec Mon Sep 17 00:00:00 2001 From: somiljain2006 Date: Sun, 12 Apr 2026 11:36:48 +0530 Subject: [PATCH 2/5] Fixes multiple test failures --- .../plugins/casc/BaseConfigurator.java | 188 ++++++++++-------- 1 file changed, 108 insertions(+), 80 deletions(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java index 43e85038fe..1b69c8da80 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java @@ -1,5 +1,8 @@ package io.jenkins.plugins.casc; +import static java.lang.reflect.Array.newInstance; +import static java.lang.reflect.Array.set; + import edu.umd.cs.findbugs.annotations.NonNull; import hudson.BulkChange; import hudson.model.Describable; @@ -75,7 +78,6 @@ public abstract class BaseConfigurator implements Configurator { } final Class target = getTarget(); - Map getterCache = new HashMap<>(); Map> methodsByProperty = new HashMap<>(); // Resolve the methods and merging overrides to more concretized signatures // because the methods can to have been overridden with concretized type @@ -84,10 +86,7 @@ public abstract class BaseConfigurator implements Configurator { if (method.getParameterCount() == 0 && methodName.startsWith("get") && PersistedList.class.isAssignableFrom(method.getReturnType())) { - - String propertySuffix = methodName.substring(3); - final String name = StringUtils.uncapitalize(propertySuffix); - + String name = StringUtils.uncapitalize(methodName.substring(3)); if (exclusions.contains(name)) { continue; } @@ -98,10 +97,6 @@ public abstract class BaseConfigurator implements Configurator { if (attribute != null) { attribute.deprecated(method.getAnnotation(Deprecated.class) != null); - final Restricted r = method.getAnnotation(Restricted.class); - if (r != null) { - attribute.restrictions(r.value()); - } attributes.putIfAbsent(name, attribute); } continue; @@ -123,71 +118,72 @@ public abstract class BaseConfigurator implements Configurator { for (Map.Entry> entry : methodsByProperty.entrySet()) { final String propertySuffix = entry.getKey(); final String name = StringUtils.uncapitalize(propertySuffix); - final List methods = entry.getValue(); - TypePair getterType = getterCache.computeIfAbsent(propertySuffix, prop -> { - Method g = findGetter(target, prop); - return g != null ? TypePair.ofReturnType(g) : null; - }); + Method g = findGetter(target, propertySuffix); - if (getterType == null) { - LOGGER.log(Level.FINER, "Ignoring property {0}: no compatible getter found", name); + if (g == null) { continue; } - final Class rawType = getterType.rawType; + Class getterRawType = g.getReturnType(); - List validSetters = methods.stream() + List candidateSetters = entry.getValue().stream() .filter(m -> { - Class paramType = m.getParameterCount() == 0 ? m.getReturnType() : m.getParameterTypes()[0]; - return paramType.equals(rawType) - || rawType.isAssignableFrom(paramType) - || paramType.isAssignableFrom(rawType); + Class paramType = m.getParameterTypes()[0]; + return isSameType(paramType, getterRawType) + || getterRawType.isAssignableFrom(paramType) + || paramType.isAssignableFrom(getterRawType); }) - .toList(); + .collect(Collectors.toList()); - if (validSetters.isEmpty()) { - LOGGER.log( - Level.FINER, - "Ignoring property {0}: Setters exist, but none are compatible with getter type {1}", - new Object[] {name, getterType.rawType.getName()}); - continue; + if (candidateSetters.isEmpty()) { + candidateSetters = entry.getValue(); } - LOGGER.log(Level.FINER, "Processing {0} property", name); - - Method bestMethod = resolveBestSetter(validSetters, getterType.rawType); - + Method bestMethod = resolveBestSetter(candidateSetters, getterRawType); TypePair type = TypePair.ofParameter(bestMethod, 0); + TypePair getterType = TypePair.ofReturnType(g); + if (type.rawType.isAssignableFrom(getterType.rawType)) { + type = getterType; + } + if (Map.class.isAssignableFrom(type.rawType)) { // yaml has support for Maps, but as nobody seem to like them we agreed not to support them LOGGER.log(Level.FINER, "{0} is a Map. We decided not to support Maps.", name); continue; } + final TypePair finalType = type; + @SuppressWarnings("unchecked") - Attribute attribute = (Attribute) createAttribute(name, type); - if (attribute == null) { + Attribute rawAttribute = (Attribute) createAttribute(name, type); + if (rawAttribute == null) { continue; } + rawAttribute.setter((targetInstance, value) -> { + Object finalValue = value; + + if (value instanceof Collection collection && finalType.rawType.isArray()) { + Object array = newInstance(rawAttribute.getType(), collection.size()); + int i = 0; + for (Object item : collection) { + set(array, i++, item); + } + finalValue = array; + } + bestMethod.invoke(targetInstance, finalValue); + }); - attribute.deprecated(bestMethod.getAnnotation(Deprecated.class) != null); + rawAttribute.deprecated(bestMethod.getAnnotation(Deprecated.class) != null); final Restricted r = bestMethod.getAnnotation(Restricted.class); if (r != null) { - attribute.restrictions(r.value()); + rawAttribute.restrictions(r.value()); } Attribute prevAttribute = attributes.get(name); - if (prevAttribute == null) { - attributes.put(name, attribute); - } else { - Class prevType = prevAttribute.type; - Class currentType = attribute.type; - - if (prevType.isAssignableFrom(currentType)) { - attributes.put(name, attribute); - } + if (prevAttribute == null || ((Class) prevAttribute.type).isAssignableFrom(rawAttribute.type)) { + attributes.put(name, rawAttribute); } } @@ -550,25 +546,27 @@ public int hashCode() { } private Method resolveBestSetter(List methods, Class getterRawType) { + List realMethods = + methods.stream().filter(m -> !m.isBridge() && !m.isSynthetic()).collect(Collectors.toList()); + if (!realMethods.isEmpty()) { + methods = realMethods; + } if (methods.size() == 1) { return methods.get(0); } - if (LOGGER.isLoggable(Level.FINE)) { - String methodNames = methods.stream() - .map(m -> m.getParameterCount() > 0 ? m.getParameterTypes()[0].getSimpleName() : "No-Args") - .collect(Collectors.joining(", ")); - LOGGER.log(Level.FINE, "Multiple setters found for getter type {0}. Candidates: [{1}]", new Object[] { - getterRawType.getSimpleName(), methodNames - }); - } - - for (Method m : methods) { - Class currentType = m.getParameterTypes()[0]; - if (currentType.equals(getterRawType)) { - return m; + if (getterRawType != null) { + for (Method m : methods) { + if (isSameType(m.getParameterTypes()[0], getterRawType)) { + return m; + } } } + List arrayMethods = + methods.stream().filter(m -> m.getParameterTypes()[0].isArray()).collect(Collectors.toList()); + if (!arrayMethods.isEmpty()) { + methods = arrayMethods; + } Method best = null; Class bestType = null; @@ -585,36 +583,66 @@ private Method resolveBestSetter(List methods, Class getterRawType) { if (bestType.isAssignableFrom(currentType)) { best = m; bestType = currentType; - continue; - } else if (currentType.isAssignableFrom(bestType)) { - continue; - } - - boolean currentCompatible = getterRawType.isAssignableFrom(currentType); - boolean bestCompatible = getterRawType.isAssignableFrom(bestType); + } else if (!currentType.isAssignableFrom(bestType)) { + boolean currentMatch = (getterRawType != null && getterRawType.isAssignableFrom(currentType)); + boolean bestMatch = (getterRawType != null && getterRawType.isAssignableFrom(bestType)); - if (currentCompatible && !bestCompatible) { - best = m; - bestType = currentType; - } else if (currentCompatible == bestCompatible) { - if (!currentType.isInterface() && bestType.isInterface()) { + if (currentMatch && !bestMatch) { best = m; bestType = currentType; - } else if (!currentType.isInterface() || bestType.isInterface()) { - String previousBestName = bestType.getName(); - if (currentType.getName().compareTo(bestType.getName()) < 0) { + } else if (currentMatch == bestMatch) { + if ((!currentType.isInterface() && bestType.isInterface()) + || (currentType.getName().compareTo(bestType.getName()) < 0)) { best = m; bestType = currentType; } - LOGGER.log( - Level.FINER, - "Ambiguous setters for property type {0}: {1} vs {2}. Deterministically chose {3}", - new Object[] { - getterRawType.getName(), previousBestName, currentType.getName(), bestType.getName() - }); } } } return best; } + + private boolean isSameType(Class a, Class b) { + if (a == b) { + return true; + } + if (a == null || b == null) { + return false; + } + if (a.isPrimitive() && !b.isPrimitive()) { + return isWrapper(b, a); + } + if (b.isPrimitive() && !a.isPrimitive()) { + return isWrapper(a, b); + } + return false; + } + + private boolean isWrapper(Class wrapper, Class primitive) { + if (primitive == int.class) { + return wrapper == Integer.class; + } + if (primitive == boolean.class) { + return wrapper == Boolean.class; + } + if (primitive == long.class) { + return wrapper == Long.class; + } + if (primitive == double.class) { + return wrapper == Double.class; + } + if (primitive == float.class) { + return wrapper == Float.class; + } + if (primitive == byte.class) { + return wrapper == Byte.class; + } + if (primitive == char.class) { + return wrapper == Character.class; + } + if (primitive == short.class) { + return wrapper == Short.class; + } + return false; + } } From d8440b78b157fed843fe2d7b92651c4426489576 Mon Sep 17 00:00:00 2001 From: somiljain2006 Date: Sun, 12 Apr 2026 13:30:14 +0530 Subject: [PATCH 3/5] Added more tests and remove dead code --- .../plugins/casc/BaseConfigurator.java | 14 +- .../plugins/casc/BaseConfiguratorTest.java | 285 +++++++++++++++++- 2 files changed, 287 insertions(+), 12 deletions(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java index 1b69c8da80..1720ce90a1 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java @@ -587,15 +587,12 @@ private Method resolveBestSetter(List methods, Class getterRawType) { boolean currentMatch = (getterRawType != null && getterRawType.isAssignableFrom(currentType)); boolean bestMatch = (getterRawType != null && getterRawType.isAssignableFrom(bestType)); - if (currentMatch && !bestMatch) { + if (currentMatch == bestMatch + && ((!currentType.isInterface() && bestType.isInterface()) + || (currentType.getName().compareTo(bestType.getName()) < 0))) { + best = m; bestType = currentType; - } else if (currentMatch == bestMatch) { - if ((!currentType.isInterface() && bestType.isInterface()) - || (currentType.getName().compareTo(bestType.getName()) < 0)) { - best = m; - bestType = currentType; - } } } } @@ -606,9 +603,6 @@ private boolean isSameType(Class a, Class b) { if (a == b) { return true; } - if (a == null || b == null) { - return false; - } if (a.isPrimitive() && !b.isPrimitive()) { return isWrapper(b, a); } diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java index d173aac4f1..7f02276508 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java @@ -1,8 +1,13 @@ package io.jenkins.plugins.casc; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import io.jenkins.plugins.casc.model.Mapping; +import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -20,9 +25,33 @@ public static class Vehicle {} public static class Car extends Vehicle {} + public static class FakeSecret {} + + public static class A_Shape {} + + public static class B_Polygon extends A_Shape {} + + public static class C_Square extends B_Polygon {} + + public interface Z_Interface {} + + public static class A_Concrete {} + + public static class V_Class {} + + public static class W_Class {} + + public static class X_Class {} + + public static class Y_Class {} + + public static class Z_Class {} + @SuppressWarnings("unused") public static class DummyTarget { + private String[] items; + public String getStandard() { return null; } @@ -56,6 +85,120 @@ public Animal getAmbiguous() { public void setAmbiguous(Dog dog) {} public void setAmbiguous(Cat cat) {} + + public void setWriteOnly(String val) {} + + public FakeSecret getMismatchedToken() { + return null; + } + + public void setMismatchedToken(String val) {} + + public String[] getItems() { + return items; + } + + public void setItems(String[] items) { + this.items = items; + } + + public List getArrayFallback() { + return null; + } + + public void setArrayFallback(String val) {} + + public void setArrayFallback(String[] val) {} + + public FakeSecret getShapeSubtype() { + return null; + } + + public void setShapeSubtype(A_Shape val) {} + + public void setShapeSubtype(B_Polygon val) {} + + public void setShapeSubtype(C_Square val) {} + + public Object getConcreteWins() { + return null; + } + + public void setConcreteWins(Z_Interface val) {} + + public void setConcreteWins(A_Concrete val) {} + + public Object getReverseAlphabetical() { + return null; + } + + public void setReverseAlphabetical(Z_Class val) {} + + public void setReverseAlphabetical(Y_Class val) {} + + public void setReverseAlphabetical(X_Class val) {} + + public void setReverseAlphabetical(W_Class val) {} + + public void setReverseAlphabetical(V_Class val) {} + + public Integer getPrimitiveSetter() { + return null; + } + + public void setPrimitiveSetter(int val) {} + + public int getWrapperSetter() { + return 0; + } + + public void setWrapperSetter(Integer val) {} + + public Boolean getPrimitiveBoolean() { + return null; + } + + public void setPrimitiveBoolean(boolean val) {} + + public Long getPrimitiveLong() { + return null; + } + + public void setPrimitiveLong(long val) {} + + public Double getPrimitiveDouble() { + return null; + } + + public void setPrimitiveDouble(double val) {} + + public Float getPrimitiveFloat() { + return null; + } + + public void setPrimitiveFloat(float val) {} + + public Byte getPrimitiveByte() { + return null; + } + + public void setPrimitiveByte(byte val) {} + + public Character getPrimitiveChar() { + return null; + } + + public void setPrimitiveChar(char val) {} + + public Short getPrimitiveShort() { + return null; + } + + public void setPrimitiveShort(short val) {} + + public void getVoidEdgeCase() {} + + public void setVoidEdgeCase(String val) {} } public static class DummyConfigurator extends BaseConfigurator { @@ -70,6 +213,35 @@ protected DummyTarget instance(Mapping mapping, ConfigurationContext context) { } } + @SuppressWarnings("unused") + public static class NoGetterTarget { + public void setCompletelyMissing(String val) {} + + public void setRequiresParam(String val) {} + + public String getRequiresParam(String param) { + return param; + } + + public void setFakeBoolean(String val) {} + + public String isFakeBoolean() { + return "I am a String, not a boolean"; + } + } + + public static class NoGetterConfigurator extends BaseConfigurator { + @Override + public Class getTarget() { + return NoGetterTarget.class; + } + + @Override + protected NoGetterTarget instance(Mapping mapping, ConfigurationContext context) { + return new NoGetterTarget(); + } + } + @Test public void testDescribeResolvesBestSetters() { DummyConfigurator configurator = new DummyConfigurator(); @@ -78,7 +250,7 @@ public void testDescribeResolvesBestSetters() { Map> resolvedAttributes = attributes.stream().collect(Collectors.toMap(Attribute::getName, attr -> (Class) attr.getType())); - assertEquals("Should discover exactly 4 configurable properties", 4, resolvedAttributes.size()); + assertEquals("Should discover exactly 20 configurable properties", 20, resolvedAttributes.size()); assertEquals("Standard setter should resolve to String", String.class, resolvedAttributes.get("standard")); @@ -93,6 +265,115 @@ public void testDescribeResolvesBestSetters() { Cat.class, resolvedAttributes.get("ambiguous")); - assertEquals(Set.of("standard", "ride", "pet", "ambiguous"), resolvedAttributes.keySet()); + assertEquals( + "Disjoint getter/setter types should fallback to the available setter parameter type", + String.class, + resolvedAttributes.get("mismatchedToken")); + + assertEquals( + "Array types should be resolved to their component type", + String.class, + resolvedAttributes.get("items")); + + assertEquals( + "When no exact match exists between getters and setters, array setters should be preferred", + String.class, + resolvedAttributes.get("arrayFallback")); + + assertEquals( + "When multiple setters exist in an inheritance hierarchy, the most specific subtype should win", + C_Square.class, + resolvedAttributes.get("shapeSubtype")); + + assertEquals( + "Concrete class should win over interface when both are candidates", + A_Concrete.class, + resolvedAttributes.get("concreteWins")); + + assertEquals( + "Alphabetical fallback should resolve to the alphabetically first class", + V_Class.class, + resolvedAttributes.get("reverseAlphabetical")); + + assertEquals( + "Should map primitive setter with wrapper getter", + int.class, + resolvedAttributes.get("primitiveSetter")); + + assertEquals( + "Should map wrapper setter with primitive getter", + Integer.class, + resolvedAttributes.get("wrapperSetter")); + + assertEquals(boolean.class, resolvedAttributes.get("primitiveBoolean")); + assertEquals(long.class, resolvedAttributes.get("primitiveLong")); + assertEquals(double.class, resolvedAttributes.get("primitiveDouble")); + assertEquals(float.class, resolvedAttributes.get("primitiveFloat")); + assertEquals(byte.class, resolvedAttributes.get("primitiveByte")); + assertEquals(char.class, resolvedAttributes.get("primitiveChar")); + assertEquals(short.class, resolvedAttributes.get("primitiveShort")); + assertEquals(String.class, resolvedAttributes.get("voidEdgeCase")); + + Attribute itemsAttr = attributes.stream() + .filter(a -> a.getName().equals("items")) + .findFirst() + .orElseThrow(() -> new AssertionError("items attribute not found")); + assertTrue("items attribute should be marked as multiple", itemsAttr.isMultiple()); + + assertEquals( + Set.of( + "standard", + "ride", + "pet", + "ambiguous", + "mismatchedToken", + "items", + "arrayFallback", + "shapeSubtype", + "concreteWins", + "reverseAlphabetical", + "primitiveSetter", + "wrapperSetter", + "primitiveBoolean", + "primitiveLong", + "primitiveDouble", + "primitiveFloat", + "primitiveByte", + "primitiveChar", + "primitiveShort", + "voidEdgeCase"), + resolvedAttributes.keySet()); + } + + @Test + @SuppressWarnings({"unchecked", "rawtypes"}) + public void testCollectionToArrayConversion() throws Exception { + DummyConfigurator configurator = new DummyConfigurator(); + Set> attributes = configurator.describe(); + + Attribute itemsAttr = attributes.stream() + .filter(a -> a.getName().equals("items")) + .findFirst() + .orElseThrow(() -> new AssertionError("items attribute not found")); + + DummyTarget target = new DummyTarget(); + + List inputCollection = Arrays.asList("foo", "bar", "baz"); + + ((Attribute) itemsAttr).setValue(target, inputCollection); + + String[] result = target.getItems(); + assertNotNull("Array should have been set", result); + assertEquals("Array should have the same size as the collection", 3, result.length); + assertArrayEquals(new String[] {"foo", "bar", "baz"}, result); + } + + @Test + public void testFindGetterReturnsNullForMissingOrInvalidGetters() { + NoGetterConfigurator configurator = new NoGetterConfigurator(); + + Set> attributes = configurator.describe(); + + assertTrue("Properties without valid getters should yield no attributes", attributes.isEmpty()); } } From af7c41c03047542026f499f0714237fc1775ff92 Mon Sep 17 00:00:00 2001 From: somiljain2006 Date: Sun, 12 Apr 2026 14:32:06 +0530 Subject: [PATCH 4/5] Forcing the superclass to subclass order in test --- .../plugins/casc/BaseConfiguratorTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java index 7f02276508..30ccfd2a2d 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.assertTrue; import io.jenkins.plugins.casc.model.Mapping; +import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -376,4 +377,21 @@ public void testFindGetterReturnsNullForMissingOrInvalidGetters() { assertTrue("Properties without valid getters should yield no attributes", attributes.isEmpty()); } + + @Test + public void testResolveBestSetterBranchCoverage() throws Exception { + DummyConfigurator configurator = new DummyConfigurator(); + + Method resolveMethod = BaseConfigurator.class.getDeclaredMethod("resolveBestSetter", List.class, Class.class); + resolveMethod.setAccessible(true); + + Method setObj = DummyTarget.class.getMethod("setPet", Object.class); + Method setAnimal = DummyTarget.class.getMethod("setPet", Animal.class); + + List orderedMethods = Arrays.asList(setObj, setAnimal); + + Method best = (Method) resolveMethod.invoke(configurator, orderedMethods, null); + + assertEquals("Should upgrade bestType and resolve to the more specific Animal setter", setAnimal, best); + } } From 52f928f916324e9c62ddcb4d2ae5df02f2b4ba08 Mon Sep 17 00:00:00 2001 From: somiljain2006 Date: Mon, 13 Apr 2026 03:05:35 +0530 Subject: [PATCH 5/5] Fix setter determinism, propagate @Restricted, and improve tests --- .../plugins/casc/BaseConfigurator.java | 11 ++++- .../plugins/casc/BaseConfiguratorTest.java | 47 ++++++++++++++++--- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java index 1720ce90a1..7bc7f30419 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/BaseConfigurator.java @@ -39,6 +39,7 @@ import org.kohsuke.accmod.AccessRestriction; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.Beta; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.accmod.restrictions.None; /** @@ -97,6 +98,10 @@ public abstract class BaseConfigurator implements Configurator { if (attribute != null) { attribute.deprecated(method.getAnnotation(Deprecated.class) != null); + final Restricted r = method.getAnnotation(Restricted.class); + if (r != null) { + attribute.restrictions(r.value()); + } attributes.putIfAbsent(name, attribute); } continue; @@ -545,7 +550,8 @@ public int hashCode() { return getTarget().hashCode(); } - private Method resolveBestSetter(List methods, Class getterRawType) { + @Restricted(NoExternalUse.class) + Method resolveBestSetter(List methods, Class getterRawType) { List realMethods = methods.stream().filter(m -> !m.isBridge() && !m.isSynthetic()).collect(Collectors.toList()); if (!realMethods.isEmpty()) { @@ -589,7 +595,8 @@ private Method resolveBestSetter(List methods, Class getterRawType) { if (currentMatch == bestMatch && ((!currentType.isInterface() && bestType.isInterface()) - || (currentType.getName().compareTo(bestType.getName()) < 0))) { + || (currentType.isInterface() == bestType.isInterface() + && currentType.getName().compareTo(bestType.getName()) < 0))) { best = m; bestType = currentType; diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java index 30ccfd2a2d..f80c34de81 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/casc/BaseConfiguratorTest.java @@ -1,10 +1,13 @@ package io.jenkins.plugins.casc; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsEmptyCollection.empty; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import hudson.util.PersistedList; import io.jenkins.plugins.casc.model.Mapping; import java.lang.reflect.Method; import java.util.Arrays; @@ -13,6 +16,9 @@ import java.util.Set; import java.util.stream.Collectors; import org.junit.Test; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.Beta; +import org.kohsuke.accmod.restrictions.NoExternalUse; public class BaseConfiguratorTest { @@ -200,6 +206,18 @@ public void setPrimitiveShort(short val) {} public void getVoidEdgeCase() {} public void setVoidEdgeCase(String val) {} + + @Restricted(NoExternalUse.class) + public PersistedList getRestrictedList() { + return null; + } + + public String getRestrictedSetter() { + return null; + } + + @Restricted(Beta.class) + public void setRestrictedSetter(String val) {} } public static class DummyConfigurator extends BaseConfigurator { @@ -251,7 +269,7 @@ public void testDescribeResolvesBestSetters() { Map> resolvedAttributes = attributes.stream().collect(Collectors.toMap(Attribute::getName, attr -> (Class) attr.getType())); - assertEquals("Should discover exactly 20 configurable properties", 20, resolvedAttributes.size()); + assertEquals("Should discover exactly 22 configurable properties", 22, resolvedAttributes.size()); assertEquals("Standard setter should resolve to String", String.class, resolvedAttributes.get("standard")); @@ -321,6 +339,22 @@ public void testDescribeResolvesBestSetters() { .orElseThrow(() -> new AssertionError("items attribute not found")); assertTrue("items attribute should be marked as multiple", itemsAttr.isMultiple()); + Attribute restrictedListAttr = attributes.stream() + .filter(a -> a.getName().equals("restrictedList")) + .findFirst() + .orElseThrow(() -> new AssertionError("restrictedList attribute not found")); + assertTrue( + "restrictedList attribute should contain NoExternalUse restriction", + Arrays.asList(restrictedListAttr.getRestrictions()).contains(NoExternalUse.class)); + + Attribute restrictedSetterAttr = attributes.stream() + .filter(a -> a.getName().equals("restrictedSetter")) + .findFirst() + .orElseThrow(() -> new AssertionError("restrictedSetter attribute not found")); + assertTrue( + "restrictedSetter attribute should contain Beta restriction", + Arrays.asList(restrictedSetterAttr.getRestrictions()).contains(Beta.class)); + assertEquals( Set.of( "standard", @@ -342,7 +376,9 @@ public void testDescribeResolvesBestSetters() { "primitiveByte", "primitiveChar", "primitiveShort", - "voidEdgeCase"), + "voidEdgeCase", + "restrictedList", + "restrictedSetter"), resolvedAttributes.keySet()); } @@ -375,22 +411,19 @@ public void testFindGetterReturnsNullForMissingOrInvalidGetters() { Set> attributes = configurator.describe(); - assertTrue("Properties without valid getters should yield no attributes", attributes.isEmpty()); + assertThat("Properties without valid getters should yield no attributes", attributes, empty()); } @Test public void testResolveBestSetterBranchCoverage() throws Exception { DummyConfigurator configurator = new DummyConfigurator(); - Method resolveMethod = BaseConfigurator.class.getDeclaredMethod("resolveBestSetter", List.class, Class.class); - resolveMethod.setAccessible(true); - Method setObj = DummyTarget.class.getMethod("setPet", Object.class); Method setAnimal = DummyTarget.class.getMethod("setPet", Animal.class); List orderedMethods = Arrays.asList(setObj, setAnimal); - Method best = (Method) resolveMethod.invoke(configurator, orderedMethods, null); + Method best = configurator.resolveBestSetter(orderedMethods, null); assertEquals("Should upgrade bestType and resolve to the more specific Animal setter", setAnimal, best); }