ClassInfo::getAnnotations bug

[alesj]

While doing annotation scanning on top of Reflect,
I stumbled upon what seems to me an invalid impl of how we get annotations of the class.

ClassInfoImpl uses InheritableAnnotationHolder who's getAnnotations returns all annotations in the class hierarchy.
This way we cannot just get the annotations declared on the class,
making it look like all annotations are marked as @Inherited.

In my case AnnotationRepository was picking up class which wasn't annotated, but its super class was.

Flavia and me fixed this by introducing a notion of getDeclaredAnnotations on InheritableAnnotationHolder,
which is then used by ClassInfoImpl.

But this leads to another problem. :-(
While testing Deployers, ManagedObjectClassLoadingParserUnitTestCase was failing for me.
This is due to the fact that Managed is trying to create ManagedObject
while looking for @ManagementObject on the attachment's class.

In this case the attachment is of ClassLoadingMetaData10,
where @ManagementObject is on its superclass, ClassLoadingMetaData.

With the current Reflection fix AbstractManagedObjectFactory::getAnnotation only checks top class,
not the whole hierarchy as before.

The fix for Managed could be:
* make @ManagementObject @Inherited
* check the whole hierarchy

But this seems more generic problem as just Managed.
I guess only change to Reflect 2.2.x will expose them all.

Or, is there actually a reason behind why we returned all annotations?
If there is one, how did we expect to just get class's annotations?
e.g. clazz::getAnnotations - clazz.getSuperclass.getAnnotations
I still prefer the way JDK does it. :-)

[flavia.rainone]

"Ales" wrote:
The fix for Managed could be:
* make @ManagementObject @Inherited
* check the whole hierarchy

I think there is a misunderstanding here. You said you wanted ClassInfo.getAnnotations to return only the declared annotations. I followed the nomenclature used on JDK, that says that declared annotations excludes inherited annotations. So, the way it is implemented now, it doesn't matter if @ManagementObject is inherited or not, because getAnnotations method is ignoring the inherited annotations.

Before we changed the getAnnotations implementation, this method was returning exactly all the annotations of that particular class plus the inherited ones, as it was delegating to Class<?>.getAnnotations. This was being tested and that's why I had to change the tests to make them pass with the new implementation.

"Ales" wrote:
I still prefer the way JDK does it. :-)

So maybe we should revert the changes and add a getDeclaredAnnotations method to AnnotatedInfo, similar to java.lang.reflect.AnnotatedElement interface?

[alesj]

Forget about this post. :-)

Looks like there was several oversights from my side.
* @ManagementObject is already @Inherited.
* gave bad instructions to Flavia
* probably my Deployers failing test was a result of some bug in my mc-ann code.

I guess we could introduce declared annotations,
but since this only differs for classes,
in that case one can use my set intersection suggestion.