From: Josh Micich Date: Mon, 6 Apr 2009 19:57:21 +0000 (+0000) Subject: Bugzilla 46973 - fixed defined names to behave better when refersToFormula is unset X-Git-Tag: REL_3_5_BETA6~68 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b302384e4dafd948df99e4ef79f187460c02358f;p=poi.git Bugzilla 46973 - fixed defined names to behave better when refersToFormula is unset git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@762479 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index cd5b4fd678..4929d519e6 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 46973 - Fixed defined names to behave better when refersToFormula is unset 46832 - Allow merged regions with columns greater than 255 or rows bigger than 65536 in XSSF 46951 - Fixed formula parser to better handle range operators and whole row/column refs. 46948 - Fixed evaluation of range operator to allow for area-ref operands diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 1fd8344630..8077cd8de2 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 46973 - Fixed defined names to behave better when refersToFormula is unset 46832 - Allow merged regions with columns greater than 255 or rows bigger than 65536 in XSSF 46951 - Fixed formula parser to better handle range operators and whole row/column refs. 46948 - Fixed evaluation of range operator to allow for area-ref operands diff --git a/src/java/org/apache/poi/hssf/record/formula/Ptg.java b/src/java/org/apache/poi/hssf/record/formula/Ptg.java index 08f690ce47..46c3ab9526 100644 --- a/src/java/org/apache/poi/hssf/record/formula/Ptg.java +++ b/src/java/org/apache/poi/hssf/record/formula/Ptg.java @@ -321,4 +321,32 @@ public abstract class Ptg implements Cloneable { * @return false if this token is classified as 'reference', 'value', or 'array' */ public abstract boolean isBaseToken(); + + public static boolean doesFormulaReferToDeletedCell(Ptg[] ptgs) { + for (int i = 0; i < ptgs.length; i++) { + if (isDeletedCellRef(ptgs[i])) { + return true; + } + } + return false; + } + private static boolean isDeletedCellRef(Ptg ptg) { + if (ptg == ErrPtg.REF_INVALID) { + return true; + } + if (ptg instanceof DeletedArea3DPtg) { + return true; + } + if (ptg instanceof DeletedRef3DPtg) { + return true; + } + if (ptg instanceof AreaErrPtg) { + return true; + } + if (ptg instanceof RefErrorPtg) { + return true; + } + return false; + } + } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFName.java b/src/java/org/apache/poi/hssf/usermodel/HSSFName.java index a0f278cc6c..bceaf340ad 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFName.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFName.java @@ -21,8 +21,8 @@ import org.apache.poi.hssf.model.HSSFFormulaParser; import org.apache.poi.hssf.model.Workbook; import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.formula.Ptg; -import org.apache.poi.ss.usermodel.Name; import org.apache.poi.ss.formula.FormulaType; +import org.apache.poi.ss.usermodel.Name; /** * High Level Representation of a 'defined name' which could be a 'built-in' name, @@ -160,46 +160,26 @@ public final class HSSFName implements Name { setRefersToFormula(ref); } - /** - * Sets the formula that the name is defined to refer to. The following are representative examples: - * - *
    - *
  • 'My Sheet'!$A$3
  • - *
  • 8.3
  • - *
  • HR!$A$1:$Z$345
  • - *
  • SUM(Sheet1!A1,Sheet2!B2)
  • - *
  • -PMT(Interest_Rate/12,Number_of_Payments,Loan_Amount)
  • - *
- * - * @param formulaText the reference for this name - * @throws IllegalArgumentException if the specified reference is unparsable - */ public void setRefersToFormula(String formulaText) { Ptg[] ptgs = HSSFFormulaParser.parse(formulaText, _book, FormulaType.NAMEDRANGE, getSheetIndex()); _definedNameRec.setNameDefinition(ptgs); } - /** - * Returns the formula that the name is defined to refer to. The following are representative examples: - * - * @return the reference for this name - * @see #setRefersToFormula(String) - */ public String getRefersToFormula() { if (_definedNameRec.isFunctionName()) { throw new IllegalStateException("Only applicable to named ranges"); } - return HSSFFormulaParser.toFormulaString(_book, _definedNameRec.getNameDefinition()); + Ptg[] ptgs = _definedNameRec.getNameDefinition(); + if (ptgs.length < 1) { + // 'refersToFormula' has not been set yet + return null; + } + return HSSFFormulaParser.toFormulaString(_book, ptgs); } - /** - * Tests if this name points to a cell that no longer exists - * - * @return true if the name refers to a deleted cell, false otherwise - */ public boolean isDeleted(){ - String formulaText = getRefersToFormula(); - return formulaText.indexOf("#REF!") != -1; + Ptg[] ptgs = _definedNameRec.getNameDefinition(); + return Ptg.doesFormulaReferToDeletedCell(ptgs); } /** diff --git a/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Name.java b/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Name.java index 37eff5a422..696eb3c1ac 100644 --- a/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Name.java +++ b/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Name.java @@ -118,7 +118,7 @@ public interface Name { /** * Returns the formula that the name is defined to refer to. * - * @return the reference for this name + * @return the reference for this name, null if it has not been set yet. Never empty string * @see #setRefersToFormula(String) */ String getRefersToFormula(); @@ -134,10 +134,10 @@ public interface Name { *
  • -PMT(Interest_Rate/12,Number_of_Payments,Loan_Amount)
  • * * - * @param ref the reference for this name - * @throws IllegalArgumentException if the specified reference is unparsable + * @param formulaText the reference for this name + * @throws IllegalArgumentException if the specified formulaText is unparsable */ - void setRefersToFormula(String ref); + void setRefersToFormula(String formulaText); /** * Checks if this name is a function name @@ -149,7 +149,7 @@ public interface Name { /** * Checks if this name points to a cell that no longer exists * - * @return true if the name refers to a deleted cell, false otherwise + * @return true if the name refers to a deleted cell, false otherwise */ boolean isDeleted(); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java index 4517f1292f..9f3c7f0125 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java @@ -181,25 +181,18 @@ public final class XSSFName implements Name { ctName.setName(name); } - /** - * Returns the reference of this named range, such as Sales!C20:C30. - * - * @return the reference of this named range - */ public String getRefersToFormula() { - return ctName.getStringValue(); + String result = ctName.getStringValue(); + if (result == null || result.length() < 1) { + return null; + } + return result; } - /** - * Sets the reference of this named range, such as Sales!C20:C30. - * - * @param formulaText the reference to set - * @throws IllegalArgumentException if the specified reference is unparsable - */ public void setRefersToFormula(String formulaText) { XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(workbook); try { - Ptg[] ptgs = FormulaParser.parse(formulaText, fpb, FormulaType.NAMEDRANGE, getSheetIndex()); + FormulaParser.parse(formulaText, fpb, FormulaType.NAMEDRANGE, getSheetIndex()); } catch (RuntimeException e) { if (e.getClass().getName().startsWith(FormulaParser.class.getName())) { throw new IllegalArgumentException("Unparsable formula '" + formulaText + "'", e); @@ -209,14 +202,14 @@ public final class XSSFName implements Name { ctName.setStringValue(formulaText); } - /** - * Tests if this name points to a cell that no longer exists - * - * @return true if the name refers to a deleted cell, false otherwise - */ public boolean isDeleted(){ - String ref = getRefersToFormula(); - return ref != null && ref.indexOf("#REF!") != -1; + String formulaText = getRefersToFormula(); + if (formulaText == null) { + return false; + } + XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(workbook); + Ptg[] ptgs = FormulaParser.parse(formulaText, fpb, FormulaType.NAMEDRANGE, getSheetIndex()); + return Ptg.doesFormulaReferToDeletedCell(ptgs); } /** diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java index 253d6b50aa..a645db57b3 100755 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java @@ -17,13 +17,13 @@ package org.apache.poi.ss.usermodel; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; + +import org.apache.poi.hssf.usermodel.HSSFName; import org.apache.poi.ss.ITestDataProvider; -import org.apache.poi.ss.formula.FormulaParser; -import org.apache.poi.ss.formula.FormulaType; -import org.apache.poi.ss.util.CellReference; import org.apache.poi.ss.util.AreaReference; -import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.ss.util.CellReference; /** * Tests of implementations of {@link org.apache.poi.ss.usermodel.Name}. @@ -395,7 +395,7 @@ public abstract class BaseTestNamedRange extends TestCase { * Test that multiple named ranges can be added written and read */ public void testMultipleNamedWrite() { - Workbook wb = getTestDataProvider().createWorkbook(); + Workbook wb = getTestDataProvider().createWorkbook(); wb.createSheet("testSheet1"); @@ -498,4 +498,50 @@ public abstract class BaseTestNamedRange extends TestCase { assertEquals("Contents of cell retrieved by its named reference", contents, cvalue); } + + /** + * Bugzilla attachment 23444 (from bug 46973) has a NAME record with the following encoding: + *
    +     * 00000000 | 18 00 17 00 00 00 00 08 00 00 00 00 00 00 00 00 | ................
    +     * 00000010 | 00 00 00 55 50 53 53 74 61 74 65                | ...UPSState
    +     * 
    + * + * This caused trouble for anything that requires {@link HSSFName#getRefersToFormula()} + * It is easy enough to re-create the the same data (by not setting the formula). Excel + * seems to gracefully remove this uninitialized name record. It would be nice if POI + * could do the same, but that would involve adjusting subsequent name indexes across + * all formulas.

    + * + * For the moment, POI has been made to behave more sensibly with uninitialised name + * records. + */ + public final void testUninitialisedNameGetRefersToFormula_bug46973() { + Workbook wb = getTestDataProvider().createWorkbook(); + Name n = wb.createName(); + n.setNameName("UPSState"); + String formula; + try { + formula = n.getRefersToFormula(); + } catch (IllegalArgumentException e) { + if (e.getMessage().equals("ptgs must not be null")) { + throw new AssertionFailedError("Identified bug 46973"); + } + throw e; + } + assertNull(formula); + assertFalse(n.isDeleted()); // according to exact definition of isDeleted() + } + + public void testDeletedCell() { + Workbook wb = getTestDataProvider().createWorkbook(); + Name n = wb.createName(); + n.setNameName("MyName"); + // contrived example to expose bug: + n.setRefersToFormula("if(A1,\"#REF!\", \"\")"); + + if (n.isDeleted()) { + throw new AssertionFailedError("Identified bug in recoginising formulas referring to deleted cells"); + } + + } } \ No newline at end of file