[jboss-dev-forums] [Design the new POJO MicroContainer] - Re: MDR doesn't work on annotated privates

[adrian@jboss.org]

"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