Hi

I think it would be confusing if there is a setFault but no getFault?
But I understand the reason when you want to use OUT for both regular
OUT and FAULT. Where a header determine the type.

As its uncommon to set a fault we could as James suggested have a
factory method on Exchange to create a new one:

Message msg = exchange.createFault();
msg.setBody("Unknown order id " + id);
exchange.setOut(msg);

This could reduce any get/set fault etc. on the Exchange to only the
factory method.

Then what is left is probably a either to
- keep the hasFault on exchange
- or move it to Message and name it isFault()


Then we only get IN and OUT on the Exchange, and the API is reduced.



On Thu, Jul 9, 2009 at 4:35 PM, Hadrian Zbarcea<hzbar...@gmail.com> wrote:
> My intention with this thread is to try to cleanup the api for 2.0.
> Implementation we can fix in any 2.x.
> Although I agree with having a Fault being stored as an out with a flag,
> it's not very relevant right now for me.
>
> As api changes, if I understand correctly, you are suggesting;
>
>   Message getOut();
>   boolean hasOut();
>   void setOut(Message out);
>
>   boolean hasFault();
>   void setFault(boolean value);
>
> These methods go:
>   Message getOut(boolean lazyCreate);
>   Message getFault();
>   Message getFault(boolean lazyCreate);
>   void removeFault();
>
> The equivalency being:
>
>   Message fault = getFault();
> equivalent to:
>   if (hasFault())
>       Message fault = getOut();
>
> or to set the fault instead of the lazyCreate thing:
>   setOut(message);
>   setFault(true);
>
> removeFault() becomes:
>   setOut(null);
>
> I am perfectly fine with this.  It's the responsibility of the caller to
> check if the out is a fault or not, which may or may not be relevant
> depending on the component.  We just need to clearly document that.
>
> +1
> Hadrian
>
>
>
> On Jul 9, 2009, at 6:06 AM, James Strachan wrote:
>
>> A few random thoughts.
>>
>> So we definitely need a way to determine if the output on an exchange
>> is an OUT, fault, exception. We could have an OUT and a Fault Message;
>> or have a single OUT Message with a boolean fault property.
>>
>> We could store an Exception as a body of the OUT maybe, though I can't
>> help feel that an Exception is a bit different to an OUT/Fault (which
>> are messages). e.g. you might want to clear the exception but keep the
>> OUT?
>>
>> To process the output in a pipeline we could do something like...
>>
>>  Throwable e = exchange.getException();
>>  if (e != null) {
>>   // process the exception
>>  }
>>  else {
>>    // we should now have now valid output
>>   Message out = exchange.getOut();
>>   if (out == null) {
>>     // no output created, so reuse input?
>>     out = exchange.getIn();
>>   }
>>
>>   // we might not care if its a fault or out, we might just wanna
>> return it anyway
>>  if (out.isFault()) {
>>    // do some fault specific coding here...
>>  }
>> }
>>
>> So we could use the OUT for a fault or a regular OUT (as you could say
>> its an output message, whether a fault or real response) so code might
>> not care if its a fault or not? So maybe a flag on OUT is neater?
>>
>> Exceptions seem different though; its typically something you'd wanna
>> look at (and might wanna know what the OUT was when the exception
>> threw), so having that as a property on Exchange feels right to me.
>>
>>
>> Main problem seems to be the lazy create stuff. (Bad James!). Maybe we
>> just need OUT to be null and if someone wants to create an OUT there's
>> a factory method...
>>
>> Message out = exchange.createOut();
>>
>> (it could maybe be smart enough that if there's already an OUT defined
>> it returns that one?). Once someone calls createOut() then the OUT is
>> set and getOut() returns a non null value?
>>
>> Then if its a fault...
>>
>> out.setFault(true);
>>
>>
>> Then if folks call exchange.getOut() it will typically return null and
>> not do any lazy creation or gobble up messages etc as Claus says?
>>
>>
>> 2009/7/9 Claus Ibsen <claus.ib...@gmail.com>:
>>>
>>> On Thu, Jul 9, 2009 at 9:03 AM, Guillaume Nodet<gno...@gmail.com> wrote:
>>>>
>>>> In web services world, faults are not exceptions, but usually an xml
>>>> payload.  In the java world, faults would be like checked exceptions
>>>> and errors runtime exceptions.  However, distinguishing a fault from
>>>> an out message by (instanceof Exception) is imho not sufficient
>>>
>>> Yeah I was about to say the same. I think the FAULT message makes
>>> sense. Fits better with the web service if you have declared faults in
>>> the wsdl.
>>> Then you can construct a XML message and set it as getFault().setBody().
>>>
>>> And I would also think it is much more confusing to set a fault
>>> message, wrapped as an exception on the OUT message as opposed to use
>>> setException on the exchange instead. That would be odd.
>>>
>>> So the API is good, my only griefs are
>>> a) the OUT creating empty messages.
>>> b) and that people might think as if they can during routing processes
>>> piece by piece assemble an OUT message.
>>>
>>> a)
>>> See previous mails about this.
>>>
>>> b)
>>> An example to illustrate this. For instance in the route below A, B, C
>>> is independent steps that enrich a message with order details.
>>> Suppose the input message is an order id. And the expected message is
>>> details about this order.
>>>
>>> from("direct:start").process(A).process(B).process(C);
>>>
>>> from
>>> IN = 123
>>> OUT = null
>>>
>>> A
>>> IN = 123
>>> OUT = Orderid: 123.
>>>
>>> B
>>> IN = 123
>>> OUT = Orderid: 123. Status = IN PROGRESS
>>>
>>> C
>>> IN = 123
>>> OUT = Orderid: 123. Status = IN PROGRESS. ETA: 2009/08/13
>>>
>>> Client receives
>>> OUT: Orderid: 123. Status = IN PROGRESS. ETA: 2009/08/13
>>>
>>>
>>> But the nature of pipes and filter, this is what happens
>>>
>>> from
>>> IN = 123
>>> OUT = null
>>>
>>> A
>>> IN = 123
>>> OUT = Orderid: 123.
>>>
>>> B
>>> IN = Orderid: 123.
>>> OUT = null
>>>
>>> Kabom!!! now we got a partly OUT message as IN instead of the original
>>> IN message.
>>>
>>> This is right according to the pipes and filters, where previous OUT
>>> should be next IN.
>>>
>>>
>>> But people should just be aware with this in relation to IN and OUT -
>>> that the OUT is not something that you can piece by piece assemble.
>>> And OUT is really not that useable.
>>>
>>>
>>>
>>>
>>>>
>>>> On Thu, Jul 9, 2009 at 07:35, Hadrian Zbarcea<hzbar...@gmail.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Comments inline.
>>>>>
>>>>> Hadrian
>>>>>
>>>>> On Jul 9, 2009, at 12:54 AM, Claus Ibsen wrote:
>>>>>
>>>>>> On Wed, Jul 8, 2009 at 9:52 PM, Hadrian Zbarcea<hzbar...@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> As we approach the 2.0, there is one more hanging issue I would like
>>>>>>> addressed, if possible.  It's the thorny issue of Faults and
>>>>>>> Exceptions
>>>>>>> that
>>>>>>> started in
>>>>>>> http://issues.apache.org/activemq/browse/CAMEL-316 (see also the
>>>>>>> related
>>>>>>> nabble thread linked in the issue description).
>>>>>>>
>>>>>>> I am less concerned about how the DefaultExchange is implemented and
>>>>>>> I
>>>>>>> hope
>>>>>>> to reach an agreement on what the Exchange api should be (please find
>>>>>>> the
>>>>>>> list of Exchange methods below).
>>>>>>>
>>>>>>> As far as faults/exceptions are concerned, Roman thinks that the
>>>>>>> whole
>>>>>>> concept of in/out/fault/exception is artificial, and only one payload
>>>>>>> (message) api should be enough (Roman please correct me if I
>>>>>>> misinterpret
>>>>>>> your position).  My opinion is that we *must* distinguish between
>>>>>>> persistent
>>>>>>> (fault) and transient (exception) errors for the simple reason that
>>>>>>> they
>>>>>>> have different semantics.  As Roman correctly points out, faults are
>>>>>>> more
>>>>>>> like outputs, have more of application level semantics and are
>>>>>>> normally
>>>>>>> handled by the client, where exceptions (transient errors) are
>>>>>>> something
>>>>>>> camel could try to recover from, without much knowledge about the
>>>>>>> application.  I think that the presence of fault in the camel api is
>>>>>>> not
>>>>>>> due
>>>>>>> to it being explicitly modeled by jbi and wsdl, as Roman suggests,
>>>>>>> and
>>>>>>> Camel
>>>>>>> simply copying that, but it's modeled in Camel for the same reason
>>>>>>> it's
>>>>>>> modeled in jbi and wsdl, to differentiate transient from persistent
>>>>>>> errors
>>>>>>> in a non ambiguous way.
>>>>>>
>>>>>> I am one of the persons that would love the Camel Exchange / Message
>>>>>> API to be a bit simpler. It has a fair shares of methods.
>>>>>>
>>>>>> Having listening and discussing with Hadrian on this and doing my own
>>>>>> investigations and whatnot I do belive that Hadrian is absolutely
>>>>>> right when it comes to FAULT. It has a good place in the API.  I am +1
>>>>>> on having FAULT as we do now.
>>>>>>
>>>>>> The grief I have left is that the IN and OUT. It makes sense to have
>>>>>> them and they provide a good value. However they have a big drawnback
>>>>>> in how they are routed in Camel with the Pipeline processor, that
>>>>>> mimics the pipes and filters EIP. And as a result the OUT will be used
>>>>>> as IN
>>>>>> in the next step in the route. So its not like you can steadily build
>>>>>> up an OUT message on-the-fly during many steps in the route path.
>>>>>>
>>>>>> Example
>>>>>> from("direct:start").process(new Processor()).to("log:foo");
>>>>>>
>>>>>> a) From
>>>>>> IN = Hello World
>>>>>> OUT = null
>>>>>>
>>>>>> b) Processor
>>>>>> IN Hello World
>>>>>> OUT = Bye World
>>>>>>
>>>>>> c) Log
>>>>>> IN = Bye World
>>>>>> OUT = null
>>>>>>
>>>>> Yes, from an external observer's perspective, this is precisely what
>>>>> happens.  How we decide to store it, how many fields we need, is an
>>>>> implementation detail of the DefaultExchange.  I don't think the copy
>>>>> from
>>>>> out/in is too expensive, but I would be ok with having only one field
>>>>> to
>>>>> store the current message in DefaultExchange (I assume that's what you
>>>>> propose).  However, my question is about what the api should be.
>>>>>
>>>>>
>>>>>>
>>>>>> And then the getOut() method that lazy creates a new empty OUT message
>>>>>> is also a pita, as it can lead to people loosing their messages if
>>>>>> they do some System out logging of their own
>>>>>>
>>>>>> public void process(Exchange e) {
>>>>>>  System.out.println(exchange.getIn());
>>>>>>  System.out.println(exchange.getOut());
>>>>>>  // boom you lost your message when its routed to next node in route
>>>>>> path, as getOut() created a new empty OUT message that will by used in
>>>>>> the pipes and filters EIP routed with the Pipeline
>>>>>> }
>>>>>>
>>>>>> We had this IN OUT discussion a while back and at that time we ended
>>>>>> up with a compromise of having a hasOut() method so you should do, to
>>>>>> be safe:
>>>>>>  System.out.println(exchange.getIn());
>>>>>>  if (exchange.hasOut()) {
>>>>>>     System.out.println(exchange.getOut());
>>>>>>  }
>>>>>>
>>>>>> Still a pita with the lazy creation IMHO.
>>>>>
>>>>> The lazyCreate methods are actually deprecated, and imho should be
>>>>> removed
>>>>> now.  This would eliminate the confusion.
>>>>>
>>>>>
>>>>>>>
>>>>>>> If we were to go with only get/setMessage() api, we would still need
>>>>>>> methods
>>>>>>> (or some ways) to distinguish between the kind of message we are
>>>>>>> dealing
>>>>>>> with (in/out/fault/exception) so we'd only move the problem somewhere
>>>>>>> else.
>>>>>>>
>>>>>>> So the question becomes if we leave the api the way it is, or we
>>>>>>> replace
>>>>>>> the
>>>>>>> get/setFault apis with get/setOut, in which case we'll need something
>>>>>>> like:
>>>>>>>  boolean isFault();
>>>>>>> method in the Message api or keep the hasFault() method on the
>>>>>>> Exchange.
>>>>>>
>>>>>> Good question
>>>>>>
>>>>>> If you use OUT instead then we need to add a isFault() on the
>>>>>> org.apache.camel.Message API that
>>>>>> the IN message also implements.
>>>>>>
>>>>>> It could make sense to use OUT as well for FAULT.
>>>>>> But how should the API look like to set an OUT as Fault?
>>>>>>
>>>>>> Something a like this?
>>>>>>
>>>>>> getOut().setBody("Unknown bank account number.");
>>>>>> getOut().setFault(true);
>>>>>
>>>>> Not quite, I had something like this in mind (there is no setFault()
>>>>> method):
>>>>> getOut().setBody(java-lang-Exception-derivedObject);
>>>>>
>>>>> boolean isFault() {
>>>>>   return getBody() instanceof Exception;
>>>>> }
>>>>>
>>>>> Personally I am ok with the limitation not being able to have an out of
>>>>> a
>>>>> java.lang.Exception type (that would then be a Fault).  I can't imagine
>>>>> a
>>>>> case where an Exception would be an expected out, and in such cases one
>>>>> could always serialize or wrap it.  The fact that the Fault would be an
>>>>> Exception type would be a camel convention that needs to be followed by
>>>>> all
>>>>> components.
>>>>>
>>>>> Another option would be add  header HAS_FAULT or something like that,
>>>>> in
>>>>> which case both the out and the fault could be of any type.
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>>
>>>>>>>  ExchangePattern getPattern();
>>>>>>>  void setPattern(ExchangePattern pattern);
>>>>>>>
>>>>>>>  Object getProperty(String name);
>>>>>>>  <T> T getProperty(String name, Class<T> type);
>>>>>>>  void setProperty(String name, Object value);
>>>>>>>  Object removeProperty(String name);
>>>>>>>  Map<String, Object> getProperties();
>>>>>>>
>>>>>>>  Message getIn();
>>>>>>>  void setIn(Message in);
>>>>>>>
>>>>>>>  Message getOut();
>>>>>>>  boolean hasOut();
>>>>>>>  Message getOut(boolean lazyCreate);
>>>>>>>  void setOut(Message out);
>>>>>>>
>>>>>>>  Message getFault();
>>>>>>>  boolean hasFault();
>>>>>>>  Message getFault(boolean lazyCreate);
>>>>>>>  void removeFault();
>>>>>>> // removeFault() is only used in one place
>>>>>>
>>>>>> +1 to remove it. You can just do setFault(null) instead. I will fix it
>>>>>> asap.
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>  Exception getException();
>>>>>>>  <T> T getException(Class<T> type);
>>>>>>>  void setException(Exception e);
>>>>>>>  boolean isFailed();
>>>>>>>
>>>>>>>  boolean isTransacted();
>>>>>>>  boolean isRollbackOnly();
>>>>>>>
>>>>>>>  CamelContext getContext();
>>>>>>>
>>>>>>>  Exchange newInstance();
>>>>>>>  Exchange copy();
>>>>>>>  Exchange newCopy(boolean handoverOnCompletion);
>>>>>>>  void copyFrom(Exchange source);
>>>>>>>
>>>>>>>  Endpoint getFromEndpoint();
>>>>>>>  void setFromEndpoint(Endpoint fromEndpoint);
>>>>>>>
>>>>>>>  UnitOfWork getUnitOfWork();
>>>>>>>  void setUnitOfWork(UnitOfWork unitOfWork);
>>>>>>>
>>>>>>>  String getExchangeId();
>>>>>>>  void setExchangeId(String id);
>>>>>>>
>>>>>>>  void addOnCompletion(Synchronization onCompletion);
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Claus Ibsen
>>>>>> Apache Camel Committer
>>>>>>
>>>>>> Open Source Integration: http://fusesource.com
>>>>>> Blog: http://davsclaus.blogspot.com/
>>>>>> Twitter: http://twitter.com/davsclaus
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Cheers,
>>>> Guillaume Nodet
>>>> ------------------------
>>>> Blog: http://gnodet.blogspot.com/
>>>> ------------------------
>>>> Open Source SOA
>>>> http://fusesource.com
>>>>
>>>
>>>
>>>
>>> --
>>> Claus Ibsen
>>> Apache Camel Committer
>>>
>>> Open Source Integration: http://fusesource.com
>>> Blog: http://davsclaus.blogspot.com/
>>> Twitter: http://twitter.com/davsclaus
>>>
>>
>>
>>
>> --
>> James
>> -------
>> http://macstrac.blogspot.com/
>>
>> Open Source Integration
>> http://fusesource.com/
>
>



-- 
Claus Ibsen
Apache Camel Committer

Open Source Integration: http://fusesource.com
Blog: http://davsclaus.blogspot.com/
Twitter: http://twitter.com/davsclaus

Reply via email to