Should a deployment fail if duplicate Transformers are detected

[beve]

Hi,

I'm running into this issue at the moment upon deployment of a quickstart I get the following error in AS 7:

15:00:36,315 ERROR [org.jboss.msc.service.fail] (MSC service thread 1-1) MSC00001: Failed to start service jboss.deployment.unit."switchyard-quickstarts-hornetq-binding-0.2.0-SNAPSHOT.jar".SwitchYardService: org.jboss.msc.service.StartException in service jboss.deployment.unit."switchyard-quickstarts-hornetq-binding-0.2.0-SNAPSHOT.jar".SwitchYardService: org.switchyard.exception.SwitchYardException: Failed to register Transformer 'org.switchyard.component.hornetq.transformer.ByteArrayToStringTransformer(java:byte[], java:java.lang.String)'.  A Transformer for these types is already registered: 'org.switchyard.component.camel.transformer.CamelTransformer(java:byte[], java:java.lang.String)'.
        at org.switchyard.as7.extension.services.SwitchYardService.start(SwitchYardService.java:90)
        at org.jboss.msc.service.ServiceControllerImpl$StartTask.run(ServiceControllerImpl.java:1765)
        at org.jboss.msc.service.ServiceControllerImpl$ClearTCCLTask.run(ServiceControllerImpl.java:2291)
        at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886) [:1.6.0_26]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908) [:1.6.0_26]
        at java.lang.Thread.run(Thread.java:680) [:1.6.0_26]
Caused by: org.switchyard.exception.SwitchYardException: Failed to register Transformer 'org.switchyard.component.hornetq.transformer.ByteArrayToStringTransformer(java:byte[], java:java.lang.String)'.  A Transformer for these types is already registered: 'org.switchyard.component.camel.transformer.CamelTransformer(java:byte[], java:java.lang.String)'.
        at org.switchyard.transform.TransformerRegistryLoader.registerTransformers(TransformerRegistryLoader.java:88)
        at org.switchyard.transform.TransformerRegistryLoader.loadOOTBTransforms(TransformerRegistryLoader.java:131)
        at org.switchyard.deploy.internal.AbstractDeployment.init(AbstractDeployment.java:148)
        at org.switchyard.as7.extension.deployment.SwitchYardDeployment.start(SwitchYardDeployment.java:89)
        at org.switchyard.as7.extension.services.SwitchYardService.start(SwitchYardService.java:83)
        ... 5 more

Now this is correct and I think this situation might arise now and again. In this case the hornetq component has a transformer and so does the camel component for the same types. But if the camel component is not installed as a subsystem the hornetq one would be used instead.

Could we change this into logging a  warning instead so that the deployment does not fail?

Regards,

/Daniel

[kcbabo]

Hmm ... this is an interesting situation.  There are actually two possible scenarios here:

1)  One or more runtime components (including the core runtime's OOTB stuff) have transformers which overlap.  This is what you're hitting now. 

2) An application is deployed with a transformer which conflicts with an existing transformer.

For #1, I agree with you that it should be logged as a warning.  For #2, I think this should result in a deployment error.  If the user is explicitly including a transformer for a set of types where a transformer already exists, then we should fail fast and raise a deployment exception.

So what if the transformer registry threw a DuplicateTransformerException from the register method?  The code which loads the OOTB transformers could catch this exception and print a warning message.  The code which loads application transformers can treat it as an error.

Thoughts?

[tfennelly]

We could try it for a while anyway and see how it works.  I purposely coded it to fail fast, but it has caused me a PITA on a few occassions too.

[kcbabo]

This is why I like the idea of throwing an explicit exception related to a duplicate entry.  That way, the code handling the registration can decide whether it's fatal or not.

[beve]

Something like this perhaps?

[tfennelly]

Looks good Dan.  However... I would have it extend SwitchYardException.  I'd guess that SwitchYardException will become a checked exception at some point.

[kcbabo]

Yeah, that looks spot on.

[beve]

However... I would have it extend SwitchYardException.  I'd guess that SwitchYardException will become a checked exception at some point.

I started out doing that but was not sure about what to do in the catch clause in registerTransformers:

try {
            for (TransformModel transformModel : transforms.getTransforms()) {
                Collection<Transformer<?, ?>> transformers = TransformerUtil.newTransformers(transformModel);

                for (Transformer<?, ?> transformer : transformers) {
                    if (_transformerRegistry.hasTransformer(transformer.getFrom(), transformer.getTo())) {
                        Transformer<?, ?> registeredTransformer = _transformerRegistry.getTransformer(transformer.getFrom(), transformer.getTo());
                        throw new DuplicateTransformerException("Failed to register Transformer '" + toDescription(transformer)
                                + "'.  A Transformer for these types is already registered: '"
                                + toDescription(registeredTransformer) + "'.");
                    }

                    _log.debug("Adding transformer =>"
                            + " From: " + transformer.getFrom()
                            + ", To:" + transformer.getTo());
                    _transformerRegistry.addTransformer(transformer);
                    _transformers.add(transformer);
                }
            }
        } catch (RuntimeException e) {
            // If there was an exception for any reason... remove all Transformer instance that have
            // already been registered with the domain...
            unregisterTransformers();
            throw e;
        }

When I made DuplicateTransformerException extends SwitchYardException the catch clause is entered which is not really what we want. Should this be changed now or should we wait until SwitchYardException is a checked exception?

I created a jira for this now.