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.

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.

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.

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.

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?

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.

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.

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>

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.