[GitHub] reta commented on a change in pull request #510: Fix CXF-7966: allows more flexibility in Beanspector

[GitBox <gi...@apache.org>]

reta commented on a change in pull request #510: Fix CXF-7966: allows more flexibility in Beanspector
URL: https://github.com/apache/cxf/pull/510#discussion_r255266525
 
 

 ##########
 File path: rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/Beanspector.java
 ##########
 @@ -58,31 +58,49 @@ 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)) {
-                getters.put(getPropertyName(m), m);
-            } else if (isSetter(m)) {
-                setters.put(getPropertyName(m), 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 (!getterClass.equals(setterClass)) {
-                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(); 
+        } 
+        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)) { 
 
 Review comment:
   Shouldn't it be reversed to `if (!getterClass.isAssignableFrom(setterClass)) {`? Like for example getter I think should always return the subclass (or exact type) of setter. Example here:
   ```
       class BaseBean {
           public Number getNumber() {
               return 0L;
           }
   
           public void setNumber(Number val) {
           }
       }
       
       class InheritableBean extends BaseBean {
           public void setNumber(Long val) {
           }
       }
   ```
   
   The `getNumber` return `Number` which could be assigned from `Long`, however `Long` is not assignable from `Number`. What do you think?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services