reta commented on code in PR #1784:
URL: https://github.com/apache/cxf/pull/1784#discussion_r1552552637


##########
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:
   > 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.
   
   So we basically again are victims of the ordering, I would prefer to have 
make selection "smarter" by determining pairs of getters and setters based on 
arg / return type matches (otherwise we are not really solving the problem that 
you've identified). Happy to help if needed.



##########
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:
   > 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.
   
   So we basically again are victims of the ordering, I would prefer to make 
selection "smarter" by determining pairs of getters and setters based on arg / 
return type matches (otherwise we are not really solving the problem that 
you've identified). Happy to help if needed.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to