1 2 Previous Next 17 Replies Latest reply on Sep 4, 2007 11:15 AM by Adrian Brock

    Annotation processing

    Adrian Brock Master

      While I was doing the scoping changes I broke the annotation processing
      and then had no idea what was going on.
      This processing has no error messages so you can't understand it
      if it doesn't work as you expect.

      I added some simple trace logging so you can at least debug which
      annotations are (not) being applied without having to attach a debugger. :-)

      Also the BasicBeanAdapter and related classes need fixing such that they use the
      MetaData not the MetaDataRetrieval.

      The same is true of its usage in the Scoping.

      In fact, the getMetaDataRetrieval() on the kernel repository should be removed.

      It's only real use at the moment is in AOP to find out if there are instance annotations.
      That processing is wrong anyway. There should be a
      MetaData.hasInstanceMetaData()
      instead.

        • 1. Re: Annotation processing
          Ales Justin Master

           

          "adrian@jboss.org" wrote:
          While I was doing the scoping changes I broke the annotation processing
          and then had no idea what was going on.
          This processing has no error messages so you can't understand it
          if it doesn't work as you expect.

          I added some simple trace logging so you can at least debug which
          annotations are (not) being applied without having to attach a debugger. :-)

          I'll have a look.
          Since I still have to add javadocs and definitely improve logging.
          But using debugger sounds like fun. :-)

          "adrian@jboss.org" wrote:

          Also the BasicBeanAdapter and related classes need fixing such that they use the MetaData not the MetaDataRetrieval.

          The same is true of its usage in the Scoping.

          MetaData has all the required functionality - as MetaDataRetrieval?

          Scoping as in PreInstall stuff that we did?

          "adrian@jboss.org" wrote:

          In fact, the getMetaDataRetrieval() on the kernel repository should be removed.

          It's only real use at the moment is in AOP to find out if there are instance annotations.
          That processing is wrong anyway. There should be a
          MetaData.hasInstanceMetaData()
          instead.

          Once I'm done, I'll try to remove it.

          • 2. Re: Annotation processing
            Ales Justin Master

             

            "adrian@jboss.org" wrote:

            It's only real use at the moment is in AOP to find out if there are instance annotations.
            That processing is wrong anyway. There should be a
            MetaData.hasInstanceMetaData() instead.


            What's the difference between local and non-local retrievers (from MetaDataContext):
             /**
             * Get the retrievals
             *
             * @return the retrievals
             */
             List<MetaDataRetrieval> getRetrievals();
            
             /**
             * Get the local retrievals
             *
             * @return the local retrievals
             */
             List<MetaDataRetrieval> getLocalRetrievals();
            


            I guess this needs removal as well (from KernelControllerContextAction):
             if (joinpoint instanceof KernelControllerContextAware)
             ((KernelControllerContextAware) joinpoint).setKernelControllerContext(context);
            ...
             if (joinpoint instanceof KernelControllerContextAware)
             ((KernelControllerContextAware) joinpoint).unsetKernelControllerContext(null);
            


            Since all information for AOPConstructorJoinpoint should then be in MetaData instance taken from the stack?

            Any quick tips on how MetaData.hasInstanceMetaData() should look like?
            Should it still use MetaDataContext as current impl in AOPConstructorJoinpoint does?

            • 3. Re: Annotation processing
              Ales Justin Master

               

              "alesj" wrote:

              "adrian@jboss.org" wrote:

              Also the BasicBeanAdapter and related classes need fixing such that they use the MetaData not the MetaDataRetrieval.

              The same is true of its usage in the Scoping.

              MetaData has all the required functionality - as MetaDataRetrieval?

              Scoping as in PreInstall stuff that we did?

              Yup, it was easy to move annotations handling to plain MetaData lookup. :-)

              I also (re)moved the part in PreInstallAction where MetaDataRetrieval was referenced from KernelMetaDataRepository.

              But I guess this code cannot be changed to use plain MetaData since I _need_ to build the whole MetaDataRetrieval for the scope - similar to how we do it for ControllerContext's MetaData (hidden in BasicKernelMetaDataRepository).

               // find scoped controller
               MutableMetaDataRepository mmdr = repository.getMetaDataRepository();
               MetaDataRetrieval mdr = mmdr.getMetaDataRetrieval(scopeKey);
               if (mdr == null)
               {
               mdr = new MemoryMetaDataLoader(scopeKey);
               mmdr.addMetaDataRetrieval(mdr);
               }
               MetaDataItem<ScopedKernelController> controllerItem = mdr.retrieveMetaData(ScopedKernelController.class);
               ScopedKernelController scopedController;
               if (controllerItem != null)
               {
               scopedController = controllerItem.getValue();
               }
               else
               {
               AbstractController parentController = null;
               ScopeKey parentKey = scopeKey.getParent();
               while (parentController == null && parentKey != null)
               {
               MetaDataRetrieval pmdr = mmdr.getMetaDataRetrieval(parentKey);
               if (pmdr != null)
               {
               MetaDataItem<ScopedKernelController> pci = pmdr.retrieveMetaData(ScopedKernelController.class);
               if (pci != null)
               {
               parentController = pci.getValue();
               }
               }
               parentKey = parentKey.getParent();
               }
               if (parentController == null)
               {
               if (controller instanceof AbstractController == false)
               throw new IllegalArgumentException("Underlying controller not AbstractController instance!");
               parentController = (AbstractController) controller;
               }
               scopedController = new ScopedKernelController(controller.getKernel(), parentController);
               ((MutableMetaData)mdr).addMetaData(scopedController, ScopedKernelController.class);
               }
               scopedController.addControllerContext(context);
               }
              


              • 4. Re: Annotation processing
                Adrian Brock Master

                 

                "alesj" wrote:

                What's the difference between local and non-local retrievers (from MetaDataContext):
                 /**
                 * Get the retrievals
                 *
                 * @return the retrievals
                 */
                 List<MetaDataRetrieval> getRetrievals();
                
                 /**
                 * Get the local retrievals
                 *
                 * @return the local retrievals
                 */
                 List<MetaDataRetrieval> getLocalRetrievals();
                



                A MetaDataContext is made of local retrievals and a parent context.
                retrievals = local + parent retrievals.


                Any quick tips on how MetaData.hasInstanceMetaData() should look like?
                Should it still use MetaDataContext as current impl in AOPConstructorJoinpoint does?


                Just move the AOP alogrithm into the MetaData/bridge.
                i.e. see whether the backing MetaDataContext has an INSTANCE scope
                and whether that scope has local annotations or metadata.

                A more complete api would be to allow
                MetaData.getScopeMetaData(ScopeKey)
                which would produce a MetaData backed by the local retrievals of that scope
                and add an isEmpty() to the MetaData api.

                The hasInstanceMetaData() could then be implemented as:
                MetaData instance = getScopedMetaData(instanceScope);
                return instance.isEmpty() == false;
                

                making it redundant.

                But that might be a bit more complicated to implement with the caching, etc.?

                • 5. Re: Annotation processing
                  Ales Justin Master

                   

                  "adrian@jboss.org" wrote:

                  and add an isEmpty() to the MetaData api.

                  What would mean that MetaData.isEmpty() == true?

                  This?
                   public boolean isEmpty()
                   {
                   MetaDatasItem items = retrieval.retrieveMetaData();
                   return items == null || items.getMetaDatas() == null || items.getMetaDatas().length == 0;
                   }
                  


                  • 6. Re: Annotation processing
                    Adrian Brock Master

                     

                    "alesj" wrote:
                    "adrian@jboss.org" wrote:

                    and add an isEmpty() to the MetaData api.

                    What would mean that MetaData.isEmpty() == true?

                    This?
                     public boolean isEmpty()
                     {
                     MetaDatasItem items = retrieval.retrieveMetaData();
                     return items == null || items.getMetaDatas() == null || items.getMetaDatas().length == 0;
                     }
                    


                    Yes, but I think you should propogate the isEmpty() through the api
                    such that it can be done in a more efficient manner where possible.
                    i.e. no need to construct the metadata array from the annotations and plain
                    metadata just to see if there are any.

                    • 7. Re: Annotation processing
                      Ales Justin Master

                      Regarding MetaData.getScopeMetaData() I have this next drawback. :-)
                      Looks like introduction of MetaDataContext into MetaDataRetrievalToMetaDataBridge doesn't really fit:

                       public MetaData getScopeMetaData(ScopeKey scopeKey)
                       {
                       if (retrieval instanceof MetaDataContext)
                       {
                       MetaDataContext context = (MetaDataContext)retrieval;
                       ...
                       }
                       return null;
                       }
                      


                      Perhaps having a new MetaDataContextToMetaDataBridge would do the trick, but this, at a quick glance, looks like a big propagation through the metadata stuff ...

                      • 8. Re: Annotation processing
                        Adrian Brock Master

                        If you want me to do it then I will, but don't ask to write all the code in the forum. :-)

                        As for your "drawback"

                        public interface MetaDataContext extends MetaDataRetrieval
                        
                        AND
                        
                         /**
                         * Create a new AbstractMetaDataContext.
                         *
                         * @param parent the parent
                         * @param retrievals the retrievals
                         */
                         public AbstractMetaDataContext(MetaDataContext parent, List<MetaDataRetrieval> retrievals)
                        


                        Since you need to reference ALL the local retrievals.

                        For the INSTANCE scope there's only probably one, but that certainly isn't true for a generic scope.

                        • 9. Re: Annotation processing
                          Adrian Brock Master

                          If you mean it isn't a MetaDataContext in the first place,
                          that's not going to happen in practice for the real scoped metadata used by applications,
                          but if it were true then it would represent only be a single local scope.

                          So the code would be:

                          if (retrieval instanceof MetaDataContext && scopeFoundInContext())
                           createBridgeForNewContextOfLocalRetrievals();
                          else if (singleRetrievalMatchesScope())
                           createBridgeForRetrieval();
                          else
                           // Not found
                           return null;
                          


                          • 10. Re: Annotation processing
                            Ales Justin Master

                             

                            "adrian@jboss.org" wrote:
                            If you want me to do it then I will, but don't ask to write all the code in the forum. :-)

                            I think this is the first attempt of somebody else then you writing anything to do with metadata. So I guess it's only fair to ask you before. :-)

                            "adrian@jboss.org" wrote:

                            public interface MetaDataContext extends MetaDataRetrieval
                            
                            AND
                            
                             /**
                             * Create a new AbstractMetaDataContext.
                             *
                             * @param parent the parent
                             * @param retrievals the retrievals
                             */
                             public AbstractMetaDataContext(MetaDataContext parent, List<MetaDataRetrieval> retrievals)
                            


                            Hmmm, I think I'm able to see the MetaDataContext extends MetaDataRetrieval. And looking at the impl's constructor. ;-)

                            "adrian@jboss.org" wrote:
                            If you mean it isn't a MetaDataContext in the first place,

                            No.
                            I mean that referencing MetaDataContext in MetaDataRetrievalToMetaDataBridge via 'instance of' is not what I would consider good OO. Specially with a name yelling out the purpose - Retrieval bridge ... not Context bridge. :-)


                            • 11. Re: Annotation processing
                              Adrian Brock Master

                               

                              "alesj" wrote:

                              No.
                              I mean that referencing MetaDataContext in MetaDataRetrievalToMetaDataBridge via 'instance of' is not what I would consider good OO. Specially with a name yelling out the purpose - Retrieval bridge ... not Context bridge. :-)


                              I agree, but the MetaData was never originally designed to retrieve structure
                              which is what MetaDataContext adds to the retrievals.

                              i.e. MetaDataRetrieval instanceof MetaDataContext == isRetrievalStructured()

                              The MetaData is actually designed to hide the structure so the client doesn't have to know
                              about it, i.e. it does not (need to) know what hierarchical scopes are linking retrievals together.

                              Its only this AOP requirement/optimization to ask if there is any instance metadata
                              (and only create a proxy if there is) that introduces the need to know structure.

                              Maybe we should leave it as the simple hasInstanceMetaData() on the MetaData api
                              and forget about revealing structure through an api that wasn't designed for it?
                              But you're still going to need to do the ugliness inside the bridge either way.

                              It's still better to have the ugliness hidden inside MetaData than
                              have AOP + others trying to play with things they almost certainly don't understand. :-)

                              I have similar thoughts about some of the code I did for the
                              ScopeInfo/ScopeBuilder which virtually duplicate the same code
                              to initialize non-existant scopes.
                              It would be much better to hide the ugliness inside the metadata api.

                              • 12. Re: Annotation processing
                                Ales Justin Master

                                 

                                "adrian@jboss.org" wrote:

                                Maybe we should leave it as the simple hasInstanceMetaData() on the MetaData api
                                and forget about revealing structure through an api that wasn't designed for it?
                                But you're still going to need to do the ugliness inside the bridge either way.

                                OK, I'll then put the getScopeMetaData/isEmpty ugliness into RetrievalBridge. :-)

                                • 13. Re: Annotation processing
                                  Ales Justin Master

                                   

                                  "alesj" wrote:

                                  OK, I'll then put the getScopeMetaData/isEmpty ugliness into RetrievalBridge. :-)

                                   public MetaData getScopeMetaData(ScopeKey scopeKey)
                                   {
                                   if (scopeKey == null)
                                   throw new IllegalArgumentException("Null scope key");
                                  
                                   if (retrieval instanceof MetaDataContext)
                                   {
                                   MetaDataContext context = (MetaDataContext)retrieval;
                                   List<MetaDataRetrieval> matchingRetrievals = new ArrayList<MetaDataRetrieval>();
                                   List<MetaDataRetrieval> localRetrievals = context.getLocalRetrievals();
                                   for (MetaDataRetrieval localRetrieval : localRetrievals)
                                   {
                                   if (scopeKey.equals(localRetrieval.getScope()))
                                   matchingRetrievals.add(localRetrieval);
                                   }
                                   if (matchingRetrievals.isEmpty() == false)
                                   return new MetaDataRetrievalToMetaDataBridge(new AbstractMetaDataContext(context, matchingRetrievals));
                                   }
                                   else if (scopeKey.equals(retrieval.getScope()))
                                   return new MetaDataRetrievalToMetaDataBridge(new AbstractMetaDataContext(retrieval));
                                   return null;
                                   }
                                  
                                   public boolean isEmpty()
                                   {
                                   return retrieval.isEmpty();
                                   }
                                  


                                  This is what I've pushed in (also added MetaDataRetrieval.isEmpty()). I think it should do the trick.

                                  But if I want to remove the hack with joinpoint being ControllerContext aware, I need to get the context's name from somewhere.


                                   ScopeKey instanceKey = new ScopeKey(CommonLevels.INSTANCE, "<context_name>");
                                   MetaData instanceMetaData = metaData.getScopeMetaData(instanceKey);
                                   if (instanceMetaData != null && instanceMetaData.isEmpty() == false)
                                   {
                                   return true;
                                   }
                                  


                                  Perhaps put in in the MetaData under 'ControllerContext.name' name?
                                  And push in/out this info in KernelControllerContextAction.dispatchJoinPoint invocation?
                                  Or just simply put in in for good when we first create MetaData for context - in PreInstallAction?

                                  • 14. Re: Annotation processing
                                    Ales Justin Master

                                     

                                    "alesj" wrote:

                                    Perhaps put in in the MetaData under 'ControllerContext.name' name?
                                    And push in/out this info in KernelControllerContextAction.dispatchJoinPoint invocation?
                                    Or just simply put in in for good when we first create MetaData for context - in PreInstallAction?


                                    Is doing this too hacky?

                                     protected void addMetaData(MutableMetaDataRepository repository, ControllerContext context, MemoryMetaDataLoader mutable)
                                     {
                                     mutable.addMetaData(ControllerContext.class.getSimpleName() + ".name", context.getName(), Object.class);
                                     }
                                    
                                     ...
                                    
                                     Object name = metaData.getMetaData("ControllerContext.name");
                                     if (name == null)
                                     return false;
                                    
                                     ScopeKey instanceKey = new ScopeKey(CommonLevels.INSTANCE, name.toString());
                                    
                                    


                                    I'm passing all tests with this new getScopeMetaData/isEmpty impl. :-)

                                    1 2 Previous Next