From fea44cd9047e495cff534ce392b85799743217b5 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Sun, 20 Jul 2014 18:56:35 +0000 Subject: [PATCH] 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 --- .../apache/poi/ss/formula/FormulaShifter.java | 1 + .../poi/xssf/usermodel/XSSFWorkbook.java | 8 +- .../usermodel/helpers/XSSFFormulaUtils.java | 77 ++++++++----------- .../poi/xssf/usermodel/TestXSSFBugs.java | 4 +- 4 files changed, 41 insertions(+), 49 deletions(-) diff --git a/src/java/org/apache/poi/ss/formula/FormulaShifter.java b/src/java/org/apache/poi/ss/formula/FormulaShifter.java index 9439000d23..f74bf91202 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaShifter.java +++ b/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); } 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 84e118233c..dbfd66aca9 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -1310,6 +1310,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable 31) sheetname = sheetname.substring(0, 31); @@ -1317,11 +1318,16 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable *

* 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 - * is constructed from the old workbook (sheet name is not yet updated) and - * the FormulaRenderingWorkbook passed to FormulaRenderer#toFormulaString is a custom implementation that - * returns the new sheet name. + * with the updated sheet name. This is done by parsing into Ptgs, + * looking for ones with sheet references in them, and changing those *

* * @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) { - - /** - * 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); - } - }; - + public void updateSheetName(final int sheetIndex, final String oldName, final String newName) { // 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); + } + } + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index b1f223e1c3..7f64b192b6 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/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