[jdom-interest] Bug in Document(List) -> setMixedContent()

[philip.nelson at omniresources.com]

Here is a patch to Document that fixes a problem similar to one reported
earlier today with Element.  
This really can't work unless the document already existed.  oldRoot is null
on a new document which causes NPE's so this patch allows you to create a
new document from a starter List.

********** marks the changed areas

    public Document setMixedContent(List mixedContent) {

        if (mixedContent == null) {
            return this;
        }

        // Save original content and create a new list
        Element oldRoot = getRootElement();
        List oldContent = content;
        content = new ArrayList(INITIAL_ARRAY_SIZE);

        RuntimeException ex = null;
        boolean didRoot = false;
        int itemsAdded = 0;

        try {
            for (Iterator i = mixedContent.iterator(); i.hasNext(); ) {
                Object obj = i.next();
                if (obj instanceof Element) {
                    if (didRoot == false) {
                        setRootElement((Element)obj);
                        // Manually remove old root's doc ref, because
                        // setRootElement() can't see the old content list
to
                        // do it for us
                        if (oldRoot != null) //****************
                        	oldRoot.setDocument(null);
                        didRoot = true;
                    }
                    else {
                        throw new IllegalAddException(
                          "A Document may contain only one root element");
                    }
                }
                else if (obj instanceof Comment) {
                    addContent((Comment)obj);
                }
                else if (obj instanceof ProcessingInstruction) {
                    addContent((ProcessingInstruction)obj);
                }
                else {
                    throw new IllegalAddException(
                      "A Document may directly contain only objects of type
" +
                      "Element, Comment, and ProcessingInstruction: " +
                      (obj == null ? "null" : obj.getClass().getName()) + 
                      " is not allowed");
                }
                itemsAdded++;
            }
    
            // Make sure they set a root element
            if (didRoot == false) {
                throw new IllegalAddException(
                    "A Document must contain a root element");
            }
        }
        catch (RuntimeException e) {
            ex = e;
        }
        finally {
            if (ex != null) {
                // Restore the original state and parentage and throw
                if (oldRoot != null) { //*******************
	                content = oldContent;
	                oldRoot.setDocument(this);
	                // Unmodify all modified elements.  DO NOT change
any later
	                // elements tho because they may already have
parents!
	                Iterator itr = mixedContent.iterator();
	                while (itemsAdded-- > 0) {
	                    Object obj = itr.next();
	                    if (obj instanceof Element) {
	                        ((Element)obj).setDocument(null);
	                    }
	                    else if (obj instanceof Comment) {
	                        ((Comment)obj).setDocument(null);
	                    }
	                    else if (obj instanceof ProcessingInstruction) {
	
((ProcessingInstruction)obj).setDocument(null);
	                    }
	                }
	                
                }
                throw ex;
            }
        }
snip

I also patched Document(List, DocType)to throw an NPE if the list is null
which would result in an invalid state. I suppose the NPE is arguable but
this is better than returning a useless document.


    public Document(List content, DocType docType) {
	    if (content == null) 
		    throw new NullPointerException();

        setMixedContent(content);
        setDocType(docType);
    }

My updated test suite still has a couple of other places where Document
doesn't handle nulls but these are documented as TODO's and can wait.