Bug 63153 - Removing a data series from a scatter chart corrupts the workbook
Summary: Removing a data series from a scatter chart corrupts the workbook
Status: NEW
Alias: None
Product: POI
Classification: Unclassified
Component: XDDF (show other bugs)
Version: 4.0.x-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-07 16:41 UTC by David Gauntt
Modified: 2019-05-11 13:19 UTC (History)
0 users



Attachments
Patch to XDDFChartData to fix 63153 (1.51 KB, patch)
2019-05-10 11:52 UTC, David Gauntt
Details | Diff
Additional patch to fix bug 63153 (79 bytes, patch)
2019-05-10 11:57 UTC, David Gauntt
Details | Diff
JUnit test case to verify bug fix for 63153 (6.46 KB, patch)
2019-05-10 12:05 UTC, David Gauntt
Details | Diff
Revised JUnit test case to verify bug fix for 63153 (6.87 KB, text/plain)
2019-05-11 13:19 UTC, David Gauntt
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Gauntt 2019-02-07 16:41:24 UTC
If a series is removed from a scatter chart, the workbook is corrupted.  When Excel 2011 for Mac or Excel 2010 opens the workbook, Excel reports that the workbook is damaged.  I am using POI version 4.0.1

This has been seen both in charts added to a workbook, and charts that are already in existing workbooks.

The code below adds a sheet to a workbook, adds a scatter chart to the sheet, and adds two data series to the chart.  If nMaxSeries is set to 2 or more, the series are not removed from the chart, and the workbook is not corrupted.  If nMaxSeries is set to less than 2, at least one series is removed from the chart and the workbook is corrupted.

	/**
	 * This method creates a scatter chart in workbook, and adds two data
	 * series. If either scatter series is removed, the chart is damaged.
	 *
	 * @param dstWorkbook
	 */
	private static void removeChartSeriesTest(XSSFWorkbook dstWorkbook) {
		final String procName = "removeChartSeriesTest";
		final int nMaxSeries = 1;
		final XSSFSheet sheet = dstWorkbook.createSheet("Remove series test");
		final XSSFDrawing xssfDrawing = sheet.createDrawingPatriarch();
		final XSSFClientAnchor anchor = xssfDrawing.createAnchor(0, 0, 0, 0, 1, 5, 20, 20);
		final XDDFChart chart = xssfDrawing.createChart(anchor);
		final XDDFValueAxis bottomAxis = chart.createValueAxis(AxisPosition.BOTTOM);
		final XDDFValueAxis leftAxis = chart.createValueAxis(AxisPosition.LEFT);

		// Initialize data data sources

		final Double dX[] = new Double[5];
		final Double dY1[] = new Double[5];
		final Double dY2[] = new Double[5];

		for (int n = 0; n < 5; ++n) {
			dX[n] = (double) n;
			dY1[n] = 2.0 * n;
			dY2[n] = (double) (n * n);

		}

		final XDDFNumericalDataSource<Double> xData = XDDFDataSourcesFactory.fromArray(dX, null);
		final XDDFNumericalDataSource<Double> yData1 = XDDFDataSourcesFactory.fromArray(dY1, null);
		final XDDFNumericalDataSource<Double> yData2 = XDDFDataSourcesFactory.fromArray(dY2, null);

		// Creat the chartdata

		final XDDFScatterChartData chartData = (XDDFScatterChartData) chart.createData(ChartTypes.SCATTER, bottomAxis,
				leftAxis);

		// Add the series

		chartData.addSeries(xData, yData1);
		chartData.addSeries(xData, yData2);

		System.out.println(String.format("\r\n%s: chartData.getSeries.size=%d nMaxSheets=%d", procName,
				chartData.getSeries().size(), nMaxSeries));

		// Remove any extra series

		while (chartData.getSeries().size() > nMaxSeries) {
			chartData.getSeries().remove(0);
		}
		System.out.println(String.format("%s: chartData.getSeries.size=%d", procName, chartData.getSeries().size()));

		// Finish up
		chart.plot(chartData);
		final int nSheet = dstWorkbook.getSheetIndex(sheet);
		dstWorkbook.setSelectedTab(nSheet);
		dstWorkbook.setActiveSheet(nSheet);
		dstWorkbook.setFirstVisibleTab(nSheet);

	}
Comment 1 David Gauntt 2019-05-10 11:52:37 UTC
Created attachment 36579 [details]
Patch to XDDFChartData to fix 63153

This code should replace the method getSeries() in XDDFChartData.  The patch includes the method _removeSeries(int), which should be made abstract and implemented in all classes that extend XDDFChartData.  I will add a patch to XDDFScatterChartData  and a JUnit test case shortly.
Comment 2 David Gauntt 2019-05-10 11:57:09 UTC
Created attachment 36580 [details]
Additional patch to fix bug 63153

This patch should be added to the following classes, which is the list of classes that extend XDDFChartData

XDDFBarChartData
XDDFLineChartData
XDDFPieChartData
XDDFRadarChartData
XDDFScatterChartData

After the patch has been added to all of these classes, then _removeSeries in XDDFChartData should be made abstract.

A test case for XDDFScatterChartData will be posted shortly.
Comment 3 David Gauntt 2019-05-10 12:05:38 UTC
Created attachment 36581 [details]
JUnit test case to verify bug fix for 63153

This test case writes 5 workbooks to the folder custom-reports-test.  Each workbook contains a single sheet with a single scatter chart.

testDontRemoveSeries.xlsx: the scatter chart has two data series. Excel 2010 opens and displays it without trouble.

testRemoveSeries0.xlsx: the first data series has been removed by calling getSeries().remove(0).  The workbook is corrupted, and Excel 2010 fixes it by deleting the chart.

testBugFixRemoveSeries0.xlsx: the first data series has been removed by calling removeSeries(0).   Excel 2010 opens and displays it without trouble.  However, the appearance of the remaining data series is different than in testDontRemoveSeries.xlsx.  I will open a new bug report to deal with this.

testRemoveSeries1.xlsx, testBugFixRemoveSeries1.xlsx: the same as the two preceding workbooks, but with the second series removed instead of the first.
Comment 4 David Gauntt 2019-05-11 13:19:01 UTC
Created attachment 36582 [details]
Revised JUnit test case to verify bug fix for 63153

This test case writes 5 workbooks to the folder custom-reports-test.  Each workbook contains a single sheet with a single scatter chart

testDontRemoveSeries.xlsx: this tests adds two data series and removes none. Excel 2010 opens and displays it without trouble.

testRemoveSeries0.xlsx, testRemoveLastSeries.xlsx: These tests add three data series and removes the either the first or the last series by calling getSeries().remove(n).  The workbook is corrupted, and Excel 2010 fixes it by deleting the chart.

testBugFixRemoveSeries0.xlsx, testBugFixRemoveLastSeries.xlsx:  These tests add three data series and removes the either the first or the last series by calling removeSeries(n).  Excel opens and displays the workbook without trouble.  The appearance of the charts is similar to the chart in testDontRemoveSeries.