jgoodyear commented on code in PR #1784:
URL: https://github.com/apache/cxf/pull/1784#discussion_r1552527419
##########
rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/Beanspector.java:
##########
@@ -58,49 +58,59 @@ public Beanspector(T tobj) {
init();
}
- @SuppressWarnings("unchecked")
- private void init() {
- if (tclass == null) {
- tclass = (Class<T>)tobj.getClass();
- }
- for (Method m : tclass.getMethods()) {
- if (isGetter(m)) {
- String pname = getPropertyName(m);
- if (!getters.containsKey(pname)) {
- getters.put(getPropertyName(m), m);
- } else {
- // Prefer the getter that has the most specialized class
as a return type
- Method met = getters.get(pname);
- if
(met.getReturnType().isAssignableFrom(m.getReturnType())) {
- getters.put(pname, m);
- }
- }
- } else if (isSetter(m)) {
- String pname = getPropertyName(m);
- if (!setters.containsKey(pname)) {
- setters.put(getPropertyName(m), m);
- } else {
- // Prefer the setter that has the most specialized class
as a parameter
- Method met = setters.get(pname);
- if
(met.getParameterTypes()[0].isAssignableFrom(m.getParameterTypes()[0])) {
- setters.put(pname, m);
- }
- }
- }
- }
- // check type equality for getter-setter pairs
- Set<String> pairs = new HashSet<>(getters.keySet());
- pairs.retainAll(setters.keySet());
- for (String accessor : pairs) {
- Class<?> getterClass = getters.get(accessor).getReturnType();
- Class<?> setterClass =
setters.get(accessor).getParameterTypes()[0];
- if (!setterClass.isAssignableFrom(getterClass)) {
- throw new IllegalArgumentException(String
- .format("Accessor '%s' type mismatch, getter type is
%s while setter type is %s",
- accessor, getterClass.getName(),
setterClass.getName()));
- }
- }
- }
+ @SuppressWarnings("unchecked")
+ private void init() {
+ if (tclass == null) {
+ tclass = (Class<T>)tobj.getClass();
+ }
+
+ // Class.getMethods - The elements in the returned array are not
sorted and are not in any particular order.
+ // To provide some ordering, we can process DeclaredMethods first,
then pick up the remaining.
+ processMethods(tclass.getDeclaredMethods());
Review Comment:
Here is a sample of the getMethods return values for Azul Zulu and IBM
Semeru:
`Azul:
public org.apache.cxf.jaxrs.ext.search.BeanspectorTest$SimpleBean
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$MismatchedOverriddenBean.getSimpleBean()
public org.apache.cxf.jaxrs.ext.search.BeanspectorTest$OverriddenSimpleBean
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$MismatchedOverriddenBean.getSimpleBean()
public void
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$MismatchedOverriddenBean.setSimpleBean(java.lang.String)
public void
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$AnotherBean.setSimpleBean(org.apache.cxf.jaxrs.ext.search.BeanspectorTest$SimpleBean)
public final void java.lang.Object.wait(long,int) throws
java.lang.InterruptedException
public final void java.lang.Object.wait() throws
java.lang.InterruptedException
public final native void java.lang.Object.wait(long) throws
java.lang.InterruptedException
public boolean java.lang.Object.equals(java.lang.Object)
public java.lang.String java.lang.Object.toString()
public native int java.lang.Object.hashCode()
public final native java.lang.Class java.lang.Object.getClass()
public final native void java.lang.Object.notify()
public final native void java.lang.Object.notifyAll()
IBM:
public final native java.lang.Class java.lang.Object.getClass()
public final void java.lang.Object.wait() throws
java.lang.InterruptedException
public final void java.lang.Object.wait(long) throws
java.lang.InterruptedException
public final void java.lang.Object.wait(long,int) throws
java.lang.InterruptedException
public final native void java.lang.Object.notifyAll()
public void
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$AnotherBean.setSimpleBean(org.apache.cxf.jaxrs.ext.search.BeanspectorTest$SimpleBean)
public void
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$MismatchedOverriddenBean.setSimpleBean(java.lang.String)
public final native void java.lang.Object.notify()
public int java.lang.Object.hashCode()
public boolean java.lang.Object.equals(java.lang.Object)
public java.lang.String java.lang.Object.toString()
public org.apache.cxf.jaxrs.ext.search.BeanspectorTest$SimpleBean
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$MismatchedOverriddenBean.getSimpleBean()
public org.apache.cxf.jaxrs.ext.search.BeanspectorTest$OverriddenSimpleBean
org.apache.cxf.jaxrs.ext.search.BeanspectorTest$MismatchedOverriddenBean.getSimpleBean()`
You'll notice that the declared methods are listed last for IBM - other JVMs
have ordering the same or vastly similar to the Azul Zulu sample.
Now, why does this matter?
The Beanspector code picks up the first instance of methods on a class, so
for most JVMs the code gets the STRING SimpleBean, IBM gets
setSimpleBean(org.apache.cxf.jaxrs.ext.search.BeanspectorTest$SimpleBean) ,
which results in an Illegal Argument being thrown even though the correct
setter/getter pair does exist.
When we force the declared methods to be upfront in the return set, then
they appear to find the correct pairings. I originally set a flag for IBM
behavoir to protect the ordering from affecting any other JVM. This approach
puts all JVMs on the same ordering sematic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@cxf.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
