Fix for bug 45046 - allowed DEFINEDNAME records without EXTERNALBOOK records

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

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="add">45046 - allowed EXTERNALBOOK(0x01AE) to be optional in the LinkTable</action>
            <action dev="POI-DEVELOPERS" type="add">45066 - fixed sheet encoding size mismatch problems</action>
            <action dev="POI-DEVELOPERS" type="add">45003 - Support embeded HDGF visio documents</action>
            <action dev="POI-DEVELOPERS" type="fix">45001 - Partial fix for HWPF Range.insertBefore() and Range.delete() with unicode characters</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="add">45046 - allowed EXTERNALBOOK(0x01AE) to be optional in the LinkTable</action>
            <action dev="POI-DEVELOPERS" type="add">45066 - fixed sheet encoding size mismatch problems</action>
            <action dev="POI-DEVELOPERS" type="add">45003 - Support embeded HDGF visio documents</action>
            <action dev="POI-DEVELOPERS" type="fix">45001 - Partial fix for HWPF Range.insertBefore() and Range.delete() with unicode characters</action>

src/java/org/apache/poi/hssf/model/LinkTable.java

@@ -41,7 +41,7 @@ import org.apache.poi.hssf.record.SupBookRecord;
  *
  *  In BIFF8 the Link Table consists of
  *  <ul>
- *  <li>one or more EXTERNALBOOK Blocks<p/>
+ *  <li>zero or more EXTERNALBOOK Blocks<p/>
  *  	each consisting of
  *  	<ul>
  *  	<li>exactly one EXTERNALBOOK (0x01AE) record</li>
@@ -55,7 +55,7 @@ import org.apache.poi.hssf.record.SupBookRecord;
  *  	</li>
  *  	</ul>
  *  </li>
- *  <li>exactly one EXTERNSHEET (0x0017) record</li>
+ *  <li>zero or one EXTERNSHEET (0x0017) record</li>
  *  <li>zero or more DEFINEDNAME (0x0018) records</li>
  *  </ul>
  *
@@ -63,6 +63,7 @@ import org.apache.poi.hssf.record.SupBookRecord;
  * @author Josh Micich
  */
 final class LinkTable {
+	// TODO make this class into a record aggregate
 
 	private static final class CRNBlock {
 
@@ -79,8 +80,8 @@ final class LinkTable {
 			_crns = crns;
 		}
 		public CRNRecord[] getCrns() {
-            return (CRNRecord[]) _crns.clone();
-        }
+			return (CRNRecord[]) _crns.clone();
+		}
 	}
 
 	private static final class ExternalBookBlock {
@@ -136,16 +137,19 @@ final class LinkTable {
 		while(rs.peekNextClass() == SupBookRecord.class) {
 		   temp.add(new ExternalBookBlock(rs));
 		}
-		if(temp.size() < 1) {
-			throw new RuntimeException("Need at least one EXTERNALBOOK blocks");
-		}
+		
 		_externalBookBlocks = new ExternalBookBlock[temp.size()];
 		temp.toArray(_externalBookBlocks);
 		temp.clear();
-
-		// If link table is present, there is always 1 of ExternSheetRecord
-		Record next = rs.getNext();
-		_externSheetRecord = (ExternSheetRecord)next;
+		
+		if (_externalBookBlocks.length > 0) {
+			// If any ExternalBookBlock present, there is always 1 of ExternSheetRecord
+			Record next = rs.getNext();
+			_externSheetRecord = (ExternSheetRecord) next;
+		} else {
+			_externSheetRecord = null;
+		}
+		
 		_definedNames = new ArrayList();
 		// collect zero or more DEFINEDNAMEs id=0x18
 		while(rs.peekNextClass() == NameRecord.class) {
@@ -222,7 +226,7 @@ final class LinkTable {
 	public void addName(NameRecord name) {
 		_definedNames.add(name);
 
-	   // TODO - this is messy
+		// TODO - this is messy
 		// Not the most efficient way but the other way was causing too many bugs
 		int idx = findFirstRecordLocBySid(ExternSheetRecord.sid);
 		if (idx == -1) idx = findFirstRecordLocBySid(SupBookRecord.sid);
@@ -242,8 +246,8 @@ final class LinkTable {
 
 	public int getSheetIndexFromExternSheetIndex(int externSheetNumber) {
 		if (externSheetNumber >= _externSheetRecord.getNumOfREFStructures()) {
-            return -1;
-        }
+			return -1;
+		}
 		return _externSheetRecord.getREFRecordAt(externSheetNumber).getIndexToFirstSupBook();
 	}
 
@@ -265,7 +269,7 @@ final class LinkTable {
 			ExternSheetSubRecord esr = _externSheetRecord.getREFRecordAt(i);
 
 			if (esr.getIndexToFirstSupBook() ==  sheetNumber
-			        && esr.getIndexToLastSupBook() == sheetNumber){
+					&& esr.getIndexToLastSupBook() == sheetNumber){
 				return i;
 			}
 		}

src/java/org/apache/poi/hssf/model/Workbook.java

@@ -191,12 +191,11 @@ public class Workbook implements Model
                 case ExternSheetRecord.sid :
                     throw new RuntimeException("Extern sheet is part of LinkTable");
                 case NameRecord.sid :
-                    throw new RuntimeException("DEFINEDNAME is part of LinkTable");
                 case SupBookRecord.sid :
+                    // LinkTable can start with either of these
                     if (log.check( POILogger.DEBUG ))
                         log.log(DEBUG, "found SupBook record at " + k);
                     retval.linkTable = new LinkTable(recs, k, retval.records);
-                    //                    retval.records.supbookpos = k;
                     k+=retval.linkTable.getRecordCount() - 1;
                     continue;
                 case FormatRecord.sid :

src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java

@@ -28,7 +28,7 @@ import junit.framework.TestSuite;
 public class AllUserModelTests {
 	
 	public static Test suite() {
-		TestSuite result = new TestSuite("Tests for org.apache.poi.hssf.usermodel");
+		TestSuite result = new TestSuite(AllUserModelTests.class.getName());
 		
 		result.addTestSuite(TestBugs.class);
 		result.addTestSuite(TestCellStyle.class);
@@ -58,6 +58,7 @@ public class AllUserModelTests {
 		result.addTestSuite(TestHSSFSheetSetOrder.class);
 		result.addTestSuite(TestHSSFTextbox.class);
 		result.addTestSuite(TestHSSFWorkbook.class);
+		result.addTestSuite(TestLinkTable.class);
 		result.addTestSuite(TestNamedRange.class);
 		result.addTestSuite(TestOLE2Embeding.class);
 		result.addTestSuite(TestPOIFSProperties.class);

src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java

@@ -0,0 +1,44 @@
+package org.apache.poi.hssf.usermodel;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.HSSFTestDataSamples;
+/**
+ * Tests for LinkTable
+ * 
+ * @author Josh Micich
+ */
+public final class TestLinkTable extends TestCase {
+
+	/**
+	 * The example file attached to bugzilla 45046 is a clear example of Name records being present
+	 * without an External Book (SupBook) record.  Excel has no trouble reading this file.<br/>
+	 * TODO get OOO documentation updated to reflect this (that EXTERNALBOOK is optional). 
+	 * 
+	 * It's not clear what exact steps need to be taken in Excel to create such a workbook 
+	 */
+	public void testLinkTableWithoutExternalBookRecord_bug45046() {
+		HSSFWorkbook wb;
+
+		try {
+			wb = HSSFTestDataSamples.openSampleWorkbook("ex45046-21984.xls");
+		} catch (RuntimeException e) {
+			if ("DEFINEDNAME is part of LinkTable".equals(e.getMessage())) {
+				throw new AssertionFailedError("Identified bug 45046 b");
+			}
+			throw e;
+		}
+		// some other sanity checks
+		assertEquals(3, wb.getNumberOfSheets());
+		String formula = wb.getSheetAt(0).getRow(4).getCell(13).getCellFormula();
+		
+		if ("ipcSummenproduktIntern($P5,N$6,$A$9,N$5)".equals(formula)) {
+			// The reported symptom of this bugzilla is an earlier bug (already fixed) 
+			throw new AssertionFailedError("Identified bug 41726"); 
+			// This is observable in version 3.0
+		}
+		
+		assertEquals("ipcSummenproduktIntern($C5,N$2,$A$9,N$1)", formula);
+	}
+}