You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by Thilina Gunarathne <cs...@gmail.com> on 2008/12/18 04:55:05 UTC
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Hi Andreas,
I'm sorry, I missed this mail.. Saw it only now...
I agree with you regarding the 1. But I guess the solution will need to
address deferred building, which will make it bit complex... Something like
implementing a pushbackInputStream which will directly give the bytes from
the transport inputstream while buffering it to give it the next time...
Regarding 2, I don't think we can call anything "the" right solution for
this. Normally Axis2 uses OMDataSources to carry native data as long as it
can, so that if an entity which knows how to process the native data can
take advantage of it.. Also using the OMSourcedElement, clearly distinguish
the usage of unknown content from other messages...
Regarding the 3, my apologies once again... I was not aware of such a thing
when I wrote the above. IMHO builder should live inside Axis2.. I did this
(and the mime support) as a solution to the issue raised in Synapse. Wonder
why they did not simply use the impl you mentioned.... May be I'm missing
something.. Let's see how we can combine these efforts...
thanks,
Thilina.
1. The class InputStreamDataSource (the one in
> org.apache.axis2.builder.unknowncontent) violates the
> javax.activation.DataSource contract which says for the getInputStream
> method that "a new InputStream object must be returned each time this
> method is called, and [that] the stream must be positioned at the
> beginning of the data." The consequence will be that the message
> produced by UnknownContentBuilder can only be read once. This is a
> serious flaw.
>
> 2. The AXIOM tree produced by UnknownContentBuilder has only two
> nodes: an OMElement and an OMText (with a DataHandler). Using an
> OMSourcedElement/OMDataSource is not justified for this and would
> introduce unnecessary complexity and overhead.
>
> 3. The code in UnknownContentBuilder to a large extend duplicates the
> code in org.apache.axis2.format.BinaryBuilder (in
> axis2-transport-base), which doesn't have problems 1 and 2.
>
> Could you please make a proposal how to improve this?
>
> Regards,
>
> Andreas
>
--
Thilina Gunarathne - http://thilinag.blogspot.com
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Andreas Veithen wrote:
>
> Questions are:
>
> * While TextMessageBuilder and TextMessageFormatter should clearly
> remain optional (i.e. they would only be implemented by builders and
> formatters that deal with text content), MessageFormatterEx could also
> be merged into MessageFormatter. The additional getDataSource method
> can easily be implemented (message formatters are already required to
> return byte[], so returning a DataSource is not difficult). However
> this would break existing MessageFormatter implementations outside of
> Axis2. What do you think?
+1 for merging getDataSource() into MessageFormatter.
> * Is this in scope for Axis2 1.5?
No because Axis2 1.5 is already in progress. This is a breaking change and
hence should not be rushed.
Sanjiva.
--
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/
Blog: http://sanjiva.weerawarana.org/
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Thilina Gunarathne <cs...@gmail.com>.
[Discussion moved to Axis-Dev to avoid cross posting
http://markmail.org/message/i6cq72unknnnvjy5]
On Thu, Dec 18, 2008 at 10:53 AM, Andreas Veithen <andreas.veithen@gmail.com
> wrote:
> Moving this stuff to Axis2 is something I wanted to propose anyway.
> The reason is that it also defines a couple of extension interfaces
> that can be optionally implemented by message builders and formatters:
>
> * MessageFormatterEx: Adds a method to get the content of the message
> as a DataSource (useful for the mail transport).
> * TextMessageBuilder: Defines methods to build a message from a
> java.io.Reader or String (used to handle JMS TextMessages).
> ApplicationXMLBuilder and SOAPBuilder should be enhanced to implement
> this.
> * There should also be a TextMessageFormatter, but this is not yet
> implemented.
>
> Questions are:
>
> * While TextMessageBuilder and TextMessageFormatter should clearly
> remain optional (i.e. they would only be implemented by builders and
> formatters that deal with text content), MessageFormatterEx could also
> be merged into MessageFormatter. The additional getDataSource method
> can easily be implemented (message formatters are already required to
> return byte[], so returning a DataSource is not difficult). However
> this would break existing MessageFormatter implementations outside of
> Axis2. What do you think?
> * Is this in scope for Axis2 1.5?
>
> Andreas
>
> On Thu, Dec 18, 2008 at 15:37, Deepal jayasinghe <de...@gmail.com>
> wrote:
> > +1, I think that is where it belongs.
> >
> > Deepal
> >> Can we please move the BinaryBuilder to axis2 with the rest of the
> >> message builders?
> >>
> >> Sanjiva.
> >>
> >> Thilina Gunarathne wrote:
> >>> Hi Andreas,
> >>> I'm sorry, I missed this mail.. Saw it only now...
> >>>
> >>> I agree with you regarding the 1. But I guess the solution will need
> >>> to address deferred building, which will make it bit complex...
> >>> Something like implementing a pushbackInputStream which will directly
> >>> give the bytes from the transport inputstream while buffering it to
> >>> give it the next time...
> >>>
> >>> Regarding 2, I don't think we can call anything "the" right solution
> >>> for this. Normally Axis2 uses OMDataSources to carry native data as
> >>> long as it can, so that if an entity which knows how to process the
> >>> native data can take advantage of it.. Also using the
> >>> OMSourcedElement, clearly distinguish the usage of unknown content
> >>> from other messages...
> >>>
> >>> Regarding the 3, my apologies once again... I was not aware of such a
> >>> thing when I wrote the above. IMHO builder should live inside Axis2..
> >>> I did this (and the mime support) as a solution to the issue raised
> >>> in Synapse. Wonder why they did not simply use the impl you
> >>> mentioned.... May be I'm missing something.. Let's see how we can
> >>> combine these efforts...
> >>>
> >>> thanks,
> >>> Thilina.
> >>>
> >>> 1. The class InputStreamDataSource (the one in
> >>> org.apache.axis2.builder.unknowncontent) violates the
> >>> javax.activation.DataSource contract which says for the
> >>> getInputStream
> >>> method that "a new InputStream object must be returned each time
> >>> this
> >>> method is called, and [that] the stream must be positioned at the
> >>> beginning of the data." The consequence will be that the message
> >>> produced by UnknownContentBuilder can only be read once. This is a
> >>> serious flaw.
> >>>
> >>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
> >>> nodes: an OMElement and an OMText (with a DataHandler). Using an
> >>> OMSourcedElement/OMDataSource is not justified for this and would
> >>> introduce unnecessary complexity and overhead.
> >>>
> >>>
> >>> 3. The code in UnknownContentBuilder to a large extend duplicates
> >>> the
> >>> code in org.apache.axis2.format.BinaryBuilder (in
> >>> axis2-transport-base), which doesn't have problems 1 and 2.
> >>>
> >>> Could you please make a proposal how to improve this?
> >>>
> >>> Regards,
> >>>
> >>> Andreas
> >>>
> >>>
> >>>
> >>>
> >>> --
> >>> Thilina Gunarathne - http://thilinag.blogspot.com
> >>
> >>
> >
> >
> > --
> > Thank you!
> >
> >
> > http://blogs.deepal.org
> > http://deepal.org
> >
> >
>
--
Thilina Gunarathne - http://thilinag.blogspot.com
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Andreas Veithen wrote:
>
> Questions are:
>
> * While TextMessageBuilder and TextMessageFormatter should clearly
> remain optional (i.e. they would only be implemented by builders and
> formatters that deal with text content), MessageFormatterEx could also
> be merged into MessageFormatter. The additional getDataSource method
> can easily be implemented (message formatters are already required to
> return byte[], so returning a DataSource is not difficult). However
> this would break existing MessageFormatter implementations outside of
> Axis2. What do you think?
+1 for merging getDataSource() into MessageFormatter.
> * Is this in scope for Axis2 1.5?
No because Axis2 1.5 is already in progress. This is a breaking change and
hence should not be rushed.
Sanjiva.
--
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/
Blog: http://sanjiva.weerawarana.org/
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Andreas Veithen <an...@gmail.com>.
Moving this stuff to Axis2 is something I wanted to propose anyway.
The reason is that it also defines a couple of extension interfaces
that can be optionally implemented by message builders and formatters:
* MessageFormatterEx: Adds a method to get the content of the message
as a DataSource (useful for the mail transport).
* TextMessageBuilder: Defines methods to build a message from a
java.io.Reader or String (used to handle JMS TextMessages).
ApplicationXMLBuilder and SOAPBuilder should be enhanced to implement
this.
* There should also be a TextMessageFormatter, but this is not yet implemented.
Questions are:
* While TextMessageBuilder and TextMessageFormatter should clearly
remain optional (i.e. they would only be implemented by builders and
formatters that deal with text content), MessageFormatterEx could also
be merged into MessageFormatter. The additional getDataSource method
can easily be implemented (message formatters are already required to
return byte[], so returning a DataSource is not difficult). However
this would break existing MessageFormatter implementations outside of
Axis2. What do you think?
* Is this in scope for Axis2 1.5?
Andreas
On Thu, Dec 18, 2008 at 15:37, Deepal jayasinghe <de...@gmail.com> wrote:
> +1, I think that is where it belongs.
>
> Deepal
>> Can we please move the BinaryBuilder to axis2 with the rest of the
>> message builders?
>>
>> Sanjiva.
>>
>> Thilina Gunarathne wrote:
>>> Hi Andreas,
>>> I'm sorry, I missed this mail.. Saw it only now...
>>>
>>> I agree with you regarding the 1. But I guess the solution will need
>>> to address deferred building, which will make it bit complex...
>>> Something like implementing a pushbackInputStream which will directly
>>> give the bytes from the transport inputstream while buffering it to
>>> give it the next time...
>>>
>>> Regarding 2, I don't think we can call anything "the" right solution
>>> for this. Normally Axis2 uses OMDataSources to carry native data as
>>> long as it can, so that if an entity which knows how to process the
>>> native data can take advantage of it.. Also using the
>>> OMSourcedElement, clearly distinguish the usage of unknown content
>>> from other messages...
>>>
>>> Regarding the 3, my apologies once again... I was not aware of such a
>>> thing when I wrote the above. IMHO builder should live inside Axis2..
>>> I did this (and the mime support) as a solution to the issue raised
>>> in Synapse. Wonder why they did not simply use the impl you
>>> mentioned.... May be I'm missing something.. Let's see how we can
>>> combine these efforts...
>>>
>>> thanks,
>>> Thilina.
>>>
>>> 1. The class InputStreamDataSource (the one in
>>> org.apache.axis2.builder.unknowncontent) violates the
>>> javax.activation.DataSource contract which says for the
>>> getInputStream
>>> method that "a new InputStream object must be returned each time
>>> this
>>> method is called, and [that] the stream must be positioned at the
>>> beginning of the data." The consequence will be that the message
>>> produced by UnknownContentBuilder can only be read once. This is a
>>> serious flaw.
>>>
>>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>>> OMSourcedElement/OMDataSource is not justified for this and would
>>> introduce unnecessary complexity and overhead.
>>>
>>>
>>> 3. The code in UnknownContentBuilder to a large extend duplicates
>>> the
>>> code in org.apache.axis2.format.BinaryBuilder (in
>>> axis2-transport-base), which doesn't have problems 1 and 2.
>>>
>>> Could you please make a proposal how to improve this?
>>>
>>> Regards,
>>>
>>> Andreas
>>>
>>>
>>>
>>>
>>> --
>>> Thilina Gunarathne - http://thilinag.blogspot.com
>>
>>
>
>
> --
> Thank you!
>
>
> http://blogs.deepal.org
> http://deepal.org
>
>
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Andreas Veithen <an...@gmail.com>.
Moving this stuff to Axis2 is something I wanted to propose anyway.
The reason is that it also defines a couple of extension interfaces
that can be optionally implemented by message builders and formatters:
* MessageFormatterEx: Adds a method to get the content of the message
as a DataSource (useful for the mail transport).
* TextMessageBuilder: Defines methods to build a message from a
java.io.Reader or String (used to handle JMS TextMessages).
ApplicationXMLBuilder and SOAPBuilder should be enhanced to implement
this.
* There should also be a TextMessageFormatter, but this is not yet implemented.
Questions are:
* While TextMessageBuilder and TextMessageFormatter should clearly
remain optional (i.e. they would only be implemented by builders and
formatters that deal with text content), MessageFormatterEx could also
be merged into MessageFormatter. The additional getDataSource method
can easily be implemented (message formatters are already required to
return byte[], so returning a DataSource is not difficult). However
this would break existing MessageFormatter implementations outside of
Axis2. What do you think?
* Is this in scope for Axis2 1.5?
Andreas
On Thu, Dec 18, 2008 at 15:37, Deepal jayasinghe <de...@gmail.com> wrote:
> +1, I think that is where it belongs.
>
> Deepal
>> Can we please move the BinaryBuilder to axis2 with the rest of the
>> message builders?
>>
>> Sanjiva.
>>
>> Thilina Gunarathne wrote:
>>> Hi Andreas,
>>> I'm sorry, I missed this mail.. Saw it only now...
>>>
>>> I agree with you regarding the 1. But I guess the solution will need
>>> to address deferred building, which will make it bit complex...
>>> Something like implementing a pushbackInputStream which will directly
>>> give the bytes from the transport inputstream while buffering it to
>>> give it the next time...
>>>
>>> Regarding 2, I don't think we can call anything "the" right solution
>>> for this. Normally Axis2 uses OMDataSources to carry native data as
>>> long as it can, so that if an entity which knows how to process the
>>> native data can take advantage of it.. Also using the
>>> OMSourcedElement, clearly distinguish the usage of unknown content
>>> from other messages...
>>>
>>> Regarding the 3, my apologies once again... I was not aware of such a
>>> thing when I wrote the above. IMHO builder should live inside Axis2..
>>> I did this (and the mime support) as a solution to the issue raised
>>> in Synapse. Wonder why they did not simply use the impl you
>>> mentioned.... May be I'm missing something.. Let's see how we can
>>> combine these efforts...
>>>
>>> thanks,
>>> Thilina.
>>>
>>> 1. The class InputStreamDataSource (the one in
>>> org.apache.axis2.builder.unknowncontent) violates the
>>> javax.activation.DataSource contract which says for the
>>> getInputStream
>>> method that "a new InputStream object must be returned each time
>>> this
>>> method is called, and [that] the stream must be positioned at the
>>> beginning of the data." The consequence will be that the message
>>> produced by UnknownContentBuilder can only be read once. This is a
>>> serious flaw.
>>>
>>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>>> OMSourcedElement/OMDataSource is not justified for this and would
>>> introduce unnecessary complexity and overhead.
>>>
>>>
>>> 3. The code in UnknownContentBuilder to a large extend duplicates
>>> the
>>> code in org.apache.axis2.format.BinaryBuilder (in
>>> axis2-transport-base), which doesn't have problems 1 and 2.
>>>
>>> Could you please make a proposal how to improve this?
>>>
>>> Regards,
>>>
>>> Andreas
>>>
>>>
>>>
>>>
>>> --
>>> Thilina Gunarathne - http://thilinag.blogspot.com
>>
>>
>
>
> --
> Thank you!
>
>
> http://blogs.deepal.org
> http://deepal.org
>
>
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Deepal jayasinghe <de...@gmail.com>.
+1, I think that is where it belongs.
Deepal
> Can we please move the BinaryBuilder to axis2 with the rest of the
> message builders?
>
> Sanjiva.
>
> Thilina Gunarathne wrote:
>> Hi Andreas,
>> I'm sorry, I missed this mail.. Saw it only now...
>>
>> I agree with you regarding the 1. But I guess the solution will need
>> to address deferred building, which will make it bit complex...
>> Something like implementing a pushbackInputStream which will directly
>> give the bytes from the transport inputstream while buffering it to
>> give it the next time...
>>
>> Regarding 2, I don't think we can call anything "the" right solution
>> for this. Normally Axis2 uses OMDataSources to carry native data as
>> long as it can, so that if an entity which knows how to process the
>> native data can take advantage of it.. Also using the
>> OMSourcedElement, clearly distinguish the usage of unknown content
>> from other messages...
>>
>> Regarding the 3, my apologies once again... I was not aware of such a
>> thing when I wrote the above. IMHO builder should live inside Axis2..
>> I did this (and the mime support) as a solution to the issue raised
>> in Synapse. Wonder why they did not simply use the impl you
>> mentioned.... May be I'm missing something.. Let's see how we can
>> combine these efforts...
>>
>> thanks,
>> Thilina.
>>
>> 1. The class InputStreamDataSource (the one in
>> org.apache.axis2.builder.unknowncontent) violates the
>> javax.activation.DataSource contract which says for the
>> getInputStream
>> method that "a new InputStream object must be returned each time
>> this
>> method is called, and [that] the stream must be positioned at the
>> beginning of the data." The consequence will be that the message
>> produced by UnknownContentBuilder can only be read once. This is a
>> serious flaw.
>>
>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>> OMSourcedElement/OMDataSource is not justified for this and would
>> introduce unnecessary complexity and overhead.
>>
>>
>> 3. The code in UnknownContentBuilder to a large extend duplicates
>> the
>> code in org.apache.axis2.format.BinaryBuilder (in
>> axis2-transport-base), which doesn't have problems 1 and 2.
>>
>> Could you please make a proposal how to improve this?
>>
>> Regards,
>>
>> Andreas
>>
>>
>>
>>
>> --
>> Thilina Gunarathne - http://thilinag.blogspot.com
>
>
--
Thank you!
http://blogs.deepal.org
http://deepal.org
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Deepal jayasinghe <de...@gmail.com>.
+1, I think that is where it belongs.
Deepal
> Can we please move the BinaryBuilder to axis2 with the rest of the
> message builders?
>
> Sanjiva.
>
> Thilina Gunarathne wrote:
>> Hi Andreas,
>> I'm sorry, I missed this mail.. Saw it only now...
>>
>> I agree with you regarding the 1. But I guess the solution will need
>> to address deferred building, which will make it bit complex...
>> Something like implementing a pushbackInputStream which will directly
>> give the bytes from the transport inputstream while buffering it to
>> give it the next time...
>>
>> Regarding 2, I don't think we can call anything "the" right solution
>> for this. Normally Axis2 uses OMDataSources to carry native data as
>> long as it can, so that if an entity which knows how to process the
>> native data can take advantage of it.. Also using the
>> OMSourcedElement, clearly distinguish the usage of unknown content
>> from other messages...
>>
>> Regarding the 3, my apologies once again... I was not aware of such a
>> thing when I wrote the above. IMHO builder should live inside Axis2..
>> I did this (and the mime support) as a solution to the issue raised
>> in Synapse. Wonder why they did not simply use the impl you
>> mentioned.... May be I'm missing something.. Let's see how we can
>> combine these efforts...
>>
>> thanks,
>> Thilina.
>>
>> 1. The class InputStreamDataSource (the one in
>> org.apache.axis2.builder.unknowncontent) violates the
>> javax.activation.DataSource contract which says for the
>> getInputStream
>> method that "a new InputStream object must be returned each time
>> this
>> method is called, and [that] the stream must be positioned at the
>> beginning of the data." The consequence will be that the message
>> produced by UnknownContentBuilder can only be read once. This is a
>> serious flaw.
>>
>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>> OMSourcedElement/OMDataSource is not justified for this and would
>> introduce unnecessary complexity and overhead.
>>
>>
>> 3. The code in UnknownContentBuilder to a large extend duplicates
>> the
>> code in org.apache.axis2.format.BinaryBuilder (in
>> axis2-transport-base), which doesn't have problems 1 and 2.
>>
>> Could you please make a proposal how to improve this?
>>
>> Regards,
>>
>> Andreas
>>
>>
>>
>>
>> --
>> Thilina Gunarathne - http://thilinag.blogspot.com
>
>
--
Thank you!
http://blogs.deepal.org
http://deepal.org
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Can we please move the BinaryBuilder to axis2 with the rest of the message
builders?
Sanjiva.
Thilina Gunarathne wrote:
> Hi Andreas,
> I'm sorry, I missed this mail.. Saw it only now...
>
> I agree with you regarding the 1. But I guess the solution will need to
> address deferred building, which will make it bit complex... Something
> like implementing a pushbackInputStream which will directly give the
> bytes from the transport inputstream while buffering it to give it the
> next time...
>
> Regarding 2, I don't think we can call anything "the" right solution for
> this. Normally Axis2 uses OMDataSources to carry native data as long as
> it can, so that if an entity which knows how to process the native data
> can take advantage of it.. Also using the OMSourcedElement, clearly
> distinguish the usage of unknown content from other messages...
>
> Regarding the 3, my apologies once again... I was not aware of such a
> thing when I wrote the above. IMHO builder should live inside Axis2.. I
> did this (and the mime support) as a solution to the issue raised in
> Synapse. Wonder why they did not simply use the impl you mentioned....
> May be I'm missing something.. Let's see how we can combine these efforts...
>
> thanks,
> Thilina.
>
> 1. The class InputStreamDataSource (the one in
> org.apache.axis2.builder.unknowncontent) violates the
> javax.activation.DataSource contract which says for the getInputStream
> method that "a new InputStream object must be returned each time this
> method is called, and [that] the stream must be positioned at the
> beginning of the data." The consequence will be that the message
> produced by UnknownContentBuilder can only be read once. This is a
> serious flaw.
>
> 2. The AXIOM tree produced by UnknownContentBuilder has only two
> nodes: an OMElement and an OMText (with a DataHandler). Using an
> OMSourcedElement/OMDataSource is not justified for this and would
> introduce unnecessary complexity and overhead.
>
>
>
> 3. The code in UnknownContentBuilder to a large extend duplicates the
> code in org.apache.axis2.format.BinaryBuilder (in
> axis2-transport-base), which doesn't have problems 1 and 2.
>
> Could you please make a proposal how to improve this?
>
> Regards,
>
> Andreas
>
>
>
>
> --
> Thilina Gunarathne - http://thilinag.blogspot.com
--
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/
Blog: http://sanjiva.weerawarana.org/
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Can we please move the BinaryBuilder to axis2 with the rest of the message
builders?
Sanjiva.
Thilina Gunarathne wrote:
> Hi Andreas,
> I'm sorry, I missed this mail.. Saw it only now...
>
> I agree with you regarding the 1. But I guess the solution will need to
> address deferred building, which will make it bit complex... Something
> like implementing a pushbackInputStream which will directly give the
> bytes from the transport inputstream while buffering it to give it the
> next time...
>
> Regarding 2, I don't think we can call anything "the" right solution for
> this. Normally Axis2 uses OMDataSources to carry native data as long as
> it can, so that if an entity which knows how to process the native data
> can take advantage of it.. Also using the OMSourcedElement, clearly
> distinguish the usage of unknown content from other messages...
>
> Regarding the 3, my apologies once again... I was not aware of such a
> thing when I wrote the above. IMHO builder should live inside Axis2.. I
> did this (and the mime support) as a solution to the issue raised in
> Synapse. Wonder why they did not simply use the impl you mentioned....
> May be I'm missing something.. Let's see how we can combine these efforts...
>
> thanks,
> Thilina.
>
> 1. The class InputStreamDataSource (the one in
> org.apache.axis2.builder.unknowncontent) violates the
> javax.activation.DataSource contract which says for the getInputStream
> method that "a new InputStream object must be returned each time this
> method is called, and [that] the stream must be positioned at the
> beginning of the data." The consequence will be that the message
> produced by UnknownContentBuilder can only be read once. This is a
> serious flaw.
>
> 2. The AXIOM tree produced by UnknownContentBuilder has only two
> nodes: an OMElement and an OMText (with a DataHandler). Using an
> OMSourcedElement/OMDataSource is not justified for this and would
> introduce unnecessary complexity and overhead.
>
>
>
> 3. The code in UnknownContentBuilder to a large extend duplicates the
> code in org.apache.axis2.format.BinaryBuilder (in
> axis2-transport-base), which doesn't have problems 1 and 2.
>
> Could you please make a proposal how to improve this?
>
> Regards,
>
> Andreas
>
>
>
>
> --
> Thilina Gunarathne - http://thilinag.blogspot.com
--
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/
Blog: http://sanjiva.weerawarana.org/
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Andreas Veithen <an...@gmail.com>.
See my comments inline.
On Thu, Dec 18, 2008 at 04:55, Thilina Gunarathne <cs...@gmail.com> wrote:
> Hi Andreas,
> I'm sorry, I missed this mail.. Saw it only now...
>
> I agree with you regarding the 1. But I guess the solution will need to
> address deferred building, which will make it bit complex... Something like
> implementing a pushbackInputStream which will directly give the bytes from
> the transport inputstream while buffering it to give it the next time...
The existing BinaryBuilder uses IOUtils#toByteArray and then
constructs a DataSource from the byte[]. This avoids the problem that
UnknownContentBuilder has, but this solution is also clearly
suboptimal. Your idea sounds very good. I'm sure that a DataSource
that implements this behavior already exists somewhere in Axiom, Axis2
or Synapse.
> Regarding 2, I don't think we can call anything "the" right solution for
> this. Normally Axis2 uses OMDataSources to carry native data as long as it
> can, so that if an entity which knows how to process the native data can
> take advantage of it.. Also using the OMSourcedElement, clearly distinguish
> the usage of unknown content from other messages...
In the case of binary data, this functionality as already provided by
DataHandler/DataSource. UnknownContentOMDataSource only defers the
construction of the wrapper OMElement and the OMText object that holds
the reference to the DataHandler, which IMHO is not very useful.
Actually your argument applies much more to PlainTextBuilder, which
for the moment uses IOUtils.toString(inputStream, charSetEnc) and then
creates an OMText node with the returned string. Here it would
definitely make sense to defer building the tree as much as possible.
Note that we have code in Synapse that does this (for text output
generated by an XSL transformation). See TextFileDataSource and
WrappedTextNodeStreamReader.
> Regarding the 3, my apologies once again... I was not aware of such a thing
> when I wrote the above. IMHO builder should live inside Axis2.. I did this
> (and the mime support) as a solution to the issue raised in Synapse. Wonder
> why they did not simply use the impl you mentioned.... May be I'm missing
> something.. Let's see how we can combine these efforts...
No need to apologize. With the large volume of code in Axis2 and its
surrounding projects, probably nobody is able to say whether a given
piece of functionality already exists somewhere or not. After all
thats why we spam the dev lists with the Subversion commit logs, so
that people can review and shout when code is duplicated...
> thanks,
> Thilina.
>
>> 1. The class InputStreamDataSource (the one in
>> org.apache.axis2.builder.unknowncontent) violates the
>> javax.activation.DataSource contract which says for the getInputStream
>> method that "a new InputStream object must be returned each time this
>> method is called, and [that] the stream must be positioned at the
>> beginning of the data." The consequence will be that the message
>> produced by UnknownContentBuilder can only be read once. This is a
>> serious flaw.
>>
>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>> OMSourcedElement/OMDataSource is not justified for this and would
>> introduce unnecessary complexity and overhead.
>>
>> 3. The code in UnknownContentBuilder to a large extend duplicates the
>> code in org.apache.axis2.format.BinaryBuilder (in
>> axis2-transport-base), which doesn't have problems 1 and 2.
>>
>> Could you please make a proposal how to improve this?
>>
>> Regards,
>>
>> Andreas
>
>
>
> --
> Thilina Gunarathne - http://thilinag.blogspot.com
>
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Thilina Gunarathne <cs...@gmail.com>.
oh...BTW in middle of all this I forgot to thank you for reviewing the
commit and catching the issues...:)
On Wed, Dec 17, 2008 at 10:55 PM, Thilina Gunarathne <cs...@gmail.com>wrote:
> Hi Andreas,
> I'm sorry, I missed this mail.. Saw it only now...
>
> I agree with you regarding the 1. But I guess the solution will need to
> address deferred building, which will make it bit complex... Something like
> implementing a pushbackInputStream which will directly give the bytes from
> the transport inputstream while buffering it to give it the next time...
>
> Regarding 2, I don't think we can call anything "the" right solution for
> this. Normally Axis2 uses OMDataSources to carry native data as long as it
> can, so that if an entity which knows how to process the native data can
> take advantage of it.. Also using the OMSourcedElement, clearly distinguish
> the usage of unknown content from other messages...
>
> Regarding the 3, my apologies once again... I was not aware of such a thing
> when I wrote the above. IMHO builder should live inside Axis2.. I did this
> (and the mime support) as a solution to the issue raised in Synapse. Wonder
> why they did not simply use the impl you mentioned.... May be I'm missing
> something.. Let's see how we can combine these efforts...
>
> thanks,
> Thilina.
>
>
> 1. The class InputStreamDataSource (the one in
>> org.apache.axis2.builder.unknowncontent) violates the
>> javax.activation.DataSource contract which says for the getInputStream
>> method that "a new InputStream object must be returned each time this
>> method is called, and [that] the stream must be positioned at the
>> beginning of the data." The consequence will be that the message
>> produced by UnknownContentBuilder can only be read once. This is a
>> serious flaw.
>>
>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>> OMSourcedElement/OMDataSource is not justified for this and would
>> introduce unnecessary complexity and overhead.
>
>
>>
>> 3. The code in UnknownContentBuilder to a large extend duplicates the
>> code in org.apache.axis2.format.BinaryBuilder (in
>> axis2-transport-base), which doesn't have problems 1 and 2.
>>
>> Could you please make a proposal how to improve this?
>>
>> Regards,
>>
>> Andreas
>>
>
>
>
> --
> Thilina Gunarathne - http://thilinag.blogspot.com
>
--
Thilina Gunarathne - http://thilinag.blogspot.com
Re: Comments about the new UnknownContentBuilder (AXIS2-4153)
Posted by Thilina Gunarathne <cs...@gmail.com>.
oh...BTW in middle of all this I forgot to thank you for reviewing the
commit and catching the issues...:)
On Wed, Dec 17, 2008 at 10:55 PM, Thilina Gunarathne <cs...@gmail.com>wrote:
> Hi Andreas,
> I'm sorry, I missed this mail.. Saw it only now...
>
> I agree with you regarding the 1. But I guess the solution will need to
> address deferred building, which will make it bit complex... Something like
> implementing a pushbackInputStream which will directly give the bytes from
> the transport inputstream while buffering it to give it the next time...
>
> Regarding 2, I don't think we can call anything "the" right solution for
> this. Normally Axis2 uses OMDataSources to carry native data as long as it
> can, so that if an entity which knows how to process the native data can
> take advantage of it.. Also using the OMSourcedElement, clearly distinguish
> the usage of unknown content from other messages...
>
> Regarding the 3, my apologies once again... I was not aware of such a thing
> when I wrote the above. IMHO builder should live inside Axis2.. I did this
> (and the mime support) as a solution to the issue raised in Synapse. Wonder
> why they did not simply use the impl you mentioned.... May be I'm missing
> something.. Let's see how we can combine these efforts...
>
> thanks,
> Thilina.
>
>
> 1. The class InputStreamDataSource (the one in
>> org.apache.axis2.builder.unknowncontent) violates the
>> javax.activation.DataSource contract which says for the getInputStream
>> method that "a new InputStream object must be returned each time this
>> method is called, and [that] the stream must be positioned at the
>> beginning of the data." The consequence will be that the message
>> produced by UnknownContentBuilder can only be read once. This is a
>> serious flaw.
>>
>> 2. The AXIOM tree produced by UnknownContentBuilder has only two
>> nodes: an OMElement and an OMText (with a DataHandler). Using an
>> OMSourcedElement/OMDataSource is not justified for this and would
>> introduce unnecessary complexity and overhead.
>
>
>>
>> 3. The code in UnknownContentBuilder to a large extend duplicates the
>> code in org.apache.axis2.format.BinaryBuilder (in
>> axis2-transport-base), which doesn't have problems 1 and 2.
>>
>> Could you please make a proposal how to improve this?
>>
>> Regards,
>>
>> Andreas
>>
>
>
>
> --
> Thilina Gunarathne - http://thilinag.blogspot.com
>
--
Thilina Gunarathne - http://thilinag.blogspot.com