10 Replies Latest reply on Nov 14, 2007 1:06 PM by Scott Stark

    Leaking annotation API

    Carlo de Wolf Master

      I've got a nice API leakage:
      The @Cache annotation (which should be in jboss-ejb3-ext-api)

      package org.jboss.annotation.ejb;
      public @interface Cache
      {
       Class<? extends StatefulCache> value();
      }

      is using StatefulCache interface (deprecated until further notice),
      package org.jboss.ejb3.cache;
      public interface StatefulCache extends Cache<StatefulBeanContext>
      {
      ...
      }

      which extends Cache from jboss-ejb3-cache (will probably become jboss-ejb3-cache-spi), which is bound on StatefulBeanContext from the ejb3 code / module.

      The @Cache annotation is used by a bean developer to specify the cache strategy:
      @Cache(org.jboss.ejb3.cache.impl.NoPassivationCache)


      1. I would like to keep the limit on the annotation, it is meant to detect problems compile time;
      2. From ejb3 code perspective I want to use getAnnotation(Cache.class).value() without a type cast.

      A. How can we prevent such a leak?
      B. Where should I put NoPassivationCache? Note that there is no hard dependency from core to this class (it's configuration)..

        • 1. Re: Leaking annotation API
          Adrian Brock Master

          It looks to me like this just needs an indirection to use a factory?

          If you change the @Cache to be something like:

          public @interface Cache
          {
           Class<? extends CacheFactory> value();
          }
          


          with the CacheFactory being

          public interface CacheFactory
          {
           CacheInterface createCache();
          }
          


          Then you don't need the implementation classes on the client.

          You would need the implementation of the factory on the client
          (e.g. NoPassivationFactory) but not the cache implementation itself.

          In fact, in my testing, you don't even need the "CacheInterface" class
          on the client (at least with Sun's JDK?). It can still read the annotation
          without loading the classes on the CacheFactory methods.

          The other obvious alternative is to change the annotation to use a String
          but like you said this looses type safety.

          • 2. Re: Leaking annotation API
            Andrew Rubinger Master

            Some other leaks:

            @Clustered depends on org.jboss.ha.framework.interfaces.LoadBalancePolicy from trunk/cluster

            @PoolClass has:

            Class<? extends Pool> value();


            ..where Pool defines:

            BeanContext<?> get();


            ..and BeanContext exposes many internals: Container, etc.

            These must be resolved/reworked before the ext-api module can compile and be included into core.

            S,
            ALR

            • 3. Re: Leaking annotation API
              Scott Stark Master

               

              "ALRubinger" wrote:

              @Clustered depends on org.jboss.ha.framework.interfaces.LoadBalancePolicy from trunk/cluster

              This is also specific to a particular invocation implementation. This really can't be generically in ejb3 since its an aspect related to remoting. Its possible the server does not support anything but local ejbs, so ejb3 should not be providing this aspect.

              "ALRubinger" wrote:

              @PoolClass has:
              Class<? extends Pool> value();


              ..where Pool defines:

              BeanContext<?> get();


              ..and BeanContext exposes many internals: Container, etc.

              This is way too implementation specific for a client api. I would say you need to follow the factory approach suggested by Adrian where a client PoolFactory annotation provides the name of a registered Pool implementation. These would be registered with the factory in the ejb beans setup.

              @PoolFactory(type="EntityInstancePool",maxSize=10,timeout=300)
              



              • 4. Re: Leaking annotation API
                Brian Stansberry Master

                 

                "scott.stark@jboss.org" wrote:
                "ALRubinger" wrote:

                @Clustered depends on org.jboss.ha.framework.interfaces.LoadBalancePolicy from trunk/cluster

                This is also specific to a particular invocation implementation. This really can't be generically in ejb3 since its an aspect related to remoting. Its possible the server does not support anything but local ejbs, so ejb3 should not be providing this aspect.


                There's 2 issues here:

                1) The use of org.jboss.ha.framework.interfaces.LoadBalancePolicy in this annotation leaks the legacy org.jboss.invocation.Invocation class. My plan for dealing with this is to introduce a parent interface that doesn't include the method that leaks that type. See http://www.jboss.com/index.html?module=bb&op=viewtopic&t=120423. I need to do a serialization/deserialization test to confirm that won't break compatibility with existing clients. This I'll get done this week.

                2) Where does @Clustered go? To me it seems like it would go in the ha-client library that I started to break out of the main AS codebase a while back. That breakout was going to have to be done when we break ejb3 out of the AS codebase, and can be done sooner if need be.

                Problem with this is the name of the annotation is @org.jboss.annotation.ejb.Clustered -- not a very generic name.

                • 5. Re: Leaking annotation API
                  Scott Stark Master

                   

                  "bstansberry@jboss.com" wrote:

                  Problem with this is the name of the annotation is @org.jboss.annotation.ejb.Clustered -- not a very generic name.

                  Yes, should just be recreated using a transport/remoting oriented package name.


                  • 6. Re: Leaking annotation API
                    Brian Stansberry Master

                     

                    "scott.stark@jboss.org" wrote:
                    "bstansberry@jboss.com" wrote:

                    Problem with this is the name of the annotation is @org.jboss.annotation.ejb.Clustered -- not a very generic name.

                    Yes, should just be recreated using a transport/remoting oriented package name.


                    Carlo, unless you're willing to break existing applications, this would mean ejb-core would need to support two annotations -- @org.jboss.annotation.ejb.Clustered and @org.jboss.ha.client.remoting.Clustered. The former would presumably be deprecated, and only exist in ejb-core.jar.

                    • 7. Re: Leaking annotation API
                      Brian Stansberry Master

                      Seems the @Clustered annotation really is EJB-specific. With EJB3 you can still configure EJB homes, and that implies you should be able to configure a home load balance policy: http://jira.jboss.com/jira/browse/EJBTHREE-1110 . Adding an attribute for that makes this an ejb annotation. Can still go in the ha-client.jar though.

                      • 8. Re: Leaking annotation API
                        Andrew Rubinger Master

                         

                        "bstansberry@jboss.com" wrote:
                        Seems the @Clustered annotation really is EJB-specific. With EJB3 you can still configure EJB homes, and that implies you should be able to configure a home load balance policy: http://jira.jboss.com/jira/browse/EJBTHREE-1110 . Adding an attribute for that makes this an ejb annotation. Can still go in the ha-client.jar though.


                        FYI, today I'd moved it to HA and out of the EJB3 External API.

                        cluster/org.jboss.annotation.ha.Clustered

                        Maybe instead of adding an EJB-specific attribute to @Clustered, we make another annotation alltogether to configure the home load balance policy, placing that in EJB3?

                        S,
                        ALR

                        • 9. Re: Leaking annotation API
                          Anil Saldanha Master

                          The metadata project has a failing test.

                          Failed tests:
                           test1(org.jboss.test.metadata.annotation.ejb3.AnnotationEjb3UnitTestCase)
                          
                          Tests run: 99, Failures: 1, Errors: 0, Skipped: 0
                          


                          The error is:
                           <failure type="junit.framework.AssertionFailedError" message="no assembly de
                          scriptor defined">junit.framework.AssertionFailedError: no assembly descriptor d
                          efined^M
                           at junit.framework.Assert.fail(Assert.java:47)^M
                           at junit.framework.Assert.assertTrue(Assert.java:20)^M
                           at junit.framework.Assert.assertNotNull(Assert.java:220)^M
                           at org.jboss.test.metadata.annotation.ejb3.AnnotationEjb3UnitTestCase.te
                          st1(AnnotationEjb3UnitTestCase.java:164)^M
                           at org.jboss.test.metadata.annotation.ejb3.AnnotationEjb3UnitTestCase.te
                          st1(AnnotationEjb3UnitTestCase.java:164)^M
                          


                          • 10. Re: Leaking annotation API
                            Scott Stark Master

                            That failure is currently expected. Its part of the annotation to metadata work I'm doing.