Bug 48087

Summary: org.apache.poi.xssf.usermodel.XSSFChartSheet completely broken
Product: POI Reporter: Robin Salkeld <robin.salkeld>
Component: XSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: major    
Priority: P2    
Version: 3.5-FINAL   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   

Description Robin Salkeld 2009-10-29 11:53:12 UTC
This class extends XSSFSheet, but fails to initialize any of that class' member fields in its #read(InputStream is) method.

This means reading in any excel file with a chart sheet results in a Sheet instance that throws NPEs from just about any method.
Comment 1 Yegor Kozlov 2009-10-30 01:07:18 UTC
It is not broken, rather not implemented. 
 
Initializing of the superclass is skipped intentionally. Chart sheets don't have rows/columns and most of methods from XSSFSheet don't make sense for XSSFChartSheet. 

Can you post a use-case that results in NPE? What methods are you calling? 

I agree that NPE is not appropriate. get* methods of the Sheet interface that don't make sense should either return an empty iterator or false or 0:
At least, the code below should NOT throw NPE:
for(Row row : chartSheet)
for(int i=0; i < chartSheet.getPhysicalNumberOfRows()) 

set* modifiers should either do nothing or throw "Not supported for charts".


Yegor
Comment 2 Robin Salkeld 2009-10-30 10:26:36 UTC
(In reply to comment #1)
> It is not broken, rather not implemented. 
> 
> Initializing of the superclass is skipped intentionally. Chart sheets don't
> have rows/columns and most of methods from XSSFSheet don't make sense for
> XSSFChartSheet. 
> 
> Can you post a use-case that results in NPE? What methods are you calling? 
> 
> I agree that NPE is not appropriate. get* methods of the Sheet interface that
> don't make sense should either return an empty iterator or false or 0:
> At least, the code below should NOT throw NPE:
> for(Row row : chartSheet)
> for(int i=0; i < chartSheet.getPhysicalNumberOfRows()) 
> 
> set* modifiers should either do nothing or throw "Not supported for charts".
> 
> 
> Yegor

Nearly any method related to the data will do it, since all the fields are null. For a use case, let's say getFirstRowNum or getLastRowNum, since you'd need to call them at least to discover that a chart sheet has no actual data (or getPhysicalNumberOfRows() as you mentioned). Are you looking for a concrete test case?

I should also point out that the implicit no argument constructor is inconsistent with the deserialization method (i.e. XSSFChartSheet(PackagePart part, PackageRelationship rel) followed by a call to #read), since the former actually DOES initialize the superclass.
Comment 3 Yegor Kozlov 2009-11-03 16:11:51 UTC
Fixed in r832622

The simplest way to fix it is to initialize the superclass with a blank worksheet:

    protected void read(InputStream is) throws IOException {
        //initialize the supeclass with a blank worksheet
        super.read(new ByteArrayInputStream(BLANK_WORKSHEET));

        try {
            chartsheet = ChartsheetDocument.Factory.parse(is).getChartsheet();
        } catch (XmlException e){
            throw new POIXMLException(e);
        }
    }

This way all get* methods of the superclass work.

Yegor