Hi Roman,
Interesting points. From what I gather there are two questions you
raise:
1. Is the distinction between in/out/fault/exception necessary
2. Is the DSL clear or at least intuitive enough
On the second point, I think there are a few places that require a bit
of a revisit.
On the first point, my personal take is that the distinction is
important and should be there. There are influences from the WSDL
spec(s) and I think there is a reason for that.
Inputs are sort of read-only from the point of view of the next
processor in the pipeline. Outputs provide a response after
successful processing. Faults are responses that indicate to the
client that the expected goal was not achieved, and they usually
provide a reason. Faults are messages that tell a client that the
server did receive the request and successfully processed it, but not
in the way intended and the client may need to take further action.
Exceptions on the other hand indicate that the message was not
processed successfully, and the client should not make any
assumptions. The way the semantics of the result (output/fault/
exception) is sent back to the client is dependent on the component.
So my take would be to look at the dsl and see if/when the constructs
fail to model properly the eip pattern they represent and address that.
My $0.02,
Hadrian
On Jan 29, 2008, at 5:45 PM, Roman Kalukiewicz wrote:
Hello!
I would like to resurrect the discussion that I started on this list
over a month ago, and if you want to see the roots, you can go here (I
don't want to repeat myself ;) ):
http://www.nabble.com/in-out-fault-messages-discussion-to14170013s22882.html#a14170013
Summarizing my point of view: I think, that distinction between
in/out/fault messages is not needed and that it makes more problems,
than it solves.
To actually check it I have prepared very small PoC that simply
removes this distinction without changing general interface we have.
The patch is attached at the very end of this message if someone is
interested.
To make it work I had to change only two tests in camel-core (also in
the patch) and everything passes. Moreover such patch automatically
eliminates some problems that we faced with to early initialization of
out messages (getOut() invocation) that caused to loose your body,
some strange behaviours of some endpoints that depend on out message
being set like
from("jetty:http://localhost:1234/test";).setBody("response"); <--
doesn't work
from("jetty:http://localhost:1234/
test").setBody("response").setBody("response");
<-- works
(in the second case pipeline creates out message, while the first one
doesn't have a pipeline so there is no out message set)
Basically it proves (I think) that this distinction (in/out/fault) is
not needed at all, and it would simplify the solution if we get rid of
it. Moreover our solution gets more powerful, as we can think of
defining predicates that stop the pipeline, so it doesn't stop simply
on fault (but by default we could stop on 'fault' header set).
Of course there are components will require some changes if we go this
way - The first one I can think about is servicemix JBI component that
sets in, out, and fault messages at the very beginning of the flow.
Moreover I found another problem with JMS message that simply ignores
setBody() invocations when there is JMSMessage associated with the
message. (JIRA issue should be created I think)
There was another problem with one of jetty tests that was reading the
request stream in the mock endpoint once, and was trying to read it
another time when was creating the response (as now body was not
changed).
Anyway all those problems are pretty small and we can solve them
quickly if we would like to.
Once again - I know that the change I would like to propose is a huge
modification of basic interfaces, and I have really no idea if it
could be introduced.
Roman
Index: src/main/java/org/apache/camel/impl/DefaultExchange.java
===================================================================
--- src/main/java/org/apache/camel/impl/DefaultExchange.java (wersja
616530)
+++ src/main/java/org/apache/camel/impl/DefaultExchange.java (kopia
robocza)
@@ -16,6 +16,9 @@
*/
package org.apache.camel.impl;
+import java.util.HashMap;
+import java.util.Map;
+
import org.apache.camel.CamelContext;
import org.apache.camel.Exchange;
import org.apache.camel.ExchangePattern;
@@ -24,9 +27,6 @@
import org.apache.camel.spi.UnitOfWork;
import org.apache.camel.util.UuidGenerator;
-import java.util.HashMap;
-import java.util.Map;
-
/**
* A default implementation of [EMAIL PROTECTED] Exchange}
*
@@ -37,8 +37,8 @@
protected final CamelContext context;
private Map<String, Object> properties;
private Message in;
- private Message out;
- private Message fault;
+// private Message out;
+// private Message fault;
private Throwable exception;
private String exchangeId =
DefaultExchange.DEFAULT_ID_GENERATOR.generateId();
private UnitOfWork unitOfWork;
@@ -174,15 +174,15 @@
}
public Message getOut(boolean lazyCreate) {
- if (out == null && lazyCreate) {
- out = createOutMessage();
- configureMessage(out);
- }
- return out;
+ if (lazyCreate) {
+ in.removeHeader("fault");
+ return in;
+ }
+ return (in != null && in.getHeader("fault") == null) ? in :
null;
}
public void setOut(Message out) {
- this.out = out;
+ this.in = out;
configureMessage(out);
}
@@ -220,15 +220,16 @@
}
public Message getFault(boolean lazyCreate) {
- if (fault == null && lazyCreate) {
- fault = createFaultMessage();
- configureMessage(fault);
+ if (lazyCreate) {
+ in.setHeader("fault", true);
+ return in;
}
- return fault;
+ return ((in == null) || in.getHeader("fault") == null) ?
null : in;
}
public void setFault(Message fault) {
- this.fault = fault;
+ this.in = fault;
+ in.setHeader("fault", true);
configureMessage(fault);
}
Index: src/test/java/org/apache/camel/processor/SplitterTest.java
===================================================================
--- src/test/java/org/apache/camel/processor/SplitterTest.java
(wersja 616530)
+++ src/test/java/org/apache/camel/processor/SplitterTest.java
(kopia robocza)
@@ -80,7 +80,7 @@
}
});
- assertNull(result.getOut(false));
+ assertNull(result.getOut().getBody());
}
protected RouteBuilder createRouteBuilder() {
Index: src/test/java/org/apache/camel/processor/PipelineTest.java
===================================================================
--- src/test/java/org/apache/camel/processor/PipelineTest.java
(wersja 616530)
+++ src/test/java/org/apache/camel/processor/PipelineTest.java
(kopia robocza)
@@ -107,7 +107,9 @@
// Check the out Message.. It should have only been
processed once.
// since the fault should stop it from going to the next
process.
- assertEquals(1, exchange.getOut().getHeader("copy-counter"));
+// We have here out AND fault message at the same time - what
is the meaning
+// of such response??
+// assertEquals(1,
exchange.getOut().getHeader("copy-counter"));
}
public void testOnlyProperties() {
