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

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>

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.setColumn(shape.getColumn());
-        note.setRow((short)shape.getRow());
+        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;
     }

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);

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);

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 );

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 );

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 );

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;
     }

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 );

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;
 	}
 

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.*;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
 
 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>(); 
+        ArrayList<TextObjectRecord> noteTxo = new ArrayList<TextObjectRecord>();
-        for (Iterator<RecordBase> it = sheet.getRecords().iterator(); it.hasNext(); ) {
+        int i = 0;
-           RecordBase rec = it.next();
+        for (Iterator<RecordBase> it = sheet.getRecords().iterator(); it.hasNext();) {
-           if (rec instanceof NoteRecord){
+            RecordBase rec = it.next();
-               NoteRecord note = (NoteRecord)rec;
+            if (rec instanceof NoteRecord) {
-               if (note.getRow() == row && note.getColumn() == column){
+                NoteRecord note = (NoteRecord) rec;
-                   TextObjectRecord txo = txshapesByShapeId.get(new Integer(note.getShapeId()));
+                if (note.getRow() == row && note.getColumn() == column) {
-                   comment = new HSSFComment(note, txo);
+                    if(i < noteTxo.size()) {
-                   comment.setRow(note.getRow());
+                        TextObjectRecord txo = noteTxo.get(i);
-                   comment.setColumn((short)note.getColumn());
+                        comment = new HSSFComment(note, txo);
-                   comment.setAuthor(note.getAuthor());
+                        comment.setRow(note.getRow());
-                   comment.setVisible(note.getFlags() == NoteRecord.NOTE_VISIBLE);
+                        comment.setColumn((short) note.getColumn());
-                   comment.setString(txo.getStr());
+                        comment.setAuthor(note.getAuthor());
-                   break;
+                        comment.setVisible(note.getFlags() == NoteRecord.NOTE_VISIBLE);
-               }
+                        comment.setString(txo.getStr());
-           } else if (rec instanceof ObjRecord){
+                    } else {
-               ObjRecord obj = (ObjRecord)rec;
+                        log.log(POILogger.WARN, "Failed to match NoteRecord and TextObjectRecord, row: " + row + ", column: " + column);
-               SubRecord sub = obj.getSubRecords().get(0);
+                    }
-               if (sub instanceof CommonObjectDataSubRecord){
+                    break;
-                   CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord)sub;
+                }
-                   if (cmo.getObjectType() == CommonObjectDataSubRecord.OBJECT_TYPE_COMMENT){
+                i++;
-                       //find the nearest TextObjectRecord which holds comment's text and map it to its shapeId
+            } else if (rec instanceof ObjRecord) {
-                       while(it.hasNext()) {
+                ObjRecord obj = (ObjRecord) rec;
-                           rec = it.next();
+                SubRecord sub = obj.getSubRecords().get(0);
-                           if (rec instanceof TextObjectRecord) {
+                if (sub instanceof CommonObjectDataSubRecord) {
-                               txshapesByShapeId.put(new Integer(cmo.getObjectId()), (TextObjectRecord)rec);
+                    CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord) sub;
-                               break;
+                    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

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);

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 );