I'm A-OK with letting Dispatcher instances create the exchange instance and agree that SerivceReferenceImpl creating ExchangeImpl directly is ugly. As long as the API stays the same, this seems like an improvement.
I went to next steep which requires an agreement. Graphs thanks to draw.io. Current flow with Camel exchange is following:
In other worlds, an incoming message is lost during mediation. Camel do not keep any reference to it. If an processor returns OUTPUT message which is different than INBOUND then this message is forgotten. The OUTPUT message become only one message used by Camel.
Now if we'll take a look on another diagram when exchange pattern is IN-OUT:
We have a moment when we'll have two messages - IN sent from Gateway and OUT message created by service implementation. In this case Camel will lost IN message and it's headers (including org.switchyard.messageId) and others. Because of that some functionality may stop to work. Our tests in Camel Exchange Bus already fails because of that, and components will do the same.
I see here two solutions, which are similar to current shape of Camel Exchange Bus implementation:
- Keep IN message as exchange property after OUT message is specified
- Keep IN message headers as exchange property
The simplest solution is forcing processors to use only one message and encorcing this policy across whole mediation. However IN property (message header) may be overriden by OUT property (message header). When Consumer Callback will try to use IN property overriden by OUT we may get unexpected results. WDYT?
Can you provide a concrete example of something that fails? Reason I ask is that the current exchange setup has a single slot for message, so an exchange handler only has access to one message at any given point in time. The provider handler is arguably an exception to this rule, but only because it creates the reply message and then sends it in reply. Even in that case, however, the output of exchange.getMessage() is only one message. In the current reply path, handlers cannot get access to the original request message.
The body of message is not a problem. What will fail now is getting IN-scoped context property after calling provider handler. That's the place where new message is usualy put into exchange. Since then we have no access to old message instance sent from gateway. Actually it's not a problem of camel or switchyard api, it's rather problem with assumption I've made. What I was asuming before this problem is that property X from IN scope will be mapped to camel exchange.getIn().getHeader(X). Similar for OUT scope - exchange.getOut().getHeader(X) and Exchange scope - exchange.getProperty(X). With current model since camel exchange.getIn message may gets overriden during execution of route I started looking for workaround. Maybe best solution will be to change assumption?
Sorry, completely lost track of this thread. I wouldn't expect IN scoped properties to be available on the reply path. In fact, I would consider any code that relies on that to be a bug. You mentioned test failures above, so are you saying that we have code which relies on in-scoped properties on the reply path?
Test failures are mostly caused by "null" pointers jumping here and there - these may be fixed by improving implementation.
I didn't know that IN scope don't have to be available on reply path, if so, why we have two scopes instead of one called message?
Because that creates a problem for the provider which needs to have access to read IN-scoped properties and set OUT-scoped properties.
Sorry, but why wouldn't Scope.IN properties be available on the reply path? Nothing clears them out of the Context, at least that I'm aware of...
Because they are scoped to IN. The context might not be cleaning them out, but AFAIK that wasn't a deliberate decision to make them available. If a property is valid for request and reply paths, it should be at EXCHANGE scope.
Personally I like having all properties (IN, OUT, EXCHANGE) be available in the Context no matter if we're on the request path, or the reply path. Who knows if a developer wants to make a decision on the reply-path based on what originally came in? However, if we do make the change to clear them out, then I don't see the point of having IN and OUT scopes. The scopes would just be MESSAGE and EXCHANGE, as Lukasz mentioned.
You can't just have a MESSAGE scope unless we invent a way to read IN scoped properties and write OUT scoped properties inside a service provider implementation. If there's a suggestion for how the API can be changed to accommodate that, then I'm all for it.
Keith is right, IN properties must be available for provider processor together with OUT properties - provider may read incoming properties and write outgoing at the "same time".
However "rolling" of message in Camel impiles a mapping between scopes and message headers. Initially I assumed that we will be able to use one Camel Exchange and IN/OUT messages as placeholders for IN/OUT properties. Since Camel uses OUT message from one processor as IN message for next processor in queue it complicates things a lot. Camel Exchange is not so bad, however Camel Context starts to be workaround.
The biggest benefit I see now is error handling - since Camel and SwitchYard exchange is same thing there is no problem with different states between two - FaultInterceptStrategy is not necessary at all, everything what's needed is done in Processor implementation called at the begining of onException closure.
Summary of messaging API
The SwitchYard messaging API is clear and works really well with present bus implementations (local and camel). The main problem I do see now is relation between in/out context and messages. In general - the ide of having just one message for exchange is really pure. The message which remains in Exchange goes straight forward to consumer, when we have IN-OUT pattern set. I wouldn't change this at all. One thing which makes me afraid is lack of relation between message and context properties, even in case when message have headers. In fact our components extract all the time headers from inbound messages like SOAP/JMS/HTTP headers and put them into SwitchYard exchange context and map them back to outbound messages.
I experienced few places where components heavily rely on very specific behaviour - that everything in context is keep during the mediation. That's why we have exchange scope, it's not so good when we talk about IN scope available during sending response. The initiator of communication who initialize exchange (gateway let say) have copy of initial message. When he receives response with another message he can check. Initiator will have access to both. If I see things correctly (please correct me if I don't), only one place where we basically have two messages is service provider. But provider is the place where second (outgoing) message is usually created. If provider is not called then we had fault thrown before, because inbound message was incorrect. After calling provider inbound message dissapear and we have only outgoing message. We still keep IN properties, I would say - "just in case" when somebody would like to use them.
The places where IN and OUT properties are used at the same time are tests. However, even with some tests marked as ignored quickstart tests fails. The tests I had to disable now:
- org.switchyard.component.http.HttpGatewayTest - it was failing but after placing few additional if conditions in camel exchange bus works
The additional if conditions were necessary because of constructions like this:
exchange.getContext().setProperty("someProp", "xyz", Scope.OUT); exchange.send(exchange.createMessage());
What happens from camel point of view - we placed header "someProp" in message already attached to camel exchange and send some else with no headers since create message returns new message instance. This is not anybody fault. It's because differences in API. Am I afraid that we will have big troubles with keeping both working well together. Even if we think about changes I've made in terms of SwitchYard 2.0 and Camel 3.0 I don't expect that message headers will go out in Camel 3.0 and "hole" between APIs will remain. As said before, it's de facto how messages in 70% of cases looks like (HTTP/REST, SOAP, JMS).
The problem of "message rolling" in Camel (it's my name for this behavior) that OUT message from one processor becomes IN message for another hits really only Provider and Consumer callback processor and can be easily walked around by keeping copy of camel exchange in message attached to exchange.
If we would like to get SwitchYard exchange mapped to Camel Exchange in 1.x release we have following options:
- Force particular order of calls on exchange:
- first create message
- then set properties
- send message
- Change IN and OUT scope to MESSAGE scope
The second option is more complicated, however it have big benefit compared to first one. User implementing own handler will never do mistake, or will make it much more intentionally. Whats your thoughts about that?
I've been thinking about this a bit since the F2F and I wonder if something like the following would be the best solution. The idea behind this is to make the mapping cleaner and to allow provider implementations to get and set properties.
1) Replace Scope.IN and Scope.OUT with Scope.MESSAGE.
2) Add a getContext() to Message and Exchange.getContenxt(Scope.MESSAGE) would retrieve getMessage().getContext().
This comes with some restrictions to maintain the control we wanted with Scope.IN and Scope.OUT:
1) Message instances can not be sent again. We need an internal field which marks messages as sent and we need to check this when someone attempts to Exchange.send() with a message.
2) To be a bit more friendly about 1, we should add a copy method to Message which performs a shallow copy of the message *excluding* context properties. If the user wants to copy properties, that's a deliberate step on there part and it's easy enough to do via the Context API.
Finally, I'm thinking about dovetailing the above changes with an Exchange API change we've been talking about for a while. I would like to change send() in Exchange into something like invoke() and it should return a reply message for InOut. Maybe something like:
Future<Message> invoke(Message msg)
In the initial implementation, the behavior of this will be identical to (and replace the use of) SynchronousInOutHandler, which is all over the place right now.
With the possible exception of the Scope.IN and Scope.OUT change, this should have no impact on SY applications since this is all core and component-level code. The impact of Scope should be very limited on app projects.