Bug 56017: fix shifting comments with shifted rows and move some more tests into the base classes to run them against XSSF, SXSSF and HSSF

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

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

@@ -145,7 +145,15 @@ public class XSSFComment implements Comment {
 		_comment.setRef(newRef);
       _comments.referenceUpdated(oldRef, _comment);
       
-        if(_vmlShape != null) _vmlShape.getClientDataArray(0).setRowArray(0, new BigInteger(String.valueOf(row)));
+        if(_vmlShape != null) {
+        	_vmlShape.getClientDataArray(0).setRowArray(0, 
+        			new BigInteger(String.valueOf(row)));
+        	
+            // There is a very odd xmlbeans bug when changing the row
+            //  arrays which can lead to corrupt pointer
+            // This call seems to fix them again... See bug #50795
+            _vmlShape.getClientDataList().toString();
+        }
     }
     
     /**

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

@@ -25,11 +25,13 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
 import java.util.TreeMap;
 
 import javax.xml.namespace.QName;
@@ -131,7 +133,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
     protected CTSheet sheet;
     protected CTWorksheet worksheet;
 
-    private TreeMap<Integer, XSSFRow> _rows;
+    private SortedMap<Integer, XSSFRow> _rows;
     private List<XSSFHyperlink> hyperlinks;
     private ColumnHelper columnHelper;
     private CommentsTable sheetComments;
@@ -140,7 +142,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      * Master shared formula is the first formula in a group of shared formulas is saved in the f element.
      */
     private Map<Integer, CTCellFormula> sharedFormulas;
-    private TreeMap<String,XSSFTable> tables;
+    private SortedMap<String,XSSFTable> tables;
     private List<CellRangeAddress> arrayFormulas;
     private XSSFDataValidationHelper dataValidationHelper;
 
@@ -2555,38 +2557,87 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      */
     @Override
     @SuppressWarnings("deprecation") //YK: getXYZArray() array accessors are deprecated in xmlbeans with JDK 1.5 support
-    public void shiftRows(int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight) {
+    public void shiftRows(int startRow, int endRow, final int n, boolean copyRowHeight, boolean resetOriginalRowHeight) {
-        // first remove all rows which will be overwritten
+    	XSSFVMLDrawing vml = getVMLDrawing(false);
+
+    	// first remove all rows which will be overwritten
         for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
             XSSFRow row = (XSSFRow)it.next();
             int rownum = row.getRowNum();
 
             // check if we should remove this row as it will be overwritten by the data later
-            if (removeRow(startRow, endRow, n, rownum)) {
+            if (shouldRemoveRow(startRow, endRow, n, rownum)) {
                 // remove row from worksheet.getSheetData row array
                 int idx = _rows.headMap(row.getRowNum()).size();
                 worksheet.getSheetData().removeRow(idx);
+
                 // remove row from _rows
                 it.remove();
+
+                // also remove any comments associated with this row
+                if(sheetComments != null){
+                    CTCommentList lst = sheetComments.getCTComments().getCommentList();
+                    for (CTComment comment : lst.getCommentArray()) {
+                    	String strRef = comment.getRef();
+                    	CellReference ref = new CellReference(strRef);
+
+                    	// is this comment part of the current row?
+                    	if(ref.getRow() == rownum) {
+                            sheetComments.removeComment(strRef);
+                            vml.removeCommentShape(ref.getRow(), ref.getCol());
+                    	}
+                    }
+                }
             }
         }
 
         // then do the actual moving and also adjust comments/rowHeight
+        // we need to sort it in a way so the shifting does not mess up the structures, 
+        // i.e. when shifting down, start from down and go up, when shifting up, vice-versa
+        SortedMap<XSSFComment, Integer> commentsToShift = new TreeMap<XSSFComment, Integer>(new Comparator<XSSFComment>() {
+			public int compare(XSSFComment o1, XSSFComment o2) {
+				int row1 = new CellReference(o1.getCTComment().getRef()).getRow();
+				int row2 = new CellReference(o2.getCTComment().getRef()).getRow();
+				
+				if(row1 == row2) {
+					return 0;
+				}
+
+				// when shifting down, sort higher row-values first
+				if(n > 0) {
+					return row1 < row2 ? 1 : -1;
+				} else {
+					// sort lower-row values first when shifting up
+					return row1 > row2 ? 1 : -1;
+				}
+			}
+		});
+        
         for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
             XSSFRow row = (XSSFRow)it.next();
             int rownum = row.getRowNum();
 
             if(sheetComments != null){
-                //TODO shift Note's anchor in the associated /xl/drawing/vmlDrawings#.vml
+            	// calculate the new rownum
-                CTCommentList lst = sheetComments.getCTComments().getCommentList();
+            	int newrownum = shiftedRowNum(startRow, endRow, n, rownum);
-                for (CTComment comment : lst.getCommentArray()) {
+            	
-                    String oldRef = comment.getRef();
+            	// is there a change necessary for the current row?
-                    CellReference ref = new CellReference(oldRef);
+            	if(newrownum != rownum) {
-                    if(ref.getRow() == rownum){
+                    CTCommentList lst = sheetComments.getCTComments().getCommentList();
-                        ref = new CellReference(rownum + n, ref.getCol());
+                    for (CTComment comment : lst.getCommentArray()) {
-                        comment.setRef(ref.formatAsString());
+                    	String oldRef = comment.getRef();
-                        sheetComments.referenceUpdated(oldRef, comment);
+                    	CellReference ref = new CellReference(oldRef);
-                    }
+                    	
+                    	// is this comment part of the current row?
+                    	if(ref.getRow() == rownum) {
+                        	XSSFComment xssfComment = new XSSFComment(sheetComments, comment,
+                                    vml == null ? null : vml.findCommentShape(rownum, ref.getCol()));
+                        	
+                        	// we should not perform the shifting right here as we would then find
+                        	// already shifted comments and would shift them again...
+                        	commentsToShift.put(xssfComment, newrownum);
+                        }
+                	}
                 }
             }
 
@@ -2598,6 +2649,14 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
 
             row.shift(n);
         }
+        
+        // adjust all the affected comment-structures now
+        // the Map is sorted and thus provides them in the order that we need here, 
+        // i.e. from down to up if shifting down, vice-versa otherwise
+        for(Map.Entry<XSSFComment, Integer> entry : commentsToShift.entrySet()) {
+        	entry.getKey().setRow(entry.getValue());
+        }
+        
         XSSFRowShifter rowShifter = new XSSFRowShifter(this);
 
         int sheetIndex = getWorkbook().getSheetIndex(this);
@@ -2611,13 +2670,40 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         rowShifter.updateConditionalFormatting(shifter);
 
         //rebuild the _rows map
-        TreeMap<Integer, XSSFRow> map = new TreeMap<Integer, XSSFRow>();
+        SortedMap<Integer, XSSFRow> map = new TreeMap<Integer, XSSFRow>();
         for(XSSFRow r : _rows.values()) {
             map.put(r.getRowNum(), r);
         }
         _rows = map;
     }
 
+    private int shiftedRowNum(int startRow, int endRow, int n, int rownum) {
+		// no change if before any affected row
+    	if(rownum < startRow && (n > 0 || (startRow - rownum) > n)) {
+			return rownum;
+		}
+		
+    	// no change if after any affected row
+    	if(rownum > endRow && (n < 0 || (rownum - endRow) > n)) {
+    		return rownum;
+    	}
+    	
+    	// row before and things are moved up
+    	if(rownum < startRow) {
+    		// row is moved down by the shifting
+    		return rownum + (endRow - startRow);
+    	}
+    	
+    	// row is after and things are moved down
+    	if(rownum > endRow) {
+    		// row is moved up by the shifting
+    		return rownum - (endRow - startRow);
+    	}
+    	
+    	// row is part of the shifted block
+		return rownum + n;
+	}
+
     /**
      * Location of the top left visible cell Location of the top left visible cell in the bottom right
      * pane (when in Left-to-Right mode).
@@ -2873,7 +2959,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         return sheetPr.isSetPageSetUpPr() ? sheetPr.getPageSetUpPr() : sheetPr.addNewPageSetUpPr();
     }
 
-    private boolean removeRow(int startRow, int endRow, int n, int rownum) {
+    private boolean shouldRemoveRow(int startRow, int endRow, int n, int rownum) {
         // is this row in the target-window where the moved rows will land?
         if (rownum >= (startRow + n) && rownum <= (endRow + n)) {
             // only remove it if the current row is not part of the data that is copied

src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFSheet.java

@@ -89,6 +89,13 @@ public class TestSXSSFSheet extends BaseTestSheet {
         super.bug35084();
     }
 
+    @Test
+    public void getCellComment() throws IOException {
+    	// TODO: reading cell comments via Sheet does not work currently as it tries 
+    	// to access the underlying sheet for this, but comments are stored as
+    	// properties on Cells...
+    }
+    
     @Override
     @Test
     public void defaultColumnStyle() {

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

@@ -36,21 +36,17 @@ import org.apache.poi.poifs.crypt.HashAlgorithm;
 import org.apache.poi.ss.usermodel.AutoFilter;
 import org.apache.poi.ss.usermodel.BaseTestSheet;
 import org.apache.poi.ss.usermodel.Cell;
-import org.apache.poi.ss.usermodel.CreationHelper;
-import org.apache.poi.ss.usermodel.RichTextString;
 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.ss.util.AreaReference;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.ss.util.CellReference;
-import org.apache.poi.xssf.SXSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
 import org.apache.poi.xssf.model.CalculationChain;
 import org.apache.poi.xssf.model.CommentsTable;
 import org.apache.poi.xssf.model.StylesTable;
-import org.apache.poi.xssf.streaming.SXSSFSheet;
 import org.apache.poi.xssf.streaming.SXSSFWorkbook;
 import org.apache.poi.xssf.usermodel.helpers.ColumnHelper;
 import org.junit.Test;
@@ -60,8 +56,6 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.*;
 @SuppressWarnings("resource")
 public final class TestXSSFSheet extends BaseTestSheet {
 
-    private static final int ROW_COUNT = 40000;
-
     public TestXSSFSheet() {
         super(XSSFITestDataProvider.instance);
     }
@@ -188,39 +182,6 @@ public final class TestXSSFSheet extends BaseTestSheet {
         assertTrue(col.getBestFit());
     }
 
-    /**
-     * XSSFSheet autoSizeColumn() on empty RichTextString fails
-     */
-    @Test
-    public void bug48325() {
-        XSSFWorkbook wb = new XSSFWorkbook();
-        XSSFSheet sheet = wb.createSheet("Test");
-        CreationHelper factory = wb.getCreationHelper();
-
-        XSSFRow row = sheet.createRow(0);
-        XSSFCell cell = row.createCell(0);
-
-        XSSFFont font = wb.createFont();
-        RichTextString rts = factory.createRichTextString("");
-        rts.applyFont(font);
-        cell.setCellValue(rts);
-
-        sheet.autoSizeColumn(0);
-    }
-
-    @Test
-    public void getCellComment() {
-        XSSFWorkbook workbook = new XSSFWorkbook();
-        XSSFSheet sheet = workbook.createSheet();
-        XSSFDrawing dg = sheet.createDrawingPatriarch();
-        XSSFComment comment = dg.createCellComment(new XSSFClientAnchor());
-        XSSFCell cell = sheet.createRow(9).createCell(2);
-        comment.setAuthor("test C10 author");
-        cell.setCellComment(comment);
-
-        assertNotNull(sheet.getCellComment(9, 2));
-        assertEquals("test C10 author", sheet.getCellComment(9, 2).getAuthor());
-    }
 
     @Test
     public void setCellComment() {
@@ -268,16 +229,6 @@ public final class TestXSSFSheet extends BaseTestSheet {
         assertEquals(STPane.BOTTOM_RIGHT, ctWorksheet.getSheetViews().getSheetViewArray(0).getPane().getActivePane());
     }
 
-    @Test
-    public void newMergedRegionAt() {
-        XSSFWorkbook workbook = new XSSFWorkbook();
-        XSSFSheet sheet = workbook.createSheet();
-        CellRangeAddress region = CellRangeAddress.valueOf("B2:D4");
-        sheet.addMergedRegion(region);
-        assertEquals("B2:D4", sheet.getMergedRegion(0).formatAsString());
-        assertEquals(1, sheet.getNumMergedRegions());
-    }
-
     @Test
     public void removeMergedRegion_lowlevel() {
         XSSFWorkbook workbook = new XSSFWorkbook();
@@ -1244,53 +1195,6 @@ public final class TestXSSFSheet extends BaseTestSheet {
         }
     }
 
-    @Test
-    public void showInPaneManyRowsBug55248() {
-        XSSFWorkbook workbook = new XSSFWorkbook();
-        XSSFSheet sheet = workbook.createSheet("Sheet 1");
-
-        sheet.showInPane(0, 0);
-
-        for(int i = ROW_COUNT/2;i < ROW_COUNT;i++) {
-            sheet.createRow(i);
-            sheet.showInPane(i, 0);
-            // this one fails: sheet.showInPane((short)i, 0);
-        }
-
-        int i = 0;
-        sheet.showInPane(i, i);
-
-        XSSFWorkbook wb = XSSFTestDataSamples.writeOutAndReadBack(workbook);
-        checkRowCount(wb);
-    }
-
-    @Test
-    public void showInPaneManyRowsBug55248SXSSF() {
-        SXSSFWorkbook workbook = new SXSSFWorkbook(new XSSFWorkbook());
-        SXSSFSheet sheet = (SXSSFSheet) workbook.createSheet("Sheet 1");
-
-        sheet.showInPane(0, 0);
-
-        for(int i = ROW_COUNT/2;i < ROW_COUNT;i++) {
-            sheet.createRow(i);
-            sheet.showInPane(i, 0);
-            // this one fails: sheet.showInPane((short)i, 0);
-        }
-
-        int i = 0;
-        sheet.showInPane(i, i);
-
-        Workbook wb = SXSSFITestDataProvider.instance.writeOutAndReadBack(workbook);
-        checkRowCount(wb);
-    }
-
-    private void checkRowCount(Workbook wb) {
-        assertNotNull(wb);
-        final Sheet sh = wb.getSheet("Sheet 1");
-        assertNotNull(sh);
-        assertEquals(ROW_COUNT-1, sh.getLastRowNum());
-    }
-
     @Test
     public void bug55745() throws Exception {
         XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("55745.xlsx");
@@ -1472,16 +1376,4 @@ public final class TestXSSFSheet extends BaseTestSheet {
         XSSFSheet sheet = wb.createSheet();
         assertNotNull(sheet.createComment());
     }
-    
-    @Test
-    public void testRightToLeft() {
-        XSSFWorkbook wb = new XSSFWorkbook();
-        XSSFSheet sheet = wb.createSheet();
-
-        assertFalse(sheet.isRightToLeft());
-        sheet.setRightToLeft(true);
-        assertTrue(sheet.isRightToLeft());
-        sheet.setRightToLeft(false);
-        assertFalse(sheet.isRightToLeft());
-    }
 }

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

@@ -25,7 +25,6 @@ import org.apache.poi.ss.usermodel.Comment;
 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.ss.util.CellRangeAddress;
 import org.apache.poi.ss.util.CellUtil;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
@@ -44,11 +43,6 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows {
         // TODO - support shifting of page breaks
     }
 
-    @Override
-	public void testShiftWithComments() { // disabled test from superclass
-        // TODO - support shifting of comments.
-    }
-
 	public void testBug54524() throws IOException {
         XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook("54524.xlsx");
         XSSFSheet sheet = workbook.getSheetAt(0);
@@ -62,7 +56,6 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows {
 		assertEquals("X", cell.getStringCellValue());
 	}
 
-
 	public void testBug53798() throws IOException {
 		// NOTE that for HSSF (.xls) negative shifts combined with positive ones do work as expected  
 		Workbook wb = XSSFTestDataSamples.openSampleWorkbook("53798.xlsx");
@@ -189,19 +182,6 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows {
         assertEquals("Amdocs:\ntest\n", comment.getString().getString());
 	}
 
-	public void testBug55280() throws IOException {
-        Workbook w = new XSSFWorkbook();
-        try {
-            Sheet s = w.createSheet();
-            for (int row = 0; row < 5000; ++row)
-                s.addMergedRegion(new CellRangeAddress(row, row, 0, 3));
-
-            s.shiftRows(0, 4999, 1);        // takes a long time...
-        } finally {
-            w.close();
-        }
-	}
-
     public void test57171() throws Exception {
 	    Workbook wb = XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx");
         assertEquals(5, wb.getActiveSheetIndex());

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

@@ -17,17 +17,13 @@
 
 package org.apache.poi.ss.usermodel;
 
-import static org.junit.Assert.assertEquals;
+import static junit.framework.TestCase.assertNotNull;
-import static org.junit.Assert.assertFalse;
+import static junit.framework.TestCase.assertTrue;
-import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
+import java.io.IOException;
 import java.util.Iterator;
 
-import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.util.PaneInformation;
 import org.apache.poi.ss.ITestDataProvider;
 import org.apache.poi.ss.SpreadsheetVersion;
@@ -41,13 +37,15 @@ import org.junit.rules.ExpectedException;
  * {@link org.apache.poi.hssf.usermodel.HSSFCell}
  */
 public abstract class BaseTestSheet {
+    private static final int ROW_COUNT = 40000;
+
     @Rule
     public ExpectedException thrown = ExpectedException.none();
     
     private final ITestDataProvider _testDataProvider;
 
     protected BaseTestSheet(ITestDataProvider testDataProvider) {
-    _testDataProvider = testDataProvider;
+    	_testDataProvider = testDataProvider;
     }
 
     @Test
@@ -851,22 +849,126 @@ public abstract class BaseTestSheet {
     }
 
     @Test
-    public void bug55723_Rows() {
+    public void bug55723_Rows() throws IOException {
-        HSSFWorkbook wb = new HSSFWorkbook();
+        Workbook wb = _testDataProvider.createWorkbook();
         Sheet sheet = wb.createSheet();
 
         CellRangeAddress range = CellRangeAddress.valueOf("A4:B55000");
         AutoFilter filter = sheet.setAutoFilter(range);
         assertNotNull(filter);
+        
+        wb.close();
     }
 
     @Test
-    public void bug55723d_RowsOver65k() {
+    public void bug55723d_RowsOver65k() throws IOException {
-        HSSFWorkbook wb = new HSSFWorkbook();
+        Workbook wb = _testDataProvider.createWorkbook();
         Sheet sheet = wb.createSheet();
 
         CellRangeAddress range = CellRangeAddress.valueOf("A4:B75000");
         AutoFilter filter = sheet.setAutoFilter(range);
         assertNotNull(filter);
+        
+        wb.close();
+    }
+
+    /**
+     * XSSFSheet autoSizeColumn() on empty RichTextString fails
+     * @throws IOException 
+     */
+    @Test
+    public void bug48325() throws IOException {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("Test");
+        CreationHelper factory = wb.getCreationHelper();
+
+        Row row = sheet.createRow(0);
+        Cell cell = row.createCell(0);
+
+        Font font = wb.createFont();
+        RichTextString rts = factory.createRichTextString("");
+        rts.applyFont(font);
+        cell.setCellValue(rts);
+
+        sheet.autoSizeColumn(0);
+        
+        assertNotNull(_testDataProvider.writeOutAndReadBack(wb));
+        
+        wb.close();
+    }
+
+    @Test
+    public void getCellComment() throws IOException {
+        Workbook workbook = _testDataProvider.createWorkbook();
+        Sheet sheet = workbook.createSheet();
+        Drawing dg = sheet.createDrawingPatriarch();
+        Comment comment = dg.createCellComment(workbook.getCreationHelper().createClientAnchor());
+        Cell cell = sheet.createRow(9).createCell(2);
+        comment.setAuthor("test C10 author");
+        cell.setCellComment(comment);
+
+        assertNotNull(sheet.getCellComment(9, 2));
+        assertEquals("test C10 author", sheet.getCellComment(9, 2).getAuthor());
+        
+        assertNotNull(_testDataProvider.writeOutAndReadBack(workbook));
+        
+        workbook.close();
+    }
+
+
+    @Test
+    public void newMergedRegionAt() throws IOException {
+        Workbook workbook = _testDataProvider.createWorkbook();
+        Sheet sheet = workbook.createSheet();
+        CellRangeAddress region = CellRangeAddress.valueOf("B2:D4");
+        sheet.addMergedRegion(region);
+        assertEquals("B2:D4", sheet.getMergedRegion(0).formatAsString());
+        assertEquals(1, sheet.getNumMergedRegions());
+        
+        assertNotNull(_testDataProvider.writeOutAndReadBack(workbook));
+        
+        workbook.close();
+    }
+
+    @Test
+    public void showInPaneManyRowsBug55248() {
+        Workbook workbook = _testDataProvider.createWorkbook();
+        Sheet sheet = workbook.createSheet("Sheet 1");
+
+        sheet.showInPane(0, 0);
+
+        for(int i = ROW_COUNT/2;i < ROW_COUNT;i++) {
+            sheet.createRow(i);
+            sheet.showInPane(i, 0);
+            // this one fails: sheet.showInPane((short)i, 0);
+        }
+
+        int i = 0;
+        sheet.showInPane(i, i);
+
+        Workbook wb = _testDataProvider.writeOutAndReadBack(workbook);
+        checkRowCount(wb);
+    }
+
+    private void checkRowCount(Workbook wb) {
+        assertNotNull(wb);
+        final Sheet sh = wb.getSheet("Sheet 1");
+        assertNotNull(sh);
+        assertEquals(ROW_COUNT-1, sh.getLastRowNum());
+    }
+    
+    
+    @Test
+    public void testRightToLeft() throws IOException {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet();
+
+        assertFalse(sheet.isRightToLeft());
+        sheet.setRightToLeft(true);
+        assertTrue(sheet.isRightToLeft());
+        sheet.setRightToLeft(false);
+        assertFalse(sheet.isRightToLeft());
+        
+        wb.close();
     }
 }

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

@@ -17,8 +17,11 @@
 
 package org.apache.poi.ss.usermodel;
 
+import java.io.IOException;
+
 import junit.framework.TestCase;
 
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.ss.ITestDataProvider;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.ss.util.CellReference;
@@ -134,8 +137,7 @@ public abstract class BaseTestSheetShiftRows extends TestCase {
         assertTrue("Row number 6 should have a pagebreak", s.isRowBroken(6));
     }
 
-    public void testShiftWithComments() { // TODO - enable XSSF test
+    public void testShiftWithComments() {
-
         Workbook wb = _testDataProvider.openSampleWorkbook("comments." + _testDataProvider.getStandardFileNameExtension());
 
         Sheet sheet = wb.getSheet("Sheet1");
@@ -154,6 +156,8 @@ public abstract class BaseTestSheetShiftRows extends TestCase {
         String comment4 = sheet.getCellComment(3,0).getString().getString();
         assertEquals(comment4,"comment top row4 (index3)\n");
 
+        //Workbook wbBack = _testDataProvider.writeOutAndReadBack(wb);
+
         // Shifting all but first line down to test comments shifting
         sheet.shiftRows(1, sheet.getLastRowNum(), 1, true, true);
 
@@ -191,6 +195,26 @@ public abstract class BaseTestSheetShiftRows extends TestCase {
         assertEquals(comment3,comment3_shifted);
         comment4_shifted = sheet.getCellComment(4,0).getString().getString();
         assertEquals(comment4,comment4_shifted);
+
+        // Shifting back up again, now two rows
+        sheet.shiftRows(2, sheet.getLastRowNum(), -2, true, true);
+
+        // TODO: it seems HSSFSheet does not correctly remove comments from rows that are overwritten
+        // by shifting rows...
+        if(!(wb instanceof HSSFWorkbook)) {
+        	assertEquals(2, sheet.getLastRowNum());
+        	
+        	// Verify comments are in the position expected
+        	assertNull("Had: " + (sheet.getCellComment(0,0) == null ? "null" : sheet.getCellComment(0,0).getString()),
+        			sheet.getCellComment(0,0));
+        	assertNotNull(sheet.getCellComment(1,0));
+        	assertNotNull(sheet.getCellComment(2,0));
+        }
+
+        comment1 = sheet.getCellComment(1,0).getString().getString();
+        assertEquals(comment1,"comment top row3 (index2)\n");
+        String comment2 = sheet.getCellComment(2,0).getString().getString();
+        assertEquals(comment2,"comment top row4 (index3)\n");
     }
 
     public final void testShiftWithNames() {
@@ -378,6 +402,19 @@ public abstract class BaseTestSheetShiftRows extends TestCase {
 
         assertEquals("SUM(G29:I29)", sheet.getRow(28).getCell(9).getCellFormula());
         assertEquals("SUM(G30:I30)", sheet.getRow(29).getCell(9).getCellFormula());
-
     }
+
+	public void testBug55280() throws IOException {
+        Workbook w = _testDataProvider.createWorkbook();
+        try {
+            Sheet s = w.createSheet();
+            for (int row = 0; row < 5000; ++row)
+                s.addMergedRegion(new CellRangeAddress(row, row, 0, 3));
+
+            s.shiftRows(0, 4999, 1);        // takes a long time...
+        } finally {
+            w.close();
+        }
+	}
+
 }