Ensure that XWPFParagraph.getDocument() is not null

(Null getDocument() causes NPE when looking for comments in XWPFWordExtractorDecorator)


git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1028283 13f79535-47bb-0310-9956-ffa450edef68

src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java

@@ -37,7 +37,7 @@ import org.apache.poi.openxml4j.opc.*;
  * @author Yegor Kozlov
  */
 public class POIXMLDocumentPart {
-    private static POILogger logger = POILogFactory.getLogger(POIXMLDocumentPart.class);
+    private static final POILogger logger = POILogFactory.getLogger(POIXMLDocumentPart.class);
 
     public static final XmlOptions DEFAULT_XML_OPTIONS;
     static {
@@ -87,6 +87,22 @@ public class POIXMLDocumentPart {
         this.packageRel = rel;
     }
 
+    /**
+     * Creates an POIXMLDocumentPart representing the given package part, relationship and parent
+     * Called by {@link #read(POIXMLFactory, java.util.Map)} when reading in an exisiting file.
+     *
+     * @param parent - Parent part
+     * @param part - The package part that holds xml data represenring this sheet.
+     * @param rel - the relationship of the given package part
+     * @see #read(POIXMLFactory, java.util.Map)
+     */
+    public POIXMLDocumentPart(POIXMLDocumentPart parent, PackagePart part, PackageRelationship rel){
+        this.relations = new LinkedList<POIXMLDocumentPart>();
+        this.packagePart = part;
+        this.packageRel = rel;
+        this.parent = parent;
+    }
+
     /**
      * When you open something like a theme, call this to
      *  re-base the XML Document onto the core child of the
@@ -274,7 +290,7 @@ public class POIXMLDocumentPart {
                 }
 
                 if (!context.containsKey(p)) {
-    				POIXMLDocumentPart childPart = factory.createDocumentPart(rel, p);
+    				POIXMLDocumentPart childPart = factory.createDocumentPart(this, rel, p);
     				childPart.parent = this;
     				addRelation(childPart);
                     if(p != null){

src/ooxml/java/org/apache/poi/POIXMLFactory.java

@@ -30,11 +30,12 @@ public abstract class POIXMLFactory {
      * Create a POIXMLDocumentPart from existing package part and relation. This method is called
      * from {@link POIXMLDocument#load(POIXMLFactory)} when parsing a document
      *
+     * @param parent parent part
      * @param rel   the package part relationship
      * @param part  the PackagePart representing the created instance
      * @return A new instance of a POIXMLDocumentPart.
      */
-     public abstract POIXMLDocumentPart createDocumentPart(PackageRelationship rel, PackagePart part);
+     public abstract POIXMLDocumentPart createDocumentPart(POIXMLDocumentPart parent, PackageRelationship rel, PackagePart part);
 
     /**
      * Create a new POIXMLDocumentPart using the supplied descriptor. This method is used when adding new parts

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFactory.java

@@ -34,7 +34,7 @@ import org.apache.poi.util.POILogger;
  * @author Yegor Kozlov
  */
 public final class XSSFFactory extends POIXMLFactory  {
-    private static POILogger logger = POILogFactory.getLogger(XSSFFactory.class);
+    private static final POILogger logger = POILogFactory.getLogger(XSSFFactory.class);
 
     private XSSFFactory(){
 
@@ -46,7 +46,8 @@ public final class XSSFFactory extends POIXMLFactory  {
         return inst;
     }
 
-    public POIXMLDocumentPart createDocumentPart(PackageRelationship rel, PackagePart part){
+    @Override
+    public POIXMLDocumentPart createDocumentPart(POIXMLDocumentPart parent, PackageRelationship rel, PackagePart part){
         POIXMLRelation descriptor = XSSFRelation.getInstance(rel.getRelationshipType());
         if(descriptor == null || descriptor.getRelationClass() == null){
             logger.log(POILogger.DEBUG, "using default POIXMLDocumentPart for " + rel.getRelationshipType());
@@ -62,6 +63,7 @@ public final class XSSFFactory extends POIXMLFactory  {
         }
     }
 
+    @Override
     public POIXMLDocumentPart newDocumentPart(POIXMLRelation descriptor){
         try {
             Class<? extends POIXMLDocumentPart> cls = descriptor.getRelationClass();

src/ooxml/java/org/apache/poi/xwpf/model/XWPFHeaderFooterPolicy.java

@@ -109,7 +109,7 @@ public class XWPFHeaderFooterPolicy {
 			PackagePart hdrPart = doc.getPartById(ref.getId());
 			HdrDocument hdrDoc = HdrDocument.Factory.parse(hdrPart.getInputStream());
 			CTHdrFtr hdrFtr = hdrDoc.getHdr();
-			XWPFHeader hdr = new XWPFHeader(hdrFtr);
+			XWPFHeader hdr = new XWPFHeader(doc, hdrFtr);
 
 			// Assign it
 			Enum type = ref.getType();
@@ -119,7 +119,7 @@ public class XWPFHeaderFooterPolicy {
 			// Get the footer
 			CTHdrFtrRef ref = sectPr.getFooterReferenceArray(i);
 			PackagePart ftrPart = doc.getPartById(ref.getId());
-			XWPFFooter ftr = new XWPFFooter(
+			XWPFFooter ftr = new XWPFFooter(doc, 
 					FtrDocument.Factory.parse(ftrPart.getInputStream()).getFtr());
 
 			// Assign it
@@ -460,6 +460,6 @@ public class XWPFHeaderFooterPolicy {
 		shapeTextPath.setString(text);
 		pict.set(group);
 		// end watermark paragraph
-		return new XWPFParagraph(p, null);
+		return new XWPFParagraph(p, doc);
 	}
 }

src/ooxml/java/org/apache/poi/xwpf/usermodel/IBody.java

@@ -63,7 +63,6 @@ public interface IBody {
 	 */
     public List<XWPFParagraph> getParagraphs();
 
-	
 	/**
 	 * Return the table(s) that holds the text
 	 *  of the IBodyPart, for complex cases
@@ -124,5 +123,10 @@ public interface IBody {
 	 */
 	XWPFTableCell getTableCell(CTTc cell);
 
+    /**
+     * Return XWPFDocument
+     */
+    public XWPFDocument getXWPFDocument();
+
 }
 

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFComment.java

@@ -31,7 +31,7 @@ public class XWPFComment
     protected String author;
     protected StringBuffer text;
     
-    public XWPFComment(CTComment comment)
+    public XWPFComment(CTComment comment, XWPFDocument document)
     {
         text = new StringBuffer();
         id = comment.getId().toString();
@@ -39,7 +39,7 @@ public class XWPFComment
         
         for(CTP ctp : comment.getPList())
         {
-            XWPFParagraph p = new XWPFParagraph(ctp, null);
+            XWPFParagraph p = new XWPFParagraph(ctp, document);
             text.append(p.getText());
         }
     }

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java

@@ -165,25 +165,18 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody {
                 String relation = p.getPackageRelationship().getRelationshipType();
                 if(relation.equals(XWPFRelation.STYLES.getRelation())){
                 	this.styles = (XWPFStyles) p;
-                }
+                } else if (relation.equals(XWPFRelation.NUMBERING.getRelation())){
-                else if(relation.equals(XWPFRelation.NUMBERING.getRelation())){
                 	this.numbering = (XWPFNumbering) p;
-
+                } else if (relation.equals(XWPFRelation.FOOTER.getRelation())){
-                }
-                else if(relation.equals(XWPFRelation.FOOTER.getRelation())){
                 	footers.add((XWPFFooter)p);
-                }
+                } else if (relation.equals(XWPFRelation.HEADER.getRelation())){
-                else if(relation.equals(XWPFRelation.HEADER.getRelation())){
                 	headers.add((XWPFHeader)p);
-                }
+                } else if (relation.equals(XWPFRelation.COMMENT.getRelation())){
-
-                else if(relation.equals(XWPFRelation.COMMENT.getRelation())){
                     CommentsDocument cmntdoc = CommentsDocument.Factory.parse(p.getPackagePart().getInputStream());
                     for(CTComment ctcomment : cmntdoc.getComments().getCommentList()) {
-                        comments.add(new XWPFComment(ctcomment));
+                        comments.add(new XWPFComment(ctcomment, this));
                     }
-                }
+                } else if (relation.equals(XWPFRelation.SETTINGS.getRelation())){
-                else if(relation.equals(XWPFRelation.SETTINGS.getRelation())){
                 	settings = (XWPFSettings)p;
                 }
             }
@@ -254,6 +247,7 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody {
     /**
      * Create a new CTWorkbook with all values set to default
      */
+    @Override
     protected void onDocumentCreate() {
         hyperlinks = new ArrayList<XWPFHyperlink>();
         comments = new ArrayList<XWPFComment>();
@@ -429,6 +423,7 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody {
     /**
      * Get the document's embedded files.
      */
+    @Override
     public List<PackagePart> getAllEmbedds() throws OpenXML4JException {
         List<PackagePart> embedds = new LinkedList<PackagePart>();
 
@@ -1116,4 +1111,8 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody {
 		}
 		return tableRow.getTableCell(cell);
 	}
+
+    public XWPFDocument getXWPFDocument() {
+        return this;
+    }
 }//end class

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFFactory.java

@@ -33,7 +33,7 @@ import org.apache.poi.util.POILogger;
  */
 public final class XWPFFactory extends POIXMLFactory  {
 
-    private static POILogger logger = POILogFactory.getLogger(XWPFFactory.class);
+    private static final POILogger logger = POILogFactory.getLogger(XWPFFactory.class);
 
     private XWPFFactory(){
 
@@ -45,7 +45,8 @@ public final class XWPFFactory extends POIXMLFactory  {
         return inst;
     }
 
-    public POIXMLDocumentPart createDocumentPart(PackageRelationship rel, PackagePart part){
+    @Override
+    public POIXMLDocumentPart createDocumentPart(POIXMLDocumentPart parent, PackageRelationship rel, PackagePart part){
         POIXMLRelation descriptor = XWPFRelation.getInstance(rel.getRelationshipType());
         if(descriptor == null || descriptor.getRelationClass() == null){
             logger.log(POILogger.DEBUG, "using default POIXMLDocumentPart for " + rel.getRelationshipType());
@@ -54,13 +55,19 @@ public final class XWPFFactory extends POIXMLFactory  {
 
         try {
             Class<? extends POIXMLDocumentPart> cls = descriptor.getRelationClass();
+            try {
+                Constructor<? extends POIXMLDocumentPart> constructor = cls.getDeclaredConstructor(POIXMLDocumentPart.class, PackagePart.class, PackageRelationship.class);
+                return constructor.newInstance(parent, part, rel);
+            } catch (NoSuchMethodException e) {
                 Constructor<? extends POIXMLDocumentPart> constructor = cls.getDeclaredConstructor(PackagePart.class, PackageRelationship.class);
                 return constructor.newInstance(part, rel);
+            }
         } catch (Exception e){
             throw new POIXMLException(e);
         }
     }
 
+    @Override
     public POIXMLDocumentPart newDocumentPart(POIXMLRelation descriptor){
         try {
             Class<? extends POIXMLDocumentPart> cls = descriptor.getRelationClass();

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFFooter.java

@@ -25,6 +25,7 @@ import java.util.Map;
 
 import javax.xml.namespace.QName;
 
+import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.xmlbeans.XmlCursor;
@@ -45,9 +46,8 @@ public class XWPFFooter extends XWPFHeaderFooter {
         super();
     }
 
-	
+	public XWPFFooter(XWPFDocument doc, CTHdrFtr hdrFtr) throws IOException {
-	public XWPFFooter(CTHdrFtr hdrFtr) throws IOException {
+		super(doc, hdrFtr);
-		super(hdrFtr);
 		bodyElements = new ArrayList<IBodyElement>();
 		paragraphs = new ArrayList<XWPFParagraph>();
 		tables = new ArrayList<XWPFTable>();
@@ -69,8 +69,8 @@ public class XWPFFooter extends XWPFHeaderFooter {
         getAllPictures();
 	}
 
-	public XWPFFooter(PackagePart part, PackageRelationship rel) throws IOException {
+	public XWPFFooter(POIXMLDocumentPart parent, PackagePart part, PackageRelationship rel) throws IOException {
-		super(part, rel);
+		super(parent, part, rel);
 	}
 	
 	/**
@@ -149,5 +149,4 @@ public class XWPFFooter extends XWPFHeaderFooter {
 		public BodyType getPartType() {
 			return BodyType.FOOTER;
 		}
-
 }

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFHeader.java

@@ -42,17 +42,16 @@ import org.openxmlformats.schemas.wordprocessingml.x2006.main.HdrDocument;
  * Sketch of XWPF header class
  */
 public class XWPFHeader extends XWPFHeaderFooter {
-	
     public XWPFHeader() {
         super();
     }
 
-	public XWPFHeader(PackagePart part, PackageRelationship rel) throws IOException {
+    public XWPFHeader(POIXMLDocumentPart parent, PackagePart part, PackageRelationship rel) throws IOException {
-		super(part, rel);
+		super(parent, part, rel);
 	}
 	
-	public XWPFHeader(CTHdrFtr hdrFtr) throws IOException {
+	public XWPFHeader(XWPFDocument doc, CTHdrFtr hdrFtr) throws IOException {
-		super(hdrFtr);
+		super(doc, hdrFtr);
 		paragraphs = new ArrayList<XWPFParagraph>();
 		tables = new ArrayList<XWPFTable>();
 		XmlCursor cursor = headerFooter.newCursor();

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFHeaderFooter.java

@@ -51,17 +51,30 @@ public abstract class XWPFHeaderFooter extends POIXMLDocumentPart implements IBo
 	protected XWPFDocument document;
 	protected List<IBodyElement> bodyElements;
 	
-	protected XWPFHeaderFooter(CTHdrFtr hdrFtr){
+	protected XWPFHeaderFooter(XWPFDocument doc, CTHdrFtr hdrFtr){
+        if (doc==null) {
+            throw new NullPointerException();
+        }
+
+        document = doc;
 		headerFooter = hdrFtr;
 		readHdrFtr();
 	}
+
     protected XWPFHeaderFooter() {
-	   this(CTHdrFtr.Factory.newInstance());
+        headerFooter = CTHdrFtr.Factory.newInstance();
+        readHdrFtr();
     }
 
-	public XWPFHeaderFooter(PackagePart part, PackageRelationship rel) throws IOException {
+
-		super(part, rel);
+    public XWPFHeaderFooter(POIXMLDocumentPart parent, PackagePart part, PackageRelationship rel) throws IOException {
+		super(parent, part, rel);
 		this.document = (XWPFDocument)getParent();
+
+        if (this.document==null) {
+            throw new NullPointerException();
+        }
+
         onDocumentRead();
 	}
 	
@@ -523,5 +536,11 @@ public abstract class XWPFHeaderFooter extends POIXMLDocumentPart implements IBo
 		return tableRow.getTableCell(cell);
 	}
   	
-    
+    public XWPFDocument getXWPFDocument() {
+        if (document!=null) {
+            return document;
+        } else {
+            return (XWPFDocument)getParent();
+        }
+    }
 }//end class

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java

@@ -55,7 +55,7 @@ import org.openxmlformats.schemas.wordprocessingml.x2006.main.STTextAlignment;
  * Sketch of XWPF paragraph class
  */
 public class XWPFParagraph implements IBodyElement{
-    private CTP paragraph;
+    private final CTP paragraph;
     protected IBody part;
     /** For access to the document's hyperlink, comments, tables etc */
     protected XWPFDocument document;
@@ -63,20 +63,14 @@ public class XWPFParagraph implements IBodyElement{
     
     private StringBuffer footnoteText = new StringBuffer();
 
-    public XWPFParagraph(CTP prgrph) {
-        this(prgrph, null);
-    }
-
-
     public XWPFParagraph(CTP prgrph, IBody part) {
         this.paragraph = prgrph;
         this.part = part;
         
-        // We only care about the document (for comments,
+        this.document = part.getXWPFDocument();
-        //  hyperlinks etc) if we're attached to the
+
-        //  core document
+        if (document==null) {
-        if(part instanceof XWPFDocument) {
+            throw new NullPointerException();
-           this.document = (XWPFDocument)part;
         }
 
         runs = new ArrayList<XWPFRun>();

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFTableCell.java

@@ -30,8 +30,7 @@ import org.openxmlformats.schemas.wordprocessingml.x2006.main.CTTc;
 
 
 public class XWPFTableCell implements IBody {
-
+    private final CTTc ctTc;
-    private CTTc ctTc;
     protected List<XWPFParagraph> paragraphs = null;
     protected List<XWPFTable> tables = null;
     protected List<IBodyElement> bodyElements = null;
@@ -256,7 +255,7 @@ public class XWPFTableCell implements IBody {
 	 * @see org.apache.poi.xwpf.usermodel.IBody#getPart()
 	 */
 	public IBody getPart() {
-		return (IBody) tableRow.getTable().getPart();
+		return tableRow.getTable().getPart();
 	}
 
 
@@ -351,4 +350,8 @@ public class XWPFTableCell implements IBody {
 		}
 		return tableRow.getTableCell(cell);
 	}
+
+    public XWPFDocument getXWPFDocument() {
+        return part.getXWPFDocument();
+    }
 }// end class

src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java

@@ -57,7 +57,7 @@ public final class TestPOIXMLDocument extends TestCase {
         public TestFactory() {
             //
         }
-        public POIXMLDocumentPart createDocumentPart(PackageRelationship rel, PackagePart part){
+        public POIXMLDocumentPart createDocumentPart(POIXMLDocumentPart parent, PackageRelationship rel, PackagePart part){
             return new POIXMLDocumentPart(part, rel);
         }
 

src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFHeader.java

@@ -76,12 +76,12 @@ public final class TestXWPFHeader extends TestCase {
 		CTText t3 = ctR3.addNewT();
 		t3.setStringValue("Second paragraph for the footer");
 
-		XWPFParagraph p1 = new XWPFParagraph(ctP1);
+		XWPFParagraph p1 = new XWPFParagraph(ctP1, sampleDoc);
 		XWPFParagraph[] pars = new XWPFParagraph[1];
 		pars[0] = p1;
 
-		XWPFParagraph p2 = new XWPFParagraph(ctP2);
+		XWPFParagraph p2 = new XWPFParagraph(ctP2, sampleDoc);
-		XWPFParagraph p3 = new XWPFParagraph(ctP3, null);
+		XWPFParagraph p3 = new XWPFParagraph(ctP3, sampleDoc);
 		XWPFParagraph[] pars2 = new XWPFParagraph[2];
 		pars2[0] = p2;
 		pars2[1] = p3;
@@ -98,7 +98,7 @@ public final class TestXWPFHeader extends TestCase {
 		assertNotNull(policy.getDefaultFooter());
 		// ....and that the footer object captured above contains two
 		// paragraphs of text.
-		assertEquals(footer.getParagraphs().size(), 2);
+		assertEquals(2, footer.getParagraphs().size());
 		
 		// As an additional check, recover the defauls footer and
 		// make sure that it contains two paragraphs of text and that
@@ -111,9 +111,9 @@ public final class TestXWPFHeader extends TestCase {
 		   paras[i++] = p;
 		}
 		
-		assertEquals(paras.length, 2);
+		assertEquals(2, paras.length);
-		assertEquals(paras[0].getText(), "First paragraph for the footer");
+		assertEquals("First paragraph for the footer", paras[0].getText());
-		assertEquals(paras[1].getText(), "Second paragraph for the footer");
+		assertEquals("Second paragraph for the footer", paras[1].getText());
 	}
 
 	public void testSetWatermark() {

src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFTable.java

@@ -42,15 +42,16 @@ public class TestXWPFTable extends TestCase {
     }
 
     public void testConstructor() {
+        XWPFDocument doc = new XWPFDocument();
         CTTbl ctTable = CTTbl.Factory.newInstance();
-        XWPFTable xtab = new XWPFTable(ctTable, null);
+        XWPFTable xtab = new XWPFTable(ctTable, doc);
         assertNotNull(xtab);
         assertEquals(1, ctTable.sizeOfTrArray());
         assertEquals(1, ctTable.getTrArray(0).sizeOfTcArray());
         assertNotNull(ctTable.getTrArray(0).getTcArray(0).getPArray(0));
 
         ctTable = CTTbl.Factory.newInstance();
-        xtab = new XWPFTable(ctTable, null, 3, 2);
+        xtab = new XWPFTable(ctTable, doc, 3, 2);
         assertNotNull(xtab);
         assertEquals(3, ctTable.sizeOfTrArray());
         assertEquals(2, ctTable.getTrArray(0).sizeOfTcArray());
@@ -59,6 +60,7 @@ public class TestXWPFTable extends TestCase {
 
 
     public void testGetText() {
+        XWPFDocument doc = new XWPFDocument();
         CTTbl table = CTTbl.Factory.newInstance();
         CTRow row = table.addNewTr();
         CTTc cell = row.addNewTc();
@@ -67,12 +69,14 @@ public class TestXWPFTable extends TestCase {
         CTText text = run.addNewT();
         text.setStringValue("finally I can write!");
 
-        XWPFTable xtab = new XWPFTable(table, null);
+        XWPFTable xtab = new XWPFTable(table, doc);
         assertEquals("finally I can write!\n", xtab.getText());
     }
 
 
     public void testCreateRow() {
+        XWPFDocument doc = new XWPFDocument();
+
         CTTbl table = CTTbl.Factory.newInstance();
         CTRow r1 = table.addNewTr();
         r1.addNewTc().addNewP();
@@ -84,7 +88,7 @@ public class TestXWPFTable extends TestCase {
         r3.addNewTc().addNewP();
         r3.addNewTc().addNewP();
 
-        XWPFTable xtab = new XWPFTable(table, null);
+        XWPFTable xtab = new XWPFTable(table, doc);
         assertEquals(3, xtab.getNumberOfRows());
         assertNotNull(xtab.getRow(2));
 
@@ -95,16 +99,18 @@ public class TestXWPFTable extends TestCase {
         assertEquals(2, table.getTrArray(0).sizeOfTcArray());
 
         //check creation of first row
-        xtab = new XWPFTable(CTTbl.Factory.newInstance(), null);
+        xtab = new XWPFTable(CTTbl.Factory.newInstance(), doc);
         assertEquals(1, xtab.getCTTbl().getTrArray(0).sizeOfTcArray());
     }
 
 
     public void testSetGetWidth() {
+        XWPFDocument doc = new XWPFDocument();
+        
         CTTbl table = CTTbl.Factory.newInstance();
         table.addNewTblPr().addNewTblW().setW(new BigInteger("1000"));
 
-        XWPFTable xtab = new XWPFTable(table, null);
+        XWPFTable xtab = new XWPFTable(table, doc);
 
         assertEquals(1000, xtab.getWidth());
 
@@ -113,9 +119,11 @@ public class TestXWPFTable extends TestCase {
     }
 
     public void testSetGetHeight() {
+        XWPFDocument doc = new XWPFDocument();
+
         CTTbl table = CTTbl.Factory.newInstance();
 
-        XWPFTable xtab = new XWPFTable(table, null);
+        XWPFTable xtab = new XWPFTable(table, doc);
         XWPFTableRow row = xtab.createRow();
         row.setHeight(20);
         assertEquals(20, row.getHeight());