mrulli 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_r294174293
##########
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:
The rationale behind `if (!getterClass.isAssignableFrom(setterClass)) {` is
this: setters are neither covariant or contravariant while getters are
covariant. So I think the healthy behavior should be the following:
```
public class TestBean {
public Number getNumber() {
return 0L;
}
public void setNumber(Number val) { }
}
// FAIL (ambiguous setters)
public class InheritableBean1 extends TestBean {
public void setNumber(Number val) { }
public void setNumber(Long val) { }
}
// PASS (getters covariance)
public class InheritableBean2 extends TestBean {
public Long getNumber() {
return 0L;
}
}
// PASS (getters covariance and setter override)
public class InheritableBean3 extends TestBean {
public void setNumber(Number val) { }
public Long getNumber() {
return 0L;
}
}
```
This is provided through the current implementation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
