fixed bug 53028, added check before serialization if any cell contains 2 or more comments

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

src/java/org/apache/poi/hssf/record/EscherAggregate.java

@@ -773,7 +773,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      *         Every HSSFComment shape has a link to a NoteRecord from the tailRec collection.
      */
     public Map<Integer, NoteRecord> getTailRecords() {
-        return tailRec;
+        return Collections.unmodifiableMap(tailRec);
     }
 
     /**

src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java

@@ -17,15 +17,16 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
 
 import org.apache.poi.ddf.*;
 import org.apache.poi.hssf.model.DrawingManager2;
 import org.apache.poi.hssf.record.EscherAggregate;
+import org.apache.poi.hssf.record.NoteRecord;
+import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.ss.usermodel.Chart;
+import org.apache.poi.util.POILogFactory;
+import org.apache.poi.util.POILogger;
 import org.apache.poi.util.StringUtil;
 import org.apache.poi.util.Internal;
 import org.apache.poi.ss.usermodel.Drawing;
@@ -38,6 +39,7 @@ import org.apache.poi.ss.usermodel.ClientAnchor;
  * @author Glen Stampoultzis (glens at apache.org)
  */
 public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
+    private static POILogger log = POILogFactory.getLogger(HSSFPatriarch.class);
     private final List<HSSFShape> _shapes = new ArrayList<HSSFShape>();
 
     private final EscherSpgrRecord _spgrRecord;
@@ -55,6 +57,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
      * Creates the patriarch.
      *
      * @param sheet the sheet this patriarch is stored in.
+     * @param boundAggregate -low level representation of all binary data inside sheet
      */
     HSSFPatriarch(HSSFSheet sheet, EscherAggregate boundAggregate) {
         _sheet = sheet;
@@ -90,6 +93,27 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         return newPatriarch;
     }
 
+    /**
+     * check if any shapes contain wrong data
+     * At now(13.08.2010) check if patriarch contains 2 or more comments with same coordinates
+     */
+    protected void preSerialize(){
+        Map<Integer, NoteRecord> tailRecords = _boundAggregate.getTailRecords();
+        /**
+         * contains coordinates of comments we iterate over
+         */
+        Set<String> coordinates = new HashSet<String>(tailRecords.size());
+        for(NoteRecord rec : tailRecords.values()){
+            String noteRef = new CellReference(rec.getRow(),
+                    rec.getColumn()).formatAsString(); // A1-style notation
+            if(coordinates.contains(noteRef )){
+                throw new IllegalStateException("found multiple cell comments for cell " + noteRef );
+            } else {
+                coordinates.add(noteRef);
+            }
+        }
+    }
+
     /**
      * @param shape to be removed
      * @return true of shape is removed
@@ -146,6 +170,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
      *
      * @param anchor the client anchor describes how this group is attached
      *               to the sheet.
+     * @param pictureIndex - pointer to the byte array saved inside workbook in escher bse record
      * @return the newly created shape.
      */
     public HSSFPicture createPicture(HSSFClientAnchor anchor, int pictureIndex) {
@@ -365,6 +390,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
 
     /**
      * Returns the aggregate escher record we're bound to
+     * @return - low level representation of sheet drawing data
      */
     protected EscherAggregate _getBoundAggregate() {
         return _boundAggregate;
@@ -406,9 +432,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
 
         for (int i = 0; i < spgrChildren.size(); i++) {
             EscherContainerRecord spContainer = spgrChildren.get(i);
-            if (i == 0) {
+            if (i != 0) {
-                continue;
-            } else {
                 HSSFShapeFactory.createShapeTree(spContainer, _boundAggregate, this, _sheet.getWorkbook().getRootDirectory());
             }
         }

src/java/org/apache/poi/hssf/usermodel/HSSFRow.java

@@ -458,6 +458,7 @@ public final class HSSFRow implements Row {
     {
         if(height == -1){
             row.setHeight((short)(0xFF | 0x8000));
+            row.setBadFontHeight(false);
         } else {
             row.setBadFontHeight(true);
             row.setHeight(height);

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -136,6 +136,15 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
         return sheet;
     }
 
+    /**
+     * check whether the data of sheet can be serialized
+     */
+    protected void preSerialize(){
+        if (_patriarch != null){
+            _patriarch.preSerialize();
+        }
+    }
+
     /**
      * Return the parent workbook
      *
@@ -215,6 +224,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
         HSSFRow row = new HSSFRow(_workbook, this, rownum);
         // new rows inherit default height from the sheet
         row.setHeight(getDefaultRowHeight());
+        row.getRowRecord().setBadFontHeight(false);
 
         addRow(row, true);
         return row;

src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -1245,6 +1245,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
         // serialization is about to occur.
         for (int i = 0; i < nSheets; i++) {
             sheets[i].getSheet().preSerialize();
+            sheets[i].preSerialize();
         }
 
         int totalsize = workbook.getSize();

src/testcases/org/apache/poi/hssf/usermodel/TestComment.java

@@ -108,8 +108,10 @@ public class TestComment extends TestCase {
         int idx = wb.addPicture(new byte[]{1,2,3}, HSSFWorkbook.PICTURE_TYPE_PNG);
 
         HSSFComment comment = patriarch.createCellComment(new HSSFClientAnchor());
+        comment.setColumn(5);
         comment.setString(new HSSFRichTextString("comment1"));
         comment = patriarch.createCellComment(new HSSFClientAnchor(0,0,100,100,(short)0,0,(short)10,10));
+        comment.setRow(5);
         comment.setString(new HSSFRichTextString("comment2"));
         comment.setBackgroundImage(idx);
         assertEquals(comment.getBackgroundImageId(), idx);
@@ -265,4 +267,22 @@ public class TestComment extends TestCase {
         assertEquals(comment.getShapeId(), 2024);
         assertEquals(comment.getNoteRecord().getShapeId(), 1000);
     }
+    
+    public void testAttemptToSave2CommentsWithSameCoordinates(){
+        Object err = null;
+        
+        HSSFWorkbook wb = new HSSFWorkbook();
+        HSSFSheet sh = wb.createSheet();
+        HSSFPatriarch patriarch = sh.createDrawingPatriarch();
+        patriarch.createCellComment(new HSSFClientAnchor());
+        patriarch.createCellComment(new HSSFClientAnchor());
+        
+        try{
+            HSSFTestDataSamples.writeOutAndReadBack(wb);
+        } catch (IllegalStateException e){
+            err = 1;
+            assertEquals(e.getMessage(), "found multiple cell comments for cell $A$1");
+        }
+        assertNotNull(err);
+    }
 }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java

@@ -116,4 +116,21 @@ public final class TestHSSFRow extends BaseTestRow {
         assertEquals(2, row.getFirstCellNum());
         assertEquals(6, row.getLastCellNum());
     }
+
+    public void testRowHeight(){
+        HSSFWorkbook workbook = new HSSFWorkbook();
+        HSSFSheet sheet = workbook.createSheet();
+        HSSFRow row = sheet.createRow(0);
+
+        assertEquals(row.getHeight(), sheet.getDefaultRowHeight());
+        assertEquals(row.getRowRecord().getBadFontHeight(), false);
+
+        row.setHeight((short) 123);
+        assertEquals(row.getHeight(), 123);
+        assertEquals(row.getRowRecord().getBadFontHeight(), true);
+
+        row.setHeight((short) -1);
+        assertEquals(row.getHeight(), sheet.getDefaultRowHeight());
+        assertEquals(row.getRowRecord().getBadFontHeight(), false);
+    }
 }