[#CLJ-978] bean unable to handle non-public classes - Clojure JIRA

Details

Type: Defect
Status: Open
Priority: 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>.

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

clojure--bean-support-for-private-implementation-classes-v3.diff

06/May/12 2:59 PM

5 kB

Charles Duffy

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Charles Duffy added a comment - 30/Apr/12 10:40 PM

Fix inaccurate documentation string

Show

Charles Duffy added a comment - 30/Apr/12 10:40 PM Fix inaccurate documentation string

Charles Duffy made changes - 30/Apr/12 10:40 PM

Field	Original Value	New Value
Attachment		clojure--bean-support-for-private-implementation-classes-v1.diff [ 11145 ]

Charles Duffy made changes - 30/Apr/12 10:40 PM

Attachment

clojure--bean-support-for-private-implementation-classes-v0.diff [ 11144 ]

Hide

Permalink

Charles Duffy added a comment - 01/May/12 9:41 AM

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 - 01/May/12 9:41 AM 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

Permalink

Charles Duffy added a comment - 01/May/12 10:11 AM

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

Show

Charles Duffy added a comment - 01/May/12 10:11 AM Prior version of patch were missing new test suite files. Corrected.

Charles Duffy made changes - 01/May/12 10:11 AM

Attachment

clojure--bean-support-for-private-implementation-classes-v2.diff [ 11146 ]

Hide

Permalink

Andy Fingerhut added a comment - 04/May/12 2:48 AM

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 - 04/May/12 2:48 AM 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

Permalink

Charles Duffy added a comment - 06/May/12 2:59 PM

Added a patch created per documented process.

Show

Charles Duffy added a comment - 06/May/12 2:59 PM Added a patch created per documented process.

Charles Duffy made changes - 06/May/12 2:59 PM

Attachment

clojure--bean-support-for-private-implementation-classes-v3.diff [ 11173 ]

Charles Duffy made changes - 06/May/12 2:59 PM

Attachment

clojure--bean-support-for-private-implementation-classes-v1.diff [ 11145 ]

Charles Duffy made changes - 06/May/12 2:59 PM

Attachment

clojure--bean-support-for-private-implementation-classes-v2.diff [ 11146 ]

Hide

Permalink

Gary Trakhman added a comment - 04/Oct/12 6:44 PM

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 - 04/Oct/12 6:44 PM 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

Permalink

Rich Hickey added a comment - 29/Nov/12 10:01 AM

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 - 29/Nov/12 10:01 AM 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.

People

Assignee:

Unassigned

Reporter:

Charles Duffy

Vote (1)

Watch (1)

Dates

Created:

30/Apr/12 9:58 PM

Updated:

29/Nov/12 10:01 AM