Adding ManagedObjectCreator support to AbstractSimpleRealDep

[starksm64]

This seems like a natural boilerplate method that should be added to the AbstractSimpleRealDeployer to add support for ManagedObjectCreator. Do you agree?

 public void build(DeploymentUnit unit, Map<String, ManagedObject> managedObjects)
 throws DeploymentException
 {
 T deployment = unit.getAttachment(getInput());
 if (deployment != null)
 undeploy(unit, deployment);
 {
 ManagedObjectFactory factory = ManagedObjectFactoryBuilder.create();
 ManagedObject mo = factory.initManagedObject(deployment, null, null);
 if (mo != null)
 managedObjects.put(mo.getName(), mo);
 }
 }

[adrian.brock]

Looks ok to me since you would at least have to add @ManagementObject
to the metadata class for the factory NOT to return null.

So there can't be any accidents. :-)

[adrian.brock]

NOTE: There's an AbstractOptionalRealDeployer as well which could have it as well.

And I think the undeploy() is a cut and paste error. :-)

[starksm64]

Note that the parameterized type for these deployers should really be AbstractSimpleRealDeployer<T extends Serializable> as the deployment instance needs to be castable to Serializable for the ManagedObjectFactory.

[adrian.brock]

Note that the parameterized type for these deployers should really be AbstractSimpleRealDeployer<T extends Serializable> as the deployment instance needs to be castable to Serializable for the ManagedObjectFactory.

You can't do that. Its perfectly possible that the AbstractRealDeployer
is dealing with a transient attachment that is not intended for the management layer.

In fact, thinking about it. We shouldn't be encouraging the real deployers to do
the managed objects anyway. That should really be done in the parsing deployers.
I would be in favour of making the parsing deployers require Serializable attachments
which would mean the AppParsing deployer would currently fail to compile. ;-)

If both the parsing deployer and the real deployer have this code
for the same attachment its going to duplicate work.

So I'd like to change my vote to no. :-)

Although you could add a configuration flag to the deployer:

 private boolean buildManagedObject = false; // + getter and setter
 public void build(DeploymentUnit unit, Map<String, ManagedObject> managedObjects)
 {
 if (buildManagedObject)
 {
 ...
 }
 }

so it would be easy to enable it in other cases, i.e. via the configuration of the deployer.

[starksm64]

You can't do that. Its perfectly possible that the AbstractRealDeployer
is dealing with a transient attachment that is not intended for the management layer.

In fact, thinking about it. We shouldn't be encouraging the real deployers to do
the managed objects anyway. That should really be done in the parsing deployers.
I would be in favour of making the parsing deployers require Serializable attachments
which would mean the AppParsing deployer would currently fail to compile. ;-)

that is true and will show up when we try to do management in a profile service setup without the real deployers. The first parsing deployer AbstractParsingDeployerWithOutput. The buildManagedObject could control whether the managed object is created and the Serializable cast could be implicit rather than explicit. In the future we may want to relax the Serializable since jboss serialization does not need it.

[alesj]

that is true and will show up when we try to do management in a profile service setup without the real deployers. The first parsing deployer AbstractParsingDeployerWithOutput. The buildManagedObject could control whether the managed object is created and the Serializable cast could be implicit rather than explicit. In the future we may want to relax the Serializable since jboss serialization does not need it.

I changed the code that you've added.
There is no need to search/find attachment in order to determine if it is Serializable.
And there was a small bug - no return on non-Serializable. ;-)

Previous code:

 public void build(DeploymentUnit unit, Map<String, ManagedObject> managedObjects)
 throws DeploymentException
 {
 if (buildManagedObject == false)
 return;

 T deployment = unit.getAttachment(getOutput());
 if ((deployment instanceof Serializable) == false)
 log.debug("Skipping ManagedObject since T("+getOutput()+") is not Serializable");

 Serializable instance = (Serializable) deployment;
 if (deployment != null)
 {
 ManagedObjectFactory factory = ManagedObjectFactoryBuilder.create();
 ManagedObject mo = factory.initManagedObject(instance, null, null);
 if (mo != null)
 managedObjects.put(mo.getName(), mo);
 }
 }

My fix:

 public void build(DeploymentUnit unit, Map<String, ManagedObject> managedObjects) throws DeploymentException
 {
 if (buildManagedObject)
 {
 // we can check for Serializable w/o searching for attachment
 if (Serializable.class.isAssignableFrom(getOutput()) == false)
 {
 log.debug("Skipping ManagedObject since T(" + getOutput() + ") is not Serializable");
 return;
 }

 T deployment = unit.getAttachment(getOutput());
 if (deployment != null)
 {
 // must be Serializable - see getAttachment method contract (expectedType.cast(result))
 Serializable instance = (Serializable) deployment;
 ManagedObjectFactory factory = ManagedObjectFactoryBuilder.create();
 ManagedObject mo = factory.initManagedObject(instance, null, null);
 if (mo != null)
 managedObjects.put(mo.getName(), mo);
 }
 }
 }

OK?

[starksm64]

Yes.