XmlConfiguration always checks for setter method (#6398)

* Fixed #6375 XmlConfiguration to always checks  for a matching setter, even if the property attribute is given but not set.
This commit is contained in:
Greg Wilkins 2021-06-15 13:57:40 +10:00 committed by GitHub
parent 09d6a2692d
commit a492473a73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 90 additions and 27 deletions

View File

@ -515,25 +515,11 @@ public class XmlConfiguration
*/ */
private void set(Object obj, XmlParser.Node node) throws Exception private void set(Object obj, XmlParser.Node node) throws Exception
{ {
String attr = node.getAttribute("name"); String name = node.getAttribute("name");
String setter = "set" + name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1);
String id = node.getAttribute("id"); String id = node.getAttribute("id");
String property = node.getAttribute("property"); String property = node.getAttribute("property");
String propertyValue = null; String propertyValue = null;
// Look for a property value
if (property != null)
{
Map<String, String> properties = _configuration.getProperties();
propertyValue = properties.get(property);
// If no property value, then do not set
if (propertyValue == null)
return;
}
String name = "set" + attr.substring(0, 1).toUpperCase(Locale.ENGLISH) + attr.substring(1);
Object value = value(obj, node);
if (value == null)
value = propertyValue;
Object[] arg = {value};
Class<?> oClass = nodeClass(node); Class<?> oClass = nodeClass(node);
if (oClass != null) if (oClass != null)
@ -541,12 +527,38 @@ public class XmlConfiguration
else else
oClass = obj.getClass(); oClass = obj.getClass();
// Look for a property value
if (property != null)
{
Map<String, String> properties = _configuration.getProperties();
propertyValue = properties.get(property);
// If no property value, then do not set
if (propertyValue == null)
{
// check that there is at least one setter or field that could have matched
if (Arrays.stream(oClass.getMethods()).noneMatch(m -> m.getName().equals(setter)) &&
Arrays.stream(oClass.getFields()).filter(f -> Modifier.isPublic(f.getModifiers())).noneMatch(f -> f.getName().equals(name)))
{
NoSuchMethodException e = new NoSuchMethodException(String.format("No method '%s' on %s", setter, oClass.getName()));
e.addSuppressed(new NoSuchFieldException(String.format("No field '%s' on %s", name, oClass.getName())));
throw e;
}
// otherwise it is a noop
return;
}
}
Object value = value(obj, node);
if (value == null)
value = propertyValue;
Object[] arg = {value};
Class<?>[] vClass = {Object.class}; Class<?>[] vClass = {Object.class};
if (value != null) if (value != null)
vClass[0] = value.getClass(); vClass[0] = value.getClass();
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug("XML {}.{} ({})", (obj != null ? obj.toString() : oClass.getName()), name, value); LOG.debug("XML {}.{} ({})", (obj != null ? obj.toString() : oClass.getName()), setter, value);
MultiException me = new MultiException(); MultiException me = new MultiException();
@ -557,7 +569,7 @@ public class XmlConfiguration
// Try for trivial match // Try for trivial match
try try
{ {
Method set = oClass.getMethod(name, vClass); Method set = oClass.getMethod(setter, vClass);
invokeMethod(set, obj, arg); invokeMethod(set, obj, arg);
return; return;
} }
@ -572,7 +584,7 @@ public class XmlConfiguration
{ {
Field type = vClass[0].getField("TYPE"); Field type = vClass[0].getField("TYPE");
vClass[0] = (Class<?>)type.get(null); vClass[0] = (Class<?>)type.get(null);
Method set = oClass.getMethod(name, vClass); Method set = oClass.getMethod(setter, vClass);
invokeMethod(set, obj, arg); invokeMethod(set, obj, arg);
return; return;
} }
@ -585,7 +597,7 @@ public class XmlConfiguration
// Try a field // Try a field
try try
{ {
Field field = oClass.getField(attr); Field field = oClass.getField(name);
if (Modifier.isPublic(field.getModifiers())) if (Modifier.isPublic(field.getModifiers()))
{ {
try try
@ -622,18 +634,18 @@ public class XmlConfiguration
// Search for a match by trying all the set methods // Search for a match by trying all the set methods
Method[] sets = oClass.getMethods(); Method[] sets = oClass.getMethods();
Method set = null; Method set = null;
for (Method setter : sets) for (Method s : sets)
{ {
if (setter.getParameterCount() != 1) if (s.getParameterCount() != 1)
continue; continue;
Class<?>[] paramTypes = setter.getParameterTypes(); Class<?>[] paramTypes = s.getParameterTypes();
if (name.equals(setter.getName())) if (setter.equals(s.getName()))
{ {
types = types == null ? paramTypes[0].getName() : (types + "," + paramTypes[0].getName()); types = types == null ? paramTypes[0].getName() : (types + "," + paramTypes[0].getName());
// lets try it // lets try it
try try
{ {
set = setter; set = s;
invokeMethod(set, obj, arg); invokeMethod(set, obj, arg);
return; return;
} }
@ -650,7 +662,7 @@ public class XmlConfiguration
if (paramTypes[0].isAssignableFrom(c)) if (paramTypes[0].isAssignableFrom(c))
{ {
setValue = convertArrayToCollection(value, c); setValue = convertArrayToCollection(value, c);
invokeMethod(setter, obj, setValue); invokeMethod(s, obj, setValue);
return; return;
} }
} }
@ -703,7 +715,7 @@ public class XmlConfiguration
} }
// No Joy // No Joy
String message = oClass + "." + name + "(" + vClass[0] + ")"; String message = oClass + "." + setter + "(" + vClass[0] + ")";
if (types != null) if (types != null)
message += ". Found setters for " + types; message += ". Found setters for " + types;
NoSuchMethodException failure = new NoSuchMethodException(message); NoSuchMethodException failure = new NoSuchMethodException(message);

View File

@ -327,6 +327,30 @@ public class XmlConfigurationTest
assertEquals(configuration.getIdMap().get("test"), "This is a property value"); assertEquals(configuration.getIdMap().get("test"), "This is a property value");
} }
@Test
public void testSetFieldWithProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"testField1\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().put("prop", "42");
TestConfiguration tc = new TestConfiguration();
tc.testField1 = -1;
configuration.configure(tc);
assertEquals(42, tc.testField1);
assertEquals(configuration.getIdMap().get("test"), "42");
}
@Test
public void testNotSetFieldWithNullProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"testField1\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().remove("prop");
TestConfiguration tc = new TestConfiguration();
tc.testField1 = -1;
configuration.configure(tc);
assertEquals(-1, tc.testField1);
assertEquals(configuration.getIdMap().get("test"), null);
}
@Test @Test
public void testSetWithNullProperty() throws Exception public void testSetWithNullProperty() throws Exception
{ {
@ -363,6 +387,33 @@ public class XmlConfigurationTest
assertNull(configuration.getIdMap().get("test")); assertNull(configuration.getIdMap().get("test"));
} }
@Test
public void testSetWithWrongNameAndProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"WrongName\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().put("prop", "This is a property value");
TestConfiguration tc = new TestConfiguration();
tc.setTestString("default");
NoSuchMethodException e = assertThrows(NoSuchMethodException.class, () -> configuration.configure(tc));
assertThat(e.getMessage(), containsString("setWrongName"));
assertEquals("default", tc.getTestString());
}
@Test
public void testSetWithWrongNameAndNullProperty() throws Exception
{
XmlConfiguration configuration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\"><Set name=\"WrongName\" property=\"prop\" id=\"test\"/></Configure>");
configuration.getProperties().remove("prop");
TestConfiguration tc = new TestConfiguration();
tc.setTestString("default");
NoSuchMethodException e = assertThrows(NoSuchMethodException.class, () -> configuration.configure(tc));
assertThat(e.getMessage(), containsString("setWrongName"));
assertThat(e.getSuppressed()[0], instanceOf(NoSuchFieldException.class));
assertEquals("default", tc.getTestString());
}
@Test @Test
public void testMeaningfullSetException() throws Exception public void testMeaningfullSetException() throws Exception
{ {