From bd4dae1442798085e6033b157a6d096d86f7237d Mon Sep 17 00:00:00 2001 From: Tomas Jura Date: Thu, 4 Jul 2019 09:50:31 +0200 Subject: [PATCH] fix of issue #104: XAFactories which implement java.util.Map (MQXAConnectionFactory) cannot be initialized properly --- .../java/bitronix/tm/utils/PropertyUtils.java | 163 +++++++++++++----- pom.xml | 8 +- 2 files changed, 123 insertions(+), 48 deletions(-) diff --git a/btm/src/main/java/bitronix/tm/utils/PropertyUtils.java b/btm/src/main/java/bitronix/tm/utils/PropertyUtils.java index cfccf12e..211a6f22 100644 --- a/btm/src/main/java/bitronix/tm/utils/PropertyUtils.java +++ b/btm/src/main/java/bitronix/tm/utils/PropertyUtils.java @@ -22,6 +22,8 @@ import java.util.Iterator; import java.util.Map; import java.util.TreeMap; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Smart reflection helper. @@ -29,6 +31,7 @@ * @author Ludovic Orban */ public final class PropertyUtils { + static final Logger log = LoggerFactory.getLogger(PropertyUtils.class); private PropertyUtils() { } @@ -37,60 +40,132 @@ private PropertyUtils() { * Set a direct or indirect property (dotted property: prop1.prop2.prop3) on the target object. This method tries * to be smart in the way that intermediate properties currently set to null are set if it is possible to create * and set an object. Conversions from propertyValue to the proper destination type are performed when possible. + + * There are nasty marginal case (see get/set ClonedProps in PropertyUtilsTest.java ), where parent getter do not + * return an own object (but its clone) and a change on obtained the property object needs to be followed by call of + * parent setter. This case leads to unsolvable cases in code. + + * First, read only properties are perfectly legal construction, and setters for them must not be called (raises an + * exception) or not exists. ( Which is oposite scenario to ClonedProps ) + + * There is the wanted feature that missing classes along property path are subject of creation if they are null and + * set. Taking in account this feature and cases above we have a contradictory situation. Either we call setter to + * assure to handle clonedProps case, but break read only properties. Or do not call parent setter after get and break + * clonedProps. In other words we cannot safely say if the setter has to be called for clonedProps or not. + + * A configuation language ( OGNL, JavaScript, BeanShell ) is a solution of the problem. The frontier between + * programming and configuration can be very thin. + + * @author Tomas Jura * @param target the target object on which to set the property. * @param propertyName the name of the property to set. * @param propertyValue the value of the property to set. * @throws PropertyException if an error happened while trying to set the property. */ public static void setProperty(Object target, String propertyName, Object propertyValue) throws PropertyException { - String[] propertyNames = propertyName.split("\\."); - - StringBuilder visitedPropertyName = new StringBuilder(); - Object currentTarget = target; - Object parentTarget = target; - String parentName = null; - int i = 0; - while (i < propertyNames.length -1) { - String name = propertyNames[i]; - Object result = callGetter(currentTarget, name); - if (result == null) { - // try to instantiate the object & set it in place - Class propertyType = getPropertyType(target, name); - try { - result = propertyType.newInstance(); - } catch (InstantiationException ex) { - throw new PropertyException("cannot set property '" + propertyName + "' - '" + name + "' is null and cannot be auto-filled", ex); - } catch (IllegalAccessException ex) { - throw new PropertyException("cannot set property '" + propertyName + "' - '" + name + "' is null and cannot be auto-filled", ex); - } - callSetter(currentTarget, name, result); - } + if (target == null) { throw new PropertyException("target is null"); } + if (propertyName == null) { throw new PropertyException("propertyName is null"); } + int dot = propertyName.indexOf('.'); + String prop = (dot==-1)? propertyName : propertyName.substring(0, dot); + if (dot == -1) { // direct set property + Method setter = null; + String setterName = "set"+ prop.substring(0, 1).toUpperCase() + prop.substring(1); + for (Method m : target.getClass().getMethods()) { + if (m.getName().equals(setterName) && m.getReturnType().equals(void.class) && m.getParameterTypes().length == 1) { + setter=m; + break; + } + } + if (setter == null) { + if ( target instanceof Map ) { + log.debug("cannot find property '{}' on {} and using it as a map with key {}",prop,target.getClass().getCanonicalName(),propertyName); + @SuppressWarnings("unchecked") Map map=(Map) target; + map.put(prop,propertyValue); + return; + } + else throw new PropertyException("cannot set property '" + prop +"' for class " + target.getClass().getCanonicalName()); + } - parentTarget = currentTarget; - currentTarget = result; - visitedPropertyName.append(name); - visitedPropertyName.append('.'); - i++; - - // if it's a Map object -> the non-visited part of the key should be used - // as this Map's object key so stop iterating over the dotted properties. - if (currentTarget instanceof Map) { - parentName = name; - break; - } + Class parameterType = setter.getParameterTypes()[0]; + try { + if (propertyValue != null) { + Object transformedPropertyValue = transform(propertyValue, parameterType); + setter.invoke(target, transformedPropertyValue); + } else { + setter.invoke(target); + } + } catch (IllegalAccessException ex) { + throw new PropertyException("property '" + propertyName + "' is not accessible", ex); + } catch (InvocationTargetException ex) { + throw new PropertyException("property '" + propertyName + "' access threw an exception", ex); + } + } + else { // sub property + Method getter = null; + String getterName = "get"+ prop.substring(0, 1).toUpperCase() + prop.substring(1); + try { + getter = target.getClass().getMethod(getterName); + if ( getter.getReturnType().equals(void.class) ) { getter = null; } + } + catch (NoSuchMethodException ex) { } + catch (SecurityException ex) { log.warn( "getter for {} found, but is not accesible. Ignoring it.",prop); } + if (getter == null) { + if ( target instanceof Map ) { + log.warn("cannot find property '{}' on {} and using it as a map with key {}",prop,target.getClass().getCanonicalName(),propertyName); + @SuppressWarnings("unchecked") Map map=(Map)target; + map.put(propertyName,propertyValue); + return; + } + else throw new PropertyException("cannot get property '" + prop +"' for class " + target.getClass().getCanonicalName()); } - String lastPropertyName = propertyName.substring(visitedPropertyName.length(), propertyName.length()); - if (currentTarget instanceof Map) { - @SuppressWarnings("unchecked") - Map p = (Map) currentTarget; - p.put(lastPropertyName, propertyValue.toString()); - if (parentName != null) { - callSetter(parentTarget, parentName, p); - } - } else { - setDirectProperty(currentTarget, lastPropertyName, propertyValue); + Object newTarget; + Boolean setterNeeded = false; + try { + newTarget = getter.invoke(target); + } catch ( IllegalAccessException ex) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is not accesible", ex); + } catch ( InvocationTargetException ex) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "'", ex); + } + Class propertyType = getter.getReturnType(); + if (newTarget == null) { // then try to instantiate the object & set it in place + setterNeeded = true; + try { + newTarget = propertyType.newInstance(); + } catch (InstantiationException ex) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and cannot be auto-filled", ex); + } catch (IllegalAccessException ex) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and cannot be auto-filled", ex); + } + } + setProperty( newTarget, propertyName.substring(dot+1), propertyValue ); + try { + String setterName = "set"+ prop.substring(0, 1).toUpperCase() + prop.substring(1); + Method setter = target.getClass().getMethod(setterName,propertyType); + setter.invoke(target,newTarget); + } catch (NoSuchMethodException ex ) { + if (setterNeeded) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and setter call failed.", ex); + } + } catch (SecurityException ex ) { + if (setterNeeded) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and setter call failed.", ex); + } + } catch (IllegalAccessException ex ) { + if (setterNeeded) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and setter call failed.", ex); + } + } catch (IllegalArgumentException ex ) { + if (setterNeeded) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and setter call failed.", ex); + } + } catch (InvocationTargetException ex ) { + if (setterNeeded) { + throw new PropertyException("cannot set property '" + propertyName + "' - '" + prop + "' is null and setter call failed.", ex); + } } + } } /** diff --git a/pom.xml b/pom.xml index 56106589..ac1426ad 100644 --- a/pom.xml +++ b/pom.xml @@ -126,17 +126,17 @@ org.slf4j slf4j-api - 1.6.4 + 1.7.26 org.slf4j jcl-over-slf4j - 1.6.4 + 1.7.26 cglib cglib - 2.2.2 + 3.2.6 org.javassist @@ -243,7 +243,7 @@ org.apache.maven.plugins maven-surefire-plugin - 2.12.4 + 2.22.2 ${surefire.jvm.settings}