Avoid spurious missing lines with the MissingRecordAware event code, and odd files that contain RowRecords in the middle of the cell Records.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@672562 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.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">Avoid spurious missing lines with the MissingRecordAware event code, and odd files that contain RowRecords in the middle of the cell Records.</action>
            <action dev="POI-DEVELOPERS" type="add">Support for parsing formulas during EventUserModel processing, via the new EventWorkbookBuilder</action>
         </release>
         <release version="3.1-final" date="2008-06-29">

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">Avoid spurious missing lines with the MissingRecordAware event code, and odd files that contain RowRecords in the middle of the cell Records.</action>
            <action dev="POI-DEVELOPERS" type="add">Support for parsing formulas during EventUserModel processing, via the new EventWorkbookBuilder</action>
         </release>
         <release version="3.1-final" date="2008-06-29">

src/java/org/apache/poi/hssf/eventusermodel/MissingRecordAwareHSSFListener.java

@@ -46,8 +46,14 @@ import org.apache.poi.hssf.record.RowRecord;
  */
 public class MissingRecordAwareHSSFListener implements HSSFListener {
 	private HSSFListener childListener;
-	private int lastSeenRow = -1;
+	
-	private int lastSeenColumn = -1;
+	// Need to have different counters for cell rows and
+	//  row rows, as you sometimes get a RowRecord in the
+	//  middle of some cells, and that'd break everything
+	private int lastRowRow = -1;
+	
+	private int lastCellRow = -1;
+	private int lastCellColumn = -1;
 	
 	/**
 	 * Constructs a new MissingRecordAwareHSSFListener, which
@@ -71,14 +77,16 @@ public class MissingRecordAwareHSSFListener implements HSSFListener {
                 if (bof.getType() == bof.TYPE_WORKBOOK)
                 {
                 	// Reset the row and column counts - new workbook
-                	lastSeenRow = -1;
+                	lastRowRow = -1;
-                	lastSeenColumn = -1;
+                	lastCellRow = -1;
+                	lastCellColumn = -1;
                     //System.out.println("Encountered workbook");
                 } else if (bof.getType() == bof.TYPE_WORKSHEET)
                 {
                 	// Reset the row and column counts - new sheet
-                	lastSeenRow = -1;
+                	lastRowRow = -1;
-                	lastSeenColumn = -1;
+                	lastCellRow = -1;
+                	lastCellColumn = -1;
                     //System.out.println("Encountered sheet reference");
                 }
                 break;
@@ -92,15 +100,15 @@ public class MissingRecordAwareHSSFListener implements HSSFListener {
                 //        + rowrec.getFirstCol() + " last column at " + rowrec.getLastCol());
                 
                 // If there's a jump in rows, fire off missing row records
-                if(lastSeenRow+1 < rowrec.getRowNumber()) {
+                if(lastRowRow+1 < rowrec.getRowNumber()) {
-                	for(int i=(lastSeenRow+1); i<rowrec.getRowNumber(); i++) {
+                	for(int i=(lastRowRow+1); i<rowrec.getRowNumber(); i++) {
                 		MissingRowDummyRecord dr = new MissingRowDummyRecord(i);
                 		childListener.processRecord(dr);
                 	}
                 }
                 
                 // Record this as the last row we saw
-                lastSeenRow = rowrec.getRowNumber();
+                lastRowRow = rowrec.getRowNumber();
                 break;
                 
                 
@@ -157,45 +165,49 @@ public class MissingRecordAwareHSSFListener implements HSSFListener {
             	break;
         }
 		
-		// Do we need to fire dummy end-of-row records?
+		// If we're on cells, and this cell isn't in the same
-		if(thisRow != lastSeenRow) {
+		//  row as the last one, then fire the 
-			for(int i=lastSeenRow; i<thisRow; i++) {
+		//  dummy end-of-row records?
+		if(thisRow != lastCellRow && lastCellRow > -1) {
+			for(int i=lastCellRow; i<thisRow; i++) {
 				int cols = -1;
-				if(i == lastSeenRow) {
+				if(i == lastCellRow) {
-					cols = lastSeenColumn;
+					cols = lastCellColumn;
 				}
 				LastCellOfRowDummyRecord r = new LastCellOfRowDummyRecord(i, cols);
 				childListener.processRecord(r);
 			}
 		}
-		// If we've finished with the columns, then fire the final
+		
-		//  dummy end-of-row record
+		// If we've just finished with the cells, then fire the
-		if(lastSeenRow != -1 && lastSeenColumn != -1 && thisRow == -1) {
+		//  final dummy end-of-row record
-			LastCellOfRowDummyRecord r = new LastCellOfRowDummyRecord(lastSeenRow, lastSeenColumn);
+		if(lastCellRow != -1 && lastCellColumn != -1 && thisRow == -1) {
+			LastCellOfRowDummyRecord r = new LastCellOfRowDummyRecord(lastCellRow, lastCellColumn);
 			childListener.processRecord(r);
 			
-			lastSeenRow = -1;
+			lastCellRow = -1;
-			lastSeenColumn = -1;
+			lastCellColumn = -1;
 		}
 		
 		// If we've moved onto a new row, the ensure we re-set
 		//  the column counter
-		if(thisRow != lastSeenRow) {
+		if(thisRow != lastCellRow) {
-			lastSeenColumn = -1;
+			lastCellColumn = -1;
 		}
 		
-		// Do we need to fire dummy cell records?
+		// If there's a gap in the cells, then fire
-		if(lastSeenColumn != (thisColumn-1)) {
+		//  the dummy cell records?
-			for(int i=lastSeenColumn+1; i<thisColumn; i++) {
+		if(lastCellColumn != (thisColumn-1)) {
+			for(int i=lastCellColumn+1; i<thisColumn; i++) {
 				MissingCellDummyRecord r = new MissingCellDummyRecord(thisRow, i);
 				childListener.processRecord(r);
 			}
 		}
 		
-		// Update cell and row counts if doing cells
+		// Update cell and row counts as needed
 		if(thisColumn != -1) {
-			lastSeenRow = thisRow;
+			lastCellColumn = thisColumn;
-			lastSeenColumn = thisColumn;
+			lastCellRow = thisRow;
 		}
 		
 		childListener.processRecord(record);

src/testcases/org/apache/poi/hssf/eventusermodel/TestMissingRecordAwareHSSFListener.java

@@ -16,8 +16,6 @@
 ==================================================================== */
 
 package org.apache.poi.hssf.eventusermodel;
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
@@ -29,6 +27,7 @@ import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.eventusermodel.dummyrecord.LastCellOfRowDummyRecord;
 import org.apache.poi.hssf.eventusermodel.dummyrecord.MissingCellDummyRecord;
 import org.apache.poi.hssf.eventusermodel.dummyrecord.MissingRowDummyRecord;
+import org.apache.poi.hssf.record.BOFRecord;
 import org.apache.poi.hssf.record.LabelSSTRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RowRecord;
@@ -40,8 +39,7 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 	
 	private Record[] r;
 
-	public void setUp() {
+	public void openNormal() {
-
 		HSSFRequest req = new HSSFRequest();
 		MockHSSFListener mockListen = new MockHSSFListener();
 		MissingRecordAwareHSSFListener listener = new MissingRecordAwareHSSFListener(mockListen);
@@ -55,10 +53,31 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 		} catch (IOException e) {
 			throw new RuntimeException(e);
 		}
+		
 		r = mockListen.getRecords();
+		assertTrue(r.length > 100);
+	} 
+	public void openAlt() {
+		HSSFRequest req = new HSSFRequest();
+		MockHSSFListener mockListen = new MockHSSFListener();
+		MissingRecordAwareHSSFListener listener = new MissingRecordAwareHSSFListener(mockListen);
+		req.addListenerForAllRecords(listener);
+		
+		HSSFEventFactory factory = new HSSFEventFactory();
+		try {
+			InputStream is = HSSFTestDataSamples.openSampleFileStream("MRExtraLines.xls");
+			POIFSFileSystem fs = new POIFSFileSystem(is);
+			factory.processWorkbookEvents(req, fs);
+		} catch (IOException e) {
+			throw new RuntimeException(e);
+		}
+		
+		r = mockListen.getRecords();
+		assertTrue(r.length > 100);
 	} 
 	
 	public void testMissingRowRecords() throws Exception {
+		openNormal();
 		
 		// We have rows 0, 1, 2, 20 and 21
 		int row0 = -1;
@@ -108,6 +127,7 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 	}
 	
 	public void testEndOfRowRecords() throws Exception {
+		openNormal();
 		
 		// Find the cell at 0,0
 		int cell00 = -1;
@@ -194,7 +214,7 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 		assertTrue(r[cell00+57] instanceof LastCellOfRowDummyRecord);
 		
 		// Check the numbers of the last seen columns
-		LastCellOfRowDummyRecord[] lrs = new LastCellOfRowDummyRecord[23];
+		LastCellOfRowDummyRecord[] lrs = new LastCellOfRowDummyRecord[24];
 		int lrscount = 0;
 		for(int i=0; i<r.length; i++) {
 			if(r[i] instanceof LastCellOfRowDummyRecord) {
@@ -229,6 +249,7 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 	
 	
 	public void testMissingCellRecords() throws Exception {
+		openNormal();
 		
 		// Find the cell at 0,0
 		int cell00 = -1;
@@ -327,9 +348,40 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 		assertEquals(10, mc.getColumn());
 	}
 	
+	// Make sure we don't put in any extra new lines
+	//  that aren't already there
+	public void testNoExtraNewLines() throws Exception {
+		// Load a different file
+		openAlt();
+		for(int i=0; i<r.length; i++) {
+			System.err.println(r[i]);
+		}
+		
+		
+		// This file has has something in lines 1-33
+		List lcor = new ArrayList();
+		for(int i=0; i<r.length; i++) {
+			if(r[i] instanceof LastCellOfRowDummyRecord)
+				lcor.add( (LastCellOfRowDummyRecord)r[i] );
+		}
+		
+		// Check we got the 33 rows
+		assertEquals(33, lcor.size());
+		LastCellOfRowDummyRecord[] rowEnds = (LastCellOfRowDummyRecord[])
+			lcor.toArray(new LastCellOfRowDummyRecord[lcor.size()]);
+		assertEquals(33, rowEnds.length);
+		
+		// And check they have the right stuff in them,
+		//  no repeats
+		for(int i=0; i<rowEnds.length; i++) {
+			assertEquals(i, rowEnds[i].getRow());
+		}
+	}
+
 	private static final class MockHSSFListener implements HSSFListener {
 		public MockHSSFListener() {}
 		private final List _records = new ArrayList();
+		private boolean logToStdOut = false;
 
 		public void processRecord(Record record) {
 			_records.add(record);
@@ -346,9 +398,20 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase {
 				LastCellOfRowDummyRecord lc = (LastCellOfRowDummyRecord)record;
 				log("Got end-of row, row was " + lc.getRow() + ", last column was " + lc.getLastColumnNumber());
 			}
+			
+			if(record instanceof BOFRecord) {
+				BOFRecord r = (BOFRecord)record;
+				if(r.getType() == BOFRecord.TYPE_WORKSHEET) {
+					log("On new sheet");
+				}
+			}
+			if(record instanceof RowRecord) {
+				RowRecord rr = (RowRecord)record;
+				log("Starting row #" + rr.getRowNumber());
+			}
 		}
-		private static void log(String msg) {
+		private void log(String msg) {
-			if(false) { // successful tests should be quiet
+			if(logToStdOut) {
 				System.out.println(msg);
 			}
 		}