"Empty" default values in annotations

[brian.stansberry]

There are a couple of places I've seen quasi-empty values used as defaults for annotation attributes. In both cases, the quasi-empty value can be taken to mean the user is saying "use the server default, whatever that is in the current context".

Question is, how should that "server default" be configured? Currently it is hard coded. Shouldn't it be externalized? If so, how, since EJB3 is using class annotations as the mechanism for passing around metadata, rather than mutable objects from jboss-metadata.jar?

Examples:

1) @Clustered.loadBalancePolicy. Specifies an impl of LoadBalancePolicy interface. Default is LoadBalancePolicy.class, i.e. the interface itself, not an impl.

The clustered proxy factories check for this condition and instantiate a hard coded default impl. StatelessClusteredProxyFactory uses RoundRobin; stateful uses FirstAvailable. So, context in which the annotation is applied helps drive the meaning of the attribute.

See http://www.jboss.com/index.html?module=bb&op=viewtopic&t=123791
and http://www.jboss.com/index.html?module=bb&op=viewtopic&t=120423
for discussion of why externalized configuration of these defaults could be helpful.

2) @CacheConfig.name. Specifies the name of the JBC instance to use to store clustered beans. Default was "jboss.cache:service=EJB3SFSBClusteredCache"; Al's recent changes converted it to "". I prefer "" -- using an ObjectName is too much of an implementation detail to be leaking as a default value.

This change in defaults is breaking deployments of clustered SFSBs, so I'm going to have to add code to figure out what to do when the value is "".

[starksm64]

1) @Clustered.loadBalancePolicy. Specifies an impl of LoadBalancePolicy interface. Default is LoadBalancePolicy.class, i.e. the interface itself, not an impl.
...

Falling back to hard-coded defaults seems ok, but the code that is setting up the proxy factory should be also getting the full configuration setup based on a container default configuration. I'd rather move away from the injected default annotations to a complete pojo metadata model where the "Stateless Bean" aop domain has a complete specification.

2) @CacheConfig.name. Specifies the name of the JBC instance to use to store clustered beans. Default was "jboss.cache:service=EJB3SFSBClusteredCache"; Al's recent changes converted it to "". I prefer "" -- using an ObjectName is too much of an implementation detail to be leaking as a default value.

This change in defaults is breaking deployments of clustered SFSBs, so I'm going to have to add code to figure out what to do when the value is "".

The only thing I can see to do here is to change the default to something like DefaultClusteredCache and expect that the factory bean is configured to alias this correctly.

[alrubinger]

2) @CacheConfig.name. Specifies the name of the JBC instance to use to store clustered beans. Default was "jboss.cache:service=EJB3SFSBClusteredCache"; Al's recent changes converted it to "". I prefer "" -- using an ObjectName is too much of an implementation detail to be leaking as a default value.

This change in defaults is breaking deployments of clustered SFSBs, so I'm going to have to add code to figure out what to do when the value is "".

Yes, by centralizing @CacheConfig from 2 annotations (in separate packages) to one, the context of the SFSB (clustered or not) should determine the default for "name", so for the time being I kept it as an empty String.

Maybe we introduce some logic:

if(bean.getAnnotation(Clustered.class)!=null)
{
 bean.getAnnotation(CacheConfig).setName = DefaultClusteredInterface.class;
}
else
{
 bean.getAnnotation(CacheConfig).setName = DefaultNonClusteredInterface.class;
}

...and change "name" from a String to a Class<?>. ?

S,
ALR

[wolfc]

Configuration in EJB 3 comes from ejb3-interceptors-aop.xml.

Here the augmenting and processing issue of aop.xml is relevant.

[bdecoste]

If you are interested in EJB3 configuration, please see the new online tutorial titled "Configuration". It describes ejb3-interceptors-aop.xml in vivid detail.

[brian.stansberry]

Is there a link?

If replacing the value of an attribute in an existing annotation can be done via ejb3-interceptors-aop.xml, that's great.

[bdecoste]

http://labs.jboss.org/jbossejb3/docs/tutorial/configuration/configuration.html

I just stuck that up there Fri, so feedback is appreciated!

[brian.stansberry]

Looking at the current usage in ejb3-interceptors-aop.xml I don't see anything that looks like changing the value of the attribute of an existing annotation; just the addition of annotations if they are missing. There may be some secret sauce in jboss-aop to do this, although I doubt it. Even if there is, I gotta imagine the xml to specify it would be really complex/breakable.

Falling back to hard-coded defaults seems ok, but the code that is setting up the proxy factory should be also getting the full configuration setup based on a container default configuration. I'd rather move away from the injected default annotations to a complete pojo metadata model where the "Stateless Bean" aop domain has a complete specification.

Yes, the way EJB2 metadata is merged from standardjboss.xml, jboss.xml container-configuration and the bean declaration itself is nice. Obviously EJB3 has different requirements with the use of annotations and nothing like standardjboss.xml.

I'll leave the hard-coded defaults; my concern was less with having those than with launching a discussion on having an external way to change the default.

The only thing I can see to do here is to change the default to something like DefaultClusteredCache and expect that the factory bean is configured to alias this correctly.

Perhaps "DefaultCache". Al points out this attribute could be consumed by a non-clustered cache as well. A value like that seems better than "" since if we ever wanted to externalize the aliasing its easier to map in xml.

[kabirkhan]

Looking at the current usage in ejb3-interceptors-aop.xml I don't see anything that looks like changing the value of the attribute of an existing annotation; just the addition of annotations if they are missing. There may be some secret sauce in jboss-aop to do this, although I doubt it. Even if there is, I gotta imagine the xml to specify it would be really complex/breakable.

Yes, it only adds more annotations. You can replace exisiting annotations, but not attributes

[alrubinger]

Yes, it only adds more annotations. You can replace exisiting annotations, but not attributes

Thanks Kabir, spent a bit today looking through the AOP docs for this specifically; was going to bug you about it tomorrow. :)

So back to the topic; looks like we'll have to address the blank @CacheConfig.name programmatically in Ejb3DescriptorHandler with regards to container.getAnnotation(Clustered.class) ...

How does my psudocode sample previously posted work for everyone?

S,
ALR

[brian.stansberry]

What's your thinking behind making the @CacheConfig.name attribute type a Class? At least for the current usage in the clustered case a String is fine.

[alrubinger]

What's your thinking behind making the @CacheConfig.name attribute type a Class? At least for the current usage in the clustered case a String is fine.

Compile-time constraints for the bean provider. Course, we're using Strings elsewhere (@Cache, @Pool) so maybe that'd make more sense for consistency.

S,
ALR

[brian.stansberry]

OK, well that's up to you guys then. The information a clustered SFSB cache would need is a key for a registry lookup, to use to access the underlying JBoss Cache. A simple String type for CacheConfig.name handles that. If we switched to an interface it would need to expose a method to get that key.