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

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<Sheet> sheetIterator() {
+        Iterator<Sheet> result = new SheetIterator<Sheet>();
+        return result;
+    }
+
+    /**
+     * Alias for {@link #sheetIterator()} to allow
+     * foreach loops
+     */
+    public Iterator<Sheet> iterator() {
+        return sheetIterator();
+    }
+    
+    private final class SheetIterator<T extends Sheet> implements Iterator<T> {
+        final private Iterator<T> it;
+        private T cursor = null;
+        @SuppressWarnings("unchecked")
+        public SheetIterator() {
+            it = (Iterator<T>) _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<Sheet> {
 
     /** 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<Sheet> 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<Sheet> sheetIterator() {
+        return new SheetIterator<Sheet>();
+    }
+    
+    private final class SheetIterator<T extends Sheet> implements Iterator<T> {
+        final private Iterator<XSSFSheet> it;
+        @SuppressWarnings("unchecked")
+        public SheetIterator() {
+            it = (Iterator<XSSFSheet>)(Iterator<? extends Sheet>) _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<Sheet> 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<XSSFSheet> {
+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<X
     }
 
     /**
-     * Allows foreach loops:
-     * <pre><code>
-     * 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.
      *
-     * }
-     * </code></pre>
+     * Note: remove() is not supported on this iterator.
+     * Use {@link #removeSheetAt(int)} to remove sheets instead.
+     *
+     * @return an iterator of the sheets.
+     */
+    public Iterator<Sheet> sheetIterator() {
+        return new SheetIterator<Sheet>();
+    }
+    
+    /**
+     * Alias for {@link #sheetIterator()} to allow
+     * foreach loops
+     * 
+     * <code>Iterator<XSSFSheet> iterator()<code> was replaced with <code>Iterator<Sheet> iterator()</code>
+     * 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<XSSFSheet> iterator() {
-        return sheets.iterator();
+    public Iterator<Sheet> iterator() {
+        return sheetIterator();
     }
+    
+    private final class SheetIterator<T extends Sheet> implements Iterator<T> {
+        final private Iterator<T> it;
+        @SuppressWarnings("unchecked")
+        public SheetIterator() {
+            it = (Iterator<T>) 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<Sheet> iterator()
+     *  interface less painful for projects currently using POI.
+     *  
+     *  If your code was written using a for-each loop:
+     *  <pre><code>
+     *  for (XSSFSheet sh : wb) {
+     *      sh.createRow(0);
+     *  }
+     *  </code></pre>
+     *  
+     *  There are two ways to upgrade your code:
+     *  // Option A:
+     *  <pre><code>
+     *      for (XSSFSheet sh : (Iterable<XSSFSheet>) (Iterable<? extends Sheet>) wb) {
+     *          sh.createRow(0);
+     *      }
+     *  </code></pre>
+     *      
+     *  // Option B (preferred for new code):
+     *  <pre><code>
+     *  for (Sheet sh : wb) {
+     *      sh.createRow(0);
+     *  }
+     *  </code></pre>
+     *  
+     *  
+     *  
+     *  If your code was written using an iterator variable:
+     *  <pre><code>
+     *  Iterator<XSSFSheet> it = wb.iterator();
+     *  XSSFSheet sh = it.next();
+     *  sh.createRow(0);
+     *  </code></pre>
+     *  
+     *  There are three ways to upgrade your code:
+     *  // Option A:
+     *  <pre><code>
+     *  Iterator<XSSFSheet> it = (Iterator<XSSFSheet>) (Iterator<? extends Sheet>) wb.iterator();
+     *  XSSFSheet sh = it.next();
+     *  sh.createRow(0);
+     *  </code></pre>
+     *
+     *  // Option B:
+     *  <pre><code>
+     *  @SuppressWarnings("deprecation")
+     *  Iterator<XSSFSheet> it = wb.xssfSheetIterator();
+     *  XSSFSheet sh = it.next();
+     *  sh.createRow(0);
+     *  </code></pre>
+     *
+     *  // Option C (preferred for new code):
+     *  <pre><code>
+     *  Iterator<Sheet> it = wb.iterator();
+     *  Sheet sh = it.next();
+     *  sh.createRow(0);
+     *  </code></pre>
+     *  
+     *  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<XSSFSheet> xssfSheetIterator() {
+        return new SheetIterator<XSSFSheet>();
+    }
+    
     /**
      * 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<XSSFSheet> XSSFWorkbook.iterator was committed in r700472 on 2008-09-30
+     *  and has been replaced with Iterator<Sheet> 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<XSSFSheet> iterator and Iterator<XSSFSheet> sheetIterator
+     *  have been replaced with Iterator<Sheet>  {@link #iterator} and
+     *  Iterator<Sheet> {@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<XSSFSheet>) (Iterable<? extends Sheet>) 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<XSSFSheet> it = wb.iterator();
+            XSSFSheet sh = it.next();
+            sh.createRow(0);
+            */
+            
+            // Option A:
+            {
+                Iterator<XSSFSheet> it = (Iterator<XSSFSheet>) (Iterator<? extends Sheet>) wb.iterator();
+                XSSFSheet sh = it.next();
+                sh.createRow(0);
+            }
+            
+            // Option B:
+            {
+                @SuppressWarnings("deprecation")
+                Iterator<XSSFSheet> it = wb.xssfSheetIterator();
+                XSSFSheet sh = it.next();
+                sh.createRow(0);
+            }
+            
+            // Option C (preferred for new code):
+            {
+                Iterator<Sheet> 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<Sheet> 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<Sheet> 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<Sheet> 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() {