Change how we update sheet names in XSSF formulas and names, when renaming sheets, to take advantage of the simpler structure that Pxg now offers

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

src/java/org/apache/poi/ss/formula/FormulaShifter.java

@@ -173,6 +173,7 @@ public final class FormulaShifter {
 		    if (rpxg.getExternalWorkbookNumber() > 0 ||
 		           ! _sheetName.equals(rpxg.getSheetName())) {
                 // only move 3D refs that refer to the sheet with cells being moved
+		        return null;
 		    }
             return rowMoveRefPtg(rpxg);
 		}

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

@@ -1310,6 +1310,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
     @Override
     public void setSheetName(int sheetIndex, String sheetname) {
         validateSheetIndex(sheetIndex);
+        String oldSheetName = getSheetName(sheetIndex);
 
         // YK: Mimic Excel and silently truncate sheet names longer than 31 characters
         if(sheetname != null && sheetname.length() > 31) sheetname = sheetname.substring(0, 31);
@@ -1317,11 +1318,16 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
         // findbugs fix - validateSheetName has already checked for null value
         assert(sheetname != null); 
 
+        // Do nothing if no change
+        if (sheetname.equals(oldSheetName)) return;
+        
+        // Check it isn't already taken
         if (containsSheet(sheetname, sheetIndex ))
             throw new IllegalArgumentException( "The workbook already contains a sheet of this name" );
 
+        // Update references to the name
         XSSFFormulaUtils utils = new XSSFFormulaUtils(this);
-        utils.updateSheetName(sheetIndex, sheetname);
+        utils.updateSheetName(sheetIndex, oldSheetName, sheetname);
 
         workbook.getSheets().getSheetArray(sheetIndex).setName(sheetname);
     }

src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFFormulaUtils.java

@@ -19,14 +19,11 @@
 
 package org.apache.poi.xssf.usermodel.helpers;
 
-import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalSheet;
 import org.apache.poi.ss.formula.FormulaParser;
 import org.apache.poi.ss.formula.FormulaRenderer;
-import org.apache.poi.ss.formula.FormulaRenderingWorkbook;
 import org.apache.poi.ss.formula.FormulaType;
-import org.apache.poi.ss.formula.ptg.NamePtg;
-import org.apache.poi.ss.formula.ptg.NameXPtg;
 import org.apache.poi.ss.formula.ptg.Ptg;
+import org.apache.poi.ss.formula.ptg.Pxg;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
@@ -56,47 +53,20 @@ public final class XSSFFormulaUtils {
      * <p/>
      * <p>
      * The idea is to parse every formula and render it back to string
-     * with the updated sheet name. The FormulaParsingWorkbook passed to the formula parser
+     * with the updated sheet name. This is done by parsing into Ptgs,
-     * is constructed from the old workbook (sheet name is not yet updated) and
+     * looking for ones with sheet references in them, and changing those
-     * the FormulaRenderingWorkbook passed to FormulaRenderer#toFormulaString is a custom implementation that
-     * returns the new sheet name.
      * </p>
      *
      * @param sheetIndex the 0-based index of the sheet being changed
-     * @param name       the new sheet name
+     * @param oldName    the old sheet name
+     * @param newName    the new sheet name
      */
-    public void updateSheetName(final int sheetIndex, final String name) {
+    public void updateSheetName(final int sheetIndex, final String oldName, final String newName) {
-
-        /**
-         * An instance of FormulaRenderingWorkbook that returns
-         */
-        FormulaRenderingWorkbook frwb = new FormulaRenderingWorkbook() {
-
-            public ExternalSheet getExternalSheet(int externSheetIndex) {
-                return _fpwb.getExternalSheet(externSheetIndex);
-            }
-
-            public String getSheetNameByExternSheet(int externSheetIndex) {
-                if (externSheetIndex == sheetIndex)
-                	return name;
-
-                return _fpwb.getSheetNameByExternSheet(externSheetIndex);
-            }
-
-            public String resolveNameXText(NameXPtg nameXPtg) {
-                return _fpwb.resolveNameXText(nameXPtg);
-            }
-
-            public String getNameText(NamePtg namePtg) {
-                return _fpwb.getNameText(namePtg);
-            }
-        };
-
         // update named ranges
         for (int i = 0; i < _wb.getNumberOfNames(); i++) {
             XSSFName nm = _wb.getNameAt(i);
             if (nm.getSheetIndex() == -1 || nm.getSheetIndex() == sheetIndex) {
-                updateName(nm, frwb);
+                updateName(nm, oldName, newName);
             }
         }
 
@@ -105,7 +75,7 @@ public final class XSSFFormulaUtils {
             for (Row row : sh) {
                 for (Cell cell : row) {
                     if (cell.getCellType() == Cell.CELL_TYPE_FORMULA) {
-                        updateFormula((XSSFCell) cell, frwb);
+                        updateFormula((XSSFCell) cell, oldName, newName);
                     }
                 }
             }
@@ -113,37 +83,52 @@ public final class XSSFFormulaUtils {
     }
 
     /**
-     * Parse cell formula and re-assemble it back using the specified FormulaRenderingWorkbook.
+     * Parse cell formula and re-assemble it back using the new sheet name
      *
      * @param cell the cell to update
-     * @param frwb the formula rendering workbbok that returns new sheet name
      */
-    private void updateFormula(XSSFCell cell, FormulaRenderingWorkbook frwb) {
+    private void updateFormula(XSSFCell cell, String oldName, String newName) {
         CTCellFormula f = cell.getCTCell().getF();
         if (f != null) {
             String formula = f.getStringValue();
             if (formula != null && formula.length() > 0) {
                 int sheetIndex = _wb.getSheetIndex(cell.getSheet());
                 Ptg[] ptgs = FormulaParser.parse(formula, _fpwb, FormulaType.CELL, sheetIndex);
-                String updatedFormula = FormulaRenderer.toFormulaString(frwb, ptgs);
+                for (Ptg ptg : ptgs) {
+                    updatePtg(ptg, oldName, newName);
+                }
+                String updatedFormula = FormulaRenderer.toFormulaString(_fpwb, ptgs);
                 if (!formula.equals(updatedFormula)) f.setStringValue(updatedFormula);
             }
         }
     }
 
     /**
-     * Parse formula in the named range and re-assemble it  back using the specified FormulaRenderingWorkbook.
+     * Parse formula in the named range and re-assemble it back using the new sheet name.
      *
      * @param name the name to update
-     * @param frwb the formula rendering workbbok that returns new sheet name
      */
-    private void updateName(XSSFName name, FormulaRenderingWorkbook frwb) {
+    private void updateName(XSSFName name, String oldName, String newName) {
         String formula = name.getRefersToFormula();
         if (formula != null) {
             int sheetIndex = name.getSheetIndex();
             Ptg[] ptgs = FormulaParser.parse(formula, _fpwb, FormulaType.NAMEDRANGE, sheetIndex);
-            String updatedFormula = FormulaRenderer.toFormulaString(frwb, ptgs);
+            for (Ptg ptg : ptgs) {
+                updatePtg(ptg, oldName, newName);
+            }
+            String updatedFormula = FormulaRenderer.toFormulaString(_fpwb, ptgs);
             if (!formula.equals(updatedFormula)) name.setRefersToFormula(updatedFormula);
         }
     }
+    
+    private void updatePtg(Ptg ptg, String oldName, String newName) {
+        if (ptg instanceof Pxg) {
+            Pxg pxg = (Pxg)ptg;
+            if (pxg.getExternalWorkbookNumber() < 1) {
+                if (pxg.getSheetName().equals(oldName)) {
+                    pxg.setSheetName(newName);
+                }
+            }
+        }
+    }
 }

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

@@ -1669,7 +1669,6 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
      * org.apache.poi.ss.formula.FormulaParseException: Parse error near char 0 '[' in specified formula '[0]!NR_Global_B2'. Expected number, string, or defined name 
      */
     @Test
-    @Ignore("Bug 56737 remains outstanding to fix")
     public void bug56737() throws IOException {
         Workbook wb = XSSFTestDataSamples.openSampleWorkbook("56737.xlsx");
         
@@ -1689,7 +1688,8 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
         Cell cRefWName = s.getRow(2).getCell(3);
         
         assertEquals("Defines!NR_To_A1", cRefSName.getCellFormula());
-        // TODO Why aren't we showing the real filename as Excel does?
+        // Note the formula, as stored in the file, has the external name index not filename
+        // TODO Provide a way to get the one with the filename
         assertEquals("[0]!NR_Global_B2", cRefWName.getCellFormula());
         
         // Try to evaluate them