[jboss-dev-forums] [Design the new POJO MicroContainer] - Re: MDR doesn't work on annotated privates
adrian@jboss.org
do-not-reply at jboss.com
Fri Aug 15 08:02:35 EDT 2008
"wolfc" wrote : "adrian at jboss.org" wrote : How's it supposed to know whether addAnnotation(Method, Annotation)
| | should be a MethodSignature or DeclaredMethodSignature?
| That's always a declared method, because else you won't be able to follow the Java rules for annotations on methods.
|
| Not it's not a DeclaredMethodSignature. The metadata contexts associated
| with those are not populated with the data loaded into MethodSignatures.
|
| The DeclaredMethodSignatures is a hack so you can shadow
| methods on the super class.
|
| It's not the same semantic. The semantic is designed to be the same as
| Class.get{Declared}Method(String name, Class.. parameters);
| the idea is you don't need to know the method/class, only its signature,
| that's why the class/key is called "Signature". :-)
|
| anonymous wrote :
| | Right now MethodSignature semantics is horribly broken as AnnotatedElementLoaderNotPublicUnitTestCase.testSameName shows.
|
| No it's not. It just does not do the horrible semantic you want it to do
| which is why we introduced the DeclaredMethodSignature.
|
| The test currently passes anyway, but then it is a useless test.
|
| The test that demonstrates the semantic you asked for is
| AnnotatedElementLoaderDeclaredMethodSignatureTestCase.
|
| The root of your difficulty is this call
|
| | MetaData superMethodMetaData = metaData.getComponentMetaData(Signature.getSignature(superMethod));
| |
|
| which doesn't return the superMethod metadata.
|
| When you should be doing
|
|
| | MetaData superMethodMetaData = metaData.getComponentMetaData(new DeclaredMethodSignature(superMethod));
| |
|
| We could add a new method:
|
| | public static Signature getSignature(Class<?> clazz, Member member)
| | {
| | if (member == null)
| | throw new IllegalArgumentException("Null member");
| |
| | if (member instanceof Method)
| | {
| | Method method = Method.class.cast(member);
| | Method other = method;
| | // See if we are masking the method
| | if (clazz != method.getDeclaringClass())
| | other = ReflectionUtils.findMethod(clazz, method);
| | // Yes
| | if (other.equals(method) == false)
| | return new DeclaredMethodSignature(member);
| | // No
| | else
| | return new MethodSignature(method);
| | }
| | ...
| |
|
| But then it would be horribly slow for all other users.
|
| Alternatively, we could change MethodSIgnature to include the "DeclaringClass"
| in its equals(). But then people who don't know the DeclaringClass,
| e.g. populating metadata from xml would have to do the findMethod().getDeclaringClass()
| upfront to populate the MethodSignature, otherwise the equals() would no longer work.
|
| Since Signature.equals() is designed to work before the classes are loaded
| that would be difficult - though not impossible with javassist,
|
| Nothing actually uses it that way at the moment, but there are obvious performance
| improvements (on the roadmap) to be gained by not duplicating annotation
| loading, javassist parsing and caching work across AOP, annotation scanning, ClassInfo, etc.
|
| If you continue to just say it is a bug, instead of suggesting how we make
| your semantic work without breaking (including performance) every other
| use case, I'd be inclined to just remove the *experimental* DeclaredMethodSignature
| and tell you to do the hack yourself in your code. :-)
View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4170737#4170737
Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=4170737
More information about the jboss-dev-forums
mailing list