Bug 48087 - org.apache.poi.xssf.usermodel.XSSFChartSheet completely broken
Summary: org.apache.poi.xssf.usermodel.XSSFChartSheet completely broken
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.5-FINAL
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-29 11:53 UTC by Robin Salkeld
Modified: 2009-11-03 16:11 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
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