Keith:
PayloadTypeName - I'm not sure I get this one. A potential use I see is if you have a naming strategy that's different from the (admittedly ugly)
class+operation+parameter format used by default. But that doesn't appear to be the way it's used currently. Seems more like it's a workaround
to address the fact that we don't have a standard type name for HandlerException when it's used as the content of a fault message. Is that right?
If so, seems reasonable and it looks useful in your test code. BTW, we should probably move this annotation and the JavaService class to
org.switchyard.metadata.java. We also need to add a getFaultName() method to ServiceOperation, right?
Right, and I explicitly made that annotation @Inherited. My immediate use for it was on the HandlerException class and because it's inherited, it means we have a generalized data type for all our exceptions, unless a specific implementation of HandlerException redefine that annotation themselves (if they want a different transform). So in the case of the soap component, I added that general HandlerExceptionTransform which will transform any HandlerException type (sent as a fault payload) to a SOAP Fault. So I think it's a useful enough way of supporting the creation of generalized transformers for family of types.
Keith:
ServiceOperation.Name - this is probably the thing I like the least about the changes. I realize you did it to standardize on the naming of
operation name in the context. I wonder if we should have a specific ExchangeContext interface with a getOperationName() method?
Yeah… that's a bit shit for sure Yes… I think making the ServiceOperation (from the ServiceInterface) available on the Exchange makes sense (not just the operation name).
Keith:
TransformHandler "static String toMessageType" seems like a weird place to put this method. I think
the public contract for a handler should be what's defined in the ExchangeHandler interface and any
utility methods should be move to another class.
Yeah… I was just being a bit lazy tbh. I wasn't sure at the time where to put it and so just left it there. I think it's doing stuff similar to what the JavaService class does, so should be rolled in there somewhere.
Keith:
Invocation - I don't quite get why assertOK() exists. Seems like this tests whether transformation
happened vs. just testing to make sure the message type matches the method parameter. I have a
feeling I'm missing a nuance here.
It does a bit more than just check if transforms were applied, even just for that, I think it's more appropriate to check if the actual defined transform sequence was applied because what if the resulting java type of the transform was the same as the input type to the transform? Your java type check would pass, but the content of the instance is probably not what it should be.
Keith:
SwitchYardCDIExtension - discovery of Transformer instances seems reasonable to me, although I
wonder whether this should be done by the Bean container of if it should be done by the
SwitchYardDeployer (i.e. part of our general application deployment framework).
Yeah… I have a feeling this will go. I like the convenience of it, but I think it's a bit to magical/"funky".
I do think we could come up with some useful "convention" based mechanisms e.g. users define a transformers.xml file in the META-INF/switchyard folder of their module. This could be a convention based way of explicitly registering the transformers you want to have registered by the deployer (which just scans the cp for all "META-INF/switchyard/transformers.xml" resources).
Keith:
DefaultMessageDecomposer - I would really like to see us avoid coding converter logic into individual
components. The SOAP gateway expects XML and that can come in a bunch of forms : byte[], String,
InputStream, Reader, Element, Node, XMLStreamReader, Source, etc, etc. We should use our own super
awesome transformation framework to handle these cases.
You saying we should avoid the decomposer mechansim completely and just use the transform mechanism within exchange? I'd agree with that.
Keith:
HandlerExceptionTransformer - any reason why we're not using a typed Transformer definition here
e.g. Transformer<HandlerException, String>
There is a reason… I'm a lazy ***
Keith:
InboundHandler - is it possible for the runtime to assign a TransformSequence based on the message
name in the exchange and the service operation's message name? This would replace
associateInTransformSequence and associateOutTransformSequence in InboundHandler.
Yeah… I've been thinking about this too. I think it would make sense and goes back to the earlier point… making it possible to hook the ServiceOperation (from the ServiceInterface) onto the Exchange instance. In this case, the SOAP Gateway would perform this task based on the incoming SOAP request. It would simply lookup the ServiecOperation from the Service and set it onto the Exchange.
Then, we could have another default handler for processing this on all exchanges (if present), creating and attaching the IN and OUT TransformSequence instances in a generalized way. I wouldn't do this in the TransformHandler.
Keith:
TransformSequence - the cacheOutput and cacheFault methods seem a bit funky to me. I realize
why they are there, I'm just wondering if there's another way to go about it. This is probably
tied to the whole input/output phase thing you have been talking about.
I think the answer to this is tied up with the answer to the previous one. As well as attaching details about the target ServiceOperation, it also needs to attach the "requirements" it has in terms of the response coming back.
Perhaps we have something like an InvocationMetaData type that can get set on an Exchange (by e.g. the SOAPGateway) e.g.
public interface InvocationMetaData {
ServiceOperation getServiceOperation();
InvokerOperationMetaData getInvokerOperationMetaData();
}
public inteface InvokerOperationMetaData {
Set<String> acceptedResponseTypes;
Set<String> acceptedFaultTypes;
}