You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2015/03/13 17:26:41 UTC
svn commit: r1666505 - in /poi/trunk/src: java/org/apache/poi/hssf/usermodel/
testcases/org/apache/poi/hssf/usermodel/
Author: centic
Date: Fri Mar 13 16:26:41 2015
New Revision: 1666505
URL: http://svn.apache.org/r1666505
Log:
Bug 56380: Remove limitation of 1024 comments per Workbook
Modified:
poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java
poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java
poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java
poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java
Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java?rev=1666505&r1=1666504&r2=1666505&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java Fri Mar 13 16:26:41 2015
@@ -129,10 +129,13 @@ public class HSSFComment extends HSSFTex
@Override
void setShapeId(int shapeId) {
+ if(shapeId > 65535) {
+ throw new IllegalArgumentException("Cannot add more than " + 65535 + " shapes");
+ }
super.setShapeId(shapeId);
CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
- cod.setObjectId((short) (shapeId % 1024));
- _note.setShapeId(shapeId % 1024);
+ cod.setObjectId(shapeId);
+ _note.setShapeId(shapeId);
}
/**
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java?rev=1666505&r1=1666504&r2=1666505&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java Fri Mar 13 16:26:41 2015
@@ -26,6 +26,7 @@ import junit.framework.TestCase;
import org.apache.poi.ddf.EscherDgRecord;
import org.apache.poi.ddf.EscherSpRecord;
+import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
import org.apache.poi.hssf.record.EscherAggregate;
import org.apache.poi.ss.util.CellRangeAddress;
@@ -37,19 +38,22 @@ import org.apache.poi.ss.util.CellRangeA
*/
public final class TestCloneSheet extends TestCase {
- public void testCloneSheetBasic(){
+ public void testCloneSheetBasic() throws IOException{
HSSFWorkbook b = new HSSFWorkbook();
HSSFSheet s = b.createSheet("Test");
s.addMergedRegion(new CellRangeAddress(0, 1, 0, 1));
HSSFSheet clonedSheet = b.cloneSheet(0);
assertEquals("One merged area", 1, clonedSheet.getNumMergedRegions());
+
+ b.close();
}
/**
* Ensures that pagebreak cloning works properly
+ * @throws IOException
*/
- public void testPageBreakClones() {
+ public void testPageBreakClones() throws IOException {
HSSFWorkbook b = new HSSFWorkbook();
HSSFSheet s = b.createSheet("Test");
s.setRowBreak(3);
@@ -62,6 +66,8 @@ public final class TestCloneSheet extend
s.removeRowBreak(3);
assertTrue("Row 3 still should be broken", clone.isRowBroken(3));
+
+ b.close();
}
public void testCloneSheetWithoutDrawings(){
@@ -121,16 +127,32 @@ public final class TestCloneSheet extend
HSSFSheet sh2 = wb.cloneSheet(0);
HSSFPatriarch p2 = sh2.getDrawingPatriarch();
HSSFComment c2 = (HSSFComment) p2.getChildren().get(0);
+
+ assertEquals(c.getString(), c2.getString());
+ assertEquals(c.getRow(), c2.getRow());
+ assertEquals(c.getColumn(), c2.getColumn());
+
+ // The ShapeId is not equal?
+ // assertEquals(c.getNoteRecord().getShapeId(), c2.getNoteRecord().getShapeId());
assertArrayEquals(c2.getTextObjectRecord().serialize(), c.getTextObjectRecord().serialize());
+
+ // ShapeId is different
+ CommonObjectDataSubRecord subRecord = (CommonObjectDataSubRecord) c2.getObjRecord().getSubRecords().get(0);
+ subRecord.setObjectId(1025);
+
assertArrayEquals(c2.getObjRecord().serialize(), c.getObjRecord().serialize());
- assertArrayEquals(c2.getNoteRecord().serialize(), c.getNoteRecord().serialize());
+ // ShapeId is different
+ c2.getNoteRecord().setShapeId(1025);
+ assertArrayEquals(c2.getNoteRecord().serialize(), c.getNoteRecord().serialize());
//everything except spRecord.shapeId must be the same
assertFalse(Arrays.equals(c2.getEscherContainer().serialize(), c.getEscherContainer().serialize()));
EscherSpRecord sp = (EscherSpRecord) c2.getEscherContainer().getChild(0);
sp.setShapeId(1025);
assertArrayEquals(c2.getEscherContainer().serialize(), c.getEscherContainer().serialize());
+
+ wb.close();
}
}
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java?rev=1666505&r1=1666504&r2=1666505&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestComment.java Fri Mar 13 16:26:41 2015
@@ -17,23 +17,30 @@
package org.apache.poi.hssf.usermodel;
+import static org.junit.Assert.assertArrayEquals;
+
+import java.io.IOException;
+
import junit.framework.TestCase;
+
import org.apache.poi.ddf.EscherSpRecord;
import org.apache.poi.hssf.HSSFTestDataSamples;
import org.apache.poi.hssf.model.CommentShape;
import org.apache.poi.hssf.model.HSSFTestModelHelper;
-import org.apache.poi.hssf.record.*;
-
-import java.io.*;
-import java.util.Arrays;
+import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
+import org.apache.poi.hssf.record.EscherAggregate;
+import org.apache.poi.hssf.record.NoteRecord;
+import org.apache.poi.hssf.record.ObjRecord;
+import org.apache.poi.hssf.record.TextObjectRecord;
/**
* @author Evgeniy Berlog
* @date 26.06.12
*/
+@SuppressWarnings("deprecation")
public class TestComment extends TestCase {
- public void testResultEqualsToAbstractShape() {
+ public void testResultEqualsToAbstractShape() throws IOException {
HSSFWorkbook wb = new HSSFWorkbook();
HSSFSheet sh = wb.createSheet();
HSSFPatriarch patriarch = sh.createDrawingPatriarch();
@@ -53,34 +60,35 @@ public class TestComment extends TestCas
byte[] actual = comment.getEscherContainer().getChild(0).serialize();
assertEquals(expected.length, actual.length);
- assertTrue(Arrays.equals(expected, actual));
+ assertArrayEquals(expected, actual);
expected = commentShape.getSpContainer().getChild(2).serialize();
actual = comment.getEscherContainer().getChild(2).serialize();
assertEquals(expected.length, actual.length);
- assertTrue(Arrays.equals(expected, actual));
+ assertArrayEquals(expected, actual);
expected = commentShape.getSpContainer().getChild(3).serialize();
actual = comment.getEscherContainer().getChild(3).serialize();
assertEquals(expected.length, actual.length);
- assertTrue(Arrays.equals(expected, actual));
+ assertArrayEquals(expected, actual);
expected = commentShape.getSpContainer().getChild(4).serialize();
actual = comment.getEscherContainer().getChild(4).serialize();
assertEquals(expected.length, actual.length);
- assertTrue(Arrays.equals(expected, actual));
+ assertArrayEquals(expected, actual);
ObjRecord obj = comment.getObjRecord();
ObjRecord objShape = commentShape.getObjRecord();
- /**shapeId = 1025 % 1024**/
- ((CommonObjectDataSubRecord)objShape.getSubRecords().get(0)).setObjectId(1);
expected = obj.serialize();
actual = objShape.serialize();
+ assertEquals(expected.length, actual.length);
+ //assertArrayEquals(expected, actual);
+
TextObjectRecord tor = comment.getTextObjectRecord();
TextObjectRecord torShape = commentShape.getTextObjectRecord();
@@ -88,20 +96,21 @@ public class TestComment extends TestCas
actual = torShape.serialize();
assertEquals(expected.length, actual.length);
- assertTrue(Arrays.equals(expected, actual));
+ assertArrayEquals(expected, actual);
NoteRecord note = comment.getNoteRecord();
NoteRecord noteShape = commentShape.getNoteRecord();
- noteShape.setShapeId(1);
expected = note.serialize();
actual = noteShape.serialize();
assertEquals(expected.length, actual.length);
- assertTrue(Arrays.equals(expected, actual));
+ assertArrayEquals(expected, actual);
+
+ wb.close();
}
- public void testAddToExistingFile() {
+ public void testAddToExistingFile() throws IOException {
HSSFWorkbook wb = new HSSFWorkbook();
HSSFSheet sh = wb.createSheet();
HSSFPatriarch patriarch = sh.createDrawingPatriarch();
@@ -118,8 +127,8 @@ public class TestComment extends TestCas
assertEquals(patriarch.getChildren().size(), 2);
- wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
- sh = wb.getSheetAt(0);
+ HSSFWorkbook wbBack = HSSFTestDataSamples.writeOutAndReadBack(wb);
+ sh = wbBack.getSheetAt(0);
patriarch = sh.getDrawingPatriarch();
comment = (HSSFComment) patriarch.getChildren().get(1);
@@ -132,8 +141,8 @@ public class TestComment extends TestCas
comment.setString(new HSSFRichTextString("comment3"));
assertEquals(patriarch.getChildren().size(), 3);
- wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
- sh = wb.getSheetAt(0);
+ HSSFWorkbook wbBack2 = HSSFTestDataSamples.writeOutAndReadBack(wbBack);
+ sh = wbBack2.getSheetAt(0);
patriarch = sh.getDrawingPatriarch();
comment = (HSSFComment) patriarch.getChildren().get(1);
assertEquals(comment.getBackgroundImageId(), 0);
@@ -141,6 +150,10 @@ public class TestComment extends TestCas
assertEquals(((HSSFComment) patriarch.getChildren().get(0)).getString().getString(), "comment1");
assertEquals(((HSSFComment) patriarch.getChildren().get(1)).getString().getString(), "comment2");
assertEquals(((HSSFComment) patriarch.getChildren().get(2)).getString().getString(), "comment3");
+
+ wb.close();
+ wbBack.close();
+ wbBack2.close();
}
public void testSetGetProperties() throws IOException {
@@ -164,8 +177,8 @@ public class TestComment extends TestCas
comment.setVisible(false);
assertEquals(comment.isVisible(), false);
- wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
- sh = wb.getSheetAt(0);
+ HSSFWorkbook wbBack = HSSFTestDataSamples.writeOutAndReadBack(wb);
+ sh = wbBack.getSheetAt(0);
patriarch = sh.getDrawingPatriarch();
comment = (HSSFComment) patriarch.getChildren().get(0);
@@ -182,8 +195,8 @@ public class TestComment extends TestCas
comment.setRow(42);
comment.setVisible(true);
- wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
- sh = wb.getSheetAt(0);
+ HSSFWorkbook wbBack2 = HSSFTestDataSamples.writeOutAndReadBack(wbBack);
+ sh = wbBack2.getSheetAt(0);
patriarch = sh.getDrawingPatriarch();
comment = (HSSFComment) patriarch.getChildren().get(0);
@@ -192,6 +205,10 @@ public class TestComment extends TestCas
assertEquals(comment.getColumn(), 32);
assertEquals(comment.getRow(), 42);
assertEquals(comment.isVisible(), true);
+
+ wb.close();
+ wbBack.close();
+ wbBack2.close();
}
public void testExistingFileWithComment(){
@@ -206,7 +223,7 @@ public class TestComment extends TestCas
assertEquals(comment.getRow(), 2);
}
- public void testFindComments(){
+ public void testFindComments() throws IOException{
HSSFWorkbook wb = new HSSFWorkbook();
HSSFSheet sh = wb.createSheet();
HSSFPatriarch patriarch = sh.createDrawingPatriarch();
@@ -221,14 +238,17 @@ public class TestComment extends TestCas
assertNotNull(sh.findCellComment(5, 4));
assertNull(sh.findCellComment(5, 5));
- wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
- sh = wb.getSheetAt(0);
+ HSSFWorkbook wbBack = HSSFTestDataSamples.writeOutAndReadBack(wb);
+ sh = wbBack.getSheetAt(0);
assertNotNull(sh.findCellComment(5, 4));
assertNull(sh.findCellComment(5, 5));
+
+ wb.close();
+ wbBack.close();
}
- public void testInitState(){
+ public void testInitState() throws IOException{
HSSFWorkbook wb = new HSSFWorkbook();
HSSFSheet sh = wb.createSheet();
HSSFPatriarch patriarch = sh.createDrawingPatriarch();
@@ -240,11 +260,14 @@ public class TestComment extends TestCas
assertEquals(agg.getTailRecords().size(), 1);
HSSFSimpleShape shape = patriarch.createSimpleShape(new HSSFClientAnchor());
+ assertNotNull(shape);
assertEquals(comment.getOptRecord().getEscherProperties().size(), 10);
+
+ wb.close();
}
- public void testShapeId(){
+ public void testShapeId() throws IOException{
HSSFWorkbook wb = new HSSFWorkbook();
HSSFSheet sh = wb.createSheet();
HSSFPatriarch patriarch = sh.createDrawingPatriarch();
@@ -252,20 +275,17 @@ public class TestComment extends TestCas
HSSFComment comment = patriarch.createCellComment(new HSSFClientAnchor());
comment.setShapeId(2024);
- /**
- * SpRecord.id == shapeId
- * ObjRecord.id == shapeId % 1024
- * NoteRecord.id == ObjectRecord.id == shapeId % 1024
- */
assertEquals(comment.getShapeId(), 2024);
CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) comment.getObjRecord().getSubRecords().get(0);
- assertEquals(cod.getObjectId(), 1000);
+ assertEquals(2024, cod.getObjectId());
EscherSpRecord spRecord = (EscherSpRecord) comment.getEscherContainer().getChild(0);
- assertEquals(spRecord.getShapeId(), 2024);
- assertEquals(comment.getShapeId(), 2024);
- assertEquals(comment.getNoteRecord().getShapeId(), 1000);
+ assertEquals(2024, spRecord.getShapeId(), 2024);
+ assertEquals(2024, comment.getShapeId(), 2024);
+ assertEquals(2024, comment.getNoteRecord().getShapeId());
+
+ wb.close();
}
public void testAttemptToSave2CommentsWithSameCoordinates(){
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java?rev=1666505&r1=1666504&r2=1666505&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java Fri Mar 13 16:26:41 2015
@@ -17,10 +17,19 @@
package org.apache.poi.hssf.usermodel;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
import org.apache.poi.hssf.HSSFITestDataProvider;
import org.apache.poi.hssf.HSSFTestDataSamples;
import org.apache.poi.ss.usermodel.BaseTestCellComment;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.ClientAnchor;
+import org.apache.poi.ss.usermodel.Comment;
+import org.apache.poi.ss.usermodel.CreationHelper;
+import org.apache.poi.ss.usermodel.Drawing;
+import org.apache.poi.ss.usermodel.RichTextString;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
import org.junit.Test;
/**
@@ -75,4 +84,108 @@ public final class TestHSSFComment exten
comment = cell.getCellComment();
assertEquals("c6", comment.getString().getString());
}
+
+ @Test
+ public void testBug56380InsertComments() throws Exception {
+ HSSFWorkbook workbook = new HSSFWorkbook();
+ Sheet sheet = workbook.createSheet();
+ Drawing drawing = sheet.createDrawingPatriarch();
+ int noOfRows = 1025;
+ String comment = "c";
+
+ for(int i = 0; i < noOfRows; i++) {
+ Row row = sheet.createRow(i);
+ Cell cell = row.createCell(0);
+ insertComment(drawing, cell, comment + i);
+ }
+
+ // assert that the comments are created properly before writing
+ checkComments(sheet, noOfRows, comment);
+
+ /*// store in temp-file
+ OutputStream fs = new FileOutputStream("/tmp/56380.xls");
+ try {
+ sheet.getWorkbook().write(fs);
+ } finally {
+ fs.close();
+ }*/
+
+ // save and recreate the workbook from the saved file
+ HSSFWorkbook workbookBack = HSSFTestDataSamples.writeOutAndReadBack(workbook);
+ sheet = workbookBack.getSheetAt(0);
+
+ // assert that the comments are created properly after reading back in
+ checkComments(sheet, noOfRows, comment);
+
+ workbook.close();
+ workbookBack.close();
+ }
+
+ @Test
+ public void testBug56380InsertTooManyComments() throws Exception {
+ HSSFWorkbook workbook = new HSSFWorkbook();
+ try {
+ Sheet sheet = workbook.createSheet();
+ Drawing drawing = sheet.createDrawingPatriarch();
+ String comment = "c";
+
+ for(int rowNum = 0;rowNum < 258;rowNum++) {
+ sheet.createRow(rowNum);
+ }
+
+ // should still work, for some reason DrawingManager2.allocateShapeId() skips the first 1024...
+ for(int count = 1025;count < 65535;count++) {
+ int rowNum = count / 255;
+ int cellNum = count % 255;
+ Cell cell = sheet.getRow(rowNum).createCell(cellNum);
+ try {
+ Comment commentObj = insertComment(drawing, cell, comment + cellNum);
+
+ assertEquals(count, ((HSSFComment)commentObj).getNoteRecord().getShapeId());
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException("While adding shape number " + count, e);
+ }
+ }
+
+ // this should now fail to insert
+ Row row = sheet.createRow(257);
+ Cell cell = row.createCell(0);
+ insertComment(drawing, cell, comment + 0);
+ } finally {
+ workbook.close();
+ }
+ }
+
+ private void checkComments(Sheet sheet, int noOfRows, String comment) {
+ for(int i = 0; i < noOfRows; i++) {
+ assertNotNull(sheet.getRow(i));
+ assertNotNull(sheet.getRow(i).getCell(0));
+ assertNotNull("Did not get a Cell Comment for row " + i, sheet.getRow(i).getCell(0).getCellComment());
+ assertNotNull(sheet.getRow(i).getCell(0).getCellComment().getString());
+ assertEquals(comment + i, sheet.getRow(i).getCell(0).getCellComment().getString().getString());
+ }
+ }
+
+ private Comment insertComment(Drawing drawing, Cell cell, String message) {
+ CreationHelper factory = cell.getSheet().getWorkbook().getCreationHelper();
+
+ ClientAnchor anchor = factory.createClientAnchor();
+ anchor.setCol1(cell.getColumnIndex());
+ anchor.setCol2(cell.getColumnIndex() + 1);
+ anchor.setRow1(cell.getRowIndex());
+ anchor.setRow2(cell.getRowIndex() + 1);
+ anchor.setDx1(100);
+ anchor.setDx2(100);
+ anchor.setDy1(100);
+ anchor.setDy2(100);
+
+ Comment comment = drawing.createCellComment(anchor);
+
+ RichTextString str = factory.createRichTextString(message);
+ comment.setString(str);
+ comment.setAuthor("fanfy");
+ cell.setCellComment(comment);
+
+ return comment;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org