On Thu, Oct 8, 2009 at 2:15 AM, Andreas Veithen
<[email protected]>wrote:
> -1 for this change.
>
> Reasons:
>
> 1. It breaks the contract of the method in two ways:
>
> - It doesn't work if the XMLStreamReader is non coalescing.
I am not clear how you going to explain this.
In earlier code, when it comes to while loop it has executed
1. reader.next() which is at the if statement
2. reader.getText() which is just above
after this change same thing applies. Now it has the reader.next() at the
top.
> In this
> case it would only read part of the content. Please refer to the
> second part of the code in that method, more precisely the part tagged
> with the comment "Take into account that in non coalescing mode, there
> may be additional CHARACTERS events".
> - Even if the parser is coalescing, the change still violates the
> contract for elements that don't have the expected content, i.e. which
> have child nodes other than text nodes. In this case, the changed
> method neither satisfies the postcondition, nor does it throw an
> exception.
>
Can you please explain the lines of codes which has broken this?
>
> 2. The description of the change is not accurate: "supporting empty
> elements for base64Binary elements." The previous version of the code
> does support empty elements, at least if getElementText conforms to
> the StAX specs.
I think you refer here.
if (dhr == null) {
// since we have advance the reader to next have to use the
// reader.getText
base64 = reader.getText();
reader.next();
}
this code get executed only if dhr is null. that is not the case for this.
try to invoke this service[1] RetByteArray method with an empty datahandler.
i.e. new DataHandler(new ByteArrayDataSource(new byte[0])); with generated
adb code.
[1]
http://131.107.72.15/SoapWsdl_BaseDataTypes_XmlFormatter_Service_Indigo/BaseDataTypesDocLitB.svc?wsdl
Here is the problem.
ADB parse method invoke this method when the current location is at the
start element of this element.
<ns3:inByteArray xmlns:ns3="http://tempuri.org/";></ns3:inByteArray>
this method throws an exception here.
I think this is wrong and it should have return an empty datahandler.
i.e. new DataHandler(new ByteArrayDataSource(new byte[0]))
> I will provide a unit test that shows this.
>
+1. Please provide a unit test case which does work with earlier version and
does not work with the changed
version.
> If there is a problem around this method, then probably the root cause
> is located in a completely different area. There are two things that
> come to my mind:
>
> - Could it be that the XMLStreamReader that is passed to the method
> has an incorrect implementation of getElementText?
> - Could it be that the method is called in the wrong context, e.g. in
> a context where the element represented by the current event may have
> a child element?
>
please try to invoke the service I have given and have a look at.
thanks,
Amila.
>
> Andreas
>
>
> On Wed, Oct 7, 2009 at 16:08, <[email protected]> wrote:
> > Author: amilas
> > Date: Wed Oct 7 14:08:20 2009
> > New Revision: 822747
> >
> > URL: http://svn.apache.org/viewvc?rev=822747&view=rev
> > Log:
> > supporting empty elements for base64Binary elements.
> >
> > Modified:
> >
>
> webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/util/stax/XMLStreamReaderUtils.java
> >
> > Modified:
> webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/util/stax/XMLStreamReaderUtils.java
> > URL:
> http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/util/stax/XMLStreamReaderUtils.java?rev=822747&r1=822746&r2=822747&view=diff
> >
> ==============================================================================
> > ---
> webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/util/stax/XMLStreamReaderUtils.java
> (original)
> > +++
> webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/util/stax/XMLStreamReaderUtils.java
> Wed Oct 7 14:08:20 2009
> > @@ -64,17 +64,26 @@
> > */
> > public static DataHandler getDataHandlerFromElement(XMLStreamReader
> reader)
> > throws XMLStreamException {
> > +
> > + // according to the pre and post conditions it is possible to
> have an
> > + // empty element eg. <ns3:inByteArray xmlns:ns3="
> http://tempuri.org/";></ns3:inByteArray> for empty data handlers
> > + // in that case we return a new data handler.
> > + // This method is used by adb parser we can not return null
> since this element is not null.
> > + reader.next();
> > + if (!reader.hasText()){
> > + DataHandler dataHandler = new DataHandler(new
> ByteArrayDataSource(new byte[0]));
> > + // return from here since reader at the end element
> > + return dataHandler;
> > + }
> >
> > DataHandlerReader dhr = getDataHandlerReader(reader);
> > String base64;
> > if (dhr == null) {
> > - // In this case the best way to get the content of the
> element is using
> > - // the getElementText method
> > - base64 = reader.getElementText();
> > + // since we have advance the reader to next have to use the
> > + // reader.getText
> > + base64 = reader.getText();
> > + reader.next();
> > } else {
> > - if (reader.next() != XMLStreamConstants.CHARACTERS) {
> > - throw new XMLStreamException("Expected a CHARACTER
> event");
> > - }
> > if (dhr.isBinary()) {
> > DataHandler dh = dhr.getDataHandler();
> > reader.next();
> >
> >
> >
>
--
Amila Suriarachchi
WSO2 Inc.
blog: http://amilachinthaka.blogspot.com/
