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

7 files changed, 81 insertions, 8 deletions

diff --git a/src/java/org/apache/poi/hssf/record/EscherAggregate.java b/src/java/org/apache/poi/hssf/record/EscherAggregate.java
index 7579e71cf7..9ca4fefc06 100644
--- a/src/java/org/apache/poi/hssf/record/EscherAggregate.java
+++ b/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);
     }
 
     /**
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java b/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
index de3a123bc6..3f634b02b9 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
+++ b/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.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 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;
@@ -91,6 +94,27 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
     }
 
     /**
+     * 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) {
-                continue;
-            } else {
+            if (i != 0) {
                 HSSFShapeFactory.createShapeTree(spContainer, _boundAggregate, this, _sheet.getWorkbook().getRootDirectory());
             }
         }
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java b/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java
index 2397129bf2..0c6a612fcf 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java
+++ b/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);
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
index bb3c92ced0..7fc4882551 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
@@ -137,6 +137,15 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
     }
 
     /**
+     * check whether the data of sheet can be serialized
+     */
+    protected void preSerialize(){
+        if (_patriarch != null){
+            _patriarch.preSerialize();
+        }
+    }
+
+    /**
      * Return the parent workbook
      *
      * @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;
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
index f1828fa387..add83e7e68 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
+++ b/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();
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java b/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java
index 6c0919894f..cac444baef 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java
+++ b/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);

+    }

 }

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java
index 55ee037042..654db6aa0a 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java
+++ b/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);
+    }
 }