You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2009/06/24 21:45:45 UTC
svn commit: r788157 - in /poi/trunk/src: documentation/content/xdocs/
java/org/apache/poi/hssf/record/ java/org/apache/poi/hssf/record/aggregates/
testcases/org/apache/poi/hssf/record/aggregates/
Author: josh
Date: Wed Jun 24 19:45:44 2009
New Revision: 788157
URL: http://svn.apache.org/viewvc?rev=788157&view=rev
Log:
Bugzilla 47415 - Fixed PageSettingsBlock to allow multiple PLS records
Modified:
poi/trunk/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/hssf/record/UnknownRecord.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=788157&r1=788156&r2=788157&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Wed Jun 24 19:45:44 2009
@@ -33,6 +33,7 @@
<changes>
<release version="3.5-beta7" date="2009-??-??">
+ <action dev="POI-DEVELOPERS" type="fix">47415 - Fixed PageSettingsBlock to allow multiple PLS records</action>
<action dev="POI-DEVELOPERS" type="fix">47412 - Fixed concurrency issue with EscherProperties.initProps()</action>
<action dev="POI-DEVELOPERS" type="fix">47143 - Fixed OOM in HSSFWorkbook#getAllPictures when reading .xls files containing metafiles</action>
<action dev="POI-DEVELOPERS" type="add">Added implementation for ISNA()</action>
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/UnknownRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/UnknownRecord.java?rev=788157&r1=788156&r2=788157&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/UnknownRecord.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/UnknownRecord.java Wed Jun 24 19:45:44 2009
@@ -42,6 +42,9 @@
* The few POI test samples with this record have data { 0x03, 0x00 }.
*/
public static final int PRINTSIZE_0033 = 0x0033;
+ /**
+ * Environment-Specific Print Record
+ */
public static final int PLS_004D = 0x004D;
public static final int SHEETPR_0081 = 0x0081;
public static final int SORT_0090 = 0x0090;
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=788157&r1=788156&r2=788157&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 Wed Jun 24 19:45:44 2009
@@ -6,7 +6,7 @@
(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
+ 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,
@@ -19,7 +19,6 @@
import java.util.ArrayList;
import java.util.Iterator;
-import java.util.LinkedList;
import java.util.List;
import org.apache.poi.hssf.model.RecordStream;
@@ -44,14 +43,52 @@
/**
* Groups the page settings records for a worksheet.<p/>
- *
+ *
* See OOO excelfileformat.pdf sec 4.4 'Page Settings Block'
- *
+ *
* @author Josh Micich
*/
public final class PageSettingsBlock extends RecordAggregate {
- // Every one of these component records is optional
- // (The whole PageSettingsBlock may not be present)
+
+ /**
+ * PLS is potentially a <i>continued</i> record, but is currently uninterpreted by POI
+ */
+ private static final class PLSAggregate extends RecordAggregate {
+ private static final ContinueRecord[] EMPTY_CONTINUE_RECORD_ARRAY = { };
+ private final Record _pls;
+ /**
+ * holds any continue records found after the PLS record.<br/>
+ * This would not be required if PLS was properly interpreted.
+ * Currently, PLS is an {@link UnknownRecord} and does not automatically
+ * include any trailing {@link ContinueRecord}s.
+ */
+ private ContinueRecord[] _plsContinues;
+
+ public PLSAggregate(RecordStream rs) {
+ _pls = rs.getNext();
+ if (rs.peekNextSid()==ContinueRecord.sid) {
+ List<ContinueRecord> temp = new ArrayList<ContinueRecord>();
+ while (rs.peekNextSid()==ContinueRecord.sid) {
+ temp.add((ContinueRecord)rs.getNext());
+ }
+ _plsContinues = new ContinueRecord[temp.size()];
+ temp.toArray(_plsContinues);
+ } else {
+ _plsContinues = EMPTY_CONTINUE_RECORD_ARRAY;
+ }
+ }
+
+ @Override
+ public void visitContainedRecords(RecordVisitor rv) {
+ rv.visitRecord(_pls);
+ for (int i = 0; i < _plsContinues.length; i++) {
+ rv.visitRecord(_plsContinues[i]);
+ }
+ }
+ }
+
+ // Every one of these component records is optional
+ // (The whole PageSettingsBlock may not be present)
private PageBreakRecord _rowBreaksRecord;
private PageBreakRecord _columnBreaksRecord;
private HeaderRecord _header;
@@ -62,20 +99,14 @@
private RightMarginRecord _rightMargin;
private TopMarginRecord _topMargin;
private BottomMarginRecord _bottomMargin;
- private Record _pls;
- /**
- * holds any continue records found after the PLS record.<br/>
- * This would not be required if PLS was properly interpreted.
- * Currently, PLS is an {@link UnknownRecord} and does not automatically
- * include any trailing {@link ContinueRecord}s.
- */
- private List<ContinueRecord> _plsContinues;
+ private final List<PLSAggregate> _plsRecords;
private PrintSetupRecord _printSetup;
private Record _bitmap;
private Record _headerFooter;
private Record _printSize;
public PageSettingsBlock(RecordStream rs) {
+ _plsRecords = new ArrayList<PLSAggregate>();
while(true) {
if (!readARecord(rs)) {
break;
@@ -87,6 +118,7 @@
* Creates a PageSettingsBlock with default settings
*/
public PageSettingsBlock() {
+ _plsRecords = new ArrayList<PLSAggregate>();
_rowBreaksRecord = new HorizontalPageBreakRecord();
_columnBreaksRecord = new VerticalPageBreakRecord();
_header = new HeaderRecord("");
@@ -97,7 +129,7 @@
}
/**
- * @return <code>true</code> if the specified Record sid is one belonging to the
+ * @return <code>true</code> if the specified Record sid is one belonging to the
* 'Page Settings Block'.
*/
public static boolean isComponentRecord(int sid) {
@@ -115,8 +147,8 @@
case UnknownRecord.PLS_004D:
case PrintSetupRecord.sid:
case UnknownRecord.BITMAP_00E9:
- case UnknownRecord.PRINTSIZE_0033:
- case UnknownRecord.HEADER_FOOTER_089C: // extra header/footer settings supported by Excel 2007
+ case UnknownRecord.PRINTSIZE_0033:
+ case UnknownRecord.HEADER_FOOTER_089C: // extra header/footer settings supported by Excel 2007
return true;
}
return false;
@@ -165,14 +197,7 @@
_bottomMargin = (BottomMarginRecord) rs.getNext();
break;
case UnknownRecord.PLS_004D:
- checkNotPresent(_pls);
- _pls = rs.getNext();
- while (rs.peekNextSid()==ContinueRecord.sid) {
- if (_plsContinues==null) {
- _plsContinues = new LinkedList<ContinueRecord>();
- }
- _plsContinues.add((ContinueRecord)rs.getNext());
- }
+ _plsRecords.add(new PLSAggregate(rs));
break;
case PrintSetupRecord.sid:
checkNotPresent(_printSetup);
@@ -243,7 +268,7 @@
visitIfPresent(_rowBreaksRecord, rv);
visitIfPresent(_columnBreaksRecord, rv);
- // Write out empty header / footer records if these are missing
+ // Write out empty header / footer records if these are missing
if (_header == null) {
rv.visitRecord(new HeaderRecord(""));
} else {
@@ -260,11 +285,8 @@
visitIfPresent(_rightMargin, rv);
visitIfPresent(_topMargin, rv);
visitIfPresent(_bottomMargin, rv);
- visitIfPresent(_pls, rv);
- if (_plsContinues != null) {
- for (ContinueRecord cr : _plsContinues) {
- visitIfPresent(cr, rv);
- }
+ for (RecordAggregate pls : _plsRecords) {
+ pls.visitContainedRecords(rv);
}
visitIfPresent(_printSetup, rv);
visitIfPresent(_bitmap, rv);
@@ -569,11 +591,10 @@
public HCenterRecord getHCenter() {
return _hCenter;
}
-
+
/**
* HEADERFOOTER is new in 2007. Some apps seem to have scattered this record long after
* the {@link PageSettingsBlock} where it belongs.
- *
*/
public void addLateHeaderFooter(Record rec) {
if (_headerFooter != null) {
@@ -596,21 +617,21 @@
* <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.
+ * <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
+ * 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
+ * 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) {
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=788157&r1=788156&r2=788157&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 Wed Jun 24 19:45:44 2009
@@ -27,6 +27,7 @@
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.ContinueRecord;
import org.apache.poi.hssf.record.DimensionsRecord;
import org.apache.poi.hssf.record.EOFRecord;
import org.apache.poi.hssf.record.FooterRecord;
@@ -47,19 +48,19 @@
/**
* Tess for {@link PageSettingsBlock}
- *
+ *
* @author Dmitriy Kumshayev
*/
public final class TestPageSettingsBlock extends TestCase {
-
+
public void testPrintSetup_bug46548() {
-
- // PageSettingBlock in this file contains PLS (sid=x004D) record
- // followed by ContinueRecord (sid=x003C)
+
+ // PageSettingBlock in this file contains PLS (sid=x004D) record
+ // followed by ContinueRecord (sid=x003C)
HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex46548-23133.xls");
HSSFSheet sheet = wb.getSheetAt(0);
HSSFPrintSetup ps = sheet.getPrintSetup();
-
+
try {
ps.getCopies();
} catch (NullPointerException e) {
@@ -71,7 +72,6 @@
/**
* Bug 46840 occurred because POI failed to recognise HEADERFOOTER as part of the
* {@link PageSettingsBlock}.
- *
*/
public void testHeaderFooter_bug46840() {
@@ -157,7 +157,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
+ * 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>
@@ -238,7 +238,7 @@
private static UnknownRecord ur(int sid, String hexData) {
return new UnknownRecord(sid, HexRead.readFromString(hexData));
}
-
+
/**
* Excel tolerates missing header / footer records, but adds them (empty) in when re-saving.
* This is not critical functionality but it has been decided to keep POI consistent with
@@ -257,7 +257,7 @@
RecordCollector rc = new RecordCollector();
psb.visitContainedRecords(rc);
Record[] outRecs = rc.getRecords();
-
+
if (outRecs.length == 2) {
throw new AssertionFailedError("PageSettingsBlock didn't add missing header/footer records");
}
@@ -266,11 +266,53 @@
assertEquals(FooterRecord.class, outRecs[1].getClass());
assertEquals(HCenterRecord.class, outRecs[2].getClass());
assertEquals(VCenterRecord.class, outRecs[3].getClass());
-
- // make sure the added header / footer records are empty
+
+ // make sure the added header / footer records are empty
HeaderRecord hr = (HeaderRecord) outRecs[0];
assertEquals("", hr.getText());
FooterRecord fr = (FooterRecord) outRecs[1];
assertEquals("", fr.getText());
}
+
+ /**
+ * Apparently it's OK to have more than one PLS record.
+ * Attachment 23866 from bug 47415 had a PageSettingsBlock with two PLS records. This file
+ * seems to open OK in Excel(2007) but both PLS records are removed (perhaps because the
+ * specified printers were not available on the testing machine). Since the example file does
+ * not upset Excel, POI will preserve multiple PLS records.</p>
+ *
+ * As of June 2009, PLS is still uninterpreted by POI
+ */
+ public void testDuplicatePLS_bug47415() {
+ Record plsA = ur(UnknownRecord.PLS_004D, "BA AD F0 0D");
+ Record plsB = ur(UnknownRecord.PLS_004D, "DE AD BE EF");
+ Record contB1 = new ContinueRecord(HexRead.readFromString("FE ED"));
+ Record contB2 = new ContinueRecord(HexRead.readFromString("FA CE"));
+ Record[] recs = {
+ new HeaderRecord("&LSales Figures"),
+ new FooterRecord("&LInventory"),
+ new HCenterRecord(),
+ new VCenterRecord(),
+ plsA,
+ plsB, contB1, contB2, // make sure continuing PLS is still OK
+ };
+ RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
+ PageSettingsBlock psb;
+ try {
+ psb = new PageSettingsBlock(rs);
+ } catch (RecordFormatException e) {
+ if ("Duplicate PageSettingsBlock record (sid=0x4d)".equals(e.getMessage())) {
+ throw new AssertionFailedError("Identified bug 47415");
+ }
+ throw e;
+ }
+
+ // serialize the PSB to see what records come out
+ RecordCollector rc = new RecordCollector();
+ psb.visitContainedRecords(rc);
+ Record[] outRecs = rc.getRecords();
+
+ // records were assembled in standard order, so this simple check is OK
+ assertTrue(Arrays.equals(recs, outRecs));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org