Bugzilla 53010, patch from June 3: improved support for Continue records in drawing blocks

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

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

@@ -1508,31 +1508,10 @@ public final class InternalSheet {
             return loc;
         }
         List<RecordBase> records = getRecords();
-        EscherAggregate r = EscherAggregate.createAggregate( records, loc, drawingManager );
-        int startloc = loc;
-        while ( loc + 1 < records.size()
-                && records.get( loc ) instanceof DrawingRecord
-                && (records.get( loc + 1 ) instanceof ObjRecord ||
-                    records.get( loc + 1 ) instanceof TextObjectRecord) )
-        {
-            loc += 2;
-            if (records.get( loc ) instanceof NoteRecord) loc ++;
-            while ( loc + 1 < records.size()
-                    && records.get( loc ) instanceof ContinueRecord
-                    && (records.get( loc + 1 ) instanceof ObjRecord ||
-                    records.get( loc + 1 ) instanceof TextObjectRecord) )
-            {
-                loc += 2;
-                if (records.get( loc ) instanceof NoteRecord) loc ++;
-            }
-        }
 
-        int endloc = loc-1;
-        for(int i = 0; i < (endloc - startloc + 1); i++)
-            records.remove(startloc);
-        records.add(startloc, r);
+        EscherAggregate.createAggregate( records, loc, drawingManager );
 
-        return startloc;
+        return loc;
     }
 
     /**

src/java/org/apache/poi/hssf/record/DrawingRecord.java

@@ -20,22 +20,21 @@ package org.apache.poi.hssf.record;
 import org.apache.poi.util.LittleEndianOutput;
 /**
  * DrawingRecord (0x00EC)<p/>
- *
  */
 public final class DrawingRecord extends StandardRecord {
     public static final short sid = 0x00EC;
 
-	private static final byte[] EMPTY_BYTE_ARRAY = { };
+    private static final byte[] EMPTY_BYTE_ARRAY = {};
 
     private byte[] recordData;
     private byte[] contd;
 
     public DrawingRecord() {
-    	recordData = EMPTY_BYTE_ARRAY;
+        recordData = EMPTY_BYTE_ARRAY;
     }
 
     public DrawingRecord(RecordInputStream in) {
-      recordData = in.readRemainder();
+        recordData = in.readRemainder();
     }
 
     public void processContinueRecord(byte[] record) {
@@ -46,6 +45,7 @@ public final class DrawingRecord extends StandardRecord {
     public void serialize(LittleEndianOutput out) {
         out.write(recordData);
     }
+
     protected int getDataSize() {
         return recordData.length;
     }
@@ -55,12 +55,12 @@ public final class DrawingRecord extends StandardRecord {
     }
 
     public byte[] getData() {
-        if(contd != null) {
-            byte[] newBuffer = new byte[ recordData.length + contd.length ];
-            System.arraycopy( recordData, 0, newBuffer, 0, recordData.length );
-            System.arraycopy( contd, 0, newBuffer, recordData.length, contd.length);
-            return newBuffer;
-        }
+//        if (continueData.size() != 0) {
+//            byte[] newBuffer = new byte[recordData.length + continueData.size()];
+//            System.arraycopy(recordData, 0, newBuffer, 0, recordData.length);
+//            System.arraycopy(continueData.toByteArray(), 0, newBuffer, recordData.length, continueData.size());
+//            return newBuffer;
+//        }
         return recordData;
     }
 
@@ -69,21 +69,20 @@ public final class DrawingRecord extends StandardRecord {
     }
 
     public void setData(byte[] thedata) {
-    	if (thedata == null) {
-    		throw new IllegalArgumentException("data must not be null");
-    	}
+        if (thedata == null) {
+            throw new IllegalArgumentException("data must not be null");
+        }
         recordData = thedata;
     }
 
     public Object clone() {
-    	DrawingRecord rec = new DrawingRecord();
-    	
-    	rec.recordData = recordData.clone();
-    	if (contd != null) {
-	    	// TODO - this code probably never executes
-	    	rec.contd = contd.clone();
-    	}
-    	
-    	return rec;
+        DrawingRecord rec = new DrawingRecord();
+        rec.recordData = recordData.clone();
+        if (contd != null) {
+            // TODO - this code probably never executes
+            rec.contd = contd.clone();
+        }
+
+        return rec;
     }
-}
+}

src/java/org/apache/poi/hssf/record/RecordFactoryInputStream.java

@@ -338,14 +338,14 @@ public final class RecordFactoryInputStream {
 			}
 			if (_lastRecord instanceof DrawingGroupRecord) {
 				((DrawingGroupRecord) _lastRecord).processContinueRecord(contRec.getData());
-				return null;
-			}
-			if (_lastRecord instanceof DrawingRecord) {
-				((DrawingRecord) _lastRecord).processContinueRecord(contRec.getData());
-				return null;
-			}
-			if (_lastRecord instanceof UnknownRecord) {
-				//Gracefully handle records that we don't know about,
+				return null;
+			}
+			if (_lastRecord instanceof DrawingRecord) {
+//				((DrawingRecord) _lastRecord).appendContinueRecord(contRec.getData());
+				return contRec;
+			}
+			if (_lastRecord instanceof UnknownRecord) {
+				//Gracefully handle records that we don't know about,
 				//that happen to be continued
 				return record;
 			}

src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java

@@ -17,11 +17,7 @@
 package org.apache.poi.hssf.model;
 
 import junit.framework.TestCase;
-import org.apache.poi.ddf.EscherClientDataRecord;
-import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherDggRecord;
-import org.apache.poi.ddf.EscherRecord;
-import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.record.*;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
@@ -30,147 +26,199 @@ import org.apache.poi.hssf.usermodel.HSSFTestHelper;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.util.HexRead;
 
-import java.io.ByteArrayInputStream;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
+import java.io.*;
+import java.util.Arrays;
 import java.util.List;
-import java.util.Map;
 
 /**
  * @author Yegor Kozlov
  * @author Evgeniy Berlog
  */
 public class TestDrawingAggregate extends TestCase {
+    private static byte[] toByteArray(List<RecordBase> records){
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            for(RecordBase rb : records) {
+                    Record r = (Record)rb;
+                    try {
+                            out.write(r.serialize());
+                        } catch (IOException e){
+                            throw new RuntimeException(e);
+                        }
+                }
+            return out.toByteArray();
+        }
+
     /**
-     * Serialize escher aggregate, read back and assert that the drawing data is preserved.
-     *
-     * @param agg the aggregate to test
-     * @return verified aggregate (serialized and read back)
+     * test reading drawing aggregate from a test file from Bugzilla 45129
      */
-    public static EscherAggregate assertWriteAndReadBack(EscherAggregate agg) {
-        byte[] dgBytes = agg.serialize();
-
+    public void test45129() {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("45129.xls");
+        HSSFSheet sh = wb.getSheetAt(0);
 
-        List<Record> dgRecords = RecordFactory.createRecords(new ByteArrayInputStream(dgBytes));
+        InternalWorkbook iworkbook = HSSFTestHelper.getWorkbookForTest(wb);
+        InternalSheet isheet = HSSFTestHelper.getSheetForTest(sh);
 
-        DrawingManager2 drawingManager = new DrawingManager2(new EscherDggRecord());
+        List<RecordBase> records = isheet.getRecords();
 
-        // create a dummy sheet consisting of our test data
-        InternalSheet sheet = InternalSheet.createSheet();
-        List<RecordBase> records = sheet.getRecords();
-        records.clear();
-        records.addAll(dgRecords);
-        records.add(EOFRecord.instance);
+        // the sheet's drawing is not aggregated
+        assertEquals("wrong size of sheet records stream", 394, records.size());
+        // the last record before the drawing block
+        assertTrue(
+                "records.get(18) is expected to be RowRecordsAggregate but was " + records.get(18).getClass().getSimpleName(),
+                records.get(18) instanceof RowRecordsAggregate);
 
+        // records to be aggregated
+        List<RecordBase> dgRecords = records.subList(19, 389);
+        // collect drawing records into a byte buffer.
+        byte[] dgBytes = toByteArray(dgRecords);
 
-        sheet.aggregateDrawingRecords(drawingManager, false);
-        assertEquals("drawing was not fully aggregated", 2, records.size());
-        assertTrue("expected EscherAggregate", records.get(0) instanceof EscherAggregate);
-        assertTrue("expected EOFRecord", records.get(1) instanceof EOFRecord);
-        EscherAggregate agg2 = (EscherAggregate) records.get(0);
+        for (RecordBase rb : dgRecords) {
+            Record r = (Record) rb;
+            short sid = r.getSid();
+            // we expect that drawing block consists of either
+            // DrawingRecord or ContinueRecord or ObjRecord or TextObjectRecord
+            assertTrue(
+                    sid == DrawingRecord.sid ||
+                            sid == ContinueRecord.sid ||
+                            sid == ObjRecord.sid ||
+                            sid == TextObjectRecord.sid);
+        }
 
-        assertEquals(agg.getEscherRecords().size(), agg2.getEscherRecords().size());
+        // the first record after the drawing block
+        assertTrue(
+                "records.get(389) is expected to be Window2",
+                records.get(389) instanceof WindowTwoRecord);
 
-        // assert that both pre- and after- serialize aggregates have the same xml representation
-        for (int i = 0; i < agg.getEscherRecords().size(); i++) {
-            EscherRecord r1 = agg.getEscherRecords().get(i);
-            EscherRecord r2 = agg2.getEscherRecords().get(i);
+        // aggregate drawing records.
+        // The subrange [19, 388] is expected to be replaced with a EscherAggregate object
+        DrawingManager2 drawingManager = iworkbook.findDrawingGroup();
+        int loc = isheet.aggregateDrawingRecords(drawingManager, false);
+        EscherAggregate agg = (EscherAggregate) records.get(loc);
 
-            assertEquals(r1.toXml(), r2.toXml());
-        }
+        assertEquals("wrong size of the aggregated sheet records stream", 25, records.size());
+        assertTrue(
+                "records.get(18) is expected to be RowRecordsAggregate but was " + records.get(18).getClass().getSimpleName(),
+                records.get(18) instanceof RowRecordsAggregate);
+        assertTrue("records.get(19) is expected to be EscherAggregate but was " + records.get(19).getClass().getSimpleName(),
+                records.get(19) instanceof EscherAggregate);
+        assertTrue("records.get(20) is expected to be Window2 but was " + records.get(20).getClass().getSimpleName(),
+                records.get(20) instanceof WindowTwoRecord);
 
-        return agg2;
+        byte[] dgBytesAfterSave = agg.serialize();
+        assertEquals("different size of drawing data before and after save", dgBytes.length, dgBytesAfterSave.length);
+        assertTrue("drawing data brefpore and after save is different", Arrays.equals(dgBytes, dgBytesAfterSave));
     }
 
     /**
-     * assert that mapping of Obj records to escher shape containers is the same in both aggregates
+     * Try to check file with such record sequence
+     * ...
+     * DrawingRecord
+     * ContinueRecord
+     * ObjRecord | TextObjRecord
+     * ...
      */
-    public static void assertObjectMappingSame(EscherAggregate agg1, EscherAggregate agg2) {
-
-        // map EscherClientDataRecord and EscherTextboxRecord to their parents
-        Map<EscherRecord, EscherContainerRecord> map1 = new LinkedHashMap<EscherRecord, EscherContainerRecord>();
-        for (EscherRecord r : agg1.getEscherRecords()) mapShapeContainers(r, map1);
-
-        Map<EscherRecord, EscherContainerRecord> map2 = new LinkedHashMap<EscherRecord, EscherContainerRecord>();
-        for (EscherRecord r : agg2.getEscherRecords()) mapShapeContainers(r, map2);
-
-        assertEquals("aggregates have different number of shapes", map1.size(), map2.size());
-
-        // for each EscherClientDataRecord get parent SP_CONTAINER and corresponding ObjRecord
-        // verify that ObjRecord to
-        List<EscherRecord> l1 = new ArrayList<EscherRecord>(map1.keySet());
-        List<EscherRecord> l2 = new ArrayList<EscherRecord>(map2.keySet());
-        for (int i = 0; i < l1.size(); i++) {
-            EscherRecord e1 = l1.get(i);
-            EscherRecord e2 = l2.get(i);
-            ObjRecord obj1 = (ObjRecord) HSSFRecordTestHelper.getShapeToObjForTest(agg1).get(e1);
-            ObjRecord obj2 = (ObjRecord) HSSFRecordTestHelper.getShapeToObjForTest(agg2).get(e2);
-
-            CommonObjectDataSubRecord cmo1 = (CommonObjectDataSubRecord) obj1.getSubRecords().get(0);
-            CommonObjectDataSubRecord cmo2 = (CommonObjectDataSubRecord) obj2.getSubRecords().get(0);
-
-            assertEquals(cmo1.getObjectId(), cmo2.getObjectId());
-            assertEquals(obj1.toString(), obj2.toString());
-
-            // test that obj parents have the same shapeId, that is, that shape is the same
-            EscherContainerRecord p1 = map1.get(e1);
-            EscherContainerRecord p2 = map2.get(e2);
-            EscherSpRecord sp1 = (EscherSpRecord) p1.getChildById(EscherSpRecord.RECORD_ID);
-            EscherSpRecord sp2 = (EscherSpRecord) p2.getChildById(EscherSpRecord.RECORD_ID);
-            assertEquals(sp1.getShapeId(), sp2.getShapeId());
-
-            assertEquals("wrong shape2obj mapping", sp1.getShapeId() % 1024, cmo1.getObjectId());
-            assertEquals(p1.toXml(), p2.toXml());
+    public void testSerializeDrawingBigger8k() {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("DrawingContinue.xls");
+        InternalWorkbook iworkbook = HSSFTestHelper.getWorkbookForTest(wb);
+        HSSFSheet sh = wb.getSheetAt(0);
+        InternalSheet isheet = HSSFTestHelper.getSheetForTest(sh);
+
+
+        List<RecordBase> records = isheet.getRecords();
+
+        // the sheet's drawing is not aggregated
+        assertEquals("wrong size of sheet records stream", 32, records.size());
+        // the last record before the drawing block
+        assertTrue(
+                "records.get(18) is expected to be RowRecordsAggregate but was " + records.get(18).getClass().getSimpleName(),
+                records.get(18) instanceof RowRecordsAggregate);
+
+        // records to be aggregated
+        List<RecordBase> dgRecords = records.subList(19, 26);
+        for (RecordBase rb : dgRecords) {
+            Record r = (Record) rb;
+            short sid = r.getSid();
+            // we expect that drawing block consists of either
+            // DrawingRecord or ContinueRecord or ObjRecord or TextObjectRecord
+            assertTrue(
+                    sid == DrawingRecord.sid ||
+                            sid == ContinueRecord.sid ||
+                            sid == ObjRecord.sid ||
+                            sid == NoteRecord.sid ||
+                            sid == TextObjectRecord.sid);
         }
+        // collect drawing records into a byte buffer.
+        byte[] dgBytes = toByteArray(dgRecords);
+
+        // the first record after the drawing block
+        assertTrue(
+                "records.get(26) is expected to be Window2",
+                records.get(26) instanceof WindowTwoRecord);
+
+        // aggregate drawing records.
+        // The subrange [19, 38] is expected to be replaced with a EscherAggregate object
+        DrawingManager2 drawingManager = iworkbook.findDrawingGroup();
+        int loc = isheet.aggregateDrawingRecords(drawingManager, false);
+        EscherAggregate agg = (EscherAggregate) records.get(loc);
+
+        assertEquals("wrong size of the aggregated sheet records stream", 26, records.size());
+        assertTrue(
+                "records.get(18) is expected to be RowRecordsAggregate but was " + records.get(18).getClass().getSimpleName(),
+                records.get(18) instanceof RowRecordsAggregate);
+        assertTrue("records.get(19) is expected to be EscherAggregate but was " + records.get(19).getClass().getSimpleName(),
+                records.get(19) instanceof EscherAggregate);
+        assertTrue("records.get(20) is expected to be Window2 but was " + records.get(20).getClass().getSimpleName(),
+                records.get(20) instanceof WindowTwoRecord);
+
+        byte[] dgBytesAfterSave = agg.serialize();
+        assertEquals("different size of drawing data before and after save", dgBytes.length, dgBytesAfterSave.length);
+        assertTrue("drawing data before and after save is different", Arrays.equals(dgBytes, dgBytesAfterSave));
+
+
     }
 
-    /**
-     * recursively map EscherClientDataRecords  to their parent shape containers:
-     * <p/>
-     * EscherClientDataRecord1 --> EscherContainerRecord1
-     * EscherClientDataRecord2 --> EscherContainerRecord2
-     * ...
-     * <p/>
-     * TODO: YK: this method can be avoided if we have EscherRecord.getParent()
-     */
-    private static void mapShapeContainers(EscherRecord parent, Map<EscherRecord, EscherContainerRecord> map) {
-        if (parent.isContainerRecord()) {
-            if (parent.getRecordId() == EscherContainerRecord.SP_CONTAINER) {
-                // iterate over shape's children and search for EscherClientDataRecord
-                for (EscherRecord r : parent.getChildRecords()) {
-                    if (r.getRecordId() == EscherClientDataRecord.RECORD_ID) {
-                        map.put(r, (EscherContainerRecord) parent);
-                    }
-                }
-            } else {
-                for (EscherRecord ch : parent.getChildRecords()) {
-                    mapShapeContainers(ch, map);
-                }
+
+    public void testSerializeDrawingBigger8k_noAggregation() {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("DrawingContinue.xls");
+
+        InternalSheet isheet = HSSFTestHelper.getSheetForTest(wb.getSheetAt(0));
+        List<RecordBase> records = isheet.getRecords();
+
+        HSSFWorkbook wb2 = HSSFTestDataSamples.writeOutAndReadBack(wb);
+        InternalSheet isheet2 = HSSFTestHelper.getSheetForTest( wb2.getSheetAt(0));
+        List<RecordBase> records2 = isheet2.getRecords();
+
+        assertEquals(records.size(), records2.size());
+        for(int i = 0; i < records.size(); i++) {
+            RecordBase r1 = records.get(i);
+            RecordBase r2 = records2.get(i);
+            assertTrue(r1.getClass() == r2.getClass());
+            assertEquals(r1.getRecordSize(), r2.getRecordSize());
+            if(r1 instanceof Record ){
+                assertEquals(((Record)r1).getSid(), ((Record)r2).getSid());
+                assertTrue(Arrays.equals(((Record) r1).serialize(), ((Record) r2).serialize()));
             }
         }
+
     }
 
-    /**
-     * test reading drawing aggregate from a test file from Bugzilla 45129
-     */
-    public void test45129() {
-        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("45129.xls");
+    public void testSerializeDrawingWithComments() throws IOException {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("DrawingAndComments.xls");
         HSSFSheet sh = wb.getSheetAt(0);
-
         InternalWorkbook iworkbook = HSSFTestHelper.getWorkbookForTest(wb);
         InternalSheet isheet = HSSFTestHelper.getSheetForTest(sh);
 
         List<RecordBase> records = isheet.getRecords();
 
         // the sheet's drawing is not aggregated
-        assertEquals("wrong size of sheet records stream", 394, records.size());
+        assertEquals("wrong size of sheet records stream", 46, records.size());
         // the last record before the drawing block
         assertTrue(
                 "records.get(18) is expected to be RowRecordsAggregate but was " + records.get(18).getClass().getSimpleName(),
                 records.get(18) instanceof RowRecordsAggregate);
 
         // records to be aggregated
-        List<RecordBase> dgRecords = records.subList(19, 388);
+        List<RecordBase> dgRecords = records.subList(19, 39);
         for (RecordBase rb : dgRecords) {
             Record r = (Record) rb;
             short sid = r.getSid();
@@ -180,21 +228,24 @@ public class TestDrawingAggregate extends TestCase {
                     sid == DrawingRecord.sid ||
                             sid == ContinueRecord.sid ||
                             sid == ObjRecord.sid ||
+                            sid == NoteRecord.sid ||
                             sid == TextObjectRecord.sid);
         }
+        // collect drawing records into a byte buffer.
+        byte[] dgBytes = toByteArray(dgRecords);
 
         // the first record after the drawing block
         assertTrue(
-                "records.get(389) is expected to be Window2",
-                records.get(389) instanceof WindowTwoRecord);
+                "records.get(39) is expected to be Window2",
+                records.get(39) instanceof WindowTwoRecord);
 
         // aggregate drawing records.
-        // The subrange [19, 388] is expected to be replaced with a EscherAggregate object
+        // The subrange [19, 38] is expected to be replaced with a EscherAggregate object
         DrawingManager2 drawingManager = iworkbook.findDrawingGroup();
         int loc = isheet.aggregateDrawingRecords(drawingManager, false);
         EscherAggregate agg = (EscherAggregate) records.get(loc);
 
-        assertEquals("wrong size of the aggregated sheet records stream", 25, records.size());
+        assertEquals("wrong size of the aggregated sheet records stream", 27, records.size());
         assertTrue(
                 "records.get(18) is expected to be RowRecordsAggregate but was " + records.get(18).getClass().getSimpleName(),
                 records.get(18) instanceof RowRecordsAggregate);
@@ -203,11 +254,12 @@ public class TestDrawingAggregate extends TestCase {
         assertTrue("records.get(20) is expected to be Window2 but was " + records.get(20).getClass().getSimpleName(),
                 records.get(20) instanceof WindowTwoRecord);
 
-        EscherAggregate agg2 = assertWriteAndReadBack(agg);
-
-        assertObjectMappingSame(agg, agg2);
+        byte[] dgBytesAfterSave = agg.serialize();
+        assertEquals("different size of drawing data before and after save", dgBytes.length, dgBytesAfterSave.length);
+        assertTrue("drawing data before and after save is different", Arrays.equals(dgBytes, dgBytesAfterSave));
     }
 
+
     public void testFileWithPictures() {
         HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ContinueRecordProblem.xls");
         HSSFSheet sh = wb.getSheetAt(0);
@@ -225,7 +277,7 @@ public class TestDrawingAggregate extends TestCase {
                 records.get(21) instanceof RowRecordsAggregate);
 
         // records to be aggregated
-        List<RecordBase> dgRecords = records.subList(22, 299);
+        List<RecordBase> dgRecords = records.subList(22, 300);
         for (RecordBase rb : dgRecords) {
             Record r = (Record) rb;
             short sid = r.getSid();
@@ -237,6 +289,8 @@ public class TestDrawingAggregate extends TestCase {
                             sid == ObjRecord.sid ||
                             sid == TextObjectRecord.sid);
         }
+        // collect drawing records into a byte buffer.
+        byte[] dgBytes = toByteArray(dgRecords);
 
         // the first record after the drawing block
         assertTrue(
@@ -244,7 +298,7 @@ public class TestDrawingAggregate extends TestCase {
                 records.get(300) instanceof WindowTwoRecord);
 
         // aggregate drawing records.
-        // The subrange [19, 388] is expected to be replaced with a EscherAggregate object
+        // The subrange [19, 299] is expected to be replaced with a EscherAggregate object
         DrawingManager2 drawingManager = iworkbook.findDrawingGroup();
         int loc = isheet.aggregateDrawingRecords(drawingManager, false);
         EscherAggregate agg = (EscherAggregate) records.get(loc);
@@ -258,9 +312,9 @@ public class TestDrawingAggregate extends TestCase {
         assertTrue("records.get(23) is expected to be Window2 but was " + records.get(23).getClass().getSimpleName(),
                 records.get(23) instanceof WindowTwoRecord);
 
-        EscherAggregate agg2 = assertWriteAndReadBack(agg);
-
-        assertObjectMappingSame(agg, agg2);
+        byte[] dgBytesAfterSave = agg.serialize();
+        assertEquals("different size of drawing data before and after save", dgBytes.length, dgBytesAfterSave.length);
+        assertTrue("drawing data brefpore and after save is different", Arrays.equals(dgBytes, dgBytesAfterSave));
     }
 
     public void testUnhandledContinue() {
@@ -972,10 +1026,9 @@ public class TestDrawingAggregate extends TestCase {
         assertTrue("expected EOFRecord", records.get(1) instanceof EOFRecord);
         EscherAggregate agg = (EscherAggregate) records.get(0);
 
-        // serialize, read back and assert that the drawing data is preserved
-        EscherAggregate agg2 = assertWriteAndReadBack(agg);
-
-        assertObjectMappingSame(agg, agg2);
+        byte[] dgBytesAfterSave = agg.serialize();
+        assertEquals("different size of drawing data before and after save", dgBytes.length, dgBytesAfterSave.length);
+        assertTrue("drawing data before and after save is different", Arrays.equals(dgBytes, dgBytesAfterSave));
     }
 
     public void testUnhandledContinue2() {
@@ -1926,8 +1979,8 @@ public class TestDrawingAggregate extends TestCase {
 
         EscherAggregate agg = (EscherAggregate) records.get(0);
 
-        EscherAggregate agg2 = assertWriteAndReadBack(agg);
-
-        assertObjectMappingSame(agg, agg2);
+        byte[] dgBytesAfterSave = agg.serialize();
+        assertEquals("different size of drawing data before and after save", dgBytes.length, dgBytesAfterSave.length);
+        assertTrue("drawing data brefpore and after save is different", Arrays.equals(dgBytes, dgBytesAfterSave));
     }
 }

src/testcases/org/apache/poi/hssf/record/HSSFRecordTestHelper.java

@@ -1,34 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- *    contributor license agreements.  See the NOTICE file distributed with
- *    this work for additional information regarding copyright ownership.
- *    The ASF licenses this file to You under the Apache License, Version 2.0
- *    (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
- *
- *    Unless required by applicable law or agreed to in writing, software
- *    distributed under the License is distributed on an "AS IS" BASIS,
- *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- *    See the License for the specific language governing permissions and
- *    limitations under the License.
- */
-
-package org.apache.poi.hssf.record;
-
-import org.apache.poi.ddf.EscherRecord;
-
-import java.util.Map;
-
-/**
- * @author Evgeniy Berlog
- * date: 30.05.12
- */
-public class HSSFRecordTestHelper {
-
-    public static Map<EscherRecord, Record> getShapeToObjForTest(EscherAggregate agg){
-        return agg.shapeToObj;
-    }
-
-}

src/testcases/org/apache/poi/hssf/record/TestDrawingRecord.java

@@ -49,17 +49,12 @@ public final class TestDrawingRecord extends TestCase {
         out.write(cn.serialize());
 
         List<Record> rec = RecordFactory.createRecords(new ByteArrayInputStream(out.toByteArray()));
-        assertEquals(1, rec.size());
+        assertEquals(2, rec.size());
         assertTrue(rec.get(0) instanceof DrawingRecord);
+        assertTrue(rec.get(1) instanceof ContinueRecord);
 
-        //DrawingRecord.getData() should return concatenated data1 and data2
-        byte[] tmp = new byte[data1.length + data2.length];
-        System.arraycopy(data1, 0, tmp, 0, data1.length);
-        System.arraycopy(data2, 0, tmp, data1.length, data2.length);
-
-        DrawingRecord dg2 = (DrawingRecord)rec.get(0);
-        assertEquals(data1.length + data2.length, dg2.getData().length);
-        assertTrue(Arrays.equals(tmp, dg2.getData()));
+        assertTrue(Arrays.equals(data1, ((DrawingRecord)rec.get(0)).getData()));
+        assertTrue(Arrays.equals(data2, ((ContinueRecord)rec.get(1)).getData()));
 
     }