Adding getAnnotationsAnnotatedWith() to MDR

[kabirkhan]

As mentioned here
http://www.jboss.org/index.html?module=bb&op=viewtopic&t=163887&start=10

"alesj" wrote:

Since most qualifiers I've seen (in jsr-299 and jsr-330) are picked out using annotations on the annotation, I propose adding something to BeanAnnotationAdapter to handle meta-annotations.

I would put this support directly to Reflect or MDR
- which ever suites best, but probably both will have to adapt.

BAA should just use this feature, not implement it.

I've added this to MetaData:

 /**
 * Get all the annotations annotated with the given meta annotation
 *
 * @param the meta annotation
 * @return the annotations annotated with the meta annotation
 */
 Annotation[] getAnnotationsAnnotatedWith(Class<? extends Annotation> meta);

and this to MetaDataRetrieval

 /**
 * Get all the annotations annotated with the given meta annotation
 *
 * @param the meta annotation
 * @return the annotations annotated with the meta annotation
 */
 AnnotationsItem retrieveAnnotationsAnnotatedWith(Class<? extends Annotation> meta);

The implemetation of this in AbstractMetaDataLoader

 public AnnotationsItem retrieveAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
 {
 AnnotationsItem annotations = retrieveAnnotations();
 List<AnnotationItem<? extends Annotation>> values = new ArrayList<AnnotationItem<? extends Annotation>>(annotations.getAnnotations().length);
 for (AnnotationItem<? extends Annotation> item : annotations.getAnnotations())
 {
 for (Annotation ann : item.getAnnotation().annotationType().getAnnotations())
 {
 if (meta == ann.annotationType())
 {
 values.add(item);
 break;
 }
 }
 }
 return new BasicAnnotationsItem(this, values.toArray(new AnnotationItem[values.size()]));
 }

And then the implemetation in AbstractMetaDataContext

 public AnnotationsItem retrieveAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
 {
 return new CummulativeAnnotationsItem(this, true, new AnnotationsAnnotatedWithFilter(meta));
 }

 private static class AnnotationsAnnotatedWithFilter implements CummulativeAnnotationsFilter
 {
 Class<? extends Annotation> meta;

 public AnnotationsAnnotatedWithFilter(Class<? extends Annotation> meta)
 {
 this.meta = meta;
 }

 public AnnotationsItem getAnnotations(MetaDataRetrieval retrieval)
 {
 return retrieval.retrieveAnnotationsAnnotatedWith(meta);
 }
 }

Changes to CummulativeAnnotationsItem

Index: src/main/java/org/jboss/metadata/spi/retrieval/cummulative/CummulativeAnnotationsItem.java
===================================================================
--- src/main/java/org/jboss/metadata/spi/retrieval/cummulative/CummulativeAnnotationsItem.java (revision 97282)
+++ src/main/java/org/jboss/metadata/spi/retrieval/cummulative/CummulativeAnnotationsItem.java (working copy)
@@ -49,6 +49,8 @@
 /** The valid time */
 private long validTime;

+ private CummulativeAnnotationsFilter filter = AllAnnotationsFilter.INSTANCE;
+
 /**
 * Create a new CummulativeAnnotationsItem.
 *
@@ -57,11 +59,25 @@
 */
 public CummulativeAnnotationsItem(MetaDataContext context, boolean includeParent)
 {
+ this(context, includeParent, null);
+ }
+
+ /**
+ * Create a new CummulativeAnnotationsItem.
+ *
+ * @param context the context
+ * @param includeParent whether to include the parent
+ */
+ public CummulativeAnnotationsItem(MetaDataContext context, boolean includeParent, CummulativeAnnotationsFilter filter)
+ {
 if (context == null)
 throw new IllegalArgumentException("Null context");

 this.context = context;
 this.includeParent = includeParent;
+ if (filter != null)
+ this.filter = filter;
+
 init(context.getValidTime().getValidTime());
 }

@@ -131,7 +147,7 @@

 for (MetaDataRetrieval retrieval : retrievals)
 {
- AnnotationsItem item = retrieval.retrieveAnnotations();
+ AnnotationsItem item = filter.getAnnotations(retrieval);
 if (item != null)
 {
 AnnotationItem<? extends Annotation>[] items = item.getAnnotations();
@@ -150,4 +166,14 @@
 setAnnotationItems(items);
 this.validTime = validTime;
 }
+
+ private static class AllAnnotationsFilter implements CummulativeAnnotationsFilter
+ {
+ private static final CummulativeAnnotationsFilter INSTANCE = new AllAnnotationsFilter();
+
+ public AnnotationsItem getAnnotations(MetaDataRetrieval retrieval)
+ {
+ return retrieval.retrieveAnnotations();
+ }
+ }
 }

I am currently reworking the current tests to also check this new functionality. One issue I have not yet looked into is the caching of this information, which is next on my list.

[kabirkhan]

Some more changes to AbstractMDL to avoid having to iterate over the contexts every time

===================================================================
--- src/main/java/org/jboss/metadata/plugins/loader/AbstractMetaDataLoader.java (revision 97282)
+++ src/main/java/org/jboss/metadata/plugins/loader/AbstractMetaDataLoader.java (working copy)
@@ -22,6 +22,8 @@
 package org.jboss.metadata.plugins.loader;

 import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.List;

 import org.jboss.metadata.spi.loader.MetaDataLoader;
 import org.jboss.metadata.spi.retrieval.AnnotationItem;
@@ -31,6 +33,7 @@
 import org.jboss.metadata.spi.retrieval.MetaDataRetrieval;
 import org.jboss.metadata.spi.retrieval.MetaDatasItem;
 import org.jboss.metadata.spi.retrieval.ValidTime;
+import org.jboss.metadata.spi.retrieval.basic.BasicAnnotationsItem;
 import org.jboss.metadata.spi.retrieval.helper.AnnotationToMetaDataBridge;
 import org.jboss.metadata.spi.retrieval.helper.AnnotationsToMetaDatasBridge;
 import org.jboss.metadata.spi.scope.ScopeKey;
@@ -54,6 +57,8 @@
 /** The scope key */
 private ScopeKey scopeKey;

+ private volatile AnnotationsAnnotatedWithCache cache;
+
 /**
 * Create a new AbstractMetaDataLoader.
 */
@@ -92,7 +97,47 @@
 {
 return retrieveAnnotations();
 }
+
+ protected AnnotationsAnnotatedWithCache getAnnotationsAnnotatedWithCache()
+ {
+ if (cache == null)
+ {
+ synchronized (this)
+ {
+ if (cache == null)
+ {
+ cache = new AnnotationsAnnotatedWithCache();
+ }
+ }
+ }
+ return cache;
+ }

+ public AnnotationsItem retrieveAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
+ {
+ AnnotationsItem item = getAnnotationsAnnotatedWithCache().getAnnotationsAnnotatedWith(meta);
+ if (item == null)
+ {
+ AnnotationsItem annotations = retrieveAnnotations();
+ List<AnnotationItem<? extends Annotation>> values = new ArrayList<AnnotationItem<? extends Annotation>>(annotations.getAnnotations().length);
+ for (AnnotationItem<? extends Annotation> current : annotations.getAnnotations())
+ {
+ for (Annotation ann : current.getAnnotation().annotationType().getAnnotations())
+ {
+ if (meta == ann.annotationType())
+ {
+ values.add(current);
+ break;
+ }
+ }
+ }
+ item = new BasicAnnotationsItem(this, values.toArray(new AnnotationItem[values.size()]));
+ cache.addAnnotationsAnnotatedWith(meta, item);
+ }
+ return item;
+ }
+
+
 @SuppressWarnings("unchecked")
 public <T> MetaDataItem<T> retrieveMetaData(Class<T> type)
 {
@@ -141,5 +186,12 @@
 public void invalidate()
 {
 validTime.invalidate();
+ invalidateAnnotationsAnnotatedWithCache();
 }
+
+ protected void invalidateAnnotationsAnnotatedWithCache()
+ {
+ if (cache != null)
+ cache.invalidate();
+ }
 }

The cache is basically just a wrapper around a map

public class AnnotationsAnnotatedWithCache
{
 private ConcurrentMap<Class<? extends Annotation>, AnnotationsItem> annotationsByMetaAnnotation = new ConcurrentHashMap<Class<? extends Annotation>, AnnotationsItem>();

 public AnnotationsItem getAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
 {
 AnnotationsItem annotationsItem = annotationsByMetaAnnotation.get(meta);
 if (annotationsItem != null)
 return annotationsItem;
 return null;
 }

 public void addAnnotationsAnnotatedWith(Class<? extends Annotation> meta, AnnotationsItem item)
 {
 annotationsByMetaAnnotation.putIfAbsent(meta, item);
 }

 public void invalidate()
 {
 annotationsByMetaAnnotation.clear();
 }
}

AMDL.invalidateCache() gets called when annotations are added/removed from MemoryMetaDataLoader, which seems to be the only AMDL subclass that changes the annotations

[kabirkhan]

What I have so far has been committed against https://jira.jboss.org/jira/browse/JBREFLECT-82.

I have not done anything about caching in CachingMetaDataContext or changed the SimpleCacheMetaDataContextUnitTestCase and SimpleCacheParentMetaDataContextUnitTestCase tests, which I guess work with CachingMetaDataContext?

[alesj]

Perhaps this cache impl / feature belongs into out CachingMDContext?

Or, if you already go over the annotations,
you could build the entire cache then - indexing all existing annotations and its metadata annotations.
So next time you ask for a meta annotation, you would already have the result.

[alesj]

What I have so far has been committed against https://jira.jboss.org/jira/browse/JBREFLECT-82.

This was moved to MDR's JIRA. ;-)

I have not done anything about caching in CachingMetaDataContext or changed the SimpleCacheMetaDataContextUnitTestCase and SimpleCacheParentMetaDataContextUnitTestCase tests, which I guess work with CachingMetaDataContext?

Thinking about your cache, this really belongs here -- into CachingMDContext.
And I would impl it in the way I described it before -- full indexing.

Since actually using CachingMDC full time is a complete overhead (we only use info at deploy/install time),
you should just wrap non-caching MDR before you do your (meta) annotations check.
This way meta cache will kick in almost transparently + it will be impled at the right level.
Then adding the tests is a must. ;-)

[kabirkhan]

When creating the BasicAnnotationsItem for the cache from the CachingMDC I need a reference to a MetaDataLoader which is used to determine if it is cachable:

 public BasicAnnotationItem(MetaDataLoader loader, T annotation)
 {
 super(loader, annotation.annotationType().getName(), annotation);
 }

 public T getAnnotation()
 {
 return getValue();
 }

CachingMDC does not implement MetaDataLoader so I can't pass in 'this'. My current ideas are either:
-make CachingMDC implement MDL and always return true from isCachable()
-create a MDL implementation that returns true if all the AnnotationItems are cachable, otherwise return false.

[kabirkhan]

This is the code I meant to post in my previous post, the loader is used from BasicItem

 public BasicItem(MetaDataLoader loader)
 {
 this.loader = loader;
 }

 public boolean isCachable()
 {
 return loader.isCachable(this);
 }

[kabirkhan]

So the choices should be
1)Make MDR implement isCachable() as described before
2Pass in null for the loader and create a subclass of BasicAnnotationsItem that overrides isCachable() and returns
a) true/false depending on if all the AnnotationItems were cachable
b) true always

I'll go with 2b) for now

[alesj]

Why would you need this?
I don't see anything similar in CachingMDC for other retrieveX methods.

All you need to do is cache the value from super.
Where is super you just calculate the result every time.

[kabirkhan]

Actually, I see BasicAnnotationsItem already does 2b) so I'll go with that bypassing the loader:

public class CachedAnnotationsItem extends BasicAnnotationsItem
{
 public CachedAnnotationsItem(AnnotationItem<? extends Annotation>[] annotationItems)
 {
 super(null, annotationItems);
 }

 @Override
 public boolean isCachable()
 {
 for (AnnotationItem<? extends Annotation> item : getAnnotations())
 {
 if (item.isCachable() == false)
 return false;
 }
 return true;
 }

[kabirkhan]

"alesj" wrote:
Why would you need this?
I don't see anything similar in CachingMDC for other retrieveX methods.

The problem is that I want to only iterate over everything once, which means I have to create the annotations item myself, e.g.:

 @SuppressWarnings("unchecked")
 Cache<Class, AnnotationsItem> cached = cachedMetaAnnotations;
 if (cached == null)
 {
 AnnotationsItem item = retrieveAnnotations();
 if (item.getAnnotations().length > 0)
 {
 Map<Class<? extends Annotation>, List<AnnotationItem<?>>> annotations = new HashMap<Class<? extends Annotation>, List<AnnotationItem<?>>>();
 for (AnnotationItem<?> annotation : item.getAnnotations())
 {
 for (Annotation current : annotation.getAnnotation().annotationType().getAnnotations())
 {
 List<AnnotationItem<?>> annotationsForMeta = annotations.get(current.annotationType());
 if (annotationsForMeta == null)
 {
 annotationsForMeta = new ArrayList<AnnotationItem<?>>();
 annotations.put(current.annotationType(), annotationsForMeta);
 }
 annotationsForMeta.add(annotation);
 }
 }

 cached = factory.createCache(Class.class, AnnotationsItem.class, getFqn());
 for (Map.Entry<Class<? extends Annotation>, List<AnnotationItem<?>>> entry : annotations.entrySet())
 {

 AnnotationsItem basic = new BasicAnnotationsItem(null, entry.getValue().toArray(new AnnotationItem<?>[entry.getValue().size()] ));
 cached.put(entry.getKey(), basic);
 }
 }
 cachedMetaAnnotations = cached;
 }
 return cached.get(meta);

So, I need to create the annotations item myself. If I don't pass in a loader, the BasicAnnotationsItem will fail on isCachable() since that calls the loader. This problem will also occur if when I move the pre-indexing logic into AbstractMetaDataContext.

[alesj]

I had something along this lines in mind

 public AnnotationsItem retrieveAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
 {
 Map<Class<?>, AnnotationsItems> index = retriveAnnotationsIndex();
 return new Commulative(this, true, index.get(meta));
 }

where CachingMDC does

protected Map<Class<?>, AnnotationsItem> retriveAnnotationsIndex()
{
 Map<Class<?>, AnnotationsItem> cached = cache.get(key);
 if (cached == null)
 {
 cached = super.retriveAnnotationsIndex();
 cache.put(key, cached);
 }
 else
 // validate, etc, ...

 return cached;
}