From 64c5d60d33551ec0c9acaa2f0c3028ac78e3bc16 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Thu, 17 Sep 2015 11:10:11 +0000 Subject: [PATCH] Patch from Javen ONeal from bug #58245 - Make Workbook support iterating over Sheets git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1703573 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/usermodel/HSSFWorkbook.java | 50 ++++++- .../org/apache/poi/ss/usermodel/Workbook.java | 11 +- .../poi/xssf/streaming/SXSSFWorkbook.java | 51 +++++++ .../poi/xssf/usermodel/XSSFWorkbook.java | 134 ++++++++++++++++-- .../usermodel/helpers/XSSFRowShifter.java | 5 +- .../usermodel/TestXSSFRichTextString.java | 9 +- .../poi/xssf/usermodel/TestXSSFWorkbook.java | 80 +++++++++++ .../poi/ss/usermodel/BaseTestWorkbook.java | 75 ++++++++++ 8 files changed, 398 insertions(+), 17 deletions(-) diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index dfca245899..09c3066668 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -33,6 +33,7 @@ import java.util.Hashtable; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.regex.Pattern; import org.apache.commons.codec.digest.DigestUtils; @@ -82,6 +83,7 @@ import org.apache.poi.ss.formula.udf.AggregatingUDFFinder; import org.apache.poi.ss.formula.udf.IndexedUDFFinder; import org.apache.poi.ss.formula.udf.UDFFinder; import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; +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.WorkbookUtil; @@ -913,12 +915,58 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss sheet.setActive(isOnlySheet); return sheet; } + + /** + * Returns an iterator of the sheets in the workbook + * in sheet order. Includes hidden and very hidden sheets. + * + * @return an iterator of the sheets. + */ + public Iterator sheetIterator() { + Iterator result = new SheetIterator(); + return result; + } + + /** + * Alias for {@link #sheetIterator()} to allow + * foreach loops + */ + public Iterator iterator() { + return sheetIterator(); + } + + private final class SheetIterator implements Iterator { + final private Iterator it; + private T cursor = null; + @SuppressWarnings("unchecked") + public SheetIterator() { + it = (Iterator) _sheets.iterator(); + } + @Override + public boolean hasNext() { + return it.hasNext(); + } + @Override + public T next() throws NoSuchElementException { + cursor = it.next(); + return cursor; + } + /** + * Unexpected behavior may occur if sheets are reordered after iterator + * has been created. Support for the remove method may be added in the future + * if someone can figure out a reliable implementation. + */ + @Override + public void remove() throws IllegalStateException { + throw new UnsupportedOperationException("remove method not supported on HSSFWorkbook.iterator(). "+ + "Use Sheet.removeSheetAt(int) instead."); + } + } /** * get the number of spreadsheets in the workbook (this will be three after serialization) * @return number of sheets */ - @Override public int getNumberOfSheets() { diff --git a/src/java/org/apache/poi/ss/usermodel/Workbook.java b/src/java/org/apache/poi/ss/usermodel/Workbook.java index e5fc3c9de7..fad937a646 100644 --- a/src/java/org/apache/poi/ss/usermodel/Workbook.java +++ b/src/java/org/apache/poi/ss/usermodel/Workbook.java @@ -20,6 +20,7 @@ package org.apache.poi.ss.usermodel; import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; +import java.util.Iterator; import java.util.List; import org.apache.poi.ss.formula.udf.UDFFinder; @@ -31,7 +32,7 @@ import org.apache.poi.ss.util.CellRangeAddress; * will construct whether they are reading or writing a workbook. It is also the * top level object for creating new sheets/etc. */ -public interface Workbook extends Closeable { +public interface Workbook extends Closeable, Iterable { /** Extended windows meta file */ public static final int PICTURE_TYPE_EMF = 2; @@ -231,6 +232,14 @@ public interface Workbook extends Closeable { */ Sheet cloneSheet(int sheetNum); + + /** + * Returns an iterator of the sheets in the workbook + * in sheet order. Includes hidden and very hidden sheets. + * + * @return an iterator of the sheets. + */ + Iterator sheetIterator(); /** * Get the number of spreadsheets in the workbook diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java index 413b5adb51..3c16b2cbe8 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java @@ -26,7 +26,9 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.util.Enumeration; import java.util.HashMap; +import java.util.Iterator; import java.util.List; +import java.util.NoSuchElementException; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; @@ -679,6 +681,55 @@ public class SXSSFWorkbook implements Workbook { { return _wb.getNumberOfSheets(); } + + /** + * Returns an iterator of the sheets in the workbook + * in sheet order. Includes hidden and very hidden sheets. + * + * @return an iterator of the sheets. + */ + @Override + public Iterator sheetIterator() { + return new SheetIterator(); + } + + private final class SheetIterator implements Iterator { + final private Iterator it; + @SuppressWarnings("unchecked") + public SheetIterator() { + it = (Iterator)(Iterator) _wb.iterator(); + } + @Override + public boolean hasNext() { + return it.hasNext(); + } + @Override + @SuppressWarnings("unchecked") + public T next() throws NoSuchElementException { + final XSSFSheet xssfSheet = it.next(); + final T sxssfSheet = (T) getSXSSFSheet(xssfSheet); + return sxssfSheet; + } + /** + * Unexpected behavior may occur if sheets are reordered after iterator + * has been created. Support for the remove method may be added in the future + * if someone can figure out a reliable implementation. + */ + @Override + public void remove() throws IllegalStateException { + throw new UnsupportedOperationException("remove method not supported on XSSFWorkbook.iterator(). "+ + "Use Sheet.removeSheetAt(int) instead."); + } + } + + /** + * Alias for {@link #sheetIterator()} to allow + * foreach loops + */ + @Override + public Iterator iterator() { + return sheetIterator(); + } /** * Get the Sheet object at the given index. diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java index c1cd1ff3f7..3af73134a7 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -33,6 +33,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.regex.Pattern; import javax.xml.namespace.QName; @@ -104,7 +105,7 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.WorkbookDocument; * will construct whether they are reading or writing a workbook. It is also the * top level object for creating new sheets/etc. */ -public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable { +public class XSSFWorkbook extends POIXMLDocument implements Workbook { private static final Pattern COMMA_PATTERN = Pattern.compile(","); /** @@ -1055,18 +1056,133 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable - * XSSFWorkbook wb = new XSSFWorkbook(package); - * for(XSSFSheet sheet : wb){ + * Returns an iterator of the sheets in the workbook + * in sheet order. Includes hidden and very hidden sheets. * - * } - * + * Note: remove() is not supported on this iterator. + * Use {@link #removeSheetAt(int)} to remove sheets instead. + * + * @return an iterator of the sheets. + */ + public Iterator sheetIterator() { + return new SheetIterator(); + } + + /** + * Alias for {@link #sheetIterator()} to allow + * foreach loops + * + * Iterator iterator() was replaced with Iterator iterator() + * to make iterating over a container (Workbook, Sheet, Row) consistent across POI spreadsheets. + * This breaks backwards compatibility and may affect your code. + * See {@link XSSFWorkbook#xssfSheetIterator} for how to upgrade your code to be compatible + * with the new interface. + * + * Note: remove() is not supported on this iterator. + * Use {@link #removeSheetAt(int)} to remove sheets instead. + * + * @return an iterator of the sheets. */ @Override - public Iterator iterator() { - return sheets.iterator(); + public Iterator iterator() { + return sheetIterator(); } + + private final class SheetIterator implements Iterator { + final private Iterator it; + @SuppressWarnings("unchecked") + public SheetIterator() { + it = (Iterator) sheets.iterator(); + } + @Override + public boolean hasNext() { + return it.hasNext(); + } + @Override + public T next() throws NoSuchElementException { + return it.next(); + } + /** + * Unexpected behavior may occur if sheets are reordered after iterator + * has been created. Support for the remove method may be added in the future + * if someone can figure out a reliable implementation. + */ + @Override + public void remove() throws IllegalStateException { + throw new UnsupportedOperationException("remove method not supported on XSSFWorkbook.iterator(). "+ + "Use Sheet.removeSheetAt(int) instead."); + } + } + + /** + * + * xssfSheetIterator was added to make transitioning to the new Iterator iterator() + * interface less painful for projects currently using POI. + * + * If your code was written using a for-each loop: + * 

+     *  for (XSSFSheet sh : wb) {
+     *      sh.createRow(0);
+     *  }
+     *
+ * + * There are two ways to upgrade your code: + * // Option A: + * 

+     *      for (XSSFSheet sh : (Iterable) (Iterable) wb) {
+     *          sh.createRow(0);
+     *      }
+     *
+ * + * // Option B (preferred for new code): + * 

+     *  for (Sheet sh : wb) {
+     *      sh.createRow(0);
+     *  }
+     *
+ * + * + * + * If your code was written using an iterator variable: + * 

+     *  Iterator it = wb.iterator();
+     *  XSSFSheet sh = it.next();
+     *  sh.createRow(0);
+     *
+ * + * There are three ways to upgrade your code: + * // Option A: + * 

+     *  Iterator it = (Iterator) (Iterator) wb.iterator();
+     *  XSSFSheet sh = it.next();
+     *  sh.createRow(0);
+     *
+ * + * // Option B: + * 

+     *  @SuppressWarnings("deprecation")
+     *  Iterator it = wb.xssfSheetIterator();
+     *  XSSFSheet sh = it.next();
+     *  sh.createRow(0);
+     *
+ * + * // Option C (preferred for new code): + * 

+     *  Iterator it = wb.iterator();
+     *  Sheet sh = it.next();
+     *  sh.createRow(0);
+     *
+ * + * New projects should use the preferred options. Note: XSSFWorkbook.xssfSheetIterator + * is deprecated and will be removed in a future version. + * + * @return an iterator of the sheets. + */ + @Deprecated + public Iterator xssfSheetIterator() { + return new SheetIterator(); + } + /** * Are we a normal workbook (.xlsx), or a * macro enabled workbook (.xlsm)? diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java index cfa0a77771..075320c026 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java @@ -32,6 +32,7 @@ import org.apache.poi.ss.formula.ptg.AreaPtg; import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.Row; +import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; @@ -148,13 +149,13 @@ public final class XSSFRowShifter { //update formulas on other sheets XSSFWorkbook wb = sheet.getWorkbook(); - for (XSSFSheet sh : wb) { + for (Sheet sh : wb) { if (sheet == sh) continue; updateSheetFormulas(sh, shifter); } } - private void updateSheetFormulas(XSSFSheet sh, FormulaShifter shifter) { + private void updateSheetFormulas(Sheet sh, FormulaShifter shifter) { for (Row r : sh) { XSSFRow row = (XSSFRow) r; updateRowFormulas(row, shifter); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java index b5f0a19b4e..ea11d273da 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java @@ -22,6 +22,7 @@ import java.util.TreeMap; import junit.framework.TestCase; +import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.xssf.XSSFTestDataSamples; @@ -441,20 +442,20 @@ public final class TestXSSFRichTextString extends TestCase { @Test public void testBug56511() { XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("56511.xlsx"); - for (XSSFSheet sheet : wb) { + for (Sheet sheet : wb) { int lastRow = sheet.getLastRowNum(); for (int rowIdx = sheet.getFirstRowNum(); rowIdx <= lastRow; rowIdx++) { - XSSFRow row = sheet.getRow(rowIdx); + Row row = sheet.getRow(rowIdx); if(row != null) { int lastCell = row.getLastCellNum(); for (int cellIdx = row.getFirstCellNum(); cellIdx <= lastCell; cellIdx++) { - XSSFCell cell = row.getCell(cellIdx); + Cell cell = row.getCell(cellIdx); if (cell != null) { //System.out.println("row " + rowIdx + " column " + cellIdx + ": " + cell.getCellType() + ": " + cell.toString()); - XSSFRichTextString richText = cell.getRichStringCellValue(); + XSSFRichTextString richText = (XSSFRichTextString) cell.getRichStringCellValue(); int anzFormattingRuns = richText.numFormattingRuns(); for (int run = 0; run < anzFormattingRuns; run++) { /*XSSFFont font =*/ richText.getFontOfFormattingRun(run); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java index d5070753cd..f5fdd4363d 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java @@ -32,6 +32,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.util.Iterator; import java.util.List; import java.util.zip.CRC32; @@ -940,4 +941,83 @@ public final class TestXSSFWorkbook extends BaseTestWorkbook { // workbook.write(fileOutputStream); // fileOutputStream.close(); } + + @SuppressWarnings("unchecked") + @Test + /** + * Iterator XSSFWorkbook.iterator was committed in r700472 on 2008-09-30 + * and has been replaced with Iterator XSSFWorkbook.iterator + * + * In order to make code for looping over sheets in workbooks standard, regardless + * of the type of workbook (HSSFWorkbook, XSSFWorkbook, SXSSFWorkbook), the previously + * available Iterator iterator and Iterator sheetIterator + * have been replaced with Iterator {@link #iterator} and + * Iterator {@link #sheetIterator}. This makes iterating over sheets in a workbook + * similar to iterating over rows in a sheet and cells in a row. + * + * Note: this breaks backwards compatibility! Existing codebases will need to + * upgrade their code with either of the following options presented in this test case. + * + */ + public void bug58245_XSSFSheetIterator() { + final XSSFWorkbook wb = new XSSFWorkbook(); + try { + wb.createSheet(); + + // ===================================================================== + // Case 1: Existing code uses XSSFSheet for-each loop + // ===================================================================== + // Original code (no longer valid) + /* + for (XSSFSheet sh : wb) { + sh.createRow(0); + } + */ + + // Option A: + for (XSSFSheet sh : (Iterable) (Iterable) wb) { + sh.createRow(0); + } + + // Option B (preferred for new code): + for (Sheet sh : wb) { + sh.createRow(0); + } + + // ===================================================================== + // Case 2: Existing code creates an iterator variable + // ===================================================================== + // Original code (no longer valid) + /* + Iterator it = wb.iterator(); + XSSFSheet sh = it.next(); + sh.createRow(0); + */ + + // Option A: + { + Iterator it = (Iterator) (Iterator) wb.iterator(); + XSSFSheet sh = it.next(); + sh.createRow(0); + } + + // Option B: + { + @SuppressWarnings("deprecation") + Iterator it = wb.xssfSheetIterator(); + XSSFSheet sh = it.next(); + sh.createRow(0); + } + + // Option C (preferred for new code): + { + Iterator it = wb.iterator(); + Sheet sh = it.next(); + sh.createRow(0); + } + } + finally { + IOUtils.closeQuietly(wb); + } + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java index d51f8e0f4c..4a9dd65c09 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java @@ -26,6 +26,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; +import java.util.ConcurrentModificationException; +import java.util.Iterator; import junit.framework.AssertionFailedError; @@ -43,6 +45,79 @@ public abstract class BaseTestWorkbook { protected BaseTestWorkbook(ITestDataProvider testDataProvider) { _testDataProvider = testDataProvider; } + + @Test + public void sheetIterator_forEach() { + final Workbook wb = _testDataProvider.createWorkbook(); + wb.createSheet("Sheet0"); + wb.createSheet("Sheet1"); + wb.createSheet("Sheet2"); + int i = 0; + for (Sheet sh : wb) { + assertEquals("Sheet"+i, sh.getSheetName()); + i++; + } + } + + @Test + public void sheetIterator_sheetsReordered() { + final Workbook wb = _testDataProvider.createWorkbook(); + wb.createSheet("Sheet0"); + wb.createSheet("Sheet1"); + wb.createSheet("Sheet2"); + + Iterator it = wb.sheetIterator(); + it.next(); + wb.setSheetOrder("Sheet2", 1); + + // Iterator order should be fixed when iterator is created + try { + assertEquals("Sheet1", it.next().getSheetName()); + fail("Expected ConcurrentModificationException: "+ + "should not be able to advance an iterator when the "+ + "underlying data has been reordered"); + } catch (final ConcurrentModificationException e) { + // expected + } + } + + @Test + public void sheetIterator_sheetRemoved() { + final Workbook wb = _testDataProvider.createWorkbook(); + wb.createSheet("Sheet0"); + wb.createSheet("Sheet1"); + wb.createSheet("Sheet2"); + + Iterator it = wb.sheetIterator(); + wb.removeSheetAt(1); + + // Iterator order should be fixed when iterator is created + try { + it.next(); + fail("Expected ConcurrentModificationException: "+ + "should not be able to advance an iterator when the "+ + "underlying data has been reordered"); + } catch (final ConcurrentModificationException e) { + // expected + } + } + + @Test + public void sheetIterator_remove() { + final Workbook wb = _testDataProvider.createWorkbook(); + wb.createSheet("Sheet0"); + + Iterator it = wb.sheetIterator(); + it.next(); //Sheet0 + try { + it.remove(); + fail("Expected UnsupportedOperationException: "+ + "should not be able to remove sheets from the sheet iterator"); + } catch (final UnsupportedOperationException e) { + // expected + } + } + @Test public void createSheet() { -- 2.39.5