fixed CommonObjectDataSubRecord.field_2_objectId to be unsigned, also fixed HSSFCell.findCellComment to handle sheets with more than 65536 comments

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

13 files changed, 68 insertions, 62 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 6ba397d182..5438a69704 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,7 +34,8 @@
     <changes>
         <release version="3.5-beta7" date="2009-??-??">
         </release>
-        <release version="3.5-beta6" date="2009-06-11">
+        <release version="3.5-beta6" date="2009-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">47309 - Fixed logic in HSSFCell.getCellComment to handle sheets with more than 65536 comments</action>
            <action dev="POI-DEVELOPERS" type="fix">46776 - Added clone() method to MulBlankRecord to fix crash in Sheet.cloneSheet()</action>
            <action dev="POI-DEVELOPERS" type="fix">47244 - Fixed HSSFSheet to handle missing header / footer records</action>
            <action dev="POI-DEVELOPERS" type="fix">47312 - Fixed formula parser to properly reject cell references with a '0' row component</action>
diff --git a/src/java/org/apache/poi/hssf/model/CommentShape.java b/src/java/org/apache/poi/hssf/model/CommentShape.java
index 0fc3cd4636..49f52e0e83 100644
--- a/src/java/org/apache/poi/hssf/model/CommentShape.java
+++ b/src/java/org/apache/poi/hssf/model/CommentShape.java
@@ -74,10 +74,10 @@ public class CommentShape extends TextboxShape {
      private NoteRecord createNoteRecord( HSSFComment shape, int shapeId )
     {
         NoteRecord note = new NoteRecord();
-        note.setColumn((short)shape.getColumn());
-        note.setRow((short)shape.getRow());
+        note.setColumn(shape.getColumn());
+        note.setRow(shape.getRow());
         note.setFlags(shape.isVisible() ? NoteRecord.NOTE_VISIBLE : NoteRecord.NOTE_HIDDEN);
-        note.setShapeId((short)shapeId);
+        note.setShapeId(shapeId);
         note.setAuthor(shape.getAuthor() == null ? "" : shape.getAuthor());
         return note;
     }
diff --git a/src/java/org/apache/poi/hssf/model/LineShape.java b/src/java/org/apache/poi/hssf/model/LineShape.java
index b27317afad..74913b3afc 100644
--- a/src/java/org/apache/poi/hssf/model/LineShape.java
+++ b/src/java/org/apache/poi/hssf/model/LineShape.java
@@ -97,7 +97,7 @@ public class LineShape
         ObjRecord obj = new ObjRecord();
         CommonObjectDataSubRecord c = new CommonObjectDataSubRecord();
         c.setObjectType((short) ((HSSFSimpleShape)shape).getShapeType());
-        c.setObjectId((short) ( shapeId ));
+        c.setObjectId(shapeId);
         c.setLocked(true);
         c.setPrintable(true);
         c.setAutofill(true);
diff --git a/src/java/org/apache/poi/hssf/model/PictureShape.java b/src/java/org/apache/poi/hssf/model/PictureShape.java
index 960b636c01..01cebe56d7 100644
--- a/src/java/org/apache/poi/hssf/model/PictureShape.java
+++ b/src/java/org/apache/poi/hssf/model/PictureShape.java
@@ -100,7 +100,7 @@ public class PictureShape
         CommonObjectDataSubRecord c = new CommonObjectDataSubRecord();
         c.setObjectType((short) ((HSSFSimpleShape)shape).getShapeType());
 //        c.setObjectId((short) ( 1 ));
-        c.setObjectId((short) ( shapeId ));
+        c.setObjectId(shapeId);
         c.setLocked(true);
         c.setPrintable(true);
         c.setAutofill(true);
diff --git a/src/java/org/apache/poi/hssf/model/PolygonShape.java b/src/java/org/apache/poi/hssf/model/PolygonShape.java
index d216a2dc58..eeb18b0718 100644
--- a/src/java/org/apache/poi/hssf/model/PolygonShape.java
+++ b/src/java/org/apache/poi/hssf/model/PolygonShape.java
@@ -134,7 +134,7 @@ public class PolygonShape
         ObjRecord obj = new ObjRecord();
         CommonObjectDataSubRecord c = new CommonObjectDataSubRecord();
         c.setObjectType( OBJECT_TYPE_MICROSOFT_OFFICE_DRAWING );
-        c.setObjectId( (short) ( shapeId ) );
+        c.setObjectId(shapeId);
         c.setLocked( true );
         c.setPrintable( true );
         c.setAutofill( true );
diff --git a/src/java/org/apache/poi/hssf/model/SimpleFilledShape.java b/src/java/org/apache/poi/hssf/model/SimpleFilledShape.java
index f10e1086ad..8c14b3d39b 100644
--- a/src/java/org/apache/poi/hssf/model/SimpleFilledShape.java
+++ b/src/java/org/apache/poi/hssf/model/SimpleFilledShape.java
@@ -101,7 +101,7 @@ public class SimpleFilledShape
         ObjRecord obj = new ObjRecord();
         CommonObjectDataSubRecord c = new CommonObjectDataSubRecord();
         c.setObjectType( (short) ( (HSSFSimpleShape) shape ).getShapeType() );
-        c.setObjectId( (short) ( shapeId ) );
+        c.setObjectId( shapeId );
         c.setLocked( true );
         c.setPrintable( true );
         c.setAutofill( true );
diff --git a/src/java/org/apache/poi/hssf/model/TextboxShape.java b/src/java/org/apache/poi/hssf/model/TextboxShape.java
index e74197711b..4985ea31d7 100644
--- a/src/java/org/apache/poi/hssf/model/TextboxShape.java
+++ b/src/java/org/apache/poi/hssf/model/TextboxShape.java
@@ -59,7 +59,7 @@ public class TextboxShape
         ObjRecord obj = new ObjRecord();
         CommonObjectDataSubRecord c = new CommonObjectDataSubRecord();
         c.setObjectType( (short) ( (HSSFSimpleShape) shape ).getShapeType() );
-        c.setObjectId( (short) ( shapeId ) );
+        c.setObjectId( shapeId );
         c.setLocked( true );
         c.setPrintable( true );
         c.setAutofill( true );
diff --git a/src/java/org/apache/poi/hssf/record/CommonObjectDataSubRecord.java b/src/java/org/apache/poi/hssf/record/CommonObjectDataSubRecord.java
index 185c69558f..3cd2f282c1 100644
--- a/src/java/org/apache/poi/hssf/record/CommonObjectDataSubRecord.java
+++ b/src/java/org/apache/poi/hssf/record/CommonObjectDataSubRecord.java
@@ -69,7 +69,7 @@ public final class CommonObjectDataSubRecord extends SubRecord {
     public final static short OBJECT_TYPE_MICROSOFT_OFFICE_DRAWING = 30;
     
     private  short      field_1_objectType;
-    private  short      field_2_objectId;
+    private  int        field_2_objectId;
     private  short      field_3_option;
     private  int        field_4_reserved1;
     private  int        field_5_reserved2;
@@ -86,7 +86,7 @@ public final class CommonObjectDataSubRecord extends SubRecord {
             throw new RecordFormatException("Expected size 18 but got (" + size + ")");
         }
         field_1_objectType             = in.readShort();
-        field_2_objectId               = in.readShort();
+        field_2_objectId               = in.readUShort();
         field_3_option                 = in.readShort();
         field_4_reserved1              = in.readInt();
         field_5_reserved2              = in.readInt();
@@ -252,7 +252,7 @@ public final class CommonObjectDataSubRecord extends SubRecord {
     /**
      * Get the object id field for the CommonObjectData record.
      */
-    public short getObjectId()
+    public int getObjectId()
     {
         return field_2_objectId;
     }
@@ -260,7 +260,7 @@ public final class CommonObjectDataSubRecord extends SubRecord {
     /**
      * Set the object id field for the CommonObjectData record.
      */
-    public void setObjectId(short field_2_objectId)
+    public void setObjectId(int field_2_objectId)
     {
         this.field_2_objectId = field_2_objectId;
     }
diff --git a/src/java/org/apache/poi/hssf/record/EscherAggregate.java b/src/java/org/apache/poi/hssf/record/EscherAggregate.java
index e96de6ec72..a6267ef3b8 100644
--- a/src/java/org/apache/poi/hssf/record/EscherAggregate.java
+++ b/src/java/org/apache/poi/hssf/record/EscherAggregate.java
@@ -812,7 +812,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 		ObjRecord obj = new ObjRecord();
 		CommonObjectDataSubRecord cmo = new CommonObjectDataSubRecord();
 		cmo.setObjectType( CommonObjectDataSubRecord.OBJECT_TYPE_GROUP );
-		cmo.setObjectId( (short) ( shapeId ) );
+		cmo.setObjectId( shapeId );
 		cmo.setLocked( true );
 		cmo.setPrintable( true );
 		cmo.setAutofill( true );
diff --git a/src/java/org/apache/poi/hssf/record/NoteRecord.java b/src/java/org/apache/poi/hssf/record/NoteRecord.java
index f24ea0b4ae..f5c8d6acd9 100644
--- a/src/java/org/apache/poi/hssf/record/NoteRecord.java
+++ b/src/java/org/apache/poi/hssf/record/NoteRecord.java
@@ -45,7 +45,7 @@ public final class NoteRecord extends StandardRecord {
 	private int field_1_row;
 	private int field_2_col;
 	private short field_3_flags;
-	private short field_4_shapeid;
+	private int field_4_shapeid;
 	private boolean field_5_hasMultibyte;
 	private String field_6_author;
 	/**
@@ -77,10 +77,10 @@ public final class NoteRecord extends StandardRecord {
 	 * Read the record data from the supplied <code>RecordInputStream</code>
 	 */
 	public NoteRecord(RecordInputStream in) {
-		field_1_row = in.readShort();
+		field_1_row = in.readUShort();
 		field_2_col = in.readShort();
 		field_3_flags = in.readShort();
-		field_4_shapeid = in.readShort();
+		field_4_shapeid = in.readUShort();
 		int length = in.readShort();
 		field_5_hasMultibyte = in.readByte() != 0x00;
 		if (field_5_hasMultibyte) {
@@ -194,14 +194,14 @@ public final class NoteRecord extends StandardRecord {
 	/**
 	 * Object id for OBJ record that contains the comment
 	 */
-	public short getShapeId() {
+	public int getShapeId() {
 		return field_4_shapeid;
 	}
 
 	/**
 	 * Object id for OBJ record that contains the comment
 	 */
-	public void setShapeId(short id) {
+	public void setShapeId(int id) {
 		field_4_shapeid = id;
 	}
 
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
index d985216c9d..29a46c5d34 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
@@ -19,11 +19,7 @@ package org.apache.poi.hssf.usermodel;
 
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
-import java.util.Calendar;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 import org.apache.poi.hssf.model.HSSFFormulaParser;
 import org.apache.poi.hssf.model.Sheet;
@@ -56,6 +52,8 @@ import org.apache.poi.ss.usermodel.Hyperlink;
 import org.apache.poi.ss.usermodel.RichTextString;
 import org.apache.poi.ss.formula.FormulaType;
 import org.apache.poi.ss.SpreadsheetVersion;
+import org.apache.poi.util.POILogger;
+import org.apache.poi.util.POILogFactory;
 
 /**
  * High level representation of a cell in a row of a spreadsheet.
@@ -75,6 +73,7 @@ import org.apache.poi.ss.SpreadsheetVersion;
  * @author  Yegor Kozlov cell comments support
  */
 public class HSSFCell implements Cell {
+    private static POILogger log = POILogFactory.getLogger(HSSFCell.class);
 
     private static final String FILE_FORMAT_NAME  = "BIFF8";
     /**
@@ -980,7 +979,7 @@ public class HSSFCell implements Cell {
             return;
         }
 
-        comment.setRow((short)_record.getRow());
+        comment.setRow(_record.getRow());
         comment.setColumn(_record.getColumn());
         _comment = (HSSFComment)comment;
     }
@@ -1047,45 +1046,51 @@ public class HSSFCell implements Cell {
      *
      * @return cell comment or <code>null</code> if not found
      */
-    protected static HSSFComment findCellComment(Sheet sheet, int row, int column){
+    protected static HSSFComment findCellComment(Sheet sheet, int row, int column) {
         // TODO - optimise this code by searching backwards, find NoteRecord first, quit if not found. Find one TXO by id
         HSSFComment comment = null;
-        HashMap<Integer, TextObjectRecord> txshapesByShapeId = new HashMap<Integer, TextObjectRecord>(); 
-        for (Iterator<RecordBase> it = sheet.getRecords().iterator(); it.hasNext(); ) {
-           RecordBase rec = it.next();
-           if (rec instanceof NoteRecord){
-               NoteRecord note = (NoteRecord)rec;
-               if (note.getRow() == row && note.getColumn() == column){
-                   TextObjectRecord txo = txshapesByShapeId.get(new Integer(note.getShapeId()));
-                   comment = new HSSFComment(note, txo);
-                   comment.setRow(note.getRow());
-                   comment.setColumn((short)note.getColumn());
-                   comment.setAuthor(note.getAuthor());
-                   comment.setVisible(note.getFlags() == NoteRecord.NOTE_VISIBLE);
-                   comment.setString(txo.getStr());
-                   break;
-               }
-           } else if (rec instanceof ObjRecord){
-               ObjRecord obj = (ObjRecord)rec;
-               SubRecord sub = obj.getSubRecords().get(0);
-               if (sub instanceof CommonObjectDataSubRecord){
-                   CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord)sub;
-                   if (cmo.getObjectType() == CommonObjectDataSubRecord.OBJECT_TYPE_COMMENT){
-                       //find the nearest TextObjectRecord which holds comment's text and map it to its shapeId
-                       while(it.hasNext()) {
-                           rec = it.next();
-                           if (rec instanceof TextObjectRecord) {
-                               txshapesByShapeId.put(new Integer(cmo.getObjectId()), (TextObjectRecord)rec);
-                               break;
-                           }
-                       }
-
-                   }
-               }
-           }
+        ArrayList<TextObjectRecord> noteTxo = new ArrayList<TextObjectRecord>();
+        int i = 0;
+        for (Iterator<RecordBase> it = sheet.getRecords().iterator(); it.hasNext();) {
+            RecordBase rec = it.next();
+            if (rec instanceof NoteRecord) {
+                NoteRecord note = (NoteRecord) rec;
+                if (note.getRow() == row && note.getColumn() == column) {
+                    if(i < noteTxo.size()) {
+                        TextObjectRecord txo = noteTxo.get(i);
+                        comment = new HSSFComment(note, txo);
+                        comment.setRow(note.getRow());
+                        comment.setColumn((short) note.getColumn());
+                        comment.setAuthor(note.getAuthor());
+                        comment.setVisible(note.getFlags() == NoteRecord.NOTE_VISIBLE);
+                        comment.setString(txo.getStr());
+                    } else {
+                        log.log(POILogger.WARN, "Failed to match NoteRecord and TextObjectRecord, row: " + row + ", column: " + column);
+                    }
+                    break;
+                }
+                i++;
+            } else if (rec instanceof ObjRecord) {
+                ObjRecord obj = (ObjRecord) rec;
+                SubRecord sub = obj.getSubRecords().get(0);
+                if (sub instanceof CommonObjectDataSubRecord) {
+                    CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord) sub;
+                    if (cmo.getObjectType() == CommonObjectDataSubRecord.OBJECT_TYPE_COMMENT) {
+                        //find the next TextObjectRecord which holds the comment's text
+                        //the order of TXO matches the order of NoteRecords: i-th TXO record corresponds to the i-th NoteRecord
+                        while (it.hasNext()) {
+                            rec = it.next();
+                            if (rec instanceof TextObjectRecord) {
+                                noteTxo.add((TextObjectRecord) rec);
+                                break;
+                            }
+                        }
+                    }
+                }
+            }
         }
         return comment;
-   }
+    }
 
     /**
      * @return hyperlink associated with this cell or <code>null</code> if not found
diff --git a/src/testcases/org/apache/poi/hssf/record/TestCommonObjectDataSubRecord.java b/src/testcases/org/apache/poi/hssf/record/TestCommonObjectDataSubRecord.java
index 8e6e7ce561..7ff8623852 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestCommonObjectDataSubRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestCommonObjectDataSubRecord.java
@@ -56,7 +56,7 @@ public final class TestCommonObjectDataSubRecord extends TestCase {
 		CommonObjectDataSubRecord record = new CommonObjectDataSubRecord();
 
 		record.setObjectType(CommonObjectDataSubRecord.OBJECT_TYPE_LIST_BOX);
-		record.setObjectId((short) 1);
+		record.setObjectId( 1);
 		record.setOption((short) 1);
 		record.setLocked(true);
 		record.setPrintable(false);
diff --git a/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java b/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java
index 76e8788a5c..de53ecb3e9 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestObjRecord.java
@@ -83,7 +83,7 @@ public final class TestObjRecord extends TestCase {
         ObjRecord record = new ObjRecord();
         CommonObjectDataSubRecord ftCmo = new CommonObjectDataSubRecord();
         ftCmo.setObjectType( CommonObjectDataSubRecord.OBJECT_TYPE_COMMENT);
-        ftCmo.setObjectId( (short) 1024 );
+        ftCmo.setObjectId( 1024 );
         ftCmo.setLocked( true );
         ftCmo.setPrintable( true );
         ftCmo.setAutofill( true );