From 8af99633505719c265a8880272bce7c96e266519 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:13:55 -0800 Subject: [PATCH 1/2] Allow Spreadsheet Serialization Fix #4324. Serialization was explicity forbidden by PR #3199. This was in response to several issues, and concern that the Spreadsheet object contained non-serializable properties. This PR restores the ability to serialize a spreadsheet. Json serialization remains unsupported. Fix #1757, closed in Nov. 2023 but just reopened. At the time, Cell property `formulaAttributes` was stored as a SimpleXmlElement. Dynamic arrays PR #3962 defined that property as `null|array` in the doc block. However, it left the formal Php type for the property as `mixed`. This PR changes the formal type to `?array`. Fix #1741, closed in Dec. 2020 but just reopened. Calculation property `referenceHelper` was defined as static, and, since static properties don't take part in serialization, this caused a problem after unserialization. There are at least 3 trivial ways to deal with this - make it an instance property, reinitialize it when unserialized using a wakeup method, or remove the property altogether. This PR uses the last of those 3. Calculation does have other static properties. Almost all of these deal with locale. So serialize/unserialize might wind up using a default locale when non-default is desired (but not necessarily required). If that is a problem for end-users, it will be a new one, and I will work on a solution if and when the time comes. Static property `returnArrayAsType` is potentially problematic. However, instance property `instanceArrayReturnType` is the preferred method of handling this, and using that will avoid any problems. Issue #932 also dealt with serialization. I do not have the wherewithal to investigate that issue. If it is not solved by this and the earlier PR's, I will have to leave it to others to re-raise it. Spreadsheet `copy` is now simplified to use serialize followed by unserialize. Formal tests are added. In addition, I have made a number of informal tests on very complicated spreadsheets, and it has performed correctly for all of them. --- .../Calculation/Calculation.php | 19 ++-- src/PhpSpreadsheet/Cell/Cell.php | 2 +- src/PhpSpreadsheet/Spreadsheet.php | 23 +---- src/PhpSpreadsheet/Worksheet/Worksheet.php | 1 - .../SpreadsheetSerializeTest.php | 96 +++++++++++++++++++ tests/PhpSpreadsheetTests/SpreadsheetTest.php | 18 ---- .../Worksheet/CloneTest.php | 16 +++- 7 files changed, 117 insertions(+), 58 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/SpreadsheetSerializeTest.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index c31ea2c749..179aca630d 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -133,11 +133,6 @@ class Calculation */ public ?string $formulaError = null; - /** - * Reference Helper. - */ - private static ReferenceHelper $referenceHelper; - /** * An array of the nested cell references accessed by the calculation engine, used for the debug log. */ @@ -2890,7 +2885,6 @@ public function __construct(?Spreadsheet $spreadsheet = null) $this->cyclicReferenceStack = new CyclicReferenceStack(); $this->debugLog = new Logger($this->cyclicReferenceStack); $this->branchPruner = new BranchPruner($this->branchPruningEnabled); - self::$referenceHelper = ReferenceHelper::getInstance(); } private static function loadLocales(): void @@ -5732,11 +5726,14 @@ private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksh $recursiveCalculationCellAddress = $recursiveCalculationCell->getCoordinate(); // Adjust relative references in ranges and formulae so that we execute the calculation for the correct rows and columns - $definedNameValue = self::$referenceHelper->updateFormulaReferencesAnyWorksheet( - $definedNameValue, - Coordinate::columnIndexFromString($cell->getColumn()) - 1, - $cell->getRow() - 1 - ); + $definedNameValue = ReferenceHelper::getInstance() + ->updateFormulaReferencesAnyWorksheet( + $definedNameValue, + Coordinate::columnIndexFromString( + $cell->getColumn() + ) - 1, + $cell->getRow() - 1 + ); $this->debugLog->writeDebugLog('Value adjusted for relative references is %s', $definedNameValue); diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php index 89321abeda..c142b06b78 100644 --- a/src/PhpSpreadsheet/Cell/Cell.php +++ b/src/PhpSpreadsheet/Cell/Cell.php @@ -63,7 +63,7 @@ class Cell implements Stringable * * @var null|array */ - private mixed $formulaAttributes = null; + private ?array $formulaAttributes = null; private IgnoredErrors $ignoredErrors; diff --git a/src/PhpSpreadsheet/Spreadsheet.php b/src/PhpSpreadsheet/Spreadsheet.php index 943db95cc3..c438e32983 100644 --- a/src/PhpSpreadsheet/Spreadsheet.php +++ b/src/PhpSpreadsheet/Spreadsheet.php @@ -7,15 +7,12 @@ use PhpOffice\PhpSpreadsheet\Cell\IValueBinder; use PhpOffice\PhpSpreadsheet\Document\Properties; use PhpOffice\PhpSpreadsheet\Document\Security; -use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader; use PhpOffice\PhpSpreadsheet\Shared\Date; -use PhpOffice\PhpSpreadsheet\Shared\File; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; use PhpOffice\PhpSpreadsheet\Style\Style; use PhpOffice\PhpSpreadsheet\Worksheet\Iterator; use PhpOffice\PhpSpreadsheet\Worksheet\Table; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; -use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter; class Spreadsheet implements JsonSerializable { @@ -1042,17 +1039,7 @@ public function getWorksheetIterator(): Iterator */ public function copy(): self { - $filename = File::temporaryFilename(); - $writer = new XlsxWriter($this); - $writer->setIncludeCharts(true); - $writer->save($filename); - - $reader = new XlsxReader(); - $reader->setIncludeCharts(true); - $reloadedSpreadsheet = $reader->load($filename); - unlink($filename); - - return $reloadedSpreadsheet; + return unserialize(serialize($this)); } public function __clone() @@ -1516,14 +1503,6 @@ public function reevaluateAutoFilters(bool $resetToMax): void } } - /** - * @throws Exception - */ - public function __serialize(): array - { - throw new Exception('Spreadsheet objects cannot be serialized'); - } - /** * @throws Exception */ diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 546a6ffe5b..ce69863120 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -381,7 +381,6 @@ public function __destruct() public function __wakeup(): void { $this->hash = spl_object_id($this); - $this->parent = null; } /** diff --git a/tests/PhpSpreadsheetTests/SpreadsheetSerializeTest.php b/tests/PhpSpreadsheetTests/SpreadsheetSerializeTest.php new file mode 100644 index 0000000000..8128929e4e --- /dev/null +++ b/tests/PhpSpreadsheetTests/SpreadsheetSerializeTest.php @@ -0,0 +1,96 @@ +spreadsheet !== null) { + $this->spreadsheet->disconnectWorksheets(); + $this->spreadsheet = null; + } + } + + public function testSerialize(): void + { + $this->spreadsheet = new Spreadsheet(); + $sheet = $this->spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValue(10); + + $serialized = serialize($this->spreadsheet); + $newSpreadsheet = unserialize($serialized); + self::assertInstanceOf(Spreadsheet::class, $newSpreadsheet); + self::assertNotSame($this->spreadsheet, $newSpreadsheet); + $newSheet = $newSpreadsheet->getActiveSheet(); + self::assertSame(10, $newSheet->getCell('A1')->getValue()); + $newSpreadsheet->disconnectWorksheets(); + } + + public function testNotJsonEncodable(): void + { + $this->spreadsheet = new Spreadsheet(); + + $this->expectException(SpreadsheetException::class); + $this->expectExceptionMessage('Spreadsheet objects cannot be json encoded'); + json_encode($this->spreadsheet); + } + + /** + * These tests are a bit weird. + * If prepareSerialize and readSerialize are run in the same + * process, the latter's assertions will always succeed. + * So to demonstrate that the + * problem is solved, they need to run in separate processes. + * But then they can't share the file name. So we need to send + * the file to a semi-hard-coded destination. + */ + private static function getTempFileName(): string + { + $helper = new Sample(); + + return $helper->getTemporaryFolder() . '/spreadsheet.serialize.test.txt'; + } + + public function testPrepareSerialize(): void + { + $this->spreadsheet = new Spreadsheet(); + $sheet = $this->spreadsheet->getActiveSheet(); + $this->spreadsheet->addNamedRange(new NamedRange('summedcells', $sheet, '$A$1:$A$5')); + $sheet->setCellValue('A1', 1); + $sheet->setCellValue('A2', 2); + $sheet->setCellValue('A3', 3); + $sheet->setCellValue('A4', 4); + $sheet->setCellValue('A5', 5); + $sheet->setCellValue('C1', '=SUM(summedcells)'); + $ser = serialize($this->spreadsheet); + $this->spreadsheet->disconnectWorksheets(); + $outputFileName = self::getTempFileName(); + self::assertNotFalse( + file_put_contents($outputFileName, $ser) + ); + } + + #[Attributes\RunInSeparateProcess] + public function testReadSerialize(): void + { + $inputFileName = self::getTempFileName(); + $ser = (string) file_get_contents($inputFileName); + unlink($inputFileName); + $this->spreadsheet = unserialize($ser); + $sheet = $this->spreadsheet->getActiveSheet(); + self::assertSame('=SUM(summedcells)', $sheet->getCell('C1')->getValue()); + self::assertSame(15, $sheet->getCell('C1')->getCalculatedValue()); + } +} diff --git a/tests/PhpSpreadsheetTests/SpreadsheetTest.php b/tests/PhpSpreadsheetTests/SpreadsheetTest.php index 5fbeb4fa4e..f6556a5367 100644 --- a/tests/PhpSpreadsheetTests/SpreadsheetTest.php +++ b/tests/PhpSpreadsheetTests/SpreadsheetTest.php @@ -291,22 +291,4 @@ public function testAddExternalRowDimensionStyles(): void self::assertEquals($countXfs + $index, $sheet3->getCell('A2')->getXfIndex()); self::assertEquals($countXfs + $index, $sheet3->getRowDimension(2)->getXfIndex()); } - - public function testNotSerializable(): void - { - $this->spreadsheet = new Spreadsheet(); - - $this->expectException(Exception::class); - $this->expectExceptionMessage('Spreadsheet objects cannot be serialized'); - serialize($this->spreadsheet); - } - - public function testNotJsonEncodable(): void - { - $this->spreadsheet = new Spreadsheet(); - - $this->expectException(Exception::class); - $this->expectExceptionMessage('Spreadsheet objects cannot be json encoded'); - json_encode($this->spreadsheet); - } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php b/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php index b66a9dd8de..415aebadec 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php @@ -44,12 +44,18 @@ public function testGetCloneIndex(): void public function testSerialize1(): void { - // If worksheet attached to spreadsheet, can't serialize it. - $this->expectException(SpreadsheetException::class); - $this->expectExceptionMessage('cannot be serialized'); $spreadsheet = new Spreadsheet(); $sheet1 = $spreadsheet->getActiveSheet(); - serialize($sheet1); + $sheet1->getCell('A1')->setValue(10); + $serialized = serialize($sheet1); + $newSheet = unserialize($serialized); + self::assertInstanceOf(Worksheet::class, $newSheet); + self::assertSame(10, $newSheet->getCell('A1')->getValue()); + self::assertNotEquals($newSheet->getHashInt(), $sheet1->getHashInt()); + self::assertNotNull($newSheet->getParent()); + self::assertNotSame($newSheet->getParent(), $sheet1->getParent()); + $newSheet->getParent()->disconnectWorksheets(); + $spreadsheet->disconnectWorksheets(); } public function testSerialize2(): void @@ -57,8 +63,8 @@ public function testSerialize2(): void $sheet1 = new Worksheet(); $sheet1->getCell('A1')->setValue(10); $serialized = serialize($sheet1); - /** @var Worksheet */ $newSheet = unserialize($serialized); + self::assertInstanceOf(Worksheet::class, $newSheet); self::assertSame(10, $newSheet->getCell('A1')->getValue()); self::assertNotEquals($newSheet->getHashInt(), $sheet1->getHashInt()); } From e58dd89642c255df65a73ab61fee633590789b40 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 7 Feb 2025 08:43:18 -0800 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab7eff4146..6d0bcdc352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Added - Pdf Charts and Drawings. [Discussion #4129](https://github.com/PHPOffice/PhpSpreadsheet/discussions/4129) [Discussion #4168](https://github.com/PHPOffice/PhpSpreadsheet/discussions/4168) [PR #4327](https://github.com/PHPOffice/PhpSpreadsheet/pull/4327) +- Allow spreadsheet serialization. [Discussion #4324](https://github.com/PHPOffice/PhpSpreadsheet/discussions/4324) [Issue #1741](https://github.com/PHPOffice/PhpSpreadsheet/issues/1741) [Issue #1757](https://github.com/PHPOffice/PhpSpreadsheet/issues/1757) [PR #4326](https://github.com/PHPOffice/PhpSpreadsheet/pull/4326) ### Removed