improved XSSFName:check for name uniqueness, validate name name, more unit tests and javadocs

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

6 files changed, 321 insertions, 87 deletions

diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithPageSetup.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithPageSetup.java
index 6c8813c984..2e589484d9 100755
--- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithPageSetup.java
+++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/WorkingWithPageSetup.java
@@ -67,11 +67,6 @@ public class WorkingWithPageSetup {
         // Set the the repeating rows and columns on the second sheet.
         wb.setRepeatingRowsAndColumns(1,4,5,1,2);
 
-        // Set the the repeating rows and columns on the second sheet
-        wb.setRepeatingRowsAndColumns(1, 4, 5, 1, 2);
-
-
-
         //set the print area for the first sheet
         wb.setPrintArea(0, 1, 2, 0, 3);
 
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 7fa2ef967c..b5188e4d76 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java
@@ -17,34 +17,52 @@
 package org.apache.poi.xssf.usermodel;
 
 import org.apache.poi.ss.usermodel.Name;
+import org.apache.poi.ss.util.AreaReference;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedName;
 
 /**
- * XSSF Implementation of a Named Range
+ * Represents a defined named range in a SpreadsheetML workbook.
+ * <p>
+ * Defined names are descriptive text that is used to represents a cell, range of cells, formula, or constant value.
+ * Use easy-to-understand names, such as Products, to refer to hard to understand ranges, such as <code>Sales!C20:C30</code>.
+ * </p>
+ * Example:
+ * <pre><blockquote>
+ *   XSSFWorkbook wb = new XSSFWorkbook();
+ *   XSSFSheet sh = wb.createSheet("Sheet1");
+ *
+ *   //applies to the entire workbook
+ *   XSSFName name1 = wb.createName();
+ *   name1.setNameName("FMLA");
+ *   name1.setReference("Sheet1!$B$3");
+ *
+ *   //applies to Sheet1
+ *   XSSFName name2 = wb.createName();
+ *   name2.setNameName("SheetLevelName");
+ *   name2.setComment("This name is scoped to Sheet1");
+ *   name2.setLocalSheetId(0);
+ *   name2.setReference("Sheet1!$B$3");
+ *
+ * </blockquote></pre>
+ *
+ * @author Nick Burch
+ * @author Yegor Kozlov
  */
 public class XSSFName implements Name {
-    /**
-     * The following built-in names are defined in this SpreadsheetML
-     * specification:
-     * Built-in names reserved by SpreadsheetML begin with "_xlnm.". End users shall not use
-     * this string for custom names in the user interface.
-     */
 
     /**
-     * this defined name specifies the workbook's print area
+     * A built-in defined name that specifies the workbook's print area
      */
     public final static String BUILTIN_PRINT_AREA = "_xlnm.Print_Area";
 
     /**
-     * this defined name specifies the row(s) or column(s) to repeat
+     * A built-in defined name that specifies the row(s) or column(s) to repeat
      * at the top of each printed page.
      */
     public final static String BUILTIN_PRINT_TITLE = "_xlnm.Print_Titles";
 
-    //Filter & Advanced Filter
-
     /**
-     * this defined name refers to a range containing the criteria values
+     * A built-in defined name that refers to a range containing the criteria values
      * to be used in applying an advanced filter to a range of data
      */
     public final static String BUILTIN_CRITERIA = "_xlnm.Criteria:";
@@ -58,49 +76,44 @@ public class XSSFName implements Name {
     public final static String BUILTIN_EXTRACT = "_xlnm.Extract:";
 
     /**
-     * can be one of the following
-     * a. this defined name refers to a range to which an advanced filter has been
+     * ?an be one of the following
+     * <li> this defined name refers to a range to which an advanced filter has been
      * applied. This represents the source data range, unfiltered.
-     * b. This defined name refers to a range to which an AutoFilter has been
+     * <li> This defined name refers to a range to which an AutoFilter has been
      * applied
      */
     public final static String BUILTIN_FILTER_DB = "_xlnm._FilterDatabase:";
 
-    //Miscellaneous
-
     /**
-     * the defined name refers to a consolidation area
+     * A built-in defined name that refers to a consolidation area
      */
     public final static String BUILTIN_CONSOLIDATE_AREA = "_xlnm.Consolidate_Area";
 
     /**
-     * the range specified in the defined name is from a database data source
+     * A built-in defined name that specified that the range specified is from a database data source
      */
     public final static String BUILTIN_DATABASE = "_xlnm.Database";
 
     /**
-     * the defined name refers to a sheet title.
+     * A built-in defined name that refers to a sheet title.
      */
     public final static String BUILTIN_SHEET_TITLE = "_xlnm.Sheet_Title";
 
     private XSSFWorkbook workbook;
     private CTDefinedName ctName;
 
-    protected XSSFName(XSSFWorkbook workbook) {
-        this.workbook = workbook;
-        this.ctName = CTDefinedName.Factory.newInstance();
-    }
-
+    /**
+     * Creates an XSSFName object - called internally by XSSFWorkbook.
+     *
+     * @param name - the xml bean that holds data represenring this defined name.
+     * @param workbook - the workbook object associated with the name
+     * @see org.apache.poi.xssf.usermodel.XSSFWorkbook#createName()
+     */
     protected XSSFName(CTDefinedName name, XSSFWorkbook workbook) {
         this.workbook = workbook;
         this.ctName = name;
     }
 
-    public boolean isFunctionName() {
-        // TODO Figure out how HSSF does this, and do the same!
-        return ctName.getFunction(); // maybe this works - verify
-    }
-
     /**
      * Returns the underlying named range object
      */
@@ -108,84 +121,199 @@ public class XSSFName implements Name {
         return ctName;
     }
 
+    /**
+     * Returns the name that will appear in the user interface for the defined name.
+     *
+     * @return text name of this defined name
+     */
     public String getNameName() {
         return ctName.getName();
     }
 
-    public void setNameName(String nameName) {
-        ctName.setName(nameName);
+    /**
+     * Sets the name that will appear in the user interface for the defined name.
+     * Names must begin with a letter or underscore, not contain spaces and be unique across the workbook.
+     *
+     * @param name name of this defined name
+     * @throws IllegalArgumentException if the name is invalid or the workbook already contains this name (case-insensitive)
+     */
+    public void setNameName(String name) {
+        validateName(name);
+
+        //Check to ensure no other names have the same case-insensitive name
+        for (int i = 0; i < workbook.getNumberOfNames(); i++) {
+            XSSFName nm = workbook.getNameAt(i);
+            if (nm != this && nm.getNameName().equalsIgnoreCase(name)) {
+                throw new IllegalArgumentException("The workbook already contains this name: " + 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 getReference() {
         return ctName.getStringValue();
     }
 
+    /**
+     * Sets the reference of this named range, such as Sales!C20:C30.
+     *
+     * @param ref the reference to set
+     * @throws IllegalArgumentException if the specified reference is unparsable
+     */
     public void setReference(String ref) {
-        ctName.setStringValue(ref);
+        String normalizedRef = AreaReference.isContiguous(ref) ? new AreaReference(ref).formatAsString() : ref;
+        ctName.setStringValue(normalizedRef);
     }
 
+    /**
+     * 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 = getReference();
+        return ref != null && ref.indexOf("#REF!") != -1;
+    }
+
+    /**
+     * Tell Excel that this name applies to the worksheet with the specified index instead of the entire workbook.
+     *
+     * @param sheetId the sheet index this name applies to, -1 unsets this property making the name workbook-global
+     */
     public void setLocalSheetId(int sheetId) {
-        ctName.setLocalSheetId(sheetId);
+        if(sheetId == -1) ctName.unsetLocalSheetId();
+        else ctName.setLocalSheetId(sheetId);
     }
 
+    /**
+     * Returns the sheet index this name applies to.
+     *
+     * @return the sheet index this name applies to, -1 if this name applies to the entire workbook
+     */
     public int getLocalSheetId() {
-        return (int) ctName.getLocalSheetId();
+        return ctName.isSetLocalSheetId() ? (int) ctName.getLocalSheetId() : -1;
     }
 
-
+    /**
+     * Indicates that the defined name refers to a user-defined function.
+     * This attribute is used when there is an add-in or other code project associated with the file.
+     *
+     * @param value <code>true</code> indicates the name refers to a function.
+     */
     public void setFunction(boolean value) {
         ctName.setFunction(value);
     }
 
+    /**
+     * Indicates that the defined name refers to a user-defined function.
+     * This attribute is used when there is an add-in or other code project associated with the file.
+     *
+     * @return <code>true</code> indicates the name refers to a function.
+     */
     public boolean getFunction() {
         return ctName.getFunction();
     }
 
+    /**
+     * Specifies the function group index if the defined name refers to a function. The function
+     * group defines the general category for the function. This attribute is used when there is
+     * an add-in or other code project associated with the file.
+     *
+     * @param functionGroupId the function group index that defines the general category for the function
+     */
     public void setFunctionGroupId(int functionGroupId) {
         ctName.setFunctionGroupId(functionGroupId);
     }
 
+    /**
+     * Returns the function group index if the defined name refers to a function. The function
+     * group defines the general category for the function. This attribute is used when there is
+     * an add-in or other code project associated with the file.
+     *
+     * @return the function group index that defines the general category for the function
+     */
     public int getFunctionGroupId() {
         return (int) ctName.getFunctionGroupId();
     }
 
+    /**
+     * Get the sheets name which this named range is referenced to
+     *
+     * @return sheet name, which this named range referred to.
+     * Empty string if the referenced sheet name weas not found.
+     */
     public String getSheetName() {
         if (ctName.isSetLocalSheetId()) {
             // Given as explicit sheet id
-            long sheetId = ctName.getLocalSheetId();
-            if (sheetId >= 0) {
-                return workbook.getSheetName((int) sheetId);
-            }
+            int sheetId = (int)ctName.getLocalSheetId();
+            return workbook.getSheetName(sheetId);
         } else {
-            // Is it embeded in the reference itself?
-            int excl = getReference().indexOf('!');
-            if (excl > -1) {
-                return getReference().substring(0, excl);
-            }
+            String ref = getReference();
+            AreaReference areaRef = new AreaReference(ref);
+            return areaRef.getFirstCell().getSheetName();
         }
+    }
 
-        // Not given at all
-        return null;
+    /**
+     * Is the name refers to a user-defined function ?
+     *
+     * @return <code>true</code> if this name refers to a user-defined function
+     */
+    public boolean isFunctionName() {
+        return getFunction();
     }
 
+    /**
+     * Returns the comment the user provided when the name was created.
+     *
+     * @return the user comment for this named range
+     */
     public String getComment() {
         return ctName.getComment();
     }
 
+    /**
+     * Specifies the comment the user provided when the name was created.
+     *
+     * @param comment  the user comment for this named range
+     */
     public void setComment(String comment) {
         ctName.setComment(comment);
     }
 
-
+    @Override
     public int hashCode() {
         return ctName.toString().hashCode();
     }
 
+    /**
+     * Compares this name to the specified object.
+     * The result is <code>true</code> if the argument is XSSFName and the
+     * underlying CTDefinedName bean equals to the CTDefinedName representing this name
+     *
+     * @param   o   the object to compare this <code>XSSFName</code> against.
+     * @return  <code>true</code> if the <code>XSSFName </code>are equal;
+     *          <code>false</code> otherwise.
+     */
+    @Override
     public boolean equals(Object o) {
+        if(o == this) return true;
+
         if (!(o instanceof XSSFName)) return false;
+
         XSSFName cf = (XSSFName) o;
         return ctName.toString().equals(cf.getCTName().toString());
     }
 
-
+    private static void validateName(String name){
+        char c = name.charAt(0);
+        if(!(c == '_' || Character.isLetter(c)) || name.indexOf(' ') != -1) {
+            throw new IllegalArgumentException("Invalid name: '"+name+"'; Names must begin with a letter or underscore and not contain spaces");
+        }
+    }
 }
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
index 8efef69b48..654802dd7d 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
@@ -215,6 +215,15 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
     }
 
     /**
+     * Returns the name of this sheet
+     * 
+     * @return the name of this sheet
+     */
+    public String getSheetName() {
+        return sheet.getName();
+    }
+
+    /**
      * Sdds a merged region of cells (hence those cells form one)
      *
      * @param cra (rowfrom/colfrom-rowto/colto) to merge
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 fa8dea9fbd..ebb20e96eb 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
@@ -375,7 +375,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
      * @return named range high level
      */
     public XSSFName createName() {
-        XSSFName name = new XSSFName(this);
+        XSSFName name = new XSSFName(CTDefinedName.Factory.newInstance(), this);
         namedRanges.add(name);
         return name;
     }
@@ -488,6 +488,12 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
         return pictures;
     }
 
+    /**
+     * gGet the cell style object at the given index
+     *
+     * @param idx  index within the set of styles
+     * @return XSSFCellStyle object at the index
+     */
     public XSSFCellStyle getCellStyleAt(short idx) {
         return stylesSource.getStyleAt(idx);
     }
@@ -699,35 +705,29 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
     }
 
     /**
-     * removes the name
+     * Removes the name by its index
      *
-     * @param nameIndex name index
+     * @param nameIndex 0-based index of the name to remove.
      */
     public void removeName(int nameIndex) {
-        if (namedRanges.size() > nameIndex) {
-            XSSFName name = getNameAt(nameIndex);
-            int cont = 0;
-            for (XSSFName nameRange : namedRanges) {
-                if (nameRange.getReference().equals(name.getReference())) {
-                    namedRanges.remove(cont);
-                    getDefinedNames().removeDefinedName(nameIndex);
-                    break;
-                }
-                cont++;
-            }
-        }
+        namedRanges.remove(nameIndex);
     }
 
     /**
-     * removes the name
+     * Remove the named range by its name
      *
-     * @param name range
-     *             name index
+     * @param name named range name
+     * @throws IllegalArgumentException if the name was not found
      */
     public void removeName(String name) {
-        //TODO
-        //int index=getNameIndex(name);
-        //removeName(index);
+        for (int i = 0; i < namedRanges.size(); i++) {
+            XSSFName nm = namedRanges.get(i);
+            if(nm.getNameName().equalsIgnoreCase(name)) {
+                removeName(i);
+                return;
+            }
+        }
+        throw new IllegalArgumentException("Named range was not found: " + name);
     }
 
     /**
@@ -852,9 +852,9 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
             name = createBuiltInName(XSSFName.BUILTIN_PRINT_AREA, sheetIndex);
             namedRanges.add(name);
         }
-	//short externSheetIndex = getWorkbook().checkExternSheet(sheetIndex);
-	//name.setExternSheetNumber(externSheetIndex);
-	name.setReference(reference);
+        //short externSheetIndex = getWorkbook().checkExternSheet(sheetIndex);
+        //name.setExternSheetNumber(externSheetIndex);
+        name.setReference(reference);
     }
 
     /**
@@ -948,11 +948,6 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
         return "'" + sheetName + "'!$" + colRef.getCellRefParts()[2] + "$" + colRef.getCellRefParts()[1] + ":$" + colRef2.getCellRefParts()[2] + "$" + colRef2.getCellRefParts()[1];
     }
 
-    private CTDefinedNames getDefinedNames() {
-        return workbook.getDefinedNames() == null ? workbook.addNewDefinedNames() : workbook.getDefinedNames();
-    }
-
-
     private XSSFName getBuiltInName(String builtInCode, int sheetNumber) {
         for (XSSFName name : namedRanges) {
             if (name.getNameName().equalsIgnoreCase(builtInCode) && name.getLocalSheetId() == sheetNumber) {
@@ -972,7 +967,8 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
             throw new IllegalArgumentException("Sheet number [" + sheetNumber + "]is not valid ");
         }
 
-        CTDefinedName nameRecord = getDefinedNames().addNewDefinedName();
+        CTDefinedNames names = workbook.getDefinedNames() == null ? workbook.addNewDefinedNames() : workbook.getDefinedNames();
+        CTDefinedName nameRecord = names.addNewDefinedName();
         nameRecord.setName(builtInName);
         nameRecord.setLocalSheetId(sheetNumber);
 
@@ -1068,7 +1064,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
     }
 
     /**
-     * Method write - write out this workbook to an Outputstream.
+     * Write out this workbook to an Outputstream.
      *
      * @param stream - the java OutputStream you wish to write the XLS to
      *
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java
new file mode 100755
index 0000000000..1ba9064e91
--- /dev/null
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFName.java
@@ -0,0 +1,106 @@
+/* ====================================================================

+   Licensed to the Apache Software Foundation (ASF) under one or more

+   contributor license agreements.  See the NOTICE file distributed with

+   this work for additional information regarding copyright ownership.

+   The ASF licenses this file to You under the Apache License, Version 2.0

+   (the "License"); you may not use this file except in compliance with

+   the License.  You may obtain a copy of the License at

+

+       http://www.apache.org/licenses/LICENSE-2.0

+

+   Unless required by applicable law or agreed to in writing, software

+   distributed under the License is distributed on an "AS IS" BASIS,

+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

+   See the License for the specific language governing permissions and

+   limitations under the License.

+==================================================================== */

+package org.apache.poi.xssf.usermodel;

+

+import junit.framework.TestCase;

+import org.apache.poi.xssf.XSSFTestDataSamples;

+

+/**

+ * @author Yegor Kozlov

+ */

+public class TestXSSFName extends TestCase {

+

+    public void testCreate(){

+        // Create a new workbook

+        XSSFWorkbook wb = new XSSFWorkbook();

+

+

+        // Create a worksheet 'sheet1' in the new workbook

+        XSSFName name1 = wb.createName();

+        name1.setNameName("testOne");

+

+        XSSFName name2 = wb.createName();

+        try {

+            name2.setNameName("testOne");

+            fail("expected exception");

+        } catch (IllegalArgumentException e){

+            assertEquals("The workbook already contains this name: testOne", e.getMessage());

+        }

+

+        name2.setNameName("testTwo");

+

+        String ref1 = "Test1!$A$1:$B$1";

+        name1.setReference(ref1);

+        assertEquals(ref1, name1.getReference());

+        assertEquals("Test1", name1.getSheetName());

+

+        String ref2 = "Testing Named Ranges!$A$1:$B$1";

+        name1.setReference(ref2);

+        //XSSFName#setReference stores the reference in canonical form and puts the sheet name in single quotes

+        assertEquals("'Testing Named Ranges'!$A$1:$B$1", name1.getReference());

+        assertEquals("Testing Named Ranges", name1.getSheetName());

+

+        //setting invalid reference should throw IllegalArgumentException

+        try {

+            name1.setReference("invalid");

+            fail("expected exception");

+        } catch (IllegalArgumentException e){

+            ;

+        }

+

+        assertEquals(-1, name1.getLocalSheetId());

+        name1.setLocalSheetId(1);

+        assertEquals(1, name1.getLocalSheetId());

+    }

+

+    public void testUnicodeNamedRange() {

+        XSSFWorkbook workBook = new XSSFWorkbook();

+        workBook.createSheet("Test");

+        XSSFName name = workBook.createName();

+        name.setNameName("\u03B1");

+        name.setReference("Test!$D$3:$E$8");

+

+

+        XSSFWorkbook workBook2 = XSSFTestDataSamples.writeOutAndReadBack(workBook);

+        XSSFName name2 = workBook2.getNameAt(0);

+

+        assertEquals("\u03B1", name2.getNameName());

+        assertEquals("Test!$D$3:$E$8", name2.getReference());

+    }

+

+    public void testAddRemove() {

+        XSSFWorkbook wb = new XSSFWorkbook();

+        assertEquals(0, wb.getNumberOfNames());

+        XSSFName name1 = wb.createName();

+        name1.setNameName("name1");

+        assertEquals(1, wb.getNumberOfNames());

+

+        XSSFName name2 = wb.createName();

+        name2.setNameName("name2");

+        assertEquals(2, wb.getNumberOfNames());

+

+        XSSFName name3 = wb.createName();

+        name3.setNameName("name3");

+        assertEquals(3, wb.getNumberOfNames());

+

+        wb.removeName("name2");

+        assertEquals(2, wb.getNumberOfNames());

+

+        wb.removeName(0);

+        assertEquals(1, wb.getNumberOfNames());

+    }

+}

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 d54ba35ae3..079cee2e93 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
@@ -208,8 +208,8 @@ public final class TestXSSFWorkbook extends TestCase {
 		
 		
 		nwb.setRepeatingRowsAndColumns(1, -1, -1, -1, -1);
-		
-		
+
+
 		if (false) {
 			// In case you fancy checking in excel, to ensure it
 			//  won't complain about the file now