11 Replies Latest reply on Dec 7, 2009 10:27 AM by Ales Justin

    Adding getAnnotationsAnnotatedWith() to MDR

    Kabir Khan Master

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

      "alesj" wrote:
      "kabir.khan@jboss.com" 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.

        • 1. Re: Adding getAnnotationsAnnotatedWith() to MDR
          Kabir Khan Master

          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

          • 2. Re: Adding getAnnotationsAnnotatedWith() to MDR
            Kabir Khan Master

            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?

            • 3. Re: Adding getAnnotationsAnnotatedWith() to MDR
              Ales Justin Master

              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.

              • 4. Re: Adding getAnnotationsAnnotatedWith() to MDR
                Ales Justin Master

                 

                "kabir.khan@jboss.com" wrote:
                What I have so far has been committed against https://jira.jboss.org/jira/browse/JBREFLECT-82.

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

                "kabir.khan@jboss.com" wrote:

                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. ;-)


                • 5. Re: Adding getAnnotationsAnnotatedWith() to MDR
                  Kabir Khan Master

                  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.

                  • 6. Re: Adding getAnnotationsAnnotatedWith() to MDR
                    Kabir Khan Master

                    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);
                     }
                    
                    


                    • 7. Re: Adding getAnnotationsAnnotatedWith() to MDR
                      Kabir Khan Master

                      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

                      • 8. Re: Adding getAnnotationsAnnotatedWith() to MDR
                        Ales Justin Master

                        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.

                        • 9. Re: Adding getAnnotationsAnnotatedWith() to MDR
                          Kabir Khan Master

                          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;
                           }
                          


                          • 10. Re: Adding getAnnotationsAnnotatedWith() to MDR
                            Kabir Khan Master

                             

                            "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.



                            • 11. Re: Adding getAnnotationsAnnotatedWith() to MDR
                              Ales Justin Master

                              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;
                              }