diff --git a/.classpath b/.classpath index ae3f06128..3b3ff5b64 100644 --- a/.classpath +++ b/.classpath @@ -18,18 +18,19 @@ - + - - + + - + - - - + + + + diff --git a/build.xml b/build.xml index 0285acae3..55abab1f2 100644 --- a/build.xml +++ b/build.xml @@ -184,6 +184,9 @@ under the License. + + @@ -326,6 +329,7 @@ under the License. + @@ -623,6 +627,7 @@ under the License. + @@ -633,6 +638,7 @@ under the License. + @@ -1874,7 +1880,7 @@ under the License. - + @@ -2116,6 +2122,7 @@ under the License. + diff --git a/maven/poi-ooxml.pom b/maven/poi-ooxml.pom index 0f3cdb1d5..2f4c286bc 100644 --- a/maven/poi-ooxml.pom +++ b/maven/poi-ooxml.pom @@ -74,5 +74,10 @@ curvesapi 1.04 + + org.apache.commons + commons-collections4 + 4.1 + diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 35f986211..ba015a9f1 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -92,6 +92,7 @@ import org.apache.poi.ss.formula.SheetNameFormatter; 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.Name; import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; @@ -1530,6 +1531,11 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss return names.get(nameIndex); } + @Override + public List getAllNames() { + return Collections.unmodifiableList(names); + } + public NameRecord getNameRecord(int nameIndex) { return getWorkbook().getNameRecord(nameIndex); } @@ -1701,8 +1707,9 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss * * @param name the name to remove. */ - void removeName(HSSFName name) { - int index = getNameIndex(name); + @Override + public void removeName(Name name) { + int index = getNameIndex((HSSFName) name); removeName(index); } diff --git a/src/java/org/apache/poi/ss/usermodel/Workbook.java b/src/java/org/apache/poi/ss/usermodel/Workbook.java index f1f1a4cd2..e52615fb1 100644 --- a/src/java/org/apache/poi/ss/usermodel/Workbook.java +++ b/src/java/org/apache/poi/ss/usermodel/Workbook.java @@ -369,6 +369,13 @@ public interface Workbook extends Closeable, Iterable { */ List getNames(String name); + /** + * Returns all defined names. + * + * @return a list of the defined names. An empty list is returned if none is found. + */ + List getAllNames(); + /** * @param nameIndex position of the named range (0-based) * @return the defined name at the specified index @@ -407,6 +414,13 @@ public interface Workbook extends Closeable, Iterable { */ void removeName(String name); + /** + * Remove a defined name + * + * @param name the name of the defined name + */ + void removeName(Name name); + /** * Adds the linking required to allow formulas referencing * the specified external workbook to be added to this one. 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 58e16c772..331ad9a0f 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java @@ -1006,12 +1006,25 @@ public class SXSSFWorkbook implements Workbook { return _wb.getNames(name); } + /** + * Returns all defined names + * + * @return all defined names + */ + @Override + public List getAllNames() + { + return _wb.getAllNames(); + } + /** * @param nameIndex position of the named range (0-based) * @return the defined name at the specified index * @throws IllegalArgumentException if the supplied index is invalid + * @deprecated 3.16. New projects should avoid accessing named ranges by index. */ @Override + @Deprecated public Name getNameAt(int nameIndex) { return _wb.getNameAt(nameIndex); @@ -1036,8 +1049,12 @@ public class SXSSFWorkbook implements Workbook { * * @param name the name of the defined name * @return zero based index of the defined name. -1 if not found. + * + * @deprecated 3.16. New projects should avoid accessing named ranges by index. + * Use {@link #getName(String)} instead. */ @Override + @Deprecated public int getNameIndex(String name) { return _wb.getNameIndex(name); @@ -1047,8 +1064,11 @@ public class SXSSFWorkbook implements Workbook { * Remove the defined name at the specified index * * @param index named range index (0 based) + * + * @deprecated 3.16. New projects should use {@link #removeName(Name)}. */ @Override + @Deprecated public void removeName(int index) { _wb.removeName(index); @@ -1057,10 +1077,24 @@ public class SXSSFWorkbook implements Workbook { /** * Remove a defined name by name * - * @param name the name of the defined name + * @param name the name of the defined name + * + * @deprecated 3.16. New projects should use {@link #removeName(Name)}. */ @Override + @Deprecated public void removeName(String name) + { + _wb.removeName(name); + } + + /** + * Remove the given defined name + * + * @param name the name to remove + */ + @Override + public void removeName(Name name) { _wb.removeName(name); } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java index f3c6bc267..9c5e6ffb1 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java @@ -232,7 +232,7 @@ public abstract class BaseXSSFEvaluationWorkbook implements FormulaRenderingWork // Otherwise, try it as a named range if (sheet == null) { - if (_uBook.getNameIndex(name) > -1) { + if (!_uBook.getNames(name).isEmpty()) { return new NameXPxg(null, name); } return null; 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 462bd6517..071062620 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java @@ -167,19 +167,18 @@ public final class XSSFName implements Name { public void setNameName(String name) { validateName(name); + String oldName = getNameName(); int sheetIndex = getSheetIndex(); - int numberOfNames = _workbook.getNumberOfNames(); //Check to ensure no other names have the same case-insensitive name at the same scope - for (int i = 0; i < numberOfNames; i++) { - XSSFName nm = _workbook.getNameAt(i); - if ((nm != this) - && name.equalsIgnoreCase(nm.getNameName()) - && (sheetIndex == nm.getSheetIndex())) { + for (XSSFName foundName : _workbook.getNames(name)) { + if (foundName.getSheetIndex() == sheetIndex && foundName != this) { String msg = "The "+(sheetIndex == -1 ? "workbook" : "sheet")+" already contains this name: " + name; throw new IllegalArgumentException(msg); } } _ctName.setName(name); + //Need to update the name -> named ranges map + _workbook.updateName(this, oldName); } public String getRefersToFormula() { 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 1e9b3d6ce..bcc2ed27d 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -29,16 +29,20 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.NoSuchElementException; import java.util.regex.Pattern; import javax.xml.namespace.QName; +import org.apache.commons.collections4.ListValuedMap; +import org.apache.commons.collections4.multimap.ArrayListValuedHashMap; import org.apache.poi.POIXMLDocument; import org.apache.poi.POIXMLDocumentPart; import org.apache.poi.POIXMLException; @@ -59,6 +63,7 @@ import org.apache.poi.ss.formula.SheetNameFormatter; 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.Name; import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; import org.apache.poi.ss.usermodel.Sheet; @@ -140,6 +145,11 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { */ private List sheets; + /** + * this holds the XSSFName objects attached to this workbook, keyed by lower-case name + */ + private ListValuedMap namedRangesByName; + /** * this holds the XSSFName objects attached to this workbook */ @@ -442,6 +452,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { stylesSource.setWorkbook(this); namedRanges = new ArrayList(); + namedRangesByName = new ArrayListValuedHashMap(); sheets = new ArrayList(); pivotTables = new ArrayList(); } @@ -733,8 +744,13 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { public XSSFName createName() { CTDefinedName ctName = CTDefinedName.Factory.newInstance(); ctName.setName(""); + return createAndStoreName(ctName); + } + + private XSSFName createAndStoreName(CTDefinedName ctName) { XSSFName name = new XSSFName(ctName, this); namedRanges.add(name); + namedRangesByName.put(ctName.getName().toLowerCase(Locale.ENGLISH), name); return name; } @@ -938,28 +954,47 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { return stylesSource.getFontAt(idx); } + /** + * Get the first named range with the given name. + * + * Note: names of named ranges are not unique as they are scoped by sheet. + * {@link #getNames(String name)} returns all named ranges with the given name. + * + * @param name named range name + * @return XSSFName with the given name. null is returned no named range could be found. + */ @Override public XSSFName getName(String name) { - int nameIndex = getNameIndex(name); - if (nameIndex < 0) { + Collection list = getNames(name); + if (list.isEmpty()) { return null; } - return namedRanges.get(nameIndex); + return list.iterator().next(); } + /** + * Get the named ranges with the given name. + * Note:Excel named ranges are case-insensitive and + * this method performs a case-insensitive search. + * + * @param name named range name + * @return list of XSSFNames with the given name. An empty list if no named ranges could be found + */ @Override public List getNames(String name) { - List names = new ArrayList(); - for(XSSFName nr : namedRanges) { - if(nr.getNameName().equals(name)) { - names.add(nr); - } - } - - return names; + return Collections.unmodifiableList(namedRangesByName.get(name.toLowerCase(Locale.ENGLISH))); } + /** + * Get the named range at the given index. + * + * @param nameIndex the index of the named range + * @return the XSSFName at the given index + * + * @deprecated 3.16. New projects should avoid accessing named ranges by index. + */ @Override + @Deprecated public XSSFName getNameAt(int nameIndex) { int nNames = namedRanges.size(); if (nNames < 1) { @@ -973,21 +1008,30 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { } /** - * Gets the named range index by his name - * Note:Excel named ranges are case-insensitive and - * this method performs a case-insensitive search. + * Get a list of all the named ranges in the workbook. * - * @param name named range name - * @return named range index + * @return list of XSSFNames in the workbook */ @Override + public List getAllNames() { + return Collections.unmodifiableList(namedRanges); + } + + /** + * Gets the named range index by name. + * + * @param name named range name + * @return named range index. -1 is returned if no named ranges could be found. + * + * @deprecated 3.16. New projects should avoid accessing named ranges by index. + * Use {@link #getName(String)} instead. + */ + @Override + @Deprecated public int getNameIndex(String name) { - int i = 0; - for(XSSFName nr : namedRanges) { - if(nr.getNameName().equals(name)) { - return i; - } - i++; + XSSFName nm = getName(name); + if (nm != null) { + return namedRanges.indexOf(nm); } return -1; } @@ -1258,22 +1302,40 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { return getPackagePart().getContentType().equals(XSSFRelation.MACROS_WORKBOOK.getContentType()); } + /** + * Remove the named range at the given index. + * + * @param index the index of the named range name to remove + * + * @deprecated 3.16. New projects should use {@link #removeName(Name)}. + */ @Override + @Deprecated public void removeName(int nameIndex) { - namedRanges.remove(nameIndex); + removeName(getNameAt(nameIndex)); } + /** + * Remove the first named range found with the given name. + * + * Note: names of named ranges are not unique (name + sheet + * index is unique), so {@link #removeName(Name)} should + * be used if possible. + * + * @param name the named range name to remove + * + * @throws IllegalArgumentException if no named range could be found + * + * @deprecated 3.16. New projects should use {@link #removeName(Name)}. + */ @Override + @Deprecated public void removeName(String name) { - int idx = 0; - for (XSSFName nm : namedRanges) { - if(nm.getNameName().equalsIgnoreCase(name)) { - removeName(idx); - return; - } - idx++; + List names = namedRangesByName.get(name.toLowerCase(Locale.ENGLISH)); + if (names.isEmpty()) { + throw new IllegalArgumentException("Named range was not found: " + name); } - throw new IllegalArgumentException("Named range was not found: " + name); + removeName(names.get(0)); } @@ -1282,13 +1344,24 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { * (name + sheet index is unique), this method is more accurate. * * @param name the name to remove. + * + * @throws IllegalArgumentException if the named range is not a part of this XSSFWorkbook */ - void removeName(XSSFName name) { - if (!namedRanges.remove(name)) { + @Override + public void removeName(Name name) { + if (!namedRangesByName.removeMapping(name.getNameName().toLowerCase(Locale.ENGLISH), name) + || !namedRanges.remove(name)) { throw new IllegalArgumentException("Name was not found: " + name); } } + void updateName(XSSFName name, String oldName) { + if (!namedRangesByName.removeMapping(oldName.toLowerCase(Locale.ENGLISH), name)) { + throw new IllegalArgumentException("Name was not found: " + name); + } + namedRangesByName.put(name.getNameName().toLowerCase(Locale.ENGLISH), name); + } + /** * Delete the printarea for the sheet specified @@ -1297,13 +1370,9 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { */ @Override public void removePrintArea(int sheetIndex) { - int cont = 0; - for (XSSFName name : namedRanges) { - if (name.getNameName().equals(XSSFName.BUILTIN_PRINT_AREA) && name.getSheetIndex() == sheetIndex) { - namedRanges.remove(cont); - break; - } - cont++; + XSSFName name = getBuiltInName(XSSFName.BUILTIN_PRINT_AREA, sheetIndex); + if (name != null) { + removeName(name); } } @@ -1369,17 +1438,20 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { } //adjust indices of names ranges - for (Iterator it = namedRanges.iterator(); it.hasNext();) { - XSSFName nm = it.next(); + List toRemove = new ArrayList(); + for (XSSFName nm : namedRanges) { CTDefinedName ct = nm.getCTName(); if(!ct.isSetLocalSheetId()) continue; if (ct.getLocalSheetId() == index) { - it.remove(); + toRemove.add(nm); } else if (ct.getLocalSheetId() > index){ // Bump down by one, so still points at the same sheet ct.setLocalSheetId(ct.getLocalSheetId()-1); } } + for (XSSFName nm : toRemove) { + removeName(nm); + } } /** @@ -1514,8 +1586,8 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { } XSSFName getBuiltInName(String builtInCode, int sheetNumber) { - for (XSSFName name : namedRanges) { - if (name.getNameName().equalsIgnoreCase(builtInCode) && name.getSheetIndex() == sheetNumber) { + for (XSSFName name : namedRangesByName.get(builtInCode.toLowerCase(Locale.ENGLISH))) { + if (name.getSheetIndex() == sheetNumber) { return name; } } @@ -1537,15 +1609,12 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { nameRecord.setName(builtInName); nameRecord.setLocalSheetId(sheetNumber); - XSSFName name = new XSSFName(nameRecord, this); - for (XSSFName nr : namedRanges) { - if (nr.equals(name)) - throw new POIXMLException("Builtin (" + builtInName - + ") already exists for sheet (" + sheetNumber + ")"); + if (getBuiltInName(builtInName, sheetNumber) != null) { + throw new POIXMLException("Builtin (" + builtInName + + ") already exists for sheet (" + sheetNumber + ")"); } - namedRanges.add(name); - return name; + return createAndStoreName(nameRecord); } /** @@ -1665,10 +1734,11 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { } private void reprocessNamedRanges() { + namedRangesByName = new ArrayListValuedHashMap(); namedRanges = new ArrayList(); if(workbook.isSetDefinedNames()) { for(CTDefinedName ctName : workbook.getDefinedNames().getDefinedNameArray()) { - namedRanges.add(new XSSFName(ctName, this)); + createAndStoreName(ctName); } } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFFormulaUtils.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFFormulaUtils.java index 1d146c09c..ef0c5ea63 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFFormulaUtils.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFFormulaUtils.java @@ -65,9 +65,7 @@ public final class XSSFFormulaUtils { */ public void updateSheetName(final int sheetIndex, final String oldName, final String newName) { // update named ranges - final int numberOfNames = _wb.getNumberOfNames(); - for (int i = 0; i < numberOfNames; i++) { - XSSFName nm = _wb.getNameAt(i); + for (XSSFName nm : _wb.getAllNames()) { if (nm.getSheetIndex() == -1 || nm.getSheetIndex() == sheetIndex) { updateName(nm, oldName, newName); } 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 63104dd9e..d11ed1fa8 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 @@ -83,9 +83,7 @@ public final class XSSFRowShifter extends RowShifter { public void updateNamedRanges(FormulaShifter shifter) { Workbook wb = sheet.getWorkbook(); XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create((XSSFWorkbook) wb); - final int numberOfNames = wb.getNumberOfNames(); - for (int i = 0; i < numberOfNames; i++) { - Name name = wb.getNameAt(i); + for (Name name : wb.getAllNames()) { String formula = name.getRefersToFormula(); int sheetIndex = name.getSheetIndex(); final int rowIndex = -1; //don't care, named ranges are not allowed to include structured references 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 3280a9121..2be21e830 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -115,25 +115,25 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { assertFalse(wb.isMacroEnabled()); assertEquals(3, wb.getNumberOfNames()); - assertEquals(0, wb.getNameAt(0).getCTName().getLocalSheetId()); - assertFalse(wb.getNameAt(0).getCTName().isSetLocalSheetId()); - assertEquals("SheetA!$A$1", wb.getNameAt(0).getRefersToFormula()); - assertEquals("SheetA", wb.getNameAt(0).getSheetName()); + assertEquals(0, wb.getName("SheetAA1").getCTName().getLocalSheetId()); + assertFalse(wb.getName("SheetAA1").getCTName().isSetLocalSheetId()); + assertEquals("SheetA!$A$1", wb.getName("SheetAA1").getRefersToFormula()); + assertEquals("SheetA", wb.getName("SheetAA1").getSheetName()); - assertEquals(0, wb.getNameAt(1).getCTName().getLocalSheetId()); - assertFalse(wb.getNameAt(1).getCTName().isSetLocalSheetId()); - assertEquals("SheetB!$A$1", wb.getNameAt(1).getRefersToFormula()); - assertEquals("SheetB", wb.getNameAt(1).getSheetName()); + assertEquals(0, wb.getName("SheetBA1").getCTName().getLocalSheetId()); + assertFalse(wb.getName("SheetBA1").getCTName().isSetLocalSheetId()); + assertEquals("SheetB!$A$1", wb.getName("SheetBA1").getRefersToFormula()); + assertEquals("SheetB", wb.getName("SheetBA1").getSheetName()); - assertEquals(0, wb.getNameAt(2).getCTName().getLocalSheetId()); - assertFalse(wb.getNameAt(2).getCTName().isSetLocalSheetId()); - assertEquals("SheetC!$A$1", wb.getNameAt(2).getRefersToFormula()); - assertEquals("SheetC", wb.getNameAt(2).getSheetName()); + assertEquals(0, wb.getName("SheetCA1").getCTName().getLocalSheetId()); + assertFalse(wb.getName("SheetCA1").getCTName().isSetLocalSheetId()); + assertEquals("SheetC!$A$1", wb.getName("SheetCA1").getRefersToFormula()); + assertEquals("SheetC", wb.getName("SheetCA1").getSheetName()); // Save and re-load, still there XSSFWorkbook nwb = XSSFTestDataSamples.writeOutAndReadBack(wb); assertEquals(3, nwb.getNumberOfNames()); - assertEquals("SheetA!$A$1", nwb.getNameAt(0).getRefersToFormula()); + assertEquals("SheetA!$A$1", nwb.getName("SheetAA1").getRefersToFormula()); nwb.close(); wb.close(); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java index 3e0f0b165..a188a11ed 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java @@ -53,9 +53,8 @@ public final class TestXSSFName extends BaseTestNamedRange { //sheet.createFreezePane(0, 3); } assertEquals(1, wb.getNumberOfNames()); - XSSFName nr1 = wb.getNameAt(0); + XSSFName nr1 = wb.getName(XSSFName.BUILTIN_PRINT_TITLE); - assertEquals(XSSFName.BUILTIN_PRINT_TITLE, nr1.getNameName()); assertEquals("'First Sheet'!$A:$A,'First Sheet'!$1:$4", nr1.getRefersToFormula()); //remove the columns part @@ -77,9 +76,8 @@ public final class TestXSSFName extends BaseTestNamedRange { wb.close(); assertEquals(1, nwb.getNumberOfNames()); - nr1 = nwb.getNameAt(0); + nr1 = nwb.getName(XSSFName.BUILTIN_PRINT_TITLE); - assertEquals(XSSFName.BUILTIN_PRINT_TITLE, nr1.getNameName()); assertEquals("'First Sheet'!$A:$A,'First Sheet'!$1:$4", nr1.getRefersToFormula()); // check that setting RR&C on a second sheet causes a new Print_Titles built-in @@ -89,7 +87,7 @@ public final class TestXSSFName extends BaseTestNamedRange { sheet2.setRepeatingColumns(CellRangeAddress.valueOf("B:C")); assertEquals(2, nwb.getNumberOfNames()); - XSSFName nr2 = nwb.getNameAt(1); + XSSFName nr2 = nwb.getNames(XSSFName.BUILTIN_PRINT_TITLE).get(1); assertEquals(XSSFName.BUILTIN_PRINT_TITLE, nr2.getNameName()); assertEquals("SecondSheet!$B:$C,SecondSheet!$1:$1", nr2.getRefersToFormula()); @@ -98,4 +96,38 @@ public final class TestXSSFName extends BaseTestNamedRange { sheet2.setRepeatingColumns(null); nwb.close(); } + + @Test + public void testSetNameName() throws Exception { + // Test that renaming named ranges doesn't break our new named range map + XSSFWorkbook wb = new XSSFWorkbook(); + wb.createSheet("First Sheet"); + + // Two named ranges called "name1", one scoped to sheet1 and one globally + XSSFName nameSheet1 = wb.createName(); + nameSheet1.setNameName("name1"); + nameSheet1.setRefersToFormula("'First Sheet'!$A$1"); + nameSheet1.setSheetIndex(0); + + XSSFName nameGlobal = wb.createName(); + nameGlobal.setNameName("name1"); + nameGlobal.setRefersToFormula("'First Sheet'!$B$1"); + + // Rename sheet-scoped name to "name2", check everything is updated properly + // and that the other name is unaffected + nameSheet1.setNameName("name2"); + assertEquals(1, wb.getNames("name1").size()); + assertEquals(1, wb.getNames("name2").size()); + assertEquals(nameGlobal, wb.getName("name1")); + assertEquals(nameSheet1, wb.getName("name2")); + + // Rename the other name to "name" and check everything again + nameGlobal.setNameName("name2"); + assertEquals(0, wb.getNames("name1").size()); + assertEquals(2, wb.getNames("name2").size()); + assertTrue(wb.getNames("name2").contains(nameGlobal)); + assertTrue(wb.getNames("name2").contains(nameSheet1)); + + wb.close(); + } } 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 c3420c780..7f832c4d9 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java @@ -1140,4 +1140,44 @@ public final class TestXSSFWorkbook extends BaseTestXWorkbook { wb.close(); } + + @Test + public void testRemoveSheet() throws IOException { + // Test removing a sheet maintains the named ranges correctly + XSSFWorkbook wb = new XSSFWorkbook(); + wb.createSheet("Sheet1"); + wb.createSheet("Sheet2"); + + XSSFName sheet1Name = wb.createName(); + sheet1Name.setNameName("name1"); + sheet1Name.setSheetIndex(0); + sheet1Name.setRefersToFormula("Sheet1!$A$1"); + + XSSFName sheet2Name = wb.createName(); + sheet2Name.setNameName("name1"); + sheet2Name.setSheetIndex(1); + sheet2Name.setRefersToFormula("Sheet2!$A$1"); + + assertTrue(wb.getAllNames().contains(sheet1Name)); + assertTrue(wb.getAllNames().contains(sheet2Name)); + + assertEquals(2, wb.getNames("name1").size()); + assertEquals(sheet1Name, wb.getNames("name1").get(0)); + assertEquals(sheet2Name, wb.getNames("name1").get(1)); + + // Remove sheet1, we should only have sheet2Name now + wb.removeSheetAt(0); + + assertFalse(wb.getAllNames().contains(sheet1Name)); + assertTrue(wb.getAllNames().contains(sheet2Name)); + assertEquals(1, wb.getNames("name1").size()); + assertEquals(sheet2Name, wb.getNames("name1").get(0)); + + // Check by index as well for sanity + assertEquals(1, wb.getNumberOfNames()); + assertEquals(0, wb.getNameIndex("name1")); + assertEquals(sheet2Name, wb.getNameAt(0)); + + wb.close(); + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java index 75fb77e82..92e362df5 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestNamedRange.java @@ -201,11 +201,7 @@ public abstract class BaseTestNamedRange { assertEquals("The sheet already contains this name: aaa", e.getMessage()); } - int cnt = 0; - for (int i = 0; i < wb.getNumberOfNames(); i++) { - if("aaa".equals(wb.getNameAt(i).getNameName())) cnt++; - } - assertEquals(3, cnt); + assertEquals(3, wb.getNames("aaa").size()); wb.close(); } @@ -250,11 +246,11 @@ public abstract class BaseTestNamedRange { // Write the workbook to a file // Read the Excel file and verify its content Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1); - Name nm1 = wb2.getNameAt(wb2.getNameIndex("RangeTest1")); + Name nm1 = wb2.getName("RangeTest1"); assertTrue("Name is "+nm1.getNameName(),"RangeTest1".equals(nm1.getNameName())); assertTrue("Reference is "+nm1.getRefersToFormula(),(wb2.getSheetName(0)+"!$A$1:$L$41").equals(nm1.getRefersToFormula())); - Name nm2 = wb2.getNameAt(wb2.getNameIndex("RangeTest2")); + Name nm2 = wb2.getName("RangeTest2"); assertTrue("Name is "+nm2.getNameName(),"RangeTest2".equals(nm2.getNameName())); assertTrue("Reference is "+nm2.getRefersToFormula(),(wb2.getSheetName(1)+"!$A$1:$O$21").equals(nm2.getRefersToFormula())); @@ -466,11 +462,11 @@ public abstract class BaseTestNamedRange { wb1.getNameAt(0); Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1); - Name nm =wb2.getNameAt(wb2.getNameIndex("RangeTest")); + Name nm =wb2.getName("RangeTest"); assertTrue("Name is "+nm.getNameName(),"RangeTest".equals(nm.getNameName())); assertTrue("Reference is "+nm.getRefersToFormula(),(wb2.getSheetName(0)+"!$D$4:$E$8").equals(nm.getRefersToFormula())); - nm = wb2.getNameAt(wb2.getNameIndex("AnotherTest")); + nm = wb2.getName("AnotherTest"); assertTrue("Name is "+nm.getNameName(),"AnotherTest".equals(nm.getNameName())); assertTrue("Reference is "+nm.getRefersToFormula(),newNamedRange2.getRefersToFormula().equals(nm.getRefersToFormula())); @@ -499,8 +495,7 @@ public abstract class BaseTestNamedRange { namedCell.setRefersToFormula(reference); // retrieve the newly created named range - int namedCellIdx = wb.getNameIndex(cellName); - Name aNamedCell = wb.getNameAt(namedCellIdx); + Name aNamedCell = wb.getName(cellName); assertNotNull(aNamedCell); // retrieve the cell at the named range and test its contents @@ -540,8 +535,7 @@ public abstract class BaseTestNamedRange { namedCell.setRefersToFormula(reference); // retrieve the newly created named range - int namedCellIdx = wb.getNameIndex(cname); - Name aNamedCell = wb.getNameAt(namedCellIdx); + Name aNamedCell = wb.getName(cname); assertNotNull(aNamedCell); // retrieve the cell at the named range and test its contents diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java index 3f8febb11..efae0d80c 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java @@ -261,17 +261,17 @@ public abstract class BaseTestSheetShiftRows { name4.setSheetIndex(1); sheet1.shiftRows(0, 1, 2); //shift down the top row on Sheet1. - name1 = wb.getNameAt(0); + name1 = wb.getName("name1"); assertEquals("Sheet1!$A$3+Sheet1!$B$3", name1.getRefersToFormula()); - name2 = wb.getNameAt(1); + name2 = wb.getName("name2"); assertEquals("Sheet1!$A$3", name2.getRefersToFormula()); //name3 and name4 refer to Sheet2 and should not be affected - name3 = wb.getNameAt(2); + name3 = wb.getName("name3"); assertEquals("Sheet2!$A$1", name3.getRefersToFormula()); - name4 = wb.getNameAt(3); + name4 = wb.getName("name4"); assertEquals("A1", name4.getRefersToFormula()); wb.close();