Fix for bug #44167 - patch from Paolo Mottadelli

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

7 files changed, 260 insertions, 16 deletions

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index f9a6353202..61006908f0 100644
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -36,6 +36,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release versionse version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="fix">44167 - Fix for non-contiguous named ranges</action>
             <action dev="POI-DEVELOPERS" type="fix">44070 - Fix for shifting comments when shifting rows</action>
         </release>
         <release version="3.0.2-BETA2" date="2008-01-12">
diff --git a/src/documentation/content/xdocs/hssf/quick-guide.xml b/src/documentation/content/xdocs/hssf/quick-guide.xml
index ddbd447fd1..594a1a761f 100644
--- a/src/documentation/content/xdocs/hssf/quick-guide.xml
+++ b/src/documentation/content/xdocs/hssf/quick-guide.xml
@@ -1159,7 +1159,34 @@ Examples:
         // extract the cell contents based on cell type etc.
     }
             </source>
+            <p>
+            Reading from non-contiguous Named Ranges
+            </p>
+            <source>
+    // Setup code
+    String cname = "TestName";
+    HSSFWorkbook wb = getMyWorkbook(); // retrieve workbook
 
+    // Retrieve the named range
+    // Will be something like "$C$10,$D$12:$D$14";
+    int namedCellIdx = wb.getNameIndex(cellName);
+    HSSFName aNamedCell = wb.getNameAt(namedCellIdx);
+
+    // Retrieve the cell at the named range and test its contents
+    // Will get back one AreaReference for C10, and
+    //  another for D12 to D14
+    AreaReference[] arefs = AreaReference.generateContiguous(aNamedCell.getReference());
+    for(int i=0; i<arefs.length; i++) {
+        CellReference[] crefs = arefs[i].getCells();
+        for (int j=0; j<crefs.length; j++) {
+	    // Check it turns into real stuff
+	    HSSFSheet s = wb.getSheet(crefs[j].getSheetName());
+	    HSSFRow r = s.getRow(crefs[j].getRow());
+	    HSSFCell c = r.getCell(crefs[j].getCol());
+            // extract the cell contents based on cell type etc.
+        }
+    }
+            </source>
         </section>
         <anchor id="CellComments"/>
         <section><title>Cell Comments</title>
diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 03ed8a9db5..d3af731337 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -33,6 +33,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="fix">44167 - Fix for non-contiguous named ranges</action>
             <action dev="POI-DEVELOPERS" type="fix">44070 - Fix for shifting comments when shifting rows</action>
         </release>
         <release version="3.0.2-BETA2" date="2008-01-12">
diff --git a/src/java/org/apache/poi/hssf/record/NameRecord.java b/src/java/org/apache/poi/hssf/record/NameRecord.java
index 4014d7e7ed..9ddec5a819 100644
--- a/src/java/org/apache/poi/hssf/record/NameRecord.java
+++ b/src/java/org/apache/poi/hssf/record/NameRecord.java
@@ -20,13 +20,10 @@ package org.apache.poi.hssf.record;
 
 import java.util.List;
 import java.util.Stack;
+import java.util.Iterator;
 
 import org.apache.poi.hssf.model.Workbook;
-import org.apache.poi.hssf.record.formula.Area3DPtg;
-import org.apache.poi.hssf.record.formula.DeletedArea3DPtg;
-import org.apache.poi.hssf.record.formula.DeletedRef3DPtg;
-import org.apache.poi.hssf.record.formula.Ptg;
-import org.apache.poi.hssf.record.formula.Ref3DPtg;
+import org.apache.poi.hssf.record.formula.*;
 import org.apache.poi.hssf.util.RangeAddress;
 import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
@@ -648,15 +645,44 @@ public class NameRecord extends Record {
         Ptg ptg = (Ptg) field_13_name_definition.peek();
         String result = "";
 
-        if (ptg.getClass() == Area3DPtg.class){
-            result = ptg.toFormulaString(book);
+        // If it's a union, descend in and process
+        if (ptg.getClass() == UnionPtg.class) {
+            Iterator it =field_13_name_definition.iterator();
+            while( it.hasNext() ) {
+                Ptg p = (Ptg)it.next();
+
+                String thisRes = getAreaRefString(p, book);
+                if(thisRes.length() > 0) {
+                    // Add a comma to the end if needed
+                    if(result.length() > 0 && !result.endsWith(",")) {
+                        result += ",";
+                    }
+                    // And add the string it corresponds to
+                    result += thisRes;
+                }
+            }
+        } else {
+            // Otherwise just get the string
+            result = getAreaRefString(ptg, book);
+        }
+
+        return result;
+    }
 
+    /**
+     * Turn the given ptg into a string, or
+     *  return an empty string if nothing is possible
+     *  for it.
+     */
+    private String getAreaRefString(Ptg ptg,Workbook book) {
+        if (ptg.getClass() == Area3DPtg.class){
+            return ptg.toFormulaString(book);
         } else if (ptg.getClass() == Ref3DPtg.class){
-            result = ptg.toFormulaString(book);
+            return ptg.toFormulaString(book);
         } else if (ptg.getClass() == DeletedArea3DPtg.class || ptg.getClass() == DeletedRef3DPtg.class) {
-        	result = "#REF!"   ;     }
-
-        return result;
+        	return "#REF!";
+        }
+        return "";
     }
 
     /** sets the reference , the area only (range)
diff --git a/src/java/org/apache/poi/hssf/util/AreaReference.java b/src/java/org/apache/poi/hssf/util/AreaReference.java
index a881dbabb9..ae83431983 100644
--- a/src/java/org/apache/poi/hssf/util/AreaReference.java
+++ b/src/java/org/apache/poi/hssf/util/AreaReference.java
@@ -18,15 +18,24 @@
 
 package org.apache.poi.hssf.util;
 
+import java.util.ArrayList;
+import java.util.StringTokenizer;
+
 public class AreaReference {
 
 
 private CellReference [] cells;
 private int dim;
 
-    /** Create an area ref from a string representation
+    /**
+     * Create an area ref from a string representation.
+     * The area reference must be contiguous
      */
     public AreaReference(String reference) {
+        if(! isContiguous(reference)) {
+            throw new IllegalArgumentException("References passed to the AreaReference must be contiguous, use generateContiguous(ref) if you have non-contiguous references");
+        }
+
         String[] refs = seperateAreaRefs(reference);
         dim = refs.length;
         cells = new CellReference[dim];
@@ -34,6 +43,36 @@ private int dim;
             cells[i]=new CellReference(refs[i]);
         }
     }
+
+    /**
+     * Is the reference for a contiguous (i.e.
+     *  unbroken) area, or is it made up of
+     *  several different parts?
+     * (If it is, you will need to call
+     *  ....
+     */
+    public static boolean isContiguous(String reference) {
+        if(reference.indexOf(',') == -1) {
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Takes a non-contiguous area reference, and
+     *  returns an array of contiguous area references.
+     */
+    public static AreaReference[] generateContiguous(String reference) {
+        ArrayList refs = new ArrayList();
+        StringTokenizer st = new StringTokenizer(reference, ",");
+        while(st.hasMoreTokens()) {
+            refs.add(
+                    new AreaReference(st.nextToken())
+            );
+        }
+        return (AreaReference[])refs.toArray(new AreaReference[refs.size()]);
+    }
+
     //not sure if we need to be flexible here!
     /** return the dimensions of this area
      **/
diff --git a/src/testcases/org/apache/poi/hssf/data/44167.xls b/src/testcases/org/apache/poi/hssf/data/44167.xls
new file mode 100755
index 0000000000..c5470210d8
--- /dev/null
+++ b/src/testcases/org/apache/poi/hssf/data/44167.xls
diff --git a/src/testcases/org/apache/poi/hssf/util/TestAreaReference.java b/src/testcases/org/apache/poi/hssf/util/TestAreaReference.java
index 0663920461..b58e498bf0 100644
--- a/src/testcases/org/apache/poi/hssf/util/TestAreaReference.java
+++ b/src/testcases/org/apache/poi/hssf/util/TestAreaReference.java
@@ -21,9 +21,16 @@ package org.apache.poi.hssf.util;
 
 import junit.framework.TestCase;
 
-import org.apache.poi.hssf.usermodel.HSSFCell;
-import org.apache.poi.hssf.usermodel.HSSFRow;
-import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.*;
+import org.apache.poi.hssf.model.Workbook;
+import org.apache.poi.hssf.record.NameRecord;
+import org.apache.poi.hssf.record.formula.MemFuncPtg;
+import org.apache.poi.hssf.record.formula.Area3DPtg;
+import org.apache.poi.hssf.record.formula.UnionPtg;
+
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.util.List;
 
 public class TestAreaReference extends TestCase {
      public TestAreaReference(String s) {
@@ -60,8 +67,151 @@ public class TestAreaReference extends TestCase {
     	assertEquals("Not Column B", (short)1,myCellReference[0].getCol());
 		assertEquals("Not Row 5", 4,myCellReference[0].getRow());
     }
+
+    private static class HSSFWB extends HSSFWorkbook {
+        private HSSFWB(InputStream in) throws Exception {
+            super(in);
+        }
+        public Workbook getWorkbook() {
+            return super.getWorkbook();
+        }
+    }
+
+    public void testContiguousReferences() throws Exception {
+        String refSimple = "$C$10";
+        String ref2D = "$C$10:$D$11";
+        String refDCSimple = "$C$10,$D$12,$E$14";
+        String refDC2D = "$C$10:$C$11,$D$12,$E$14:$E$20";
+        String refDC3D = "Tabelle1!$C$10:$C$14,Tabelle1!$D$10:$D$12";
+
+        // Check that we detect as contiguous properly
+        assertTrue(AreaReference.isContiguous(refSimple));
+        assertTrue(AreaReference.isContiguous(ref2D));
+        assertFalse(AreaReference.isContiguous(refDCSimple));
+        assertFalse(AreaReference.isContiguous(refDC2D));
+        assertFalse(AreaReference.isContiguous(refDC3D));
+
+        // Check we can only create contiguous entries
+        new AreaReference(refSimple);
+        new AreaReference(ref2D);
+        try {
+            new AreaReference(refDCSimple);
+            fail();
+        } catch(IllegalArgumentException e) {}
+        try {
+            new AreaReference(refDC2D);
+            fail();
+        } catch(IllegalArgumentException e) {}
+        try {
+            new AreaReference(refDC3D);
+            fail();
+        } catch(IllegalArgumentException e) {}
+
+        // Test that we split as expected
+        AreaReference[] refs;
+
+        refs = AreaReference.generateContiguous(refSimple);
+        assertEquals(1, refs.length);
+        assertEquals(1, refs[0].getDim());
+        assertEquals("$C$10", refs[0].toString());
+
+        refs = AreaReference.generateContiguous(ref2D);
+        assertEquals(1, refs.length);
+        assertEquals(2, refs[0].getDim());
+        assertEquals("$C$10:$D$11", refs[0].toString());
+
+        refs = AreaReference.generateContiguous(refDCSimple);
+        assertEquals(3, refs.length);
+        assertEquals(1, refs[0].getDim());
+        assertEquals(1, refs[1].getDim());
+        assertEquals(1, refs[2].getDim());
+        assertEquals("$C$10", refs[0].toString());
+        assertEquals("$D$12", refs[1].toString());
+        assertEquals("$E$14", refs[2].toString());
+
+        refs = AreaReference.generateContiguous(refDC2D);
+        assertEquals(3, refs.length);
+        assertEquals(2, refs[0].getDim());
+        assertEquals(1, refs[1].getDim());
+        assertEquals(2, refs[2].getDim());
+        assertEquals("$C$10:$C$11", refs[0].toString());
+        assertEquals("$D$12", refs[1].toString());
+        assertEquals("$E$14:$E$20", refs[2].toString());
+
+        refs = AreaReference.generateContiguous(refDC3D);
+        assertEquals(2, refs.length);
+        assertEquals(2, refs[0].getDim());
+        assertEquals(2, refs[1].getDim());
+        assertEquals("$C$10:$C$14", refs[0].toString());
+        assertEquals("$D$10:$D$12", refs[1].toString());
+        assertEquals("Tabelle1", refs[0].getCells()[0].getSheetName());
+        assertEquals("Tabelle1", refs[0].getCells()[1].getSheetName());
+        assertEquals("Tabelle1", refs[1].getCells()[0].getSheetName());
+        assertEquals("Tabelle1", refs[1].getCells()[1].getSheetName());
+    }
+
+    public void testDiscontinousReference() throws Exception {
+        String filename = System.getProperty( "HSSF.testdata.path" );
+        filename = filename + "/44167.xls";
+        FileInputStream fin = new FileInputStream( filename );
+        HSSFWB wb = new HSSFWB( fin );
+        Workbook workbook = wb.getWorkbook();
+        fin.close();
+
+        assertEquals(1, wb.getNumberOfNames());
+        String sheetName = "Tabelle1";
+        String rawRefA = "$C$10:$C$14";
+        String rawRefB = "$C$16:$C$18";
+        String refA = sheetName + "!" + rawRefA;
+        String refB = sheetName + "!" + rawRefB;
+        String ref = refA + "," + refB;
+
+        // Check the low level record
+        NameRecord nr = workbook.getNameRecord(0);
+        assertNotNull(nr);
+        assertEquals("test", nr.getNameText());
+
+        List def =nr.getNameDefinition();
+        assertEquals(4, def.size());
+
+        MemFuncPtg ptgA = (MemFuncPtg)def.get(0);
+        Area3DPtg ptgB = (Area3DPtg)def.get(1);
+        Area3DPtg ptgC = (Area3DPtg)def.get(2);
+        UnionPtg ptgD = (UnionPtg)def.get(3);
+        assertEquals("", ptgA.toFormulaString(workbook));
+        assertEquals(refA, ptgB.toFormulaString(workbook));
+        assertEquals(refB, ptgC.toFormulaString(workbook));
+        assertEquals(",", ptgD.toFormulaString(workbook));
+
+        assertEquals(ref, nr.getAreaReference(workbook));
+
+        // Check the high level definition
+        int idx = wb.getNameIndex("test");
+        assertEquals(0, idx);
+        HSSFName aNamedCell = wb.getNameAt(idx);
+
+        // Should have 2 references
+        assertEquals(ref, aNamedCell.getReference());
+
+        // Check the parsing of the reference into cells
+        assertFalse(AreaReference.isContiguous(aNamedCell.getReference()));
+        AreaReference[] arefs = AreaReference.generateContiguous(aNamedCell.getReference());
+        assertEquals(2, arefs.length);
+        assertEquals(rawRefA, arefs[0].toString());
+        assertEquals(rawRefB, arefs[1].toString());
+
+        for(int i=0; i<arefs.length; i++) {
+            CellReference[] crefs = arefs[i].getCells();
+            for (int j=0; j<crefs.length; j++) {
+                // Check it turns into real stuff
+                HSSFSheet s = wb.getSheet(crefs[j].getSheetName());
+                HSSFRow r = s.getRow(crefs[j].getRow());
+                HSSFCell c = r.getCell(crefs[j].getCol());
+            }
+        }
+    }
     
-	public static void main(java.lang.String[] args) {        
+    public static void main(java.lang.String[] args) {
 		junit.textui.TestRunner.run(TestAreaReference.class);
 	}