You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Benedikt Ritter (JIRA)" <ji...@apache.org> on 2012/06/18 22:27:42 UTC
[jira] [Created] (SANDBOX-423) [BeanUtils2] Wrap checked exceptions
from BeanAccessor.get() into BeanReflectionException
Benedikt Ritter created SANDBOX-423:
---------------------------------------
Summary: [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
Key: SANDBOX-423
URL: https://issues.apache.org/jira/browse/SANDBOX-423
Project: Commons Sandbox
Issue Type: Sub-task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
Fix For: Nightly Builds
* Change method signature of BeanAccessor.get()
* in DefaultBeanAccessor.get() catch all checked exceptions
* modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13396724#comment-13396724 ]
Simone Tripodi commented on SANDBOX-423:
----------------------------------------
I think Jörg approach is interesting - I'd like you could do anyway an experiment by starting from the exception you already defined in the first patch, and specializing pattern/arguments in derived exceptions, i.e. (quick sample)
{code}
BeanPropertyNotFoundException( String propertyName, Class<?> target, Throwable cause )
{
super( cause, format( "Property %s not found in %s type", propertyName, target.getName() ) );
this.propertyName = propertyName;
this.target = target;
}
{code}
If the experimentation fails, we'll be always in time to change the signatures since we're still in sandbox - rooms for improvements will always be available :)
best and TIA
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13408874#comment-13408874 ]
Simone Tripodi commented on SANDBOX-423:
----------------------------------------
Hi Bene,
{quote}
We discussed to throw exceptions as soon as possible to get rid of all the try/catch code in DefaultBeanAccessor. I started to implement that, but things got way out of hand, so I decided to go back to the first solution to make it easier to review.
We can change the exception handling later.
{quote}
I like the way you are implementing that feature, go ahead in that way!!! :)
{quote}
One thing I don't like about my solution is, that an IntrospectionException will be wrapped in a NoSuchPropertyException, but this is due to the Introspection API.
{quote}
Let's keep it as it is - hopefully a (maybe?) better solution will come up later...
{quote}
Also constructing a Property[Setter|Getter]InvocationException feels clumsy because one has to pass in both, the property name and the property accessor method name. Maybe you have a suggestion how to improve that?
{quote}
I am not sure there's a better/automatic way... take in consideration that via the {{*BeanInfo}} classes, users can alter accessor methods, so users' beans/POJOs not necessarily respect the code conventions...
Positive feedback from my side, I think you can go ahead with on getting rid of checked exceptions on other {{BeanAccessor}} methods, so I will apply the patch. Well done! :)
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch, SANDBOX-423v2.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Benedikt Ritter (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13396683#comment-13396683 ]
Benedikt Ritter commented on SANDBOX-423:
-----------------------------------------
Ah okay, I misunderstood your comment on the ML. I thought that we'd implement additional excpetions when they are requested by users. I'll improve the patch tonight. Would be good if you find the time to share your thoughts on Jörgs suggestion to use something like o.a.c.lang3.exception.Contexted(Runtime)Exception instead of adapting the code from the ErrorMessage class.
Benedikt
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Comment Edited] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13396571#comment-13396571 ]
Simone Tripodi edited comment on SANDBOX-423 at 6/19/12 7:10 AM:
-----------------------------------------------------------------
Patch looks already good. Before to commit it, I'd suggest you (as I suggested in the thread on ML) to provide a new patch where introducing {{BeanReflectionException}} specializations, such as {{BeanPropertyNotFoundException}}, {{Property(Reader|Writer)MethodNotFound}} and so on, in order to drop the {{TODO}}s in your testcase.
Well done anyway.
was (Author: simone.tripodi):
Patches looks already good. Before to commit it, I'd suggest you (as I suggested in the thread on ML) to provide a new patch where introducing {{BeanReflectionException}} specializations, such as {{BeanPropertyNotFoundException}}, {{Property(Reader|Writer)MethodNotFound}} and so on, in order to drop the {{TODO}}s in your testcase.
Well done anyway.
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (SANDBOX-423) [BeanUtils2] Wrap checked exceptions
from BeanAccessor.get() into BeanReflectionException
Posted by "Benedikt Ritter (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benedikt Ritter updated SANDBOX-423:
------------------------------------
Attachment: SANDBOX-423.patch
I've attached a patch for this issue. There are a few things to note:
* There is a lot of try-catch-boiler plate code. To reduce that code I see two possibilities:
** Use some kind of template method implementation
** only catch Exception e
* I'm not sure how to make sure that the BeanReflectionException being thrown in GetPropertyTestCase wraps the right exception. Maybe there are matchers for this in hamcrest that I'm currently not aware of. I've marked that as TODOs.
TIA
Benedikt
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (SANDBOX-423) [BeanUtils2] Wrap checked exceptions
from BeanAccessor.get() into BeanReflectionException
Posted by "Benedikt Ritter (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benedikt Ritter updated SANDBOX-423:
------------------------------------
Attachment: SANDBOX-423v2.patch
I finally had the time to do some more work on this issue.
I have attached a new patch file.
We discussed to throw exceptions as soon as possible to get rid of all the try/catch code in {{DefaultBeanAccessor}}. I started to implement that, but things got way out of hand, so I decided to go back to the first solution to make it easier to review.
We can change the exception handling later.
One thing I don't like about my solution is, that an {{IntrospectionException}} will be wrapped in a {{NoSuchPropertyException}}, but this is due to the Introspection API.
Also constructing a Property[Setter|Getter]InvocationException feels clumsy because one has to pass in both, the property name and the property accessor method name. Maybe you have a suggestion how to improve that?
TIA
Benedikt
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch, SANDBOX-423v2.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Benedikt Ritter (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13422582#comment-13422582 ]
Benedikt Ritter commented on SANDBOX-423:
-----------------------------------------
Now that SANDBOX-419 has been applied, I was able to finish my work on this issue. Since removing all checked exceptions from BeanAccessor involved removing checked exceptions from other party of the API, I had to do it all in one patch. I'm adding that patch to SANDBOX-422, because that feels more appropriate.
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch, SANDBOX-423v2.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Simone Tripodi resolved SANDBOX-423.
------------------------------------
Resolution: Fixed
Assignee: Simone Tripodi
closed as per SANDBOX-422 resolution
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Assignee: Simone Tripodi
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch, SANDBOX-423v2.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-423) [BeanUtils2] Wrap checked
exceptions from BeanAccessor.get() into BeanReflectionException
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13396571#comment-13396571 ]
Simone Tripodi commented on SANDBOX-423:
----------------------------------------
Patches looks already good. Before to commit it, I'd suggest you (as I suggested in the thread on ML) to provide a new patch where introducing {{BeanReflectionException}} specializations, such as {{BeanPropertyNotFoundException}}, {{Property(Reader|Writer)MethodNotFound}} and so on, in order to drop the {{TODO}}s in your testcase.
Well done anyway.
> [BeanUtils2] Wrap checked exceptions from BeanAccessor.get() into BeanReflectionException
> -----------------------------------------------------------------------------------------
>
> Key: SANDBOX-423
> URL: https://issues.apache.org/jira/browse/SANDBOX-423
> Project: Commons Sandbox
> Issue Type: Sub-task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Fix For: Nightly Builds
>
> Attachments: SANDBOX-423.patch
>
>
> * Change method signature of BeanAccessor.get()
> * in DefaultBeanAccessor.get() catch all checked exceptions
> * modify GetPropertyTestCase accordingly
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira