AOPConstructorJoinpoint::methodHasAnnotations is wrong

[alesj]

When doing a check if current underlying context needs to be instantiated as an AOP proxy, we were looking in both cases - class and methods - if there is an INSTANCE scope metadata present.

But in case I have this, adding annotations to property:

 <bean name="Bean6" class="org.jboss.test.microcontainer.support.SimpleBeanImpl">
 <property name="property">
 <annotation>@org.jboss.test.microcontainer.support.SomeNonIA</annotation>
 <annotation>@org.jboss.test.microcontainer.support.ContainsIA</annotation>
 <value>123</value>
 </property>
 </bean>

this is called - a place where this xml annotations get put into metadata:
KernelScopeInfo

 /**
 * Add annotations for a method
 *
 * @param classloader the classloader
 * @param mutable the mutable metadata
 * @param methodInfo the method info
 * @param annotations the annotations
 */
 private void addAnnotations(ClassLoader classloader, MemoryMetaDataLoader mutable, MethodInfo methodInfo, Set<AnnotationMetaData> annotations)
 {
 ScopeKey scope = new ScopeKey(CommonLevels.JOINPOINT_OVERRIDE, methodInfo.getName());
 MemoryMetaDataLoader loader = new MemoryMetaDataLoader(scope);
 addAnnotations(classloader, loader, annotations);
 mutable.addComponentMetaDataRetrieval(new MethodSignature(methodInfo), loader);
 }

Property annotations are added to JOINPOINT_OVERRIDE level, not to INSTANCE.

So, this really never worked in AOPConstructorJoinpoint - to get a proxy for beans that have annotations added to methods via xml.

Whether property annotations are put to the wrong place, or we are doing a wrong level lookup. I suspect the other. :-)

I'll fix code to use JOINPOINT_OVERRIDE level when checking whether methods have 'instance' metadata.

[alesj]

"alesj" wrote:

Whether property annotations are put to the wrong place, or we are doing a wrong level lookup. I suspect the other. :-)

Looking at Kabir's comment on recent change:

[JBMICROCONT-250] When checking component metadata for a method, only return that we have joinpoint annotations if they exist at instance level or below.

Looks like a case of wrong impl or wrong conclusion on how MetaData.getScopeMetaData(ScopeLevel) works.

Should we really always check for all levels below and including INSTANCE?
How do we know level hierarchy?

[alesj]

"alesj" wrote:

How do we know level hierarchy?

OK, ScopeLevel implements Comparable. :-)

[alesj]

"alesj" wrote:

"alesj" wrote:

How do we know level hierarchy?

OK, ScopeLevel implements Comparable. :-)

I've added simple util class, to get sub levels:

package org.jboss.metadata.spi.scope;

public class CommonLevelsUtil
{
 ...

 /**
 * Get the levels (including level param) that
 * are below level param.
 *
 * @param level the flag level
 * @return sub list of levels
 */
 public static List<ScopeLevel> getSubLevels(ScopeLevel level)
 {
 int index = levels.indexOf(level);
 if (index < 0)
 throw new IllegalArgumentException("No such scope level in levels: " + level);
 return levels.subList(index, size);
 }

 /**
 * Get the levels that are exclusively below level param.
 *
 * @param level the flag level
 * @return sub list of levels
 */
 public static List<ScopeLevel> getExclusiveSubLevels(ScopeLevel level)
 {
 int index = levels.indexOf(level);
 if (index < 0)
 throw new IllegalArgumentException("No such scope level in levels: " + level);

 if (index + 1 == size)
 return Collections.emptyList();

 return levels.subList(index + 1, size);
 }
}

[alesj]

"alesj" wrote:

Looking at Kabir's comment on recent change:

[JBMICROCONT-250] When checking component metadata for a method, only return that we have joinpoint annotations if they exist at instance level or below.

Looks like a case of wrong impl or wrong conclusion on how MetaData.getScopeMetaData(ScopeLevel) works.

Should we really always check for all levels below and including INSTANCE?

I've added this hierarchy check:

 private boolean checkForMetaDataAtSubInstanceLevel(MetaData metaData)
 {
 if (metaData != null)
 {
 List<ScopeLevel> levels = CommonLevelsUtil.getSubLevels(CommonLevels.INSTANCE);
 for (ScopeLevel level : levels)
 {
 boolean hasMetaData = hasMetaDataAtLevel(metaData, level);
 if (hasMetaData)
 return true;
 }
 }
 return false;
 }

And I'm ignoring any annotations that are marked with InstanceAnnotation(false):

 protected boolean hasMetaDataAtLevel(MetaData metaData, ScopeLevel level)
 {
 MetaData levelMetaData = metaData.getScopeMetaData(level);
 if (levelMetaData != null && levelMetaData.isEmpty() == false)
 {
 Set<Object> checkSet = new HashSet<Object>();
 Object[] allMetaData = levelMetaData.getMetaData();
 Annotation[] annotations = levelMetaData.getAnnotations();
 // all meta data is not null, since instance metadata is not empty
 checkSet.addAll(Arrays.asList(allMetaData));
 checkSet.removeAll(Arrays.asList(annotations));

 // do we have something else than annotations
 if (checkSet.isEmpty() == false)
 return true;
 else
 {
 // do we have an annotation that's not marked with IA
 for (Annotation annotation : annotations)
 {
 InstanceAnnotation ia = annotation.annotationType().getAnnotation(InstanceAnnotation.class);
 if (ia == null || ia.value())
 return true;
 }
 }
 }
 return false;
 }

[alesj]

While going over all sub INSTANCE levels, I also use JOINPOINT, where 'real' method annotations live.
This means that I get hasSubInstanceMetaData == true for beans that have class annotated methods.
So this:
- http://www.jboss.org/index.html?module=bb&op=viewtopic&t=131170&start=10
again fails.

I've added InstanceAnnotation(false) to all our IoC annotations.
So that makes the tests pass.
But it would still create proxy for any other non-InstanceAnnotation marked annotations that would live on methods (on class).

Should I exclude JOINPOINT in that hierarchy?

[kabirkhan]

You added the @HasInstanceAnnotaiton to the @JMX annotation which causes the JMXDecoratedTestCase to fail. I have reverted that change, but it causes your HasInstanceAnnotation test to fail.

[alesj]

You added the @HasInstanceAnnotaiton to the @JMX annotation which causes the JMXDecoratedTestCase to fail. I have reverted that change, but it causes your HasInstanceAnnotation test to fail.

See this discussion:
- http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4140167#4140167
And these two JIRAs:
- http://jira.jboss.com/jira/browse/JBMDR-21
- http://jira.jboss.com/jira/browse/JBMICROCONT-279

What's the usage of @JMX in that JMXDecoratedTC, apart for marking the bean to register it into MBeanServer?

[kabirkhan]

OK having read that thread, I agree. The test I mentioned is there for legacy reasons, and works using the KernelControllerContextAware introduction + a binding + pointcut intercepting that. I think that this test can be removed since it is superseded by JMXLifecycleTest anyway (which uses lifecycle-configure)

I have reverted the change I made to the @JMX annotation

[kabirkhan]

As mentioned here http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4141090#4141090 I am having second thoughts on the @InstanceAnnotation