Bug 53010

Summary: [GSoC2012] Improve drawing support in HSSF
Product: POI Reporter: Evgeniy Berlog <superrubiroyd>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 3.8-dev   
Target Milestone: ---   
Hardware: PC   
OS: All   
Bug Depends on: 45129, 47624, 48590, 48654, 48873, 50696, 51287, 51341, 51455, 51675, 51676, 51683, 51796, 52272, 52300, 52764, 53302, 53361, 53372, 53561    
Bug Blocks:    
Attachments: improved work with continue records in drawing layer, added new tests
improved work with continue records in drawing layer, added new tests
file with NoteRecords
file for test with new records sequence

Description Evgeniy Berlog 2012-03-31 14:46:40 UTC
One of drawbacks of the current implementation is limited support for Excel drawing layer: HSSF can create new drawings from scratch but cannot modify existing ones. 

This drawback is frequently reported on the mailing lists and the fix is in high demand.
Comment 1 Evgeniy Berlog 2012-05-30 12:25:38 UTC
Created attachment 28861 [details]
improved work with continue records in drawing layer, added new tests

Added such improvements:
1. new tests which verify that POI HSSF module correctly reads drawing data, build correctly tree from escher records and serialize it into correct byte array.
2. Fixed bug with twice read continue records
Comment 2 Yegor Kozlov 2012-05-31 09:47:40 UTC
Applied in r1344621

Yegor
Comment 3 Evgeniy Berlog 2012-06-03 21:13:46 UTC
Created attachment 28877 [details]
improved work with continue records in drawing layer, added new tests
Comment 4 Evgeniy Berlog 2012-06-03 21:15:38 UTC
Created attachment 28878 [details]
file with NoteRecords
Comment 5 Evgeniy Berlog 2012-06-03 21:17:54 UTC
Created attachment 28879 [details]
file for test with new records sequence

HSSF realization could read drawing layer only in such sequence:
DrawingRecord
ObjRecord | TextObjRecord
...
DrawingRecord
ObjRecord | TextObjRecord

Xls files can contain such sequence:
DrawingRecord
ContinueRecord
...
ContinueRecord
ObjRecord | TextObjectRecord
.....
ContinueRecord
 ...
ContinueRecord
ObjRecord | TextObjectRecord 

Fixed in uploaded patch
Comment 6 Yegor Kozlov 2012-06-04 08:09:16 UTC
Applied in r1345858

Yegor

(In reply to comment #3)
> Created attachment 28877 [details]
> improved work with continue records in drawing layer, added new tests
Comment 7 Mark B 2012-07-29 11:02:15 UTC
Not so sure that this is an error but it certainly could be a problem with the javadoc for the XSSFWorkbook/OPCPackage classes.

Had a quick play with the code this morning and read the full stacktrace produced when the exception is thrown. The source of the exception is the write method of the workbook - or one of the methods it calls - and it appears to be caused by the fact that the OPCPackage is no longer available when the workbook is being written out. If the call to the close() method of the OPCPackage instance if moved to follow the write() method of the workbook, then the exception is not thrown.

So, it seems that if a workbook is cretade from an instance of the OPCPackage class, the OPCPacakge instance must not be disposed of until after the workbook has been written. This change the the code, removed the problem completely;

org.apache.poi.openxml4j.opc.OPCPackage opc = 
   org.apache.poi.openxml4j.opc.OPCPackage.open(filename);
org.apache.poi.xssf.usermodel.XSSFWorkbook wb =
   new org.apache.poi.xssf.usermodel.XSSFWorkbook(opc);
java.io.FileOutputStream fileOut = new java.io.FileOutputStream(filename);
wb.write(fileOut);
opc.close();
fileOut.close();  

Perhaps documenting this in the javadoc for the workbook and OPCPackage classes would be sufficient.

Mark B
Comment 8 Mark B 2012-07-29 11:03:40 UTC
Please ignore the above. Posted to the wrong bug listing, it should have been 53613. Off to correct this now, sorry.

Mark B
Comment 9 Evgeniy Berlog 2012-08-21 21:08:01 UTC
GSoC project is complete.
All related tickets are resolved and all changes are committed to the trunk.

With best regards,
Evgeniy Berlog