diff options
16 files changed, 310 insertions, 111 deletions
diff --git a/.classpath b/.classpath index ae3f061285..3b3ff5b64c 100644 --- a/.classpath +++ b/.classpath @@ -18,18 +18,19 @@ <classpathentry kind="lib" path="lib/ant-1.9.4.jar"/> <classpathentry kind="lib" path="lib/ant-launcher-1.9.4.jar"/> <classpathentry kind="lib" path="lib/log4j-1.2.17.jar"/> - <classpathentry kind="lib" path="ooxml-lib/xmlbeans-2.6.0.jar"/> + <classpathentry exported="true" kind="lib" path="ooxml-lib/xmlbeans-2.6.0.jar"/> <classpathentry kind="lib" path="lib/hamcrest-core-1.3.jar"/> <classpathentry kind="lib" path="lib/junit-4.12.jar"/> <classpathentry kind="lib" path="ooxml-lib/curvesapi-1.04.jar"/> - <classpathentry kind="lib" path="ooxml-lib/ooxml-schemas-1.3.jar" sourcepath="ooxml-lib/ooxml-schemas-1.3-sources.jar"/> - <classpathentry kind="lib" path="ooxml-lib/ooxml-security-1.1.jar" sourcepath="ooxml-lib/ooxml-security-1.1-sources.jar"/> + <classpathentry exported="true" kind="lib" path="ooxml-lib/ooxml-schemas-1.3.jar" sourcepath="ooxml-lib/ooxml-schemas-1.3-sources.jar"/> + <classpathentry exported="true" kind="lib" path="ooxml-lib/ooxml-security-1.1.jar" sourcepath="ooxml-lib/ooxml-security-1.1-sources.jar"/> <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.6"/> - <classpathentry kind="lib" path="compile-lib/slf4j-api-1.7.12.jar"/> + <classpathentry exported="true" kind="lib" path="compile-lib/slf4j-api-1.7.12.jar"/> <classpathentry kind="lib" path="compile-lib/bcpkix-jdk15on-1.54.jar"/> <classpathentry kind="lib" path="compile-lib/bcprov-ext-jdk15on-1.54.jar"/> - <classpathentry kind="lib" path="compile-lib/xmlsec-2.0.6.jar"/> - <classpathentry kind="lib" path="lib/commons-codec-1.10.jar"/> - <classpathentry kind="lib" path="lib/commons-logging-1.2.jar"/> + <classpathentry exported="true" kind="lib" path="compile-lib/xmlsec-2.0.6.jar"/> + <classpathentry exported="true" kind="lib" path="lib/commons-codec-1.10.jar"/> + <classpathentry exported="true" kind="lib" path="lib/commons-logging-1.2.jar"/> + <classpathentry kind="lib" path="ooxml-lib/commons-collections4-4.1.jar"/> <classpathentry kind="output" path="build/eclipse"/> </classpath> @@ -184,6 +184,9 @@ under the License. <property name="ooxml.curvesapi.jar" location="${ooxml.lib}/curvesapi-1.04.jar"/> <property name="ooxml.curvesapi.url" value="${repository.m2}/maven2/com/github/virtuald/curvesapi/1.04/curvesapi-1.04.jar"/> + <property name="ooxml.commons-collections4.jar" location="${ooxml.lib}/commons-collections4-4.1.jar"/> + <property name="ooxml.commons-collections4.url" + value="${repository.m2}/maven2/org/apache/commons/commons-collections4/4.1/commons-collections4-4.1.jar"/> <property name="ooxml.xmlbeans23.jar" location="${ooxml.lib}/xmlbeans-2.3.0.jar"/> <property name="ooxml.xmlbeans23.url" value="${repository.m2}/maven2/org/apache/xmlbeans/xmlbeans/2.3.0/xmlbeans-2.3.0.jar"/> @@ -326,6 +329,7 @@ under the License. <path id="ooxml.classpath"> <pathelement location="${ooxml.curvesapi.jar}"/> + <pathelement location="${ooxml.commons-collections4.jar}"/> <pathelement location="${ooxml.xmlbeans26.jar}"/> <pathelement location="${ooxml.xsds.jar}"/> <path refid="main.classpath"/> @@ -623,6 +627,7 @@ under the License. <or> <and> <available file="${ooxml.curvesapi.jar}"/> + <available file="${ooxml.commons-collections4.jar}"/> <available file="${ooxml.xmlbeans23.jar}"/> <available file="${ooxml.xmlbeans26.jar}"/> </and> @@ -633,6 +638,7 @@ under the License. <target name="fetch-ooxml-jars" depends="check-ooxml-jars" unless="ooxml.jars.present"> <mkdir dir="${ooxml.lib}"/> <downloadfile src="${ooxml.curvesapi.url}" dest="${ooxml.curvesapi.jar}"/> + <downloadfile src="${ooxml.commons-collections4.url}" dest="${ooxml.commons-collections4.jar}"/> <downloadfile src="${ooxml.xmlbeans23.url}" dest="${ooxml.xmlbeans23.jar}.orig"/> <downloadfile src="${ooxml.xmlbeans26.url}" dest="${ooxml.xmlbeans26.jar}.orig"/> <!-- remove piccolo parser, so we don't use unsafe calls to it instead of using jaxp --> @@ -1874,7 +1880,7 @@ under the License. <globmapper from="*" to="${zipdir}/lib/*"/> </mappedresources> <mappedresources cache="true"> - <fileset dir="${ooxml.lib}" includes="xmlbeans-2.6.0.jar.orig,curvesapi-*.jar"/> + <fileset dir="${ooxml.lib}" includes="xmlbeans-2.6.0.jar.orig,commons-collections4-*.jar,curvesapi-*.jar"/> <regexpmapper from="^(.*\.jar)(\.orig)?$$" to="${zipdir}/ooxml-lib/\1"/> </mappedresources> <mappedresources cache="true"> @@ -2116,6 +2122,7 @@ under the License. <auxClasspath path="${ooxml.xsds.jar}" /> <auxClasspath path="${ooxml.security.jar}" /> <auxClasspath path="${ooxml.curvesapi.jar}" /> + <auxClasspath path="${ooxml.commons-collections4.jar}" /> <auxClasspath path="${ooxml.xmlbeans26.jar}" /> <auxClasspath path="${main.commons-codec.jar}" /> <auxClasspath path="${main.commons-logging.jar}" /> diff --git a/maven/poi-ooxml.pom b/maven/poi-ooxml.pom index 0f3cdb1d51..2f4c286bce 100644 --- a/maven/poi-ooxml.pom +++ b/maven/poi-ooxml.pom @@ -74,5 +74,10 @@ <artifactId>curvesapi</artifactId> <version>1.04</version> </dependency> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-collections4</artifactId> + <version>4.1</version> + </dependency> </dependencies> </project> diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 35f9862116..ba015a9f11 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<HSSFName> 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 f1f1a4cd2c..e52615fb1a 100644 --- a/src/java/org/apache/poi/ss/usermodel/Workbook.java +++ b/src/java/org/apache/poi/ss/usermodel/Workbook.java @@ -370,6 +370,13 @@ public interface Workbook extends Closeable, Iterable<Sheet> { List<? extends Name> getNames(String name); /** + * Returns all defined names. + * + * @return a list of the defined names. An empty list is returned if none is found. + */ + List<? extends Name> 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 @@ -408,6 +415,13 @@ public interface Workbook extends Closeable, Iterable<Sheet> { 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. * <p>In order for formulas such as "[MyOtherWorkbook]Sheet3!$A$5" 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 58e16c772c..331ad9a0fd 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java @@ -1007,11 +1007,24 @@ public class SXSSFWorkbook implements Workbook { } /** + * Returns all defined names + * + * @return all defined names + */ + @Override + public List<? extends Name> 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. <code>-1</code> 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,14 +1077,28 @@ 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); + } + /** * Sets the printarea for the sheet provided * <p> 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 f3c6bc2673..9c5e6ffb10 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 462bd6517f..0710626201 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 1e9b3d6cee..bcc2ed27d7 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; @@ -141,6 +146,11 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { private List<XSSFSheet> sheets; /** + * this holds the XSSFName objects attached to this workbook, keyed by lower-case name + */ + private ListValuedMap<String, XSSFName> namedRangesByName; + + /** * this holds the XSSFName objects attached to this workbook */ private List<XSSFName> namedRanges; @@ -442,6 +452,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { stylesSource.setWorkbook(this); namedRanges = new ArrayList<XSSFName>(); + namedRangesByName = new ArrayListValuedHashMap<String, XSSFName>(); sheets = new ArrayList<XSSFSheet>(); pivotTables = new ArrayList<XSSFPivotTable>(); } @@ -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. <code>null</code> is returned no named range could be found. + */ @Override public XSSFName getName(String name) { - int nameIndex = getNameIndex(name); - if (nameIndex < 0) { + Collection<XSSFName> list = getNames(name); + if (list.isEmpty()) { return null; } - return namedRanges.get(nameIndex); + return list.iterator().next(); } + /** + * Get the named ranges with the given name. + * <i>Note:</i>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<XSSFName> getNames(String name) { - List<XSSFName> names = new ArrayList<XSSFName>(); - 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 - * <i>Note:</i>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. + * + * @return list of XSSFNames in the workbook + */ + @Override + public List<XSSFName> getAllNames() { + return Collections.unmodifiableList(namedRanges); + } + + /** + * Gets the named range index by name. * * @param name named range name - * @return named range index + * @return named range index. <code>-1</code> 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<XSSFName> 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<XSSFName> it = namedRanges.iterator(); it.hasNext();) { - XSSFName nm = it.next(); + List<XSSFName> toRemove = new ArrayList<XSSFName>(); + 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<String, XSSFName>(); namedRanges = new ArrayList<XSSFName>(); 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 1d146c09c6..ef0c5ea633 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 63104dd9e2..d11ed1fa81 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 3280a9121b..2be21e830c 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 3e0f0b1651..a188a11ed3 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 c3420c780a..7f832c4d9c 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 75fb77e827..92e362df57 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 3f8febb11a..efae0d80c6 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(); |