poi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r780774 - in /poi/trunk/src: documentation/content/xdocs/ java/org/apache/poi/hssf/model/ java/org/apache/poi/hssf/record/aggregates/ testcases/org/apache/poi/hssf/record/aggregates/
Date Mon, 01 Jun 2009 18:41:01 GMT
Author: josh
Date: Mon Jun  1 18:41:01 2009
New Revision: 780774

URL: http://svn.apache.org/viewvc?rev=780774&view=rev
Log:
Bugzilla 47199 - Fixed PageSettingsBlock/Sheet to tolerate margin records after other non-PSB
records

Modified:
    poi/trunk/src/documentation/content/xdocs/status.xml
    poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java
    poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java
    poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java

Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Mon Jun  1 18:41:01 2009
@@ -35,6 +35,7 @@
         <release version="3.5-beta7" date="2009-??-??">
         </release>
         <release version="3.5-beta6" date="2009-06-11">
+           <action dev="POI-DEVELOPERS" type="fix">47199 - Fixed PageSettingsBlock/Sheet
to tolerate margin records after other non-PSB records</action>
            <action dev="POI-DEVELOPERS" type="fix">47069 - Fixed HSSFSheet#getFirstRowNum
and HSSFSheet#getLastRowNum to return correct values after removal of all rows</action>
            <action dev="POI-DEVELOPERS" type="fix">47278 - Fixed XSSFCell to avoid
generating xsi:nil entries in shared string table</action>
            <action dev="POI-DEVELOPERS" type="fix">47206 - Fixed XSSFCell to properly
read inline strings</action>

Modified: poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java Mon Jun  1 18:41:01 2009
@@ -57,7 +57,6 @@
 import org.apache.poi.hssf.record.ScenarioProtectRecord;
 import org.apache.poi.hssf.record.SelectionRecord;
 import org.apache.poi.hssf.record.UncalcedRecord;
-import org.apache.poi.hssf.record.UnknownRecord;
 import org.apache.poi.hssf.record.WSBoolRecord;
 import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.record.aggregates.ChartSubstreamRecordAggregate;
@@ -207,7 +206,7 @@
                 records.add(rra); //only add the aggregate once
                 continue;
             }
-            
+
             if (CustomViewSettingsRecordAggregate.isBeginRecord(recSid)) {
                 // This happens three times in test sample file "29982.xls"
                 // Also several times in bugzilla samples 46840-23373 and 46840-23374
@@ -217,28 +216,13 @@
 
             if (PageSettingsBlock.isComponentRecord(recSid)) {
                 if (_psBlock == null) {
-                    // typical case - just one PSB (so far)
+                    // first PSB record encountered - read all of them:
                     _psBlock = new PageSettingsBlock(rs);
                     records.add(_psBlock);
-                    continue;
-                }
-                if (recSid == UnknownRecord.HEADER_FOOTER_089C) {
-                    // test samples: SharedFormulaTest.xls, ex44921-21902.xls, ex42570-20305.xls
-                    _psBlock.addLateHeaderFooter(rs.getNext());
-                    continue;
-                }
-                // Some apps write PLS, WSBOOL, <psb> but PLS is part of <psb>
-                // This happens in the test sample file "NoGutsRecords.xls" and "WORKBOOK_in_capitals.xls"
-                // In this case the first PSB is two records back
-                int prevPsbIx = records.size()-2;
-                if (_psBlock != records.get(prevPsbIx) || !(records.get(prevPsbIx+1) instanceof
WSBoolRecord)) {
-                    // not quite the expected situation
-                    throw new RuntimeException("two Page Settings Blocks found in the same
sheet");
+                } else {
+                    // one or more PSB records found after some intervening non-PSB records
+                    _psBlock.addLateRecords(rs);
                 }
-                records.remove(prevPsbIx); // WSBOOL will drop down one position.
-                PageSettingsBlock latePsb = new PageSettingsBlock(rs);
-                _psBlock = mergePSBs(_psBlock, latePsb);
-                records.add(_psBlock);
                 continue;
             }
 
@@ -247,7 +231,7 @@
                 _mergedCellsTable.read(rs);
                 continue;
             }
-            
+
             if (recSid == BOFRecord.sid) {
                 ChartSubstreamRecordAggregate chartAgg = new ChartSubstreamRecordAggregate(rs);
                 if (false) {
@@ -372,34 +356,7 @@
                 recs.add(r);
             }});
     }
-    /**
-     * Hack to recover from the situation where the page settings block has been split by
-     * an intervening {@link WSBoolRecord}
-     */
-    private static PageSettingsBlock mergePSBs(PageSettingsBlock a, PageSettingsBlock b)
{
-        List<Record> temp = new ArrayList<Record>();
-        RecordTransferrer rt = new RecordTransferrer(temp);
-        a.visitContainedRecords(rt);
-        b.visitContainedRecords(rt);
-        RecordStream rs = new RecordStream(temp, 0);
-        PageSettingsBlock result = new PageSettingsBlock(rs);
-        if (rs.hasNext()) {
-            throw new RuntimeException("PageSettingsBlocks did not merge properly");
-        }
-        return result;
-    }
-
-    private static final class RecordTransferrer  implements RecordVisitor {
-
-        private final List<Record> _destList;
 
-        public RecordTransferrer(List<Record> destList) {
-            _destList = destList;
-        }
-        public void visitRecord(Record r) {
-            _destList.add(r);
-        }
-    }
     private static final class RecordCloner implements RecordVisitor {
 
         private final List<RecordBase> _destList;
@@ -1099,7 +1056,7 @@
      */
     public void setColumnWidth(int column, int width) {
         if(width > 255*256) throw new IllegalArgumentException("The maximum column width
for an individual cell is 255 characters.");
-        
+
         setColumn(column, null, new Integer(width), null, null, null);
     }
 

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java Mon Jun
 1 18:41:01 2009
@@ -125,36 +125,47 @@
 	private boolean readARecord(RecordStream rs) {
 		switch (rs.peekNextSid()) {
 			case HorizontalPageBreakRecord.sid:
+				checkNotPresent(_rowBreaksRecord);
 				_rowBreaksRecord = (PageBreakRecord) rs.getNext();
 				break;
 			case VerticalPageBreakRecord.sid:
+				checkNotPresent(_columnBreaksRecord);
 				_columnBreaksRecord = (PageBreakRecord) rs.getNext();
 				break;
 			case HeaderRecord.sid:
+				checkNotPresent(_header);
 				_header = (HeaderRecord) rs.getNext();
 				break;
 			case FooterRecord.sid:
+				checkNotPresent(_footer);
 				_footer = (FooterRecord) rs.getNext();
 				break;
 			case HCenterRecord.sid:
+				checkNotPresent(_hCenter);
 				_hCenter = (HCenterRecord) rs.getNext();
 				break;
 			case VCenterRecord.sid:
+				checkNotPresent(_vCenter);
 				_vCenter = (VCenterRecord) rs.getNext();
 				break;
 			case LeftMarginRecord.sid:
+				checkNotPresent(_leftMargin);
 				_leftMargin = (LeftMarginRecord) rs.getNext();
 				break;
 			case RightMarginRecord.sid:
+				checkNotPresent(_rightMargin);
 				_rightMargin = (RightMarginRecord) rs.getNext();
 				break;
 			case TopMarginRecord.sid:
+				checkNotPresent(_topMargin);
 				_topMargin = (TopMarginRecord) rs.getNext();
 				break;
 			case BottomMarginRecord.sid:
+				checkNotPresent(_bottomMargin);
 				_bottomMargin = (BottomMarginRecord) rs.getNext();
 				break;
 			case UnknownRecord.PLS_004D:
+				checkNotPresent(_pls);
 				_pls = rs.getNext();
 				while (rs.peekNextSid()==ContinueRecord.sid) {
 					if (_plsContinues==null) {
@@ -164,15 +175,19 @@
 				}
 				break;
 			case PrintSetupRecord.sid:
+				checkNotPresent(_printSetup);
 				_printSetup = (PrintSetupRecord)rs.getNext();
 				break;
 			case UnknownRecord.BITMAP_00E9:
+				checkNotPresent(_bitmap);
 				_bitmap = rs.getNext();
 				break;
 			case UnknownRecord.PRINTSIZE_0033:
+				checkNotPresent(_printSize);
 				_printSize = rs.getNext();
 				break;
 			case UnknownRecord.HEADER_FOOTER_089C:
+				checkNotPresent(_headerFooter);
 				_headerFooter = rs.getNext();
 				break;
 			default:
@@ -182,6 +197,13 @@
 		return true;
 	}
 
+	private void checkNotPresent(Record rec) {
+		if (rec != null) {
+			throw new RecordFormatException("Duplicate PageSettingsBlock record (sid=0x"
+					+ Integer.toHexString(rec.getSid()) + ")");
+		}
+	}
+
 	private PageBreakRecord getRowBreaksRecord() {
 		if (_rowBreaksRecord == null) {
 			_rowBreaksRecord = new HorizontalPageBreakRecord();
@@ -551,4 +573,40 @@
 		}
 		_headerFooter = rec;
 	}
+
+	/**
+	 * This method reads PageSettingsBlock records from the supplied RecordStream until the
first
+	 * non-PageSettingsBlock record is encountered.  As each record is read, it is incorporated
+	 * into this PageSettingsBlock.
+	 * <p/>
+	 * The latest Excel version seems to write the PageSettingsBlock uninterrupted. However
there
+	 * are several examples (that Excel reads OK) where these records are not written together:
+	 * <ul>
+	 * <li><b>HEADER_FOOTER(0x089C) after WINDOW2</b> - This record is new
in 2007.  Some apps
+	 * seem to have scattered this record long after the PageSettingsBlock where it belongs
+	 * test samples: SharedFormulaTest.xls, ex44921-21902.xls, ex42570-20305.xls</li>
+	 * <li><b>PLS, WSBOOL, PageSettingsBlock</b> - WSBOOL is not a PSB record.

+	 * This happens in the test sample file "NoGutsRecords.xls" and "WORKBOOK_in_capitals.xls"</li>
+	 * <li><b>Margins after DIMENSION</b> - All of PSB should be before DIMENSION.
(Bug-47199)</li>
+	 * </ul>
+	 * These were probably written by other applications (or earlier versions of Excel). It
was 
+	 * decided to not write specific code for detecting each of these cases.  POI now tolerates
+	 * PageSettingsBlock records scattered all over the sheet record stream, and in any order,
but
+	 * does not allow duplicates of any of those records.
+	 * 
+	 * <p/>
+	 * <b>Note</b> - when POI writes out this PageSettingsBlock, the records will
always be written
+	 * in one consolidated block (in the standard ordering) regardless of how scattered the
records
+	 * were when they were originally read. 
+	 * 
+	 * @throws  RecordFormatException if any PSB record encountered has the same type (sid)
as 
+	 * a record that is already part of this PageSettingsBlock
+	 */
+	public void addLateRecords(RecordStream rs) {
+		while(true) {
+			if (!readARecord(rs)) {
+				break;
+			}
+		}
+	}
 }

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java?rev=780774&r1=780773&r2=780774&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java
(original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestPageSettingsBlock.java
Mon Jun  1 18:41:01 2009
@@ -26,6 +26,7 @@
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.model.Sheet;
 import org.apache.poi.hssf.record.BOFRecord;
+import org.apache.poi.hssf.record.BottomMarginRecord;
 import org.apache.poi.hssf.record.DimensionsRecord;
 import org.apache.poi.hssf.record.EOFRecord;
 import org.apache.poi.hssf.record.FooterRecord;
@@ -33,6 +34,7 @@
 import org.apache.poi.hssf.record.IndexRecord;
 import org.apache.poi.hssf.record.NumberRecord;
 import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.UnknownRecord;
 import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.usermodel.HSSFPrintSetup;
@@ -135,7 +137,7 @@
 		Sheet sheet = Sheet.createSheet(rs);
 
 		RecordCollector rv = new RecordCollector();
-		sheet.visitContainedRecords(rv, rowIx);
+		sheet.visitContainedRecords(rv, 0);
 		Record[] outRecs = rv.getRecords();
 		if (outRecs[4] == EOFRecord.instance) {
 			throw new AssertionFailedError("Identified bug 46953 - EOF incorrectly appended to PSB");
@@ -151,6 +153,85 @@
 		assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
 		assertEquals(EOFRecord.instance, outRecs[7]);
 	}
+	/**
+	 * Bug 47199 was due to the margin records being located well after the initial PSB records.
+	 * The example file supplied (attachment 23710) had three non-PSB record types 
+	 * between the PRINTSETUP record and first MARGIN record:
+	 * <ul>
+	 * <li>PRINTSETUP(0x00A1)</li>
+	 * <li>DEFAULTCOLWIDTH(0x0055)</li>
+	 * <li>COLINFO(0x007D)</li>
+	 * <li>DIMENSIONS(0x0200)</li>
+	 * <li>BottomMargin(0x0029)</li>
+	 * </ul>
+	 */
+	public void testLateMargins_bug47199() {
+
+		Record[] recs = {
+				BOFRecord.createSheetBOF(),
+				new HeaderRecord("&LSales Figures"),
+				new FooterRecord("&LJanuary"),
+				new DimensionsRecord(),
+				createBottomMargin(0.787F),
+				new WindowTwoRecord(),
+				EOFRecord.instance,
+		};
+		RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+
+		Sheet sheet;
+		try {
+			sheet = Sheet.createSheet(rs);
+		} catch (RuntimeException e) {
+			if (e.getMessage().equals("two Page Settings Blocks found in the same sheet")) {
+				throw new AssertionFailedError("Identified bug 47199a - failed to process late margings
records");
+			}
+			throw e;
+		}
+
+		RecordCollector rv = new RecordCollector();
+		sheet.visitContainedRecords(rv, 0);
+		Record[] outRecs = rv.getRecords();
+		assertEquals(recs.length+1, outRecs.length); // +1 for index record
+
+		assertEquals(BOFRecord.class, outRecs[0].getClass());
+		assertEquals(IndexRecord.class, outRecs[1].getClass());
+		assertEquals(HeaderRecord.class, outRecs[2].getClass());
+		assertEquals(FooterRecord.class, outRecs[3].getClass());
+		assertEquals(DimensionsRecord.class, outRecs[5].getClass());
+		assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
+		assertEquals(EOFRecord.instance, outRecs[7]);
+	}
+
+	private Record createBottomMargin(float value) {
+		BottomMarginRecord result = new BottomMarginRecord();
+		result.setMargin(value);
+		return result;
+	}
+
+	/**
+	 * The PageSettingsBlock should not allow multiple copies of the same record.  This extra
assertion
+	 * was added while fixing bug 47199.  All existing POI test samples comply with this requirement.
+	 */
+	public void testDuplicatePSBRecord_bug47199() {
+
+		// Hypothetical setup of PSB records which should cause POI to crash
+		Record[] recs = {
+				new HeaderRecord("&LSales Figures"),
+				new HeaderRecord("&LInventory"),
+		};
+		RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+
+		try {
+			new PageSettingsBlock(rs);
+			throw new AssertionFailedError("Identified bug 47199b - duplicate PSB records should not
be allowed");
+		} catch (RecordFormatException e) {
+			if (e.getMessage().equals("Duplicate PageSettingsBlock record (sid=0x14)")) {
+				// expected during successful test
+			} else {
+				throw new AssertionFailedError("Expected RecordFormatException due to duplicate PSB record");
+			}
+		}
+	}
 
 	private static UnknownRecord ur(int sid, String hexData) {
 		return new UnknownRecord(sid, HexRead.readFromString(hexData));



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org


Mime
View raw message