"wolfc" wrote : &quot;adrian(a)jboss.org&quot; 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#... Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&a...