[
http://issues.apache.org/jira/browse/AXIS2-919?page=comments#action_12432700 ]
Derek Foster commented on AXIS2-919:
------------------------------------
Hi, Eran.
As for me helping fix the iterator issue: I just got back from vacation, and
have a lot of catching up to do. I might be able to help after a few days. I'm
probably not going to be much help with the XMLBeans binding problem, though: I
took a look at that code and couldn't really figure out what was supposed to be
happening (documentation is fairly poor in that code), or why the iterator was
hitting the end of the XML document prematurely as it seems to be.
As far as collections vs. interfaces, I think that it is a poor design decision
to force an awkward and difficult-to-use Iterator interface on clients of the
class just because the "right" easy-to-use interface would allow them to call a
method that MIGHT have poor performance under some circumstances. I think that
simply documenting the consequences of calling that method should be
sufficient, so that the user can decide whether or not the convenience is worth
the performance penalty, if any, particularly since there are many cases where
calling size() will have pretty much no performance impact (such as when the
user already knows that they have already cached all of the elements in
question, or when they know that they will be iterating over the entire list
immediately after having cached it by calling size()). In any case, users
should have little reason to call size(), since the Collection interface does
not provide a get(int) method, so the typical 'for' loop iterating from 0 to
size() is not likely to be used. (Whether or not the method should return
Collection versus List is another question, however.)
The collection approach can be implemented reasonably efficiently as:
private final Collection contents = new AbstractCollection()
{
private int cachedSize = -1;
public Iterator iterator() {
return createAnIteratorLikeTheOneThatCurrentlyExists();
}
public int size()
{
if (cachedSize < 0)
cachedSize = super.size();
return cachedSize;
}
}
public Collection getTheWidgets()
{
return contents;
}
As far as moving this discussion to commons-dev: I think that there are two
real bugs to be fixed here (in the XMLBeans binding code, as well as in the
Iterator implementation), so I think that this bug report should remain until
those are fixed. As far as the collections vs. iterator interface discussion, I
suppose that could be moved to commons-dev, or to a new bug report, but I must
confess to not being too thrilled about the idea of having to subscribe to
another high-volume mailing list just so I can monitor and contribute to the
discussion. (Just keeping up with Axis2-user is already drowning me in email.)
I wish that the Axis2 mailing lists allowed a "post-only" list membership so I
could just read the emails on the website.
Derek
> Unable to print out entire SOAP message: bug in OMElement.serialize()
> ---------------------------------------------------------------------
>
> Key: AXIS2-919
> URL: http://issues.apache.org/jira/browse/AXIS2-919
> Project: Apache Axis 2.0 (Axis2)
> Issue Type: Bug
> Components: core, om
> Affects Versions: 1.0
> Reporter: Derek Foster
> Assigned To: Eran Chinthaka
> Priority: Blocker
>
> I was attempting, inside an XMLBeans-generated service method, to print out
> the entire SOAP message to a log file. Towards that end, I tried to execute
> the following code:
> final StringWriter writer = new StringWriter();
> messageContext.getEnvelope().serialize(writer);
> System.out.println(writer.toString());
> However, I was surprised to get the following exception resulting from this:
> org.apache.axiom.om.OMException
> at
> org.apache.axiom.om.impl.llom.OMElementImpl.getNextOMSibling(OMElementImpl.java:266)
> at
> org.apache.axiom.om.impl.traverse.OMChildrenIterator.next(OMChildrenIterator.java:111)
> at
> org.apache.axiom.om.impl.llom.OMElementImpl.internalSerialize(OMElementImpl.java:771)
> at
> org.apache.axiom.soap.impl.llom.SOAPEnvelopeImpl.internalSerialize(SOAPEnvelopeImpl.java:177)
> at
> org.apache.axiom.om.impl.llom.OMElementImpl.internalSerialize(OMElementImpl.java:756)
> at
> org.apache.axiom.om.impl.llom.OMNodeImpl.serialize(OMNodeImpl.java:310)
> at
> org.apache.axiom.om.impl.llom.OMNodeImpl.serialize(OMNodeImpl.java:352)
> ...
> The cause of the problem appears to be the fact that the SOAP envelope is a
> top-level XML object. Thus, the item immediately following it is a
> DOCUMENT_END. The OMElementImpl.internalSerialize(XMLStreamWriter, boolean
> cache) method contains the following code:
> Iterator children = this.getChildren();
> while (children.hasNext()) {
> ((OMNodeEx) children.next()).internalSerialize(writer);
> which seems plausible. However, the iterator that is returned is of type
> OMChildrenIterator. Its hasNext method looks like this:
> /**
> * Returns <tt>true</tt> if the iteration has more elements. (In other
> * words, returns <tt>true</tt> if <tt>next</tt> would return an element
> * rather than throwing an exception.)
> *
> * @return Returns <tt>true</tt> if the iterator has more elements.
> */
> public boolean hasNext() {
> return (currentChild != null);
> }
> but its next() method looks like this:
> /**
> * Returns the next element in the iteration.
> *
> * @return Returns the next element in the iteration.
> * @throws java.util.NoSuchElementException
> * iteration has no more elements.
> */
> public Object next() {
> nextCalled = true;
> removeCalled = false;
> if (hasNext()) {
> lastChild = currentChild;
> currentChild = currentChild.getNextOMSibling();
> return lastChild;
> }
> return null;
> }
> which is a problem, because currentChild.getNextOMSibling() looks like this:
> /**
> * Gets the next sibling. This can be an OMAttribute or OMText or
> * OMELement for others.
> *
> * @throws OMException
> */
> public OMNode getNextOMSibling() throws OMException {
> while (!done) {
> int token = builder.next();
> if (token == XMLStreamConstants.END_DOCUMENT) {
> throw new OMException();
> }
> }
> return super.getNextOMSibling();
> }
> which will under some circumstances throw an OMException, which is not in the
> contract of the iterator's next() method to throw.
> Thus, iterator.hasNext() is returning true, implying that there is a next
> element, but when next() is actually called, the iterator discovers that it
> has reached the end of the document, and that therefore there really is no
> next element. It therefore throws an OMException.
> Firstly, if the next() method is going to be implemented this way, then
> hasNext() should perform enough lookahead on the document to detect that the
> next item in the input stream is indeed a END_DOCUMENT, and should therefore
> return false, since under these circumstances there truly isn't a next
> element (and calling next() would therefore always throw an OMException).
> That way, when the end of the document is hit, the serialize method will
> simply stop and return its results rather than trying to advance past the end
> of the document as it does now.
> Secondly, this OMException really should have some meaningful error text in
> it, such as "Can't call 'next()' on an iterator which is positioned at
> element 'foo' which is the last element in the document" or even better,
> "Can't call next() when hasNext() returns false" (see below). I can't think
> of any cases in typical library code where it is desirable to throw
> exceptions that don't have any message text. This is really unpleasant for
> anybody that is trying to debug a problem. I would encourage someone to do a
> search through the source code for "throw new OMException()" and add an
> explanatory error message wherever one is found.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
