Bug 57164 - XSSFDrawing.createCellComment() does not honor dx and dy values passed in

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

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFComment.java

@@ -16,6 +16,8 @@
 ==================================================================== */
 package org.apache.poi.xssf.usermodel;
 
+import static org.apache.poi.util.Units.EMU_PER_PIXEL;
+
 import java.math.BigInteger;
 
 import org.apache.poi.ss.usermodel.ClientAnchor;
@@ -182,7 +184,7 @@ public class XSSFComment implements Comment {
         for (String s : position.split(",")) {
             pos[i++] = Integer.parseInt(s.trim());
         }
-        XSSFClientAnchor ca = new XSSFClientAnchor(0, 0, 0, 0, pos[0], pos[2], pos[4], pos[6]);
+        XSSFClientAnchor ca = new XSSFClientAnchor(pos[1]*EMU_PER_PIXEL, pos[3]*EMU_PER_PIXEL, pos[5]*EMU_PER_PIXEL, pos[7]*EMU_PER_PIXEL, pos[0], pos[2], pos[4], pos[6]);
         return ca;
     }
 

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDrawing.java

@@ -35,6 +35,7 @@ import org.apache.poi.ss.usermodel.ClientAnchor;
 import org.apache.poi.ss.usermodel.Drawing;
 import org.apache.poi.ss.util.CellReference;
 import org.apache.poi.util.Internal;
+import org.apache.poi.util.Units;
 import org.apache.poi.xssf.model.CommentsTable;
 import org.apache.xmlbeans.XmlCursor;
 import org.apache.xmlbeans.XmlException;
@@ -215,7 +216,6 @@ public final class XSSFDrawing extends POIXMLDocumentPart implements Drawing {
      * @param pictureIndex the index of the picture in the workbook collection of pictures,
      *   {@link org.apache.poi.xssf.usermodel.XSSFWorkbook#getAllPictures()} .
      */
-    @SuppressWarnings("resource")
     protected PackageRelationship addPictureReference(int pictureIndex){
         XSSFWorkbook wb = (XSSFWorkbook)getParent().getParent();
         XSSFPictureData data = wb.getAllPictures().get(pictureIndex);
@@ -299,9 +299,17 @@ public final class XSSFDrawing extends POIXMLDocumentPart implements Drawing {
         XSSFVMLDrawing vml = sheet.getVMLDrawing(true);
         schemasMicrosoftComVml.CTShape vmlShape = vml.newCommentShape();
         if(ca.isSet()){
+            // convert offsets from emus to pixels since we get a DrawingML-anchor
+            // but create a VML Drawing
+            int dx1Pixels = ca.getDx1()/Units.EMU_PER_PIXEL;
+            int dy1Pixels = ca.getDy1()/Units.EMU_PER_PIXEL;
+            int dx2Pixels = ca.getDx2()/Units.EMU_PER_PIXEL;
+            int dy2Pixels = ca.getDy2()/Units.EMU_PER_PIXEL;
             String position =
-                    ca.getCol1() + ", 0, " + ca.getRow1() + ", 0, " +
+                    ca.getCol1() + ", " + dx1Pixels + ", " +
-                    ca.getCol2() + ", 0, " + ca.getRow2() + ", 0";
+                    ca.getRow1() + ", " + dy1Pixels + ", " +
+                    ca.getCol2() + ", " + dx2Pixels + ", " +
+                    ca.getRow2() + ", " + dy2Pixels;
             vmlShape.getClientDataArray(0).setAnchorArray(0, position);
         }
         String ref = new CellReference(ca.getRow1(), ca.getCol1()).formatAsString();

src/ooxml/testcases/org/apache/poi/xssf/model/TestCommentsTable.java

@@ -17,24 +17,43 @@
 
 package org.apache.poi.xssf.model;
 
-import junit.framework.TestCase;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
-import org.apache.poi.ss.usermodel.*;
+import java.io.IOException;
+
+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.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.xssf.XSSFTestDataSamples;
-import org.apache.poi.xssf.usermodel.*;
+import org.apache.poi.xssf.usermodel.XSSFClientAnchor;
+import org.apache.poi.xssf.usermodel.XSSFRichTextString;
+import org.apache.poi.xssf.usermodel.XSSFSheet;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.Test;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTComment;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCommentList;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTComments;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTRst;
 
 
-public class TestCommentsTable extends TestCase {
+public class TestCommentsTable {
 
 	private static final String TEST_A2_TEXT = "test A2 text";
 	private static final String TEST_A1_TEXT = "test A1 text";
 	private static final String TEST_AUTHOR = "test author";
 
-	public void testFindAuthor() throws Exception {
+	@Test
+	public void findAuthor() throws Exception {
 		CommentsTable sheetComments = new CommentsTable();
         assertEquals(1, sheetComments.getNumberOfAuthors());
         assertEquals(0, sheetComments.findAuthor(""));
@@ -47,7 +66,8 @@ public class TestCommentsTable extends TestCase {
 		assertEquals(2, sheetComments.findAuthor("another author"));
 	}
 
-	public void testGetCellComment() throws Exception {
+	@Test
+	public void getCellComment() throws Exception {
 		CommentsTable sheetComments = new CommentsTable();
 
 		CTComments comments = sheetComments.getCTComments();
@@ -72,7 +92,8 @@ public class TestCommentsTable extends TestCase {
 	}
 
 
-	public void testExisting() {
+	@Test
+	public void existing() {
 		Workbook workbook = XSSFTestDataSamples.openSampleWorkbook("WithVariousData.xlsx");
 		Sheet sheet1 = workbook.getSheetAt(0);
 		Sheet sheet2 = workbook.getSheetAt(1);
@@ -102,7 +123,8 @@ public class TestCommentsTable extends TestCase {
 		assertEquals(2, cc7.getColumn());
 	}
 
-	public void testWriteRead() {
+	@Test
+	public void writeRead() {
 		XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook("WithVariousData.xlsx");
 		XSSFSheet sheet1 = workbook.getSheetAt(0);
 		XSSFSheet sheet2 = workbook.getSheetAt(1);
@@ -150,7 +172,8 @@ public class TestCommentsTable extends TestCase {
 				sheet1.getRow(4).getCell(2).getCellComment().getString().getString());
 	}
 
-	public void testReadWriteMultipleAuthors() {
+	@Test
+	public void readWriteMultipleAuthors() {
 		XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook("WithMoreVariousData.xlsx");
 		XSSFSheet sheet1 = workbook.getSheetAt(0);
 		XSSFSheet sheet2 = workbook.getSheetAt(1);
@@ -185,7 +208,8 @@ public class TestCommentsTable extends TestCase {
 		// Todo - check text too, once bug fixed
 	}
 
-    public void testRemoveComment() throws Exception {
+	@Test
+	public void removeComment() throws Exception {
         CommentsTable sheetComments = new CommentsTable();
         CTComment a1 = sheetComments.newComment("A1");
         CTComment a2 = sheetComments.newComment("A2");
@@ -215,7 +239,8 @@ public class TestCommentsTable extends TestCase {
         assertNull(sheetComments.getCTComment("A3"));
     }
 
-    public void testBug54920() {
+	@Test
+    public void bug54920() throws IOException {
         final Workbook workbook = new XSSFWorkbook();
         final Sheet sheet = workbook.createSheet("sheet01");
         // create anchor
@@ -246,6 +271,8 @@ public class TestCommentsTable extends TestCase {
         commentA1 = A1.getCellComment();
         assertNotNull("Should still find the previous comment in A1, but had null", commentA1);
         assertEquals("should find correct comment in A1, but had null: " + commentA1, "for A1", commentA1.getString().getString());
+        
+        workbook.close();
     }
     
     // Set the comment on a sheet

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFComment.java

@@ -17,15 +17,20 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
+
 import org.apache.poi.hssf.usermodel.HSSFRichTextString;
 import org.apache.poi.ss.usermodel.BaseTestCellComment;
-import org.apache.poi.ss.usermodel.ClientAnchor;
 import org.apache.poi.ss.usermodel.IndexedColors;
 import org.apache.poi.ss.util.CellReference;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 import org.apache.poi.xssf.model.CommentsTable;
 import org.apache.xmlbeans.XmlObject;
+import org.junit.Test;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTComment;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTRPrElt;
 
@@ -45,7 +50,8 @@ public final class TestXSSFComment extends BaseTestCellComment  {
     /**
      * test properties of a newly constructed comment
      */
-    public void testConstructor() {
+    @Test
+    public void constructor() {
         CommentsTable sheetComments = new CommentsTable();
         assertNotNull(sheetComments.getCTComments().getCommentList());
         assertNotNull(sheetComments.getCTComments().getAuthors());
@@ -63,7 +69,8 @@ public final class TestXSSFComment extends BaseTestCellComment  {
         assertEquals(false, comment.isVisible());
     }
 
-    public void testGetSetCol() {
+    @Test
+    public void getSetCol() {
         CommentsTable sheetComments = new CommentsTable();
         XSSFVMLDrawing vml = new XSSFVMLDrawing();
         CTComment ctComment = sheetComments.newComment("A1");
@@ -81,7 +88,8 @@ public final class TestXSSFComment extends BaseTestCellComment  {
         assertEquals(5, vmlShape.getClientDataArray(0).getColumnArray(0).intValue());
     }
 
-    public void testGetSetRow() {
+    @Test
+    public void getSetRow() {
         CommentsTable sheetComments = new CommentsTable();
         XSSFVMLDrawing vml = new XSSFVMLDrawing();
         CTComment ctComment = sheetComments.newComment("A1");
@@ -99,7 +107,8 @@ public final class TestXSSFComment extends BaseTestCellComment  {
         assertEquals(5, vmlShape.getClientDataArray(0).getRowArray(0).intValue());
     }
 
-    public void testSetString() {
+    @Test
+    public void setString() {
         XSSFWorkbook wb = new XSSFWorkbook();
         XSSFSheet sh = wb.createSheet();
         XSSFComment comment = sh.createDrawingPatriarch().createCellComment(new XSSFClientAnchor());
@@ -145,14 +154,15 @@ public final class TestXSSFComment extends BaseTestCellComment  {
         //check that the rich text is set in the comment
         CTRPrElt rPr = richText.getCTRst().getRArray(0).getRPr();
         assertEquals(true, rPr.getIArray(0).getVal());
-        assertEquals(8.5, rPr.getSzArray(0).getVal());
+        assertEquals(8.5, rPr.getSzArray(0).getVal(), 0);
         assertEquals(IndexedColors.BLUE_GREY.getIndex(), rPr.getColorArray(0).getIndexed());
         assertEquals("Tahoma", rPr.getRFontArray(0).getVal());
         
         assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb));
     }
 
-    public void testAuthor() {
+    @Test
+    public void author() {
         CommentsTable sheetComments = new CommentsTable();
         CTComment ctComment = sheetComments.newComment("A1");
 
@@ -168,34 +178,4 @@ public final class TestXSSFComment extends BaseTestCellComment  {
         assertEquals("", comment.getAuthor());
         assertEquals(2, sheetComments.getNumberOfAuthors());
     }
-
-    public void testGetClientAnchor() {
-        XSSFWorkbook wb = new XSSFWorkbook();
-        XSSFSheet sheet = wb.createSheet();
-        XSSFDrawing drawing = sheet.createDrawingPatriarch();
-        XSSFComment comment;
-        ClientAnchor anchor;
-
-        comment = drawing.createCellComment(new XSSFClientAnchor(101, 102, 103, 104, 1, 2, 3, 4));
-        anchor = comment.getClientAnchor();
-        assertEquals(0, anchor.getDx1());
-        assertEquals(0, anchor.getDy1());
-        assertEquals(0, anchor.getDx2());
-        assertEquals(0, anchor.getDy2());
-        assertEquals(1, anchor.getCol1());
-        assertEquals(2, anchor.getRow1());
-        assertEquals(3, anchor.getCol2());
-        assertEquals(4, anchor.getRow2());
-
-        comment = drawing.createCellComment(new XSSFClientAnchor());
-        anchor = comment.getClientAnchor();
-        assertEquals(0, anchor.getDx1());
-        assertEquals(0, anchor.getDy1());
-        assertEquals(0, anchor.getDx2());
-        assertEquals(0, anchor.getDy2());
-        assertEquals(1, anchor.getCol1());
-        assertEquals(0, anchor.getRow1());
-        assertEquals(3, anchor.getCol2());
-        assertEquals(3, anchor.getRow2());
-    }
 }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java

@@ -16,10 +16,12 @@
 ==================================================================== */
 package org.apache.poi.hssf.usermodel;
 
+import static org.junit.Assert.assertEquals;
+
 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.ClientAnchor;
+import org.junit.Test;
 
 /**
  * Tests TestHSSFCellComment.
@@ -32,7 +34,8 @@ public final class TestHSSFComment extends BaseTestCellComment {
         super(HSSFITestDataProvider.instance);
     }
 
-    public void testDefaultShapeType() {
+    @Test
+    public void defaultShapeType() {
         HSSFComment comment = new HSSFComment((HSSFShape)null, new HSSFClientAnchor());
         assertEquals(HSSFSimpleShape.OBJECT_TYPE_COMMENT, comment.getShapeType());
     }
@@ -41,7 +44,8 @@ public final class TestHSSFComment extends BaseTestCellComment {
      *  HSSFCell#findCellComment should NOT rely on the order of records
      * when matching cells and their cell comments. The correct algorithm is to map
      */
-    public void test47924() {
+    @Test
+    public void bug47924() {
         HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("47924.xls");
         HSSFSheet sheet = wb.getSheetAt(0);
         HSSFCell cell;
@@ -71,34 +75,4 @@ public final class TestHSSFComment extends BaseTestCellComment {
         comment = cell.getCellComment();
         assertEquals("c6", comment.getString().getString());
     }
-
-    public void testGetClientAnchor() {
-        HSSFWorkbook wb = new HSSFWorkbook();
-        HSSFSheet sheet = wb.createSheet();
-        HSSFPatriarch drawing = sheet.createDrawingPatriarch();
-        HSSFComment comment;
-        ClientAnchor anchor;
-
-        comment = drawing.createCellComment(new HSSFClientAnchor(101, 102, 103, 104, (short) 1, 2, (short) 3, 4));
-        anchor = comment.getClientAnchor();
-        assertEquals(101, anchor.getDx1());
-        assertEquals(102, anchor.getDy1());
-        assertEquals(103, anchor.getDx2());
-        assertEquals(104, anchor.getDy2());
-        assertEquals(1, anchor.getCol1());
-        assertEquals(2, anchor.getRow1());
-        assertEquals(3, anchor.getCol2());
-        assertEquals(4, anchor.getRow2());
-
-        comment = drawing.createCellComment(new HSSFClientAnchor());
-        anchor = comment.getClientAnchor();
-        assertEquals(0, anchor.getDx1());
-        assertEquals(0, anchor.getDy1());
-        assertEquals(0, anchor.getDx2());
-        assertEquals(0, anchor.getDy2());
-        assertEquals(0, anchor.getCol1());
-        assertEquals(0, anchor.getRow1());
-        assertEquals(0, anchor.getCol2());
-        assertEquals(0, anchor.getRow2());
-    }
 }

src/testcases/org/apache/poi/ss/usermodel/BaseTestCellComment.java

@@ -17,15 +17,26 @@
 
 package org.apache.poi.ss.usermodel;
 
-import junit.framework.TestCase;
+import static org.apache.poi.util.Units.EMU_PER_PIXEL;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.ss.ITestDataProvider;
+import org.apache.poi.util.Units;
+import org.junit.Test;
 
 /**
  * Common superclass for testing implementations of
  * {@link Comment}
  */
-public abstract class BaseTestCellComment extends TestCase {
+public abstract class BaseTestCellComment {
 
     private final ITestDataProvider _testDataProvider;
 
@@ -33,7 +44,8 @@ public abstract class BaseTestCellComment extends TestCase {
         _testDataProvider = testDataProvider;
     }
 
-    public final void testFind() {
+    @Test
+    public final void find() {
         Workbook book = _testDataProvider.createWorkbook();
         Sheet sheet = book.createSheet();
         assertNull(sheet.getCellComment(0, 0));
@@ -44,7 +56,8 @@ public abstract class BaseTestCellComment extends TestCase {
         assertNull(cell.getCellComment());
     }
 
-    public final void testCreate() {
+    @Test
+    public final void create() {
         String cellText = "Hello, World";
         String commentText = "We can set comments in POI";
         String commentAuthor = "Apache Software Foundation";
@@ -118,7 +131,8 @@ public abstract class BaseTestCellComment extends TestCase {
     /**
      * test that we can read cell comments from an existing workbook.
      */
-    public final void testReadComments() {
+    @Test
+    public final void readComments() {
 
         Workbook wb = _testDataProvider.openSampleWorkbook("SimpleWithComments." + _testDataProvider.getStandardFileNameExtension());
 
@@ -154,7 +168,8 @@ public abstract class BaseTestCellComment extends TestCase {
     /**
      * test that we can modify existing cell comments
      */
-    public final void testModifyComments() {
+    @Test
+    public final void modifyComments() {
 
         Workbook wb = _testDataProvider.openSampleWorkbook("SimpleWithComments." + _testDataProvider.getStandardFileNameExtension());
         CreationHelper factory = wb.getCreationHelper();
@@ -186,7 +201,8 @@ public abstract class BaseTestCellComment extends TestCase {
         }
     }
 
-    public final void testDeleteComments() {
+    @Test
+    public final void deleteComments() {
         Workbook wb = _testDataProvider.openSampleWorkbook("SimpleWithComments." + _testDataProvider.getStandardFileNameExtension());
         Sheet sheet = wb.getSheetAt(0);
 
@@ -215,7 +231,8 @@ public abstract class BaseTestCellComment extends TestCase {
     /**
      * code from the quick guide
      */
-    public void testQuickGuide(){
+    @Test
+    public void quickGuide(){
         Workbook wb = _testDataProvider.createWorkbook();
 
         CreationHelper factory = wb.getCreationHelper();
@@ -245,4 +262,77 @@ public abstract class BaseTestCellComment extends TestCase {
         assertEquals(3, comment.getRow());
         assertEquals(5, comment.getColumn());
     }
+
+    @Test
+    public void getClientAnchor() throws IOException {
+        Workbook wb = _testDataProvider.createWorkbook();
+        
+        Sheet sheet = wb.createSheet();
+        Row row = sheet.createRow(10);
+        Cell cell = row.createCell(5);
+        CreationHelper factory = wb.getCreationHelper();
+        
+        Drawing drawing = sheet.createDrawingPatriarch();
+        
+        double r_mul, c_mul;
+        if (sheet instanceof HSSFSheet) {
+            double rowheight = Units.toEMU(row.getHeightInPoints())/EMU_PER_PIXEL;
+            r_mul = 256.0/rowheight;
+            double colwidth = sheet.getColumnWidthInPixels(2);
+            c_mul = 1024.0/colwidth;
+        } else {
+            r_mul = c_mul = EMU_PER_PIXEL;
+        }
+
+        int dx1 = (int)Math.round(10*c_mul);
+        int dy1 = (int)Math.round(10*r_mul);
+        int dx2 = (int)Math.round(3*c_mul);
+        int dy2 = (int)Math.round(4*r_mul);
+        int col1 = cell.getColumnIndex()+1;
+        int row1 = row.getRowNum();
+        int col2 = cell.getColumnIndex()+2;
+        int row2 = row.getRowNum()+1;
+        
+        ClientAnchor anchor = drawing.createAnchor(dx1, dy1, dx2, dy2, col1, row1, col2, row2);
+        Comment comment = drawing.createCellComment(anchor);
+        comment.setVisible(true);
+        cell.setCellComment(comment);
+        
+        anchor = comment.getClientAnchor();
+        assertEquals(dx1, anchor.getDx1());
+        assertEquals(dy1, anchor.getDy1());
+        assertEquals(dx2, anchor.getDx2());
+        assertEquals(dy2, anchor.getDy2());
+        assertEquals(col1, anchor.getCol1());
+        assertEquals(row1, anchor.getRow1());
+        assertEquals(col2, anchor.getCol2());
+        assertEquals(row2, anchor.getRow2());
+
+        anchor = factory.createClientAnchor();
+        comment = drawing.createCellComment(anchor);
+        cell.setCellComment(comment);
+        anchor = comment.getClientAnchor();
+        
+        if (sheet instanceof HSSFSheet) {
+            assertEquals(0, anchor.getCol1());
+            assertEquals(0, anchor.getDx1());
+            assertEquals(0, anchor.getRow1());
+            assertEquals(0, anchor.getDy1());
+            assertEquals(0, anchor.getCol2());
+            assertEquals(0, anchor.getDx2());
+            assertEquals(0, anchor.getRow2());
+            assertEquals(0, anchor.getDy2());            
+        } else {
+            // when anchor is initialized without parameters, the comment anchor attributes default to
+            // "1, 15, 0, 2, 3, 15, 3, 16" ... see XSSFVMLDrawing.newCommentShape()
+            assertEquals( 1, anchor.getCol1());
+            assertEquals(15*EMU_PER_PIXEL, anchor.getDx1());
+            assertEquals( 0, anchor.getRow1());
+            assertEquals( 2*EMU_PER_PIXEL, anchor.getDy1());
+            assertEquals( 3, anchor.getCol2());
+            assertEquals(15*EMU_PER_PIXEL, anchor.getDx2());
+            assertEquals( 3, anchor.getRow2());
+            assertEquals(16*EMU_PER_PIXEL, anchor.getDy2());
+        }
+    }
 }