bug 46250 - Fixed cloning worksheets with images

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta4" date="2011-??-??">
+           <action dev="poi-developers" type="fix">46250 - Fixed cloning worksheets with images</action>
            <action dev="poi-developers" type="fix">51524 - PapBinTable constructor is slow (regression)</action>
            <action dev="poi-developers" type="fix">51514 - allow HSSFObjectData to work with both POIFS and NPOIFS</action>
            <action dev="poi-developers" type="fix">51514 - avoid NPE when copying nodes from one HSSF workbook to a new one, when opened from NPOIFS</action>

src/java/org/apache/poi/ddf/EscherOptRecord.java

@@ -40,7 +40,7 @@ public class EscherOptRecord
     public static final short RECORD_ID = (short) 0xF00B;
     public static final String RECORD_DESCRIPTION = "msofbtOPT";
 
-    private List properties = new ArrayList();
+    private List<EscherProperty> properties = new ArrayList<EscherProperty>();
 
     public int fillFields(byte[] data, int offset, EscherRecordFactory recordFactory) {
         int bytesRemaining = readHeader( data, offset );
@@ -126,7 +126,7 @@ public class EscherOptRecord
     /**
      * The list of properties stored by this record.
      */
-    public List getEscherProperties()
+    public List<EscherProperty> getEscherProperties()
     {
         return properties;
     }
@@ -136,7 +136,7 @@ public class EscherOptRecord
      */
     public EscherProperty getEscherProperty( int index )
     {
-        return (EscherProperty) properties.get( index );
+        return properties.get( index );
     }
 
     /**
@@ -163,5 +163,14 @@ public class EscherOptRecord
         } );
     }
 
+    public EscherProperty lookup(int propId)
+    {
+        for (EscherProperty prop : properties)
+        {
+            if (prop.getPropertyNumber() == propId)
+                return prop;
+        }
+        return null;
+    }
 
 }

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

@@ -54,6 +54,7 @@ import org.apache.poi.hssf.record.RowRecord;
 import org.apache.poi.hssf.record.SCLRecord;
 import org.apache.poi.hssf.record.SaveRecalcRecord;
 import org.apache.poi.hssf.record.SelectionRecord;
+import org.apache.poi.hssf.record.TextObjectRecord;
 import org.apache.poi.hssf.record.UncalcedRecord;
 import org.apache.poi.hssf.record.WSBoolRecord;
 import org.apache.poi.hssf.record.WindowTwoRecord;
@@ -1537,10 +1538,13 @@ public final class InternalSheet {
         int startloc = loc;
         while ( loc + 1 < records.size()
                 && records.get( loc ) instanceof DrawingRecord
-                && records.get( loc + 1 ) instanceof ObjRecord )
+                && (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);

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

@@ -35,6 +35,7 @@ import org.apache.poi.ddf.EscherOptRecord;
 import org.apache.poi.ddf.EscherProperties;
 import org.apache.poi.ddf.EscherRGBProperty;
 import org.apache.poi.ddf.EscherRecord;
+import org.apache.poi.ddf.EscherSimpleProperty;
 import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.ddf.EscherSplitMenuColorsRecord;
 import org.apache.poi.hssf.record.BOFRecord;
@@ -81,10 +82,9 @@ import org.apache.poi.hssf.record.WindowProtectRecord;
 import org.apache.poi.hssf.record.WriteAccessRecord;
 import org.apache.poi.hssf.record.WriteProtectRecord;
 import org.apache.poi.hssf.record.common.UnicodeString;
-import org.apache.poi.ss.formula.ptg.NameXPtg;
 import org.apache.poi.ss.formula.FormulaShifter;
 import org.apache.poi.ss.formula.udf.UDFFinder;
-import org.apache.poi.ss.formula.ptg.Ptg;
+import org.apache.poi.ss.formula.ptg.*;
 import org.apache.poi.hssf.util.HSSFColor;
 import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalName;
 import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalSheet;
@@ -2075,15 +2075,23 @@ public final class InternalWorkbook {
                 }
 
                 EscherDggRecord dgg = null;
+                EscherContainerRecord bStore = null;
                 for(Iterator<EscherRecord> it = cr.getChildIterator(); it.hasNext();) {
-                    Object er = it.next();
+                    EscherRecord er = it.next();
                     if(er instanceof EscherDggRecord) {
                         dgg = (EscherDggRecord)er;
+                    } else if (er.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER) {
+                        bStore = (EscherContainerRecord) er;
                     }
                 }
 
                 if(dgg != null) {
                     drawingManager = new DrawingManager2(dgg);
+                    if(bStore != null){
+                        for(EscherRecord bs : bStore.getChildRecords()){
+                            if(bs instanceof EscherBSERecord) escherBSERecords.add((EscherBSERecord)bs);
+                        }
+                    }
                     return;
                 }
             }
@@ -2096,14 +2104,22 @@ public final class InternalWorkbook {
         if(dgLoc != -1) {
             DrawingGroupRecord dg = (DrawingGroupRecord)records.get(dgLoc);
             EscherDggRecord dgg = null;
+            EscherContainerRecord bStore = null;
             for(EscherRecord er : dg.getEscherRecords()) {
-                if(er instanceof EscherDggRecord) {
+                if (er instanceof EscherDggRecord) {
-                    dgg = (EscherDggRecord)er;
+                    dgg = (EscherDggRecord) er;
+                } else if (er.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER) {
+                    bStore = (EscherContainerRecord) er;
                 }
             }
 
             if(dgg != null) {
                 drawingManager = new DrawingManager2(dgg);
+                if(bStore != null){
+                    for(EscherRecord bs : bStore.getChildRecords()){
+                        if(bs instanceof EscherBSERecord) escherBSERecords.add((EscherBSERecord)bs);
+                    }
+                }
             }
         }
     }
@@ -2357,23 +2373,61 @@ public final class InternalWorkbook {
                     //update id of the drawing in the cloned sheet
                     dg.setOptions( (short) ( dgId << 4 ) );
                 } else if (er instanceof EscherContainerRecord){
-                    //recursively find shape records and re-generate shapeId
+                    // iterate over shapes and re-generate shapeId
-                    List<EscherRecord> spRecords = new ArrayList<EscherRecord>();
                     EscherContainerRecord cp = (EscherContainerRecord)er;
-                    cp.getRecordsById(EscherSpRecord.RECORD_ID,  spRecords);
+                    for(Iterator<EscherRecord> spIt = cp.getChildRecords().iterator(); spIt.hasNext();) {
-                    for(Iterator<EscherRecord> spIt = spRecords.iterator(); spIt.hasNext();) {
+                        EscherContainerRecord shapeContainer = (EscherContainerRecord)spIt.next();
-                        EscherSpRecord sp = (EscherSpRecord)spIt.next();
+                        for(EscherRecord shapeChildRecord : shapeContainer.getChildRecords()) {
-                        int shapeId = drawingManager.allocateShapeId((short)dgId, dg);
+                            int recordId = shapeChildRecord.getRecordId();
-                        //allocateShapeId increments the number of shapes. roll back to the previous value
+                            if (recordId == EscherSpRecord.RECORD_ID){
-                        dg.setNumShapes(dg.getNumShapes()-1);
+                                EscherSpRecord sp = (EscherSpRecord)shapeChildRecord;
-                        sp.setShapeId(shapeId);
+                                int shapeId = drawingManager.allocateShapeId((short)dgId, dg);
+                                //allocateShapeId increments the number of shapes. roll back to the previous value
+                                dg.setNumShapes(dg.getNumShapes()-1);
+                                sp.setShapeId(shapeId);
+                            } else if (recordId == EscherOptRecord.RECORD_ID){
+                                EscherOptRecord opt = (EscherOptRecord)shapeChildRecord;
+                                EscherSimpleProperty prop = (EscherSimpleProperty)opt.lookup(
+                                        EscherProperties.BLIP__BLIPTODISPLAY );
+                                if (prop != null){
+                                    int pictureIndex = prop.getPropertyValue();
+                                    // increment reference count for pictures
+                                    EscherBSERecord bse = getBSERecord(pictureIndex);
+                                    bse.setRef(bse.getRef() + 1);
+                                }
+
+                            }
+                        }
                     }
                 }
             }
-
         }
     }
+    
+    public NameRecord cloneFilter(int filterDbNameIndex, int newSheetIndex){
+        NameRecord origNameRecord = getNameRecord(filterDbNameIndex);
+        // copy original formula but adjust 3D refs to the new external sheet index
+        int newExtSheetIx = checkExternSheet(newSheetIndex);
+        Ptg[] ptgs = origNameRecord.getNameDefinition();
+        for (int i=0; i< ptgs.length; i++) {
+            Ptg ptg = ptgs[i];
 
+            if (ptg instanceof Area3DPtg) {
+                Area3DPtg a3p = (Area3DPtg) ((OperandPtg) ptg).copy();
+                a3p.setExternSheetIndex(newExtSheetIx);
+                ptgs[i] = a3p;
+            } else if (ptg instanceof Ref3DPtg) {
+                Ref3DPtg r3p = (Ref3DPtg) ((OperandPtg) ptg).copy();
+                r3p.setExternSheetIndex(newExtSheetIx);
+                ptgs[i] = r3p;
+            }
+        }
+        NameRecord newNameRecord = createBuiltInName(NameRecord.BUILTIN_FILTER_DB, newSheetIndex+1);
+        newNameRecord.setNameDefinition(ptgs);
+        newNameRecord.setHidden(true);
+        return newNameRecord;
+
+    }
     /**
      * Updates named ranges due to moving of cells
      */

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

@@ -631,8 +631,8 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 					// org.apache.poi.hslf.model.Picture.getPictureIndex()
 					EscherOptRecord opt = (EscherOptRecord) getEscherChild(
 							shapeContainer, EscherOptRecord.RECORD_ID );
-					EscherSimpleProperty prop = (EscherSimpleProperty) getEscherProperty(
+					EscherSimpleProperty prop = (EscherSimpleProperty)opt.lookup(
-							opt, EscherProperties.BLIP__BLIPTODISPLAY );
+							EscherProperties.BLIP__BLIPTODISPLAY );
 					if (prop == null)
 					{
 						log.log( POILogger.WARN,
@@ -965,23 +965,4 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 		return null;
 	}
 
-	/**
-	 * Returns escher property by id.
-	 * 
-	 * @return escher property or <code>null</code> if not found.
-	 */
-	private static EscherProperty getEscherProperty(EscherOptRecord opt,
-			int propId)
-	{
-		if (opt != null)
-			for (Iterator iterator = opt.getEscherProperties().iterator(); iterator
-					.hasNext();)
-			{
-				EscherProperty prop = (EscherProperty) iterator.next();
-				if (prop.getPropertyNumber() == propId)
-					return prop;
-			}
-		return null;
-	}
-
 }

src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -683,7 +683,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
 
     public HSSFSheet cloneSheet(int sheetIndex) {
         validateSheetIndex(sheetIndex);
-        HSSFSheet srcSheet = (HSSFSheet) _sheets.get(sheetIndex);
+        HSSFSheet srcSheet = _sheets.get(sheetIndex);
         String srcName = workbook.getSheetName(sheetIndex);
         HSSFSheet clonedSheet = srcSheet.cloneSheet(this);
         clonedSheet.setSelected(false);
@@ -696,33 +696,13 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
 
         // Check this sheet has an autofilter, (which has a built-in NameRecord at workbook level)
         int filterDbNameIndex = findExistingBuiltinNameRecordIdx(sheetIndex, NameRecord.BUILTIN_FILTER_DB);
-        if (filterDbNameIndex >=0) {
+        if (filterDbNameIndex != -1) {
-            NameRecord origNameRecord = workbook.getNameRecord(filterDbNameIndex);
+            NameRecord newNameRecord = workbook.cloneFilter(filterDbNameIndex, newSheetIndex);
-            // copy original formula but adjust 3D refs to the new external sheet index
-            int newExtSheetIx = workbook.checkExternSheet(newSheetIndex);
-            Ptg[] ptgs = origNameRecord.getNameDefinition();
-            for (int i=0; i< ptgs.length; i++) {
-                Ptg ptg = ptgs[i];
-
-                if (ptg instanceof Area3DPtg) {
-                    Area3DPtg a3p = (Area3DPtg) ((OperandPtg) ptg).copy();
-                    a3p.setExternSheetIndex(newExtSheetIx);
-                    ptgs[i] = a3p;
-                } else if (ptg instanceof Ref3DPtg) {
-                    Ref3DPtg r3p = (Ref3DPtg) ((OperandPtg) ptg).copy();
-                    r3p.setExternSheetIndex(newExtSheetIx);
-                    ptgs[i] = r3p;
-                }
-            }
-            NameRecord newNameRecord = workbook.createBuiltInName(NameRecord.BUILTIN_FILTER_DB, newSheetIndex+1);
-            newNameRecord.setNameDefinition(ptgs);
-            newNameRecord.setHidden(true);
             HSSFName newName = new HSSFName(this, newNameRecord);
             names.add(newName);
-
-            workbook.cloneDrawings(clonedSheet.getSheet());
         }
         // TODO - maybe same logic required for other/all built-in name records
+        workbook.cloneDrawings(clonedSheet.getSheet());
 
         return clonedSheet;
     }

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

@@ -17,42 +17,27 @@
 
 package org.apache.poi.hssf.model;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
-
+import org.apache.poi.ddf.EscherDggRecord;
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.hssf.record.BOFRecord;
+import org.apache.poi.hssf.record.*;
-import org.apache.poi.hssf.record.BlankRecord;
-import org.apache.poi.hssf.record.CellValueRecordInterface;
-import org.apache.poi.hssf.record.ColumnInfoRecord;
-import org.apache.poi.hssf.record.DimensionsRecord;
-import org.apache.poi.hssf.record.EOFRecord;
-import org.apache.poi.hssf.record.FormulaRecord;
-import org.apache.poi.hssf.record.GutsRecord;
-import org.apache.poi.hssf.record.IndexRecord;
-import org.apache.poi.hssf.record.MergeCellsRecord;
-import org.apache.poi.hssf.record.MulBlankRecord;
-import org.apache.poi.hssf.record.NumberRecord;
-import org.apache.poi.hssf.record.Record;
-import org.apache.poi.hssf.record.RecordBase;
-import org.apache.poi.hssf.record.RowRecord;
-import org.apache.poi.hssf.record.StringRecord;
-import org.apache.poi.hssf.record.UncalcedRecord;
-import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.record.aggregates.ConditionalFormattingTable;
 import org.apache.poi.hssf.record.aggregates.PageSettingsBlock;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
-import org.apache.poi.ss.formula.FormulaShifter;
 import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFRichTextString;
 import org.apache.poi.hssf.usermodel.HSSFRow;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector;
+import org.apache.poi.ss.formula.FormulaShifter;
 import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.util.HexRead;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * Unit test for the {@link InternalSheet} class.
@@ -82,10 +67,10 @@ public final class TestSheet extends TestCase {
 
 		int pos = 0;
 		assertTrue(outRecs[pos++] instanceof BOFRecord );
-		assertTrue(outRecs[pos++] instanceof IndexRecord );
+		assertTrue(outRecs[pos++] instanceof IndexRecord);
-		assertTrue(outRecs[pos++] instanceof DimensionsRecord );
+		assertTrue(outRecs[pos++] instanceof DimensionsRecord);
 		assertTrue(outRecs[pos++] instanceof WindowTwoRecord );
-		assertTrue(outRecs[pos++] instanceof EOFRecord );
+		assertTrue(outRecs[pos++] instanceof EOFRecord);
 	}
 
 	private static Record createWindow2Record() {
@@ -701,4 +686,91 @@ public final class TestSheet extends TestCase {
 		Record[] clonedRecs = rc.getRecords();
 		assertEquals(recs.length+2, clonedRecs.length); // +2 for INDEX and DBCELL
 	}
+
+    public void testCreateAggregate() {
+        String msoDrawingRecord1 =
+                "0F 00 02 F0 20 01 00 00 10 00 08 F0 08 00 00 00 \n" +
+                "03 00 00 00 02 04 00 00 0F 00 03 F0 08 01 00 00 \n" +
+                "0F 00 04 F0 28 00 00 00 01 00 09 F0 10 00 00 00 \n" +
+                "00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \n" +
+                "02 00 0A F0 08 00 00 00 00 04 00 00 05 00 00 00 \n" +
+                "0F 00 04 F0 64 00 00 00 42 01 0A F0 08 00 00 00 \n" +
+                "01 04 00 00 00 0A 00 00 73 00 0B F0 2A 00 00 00 \n" +
+                "BF 00 08 00 08 00 44 01 04 00 00 00 7F 01 00 00 \n" +
+                "01 00 BF 01 00 00 11 00 C0 01 40 00 00 08 FF 01 \n" +
+                "10 00 10 00 BF 03 00 00 08 00 00 00 10 F0 12 00 \n" +
+                "00 00 00 00 01 00 54 00 05 00 45 00 01 00 88 03 \n" +
+                "05 00 94 00 00 00 11 F0 00 00 00 00";
+
+        String msoDrawingRecord2 =
+                "0F 00 04 F0 64 00 00 00 42 01 0A F0 08 00 00 00 " +
+                "02 04 00 00 80 0A 00 00 73 00 0B F0 2A 00 00 00 " +
+                "BF 00 08 00 08 00 44 01 04 00 00 00 7F 01 00 00 " +
+                "01 00 BF 01 00 00 11 00 C0 01 40 00 00 08 FF 01 " +
+                "10 00 10 00 BF 03 00 00 08 00 00 00 10 F0 12 00 " +
+                "00 00 00 00 01 00 8D 03 05 00 E4 00 03 00 4D 03 " +
+                "0B 00 0C 00 00 00 11 F0 00 00 00 00";
+
+        DrawingRecord d1 = new DrawingRecord();
+        d1.setData( HexRead.readFromString( msoDrawingRecord1 ) );
+
+        ObjRecord r1 = new ObjRecord();
+
+        DrawingRecord d2 = new DrawingRecord();
+        d2.setData( HexRead.readFromString( msoDrawingRecord2 ) );
+
+        TextObjectRecord r2 = new TextObjectRecord();
+        r2.setStr(new HSSFRichTextString("Aggregated"));
+        NoteRecord n2 = new NoteRecord();
+
+        List<Record> recordStream = new ArrayList<Record>();
+        recordStream.add(InternalSheet.createBOF());
+        recordStream.add( d1 );
+        recordStream.add( r1 );
+        recordStream.add(createWindow2Record());
+        recordStream.add(EOFRecord.instance);
+
+        confirmAggregatedRecords(recordStream);
+
+
+        recordStream = new ArrayList<Record>();
+        recordStream.add(InternalSheet.createBOF());
+        recordStream.add( d1 );
+        recordStream.add( r1 );
+        recordStream.add( d2 );
+        recordStream.add( r2 );
+        recordStream.add(createWindow2Record());
+        recordStream.add(EOFRecord.instance);
+
+        confirmAggregatedRecords(recordStream);
+
+        recordStream = new ArrayList<Record>();
+        recordStream.add(InternalSheet.createBOF());
+        recordStream.add( d1 );
+        recordStream.add( r1 );
+        recordStream.add( d2 );
+        recordStream.add( r2 );
+        recordStream.add( n2 );
+        recordStream.add(createWindow2Record());
+        recordStream.add(EOFRecord.instance);
+
+        confirmAggregatedRecords(recordStream);
+     }
+
+    private void confirmAggregatedRecords(List<Record> recordStream){
+        InternalSheet sheet = InternalSheet.createSheet();
+        sheet.getRecords().clear();
+        sheet.getRecords().addAll(recordStream);
+
+        List<RecordBase> sheetRecords = sheet.getRecords();
+
+        DrawingManager2 drawingManager = new DrawingManager2(new EscherDggRecord() );
+        sheet.aggregateDrawingRecords(drawingManager, false);
+
+        assertEquals(4, sheetRecords.size());
+        assertEquals(BOFRecord.sid, ((Record)sheetRecords.get(0)).getSid());
+        assertEquals(EscherAggregate.sid, ((Record)sheetRecords.get(1)).getSid());
+        assertEquals(WindowTwoRecord.sid, ((Record)sheetRecords.get(2)).getSid());
+        assertEquals(EOFRecord.sid, ((Record)sheetRecords.get(3)).getSid());
+    }
 }

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

@@ -38,6 +38,7 @@ import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.POIDataSamples;
+import org.apache.poi.ddf.EscherBSERecord;
 import org.apache.poi.hpsf.ClassID;
 
 /**
@@ -689,4 +690,27 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
            return false;
        }
    }
+
+    public void testClonePictures() throws IOException {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("SimpleWithImages.xls");
+        InternalWorkbook iwb = wb.getWorkbook();
+        iwb.findDrawingGroup();
+        
+        for(int pictureIndex=1; pictureIndex <= 4; pictureIndex++){
+            EscherBSERecord bse = iwb.getBSERecord(pictureIndex);
+            assertEquals(1, bse.getRef());
+        }
+
+        wb.cloneSheet(0);
+        for(int pictureIndex=1; pictureIndex <= 4; pictureIndex++){
+            EscherBSERecord bse = iwb.getBSERecord(pictureIndex);
+            assertEquals(2, bse.getRef());
+        }
+
+        wb.cloneSheet(0);
+        for(int pictureIndex=1; pictureIndex <= 4; pictureIndex++){
+            EscherBSERecord bse = iwb.getBSERecord(pictureIndex);
+            assertEquals(3, bse.getRef());
+        }
+    }
 }