proposed mods to SARDeployer

[sebersole]

The HARDeployer has a lot of duplicated code in common with the SARDeployer. The only real differences between the two being:
1) The service descriptor used to describe the service
2) "preparation of the defined mbeans" during create, which is something extra the HARDeployer does to inject some values into its MBean.

So locally, I've modified the HARDeployer to simply extend a slightly modified version of the SARDeployer. The only differences in the new SARDeployer:
1) a new getRelativeServiceDescriptorName(), overrideable by subclasses, which defines the deployment-relative descriptor to locate; used throughout instead of hard-coded "META-INF/jboss-service.xml";
2) a slightly more modularized init() method to allow subclasses to override certain pieces of init-processing;
3) a slightly more modularized create() method to allow subclasses to override stuff here also;
4) changes to log statements that hard-coded "SAR"

Any objections to committing these changes?

[adrian.brock]

Common code should be in SubDeployerSupport.

I even created a SimpleSubDeployerSupport, that models more how deployment should be done, rather than exposing all the details of SubDeployerSupport.

1) Define your extension - getExtension()
2) Define where your metadata xml exists relative the main deployment - getMetaDataURL()
3) Define your deployment's ObjectName - getObjectName()
4) Define your deployment's class - getDeploymentClass()
5) Define your xml parsing - getObjectModelFactory()

The idea is that the deployer creates an MBean for each deployment.

Then the SimpleSubDeployer will create the MBeans of type getDeploymentClass()
with the name getObjectName() for each deployment and handle the lifecycle for you,
including injecting the parsed xml document's javabean equivalent into the deployment

The RAR deployer is currently the only one that uses it.

[sebersole]

So you and I did a bunch of the same stuff ;)

So really I was extending SARDeployer for
1) its XML parsing; since it is the same logic needed for the HAR;
2) its deployment lifecycle logic; since, aside from the two deviations mentioned previously, it is exactly the same needed for the HAR

One significant difference is that HAR and SAR can create multiple MBeans per deployment. The HAR descriptor, for example, might define both a Hibernate-MBean and a TreeCache-MBean. Certainly this distinction goes away after the ability is added to allow multiple deployers to be able to interact with a single deployment; but it is currently a limitation here.

[sebersole]

Just to complete my thought ;)

Given the limitation I mentioned regarding the potential of multiple heterogeneous-typed mbeans, would your suggestion be to extract the pertinent lifecycle logic out of SARDeployer into a new deployer support class (say, HeterogeneousSubDeployerSupport) which provides that same kind of "lifecycle simplification" to this deployment scenario and having both SARDeployer and HARDeployer extend that new deployer support?

[adrian.brock]

Can't you just extend SimpleSubDeployerSupport to have multiple mbeans deployed?
i.e. create a MultipleSubDeployerSupport or whatever you want to call it.

Alternatively (and probably better), you could do it the other way around and make a
SingleSubDeployerSupport that specialises the multiple deployment version
with just one MBean deployed.

[sebersole]

I don't understand what you mean by

specialises the multiple deployment version with just one MBean deployed

By "one MBean deployed" are you refering to the deployer MBean? Or the MBean deployed by the deployer? ;) I assume you mean that we create a SARDeployment and HARDeployment similiar to the RARDeployment used in the RARDeployer to plug into the getDeploymentClass()?

But the getDeploymentClass() piece is a bit limiting here I think, when you look at it from the perspective of SAR/HAR (unless I am missing something, which is always a possibility ;) The reason is that the JCA deployment is failry close-ended, whereas the SAR/HAR is open-ended. What I mean by this is that a JCA adapter is always going to have the same fixed number of "subordinate" collaboration MBeans (i.e., MCF-MBean, CM, etc). Fixed both in terms of the number as well as the type of each. That's not the case for the SAR/HAR stuff though. So from what I saw in the RARDeployment class, it takes on the "configuration face" for the JCA deployment for the end-user; they interact with it, and then it interacts with the various back-end pieces. That would be pretty difficult to acheive with SAR/HAR deployments.


Essentially, all the code I want is already defined in SARDeployer. The reason is that a HAR is really just a simple service archive except that the HibernateMBean additionally needs to know about the deployment url (so that it can locate the hibernate mappings).

If y'all want that I take that SARDeployer code and move it into another base class intended for the SAR/HAR use-cases, then I'm fine with that. The other option is that SARDeployer could just take on that role. Either way...

[adrian.brock]

I'm speaking in the abstract because I don't fully understand what you
are trying to achieve.

I am saying don't extend SARDeployer. Just basic OO principles tells me
that the HAR deployer is NOT a SAR deployer.

public class X extends Y
means "X isA Y" in OO speak.

I don't see why you need to touch SARDeployer. It is something that
works now. In fact, the whole problem is a bit moot since we are changing
the way deployers works anyway for JBoss5.

[sebersole]

I am saying don't extend SARDeployer. Just basic OO principles tells me
that the HAR deployer is NOT a SAR deployer.

public class X extends Y
means "X isA Y" in OO speak.

I don't disagree. But basic software maintence philosphy tells me to not write a class that is 340 lines of code where approximately 300 is duplicated somewhere else ;)

I need to also make changes in JB4. Is the deployer redesign going to be backported?

[adrian.brock]

On my proposal. I am saying:

1) SubDeployerSupport is too generic (deliberatly) since it allows things like
Tomcat deployments to be processed through it.

2) Ideally, deployments should be:
archive/file -< MBeans/JavaBeans
-< means many-to-one

3) I wrote SimpleSubDeployerSupport that does
archive/file -- MBean
-- means one-to-one

So my proposal is to have something like:
StandardSubDeployerSupport extends SubDeployerSupport
SimpleSubDeployerSupport extends StandardSubDeployerSupport

StandardSubDeployerSupport would allow multiple mbeans per deployment
something like
String[] getDeploymentClasses()
etc.
and SimpleSubDeployerSupport would just be a specialzation one for deployment
String[] getDeploymentClasses()
{
return new String[] { getDeploymentClass() };
}
and even that depends upon whether you think such a simplication is useful.
e.g. you may want something closer to the wire in the standard deployer?
ServiceMBean[] getMBeans()

But like I say, there isn't much point spending too much time on figuring out
an ideal abstraction when there will be a much better deployer framework in JBoss5.

[adrian.brock]

I don't disagree. But basic software maintence philosphy tells me to not write a class that is 340 lines of code where approximately 300 is duplicated somewhere else ;)

Agreed, but also 300 lines across 4 or five classes isn't that big an issue
when they hardly ever change.
It becomes an issue when you want to add cross cutting behaviour
on the fly (hence the need for the new framework).

I need to also make changes in JB4. Is the deployer redesign going to be backported?

Not that I'm aware of, although Dimitris was doing an experiment with
the deployment aspects working as XMBean interceptors?
http://www.jboss.org/index.html?module=bb&op=viewtopic&t=57296

[dimitris]

I think the experiment overall worked (although Scott had some reservations about it so it's in HEAD but not 4.x).

It was about providing the EJBDeployer (in fact, it should work with all other deployers) the ability to instantiate and tear-down security domains, in the presence of a META-INF/login-config.xml.

In fact, a lot of the the functionlity was already there, you write a JMX interceptor and apply it to an XMBean, so you need (a) to XMBean-ize the target deployer, (not a big deal) and (b) make sure your deployer is always accessed through the MBeanServer and not using a direct reference (some subdeployers do that).

So I added the ability to attach/detach this interceptor dynamically to an XMBean. As a convenience I've made org.jboss.deployment.SubDeployerInterceptor that can be subclassed to demonstrate this.

If the HARDeployer can be thought-of as an add-on (aspect, interceptor, whatever) to SARDeployer or other subdeployers then this will probably work.

[sebersole]

I think Adrian is correct in that a HARDeployer and a SARDeployer are conceptually very different "things". There is certainly a lot of overlap in functionality (the worst part being the xml parsing).

After further thought on this, it seems a much better approach here is the simply better encapsulate this redundant logic into delegates. Especially if you think about the parsing of the service descriptors. Maybe even a JBoss XMBean "thingy". Then others can easily re-use that logic in custom deployers.

I remember now what brought me to this situation now. Currently what the HARDeployer does is to essentially say that its 'hibernate-service.xml' file is a sub-deployment (i.e., SubDeployerSupport.isDeployable). The sole reason it does this is so that the SARDeployer can then pick it up and parse the xml descriptor, register mbeans... yuck!

I was trying to clean that up so that the "nested deployment" concept was not needed.

So I guess, in a way, a HAR deployment *is* is SAR :)

[starksm64]

This is why the JBossXB framework needs to be adopted. Alexey already has the basis for this as one of the JBossXB testcases. The processing of the mbean deployment info could then be extracted out and even put into a xmbean interceptor that placed the deployment metadata into the DeploymentInfo context.

The problem I had with the solution Dimtris came up with was with the LoginConfigInterceptor specifically, not the more general ability to add interceptors which was fine. A generalization which would improve the extensibility of the deployer configuration is described in this thread, but its not necessary to this discussion.

http://www.jboss.org/index.html?module=bb&op=viewtopic&p=3864275#3864275

Maybe you could look at extracting out the xml processing using JBossXB and encapsulate this into a resulable xmbean interceptor Dimitris.

[dimitris]

It seems to me the whole hibernate deployer could be replaced by an xmbean interceptor to the SARDeployer that checks in the init step if any of the specified MBeans is of type org.jboss.hibernate.jmx.Hibernate, in order to do the extra "HarUrl" call to it to complete the setup.

However, that means that we don't need the .har archives any more. Those will be ordinary .sar deployments with that extra capability.

Also, I don't quite understand why/how using JBossXB would help in this case. Currently the deployed MBean for hibernate looks like:

<mbean code="org.jboss.hibernate.jmx.Hibernate" name="jboss.har:service=Hibernate">
<attribute name="DatasourceName">java:/DefaultDS</attribute>
<attribute name="SessionFactoryName">java:/hibernate/SessionFactory</attribute>
<attribute name="Dialect">net.sf.hibernate.dialect.HSQLDialect</attribute>
<attribute name="CacheProviderClass">net.sf.hibernate.cache.HashtableCacheProvider</attribute>
<attribute name="Hbm2ddlAuto">create-drop</attribute>
</mbean>

[starksm64]

Yes, after looking at what the HARDeployer is doing this looks correct. If there was a general behavior of exposing a SarURL attribute from the ServiceMBeanSupport level the Hibernate service could simply use that and drop the HarUrl. There is a notion in the Hibernate service that allows the HarUrl to be updated at runtime, but I don't see any usage of this feature.

The point of the JBossXB usage was to extract the parsing of the deployment metadata and reuse the object based methods of the SARDeployer, but this is not needed with the current implementation.