From 5c13b179a162a87c07f10324743cc84dadadc612 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 13 Aug 2022 18:14:25 -0700 Subject: [PATCH] Replace Dev jpgraph/jpgraph with mitoteam/jpgraph (#2997) * Replace Dev jpgraph/jpgraph with mitoteam/jpgraph PR #2979 added support for mitoteam/jpgraph as an alternative to jpgraph/jpgraph. The package jpgraph/jpgraph is abandoned in composer, and the version loaded with composer has been unusable for some time. This PR removes the dev requirement for jpgraph/jpgraph, and adds a dev requirement for mitoteam/jpgraph in its place. With a usable graph library, a number of tests and samples that had been disabled are now re-enabled. A lot of new functionality has been added to Charts recently. Some of that new code has exposed bugs in JpgraphRendererBase. I have fixed those where I could. A handful of exceptions remain; I will investigate, and hopefully fix, those over time, but I don't feel it is necessary to fix them all before installing this PR - we are already way ahead of the game with the graphs that are working. Three members had been ignoring code coverage in whole or in part because of the unavailability of a usable graph libray. Code coverage is restored in them. I am relieved to report that, although they aren't completely covered, adding them did not reduce code coverage by much - it is still over 90.4%. I took a look at JpgraphRendererBase and Phpstan. Phpstan reports 128 problems. When I added some docblocks to correct some of those, the number increased to 284. Sigh. I will investigate over time, but, for now, we will still suppress Phpstan for JpgraphRendererBase. I do not find a License file for mitoteam. However, there also wasn't one for jpgraph in the first place. Based on that and the discussion in #2996 (mitoteam will be used in exactly the same manner as mpdf), I don't think this is a problem. IANAL. * PHP 8.2 Problems Tons of "cannot create dynamic property" deprecations in jpgraph. Disable the test with most of those for now; leave the two with only a handful of messages enabled. * Correct Failures in 2 Stock Charts Down to 6 templates on which Render fails. --- composer.json | 9 +- composer.lock | 88 ++++++++++--------- phpstan.neon.dist | 1 - samples/Chart/32_Chart_read_write_HTML.php | 3 +- samples/Chart/32_Chart_read_write_PDF.php | 3 +- samples/Chart/35_Chart_render.php | 26 ++++-- src/PhpSpreadsheet/Chart/Chart.php | 4 - .../Chart/Renderer/JpGraphRendererBase.php | 22 +++-- .../Chart/Renderer/MtJpGraphRenderer.php | 2 - src/PhpSpreadsheet/Writer/Html.php | 10 --- .../PhpSpreadsheetTests/Chart/RenderTest.php | 15 ++++ .../PhpSpreadsheetTests/Helper/SampleTest.php | 9 +- 12 files changed, 110 insertions(+), 82 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Chart/RenderTest.php diff --git a/composer.json b/composer.json index 16991514..4ef1c1b4 100644 --- a/composer.json +++ b/composer.json @@ -81,7 +81,7 @@ "dealerdirect/phpcodesniffer-composer-installer": "dev-master", "dompdf/dompdf": "^1.0 || ^2.0", "friendsofphp/php-cs-fixer": "^3.2", - "jpgraph/jpgraph": "^4.0", + "mitoteam/jpgraph": "^10.1", "mpdf/mpdf": "8.1.1", "phpcompatibility/php-compatibility": "^9.3", "phpstan/phpstan": "^1.1", @@ -91,10 +91,11 @@ "tecnickcom/tcpdf": "^6.4" }, "suggest": { + "ext-intl": "PHP Internationalization Functions", "mpdf/mpdf": "Option for rendering PDF with PDF Writer", - "dompdf/dompdf": "Option for rendering PDF with PDF Writer (doesn't yet support PHP8)", - "tecnickcom/tcpdf": "Option for rendering PDF with PDF Writer (doesn't yet support PHP8)", - "jpgraph/jpgraph": "Option for rendering charts, or including charts with PDF or HTML Writers" + "dompdf/dompdf": "Option for rendering PDF with PDF Writer", + "tecnickcom/tcpdf": "Option for rendering PDF with PDF Writer (doesn't yet fully support PHP8)", + "mitoteam/jpgraph": "Option for rendering charts, or including charts with PDF or HTML Writers" }, "autoload": { "psr-4": { diff --git a/composer.lock b/composer.lock index 3f82754d..bbbd0c75 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "6cbb20f8d8f2daae0aeb72431cda0980", + "content-hash": "fc6928651785d4bb82d727d22f50227d", "packages": [ { "name": "ezyang/htmlpurifier", @@ -1192,47 +1192,6 @@ ], "time": "2021-12-11T16:25:08+00:00" }, - { - "name": "jpgraph/jpgraph", - "version": "4.0.2", - "source": { - "type": "git", - "url": "https://github.com/ztec/JpGraph.git", - "reference": "e82db7da6a546d3926c24c9a346226da7aa49094" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/ztec/JpGraph/zipball/e82db7da6a546d3926c24c9a346226da7aa49094", - "reference": "e82db7da6a546d3926c24c9a346226da7aa49094", - "shasum": "" - }, - "type": "library", - "autoload": { - "classmap": [ - "lib/JpGraph.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "QPL 1.0" - ], - "authors": [ - { - "name": "JpGraph team" - } - ], - "description": "jpGraph, library to make graphs and charts", - "homepage": "http://jpgraph.net/", - "keywords": [ - "chart", - "data", - "graph", - "jpgraph", - "pie" - ], - "abandoned": true, - "time": "2017-02-23T09:44:15+00:00" - }, { "name": "masterminds/html5", "version": "2.7.5", @@ -1298,6 +1257,49 @@ ], "time": "2021-07-01T14:25:37+00:00" }, + { + "name": "mitoteam/jpgraph", + "version": "10.1.3", + "source": { + "type": "git", + "url": "https://github.com/mitoteam/jpgraph.git", + "reference": "425a2a0f0c97a28fe0aca60a4384ce85880e438a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/mitoteam/jpgraph/zipball/425a2a0f0c97a28fe0aca60a4384ce85880e438a", + "reference": "425a2a0f0c97a28fe0aca60a4384ce85880e438a", + "shasum": "" + }, + "require": { + "php": ">=5.5" + }, + "type": "library", + "autoload": { + "classmap": [ + "src/MtJpGraph.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "QPL-1.0" + ], + "authors": [ + { + "name": "JpGraph team" + } + ], + "description": "Composer compatible version of JpGraph library with PHP 8.1 support", + "homepage": "https://github.com/mitoteam/jpgraph", + "keywords": [ + "jpgraph" + ], + "support": { + "issues": "https://github.com/mitoteam/jpgraph/issues", + "source": "https://github.com/mitoteam/jpgraph/tree/10.1.3" + }, + "time": "2022-07-05T16:46:34+00:00" + }, { "name": "mpdf/mpdf", "version": "v8.1.1", @@ -5273,5 +5275,5 @@ "ext-zlib": "*" }, "platform-dev": [], - "plugin-api-version": "1.1.0" + "plugin-api-version": "2.2.0" } diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 92767872..5cac36a1 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -12,7 +12,6 @@ parameters: excludePaths: - src/PhpSpreadsheet/Chart/Renderer/JpGraph.php - src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php - - src/PhpSpreadsheet/Chart/Renderer/MtJpGraphRenderer.php parallel: processTimeout: 300.0 checkMissingIterableValueType: false diff --git a/samples/Chart/32_Chart_read_write_HTML.php b/samples/Chart/32_Chart_read_write_HTML.php index 5febbf93..90d61c5d 100644 --- a/samples/Chart/32_Chart_read_write_HTML.php +++ b/samples/Chart/32_Chart_read_write_HTML.php @@ -6,7 +6,8 @@ use PhpOffice\PhpSpreadsheet\Settings; require __DIR__ . '/../Header.php'; // Change these values to select the Rendering library that you wish to use -Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph::class); +//Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph::class); +Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\MtJpGraphRenderer::class); $inputFileType = 'Xlsx'; $inputFileNames = __DIR__ . '/../templates/36write*.xlsx'; diff --git a/samples/Chart/32_Chart_read_write_PDF.php b/samples/Chart/32_Chart_read_write_PDF.php index ee3ad0e0..214c3d38 100644 --- a/samples/Chart/32_Chart_read_write_PDF.php +++ b/samples/Chart/32_Chart_read_write_PDF.php @@ -8,7 +8,8 @@ require __DIR__ . '/../Header.php'; IOFactory::registerWriter('Pdf', \PhpOffice\PhpSpreadsheet\Writer\Pdf\Mpdf::class); // Change these values to select the Rendering library that you wish to use -Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph::class); +//Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph::class); +Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\MtJpGraphRenderer::class); $inputFileType = 'Xlsx'; $inputFileNames = __DIR__ . '/../templates/36write*.xlsx'; diff --git a/samples/Chart/35_Chart_render.php b/samples/Chart/35_Chart_render.php index ebab16a7..2376008c 100644 --- a/samples/Chart/35_Chart_render.php +++ b/samples/Chart/35_Chart_render.php @@ -5,16 +5,13 @@ use PhpOffice\PhpSpreadsheet\Settings; require __DIR__ . '/../Header.php'; -if (PHP_VERSION_ID >= 80000) { - $helper->log('Jpgraph no longer runs against PHP8'); - exit; -} - // Change these values to select the Rendering library that you wish to use -Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph::class); +//Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraph::class); +Settings::setChartRenderer(\PhpOffice\PhpSpreadsheet\Chart\Renderer\MtJpGraphRenderer::class); $inputFileType = 'Xlsx'; $inputFileNames = __DIR__ . '/../templates/32readwrite*[0-9].xlsx'; +//$inputFileNames = __DIR__ . '/../templates/32readwriteStockChart5.xlsx'; if ((isset($argc)) && ($argc > 1)) { $inputFileNames = []; @@ -24,6 +21,18 @@ if ((isset($argc)) && ($argc > 1)) { } else { $inputFileNames = glob($inputFileNames); } +if (count($inputFileNames) === 1) { + $unresolvedErrors = []; +} else { + $unresolvedErrors = [ + '32readwriteBubbleChart2.xlsx', + '32readwritePieChart3.xlsx', + '32readwritePieChart4.xlsx', + '32readwritePieChart3D1.xlsx', + '32readwritePieChartExploded1.xlsx', + '32readwritePieChartExploded3D1.xlsx', + ]; +} foreach ($inputFileNames as $inputFileName) { $inputFileNameShort = basename($inputFileName); @@ -32,6 +41,11 @@ foreach ($inputFileNames as $inputFileName) { continue; } + if (in_array($inputFileNameShort, $unresolvedErrors, true)) { + $helper->log('File ' . $inputFileNameShort . ' does not yet work with this script'); + + continue; + } $helper->log("Load Test from $inputFileType file " . $inputFileNameShort); diff --git a/src/PhpSpreadsheet/Chart/Chart.php b/src/PhpSpreadsheet/Chart/Chart.php index b962978d..036338c6 100644 --- a/src/PhpSpreadsheet/Chart/Chart.php +++ b/src/PhpSpreadsheet/Chart/Chart.php @@ -661,14 +661,10 @@ class Chart /** * Render the chart to given file (or stream). - * Unable to cover code until a usable current version of JpGraph - * is made available through Composer. * * @param string $outputDestination Name of the file render to * * @return bool true on success - * - * @codeCoverageIgnore */ public function render($outputDestination = null) { diff --git a/src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php b/src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php index 4d5526b8..f0ce1f65 100644 --- a/src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php +++ b/src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php @@ -277,7 +277,8 @@ abstract class JpGraphRendererBase implements IRenderer { $grouping = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotGrouping(); - $labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex(0)->getPointCount(); + $index = array_keys($this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotOrder())[0]; + $labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getPointCount(); if ($labelCount > 0) { $datasetLabels = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getDataValues(); $datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $labelCount); @@ -294,8 +295,9 @@ abstract class JpGraphRendererBase implements IRenderer // Loop through each data series in turn for ($i = 0; $i < $seriesCount; ++$i) { - $dataValues = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($i)->getDataValues(); - $marker = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($i)->getPointMarker(); + $index = array_keys($this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotOrder())[$i]; + $dataValues = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getDataValues(); + $marker = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getPointMarker(); if ($grouping == 'percentStacked') { $dataValues = $this->percentageAdjustValues($dataValues, $sumValues); @@ -324,7 +326,7 @@ abstract class JpGraphRendererBase implements IRenderer // Set the appropriate plot marker $this->formatPointMarker($seriesPlot, $marker); } - $dataLabel = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotLabelByIndex($i)->getDataValue(); + $dataLabel = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotLabelByIndex($index)->getDataValue(); $seriesPlot->SetLegend($dataLabel); $seriesPlots[] = $seriesPlot; @@ -347,7 +349,8 @@ abstract class JpGraphRendererBase implements IRenderer } $grouping = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotGrouping(); - $labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex(0)->getPointCount(); + $index = array_keys($this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotOrder())[0]; + $labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getPointCount(); if ($labelCount > 0) { $datasetLabels = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getDataValues(); $datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $labelCount, $rotation); @@ -371,7 +374,8 @@ abstract class JpGraphRendererBase implements IRenderer // Loop through each data series in turn for ($j = 0; $j < $seriesCount; ++$j) { - $dataValues = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($j)->getDataValues(); + $index = array_keys($this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotOrder())[$j]; + $dataValues = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getDataValues(); if ($grouping == 'percentStacked') { $dataValues = $this->percentageAdjustValues($dataValues, $sumValues); } @@ -535,6 +539,10 @@ abstract class JpGraphRendererBase implements IRenderer $dataValues = []; // Loop through each data series in turn and build the plot arrays foreach ($plotOrder as $i => $v) { + $dataValuesX = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($v); + if ($dataValuesX === false) { + continue; + } $dataValuesX = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($v)->getDataValues(); foreach ($dataValuesX as $j => $dataValueX) { $dataValues[$plotOrder[$i]][$j] = $dataValueX; @@ -549,7 +557,7 @@ abstract class JpGraphRendererBase implements IRenderer $jMax = count($dataValues[0]); for ($j = 0; $j < $jMax; ++$j) { for ($i = 0; $i < $seriesCount; ++$i) { - $dataValuesPlot[] = $dataValues[$i][$j]; + $dataValuesPlot[] = $dataValues[$i][$j] ?? null; } } diff --git a/src/PhpSpreadsheet/Chart/Renderer/MtJpGraphRenderer.php b/src/PhpSpreadsheet/Chart/Renderer/MtJpGraphRenderer.php index 3fef3b6e..e1f0f90a 100644 --- a/src/PhpSpreadsheet/Chart/Renderer/MtJpGraphRenderer.php +++ b/src/PhpSpreadsheet/Chart/Renderer/MtJpGraphRenderer.php @@ -9,8 +9,6 @@ namespace PhpOffice\PhpSpreadsheet\Chart\Renderer; * https://packagist.org/packages/mitoteam/jpgraph * * This package is up to date for August 2022 and has PHP 8.1 support. - * - * @codeCoverageIgnore */ class MtJpGraphRenderer extends JpGraphRendererBase { diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 6e51b7a8..f6c34a8a 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -551,15 +551,10 @@ class Html extends BaseWriter * Extend Row if chart is placed after nominal end of row. * This code should be exercised by sample: * Chart/32_Chart_read_write_PDF.php. - * However, that test is suppressed due to out-of-date - * Jpgraph code issuing warnings. So, don't measure - * code coverage for this function till that is fixed. * * @param int $row Row to check for charts * * @return array - * - * @codeCoverageIgnore */ private function extendRowsForCharts(Worksheet $worksheet, int $row) { @@ -725,11 +720,6 @@ class Html extends BaseWriter * Generate chart tag in cell. * This code should be exercised by sample: * Chart/32_Chart_read_write_PDF.php. - * However, that test is suppressed due to out-of-date - * Jpgraph code issuing warnings. So, don't measure - * code coverage for this function till that is fixed. - * - * @codeCoverageIgnore */ private function writeChartInCell(Worksheet $worksheet, string $coordinates): string { diff --git a/tests/PhpSpreadsheetTests/Chart/RenderTest.php b/tests/PhpSpreadsheetTests/Chart/RenderTest.php new file mode 100644 index 00000000..f0eaffee --- /dev/null +++ b/tests/PhpSpreadsheetTests/Chart/RenderTest.php @@ -0,0 +1,15 @@ +render()); + } +} diff --git a/tests/PhpSpreadsheetTests/Helper/SampleTest.php b/tests/PhpSpreadsheetTests/Helper/SampleTest.php index 50a650f8..a104e8ff 100644 --- a/tests/PhpSpreadsheetTests/Helper/SampleTest.php +++ b/tests/PhpSpreadsheetTests/Helper/SampleTest.php @@ -26,10 +26,13 @@ class SampleTest extends TestCase public function providerSample(): array { $skipped = [ - 'Chart/32_Chart_read_write_PDF.php', // Unfortunately JpGraph is not up to date for latest PHP and raise many warnings - 'Chart/32_Chart_read_write_HTML.php', // idem - 'Chart/35_Chart_render.php', // idem ]; + if (PHP_VERSION_ID >= 80200) { + // Hopefully temporary. Continue to try + // 32_chart_read_write_PDF/HTML + // so as not to lose track of the problem. + $skipped[] = 'Chart/35_Chart_render.php'; + } // Unfortunately some tests are too long to run with code-coverage // analysis on GitHub Actions, so we need to exclude them