PHPOffice / PhpSpreadsheet

A pure PHP library for reading and writing spreadsheet files
https://phpspreadsheet.readthedocs.io
MIT License
13.31k stars 3.45k forks source link

addExternalSheet should update CalculationEngine->spreadsheet->workSheetCollection (Crash) #1944

Open alaindev opened 3 years ago

alaindev commented 3 years ago

This is:

- [X ] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

Using addExternalSheet to a worksheet should update CalculationEngine->spreadsheet->workSheetCollection

What is the current behavior?

CalculationEngine->spreadsheet->workSheetCollection stays stuck to original loaded file1

$objWriter->generateHTMLAll(); will crash in Calculation since Engine cannot find sheets

What are the steps to reproduce?

$objPHPExcel= \PhpOffice\PhpSpreadsheet\IOFactory::load($excelfile_1); $spreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load($excelfile_2); $objPHPExcel->addExternalSheet(clone spreadsheet );

$objWriter = new \PhpOffice\PhpSpreadsheet\Writer\Html($objPHPExcel); echo $objWriter->generateHTMLAll(); // will crash in Calculation.php 3246
// $this->spreadsheet->getSheetByName($cellAddress['sheet'])->getCell($cellAddress['cell']);

Which versions of PhpSpreadsheet and PHP are affected?

1.17.1 and below

MarkBaker commented 3 years ago

What exactly are you doing is this line:

$objPHPExcel->addExternalSheet(clone $spreadsheet);

Besides being invalid PHP (spreadsheet isn't a variable, no $), it's also an invalid object type to pass to the addExternalSheet() method, which requires a Worksheet object, not a Spreadsheet object.

alaindev commented 3 years ago

OK Mark Please note that youcan spell a variable $xxx the way uou want! The pseudo code was indeed not good. Please check //load Excel 1 $objPHPExcel= \PhpOffice\PhpSpreadsheet\IOFactory::load($excelfile_1);

// load Excel 2 $spreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load($excelfile_2); $addedsheet = $spreadsheet->getSheet(1); // get 2nd sheet from Excel 2

// add sheet to Excel1 $objPHPExcel->addExternalSheet(clone $addedsheet );

$objWriter = new \PhpOffice\PhpSpreadsheet\Writer\Html($objPHPExcel); echo $objWriter->generateHTMLAll(); // will crash in Calculation.php 3246 // $this->spreadsheet->getSheetByName($cellAddress['sheet'])->getCell($cellAddress['cell']);

MarkBaker commented 3 years ago

Unable to replicate the problem: I'm using the following code, with two spreadsheets (the first one a multi-sheet spreadsheet) that contain some quite complicated formulae, and it's writing all formats (xlsx, xls, ods and html) as I would expect.

use PhpOffice\PhpSpreadsheet\IOFactory;
use PhpOffice\PhpSpreadsheet\Spreadsheet;

error_reporting(E_ALL);
set_time_limit(0);

date_default_timezone_set('UTC');

// Adjust the path as required to reference the Autoloader file
require_once __DIR__ . '/../vendor/autoload.php';

$inputFileType1 = 'Xlsx';
$fileBaseName1 = 'NamedRangeTest';

$inputFileType2 = 'Xlsx';
$fileBaseName2 = 'dataBarTest';

$spreadsheet1 = loadSpreadsheet($inputFileType1, $fileBaseName1);
$spreadsheet2 = loadSpreadsheet($inputFileType2, $fileBaseName2);

$mergeSheet = $spreadsheet2->getActiveSheet();

$spreadsheet1->addExternalSheet($mergeSheet);

saveMergedSheet($spreadsheet1, 'mergeTest');

// Echo memory usage
echo ' Current memory usage: ' , (memory_get_usage(true) / 1024) , ' KB' , PHP_EOL;
echo '    Peak memory usage: ' , (memory_get_peak_usage(true) / 1024) , ' KB' , PHP_EOL;

function loadSpreadsheet(string $inputFileType, string $inputFileName): Spreadsheet {
    $extension = strtolower($inputFileType);
    $inputFileName = __DIR__ . "/{$inputFileName}.{$extension}";

    $callStartTime = microtime(true);

    $reader = IOFactory::createReader(ucfirst($inputFileType));
    $spreadsheet = $reader->load($inputFileName);

    $callEndTime = microtime(true);
    $loadCallTime = $callEndTime - $callStartTime;

    echo PHP_EOL;
    echo "Call time to read {$inputFileName}.{$extension} was " , sprintf('%.4f', $loadCallTime) , ' seconds' , PHP_EOL;

    return $spreadsheet;
}

function saveMergedSheet(Spreadsheet $spreadsheet, string $outputFileName)
{
    saveFileType($spreadsheet, $outputFileName, 'xlsx');
    saveFileType($spreadsheet, $outputFileName, 'xls');
    saveFileType($spreadsheet, $outputFileName, 'ods');
    saveFileType($spreadsheet, $outputFileName, 'html');
}

function saveFileType(Spreadsheet $spreadsheet, string $fileName, string $extension)
{
    if ($extension === 'html') {
        $objWriter = new \PhpOffice\PhpSpreadsheet\Writer\Html($spreadsheet);
        echo $objWriter->generateHTMLAll();

        return;
    }

    $callStartTime = microtime(true);

    $writer = IOFactory::createWriter($spreadsheet, ucfirst($extension));
    $writer->save("{$fileName}.{$extension}");

    $callEndTime = microtime(true);
    $saveCallTime = $callEndTime - $callStartTime;

    echo PHP_EOL;
    echo "Call time to write {$extension} file to {$fileName} was " , sprintf('%.4f', $saveCallTime) , ' seconds' , PHP_EOL;
}
MarkBaker commented 3 years ago

Is it possible that the merge has forced the added worksheet to be renamed to avoid duplicate sheet names?