From 98f64db1c75231a7c6ac1b3d1cf285dd9fd79fa9 Mon Sep 17 00:00:00 2001 From: Evgeniy Berlog Date: Tue, 14 Aug 2012 18:22:31 +0000 Subject: [PATCH] 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 --- .../poi/hssf/record/EscherAggregate.java | 2 +- .../poi/hssf/usermodel/HSSFPatriarch.java | 38 +++++++++++++++---- .../apache/poi/hssf/usermodel/HSSFRow.java | 1 + .../apache/poi/hssf/usermodel/HSSFSheet.java | 10 +++++ .../poi/hssf/usermodel/HSSFWorkbook.java | 1 + .../poi/hssf/usermodel/TestComment.java | 20 ++++++++++ .../poi/hssf/usermodel/TestHSSFRow.java | 17 +++++++++ 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 7579e71cf..9ca4fefc0 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 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 de3a123bc..3f634b02b 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 _shapes = new ArrayList(); 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 tailRecords = _boundAggregate.getTailRecords(); + /** + * contains coordinates of comments we iterate over + */ + Set coordinates = new HashSet(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 2397129bf..0c6a612fc 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 bb3c92ced..7fc488255 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java +++ b/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; diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index f1828fa38..add83e7e6 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 6c0919894..cac444bae 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 55ee03704..654db6aa0 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); + } }