Clojure

bean unable to handle non-public classes

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: Release 1.4
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

Take the following Java as an example:

public interface IFoo {
  String getBar();
}

class FooImpl {
  String getBar() { return "bar"; }
}

As presently implemented, (bean my-foo) tries to invoke the following:

(. #<Method public java.lang.String FooImpl.getBar> (invoke my-foo nil))

However, as FooImpl is not public, this fails:

java.lang.IllegalAccessException: Class clojure.core$bean$fn__1827$fn__1828 can not access a member of class FooImpl with modifiers "public"
 at sun.reflect.Reflection.ensureMemberAccess (Reflection.java:65)
    java.lang.reflect.Method.invoke (Method.java:588)
    clojure.core$bean$fn__1827$fn__1828.invoke (core_proxy.clj:382)
    clojure.core$bean$v__1832.invoke (core_proxy.clj:388)
    clojure.core$bean$fn__1838$thisfn__1839$fn__1840.invoke (core_proxy.clj:406)
    clojure.lang.LazySeq.sval (LazySeq.java:42)
    clojure.lang.LazySeq.seq (LazySeq.java:60)
    clojure.lang.RT.seq (RT.java:473)

However, the same thing succeeds if we call #<Method public java.lang.String Foo.getBar> rather than #<Method public java.lang.String FooImpl.getBar>.

Activity

Hide
Rich Hickey added a comment -

Charles, I think we should follow Apache BeanUtils on this. Exceptions not thrown are cheap. Ordinarily, exception for control flow are bad, but this is forced by bad design of reflection API.

Show
Rich Hickey added a comment - Charles, I think we should follow Apache BeanUtils on this. Exceptions not thrown are cheap. Ordinarily, exception for control flow are bad, but this is forced by bad design of reflection API.
Hide
Gary Trakhman added a comment -

I found in my code that it's possible to get a NPE if there is no read-method, for instance on the http://docs.cascading.org/cascading/2.0/javadoc/cascading/flow/hadoop/HadoopFlow.html object which has a setCascade method but no getter. I fixed this in our code by inlining the is-zero-args check into the public-method definition and and-ing the whole thing with 'method' like the original 'bean' code, like so:

public-method (and method (zero? (alength (. method (getParameterTypes))))
(or (and (java.lang.reflect.Modifier/isPublic (. c (getModifiers)))
method)
(public-version-of-method method)))

Show
Gary Trakhman added a comment - I found in my code that it's possible to get a NPE if there is no read-method, for instance on the http://docs.cascading.org/cascading/2.0/javadoc/cascading/flow/hadoop/HadoopFlow.html object which has a setCascade method but no getter. I fixed this in our code by inlining the is-zero-args check into the public-method definition and and-ing the whole thing with 'method' like the original 'bean' code, like so: public-method (and method (zero? (alength (. method (getParameterTypes)))) (or (and (java.lang.reflect.Modifier/isPublic (. c (getModifiers))) method) (public-version-of-method method)))
Hide
Charles Duffy added a comment -

Added a patch created per documented process.

Show
Charles Duffy added a comment - Added a patch created per documented process.
Hide
Andy Fingerhut added a comment -

Thanks for the patches, Charles. Could you please create a patch in the desired format and attach that, and then remove the obsolete patches? Instructions for creating a patch are under the heading "Development" at this page: http://dev.clojure.org/display/design/JIRA+workflow

Instructions for removing patches are under the heading "Removing patches" on that same page.

Show
Andy Fingerhut added a comment - Thanks for the patches, Charles. Could you please create a patch in the desired format and attach that, and then remove the obsolete patches? Instructions for creating a patch are under the heading "Development" at this page: http://dev.clojure.org/display/design/JIRA+workflow Instructions for removing patches are under the heading "Removing patches" on that same page.
Hide
Charles Duffy added a comment -

Prior version of patch were missing new test suite files. Corrected.

Show
Charles Duffy added a comment - Prior version of patch were missing new test suite files. Corrected.
Hide
Charles Duffy added a comment -

Apache Commons Beanutils has their own implementation of this, at http://www.docjar.com/html/api/org/apache/commons/beanutils/MethodUtils.java.html#771 – notably, it tries to reflect a method with the given signature and catches the exception on failure, rather than iterating through the whole list. This may be a better approach – I'm unfamiliar with how the cost of exception handling compares with that of reflecting on the full method list of a class.

Show
Charles Duffy added a comment - Apache Commons Beanutils has their own implementation of this, at http://www.docjar.com/html/api/org/apache/commons/beanutils/MethodUtils.java.html#771 – notably, it tries to reflect a method with the given signature and catches the exception on failure, rather than iterating through the whole list. This may be a better approach – I'm unfamiliar with how the cost of exception handling compares with that of reflecting on the full method list of a class.
Hide
Charles Duffy added a comment -

Fix inaccurate documentation string

Show
Charles Duffy added a comment - Fix inaccurate documentation string

People

Vote (2)
Watch (1)

Dates

  • Created:
    Updated: