Deploying multiple service through the ServiceDeployer

[thomas.diesler]

Folks,

I have/had an issue with a deployment wanting to create multiple services

1) EJBDeployer attaches ServiceMetaData
2) WebServiceDeployerEJB generates and attaches JBossWebMetaData
3) TomcatDeployer also attaches ServiceMetaData

my proposed fix is

 // In case there already is a ServiceMetaData object attached, we use a set
 ServiceMetaData prevService = unit.getAttachment(ServiceMetaData.class);
 if (prevService != null)
 {
 ServiceMetaDataSet services = new ServiceMetaDataSet();
 services.add(prevService);
 services.add(webModule);
 unit.addAttachment(ServiceMetaDataSet.class, services);
 unit.removeAttachment(ServiceMetaData.class);
 }
 else
 {
 unit.addAttachment(ServiceMetaData.class, webModule);
 }

and a new deployer

public class MultipleServicesDeployer extends AbstractSimpleRealDeployer<ServiceMetaDataSet>
{
 private ServiceDeployer serviceDeployer;

 public MultipleServicesDeployer()
 {
 super(ServiceMetaDataSet.class);
 }

 public void setServiceDeployer(ServiceDeployer serviceDeployer)
 {
 this.serviceDeployer = serviceDeployer;
 }

 @Override
 public void deploy(DeploymentUnit unit, ServiceMetaDataSet deployment) throws DeploymentException
 {
 Iterator<ServiceMetaData> it = deployment.iterator();
 while (it.hasNext())
 {
 ServiceMetaData service = it.next();
 unit.addAttachment(ServiceMetaData.class, service);
 try
 {
 serviceDeployer.deploy(unit, service);
 }
 finally
 {
 unit.removeAttachment(ServiceMetaData.class);
 }
 }
 }
 ...

Please review.

[alesj]

No, this is already taken care of.
You just need to create new component on the deployment unit, with your new ServiceMetaData attached.
See ServiceDeploymentDeployer for more details.

[adrian.brock]

No, you don't create the components yourself.
You leave that to the component deployer.

The key part is that the attachment doesn't have to be named ServiceMetaData.class.getName(). Most people call it that,
but that's just a convention.
(It's always called that at the component level though).

The component deployer looks for any attachment that implements the
type, it's doesn't care what it is called and creates a component for it.

JMX component deployer
http://viewvc.jboss.org/cgi-bin/viewvc.cgi/jbossas/trunk/system-jmx/src/main/org/jboss/system/deployers/ServiceDeploymentDeployer.java?revision=63732&view=markup

Helper
http://viewvc.jboss.org/cgi-bin/viewvc.cgi/jbossas/projects/microcontainer/trunk/deployers-spi/src/main/org/jboss/deployers/spi/deployer/helpers/AbstractComponentDeployer.java?revision=67198&view=markup

 protected void deployComponents(DeploymentUnit unit) throws DeploymentException
 {
 if (compVisitor == null)
 return;

 Set<? extends C> components = unit.getAllMetaData(getOutput());
 for (C component : components)
 compVisitor.deploy(unit, component);
 }

And because it uses a visitor pattern, it knows how to unwind errors.

Your version suffers from the usual, "I'm not going to bother to tidyup
if I get an error part way through" bug. Which is just going to lead to
"InstanceAlreadyExists" errors in the MBeanServer when you try to redeploy.

From a brief look at the code you posted,
it breaks the following basic design princples of the deployers

1) It sidesteps the deployer chain with one deployer calling another directly
2) Since the metadata is deleted it won't be available to the management tool
3) It doesn't create components for the individual services, so there's no
way the IncompleteDeploymentException to know which MBeans are part of
the deployment and give a reasonable error message.

Finally, there already is a ServiceMetaDataSet, its called a ServiceDeployment
http://viewvc.jboss.org/cgi-bin/viewvc.cgi/jbossas/trunk/system-jmx/src/main/org/jboss/system/metadata/ServiceDeployment.java?revision=65120&view=markup

[adrian.brock]

my proposed fix is
...
Please review.

I be happier if you proposed and asked before you committed.
Now you'll have to unpick it all since you just reinvented the wheel again. ;-)

[adrian.brock]

1) EJBDeployer attaches ServiceMetaData
2) WebServiceDeployerEJB generates and attaches JBossWebMetaData
3) TomcatDeployer also attaches ServiceMetaData

If there's a conflict of attachment names between these deployers
(and you need these in the same deployment)
then we need to get people together to come up with unique attachment names.

Like I said above, the only time the attachment needs to be called
ServiceMetaData.class.getName() is when it becomes a component.

[thomas.diesler]

Please don't rollback just yet. I modify based on your feedback.

[adrian.brock]

Please don't rollback just yet. I modify based on your feedback.

I'd only roll it back if you broke the compilation or testsuite.

Otherwise, the "you broke it, you fix it" rule applies. ;-)

[thomas.diesler]

Removed the MultipleServicesDeployer and changed to

 unit.addAttachment("WarServiceMetaData", webModule, ServiceMetaData.class);

and

 unit.addAttachment("EjbServiceMetaData", ejbModule, ServiceMetaData.class);