WW-4744: AnnotationUtils supports non-public methods #124
Conversation
| * @author Sam Brannen | ||
| * @author Mark Fisher | ||
| * @author Chris Beams | ||
| * @author Phillip Webb |
There was a problem hiding this comment.
I'm not sure if adding those authors here makes any sense, in most cases we (at ASF) trying to remove the @author tag also asking those users about this class can confuse them.
| Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes()); | ||
| result = getAnnotation(equivalentMethod, annotationType); | ||
| } catch (NoSuchMethodException ex) { | ||
| // No equivalent method found |
There was a problem hiding this comment.
no need to LOG this exception?
There was a problem hiding this comment.
No, it is common to not found the method in a super class during our hierarchically search.
|
Looks good, just have some doubts about putting |
|
Lukasz Lenart on dev@struts.apache.org replies: Should I ask legal [1] about that? What do you think? [1] https://issues.apache.org/jira/browse/LEGAL/ Regards
|
That's the class that has been copied from spring. Yes, I think it's better to ask how to handle this. |
I meant, coping classes is ok if the license allows that (Spring uses AL 2.0) but my questions was about adding authors to some other class (not copied from Spring), maybe let's allow Yasser explain why he did that. |
|
Do we need custom annotation utility at all? Can't we just use existing libraries e.g. Apache Commons MethodUtils? |
Thank you for your review.
Currently, I am searching Internet for same to see what people do. Finally, I also may ask Spring. |
|
We must rather ask ASF legal body, you can post an issue in JIRA (see above) |
Thank you. As I could not find similar issue, I created Merge others codes having same license but different copyrights. |
|
I also wait for ASF LEGAL part's answer but after some study, it seems I was not allowed to do such work 😞 If so, I think I have to write my own utils at where @aleksandr-m mentioned; re-inventing the wheel :( Below are references; what do you think, please? References:
[2] http://www.apache.org/legal/src-headers.html
[3] http://www.apache.org/dev/apply-license.html
|
Thank you @aleksandr-m ! I am working on it. I started by creating LANG-1317 |
|
✅ All copied Spring's code including what I was imported in previously merged PR#117 removed 👌 After merge of LANG-1317 PR and S2's upgrade to use commons lang 3.6, I'll remove deprecated |
|
So as I understand this is ready to be merged with a future notice to use Commons Lang 3.6 when available? |
|
@lukaszlenart , Yes in my opinion, Thank you! |
|
Cool, I will review this tomorrow just to double check and LGTM! Great work! |
|
Great work, LGTM! 👍 |
With these changes, AnnotationUtils also has equipped with an internal cache. Mainly, getAnnotatedMethods and all it's usages have improved and equipped with enough unit tests (also all findAnnotation usages have equipped).
NOTES:
To avoid reinventing the wheel, I copied from Spring framework, So, I copied their license and authors as well where needed :)